fix(codex): stop replaying reasoning items the backend can't verify - #3199
Conversation
ADR 0097 recorded encrypted-reasoning replay as "didn't block the tool loop".
It was not absent — it was half wired, and the missing half is a 400 that
bricks a thread:
400 invalid_encrypted_content — The encrypted content for item rs_… could
not be verified. Reason: Encrypted content could not be decrypted or parsed.
With store=false the backend keeps no reasoning state, so a replayed reasoning
item must carry its own encrypted_content. langchain-openai's STREAMING
Responses path never captures that blob (it reads the item at
response.output_item.added, where the field is still null, and the terminal
response.completed event keeps only parsed/usage/response_metadata) — but the
item's rs_… id DOES survive into additional_kwargs and gets replayed.
protoAgent always streams on this provider, so that was the only shape it ever
sent: an item referenced by an id the backend never stored.
The item is checkpointed, so every later turn in the thread re-sent it and
failed identically — the thread was bricked, the same failure class
ToolCallRepairMiddleware exists to heal for a dangling tool_call.
Containment, both halves:
- CodexChatOpenAI sanitizes the outbound Responses `input`: a reasoning item
with no blob is dropped (restoring the stateless continuity the ADR believed
it already had); one that carries a blob keeps it but loses its `id`, which
store=false cannot resolve.
- CodexReasoningReplayRecoveryMiddleware heals a thread when the provider
rejects a blob for reasons the sender can't see ahead of time —
encrypted_content is sealed to the endpoint that minted it, and this repo
lets each slot name its own connection, each chat tab override the model per
turn, and a failed turn retry against the fallback chain. It strips the
replay state, retries once, then rewrites the offending assistant messages in
place by id so the bad item leaves the checkpoint. Registered on the lead and
subagent stacks, INSIDE the failover wrapper (which swallows each attempt's
error and re-raises the primary one, hiding exactly this 400 from anything
outside it) and outside provider shaping.
Capturing the blob so replay actually works needs an output_item.done handler
langchain-openai does not have — filed as an open item on ADR 0097.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 12 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![protoreview[bot]](https://avatars.githubusercontent.com/in/4233969?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA panel review — PASS
code-review-structural · head 958a6fdc1bf3 · formal
⚠️ PR advanced 2 commit(s) during this round (958a6fdc1bf3→a362a86da1dd); 0 finding(s) in the delta were demoted to possibly addressed.
The PR is a single changelog fragment (changelog.d/3199.fixed.md) documenting a fix for Codex/ChatGPT-subscription chat bricking on unverifiable reasoning items. Overall risk is low — it's a documentation-only change with no code impact. The panel's sole finding (that the fragment lacked a - bullet and a (#NNNN) reference) was refuted by verification: the actual diff content shows both are present and correctly formatted. No defects survive. The structural pass completed without additional findings.
No findings — the review came back clean.
findings JSON (machine-readable)
[]
There was a problem hiding this comment.
QA panel review — PASS
code-review-structural · head a362a86da1dd · formal
All four LLM finders returned clean on PR #3199; the verifier confirmed the empty findings array and added no annotations. Overall risk is low. There is no fix-first item. The panel did not disagree on any point. The one coverage gap is a skipped structural/AST pass: protoPatch's gateway auth failed, so no tool-sourced findings were produced. This is a process gap, not a code defect — no action is required beyond awareness, but it means the review lacks the cross-engine confirmation a successful protoPatch run would have provided.
No findings — the review came back clean.
findings JSON (machine-readable)
[]…easoning-replay # Conflicts: # docs/adr/0097-native-oauth-subscription-providers.md
There was a problem hiding this comment.
QA panel review — PASS
code-review-structural · head 89c9c206644f · formal
Overall risk is low: four of five finders completed their lanes thoroughly and found no defects, and the verifier confirmed the empty findings array. The structural pass (protopatch) was skipped due to a gateway error, so the panel's coverage is LLM-only on this round — a minor gap, not a code concern. One open question was noted by a finder regarding test_async_path_retries_the_same_way and whether @pytest.mark.asyncio is correctly applied, but it was not emitted as a formal finding and the verifier did not elevate it; it remains an unconfirmed observation, not a blocker. No prior requests to disposition. Nothing to fix before merge on the evidence available.
No findings — the review came back clean.
findings JSON (machine-readable)
[]
…suer (#3207) * feat(codex): capture encrypted reasoning and replay it only to its issuer Closes the "contained, not delivered" open item #3199 left on ADR 0097. Cross-turn reasoning continuity on `openai-codex` was off because the blob that makes it possible was never captured; this wires the capability and the guard that makes it safe on a shared thread. Capture. langchain-openai's streaming Responses path has no response.output_item.done branch for reasoning (it has one for `compaction`, which carries the same kind of blob), and the terminal response.completed event keeps only parsed/usage/response_metadata — so encrypted_content is visible in exactly one event, which the converter drops. codex_client re-emits that event as a content-block delta that merges onto the reasoning block already in flight, by index. The wrapper sits on the shared module-level converter (there is no instance seam) but is inert unless a contextvar this module's client sets is present, so every other ChatOpenAI in the process goes through the original path unchanged. output_version flipped to responses/v1. "v0" collapses a turn's reasoning into ONE additional_kwargs slot — later items overwrite earlier ones, and streamed fragments of two different items merge into each other — so it structurally cannot carry per-item blobs. The block format keeps each item separate and in order, and langchain replays it that way. The rendering half of the v0 pin was already paid off (every answer site reads AIMessage.text); text_of now skips reasoning blocks rather than writing a _[reasoning]_ placeholder into exports, session memory and chat bundles, which is what ADR 0021 asks for anyway. PROTOAGENT_CODEX_OUTPUT_VERSION=v0 is the escape hatch. Issuer stamping. encrypted_content is sealed to the endpoint AND account that minted it. Each captured item carries a truncated digest of (base_url, account_id) — so a checkpoint never stores a raw account id — and replay drops items stamped with a different issuer. Unstamped items still replay. This is what makes per-slot providers, per-tab model override and the fallback chain safe on one thread; without it the #3199 recovery middleware would fire routinely instead of never. Not verified against a live ChatGPT subscription. If the backend objects, the #3199 recovery valve strips the replay state and retries, so the thread degrades to stateless continuity rather than breaking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(changelog): fragment for #3207 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(codex): arm the reasoning capture on the dispatched stream seam The contextvar that gates the capture was set in an override of _stream_responses — which ChatOpenAI._stream reaches via `super()._stream_responses(...)`, an explicit super(ChatOpenAI, self) bind that skips the subclass entirely. So the override was dead code and the capture would never have armed in production, while every unit test that set the contextvar by hand still passed. Armed on _stream/_astream instead (dispatched on the instance), and covered by a test that drives the client's OWN stream against a stubbed root_client, so nothing but the client can set the contextvar. Verified to fail against the old seam. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(codex): stamp the issuer on langchain-openai >= 1.6 too CI installs deps UNPINNED while uv.lock holds langchain-openai 1.3.0, so CI runs 1.6.0 and local runs 1.3.0. 1.6.0 added the reasoning `response.output_item.done` branch this wrapper existed to supply — and the wrapper bailed out whenever the original produced a chunk, so on 1.6.0 the blob was captured but NEVER STAMPED. That is not a test artifact: an unstamped item is treated as legacy and always replayed, i.e. the cross-issuer guard was silently inoperative on exactly the version CI (and any fresh install) uses. CI caught it; the local suite could not. The wrapper now augments instead of bailing: it stamps the issuer whether the installed langchain surfaced the blob or we had to synthesize it. Verified against BOTH dependency sets — 1.3.0 (uv.lock) and 1.6.0 (a CI-equivalent venv): full suite green on each. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
The reported error
What was actually wrong
ADR 0097's live-validation note records encrypted-reasoning replay as having "not blocked the tool loop in practice". True of the tool loop, wrong about the wire: replay wasn't absent, it was half wired — and the missing half is a 400 that bricks a thread.
With
store=falsethe backend keeps no reasoning state, so a replayed reasoning item must carry its ownencrypted_content. langchain-openai's streaming Responses path never captures that blob — it reads the item atresponse.output_item.added(where the field is still null), and the terminalresponse.completedevent rebuilds the full message but keeps onlyparsed/usage/response_metadatafrom it. The item'srs_…id does survive intoadditional_kwargs["reasoning"], and langchain replays it. protoAgent always streams on this provider (the backend mandates it), so that was the only shape it ever sent: an item referenced by an id the backend never stored, with nothing to verify.include=["reasoning.encrypted_content"]was asking for a blob nothing read.Reproduced through the installed libs before writing a line:
Worse than a failed turn: the item is checkpointed, so every later turn in the thread re-sent it and failed identically — the thread was bricked. Same failure class
ToolCallRepairMiddlewareexists to heal for a danglingtool_call.What this PR does
Containment, both halves. (Capturing the blob so replay actually works needs an
output_item.donehandler langchain-openai doesn't have — filed as an ADR 0097 open item, and worth an upstream issue.)graph/providers/codex_client.py—CodexChatOpenAIsanitizes the outbound Responsesinput:id—store=falsecannot resolve an item id, and the blob is self-contained.graph/middleware/codex_reasoning_replay.py— the same 400 can still arrive from causes the sender can't see ahead of time:encrypted_contentis sealed to the endpoint that minted it, and this repo lets each slot name its own connection (gateway:/anthropic-oauth:/openai-codex:), lets each chat tab override the model per turn, and retries a failed turn against the fallback chain — every one of which replays one thread's history to an endpoint that didn't mint it (a rotated credential does the same). The middleware strips the replay state, retries once, then rewrites the offending assistant messages in place by id so the bad item leaves the checkpoint instead of being dodged for one call.Guarded so it can only touch the thread it's meant to fix: an unrelated error propagates unchanged, a matching error on a history with no replay state propagates unchanged (the provider is objecting to something we didn't send), and a second failure after the strip propagates too. No-op on every healthy turn, which is why it's registered unconditionally — a gateway relaying the Responses API can raise this just as easily.
Placement is load-bearing (pinned by a test): registered on the lead and subagent stacks, inside the failover wrapper — which swallows each attempt's error and re-raises the primary one, so a recovery placed outside it never sees the 400 a fallback attempt caused, and a fallback attempt is exactly when a thread's reasoning items meet an endpoint that didn't mint them — and outside provider shaping, so the retried request is still shaped for whichever model it lands on.
Hermes's Codex adapter reached the same two sanitation rules independently, including the id strip and a session-wide replay kill switch; its
_issuer_kindstamp is the model for the cross-issuer filter now listed as an ADR open item.Verification
tests/test_codex_reasoning_replay.py— 27 tests: sanitation (incl. end-to-end through the real client, driving the exact streamed shape down to the payload), classification against the verbatim reported error string plus three unrelated 400s, strip/repair helpers, sync + async retry, the three no-retry guards, checkpoint repair, session scoping, and the chain-ordering invariant.ruff check .clean ·lint-imports3/3 kept ·scripts/live_smoke.pyPASSED.Console untouched.
🤖 Generated with Claude Code