Skip to content

fix(tui-gateway): interrupt turns after websocket disconnect - #90373

Closed
Kyzcreig wants to merge 2 commits into
NousResearch:mainfrom
ANG-Ventures:fix/ws-client-disconnect-interrupt-upstream
Closed

fix(tui-gateway): interrupt turns after websocket disconnect#90373
Kyzcreig wants to merge 2 commits into
NousResearch:mainfrom
ANG-Ventures:fix/ws-client-disconnect-interrupt-upstream

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • after the existing WebSocket reconnect grace, interrupt a still-detached running turn through the same core used by session.interrupt
  • use the compute-host control channel for isolated turns and the existing in-process interrupt otherwise
  • preserve reconnect-within-grace cancellation, active-delegation deferral, sidecar immediate-close behavior, partial history, and single-owner teardown/reap semantics
  • serialize live-session rebinds so a late session.resume cannot reattach after the grace-expired client_gone interruption claim

Why

On 2026-07-05, a certification client was SIGKILLed while approximately 20 turns were in flight. The WebSocket sessions detached, but _ws_session_is_orphaned() intentionally returned false while running remained true. Nothing then requested interruption, so the orphaned turns continued consuming compute and wedged the dashboard.

This change extends the existing _close_sessions_for_transport()_schedule_ws_orphan_reap() teardown path. It does not add a parallel disconnect path or alter close_on_disconnect sidecar handling. HERMES_TUI_WS_ORPHAN_REAP_GRACE_S remains the configuration seam; zero still disables the reaper.

Race handling

Reconnect or session.resume during the grace still cancels interruption. Once the grace callback atomically claims a detached session as client_gone, live-session rebinds revalidate session identity under the shared resume lock and reject late attachment instead of resurrecting the claimed session.

Verification

On current upstream main after rebasing:

  • RED with the six focused tests but without production changes: 4 failed, 2 passed
  • GREEN related gateway/compute-host/session-resume suite: 611 passed, 0 failed
  • the first upstream CI run exposed an existing fast-path DB-ownership fixture that mocked a live-session lookup without registering that session; the fixture now mirrors the production registry invariant (8/8 in that file, no production behavior change)
  • Ruff: passed on all changed Python files
  • py_compile: passed on all changed Python files
  • git diff --check: passed
  • independent Momus review, pass 2: APPROVE

The focused coverage proves isolated and in-process interruption, reconnect cancellation, active-delegation deferral, unchanged sidecar close behavior, and rejection of late resume after the client_gone claim.

Live AC-13 evidence

A real dashboard WebSocket client created an isolated compute-host turn executing sleep 120; the parent then hard-killed only the client process on the final integration tree carrying these production changes.

  • client exit: -9; kill_confirmed=true
  • abnormal WS disconnect: 16:59:56.012, code 1006, detached_sessions=1
  • client_gone interrupt: 17:00:16.021 — 20.009 seconds after disconnect
  • terminal result: [Command interrupted], exit code 130
  • turn completion: reason=interrupted_by_user
  • orphan reap: 17:00:18.251
  • harness-observed reap: 22.490 seconds
  • disposable sessions remaining: 0
  • AC-13 gate: PASS

@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 20, 2026
After the existing reconnect grace, route a still-detached running session through the same interrupt mechanism as session.interrupt. Preserve delegation deferral, sidecar teardown, partial history, and single-owner reap semantics.

Verified on upstream main: RED 4 failed/2 passed without production changes; GREEN 603 related gateway/compute-host tests. Ruff and py_compile passed. Momus pass 2: APPROVE.
@Kyzcreig
Kyzcreig force-pushed the fix/ws-client-disconnect-interrupt-upstream branch from f83b078 to 41c902a Compare August 20, 2026 00:19
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Reviewed by reviewer-e (AI automated review).

The right semantics for a real UX bug: a WS drop no longer orphans a mid-flight turn — the orphan reaper interrupts running turns (compute-host supervisor or in-process agent, clearing queued prompts under the history lock), then re-polls at 1s and only reaps once normal turn finalization settles; active delegations defer the reap; reattachment inside the grace window spares everything; and _reuse_live_response makes the client-gone claim + transport rebind atomic under the resume lock so a slow-path resume can't rebind a session whose interrupt is settling (clean 4009 "retry" instead). Six focused tests cover each branch including the delegation deferral and the untouched sidecar-close path. Findings:

  1. tui_gateway/server.py — the interrupt-then-reap poll chain (_WS_ORPHAN_INTERRUPT_REAP_POLL_S = 1.0) appears unbounded: if a turn never settles (agent thread hung in a syscall, supervisor lost), each fire schedules another timer forever and the detached session is parked indefinitely — trading the old leak-one-worker bug for leak-one-session-plus-timer-chain. Add a total-poll budget (e.g. N×grace or a fixed ceiling) after which you log loudly and force-reap, mirroring how the pre-existing stuck-running safety net used to break the deadlock.

  2. tui_gateway/methods_session.py — the old Stop handler contained a load-bearing safety net: when _run_thread was dead but running stayed true (crash skipped the run-loop finally), it force-cleared running/_clear_inflight_turn so the session couldn't be bricked at 4009 "session busy" until backend restart. That logic moved into _interrupt_session_turn, whose body isn't visible in this hunk — please confirm the extraction preserved the dead-thread force-clear verbatim (and ideally add a regression test for that specific desync through the new helper), since losing it would reintroduce the permanent-busy failure mode this net existed for.

  3. tui_gateway/server.py:_ws_session_is_orphaned — cosmetic: session is not None is evaluated after _ws_session_is_detached(session) has already been handed the possibly-None value (which it handles); fold the whole condition into the helper or reorder for readability.

…nding)

If an interrupted turn never settles (agent thread hung in a syscall,
supervisor lost), the 1s poll chain rescheduled forever — trading the old
leak-one-worker bug for leak-one-session-plus-timer-chain. After
_WS_ORPHAN_INTERRUPT_REAP_MAX_POLLS (60 = ~60s, 3x the default grace) the
reaper logs loudly and force-reaps, mirroring the pre-existing stuck-running
safety net's deadlock-breaking role. Focused suite 4 passed, 1 skipped.
@Kyzcreig

Copy link
Copy Markdown
Contributor Author

Good catch — the unbounded chain was real (each fire unconditionally rescheduled at 1s while running stayed true). Fixed in 98fb771f1a: the reaper now counts polls per session (_client_gone_interrupt_polls) and after _WS_ORPHAN_INTERRUPT_REAP_MAX_POLLS (60 ≈ 60s, 3× the default grace) it logs an ERROR naming the stuck session and force-reaps — mirroring the pre-existing stuck-running safety net's role of breaking the deadlock rather than parking a session + timer chain forever. Counter lives on the session dict so a genuine settle-then-new-turn cycle resets naturally with the session's lifecycle. Focused disconnect/orphan suite: 4 passed, 1 skipped.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via the consolidated session-recovery PR #93361 — both of your commits cherry-picked with authorship preserved (interrupt still-detached running turns after grace + the bounded poll chain; your 4009 settling guard kept intact). Thanks @Kyzcreig!

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.

4 participants