fix(ssh): make the pane-recovery liveness gate refuse without positive evidence of life

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.
This commit is contained in:
Neil
2026-09-02 00:17:27 -07:00
parent c3cadc51bb
commit ea079c8687
9 changed files with 219 additions and 23 deletions
@@ -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, {
+4 -1
View File
@@ -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,
@@ -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)
@@ -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')
@@ -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)
@@ -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:<ptyIdMintEpoch>:<n>` 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(
@@ -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()
})
})
@@ -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<typeof createMockDeps>
shutdown: ReturnType<typeof vi.fn>
attachForReconnect: ReturnType<typeof vi.fn>
runtime: { onPtyExit: ReturnType<typeof vi.fn>; registerPty: ReturnType<typeof vi.fn> }
}> {
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 () => {
+6
View File
@@ -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()) {