fix(process-identity): desktop reap, Windows updater and profile liveness use the canonical matchers
Two kill/relaunch predicates decided identity by argv substring, the bug class root AGENTS.md forbids: hermes_cli/dashboard_procs.py::_is_desktop_local_serve_cmdline (`"serve" not in cmd`, on the orphan-reap KILL path) and hermes_cli/update_cmd_windows.py::_is_backend_argv (`" serve" in argv_low`, in the very file that defines _hermes_holder_subcommand). Both now ask the canonical token classifier; host/port are read as flag values, not substrings. hermes_cli/profiles.py::_check_gateway_running open-coded rungs 1/3 of gateway.status.resolve_gateway_liveness and skipped the multiplexer rung; it is now that ladder scoped to the profile dir (pid probe keeps cleanup_stale=False so a probe for another profile never unlinks its PID file). The gateway/status.py ladder itself is untouched. Behavior change: `hermes kanban --preserve-cache --host 127.0.0.1 --port 0` and `-m dashboard serve`-style argv are no longer classified as serve backends (never killed / relaunched as one); a named profile served by the live default multiplexer now reads as running from _check_gateway_running (previously only via the separate _served_by_running_multiplexer OR at some call sites).
This commit is contained in:
@@ -492,12 +492,24 @@ def _is_desktop_local_serve_cmdline(command: str) -> bool:
|
||||
Long-lived headless serves (``--host <tailscale-ip> --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:
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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:
|
||||
|
||||
28
hermes_state_ids.py
Normal file
28
hermes_state_ids.py
Normal file
@@ -0,0 +1,28 @@
|
||||
"""Session-id minting: the ONE place that knows the ``YYYYMMDD_HHMMSS_<hex>`` 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:
|
||||
"""``<timestamp>_<random hex>`` 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]}"
|
||||
31
tests/gateway/test_qqbot_update_prompt_key.py
Normal file
31
tests/gateway/test_qqbot_update_prompt_key.py
Normal file
@@ -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"
|
||||
@@ -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"
|
||||
)
|
||||
|
||||
|
||||
|
||||
63
tests/hermes_cli/test_process_identity_canonical_matchers.py
Normal file
63
tests/hermes_cli/test_process_identity_canonical_matchers.py
Normal file
@@ -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)]
|
||||
48
tests/test_hermes_state_ids.py
Normal file
48
tests/test_hermes_state_ids.py
Normal file
@@ -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))
|
||||
Reference in New Issue
Block a user