From 56e455b6cf2f93fd7c63f33a53e00d5585d9b200 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 24 Aug 2026 15:29:46 -0700 Subject: [PATCH 1/2] Pretty-spill oversized tool results into session files Over the truncation gate, leisure-materialize content before spilling to the session blob store so the model gets a tool-output URI and an absolute on-disk path to the pretty (or preserved NDJSON) full result. --- docs/ARCHITECTURE.md | 1 + src/agent/posix-tool-plugins.ts | 12 +- src/agent/tools.ts | 10 +- src/exec/runner.ts | 1 + src/mcp/plugin.test.ts | 106 +++++++-- src/mcp/plugin.ts | 21 +- src/plugins/result-truncation-plugin.test.ts | 233 +++++++++++++++++-- src/plugins/result-truncation-plugin.ts | 219 +++++++++++++---- src/plugins/tool-result-materialize.test.ts | 77 ++++++ src/plugins/tool-result-materialize.ts | 110 +++++++++ src/tui/runner.ts | 2 + 11 files changed, 706 insertions(+), 86 deletions(-) create mode 100644 src/plugins/tool-result-materialize.test.ts create mode 100644 src/plugins/tool-result-materialize.ts diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 7361166d6..1e392e990 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -347,6 +347,7 @@ tool call **Rejection behavior:** Any plugin can short-circuit by returning a `ToolResult` with `isError: true`; the error propagates to the agent and downstream plugins/execution are skipped. +- **Result truncation / leisure materialization** (`result-truncation-plugin.ts`, `tool-result-materialize.ts`) — Caps model-facing tool results at 10,000 chars (aligned with the reactor size-cap). Over the gate, content is leisure-materialized first (minified JSON → pretty `application/json`; NDJSON preserved; else `text/plain`), then the formatted bytes are spilled to the session blob store under `{callId}:full` and truncated inline with a `tool-output:///` URI plus absolute `contextDir/tool-output/…` path when plumbed. Under-gate results are unchanged (no pretty, no spill). MCP tools apply the same scrub-then-truncate path via `mcpClientToAgentTools` since they skip the posix middleware chain. - **Path Escape** (`path-escape-plugin.ts`) — Canonicalizes path-like arguments against `cwd` and blocks `..` escapes, except into a root the permission layer's worktree-roots provider allowlists (e.g. a sibling git worktree of the same repo). Runs first so later plugins see resolved paths. - **Tool-output URI** (`tool-output-uri-plugin.ts`) — Normalizes mistaken `read_file` blob URIs to `tool-output:///id` (corbits-only; interchange stays unpatched). - **Secret Guard** (`secret-guard-plugin.ts`) — Hard-denies path-keyed tool calls (`read_file`, `write_file`, …) that would put a sensitive file into (or write it from) the model context. Runs before the permission plugin, so the path-arg deny holds even under `--dangerously-skip-permissions`. Shell commands that _reference_ a sensitive path (tokenized so `cat .env`, `bun --env-file=.env run …`, and quote/env-assignment forms are detected) are not hard-denied here: they require operator approval via the permission gate, and auto mode forces an ask through the auto-shell policy (`sensitive-path` rule). Once the operator approves, the command runs. Shell detection is best-effort: token matching defeats quoting and env-assignment/redirection forms but not dynamic path construction (variable indirection, `printf` assembly). Tool-result secret scrub still redacts credential-shaped output. diff --git a/src/agent/posix-tool-plugins.ts b/src/agent/posix-tool-plugins.ts index 83808e30f..90e6b8898 100644 --- a/src/agent/posix-tool-plugins.ts +++ b/src/agent/posix-tool-plugins.ts @@ -33,6 +33,8 @@ export interface CorePosixToolPluginsArgs { // Session blob-store writer oversized tool results spill their full content // into. See result-truncation-plugin.ts. getBlobWriter?: () => SpillBlobWriter | undefined; + // Absolute session context dir for the truncation notice's on-disk path. + getContextDir?: () => string | undefined; // Per-project settings.env, merged into the run_shell spawn environment. shellEnv?: Record; } @@ -67,6 +69,7 @@ export function buildCorePosixToolPlugins(args: CorePosixToolPluginsArgs): ToolP extraToolPlugins = [], readFileGuard = {}, getBlobWriter, + getContextDir, shellEnv, } = args; // Pre-gate sandboxes honor yolo mode so outside-workspace path tools and shell @@ -75,8 +78,15 @@ export function buildCorePosixToolPlugins(args: CorePosixToolPluginsArgs): ToolP // rebuilding the plugin stack. Secret-guard and authz still hard-deny // regardless. const allowOutside = (): boolean => permissionGate.getSkipPermissions(); + const truncationOptions = + getBlobWriter !== undefined || getContextDir !== undefined + ? { + ...(getBlobWriter !== undefined ? { getBlobWriter } : {}), + ...(getContextDir !== undefined ? { getContextDir } : {}), + } + : {}; return [ - resultTruncationPlugin(getBlobWriter !== undefined ? { getBlobWriter } : {}), + resultTruncationPlugin(truncationOptions), toolResultSecretScrubPlugin(), pathEscapePlugin(cwd, createWorktreeRootsProvider(cwd), { allowOutside }), deleteFilePlugin(cwd, { allowOutside }), diff --git a/src/agent/tools.ts b/src/agent/tools.ts index bac8e26a0..894646190 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -126,6 +126,9 @@ export interface AgentToolsetArgs { // write into (tests). Persists with the rest of the session's committed // history — no separate cleanup. getBlobWriter?: () => SpillBlobWriter | undefined; + // Absolute session context dir (`…/context`) for the truncation notice's + // on-disk path. Re-read live like getBlobWriter across session rotation. + getContextDir?: () => string | 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 @@ -216,6 +219,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise { ...(toolWatchdog !== undefined ? { toolWatchdog } : {}), ...(localSettingsForMode?.env !== undefined ? { shellEnv: localSettingsForMode.env } : {}), getBlobWriter: () => currentStorage?.writeBlob, + getContextDir: () => workdir, getBlobReader: () => { if (currentAgent === null) { throw new Error("blob reader requested before agent init"); diff --git a/src/mcp/plugin.test.ts b/src/mcp/plugin.test.ts index a3fe5d4e0..b5ede2021 100644 --- a/src/mcp/plugin.test.ts +++ b/src/mcp/plugin.test.ts @@ -1,6 +1,11 @@ import { describe, test, expect } from "bun:test"; import { mcpClientToAgentTools } from "./plugin.js"; import { createPermissionGate } from "../permission/gate.js"; +import { + MAX_RESULT_CHARS, + spillBlobKey, +} from "../plugins/result-truncation-plugin.js"; +import { toolOutputAbsolutePath } from "../plugins/tool-result-materialize.js"; import { CREDENTIAL_REDACTION } from "../plugins/tool-result-secret-scrub.js"; import type { MCPClient } from "./client.js"; @@ -19,15 +24,29 @@ function fakeClient(reply: string): MCPClient { }; } +function fakeBlobStore() { + const blobs = new Map(); + return { + blobs, + writeBlob: async (key: string, bytes: Uint8Array, contentType: string) => { + blobs.set(key, { bytes, contentType }); + }, + }; +} + +function skipGate() { + return createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: true, + cwd: process.cwd(), + }); +} + describe("mcpClientToAgentTools", () => { test("scrubs a credential-shaped MCP result the same as built-in tools", async () => { - const gate = createPermissionGate({ - approvals: [], - interactive: false, - skipPermissions: true, - cwd: process.cwd(), - }); - const client = fakeClient("here is the key: sk-live-abc123xyz789012345678"); + const gate = skipGate(); + const client = fakeClient("here is the key: sk-live-abc123abcdefghijklmnopqrst"); const [tool] = mcpClientToAgentTools(client, gate); expect(tool?.kind).toBe("full"); if (tool?.kind !== "full") throw new Error("expected full tool"); @@ -42,12 +61,7 @@ describe("mcpClientToAgentTools", () => { }); test("truncates an oversized MCP result the same as built-in tools", async () => { - const gate = createPermissionGate({ - approvals: [], - interactive: false, - skipPermissions: true, - cwd: process.cwd(), - }); + const gate = skipGate(); const huge = "x".repeat(90_000); const client = fakeClient(huge); const [tool] = mcpClientToAgentTools(client, gate); @@ -62,4 +76,70 @@ describe("mcpClientToAgentTools", () => { expect((result.content as string).length).toBeLessThan(huge.length); expect(result.content).toContain("output truncated"); }); + + test("pretty-spills oversized minified JSON with contextDir in the notice", async () => { + const gate = skipGate(); + const store = fakeBlobStore(); + const contextDir = "/tmp/session/context"; + + const obj: Record = {}; + for (let i = 0; i < 400; i++) { + obj[`key_${i}`] = `value_${i}_${"x".repeat(20)}`; + } + const minified = JSON.stringify(obj); + expect(minified.length).toBeGreaterThan(MAX_RESULT_CHARS); + const pretty = JSON.stringify(obj, null, 2); + + const client = fakeClient(minified); + const [tool] = mcpClientToAgentTools(client, gate, { + getBlobWriter: () => store.writeBlob, + getContextDir: () => contextDir, + }); + if (tool?.kind !== "full") throw new Error("expected full tool"); + + const result = await tool.handler( + { id: "c-mcp-json", name: "mcp__acme__fetch_secret", arguments: {} }, + new AbortController().signal, + ); + + const key = spillBlobKey("c-mcp-json"); + const entry = store.blobs.get(key); + expect(entry).toBeDefined(); + expect(entry?.contentType).toBe("application/json"); + expect(new TextDecoder().decode(entry!.bytes)).toBe(pretty); + + const uri = `tool-output:///${key}`; + const abs = toolOutputAbsolutePath(contextDir, key, "application/json"); + expect(result.content).toContain(uri); + expect(result.content).toContain(abs); + expect(result.content).toContain("application/json"); + expect(result.content).toContain("output truncated"); + }); + + test("spills oversized plain text under :full and names contextDir path", async () => { + const gate = skipGate(); + const store = fakeBlobStore(); + const contextDir = "/session/context"; + const huge = "z".repeat(MAX_RESULT_CHARS + 500); + const client = fakeClient(huge); + const [tool] = mcpClientToAgentTools(client, gate, { + getBlobWriter: () => store.writeBlob, + getContextDir: () => contextDir, + }); + if (tool?.kind !== "full") throw new Error("expected full tool"); + + const result = await tool.handler( + { id: "c-mcp-txt", name: "mcp__acme__fetch_secret", arguments: {} }, + new AbortController().signal, + ); + + const key = spillBlobKey("c-mcp-txt"); + const entry = store.blobs.get(key); + expect(entry?.contentType).toBe("text/plain"); + expect(new TextDecoder().decode(entry!.bytes)).toBe(huge); + expect(result.content).toContain(`tool-output:///${key}`); + expect(result.content).toContain( + toolOutputAbsolutePath(contextDir, key, "text/plain"), + ); + }); }); diff --git a/src/mcp/plugin.ts b/src/mcp/plugin.ts index 8c76c74b7..b5635b5ee 100644 --- a/src/mcp/plugin.ts +++ b/src/mcp/plugin.ts @@ -10,13 +10,18 @@ import { import type { MCPClient } from "./client.js"; import { mcpToolName } from "./tool-name.js"; +export interface McpSpillOptions { + getBlobWriter?: () => SpillBlobWriter | undefined; + getContextDir?: () => string | undefined; +} + // MCP results never reach the posix runner, so the secret-scrub and truncation // 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, - spill?: { callId: string; writeBlob: SpillBlobWriter }, + spill?: { callId: string; writeBlob: SpillBlobWriter; contextDir?: string }, ): Promise { return truncateToolResultContent(scrubSecretShapedToolResultContent(content), undefined, spill); } @@ -27,8 +32,10 @@ function sanitizeMcpResultContent( export function mcpClientToAgentTools( client: MCPClient, gate: PermissionGate, - getBlobWriter?: () => SpillBlobWriter | undefined, + spillOptions: McpSpillOptions = {}, ): AgentTool[] { + const { getBlobWriter, getContextDir } = spillOptions; + return client.tools.map((tool) => ({ kind: "full" as const, definition: { @@ -41,7 +48,15 @@ export function mcpClientToAgentTools( try { const content = await client.call(tool.name, call.arguments, signal); const writeBlob = getBlobWriter?.(); - const spill = writeBlob !== undefined ? { callId: call.id, writeBlob } : undefined; + const contextDir = getContextDir?.(); + const spill = + writeBlob !== undefined + ? { + callId: call.id, + writeBlob, + ...(contextDir !== undefined ? { contextDir } : {}), + } + : undefined; return { callId: call.id, content: await sanitizeMcpResultContent(content, spill) }; } catch (err) { return { diff --git a/src/plugins/result-truncation-plugin.test.ts b/src/plugins/result-truncation-plugin.test.ts index 24a2b2827..0fb5f7c1b 100644 --- a/src/plugins/result-truncation-plugin.test.ts +++ b/src/plugins/result-truncation-plugin.test.ts @@ -7,19 +7,23 @@ import { spillBlobKey, truncateToolResultContent, } from "./result-truncation-plugin.js"; +import { toolOutputAbsolutePath } from "./tool-result-materialize.js"; +import { CREDENTIAL_REDACTION } from "./tool-result-secret-scrub.js"; +import { toolResultSecretScrubPlugin } from "./tool-result-secret-scrub-plugin.js"; +import type { ToolPlugin } from "@intx/tools-posix"; /** In-memory stand-in for ContextStore's writeBlob/readBlob pair, for tests. */ function fakeBlobStore() { - const blobs = new Map(); + const blobs = new Map(); return { blobs, - writeBlob: async (key: string, bytes: Uint8Array) => { - blobs.set(key, bytes); + writeBlob: async (key: string, bytes: Uint8Array, contentType: string) => { + blobs.set(key, { bytes, contentType }); }, readBlob: async (key: string) => { - const bytes = blobs.get(key); - if (bytes === undefined) throw new Error(`Blob not found: ${key}`); - return bytes; + const entry = blobs.get(key); + if (entry === undefined) throw new Error(`Blob not found: ${key}`); + return entry.bytes; }, }; } @@ -30,6 +34,18 @@ describe("truncateToolResultContent", () => { expect(await truncateToolResultContent(content)).toBe(content); }); + test("under-gate minified JSON is left unchanged (no pretty, no spill)", async () => { + const store = fakeBlobStore(); + const minified = JSON.stringify({ a: 1, b: 2 }); + expect(minified.length).toBeLessThan(MAX_RESULT_CHARS); + const out = await truncateToolResultContent(minified, MAX_RESULT_CHARS, { + callId: "call-small", + writeBlob: store.writeBlob, + }); + expect(out).toBe(minified); + expect(store.blobs.size).toBe(0); + }); + 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 = await truncateToolResultContent(content); @@ -44,11 +60,12 @@ describe("truncateToolResultContent", () => { expect(truncated).not.toContain("session ends"); }); - test("the inlined portion stays bounded regardless of blob-store support", async () => { + test("the inlined portion stays within the cap (notice reserved inside the budget)", 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); + // Notice is reserved before slicing so the reactor 10k size-cap cannot strip it. + expect(truncated.length).toBeLessThanOrEqual(MAX_RESULT_CHARS); + expect(truncated).toContain("[output truncated"); }); describe("with a blob store", () => { @@ -60,11 +77,12 @@ describe("truncateToolResultContent", () => { writeBlob: store.writeBlob, }); - // Inline content is bounded and does not itself contain the discarded tail. + // Inline content is within the reactor cap and does not itself contain the discarded tail. expect(truncated).not.toContain("TAIL-MARKER"); - expect(truncated.length).toBeLessThan(MAX_RESULT_CHARS + 1000); + expect(truncated.length).toBeLessThanOrEqual(MAX_RESULT_CHARS); const uriMatch = /tool-output:\/\/\/\S+/.exec(truncated); + expect(uriMatch).not.toBeNull(); const uri = uriMatch?.[0].replace(/[.\]]+$/, "") ?? ""; expect(uri).toBe(`tool-output:///${spillBlobKey("call-42")}`); @@ -84,6 +102,57 @@ describe("truncateToolResultContent", () => { expect(truncated).not.toContain("session ends"); }); + test("minified JSON over the gate is pretty-spilled as application/json", async () => { + const store = fakeBlobStore(); + // Build a compact object whose minified form exceeds the 10k gate. + const obj: Record = {}; + for (let i = 0; i < 400; i++) { + obj[`key_${i}`] = `value_${i}_${"x".repeat(20)}`; + } + const minified = JSON.stringify(obj); + expect(minified.length).toBeGreaterThan(MAX_RESULT_CHARS); + const pretty = JSON.stringify(obj, null, 2); + + const truncated = await truncateToolResultContent(minified, MAX_RESULT_CHARS, { + callId: "call-json", + writeBlob: store.writeBlob, + contextDir: "/tmp/session/context", + }); + + const key = spillBlobKey("call-json"); + const entry = store.blobs.get(key); + expect(entry).toBeDefined(); + expect(entry?.contentType).toBe("application/json"); + expect(new TextDecoder().decode(entry!.bytes)).toBe(pretty); + + const uri = `tool-output:///${key}`; + const abs = toolOutputAbsolutePath("/tmp/session/context", key, "application/json"); + expect(truncated).toContain(uri); + expect(truncated).toContain(abs); + expect(truncated).toContain("application/json"); + expect(truncated).not.toContain(pretty.slice(-40)); + }); + + test("NDJSON over the gate is spilled unchanged as application/x-ndjson", async () => { + const store = fakeBlobStore(); + const lines = Array.from({ length: 200 }, (_, i) => + JSON.stringify({ i, pad: "y".repeat(80) }), + ); + const ndjson = `${lines.join("\n")}\n`; + expect(ndjson.length).toBeGreaterThan(MAX_RESULT_CHARS); + + const truncated = await truncateToolResultContent(ndjson, MAX_RESULT_CHARS, { + callId: "call-ndjson", + writeBlob: store.writeBlob, + }); + + const key = spillBlobKey("call-ndjson"); + const entry = store.blobs.get(key); + expect(entry?.contentType).toBe("application/x-ndjson"); + expect(new TextDecoder().decode(entry!.bytes)).toBe(ndjson); + expect(truncated).toContain("application/x-ndjson"); + }); + test("within-cap content never writes a blob", async () => { const store = fakeBlobStore(); await truncateToolResultContent("x".repeat(100), MAX_RESULT_CHARS, { @@ -94,41 +163,73 @@ describe("truncateToolResultContent", () => { }); 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)", + "leisure notice (URI + session path) survives the reactor 10k size-cap " + + "(CL-7055: reserved notice keeps inline result within-cap)", async () => { const store = fakeBlobStore(); - const original = "p".repeat(500_000); + const contextDir = "/tmp/session/context"; + const original = "p".repeat(50_000); const truncated = await truncateToolResultContent(original, MAX_RESULT_CHARS, { callId: "call-1", writeBlob: store.writeBlob, + contextDir, }); - // 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 key = spillBlobKey("call-1"); + const uri = `tool-output:///${key}`; + const abs = toolOutputAbsolutePath(contextDir, key, "text/plain"); + expect(truncated.length).toBeLessThanOrEqual(MAX_RESULT_CHARS); + expect(truncated).toContain(uri); + expect(truncated).toContain(abs); + + // Production pipeline: leisure middleware → reactor size-cap (always on, 10k). const reactorCap = createSizeCapTransform({ maxChars: 10_000, contextStore: { writeBlob: store.writeBlob }, }); const result: ToolResult = { callId: "call-1", content: truncated, isError: false }; - await reactorCap.apply( + const capped = 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 modelFacing = String(capped.output.content); + expect(modelFacing).toContain(uri); + expect(modelFacing).toContain(abs); + expect(modelFacing).toContain(":full"); + // Within-cap → reactor must not replace the leisure notice with its own. + expect(modelFacing).not.toContain("Tool output truncated"); + expect(store.blobs.has("call-1")).toBe(false); + + const blobReader = createBlobReader(store); + const recovered = new TextDecoder().decode(await blobReader.read(uri)); + expect(recovered).toBe(original); + expect(recovered.length).toBe(50_000); + }, + ); + + test( + "a same-keyed reactor spill cannot clobber the :full blob (CL-6908)", + async () => { + const store = fakeBlobStore(); + const original = "p".repeat(50_000); + await truncateToolResultContent(original, MAX_RESULT_CHARS, { + callId: "call-1", + writeBlob: store.writeBlob, + }); + + // Simulate a lossy same-id write the reactor would do on an over-cap result. + await store.writeBlob("call-1", new TextEncoder().encode("LOSSY"), "text/plain"); + 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); + expect(new TextDecoder().decode(store.blobs.get("call-1")!.bytes)).toBe("LOSSY"); }, ); + }); }); @@ -136,7 +237,10 @@ 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 }); + const plugin = resultTruncationPlugin({ + getBlobWriter: () => store.writeBlob, + getContextDir: () => "/session/context", + }); if (plugin.middleware === undefined) throw new Error("expected middleware"); const middleware = plugin.middleware(async (call) => ({ callId: call.id, @@ -150,6 +254,9 @@ describe("resultTruncationPlugin", () => { const uri = `tool-output:///${spillBlobKey("call-99")}`; expect(result.content).toContain(uri); + expect(result.content).toContain( + toolOutputAbsolutePath("/session/context", spillBlobKey("call-99"), "text/plain"), + ); const recovered = new TextDecoder().decode(await createBlobReader(store).read(uri)); expect(recovered).toBe(original); }); @@ -167,4 +274,82 @@ describe("resultTruncationPlugin", () => { ); expect(result.content).toContain("NOT retrievable"); }); + + test("pretty-serializes then spills an oversized Record content", async () => { + const store = fakeBlobStore(); + const record: Record = {}; + for (let i = 0; i < 400; i++) { + record[`k${i}`] = `v${i}_${"z".repeat(20)}`; + } + expect(JSON.stringify(record).length).toBeGreaterThan(MAX_RESULT_CHARS); + + 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: record, + })); + + const result = await middleware( + { id: "call-rec", name: "web_fetch", arguments: {} }, + new AbortController().signal, + ); + + expect(typeof result.content).toBe("string"); + const key = spillBlobKey("call-rec"); + const entry = store.blobs.get(key); + expect(entry?.contentType).toBe("application/json"); + expect(new TextDecoder().decode(entry!.bytes)).toBe(JSON.stringify(record, null, 2)); + }); + + test("under-gate Record content is left unchanged", async () => { + const store = fakeBlobStore(); + const record = { ok: true, n: 1 }; + 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: record, + })); + const result = await middleware( + { id: "call-rec-small", name: "web_fetch", arguments: {} }, + new AbortController().signal, + ); + expect(result.content).toEqual(record); + expect(store.blobs.size).toBe(0); + }); +}); + +describe("scrub-before-spill", () => { + test("secret scrub runs on the full content before truncation spills", async () => { + const store = fakeBlobStore(); + // Compose the same order as buildCorePosixToolPlugins: truncation outer, + // scrub inner — so scrub sees the full payload and the spill is redacted. + // Put the credential near the start so the kept (≤10k) slice also proves scrub + // ran; a secret past the cut would only show up in the spill. + const secret = `prefix sk-live-${"a".repeat(24)} ${"x".repeat(MAX_RESULT_CHARS)} suffix`; + const scrub: ToolPlugin = toolResultSecretScrubPlugin(); + const trunc: ToolPlugin = resultTruncationPlugin({ getBlobWriter: () => store.writeBlob }); + if (scrub.middleware === undefined || trunc.middleware === undefined) { + throw new Error("expected middleware"); + } + + const inner = scrub.middleware(async (call) => ({ + callId: call.id, + content: secret, + })); + const outer = trunc.middleware(inner); + + const result = await outer( + { id: "call-scrub", name: "run_shell", arguments: {} }, + new AbortController().signal, + ); + + expect(String(result.content)).toContain(CREDENTIAL_REDACTION); + expect(String(result.content)).not.toContain("sk-live-"); + + const spilled = new TextDecoder().decode(store.blobs.get(spillBlobKey("call-scrub"))!.bytes); + expect(spilled).toContain(CREDENTIAL_REDACTION); + expect(spilled).not.toContain("sk-live-"); + }); }); diff --git a/src/plugins/result-truncation-plugin.ts b/src/plugins/result-truncation-plugin.ts index b958748bd..2d48cf85e 100644 --- a/src/plugins/result-truncation-plugin.ts +++ b/src/plugins/result-truncation-plugin.ts @@ -1,10 +1,19 @@ import type { ToolPlugin } from "@intx/tools-posix"; +import { + materializeToolResultContent, + materializeToolResultRecord, + toolOutputAbsolutePath, + type MaterializedToolResult, +} from "./tool-result-materialize.js"; const TRUNCATABLE_TOOLS = new Set(["read_file", "grep", "run_shell", "search_files", "web_fetch"]); // Characters, not tokens — conversion ratio is roughly 4 chars/token. -// 80 000 chars ≈ 20 000 tokens. Keeps a single result from dominating context. -export const MAX_RESULT_CHARS = 80_000; +// Match the reactor's default size-cap (vendor/intx-inference assembly.ts) so +// leisure materialization owns the spill of the pretty/full bytes under the +// `:full` key before the reactor's own 10k transform can write a lossier copy +// under the bare call id. +export const MAX_RESULT_CHARS = 10_000; /** Writes a blob to the session's context store (ContextStore.writeBlob's shape). */ export type SpillBlobWriter = ( @@ -17,24 +26,120 @@ export type SpillBlobWriter = ( export interface TruncationSpillOptions { callId: string; writeBlob: SpillBlobWriter; + /** + * Absolute path to the session context dir (`…/context`). When set, the + * truncation notice also names the on-disk tool-output path beside the + * `tool-output:///` URI so an operator can open the spill directly. + */ + contextDir?: string; } /** * 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. + * after this middleware returns it. Leisure truncation keeps the inline + * result (kept + notice) ≤ maxChars so that transform normally passes through + * within-cap, but any other path that still exceeds the cap would spill under + * the bare call id and clobber a same-keyed full write. The ":full" suffix + * keeps our blob a distinct entry the reactor never touches. */ export function spillBlobKey(callId: string): string { return `${callId}:full`; } +function truncationNotice(args: { + maxChars: number; + remaining: number; + fullLength: number; + contentType: string; + uri?: string; + absolutePath?: string; +}): string { + const { maxChars, remaining, fullLength, contentType, uri, absolutePath } = args; + if (uri === undefined) { + return ( + `\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 pathBit = + absolutePath !== undefined ? ` (session path: ${absolutePath})` : ""; + return ( + `\n[output truncated at ${maxChars.toLocaleString()} chars — ` + + `${remaining.toLocaleString()} more chars omitted here. The full result ` + + `(${fullLength.toLocaleString()} chars, ${contentType}) is saved at ${uri}` + + `${pathBit} — use read_file with that URI (offset/limit supported) to see the rest.]` + ); +} + +/** + * Truncates so the FINAL result (kept + notice) never exceeds maxChars — the + * notice is reserved before slicing, not appended after. Without this, leisure + * output is maxChars+noticeLen and the reactor's always-on 10k size-cap + * (createSizeCapTransform) replaces the whole string, stripping the leisure + * URI+path the model needs. Notice length depends on digit counts of + * remaining/fullLength (and optional absolutePath), so shrink kept until the + * assembled result fits. + */ +function truncateWithReservedNotice( + text: string, + maxChars: number, + buildNotice: (keptLen: number) => string, +): string { + let keptLen = maxChars; + for (let i = 0; i < 8; i++) { + const notice = buildNotice(keptLen); + const total = keptLen + notice.length; + if (total <= maxChars) return text.slice(0, keptLen) + notice; + keptLen -= total - maxChars; + if (keptLen < 0) keptLen = 0; + } + + const notice = buildNotice(keptLen); + return (text.slice(0, keptLen) + notice).slice(0, maxChars); +} + +async function spillAndTruncate( + materialized: MaterializedToolResult, + maxChars: number, + spill?: TruncationSpillOptions, +): Promise { + const { text, contentType } = materialized; + if (text.length <= maxChars) return text; + + if (spill === undefined) { + return truncateWithReservedNotice(text, maxChars, (keptLen) => + truncationNotice({ + maxChars, + remaining: text.length - keptLen, + fullLength: text.length, + contentType, + }), + ); + } + + const key = spillBlobKey(spill.callId); + const uri = `tool-output:///${key}`; + await spill.writeBlob(key, new TextEncoder().encode(text), contentType); + const absolutePath = + spill.contextDir !== undefined + ? toolOutputAbsolutePath(spill.contextDir, key, contentType) + : undefined; + return truncateWithReservedNotice(text, maxChars, (keptLen) => + truncationNotice({ + maxChars, + remaining: text.length - keptLen, + fullLength: text.length, + contentType, + uri, + ...(absolutePath !== undefined ? { absolutePath } : {}), + }), + ); +} + // 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 @@ -43,14 +148,20 @@ export function spillBlobKey(callId: string): string { // posix-tool-plugins.ts, so plugins like ripgrepPlugin that answer without // calling next() no longer need to apply the cap themselves. // -// When `spill` is supplied, the FULL (pre-cut) content is written to the +// When content exceeds `maxChars`, it is leisure-materialized first (pretty +// JSON / preserved NDJSON / raw text) and the FORMATTED bytes are what we +// spill and what we truncate inline. Under-gate content is returned unchanged +// (no pretty, no blob). +// +// When `spill` is supplied, the full formatted 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. +// — and the notice names that real URI (plus the absolute session path when +// `contextDir` is plumbed). 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 @@ -61,30 +172,27 @@ export async function truncateToolResultContent( spill?: TruncationSpillOptions, ): Promise { if (content.length <= maxChars) return content; + return spillAndTruncate(materializeToolResultContent(content), maxChars, spill); +} - const remaining = content.length - maxChars; - 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.]` - ); +/** + * Same gate/spill path as {@link truncateToolResultContent} for structured + * `ToolResult.content` Records: pretty-serialize, then spill/truncate the + * formatted JSON when over the gate. + */ +export async function truncateToolResultRecord( + content: Record, + maxChars: number = MAX_RESULT_CHARS, + spill?: TruncationSpillOptions, +): Promise { + const compactLength = JSON.stringify(content).length; + if (compactLength <= maxChars) { + // Under-gate Records stay Records at the plugin layer; this helper is only + // reached when the plugin has already decided to serialize. Return pretty + // so callers that asked for a string get a stable shape. + return materializeToolResultRecord(content).text; } - - const key = spillBlobKey(spill.callId); - const uri = `tool-output:///${key}`; - await spill.writeBlob(key, new TextEncoder().encode(content), "text/plain"); - return ( - kept + - `\n[output truncated at ${maxChars.toLocaleString()} chars — ` + - `${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.]` - ); + return spillAndTruncate(materializeToolResultRecord(content), maxChars, spill); } export interface ResultTruncationPluginOptions { @@ -94,22 +202,45 @@ export interface ResultTruncationPluginOptions { // session store to spill into (tests, ad-hoc toolsets) — truncation still // runs, just without a retrievable remainder. getBlobWriter?: () => SpillBlobWriter | undefined; + // Live getter for the absolute session context dir, re-read like + // getBlobWriter so rotation picks up the new path for the notice. + getContextDir?: () => string | undefined; } export function resultTruncationPlugin(options: ResultTruncationPluginOptions = {}): ToolPlugin { - const { getBlobWriter } = options; + const { getBlobWriter, getContextDir } = options; return { middleware: (next) => async (call, signal) => { const result = await next(call, signal); if (!TRUNCATABLE_TOOLS.has(call.name) || result.isError) return result; - const { content } = result; - if (typeof content !== "string") return result; 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 }; + const contextDir = getContextDir?.(); + const spill = + writeBlob !== undefined + ? { + callId: call.id, + writeBlob, + ...(contextDir !== undefined ? { contextDir } : {}), + } + : undefined; + + const { content } = result; + if (typeof content === "string") { + const truncated = await truncateToolResultContent(content, MAX_RESULT_CHARS, spill); + if (truncated === content) return result; + return { ...result, content: truncated }; + } + + if (content !== null && typeof content === "object") { + const record = content as Record; + const compact = JSON.stringify(record); + if (compact.length <= MAX_RESULT_CHARS) return result; + const truncated = await truncateToolResultRecord(record, MAX_RESULT_CHARS, spill); + return { ...result, content: truncated }; + } + + return result; }, }; } diff --git a/src/plugins/tool-result-materialize.test.ts b/src/plugins/tool-result-materialize.test.ts new file mode 100644 index 000000000..56f841182 --- /dev/null +++ b/src/plugins/tool-result-materialize.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, test } from "bun:test"; +import { + materializeToolResultContent, + materializeToolResultRecord, + toolOutputAbsolutePath, +} from "./tool-result-materialize.js"; + +describe("materializeToolResultContent", () => { + test("pretty-prints a minified JSON object as application/json", () => { + const minified = JSON.stringify({ a: 1, b: [2, 3], nested: { ok: true } }); + const out = materializeToolResultContent(minified); + expect(out.contentType).toBe("application/json"); + expect(out.text).toBe(JSON.stringify(JSON.parse(minified), null, 2)); + expect(out.text).toContain("\n"); + }); + + test("pretty-prints a minified JSON array", () => { + const minified = JSON.stringify([{ id: 1 }, { id: 2 }]); + const out = materializeToolResultContent(minified); + expect(out.contentType).toBe("application/json"); + expect(out.text).toBe(JSON.stringify(JSON.parse(minified), null, 2)); + }); + + test("preserves NDJSON (two or more JSON lines) without reformatting", () => { + const ndjson = `${JSON.stringify({ n: 1 })}\n${JSON.stringify({ n: 2 })}\n`; + const out = materializeToolResultContent(ndjson); + expect(out.contentType).toBe("application/x-ndjson"); + expect(out.text).toBe(ndjson); + }); + + test("classifies CRLF-delimited NDJSON without rewriting the bytes", () => { + const ndjson = `${JSON.stringify({ n: 1 })}\r\n${JSON.stringify({ n: 2 })}\r\n`; + const out = materializeToolResultContent(ndjson); + expect(out.contentType).toBe("application/x-ndjson"); + expect(out.text).toBe(ndjson); + }); + + test("leaves non-JSON text as text/plain", () => { + const raw = "hello\nworld\nnot json"; + expect(materializeToolResultContent(raw)).toEqual({ + text: raw, + contentType: "text/plain", + }); + }); + + test("skips pretty-print for huge payloads and returns text/plain", () => { + // Over the ~8MB ceiling: starts like JSON but must not be parsed/pretty-printed. + const huge = `{"k":"${"x".repeat(9 * 1024 * 1024)}"}`; + const out = materializeToolResultContent(huge); + expect(out.contentType).toBe("text/plain"); + expect(out.text).toBe(huge); + }); +}); + +describe("materializeToolResultRecord", () => { + test("pretty-serializes a Record as application/json", () => { + const out = materializeToolResultRecord({ hello: "world", n: 1 }); + expect(out.contentType).toBe("application/json"); + expect(out.text).toBe(JSON.stringify({ hello: "world", n: 1 }, null, 2)); + }); +}); + +describe("toolOutputAbsolutePath", () => { + test("mirrors store naming including :full → _full and .json extension", () => { + const abs = toolOutputAbsolutePath( + "/tmp/session/context", + "call-42:full", + "application/json", + ); + expect(abs).toBe("/tmp/session/context/tool-output/call-42_full.json"); + }); + + test("uses .txt for text/plain and no extension for unknown types", () => { + expect(toolOutputAbsolutePath("/c", "k", "text/plain")).toBe("/c/tool-output/k.txt"); + expect(toolOutputAbsolutePath("/c", "k", "application/x-ndjson")).toBe("/c/tool-output/k"); + }); +}); diff --git a/src/plugins/tool-result-materialize.ts b/src/plugins/tool-result-materialize.ts new file mode 100644 index 000000000..c49f6d498 --- /dev/null +++ b/src/plugins/tool-result-materialize.ts @@ -0,0 +1,110 @@ +import path from "node:path"; + +/** MIME written with spilled tool-output blobs. */ +export type ToolResultContentType = + | "application/json" + | "application/x-ndjson" + | "text/plain"; + +export interface MaterializedToolResult { + text: string; + contentType: ToolResultContentType; +} + +// Pretty-printing a multi-megabyte blob is not worth the CPU/memory; leave it raw. +const PRETTY_SIZE_CEILING_CHARS = 8 * 1024 * 1024; + +// Mirror vendor/intx-storage-isogit + optimized-context-store blob filenames so the +// absolute path named in the truncation notice matches what writeBlob actually wrote. +const TOOL_OUTPUT_DIR = "tool-output"; +const UNSAFE_FILENAME_CHARS = /[^a-zA-Z0-9_-]/g; +const BLOB_EXTENSIONS: Readonly> = { + "text/plain": ".txt", + "application/json": ".json", +}; + +function blobExtensionFor(contentType: string): string { + return BLOB_EXTENSIONS[contentType] ?? ""; +} + +function sanitizeCallId(callId: string): string { + if (callId.includes("..") || callId.includes("/")) { + throw new Error(`callId contains unsafe characters: ${JSON.stringify(callId)}`); + } + return callId.replace(UNSAFE_FILENAME_CHARS, "_"); +} + +/** + * Absolute filesystem path the session store will use for a spilled blob key. + * Must stay in lockstep with ContextStore.writeBlob's on-disk naming. + */ +export function toolOutputAbsolutePath( + contextDir: string, + key: string, + contentType: string, +): string { + const filename = `${sanitizeCallId(key)}${blobExtensionFor(contentType)}`; + return path.join(contextDir, TOOL_OUTPUT_DIR, filename); +} + +function looksLikeJsonDocument(text: string): boolean { + const trimmed = text.trimStart(); + return trimmed.startsWith("{") || trimmed.startsWith("["); +} + +function isNdjson(text: string): boolean { + // Require at least two non-empty lines that each parse as JSON. A single JSON + // object on one line is handled by the document path instead. Strip a trailing + // CR so CRLF-delimited NDJSON still classifies (JSON.parse rejects `"…}\r"`). + const lines = text.split("\n").filter((line) => line.length > 0); + if (lines.length < 2) return false; + for (const line of lines) { + const candidate = line.endsWith("\r") ? line.slice(0, -1) : line; + if (candidate.length === 0) return false; + try { + JSON.parse(candidate); + } catch { + return false; + } + } + return true; +} + +/** + * Choose a durable representation for a tool result before it is spilled: + * minified JSON → pretty application/json; multi-line NDJSON → keep as-is; + * everything else → text/plain. Skips pretty-print above ~8MB. + */ +export function materializeToolResultContent(content: string): MaterializedToolResult { + if (content.length > PRETTY_SIZE_CEILING_CHARS) { + return { text: content, contentType: "text/plain" }; + } + + if (looksLikeJsonDocument(content)) { + try { + const parsed: unknown = JSON.parse(content); + return { + text: JSON.stringify(parsed, null, 2), + contentType: "application/json", + }; + } catch { + // Not valid JSON — fall through to NDJSON / raw. + } + } + + if (isNdjson(content)) { + return { text: content, contentType: "application/x-ndjson" }; + } + + return { text: content, contentType: "text/plain" }; +} + +/** Pretty-serialize a structured ToolResult Record for spill/truncation. */ +export function materializeToolResultRecord( + content: Record, +): MaterializedToolResult { + return { + text: JSON.stringify(content, null, 2), + contentType: "application/json", + }; +} diff --git a/src/tui/runner.ts b/src/tui/runner.ts index d19b19ab3..5360af726 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -1271,6 +1271,8 @@ export async function runTUI(initialConfig: Config): Promise { toolWatchdog: liveToolWatchdog, getBlobReader: () => currentAgent.blobReader, getBlobWriter: () => currentStorage?.writeBlob, + getContextDir: () => workdir, + isWorkflowActive: () => workflowControllerHolder.instance?.isActive() === true, ...(extraToolPlugins.length > 0 ? { extraToolPlugins } : {}), onOperatorGate: (question, options) => From c923c11a04d13bb46cfb103d6fbcae18b121db79 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 24 Aug 2026 22:06:54 -0700 Subject: [PATCH 2/2] Harden spilled tool result materialization --- src/mcp/plugin.test.ts | 41 ++++++++-- src/plugins/result-truncation-plugin.test.ts | 57 +++++++++----- src/plugins/result-truncation-plugin.ts | 7 +- src/plugins/tool-result-materialize.test.ts | 6 +- src/plugins/tool-result-materialize.ts | 5 +- src/subagent/run-codex-proxy.test.ts | 83 ++++++++++++++++++++ src/subagent/run.ts | 39 ++++++--- 7 files changed, 187 insertions(+), 51 deletions(-) create mode 100644 src/subagent/run-codex-proxy.test.ts diff --git a/src/mcp/plugin.test.ts b/src/mcp/plugin.test.ts index b5ede2021..e175f7208 100644 --- a/src/mcp/plugin.test.ts +++ b/src/mcp/plugin.test.ts @@ -1,10 +1,7 @@ import { describe, test, expect } from "bun:test"; import { mcpClientToAgentTools } from "./plugin.js"; import { createPermissionGate } from "../permission/gate.js"; -import { - MAX_RESULT_CHARS, - spillBlobKey, -} from "../plugins/result-truncation-plugin.js"; +import { MAX_RESULT_CHARS, spillBlobKey } from "../plugins/result-truncation-plugin.js"; import { toolOutputAbsolutePath } from "../plugins/tool-result-materialize.js"; import { CREDENTIAL_REDACTION } from "../plugins/tool-result-secret-scrub.js"; import type { MCPClient } from "./client.js"; @@ -116,6 +113,38 @@ describe("mcpClientToAgentTools", () => { expect(result.content).toContain("output truncated"); }); + test("scrubs escaped secrets after oversized JSON pretty materialization", async () => { + const gate = skipGate(); + const store = fakeBlobStore(); + const escapedSecret = `sk-\\u006cive-${"b".repeat(24)}`; + const minified = `{"secret":"${escapedSecret}","pad":"${"x".repeat(MAX_RESULT_CHARS)}"}`; + expect(minified).not.toContain("sk-live-"); + + const client = fakeClient(minified); + const [tool] = mcpClientToAgentTools(client, gate, { + getBlobWriter: () => store.writeBlob, + }); + if (tool?.kind !== "full") throw new Error("expected full tool"); + + const result = await tool.handler( + { + id: "c-mcp-json-secret", + name: "mcp__acme__fetch_secret", + arguments: {}, + }, + new AbortController().signal, + ); + + const spilled = new TextDecoder().decode( + store.blobs.get(spillBlobKey("c-mcp-json-secret"))!.bytes, + ); + expect(result.content).toContain(CREDENTIAL_REDACTION); + expect(result.content).not.toContain("sk-live-"); + expect(spilled).toContain(CREDENTIAL_REDACTION); + expect(spilled).not.toContain("sk-live-"); + expect(spilled).not.toContain(escapedSecret); + }); + test("spills oversized plain text under :full and names contextDir path", async () => { const gate = skipGate(); const store = fakeBlobStore(); @@ -138,8 +167,6 @@ describe("mcpClientToAgentTools", () => { expect(entry?.contentType).toBe("text/plain"); expect(new TextDecoder().decode(entry!.bytes)).toBe(huge); expect(result.content).toContain(`tool-output:///${key}`); - expect(result.content).toContain( - toolOutputAbsolutePath(contextDir, key, "text/plain"), - ); + expect(result.content).toContain(toolOutputAbsolutePath(contextDir, key, "text/plain")); }); }); diff --git a/src/plugins/result-truncation-plugin.test.ts b/src/plugins/result-truncation-plugin.test.ts index 0fb5f7c1b..5f397496e 100644 --- a/src/plugins/result-truncation-plugin.test.ts +++ b/src/plugins/result-truncation-plugin.test.ts @@ -133,6 +133,27 @@ describe("truncateToolResultContent", () => { expect(truncated).not.toContain(pretty.slice(-40)); }); + test("oversized JSON with an escaped secret is scrubbed after pretty materialization", async () => { + const store = fakeBlobStore(); + const escapedSecret = `sk-\\u006cive-${"a".repeat(24)}`; + const minified = `{"secret":"${escapedSecret}","pad":"${"x".repeat(MAX_RESULT_CHARS)}"}`; + expect(minified).not.toContain("sk-live-"); + + const truncated = await truncateToolResultContent(minified, MAX_RESULT_CHARS, { + callId: "call-json-secret", + writeBlob: store.writeBlob, + }); + + const spilled = new TextDecoder().decode( + store.blobs.get(spillBlobKey("call-json-secret"))!.bytes, + ); + expect(truncated).toContain(CREDENTIAL_REDACTION); + expect(truncated).not.toContain("sk-live-"); + expect(spilled).toContain(CREDENTIAL_REDACTION); + expect(spilled).not.toContain("sk-live-"); + expect(spilled).not.toContain(escapedSecret); + }); + test("NDJSON over the gate is spilled unchanged as application/x-ndjson", async () => { const store = fakeBlobStore(); const lines = Array.from({ length: 200 }, (_, i) => @@ -208,28 +229,24 @@ describe("truncateToolResultContent", () => { }, ); - test( - "a same-keyed reactor spill cannot clobber the :full blob (CL-6908)", - async () => { - const store = fakeBlobStore(); - const original = "p".repeat(50_000); - await truncateToolResultContent(original, MAX_RESULT_CHARS, { - callId: "call-1", - writeBlob: store.writeBlob, - }); - - // Simulate a lossy same-id write the reactor would do on an over-cap result. - await store.writeBlob("call-1", new TextEncoder().encode("LOSSY"), "text/plain"); + test("a same-keyed reactor spill cannot clobber the :full blob (CL-6908)", async () => { + const store = fakeBlobStore(); + const original = "p".repeat(50_000); + await truncateToolResultContent(original, MAX_RESULT_CHARS, { + callId: "call-1", + writeBlob: store.writeBlob, + }); - const blobReader = createBlobReader(store); - const recovered = new TextDecoder().decode( - await blobReader.read(`tool-output:///${spillBlobKey("call-1")}`), - ); - expect(recovered).toBe(original); - expect(new TextDecoder().decode(store.blobs.get("call-1")!.bytes)).toBe("LOSSY"); - }, - ); + // Simulate a lossy same-id write the reactor would do on an over-cap result. + await store.writeBlob("call-1", new TextEncoder().encode("LOSSY"), "text/plain"); + const blobReader = createBlobReader(store); + const recovered = new TextDecoder().decode( + await blobReader.read(`tool-output:///${spillBlobKey("call-1")}`), + ); + expect(recovered).toBe(original); + expect(new TextDecoder().decode(store.blobs.get("call-1")!.bytes)).toBe("LOSSY"); + }); }); }); diff --git a/src/plugins/result-truncation-plugin.ts b/src/plugins/result-truncation-plugin.ts index 2d48cf85e..561abe011 100644 --- a/src/plugins/result-truncation-plugin.ts +++ b/src/plugins/result-truncation-plugin.ts @@ -5,6 +5,7 @@ import { toolOutputAbsolutePath, type MaterializedToolResult, } from "./tool-result-materialize.js"; +import { scrubSecretShapedToolResultContent } from "./tool-result-secret-scrub.js"; const TRUNCATABLE_TOOLS = new Set(["read_file", "grep", "run_shell", "search_files", "web_fetch"]); @@ -65,8 +66,7 @@ function truncationNotice(args: { `Use offset/limit or a narrower query.]` ); } - const pathBit = - absolutePath !== undefined ? ` (session path: ${absolutePath})` : ""; + const pathBit = absolutePath !== undefined ? ` (session path: ${absolutePath})` : ""; return ( `\n[output truncated at ${maxChars.toLocaleString()} chars — ` + `${remaining.toLocaleString()} more chars omitted here. The full result ` + @@ -107,7 +107,8 @@ async function spillAndTruncate( maxChars: number, spill?: TruncationSpillOptions, ): Promise { - const { text, contentType } = materialized; + const { contentType } = materialized; + const text = scrubSecretShapedToolResultContent(materialized.text); if (text.length <= maxChars) return text; if (spill === undefined) { diff --git a/src/plugins/tool-result-materialize.test.ts b/src/plugins/tool-result-materialize.test.ts index 56f841182..8b53f4a7c 100644 --- a/src/plugins/tool-result-materialize.test.ts +++ b/src/plugins/tool-result-materialize.test.ts @@ -62,11 +62,7 @@ describe("materializeToolResultRecord", () => { describe("toolOutputAbsolutePath", () => { test("mirrors store naming including :full → _full and .json extension", () => { - const abs = toolOutputAbsolutePath( - "/tmp/session/context", - "call-42:full", - "application/json", - ); + const abs = toolOutputAbsolutePath("/tmp/session/context", "call-42:full", "application/json"); expect(abs).toBe("/tmp/session/context/tool-output/call-42_full.json"); }); diff --git a/src/plugins/tool-result-materialize.ts b/src/plugins/tool-result-materialize.ts index c49f6d498..f27eb69e6 100644 --- a/src/plugins/tool-result-materialize.ts +++ b/src/plugins/tool-result-materialize.ts @@ -1,10 +1,7 @@ import path from "node:path"; /** MIME written with spilled tool-output blobs. */ -export type ToolResultContentType = - | "application/json" - | "application/x-ndjson" - | "text/plain"; +export type ToolResultContentType = "application/json" | "application/x-ndjson" | "text/plain"; export interface MaterializedToolResult { text: string; diff --git a/src/subagent/run-codex-proxy.test.ts b/src/subagent/run-codex-proxy.test.ts new file mode 100644 index 000000000..3a7ce4892 --- /dev/null +++ b/src/subagent/run-codex-proxy.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, test } from "bun:test"; +import { createToolRunner } from "@intx/agent"; +import { createBlobReader, type ToolCall, type ToolResult } from "@intx/types/runtime"; + +import { createCodexToolProxies, type CodexRunManageTasks } from "../agent/codex-tool-proxies.js"; +import { + MAX_RESULT_CHARS, + truncateToolResultContent, +} from "../plugins/result-truncation-plugin.js"; +import { createCodexProxyRunTool } from "./run.js"; + +function fakeBlobStore() { + const blobs = new Map(); + return { + blobs, + writeBlob: async (key: string, bytes: Uint8Array, contentType: string) => { + blobs.set(key, { bytes, contentType }); + }, + readBlob: async (key: string) => { + const entry = blobs.get(key); + if (entry === undefined) throw new Error(`Blob not found: ${key}`); + return entry.bytes; + }, + }; +} + +const unusedManageTasks: CodexRunManageTasks = async () => ({ content: "unused" }); + +function extractToolOutputURI(content: unknown): string { + const match = /tool-output:\/\/\/\S+/.exec(String(content)); + if (match === null) throw new Error(`missing tool-output URI in ${String(content)}`); + return match[0].replace(/[.\]]+$/, ""); +} + +describe("createCodexProxyRunTool", () => { + test("oversized proxied shell calls get distinct recoverable spill URIs", async () => { + const store = fakeBlobStore(); + const outputs: [string, string] = [ + `${"a".repeat(MAX_RESULT_CHARS)}FIRST-TAIL`, + `${"b".repeat(MAX_RESULT_CHARS)}SECOND-TAIL`, + ]; + const seenCallIds: string[] = []; + const posixTools = { + run: async (call: ToolCall): Promise => { + seenCallIds.push(call.id); + const index = seenCallIds.length - 1; + return { + callId: call.id, + content: await truncateToolResultContent(outputs[index] ?? "", MAX_RESULT_CHARS, { + callId: call.id, + writeBlob: store.writeBlob, + }), + }; + }, + }; + + const tools = createCodexToolProxies({ + isCodex: true, + runTool: createCodexProxyRunTool(posixTools), + readRawFile: async () => ({ content: "unused" }), + runManageTasks: unusedManageTasks, + }); + const runner = createToolRunner(tools); + + const first = await runner.run( + { id: "outer-1", name: "shell", arguments: { command: "first" } }, + new AbortController().signal, + ); + const second = await runner.run( + { id: "outer-2", name: "shell", arguments: { command: "second" } }, + new AbortController().signal, + ); + + const firstURI = extractToolOutputURI(first.content); + const secondURI = extractToolOutputURI(second.content); + expect(firstURI).not.toBe(secondURI); + expect(seenCallIds).toEqual(["codex-proxy-1", "codex-proxy-2"]); + + const reader = createBlobReader(store); + expect(new TextDecoder().decode(await reader.read(firstURI))).toBe(outputs[0]); + expect(new TextDecoder().decode(await reader.read(secondURI))).toBe(outputs[1]); + }); +}); diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 0124903af..7e614a0df 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -23,7 +23,13 @@ import { type } from "arktype"; import { createPosixTools } from "@intx/tools-posix"; import { createDynamicToolRunner } from "../tui/dynamic-tool-runner.js"; import type { ReactorEmittedEvent } from "@intx/inference"; -import type { BlobReader, InboundMessage, ToolDefinition } from "@intx/types/runtime"; +import type { + BlobReader, + InboundMessage, + ToolCall, + ToolDefinition, + ToolResult, +} from "@intx/types/runtime"; import { seedPricingMetadataFromCache } from "../cost/pricing-metadata.js"; import { defaultPricingCachePath } from "../cost/pricing-fetcher.js"; @@ -321,6 +327,25 @@ const submitResultDefinition: ToolDefinition = { }, }; +interface CodexProxyToolRunner { + run(call: ToolCall, signal: AbortSignal): Promise; +} + +export function createCodexProxyRunTool(posixTools: CodexProxyToolRunner): CodexRunTool { + let invocation = 0; + return async (name, args) => { + invocation += 1; + const result = await posixTools.run( + { id: `codex-proxy-${invocation}`, name, arguments: args }, + new AbortController().signal, + ); + return { + content: typeof result.content === "string" ? result.content : JSON.stringify(result.content), + ...(result.isError === true ? { isError: true } : {}), + }; + }; +} + // Spin up an isolated, autonomous agent loop, hand it one task, and return // its final report. `params.cwd` is either the dispatcher's own cwd (shared // mode) or a worktree snapshotted from the dispatcher's last commit @@ -411,17 +436,7 @@ export async function runSubAgent(params: RunSubAgentParams): Promise { - const result = await posixTools.run( - { id: "codex-proxy", name, arguments: args }, - new AbortController().signal, - ); - return { - content: - typeof result.content === "string" ? result.content : JSON.stringify(result.content), - ...(result.isError === true ? { isError: true } : {}), - }; - }; + const runTool = createCodexProxyRunTool(posixTools); // manage_tasks is not a posix tool — task state here is owned by the // director observing manage_tasks tool_calls in the model's own output, // not by this handler's return value (see applyManageTasksToolCall in