fix(tui_gateway): keep a detached session alive while it owns an active /loop or /heartbeat
The per-session notification poller is the only driver of a Desktop/TUI (route-less) /loop or /heartbeat. When the client disconnects, the WS-orphan reaper collects the idle session ~20s later and the TTL/LRU reapers can collect it after that; either stops the poller, so the schedule freezes (status still 'active') until a client reattaches and the overdue tick fires. Treat an active, non-gateway-routed loop or heartbeat like live delegated work: the orphan reap re-arms instead of popping the session, and the TTL and session-cap reapers skip it. Gateway-routed schedules are fired by the gateway's own scanner and do not pin the session. The exemption is bounded by the loop's tick budget and ends on stop/pause. Memory-trim quiescence is unchanged: an idle looping session does not block a trim.
This commit is contained in:
129
tests/tui_gateway/test_detached_loop_keepalive.py
Normal file
129
tests/tui_gateway/test_detached_loop_keepalive.py
Normal file
@@ -0,0 +1,129 @@
|
||||
"""A detached Desktop/TUI session that owns an active /loop must survive the orphan reapers.
|
||||
|
||||
The per-session notification poller is the only driver of a route-less (Desktop/TUI) loop; reaping the
|
||||
detached session stops that poller and freezes the loop until a client reattaches. Gateway-routed loops are
|
||||
fired by the gateway's own scanner, so they must not pin the session.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def hermes_home(tmp_path, monkeypatch):
|
||||
home = tmp_path / ".hermes"
|
||||
home.mkdir()
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
from hermes_cli import goals
|
||||
|
||||
goals._DB_CACHE.clear()
|
||||
yield home
|
||||
goals._DB_CACHE.clear()
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def server(hermes_home):
|
||||
with patch.dict("sys.modules", {"hermes_cli.env_loader": MagicMock(), "hermes_cli.banner": MagicMock()}):
|
||||
mod = importlib.import_module("tui_gateway.server")
|
||||
yield mod
|
||||
mod._sessions.clear()
|
||||
mod._pending_ws_reaps.clear()
|
||||
|
||||
|
||||
class _Timer:
|
||||
"""Captures the reap callback instead of sleeping on it."""
|
||||
|
||||
created: list = []
|
||||
|
||||
def __init__(self, delay, callback):
|
||||
self.delay, self.callback = delay, callback
|
||||
_Timer.created.append(self)
|
||||
|
||||
def start(self):
|
||||
pass
|
||||
|
||||
def cancel(self):
|
||||
pass
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def detached(server, monkeypatch):
|
||||
_Timer.created = []
|
||||
monkeypatch.setattr(server.threading, "Timer", _Timer)
|
||||
monkeypatch.setattr(server, "_WS_ORPHAN_REAP_GRACE_S", 20)
|
||||
monkeypatch.setattr(server, "_session_has_active_delegations", lambda *a: False)
|
||||
popped = []
|
||||
monkeypatch.setattr(server, "_teardown_popped_session", lambda s, **kw: popped.append(s))
|
||||
sid = "sid-detached-loop"
|
||||
session = {
|
||||
"session_key": "desktop-loop-session",
|
||||
"transport": server._detached_ws_transport,
|
||||
"running": False,
|
||||
"history_lock": threading.Lock(),
|
||||
"last_active": 0.0,
|
||||
"created_at": 0.0,
|
||||
}
|
||||
server._sessions[sid] = session
|
||||
return sid, session, popped
|
||||
|
||||
|
||||
def _set_loop(session_key: str, route: dict | None = None):
|
||||
from hermes_cli.loops import LoopManager
|
||||
|
||||
LoopManager(session_key).set("reconcile the spec", interval_seconds=1800, route=route)
|
||||
|
||||
|
||||
def test_orphan_reap_keeps_detached_session_with_active_desktop_loop(server, detached):
|
||||
sid, session, popped = detached
|
||||
_set_loop(session["session_key"])
|
||||
|
||||
server._schedule_ws_orphan_reap(sid)
|
||||
_Timer.created[-1].callback()
|
||||
|
||||
assert server._sessions.get(sid) is session and not popped
|
||||
assert server._pending_ws_reaps[sid] is _Timer.created[-1] # re-armed, re-checked each grace
|
||||
|
||||
|
||||
def test_orphan_reap_still_collects_session_once_loop_stops(server, detached):
|
||||
sid, session, popped = detached
|
||||
_set_loop(session["session_key"])
|
||||
from hermes_cli.loops import LoopManager
|
||||
|
||||
LoopManager(session["session_key"]).clear()
|
||||
|
||||
server._schedule_ws_orphan_reap(sid)
|
||||
_Timer.created[-1].callback()
|
||||
|
||||
assert sid not in server._sessions and popped == [session]
|
||||
|
||||
|
||||
def test_gateway_routed_loop_does_not_pin_detached_session(server, detached):
|
||||
sid, session, popped = detached
|
||||
_set_loop(session["session_key"], route={"platform": "discord", "chat_id": "123"})
|
||||
|
||||
server._schedule_ws_orphan_reap(sid)
|
||||
_Timer.created[-1].callback()
|
||||
|
||||
assert sid not in server._sessions and popped == [session]
|
||||
|
||||
|
||||
def test_ttl_and_cap_reapers_spare_session_with_active_desktop_loop(server, detached, monkeypatch):
|
||||
sid, session, _ = detached
|
||||
monkeypatch.setattr(server, "_SESSION_TTL_S", 1)
|
||||
now = time.time()
|
||||
assert server._session_is_evictable(sid, session, now) # idle, detached, past TTL: reapable today
|
||||
|
||||
_set_loop(session["session_key"])
|
||||
|
||||
assert not server._session_is_evictable(sid, session, now)
|
||||
assert not server._session_is_reapable(sid, session)
|
||||
# An idle looping session must not block a memory trim the way live work does.
|
||||
assert server._session_is_lru_evictable(sid, session)
|
||||
@@ -934,7 +934,9 @@ def _schedule_ws_orphan_reap(
|
||||
current.pop("_client_gone_interrupt_polls", None)
|
||||
_pending_ws_reaps.pop(sid, None)
|
||||
return
|
||||
if _session_has_active_delegations(sid, current):
|
||||
if _session_has_active_delegations(sid, current) or (
|
||||
not current.get("running") and _session_owns_live_wakeup_schedule(current)):
|
||||
# Live background work, or an active /loop or /heartbeat only this process's poller can fire.
|
||||
reschedule_delay = _WS_ORPHAN_REAP_GRACE_S
|
||||
elif not current.get("running"):
|
||||
session = _pop_session_by_id(sid)
|
||||
|
||||
@@ -270,6 +270,28 @@ def _loop_route_is_gateway_chat(state) -> bool:
|
||||
return bool(route.get("platform") and route.get("chat_id"))
|
||||
|
||||
|
||||
def _session_owns_live_wakeup_schedule(session: dict) -> bool:
|
||||
"""True while THIS process's notification poller is the only driver of an active /loop or /heartbeat for
|
||||
``session`` — the reapers must then keep the detached session (and with it the poller) alive, or the schedule
|
||||
freezes until a client reattaches. Gateway-routed schedules are fired by the gateway, so they never pin the
|
||||
session here. Bounded: a loop pauses at its tick budget and a stopped/paused schedule releases the session.
|
||||
Fail-open to False: an unreadable store must not make a session unreapable."""
|
||||
if not (sid_key := session.get("session_key") or ""):
|
||||
return False
|
||||
try:
|
||||
from hermes_cli.heartbeat import HeartbeatManager
|
||||
from hermes_cli.loops import LoopManager
|
||||
with _session_profile_runtime_scope(session, hydrate_secrets=False):
|
||||
loop = LoopManager(session_id=sid_key)
|
||||
if loop.is_active() and not _loop_route_is_gateway_chat(loop.state):
|
||||
return True
|
||||
heartbeat = HeartbeatManager(session_id=sid_key)
|
||||
return heartbeat.is_active() and not _notif_gateway_owns_heartbeat(session, sid_key)
|
||||
except Exception:
|
||||
logger.debug("wakeup-schedule check failed for %s", sid_key, exc_info=True)
|
||||
return False
|
||||
|
||||
|
||||
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."""
|
||||
|
||||
@@ -239,9 +239,16 @@ def _sessions_quiescent(exclude: str | None = None) -> bool:
|
||||
return all(_session_is_lru_evictable(sid, s) for sid, s in others)
|
||||
|
||||
|
||||
def _session_is_reapable(sid: str, session: dict) -> bool:
|
||||
"""Reaper eligibility: the LRU exemptions, and not the sole driver of an active /loop or /heartbeat
|
||||
(closing the session stops the notification poller that fires it, freezing the schedule until a client
|
||||
reattaches). Kept out of ``_sessions_quiescent``: an idle looping session does not block a memory trim."""
|
||||
return _session_is_lru_evictable(sid, session) and not _session_owns_live_wakeup_schedule(session)
|
||||
|
||||
|
||||
def _session_is_evictable(sid: str, session: dict, now: float) -> bool:
|
||||
"""TTL eviction: the LRU exemptions plus idle-for-TTL AND older-than-TTL."""
|
||||
if not _session_is_lru_evictable(sid, session):
|
||||
"""TTL eviction: the reap exemptions plus idle-for-TTL AND older-than-TTL."""
|
||||
if not _session_is_reapable(sid, session):
|
||||
return False
|
||||
last_active = float(session.get("last_active") or 0.0)
|
||||
created_at = float(session.get("created_at") or 0.0)
|
||||
@@ -343,7 +350,7 @@ def _enforce_session_cap() -> None:
|
||||
with _sessions_lock:
|
||||
if len(_sessions) <= cap:
|
||||
return
|
||||
evictable = [(sid, s) for sid, s in _sessions.items() if _session_is_lru_evictable(sid, s)]
|
||||
evictable = [(sid, s) for sid, s in _sessions.items() if _session_is_reapable(sid, s)]
|
||||
# Oldest-touched first; evict only down to the cap (may stop short: live sessions are never eligible).
|
||||
evictable.sort(key=lambda kv: float(kv[1].get("last_active") or 0.0))
|
||||
for sid, _s in evictable:
|
||||
@@ -351,7 +358,7 @@ def _enforce_session_cap() -> None:
|
||||
if len(_sessions) <= cap:
|
||||
break
|
||||
_close_session_by_id(
|
||||
sid, end_reason="lru_evict", predicate=lambda session, vs=sid: _session_is_lru_evictable(vs, session))
|
||||
sid, end_reason="lru_evict", predicate=lambda session, vs=sid: _session_is_reapable(vs, session))
|
||||
|
||||
|
||||
def _reaper_daemon_timer(delay: float, fn, fail_log: str, level: str = "debug") -> None:
|
||||
|
||||
@@ -85,6 +85,8 @@ Examples:
|
||||
|
||||
Works on the CLI, the TUI (`hermes --tui`), the web dashboard chat, the desktop app, and every gateway platform (Telegram, Discord, Slack, WhatsApp, …). On messaging platforms the gateway fires wakeups even between your messages — the loop belongs to the chat's session, and its results arrive as ordinary replies — also when that session is open in the TUI / Desktop app, which leaves the routed wakeup to the gateway.
|
||||
|
||||
In the desktop app and dashboard chat, the backend keeps firing an active loop after you close the window or the app disconnects: a session with an active `/loop` (or `/heartbeat`) is exempt from the disconnected-session reaper, so the wakeups and their results are waiting when you reopen it. The exemption ends when the loop stops, pauses, or reaches its tick budget, and the session is then collected normally. The backend process itself must stay running — a Desktop-spawned local backend exits with the app, so for unattended loops run the session on a long-lived backend (a remote `hermes serve`, or the dashboard).
|
||||
|
||||
## Mixing with `/goal`
|
||||
|
||||
Both features inject synthetic turns at idle boundaries, so they follow one rule: **an active goal owns the session.** While a `/goal` is actively driving (judge saying "continue"), loop wakeups defer. The loop resumes using idle time as soon as the goal finishes, pauses, or parks itself on a wait barrier (`/goal wait`, or the judge's automatic WAIT verdict). A parked goal plus a `/loop` is a natural combo: the goal waits on the big async thing while the loop keeps a heartbeat on something else.
|
||||
|
||||
Reference in New Issue
Block a user