diff --git a/config/unproven-pty-retirement-baseline.txt b/config/unproven-pty-retirement-baseline.txt index 657349c427c..77ae1a16021 100644 --- a/config/unproven-pty-retirement-baseline.txt +++ b/config/unproven-pty-retirement-baseline.txt @@ -5,7 +5,6 @@ 1 src/main/ipc/pty/ipc/renderer-kill.ts # SSH provider unregistered: R4-4 tab-close replay 1 src/main/ipc/pty/pane/absent-pane-owner.ts # host-confirmed absence, untyped -1 src/main/ipc/pty/pane/spawn-registration.ts # drops ownership even when the discard shutdown threw 1 src/main/ipc/pty/provider/bind-listeners.ts # observed provider exit, untyped 1 src/main/ipc/pty/provider/local-configure.ts # observed local exit, untyped 1 src/main/ipc/pty/provider/ownership-state.ts # deletePtyOwnership, test-only caller diff --git a/src/main/ipc/pty-ssh-stop-verdict.test.ts b/src/main/ipc/pty-ssh-stop-verdict.test.ts index 8599029026f..497869511ef 100644 --- a/src/main/ipc/pty-ssh-stop-verdict.test.ts +++ b/src/main/ipc/pty-ssh-stop-verdict.test.ts @@ -2,7 +2,12 @@ import { describe, expect, it, vi } from 'vitest' import { SSH_PROVIDER_UNREGISTERED_REASON } from '../../shared/pty-liveness-verdict' import { setupPtyIpcSuite } from './pty-ipc-test-harness' import { registerPtyHandlers } from './pty/register-handlers' -import { deletePtyOwnership, setPtyOwnership } from './pty/provider/ownership-state' +import { + deletePtyOwnership, + ptyIncarnationById, + ptyOwnership, + setPtyOwnership +} from './pty/provider/ownership-state' import { getLocalPtyProvider } from './pty/provider/registry' vi.mock('electron', () => import('./pty-ipc-mock-registry').then((m) => m.electronModuleMock())) @@ -58,7 +63,11 @@ describe('stopping a PTY whose SSH provider is unregistered', () => { controller: { kill: (ptyId: string) => boolean listProcesses: (hostId?: string) => Promise<{ id: string }[]> - retireRejectedPty: (ptyId: string, stopConfirmed: boolean) => void + retireRejectedPty: ( + ptyId: string, + stopConfirmed: boolean, + incarnationId: string | undefined + ) => void stopAndWait: (ptyId: string, opts?: { deadlineMs?: number }) => Promise } runtime: { @@ -131,7 +140,7 @@ describe('stopping a PTY whose SSH provider is unregistered', () => { setPtyOwnership(ptyId, 'ssh:ssh-dropped') const { controller, runtime } = installController() try { - controller.retireRejectedPty(ptyId, false) + controller.retireRejectedPty(ptyId, false, undefined) expect(runtime.markPtyLivenessUnverifiable).toHaveBeenCalledWith( ptyId, @@ -144,6 +153,26 @@ describe('stopping a PTY whose SSH provider is unregistered', () => { } }) + it('leaves a same-id respawn alone when the rejected incarnation is retired', () => { + const ptyId = 'ssh-respawned-split' + // The rejected spawn's stop already retired its incarnation; a respawn took the id since. + setPtyOwnership(ptyId, 'ssh:ssh-live') + ptyIncarnationById.set(ptyId, 'inc-respawn') + const { controller, runtime } = installController() + try { + controller.retireRejectedPty(ptyId, true, 'inc-rejected') + controller.retireRejectedPty(ptyId, false, 'inc-rejected') + + expect(ptyOwnership.get(ptyId)).toBe('ssh:ssh-live') + expect(ptyIncarnationById.get(ptyId)).toBe('inc-respawn') + expect(runtime.onPtyExit).not.toHaveBeenCalled() + expect(runtime.markPtyLivenessUnverifiable).not.toHaveBeenCalled() + } finally { + ptyIncarnationById.delete(ptyId) + deletePtyOwnership(ptyId) + } + }) + it('still confirms a stop the owning provider actually performed', async () => { const daemon = installObservableDaemonTestProvider() vi.spyOn(getLocalPtyProvider(), 'listProcesses').mockResolvedValue([]) diff --git a/src/main/ipc/pty/ipc/renderer-kill.ts b/src/main/ipc/pty/ipc/renderer-kill.ts index 1995ff36621..a69b92b2f00 100644 --- a/src/main/ipc/pty/ipc/renderer-kill.ts +++ b/src/main/ipc/pty/ipc/renderer-kill.ts @@ -106,19 +106,23 @@ async function stopRendererOwnedPtyProcess( return } const shutdownProvider = provider ?? getProviderForPty(args.id) + const stoppingIncarnationId = ptyIncarnationById.get(args.id) let proof: PtyExitProof try { const providerExitObserved = await shutdownProviderAndDetectExit(shutdownProvider, args.id, { immediate: true, keepHistory: args.keepHistory ?? false }) - proof = PtyExitProof.from(providerExitObserved ? 'exit-observed' : 'shutdown-acknowledged') + proof = PtyExitProof.from( + providerExitObserved ? 'exit-observed' : 'shutdown-acknowledged', + stoppingIncarnationId + ) } catch (err) { if (!isPtyAlreadyGoneError(err)) { // Why: a failed shutdown can leave the local daemon's process alive; keep ownership so a retry can reach it. throw err } - proof = PtyExitProof.from('absent') + proof = PtyExitProof.from('absent', stoppingIncarnationId) } // Why: some shutdown paths do not emit onExit through the provider listener. // Explicit cleanup is idempotent and covers already-dead PTYs. diff --git a/src/main/ipc/pty/ipc/spawn-commit-persist.ts b/src/main/ipc/pty/ipc/spawn-commit-persist.ts index 03b7ef9e0bd..bca9bb7086d 100644 --- a/src/main/ipc/pty/ipc/spawn-commit-persist.ts +++ b/src/main/ipc/pty/ipc/spawn-commit-persist.ts @@ -1,4 +1,3 @@ -import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host' import { closeStartupQueryAuthorityForPty } from '../provider/registry' import { createTerminalSessionStateSaveFailureMessage } from '../../../../shared/terminal-session-state-save-failure' import { recordCodexPaneAccountForSpawn } from '../host-env/codex-home' @@ -9,7 +8,6 @@ import { pendingPtyIdBySerializerGeneration, rendererSerializerReadiness } from '../pane/serializer-state' -import { ptyOwnership, ptyIncarnationById } from '../provider/ownership-state' import { ptySizes } from '../delivery/visibility-state' import { resolveCommittedPtySize, type PtyGrid } from '../delivery/attached-pty-size' import { discardUnpersistedPtySpawn } from '../pane/spawn-registration' @@ -70,7 +68,7 @@ export async function persistPtyIpcSpawnCommit(ctx: PtyIpcSpawnState): Promise