fix: cap oversized tool results

[RealBhupesh]

Summary

• add a shared 20,000-character, marker-aware head/tail cap for text returned to model context
• cap serialized structured results so JSON cannot bypass the limit
• cover normal handlers, optimized interactive flows, subagents, Bash, and custom tools
• consolidate the duplicate truncation helper and add focused regression coverage

Closes #769
Part of #772

Root cause

Tool results enter model context through several execution paths. Structured results also replace text in the AI SDK converter, so capping only content left diagnostics and future MCP JSON unbounded.

Validation

• focused changed-area suites: 126 passed
• pnpm run test:format
• pnpm run test:lint
• pnpm run test:knip
• pnpm run test:audit
• git diff --check

Current type/build/unit CI failures are inherited from the ACP SDK regression on main; prerequisite fix: #830.

[luisangelrod]

The hard-cap behavior here is well specified: the marker is deducted from contentBudget, and the tests assert result.length === maxLength at both the helper and call-site levels. I traced the normal, optimized MessageBuilder, ACP/plain (through processToolUse), and direct subagent paths; the coverage matches the issue's intended choke points while leaving structured payloads intact.

One merge-order concern: #777 adds a second shared 40/60 head-tail helper (truncateHeadAndTail) for the two existing 2,000-character call sites. Its limit means “retained source characters,” so its returned string exceeds the limit once the marker is added; this PR's truncateToolResult correctly treats the limit as a hard ceiling. If both land unchanged, the repo will have two near-identical helpers with incompatible budget semantics.

I recommend coordinating the two PRs so there is one explicit contract: either make #777 reuse this hard-cap helper (with a configurable/actionable marker if needed), or keep the specialized helper but name/document its retained-content semantics clearly. For the global cap, I would preserve this PR's exact-length behavior.

CI confirms type, format, lint, build, dependency, audit, and security checks are green. The unit-test job is red, matching the unrelated settings-tab baseline failure disclosed in the PR; I did not reproduce the full suite locally because the checkout's frozen install currently reports a patchedDependencies/lockfile mismatch.

[will-lamerton]

Hey @RealBhupesh! Thanks for this. Picking processToolUse as the choke point was the right instinct and better than what I suggested in the issue. I'd pointed at ToolManager, but message-handler.processToolUse turns out to be genuinely shared by the interactive loop, plain/conversation.ts, ACP, tool-executor and auto-diagnostics, so capping there plus MessageBuilder and the subagent path covers every text route I could find. The 20,000 character budget sitting above the 2,000 Bash preview limit is a sensible split too.

Two things before I can merge it.

Structured results bypass the cap entirely. The PR caps content and passes structuredContent through untouched, but message-converter.ts:81-83 only sends one or the other:

const output =
    msg.structuredContent !== undefined
        ? ({type: 'json', value: msg.structuredContent} as const)
        : ({type: 'text', value: msg.content} as const);

When a tool returns structured output the capped text is discarded and the uncapped JSON is what actually reaches the model, so the cap is a no-op on exactly those calls. Right now only lsp_get_diagnostics (lsp-get-diagnostics.tsx:66) emits structured output, so the practical impact is limited today. But that is the tool whose output balloons on a badly broken file, and the point of #769 was a cap that holds by construction rather than one a tool can opt out of. It also means the MCP case that motivated the issue would slip straight through as soon as an MCP tool returns structured content. Bounding the serialized JSON, or at minimum capping its size before it goes out, would close it.

There is now a duplicate helper. This is my fault for not sequencing the issues better: #777 landed earlier today and added source/utils/truncate-output.ts with truncateHeadAndTail, which does the same head/tail split as your new truncate-tool-result.ts. Your base predates it so there is no conflict to warn you, but merged as-is main ends up with two near-identical helpers.

Worth saying that yours is the better of the two and should be the one that survives. You subtract the marker from the budget before splitting; #777's does not:

truncateToolResult(50k, cap 20000)  -> 20000 chars, respects the cap
truncateHeadAndTail(50k, cap 20000) -> 20053 chars, overshoots

So the consolidation I'd like is to keep your marker-aware budgeting and move #777's two call sites (execute-bash.tsx and custom-tools/handler.ts) onto it, then delete truncate-output.ts.

Separately, this branch is 53 commits behind main, as are your other two open PRs, which is where that settings-tabs.spec.tsx failure comes from. It passes on current main. Could you rebase all three onto main? It will clear the phantom failure and, on this one, surface the helper collision so you can do the consolidation in place.

[RealBhupesh]

Thanks for the careful review. All requested blockers are addressed in the rebased branch:

• structured results are serialized and sent through the same marker-aware cap, so JSON can no longer bypass the limit
• Bash and custom-tool call sites now use truncateToolResult
• the duplicate truncate-output helper and tests are removed
• added regression coverage for structured output and the consolidated paths

Focused changed-area suites pass (126 tests), and format, lint, Knip, audit, and diff checks pass locally.

The three red workflow jobs now reproduce on current main: #822 upgraded the ACP SDK without adding its newly required providerId. I opened #830 as the minimal prerequisite fix; it passes the full local gate (6,278 tests, build, types, Knip, and audit).