Skip to content

fix: reconcile session state after desktop reconnect - #71475

Open
javierdici wants to merge 3 commits into
NousResearch:mainfrom
javierdici:codex/desktop-reconcile-reconnect-2026-07-25
Open

fix: reconcile session state after desktop reconnect#71475
javierdici wants to merge 3 commits into
NousResearch:mainfrom
javierdici:codex/desktop-reconcile-reconnect-2026-07-25

Conversation

@javierdici

@javierdici javierdici commented Jul 25, 2026

Copy link
Copy Markdown

Summary

  • publish a monotonic per-profile connection epoch on every real WebSocket open
  • reconcile the routed session when that epoch advances even if React batches the state transition
  • reuse the current warm-session session.activate path to rebind the replacement transport and refresh authoritative messages/running state
  • clear an impossible backend running/inflight_turn projection when its prompt worker has already exited
  • authenticate remote readiness probes so OAuth/password-gated legacy backends can reconnect after a desktop restart

Root cause

If the WebSocket disconnected during a turn, the backend could finish and persist the assistant final while its transport was detached. A fast closed -> connecting -> open cycle could be batched so the route hook rendered only the final open value and never ran selected-session reconciliation. Desktop could therefore keep showing Thinking until the user sent another message.

The affected live session also exposed a dead prompt worker with running=true, so an authoritative activation still reproduced the stale busy projection. The gateway now reconciles that impossible state before returning a live payload, while leaving live workers and the brief pre-registration window untouched.

During live installation, current Desktop main also found the deployed 0.18.2 remote through Test remote but failed bootstrap with 401 no_cookie: readiness probing used the public/token fetch path instead of the authenticated Electron session. OAuth-gated readiness now uses that session for both /api/health and the legacy /api/status fallback.

Tests

  • npm run test:ui -- src/app/session/hooks/use-route-resume.test.tsx src/app/session/hooks/use-session-actions.test.tsx (49 passed)
  • npm run test:desktop:platforms -- electron/backend-health.test.ts (7 passed)
  • scripts/run_tests.sh tests/test_tui_gateway_server.py -q -k session_activate (4 passed)
  • ruff check tui_gateway/server.py tests/test_tui_gateway_server.py
  • npm run typecheck
  • npm run lint -- --quiet
  • npm run build

Live proof

Installed a clean local macOS build from 19656e12c5f4, started a harmless 10-second response, quit Desktop while it was running, waited for the detached backend to finish, and relaunched. Desktop displayed HERMES_RECONNECT_RECOVERY_OK and returned to an idle composer without a rescue message or stale Stop control.

@javierdici
javierdici force-pushed the codex/desktop-reconcile-reconnect-2026-07-25 branch from 9d43c23 to e1f27d7 Compare July 25, 2026 17:19
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #58109 also addresses Desktop reconnects, but it reconciles stale busy state across sessions; this patch reliably re-activates the selected routed session after a batched reconnect. The mechanisms are complementary, not duplicates.

@javierdici javierdici changed the title fix(desktop): reconcile active session after websocket reconnect fix: reconcile session state after desktop reconnect Jul 25, 2026
@javierdici
javierdici marked this pull request as ready for review July 25, 2026 17:42
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating the reconnect path and covering the batched-state case.

Problems

  • The readiness-probe portion is already superseded by the stronger current-main implementation: apps/desktop/electron/backend-health.ts:120-160 distinguishes credentialed 401/403 from anonymous gated fallback, and apps/desktop/electron/main.ts:5052-5099 selects bearer/cookie/token probe credentials. This landed in 8d025489cf and 75456183b6.
  • The gateway handler was moved by the TUI method split: session.activate is now tui_gateway/methods_session.py:764-785, while _live_session_payload is tui_gateway/server.py:7516-7555.

Suggested changes

  • Preserve the epoch-based reconnect reactivation. Current apps/desktop/src/app/session/hooks/use-route-resume.ts:107-157 only recognizes a rendered closed-to-open transition, so the reported batched reconnect remains possible.
  • Port dead-worker reconciliation into the current _live_session_payload before it snapshots running and inflight; current cleanup is limited to the Stop path at tui_gateway/methods_session.py:2734-2753.
  • Omit the readiness-probe commits during salvage because current main already provides broader behavior and coverage.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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 30, 2026
@100yenadmin

Copy link
Copy Markdown

The still-relevant connection-epoch and dead-worker recovery behavior from this PR has been salvaged onto current main in #83166. Javier SM's original commits are preserved as authored; the superseded readiness-auth commit is intentionally excluded. This cross-link does not close or rewrite this PR.

@alt-glitch alt-glitch added P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists and removed P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 10, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Superseded by #83166, which salvages this PR's still-relevant connection-epoch and dead-worker recovery behavior onto current main while preserving the original authorship.

@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Superseded by the active salvage #83166, which carries this PR's still-relevant reconnect recovery work.

@alt-glitch alt-glitch removed the duplicate This issue or pull request already exists label Aug 10, 2026
@alt-glitch alt-glitch added the comp/tui Terminal UI (ui-tui/ + tui_gateway/) label Aug 10, 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/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-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