Skip to content

fix(bot-mode): keep active turns running after viewer detach - #94697

Open
dokterdok wants to merge 2 commits into
NousResearch:mainfrom
dokterdok:fix/desktop-bot-turn-detach
Open

fix(bot-mode): keep active turns running after viewer detach#94697
dokterdok wants to merge 2 commits into
NousResearch:mainfrom
dokterdok:fix/desktop-bot-turn-detach

Conversation

@dokterdok

@dokterdok dokterdok commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Start a long task in Bot A, then open Bot B or quit Desktop. Nobody pressed
Stop, but current gateway behavior can interrupt Bot A one WebSocket-orphan
grace period later and lose the remaining work.

This makes viewer loss different from Stop. A Bot-owned turn that the backend
already accepted keeps running while the user switches Bots or gateways,
navigates elsewhere, or quits Desktop. Returning from the same or another
client rehydrates its durable Bot Chat history.

Ordinary Sessions keep their existing orphan cleanup. Explicit Stop,
session.interrupt, sidecar close_on_disconnect, provider/tool limits, and
gateway shutdown remain authoritative.

Why the boundary is explicit

The gateway normally interrupts a running session after its final WebSocket
viewer has been gone for the configured grace period. That protects generic
sessions from consuming compute after a killed client, but it contradicts Bot
Mode's backend-resident work contract.

This PR adds an opt-in per-session policy. While an opted-in Bot turn is
running, the orphan reaper preserves it. After the turn settles, the detached
runtime is reclaimed normally.

Related work

Changes Made

  • Gateway create, resume, activate, prompt-submit, and orphan-reaper paths carry
    optional preserve_running_on_disconnect state.
  • A current backend recognizes canonical Bot Chats at turn start, including
    compressed continuation lineage, so older Desktop clients gain direct-Bot
    protection.
  • Typed Desktop Bot workspace hooks request the policy for create, resume,
    activate, and submit.
  • canonical-chat.ts and group-turns.ts request it for direct Bot Chats and
    Group Chat member sessions.
  • Behavioral tests keep ordinary Sessions, explicit Stop, and sidecar siblings
    unchanged.

Compatibility

The request field is optional. Older gateways ignore it and retain their
previous disconnect behavior. The continuity guarantee begins on a backend
containing this server policy.

This does not make an app-managed local backend survive termination of its own
process. Persistent hosting is a separate lifecycle boundary.

For a Group Chat, this preserves a member turn already admitted by its gateway.
It does not by itself move room coordination out of Desktop; hosted Group Chat
authority is layered separately.

How to Test

  1. Start a long Bot turn, then navigate away for longer than the WebSocket
    orphan grace period.
  2. Observe the Bot complete instead of ending during its API/tool call.
  3. Repeat on a separately hosted backend, quit Desktop, then reopen and confirm
    the completed timeline is present.
  4. Press Stop; it must still interrupt immediately.
  5. Repeat with an ordinary Sessions chat; its existing cleanup is unchanged.

Validation

Post-#96726 head 72fbcd0303 on main 7eee066c30:

  • gateway lifecycle suite: 623 passed
  • targeted Desktop lifecycle tests: 254 passed
  • complete typed Bot Mode suite: 57 files / 557 tests passed
  • Desktop renderer, Electron, and E2E typecheck: passed
  • Ruff and git diff --check: passed
  • ESLint: 0 errors; one unchanged existing hook warning
  • all selected GitHub checks on this exact head: passed

Real two-gateway UAT on the integrated field build reproduced the orphan
interruption, then verified navigation-away, Desktop hard-quit, backend
completion, and transcript rehydration with this policy enabled. The current
PR is the narrow exact rebase of that lifecycle boundary; cross-gateway room
coordination remains outside this PR.

The two commits remain independently reviewable: backend lifecycle policy,
then typed Desktop opt-in wiring.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 25, 2026
@dokterdok
dokterdok force-pushed the fix/desktop-bot-turn-detach branch from 5d1b55c to 38781f6 Compare August 25, 2026 11:36
@Enough1122

Copy link
Copy Markdown
Contributor

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

This PR adds a preserve_running_on_disconnect flag that prevents Bot Mode turns from being interrupted when the desktop viewer detaches (navigates away, switches bots, or quits the app). The flag is threaded through all Bot Mode RPCs (session.create, session.resume, session.activate, prompt.submit) on both the desktop client and TUI gateway. The orphan reaper checks the flag before interrupting a running turn, and _sync_bot_capabilities auto-applies it for sessions titled "Bot Chat" (including compressed lineage roots) so older Desktop clients are covered. The test coverage is comprehensive — covering the preserve path, capability sync adoption, prompt.submit adoption, compressed lineage inheritance, non-bot exclusion, and session.activate adoption.

