mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
fix(ssh): preserve live PTY after delivery recovery failure
Surface repeated output-delivery recovery failure without retiring the remote lease or cold-starting a duplicate agent. Keep renderer retry authority bounded and explain that the remote session remains live.
This commit is contained in:
+71
@@ -276,6 +276,77 @@ describe('connectPanePty', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('does not replace a live SSH session when only output delivery cannot resume', async () => {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const restoredPtyId = toAppSshPtyId('target-a', 'pty-live')
|
||||
const transport = createMockTransport()
|
||||
transport.connect.mockImplementationOnce(async (options) => {
|
||||
options.callbacks?.onError?.(`SSH_PTY_RESTORE_REQUIRED: ${restoredPtyId}`)
|
||||
return { id: restoredPtyId, deliveryUnresumable: true }
|
||||
})
|
||||
transportFactoryQueue.push(transport)
|
||||
const pendingRetry = {
|
||||
attemptId: 'attempt-delivery-unresumable',
|
||||
authority: {
|
||||
targetId: 'target-a',
|
||||
providerEpoch: 'epoch-1',
|
||||
connectionGeneration: 3
|
||||
},
|
||||
tabGeneration: 7,
|
||||
startedAt: 1
|
||||
}
|
||||
const settleDirectSshPaneRetry = vi.fn()
|
||||
const deps = createDeps()
|
||||
mockStoreState = {
|
||||
...mockStoreState,
|
||||
tabsByWorktree: {
|
||||
'wt-1': [{ id: 'tab-1', ptyId: restoredPtyId, generation: 7 }]
|
||||
},
|
||||
ptyIdsByTabId: { 'tab-1': [restoredPtyId] },
|
||||
repos: [{ id: 'repo1', connectionId: 'target-a', displayName: 'orca' }],
|
||||
sshConnectionStates: new Map([
|
||||
[
|
||||
'target-a',
|
||||
{
|
||||
targetId: 'target-a',
|
||||
status: 'connected',
|
||||
providerEpoch: 'epoch-1',
|
||||
connectionGeneration: 3
|
||||
}
|
||||
]
|
||||
]),
|
||||
directSshPaneRetryByTabId: { 'tab-1': pendingRetry },
|
||||
settleDirectSshPaneRetry
|
||||
}
|
||||
|
||||
connectPanePty(
|
||||
createPane(1) as never,
|
||||
createManager(1) as never,
|
||||
createDeps({
|
||||
...deps,
|
||||
restoredLeafId: LEAF_1,
|
||||
restoredPtyIdByLeafId: { [LEAF_1]: restoredPtyId }
|
||||
}) as never
|
||||
)
|
||||
await flushAsyncTicks(12)
|
||||
|
||||
expect(transport.connect).toHaveBeenCalledTimes(1)
|
||||
expect(deps.clearExitedPanePtyLayoutBinding).not.toHaveBeenCalled()
|
||||
expect(deps.clearTabPtyId).not.toHaveBeenCalled()
|
||||
expect(deps.updateTabPtyId).not.toHaveBeenCalled()
|
||||
expect(deps.onPtyErrorRef.current).toHaveBeenCalledWith(
|
||||
1,
|
||||
`SSH_PTY_RESTORE_REQUIRED: ${restoredPtyId}`
|
||||
)
|
||||
expect(settleDirectSshPaneRetry).toHaveBeenCalledExactlyOnceWith({
|
||||
status: 'failed',
|
||||
tabId: 'tab-1',
|
||||
attemptId: pendingRetry.attemptId,
|
||||
authority: pendingRetry.authority,
|
||||
tabGeneration: pendingRetry.tabGeneration
|
||||
})
|
||||
})
|
||||
|
||||
it('rejects expired reattach state after its direct SSH retry lease is revoked', async () => {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const restoredPtyId = toAppSshPtyId('target-a', 'pty-stale-reattach')
|
||||
|
||||
@@ -8104,6 +8104,11 @@ export function connectPanePty(
|
||||
// Why: an obsolete reattach must stop consuming frames without killing the durable PTY a newer lease may adopt.
|
||||
return false
|
||||
}
|
||||
if (connectResult?.deliveryUnresumable) {
|
||||
// Why: replacing a live PTY after output recovery fails would duplicate the remote agent.
|
||||
settleDirectSshPaneRetryAttempt(directSshRetryAttempt, 'failed')
|
||||
return false
|
||||
}
|
||||
const ptyId =
|
||||
connectResult?.id ?? (typeof result === 'string' ? result : transport.getPtyId())
|
||||
if (!ptyId) {
|
||||
|
||||
@@ -192,6 +192,42 @@ describe('createIpcPtyTransport', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps a live SSH session distinct when only 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'))
|
||||
;(globalThis as { window: typeof window }).window = {
|
||||
...originalWindow,
|
||||
api: {
|
||||
...originalWindow?.api,
|
||||
pty: {
|
||||
...originalWindow?.api?.pty,
|
||||
spawn: spawnMock,
|
||||
write: vi.fn(),
|
||||
resize: vi.fn(),
|
||||
kill: vi.fn(),
|
||||
onData: vi.fn(() => () => {}),
|
||||
onReplay: vi.fn(() => () => {}),
|
||||
onExit: vi.fn(() => () => {})
|
||||
}
|
||||
}
|
||||
} as unknown as typeof window
|
||||
|
||||
const onError = vi.fn()
|
||||
const result = await createIpcPtyTransport({ connectionId: 'ssh-1' }).connect({
|
||||
url: '',
|
||||
sessionId: 'ssh:ssh-1@@pty-live',
|
||||
callbacks: { onError }
|
||||
})
|
||||
|
||||
expect(onError).toHaveBeenCalledWith('SSH_PTY_RESTORE_REQUIRED: ssh:ssh-1@@pty-live')
|
||||
expect(result).toEqual({
|
||||
id: 'ssh:ssh-1@@pty-live',
|
||||
deliveryUnresumable: true
|
||||
})
|
||||
})
|
||||
|
||||
it('surfaces terminal session state save failures without the Electron IPC wrapper', async () => {
|
||||
const { createIpcPtyTransport } = await import('./pty-transport')
|
||||
const wrappedMessage = `Error invoking remote method 'pty:spawn': Error: ${createTerminalSessionStateSaveFailureMessage()}`
|
||||
|
||||
@@ -89,6 +89,8 @@ export type PtyConnectResult = {
|
||||
snapshotSeq?: number
|
||||
isAlternateScreen?: boolean
|
||||
sessionExpired?: boolean
|
||||
/** The host confirmed the session live but could not reopen its output delivery. */
|
||||
deliveryUnresumable?: true
|
||||
coldRestore?: { scrollback: string; cwd: string; cols?: number; rows?: number }
|
||||
replay?: string
|
||||
startupCwdFallback?: { kind: 'worktree'; cwd: string }
|
||||
|
||||
@@ -72,6 +72,7 @@ export type {
|
||||
export { extractLastOscTitle } from '../../../../shared/agent-detection'
|
||||
|
||||
const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED'
|
||||
const SSH_PTY_RESTORE_REQUIRED_ERROR = 'SSH_PTY_RESTORE_REQUIRED'
|
||||
// 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
|
||||
@@ -945,6 +946,17 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
|
||||
return spawnResult.id
|
||||
} catch (err) {
|
||||
const msg = extractIpcErrorMessage(err, err instanceof Error ? err.message : String(err))
|
||||
if (
|
||||
connectionId &&
|
||||
options.sessionId &&
|
||||
msg.includes(SSH_PTY_RESTORE_REQUIRED_ERROR)
|
||||
) {
|
||||
storedCallbacks.onError?.(msg)
|
||||
return {
|
||||
id: options.sessionId,
|
||||
deliveryUnresumable: true
|
||||
} satisfies PtyConnectResult
|
||||
}
|
||||
if (
|
||||
connectionId &&
|
||||
options.sessionId &&
|
||||
|
||||
Reference in New Issue
Block a user