fix(tui): route bg process notifications to owning session, drop orphaned events

[yingliang-zhang]

What does this PR do?

Fixes cross-session background-process notification leakage in the TUI/Desktop multi-session path — two complementary paths:

1. Poller orphan guard

After _notification_event_belongs_elsewhere returns False, if the event has a non-empty session_key that differs from the current session, the owner is gone. Previously these orphans were consumed by whichever poller dequeued them — injecting an unrelated [IMPORTANT: Background process ...] notification into the wrong session. Now they are dropped.

2. Post-turn drain ownership filter

The existing process_registry.drain_notifications() pops every event from the global queue regardless of ownership — a turn finishing in session B could consume an event started by session A. Added _drain_owned_notifications() which applies the same ownership routing as the poller (consume own, requeue foreign-live, drop orphan), and wired it into the post-turn safety drain.

Related Issues

Fixes #42674
Fixes #42731 (this PR subsumes #42731 by implementing the same _drain_owned_notifications post-turn drain filter, PLUS the poller orphan path fix)
Related to #35652

Type of Change

• 🐛 Bug fix
• ✅ Tests (regression coverage for both code paths)

Changes Made

• tui_gateway/server.py
  • Added orphan guard in _notification_poller_loop main loop + shutdown drain (drop events whose owner session is no longer live)
  • Added _drain_owned_notifications() — ownership-filtered drain that replaces the raw drain_notifications() call in the post-turn path
  • Wired into the post-turn safety drain in _run_prompt_submit
• tests/test_tui_gateway_server.py
  • test_notification_poller_drops_orphaned_events — orphaned completion events are dropped, not hijacked
  • test_notification_poller_delivers_owned_events — regression guard: events owned by this session are still delivered
  • test_drain_owned_notifications_routes_by_session_key — drain correctly routes owned/foreign/orphan/global events
  • Updated _notification_event_belongs_elsewhere test comment to reflect new orphan handling

How to Test

1. Open two TUI/Desktop sessions (A and B).
2. In session A, start a long background process with notify_on_complete=True.
3. Close session A (or /new away), then interact with session B.
4. Previously: the completion notification would surface in session B, corrupting its transcript.
5. Now: the notification is dropped silently when session A is gone.
6. If both sessions stay open, the notification still routes to session A (unchanged behavior).

[yingliang-zhang]

Fixes #42674
Related to #35652

Complementary to #42731 which addresses the post-turn drain path; this PR additionally fixes the poller orphan fallback path.

[tonydwb]

Code Review Summary

Verdict: LGTM

Well-scoped fix for cross-session background-process notification leakage. The ownership filter is correctly implemented in both the poller loop and the post-turn drain. The test coverage is thorough, covering orphaned events, owned events, and mixed-event scenarios.

Looks Good

• Clean implementation of ownership-aware drain
• Comprehensive test coverage (3 new tests)
• Properly drops orphaned events instead of hijacking them
• Re-queues events for live foreign sessions
• Follows existing patterns in the codebase
• Clear description of the root cause and fix

---

Reviewed by Hermes Agent

[2751738943]

Independent validation: partial fix; post-turn ordinary-completion leak remains

I independently reproduced the cross-session background-process notification leak in Hermes Desktop and validated this PR on macOS. The PR fixes the poller orphan-adoption path, but it does not currently fix the separate post-turn drain path described in the PR body.

Environment

• Hermes Agent: v0.18.2 (2026.7.7.2)
• Upstream main tested: b9b463f3
• PR head tested: 1c3e42dd
• Platform: macOS 26.5.1 (25F80), Electron Desktop
• Profile: one Desktop backend with multiple live chat sessions
• All local usernames, paths, commands, VM names, process IDs, and session keys are redacted below.

Real-world symptom (sanitized)

A background task owned by Session A completed while Session B was active. Session B received:

[IMPORTANT: Background process proc_[REDACTED_A] completed normally (exit code 0).
Command: [REDACTED_COMMAND]
Output:
[REDACTED_SUCCESS_OUTPUT]
]

A second, unrelated Session-A process later failed and Session B also received:

