fix(ssh): a bare not-found must not retire a pane's owner

Retiring a stable pane's owner authorizes a replacement that carries the pane's
agent resume payload. Until this branch, an SSH reattach failure reached that
decision as SSH_SESSION_EXPIRED, which the gone-check did not match, so it never
fired for SSH. Retiring the not-found mapping changed that: the raw
`PTY "<id>" not found` now matches, so a replaced relay answering for a shell its
predecessor is still running would retire the owner, fabricate an exit, and
respawn with the resume payload — the double resume, reintroduced by the commit
meant to prevent it.

For an SSH pane the two proving answers are the relay's own observed exit and the
expiry the reattach mints only after verifying that proof names this shell. A
bare not-found is neither, and now propagates instead: the pane keeps its owner
and surfaces as disconnected.

Local and daemon ptys are unchanged — their provider owns its ptys, so absence
really is proof. The shutdown paths that also use the gone-check are untouched:
there, "not found" is the outcome being asked for.

The covering test drove the dangerous shape directly — bare not-found, retire,
respawn with `codex resume …`. It now drives the proof, and the bare not-found
case is pinned beside it.
This commit is contained in:
Neil
2026-08-12 17:57:10 -07:00
parent 901dd06b8d
commit e39514e9ab
2 changed files with 152 additions and 2 deletions
+135 -1
View File
@@ -10396,6 +10396,137 @@ describe('registerPtyHandlers', () => {
expect(runtime.onPtyExit).toHaveBeenCalledWith('pty-already-retired-owner', 0, undefined)
})
// Sibling of the clause above, and the reason that one is driven by a proof. Retiring an owner
// authorizes a replacement carrying the pane's agent resume payload, so a bare not-found must
// not do it: a replaced relay answers exactly that for shells its predecessor still runs.
it('does not retire an SSH owner, nor respawn it, on a bare not-found', async () => {
const connectionId = 'ssh-unproven-stable-pane'
const tabId = 'tab-unproven-ssh-owner'
const leafId = '35353535-3535-4535-8535-353535353535'
const paneKey = makePaneKey(tabId, leafId)
const worktreeId = 'repo-ssh::/remote/unproven-stable-pane'
const livePtyId = `ssh:${connectionId}@@live-relay-pty`
const freshPtyId = `ssh:${connectionId}@@fresh-relay-pty`
const remoteSpawn = vi.fn(async (options: { attachOnly?: boolean; command?: string }) => {
if (options.attachOnly) {
// The reattach mints this only after verifying the relay's observed exit names this shell.
// A bare not-found reaches here too and must NOT retire the owner — pinned by the sibling
// clause below, because retirement authorizes a replacement carrying the resume payload.
throw new Error('PTY "live-relay-pty" not found')
}
return { id: freshPtyId, incarnationId: 'inc-fresh-ssh-owner' }
})
registerSshPtyProvider(connectionId, {
spawn: remoteSpawn,
write: vi.fn(),
resize: vi.fn(),
shutdown: vi.fn(),
sendSignal: vi.fn(),
getCwd: vi.fn(),
getInitialCwd: vi.fn(),
clearBuffer: vi.fn(),
acknowledgeDataEvent: vi.fn(),
onData: vi.fn(() => () => {}),
onReplay: vi.fn(() => () => {}),
onExit: vi.fn(() => () => {}),
listProcesses: vi.fn(),
hasChildProcesses: vi.fn(),
getForegroundProcess: vi.fn(),
serialize: vi.fn(),
revive: vi.fn(),
getDefaultShell: vi.fn(),
getProfiles: vi.fn()
} as never)
let session = {
tabsByWorktree: {
[worktreeId]: [{ id: tabId, worktreeId, ptyId: livePtyId }]
},
terminalLayoutsByTabId: {
[tabId]: {
root: { type: 'leaf' as const, leafId },
activeLeafId: leafId,
expandedLeafId: null,
ptyIdsByLeafId: { [leafId]: livePtyId }
}
},
terminalPtyIncarnationsByPaneKey: { [paneKey]: 'inc-live-ssh-owner' }
}
const store = {
// STA-3077 step P inverted these: the reader/retirer used to target the
// per-target `ssh:<connectionId>` partition while the renderer published
// pane membership to `local` — two homes, so supersession no-opped. Both
// sides now address the single default (local) partition, i.e. no hostId.
getWorkspaceSession: vi.fn((requestedHostId?: string) => {
expect(requestedHostId).toBeUndefined()
return session
}),
setWorkspaceSession: vi.fn((next, requestedHostId?: string) => {
expect(requestedHostId).toBeUndefined()
session = next
}),
flushOrThrow: vi.fn(),
persistPtyBinding: vi.fn(),
upsertSshRemotePtyLease: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn()
}
const runtime = {
setPtyController: vi.fn(),
resolveTerminalPane: vi.fn(() => {
throw new Error('terminal_not_found')
}),
createPreAllocatedTerminalHandle: vi.fn(() => 'term-fresh-ssh-owner'),
registerPreAllocatedHandleForPty: vi.fn(),
beginPtyRegistration: vi.fn(),
cancelPendingPtyRegistration: vi.fn(),
assertPtyRegistrationAllowed: vi.fn(),
registerPty: vi.fn(),
noteTerminalSpawnCommand: vi.fn(),
seedHeadlessTerminal: vi.fn(),
getDriver: vi.fn(() => ({ kind: 'host' })),
onPtySpawned: vi.fn(),
onPtyExit: vi.fn(),
onPtyData: vi.fn()
}
try {
registerPtyHandlers(
mainWindow as never,
runtime as never,
undefined,
undefined,
undefined,
store as never
)
await expect(
handlers.get('pty:spawn')!(null, {
cols: 80,
rows: 24,
cwd: '/remote/unproven-stable-pane',
command: 'codex resume a-session-that-may-still-be-running',
connectionId,
worktreeId,
tabId,
leafId,
env: {
ORCA_PANE_KEY: paneKey,
ORCA_TAB_ID: tabId,
ORCA_WORKTREE_ID: worktreeId
}
})
).rejects.toThrow(/not found/i)
// Only the attach ran: no replacement carrying the resume payload.
expect(remoteSpawn).toHaveBeenCalledTimes(1)
expect(remoteSpawn.mock.calls[0]?.[0]).toMatchObject({ attachOnly: true })
// The pane keeps its owner, so the shell stays reattachable.
expect(store.setWorkspaceSession).not.toHaveBeenCalled()
expect(runtime.onPtyExit).not.toHaveBeenCalled()
} finally {
unregisterSshPtyProvider(connectionId)
}
})
it('retires a dead owner from the local session — the one pane-binding home — before fresh recovery', async () => {
const connectionId = 'ssh-dead-stable-pane'
const tabId = 'tab-dead-ssh-owner'
@@ -10406,7 +10537,10 @@ describe('registerPtyHandlers', () => {
const freshPtyId = `ssh:${connectionId}@@fresh-relay-pty`
const remoteSpawn = vi.fn(async (options: { attachOnly?: boolean; command?: string }) => {
if (options.attachOnly) {
throw new Error('PTY "dead-relay-pty" not found')
// The reattach mints this only after verifying the relay's observed exit names this shell.
// A bare not-found reaches here too and must NOT retire the owner — pinned by the sibling
// clause below, because retirement authorizes a replacement carrying the resume payload.
throw new Error('SSH_SESSION_EXPIRED: dead-relay-pty')
}
return { id: freshPtyId, incarnationId: 'inc-fresh-ssh-owner' }
})
+17 -1
View File
@@ -934,7 +934,7 @@ async function attachStablePaneOwner(
if (isDaemonEndpointGoneError(error)) {
throw new TerminalHostGoneError()
}
if (!isPtyAlreadyGoneError(error)) {
if (!isPtyProvenGoneForReplacement(error, owner.ptyId)) {
throw error
}
const ownerBeforeRetire = args.resolveOwner?.()
@@ -1100,6 +1100,22 @@ function normalizeNodePtySpawnError(err: unknown): Error {
return new Error(hintedMessage)
}
/**
* Retiring a pane's owner authorizes a replacement that carries its agent resume payload, so it
* needs proof — unlike a shutdown, where "not found" is simply the outcome we asked for. A bare
* not-found proves an exit only from a provider that owns its ptys; a replacement relay answers the
* same for shells its predecessor is still running.
*/
function isPtyProvenGoneForReplacement(err: unknown, ptyId: string): boolean {
if (!parseAppSshPtyId(ptyId)) {
return isPtyAlreadyGoneError(err)
}
// For SSH, the two proving answers are the relay's own observed exit and the expiry the reattach
// mints only after verifying that proof names this shell. A bare not-found is neither.
const message = err instanceof Error ? err.message : String(err)
return isSshPtyExitedError(err) || message.includes(SSH_SESSION_EXPIRED_ERROR)
}
function isPtyAlreadyGoneError(err: unknown): boolean {
const message = err instanceof Error ? err.message : String(err)
// A proven exit is the strongest form of "already gone"; it only reads differently because the