fix(desktop): make remote gateway 'Test remote' validate the live WebSocket - #39098
fix(desktop): make remote gateway 'Test remote' validate the live WebSocket#39098xxxigm wants to merge 2 commits into
Conversation
Adds electron/gateway-ws-probe.cjs: a small helper that opens a gateway WebSocket URL and classifies the handshake (open/frame → ok; error or close before open → fail; open-then-early-close → credential rejected; never-opens → timeout). The WebSocket implementation is injected so it can be unit-tested without a real socket. Wires gateway-ws-probe.test.cjs into test:desktop:platforms, covering every handshake outcome plus constructor-throw and missing-impl.
The "Test remote" button only checked HTTP GET /api/status, but the chat surface depends on the renderer opening a live WebSocket to /api/ws — a separate transport with separate server-side guards (Host/Origin checks, ws-ticket/token auth, peer-IP checks). A gateway could pass the HTTP check yet reject the WebSocket, so the test reported "reachable" while boot still failed with the opaque "Could not connect to Hermes gateway". testDesktopConnectionConfig now mirrors the renderer's connect: after the status check it opens the WS URL (token/local) or a freshly minted ws-ticket (OAuth) and confirms the upgrade is accepted and not immediately torn down by a post-handshake auth rejection. Failures surface an actionable message instead of a false-positive. The WS leg is skipped when the runtime lacks a global WebSocket so it never fails spuriously.
0cdc64f to
286ccbe
Compare
YoussefEttamimi
left a comment
There was a problem hiding this comment.
Thanks for tightening the remote gateway test path. I found a blocking false-positive in the OAuth branch.
resolveTestWsUrl() currently catches ticket-mint failures and returns null, and testDesktopConnectionConfig() only runs the live WS probe when a wsUrl exists. In token mode that is fine because a URL is always constructed, but in OAuth mode a stale/invalid session can fail mintGatewayWsTicket(baseUrl), return null, skip the WebSocket validation entirely, and still report the remote test as successful after GET /api/status passes.
That is exactly the class of problem this PR is meant to catch: HTTP status can succeed while the renderer cannot authenticate /api/ws. The normal connection flow treats failure to mint the WS ticket as an auth/session failure, so the test button should do the same (or at least surface an actionable ticket-mint error) instead of silently skipping the probe.
Suggested fix: distinguish “cannot test because this runtime has no WebSocket” from “OAuth ticket mint failed”. For OAuth, propagate/report the ticket-mint failure and add coverage for:
- token mode + WS failure
- OAuth ticket mint success + WS failure
- OAuth ticket mint failure should fail the remote test, not skip WS validation
Checks I ran/verified during review:
node --test electron/gateway-ws-probe.test.cjs— passednpm run test:desktop:platforms— passednode --check electron/gateway-ws-probe.cjs electron/gateway-ws-probe.test.cjs— passed
The helper itself looks solid; the issue is in the integration path for OAuth ticket mint failures.
|
Merged via #39511. Your two commits were cherry-picked onto current Thanks for the fix — the live WebSocket probe is exactly the right approach. The only addition was resolving @YoussefEttamimi's review: an OAuth ticket-mint failure now fails the test (with an actionable "sign in again" message) instead of silently skipping the probe, mirroring the real boot path. 62/62 desktop platform tests passing. |
Context / report
Discord report (multiple users hitting the same thing):
https://discord.com/channels/1053877538025386074/1511993870957674587/1511993870957674587
Users connecting the Desktop app to a remote Hermes backend see:
Remote Hermes backend is readyQuotes from the thread:
Root cause
Desktop boot touches a remote gateway over two independent paths:
GET /api/status(token in a header) to confirm the backend is up. This is what the boot logs print asRemote Hermes backend is ready, and what Test remote (testDesktopConnectionConfig) checked./api/ws(credential in a query param) viagateway.connect(). The chat surface only works once this succeeds; on failure it throwsconnectErrorMessage = "Could not connect to Hermes gateway"(apps/shared/src/json-rpc-gateway.ts).These two paths use different processes, transports, and credentials, and the server applies extra guards to the WS upgrade that the HTTP status route never sees — see
web_server.py:_ws_host_origin_reason(Host/Origin guard, mirrored from HTTP middleware because FastAPI middleware doesn't run for WS routes)_ws_auth_reason(ws-ticket / internal / legacy?token=validation, with?token=rejected outright once the OAuth gate is engaged)_ws_client_reason(peer-IP guard)So a remote gateway can pass
/api/statusyet reject the WebSocket (blocked by a reverse proxy that doesn't forwardUpgrade, a firewall, or one of the WS guards). The old test only exercised path #1, giving a false-positive "reachable" while the real boot failed on path #2.Fix
Make the connection test exercise the same transport the app actually uses.
After the
/api/statuscheck,testDesktopConnectionConfignow opens the gateway WebSocket and confirms the upgrade is accepted:ws(s)://…/api/ws?token=…?ticket=…(POST /api/auth/ws-ticket); skipped (no false failure) when there's no live sessionThe probe (
electron/gateway-ws-probe.cjs) classifies the handshake:On failure the test throws an actionable message instead of reporting success:
The
WebSocketimplementation is injected (Electron's globalWebSocket; skipped if a runtime lacks one, so no spurious failures on older builds).Tests
electron/gateway-ws-probe.test.cjs(wired intotest:desktop:platforms) drives a fake socket through every outcome: open-stays-open, frame, error-before-open, close-before-open, accept-then-early-close, timeout, constructor throw, and missing impl.Test plan
npm run test:desktop:platformsgreenUpgrade: Test remote now fails with the WS message instead of a false "reachable"