[IMPORTANT: Background process proc_[REDACTED_B] exited (exit code 1).
Command: [REDACTED_COMMAND]
Output:
[REDACTED_ERROR_OUTPUT]
]

The persisted process metadata identified the owner as Session A, while the notifications were injected into Session B. This confirms that both successful and failed ordinary process completions can cross session boundaries.

Safe minimal reproduction

1. Open Desktop Session A.
2. Start a bounded background process with notify_on_complete=true:

terminal(
    command="python -c \"import time; time.sleep(3); print('SESSION_A_DONE')\"",
    background=True,
    notify_on_complete=True,
)

3. Switch to Desktop Session B and submit an unrelated prompt before the process finishes.
4. Observe that Session B can receive Session A's [IMPORTANT: Background process ...] completion.
5. Repeat with a non-zero exit to cover the failure notification path:

terminal(
    command="python -c \"import time; time.sleep(3); raise SystemExit(7)\"",
    background=True,
    notify_on_complete=True,
)

Root cause

There are two independent consumers of the process-wide notification queue:

1. The autonomous notification poller in tui_gateway/server.py.
2. The post-turn safety drain in _run_prompt_submit().

This PR adds _drain_owned_notifications() and fixes orphan adoption in the poller. However, _run_prompt_submit() still calls:

process_registry.drain_notifications(
    session_key=session.get("session_key", ""),
    owns_event=lambda e: _session_owns_notification_event(sid, session, e),
)

In tools/process_registry.py, drain_notifications() currently applies owns_event / session_key filtering only when:

if evt.get("type") == "async_delegation":

An ordinary event with type == "completion" therefore bypasses ownership filtering and is returned to whichever session performs the post-turn drain first.

The new helper is not wired into _run_prompt_submit(). A source-level check on the PR head reports:

RUN_PROMPT_USES_NEW_HELPER=False
RUN_PROMPT_USES_LEGACY_DRAIN=True

Additional compression-lineage issue

The existing _session_owns_notification_event() contract is compression-aware: it resolves a pre-compression parent key to the live continuation through resolve_resume_session_id().

The new _drain_owned_notifications() helper uses direct string equality and a set of current raw session keys instead. In an ad-hoc validation:

COMPRESSION_POSITIVE_PROOF_OWNER=True
COMPRESSION_HELPER_DELIVERED=0

That means simply replacing the old call with the new helper would drop a completion that is positively owned by the current continuation after compression.

Validation evidence

RED on current main with PR tests only

The PR's three new tests were applied to current origin/main without production changes:

2 failed, 1 passed, 316 deselected

Expected failures:

• orphan completion was emitted into the non-owner session;
• _drain_owned_notifications did not exist;
• owned completion behavior remained passing.

GREEN for PR-provided tests

On PR head:

3 passed, 0 failed

Related canonical regression suite

On PR head:

421 passed, 0 failed

After applying the PR commit cleanly on current origin/main:

422 passed, 0 failed

Command:

scripts/run_tests.sh tests/test_tui_gateway_server.py tests/tools/test_process_registry.py -q

Additional checks:

py_compile: passed
git diff --check: passed
ruff check: passed

Focused ad-hoc validation on both PR head and current-main + PR

POST_TURN_FOREIGN_DRAINED=['proc_foreign']
POST_TURN_QUEUE_REMAINING=0
RUN_PROMPT_USES_NEW_HELPER=False
RUN_PROMPT_USES_LEGACY_DRAIN=True
COMPRESSION_POSITIVE_PROOF_OWNER=True
COMPRESSION_HELPER_DELIVERED=0
PR_VALIDATION_BLOCKER=foreign ordinary completion is still drained by session-b
PR_VALIDATION_BLOCKER=new owner-aware helper is not wired into _run_prompt_submit
PR_VALIDATION_BLOCKER=new helper drops a completion owned through compression lineage

This is ad-hoc verification of the missing path, not a claim that the complete Hermes suite failed.

Recommended fix

The smallest safe fix is to generalize the existing ProcessRegistry.drain_notifications() ownership gate to all addressed notification events, rather than only async_delegation, whenever an owns_event callback is supplied.

Conceptually:

