Skip to content

fix: harden desktop turn recovery - #46837

Closed
Reza2kn wants to merge 4 commits into
NousResearch:mainfrom
Reza2kn:fix/desktop-turn-recovery-rebase
Closed

fix: harden desktop turn recovery#46837
Reza2kn wants to merge 4 commits into
NousResearch:mainfrom
Reza2kn:fix/desktop-turn-recovery-rebase

Conversation

@Reza2kn

@Reza2kn Reza2kn commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • hardens Desktop active-turn recovery across gateway reconnect/resume flows
  • adds gateway-side inflight turn retention/replay for running and failed turns
  • blocks sends from detached visible transcripts so context is not silently lost
  • emits terminal message.complete error frames with recoverable partial state when turns fail
  • fixes Desktop lint/test fallout after the recovery hardening changes

Reliability coverage

  • WebSocket reconnect/resume during active turns
  • gateway inflight_turn replay/retention
  • detached-session send guard
  • terminal failed-turn completion/error semantics with partial replay state

Test Plan

  • uv run --extra dev --extra messaging python -m pytest tests/tui_gateway/test_protocol.py tests/test_tui_gateway_ws.py -o 'addopts=' -q
  • npm run lint --workspace apps/desktop
  • npm run typecheck --workspace apps/desktop
  • npm run test:ui --workspace apps/desktop
  • npm run test:desktop:platforms --workspace apps/desktop

@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists labels Jun 15, 2026
@Reza2kn
Reza2kn force-pushed the fix/desktop-turn-recovery-rebase branch from c27b95d to 8fe1e76 Compare June 15, 2026 19:51
@alt-glitch alt-glitch added comp/desktop Electron desktop app (apps/desktop/*) 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 and removed comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 26, 2026

@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 the recovery work. The reconnect and terminal-error direction is useful, but one failed-turn path remains unprotected.

Problems

  • tui_gateway/server.py:9140-9174 classifies a returned result["error"] as status="error", emits message.complete, then clears inflight_turn. The PR's new terminal_error_emitted guard only becomes true in the exception handler, so a normal returned error result still has no replay state if its terminal frame is lost during reconnect.

Suggested changes

  • Route returned error results through the retained terminal-error path as well, preserving any raw or streamed partial text, and add a regression test covering run_conversation() returning an error result followed by session.resume.
  • Port the Desktop portions onto the current split hooks (use-prompt-actions/submit.ts, use-session-actions/index.ts, and use-message-stream/gateway-event.ts) rather than applying the pre-split files.

Automated hermes-sweeper review.

Comment thread tui_gateway/server.py
@@ -6089,7 +6188,8 @@ def _stream(delta):
with session["history_lock"]:
session["running"] = False
session["last_active"] = time.time()

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.

This guard only retains state after an exception. A normal run_conversation() result with error still takes the regular completion path, leaves terminal_error_emitted false, and clears inflight_turn; retain that logical-error path too so reconnect can replay it.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 14, 2026
@OutThisLife

Copy link
Copy Markdown
Collaborator

Superseded by #71184, which re-authors the tui_gateway failed-turn retention onto current main and also routes the returned-error result path through retention (the gap flagged in review), with regression tests for both paths. The desktop portions were rebuilt onto the split hooks. Thanks @Reza2kn, credited via Co-authored-by.

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

Labels

comp/desktop Electron desktop app (apps/desktop/*) comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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