fix(responses): routed-destination sanitize + opaque-state recovery series (#2254 rebased) - #2258
Merged
Conversation
A replayed compaction item carries an `encrypted_content` blob only its minting backend can decode, and Codex replays it on every later turn. Two paths modified or misrouted it, and because the item outlives the failure in the client transcript, both wedged the session until its history was cleared — the routed compaction turn the proxy itself drives replays the same item. Relay: `scrubOcxCompactionItems` treated every non-`ocx1:` blob as OpenAI's and forwarded it verbatim, with no check that the destination was the issuer. A session that compacted on a canonical route and then switched to a routed provider sent that blob to an upstream that could only answer "Could not decode the compaction blob". Native blobs now travel only to destinations that mint them — forward-auth routes, which relay the caller's own OpenAI credentials to the ChatGPT backend or a relay in front of it, and the official OpenAI API under key auth — and degrade elsewhere to the same opaque note the bridged parser uses. Backfill: the response-side exemption list named `compaction` alone, so `compaction_summary` and `context_compaction` received synthesized ids that the client stored and replayed as "modified from the compact response". That divergence was possible because the compact wire family was enumerated in three places; it is now one predicate in `src/responses/compaction.ts`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ontent Codex replays the reasoning item it received in the next request's input, and a backend that issued `encrypted_content` verifies what comes back. The content-to-summary channel rewrite deletes `content` and substitutes a synthesized `summary`, so the client stored and replayed an item the issuer had never sent, and every later turn failed with "Could not decrypt the provided encrypted_content. Ensure the value is the unmodified encrypted_content from a previous response." No route change is needed to reach this: it fires on the second turn of a fresh session. The rewrite's replay round trip was verified against DeepSeek, which is `statelessResponses` and issues no blob — its reasoning replay goes through the proxy-side cache instead. Providers that do issue a blob joined the same route later through `preserveReasoningContentModels`, a flag whose own purpose is Chat-wire prompt-cache replay, and the verified premise did not follow them. Only the stored item is exempt. The `reasoning_text` delta events carry no blob and still route to the summary channel, so the expandable trace Codex renders for the live turn is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…assthrough
Codex serializes an absent reasoning content channel as `"content": null`, and
the sanitizer only acted on a non-empty array, so the null went to the wire
verbatim. xAI rejects the item and blames the sibling field:
{"code":"invalid-argument",
"error":"Could not decode the compaction blob. Ensure it is unmodified from
the compact response."}
The blob is not the problem. Captured from a live failing request and bisected
against it: replaying the body verbatim reproduces the 400, deleting only the
`content` key returns 200, and setting it to `[]` also returns 200 — while
removing `encrypted_content` instead fails schema validation, so the blob is
both required and intact. The proxy was verified not to alter the blob: the
value grok streamed to the client and the value replayed upstream matched in
length, prefix and suffix, under identical `x-grok-conv-id`, `x-grok-session-id`
and account.
This bites the second turn of every Grok conversation — the first request that
replays a reasoning item — which is why a fresh session fails just as reliably
as a resumed one, and why the error looked like stale compaction state.
The field is optional and null carries nothing, so the key is dropped rather
than rewritten; an array content channel still follows the existing rules.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rd auth Review found the discriminator unsound, and it was. `authMode === "forward"` describes local credential handling, not which backend answers: the adapter forwards caller credentials only to the canonical ChatGPT Codex surface, so a noncanonical forward provider receives none and may point anywhere. That produced both errors at once. A self-hosted or xAI-backed forward gateway was classified as able to decode a foreign blob, was sent it unchanged, and stayed wedged — the exact failure this branch exists to fix. Meanwhile a key-auth relay genuinely fronting OpenAI was classified as unable to decode and needlessly lost its compacted context. Relay is now positive only for the canonical surface, the exact official OpenAI API, or a destination whose operator opts in with the new `decodesNativeCompactionBlobs` provider flag. Verified that the flag survives config derivation and reaches the predicate, since the unit tests construct provider literals and would not have caught it being dropped there. Also corrects a stale line in the transport notes: compact-wire items are not exempt from the `store: false` item-id strip. That exemption was deliberately reverted to match codex-rs (`core/src/client.rs:918-925`). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…vation guard The guard is sound, but its comments claimed it fixed Grok's `Could not decrypt the provided encrypted_content` failure. Live bisection disproved that: Grok emits summary-channel reasoning natively, so `reasoningItemToSummaryShape` returns early and this rewrite never fires on that route. The real cause was `"content": null` on the replayed reasoning item, fixed separately. A false causal claim in a comment is worse than none — the next reader trusts it. The rule is restated on its own terms: an item carrying opaque provider state should not have its stored shape changed unless that backend has an explicit replay contract, which is why DeepSeek was safe and why the Kimi/GLM/NeuralWatt routes now on `preserveReasoningContentModels` are the ones this actually guards. Comments and prose only; no behaviour change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The first version stripped `"content": null` from every reasoning item, which broke OpenAI. Caught in live traffic minutes after deploying it locally: 400 invalid_request_error The encrypted content k7pQ...Px7D could not be verified. Reason: Encrypted content could not be decrypted or parsed. An OpenAI-operated backend binds the blob to the item's exact shape, so removing a field invalidates it. The two requirements are exactly opposed: xAI refuses the null key, OpenAI needs it kept — so the strip has to follow the destination. The predicate is deliberately not `authMode === "forward"`. A noncanonical forward provider never receives the caller's credentials, so forward auth says nothing about which backend answers; only the canonical ChatGPT surface and the official OpenAI API are treated as OpenAI-operated, and a self-hosted relay is routed like any other gateway. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… boundary The namespace boundary lowered complete groups but still let several Codex-private shapes reach a strict gateway, each reproducing the pre-inference rejection the boundary exists to prevent. No `type: "namespace"` value survives now. A group the layer cannot express — empty, nested, or with an unusable child name — is dropped along with the children it cannot represent. Relaying the private shape costs the whole request rather than one tool, so "preserve rather than lose a tool" was losing strictly more. Replayed call items are lowered whether or not this turn declares the group they name. The routed compaction turn strips the entire tool surface before the boundary runs, so every compaction after a namespaced tool call shipped the private `namespace` key this layer's own restoration had stamped on the item. Only tool_choice resolves a bare name through the catalog: a history item records which tool actually ran, so re-pointing it at a same-named namespace child would rewrite that record on a coincidence rather than translate it. Codex-private tool fields now come from one table instead of one bespoke pass each, and it gains `defer_loading` — `activateDeferredTool` clears that only for tools a `tool_search_output` already loaded, so the first turn of a deferred catalog carried it to the wire — and the `web_search_preview` variant. A bare declaration and a `functions` child of the same name are one logical tool: `buildTools` flattens the reserved group without a namespace, the parser tolerates the duplicate, and `promoteClientLoadedTools` produces it. That shape raised a wire-name collision that escaped every catch up to the Bun handler, so an ordinary catalog became an unstructured 500 with no request log — while the rotation-rebuild path answered 400 for the identical throw. It is now deduped, and a genuine collision is a typed error the passthrough maps to 400. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…route switch
Switching models mid-conversation broke the next turn. Reproduced end to end
through the proxy: mint a reasoning item on xai/grok-4.6, replay to
openai/gpt-5.6-sol.
replay grok -> grok : OK
replay grok -> SOL : Unknown parameter: 'input[1].status'
... status removed:
replay grok -> SOL : The encrypted content ZvQ+...fBJg could not be verified.
... status and encrypted_content removed:
replay grok -> SOL : OK
Two independent problems. Grok emits an output-only `status` on reasoning items
that OpenAI rejects on input, and a reasoning blob is decodable only by the
backend that minted it, so after a switch the client replays blobs the new
destination cannot read.
This extends the mechanism the repo already uses for opaque provider state
rather than adding a retry: `reasoning-replay-cache` already keeps a bounded,
thread-scoped store and already computes the provider/destination/adapter/model/
credential identity. It now also records which identity served a thread last, and
a request whose identity differs from that record drops `encrypted_content` from
replayed reasoning items before they go out. No record — fresh process, evicted,
expired, no client thread — keeps the blobs rather than discarding valid cached
reasoning on a guess; that leaves a switch spanning a proxy restart uncovered,
which the comment states rather than implies.
`status` is stripped only from items that are not forwarding a blob. An
OpenAI-operated backend binds the blob to the item's exact shape, so removing any
field from an item we still expect it to decode can invalidate it — the same
failure an unconditional `content` strip already produced once on this codebase.
Content blanking predates that invariant and is unchanged; an item carrying both
a native blob and raw content is a known unresolved conflict, noted in place.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…stom calls by wire identity
Review found two defects in the flattening layer; both are fixed here.
Deduplication depended on declaration order. A bare declaration and a `functions`
child of the same name are one logical tool, but which one owned the wire name —
and therefore which one was emitted — followed whichever container the rewrite
reached first. The plan now records the bare wire names from the complete catalog
and the bare declaration always wins, so the same catalog flattens identically
whichever container declares it.
Custom-call restoration used the wrong coordinate. A custom tool inside a
non-`functions` namespace is lowered twice on the way out (custom to function,
then renamed to `<ns>__<name>`), while on the way back namespace restore runs
first and replaces the wire name with the bare one. Custom restore then matched
that bare name and could convert an unrelated same-named function call, sending
Codex a `custom_tool_call` with the wrong payload shape.
Converted custom tools are now tracked by their final upstream wire name, and
restoration reconstructs that identity from the `{namespace, name}` an earlier
rewrite restored. A namespaced custom and a namespaced function sharing a child
name now round-trip to their own item types, on both the JSON and SSE paths.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The serving-identity record compared `credentialIdentity`, which for OAuth is `accountId + generation` and therefore changes on every token refresh. Six of the eight `bindRouteReasoningReplayScope` call sites are key-rotation or OAuth-refresh rebinds, so an ordinary refresh registered as "the backend changed" and the next turn on that thread dropped a valid blob. Key-pool providers would have paid that repeatedly, and silently — nothing errors, the model just loses cached reasoning. The module already distinguishes the durable dimensions for exactly this reason (#1926: the rotating generation deliberately does not participate). The serving record now compares `providerDestinationDurableIdentity` and `credentialDurableIdentity`, and refuses to record at all when those are missing rather than falling back to the volatile pair: a missed strip costs one degraded turn, a spurious strip is a permanent quality regression. The proxy-owned replay cache keeps its stricter key, which is deliberate. Also documents two behaviours that would otherwise read as bugs: a combo that rotates targets between turns legitimately drops blobs while the SSE model-name rewrite hides the switch from the client, and the image/web-search loops consume the replay scope without rebinding, which is what stops an internal small-model call from poisoning the record for the main conversation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ok-responses-fixes
…ok-responses-fixes
…ok-responses-fixes
The thread-scoped serving-identity record strips replayed blobs
deterministically, but it is in-process and bounded, and it deliberately
keeps blobs when it has no record — stripping on "unknown" would discard
valid reasoning after every restart.
That leaves a failure users hit routinely. From the live usage log, one
conversation:
19:33:31 xai grok-4.6 200 <- last grok turn
19:38 proxy restarted (records wiped)
19:48:11 openai gpt-5.6-sol 400
"The encrypted content Py6J...kwW9 could not be verified.
Reason: Encrypted content could not be decrypted or parsed."
The proxy never served the turn that minted those blobs, so it cannot know
they are foreign. TTL expiry, LRU eviction and any transcript older than the
process open the same hole.
Register a recovery kind rather than invent a retry path: `image-413`
already reacts to an upstream rejection by rebuilding the body once and
refetching inside the recovery loop, with a single-attempt guard. This adds
`opaque-blob-rejection` on the same shape, triggered only by a decoder's own
4xx identity — OpenAI's nested `invalid_encrypted_content`, or xAI's two
concrete decoder messages — and only when the exact outbound body still
carried a blob, so an unrelated `invalid-argument` never gains a hidden
resend and a blobless body never triggers an identical resend.
The deterministic pre-flight stays primary: when a record exists the first
request is already correct and this never runs. Cost when it does run is one
extra round trip and one turn of degraded reasoning.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`scrubOcxCompactionItems` forwarded any non-`ocx1:` blob whenever the
destination could decode native blobs. That is sound only if native blobs
have a single minter, and they do not: xAI mints them as well, so an
xAI-minted compaction blob replayed to an OpenAI-operated destination was
forwarded verbatim and rejected.
Reproduced against the live proxy on a thread whose serving identity had
already changed and was known to have changed — the reasoning path stripped
correctly while the compaction item sailed through:
POST /v1/responses model=gpt-5.6-sol, thread last served by xai/grok-4.6
input: [{"type":"compaction","encrypted_content":<opaque non-ocx blob>}, ...]
-> 400 invalid_encrypted_content
"The encrypted content rmey...SQ== could not be verified."
Reuse the signal the reasoning path already consumes rather than recomputing
identity in the adapter: on a known mismatch a native blob degrades through
the existing `compactionItemToText` note instead of being forwarded. With no
known mismatch, behaviour is unchanged.
This covers threads the process has served. A cold record — after a restart,
TTL expiry or eviction — still forwards, which is a separate change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The cross-backend fix kept `status` on any reasoning item that forwarded its `encrypted_content`, to honour "an item whose blob is forwarded is not otherwise modified". That invariant was defensive rather than observed, and it broke the cold-record recovery path. With no provenance record — after a restart, TTL expiry or eviction — the blob is retained, so `status` is retained too, and OpenAI rejects the request on the field before it ever validates the blob: 400 Unknown parameter: 'input[1].status'. The opaque-blob recovery correctly does not match that error, so the conversation stayed broken. Measured against the live backends: - OpenAI never mints `status` on a reasoning item (keys are content, encrypted_content, id, summary, type), so the retain branch could only ever fire for an item minted elsewhere — the exact item OpenAI then rejects. It never protected an OpenAI-minted item. - Grok accepts its own 1707-char blob with `status` removed: 200. - With `status` removed, that same item replayed to gpt-5.6-sol returns 200 and the usage log records sendCount=2, recoveryKinds=['opaque-blob-rejection'] — removing the field is what lets the request reach the blob check the recovery is armed for. The `content` rule is untouched: blanking predates this and is required by ChatGPT's input contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two units landed separately and left duplication. The recovery unit was written on a branch without the compaction-provenance change, so it degraded compaction items itself by rewriting `parsed._rawBody.input` in place. Once both are merged that walk is redundant: it sets `_stripReasoningEncryptedContent`, which is exactly the signal the adapter's own compaction scrub consumes. Verified rather than assumed, since the two call sites rebuild through different adapters. Both reach `openai-responses` (the recovery predicate restricts to it), whose `buildRequest` consumes `_rawBody` and runs `scrubOcxCompactionItems`; the native passthrough site resolves a passthrough retry adapter, the generic site rebuilds through the retained `activeAdapter`. So the manual walk changes no outbound body on either path, and dropping it removes a mutation whose side effect outlived the request. The native Responses branch returns before the generic `recovery:` loop, so the recovery block was also written out twice. Whoever next adds a recovery kind to the generic loop would not know a second loop exists. Extract the shared predicate, guard, preparation, body cancellation and rebuild into one `attemptOpaqueBlobRecovery` helper both sites call, each keeping its own control flow and its site-specific rebuild — the generic one still invalidates the same-target request. Cross-reference comments on both loops name the other. No outbound behaviour changes. Existing recovery tests are untouched; added coverage for routed compaction recovery through the generic loop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The recovery suite asserted the resend well but said nothing about the first outbound send beyond "it carries a blob". That the first send has `status` already stripped is load-bearing: the recovery is armed for the upstream's blob-rejection error, and if `status` survives, OpenAI answers 400 Unknown parameter: 'input[1].status'. before it validates the blob. The recovery correctly does not match that error, so it never fires. That exact regression shipped once — `stripOutputStatus` was gated on the item not forwarding its `encrypted_content`, which is precisely the cold provenance case — and the entire suite stayed green while the live path was unchanged. Assert the first send's reasoning item by shape: blob present, no `status`. Verified the guard bites: reintroducing the old condition turns this test red, where before it left the suite green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4 tasks
Contributor
|
✅ Deterministic PR hygiene checks passed. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0c8912574
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+513
to
+514
| if (updateReasoningReplayServingIdentity(parsed._reasoningReplayScope)) { | ||
| parsed._stripReasoningEncryptedContent = true; |
There was a problem hiding this comment.
When a request switches routes, this call both detects the mismatch and immediately overwrites the thread's recorded serving identity before request construction or the upstream attempt succeeds. If the new route then fails—for example because request building throws, a transport fails, or a 4xx/429 remains terminal—the client still holds blobs minted by the old route, but returning to that old route is now misclassified as another switch; the adapter strips valid encrypted reasoning and replaces a decodable compaction summary with the opaque note, silently losing usable context. Separate comparison from committing the identity, or roll the update back unless the new route successfully serves the turn.
Useful? React with 👍 / 👎.
| supportsServiceTier: z.boolean().optional(), | ||
| modelSupportsServiceTier: z.record(z.string().min(1), z.boolean()).optional(), | ||
| preserveResponsesReasoningContent: z.boolean().optional(), | ||
| decodesNativeCompactionBlobs: z.boolean().optional(), |
There was a problem hiding this comment.
This adds a user-configurable provider option required for a relay to retain native compaction blobs; without it, the new default degrades those blobs to an opaque note. A repository-wide search finds decodesNativeCompactionBlobs only in runtime types/configuration and the internal structure/ note, with no docs-site/ documentation, so operators cannot discover how to preserve compaction context for a compatible relay. Add the option and its security/compatibility implications to the provider configuration documentation.
AGENTS.md reference: AGENTS.md:L279-L280
Useful? React with 👍 / 👎.
2 tasks
lidge-jun
added a commit
that referenced
this pull request
Aug 21, 2026
…only table (#2262) The #2258 series unconditional strip collided with the #2238 capability contract: official OpenAI API-key traffic lost external_web_access (lidge full suite 1/14025 fail). The table row is now capabilityGated, active only when supportsOpenAiWebSearchToolFields === false; defer_loading stays unconditional. 387/0 across six suites.
4 tasks
This was referenced Aug 21, 2026
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lands PR #2254 by @olddonkey (the consolidated 8-PR responses series: #2217 #2228 #2229 #2237 #2248 #2249 #2251 #2252), rebased onto current dev with all 22 commits and their evidence preserved.
Verification
Checklist