Skip to content

fix(desktop): reattach sessions after websocket reconnect - #45653

Closed
GwendalObsidian wants to merge 1 commit into
NousResearch:mainfrom
GwendalObsidian:fix/desktop-reattach-detached-sessions
Closed

fix(desktop): reattach sessions after websocket reconnect#45653
GwendalObsidian wants to merge 1 commit into
NousResearch:mainfrom
GwendalObsidian:fix/desktop-reattach-detached-sessions

Conversation

@GwendalObsidian

Copy link
Copy Markdown

Summary

Fixes a Desktop reconnect edge case where sessions that were mid-turn when the WebSocket dropped could remain visually stuck after the renderer reconnected.

This change:

  • notifies the app after a successful post-boot reconnect so it can explicitly reattach live stored sessions
  • resumes selected, working, and attention sessions after reconnect and remaps their runtime session ids/state
  • routes reconnect recovery through the owning profile gateway instead of always using the active gateway
  • keeps secondary profile reconnects awaitable so recovery does not race an in-flight reconnect
  • keeps WS-detached backend sessions on the orphan reaper until they either reattach or settle and become safe to reap

Why

A dropped Desktop WebSocket moves backend sessions to the detached/drop transport. Without an explicit session.resume after the fresh WebSocket opens, final events can continue to be emitted to the drop transport, leaving Desktop looking like the turn is still stuck.

There was also a backend cleanup gap: if the first orphan-reap timer fired while the detached session was still running, it skipped the session once and never checked again, so the session could become immortal after the turn settled.

Tests

  • venv/bin/python -m pytest tests/test_tui_gateway_server.py::test_ws_orphan_reap_reschedules_detached_mid_turn_until_it_settles tests/test_tui_gateway_server.py::test_ws_orphan_reap_disabled_when_grace_zero tests/test_tui_gateway_server.py::test_ws_orphan_reap_spares_reattached_session -q -o 'addopts='
  • npm --workspace apps/desktop run test:ui -- src/app/gateway/hooks/use-gateway-boot.test.tsx
  • npm --workspace apps/desktop run typecheck
  • npm --workspace apps/desktop run build
  • npx eslint src/app/desktop-controller.tsx src/app/gateway/hooks/use-gateway-boot.ts src/app/gateway/hooks/use-gateway-boot.test.tsx src/app/session/hooks/use-session-actions.ts src/types/hermes.ts src/store/gateway.ts

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/gateway Gateway runner, session dispatch, delivery labels Jun 13, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Correctness

  • Desktop app now reattaches sessions after WebSocket reconnect, preventing session loss after temporary disconnection.
  • use-gateway-boot.ts handles reconnection with proper state management.
  • gateway.ts store updated to track reconnection state.
  • tui_gateway/server.py server-side support for session reattachment.

Testing

  • use-gateway-boot.test.tsx tests the reconnection hooks.
  • test_tui_gateway_server.py additions test server-side session reattachment.

No Issues Found

  • No secrets, no debug artifacts.

Reviewed by Hermes Agent

@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 isolating both the detach/reconnect and orphan-lifetime paths. The server-side orphan-reap fix remains relevant: current tui_gateway/server.py:760-762 excludes running detached sessions, while _schedule_ws_orphan_reap() returns at :786-787 without scheduling another check after that turn settles.

Problems

  • The Desktop callback is only attached to useGatewayBoot in the primary reconnect path. Current main reconnects secondary profile sockets independently in apps/desktop/src/store/gateway.ts:167-176, and that path has no reattach notification. A detached turn on a secondary profile would therefore remain detached after its own socket reconnects.
  • Current main already owns foreground re-resume in apps/desktop/src/app/session/hooks/use-route-resume.ts:145-150. It deliberately re-runs resumeSession() on a closed→open transition even when the cached runtime ID appears active. A salvage should compose with that owner rather than add a parallel foreground reconciler.

Suggested changes

  • Keep the orphan-reap reschedule and regression test, rebased onto the current teardown code.
  • Drive background-session recovery from each profile gateway's open transition and add a secondary-profile reconnect regression.

Automated hermes-sweeper review.

onGatewayReady: g => {
gatewayRef.current = g
},
onReconnectReady: recoverDetachedSessions,

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.

onReconnectReady only runs after the primary useGatewayBoot socket reconnects. Secondary profile sockets reconnect independently; wire recovery to each profile gateway's open transition (and test a detached secondary-profile turn), otherwise that session remains on the detached transport.

@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 area/sessions Session lifecycle, resume, persistence, history labels Jul 14, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as implemented on main: the server half (mid-turn reap re-arm) landed via #93361 (salvaged #85598, first submitted in #44102 by @AIalliAI) with an interrupt-after-grace refinement, and the desktop half is covered by resetTileRuntimeBindings on reconnect (#93292) + force-redial of half-open sockets (#93361, salvaged #89092). Thanks @GwendalObsidian — both halves of your diagnosis were right.

@teknium1 teknium1 closed this Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) comp/gateway Gateway runner, session dispatch, delivery comp/tui Terminal UI (ui-tui/ + tui_gateway/) 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-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