From ea079c868759ce3f6212fa5e020b60b3e4bdab27 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:30:11 -0700 Subject: [PATCH] fix(ssh): make the pane-recovery liveness gate refuse without positive evidence of life MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate refused only `live` and `unverifiable` and passed on `null` — but the register is an in-memory Map, so `null` is equally what a fresh app start, a never-asked host and a certified death look like. Absence of evidence was reading as authorization to spawn a shell over a possibly-live remote process: `!pty.connected` is cleared for every PTY a dropped relay owned, and `expired` only ever says the CLIENT lost its route. - `exited` is now RETAINED rather than deleted, so the register is three-valued in the map as well as in the type. Its one writer is a host-delivered exit frame — an exit with a real code, or an explicit `hostExitConfirmed` — which records the certificate instead of merely dropping the doubt. - `recoverTerminalPane` refuses on `live` and `unverifiable`, and deliberately does NOT demand a positive `exited`. The only answer that ever reaches this gate is a reachable relay reporting no such id, and that is a union: pty.attach throws not-found for an unknown id with no liveness check, and a relay restart makes every previously minted id unknown (ids carry a per-start `ptyIdMintEpoch`). No writer of `exited` co-occurs with a reattachable `expired` lease either — a host-delivered exit frame tombstones the lease `terminated` — so requiring one would close the gate permanently. - `handlePtyReattachFailure`'s not-found branch publishes `code: -1` to the renderer and does not call `runtime.onPtyExit`. The relay's not-found answer is not a death certificate, and #17963's ratchet on the same branch pins that. - The inventory's `observed === false` hunk keeps dropping doubt rather than asserting a death: `pty.listProcesses` returns the relay's CURRENT session map, so a restarted relay omits every previously minted id whether or not those shells died — the same union, one hop away. A live or unprovable pane refuses; a disowned one still recovers. No wire change. The gate's ratchets live in terminal-pane-recovery-liveness-gate.test.ts: config/vitest.config.ts — the config CI runs — matches only `*.test.ts`, so cases placed under orca-runtime-tests/*.spec.ts would never execute. --- ...-runtime-mark-pty-liveness-unverifiable.ts | 16 ++- src/main/runtime/orca-runtime-on-pty-exit.ts | 5 +- ...ktree-records-with-controller-inventory.ts | 4 + .../orca-runtime-resolve-terminal-pane.ts | 11 +- .../terminal-handles.spec.ts | 13 +- .../pty-inventory-liveness-verdict.test.ts | 21 ++- ...rminal-pane-recovery-liveness-gate.test.ts | 128 ++++++++++++++++++ .../ssh-relay-orphan-abandon-paths.test.ts | 38 ++++-- src/main/ssh/ssh-relay-session.ts | 6 + 9 files changed, 219 insertions(+), 23 deletions(-) create mode 100644 src/main/runtime/terminal-pane-recovery-liveness-gate.test.ts diff --git a/src/main/runtime/orca-runtime-mark-pty-liveness-unverifiable.ts b/src/main/runtime/orca-runtime-mark-pty-liveness-unverifiable.ts index 50fae68e6da..548c2e4a0bc 100644 --- a/src/main/runtime/orca-runtime-mark-pty-liveness-unverifiable.ts +++ b/src/main/runtime/orca-runtime-mark-pty-liveness-unverifiable.ts @@ -39,7 +39,13 @@ export class OrcaRuntimeWithMarkPtyLivenessUnverifiable extends OrcaRuntimeWithO return this.stopRequestedPtyIds.has(ptyId) } - /** Null when nothing has been observed either way, so callers keep their own default. */ + /** + * Null when this register holds no defensible claim — a never-asked host, a fresh app start, or + * an absence observation too weak to name (a relay that answered but does not know the id: see + * the inventory sweep and handlePtyReattachFailure). It is NOT a death certificate, so a caller + * authorizing a kill must fail closed on it; a caller that only has this evidence to work with, + * like terminal.recoverPane, refuses on the positive verdicts instead. + */ getPtyLivenessVerdict(ptyId: string): PtyLivenessVerdict | null { return this.ptyLivenessVerdictByPtyId.get(ptyId)?.verdict ?? null } @@ -92,11 +98,9 @@ export class OrcaRuntimeWithMarkPtyLivenessUnverifiable extends OrcaRuntimeWithO } protected rememberPtyLivenessVerdict(ptyId: string, verdict: PtyLivenessVerdict): void { - if (verdict.status === 'exited') { - // An earned death certificate ends the question; nothing left to remember. - this.ptyLivenessVerdictByPtyId.delete(ptyId) - return - } + // An earned death certificate is KEPT, not dropped, so the register is three-valued on disk as + // well as in the type. Its only writer is a host-delivered exit frame; nothing weaker may + // reach it (docs/reference/ssh-execution-boundary.md). this.ptyLivenessVerdictByPtyId.delete(ptyId) this.ptyLivenessObservationSequence += 1 this.ptyLivenessVerdictByPtyId.set(ptyId, { diff --git a/src/main/runtime/orca-runtime-on-pty-exit.ts b/src/main/runtime/orca-runtime-on-pty-exit.ts index 07f8d4fbbd4..58fc9d6a1e7 100644 --- a/src/main/runtime/orca-runtime-on-pty-exit.ts +++ b/src/main/runtime/orca-runtime-on-pty-exit.ts @@ -202,7 +202,10 @@ export class OrcaRuntimeWithOnPtyExit extends OrcaRuntimeWithOnClientDisconnecte pty.lastExitCode = exitCode pty.lastExitCause = exitCause if (exitCode >= 0 || options.hostExitConfirmed === true) { - this.forgetPtyLivenessVerdict(ptyId) + // Record the certificate rather than merely dropping the doubt: a reader that has to + // authorize a respawn cannot distinguish "the host reported this process gone" from "this + // runtime has never asked" if both are absence. + this.rememberPtyLivenessVerdict(ptyId, { status: 'exited' }) } // Why: the exited process's live frames say nothing about a replacement. // A same-id respawn makes the leaf writable again before any new title, diff --git a/src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts b/src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts index 3e3114d4b7a..5b2dcd71f19 100644 --- a/src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts +++ b/src/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts @@ -287,6 +287,10 @@ export class OrcaRuntimeWithRefreshPtyWorktreeRecordsWithControllerInventory ext // clears `connected` for every one of its PTYs at once. Only `false` here // is an observed absence; `null` means no provider could be asked. if (observed === false) { + // Drops the doubt without asserting a death: `pty.listProcesses` returns the relay's + // CURRENT session map, so a restarted relay omits every id the previous one minted + // whether or not those shells died. That is the same union as pty.attach's not-found, + // and neither earns `exited` (docs/reference/ssh-execution-boundary.md). this.forgetPtyLivenessVerdict(pty.ptyId) } else if (observed === null && this.isSshOwnedPtyId(pty.ptyId)) { this.markPtyLivenessUnverifiable(pty.ptyId, NO_OBSERVING_PROVIDER_REASON) diff --git a/src/main/runtime/orca-runtime-resolve-terminal-pane.ts b/src/main/runtime/orca-runtime-resolve-terminal-pane.ts index 6ac8fb54975..6d0dd931b77 100644 --- a/src/main/runtime/orca-runtime-resolve-terminal-pane.ts +++ b/src/main/runtime/orca-runtime-resolve-terminal-pane.ts @@ -101,8 +101,15 @@ export class OrcaRuntimeWithResolveTerminalPane extends OrcaRuntimeWithGetTermin // above). `!pty.connected` is the same inference: one dropped relay clears it for every PTY it // owned. So the pair can hold over a remote shell that is still running, and createTerminal // would rebind the pane away from it, leaving the original orphaned and its agent duplicated. - // The runtime already grades this: a host-confirmed exit leaves no verdict, while lost contact - // is recorded `unverifiable` (docs/reference/ssh-execution-boundary.md, shared/pty-liveness-verdict.ts). + // The runtime grades that: `live` is the host proving the shell survived, `unverifiable` is the + // client losing contact, and both refuse. What this gate must NOT require is a positive + // `exited`: the only answer that ever reaches it is a reachable relay reporting it has no such + // id, and that is a union — pty.attach throws not-found for an unknown id with no liveness + // check, and a relay restart makes every previously minted id unknown. No writer of + // `exited` co-occurs with a reattachable `expired` lease either, since a host-delivered exit + // frame tombstones the lease `terminated`. Demanding one would close this gate permanently, and + // an unrecoverable pane is its own failure (docs/reference/ssh-execution-boundary.md, + // shared/pty-liveness-verdict.ts). const liveness = this.getPtyLivenessVerdict(pty.ptyId) if (liveness?.status === 'unverifiable' || liveness?.status === 'live') { throw new Error('terminal_not_recoverable') diff --git a/src/main/runtime/orca-runtime-tests/terminal-handles.spec.ts b/src/main/runtime/orca-runtime-tests/terminal-handles.spec.ts index 359abb07f63..4ae933afef4 100644 --- a/src/main/runtime/orca-runtime-tests/terminal-handles.spec.ts +++ b/src/main/runtime/orca-runtime-tests/terminal-handles.spec.ts @@ -624,11 +624,14 @@ describe('OrcaRuntimeService', () => { }) it('does not recreate a shell for an expired lease whose PTY liveness is unverifiable', async () => { - // The production sequence this guards: a relay reattach fails, so ssh-relay-session marks the - // lease 'expired' AND sends a synthetic pty:exit code -1. Neither observed the process — every - // writer of 'expired' documents it as "the client lost its route", and code -1 with no host - // confirmation is recorded 'unverifiable'. Spawning a replacement there rebinds the pane away - // from a remote shell still running on the host and duplicates its agent. + // The production sequence this guards: the relay delivers an exit frame carrying code -1 for an + // SSH pane with no host confirmation (`preservesAbnormalSshSurface`), while the pane's lease is + // already 'expired'. Neither observed the process — every writer of 'expired' documents it as + // "the client lost its route", and code -1 with no host confirmation is recorded + // 'unverifiable'. Spawning a replacement there rebinds the pane away from a remote shell still + // running on the host and duplicates its agent. This is the `unverifiable` arm only; the + // relay's own absence branch (`handlePtyReattachFailure`) reaches the runtime with no verdict + // at all, and is covered by the relay-disowned case in terminal-handles-part-02.spec.ts. const tabId = 'tab-unverifiable' const ptyId = 'ssh:ssh-target@@pty-3' const runtime = createRuntimeWithSshLease(ptyId, tabId) diff --git a/src/main/runtime/pty-inventory-liveness-verdict.test.ts b/src/main/runtime/pty-inventory-liveness-verdict.test.ts index 38e3dda20eb..e5c31f66afa 100644 --- a/src/main/runtime/pty-inventory-liveness-verdict.test.ts +++ b/src/main/runtime/pty-inventory-liveness-verdict.test.ts @@ -89,7 +89,9 @@ describe('inventory sweep liveness verdicts', () => { runtime.onPtyExit(REMOTE_PTY_ID, -1, undefined, { hostExitConfirmed: true }) - expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toBeNull() + // A host-delivered exit frame is the one signal that observes the process, so it both clears + // the lost-contact doubt and is retained as the certificate itself. + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toEqual({ status: 'exited' }) }) it('records lost contact when no provider can answer for the PTY', async () => { @@ -112,6 +114,23 @@ describe('inventory sweep liveness verdicts', () => { expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toBeNull() }) + it('records no death certificate when a listing of the owning host omits the PTY', async () => { + // The host answered and named a sibling on the same relay, so this is the strongest absence the + // inventory can report — and it is still not a certificate. `pty.listProcesses` returns the + // relay's CURRENT session map, so a relay that restarted omits every id the previous one minted + // (ids are `pty2::` with a fresh epoch per relay start) whether or not those + // shells ever died. Recording `exited` here would only relocate the fabrication that + // handlePtyReattachFailure was corrected for (docs/reference/ssh-execution-boundary.md). + const runtime = makeRuntimeMissingFromInventory( + () => false, + vi.fn(async () => [{ id: 'ssh:conn-1@@relay-sibling', worktreeId: WORKTREE_ID }]) + ) + + await runtime.listTerminals(`id:${WORKTREE_ID}`) + + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toBeNull() + }) + it('clears lost-contact doubt when reconnect inventory observes the PTY live', async () => { let reconnected = false const runtime = makeRuntimeMissingFromInventory( diff --git a/src/main/runtime/terminal-pane-recovery-liveness-gate.test.ts b/src/main/runtime/terminal-pane-recovery-liveness-gate.test.ts new file mode 100644 index 00000000000..441d04a960b --- /dev/null +++ b/src/main/runtime/terminal-pane-recovery-liveness-gate.test.ts @@ -0,0 +1,128 @@ +import { describe, expect, it, vi } from 'vitest' +import { + HEADLESS_LEAF_ID, + TEST_WORKTREE_ID, + createRuntimeWithSshLease +} from './orca-runtime-test-fixtures.spec' +import { makePaneKey } from './orca-runtime-test-mocks.spec' + +// What `terminal.recoverPane` may and may not treat as authority to spawn a replacement shell over +// a remote pane. Lives in a `.test.ts` rather than beside the other recoverPane cases in +// orca-runtime-tests/*.spec.ts because config/vitest.config.ts — the config CI runs — includes only +// `*.test.ts`, so a ratchet placed there would never execute. + +describe('terminal.recoverPane liveness gate', () => { + it('recreates a shell for an SSH pane the relay disowned, the only evidence this gate ever gets', async () => { + // The whole production route into recoverPane, end to end. A reachable relay answered for this + // exact id and did not name it — via pty.attach in handlePtyReattachFailure, or the identical + // answer in the inventory listing below — and that answer is a union: pty.attach throws + // not-found for an unknown id with no liveness check, and pty.listProcesses returns only + // the CURRENT session map, which after a relay restart omits every previously minted id. So no + // `exited` certificate exists to demand here, and no writer of one co-occurs with a + // reattachable `expired` lease: a host-delivered exit frame tombstones the lease `terminated` + // instead. Requiring `exited` therefore closes this gate permanently + // (docs/reference/ssh-execution-boundary.md). + const tabId = 'tab-relay-disowned' + const ptyId = 'ssh:ssh-target@@pty-10' + const runtime = createRuntimeWithSshLease(ptyId, tabId) + const paneKey = makePaneKey(tabId, HEADLESS_LEAF_ID) + runtime.setPtyController({ + write: () => true, + kill: () => true, + hasPty: (id: string) => id !== ptyId, + // A sibling on the same relay still reports, so the host itself answered this listing. + listProcesses: async () => [ + { id: 'ssh:ssh-target@@pty-sibling', worktreeId: TEST_WORKTREE_ID } + ], + getForegroundProcess: async () => null + } as never) + runtime.registerPty(ptyId, TEST_WORKTREE_ID, 'ssh-target', { tabId, leafId: HEADLESS_LEAF_ID }) + const handle = runtime.resolveTerminalPane(paneKey, TEST_WORKTREE_ID).handle + // The sweep is what disconnects the pane: handlePtyReattachFailure tells only the renderer and + // the reattach spawn path only expires the lease, so nothing else reaches the runtime record. + await runtime.listTerminals(`id:${TEST_WORKTREE_ID}`) + expect(runtime.getPtyLivenessVerdict(ptyId)).toBeNull() + const createTerminal = vi.spyOn(runtime, 'createTerminal').mockResolvedValue({ + handle: 'term-replacement', + tabId, + paneKey, + ptyId: 'pty-replacement', + worktreeId: TEST_WORKTREE_ID, + title: null, + surface: 'background' + }) + + await expect( + runtime.recoverTerminalPane(paneKey, TEST_WORKTREE_ID, handle) + ).resolves.toMatchObject({ handle: 'term-replacement' }) + expect(createTerminal).toHaveBeenCalledOnce() + }) + + it('refuses to recreate a shell for an expired lease the host proved is still live', async () => { + // The production writer: reattach SUCCEEDED and then persistPtyBinding refused the surface, so + // ssh-relay-session records `live` before writing `expired`. `expired` alone would read as + // "reattach gave up", and spawning here would put a second agent on a transcript the host just + // proved is still running. + const tabId = 'tab-proved-live' + const ptyId = 'ssh:ssh-target@@pty-11' + const runtime = createRuntimeWithSshLease(ptyId, tabId) + const paneKey = makePaneKey(tabId, HEADLESS_LEAF_ID) + runtime.registerPty(ptyId, TEST_WORKTREE_ID, 'ssh-target', { tabId, leafId: HEADLESS_LEAF_ID }) + const handle = runtime.resolveTerminalPane(paneKey, TEST_WORKTREE_ID).handle + runtime.onPtyExit(ptyId, -1) + runtime.markPtyLivenessLive(ptyId) + const createTerminal = vi.spyOn(runtime, 'createTerminal') + + await expect(runtime.recoverTerminalPane(paneKey, TEST_WORKTREE_ID, handle)).rejects.toThrow( + 'terminal_not_recoverable' + ) + expect(createTerminal).not.toHaveBeenCalled() + }) + + it('refuses to recreate a shell for an expired lease whose PTY liveness is unverifiable', async () => { + // The relay delivers an exit frame carrying code -1 for an SSH pane with no host confirmation + // (`preservesAbnormalSshSurface`) while the lease is already 'expired'. Neither observed the + // process, and spawning here rebinds the pane away from a shell still running on the host. + const tabId = 'tab-unverifiable-gate' + const ptyId = 'ssh:ssh-target@@pty-12' + const runtime = createRuntimeWithSshLease(ptyId, tabId) + const paneKey = makePaneKey(tabId, HEADLESS_LEAF_ID) + runtime.registerPty(ptyId, TEST_WORKTREE_ID, 'ssh-target', { tabId, leafId: HEADLESS_LEAF_ID }) + const handle = runtime.resolveTerminalPane(paneKey, TEST_WORKTREE_ID).handle + runtime.onPtyExit(ptyId, -1) + expect(runtime.getPtyLivenessVerdict(ptyId)?.status).toBe('unverifiable') + const createTerminal = vi.spyOn(runtime, 'createTerminal') + + await expect(runtime.recoverTerminalPane(paneKey, TEST_WORKTREE_ID, handle)).rejects.toThrow( + 'terminal_not_recoverable' + ) + expect(createTerminal).not.toHaveBeenCalled() + }) + + it('still recreates a shell for an SSH pane whose host attested the exit', async () => { + // The negative control on the other side: a host-delivered exit frame is a real certificate, so + // the pane a paired client asks about must still get a replacement. + const tabId = 'tab-attested-gate' + const ptyId = 'ssh:ssh-target@@pty-13' + const runtime = createRuntimeWithSshLease(ptyId, tabId) + const paneKey = makePaneKey(tabId, HEADLESS_LEAF_ID) + runtime.registerPty(ptyId, TEST_WORKTREE_ID, 'ssh-target', { tabId, leafId: HEADLESS_LEAF_ID }) + const handle = runtime.resolveTerminalPane(paneKey, TEST_WORKTREE_ID).handle + runtime.onPtyExit(ptyId, -1, undefined, { hostExitConfirmed: true }) + expect(runtime.getPtyLivenessVerdict(ptyId)?.status).toBe('exited') + const createTerminal = vi.spyOn(runtime, 'createTerminal').mockResolvedValue({ + handle: 'term-replacement', + tabId, + paneKey, + ptyId: 'pty-replacement', + worktreeId: TEST_WORKTREE_ID, + title: null, + surface: 'background' + }) + + await expect( + runtime.recoverTerminalPane(paneKey, TEST_WORKTREE_ID, handle) + ).resolves.toMatchObject({ handle: 'term-replacement' }) + expect(createTerminal).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/ssh/ssh-relay-orphan-abandon-paths.test.ts b/src/main/ssh/ssh-relay-orphan-abandon-paths.test.ts index 9d08ac7da0f..6d1e5b6c67d 100644 --- a/src/main/ssh/ssh-relay-orphan-abandon-paths.test.ts +++ b/src/main/ssh/ssh-relay-orphan-abandon-paths.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { SshRelaySession } from './ssh-relay-session' import { createMockDeps, mockDeploySuccess } from './ssh-relay-session-test-fixtures' +import { isProvenProcessExit } from '../../shared/terminal-exit-cause' const { muxRequestMock, openConsumerSessionMock } = vi.hoisted(() => ({ muxRequestMock: vi.fn(), @@ -127,6 +128,7 @@ describe('SshRelaySession abandoned remote PTYs', () => { deps: ReturnType shutdown: ReturnType attachForReconnect: ReturnType + runtime: { onPtyExit: ReturnType; registerPty: ReturnType } }> { const deps = createMockDeps() const shutdown = vi.fn().mockResolvedValue(undefined) @@ -139,27 +141,30 @@ describe('SshRelaySession abandoned remote PTYs', () => { vi.mocked(deps.mockStore.getSshRemotePtyLeases).mockReturnValue([detachedLease()] as ReturnType< typeof deps.mockStore.getSshRemotePtyLeases >) + const runtime = { onPtyExit: vi.fn(), registerPty: vi.fn() } const session = new SshRelaySession( 'target-1', deps.getMainWindow, deps.mockStore, - deps.mockPortForward + deps.mockPortForward, + runtime as never ) await session.establish(deps.mockConn) - return { deps, shutdown, attachForReconnect } + return { deps, shutdown, attachForReconnect, runtime } } it('leaves a live shell running and reachable when reattach attempts are exhausted', async () => { // A transport stall proves nothing about the remote shell; the user's long-running process // may be untouched, so the lease must stay terminable rather than be tombstoned as expired. - const { deps, shutdown, attachForReconnect } = await establishWithFailingReattach( + const { deps, shutdown, attachForReconnect, runtime } = await establishWithFailingReattach( new Error('PTY reattach attempt timed out after 15000ms') ) expect(attachForReconnect).toHaveBeenCalled() expect(shutdown).not.toHaveBeenCalled() + expect(runtime.onPtyExit).not.toHaveBeenCalled() expect(deps.mockStore.markSshRemotePtyLease).not.toHaveBeenCalledWith( 'target-1', 'pty-live', @@ -172,12 +177,13 @@ describe('SshRelaySession abandoned remote PTYs', () => { it('leaves another pane live shell running when the relay reports an identity mismatch', async () => { // The relay answered that a *live* PTY holds this id under a different pane identity. Killing // it would destroy an unrelated terminal, so this path may only stop claiming the id. - const { deps, shutdown, attachForReconnect } = await establishWithFailingReattach( + const { deps, shutdown, attachForReconnect, runtime } = await establishWithFailingReattach( new Error('PTY "pty-live" not found (identity mismatch)') ) expect(attachForReconnect).toHaveBeenCalled() expect(shutdown).not.toHaveBeenCalled() + expect(runtime.onPtyExit).not.toHaveBeenCalled() expect(deps.mockStore.markSshRemotePtyLease).not.toHaveBeenCalledWith( 'target-1', 'pty-live', @@ -186,14 +192,21 @@ describe('SshRelaySession abandoned remote PTYs', () => { expect(clearProviderPtyState).not.toHaveBeenCalledWith(APP_PTY_ID) }) - it('retires the lease without a kill when the relay proves the PTY is gone', async () => { - // pty.attach verifies process liveness before answering not-found, so this is the one branch - // with positive proof of death — and a dead process needs no shutdown request. - const { deps, shutdown } = await establishWithFailingReattach( + it('stops claiming the id without asserting an exit when the relay answers not-found', async () => { + // pty.attach answers not-found both when it verified the pid is dead AND when its session map + // simply has no such id — which is every id after a relay restart, since ids are minted from a + // fresh per-start `ptyIdMintEpoch`. The client cannot tell those apart, so this branch may + // release the id but must not certify a death: the exit it publishes carries the + // unverified-loss sentinel, never a status the renderer would read as a real exit. + const { deps, shutdown, runtime } = await establishWithFailingReattach( new Error('PTY "pty-live" not found') ) expect(shutdown).not.toHaveBeenCalled() + // The runtime is where a death certificate would land (`hostExitConfirmed`), and this branch + // holds no evidence to write one from. + expect(runtime.onPtyExit).not.toHaveBeenCalled() + // 'expired' records that reattach gave up on the id, not that the shell died; ssh:terminateSessions still reaches it. expect(deps.mockStore.markSshRemotePtyLease).toHaveBeenCalledWith( 'target-1', 'pty-live', @@ -204,6 +217,15 @@ describe('SshRelaySession abandoned remote PTYs', () => { id: APP_PTY_ID, code: -1 }) + const exitCall = vi + .mocked(deps.mockWindow.webContents.send) + .mock.calls.find(([channel]) => channel === 'pty:exit') + if (!exitCall) { + throw new Error('expected a pty:exit publication') + } + // The ratchet that makes the above safe: swapping -1 for any provable status would turn an + // unreachable relay into a death certificate, closing tabs and dropping leaf↔PTY bindings. + expect(isProvenProcessExit((exitCall[1] as { code: number }).code)).toBe(false) }) it('keeps a recovered session attached when reattach succeeds after an earlier drop', async () => { diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index ed579501b21..54fade04651 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -2836,6 +2836,12 @@ export class SshRelaySession { ) clearProviderPtyState(appPtyId) deletePtyOwnership(appPtyId) + // Deliberately does NOT call runtime.onPtyExit: pty.attach answers not-found both when it + // verified the pid is dead and when its session map simply has no such id (no liveness check on + // that path at all) — which is every id after a relay restart, since ids carry a per-start + // `ptyIdMintEpoch`. This branch may release the id, but certifying a death from that union + // would orphan a live remote shell (docs/reference/ssh-execution-boundary.md). The renderer + // gets code -1, which every reader treats as unverified loss. this.store.markSshRemotePtyLease(this.targetId, ptyId, 'expired') const win = this.getMainWindow() if (win && !win.isDestroyed()) {