diff --git a/packages/core/src/tools/grep.test.ts b/packages/core/src/tools/grep.test.ts index 860169c02d9..499a1c1247e 100644 --- a/packages/core/src/tools/grep.test.ts +++ b/packages/core/src/tools/grep.test.ts @@ -45,6 +45,7 @@ describe('GrepTool', () => { let mockConfig: Config; beforeEach(async () => { + vi.mocked(execStreaming).mockReset(); tempRootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'grep-tool-root-')); mockConfig = { @@ -155,12 +156,10 @@ describe('GrepTool', () => { expect(grepTool.validateToolParams(params)).toContain('nonexistent'); }); - it('should return error if path is a file, not a directory', async () => { + it('should allow path to be a file', async () => { const filePath = path.join(tempRootDir, 'fileA.txt'); const params: GrepToolParams = { pattern: 'hello', dir_path: filePath }; - expect(grepTool.validateToolParams(params)).toContain( - `Path is not a directory: ${filePath}`, - ); + expect(grepTool.validateToolParams(params)).toBeNull(); }); }); @@ -342,6 +341,39 @@ describe('GrepTool', () => { ); }); + it('should search an explicit file path with git grep', async () => { + await fs.mkdir(path.join(tempRootDir, '.git')); + vi.mocked(execStreaming).mockImplementationOnce(() => + createLineGenerator(['fileA.txt:1:hello world']), + ); + + const invocation = grepTool.build({ + pattern: 'hello', + dir_path: 'fileA.txt', + }) as unknown as { + isCommandAvailable: (command: string) => Promise; + execute: (options: ExecuteOptions) => Promise; + }; + invocation.isCommandAvailable = vi.fn( + async (command: string) => command === 'git', + ); + + const result = await invocation.execute({ abortSignal }); + + expect(execStreaming).toHaveBeenCalledWith( + 'git', + expect.arrayContaining(['grep', '--', 'fileA.txt']), + expect.objectContaining({ + cwd: tempRootDir, + }), + ); + const gitArgs = vi.mocked(execStreaming).mock.calls[0][1]; + expect(gitArgs).not.toContain('--json'); + expect(result.llmContent).toContain('Found 1 match'); + expect(result.llmContent).toContain('File: fileA.txt'); + expect(result.llmContent).toContain('L1: hello world'); + }); + it('should throw an error if params are invalid', async () => { const params = { dir_path: '.' } as unknown as GrepToolParams; // Invalid: pattern missing expect(() => grepTool.build(params)).toThrow( diff --git a/packages/core/src/tools/grep.ts b/packages/core/src/tools/grep.ts index 8d1804886f0..2965275e1a4 100644 --- a/packages/core/src/tools/grep.ts +++ b/packages/core/src/tools/grep.ts @@ -145,6 +145,7 @@ class GrepToolInvocation extends BaseToolInvocation< const pathParam = this.params.dir_path; let searchDirAbs: string | null = null; + let searchDirStats: fs.Stats | null = null; if (pathParam) { searchDirAbs = path.resolve(this.config.getTargetDir(), pathParam); const validationError = this.config.validatePathAccess( @@ -163,13 +164,14 @@ class GrepToolInvocation extends BaseToolInvocation< } try { - const stats = await fsPromises.stat(searchDirAbs); - if (!stats.isDirectory()) { + searchDirStats = await fsPromises.stat(searchDirAbs); + if (!searchDirStats.isDirectory() && !searchDirStats.isFile()) { + const message = `Path is not a valid directory or file: ${searchDirAbs}`; return { - llmContent: `Path is not a directory: ${searchDirAbs}`, - returnDisplay: 'Error: Path is not a directory.', + llmContent: message, + returnDisplay: 'Error: Path is not a valid directory or file.', error: { - message: `Path is not a directory: ${searchDirAbs}`, + message, type: ToolErrorType.PATH_IS_NOT_A_DIRECTORY, }, }; @@ -200,13 +202,19 @@ class GrepToolInvocation extends BaseToolInvocation< const searchDirDisplay = pathParam || '.'; // Determine which directories to search - let searchDirectories: readonly string[]; + let searchTargets: ReadonlyArray<{ + directory: string; + filePath?: string; + }>; if (searchDirAbs === null) { // No path specified - search all workspace directories - searchDirectories = workspaceContext.getDirectories(); + searchTargets = workspaceContext + .getDirectories() + .map((directory) => ({ directory })); } else { - // Specific path provided - search only that directory - searchDirectories = [searchDirAbs]; + searchTargets = searchDirStats?.isFile() + ? [{ directory: path.dirname(searchDirAbs), filePath: searchDirAbs }] + : [{ directory: searchDirAbs }]; } // Collect matches from all search directories @@ -237,13 +245,14 @@ class GrepToolInvocation extends BaseToolInvocation< } try { - for (const searchDir of searchDirectories) { + for (const searchTarget of searchTargets) { const remainingLimit = totalMaxMatches - allMatches.length; if (remainingLimit <= 0) break; const matches = await this.performGrepSearch({ pattern: this.params.pattern, - path: searchDir, + path: searchTarget.directory, + filePath: searchTarget.filePath, include_pattern: this.params.include_pattern, exclude_pattern: this.params.exclude_pattern, maxMatches: remainingLimit, @@ -252,8 +261,8 @@ class GrepToolInvocation extends BaseToolInvocation< }); // Add directory prefix if searching multiple directories - if (searchDirectories.length > 1) { - const dirName = path.basename(searchDir); + if (searchTargets.length > 1) { + const dirName = path.basename(searchTarget.directory); matches.forEach((match) => { match.filePath = path.join(dirName, match.filePath); }); @@ -395,6 +404,7 @@ class GrepToolInvocation extends BaseToolInvocation< private async performGrepSearch(options: { pattern: string; path: string; // Expects absolute path + filePath?: string; include_pattern?: string; exclude_pattern?: string; maxMatches: number; @@ -404,6 +414,7 @@ class GrepToolInvocation extends BaseToolInvocation< const { pattern, path: absolutePath, + filePath, include_pattern, exclude_pattern, maxMatches, @@ -434,8 +445,13 @@ class GrepToolInvocation extends BaseToolInvocation< if (max_matches_per_file) { gitArgs.push('--max-count', max_matches_per_file.toString()); } - if (include_pattern) { - gitArgs.push('--', include_pattern); + const pathspecs = filePath + ? [path.relative(absolutePath, filePath)] + : include_pattern + ? [include_pattern] + : []; + if (pathspecs.length > 0) { + gitArgs.push('--', ...pathspecs); } try { @@ -503,11 +519,11 @@ class GrepToolInvocation extends BaseToolInvocation< if (max_matches_per_file) { grepArgs.push('--max-count', max_matches_per_file.toString()); } - if (include_pattern) { + if (include_pattern && !filePath) { grepArgs.push(`--include=${include_pattern}`); } grepArgs.push(pattern); - grepArgs.push('.'); + grepArgs.push(filePath ? path.relative(absolutePath, filePath) : '.'); const results: GrepMatch[] = []; try { @@ -554,14 +570,16 @@ class GrepToolInvocation extends BaseToolInvocation< const globPattern = include_pattern ? include_pattern : '**/*'; const ignorePatterns = this.fileExclusions.getGlobExcludes(); - const filesStream = globStream(globPattern, { - cwd: absolutePath, - dot: true, - ignore: ignorePatterns, - absolute: true, - nodir: true, - signal: options.signal, - }); + const filesStream = filePath + ? [filePath] + : globStream(globPattern, { + cwd: absolutePath, + dot: true, + ignore: ignorePatterns, + absolute: true, + nodir: true, + signal: options.signal, + }); const regex = new RegExp(pattern, 'i'); const allMatches: GrepMatch[] = []; @@ -734,11 +752,11 @@ export class GrepTool extends BaseDeclarativeTool { return validationError; } - // We still want to check if it's a directory + // We still want to check if it points to searchable filesystem content. try { const stats = fs.statSync(resolvedPath); - if (!stats.isDirectory()) { - return `Path is not a directory: ${resolvedPath}`; + if (!stats.isDirectory() && !stats.isFile()) { + return `Path is not a valid directory or file: ${resolvedPath}`; } } catch (error: unknown) { if (isNodeError(error) && error.code === 'ENOENT') { diff --git a/packages/core/src/tools/ripGrep.test.ts b/packages/core/src/tools/ripGrep.test.ts index d46b257ba07..6d20833fea5 100644 --- a/packages/core/src/tools/ripGrep.test.ts +++ b/packages/core/src/tools/ripGrep.test.ts @@ -64,11 +64,12 @@ const mockSpawn = vi.mocked(spawn); function createMockSpawn( options: { outputData?: string; + errorData?: string; exitCode?: number | null; signal?: string; } = {}, ) { - const { outputData, exitCode = 0, signal } = options; + const { outputData, errorData, exitCode = 0, signal } = options; return () => { // strict Readable implementation @@ -97,6 +98,11 @@ function createMockSpawn( // Emulating process exit setTimeout(() => { + if (errorData) { + stderr.end(errorData); + } else { + stderr.end(); + } mockProcess.emit('close', exitCode, signal); }, 10); @@ -125,6 +131,9 @@ function createMockConfig( getFileFilteringRespectGeminiIgnore(this: Config) { return this.getFileFilteringOptions().respectGeminiIgnore; }, + getFileExclusions: () => ({ + getGlobExcludes: () => [], + }), storage: { getProjectTempDir: vi.fn().mockReturnValue('/tmp/project'), }, @@ -684,18 +693,61 @@ describe('RipGrepTool', () => { ); }); - it('should throw an error if ripgrep is not available', async () => { + it('should fall back when ripgrep is not available', async () => { vi.mocked(mockConfig.getRipgrepPath).mockResolvedValue(null); + mockSpawn.mockImplementation(createMockSpawn({ exitCode: 1 })); const params: RipGrepToolParams = { pattern: 'world' }; const invocation = grepTool.build(params); const result = await invocation.execute({ abortSignal }); - expect(result.llmContent).toContain('Cannot find bundled ripgrep binary'); + expect(result.llmContent).toContain( + 'Found 3 matches for pattern "world"', + ); + expect(result.llmContent).toContain('File: fileA.txt'); + expect(result.llmContent).toContain( + `File: ${path.join('sub', 'fileC.txt')}`, + ); // restore the mock for subsequent tests vi.mocked(mockConfig.getRipgrepPath).mockResolvedValue('/mock/rg'); }); + + it('should fall back to git grep without ripgrep-only arguments when rg fails at runtime', async () => { + await fs.mkdir(path.join(tempRootDir, '.git')); + mockSpawn + .mockImplementationOnce( + createMockSpawn({ + errorData: 'error: unknown flag `json`', + exitCode: 64, + }), + ) + .mockImplementationOnce(createMockSpawn()) + .mockImplementationOnce( + createMockSpawn({ + outputData: 'fileA.txt:1:hello world\n', + }), + ); + + const invocation = grepTool.build({ + pattern: 'hello', + dir_path: 'fileA.txt', + }); + const result = await invocation.execute({ abortSignal }); + + const gitCall = mockSpawn.mock.calls.find( + ([command, args]) => + command === 'git' && Array.isArray(args) && args.includes('grep'), + ); + expect(gitCall).toBeDefined(); + expect(gitCall?.[1]).not.toContain('--json'); + expect(gitCall?.[1]).toEqual( + expect.arrayContaining(['grep', '--', 'fileA.txt']), + ); + expect(result.llmContent).toContain('Found 1 match'); + expect(result.llmContent).toContain('File: fileA.txt'); + expect(result.llmContent).toContain('L1: hello world'); + }); }); describe('multi-directory workspace', () => { diff --git a/packages/core/src/tools/ripGrep.ts b/packages/core/src/tools/ripGrep.ts index 214df804f09..010683c3c75 100644 --- a/packages/core/src/tools/ripGrep.ts +++ b/packages/core/src/tools/ripGrep.ts @@ -36,6 +36,7 @@ import { } from '../utils/ignorePatterns.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; import { execStreaming, resolveExecutable } from '../utils/shell-utils.js'; +import { GrepTool } from './grep.js'; import { DEFAULT_TOTAL_MAX_MATCHES, DEFAULT_SEARCH_TIMEOUT_MS, @@ -280,6 +281,15 @@ class GrepToolInvocation extends BaseToolInvocation< `Operation timed out after ${timeoutMs}ms. In large repositories, consider narrowing your search scope by specifying a 'dir_path' or an 'include_pattern'.`, ); } + const fallbackResult = await this.executeGrepFallback( + pathParam, + totalMaxMatches, + signal, + error, + ); + if (fallbackResult) { + return fallbackResult; + } throw error; } finally { clearTimeout(timeoutId); @@ -391,6 +401,52 @@ class GrepToolInvocation extends BaseToolInvocation< return allMatches; } + private async executeGrepFallback( + pathParam: string, + totalMaxMatches: number, + signal: AbortSignal, + error: unknown, + ): Promise { + const errorMessage = getErrorMessage(error); + if ( + !( + errorMessage.includes('Cannot find bundled ripgrep binary') || + /Process exited with code (64|126|127)\b/.test(errorMessage) || + /\b(EACCES|ENOENT)\b/.test(errorMessage) + ) + ) { + return null; + } + + if ( + this.params.case_sensitive || + this.params.fixed_strings || + this.params.context !== undefined || + this.params.after !== undefined || + this.params.before !== undefined || + this.params.no_ignore + ) { + return null; + } + + debugLogger.debug( + `GrepLogic: ripgrep failed, trying GrepTool fallback: ${errorMessage}`, + ); + + const fallbackTool = new GrepTool(this.config, this.messageBus); + const invocation = fallbackTool.build({ + pattern: this.params.pattern, + dir_path: pathParam, + include_pattern: this.params.include_pattern, + exclude_pattern: this.params.exclude_pattern, + names_only: this.params.names_only, + max_matches_per_file: this.params.max_matches_per_file, + total_max_matches: totalMaxMatches, + }); + + return invocation.execute({ abortSignal: signal }); + } + private async performRipgrepSearch(options: { pattern: string; path: string | string[];