fix(desktop): retry transient remote gateway boot dials

2026-09-05 15:50:03 -04:00
parent 511633be90
commit 60e90ed6e7
2 changed files with 265 additions and 23 deletions
@@ -1,3 +1,4 @@
+import { GatewayReauthRequiredError } from '@hermes/shared'
 import { act, cleanup, render } from '@testing-library/react'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

@@ -174,7 +175,7 @@ class FakeWebSocket {
 }

 const primaryConn = {
-  authMode: 'token' as const,
+  authMode: 'token' as 'oauth' | 'token',
  baseUrl: 'https://vps.example.com',
  connectionId: 'primary-vps',
  profile: 'default',
@@ -182,6 +183,8 @@ const primaryConn = {
  wsUrl: 'wss://vps.example.com/api/ws?token=t'
 }

+const remotePrimaryConn = { ...primaryConn, mode: 'remote' as const }
+
 const coderConn = {
  authMode: 'token' as const,
  baseUrl: 'https://coder.example.com',
@@ -1626,6 +1629,209 @@ describe('useGatewayBoot remote reconnect loop (real hook, fake socket)', () =>
    expect($desktopBoot.get().error).toBeNull()
  })

+  it('RETRY CONTRACT: a resolved remote whose first renderer gateway dial fails retries even when main still reports stale non-retryable ready progress', async () => {
+    // Renderer reload against a saved direct remote: Electron has already
+    // reported a successful backend.ready snapshot, so that stale progress
+    // cannot classify the renderer-owned WebSocket dial which follows it.
+    const desktop = fakeDesktop()
+    desktop.getConnection = vi.fn(async () => remotePrimaryConn)
+    desktop.getBootProgress = vi.fn(async () => ({
+      error: null,
+      fakeMode: false,
+      message: 'Hermes is ready',
+      phase: 'backend.ready',
+      progress: 100,
+      retryable: false,
+      running: true,
+      timestamp: 1
+    }))
+    ;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
+    FakeWebSocket.mode = 'fail'
+
+    render(<Harness />)
+    await flushAsync()
+
+    // getConnection resolved the saved REMOTE descriptor; only its first
+    // renderer-owned gateway.connect() failed.
+    expect(desktop.getConnection).toHaveBeenCalledTimes(1)
+    expect(FakeWebSocket.instances).toHaveLength(1)
+    expect($desktopBoot.get().error).toBeNull()
+
+    // The same endpoint becomes reachable before the bounded retry fires.
+    FakeWebSocket.mode = 'open'
+    await advanceBackoff()
+
+    expect(desktop.getConnection).toHaveBeenCalledTimes(2)
+    expect($gatewayState.get()).toBe('open')
+    expect($desktopBoot.get().error).toBeNull()
+  })
+
+  it('RETRY CONTRACT: a local descriptor never enters the renderer-side remote retry branch', async () => {
+    const desktop = fakeDesktop()
+    desktop.getConnection.mockImplementation(async () => ({ ...remotePrimaryConn, mode: 'local' as const }) as never)
+    desktop.getBootProgress = vi.fn(async () => ({
+      error: 'contradictory main snapshot',
+      fakeMode: false,
+      message: 'Desktop boot failed',
+      phase: 'backend.error',
+      progress: 24,
+      retryable: true,
+      running: false,
+      timestamp: 1
+    }))
+    ;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
+    FakeWebSocket.mode = 'fail'
+
+    render(<Harness />)
+    await flushAsync()
+
+    expect($desktopBoot.get().error).toBeTruthy()
+    expect(desktop.getConnection).toHaveBeenCalledTimes(1)
+    await advanceBackoff()
+    expect(desktop.getConnection).toHaveBeenCalledTimes(1)
+  })
+
+  it('RETRY CONTRACT: a confirmed remote reauth rejection remains terminal despite contradictory retryable progress', async () => {
+    const desktop = fakeDesktop()
+    desktop.getConnection = vi.fn(async () => ({ ...remotePrimaryConn, authMode: 'oauth' as const }))
+    desktop.getGatewayWsUrl = vi.fn(async () => {
+      throw new GatewayReauthRequiredError('Sign in again')
+    })
+    desktop.getBootProgress = vi.fn(async () => ({
+      error: 'contradictory main snapshot',
+      fakeMode: false,
+      message: 'Desktop boot failed',
+      phase: 'backend.error',
+      progress: 24,
+      retryable: true,
+      running: false,
+      timestamp: 1
+    }))
+    ;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
+
+    render(<Harness />)
+    await flushAsync()
+
+    expect($desktopBoot.get().error).toBeTruthy()
+    expect(desktop.getConnection).toHaveBeenCalledTimes(1)
+    expect(FakeWebSocket.instances).toHaveLength(0)
+    await advanceBackoff()
+    expect(desktop.getConnection).toHaveBeenCalledTimes(1)
+  })
+
+  it('RETRY CONTRACT: invalid remote WebSocket URLs remain terminal despite contradictory retryable progress', async () => {
+    const desktop = fakeDesktop()
+    desktop.getConnection = vi.fn(async () => ({ ...remotePrimaryConn, wsUrl: 'not a WebSocket URL' }))
+    desktop.getGatewayWsUrl = vi.fn(async () => 'not a WebSocket URL')
+    desktop.getBootProgress = vi.fn(async () => ({
+      error: 'contradictory main snapshot',
+      fakeMode: false,
+      message: 'Desktop boot failed',
+      phase: 'backend.error',
+      progress: 24,
+      retryable: true,
+      running: false,
+      timestamp: 1
+    }))
+    ;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
+    FakeWebSocket.mode = 'fail'
+
+    render(<Harness />)
+    await flushAsync()
+
+    expect($desktopBoot.get().error).toBeTruthy()
+    expect(desktop.getConnection).toHaveBeenCalledTimes(1)
+    await advanceBackoff()
+    expect(desktop.getConnection).toHaveBeenCalledTimes(1)
+  })
+
+  it('RETRY CONTRACT: missing OAuth ticket capability remains terminal despite contradictory retryable progress', async () => {
+    const desktop = fakeDesktop()
+    desktop.getConnection = vi.fn(async () => ({ ...remotePrimaryConn, authMode: 'oauth' as const }))
+    Reflect.deleteProperty(desktop, 'getGatewayWsUrl')
+    desktop.getBootProgress = vi.fn(async () => ({
+      error: 'contradictory main snapshot',
+      fakeMode: false,
+      message: 'Desktop boot failed',
+      phase: 'backend.error',
+      progress: 24,
+      retryable: true,
+      running: false,
+      timestamp: 1
+    }))
+    ;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
+
+    render(<Harness />)
+    await flushAsync()
+
+    expect($desktopBoot.get().error).toMatch(/cannot refresh OAuth WebSocket tickets/i)
+    expect(desktop.getConnection).toHaveBeenCalledTimes(1)
+    expect(FakeWebSocket.instances).toHaveLength(0)
+    await advanceBackoff()
+    expect(desktop.getConnection).toHaveBeenCalledTimes(1)
+  })
+
+  it('RETRY CONTRACT: a post-connect failure stays terminal even when its socket closes before boot catches it', async () => {
+    const desktop = fakeDesktop()
+    desktop.getConnection = vi.fn(async () => remotePrimaryConn)
+    desktop.getBootProgress = vi.fn(async () => ({
+      error: 'contradictory main snapshot',
+      fakeMode: false,
+      message: 'Desktop boot failed',
+      phase: 'backend.error',
+      progress: 24,
+      retryable: true,
+      running: false,
+      timestamp: 1
+    }))
+    ;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
+
+    const refreshHermesConfig = vi.fn(async () => {
+      FakeWebSocket.instances[0]?.drop()
+      throw new Error('post-connect initialization failed')
+    })
+
+    render(<Harness refreshHermesConfig={refreshHermesConfig} />)
+    await flushAsync()
+
+    expect(refreshHermesConfig).toHaveBeenCalledTimes(1)
+    expect($desktopBoot.get().error).toBeTruthy()
+    expect(desktop.getConnection).toHaveBeenCalledTimes(1)
+    await advanceBackoff()
+    expect(desktop.getConnection).toHaveBeenCalledTimes(1)
+  })
+
+  it('RETRY CONTRACT: a persistent renderer-side remote dial outage consumes the existing five-attempt bound', async () => {
+    const desktop = fakeDesktop()
+    desktop.getConnection = vi.fn(async () => remotePrimaryConn)
+    desktop.getBootProgress = vi.fn(async () => ({
+      error: null,
+      fakeMode: false,
+      message: 'Hermes is ready',
+      phase: 'backend.ready',
+      progress: 100,
+      retryable: false,
+      running: true,
+      timestamp: 1
+    }))
+    ;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
+    FakeWebSocket.mode = 'fail'
+
+    render(<Harness />)
+    await flushAsync()
+
+    for (let i = 0; i < 7; i += 1) {
+      await advanceBackoff()
+    }
+
+    expect(desktop.getConnection).toHaveBeenCalledTimes(6)
+    expect(FakeWebSocket.instances).toHaveLength(6)
+    expect($desktopBoot.get().error).toBeTruthy()
+    await advanceBackoff()
+    expect(desktop.getConnection).toHaveBeenCalledTimes(6)
+    expect(FakeWebSocket.instances).toHaveLength(6)
+  })
+
  it('FIX #82679: boot retries are BOUNDED — a persistently dead remote ends in the recovery overlay, not a spinner', async () => {
    const desktop = fakeDesktop()
    desktop.getConnection = vi.fn(async () => {
@@ -108,15 +108,14 @@ const RECONNECT_ESCALATE_AFTER_MS = 300_000
 // only a STREAK of unanswered pings rebuilds the transport.
 const GATEWAY_LIVENESS_PROBE_TIMEOUT_MS = 5_000

-// Bounded self-heal for a failed REMOTE boot (#82679): when the primary boot
-// fails on a transient remote fault (dropped SSH/HTTP registered connection,
-// mint timeout — main tags those `retryable` on the boot progress), the
-// renderer re-attempts the whole boot with the same full-jitter backoff the
-// post-boot reconnect loop uses, up to this many attempts. Retries are
-// bounded and end in the real recovery affordance (the boot-failure overlay
-// with Retry / Settings), never an infinite spinner. Local failures and
-// confirmed reauth rejections never enter this loop — a missing capability
-// differs from a transient failure.
+// Bounded self-heal for a failed REMOTE boot (#82679): main classifies faults
+// before a descriptor exists; the renderer also classifies a valid remote
+// WebSocket dial that fails before becoming usable. The renderer re-attempts
+// the whole boot with the same full-jitter backoff the post-boot reconnect loop
+// uses, up to this many attempts. Retries are bounded and end in the real
+// recovery affordance (the boot-failure overlay with Retry / Settings), never
+// an infinite spinner. Local failures, mint/capability failures, and confirmed
+// reauth rejections never enter this loop.
 const BOOT_RETRY_MAX_ATTEMPTS = 5
 // Base delay for boot retries. Deliberately slower than the socket reconnect
 // loop's 300ms: each attempt may rebuild an SSH master + remote dashboard.
@@ -140,6 +139,16 @@ export function primaryRuntimeConnectionId(connection: Pick<HermesConnection, 'c
  return connection.mode === 'local' ? 'local' : null
 }

+function isGatewayWebSocketUrl(value: string): boolean {
+  try {
+    const url = new URL(value)
+
+    return url.protocol === 'ws:' || url.protocol === 'wss:'
+  } catch {
+    return false
+  }
+}
+
 interface GatewayBootOptions {
  beforeConnectionSwitch: () => void
  handleGatewayEvent: (event: RpcEvent) => void
@@ -1017,6 +1026,14 @@ export function useGatewayBoot({
    })

    async function boot() {
+      // These are historical facts for one boot attempt, not a late read of
+      // gateway.connectionState. A socket can close after a successful dial;
+      // later initialization errors must not be reclassified as boot dials.
+      let connectionDescriptorResolved = false
+      let resolvedRemoteConnection = false
+      let remoteGatewayConnectAttempted = false
+      let gatewayConnectSucceeded = false
+
      try {
        // A profile-pinned helper window (the HUD) dials its target profile's
        // backend directly — ensureBackend spawns/reuses it from the pool.
@@ -1035,6 +1052,9 @@ export function useGatewayBoot({
          return
        }

+        connectionDescriptorResolved = true
+        resolvedRemoteConnection = conn.mode === 'remote'
+
        setDesktopBootStep({
          phase: 'renderer.gateway.connect',
          message: translateNow('boot.steps.connectingGateway'),
@@ -1059,17 +1079,23 @@ export function useGatewayBoot({
        // Mint a fresh WS URL right before connecting. For OAuth gateways the
        // ticket is single-use with a short TTL, so the ticket baked into
        // conn.wsUrl is stale; resolveGatewayWsUrl() re-mints it rather than
-        // connecting with a dead ticket. Auth rejection asks for sign-in;
-        // connectivity failures remain retryable. Bounded like the reconnect
-        // path (#93454) so a wedged mint fails into boot retry instead of
-        // hanging "Starting Hermes…" forever.
+        // connecting with a dead ticket. Auth rejection asks for sign-in; the
+        // Electron mint path has its own bounded transport retries. This await
+        // is bounded like the reconnect path (#93454) so a wedged mint reaches
+        // the recovery affordance instead of hanging "Starting Hermes…".
        const wsUrl = await withTimeout(
          resolveGatewayWsUrl(desktop, conn),
          RECONNECT_ATTEMPT_TIMEOUT_MS,
          'Timed out minting the gateway WebSocket URL'
        )

+        // The complementary retry classification is deliberately narrower
+        // than every remote pre-open error: only a valid WebSocket dial after
+        // a resolved remote descriptor is transient here. URL and capability
+        // failures stay terminal at their own boundaries.
+        remoteGatewayConnectAttempted = resolvedRemoteConnection && isGatewayWebSocketUrl(wsUrl)
        await gateway.connect(wsUrl)
+        gatewayConnectSucceeded = true

        if (cancelled) {
          return
@@ -1116,15 +1142,25 @@ export function useGatewayBoot({
        if (!cancelled) {
          const message = err instanceof Error ? err.message : String(err)

-          // Transient remote failure (dropped SSH/HTTP registered connection,
-          // mint timeout): self-heal with bounded, jittered retries instead of
-          // parking on "Desktop boot failed" until the user re-enters the same
-          // connection details (#82679). Main already cleared the failed cached
-          // descriptor, so the next getConnection() rebuilds the connection —
-          // exactly what manual re-entry forced. Exhausted retries, local
-          // failures, and confirmed reauth rejections end in the real recovery
-          // affordance (the boot-failure overlay), never an infinite spinner.
-          if (bootRetryAttempt < BOOT_RETRY_MAX_ATTEMPTS && (await bootFailureIsRetryable()) && !cancelled) {
+          // Preserve the main-process classification for failures before a
+          // descriptor is available. Once this renderer has a descriptor, the
+          // only complementary retry is a transient, valid remote WebSocket
+          // dial which never became usable. A confirmed reauth rejection,
+          // invalid URL/capability failure, local descriptor, and anything
+          // after a successful dial retain the terminal recovery surface.
+          const retryableBeforeDescriptor = !connectionDescriptorResolved && (await bootFailureIsRetryable())
+
+          const retryableRemoteGatewayDial =
+            resolvedRemoteConnection &&
+            remoteGatewayConnectAttempted &&
+            !gatewayConnectSucceeded &&
+            !isGatewayReauthRequired(err)
+
+          if (
+            bootRetryAttempt < BOOT_RETRY_MAX_ATTEMPTS &&
+            (retryableBeforeDescriptor || retryableRemoteGatewayDial) &&
+            !cancelled
+          ) {
            const delay = reconnectBackoffDelayMs(bootRetryAttempt, { baseDelayMs: BOOT_RETRY_BASE_DELAY_MS })
            bootRetryAttempt += 1
            resumeDesktopBootForRetry(translateNow('boot.steps.retryingRemoteBackend'))