From 2f45f4d043add975845c47ef68fdf0be44573232 Mon Sep 17 00:00:00 2001 From: Halldrix <12357213+Halldrix@users.noreply.github.com> Date: Thu, 3 Sep 2026 05:55:21 -0500 Subject: [PATCH] fix(tui): drive /heartbeat firing from the session-owner process (#102056) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /heartbeat is parsed by the slash-worker process, which starts its own heartbeat watchdog thread there — but that process never runs turns, so the queued prompt is armed-but-dead (countdown froze at ~0s, no turn ever injected). Mirror the existing /loop driver: poll for due heartbeats in the per-session notification poller and re-enter the live session through _run_prompt_submit as a plain user turn, preserving alternation and prompt caching. (cherry picked from commit 23ae971075f2d8156642f4c268dd010510a7af41) --- tests/tui_gateway/test_heartbeat_tui_tick.py | 106 +++++++++++++++++++ tui_gateway/session_notifications.py | 48 ++++++++- 2 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 tests/tui_gateway/test_heartbeat_tui_tick.py diff --git a/tests/tui_gateway/test_heartbeat_tui_tick.py b/tests/tui_gateway/test_heartbeat_tui_tick.py new file mode 100644 index 0000000000..6b9d02f7cf --- /dev/null +++ b/tests/tui_gateway/test_heartbeat_tui_tick.py @@ -0,0 +1,106 @@ +"""Tests for the TUI/Desktop heartbeat driver (issue #102056). + +The slash worker that parses ``/heartbeat`` starts a watchdog thread in its +own process, where the queued prompt is never consumed. The real driver lives +in ``tui_gateway/server._maybe_fire_tui_heartbeat_tick``, which polls the +session-owner process and re-enters the live session via ``_run_prompt_submit``. +""" + +import time +from types import SimpleNamespace + +import pytest + +import tui_gateway.server as server +from hermes_cli.heartbeat import HeartbeatManager, HeartbeatState + + +def _due_manager(session_id): + """A HeartbeatManager whose state is already due, bypassing SessionDB.""" + mgr = HeartbeatManager.__new__(HeartbeatManager) + mgr.session_id = session_id + # created long ago so is_due() is immediately true. + mgr._state = HeartbeatState( + prompt="report backend health", + interval_seconds=60, + status="active", + created_at=time.time() - 3600, + ) + return mgr + + +def _idle_session(): + return { + "agent": SimpleNamespace(), + "session_key": "hb-tui-key", + "history": [], + "history_lock": server.threading.Lock(), + "running": False, + } + + +def test_tui_heartbeat_fires_when_idle_and_due(monkeypatch): + submitted = [] + + def _submit(_rid, sid, session, text): + submitted.append(text) + + monkeypatch.setattr(server, "_run_prompt_submit", _submit) + monkeypatch.setattr(server, "_emit", lambda *a, **k: None) + monkeypatch.setattr( + "hermes_cli.heartbeat.HeartbeatManager", lambda session_id: _due_manager(session_id) + ) + + session = _idle_session() + server._maybe_fire_tui_heartbeat_tick("sid-hb", session) + + assert len(submitted) == 1 + assert "report backend health" in submitted[0] + assert session["running"] is True + + +def test_tui_heartbeat_skips_when_busy(monkeypatch): + submitted = [] + + def _submit(_rid, sid, session, text): + submitted.append(text) + + monkeypatch.setattr(server, "_run_prompt_submit", _submit) + monkeypatch.setattr(server, "_emit", lambda *a, **k: None) + monkeypatch.setattr( + "hermes_cli.heartbeat.HeartbeatManager", lambda session_id: _due_manager(session_id) + ) + + session = _idle_session() + session["running"] = True # an in-flight turn owns the session + server._maybe_fire_tui_heartbeat_tick("sid-hb", session) + + assert submitted == [] + # The tick stays due; the driver never claims the session. + assert session["running"] is True + + +def test_tui_heartbeat_skips_when_not_due(monkeypatch): + submitted = [] + + def _submit(_rid, sid, session, text): + submitted.append(text) + + monkeypatch.setattr(server, "_run_prompt_submit", _submit) + monkeypatch.setattr(server, "_emit", lambda *a, **k: None) + + mgr = HeartbeatManager.__new__(HeartbeatManager) + mgr.session_id = "hb-tui-key" + mgr._state = HeartbeatState( + prompt="not yet", + interval_seconds=60, + status="active", + created_at=time.time(), # just armed — not due + ) + monkeypatch.setattr("hermes_cli.heartbeat.HeartbeatManager", lambda session_id: mgr) + + session = _idle_session() + server._maybe_fire_tui_heartbeat_tick("sid-hb", session) + + assert submitted == [] + assert session["running"] is False \ No newline at end of file diff --git a/tui_gateway/session_notifications.py b/tui_gateway/session_notifications.py index 3457bfa7ed..1cef2242be 100644 --- a/tui_gateway/session_notifications.py +++ b/tui_gateway/session_notifications.py @@ -113,6 +113,7 @@ def _notification_event_dedup_key(evt: dict) -> tuple: # past them and they can't wedge a later completed/blocked event behind an unclaimed row. _KANBAN_NOTIFY_KINDS = ("completed", "blocked", "gave_up", "crashed", "timed_out", "status", "archived", "unblocked") _KANBAN_POLL_SECONDS = _LOOP_POLL_SECONDS = 5.0 +_HEARTBEAT_POLL_SECONDS = 5.0 def _notif_release_turn(session: dict) -> None: @@ -173,6 +174,42 @@ def _notif_slash_loop_tick(rid: str, sid: str, session: dict, mgr, wakeup: str) _notif_loop_status(sid, decision["message"]) +def _maybe_fire_tui_heartbeat_tick(sid: str, session: dict) -> None: + """Fire a due /heartbeat prompt for an idle TUI/Desktop session (issue #102056). + + The slash-worker process that parses ``/heartbeat`` starts a heartbeat watchdog thread in its + OWN process, where the queued prompt has no turn loop to consume it — armed-but-dead. The + durable state lives in SessionDB, so the real driver belongs in the session-owner process: + this mirror of ``_maybe_fire_tui_loop_tick`` polls from the per-session notification poller + and re-enters the live session through ``_run_prompt_submit`` as a plain user turn (alternation + + prompt caching untouched). + """ + try: + from hermes_cli.heartbeat import HeartbeatManager + except Exception: + return + sid_key = session.get("session_key") or "" + if not sid_key: + return + mgr = HeartbeatManager(session_id=sid_key) + if not mgr.is_active() or not mgr.state.is_due(): + return + if not _notif_claim_turn(session): + return # busy — tick coalesces to the next idle poll + prompt = mgr.due_prompt() + if not prompt: + _notif_release_turn(session) + return + rid = f"__heartbeat__{int(time.time() * 1000)}" + try: + _emit("status.update", sid, {"kind": "heartbeat", "text": "♥ heartbeat firing…"}) + _emit("message.start", sid) + _run_prompt_submit(rid, sid, session, prompt) + except Exception as exc: + _notif_log_failure("heartbeat dispatch failed", exc) + _notif_release_turn(session) + + def _maybe_fire_tui_loop_tick(sid: str, session: dict) -> None: """Fire a due /loop wakeup for an idle TUI/Desktop/dashboard session (per-session poller, coarse cadence). Claims the session (running=True) before dispatching so a racing user prompt wins; the post-turn hook completes the tick.""" @@ -416,9 +453,18 @@ def _notification_poller_loop(stop_event: threading.Event, sid: str, session: di emitted: set = set() # dedup re-queued events so one completion isn't emitted 50 times while busy handle = lambda evt, deferred: _notif_handle_event( # noqa: E731 sid, session, evt, emitted, process_registry, format_process_notification, deferred) - last_kanban_poll = last_loop_poll = 0.0 + last_kanban_poll = last_loop_poll = last_heartbeat_poll = 0.0 while not stop_event.is_set() and not session.get("_finalized"): now = time.monotonic() + # ── /heartbeat driver ───────────────────────────────────────────────── + # The slash worker that parses /heartbeat cannot drive firing (its watchdog queues into a process that + # never runs turns), so the session-owner process polls for due heartbeats itself (#102056). + if now - last_heartbeat_poll >= _HEARTBEAT_POLL_SECONDS: + last_heartbeat_poll = now + try: + _maybe_fire_tui_heartbeat_tick(sid, session) + except Exception as hb_exc: + _notif_log_failure("heartbeat poll failed", hb_exc) # /loop wakeup driver: fire a due tick for THIS session while idle (same claim-under-lock as kanban dispatch). # An active non-parked /goal owns the idle boundary and defers it. if now - last_loop_poll >= _LOOP_POLL_SECONDS: