Skip to content

fix(desktop): settle the gateway connect on gateway.ready and keep a stale boot from finishing after a switch - #73821

Open
Sora-bluesky wants to merge 1 commit into
NousResearch:mainfrom
Sora-bluesky:fix/desktop-boot-connect-retry
Open

fix(desktop): settle the gateway connect on gateway.ready and keep a stale boot from finishing after a switch#73821
Sora-bluesky wants to merge 1 commit into
NousResearch:mainfrom
Sora-bluesky:fix/desktop-boot-connect-retry

Conversation

@Sora-bluesky

@Sora-bluesky Sora-bluesky commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Reworked on current main (the earlier version of this branch retried the boot-time WS dial with its own ladder; upstream 2b7f496 now owns boot retry policy, so that part is gone).

JsonRpcGatewayClient.connect() resolved on the raw WebSocket open and connectionState became 'open' at the same time. The desktop boot in use-gateway-boot.ts treats a resolved connect() as a usable gateway (adopts the profile, hydrates config/sessions, calls completeDesktopBoot()), and its reconnect bookkeeping (reconnectAttempt, reconnectFailingSince, escalated) resets on every 'open'. So a gateway that accepts the upgrade and closes right away looks healthy: boot can complete on a dead socket, and a persistently rejected socket never reaches the 45 s escalation because each 'open' clears the failure clock. request() only checked the socket's readyState, so callers could send during the handshake. This is the client half of #88607: once /api/ws rejections are delivered as close codes (as #88619 does for the other endpoints), the client has to treat open followed by a close as a rejection, not a connection.

Every /api/ws connection starts with a gateway.ready event (tui_gateway/ws.py::handle_ws writes it right after accept()), so that frame is where the connection is known to be usable.

  • connect() resolves, and the state becomes 'open', only on the first frame, which must be gateway.ready. A different first frame is a protocol failure (state 'error', generic connect error). Raw open leaves the state 'connecting' and the promise pending. request() requires the semantic 'open'.
  • A close during the handshake settles the attempt immediately with a GatewayConnectError (exported from @hermes/shared) carrying wsCloseCode. 4401 sets needsOauthLogin and uses the new authRejectedErrorMessage option, so isGatewayReauthRequired() is true and the boot-failure overlay routes to the sign-in path (HermesGateway passes the canonical "Your remote gateway session has expired…" text that isRemoteReauthError() matches). 4403/4400 keep the code without the auth marker. onSocketClose still applies to established connections only.
  • One connect-attempt record owns the socket, timer and promise: a concurrent connect() for the same URL awaits the in-flight attempt, a different URL is rejected, close() during the handshake rejects it at once, and the abandoned connect timeout can no longer flip a newer, ready connection to 'error'.
  • Desktop boot: boot(gen) carries a boot generation. softSwitch() increments it before doing anything else, so an older boot still awaiting getConnection() / connect() / hydration cannot publish its connection, adopt a profile, commit the boot-owned cwd, schedule a retry, report a failure or complete boot after the switch. Upstream's bounded retry timer re-checks the generation when it fires. Reauth (4401) and explicit refusals (4400/4403) skip the automatic boot retry and go to the recovery overlay. Retry policy, jitter, limits and the 45 s reconnect escalation are unchanged.

How was it tested?

  • Fail-before: 12 new tests, 11 of which fail on main and pass here. apps/desktop/src/lib/json-rpc-gateway-url-guard.test.ts: raw open keeps connecting and the promise pending; gateway.ready opens and resolves; request() before readiness rejects without sending; open→4401 close rejects with needsOauthLogin, wsCloseCode and the auth message; open→4403 keeps the code without the auth marker; a non-ready first frame is a protocol failure; close() during the handshake rejects at once and the old timeout cannot poison the next connection; same-URL concurrent connect shares the attempt, different URL is rejected. use-gateway-boot.test.tsx: 4401 at boot fails with the reauth message and never auto-retries; 4403 fails generically and never retries; a soft switch keeps a pending older boot from publishing/adopting/completing; a queued retry from an older generation cannot restart boot after a switch.
  • Existing fixtures now emit gateway.ready after open, which is the actual protocol; all existing tests in the two files still pass (31/31). Web vitest 275/275. Full desktop vitest shows only the same environment-specific failures as main. Web tsc clean; desktop tsc clean apart from a fixture that is missing in my sparse checkout.
  • Against today's server (/api/ws still closes before accept, which uvicorn turns into HTTP 403) behaviour is unchanged: rejected upgrades still reject via error, successful ones still send gateway.ready.

