Skip to content

Commit aede3e7

Browse files
committed
fix: 🐛 Add path and -l support
1 parent b8c5ced commit aede3e7

File tree

4 files changed

+246
-10
lines changed

4 files changed

+246
-10
lines changed

core/tools/definitions/grepSearch.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,28 @@ export const grepSearchTool: Tool = {
2121
query: {
2222
type: "string",
2323
description:
24-
"The search query to use. Must be a valid ripgrep regex expression, escaped where needed",
24+
"The search query to use. Must be a valid ripgrep regex expression. For word boundaries use \\b, for literal parentheses use \\(, for alternations use (pattern1|pattern2). Examples: 'function.*add', '\\bclass\\b', 'add.*\\(', '\\b(def|function)\\b'",
2525
},
2626
args: {
2727
type: "array",
2828
items: { type: "string" },
2929
description:
3030
"Optional additional ripgrep arguments, for example ['-n', '-w']",
3131
},
32+
path: {
33+
type: "string",
34+
description:
35+
"Optional path to search in - can be a specific file (e.g., 'src/main.ts') or directory (e.g., 'src/'). If not provided, searches entire project.",
36+
},
3237
},
3338
},
3439
},
3540
defaultToolPolicy: "allowedWithoutPermission",
3641
systemMessageDescription: {
3742
prefix: `To perform a grep search within the project, call the ${BuiltInToolNames.GrepSearch} tool with the query pattern to match. For example:`,
38-
exampleArgs: [["query", ".*main_services.*"]],
43+
exampleArgs: [
44+
["query", ".*main_services.*"],
45+
["query", "throw new Error"],
46+
],
3947
},
4048
};