evt_key = str(evt.get("session_key") or "")
origin_sid = str(evt.get("origin_ui_session_id") or "")
has_owner = bool(evt_key or origin_sid)

if owns_event is not None and has_owner and not owns_event(evt):
    requeue.append(evt)
    continue

if owns_event is None and session_key and evt_key and evt_key != session_key:
    requeue.append(evt)
    continue

Why this is preferable to direct key comparison in a new TUI-only helper:

• preserves _session_owns_notification_event() positive-proof semantics;
• preserves origin_ui_session_id preference;
• preserves compression-parent → live-continuation routing;
• fixes the existing _run_prompt_submit() call without duplicating ownership logic;
• leaves CLI callers that do not provide session ownership metadata unchanged;
• lets the poller retain this PR's explicit orphan-drop behavior.

The post-turn path should fail closed: if the current session cannot positively prove ownership, it must requeue or persist the event, never inject it into the current conversation.

Regression tests still needed

Please add behavior tests through the real consumer path, not only direct helper tests:

1. Session A completion + Session B post-turn drain → B receives 0; event remains for A.
2. Session A post-turn drain → A receives exactly 1.
3. Repeat for non-zero exit completion.
4. Pre-compression owner key resolves to live continuation → continuation receives exactly 1.
5. origin_ui_session_id owner with stale durable key → origin session receives exactly 1.
6. Closed/orphan owner → no unrelated session receives the event.
7. Legacy ownerless ordinary completion → explicit policy test (fail closed or status-only; never silently treat it as an arbitrary chat-owned event).
8. Ensure a foreign event is requeued once rather than causing a busy loop.

Review verdict

Changes requested / not complete yet. The poller orphan guard is useful and the existing tests pass, but the user-visible post-turn leak remains reproducible on the PR head and after applying the PR to current main.

[2751738943]

Supplemental commit rebased onto the current PR head

Following up on my earlier independent validation: the PR head advanced to de001477 while this supplemental change was under review. I rebased the supplement so its parent is now exactly the current PR head.

Commit

• Commit: 2751738943@663080c
• Branch: https://github.com/2751738943/hermes-agent/tree/fix/pr54785-post-turn-ownership
• Cherry-pick:

git cherry-pick 663080c2adeb7db18e4245bed660dd875407763c

What the supplement adds

• Reuses the existing compression-aware _session_owns_notification_event() authority for every addressed notification carrying session_key or origin_ui_session_id.
• Prevents ordinary success and failure completions from entering another session through the real _run_prompt_submit() post-turn path.
• Preserves compression parent-to-continuation ownership and origin_ui_session_id priority.
• Requeues foreign addressed events before applying process-global consumed/poll-observed suppression, so a non-owner cannot discard the owner's queued notification.
• Preserves TUI delivery for completions observed by read-only poll(), while retaining the existing CLI suppression behavior.
• Requeues every unstarted event in a drained batch when the first notification starts an asynchronous turn; later events are no longer lost.
• Applies the same positive-proof semantics in the live and shutdown poller paths, while preserving legacy delivery for truly ownerless ordinary notifications and fail-closed behavior for ownerless async-delegation payloads.
• Replaces the raw-key-only helper with the shared ownership authority and adds real consumer-path regression coverage.

Verification

• Current PR head de001477 + supplement: 438 passed, 0 failed across tests/test_tui_gateway_server.py and tests/tools/test_process_registry.py.
• Latest main (7acaff5e) + both current PR commits + supplement: 439 passed, 0 failed across the same canonical suites.
• py_compile: passed.
• Ruff: passed.
• git diff --check: passed.
• Independent spec, correctness, security, queue-ordering, and follow-up merge-gate reviews: approved after all blocking findings were resolved with RED/GREEN tests.

No competing pull request was opened.

[yingliang-zhang]

Supplemental commit incorporated

Cherry-picked @2751738943's supplemental commit (663080c) onto this PR branch.

What changed

The supplemental commit generalizes ownership routing to all addressed notification events in process_registry.drain_notifications(), rather than only async_delegation events. This closes the post-turn drain leak identified in the independent validation:

