From 7affee529b9fe0cadfd83bd4f64597d58cd1c256 Mon Sep 17 00:00:00 2001 From: Aiden Cline <63023139+rekram1-node@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:23:53 -0500 Subject: [PATCH] fix(core): harden grep search behavior (#38922) --- packages/core/src/tool/grep.ts | 26 +++++++---- packages/core/test/tool-search.test.ts | 64 +++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 9 deletions(-) diff --git a/packages/core/src/tool/grep.ts b/packages/core/src/tool/grep.ts index fa7326302d8..0f8a2d60d3b 100644 --- a/packages/core/src/tool/grep.ts +++ b/packages/core/src/tool/grep.ts @@ -15,7 +15,9 @@ import { Tool } from "./tool" export const name = "grep" export const Input = Schema.Struct({ - pattern: FileSystem.GrepInput.fields.pattern.annotate({ + pattern: FileSystem.GrepInput.fields.pattern.check( + Schema.isMinLength(1, { message: "Pattern must not be empty" }), + ).annotate({ description: "Regex pattern to search for in file contents", }), path: RelativePath.pipe(Schema.optional).annotate({ @@ -33,7 +35,7 @@ export const Output = Schema.Array(FileSystem.Match) type ModelOutput = typeof Output.Encoded /** Format raw search matches into the familiar concise model output. */ -export const toModelOutput = (output: ModelOutput) => { +export const toModelOutput = (output: ModelOutput, truncated = false) => { const lines = output.length === 0 ? ["No files found"] : [`Found ${output.length} matches`] let current = "" for (const match of output) { @@ -44,6 +46,11 @@ export const toModelOutput = (output: ModelOutput) => { } lines.push(` Line ${match.line}: ${match.text}`) } + if (truncated) + lines.push( + "", + `(Results are truncated: showing first ${output.length} results. Consider using a more specific path or pattern.)`, + ) return lines.join("\n") } @@ -89,13 +96,14 @@ export const Plugin = { Effect.fail(new ToolFailure({ message: `Search path does not exist: ${input.path ?? "."}` })), ), ) - return yield* ripgrep + const limit = input.limit ?? FileSystem.DEFAULT_SEARCH_LIMIT + const matches = yield* ripgrep .grep({ cwd: info?.type === "Directory" ? target : path.dirname(target), pattern: input.pattern, file: info?.type === "File" ? path.basename(target) : undefined, include: input.include, - limit: input.limit ?? FileSystem.DEFAULT_SEARCH_LIMIT, + limit: limit + 1, }) .pipe( Effect.map((result) => @@ -118,16 +126,18 @@ export const Plugin = { ), ), ) + return { matches: matches.slice(0, limit), truncated: matches.length > limit } }).pipe( - Effect.map((output) => ({ - output, + Effect.map((result) => ({ + output: result.matches, content: toModelOutput( - output.map((match) => ({ + result.matches.map((match) => ({ ...match, entry: { ...match.entry, path: path.resolve(location.directory, match.entry.path) }, })), + result.truncated, ), - metadata: { matches: output.length }, + metadata: { matches: result.matches.length, truncated: result.truncated }, })), Effect.mapError((error) => error instanceof ToolFailure diff --git a/packages/core/test/tool-search.test.ts b/packages/core/test/tool-search.test.ts index b9da2007639..5318e527980 100644 --- a/packages/core/test/tool-search.test.ts +++ b/packages/core/test/tool-search.test.ts @@ -104,7 +104,7 @@ describe("search tools", () => { const grep = yield* executeTool(registry, call("grep", { pattern: "needle" })) expect(glob.metadata).toEqual({ count: FileSystem.DEFAULT_SEARCH_LIMIT, truncated: true }) - expect(grep.metadata).toEqual({ matches: FileSystem.DEFAULT_SEARCH_LIMIT }) + expect(grep.metadata).toEqual({ matches: FileSystem.DEFAULT_SEARCH_LIMIT, truncated: true }) expect(glob.content).toHaveLength(1) expect(grep.content).toHaveLength(1) const globText = glob.content?.[0]?.type === "text" ? glob.content[0].text : "" @@ -114,6 +114,9 @@ describe("search tools", () => { `(Results are truncated: showing first ${FileSystem.DEFAULT_SEARCH_LIMIT} results. Consider using a more specific path or pattern.)`, ) expect(grepText).toStartWith(`Found ${FileSystem.DEFAULT_SEARCH_LIMIT} matches\n`) + expect(grepText).toEndWith( + `(Results are truncated: showing first ${FileSystem.DEFAULT_SEARCH_LIMIT} results. Consider using a more specific path or pattern.)`, + ) }), ) }), @@ -121,6 +124,65 @@ describe("search tools", () => { ), ) + it.live("rejects an empty grep pattern", () => + Effect.acquireUseRelease( + Effect.promise(() => tmpdir()), + (tmp) => + withTools(tmp.path, (registry) => + Effect.gen(function* () { + expect(yield* executeTool(registry, call("grep", { pattern: "" }))).toEqual({ + status: "error", + error: { + type: "tool.execution", + message: 'Invalid tool input: Pattern must not be empty\n at ["pattern"]', + }, + }) + }), + ), + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ), + ) + + it.live("handles explicit grep file and directory paths", () => + Effect.acquireUseRelease( + Effect.promise(() => tmpdir()), + (tmp) => + Effect.promise(() => + Promise.all([ + fs.writeFile(path.join(tmp.path, "target.txt"), "needle\n"), + fs.writeFile(path.join(tmp.path, "other.txt"), "needle\n"), + ]), + ).pipe( + Effect.andThen( + withTools(tmp.path, (registry) => + Effect.gen(function* () { + const file = yield* executeTool(registry, call("grep", { path: "target.txt", pattern: "needle" })) + expect(file).toMatchObject({ + status: "completed", + output: [{ entry: { path: "target.txt" }, line: 1, text: "needle\n" }], + metadata: { matches: 1, truncated: false }, + }) + + const directory = yield* executeTool(registry, call("grep", { path: ".", pattern: "needle" })) + expect(directory).toMatchObject({ + status: "completed", + metadata: { matches: 2, truncated: false }, + }) + if (directory.status !== "completed") return + expect(directory.output).toEqual( + expect.arrayContaining([ + expect.objectContaining({ entry: expect.objectContaining({ path: "target.txt" }) }), + expect.objectContaining({ entry: expect.objectContaining({ path: "other.txt" }) }), + ]), + ) + }), + ), + ), + ), + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ), + ) + for (const name of ["glob", "grep"] as const) { it.live(`${name} reports a missing search path`, () => Effect.acquireUseRelease(