From 5757e52fdeb8a0ad581da7e44fab8e745c005a96 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 2 Sep 2026 01:17:09 -0700 Subject: [PATCH] fix(wsl): name the spawn directory at the six remaining wsl.exe sites The first commit fixed the WSL command builders. Six spawn sites were left inheriting the process cwd, which is the same deletable `\\wsl.localhost` worktree: `wsl-availability` (both probes), the WSL filesystem watcher, the agent-hook relay launch, the UNC delete, and the local worktree filesystem. `wsl-availability` is the one that matters most, and it turns the bug into a latching false negative. `isRetryableWslProbeFailure` returns false for ENOENT, so a spawn that failed only because the inherited cwd was gone is cached as "WSL is not installed" on the 10-minute definitive TTL with exponential backoff up to 30 minutes. Git keeps working and Orca reports WSL unavailable -- worse than the bug being fixed. ENOENT stays non-retryable. It is answer-shaped for the reason it is meant to be -- wsl.exe is not on PATH -- and naming the directory is what removes the one cause that was not. Making it retryable would instead re-probe every non-WSL Windows machine on the short window, and would leave the false ENOENT in place for the other five sites, which have no cache to correct. Three of these are also on the `runWslProcess` W3 migration allowlist; this is the interim until they move, and matches what #17837 does inside the runner. --- .../agent-hooks/wsl-hook-relay-launch.test.ts | 25 ++++++++++++++ src/main/agent-hooks/wsl-hook-relay-launch.ts | 7 +++- src/main/ipc/filesystem-watcher-wsl.test.ts | 6 +++- src/main/ipc/filesystem-watcher-wsl.ts | 7 +++- src/main/local-worktree-filesystem.test.ts | 6 +++- src/main/local-worktree-filesystem.ts | 5 +++ src/main/wsl-availability.ts | 18 ++++++++-- src/main/wsl-unc-delete.test.ts | 5 ++- src/main/wsl-unc-delete.ts | 5 ++- src/main/wsl.test.ts | 34 +++++++++++++++++++ 10 files changed, 110 insertions(+), 8 deletions(-) create mode 100644 src/main/agent-hooks/wsl-hook-relay-launch.test.ts diff --git a/src/main/agent-hooks/wsl-hook-relay-launch.test.ts b/src/main/agent-hooks/wsl-hook-relay-launch.test.ts new file mode 100644 index 00000000000..6ed9cf2625a --- /dev/null +++ b/src/main/agent-hooks/wsl-hook-relay-launch.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it, vi } from 'vitest' + +const { spawnMock } = vi.hoisted(() => ({ + spawnMock: vi.fn((..._args: unknown[]) => ({ pid: 1 })) +})) + +vi.mock('node:child_process', () => ({ spawn: spawnMock })) + +import { spawnWslRelayProcess } from './wsl-hook-relay-launch' + +describe('spawnWslRelayProcess', () => { + it('names an explicit Windows directory rather than inheriting one', () => { + spawnWslRelayProcess('Ubuntu', {}, '1.2.3') + + // Why (#16463): the guest path is inside the `sh -c` command, so the Windows + // cwd only decides whether CreateProcessW succeeds. Omitting it inherits + // Orca's own — a `\\wsl.localhost` worktree the user can delete, after which + // every relay launch fails `spawn wsl.exe ENOENT` for the rest of the session. + expect(spawnMock).toHaveBeenCalledWith( + 'wsl.exe', + expect.arrayContaining(['-d', 'Ubuntu', '--exec']), + expect.objectContaining({ cwd: expect.any(String) }) + ) + }) +}) diff --git a/src/main/agent-hooks/wsl-hook-relay-launch.ts b/src/main/agent-hooks/wsl-hook-relay-launch.ts index 9f32de10bd5..ae1ea9c20d7 100644 --- a/src/main/agent-hooks/wsl-hook-relay-launch.ts +++ b/src/main/agent-hooks/wsl-hook-relay-launch.ts @@ -16,6 +16,7 @@ import { } from './wsl-hook-relay-sentinel' import { addOrcaWslInteropEnv } from '../pty/wsl-orca-env' import { runWslProcess } from '../wsl/wsl-runner' +import { resolveWslInteropSpawnCwd } from '../wsl-interop-spawn-directory' import { listRunningWslDistrosAsync } from '../wsl' import { WSL_HOOK_RELAY_BUNDLE_NAME, @@ -137,7 +138,11 @@ export function spawnWslRelayProcess( return spawn('wsl.exe', ['-d', distro, '--exec', 'sh', '-c', command], { env, stdio: ['pipe', 'pipe', 'pipe'], - windowsHide: true + windowsHide: true, + // Why explicit (#16463): the guest path is in `command`, so the Windows cwd + // only decides whether CreateProcessW succeeds -- and an inherited one is a + // worktree the user can delete, which kills every later relay launch. + cwd: resolveWslInteropSpawnCwd() }) } diff --git a/src/main/ipc/filesystem-watcher-wsl.test.ts b/src/main/ipc/filesystem-watcher-wsl.test.ts index 13346047d86..84d8d0a2306 100644 --- a/src/main/ipc/filesystem-watcher-wsl.test.ts +++ b/src/main/ipc/filesystem-watcher-wsl.test.ts @@ -95,7 +95,11 @@ describe('createWslWatcher', () => { ['-d', 'Ubuntu', '--exec', 'sh', '-s', '--', '/home/me/repo'], expect.objectContaining({ stdio: ['pipe', 'pipe', 'pipe'], - windowsHide: true + windowsHide: true, + // Why a concrete directory (#16463): the watched path rides in argv, so + // an omitted cwd only means CreateProcessW inherits Orca's -- a worktree + // that can be deleted, after which every watcher start is ENOENT. + cwd: expect.any(String) }) ) }) diff --git a/src/main/ipc/filesystem-watcher-wsl.ts b/src/main/ipc/filesystem-watcher-wsl.ts index 86f10c2ee68..a444113c1cf 100644 --- a/src/main/ipc/filesystem-watcher-wsl.ts +++ b/src/main/ipc/filesystem-watcher-wsl.ts @@ -14,6 +14,7 @@ import { parseWslUncPath } from '../../shared/wsl-paths' import { createWslWatcherProcessExit, createWslWatcherStartup } from './wsl-watcher-process-exit' import { reserveWatcherChild, WatcherChildCapacityError } from './parcel-watcher-child-registry' import { createDebouncedBatch, type DebouncedBatch } from './filesystem-watcher-batch-control' +import { resolveWslInteropSpawnCwd } from '../wsl-interop-spawn-directory' export type WatcherSubscription = { unsubscribe(): Promise @@ -244,7 +245,11 @@ export async function createWslWatcher( try { child = spawn('wsl.exe', ['-d', distro, '--exec', 'sh', '-s', '--', linuxPath], { stdio: ['pipe', 'pipe', 'pipe'], - windowsHide: true + windowsHide: true, + // Why explicit (#16463): the watched directory rides in argv, and an + // inherited cwd is a worktree that can be deleted -- after which every + // watcher start fails `spawn wsl.exe ENOENT`. + cwd: resolveWslInteropSpawnCwd() }) } catch (error) { releaseChildReservation() diff --git a/src/main/local-worktree-filesystem.test.ts b/src/main/local-worktree-filesystem.test.ts index 52b1d16f21a..c9e97f72e90 100644 --- a/src/main/local-worktree-filesystem.test.ts +++ b/src/main/local-worktree-filesystem.test.ts @@ -188,7 +188,11 @@ describe('local worktree filesystem runtime access', () => { 1, expect.objectContaining({ program: 'wsl.exe', - args: expect.arrayContaining(['-d', 'Ubuntu']) + args: expect.arrayContaining(['-d', 'Ubuntu']), + // Why a concrete directory (#16463): the guest path is inside the + // command, and these run while a worktree is being removed -- which is + // the cwd an omitted one would inherit. + cwd: expect.any(String) }) ) const removeArgs = runProcessMock.mock.calls[2]?.[0].args as string[] diff --git a/src/main/local-worktree-filesystem.ts b/src/main/local-worktree-filesystem.ts index f5d8714e196..1078b1f50bc 100644 --- a/src/main/local-worktree-filesystem.ts +++ b/src/main/local-worktree-filesystem.ts @@ -3,6 +3,7 @@ import { lstat, readFile } from 'node:fs/promises' import { buildWslExecArgs, quotePosixShell } from '../shared/wsl-login-shell-command' import { removeHostTree } from './host-tree-removal' import { toLinuxPath } from './wsl' +import { resolveWslInteropSpawnCwd } from './wsl-interop-spawn-directory' import type { ReadPath, StatPath } from './worktree-orphan-gitdir-proof' export { toHostFilesystemPath, toHostRemovalPath } from './host-tree-removal' @@ -36,6 +37,10 @@ async function runWslCommand(distro: string, command: string): Promise { const result = await runProcess({ program: 'wsl.exe', args: buildWslExecArgs(distro, ['sh', '-c', command]), + // Why explicit (#16463): the guest path is inside `command`, so this only + // decides whether CreateProcessW succeeds -- and these calls run while a + // worktree is being removed, which is the cwd an inherited one would be. + cwd: resolveWslInteropSpawnCwd(), timeoutMs: WSL_FILE_OPERATION_TIMEOUT_MS }) if (result.timedOut) { diff --git a/src/main/wsl-availability.ts b/src/main/wsl-availability.ts index c44476c9d60..1d14f526413 100644 --- a/src/main/wsl-availability.ts +++ b/src/main/wsl-availability.ts @@ -1,4 +1,5 @@ import { execFile, execFileSync } from 'node:child_process' +import { resolveWslInteropSpawnCwd } from './wsl-interop-spawn-directory' type WslAvailabilityCache = | { available: true } @@ -35,6 +36,9 @@ function wslAvailabilityRetryDelayMs(cache: { retryable: boolean; failures: numb return Math.min(base * 2 ** (cache.failures - 1), WSL_AVAILABILITY_MAX_RETRY_DELAY_MS) } +// Why ENOENT stays definitive: it means wsl.exe is not on PATH. It used to also mean +// "the cwd this process inherited was deleted", which is not answer-shaped at all -- +// naming an explicit spawn directory below is what removes that source (#16463). // Why: a non-zero exit (wsl.exe ran and said no) or ENOENT (not installed) is answer-shaped, // so it earns a long window rather than the short one a timeout gets. execFileSync reports the // exit code as `status`, the execFile callback as a numeric `code`; both must count as @@ -95,7 +99,14 @@ function probeWslStatus(): Promise { execFile( 'wsl.exe', ['--status'], - { timeout: WSL_AVAILABILITY_PROBE_TIMEOUT_MS, windowsHide: true }, + { + timeout: WSL_AVAILABILITY_PROBE_TIMEOUT_MS, + windowsHide: true, + // Why explicit (#16463): inheriting a cwd the user deleted makes + // CreateProcessW fail ENOENT, which this cache reads as "WSL is not + // installed" and holds on the definitive TTL with backoff. + cwd: resolveWslInteropSpawnCwd() + }, (error: unknown) => { if (error) { reject(error) @@ -129,7 +140,10 @@ export function isWslAvailable(): boolean { try { execFileSync('wsl.exe', ['--status'], { stdio: ['pipe', 'pipe', 'pipe'], - timeout: WSL_AVAILABILITY_PROBE_TIMEOUT_MS + timeout: WSL_AVAILABILITY_PROBE_TIMEOUT_MS, + // Same reason as the async twin: they share one cache, so a false ENOENT + // from either poisons both. + cwd: resolveWslInteropSpawnCwd() }) return cacheWslAvailabilityProbeResult(null, startedAtGeneration) } catch (error) { diff --git a/src/main/wsl-unc-delete.test.ts b/src/main/wsl-unc-delete.test.ts index 4ca6c43312c..a902b4b6d53 100644 --- a/src/main/wsl-unc-delete.test.ts +++ b/src/main/wsl-unc-delete.test.ts @@ -50,8 +50,11 @@ describe('tryDeleteWslUncPath', () => { }) expect(execFileMock).toHaveBeenCalledTimes(1) - const [binary, spawnArgs] = execFileMock.mock.calls[0] + const [binary, spawnArgs, spawnOptions] = execFileMock.mock.calls[0] expect(binary).toBe('wsl.exe') + // Why a concrete directory (#16463): this deletes worktrees, so the cwd it + // would otherwise inherit is the very directory about to disappear. + expect(spawnOptions).toEqual(expect.objectContaining({ cwd: expect.any(String) })) expect(spawnArgs).toEqual([ '-d', 'Ubuntu', diff --git a/src/main/wsl-unc-delete.ts b/src/main/wsl-unc-delete.ts index 9cc62c9b236..b094a152beb 100644 --- a/src/main/wsl-unc-delete.ts +++ b/src/main/wsl-unc-delete.ts @@ -1,5 +1,6 @@ import { execFile } from 'node:child_process' import { parseWslPath } from './wsl' +import { resolveWslInteropSpawnCwd } from './wsl-interop-spawn-directory' import { containedDeleteCommand, rejectionFromWslDeleteStderr, @@ -69,7 +70,9 @@ function execFileWsl(distro: string, command: string[]): Promise { ['-d', distro, '--exec', ...command], // Why: a generous bound so deleting a large directory tree on the WSL fs // doesn't abort mid-delete, while still capping a wedged wsl.exe. - { encoding: 'utf-8', timeout: 30000 }, + // Why an explicit cwd (#16463): the target rides in argv, and this deletes + // worktrees -- so an inherited cwd is exactly the directory about to go. + { encoding: 'utf-8', timeout: 30000, cwd: resolveWslInteropSpawnCwd() }, (error, _stdout, stderr) => { if (error) { reject(wslDeleteError(error, stderr)) diff --git a/src/main/wsl.test.ts b/src/main/wsl.test.ts index bc3498b1145..6ef8adbbb61 100644 --- a/src/main/wsl.test.ts +++ b/src/main/wsl.test.ts @@ -392,6 +392,40 @@ describe('WSL availability cache', () => { }) }) + // Why this site matters more than the other wsl.exe spawns (#16463): ENOENT is + // deliberately non-retryable here, so a spawn that failed only because the + // inherited cwd had been deleted was cached as "WSL is not installed" on the + // 10-minute definitive TTL with exponential backoff. Git kept working and Orca + // reported WSL unavailable -- a worse state than the bug being fixed. Naming + // the directory is what keeps ENOENT meaning "wsl.exe is not on PATH". + it('names an explicit spawn directory on both probes, so no deleted cwd can read as ENOENT', async () => { + execFileSyncMock.mockReturnValueOnce('') + execFileMock.mockImplementation((_command, _args, _options, callback) => { + callback(null, '', '') + }) + + withPlatform('win32', () => { + expect(isWslAvailable()).toBe(true) + }) + expect(execFileSyncMock).toHaveBeenCalledWith( + 'wsl.exe', + ['--status'], + expect.objectContaining({ cwd: expect.any(String) }) + ) + + // The two probes share one cache, so a false ENOENT from either poisons both. + _resetWslCachesForTests() + await withPlatformAsync('win32', async () => { + await expect(isWslAvailableAsync()).resolves.toBe(true) + }) + expect(execFileMock).toHaveBeenCalledWith( + 'wsl.exe', + ['--status'], + expect.objectContaining({ cwd: expect.any(String) }), + expect.any(Function) + ) + }) + it('shares one wsl.exe spawn between concurrent async probes', async () => { execFileMock.mockImplementation((_command, _args, _options, callback) => { setTimeout(() => callback(null, '', ''), 0)