diff --git a/hermes_cli/dashboard_procs.py b/hermes_cli/dashboard_procs.py index 8fe8cc4fd6..dd58c7c756 100644 --- a/hermes_cli/dashboard_procs.py +++ b/hermes_cli/dashboard_procs.py @@ -492,12 +492,24 @@ def _is_desktop_local_serve_cmdline(command: str) -> bool: Long-lived headless serves (``--host --port 9119``) must never match — those are operator-managed remote backends that legitimately run with ppid 1. """ - cmd = command.lower() - if "serve" not in cmd or ("hermes" not in cmd and "hermes_cli" not in cmd): + from hermes_cli.update_cmd_windows import _hermes_holder_subcommand + # Canonical token matcher, never argv substrings: ``kanban --preserve-cache`` contains "serve" and + # ``vim notes about hermes serve`` contains both markers — this predicate decides a kill. + if _hermes_holder_subcommand(command) != "serve": return False - has_loopback = any(tok in cmd for tok in ( - "--host 127.0.0.1", "--host=127.0.0.1", "--host localhost", "--host=localhost")) - return has_loopback and ("--port 0" in cmd or "--port=0" in cmd) + tokens = command.lower().split() + host = _flag_value(tokens, "--host") + return host in ("127.0.0.1", "localhost") and _flag_value(tokens, "--port") == "0" + + +def _flag_value(tokens: list[str], flag: str) -> str | None: + """``--flag value`` / ``--flag=value`` from split argv, or None.""" + for i, tok in enumerate(tokens): + if tok == flag and i + 1 < len(tokens): + return tokens[i + 1] + if tok.startswith(flag + "="): + return tok.partition("=")[2] + return None def _process_ppid(pid: int) -> int | None: diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index e0204201d4..02384dba8a 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -532,17 +532,11 @@ def _check_gateway_running(profile_dir: Path) -> bool: the lock isn't held by *this* reader: dashboard as a separate s6 service, launch-service gateways with no live PID file); fallback validates the PID in ``gateway_state.json`` against the process table, matching ``/api/status``.""" - try: - from gateway.status import get_running_pid, get_runtime_status_running_pid, read_runtime_status - if get_running_pid(profile_dir / "gateway.pid", cleanup_stale=False) is not None: - return True - except Exception: - pass - try: - runtime = read_runtime_status(profile_dir / "gateway_state.json") - return get_runtime_status_running_pid(runtime, expected_home=profile_dir) is not None - except Exception: - return False + from gateway.status import get_running_pid, resolve_gateway_liveness + # cleanup_stale=False: a status probe for ANOTHER profile must never unlink its PID file. + return resolve_gateway_liveness( + profile_dir=profile_dir, use_cache=False, + pid_probe=lambda path: get_running_pid(path, cleanup_stale=False)).running def _served_by_running_multiplexer(profile_name: str) -> bool: diff --git a/hermes_cli/update_cmd_windows.py b/hermes_cli/update_cmd_windows.py index 97deeed3c9..eda8a7b7df 100644 --- a/hermes_cli/update_cmd_windows.py +++ b/hermes_cli/update_cmd_windows.py @@ -424,8 +424,9 @@ def _relaunch_stopped_serves(token: dict) -> None: def _is_backend_argv(argv_low: str) -> bool: - """Whether a lower-cased argv is a Desktop backend (``hermes_cli.main`` running ``serve``/``dashboard``).""" - return "hermes_cli.main" in argv_low and (" serve" in argv_low or " dashboard" in argv_low) + """Whether an argv is a Desktop backend: the canonical holder classifier says ``serve``/``dashboard`` + (a substring test matched ``kanban --preserve-cache`` and ``-m dashboard serve`` wrong, #91869).""" + return _hermes_holder_subcommand(argv_low) in ("serve", "dashboard") def _live_argv_low(psutil, pid, cmdline: str) -> str | None: diff --git a/hermes_state_ids.py b/hermes_state_ids.py new file mode 100644 index 0000000000..4f38a4f2c9 --- /dev/null +++ b/hermes_state_ids.py @@ -0,0 +1,28 @@ +"""Session-id minting: the ONE place that knows the ``YYYYMMDD_HHMMSS_`` shape. + +stdlib-only on purpose: ``agent/``, ``cli.py``, ``gateway/`` and ``tui_gateway/`` all mint ids and +must not pull the SessionDB import graph in to do it. ``hermes_cli/session_lost_and_found.py`` +classifies schema-less salvage rows by ``SESSION_ID_PATTERN``, so a shape change here is a +recovery-classification change — keep the prefix stable. +""" + +from __future__ import annotations + +import re +import uuid +from datetime import datetime +from typing import Optional + +SESSION_ID_PATTERN = re.compile(r"^\d{8}_\d{6}_") + +# Interactive surfaces (CLI, TUI, agent, branches, imports) share 6 hex chars — the Desktop's +# session-id candidate regex is pinned to that width. Gateway keys are 8, portability imports 12 +# (many rows minted in the same second). +DEFAULT_HEX_LEN = 6 + + +def new_session_id(now: Optional[datetime] = None, *, hex_len: int = DEFAULT_HEX_LEN) -> str: + """``_`` for a fresh session; ``now`` pins the timestamp to a clock the + caller already captured (``agent.session_start``) so the id and the row agree to the second.""" + stamp = (now or datetime.now()).strftime("%Y%m%d_%H%M%S") + return f"{stamp}_{uuid.uuid4().hex[:hex_len]}" diff --git a/tests/gateway/test_qqbot_update_prompt_key.py b/tests/gateway/test_qqbot_update_prompt_key.py new file mode 100644 index 0000000000..812e2f3bac --- /dev/null +++ b/tests/gateway/test_qqbot_update_prompt_key.py @@ -0,0 +1,31 @@ +"""The QQ update-prompt authz key comes from ``build_session_key`` (profile-namespaced), not a +hard-coded ``agent:main:`` literal — a multiplexed secondary bot's clicks were rejected otherwise.""" + +from __future__ import annotations + +from types import SimpleNamespace + +from gateway.config import Platform +from gateway.platforms.qqbot.adapter import QQAdapter +from gateway.platforms.qqbot.keyboards import InteractionEvent +from gateway.session import SessionSource, build_session_key + + +def _adapter(owner_profile=None): + adapter = QQAdapter.__new__(QQAdapter) + adapter.config = SimpleNamespace(extra={}) + adapter.platform = Platform.QQBOT + if owner_profile: + adapter._owner_profile = owner_profile + return adapter + + +def test_update_prompt_key_is_the_canonical_session_key_per_profile(): + event = InteractionEvent(scene="c2c", user_openid="U1") + for profile in (None, "ops"): + adapter = _adapter(profile) + key = adapter._update_prompt_session_key(event, "U1") + source = SessionSource(platform=Platform.QQBOT, chat_id="U1", chat_type="c2c", profile=profile) + assert key == build_session_key(source, profile=profile) + assert adapter._is_authorized_interaction_for_session(event, key) + assert _adapter()._update_prompt_session_key(event, "U1") == "agent:main:qqbot:c2c:U1" diff --git a/tests/hermes_cli/test_orphan_desktop_serve_reap.py b/tests/hermes_cli/test_orphan_desktop_serve_reap.py index 8d1a7e82eb..46d58d9f18 100644 --- a/tests/hermes_cli/test_orphan_desktop_serve_reap.py +++ b/tests/hermes_cli/test_orphan_desktop_serve_reap.py @@ -37,8 +37,9 @@ def test_desktop_local_serve_shape_spares_fixed_port_and_non_serve(): "hermes serve --host 127.0.0.1 --port 9119" ) assert not _is_desktop_local_serve_cmdline("hermes gateway run --replace") + # "serve" inside another token is not the serve subcommand (token matcher, not substring). assert not _is_desktop_local_serve_cmdline( - "vim notes about hermes serve --port 0" + "hermes kanban --preserve-cache --host 127.0.0.1 --port 0" ) diff --git a/tests/hermes_cli/test_process_identity_canonical_matchers.py b/tests/hermes_cli/test_process_identity_canonical_matchers.py new file mode 100644 index 0000000000..93e8a84252 --- /dev/null +++ b/tests/hermes_cli/test_process_identity_canonical_matchers.py @@ -0,0 +1,63 @@ +"""Process-identity contract: the two kill/relaunch predicates and the profile liveness probe +defer to the canonical matchers instead of argv substrings (root AGENTS.md process-identity rule). +""" + +from __future__ import annotations + +import pytest + +from hermes_cli.dashboard_procs import _is_desktop_local_serve_cmdline +from hermes_cli.update_cmd_windows import _hermes_holder_subcommand, _is_backend_argv + +LOOPBACK = "--host 127.0.0.1 --port 0" + +# (cmdline, holder subcommand, desktop-local reap?). Substring scanners get every "trap" row wrong: +# "serve" appears inside --preserve-cache / observer.py / a flag value. +CMDLINES = [ + ("python -m hermes_cli.main serve " + LOOPBACK, "serve", True), + ("/venv/bin/hermes serve --isolated --host=127.0.0.1 --port=0 --ssh-owner-nonce abc", "serve", True), + ("hermes --profile ops serve " + LOOPBACK, "serve", True), + ("hermes -m serve kanban --preserve-cache " + LOOPBACK, "kanban", False), + ("python -m hermes_cli.main kanban --preserve-cache " + LOOPBACK, "kanban", False), + ("hermes --reasoning high dashboard " + LOOPBACK, "dashboard", False), + ("hermes gateway run --replace", "gateway", False), + ("hermes chat --model serve", "chat", False), + ("python observer.py serve " + LOOPBACK, None, False), +] + + +@pytest.mark.parametrize("cmdline,subcommand,reapable", CMDLINES) +def test_kill_and_relaunch_predicates_agree_with_the_canonical_holder_matcher(cmdline, subcommand, reapable): + assert _hermes_holder_subcommand(cmdline) == subcommand + # Desktop-local reap (a KILL path): serve + loopback + ephemeral port, decided by tokens. + assert _is_desktop_local_serve_cmdline(cmdline) is reapable + # Windows updater backend classifier (stop + relaunch path). + assert _is_backend_argv(cmdline.lower()) is (subcommand in ("serve", "dashboard")) + + +def test_desktop_local_serve_spares_fixed_port_and_remote_hosts(): + assert not _is_desktop_local_serve_cmdline("hermes serve --host 100.106.105.2 --port 9119 --skip-build") + assert not _is_desktop_local_serve_cmdline("hermes serve --host 127.0.0.1 --port 9119") + assert _is_desktop_local_serve_cmdline("hermes serve --host localhost --port 0") + + +def test_profile_liveness_is_the_shared_ladder(tmp_path, monkeypatch): + """``_check_gateway_running`` is ``resolve_gateway_liveness`` scoped to the profile dir, with the + PID rung reading (never cleaning) THAT profile's ``gateway.pid``.""" + import gateway.status as gw_status + from hermes_cli.profiles import _check_gateway_running + + seen: dict = {} + + def fake_resolve(**kwargs): + seen.update(kwargs) + return gw_status.GatewayLiveness(running=True, pid=1, source="pid") + + monkeypatch.setattr(gw_status, "resolve_gateway_liveness", fake_resolve) + calls: list = [] + monkeypatch.setattr(gw_status, "get_running_pid", + lambda path, cleanup_stale=True: calls.append((path, cleanup_stale))) + assert _check_gateway_running(tmp_path) is True + assert seen["profile_dir"] == tmp_path + seen["pid_probe"](tmp_path / "gateway.pid") + assert calls == [(tmp_path / "gateway.pid", False)] diff --git a/tests/test_hermes_state_ids.py b/tests/test_hermes_state_ids.py new file mode 100644 index 0000000000..d33e1c3d2e --- /dev/null +++ b/tests/test_hermes_state_ids.py @@ -0,0 +1,48 @@ +"""Session-id minting contract: every surface that creates a session mints through +``hermes_state_ids.new_session_id`` and the id it produces is what lost-and-found salvage classifies +as a session id (the shape is the recovery sentinel for schema-less rows). +""" + +from __future__ import annotations + +import importlib +import re +from datetime import datetime + +import pytest + +import hermes_state_ids +from hermes_state_ids import SESSION_ID_PATTERN, new_session_id + +# (module, callable(mint) -> str) for each minting site; ``mint`` is the patched helper. +SITES = [ + ("hermes_cli.foreign_sessions", None), + ("agent.conversation_compression", None), + ("hermes_cli.cli_commands_mixin", None), + ("hermes_cli.cli_session_mixin", None), + ("agent.agent_init", None), + ("tui_gateway.server", lambda mod: mod._new_session_key()), + ("gateway.session_lifecycle", lambda mod: mod._new_session_id(datetime(2026, 1, 2, 3, 4, 5))), + ("hermes_state_portability", None), + ("cli", None), +] + + +@pytest.mark.parametrize("hex_len,expected_re", [(6, r"^\d{8}_\d{6}_[0-9a-f]{6}$"), (8, r"^\d{8}_\d{6}_[0-9a-f]{8}$"), + (12, r"^\d{8}_\d{6}_[0-9a-f]{12}$")]) +def test_minted_ids_are_what_salvage_classifies_as_session_ids(hex_len, expected_re): + from hermes_cli.session_lost_and_found import _is_session_id + sid = new_session_id(datetime(2026, 1, 2, 3, 4, 5), hex_len=hex_len) + assert re.fullmatch(expected_re, sid) and sid.startswith("20260102_030405_") + assert _is_session_id(sid) + assert SESSION_ID_PATTERN is importlib.import_module("hermes_cli.session_lost_and_found").SESSION_ID_PATTERN + + +@pytest.mark.parametrize("module_name,call", SITES) +def test_every_minting_site_imports_the_one_helper(module_name, call): + mod = importlib.import_module(module_name) + minted = [name for name in ("new_session_id", "mint_session_id") + if getattr(mod, name, None) is hermes_state_ids.new_session_id] + assert minted, f"{module_name} does not mint through hermes_state_ids.new_session_id" + if call is not None: + assert SESSION_ID_PATTERN.match(call(mod))