Skip to content

fix(gateway): reconcile finished prompt-run threads at session teardown - #89978

Open
100yenadmin wants to merge 1 commit into
NousResearch:mainfrom
100yenadmin:upstream/reconcile-finished-run-threads
Open

fix(gateway): reconcile finished prompt-run threads at session teardown#89978
100yenadmin wants to merge 1 commit into
NousResearch:mainfrom
100yenadmin:upstream/reconcile-finished-run-threads

Conversation

@100yenadmin

Copy link
Copy Markdown

What does this PR do?

When a prompt-run worker thread finishes (or dies) but its session still has running: True and a stale _run_thread reference, the next activation of that session can see it as busy and refuse to dispatch. This reconciles that state at session activation: if the recorded run thread is no longer alive, the inflight-turn state is cleared so the session is usable again.

This is a standalone, self-contained piece of the WebSocket-recovery work in #83166 — it has no dependency on the heartbeat wire contract (#89958) or the transport-ownership machinery, so it's filed on its own.

Related Issue

Part of the recovery work in #83166.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tui_gateway/server.py_reconcile_finished_run_thread(session) helper (clears inflight state when _run_thread is no longer alive), called from _live_session_payload.
  • tests/test_tui_gateway_server.py — one test that activating a session with a dead run thread clears the stale busy state.

How to Test

scripts/run_tests.sh tests/test_tui_gateway_server.py -k activate_clears_stale_busy

Fail-before (reconcile call removed): 1 failed (session still reports running). Pass-after: 1 passed.

Checklist

Code

  • I've read the Contributing Guide
  • Conventional Commits
  • Searched existing PRs
  • Only changes related to this fix
  • Ran the affected test
  • Added a test
  • Tested on my platform: macOS 15

Documentation & Housekeeping

  • Docs — N/A
  • cli-config.yaml.example — N/A
  • CONTRIBUTING.md/AGENTS.md — N/A
  • Cross-platform — N/A
  • Tool descriptions/schemas — N/A

Salvage-friendly: single commit on current main (13ce0c5c67), no dependencies — cherry-pick welcome, authorship preservation appreciated but optional.

A finished or dead prompt-run worker thread could leave session state
wedged: `running` and `inflight_turn` stayed pinned forever after the
worker had already exited (e.g. a dropped transport). Reconnect callers
read the live payload as their authority, so a stale busy projection
stranded the client on a spinner.

Reconcile that impossible state in `_live_session_payload` before it is
returned: if the session is marked running but its `_run_thread` has
exited, clear `running` and the inflight turn. A missing thread is left
untouched (other paths briefly set `running` before registering a
worker).

This is a standalone fix, independent of the WebSocket heartbeat wire
contract (NousResearch#89958); part of the NousResearch#83166 recovery work.

Receipts:
  scripts/run_tests.sh tests/test_tui_gateway_server.py \
    -k activate_clears_stale_busy
  fail-before (reconcile call removed): 1 failed
  pass-after: 1 passed, 0 failed
@Enough1122

Copy link
Copy Markdown
Contributor

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

Right reconciliation point and correct polarity: reconciling inside _live_session_payload under history_lock means every reconnect/activate consumer sees a truthful projection, and leaving a missing thread untouched avoids clobbering the brief window where running is set before worker registration — that asymmetry is documented in the docstring, which is what makes it safe. The dead-thread test pins both the payload shape (running: false, no inflight) and the session mutation. One nit:

  • tui_gateway/server.py:8841 — nit — is_alive() raising is treated as "leave everything pinned"; a comment naming that choice (fail-safe toward busy rather than idle) would clarify it isn't an oversight.

No blocking issues found.

— reviewer-b (automated review)

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.

3 participants