From c5666e153ffba0bfdb8eabda9a39a1a2fea77df1 Mon Sep 17 00:00:00 2001 From: can1357 Date: Fri, 24 Apr 2026 05:49:10 +0200 Subject: [PATCH] feat(grep): route comma-separated explicit files to exact-file grep resolveMultiSearchPath now reports `exactFilePaths` when every token resolves to a plain file (no globs, no suffix glob) and accepts a single resolvable token so partially-missing lists still search the resolvable subset. grep iterates those exact files individually instead of collapsing them into a brace-union glob, which preserves the user's explicit file set even when siblings share a basename. Also adds a small `[grep] match lines use ':'; context lines use '-'` banner when context lines are rendered, and splits the per-file rendering helpers so files with no remaining matches no longer emit empty headers. --- packages/coding-agent/src/tools/grep.ts | 131 +++++++++++++----- packages/coding-agent/src/tools/path-utils.ts | 34 ++++- .../test/tools/search-path-lists.test.ts | 75 ++++++++++ 3 files changed, 200 insertions(+), 40 deletions(-) diff --git a/packages/coding-agent/src/tools/grep.ts b/packages/coding-agent/src/tools/grep.ts index f4501aafd..55b00d06e 100644 --- a/packages/coding-agent/src/tools/grep.ts +++ b/packages/coding-agent/src/tools/grep.ts @@ -124,6 +124,7 @@ export class GrepTool implements AgentTool { }; let searchPath: string; let scopePath: string; + let exactFilePaths: string[] | undefined; let globFilter = glob ? normalizePathLikeInput(glob) || undefined : undefined; const internalRouter = this.session.internalRouter; if (searchDir?.trim()) { @@ -142,7 +143,8 @@ export class GrepTool implements AgentTool { const multiSearchPath = await resolveMultiSearchPath(rawPath, this.session.cwd, globFilter); if (multiSearchPath) { searchPath = multiSearchPath.basePath; - globFilter = multiSearchPath.glob; + globFilter = multiSearchPath.exactFilePaths ? undefined : multiSearchPath.glob; + exactFilePaths = multiSearchPath.exactFilePaths; scopePath = multiSearchPath.scopePath; } else { const parsedPath = parseSearchPath(rawPath); @@ -174,26 +176,61 @@ export class GrepTool implements AgentTool { // Run grep let result: GrepResult; try { - result = await grep( - { - pattern: normalizedPattern, - path: searchPath, - glob: globFilter, - type: type?.trim() || undefined, - ignoreCase, - multiline: effectiveMultiline, - hidden: true, - gitignore: useGitignore, - cache: false, - maxCount: internalLimit, - offset: normalizedOffset > 0 ? normalizedOffset : undefined, - contextBefore: normalizedContextBefore, - contextAfter: normalizedContextAfter, - maxColumns: DEFAULT_MAX_COLUMN, - mode: effectiveOutputMode, - }, - undefined, - ); + if (exactFilePaths) { + const matches: GrepMatch[] = []; + let limitReached = false; + for (const exactFilePath of exactFilePaths) { + const fileResult = await grep( + { + pattern: normalizedPattern, + path: exactFilePath, + type: type?.trim() || undefined, + ignoreCase, + multiline: effectiveMultiline, + hidden: true, + gitignore: useGitignore, + cache: false, + contextBefore: normalizedContextBefore, + contextAfter: normalizedContextAfter, + maxColumns: DEFAULT_MAX_COLUMN, + mode: effectiveOutputMode, + }, + undefined, + ); + limitReached = limitReached || Boolean(fileResult.limitReached); + const relativeFilePath = path.relative(searchPath, exactFilePath).replace(/\\/g, "/"); + matches.push(...fileResult.matches.map(match => ({ ...match, path: relativeFilePath }))); + } + const offsetMatches = matches.slice(normalizedOffset); + result = { + matches: offsetMatches, + totalMatches: offsetMatches.length, + filesWithMatches: new Set(offsetMatches.map(match => match.path)).size, + filesSearched: exactFilePaths.length, + limitReached, + }; + } else { + result = await grep( + { + pattern: normalizedPattern, + path: searchPath, + glob: globFilter, + type: type?.trim() || undefined, + ignoreCase, + multiline: effectiveMultiline, + hidden: true, + gitignore: useGitignore, + cache: false, + maxCount: internalLimit, + offset: normalizedOffset > 0 ? normalizedOffset : undefined, + contextBefore: normalizedContextBefore, + contextAfter: normalizedContextAfter, + maxColumns: DEFAULT_MAX_COLUMN, + mode: effectiveOutputMode, + }, + undefined, + ); + } } catch (err) { if (err instanceof Error && err.message.startsWith("regex parse error")) { throw new ToolError(err.message); @@ -258,6 +295,7 @@ export class GrepTool implements AgentTool { } const outputLines: string[] = []; let linesTruncated = false; + const hasContextLines = normalizedContextBefore > 0 || normalizedContextAfter > 0; const matchesByFile = new Map(); for (const match of selectedMatches) { const relativePath = formatPath(match.path); @@ -289,10 +327,11 @@ export class GrepTool implements AgentTool { } chunkMatchesByFile.get(match.displayPath)!.push(match); } - const renderChunkedMatchesForFile = (relativePath: string) => { + const renderChunkedMatchesForFile = (relativePath: string): string[] => { + const renderedLines: string[] = []; const fileMatches = chunkMatchesByFile.get(relativePath) ?? []; if (fileMatches.length === 0) { - return; + return renderedLines; } const lineWidth = fileMatches[0]?.fileLineCount.toString().length ?? 1; const matchesByChunk = new Map(); @@ -310,13 +349,14 @@ export class GrepTool implements AgentTool { const anchor = chunkChecksum ? `${dashes}@${chunkPath}#${chunkChecksum}` : `${dashes}@${chunkPath}`; - outputLines.push(anchor); + renderedLines.push(anchor); } for (const match of chunkMatches) { - outputLines.push(` ${match.lineNumber.toString().padStart(lineWidth, " ")} |${match.line}`); + renderedLines.push(` ${match.lineNumber.toString().padStart(lineWidth, " ")} |${match.line}`); fileMatchCounts.set(relativePath, (fileMatchCounts.get(relativePath) ?? 0) + 1); } } + return renderedLines; }; if (isDirectory) { const filesByDirectory = new Map(); @@ -330,26 +370,32 @@ export class GrepTool implements AgentTool { for (const [directory, directoryFiles] of filesByDirectory) { if (directory === ".") { for (const relativePath of directoryFiles) { + const renderedLines = renderChunkedMatchesForFile(relativePath); + if (renderedLines.length === 0) continue; if (outputLines.length > 0) { outputLines.push(""); } outputLines.push(`# ${path.basename(relativePath)}`); - renderChunkedMatchesForFile(relativePath); + outputLines.push(...renderedLines); } continue; } + const renderedFiles = directoryFiles + .map(relativePath => ({ relativePath, lines: renderChunkedMatchesForFile(relativePath) })) + .filter(file => file.lines.length > 0); + if (renderedFiles.length === 0) continue; if (outputLines.length > 0) { outputLines.push(""); } outputLines.push(`# ${directory}`); - for (const relativePath of directoryFiles) { + for (const { relativePath, lines } of renderedFiles) { outputLines.push(`## └─ ${path.basename(relativePath)}`); - renderChunkedMatchesForFile(relativePath); + outputLines.push(...lines); } } } else { for (const relativePath of fileList) { - renderChunkedMatchesForFile(relativePath); + outputLines.push(...renderChunkedMatchesForFile(relativePath)); } } const rawOutput = outputLines.join("\n"); @@ -380,7 +426,8 @@ export class GrepTool implements AgentTool { } return resultBuilder.done(); } - const renderMatchesForFile = (relativePath: string) => { + const renderMatchesForFile = (relativePath: string): string[] => { + const renderedLines: string[] = []; const fileMatches = matchesByFile.get(relativePath) ?? []; for (const match of fileMatches) { const lineNumbers: number[] = [match.lineNumber]; @@ -399,20 +446,21 @@ export class GrepTool implements AgentTool { formatMatchLine(lineNumber, line, isMatch, { useHashLines, lineWidth }); if (match.contextBefore) { for (const ctx of match.contextBefore) { - outputLines.push(formatLine(ctx.lineNumber, ctx.line, false)); + renderedLines.push(formatLine(ctx.lineNumber, ctx.line, false)); } } - outputLines.push(formatLine(match.lineNumber, match.line, true)); + renderedLines.push(formatLine(match.lineNumber, match.line, true)); if (match.truncated) { linesTruncated = true; } if (match.contextAfter) { for (const ctx of match.contextAfter) { - outputLines.push(formatLine(ctx.lineNumber, ctx.line, false)); + renderedLines.push(formatLine(ctx.lineNumber, ctx.line, false)); } } fileMatchCounts.set(relativePath, (fileMatchCounts.get(relativePath) ?? 0) + 1); } + return renderedLines; }; if (isDirectory) { const filesByDirectory = new Map(); @@ -426,28 +474,37 @@ export class GrepTool implements AgentTool { for (const [directory, directoryFiles] of filesByDirectory) { if (directory === ".") { for (const relativePath of directoryFiles) { + const renderedLines = renderMatchesForFile(relativePath); + if (renderedLines.length === 0) continue; if (outputLines.length > 0) { outputLines.push(""); } outputLines.push(`# ${path.basename(relativePath)}`); - renderMatchesForFile(relativePath); + outputLines.push(...renderedLines); } continue; } + const renderedFiles = directoryFiles + .map(relativePath => ({ relativePath, lines: renderMatchesForFile(relativePath) })) + .filter(file => file.lines.length > 0); + if (renderedFiles.length === 0) continue; if (outputLines.length > 0) { outputLines.push(""); } outputLines.push(`# ${directory}`); - for (const relativePath of directoryFiles) { + for (const { relativePath, lines } of renderedFiles) { outputLines.push(`## └─ ${path.basename(relativePath)}`); - renderMatchesForFile(relativePath); + outputLines.push(...lines); } } } else { for (const relativePath of fileList) { - renderMatchesForFile(relativePath); + outputLines.push(...renderMatchesForFile(relativePath)); } } + if (hasContextLines && outputLines.length > 0) { + outputLines.unshift("[grep] match lines use ':'; context lines use '-'."); + } const rawOutput = outputLines.join("\n"); const truncation = truncateHead(rawOutput, { maxLines: Number.MAX_SAFE_INTEGER }); const output = truncation.content; diff --git a/packages/coding-agent/src/tools/path-utils.ts b/packages/coding-agent/src/tools/path-utils.ts index 42c2720c0..191d07f81 100644 --- a/packages/coding-agent/src/tools/path-utils.ts +++ b/packages/coding-agent/src/tools/path-utils.ts @@ -193,6 +193,7 @@ export interface ResolvedMultiSearchPath { basePath: string; glob?: string; scopePath: string; + exactFilePaths?: string[]; } export interface ResolvedMultiFindPattern { @@ -438,6 +439,28 @@ async function areDelimitedTokensResolvable( return true; } +async function filterResolvableTokens( + tokens: string[], + cwd: string, + parseBasePath: (value: string) => string, +): Promise { + const out: string[] = []; + for (const token of tokens) { + if (TOP_LEVEL_INTERNAL_URL_PREFIXES.some(prefix => token.startsWith(prefix))) continue; + const basePath = parseBasePath(token); + const resolvedBasePath = resolveToCwd(basePath, cwd); + if (await pathExists(resolvedBasePath)) { + out.push(token); + continue; + } + const resolvedExactPath = resolveToCwd(token, cwd); + if (await pathExists(resolvedExactPath)) { + out.push(token); + } + } + return out; +} + async function splitDelimitedSearchInput( rawInput: string, cwd: string, @@ -452,8 +475,11 @@ async function splitDelimitedSearchInput( } const commaSeparated = splitTopLevel(trimmed, "comma"); - if (commaSeparated.length > 1 && (await areDelimitedTokensResolvable(commaSeparated, cwd, parseBasePath, true))) { - return [...new Set(commaSeparated)]; + if (commaSeparated.length > 1) { + const resolvable = await filterResolvableTokens(commaSeparated, cwd, parseBasePath); + if (resolvable.length >= 1) { + return [...new Set(resolvable)]; + } } const whitespaceSeparated = splitTopLevel(trimmed, "whitespace"); @@ -473,7 +499,7 @@ export async function resolveMultiSearchPath( suffixGlob?: string, ): Promise { const pathItems = await splitDelimitedSearchInput(rawPath, cwd, value => parseSearchPath(value).basePath); - if (!pathItems || pathItems.length <= 1) { + if (!pathItems || pathItems.length < 1) { return undefined; } @@ -486,6 +512,7 @@ export async function resolveMultiSearchPath( }), ); + const allExactFiles = !suffixGlob && parsedItems.every(item => !item.parsedPath.glob && item.stat.isFile()); const commonBasePath = findCommonBasePath(parsedItems.map(item => item.absoluteBasePath)); const combinedPatterns = parsedItems.map(item => { const relativeBasePath = normalizePosixPath(path.relative(commonBasePath, item.absoluteBasePath)) || "."; @@ -507,6 +534,7 @@ export async function resolveMultiSearchPath( basePath: commonBasePath, glob: buildBraceUnion(combinedPatterns), scopePath: toScopeDisplay(pathItems), + exactFilePaths: allExactFiles ? parsedItems.map(item => item.absoluteBasePath) : undefined, }; } diff --git a/packages/coding-agent/test/tools/search-path-lists.test.ts b/packages/coding-agent/test/tools/search-path-lists.test.ts index fb985c36d..fb219517a 100644 --- a/packages/coding-agent/test/tools/search-path-lists.test.ts +++ b/packages/coding-agent/test/tools/search-path-lists.test.ts @@ -281,4 +281,79 @@ describe("search tool path lists", () => { expect(details?.fileCount).toBe(3); expect(details?.scopePath).toBe("apps, packages, phases"); }); + + it("grep keeps comma-separated explicit files exact", async () => { + await fs.mkdir(path.join(tempDir, "nested"), { recursive: true }); + await Bun.write(path.join(tempDir, "alpha.txt"), "exact-needle alpha\n"); + await Bun.write(path.join(tempDir, "beta.txt"), "exact-needle beta\n"); + await Bun.write(path.join(tempDir, "nested", "alpha.txt"), "exact-needle nested alpha\n"); + await Bun.write(path.join(tempDir, "nested", "beta.txt"), "exact-needle nested beta\n"); + + const tools = await createTools(createTestSession(tempDir)); + const tool = tools.find(entry => entry.name === "grep"); + expect(tool).toBeDefined(); + if (!tool) throw new Error("Missing grep tool"); + + const result = await tool.execute("grep-exact-comma-files", { + pattern: "exact-needle", + path: "alpha.txt,beta.txt", + }); + const text = getText(result); + const details = result.details as { fileCount?: number; scopePath?: string } | undefined; + + expect(text).toContain("# alpha.txt"); + expect(text).toContain("# beta.txt"); + expect(text).toContain("exact-needle alpha"); + expect(text).toContain("exact-needle beta"); + expect(text).not.toContain("nested"); + expect(details?.fileCount).toBe(2); + expect(details?.scopePath).toBe("alpha.txt, beta.txt"); + }); + + it("grep renders only file headings that have child lines", async () => { + const tools = await createTools(createTestSession(tempDir)); + const tool = tools.find(entry => entry.name === "grep"); + expect(tool).toBeDefined(); + if (!tool) throw new Error("Missing grep tool"); + + const result = await tool.execute("grep-no-empty-headings", { + pattern: "shared-needle", + path: "apps/,packages/,phases/", + limit: 2, + }); + const lines = getText(result).split("\n"); + + for (let index = 0; index < lines.length; index += 1) { + if (!lines[index].startsWith("#")) continue; + const nextIndex = lines.findIndex((line, candidateIndex) => candidateIndex > index && line.trim().length > 0); + expect(nextIndex, `heading ${lines[index]} should have rendered children`).toBeGreaterThan(index); + if (lines[index].startsWith("##")) { + expect(lines[nextIndex].startsWith("#")).toBe(false); + } else if (!lines[nextIndex].startsWith("##")) { + expect(lines[nextIndex].startsWith("#")).toBe(false); + } + } + }); + + it("grep explains context-line gutters without changing match and context separators", async () => { + await Bun.write(path.join(tempDir, "context.txt"), "#if FLAG\nneedle\n#endif\n"); + + const tools = await createTools(createTestSession(tempDir)); + const tool = tools.find(entry => entry.name === "grep"); + expect(tool).toBeDefined(); + if (!tool) throw new Error("Missing grep tool"); + + const result = await tool.execute("grep-context-label", { + pattern: "needle", + path: "context.txt", + pre: 1, + post: 1, + }); + const text = getText(result); + + expect(text).toContain("match lines use ':'; context lines use '-'"); + expect(text).toMatch(/1(?:#[A-Z0-9]+)?-#if FLAG/); + expect(text).toMatch(/2(?:#[A-Z0-9]+)?:needle/); + expect(text).toMatch(/3(?:#[A-Z0-9]+)?-#endif/); + }); });