Skip to content

feat(responses): synthesize reasoning.encrypted_content when requested - #342

Open
ranst91 wants to merge 12 commits into
mainfrom
feat/responses-encrypted-reasoning
Open

feat(responses): synthesize reasoning.encrypted_content when requested#342
ranst91 wants to merge 12 commits into
mainfrom
feat/responses-encrypted-reasoning

Conversation

@ranst91

@ranst91 ranst91 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

What / why (plain)

Ask a reasoning model (gpt-5.x) over the OpenAI Responses API to call two tools in a row, replayed statelessly, and agent-framework-openai >= 1.11.0 crashes on the second tool call. aimock is the trigger: it never modeled OpenAI's encrypted-reasoning blob, so a replayed reasoning item comes back with no blob, and that client now hard-fails instead of degrading.

This makes aimock emit the blob (like real OpenAI does), so the client can replay the reasoning item and the chain completes. Unblocks the CopilotKit showcase ms-agent-python tool-rendering-reasoning-chain D6 cell on agent-framework 1.0 without pinning the framework back.

Background

Real OpenAI returns an opaque reasoning.encrypted_content on reasoning items only when the request opts in via include: ["reasoning.encrypted_content"] — which is exactly what agent-framework-openai >= 1.11.0 sends on its stateless-replay path. Without the blob, that client parses a reasoning item that has an id but no protected_data, and a new preflight rejects the follow-up request before it is sent (root cause: microsoft/agent-framework#7233). aimock had no representation of this field at any layer (record, schema, replay), so re-recording fixtures could not fix it.

Change

  • Synthesize a deterministic placeholder encrypted_content on reasoning items, gated on the request's include containing reasoning.encrypted_content.
  • Gating keeps the blast radius to the agent-framework stateless path only — every other consumer's replay is byte-identical (default off).
  • Covers both the streaming and non-streaming reasoning builders.
  • The echoed-back blob is ignored by aimock's content-based matching, so any stable string is safe.

Verification

  • Build, lint clean. Full suite: 4772 pass / 3 skip (+4 new tests here).
  • End-to-end over a real network round-trip: agent-framework-core==1.12.1 / agent-framework-openai==1.11.0 (the versions that crash) against this build → reasoning protected_data is populated and the stateless follow-up request builds without raising.

Note: aimock has no real ciphertext; the placeholder only needs to be present and stable, which is sufficient for replay.

Real OpenAI returns an opaque `encrypted_content` blob on reasoning items only
when a request opts in via `include: ["reasoning.encrypted_content"]` (the
stateless-replay path used by agent-framework-openai >= 1.11.0). aimock never
modeled it, so a replayed reasoning item carried no blob. On that path
agent-framework-openai >= 1.11.0 hard-fails a reasoning + multi-tool chain on
the follow-up request (microsoft/agent-framework#7233): it parses a reasoning
item with an id but no protected_data and rejects the request before sending.

Emit a deterministic synthetic encrypted_content on reasoning items, gated on
the request's `include` so only opted-in requests see it (mirrors real OpenAI;
every other consumer's replay stays byte-identical). Covers the streaming and
non-streaming reasoning builders.
@pkg-pr-new

pkg-pr-new Bot commented Jul 28, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@copilotkit/aimock@342

commit: ca41785

@jpr5 jpr5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to reflect 1157c54. Most of my original list is resolved or was wrong — see the follow-up comment for the retraction of the added-placement finding and the assistantCount concession. What remains blocking:

1. WS encrypted-reasoning path has zero assertionssrc/ws-responses.ts:176,177,180,291,335,380. Deleting the include/store threading is a two-line change that leaves all 4732 tests green; hardcoding the gate to false also passes. CHANGELOG.md:7 claims HTTP+WS parity. ws-responses.test.ts already has reasoning fixtures to build on. (The wiring itself works — I drove it and WS matches HTTP event-for-event. It's untested, not broken.)

2. Three emission sites unassertedsrc/responses.ts:1040 (non-streaming blocks branch), :1366 (streaming content+tool), :1482 (streaming tool-only). All three are live code: a wire probe shows pristine emits the blob at each, and removing it at any one site flips only that site to encrypted_content absent while the suite stays green. :1366/:1482 are the streaming reasoning + multi-tool path this PR is about.

Related: the test named …carries the blob (buildContentWithToolCallsResponse) never enters that function — its fixture has no blocks, so it takes the legacy branch into buildOutputPrefix.

3. src/responses.ts:483 couples the blob to summary presence. emitEncrypted: true silently no-ops unless summaryText !== undefined, so summary: [] with a populated blob is inexpressible — and that is exactly the shape the real captures show at added (844-char blob, empty summary). Concrete effect: a fixture whose response is output: ["function_call"] with no reasoning string emits no blob even with store: false and include set, so recorded fixtures lacking a summary get nothing. An explicit phase argument would express both shapes without the implicit invariant.

4. The new drift leg cannot fail. Three independent reasons: it posts to a local aimock server with gpt-4o-mini — denylisted as non-reasoning at model-utils.ts:46 — and never reaches api.openai.com; a plain expect failure inside a .drift.ts is console.warn-swallowed whenever any drift entry exists (scripts/drift-report-collector.ts:920); and it runs under a separate vitest config, so it is in neither pnpm test nor test-unit.yml. sdk-shapes.ts:459 still has no encrypted_content pin, so there is no spec anchor.

Unrelated to this PR but worth knowing: .github/workflows/test-drift.yml has no concurrency group, so every push to a drift-path PR starts another ~30-minute two-run paid live-API job with nothing cancelling the previous one. And at :565, BASE_CONCLUSION="$BASE_CONCLUSION" sits after node -e, so it is argv rather than env — process.env.BASE_CONCLUSION is always undefined and the gh run view fallback is dead code.

…overage

Follow-up to review on #342:

- encrypted_content now only on the terminal reasoning item (done /
  non-streaming output[]), never on `added` — matches real OpenAI and avoids
  masking clients that read reasoning at `added` (langchain-ai/langchainjs#10844).
- Gate on `include: ["reasoning.encrypted_content"]` OR stateless `store: false`
  (openai-python 2.46.0 made the blob default-on for non-stored responses); was
  include-only.
- Wire the WebSocket Responses transport too (export the gate, thread `include`
  / `store` through the rebuilt request) so #7233 doesn't still reproduce over WS.
- base64 the placeholder (opaque like the real field); declare `include` / `store`
  on ResponsesRequest instead of casting; drop the unused `model` param; collapse
  the 5 hand-built reasoning items into one `buildReasoningOutputItem`.
- Tests: pin added-vs-done placement, id→blob determinism, both gate triggers,
  response.completed.output, and all three non-streaming builders; add an
  opted-in drift leg. CHANGELOG entry added.
@ranst91

ranst91 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review @jpr5, all comments addressed, but the 3 I left out of scope (pre-existing / unrelated, happy to file separately):

  1. Parallel multi-tool assistantCount mismatch — pre-existing router behavior; this PR only makes the shape reachable, didn't introduce it.
  2. No typecheck in CI — build + eslint pass on a type error; CI/tooling gap unrelated to reasoning.
  3. stream-collapse drops delta.reasoning — different provider + code path; its own bug.

@jpr5 jpr5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrections to my earlier review. Trimming this down to just the parts that are mine to fix.

Retracting my finding 1 — added placement. I was wrong, and you implemented it.

Real OpenAI does populate encrypted_content on response.output_item.added. Recorded fixtures in vercel/ai (openai-reasoning-encrypted-content.1, programmatic-tool-calling.1, an Azure one, and openai-compaction.1) carry it at added: 844 chars at added → 1060 at done → 1060 at completed, done and completed differing in bytes but not length (same plaintext, fresh Fernet IV).

I checked provenance this time. Every blob is a valid Fernet token whose embedded timestamp lands 0–4s after its own file's created_at; ciphertext is AES-CBC block-aligned in 9 of 9 tokens; entropy 5.94/6.0; the files come from a save-raw-chunks.ts recording harness and carry obfuscation SSE padding. Those are recordings, not hand-written mocks.

The three sources I originally cited don't hold up:

  • langchainjs#10844 is a hand-written msw mock — encrypted_content: "ENCRYPTED_BLOB", ids rs_1. And the reporter's premise is wrong independent of that: the converter's added handler is a {id, type, summary} whitelist, so the bug reproduces identically whether or not added is populated. My "aimock would mask a live client bug" argument was wrong twice over.
  • pydantic-ai passes on reasoning at added by design; its "done replaces this" comment is about compaction and points the other way.
  • microsoft/agent-framework foundry_hosting/_responses.py is a server emitter under azure.ai.agentserver, not a mirror of OpenAI — and it emits status="in_progress" on reasoning items, which real OpenAI never does.

I treated client read-paths, another vendor's server, and issue-thread prose as evidence about OpenAI's wire bytes. They aren't.

Please don't invert the code back. The spec declares the field anyOf: [string, null] and non-required, no consumer requires it at added, and the sensible client guidance is to read at done and treat added as opportunistic — so omitting it is legal and harmless. The thing worth changing is that the new unit test and drift leg pin absence at added as if it were the contract (a mutation that emits at added is currently caught). Loosening those to "absent or a string" would stop the suite enforcing the less-faithful shape.

The store: false branch is right; my justification for it wasn't

Keep the branch — store defaults to true (openapi.yaml:35922), absent store → no blob in 3/3 captures, store: false → blob in 2/2. But openai-python 2.46.0's note says "populated by default" and never mentions store, and agent-framework-openai 1.11.0 never sends store at all — it auto-appends include (_chat_client.py:1390-1400). So the branch is inert for every real client, and the docstring at :436 overstates it as a server-side default trigger.

You were right about the assistantCount item — dropped

The defect is real (journal ground truth: 2 parallel tools → assistant count 2, single tool → 1, off by N−1), but git diff f988b1c HEAD touches neither responsesInputToMessages nor router.ts, and the 2-turn chain completes. Pre-existing, not yours, and out of scope here as you said.

One detail for whenever it gets filed: in default mode it doesn't 503 — it returns HTTP 200 with the wrong fixture, silently skipping a turn (a 3-turn script served leg 3 where leg 1 was wanted). Only AIMOCK_STRICT_TURN_INDEX=1 makes it a visible no_fixture_match. The silent path is the nastier half.

ranst91 and others added 4 commits July 29, 2026 23:59
…uple blob

Follow-up to jpr5's second pass on #342:

- Decouple the blob from summary presence: `buildReasoningOutputItem` now emits
  encrypted_content on `emitEncrypted` alone (empty-summary + blob is expressible).
- WS transport coverage: new ws-responses.test.ts cases assert the blob over the
  WebSocket surface for content+reasoning and tool+reasoning, both gate triggers
  (include / store:false), and absence when not opted in. (Mutation-checked:
  hardcoding the WS gate to false fails 3 tests.)
- Streaming coverage for every builder: buildTextStreamEvents,
  buildToolCallStreamEvents, buildContentWithToolCallsStreamEvents (legacy +
  blocks branches) + response.completed.output; and the non-streaming blocks
  branch of buildContentWithToolCallsResponse (the old test never entered it).
  (Mutation-checked: dropping the blob fails 10 tests.)
- `added` placement is no longer pinned as absence (real OpenAI populates it;
  aimock omits it — both legal). Flag-off still pins genuine absence.
- Drift: removed the ineffective local leg (couldn't fail); added the real spec
  anchor (encrypted_content on the terminal item in sdk-shapes.ts).
- Docstring: correct the store-branch wording (include is the real-client
  trigger; store:false is the secondary/captured one).
… pass

The `drift-live-pr` required check failed on every run of this branch with
`[critical] item.encrypted_content ... Mock: <absent>`, and no code change
could have cleared it.

`openaiResponsesReasoningEventShapes()` pinned `encrypted_content` on the
terminal reasoning item unconditionally, but its only consumer — the
"triangulate against SDK expectations" leg — posts an OPTED-OUT request (no
`include`, no `store: false`). aimock correctly emits no blob there:
`requestWantsEncryptedReasoning()` is false, so `buildReasoningOutputItem`
omits the field by design. Triangulating an unconditional SDK pin against a
legitimately-absent mock field lands in triangulate()'s "in SDK + real but
not mock" branch, which is always critical.

So the pin asserted the mock had a field the request never asked for. It
reported critical on every run regardless of whether the emission worked,
which meant it gated nothing — and because the key was new relative to main,
computeDelta() classified it as block[] (new-in-head), failing the required
check outright rather than degrading to an advisory.

Fix follows the harness's existing convention for variant shapes (separate
parameterless functions, as in fal-queue-contract.ts and the realtime
text/beta pair) rather than an options bag, and mirrors production's own
`buildReasoningOutputItem({ emitEncrypted })` gate:

- `openaiResponsesReasoningEventShapes()` — no blob; grades opted-out legs.
- `openaiResponsesEncryptedReasoningEventShapes()` — blob; grades opted-in.

The existing leg now pins the no-blob variant matching the request it sends,
and a new leg posts `include: ["reasoning.encrypted_content"]` (what
agent-framework-openai >= 1.11.0 sends on stateless replay) and pins the blob
variant. That new leg is what actually gates the emission: deleting the
`encrypted_content` assignment in src/responses.ts flips it critical while the
opted-out leg stays clean. Verified locally — collector exit 2 -> 0, delta gate
BLOCKED -> PASSED, and the sabotage re-reds only the opted-in leg.

No production behavior change.
…uilders

The existing suite proved the `reasoning.encrypted_content` blob IS emitted
where it should be, but nothing proved it is NOT emitted when the request never
opted in. That is the more dangerous direction for a mock: an unconditional
emission silently changes response bytes for every consumer that sent neither
`include: ["reasoning.encrypted_content"]` nor `store: false`, breaking the
byte-identical stored/opted-out replay.

Forcing `emitEncrypted: true` at each of the three non-streaming sites left the
entire suite green (4741 passed), so all three were genuinely uncovered:

- `buildOutputPrefix` (text + content+tool legacy)
- `buildToolCallResponse` (tool-only)
- the `blocks` branch of `buildContentWithToolCallsResponse`

Add one negative per site, reusing the existing non-streaming case table, each
pinning genuine key ABSENCE via `not.toHaveProperty` rather than a falsy value.
Every negative fails under exactly its own site's mutation and no other.

Also add the missing WS case for the content+toolCalls builder — the WS dispatch
threads the gate into each streaming builder separately, and dropping the flag
from that one call site left the existing text and tool-only WS cases green.

No production code changed.
The entry said the blob is withheld from the in-progress `added` item
"matching real OpenAI". That is backwards, and the claim originated in my
review feedback on this PR, not from the author. Recorded OpenAI captures carry
a populated blob on `response.output_item.added` beside an empty summary,
re-encrypted by `done`; the code comments added in a3a5446 already say so, so
the changelog was the last place still asserting parity.

The langchainjs#10844 rationale does not hold either: that payload is a
hand-written msw mock, and its converter whitelists {id, type, summary} at
`added`, so the reported bug reproduces whether or not `added` is populated.

Withholding at `added` is a deliberate, contract-legal simplification (the
field is `anyOf: [string, null]` and non-required, and no known consumer needs
it), so the entry now describes it that way. Also softens "hard-failing" to
"related": without the blob agent-framework-openai skips the reasoning item
rather than hard-failing the request.
@jpr5

jpr5 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Pushed three commits to your branch to close out the review items. Production code is untouched — git diff a3a5446..39e8244 -- src/responses.ts src/ws-responses.ts is empty. The implementation stays yours.

Lead with this one, because it was costing you a red CI: drift-live-pr was failing at a3a5446, and it wasn't flaky or unrelated. The encrypted_content pin you added to openaiResponsesReasoningEventShapes() was unconditional, but the only leg consuming that shape posts an opted-OUT request (no include, no store: false) — where aimock correctly emits nothing. So triangulate() took its "in SDK + real API but missing from mock" branch on every run and reported [critical] item.encrypted_content … Mock: <absent>, regardless of whether the emission worked. Being new-in-head, that critical key also went into block[] and stopped computeDelta (scripts/drift-delta.ts:99-105).

57ad4c0 splits it the way the harness models variants elsewhere (fal-queue-contract.ts, realtime text/beta, gemini chunk/lastChunk): a private reasoningEventShapes(emitEncrypted) behind openaiResponsesReasoningEventShapes() (no blob) and openaiResponsesEncryptedReasoningEventShapes() (blob), with an added opted-IN leg that sends include. Verified by feeding the real CI base artifact through computeDelta: BLOCKED with 1 block key → PASSED with 0. And it still gates — deleting the emission in responses.ts re-reds at collector exit 2, with only the opted-in leg failing.

fa264d5 adds the leak direction. Your coverage proves the blob is emitted where it should be; nothing proved it isn't emitted when nobody asked. Forcing emitEncrypted: true at buildOutputPrefix, buildToolCallResponse, or the blocks branch each left the full suite green. Four negatives now catch that, plus a WS case for content+toolCalls — there was no fixture combining content + toolCalls + reasoning, so buildContentWithToolCallsStreamEvents was never reached over WS.

39e8244 fixes the CHANGELOG, which still claimed the blob is withheld from added "matching real OpenAI". That sentence was wrong, and it was wrong because I told you it was true in my first review — your own code comments in a3a5446 already had it right.

On that, and on two other things I got wrong:

The added-placement finding was my worst call. I cited langchainjs#10844, pydantic-ai, and agent-framework's foundry_hosting server. On checking properly: #10844's payload is a hand-written msw mock (encrypted_content: "ENCRYPTED_BLOB", ids rs_1), and its converter whitelists {id, type, summary} at added, so that bug reproduces either way; pydantic-ai skips reasoning at added by design; and foundry_hosting is a server emitter under azure.ai.agentserver that emits status="in_progress", which real OpenAI reasoning items never carry. Recorded captures in vercel/ai show a populated 844-char blob at added, re-encrypted by done. Since the field is anyOf: [string, null] and non-required, your omission is legal — but it's aimock's choice, not upstream parity, and your done-only pin rationale in sdk-shapes.ts says that better than my review did.

The assistantCount item: you were right, I dropped it. Pre-existing, router.ts and responsesInputToMessages untouched by this PR, and the 2-turn chain completes. One detail for whenever it gets filed — in default mode it doesn't 503, it returns 200 with the wrong fixture, silently skipping a turn; only AIMOCK_STRICT_TURN_INDEX=1 surfaces it.

And your one-line if (opts.emitEncrypted) is a better decoupling than the discriminated union I'd built for it.

CI at 39e8244: 25 success, 2 skipped, 0 failing. Suite 4790 passed / 3 skipped across three consecutive runs.

Two things I did not fix, both pre-existing and out of scope here: .github/workflows/test-drift.yml has no concurrency group, so every push to a drift-path PR starts another ~30-minute two-run paid live-API job with nothing cancelling the last; and proxy-buffer-cap.test.ts runs 3.2–4.3s against a 5s timeout, so it will flake in CI under load.

jpr5 added 6 commits July 29, 2026 18:02
Every reasoning item aimock emitted originated in a fixture's declared
`reasoning` summary text: all five `buildReasoningOutputItem` call sites
sat inside an `if (reasoning)` branch. So a request that opted in via
`include: ["reasoning.encrypted_content"]` against a fixture with no
declared summary got no reasoning item, and therefore no blob — starving
the exact case the feature exists to serve, since a fixture RECORDED from
the real stateless agent-framework flow has no summary (OpenAI returns
`summary: []` unless summaries are requested).

Verified on live bytes before the change: an opted-in gpt-5 request
against a summary-less fixture returned `output: [message,
function_call]` with zero occurrences of `encrypted_content`, and the SSE
stream carried no reasoning events at all.

Wire rationale: real OpenAI, for a reasoning-capable model, returns a
reasoning item with `summary: []` and a populated `encrypted_content`
whether or not summaries were requested — the item is the carrier for
the blob the client must replay. So synthesize one, gated on:

  - the EXPLICIT `include` opt-in ONLY, which is NARROWER than the gate
    controlling the blob itself. Creating an output item that did not
    previously exist is a bigger behavior change than adding a field to
    one already on the wire, so it takes the gate directly observable in
    the request rather than the inferred `store: false` one, whose
    supporting capture evidence is not verifiable from this repo.
    `agent-framework-openai` >= 1.11.0 sends `include` and never sends
    `store`, so the narrowing costs the feature nothing.
  - the REQUESTED model's reasoning capability. gpt-4o has no reasoning
    channel, so an item there would be less faithful, not more. Unlike
    `resolveReasoningForModel`, which fails open for a declared summary
    (a recorded summary is evidence the model did reason), this gate is
    hard: nothing exists to preserve, so a synthesized item on a
    non-reasoning model would be pure fabrication.

The resulting matrix on a reasoning-capable model, all five rows pinned
by tests and verified on live bytes:

  include   + summary    → item with summary + blob   (unchanged)
  include   + NO summary → synthesized `summary: []` + blob   (new)
  store:F   + summary    → item with summary + blob   (unchanged)
  store:F   + NO summary → NO reasoning item          (unchanged)
  neither   + either     → no blob anywhere           (unchanged)

Streaming emits `output_item.added` → `output_item.done` with nothing
between: an empty `summary` has no parts, so a
`reasoning_summary_part`/`_text` event would describe a part that is not
on the item. The synthesized item takes `output_index` 0 through the
existing preamble accounting, so `nextOutputIndex` shifts the message /
function_call / web_search items exactly as a declared summary already
does, and every event's `output_index` still equals that item's slot in
`response.completed.output`. The recorder is unaffected: stream-collapse
derives `reasoning` only from `reasoning_summary_text.delta`, which a
summary-less item never emits, so a re-record round-trips to a
summary-less fixture rather than fabricating summary text.

Both transports are covered — the WS Responses path shares these
builders and computes both gates itself, so its rows are pinned
separately and the narrowing cannot regress on one transport only.
…ogy, stale claim)

The encrypted-reasoning suite had three defects that let real regressions
through. All three were confirmed by mutation before being fixed.

1. Leak direction covered only 5 of the 9 gate call sites. Hardcoding the
   emission ON at responses.ts:1371 (content+tool streaming), responses.ts:1487
   (tool-only streaming), ws-responses.ts:293 (WS content+tool) or
   ws-responses.ts:382 (WS tool-only) left the ENTIRE suite green — a silent
   byte-level leak for every consumer that never opted in. Both single-case
   negatives are replaced by a loop generating one negative per streaming
   builder, so the sweep now catches 9/9 sites. Negatives pin genuine key
   absence via not.toHaveProperty, not a falsy value.

2. "streaming reflects the blob in response.completed.output" was tautological.
   buildResponsePreamble pushes the output_item.done event's item object into
   prefixOutputItems, so completed.response.output's reasoning item is the SAME
   REFERENCE — comparing the two was undefined === undefined, and the test
   passed with the feature entirely disabled. It now pins the id-derived base64
   independently against completed.output.

3. The section header asserted real OpenAI "never" populates the in-progress
   `added` item. That is false (it does populate it opportunistically) and
   contradicted responses.ts's own note. Reworded to describe aimock's
   deliberate choice to carry the blob only on the terminal item — legal, since
   the field is anyOf:[string,null] and non-required, but not upstream parity.

Also replaces assertAddedNotPinned, a conditional expect whose branch could
never execute (aimock builds `added` with no emitEncrypted opt), so all five
call sites asserted nothing — proven by putting a throw in the branch and
watching 118/118 still pass. Now a real assertion of the omission.

Test-only change; src/responses.ts and src/ws-responses.ts are untouched.
… legs' delta keys

Three reviewer claims about 57ad4c0's drift gate; verified each locally before
changing anything. Two were real, one was overstated.

CLAIM 1 (paths filter omits src/responses.ts) — fact TRUE, conclusion FALSE. The
filter does omit it, but test-unit.yml has NO paths filter, so `pnpm test` runs on
every PR and its value pins hard-fail on a dropped/null/empty blob (11 tests red
in responses.test.ts + ws-responses.test.ts against a null blob). So this is a
cost/latency scoping choice, not a coverage hole — adding a graded production
file would pull in nearly all of src/ and run a ~30-min live-credit job on almost
every PR. Documented the deliberate scope on the filter instead of widening it.

CLAIM 2 (the two legs collide in delta keying) — TRUE. `computeDelta` keys by
`provider::id`, `parseDriftBlock` sets `id: path`, and `DriftEntry.scenario` never
enters the key. Verified against the real computeDelta: two entries differing only
by scenario collapse to ONE key, and an opted-OUT finding on base demotes a
new-in-head opted-IN finding at the same path from block[] to advisory[] — it
stops failing the required check. Fixed inside this leg (scenario+event-type path
prefix) rather than re-keying the shared delta machinery every surface depends on.

CLAIM 3 (the assertion passes on null; opted-out gates nothing) — TRUE, both
halves, reproduced against the live suite:
  - opted IN, blob `null` → schema.ts's null-vs-other exemption → leg GREEN.
  - opted IN, blob `""`   → still kind "string"                 → leg GREEN.
  - opted OUT, blob present → "MOCK EXTRA FIELD" grades `info`, and the legs fail
    only on `critical`                                          → leg GREEN.
Only outright ABSENCE was caught. `gradeEncryptedBlob` now grades the terminal
item's value in both directions and emits through formatDriftReport with the
openai-responses surface marker, so a violation is a routed exit-2 finding for the
delta gate rather than a bare expect failure the collector could only quarantine.

Red-green via the command test-drift.yml actually gates with
(`npx tsx scripts/drift-report-collector.ts --out drift-report-head.json`, then
the delta gate) — all four sabotages of src/responses.ts now flip collector
exit 0 -> 2 and delta gate PASSED -> BLOCKED, on distinct per-leg delta keys:
absent, null and empty block on `OpenAI Responses Encrypted Reasoning:...`, while
over-emission blocks on `OpenAI Responses Reasoning:...`. Production restored and
byte-identical afterwards.

No production behavior change.
…urviving helper

The merge of three branches left `src/__tests__/responses.test.ts` calling a
helper that no longer exists. `a3a5446` added `assertAddedNotPinned` plus two
call sites; `f8e5e3d` added a third (the summary-less streaming loop) from a
different branch; `ecb05df` renamed the helper to `expectAddedOmitsBlob` and
updated only the two call sites it could see. The third was orphaned.

`pnpm test` died with `ReferenceError: assertAddedNotPinned is not defined` on
4 tests. Nothing else catches it: `tsconfig.json` excludes `src/__tests__`, so
`tsc --noEmit` exits 0 on the broken tree, and CI has no typecheck job at all —
`pnpm test` was the only guard.

Before: Tests 4 failed | 4816 passed | 3 skipped (4823)
After:  Tests 4820 passed | 3 skipped (4823)
Every existing leak negative sends NO `include` at all, so the suite pinned
whether the blob gate reads `include` but never that it reads the RIGHT VALUE.
Broadening `requestIncludesEncryptedReasoning` from
`include.includes("reasoning.encrypted_content")` to any non-empty `include`
left all 4823 tests green, and a live aimock instance then handed the blob to a
request carrying `include: ["file_search_call.results"]` — byte-level
over-emission to a client that never asked for reasoning at all.

Adds four negatives using `file_search_call.results`, a real member of the SDK's
`ResponseIncludable` union (`'file_search_call.results' |
'message.input_image.image_url' | 'computer_call_output.output.image_url' |
'reasoning.encrypted_content'` in openai@4.104.0), with no `store` so the gate's
other branch cannot be what makes them pass:

  - HTTP streaming: no `encrypted_content` anywhere in the raw SSE bytes
  - HTTP non-streaming: same, on the JSON body
  - synthesis gate: an unrelated include value conjures no reasoning item
  - WS: same, over the WebSocket transport, which reads `include` off the
    parsed frame itself

RED-GREEN (`pnpm test`, full suite):

  mutate `requestIncludesEncryptedReasoning` body -> `include.length > 0`
    FAIL responses.test.ts > streaming omits the blob for a non-empty include...
    FAIL responses.test.ts > non-streaming omits the blob for a non-empty include...
    FAIL responses.test.ts > a non-empty include ... synthesizes NOTHING
    FAIL ws-responses.test.ts > omits the blob for a non-empty include...
    Tests 4 failed | 4820 passed | 3 skipped (4827)

  mutate `requestWantsEncryptedReasoning`'s include branch -> `include.length > 0`
    Tests 3 failed | 4821 passed | 3 skipped (4827)

  restored
    Test Files 166 passed (166) / Tests 4824 passed | 3 skipped (4827)
…added`

`expectAddedOmitsBlob` asserted that the in-progress `response.output_item.added`
reasoning item has NO `encrypted_content`, for OPTED-IN requests. That is wrong
to pin: real OpenAI populates `added` opportunistically, and the field is
`anyOf: [string, null]` and non-required — so aimock omitting it is legal, but it
is aimock's own CHOICE, not the provider contract. The commit that introduced the
helper said so in its own message and in the production comment on
`buildReasoningOutputItem`, then pinned the omission anyway. 9 tests would fail
any future change that made aimock MORE faithful.

The drift contract in this same tree already draws the line correctly, and this
brings the unit suite into line with it: `sdk-shapes.ts` anchors the blob on
`done` ONLY — "pinning `added` would flag an intentional, legal shape choice as
drift" — and `gradeEncryptedBlob` grades the terminal item alone.

Renamed to `expectAddedBlobIsContractLegal`, which now grades only what is
contractual for an opted-in request: absent is fine, and if present it must be a
string equal to the id-derived `expectedBlob(id)`. It does NOT assert presence,
so aimock's current omission keeps passing. Opted-out call sites are untouched —
there, absence genuinely IS the contract, and they assert it directly.

Nothing real is lost, verified by mutation rather than assumed (`pnpm test`):

  blob on `added` IN ADDITION to terminal (contract-legal, must survive)
    before: 9 failed  ->  after: Tests 4824 passed | 3 skipped (4827)

  blob on `added` INSTEAD of terminal (the property that matters, P1)
    still CAUGHT: 17 failed | 4807 passed — every failure via a terminal-item
    assertion, in both the HTTP and WS suites

  wrong blob VALUE on `added` when opted in (is the relaxed helper vacuous?)
    CAUGHT: 9 failed | 4815 passed — exactly the 9 call sites
@ranst91

ranst91 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Round 2 addressed in a3a5446 — thanks, and noted on the added retraction (kept it omitted, and stopped pinning its absence):

  • WS transport now covered — content+reasoning, tool+reasoning, both gate triggers, absence when not opted in (mutation-checked: hardcoding the WS gate to false fails 3).
  • Streaming + blocks — every streaming builder (text / tool-only / content+tool) + response.completed.output, and the non-streaming blocks branch the old test never entered (mutation-checked: dropping the blob fails 10).
  • Blob decoupled from summaryemitEncrypted alone; empty-summary + blob is expressible.
  • Drift — dropped the can't-fail local leg; added the spec anchor (encrypted_content on the terminal item in sdk-shapes.ts).
  • Docstring — corrected: include is the real-client trigger, store:false the secondary/captured one.

The 3 out-of-scope items from before still stand (the workflow concurrency / BASE_CONCLUSION nits you noted are unrelated too — happy to file all separately).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants