Skip to content

fix(codex): stop re-sending file reads in full after they were trimmed - #99

Merged
Xubqpanda merged 2 commits into
zjunlp:mainfrom
boxabirds:claude/upstream-4-codex-repeat-read
Oct 3, 2026
Merged

Xubqpanda merged 2 commits into
zjunlp:mainfrom
boxabirds:claude/upstream-4-codex-repeat-read

Conversation

@boxabirds

Copy link
Copy Markdown

Summary

Same bug and same fix as #92, this time in the Codex proxy.

TokenPilot shortens large file reads before Codex sends them to the model. Because of a bug, the next request sent the same file in full again, which undid the saving and broke the prompt cache from that point on. This PR keeps a file shortened for as long as that same read is in the conversation.

In a local probe through the proxy's reduction path, a read trimmed to 2,793 characters went out at its full 78,248 characters on the very next request.

This only affects tools named read or file_read (for example MCP or custom read tools). Codex's built-in shell tool never records a read path, so it isn't affected.

What was going wrong

tool_payload_trim has a deliberate rule: if the model reads a file it has already seen in shortened form, it probably wants the whole file this time, so that second read is left in full.

To apply the rule, the proxy saves every path it has shortened (disclosedReadPaths) and passes the list back in on the next request. Codex resends the entire input history on every request, so the first read is sent again too. The pass then mistook that same old read for a new re-read and sent it in full.

The fix

The session snapshot now also records which read first disclosed each path, by its call_id (disclosedReadOwners). A path is carried into the next request only when that read is no longer in the history.

It follows the refinement made in #92: only a read/file_read result that was actually trimmed can own a path, and the owner list is rebuilt from the paths the pass reports on each request.

Situation Before After
The same read is still in the history Sent in full (bug) Stays shortened, byte-identical
That read has left the history and the model reads the file again Sent in full Sent in full (unchanged)
Snapshot from an older version (paths but no owners) Paths carried Ignored; the read is shortened as normal

Files changed:

  • src/reduction.ts: owner tracking, and the carry rule above.
  • src/session-state.ts and src/proxy-runtime.ts: save, load and merge the new field alongside disclosedReadPaths.

Tests

New file tests/reduction-disclosed-reads.test.ts:

  • O1–O3: unit tests for the three helpers.
  • O4: a non-read tool using the same path can't become the owner.
  • O5: owners for paths the pass no longer reports are dropped.
  • D1: the same read stays shortened and byte-identical on the next request. This is the regression test.
  • D2: a genuine re-read after the original has left the history is sent in full.
  • D3: an old-format snapshot does not force reads to go in full.
  • D4: the new field survives saving and session merges.
  • D5: when two reads share a path, the owner is the one that was actually trimmed.

In tests/reduction.test.ts, the existing "reuses disclosed read paths" test now also saves the new field, as the proxy does. What it checks is unchanged: a new read of the same file still comes back in full.

Checks run:

  • With the fix removed, O1–O5, D1, D3, D4 and D5 fail.
  • pnpm build, pnpm typecheck and check:boundaries pass.
  • codex passes 503 with 1 skipped, the same skip as main.

Prepared with help from Claude Code; the commits are marked Co-Authored-By: Claude.

claude added 2 commits October 3, 2026 09:16
The tool_payload_trim pass sends a file read in full when its path was already
disclosed, taking it as the model asking for the whole file again. The Codex
proxy persisted every disclosed path and passed them all back in on the next
request. Codex resends the whole input history, so the pass saw the same read
as a repeat of itself and sent it untrimmed (a probe: 2,793 chars on one request,
78,248 on the next), breaking the prompt cache from there on. Affects tools
named `read`/`file_read`; the built-in shell tool is not affected.

The session snapshot now records which call_id disclosed each path
(disclosedReadOwners). A path is carried into the next request only when that
read is no longer in the history, so a genuine re-read after compaction still
comes back in full. Snapshots from before this change are ignored rather than
carried.

Tests: tests/reduction-disclosed-reads.test.ts (O1-O3, D1-D4). The existing
"reuses disclosed read paths" test now persists owners as the proxy does; its
expectation (a new read of the same path comes back in full) is unchanged. D1
and D3 fail against the previous behaviour.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S6it2Si8k8XNGMF8RdyMQz
Applies the refinement made to the Claude Code fix in zjunlp#92 (eb1eb2c) to Codex:

- a path's owner must be a `read`/`file_read` result that tool_payload_trim
  actually trimmed in this request, not merely the first tool result with that
  path (a write to the same file, or an untrimmed read, can no longer own it);
- owners are rebuilt from the pass's bounded reported path set instead of
  accumulating.

Tests O4, O5 and D5 mirror the Claude Code ones; O4 and D5 fail without the
new ownership filter.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S6it2Si8k8XNGMF8RdyMQz
@Xubqpanda
Xubqpanda merged commit 9f0f193 into zjunlp:main Oct 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants