fix(core): Bound active tool result history - #5111

@qwen-code /triage

✅ Local verification report — build, focused tests, real-execution harness, and a live --debug session

I verified this PR locally three ways: (1) reproduced the author's build/test/typecheck commands, (2) drove the built size-budget algorithm directly with controlled synthetic history to check every edge case, and (3) ran a real interactive session under tmux with --debug and confirmed the new size-triggered microcompaction actually fires. Posting as a merge reference.

Setup

• Branch codex/fix-5101-tool-result-history-budget @ 3f44646 (fresh git worktree + npm install + npm run build), CLI v0.18.0
• Platform 🐧 Linux / Node v22.22.2 — the PR table marks Linux ⚠ untested, so this covers it.

1) Author's commands reproduced — all green

2) Real-execution harness against the built module — 17/17

I imported the compiled microcompactHistory from dist and fed it controlled histories. Every behavior the PR claims holds:

PASS  1. threshold -1 disables size trigger
PASS  2. legacy minutes:-1 (new field unset) disables size trigger
PASS  3. total (1500) <= threshold (3000) → no clearing
PASS  4a. size trigger fired (triggerReason=size)  — {before:8000, after:3000, thr:3000, cleared:5, kept:2}
PASS  4b–4f. before=8000, after≤3000, cleared OLDEST first, recent 2 preserved, history length kept
PASS  5. error results not counted → no clearing despite 30000 raw chars
PASS  6. already-cleared results not re-counted/cleared
PASS  7. non-compactable tool not counted → no clearing
PASS  8. pending tail counts toward virtual total (before=3500) but is NOT itself cleared; history length unchanged
==== 17 passed, 0 failed ====

This independently confirms: oldest-first clearing, total bounded to the threshold, recent-N protected, and correct skipping of error / already-cleared / non-compactable / pending-tail parts — matching the PR's own new tests (size-compacts old tool results…, counts pending content as a virtual tail…, does not clear protected recent results…, does not size-compact errors or already-cleared…, etc.).

3) Live tmux session with --debug — the size trigger fires for real

Test project with context.clearContextOnIdle.toolResultsTotalCharsThreshold: 4000, toolResultsNumToKeep: 2. Launched qwen --yolo --debug, had the model run seq 1 800 (~3,092 chars/result) several times via run_shell_command, accumulating compactable tool history. The new [TOOL-RESULT MC] path fired (debug log):

[CLIENT] [TOOL-RESULT MC] tool result chars 9603 > 4000, cleared 1 tool result(s) (~801 tokens), now 6402, kept 1 tool result(s)

• ✅ Size-triggered microcompaction fires on the ToolResult pre-send path, with the pending result counted as a virtual tail (9603 > 4000).
• ✅ 0 microcompaction failures in the whole session.
• ✅ The idle ([TIME-BASED MC]) path correctly did not fire (the session was active, not idle) — the two triggers are cleanly separated.

Observations (none are blockers)

1. The threshold is a soft bound, by design. When the most-recent toolResultsNumToKeep results alone exceed the threshold, clearing stops with history still above it (the live log shows now 6402 > 4000, kept 1). This is correct — the protected recent tail is never evicted — but worth a one-line doc note so it isn't mistaken for under-clearing.
2. Size trigger accounts for and clears text tool outputs only (functionResponse.response.output strings); media/inline-data parts are intentionally preserved on the size path (mediaKept = 0) and remain bounded only by the idle path. This matches the PR's "media preservation for size-triggered cleanup" note — flagging it purely as a scope reminder.
3. Default 500000 is conservative and the legacy toolResultsThresholdMinutes: -1 compatibility (disables the size trigger too unless explicitly set) is verified, so existing users see no behavior change.

Verdict

Solid, well-tested fix that does exactly what #5101 needs: a client-side cumulative budget that bounds active tool-result history before the provider request, reusing the existing microcompaction path. Build/typecheck/tests are clean, the algorithm is correct across all edge cases under direct execution, and it fires correctly in a live session. LGTM — recommend merge. The only follow-up I'd suggest is a one-line doc note that the threshold is a soft bound (recent results are always preserved).

==== 17 passed, 0 failed ====

[CLIENT] [TOOL-RESULT MC] tool result chars 9603 > 4000, cleared 1 tool result(s) (~801 tokens), now 6402, kept 1 tool result(s)

@qwen-code /triage

✅ Local runtime verification — active tool-result history is bounded on the wire (A/B)

I drove the real CLI through an agentic tool loop and captured the outbound provider payload to confirm this PR clears older compactable tool results once the cumulative-char budget is exceeded — with a before/after A/B that uses the PR's own on/off setting. Tested at PR head aad757e50 on 🐧 Linux, Node v22.22.2.

How I tested

• Built the PR in an isolated worktree (npm install → full workspace tsc build + bundle, exit 0). The fetched head is exactly the 12 PR files (no unrelated commits bundled).
• Ran the real CLI non-interactively (-p … --approval-mode yolo, which routes tool results through SendMessageType.ToolResult → the new size-clear checkpoint in client.ts before each provider request) against a mock OpenAI provider that captures every outbound messages[].
• The mock drives a 2-round run_shell_command loop; each call emits ~22k chars of compactable output (under the 30k shell cap), tagged MARKER_R1 / MARKER_R2.
• A/B is a pure settings flip (no code edit — this is the PR's own switch), with QWEN_MC_KEEP_RECENT=1 so only the 1 most-recent result is protected:
  • ON: context.clearContextOnIdle.toolResultsTotalCharsThreshold = 30000
  • OFF: … = -1 (documented disable)

Result — REQ#3 (the provider request after round 2's tool result)

With the budget active, once R1 + pending R2 exceeded 30000 chars the older result was replaced by the placeholder before the request left the client, while the most-recent result was protected — exactly the cumulative client-side bound #5101 asked for. With the size trigger disabled (-1), both results stay full and history grows unbounded across rounds (the bug). The only variable changed between the two runs is toolResultsTotalCharsThreshold.

Result — PR's automated coverage + build

Notes for review

• The clear happens on ToolResult turns before the provider dispatch (client.ts microcompactHistoryBeforeSend({sizeOnly:true, pendingContent})), and the pending (not-yet-in-history) tool result is counted as a virtual tail toward the budget but is itself protected — I confirmed R2 (the pending tail) was never cleared.
• Recent protection is count-based (toolResultsNumToKeep, default 5; I used 1 for a 2-round demo via QWEN_MC_KEEP_RECENT). It's a soft threshold — protected recents can keep the total above it by design.
• Compactable tools are read_file, run_shell_command, grep, glob, web_fetch, replace, write_file; error / already-cleared / non-string (media) outputs are not counted or cleared on the size path.
• Disable semantics verified as documented: toolResultsTotalCharsThreshold: -1 disables only the size trigger; legacy toolResultsThresholdMinutes: -1 (with the new key unset) disables both idle and size.
• Risk noted in the PR holds: very old tool output beyond the protected window is replaced by the placeholder and no longer visible to the model — acceptable trade for bounding provider history.

Verdict

Confirmed end-to-end on the wire: older compactable tool results are cleared once the cumulative char budget is exceeded, recent results are preserved, and the change is fully gated by the new (and -1-disable) setting. Focused core/cli suites + typecheck + build all green. Resolves #5101. LGTM from a verification standpoint. 👍