diff --git a/src/relay/fs-list-files-cancel.integration.test.ts b/src/relay/fs-list-files-cancel.integration.test.ts index 588ddb5fe51..e4e689e947c 100644 --- a/src/relay/fs-list-files-cancel.integration.test.ts +++ b/src/relay/fs-list-files-cancel.integration.test.ts @@ -66,7 +66,6 @@ import { 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 { // The in-memory pipe defers each hop with setImmediate; a few macrotask @@ -151,29 +150,31 @@ describe('Integration: cancellable fs.listFiles (#7721)', () => { 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 - ) + // Why #7769: two independent callers on one SSH connection (one relay + // clientId) — e.g. the editor's markdown-document scan (no excludePaths) and + // Quick Open (nested-worktree excludePaths) — carry different scan keys. + // Neither may supersede the other: the earlier one must complete and return + // its own result, not fail with a "superseded" error nobody triggered. + it('runs two concurrent unrelated listings without superseding either', async () => { + const markdown = mux.request('fs.listFiles', { rootPath: '/workspace/a' }) await flushPipe() expect(fakeListFiles.scans).toHaveLength(1) - const second = mux.request('fs.listFiles', { rootPath: '/workspace/b' }) + const quickOpen = mux.request('fs.listFiles', { + rootPath: '/workspace/a', + excludePaths: ['/workspace/a/worktrees/feature'] + }) 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) + // The first scan is not aborted by the second — both stay live. + expect(fakeListFiles.scans[0].signal?.aborted).toBe(false) + expect(fakeListFiles.scans[1].signal?.aborted).toBe(false) - fakeListFiles.scans[1].resolve(['b.ts']) - await expect(second).resolves.toEqual(['b.ts']) + fakeListFiles.scans[0].resolve(['README.md']) + fakeListFiles.scans[1].resolve(['src/index.ts']) + await expect(markdown).resolves.toEqual(['README.md']) + await expect(quickOpen).resolves.toEqual(['src/index.ts']) }) it('identical concurrent requests coalesce into a single scan', async () => { diff --git a/src/relay/fs-list-files-scan-coordinator.test.ts b/src/relay/fs-list-files-scan-coordinator.test.ts index a142c120cdb..73f75da218c 100644 --- a/src/relay/fs-list-files-scan-coordinator.test.ts +++ b/src/relay/fs-list-files-scan-coordinator.test.ts @@ -1,8 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { - LIST_FILES_SUPERSEDED_MESSAGE, - ListFilesScanCoordinator -} from './fs-list-files-scan-coordinator' +import { ListFilesScanCoordinator } from './fs-list-files-scan-coordinator' import { FileListingCancelledError, isFileListingCancellation @@ -67,20 +64,27 @@ describe('ListFilesScanCoordinator', () => { await expect(second).resolves.toEqual(['x.ts']) }) - it('supersedes a different-key scan: aborts the old one and rejects it fast', async () => { + // Why #7769: two unrelated callers on one relay client (one SSH connection) + // — e.g. the editor's markdown-document scan and Quick Open — carry + // different keys. Neither may supersede the other: the loser used to reject + // with the "superseded" error even though nobody cancelled it. + it('runs different-key scans concurrently instead of superseding (#7769)', async () => { const coordinator = new ListFilesScanCoordinator() - const scan = controllableScan() + const scanA = controllableScan() + const scanB = 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 }) + const first = coordinator.run({ clientId: 1, key: 'workspace-a', start: scanA.start }) + const second = coordinator.run({ clientId: 1, key: 'workspace-b', start: scanB.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)) + // Each caller gets its own live scan; neither aborts the other. + expect(scanA.starts).toHaveLength(1) + expect(scanB.starts).toHaveLength(1) + expect(scanA.starts[0].aborted).toBe(false) + expect(scanB.starts[0].aborted).toBe(false) - expect(scan.starts[1].aborted).toBe(false) - scan.finish(['b.ts']) + scanA.finish(['a.ts']) + scanB.finish(['b.ts']) + await expect(first).resolves.toEqual(['a.ts']) await expect(second).resolves.toEqual(['b.ts']) }) diff --git a/src/relay/fs-list-files-scan-coordinator.ts b/src/relay/fs-list-files-scan-coordinator.ts index 6e6e30c5f0d..a6a42b5820b 100644 --- a/src/relay/fs-list-files-scan-coordinator.ts +++ b/src/relay/fs-list-files-scan-coordinator.ts @@ -4,21 +4,21 @@ * 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: + * coordinator guarantees at most one scan in flight per (client, scan key): * - 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), + * - a request for a DIFFERENT root/excludes runs as its own concurrent + * single-flight scan. Why #7769: two independent callers on one SSH + * connection — e.g. the editor's markdown-document scan (no excludes) and + * Quick Open (nested-worktree excludes) — share one relay clientId. Keying + * only by clientId let a newer request supersede an older unrelated one, + * surfacing a spurious "superseded"/empty-list failure to a caller that + * never cancelled. Abandoned scans are still stopped via their requester's + * rpc.cancel (see attach below), not by evicting a sibling caller, * - 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' +import { fileListingCancellationError } from '../shared/file-listing-cancellation' type ScanEntry = { key: string @@ -28,7 +28,7 @@ type ScanEntry = { } export class ListFilesScanCoordinator { - private readonly scansByClient = new Map() + private readonly scansByClient = new Map>() run(opts: { clientId: number @@ -41,12 +41,14 @@ export class ListFilesScanCoordinator { return Promise.reject(fileListingCancellationError(signal)) } - const existing = this.scansByClient.get(clientId) - if (existing && existing.key === key && !existing.controller.signal.aborted) { + let byKey = this.scansByClient.get(clientId) + const existing = byKey?.get(key) + if (existing && !existing.controller.signal.aborted) { return this.attach(existing, signal) } - if (existing) { - existing.controller.abort(new FileListingCancelledError(LIST_FILES_SUPERSEDED_MESSAGE)) + if (!byKey) { + byKey = new Map() + this.scansByClient.set(clientId, byKey) } const controller = new AbortController() @@ -56,10 +58,14 @@ export class ListFilesScanCoordinator { promise: Promise.resolve([]), attachedCount: 0 } - this.scansByClient.set(clientId, entry) + byKey.set(key, entry) entry.promise = start(controller.signal).finally(() => { - if (this.scansByClient.get(clientId) === entry) { - this.scansByClient.delete(clientId) + const scans = this.scansByClient.get(clientId) + if (scans?.get(key) === entry) { + scans.delete(key) + if (scans.size === 0) { + this.scansByClient.delete(clientId) + } } }) return this.attach(entry, signal)