fix(remote-runtime): retry a pane attach that fails while the runtime is unreachable

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()
+    }
+  })
+})

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<PtyTransport['connect']>[0] | null = null
   let lastAttachOptions: Parameters<PtyTransport['attach']>[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)
       })
     },

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()
     }
   }, [])
 }