A few technical concerns:

  1. Unbounded polling for preserved sessions: In the orphan reaper (server.py:766-777), when preserve_running_on_disconnect is set and the turn is running, the reaper reschedules with _WS_ORPHAN_REAP_GRACE_S indefinitely — there's no max-polls safety valve like the non-preserved path has (_WS_ORPHAN_INTERRUPT_REAP_MAX_POLLS). If a Bot Mode turn hangs (e.g., a stuck tool call, a deadlocked MCP server, a model that never responds), the reaper polls forever without force-reaping. Consider adding a max-polls cap for preserved sessions (perhaps much larger than the interrupt path's cap) to eventually reclaim stuck sessions.

  2. Flag is never downgraded: Once preserve_running_on_disconnect is set to True, nothing in the code sets it back to False. The comment at methods_session.py:688 explains this is intentional during a running turn, but after the turn settles, the session retains the flag. If the same session is later reused for a non-Bot turn (e.g., a user types into a former Bot Chat), it would still preserve on disconnect — potentially leaving orphaned running turns the user expects to stop. Consider clearing the flag when the turn finishes, or on the next non-preserved submit.

  3. Title-based auto-detection fragility: _sync_bot_capabilities (server.py:5730) auto-applies the flag based on the session title being "Bot Chat". If a user renames their Bot Chat session, the flag won't be applied on future capability syncs. The root-lookup for compressed sessions (server.py:5734) mitigates this for compression, but a manual rename still breaks detection. Consider using a session metadata flag (set at creation time) rather than title-based detection.

  4. DB query on every capability sync: The root-lookup at server.py:5734 (db.get_conversation_root(key)) adds a database query to every _sync_bot_capabilities call for non-"Bot Chat"-titled sessions. If capability sync runs frequently (e.g., on every turn boundary), this could add latency. Consider caching the root lookup result per session.

  5. preserve_running_on_disconnect in _reuse_live_response: At methods_session.py:638, the flag is set on a live session when preserve_running_on_disconnect is in the params. But at line 691, it's also set when reusing a live session. The comment says "a later reuse without the optional flag must not downgrade its detach policy" — but the code at line 691 sets it unconditionally based on the current params, not the existing flag. If the params don't include the flag, preserve_running_on_disconnect would be False, and the session's existing True would be... actually, looking more carefully, the code at 691 only sets it if preserve_running_on_disconnect: — so it only upgrades, never downgrades. This is correct, but the comment could be clearer.

The compression-lineage root lookup is a thoughtful detail — without it, a compressed Bot Chat would lose its background-work policy exactly when long work needs it most.

@dokterdok

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review. I pushed 2701aa7e0df and reconciled the points against the lifecycle contract:

  • Positive canonical Bot Chat lineage is now cached on the live session, so compressed continuation tips do not repeat two DB reads at every turn boundary. Negative classification is not cached.
  • The orphan regression now runs past an artificially tiny ordinary-session force-reap budget and proves it is not a hidden lifetime cap for opted-in Bot work; the turn is reclaimed immediately after it settles.
  • The preserve flag is intentionally monotone only for that live Bot session. It disappears with the session record, and a settled detached runtime is still reaped.
  • I did not add a viewer-driven max runtime: that would reintroduce the bug as a delayed implicit Stop. Provider/tool/iteration limits, explicit interrupt/close, sidecar ownership, and gateway shutdown remain authoritative.
  • Manual retitle identity is broader canonical-session work tracked by [Bug] Retitling a canonical Bot Chat breaks Bot Mode resolution — title collision mints orphan intro sessions on every click #92473; this PR keeps the current exact-title ruling rather than adding a second metadata identity.

Validation on the new head: tests/test_tui_gateway_server.py 617 passed; Ruff and git diff --check are clean.

@dokterdok
dokterdok force-pushed the fix/desktop-bot-turn-detach branch from 2701aa7 to ae155d5 Compare August 26, 2026 10:03
@dokterdok
dokterdok force-pushed the fix/desktop-bot-turn-detach branch from ae155d5 to 35b5d16 Compare August 27, 2026 21:56
@dokterdok
dokterdok marked this pull request as draft August 28, 2026 21:30
dokterdok added a commit to dokterdok/hermes-agent that referenced this pull request Aug 28, 2026
Recreates the backend-only portions of NousResearch#94697 commits 8cb3e09 and 35b5d16 against post-NousResearch#96726 main. Explicit interrupt, Stop, session close, and backend shutdown remain authoritative.
@dokterdok
dokterdok force-pushed the fix/desktop-bot-turn-detach branch from 35b5d16 to 580ef43 Compare August 28, 2026 22:24
Recreates the backend-only portions of NousResearch#94697 commits 8cb3e09 and 35b5d16 against post-NousResearch#96726 main. Explicit interrupt, Stop, session close, and backend shutdown remain authoritative.
@dokterdok
dokterdok force-pushed the fix/desktop-bot-turn-detach branch from 580ef43 to 72fbcd0 Compare August 29, 2026 05:47
@dokterdok
dokterdok marked this pull request as ready for review August 29, 2026 06:25
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/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