Compatibility

New client + current server: unchanged behaviour. New client + a server that never sends gateway.ready first (an old or custom gateway): connect() times out after the existing 15 s instead of resolving on open. tui_gateway/ws.py has emitted gateway.ready since it was added (April 2026, f49afd3), so this only affects gateways that are not this repo's. Old client + a server that moves /api/ws to accept-then-close: the false-success this PR fixes, which is why the server switch (#88607) waits for this to ship.

Follow-up (separate PR)

Move the three /api/ws gates in hermes_cli/web_server.py to _ws_reject and drop the allowlist entry in tests/hermes_cli/test_web_server_ws_reject.py, once #88619 has landed and a desktop with this client is out. In the same change, teach the web dashboard's GatewayClient to route a 4401 that arrives during the handshake to maybeReloadForLoopbackWsAuthFailure (today that hook only sees post-ready closes; against the current server a handshake rejection is an error, so nothing changes yet).

Relates to #73722 (mechanism 1 was superseded by 2b7f496; this covers what was left of it).

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have labels Jul 29, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing a real cold-boot failure. Current main makes the initial gateway.connect() failure terminal in apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts:487-533, and a handshake close does not settle the pending dial in apps/shared/src/json-rpc-gateway.ts:139-210.

Problems

  • apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts:672 raises the recovery overlay after the seventh failed dial, not six: bootRetryAttempt is incremented only while scheduling at line 524, after the threshold is checked. The PR description promises escalation after six consecutive failures.
  • apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts:182-200 still schedules another primary reconnect after isGatewayReauthRequired(err). The new 4401 handshake classification will make that loop fail sooner, but it will not fail fast at the recovery-policy level as described.

Suggested changes

  • Count the current failure before checking the boot threshold and add an exact-six-failures test.
  • Stop scheduling the post-boot reconnect loop after a confirmed reauth handshake rejection, with a regression test for that path.

Automated hermes-sweeper review.

isGatewayReauthRequired(err) || typeof (err as { wsCloseCode?: unknown } | null)?.wsCloseCode === 'number'

