From 5103500ea84c2e39862b2313c11fdfd2beaba513 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:53:22 -0700 Subject: [PATCH] Defend against transient PTY setup failures with state cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Host-env setup failures now trigger cleanup of runtime-allocated PTY state. Cached PTY geometry is preserved after transient reattach failures but cleared when the provider reports the PTY exited before the spawn reply—preventing stale geometry from corrupting future operations. Error handling now distinguishes expired SSH sessions and early-exit conditions to preserve geometry appropriately. --- .../ipc/pty-applied-size-reporting.test.ts | 81 +++++++++++++++++++ .../pty-controller-spawn-admission.test.ts | 73 +++++++++++++++++ src/main/ipc/pty/ipc/spawn-execute.ts | 19 +++-- src/main/ipc/pty/runtime/spawn-execute.ts | 19 +++-- src/main/ipc/pty/runtime/spawn-preflight.ts | 55 +++++++------ 5 files changed, 210 insertions(+), 37 deletions(-) diff --git a/src/main/ipc/pty-applied-size-reporting.test.ts b/src/main/ipc/pty-applied-size-reporting.test.ts index 5607c7cab61..87b8c5e086f 100644 --- a/src/main/ipc/pty-applied-size-reporting.test.ts +++ b/src/main/ipc/pty-applied-size-reporting.test.ts @@ -75,6 +75,30 @@ describe('registerPtyHandlers', () => { return write } + function setupProviderWithTransientAttachFailure(earlyExit = false): ReturnType { + const spawn = vi.fn(async (opts: { sessionId?: string }) => { + if (spawn.mock.calls.length > 1) { + if (earlyExit) { + return { id: opts.sessionId ?? 'daemon-pty', exitedBeforeSpawnReply: true as const } + } + throw new Error('transport unavailable') + } + return { id: opts.sessionId ?? 'daemon-pty' } + }) + setLocalPtyProvider({ + spawn, + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(), + shutdown: vi.fn(), + onData: vi.fn(() => vi.fn()), + onExit: vi.fn(() => vi.fn()), + listProcesses: vi.fn(async () => []), + getForegroundProcess: vi.fn(async () => null) + } as never) + return spawn + } + const resizeListener = (): ((event: unknown, args: unknown) => void) => { const call = onMock.mock.calls.find((entry: unknown[]) => entry[0] === 'pty:resize') if (!call) { @@ -150,6 +174,63 @@ describe('registerPtyHandlers', () => { const reported = await handlers.get('pty:getSize')!(null, { id }) expect(reported).toEqual({ cols: 100, rows: 30 }) }) + it('preserves cached geometry after a transient renderer attach failure', async () => { + setupProviderWithTransientAttachFailure() + handlers.clear() + registerPtyHandlers(mainWindow as never) + const request = { sessionId: 'daemon-existing-size', env: {} } + await handlers.get('pty:spawn')!(null, { ...request, cols: 200, rows: 50 }) + + await expect( + handlers.get('pty:spawn')!(null, { ...request, cols: 80, rows: 24 }) + ).rejects.toThrow('transport unavailable') + + await expect(handlers.get('pty:getSize')!(null, { id: request.sessionId })).resolves.toEqual({ + cols: 200, + rows: 50 + }) + }) + it('preserves cached geometry after a transient runtime attach failure', async () => { + setupProviderWithTransientAttachFailure() + let controller: + | { + spawn: (args: Record) => Promise + getSize: (id: string) => { cols: number; rows: number } | null + } + | undefined + const runtime = { + setPtyController: vi.fn((next) => { + controller = next + }), + registerPreAllocatedHandleForPty: vi.fn(), + registerPty: vi.fn() + } + handlers.clear() + registerPtyHandlers(mainWindow as never, runtime as never) + const request = { sessionId: 'runtime-existing-size' } + await controller!.spawn({ ...request, cols: 200, rows: 50 }) + + await expect(controller!.spawn({ ...request, cols: 80, rows: 24 })).rejects.toThrow( + 'transport unavailable' + ) + + expect(controller!.getSize(request.sessionId)).toEqual({ cols: 200, rows: 50 }) + }) + it('clears cached geometry when the provider proves the PTY exited before reply', async () => { + setupProviderWithTransientAttachFailure(true) + handlers.clear() + registerPtyHandlers(mainWindow as never) + const request = { sessionId: 'daemon-exited-before-reply', env: {} } + await handlers.get('pty:spawn')!(null, { ...request, cols: 200, rows: 50 }) + + await expect( + handlers.get('pty:spawn')!(null, { ...request, cols: 80, rows: 24 }) + ).rejects.toThrow('agent_session_exited_during_start') + + await expect( + handlers.get('pty:getSize')!(null, { id: request.sessionId }) + ).resolves.toBeNull() + }) it('fans out accepted desktop resizes to the runtime after provider resize', async () => { const resize = vi.fn() setupProviderWithAppliedSize({ applied: { cols: 120, rows: 30 }, resize }) diff --git a/src/main/ipc/pty-controller-spawn-admission.test.ts b/src/main/ipc/pty-controller-spawn-admission.test.ts index ae4af67434c..65f5ba565c9 100644 --- a/src/main/ipc/pty-controller-spawn-admission.test.ts +++ b/src/main/ipc/pty-controller-spawn-admission.test.ts @@ -1,4 +1,9 @@ import { describe, expect, it, vi } from 'vitest' +import { + openCodeBuildPtyEnvMock, + openCodeClearPtyMock, + piClearPtyMock +} from './pty-ipc-mock-registry' import { setupPtyIpcSuite } from './pty-ipc-test-harness' import type { AgentSessionOwnerBinding } from '../../shared/agent-session-host-authority' import { OrcaRuntimeService } from '../runtime/orca-runtime' @@ -227,6 +232,74 @@ describe('registerPtyHandlers', () => { expect.objectContaining({ sessionId: 'serve-stable-session', isNewSession: true }) ) }) + it('clears a fresh runtime session when host-env setup fails', async () => { + const provider = createAgentClaimProvider({ spawn: vi.fn() }) + setLocalPtyProvider(provider as never) + openCodeBuildPtyEnvMock.mockImplementationOnce(() => { + throw new Error('host env failed') + }) + const controller = registerAgentClaimController() + + await expect( + controller.spawn({ + cols: 80, + rows: 24, + cwd: '/tmp/runtime-host-env-failure', + worktreeId: 'repo::/tmp/runtime-host-env-failure' + }) + ).rejects.toThrow('host env failed') + + expect(provider.spawn).not.toHaveBeenCalled() + expect(openCodeClearPtyMock).toHaveBeenCalledOnce() + expect(piClearPtyMock).toHaveBeenCalledOnce() + }) + it('preserves caller-owned runtime state when host-env setup fails', async () => { + const provider = createAgentClaimProvider({ spawn: vi.fn() }) + setLocalPtyProvider(provider as never) + openCodeBuildPtyEnvMock.mockImplementationOnce(() => { + throw new Error('host env failed') + }) + const controller = registerAgentClaimController() + + await expect( + controller.spawn({ + cols: 80, + rows: 24, + sessionId: 'caller-owned-session', + isNewSession: true + }) + ).rejects.toThrow('host env failed') + + expect(provider.spawn).not.toHaveBeenCalled() + expect(openCodeClearPtyMock).not.toHaveBeenCalled() + expect(piClearPtyMock).not.toHaveBeenCalled() + }) + it('preserves operation-owned runtime state when retry host-env setup fails', async () => { + const physicalSpawn = vi.fn(async (options: { sessionId?: string }) => ({ + id: options.sessionId ?? 'unexpected-spawn' + })) + const provider = createAgentClaimProvider({ spawn: physicalSpawn }) + setLocalPtyProvider(provider as never) + const controller = registerAgentClaimController() + const request = { + cols: 80, + rows: 24, + worktreeId: 'repo::/tmp/operation-retry', + agentSessionCreateOperationId: 'a'.repeat(43) + } + await controller.spawn(request) + openCodeClearPtyMock.mockClear() + piClearPtyMock.mockClear() + openCodeBuildPtyEnvMock.mockImplementationOnce(() => { + throw new Error('host env failed') + }) + + await expect(controller.spawn(request)).rejects.toThrow('host env failed') + + expect(physicalSpawn).toHaveBeenCalledOnce() + expect(openCodeClearPtyMock).not.toHaveBeenCalled() + expect(piClearPtyMock).not.toHaveBeenCalled() + }) it('adopts a daemon owner recovered from provider listing before claimed ensure', async () => { const owner: AgentSessionOwnerBinding = { claim: recoveredAgentClaim, diff --git a/src/main/ipc/pty/ipc/spawn-execute.ts b/src/main/ipc/pty/ipc/spawn-execute.ts index 73c8c535119..58525e162aa 100644 --- a/src/main/ipc/pty/ipc/spawn-execute.ts +++ b/src/main/ipc/pty/ipc/spawn-execute.ts @@ -140,19 +140,24 @@ export async function executePtyIpcSpawn(ctx: PtyIpcSpawnState): Promise { const spawnError = normalizeNodePtySpawnError(err) const isIdentityMismatch = isSshPtyIdentityMismatchError(spawnError) || isSshPtyIdentityMismatchError(rawMessage) + const isExpiredSshSession = + Boolean(args.connectionId) && + (spawnError.message.includes(SSH_SESSION_EXPIRED_ERROR) || + rawMessage.includes(SSH_SESSION_EXPIRED_ERROR)) + const exitedBeforeSpawnReply = + ctx.rejectedRegistrationCandidate?.exitedBeforeSpawnReply === true if (ctx.effectiveSessionAppId !== undefined) { - if (isIdentityMismatch && ctx.hadSessionSizeBeforeAttach && ctx.sessionSizeBeforeAttach) { + if ( + ctx.hadSessionSizeBeforeAttach && + ctx.sessionSizeBeforeAttach && + (isIdentityMismatch || (!isExpiredSshSession && !exitedBeforeSpawnReply)) + ) { ptySizes.set(ctx.effectiveSessionAppId, ctx.sessionSizeBeforeAttach) } else { ptySizes.delete(ctx.effectiveSessionAppId) } } - if ( - args.connectionId && - ctx.effectiveSessionRelayId !== undefined && - (spawnError.message.includes(SSH_SESSION_EXPIRED_ERROR) || - rawMessage.includes(SSH_SESSION_EXPIRED_ERROR)) - ) { + if (args.connectionId && ctx.effectiveSessionRelayId !== undefined && isExpiredSshSession) { // Why: expired remote reattach = relay already dropped the PTY; clear the lease so writes can't restore the stale binding. if (ctx.effectiveSessionAppId !== undefined && !isIdentityMismatch) { clearProviderPtyState(ctx.effectiveSessionAppId) diff --git a/src/main/ipc/pty/runtime/spawn-execute.ts b/src/main/ipc/pty/runtime/spawn-execute.ts index 5adb0437d77..99829f119bb 100644 --- a/src/main/ipc/pty/runtime/spawn-execute.ts +++ b/src/main/ipc/pty/runtime/spawn-execute.ts @@ -220,19 +220,24 @@ export async function executeRuntimePtySpawn(ctx: RuntimePtySpawnState): Promise const spawnError = normalizeNodePtySpawnError(err) const isIdentityMismatch = isSshPtyIdentityMismatchError(spawnError) || isSshPtyIdentityMismatchError(rawMessage) + const isExpiredSshSession = + Boolean(args.connectionId) && + (spawnError.message.includes(SSH_SESSION_EXPIRED_ERROR) || + rawMessage.includes(SSH_SESSION_EXPIRED_ERROR)) + const exitedBeforeSpawnReply = + ctx.rejectedRegistrationCandidate?.exitedBeforeSpawnReply === true if (ctx.effectiveSessionAppId !== undefined) { - if (isIdentityMismatch && ctx.hadSessionSizeBeforeAttach && ctx.sessionSizeBeforeAttach) { + if ( + ctx.hadSessionSizeBeforeAttach && + ctx.sessionSizeBeforeAttach && + (isIdentityMismatch || (!isExpiredSshSession && !exitedBeforeSpawnReply)) + ) { ptySizes.set(ctx.effectiveSessionAppId, ctx.sessionSizeBeforeAttach) } else { ptySizes.delete(ctx.effectiveSessionAppId) } } - if ( - args.connectionId && - ctx.effectiveSessionRelayId !== undefined && - (spawnError.message.includes(SSH_SESSION_EXPIRED_ERROR) || - rawMessage.includes(SSH_SESSION_EXPIRED_ERROR)) - ) { + if (args.connectionId && ctx.effectiveSessionRelayId !== undefined && isExpiredSshSession) { if (ctx.effectiveSessionAppId !== undefined && !isIdentityMismatch) { clearProviderPtyState(ctx.effectiveSessionAppId) deletePtyOwnership(ctx.effectiveSessionAppId) diff --git a/src/main/ipc/pty/runtime/spawn-preflight.ts b/src/main/ipc/pty/runtime/spawn-preflight.ts index f47a88c0ed3..45c3ea1172f 100644 --- a/src/main/ipc/pty/runtime/spawn-preflight.ts +++ b/src/main/ipc/pty/runtime/spawn-preflight.ts @@ -36,6 +36,7 @@ import { resolveLocalProjectRuntimeForWorktreeId } from '../../../local-project- import { resolvePathEnvKey } from '../../../pty/windows-environment-path' import { stampWslOrchestrationCompatibilityHost } from '../../../pty/wsl-orca-env' import { ensureCodexStateDbBackfillRecoveryStarted } from '../../../codex/codex-state-db-backfill-recovery' +import { clearProviderPtyState } from '../provider/state-cleanup' import type { RuntimePtySpawnState } from './spawn-state' export async function prepareRuntimePtySpawn( @@ -243,29 +244,37 @@ export async function prepareRuntimePtySpawn( if (!isSafePtySessionId(ctx.sessionId, getAppEnvironment().getPath('userData'))) { throw new Error('Invalid PTY session id') } - ctx.env = buildPtyHostEnv(ctx.sessionId, ctx.env ?? {}, { - isPackaged: getAppEnvironment().isPackaged(), - resourcesPath: process.resourcesPath, - userDataPath: getAppEnvironment().getPath('userData'), - selectedCodexHomePath: ctx.selectedCodexHomePath, - skipCodexHomeEnv: ctx.skipCodexHomeEnv, - stripInheritedOrcaCodexHome: ctx.stripInheritedOrcaCodexHome, - launchCommand: ctx.launchCommand, - launchAgent: isTuiAgent(args.launchAgent) ? args.launchAgent : undefined, - isWsl: shouldSkipCodexHomeEnvForWindowsShell(ctx.daemonShellOverride, ctx.cwd), - wslDistro: ctx.codexSelectionTarget.runtime === 'wsl' ? ctx.expectedWslDistro : null, - agentStatusHooksEnabled: isAgentStatusHooksEnabled(ptySettings), - codexStatusHooksEnabled: isCodexStatusHooksEnabled(ptySettings), - networkProxySettings: ptySettings, - deferGitConfigGuardToDaemon: - ctx.provider.supportsGitCredentialGuardHost?.(ctx.sessionId) === true - }) - stampWslOrchestrationCompatibilityHost( - ctx.env, - ctx.deps.runtime?.getOrchestrationCompatibilityHostId?.(), - ctx.codexSelectionTarget.runtime === 'wsl' ? ctx.expectedWslDistro : null - ) - promoteAgentTeamsShimPath(ctx.env, ctx.requestedAgentTeamsPath) + try { + ctx.env = buildPtyHostEnv(ctx.sessionId, ctx.env ?? {}, { + isPackaged: getAppEnvironment().isPackaged(), + resourcesPath: process.resourcesPath, + userDataPath: getAppEnvironment().getPath('userData'), + selectedCodexHomePath: ctx.selectedCodexHomePath, + skipCodexHomeEnv: ctx.skipCodexHomeEnv, + stripInheritedOrcaCodexHome: ctx.stripInheritedOrcaCodexHome, + launchCommand: ctx.launchCommand, + launchAgent: isTuiAgent(args.launchAgent) ? args.launchAgent : undefined, + isWsl: shouldSkipCodexHomeEnvForWindowsShell(ctx.daemonShellOverride, ctx.cwd), + wslDistro: ctx.codexSelectionTarget.runtime === 'wsl' ? ctx.expectedWslDistro : null, + agentStatusHooksEnabled: isAgentStatusHooksEnabled(ptySettings), + codexStatusHooksEnabled: isCodexStatusHooksEnabled(ptySettings), + networkProxySettings: ptySettings, + deferGitConfigGuardToDaemon: + ctx.provider.supportsGitCredentialGuardHost?.(ctx.sessionId) === true + }) + stampWslOrchestrationCompatibilityHost( + ctx.env, + ctx.deps.runtime?.getOrchestrationCompatibilityHostId?.(), + ctx.codexSelectionTarget.runtime === 'wsl' ? ctx.expectedWslDistro : null + ) + promoteAgentTeamsShimPath(ctx.env, ctx.requestedAgentTeamsPath) + } catch (error) { + // Why: host-env setup can materialize agent hooks/extensions before failing. + if (ctx.requestedSessionId === undefined) { + clearProviderPtyState(ctx.sessionId) + } + throw error + } } return null