Skip to content

Commit fbc2c69

Browse files
Merge pull request #400 from corbitsdev/cl-5674-four-different-tool-output-truncation-implementations
Consolidate the four grep-output truncation implementations into one
2 parents 336cfd5 + 664d799 commit fbc2c69

7 files changed

Lines changed: 170 additions & 46 deletions

File tree

src/plugins/bounded-grep-fallback.ts

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,12 @@ export const BOUNDED_GREP_MAX_DIRECTORY_ENTRIES = 25_000;
1212
/** Max bytes read from any single file during content search. */
1313
export const BOUNDED_GREP_MAX_PER_FILE_BYTES = 512_000;
1414

15-
/** Max bytes in the formatted result string. */
16-
export const BOUNDED_GREP_MAX_OUTPUT_BYTES = 512_000;
17-
1815
export const BOUNDED_GREP_DEFAULT_MAX_RESULTS = 500;
1916
export const BOUNDED_SEARCH_DEFAULT_MAX_RESULTS = 1000;
2017

2118
export type BoundedGrepLimits = {
2219
maxDirectoryEntries?: number;
2320
maxPerFileBytes?: number;
24-
maxOutputBytes?: number;
2521
};
2622

2723
export type BoundedGrepArgs = {
@@ -97,15 +93,6 @@ function isBinary(buf: Buffer): boolean {
9793
return buf.includes(0);
9894
}
9995

100-
function capOutput(text: string, maxBytes: number): string {
101-
if (Buffer.byteLength(text, "utf8") <= maxBytes) return text;
102-
let cut = text;
103-
while (cut.length > 0 && Buffer.byteLength(cut, "utf8") > maxBytes) {
104-
cut = cut.slice(0, Math.floor(cut.length * 0.9));
105-
}
106-
return `${cut}\n... (output truncated at ${maxBytes} bytes; narrow path/glob or pattern)`;
107-
}
108-
10996
async function collectFilePaths(
11097
basePath: string,
11198
globFilter: RegExp | null,
@@ -270,7 +257,6 @@ export async function runBoundedGrep(
270257

271258
const maxDirectoryEntries = limits.maxDirectoryEntries ?? BOUNDED_GREP_MAX_DIRECTORY_ENTRIES;
272259
const maxPerFileBytes = limits.maxPerFileBytes ?? BOUNDED_GREP_MAX_PER_FILE_BYTES;
273-
const maxOutputBytes = limits.maxOutputBytes ?? BOUNDED_GREP_MAX_OUTPUT_BYTES;
274260

275261
const basePath = resolve(baseCwd, args.path ?? ".");
276262
const contextLines = args.context ?? 0;
@@ -329,7 +315,7 @@ export async function runBoundedGrep(
329315
if (walkTruncated) {
330316
output += `\n... (directory walk capped at ${maxDirectoryEntries} files; narrow path/glob)`;
331317
}
332-
return capOutput(output, maxOutputBytes);
318+
return output;
333319
}
334320

335321
export async function runBoundedSearchFiles(
@@ -341,7 +327,6 @@ export async function runBoundedSearchFiles(
341327
signal.throwIfAborted();
342328

343329
const maxDirectoryEntries = limits.maxDirectoryEntries ?? BOUNDED_GREP_MAX_DIRECTORY_ENTRIES;
344-
const maxOutputBytes = limits.maxOutputBytes ?? BOUNDED_GREP_MAX_OUTPUT_BYTES;
345330

346331
const basePath = resolve(baseCwd, args.path ?? ".");
347332
const maxResults = args.max_results ?? BOUNDED_SEARCH_DEFAULT_MAX_RESULTS;
@@ -391,6 +376,6 @@ export async function runBoundedSearchFiles(
391376
if (walkTruncated) {
392377
result += `\n... (directory walk capped at ${maxDirectoryEntries} files; narrow path/glob)`;
393378
}
394-
return capOutput(result, maxOutputBytes);
379+
return result;
395380
}
396381

src/plugins/result-truncation-plugin.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,23 @@ const TRUNCATABLE_TOOLS = new Set(["read_file", "grep", "run_shell", "search_fil
44

55
// Characters, not tokens — conversion ratio is roughly 4 chars/token.
66
// 80 000 chars ≈ 20 000 tokens. Keeps a single result from dominating context.
7-
const MAX_RESULT_CHARS = 80_000;
7+
export const MAX_RESULT_CHARS = 80_000;
88

9-
// Shared with the MCP tool runner (src/mcp/plugin.ts), which is not part of the
10-
// posix runner this middleware wraps and so applies the same truncation directly.
11-
export function truncateToolResultContent(content: string): string {
12-
if (content.length <= MAX_RESULT_CHARS) return content;
9+
// The single primitive for size truncation: callers may pass their own
10+
// threshold but never invent their own wording, so a result can never carry
11+
// two differently-worded "truncated" notices. Called directly by runners this
12+
// middleware does not wrap — the MCP tool runner (src/mcp/plugin.ts), and
13+
// ripgrep-plugin.ts, which answers grep without calling next and so never
14+
// reaches this middleware despite sitting earlier in the same plugin array.
15+
export function truncateToolResultContent(
16+
content: string,
17+
maxChars: number = MAX_RESULT_CHARS,
18+
): string {
19+
if (content.length <= maxChars) return content;
1320

14-
const remaining = content.length - MAX_RESULT_CHARS;
21+
const remaining = content.length - maxChars;
1522
return (
16-
content.slice(0, MAX_RESULT_CHARS) +
23+
content.slice(0, maxChars) +
1724
`\n[output truncated — ${remaining.toLocaleString()} characters omitted. ` +
1825
`Use offset/limit params or a more targeted query to see the rest.]`
1926
);

src/plugins/rg-output.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,11 @@ const line = "big.txt:1:match line here\n";
77
test("the cap fires on the chunk that breaches it", () => {
88
const collector = createRgCollector(200);
99
expect(collector.push(line.repeat(4))).toBeUndefined();
10-
expect(collector.push(line.repeat(20))).toMatchObject({
11-
kind: "partial",
12-
notice: expect.stringContaining("exceeded 200 bytes"),
13-
});
10+
const outcome = collector.push(line.repeat(20));
11+
expect(outcome).toMatchObject({ kind: "partial" });
12+
// No notice of its own: the final tool result gets exactly one truncation
13+
// notice, from result-truncation-plugin.ts, not one per cap that fired.
14+
expect(outcome?.kind === "partial" ? outcome.notice : "defined").toBeUndefined();
1415
});
1516

1617
test("an over-cap run reports no more than the cap, cut at a line boundary", () => {

src/plugins/rg-output.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export type RgOutcome =
99
| { kind: "output"; stdout: string }
1010
| { kind: "no-match" }
1111
| { kind: "error"; message: string }
12-
| { kind: "partial"; stdout: string; notice: string };
12+
| { kind: "partial"; stdout: string; notice?: string };
1313

1414
export type RgCollector = {
1515
/** Returns an outcome once the cap is breached, otherwise undefined. */
@@ -37,12 +37,15 @@ export function createRgCollector(maxOutputBytes: number): RgCollector {
3737
return outcome;
3838
};
3939

40+
// No notice here: the cap only stops collection early to bound memory
41+
// while the stream is still live. The result-truncation-plugin.ts pass
42+
// that runs over the final tool result is the single place a "truncated"
43+
// notice gets attached, so this cap does not add one of its own.
4044
const overCap = (): RgOutcome | undefined => {
4145
if (stdout.length <= maxOutputBytes) return undefined;
4246
return settle({
4347
kind: "partial",
4448
stdout: truncateToWholeLines(stdout, maxOutputBytes),
45-
notice: `search output exceeded ${maxOutputBytes} bytes — showing partial results; narrow path/glob or pattern`,
4649
});
4750
};
4851

src/plugins/rg-run.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ test("an over-cap run is capped regardless of how stdout is chunked", async () =
4848
expect(result.kind).toBe("partial");
4949
if (result.kind !== "partial") continue;
5050
expect(result.stdout.length).toBeLessThanOrEqual(200);
51-
expect(result.notice).toContain("exceeded 200 bytes");
51+
// No notice of its own: the final tool result gets exactly one
52+
// truncation notice, from result-truncation-plugin.ts.
53+
expect(result.notice).toBeUndefined();
5254
}
5355
});
5456

src/plugins/ripgrep-plugin.ts

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
import { statSync } from "node:fs";
22
import { dirname, basename } from "node:path";
33
import type { ToolPlugin } from "@intx/tools-posix";
4+
import type { ToolResult } from "@intx/types/runtime";
45

56
import {
67
runBoundedGrep,
78
runBoundedSearchFiles,
89
type BoundedGrepArgs,
910
} from "./bounded-grep-fallback.js";
1011
import { createRgCollector } from "./rg-output.js";
12+
import { truncateToolResultContent } from "./result-truncation-plugin.js";
1113
import { MAX_OUTPUT_BYTES, runRg, type RgLimits, type SpawnRg } from "./rg-run.js";
1214

1315
// A grep over a large tree with the pure-TypeScript walker enumerates the whole
@@ -21,19 +23,34 @@ import { MAX_OUTPUT_BYTES, runRg, type RgLimits, type SpawnRg } from "./rg-run.j
2123
const DEFAULT_GREP_MAX = 500;
2224
const DEFAULT_SEARCH_MAX = 1000;
2325

26+
// Dropping matches is a different fact from dropping characters, and the size
27+
// pass below cannot infer it: a run can be well under the char cap and still
28+
// have discarded thousands of matches. That omission is announced here; the
29+
// size cap announces its own.
2430
function capLines(text: string, max: number): string {
2531
const lines = text.split("\n").filter((line) => line.length > 0);
2632
if (lines.length <= max) return lines.join("\n");
27-
return `${lines.slice(0, max).join("\n")}\n... (showing first ${max} of ${lines.length}+ lines; narrow path/glob)`;
33+
return `${lines.slice(0, max).join("\n")}\n... (showing first ${max} of ${lines.length}+ matches; narrow path/glob)`;
34+
}
35+
36+
// ripgrepPlugin answers grep and search_files without calling next, so the
37+
// result-truncation middleware sitting later in the chain never sees these
38+
// results. The shared primitive is applied here instead, keeping one wording
39+
// for size truncation on a path that would otherwise return uncapped.
40+
function bounded(callId: string, content: string): ToolResult {
41+
return { callId, content: truncateToolResultContent(content) };
2842
}
2943

3044
// Mirrors read_file's truncate-and-offer behavior: a cap or timeout still
3145
// surfaces whatever matches were collected before it fired, instead of
32-
// discarding them behind a bare error.
33-
function partialContent(stdout: string, maxResults: number, notice: string): string {
46+
// discarding them behind a bare error. `notice` is only set for conditions
47+
// neither cap describes, like a run timing out.
48+
function partialContent(stdout: string, maxResults: number, notice?: string): string {
3449
const capped = capLines(stdout, maxResults);
35-
if (capped.length === 0) return `no matches collected before ${notice}`;
36-
return `${capped}\n... ${notice}`;
50+
if (capped.length === 0) {
51+
return notice === undefined ? "no matches collected" : `no matches collected before ${notice}`;
52+
}
53+
return notice === undefined ? capped : `${capped}\n... ${notice}`;
3754
}
3855

3956
// The fallback walker collects its whole result in memory before returning, so
@@ -97,7 +114,7 @@ export function ripgrepPlugin(cwd: string, limits: RgLimits = {}, spawnChild?: S
97114
};
98115
if (glob !== undefined) boundedArgs.glob = glob;
99116
const content = await runBoundedGrep(boundedArgs, signal, rgCwd);
100-
return { callId: call.id, content: boundedContent(content, maxResults, maxBytes) };
117+
return bounded(call.id, boundedContent(content, maxResults, maxBytes));
101118
} catch (err) {
102119
return {
103120
callId: call.id,
@@ -113,9 +130,9 @@ export function ripgrepPlugin(cwd: string, limits: RgLimits = {}, spawnChild?: S
113130
return { callId: call.id, content: result.message, isError: true };
114131
}
115132
if (result.kind === "partial") {
116-
return { callId: call.id, content: partialContent(result.stdout, maxResults, result.notice) };
133+
return bounded(call.id, partialContent(result.stdout, maxResults, result.notice));
117134
}
118-
return { callId: call.id, content: capLines(result.stdout, maxResults) };
135+
return bounded(call.id, capLines(result.stdout, maxResults));
119136
}
120137

121138
if (call.name === "search_files") {
@@ -133,7 +150,7 @@ export function ripgrepPlugin(cwd: string, limits: RgLimits = {}, spawnChild?: S
133150
signal,
134151
rgCwd,
135152
);
136-
return { callId: call.id, content: boundedContent(content, maxResults, maxBytes) };
153+
return bounded(call.id, boundedContent(content, maxResults, maxBytes));
137154
} catch (err) {
138155
return {
139156
callId: call.id,
@@ -149,9 +166,9 @@ export function ripgrepPlugin(cwd: string, limits: RgLimits = {}, spawnChild?: S
149166
return { callId: call.id, content: result.message, isError: true };
150167
}
151168
if (result.kind === "partial") {
152-
return { callId: call.id, content: partialContent(result.stdout, maxResults, result.notice) };
169+
return bounded(call.id, partialContent(result.stdout, maxResults, result.notice));
153170
}
154-
return { callId: call.id, content: capLines(result.stdout, maxResults) };
171+
return bounded(call.id, capLines(result.stdout, maxResults));
155172
}
156173

157174
return next(call, signal);

tests/unit/ripgrep-plugin.test.ts

Lines changed: 113 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@ import { join } from "node:path";
44
import { tmpdir } from "node:os";
55
import type { ToolCall, ToolResult } from "@intx/types/runtime";
66

7+
import { createPosixTools } from "@intx/tools-posix";
8+
79
import { ripgrepPlugin } from "../../src/plugins/ripgrep-plugin.js";
10+
import { MAX_RESULT_CHARS } from "../../src/plugins/result-truncation-plugin.js";
11+
import { buildCorePosixToolPlugins } from "../../src/agent/posix-tool-plugins.js";
12+
import { createPermissionGate } from "../../src/permission/gate.js";
813
import type { RgChild, SpawnRg } from "../../src/plugins/rg-run.js";
914

1015
// Repo root derived from this file, not process.cwd(): these cases search real
@@ -32,6 +37,34 @@ const stalledSpawn: SpawnRg = (): RgChild => ({
3237
kill: () => undefined,
3338
});
3439

40+
// A child whose stdout is scripted directly, bypassing a real `rg` process
41+
// (and its own --max-count filtering) so the byte cap and the line-count cap
42+
// can both be forced to fire on the same run.
43+
function scriptedSpawn(stdout: string, code: number | null): SpawnRg {
44+
return () => {
45+
let onData: ((chunk: unknown) => void) | undefined;
46+
let onClose: ((code: number | null) => void) | undefined;
47+
const child: RgChild = {
48+
pid: undefined,
49+
stdout: {
50+
on: (_event, listener) => {
51+
onData = listener;
52+
},
53+
},
54+
stderr: { on: () => undefined },
55+
on: ((event: string, listener: (arg: never) => void) => {
56+
if (event === "close") onClose = listener as (code: number | null) => void;
57+
}) as RgChild["on"],
58+
kill: () => undefined,
59+
};
60+
queueMicrotask(() => {
61+
onData?.(stdout);
62+
onClose?.(code);
63+
});
64+
return child;
65+
};
66+
}
67+
3568
async function withTempDir(run: (dir: string) => Promise<void>): Promise<void> {
3669
const dir = await mkdtemp(join(tmpdir(), "ripgrep-plugin-"));
3770
try {
@@ -90,8 +123,7 @@ test("grep returns partial matches when the output byte cap is hit", async () =>
90123
);
91124
expect(result.isError).toBeUndefined();
92125
expect(result.content).toContain("match line here");
93-
expect(result.content).toContain("exceeded 200 bytes");
94-
expect(result.content).toContain("narrow path/glob or pattern");
126+
expect(String(result.content).length).toBeLessThanOrEqual(200);
95127
});
96128
});
97129

@@ -117,8 +149,85 @@ test("the output byte cap holds when ripgrep is unavailable", async () => {
117149
);
118150
expect(result.isError).toBeUndefined();
119151
expect(result.content).toContain("match line here");
120-
expect(result.content).toContain("exceeded 200 bytes");
121-
expect(result.content.length).toBeLessThan(400);
152+
expect(String(result.content).length).toBeLessThan(400);
153+
});
154+
});
155+
});
156+
157+
// A grep run that both breaches the byte cap (rg-output.ts) and matches more
158+
// lines than max_results (ripgrep-plugin.ts's own count cap) used to stack the
159+
// byte cap's wording on top of the count cap's. The byte cap is silent now, so
160+
// what is left describes the omission the reader cannot otherwise detect: how
161+
// many matches were dropped.
162+
test("a grep result that hits both the byte cap and the match-count cap announces the dropped matches once", async () => {
163+
// 400 matched lines emitted directly, bypassing a real `rg` process so
164+
// nothing upstream of ripgrep-plugin.ts pre-limits the line count.
165+
const result = await run(
166+
{ id: "c", name: "grep", arguments: { pattern: "match", path: cwd, max_results: 3 } },
167+
{ maxOutputBytes: 200 },
168+
scriptedSpawn("big.txt:1:match line here\n".repeat(400), 0),
169+
);
170+
171+
expect(result.isError).toBeUndefined();
172+
const content = String(result.content);
173+
expect(content.split("\n").filter((l) => l.includes("match line here")).length).toBe(3);
174+
expect((content.match(/showing first/g) ?? []).length).toBe(1);
175+
expect(content).not.toMatch(/exceeded \d+ bytes|timed out|\[output truncated/);
176+
});
177+
178+
// Composed through buildCorePosixToolPlugins, not a hand-assembled pair:
179+
// ripgrepPlugin sits at an earlier array index than resultTruncationPlugin and
180+
// answers grep without calling next, so resultTruncationPlugin never sees a
181+
// grep result. Assembling the two by hand in the other order hides that and
182+
// lets an oversized result reach the model uncapped and unannounced.
183+
async function grepThroughRealChain(dir: string): Promise<string> {
184+
const gate = createPermissionGate({
185+
approvals: [],
186+
interactive: false,
187+
skipPermissions: true,
188+
cwd: dir,
189+
});
190+
const runner = createPosixTools({
191+
cwd: dir,
192+
plugins: buildCorePosixToolPlugins({ cwd: dir, permissionGate: gate }),
193+
});
194+
const result = await runner.run(
195+
{ id: "c", name: "grep", arguments: { pattern: "line", path: dir } },
196+
new AbortController().signal,
197+
);
198+
expect(result.isError).not.toBe(true);
199+
return String(result.content);
200+
}
201+
202+
async function writeOversizedHaystack(dir: string): Promise<void> {
203+
const lines = Array.from({ length: 5000 }, (_, i) => `line ${i} ${"x".repeat(300)}`);
204+
await writeFile(join(dir, "big.txt"), lines.join("\n") + "\n");
205+
}
206+
207+
function truncationNoticeCount(content: string): number {
208+
return (content.match(/\[output truncated/g) ?? []).length;
209+
}
210+
211+
test("an oversized grep result is capped and announced once through the real plugin chain", async () => {
212+
await withTempDir(async (dir) => {
213+
await writeOversizedHaystack(dir);
214+
const content = await grepThroughRealChain(dir);
215+
216+
expect(content.length).toBeLessThanOrEqual(MAX_RESULT_CHARS + 200);
217+
expect(truncationNoticeCount(content)).toBe(1);
218+
expect(content).not.toContain("showing first");
219+
});
220+
});
221+
222+
test("an oversized grep result is capped and announced once when ripgrep is unavailable", async () => {
223+
await withTempDir(async (dir) => {
224+
await writeOversizedHaystack(dir);
225+
await withoutRipgrep(async () => {
226+
const content = await grepThroughRealChain(dir);
227+
228+
expect(content.length).toBeLessThanOrEqual(MAX_RESULT_CHARS + 200);
229+
expect(truncationNoticeCount(content)).toBe(1);
230+
expect(content).not.toContain("showing first");
122231
});
123232
});
124233
});

0 commit comments

Comments
 (0)