Skip to content

fix(tui_gateway): event-driven delivery for notify_on_complete + watch_patterns - #21597

Closed
liftaris wants to merge 3 commits into
NousResearch:mainfrom
liftaris:fix/tui-notify-on-complete-event-driven
Closed

fix(tui_gateway): event-driven delivery for notify_on_complete + watch_patterns#21597
liftaris wants to merge 3 commits into
NousResearch:mainfrom
liftaris:fix/tui-notify-on-complete-event-driven

Conversation

@liftaris

@liftaris liftaris commented May 8, 2026

Copy link
Copy Markdown

What does this PR do?

Makes terminal(background=true, notify_on_complete=true) and watch_patterns deliver their notifications to TUI sessions. Currently they work everywhere except the TUI: events produced by tools.process_registry.completion_queue are drained by cli.py and gateway/run.py between agent turns, but tui_gateway/server.py has no consumer — events pile up in the queue and the agent never takes a follow-up turn on them.

The fix adds a router thread that blocks on completion_queue.get() and hands each event to a short-lived worker from a bounded ThreadPoolExecutor. Workers wait on a new per-session threading.Event (idle) before synthesizing a prompt.submit that re-enters the normal turn dispatch. idle is seeded at session construction and mirrored onto the existing running flag at every transition via a single _set_session_running() helper. If a goal-continuation turn or a real user prompt wins the race to the slot (4009 session busy), the dispatcher re-waits on the next idle edge and retries, bounded by the same overall 300s deadline. Entirely event-driven — every wait is on a threading.Event or a blocking queue.get(), no polling.

Message text matches gateway/run.py::_format_gateway_process_notification and cli.py::_format_process_notification, so the agent sees the same prompt regardless of transport.

Builds on and credits #15329 by @MestreY0d4-Uninter, whose PR established the right high-level shape but was closed for lack of maintainer signal with an explicit invitation to rebuild. This rebuild swaps the 100ms session["running"] polling for edge-triggered threading.Event, swaps the single serial worker for a bounded per-event pool (so a busy session can no longer head-of-line-block an idle sibling), and adds a 300s idle-timeout circuit breaker.

Related Issue

Fixes #15248

