From 60e90ed6e700146039ef68e08ef764da4381bbd5 Mon Sep 17 00:00:00 2001 From: cervantesh <11169707+cervantesh@users.noreply.github.com> Date: Sat, 5 Sep 2026 15:50:03 -0400 Subject: [PATCH] fix(desktop): retry transient remote gateway boot dials --- .../gateway/hooks/use-gateway-boot.test.tsx | 208 +++++++++++++++++- .../src/app/gateway/hooks/use-gateway-boot.ts | 80 +++++-- 2 files changed, 265 insertions(+), 23 deletions(-) diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx index 4f6d214ca1..a6f01b4e67 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx @@ -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() + 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() + 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() + 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() + 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() + 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() + 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() + 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 () => { diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts index 5f3096316d..0f42cd2dd3 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts @@ -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 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'))