From d6e289be0ae6bed2b6ae414bce8f53e923619b95 Mon Sep 17 00:00:00 2001 From: OrcaWin <293788423+OrcaWin@users.noreply.github.com> Date: Sun, 9 Aug 2026 23:33:16 -0700 Subject: [PATCH] fix(git): retry transient WSL route probes safely --- .../runner-wsl-linked-gitdir-timeout.test.ts | 98 +++++++++++ .../wsl-linked-worktree-git-routing.test.ts | 158 +++++++++++++++++- .../git/wsl-linked-worktree-git-routing.ts | 104 +++++++++++- 3 files changed, 344 insertions(+), 16 deletions(-) create mode 100644 src/main/git/runner-wsl-linked-gitdir-timeout.test.ts diff --git a/src/main/git/runner-wsl-linked-gitdir-timeout.test.ts b/src/main/git/runner-wsl-linked-gitdir-timeout.test.ts new file mode 100644 index 00000000000..efc37c192f7 --- /dev/null +++ b/src/main/git/runner-wsl-linked-gitdir-timeout.test.ts @@ -0,0 +1,98 @@ +import { EventEmitter } from 'node:events' +import type * as FsPromises from 'node:fs/promises' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const { execFileMock, statMock, readFileMock } = vi.hoisted(() => ({ + execFileMock: vi.fn(), + statMock: vi.fn(), + readFileMock: vi.fn() +})) + +vi.mock('node:child_process', () => ({ + execFile: execFileMock, + execFileSync: vi.fn(), + spawn: vi.fn() +})) + +vi.mock('node:fs/promises', async (importOriginal) => ({ + ...(await importOriginal()), + stat: statMock, + readFile: readFileMock +})) + +import { gitExecFileAsync } from './runner' +import { + resetWslLinkedWorktreeGitRoutingForTests, + WSL_LINKED_WORKTREE_ROUTE_PROBE_TIMEOUT_MS +} from './wsl-linked-worktree-git-routing' + +function createMockChild(): EventEmitter & { pid: number; kill: ReturnType } { + const child = new EventEmitter() as EventEmitter & { + pid: number + kill: ReturnType + } + child.pid = 1234 + child.kill = vi.fn() + return child +} + +async function withWindowsPlatform(run: () => Promise): Promise { + const original = process.platform + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + try { + await run() + } finally { + Object.defineProperty(process, 'platform', { configurable: true, value: original }) + } +} + +afterEach(() => { + vi.useRealTimers() + vi.restoreAllMocks() + resetWslLinkedWorktreeGitRoutingForTests() +}) + +describe('WSL linked-worktree routing probe timeout', () => { + it('retries discovery on the next production Git call', async () => { + vi.useFakeTimers() + await withWindowsPlatform(async () => { + statMock + .mockImplementationOnce(() => new Promise(() => {})) + .mockResolvedValue({ isDirectory: () => false, isFile: () => true }) + readFileMock.mockResolvedValue('gitdir: C:/main/.git/worktrees/linked\n') + execFileMock.mockImplementation((command, _args, _options, callback) => { + const child = createMockChild() + queueMicrotask(() => { + if (command === 'wsl.exe') { + callback?.( + Object.assign(new Error('not a git repository'), { code: 128 }), + '', + 'fatal: not a git repository' + ) + } else { + callback?.(null, 'host git recovered\n', '') + } + }) + return child + }) + + const first = gitExecFileAsync(['status', '--short'], { + cwd: String.raw`C:\repo`, + wslDistro: 'Ubuntu' + }) + const firstFailure = expect(first).rejects.toThrow('not a git repository') + await vi.advanceTimersByTimeAsync(WSL_LINKED_WORKTREE_ROUTE_PROBE_TIMEOUT_MS) + await firstFailure + + await expect( + gitExecFileAsync(['status', '--short'], { + cwd: String.raw`C:\repo`, + wslDistro: 'Ubuntu' + }) + ).resolves.toEqual({ stdout: 'host git recovered\n', stderr: '' }) + + expect(statMock).toHaveBeenCalledTimes(2) + expect(execFileMock.mock.calls.map(([command]) => command)).toEqual(['wsl.exe', 'git']) + }) + }) +}) diff --git a/src/main/git/wsl-linked-worktree-git-routing.test.ts b/src/main/git/wsl-linked-worktree-git-routing.test.ts index 25cf73d9521..9ab07309b1f 100644 --- a/src/main/git/wsl-linked-worktree-git-routing.test.ts +++ b/src/main/git/wsl-linked-worktree-git-routing.test.ts @@ -6,7 +6,10 @@ import { seedWslLinkedWorktreeGitRoutingForTests, usesHostGitForWslLinkedWorktree, WSL_LINKED_WORKTREE_ROUTE_CACHE_PRUNE_THRESHOLD, + WSL_LINKED_WORKTREE_ROUTE_MAX_PROBES_PER_CWD, + WSL_LINKED_WORKTREE_ROUTE_MAX_PROBES_TOTAL, WSL_LINKED_WORKTREE_ROUTE_PROBE_TIMEOUT_MS, + WSL_LINKED_WORKTREE_ROUTE_RETRY_BASE_MS, WSL_LINKED_WORKTREE_ROUTE_TTL_MS, type WslLinkedWorktreeRoutingFileSystem } from './wsl-linked-worktree-git-routing' @@ -120,12 +123,13 @@ describe('prepareWslLinkedWorktreeGitRouting', () => { expect(fileSystem.readFile).not.toHaveBeenCalled() }) - it('fails closed when a marker cannot be read', async () => { + it('fails closed without caching a marker read error', async () => { const fileSystem: WslLinkedWorktreeRoutingFileSystem = { stat: vi.fn(async () => fileMarker), - readFile: vi.fn(async () => { - throw Object.assign(new Error('access denied'), { code: 'EACCES' }) - }) + readFile: vi + .fn() + .mockRejectedValueOnce(Object.assign(new Error('access denied'), { code: 'EACCES' })) + .mockResolvedValueOnce('gitdir: C:/main/.git/worktrees/linked\n') } await expect( @@ -134,6 +138,42 @@ describe('prepareWslLinkedWorktreeGitRouting', () => { fileSystem }) ).resolves.toBe(false) + await expect( + prepareWslLinkedWorktreeGitRouting(String.raw`C:\repo`, 'Ubuntu', { + platform: 'win32', + fileSystem + }) + ).resolves.toBe(true) + expect(fileSystem.stat).toHaveBeenCalledTimes(2) + expect(fileSystem.readFile).toHaveBeenCalledTimes(2) + }) + + it('backs off repeated marker stat errors after one immediate retry', async () => { + let currentTime = 1_000 + const now = (): number => currentTime + const fileSystem: WslLinkedWorktreeRoutingFileSystem = { + stat: vi.fn(async () => { + throw Object.assign(new Error('device unavailable'), { code: 'EIO' }) + }), + readFile: vi.fn(async () => '') + } + const prepare = (): Promise => + prepareWslLinkedWorktreeGitRouting(String.raw`C:\repo`, 'Ubuntu', { + platform: 'win32', + fileSystem, + now + }) + + await expect(prepare()).resolves.toBe(false) + await expect(prepare()).resolves.toBe(false) + await expect(prepare()).resolves.toBe(false) + currentTime += WSL_LINKED_WORKTREE_ROUTE_RETRY_BASE_MS - 1 + await expect(prepare()).resolves.toBe(false) + expect(fileSystem.stat).toHaveBeenCalledTimes(2) + + currentTime += 1 + await expect(prepare()).resolves.toBe(false) + expect(fileSystem.stat).toHaveBeenCalledTimes(3) }) it('does not inherit a linked-parent route across a nested repository boundary', async () => { @@ -380,11 +420,56 @@ describe('prepareWslLinkedWorktreeGitRouting', () => { expect(fileSystem.stat).toHaveBeenCalledTimes(1) }) - it('fails closed and clears coalescing after a stalled discovery deadline', async () => { + it('ignores an old probe that settles after reset and a new same-path probe starts', async () => { + let releaseOld: ((marker: typeof fileMarker) => void) | undefined + const oldMarker = new Promise((resolve) => { + releaseOld = resolve + }) + const oldFileSystem: WslLinkedWorktreeRoutingFileSystem = { + stat: vi.fn(() => oldMarker), + readFile: vi.fn(async () => 'gitdir: C:/main/.git/worktrees/linked\n') + } + const oldRoute = prepareWslLinkedWorktreeGitRouting(String.raw`C:\repo`, 'Ubuntu', { + platform: 'win32', + fileSystem: oldFileSystem + }) + + resetWslLinkedWorktreeGitRoutingForTests() + let releaseCurrent: ((marker: typeof directoryMarker) => void) | undefined + const currentMarker = new Promise((resolve) => { + releaseCurrent = resolve + }) + const currentFileSystem: WslLinkedWorktreeRoutingFileSystem = { + stat: vi.fn(() => currentMarker), + readFile: vi.fn(async () => '') + } + const currentRoute = prepareWslLinkedWorktreeGitRouting(String.raw`C:\repo`, 'Ubuntu', { + platform: 'win32', + fileSystem: currentFileSystem + }) + + releaseOld?.(fileMarker) + await expect(oldRoute).resolves.toBe(false) + expect(usesHostGitForWslLinkedWorktree(String.raw`C:\repo`, 'Ubuntu', 'win32')).toBe(false) + const currentJoiner = prepareWslLinkedWorktreeGitRouting(String.raw`C:\repo`, 'Ubuntu', { + platform: 'win32', + fileSystem: currentFileSystem + }) + expect(currentFileSystem.stat).toHaveBeenCalledTimes(1) + + releaseCurrent?.(directoryMarker) + await expect(Promise.all([currentRoute, currentJoiner])).resolves.toEqual([false, false]) + expect(currentFileSystem.stat).toHaveBeenCalledTimes(1) + }) + + it('fails closed and retries after a stalled discovery deadline', async () => { vi.useFakeTimers() try { const fileSystem: WslLinkedWorktreeRoutingFileSystem = { - stat: vi.fn(() => new Promise(() => {})), + stat: vi + .fn() + .mockImplementationOnce(() => new Promise(() => {})) + .mockResolvedValueOnce(fileMarker), readFile: vi.fn(async () => 'gitdir: C:/main/.git/worktrees/linked\n') } @@ -405,8 +490,67 @@ describe('prepareWslLinkedWorktreeGitRouting', () => { platform: 'win32', fileSystem }) + ).resolves.toBe(true) + expect(fileSystem.stat).toHaveBeenCalledTimes(2) + expect(fileSystem.readFile).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) + + it('caps outstanding probes for one working directory after repeated deadlines', async () => { + vi.useFakeTimers() + try { + const fileSystem: WslLinkedWorktreeRoutingFileSystem = { + stat: vi.fn(() => new Promise(() => {})), + readFile: vi.fn(async () => '') + } + const prepare = (): Promise => + prepareWslLinkedWorktreeGitRouting(String.raw`C:\repo`, 'Ubuntu', { + platform: 'win32', + fileSystem + }) + + for (let index = 0; index < WSL_LINKED_WORKTREE_ROUTE_MAX_PROBES_PER_CWD; index += 1) { + const route = prepare() + await vi.advanceTimersByTimeAsync(WSL_LINKED_WORKTREE_ROUTE_PROBE_TIMEOUT_MS) + await expect(route).resolves.toBe(false) + } + await vi.advanceTimersByTimeAsync(WSL_LINKED_WORKTREE_ROUTE_RETRY_BASE_MS) + await expect(prepare()).resolves.toBe(false) + expect(fileSystem.stat).toHaveBeenCalledTimes(WSL_LINKED_WORKTREE_ROUTE_MAX_PROBES_PER_CWD) + } finally { + vi.useRealTimers() + } + }) + + it('caps outstanding probes across working directories', async () => { + vi.useFakeTimers() + try { + const fileSystem: WslLinkedWorktreeRoutingFileSystem = { + stat: vi.fn(() => new Promise(() => {})), + readFile: vi.fn(async () => '') + } + const pending = Array.from( + { length: WSL_LINKED_WORKTREE_ROUTE_MAX_PROBES_TOTAL }, + (_, index) => + prepareWslLinkedWorktreeGitRouting(`C:\\repo-${index}`, 'Ubuntu', { + platform: 'win32', + fileSystem + }) + ) + + await expect( + prepareWslLinkedWorktreeGitRouting(String.raw`C:\blocked`, 'Ubuntu', { + platform: 'win32', + fileSystem + }) ).resolves.toBe(false) - expect(fileSystem.stat).toHaveBeenCalledTimes(1) + expect(fileSystem.stat).toHaveBeenCalledTimes(WSL_LINKED_WORKTREE_ROUTE_MAX_PROBES_TOTAL) + await vi.advanceTimersByTimeAsync(WSL_LINKED_WORKTREE_ROUTE_PROBE_TIMEOUT_MS) + await expect(Promise.all(pending)).resolves.toEqual( + Array.from({ length: WSL_LINKED_WORKTREE_ROUTE_MAX_PROBES_TOTAL }, () => false) + ) } finally { vi.useRealTimers() } diff --git a/src/main/git/wsl-linked-worktree-git-routing.ts b/src/main/git/wsl-linked-worktree-git-routing.ts index 23b22b7f370..440307fd5f0 100644 --- a/src/main/git/wsl-linked-worktree-git-routing.ts +++ b/src/main/git/wsl-linked-worktree-git-routing.ts @@ -3,14 +3,23 @@ import { win32 } from 'node:path' import type { Stats } from 'node:fs' type CachedRoute = { usesHostGit: boolean; expiresAt: number } +type TransientRouteFailure = { count: number; retryAfter: number } // Bounds stale routing after delete/recreate while amortizing async parent walks on slow storage. export const WSL_LINKED_WORKTREE_ROUTE_TTL_MS = 30_000 export const WSL_LINKED_WORKTREE_ROUTE_CACHE_PRUNE_THRESHOLD = 512 export const WSL_LINKED_WORKTREE_ROUTE_PROBE_TIMEOUT_MS = 5_000 +export const WSL_LINKED_WORKTREE_ROUTE_MAX_PROBES_PER_CWD = 2 +export const WSL_LINKED_WORKTREE_ROUTE_MAX_PROBES_TOTAL = 32 +export const WSL_LINKED_WORKTREE_ROUTE_RETRY_BASE_MS = 1_000 +const WSL_LINKED_WORKTREE_ROUTE_RETRY_MAX_MS = 30_000 const routeByCwd = new Map() const pendingRoutes = new Map>() +const transientFailuresByCwd = new Map() +const activeProbeCountByCwd = new Map() +let activeProbeCount = 0 +let routeProbeGeneration = 0 let nextRoutePruneAt = 0 export function isWslLinkedWorktreeGitRoutingCandidate( @@ -79,7 +88,7 @@ async function shouldUseHostGit( } catch (error) { const code = error && typeof error === 'object' ? (error as NodeJS.ErrnoException).code : null if (code !== 'ENOENT' && code !== 'ENOTDIR') { - return false + throw error } } if (candidate === driveRoot) { @@ -110,6 +119,55 @@ function routingAbortError(): Error { return Object.assign(new Error('The operation was aborted.'), { name: 'AbortError' }) } +function rememberTransientFailure(cwd: string, now: number): void { + const count = (transientFailuresByCwd.get(cwd)?.count ?? 0) + 1 + const retryDelay = + count === 1 + ? 0 + : Math.min( + WSL_LINKED_WORKTREE_ROUTE_RETRY_BASE_MS * 2 ** Math.min(count - 2, 10), + WSL_LINKED_WORKTREE_ROUTE_RETRY_MAX_MS + ) + if ( + !transientFailuresByCwd.has(cwd) && + transientFailuresByCwd.size >= WSL_LINKED_WORKTREE_ROUTE_CACHE_PRUNE_THRESHOLD + ) { + const oldest = transientFailuresByCwd.keys().next().value + if (oldest !== undefined) { + transientFailuresByCwd.delete(oldest) + } + } + transientFailuresByCwd.delete(cwd) + transientFailuresByCwd.set(cwd, { count, retryAfter: now + retryDelay }) +} + +function canStartRouteProbe(cwd: string, now: number): boolean { + const failure = transientFailuresByCwd.get(cwd) + return Boolean( + (!failure || failure.retryAfter <= now) && + (activeProbeCountByCwd.get(cwd) ?? 0) < WSL_LINKED_WORKTREE_ROUTE_MAX_PROBES_PER_CWD && + activeProbeCount < WSL_LINKED_WORKTREE_ROUTE_MAX_PROBES_TOTAL + ) +} + +function trackRouteProbe(cwd: string): () => void { + const generation = routeProbeGeneration + activeProbeCount += 1 + activeProbeCountByCwd.set(cwd, (activeProbeCountByCwd.get(cwd) ?? 0) + 1) + return () => { + if (generation !== routeProbeGeneration) { + return + } + activeProbeCount -= 1 + const remaining = (activeProbeCountByCwd.get(cwd) ?? 1) - 1 + if (remaining === 0) { + activeProbeCountByCwd.delete(cwd) + } else { + activeProbeCountByCwd.set(cwd, remaining) + } + } +} + function waitForRouteUnlessAborted( route: Promise, signal?: AbortSignal @@ -145,20 +203,38 @@ function discoverRoute( now: () => number ): Promise { return new Promise((resolve) => { + const generation = routeProbeGeneration let settled = false - const finish = (usesHostGit: boolean): void => { + const finish = (usesHostGit: boolean, cacheable: boolean): void => { if (settled) { return } settled = true clearTimeout(timer) - resolve(rememberRoute(cwd, usesHostGit, now())) + if (generation !== routeProbeGeneration) { + resolve(false) + return + } + if (cacheable) { + transientFailuresByCwd.delete(cwd) + resolve(rememberRoute(cwd, usesHostGit, now())) + } else { + rememberTransientFailure(cwd, now()) + resolve(usesHostGit) + } } - const timer = setTimeout(() => finish(false), WSL_LINKED_WORKTREE_ROUTE_PROBE_TIMEOUT_MS) + const timer = setTimeout(() => finish(false, false), WSL_LINKED_WORKTREE_ROUTE_PROBE_TIMEOUT_MS) timer.unref() + const releaseProbe = trackRouteProbe(cwd) void shouldUseHostGit(cwd, fileSystem).then( - (usesHostGit) => finish(usesHostGit), - () => finish(false) + (usesHostGit) => { + releaseProbe() + finish(usesHostGit, true) + }, + () => { + releaseProbe() + finish(false, false) + } ) }) } @@ -186,9 +262,15 @@ export async function prepareWslLinkedWorktreeGitRouting( if (pending) { return waitForRouteUnlessAborted(pending, options.signal) } - const route = discoverRoute(normalizedCwd, fileSystem, now).finally(() => - pendingRoutes.delete(normalizedCwd) - ) + if (!canStartRouteProbe(normalizedCwd, now())) { + return false + } + const discovery = discoverRoute(normalizedCwd, fileSystem, now) + const route = discovery.finally(() => { + if (pendingRoutes.get(normalizedCwd) === route) { + pendingRoutes.delete(normalizedCwd) + } + }) pendingRoutes.set(normalizedCwd, route) return waitForRouteUnlessAborted(route, options.signal) } @@ -208,6 +290,10 @@ export function usesHostGitForWslLinkedWorktree( export function resetWslLinkedWorktreeGitRoutingForTests(): void { routeByCwd.clear() pendingRoutes.clear() + transientFailuresByCwd.clear() + activeProbeCountByCwd.clear() + activeProbeCount = 0 + routeProbeGeneration += 1 nextRoutePruneAt = 0 }