diff --git a/cli.py b/cli.py index 09bc062565e..45ccdb6d020 100644 --- a/cli.py +++ b/cli.py @@ -10442,14 +10442,8 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin): # Must run BEFORE goal continuation so the notification # reflects actual turn completion (mirrors the TUI path). try: - from tools.notify_utils import ( - is_notify_pending, - clear_notify_flag, - fire_notification, - ) - if is_notify_pending(): - clear_notify_flag() - fire_notification() + from tools.notify_utils import consume_pending_notification + consume_pending_notification() except Exception as e: logging.debug("notify idle-check failed: %s", e) @@ -13131,14 +13125,8 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin): # reflects the actual turn completion, not a # potentially-auto-continued turn. try: - from tools.notify_utils import ( - is_notify_pending, - clear_notify_flag, - fire_notification, - ) - if is_notify_pending(): - clear_notify_flag() - fire_notification() + from tools.notify_utils import consume_pending_notification + consume_pending_notification() except Exception as e: logging.debug("notify idle-check failed: %s", e) diff --git a/tests/tools/test_notify_utils.py b/tests/tools/test_notify_utils.py new file mode 100644 index 00000000000..bb21e4e1770 --- /dev/null +++ b/tests/tools/test_notify_utils.py @@ -0,0 +1,87 @@ +"""Tests for the per-session /notify sentinel + consume helper. + +Covers the behavior the supersede added on top of the original PR: +the sentinel is scoped per session so a /notify in one TUI/dashboard +session never fires on another session's turn completion. +""" + +import pytest + +from tools import notify_utils + + +@pytest.fixture +def home(tmp_path, monkeypatch): + monkeypatch.setattr(notify_utils, "_hermes_home", lambda: tmp_path) + # Never hit the OS during tests. + monkeypatch.setattr(notify_utils, "_show_desktop_notification", lambda *a, **k: None) + return tmp_path + + +def test_set_is_pending_clear_roundtrip(home): + assert notify_utils.is_notify_pending("sess-a") is False + assert notify_utils.set_notify_flag("sess-a") is True + assert notify_utils.is_notify_pending("sess-a") is True + assert notify_utils.clear_notify_flag("sess-a") is True + assert notify_utils.is_notify_pending("sess-a") is False + # Clearing again is a no-op (nothing to remove). + assert notify_utils.clear_notify_flag("sess-a") is False + + +def test_sessions_are_independent(home): + """A /notify set in session A must not register as pending for B.""" + notify_utils.set_notify_flag("sess-a") + assert notify_utils.is_notify_pending("sess-a") is True + assert notify_utils.is_notify_pending("sess-b") is False + + +def test_distinct_keys_get_distinct_sentinels(home): + pa = notify_utils.get_notify_sentinel_path("sess-a") + pb = notify_utils.get_notify_sentinel_path("sess-b") + assert pa != pb + + +def test_empty_key_uses_default_sentinel(home): + # Classic CLI (no session key) gets the unsuffixed default file. + assert notify_utils.get_notify_sentinel_path("").name == ".notify_pending" + assert notify_utils.get_notify_sentinel_path(None).name == ".notify_pending" + + +def test_consume_fires_once_and_clears(home, monkeypatch): + fired = [] + monkeypatch.setattr( + notify_utils, "fire_notification", + lambda **kw: fired.append(kw), + ) + notify_utils.set_notify_flag("sess-a") + + assert notify_utils.consume_pending_notification("sess-a") is True + assert len(fired) == 1 + # Sentinel consumed — a second consume is a no-op. + assert notify_utils.consume_pending_notification("sess-a") is False + assert len(fired) == 1 + + +def test_consume_is_scoped_to_its_session(home, monkeypatch): + fired = [] + monkeypatch.setattr( + notify_utils, "fire_notification", + lambda **kw: fired.append(kw), + ) + notify_utils.set_notify_flag("sess-a") + + # Another session completing must NOT consume A's pending notify. + assert notify_utils.consume_pending_notification("sess-b") is False + assert fired == [] + assert notify_utils.is_notify_pending("sess-a") is True + + +def test_key_resolves_from_session_env(home, monkeypatch): + """When no explicit key is passed, the current session context is used.""" + monkeypatch.setattr( + notify_utils, "_resolve_session_key", + lambda sk: "ctx-key" if sk is None else sk, + ) + notify_utils.set_notify_flag() # resolves to "ctx-key" + assert notify_utils.is_notify_pending("ctx-key") is True + assert notify_utils.is_notify_pending() is True diff --git a/tools/notify_utils.py b/tools/notify_utils.py index 37cfaf8497a..5550b1769bd 100644 --- a/tools/notify_utils.py +++ b/tools/notify_utils.py @@ -8,6 +8,7 @@ Windows (PowerShell), and WSL (bridges to Windows via powershell.exe, preferring notify-send via WSLg when available). """ +import hashlib import logging import platform import shutil @@ -46,17 +47,52 @@ def _is_wsl() -> bool: # --------------------------------------------------------------------------- -# Sentinel file +# Sentinel file (per-session) # --------------------------------------------------------------------------- - -def get_notify_sentinel_path() -> Path: - return _hermes_home() / ".notify_pending" +# +# The pending-notify flag is scoped to a *session*, not the whole process. +# The TUI gateway and dashboard serve many concurrent sessions from one +# process sharing one HERMES_HOME; a single global sentinel would let a +# ``/notify`` set in session A fire on session B's turn completion. Keying +# the sentinel by HERMES_SESSION_KEY keeps each session's pending flag +# independent. Classic single-process CLI has no session key and falls back +# to the unsuffixed default file — same behavior as before. -def set_notify_flag() -> bool: +def _resolve_session_key(session_key: Optional[str]) -> str: + """Resolve the session key for the current context. + + Explicit *session_key* wins (used by the TUI gateway, which serves many + sessions from one process and must name them explicitly). Otherwise read + ``HERMES_SESSION_KEY`` from the session context — a contextvar bound + per-turn in the gateway, or ``os.environ`` in the classic CLI and the + slash worker. Falls back to ``""`` (the default sentinel). + """ + if session_key is not None: + return session_key + try: + from gateway.session_context import get_session_env + return get_session_env("HERMES_SESSION_KEY", "") or "" + except Exception: + return "" + + +def _sentinel_name(session_key: str) -> str: + key = (session_key or "").strip() + if not key: + return ".notify_pending" + digest = hashlib.sha1(key.encode("utf-8", "replace")).hexdigest()[:16] + return f".notify_pending-{digest}" + + +def get_notify_sentinel_path(session_key: Optional[str] = None) -> Path: + return _hermes_home() / _sentinel_name(_resolve_session_key(session_key)) + + +def set_notify_flag(session_key: Optional[str] = None) -> bool: """Write the sentinel file to signal a pending notification.""" try: - p = get_notify_sentinel_path() + p = get_notify_sentinel_path(session_key) p.parent.mkdir(parents=True, exist_ok=True) p.touch() return True @@ -65,10 +101,10 @@ def set_notify_flag() -> bool: return False -def clear_notify_flag() -> bool: +def clear_notify_flag(session_key: Optional[str] = None) -> bool: """Remove the sentinel file (cancel or consume notification).""" try: - p = get_notify_sentinel_path() + p = get_notify_sentinel_path(session_key) if not p.exists(): return False p.unlink() @@ -78,9 +114,9 @@ def clear_notify_flag() -> bool: return False -def is_notify_pending() -> bool: - """Check if a notification is pending.""" - return get_notify_sentinel_path().exists() +def is_notify_pending(session_key: Optional[str] = None) -> bool: + """Check if a notification is pending for this session.""" + return get_notify_sentinel_path(session_key).exists() # --------------------------------------------------------------------------- @@ -205,7 +241,6 @@ def _show_desktop_notification(title: str, message: str) -> None: # --------------------------------------------------------------------------- def fire_notification( - config: Optional[dict] = None, *, title: str = "Hermes Agent", message: str = "Task complete", @@ -216,7 +251,6 @@ def fire_notification( crash the idle loop. Args: - config: Optional config dict. Reads from config.yaml when None. title: Desktop notification title. message: Desktop notification body. """ @@ -229,4 +263,27 @@ def fire_approval_request_notification() -> None: This intentionally does not clear the /notify sentinel; the final turn-complete notification should still fire after the user responds. """ - fire_notification(message="Input needed: approval required") \ No newline at end of file + fire_notification(message="Input needed: approval required") + + +def consume_pending_notification( + session_key: Optional[str] = None, + *, + title: str = "Hermes Agent", + message: str = "Task complete", +) -> bool: + """Fire-and-clear the pending notification for *session_key*, if any. + + Single entry point for the turn-complete sites (CLI idle loop, CLI + process loop, TUI gateway success/error paths) so the + check→clear→fire sequence lives in one place. Returns True when a + notification was fired. Fully fail-safe. + """ + try: + if is_notify_pending(session_key): + clear_notify_flag(session_key) + fire_notification(title=title, message=message) + return True + except Exception as e: + logger.debug("notify consume failed: %s", e) + return False diff --git a/tui_gateway/server.py b/tui_gateway/server.py index cd1e0d76bdf..fd0d1b44706 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -6330,19 +6330,13 @@ def _run_prompt_submit(rid, sid: str, session: dict, text: Any) -> None: file=sys.stderr, ) - # Notify check — fire if /notify was set for this turn. - # (The sentinel was set by the SlashWorker that handled the - # /notify command; the notification fires here after the - # agent's turn completes.) + # Notify check — fire if /notify was set for THIS session's turn. + # The sentinel is per-session (keyed by session_key), so a + # /notify in one TUI session never fires on another session's + # completion. Set by the SlashWorker that handled /notify. try: - from tools.notify_utils import ( - is_notify_pending, - clear_notify_flag, - fire_notification, - ) - if is_notify_pending(): - clear_notify_flag() - fire_notification() + from tools.notify_utils import consume_pending_notification + consume_pending_notification(session.get("session_key")) except Exception as e: logging.debug("tui notify idle-check failed: %s", e) @@ -6426,18 +6420,11 @@ def _run_prompt_submit(rid, sid: str, session: dict, text: Any) -> None: f"[gateway-turn] {type(e).__name__}: {e}", file=sys.stderr, flush=True ) _emit("error", sid, {"message": str(e)}) - # If /notify was set for this failed turn, consume it here too. - # Otherwise the global sentinel can leak into a later unrelated - # turn after the TUI error path exits. + # If /notify was set for this failed turn, consume it here too so + # the (per-session) sentinel doesn't survive into a later turn. try: - from tools.notify_utils import ( - is_notify_pending, - clear_notify_flag, - fire_notification, - ) - if is_notify_pending(): - clear_notify_flag() - fire_notification() + from tools.notify_utils import consume_pending_notification + consume_pending_notification(session.get("session_key")) except Exception as notify_exc: logging.debug("tui notify error-path check failed: %s", notify_exc) finally: