diff --git a/src/main/ipc/filesystem-list-files-git-fallback-real.test.ts b/src/main/ipc/filesystem-list-files-git-fallback-real.test.ts index 0cf672fa30f..69dde7afaf3 100644 --- a/src/main/ipc/filesystem-list-files-git-fallback-real.test.ts +++ b/src/main/ipc/filesystem-list-files-git-fallback-real.test.ts @@ -1,17 +1,19 @@ -import { execFile as execFileCallback } from 'node:child_process' +import { execFile as execFileCallback, spawn, type SpawnOptions } from 'node:child_process' import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' import { promisify } from 'node:util' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { Store } from '../persistence' +import type * as GitRunner from '../git/runner' -const { checkRgAvailableMock } = vi.hoisted(() => ({ - checkRgAvailableMock: vi.fn() +const { wslAwareSpawnMock } = vi.hoisted(() => ({ + wslAwareSpawnMock: vi.fn() })) -vi.mock('./rg-availability', () => ({ - checkRgAvailable: checkRgAvailableMock +vi.mock('../git/runner', async (importOriginal) => ({ + ...(await importOriginal()), + wslAwareSpawn: wslAwareSpawnMock })) import { listQuickOpenFiles } from './filesystem-list-files' @@ -50,6 +52,16 @@ async function initRepo(repoPath: string): Promise { describe('filesystem-list-files real git fallback', () => { let tempDir: string | null = null + beforeEach(() => { + wslAwareSpawnMock.mockImplementation( + (_command: string, _args: string[], options: SpawnOptions & { cwd?: string }) => + spawn('orca-definitely-missing-rg', [], { + cwd: options.cwd, + stdio: options.stdio + }) + ) + }) + afterEach(async () => { if (tempDir) { await rm(tempDir, { recursive: true, force: true }) @@ -59,7 +71,6 @@ describe('filesystem-list-files real git fallback', () => { }) it('returns real paths for UTF-8 filenames from the git fallback', async () => { - checkRgAvailableMock.mockResolvedValue(false) tempDir = await mkdtemp(join(tmpdir(), 'orca-quick-open-git-fallback-')) const repoPath = join(tempDir, 'repo') await execFile('git', ['init', '-q', repoPath]) @@ -71,7 +82,6 @@ describe('filesystem-list-files real git fallback', () => { }) it('fills nested git repos from gitlink and untracked embedded-repo entries', async () => { - checkRgAvailableMock.mockResolvedValue(false) tempDir = await mkdtemp(join(tmpdir(), 'orca-quick-open-monorepo-')) const repoPath = join(tempDir, 'parent') const appPath = join(repoPath, 'packages', 'app') @@ -117,7 +127,6 @@ describe('filesystem-list-files real git fallback', () => { }) it('walks a non-git root instead of returning an empty git fallback result', async () => { - checkRgAvailableMock.mockResolvedValue(false) tempDir = await mkdtemp(join(tmpdir(), 'orca-quick-open-non-git-')) await writeRel(tempDir, 'folder/file.ts') @@ -127,7 +136,6 @@ describe('filesystem-list-files real git fallback', () => { }) it('bounds a non-git readdir fallback without treating the limit as an error', async () => { - checkRgAvailableMock.mockResolvedValue(false) tempDir = await mkdtemp(join(tmpdir(), 'orca-quick-open-bounded-non-git-')) await writeRel(tempDir, 'a.ts') await writeRel(tempDir, 'b.ts') @@ -139,7 +147,6 @@ describe('filesystem-list-files real git fallback', () => { }) it('rejects abnormal git ls-files failures instead of resolving an empty list', async () => { - checkRgAvailableMock.mockResolvedValue(false) tempDir = await mkdtemp(join(tmpdir(), 'orca-quick-open-bad-index-')) const repoPath = join(tempDir, 'repo') await initRepo(repoPath) @@ -151,7 +158,6 @@ describe('filesystem-list-files real git fallback', () => { }) it('resolves an empty repo as an empty list', async () => { - checkRgAvailableMock.mockResolvedValue(false) tempDir = await mkdtemp(join(tmpdir(), 'orca-quick-open-empty-repo-')) const repoPath = join(tempDir, 'repo') await initRepo(repoPath) diff --git a/src/main/ipc/filesystem-list-files-install-rg.test.ts b/src/main/ipc/filesystem-list-files-install-rg.test.ts index 7f4c34f5eda..f0221d8e24e 100644 --- a/src/main/ipc/filesystem-list-files-install-rg.test.ts +++ b/src/main/ipc/filesystem-list-files-install-rg.test.ts @@ -1,16 +1,21 @@ +import { spawn, type ChildProcess, type SpawnOptions } from 'node:child_process' +import { EventEmitter } from 'node:events' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { Store } from '../persistence' +import type * as GitRunner from '../git/runner' const { listFilesWithGitMock, resolveAuthorizedPathMock, checkRgAvailableMock, - getLocalGitOptionsForRegisteredWorktreeMock + getLocalGitOptionsForRegisteredWorktreeMock, + wslAwareSpawnMock } = vi.hoisted(() => ({ listFilesWithGitMock: vi.fn(), resolveAuthorizedPathMock: vi.fn(), checkRgAvailableMock: vi.fn(), - getLocalGitOptionsForRegisteredWorktreeMock: vi.fn() + getLocalGitOptionsForRegisteredWorktreeMock: vi.fn(), + wslAwareSpawnMock: vi.fn() })) vi.mock('./filesystem-list-files-git-fallback', () => ({ @@ -29,14 +34,39 @@ vi.mock('./local-worktree-runtime-options', () => ({ getLocalGitOptionsForRegisteredWorktree: getLocalGitOptionsForRegisteredWorktreeMock })) +vi.mock('../git/runner', async (importOriginal) => ({ + ...(await importOriginal()), + wslAwareSpawn: wslAwareSpawnMock +})) + import { listQuickOpenFiles } from './filesystem-list-files' +function createStartedRipgrepProcess(): ChildProcess { + const child = new EventEmitter() as ChildProcess + ;(child as unknown as Record).stdout = new EventEmitter() + ;( + (child as unknown as Record).stdout as EventEmitter & { + setEncoding: () => void + } + ).setEncoding = vi.fn() + ;(child as unknown as Record).stderr = new EventEmitter() + ;(child as unknown as Record).kill = vi.fn() + ;(child as unknown as Record).exitCode = null + ;(child as unknown as Record).signalCode = null + Object.defineProperty(child, 'pid', { value: 1 }) + return child +} + describe('filesystem-list-files ripgrep guidance', () => { beforeEach(() => { vi.clearAllMocks() resolveAuthorizedPathMock.mockImplementation(async (path) => path) - checkRgAvailableMock.mockResolvedValue(false) + checkRgAvailableMock.mockResolvedValue(true) getLocalGitOptionsForRegisteredWorktreeMock.mockReturnValue({}) + wslAwareSpawnMock.mockImplementation( + (_command: string, _args: string[], options: SpawnOptions & { cwd?: string }) => + spawn('orca-definitely-missing-rg', [], { stdio: options.stdio }) + ) }) it('turns only a readdir budget failure into install guidance', async () => { @@ -51,6 +81,31 @@ describe('filesystem-list-files ripgrep guidance', () => { ) }) + it('keeps the WSL preflight and falls back before starting real rg', async () => { + getLocalGitOptionsForRegisteredWorktreeMock.mockReturnValue({ wslDistro: 'Ubuntu' }) + checkRgAvailableMock.mockResolvedValue(false) + listFilesWithGitMock.mockResolvedValue(['src/index.ts']) + + await expect(listQuickOpenFiles('C:\\repo', {} as Store)).resolves.toEqual(['src/index.ts']) + expect(checkRgAvailableMock).toHaveBeenCalledWith('C:\\repo', 'Ubuntu') + expect(wslAwareSpawnMock).not.toHaveBeenCalled() + }) + + it("falls back when a native launcher exits outside ripgrep's contract", async () => { + const primary = createStartedRipgrepProcess() + const ignored = createStartedRipgrepProcess() + let callIndex = 0 + wslAwareSpawnMock.mockImplementation(() => (++callIndex === 1 ? primary : ignored)) + listFilesWithGitMock.mockResolvedValue(['src/index.ts']) + + const listing = listQuickOpenFiles('/workspace', {} as Store) + await Promise.resolve() + primary.emit('close', 127, null) + + await expect(listing).resolves.toEqual(['src/index.ts']) + expect(listFilesWithGitMock).toHaveBeenCalledTimes(1) + }) + it('keeps cancellation and Git errors unchanged', async () => { const cancellation = new Error('File listing cancelled') listFilesWithGitMock.mockRejectedValueOnce(cancellation) diff --git a/src/main/ipc/filesystem-list-files.test.ts b/src/main/ipc/filesystem-list-files.test.ts index 946f8653dd6..7debd516760 100644 --- a/src/main/ipc/filesystem-list-files.test.ts +++ b/src/main/ipc/filesystem-list-files.test.ts @@ -36,6 +36,7 @@ import { listQuickOpenFiles } from './filesystem-list-files' import { EventEmitter } from 'node:events' import type { Store } from '../persistence' import type { ChildProcess } from 'node:child_process' +import { FileListingCancelledError } from '../../shared/file-listing-cancellation' const SHA1 = '0123456789abcdef0123456789abcdef01234567' @@ -55,10 +56,24 @@ function createMockProcess(): ChildProcess { ;(p as unknown as Record).kill = vi.fn() ;(p as unknown as Record).exitCode = null ;(p as unknown as Record).signalCode = null + Object.defineProperty(p, 'pid', { configurable: true, value: 1 }) return p } +function createMissingRipgrepProcess(): ChildProcess { + const child = createMockProcess() + Object.defineProperty(child, 'pid', { value: undefined }) + void Promise.resolve().then(() => child.emit('close', -2, null)) + return child +} + +async function flushMicrotasks(): Promise { + for (let index = 0; index < 12; index++) { + await Promise.resolve() + } +} + function isIgnoredRgPass(args: string[]): boolean { return args.includes('--no-ignore-vcs') } @@ -93,6 +108,8 @@ describe('filesystem-list-files', () => { expect(p1.kill).toHaveBeenCalled() expect(p2.kill).not.toHaveBeenCalled() expect(spawnMock).toHaveBeenCalledTimes(1) + expect(checkRgAvailableMock).not.toHaveBeenCalled() + expect(spawnMock.mock.calls[0]?.[1]).not.toContain('--version') }) it('merges normal files and ignored files and filters correctly', async () => { @@ -108,6 +125,8 @@ describe('filesystem-list-files', () => { const storeMock = {} as unknown as Store const promise = listQuickOpenFiles('/mock/root', storeMock) + await flushMicrotasks() + expect(spawnMock).toHaveBeenCalledTimes(2) // Simulate stdout output for normal files setTimeout(() => { @@ -192,6 +211,24 @@ describe('filesystem-list-files', () => { await expect(promise).resolves.toEqual(['src/index.ts']) }) + it('does not mistake a WSL-routed executable exit 127 for missing rg', async () => { + const p1 = createMockProcess() + const p2 = createMockProcess() + Object.defineProperty(p1, 'pid', { value: 1 }) + Object.defineProperty(p2, 'pid', { value: 2 }) + getLocalGitOptionsForRegisteredWorktreeMock.mockReturnValue({ wslDistro: 'Ubuntu' }) + spawnMock.mockImplementation((_cmd, args: string[]) => (isIgnoredRgPass(args) ? p2 : p1)) + + const promise = listQuickOpenFiles('C:\\repo', {} as unknown as Store) + setTimeout(() => { + p1.emit('close', 127, null) + p2.emit('close', 0, null) + }, 0) + + await expect(promise).rejects.toThrow('rg exited with code 127') + expect(spawnMock.mock.calls.some((call) => call[0] === 'git')).toBe(false) + }) + it('rejects rg failures instead of resolving a false-empty list', async () => { const p1 = createMockProcess() const p2 = createMockProcess() @@ -330,16 +367,90 @@ describe('filesystem-list-files', () => { expect(result).toEqual(['valid.ts']) }) - describe('git ls-files fallback', () => { - it('falls back to git ls-files when rg is not available', async () => { - checkRgAvailableMock.mockResolvedValue(false) + it('lets cancellation win a native-unavailable race before Git starts', async () => { + const first = createMockProcess() + Object.defineProperty(first, 'pid', { value: undefined }) + spawnMock.mockReturnValue(first) + const controller = new AbortController() + const cancellation = new FileListingCancelledError('superseded') + const promise = listQuickOpenFiles( + '/mock/root', + {} as unknown as Store, + undefined, + controller.signal + ) + await flushMicrotasks() + controller.abort(cancellation) + first.emit('close', -2, null) + + await expect(promise).rejects.toBe(cancellation) + expect(spawnMock).toHaveBeenCalledTimes(1) + expect(spawnMock.mock.calls.some((call) => call[0] === 'git')).toBe(false) + const error = Object.assign(new Error('spawn rg ENOENT'), { code: 'ENOENT' }) + expect(() => first.emit('error', error)).not.toThrow() + expect(first.listenerCount('error')).toBe(0) + }) + + describe('git ls-files fallback', () => { + it('kills only the admitted pass when ignored rg fails before spawn', async () => { + const primary = createMockProcess() + const missingIgnored = createMockProcess() + const revParse = createMockProcess() + const gitPrimary = createMockProcess() + const gitIgnored = createMockProcess() + Object.defineProperty(missingIgnored, 'pid', { value: undefined }) + let gitPassIndex = 0 + spawnMock.mockImplementation((cmd: string, args: string[]) => { + if (cmd === 'rg') { + return isIgnoredRgPass(args) ? missingIgnored : primary + } + if (args.includes('rev-parse')) { + return revParse + } + if (args.includes('ls-files')) { + return gitPassIndex++ === 0 ? gitPrimary : gitIgnored + } + return createMockProcess() + }) + + const promise = listQuickOpenFiles('/mock/root', {} as unknown as Store) + await flushMicrotasks() + expect(spawnMock.mock.calls.filter((call) => call[0] === 'rg')).toHaveLength(2) + missingIgnored.emit('close', -2, null) + + await vi.waitFor(() => expect(primary.kill).toHaveBeenCalled()) + expect(missingIgnored.kill).not.toHaveBeenCalled() + const error = Object.assign(new Error('spawn rg ENOENT'), { code: 'ENOENT' }) + expect(() => missingIgnored.emit('error', error)).not.toThrow() + await vi.waitFor(() => + expect( + spawnMock.mock.calls.some((call) => (call[1] as string[]).includes('rev-parse')) + ).toBe(true) + ) + revParse.emit('close', 0, null) + await vi.waitFor(() => + expect( + spawnMock.mock.calls.filter((call) => (call[1] as string[]).includes('ls-files')) + ).toHaveLength(2) + ) + gitPrimary.emit('close', 0, null) + gitIgnored.emit('close', 0, null) + + await expect(promise).resolves.toEqual([]) + expect(missingIgnored.listenerCount('error')).toBe(0) + }) + + it('falls back to git ls-files when rg is not available', async () => { let callIndex = 0 const revParseProc = createMockProcess() const gitP1 = createMockProcess() const gitP2 = createMockProcess() spawnMock.mockImplementation((cmd: string, args: string[]) => { + if (cmd === 'rg') { + return createMissingRipgrepProcess() + } if (cmd === 'git' && args.includes('rev-parse')) { return revParseProc } @@ -378,9 +489,10 @@ describe('filesystem-list-files', () => { const result = await promise - // Verify rg was never called + // The primary real command doubles as the availability check. const rgCalls = spawnMock.mock.calls.filter((call) => call[0] === 'rg') - expect(rgCalls.length).toBe(0) + expect(rgCalls.length).toBe(1) + expect(rgCalls.every((call) => !(call[1] as string[]).includes('--version'))).toBe(true) // Verify git ls-files was called const gitCalls = spawnMock.mock.calls.filter( @@ -402,13 +514,15 @@ describe('filesystem-list-files', () => { }) it('stops after primary Git files fill the result budget', async () => { - checkRgAvailableMock.mockResolvedValue(false) const revParseProc = createMockProcess() const gitP1 = createMockProcess() const gitP2 = createMockProcess() let callIndex = 0 spawnMock.mockImplementation((cmd: string, args: string[]) => { + if (cmd === 'rg') { + return createMissingRipgrepProcess() + } if (cmd === 'git' && args.includes('rev-parse')) { return revParseProc } @@ -439,10 +553,12 @@ describe('filesystem-list-files', () => { }) it('does not let a discarded Git directory placeholder consume the result budget', async () => { - checkRgAvailableMock.mockResolvedValue(false) const revParseProc = createMockProcess() const primary = createMockProcess() spawnMock.mockImplementation((cmd: string, args: string[]) => { + if (cmd === 'rg') { + return createMissingRipgrepProcess() + } if (cmd === 'git' && args.includes('rev-parse')) { return revParseProc } @@ -469,14 +585,15 @@ describe('filesystem-list-files', () => { }) it('git fallback applies hidden dir blocklist', async () => { - checkRgAvailableMock.mockResolvedValue(false) - const revParseProc = createMockProcess() const gitP1 = createMockProcess() const gitP2 = createMockProcess() let callIndex = 0 spawnMock.mockImplementation((cmd: string, args: string[]) => { + if (cmd === 'rg') { + return createMissingRipgrepProcess() + } if (cmd === 'git' && args.includes('rev-parse')) { return revParseProc } @@ -518,7 +635,6 @@ describe('filesystem-list-files', () => { }) it('settles and detaches git fallback scans that ignore timeout kills', async () => { - checkRgAvailableMock.mockResolvedValue(false) vi.useFakeTimers() try { @@ -528,6 +644,9 @@ describe('filesystem-list-files', () => { let callIndex = 0 spawnMock.mockImplementation((cmd: string, args: string[]) => { + if (cmd === 'rg') { + return createMissingRipgrepProcess() + } if (cmd === 'git' && args.includes('rev-parse')) { return revParseProc } @@ -541,13 +660,9 @@ describe('filesystem-list-files', () => { const storeMock = {} as unknown as Store const promise = listQuickOpenFiles('/mock/root', storeMock) - await Promise.resolve() - await Promise.resolve() - await Promise.resolve() + await flushMicrotasks() revParseProc.emit('close', 0, null) - await Promise.resolve() - await Promise.resolve() - await Promise.resolve() + await flushMicrotasks() ;(gitP1.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\0partial') @@ -567,7 +682,6 @@ describe('filesystem-list-files', () => { }) it('keeps primary results when only the ignored pass times out', async () => { - checkRgAvailableMock.mockResolvedValue(false) const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) vi.useFakeTimers() @@ -578,6 +692,9 @@ describe('filesystem-list-files', () => { let callIndex = 0 spawnMock.mockImplementation((cmd: string, args: string[]) => { + if (cmd === 'rg') { + return createMissingRipgrepProcess() + } if (cmd === 'git' && args.includes('rev-parse')) { return revParseProc } @@ -591,13 +708,9 @@ describe('filesystem-list-files', () => { const storeMock = {} as unknown as Store const promise = listQuickOpenFiles('/mock/root', storeMock) - await Promise.resolve() - await Promise.resolve() - await Promise.resolve() + await flushMicrotasks() revParseProc.emit('close', 0, null) - await Promise.resolve() - await Promise.resolve() - await Promise.resolve() + await flushMicrotasks() ;(gitP1.stdout as unknown as EventEmitter).emit( 'data', diff --git a/src/main/ipc/filesystem-list-files.ts b/src/main/ipc/filesystem-list-files.ts index 7edb688de72..c1666234cd0 100644 --- a/src/main/ipc/filesystem-list-files.ts +++ b/src/main/ipc/filesystem-list-files.ts @@ -17,6 +17,12 @@ import { import { isQuickOpenReaddirBudgetError } from '../../shared/quick-open-readdir-walk' import { buildInstallRgMessage } from '../../shared/quick-open-install-rg' import { listFilesWithGit } from './filesystem-list-files-git-fallback' +import { + absorbPendingRipgrepSpawnError, + isRipgrepUnavailableExit, + killSpawnedRipgrepProcess, + RipgrepUnavailableError +} from '../../shared/ripgrep-process-availability' export async function listQuickOpenFiles( rootPath: string, @@ -37,13 +43,9 @@ export async function listQuickOpenFiles( // every worktree instead of just the active one. The shared helper // normalizes, validates, and root-relativizes every input. const excludePathPrefixes = buildExcludePathPrefixes(authorizedRootPath, excludePaths) + const wslDistroForOutput = parseWslPath(authorizedRootPath)?.distro ?? localGitOptions.wslDistro - // Why: checking rg availability upfront avoids a race condition where - // spawn('rg') emits 'close' before 'error' on some platforms, causing - // the handler to resolve with empty results before the git fallback - // can run. - const rgAvailable = await checkRgAvailable(authorizedRootPath, localGitOptions.wslDistro) - if (!rgAvailable) { + const listWithoutRipgrep = async (): Promise => { try { return await listFilesWithGit( authorizedRootPath, @@ -59,6 +61,12 @@ export async function listQuickOpenFiles( throw new Error(await buildInstallRgMessage(err)) } } + if ( + wslDistroForOutput && + !(await checkRgAvailable(authorizedRootPath, localGitOptions.wslDistro)) + ) { + return listWithoutRipgrep() + } const files = new Set() const children: { @@ -68,8 +76,6 @@ export async function listQuickOpenFiles( }[] = [] // Why: WSL-routed rg can emit Linux-native absolute paths. UNC repos carry // their distro in the path; Windows-path repos carry it in project runtime. - const wslDistroForOutput = parseWslPath(authorizedRootPath)?.distro ?? localGitOptions.wslDistro - const { primary, ignoredPass } = buildRgArgsForQuickOpen({ // Why: rg evaluates root-relative exclude globs against cwd only when the // search target is cwd-relative. With an absolute target, `!packages/app` @@ -86,6 +92,8 @@ export async function listQuickOpenFiles( let buf = '' let done = false let parseablePathCount = 0 + let processErrorObserved = false + let unavailableExitObserved = false const processLine = (rawLine: string): boolean => { const translated = @@ -138,12 +146,27 @@ export async function listQuickOpenFiles( /* drain */ } const handleError = (): void => { + processErrorObserved = true // Why: treat spawn errors like an abnormal exit — discard residual // buffer so a truncated final byte sequence cannot leak as a path. buf = '' + if (isRipgrepUnavailableExit(child, null, null)) { + finish(new RipgrepUnavailableError()) + return + } finish(new Error('rg failed to start')) } const handleClose = (code: number | null, signal: NodeJS.Signals | null): void => { + if ( + isRipgrepUnavailableExit(child, code, signal, { + classifyNativeLauncherExit: !wslDistroForOutput + }) + ) { + unavailableExitObserved = true + buf = '' + finish(new RipgrepUnavailableError()) + return + } if (signal) { // Why: a signal exit means timeout/OOM/external kill. Returning the // already-streamed prefix would recreate the false-empty bug this @@ -179,6 +202,10 @@ export async function listQuickOpenFiles( child.stderr!.off('data', handleStderrData) child.off('error', handleError) child.off('close', handleClose) + absorbPendingRipgrepSpawnError(child, { + errorObserved: processErrorObserved, + unavailableExitObserved + }) if (err) { reject(err) } else { @@ -197,7 +224,7 @@ export async function listQuickOpenFiles( // Why: on timeout, the buffer is likely truncated mid-path. Discard // it so Quick Open never displays a malformed entry. buf = '' - child.kill() + killSpawnedRipgrepProcess(child) finish(new Error('rg list timed out')) }, 10000) }) @@ -213,7 +240,7 @@ export async function listQuickOpenFiles( } entry.finish() if (entry.child.exitCode === null && entry.child.signalCode === null) { - entry.child.kill() + killSpawnedRipgrepProcess(entry.child) } } } @@ -225,24 +252,30 @@ export async function listQuickOpenFiles( } entry.finish() if (entry.child.exitCode === null && entry.child.signalCode === null) { - entry.child.kill() + killSpawnedRipgrepProcess(entry.child) } } } - try { + const primaryRun = runRg(primary) if (maxResults === undefined) { - await Promise.all([runRg(primary), runRg(ignoredPass)]) + // Why: a pid-less primary proves launch failure; avoid doubling the failed spawn. + await (children[0]?.child.pid === undefined + ? primaryRun + : Promise.all([primaryRun, runRg(ignoredPass)])) } else { // Why: ignored-file output can be much larger and faster than the primary // pass; let source files claim the bounded autocomplete budget first. - await runRg(primary) + await primaryRun if (files.size < maxResults) { await runRg(ignoredPass) } } } catch (err) { killSurvivors() + if (err instanceof RipgrepUnavailableError) { + return listWithoutRipgrep() + } throw err } return Array.from(files).slice(0, maxResults) diff --git a/src/main/ipc/filesystem-search-rg-timeout.test.ts b/src/main/ipc/filesystem-search-rg-timeout.test.ts index 8d9881b2261..9e0a259d2d3 100644 --- a/src/main/ipc/filesystem-search-rg-timeout.test.ts +++ b/src/main/ipc/filesystem-search-rg-timeout.test.ts @@ -7,6 +7,7 @@ const { resolveAuthorizedPathMock, checkRgAvailableMock, getLocalGitOptionsForRegisteredWorktreeMock, + searchWithGitGrepMock, wslAwareSpawnMock, toWindowsWslPathMock } = vi.hoisted(() => ({ @@ -14,6 +15,7 @@ const { resolveAuthorizedPathMock: vi.fn(), checkRgAvailableMock: vi.fn(), getLocalGitOptionsForRegisteredWorktreeMock: vi.fn(), + searchWithGitGrepMock: vi.fn(), wslAwareSpawnMock: vi.fn(), toWindowsWslPathMock: vi.fn((value: string) => value) })) @@ -56,7 +58,7 @@ vi.mock('./filesystem-mutations', () => ({ })) vi.mock('./filesystem-search-git', () => ({ - searchWithGitGrep: vi.fn() + searchWithGitGrep: searchWithGitGrepMock })) vi.mock('./local-worktree-runtime-options', () => ({ @@ -87,6 +89,12 @@ function createMockProcess(): ChildProcess { return p } +async function flushMicrotasks(): Promise { + for (let index = 0; index < 8; index++) { + await Promise.resolve() + } +} + describe('filesystem rg search timeout', () => { beforeEach(() => { handlers.clear() @@ -120,6 +128,7 @@ describe('filesystem rg search timeout', () => { const result = await promise expect(result.truncated).toBe(true) + expect(checkRgAvailableMock).not.toHaveBeenCalled() expect(child.kill).toHaveBeenCalled() expect((child.stdout as unknown as EventEmitter).listenerCount('data')).toBe(0) expect((child.stderr as unknown as EventEmitter).listenerCount('data')).toBe(0) @@ -130,8 +139,77 @@ describe('filesystem rg search timeout', () => { } }) + it.each(['error-first', 'close-first'] as const)( + 'falls back once when a native launch failure is %s', + async (order) => { + const child = createMockProcess() + Object.defineProperty(child, 'pid', { value: undefined }) + wslAwareSpawnMock.mockReturnValue(child) + const fallback = { files: [], totalMatches: 0, truncated: false } + searchWithGitGrepMock.mockResolvedValue(fallback) + registerFilesystemHandlers({} as never) + + const promise = handlers.get('fs:search')!( + { sender: { id: 7 } }, + { rootPath: '/repo', query: 'ok' } + ) as Promise + await flushMicrotasks() + const error = Object.assign(new Error('spawn rg ENOENT'), { code: 'ENOENT' }) + if (order === 'error-first') { + expect(() => child.emit('error', error)).not.toThrow() + child.emit('close', -2, null) + } else { + child.emit('close', -2, null) + expect(() => child.emit('error', error)).not.toThrow() + } + + await expect(promise).resolves.toBe(fallback) + expect(searchWithGitGrepMock).toHaveBeenCalledTimes(1) + expect(checkRgAvailableMock).not.toHaveBeenCalled() + expect(child.listenerCount('error')).toBe(0) + expect(child.listenerCount('close')).toBe(0) + } + ) + + it('keeps post-spawn errors on the existing empty-result path', async () => { + const child = createMockProcess() + Object.defineProperty(child, 'pid', { value: 1 }) + wslAwareSpawnMock.mockReturnValue(child) + registerFilesystemHandlers({} as never) + + const promise = handlers.get('fs:search')!( + { sender: { id: 7 } }, + { rootPath: '/repo', query: 'ok' } + ) as Promise<{ files: unknown[] }> + await flushMicrotasks() + child.emit('error', new Error('post-spawn failure')) + + await expect(promise).resolves.toMatchObject({ files: [] }) + expect(searchWithGitGrepMock).not.toHaveBeenCalled() + }) + + it("falls back when a native launcher exits outside ripgrep's contract", async () => { + const child = createMockProcess() + Object.defineProperty(child, 'pid', { value: 1 }) + wslAwareSpawnMock.mockReturnValue(child) + const fallback = { files: [], totalMatches: 0, truncated: false } + searchWithGitGrepMock.mockResolvedValue(fallback) + registerFilesystemHandlers({} as never) + + const promise = handlers.get('fs:search')!( + { sender: { id: 7 } }, + { rootPath: '/repo', query: 'ok' } + ) as Promise + await flushMicrotasks() + child.emit('close', 127, null) + + await expect(promise).resolves.toBe(fallback) + expect(searchWithGitGrepMock).toHaveBeenCalledTimes(1) + }) + it('routes rg through the registered WSL project runtime for Windows-path worktrees', async () => { const child = createMockProcess() + Object.defineProperty(child, 'pid', { value: 1 }) wslAwareSpawnMock.mockReturnValue(child) getLocalGitOptionsForRegisteredWorktreeMock.mockReturnValue({ wslDistro: 'Ubuntu' }) registerFilesystemHandlers({} as never) @@ -142,12 +220,13 @@ describe('filesystem rg search timeout', () => { ) as Promise setTimeout(() => { - child.emit('close') + child.emit('close', 127, null) }, 10) await promise expect(checkRgAvailableMock).toHaveBeenCalledWith('C:\\repo', 'Ubuntu') + expect(searchWithGitGrepMock).not.toHaveBeenCalled() expect(wslAwareSpawnMock).toHaveBeenCalledWith( 'rg', expect.any(Array), @@ -158,6 +237,23 @@ describe('filesystem rg search timeout', () => { ) }) + it('keeps the WSL search preflight and falls back before starting real rg', async () => { + const fallback = { files: [], totalMatches: 0, truncated: false } + getLocalGitOptionsForRegisteredWorktreeMock.mockReturnValue({ wslDistro: 'Ubuntu' }) + checkRgAvailableMock.mockResolvedValue(false) + searchWithGitGrepMock.mockResolvedValue(fallback) + registerFilesystemHandlers({} as never) + + const promise = handlers.get('fs:search')!( + { sender: { id: 7 } }, + { rootPath: 'C:\\repo', query: 'ok' } + ) as Promise + + await expect(promise).resolves.toBe(fallback) + expect(checkRgAvailableMock).toHaveBeenCalledWith('C:\\repo', 'Ubuntu') + expect(wslAwareSpawnMock).not.toHaveBeenCalled() + }) + it('translates WSL rg output for Windows-path project search results', async () => { const child = createMockProcess() wslAwareSpawnMock.mockReturnValue(child) diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index 4ca9547b77c..d5fb98086f8 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -112,6 +112,11 @@ import { } from './source-control-ai-linked-issue' import { listMarkdownDocuments, markdownDocumentsFromRelativePaths } from './markdown-documents' import { checkRgAvailable } from './rg-availability' +import { + absorbPendingRipgrepSpawnError, + isRipgrepUnavailableExit, + killSpawnedRipgrepProcess +} from '../../shared/ripgrep-process-availability' import { getSshFilesystemProvider, requireSshFilesystemProvider @@ -980,32 +985,35 @@ export function registerFilesystemHandlers( Math.min(args.maxResults ?? DEFAULT_SEARCH_MAX_RESULTS, DEFAULT_SEARCH_MAX_RESULTS) ) const searchKey = `${event.sender.id}:${rootPath}` + // Why: WSL's bash exit 127 is ambiguous with a real executable returning 127. + const wslDistroForOutput = parseWslPath(rootPath)?.distro ?? localGitOptions.wslDistro - // Why: probe rg upfront; on some platforms spawn emits 'close' before 'error', resolving empty before the git-grep fallback runs. - const rgAvailable = await checkRgAvailable(rootPath, localGitOptions.wslDistro) - if (!rgAvailable) { + if (wslDistroForOutput && !(await checkRgAvailable(rootPath, localGitOptions.wslDistro))) { return searchWithGitGrep(rootPath, args, maxResults, localGitOptions) } - return new Promise((resolvePromise) => { + return new Promise((resolvePromise) => { const rgArgs = buildRgArgs(args.query, rootPath, args) // Why: kill the prior rg so it stops parsing thousands of matches on the main thread (the large-repo freeze) after the UI moved on. - activeTextSearches.get(searchKey)?.kill() + const previousChild = activeTextSearches.get(searchKey) + if (previousChild) { + killSpawnedRipgrepProcess(previousChild) + } const acc = createAccumulator() let stdoutBuffer = '' let resolved = false + let processErrorObserved = false + let unavailableExitObserved = false let child: ChildProcess | null = null let killTimeout: ReturnType - // Why: WSL-routed rg emits Linux paths; UNC repos carry the distro in the path, Windows-path repos in project runtime. - const wslDistroForOutput = parseWslPath(rootPath)?.distro ?? localGitOptions.wslDistro const transformAbsPath = wslDistroForOutput ? (p: string): string => (p.startsWith('/') ? toWindowsWslPath(p, wslDistroForOutput) : p) : undefined - const resolveOnce = (): void => { + const finish = (result: SearchResult | PromiseLike): void => { if (resolved) { return } @@ -1019,13 +1027,22 @@ export function registerFilesystemHandlers( child?.stderr?.off('data', handleStderrData) child?.off('error', handleError) child?.off('close', handleClose) - resolvePromise(finalize(acc)) + if (child) { + absorbPendingRipgrepSpawnError(child, { + errorObserved: processErrorObserved, + unavailableExitObserved + }) + } + resolvePromise(result) } + const resolveOnce = (): void => finish(finalize(acc)) + const resolveWithoutRipgrep = (): void => + finish(searchWithGitGrep(rootPath, args, maxResults, localGitOptions)) const processLine = (line: string): void => { const verdict = ingestRgJsonLine(line, rootPath, acc, maxResults, transformAbsPath) - if (verdict === 'stop') { - child?.kill() + if (verdict === 'stop' && child) { + killSpawnedRipgrepProcess(child) } } @@ -1049,9 +1066,24 @@ export function registerFilesystemHandlers( // Drain stderr so rg cannot block on a full pipe. } const handleError = (): void => { + processErrorObserved = true + if (child && isRipgrepUnavailableExit(child, null, null)) { + resolveWithoutRipgrep() + return + } resolveOnce() } - const handleClose = (): void => { + const handleClose = (code: number | null, signal: NodeJS.Signals | null): void => { + if ( + child && + isRipgrepUnavailableExit(child, code, signal, { + classifyNativeLauncherExit: !wslDistroForOutput + }) + ) { + unavailableExitObserved = true + resolveWithoutRipgrep() + return + } if (stdoutBuffer) { processLine(stdoutBuffer) } @@ -1067,7 +1099,9 @@ export function registerFilesystemHandlers( // Why: timeout kills the child mid-scan; mark truncated so the UI shows incomplete results. killTimeout = setTimeout(() => { acc.truncated = true - child?.kill() + if (child) { + killSpawnedRipgrepProcess(child) + } resolveOnce() }, SEARCH_TIMEOUT_MS) }) diff --git a/src/main/runtime/orca-runtime-files.test.ts b/src/main/runtime/orca-runtime-files.test.ts index e5c8a18479e..ce96eddb5e0 100644 --- a/src/main/runtime/orca-runtime-files.test.ts +++ b/src/main/runtime/orca-runtime-files.test.ts @@ -22,6 +22,7 @@ const { closeWatcherInWatcherProcessMock, checkRgAvailableMock, getLocalGitOptionsForRegisteredWorktreeMock, + searchWithGitGrepMock, wslAwareSpawnMock, watchMock } = vi.hoisted(() => ({ @@ -35,6 +36,7 @@ const { statMock: vi.fn(), watchInWatcherProcessMock: vi.fn(), closeWatcherInWatcherProcessMock: vi.fn(), + searchWithGitGrepMock: vi.fn(), wslAwareSpawnMock: vi.fn(), watchMock: vi.fn() })) @@ -91,6 +93,10 @@ vi.mock('../ipc/local-worktree-runtime-options', () => ({ getLocalGitOptionsForRegisteredWorktree: getLocalGitOptionsForRegisteredWorktreeMock })) +vi.mock('../ipc/filesystem-search-git', () => ({ + searchWithGitGrep: searchWithGitGrepMock +})) + vi.mock('../providers/ssh-filesystem-dispatch', () => ({ getSshFilesystemProvider: vi.fn(), onSshFilesystemProviderRegistered: () => () => undefined, @@ -202,6 +208,12 @@ function createRuntimeSearchChild(): MockRuntimeSearchChild { return child } +async function flushRuntimeSearchMicrotasks(): Promise { + for (let index = 0; index < 8; index++) { + await Promise.resolve() + } +} + describe('RuntimeFileCommands', () => { const originalPlatform = process.platform @@ -217,6 +229,7 @@ describe('RuntimeFileCommands', () => { closeWatcherInWatcherProcessMock.mockReset() watchMock.mockReset() checkRgAvailableMock.mockReset() + searchWithGitGrepMock.mockReset() vi.mocked(getSshFilesystemProvider).mockReset() resetSshConnectionGenerations() getLocalGitOptionsForRegisteredWorktreeMock.mockReset() @@ -754,6 +767,68 @@ describe('RuntimeFileCommands', () => { expect(child.stderr.listenerCount('data')).toBe(0) expect(child.listenerCount('error')).toBe(0) expect(child.listenerCount('close')).toBe(0) + expect(checkRgAvailableMock).not.toHaveBeenCalled() + }) + + it.each(['error-first', 'close-first'] as const)( + 'falls back once when runtime rg native launch failure is %s', + async (order) => { + const resolveRuntimeFileTarget = vi.fn(async () => ({ + worktree: { id: 'wt-1', repoId: 'repo-1', path: '/repo' }, + connectionId: null + })) + const { commands } = createRuntimeFileCommands({ resolveRuntimeFileTarget }) + const child = createRuntimeSearchChild() + Object.defineProperty(child, 'pid', { value: undefined }) + resolveAuthorizedPathMock.mockResolvedValue('/repo') + wslAwareSpawnMock.mockReturnValue(child) + const fallback = { files: [], totalMatches: 0, truncated: false } + searchWithGitGrepMock.mockResolvedValue(fallback) + + const resultPromise = commands.searchRuntimeFiles('id:wt-1', { + query: 'needle', + maxResults: 10 + }) + await flushRuntimeSearchMicrotasks() + const error = Object.assign(new Error('spawn rg ENOENT'), { code: 'ENOENT' }) + if (order === 'error-first') { + expect(() => child.emit('error', error)).not.toThrow() + child.emit('close', -2, null) + } else { + child.emit('close', -2, null) + expect(() => child.emit('error', error)).not.toThrow() + } + + await expect(resultPromise).resolves.toBe(fallback) + expect(searchWithGitGrepMock).toHaveBeenCalledTimes(1) + expect(checkRgAvailableMock).not.toHaveBeenCalled() + expect(child.listenerCount('error')).toBe(0) + expect(child.listenerCount('close')).toBe(0) + } + ) + + it("falls back when a runtime native launcher exits outside ripgrep's contract", async () => { + const resolveRuntimeFileTarget = vi.fn(async () => ({ + worktree: { id: 'wt-1', repoId: 'repo-1', path: '/repo' }, + connectionId: null + })) + const { commands } = createRuntimeFileCommands({ resolveRuntimeFileTarget }) + const child = createRuntimeSearchChild() + Object.defineProperty(child, 'pid', { value: 1 }) + resolveAuthorizedPathMock.mockResolvedValue('/repo') + wslAwareSpawnMock.mockReturnValue(child) + const fallback = { files: [], totalMatches: 0, truncated: false } + searchWithGitGrepMock.mockResolvedValue(fallback) + + const resultPromise = commands.searchRuntimeFiles('id:wt-1', { + query: 'needle', + maxResults: 10 + }) + await flushRuntimeSearchMicrotasks() + child.emit('close', 127, null) + + await expect(resultPromise).resolves.toBe(fallback) + expect(searchWithGitGrepMock).toHaveBeenCalledTimes(1) }) it('routes runtime rg searches through the registered WSL project runtime', async () => { @@ -767,6 +842,7 @@ describe('RuntimeFileCommands', () => { })) const { commands, store } = createRuntimeFileCommands({ resolveRuntimeFileTarget }) const child = createRuntimeSearchChild() + Object.defineProperty(child, 'pid', { value: 1 }) resolveAuthorizedPathMock.mockResolvedValue('C:\\repo') checkRgAvailableMock.mockResolvedValue(true) getLocalGitOptionsForRegisteredWorktreeMock.mockReturnValue({ wslDistro: 'Ubuntu' }) @@ -779,7 +855,7 @@ describe('RuntimeFileCommands', () => { await Promise.resolve() await Promise.resolve() await Promise.resolve() - child.emit('close') + child.emit('close', 127, null) await expect(resultPromise).resolves.toMatchObject({ files: [] }) expect(getLocalGitOptionsForRegisteredWorktreeMock).toHaveBeenCalledWith( @@ -788,6 +864,7 @@ describe('RuntimeFileCommands', () => { 'C:\\repo' ) expect(checkRgAvailableMock).toHaveBeenCalledWith('C:\\repo', 'Ubuntu') + expect(searchWithGitGrepMock).not.toHaveBeenCalled() expect(wslAwareSpawnMock).toHaveBeenCalledWith( 'rg', expect.any(Array), @@ -798,6 +875,25 @@ describe('RuntimeFileCommands', () => { ) }) + it('keeps the runtime WSL preflight and falls back before starting real rg', async () => { + const resolveRuntimeFileTarget = vi.fn(async () => ({ + worktree: { id: 'wt-1', repoId: 'repo-1', path: 'C:\\repo' }, + connectionId: null + })) + const { commands } = createRuntimeFileCommands({ resolveRuntimeFileTarget }) + const fallback = { files: [], totalMatches: 0, truncated: false } + resolveAuthorizedPathMock.mockResolvedValue('C:\\repo') + getLocalGitOptionsForRegisteredWorktreeMock.mockReturnValue({ wslDistro: 'Ubuntu' }) + checkRgAvailableMock.mockResolvedValue(false) + searchWithGitGrepMock.mockResolvedValue(fallback) + + await expect( + commands.searchRuntimeFiles('id:wt-1', { query: 'needle', maxResults: 10 }) + ).resolves.toBe(fallback) + expect(checkRgAvailableMock).toHaveBeenCalledWith('C:\\repo', 'Ubuntu') + expect(wslAwareSpawnMock).not.toHaveBeenCalled() + }) + describe('resolveTerminalPath', () => { let tempDirs: string[] = [] diff --git a/src/main/runtime/orca-runtime-files.ts b/src/main/runtime/orca-runtime-files.ts index 68000f1b297..f85bff8abde 100644 --- a/src/main/runtime/orca-runtime-files.ts +++ b/src/main/runtime/orca-runtime-files.ts @@ -58,6 +58,11 @@ import { listQuickOpenFiles } from '../ipc/filesystem-list-files' import { searchWithGitGrep } from '../ipc/filesystem-search-git' import { getLocalGitOptionsForRegisteredWorktree } from '../ipc/local-worktree-runtime-options' import { checkRgAvailable } from '../ipc/rg-availability' +import { + absorbPendingRipgrepSpawnError, + isRipgrepUnavailableExit, + killSpawnedRipgrepProcess +} from '../../shared/ripgrep-process-availability' import { listMarkdownDocuments, markdownDocumentsFromRelativePaths @@ -1920,26 +1925,33 @@ export class RuntimeFileCommands { 1, Math.min(options.maxResults ?? DEFAULT_SEARCH_MAX_RESULTS, DEFAULT_SEARCH_MAX_RESULTS) ) - const rgAvailable = await checkRgAvailable(authorizedRootPath, localGitOptions.wslDistro) - if (!rgAvailable) { + const wslInfo = parseWslPath(authorizedRootPath) + if ( + (wslInfo || localGitOptions.wslDistro) && + !(await checkRgAvailable(authorizedRootPath, localGitOptions.wslDistro)) + ) { return searchWithGitGrep(authorizedRootPath, options, maxResults, localGitOptions) } - return new Promise((resolvePromise) => { + return new Promise((resolvePromise) => { const searchKey = `${this.host.getRuntimeId()}:${authorizedRootPath}` const rgArgs = buildRgArgs(options.query, authorizedRootPath, options) - this.activeRuntimeTextSearches.get(searchKey)?.kill() + const previousChild = this.activeRuntimeTextSearches.get(searchKey) + if (previousChild) { + killSpawnedRipgrepProcess(previousChild) + } const acc = createAccumulator() let stdoutBuffer = '' let resolved = false + let processErrorObserved = false + let unavailableExitObserved = false let child: ChildProcess | null = null - const wslInfo = parseWslPath(authorizedRootPath) const transformAbsPath = wslInfo ? (p: string): string => toWindowsWslPath(p, wslInfo.distro) : undefined - const resolveOnce = (): void => { + const finish = (result: SearchResult | PromiseLike): void => { if (resolved) { return } @@ -1948,8 +1960,11 @@ export class RuntimeFileCommands { this.activeRuntimeTextSearches.delete(searchKey) } cleanupListeners() - resolvePromise(finalize(acc)) + resolvePromise(result) } + const resolveOnce = (): void => finish(finalize(acc)) + const resolveWithoutRipgrep = (): void => + finish(searchWithGitGrep(authorizedRootPath, options, maxResults, localGitOptions)) let killTimeout: ReturnType | null = null const cleanupListeners = (): void => { @@ -1961,6 +1976,12 @@ export class RuntimeFileCommands { child?.stderr?.off('data', onStderrData) child?.off('error', onError) child?.off('close', onClose) + if (child) { + absorbPendingRipgrepSpawnError(child, { + errorObserved: processErrorObserved, + unavailableExitObserved + }) + } } const processLine = (line: string): void => { @@ -1971,8 +1992,8 @@ export class RuntimeFileCommands { maxResults, transformAbsPath ) - if (verdict === 'stop') { - child?.kill() + if (verdict === 'stop' && child) { + killSpawnedRipgrepProcess(child) } } @@ -1996,8 +2017,25 @@ export class RuntimeFileCommands { const onStderrData = (): void => { // Drain stderr so rg cannot block on a full pipe. } - const onError = (): void => resolveOnce() - const onClose = (): void => { + const onError = (): void => { + processErrorObserved = true + if (child && isRipgrepUnavailableExit(child, null, null)) { + resolveWithoutRipgrep() + return + } + resolveOnce() + } + const onClose = (code: number | null, signal: NodeJS.Signals | null): void => { + if ( + child && + isRipgrepUnavailableExit(child, code, signal, { + classifyNativeLauncherExit: !(wslInfo || localGitOptions.wslDistro) + }) + ) { + unavailableExitObserved = true + resolveWithoutRipgrep() + return + } if (stdoutBuffer) { processLine(stdoutBuffer) } @@ -2011,7 +2049,9 @@ export class RuntimeFileCommands { killTimeout = setTimeout(() => { acc.truncated = true - child?.kill() + if (child) { + killSpawnedRipgrepProcess(child) + } resolveOnce() }, SEARCH_TIMEOUT_MS) }) diff --git a/src/relay/fs-handler-list-files-cancel.test.ts b/src/relay/fs-handler-list-files-cancel.test.ts index 02cd8da6afc..baf6101600c 100644 --- a/src/relay/fs-handler-list-files-cancel.test.ts +++ b/src/relay/fs-handler-list-files-cancel.test.ts @@ -31,6 +31,7 @@ function createMockProcess(): ChildProcess { ;(p as unknown as Record).kill = vi.fn() ;(p as unknown as Record).exitCode = null ;(p as unknown as Record).signalCode = null + Object.defineProperty(p, 'pid', { configurable: true, value: 1 }) return p } diff --git a/src/relay/fs-handler-list-files-ignored.test.ts b/src/relay/fs-handler-list-files-ignored.test.ts index 1ca5c2f5327..b819d3ac85c 100644 --- a/src/relay/fs-handler-list-files-ignored.test.ts +++ b/src/relay/fs-handler-list-files-ignored.test.ts @@ -16,6 +16,11 @@ import { tmpdir } from 'node:os' import { listFilesWithGit } from './fs-handler-git-fallback' import { listFilesWithRg } from './fs-handler-list-files' import { searchWithRg } from './fs-handler-utils' +import { RipgrepUnavailableError } from '../shared/ripgrep-process-availability' +import { + ListFilesScanCoordinator, + LIST_FILES_SUPERSEDED_MESSAGE +} from './fs-list-files-scan-coordinator' const tempDirs: string[] = [] const SHA1 = '0123456789abcdef0123456789abcdef01234567' @@ -36,6 +41,7 @@ function createMockProcess(): ChildProcess { ;(p as unknown as Record).kill = vi.fn() ;(p as unknown as Record).exitCode = null ;(p as unknown as Record).signalCode = null + Object.defineProperty(p, 'pid', { configurable: true, value: 1 }) return p } @@ -72,6 +78,7 @@ describe('relay quick open ignored file listing', () => { }) const promise = listFilesWithRg('/remote/root', ['packages/other']) + expect(spawnMock).toHaveBeenCalledTimes(2) setTimeout(() => { ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\n') @@ -115,6 +122,82 @@ describe('relay quick open ignored file listing', () => { expect(callIndex).toBe(1) }) + it.each(['error-first', 'close-first'] as const)( + 'tags a %s pre-spawn listing failure without starting the ignored pass', + async (order) => { + const root = await makeTempRoot() + const missing = createMockProcess() + Object.defineProperty(missing, 'pid', { value: undefined }) + spawnMock.mockReturnValue(missing) + const error = Object.assign(new Error('spawn rg ENOENT'), { code: 'ENOENT' }) + + const promise = listFilesWithRg(root) + if (order === 'error-first') { + expect(() => missing.emit('error', error)).not.toThrow() + } else { + missing.emit('close', -2, null) + } + + await expect(promise).rejects.toBeInstanceOf(RipgrepUnavailableError) + if (order === 'error-first') { + missing.emit('close', -2, null) + } else { + expect(() => missing.emit('error', error)).not.toThrow() + } + expect(spawnMock).toHaveBeenCalledTimes(1) + expect(missing.listenerCount('error')).toBe(0) + expect(missing.listenerCount('close')).toBe(0) + } + ) + + it('kills only the admitted pass when ignored rg fails before spawn', async () => { + const root = await makeTempRoot() + const primary = createMockProcess() + const missingIgnored = createMockProcess() + Object.defineProperty(missingIgnored, 'pid', { value: undefined }) + spawnMock.mockImplementation((_cmd: string, args: string[]) => + args.includes('--no-ignore-vcs') ? missingIgnored : primary + ) + + const promise = listFilesWithRg(root) + expect(spawnMock).toHaveBeenCalledTimes(2) + missingIgnored.emit('close', -2, null) + + await expect(promise).rejects.toBeInstanceOf(RipgrepUnavailableError) + expect(primary.kill).toHaveBeenCalled() + expect(missingIgnored.kill).not.toHaveBeenCalled() + const error = Object.assign(new Error('spawn rg ENOENT'), { code: 'ENOENT' }) + expect(() => missingIgnored.emit('error', error)).not.toThrow() + expect(missingIgnored.listenerCount('error')).toBe(0) + }) + + it('does not signal failed-spawn passes when a same-client scan is superseded', async () => { + const root = await makeTempRoot() + const firstChild = createMockProcess() + Object.defineProperty(firstChild, 'pid', { value: undefined }) + spawnMock.mockReturnValue(firstChild) + const coordinator = new ListFilesScanCoordinator() + const first = coordinator.run({ + clientId: 1, + key: 'first', + start: (signal) => listFilesWithRg(root, [], { signal }) + }) + const firstOutcome = first.catch((error: unknown) => error) + + const second = coordinator.run({ + clientId: 1, + key: 'second', + start: async () => ['second.ts'] + }) + + await expect(firstOutcome).resolves.toMatchObject({ message: LIST_FILES_SUPERSEDED_MESSAGE }) + await expect(second).resolves.toEqual(['second.ts']) + expect(firstChild.kill).not.toHaveBeenCalled() + expect(spawnMock).toHaveBeenCalledTimes(1) + const error = Object.assign(new Error('spawn rg ENOENT'), { code: 'ENOENT' }) + expect(() => firstChild.emit('error', error)).not.toThrow() + }) + it('git fallback ignored pass includes ignored non-env files', async () => { const root = await makeTempRoot() await writeRel(root, 'dist/generated.js') @@ -430,4 +513,101 @@ describe('relay quick open ignored file listing', () => { vi.useRealTimers() } }) + + it.each(['error-first', 'close-first'] as const)( + 'tags only a %s pre-spawn rg search failure as unavailable', + async (order) => { + const root = await makeTempRoot() + const missing = createMockProcess() + Object.defineProperty(missing, 'pid', { value: undefined }) + spawnMock.mockReturnValueOnce(missing) + const unavailable = searchWithRg(root, 'ok', { maxResults: 100 }) + const error = Object.assign(new Error('spawn rg ENOENT'), { code: 'ENOENT' }) + if (order === 'error-first') { + expect(() => missing.emit('error', error)).not.toThrow() + missing.emit('close', -2, null) + } else { + missing.emit('close', -2, null) + expect(() => missing.emit('error', error)).not.toThrow() + } + + await expect(unavailable).rejects.toBeInstanceOf(RipgrepUnavailableError) + expect(missing.listenerCount('error')).toBe(0) + expect(missing.listenerCount('close')).toBe(0) + } + ) + + it('tags unsupported native launcher exits as unavailable', async () => { + const root = await makeTempRoot() + const child = createMockProcess() + Object.defineProperty(child, 'pid', { value: 1 }) + spawnMock.mockReturnValueOnce(child) + const unavailable = searchWithRg(root, 'ok', { maxResults: 100 }) + + child.emit('close', 127, null) + + await expect(unavailable).rejects.toBeInstanceOf(RipgrepUnavailableError) + }) + + it('keeps missing-root launch errors on their prior non-fallback paths', async () => { + const missingRoot = await makeTempRoot() + await rm(missingRoot, { recursive: true, force: true }) + const listFirst = createMockProcess() + const listProbe = createMockProcess() + Object.defineProperty(listFirst, 'pid', { value: undefined }) + Object.defineProperty(listProbe, 'pid', { value: 1 }) + let callIndex = 0 + spawnMock.mockImplementation(() => [listFirst, listProbe][callIndex++]) + const listError = Object.assign(new Error('spawn rg ENOENT'), { code: 'ENOENT' }) + const listing = listFilesWithRg(missingRoot) + listFirst.emit('error', listError) + + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(2)) + expect(spawnMock.mock.calls[1]).toEqual(['rg', ['--version'], { stdio: 'ignore' }]) + listProbe.emit('close', 0, null) + await expect(listing).rejects.toBe(listError) + + spawnMock.mockReset() + const searchChild = createMockProcess() + const searchProbe = createMockProcess() + Object.defineProperty(searchChild, 'pid', { value: undefined }) + Object.defineProperty(searchProbe, 'pid', { value: 1 }) + spawnMock.mockReturnValueOnce(searchChild).mockReturnValueOnce(searchProbe) + const search = searchWithRg(missingRoot, 'ok', { maxResults: 100 }) + searchChild.emit('error', listError) + + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(2)) + searchProbe.emit('close', 0, null) + await expect(search).resolves.toMatchObject({ files: [], totalMatches: 0 }) + }) + + it('keeps missing-rg precedence when the root also disappeared', async () => { + const missingRoot = await makeTempRoot() + await rm(missingRoot, { recursive: true, force: true }) + const first = createMockProcess() + const probe = createMockProcess() + for (const child of [first, probe]) { + Object.defineProperty(child, 'pid', { value: undefined }) + } + let callIndex = 0 + spawnMock.mockImplementation(() => [first, probe][callIndex++]) + const error = Object.assign(new Error('spawn rg ENOENT'), { code: 'ENOENT' }) + const listing = listFilesWithRg(missingRoot) + first.emit('error', error) + + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(2)) + probe.emit('close', -2, null) + + await expect(listing).rejects.toBeInstanceOf(RipgrepUnavailableError) + expect(() => probe.emit('error', error)).not.toThrow() + }) + + it('keeps post-spawn rg search errors on the existing empty-result path', async () => { + const started = createMockProcess() + Object.defineProperty(started, 'pid', { value: 1 }) + spawnMock.mockReturnValueOnce(started) + const ordinaryFailure = searchWithRg('/remote/root', 'ok', { maxResults: 100 }) + started.emit('error', new Error('post-spawn failure')) + await expect(ordinaryFailure).resolves.toMatchObject({ files: [], totalMatches: 0 }) + }) }) diff --git a/src/relay/fs-handler-list-files.ts b/src/relay/fs-handler-list-files.ts index c63a352786e..3ec7b99026d 100644 --- a/src/relay/fs-handler-list-files.ts +++ b/src/relay/fs-handler-list-files.ts @@ -1,7 +1,5 @@ /** * Ripgrep-based file listing for Quick Open. - * Extracted from fs-handler-utils.ts to keep it under 300 lines (oxlint max-lines). - * * Why a full rewrite vs. the older execFile+maxBuffer version: on a home-dir * worktree over SSH, rg descended into every dotfile cache, hit the timeout, * and silently resolved with a partial list — Quick Open then showed "No @@ -23,6 +21,13 @@ import { shouldExcludeQuickOpenRelPath, shouldIncludeQuickOpenPath } from '../shared/quick-open-filter' +import { + absorbPendingRipgrepSpawnError, + isRipgrepUnavailableAfterLaunchFailure, + isRipgrepUnavailableExit, + killSpawnedRipgrepProcess, + RipgrepUnavailableError +} from '../shared/ripgrep-process-availability' export const LIST_FILES_TIMEOUT_MS = 25_000 @@ -78,6 +83,9 @@ export function listFilesWithRg( let passBuf = '' let passDone = false let passFileCount = 0 + let processErrorObserved = false + let unavailableExitObserved = false + let launchFailureCheck: Promise | null = null // --no-messages: permission-denied noise on the remote (e.g. .ssh, // root-owned mounts) would otherwise flood stderr. // cwd: rootPath — root-relative exclude globs like `!packages/app/**` @@ -98,6 +106,10 @@ export function listFilesWithRg( child.stderr!.off('data', handleStderrData) child.off('error', handleError) child.off('close', handleClose) + absorbPendingRipgrepSpawnError(child, { + errorObserved: processErrorObserved, + unavailableExitObserved + }) } const rejectPass = (error: Error): void => { if (passDone) { @@ -116,6 +128,16 @@ export function listFilesWithRg( cleanup() passResolve() } + const rejectLaunchFailure = (error: Error): void => { + if (launchFailureCheck) { + return + } + launchFailureCheck = isRipgrepUnavailableAfterLaunchFailure(rootPath).then( + (unavailable) => { + rejectPass(unavailable ? new RipgrepUnavailableError() : error) + } + ) + } children.push({ child, isDone: () => passDone, @@ -125,7 +147,7 @@ export function listFilesWithRg( timer = setTimeout(() => { // Discard residual buffer on abnormal exit — a truncated byte // sequence could look like a valid path. - child.kill() + killSpawnedRipgrepProcess(child) rejectPass(new Error('rg list timed out')) }, LIST_FILES_TIMEOUT_MS) @@ -149,12 +171,28 @@ export function listFilesWithRg( /* drain to prevent backpressure stalls */ } function handleError(err: Error): void { + processErrorObserved = true + if (isRipgrepUnavailableExit(child, null, null)) { + passBuf = '' + rejectLaunchFailure(err) + return + } rejectPass(err) } function handleClose(code: number | null, signal: NodeJS.Signals | null): void { if (passDone) { return } + if ( + isRipgrepUnavailableExit(child, code, signal, { + classifyNativeLauncherExit: true + }) + ) { + unavailableExitObserved = true + passBuf = '' + rejectLaunchFailure(new Error(`rg exited with code ${code}`)) + return + } // Why signal != null is a failure: the only way spawn gets a signal // is if the process was killed (timeout, OOM, external SIGKILL). // Trusting its stdout could surface a truncated list as a success. @@ -199,7 +237,7 @@ export function listFilesWithRg( continue } if (entry.child.exitCode === null && entry.child.signalCode === null) { - entry.child.kill() + killSpawnedRipgrepProcess(entry.child) } entry.reject(new Error(reason)) } @@ -228,12 +266,15 @@ export function listFilesWithRg( } signal?.addEventListener('abort', onAbort, { once: true }) + const primaryPass = runPass(primary) const passes = maxResults === undefined - ? Promise.all([runPass(primary), runPass(ignoredPass)]) + ? children[0]?.child.pid === undefined + ? primaryPass + : Promise.all([primaryPass, runPass(ignoredPass)]) : // Why: deterministic primary-first budgeting prevents a large ignored // tree from starving ordinary source paths on a remote host. - runPass(primary).then(() => + primaryPass.then(() => files.size < maxResults ? runPass(ignoredPass) : Promise.resolve() ) diff --git a/src/relay/fs-handler-rg-availability.test.ts b/src/relay/fs-handler-rg-availability.test.ts deleted file mode 100644 index 8b14a3d61af..00000000000 --- a/src/relay/fs-handler-rg-availability.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { EventEmitter } from 'node:events' -import { describe, expect, it, vi } from 'vitest' - -const { execFileMock, spawnMock } = vi.hoisted(() => ({ - execFileMock: vi.fn(), - spawnMock: vi.fn() -})) - -vi.mock('child_process', () => ({ - execFile: execFileMock, - spawn: spawnMock -})) - -import { checkRgAvailable } from './fs-handler-utils' - -class FakeChildProcess extends EventEmitter { - kill = vi.fn() -} - -describe('relay rg availability', () => { - it('removes listeners after a successful probe', async () => { - const child = new FakeChildProcess() - execFileMock.mockReturnValueOnce(child) - - const result = checkRgAvailable() - child.emit('close', 0) - - await expect(result).resolves.toBe(true) - expect(child.listenerCount('error')).toBe(0) - expect(child.listenerCount('close')).toBe(0) - }) - - it('settles and detaches when a wedged probe ignores timeout kill', async () => { - vi.useFakeTimers() - try { - const child = new FakeChildProcess() - execFileMock.mockReturnValueOnce(child) - - const result = checkRgAvailable() - await vi.advanceTimersByTimeAsync(5000) - - await expect(result).resolves.toBe(false) - expect(child.kill).toHaveBeenCalledTimes(1) - expect(child.listenerCount('error')).toBe(0) - expect(child.listenerCount('close')).toBe(0) - } finally { - vi.useRealTimers() - } - }) -}) diff --git a/src/relay/fs-handler-ripgrep-fallback.test.ts b/src/relay/fs-handler-ripgrep-fallback.test.ts new file mode 100644 index 00000000000..93d89fd9008 --- /dev/null +++ b/src/relay/fs-handler-ripgrep-fallback.test.ts @@ -0,0 +1,121 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type * as ChildProcessModule from 'node:child_process' +import type * as FsHandlerGitFallback from './fs-handler-git-fallback' +import type * as FsHandlerUtils from './fs-handler-utils' + +const { + execFileMock, + listFilesWithGitMock, + listFilesWithRgMock, + searchWithGitGrepMock, + searchWithRgMock +} = vi.hoisted(() => ({ + execFileMock: vi.fn(), + listFilesWithGitMock: vi.fn(), + listFilesWithRgMock: vi.fn(), + searchWithGitGrepMock: vi.fn(), + searchWithRgMock: vi.fn() +})) + +vi.mock('node:child_process', async (importOriginal) => ({ + ...(await importOriginal()), + execFile: execFileMock +})) + +vi.mock('./fs-handler-utils', async (importOriginal) => ({ + ...(await importOriginal()), + listFilesWithRg: listFilesWithRgMock, + searchWithRg: searchWithRgMock +})) + +vi.mock('./fs-handler-git-fallback', async (importOriginal) => ({ + ...(await importOriginal()), + listFilesWithGit: listFilesWithGitMock, + searchWithGitGrep: searchWithGitGrepMock +})) + +import { FileListingCancelledError } from '../shared/file-listing-cancellation' +import { RipgrepUnavailableError } from '../shared/ripgrep-process-availability' +import { RelayContext } from './context' +import { FsHandler } from './fs-handler' + +type FsHandlerInternals = { + search(params: Record): Promise + runListFilesScan( + rootPath: string, + excludePathPrefixes: string[], + signal: AbortSignal, + maxResults?: number + ): Promise +} + +function createHandler(): FsHandlerInternals { + const dispatcher = { + onRequest: vi.fn(), + onNotification: vi.fn(), + onClientDetached: vi.fn(() => () => undefined) + } + const watcherPool = { + dispose: vi.fn(), + forgetRoot: vi.fn(), + subscribe: vi.fn() + } + return new FsHandler(dispatcher as never, new RelayContext(), watcherPool as never) as never +} + +describe('relay direct ripgrep admission', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('falls back only for a tagged search launch failure', async () => { + const handler = createHandler() + const fallback = { files: [], totalMatches: 0, truncated: false } + searchWithRgMock.mockRejectedValueOnce(new RipgrepUnavailableError()) + searchWithGitGrepMock.mockResolvedValueOnce(fallback) + + await expect(handler.search({ rootPath: '/repo', query: 'needle' })).resolves.toBe(fallback) + expect(searchWithRgMock).toHaveBeenCalledTimes(1) + expect(searchWithGitGrepMock).toHaveBeenCalledTimes(1) + + const ordinaryFailure = new Error('rg failed after spawn') + searchWithRgMock.mockRejectedValueOnce(ordinaryFailure) + await expect(handler.search({ rootPath: '/repo', query: 'needle' })).rejects.toBe( + ordinaryFailure + ) + expect(searchWithGitGrepMock).toHaveBeenCalledTimes(1) + }) + + it('falls back only for a tagged listing launch failure', async () => { + const handler = createHandler() + const controller = new AbortController() + listFilesWithRgMock.mockRejectedValueOnce(new RipgrepUnavailableError()) + execFileMock.mockImplementationOnce((_command, _args, _options, callback) => { + callback(null) + return undefined + }) + listFilesWithGitMock.mockResolvedValueOnce(['src/index.ts']) + + await expect(handler.runListFilesScan('/repo', [], controller.signal)).resolves.toEqual([ + 'src/index.ts' + ]) + expect(listFilesWithRgMock).toHaveBeenCalledTimes(1) + expect(listFilesWithGitMock).toHaveBeenCalledTimes(1) + }) + + it('lets cancellation win an unavailable-listing race before Git starts', async () => { + const handler = createHandler() + const controller = new AbortController() + const cancellation = new FileListingCancelledError('superseded') + listFilesWithRgMock.mockImplementationOnce(async () => { + controller.abort(cancellation) + throw new RipgrepUnavailableError() + }) + + await expect(handler.runListFilesScan('/repo', [], controller.signal)).rejects.toBe( + cancellation + ) + expect(execFileMock).not.toHaveBeenCalled() + expect(listFilesWithGitMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/relay/fs-handler-utils.ts b/src/relay/fs-handler-utils.ts index 3fcf50a6eb2..83726c686bf 100644 --- a/src/relay/fs-handler-utils.ts +++ b/src/relay/fs-handler-utils.ts @@ -5,7 +5,7 @@ * These functions depend only on their arguments (plus `rg` being on PATH), * so they are straightforward to test independently. */ -import { spawn, execFile } from 'node:child_process' +import { spawn } from 'node:child_process' import { open } from 'node:fs/promises' import { buildRgArgs, @@ -16,6 +16,13 @@ import { } from '../shared/text-search' import { IMAGE_FILE_MIME_TYPES } from '../shared/image-file-extensions' import type { SearchResult as SharedSearchResult } from '../shared/types' +import { + absorbPendingRipgrepSpawnError, + isRipgrepUnavailableAfterLaunchFailure, + isRipgrepUnavailableExit, + killSpawnedRipgrepProcess, + RipgrepUnavailableError +} from '../shared/ripgrep-process-availability' // ─── Constants ─────────────────────────────────────────────────────── @@ -89,11 +96,14 @@ export function searchWithRg( query: string, opts: SearchOptions ): Promise { - return new Promise((resolve) => { + return new Promise((resolve, reject) => { const rgArgs = buildRgArgs(query, rootPath, opts) const acc = createAccumulator() let buffer = '' let resolved = false + let processErrorObserved = false + let unavailableExitObserved = false + let launchFailureCheck: Promise | null = null // Why: spawn can throw synchronously on invalid options (e.g. bad cwd), // which would leak out of the `new Promise` executor and leave the @@ -124,13 +134,47 @@ export function searchWithRg( child.stderr!.off('data', handleStderrData) child.off('error', handleError) child.off('close', handleClose) + absorbPendingRipgrepSpawnError(child, { + errorObserved: processErrorObserved, + unavailableExitObserved + }) resolve(finalize(acc)) } + function rejectUnavailable(): void { + if (resolved) { + return + } + resolved = true + clearTimeout(killTimeout) + child.stdout!.off('data', handleStdoutData) + child.stderr!.off('data', handleStderrData) + child.off('error', handleError) + child.off('close', handleClose) + absorbPendingRipgrepSpawnError(child, { + errorObserved: processErrorObserved, + unavailableExitObserved + }) + reject(new RipgrepUnavailableError()) + } + + function settleLaunchFailure(): void { + if (launchFailureCheck) { + return + } + launchFailureCheck = isRipgrepUnavailableAfterLaunchFailure(rootPath).then((unavailable) => { + if (unavailable) { + rejectUnavailable() + } else { + resolveOnce() + } + }) + } + function processLine(line: string): void { const verdict = ingestRgJsonLine(line, rootPath, acc, opts.maxResults) if (verdict === 'stop') { - child.kill() + killSpawnedRipgrepProcess(child) } } @@ -148,10 +192,24 @@ export function searchWithRg( } function handleError(): void { + processErrorObserved = true + if (isRipgrepUnavailableExit(child, null, null)) { + settleLaunchFailure() + return + } resolveOnce() } - function handleClose(): void { + function handleClose(code: number | null, signal: NodeJS.Signals | null): void { + if ( + isRipgrepUnavailableExit(child, code, signal, { + classifyNativeLauncherExit: true + }) + ) { + unavailableExitObserved = true + settleLaunchFailure() + return + } if (buffer) { processLine(buffer) } @@ -166,57 +224,11 @@ export function searchWithRg( killTimeout = setTimeout(() => { acc.truncated = true - child.kill() + killSpawnedRipgrepProcess(child) resolveOnce() }, SEARCH_TIMEOUT_MS) }) } -// ─── rg availability check ────────────────────────────────────────── - -// Why no cache: `rg --version` is a sub-10ms local spawn, and caching the -// result caused a footgun — a negative cache persisted across rg installs -// (forcing a relay restart), while a positive cache could mask an rg that -// was uninstalled or broken mid-session. The `settled` flag below closes -// the original race between 'error' and 'close' that the cache was added -// to paper over, so re-checking per call is both simpler and safer. -const RG_AVAILABILITY_TIMEOUT_MS = 5000 - -export function checkRgAvailable(): Promise { - return new Promise((resolve) => { - let settled = false - const child = execFile('rg', ['--version']) - let timeout: ReturnType | null = null - const cleanup = (): void => { - if (timeout) { - clearTimeout(timeout) - timeout = null - } - child.off('error', onError) - child.off('close', onClose) - } - const settle = (available: boolean, options?: { kill?: boolean }): void => { - if (settled) { - return - } - settled = true - cleanup() - if (options?.kill) { - child.kill() - } - resolve(available) - } - const onError = (): void => settle(false) - const onClose = (code: number | null): void => settle(code === 0) - - child.once('error', onError) - child.once('close', onClose) - timeout = setTimeout(() => settle(false, { kill: true }), RG_AVAILABILITY_TIMEOUT_MS) - if (typeof timeout.unref === 'function') { - timeout.unref() - } - }) -} - // Moved to fs-handler-list-files.ts to keep this file under 300 lines (oxlint) export { listFilesWithRg, LIST_FILES_TIMEOUT_MS } from './fs-handler-list-files' diff --git a/src/relay/fs-handler.ts b/src/relay/fs-handler.ts index d07e2bf397f..b7dd2dfa6b5 100644 --- a/src/relay/fs-handler.ts +++ b/src/relay/fs-handler.ts @@ -10,12 +10,7 @@ import type { RelayContext } from './context' // Why: RelayContext is accepted in the constructor for protocol back-compat // (see docs/relay-fs-allowlist-removal.md), but no longer consulted on FS ops. import { expandTilde } from './context' -import { - DEFAULT_MAX_RESULTS, - searchWithRg, - listFilesWithRg, - checkRgAvailable -} from './fs-handler-utils' +import { DEFAULT_MAX_RESULTS, searchWithRg, listFilesWithRg } from './fs-handler-utils' import { listFilesWithGit, searchWithGitGrep } from './fs-handler-git-fallback' import { listFilesWithReaddir } from './fs-handler-readdir-fallback' import { ListFilesScanCoordinator } from './fs-list-files-scan-coordinator' @@ -35,6 +30,7 @@ import { RelayStreamRegistry } from './fs-stream-registry' import { scanWorkspaceSpaceDirectory } from './workspace-space-scan' import { buildRelayCommandEnv } from './relay-command-env' import { assertNoClobberRenameDestinationAvailable } from '../shared/filesystem-rename-collision' +import { RipgrepUnavailableError } from '../shared/ripgrep-process-availability' import { RelayFilesystemWatchRegistry } from './relay-filesystem-watch-registry' import type { RelayWatcherProcessPool } from './relay-watcher-process-pool' @@ -328,26 +324,22 @@ export class FsHandler { DEFAULT_MAX_RESULTS ) - const rgAvailable = await checkRgAvailable() - if (!rgAvailable) { - return searchWithGitGrep(rootPath, query, { - caseSensitive, - wholeWord, - useRegex, - includePattern, - excludePattern, - maxResults - }) - } - - return searchWithRg(rootPath, query, { + const options = { caseSensitive, wholeWord, useRegex, includePattern, excludePattern, maxResults - }) + } + try { + return await searchWithRg(rootPath, query, options) + } catch (error) { + if (!(error instanceof RipgrepUnavailableError)) { + throw error + } + return searchWithGitGrep(rootPath, query, options) + } } private listFiles(params: Record, context?: RequestContext): Promise { @@ -380,10 +372,14 @@ export class FsHandler { signal: AbortSignal, maxResults?: number ): Promise { - const rgAvailable = await checkRgAvailable() throwIfFileListingCancelled(signal) - if (rgAvailable) { - return listFilesWithRg(rootPath, excludePathPrefixes, { signal, maxResults }) + try { + return await listFilesWithRg(rootPath, excludePathPrefixes, { signal, maxResults }) + } catch (error) { + throwIfFileListingCancelled(signal) + if (!(error instanceof RipgrepUnavailableError)) { + throw error + } } // Why: git ls-files only works inside git repos. Use rev-parse to detect // git ancestry — unlike checking for a local .git entry, this works from diff --git a/src/relay/fs-list-files-cancel.integration.test.ts b/src/relay/fs-list-files-cancel.integration.test.ts index 588ddb5fe51..f54ebaea369 100644 --- a/src/relay/fs-list-files-cancel.integration.test.ts +++ b/src/relay/fs-list-files-cancel.integration.test.ts @@ -54,7 +54,6 @@ vi.mock('./fs-handler-utils', async (importOriginal) => { const original = (await importOriginal()) as Record return { ...original, - checkRgAvailable: () => Promise.resolve(true), listFilesWithRg: fakeListFiles } }) diff --git a/src/shared/ripgrep-process-availability.test.ts b/src/shared/ripgrep-process-availability.test.ts new file mode 100644 index 00000000000..033df296666 --- /dev/null +++ b/src/shared/ripgrep-process-availability.test.ts @@ -0,0 +1,90 @@ +import { spawn, type ChildProcess } from 'node:child_process' +import { EventEmitter } from 'node:events' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { + absorbPendingRipgrepSpawnError, + isRipgrepUnavailableExit, + isRipgrepSpawnCwdUsable, + killSpawnedRipgrepProcess +} from './ripgrep-process-availability' + +function createChild(pid: number | undefined): ChildProcess { + const child = new EventEmitter() as ChildProcess + Object.defineProperty(child, 'pid', { value: pid }) + return child +} + +describe('ripgrep process availability', () => { + it('recognizes native pre-spawn failures before either event wins the race', () => { + expect(isRipgrepUnavailableExit(createChild(undefined), null, null)).toBe(true) + expect(isRipgrepUnavailableExit(createChild(undefined), -2, null)).toBe(true) + }) + + it('keeps all post-spawn and signal exits on their existing paths', () => { + for (const code of [0, 1, 2, 127]) { + expect(isRipgrepUnavailableExit(createChild(1), code, null)).toBe(false) + } + expect(isRipgrepUnavailableExit(createChild(1), null, 'SIGTERM')).toBe(false) + }) + + it('tags only unsupported native launcher exits after spawn', () => { + for (const code of [0, 1, 2]) { + expect( + isRipgrepUnavailableExit(createChild(1), code, null, { + classifyNativeLauncherExit: true + }) + ).toBe(false) + } + for (const code of [3, 126, 127, 9009]) { + expect( + isRipgrepUnavailableExit(createChild(1), code, null, { + classifyNativeLauncherExit: true + }) + ).toBe(true) + } + }) + + it('does not signal a real failed-spawn handle', async () => { + const child = spawn('orca-definitely-missing-rg-admission-test', []) + const error = new Promise((resolve) => child.once('error', () => resolve())) + const close = new Promise((resolve) => child.once('close', () => resolve())) + + expect(killSpawnedRipgrepProcess(child)).toBe(false) + await Promise.all([error, close]) + }) + + it('distinguishes a usable root from a missing spawn cwd', async () => { + await expect(isRipgrepSpawnCwdUsable(process.cwd())).resolves.toBe(true) + await expect( + isRipgrepSpawnCwdUsable(join(process.cwd(), 'orca-definitely-missing-rg-cwd')) + ).resolves.toBe(false) + }) + + it('absorbs a queued spawn error when cleanup wins the race', () => { + const child = createChild(undefined) + absorbPendingRipgrepSpawnError(child, { + errorObserved: false, + unavailableExitObserved: false + }) + + expect(() => child.emit('error', new Error('spawn rg ENOENT'))).not.toThrow() + expect(child.listenerCount('error')).toBe(0) + }) + + it('does not retain a sink for started or already-observed processes', () => { + const started = createChild(1) + absorbPendingRipgrepSpawnError(started, { + errorObserved: false, + unavailableExitObserved: false + }) + const observed = createChild(undefined) + absorbPendingRipgrepSpawnError(observed, { + errorObserved: true, + unavailableExitObserved: true + }) + + expect(started.listenerCount('error')).toBe(0) + expect(observed.listenerCount('error')).toBe(0) + }) +}) diff --git a/src/shared/ripgrep-process-availability.ts b/src/shared/ripgrep-process-availability.ts new file mode 100644 index 00000000000..0b5ef026c0e --- /dev/null +++ b/src/shared/ripgrep-process-availability.ts @@ -0,0 +1,124 @@ +import { constants } from 'node:fs' +import { access, stat } from 'node:fs/promises' +import { spawn, type ChildProcess } from 'node:child_process' + +const RIPGREP_CWD_CHECK_TIMEOUT_MS = 1000 +const RIPGREP_FAILURE_PROBE_TIMEOUT_MS = 5000 + +export class RipgrepUnavailableError extends Error { + constructor() { + super('ripgrep is unavailable') + this.name = 'RipgrepUnavailableError' + } +} + +function ignoreRipgrepSpawnError(): void {} + +export function killSpawnedRipgrepProcess(child: ChildProcess): boolean { + // Why: killing a failed-spawn handle can signal the relay's own process group. + if (Object.hasOwn(child, 'pid') && child.pid === undefined) { + return false + } + return child.kill() +} + +export function absorbPendingRipgrepSpawnError( + child: ChildProcess, + state: { errorObserved: boolean; unavailableExitObserved: boolean } +): void { + if ( + state.errorObserved || + (!state.unavailableExitObserved && !(Object.hasOwn(child, 'pid') && child.pid === undefined)) + ) { + return + } + // Why: concurrent-pass cleanup can win before Node delivers the queued spawn error. + child.once('error', ignoreRipgrepSpawnError) +} + +export async function isRipgrepSpawnCwdUsable(cwd: string): Promise { + let timeout: ReturnType | null = null + const timedOut = new Promise((resolve) => { + timeout = setTimeout(() => resolve(false), RIPGREP_CWD_CHECK_TIMEOUT_MS) + timeout.unref?.() + }) + const checked = Promise.all([stat(cwd), access(cwd, constants.X_OK)]).then( + ([entry]) => entry.isDirectory(), + () => false + ) + try { + return await Promise.race([checked, timedOut]) + } finally { + if (timeout) { + clearTimeout(timeout) + } + } +} + +function checkRipgrepAvailableWithoutCwd(): Promise { + return new Promise((resolve) => { + let child: ChildProcess + try { + child = spawn('rg', ['--version'], { stdio: 'ignore' }) + } catch { + resolve(false) + return + } + let settled = false + let errorObserved = false + let unavailableExitObserved = false + let timeout: ReturnType | null = null + const settle = (available: boolean, kill = false): void => { + if (settled) { + return + } + settled = true + if (timeout) { + clearTimeout(timeout) + } + child.off('error', onError) + child.off('close', onClose) + if (kill) { + child.once('error', ignoreRipgrepSpawnError) + killSpawnedRipgrepProcess(child) + } else { + absorbPendingRipgrepSpawnError(child, { errorObserved, unavailableExitObserved }) + } + resolve(available) + } + const onError = (): void => { + errorObserved = true + settle(false) + } + const onClose = (code: number | null): void => { + unavailableExitObserved = code !== null && code < 0 + settle(code === 0) + } + child.once('error', onError) + child.once('close', onClose) + timeout = setTimeout(() => settle(false, true), RIPGREP_FAILURE_PROBE_TIMEOUT_MS) + timeout.unref?.() + }) +} + +export async function isRipgrepUnavailableAfterLaunchFailure(cwd: string): Promise { + if (await isRipgrepSpawnCwdUsable(cwd)) { + return true + } + return !(await checkRipgrepAvailableWithoutCwd()) +} + +export function isRipgrepUnavailableExit( + child: ChildProcess, + code: number | null, + signal: NodeJS.Signals | null, + options: { classifyNativeLauncherExit?: boolean } = {} +): boolean { + if (signal) { + return false + } + if ((Object.hasOwn(child, 'pid') && child.pid === undefined) || (code !== null && code < 0)) { + return true + } + return Boolean(options.classifyNativeLauncherExit && code !== null && code > 2) +}