diff --git a/src/agent/posix-tool-plugins.ts b/src/agent/posix-tool-plugins.ts index c5e2f8c46..83808e30f 100644 --- a/src/agent/posix-tool-plugins.ts +++ b/src/agent/posix-tool-plugins.ts @@ -11,7 +11,10 @@ import { editFileLineRangePlugin } from "../plugins/edit-file-line-range-plugin. import { ripgrepPlugin } from "../plugins/ripgrep-plugin.js"; import { toolOutputUriPlugin } from "../plugins/tool-output-uri-plugin.js"; import { lspHintPlugin } from "../plugins/lsp-hint-plugin.js"; -import { resultTruncationPlugin } from "../plugins/result-truncation-plugin.js"; +import { + resultTruncationPlugin, + type SpillBlobWriter, +} from "../plugins/result-truncation-plugin.js"; import { toolResultSecretScrubPlugin } from "../plugins/tool-result-secret-scrub-plugin.js"; import { shellGuardPlugin, type ShellTimeoutConfig } from "../plugins/shell-guard-plugin.js"; import { @@ -27,6 +30,9 @@ export interface CorePosixToolPluginsArgs { shellTimeout?: ShellTimeoutConfig; extraToolPlugins?: ToolPlugin[]; readFileGuard?: ReadFileGuardPluginOptions; + // Session blob-store writer oversized tool results spill their full content + // into. See result-truncation-plugin.ts. + getBlobWriter?: () => SpillBlobWriter | undefined; // Per-project settings.env, merged into the run_shell spawn environment. shellEnv?: Record; } @@ -60,6 +66,7 @@ export function buildCorePosixToolPlugins(args: CorePosixToolPluginsArgs): ToolP shellTimeout, extraToolPlugins = [], readFileGuard = {}, + getBlobWriter, shellEnv, } = args; // Pre-gate sandboxes honor yolo mode so outside-workspace path tools and shell @@ -69,7 +76,7 @@ export function buildCorePosixToolPlugins(args: CorePosixToolPluginsArgs): ToolP // regardless. const allowOutside = (): boolean => permissionGate.getSkipPermissions(); return [ - resultTruncationPlugin(), + resultTruncationPlugin(getBlobWriter !== undefined ? { getBlobWriter } : {}), toolResultSecretScrubPlugin(), pathEscapePlugin(cwd, createWorktreeRootsProvider(cwd), { allowOutside }), deleteFilePlugin(cwd, { allowOutside }), diff --git a/src/agent/tools.ts b/src/agent/tools.ts index dcfecbbef..0760e6370 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -21,6 +21,7 @@ import type { PermissionGate } from "../permission/gate.js"; import { buildCorePosixToolPlugins } from "./posix-tool-plugins.js"; import { createLazyBlobReader } from "./lazy-blob-reader.js"; import type { BlobReader } from "@intx/types/runtime"; +import type { SpillBlobWriter } from "../plugins/result-truncation-plugin.js"; import { connectMCPServer, type MCPClient } from "../mcp/client.js"; import { mcpClientToAgentTools } from "../mcp/plugin.js"; import { createDynamicToolRunner, type DynamicToolRunner } from "../tui/dynamic-tool-runner.js"; @@ -101,6 +102,15 @@ export interface AgentToolsetArgs { // Session blob store for tool-output:// reads; resolved when tools run so agent // rebuilds do not require recreating the posix toolset. getBlobReader?: () => BlobReader | undefined; + // Session blob-store writer oversized tool results spill their full, + // untruncated content into (see result-truncation-plugin.ts) — the same + // context store getBlobReader reads from, keyed distinctly so the reactor's + // own downstream size-cap transform never overwrites the spill. Resolved + // lazily like getBlobReader so a mid-process session rotation spills into + // the new session's store. Omitted only where there is no session store to + // write into (tests). Persists with the rest of the session's committed + // history — no separate cleanup. + getBlobWriter?: () => SpillBlobWriter | undefined; // Per-project settings.env, merged into the run_shell tool's spawn environment. shellEnv?: Record; // Whether a workflow is currently running. advance_workflow rides the wire @@ -190,6 +200,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise { }; let currentAgent: Agent | null = null; + let currentStorage: ContextStore | null = null; const overlay = resolveExecDirectorOverlay(config.director); @@ -396,6 +402,7 @@ export async function runExec(config: Config): Promise { ...(shellTimeout !== undefined ? { shellTimeout } : {}), ...(toolWatchdog !== undefined ? { toolWatchdog } : {}), ...(localSettingsForMode?.env !== undefined ? { shellEnv: localSettingsForMode.env } : {}), + getBlobWriter: () => currentStorage?.writeBlob, getBlobReader: () => { if (currentAgent === null) { throw new Error("blob reader requested before agent init"); @@ -610,6 +617,7 @@ export async function runExec(config: Config): Promise { const buildAgent = async (): Promise => { const storage = await createOptimizedContextStore(workdir); + currentStorage = storage; const sources = liveSources.length > 0 ? liveSources : [liveSource]; const defaultSource = liveDefaultSource.length > 0 ? liveDefaultSource : liveSource.id; // Prefer liveSource credentials on the active id when OAuth was refreshed. diff --git a/src/mcp/plugin.ts b/src/mcp/plugin.ts index 6a1aca549..8c76c74b7 100644 --- a/src/mcp/plugin.ts +++ b/src/mcp/plugin.ts @@ -3,7 +3,10 @@ import type { ToolCall, ToolResult } from "@intx/types/runtime"; import type { PermissionGate } from "../permission/gate.js"; import { gateToolCall } from "../plugins/permission-plugin.js"; import { scrubSecretShapedToolResultContent } from "../plugins/tool-result-secret-scrub.js"; -import { truncateToolResultContent } from "../plugins/result-truncation-plugin.js"; +import { + truncateToolResultContent, + type SpillBlobWriter, +} from "../plugins/result-truncation-plugin.js"; import type { MCPClient } from "./client.js"; import { mcpToolName } from "./tool-name.js"; @@ -11,14 +14,21 @@ import { mcpToolName } from "./tool-name.js"; // middleware in src/plugins never see them. Apply the same scrub-then-truncate // order here directly (see buildCorePosixToolPlugins) so a compromised MCP // server cannot leak credential-shaped strings or flood the transcript. -function sanitizeMcpResultContent(content: string): string { - return truncateToolResultContent(scrubSecretShapedToolResultContent(content)); +function sanitizeMcpResultContent( + content: string, + spill?: { callId: string; writeBlob: SpillBlobWriter }, +): Promise { + return truncateToolResultContent(scrubSecretShapedToolResultContent(content), undefined, spill); } // Convert a connected client's tools into AgentTools for the dynamic runner used // by the TUI. These tools live in a separate runner from the posix tool plugin // chain, so each handler is wrapped with the permission gate directly. -export function mcpClientToAgentTools(client: MCPClient, gate: PermissionGate): AgentTool[] { +export function mcpClientToAgentTools( + client: MCPClient, + gate: PermissionGate, + getBlobWriter?: () => SpillBlobWriter | undefined, +): AgentTool[] { return client.tools.map((tool) => ({ kind: "full" as const, definition: { @@ -30,7 +40,9 @@ export function mcpClientToAgentTools(client: MCPClient, gate: PermissionGate): gateToolCall(gate, call, signal, async () => { try { const content = await client.call(tool.name, call.arguments, signal); - return { callId: call.id, content: sanitizeMcpResultContent(content) }; + const writeBlob = getBlobWriter?.(); + const spill = writeBlob !== undefined ? { callId: call.id, writeBlob } : undefined; + return { callId: call.id, content: await sanitizeMcpResultContent(content, spill) }; } catch (err) { return { callId: call.id, diff --git a/src/plugins/result-truncation-plugin.test.ts b/src/plugins/result-truncation-plugin.test.ts index 008c9fdbb..24a2b2827 100644 --- a/src/plugins/result-truncation-plugin.test.ts +++ b/src/plugins/result-truncation-plugin.test.ts @@ -1,61 +1,170 @@ import { describe, expect, test } from "bun:test"; import { createSizeCapTransform } from "@intx/inference"; -import type { StrategyContext, ToolResult } from "@intx/types/runtime"; -import { MAX_RESULT_CHARS, truncateToolResultContent } from "./result-truncation-plugin.js"; +import { createBlobReader, type StrategyContext, type ToolResult } from "@intx/types/runtime"; +import { + MAX_RESULT_CHARS, + resultTruncationPlugin, + spillBlobKey, + truncateToolResultContent, +} from "./result-truncation-plugin.js"; + +/** In-memory stand-in for ContextStore's writeBlob/readBlob pair, for tests. */ +function fakeBlobStore() { + const blobs = new Map(); + return { + blobs, + writeBlob: async (key: string, bytes: Uint8Array) => { + blobs.set(key, bytes); + }, + readBlob: async (key: string) => { + const bytes = blobs.get(key); + if (bytes === undefined) throw new Error(`Blob not found: ${key}`); + return bytes; + }, + }; +} describe("truncateToolResultContent", () => { - test("within-cap content passes through unchanged", () => { + test("within-cap content passes through unchanged", async () => { const content = "x".repeat(100); - expect(truncateToolResultContent(content)).toBe(content); + expect(await truncateToolResultContent(content)).toBe(content); }); - test("oversized content gets a marker that never promises retrievable remainder", () => { + test("oversized content with no blob store gets a marker that never promises retrievable remainder", async () => { const content = "x".repeat(MAX_RESULT_CHARS + 500); - const truncated = truncateToolResultContent(content); + const truncated = await truncateToolResultContent(content); expect(truncated).toContain("[output truncated"); expect(truncated).toContain("NOT retrievable"); // The pre-cap discard must never be described as recoverable elsewhere. expect(truncated).not.toContain("see the rest"); expect(truncated).not.toContain("Full output available"); + // And it must never promise a lifetime it doesn't control either way. + expect(truncated).not.toContain("removed"); + expect(truncated).not.toContain("session ends"); }); - test("truncation marker survives the size-cap blob spill", async () => { - // Reproduce the production pipeline for an output over MAX_RESULT_CHARS: - // truncation runs first (at the tool), size-cap spills the already-cut - // text to a blob and tells the model the blob holds the full output. The - // blob's tail must therefore carry the honest "discarded, NOT retrievable" - // marker so the model does not loop re-running the command. - const original = "x".repeat(MAX_RESULT_CHARS + 500); - const truncated = truncateToolResultContent(original); - - const blobs = new Map(); - const transform = createSizeCapTransform({ - maxChars: 10_000, - contextStore: { - writeBlob: async (key: string, bytes: Uint8Array) => { - blobs.set(key, new TextDecoder().decode(bytes)); - }, - }, + test("the inlined portion stays bounded regardless of blob-store support", async () => { + const content = "x".repeat(MAX_RESULT_CHARS * 3); + const truncated = await truncateToolResultContent(content); + // The marker text itself adds a bounded amount of overhead on top of the cap. + expect(truncated.length).toBeLessThan(MAX_RESULT_CHARS + 1000); + }); + + describe("with a blob store", () => { + test("a result over the cap is fully recoverable by following the notice's read_file instructions verbatim", async () => { + const store = fakeBlobStore(); + const original = `${"x".repeat(MAX_RESULT_CHARS)}TAIL-MARKER-${"y".repeat(500)}`; + const truncated = await truncateToolResultContent(original, MAX_RESULT_CHARS, { + callId: "call-42", + writeBlob: store.writeBlob, + }); + + // Inline content is bounded and does not itself contain the discarded tail. + expect(truncated).not.toContain("TAIL-MARKER"); + expect(truncated.length).toBeLessThan(MAX_RESULT_CHARS + 1000); + + const uriMatch = /tool-output:\/\/\/\S+/.exec(truncated); + expect(uriMatch).not.toBeNull(); + const uri = uriMatch?.[0].replace(/[.\]]+$/, "") ?? ""; + expect(uri).toBe(`tool-output:///${spillBlobKey("call-42")}`); + + // Follow the notice's instructions literally: read_file with that URI, + // via the real BlobReader machinery read_file itself uses. + const blobReader = createBlobReader(store); + const recoveredBytes = await blobReader.read(uri); + const recovered = new TextDecoder().decode(recoveredBytes); + expect(recovered).toBe(original); + expect(recovered).toContain("TAIL-MARKER"); + expect(recovered.length).toBe(original.length); + + // No false lifetime claim: the blob is part of the committed session + // history, not something with its own expiry. + expect(truncated).not.toContain("removed"); + expect(truncated).not.toContain("session ends"); }); - const result: ToolResult = { - callId: "call-1", - content: truncated, - isError: false, - }; - const { output } = await transform.apply( - { call: { id: "call-1", name: "run_shell", arguments: {} }, result }, - {} as StrategyContext, + test("within-cap content never writes a blob", async () => { + const store = fakeBlobStore(); + await truncateToolResultContent("x".repeat(100), MAX_RESULT_CHARS, { + callId: "call-1", + writeBlob: store.writeBlob, + }); + expect(store.blobs.size).toBe(0); + }); + + test( + "the full spill survives the reactor's own downstream size-cap transform " + + "(CL-6908 regression: a same-keyed write here would let that second write clobber it)", + async () => { + const store = fakeBlobStore(); + const original = "p".repeat(500_000); + const truncated = await truncateToolResultContent(original, MAX_RESULT_CHARS, { + callId: "call-1", + writeBlob: store.writeBlob, + }); + + // Reproduce the production pipeline: this middleware's ToolResult + // continues into the reactor, which always runs its own size-cap + // transform (vendor/intx-inference, default cap 10,000 chars) on + // every result, keyed by the bare call id. + const reactorCap = createSizeCapTransform({ + maxChars: 10_000, + contextStore: { writeBlob: store.writeBlob }, + }); + const result: ToolResult = { callId: "call-1", content: truncated, isError: false }; + await reactorCap.apply( + { call: { id: "call-1", name: "run_shell", arguments: {} }, result }, + {} as StrategyContext, + ); + + // The reactor wrote its own (lossy) blob under the bare "call-1" key. + expect(store.blobs.has("call-1")).toBe(true); + // Our full spill lives under a distinct key and is untouched. + const blobReader = createBlobReader(store); + const recovered = new TextDecoder().decode( + await blobReader.read(`tool-output:///${spillBlobKey("call-1")}`), + ); + expect(recovered).toBe(original); + expect(recovered.length).toBe(500_000); + }, + ); + }); +}); + +describe("resultTruncationPlugin", () => { + test("spills oversized run_shell/grep/search_files/web_fetch results via the live getBlobWriter getter", async () => { + const store = fakeBlobStore(); + const original = "q".repeat(MAX_RESULT_CHARS + 200); + const plugin = resultTruncationPlugin({ getBlobWriter: () => store.writeBlob }); + if (plugin.middleware === undefined) throw new Error("expected middleware"); + const middleware = plugin.middleware(async (call) => ({ + callId: call.id, + content: original, + })); + + const result = await middleware( + { id: "call-99", name: "run_shell", arguments: {} }, + new AbortController().signal, ); - const spilled = blobs.get("call-1"); - expect(spilled).toBe(truncated); - // The blob's tail tells the truth about the pre-spill discard. - expect(spilled).toContain("NOT retrievable"); - expect(spilled?.endsWith("Use offset/limit or a narrower query.]")).toBe(true); - // The inline marker's blob promise is now genuine: the blob really does - // hold everything that still exists. - expect(output.content).toContain("tool-output:///call-1"); + const uri = `tool-output:///${spillBlobKey("call-99")}`; + expect(result.content).toContain(uri); + const recovered = new TextDecoder().decode(await createBlobReader(store).read(uri)); + expect(recovered).toBe(original); + }); + + test("falls back to the honest no-store notice when getBlobWriter resolves undefined", async () => { + const plugin = resultTruncationPlugin({ getBlobWriter: () => undefined }); + if (plugin.middleware === undefined) throw new Error("expected middleware"); + const middleware = plugin.middleware(async (call) => ({ + callId: call.id, + content: "r".repeat(MAX_RESULT_CHARS + 1), + })); + const result = await middleware( + { id: "call-1", name: "grep", arguments: {} }, + new AbortController().signal, + ); + expect(result.content).toContain("NOT retrievable"); }); }); diff --git a/src/plugins/result-truncation-plugin.ts b/src/plugins/result-truncation-plugin.ts index c77796813..b958748bd 100644 --- a/src/plugins/result-truncation-plugin.ts +++ b/src/plugins/result-truncation-plugin.ts @@ -6,6 +6,35 @@ const TRUNCATABLE_TOOLS = new Set(["read_file", "grep", "run_shell", "search_fil // 80 000 chars ≈ 20 000 tokens. Keeps a single result from dominating context. export const MAX_RESULT_CHARS = 80_000; +/** Writes a blob to the session's context store (ContextStore.writeBlob's shape). */ +export type SpillBlobWriter = ( + key: string, + bytes: Uint8Array, + contentType: string, +) => Promise; + +/** Session blob-store handle a truncation can spill its full content into. */ +export interface TruncationSpillOptions { + callId: string; + writeBlob: SpillBlobWriter; +} + +/** + * Blob key the full pre-cut content is written under. Deliberately NOT the + * bare callId: the reactor's own size-cap transform (vendor/intx-inference's + * assembly.ts, always on, default cap 10,000 chars) runs on every ToolResult + * after this middleware returns it, and — because our inline "kept" text can + * itself exceed that cap — spills its own (already-truncated-by-us) copy to + * `contextStore.writeBlob(call.id, ...)`. Writing our full spill under the + * same key would let that second write silently clobber it with a lossier + * copy (confirmed by reproducing the two writes back to back in + * result-truncation-plugin.test.ts). The ":full" suffix keeps our blob a + * distinct entry the reactor never touches. + */ +export function spillBlobKey(callId: string): string { + return `${callId}:full`; +} + // The single primitive for size truncation: callers may pass their own // threshold but never invent their own wording, so a result can never carry // two differently-worded "truncated" notices. Called directly by runners this @@ -13,28 +42,62 @@ export const MAX_RESULT_CHARS = 80_000; // posix chain gets this middleware prepended unconditionally in // posix-tool-plugins.ts, so plugins like ripgrepPlugin that answer without // calling next() no longer need to apply the cap themselves. -export function truncateToolResultContent( +// +// When `spill` is supplied, the FULL (pre-cut) content is written to the +// session's own blob store — the same `ContextStore.writeBlob` / +// `tool-output:///{key}` machinery the reactor's own size-cap transform uses +// — and the notice names that real URI. Writing into the blob store (rather +// than a side file) means the content is staged and committed with the rest +// of the turn (see createOptimizedContextStore), so it persists exactly as +// long as the session's own history does: forever, by design, same as every +// other spilled tool output. No separate cleanup exists or is needed. +// +// Without `spill` (tests, or a caller with no session store to write into) +// the notice says plainly that the rest is gone; it must never claim a +// retrieval path that does not exist (CL-6908). +export async function truncateToolResultContent( content: string, maxChars: number = MAX_RESULT_CHARS, -): string { + spill?: TruncationSpillOptions, +): Promise { if (content.length <= maxChars) return content; const remaining = content.length - maxChars; - // Truncation happens here, at the source — before the reactor's size-cap - // transform spills to a tool-output:/// blob. The blob therefore holds only - // this already-truncated text, so the marker must say the remainder is gone: - // a "see the blob for the rest" promise would send the model chasing content - // that does not exist and re-running the command in a loop. + const kept = content.slice(0, maxChars); + + if (spill === undefined) { + return ( + kept + + `\n[output truncated at ${maxChars.toLocaleString()} chars — ` + + `${remaining.toLocaleString()} chars discarded, NOT retrievable ` + + `(no blob store is configured; re-running gives the same cut). ` + + `Use offset/limit or a narrower query.]` + ); + } + + const key = spillBlobKey(spill.callId); + const uri = `tool-output:///${key}`; + await spill.writeBlob(key, new TextEncoder().encode(content), "text/plain"); return ( - content.slice(0, maxChars) + + kept + `\n[output truncated at ${maxChars.toLocaleString()} chars — ` + - `${remaining.toLocaleString()} chars discarded, NOT retrievable ` + - `(no tool-output URI has them; re-running gives the same cut). ` + - `Use offset/limit or a narrower query.]` + `${remaining.toLocaleString()} more chars omitted here. The full result ` + + `(${content.length.toLocaleString()} chars) is saved at ${uri} — ` + + `use read_file with that URI (offset/limit supported) to see the rest.]` ); } -export function resultTruncationPlugin(): ToolPlugin { +export interface ResultTruncationPluginOptions { + // Live getter for the session's blob writer, re-read on every call so a + // session rotation (new sessionId mid-process) spills into the new + // session's store rather than a stale one. Omitted only where there is no + // session store to spill into (tests, ad-hoc toolsets) — truncation still + // runs, just without a retrievable remainder. + getBlobWriter?: () => SpillBlobWriter | undefined; +} + +export function resultTruncationPlugin(options: ResultTruncationPluginOptions = {}): ToolPlugin { + const { getBlobWriter } = options; return { middleware: (next) => async (call, signal) => { const result = await next(call, signal); @@ -42,7 +105,9 @@ export function resultTruncationPlugin(): ToolPlugin { const { content } = result; if (typeof content !== "string") return result; - const truncated = truncateToolResultContent(content); + const writeBlob = getBlobWriter?.(); + const spill = writeBlob !== undefined ? { callId: call.id, writeBlob } : undefined; + const truncated = await truncateToolResultContent(content, MAX_RESULT_CHARS, spill); if (truncated === content) return result; return { ...result, content: truncated }; }, diff --git a/src/tui/runner.ts b/src/tui/runner.ts index 26ea3a151..73b7a3f96 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -147,7 +147,12 @@ import { observeFleet, taskToolDefinition, } from "../subagent/index.js"; -import type { InferenceSource, ToolDefinition, InboundMessage } from "@intx/types/runtime"; +import type { + ContextStore, + InferenceSource, + ToolDefinition, + InboundMessage, +} from "@intx/types/runtime"; import { OPERATOR_ORIGINATED_FLAG } from "../agent/message-provenance.js"; import { createSessionOperationQueue } from "./session-operation-queue.js"; import { setAgentSourceUnlessClosed } from "./agent-source-sync.js"; @@ -1205,6 +1210,9 @@ export async function runTUI(initialConfig: Config): Promise { // Assigned before any tool runs; getter wires session blob reads into posix tools. let currentAgent!: Agent; + // Set alongside currentAgent in buildAgent; getter wires the session's own + // blob store into the truncation spill path (see result-truncation-plugin.ts). + let currentStorage: ContextStore | null = null; const toolset = await createAgentToolset({ cwd: config.cwd, @@ -1216,6 +1224,7 @@ export async function runTUI(initialConfig: Config): Promise { ...(localSettingsForEnv?.env !== undefined ? { shellEnv: localSettingsForEnv.env } : {}), toolWatchdog: liveToolWatchdog, getBlobReader: () => currentAgent.blobReader, + getBlobWriter: () => currentStorage?.writeBlob, isWorkflowActive: () => workflowControllerHolder.instance?.isActive() === true, ...(extraToolPlugins.length > 0 ? { extraToolPlugins } : {}), onOperatorGate: (question, options) => @@ -1484,6 +1493,7 @@ export async function runTUI(initialConfig: Config): Promise { const buildAgent = async (): Promise => { const storage = await createOptimizedContextStore(workdir); + currentStorage = storage; const sources = liveSources.length > 0 ? liveSources : [liveSource]; const defaultSource = liveDefaultSource.length > 0 ? liveDefaultSource : liveSource.id; return createAgentWithLiveToolDispatch(def, {