diff --git a/apps/desktop/src/api/plugins.test.ts b/apps/desktop/src/api/plugins.test.ts new file mode 100644 index 0000000000..9a06752107 --- /dev/null +++ b/apps/desktop/src/api/plugins.test.ts @@ -0,0 +1,49 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { setApiRequestConnection, setApiRequestProfile } from '@/hermes' + +import { activeConnection } from './plugins' + +// desktop.getConnection/getConnectionFor are IPC round-trips into the main +// process with no timeout of their own (#93454). A wedged main-process +// round-trip must reject instead of hanging pluginSocket's connect() forever. +describe('activeConnection connection timeout (#93454)', () => { + afterEach(() => { + setApiRequestConnection(null) + setApiRequestProfile(null) + Reflect.deleteProperty(window, 'hermesDesktop') + vi.useRealTimers() + }) + + it('rejects instead of hanging forever when getConnection() wedges', async () => { + vi.useFakeTimers() + setApiRequestProfile('coder') + Object.defineProperty(window, 'hermesDesktop', { + configurable: true, + value: { getConnection: vi.fn(() => new Promise(() => undefined)) } + }) + + const pending = expect(activeConnection()).rejects.toThrow('Timed out connecting to profile "coder"') + + await vi.advanceTimersByTimeAsync(20_000) + await pending + }) + + it('rejects instead of hanging forever when getConnectionFor() wedges', async () => { + vi.useFakeTimers() + setApiRequestConnection('gw-tailscale') + setApiRequestProfile('research') + Object.defineProperty(window, 'hermesDesktop', { + configurable: true, + value: { + getConnection: vi.fn(() => new Promise(() => undefined)), + getConnectionFor: vi.fn(() => new Promise(() => undefined)) + } + }) + + const pending = expect(activeConnection()).rejects.toThrow('Timed out connecting to profile "research"') + + await vi.advanceTimersByTimeAsync(20_000) + await pending + }) +}) diff --git a/apps/desktop/src/api/plugins.ts b/apps/desktop/src/api/plugins.ts index ff8e4a6b0c..32d482356a 100644 --- a/apps/desktop/src/api/plugins.ts +++ b/apps/desktop/src/api/plugins.ts @@ -1,5 +1,6 @@ import type { HermesConnection } from '@/global' import { reconnectBackoffDelayMs } from '@/lib/reconnect-backoff' +import { RECONNECT_ATTEMPT_TIMEOUT_MS, withTimeout } from '@/lib/with-timeout' import { getApiRequestConnection, getApiRequestProfile, hermesApi, profileScoped } from './client' @@ -8,16 +9,33 @@ import { getApiRequestConnection, getApiRequestProfile, hermesApi, profileScoped * registry agent's descriptor comes from getConnectionFor (its SOURCE * connection), everything else from the profile-keyed local pool. The * getConnectionFor bridge is optional (older Desktop mains); without it the - * profile-scoped pool lookup is the best available answer. */ -async function activeConnection(): Promise { + * profile-scoped pool lookup is the best available answer. + * + * Both branches are IPC round-trips into the main process with no timeout of + * their own (#93454) — a wedged main-process round-trip otherwise hangs + * pluginSocket's connect() forever instead of falling back to the polling + * fallback every consumer already has. Bound the same way store/gateway's + * openSecondary bounds the same *For/plain pair. + * + * Exported for tests. */ +export async function activeConnection(): Promise { const getConnectionFor = window.hermesDesktop.getConnectionFor const connectionId = getApiRequestConnection() + const profile = getApiRequestProfile() if (connectionId && getConnectionFor) { - return getConnectionFor({ connectionId, profile: getApiRequestProfile() }) + return withTimeout( + getConnectionFor({ connectionId, profile }), + RECONNECT_ATTEMPT_TIMEOUT_MS, + `Timed out connecting to profile "${profile}"` + ) } - return window.hermesDesktop.getConnection(getApiRequestProfile()) + return withTimeout( + window.hermesDesktop.getConnection(profile), + RECONNECT_ATTEMPT_TIMEOUT_MS, + `Timed out connecting to profile "${profile}"` + ) } /** Options for a plugin REST call — mirrors the app's own `hermesDesktop.api` 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 15ec259597..f2c6f3f2ca 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts @@ -7,7 +7,7 @@ import { HermesGateway } from '@/hermes' import { translateNow } from '@/i18n' import { desktopDefaultCwd } from '@/lib/desktop-fs' import { reconnectBackoffDelayMs } from '@/lib/reconnect-backoff' -import { BACKEND_BOOT_WAIT_TIMEOUT_MS, withTimeout } from '@/lib/with-timeout' +import { BACKEND_BOOT_WAIT_TIMEOUT_MS, RECONNECT_ATTEMPT_TIMEOUT_MS, withTimeout } from '@/lib/with-timeout' import { $desktopBoot, applyDesktopBootProgress, @@ -113,18 +113,12 @@ const BOOT_RETRY_MAX_ATTEMPTS = 5 // loop's 300ms: each attempt may rebuild an SSH master + remote dashboard. const BOOT_RETRY_BASE_DELAY_MS = 2_000 -// desktop.revalidateConnection() / getConnection() / resolveGatewayWsUrl() are -// IPC round-trips into the main process with no timeout of their own (#93454). -// A remote backend that looks alive to a fresh probe but leaves the -// main-process reconnect path stuck (e.g. a wedged revalidation after a -// liveness-probe trip) hangs these awaits forever. While any is pending, -// `reconnecting` never clears, so scheduleReconnect()/attemptReconnect() -// early-return permanently and the backoff loop is latched — the UI stays -// "reconnecting" until the app is restarted even though the gateway is -// reachable again. Bound all three so a stall rejects instead, letting the -// existing catch/finally clear the guard and resume backoff. gateway.connect() -// already has its own connect timeout. -const RECONNECT_ATTEMPT_TIMEOUT_MS = 20_000 +// While any of the RECONNECT_ATTEMPT_TIMEOUT_MS-bounded awaits below is +// pending, `reconnecting` never clears, so scheduleReconnect()/ +// attemptReconnect() early-return permanently and the backoff loop is +// latched — the UI stays "reconnecting" until the app is restarted even +// though the gateway is reachable again. gateway.connect() already has its +// own connect timeout. /** Registry identity whose runtimes died with the primary connection. */ export function primaryRuntimeConnectionId(connection: Pick): null | string { diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-request.test.ts b/apps/desktop/src/app/gateway/hooks/use-gateway-request.test.ts index b3dc038143..5495403410 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-request.test.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-request.test.ts @@ -358,4 +358,34 @@ describe('useGatewayRequest', () => { expect(desktop.getConnectionFor).not.toHaveBeenCalled() expect(desktop.getGatewayWsUrlFor).not.toHaveBeenCalled() }) + + it('rejects instead of hanging forever when the reconnect getConnection() wedges (#93454)', async () => { + // Repro: a request lands on a dropped socket, the "not connected" catch + // kicks off a reconnect, and the IPC round-trip into main + // (desktop.getConnection) never settles — e.g. a wedged revalidation after + // a liveness-probe trip. Without an internal timeout on that await, + // reconnectingRef never clears and requestGateway hangs forever instead of + // surfacing the original transport error. + vi.useFakeTimers() + + const dropped = { + connectionState: 'closed', + request: vi.fn().mockRejectedValue(new Error('connection closed')) + } as unknown as HermesGateway + const getConnection = vi.fn(() => new Promise(() => undefined)) + + ;(window as unknown as { hermesDesktop: unknown }).hermesDesktop = { getConnection } + $gateway.set(dropped) + + const { result } = renderHook(() => useGatewayRequest()) + + const pending = expect(result.current.requestGateway('some.method')).rejects.toThrow('connection closed') + + // Advance past the internal reconnect-attempt timeout (20s) — the stalled + // getConnection() await must reject so the reconnect gives up and the + // original transport error surfaces, instead of requestGateway() never + // settling. + await vi.advanceTimersByTimeAsync(20_000) + await pending + }) }) 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 dd659e76ed..448e190354 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts @@ -3,6 +3,7 @@ import { useStore } from '@nanostores/react' import { useCallback, useEffect, useRef } from 'react' import type { HermesGateway } from '@/hermes' +import { RECONNECT_ATTEMPT_TIMEOUT_MS, withTimeout } from '@/lib/with-timeout' import { $gateway, ensureActiveGatewayOpen, isActivePrimary } from '@/store/gateway' import { $activeGatewayProfile } from '@/store/profile' import { $gatewayState, setConnection } from '@/store/session' @@ -72,8 +73,17 @@ export function useGatewayRequest() { try { // Reconnect to whichever profile the gateway is currently routed to (not // always the primary), so a sleep/wake reconnect keeps the user on the - // profile they were chatting in. - const conn = await desktop.getConnection($activeGatewayProfile.get()) + // profile they were chatting in. Both awaits below are IPC round-trips + // into the main process with no timeout of their own (#93454) — a + // wedged main-process round-trip otherwise hangs this await forever, + // latching reconnectingRef.current so every later requestGateway() call + // returns the same never-settling promise. Bound the same way + // use-gateway-boot.ts bounds the primary boot/soft-switch equivalents. + const conn = await withTimeout( + desktop.getConnection($activeGatewayProfile.get()), + RECONNECT_ATTEMPT_TIMEOUT_MS, + 'Timed out reconnecting to Hermes backend' + ) connectionRef.current = conn setConnection(conn) // Re-mint the WS URL before reconnecting. OAuth tickets are single-use @@ -82,7 +92,11 @@ export function useGatewayRequest() { // 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) + const wsUrl = await withTimeout( + resolveGatewayWsUrl(desktop, conn), + RECONNECT_ATTEMPT_TIMEOUT_MS, + 'Timed out re-minting the gateway WebSocket URL' + ) await existing.connect(wsUrl) return existing diff --git a/apps/desktop/src/lib/voice-playback.routing.test.ts b/apps/desktop/src/lib/voice-playback.routing.test.ts index 682870acf6..90e70ad4f9 100644 --- a/apps/desktop/src/lib/voice-playback.routing.test.ts +++ b/apps/desktop/src/lib/voice-playback.routing.test.ts @@ -41,6 +41,7 @@ describe('resolveSpeakStreamUrl', () => { setApiRequestConnection(null) setApiRequestProfile(null) Reflect.deleteProperty(window, 'hermesDesktop') + vi.useRealTimers() }) it('resolves through the registry (connection, profile) bridges when a registry connection is active', async () => { @@ -102,4 +103,20 @@ describe('resolveSpeakStreamUrl', () => { expect(url).toContain('/api/audio/speak-stream') expect(getConnection).toHaveBeenCalledWith('research') }) + + it('resolves to null instead of hanging forever when getConnection() wedges (#93454)', async () => { + // desktop.getConnection/getConnectionFor/resolveGatewayWsUrl are IPC + // round-trips into the main process with no timeout of their own. A + // wedged main-process round-trip otherwise hangs voice mode's "speaking" + // state forever instead of falling back to playSpeechText. + vi.useFakeTimers() + setApiRequestProfile('coder') + getConnection.mockImplementation(() => new Promise(() => undefined)) + + const pending = resolveSpeakStreamUrl() + + await vi.advanceTimersByTimeAsync(20_000) + + await expect(pending).resolves.toBeNull() + }) }) diff --git a/apps/desktop/src/lib/voice-playback.ts b/apps/desktop/src/lib/voice-playback.ts index eb64c4feb3..02814b678f 100644 --- a/apps/desktop/src/lib/voice-playback.ts +++ b/apps/desktop/src/lib/voice-playback.ts @@ -7,6 +7,7 @@ import { type DirectTtsConfig, synthesizeSpeechClientDirect } from '@/lib/voice-client-direct' +import { RECONNECT_ATTEMPT_TIMEOUT_MS, withTimeout } from '@/lib/with-timeout' import { $voicePlayback, setVoicePlaybackState, @@ -125,10 +126,23 @@ export async function resolveSpeakStreamUrl(): Promise { const profile = getApiRequestProfile() const connectionId = getApiRequestConnection() + // Both awaits below are IPC round-trips into the main process with no + // timeout of their own (#93454) — a wedged main-process round-trip + // otherwise hangs voice mode's "speaking" state forever instead of + // falling back to playSpeechText. Bound the same way + // store/gateway's openSecondary bounds the same *For/plain pair. const conn = connectionId && desktop.getConnectionFor - ? await desktop.getConnectionFor({ connectionId, profile }) - : await desktop.getConnection(profile) + ? await withTimeout( + desktop.getConnectionFor({ connectionId, profile }), + RECONNECT_ATTEMPT_TIMEOUT_MS, + `Timed out connecting to profile "${profile}"` + ) + : await withTimeout( + desktop.getConnection(profile), + RECONNECT_ATTEMPT_TIMEOUT_MS, + `Timed out connecting to profile "${profile}"` + ) const wsDeps = connectionId && desktop.getGatewayWsUrlFor @@ -137,7 +151,11 @@ export async function resolveSpeakStreamUrl(): Promise { ? {} : desktop - const wsUrl = await resolveGatewayWsUrl(wsDeps, conn) + const wsUrl = await withTimeout( + resolveGatewayWsUrl(wsDeps, conn), + RECONNECT_ATTEMPT_TIMEOUT_MS, + `Timed out re-minting the gateway WebSocket URL for profile "${profile}"` + ) const url = new URL(wsUrl) if (!url.pathname.endsWith('/api/ws')) { diff --git a/apps/desktop/src/lib/with-timeout.ts b/apps/desktop/src/lib/with-timeout.ts index f0b1433d13..3bcbf94ac4 100644 --- a/apps/desktop/src/lib/with-timeout.ts +++ b/apps/desktop/src/lib/with-timeout.ts @@ -5,9 +5,16 @@ * healthy cold boot publishes well within this; anything longer means the * backend is not coming and the caller should fail instead of hanging. * Reconnect-class awaits against an already-spawned backend use the shorter - * RECONNECT_ATTEMPT_TIMEOUT_MS (use-gateway-boot.ts) instead. */ + * RECONNECT_ATTEMPT_TIMEOUT_MS below instead. */ export const BACKEND_BOOT_WAIT_TIMEOUT_MS = 45_000 +// desktop.getConnection() / getConnectionFor() / revalidateConnection() / +// resolveGatewayWsUrl() are IPC round-trips into the main process with no +// timeout of their own (#93454). A wedged main-process round-trip (e.g. a +// stuck revalidation after a liveness-probe trip) otherwise hangs an awaiting +// caller forever. Every caller of these bounds them with this shared budget. +export const RECONNECT_ATTEMPT_TIMEOUT_MS = 20_000 + /** Rejection raised by withTimeout. The bounded work is NOT cancelled — the * caller decides what a straggler that settles later means. */ export class TimeoutError extends Error { diff --git a/apps/desktop/src/store/gateway.test.ts b/apps/desktop/src/store/gateway.test.ts index fc0b223ba4..9caf9b91e6 100644 --- a/apps/desktop/src/store/gateway.test.ts +++ b/apps/desktop/src/store/gateway.test.ts @@ -234,3 +234,75 @@ describe('profile switch mid-WS-handshake (#92434 close-candidate pin)', () => { expect(gatewayMocks.instances).toHaveLength(1) }) }) + +describe('secondary connection timeout (#93454)', () => { + it("rejects instead of hanging forever when openSecondary's getConnection() wedges", async () => { + // Repro: desktop.getConnection is an IPC round-trip into the main process + // with no timeout of its own. A wedged main-process round-trip (e.g. a + // stuck revalidation) hangs this await forever, latching + // entry.connectPromise so every routed action against this secondary + // (SSH terminal, messaging DELETE, session send, …) never settles either. + vi.useFakeTimers() + + let callCount = 0 + const getConnection = vi.fn(({ profile }: { profile: string }) => { + callCount += 1 + + // First call is sharedPrimaryRoute's probe — resolves fast, not the + // shared primary. Every call after (openSecondary's actual dial) wedges. + if (callCount === 1) { + return Promise.resolve({ sharedPrimary: false }) + } + + return new Promise(() => undefined) + }) + + installDesktop({ getConnection }) + + const pending = expect(ensureGatewayForProfile('work')).rejects.toThrow('Timed out connecting to profile "work"') + + // Advance past the internal reconnect-attempt timeout (20s) — the stalled + // await must reject instead of hanging forever. + await vi.advanceTimersByTimeAsync(20_000) + await pending + }) + + it('does not let a wedged shared-primary-route probe block the secondary dial forever', async () => { + // Same unbounded-IPC hazard as above, but for sharedPrimaryRoute's own + // getConnection() probe, which runs BEFORE openSecondary on every route — + // a wedge there must resolve to "not the shared primary" and fall through + // to the ordinary secondary dial instead of hanging the whole route + // decision forever. + vi.useFakeTimers() + + let callCount = 0 + const getConnection = vi.fn(({ profile }: { profile: string }) => { + callCount += 1 + + if (callCount === 1) { + return new Promise(() => undefined) + } + + return Promise.resolve({ + authMode: 'token', + baseUrl: `https://${profile}.invalid`, + mode: 'local', + profile, + token: 'fake-test-token', + wsUrl: `wss://${profile}.invalid/ws` + }) + }) + + installDesktop({ getConnection }) + + const pending = ensureGatewayForProfile('work') + + await vi.advanceTimersByTimeAsync(20_000) + await pending + + // The probe's own bound (not just openSecondary's) is what let this + // resolve after a single 20s timeout instead of two stacked ones. + expect(callCount).toBe(2) + expect(activeGateway()).toBe(gatewayMocks.instances[0]) + }) +}) diff --git a/apps/desktop/src/store/gateway.ts b/apps/desktop/src/store/gateway.ts index c2075f1e4e..c7c553d65a 100644 --- a/apps/desktop/src/store/gateway.ts +++ b/apps/desktop/src/store/gateway.ts @@ -4,6 +4,7 @@ import { atom } from 'nanostores' import type { HermesConnection } from '@/global' import { HermesGateway, setApiRequestConnection } from '@/hermes' import { reconnectBackoffDelayMs } from '@/lib/reconnect-backoff' +import { RECONNECT_ATTEMPT_TIMEOUT_MS, withTimeout } from '@/lib/with-timeout' import { markNativeNotifyBaseline } from '@/store/notify-baseline' import { setConnection, setGatewayState } from '@/store/session' @@ -487,11 +488,25 @@ async function openSecondary(entry: Secondary): Promise { } // Registry-scoped entries dial through getConnectionFor when the bridge has - // it. Local/legacy entries retain the existing getConnection path. + // it. Local/legacy entries retain the existing getConnection path. Both are + // IPC round-trips into the main process with no timeout of their own + // (#93454) — a wedged main-process round-trip otherwise hangs this await + // forever, latching entry.connectPromise so every routed action against + // this secondary (SSH terminal, messaging DELETE, session send, …) never + // settles either. Bound the same way use-gateway-boot.ts bounds the + // primary's equivalent awaits. const conn = entry.connectionId && desktop.getConnectionFor - ? await desktop.getConnectionFor({ connectionId: entry.connectionId, profile: entry.profile }) - : await desktop.getConnection(entry.profile) + ? await withTimeout( + desktop.getConnectionFor({ connectionId: entry.connectionId, profile: entry.profile }), + RECONNECT_ATTEMPT_TIMEOUT_MS, + `Timed out connecting to profile "${entry.profile}"` + ) + : await withTimeout( + desktop.getConnection(entry.profile), + RECONNECT_ATTEMPT_TIMEOUT_MS, + `Timed out connecting to profile "${entry.profile}"` + ) entry.connection = conn @@ -505,7 +520,11 @@ async function openSecondary(entry: Secondary): Promise { ? {} : desktop - const wsUrl = await resolveGatewayWsUrl(wsDeps, conn) + const wsUrl = await withTimeout( + resolveGatewayWsUrl(wsDeps, conn), + RECONNECT_ATTEMPT_TIMEOUT_MS, + `Timed out re-minting the gateway WebSocket URL for profile "${entry.profile}"` + ) try { await entry.gateway.connect(wsUrl) @@ -696,7 +715,15 @@ async function sharedPrimaryRoute(profile: string): Promise { } try { - const conn = await desktop.getConnection(profile) + // Unbounded IPC round-trip into main (#93454) — a wedge here must reject + // like any other failure, not hang the route decision forever, since + // every caller (gatewayForProfile → requestGatewayForProfile/Agent) awaits + // this before it can fall back to dialing a secondary. + const conn = await withTimeout( + desktop.getConnection(profile), + RECONNECT_ATTEMPT_TIMEOUT_MS, + `Timed out resolving the shared-primary route for profile "${profile}"` + ) return Boolean(conn && typeof conn === 'object' && (conn as { sharedPrimary?: boolean }).sharedPrimary === true) } catch {