Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 36 additions & 4 deletions packages/core/src/tools/grep.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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();
});
});

Expand Down Expand Up @@ -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<boolean>;
execute: (options: ExecuteOptions) => Promise<ToolResult>;
};
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(
Expand Down
74 changes: 46 additions & 28 deletions packages/core/src/tools/grep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
},
};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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);
});
Expand Down Expand Up @@ -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;
Expand All @@ -404,6 +414,7 @@ class GrepToolInvocation extends BaseToolInvocation<
const {
pattern,
path: absolutePath,
filePath,
include_pattern,
exclude_pattern,
maxMatches,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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[] = [];
Expand Down Expand Up @@ -734,11 +752,11 @@ export class GrepTool extends BaseDeclarativeTool<GrepToolParams, ToolResult> {
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') {
Expand Down
58 changes: 55 additions & 3 deletions packages/core/src/tools/ripGrep.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -97,6 +98,11 @@ function createMockSpawn(

// Emulating process exit
setTimeout(() => {
if (errorData) {
stderr.end(errorData);
} else {
stderr.end();
}
mockProcess.emit('close', exitCode, signal);
}, 10);

Expand Down Expand Up @@ -125,6 +131,9 @@ function createMockConfig(
getFileFilteringRespectGeminiIgnore(this: Config) {
return this.getFileFilteringOptions().respectGeminiIgnore;
},
getFileExclusions: () => ({
getGlobExcludes: () => [],
}),
storage: {
getProjectTempDir: vi.fn().mockReturnValue('/tmp/project'),
},
Expand Down Expand Up @@ -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', () => {
Expand Down
Loading
Loading