From 893820024fd04ea31eaee11700e4ccbf536ef6a0 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 20 Aug 2026 02:09:31 -0700 Subject: [PATCH] fix(ssh): stop a stuck reattach from marking a dead remote pane live MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On the restoreRequired path the retry cancels the old delivery, and the exit fence is already closed by then. A pty.exit arriving during that round trip was published and tombstoned, and the unconditional acceptLivePty that followed erased it — probePtyLiveness then answered "live" forever for a process the owning host had reported dead, so no terminal_gone was raised and the renderer kept a dead pane bound. Open live evidence before the cancel round trip and settle it after, so an exit observed in that window invalidates the evidence and refuses the promotion. --- src/main/providers/ssh-pty-provider.ts | 3 ++ .../ssh-pty-restore-required-reattach.test.ts | 36 +++++++++++++++++++ .../providers/ssh-pty-session-reattach.ts | 20 +++++++++-- 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/main/providers/ssh-pty-provider.ts b/src/main/providers/ssh-pty-provider.ts index 9f118b45c6e..26d87d95cda 100644 --- a/src/main/providers/ssh-pty-provider.ts +++ b/src/main/providers/ssh-pty-provider.ts @@ -100,6 +100,9 @@ export class SshPtyProvider implements IPtyProvider { rememberPtyIncarnation: (relayPtyId, incarnationId) => this.outputState.acceptPtyIncarnation(relayPtyId, incarnationId), acceptLivePty: (relayPtyId) => this.acceptLivePty(relayPtyId), + beginLivePtyEvidence: (appPtyId) => this.beginLivePtyEvidence(appPtyId), + settleLivePtyEvidence: (appPtyId, evidence, acceptLive) => + this.settleLivePtyEvidence(appPtyId, evidence, acceptLive), acceptUnverifiablePty: (relayPtyId) => this.acceptUnverifiablePty(relayPtyId), acceptAmbiguousExitPty: (relayPtyId) => this.acceptAmbiguousExitPty(relayPtyId), acceptExitedPty: (relayPtyId) => this.acceptExitedPty(relayPtyId) diff --git a/src/main/providers/ssh-pty-restore-required-reattach.test.ts b/src/main/providers/ssh-pty-restore-required-reattach.test.ts index 0b00c7de77d..c4063c64ef9 100644 --- a/src/main/providers/ssh-pty-restore-required-reattach.test.ts +++ b/src/main/providers/ssh-pty-restore-required-reattach.test.ts @@ -353,6 +353,42 @@ describe('SSH PTY reattach when the relay requires source restoration', () => { }) }) + it('keeps a host exit authoritative when it lands during the unproven cancellation', async () => { + const mux = createMockMux() + const activation = { + status: 'pending', + clientGeneration: 2, + ownerGeneration: 3, + ptyIncarnation: 'incarnation-reattached', + deliveryToken: 'token-first', + checkpointSourceEndSu: 0, + recoveryEndSu: 0 + } + mux.request.mockImplementation(async (method: string) => { + if (method === 'pty.cancelDelivery') { + // The owning host reports the process dead while the cancel round trip is in flight. + notificationHandler(mux)('pty.exit', { + id: 'pty-old', + code: 0, + incarnationId: 'incarnation-reattached' + }) + return { canceled: false, sentEndSu: 0, creditedEndSu: 0 } + } + if (method === 'pty.attach') { + return { ...restoreRequiredAnswer, sourceActivation: activation } + } + return undefined + }) + const provider = new SshPtyProvider('conn-1', mux as never) + provider.onExit((payload) => provider.acceptExitedPty(payload.id)) + + const message = await spawnError(provider) + + expect(message).toContain(SSH_PTY_RESTORE_REQUIRED_ERROR) + expect(provider.hasPty('ssh:conn-1@@pty-old')).toBe(false) + await expect(provider.probePtyLiveness('ssh:conn-1@@pty-old')).resolves.toBe(false) + }) + it('never reports a delivery failure as session expiry when restoration keeps failing', async () => { const mux = createMockMux() mux.request.mockResolvedValue(restoreRequiredAnswer) diff --git a/src/main/providers/ssh-pty-session-reattach.ts b/src/main/providers/ssh-pty-session-reattach.ts index 5fb8cc2460c..128392d3be4 100644 --- a/src/main/providers/ssh-pty-session-reattach.ts +++ b/src/main/providers/ssh-pty-session-reattach.ts @@ -23,6 +23,7 @@ import { type PtySourceReceivingActivation } from '../../shared/pty-source-receiving-activation' import type { SshPtyReceivingActivationLease } from './ssh-pty-notification-routing' +import type { SshPtyLiveEvidence } from './ssh-pty-liveness-state' import { parseSshPtySourceRecoveryResult } from './ssh-pty-source-recovery-result' export type SshPtyAttachResult = { @@ -231,6 +232,12 @@ export async function reattachSshPtySessionWithExitFence( export async function reattachSshPtySessionForSpawn( args: Parameters[0] & { acceptLivePty: (relayPtyId: string) => void + beginLivePtyEvidence: (appPtyId: string) => SshPtyLiveEvidence + settleLivePtyEvidence: ( + appPtyId: string, + evidence: SshPtyLiveEvidence, + acceptLive: boolean + ) => void acceptUnverifiablePty: (relayPtyId: string) => void acceptAmbiguousExitPty: (relayPtyId: string) => void acceptExitedPty: (relayPtyId: string) => void @@ -244,11 +251,18 @@ export async function reattachSshPtySessionForSpawn( const unresumable = result result = undefined if (unresumable.sourceActivationLease) { - const canceled = await unresumable.sourceActivationLease.rollback() - if (!canceled) { + // Why: the exit fence is already closed, so a host exit landing during this cancel round + // trip must be able to refuse the live promotion below instead of being overwritten. + const liveEvidence = args.beginLivePtyEvidence(unresumable.id) + let unprovenCancel = false + try { + unprovenCancel = !(await unresumable.sourceActivationLease.rollback()) + } finally { + args.settleLivePtyEvidence(unresumable.id, liveEvidence, unprovenCancel) + } + if (unprovenCancel) { // Why: the attach answered with an incarnation, so the PTY is live; only its delivery // is stuck, and an unproven cancellation just blocks the second attach. - args.acceptLivePty(unresumable.id) throw new Error( `${SSH_PTY_RESTORE_REQUIRED_ERROR}: ${toRelaySshPtyId(args.connectionId, unresumable.id)}` )