fix(dashboard): bound WS ticket minting on the events + PTY sockets (supersedes #81931) - #81978
Conversation
ChatPage's connect awaits a single-use ticket from `api.buildWsUrl()` before `new WebSocket()`. That request produces no socket, so a rejection or a hang emits no `close` event and never arms PTY_CONNECTING_TIMEOUT_MS (set after the socket is constructed). The tab stranded on "connecting" with `connectInFlightRef` stuck true, which also suppresses the page-resume reconnect path. Give the ticket phase its own deadline and route both failure modes into the existing backoff. A `ticketSuperseded` flag invalidates a late ticket result so a timed-out attempt cannot open a socket behind the replacement it scheduled, and cleanup clears the timer on unmount. `scheduleReconnect` now takes `number | null` so an attempt that died before any socket existed omits the "(code N)" banner suffix instead of inventing one. Same bug class as the events-feed fix in the preceding commit, on the main chat surface. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
Mirrors the events-feed cases in ChatSidebar.test.tsx: a rejected ticket retries, a stalled ticket times out and its late resolution cannot open a superseded socket, and a settled ticket disarms the deadline so PTY_CONNECTING_TIMEOUT_MS remains the only guard on a wedged handshake (NS-591 regression). Both failure cases fail against ChatPage.tsx without the preceding fix. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
૮ >ﻌ< ა ci reviewran on 5acd662
|
There was a problem hiding this comment.
Pull request overview
Fixes a reconnection dead-end in the dashboard’s WebSocket flows by ensuring the pre-socket ticket-minting phase is also bounded by timeouts, so failures that occur before new WebSocket() exists still route into the existing backoff/retry state machines (events feed + PTY chat).
Changes:
- Add an events connect deadline (
EVENTS_CONNECT_TIMEOUT_MS) that covers ticket minting + handshake, with generation-guarding to prevent late tickets from creating superseded sockets. - Add a PTY ticket deadline (
PTY_TICKET_TIMEOUT_MS) that covers the pre-socket ticket request, wiring timeout/rejection into the existing PTY reconnect backoff and preventing late tickets from opening stale sockets. - Extend PTY reconnect scheduling to accept
number | nullso attempts that fail before any socket exists don’t fabricate close codes in the banner.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/pages/ChatPage.tsx | Adds a bounded ticket-mint phase and supersession guard for the PTY WebSocket connection attempt, feeding failures into the existing reconnect/backoff path. |
| web/src/pages/ChatPage.test.tsx | Adds PTY-specific tests for ticket rejection, ticket hang + late resolution, and the “ticket deadline disarmed once socket exists” regression guard. |
| web/src/lib/pty-reconnect.ts | Introduces PTY_TICKET_TIMEOUT_MS alongside existing PTY reconnect policy constants. |
| web/src/lib/events-reconnect.ts | Introduces EVENTS_CONNECT_TIMEOUT_MS for bounding events feed ticket minting + handshake. |
| web/src/components/ChatSidebar.tsx | Wraps the entire events socket connection attempt with a timeout and generation guard so pre-socket failures can’t strand reconnect state. |
| web/src/components/ChatSidebar.test.tsx | Adds/updates tests covering URL construction rejection, stalled URL request, and stalled handshake under the new connection deadline. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…supersedes NousResearch#81931) (NousResearch#81978) * fix(dashboard): retry stalled events feed reconnects * fix(dashboard): bound the PTY ticket request before the socket exists ChatPage's connect awaits a single-use ticket from `api.buildWsUrl()` before `new WebSocket()`. That request produces no socket, so a rejection or a hang emits no `close` event and never arms PTY_CONNECTING_TIMEOUT_MS (set after the socket is constructed). The tab stranded on "connecting" with `connectInFlightRef` stuck true, which also suppresses the page-resume reconnect path. Give the ticket phase its own deadline and route both failure modes into the existing backoff. A `ticketSuperseded` flag invalidates a late ticket result so a timed-out attempt cannot open a socket behind the replacement it scheduled, and cleanup clears the timer on unmount. `scheduleReconnect` now takes `number | null` so an attempt that died before any socket existed omits the "(code N)" banner suffix instead of inventing one. Same bug class as the events-feed fix in the preceding commit, on the main chat surface. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com> * test(dashboard): cover the PTY ticket connect deadline Mirrors the events-feed cases in ChatSidebar.test.tsx: a rejected ticket retries, a stalled ticket times out and its late resolution cannot open a superseded socket, and a settled ticket disarms the deadline so PTY_CONNECTING_TIMEOUT_MS remains the only guard on a wedged handshake (NS-591 regression). Both failure cases fail against ChatPage.tsx without the preceding fix. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com> --------- Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
…supersedes NousResearch#81931) (NousResearch#81978) * fix(dashboard): retry stalled events feed reconnects * fix(dashboard): bound the PTY ticket request before the socket exists ChatPage's connect awaits a single-use ticket from `api.buildWsUrl()` before `new WebSocket()`. That request produces no socket, so a rejection or a hang emits no `close` event and never arms PTY_CONNECTING_TIMEOUT_MS (set after the socket is constructed). The tab stranded on "connecting" with `connectInFlightRef` stuck true, which also suppresses the page-resume reconnect path. Give the ticket phase its own deadline and route both failure modes into the existing backoff. A `ticketSuperseded` flag invalidates a late ticket result so a timed-out attempt cannot open a socket behind the replacement it scheduled, and cleanup clears the timer on unmount. `scheduleReconnect` now takes `number | null` so an attempt that died before any socket existed omits the "(code N)" banner suffix instead of inventing one. Same bug class as the events-feed fix in the preceding commit, on the main chat surface. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com> * test(dashboard): cover the PTY ticket connect deadline Mirrors the events-feed cases in ChatSidebar.test.tsx: a rejected ticket retries, a stalled ticket times out and its late resolution cannot open a superseded socket, and a settled ticket disarms the deadline so PTY_CONNECTING_TIMEOUT_MS remains the only guard on a wedged handshake (NS-591 regression). Both failure cases fail against ChatPage.tsx without the preceding fix. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com> --------- Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
…supersedes NousResearch#81931) (NousResearch#81978) * fix(dashboard): retry stalled events feed reconnects * fix(dashboard): bound the PTY ticket request before the socket exists ChatPage's connect awaits a single-use ticket from `api.buildWsUrl()` before `new WebSocket()`. That request produces no socket, so a rejection or a hang emits no `close` event and never arms PTY_CONNECTING_TIMEOUT_MS (set after the socket is constructed). The tab stranded on "connecting" with `connectInFlightRef` stuck true, which also suppresses the page-resume reconnect path. Give the ticket phase its own deadline and route both failure modes into the existing backoff. A `ticketSuperseded` flag invalidates a late ticket result so a timed-out attempt cannot open a socket behind the replacement it scheduled, and cleanup clears the timer on unmount. `scheduleReconnect` now takes `number | null` so an attempt that died before any socket existed omits the "(code N)" banner suffix instead of inventing one. Same bug class as the events-feed fix in the preceding commit, on the main chat surface. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com> * test(dashboard): cover the PTY ticket connect deadline Mirrors the events-feed cases in ChatSidebar.test.tsx: a rejected ticket retries, a stalled ticket times out and its late resolution cannot open a superseded socket, and a settled ticket disarms the deadline so PTY_CONNECTING_TIMEOUT_MS remains the only guard on a wedged handshake (NS-591 regression). Both failure cases fail against ChatPage.tsx without the preceding fix. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com> --------- Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
…supersedes NousResearch#81931) (NousResearch#81978) * fix(dashboard): retry stalled events feed reconnects * fix(dashboard): bound the PTY ticket request before the socket exists ChatPage's connect awaits a single-use ticket from `api.buildWsUrl()` before `new WebSocket()`. That request produces no socket, so a rejection or a hang emits no `close` event and never arms PTY_CONNECTING_TIMEOUT_MS (set after the socket is constructed). The tab stranded on "connecting" with `connectInFlightRef` stuck true, which also suppresses the page-resume reconnect path. Give the ticket phase its own deadline and route both failure modes into the existing backoff. A `ticketSuperseded` flag invalidates a late ticket result so a timed-out attempt cannot open a socket behind the replacement it scheduled, and cleanup clears the timer on unmount. `scheduleReconnect` now takes `number | null` so an attempt that died before any socket existed omits the "(code N)" banner suffix instead of inventing one. Same bug class as the events-feed fix in the preceding commit, on the main chat surface. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com> * test(dashboard): cover the PTY ticket connect deadline Mirrors the events-feed cases in ChatSidebar.test.tsx: a rejected ticket retries, a stalled ticket times out and its late resolution cannot open a superseded socket, and a settled ticket disarms the deadline so PTY_CONNECTING_TIMEOUT_MS remains the only guard on a wedged handshake (NS-591 regression). Both failure cases fail against ChatPage.tsx without the preceding fix. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com> --------- Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
…supersedes NousResearch#81931) (NousResearch#81978) * fix(dashboard): retry stalled events feed reconnects * fix(dashboard): bound the PTY ticket request before the socket exists ChatPage's connect awaits a single-use ticket from `api.buildWsUrl()` before `new WebSocket()`. That request produces no socket, so a rejection or a hang emits no `close` event and never arms PTY_CONNECTING_TIMEOUT_MS (set after the socket is constructed). The tab stranded on "connecting" with `connectInFlightRef` stuck true, which also suppresses the page-resume reconnect path. Give the ticket phase its own deadline and route both failure modes into the existing backoff. A `ticketSuperseded` flag invalidates a late ticket result so a timed-out attempt cannot open a socket behind the replacement it scheduled, and cleanup clears the timer on unmount. `scheduleReconnect` now takes `number | null` so an attempt that died before any socket existed omits the "(code N)" banner suffix instead of inventing one. Same bug class as the events-feed fix in the preceding commit, on the main chat surface. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com> * test(dashboard): cover the PTY ticket connect deadline Mirrors the events-feed cases in ChatSidebar.test.tsx: a rejected ticket retries, a stalled ticket times out and its late resolution cannot open a superseded socket, and a settled ticket disarms the deadline so PTY_CONNECTING_TIMEOUT_MS remains the only guard on a wedged handshake (NS-591 regression). Both failure cases fail against ChatPage.tsx without the preceding fix. Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com> --------- Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
Summary
Supersedes #81931.
The dashboard opens three WebSockets, and each one first
awaits a fresh single-use ticket fromapi.buildWsUrl()before callingnew WebSocket(). That await is the whole problem: it produces no socket, so a ticket request that rejects or never settles emits nocloseevent and there is nothing for the retry logic to react to. The connection state machine stops where it stood.@helix4u caught and fixed this on the events feed in #81931 — the sidebar would strand on
reconnecting in 1s...forever. That diagnosis is correct and the fix is kept here essentially verbatim, as the first commit.The same structure exists on
ChatPage's PTY socket, which is the main chat surface rather than a sidebar title feed.PTY_CONNECTING_TIMEOUT_MS(NS-591) does not cover it: that timer is armed afternew WebSocket(url)returns, so a ticket request that never settles never arms it. The tab sits on "connecting" withconnectInFlightRefstucktrue, which additionally suppresses the page-resume reconnect path — so returning to the tab can't recover it either.ChatPagehad already solved the second half of this failure mode (a wedged handshake) and left the first half (the await gap) open; this PR closes it with the same shape, so the two halves match.HermesConsoleModalhas the third instance of the pattern but already catches, and has no retry loop to strand — left alone deliberately.What this PR does
EVENTS_CONNECT_TIMEOUT_MS, feeds setup failures and deadline expiry into the existing guarded backoff, and adds a generation guard so a late ticket can't create a superseded socket.PTY_TICKET_TIMEOUT_MSdeadline, routes rejection and expiry into the existingscheduleReconnectbackoff, and uses aticketSupersededflag so a timed-out attempt's late ticket cannot open a socket behind its replacement. Cleanup clears the timer and invalidates the attempt on unmount.scheduleReconnectnow takesnumber | nullso an attempt that died before any socket existed omits the(code N)banner suffix instead of inventing one.The new constant sits next to
PTY_CONNECTING_TIMEOUT_MSinweb/src/lib/pty-reconnect.ts, the module that already owns this policy — no new module, no new config key.What was dropped from #81931
Nothing. The original three commits' worth of behavior is the first commit here, unmodified.
Test plan
cd web && npx vitest run→ 27 files, 197 tests pass (194 onmain+ 3 new PTY cases)cd web && npm run check(typecheck + test + lint, the CI gate) → 0 errorsChatSidebar.tsxfails exactly the 3 events-feed tests; reverting onlyChatPage.tsxfails exactly the 2 new PTY failure-path tests. Neither set is a false green.PTY_CONNECTING_TIMEOUT_MSstays the only thing that may force-close a wedged handshake.Reported on Discord: https://discord.com/channels/1053877538025386074/1535696700121948190
Follow-up to #79524.
Credit: @helix4u (primary — original diagnosis and the events-feed fix, carried via
Co-authored-by).