mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(ssh): make fs.listFiles cancellable and single-flight per relay client (#7769)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -2316,4 +2316,39 @@ describe('registerFilesystemHandlers', () => {
|
||||
excludePaths: ['/home/user/repo/worktrees/feature']
|
||||
})
|
||||
})
|
||||
|
||||
// Why #7721: without a cancel path, every workspace switch left the previous
|
||||
// workspace's full-tree SSH scan running, stacking scans on the relay until
|
||||
// interactive fs.readDir/fs.stat starved past their 30s timeout.
|
||||
it('fs:cancelListFiles aborts an in-flight SSH listing by request token (#7721)', async () => {
|
||||
let capturedSignal: AbortSignal | undefined
|
||||
const listFilesMock = vi.fn(
|
||||
(_rootPath: string, options: { signal?: AbortSignal }) =>
|
||||
new Promise<string[]>((_resolve, reject) => {
|
||||
capturedSignal = options.signal
|
||||
options.signal?.addEventListener('abort', () => reject(new Error('listing cancelled')), {
|
||||
once: true
|
||||
})
|
||||
})
|
||||
)
|
||||
getSshFilesystemProviderMock.mockReturnValue({ listFiles: listFilesMock })
|
||||
|
||||
registerFilesystemHandlers(store as never)
|
||||
|
||||
const pending = handlers.get('fs:listFiles')!(null, {
|
||||
rootPath: '/home/user/repo',
|
||||
connectionId: 'conn-1',
|
||||
requestToken: 'token-1'
|
||||
}) as Promise<string[]>
|
||||
|
||||
expect(capturedSignal?.aborted).toBe(false)
|
||||
await handlers.get('fs:cancelListFiles')!(null, { requestToken: 'token-1' })
|
||||
expect(capturedSignal?.aborted).toBe(true)
|
||||
await expect(pending).rejects.toThrow('listing cancelled')
|
||||
|
||||
// Unknown or already-settled tokens are a no-op, not an error.
|
||||
expect(() =>
|
||||
handlers.get('fs:cancelListFiles')!(null, { requestToken: 'unknown' })
|
||||
).not.toThrow()
|
||||
})
|
||||
})
|
||||
|
||||
+42
-14
@@ -1026,30 +1026,58 @@ export function registerFilesystemHandlers(
|
||||
)
|
||||
|
||||
// ─── List all files (for quick-open) ─────────────────────
|
||||
// Why #7721: keyed by renderer-generated token so a workspace switch can
|
||||
// abort the previous workspace's full-tree scan (SSH relays otherwise stack
|
||||
// scans that starve interactive fs.readDir/fs.stat past their 30s timeout).
|
||||
const listFilesCancellations = new Map<string, AbortController>()
|
||||
ipcMain.handle(
|
||||
'fs:listFiles',
|
||||
async (
|
||||
_event,
|
||||
args: { rootPath: string; connectionId?: string; excludePaths?: string[] }
|
||||
args: {
|
||||
rootPath: string
|
||||
connectionId?: string
|
||||
excludePaths?: string[]
|
||||
requestToken?: string
|
||||
}
|
||||
): Promise<string[]> => {
|
||||
if (args.connectionId) {
|
||||
const provider = getSshFilesystemProvider(args.connectionId)
|
||||
// Why: when the SSH connection is not yet established (cold start) or
|
||||
// temporarily disconnected, return [] so quick-open shows "No matching
|
||||
// files" instead of an error banner. The file list will repopulate when
|
||||
// the user re-opens quick-open after the connection is restored.
|
||||
if (!provider) {
|
||||
return []
|
||||
const controller = args.requestToken ? new AbortController() : null
|
||||
if (controller && args.requestToken) {
|
||||
listFilesCancellations.set(args.requestToken, controller)
|
||||
}
|
||||
try {
|
||||
if (args.connectionId) {
|
||||
const provider = getSshFilesystemProvider(args.connectionId)
|
||||
// Why: when the SSH connection is not yet established (cold start) or
|
||||
// temporarily disconnected, return [] so quick-open shows "No matching
|
||||
// files" instead of an error banner. The file list will repopulate when
|
||||
// the user re-opens quick-open after the connection is restored.
|
||||
if (!provider) {
|
||||
return []
|
||||
}
|
||||
// Why: forward excludePaths through to the remote provider.
|
||||
// Dropping it here would silently double-scan nested linked worktrees
|
||||
// over SSH and contribute to timeout-induced partial results.
|
||||
return await provider.listFiles(args.rootPath, {
|
||||
excludePaths: args.excludePaths,
|
||||
signal: controller?.signal
|
||||
})
|
||||
}
|
||||
// Why: forward excludePaths through to the remote provider.
|
||||
// Dropping it here would silently double-scan nested linked worktrees
|
||||
// over SSH and contribute to timeout-induced partial results.
|
||||
return provider.listFiles(args.rootPath, { excludePaths: args.excludePaths })
|
||||
return await listQuickOpenFiles(args.rootPath, store, args.excludePaths)
|
||||
} finally {
|
||||
if (args.requestToken) {
|
||||
listFilesCancellations.delete(args.requestToken)
|
||||
}
|
||||
}
|
||||
return listQuickOpenFiles(args.rootPath, store, args.excludePaths)
|
||||
}
|
||||
)
|
||||
|
||||
ipcMain.handle('fs:cancelListFiles', (_event, args: { requestToken: string }): void => {
|
||||
// Why: best-effort — the entry is gone once the listing settles, and
|
||||
// local scans are fast enough to simply let them finish.
|
||||
listFilesCancellations.get(args.requestToken)?.abort()
|
||||
})
|
||||
|
||||
// ─── Git operations ─────────────────────────────────────
|
||||
ipcMain.handle(
|
||||
'git:status',
|
||||
|
||||
@@ -417,7 +417,11 @@ describe('SshFilesystemProvider', () => {
|
||||
it('listFiles sends fs.listFiles request', async () => {
|
||||
mux.request.mockResolvedValue(['src/index.ts', 'package.json'])
|
||||
const result = await provider.listFiles('/home/user/project')
|
||||
expect(mux.request).toHaveBeenCalledWith('fs.listFiles', { rootPath: '/home/user/project' })
|
||||
expect(mux.request).toHaveBeenCalledWith(
|
||||
'fs.listFiles',
|
||||
{ rootPath: '/home/user/project' },
|
||||
{ signal: undefined }
|
||||
)
|
||||
expect(result).toEqual(['src/index.ts', 'package.json'])
|
||||
})
|
||||
|
||||
@@ -426,16 +430,35 @@ describe('SshFilesystemProvider', () => {
|
||||
await provider.listFiles('/home/user/project', {
|
||||
excludePaths: ['/home/user/project/worktrees/b']
|
||||
})
|
||||
expect(mux.request).toHaveBeenCalledWith('fs.listFiles', {
|
||||
rootPath: '/home/user/project',
|
||||
excludePaths: ['/home/user/project/worktrees/b']
|
||||
})
|
||||
expect(mux.request).toHaveBeenCalledWith(
|
||||
'fs.listFiles',
|
||||
{
|
||||
rootPath: '/home/user/project',
|
||||
excludePaths: ['/home/user/project/worktrees/b']
|
||||
},
|
||||
{ signal: undefined }
|
||||
)
|
||||
})
|
||||
|
||||
it('listFiles omits excludePaths when empty', async () => {
|
||||
mux.request.mockResolvedValue([])
|
||||
await provider.listFiles('/home/user/project', { excludePaths: [] })
|
||||
expect(mux.request).toHaveBeenCalledWith('fs.listFiles', { rootPath: '/home/user/project' })
|
||||
expect(mux.request).toHaveBeenCalledWith(
|
||||
'fs.listFiles',
|
||||
{ rootPath: '/home/user/project' },
|
||||
{ signal: undefined }
|
||||
)
|
||||
})
|
||||
|
||||
it('listFiles forwards the cancellation signal to the mux request (#7721)', async () => {
|
||||
mux.request.mockResolvedValue([])
|
||||
const controller = new AbortController()
|
||||
await provider.listFiles('/home/user/project', { signal: controller.signal })
|
||||
expect(mux.request).toHaveBeenCalledWith(
|
||||
'fs.listFiles',
|
||||
{ rootPath: '/home/user/project' },
|
||||
{ signal: controller.signal }
|
||||
)
|
||||
})
|
||||
|
||||
describe('watch', () => {
|
||||
|
||||
@@ -283,12 +283,20 @@ export class SshFilesystemProvider implements IFilesystemProvider {
|
||||
return (await this.mux.request('fs.search', opts)) as SearchResult
|
||||
}
|
||||
|
||||
async listFiles(rootPath: string, options?: { excludePaths?: string[] }): Promise<string[]> {
|
||||
async listFiles(
|
||||
rootPath: string,
|
||||
options?: { excludePaths?: string[]; signal?: AbortSignal }
|
||||
): Promise<string[]> {
|
||||
const params: Record<string, unknown> = { rootPath }
|
||||
if (options?.excludePaths && options.excludePaths.length > 0) {
|
||||
params.excludePaths = options.excludePaths
|
||||
}
|
||||
return (await this.mux.request('fs.listFiles', params)) as string[]
|
||||
// Why #7721: the signal lets a workspace switch send rpc.cancel so the
|
||||
// relay aborts the full-tree scan instead of stacking abandoned scans
|
||||
// that starve interactive fs.readDir/fs.stat on the shared SSH channel.
|
||||
return (await this.mux.request('fs.listFiles', params, {
|
||||
signal: options?.signal
|
||||
})) as string[]
|
||||
}
|
||||
|
||||
async watch(rootPath: string, callback: (events: FsChangeEvent[]) => void): Promise<() => void> {
|
||||
|
||||
@@ -197,7 +197,10 @@ export type IFilesystemProvider = {
|
||||
copy(source: string, destination: string): Promise<void>
|
||||
realpath(filePath: string): Promise<string>
|
||||
search(opts: SearchOptions): Promise<SearchResult>
|
||||
listFiles(rootPath: string, options?: { excludePaths?: string[] }): Promise<string[]>
|
||||
listFiles(
|
||||
rootPath: string,
|
||||
options?: { excludePaths?: string[]; signal?: AbortSignal }
|
||||
): Promise<string[]>
|
||||
scanWorkspaceSpace?(
|
||||
rootPath: string,
|
||||
options?: { signal?: AbortSignal }
|
||||
|
||||
@@ -2247,7 +2247,9 @@ export type PreloadApi = {
|
||||
rootPath: string
|
||||
connectionId?: string
|
||||
excludePaths?: string[]
|
||||
requestToken?: string
|
||||
}) => Promise<string[]>
|
||||
cancelListFiles: (args: { requestToken: string }) => Promise<void>
|
||||
search: (args: SearchOptions & { connectionId?: string }) => Promise<SearchResult>
|
||||
importExternalPaths: (args: {
|
||||
sourcePaths: string[]
|
||||
|
||||
@@ -2648,7 +2648,10 @@ const api = {
|
||||
rootPath: string
|
||||
connectionId?: string
|
||||
excludePaths?: string[]
|
||||
requestToken?: string
|
||||
}): Promise<string[]> => ipcRenderer.invoke('fs:listFiles', args),
|
||||
cancelListFiles: (args: { requestToken: string }): Promise<void> =>
|
||||
ipcRenderer.invoke('fs:cancelListFiles', args),
|
||||
search: (args: {
|
||||
query: string
|
||||
rootPath: string
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
* a git-focused app.
|
||||
*/
|
||||
import { spawn } from 'node:child_process'
|
||||
import { fileListingCancellationError } from '../shared/file-listing-cancellation'
|
||||
import type { SearchOptions, SearchResult } from './fs-handler-utils'
|
||||
import { buildGitLsFilesArgsForQuickOpen } from '../shared/quick-open-filter'
|
||||
import { expandQuickOpenGitFilesWithNestedRepos } from '../shared/quick-open-readdir-walk'
|
||||
@@ -31,8 +32,13 @@ import { buildRelayCommandEnv } from './relay-command-env'
|
||||
*/
|
||||
export function listFilesWithGit(
|
||||
rootPath: string,
|
||||
excludePathPrefixes: readonly string[] = []
|
||||
excludePathPrefixes: readonly string[] = [],
|
||||
options: { signal?: AbortSignal } = {}
|
||||
): Promise<string[]> {
|
||||
const { signal } = options
|
||||
if (signal?.aborted) {
|
||||
return Promise.reject(fileListingCancellationError(signal))
|
||||
}
|
||||
const gitPaths = new Set<string>()
|
||||
const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen(excludePathPrefixes)
|
||||
const children: {
|
||||
@@ -145,7 +151,7 @@ export function listFilesWithGit(
|
||||
})
|
||||
}
|
||||
|
||||
const killSurvivors = (): void => {
|
||||
const killSurvivors = (reason: string): void => {
|
||||
// Why: Promise.all returns after the first failed pass, but the sibling
|
||||
// git process can keep streaming on SSH unless we cancel it explicitly.
|
||||
for (const entry of children) {
|
||||
@@ -155,22 +161,35 @@ export function listFilesWithGit(
|
||||
if (entry.child.exitCode === null && entry.child.signalCode === null) {
|
||||
entry.child.kill()
|
||||
}
|
||||
entry.reject(new Error('git ls-files canceled after sibling failure'))
|
||||
entry.reject(new Error(reason))
|
||||
}
|
||||
}
|
||||
|
||||
// Why: a cancelled scan (workspace switch, superseded request) must stop
|
||||
// its git children right away instead of streaming a huge tree the caller
|
||||
// has already abandoned over the shared SSH channel.
|
||||
const onAbort = (): void => killSurvivors('git ls-files cancelled')
|
||||
signal?.addEventListener('abort', onAbort, { once: true })
|
||||
|
||||
return Promise.all([runGitLsFiles(primary), runGitLsFiles(ignoredPass)])
|
||||
.then(() =>
|
||||
expandQuickOpenGitFilesWithNestedRepos({
|
||||
rootPath,
|
||||
gitPaths,
|
||||
excludePathPrefixes
|
||||
excludePathPrefixes,
|
||||
signal
|
||||
})
|
||||
)
|
||||
.catch((err) => {
|
||||
killSurvivors()
|
||||
killSurvivors('git ls-files canceled after sibling failure')
|
||||
if (signal?.aborted) {
|
||||
throw fileListingCancellationError(signal)
|
||||
}
|
||||
throw err
|
||||
})
|
||||
.finally(() => {
|
||||
signal?.removeEventListener('abort', onAbort)
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
/**
|
||||
* Cancellation behavior of the relay file-list scanners (#7721): an aborted
|
||||
* scan must kill its child processes immediately and reject with a
|
||||
* cancellation error instead of streaming an abandoned tree to completion.
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { spawnMock } = vi.hoisted(() => ({
|
||||
spawnMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('child_process', () => ({
|
||||
spawn: spawnMock
|
||||
}))
|
||||
|
||||
import { EventEmitter } from 'node:events'
|
||||
import type { ChildProcess } from 'node:child_process'
|
||||
import { listFilesWithRg } from './fs-handler-list-files'
|
||||
import { listFilesWithGit } from './fs-handler-git-fallback'
|
||||
import { isFileListingCancellation } from '../shared/file-listing-cancellation'
|
||||
|
||||
function createMockProcess(): ChildProcess {
|
||||
const p = new EventEmitter() as unknown as ChildProcess
|
||||
;(p as unknown as Record<string, unknown>).stdout = new EventEmitter()
|
||||
;(
|
||||
(p as unknown as Record<string, unknown>).stdout as EventEmitter & {
|
||||
setEncoding: () => void
|
||||
}
|
||||
).setEncoding = vi.fn()
|
||||
;(p as unknown as Record<string, unknown>).stderr = new EventEmitter()
|
||||
;(p as unknown as Record<string, unknown>).kill = vi.fn()
|
||||
;(p as unknown as Record<string, unknown>).exitCode = null
|
||||
;(p as unknown as Record<string, unknown>).signalCode = null
|
||||
return p
|
||||
}
|
||||
|
||||
describe('relay list-files cancellation', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('listFilesWithRg kills both rg passes and rejects when aborted mid-flight', async () => {
|
||||
const primaryProc = createMockProcess()
|
||||
const ignoredProc = createMockProcess()
|
||||
spawnMock.mockImplementation((_cmd: string, args: string[]) =>
|
||||
args.includes('--no-ignore-vcs') ? ignoredProc : primaryProc
|
||||
)
|
||||
|
||||
const controller = new AbortController()
|
||||
const promise = listFilesWithRg('/remote/root', [], { signal: controller.signal })
|
||||
|
||||
// Partial output before the abort — must be discarded, not resolved.
|
||||
;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\n')
|
||||
controller.abort()
|
||||
|
||||
await expect(promise).rejects.toSatisfy(isFileListingCancellation)
|
||||
expect(primaryProc.kill).toHaveBeenCalled()
|
||||
expect(ignoredProc.kill).toHaveBeenCalled()
|
||||
|
||||
// Late close events after cancellation must not fire anything.
|
||||
primaryProc.emit('close', null, 'SIGTERM')
|
||||
ignoredProc.emit('close', null, 'SIGTERM')
|
||||
})
|
||||
|
||||
it('listFilesWithRg rejects without spawning when the signal is already aborted', async () => {
|
||||
const controller = new AbortController()
|
||||
controller.abort()
|
||||
|
||||
await expect(
|
||||
listFilesWithRg('/remote/root', [], { signal: controller.signal })
|
||||
).rejects.toSatisfy(isFileListingCancellation)
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('listFilesWithRg still resolves normally when a signal is provided but never aborted', async () => {
|
||||
const primaryProc = createMockProcess()
|
||||
const ignoredProc = createMockProcess()
|
||||
spawnMock.mockImplementation((_cmd: string, args: string[]) =>
|
||||
args.includes('--no-ignore-vcs') ? ignoredProc : primaryProc
|
||||
)
|
||||
|
||||
const controller = new AbortController()
|
||||
const promise = listFilesWithRg('/remote/root', [], { signal: controller.signal })
|
||||
|
||||
setTimeout(() => {
|
||||
;(primaryProc.stdout as unknown as EventEmitter).emit('data', 'src/index.ts\n')
|
||||
primaryProc.emit('close', 0, null)
|
||||
;(ignoredProc.stdout as unknown as EventEmitter).emit('data', 'dist/out.js\n')
|
||||
ignoredProc.emit('close', 0, null)
|
||||
}, 5)
|
||||
|
||||
await expect(promise).resolves.toEqual(['src/index.ts', 'dist/out.js'])
|
||||
})
|
||||
|
||||
it('listFilesWithGit kills both git passes and rejects when aborted mid-flight', async () => {
|
||||
const procs: ChildProcess[] = []
|
||||
spawnMock.mockImplementation(() => {
|
||||
const proc = createMockProcess()
|
||||
procs.push(proc)
|
||||
return proc
|
||||
})
|
||||
|
||||
const controller = new AbortController()
|
||||
const promise = listFilesWithGit('/remote/root', [], { signal: controller.signal })
|
||||
|
||||
expect(procs).toHaveLength(2)
|
||||
controller.abort()
|
||||
|
||||
await expect(promise).rejects.toSatisfy(isFileListingCancellation)
|
||||
expect(procs[0].kill).toHaveBeenCalled()
|
||||
expect(procs[1].kill).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('listFilesWithGit rejects without spawning when the signal is already aborted', async () => {
|
||||
const controller = new AbortController()
|
||||
controller.abort()
|
||||
|
||||
await expect(
|
||||
listFilesWithGit('/remote/root', [], { signal: controller.signal })
|
||||
).rejects.toSatisfy(isFileListingCancellation)
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -16,6 +16,7 @@
|
||||
* denied on a single subdir is expected on home-dir roots)
|
||||
*/
|
||||
import { spawn, type ChildProcess } from 'node:child_process'
|
||||
import { fileListingCancellationError } from '../shared/file-listing-cancellation'
|
||||
import {
|
||||
buildRgArgsForQuickOpen,
|
||||
normalizeQuickOpenRgLine,
|
||||
@@ -27,8 +28,13 @@ export const LIST_FILES_TIMEOUT_MS = 25_000
|
||||
|
||||
export function listFilesWithRg(
|
||||
rootPath: string,
|
||||
excludePathPrefixes: readonly string[] = []
|
||||
excludePathPrefixes: readonly string[] = [],
|
||||
options: { signal?: AbortSignal } = {}
|
||||
): Promise<string[]> {
|
||||
const { signal } = options
|
||||
if (signal?.aborted) {
|
||||
return Promise.reject(fileListingCancellationError(signal))
|
||||
}
|
||||
return new Promise((resolve, reject) => {
|
||||
const files = new Set<string>()
|
||||
let done = false
|
||||
@@ -177,7 +183,7 @@ export function listFilesWithRg(
|
||||
child.once('close', handleClose)
|
||||
})
|
||||
|
||||
const killSurvivors = (): void => {
|
||||
const killSurvivors = (reason: string): void => {
|
||||
// Why: when one pass rejects, Promise.all surfaces the error immediately
|
||||
// but the sibling rg keeps running up to LIST_FILES_TIMEOUT_MS. Kill it
|
||||
// so repeated Quick Open opens don't pile up orphan rg processes on the
|
||||
@@ -189,16 +195,30 @@ export function listFilesWithRg(
|
||||
if (entry.child.exitCode === null && entry.child.signalCode === null) {
|
||||
entry.child.kill()
|
||||
}
|
||||
entry.reject(new Error('rg list canceled after sibling failure'))
|
||||
entry.reject(new Error(reason))
|
||||
}
|
||||
}
|
||||
|
||||
// Why: a cancelled scan (workspace switch, superseded request) must stop
|
||||
// its rg children immediately instead of letting them walk the tree to
|
||||
// completion and flood the relay with stdout it will only discard.
|
||||
const onAbort = (): void => {
|
||||
if (done) {
|
||||
return
|
||||
}
|
||||
done = true
|
||||
killSurvivors('rg list cancelled')
|
||||
reject(fileListingCancellationError(signal))
|
||||
}
|
||||
signal?.addEventListener('abort', onAbort, { once: true })
|
||||
|
||||
Promise.all([runPass(primary), runPass(ignoredPass)])
|
||||
.then(() => {
|
||||
if (done) {
|
||||
return
|
||||
}
|
||||
done = true
|
||||
signal?.removeEventListener('abort', onAbort)
|
||||
resolve(Array.from(files))
|
||||
})
|
||||
.catch((err) => {
|
||||
@@ -206,7 +226,8 @@ export function listFilesWithRg(
|
||||
return
|
||||
}
|
||||
done = true
|
||||
killSurvivors()
|
||||
signal?.removeEventListener('abort', onAbort)
|
||||
killSurvivors('rg list canceled after sibling failure')
|
||||
reject(err instanceof Error ? err : new Error(String(err)))
|
||||
})
|
||||
})
|
||||
|
||||
@@ -23,10 +23,12 @@ import {
|
||||
*/
|
||||
export async function listFilesWithReaddir(
|
||||
rootPath: string,
|
||||
excludePathPrefixes: readonly string[] = []
|
||||
excludePathPrefixes: readonly string[] = [],
|
||||
options: { signal?: AbortSignal } = {}
|
||||
): Promise<string[]> {
|
||||
return listQuickOpenFilesWithReaddir(rootPath, {
|
||||
excludePathPrefixes,
|
||||
budget: createQuickOpenReaddirBudget()
|
||||
budget: createQuickOpenReaddirBudget(),
|
||||
signal: options.signal
|
||||
})
|
||||
}
|
||||
|
||||
+33
-5
@@ -17,6 +17,11 @@ import {
|
||||
} 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'
|
||||
import {
|
||||
isFileListingCancellation,
|
||||
throwIfFileListingCancelled
|
||||
} from '../shared/file-listing-cancellation'
|
||||
import { isQuickOpenReaddirBudgetError } from '../shared/quick-open-readdir-walk'
|
||||
import { buildExcludePathPrefixes } from '../shared/quick-open-filter'
|
||||
import { buildInstallRgMessage } from './fs-handler-install-rg'
|
||||
@@ -82,6 +87,7 @@ export class FsHandler {
|
||||
private dispatcher: RelayDispatcher
|
||||
private watches = new Map<string, WatchState>()
|
||||
private streamRegistry = new RelayStreamRegistry()
|
||||
private listFilesScans = new ListFilesScanCoordinator()
|
||||
|
||||
constructor(dispatcher: RelayDispatcher, _context: RelayContext) {
|
||||
this.dispatcher = dispatcher
|
||||
@@ -114,7 +120,7 @@ export class FsHandler {
|
||||
this.dispatcher.onRequest('fs.copy', (p) => this.copy(p))
|
||||
this.dispatcher.onRequest('fs.realpath', (p) => this.realpath(p))
|
||||
this.dispatcher.onRequest('fs.search', (p) => this.search(p))
|
||||
this.dispatcher.onRequest('fs.listFiles', (p) => this.listFiles(p))
|
||||
this.dispatcher.onRequest('fs.listFiles', (p, c) => this.listFiles(p, c))
|
||||
this.dispatcher.onRequest('fs.workspaceSpaceScan', (p, c) => this.workspaceSpaceScan(p, c))
|
||||
this.dispatcher.onRequest('fs.watch', (p, context) => this.watch(p, context))
|
||||
this.dispatcher.onNotification('fs.unwatch', (p, context) => this.unwatch(p, context))
|
||||
@@ -331,16 +337,33 @@ export class FsHandler {
|
||||
})
|
||||
}
|
||||
|
||||
private async listFiles(params: Record<string, unknown>): Promise<string[]> {
|
||||
private listFiles(params: Record<string, unknown>, context?: RequestContext): Promise<string[]> {
|
||||
const rootPath = expandTilde(params.rootPath as string)
|
||||
// Why: the main-to-relay RPC adds excludePaths so nested linked worktrees
|
||||
// don't get double-scanned. The shared helper validates the shape and
|
||||
// normalizes into root-relative prefixes; malformed input yields [] so
|
||||
// the request still succeeds (older apps omit the field entirely).
|
||||
const excludePathPrefixes = buildExcludePathPrefixes(rootPath, params.excludePaths)
|
||||
// Why #7721: full-tree scans are the relay's most expensive request; the
|
||||
// coordinator caps them at one per client, coalescing duplicates and
|
||||
// aborting a stale scan when the workspace changes or the host cancels.
|
||||
return this.listFilesScans.run({
|
||||
clientId: context?.clientId ?? 0,
|
||||
key: JSON.stringify([rootPath, excludePathPrefixes]),
|
||||
signal: context?.signal,
|
||||
start: (signal) => this.runListFilesScan(rootPath, excludePathPrefixes, signal)
|
||||
})
|
||||
}
|
||||
|
||||
private async runListFilesScan(
|
||||
rootPath: string,
|
||||
excludePathPrefixes: string[],
|
||||
signal: AbortSignal
|
||||
): Promise<string[]> {
|
||||
const rgAvailable = await checkRgAvailable()
|
||||
throwIfFileListingCancelled(signal)
|
||||
if (rgAvailable) {
|
||||
return listFilesWithRg(rootPath, excludePathPrefixes)
|
||||
return listFilesWithRg(rootPath, excludePathPrefixes, { signal })
|
||||
}
|
||||
// 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
|
||||
@@ -361,7 +384,7 @@ export class FsHandler {
|
||||
// budget errors into install-rg guidance; genuine git failures keep
|
||||
// their own messages.
|
||||
try {
|
||||
return await listFilesWithGit(rootPath, excludePathPrefixes)
|
||||
return await listFilesWithGit(rootPath, excludePathPrefixes, { signal })
|
||||
} catch (err) {
|
||||
if (isQuickOpenReaddirBudgetError(err)) {
|
||||
throw new Error(await buildInstallRgMessage(err))
|
||||
@@ -375,8 +398,13 @@ export class FsHandler {
|
||||
// problem, so translate the opaque cap error into actionable guidance
|
||||
// the user can act on directly from the error toast.
|
||||
try {
|
||||
return await listFilesWithReaddir(rootPath, excludePathPrefixes)
|
||||
return await listFilesWithReaddir(rootPath, excludePathPrefixes, { signal })
|
||||
} catch (err) {
|
||||
// Why: a cancelled scan is not an rg-availability problem; wrapping it
|
||||
// in install-rg guidance would surface bogus advice on the client.
|
||||
if (isFileListingCancellation(err)) {
|
||||
throw err
|
||||
}
|
||||
throw new Error(await buildInstallRgMessage(err))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,209 @@
|
||||
/**
|
||||
* End-to-end in-process regression tests for #7721: cancellable fs.listFiles.
|
||||
*
|
||||
* Wires the client-side SshChannelMultiplexer to the relay-side
|
||||
* RelayDispatcher + FsHandler through an in-memory pipe (no SSH). The scan
|
||||
* body is a controllable fake so the tests are deterministic:
|
||||
* - aborting the client request sends rpc.cancel and stops the relay scan,
|
||||
* - interactive fs.readDir is served while a scan is in flight,
|
||||
* - a scan for a different workspace supersedes the previous one,
|
||||
* - identical concurrent requests coalesce into one scan.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest'
|
||||
import { mkdtempSync, writeFileSync } from 'node:fs'
|
||||
import { rm } from 'node:fs/promises'
|
||||
import * as path from 'node:path'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
const { fakeListFiles } = vi.hoisted(() => {
|
||||
type ScanRecord = {
|
||||
rootPath: string
|
||||
signal: AbortSignal | undefined
|
||||
resolve: (files: string[]) => void
|
||||
}
|
||||
const scans: ScanRecord[] = []
|
||||
const fakeListFiles = Object.assign(
|
||||
vi.fn(
|
||||
(
|
||||
rootPath: string,
|
||||
_excludes: readonly string[] = [],
|
||||
options: { signal?: AbortSignal } = {}
|
||||
) =>
|
||||
new Promise<string[]>((resolve, reject) => {
|
||||
scans.push({ rootPath, signal: options.signal, resolve })
|
||||
options.signal?.addEventListener(
|
||||
'abort',
|
||||
() =>
|
||||
// Mirror the real scanners: surface the abort reason (e.g. the
|
||||
// "superseded" error) so the dispatcher reports it to the host.
|
||||
reject(
|
||||
options.signal?.reason instanceof Error
|
||||
? options.signal.reason
|
||||
: new Error('File listing cancelled')
|
||||
),
|
||||
{ once: true }
|
||||
)
|
||||
})
|
||||
),
|
||||
{ scans }
|
||||
)
|
||||
return { fakeListFiles }
|
||||
})
|
||||
|
||||
vi.mock('./fs-handler-utils', async (importOriginal) => {
|
||||
const original = (await importOriginal()) as Record<string, unknown>
|
||||
return {
|
||||
...original,
|
||||
checkRgAvailable: () => Promise.resolve(true),
|
||||
listFilesWithRg: fakeListFiles
|
||||
}
|
||||
})
|
||||
|
||||
import {
|
||||
SshChannelMultiplexer,
|
||||
type MultiplexerTransport
|
||||
} from '../main/ssh/ssh-channel-multiplexer'
|
||||
import { RelayDispatcher } from './dispatcher'
|
||||
import { RelayContext } from './context'
|
||||
import { FsHandler } from './fs-handler'
|
||||
import { LIST_FILES_SUPERSEDED_MESSAGE } from './fs-list-files-scan-coordinator'
|
||||
|
||||
async function flushPipe(): Promise<void> {
|
||||
// The in-memory pipe defers each hop with setImmediate; a few macrotask
|
||||
// turns guarantee request/notification frames have crossed both directions.
|
||||
for (let i = 0; i < 10; i++) {
|
||||
await new Promise((resolve) => setImmediate(resolve))
|
||||
}
|
||||
}
|
||||
|
||||
describe('Integration: cancellable fs.listFiles (#7721)', () => {
|
||||
let tmpDir: string
|
||||
let mux: SshChannelMultiplexer
|
||||
let dispatcher: RelayDispatcher
|
||||
let fsHandler: FsHandler
|
||||
|
||||
beforeEach(() => {
|
||||
fakeListFiles.mockClear()
|
||||
fakeListFiles.scans.length = 0
|
||||
tmpDir = mkdtempSync(path.join(tmpdir(), 'relay-listfiles-cancel-'))
|
||||
|
||||
let relayFeedFn: (data: Buffer) => void
|
||||
const clientDataCallbacks: ((data: Buffer) => void)[] = []
|
||||
const clientTransport: MultiplexerTransport = {
|
||||
write: (data: Buffer) => {
|
||||
setImmediate(() => relayFeedFn?.(data))
|
||||
},
|
||||
onData: (cb) => {
|
||||
clientDataCallbacks.push(cb)
|
||||
},
|
||||
onClose: () => {}
|
||||
}
|
||||
dispatcher = new RelayDispatcher((data: Buffer) => {
|
||||
setImmediate(() => {
|
||||
for (const cb of clientDataCallbacks) {
|
||||
cb(data)
|
||||
}
|
||||
})
|
||||
})
|
||||
relayFeedFn = (data: Buffer) => dispatcher.feed(data)
|
||||
fsHandler = new FsHandler(dispatcher, new RelayContext())
|
||||
mux = new SshChannelMultiplexer(clientTransport)
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
mux.dispose()
|
||||
dispatcher.dispose()
|
||||
fsHandler.dispose()
|
||||
await rm(tmpDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it('serves fs.readDir while a full-tree scan is in flight', async () => {
|
||||
writeFileSync(path.join(tmpDir, 'a.txt'), 'a')
|
||||
|
||||
const scanPromise = mux.request('fs.listFiles', { rootPath: '/big/workspace' })
|
||||
await flushPipe()
|
||||
expect(fakeListFiles.scans).toHaveLength(1)
|
||||
|
||||
// The interactive request must complete while the scan is still pending.
|
||||
const entries = (await mux.request('fs.readDir', { dirPath: tmpDir })) as { name: string }[]
|
||||
expect(entries.map((e) => e.name)).toEqual(['a.txt'])
|
||||
|
||||
fakeListFiles.scans[0].resolve(['src/index.ts'])
|
||||
await expect(scanPromise).resolves.toEqual(['src/index.ts'])
|
||||
})
|
||||
|
||||
it('client abort sends rpc.cancel and stops the relay-side scan', async () => {
|
||||
const controller = new AbortController()
|
||||
const scanPromise = mux.request(
|
||||
'fs.listFiles',
|
||||
{ rootPath: '/big/workspace' },
|
||||
{ signal: controller.signal }
|
||||
)
|
||||
await flushPipe()
|
||||
expect(fakeListFiles.scans).toHaveLength(1)
|
||||
expect(fakeListFiles.scans[0].signal?.aborted).toBe(false)
|
||||
|
||||
controller.abort()
|
||||
await expect(scanPromise).rejects.toThrow('was cancelled')
|
||||
await flushPipe()
|
||||
|
||||
// The rpc.cancel notification must have aborted the scan on the relay.
|
||||
expect(fakeListFiles.scans[0].signal?.aborted).toBe(true)
|
||||
})
|
||||
|
||||
it('a scan for a different workspace supersedes the previous scan', async () => {
|
||||
const first = mux.request('fs.listFiles', { rootPath: '/workspace/a' })
|
||||
// Attach the rejection handler up front: the error response arrives on a
|
||||
// macrotask inside flushPipe, before any later `await expect` could.
|
||||
const firstOutcome = first.then(
|
||||
() => null,
|
||||
(err: Error) => err
|
||||
)
|
||||
await flushPipe()
|
||||
expect(fakeListFiles.scans).toHaveLength(1)
|
||||
|
||||
const second = mux.request('fs.listFiles', { rootPath: '/workspace/b' })
|
||||
await flushPipe()
|
||||
expect(fakeListFiles.scans).toHaveLength(2)
|
||||
|
||||
// The stale scan is aborted and its request fails fast — it does not run
|
||||
// to completion behind the new one and does not hit the 30s timeout.
|
||||
expect(fakeListFiles.scans[0].signal?.aborted).toBe(true)
|
||||
const firstError = await firstOutcome
|
||||
expect(firstError?.message).toContain(LIST_FILES_SUPERSEDED_MESSAGE)
|
||||
|
||||
fakeListFiles.scans[1].resolve(['b.ts'])
|
||||
await expect(second).resolves.toEqual(['b.ts'])
|
||||
})
|
||||
|
||||
it('identical concurrent requests coalesce into a single scan', async () => {
|
||||
const first = mux.request('fs.listFiles', { rootPath: '/workspace/a' })
|
||||
const second = mux.request('fs.listFiles', { rootPath: '/workspace/a' })
|
||||
await flushPipe()
|
||||
|
||||
expect(fakeListFiles.scans).toHaveLength(1)
|
||||
fakeListFiles.scans[0].resolve(['shared.ts'])
|
||||
await expect(first).resolves.toEqual(['shared.ts'])
|
||||
await expect(second).resolves.toEqual(['shared.ts'])
|
||||
})
|
||||
|
||||
it('cancelling one coalesced requester keeps the scan alive for the other', async () => {
|
||||
const controller = new AbortController()
|
||||
const first = mux.request(
|
||||
'fs.listFiles',
|
||||
{ rootPath: '/workspace/a' },
|
||||
{ signal: controller.signal }
|
||||
)
|
||||
const second = mux.request('fs.listFiles', { rootPath: '/workspace/a' })
|
||||
await flushPipe()
|
||||
expect(fakeListFiles.scans).toHaveLength(1)
|
||||
|
||||
controller.abort()
|
||||
await expect(first).rejects.toThrow('was cancelled')
|
||||
await flushPipe()
|
||||
expect(fakeListFiles.scans[0].signal?.aborted).toBe(false)
|
||||
|
||||
fakeListFiles.scans[0].resolve(['still.ts'])
|
||||
await expect(second).resolves.toEqual(['still.ts'])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,222 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
LIST_FILES_SUPERSEDED_MESSAGE,
|
||||
ListFilesScanCoordinator
|
||||
} from './fs-list-files-scan-coordinator'
|
||||
import {
|
||||
FileListingCancelledError,
|
||||
isFileListingCancellation
|
||||
} from '../shared/file-listing-cancellation'
|
||||
|
||||
type Deferred = {
|
||||
promise: Promise<string[]>
|
||||
resolve: (files: string[]) => void
|
||||
reject: (error: Error) => void
|
||||
}
|
||||
|
||||
function deferred(): Deferred {
|
||||
let resolve!: (files: string[]) => void
|
||||
let reject!: (error: Error) => void
|
||||
const promise = new Promise<string[]>((res, rej) => {
|
||||
resolve = res
|
||||
reject = rej
|
||||
})
|
||||
return { promise, resolve, reject }
|
||||
}
|
||||
|
||||
/** Scan runner that resolves/rejects on demand and rejects when aborted. */
|
||||
function controllableScan(): {
|
||||
start: (signal: AbortSignal) => Promise<string[]>
|
||||
starts: AbortSignal[]
|
||||
finish: (files: string[]) => void
|
||||
} {
|
||||
const starts: AbortSignal[] = []
|
||||
let current: Deferred | null = null
|
||||
return {
|
||||
starts,
|
||||
start: (signal: AbortSignal) => {
|
||||
starts.push(signal)
|
||||
const scan = deferred()
|
||||
current = scan
|
||||
signal.addEventListener(
|
||||
'abort',
|
||||
() => {
|
||||
scan.reject(
|
||||
signal.reason instanceof Error ? signal.reason : new FileListingCancelledError()
|
||||
)
|
||||
},
|
||||
{ once: true }
|
||||
)
|
||||
return scan.promise
|
||||
},
|
||||
finish: (files: string[]) => current?.resolve(files)
|
||||
}
|
||||
}
|
||||
|
||||
describe('ListFilesScanCoordinator', () => {
|
||||
it('coalesces same-key concurrent requests into one scan', async () => {
|
||||
const coordinator = new ListFilesScanCoordinator()
|
||||
const scan = controllableScan()
|
||||
|
||||
const first = coordinator.run({ clientId: 1, key: 'a', start: scan.start })
|
||||
const second = coordinator.run({ clientId: 1, key: 'a', start: scan.start })
|
||||
|
||||
expect(scan.starts).toHaveLength(1)
|
||||
scan.finish(['x.ts'])
|
||||
await expect(first).resolves.toEqual(['x.ts'])
|
||||
await expect(second).resolves.toEqual(['x.ts'])
|
||||
})
|
||||
|
||||
it('supersedes a different-key scan: aborts the old one and rejects it fast', async () => {
|
||||
const coordinator = new ListFilesScanCoordinator()
|
||||
const scan = controllableScan()
|
||||
|
||||
const first = coordinator.run({ clientId: 1, key: 'workspace-a', start: scan.start })
|
||||
const second = coordinator.run({ clientId: 1, key: 'workspace-b', start: scan.start })
|
||||
|
||||
expect(scan.starts).toHaveLength(2)
|
||||
expect(scan.starts[0].aborted).toBe(true)
|
||||
await expect(first).rejects.toThrow(LIST_FILES_SUPERSEDED_MESSAGE)
|
||||
await first.catch((err) => expect(isFileListingCancellation(err)).toBe(true))
|
||||
|
||||
expect(scan.starts[1].aborted).toBe(false)
|
||||
scan.finish(['b.ts'])
|
||||
await expect(second).resolves.toEqual(['b.ts'])
|
||||
})
|
||||
|
||||
it('keeps scans from different clients independent', async () => {
|
||||
const coordinator = new ListFilesScanCoordinator()
|
||||
const scanA = controllableScan()
|
||||
const scanB = controllableScan()
|
||||
|
||||
const first = coordinator.run({ clientId: 1, key: 'workspace-a', start: scanA.start })
|
||||
const second = coordinator.run({ clientId: 2, key: 'workspace-b', start: scanB.start })
|
||||
|
||||
expect(scanA.starts[0].aborted).toBe(false)
|
||||
scanA.finish(['a.ts'])
|
||||
scanB.finish(['b.ts'])
|
||||
await expect(first).resolves.toEqual(['a.ts'])
|
||||
await expect(second).resolves.toEqual(['b.ts'])
|
||||
})
|
||||
|
||||
it('aborts the scan when its only requester cancels', async () => {
|
||||
const coordinator = new ListFilesScanCoordinator()
|
||||
const scan = controllableScan()
|
||||
const requester = new AbortController()
|
||||
|
||||
const result = coordinator.run({
|
||||
clientId: 1,
|
||||
key: 'a',
|
||||
signal: requester.signal,
|
||||
start: scan.start
|
||||
})
|
||||
|
||||
requester.abort()
|
||||
expect(scan.starts[0].aborted).toBe(true)
|
||||
await expect(result).rejects.toSatisfy(isFileListingCancellation)
|
||||
})
|
||||
|
||||
it('keeps a coalesced scan alive while another requester still waits', async () => {
|
||||
const coordinator = new ListFilesScanCoordinator()
|
||||
const scan = controllableScan()
|
||||
const first = new AbortController()
|
||||
const second = new AbortController()
|
||||
|
||||
const firstResult = coordinator.run({
|
||||
clientId: 1,
|
||||
key: 'a',
|
||||
signal: first.signal,
|
||||
start: scan.start
|
||||
})
|
||||
const secondResult = coordinator.run({
|
||||
clientId: 1,
|
||||
key: 'a',
|
||||
signal: second.signal,
|
||||
start: scan.start
|
||||
})
|
||||
|
||||
first.abort()
|
||||
expect(scan.starts[0].aborted).toBe(false)
|
||||
// The aborting requester observes its own cancellation immediately,
|
||||
// while the shared scan keeps running for the sibling.
|
||||
await expect(firstResult).rejects.toSatisfy(isFileListingCancellation)
|
||||
|
||||
second.abort()
|
||||
expect(scan.starts[0].aborted).toBe(true)
|
||||
await expect(secondResult).rejects.toSatisfy(isFileListingCancellation)
|
||||
})
|
||||
|
||||
it('rejects an aborted coalesced requester while the sibling still resolves', async () => {
|
||||
const coordinator = new ListFilesScanCoordinator()
|
||||
const scan = controllableScan()
|
||||
const first = new AbortController()
|
||||
|
||||
const firstResult = coordinator.run({
|
||||
clientId: 1,
|
||||
key: 'a',
|
||||
signal: first.signal,
|
||||
start: scan.start
|
||||
})
|
||||
const secondResult = coordinator.run({ clientId: 1, key: 'a', start: scan.start })
|
||||
|
||||
first.abort()
|
||||
scan.finish(['kept.ts'])
|
||||
|
||||
await expect(firstResult).rejects.toSatisfy(isFileListingCancellation)
|
||||
await expect(secondResult).resolves.toEqual(['kept.ts'])
|
||||
})
|
||||
|
||||
it('rejects immediately when the requester is already cancelled', async () => {
|
||||
const coordinator = new ListFilesScanCoordinator()
|
||||
const start = vi.fn()
|
||||
const requester = new AbortController()
|
||||
requester.abort()
|
||||
|
||||
await expect(
|
||||
coordinator.run({
|
||||
clientId: 1,
|
||||
key: 'a',
|
||||
signal: requester.signal,
|
||||
start: start as unknown as (signal: AbortSignal) => Promise<string[]>
|
||||
})
|
||||
).rejects.toSatisfy(isFileListingCancellation)
|
||||
expect(start).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('starts a fresh scan for the same key after the previous one settled', async () => {
|
||||
const coordinator = new ListFilesScanCoordinator()
|
||||
const scan = controllableScan()
|
||||
|
||||
const first = coordinator.run({ clientId: 1, key: 'a', start: scan.start })
|
||||
scan.finish(['old.ts'])
|
||||
await expect(first).resolves.toEqual(['old.ts'])
|
||||
|
||||
const second = coordinator.run({ clientId: 1, key: 'a', start: scan.start })
|
||||
expect(scan.starts).toHaveLength(2)
|
||||
scan.finish(['new.ts'])
|
||||
await expect(second).resolves.toEqual(['new.ts'])
|
||||
})
|
||||
|
||||
it('does not join a scan that is already aborted; starts a replacement', async () => {
|
||||
const coordinator = new ListFilesScanCoordinator()
|
||||
const scan = controllableScan()
|
||||
const requester = new AbortController()
|
||||
|
||||
const first = coordinator.run({
|
||||
clientId: 1,
|
||||
key: 'a',
|
||||
signal: requester.signal,
|
||||
start: scan.start
|
||||
})
|
||||
requester.abort()
|
||||
await expect(first).rejects.toSatisfy(isFileListingCancellation)
|
||||
|
||||
// The aborted entry may still be in the map until its promise settles;
|
||||
// a new same-key request must get a live scan, not the dead one.
|
||||
const second = coordinator.run({ clientId: 1, key: 'a', start: scan.start })
|
||||
expect(scan.starts).toHaveLength(2)
|
||||
expect(scan.starts[1].aborted).toBe(false)
|
||||
scan.finish(['fresh.ts'])
|
||||
await expect(second).resolves.toEqual(['fresh.ts'])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,104 @@
|
||||
/**
|
||||
* Single-flight coordinator for fs.listFiles full-tree scans (#7721).
|
||||
*
|
||||
* Why: rapid workspace switching used to stack N concurrent full-tree scans
|
||||
* on the single-threaded relay and its one SSH channel, starving small
|
||||
* interactive fs.readDir/fs.stat requests past their 30s timeout. This
|
||||
* coordinator guarantees at most one scan in flight per client:
|
||||
* - a request for the same root/excludes joins the in-flight scan
|
||||
* (Quick Open + file-explorer filter share one scan),
|
||||
* - a request for a different root supersedes it — the old scan is aborted
|
||||
* and its request fails fast instead of running to completion (this also
|
||||
* protects against older Orca clients that never send rpc.cancel),
|
||||
* - when every joined requester cancels (rpc.cancel / client detach), the
|
||||
* scan is aborted so abandoned work stops immediately.
|
||||
*/
|
||||
import {
|
||||
FileListingCancelledError,
|
||||
fileListingCancellationError
|
||||
} from '../shared/file-listing-cancellation'
|
||||
|
||||
export const LIST_FILES_SUPERSEDED_MESSAGE = 'File listing superseded by a newer request'
|
||||
|
||||
type ScanEntry = {
|
||||
key: string
|
||||
controller: AbortController
|
||||
promise: Promise<string[]>
|
||||
attachedCount: number
|
||||
}
|
||||
|
||||
export class ListFilesScanCoordinator {
|
||||
private readonly scansByClient = new Map<number, ScanEntry>()
|
||||
|
||||
run(opts: {
|
||||
clientId: number
|
||||
key: string
|
||||
signal?: AbortSignal
|
||||
start: (signal: AbortSignal) => Promise<string[]>
|
||||
}): Promise<string[]> {
|
||||
const { clientId, key, signal, start } = opts
|
||||
if (signal?.aborted) {
|
||||
return Promise.reject(fileListingCancellationError(signal))
|
||||
}
|
||||
|
||||
const existing = this.scansByClient.get(clientId)
|
||||
if (existing && existing.key === key && !existing.controller.signal.aborted) {
|
||||
return this.attach(existing, signal)
|
||||
}
|
||||
if (existing) {
|
||||
existing.controller.abort(new FileListingCancelledError(LIST_FILES_SUPERSEDED_MESSAGE))
|
||||
}
|
||||
|
||||
const controller = new AbortController()
|
||||
const entry: ScanEntry = {
|
||||
key,
|
||||
controller,
|
||||
promise: Promise.resolve([]),
|
||||
attachedCount: 0
|
||||
}
|
||||
this.scansByClient.set(clientId, entry)
|
||||
entry.promise = start(controller.signal).finally(() => {
|
||||
if (this.scansByClient.get(clientId) === entry) {
|
||||
this.scansByClient.delete(clientId)
|
||||
}
|
||||
})
|
||||
return this.attach(entry, signal)
|
||||
}
|
||||
|
||||
private attach(entry: ScanEntry, signal?: AbortSignal): Promise<string[]> {
|
||||
entry.attachedCount++
|
||||
if (!signal) {
|
||||
return entry.promise
|
||||
}
|
||||
// Why: an aborting requester must see its own cancellation even though
|
||||
// the shared scan keeps running (and may later resolve) for coalesced
|
||||
// siblings — so each attachment gets its own promise.
|
||||
return new Promise<string[]>((resolve, reject) => {
|
||||
let settled = false
|
||||
const settle = (complete: () => void): void => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
settled = true
|
||||
signal.removeEventListener('abort', onAbort)
|
||||
complete()
|
||||
}
|
||||
const onAbort = (): void =>
|
||||
settle(() => {
|
||||
entry.attachedCount--
|
||||
const cancellation = fileListingCancellationError(signal)
|
||||
// Why: only stop the shared scan when nobody is left waiting on it —
|
||||
// one requester cancelling must not break a coalesced sibling.
|
||||
if (entry.attachedCount <= 0) {
|
||||
entry.controller.abort(cancellation)
|
||||
}
|
||||
reject(cancellation)
|
||||
})
|
||||
signal.addEventListener('abort', onAbort, { once: true })
|
||||
entry.promise.then(
|
||||
(files) => settle(() => resolve(files)),
|
||||
(error) => settle(() => reject(error))
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -10,9 +10,11 @@ import type { AppState } from '@/store/types'
|
||||
import { useRuntimeFileListForWorktree, type RuntimeFileListState } from './quick-open-file-list'
|
||||
|
||||
const listRuntimeFilesMock = vi.hoisted(() => vi.fn())
|
||||
const cancelRuntimeFileListMock = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('@/runtime/runtime-file-client', () => ({
|
||||
listRuntimeFiles: listRuntimeFilesMock
|
||||
listRuntimeFiles: listRuntimeFilesMock,
|
||||
cancelRuntimeFileList: cancelRuntimeFileListMock
|
||||
}))
|
||||
|
||||
const initialAppState = useAppStore.getInitialState()
|
||||
@@ -104,6 +106,7 @@ async function renderProbe(args: {
|
||||
beforeEach(() => {
|
||||
useAppStore.setState(initialAppState, true)
|
||||
listRuntimeFilesMock.mockReset().mockResolvedValue(['packages/app/package.json'])
|
||||
cancelRuntimeFileListMock.mockReset()
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
@@ -155,9 +158,37 @@ describe('useRuntimeFileListForWorktree', () => {
|
||||
}),
|
||||
{
|
||||
rootPath: '/srv/platform',
|
||||
excludePaths: undefined
|
||||
excludePaths: undefined,
|
||||
requestToken: expect.any(String)
|
||||
}
|
||||
)
|
||||
expect(states.at(-1)?.files).toEqual(['packages/app/package.json'])
|
||||
})
|
||||
|
||||
it('cancels the in-flight scan with the same request token on unmount (#7721)', async () => {
|
||||
const workspaceKey = folderWorkspaceKey('folder-workspace-1')
|
||||
|
||||
useAppStore.setState({
|
||||
folderWorkspaces: [makeFolderWorkspace({ connectionId: 'ssh-1' })],
|
||||
projectGroups: [makeProjectGroup({ connectionId: 'ssh-1' })],
|
||||
repos: [],
|
||||
worktreesByRepo: {}
|
||||
} as Partial<AppState>)
|
||||
|
||||
const root = await renderProbe({
|
||||
enabled: true,
|
||||
onState: () => {},
|
||||
worktreeId: workspaceKey
|
||||
})
|
||||
await waitForListRuntimeFilesCall()
|
||||
|
||||
const [listContext, listRequest] = listRuntimeFilesMock.mock.calls[0]
|
||||
expect(cancelRuntimeFileListMock).not.toHaveBeenCalled()
|
||||
|
||||
await act(async () => {
|
||||
root.unmount()
|
||||
})
|
||||
|
||||
expect(cancelRuntimeFileListMock).toHaveBeenCalledWith(listContext, listRequest.requestToken)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -3,8 +3,9 @@ import { useEffect, useMemo, useRef, useState } from 'react'
|
||||
import type { Worktree } from '../../../shared/types'
|
||||
import { isWindowsAbsolutePathLike } from '../../../shared/cross-platform-path'
|
||||
import { getConnectionIdFromState } from '@/lib/connection-context'
|
||||
import { createBrowserUuid } from '@/lib/browser-uuid'
|
||||
import { getSettingsForWorktreeRuntimeOwner } from '@/lib/worktree-runtime-owner'
|
||||
import { listRuntimeFiles } from '@/runtime/runtime-file-client'
|
||||
import { cancelRuntimeFileList, listRuntimeFiles } from '@/runtime/runtime-file-client'
|
||||
import { useAppStore } from '@/store'
|
||||
import { useWorktreesForRepo } from '@/store/selectors'
|
||||
|
||||
@@ -152,21 +153,21 @@ export function useRuntimeFileListForWorktree({
|
||||
setLoading(true)
|
||||
|
||||
const excludePaths = excludeRequest.paths.length > 0 ? excludeRequest.paths : undefined
|
||||
const requestToken = createBrowserUuid()
|
||||
const requestContext = {
|
||||
// Why: Quick Open lists files for the selected workspace. It must
|
||||
// follow that workspace's owner host, not the globally focused host.
|
||||
settings: getSettingsForWorktreeRuntimeOwner(useAppStore.getState(), worktreeId),
|
||||
worktreeId,
|
||||
worktreePath,
|
||||
connectionId
|
||||
}
|
||||
|
||||
void listRuntimeFiles(
|
||||
{
|
||||
// Why: Quick Open lists files for the selected workspace. It must
|
||||
// follow that workspace's owner host, not the globally focused host.
|
||||
settings: getSettingsForWorktreeRuntimeOwner(useAppStore.getState(), worktreeId),
|
||||
worktreeId,
|
||||
worktreePath,
|
||||
connectionId
|
||||
},
|
||||
{
|
||||
rootPath: worktreePath,
|
||||
excludePaths
|
||||
}
|
||||
)
|
||||
void listRuntimeFiles(requestContext, {
|
||||
rootPath: worktreePath,
|
||||
excludePaths,
|
||||
requestToken
|
||||
})
|
||||
.then((result) => {
|
||||
if (!cancelled) {
|
||||
setFiles(result)
|
||||
@@ -186,6 +187,11 @@ export function useRuntimeFileListForWorktree({
|
||||
|
||||
return () => {
|
||||
cancelled = true
|
||||
// Why #7721: switching workspaces (or closing the palette) must abort
|
||||
// the previous full-tree scan host- and relay-side. Over SSH, abandoned
|
||||
// scans otherwise stack up and starve fs.readDir/fs.stat past their
|
||||
// 30s timeout ("Could not load files for this workspace").
|
||||
cancelRuntimeFileList(requestContext, requestToken)
|
||||
}
|
||||
}, [connectionId, enabled, excludeRequest, requestKey, target.canList, worktreeId, worktreePath])
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ preload API plus remote fallbacks; keeping route coverage together makes local
|
||||
versus environment behavior easy to audit. */
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
cancelRuntimeFileList,
|
||||
copyRuntimePath,
|
||||
createRuntimePath,
|
||||
deleteRuntimePath,
|
||||
@@ -37,6 +38,8 @@ const fsDeletePath = vi.fn()
|
||||
const fsStat = vi.fn()
|
||||
const fsPathExists = vi.fn()
|
||||
const fsSearch = vi.fn()
|
||||
const fsListFiles = vi.fn()
|
||||
const fsCancelListFiles = vi.fn()
|
||||
const fsDownloadFile = vi.fn()
|
||||
const fsSaveDownloadedFile = vi.fn()
|
||||
const fsStartDownloadedFile = vi.fn()
|
||||
@@ -63,6 +66,9 @@ beforeEach(() => {
|
||||
fsStat.mockReset()
|
||||
fsPathExists.mockReset()
|
||||
fsSearch.mockReset()
|
||||
fsListFiles.mockReset()
|
||||
fsCancelListFiles.mockReset()
|
||||
fsCancelListFiles.mockResolvedValue(undefined)
|
||||
fsDownloadFile.mockReset()
|
||||
fsSaveDownloadedFile.mockReset()
|
||||
fsStartDownloadedFile.mockReset()
|
||||
@@ -102,6 +108,8 @@ beforeEach(() => {
|
||||
stat: fsStat,
|
||||
pathExists: fsPathExists,
|
||||
search: fsSearch,
|
||||
listFiles: fsListFiles,
|
||||
cancelListFiles: fsCancelListFiles,
|
||||
downloadFile: fsDownloadFile,
|
||||
saveDownloadedFile: fsSaveDownloadedFile,
|
||||
startDownloadedFile: fsStartDownloadedFile,
|
||||
@@ -1357,6 +1365,56 @@ describe('runtime file client', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('passes the cancellation token through the IPC file listing path (#7721)', async () => {
|
||||
fsListFiles.mockResolvedValue(['src/index.ts'])
|
||||
|
||||
await expect(
|
||||
listRuntimeFiles(
|
||||
{
|
||||
settings: {},
|
||||
worktreeId: 'wt-1',
|
||||
worktreePath: '/remote/repo',
|
||||
connectionId: 'ssh-1'
|
||||
},
|
||||
{
|
||||
rootPath: '/remote/repo',
|
||||
requestToken: 'token-1'
|
||||
}
|
||||
)
|
||||
).resolves.toEqual(['src/index.ts'])
|
||||
|
||||
expect(fsListFiles).toHaveBeenCalledWith({
|
||||
rootPath: '/remote/repo',
|
||||
connectionId: 'ssh-1',
|
||||
excludePaths: undefined,
|
||||
requestToken: 'token-1'
|
||||
})
|
||||
})
|
||||
|
||||
it('cancelRuntimeFileList aborts the IPC listing but not environment listings (#7721)', () => {
|
||||
cancelRuntimeFileList(
|
||||
{
|
||||
settings: {},
|
||||
worktreeId: 'wt-1',
|
||||
worktreePath: '/remote/repo',
|
||||
connectionId: 'ssh-1'
|
||||
},
|
||||
'token-1'
|
||||
)
|
||||
expect(fsCancelListFiles).toHaveBeenCalledWith({ requestToken: 'token-1' })
|
||||
|
||||
fsCancelListFiles.mockClear()
|
||||
cancelRuntimeFileList(
|
||||
{
|
||||
settings: { activeRuntimeEnvironmentId: 'env-1' },
|
||||
worktreeId: 'wt-1',
|
||||
worktreePath: '/remote/repo'
|
||||
},
|
||||
'token-2'
|
||||
)
|
||||
expect(fsCancelListFiles).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('routes markdown document listing and stat through the selected runtime', async () => {
|
||||
runtimeEnvironmentCall.mockResolvedValue({
|
||||
id: 'rpc-1',
|
||||
|
||||
@@ -654,14 +654,15 @@ export async function searchRuntimeFiles(
|
||||
|
||||
export async function listRuntimeFiles(
|
||||
context: RuntimeFileOperationArgs,
|
||||
args: { rootPath: string; excludePaths?: string[] }
|
||||
args: { rootPath: string; excludePaths?: string[]; requestToken?: string }
|
||||
): Promise<string[]> {
|
||||
const target = getActiveRuntimeTarget(context.settings)
|
||||
if (target.kind !== 'environment' || !context.worktreeId) {
|
||||
return window.api.fs.listFiles({
|
||||
rootPath: args.rootPath,
|
||||
connectionId: context.connectionId,
|
||||
excludePaths: args.excludePaths
|
||||
excludePaths: args.excludePaths,
|
||||
requestToken: args.requestToken
|
||||
})
|
||||
}
|
||||
return callRuntimeRpc<string[]>(
|
||||
@@ -675,6 +676,24 @@ export async function listRuntimeFiles(
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Best-effort abort of an in-flight listRuntimeFiles call (#7721). Switching
|
||||
* workspaces must stop the previous workspace's full-tree scan — over SSH an
|
||||
* abandoned scan keeps loading the relay and starves fs.readDir/fs.stat.
|
||||
*/
|
||||
export function cancelRuntimeFileList(
|
||||
context: RuntimeFileOperationArgs,
|
||||
requestToken: string
|
||||
): void {
|
||||
const target = getActiveRuntimeTarget(context.settings)
|
||||
if (target.kind !== 'environment' || !context.worktreeId) {
|
||||
void window.api.fs.cancelListFiles({ requestToken }).catch(() => {
|
||||
/* cancellation is advisory; the request path has its own timeouts */
|
||||
})
|
||||
}
|
||||
// Environment runtimes bound files.listAll with their own RPC timeout.
|
||||
}
|
||||
|
||||
export async function listRuntimeMarkdownDocuments(
|
||||
context: RuntimeFileOperationArgs,
|
||||
rootPath: string
|
||||
|
||||
@@ -1509,6 +1509,10 @@ function createFileApi(): NonNullable<Partial<PreloadApi>['fs']> {
|
||||
)
|
||||
return result.files.map((entry) => entry.relativePath)
|
||||
},
|
||||
cancelListFiles: async () => {
|
||||
// Why: the paired-web path lists files over runtime RPC with its own
|
||||
// request timeout; there is no host-side scan to abort from here.
|
||||
},
|
||||
search: async (args) => {
|
||||
const file = await resolveRuntimeFilePath(args.rootPath)
|
||||
return callRuntimeResult<SearchResult>('files.search', {
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
/**
|
||||
* Cancellation identity for workspace file-list scans (fs.listFiles).
|
||||
*
|
||||
* Why a dedicated error class: the readdir fallback wraps budget errors into
|
||||
* "install rg" guidance and Quick Open surfaces load errors verbatim, so a
|
||||
* cancelled scan must stay distinguishable from a genuine listing failure at
|
||||
* every layer (relay rg/git/readdir, main-process local scan, renderer).
|
||||
*/
|
||||
export class FileListingCancelledError extends Error {
|
||||
constructor(message = 'File listing cancelled') {
|
||||
super(message)
|
||||
this.name = 'FileListingCancelledError'
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the rejection for an aborted scan, preferring the abort reason so a
|
||||
* superseded scan reports "superseded" rather than a generic cancellation.
|
||||
* Only FileListingCancelledError reasons pass through — a bare abort() sets
|
||||
* signal.reason to a DOMException that must not leak past the classifier.
|
||||
*/
|
||||
export function fileListingCancellationError(signal?: AbortSignal): Error {
|
||||
const reason = signal?.reason
|
||||
if (reason instanceof FileListingCancelledError) {
|
||||
return reason
|
||||
}
|
||||
return new FileListingCancelledError()
|
||||
}
|
||||
|
||||
export function isFileListingCancellation(error: unknown): boolean {
|
||||
return error instanceof FileListingCancelledError
|
||||
}
|
||||
|
||||
export function throwIfFileListingCancelled(signal?: AbortSignal): void {
|
||||
if (signal?.aborted) {
|
||||
throw fileListingCancellationError(signal)
|
||||
}
|
||||
}
|
||||
@@ -24,6 +24,7 @@ import {
|
||||
listQuickOpenFilesWithReaddir,
|
||||
parseQuickOpenGitLsFilesEntry
|
||||
} from './quick-open-readdir-walk'
|
||||
import { isFileListingCancellation } from './file-listing-cancellation'
|
||||
|
||||
const tempDirs: string[] = []
|
||||
const SHA1 = '0123456789abcdef0123456789abcdef01234567'
|
||||
@@ -259,4 +260,35 @@ describe('quick-open readdir walk', () => {
|
||||
})
|
||||
).resolves.toEqual(['packages/app [one] space/src/main.ts'])
|
||||
})
|
||||
|
||||
it('stops the walk with a cancellation error when the signal aborts (#7721)', async () => {
|
||||
const root = await makeTempRoot()
|
||||
await writeRel(root, 'src/a.ts')
|
||||
await writeRel(root, 'src/b.ts')
|
||||
|
||||
const controller = new AbortController()
|
||||
controller.abort()
|
||||
|
||||
const rejection = listQuickOpenFilesWithReaddir(root, { signal: controller.signal })
|
||||
await expect(rejection).rejects.toSatisfy(isFileListingCancellation)
|
||||
// Cancellation must never be mistaken for a budget error, which callers
|
||||
// translate into "install rg" guidance.
|
||||
await rejection.catch((err) => expect(isQuickOpenReaddirBudgetError(err)).toBe(false))
|
||||
})
|
||||
|
||||
it('stops nested-repo expansion when the signal aborts (#7721)', async () => {
|
||||
const root = await makeTempRoot()
|
||||
await writeRel(root, 'src/kept.ts')
|
||||
|
||||
const controller = new AbortController()
|
||||
controller.abort()
|
||||
|
||||
await expect(
|
||||
expandQuickOpenGitFilesWithNestedRepos({
|
||||
rootPath: root,
|
||||
gitPaths: ['src/kept.ts'],
|
||||
signal: controller.signal
|
||||
})
|
||||
).rejects.toSatisfy(isFileListingCancellation)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { lstat, readdir } from 'node:fs/promises'
|
||||
import { join, relative } from 'node:path'
|
||||
import { throwIfFileListingCancelled } from './file-listing-cancellation'
|
||||
import {
|
||||
HIDDEN_DIR_BLOCKLIST,
|
||||
shouldExcludeQuickOpenRelPath,
|
||||
@@ -157,6 +158,7 @@ export async function listQuickOpenFilesWithReaddir(
|
||||
opts: {
|
||||
excludePathPrefixes?: readonly string[]
|
||||
budget?: QuickOpenReaddirBudget
|
||||
signal?: AbortSignal
|
||||
} = {}
|
||||
): Promise<string[]> {
|
||||
const files: string[] = []
|
||||
@@ -164,6 +166,9 @@ export async function listQuickOpenFilesWithReaddir(
|
||||
const excludePathPrefixes = opts.excludePathPrefixes ?? []
|
||||
|
||||
async function walk(dir: string): Promise<void> {
|
||||
// Why: an abandoned scan (workspace switch) must stop consuming IO and
|
||||
// event-loop time on the single-threaded relay, not just run to budget.
|
||||
throwIfFileListingCancelled(opts.signal)
|
||||
assertWithinDeadline(budget)
|
||||
|
||||
let entries
|
||||
@@ -176,6 +181,7 @@ export async function listQuickOpenFilesWithReaddir(
|
||||
}
|
||||
|
||||
for (const entry of entries) {
|
||||
throwIfFileListingCancelled(opts.signal)
|
||||
assertWithinDeadline(budget)
|
||||
|
||||
const name = entry.name
|
||||
@@ -206,6 +212,7 @@ export async function expandQuickOpenGitFilesWithNestedRepos(opts: {
|
||||
gitPaths: Iterable<string>
|
||||
excludePathPrefixes?: readonly string[]
|
||||
budget?: QuickOpenReaddirBudget
|
||||
signal?: AbortSignal
|
||||
}): Promise<string[]> {
|
||||
const files = new Set<string>()
|
||||
const excludePathPrefixes = opts.excludePathPrefixes ?? []
|
||||
@@ -224,6 +231,7 @@ export async function expandQuickOpenGitFilesWithNestedRepos(opts: {
|
||||
}
|
||||
|
||||
for (const rawPath of opts.gitPaths) {
|
||||
throwIfFileListingCancelled(opts.signal)
|
||||
assertWithinDeadline(budget)
|
||||
|
||||
const { kind, relPath } = await classifyQuickOpenGitEntry(opts.rootPath, rawPath)
|
||||
@@ -240,7 +248,8 @@ export async function expandQuickOpenGitFilesWithNestedRepos(opts: {
|
||||
// nested repo so the walk prunes excluded subtrees during traversal
|
||||
// instead of burning the shared budget and filtering them at the end.
|
||||
excludePathPrefixes: rebaseExcludePrefixesForNestedRepo(excludePathPrefixes, relPath),
|
||||
budget
|
||||
budget,
|
||||
signal: opts.signal
|
||||
})
|
||||
for (const nestedFile of nestedFiles) {
|
||||
addFinalPath(`${relPath}/${nestedFile}`)
|
||||
|
||||
Reference in New Issue
Block a user