fix(ssh): stop reporting live relay PTYs as expired sessions

A `pty.attach` reply carrying `sourceRecovery: restoreRequired` is the relay
answering for a PTY it just found in its pool and proved alive with
`isProcessAlive`; only the stale output delivery was retired. Main converted
that into `SSH_SESSION_EXPIRED`, which is the token every caller uses to retire
the pane binding and cold-restore the agent, so a transient reconnect started a
second `claude --resume` over a running one's transcript and left the previous
remote PTY detached — one more per reconnect until the host refused to fork.

Retry the attach once (the relay retires the stale delivery as it answers, so
the next attach opens a fresh one with full replay), then fail with a
restore-required verdict that makes no claim about absence. Callers already
route anything short of absence to the unverifiable pane-recovery path.

Also tighten the renderer's expiry verdict, which was a bare substring test: an
identity mismatch names a LIVE PTY owned by another pane and observes nothing
about this one, and main's own gate already refuses to respawn on it.

Refs #11006, #9034
This commit is contained in:
Neil
2026-09-02 20:36:16 -07:00
committed by Neil
parent 21210aad34
commit cd0622baae
11 changed files with 240 additions and 42 deletions
+8
View File
@@ -1,5 +1,13 @@
export const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED'
export const SSH_PTY_IDENTITY_MISMATCH_ERROR = 'SSH_PTY_IDENTITY_MISMATCH'
/**
* The relay accepted the attach for a PTY it had just proven alive and only retired the stale
* output delivery. Deliberately not `SSH_SESSION_EXPIRED`: every consumer of that token retires the
* pane binding and cold-restores the agent, which duplicates a running agent onto one transcript
* (docs/reference/ssh-execution-boundary.md — respawning needs host evidence of absence, and this
* reply is host evidence of the opposite).
*/
export const SSH_PTY_SOURCE_RESTORE_REQUIRED_ERROR = 'SSH_PTY_SOURCE_RESTORE_REQUIRED'
export function isSshPtyNotFoundError(error: unknown): boolean {
const message = error instanceof Error ? error.message : String(error)
@@ -0,0 +1,104 @@
import { describe, expect, it, vi } from 'vitest'
import {
SSH_PTY_SOURCE_RESTORE_REQUIRED_ERROR,
SSH_SESSION_EXPIRED_ERROR,
isSshPtyAbsentFromRelayError
} from './ssh-pty-errors'
import { SshPtyProvider } from './ssh-pty-provider'
const RESTORE_REQUIRED = {
incarnationId: 'incarnation-1',
sourceRecovery: { status: 'restoreRequired', reason: 'checkpointUnavailable' }
}
function providerWithAttachReplies(replies: unknown[]): {
provider: SshPtyProvider
request: ReturnType<typeof vi.fn>
} {
const request = vi.fn()
for (const reply of replies) {
request.mockResolvedValueOnce(reply)
}
const mux = {
request,
notify: vi.fn(),
onNotification: vi.fn().mockReturnValue(vi.fn())
}
return { provider: new SshPtyProvider('conn-1', mux as never), request }
}
describe('a live PTY whose source delivery needs restoring', () => {
// The relay only reaches a restoreRequired reply after finding the managed PTY and confirming its
// process is alive, so the reply is evidence of liveness. `SSH_SESSION_EXPIRED` is the token every
// caller uses to retire the pane binding and cold-restore the agent — emitting it here put a
// second `claude --resume` onto the transcript of a still-running one (#11006), and leaked the
// abandoned remote PTY on every reconnect until the host refused to fork (#9034).
it('does not claim the session expired after the retry still needs a restore', async () => {
const { provider, request } = providerWithAttachReplies([RESTORE_REQUIRED, RESTORE_REQUIRED])
const rejection = await provider.spawn({ cols: 80, rows: 24, sessionId: 'pty-1' }).then(
() => undefined,
(error: unknown) => error
)
expect((rejection as Error).message).not.toContain(SSH_SESSION_EXPIRED_ERROR)
expect((rejection as Error).message).toContain(SSH_PTY_SOURCE_RESTORE_REQUIRED_ERROR)
expect(isSshPtyAbsentFromRelayError(rejection)).toBe(false)
expect(request).toHaveBeenCalledTimes(2)
})
it('reattaches the live PTY when the retry opens a fresh delivery', async () => {
const { provider, request } = providerWithAttachReplies([
RESTORE_REQUIRED,
{ incarnationId: 'incarnation-1', replay: 'restored scrollback' }
])
const result = await provider.spawn({ cols: 80, rows: 24, sessionId: 'pty-1' })
expect(result).toMatchObject({
id: 'ssh:conn-1@@pty-1',
isReattach: true,
replay: 'restored scrollback'
})
expect(request).toHaveBeenCalledTimes(2)
})
it('stops retrying rather than stacking a delivery on an unconfirmed cancellation', async () => {
const request = vi.fn().mockResolvedValue({
...RESTORE_REQUIRED,
sourceActivation: {
status: 'pending',
clientGeneration: 1,
ownerGeneration: 1,
ptyIncarnation: 'incarnation-1',
deliveryToken: 'token-1',
checkpointSourceEndSu: 0,
recoveryEndSu: 0
}
})
const rollback = vi.fn().mockResolvedValue(false)
const mux = {
request,
notify: vi.fn(),
onNotification: vi.fn().mockReturnValue(vi.fn())
}
const provider = new SshPtyProvider('conn-1', mux as never)
const outputState = (provider as unknown as { outputState: Record<string, unknown> })
.outputState
outputState.installReceivingActivation = () => ({
commit: vi.fn(),
rollback,
transferToRecovery: vi.fn()
})
const rejection = await provider.spawn({ cols: 80, rows: 24, sessionId: 'pty-1' }).then(
() => undefined,
(error: unknown) => error
)
expect((rejection as Error).message).toContain(SSH_PTY_SOURCE_RESTORE_REQUIRED_ERROR)
expect((rejection as Error).message).not.toContain(SSH_SESSION_EXPIRED_ERROR)
expect(request).toHaveBeenCalledTimes(1)
expect(rollback).toHaveBeenCalledTimes(1)
})
})
@@ -1,5 +1,5 @@
import { describe, expect, it, vi } from 'vitest'
import { SSH_SESSION_EXPIRED_ERROR } from './ssh-pty-errors'
import { SSH_PTY_SOURCE_RESTORE_REQUIRED_ERROR } from './ssh-pty-errors'
import { SshPtyProvider } from './ssh-pty-provider'
describe('SSH PTY provider session reattach incarnation', () => {
@@ -30,7 +30,7 @@ describe('SSH PTY provider session reattach incarnation', () => {
)
})
it('fails closed when generic reattach requires source restoration', async () => {
it('fails closed without claiming expiry when reattach requires source restoration', async () => {
const mux = {
request: vi.fn().mockResolvedValue({
incarnationId: 'incarnation-reattached',
@@ -44,8 +44,10 @@ describe('SSH PTY provider session reattach incarnation', () => {
}
const provider = new SshPtyProvider('conn-1', mux as never)
// The relay proved the PTY alive before answering restoreRequired, so the rejection must not
// carry the token that makes callers retire the binding and cold-restore the agent.
await expect(provider.spawn({ cols: 80, rows: 24, sessionId: 'pty-old' })).rejects.toThrow(
`${SSH_SESSION_EXPIRED_ERROR}: pty-old`
new RegExp(`^${SSH_PTY_SOURCE_RESTORE_REQUIRED_ERROR}: pty-old`)
)
})
})
@@ -1,6 +1,7 @@
import { describe, expect, it, vi } from 'vitest'
import {
SSH_PTY_IDENTITY_MISMATCH_ERROR,
SSH_PTY_SOURCE_RESTORE_REQUIRED_ERROR,
SSH_SESSION_EXPIRED_ERROR,
isSshPtyAbsentFromRelayError
} from './ssh-pty-errors'
@@ -73,6 +74,8 @@ describe('SSH PTY relay absence verdict', () => {
)
expect(isSshPtyAbsentFromRelayError(rejection)).toBe(false)
expect((rejection as Error).message).toContain(SSH_SESSION_EXPIRED_ERROR)
// Nor may it wear the expiry token: that is what the renderer retires a pane binding on.
expect((rejection as Error).message).not.toContain(SSH_SESSION_EXPIRED_ERROR)
expect((rejection as Error).message).toContain(SSH_PTY_SOURCE_RESTORE_REQUIRED_ERROR)
})
})
+43 -20
View File
@@ -2,6 +2,7 @@ import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer'
import { isPtyIncarnationId, type PtyIncarnationId } from '../../shared/pty-incarnation'
import {
SSH_PTY_IDENTITY_MISMATCH_ERROR,
SSH_PTY_SOURCE_RESTORE_REQUIRED_ERROR,
SSH_SESSION_EXPIRED_ERROR,
SshPtyAbsentFromRelayError,
isSshPtyIdentityMismatchError,
@@ -172,6 +173,8 @@ function sameSourceActivation(
export type { PtySourceRecoveryRequest }
const RESTORE_REQUIRED_ATTACH_ATTEMPTS = 2
export async function reattachSshPtySession(args: {
mux: SshChannelMultiplexer
connectionId: string
@@ -270,8 +273,8 @@ export async function reattachSshPtySessionWithExitFence(
/**
* The full reattach path a spawn takes when it carries a sessionId: fence the
* exit race, reject a session the relay can no longer restore, and commit or
* roll back the source-activation lease.
* exit race, reopen a delivery the relay retired, and commit or roll back the
* source-activation lease.
*
* Lives here rather than in SshPtyProvider.spawn so the lease's commit and
* rollback stay in one place — a caller that only wrapped the fence could
@@ -282,24 +285,44 @@ export async function reattachSshPtySessionForSpawn(
acceptLivePty: (relayPtyId: string) => void
}
): Promise<PtySpawnResult> {
let result: SshPtyReattachResult | undefined
try {
result = await reattachSshPtySessionWithExitFence(args)
if (result.sourceRecovery?.status === 'restoreRequired') {
throw new Error(
`${SSH_SESSION_EXPIRED_ERROR}: ${toRelaySshPtyId(args.connectionId, result.id)}`
)
let restoreRequiredReason = 'unknown'
// The relay retires the stale delivery as it answers restoreRequired, so the next attach opens a
// fresh one with full replay. One immediate retry keeps the ordinary reconnect off the renderer's
// 15s-cooldown pane-recovery ladder.
for (let attempt = 0; attempt < RESTORE_REQUIRED_ATTACH_ATTEMPTS; attempt++) {
let result: SshPtyReattachResult | undefined
try {
result = await reattachSshPtySessionWithExitFence(args)
if (result.sourceRecovery?.status === 'restoreRequired') {
restoreRequiredReason = result.sourceRecovery.reason
const lease = result.sourceActivationLease
// An unconfirmed cancellation must not stack a second delivery on the first.
if (lease && !(await lease.rollback())) {
break
}
continue
}
args.acceptLivePty(result.id)
result.sourceActivationLease?.commit()
const {
sourceActivationLease: _lease,
sourceRecovery: _sourceRecovery,
...spawnResult
} = result
return spawnResult
} catch (error) {
result?.sourceActivationLease?.rollback()
throw error
}
args.acceptLivePty(result.id)
result.sourceActivationLease?.commit()
const {
sourceActivationLease: _lease,
sourceRecovery: _sourceRecovery,
...spawnResult
} = result
return spawnResult
} catch (error) {
result?.sourceActivationLease?.rollback()
throw error
}
// Why not SSH_SESSION_EXPIRED: the relay only reaches a restoreRequired reply after finding the
// managed PTY and confirming its process is alive, so this is `unverifiable` about the delivery
// and positive evidence the PTY is live. Claiming expiry here made the caller retire the pane
// binding and cold-restore the agent into a second copy of a running session.
throw new Error(
`${SSH_PTY_SOURCE_RESTORE_REQUIRED_ERROR}: ${toRelaySshPtyId(
args.connectionId,
args.sessionId
)} ${restoreRequiredReason}`
)
}
@@ -12,10 +12,10 @@ import {
import { projectIpcPtyConnectResult } from './ipc-pty-connect-result'
import { waitAtTerminalPtyPreSpawnE2EBarrier } from './terminal-pty-pre-spawn-e2e-barrier'
import type { IpcPtySessionHandlers } from './ipc-pty-session-handlers'
import { isSshSessionGoneError } from './pty-connection/pty-connect-limits'
import { spawnIpcPty } from './ipc-pty-spawn-request'
import type { IpcPtyTransportOptions, PtyConnectResult, PtyTransport } from './pty-transport-types'
const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED'
const SSH_PTY_CONNECTION_MISMATCH_MARKER = 'belongs to SSH connection'
type PtyConnectOptions = Parameters<PtyTransport['connect']>[0]
@@ -183,8 +183,7 @@ function handleConnectError(
if (
connectionId &&
options.sessionId &&
(message.includes(SSH_SESSION_EXPIRED_ERROR) ||
message.includes(SSH_PTY_CONNECTION_MISMATCH_MARKER))
(isSshSessionGoneError(message) || message.includes(SSH_PTY_CONNECTION_MISMATCH_MARKER))
) {
return { id: options.sessionId, sessionExpired: true }
}
@@ -3,12 +3,9 @@ import { useAppStore } from '@/store'
import { isRuntimeOwnedSshTargetId } from '../../../../../shared/execution-host'
import { resolveSshPaneConnectGate } from '../ssh-pane-connect-gate'
import {
isSshSessionExpiredError,
waitForUserInitiatedSshConnect,
waitForSshConnection
} from './ssh-session-connect'
import { waitForUserInitiatedSshConnect, waitForSshConnection } from './ssh-session-connect'
import { isRemoteRuntimePtyId } from './paired-parked-terminal-restore'
import { isSshSessionGoneError } from './pty-connect-limits'
import { toProcessExitStartup } from './process-exit-startup'
import type { ConnectPanePtySession } from './connect-pane-pty-session'
@@ -153,7 +150,7 @@ export function runDeferredSessionAttach(session: ConnectPanePtySession): void {
session.clearHiddenOutputRestoreState()
const outputCallbacks = session.captureTransportOutputCallbacks(
(message) => {
if (isSshSessionExpiredError(message)) {
if (isSshSessionGoneError(message)) {
expiredReattachError = true
return
}
@@ -287,7 +284,7 @@ export function runDeferredSessionAttach(session: ConnectPanePtySession): void {
if (session.rejectObsoleteDirectSshReattach(pendingSessionId)) {
return
}
if (isSshSessionExpiredError(err)) {
if (isSshSessionGoneError(err)) {
useAppStore.getState().removeDeferredSshSessionId(session.deps.tabId)
session.clearExitedPanePtyLayoutBinding(pendingSessionId)
session.deps.clearTabPtyId(session.deps.tabId, pendingSessionId)
@@ -1,6 +1,6 @@
import { warnTerminalLifecycleAnomaly } from '../terminal-lifecycle-diagnostics'
import { recordPtyConnectDiagnostic } from './pty-connect-limits'
import { isSshSessionExpiredError } from './ssh-session-connect'
import { isSshSessionGoneError } from './pty-connect-limits'
import { isRemoteRuntimePtyId } from './paired-parked-terminal-restore'
import { toProcessExitStartup } from './process-exit-startup'
import { recoverUnverifiableDirectSshReattach } from './direct-ssh-reattach-recovery'
@@ -28,7 +28,7 @@ export function startDeferredSessionReattach(
const coldRestoreStartup = session.buildColdRestoreAgentResumeStartup()
const outputCallbacks = session.captureTransportOutputCallbacks(
(message) => {
if (isSshSessionExpiredError(message)) {
if (isSshSessionGoneError(message)) {
expiredReattachError = true
return
}
@@ -165,7 +165,7 @@ export function startDeferredSessionReattach(
ptyId: deferredReattachSessionId,
reason: message
})
if (session.connectionId && isSshSessionExpiredError(err)) {
if (session.connectionId && isSshSessionGoneError(err)) {
session.clearExitedPanePtyLayoutBinding(deferredReattachSessionId)
session.deps.clearTabPtyId(session.deps.tabId, deferredReattachSessionId)
session.startFreshColdRestoreAgentResume(coldRestoreStartup, {
@@ -3,6 +3,25 @@ import { e2eConfig } from '@/lib/e2e-config'
export const pendingSpawnByPaneKey = new Map<string, Promise<string | null>>()
export const pendingSpawnGenerationByPaneKey = new Map<string, number>()
export const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED'
const SSH_PTY_IDENTITY_MISMATCH_ERROR = 'SSH_PTY_IDENTITY_MISMATCH'
/**
* True only when the host answered that this pane's PTY is gone, which is the one thing that
* licenses retiring the binding and cold-restoring the agent into a fresh shell.
*
* The mismatch suffix is excluded because it means the opposite: the relay found a LIVE PTY under
* that id owned by another pane, and says nothing about this pane's process. Respawning there puts
* a second agent on one transcript (docs/reference/ssh-execution-boundary.md). Main already refuses
* to respawn on it — `isPtyAlreadyGoneError` takes the class, not the message — so a bare substring
* test here silently disagreed with the gate one process over.
*/
export function isSshSessionGoneError(error: unknown): boolean {
const message = error instanceof Error ? error.message : String(error)
return (
message.includes(SSH_SESSION_EXPIRED_ERROR) &&
!message.includes(SSH_PTY_IDENTITY_MISMATCH_ERROR)
)
}
// Why: relay requests expire at 30s; leave one second for their fallback before re-arming locally.
export const DIRECT_SSH_PANE_RETRY_SETTLEMENT_TIMEOUT_MS = 31_000
export const REMOTE_PTY_ID_PREFIX = 'remote:'
@@ -1,5 +1,4 @@
import { useAppStore } from '@/store'
import { SSH_SESSION_EXPIRED_ERROR } from './pty-connect-limits'
import type { ConnectPanePtySession } from './connect-pane-pty-session'
// Why: when multiple panes/tabs need the same deferred SSH connection,
@@ -11,10 +10,6 @@ type UserInitiatedSshConnectOutcome = 'connected' | 'cancelled' | 'failed'
const sshConnectPromises = new Map<string, Promise<SshConnectResult>>()
export function isSshSessionExpiredError(err: unknown): boolean {
return (err instanceof Error ? err.message : String(err)).includes(SSH_SESSION_EXPIRED_ERROR)
}
function sshPromptConnectOutcomeForStatus(
status: string | undefined,
sawNonDisconnected: boolean
@@ -0,0 +1,48 @@
import { describe, expect, it } from 'vitest'
import { isSshSessionGoneError } from './pty-connect-limits'
// The only gate the renderer has for "retire this pane's binding and cold-restore the agent into a
// fresh shell". Anything it accepts that is not host-attested absence puts a second `--resume` on a
// running agent's transcript (docs/reference/ssh-execution-boundary.md).
describe('the renderer verdict that licenses replacing a pane PTY', () => {
it('accepts the relay answering that this pane PTY is gone', () => {
expect(isSshSessionGoneError(new Error('SSH_SESSION_EXPIRED: ssh:conn-1@@pty-1'))).toBe(true)
})
// The relay found a LIVE PTY under that id belonging to another pane. It observed nothing about
// this pane's process, and main's own gate (`isPtyAlreadyGoneError`) already refuses to respawn
// on it — the renderer read the same message as absence and respawned anyway.
it('refuses an identity mismatch, which names a live PTY owned by another pane', () => {
expect(
isSshSessionGoneError(
new Error('SSH_SESSION_EXPIRED: ssh:conn-1@@pty-1 SSH_PTY_IDENTITY_MISMATCH')
)
).toBe(false)
})
it('refuses an identity mismatch wrapped by the IPC boundary', () => {
expect(
isSshSessionGoneError(
"Error invoking remote method 'pty:spawn': Error: SSH_SESSION_EXPIRED: ssh:conn-1@@pty-1 SSH_PTY_IDENTITY_MISMATCH"
)
).toBe(false)
})
// A live PTY whose output delivery needs reopening: main no longer wears the expiry token for it,
// and the verdict must stay `unverifiable` even if some future caller reintroduces the wording.
it('refuses a source-restore verdict for a PTY the relay just proved alive', () => {
expect(
isSshSessionGoneError(
new Error('SSH_PTY_SOURCE_RESTORE_REQUIRED: ssh:conn-1@@pty-1 checkpointUnavailable')
)
).toBe(false)
})
it.each([
['a lost link', 'SSH connection lost, reconnecting...'],
['a request timeout', 'Request "pty.attach" timed out after 10000ms'],
['a disposed multiplexer', 'Multiplexer disposed']
])('refuses %s', (_label, message) => {
expect(isSshSessionGoneError(new Error(message))).toBe(false)
})
})