• Removes the TUI-only _drain_owned_notifications() helper (exact-key comparison)
• Reuses the existing compression-aware _session_owns_notification_event() authority for every addressed notification
• Requeues foreign addressed events before applying process-global consumed/poll-observed suppression
• Preserves legacy delivery for truly ownerless ordinary notifications
• Adds real consumer-path regression tests (post-turn drain, poll-observed, compression-lineage, orphan, non-zero exit)

Verification

438 passed, 0 failed across tests/test_tui_gateway_server.py (325) and tests/tools/test_process_registry.py (113).

Credits: @2751738943 for the independent validation, root-cause analysis, and supplemental commit.

[teknium1]

Thanks for the focused ownership-routing fix. The reported defect remains on current main: tui_gateway/server.py:9372-9375 supplies an ownership callback to the post-turn drain, while tools/process_registry.py:1183-1205 applies ownership filtering only for async_delegation, allowing ordinary completion events to be consumed by the wrong TUI session. The poller also permits an ownerless ordinary completion past its live-owner check at tui_gateway/server.py:8713-8729.

The final PR commit 35803dd50e0e71e4cfd9504d14f0a1f4fd6d6405 moves routing ahead of local suppression for all addressed events, reuses the existing compression-aware _session_owns_notification_event() authority (tui_gateway/server.py:8614-8645), and adds consumer-path coverage for foreign, compression-lineage, origin-session, poll-observed, orphan, and batched notifications. No additional correctness or design-fit issue was verified.

Automated hermes-sweeper review.

[alt-glitch]

This was generated by AI during triage.

Related cluster (same cross-session notification ownership-routing family, all open): this PR (poller orphan-guard + post-turn owned-drain), #63317 (extends the ownership filter to all notification types), #42731 (post-turn only). #35667 (poller) and #57586 (compression-chain) are closed. Adding needs-decision: @2751738943's independent validation in this thread reports the fix is partial — the post-turn ordinary-completion leak reportedly remains. A maintainer should pick the canonical mechanism and confirm full coverage before merge.

[2751738943]

Clarification and differential validation of the overlapping implementations

@alt-glitch, for clarity, the partial-fix finding in my earlier validation applied to the original #54785 head 1c3e42dd, before the supplemental commit was incorporated. The current #54785 head is 35803dd50e0e71e4cfd9504d14f0a1f4fd6d6405 and includes that supplemental fix.

I compared the three open implementations against the same ownership-routing contract.

Recommendation

Among these overlapping PRs, #54785 at 35803dd5 is the strongest consolidated reference implementation and the best integration baseline, but the maintainers should make the canonical decision. I recommend carrying its mechanism and stronger regression coverage forward during a current-main rebase. The branch to carry forward is yingliang-zhang:fix/tui-bg-notification-session-routing (PR #54785). It is not merge-ready as-is because #54785, #63317, and #42731 currently conflict with main.

The original supplemental branch 2751738943:fix/pr54785-post-turn-ownership is not a separate competing implementation: its commit was already cherry-picked into #54785 as 35803dd5, with author attribution preserved.

Why

The required invariant is that an addressed notification is routed to its proven owner before any current-session-local suppression is applied. A non-owner must receive zero deliveries and must not consume or discard the owner's event.

#54785 covers the core contract more completely by:

• routing addressed events before _poll_observed / _completion_consumed suppression;
• reusing the compression-aware _session_owns_notification_event() authority;
• preserving origin_ui_session_id routing and compression lineage;
• allowing the TUI to deliver a completion that was only observed by read-only poll();
• requeuing all unstarted events when a real asynchronous notification turn makes the session busy;
• preserving legacy ownerless ordinary-completion behavior while keeping ownerless async-delegation payloads fail-closed;
• covering the real _run_prompt_submit() consumer path.

#63317 is directionally correct and its own suites pass, but it is incomplete as-is. Running the stronger #54785 regression contract against the #63317 production code produced 6 passed, 6 failed. The failures show that:

1. _poll_observed and _completion_consumed suppression still runs before ownership, so a non-owner can discard a foreign addressed event before the ownership callback is consulted.
2. There is no skip_poll_observed=False gateway/TUI path, so a completion observed by read-only poll() is not delivered through the real post-turn consumer.
3. In the real-thread three-event batch case, only the second unstarted event is requeued; the third is lost.
4. Ownerless ordinary-completion behavior changes from legacy delivery to fail-closed requeue. That may be a possible future policy, but it is a compatibility change and should not happen implicitly while fixing addressed-event routing.

