From 272bbaf7928e51c1f6c83c9aba0281b4fa8e7738 Mon Sep 17 00:00:00 2001 From: Gille <4317663+helix4u@users.noreply.github.com> Date: Mon, 20 Jul 2026 18:54:36 -0600 Subject: [PATCH] fix(desktop): avoid false remote gateway reauthentication (#68250) * fix(desktop): avoid false remote gateway reauthentication Co-authored-by: Rod-fernandez Co-authored-by: David Andrews (LexGenius.ai) * fix(desktop): harden remote revalidation state --------- Co-authored-by: Rod-fernandez Co-authored-by: David Andrews (LexGenius.ai) --- apps/desktop/AGENTS.md | 8 +- .../electron/connection-config.test.ts | 69 ++++- apps/desktop/electron/connection-config.ts | 62 ++++- apps/desktop/electron/main.ts | 64 ++--- apps/desktop/electron/remote-liveness.test.ts | 253 ++++++++++++++++++ apps/desktop/electron/remote-liveness.ts | 182 +++++++++++++ .../src/app/gateway/hooks/use-gateway-boot.ts | 13 +- .../app/gateway/hooks/use-gateway-request.ts | 7 +- apps/desktop/src/global.d.ts | 4 +- apps/desktop/src/lib/gateway-ws-url.test.ts | 55 +++- apps/shared/src/index.ts | 1 + apps/shared/src/websocket-url.ts | 50 +++- 12 files changed, 685 insertions(+), 83 deletions(-) create mode 100644 apps/desktop/electron/remote-liveness.test.ts create mode 100644 apps/desktop/electron/remote-liveness.ts diff --git a/apps/desktop/AGENTS.md b/apps/desktop/AGENTS.md index ca3a3d44d054..6908c2a42343 100644 --- a/apps/desktop/AGENTS.md +++ b/apps/desktop/AGENTS.md @@ -125,9 +125,11 @@ normalization alike. Learn the shape, not a snapshot of the current rungs. Two auth-flavored corollaries worth naming because they are easy to get wrong: - **One-time credentials are never reused.** An OAuth gateway connection mints a - fresh WebSocket ticket on every dial; a mint failure means reauthentication, - not "fall back to the cached URL." Only long-lived token/local auth may reuse - a cached URL as a lower rung. + fresh WebSocket ticket on every dial and never falls back to the cached URL. + Only a confirmed 401/403 (or an explicitly tagged auth rejection) means + reauthentication; timeout, network, malformed-response, and server failures + remain connectivity errors. Only long-lived token/local auth may reuse a + cached URL as a lower rung. - **A connection test must exercise the leg you'll actually use.** An HTTP status probe passing while the WebSocket/auth leg fails is a false positive that ships as "it said connected but nothing works." diff --git a/apps/desktop/electron/connection-config.test.ts b/apps/desktop/electron/connection-config.test.ts index 425e63b15f21..be7d6228d20f 100644 --- a/apps/desktop/electron/connection-config.test.ts +++ b/apps/desktop/electron/connection-config.test.ts @@ -23,6 +23,9 @@ import { cookiesHaveLiveSession, cookiesHavePrivySession, cookiesHaveSession, + gatewayTicketFailure, + gatewayWsUrlIpcResult, + isGatewayAuthRejection, modeIsRemoteLike, normalizeRemoteBaseUrl, normAuthMode, @@ -431,12 +434,14 @@ test('resolveTestWsUrl (oauth, mint ok) builds a ?ticket= URL', async () => { assert.equal(url, 'wss://gw.example.com/api/ws?ticket=tkt-9') }) -test('resolveTestWsUrl (oauth, mint FAILS) throws — must NOT skip WS validation', async () => { +test('resolveTestWsUrl (oauth, auth rejected) requests sign-in and does not skip WS validation', async () => { + const cause = Object.assign(new Error('ticket mint failed'), { statusCode: 401 }) + await assert.rejects( () => resolveTestWsUrl('https://gw.example.com', 'oauth', null, { mintTicket: async () => { - throw new Error('401 ticket mint failed') + throw cause } }), (err: any) => { @@ -452,6 +457,66 @@ test('resolveTestWsUrl (oauth, mint FAILS) throws — must NOT skip WS validatio ) }) +test('resolveTestWsUrl (oauth, transport failure) remains a retryable connection error', async () => { + const cause = new Error('socket timed out') + + await assert.rejects( + () => + resolveTestWsUrl('https://gw.example.com', 'oauth', null, { + mintTicket: async () => { + throw cause + } + }), + (err: any) => { + assert.match(err.message, /could not mint a WebSocket ticket/i) + assert.equal(err.needsOauthLogin, undefined) + assert.equal(err.cause, cause) + + return true + } + ) +}) + +test('gateway ticket failures classify only explicit auth rejection statuses as reauth', () => { + assert.equal(isGatewayAuthRejection({ statusCode: 401 }), true) + assert.equal(isGatewayAuthRejection({ statusCode: 403 }), true) + assert.equal(isGatewayAuthRejection({ needsOauthLogin: true }), true) + assert.equal(isGatewayAuthRejection({ statusCode: 500 }), false) + assert.equal(isGatewayAuthRejection(new Error('network timeout')), false) + + const serverFailure = gatewayTicketFailure(new Error('network timeout'), 'sign in', 'retry connection') as any + assert.equal(serverFailure.message, 'retry connection') + assert.equal(serverFailure.needsOauthLogin, undefined) +}) + +test('gateway WS URL IPC result serializes success and the auth-vs-transport matrix', async () => { + assert.deepEqual(await gatewayWsUrlIpcResult(async () => 'wss://gateway.example.com/api/ws?ticket=fresh'), { + ok: true, + wsUrl: 'wss://gateway.example.com/api/ws?ticket=fresh' + }) + + for (const statusCode of [401, 403]) { + const error = Object.assign(new Error(`${statusCode}: rejected`), { statusCode }) + + assert.deepEqual(await gatewayWsUrlIpcResult(async () => Promise.reject(error)), { + error: `${statusCode}: rejected`, + needsOauthLogin: true, + ok: false + }) + } + + for (const error of [ + Object.assign(new Error('500: unavailable'), { statusCode: 500 }), + new Error('Timed out connecting to Hermes backend after 8000ms'), + Object.assign(new Error('socket reset'), { code: 'ECONNRESET' }) + ]) { + assert.deepEqual(await gatewayWsUrlIpcResult(async () => Promise.reject(error)), { + error: error.message, + ok: false + }) + } +}) + test('resolveTestWsUrl (oauth) requires a mintTicket function', async () => { await assert.rejects( () => resolveTestWsUrl('https://gw.example.com', 'oauth', null), diff --git a/apps/desktop/electron/connection-config.ts b/apps/desktop/electron/connection-config.ts index 569f9cc07261..5a04da406cd5 100644 --- a/apps/desktop/electron/connection-config.ts +++ b/apps/desktop/electron/connection-config.ts @@ -88,6 +88,43 @@ function buildGatewayWsUrlWithTicket(baseUrl, ticket) { return `${wsScheme}://${parsed.host}${prefix}/api/ws?ticket=${encodeURIComponent(ticket)}` } +/** True only when a gateway explicitly rejected the current OAuth session. */ +function isGatewayAuthRejection(error) { + if (error && typeof error === 'object' && (error as any).needsOauthLogin === true) { + return true + } + + const statusCode = Number(error && typeof error === 'object' ? (error as any).statusCode : NaN) + + return statusCode === 401 || statusCode === 403 +} + +function gatewayTicketFailure(error, authMessage, transportMessage) { + const needsOauthLogin = isGatewayAuthRejection(error) + const err = new Error(needsOauthLogin ? authMessage : transportMessage) + + if (needsOauthLogin) { + ;(err as any).needsOauthLogin = true + } + + err.cause = error + + return err +} + +/** Serialize a fresh-WS-URL attempt across Electron's IPC boundary. */ +async function gatewayWsUrlIpcResult(resolveWsUrl: () => Promise) { + try { + return { ok: true as const, wsUrl: await resolveWsUrl() } + } catch (error) { + return { + error: error instanceof Error ? error.message : String(error), + ...(isGatewayAuthRejection(error) ? { needsOauthLogin: true as const } : {}), + ok: false as const + } + } +} + /** * Build the WS URL the renderer would connect with, so the connection test can * exercise the same transport the app actually uses. @@ -102,12 +139,10 @@ function buildGatewayWsUrlWithTicket(baseUrl, ticket) { * - oauth, mint ok → ws(s)://…/api/ws?ticket=… * - oauth, mint fails → THROWS (NOT a skip) * - * The oauth-mint-failure throw is the important case: the real boot path - * (resolveRemoteBackend in main.ts) treats a mint failure as a hard - * "session expired" auth error and refuses to connect. Swallowing it here - * would re-introduce the exact false-positive this test exists to catch — - * HTTP /api/status passes, the test reports "reachable", then the renderer - * can't authenticate /api/ws and boot dies with "Could not connect". + * The oauth-mint-failure throw is the important case: swallowing it here would + * re-introduce the exact false-positive this test exists to catch. An explicit + * 401/403 asks for sign-in; transport and server failures remain connectivity + * errors so a temporary outage is not mislabeled as an expired session. * * @param {string} baseUrl * @param {'token'|'oauth'} authMode @@ -128,14 +163,12 @@ async function resolveTestWsUrl(baseUrl, authMode, token, deps: any = {}) { try { ticket = await mintTicket(baseUrl) } catch (error) { - const err = new Error( - 'Reached the gateway over HTTP, but could not mint a WebSocket ticket for the OAuth session ' + - '(it may have expired). Open Settings → Gateway and sign in again.' + throw gatewayTicketFailure( + error, + 'Reached the gateway over HTTP, but the OAuth session was rejected while minting a WebSocket ticket. ' + + 'Open Settings → Gateway and sign in again.', + 'Reached the gateway over HTTP, but could not mint a WebSocket ticket. Check the remote gateway connection and try again.' ) - - ;(err as any).needsOauthLogin = true - err.cause = error - throw err } return buildGatewayWsUrlWithTicket(baseUrl, ticket) @@ -337,6 +370,9 @@ export { cookiesHaveLiveSession, cookiesHavePrivySession, cookiesHaveSession, + gatewayTicketFailure, + gatewayWsUrlIpcResult, + isGatewayAuthRejection, modeIsRemoteLike, normalizeRemoteBaseUrl, normAuthMode, diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 141497cf6b4d..4d7108d9f794 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -47,6 +47,8 @@ import { cookiesHaveLiveSession, cookiesHavePrivySession, cookiesHaveSession, + gatewayTicketFailure, + gatewayWsUrlIpcResult, modeIsRemoteLike, normalizeRemoteBaseUrl, normAuthMode, @@ -109,6 +111,7 @@ import { ensureMainWindow } from './main-window-lifecycle' import { serializeJsonBody, setJsonRequestHeaders } from './oauth-net-request' import { createKeepAwake } from './power-save' import { decideProfileDeleteAction, profileNameFromDeleteRequest, resolveRouteProfile } from './profile-delete-routing' +import { RemoteLivenessTracker, RemoteRevalidationCoordinator, revalidateRemoteConnection } from './remote-liveness' import { buildSessionWindowUrl, chatWindowWebPreferences, @@ -935,6 +938,8 @@ function registerMediaProtocol() { let mainWindow = null const backendConnectionState = createBackendConnectionState, any>() +const remoteLiveness = new RemoteLivenessTracker() +const remoteRevalidation = new RemoteRevalidationCoordinator() // True while connection-config:apply soft-rehomes the primary — suppresses the // backend-exit toast so an intentional kill doesn't look like a crash. let softRehomeInProgress = false @@ -6333,13 +6338,11 @@ async function buildRemoteConnection(rawUrl, authMode, token, source) { try { ticket = await mintGatewayWsTicket(baseUrl) } catch (error) { - const err = new Error( - 'Your remote gateway session has expired. ' + 'Open Settings → Gateway and click "Sign in" again.' - ) as any - - err.needsOauthLogin = true - err.cause = error - throw err + throw gatewayTicketFailure( + error, + 'Your remote gateway session has expired. Open Settings → Gateway and click "Sign in" again.', + 'Could not reach the remote Hermes gateway while refreshing its WebSocket ticket. Try reconnecting.' + ) } return { @@ -6621,6 +6624,7 @@ function stopBackendChild(child) { // switch / crash recovery), which still resets boot progress + reloads. function resetHermesConnection({ soft = false } = {}) { backendStartFailure = null + remoteLiveness.clear() const hermesProcess = backendConnectionState.invalidate() stopBackendChild(hermesProcess) @@ -7857,42 +7861,28 @@ ipcMain.handle('hermes:connection:revalidate', async () => { return { ok: true, rebuilt: false } } - let conn = null - - try { - conn = await connectionPromise - } catch { - // The cached boot already rejected (its own catch clears the promise); - // nothing to revalidate — the next getConnection() builds fresh. - return { ok: true, rebuilt: false } - } - - if (!conn || conn.mode !== 'remote' || !conn.baseUrl) { - return { ok: true, rebuilt: false } - } - - const base = conn.baseUrl.replace(/\/+$/, '') - - try { - await fetchPublicJson(`${base}/api/status`, { timeoutMs: 2_500 }) - - return { ok: true, rebuilt: false } - } catch { - // Unreachable remote: drop the stale cache so the renderer's next reconnect - // tick rebuilds a fresh, reachable descriptor. resetHermesConnection only - // clears the connection promise for a remote (no child to SIGTERM). - rememberLog('Cached remote Hermes backend failed liveness probe; dropping stale connection.') - resetHermesConnection() - - return { ok: true, rebuilt: true } - } + // Main and every session pop-out have their own renderer reconnect loop but + // share this primary connection. Coalesce simultaneous requests so one outage + // produces one failure observation rather than exhausting the whole streak. + return remoteRevalidation.run(connectionPromise, () => + revalidateRemoteConnection({ + connectionPromise, + currentConnectionPromise: () => backendConnectionState.getPromise(), + log: rememberLog, + probe: fetchPublicJson, + resetConnection: resetHermesConnection, + tracker: remoteLiveness + }) + ) }) ipcMain.handle('hermes:backend:touch', async (_event, profile) => { touchPoolBackend(profile) return { ok: true } }) -ipcMain.handle('hermes:gateway:ws-url', async (_event, profile) => freshGatewayWsUrl(profile)) +ipcMain.handle('hermes:gateway:ws-url', async (_event, profile) => { + return gatewayWsUrlIpcResult(() => freshGatewayWsUrl(profile)) +}) ipcMain.handle('hermes:window:openSession', async (_event, sessionId, opts) => { if (typeof sessionId !== 'string' || !sessionId.trim()) { return { ok: false, error: 'invalid-session-id' } diff --git a/apps/desktop/electron/remote-liveness.test.ts b/apps/desktop/electron/remote-liveness.test.ts new file mode 100644 index 000000000000..6c52e11f69f9 --- /dev/null +++ b/apps/desktop/electron/remote-liveness.test.ts @@ -0,0 +1,253 @@ +import { describe, expect, it, vi } from 'vitest' + +import { + REMOTE_LIVENESS_FAILURE_LIMIT, + REMOTE_LIVENESS_FAILURE_WINDOW_MS, + REMOTE_LIVENESS_TIMEOUT_MS, + RemoteLivenessTracker, + RemoteRevalidationCoordinator, + revalidateRemoteConnection +} from './remote-liveness' + +describe('RemoteLivenessTracker', () => { + it('requires consecutive failures before resetting a connection', () => { + const tracker = new RemoteLivenessTracker() + + for (let failures = 1; failures < REMOTE_LIVENESS_FAILURE_LIMIT; failures += 1) { + expect(tracker.recordFailure('https://gateway.example.com')).toEqual({ failures, shouldReset: false }) + } + + expect(tracker.recordFailure('https://gateway.example.com')).toEqual({ + failures: REMOTE_LIVENESS_FAILURE_LIMIT, + shouldReset: true + }) + }) + + it('clears a failure streak after a successful probe', () => { + const tracker = new RemoteLivenessTracker() + + tracker.recordFailure('https://gateway.example.com') + tracker.recordFailure('https://gateway.example.com') + tracker.recordSuccess('https://gateway.example.com') + + expect(tracker.recordFailure('https://gateway.example.com')).toEqual({ failures: 1, shouldReset: false }) + }) + + it('tracks different gateways independently', () => { + const tracker = new RemoteLivenessTracker(2) + + expect(tracker.recordFailure('https://one.example.com')).toEqual({ failures: 1, shouldReset: false }) + expect(tracker.recordFailure('https://two.example.com')).toEqual({ failures: 1, shouldReset: false }) + expect(tracker.recordFailure('https://one.example.com')).toEqual({ failures: 2, shouldReset: true }) + expect(tracker.recordFailure('https://two.example.com')).toEqual({ failures: 2, shouldReset: true }) + }) + + it('clears only the successful gateway streak', () => { + const tracker = new RemoteLivenessTracker(3) + + tracker.recordFailure('https://one.example.com') + tracker.recordFailure('https://two.example.com') + tracker.recordSuccess('https://one.example.com') + + expect(tracker.recordFailure('https://one.example.com')).toEqual({ failures: 1, shouldReset: false }) + expect(tracker.recordFailure('https://two.example.com')).toEqual({ failures: 2, shouldReset: false }) + }) + + it('does not accumulate isolated failures across separate reconnect episodes', () => { + let now = 0 + const tracker = new RemoteLivenessTracker(3, REMOTE_LIVENESS_FAILURE_WINDOW_MS, () => now) + + expect(tracker.recordFailure('https://gateway.example.com')).toEqual({ failures: 1, shouldReset: false }) + now += REMOTE_LIVENESS_FAILURE_WINDOW_MS + 1 + expect(tracker.recordFailure('https://gateway.example.com')).toEqual({ failures: 1, shouldReset: false }) + }) + + it('clears all failure streaks when the connection state resets', () => { + const tracker = new RemoteLivenessTracker(3) + + tracker.recordFailure('https://one.example.com') + tracker.recordFailure('https://two.example.com') + tracker.clear() + + expect(tracker.recordFailure('https://one.example.com')).toEqual({ failures: 1, shouldReset: false }) + expect(tracker.recordFailure('https://two.example.com')).toEqual({ failures: 1, shouldReset: false }) + }) + + it('starts a fresh streak after the reset threshold is consumed', () => { + const tracker = new RemoteLivenessTracker(1) + + expect(tracker.recordFailure('https://gateway.example.com')).toEqual({ failures: 1, shouldReset: true }) + expect(tracker.recordFailure('https://gateway.example.com')).toEqual({ failures: 1, shouldReset: true }) + }) + + it('rejects invalid failure limits', () => { + expect(() => new RemoteLivenessTracker(0)).toThrow(/positive integer/i) + expect(() => new RemoteLivenessTracker(1.5)).toThrow(/positive integer/i) + expect(() => new RemoteLivenessTracker(1, 0)).toThrow(/window must be positive/i) + }) +}) + +describe('RemoteRevalidationCoordinator', () => { + it('coalesces simultaneous probes for the same cached connection', async () => { + const coordinator = new RemoteRevalidationCoordinator() + const connection = Promise.resolve({ baseUrl: 'https://gateway.example.com' }) + let resolveProbe: (value: string) => void = () => undefined + + const probe = vi.fn( + () => + new Promise(resolve => { + resolveProbe = resolve + }) + ) + + const first = coordinator.run(connection, probe) + const second = coordinator.run(connection, probe) + const third = coordinator.run(connection, probe) + + await Promise.resolve() + + expect(second).toBe(first) + expect(third).toBe(first) + expect(probe).toHaveBeenCalledOnce() + + resolveProbe('healthy') + await expect(Promise.all([first, second, third])).resolves.toEqual(['healthy', 'healthy', 'healthy']) + }) + + it('runs a fresh probe after the prior one settles', async () => { + const coordinator = new RemoteRevalidationCoordinator() + const connection = Promise.resolve({ baseUrl: 'https://gateway.example.com' }) + const probe = vi.fn().mockResolvedValue('healthy') + + await coordinator.run(connection, probe) + await coordinator.run(connection, probe) + + expect(probe).toHaveBeenCalledTimes(2) + }) + + it('does not coalesce different cached connections', async () => { + const coordinator = new RemoteRevalidationCoordinator() + const probe = vi.fn().mockResolvedValue('healthy') + + await Promise.all([coordinator.run(Promise.resolve('one'), probe), coordinator.run(Promise.resolve('two'), probe)]) + + expect(probe).toHaveBeenCalledTimes(2) + }) + + it('cleans up a rejected probe so it can be retried', async () => { + const coordinator = new RemoteRevalidationCoordinator() + const connection = Promise.resolve({ baseUrl: 'https://gateway.example.com' }) + const probe = vi.fn().mockRejectedValueOnce(new Error('offline')).mockResolvedValueOnce('healthy') + + await expect(coordinator.run(connection, probe)).rejects.toThrow('offline') + await expect(coordinator.run(connection, probe)).resolves.toBe('healthy') + expect(probe).toHaveBeenCalledTimes(2) + }) +}) + +describe('revalidateRemoteConnection', () => { + function harness(overrides: Record = {}) { + const connection = { baseUrl: 'https://gateway.example.com/', mode: 'remote' } + const connectionPromise = Promise.resolve(connection) + const current = { promise: connectionPromise as null | Promise } + const log = vi.fn() + const probe = vi.fn().mockResolvedValue({ ok: true }) + const resetConnection = vi.fn() + const tracker = new RemoteLivenessTracker() + + return { + connectionPromise, + current, + log, + options: { + connectionPromise, + currentConnectionPromise: () => current.promise, + log, + probe, + resetConnection, + tracker, + ...overrides + }, + probe, + resetConnection, + tracker + } + } + + it('probes the normalized status URL with the production timeout', async () => { + const test = harness() + + await expect(revalidateRemoteConnection(test.options)).resolves.toEqual({ ok: true, rebuilt: false }) + expect(test.probe).toHaveBeenCalledWith('https://gateway.example.com/api/status', { + timeoutMs: REMOTE_LIVENESS_TIMEOUT_MS + }) + expect(test.resetConnection).not.toHaveBeenCalled() + }) + + it('keeps failures one and two, then resets on the third failure', async () => { + const probe = vi.fn().mockRejectedValue(new Error('offline')) + const test = harness({ probe }) + + await expect(revalidateRemoteConnection(test.options)).resolves.toEqual({ ok: true, rebuilt: false }) + await expect(revalidateRemoteConnection(test.options)).resolves.toEqual({ ok: true, rebuilt: false }) + await expect(revalidateRemoteConnection(test.options)).resolves.toEqual({ ok: true, rebuilt: true }) + + expect(probe).toHaveBeenCalledTimes(3) + expect(test.resetConnection).toHaveBeenCalledOnce() + expect(test.log).toHaveBeenNthCalledWith(1, expect.stringContaining('(1/3)')) + expect(test.log).toHaveBeenNthCalledWith(2, expect.stringContaining('(2/3)')) + expect(test.log).toHaveBeenLastCalledWith(expect.stringContaining('dropping stale connection')) + }) + + it('ignores a late failed probe after the cached connection is replaced', async () => { + let rejectProbe: (error: Error) => void = () => undefined + + const probe = vi.fn( + () => + new Promise((_resolve, reject) => { + rejectProbe = reject + }) + ) + + const test = harness({ probe }) + const pending = revalidateRemoteConnection(test.options) + + await Promise.resolve() + test.current.promise = Promise.resolve({ baseUrl: 'https://new.example.com', mode: 'remote' }) + rejectProbe(new Error('old connection failed')) + + await expect(pending).resolves.toEqual({ ok: true, rebuilt: false }) + expect(test.resetConnection).not.toHaveBeenCalled() + expect(test.log).not.toHaveBeenCalled() + expect(test.tracker.recordFailure('https://gateway.example.com')).toEqual({ failures: 1, shouldReset: false }) + }) + + it('does not probe a local, rejected, or already replaced connection', async () => { + const replaced = harness() + + replaced.current.promise = null + await expect(revalidateRemoteConnection(replaced.options)).resolves.toEqual({ ok: true, rebuilt: false }) + expect(replaced.probe).not.toHaveBeenCalled() + + const localConnection = { baseUrl: 'http://127.0.0.1:3000', mode: 'local' } + const localPromise = Promise.resolve(localConnection) + + const local = harness({ + connectionPromise: localPromise, + currentConnectionPromise: () => localPromise + }) + + await expect(revalidateRemoteConnection(local.options)).resolves.toEqual({ ok: true, rebuilt: false }) + expect(local.probe).not.toHaveBeenCalled() + + const rejectedPromise = Promise.reject(new Error('boot failed')) + + const rejected = harness({ + connectionPromise: rejectedPromise, + currentConnectionPromise: () => rejectedPromise + }) + + await expect(revalidateRemoteConnection(rejected.options)).resolves.toEqual({ ok: true, rebuilt: false }) + expect(rejected.probe).not.toHaveBeenCalled() + }) +}) diff --git a/apps/desktop/electron/remote-liveness.ts b/apps/desktop/electron/remote-liveness.ts new file mode 100644 index 000000000000..eedc16682e7d --- /dev/null +++ b/apps/desktop/electron/remote-liveness.ts @@ -0,0 +1,182 @@ +export const REMOTE_LIVENESS_TIMEOUT_MS = 10_000 +export const REMOTE_LIVENESS_FAILURE_LIMIT = 3 +// Even at the capped retry path, consecutive liveness observations are at most +// about 48s apart (ticket mint + socket open + backoff + the next status probe). +// One minute keeps a continuous outage together without carrying old failures. +export const REMOTE_LIVENESS_FAILURE_WINDOW_MS = 60_000 + +export interface RemoteLivenessFailure { + failures: number + shouldReset: boolean +} + +interface RemoteConnectionDescriptor { + baseUrl?: null | string + mode?: null | string +} + +export interface RevalidateRemoteConnectionOptions { + connectionPromise: Promise + currentConnectionPromise: () => null | Promise + log: (message: string) => void + probe: (url: string, options: { timeoutMs: number }) => Promise + resetConnection: () => void + tracker: RemoteLivenessTracker +} + +export interface RemoteRevalidationResult { + ok: true + rebuilt: boolean +} + +/** + * Coalesces revalidation work for one cached connection promise. + * + * Every Desktop BrowserWindow owns a renderer gateway loop. When several + * windows observe the same disconnect they can all ask the Electron main + * process to revalidate the shared primary connection at once. Those calls + * must count as one probe, not several consecutive failures. + */ +export class RemoteRevalidationCoordinator { + readonly #inflightByConnection = new WeakMap>() + + run(connection: object, task: () => Promise): Promise { + const existing = this.#inflightByConnection.get(connection) as Promise | undefined + + if (existing) { + return existing + } + + const pending = Promise.resolve().then(task) + + const clear = () => { + if (this.#inflightByConnection.get(connection) === pending) { + this.#inflightByConnection.delete(connection) + } + } + + this.#inflightByConnection.set(connection, pending) + // Clean up on both outcomes without creating an unhandled rejected branch. + void pending.then(clear, clear) + + return pending + } +} + +/** + * Tracks consecutive remote liveness failures independently per gateway. + * A successful probe clears the streak, and reaching the limit consumes it so + * a rebuilt connection starts from a clean state. + */ +export class RemoteLivenessTracker { + readonly #failureLimit: number + readonly #failureWindowMs: number + readonly #failuresByBaseUrl = new Map() + readonly #now: () => number + + constructor( + failureLimit = REMOTE_LIVENESS_FAILURE_LIMIT, + failureWindowMs = REMOTE_LIVENESS_FAILURE_WINDOW_MS, + now: () => number = Date.now + ) { + if (!Number.isInteger(failureLimit) || failureLimit < 1) { + throw new Error('Remote liveness failure limit must be a positive integer.') + } + + if (!Number.isFinite(failureWindowMs) || failureWindowMs < 1) { + throw new Error('Remote liveness failure window must be positive.') + } + + this.#failureLimit = failureLimit + this.#failureWindowMs = failureWindowMs + this.#now = now + } + + recordSuccess(baseUrl: string): void { + this.#failuresByBaseUrl.delete(baseUrl) + } + + recordFailure(baseUrl: string): RemoteLivenessFailure { + const now = this.#now() + const previous = this.#failuresByBaseUrl.get(baseUrl) + const withinFailureWindow = previous && now - previous.lastFailureAt <= this.#failureWindowMs + const failures = (withinFailureWindow ? previous.failures : 0) + 1 + const shouldReset = failures >= this.#failureLimit + + if (shouldReset) { + this.#failuresByBaseUrl.delete(baseUrl) + } else { + this.#failuresByBaseUrl.set(baseUrl, { failures, lastFailureAt: now }) + } + + return { failures, shouldReset } + } + + clear(): void { + this.#failuresByBaseUrl.clear() + } +} + +/** + * Probe the cached primary remote connection and apply the failure policy. + * The caller owns single-flight coordination; identity checks here ensure an + * old async result cannot mutate or reset a replacement connection. + */ +export async function revalidateRemoteConnection({ + connectionPromise, + currentConnectionPromise, + log, + probe, + resetConnection, + tracker +}: RevalidateRemoteConnectionOptions): Promise { + let connection: TConnection + + try { + connection = await connectionPromise + } catch { + // The cached boot already rejected; its own recovery path will clear it. + return { ok: true, rebuilt: false } + } + + if (currentConnectionPromise() !== connectionPromise) { + return { ok: true, rebuilt: false } + } + + if (connection.mode !== 'remote' || !connection.baseUrl) { + return { ok: true, rebuilt: false } + } + + const baseUrl = connection.baseUrl.replace(/\/+$/, '') + + try { + await probe(`${baseUrl}/api/status`, { timeoutMs: REMOTE_LIVENESS_TIMEOUT_MS }) + + if (currentConnectionPromise() !== connectionPromise) { + return { ok: true, rebuilt: false } + } + + tracker.recordSuccess(baseUrl) + + return { ok: true, rebuilt: false } + } catch { + if (currentConnectionPromise() !== connectionPromise) { + return { ok: true, rebuilt: false } + } + + const failure = tracker.recordFailure(baseUrl) + + if (!failure.shouldReset) { + log( + `Cached remote Hermes backend failed liveness probe (${failure.failures}/${REMOTE_LIVENESS_FAILURE_LIMIT}); keeping connection for retry.` + ) + + return { ok: true, rebuilt: false } + } + + log('Cached remote Hermes backend failed liveness probe; dropping stale connection.') + resetConnection() + + return { ok: true, rebuilt: true } + } +} 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 d60841813272..d199183c6d51 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts @@ -155,9 +155,10 @@ export function useGatewayBoot({ // with a short TTL, so the ticket baked into the cached conn.wsUrl is // dead on every reconnect after the initial boot — reusing it surfaces // as an opaque "Could not connect to Hermes gateway". resolveGatewayWsUrl - // mints a fresh ticket (or throws a reauth error in OAuth mode rather - // than connecting with a stale one). For local/token gateways the URL - // carries a long-lived token and the re-mint is a cheap no-op. + // mints a fresh ticket rather than connecting with a stale one. An + // explicit auth rejection asks for sign-in; transport failures stay in + // this reconnect loop. For local/token gateways the URL carries a + // long-lived token and the re-mint is a cheap no-op. const wsUrl = await resolveGatewayWsUrl(desktop, conn) await gateway.connect(wsUrl) @@ -454,9 +455,9 @@ export function useGatewayBoot({ publish(conn) // 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 and, on - // failure, throws a reauth error rather than connecting with a dead - // ticket (which would surface as an opaque "connection closed"). + // 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) await gateway.connect(wsUrl) diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts b/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts index d6c9ab0e029b..72bd08c98b51 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts @@ -69,9 +69,10 @@ export function useGatewayRequest() { setConnection(conn) // Re-mint the WS URL before reconnecting. OAuth tickets are single-use // and short-lived, so the cached conn.wsUrl ticket is dead here; - // resolveGatewayWsUrl() throws a reauth error in OAuth mode rather than - // connecting with a stale ticket. Stash it so requestGateway can show - // the actionable "sign in again" message. + // resolveGatewayWsUrl() never connects with a stale ticket. An explicit + // auth rejection becomes a reauth error; transport failures remain + // retryable. Stash only the former so requestGateway can show the + // actionable "sign in again" message. const wsUrl = await resolveGatewayWsUrl(desktop, conn) await existing.connect(wsUrl) diff --git a/apps/desktop/src/global.d.ts b/apps/desktop/src/global.d.ts index fe2a3eaa6dc0..89a14d443040 100644 --- a/apps/desktop/src/global.d.ts +++ b/apps/desktop/src/global.d.ts @@ -1,3 +1,5 @@ +import type { GatewayWsUrlResult } from '@hermes/shared' + import type { PetOverlayBounds, PetOverlayControl, @@ -24,7 +26,7 @@ declare global { // Keepalive: mark a pool profile backend as recently used so the idle // reaper spares it while its chat is active. touchBackend: (profile?: string | null) => Promise<{ ok: boolean }> - getGatewayWsUrl: (profile?: null | string) => Promise + getGatewayWsUrl: (profile?: null | string) => Promise // Open (or focus) a standalone OS window for a single chat session so // the user can work with multiple chats side by side. Returns ok:false // with an error code when the sessionId is empty/invalid. `watch` opens diff --git a/apps/desktop/src/lib/gateway-ws-url.test.ts b/apps/desktop/src/lib/gateway-ws-url.test.ts index e8b09765923f..48c7c7416d95 100644 --- a/apps/desktop/src/lib/gateway-ws-url.test.ts +++ b/apps/desktop/src/lib/gateway-ws-url.test.ts @@ -12,23 +12,56 @@ describe('resolveGatewayWsUrl', () => { expect(getGatewayWsUrl).toHaveBeenCalledOnce() }) - it('throws a reauth error instead of falling back to the stale cached ticket', async () => { - const getGatewayWsUrl = vi.fn().mockRejectedValue(new Error('401 cookie expired')) + it('uses the structured URL returned across the Electron IPC boundary', async () => { + const getGatewayWsUrl = vi.fn().mockResolvedValue({ ok: true, wsUrl: 'ws://host/api/ws?ticket=fresh' }) + + await expect(resolveGatewayWsUrl({ getGatewayWsUrl }, oauthConn)).resolves.toBe('ws://host/api/ws?ticket=fresh') + }) + + it('throws a reauth error when the main process reports an auth rejection', async () => { + const getGatewayWsUrl = vi.fn().mockResolvedValue({ + error: '401 cookie expired', + needsOauthLogin: true, + ok: false + }) + await expect(resolveGatewayWsUrl({ getGatewayWsUrl }, oauthConn)).rejects.toBeInstanceOf( GatewayReauthRequiredError ) }) - it('preserves the underlying mint failure as the cause', async () => { - const cause = new Error('401 cookie expired') - const getGatewayWsUrl = vi.fn().mockRejectedValue(cause) + it('preserves the main-process auth failure as the cause', async () => { + const getGatewayWsUrl = vi.fn().mockResolvedValue({ + error: '401 cookie expired', + needsOauthLogin: true, + ok: false + }) + const error = await resolveGatewayWsUrl({ getGatewayWsUrl }, oauthConn).catch(e => e) expect(error).toBeInstanceOf(GatewayReauthRequiredError) - expect((error as GatewayReauthRequiredError).cause).toBe(cause) + expect((error as GatewayReauthRequiredError).cause).toMatchObject({ message: '401 cookie expired' }) }) - it('throws a reauth error when the preload cannot mint (no method)', async () => { - await expect(resolveGatewayWsUrl({}, oauthConn)).rejects.toBeInstanceOf(GatewayReauthRequiredError) + it('keeps a transport failure retryable instead of demanding sign-in', async () => { + const getGatewayWsUrl = vi.fn().mockResolvedValue({ error: 'gateway timed out', ok: false }) + const error = await resolveGatewayWsUrl({ getGatewayWsUrl }, oauthConn).catch(e => e) + + expect(error).toMatchObject({ message: 'gateway timed out' }) + expect(isGatewayReauthRequired(error)).toBe(false) + }) + + it('rethrows an unexpected transport rejection unchanged', async () => { + const cause = new Error('socket closed') + const getGatewayWsUrl = vi.fn().mockRejectedValue(cause) + + await expect(resolveGatewayWsUrl({ getGatewayWsUrl }, oauthConn)).rejects.toBe(cause) + }) + + it('reports a missing preload method as an app capability error, not reauth', async () => { + const error = await resolveGatewayWsUrl({}, oauthConn).catch(e => e) + + expect(error).toMatchObject({ message: expect.stringMatching(/cannot refresh OAuth WebSocket tickets/i) }) + expect(isGatewayReauthRequired(error)).toBe(false) }) it('never returns the stale cached ticket on failure', async () => { @@ -45,6 +78,12 @@ describe('resolveGatewayWsUrl', () => { await expect(resolveGatewayWsUrl({ getGatewayWsUrl }, tokenConn)).resolves.toBe('ws://host/api/ws?token=fresh') }) + it('uses a structured refreshed token URL when available', async () => { + const getGatewayWsUrl = vi.fn().mockResolvedValue({ ok: true, wsUrl: 'ws://host/api/ws?token=fresh' }) + + await expect(resolveGatewayWsUrl({ getGatewayWsUrl }, tokenConn)).resolves.toBe('ws://host/api/ws?token=fresh') + }) + it('falls back to the cached URL when minting fails (token is long-lived)', async () => { const getGatewayWsUrl = vi.fn().mockRejectedValue(new Error('transient')) await expect(resolveGatewayWsUrl({ getGatewayWsUrl }, tokenConn)).resolves.toBe(tokenConn.wsUrl) diff --git a/apps/shared/src/index.ts b/apps/shared/src/index.ts index d2a21999182a..9fd163efa007 100644 --- a/apps/shared/src/index.ts +++ b/apps/shared/src/index.ts @@ -47,6 +47,7 @@ export { type GatewayAuthMode, GatewayReauthRequiredError, type GatewayWsConnection, + type GatewayWsUrlResult, type HermesWebSocketUrlOptions, isGatewayReauthRequired, resolveGatewayWsUrl, diff --git a/apps/shared/src/websocket-url.ts b/apps/shared/src/websocket-url.ts index 9384f30652de..24592c4642b0 100644 --- a/apps/shared/src/websocket-url.ts +++ b/apps/shared/src/websocket-url.ts @@ -12,9 +12,14 @@ export interface ResolveGatewayWsUrlDeps { * OAuth-gated gateways use single-use tickets, so callers should mint * immediately before opening the socket. */ - getGatewayWsUrl?: (profile?: null | string) => Promise + getGatewayWsUrl?: (profile?: null | string) => Promise } +export type GatewayWsUrlResult = + | string + | { ok: true; wsUrl: string } + | { error: string; needsOauthLogin?: boolean; ok: false } + export class GatewayReauthRequiredError extends Error { readonly needsOauthLogin = true @@ -37,27 +42,52 @@ export async function resolveGatewayWsUrl(deps: ResolveGatewayWsUrlDeps, conn: G if (conn.authMode === 'oauth') { if (!mint) { - throw new GatewayReauthRequiredError( - 'Your remote gateway session needs to be refreshed. Open Settings -> Gateway and click "Sign in" again.' - ) + throw new Error('This Desktop build cannot refresh OAuth WebSocket tickets. Update Hermes Desktop and try again.') } try { - return await mint(profile) + const result = await mint(profile) + + if (typeof result === 'string') { + return result + } + + if (result.ok) { + return result.wsUrl + } + + if (result.needsOauthLogin) { + throw new GatewayReauthRequiredError( + 'Your remote gateway session has expired. Open Settings -> Gateway and click "Sign in" again.', + { cause: new Error(result.error) } + ) + } + + throw new Error(result.error || 'Could not refresh the remote gateway WebSocket ticket.') } catch (error) { - throw new GatewayReauthRequiredError( - 'Your remote gateway session has expired. Open Settings -> Gateway and click "Sign in" again.', - { cause: error } - ) + if (isGatewayReauthRequired(error)) { + throw error instanceof GatewayReauthRequiredError + ? error + : new GatewayReauthRequiredError( + 'Your remote gateway session has expired. Open Settings -> Gateway and click "Sign in" again.', + { cause: error } + ) + } + + throw error } } if (mint) { const fresh = await mint(profile).catch(() => null) - if (fresh) { + if (typeof fresh === 'string') { return fresh } + + if (fresh?.ok) { + return fresh.wsUrl + } } return conn.wsUrl