From e1de59aaa8a4b78331374068d98e5fc7cd203595 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 10 Jul 2026 17:22:47 -0700 Subject: [PATCH] fix(quick-open): keep primary results when the git ignored pass fails (#8049) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The no-ripgrep git fallback ran both ls-files passes under Promise.all with killSurvivors on first rejection, so any ignored-pass failure (timeout, kill, non-zero exit) discarded the successful primary listing and Quick Open showed an error with zero files — the all-or-nothing failure called out in #7719. Directory collapse (#7842) bounds the enumeration so timeouts are far less likely, but a single-pass failure still nuked every result. Now, in the local main process and the SSH relay alike, only a primary-pass failure is fatal: an ignored-pass failure logs a warning and the listing resolves with the primary results plus any ignored entries streamed before the failure. Cancellation semantics are unchanged — an aborted scan still rejects via the primary pass or the expansion's cancellation check. Fixes #7719 Co-authored-by: Brennan Benson --- .../ipc/filesystem-list-files-git-fallback.ts | 14 +++- src/main/ipc/filesystem-list-files.test.ts | 54 ++++++++++++++ src/relay/fs-handler-git-fallback.ts | 17 ++++- .../fs-handler-list-files-ignored.test.ts | 72 +++++++++++++++++-- 4 files changed, 148 insertions(+), 9 deletions(-) diff --git a/src/main/ipc/filesystem-list-files-git-fallback.ts b/src/main/ipc/filesystem-list-files-git-fallback.ts index 8b5bb526f..616a9a10c 100644 --- a/src/main/ipc/filesystem-list-files-git-fallback.ts +++ b/src/main/ipc/filesystem-list-files-git-fallback.ts @@ -216,7 +216,19 @@ export async function listFilesWithGit( const onAbort = (): void => killSurvivors('git ls-files cancelled') signal?.addEventListener('abort', onAbort, { once: true }) try { - await Promise.all([runGitLsFiles(primary), runGitLsFiles(ignoredPass)]) + await Promise.all([ + runGitLsFiles(primary), + // Why: ignored files are supplementary — a failed or timed-out ignored + // pass must not discard the primary listing the user actually needs + // (#7719 root cause: the all-or-nothing failure showed zero files). + // Entries streamed before the failure are kept; a cancelled scan still + // rejects via the primary pass or the expansion's cancellation check. + runGitLsFiles(ignoredPass).catch((err: Error) => { + if (!signal?.aborted) { + console.warn('[quick-open] git ignored-file pass failed; keeping primary results:', err) + } + }) + ]) } catch (err) { killSurvivors() if (signal?.aborted) { diff --git a/src/main/ipc/filesystem-list-files.test.ts b/src/main/ipc/filesystem-list-files.test.ts index a3a556b23..ba535a258 100644 --- a/src/main/ipc/filesystem-list-files.test.ts +++ b/src/main/ipc/filesystem-list-files.test.ts @@ -475,6 +475,60 @@ 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() + + try { + const revParseProc = createMockProcess() + const gitP1 = createMockProcess() + const gitP2 = createMockProcess() + let callIndex = 0 + + spawnMock.mockImplementation((cmd: string, args: string[]) => { + if (cmd === 'git' && args.includes('rev-parse')) { + return revParseProc + } + if (cmd === 'git' && args.includes('ls-files')) { + callIndex++ + return callIndex === 1 ? gitP1 : gitP2 + } + return createMockProcess() + }) + + const storeMock = {} as unknown as Store + const promise = listQuickOpenFiles('/mock/root', storeMock) + + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() + revParseProc.emit('close', 0, null) + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() + + ;(gitP1.stdout as unknown as EventEmitter).emit( + 'data', + `${staged('100644', 'src/index.ts')}\0` + ) + gitP1.emit('close', 0, null) + // Ignored entries streamed before the timeout are kept. + ;(gitP2.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\0') + + await vi.advanceTimersByTimeAsync(10000) + + await expect(promise).resolves.toEqual( + expect.arrayContaining(['src/index.ts', 'dist/generated.js']) + ) + expect(gitP2.kill).toHaveBeenCalled() + expect(warnSpy).toHaveBeenCalled() + } finally { + vi.useRealTimers() + warnSpy.mockRestore() + } + }) + it('does not fall back to git when rg is available', async () => { checkRgAvailableMock.mockResolvedValue(true) diff --git a/src/relay/fs-handler-git-fallback.ts b/src/relay/fs-handler-git-fallback.ts index 6cceb5de7..ae418f9e8 100644 --- a/src/relay/fs-handler-git-fallback.ts +++ b/src/relay/fs-handler-git-fallback.ts @@ -176,7 +176,22 @@ export function listFilesWithGit( const onAbort = (): void => killSurvivors('git ls-files cancelled') signal?.addEventListener('abort', onAbort, { once: true }) - return Promise.all([runGitLsFiles(primary), runGitLsFiles(ignoredPass)]) + return Promise.all([ + runGitLsFiles(primary), + // Why: ignored files are supplementary — a failed or timed-out ignored + // pass must not discard the primary listing the user actually needs + // (#7719 root cause: the all-or-nothing failure showed zero files). + // Entries streamed before the failure are kept; a cancelled scan still + // rejects via the primary pass or the expansion's cancellation check. + runGitLsFiles(ignoredPass).catch((err: Error) => { + if (!signal?.aborted) { + console.warn( + '[relay quick-open] git ignored-file pass failed; keeping primary results:', + err + ) + } + }) + ]) .then(async () => { const files = await expandQuickOpenGitFileListing({ rootPath, diff --git a/src/relay/fs-handler-list-files-ignored.test.ts b/src/relay/fs-handler-list-files-ignored.test.ts index 5a1804437..d76fe40b5 100644 --- a/src/relay/fs-handler-list-files-ignored.test.ts +++ b/src/relay/fs-handler-list-files-ignored.test.ts @@ -182,7 +182,65 @@ describe('relay quick open ignored file listing', () => { ]) }) - it('git fallback rejects signal exits instead of returning partial results', async () => { + it('git fallback keeps primary results when the ignored pass is killed', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + const primaryProc = createMockProcess() + const ignoredProc = createMockProcess() + let callIndex = 0 + + spawnMock.mockImplementation(() => { + callIndex++ + return callIndex === 1 ? primaryProc : ignoredProc + }) + + const promise = listFilesWithGit('/remote/root') + + setTimeout(() => { + ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\0') + primaryProc.emit('close', 0, null) + + // Entries streamed before the kill are kept alongside the primary pass. + ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\0') + ignoredProc.emit('close', null, 'SIGTERM') + }, 10) + + await expect(promise).resolves.toEqual(['dist/generated.js', 'src/index.ts']) + expect(warnSpy).toHaveBeenCalled() + } finally { + warnSpy.mockRestore() + } + }) + + it('git fallback keeps primary results when the ignored pass exits non-zero', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + const primaryProc = createMockProcess() + const ignoredProc = createMockProcess() + let callIndex = 0 + + spawnMock.mockImplementation(() => { + callIndex++ + return callIndex === 1 ? primaryProc : ignoredProc + }) + + const promise = listFilesWithGit('/remote/root') + + setTimeout(() => { + ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\0') + primaryProc.emit('close', 0, null) + + ignoredProc.emit('close', 128, null) + }, 10) + + await expect(promise).resolves.toEqual(['src/index.ts']) + expect(warnSpy).toHaveBeenCalled() + } finally { + warnSpy.mockRestore() + } + }) + + it('git fallback rejects when the primary pass is killed', async () => { const primaryProc = createMockProcess() const ignoredProc = createMockProcess() let callIndex = 0 @@ -196,16 +254,16 @@ describe('relay quick open ignored file listing', () => { setTimeout(() => { ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\0') - primaryProc.emit('close', 0, null) + primaryProc.emit('close', null, 'SIGTERM') ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\0') - ignoredProc.emit('close', null, 'SIGTERM') + ignoredProc.emit('close', 0, null) }, 10) await expect(promise).rejects.toThrow('git ls-files killed by SIGTERM') }) - it('git fallback rejects non-zero exits instead of expanding a partial result set', async () => { + it('git fallback rejects when the primary pass exits non-zero', async () => { const primaryProc = createMockProcess() const ignoredProc = createMockProcess() let callIndex = 0 @@ -218,10 +276,10 @@ describe('relay quick open ignored file listing', () => { const promise = listFilesWithGit('/remote/root') setTimeout(() => { - ;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\0') - primaryProc.emit('close', 0, null) + primaryProc.emit('close', 128, null) - ignoredProc.emit('close', 128, null) + ;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/generated.js\0') + ignoredProc.emit('close', 0, null) }, 10) await expect(promise).rejects.toThrow('git ls-files exited with code 128')