fix(agent-adapter): replay codex code-mode MCP calls, Script envelopes, and skill rows - #435
fix(agent-adapter): replay codex code-mode MCP calls, Script envelopes, and skill rows#435Zerlight wants to merge 2 commits into
Conversation
…s, and skill rows
There was a problem hiding this comment.
Pull request overview
This PR improves Codex rollout history replay in @linkcode/host/agent-adapter, especially for codex 0.144.x code-mode sessions, so reseeding a transcript from stored rollout history preserves MCP tool cards, properly unwraps script output envelopes, filters machine-injected “user” rows, and prevents oversized pages from exceeding transport limits.
Changes:
- Replay nested code-mode MCP calls from
event_msg mcp_tool_call_endrows and dedupe against response-backed tool rows. - Unwrap code-mode
Script ... / Wall time ... / Output:envelopes (and treat failed/terminated/verification-failure receipts as failures). - Extend history paging to account for tool-call
rawOutputpayload size (with tests covering paging and new replay behaviors).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/host/agent-adapter/src/native/codex/history.ts | Replays mcp_tool_call_end rows into tool-call events; filters additional synthetic user markers. |
| packages/host/agent-adapter/src/native/codex/history-tools.ts | Adds MCP end-row mapping and script-envelope parsing updates. |
| packages/host/agent-adapter/src/history-util.ts | Updates paging budget logic to include tool-call raw outputs. |
| packages/host/agent-adapter/src/tests/history-util.test.ts | Adds coverage ensuring tool raw outputs contribute to page budgeting. |
| packages/host/agent-adapter/src/tests/codex-history.test.ts | Adds unit tests for MCP end replay, deduping, oversized results, script envelope unwrapping, and synthetic row filtering. |
| packages/host/agent-adapter/AGENTS.md | Updates adapter behavior documentation for new history replay and paging behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| function eventPayloadLength(event: AgentHistoryEvent): number { | ||
| if (event.event.type === 'tool-call') { | ||
| const raw = event.event.toolCall.rawOutput; | ||
| return raw === undefined ? 0 : JSON.stringify(raw).length; | ||
| } |
There was a problem hiding this comment.
Fixed in f7fa4f4 — the page budget now measures Buffer.byteLength(JSON.stringify(...), "utf8"), and (per the sibling thread) covers the whole serialized tool call, not just rawOutput.
| rawOutput: | ||
| raw !== undefined && JSON.stringify(raw).length <= MCP_RESULT_MAX_JSON_LENGTH | ||
| ? raw | ||
| : undefined, |
There was a problem hiding this comment.
Fixed in f7fa4f4 — the cap is now Buffer.byteLength(JSON.stringify(raw), "utf8") <= MCP_RESULT_MAX_JSON_BYTES, with a regression test whose CJK result is under the cap in UTF-16 code units but over it in UTF-8 bytes.
There was a problem hiding this comment.
Important
Two of the three replay fixes land correctly, but the MCP dedup discards the structured failure status that only the mcp_tool_call_end row carries — the PR's own new test encodes a failed call replaying as completed. Separately, the Script failed branch is dead against the pinned 0.144.6 binary, while the Script error: form that binary does emit is left unwrapped.
Reviewed changes
native/codex/history.ts— newcollectRespondedToolCallIds+event_msg/mcp_tool_call_endreplay branch, three newSYNTHETIC_USER_MARKERS.native/codex/history-tools.ts— newcodexMcpEndToolCallwith a 256 KiBrawOutputcap, newSCRIPT_ENVELOPE_REunwrapping inparseCodexToolOutput.history-util.ts—eventAttachmentLength→eventPayloadLength, now counting toolrawOutputtoward the page budget.- Tests — 6 new codex-history cases, 1 new history-util case. I verified each new test genuinely fails without its corresponding fix (not test theatre), and the suite passes locally (2 files / 48 tests).
AGENTS.md— History bullet rewritten.
Confirmed correct: the call_id ⇒ live-item-id convergence claim in AGENTS.md checks out against codex-rs at tag rust-v0.144.6 — core/src/mcp_tool_call.rs builds McpToolCallItem { id: call_id.to_string() } and app-server-protocol/src/protocol/thread_history.rs uses id: payload.call_id.clone(). Replayed and live ids will match, so the transcript seed's uptoSeq cut will not duplicate cards.
Scope note (no line to anchor to): this is stacked on #434 (ruocheng/code-575); I reviewed only the delta against that base, so #434's own changes are out of scope here.
ℹ️ Nitpicks
collectRespondedToolCallIdskeys on bothCODEX_TOOL_ANNOUNCE_TYPESandCODEX_TOOL_OUTPUT_TYPES. The stated rationale is "a response row already produced this card", but only an output row settles one. Scoping the set to output types only is strictly smaller, matches the rationale, and lets an announce-only (truncated / interrupted) call still settle from itsevent_msgend row.- Every new
Wall timetest fixture is colon-less (Wall time 0.3 seconds). The pinned binary's string table hasWall time:with a colon, clustered withExit code:/Chunk ID:next tocore/src/tools/code_mode/execute_handler.rs.[^\n]*tolerates both so behaviour is fine, but the fixtures don't match what the binary emits. - The 256 KiB
MCP_RESULT_MAX_JSON_LENGTHelision dropsrawOutputtoundefinedsilently — from the user's side an oversized MCP result is indistinguishable from one that returned nothing. A one-line placeholder would read better.
Claude Opus | 𝕏
…w failure verdicts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/host/agent-adapter/src/history-util.ts:48
- sliceHistoryEventPage() now measures tool-call payload size by JSON.stringify()ing the entire ToolCall snapshot for every event. For large transcripts (especially tool calls with big text/diff output), this creates a large intermediate JSON string just to count bytes, which can be very costly in CPU and memory during paging.
A cheaper (and still byte-accurate for the large/unbounded fields) approach is to size the specific unbounded fields (rawInput/rawOutput, and text/diff content) without serializing the full object.
function eventPayloadLength(event: AgentHistoryEvent): number {
if (event.event.type === 'tool-call') {
return Buffer.byteLength(JSON.stringify(event.event.toolCall), 'utf8');
}
There was a problem hiding this comment.
Important
The delta itself is clean — both the page-budget and the MCP-status findings are fixed correctly, and I've resolved those threads. I'm not approving only because the third finding from the prior review is untouched and unanswered: Script failed is still dead against the pinned 0.144.6 binary while Script error: — the form that binary does emit — is still neither unwrapped nor failed.
Reviewed changes — f7fa4f44 only, the follow-up commit answering review 4891643854.
- Carried the MCP end row's failure verdict onto the response settle —
mcpEndFailuresrecordsErr/isErrorfor response-backed calls and overrides the settle's output-text heuristic, so a failed direct MCP call no longer replays green. - Widened the page budget to the whole serialized tool call —
eventPayloadLengthnow measuresBuffer.byteLength(JSON.stringify(toolCall), 'utf8'), socontent-borne exec output andapply_patchdiffs count, and the docstring no longer claims per-result caps bound every adapter. - Moved the 256 KiB MCP result cap onto UTF-8 bytes —
MCP_RESULT_MAX_JSON_LENGTH→MCP_RESULT_MAX_JSON_BYTES, closing the UTF-16 undercount. - Extracted
codexMcpEndFailed— the failure discriminator is now shared betweencodexMcpEndToolCalland the dedup path instead of being computed and thrown away. - Added three regression tests — a CJK result under the cap in code units but over it in bytes, a response-backed failed call now asserting
failed, and an exec row whose body ridescontentwithrawOutput: 0.
Verification I did rather than assumed. I enumerated the row orderings the reconciliation can see — end→announce→output, announce→end→output, announce→output→end, duplicate end rows, and a plan-call id collision — and every one lands on the right final status. The late-end-row branch at history.ts:562 is genuinely reachable, not dead: recordToolEvent writes every emitted snapshot back into announced, so announced.get(id) is the settled card by then. I also checked the budget units, since the measure is in UTF-8 bytes but the constant is named for base64 length: MAX_ATTACHMENT_TOTAL_BASE64_LENGTH is 16,777,216 ASCII base64 characters, the tunnel frames raw UTF-8 with no second encoding, and the assembler evicts on aggregate pending bytes rather than per-message size — so the two branches are commensurable and the budget errs conservative. Both history test files pass 49/49, and each new test fails without its fix.
ℹ️ Nitpicks
- On the dedup path,
mcpEndFailures.addalso fires for a call that has an announce row but no output row (a truncated or interrupted rollout). Nothing consumes the set in that case, so a call the end row says failed replays stuck atin_progress. ScopingcollectRespondedToolCallIdstoCODEX_TOOL_OUTPUT_TYPESonly — the standing nit from the prior review — would let that call settle from its end row instead. AGENTS.md:87still describes the page budget as "aggregate embedded-attachment payload … so image-heavy transcripts fan across cursor pages". After this commit whole tool payloads count too, and the motivating case is result-heavy transcripts.
Claude Opus | 𝕏

Summary
Reseeding a codex conversation from history (any SWR refocus revalidation rebuilds the
transcript from the rollout replay) was badly lossy for codex 0.144.x code-mode sessions:
list_issues · linearcard) vanished — code mode persiststhem only as
event_msg mcp_tool_call_endrows, whichmapCodexHistoryEventsignored.They now replay as MCP tool cards through the shared
codexMcpSlug; the event'scall_idISthe live
mcpToolCallitem id (verified in codex-rsrust-v0.144.6), so replayed and livecards converge by id and the seed's covered-by-seed cut holds. Legacy response-backed calls
(announce → end → output in real rollouts) skip the synthesized end row — the trailing settle
would otherwise overwrite a structured
failedwithcompleted.parseCodexToolOutputnow unwraps
Script completed|failed|terminated|running with cell ID N / Wall time / Output:and fails failed/terminated scripts.
apply_patch verification failedreceipts settle asfailed instead of completed.
<skill>(the invoked SKILL.md),<recommended_plugins>, and<codex_internal_contextuser rows rendered as real user bubbles; they joinSYNTHETIC_USER_MARKERS.Transport safety for the newly replayed results:
sliceHistoryEventPagenow counts toolrawOutputtoward the page budget alongside attachments, and a single stored MCP result caps at256 KiB serialized (oversized results replay status-only), so result-heavy transcripts fan
across cursor pages instead of exceeding the tunnel's reassembly budget.
Stacked on #434 (
ruocheng/code-575); review this branch's delta against it.Closes CODE-576
Verification
Err/isErrorstatuses, legacy dedup,oversized-result cap, Script envelope + statuses, verification-failure settle, skill-row
filtering, page-budget accounting); the two focused history files pass 48/48 and the full
vitest runpasses 2778 (--maxWorkers=2).mcp__linear__list_issues/list_teamscards with no<skill>bubble and clean exec bodies;the worst MCP-heavy rollout (~18 MB of results, 1301 events / 141 MCP cards) pages as
10.6 / 6.6 / 4.5 MB — all under the 20,971,520 transport budget (previously one over-budget page).
app-server generate-ts), thematching codex-rs source, and a sweep of 840 local rollouts (0.140 → 0.146-alpha).
Checklist
pnpm check:ciandpnpm testboth pass (pluscargo fmt/clippy/testfor Rust changes)WIRE_PROTOCOL_VERSIONis bumped