fix(kanban): board identity follows the bound profile override, not the launch env
One resolver, hermes_cli.profiles.current_profile_name(): the HERMES_HOME override (a multiplexed cron tick or routed gateway turn) names the profile first; the dispatcher's HERMES_PROFILE pin is consulted only when no override is bound; the process home last. #112888 added the HERMES_HOME-derived fallback but kept the env pin FIRST, so under a multiplexer whose launch process carries a HERMES_PROFILE the served profile's writes were still re-labelled with the host's name. The same resolver replaces the per-module copies in hermes_cli/kanban.py, kanban_specify.py, cron/lifecycle_guard.py and the kanban notify-target default. Tests trimmed to two invariants (A->B->A under the override; control pin + generic absence). Closes #119859 Supersedes #112888
This commit is contained in:
teknium1
@@ -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:
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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]:
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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 (``<root>/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 ``<root>`` 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"
|
||||
|
||||
@@ -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),
|
||||
|
||||
Reference in New Issue
Block a user