diff --git a/apps/desktop/src/api/client.ts b/apps/desktop/src/api/client.ts index e996ab010946..fbdd42a6d863 100644 --- a/apps/desktop/src/api/client.ts +++ b/apps/desktop/src/api/client.ts @@ -28,6 +28,10 @@ export const PROMPT_SUBMIT_REQUEST_TIMEOUT_MS = 1_800_000 export class HermesGateway extends JsonRpcGatewayClient { constructor() { super({ + // boot-failure-reauth.ts recognizes this canonical text and routes the + // failure overlay to the remote Gateway sign-in recovery path. + authRejectedErrorMessage: + 'Your remote gateway session has expired. Open Settings → Gateway and click "Sign in" again.', closedErrorMessage: 'Hermes gateway connection closed', connectErrorMessage: 'Could not connect to Hermes gateway', createRequestId: nextId => nextId, 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 a208ae57d971..07792e906063 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,7 +1,8 @@ import { act, cleanup, render } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { $desktopBoot } from '@/store/boot' +import type * as BootStore from '@/store/boot' +import { $desktopBoot, completeDesktopBoot } from '@/store/boot' import { closeSecondaryGateways, isActivePrimary } from '@/store/gateway' import { reconnectGateway } from '@/store/gateway-reconnect' import { $activeGatewayProfile, $profiles, ensureGatewayProfile } from '@/store/profile' @@ -10,6 +11,15 @@ import { $connection, $currentCwd, $gatewayState } from '@/store/session' import { takeGatewaySurvivor } from './gateway-hmr-survivor' import { useGatewayBoot } from './use-gateway-boot' +vi.mock('@/store/boot', async importOriginal => { + const actual = await importOriginal() + + return { + ...actual, + completeDesktopBoot: vi.fn(actual.completeDesktopBoot) + } +}) + // End-to-end-ish repro of the "remote VPS → stuck on CONNECTING, no Settings" // bug that drives the REAL useGatewayBoot hook + REAL HermesGateway through a // fake WebSocket we fully control. No Docker / no real port: from the desktop's @@ -25,13 +35,14 @@ type Listener = (ev: unknown) => void let connectionApplied: null | (() => void) = null // Minimal WebSocket stand-in implementing only what json-rpc-gateway.connect() -// touches: readyState, add/removeEventListener('open'|'error'|'close'), close(). +// touches: readyState, add/removeEventListener, close(), and readiness frames. class FakeWebSocket { static OPEN = 1 static CLOSED = 3 - // Flipped by the test: 'open' = next socket connects; 'fail' = next socket - // errors (a dead remote). Mirrors a VPS going away after the first connect. - static mode: 'open' | 'fail' = 'open' + // Flipped by the test: 'open' emits transport open + gateway.ready; 'fail' + // errors (a dead remote); 'reject' opens then closes during the handshake. + static mode: 'fail' | 'open' | 'reject' = 'open' + static rejectCode = 4401 static instances: FakeWebSocket[] = [] readyState = 0 @@ -39,13 +50,25 @@ class FakeWebSocket { constructor(public url: string) { FakeWebSocket.instances.push(this) - const willOpen = FakeWebSocket.mode === 'open' - // Resolve on the next microtask/macrotask so connect()'s promise wiring is - // in place before open/error fires (matches real async socket handshake). + const mode = FakeWebSocket.mode + // Resolve on the next task so connect()'s promise wiring is in place before + // open/error/close/readiness fires (matches the real async handshake). setTimeout(() => { - if (willOpen) { + if (mode === 'open') { + this.readyState = FakeWebSocket.OPEN + this.emit('open', {}) + this.emit('message', { + data: JSON.stringify({ + jsonrpc: '2.0', + method: 'event', + params: { type: 'gateway.ready' } + }) + }) + } else if (mode === 'reject') { this.readyState = FakeWebSocket.OPEN this.emit('open', {}) + this.readyState = FakeWebSocket.CLOSED + this.emit('close', { code: FakeWebSocket.rejectCode }) } else { this.readyState = FakeWebSocket.CLOSED this.emit('error', {}) @@ -63,13 +86,13 @@ class FakeWebSocket { close() { this.readyState = FakeWebSocket.CLOSED - this.emit('close', {}) + this.emit('close', { code: 1005 }) } // Force-drop an open socket, as a sleeping laptop / restarted remote would. drop() { this.readyState = FakeWebSocket.CLOSED - this.emit('close', {}) + this.emit('close', { code: 1006 }) } private emit(type: string, ev: unknown) { @@ -132,14 +155,21 @@ function fakeDesktop() { function Harness({ beforeConnectionSwitch = () => undefined, + onConnectionReady = () => undefined, + refreshHermesConfig = async () => undefined, refreshSessions -}: { beforeConnectionSwitch?: () => void; refreshSessions?: () => Promise } = {}) { +}: { + beforeConnectionSwitch?: () => void + onConnectionReady?: (connection: Parameters[0]['onConnectionReady']>[0]) => void + refreshHermesConfig?: () => Promise + refreshSessions?: () => Promise +} = {}) { useGatewayBoot({ beforeConnectionSwitch, handleGatewayEvent: () => undefined, - onConnectionReady: () => undefined, + onConnectionReady, onGatewayReady: () => undefined, - refreshHermesConfig: async () => undefined, + refreshHermesConfig, refreshSessions: refreshSessions ?? (async () => undefined) }) @@ -166,6 +196,7 @@ beforeEach(() => { $profiles.set([]) vi.useFakeTimers() FakeWebSocket.mode = 'open' + FakeWebSocket.rejectCode = 4401 FakeWebSocket.instances = [] connectionApplied = null ;(globalThis as { WebSocket: unknown }).WebSocket = FakeWebSocket @@ -181,6 +212,7 @@ beforeEach(() => { timestamp: Date.now(), visible: true }) + vi.mocked(completeDesktopBoot).mockClear() }) afterEach(() => { @@ -225,6 +257,16 @@ async function advanceBackoff() { }) } +function deferred() { + let resolve!: (value: T | PromiseLike) => void + + const promise = new Promise(next => { + resolve = next + }) + + return { promise, resolve } +} + describe('useGatewayBoot remote reconnect loop (real hook, fake socket)', () => { it('INITIAL boot against a dead VPS: getConnection hangs (waitForHermes) → app sits in the connecting combo, then fails', async () => { // The report's actual path: a fresh launch pointed at an unreachable VPS. @@ -635,4 +677,179 @@ describe('useGatewayBoot remote reconnect loop (real hook, fake socket)', () => await advanceBackoff() expect(desktop.getConnection).toHaveBeenCalledTimes(1) }) + + it('a 4401 boot handshake fails with the canonical reauth message and never retries', async () => { + const desktop = fakeDesktop() + desktop.getBootProgress = vi.fn(async () => ({ + error: 'transient remote failure', + fakeMode: false, + message: 'Desktop boot failed: transient remote failure', + phase: 'backend.error', + progress: 24, + retryable: true, + running: false, + timestamp: Date.now() + })) + ;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop + FakeWebSocket.mode = 'reject' + FakeWebSocket.rejectCode = 4401 + + render() + await flushAsync() + + expect($desktopBoot.get().error).toBe( + 'Your remote gateway session has expired. Open Settings → Gateway and click "Sign in" again.' + ) + expect(desktop.getConnection).toHaveBeenCalledTimes(1) + expect(FakeWebSocket.instances).toHaveLength(1) + + await advanceBackoff() + + expect(desktop.getConnection).toHaveBeenCalledTimes(1) + expect(FakeWebSocket.instances).toHaveLength(1) + }) + + it('a 4403 boot handshake fails generically and never retries', async () => { + const desktop = fakeDesktop() + desktop.getBootProgress = vi.fn(async () => ({ + error: 'transient remote failure', + fakeMode: false, + message: 'Desktop boot failed: transient remote failure', + phase: 'backend.error', + progress: 24, + retryable: true, + running: false, + timestamp: Date.now() + })) + ;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop + FakeWebSocket.mode = 'reject' + FakeWebSocket.rejectCode = 4403 + + render() + await flushAsync() + + expect($desktopBoot.get().error).toBe('Could not connect to Hermes gateway') + expect($desktopBoot.get().error).not.toMatch(/remote gateway session has expired/i) + expect(desktop.getConnection).toHaveBeenCalledTimes(1) + expect(FakeWebSocket.instances).toHaveLength(1) + + await advanceBackoff() + + expect(desktop.getConnection).toHaveBeenCalledTimes(1) + expect(FakeWebSocket.instances).toHaveLength(1) + }) + + it('a soft switch prevents a pending older boot from publishing, adopting, or completing', async () => { + const desktop = fakeDesktop() + + const staleBootConnection = { + authMode: 'token' as const, + baseUrl: 'https://stale.example.com', + profile: 'stale', + token: 'stale', + wsUrl: 'wss://stale.example.com/api/ws?token=stale' + } + + const switchConnection = { + authMode: 'token' as const, + baseUrl: 'https://switch.example.com', + profile: 'switch', + token: 'switch', + wsUrl: 'wss://switch.example.com/api/ws?token=switch' + } + + const pendingBootConnection = deferred() + + desktop.getConnection = vi + .fn() + .mockImplementationOnce(() => pendingBootConnection.promise) + .mockResolvedValue(switchConnection) as unknown as typeof desktop.getConnection + desktop.profile.get = vi.fn(async () => ({ profile: 'switch' })) + ;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop + + const onConnectionReady = vi.fn() + const refreshHermesConfig = vi.fn(async () => undefined) + const refreshSessions = vi.fn(async () => undefined) + + render( + + ) + await flushAsync() + + expect(desktop.getConnection).toHaveBeenCalledTimes(1) + expect(connectionApplied).not.toBeNull() + + act(() => connectionApplied?.()) + await flushAsync() + await flushAsync() + + expect($connection.get()).toEqual(switchConnection) + expect(onConnectionReady).toHaveBeenCalledTimes(1) + expect(desktop.profile.get).toHaveBeenCalledTimes(1) + expect(refreshHermesConfig).toHaveBeenCalledTimes(1) + expect(refreshSessions).toHaveBeenCalledTimes(1) + expect(completeDesktopBoot).toHaveBeenCalledTimes(1) + + await act(async () => { + pendingBootConnection.resolve(staleBootConnection) + await vi.advanceTimersByTimeAsync(0) + }) + + expect($connection.get()).toEqual(switchConnection) + expect(onConnectionReady).toHaveBeenCalledTimes(1) + expect(desktop.profile.get).toHaveBeenCalledTimes(1) + expect(refreshHermesConfig).toHaveBeenCalledTimes(1) + expect(refreshSessions).toHaveBeenCalledTimes(1) + expect(completeDesktopBoot).toHaveBeenCalledTimes(1) + expect(FakeWebSocket.instances.some(socket => socket.url === staleBootConnection.wsUrl)).toBe(false) + }) + + it('a queued retry from an older boot generation cannot restart after a soft switch', async () => { + const desktop = fakeDesktop() + desktop.getBootProgress = vi.fn(async () => ({ + error: 'Could not verify the existing SSH backend.', + fakeMode: false, + message: 'Desktop boot failed: Could not verify the existing SSH backend.', + phase: 'backend.error', + progress: 24, + retryable: true, + running: false, + timestamp: Date.now() + })) + ;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop + + FakeWebSocket.mode = 'fail' + render() + await flushAsync() + + expect(desktop.getConnection).toHaveBeenCalledTimes(1) + expect(FakeWebSocket.instances).toHaveLength(1) + expect($desktopBoot.get().error).toBeNull() + + // Model a timer callback already queued when softSwitch clears its handle. + // The generation check in the callback remains the final ownership gate. + const clearTimeoutSpy = vi.spyOn(globalThis, 'clearTimeout').mockImplementation(() => undefined) + + try { + FakeWebSocket.mode = 'open' + act(() => connectionApplied?.()) + await flushAsync() + await flushAsync() + + expect($gatewayState.get()).toBe('open') + expect(desktop.getConnection).toHaveBeenCalledTimes(2) + expect(FakeWebSocket.instances).toHaveLength(2) + + await advanceBackoff() + + expect(desktop.getConnection).toHaveBeenCalledTimes(2) + expect(FakeWebSocket.instances).toHaveLength(2) + } finally { + clearTimeoutSpy.mockRestore() + } + }) }) 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 0300430b30ed..ae22bd540f4f 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts @@ -166,6 +166,9 @@ export function useGatewayBoot({ // Bounded automatic boot retry for transient REMOTE failures (#82679). let bootRetryAttempt = 0 let bootRetryTimer: ReturnType | null = null + let bootGeneration = 0 + + const bootIsStale = (gen: number) => cancelled || gen !== bootGeneration const clearBootRetryTimer = () => { if (bootRetryTimer !== null) { @@ -326,26 +329,44 @@ export function useGatewayBoot({ // session id against the wrong backend — the HUD then falls back to the // default profile's last session (#82285). The override wins over the // stored preference; absent, behavior is unchanged. - async function adoptPrimaryProfile() { + async function adoptPrimaryProfile(shouldCommit: () => boolean = () => true) { const override = windowProfileOverride() try { const profileKey = override ?? (await desktop.profile?.get?.())?.profile ?? '' + + if (!shouldCommit()) { + return + } + const key = normalizeProfileKey(profileKey) $activeGatewayProfile.set(key) setPrimaryGateway(gateway, key) void ensureGatewayForProfile(key) } catch { + if (!shouldCommit()) { + return + } + $activeGatewayProfile.set(normalizeProfileKey(override)) } } // Seed the working dir from the backend default on a fresh view (nothing // open yet). Shared by boot + soft switch. - async function seedDefaultCwd() { + async function seedDefaultCwd(shouldCommit: () => boolean = () => true) { await ensureDefaultWorkspaceCwd() + + if (!shouldCommit()) { + return + } + const remoteDefault = await desktopDefaultCwd().catch(() => null) + if (!shouldCommit()) { + return + } + if (remoteDefault?.cwd && !$activeSessionId.get() && !$currentCwd.get()) { setCurrentCwd(remoteDefault.cwd) setCurrentBranch(remoteDefault.branch || '') @@ -359,6 +380,7 @@ export function useGatewayBoot({ return } + bootGeneration += 1 $gatewaySwitching.set(true) clearReconnectTimer() clearBootRetryTimer() @@ -629,14 +651,14 @@ export function useGatewayBoot({ }) }) - async function boot() { + async function boot(gen: number) { try { // A profile-pinned helper window (the HUD) dials its target profile's // backend directly — ensureBackend spawns/reuses it from the pool. // Everything else keeps dialing the primary. const conn = await desktop.getConnection(windowProfileOverride() ?? undefined) - if (cancelled) { + if (bootIsStale(gen)) { return } @@ -645,6 +667,11 @@ export function useGatewayBoot({ message: translateNow('boot.steps.connectingGateway'), progress: 95 }) + + if (bootIsStale(gen)) { + return + } + publish(conn) // Seed the workspace BEFORE the gateway opens: every session-restore @@ -656,19 +683,32 @@ export function useGatewayBoot({ // post-connect pass retries the sync. try { await ensureDefaultWorkspaceCwd() + + if (bootIsStale(gen)) { + return + } } catch (err) { console.warn('Failed to seed default workspace cwd pre-connect', err) } + if (bootIsStale(gen)) { + return + } + // 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. const wsUrl = await resolveGatewayWsUrl(desktop, conn) + + if (bootIsStale(gen)) { + return + } + await gateway.connect(wsUrl) - if (cancelled) { + if (bootIsStale(gen)) { return } @@ -677,7 +717,11 @@ export function useGatewayBoot({ // (cwd seed, config, sessions) are independent REST calls — running // them serially added their sum to time-to-populated-sidebar when only // the max is needed. - await adoptPrimaryProfile() + await adoptPrimaryProfile(() => !bootIsStale(gen)) + + if (bootIsStale(gen)) { + return + } setDesktopBootStep({ phase: 'renderer.config', @@ -685,11 +729,17 @@ export function useGatewayBoot({ progress: 97 }) + if (bootIsStale(gen)) { + return + } + await Promise.all([ // The pre-connect seed already applied the configured default; this // post-connect pass covers the remote backend default. Non-fatal: a // failed sync must not abort boot (the remembered cwd remains). - seedDefaultCwd().catch(err => console.warn('Failed to sync default workspace cwd post-connect', err)), + seedDefaultCwd(() => !bootIsStale(gen)).catch(err => + console.warn('Failed to sync default workspace cwd post-connect', err) + ), callbacksRef.current.refreshHermesConfig(), // Session-list population is never boot-fatal. The gateway WS is // already open by this point — a failed sidebar fetch (transient @@ -702,7 +752,7 @@ export function useGatewayBoot({ }) ]) - if (cancelled) { + if (bootIsStale(gen)) { return } @@ -710,34 +760,57 @@ export function useGatewayBoot({ bootCompleted = true bootRetryAttempt = 0 } catch (err) { - if (!cancelled) { - const message = err instanceof Error ? err.message : String(err) + if (bootIsStale(gen)) { + return + } - // 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) { + const message = err instanceof Error ? err.message : String(err) + const wsCloseCode = (err as { wsCloseCode?: number }).wsCloseCode + + const explicitRefusal = isGatewayReauthRequired(err) || wsCloseCode === 4400 || wsCloseCode === 4403 + + // 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/policy rejections end in the real + // recovery affordance (the boot-failure overlay), never an infinite + // spinner. + if (!explicitRefusal && bootRetryAttempt < BOOT_RETRY_MAX_ATTEMPTS) { + const retryable = await bootFailureIsRetryable() + + if (bootIsStale(gen)) { + return + } + + if (retryable) { const delay = reconnectBackoffDelayMs(bootRetryAttempt, { baseDelayMs: BOOT_RETRY_BASE_DELAY_MS }) bootRetryAttempt += 1 resumeDesktopBootForRetry(translateNow('boot.steps.retryingRemoteBackend')) clearBootRetryTimer() bootRetryTimer = setTimeout(() => { bootRetryTimer = null - void boot() + + if (gen !== bootGeneration || cancelled) { + return + } + + void boot(gen) }, delay) return } + } - failDesktopBoot(message) - notifyError(err, translateNow('boot.errors.desktopBootFailed')) - setSessionsLoading(false) + if (bootIsStale(gen)) { + return } + + failDesktopBoot(message) + notifyError(err, translateNow('boot.errors.desktopBootFailed')) + setSessionsLoading(false) } } @@ -775,7 +848,7 @@ export function useGatewayBoot({ if (adoptedFromHmr) { void adoptBoot() } else { - void boot() + void boot(++bootGeneration) } return () => { diff --git a/apps/desktop/src/lib/json-rpc-gateway-url-guard.test.ts b/apps/desktop/src/lib/json-rpc-gateway-url-guard.test.ts index ca978ed3d17b..fca0a945fd21 100644 --- a/apps/desktop/src/lib/json-rpc-gateway-url-guard.test.ts +++ b/apps/desktop/src/lib/json-rpc-gateway-url-guard.test.ts @@ -1,23 +1,78 @@ // connect() must reject before WebSocket coerces garbage into // `ws:///[object%20Object]` (#68250 stale-emit boot loop). -import { JsonRpcGatewayClient, JsonRpcGatewayError } from '@hermes/shared' +import { type GatewayEvent, isGatewayReauthRequired, JsonRpcGatewayClient, JsonRpcGatewayError } from '@hermes/shared' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +type FakeEvent = { + code?: number + data?: string +} + +type FakeListener = (event: FakeEvent) => void + +const gatewayReadyFrame = JSON.stringify({ + jsonrpc: '2.0', + method: 'event', + params: { type: 'gateway.ready' } +}) + class FakeSocket { static OPEN = 1 + static CLOSED = 3 + readyState = 0 - addEventListener = vi.fn((type: string, handler: () => void) => { - if (type === 'open') { - setTimeout(() => { - this.readyState = FakeSocket.OPEN - handler() - }, 0) + private listeners = new Map>() + + addEventListener = vi.fn((type: string, handler: FakeListener) => { + let handlers = this.listeners.get(type) + + if (!handlers) { + handlers = new Set() + this.listeners.set(type, handlers) } + + handlers.add(handler) + }) + + removeEventListener = vi.fn((type: string, handler: FakeListener) => { + this.listeners.get(type)?.delete(handler) }) - removeEventListener = vi.fn() - close = vi.fn() + + close = vi.fn(() => { + if (this.readyState === FakeSocket.CLOSED) { + return + } + + this.readyState = FakeSocket.CLOSED + this.emit('close', { code: 1005 }) + }) + send = vi.fn() + + emitOpen() { + this.readyState = FakeSocket.OPEN + this.emit('open', {}) + } + + emitMessage(data: string) { + this.emit('message', { data }) + } + + emitClose(code: number) { + this.readyState = FakeSocket.CLOSED + this.emit('close', { code }) + } + + emitError() { + this.emit('error', {}) + } + + private emit(type: string, event: FakeEvent) { + for (const handler of this.listeners.get(type) ?? []) { + handler(event) + } + } } describe('JsonRpcGatewayClient connect() URL guard', () => { @@ -26,6 +81,7 @@ describe('JsonRpcGatewayClient connect() URL guard', () => { }) afterEach(() => { + vi.useRealTimers() vi.unstubAllGlobals() }) @@ -55,13 +111,228 @@ describe('JsonRpcGatewayClient connect() URL guard', () => { expect(client.connectionState).toBe('idle') }) - it('accepts ws:// and wss://', async () => { + it('accepts ws:// and wss:// after gateway.ready', async () => { for (const url of ['ws://127.0.0.1:1234/api/ws?token=t', 'wss://gw.example.com/api/ws?ticket=t']) { - const client = new JsonRpcGatewayClient({ socketFactory: () => new FakeSocket() as unknown as WebSocket }) - await client.connect(url) + const socket = new FakeSocket() + + const client = new JsonRpcGatewayClient({ + socketFactory: () => socket as unknown as WebSocket + }) + + const connectPromise = client.connect(url) + + socket.emitOpen() + socket.emitMessage(gatewayReadyFrame) + + await connectPromise expect(client.connectionState).toBe('open') + client.close() } }) + + it('keeps the connection pending and connecting after raw open', async () => { + const socket = new FakeSocket() + + const client = new JsonRpcGatewayClient({ + socketFactory: () => socket as unknown as WebSocket + }) + + const connectPromise = client.connect('ws://127.0.0.1:1234/api/ws?token=t') + let resolved = false + + void connectPromise.then( + () => { + resolved = true + }, + () => undefined + ) + + socket.emitOpen() + await Promise.resolve() + await Promise.resolve() + + expect(resolved).toBe(false) + expect(client.connectionState).toBe('connecting') + + client.close() + await expect(connectPromise).rejects.toThrow('WebSocket closed') + }) + + it('opens on gateway.ready and dispatches the readiness event before resolving', async () => { + const socket = new FakeSocket() + + const client = new JsonRpcGatewayClient({ + socketFactory: () => socket as unknown as WebSocket + }) + + const events: GatewayEvent[] = [] + const offEvent = client.onEvent(event => events.push(event)) + const connectPromise = client.connect('ws://127.0.0.1:1234/api/ws?token=t') + + socket.emitOpen() + socket.emitMessage(gatewayReadyFrame) + await connectPromise + + expect(client.connectionState).toBe('open') + expect(events).toEqual([expect.objectContaining({ type: 'gateway.ready' })]) + offEvent() + }) + + it('rejects requests before gateway.ready without sending', async () => { + const socket = new FakeSocket() + + const client = new JsonRpcGatewayClient({ + notConnectedErrorMessage: 'Hermes gateway is not connected', + socketFactory: () => socket as unknown as WebSocket + }) + + const connectPromise = client.connect('ws://127.0.0.1:1234/api/ws?token=t') + + socket.emitOpen() + + await expect(client.request('session.list')).rejects.toThrow('Hermes gateway is not connected') + expect(socket.send).not.toHaveBeenCalled() + + client.close() + await expect(connectPromise).rejects.toThrow() + }) + + it('classifies a 4401 handshake close as requiring OAuth login', async () => { + const socket = new FakeSocket() + + const authRejectedErrorMessage = + 'Your remote gateway session has expired. Open Settings → Gateway and click "Sign in" again.' + + const client = new JsonRpcGatewayClient({ + authRejectedErrorMessage, + connectErrorMessage: 'Could not connect to Hermes gateway', + socketFactory: () => socket as unknown as WebSocket + }) + + const connectPromise = client.connect('wss://gw.example.com/api/ws?ticket=stale') + + socket.emitOpen() + socket.emitClose(4401) + + const error = await connectPromise.catch(reason => reason) + + expect(error).toEqual( + expect.objectContaining({ + message: authRejectedErrorMessage, + needsOauthLogin: true, + wsCloseCode: 4401 + }) + ) + expect(isGatewayReauthRequired(error)).toBe(true) + expect(client.connectionState).toBe('closed') + }) + + it('preserves a 4403 handshake close without classifying it as reauth', async () => { + const socket = new FakeSocket() + + const client = new JsonRpcGatewayClient({ + connectErrorMessage: 'Could not connect to Hermes gateway', + socketFactory: () => socket as unknown as WebSocket + }) + + const connectPromise = client.connect('wss://gw.example.com/api/ws?ticket=t') + + socket.emitOpen() + socket.emitClose(4403) + + const error = await connectPromise.catch(reason => reason) + + expect(error).toEqual( + expect.objectContaining({ + message: 'Could not connect to Hermes gateway', + wsCloseCode: 4403 + }) + ) + expect(isGatewayReauthRequired(error)).toBe(false) + expect(client.connectionState).toBe('closed') + }) + + it('rejects a non-ready first frame as a protocol failure', async () => { + const socket = new FakeSocket() + + const client = new JsonRpcGatewayClient({ + connectErrorMessage: 'Could not connect to Hermes gateway', + socketFactory: () => socket as unknown as WebSocket + }) + + const connectPromise = client.connect('ws://127.0.0.1:1234/api/ws?token=t') + + socket.emitOpen() + socket.emitMessage( + JSON.stringify({ + jsonrpc: '2.0', + method: 'event', + params: { type: 'session.event' } + }) + ) + + await expect(connectPromise).rejects.toThrow('Could not connect to Hermes gateway') + expect(client.connectionState).toBe('error') + expect(socket.close).toHaveBeenCalledOnce() + }) + + it('close() rejects an in-flight handshake and its timeout cannot poison the next connection', async () => { + vi.useFakeTimers() + + const sockets: FakeSocket[] = [] + + const client = new JsonRpcGatewayClient({ + closedErrorMessage: 'Hermes gateway connection closed', + socketFactory: () => { + const socket = new FakeSocket() + sockets.push(socket) + + return socket as unknown as WebSocket + } + }) + + const firstConnect = client.connect('ws://127.0.0.1:1234/api/ws?token=first') + sockets[0].emitOpen() + + client.close() + + await expect(firstConnect).rejects.toThrow('Hermes gateway connection closed') + expect(client.connectionState).toBe('closed') + + const secondConnect = client.connect('ws://127.0.0.1:1234/api/ws?token=second') + sockets[1].emitOpen() + sockets[1].emitMessage(gatewayReadyFrame) + await secondConnect + + expect(client.connectionState).toBe('open') + + await vi.advanceTimersByTimeAsync(15_000) + + expect(client.connectionState).toBe('open') + }) + + it('shares a same-URL attempt and rejects a different URL while connecting', async () => { + const socket = new FakeSocket() + + const client = new JsonRpcGatewayClient({ + socketFactory: () => socket as unknown as WebSocket + }) + + const firstConnect = client.connect('ws://127.0.0.1:1234/api/ws?token=t') + const sameUrlConnect = client.connect('ws://127.0.0.1:1234/api/ws?token=t') + + expect(sameUrlConnect).toBe(firstConnect) + + await expect(client.connect('ws://127.0.0.1:4321/api/ws?token=t')).rejects.toThrow( + 'gateway connect() already in progress' + ) + + socket.emitOpen() + socket.emitMessage(gatewayReadyFrame) + + await expect(Promise.all([firstConnect, sameUrlConnect])).resolves.toEqual([undefined, undefined]) + expect(client.connectionState).toBe('open') + }) }) describe('JsonRpcGatewayClient structured errors', () => { @@ -76,6 +347,7 @@ describe('JsonRpcGatewayClient structured errors', () => { setTimeout(() => { this.readyState = RespondingSocket.OPEN handler() + this.messageHandler?.({ data: gatewayReadyFrame }) }, 0) } diff --git a/apps/shared/src/index.ts b/apps/shared/src/index.ts index 51ec33fb6a64..38459fe95bbe 100644 --- a/apps/shared/src/index.ts +++ b/apps/shared/src/index.ts @@ -49,6 +49,7 @@ export { export { type ConnectionState, type GatewayClientOptions, + GatewayConnectError, type GatewayEvent, type GatewayEventName, type GatewayRequestId, diff --git a/apps/shared/src/json-rpc-gateway.ts b/apps/shared/src/json-rpc-gateway.ts index 8a2713a3eed2..57dca56a6cdd 100644 --- a/apps/shared/src/json-rpc-gateway.ts +++ b/apps/shared/src/json-rpc-gateway.ts @@ -64,6 +64,19 @@ export class JsonRpcGatewayError extends Error { } } +/** Connection-handshake failure with optional WebSocket close metadata. */ +export class GatewayConnectError extends Error { + readonly wsCloseCode?: number + readonly needsOauthLogin?: boolean + + constructor(message: string, options?: { wsCloseCode?: number; needsOauthLogin?: boolean }) { + super(message) + this.name = 'GatewayConnectError' + this.wsCloseCode = options?.wsCloseCode + this.needsOauthLogin = options?.needsOauthLogin + } +} + export type WebSocketLike = WebSocket type PendingCall = { @@ -72,7 +85,18 @@ type PendingCall = { timer?: ReturnType } +type ConnectAttempt = { + socket: WebSocketLike + url: string + promise: Promise + resolve: () => void + reject: (error: Error) => void + timer?: ReturnType + settled: boolean +} + export interface GatewayClientOptions { + authRejectedErrorMessage?: string closedErrorMessage?: string connectErrorMessage?: string connectTimeoutMs?: number @@ -97,15 +121,19 @@ export class JsonRpcGatewayClient { private pending = new Map() private socket: WebSocketLike | null = null private state: ConnectionState = 'idle' + private attempt: ConnectAttempt | null = null private readonly eventHandlers = new Map void>>() private readonly stateHandlers = new Set<(state: ConnectionState) => void>() private readonly options: Required> & Pick constructor(options: GatewayClientOptions = {}) { + const connectErrorMessage = options.connectErrorMessage ?? 'WebSocket connection failed' + this.options = { + authRejectedErrorMessage: options.authRejectedErrorMessage ?? connectErrorMessage, closedErrorMessage: options.closedErrorMessage ?? 'WebSocket closed', - connectErrorMessage: options.connectErrorMessage ?? 'WebSocket connection failed', + connectErrorMessage, connectTimeoutMs: options.connectTimeoutMs ?? DEFAULT_CONNECT_TIMEOUT_MS, createRequestId: options.createRequestId ?? ((nextId: number) => `${options.requestIdPrefix ?? 'r'}${nextId}`), notConnectedErrorMessage: options.notConnectedErrorMessage ?? 'gateway not connected', @@ -120,7 +148,7 @@ export class JsonRpcGatewayClient { return this.state } - async connect(wsUrl: string): Promise { + connect(wsUrl: string): Promise { // Refuse garbage; WebSocket coerces non-strings into // `ws:///[object%20Object]` (#68250 stale-emit boot loop). const invalidUrl = () => { @@ -130,7 +158,7 @@ export class JsonRpcGatewayClient { } if (typeof wsUrl !== 'string') { - throw invalidUrl() + return Promise.reject(invalidUrl()) } let url: URL @@ -138,111 +166,213 @@ export class JsonRpcGatewayClient { try { url = new URL(wsUrl) } catch { - throw invalidUrl() + return Promise.reject(invalidUrl()) } if (url.protocol !== 'ws:' && url.protocol !== 'wss:') { - throw invalidUrl() + return Promise.reject(invalidUrl()) } - if (this.socket?.readyState === WebSocket.OPEN || this.state === 'connecting') { - return + if (this.state === 'open' && this.socket?.readyState === WebSocket.OPEN) { + return Promise.resolve() + } + + if (this.attempt && !this.attempt.settled) { + if (this.attempt.url === wsUrl) { + return this.attempt.promise + } + + return Promise.reject(new Error('gateway connect() already in progress')) } this.setState('connecting') - const socket = this.options.socketFactory?.(wsUrl) ?? new WebSocket(wsUrl) + let socket: WebSocketLike + + try { + socket = this.options.socketFactory?.(wsUrl) ?? new WebSocket(wsUrl) + } catch { + this.setState('error') + + return Promise.reject(new GatewayConnectError(this.options.connectErrorMessage)) + } + this.socket = socket - socket.addEventListener('message', message => { - if (this.socket !== socket) { + let resolveAttempt!: () => void + let rejectAttempt!: (error: Error) => void + + const promise = new Promise((resolve, reject) => { + resolveAttempt = resolve + rejectAttempt = reject + }) + + const attempt: ConnectAttempt = { + socket, + url: wsUrl, + promise, + resolve: resolveAttempt, + reject: rejectAttempt, + settled: false + } + + this.attempt = attempt + + const onOpen = () => { + if (this.socket !== socket || this.attempt !== attempt || attempt.settled) { return } - this.handleMessage(message.data) - }) + // A raw WebSocket open is only transport readiness. The connection stays + // in 'connecting' until the gateway identifies itself with gateway.ready. + } - socket.addEventListener('close', event => { - if (this.socket !== socket) { + const onError = () => { + if (this.socket !== socket || this.attempt !== attempt || attempt.settled) { return } - if (this.options.onSocketClose(event)) { + if (!this.settleConnectAttempt(attempt)) { return } - this.socket = null - this.setState('closed') - this.rejectAllPending(new Error(this.options.closedErrorMessage)) - }) + this.setState('error') + attempt.reject(new GatewayConnectError(this.options.connectErrorMessage)) + } - await new Promise((resolve, reject) => { - let settled = false - let timer: ReturnType | undefined + socket.addEventListener('message', message => { + if (this.socket !== socket) { + return + } + + const frame = this.parseMessage(message.data) + + if (this.attempt === attempt && !attempt.settled) { + if (frame?.method === 'event' && frame.params?.type === 'gateway.ready') { + if (!this.settleConnectAttempt(attempt)) { + return + } + + this.setState('open') + this.dispatchEvent(frame.params) + attempt.resolve() + + return + } + + if (!this.settleConnectAttempt(attempt)) { + return + } + + this.setState('error') - const cleanup = () => { - if (timer !== undefined) { - clearTimeout(timer) + try { + socket.close() + } catch { + // ignore + } finally { + if (this.socket === socket) { + this.socket = null + } } - socket.removeEventListener('open', onOpen) - socket.removeEventListener('error', onError) + attempt.reject(new GatewayConnectError(this.options.connectErrorMessage)) + + return + } + + if (frame) { + this.handleFrame(frame) } + }) - const onOpen = () => { - if (settled || this.socket !== socket) { + socket.addEventListener('close', event => { + if (this.socket !== socket) { + return + } + + if (this.attempt === attempt && !attempt.settled) { + if (!this.settleConnectAttempt(attempt)) { return } - settled = true - cleanup() - this.setState('open') - resolve() + this.socket = null + this.setState('closed') + + const needsOauthLogin = event.code === 4401 + attempt.reject( + new GatewayConnectError( + needsOauthLogin ? this.options.authRejectedErrorMessage : this.options.connectErrorMessage, + { + wsCloseCode: event.code, + needsOauthLogin: needsOauthLogin || undefined + } + ) + ) + + return } - const onError = () => { - if (settled || this.socket !== socket) { + // onSocketClose is an established-connection interception hook. Handshake + // closes are classified above and never flow through it. + if (this.state === 'open') { + if (this.options.onSocketClose(event)) { return } - settled = true - cleanup() - this.setState('error') - reject(new Error(this.options.connectErrorMessage)) + this.socket = null + this.setState('closed') + this.rejectAllPending(new Error(this.options.closedErrorMessage)) + + return } - socket.addEventListener('open', onOpen, { once: true }) - socket.addEventListener('error', onError, { once: true }) + // A failed handshake may close after its error/protocol-failure path has + // already settled. Release that socket without overwriting 'error'. + this.socket = null + }) - if (this.options.connectTimeoutMs > 0) { - timer = setTimeout(() => { - if (settled) { - return - } + socket.addEventListener('open', onOpen, { once: true }) + socket.addEventListener('error', onError, { once: true }) - settled = true - cleanup() + if (this.options.connectTimeoutMs > 0) { + attempt.timer = setTimeout(() => { + if (this.socket !== socket || this.attempt !== attempt || attempt.settled) { + return + } - // Drop the half-open socket so the next connect() starts clean - // instead of short-circuiting on a zombie 'connecting' state. - if (this.socket === socket) { - try { - socket.close() - } catch { - // ignore - } + if (!this.settleConnectAttempt(attempt)) { + return + } + // Drop the half-open socket so the next connect() starts clean + // instead of short-circuiting on a zombie 'connecting' state. + try { + socket.close() + } catch { + // ignore + } finally { + if (this.socket === socket) { this.socket = null } + } - this.setState('error') - reject(new Error(this.options.connectErrorMessage)) - }, this.options.connectTimeoutMs) - } - }) + this.setState('error') + attempt.reject(new GatewayConnectError(this.options.connectErrorMessage)) + }, this.options.connectTimeoutMs) + } + + return promise } close(): void { + const attempt = this.attempt + + if (attempt && !attempt.settled && this.settleConnectAttempt(attempt)) { + this.setState('closed') + attempt.reject(new GatewayConnectError(this.options.closedErrorMessage)) + } + const socket = this.socket if (!socket) { @@ -294,7 +424,7 @@ export class JsonRpcGatewayClient { ): Promise { const socket = this.socket - if (!socket || socket.readyState !== WebSocket.OPEN) { + if (!socket || this.state !== 'open' || socket.readyState !== WebSocket.OPEN) { return Promise.reject(new Error(this.options.notConnectedErrorMessage)) } @@ -374,16 +504,24 @@ export class JsonRpcGatewayClient { }) } - private handleMessage(raw: unknown): void { + private parseMessage(raw: unknown): JsonRpcFrame | null { const text = typeof raw === 'string' ? raw : String(raw) - let frame: JsonRpcFrame + let parsed: unknown try { - frame = JSON.parse(text) as JsonRpcFrame + parsed = JSON.parse(text) } catch { - return + return null + } + + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { + return null } + return parsed as JsonRpcFrame + } + + private handleFrame(frame: JsonRpcFrame): void { if (frame.id !== undefined && frame.id !== null) { const call = this.pending.get(frame.id) @@ -412,6 +550,23 @@ export class JsonRpcGatewayClient { } } + private settleConnectAttempt(attempt: ConnectAttempt): boolean { + if (attempt.settled || this.attempt !== attempt) { + return false + } + + attempt.settled = true + + if (attempt.timer !== undefined) { + clearTimeout(attempt.timer) + attempt.timer = undefined + } + + this.attempt = null + + return true + } + private clearPending(id: GatewayRequestId): void { const call = this.pending.get(id) diff --git a/web/src/lib/gatewayClient.test.ts b/web/src/lib/gatewayClient.test.ts index 5ed55cac5848..b47fd6023fb7 100644 --- a/web/src/lib/gatewayClient.test.ts +++ b/web/src/lib/gatewayClient.test.ts @@ -52,6 +52,7 @@ class FakeWebSocket { type EventLike = { code?: number; + data?: string; }; beforeEach(() => { @@ -84,6 +85,13 @@ describe("GatewayClient", () => { const socket = FakeWebSocket.instances[0]; socket.readyState = 1; socket.emit("open", {}); + socket.emit("message", { + data: JSON.stringify({ + jsonrpc: "2.0", + method: "event", + params: { type: "gateway.ready" }, + }), + }); await connectPromise; socket.emit("close", { code: 4401 });