(Does not attempt #10760 — that's the API server / WebUI variant with a different root cause. Happy to file a follow-up once this lands.)

Type of Change

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

Changes Made

  • tui_gateway/server.py (+293 / -6):
    • Module-top: _notify_pool ThreadPoolExecutor, _notify_router_thread singleton, _NOTIFY_IDLE_TIMEOUT_SECONDS (all three env-configurable)
    • New helpers: _format_process_notification, _session_for_process_event, _dispatch_process_notification, _process_notification_router, _ensure_notification_router, _set_session_running
    • session["idle"] seeded in session.create and _init_session
    • 6 running transition sites now route through _set_session_running(session, ...)
    • Router started lazily on first session init
  • tests/test_tui_gateway_server.py (+536): 11 new tests

How to Test

Manual

hermes --tui
# in the TUI composer:
> run `sleep 3 && echo done` in the background with notify_on_complete=true

Before: process completes, nothing happens, user has to manually ask.
After: ~3s later a [SYSTEM: Background process proc_... completed (exit code 0). ...] prompt appears and the agent takes a new turn on it.

Automated

./scripts/run_tests.sh tests/test_tui_gateway_server.py tests/tools/test_notify_on_complete.py tests/tools/test_watch_patterns.py
# 235 passed in 1.53s

New tests cover: event-edge contract (dispatcher provably blocked on idle.wait() until the edge fires), 4009-busy retry, session-close-during-wait, concurrent-sessions-no-HoL, orphaned events, already-consumed short-circuit, idle timeout, formatter parity, synthetic request ID, the _set_session_running invariant, and an integration test that puts a real event onto completion_queue and observes the synthesized prompt.

Platforms tested

  • Linux (Manjaro, Python 3.11)

Checklist

Code

Documentation & Housekeeping

  • I've updated relevant documentation — N/A (no user-facing config or schema changes; behavior matches existing CLI/gateway paths)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A (new env vars are opt-in tuning knobs, sensible defaults)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact — threading primitives and queue.Queue are portable; no Unix-only syscalls introduced
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A (no tool surface changes; only completes an existing contract)

Screenshots / Logs

Can attach a before/after ~/.hermes/logs/agent.log snippet on request.

…h_patterns

Background-process completion and watch-pattern events produced by
`tools.process_registry.completion_queue` had no consumer in `tui_gateway/`.
The CLI (`cli.py`) and messaging gateway (`gateway/run.py`) each drain the
queue between agent turns, but the TUI gateway never did — events landed
silently and the agent never received the synthesized notification turn.

Observed symptom: `terminal(background=true, notify_on_complete=true)` from
a `hermes --tui` session never triggers a follow-up turn when the process
exits, while the same tool call works correctly in all messaging
transports and in the plain CLI.  Issue NousResearch#15248 tracks the user-facing bug.

Fix: a single router thread blocks on `completion_queue.get()` (no polling)
and hands each event to a short-lived worker from `_notify_pool`
(ThreadPoolExecutor of 16).  Workers wait on a new per-session
`threading.Event` called `idle` before synthesizing a `prompt.submit` that
re-enters the normal turn dispatch path.  `idle` is seeded at session
construction and mirrored onto the existing `running` flag at every
transition via a single `_set_session_running()` helper; the 6 transition
sites in `_reset_session_agent`, `prompt.submit`, and the turn `finally` /
goal-continuation blocks were updated to route through it.

Design notes:
  - Entirely event-driven.  Router sleeps on the queue's internal condition
    variable; dispatchers sleep on `threading.Event.wait()`.  No
    `time.sleep` anywhere in the notification path.
  - No head-of-line blocking between sessions: per-event dispatcher threads
    mean a busy session A cannot delay a notification destined for idle
    session B (covered by `test_concurrent_sessions_dispatch_in_parallel`).
  - `_NOTIFY_IDLE_TIMEOUT_SECONDS` (default 300, env-configurable) is a
    last-resort circuit breaker; a turn that exceeds it logs a warning and
    drops the notification rather than hanging the dispatcher forever.
  - The post-wait `is_completion_consumed` recheck and session-existence
    recheck handle the race where `process(action='wait')` consumes the
    completion while the dispatcher is blocked on idle.
  - Idle-vs-running are always updated together under `history_lock`, same
    invariant `running` already carried.

The message text and event shape match `gateway/run.py`'s
`_format_gateway_process_notification` and `cli.py`'s
`_format_process_notification` so the agent sees the same prompt regardless
of transport.

Builds on (and credits) the prior effort in NousResearch#15329 by
@MestreY0d4-Uninter, which established the right high-level shape but
relied on 100ms polling of `session['running']` inside a single serial
worker.  The closing comment on that PR invited a fresh, smaller rebuild
against the current TUI gateway; this is that rebuild with the polling
replaced by event edges and the single worker replaced by a bounded pool.

Tests (`tests/test_tui_gateway_server.py`, +10):
  - notification_fires_after_idle_set_without_polling  (event-edge contract;
    monkeypatches `time.sleep` to raise from the dispatcher)
  - notification_dropped_when_session_closed_mid_wait  (teardown race)
  - concurrent_sessions_dispatch_in_parallel           (no HoL blocking)
  - notification_dropped_when_session_key_unresolvable (orphaned process)
  - notification_skipped_when_completion_already_consumed
  - idle_timeout_drops_notification_and_warns          (circuit breaker)
  - formatter_renders_completion_watch_and_disabled_events
  - dispatch_uses_synthetic_request_id                 (process-notify-*)
  - set_session_running_keeps_running_and_idle_in_sync (helper invariant)
  - router_and_dispatch_end_to_end                     (real queue.put()
    round-trip)

Refs: NousResearch#15248, NousResearch#15329
@liftaris
liftaris force-pushed the fix/tui-notify-on-complete-event-driven branch from 8041209 to a5048a4 Compare May 8, 2026 01:17
liftaris added 2 commits May 7, 2026 18:26
- Env-var parsing: wrap HERMES_TUI_NOTIFY_POOL_WORKERS and
  HERMES_TUI_NOTIFY_IDLE_TIMEOUT conversions in try/except so a malformed
  value can't crash the gateway at import (matches _rpc_pool_workers above).
- 4009 race: after idle.wait() returns, a goal-continuation turn or a real
  user prompt can reclaim the slot before the synthetic prompt.submit
  acquires history_lock.  Re-wait on the next idle edge and retry on 4009,
  bounded by the same overall deadline.  Guard the retry against a missing
  idle Event so a legacy session can't busy-spin.
- Drop the process-global server.time.sleep monkeypatch from
  test_notification_fires_after_idle_set_without_polling — it patched the
  real time module for every thread in the process.  The join+is_alive
  check already proves the dispatcher is blocked on idle.wait(), which is
  the property the test exists to enforce.
- Remove unreachable watch_overflow_released formatter branch
  (session_key="" events are dropped in _session_for_process_event before
  the formatter is ever called) and its test assertion.
- F841: drop unused stub local in
  test_notification_dropped_when_session_key_unresolvable.
- Add test_dispatch_retries_after_4009_until_idle covering the retry path.
@liftaris
liftaris force-pushed the fix/tui-notify-on-complete-event-driven branch from a5048a4 to ab5db75 Compare May 8, 2026 01:26
@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/) tool/terminal Terminal execution and process management labels May 8, 2026
@MestreY0d4-Uninter

Copy link
Copy Markdown
Contributor

Thanks for the credit and for picking this up, @liftaris! The switch to threading.Event and the bounded per-event pool address the exact bottlenecks I was wrestling with in #15329. The 300s idle timeout is a nice addition I hadn't considered.

If you need a second pair of eyes on the threading logic or the event-driven flow once the CI stabilizes, let me know. Happy to review.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the careful TUI delivery work. An automated hermes-sweeper review found that the requested behavior is already implemented on current main.

  • d5416284f11ccbc735c8357f0ab35ce5f683ccc3 (fix(tui): autonomous background process completion notifications) added the TUI completion-queue poller and is included in v2026.5.16.
  • tui_gateway/server.py:8658 consumes process_registry.completion_queue; tui_gateway/server.py:8721-8751 formats notifications, emits TUI process status, and starts the follow-up agent turn.
  • tui_gateway/server.py:1443-1445, 4720-4722, and 8863-8873 start that poller for TUI session initialization paths.
  • tools/process_registry.py:2080-2106 supplies shared formatting for completion, watch_match, and watch_disabled events; tests/test_tui_gateway_server.py:7646-7711 covers completion delivery into an agent turn.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 13, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 13, 2026
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:implemented-on-main Sweeper: behavior already present on current main tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TUI: notify_on_complete silently drops — no notification delivered for background terminal processes

4 participants