diff --git a/src/main/worktree-retirement-backfill-scan.test.ts b/src/main/worktree-retirement-backfill-scan.test.ts index f5957b46e88..f7e3b922663 100644 --- a/src/main/worktree-retirement-backfill-scan.test.ts +++ b/src/main/worktree-retirement-backfill-scan.test.ts @@ -1,27 +1,49 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { - RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS, RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS, RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS, - getOutstandingRetirementBackfillScanCount, - resetRetirementBackfillScanStateForTests, + type RetirementScanResult, runRetirementBackfillScan } from './worktree-retirement-backfill-scan' +const found = (names: string[], complete = true): RetirementScanResult => ({ + names: new Set(names), + complete +}) + /** A scan the test settles by hand, standing in for a `readdir` on a stalled NFS/SMB/WSL mount. */ -function stallingScan(): { run: () => Promise>; finish: (names: string[]) => void } { - let settle: (names: Set) => void = () => {} +function stallingScan(): { + run: () => Promise + finish: (names: string[]) => void + fail: () => void +} { + let settle: (result: RetirementScanResult) => void = () => {} + let reject: (error: unknown) => void = () => {} return { - run: () => new Promise>((resolve) => (settle = resolve)), - finish: (names) => settle(new Set(names)) + run: () => + new Promise((resolve, rejectScan) => { + settle = resolve + reject = rejectScan + }), + finish: (names) => settle(found(names)), + // A recovering mount usually errors the blocked call rather than answering it. + fail: () => reject(new Error('EIO')) } } +/** Drive one namespace to the state where its listing is abandoned but still stuck in the kernel. */ +async function stallPastDeadline(store: object, scanKey: string) { + const scan = stallingScan() + const pending = runRetirementBackfillScan(store, scanKey, scan.run) + const settled = expect(pending).rejects.toThrow(/exceeded/) + await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS) + await settled + return scan +} + describe('runRetirementBackfillScan', () => { beforeEach(() => { vi.useFakeTimers() - // A stalled scan is never settled by the test that started it, so its slot would leak forward. - resetRetirementBackfillScanStateForTests() }) afterEach(() => { @@ -29,39 +51,63 @@ describe('runRetirementBackfillScan', () => { }) it('gives up on a listing that never returns instead of blocking create forever', async () => { - const scan = stallingScan() - - const names = runRetirementBackfillScan({}, 'ns', scan.run) - const settled = expect(names).rejects.toThrow(/exceeded/) - await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS) - - await settled + await stallPastDeadline({}, 'ns') }) - it('retries a timed-out namespace once its backoff lapses, rather than wedging until restart', async () => { + it('does not stack a second listing while the first is still stuck', async () => { + // The deadline abandons a listing, it cannot cancel one. Retrying on the backoff alone would + // put another stuck `readdir` on the same wedged mount every 60s until the libuv pool starves. const store = {} - const first = stallingScan() - const second = vi.fn(async () => new Set(['nautilus'])) + await stallPastDeadline(store, 'ns') + const retry = vi.fn(async () => found(['nautilus'])) - const stalled = runRetirementBackfillScan(store, 'ns', first.run) - const settled = expect(stalled).rejects.toThrow(/exceeded/) - await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS) - await settled + await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS * 10) - // Why the backoff: an unabortable `readdir` still holds a libuv thread, so re-probing on every - // create would starve the pool. Until it lapses the failure is served from the memo. - await expect(runRetirementBackfillScan(store, 'ns', second)).rejects.toThrow(/exceeded/) - expect(second).not.toHaveBeenCalled() + await expect(runRetirementBackfillScan(store, 'ns', retry)).rejects.toThrow(/exceeded/) + expect(retry).not.toHaveBeenCalled() + }) + it('retries once the stuck listing settles and the backoff has lapsed', async () => { + const store = {} + const stalled = await stallPastDeadline(store, 'ns') + const retry = vi.fn(async () => found(['nautilus'])) + + // The mount comes back: the kernel errors the blocked call, freeing the namespace to relist. + stalled.fail() await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS) - await expect(runRetirementBackfillScan(store, 'ns', second)).resolves.toEqual( + + await expect(runRetirementBackfillScan(store, 'ns', retry)).resolves.toEqual( + new Set(['nautilus']) + ) + }) + + it('serves the failure from the memo until the backoff lapses', async () => { + const store = {} + const stalled = await stallPastDeadline(store, 'ns') + const retry = vi.fn(async () => found(['nautilus'])) + stalled.fail() + + await expect(runRetirementBackfillScan(store, 'ns', retry)).rejects.toThrow(/exceeded/) + expect(retry).not.toHaveBeenCalled() + }) + + it('never lets one wedged namespace stop another from scanning', async () => { + // A process-wide budget would let a single bad mount spend it on its own retries and starve + // every healthy repo, which is strictly worse than the wedge it replaces. + const store = {} + await stallPastDeadline(store, 'stuck-a') + await stallPastDeadline(store, 'stuck-b') + await stallPastDeadline(store, 'stuck-c') + + const healthy = vi.fn(async () => found(['nautilus'])) + await expect(runRetirementBackfillScan(store, 'healthy-ns', healthy)).resolves.toEqual( new Set(['nautilus']) ) }) it('scans a namespace once and shares the answer with every later caller', async () => { const store = {} - const scan = vi.fn(async () => new Set(['nautilus'])) + const scan = vi.fn(async () => found(['nautilus'])) await expect(runRetirementBackfillScan(store, 'ns', scan)).resolves.toEqual( new Set(['nautilus']) @@ -73,50 +119,12 @@ describe('runRetirementBackfillScan', () => { expect(scan).toHaveBeenCalledTimes(1) }) - it('keeps one namespace stall from failing another', async () => { - const store = {} - const stuck = stallingScan() - const healthy = vi.fn(async () => new Set(['nautilus'])) - - const stalled = runRetirementBackfillScan(store, 'stuck-ns', stuck.run) - const settled = expect(stalled).rejects.toThrow(/exceeded/) - await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS) - await settled - - await expect(runRetirementBackfillScan(store, 'healthy-ns', healthy)).resolves.toEqual( - new Set(['nautilus']) - ) - }) - - it('never defers a healthy machine, however many namespaces seed at once', async () => { - // Nested workspaces give every repo its own scan key, so several first-time backfills are - // routinely in flight together. Counting merely-in-flight listings would reject the third and - // let its create pick a name against an unseeded registry. - const scans = Array.from({ length: RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS + 2 }, () => - stallingScan() - ) - const pending = scans.map((scan, index) => - runRetirementBackfillScan({}, `ns-${index}`, scan.run) - ) - expect(getOutstandingRetirementBackfillScanCount()).toBe(0) - - scans.forEach((scan, index) => scan.finish([`name-${index}`])) - await expect(Promise.all(pending)).resolves.toEqual( - scans.map((_, index) => new Set([`name-${index}`])) - ) - }) - it('keeps a listing that lands after the deadline instead of discarding the answer', async () => { - // The WSL gate admits one scan at a time and allows it 60s — four times this deadline — so a - // late-but-correct listing is the normal case there, not an edge case. + // The WSL gate admits a single scan at a time and allows it 60s — four times this deadline — + // so a late-but-correct listing is the normal case there, not an edge case. const store = {} - const slow = stallingScan() - const rescan = vi.fn(async () => new Set(['orca'])) - - const pending = runRetirementBackfillScan(store, 'ns', slow.run) - const settled = expect(pending).rejects.toThrow(/exceeded/) - await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS) - await settled + const slow = await stallPastDeadline(store, 'ns') + const rescan = vi.fn(async () => found(['orca'])) slow.finish(['nautilus']) await vi.advanceTimersByTimeAsync(0) @@ -127,75 +135,24 @@ describe('runRetirementBackfillScan', () => { expect(rescan).not.toHaveBeenCalled() }) - it('does not arm a backoff on a namespace it never got to scan', async () => { - // A deferred namespace never touched the wedged mount; penalising it would spread one bad - // mount's outage to repos on healthy disks. - const stalls = Array.from({ length: RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS }, stallingScan) - for (const [index, stall] of stalls.entries()) { - const pending = runRetirementBackfillScan({}, `ns-${index}`, stall.run) - const settled = expect(pending).rejects.toThrow(/exceeded/) - await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS) - await settled - } - + it('does not memoize an incomplete answer, so a refused source is retried', async () => { const store = {} - const healthy = vi.fn(async () => new Set(['nautilus'])) - await expect(runRetirementBackfillScan(store, 'ns-healthy', healthy)).rejects.toThrow( - /deferred/ - ) + const partial = vi.fn(async () => found(['nautilus'], false)) + const whole = vi.fn(async () => found(['nautilus', 'orca'])) - // A slot frees, and the very next create succeeds — no backoff to wait out. - stalls[0].finish([]) - await vi.advanceTimersByTimeAsync(0) - await expect(runRetirementBackfillScan(store, 'ns-healthy', healthy)).resolves.toEqual( + await expect(runRetirementBackfillScan(store, 'ns', partial)).resolves.toEqual( new Set(['nautilus']) ) - }) - it('stops stacking listings once the abandoned ones reach the cap', async () => { - // The deadline abandons a listing, it cannot cancel one. Without a cap every lapsed backoff - // stacks another stuck `readdir` on the same wedged mount until the libuv pool is starved. - const stalls = Array.from({ length: RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS }, stallingScan) - for (const [index, stall] of stalls.entries()) { - const pending = runRetirementBackfillScan({}, `ns-${index}`, stall.run) - const settled = expect(pending).rejects.toThrow(/exceeded/) - await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS) - await settled - } - expect(getOutstandingRetirementBackfillScanCount()).toBe( - RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS - ) - - const overflow = vi.fn(async () => new Set(['nautilus'])) - await expect(runRetirementBackfillScan({}, 'ns-overflow', overflow)).rejects.toThrow(/deferred/) - expect(overflow).not.toHaveBeenCalled() - expect(getOutstandingRetirementBackfillScanCount()).toBe( - RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS - ) - }) - - it('frees the slot when an abandoned listing finally settles, so scanning resumes', async () => { - const stalls = Array.from({ length: RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS }, stallingScan) - for (const [index, stall] of stalls.entries()) { - const pending = runRetirementBackfillScan({}, `ns-${index}`, stall.run) - const settled = expect(pending).rejects.toThrow(/exceeded/) - await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS) - await settled - } - - // The mount comes back and the kernel releases the abandoned call. - stalls[0].finish([]) - await vi.advanceTimersByTimeAsync(0) - - const recovered = vi.fn(async () => new Set(['nautilus'])) - await expect(runRetirementBackfillScan({}, 'ns-recovered', recovered)).resolves.toEqual( - new Set(['nautilus']) + await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS) + await expect(runRetirementBackfillScan(store, 'ns', whole)).resolves.toEqual( + new Set(['nautilus', 'orca']) ) }) it('does not let one store inherit another store scan memo', async () => { - const first = vi.fn(async () => new Set(['nautilus'])) - const second = vi.fn(async () => new Set(['orca'])) + const first = vi.fn(async () => found(['nautilus'])) + const second = vi.fn(async () => found(['orca'])) await runRetirementBackfillScan({}, 'ns', first) await expect(runRetirementBackfillScan({}, 'ns', second)).resolves.toEqual(new Set(['orca'])) diff --git a/src/main/worktree-retirement-backfill-scan.ts b/src/main/worktree-retirement-backfill-scan.ts index 40d9f249d9b..8ca5b26ccd5 100644 --- a/src/main/worktree-retirement-backfill-scan.ts +++ b/src/main/worktree-retirement-backfill-scan.ts @@ -5,68 +5,63 @@ export const RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS = 15_000 /** How long a failed scan stays failed, so a stalled mount is not re-probed on every create. */ export const RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS = 60_000 -/** Listings that have outlived their deadline and are still holding a thread, process-wide. - * - * The deadline abandons a scan, it cannot cancel one: an unabortable `readdir` keeps its libuv - * threadpool thread until the OS releases it, and that pool defaults to four. So the backoff alone - * only paces the leak — each lapse still stacks another stuck thread. Only listings past their - * deadline count, so healthy scans (which finish in milliseconds) never deny each other a slot. - * This bounds what retirement discovery can hold; it is not a guarantee the pool stays healthy, - * since the WSL gate and its close lane hold threads of their own. Two leaves headroom for both. */ -export const RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS = 2 +export type RetirementScanResult = { + names: Set + /** False when a source was refused, so the partial answer must not be memoized as final. */ + complete: boolean +} type BackfillScan = { /** Resolves with the discovered names, or rejects once the deadline lapses. */ names: Promise> /** Monotonic ms this scan may be retried after; 0 while it is in flight or has succeeded. */ retryAfter: number + /** Its listing has not settled. Retrying now would stack a second one on the same mount. */ + outstanding: boolean } const scansByStore = new WeakMap>() -/** Process-wide on purpose: the threadpool these listings occupy is process-wide too. */ -let outstandingScans = 0 - /** Monotonic, like the WSL gate's own stuck timer: wall time misjudges a backoff across laptop * sleep or an NTP step, either pinning a namespace in its failure memo or ending it early. */ function monotonicNow(): number { return performance.now() } -export function getOutstandingRetirementBackfillScanCount(): number { - return outstandingScans -} - -/** A test that stalls a scan can never settle it, so the counter would leak into the next test. */ -export function resetRetirementBackfillScanStateForTests(): void { - outstandingScans = 0 -} - -function withScanDeadline(scan: Promise>): Promise> { - return new Promise>((resolve, reject) => { +function withScanDeadline(scan: Promise): Promise { + return new Promise((resolve, reject) => { const timer = setTimeout(() => { reject( new Error(`retirement backfill scan exceeded ${RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS}ms`) ) }, RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS) timer.unref?.() - void scan.then(resolve, reject).finally(() => clearTimeout(timer)) + void scan + .then( + () => resolve(), + (error: unknown) => reject(error) + ) + .finally(() => clearTimeout(timer)) }) } /** Memoizes the one-time retirement discovery scan per store and cwd namespace. * - * A success is kept for the process lifetime — a name never un-spends, so rescanning could only - * lose entries. A failure is not: the original cache held the scan Promise forever and its - * `readdir` had no deadline, so one stalled listing wedged this create and every later + * A complete success is kept for the process lifetime — a name never un-spends, so rescanning + * could only lose entries. A failure is not: the original cache held the scan Promise forever and + * its `readdir` had no deadline, so one stalled listing wedged this create and every later * generated-name create for the namespace until Orca restarted. * - * Retrying a failure is therefore allowed, but only while fewer than - * `RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS` listings are still stuck past their deadline. */ + * The deadline abandons a listing, it cannot cancel one — an unabortable `readdir` keeps its libuv + * threadpool thread until the OS releases it, and that pool defaults to four. So a retry is only + * allowed once the previous listing has actually settled. Without that, each lapsed backoff stacks + * another stuck thread on the same wedged mount until the process loses filesystem access. Keeping + * the rule per namespace rather than process-wide is deliberate: a global budget lets one bad mount + * spend it on its own retries and starve every healthy repo. */ export function runRetirementBackfillScan( store: object, scanKey: string, - scan: () => Promise> + scan: () => Promise ): Promise> { let storeScans = scansByStore.get(store) if (!storeScans) { @@ -74,52 +69,52 @@ export function runRetirementBackfillScan( scansByStore.set(store, storeScans) } const cached = storeScans.get(scanKey) - if (cached && (cached.retryAfter === 0 || monotonicNow() < cached.retryAfter)) { + if ( + cached && + (cached.retryAfter === 0 || cached.outstanding || monotonicNow() < cached.retryAfter) + ) { return cached.names } - if (outstandingScans >= RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS) { - // Deliberately not memoized: this namespace never got to start a listing, so arming its - // backoff would spread one wedged mount's outage to repos on healthy disks. Rejecting costs - // nothing, so the next create can just try again once a slot frees. - return Promise.reject( - new Error( - `retirement backfill scan deferred: ${outstandingScans} listings already outstanding` - ) - ) + const entry: BackfillScan = { + names: Promise.resolve(new Set()), + retryAfter: 0, + outstanding: true } - const entry: BackfillScan = { names: Promise.resolve(new Set()), retryAfter: 0 } const started = scan() - let settled = false - let holdsSlot = false - const releaseSlot = (): void => { - settled = true - if (holdsSlot) { - outstandingScans -= 1 - holdsSlot = false + void started.then( + (result) => { + entry.outstanding = false + // A listing that lands after the deadline still holds the right answer, and under WSL gate + // contention that is the common case rather than the edge one — the gate admits a single + // scan at a time and gives it 60s, four times this deadline. Dropping it would leave the + // namespace unseeded on exactly the mounts this feature exists to cover. No liveness check + // is needed: once a retry replaces this entry nothing can read it again, so the write is + // inert. + if (entry.retryAfter !== 0) { + entry.names = Promise.resolve(result.names) + entry.retryAfter = result.complete + ? 0 + : monotonicNow() + RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS + } + }, + () => { + entry.outstanding = false } - } - // Registered before the deadline wrapper so `settled` is already true when its catch runs, which - // is how a scan that failed fast is told apart from one that outlived its deadline. - void started.then((names) => { - releaseSlot() - // A listing that lands after the deadline still holds the right answer, and under WSL gate - // contention that is the common case rather than the edge one — the gate admits a single scan - // at a time and gives it 60s, four times this deadline. Dropping it would leave the namespace - // unseeded on exactly the mounts this feature exists to cover. No liveness check is needed: - // once a retry replaces this entry in the map nothing can read it again, so the write is inert. - if (entry.retryAfter !== 0) { - entry.names = Promise.resolve(names) - entry.retryAfter = 0 - } - }, releaseSlot) - entry.names = withScanDeadline(started).catch((error: unknown) => { - if (!settled) { - outstandingScans += 1 - holdsSlot = true - } - entry.retryAfter = monotonicNow() + RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS - throw error - }) + ) + entry.names = withScanDeadline(started) + .then(async () => { + const result = await started + // A refused source means names are missing. Serve what was found — under-retiring is the + // direction that leaks — but leave the entry retryable instead of memoizing the hole. + entry.retryAfter = result.complete + ? 0 + : monotonicNow() + RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS + return result.names + }) + .catch((error: unknown) => { + entry.retryAfter = monotonicNow() + RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS + throw error + }) storeScans.set(scanKey, entry) return entry.names } diff --git a/src/main/worktree-retirement-backfill-stall.test.ts b/src/main/worktree-retirement-backfill-stall.test.ts index b8daa50c80e..490696949c9 100644 --- a/src/main/worktree-retirement-backfill-stall.test.ts +++ b/src/main/worktree-retirement-backfill-stall.test.ts @@ -11,11 +11,20 @@ vi.mock('node:fs/promises', async (importOriginal) => ({ })) const { ensureRetiredWorktreeNamesBackfilled } = await import('./worktree-name-retirement') -const { - RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS, - RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS, - resetRetirementBackfillScanStateForTests -} = await import('./worktree-retirement-backfill-scan') +const { RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS, RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS } = + await import('./worktree-retirement-backfill-scan') + +/** A `readdir` the test releases by hand, standing in for a wedged NFS/SMB/WSL mount. */ +function stalledReaddir(): { install: () => void; release: () => void } { + let release: () => void = () => {} + return { + install: () => + readdirMock.mockImplementation( + () => new Promise((_resolve, reject) => (release = () => reject(new Error('EIO')))) + ), + release: () => release() + } +} const repo = { id: 'repo-a', @@ -41,8 +50,6 @@ describe('retirement backfill against a stalled listing', () => { beforeEach(() => { vi.useFakeTimers() readdirMock.mockReset() - // The stalled `readdir` of the previous test never settles, so its slot would leak forward. - resetRetirementBackfillScanStateForTests() }) afterEach(() => { @@ -61,7 +68,8 @@ describe('retirement backfill against a stalled listing', () => { }) it('recovers the namespace once the mount comes back, instead of staying wedged until restart', async () => { - readdirMock.mockImplementation(() => new Promise(() => {})) + const stall = stalledReaddir() + stall.install() const store = backfillStore() const stalled = ensureRetiredWorktreeNamesBackfilled(store, repo, settings) @@ -69,10 +77,31 @@ describe('retirement backfill against a stalled listing', () => { await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS) await settled + // The mount comes back: the kernel releases the abandoned call, which is what frees the + // namespace to be listed again. Retrying while it is still stuck would only stack a second one. + stall.release() readdirMock.mockResolvedValue([{ name: 'nautilus', isDirectory: () => true }]) await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS) await ensureRetiredWorktreeNamesBackfilled(store, repo, settings) expect(store.merged).toContain('nautilus') }) + + it('does not re-probe a mount whose previous listing is still stuck', async () => { + stalledReaddir().install() + const store = backfillStore() + + const stalled = ensureRetiredWorktreeNamesBackfilled(store, repo, settings) + const settled = expect(stalled).rejects.toThrow(/exceeded/) + await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS) + await settled + + const callsWhileStuck = readdirMock.mock.calls.length + await vi.advanceTimersByTimeAsync(RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS * 5) + await expect(ensureRetiredWorktreeNamesBackfilled(store, repo, settings)).rejects.toThrow( + /exceeded/ + ) + + expect(readdirMock.mock.calls.length).toBe(callsWhileStuck) + }) }) diff --git a/src/main/worktree-retirement-discovery-wsl.test.ts b/src/main/worktree-retirement-discovery-wsl.test.ts index fc3314f3908..7c589af4b23 100644 --- a/src/main/worktree-retirement-discovery-wsl.test.ts +++ b/src/main/worktree-retirement-discovery-wsl.test.ts @@ -74,21 +74,39 @@ describe('retirement discovery on WSL', () => { expect(runWslTranscriptFsTaskMock).not.toHaveBeenCalled() }) - it('fails the scan when the gate refuses, rather than memoizing a half-read answer', async () => { - // A swallowed refusal would cache "nothing is retired" for the process lifetime — the one - // direction that reissues a cwd whose conversation history is still on the distro. + it('reports a gate refusal as incomplete but still returns what the host side could read', async () => { + // Marking it incomplete is what stops "nothing is retired" being cached for the process + // lifetime — the one direction that reissues a cwd whose history is still on the distro. + // Throwing instead would be worse than the pre-split behaviour: for a WSL repo the UNC + // workspace root is listed first, so a stuck 9P route would also discard the plain, readable + // Windows-side bucket scan that needs no distro access at all. runWslTranscriptFsTaskMock.mockRejectedValue( new WslTranscriptFsError('timeout', 'filesystem access is taking too long') ) + const hostHome = await mkdtemp(join(tmpdir(), 'orca-wsl-host-home-')) + try { + await mkdir( + join( + hostHome, + '.claude', + 'projects', + `--wsl-localhost-ubuntu-home-ada-orca-workspaces-${FIRST}` + ), + { recursive: true } + ) - await expect( - discoverRetiredWorktreeNames({ + const retired = await discoverRetiredWorktreeNames({ workspaceRoots: [DISTRO_ROOT], - home: '/nonexistent-home', + home: hostHome, env: {}, resolveWslHome: async () => '\\\\wsl.localhost\\Ubuntu\\home\\ada' }) - ).rejects.toBeInstanceOf(WslTranscriptFsError) + + expect(retired.complete).toBe(false) + expect(retired.names).toEqual(new Set([FIRST])) + } finally { + await rm(hostHome, { force: true, recursive: true }) + } }) it('keeps a deleted WSL workspace name spent, so the next create cannot reuse its cwd', async () => { @@ -109,8 +127,10 @@ describe('retirement discovery on WSL', () => { resolveWslHome: async () => distroHome }) - expect(retired).toEqual(new Set([FIRST])) - expect(createRetiredNameLookup({ exhaustedTiers: 0, names: [...retired] })(FIRST)).toBe(true) + expect(retired.names).toEqual(new Set([FIRST])) + expect(createRetiredNameLookup({ exhaustedTiers: 0, names: [...retired.names] })(FIRST)).toBe( + true + ) } finally { await rm(distroHome, { force: true, recursive: true }) await rm(workspaceRoot, { force: true, recursive: true }) diff --git a/src/main/worktree-retirement-discovery.test.ts b/src/main/worktree-retirement-discovery.test.ts index caea3a3b98a..5613891bf1e 100644 --- a/src/main/worktree-retirement-discovery.test.ts +++ b/src/main/worktree-retirement-discovery.test.ts @@ -79,7 +79,7 @@ describe('discoverRetiredWorktreeNames', () => { home, env: {} }) - expect(retired).toEqual(new Set([FIRST])) + expect(retired.names).toEqual(new Set([FIRST])) }) }) @@ -91,7 +91,7 @@ describe('discoverRetiredWorktreeNames', () => { home, env: {} }) - expect(retired).toEqual(new Set([FIRST])) + expect(retired.names).toEqual(new Set([FIRST])) }) }) @@ -102,7 +102,7 @@ describe('discoverRetiredWorktreeNames', () => { home, env: {} }) - expect(retired).toEqual(new Set([FIRST])) + expect(retired.names).toEqual(new Set([FIRST])) }) }) @@ -115,7 +115,7 @@ describe('discoverRetiredWorktreeNames', () => { home, env: {} }) - expect(retired).toEqual(new Set([FIRST])) + expect(retired.names).toEqual(new Set([FIRST])) }) }) @@ -129,7 +129,7 @@ describe('discoverRetiredWorktreeNames', () => { // to `wsl.exe`, so on a Windows runner this unit test would boot the developer's distro. resolveWslHome: async () => null }) - expect(retired).toEqual(new Set([FIRST])) + expect(retired.names).toEqual(new Set([FIRST])) }) }) @@ -142,7 +142,7 @@ describe('discoverRetiredWorktreeNames', () => { home, env: {} }) - expect(retired).toEqual(new Set([SECOND])) + expect(retired.names).toEqual(new Set([SECOND])) } ) }) @@ -158,7 +158,7 @@ describe('discoverRetiredWorktreeNames', () => { env: { CLAUDE_CONFIG_DIR: configDir } }) // The override relocates the whole state root, so the default home is not also scanned. - expect(retired).toEqual(new Set([FIRST])) + expect(retired.names).toEqual(new Set([FIRST])) } finally { await rm(configDir, { force: true, recursive: true }) } @@ -181,7 +181,7 @@ describe('discoverRetiredWorktreeNames', () => { env: {}, resolveWslHome: async (distro) => (distro === 'Ubuntu' ? distroHome : null) }) - expect(retired).toEqual(new Set([FIRST])) + expect(retired.names).toEqual(new Set([FIRST])) } finally { await rm(distroHome, { force: true, recursive: true }) } @@ -199,7 +199,7 @@ describe('discoverRetiredWorktreeNames', () => { env: {}, resolveWslHome: async () => null }) - expect(retired).toEqual(new Set([FIRST])) + expect(retired.names).toEqual(new Set([FIRST])) } ) }) @@ -214,7 +214,7 @@ describe('discoverRetiredWorktreeNames', () => { home, env: {} }) - expect(retired).toEqual(new Set([SECOND])) + expect(retired.names).toEqual(new Set([SECOND])) } finally { await rm(root, { force: true, recursive: true }) } diff --git a/src/main/worktree-retirement-discovery.ts b/src/main/worktree-retirement-discovery.ts index be5f863f4e3..fedeb68a823 100644 --- a/src/main/worktree-retirement-discovery.ts +++ b/src/main/worktree-retirement-discovery.ts @@ -6,7 +6,7 @@ import { isClaudeProjectDirInScope } from './ai-vault/claude-project-dir-encoding' import { wslGatedReaddir } from './native-chat/wsl-transcript-fs-access' -import { WslTranscriptFsError } from './native-chat/wsl-transcript-fs-gate' +import type { RetirementScanResult } from './worktree-retirement-backfill-scan' import { normalizeRetirableGeneratedName } from './worktree-name-retirement' import { getWslHomeAsync } from './wsl' @@ -50,18 +50,26 @@ export function extractBucketLeafCandidates( } /** WSL UNC listings go through the shared 9P gate, which bounds concurrency and deadlines each - * call; off UNC it is a plain `readdir`. A gate refusal propagates on purpose — swallowing it - * would memoize a partial answer forever, which under-retires, the one direction that leaks. */ -async function readDirectoryNames(path: string): Promise { + * call; off UNC it is a plain `readdir`. + * + * A gate refusal is reported rather than thrown. Throwing abandoned the whole scan at whichever + * source failed first — and for a WSL repo the very first listing is the UNC workspace root, so a + * stuck 9P route also lost the plain, readable Windows-side `~/.claude/projects` read that the + * caller could always perform. The names that were found are still worth returning; what must not + * happen is memoizing the hole, since under-retiring is the direction that leaks. */ +async function readDirectoryNames(path: string): Promise<{ names: string[]; refused: boolean }> { try { const entries = await wslGatedReaddir(path, 'scan') - return entries.filter((entry) => entry.isDirectory()).map((entry) => entry.name) - } catch (error) { - if (error instanceof WslTranscriptFsError) { - throw error + return { + names: entries.filter((entry) => entry.isDirectory()).map((entry) => entry.name), + refused: false } - // Missing or unreadable roots are normal: the agent may never have run on this machine. - return [] + } catch (error) { + // A missing root is itself a complete answer: the agent may never have run on this machine. + // Anything else — a gate refusal, an EIO on a redirected or network home — leaves names + // unread, and memoizing that as "nothing is retired" is the under-retiring direction. + const code = (error as NodeJS.ErrnoException | null)?.code + return { names: [], refused: code !== 'ENOENT' && code !== 'ENOTDIR' } } } @@ -149,17 +157,22 @@ export async function discoverRetiredWorktreeNames(args: { env?: NodeJS.ProcessEnv /** Injected in tests; production resolves the distro's `$HOME` as a UNC path. */ resolveWslHome?: (distro: string) => Promise -}): Promise> { +}): Promise { const leafNames: string[] = [] + let refused = false for (const root of args.workspaceRoots) { - leafNames.push(...(await readDirectoryNames(root))) + const listing = await readDirectoryNames(root) + refused ||= listing.refused + leafNames.push(...listing.names) } for (const source of await claudeProjectsSources(args)) { - for (const bucket of await readDirectoryNames(source.projectsDir)) { + const listing = await readDirectoryNames(source.projectsDir) + refused ||= listing.refused + for (const bucket of listing.names) { leafNames.push(...extractBucketLeafCandidates(bucket, source.encodedParents)) } } - return collectRetiredNamesFromLeafNames(leafNames) + return { names: collectRetiredNamesFromLeafNames(leafNames), complete: !refused } }