Fix Windows ripgrep detection (#11492)

.gitignore

@@ -47,6 +47,7 @@ packages/cli/src/generated/
 packages/core/src/generated/
 .integration-tests/
 packages/vscode-ide-companion/*.vsix
+packages/cli/download-ripgrep*/
 
 # GHA credentials
 gha-creds-*.json

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

@@ -10,8 +10,8 @@ import {
   expect,
   beforeEach,
   afterEach,
+  afterAll,
   vi,
-  type Mock,
 } from 'vitest';
 import type { RipGrepToolParams } from './ripGrep.js';
 import { canUseRipgrep, RipGrepTool, ensureRgPath } from './ripGrep.js';
@@ -19,28 +19,15 @@ import path from 'node:path';
 import fs from 'node:fs/promises';
 import os, { EOL } from 'node:os';
 import type { Config } from '../config/config.js';
+import { Storage } from '../config/storage.js';
 import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js';
 import type { ChildProcess } from 'node:child_process';
 import { spawn } from 'node:child_process';
 import { downloadRipGrep } from '@joshua.litt/get-ripgrep';
-import { fileExists } from '../utils/fileUtils.js';
-
 // Mock dependencies for canUseRipgrep
 vi.mock('@joshua.litt/get-ripgrep', () => ({
   downloadRipGrep: vi.fn(),
 }));
-vi.mock('../utils/fileUtils.js', async (importOriginal) => {
-  const actual = await importOriginal<typeof import('../utils/fileUtils.js')>();
-  return {
-    ...actual,
-    fileExists: vi.fn(),
-  };
-});
-vi.mock('../config/storage.js', () => ({
-  Storage: {
-    getGlobalBinDir: vi.fn().mockReturnValue('/mock/bin/dir'),
-  },
-}));
 
 // Mock child_process for ripgrep calls
 vi.mock('child_process', () => ({
@@ -48,96 +35,167 @@ vi.mock('child_process', () => ({
 }));
 
 const mockSpawn = vi.mocked(spawn);
+const downloadRipGrepMock = vi.mocked(downloadRipGrep);
+const originalGetGlobalBinDir = Storage.getGlobalBinDir.bind(Storage);
+const storageSpy = vi.spyOn(Storage, 'getGlobalBinDir');
 
 describe('canUseRipgrep', () => {
-  beforeEach(() => {
-    vi.clearAllMocks();
+  let tempRootDir: string;
+  let binDir: string;
+
+  beforeEach(async () => {
+    downloadRipGrepMock.mockReset();
+    downloadRipGrepMock.mockResolvedValue(undefined);
+    tempRootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'ripgrep-bin-'));
+    binDir = path.join(tempRootDir, 'bin');
+    await fs.mkdir(binDir, { recursive: true });
+    storageSpy.mockImplementation(() => binDir);
+  });
+
+  afterEach(async () => {
+    storageSpy.mockImplementation(() => originalGetGlobalBinDir());
+    await fs.rm(tempRootDir, { recursive: true, force: true });
   });
 
   it('should return true if ripgrep already exists', async () => {
-    (fileExists as Mock).mockResolvedValue(true);
+    const candidateNames =
+      process.platform === 'win32' ? ['rg.exe', 'rg'] : ['rg'];
+    const existingPath = path.join(binDir, candidateNames[0]);
+    await fs.writeFile(existingPath, '');
+
     const result = await canUseRipgrep();
     expect(result).toBe(true);
-    expect(fileExists).toHaveBeenCalledWith(path.join('/mock/bin/dir', 'rg'));
-    expect(downloadRipGrep).not.toHaveBeenCalled();
+    expect(downloadRipGrepMock).not.toHaveBeenCalled();
   });
 
   it('should download ripgrep and return true if it does not exist initially', async () => {
-    (fileExists as Mock)
-      .mockResolvedValueOnce(false)
-      .mockResolvedValueOnce(true);
-    (downloadRipGrep as Mock).mockResolvedValue(undefined);
+    const candidateNames =
+      process.platform === 'win32' ? ['rg.exe', 'rg'] : ['rg'];
+    const expectedPath = path.join(binDir, candidateNames[0]);
+
+    downloadRipGrepMock.mockImplementation(async () => {
+      await fs.writeFile(expectedPath, '');
+    });
 
     const result = await canUseRipgrep();
 
     expect(result).toBe(true);
-    expect(fileExists).toHaveBeenCalledTimes(2);
-    expect(downloadRipGrep).toHaveBeenCalledWith('/mock/bin/dir');
+    expect(downloadRipGrep).toHaveBeenCalledWith(binDir);
+    await expect(fs.access(expectedPath)).resolves.toBeUndefined();
   });
 
   it('should return false if download fails and file does not exist', async () => {
-    (fileExists as Mock).mockResolvedValue(false);
-    (downloadRipGrep as Mock).mockResolvedValue(undefined);
-
     const result = await canUseRipgrep();
 
     expect(result).toBe(false);
-    expect(fileExists).toHaveBeenCalledTimes(2);
-    expect(downloadRipGrep).toHaveBeenCalledWith('/mock/bin/dir');
+    expect(downloadRipGrep).toHaveBeenCalledWith(binDir);
   });
 
   it('should propagate errors from downloadRipGrep', async () => {
     const error = new Error('Download failed');
-    (fileExists as Mock).mockResolvedValue(false);
-    (downloadRipGrep as Mock).mockRejectedValue(error);
+    downloadRipGrepMock.mockRejectedValue(error);
 
     await expect(canUseRipgrep()).rejects.toThrow(error);
-    expect(fileExists).toHaveBeenCalledTimes(1);
-    expect(downloadRipGrep).toHaveBeenCalledWith('/mock/bin/dir');
+    expect(downloadRipGrep).toHaveBeenCalledWith(binDir);
+  });
+
+  it('should only download once when called concurrently', async () => {
+    const candidateNames =
+      process.platform === 'win32' ? ['rg.exe', 'rg'] : ['rg'];
+    const expectedPath = path.join(binDir, candidateNames[0]);
+
+    downloadRipGrepMock.mockImplementation(
+      () =>
+        new Promise<void>((resolve, reject) => {
+          setTimeout(() => {
+            fs.writeFile(expectedPath, '')
+              .then(() => resolve())
+              .catch(reject);
+          }, 0);
+        }),
+    );
+
+    const firstCall = ensureRgPath();
+    const secondCall = ensureRgPath();
+
+    const [pathOne, pathTwo] = await Promise.all([firstCall, secondCall]);
+
+    expect(pathOne).toBe(expectedPath);
+    expect(pathTwo).toBe(expectedPath);
+    expect(downloadRipGrepMock).toHaveBeenCalledTimes(1);
+    await expect(fs.access(expectedPath)).resolves.toBeUndefined();
   });
 });
 
 describe('ensureRgPath', () => {
-  beforeEach(() => {
-    vi.clearAllMocks();
+  let tempRootDir: string;
+  let binDir: string;
+
+  beforeEach(async () => {
+    downloadRipGrepMock.mockReset();
+    downloadRipGrepMock.mockResolvedValue(undefined);
+    tempRootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'ripgrep-bin-'));
+    binDir = path.join(tempRootDir, 'bin');
+    await fs.mkdir(binDir, { recursive: true });
+    storageSpy.mockImplementation(() => binDir);
+  });
+
+  afterEach(async () => {
+    storageSpy.mockImplementation(() => originalGetGlobalBinDir());
+    await fs.rm(tempRootDir, { recursive: true, force: true });
   });
 
   it('should return rg path if ripgrep already exists', async () => {
-    (fileExists as Mock).mockResolvedValue(true);
+    const candidateNames =
+      process.platform === 'win32' ? ['rg.exe', 'rg'] : ['rg'];
+    const existingPath = path.join(binDir, candidateNames[0]);
+    await fs.writeFile(existingPath, '');
+
     const rgPath = await ensureRgPath();
-    expect(rgPath).toBe(path.join('/mock/bin/dir', 'rg'));
-    expect(fileExists).toHaveBeenCalledOnce();
+    expect(rgPath).toBe(existingPath);
     expect(downloadRipGrep).not.toHaveBeenCalled();
   });
 
   it('should return rg path if ripgrep is downloaded successfully', async () => {
-    (fileExists as Mock)
-      .mockResolvedValueOnce(false)
-      .mockResolvedValueOnce(true);
-    (downloadRipGrep as Mock).mockResolvedValue(undefined);
+    const candidateNames =
+      process.platform === 'win32' ? ['rg.exe', 'rg'] : ['rg'];
+    const expectedPath = path.join(binDir, candidateNames[0]);
+
+    downloadRipGrepMock.mockImplementation(async () => {
+      await fs.writeFile(expectedPath, '');
+    });
+
     const rgPath = await ensureRgPath();
-    expect(rgPath).toBe(path.join('/mock/bin/dir', 'rg'));
-    expect(downloadRipGrep).toHaveBeenCalledOnce();
-    expect(fileExists).toHaveBeenCalledTimes(2);
+    expect(rgPath).toBe(expectedPath);
+    expect(downloadRipGrep).toHaveBeenCalledTimes(1);
+    await expect(fs.access(expectedPath)).resolves.toBeUndefined();
   });
 
   it('should throw an error if ripgrep cannot be used after download attempt', async () => {
-    (fileExists as Mock).mockResolvedValue(false);
-    (downloadRipGrep as Mock).mockResolvedValue(undefined);
     await expect(ensureRgPath()).rejects.toThrow('Cannot use ripgrep.');
-    expect(downloadRipGrep).toHaveBeenCalledOnce();
-    expect(fileExists).toHaveBeenCalledTimes(2);
+    expect(downloadRipGrep).toHaveBeenCalledTimes(1);
   });
 
   it('should propagate errors from downloadRipGrep', async () => {
     const error = new Error('Download failed');
-    (fileExists as Mock).mockResolvedValue(false);
-    (downloadRipGrep as Mock).mockRejectedValue(error);
+    downloadRipGrepMock.mockRejectedValue(error);
 
     await expect(ensureRgPath()).rejects.toThrow(error);
-    expect(fileExists).toHaveBeenCalledTimes(1);
-    expect(downloadRipGrep).toHaveBeenCalledWith('/mock/bin/dir');
+    expect(downloadRipGrep).toHaveBeenCalledWith(binDir);
   });
+
+  it.runIf(process.platform === 'win32')(
+    'should detect ripgrep when only rg.exe exists on Windows',
+    async () => {
+      const expectedRgExePath = path.join(binDir, 'rg.exe');
+      await fs.writeFile(expectedRgExePath, '');
+
+      const rgPath = await ensureRgPath();
+      expect(rgPath).toBe(expectedRgExePath);
+      expect(downloadRipGrep).not.toHaveBeenCalled();
+      await expect(fs.access(expectedRgExePath)).resolves.toBeUndefined();
+    },
+  );
 });
 
 // Helper function to create mock spawn implementations
@@ -190,6 +248,9 @@ function createMockSpawn(
 
 describe('RipGrepTool', () => {
   let tempRootDir: string;
+  let tempBinRoot: string;
+  let binDir: string;
+  let ripgrepBinaryPath: string;
   let grepTool: RipGrepTool;
   const abortSignal = new AbortController().signal;
 
@@ -200,10 +261,16 @@ describe('RipGrepTool', () => {
   } as unknown as Config;
 
   beforeEach(async () => {
-    vi.clearAllMocks();
-    (downloadRipGrep as Mock).mockResolvedValue(undefined);
-    (fileExists as Mock).mockResolvedValue(true);
-    mockSpawn.mockClear();
+    downloadRipGrepMock.mockReset();
+    downloadRipGrepMock.mockResolvedValue(undefined);
+    mockSpawn.mockReset();
+    tempBinRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'ripgrep-bin-'));
+    binDir = path.join(tempBinRoot, 'bin');
+    await fs.mkdir(binDir, { recursive: true });
+    const binaryName = process.platform === 'win32' ? 'rg.exe' : 'rg';
+    ripgrepBinaryPath = path.join(binDir, binaryName);
+    await fs.writeFile(ripgrepBinaryPath, '');
+    storageSpy.mockImplementation(() => binDir);
     tempRootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'grep-tool-root-'));
     grepTool = new RipGrepTool(mockConfig);
 
@@ -228,7 +295,9 @@ describe('RipGrepTool', () => {
   });
 
   afterEach(async () => {
+    storageSpy.mockImplementation(() => originalGetGlobalBinDir());
     await fs.rm(tempRootDir, { recursive: true, force: true });
+    await fs.rm(tempBinRoot, { recursive: true, force: true });
   });
 
   describe('validateToolParams', () => {
@@ -551,9 +620,8 @@ describe('RipGrepTool', () => {
     });
 
     it('should throw an error if ripgrep is not available', async () => {
-      // Make ensureRgPath throw
-      (fileExists as Mock).mockResolvedValue(false);
-      (downloadRipGrep as Mock).mockResolvedValue(undefined);
+      await fs.rm(ripgrepBinaryPath, { force: true });
+      downloadRipGrepMock.mockResolvedValue(undefined);
 
       const params: RipGrepToolParams = { pattern: 'world' };
       const invocation = grepTool.build(params);
@@ -1353,3 +1421,6 @@ describe('RipGrepTool', () => {
     });
   });
 });
+afterAll(() => {
+  storageSpy.mockRestore();
+});

packages/core/src/tools/ripGrep.ts

@@ -21,28 +21,55 @@ import { GREP_TOOL_NAME } from './tool-names.js';
 
 const DEFAULT_TOTAL_MAX_MATCHES = 20000;
 
-function getRgPath(): string {
-  return path.join(Storage.getGlobalBinDir(), 'rg');
+function getRgCandidateFilenames(): readonly string[] {
+  return process.platform === 'win32' ? ['rg.exe', 'rg'] : ['rg'];
+}
+
+async function resolveExistingRgPath(): Promise<string | null> {
+  const binDir = Storage.getGlobalBinDir();
+  for (const fileName of getRgCandidateFilenames()) {
+    const candidatePath = path.join(binDir, fileName);
+    if (await fileExists(candidatePath)) {
+      return candidatePath;
+    }
+  }
+  return null;
+}
+
+let ripgrepAcquisitionPromise: Promise<string | null> | null = null;
+
+async function ensureRipgrepAvailable(): Promise<string | null> {
+  const existingPath = await resolveExistingRgPath();
+  if (existingPath) {
+    return existingPath;
+  }
+  if (!ripgrepAcquisitionPromise) {
+    ripgrepAcquisitionPromise = (async () => {
+      try {
+        await downloadRipGrep(Storage.getGlobalBinDir());
+        return await resolveExistingRgPath();
+      } finally {
+        ripgrepAcquisitionPromise = null;
+      }
+    })();
+  }
+  return ripgrepAcquisitionPromise;
 }
 
 /**
  * Checks if `rg` exists, if not then attempt to download it.
  */
 export async function canUseRipgrep(): Promise<boolean> {
-  if (await fileExists(getRgPath())) {
-    return true;
-  }
-
-  await downloadRipGrep(Storage.getGlobalBinDir());
-  return await fileExists(getRgPath());
+  return (await ensureRipgrepAvailable()) !== null;
 }
 
 /**
  * Ensures `rg` is downloaded, or throws.
  */
 export async function ensureRgPath(): Promise<string> {
-  if (await canUseRipgrep()) {
-    return getRgPath();
+  const downloadedPath = await ensureRipgrepAvailable();
+  if (downloadedPath) {
+    return downloadedPath;
   }
   throw new Error('Cannot use ripgrep.');
 }

third_party/get-ripgrep/src/downloadRipGrep.js

@@ -104,7 +104,7 @@ const untarGz = async (inFile, outDir) => {
   }
 }
 
-export const downloadRipGrep = async () => {
+export const downloadRipGrep = async (binPath = BIN_PATH) => {
   const target = getTarget()
   const url = `https://github.com/${REPOSITORY}/releases/download/${VERSION}/ripgrep-${VERSION}-${target}`
   const downloadPath = `${xdgCache}/vscode-ripgrep/ripgrep-${VERSION}-${target}`
@@ -114,9 +114,9 @@ export const downloadRipGrep = async () => {
     console.info(`File ${downloadPath} has been cached`)
   }
   if (downloadPath.endsWith('.tar.gz')) {
-    await untarGz(downloadPath, BIN_PATH)
+    await untarGz(downloadPath, binPath)
   } else if (downloadPath.endsWith('.zip')) {
-    await unzip(downloadPath, BIN_PATH)
+    await unzip(downloadPath, binPath)
   } else {
     throw new VError(`Invalid downloadPath ${downloadPath}`)
   }