Skip to content

fix(tui): preserve detached sessions with active work - #49900

Open
omegazheng wants to merge 1 commit into
NousResearch:mainfrom
omegazheng:tui/ws-orphan-reap-safety
Open

fix(tui): preserve detached sessions with active work#49900
omegazheng wants to merge 1 commit into
NousResearch:mainfrom
omegazheng:tui/ws-orphan-reap-safety

Conversation

@omegazheng

Copy link
Copy Markdown
Contributor

Summary

  • Preserve detached TUI WebSocket sessions that still have resumable work.
  • Treat sessions with an in-flight turn, in-progress agent build, or pending prompt/approval as not orphaned.
  • Pass the session id into the orphan check so pending prompts can be detected before reaping.

Why

A disconnected desktop/TUI WebSocket transport should not cause active or resumable work to be closed by the orphan reaper. The reaper should only close sessions that are detached and have no active turn, build, or pending user input.

Tests

  • Added coverage for detached sessions that are still building, still handling an in-flight turn, or waiting on a pending prompt.
  • Verified locally:
    • uv run python -m pytest tests/test_tui_gateway_server.py -q -o 'addopts=' -k 'ws_orphan_reap'
    • Result: 4 passed, 278 deselected

@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/) comp/gateway Gateway runner, session dispatch, delivery labels Jun 21, 2026
@JLDynamics

Copy link
Copy Markdown

Additional finding from issue #52089 (reported by a user experiencing this bug):

Even when your ws_orphan_reap fix prevents session reaping, there is a SECOND issue: after sleep/wake, the desktop app UI still loses the session reference even though the data persists in state.db. The session survives with end_reason = "active" and all messages intact, but the UI shows a blank conversation.

This means the reaper fix alone (#49900) is necessary but not sufficient. The desktop app also needs to properly recover the session reference after WebSocket reconnection (#47297).

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused lifecycle fix. Current main still has the relevant gap: tui_gateway/server.py:751-762 classifies a detached session using only _finalized, running, and transport state, and tui_gateway/server.py:775-788 uses that result to close it. Meanwhile, _sess() can start an agent build while running remains false (tui_gateway/server.py:1484-1489), and _start_agent_build() marks that build in progress (tui_gateway/server.py:1321-1325).

The proposed guards align with the established policy in the longer-lived reapers, which already spare pending and not-ready sessions (tui_gateway/server.py:860-867, 908-917). The added regression cases are focused on the missing WebSocket-specific exclusions. The separate desktop reconnection/reference issue noted in the PR discussion remains outside this change's scope.

Automated hermes-sweeper review.

@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:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 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/gateway Gateway runner, session dispatch, delivery 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-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