From f5bd77b3e16d86e3cbd75a9d6bd719f28dd8dbb9 Mon Sep 17 00:00:00 2001 From: GodsBoy Date: Sun, 3 May 2026 22:33:11 +0200 Subject: [PATCH] fix(kanban): anchor board, workspaces, and worker logs at the shared Hermes root The Kanban board is documented as shared across all Hermes profiles, but `kanban_db_path()` and `workspaces_root()` resolved through `get_hermes_home()`, which returns the active profile's HERMES_HOME. When the dispatcher spawned a worker with `hermes -p --skills kanban-worker chat -q "work kanban task "`, the worker rewrote HERMES_HOME to the profile subdirectory before kanban_db.py imported, opening a profile-local `kanban.db` that did not contain the dispatcher's task. `kanban_show` and `kanban_complete` failed; the dispatcher's row stayed `running` and was retried/crashed. The same defect applied to `_default_spawn`'s log directory and `worker_log_path`, so `hermes kanban tail` did not see the worker's output. Add `kanban_home()` in `hermes_cli/kanban_db.py` that resolves through `HERMES_KANBAN_HOME` (explicit override) then `get_default_hermes_root()`, which already understands the `/profiles/` and Docker / custom HERMES_HOME shapes. Reroute `kanban_db_path`, `workspaces_root`, the `_default_spawn` log directory, `gc_worker_logs`, and `worker_log_path` through it. Profile-specific config, `.env`, memory, and sessions stay isolated as before; only the kanban surface is shared. Add a `TestSharedBoardPaths` regression class to `tests/hermes_cli/test_kanban_db.py` covering: default install, profile-worker convergence, Docker custom HERMES_HOME, Docker profile layout, explicit `HERMES_KANBAN_HOME` override, and a real SQLite round-trip across dispatcher and worker HERMES_HOME perspectives. The dispatcher/worker convergence tests fail on origin/main and pass after the fix. Update the `kanban.md` user-guide page and the misleading docstrings in `kanban_db.py` to describe the shared-root behavior. Fixes #19348 --- hermes_cli/kanban.py | 2 +- hermes_cli/kanban_db.py | 76 ++++++++++--- tests/hermes_cli/test_kanban_db.py | 171 +++++++++++++++++++++++++++++ 3 files changed, 230 insertions(+), 19 deletions(-) diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index e23a4923f6d..46ec6c32ab4 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -366,7 +366,7 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu # --- log --- p_log = sub.add_parser( "log", - help="Print the worker log for a task (from $HERMES_HOME/kanban/logs/)", + help="Print the worker log for a task (from /kanban/logs/)", ) p_log.add_argument("task_id") p_log.add_argument("--tail", type=int, default=None, diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 1e8be214fb3..19311bcb697 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -1,8 +1,16 @@ """SQLite-backed Kanban board for multi-profile collaboration. -The board lives at ``$HERMES_HOME/kanban.db`` (profile-agnostic on purpose: -multiple profiles on the same machine all see the same board, which IS the -coordination primitive). +The board lives at ``/kanban.db`` where ```` is the **shared +Hermes root** (the parent of any active profile). Profiles intentionally +collapse onto a single board: it IS the cross-profile coordination +primitive. A worker spawned with ``hermes -p `` joins the same +board as the dispatcher that claimed the task. The same applies to +``/kanban/workspaces/`` and ``/kanban/logs/``. + +In standard installs ```` is ``~/.hermes``. In Docker / custom +deployments where ``HERMES_HOME`` points outside ``~/.hermes`` (e.g. +``/opt/hermes``), ```` is ``HERMES_HOME``. Set ``HERMES_KANBAN_HOME`` +to override the resolution explicitly (tests, unusual deployments). Schema is intentionally small: tasks, task_links, task_comments, task_events. The ``workspace_kind`` field decouples coordination from git @@ -61,16 +69,46 @@ _CTX_MAX_COMMENT_BYTES = 2 * 1024 # 2 KB per comment # Paths # --------------------------------------------------------------------------- +def kanban_home() -> Path: + """Return the shared Hermes root that anchors the kanban board. + + Resolution order: + + 1. ``HERMES_KANBAN_HOME`` env var when set and non-empty (explicit + override for tests and unusual deployments). + 2. ``get_default_hermes_root()``, which already returns ```` + when ``HERMES_HOME`` is ``/profiles/``, and returns + ``HERMES_HOME`` directly for Docker / custom deployments. + + The kanban board is shared across profiles **by design** (see the + module docstring). Resolving the kanban paths through the active + profile's ``HERMES_HOME`` would silently fork the board per profile, + which breaks the dispatcher / worker handoff. + """ + override = os.environ.get("HERMES_KANBAN_HOME", "").strip() + if override: + return Path(override) + from hermes_constants import get_default_hermes_root + return get_default_hermes_root() + + def kanban_db_path() -> Path: - """Return the path to ``kanban.db`` inside the active HERMES_HOME.""" - from hermes_constants import get_hermes_home - return get_hermes_home() / "kanban.db" + """Return the path to the shared ``kanban.db``. + + Anchored at :func:`kanban_home`, not the active profile's + ``HERMES_HOME``, so profile workers and the dispatcher converge on + the same board. + """ + return kanban_home() / "kanban.db" def workspaces_root() -> Path: - """Return the directory under which ``scratch`` workspaces are created.""" - from hermes_constants import get_hermes_home - return get_hermes_home() / "kanban" / "workspaces" + """Return the directory under which ``scratch`` workspaces are created. + + Anchored at :func:`kanban_home` so workspace paths are stable across + profile workers spawned by the dispatcher. + """ + return kanban_home() / "kanban" / "workspaces" # --------------------------------------------------------------------------- @@ -1516,12 +1554,15 @@ def archive_task(conn: sqlite3.Connection, task_id: str) -> bool: def resolve_workspace(task: Task) -> Path: """Resolve (and create if needed) the workspace for a task. - - ``scratch``: a fresh dir under ``$HERMES_HOME/kanban/workspaces//``. + - ``scratch``: a fresh dir under ``/kanban/workspaces//``, + where ```` is the shared Hermes root (see + :func:`kanban_home`). The path is the same for the dispatcher and + every profile worker, so handoff is path-stable. - ``dir:``: the path stored in ``workspace_path``. Created if missing. MUST be absolute — relative paths are rejected to prevent confused-deputy traversal where ``../../../tmp/attacker`` resolves against the dispatcher's CWD instead of a meaningful - root. Users who want a HERMES_HOME-relative workspace should + root. Users who want a kanban-root-relative workspace should compute the absolute path themselves. - ``worktree``: a git worktree at ``workspace_path``. Not created automatically in v1 -- the kanban-worker skill documents @@ -2104,9 +2145,10 @@ def _default_spawn(task: Task, workspace: str) -> Optional[int]: "chat", "-q", prompt, ]) - # Redirect output to a per-task log under HERMES_HOME/kanban/logs/. - from hermes_constants import get_hermes_home - log_dir = get_hermes_home() / "kanban" / "logs" + # Redirect output to a per-task log under /kanban/logs/. + # Anchored at the shared kanban root, not the worker's profile home, + # so `hermes kanban tail` reads the same file the worker writes to. + log_dir = kanban_home() / "kanban" / "logs" log_dir.mkdir(parents=True, exist_ok=True) log_path = log_dir / f"{task.id}.log" _rotate_worker_log(log_path, DEFAULT_LOG_ROTATE_BYTES) @@ -2591,8 +2633,7 @@ def gc_worker_logs( """Delete worker log files older than ``older_than_seconds``. Returns the number of files removed. Kept separate from ``gc_events`` because log files live on disk, not in SQLite.""" - from hermes_constants import get_hermes_home - log_dir = get_hermes_home() / "kanban" / "logs" + log_dir = kanban_home() / "kanban" / "logs" if not log_dir.exists(): return 0 cutoff = time.time() - older_than_seconds @@ -2614,8 +2655,7 @@ def gc_worker_logs( def worker_log_path(task_id: str) -> Path: """Return the path to a worker's log file. The file may not exist (task never spawned, or log already GC'd).""" - from hermes_constants import get_hermes_home - return get_hermes_home() / "kanban" / "logs" / f"{task_id}.log" + return kanban_home() / "kanban" / "logs" / f"{task_id}.log" def read_worker_log( diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index fcc6396be40..6214ab758a3 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -436,3 +436,174 @@ def test_tenant_propagates_to_events(kanban_home): # The "created" event should have tenant in its payload. created = [e for e in events if e.kind == "created"] assert created and created[0].payload.get("tenant") == "biz-a" + + +# --------------------------------------------------------------------------- +# Shared-board path resolution (issue #19348) +# +# The kanban board is a cross-profile coordination primitive: a worker +# spawned with `hermes -p ` must read/write the same kanban.db +# as the dispatcher that claimed the task. These tests exercise the +# path-resolution layer directly and would have caught the regression +# where `kanban_db_path()` resolved to the active profile's HERMES_HOME. +# --------------------------------------------------------------------------- + +class TestSharedBoardPaths: + """`kanban_home`/`kanban_db_path`/`workspaces_root`/`worker_log_path` + must anchor at the **shared root**, not the active profile's HERMES_HOME.""" + + def _set_home(self, monkeypatch, tmp_path, hermes_home): + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.delenv("HERMES_KANBAN_HOME", raising=False) + + def test_default_install_anchors_at_home_dot_hermes( + self, tmp_path, monkeypatch + ): + # Standard install: HERMES_HOME == ~/.hermes, no profile active. + default_home = tmp_path / ".hermes" + default_home.mkdir() + self._set_home(monkeypatch, tmp_path, default_home) + + assert kb.kanban_home() == default_home + assert kb.kanban_db_path() == default_home / "kanban.db" + assert kb.workspaces_root() == default_home / "kanban" / "workspaces" + assert ( + kb.worker_log_path("t_demo") + == default_home / "kanban" / "logs" / "t_demo.log" + ) + + def test_profile_worker_resolves_to_shared_root( + self, tmp_path, monkeypatch + ): + # Reproduces the bug: dispatcher uses ~/.hermes/kanban.db, + # worker spawned with -p previously resolved to + # ~/.hermes/profiles//kanban.db. After the fix both + # converge on ~/.hermes/kanban.db. + default_home = tmp_path / ".hermes" + default_home.mkdir() + profile_home = default_home / "profiles" / "nehemiahkanban" + profile_home.mkdir(parents=True) + self._set_home(monkeypatch, tmp_path, profile_home) + + # All four resolvers must anchor at the shared root, not the + # profile-local HERMES_HOME. + assert kb.kanban_home() == default_home + assert kb.kanban_db_path() == default_home / "kanban.db" + assert kb.workspaces_root() == default_home / "kanban" / "workspaces" + assert ( + kb.worker_log_path("t_0d214f19") + == default_home / "kanban" / "logs" / "t_0d214f19.log" + ) + + # Sanity: the profile-local path that used to be returned is + # explicitly NOT what we resolve to anymore. + assert kb.kanban_db_path() != profile_home / "kanban.db" + + def test_dispatcher_and_profile_worker_converge( + self, tmp_path, monkeypatch + ): + # End-to-end convergence: resolve the path under each side's + # HERMES_HOME and confirm equality. This is the property the + # dispatcher/worker handoff actually depends on. + default_home = tmp_path / ".hermes" + default_home.mkdir() + profile_home = default_home / "profiles" / "coder" + profile_home.mkdir(parents=True) + + # Dispatcher's perspective. + self._set_home(monkeypatch, tmp_path, default_home) + dispatcher_db = kb.kanban_db_path() + dispatcher_ws = kb.workspaces_root() + dispatcher_log = kb.worker_log_path("t_handoff") + + # Worker's perspective (profile activated by `hermes -p coder`). + monkeypatch.setenv("HERMES_HOME", str(profile_home)) + worker_db = kb.kanban_db_path() + worker_ws = kb.workspaces_root() + worker_log = kb.worker_log_path("t_handoff") + + assert dispatcher_db == worker_db + assert dispatcher_ws == worker_ws + assert dispatcher_log == worker_log + + def test_docker_custom_hermes_home_uses_env_path_directly( + self, tmp_path, monkeypatch + ): + # Docker / custom deployment: HERMES_HOME points outside ~/.hermes. + # `get_default_hermes_root()` returns env_home directly when it + # is not a `/profiles/` shape and not under + # `Path.home() / ".hermes"`. + custom_root = tmp_path / "opt" / "hermes" + custom_root.mkdir(parents=True) + self._set_home(monkeypatch, tmp_path, custom_root) + + assert kb.kanban_home() == custom_root + assert kb.kanban_db_path() == custom_root / "kanban.db" + + def test_docker_profile_layout_uses_grandparent( + self, tmp_path, monkeypatch + ): + # Docker profile shape: HERMES_HOME=/opt/hermes/profiles/coder; + # `get_default_hermes_root()` walks up to /opt/hermes because + # the immediate parent dir is named "profiles". + custom_root = tmp_path / "opt" / "hermes" + profile = custom_root / "profiles" / "coder" + profile.mkdir(parents=True) + self._set_home(monkeypatch, tmp_path, profile) + + assert kb.kanban_home() == custom_root + assert kb.kanban_db_path() == custom_root / "kanban.db" + + def test_explicit_override_via_hermes_kanban_home( + self, tmp_path, monkeypatch + ): + # Explicit override: HERMES_KANBAN_HOME beats every other + # resolution rule. + default_home = tmp_path / ".hermes" + profile_home = default_home / "profiles" / "any" + profile_home.mkdir(parents=True) + override = tmp_path / "shared-board" + override.mkdir() + + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(profile_home)) + monkeypatch.setenv("HERMES_KANBAN_HOME", str(override)) + + assert kb.kanban_home() == override + assert kb.kanban_db_path() == override / "kanban.db" + assert kb.workspaces_root() == override / "kanban" / "workspaces" + + def test_empty_override_falls_through(self, tmp_path, monkeypatch): + # Empty/whitespace override is treated as unset. + default_home = tmp_path / ".hermes" + default_home.mkdir() + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(default_home)) + monkeypatch.setenv("HERMES_KANBAN_HOME", " ") + + assert kb.kanban_home() == default_home + + def test_dispatcher_and_worker_share_a_real_database( + self, tmp_path, monkeypatch + ): + # Belt-and-suspenders: round-trip a task across the two + # HERMES_HOME perspectives via a real SQLite file. Without the + # fix the worker would open a different file and see no rows. + default_home = tmp_path / ".hermes" + default_home.mkdir() + profile_home = default_home / "profiles" / "nehemiahkanban" + profile_home.mkdir(parents=True) + + # Dispatcher creates the board and a task. + self._set_home(monkeypatch, tmp_path, default_home) + kb.init_db() + with kb.connect() as conn: + task_id = kb.create_task(conn, title="cross-profile") + + # Worker switches to the profile HERMES_HOME and reads. + monkeypatch.setenv("HERMES_HOME", str(profile_home)) + with kb.connect() as conn: + task = kb.get_task(conn, task_id) + assert task is not None + assert task.title == "cross-profile"