fix(desktop): validate live WebSocket in remote gateway test (salvage #39098) - #39511
Merged
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.
Youssef's review caught a residual false-positive: resolveTestWsUrl swallowed an OAuth ticket-mint failure and returned null, so the caller skipped the WS probe and reported the remote test as reachable. But the real boot path (resolveRemoteBackend) treats a mint failure as a hard 'session expired' auth error and refuses to connect — so an expired OAuth session passed the test then failed boot, the exact false-positive this PR exists to kill. Extract resolveTestWsUrl into the electron-free connection-config.cjs (injectable mintTicket) so it's unit-testable, and make OAuth mint failure throw an actionable needsOauthLogin error instead of skipping. Adds the three cases Youssef requested plus a mintTicket-required guard.
Contributor
🔎 Lint report:
|
3 tasks
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Desktop "Test remote" button now validates the live WebSocket the app actually uses — and correctly fails (instead of silently skipping) when an OAuth session can't authenticate the socket.
Salvages #39098 (@xxxigm) onto current
mainand resolves @YoussefEttamimi's blocking review.Root cause: desktop boot touches a remote gateway over two transports — HTTP
GET /api/status(what "Test remote" checked) and a live WebSocket to/api/ws(what the chat surface needs). The WS upgrade has extra server-side guards (Host/Origin, ws-ticket/token auth, peer-IP) the HTTP route never sees, so a gateway can pass/api/statusyet reject the socket — a green "reachable" followed by the opaque "Could not connect to Hermes gateway" overlay.Changes
electron/gateway-ws-probe.cjs(new): injectable WS probe that classifies the handshake (open/stays-open or frame = ok; error / close-before-open / accept-then-early-close / timeout = fail).electron/main.cjs:testDesktopConnectionConfigopens the live WS after the HTTP check; throws an actionable message on probe failure.electron/connection-config.cjs:resolveTestWsUrlextracted here (electron-free, injectablemintTicket) so it's unit-testable.needsOauthLoginerror instead of returning null + skipping the probe — mirroring the real boot path (resolveRemoteBackend), which treats a mint failure as a hard "session expired" auth error.Validation
npm run test:desktop:platforms→ 62/62 passing (57 prior + 5 newresolveTestWsUrlcases).Closes #39098. Original commits cherry-picked with @xxxigm's authorship preserved.
Infographic