core/tools/implementations/grepSearch.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,11 @@ function splitGrepResultsByFile(content: string): ContextItem[] {
4747
export const grepSearchImpl: ToolImpl = async (args, extras) => {
4848
const query = getStringArg(args, "query");
4949
const extraArgs = getOptionalStringArrayArg(args, "args");
50+
const path = args.path as string | undefined;
5051
const ripgrepArgs = buildRipgrepArgs(query, {
5152
extraArgs,
5253
maxResults: DEFAULT_GREP_SEARCH_RESULTS_LIMIT,
54+
path,
5355
});
5456

5557
const results = await extras.ide.getSearchResults(

core/util/grepSearch.ts

Lines changed: 110 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,83 @@ function sanitizeRipgrepArgs(args?: string[]): string[] {
2020
);
2121
}
2222

23+
/**
24+
* Validates and sanitizes a search path for use with ripgrep
25+
* @param searchPath The path to validate (can be file or directory)
26+
* @returns The sanitized path or throws an error if invalid
27+
*/
28+
function validateSearchPath(searchPath: string): string {
29+
// Remove any potentially dangerous characters
30+
const dangerous = /[|;&`$(){}[\]]/;
31+
if (dangerous.test(searchPath)) {
32+
throw new Error(`Invalid characters in search path: ${searchPath}`);
33+
}
34+
35+
// Normalize path separators and remove leading/trailing whitespace
36+
const normalized = searchPath.trim().replace(/\\/g, "/");
37+
38+
// Don't allow absolute paths or path traversal for security
39+
if (
40+
normalized.startsWith("/") ||
41+
normalized.includes("../") ||
42+
normalized.includes("..\\")
43+
) {
44+
throw new Error(
45+
`Absolute paths and path traversal not allowed: ${searchPath}`,
46+
);
47+
}
48+
49+
return normalized;
50+
}
51+
52+
/**
53+
* Validates and potentially fixes common regex pattern issues for ripgrep
54+
* @param query The regex pattern to validate
55+
* @returns The validated pattern or throws an error if invalid
56+
*/
57+
function validateRipgrepPattern(query: string): string {
58+
// Check for common problematic patterns and provide helpful error messages
59+
if (query.includes("\\b") && query.includes("|")) {
60+
// Common issue: complex patterns with word boundaries and alternations
61+
// These should work but may need careful escaping
62+
console.warn(
63+
"Complex pattern detected with word boundaries and alternations. Ensure proper escaping.",
64+
);
65+
}
66+
67+
if (query.match(/\\[^bdswnrtfav\\]/)) {
68+
console.warn(
69+
"Unusual escape sequence detected in pattern. Double-check escaping.",
70+
);
71+
}
72+
73+
return query;
74+
}
75+
2376
export function buildRipgrepArgs(
2477
query: string,
25-
{ extraArgs, maxResults }: { extraArgs?: string[]; maxResults?: number } = {},
78+
{
79+
extraArgs,
80+
maxResults,
81+
path,
82+
}: { extraArgs?: string[]; maxResults?: number; path?: string } = {},
2683
): string[] {
2784
const args = [...DEFAULT_RIPGREP_ARGS];
2885
const sanitized = sanitizeRipgrepArgs(extraArgs);
2986

87+
// Validate the query pattern
88+
const validatedQuery = validateRipgrepPattern(query);
89+
3090
let before = DEFAULT_CONTEXT_BEFORE;
3191
let after = DEFAULT_CONTEXT_AFTER;
3292
const remaining: string[] = [];
3393

3494
for (let i = 0; i < sanitized.length; i++) {
3595
const arg = sanitized[i];
36-
if ((arg === "-A" || arg === "-B" || arg === "-C") && i + 1 < sanitized.length) {
96+
if (
97+
(arg === "-A" || arg === "-B" || arg === "-C") &&
98+
i + 1 < sanitized.length
99+
) {
37100
const val = parseInt(sanitized[i + 1]!, 10);
38101
if (!isNaN(val)) {
39102
if (arg === "-A") {
@@ -64,14 +127,22 @@ export function buildRipgrepArgs(
64127
}
65128

66129
args.push(...remaining);
67-
args.push("-e", query, ".");
130+
131+
// Determine search target (path or current directory)
132+
const searchTarget = path ? validateSearchPath(path) : ".";
133+
args.push("-e", validatedQuery, searchTarget);
68134
return args;
69135
}
70136

71137
/*
72138
Formats the output of a grep search to reduce unnecessary indentation, lines, etc
73-
Assumes a command with these params
139+
Handles both standard ripgrep output with --heading and simple file lists (e.g., with -l flag)
140+
141+
Standard format:
74142
ripgrep -i --ignore-file .continueignore --ignore-file .gitignore -C 2 --heading -m 100 -e <query> .
143+
144+
File list format (with -l flag):
145+
ripgrep -l -i --ignore-file .continueignore --ignore-file .gitignore -e <query> .
75146
76147
Also can truncate the output to a specified number of characters
77148
*/
@@ -86,15 +157,48 @@ export function formatGrepSearchResults(
86157
let numResults = 0;
87158
const keepLines: string[] = [];
88159

160+
// Check if this looks like a simple file list (all lines start with ./ and no content)
161+
const lines = results.split("\n").filter((l) => !!l);
162+
const isFileListOnly =
163+
lines.length > 0 &&
164+
lines.every((line) => line.startsWith("./") || line === "No matches found");
165+
166+
if (isFileListOnly) {
167+
// Handle simple file list output (e.g., from -l flag)
168+
const fileLines = lines.filter((line) => line.startsWith("./"));
169+
numResults = fileLines.length;
170+
const formatted = fileLines.join("\n");
171+
172+
if (maxChars && formatted.length > maxChars) {
173+
return {
174+
formatted: formatted.substring(0, maxChars),
175+
numResults,
176+
truncated: true,
177+
};
178+
} else {
179+
return {
180+
formatted,
181+
numResults,
182+
truncated: false,
183+
};
184+
}
185+
}
186+
187+
// Handle standard format with content
89188
function countLeadingSpaces(line: string) {
90189
return line?.match(/^ */)?.[0].length ?? 0;
91190
}
92191

93192
const processResult = (lines: string[]) => {
94-
// Skip results in which only the file path was kept
193+
// Handle file path lines
95194
const resultPath = lines[0];
96195
const resultContent = lines.slice(1);
196+
197+
// For file-only results (like with -l), still include the path
97198
if (resultContent.length === 0) {
199+
if (resultPath && resultPath.startsWith("./")) {
200+
keepLines.push(resultPath);
201+
}
98202
return;
99203
}
100204

@@ -126,7 +230,7 @@ export function formatGrepSearchResults(
126230
};
127231

128232
let resultLines: string[] = [];
129-
for (const line of results.split("\n").filter((l) => !!l)) {
233+
for (const line of lines) {
130234
if (line.startsWith("./") || line === "--") {
131235
processResult(resultLines); // process previous result
132236
resultLines = [line];

core/util/grepSearch.vitest.ts

Lines changed: 124 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,16 @@ test("handles empty input", () => {
118118
expect(result.formatted).toBe("");
119119
});
120120

121-
test("handles input with only file paths (no content) - lines should be skipped", () => {
121+
test("handles input with only file paths (no content) - now includes file paths", () => {
122122
const input =
123123
"./empty-file.ts\n./another-empty.js\n./has-content.ts\n searchMatch";
124124
const result = formatGrepSearchResults(input);
125125

126126
expect(result.numResults).toBe(3);
127-
expect(result.formatted).toBe("./has-content.ts\n searchMatch");
127+
// Now includes all file paths, including empty ones, plus content for files that have matches
128+
expect(result.formatted).toBe(
129+
"./empty-file.ts\n./another-empty.js\n./has-content.ts\n searchMatch",
130+
);
128131
});
129132

130133
test("truncates output when exceeding maxChars", () => {
@@ -302,3 +305,122 @@ test("buildRipgrepArgs merges context flags", () => {
302305
".",
303306
]);
304307
});
308+
309+
// Tests for -l flag handling
310+
test("handles -l flag output (files only)", () => {
311+
const fileListOutput = `./src/main.ts
312+
./src/utils.ts
313+
./tests/main.test.ts`;
314+
315+
const result = formatGrepSearchResults(fileListOutput);
316+
317+
expect(result.numResults).toBe(3);
318+
expect(result.truncated).toBe(false);
319+
expect(result.formatted).toBe(fileListOutput);
320+
});
321+
322+
test("handles empty -l flag output", () => {
323+
const result = formatGrepSearchResults("");
324+
325+
expect(result.numResults).toBe(0);
326+
expect(result.truncated).toBe(false);
327+
expect(result.formatted).toBe("");
328+
});
329+
330+
test("handles no matches found message", () => {
331+
const noMatchesOutput = "No matches found";
332+
333+
const result = formatGrepSearchResults(noMatchesOutput);
334+
335+
expect(result.numResults).toBe(0);
336+
expect(result.truncated).toBe(false);
337+
expect(result.formatted).toBe("");
338+
});
339+
340+
test("buildRipgrepArgs handles -l flag correctly", () => {
341+
const argsWithL = buildRipgrepArgs("test", { extraArgs: ["-l"] });
342+
expect(argsWithL).toContain("-l");
343+
expect(argsWithL).toEqual([
344+
"-i",
345+
"--ignore-file",
346+
".continueignore",
347+
"--ignore-file",
348+
".gitignore",
349+
"-C",
350+
"2",
351+
"--heading",
352+
"-l",
353+
"-e",
354+
"test",
355+
".",
356+
]);
357+
});
358+
359+
// Tests for complex regex patterns
360+
test("buildRipgrepArgs handles complex regex patterns", () => {
361+
const complexPattern = "\\b(def|function).*\\badd\\b.*\\(";
362+
const args = buildRipgrepArgs(complexPattern);
363+
364+
expect(args).toContain("-e");
365+
const eIndex = args.indexOf("-e");
366+
expect(args[eIndex + 1]).toBe(complexPattern);
367+
});
368+
369+
test("buildRipgrepArgs handles word boundaries", () => {
370+
const wordBoundaryPattern = "\\bfunction\\b";
371+
const args = buildRipgrepArgs(wordBoundaryPattern);
372+
373+
expect(args).toContain("-e");
374+
const eIndex = args.indexOf("-e");
375+
expect(args[eIndex + 1]).toBe(wordBoundaryPattern);
376+
});
377+
378+
test("buildRipgrepArgs handles parentheses in patterns", () => {
379+
const parenPattern = "add.*\\(";
380+
const args = buildRipgrepArgs(parenPattern);
381+
382+
expect(args).toContain("-e");
383+
const eIndex = args.indexOf("-e");
384+
expect(args[eIndex + 1]).toBe(parenPattern);
385+
});
386+
387+
// Tests for path parameter
388+
test("buildRipgrepArgs handles file path", () => {
389+
const args = buildRipgrepArgs("test", { path: "src/main.ts" });
390+
391+
expect(args).toContain("src/main.ts");
392+
expect(args[args.length - 1]).toBe("src/main.ts");
393+
});
394+
395+
test("buildRipgrepArgs handles directory path", () => {
396+
const args = buildRipgrepArgs("test", { path: "src/" });
397+
398+
expect(args).toContain("src/");
399+
expect(args[args.length - 1]).toBe("src/");
400+
});
401+
402+
test("buildRipgrepArgs defaults to current directory when no path specified", () => {
403+
const args = buildRipgrepArgs("test");
404+
405+
expect(args[args.length - 1]).toBe(".");
406+
});
407+
408+
test("buildRipgrepArgs rejects dangerous paths", () => {
409+
expect(() => {
410+
buildRipgrepArgs("test", { path: "/etc/passwd" });
411+
}).toThrow("Absolute paths and path traversal not allowed");
412+
413+
expect(() => {
414+
buildRipgrepArgs("test", { path: "../../../etc/passwd" });
415+
}).toThrow("Absolute paths and path traversal not allowed");
416+
417+
expect(() => {
418+
buildRipgrepArgs("test", { path: "file;rm -rf /" });
419+
}).toThrow("Invalid characters in search path");
420+
});
421+
422+
test("buildRipgrepArgs normalizes paths", () => {
423+
const args = buildRipgrepArgs("test", { path: " src\\main.ts " });
424+
425+
expect(args[args.length - 1]).toBe("src/main.ts");
426+
});

0 commit comments

Comments
 (0)