Synced from GitHub
fix(server): Codex forks no longer request full turn history#24537
Problem
Forking a Codex conversation requested its complete historical turns even though T3 only consumes the returned thread metadata. Large histories can exceed the app-server transport limit. A real web fork on main sent no excludeTurns flag and returned the source turn.
Change
Send excludeTurns: true on the existing typed thread/fork request. Keep the inclusive selected-turn boundary and fork-local revert fallback. Add a regression for the outgoing request and update exact replay expectations used by existing tests.
Scope and approval
Fixes https://github.com/pingdotgg/t3code/issues/24460
GitCafe: #24460
Triaged bug via the automated issue flow. This is a focused fix to avoid transferring unused provider history; it adds no settings or persisted state.
Verification
- Regression failed before the fix because
excludeTurns: truewas missing from the actual outbound frame; it passes with the fix. - All 87 tests across
CodexAdapterV2.test.ts,ThreadFork.integration.test.ts, andThreadMergeBack.integration.test.tspass. The old integration fixtures reproduced two failures before their request expectations were updated. Server typecheck passes. Targeted lint has no errors and two preexisting unused-variable warnings. - Final scoped run at df41a3727c: 87 passing tests.
- Real dev web app on Linux with Codex CLI 0.163.0: a fresh source has a completed marker turn; the fork request sends
excludeTurns: trueand the selectedlastTurnId; its response hasturns: []; continuation on the returned native thread recalls the marker without the follow-up supplying it. - No client contracts or render paths change. The visual fixture matrix is empty.
| Evidence | Before | After |
|---|---|---|
| Native protocol | main request and response | fixed request, empty history, retained context |
| Regression | missing flag failure | focused passing output |
Evidence index and reproduction steps, extraction script, empty fixture matrix.
The reporter's private 590 MB history and old global hang were not reproduced. The runtime proof establishes response-history suppression and retained fork context on small valid history. The attempted synthetic rollout-only enlargement was discarded because it did not produce a valid large-history fixture. Other Codex versions and native client shells were not exercised.
Models: Claude Opus 5.5 through the Claude harness; GPT-6.1-Sol through the Codex harness in T3 Code.
Checks
22 successful · 7 skipped
Added 3 commits
Synced from GitHub
Approvability
Verdict: Approved at df41a37
Macroscope's review found this PR approvable — This focused server fix adds excludeTurns: true to Codex fork requests, avoiding oversized unused history responses while preserving fork boundaries and continuation behavior. The production change is isolated and covered by targeted tests and updated replay fixtures.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
Thread transfer impact
✅ Thread transfer remains within every enforced ceiling.
| Provider | Metric | Main baseline | This PR | Impact | PR ceiling | |
|---|---|---|---|---|---|---|
| Codex | Total thread wire | 5.0 KiB | 5.0 KiB | +7 B (+0.1%) | 6.8 KiB | ✅ |
| Codex | Thread snapshot wire | 3.8 KiB | 3.8 KiB | 0 B (0.0%) | 4.9 KiB | ✅ |
| Codex | Live turn WebSocket wire | 1.2 KiB | 1.2 KiB | +7 B (+0.6%) | 2.0 KiB | ✅ |
| Codex | Live turn WebSocket decoded | 20.9 KiB | 20.9 KiB | 0 B (0.0%) | 29.3 KiB | ✅ |
| Codex | Live turn messages | 2 | 2 | 0 (0.0%) | 8 | ✅ |
| Claude | Total thread wire | 5.0 KiB | 5.0 KiB | 0 B (0.0%) | 6.8 KiB | ✅ |
| Claude | Thread snapshot wire | 3.8 KiB | 3.8 KiB | 0 B (0.0%) | 4.9 KiB | ✅ |
| Claude | Live turn WebSocket wire | 1.2 KiB | 1.2 KiB | 0 B (0.0%) | 2.0 KiB | ✅ |
| Claude | Live turn WebSocket decoded | 21.2 KiB | 21.2 KiB | 0 B (0.0%) | 29.3 KiB | ✅ |
| Claude | Live turn messages | 2 | 2 | 0 (0.0%) | 8 | ✅ |
Baseline: 8176b58 · PR result: df41a37 · Source CI: success
Scenario and decoded snapshot size
10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
- Codex decoded thread snapshot: 108.5 KiB
- Claude decoded thread snapshot: 108.8 KiB
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.
Synced from GitHub
No actionable comments were generated in the recent review.
External image Recent review info
External image Run configuration
- Source mode: Auto
- Repository: pingdotgg/t3code
- Branch: fix/issue-24460
- File: .coderabbit.config.ts
- Commit: df41a37
- Layers: Repository TS config
- Review profile: CHILL
- Plan: Team
- Run ID:
7858bba3-0fd4-4df5-88d0-7589d6faf4d2
External image Commits
Reviewing files that changed from the base of the PR and between 8176b589310fac6f59f1878ede4f7287c6c36d8f and df41a3727c9e76846a5c56e3529290c08601a162.
External image Files selected for processing: 4
apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.tsapps/server/src/orchestration-v2/Adapters/CodexAdapterV2.tsapps/server/src/orchestration-v2/testkit/fixtures/thread_fork_native_prior_turn/codex_transcript.ndjsonapps/server/src/orchestration-v2/testkit/fixtures/thread_merge_back_siblings/codex_transcript.ndjson
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
Walkthrough
The Codex adapter now sends
excludeTurns: truewiththread/fork. Replay tests and fixtures cover fork requests, including a native turn-boundary case where the fork response contains metadata only.Changes: Codex fork request
Layer / File(s) Summary Fork request and replay coverage
apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts,apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts,apps/server/src/orchestration-v2/testkit/fixtures/*forkThreadaddsexcludeTurns: true. Replay coverage checks the request parameter and verifies that a native turn-boundary fork succeeds from thread metadata without a subsequent outbound request. Fixtures update the expected fork requests.Priority: External image Normal
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related PRs
- pingdotgg/t3code#24487: It makes the same Codex fork request change and updates replay expectations.
Suggested labels:
bugfixSuggested reviewers:
t3dotgg,only21mil
Merge Risk: External image Minimal · up to df41a
The fork change is mergeable on the supplied evidence; no actionable risk remains beyond normal checks.
Pre-merge checks: Passed 3 · Inconclusive 1
Failed Failed checks: 1 inconclusive
Check name Status Explanation Resolution Linked Issues check Inconclusive Directly linked issue #24460has one active coding target: the fork path must send excludeTurns: true. Evidence supports this. The test diff in CodexAdapterV2.test.ts adds excludeTurns: true to the ex…Establish whether the reviewed head already has a test that asserts pending requests complete with an error when the transport reader fails. If it does not, the missing evidence is a test result or a code reference for that behavior. Also c… Passed Passed checks: 3 passed
Check name Status Explanation Title check Passed The title clearly and concisely describes the main change: Codex forks no longer request the full turn history. Description check Passed The description covers the problem, change, scope, linked issue, regression tests, integration tests, typecheck, lint, and runtime verification. It also states known verification limits. Out of Scope Changes check Passed All changes relate to the fork request named in #24460. The adapter change adds excludeTurns: true to the existing thread/fork request. The CodexAdapterV2.test.ts changes update the exact replay expec…Full details: Out of Scope Changes check
Explanation
All changes relate to the fork request named in
#24460. The adapter change adds excludeTurns: true to the existing thread/fork request. The CodexAdapterV2.test.ts changes update the exact replay expectation for that request and add a regression for the inclusive-boundary fork. The two fixture changes update the outbound thread/fork frames in the same way, and the merge-back fixture also records the sibling forks' merge result. No settings, persisted state, client contracts, or render paths change, consistent with the PR description and the changed-file list. The adapter hunk could not be read directly, so this verdict relies on the AI summary of that hunk and on the matching test expectations. The changed-file list shows no unrelated file.
Full details: Linked Issues check
Explanation
Directly linked issue
#24460has one active coding target: the fork path must send excludeTurns: true. Evidence supports this. The test diff in CodexAdapterV2.test.ts adds excludeTurns: true to the expected thread/fork outbound frame in the paginated-history fallback test. A new test asserts that forking at the inclusive lastTurnId (native-turn-first) sends excludeTurns: true, returns the fork metadata, and sends no further request. The fixture diffs for thread_fork_native_prior_turn and thread_merge_back_siblings update the same outbound frames. The PR summary reports a runtime check with Codex CLI 0.163.0 that shows the same request and an empty turns response. The forkThread hunk in CodexAdapterV2.ts could not be read directly (changed blob unavailable). Its +1 line change rests on the AI summary and the matching test expectations. The issue also asks for focused coverage of pending-request completion when the transport reader fails. The changed files do not include a test for that path, and the PR says it did not test it. The issue expects that an oversized response must not strand pending requests. Whether existing coverage at the reviewed head already asserts that behavior was not established within the inspection limit. The fork-local revert fallback is not directly asserted by the new test. It is covered only indirectly by the updated fallback test.Resolution
Establish whether the reviewed head already has a test that asserts pending requests complete with an error when the transport reader fails. If it does not, the missing evidence is a test result or a code reference for that behavior. Also confirm that the forkThread hunk in CodexAdapterV2.ts contains only the excludeTurns: true addition, using a readable diff or blob.
Finishing Touches
📝 Generate docstrings
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
- Commit to this branch
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.