fix(quick-open): keep primary results when the git ignored pass fails (#8049)
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 <brennanbenson@Brennans-MacBook-Pro.local>
This commit is contained in:
parent
db9fd2f8ac
commit
e1de59aaa8
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
|
|||
Loading…
Reference in New Issue