fix(core): harden grep search behavior (#38922)

This commit is contained in:
Aiden Cline 2026-07-25 23:23:53 -05:00 committed by GitHub
parent 79c7e9446e
commit 7affee529b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 81 additions and 9 deletions

View file

@ -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

View file

@ -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(