mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(notify): scope sentinel per-session; dedupe consume; tidy
Builds on PCinkusz's /notify command (previous commit) to fix one design flaw and tighten the implementation: - Per-session sentinel. The pending-notify flag was a single global file (~/.hermes/.notify_pending). The TUI gateway and dashboard serve many sessions from one process sharing one HERMES_HOME, so a /notify set in session A would fire on session B's next turn completion. Key the sentinel by HERMES_SESSION_KEY (resolved from the per-turn contextvar in the gateway, the slash worker's env, or os.environ in the classic CLI). Classic single-session CLI keeps the unsuffixed default file — no behavior change. - Single consume helper. The check->clear->fire block was copy-pasted at four sites (2 in cli.py, 2 in tui_gateway/server.py). Extract consume_pending_notification(session_key) and call it everywhere; the TUI sites pass session["session_key"] explicitly since that process has no per-session contextvar bound at the consume point. - Drop the unused config= param from fire_notification; add the missing trailing newline; reuse approval._get_session_platform() in the approval-notify guard. - tests/tools/test_notify_utils.py: per-session isolation, consume fire-once/scope, default-key, env-resolution. Co-authored-by: PCinkusz <pcinkusz123321@gmail.com>
This commit is contained in:
parent
eb20289f96
commit
c4aeb8a931
4 changed files with 172 additions and 53 deletions
20
cli.py
20
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)
|
||||
|
||||
|
|
|
|||
87
tests/tools/test_notify_utils.py
Normal file
87
tests/tools/test_notify_utils.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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")
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue