diff --git a/config/scripts/wsl-hook-relay-reattach-benchmark.mjs b/config/scripts/wsl-hook-relay-reattach-benchmark.mjs index 778d9a81f14..037bdea37f5 100644 --- a/config/scripts/wsl-hook-relay-reattach-benchmark.mjs +++ b/config/scripts/wsl-hook-relay-reattach-benchmark.mjs @@ -323,15 +323,15 @@ async function main() { // Why: if jiti ever returns a second manager module, our monkey-patch would miss the singleton // that ensureWslHookRelayForReattach (loaded via pty.ts) closes over — fail closed now. const singletonProbe = [] - const previousEnsure = wslHookRelayManager.ensureForDistro.bind(wslHookRelayManager) - wslHookRelayManager.ensureForDistro = (probedDistro) => { + const previousEnsureReady = wslHookRelayManager.ensureForDistroReady.bind(wslHookRelayManager) + wslHookRelayManager.ensureForDistroReady = async (probedDistro) => { singletonProbe.push(probedDistro) } - ensureWslHookRelayForReattach( + await ensureWslHookRelayForReattach( { isReattach: true, wslDistro: '__bench-singleton-probe__' }, null ) - wslHookRelayManager.ensureForDistro = previousEnsure + wslHookRelayManager.ensureForDistroReady = previousEnsureReady if (singletonProbe.length !== 1 || singletonProbe[0] !== '__bench-singleton-probe__') { throw new Error( 'jiti duplicated the WSL hook-relay manager graph; reattach patch would not observe pty.ts' @@ -360,15 +360,15 @@ async function main() { // Why: pty.ts refreshes through the production singleton, so route that singleton at the // benchmark-scoped manager instead of calling the reattach helper from here — a removed or // mislocated integration call in pty.ts must fail this benchmark. - wslHookRelayManager.ensureForDistro = (refreshedDistro) => { + wslHookRelayManager.ensureForDistroReady = async (refreshedDistro) => { relayRefreshes.push(refreshedDistro) - manager?.ensureForDistro(refreshedDistro) + await manager?.ensureForDistroReady(refreshedDistro) } const { runtime, getController } = createRuntimeStub() // Why hooks off: fresh WSL spawn still runs buildPtyHostEnv, which calls ensureForDistro when // hooks are on. This bench isolates the reattach call site (ensureWslHookRelayForReattach); - // the manager's own hooks gate lives behind the ensureForDistro patch above, so it stays out + // the manager's own hooks gate lives behind the ensureForDistroReady patch above, so it stays out // of the measurement — `manager` below resolves its own (enabled) managed-hook settings. ptyIpc.registerPtyHandlers( createRendererWindowStub(), @@ -474,7 +474,7 @@ async function main() { ], ensureForDistro: async (startupDistro) => { startupDistros.push(startupDistro) - await manager.ensureForDistroReady(startupDistro) + await manager.ensureRunningDistroForStartup(startupDistro) } }) const startupRelayReadyMs = performance.now() - startupStartedAt diff --git a/src/main/agent-hooks/wsl-hook-relay-launch.ts b/src/main/agent-hooks/wsl-hook-relay-launch.ts index 221ea42f080..a40911b16f5 100644 --- a/src/main/agent-hooks/wsl-hook-relay-launch.ts +++ b/src/main/agent-hooks/wsl-hook-relay-launch.ts @@ -249,15 +249,26 @@ export async function launchWslRelayWithInstall(options: { onChild: (child: ChildProcessWithoutNullStreams) => void onNoNode: () => void onFailure: (message: string) => void + continueAfterFailure?: () => Promise + onContinuationBlocked: () => void connect: (transport: MultiplexerTransport, child: ChildProcessWithoutNullStreams) => Promise }): Promise { const { distro, env, bundleJsPath, version, io } = options let installTried = false let transientRetries = 0 + let continuationRequiresRunning = false for (;;) { if (options.isDisposed()) { return } + if ( + continuationRequiresRunning && + !(await canContinueRelayStartup(options.continueAfterFailure)) + ) { + options.onContinuationBlocked() + return + } + continuationRequiresRunning = false const child = io.spawnRelay(distro, env, version) options.onChild(child) try { @@ -284,13 +295,19 @@ export async function launchWslRelayWithInstall(options: { ) { transientRetries++ await new Promise((resolve) => setTimeout(resolve, io.transientRetryDelayMs)) + continuationRequiresRunning = true continue } if (!installTried) { + if (!(await canContinueRelayStartup(options.continueAfterFailure))) { + options.onContinuationBlocked() + return + } installTried = true const script = buildGuestInstallScript(io.readBundle(bundleJsPath), version) const result = await io.runInstall(distro, script, env) if (result.code === 0) { + continuationRequiresRunning = true continue } options.onFailure( @@ -304,6 +321,19 @@ export async function launchWslRelayWithInstall(options: { } } +async function canContinueRelayStartup( + continueAfterFailure: (() => Promise) | undefined +): Promise { + if (!continueAfterFailure) { + return true + } + try { + return await continueAfterFailure() + } catch { + return false + } +} + export function formatWslRelayFailure(failure: WslRelayStartupFailure): string { const detail = failure.stderr.trim() return `startup failed (${failure.kind}, code ${failure.code ?? 'unknown'})${detail ? `: ${detail}` : ''}` diff --git a/src/main/agent-hooks/wsl-hook-relay-lifecycle-state.ts b/src/main/agent-hooks/wsl-hook-relay-lifecycle-state.ts new file mode 100644 index 00000000000..7db24235840 --- /dev/null +++ b/src/main/agent-hooks/wsl-hook-relay-lifecycle-state.ts @@ -0,0 +1,21 @@ +import type { ChildProcessWithoutNullStreams } from 'node:child_process' + +import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' + +export type WslHookRelayLifecycleState = { + /** Original casing for wsl.exe argv and breadcrumbs; map keys are lowercased. */ + distro: string + phase: 'starting' | 'running' | 'failed' + child?: ChildProcessWithoutNullStreams + mux?: SshChannelMultiplexer + guestHome?: string + guestEndpointFilePath?: string + opencodeOverlayDir?: string + failures: number + cooldownUntil: number + connectedAt?: number + restartTimer?: ReturnType + reinstallTimer?: ReturnType + lastInstallAt?: number + firstAttempt?: Promise +} diff --git a/src/main/agent-hooks/wsl-hook-relay-manager.test.ts b/src/main/agent-hooks/wsl-hook-relay-manager.test.ts index 99bd16abacf..0775b254cc3 100644 --- a/src/main/agent-hooks/wsl-hook-relay-manager.test.ts +++ b/src/main/agent-hooks/wsl-hook-relay-manager.test.ts @@ -275,7 +275,7 @@ describe('WslHookRelayManager', () => { const { manager, deps } = createManager({ installHooks }) let ready = false - const firstAttempt = manager.ensureForDistroReady('Ubuntu').then(() => { + const firstAttempt = manager.ensureRunningDistroForStartup('Ubuntu').then(() => { ready = true }) await vi.waitFor(() => expect(deps.installHooks).toHaveBeenCalledTimes(1)) @@ -285,6 +285,90 @@ describe('WslHookRelayManager', () => { await firstAttempt expect(manager.getGuestEndpointFilePath('Ubuntu')).not.toBeNull() expect(ready).toBe(true) + expect(deps.isDistroRunning).not.toHaveBeenCalled() + manager.disposeAll() + }) + + it('awaits an equivalent first attempt already started by a PTY path', async () => { + let releaseInstall: (() => void) | undefined + const installHooks = vi.fn( + () => new Promise((resolve) => (releaseInstall = () => resolve([]))) + ) + const { manager, deps } = createManager({ installHooks }) + + manager.ensureForDistro('Ubuntu') + await vi.waitFor(() => expect(deps.installHooks).toHaveBeenCalledTimes(1)) + let ready = false + const startupAttempt = manager.ensureForDistroReady('ubuntu').then(() => { + ready = true + }) + await new Promise((resolve) => setImmediate(resolve)) + expect(ready).toBe(false) + + releaseInstall?.() + await startupAttempt + expect(ready).toBe(true) + expect(deps.spawnRelay).toHaveBeenCalledTimes(1) + manager.disposeAll() + }) + + it('does not respawn when a startup-owned distro stops during guest install', async () => { + const isDistroRunning = vi + .fn<() => Promise>() + .mockResolvedValueOnce(true) + .mockResolvedValueOnce(false) + const waitForSentinel = vi + .fn() + .mockRejectedValueOnce(startupError(42)) + .mockImplementationOnce(async () => guestTransport()) + const { manager, deps } = createManager({ isDistroRunning, waitForSentinel }) + + await manager.ensureRunningDistroForStartup('Ubuntu') + + expect(deps.spawnRelay).toHaveBeenCalledTimes(1) + expect(deps.runInstall).toHaveBeenCalledTimes(1) + expect(isDistroRunning).toHaveBeenCalledTimes(2) + expect(deps.warn).toHaveBeenCalledWith(expect.stringContaining('continuation skipped')) + manager.disposeAll() + }) + + it('does not retry a transient startup failure after the distro stops', async () => { + const isDistroRunning = vi.fn(async () => false) + const waitForSentinel = vi + .fn() + .mockRejectedValueOnce(startupError(1, 'Catastrophic failure (E_UNEXPECTED)')) + .mockImplementationOnce(async () => guestTransport()) + const { manager, deps } = createManager({ isDistroRunning, waitForSentinel }) + + await manager.ensureRunningDistroForStartup('Ubuntu') + + expect(deps.spawnRelay).toHaveBeenCalledTimes(1) + expect(deps.runInstall).not.toHaveBeenCalled() + expect(isDistroRunning).toHaveBeenCalledOnce() + manager.disposeAll() + }) + + it('lets a concurrent reattach retry after startup drops a stopped attempt', async () => { + let rejectStartup!: (error: Error) => void + const waitForSentinel = vi + .fn() + .mockImplementationOnce( + () => new Promise((_resolve, reject) => (rejectStartup = reject)) + ) + .mockImplementationOnce(async () => guestTransport()) + const { manager, deps } = createManager({ + isDistroRunning: vi.fn(async () => false), + waitForSentinel + }) + + const startup = manager.ensureRunningDistroForStartup('Ubuntu') + await vi.waitFor(() => expect(deps.spawnRelay).toHaveBeenCalledTimes(1)) + const reattach = manager.ensureForDistroReady('ubuntu') + rejectStartup(startupError(42)) + await Promise.all([startup, reattach]) + + expect(deps.spawnRelay).toHaveBeenCalledTimes(2) + expect(deps.installHooks).toHaveBeenCalledTimes(1) manager.disposeAll() }) diff --git a/src/main/agent-hooks/wsl-hook-relay-manager.ts b/src/main/agent-hooks/wsl-hook-relay-manager.ts index 2b4e0356552..5bbd1789de9 100644 --- a/src/main/agent-hooks/wsl-hook-relay-manager.ts +++ b/src/main/agent-hooks/wsl-hook-relay-manager.ts @@ -22,6 +22,7 @@ import { import { wireWslRelayLink } from './wsl-hook-relay-link' import { WslRelayRecovery } from './wsl-hook-relay-recovery' import { wslHookRelayStateKey } from './wsl-hook-relay-state-key' +import type { WslHookRelayLifecycleState } from './wsl-hook-relay-lifecycle-state' import { SshChannelMultiplexer, type MultiplexerTransport } from '../ssh/ssh-channel-multiplexer' import { AGENT_HOOK_REQUEST_REPLAY_METHOD } from '../../shared/agent-hook-relay' import { @@ -30,27 +31,10 @@ import { wslHookRelayEndpointFilePath } from '../../shared/wsl-hook-relay-contract' -type DistroState = { - /** Original casing for wsl.exe argv and breadcrumbs; map keys are lowercased. */ - distro: string - phase: 'starting' | 'running' | 'failed' - child?: ChildProcessWithoutNullStreams - mux?: SshChannelMultiplexer - guestHome?: string - guestEndpointFilePath?: string - opencodeOverlayDir?: string - failures: number - cooldownUntil: number - connectedAt?: number - restartTimer?: ReturnType - reinstallTimer?: ReturnType - lastInstallAt?: number -} - export class WslHookRelayManager { private deps: WslHookRelayManagerDeps private recovery: WslRelayRecovery - private states = new Map() + private states = new Map() /** Distros a hooks-off teardown stopped, so re-enabling can put them back. */ private stoppedByHooksOff = new Set() private defaultDistro: string | null = null @@ -85,18 +69,30 @@ export class WslHookRelayManager { void this.ensureForDistroReady(distro) } - /** Awaitable first attempt for the startup gate; failures retain timer-driven recovery. */ + /** Awaitable first attempt for startup and reattach ordering. */ async ensureForDistroReady(distro: string | null): Promise { + await this.ensureForDistroReadyWhile(distro) + } + + /** Startup retries must not resurrect a distro that stopped after inventory. */ + async ensureRunningDistroForStartup(distro: string): Promise { + await this.ensureForDistroReadyWhile(distro, () => this.deps.isDistroRunning(distro)) + } + + private async ensureForDistroReadyWhile( + distro: string | null, + continueAfterFailure?: () => Promise + ): Promise { if (this.disposed || !isWslHookRelayAllowed(this.deps)) { return } - await this.ensureInternal(distro).catch((err) => { + await this.ensureInternal(distro, continueAfterFailure).catch((err) => { const detail = err instanceof Error ? err.message : String(err) this.deps.warn(`[agent-hooks] WSL hook relay ensure failed: ${detail}`) }) } - private stateFor(distro: string | null): DistroState | undefined { + private stateFor(distro: string | null): WslHookRelayLifecycleState | undefined { // Empty key never matches a real (non-empty) distro state. return this.states.get(wslHookRelayStateKey(distro ?? this.defaultDistro ?? '')) } @@ -146,7 +142,10 @@ export class WslHookRelayManager { } } - private async ensureInternal(requestedDistro: string | null): Promise { + private async ensureInternal( + requestedDistro: string | null, + continueAfterFailure?: () => Promise + ): Promise { const distro = requestedDistro ?? (await this.resolveDefaultDistro()) if (!distro || this.disposed) { return @@ -158,6 +157,10 @@ export class WslHookRelayManager { void maybeRerunWslRelayGuestInstall(this.deps, existing) return } + if (existing.phase === 'starting') { + await existing.firstAttempt + return await this.ensureInternal(requestedDistro, continueAfterFailure) + } if (existing.phase !== 'failed' || Date.now() < existing.cooldownUntil) { return } @@ -182,7 +185,7 @@ export class WslHookRelayManager { if (existing) { this.recovery.clearTimers(existing) } - const state: DistroState = { + const state: WslHookRelayLifecycleState = { distro, phase: 'starting', failures: existing?.failures ?? 0, @@ -195,31 +198,44 @@ export class WslHookRelayManager { const env = buildWslRelaySpawnEnv(coords, bundle.version, instanceKey) + state.firstAttempt = launchWslRelayWithInstall({ + distro: state.distro, + env, + bundleJsPath: bundle.jsPath, + version: bundle.version, + io: this.deps, + // Why the identity half: a hooks-off teardown drops this state and kills its child, but + // that kill reads as a startup failure and the retry loop would respawn an untracked relay. + isDisposed: () => this.disposed || this.states.get(key) !== state, + onChild: (child) => { + state.child = child + }, + onNoNode: () => + this.markFailed( + state, + `no node >= 18 found in distro '${state.distro}'; agent hooks stay degraded there`, + { cooldownBaseMs: NO_NODE_COOLDOWN_MS } + ), + onFailure: (message) => + this.markFailed(state, message, { + cooldownBaseMs: FAILURE_COOLDOWN_BASE_MS + }), + continueAfterFailure, + onContinuationBlocked: () => { + state.phase = 'failed' + state.child = undefined + if (this.states.get(key) === state) { + this.states.delete(key) + } + this.deps.warn( + `[agent-hooks] WSL hook relay (${state.distro}): distro no longer confirmed running; startup continuation skipped` + ) + }, + connect: (transport, child) => this.connect(state, transport, child, instanceKey) + }) + try { - await launchWslRelayWithInstall({ - distro: state.distro, - env, - bundleJsPath: bundle.jsPath, - version: bundle.version, - io: this.deps, - // Why the identity half: a hooks-off teardown drops this state and kills its child, but - // that kill reads as a startup failure and the retry loop would respawn an untracked relay. - isDisposed: () => this.disposed || this.states.get(key) !== state, - onChild: (child) => { - state.child = child - }, - onNoNode: () => - this.markFailed( - state, - `no node >= 18 found in distro '${state.distro}'; agent hooks stay degraded there`, - { cooldownBaseMs: NO_NODE_COOLDOWN_MS } - ), - onFailure: (message) => - this.markFailed(state, message, { - cooldownBaseMs: FAILURE_COOLDOWN_BASE_MS - }), - connect: (transport, child) => this.connect(state, transport, child, instanceKey) - }) + await state.firstAttempt } catch (err) { // Why: teardown may have already recorded this failure; don't double- // count. A request-level error can leave a live child — never leak it. @@ -230,11 +246,13 @@ export class WslHookRelayManager { cooldownBaseMs: FAILURE_COOLDOWN_BASE_MS }) } + } finally { + state.firstAttempt = undefined } } private async connect( - state: DistroState, + state: WslHookRelayLifecycleState, transport: MultiplexerTransport, child: ChildProcessWithoutNullStreams, instanceKey: string @@ -306,7 +324,7 @@ export class WslHookRelayManager { * one failed relaunch must not end self-recovery; the timer's * distro-running probe keeps this from booting stopped distros. */ private markFailed( - state: DistroState, + state: WslHookRelayLifecycleState, message: string, options: { cooldownBaseMs: number } ): void { diff --git a/src/main/agent-hooks/wsl-hook-relay-reattach.test.ts b/src/main/agent-hooks/wsl-hook-relay-reattach.test.ts index 881d25a2984..669a67b4d4f 100644 --- a/src/main/agent-hooks/wsl-hook-relay-reattach.test.ts +++ b/src/main/agent-hooks/wsl-hook-relay-reattach.test.ts @@ -1,12 +1,19 @@ import { describe, expect, it, vi } from 'vitest' -import { ensureWslHookRelayForReattach } from './wsl-hook-relay-reattach' +import { + ensureWslHookRelayForReattach, + WSL_HOOK_RELAY_REATTACH_FAIL_OPEN_MS +} from './wsl-hook-relay-reattach' describe('ensureWslHookRelayForReattach', () => { - it('refreshes the surviving session distro after a local reattach', () => { + it('refreshes the surviving session distro after a local reattach', async () => { const ensure = vi.fn() - ensureWslHookRelayForReattach({ isReattach: true, wslDistro: 'Ubuntu-24.04' }, null, ensure) + await ensureWslHookRelayForReattach( + { isReattach: true, wslDistro: 'Ubuntu-24.04' }, + null, + ensure + ) expect(ensure).toHaveBeenCalledOnce() expect(ensure).toHaveBeenCalledWith('Ubuntu-24.04') @@ -19,20 +26,76 @@ describe('ensureWslHookRelayForReattach', () => { ['blank distro', { isReattach: true, wslDistro: ' ' }, null], ['SSH reattach', { isReattach: true, wslDistro: 'Ubuntu' }, 'ssh-1'], ['relay reattach', { isReattach: true, wslDistro: 'Ubuntu' }, 'relay-1'] - ])('does not refresh a %s', (_label, result, connectionId) => { + ])('does not refresh a %s', async (_label, result, connectionId) => { const ensure = vi.fn() - ensureWslHookRelayForReattach(result, connectionId, ensure) + await ensureWslHookRelayForReattach(result, connectionId, ensure) expect(ensure).not.toHaveBeenCalled() }) - it('preserves exact distro ownership across multiple local reattachments', () => { + it('preserves exact distro ownership across multiple local reattachments', async () => { const ensure = vi.fn() - ensureWslHookRelayForReattach({ isReattach: true, wslDistro: 'Ubuntu' }, null, ensure) - ensureWslHookRelayForReattach({ isReattach: true, wslDistro: 'Debian' }, null, ensure) + await ensureWslHookRelayForReattach({ isReattach: true, wslDistro: 'Ubuntu' }, null, ensure) + await ensureWslHookRelayForReattach({ isReattach: true, wslDistro: 'Debian' }, null, ensure) expect(ensure.mock.calls).toEqual([['Ubuntu'], ['Debian']]) }) + + it('awaits relay readiness only for the reattached WSL session', async () => { + let release!: () => void + const ensure = vi.fn(() => new Promise((resolve) => (release = resolve))) + let ready = false + + const reattach = ensureWslHookRelayForReattach( + { isReattach: true, wslDistro: 'Ubuntu' }, + null, + ensure + ).then(() => { + ready = true + }) + await new Promise((resolve) => setImmediate(resolve)) + expect(ready).toBe(false) + + release() + await reattach + expect(ready).toBe(true) + }) + + it('fails open a WSL reattach when optional relay readiness stalls', async () => { + vi.useFakeTimers() + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + const reattach = ensureWslHookRelayForReattach( + { isReattach: true, wslDistro: 'Ubuntu' }, + null, + () => new Promise(() => {}) + ) + await vi.advanceTimersByTimeAsync(WSL_HOOK_RELAY_REATTACH_FAIL_OPEN_MS) + await expect(reattach).resolves.toBeUndefined() + expect(warn).toHaveBeenCalledWith(expect.stringContaining('timed out')) + expect(vi.getTimerCount()).toBe(0) + } finally { + warn.mockRestore() + vi.useRealTimers() + } + }) + + it('fails open a WSL reattach when relay readiness rejects', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + await expect( + ensureWslHookRelayForReattach({ isReattach: true, wslDistro: 'Ubuntu' }, null, () => + Promise.reject(new Error('relay failed')) + ) + ).resolves.toBeUndefined() + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('reattach failed'), + expect.any(Error) + ) + } finally { + warn.mockRestore() + } + }) }) diff --git a/src/main/agent-hooks/wsl-hook-relay-reattach.ts b/src/main/agent-hooks/wsl-hook-relay-reattach.ts index 837fe82b983..1f7cdf79b48 100644 --- a/src/main/agent-hooks/wsl-hook-relay-reattach.ts +++ b/src/main/agent-hooks/wsl-hook-relay-reattach.ts @@ -3,18 +3,37 @@ import { wslHookRelayManager } from './wsl-hook-relay-manager' type ReattachResult = Pick -export function ensureWslHookRelayForReattach( +export const WSL_HOOK_RELAY_REATTACH_FAIL_OPEN_MS = 60_000 + +export async function ensureWslHookRelayForReattach( result: ReattachResult, connectionId?: string | null, - ensureForDistro: (distro: string) => void = (distro) => - wslHookRelayManager.ensureForDistro(distro) -): void { + ensureForDistro: (distro: string) => void | Promise = (distro) => + wslHookRelayManager.ensureForDistroReady(distro) +): Promise { // Why: the current renderer preference can differ from the surviving PTY's proven distro ownership. if (connectionId || result.isReattach !== true || typeof result.wslDistro !== 'string') { return } const distro = result.wslDistro.trim() if (distro) { - ensureForDistro(distro) + let timeout: ReturnType | undefined + try { + await Promise.race([ + Promise.resolve().then(() => ensureForDistro(distro)), + new Promise((resolve) => { + timeout = setTimeout(() => { + console.warn(`[agent-hooks] WSL hook relay reattach timed out for '${distro}'`) + resolve() + }, WSL_HOOK_RELAY_REATTACH_FAIL_OPEN_MS) + }) + ]) + } catch (error) { + console.warn(`[agent-hooks] WSL hook relay reattach failed for '${distro}':`, error) + } finally { + if (timeout) { + clearTimeout(timeout) + } + } } } diff --git a/src/main/agent-hooks/wsl-hook-relay-startup-reconciliation.ts b/src/main/agent-hooks/wsl-hook-relay-startup-reconciliation.ts index eb5152afb0d..3fe1cb61ee1 100644 --- a/src/main/agent-hooks/wsl-hook-relay-startup-reconciliation.ts +++ b/src/main/agent-hooks/wsl-hook-relay-startup-reconciliation.ts @@ -15,7 +15,7 @@ const defaultDeps: StartupReconciliationDeps = { platform: process.platform, listLiveProcesses: listLiveDaemonProcesses, listRunningDistros: listRunningWslDistros, - ensureForDistro: (distro) => wslHookRelayManager.ensureForDistroReady(distro) + ensureForDistro: (distro) => wslHookRelayManager.ensureRunningDistroForStartup(distro) } export async function reconcileWslHookRelaysOnStartup( diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 010f35f436e..026b6ec2b9e 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -1174,6 +1174,84 @@ describe('registerPtyHandlers', () => { clearProviderPtyState(ptyId) }) + it.each(['renderer', 'runtime'] as const)( + 'rejects %s publication when a WSL PTY exits during relay readiness', + async (entry) => { + const ptyId = `pty-${entry}-relay-exit` + const incarnationId = `incarnation-${entry}-relay-exit` + const runtime = new OrcaRuntimeService() + const registerRuntimePty = vi.spyOn(runtime, 'registerPty') + let releaseRelay!: () => void + const ensureForDistro = vi + .spyOn(wslHookRelayManager, 'ensureForDistroReady') + .mockImplementation(() => new Promise((resolve) => (releaseRelay = resolve))) + const provider = createAgentClaimProvider({ + spawn: vi.fn(async () => { + runtime.onPtySpawned(ptyId, incarnationId) + return { + id: ptyId, + incarnationId, + isReattach: true, + wslDistro: 'Ubuntu-24.04' + } + }), + authoritativeOwnerListings: false + }) + const store = { persistPtyBinding: vi.fn() } + setLocalPtyProvider(provider as never) + registerPtyHandlers( + mainWindow as never, + runtime, + undefined, + undefined, + undefined, + store as never + ) + const spawnArgs = { + cols: 80, + rows: 24, + cwd: '/tmp/worktree', + sessionId: ptyId, + worktreeId: 'repo::/tmp/worktree', + tabId: `tab-${entry}-relay-exit`, + leafId: '55555555-5555-4555-8555-555555555555' + } + const controller = ( + runtime as unknown as { + ptyController: { spawn(args: Record): Promise } + } + ).ptyController + + try { + const spawn = + entry === 'renderer' + ? (handlers.get('pty:spawn')!(null, spawnArgs) as Promise) + : controller.spawn({ + ...spawnArgs, + preAllocatedHandle: 'term-runtime-relay-exit', + persistHostSessionBinding: true + }) + const rejectedSpawn = expect(spawn).rejects.toThrow('agent_session_exited_during_start') + await vi.waitFor(() => expect(ensureForDistro).toHaveBeenCalledWith('Ubuntu-24.04')) + runtime.onPtyExit(ptyId, 0, incarnationId) + releaseRelay() + await rejectedSpawn + + expect(store.persistPtyBinding).not.toHaveBeenCalled() + expect(registerRuntimePty).not.toHaveBeenCalled() + const internals = runtime as unknown as { + earlyExitedPtyIncarnations: Map + pendingPtyRegistrationIncarnations: Map + } + expect(internals.earlyExitedPtyIncarnations.size).toBe(0) + expect(internals.pendingPtyRegistrationIncarnations.size).toBe(0) + } finally { + ensureForDistro.mockRestore() + clearProviderPtyState(ptyId) + } + } + ) + it('adopts a live controller-owned local fallback when listings cannot serialize claims', async () => { const sessions: { id: string @@ -9069,7 +9147,7 @@ describe('registerPtyHandlers', () => { 'adopts a completed runtime-owned pane before replacement launch preflight ($label)', async ({ worktreeId, cwd }) => { type StableAdoption = { - result: { id: string; incarnationId?: string; isReattach?: boolean } + result: { id: string; incarnationId?: string; isReattach?: boolean; wslDistro?: string } owner: { handle?: string; tabId: string; leafId: string; ptyId: string } materialized?: true } | null @@ -9091,6 +9169,9 @@ describe('registerPtyHandlers', () => { const tabId = 'tab-live-owner' const leafId = '66666666-6666-4666-8666-666666666666' const paneKey = makePaneKey(tabId, leafId) + const ensureForDistro = vi + .spyOn(wslHookRelayManager, 'ensureForDistroReady') + .mockResolvedValue(undefined) let ownerPublished = false let releaseAttach!: () => void let attachBarrier: Promise @@ -9110,6 +9191,7 @@ describe('registerPtyHandlers', () => { id: 'pty-live-owner', incarnationId: 'inc-live-owner', isReattach: true, + wslDistro: 'Ubuntu-24.04', snapshot: 'original-live-output', providerSequence: { value: 20, generation: 'continued' as const } } @@ -9334,6 +9416,8 @@ describe('registerPtyHandlers', () => { expect( mainWindow.webContents.send.mock.calls.filter(([channel]) => channel === 'pty:spawned') ).toHaveLength(1) + expect(ensureForDistro).toHaveBeenCalledTimes(2) + ensureForDistro.mockRestore() } ) @@ -17482,9 +17566,10 @@ describe('registerPtyHandlers', () => { it('refreshes the WSL hook relay for the distro a reattached pane already owns', async () => { // Why here and not only in the helper's unit test: nothing else catches pty.ts dropping the // reattach call — the manager owns the hooks/platform gating this spy stands in for. + let releaseRelay!: () => void const ensureForDistro = vi - .spyOn(wslHookRelayManager, 'ensureForDistro') - .mockImplementation(() => {}) + .spyOn(wslHookRelayManager, 'ensureForDistroReady') + .mockImplementation(() => new Promise((resolve) => (releaseRelay = resolve))) setLocalPtyProvider({ spawn: vi.fn(async () => ({ id: 'pty-wsl', isReattach: true, wslDistro: 'Ubuntu-24.04' })), write: vi.fn(), @@ -17499,7 +17584,23 @@ describe('registerPtyHandlers', () => { registerPtyHandlers(mainWindow as never) try { - await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24, sessionId: 'pty-wsl' }) + let spawnReady = false + const spawn = ( + handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + sessionId: 'pty-wsl' + }) as Promise + ).then(() => { + spawnReady = true + }) + await vi.waitFor(() => expect(ensureForDistro).toHaveBeenCalledWith('Ubuntu-24.04')) + await new Promise((resolve) => setImmediate(resolve)) + expect(spawnReady).toBe(false) + + releaseRelay() + await spawn + expect(spawnReady).toBe(true) expect(ensureForDistro).toHaveBeenCalledWith('Ubuntu-24.04') } finally { ensureForDistro.mockRestore() diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 3ff4e36fe1d..29b8d1ab40b 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -4414,10 +4414,6 @@ export function registerPtyHandlers( leafId: preAdoptedStablePane.owner.leafId } } - ensureWslHookRelayForReattach( - { isReattach: true, wslDistro: preAdoptedStablePane.result.wslDistro }, - args.connectionId - ) if (!args.connectionId) { options?.onCodexHomePtySpawned?.({ id: result.id, @@ -4997,7 +4993,6 @@ export function registerPtyHandlers( sequenceBeforeProviderSpawn ) } - ensureWslHookRelayForReattach(result, args.connectionId) runtime?.preparePtyExecutionContext?.( result.id, args.connectionId @@ -5006,6 +5001,9 @@ export function registerPtyHandlers( ? expectedWslDistro : result.wslDistro ) + await ensureWslHookRelayForReattach(result, args.connectionId) + // Why: the PTY can exit while WSL relay readiness is pending. + runtime?.assertPtyRegistrationAllowed?.(result.id, result.incarnationId) } catch (err) { if ( (isNewDaemonSession || preparedProvisionalExecutionContext) && @@ -6430,7 +6428,6 @@ export function registerPtyHandlers( sequenceBeforeProviderSpawn ) } - ensureWslHookRelayForReattach(result, args.connectionId) runtime?.preparePtyExecutionContext?.( result.id, args.connectionId @@ -6439,6 +6436,9 @@ export function registerPtyHandlers( ? expectedWslDistro : result.wslDistro ) + await ensureWslHookRelayForReattach(result, args.connectionId) + // Why: the PTY can exit while WSL relay readiness is pending. + runtime?.assertPtyRegistrationAllowed?.(result.id, result.incarnationId) spawnTiming.mark('provider_spawn') } catch (err) { if ((isMintedSessionId || preparedProvisionalExecutionContext) && effectiveSessionAppId) { diff --git a/src/main/startup/first-window-startup-services.test.ts b/src/main/startup/first-window-startup-services.test.ts index ca9d90b7006..d98f8fbdcf4 100644 --- a/src/main/startup/first-window-startup-services.test.ts +++ b/src/main/startup/first-window-startup-services.test.ts @@ -47,7 +47,7 @@ describe('startFirstWindowStartupServices', () => { expect(completed).toBe(true) }) - it('reconciles after both authorities and keeps local reattach gated until relays are ready', async () => { + it('starts reconciliation after both authorities without gating unrelated local PTYs', async () => { const events: string[] = [] let resolveDaemon!: () => void let resolveHooks!: () => void @@ -66,7 +66,10 @@ describe('startFirstWindowStartupServices', () => { reconcileWslHookRelays: () => new Promise((resolve) => { events.push('relay-reconciliation-started') - resolveRelays = resolve + resolveRelays = () => { + events.push('relay-reconciliation-ready') + resolve() + } }), onDaemonError: vi.fn(), onAgentHookServerError: vi.fn(), @@ -80,16 +83,22 @@ describe('startFirstWindowStartupServices', () => { resolveDaemon() await vi.waitFor(() => expect(events).toContain('relay-reconciliation-started')) + let firstWindowReady = false + void started.firstWindowReady.then(() => { + firstWindowReady = true + }) + await new Promise((resolve) => setImmediate(resolve)) + expect(firstWindowReady).toBe(true) let localPtyReady = false void started.localPtyReady.then(() => { localPtyReady = true }) - await Promise.resolve() - expect(localPtyReady).toBe(false) - - resolveRelays() await started.localPtyReady expect(localPtyReady).toBe(true) + expect(events).not.toContain('relay-reconciliation-ready') + + resolveRelays() + await vi.waitFor(() => expect(events).toContain('relay-reconciliation-ready')) }) it('reports reconciliation failure and still opens the startup gate', async () => { @@ -104,7 +113,9 @@ describe('startFirstWindowStartupServices', () => { }) await expect(started.localPtyReady).resolves.toBeUndefined() - expect(onWslHookRelayReconciliationError).toHaveBeenCalledWith(expect.any(Error)) + await vi.waitFor(() => + expect(onWslHookRelayReconciliationError).toHaveBeenCalledWith(expect.any(Error)) + ) }) it('opens the provider gate when daemon startup finishes without waiting for hooks', async () => { @@ -233,16 +244,19 @@ describe('startFirstWindowStartupServices', () => { vi.useFakeTimers() const onDaemonError = vi.fn() const onAgentHookServerError = vi.fn() + const reconcileWslHookRelays = vi.fn(async () => {}) let daemonSignal: AbortSignal | undefined try { const started = startFirstWindowStartupServices({ startDaemonPtyProvider: (signal) => { daemonSignal = signal - return new Promise(() => {}) + return new Promise((resolve) => { + signal.addEventListener('abort', () => resolve(), { once: true }) + }) }, startAgentHookServer: () => Promise.resolve(), - reconcileWslHookRelays: vi.fn(async () => {}), + reconcileWslHookRelays, onDaemonError, onAgentHookServerError, onWslHookRelayReconciliationError: vi.fn() @@ -257,6 +271,7 @@ describe('startFirstWindowStartupServices', () => { expect(onDaemonError).toHaveBeenCalledWith(expect.any(Error)) expect(onAgentHookServerError).not.toHaveBeenCalled() expect(daemonSignal?.aborted).toBe(true) + expect(reconcileWslHookRelays).not.toHaveBeenCalled() } finally { vi.useRealTimers() } diff --git a/src/main/startup/first-window-startup-services.ts b/src/main/startup/first-window-startup-services.ts index 813779d005d..19c49f65e24 100644 --- a/src/main/startup/first-window-startup-services.ts +++ b/src/main/startup/first-window-startup-services.ts @@ -79,13 +79,15 @@ export function startFirstWindowStartupServices({ const daemon = startService('daemon PTY provider', startDaemonPtyProvider, onDaemonError) const hooks = startService('agent hook server', startAgentHookServer, onAgentHookServerError) let failOpened = false - const allServicesReady = Promise.all([daemon.ready, hooks.ready]) + const startupAuthoritiesReady = Promise.all([daemon.ready, hooks.ready]).then(() => undefined) + const wslHookRelayReconciliation = startupAuthoritiesReady .then(() => (failOpened ? undefined : reconcileWslHookRelays())) .catch(onWslHookRelayReconciliationError) .then(() => undefined) + void wslHookRelayReconciliation let windowTimeout: ReturnType | null = null let failOpenTimeout: ReturnType | null = null - const servicesSettled = allServicesReady.finally(() => { + const startupAuthoritiesSettled = startupAuthoritiesReady.finally(() => { if (windowTimeout) { clearTimeout(windowTimeout) } @@ -102,12 +104,12 @@ export function startFirstWindowStartupServices({ }, LOCAL_PTY_STARTUP_FAIL_OPEN_TIMEOUT_MS) }) const firstWindowReady = Promise.race([ - servicesSettled, + startupAuthoritiesSettled, new Promise((resolve) => { windowTimeout = setTimeout(resolve, FIRST_WINDOW_STARTUP_SERVICE_TIMEOUT_MS) }) ]) - const localPtyReady = Promise.race([servicesSettled, failOpenReady]) + const localPtyReady = Promise.race([startupAuthoritiesSettled, failOpenReady]) // Why: destructive routing only needs daemon authority. A stalled optional // hook server must not hold terminal close for the full fail-open window. const localPtyProviderReady = Promise.race([daemon.ready, failOpenReady])