Skip to content

fix(tui): enforce notification session ownership across all drain paths - #63317

Closed
abhibansal-sg wants to merge 1 commit into
NousResearch:mainfrom
abhibansal-sg:fix-tui-notification-session-ownership
Closed

fix(tui): enforce notification session ownership across all drain paths#63317
abhibansal-sg wants to merge 1 commit into
NousResearch:mainfrom
abhibansal-sg:fix-tui-notification-session-ownership

Conversation

@abhibansal-sg

Copy link
Copy Markdown
Contributor

Summary

Background-process completion notifications now deliver only to the session that owns them; a multi-session tui_gateway process can no longer inject one session's process output into another session's conversation.

Closes #35652.

Changes

  • Apply the ownership filter (owns_event callback / session_key equality) in ProcessRegistry.drain_notifications() to every session-scoped notification type — previously only type == "async_delegation" was filtered (bug: delegate_task async results delivered to wrong session/thread #58684/Desktop async delegation completions can revive an old session while follow-up prompts create a new session #55578 scope), so plain completion / watch_match / watch_disabled events were consumed by whichever session's post-turn drain or poller woke first.
  • Require positive ownership (_session_owns_notification_event, compression-lineage aware) before the TUI live poller creates a conversation turn from a completion — "not provably elsewhere" is no longer sufficient.
  • Mirror the same positive-ownership rule in the shutdown drain.
  • Never inject ownerless events into any conversation — they stay retrievable in the process registry (process tool log/poll), and callback exceptions fail closed with requeue.
  • Preserve legacy consume-everything behavior for single-session callers that pass no filter (classic CLI), and keep compression-lineage resolution so a compacted session still receives its own pre-compression events.

Behaviour

Before: with two live sessions in one hermes dashboard/--tui process, session A's terminal(background=true, notify_on_complete=true) completion could surface as an [IMPORTANT: Background process …] turn in session B while A was live — exactly the residual drain path flagged by @gvago (second unscoped drain site) and reproduced by @Evisolpxe in #35652. On the reporting machine this fired 6 times in one evening, deterministically, from the busiest session's post-turn drain.

After: completions route to their owning session only; foreign events are requeued for their owner; ownerless events surface in the registry, never in a chat. Verified live on the reproduction machine in both directions (own-process → own-session, and the original steal topology with the thief session mid-turn): 6 leaks/evening → 0.

Validation

Check Result
Targeted suites (scripts/run_tests.sh tests/test_tui_gateway_server.py tests/tui_gateway/) 667 passed, 0 failed (5× repeat, order-stable)
New regressions 5: two-session post-turn drain, poller requeue-for-owner, ownerless non-injection, compression-lineage delivery, fail-closed on broken ownership callback
RED control Production fix reverted to main with tests kept: regression tests fail (wrong-session adoption reproduced); restored: green
Narrowed tests 4 existing tests that codified ownerless adoption into conversations narrowed to registry/status behavior
Live reproduction 2-session dashboard topology from #35652: before 6 wrong-session injections/evening; after 0 in both directions
git diff --check Passed

Execution-context review of every modified call site: drain_notifications() stays synchronous in its caller with non-blocking queue ops; the poller/shutdown/post-turn drains run on their existing per-session daemon threads; a slow lineage DB lookup delays only that session's poller, never the JSON-RPC loop; raw-key matches avoid DB access entirely.

Infographic

Notification session ownership — before/after

@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/tools Tool registry, model_tools, toolsets sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P2 Medium — degraded but workaround exists labels Jul 12, 2026
yingliang-zhang added a commit to yingliang-zhang/hermes-agent that referenced this pull request Jul 15, 2026
Adapt the strongest NousResearch#63317 live-loop handoff regression and cover lineage lookup failure plus addressed live-loop orphans.

Co-authored-by: Abhinav Bansal <abhibansal-sg@users.noreply.github.com>
@abhibansal-sg

Copy link
Copy Markdown
Contributor Author

Closing in favor of #54785, which carries this ownership-routing fix forward on current main (our live-loop regression coverage already adopted there).

@abhibansal-sg
abhibansal-sg deleted the fix-tui-notification-session-ownership branch July 15, 2026 14:49
teknium1 pushed a commit that referenced this pull request Jul 16, 2026
Adapt the strongest #63317 live-loop handoff regression and cover lineage lookup failure plus addressed live-loop orphans.

Co-authored-by: Abhinav Bansal <abhibansal-sg@users.noreply.github.com>
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
Adapt the strongest NousResearch#63317 live-loop handoff regression and cover lineage lookup failure plus addressed live-loop orphans.

Co-authored-by: Abhinav Bansal <abhibansal-sg@users.noreply.github.com>
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
Adapt the strongest NousResearch#63317 live-loop handoff regression and cover lineage lookup failure plus addressed live-loop orphans.

Co-authored-by: Abhinav Bansal <abhibansal-sg@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets 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.

TUI: background process completion notifications can route to the wrong live session

2 participants