You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(responses): keep a child's reasoning replay out of the parent's record - #8634
A child agent request carries its own thread-id plus x-codex-parent-thread-id. reasoningReplayConversationIdFromResponsesRequest preferred the parent header, so the child and
the parent resolved to one replay record. Once the child was served, commitReasoningReplayServingIdentity stored its provider/adapter/model tuple under the parent's
key. The parent's next Claude turn then saw a route change, set _stripReasoningEncryptedContent and dropped its replayed thinking signatures, so the prefix it
had just cached no longer matched.
The request's own thread-id now wins; the parent header stays the fallback for callers that only
carry it. Mixed headers describing one conversation still share a record, log conversation ids and
pool affinity are untouched.
Verification
bun test tests/usage/request-log-conversation.test.ts — 22 pass.
bun test tests/server/server-agent-task-recovery-replay.test.ts tests/adapters/reasoning-replay-identity.test.ts tests/lib/reasoning-replay-scope-source.test.ts tests/usage/request-log-conversation.test.ts — 77 pass.
bun test tests/server/server-combo-reasoning-replay-eligibility.test.ts and tests/server/replay-refusal-parity.test.ts run individually — pass. Running several heavy
server files in one process shares fixture ports and fails on takeSpendHome; that reproduces
without this change.
Offline script on dev before the fix: parent and child both resolved to parent-thread, and
the parent then reported providerChanged/destinationChanged/adapterChanged/modelChanged.
After the fix the child resolves to child-thread and the parent sees no change. No provider
calls; the proxy config dir was a throwaway path.
Risk
Callers that send only x-codex-parent-thread-id (no thread-id) behave exactly as before. A
client that sends a per-turn unique thread-id would lose replay continuity across its turns;
Codex sends a stable per-conversation thread id, which the updated test pins.
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
…ecord
A child agent request carries its own thread-id plus x-codex-parent-thread-id, and reasoningReplayConversationIdFromResponsesRequest preferred the parent header. Both turns shared one replay scope, so after the child was served, commitReasoningReplayServingIdentity recorded its provider/adapter/model under the parent's key. The parent's next Claude turn then saw a route change, stripped its replayed thinking signatures and re-wrote the prompt prefix (lidge-jun#8632).
Resolve the request's own thread-id first and keep the parent header as the fallback, so callers that only carry the parent identity still share one record. Log conversation ids and pool affinity are unchanged.
all PR quality gates passed; the review readiness checklist is complete.
Review readiness checklist
✅ Required local validation passed; commands, results, and any full-suite exception are documented.
✅ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
✅ I resolved all correct Codex and CodeRabbit findings.
✅ My PR is ready for review.
✅ 4/4 boxes ticked.
This pull request has been marked Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun@Ingwannu
No actionable comments were generated in the recent review.
Walkthrough
Reasoning-replay conversation identity now selects the request’s thread-id before the parent-derived clientThreadId. The parent ID remains a fallback, and the existing Cursor and session_id candidates keep their order. Tests now expect child identity when both IDs are present.
Changes: Reasoning-replay identity
Layer / File(s)
Summary
Select and verify replay identity src/types/request.ts, src/server/request-log-conversation.ts, tests/usage/request-log-conversation.test.ts, tests/server/server-agent-task-recovery-replay.test.ts
At request-log-conversation.ts:160-161, the selector checks threadIdHeader before clientThreadId. The type documentation and tests describe and verify this precedence. The conversation-id hashing behavior is unchanged.
Child requests now keep their reasoning replay record separate from the parent's, while parent-only callers behave as before. No actionable merge-blocking risk remains.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the main change: a child's reasoning replay is kept out of the parent's record. The summary and objectives describe the same change, which makes the child's own thread-id take preceden…
Linked Issues check
[#8632] Issue #8632 is closed and is historical context only. The PR objectives also state that no active directly linked targets remain. The issue's background is still used as the reference for the …
Out of Scope Changes check
The whole-PR diff (merge base d1f36ed to head 31590a9) covers the reasoning-replay precedence change in src/server/request-log-conversation.ts, its doc comment, a doc update in `src/types/reques…
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Full details: Title check
Explanation
The title names the main change: a child's reasoning replay is kept out of the parent's record. The summary and objectives describe the same change, which makes the child's own thread-id take precedence over the parent-thread fallback in replay conversation ID selection. The title is specific, concise, and in conventional-commit form.
Full details: Linked Issues check
Explanation
[#8632] Issue #8632 is closed and is historical context only. The PR objectives also state that no active directly linked targets remain. The issue's background is still used as the reference for the defect, and the core requirement is that a child turn must not overwrite the parent's replay record. At head 31590a9, reasoningReplayConversationIdFromResponsesRequest in src/server/request-log-conversation.ts (around lines 146-165) now calls firstSanitizedConversationId(input.threadIdHeader, input.clientThreadId, input.cursorConversationId, input.sessionIdHeader). The request's own thread-id now precedes clientThreadId, which is the parent-thread identity. This matches the issue's proposed fix. The parent header remains the fallback when thread-id is absent, so mixed headers that describe one conversation still share a record. The change is limited to the replay-namespace function. The conversationIdFromResponsesRequest, sessionLaneIdFromRequest, and nativeLogConversationIdFromHeaders functions are not changed in the diff, so log conversation ids and pool affinity are not touched. The test changes described in the raw summary (tests/usage/request-log-conversation.test.ts, tests/server/server-agent-task-recovery-replay.test.ts) were not executed in this review. The author's reported test results are unverified here.
Full details: Out of Scope Changes check
Explanation
The whole-PR diff (merge base d1f36ed to head 31590a9) covers the reasoning-replay precedence change in src/server/request-log-conversation.ts, its doc comment, a doc update in src/types/request.ts for OcxReasoningReplayScopeRef.clientThreadId, and the updated tests in tests/usage/request-log-conversation.test.ts and tests/server/server-agent-task-recovery-replay.test.ts. The doc and test changes directly describe and pin the same precedence change, so they support the objective and are not unrelated. No unrelated behavior change was found in the inspected hunks. Limits: I inspected only the src/server/request-log-conversation.ts diff and the head file. I did not read the diffs of the other three changed files, and I did not run the tests.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
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
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.
Fixes #8632.
Summary
A child agent request carries its own
thread-idplusx-codex-parent-thread-id.reasoningReplayConversationIdFromResponsesRequestpreferred the parent header, so the child andthe parent resolved to one replay record. Once the child was served,
commitReasoningReplayServingIdentitystored its provider/adapter/model tuple under the parent'skey. The parent's next Claude turn then saw a route change, set
_stripReasoningEncryptedContentand dropped its replayed thinking signatures, so the prefix ithad just cached no longer matched.
The request's own
thread-idnow wins; the parent header stays the fallback for callers that onlycarry it. Mixed headers describing one conversation still share a record, log conversation ids and
pool affinity are untouched.
Verification
bun test tests/usage/request-log-conversation.test.ts— 22 pass.bun test tests/server/server-agent-task-recovery-replay.test.ts tests/adapters/reasoning-replay-identity.test.ts tests/lib/reasoning-replay-scope-source.test.ts tests/usage/request-log-conversation.test.ts— 77 pass.bun test tests/server/server-combo-reasoning-replay-eligibility.test.tsandtests/server/replay-refusal-parity.test.tsrun individually — pass. Running several heavyserver files in one process shares fixture ports and fails on
takeSpendHome; that reproduceswithout this change.
devbefore the fix: parent and child both resolved toparent-thread, andthe parent then reported
providerChanged/destinationChanged/adapterChanged/modelChanged.After the fix the child resolves to
child-threadand the parent sees no change. No providercalls; the proxy config dir was a throwaway path.
Risk
Callers that send only
x-codex-parent-thread-id(nothread-id) behave exactly as before. Aclient that sends a per-turn unique
thread-idwould lose replay continuity across its turns;Codex sends a stable per-conversation thread id, which the updated test pins.
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.