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 000000000000..0275b6081365 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-attach-failure-recovery.test.ts @@ -0,0 +1,157 @@ +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 + + /** Rejects every RPC the way a paired runtime behind a dropped tunnel does. */ + 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 b1bd7336c863..e1cfe2268f2b 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 => @@ -860,13 +864,22 @@ 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 } if (lastAttachOptions) { - transport.attach(lastAttachOptions) + replayingRecoveryAttach = true + try { + transport.attach(lastAttachOptions) + } finally { + replayingRecoveryAttach = false + } return true } if (lastConnectOptions) { @@ -1629,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) { @@ -1664,6 +1681,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 } @@ -2370,11 +2395,14 @@ 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 cancelTerminalCreateRetryWait() - recovery.cancel() + if (!replayingRecoveryAttach) { + recovery.cancel() + } resetRecoveryReplacementPolicy() resetSameHandleEndReuse() clearPublishedHandleWait() @@ -2464,7 +2492,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 0ecd493b2ae3..d9c789c8e7ee 100644 --- a/src/renderer/src/runtime/use-remote-runtime-recovery-triggers.ts +++ b/src/renderer/src/runtime/use-remote-runtime-recovery-triggers.ts @@ -1,8 +1,43 @@ 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'] + +/** + * 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 +): 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 +} + +/** 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) @@ -13,9 +48,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() } }, []) }