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