fix(desktop): bound getConnection()/resolveGatewayWsUrl() on every remaining route

20s withTimeout() on the boot() and soft-switch paths (use-gateway-boot.ts), since these are IPC round-trips into the main process with no timeout of their own — a wedged main-process round-trip hangs the awaiting caller forever instead of surfacing a failure. Every other production call site of the same IPC pair was still unbounded: - store/gateway.ts's openSecondary() and sharedPrimaryRoute() — the actual connection-establishment underneath requestGatewayForProfile/Agent, ensureGatewayForProfile/Agent, and every other exported routing entry point that opens a non-primary profile's socket. - use-gateway-request.ts's on-demand reconnect (the primary gateway's "not connected" retry path hit by every RPC). - voice-playback.ts's resolveSpeakStreamUrl(). - api/plugins.ts's activeConnection() (pluginSocket's connect()). Extracted RECONNECT_ATTEMPT_TIMEOUT_MS into the shared lib/with-timeout.ts (previously local to use-gateway-boot.ts) so every call site uses the same budget instead of duplicating the constant. Regression tests mirror the existing use-gateway-boot.test.tsx hang-repro pattern: wedge getConnection()/getConnectionFor() with a never-resolving promise, advance fake timers past the 20s bound, assert the caller settles instead of hanging. Mutation-verified: reverted the production fix (kept tests) and confirmed the 6 new tests fail — 4 by genuinely timing out at the vitest level, 2 by TypeError on the not-yet-exported activeConnection — restored the fix and confirmed all 70 tests across the gateway/voice/boot/ plugins suites pass, with tsc -p . --noEmit clean throughout.
2026-08-26 10:08:16 +03:00
parent 4896cab0eb
commit d81b801fb4
10 changed files with 275 additions and 29 deletions
@@ -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
+  })
+})
@@ -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<HermesConnection> {
+ *  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<HermesConnection> {
  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`
@@ -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<HermesConnection, 'connectionId' | 'mode'>): null | string {
@@ -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
+  })
 })
@@ -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
@@ -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()
+  })
 })
@@ -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<null | string> {
    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<null | string> {
          ? {}
          : 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')) {
@@ -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 {
@@ -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])
+  })
+})
@@ -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<void> {
    }

    // 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<void> {
          ? {}
          : 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<boolean> {
  }

  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 {