#42731 should not be selected as-is: its helper filters only after the process-wide drain, uses an outdated ownership API shape, and its test does not exercise the real post-turn consumer path. The automated review on that PR reached the same conclusion.

#35667 and #57586 are already closed as superseded. #60863 is merged and provides the origin-session/compression-chain ownership foundation used by the current code; it is not a competing branch for this remaining ordinary-completion fix.

Verification

Exact PR heads (these totals confirm internal suite health, not equivalent coverage, because the branches add different tests):

• #54785 @ 35803dd5: 438 passed, 0 failed across tests/test_tui_gateway_server.py and tests/tools/test_process_registry.py.
• #63317 @ 5c39a4d2: 424 passed, 0 failed across the same files.
• Stronger fix(tui): route bg process notifications to owning session, drop orphaned events #54785 regression tests against fix(tui): enforce notification session ownership across all drain paths #63317 production code: 6 passed, 6 failed.
• Shared differential ownership contract: fix(tui): route bg process notifications to owning session, drop orphaned events #54785 passed; fix(tui): enforce notification session ownership across all drain paths #63317 failed.

Current-main simulation at 569b912d7d:

• Unmodified main: 431 passed, 0 failed across the same three files, while the shared ownership contract still fails as expected because the reported ordinary-completion defect remains.
• Both candidates required resolution of the same tools/process_registry.py conflict.
• The conflict was resolved for each candidate while preserving current main's durable-restored async-delegation safeguard from [Bug]: New CLI session adopts a dead session's async delegation completions (durable restore bypasses CLI drain ownership) #64484. During review, one current-main test function omitted by the initial fix(tui): route bg process notifications to owning session, drop orphaned events #54785 conflict resolution was restored before the final rerun.
• Corrected main + #54785: 452 passed, 0 failed in one canonical run across the two ownership suites plus tests/tools/test_restored_delegation_ownership.py; the restored current-main test was also collected and passed independently.
• main + #63317: its own 436 tests passed across the same three files, but the same stronger cross-check still produced 6 passed, 6 failed in the cases listed above.
• py_compile, Ruff, and git diff --check passed for both overlays.

Remaining merge work

#54785 is the strongest baseline, but its current tests are not complete enough to call the branch merge-ready. Its orphan/owned poller tests currently enter the shutdown-drain path rather than exercising a live-loop dequeue. During the rebase, retain or port #63317's one-loop foreign-dequeue-to-owner handoff test and lineage-DB-failure fail-closed test, and add a live-loop addressed-orphan test. The ownerless ordinary-notification behavior is also an explicit policy choice for maintainers; the compatibility difference should not be treated as the sole correctness reason to reject #63317.

Therefore, my recommendation is: carry #54785's final routing mechanism and stronger regression suite forward as the integration baseline, rebase it onto current main while preserving the #64484 restored-event hardening and current-main tests, and do not merge #63317 or #42731 as-is. The maintainers should make the final canonical/policy decision after the remaining live-poller coverage is incorporated.

[yingliang-zhang]

Rebased onto current main and addressed the remaining ownership-review coverage in 22bdb7ef2720e7d8e21fdce3defe1ff5eceaaa00.

• Resolved the tools/process_registry.py conflict semantically, preserving fix(tui): route bg process notifications to owning session, drop orphaned events #54785 generalized owner routing and current-main [Bug]: New CLI session adopts a dead session's async delegation completions (durable restore bypasses CLI drain ownership) #64484 restored-event safeguards.
• Added live-loop foreign handoff, lineage lookup fail-closed, and addressed-orphan drop coverage.
• Canonical affected matrix: 455 passed, 0 failed; independent new focused regressions: 4 passed, 0 failed.
• Ruff, py_compile, range-diff inspection, and git diff --check passed.

The force-push used an exact lease against the previously reviewed head 35803dd50e0e71e4cfd9504d14f0a1f4fd6d6405.