diff --git a/src/plugins/bounded-grep-fallback.ts b/src/plugins/bounded-grep-fallback.ts index c330d1478..5dc51df32 100644 --- a/src/plugins/bounded-grep-fallback.ts +++ b/src/plugins/bounded-grep-fallback.ts @@ -12,16 +12,12 @@ export const BOUNDED_GREP_MAX_DIRECTORY_ENTRIES = 25_000; /** Max bytes read from any single file during content search. */ export const BOUNDED_GREP_MAX_PER_FILE_BYTES = 512_000; -/** Max bytes in the formatted result string. */ -export const BOUNDED_GREP_MAX_OUTPUT_BYTES = 512_000; - export const BOUNDED_GREP_DEFAULT_MAX_RESULTS = 500; export const BOUNDED_SEARCH_DEFAULT_MAX_RESULTS = 1000; export type BoundedGrepLimits = { maxDirectoryEntries?: number; maxPerFileBytes?: number; - maxOutputBytes?: number; }; export type BoundedGrepArgs = { @@ -97,15 +93,6 @@ function isBinary(buf: Buffer): boolean { return buf.includes(0); } -function capOutput(text: string, maxBytes: number): string { - if (Buffer.byteLength(text, "utf8") <= maxBytes) return text; - let cut = text; - while (cut.length > 0 && Buffer.byteLength(cut, "utf8") > maxBytes) { - cut = cut.slice(0, Math.floor(cut.length * 0.9)); - } - return `${cut}\n... (output truncated at ${maxBytes} bytes; narrow path/glob or pattern)`; -} - async function collectFilePaths( basePath: string, globFilter: RegExp | null, @@ -270,7 +257,6 @@ export async function runBoundedGrep( const maxDirectoryEntries = limits.maxDirectoryEntries ?? BOUNDED_GREP_MAX_DIRECTORY_ENTRIES; const maxPerFileBytes = limits.maxPerFileBytes ?? BOUNDED_GREP_MAX_PER_FILE_BYTES; - const maxOutputBytes = limits.maxOutputBytes ?? BOUNDED_GREP_MAX_OUTPUT_BYTES; const basePath = resolve(baseCwd, args.path ?? "."); const contextLines = args.context ?? 0; @@ -329,7 +315,7 @@ export async function runBoundedGrep( if (walkTruncated) { output += `\n... (directory walk capped at ${maxDirectoryEntries} files; narrow path/glob)`; } - return capOutput(output, maxOutputBytes); + return output; } export async function runBoundedSearchFiles( @@ -341,7 +327,6 @@ export async function runBoundedSearchFiles( signal.throwIfAborted(); const maxDirectoryEntries = limits.maxDirectoryEntries ?? BOUNDED_GREP_MAX_DIRECTORY_ENTRIES; - const maxOutputBytes = limits.maxOutputBytes ?? BOUNDED_GREP_MAX_OUTPUT_BYTES; const basePath = resolve(baseCwd, args.path ?? "."); const maxResults = args.max_results ?? BOUNDED_SEARCH_DEFAULT_MAX_RESULTS; @@ -391,6 +376,6 @@ export async function runBoundedSearchFiles( if (walkTruncated) { result += `\n... (directory walk capped at ${maxDirectoryEntries} files; narrow path/glob)`; } - return capOutput(result, maxOutputBytes); + return result; } diff --git a/src/plugins/result-truncation-plugin.ts b/src/plugins/result-truncation-plugin.ts index 4b8985bb8..9f7ec6c29 100644 --- a/src/plugins/result-truncation-plugin.ts +++ b/src/plugins/result-truncation-plugin.ts @@ -4,16 +4,23 @@ const TRUNCATABLE_TOOLS = new Set(["read_file", "grep", "run_shell", "search_fil // Characters, not tokens — conversion ratio is roughly 4 chars/token. // 80 000 chars ≈ 20 000 tokens. Keeps a single result from dominating context. -const MAX_RESULT_CHARS = 80_000; +export const MAX_RESULT_CHARS = 80_000; -// Shared with the MCP tool runner (src/mcp/plugin.ts), which is not part of the -// posix runner this middleware wraps and so applies the same truncation directly. -export function truncateToolResultContent(content: string): string { - if (content.length <= MAX_RESULT_CHARS) return content; +// 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 +// middleware does not wrap — the MCP tool runner (src/mcp/plugin.ts), and +// ripgrep-plugin.ts, which answers grep without calling next and so never +// reaches this middleware despite sitting earlier in the same plugin array. +export function truncateToolResultContent( + content: string, + maxChars: number = MAX_RESULT_CHARS, +): string { + if (content.length <= maxChars) return content; - const remaining = content.length - MAX_RESULT_CHARS; + const remaining = content.length - maxChars; return ( - content.slice(0, MAX_RESULT_CHARS) + + content.slice(0, maxChars) + `\n[output truncated — ${remaining.toLocaleString()} characters omitted. ` + `Use offset/limit params or a more targeted query to see the rest.]` ); diff --git a/src/plugins/rg-output.test.ts b/src/plugins/rg-output.test.ts index ddfb472a6..77f2250fd 100644 --- a/src/plugins/rg-output.test.ts +++ b/src/plugins/rg-output.test.ts @@ -7,10 +7,11 @@ const line = "big.txt:1:match line here\n"; test("the cap fires on the chunk that breaches it", () => { const collector = createRgCollector(200); expect(collector.push(line.repeat(4))).toBeUndefined(); - expect(collector.push(line.repeat(20))).toMatchObject({ - kind: "partial", - notice: expect.stringContaining("exceeded 200 bytes"), - }); + const outcome = collector.push(line.repeat(20)); + expect(outcome).toMatchObject({ kind: "partial" }); + // No notice of its own: the final tool result gets exactly one truncation + // notice, from result-truncation-plugin.ts, not one per cap that fired. + expect(outcome?.kind === "partial" ? outcome.notice : "defined").toBeUndefined(); }); test("an over-cap run reports no more than the cap, cut at a line boundary", () => { diff --git a/src/plugins/rg-output.ts b/src/plugins/rg-output.ts index 1e3193009..88c6b4a61 100644 --- a/src/plugins/rg-output.ts +++ b/src/plugins/rg-output.ts @@ -9,7 +9,7 @@ export type RgOutcome = | { kind: "output"; stdout: string } | { kind: "no-match" } | { kind: "error"; message: string } - | { kind: "partial"; stdout: string; notice: string }; + | { kind: "partial"; stdout: string; notice?: string }; export type RgCollector = { /** Returns an outcome once the cap is breached, otherwise undefined. */ @@ -37,12 +37,15 @@ export function createRgCollector(maxOutputBytes: number): RgCollector { return outcome; }; + // No notice here: the cap only stops collection early to bound memory + // while the stream is still live. The result-truncation-plugin.ts pass + // that runs over the final tool result is the single place a "truncated" + // notice gets attached, so this cap does not add one of its own. const overCap = (): RgOutcome | undefined => { if (stdout.length <= maxOutputBytes) return undefined; return settle({ kind: "partial", stdout: truncateToWholeLines(stdout, maxOutputBytes), - notice: `search output exceeded ${maxOutputBytes} bytes — showing partial results; narrow path/glob or pattern`, }); }; diff --git a/src/plugins/rg-run.test.ts b/src/plugins/rg-run.test.ts index dd7bd4f87..5e55b2986 100644 --- a/src/plugins/rg-run.test.ts +++ b/src/plugins/rg-run.test.ts @@ -48,7 +48,9 @@ test("an over-cap run is capped regardless of how stdout is chunked", async () = expect(result.kind).toBe("partial"); if (result.kind !== "partial") continue; expect(result.stdout.length).toBeLessThanOrEqual(200); - expect(result.notice).toContain("exceeded 200 bytes"); + // No notice of its own: the final tool result gets exactly one + // truncation notice, from result-truncation-plugin.ts. + expect(result.notice).toBeUndefined(); } }); diff --git a/src/plugins/ripgrep-plugin.ts b/src/plugins/ripgrep-plugin.ts index 786b97c84..c0994a150 100644 --- a/src/plugins/ripgrep-plugin.ts +++ b/src/plugins/ripgrep-plugin.ts @@ -1,6 +1,7 @@ import { statSync } from "node:fs"; import { dirname, basename } from "node:path"; import type { ToolPlugin } from "@intx/tools-posix"; +import type { ToolResult } from "@intx/types/runtime"; import { runBoundedGrep, @@ -8,6 +9,7 @@ import { type BoundedGrepArgs, } from "./bounded-grep-fallback.js"; import { createRgCollector } from "./rg-output.js"; +import { truncateToolResultContent } from "./result-truncation-plugin.js"; import { MAX_OUTPUT_BYTES, runRg, type RgLimits, type SpawnRg } from "./rg-run.js"; // 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 const DEFAULT_GREP_MAX = 500; const DEFAULT_SEARCH_MAX = 1000; +// Dropping matches is a different fact from dropping characters, and the size +// pass below cannot infer it: a run can be well under the char cap and still +// have discarded thousands of matches. That omission is announced here; the +// size cap announces its own. function capLines(text: string, max: number): string { const lines = text.split("\n").filter((line) => line.length > 0); if (lines.length <= max) return lines.join("\n"); - return `${lines.slice(0, max).join("\n")}\n... (showing first ${max} of ${lines.length}+ lines; narrow path/glob)`; + return `${lines.slice(0, max).join("\n")}\n... (showing first ${max} of ${lines.length}+ matches; narrow path/glob)`; +} + +// ripgrepPlugin answers grep and search_files without calling next, so the +// result-truncation middleware sitting later in the chain never sees these +// results. The shared primitive is applied here instead, keeping one wording +// for size truncation on a path that would otherwise return uncapped. +function bounded(callId: string, content: string): ToolResult { + return { callId, content: truncateToolResultContent(content) }; } // Mirrors read_file's truncate-and-offer behavior: a cap or timeout still // surfaces whatever matches were collected before it fired, instead of -// discarding them behind a bare error. -function partialContent(stdout: string, maxResults: number, notice: string): string { +// discarding them behind a bare error. `notice` is only set for conditions +// neither cap describes, like a run timing out. +function partialContent(stdout: string, maxResults: number, notice?: string): string { const capped = capLines(stdout, maxResults); - if (capped.length === 0) return `no matches collected before ${notice}`; - return `${capped}\n... ${notice}`; + if (capped.length === 0) { + return notice === undefined ? "no matches collected" : `no matches collected before ${notice}`; + } + return notice === undefined ? capped : `${capped}\n... ${notice}`; } // 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 }; if (glob !== undefined) boundedArgs.glob = glob; const content = await runBoundedGrep(boundedArgs, signal, rgCwd); - return { callId: call.id, content: boundedContent(content, maxResults, maxBytes) }; + return bounded(call.id, boundedContent(content, maxResults, maxBytes)); } catch (err) { return { callId: call.id, @@ -113,9 +130,9 @@ export function ripgrepPlugin(cwd: string, limits: RgLimits = {}, spawnChild?: S return { callId: call.id, content: result.message, isError: true }; } if (result.kind === "partial") { - return { callId: call.id, content: partialContent(result.stdout, maxResults, result.notice) }; + return bounded(call.id, partialContent(result.stdout, maxResults, result.notice)); } - return { callId: call.id, content: capLines(result.stdout, maxResults) }; + return bounded(call.id, capLines(result.stdout, maxResults)); } if (call.name === "search_files") { @@ -133,7 +150,7 @@ export function ripgrepPlugin(cwd: string, limits: RgLimits = {}, spawnChild?: S signal, rgCwd, ); - return { callId: call.id, content: boundedContent(content, maxResults, maxBytes) }; + return bounded(call.id, boundedContent(content, maxResults, maxBytes)); } catch (err) { return { callId: call.id, @@ -149,9 +166,9 @@ export function ripgrepPlugin(cwd: string, limits: RgLimits = {}, spawnChild?: S return { callId: call.id, content: result.message, isError: true }; } if (result.kind === "partial") { - return { callId: call.id, content: partialContent(result.stdout, maxResults, result.notice) }; + return bounded(call.id, partialContent(result.stdout, maxResults, result.notice)); } - return { callId: call.id, content: capLines(result.stdout, maxResults) }; + return bounded(call.id, capLines(result.stdout, maxResults)); } return next(call, signal); diff --git a/tests/unit/ripgrep-plugin.test.ts b/tests/unit/ripgrep-plugin.test.ts index 4df6b29c6..02291a614 100644 --- a/tests/unit/ripgrep-plugin.test.ts +++ b/tests/unit/ripgrep-plugin.test.ts @@ -4,7 +4,12 @@ import { join } from "node:path"; import { tmpdir } from "node:os"; import type { ToolCall, ToolResult } from "@intx/types/runtime"; +import { createPosixTools } from "@intx/tools-posix"; + import { ripgrepPlugin } from "../../src/plugins/ripgrep-plugin.js"; +import { MAX_RESULT_CHARS } from "../../src/plugins/result-truncation-plugin.js"; +import { buildCorePosixToolPlugins } from "../../src/agent/posix-tool-plugins.js"; +import { createPermissionGate } from "../../src/permission/gate.js"; import type { RgChild, SpawnRg } from "../../src/plugins/rg-run.js"; // Repo root derived from this file, not process.cwd(): these cases search real @@ -32,6 +37,34 @@ const stalledSpawn: SpawnRg = (): RgChild => ({ kill: () => undefined, }); +// A child whose stdout is scripted directly, bypassing a real `rg` process +// (and its own --max-count filtering) so the byte cap and the line-count cap +// can both be forced to fire on the same run. +function scriptedSpawn(stdout: string, code: number | null): SpawnRg { + return () => { + let onData: ((chunk: unknown) => void) | undefined; + let onClose: ((code: number | null) => void) | undefined; + const child: RgChild = { + pid: undefined, + stdout: { + on: (_event, listener) => { + onData = listener; + }, + }, + stderr: { on: () => undefined }, + on: ((event: string, listener: (arg: never) => void) => { + if (event === "close") onClose = listener as (code: number | null) => void; + }) as RgChild["on"], + kill: () => undefined, + }; + queueMicrotask(() => { + onData?.(stdout); + onClose?.(code); + }); + return child; + }; +} + async function withTempDir(run: (dir: string) => Promise): Promise { const dir = await mkdtemp(join(tmpdir(), "ripgrep-plugin-")); try { @@ -90,8 +123,7 @@ test("grep returns partial matches when the output byte cap is hit", async () => ); expect(result.isError).toBeUndefined(); expect(result.content).toContain("match line here"); - expect(result.content).toContain("exceeded 200 bytes"); - expect(result.content).toContain("narrow path/glob or pattern"); + expect(String(result.content).length).toBeLessThanOrEqual(200); }); }); @@ -117,8 +149,85 @@ test("the output byte cap holds when ripgrep is unavailable", async () => { ); expect(result.isError).toBeUndefined(); expect(result.content).toContain("match line here"); - expect(result.content).toContain("exceeded 200 bytes"); - expect(result.content.length).toBeLessThan(400); + expect(String(result.content).length).toBeLessThan(400); + }); + }); +}); + +// A grep run that both breaches the byte cap (rg-output.ts) and matches more +// lines than max_results (ripgrep-plugin.ts's own count cap) used to stack the +// byte cap's wording on top of the count cap's. The byte cap is silent now, so +// what is left describes the omission the reader cannot otherwise detect: how +// many matches were dropped. +test("a grep result that hits both the byte cap and the match-count cap announces the dropped matches once", async () => { + // 400 matched lines emitted directly, bypassing a real `rg` process so + // nothing upstream of ripgrep-plugin.ts pre-limits the line count. + const result = await run( + { id: "c", name: "grep", arguments: { pattern: "match", path: cwd, max_results: 3 } }, + { maxOutputBytes: 200 }, + scriptedSpawn("big.txt:1:match line here\n".repeat(400), 0), + ); + + expect(result.isError).toBeUndefined(); + const content = String(result.content); + expect(content.split("\n").filter((l) => l.includes("match line here")).length).toBe(3); + expect((content.match(/showing first/g) ?? []).length).toBe(1); + expect(content).not.toMatch(/exceeded \d+ bytes|timed out|\[output truncated/); +}); + +// Composed through buildCorePosixToolPlugins, not a hand-assembled pair: +// ripgrepPlugin sits at an earlier array index than resultTruncationPlugin and +// answers grep without calling next, so resultTruncationPlugin never sees a +// grep result. Assembling the two by hand in the other order hides that and +// lets an oversized result reach the model uncapped and unannounced. +async function grepThroughRealChain(dir: string): Promise { + const gate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: true, + cwd: dir, + }); + const runner = createPosixTools({ + cwd: dir, + plugins: buildCorePosixToolPlugins({ cwd: dir, permissionGate: gate }), + }); + const result = await runner.run( + { id: "c", name: "grep", arguments: { pattern: "line", path: dir } }, + new AbortController().signal, + ); + expect(result.isError).not.toBe(true); + return String(result.content); +} + +async function writeOversizedHaystack(dir: string): Promise { + const lines = Array.from({ length: 5000 }, (_, i) => `line ${i} ${"x".repeat(300)}`); + await writeFile(join(dir, "big.txt"), lines.join("\n") + "\n"); +} + +function truncationNoticeCount(content: string): number { + return (content.match(/\[output truncated/g) ?? []).length; +} + +test("an oversized grep result is capped and announced once through the real plugin chain", async () => { + await withTempDir(async (dir) => { + await writeOversizedHaystack(dir); + const content = await grepThroughRealChain(dir); + + expect(content.length).toBeLessThanOrEqual(MAX_RESULT_CHARS + 200); + expect(truncationNoticeCount(content)).toBe(1); + expect(content).not.toContain("showing first"); + }); +}); + +test("an oversized grep result is capped and announced once when ripgrep is unavailable", async () => { + await withTempDir(async (dir) => { + await writeOversizedHaystack(dir); + await withoutRipgrep(async () => { + const content = await grepThroughRealChain(dir); + + expect(content.length).toBeLessThanOrEqual(MAX_RESULT_CHARS + 200); + expect(truncationNoticeCount(content)).toBe(1); + expect(content).not.toContain("showing first"); }); }); });