diff --git a/cron/lifecycle_guard.py b/cron/lifecycle_guard.py index d687a4a4b4..9e9e4f41d5 100644 --- a/cron/lifecycle_guard.py +++ b/cron/lifecycle_guard.py @@ -366,18 +366,10 @@ _BINARY_MAGICS = ( # --- profile identity ------------------------------------------------------------------------- def _current_profile_name() -> Optional[str]: - """Profile running the guard: ``HERMES_PROFILE_NAME``/``HERMES_PROFILE`` env first, then - ``hermes_cli.profiles.get_active_profile_name`` (from ``HERMES_HOME``); ``None`` if neither.""" - for env_name in ("HERMES_PROFILE_NAME", "HERMES_PROFILE"): - value = os.environ.get(env_name) - if value and value.strip(): - return value.strip() - try: - from hermes_cli.profiles import get_active_profile_name + """Profile running the guard (``hermes_cli.profiles.current_profile_name``); ``None`` if none.""" + from hermes_cli.profiles import current_profile_name - return get_active_profile_name() or None - except Exception: - return None + return current_profile_name() def _named_profile_is_current(named: str) -> bool: diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index e5bd14a87f..70b136b2f0 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -200,15 +200,8 @@ def kanban_command(args: argparse.Namespace) -> int: def _profile_author() -> str: """Best-effort author name for an interactive CLI call.""" - for env in ("HERMES_PROFILE_NAME", "HERMES_PROFILE"): - v = os.environ.get(env) - if v: - return v - try: - from hermes_cli.profiles import get_active_profile_name - return get_active_profile_name() or "user" - except Exception: - return "user" + from hermes_cli.profiles import current_profile_name + return current_profile_name("user") or "user" _DELEGATED_CHILD_DENIED_ACTIONS: frozenset[str] = frozenset({ diff --git a/hermes_cli/kanban_specify.py b/hermes_cli/kanban_specify.py index b0a7412cb1..eefb920467 100644 --- a/hermes_cli/kanban_specify.py +++ b/hermes_cli/kanban_specify.py @@ -117,9 +117,10 @@ def _title_body(parsed: dict) -> tuple[Optional[str], Optional[str]]: def _profile_author(default: str = "specifier") -> str: - """Mirror of ``hermes_cli.kanban._profile_author``. Kept local to - avoid a circular import when kanban.py imports this module.""" - return os.environ.get("HERMES_PROFILE") or os.environ.get("USER") or default + """Same identity contract as ``hermes_cli.kanban._profile_author``; ``$USER`` as the last + resort for a human running the CLI outside any profile.""" + from hermes_cli.profiles import current_profile_name + return current_profile_name() or os.environ.get("USER") or default def _load_triage_task(task_id: str) -> tuple[Optional[kb.Task], str]: diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index e161d15b5f..f61b56a8e6 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -1919,6 +1919,27 @@ def get_active_profile_name() -> str: return "custom" +def current_profile_name(default: str | None = None) -> str | None: + """Identity of the profile the current task runs FOR: the ``HERMES_HOME`` override when one is + bound (a multiplexed cron tick, a routed gateway turn), else a launcher-pinned + ``HERMES_PROFILE_NAME``/``HERMES_PROFILE`` (the kanban dispatcher pins it on its workers), else + the name derived from the process's ``HERMES_HOME``; *default* when nothing names a profile. + + The env pin is read only outside an override: ``os.environ`` is the LAUNCH profile's, so under + an override it would re-label a served profile's board writes with the host's identity. + """ + from hermes_constants import get_hermes_home_override + if get_hermes_home_override() is None: + for env_name in ("HERMES_PROFILE_NAME", "HERMES_PROFILE"): + value = (os.environ.get(env_name) or "").strip() + if value: + return value + try: + return get_active_profile_name() or default + except Exception: + return default + + # Export / Import def _inside_git_checkout(path: Path) -> bool: diff --git a/tests/tools/test_kanban_persisted_identity.py b/tests/tools/test_kanban_persisted_identity.py index af4a36c3c2..c9626e6399 100644 --- a/tests/tools/test_kanban_persisted_identity.py +++ b/tests/tools/test_kanban_persisted_identity.py @@ -1,16 +1,11 @@ -"""Persisted board identity resolves the ACTIVE profile, never a generic label. +"""Board identity follows the profile a kanban call runs FOR, never the generic ``"worker"``. -Regression: ``kanban_comment``/``kanban_create`` persisted ``"worker"`` whenever -the dispatcher had not pinned ``HERMES_PROFILE``, even though the running Hermes -profile was discoverable from ``HERMES_HOME`` via -``hermes_cli.profiles.get_active_profile_name``. Comment authors are injected -into future workers' prompts and ``created_by`` gates completion audits, so a -generic label silently loses handoff provenance. - -Contract under test (``tools.kanban_tools._persisted_identity``): environment -profile (``HERMES_PROFILE_NAME``/``HERMES_PROFILE``) → active profile API → -generic ``"worker"`` fallback. Caller args can never override the persisted -identity (#19713). +``kanban_comment`` / ``kanban_create`` used to read ``os.environ["HERMES_PROFILE"]`` and fall back to +``"worker"``. A multiplexed per-profile cron tick binds the profile as a ``HERMES_HOME`` override and +never mirrors it into ``os.environ``, so every card comment a served profile's turn wrote was +authored ``"worker"`` (#119859). ``hermes_cli.profiles.current_profile_name`` is the one resolver: +the bound override first, the dispatcher's ``HERMES_PROFILE`` pin only outside an override, the +process home last. Identity never comes from tool args (#19713). """ from __future__ import annotations @@ -18,22 +13,18 @@ import json import pytest +from hermes_constants import reset_hermes_home_override, set_hermes_home_override + @pytest.fixture def board_env(tmp_path, monkeypatch): - """Worker on an isolated default board whose ``HERMES_HOME`` sits under a - NAMED profile dir (``/profiles/qa-bot``) — the shape a profile launch - has when the dispatcher did not export ``HERMES_PROFILE``. The ``profiles`` - parent makes ``get_default_hermes_root()`` resolve to ```` on every - platform, so the board DB stays inside tmp_path.""" - profile_home = tmp_path / "hroot" / "profiles" / "qa-bot" - profile_home.mkdir(parents=True) - monkeypatch.setenv("HERMES_HOME", str(profile_home)) - monkeypatch.delenv("HERMES_PROFILE", raising=False) - monkeypatch.delenv("HERMES_PROFILE_NAME", raising=False) - monkeypatch.delenv("HERMES_SESSION_ID", raising=False) - for var in ("HERMES_KANBAN_DB", "HERMES_KANBAN_HOME", "HERMES_KANBAN_BOARD", - "HERMES_KANBAN_WORKSPACES_ROOT"): + """A root-profile board with two served profiles beside it and no env pin at all.""" + root = tmp_path / "hroot" + for name in ("alpha", "beta"): + (root / "profiles" / name).mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(root)) + for var in ("HERMES_PROFILE", "HERMES_PROFILE_NAME", "HERMES_SESSION_ID", "HERMES_KANBAN_DB", + "HERMES_KANBAN_HOME", "HERMES_KANBAN_BOARD", "HERMES_KANBAN_WORKSPACES_ROOT"): monkeypatch.delenv(var, raising=False) from hermes_cli import kanban_db as kb @@ -42,12 +33,12 @@ def board_env(tmp_path, monkeypatch): kb.init_db() conn = kbc.connect() try: - tid = kb.create_task(conn, title="identity-test", assignee="qa-bot") + tid = kb.create_task(conn, title="identity-test", assignee="alpha") kb.claim_task(conn, tid) finally: conn.close() monkeypatch.setenv("HERMES_KANBAN_TASK", tid) - return tid + return root, tid def _last_comment_author(tid: str) -> str: @@ -70,87 +61,41 @@ def _created_by(tid: str) -> str: conn.close() -def test_env_profile_takes_precedence(board_env, monkeypatch): - """A dispatcher-pinned ``HERMES_PROFILE`` wins over the HERMES_HOME-derived - active profile for both persisted record kinds.""" +def test_a_served_profiles_tick_authors_board_records_as_that_profile(board_env, monkeypatch): + """A→B→A under the home override with ``HERMES_PROFILE`` unset (the multiplexed tick shape): + comment author and ``created_by`` are the ticking profile's, never ``"worker"``. A launch-side + ``HERMES_PROFILE`` pin does not re-label a served profile's writes.""" from tools import kanban_tools as kt + root, tid = board_env + monkeypatch.setenv("HERMES_PROFILE", "launch-host") # the launch process's own pin + + for name in ("alpha", "beta", "alpha"): + token = set_hermes_home_override(root / "profiles" / name) + try: + out = json.loads(kt._handle_comment({"task_id": tid, "body": f"from {name}"})) + assert out["ok"], out + assert _last_comment_author(tid) == name + child = json.loads(kt._handle_create( + {"title": f"{name} child", "assignee": "peer", "parents": [tid]})) + assert child["ok"], child + assert _created_by(child["task_id"]) == name + finally: + reset_hermes_home_override(token) + + +def test_a_dispatched_worker_keeps_its_pinned_identity_and_an_unnamed_caller_stays_generic( + board_env, monkeypatch): + """Control: no override → the dispatcher's ``HERMES_PROFILE`` pin wins over the process home; + absence: nothing names a profile and the home is not a profile → ``"worker"``.""" + from tools import kanban_tools as kt + root, tid = board_env + monkeypatch.setenv("HERMES_PROFILE", "pinned-bot") + assert json.loads(kt._handle_comment({"task_id": tid, "body": "from env"}))["ok"] + assert _last_comment_author(tid) == "pinned-bot" - out = json.loads(kt._handle_comment({"task_id": board_env, "body": "from env"})) - assert out["ok"] - assert _last_comment_author(board_env) == "pinned-bot" - - child = json.loads(kt._handle_create( - {"title": "env child", "assignee": "peer", "parents": [board_env]})) - assert child["ok"] - assert _created_by(child["task_id"]) == "pinned-bot" - - -def test_active_profile_used_when_env_absent(board_env): - """The regression: no ``HERMES_PROFILE`` in the environment, but - ``HERMES_HOME`` names the active profile — persisted records must carry - that profile, not the generic ``"worker"`` label.""" - from tools import kanban_tools as kt - - out = json.loads(kt._handle_comment( - {"task_id": board_env, "body": "from active profile"})) - assert out["ok"] - assert _last_comment_author(board_env) == "qa-bot" - - child = json.loads(kt._handle_create( - {"title": "profile child", "assignee": "peer", "parents": [board_env]})) - assert child["ok"] - assert _created_by(child["task_id"]) == "qa-bot" - - -def _raiser(): - raise RuntimeError("profile store unavailable") - - -@pytest.mark.parametrize("stub", [lambda: "", _raiser]) -def test_generic_fallback_without_any_profile(board_env, monkeypatch, stub): - """No env profile AND an unusable active-profile API → generic ``"worker"`` - (records stay attributable to *a* worker instead of failing the call).""" + monkeypatch.delenv("HERMES_PROFILE") import hermes_cli.profiles as profiles - monkeypatch.setattr(profiles, "get_active_profile_name", stub) - from tools import kanban_tools as kt - - out = json.loads(kt._handle_comment({"task_id": board_env, "body": "anon"})) - assert out["ok"] - assert _last_comment_author(board_env) == "worker" - - child = json.loads(kt._handle_create( - {"title": "anon child", "assignee": "peer", "parents": [board_env]})) - assert child["ok"] - assert _created_by(child["task_id"]) == "worker" - - -def _comment_authors(tid: str) -> list[str]: - from hermes_cli import kanban_db as kb - from hermes_cli import kanban_db_connect as kbc - conn = kbc.connect() - try: - return [c.author for c in kb.list_comments(conn, tid)] - finally: - conn.close() - - -def test_caller_args_cannot_override_persisted_identity(board_env): - """#19713 stays fixed — and got stronger upstream: ``author``/``created_by`` - are undeclared parameters of ``kanban_comment``/``kanban_create``, so the - handler wrapper rejects the call outright ("Nothing changed") before it can - reach the handler. Identity can never come from tool args (see also - ``_persisted_identity``, which has no args surface).""" - from tools import kanban_tools as kt - - out = json.loads(kt._handle_comment( - {"task_id": board_env, "body": "hi", "author": "hermes-system"})) - assert not out.get("ok") - assert "unknown parameter" in out["error"] - assert _comment_authors(board_env) == [] # nothing persisted - - child = json.loads(kt._handle_create( - {"title": "forged child", "assignee": "peer", "parents": [board_env], - "created_by": "hermes-system"})) - assert not child.get("ok") - assert "unknown parameter" in child["error"] + monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "") + assert json.loads(kt._handle_comment({"task_id": tid, "body": "anon"}))["ok"] + assert _last_comment_author(tid) == "worker" diff --git a/tools/kanban_tools.py b/tools/kanban_tools.py index cfb9855815..64db0c316c 100644 --- a/tools/kanban_tools.py +++ b/tools/kanban_tools.py @@ -139,28 +139,15 @@ _UNDECLARED_ARGS: dict[str, frozenset[str]] = { def _persisted_identity() -> str: """Profile name persisted into board records (comment author, task creator). - Resolution mirrors ``hermes_cli.kanban._profile_author`` and - ``cron.lifecycle_guard._current_profile_name``: environment first (the - dispatcher pins ``HERMES_PROFILE``; some launchers set - ``HERMES_PROFILE_NAME``), else the active profile derived from - ``HERMES_HOME`` via ``hermes_cli.profiles.get_active_profile_name``, else - the generic ``"worker"`` fallback. Never taken from tool args: board - records are injected into future workers' prompts, so a caller-supplied + ``hermes_cli.profiles.current_profile_name`` resolves the profile this call runs FOR — the bound + home override under a multiplexed tick or turn, else the dispatcher's ``HERMES_PROFILE`` pin, + else the process home; the generic ``"worker"`` only when nothing names a profile. Never taken + from tool args: board records are injected into future workers' prompts, so a caller-supplied identity could forge an authoritative-looking author (see #19713). """ - for env_name in ("HERMES_PROFILE_NAME", "HERMES_PROFILE"): - value = (os.environ.get(env_name) or "").strip() - if value: - return value - try: - from hermes_cli.profiles import get_active_profile_name + from hermes_cli.profiles import current_profile_name - active = (get_active_profile_name() or "").strip() - if active: - return active - except Exception: - logger.debug("kanban identity: active-profile lookup failed", exc_info=True) - return "worker" + return current_profile_name("worker") or "worker" def _kanban_handler(tool_name: str) -> Callable: @@ -1103,13 +1090,10 @@ def _resolve_notify_target() -> Optional[dict[str, Any]]: chat_type = env("HERMES_SESSION_CHAT_TYPE", "") or None thread_id = env("HERMES_SESSION_THREAD_ID", "") or None message_id = env("HERMES_SESSION_MESSAGE_ID", "") or "" - notifier_profile = env("HERMES_SESSION_PROFILE", "") or os.environ.get("HERMES_PROFILE") + notifier_profile = env("HERMES_SESSION_PROFILE", "") if not notifier_profile: - try: - from hermes_cli.profiles import get_active_profile_name - notifier_profile = get_active_profile_name() or "default" - except Exception: - notifier_profile = "default" + from hermes_cli.profiles import current_profile_name + notifier_profile = current_profile_name("default") delivery_metadata: dict[str, Any] = { k: v for k, v in ( ("thread_id", thread_id), ("chat_type", chat_type),