Skip to content

Commit 2ccb983

Browse files
authored
feat(core): Re-download rg binary if it is deleted. (google-gemini#8126)
1 parent b2c3523 commit 2ccb983

File tree

2 files changed

+73
-3
lines changed

2 files changed

+73
-3
lines changed

packages/core/src/tools/ripGrep.test.ts

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
type Mock,
1515
} from 'vitest';
1616
import type { RipGrepToolParams } from './ripGrep.js';
17-
import { canUseRipgrep, RipGrepTool } from './ripGrep.js';
17+
import { canUseRipgrep, RipGrepTool, ensureRgPath } from './ripGrep.js';
1818
import path from 'node:path';
1919
import fs from 'node:fs/promises';
2020
import os, { EOL } from 'node:os';
@@ -97,6 +97,49 @@ describe('canUseRipgrep', () => {
9797
});
9898
});
9999

100+
describe('ensureRgPath', () => {
101+
beforeEach(() => {
102+
vi.clearAllMocks();
103+
});
104+
105+
it('should return rg path if ripgrep already exists', async () => {
106+
(fileExists as Mock).mockResolvedValue(true);
107+
const rgPath = await ensureRgPath();
108+
expect(rgPath).toBe(path.join('/mock/bin/dir', 'rg'));
109+
expect(fileExists).toHaveBeenCalledOnce();
110+
expect(downloadRipGrep).not.toHaveBeenCalled();
111+
});
112+
113+
it('should return rg path if ripgrep is downloaded successfully', async () => {
114+
(fileExists as Mock)
115+
.mockResolvedValueOnce(false)
116+
.mockResolvedValueOnce(true);
117+
(downloadRipGrep as Mock).mockResolvedValue(undefined);
118+
const rgPath = await ensureRgPath();
119+
expect(rgPath).toBe(path.join('/mock/bin/dir', 'rg'));
120+
expect(downloadRipGrep).toHaveBeenCalledOnce();
121+
expect(fileExists).toHaveBeenCalledTimes(2);
122+
});
123+
124+
it('should throw an error if ripgrep cannot be used after download attempt', async () => {
125+
(fileExists as Mock).mockResolvedValue(false);
126+
(downloadRipGrep as Mock).mockResolvedValue(undefined);
127+
await expect(ensureRgPath()).rejects.toThrow('Cannot use ripgrep.');
128+
expect(downloadRipGrep).toHaveBeenCalledOnce();
129+
expect(fileExists).toHaveBeenCalledTimes(2);
130+
});
131+
132+
it('should propagate errors from downloadRipGrep', async () => {
133+
const error = new Error('Download failed');
134+
(fileExists as Mock).mockResolvedValue(false);
135+
(downloadRipGrep as Mock).mockRejectedValue(error);
136+
137+
await expect(ensureRgPath()).rejects.toThrow(error);
138+
expect(fileExists).toHaveBeenCalledTimes(1);
139+
expect(downloadRipGrep).toHaveBeenCalledWith('/mock/bin/dir');
140+
});
141+
});
142+
100143
// Helper function to create mock spawn implementations
101144
function createMockSpawn(
102145
options: {
@@ -158,6 +201,8 @@ describe('RipGrepTool', () => {
158201

159202
beforeEach(async () => {
160203
vi.clearAllMocks();
204+
(downloadRipGrep as Mock).mockResolvedValue(undefined);
205+
(fileExists as Mock).mockResolvedValue(true);
161206
mockSpawn.mockClear();
162207
tempRootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'grep-tool-root-'));
163208
grepTool = new RipGrepTool(mockConfig);
@@ -504,6 +549,20 @@ describe('RipGrepTool', () => {
504549
/params must have required property 'pattern'/,
505550
);
506551
});
552+
553+
it('should throw an error if ripgrep is not available', async () => {
554+
// Make ensureRgPath throw
555+
(fileExists as Mock).mockResolvedValue(false);
556+
(downloadRipGrep as Mock).mockResolvedValue(undefined);
557+
558+
const params: RipGrepToolParams = { pattern: 'world' };
559+
const invocation = grepTool.build(params);
560+
561+
expect(await invocation.execute(abortSignal)).toStrictEqual({
562+
llmContent: 'Error during grep search operation: Cannot use ripgrep.',
563+
returnDisplay: 'Error: Cannot use ripgrep.',
564+
});
565+
});
507566
});
508567

509568
describe('multi-directory workspace', () => {

packages/core/src/tools/ripGrep.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { Storage } from '../config/storage.js';
2020

2121
const DEFAULT_TOTAL_MAX_MATCHES = 20000;
2222

23-
function getRgPath() {
23+
function getRgPath(): string {
2424
return path.join(Storage.getGlobalBinDir(), 'rg');
2525
}
2626

@@ -36,6 +36,16 @@ export async function canUseRipgrep(): Promise<boolean> {
3636
return await fileExists(getRgPath());
3737
}
3838

39+
/**
40+
* Ensures `rg` is downloaded, or throws.
41+
*/
42+
export async function ensureRgPath(): Promise<string> {
43+
if (await canUseRipgrep()) {
44+
return getRgPath();
45+
}
46+
throw new Error('Cannot use ripgrep.');
47+
}
48+
3949
/**
4050
* Parameters for the GrepTool
4151
*/
@@ -310,8 +320,9 @@ class GrepToolInvocation extends BaseToolInvocation<
310320
rgArgs.push(absolutePath);
311321

312322
try {
323+
const rgPath = await ensureRgPath();
313324
const output = await new Promise<string>((resolve, reject) => {
314-
const child = spawn(getRgPath(), rgArgs, {
325+
const child = spawn(rgPath, rgArgs, {
315326
windowsHide: true,
316327
});
317328

0 commit comments

Comments
 (0)