mirror of
https://github.com/stablyai/orca.git
synced 2026-10-28 08:02:54 +00:00
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.
This commit is contained in:
@@ -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)
|
||||
}
|
||||
|
||||
+9
-2
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
|
||||
|
||||
@@ -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 }
|
||||
)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user