mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 08:03:12 +00:00
fix(ssh): require a host death certificate before recreating a pane, and unstick expired leases (#18013)
* fix(ssh): match an expired lease on where its leaf lives now, not its frozen tab A lease freezes tabId at write time, but detachTerminalPaneToTab moves a live pane, so the stored tab is the one the pane LEFT. getRecentExpiredSshLease required lease.tabId === tabId, which is wrong in both directions: a viewer on a stale mirror matched under the abandoned coordinates (and resolvePersistedStable PaneOwner then reads an empty layout for that tab, so adoptStablePane is skipped entirely and a fresh shell is spawned over a possibly-live one, binding the same leaf in two tabs), while a viewer using the pane's real coordinates matched nothing and got terminal_not_recoverable. Resolve the leaf's current tab the way restoreReattachedPtyRuntime already does and compare against that, falling back to the frozen tabId only when nothing can say where the leaf lives. Both workspace partitions are read because SSH spawns bind into ssh:<target> while reattach binds into local. * fix(ssh): let a proven reattach take an expired lease back to attached #17965 authorized reattach from `expired` but the state machine refused the transition back, so a lease that reattached and proved itself alive stayed `expired` forever. That silently exempted a demonstrably running remote shell from `ssh:reset` (skips `expired`), from the SSH_TERMINATE_RECONNECT_REQUIRED ownership fence in `ssh:terminateSessions` (marks it not-owned), and from the quit-time `detached` sweep, and made it permanently ineligible to win supersession so its own successors never retired their predecessors. Only the id-qualified caller carries per-pty proof: markSshRemotePtyLeases AttachedAsync is fed the relay's `attachedLeaseIds`, so an unqualified bulk mark over a whole target still cannot revive `expired`. `terminated` stays absorbing. Re-entering `attached` drops supersededBy/relayIdRecycled, since route retirement belongs to the shell that lost the pane and this one just proved it is not that shell — the same invariant upsertSshRemotePtyLease enforces. * 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. * fix(ssh): gate paired-viewer pane recovery on the narrowed session-gone predicate isSshSessionGoneError landed on the IPC transport, which never calls terminal.recoverPane. The one caller that does — recoverExpiredHostPane in the paired-viewer transport — still triggered on a bare SSH_SESSION_EXPIRED substring, so the identity-mismatch reply (the relay found a LIVE PTY under that id owned by another pane, which is evidence of presence) still asked the HUB to replace the pane, putting a second agent on one transcript. Main already refuses the respawn on that same reply; this makes the two agree. A pane whose shell genuinely died is unaffected: plain SSH_SESSION_EXPIRED still matches. The mismatch reply now surfaces as an error instead of a respawn. * test(persistence): update the reattach ratchet for expired-lease reclaim markSshRemotePtyLeasesAttachedAsync is id-qualified, so a named pty that proved itself alive now returns to attached instead of staying expired.
This commit is contained in:
Neil
@@ -1,6 +1,12 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { TEST_WINDOW_ID, TEST_WORKTREE_ID, createRuntime } from '../orca-runtime-test-fixtures.spec'
|
||||
import '../orca-runtime-test-mocks.spec'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
HEADLESS_LEAF_ID,
|
||||
TEST_WINDOW_ID,
|
||||
TEST_WORKTREE_ID,
|
||||
createRuntime,
|
||||
createRuntimeWithSshLease
|
||||
} from '../orca-runtime-test-fixtures.spec'
|
||||
import { makePaneKey } from '../orca-runtime-test-mocks.spec'
|
||||
|
||||
describe('OrcaRuntimeService', () => {
|
||||
it('invalidates a re-keyed leaf-unique handle so in-flight waiters fail fast', async () => {
|
||||
@@ -143,4 +149,76 @@ describe('OrcaRuntimeService', () => {
|
||||
await waiting
|
||||
expect(settled).toBe('rejected')
|
||||
})
|
||||
it('recovers a moved SSH pane through the tab it now sits in, not the one its lease froze', async () => {
|
||||
// `detachTerminalPaneToTab` moves a live pane and the lease keeps naming the tab it LEFT.
|
||||
// Matching on that frozen tabId refused the pane's real coordinates outright, so a moved pane
|
||||
// could only ever be "recovered" through coordinates it had already abandoned.
|
||||
const leaseTabId = 'tab-before-move'
|
||||
const currentTabId = 'tab-after-move'
|
||||
const appPtyId = 'ssh:ssh-target@@pty-8'
|
||||
const runtime = createRuntimeWithSshLease(appPtyId, leaseTabId)
|
||||
const paneKey = makePaneKey(currentTabId, HEADLESS_LEAF_ID)
|
||||
runtime.registerPty(appPtyId, TEST_WORKTREE_ID, 'ssh-target', {
|
||||
tabId: currentTabId,
|
||||
leafId: HEADLESS_LEAF_ID
|
||||
})
|
||||
const handle = runtime.resolveTerminalPane(paneKey, TEST_WORKTREE_ID).handle
|
||||
runtime.onPtyExit(appPtyId, -1, undefined, { hostExitConfirmed: true })
|
||||
const createTerminal = vi.spyOn(runtime, 'createTerminal').mockResolvedValue({
|
||||
handle: 'term-replacement',
|
||||
tabId: currentTabId,
|
||||
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).toHaveBeenCalledWith(`id:${TEST_WORKTREE_ID}`, {
|
||||
tabId: currentTabId,
|
||||
leafId: HEADLESS_LEAF_ID,
|
||||
focus: false
|
||||
})
|
||||
})
|
||||
|
||||
it('refuses a moved SSH pane addressed through the tab it left', async () => {
|
||||
// The other half: a viewer on a stale mirror asks under the OLD tab, whose layout no longer
|
||||
// holds this leaf. Accepting it binds one leaf in two tabs and leaves the PTY under the current
|
||||
// tab orphaned with its agent still running. The handle CAS happens to refuse first here, so
|
||||
// the lease resolver is asserted directly - it is the independent second refusal.
|
||||
const leaseTabId = 'tab-stale-mirror'
|
||||
const currentTabId = 'tab-moved-to'
|
||||
const appPtyId = 'ssh:ssh-target@@pty-9'
|
||||
const runtime = createRuntimeWithSshLease(appPtyId, leaseTabId)
|
||||
const stalePaneKey = makePaneKey(leaseTabId, HEADLESS_LEAF_ID)
|
||||
runtime.registerPty(appPtyId, TEST_WORKTREE_ID, 'ssh-target', {
|
||||
tabId: leaseTabId,
|
||||
leafId: HEADLESS_LEAF_ID
|
||||
})
|
||||
const staleHandle = runtime.resolveTerminalPane(stalePaneKey, TEST_WORKTREE_ID).handle
|
||||
// The move: same leaf, same PTY, new tab.
|
||||
runtime.registerPty(appPtyId, TEST_WORKTREE_ID, 'ssh-target', {
|
||||
tabId: currentTabId,
|
||||
leafId: HEADLESS_LEAF_ID
|
||||
})
|
||||
runtime.onPtyExit(appPtyId, -1, undefined, { hostExitConfirmed: true })
|
||||
const createTerminal = vi.spyOn(runtime, 'createTerminal')
|
||||
|
||||
await expect(
|
||||
runtime.recoverTerminalPane(stalePaneKey, TEST_WORKTREE_ID, staleHandle)
|
||||
).rejects.toThrow(/terminal_not_recoverable|terminal_not_found/)
|
||||
const leases = runtime as unknown as {
|
||||
getRecentExpiredSshLease: (worktreeId: string, tabId: string, leafId?: string) => unknown
|
||||
}
|
||||
expect(
|
||||
leases.getRecentExpiredSshLease(TEST_WORKTREE_ID, leaseTabId, HEADLESS_LEAF_ID)
|
||||
).toBeNull()
|
||||
expect(
|
||||
leases.getRecentExpiredSshLease(TEST_WORKTREE_ID, currentTabId, HEADLESS_LEAF_ID)
|
||||
).not.toBeNull()
|
||||
expect(createTerminal).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user