fix(codex): restore decode behaviour lost in the extraction - #928
fix(codex): restore decode behaviour lost in the extraction#928ozymandiashh wants to merge 1 commit into
Conversation
Four gaps between the pre-extraction decoder and the one in @codeburn/core. **Fork-replay filter.** A forked session replays its parent's history clustered at fork time. The old decoder skipped eight payload types when the timestamp preceded the fork cutoff; this one applied the cutoff only inside the token_count branch. Token and cost dedup therefore stayed correct — which is exactly why a cost-based parity check cannot see the bug — while the parent's replayed tool calls, patch applies and MCP tool ends leaked into the child's turn, inflating tool counts, edit LOC and failed-edit counts. Restored to the same position (after turn_context, before dispatch) with the same eight types. **custom_tool_call.** The old decoder ran function_call and custom_tool_call through one name map; this one handled only function_call, so custom tool calls disappeared from a turn's tools and toolSequence. The old custom_tool_call_output branch fed only the task-timing machinery, which does not exist here, so nothing pairs with it. **Buffer path fields.** Lines over 32 KB are scraped field-by-field rather than JSON-parsed, and that path set neither payload.info nor payload.invocation. `info` is latent — a token_count record is a few hundred bytes and never becomes a Buffer — but `invocation` is live: an mcp_tool_call_end with large arguments does cross the threshold, and the mcp tool name was being dropped from the turn. Both restored. **exec → Bash.** Dropped from the tool-name map; Codex Desktop's custom-tool transport uses the short name for the same shell tool. Tests pin each one: the fork fixture asserts the child turn stays empty while the parent keeps its tools, and fails without the fix.
Carries the regression tests from getagentseal#928, which is closed in favour of this branch. That PR reduced to tests alone once review established its source fixes were already here — and the tests are the part this branch was missing: it restored the behaviour without anything proving it stays restored. Each pins one restoration and fails against a decoder without it: custom tool calls reaching the turn, the eight-type fork-replay skip in both directions, the Buffer path synthesizing info and invocation, and exec mapping to Bash.
|
Closing in favour of #940, which now carries these tests. Cross-PR review established that every source fix here was already present in #940 — the eight-type fork-replay skip, Reducing this PR to its genuine remainder left exactly one thing: the four GAP regression tests. Those turned out to be the part #940 was actually missing — it restored the behaviour without anything pinning it. They now live there, verified failing against a decoder without each fix and green with it. Keeping this open would have meant a permanently red PR waiting on a sibling, which is not something to hand a maintainer. |
Four gaps between the pre-extraction Codex decoder and the one now in
@codeburn/core.Fork-replay filter narrowed from eight event types to one
A forked session replays its parent's history clustered at fork time. The old decoder skipped eight payload types when the timestamp preceded the fork cutoff; this one applied the cutoff only inside the
token_countbranch.Token and cost dedup therefore stayed correct — which is exactly why a cost-based parity check cannot see this — while the parent's replayed tool calls, patch applies and MCP tool ends leaked into the child's turn, inflating tool counts, edit LOC and failed-edit counts. Restored to the same position (after
turn_context, before dispatch) with the same eight types.custom_tool_callno longer handledThe old decoder ran
function_callandcustom_tool_callthrough one name map; this one handled onlyfunction_call, so custom tool calls vanished from a turn'stoolsandtoolSequence. The oldcustom_tool_call_outputbranch fed only the task-timing machinery, which does not exist here, so nothing pairs with it.Buffer path dropped fields the old one synthesised
Lines over 32 KB are scraped field-by-field rather than JSON-parsed, and that path set neither
payload.infonorpayload.invocation.Being precise about which matters:
infois latent — atoken_countrecord is a few hundred bytes and never becomes a Buffer.invocationis live — anmcp_tool_call_endwith large arguments does cross the threshold, and the MCP tool name was being dropped from the turn. Both restored, including the tail-window fallback the old code used and this one had lost.exec→Bashmapping droppedCodex Desktop's custom-tool transport uses the short name for the same shell tool.
Verification
Core 513, CLI codex 22. The fork test pins both directions: neutralise the skip and the child wrongly receives the parent's
['Bash','Edit','mcp__srv__t1']; make the skip unconditional and the parent's own assertions fail. A test that only fails one way is half a test.Two residuals, both inherited rather than introduced
Recorded because they are real and someone should decide about them, not because this PR causes them:
invocationsitting in the dead zone — past the 64 KB head, before the 16 KB tail — combined with a complete nestedinvocationin the tail yields a wrong MCP name rather than a missing one. Inherited verbatim from the old implementation's identical head-then-tail lookup.