if (reachedConnectPhase && !wsOpen && !gatewayRefusedHandshake) {
if (bootRetryAttempt >= RECONNECT_ESCALATE_AFTER && !bootEscalated) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bootRetryAttempt is incremented only in scheduleBootRetry() after this check, so a threshold of 6 escalates on the seventh failed dial. Count this failure before evaluating the threshold (and pin the exact-six contract in a test).

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@Sora-bluesky

Copy link
Copy Markdown
Contributor Author

The off-by-one is real and is fixed in 2b0b1d3f8.

bootRetryAttempt counts retries already scheduled and scheduleBootRetry() is what increments it, so the failure being handled was not in the count yet when the threshold was checked. I measured it by stepping the fake timers and recording the dial count at the first failDesktopBoot call: seven before, six after.

I compared against bootRetryAttempt + 1 rather than swapping the two statements. Swapping changes behaviour when scheduleBootRetry takes its early return (stale generation, pending timer, or a switch in progress), since it does not increment on that path, and moving the increment out changes the backoff sequence because the delay is computed from the pre-increment value.

On the second point, that code is not in this diff. scheduleReconnect() at line 201 is the post-boot reconnect loop, which is main's behaviour on any commit and is untouched here — git diff against the merge base for this file shows the boot path only. Whether a confirmed reauth rejection should halt that loop or keep retrying is a real question, but changing it here would widen a 208-line boot fix into the reconnect policy and it belongs to whoever owns that decision.

Worth noting the loop is self-healing rather than stuck: the user gets one sign-in toast, and the recovery overlay still arrives on schedule.

@Sora-bluesky
Sora-bluesky force-pushed the fix/desktop-boot-connect-retry branch from 2b0b1d3 to 83d30c1 Compare August 11, 2026 11:52
@Sora-bluesky Sora-bluesky changed the title fix(desktop): retry transient gateway connect failures during boot fix(desktop): settle the gateway connect on gateway.ready and keep a stale boot from finishing after a switch Aug 17, 2026
@Sora-bluesky
Sora-bluesky force-pushed the fix/desktop-boot-connect-retry branch 2 times, most recently from b70ad69 to 470273b Compare August 18, 2026 04:59
…stale boot from finishing after a switch

`JsonRpcGatewayClient.connect()` resolved on the raw WebSocket `open`
event and `connectionState` became 'open' at the same moment. The desktop
boot (`use-gateway-boot.ts`) treats a resolved connect() as a usable
gateway: it adopts the profile, hydrates config/sessions and calls
`completeDesktopBoot()`, and its reconnect bookkeeping
(`reconnectAttempt`, `reconnectFailingSince`, `escalated`) resets on every
'open'. A gateway that accepts the upgrade and closes right away therefore
looked like a healthy connection: boot could complete on a dead socket, and
a persistently rejected socket never reached the 45 s escalation because
each 'open' cleared the failure clock. `request()` also only checked the
socket's readyState, so callers could send during the handshake.

Every `/api/ws` connection starts with a `gateway.ready` event
(`tui_gateway/ws.py::handle_ws` sends it right after accept), so that
frame is the point where the connection is known to be usable:

- `connect()` now resolves, and the state becomes 'open', only on the
  first frame, which must be `gateway.ready`; a different first frame is a
  protocol failure (state 'error', generic connect error). Raw `open`
  leaves the state 'connecting' and the promise pending. `request()`
  requires the semantic 'open'.
- A close during the handshake settles the attempt at once with a
  `GatewayConnectError` (exported from @hermes/shared) carrying
  `wsCloseCode`; 4401 sets `needsOauthLogin` and uses the new
  `authRejectedErrorMessage` option, so `isGatewayReauthRequired()` is
  true and the desktop's boot-failure overlay routes to the sign-in path
  (HermesGateway passes the canonical "Your remote gateway session has
  expired…" text). 4403/4400 keep the code without the auth marker.
  `onSocketClose` still applies to established connections only.
- One connect attempt record owns the socket, timer and promise: a
  concurrent connect() for the same URL awaits the in-flight attempt, a
  different URL is rejected, `close()` during the handshake rejects it
  immediately, and the abandoned connect timeout can no longer flip a
  newer, ready connection to 'error'.

Desktop boot: `boot(gen)` carries a boot generation. `softSwitch()`
increments it before doing anything else, so an older boot that is still
awaiting `getConnection()`/connect()/hydration cannot publish its
connection, adopt a profile, commit the boot-owned cwd, schedule a retry,
report a failure or complete boot after the switch; upstream's bounded
retry timer re-checks the generation when it fires. Reauth (4401) and
explicit refusals (4400/4403) are excluded from the automatic boot retry
and go straight to the recovery overlay. Retry policy, jitter, limits and
the 45 s reconnect escalation are unchanged.

Fail-before: 12 new tests (fake-socket contract in
json-rpc-gateway-url-guard.test.ts, 4401/4403 boot routing and the
generation guard in use-gateway-boot.test.tsx); 11 of them fail on main
and pass here. Existing fixtures now emit `gateway.ready` after `open`,
which is the actual protocol. Against today's server (close-before-accept
on /api/ws) behaviour is unchanged: rejected upgrades still reject via
`error`, successful ones still send gateway.ready. Moving the /api/ws
gates to accept-then-close (NousResearch#88607/NousResearch#88619 do this for the other
endpoints) becomes safe once this client is deployed.
@Sora-bluesky
Sora-bluesky force-pushed the fix/desktop-boot-connect-retry branch from 470273b to 4879345 Compare August 20, 2026 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants