Drop tool results orphaned by trimMessagesToFitTokenLimit at the removal boundary

[nordicnode]

Problem & Context

trimMessagesToFitTokenLimit removes a contiguous run of oldest messages until the token budget is met — and the run can stop exactly between an assistant message carrying tool-calls and its role: 'tool' results. The surviving tool message then reaches the provider without its call and the whole step fails with tool_call_id does not exist (observed on the openai-compatible lane; consumed via getMessagesSubset by find-files/request-files-prompt.ts:201,271).

Reproduction (pre-fix, against the real module):

messages = [
  userMessage('please write the file'),
  assistantMessage({ content: [{ type: 'tool-call', toolCallId: 'c1', toolName: 'write_file', input: { content: 'x'.repeat(4000) } }] }),
  { role: 'tool', toolName: 'write_file', toolCallId: 'c1', content: jsonToolResult('ok') },
  assistantMessage('done'),
]
trimMessagesToFitTokenLimit({ messages, systemTokens: 0, maxTotalTokens: 600, ... })
→ output roles: ["user", "user", "tool", "assistant"]   // "tool" has no preceding call → provider 400

dropUnansweredToolCalls at the conversion chokepoint intentionally handles only the mirror case (calls whose results are gone) — orphaned results were the gap.

Changes Made

• packages/agent-runtime/src/util/messages.ts (trimMessagesToFitTokenLimit): after the removal loop, drop any surviving role: 'tool' message whose toolCallId was present in the input history but is not provided by any surviving assistant message. +38 lines, no other behavior touched.
• packages/agent-runtime/src/util/__tests__/messages.test.ts: five regression/behavior-preservation tests importing the real production function.

Deliberate scoping: only results orphaned by this trim are dropped (their call existed in the input and was removed). Orphans already present in a malformed input history pass through unchanged, as before — this fix does not silently rewrite histories it did not break. providerExecuted calls are excluded from both sets, mirroring the pairing semantics of dropUnansweredToolCalls.

Architecture & Conventions Conformance

• Adheres to Dependency Injection (contracts defined in common/src/types/contracts/, no module monkey patching) — pure function change, logger injected as before
• Terminal commands use terminalCommandBroker (no direct spawn or TUI-process bypass) — no process execution touched
• Environment hygiene respected (getCliEnv() for CLI, getSdkEnv() for SDK, no forbidden getProcessEnv() imports) — no env access added
• Freebuff mode compatibility (IS_FREEBUFF preserved, no paid features introduced) — product-agnostic bug fix
• Imports ordered and explicit (import type used for types) — no import changes

Scope Verification

• All modified files are within allowed public directories: packages/agent-runtime/
• NO modifications to web/, freebuff/web/, packages/internal/, packages/billing/, packages/bigquery/, or packages/build-tools/

Testing & Verification

• bun run build:sdk (passed cleanly)
• bun run build:freebuff (passed cleanly)
• bun cli/scripts/smoke-binary.ts cli/bin/freebuff (passed cleanly — with the CI env var set and a writable HOME; the sandboxed checkout has a read-only home directory)
• Unit / integration tests added or updated in affected package
• Anti-flake principles observed (no sleeps, no ports, no filesystem, no timers; the budget-sweep test asserts a structural invariant rather than fragile token arithmetic)

Tests were proven to catch the bug: with the fix stashed, 3 of the new tests fail against the unfixed source (boundary orphan, keepDuringTruncation-interleaved orphan, budget sweep); with the fix applied, all 32 tests in the file pass. Fix and tests were additionally verified by an independent adversarial pass (targeted attacks over duplicate call ids, multi-call assistant messages, providerExecuted asymmetry, keepDuringTruncation interaction, input-mutation checks, early-return path, plus a seeded property fuzz ≥ 3000 histories): every surviving tool result either has its call, was a pre-existing orphan, or is providerExecuted-related; input never mutated; no new failures.

Verification Output / Log Snippet

$ bun test packages/agent-runtime/src/util/__tests__/messages.test.ts
 32 pass
 0 fail
 92 expect() calls
Ran 32 tests across 1 file. [454.00ms]

$ bun run --cwd packages/agent-runtime test
 597 pass / 5 fail  (identical to main baseline: MCP schema ×2 = #1259, find-files WIP ×1-2 — not this change)

$ bun run --cwd packages/agent-runtime typecheck
 0 new errors (2 agents-graveyard = #1202; 1 from uncommitted find-files WIP, not this branch's files)

$ bun cli/scripts/smoke-binary.ts cli/bin/freebuff
 smoke-binary: tree-sitter init OK.
 smoke-binary: OK (matched /\x1b\[\?1049h/, exit code null, 9176 bytes captured, attempt 1/3).

New tests (all against the production module, no local reimplementations):

1. drops a tool result whose call was removed at the boundary — the reported 400 case; also asserts the final 'done' assistant message survives
2. drops tool results orphaned after a kept keepDuringTruncation message — removal runs are not pure prefixes when keepDuringTruncation messages sit mid-run, so the orphan can appear anywhere, not just leading the kept run
3. keeps tool results whose call survives the trim (no-trim passthrough)
4. leaves pre-existing orphans in an already-malformed history untouched (scoping guard, under an active trim)
5. keeps the invariant across a sweep of budgets (six budgets × two call pairs, structural assertion)

[codebuff-team]

Good find and a tight fix. The root cause — trimMessagesToFitTokenLimit's removal boundary can land between a tool-call and its role: 'tool' result, producing a history that providers reject with tool_call_id does not exist — is real and the fix in messages.ts addresses it directly without touching unrelated logic. The five tests in messages.test.ts cover the core case, the keepDuringTruncation interaction, the no-op case when nothing is removed, the deliberate non-goal (pre-existing malformed histories are left alone), and a budget sweep, which is the right level of coverage for a change to core trimming logic.

One readability nit, not blocking: the variable removedCallIds is actually populated from all call ids in the original messages (not just the ones removed by the trim), and is only narrowed to "actually removed" by subtracting survivingCallIds afterward. A name like inputCallIds would make the two-set diff obvious at a glance; as written a future reader has to trace the loop to realize it isn't what the name implies.

Worth double-checking before porting: behavior when a single assistant message carries multiple tool-calls and only some of their results get orphaned — the per-callId matching looks correct from reading it, but it'd be good to see an explicit test for that multi-call-per-message case since it's a plausible real shape from parallel tool calls.

Scope is clean (packages/agent-runtime only), no forbidden paths touched, and the PR doesn't overreach beyond the stated bug.

[nordicnode]

Thanks — both addressed in bce9434:

1. Rename: removedCallIds → inputCallIds, so the two-set diff (inputCallIds minus survivingCallIds) reads at a glance.
2. Multi-call-per-message test: added drops only the orphaned results when one assistant message carries multiple tool calls — one assistant message with parallel calls c1/c2, a keepDuringTruncation steer message between the results, tight budget. Asserts the no-orphan invariant, that neither orphaned result survives, and that the kept steer + final reply do. Plus a positive control (keeps every result of a multi-call assistant message that survives the trim) proving per-callId matching doesn't overcorrect when the multi-call message survives.

Red/green proof for the new multi-call test: against the true pre-fix file (git show 441947ed9^:packages/agent-runtime/src/util/messages.ts, zero occurrences of either set name) it fails 1/1; with the fix the full file is 34 pass / 0 fail under bun x bun@1.3.14. Package typecheck unchanged from the baseline noted in the body (2× agents-graveyard = #1202, 1× uncommitted find-files WIP; no new errors).