From 8e6e1c61364536a185b4e82244f971dd6db4a457 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 16 Aug 2026 18:46:05 -0700 Subject: [PATCH] fix(ssh): keep retry transport loss unverifiable Distinguish host-confirmed live delivery failure from a transport loss during verification. Preserve the detached lease and pane binding for both, but reserve provider live state and live-session copy for an observed host response. --- .../pty-daemon-ssh-lease-lifecycle.test.ts | 31 ++++++++++++++++--- src/main/ipc/pty.ts | 11 +++++-- src/main/providers/ssh-pty-errors.ts | 6 ++++ .../ssh-pty-restore-required-reattach.test.ts | 12 ++++--- .../providers/ssh-pty-session-reattach.ts | 7 ++--- .../terminal-pane/TerminalErrorToast.test.ts | 8 +++++ .../terminal-pane/TerminalErrorToast.tsx | 10 ++++++ .../pty-transport-spawn-errors.test.ts | 19 ++++++++++-- .../terminal-pane/pty-transport-types.ts | 2 +- .../components/terminal-pane/pty-transport.ts | 4 ++- src/renderer/src/i18n/locales/en.json | 3 +- 11 files changed, 93 insertions(+), 20 deletions(-) diff --git a/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts b/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts index ffe56a51041..2ac9054f0b3 100644 --- a/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts +++ b/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts @@ -3,6 +3,7 @@ import { openCodeClearPtyMock, piClearPtyMock } from './pty-ipc-mock-registry' import { setupPtyIpcSuite } from './pty-ipc-test-harness' import { SSH_PTY_IDENTITY_MISMATCH_ERROR, + SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR, SSH_PTY_RESTORE_REQUIRED_ERROR, SSH_SESSION_EXPIRED_ERROR } from '../providers/ssh-pty-errors' @@ -137,12 +138,13 @@ describe('registerPtyHandlers', () => { }) // Why: a relay that cannot resume delivery has not observed the PTY exit, so the lease must // stay reattachable — `reattachKnownPtys` drops only 'terminated'/'expired' leases. - it('keeps a remote lease reattachable when the relay only lost its delivery record', async () => { + it('keeps a remote lease reattachable when exit is not observed', async () => { const scopedPtyId = 'ssh:ssh-1@@remote-pty' const remoteWrite = vi.fn() - const sshSpawn = vi.fn(async () => { - throw new Error(`${SSH_PTY_RESTORE_REQUIRED_ERROR}: remote-pty`) - }) + const sshSpawn = vi + .fn() + .mockRejectedValueOnce(new Error(`${SSH_PTY_RESTORE_REQUIRED_ERROR}: remote-pty`)) + .mockRejectedValueOnce(new Error(`${SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR}: remote-pty`)) const store = { markSshRemotePtyLease: vi.fn() } @@ -206,6 +208,27 @@ describe('registerPtyHandlers', () => { data: 'echo still-owned' }) expect(remoteWrite).toHaveBeenCalledWith(scopedPtyId, 'echo still-owned') + + store.markSshRemotePtyLease.mockClear() + await expect( + handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + env: {}, + connectionId: 'ssh-1', + sessionId: scopedPtyId + }) + ).rejects.toThrow(SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR) + expect(store.markSshRemotePtyLease).not.toHaveBeenCalledWith( + 'ssh-1', + 'remote-pty', + 'expired' + ) + expect(store.markSshRemotePtyLease).toHaveBeenCalledWith( + 'ssh-1', + 'remote-pty', + 'detached' + ) } finally { deletePtyOwnership(scopedPtyId) } diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index fb1623719d9..fb240b1d4d6 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -92,6 +92,7 @@ import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery import { SSH_SESSION_EXPIRED_ERROR, isSshPtyIdentityMismatchError, + isSshPtyLivenessUnverifiableError, isSshPtyNotFoundError, isSshPtyRestoreRequiredError } from '../providers/ssh-pty-errors' @@ -5173,7 +5174,10 @@ export function registerPtyHandlers( if ( args.connectionId && effectiveSessionRelayId !== undefined && - (isSshPtyRestoreRequiredError(spawnError) || isSshPtyRestoreRequiredError(rawMessage)) + (isSshPtyRestoreRequiredError(spawnError) || + isSshPtyRestoreRequiredError(rawMessage) || + isSshPtyLivenessUnverifiableError(spawnError) || + isSshPtyLivenessUnverifiableError(rawMessage)) ) { // Why: the relay lost delivery, not the PTY; keep the lease reattachable so the next // attach recovers the live remote session instead of cold-starting a duplicate over it. @@ -6733,7 +6737,10 @@ export function registerPtyHandlers( if ( args.connectionId && effectiveSessionRelayId !== undefined && - (isSshPtyRestoreRequiredError(spawnError) || isSshPtyRestoreRequiredError(rawMessage)) + (isSshPtyRestoreRequiredError(spawnError) || + isSshPtyRestoreRequiredError(rawMessage) || + isSshPtyLivenessUnverifiableError(spawnError) || + isSshPtyLivenessUnverifiableError(rawMessage)) ) { // Why: the relay lost delivery, not the PTY; keep the lease reattachable so the next // attach recovers the live remote session instead of cold-starting a duplicate over it. diff --git a/src/main/providers/ssh-pty-errors.ts b/src/main/providers/ssh-pty-errors.ts index 460344867c2..95ef265836a 100644 --- a/src/main/providers/ssh-pty-errors.ts +++ b/src/main/providers/ssh-pty-errors.ts @@ -3,6 +3,7 @@ export const SSH_PTY_IDENTITY_MISMATCH_ERROR = 'SSH_PTY_IDENTITY_MISMATCH' // Why: delivery could not be resumed. The relay does not report exited, so this must never // be relabelled as expiry — that retires a live remote session and cold-starts a duplicate. export const SSH_PTY_RESTORE_REQUIRED_ERROR = 'SSH_PTY_RESTORE_REQUIRED' +export const SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR = 'SSH_PTY_LIVENESS_UNVERIFIABLE' export function isSshPtyNotFoundError(error: unknown): boolean { const message = error instanceof Error ? error.message : String(error) @@ -14,6 +15,11 @@ export function isSshPtyRestoreRequiredError(error: unknown): boolean { return message.includes(SSH_PTY_RESTORE_REQUIRED_ERROR) } +export function isSshPtyLivenessUnverifiableError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error) + return message.includes(SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR) +} + export function isSshPtyIdentityMismatchError(error: unknown): boolean { const message = error instanceof Error ? error.message : String(error) return message.includes(SSH_PTY_IDENTITY_MISMATCH_ERROR) || /identity mismatch/i.test(message) diff --git a/src/main/providers/ssh-pty-restore-required-reattach.test.ts b/src/main/providers/ssh-pty-restore-required-reattach.test.ts index 875b78c80cc..b8a36cf1c65 100644 --- a/src/main/providers/ssh-pty-restore-required-reattach.test.ts +++ b/src/main/providers/ssh-pty-restore-required-reattach.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it, vi } from 'vitest' -import { SSH_PTY_RESTORE_REQUIRED_ERROR, SSH_SESSION_EXPIRED_ERROR } from './ssh-pty-errors' +import { + SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR, + SSH_PTY_RESTORE_REQUIRED_ERROR, + SSH_SESSION_EXPIRED_ERROR +} from './ssh-pty-errors' import { SshPtyProvider } from './ssh-pty-provider' // The relay answers `restoreRequired` from its DELIVERY layer only: `requireRestore` @@ -69,7 +73,7 @@ describe('SSH PTY reattach when the relay requires source restoration', () => { expect(provider.hasPty('ssh:conn-1@@pty-old')).toBe(true) }) - it('keeps the verified live PTY when the fresh delivery attach loses contact', async () => { + it('reports unverifiable when the fresh delivery attach loses contact', async () => { const mux = createMockMux() mux.request .mockResolvedValueOnce(restoreRequiredAnswer) @@ -78,9 +82,9 @@ describe('SSH PTY reattach when the relay requires source restoration', () => { const message = await spawnError(provider) - expect(message).toContain(SSH_PTY_RESTORE_REQUIRED_ERROR) + expect(message).toContain(SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR) expect(message).not.toContain(SSH_SESSION_EXPIRED_ERROR) - expect(provider.hasPty('ssh:conn-1@@pty-old')).toBe(true) + expect(provider.hasPty('ssh:conn-1@@pty-old')).toBe(false) expect(mux.request.mock.calls.filter((call) => call[0] === 'pty.attach')).toHaveLength(2) }) diff --git a/src/main/providers/ssh-pty-session-reattach.ts b/src/main/providers/ssh-pty-session-reattach.ts index e3178837305..9f93a2a8a08 100644 --- a/src/main/providers/ssh-pty-session-reattach.ts +++ b/src/main/providers/ssh-pty-session-reattach.ts @@ -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_LIVENESS_UNVERIFIABLE_ERROR, SSH_PTY_RESTORE_REQUIRED_ERROR, SSH_SESSION_EXPIRED_ERROR, isSshPtyIdentityMismatchError, @@ -282,11 +283,9 @@ export async function reattachSshPtySessionForSpawn( ) { throw error } - // Why: the first answer proved the PTY live; losing contact during fresh delivery cannot - // turn that into an exited verdict or authorize a duplicate spawn. - args.acceptLivePty(unresumable.id) + // Why: losing contact during verification is unverifiable, not live or exited. throw new Error( - `${SSH_PTY_RESTORE_REQUIRED_ERROR}: ${toRelaySshPtyId(args.connectionId, unresumable.id)}`, + `${SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR}: ${toRelaySshPtyId(args.connectionId, unresumable.id)}`, { cause: error } ) } diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts b/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts index 831070e5b21..ed9573ce563 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts @@ -156,6 +156,14 @@ describe('humanizeTerminalError', () => { expect(humanized).toContain('session is live') }) + it('reports unverifiable liveness after verification loses contact', () => { + const humanized = humanizeTerminalError('SSH_PTY_LIVENESS_UNVERIFIABLE: orca:2f1c@@pty-7') + expect(humanized).not.toContain('SSH_PTY_LIVENESS_UNVERIFIABLE') + expect(humanized).not.toContain('orca:2f1c@@pty-7') + expect(humanized).toContain('session is unverifiable') + expect(humanized).not.toContain('session is live') + }) + it('replaces only the unreattachable line in an aggregated error', () => { const humanized = humanizeTerminalError('Paste failed.\nSSH_SESSION_EXPIRED: orca:2f1c@@pty-7') expect(humanized.startsWith('Paste failed.\n')).toBe(true) diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx index dbc372b74db..84cdb4baf89 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx @@ -43,6 +43,9 @@ const UNREATTACHABLE_SESSION_REPLACE_PATTERNS = UNREATTACHABLE_SESSION_SOURCES.m const UNRESUMABLE_DELIVERY_SOURCE = 'SSH_PTY_RESTORE_REQUIRED:[ \\t]*\\S*' const UNRESUMABLE_DELIVERY_PATTERN = new RegExp(UNRESUMABLE_DELIVERY_SOURCE) const UNRESUMABLE_DELIVERY_REPLACE_PATTERN = new RegExp(UNRESUMABLE_DELIVERY_SOURCE, 'g') +const UNVERIFIABLE_LIVENESS_SOURCE = 'SSH_PTY_LIVENESS_UNVERIFIABLE:[ \\t]*\\S*' +const UNVERIFIABLE_LIVENESS_PATTERN = new RegExp(UNVERIFIABLE_LIVENESS_SOURCE) +const UNVERIFIABLE_LIVENESS_REPLACE_PATTERN = new RegExp(UNVERIFIABLE_LIVENESS_SOURCE, 'g') function isSshError(error: string): boolean { return isSshReconnectOwnedTerminalError(error) @@ -81,6 +84,7 @@ export function isExplainedTerminalError(error: string): boolean { TERMINAL_HOST_GONE_PATTERN.test(line) || LEGACY_TERMINAL_HOST_GONE_PATTERN.test(line) || UNRESUMABLE_DELIVERY_PATTERN.test(line) || + UNVERIFIABLE_LIVENESS_PATTERN.test(line) || UNREATTACHABLE_SESSION_PATTERNS.some((pattern) => pattern.test(line)) ) } @@ -115,6 +119,12 @@ export function humanizeTerminalError(error: string): string { "Orca couldn't resume this pane's output from the host. The terminal session is live — reopen this pane to reconnect to it." ) ) + humanized = humanized.replace(UNVERIFIABLE_LIVENESS_REPLACE_PATTERN, () => + translate( + 'auto.components.terminal.pane.TerminalErrorToast.sessionLivenessUnverifiable', + "Orca lost contact while reopening this pane's output. The terminal session is unverifiable, so Orca left it untouched. Reopen this pane to retry." + ) + ) humanized = humanizeUnreattachableSession(humanized) if (!isExplainedTerminalError(humanized)) { return humanized diff --git a/src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts index a96cf226d1d..26a8589db1c 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-spawn-errors.test.ts @@ -192,11 +192,12 @@ describe('createIpcPtyTransport', () => { }) }) - it('keeps a live SSH session distinct when only output delivery cannot resume', async () => { + it('keeps the SSH binding when output delivery cannot resume', async () => { const { createIpcPtyTransport } = await import('./pty-transport') const spawnMock = vi .fn() - .mockRejectedValue(new Error('SSH_PTY_RESTORE_REQUIRED: ssh:ssh-1@@pty-live')) + .mockRejectedValueOnce(new Error('SSH_PTY_RESTORE_REQUIRED: ssh:ssh-1@@pty-live')) + .mockRejectedValueOnce(new Error('SSH_PTY_LIVENESS_UNVERIFIABLE: ssh:ssh-1@@pty-live')) ;(globalThis as { window: typeof window }).window = { ...originalWindow, api: { @@ -215,7 +216,8 @@ describe('createIpcPtyTransport', () => { } as unknown as typeof window const onError = vi.fn() - const result = await createIpcPtyTransport({ connectionId: 'ssh-1' }).connect({ + const transport = createIpcPtyTransport({ connectionId: 'ssh-1' }) + const result = await transport.connect({ url: '', sessionId: 'ssh:ssh-1@@pty-live', callbacks: { onError } @@ -226,6 +228,17 @@ describe('createIpcPtyTransport', () => { id: 'ssh:ssh-1@@pty-live', deliveryUnresumable: true }) + + const unverifiableResult = await transport.connect({ + url: '', + sessionId: 'ssh:ssh-1@@pty-live', + callbacks: { onError } + }) + expect(onError).toHaveBeenCalledWith('SSH_PTY_LIVENESS_UNVERIFIABLE: ssh:ssh-1@@pty-live') + expect(unverifiableResult).toEqual({ + id: 'ssh:ssh-1@@pty-live', + deliveryUnresumable: true + }) }) it('surfaces terminal session state save failures without the Electron IPC wrapper', async () => { diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index 9eb6c21f8f4..65bda9e8629 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -89,7 +89,7 @@ export type PtyConnectResult = { snapshotSeq?: number isAlternateScreen?: boolean sessionExpired?: boolean - /** The host confirmed the session live but could not reopen its output delivery. */ + /** Output delivery failed without evidence that the remote PTY exited. */ deliveryUnresumable?: true coldRestore?: { scrollback: string; cwd: string; cols?: number; rows?: number } replay?: string diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index 6cbaada3446..2d84e230ee9 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -73,6 +73,7 @@ export { extractLastOscTitle } from '../../../../shared/agent-detection' const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED' const SSH_PTY_RESTORE_REQUIRED_ERROR = 'SSH_PTY_RESTORE_REQUIRED' +const SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR = 'SSH_PTY_LIVENESS_UNVERIFIABLE' // Why: main rejects a session reattached under a different SSH connection with this phrase; treat as stale (spawn fresh), not a crash. const SSH_PTY_CONNECTION_MISMATCH_MARKER = 'belongs to SSH connection' const STALE_TITLE_TIMEOUT = 3000 // ms before stale working title is cleared @@ -949,7 +950,8 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra if ( connectionId && options.sessionId && - msg.includes(SSH_PTY_RESTORE_REQUIRED_ERROR) + (msg.includes(SSH_PTY_RESTORE_REQUIRED_ERROR) || + msg.includes(SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR)) ) { storedCallbacks.onError?.(msg) return { diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index d6d15f0e2d4..33f8e75450f 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -2915,7 +2915,8 @@ "7ee11bc0db": "Orca couldn't confirm whether this terminal's previous session is still running, so it left the session untouched. Reopen this pane to retry.", "e16012e31e": "The terminal daemon that owned this session exited, so the session and its scrollback could not be recovered. Open a new terminal to continue.", "sessionUnavailable": "Orca couldn't reattach to this pane's terminal session on the host. Open a new terminal to continue.", - "sessionOutputUnresumable": "Orca couldn't resume this pane's output from the host. The terminal session is live — reopen this pane to reconnect to it." + "sessionOutputUnresumable": "Orca couldn't resume this pane's output from the host. The terminal session is live — reopen this pane to reconnect to it.", + "sessionLivenessUnverifiable": "Orca lost contact while reopening this pane's output. The terminal session is unverifiable, so Orca left it untouched. Reopen this pane to retry." }, "TerminalPane": { "ac112e9036": "Remove title",