Skip to content

fix(tui): add WebSocket heartbeat + auto-reconnect for silent drops (#32997) - #60727

Open
indigokarasu wants to merge 2 commits into
NousResearch:mainfrom
indigokarasu:fix/tui-ws-heartbeat-autoreconnect
Open

fix(tui): add WebSocket heartbeat + auto-reconnect for silent drops (#32997)#60727
indigokarasu wants to merge 2 commits into
NousResearch:mainfrom
indigokarasu:fix/tui-ws-heartbeat-autoreconnect

Conversation

@indigokarasu

@indigokarasu indigokarasu commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

What / Why

Issue #32997: the Ink TUI (hermes --tui) WebSocket client had no heartbeat and
no auto-reconnect. A silent drop (macOS sleep, proxy idle timeout, VPN reconnect)
kills the TCP socket without a close event, so the client hangs forever —
input is blocked and only Ctrl+C recovers.

These four issues (#46563, #40164, #53142, #32997) share one root cause: the
WebSocket transport has no unified resilience contract. Three facets are already
covered by in-flight PRs and are intentionally NOT touched here:

This PR closes the remaining facet: the TUI client now sends a periodic ping
keepalive, detects dead/silent connections, and auto-reconnects with exponential
backoff. Intentional kill() is honored (no reconnect loop).

Supersedes #30114 (exit-event-only restart, could not heal silent drops). Credit
to @sbiesaar for the original attempt.

How to test

  • npm run typecheck --prefix ui-tui
  • npm test --prefix ui-tui (gatewayClient connect/reconnect tests)
  • npx eslint src/gatewayClient.ts src/__tests__/gatewayClient.test.ts

Platforms tested

Linux (Node undici WebSocket). Logic is transport-only; ping is guarded for
environments without WebSocket.ping.

Fixes NousResearch#32997

The Ink TUI WebSocket client had no heartbeat and no auto-reconnect, so a
silent connection drop (macOS sleep, proxy idle timeout, VPN reconnect) left
the UI stranded at a dead socket with only Ctrl+C as an escape. Add a periodic
ping keepalive plus dead-connection detection that forces a reconnect with
exponential backoff; never reconnect after an intentional kill().

Supersedes NousResearch#30114 (which only restarted on the exit event and could not heal
silent drops) by moving reconnect into the transport client and adding the
heartbeat.
@indigokarasu
indigokarasu force-pushed the fix/tui-ws-heartbeat-autoreconnect branch from ee728bd to 2b5c7ff Compare July 8, 2026 06:18
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jul 8, 2026
@indigokarasu
indigokarasu marked this pull request as ready for review July 8, 2026 06:49

@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 targeting a real silent-drop gap in the TUI transport. Two correctness issues need resolution before this can be safely salvaged.

Problems

  • ui-tui/src/gatewayClient.ts:242 treats absence of inbound application frames as a dead socket. tui_gateway/ws.py:319-337 sends gateway.ready then waits for client input, so a healthy idle client has no messages to refresh lastActivityAt. The local undici WebSocket declaration exposes no ping/pong API (node_modules/undici/types/websocket.d.ts:16-66), so the optional call at PR line 237 is not an acknowledged heartbeat. This will deliberately close healthy idle sockets; the new test at ui-tui/src/__tests__/gatewayClient.test.ts:497 currently asserts that behavior.
  • ui-tui/src/gatewayClient.ts:350 schedules retry after the synchronous exit emission. The existing subscriber already invokes gw.start() for session recovery (ui-tui/src/app/useMainApp.ts:795-821), so the subsequently-created timer later replaces that fresh transport.

Suggested changes

  • Add a server-supported, acknowledged heartbeat and test that healthy idle sockets stay connected.
  • Use one recovery owner, and test an attached client with the current exit subscriber to prove a single reconnect occurs.

Automated hermes-sweeper review.

Comment thread ui-tui/src/gatewayClient.ts Outdated
Comment thread ui-tui/src/gatewayClient.ts Outdated
@indigokarasu
indigokarasu marked this pull request as draft July 10, 2026 15:42
@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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 10, 2026
@indigokarasu

Copy link
Copy Markdown
Contributor Author

Addressed the review in 383a000d6af33dee5b562e558b63014e7fa866d8.

  1. Replaced the unacknowledged idle-timeout path with a JSON-RPC gateway.ping heartbeat. The client now waits for a server response before treating the socket as dead.
  2. Added gateway.ping handling in the TUI WebSocket server.
  3. Moved reconnect scheduling before synchronous exit emission. An immediate gw.start() subscriber now cancels the pending timer instead of creating a second transport.
  4. Added regressions for healthy idle connections, missing heartbeat acknowledgements, and the exit-subscriber reconnect race.

Validation:

  1. npm test --prefix ui-tui -- src/__tests__/gatewayClient.test.ts passed, 17 tests.
  2. npm run typecheck --prefix ui-tui passed.
  3. cd ui-tui && npx eslint src/gatewayClient.ts src/__tests__/gatewayClient.test.ts passed.

@indigokarasu
indigokarasu marked this pull request as ready for review July 10, 2026 16:08
@indigokarasu

Copy link
Copy Markdown
Contributor Author

@teknium1 this is ready for another look. I addressed the review feedback and moved the PR out of draft.

@100yenadmin

Copy link
Copy Markdown

The still-relevant TUI heartbeat and bounded reconnect behavior from this PR has been salvaged onto current main in #83166. Indigo Karasu's original commits are preserved as authored, with explicit credit in the combined PR. This cross-link does not close or rewrite this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists 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-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

4 participants