From 36bda5efe557c8306fc98aa457e5322c745efb28 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 19 Oct 2026 09:59:24 -0700 Subject: [PATCH] fix(pty): an exit proof names its incarnation; a spawn is tracked from its reply (#30754) * fix(pty): an exit proof names its incarnation, and a failed discard keeps ownership A same-id respawn could be retired by its predecessor's exit proof; finishPtyShutdown now refuses a proof whose incarnation is not the record's, and retireRejectedPty is scoped to the rejected spawn's incarnation. A spawn discarded after a failed save no longer drops ownership when its shutdown failed: it reads unverifiable until a later stop reaches it. Co-Authored-By: Claude * fix(pty): a failed discard leaves a same-id replacement's verdict alone Co-Authored-By: Claude * fix(pty): a lost discard reply does not overwrite an observed exit Co-Authored-By: Claude * fix(pty): a failed discard tracks the PTY it could not stop; test the incarnation refusal The discard runs before the spawn is published, so it records ownership and incarnation before stopping; a failed stop now leaves a handle. Every exit proof states its incarnation. Co-Authored-By: Claude * refactor(pty): track a spawn from its reply, so only an exit or a proven stop removes it Ownership and incarnation are recorded at commit start instead of at publish, before the binding save. The discard no longer re-records anything, so an exit observed during the save stays exited. Co-Authored-By: Claude * fix(pty): admit a same-id respawn despite a predecessor's verdict; retire an exit seen before commit on its fence Co-Authored-By: Claude * fix(pty): admit a fresh spawn only past the runtime's incarnation-scoped start fence Replaces the id-only verdict check, so a same-id respawn is tracked and an incarnation that exited before its commit is not resurrected; registration no longer has to retire it. Co-Authored-By: Claude * fix(pty): a proof that names no incarnation still retires; tidy kill-time capture and adopted re-record Co-Authored-By: Claude --------- Co-authored-by: Claude --- config/unproven-pty-retirement-baseline.txt | 1 - src/main/ipc/pty-ssh-stop-verdict.test.ts | 35 ++++- src/main/ipc/pty/ipc/renderer-kill.ts | 8 +- src/main/ipc/pty/ipc/spawn-commit-persist.ts | 10 +- src/main/ipc/pty/ipc/spawn-commit.ts | 4 +- .../ipc/pty/pane/spawn-registration.test.ts | 81 ++++++++++ src/main/ipc/pty/pane/spawn-registration.ts | 72 +++++++-- src/main/ipc/pty/provider/liveness.ts | 16 +- src/main/ipc/pty/runtime/controller.ts | 4 +- .../ipc/pty/runtime/kill-exit-proof.test.ts | 2 +- .../kill-legacy-relay-bridged-exit.test.ts | 2 +- .../pty/runtime/kill-same-id-respawn.test.ts | 129 ++++++++++++++++ src/main/ipc/pty/runtime/kill.ts | 49 +++++-- src/main/ipc/pty/runtime/spawn-commit.ts | 20 +-- ...-host-partitioned-ssh-pty-bindings.test.ts | 2 +- ...ence-ssh-lease-tombstone-retention.test.ts | 8 +- .../persistence-ssh-remote-pty-leases.test.ts | 6 +- .../pty-spawn-replacement-durability.test.ts | 138 ++++++++++++++++++ src/main/pty-exit-proof.ts | 11 +- ...a-runtime-terminal-split-authority.test.ts | 15 +- .../runtime-pty-controller-contract.ts | 3 +- .../terminal/launch/terminal-split-surface.ts | 2 +- .../ssh-expired-lease-pane-readoption.test.ts | 2 +- .../legacy-relay-bridge-registry.ts | 2 +- .../ssh/orcad-migration-terminal-gate.test.ts | 6 +- src/main/ssh/orcad-migration-terminal-gate.ts | 2 +- 26 files changed, 537 insertions(+), 93 deletions(-) create mode 100644 src/main/ipc/pty/pane/spawn-registration.test.ts create mode 100644 src/main/ipc/pty/runtime/kill-same-id-respawn.test.ts 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