Skip to content

fix(gateway): re-bind session transport to a surviving window on pop-out close - #86039

Open
ayushnangia wants to merge 1 commit into
NousResearch:mainfrom
ayushnangia:fix/popout-stream-rebind
Open

fix(gateway): re-bind session transport to a surviving window on pop-out close#86039
ayushnangia wants to merge 1 commit into
NousResearch:mainfrom
ayushnangia:fix/popout-stream-rebind

Conversation

@ayushnangia

Copy link
Copy Markdown
Contributor

Summary

Fixes #83716: closing a pop-out window strands the session's live stream — the original window never resumes receiving events until a manual re-resume.

Root cause (as diagnosed by @CharlesR-sudo in the issue): live sessions hold a single transport. A pop-out window's session.resume rebinds it to the pop-out's socket; on pop-out close, _close_sessions_for_transport parked the session on the drop sentinel (_detached_ws_transport) and scheduled the orphan reap — nothing re-bound the original window.

Changes

  • tui_gateway/server.py::_live_session_payload: every transport that shows a session is now recorded as a viewer (stamped timestamp), alongside the transport bind.
  • tui_gateway/server.py::_close_sessions_for_transport: on disconnect, the closing transport is removed from the viewers; if a surviving live viewer exists, the session re-binds to the most recent one (dead sockets filtered via _transport_is_dead) and no orphan reap is scheduled. The drop sentinel + grace reap remain the path for the last viewer — single-window behavior is unchanged.

Semantics

case behavior
pop-out opens (2nd window) last-viewer wins — the first window pauses (same as today; full multi-window fan-out is the eventual upgrade, deliberately out of scope)
pop-out closes session re-binds to the original window automatically; events flow again, no manual click
last viewer closes drop sentinel + grace reap, exactly as before
remaining viewer's socket already dead skipped for re-bind → detached + reap (grace machinery still owns recovery)

Validation

  • scripts/run_tests.sh tests/test_tui_gateway_server.py554 passed, 1 failed — the one failure is the pre-existing flaky test_compute_host_turn_end_updates_metadata_mirror (passes in isolation, untouched by this diff).
  • Red proof: with the change stashed, both new rebind/registration tests fail.
  • 4 new regression tests: rebind-to-surviving-viewer, last-viewer-detaches (unchanged path), dead-viewer-skipped, viewer registration on resume.

Root cause credit: @CharlesR-sudo (#83716). Family F member in the stall triage #84047.

@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 14, 2026
@ayushnangia

Copy link
Copy Markdown
Contributor Author

CI note: Python slice 3/12 failed. Local evidence says flake, not this diff: scripts/run_tests.sh tests/test_tui_gateway_server.py -> 554 passed, 0 failed here, and the one test I've seen flake in this file (test_compute_host_turn_end_updates_metadata_mirror) fails intermittently under parallel runs and passes on re-run. This PR's diff only touches _live_session_payload / _close_sessions_for_transport. Worth a re-run before review.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(gateway): re-bind session transport to a surviving window on pop-out close

  1. tui_gateway/server.py (_live_session_payload, ~8250): the viewers dict is only ever pruned inside _close_sessions_for_transport for the closing transport. A long-lived session that has had several pop-out windows (or transports that died without a clean close) accumulates entries indefinitely. Consider pruning dead viewers on each register or during the orphan reap.
  2. tui_gateway/server.py (_close_sessions_for_transport, ~1132): the re-bind picks the most recent viewer by timestamp. _live_session_payload registers any resume/activate, so a window that only briefly activated to peek becomes "most recent" and could win the re-bind over the primary window. Best-effort is fine, but a tie-break that prefers the longest-lived viewer (or the one whose write() has been exercised) would be more predictable.
  3. The re-bind path does not touch last_active or emit any event to the surviving transport — acceptable for a silent fix, but if the TUI relies on transport identity for anything else (keyboard focus, stream state), verify those consumers read session["transport"] live rather than caching it at open time.

…out close

Live sessions hold one transport; a pop-out window's session.resume
rebinds it, and on pop-out close the disconnect path parked the session
on the drop sentinel — the original window never received stream events
again until a manual re-resume (NousResearch#83716).

Sessions now track every transport that has shown them (viewers, stamped
in _live_session_payload). _close_sessions_for_transport re-binds to the
most recent surviving viewer instead of detaching when one exists; dead
viewers are filtered; the drop sentinel + grace reap remain the path for
the last viewer. Root cause and repro by CharlesR-sudo on NousResearch#83716.
@ayushnangia
ayushnangia force-pushed the fix/popout-stream-rebind branch from 1ab13a8 to ba29ce8 Compare August 17, 2026 19:35
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: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.

[Bug] Desktop: opening a session in a new window freezes live updates in the original window

3 participants