Skip to content

fix(tui): route background-process notifications to their owning session (#35652) - #65419

Merged
teknium1 merged 4 commits into
mainfrom
salvage/54785-tui-notification-ownership
Jul 16, 2026
Merged

fix(tui): route background-process notifications to their owning session (#35652)#65419
teknium1 merged 4 commits into
mainfrom
salvage/54785-tui-notification-ownership

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

Background-process completion notifications in the TUI/Desktop multi-session path now deliver only to the session that owns them — a poller or post-turn drain in session B can no longer consume or inject a completion belonging to session A, and addressed events whose owner is gone are dropped instead of adopted (#35652).

Root cause: the completion queue is process-global, but only async_delegation events were ownership-filtered (#58684/#55578 scope). Plain completion / watch_match / watch_disabled events were consumed by whichever session's poller woke first, and drain_notifications() popped everything regardless of owner.

Changes

  • tools/process_registry.py: drain_notifications() extends positive-proof / key-equality routing to every addressed notification type (events carrying session_key or origin_ui_session_id); ownerless legacy events keep global delivery; restored-event fail-closed guard preserved; skip_poll_observed flag so gateway/TUI read-only polling doesn't suppress autonomous delivery.
  • tui_gateway/server.py: _notification_event_requires_owner() gate in the live poller loop and shutdown drain — foreign-live events requeue for their owner's poller, addressed orphans drop (delegation orphans still defer for a later lineage resume).
  • Tests: live-loop requeue-for-owner handoff, lineage-lookup-failure fails closed, addressed-orphan drop (both routing shapes), post-turn drain ownership, shutdown-drain semantics.

Validation

Before After
Session B poller dequeues session A's completion injected into B's chat requeued; A's poller delivers it
Addressed event, owner session gone adopted by any session dropped (delegation: deferred for resume)
tests (tui_gateway_server, process_registry, restored-ownership, cli delivery) 459/459 pass

Salvages #54785 by @yingliang-zhang + @2751738943 (authorship preserved; carries forward the direction of @abhibansal-sg's #63317, closed in favor of this). Fixes #35652.

Infographic

tui-notification-ownership

yingliang-zhang and others added 4 commits July 15, 2026 23:23
…aned events

Two complementary fixes for cross-session background-process notification
leakage in the TUI/Desktop multi-session path (#42674, #35652).

1. Poller orphan guard: after _notification_event_belongs_elsewhere
   returns False, check whether the event has a non-empty session_key
   that differs from the current session.  If so the owner session is
   gone — drop the event instead of hijacking it into an unrelated
   session transcript.

2. Post-turn drain filter: the existing drain_notifications() pops every
   event from the global queue regardless of ownership.  Added
   _drain_owned_notifications() which applies the same ownership routing
   used by the poller (consume own, requeue foreign-live, drop orphan),
   and wired it into the post-turn safety drain.

Complementary to PR #42731 which addresses a separate code path in the
same bug class.  Together they close #42674.
Apply positive-proof routing to every addressed notification in the registry and TUI poller while preserving ownerless legacy behavior and TUI delivery for poll-observed completions.

Remove the unused exact-key drain helper and cover ordinary success and failure, origin, compression-lineage, orphan, and poll-observed paths.

Complements #54785.
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>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/tools Tool registry, model_tools, toolsets comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 16, 2026
@teknium1
teknium1 merged commit 094f2b5 into main Jul 16, 2026
33 checks passed
@teknium1
teknium1 deleted the salvage/54785-tui-notification-ownership branch July 16, 2026 06:51
kevinmanase added a commit to kevinmanase/hermes-agent that referenced this pull request Aug 15, 2026
Background process output and completion notifications were routed by
durable session_key alone. Several live UI tabs can share one key, so the
gateway delivered them to whichever tab came first in iteration order --
a user could see output from a command they never ran, and the tab that
actually started it saw nothing.

Track the commissioning UI tab on the process itself:

- ProcessSession gains origin_ui_session_id, set at spawn from
  HERMES_UI_SESSION_ID for every background process (not just
  notify/watch ones -- live streaming needs the same return address).
- Completion and watch_match notifications carry it, and it survives a
  gateway restart through the checkpoint.
- _owner_sid_for_process prefers the recorded origin when that tab is
  still live, and only then falls back to session_key matching. The
  fallback now also skips finalized tabs, which would drop the output.

Processes with no recorded origin (older checkpoints, CLI sessions) keep
the previous key-matching behavior.

The ownership-routing and delegation-context portions of the original
branch landed upstream in NousResearch#65419, so this is the remaining delta rebuilt
on current main.
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/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

4 participants