Skip to content

Commit 569850a

Browse files
Merge pull request #585 from corbitsdev/cl-6965-oversized-tool-results-should-spill-to-a-file-the-agent-can
Spill oversized tool results into the committed blob store
2 parents 75e2774 + 012be2b commit 569850a

7 files changed

Lines changed: 285 additions & 62 deletions

File tree

src/agent/posix-tool-plugins.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ import { editFileLineRangePlugin } from "../plugins/edit-file-line-range-plugin.
1111
import { ripgrepPlugin } from "../plugins/ripgrep-plugin.js";
1212
import { toolOutputUriPlugin } from "../plugins/tool-output-uri-plugin.js";
1313
import { lspHintPlugin } from "../plugins/lsp-hint-plugin.js";
14-
import { resultTruncationPlugin } from "../plugins/result-truncation-plugin.js";
14+
import {
15+
resultTruncationPlugin,
16+
type SpillBlobWriter,
17+
} from "../plugins/result-truncation-plugin.js";
1518
import { toolResultSecretScrubPlugin } from "../plugins/tool-result-secret-scrub-plugin.js";
1619
import { shellGuardPlugin, type ShellTimeoutConfig } from "../plugins/shell-guard-plugin.js";
1720
import {
@@ -27,6 +30,9 @@ export interface CorePosixToolPluginsArgs {
2730
shellTimeout?: ShellTimeoutConfig;
2831
extraToolPlugins?: ToolPlugin[];
2932
readFileGuard?: ReadFileGuardPluginOptions;
33+
// Session blob-store writer oversized tool results spill their full content
34+
// into. See result-truncation-plugin.ts.
35+
getBlobWriter?: () => SpillBlobWriter | undefined;
3036
// Per-project settings.env, merged into the run_shell spawn environment.
3137
shellEnv?: Record<string, string>;
3238
}
@@ -60,6 +66,7 @@ export function buildCorePosixToolPlugins(args: CorePosixToolPluginsArgs): ToolP
6066
shellTimeout,
6167
extraToolPlugins = [],
6268
readFileGuard = {},
69+
getBlobWriter,
6370
shellEnv,
6471
} = args;
6572
// Pre-gate sandboxes honor yolo mode so outside-workspace path tools and shell
@@ -69,7 +76,7 @@ export function buildCorePosixToolPlugins(args: CorePosixToolPluginsArgs): ToolP
6976
// regardless.
7077
const allowOutside = (): boolean => permissionGate.getSkipPermissions();
7178
return [
72-
resultTruncationPlugin(),
79+
resultTruncationPlugin(getBlobWriter !== undefined ? { getBlobWriter } : {}),
7380
toolResultSecretScrubPlugin(),
7481
pathEscapePlugin(cwd, createWorktreeRootsProvider(cwd), { allowOutside }),
7582
deleteFilePlugin(cwd, { allowOutside }),

src/agent/tools.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import type { PermissionGate } from "../permission/gate.js";
2121
import { buildCorePosixToolPlugins } from "./posix-tool-plugins.js";
2222
import { createLazyBlobReader } from "./lazy-blob-reader.js";
2323
import type { BlobReader } from "@intx/types/runtime";
24+
import type { SpillBlobWriter } from "../plugins/result-truncation-plugin.js";
2425
import { connectMCPServer, type MCPClient } from "../mcp/client.js";
2526
import { mcpClientToAgentTools } from "../mcp/plugin.js";
2627
import { createDynamicToolRunner, type DynamicToolRunner } from "../tui/dynamic-tool-runner.js";
@@ -102,6 +103,15 @@ export interface AgentToolsetArgs {
102103
// Session blob store for tool-output:// reads; resolved when tools run so agent
103104
// rebuilds do not require recreating the posix toolset.
104105
getBlobReader?: () => BlobReader | undefined;
106+
// Session blob-store writer oversized tool results spill their full,
107+
// untruncated content into (see result-truncation-plugin.ts) — the same
108+
// context store getBlobReader reads from, keyed distinctly so the reactor's
109+
// own downstream size-cap transform never overwrites the spill. Resolved
110+
// lazily like getBlobReader so a mid-process session rotation spills into
111+
// the new session's store. Omitted only where there is no session store to
112+
// write into (tests). Persists with the rest of the session's committed
113+
// history — no separate cleanup.
114+
getBlobWriter?: () => SpillBlobWriter | undefined;
105115
// Per-project settings.env, merged into the run_shell tool's spawn environment.
106116
shellEnv?: Record<string, string>;
107117
// Whether a workflow is currently running. advance_workflow rides the wire
@@ -191,6 +201,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
191201
shellTimeout,
192202
toolWatchdog,
193203
getBlobReader,
204+
getBlobWriter,
194205
sessionMode = "orchestrator",
195206
shellEnv,
196207
toolAvailability = { languageServerAvailable: true },
@@ -213,6 +224,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
213224
...(sessionBlobReader !== undefined
214225
? { readFileGuard: { blobReader: sessionBlobReader } }
215226
: {}),
227+
...(getBlobWriter !== undefined ? { getBlobWriter } : {}),
216228
...(shellEnv !== undefined ? { shellEnv } : {}),
217229
}),
218230
});
@@ -441,7 +453,7 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
441453
}
442454
connectedClients.push(result.client);
443455
permissionGate.registerMcpClient(result.client);
444-
const mcpTools = mcpClientToAgentTools(result.client, permissionGate);
456+
const mcpTools = mcpClientToAgentTools(result.client, permissionGate, getBlobWriter);
445457
inheritedMcpTools.push(...mcpTools);
446458
dynamicRunner.addTools(mcpTools);
447459
callbacks.onStatus({

src/exec/runner.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,12 @@ import { detectLanguageServerAvailable } from "../agent/lsp-availability.js";
4646
import { normalizeToolDefinitionsForProvider } from "../agent/tool-schema-normalize.js";
4747
import { resolveSessionMode, type SessionMode } from "../config/session-mode.js";
4848
import { createSubAgentSessionStore, type SubAgentProvider } from "../subagent/index.js";
49-
import type { InferenceSource, ToolDefinition, InboundMessage } from "@intx/types/runtime";
49+
import type {
50+
ContextStore,
51+
InferenceSource,
52+
ToolDefinition,
53+
InboundMessage,
54+
} from "@intx/types/runtime";
5055
import { OPERATOR_ORIGINATED_FLAG } from "../agent/message-provenance.js";
5156
import { createChatDirector } from "../agent/director.js";
5257
import { loadAgentProfiles } from "../agent/profiles.js";
@@ -384,6 +389,7 @@ export async function runExec(config: Config): Promise<ExecResult> {
384389
};
385390

386391
let currentAgent: Agent | null = null;
392+
let currentStorage: ContextStore | null = null;
387393

388394
const overlay = resolveExecDirectorOverlay(config.director);
389395

@@ -396,6 +402,7 @@ export async function runExec(config: Config): Promise<ExecResult> {
396402
...(shellTimeout !== undefined ? { shellTimeout } : {}),
397403
...(toolWatchdog !== undefined ? { toolWatchdog } : {}),
398404
...(localSettingsForMode?.env !== undefined ? { shellEnv: localSettingsForMode.env } : {}),
405+
getBlobWriter: () => currentStorage?.writeBlob,
399406
getBlobReader: () => {
400407
if (currentAgent === null) {
401408
throw new Error("blob reader requested before agent init");
@@ -610,6 +617,7 @@ export async function runExec(config: Config): Promise<ExecResult> {
610617

611618
const buildAgent = async (): Promise<Agent> => {
612619
const storage = await createOptimizedContextStore(workdir);
620+
currentStorage = storage;
613621
const sources = liveSources.length > 0 ? liveSources : [liveSource];
614622
const defaultSource = liveDefaultSource.length > 0 ? liveDefaultSource : liveSource.id;
615623
// Prefer liveSource credentials on the active id when OAuth was refreshed.

src/mcp/plugin.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,32 @@ import type { ToolCall, ToolResult } from "@intx/types/runtime";
33
import type { PermissionGate } from "../permission/gate.js";
44
import { gateToolCall } from "../plugins/permission-plugin.js";
55
import { scrubSecretShapedToolResultContent } from "../plugins/tool-result-secret-scrub.js";
6-
import { truncateToolResultContent } from "../plugins/result-truncation-plugin.js";
6+
import {
7+
truncateToolResultContent,
8+
type SpillBlobWriter,
9+
} from "../plugins/result-truncation-plugin.js";
710
import type { MCPClient } from "./client.js";
811
import { mcpToolName } from "./tool-name.js";
912

1013
// MCP results never reach the posix runner, so the secret-scrub and truncation
1114
// middleware in src/plugins never see them. Apply the same scrub-then-truncate
1215
// order here directly (see buildCorePosixToolPlugins) so a compromised MCP
1316
// server cannot leak credential-shaped strings or flood the transcript.
14-
function sanitizeMcpResultContent(content: string): string {
15-
return truncateToolResultContent(scrubSecretShapedToolResultContent(content));
17+
function sanitizeMcpResultContent(
18+
content: string,
19+
spill?: { callId: string; writeBlob: SpillBlobWriter },
20+
): Promise<string> {
21+
return truncateToolResultContent(scrubSecretShapedToolResultContent(content), undefined, spill);
1622
}
1723

1824
// Convert a connected client's tools into AgentTools for the dynamic runner used
1925
// by the TUI. These tools live in a separate runner from the posix tool plugin
2026
// chain, so each handler is wrapped with the permission gate directly.
21-
export function mcpClientToAgentTools(client: MCPClient, gate: PermissionGate): AgentTool[] {
27+
export function mcpClientToAgentTools(
28+
client: MCPClient,
29+
gate: PermissionGate,
30+
getBlobWriter?: () => SpillBlobWriter | undefined,
31+
): AgentTool[] {
2232
return client.tools.map((tool) => ({
2333
kind: "full" as const,
2434
definition: {
@@ -30,7 +40,9 @@ export function mcpClientToAgentTools(client: MCPClient, gate: PermissionGate):
3040
gateToolCall(gate, call, signal, async () => {
3141
try {
3242
const content = await client.call(tool.name, call.arguments, signal);
33-
return { callId: call.id, content: sanitizeMcpResultContent(content) };
43+
const writeBlob = getBlobWriter?.();
44+
const spill = writeBlob !== undefined ? { callId: call.id, writeBlob } : undefined;
45+
return { callId: call.id, content: await sanitizeMcpResultContent(content, spill) };
3446
} catch (err) {
3547
return {
3648
callId: call.id,
Lines changed: 148 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,170 @@
11
import { describe, expect, test } from "bun:test";
22
import { createSizeCapTransform } from "@intx/inference";
3-
import type { StrategyContext, ToolResult } from "@intx/types/runtime";
4-
import { MAX_RESULT_CHARS, truncateToolResultContent } from "./result-truncation-plugin.js";
3+
import { createBlobReader, type StrategyContext, type ToolResult } from "@intx/types/runtime";
4+
import {
5+
MAX_RESULT_CHARS,
6+
resultTruncationPlugin,
7+
spillBlobKey,
8+
truncateToolResultContent,
9+
} from "./result-truncation-plugin.js";
10+
11+
/** In-memory stand-in for ContextStore's writeBlob/readBlob pair, for tests. */
12+
function fakeBlobStore() {
13+
const blobs = new Map<string, Uint8Array>();
14+
return {
15+
blobs,
16+
writeBlob: async (key: string, bytes: Uint8Array) => {
17+
blobs.set(key, bytes);
18+
},
19+
readBlob: async (key: string) => {
20+
const bytes = blobs.get(key);
21+
if (bytes === undefined) throw new Error(`Blob not found: ${key}`);
22+
return bytes;
23+
},
24+
};
25+
}
526

627
describe("truncateToolResultContent", () => {
7-
test("within-cap content passes through unchanged", () => {
28+
test("within-cap content passes through unchanged", async () => {
829
const content = "x".repeat(100);
9-
expect(truncateToolResultContent(content)).toBe(content);
30+
expect(await truncateToolResultContent(content)).toBe(content);
1031
});
1132

12-
test("oversized content gets a marker that never promises retrievable remainder", () => {
33+
test("oversized content with no blob store gets a marker that never promises retrievable remainder", async () => {
1334
const content = "x".repeat(MAX_RESULT_CHARS + 500);
14-
const truncated = truncateToolResultContent(content);
35+
const truncated = await truncateToolResultContent(content);
1536

1637
expect(truncated).toContain("[output truncated");
1738
expect(truncated).toContain("NOT retrievable");
1839
// The pre-cap discard must never be described as recoverable elsewhere.
1940
expect(truncated).not.toContain("see the rest");
2041
expect(truncated).not.toContain("Full output available");
42+
// And it must never promise a lifetime it doesn't control either way.
43+
expect(truncated).not.toContain("removed");
44+
expect(truncated).not.toContain("session ends");
2145
});
2246

23-
test("truncation marker survives the size-cap blob spill", async () => {
24-
// Reproduce the production pipeline for an output over MAX_RESULT_CHARS:
25-
// truncation runs first (at the tool), size-cap spills the already-cut
26-
// text to a blob and tells the model the blob holds the full output. The
27-
// blob's tail must therefore carry the honest "discarded, NOT retrievable"
28-
// marker so the model does not loop re-running the command.
29-
const original = "x".repeat(MAX_RESULT_CHARS + 500);
30-
const truncated = truncateToolResultContent(original);
31-
32-
const blobs = new Map<string, string>();
33-
const transform = createSizeCapTransform({
34-
maxChars: 10_000,
35-
contextStore: {
36-
writeBlob: async (key: string, bytes: Uint8Array) => {
37-
blobs.set(key, new TextDecoder().decode(bytes));
38-
},
39-
},
47+
test("the inlined portion stays bounded regardless of blob-store support", async () => {
48+
const content = "x".repeat(MAX_RESULT_CHARS * 3);
49+
const truncated = await truncateToolResultContent(content);
50+
// The marker text itself adds a bounded amount of overhead on top of the cap.
51+
expect(truncated.length).toBeLessThan(MAX_RESULT_CHARS + 1000);
52+
});
53+
54+
describe("with a blob store", () => {
55+
test("a result over the cap is fully recoverable by following the notice's read_file instructions verbatim", async () => {
56+
const store = fakeBlobStore();
57+
const original = `${"x".repeat(MAX_RESULT_CHARS)}TAIL-MARKER-${"y".repeat(500)}`;
58+
const truncated = await truncateToolResultContent(original, MAX_RESULT_CHARS, {
59+
callId: "call-42",
60+
writeBlob: store.writeBlob,
61+
});
62+
63+
// Inline content is bounded and does not itself contain the discarded tail.
64+
expect(truncated).not.toContain("TAIL-MARKER");
65+
expect(truncated.length).toBeLessThan(MAX_RESULT_CHARS + 1000);
66+
67+
const uriMatch = /tool-output:\/\/\/\S+/.exec(truncated);
68+
expect(uriMatch).not.toBeNull();
69+
const uri = uriMatch?.[0].replace(/[.\]]+$/, "") ?? "";
70+
expect(uri).toBe(`tool-output:///${spillBlobKey("call-42")}`);
71+
72+
// Follow the notice's instructions literally: read_file with that URI,
73+
// via the real BlobReader machinery read_file itself uses.
74+
const blobReader = createBlobReader(store);
75+
const recoveredBytes = await blobReader.read(uri);
76+
const recovered = new TextDecoder().decode(recoveredBytes);
77+
expect(recovered).toBe(original);
78+
expect(recovered).toContain("TAIL-MARKER");
79+
expect(recovered.length).toBe(original.length);
80+
81+
// No false lifetime claim: the blob is part of the committed session
82+
// history, not something with its own expiry.
83+
expect(truncated).not.toContain("removed");
84+
expect(truncated).not.toContain("session ends");
4085
});
4186

42-
const result: ToolResult = {
43-
callId: "call-1",
44-
content: truncated,
45-
isError: false,
46-
};
47-
const { output } = await transform.apply(
48-
{ call: { id: "call-1", name: "run_shell", arguments: {} }, result },
49-
{} as StrategyContext,
87+
test("within-cap content never writes a blob", async () => {
88+
const store = fakeBlobStore();
89+
await truncateToolResultContent("x".repeat(100), MAX_RESULT_CHARS, {
90+
callId: "call-1",
91+
writeBlob: store.writeBlob,
92+
});
93+
expect(store.blobs.size).toBe(0);
94+
});
95+
96+
test(
97+
"the full spill survives the reactor's own downstream size-cap transform " +
98+
"(CL-6908 regression: a same-keyed write here would let that second write clobber it)",
99+
async () => {
100+
const store = fakeBlobStore();
101+
const original = "p".repeat(500_000);
102+
const truncated = await truncateToolResultContent(original, MAX_RESULT_CHARS, {
103+
callId: "call-1",
104+
writeBlob: store.writeBlob,
105+
});
106+
107+
// Reproduce the production pipeline: this middleware's ToolResult
108+
// continues into the reactor, which always runs its own size-cap
109+
// transform (vendor/intx-inference, default cap 10,000 chars) on
110+
// every result, keyed by the bare call id.
111+
const reactorCap = createSizeCapTransform({
112+
maxChars: 10_000,
113+
contextStore: { writeBlob: store.writeBlob },
114+
});
115+
const result: ToolResult = { callId: "call-1", content: truncated, isError: false };
116+
await reactorCap.apply(
117+
{ call: { id: "call-1", name: "run_shell", arguments: {} }, result },
118+
{} as StrategyContext,
119+
);
120+
121+
// The reactor wrote its own (lossy) blob under the bare "call-1" key.
122+
expect(store.blobs.has("call-1")).toBe(true);
123+
// Our full spill lives under a distinct key and is untouched.
124+
const blobReader = createBlobReader(store);
125+
const recovered = new TextDecoder().decode(
126+
await blobReader.read(`tool-output:///${spillBlobKey("call-1")}`),
127+
);
128+
expect(recovered).toBe(original);
129+
expect(recovered.length).toBe(500_000);
130+
},
131+
);
132+
});
133+
});
134+
135+
describe("resultTruncationPlugin", () => {
136+
test("spills oversized run_shell/grep/search_files/web_fetch results via the live getBlobWriter getter", async () => {
137+
const store = fakeBlobStore();
138+
const original = "q".repeat(MAX_RESULT_CHARS + 200);
139+
const plugin = resultTruncationPlugin({ getBlobWriter: () => store.writeBlob });
140+
if (plugin.middleware === undefined) throw new Error("expected middleware");
141+
const middleware = plugin.middleware(async (call) => ({
142+
callId: call.id,
143+
content: original,
144+
}));
145+
146+
const result = await middleware(
147+
{ id: "call-99", name: "run_shell", arguments: {} },
148+
new AbortController().signal,
50149
);
51150

52-
const spilled = blobs.get("call-1");
53-
expect(spilled).toBe(truncated);
54-
// The blob's tail tells the truth about the pre-spill discard.
55-
expect(spilled).toContain("NOT retrievable");
56-
expect(spilled?.endsWith("Use offset/limit or a narrower query.]")).toBe(true);
57-
// The inline marker's blob promise is now genuine: the blob really does
58-
// hold everything that still exists.
59-
expect(output.content).toContain("tool-output:///call-1");
151+
const uri = `tool-output:///${spillBlobKey("call-99")}`;
152+
expect(result.content).toContain(uri);
153+
const recovered = new TextDecoder().decode(await createBlobReader(store).read(uri));
154+
expect(recovered).toBe(original);
155+
});
156+
157+
test("falls back to the honest no-store notice when getBlobWriter resolves undefined", async () => {
158+
const plugin = resultTruncationPlugin({ getBlobWriter: () => undefined });
159+
if (plugin.middleware === undefined) throw new Error("expected middleware");
160+
const middleware = plugin.middleware(async (call) => ({
161+
callId: call.id,
162+
content: "r".repeat(MAX_RESULT_CHARS + 1),
163+
}));
164+
const result = await middleware(
165+
{ id: "call-1", name: "grep", arguments: {} },
166+
new AbortController().signal,
167+
);
168+
expect(result.content).toContain("NOT retrievable");
60169
});
61170
});

0 commit comments

Comments
 (0)