fix(server): avoid full history on Codex resume - #6400
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
ApprovabilityVerdict: Needs human review This PR changes session resume behavior by adding You can customize Macroscope's approvability policy. Learn more. |
CDVolvik
left a comment
There was a problem hiding this comment.
CodexSessionRuntime.test.ts: 21/21.
Resuming with a full history replay is slow and can re-emit old events. Avoiding that on resume is correct. MERGEABLE. Related to #6671 (missing rollout) — they touch the same file; rebase if both land.
Summary
excludeTurns: trueoptionWhy
T3 already persists and renders its own projected transcript, so it does not use the historical
thread.turnsreturned by Codex during provider resume. On sufficiently large Codex rollouts, rebuilding and buffering that full JSON-RPC response can starve the local server beforesession/ready; Desktop then reports that/.well-known/t3/environmenttimed out even though port 3773 is still listening.Codex exposes
excludeTurnsspecifically for clients that only need thread metadata and the live resume subscription. T3 already initializes Codex withcapabilities.experimentalApi: true. The generated request binding omits experimental request fields, so this follows the existing raw-request-plus-schema-decode pattern used byturn/startcollaboration mode.Fixes #6399
Related: #5351, #4773, #3601, #6153
Validation
pnpm vp fmt --check apps/server/src/provider/Layers/CodexSessionRuntime.ts apps/server/src/provider/Layers/CodexSessionRuntime.test.tspnpm vp test run apps/server/src/provider/Layers/CodexSessionRuntime.test.ts(21 passed)cd apps/server && pnpm typecheckgit diff --checkThe repository requests Node 24.13.1; these checks ran successfully on Node 26.5.1 with the package manager's engine warning.
Note
Medium Risk
Touches Codex session open/resume on the critical path; behavior change is narrow (metadata-only resume) with fallback preserved, but large-thread readiness regressions are the main thing to watch.
Overview
Codex thread resume no longer asks the app-server to rebuild and return full
thread.turnshistory.openCodexThreadnow callsclient.raw.request("thread/resume", …)withexcludeTurns: true, since T3 already owns the projected transcript and large rollouts were starving the local backend beforesession/ready.Resume responses are decoded through
V2ThreadResumeResponse(same raw-request + schema pattern asturn/start).thread/startand recoverable resume → fresh start fallback are unchanged. Tests mockclient.raw, align thread fixture shapes with the v2 schema, and assert the resume wire payload includesexcludeTurns: true.Reviewed by Cursor Bugbot for commit 0143edc. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Exclude historical turns when resuming a Codex thread
openCodexThreadto useclient.raw.requestforthread/resumeinstead of the typedclient.request, addingexcludeTurns: trueto the resume payload to avoid sending full history.decodeV2ThreadResumeResponseschema decoder, mapping decode failures toCodexAppServerProtocolParseError.thread/startas before.Macroscope summarized 0143edc.