From e39cfc82fcc05c9ddfb09c0195fb645bb8cd9464 Mon Sep 17 00:00:00 2001 From: luisgonzaleznf Date: Thu, 3 Sep 2026 19:32:57 +0200 Subject: [PATCH 1/2] fix(remote-runtime): retry a pane attach that fails while the runtime is unreachable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A client that starts while its paired runtime is unreachable attaches to already-open panes instead of creating them, so the failure lands in attach() rather than connect(). attach() never sets `connected`, so the recoverable branch's scheduleResubscribeAfterTransportClose() returned at its `!connected` guard: nothing armed, `connecting` latched, and neither emitRecoveryState() nor surfaceErrorMessage() reached. getRecoveryState() then reported 'connecting', which the banner filter hides — the pane rendered as a mounted xterm bound to nothing, with no text, no error and no retry, and stayed that way after the runtime came back. connect() handles the identical error class by arming scheduleConnectRetryAfterRecoverableFailure(); that asymmetry was the bug. Arm the same retry when a recoverable error arrives with no live stream, and stop the replay wiping its own recovery window: attach() cancelled recovery on entry and its catch cancelled unconditionally, so every replay restarted the 250ms..30s ladder and the auto-recovery deadline never latched — no 'disconnected' phase, so no Reconnect button and no parked retry. connect() cancels only on the gone and fatal paths, for the same reason. Also fire pane recovery when a runtime environment becomes reachable. The existing `online` and system-resume triggers both miss a tailnet or VPN returning: navigator.onLine never went false because the LAN never dropped, and no power transition occurs. A parked pane would otherwise wait for a manual Reconnect indefinitely. Co-Authored-By: Claude Opus 5 (1M context) --- ...te-runtime-attach-failure-recovery.test.ts | 156 ++++++++++++++++++ .../remote-runtime-pty-transport.ts | 30 +++- .../use-remote-runtime-recovery-triggers.ts | 42 +++++ 3 files changed, 225 insertions(+), 3 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/remote-runtime-attach-failure-recovery.test.ts diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-attach-failure-recovery.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-attach-failure-recovery.test.ts new file mode 100644 index 00000000000..cf6c972a3ea --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-attach-failure-recovery.test.ts @@ -0,0 +1,156 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + createRemoteRuntimeTransportMocks, + type MultiplexSubscriptionCallbacks +} from './remote-runtime-pty-transport-test-harness' +import { REMOTE_RUNTIME_AUTO_RECOVERY_TIMEOUT_MS } from './remote-runtime-pty-recovery-state' + +let subscriptionCallbacks: MultiplexSubscriptionCallbacks = null +let resolvedPaneHandle = 'terminal-1' + +const { runtimeCall, resetRemoteRuntimeTransport } = createRemoteRuntimeTransportMocks({ + getCallbacks: () => subscriptionCallbacks, + setCallbacks: (callbacks) => { + subscriptionCallbacks = callbacks + }, + getResolvedPaneHandle: () => resolvedPaneHandle, + setResolvedPaneHandle: (handle) => { + resolvedPaneHandle = handle + } +}) + +// A client that starts while its paired runtime is unreachable attaches to already-open panes rather +// than creating them, so the failure lands in attach() rather than connect(). attach() never sets +// `connected`, so the recoverable branch's scheduleResubscribeAfterTransportClose() returned at its +// `!connected` guard: nothing armed, `connecting` latched, no error surfaced. The pane rendered as a +// blank xterm bound to nothing, and stayed that way after the runtime came back. +describe('recoverable attach failures on a remote runtime pane', () => { + let resolvePaneCalls = 0 + + function installUnreachableRuntime(): void { + resolvePaneCalls = 0 + runtimeCall.mockImplementation(async (args: { method: string }) => { + if (args.method === 'terminal.resolvePane') { + resolvePaneCalls += 1 + } + throw Object.assign(new Error('Remote Orca runtime closed the connection.'), { + code: 'remote_runtime_unavailable' + }) + }) + } + + // Why: the client attaches to an already-open pane on the host rather than creating one, which is + // the entry point a restored session uses and the one that had no retry armed. + const persistedPaneAttach = { + existingPtyId: 'remote:env-1@@terminal-1', + cols: 120, + rows: 40 + } as const + + beforeEach(() => { + resetRemoteRuntimeTransport() + }) + + it('arms a retry when attach fails against an unreachable runtime', async () => { + vi.useFakeTimers() + try { + installUnreachableRuntime() + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const onError = vi.fn() + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + + transport.attach({ ...persistedPaneAttach, callbacks: { onError } }) + await vi.advanceTimersByTimeAsync(1) + + // Before the fix this stayed 'connecting' forever, which the banner filter hides — a blank + // pane with no error and no retry. + expect(resolvePaneCalls).toBe(1) + expect(transport.getRecoveryState?.().phase).toBe('backoff') + expect(onError).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(1_000) + expect(resolvePaneCalls).toBeGreaterThan(1) + + transport.destroy?.() + } finally { + vi.useRealTimers() + } + }) + + it('leaves a parked retry that revives the pane when the runtime returns', async () => { + vi.useFakeTimers() + try { + installUnreachableRuntime() + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + // Why dynamic: resetRemoteRuntimeTransport() re-registers the module graph, and the retry + // registry only sees panes from the same instance the transport was loaded from. + const { retryAllRemoteRuntimePtyRecoveriesNow } = await import( + './remote-runtime-pty-recovery-state' + ) + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + + transport.attach({ ...persistedPaneAttach, callbacks: {} }) + await vi.advanceTimersByTimeAsync(REMOTE_RUNTIME_AUTO_RECOVERY_TIMEOUT_MS + 1_000) + + // An hours-long outage outlives the auto-recovery window, so the latch must stay revivable. + expect(transport.getRecoveryState?.().phase).toBe('disconnected') + const callsAtCutoff = resolvePaneCalls + + // This is what an environment-reachable trigger fires; it found nothing before the fix. + expect(retryAllRemoteRuntimePtyRecoveriesNow()).toBe(1) + await vi.advanceTimersByTimeAsync(1_000) + expect(resolvePaneCalls).toBeGreaterThan(callsAtCutoff) + + // ...and the Reconnect button must work too. + await vi.advanceTimersByTimeAsync(REMOTE_RUNTIME_AUTO_RECOVERY_TIMEOUT_MS + 1_000) + expect(transport.getRecoveryState?.().phase).toBe('disconnected') + expect(transport.retryRecovery?.()).toBe(true) + + transport.destroy?.() + } finally { + vi.useRealTimers() + } + }) + + it('still surfaces a non-recoverable attach failure instead of retrying it', async () => { + vi.useFakeTimers() + try { + resolvePaneCalls = 0 + runtimeCall.mockImplementation(async (args: { method: string }) => { + if (args.method === 'terminal.resolvePane') { + resolvePaneCalls += 1 + } + throw new Error('boom') + }) + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const { getScheduledRemoteRuntimePtyRecoveryCountForTests } = await import( + './remote-runtime-pty-recovery-state' + ) + const onError = vi.fn() + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + + transport.attach({ ...persistedPaneAttach, callbacks: { onError } }) + await vi.advanceTimersByTimeAsync(1) + + // The red error surface stays reserved for actionable failures. + expect(onError).toHaveBeenCalled() + expect(getScheduledRemoteRuntimePtyRecoveryCountForTests()).toBe(0) + + transport.destroy?.() + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index b1bd7336c86..a0b00b43f33 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -285,6 +285,10 @@ export function createRemoteRuntimePtyTransport( let terminalCreateUnknownOutcomeError: unknown = null let lastConnectOptions: Parameters[0] | null = null let lastAttachOptions: Parameters[0] | null = null + // Why: attach() resets recovery because a mount-initiated attach is a fresh start, but a recovery + // replay is the same attempt continuing. Without this the ladder restarts every replay, so the + // deadline never latches and the pane never reaches the 'disconnected' phase that shows Reconnect. + let replayingRecoveryAttach = false let resolvePaneUnavailable = false let recoveringPaneHandle: string | null = null const getRecoveryReplacementPolicy = (targetHandle: string): HostHandleReplacementPolicy => @@ -866,7 +870,12 @@ export function createRemoteRuntimePtyTransport( return false } if (lastAttachOptions) { - transport.attach(lastAttachOptions) + replayingRecoveryAttach = true + try { + transport.attach(lastAttachOptions) + } finally { + replayingRecoveryAttach = false + } return true } if (lastConnectOptions) { @@ -1664,6 +1673,14 @@ export function createRemoteRuntimePtyTransport( } if (isRecoverableRemoteRuntimeConnectionError(clientError)) { // Why: a partition is attachment state, not a terminal failure; keep the red error surface for actionable fatal errors. + if (!connected || !handle) { + // Why: an attach that never opened a stream has nothing to resubscribe, so that call would + // no-op and strand the pane blank with `connecting` latched and no retry armed (#12684 covers + // the connect() side of the same failure). Replay the entry point the way connect() does. + connecting = false + scheduleConnectRetryAfterRecoverableFailure() + return + } scheduleResubscribeAfterTransportClose() return } @@ -2374,7 +2391,9 @@ export function createRemoteRuntimePtyTransport( const attachLifecycleEpoch = ++lifecycleEpoch const generation = ++attachGeneration cancelTerminalCreateRetryWait() - recovery.cancel() + if (!replayingRecoveryAttach) { + recovery.cancel() + } resetRecoveryReplacementPolicy() resetSameHandleEndReuse() clearPublishedHandleWait() @@ -2464,7 +2483,12 @@ export function createRemoteRuntimePtyTransport( return } clearPendingViewportClaim() - recovery.cancel() + // Why: cancelling before a recoverable failure begins a fresh epoch on every replay, so the + // ladder restarts and the auto-recovery deadline never latches. connect() cancels only on the + // gone and fatal paths for the same reason; handleRemoteTerminalError owns the rest. + if (!isRecoverableRemoteRuntimeConnectionError(toRemoteRuntimeClientErrorLike(error))) { + recovery.cancel() + } handleRemoteTerminalError(error) }) }, diff --git a/src/renderer/src/runtime/use-remote-runtime-recovery-triggers.ts b/src/renderer/src/runtime/use-remote-runtime-recovery-triggers.ts index 0ecd493b2ae..4ad3546bcf3 100644 --- a/src/renderer/src/runtime/use-remote-runtime-recovery-triggers.ts +++ b/src/renderer/src/runtime/use-remote-runtime-recovery-triggers.ts @@ -1,5 +1,34 @@ import { useEffect } from 'react' import { retryAllRemoteRuntimePtyRecoveriesNow } from '@/components/terminal-pane/remote-runtime-pty-recovery-state' +import { useAppStore } from '../store' +import type { AppState } from '../store' + +type RuntimeStatusEntries = AppState['runtimeStatusByEnvironmentId'] + +// Why: a paired runtime returning is invisible to the `online` and system-resume triggers — a tailnet +// or VPN coming back changes neither `navigator.onLine` (the LAN never dropped) nor power state. A +// pane parked at 'disconnected' would then wait for a manual Reconnect forever. +function hasEnvironmentBecomeReachable( + next: RuntimeStatusEntries, + previous: RuntimeStatusEntries +): boolean { + if (next === previous) { + return false + } + for (const [environmentId, entry] of next) { + if (entry.status == null) { + continue + } + const before = previous.get(environmentId) + if (before?.status == null) { + return true + } + if ((entry.connectionGeneration ?? 0) > (before.connectionGeneration ?? 0)) { + return true + } + } + return false +} export function useRemoteRuntimeRecoveryTriggers(): void { useEffect(() => { @@ -13,9 +42,22 @@ export function useRemoteRuntimeRecoveryTriggers(): void { typeof window.api?.ui?.onSystemResumed === 'function' ? window.api.ui.onSystemResumed(advanceRemoteRuntimeRecoveryBackoffs) : null + // Why: setRuntimeEnvironmentStatus suppresses no-op re-probe writes, so this only fires on a real + // transition rather than on every poll. + const unsubscribeRuntimeStatus = useAppStore.subscribe((state, previousState) => { + if ( + hasEnvironmentBecomeReachable( + state.runtimeStatusByEnvironmentId, + previousState.runtimeStatusByEnvironmentId + ) + ) { + retryAllRemoteRuntimePtyRecoveriesNow() + } + }) return () => { window.removeEventListener('online', advanceRemoteRuntimeRecoveryBackoffs) unsubscribeSystemResumed?.() + unsubscribeRuntimeStatus() } }, []) } From 07e1f2209093a5bf12b6aa548d9c0a6c5e591d40 Mon Sep 17 00:00:00 2001 From: luisgonzaleznf Date: Thu, 3 Sep 2026 19:40:25 +0200 Subject: [PATCH 2/2] docs(remote-runtime): document the pane attach and recovery-trigger functions Satisfies the diff-scoped docstring coverage gate; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- .../remote-runtime-attach-failure-recovery.test.ts | 1 + .../terminal-pane/remote-runtime-pty-transport.ts | 11 ++++++++++- .../runtime/use-remote-runtime-recovery-triggers.ts | 12 +++++++++--- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-attach-failure-recovery.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-attach-failure-recovery.test.ts index cf6c972a3ea..0275b608136 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-attach-failure-recovery.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-attach-failure-recovery.test.ts @@ -27,6 +27,7 @@ const { runtimeCall, resetRemoteRuntimeTransport } = createRemoteRuntimeTranspor describe('recoverable attach failures on a remote runtime pane', () => { let resolvePaneCalls = 0 + /** Rejects every RPC the way a paired runtime behind a dropped tunnel does. */ function installUnreachableRuntime(): void { resolvePaneCalls = 0 runtimeCall.mockImplementation(async (args: { method: string }) => { diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index a0b00b43f33..e1cfe2268f2 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -864,7 +864,11 @@ export function createRemoteRuntimePtyTransport( return { handle: undefined, inventoryFailed: false } } - // Why: the reconnect button and a parked external trigger revive a pane the same way — replay whichever entry point opened it. + /** + * Re-opens the pane through whichever entry point last opened it — attach for a restored pane, + * connect for one this client created. The Reconnect button and a parked external trigger both + * revive a pane this way. + */ function replayLastTransportEntryPoint(): boolean { if (destroyed || terminalEnded || connected) { return false @@ -1638,6 +1642,10 @@ export function createRemoteRuntimePtyTransport( ) } + /** + * Classifies a terminal error and routes it: retirement for lifecycle evidence, recovery for + * unverifiable contact loss, and the red error surface only for actionable fatal errors. + */ function handleRemoteTerminalError(error: unknown): void { const message = runtimeTerminalErrorMessage(error) if (message === REMOTE_TERMINAL_SNAPSHOT_TOO_LARGE) { @@ -2387,6 +2395,7 @@ export function createRemoteRuntimePtyTransport( } }, + /** Binds this pane to a pty that already exists on the host, as a restored session does. */ attach(options) { const attachLifecycleEpoch = ++lifecycleEpoch const generation = ++attachGeneration diff --git a/src/renderer/src/runtime/use-remote-runtime-recovery-triggers.ts b/src/renderer/src/runtime/use-remote-runtime-recovery-triggers.ts index 4ad3546bcf3..d9c789c8e7e 100644 --- a/src/renderer/src/runtime/use-remote-runtime-recovery-triggers.ts +++ b/src/renderer/src/runtime/use-remote-runtime-recovery-triggers.ts @@ -5,9 +5,13 @@ import type { AppState } from '../store' type RuntimeStatusEntries = AppState['runtimeStatusByEnvironmentId'] -// Why: a paired runtime returning is invisible to the `online` and system-resume triggers — a tailnet -// or VPN coming back changes neither `navigator.onLine` (the LAN never dropped) nor power state. A -// pane parked at 'disconnected' would then wait for a manual Reconnect forever. +/** + * True when any environment gained a status or advanced its connection generation. + * + * Why this trigger exists at all: a paired runtime returning is invisible to `online` and + * system-resume — a tailnet or VPN coming back changes neither `navigator.onLine` (the LAN never + * dropped) nor power state, so a parked pane would wait for a manual Reconnect forever. + */ function hasEnvironmentBecomeReachable( next: RuntimeStatusEntries, previous: RuntimeStatusEntries @@ -30,8 +34,10 @@ function hasEnvironmentBecomeReachable( return false } +/** Advances parked remote-runtime backoffs when the network, the machine, or an environment returns. */ export function useRemoteRuntimeRecoveryTriggers(): void { useEffect(() => { + /** Nudges both backoff owners at once. */ const advanceRemoteRuntimeRecoveryBackoffs = (): void => { // Why: shared control and pane recovery own independent backoff timers. void window.api?.runtimeEnvironments?.retryConnectionsNow?.().catch(() => undefined)