From f811f623ed5c6dc6203c6d7cb73506090dcb708e Mon Sep 17 00:00:00 2001 From: Rodrigo Gomes <2362425+rodrigogs@users.noreply.github.com> Date: Sun, 6 Sep 2026 07:27:08 -0300 Subject: [PATCH] feat(kanban): fire kanban_worktree_created hook after git worktree add A card created with `--workspace worktree` gets a linked git worktree, and a linked worktree is a fresh checkout: tracked files and nothing else. Everything git ignores is absent by definition, and for most real projects that includes the `.env` the application cannot start without. The worker lands in a repository that looks complete and fails at the first command that reads configuration, with an error about the application rather than the checkout. Which secrets a project needs, and where they live, is deployment knowledge no code here can hold, so this adds a seam through the hook system that already exists rather than a new one. `_ensure_git_worktree` fires the new `kanban_worktree_created` plugin hook (hermes_cli/kanban_db_workspace.py) right after `git worktree add` succeeds and before any worker spawns, with task_id, board, profile_name, worktree_path, repo_root and branch. A plugin subscribes with ctx.register_hook; a shell script subscribes through the `hooks:` block in config.yaml like every other event, so `hermes hooks list/doctor/test` already know it and consent is the bridge's existing allowlist (TTY prompt, hooks_auto_accept, HERMES_ACCEPT_HOOKS). With no subscriber nothing changes. Unlike the other kanban_* hooks this one is a gate: a `{"action": "block", "message"}` result (shell bridge: exit 2, or fail_closed on a timeout or spawn failure) makes the caller remove the just-created worktree and raise with the message. That raise lands in dispatch_once's workspace guard, which records a spawn_failed and releases the claim so the card is retried; removing the tree first is what keeps that retry honest, because a worktree left behind would take `_ensure_git_worktree`'s idempotent early return, skip both the `git worktree add` and the hook, and spawn the worker into the unseeded checkout with no error attached. `git worktree remove --force` is required (a hook that failed halfway leaves untracked files); the branch is kept. If the removal itself fails the reason says so and names the path to remove by hand. The retry-safety sentence is placed before the hook's text and before git's error so `_record_task_failure`'s 500-char cut cannot drop it. Shell hooks only reach the plugin manager through register_from_config, which the CLI runs for agent commands (_prepare_agent_startup) and the gateway runs at startup; `hermes kanban` is neither, so `hermes kanban claim`, a one-shot `hermes kanban dispatch` and `hermes kanban daemon --force` fired the hook to an empty subscriber list and handed over an unseeded tree with no warning. kanban_ops.py gains `_register_config_hooks(args)` (the gateway's three lines: load_config, register_from_config with the inherited --accept-hooks flag, never raising), called in _cmd_dispatch (not on --dry-run), _cmd_daemon after init_db and _cmd_claim before resolve_workspace. Python plugin hooks were never affected (invoke_hook discovers plugins lazily). Bridge changes are two entries: `kanban_worktree_created` joins `_BLOCKING_EVENTS` in agent/shell_hooks.py (exit 2 and fail_closed now apply to it) and gets a block-only stdout parser; `hermes hooks test/doctor` gains a synthetic payload for the event, and doctor reports a block verdict on that payload as such instead of the "observer-only" line it printed for any empty stdout. Process-group kill on timeout, consent and non-TTY acceptance are the bridge's existing behaviour, reused rather than re-implemented. Tests: tests/hermes_cli/test_kanban_worktree_created_hook.py, 13 cases against a real plugin manager and real `hooks:` registration with real scripts and real `git worktree add` (no subprocess or invoke_hook mocks; the removal-failure case stubs the module's `_git` wrapper for `worktree remove`, the CLI cases stub the worker spawn): payload and after-checkout timing, once-per-creation, no-subscriber no-op, block -> discard -> raise, retry re-fires, retry verdict inside the 500-char card budget in both removal branches, shell hook seeding from the stdin payload, exit 2 with stderr as the reason, fail_closed timeout that also kills the hook's grandchild, and claim / dispatch / daemon --force each seeding through a `hooks:` block in a real config.yaml. tests/hermes_cli/test_hooks_cli.py: doctor reports the block. Docs: hooks.md (catalog row, kanban_worktree_created section under the renamed "Kanban lifecycle hooks" heading, blocking-event wording for exit 2 / fail_closed, which entry points register shell hooks), kanban.md (seeding subsection with a config.yaml + script example, which paths wire the hooks: block, what doctor prints for the seed script, `hermes kanban claim` caveat), plugins.md and developer-guide/plugins/index.md hook tables. --- agent/shell_hooks.py | 21 +- hermes_cli/hooks.py | 15 +- hermes_cli/kanban.py | 4 + hermes_cli/kanban_db_workspace.py | 118 +++- hermes_cli/kanban_ops.py | 27 + hermes_cli/plugins.py | 8 + tests/hermes_cli/test_hooks_cli.py | 15 + .../test_kanban_worktree_created_hook.py | 502 ++++++++++++++++++ website/docs/developer-guide/plugins/index.md | 3 +- website/docs/user-guide/features/hooks.md | 24 +- website/docs/user-guide/features/kanban.md | 43 +- website/docs/user-guide/features/plugins.md | 4 +- 12 files changed, 758 insertions(+), 26 deletions(-) create mode 100644 tests/hermes_cli/test_kanban_worktree_created_hook.py diff --git a/agent/shell_hooks.py b/agent/shell_hooks.py index 402b740b785ba..4bff8d46918d7 100644 --- a/agent/shell_hooks.py +++ b/agent/shell_hooks.py @@ -2,7 +2,8 @@ callbacks on the plugin hook manager, so every ``invoke_hook()`` site dispatches to the scripts. Wire: stdin JSON ``{hook_event_name, tool_name, tool_input, session_id, cwd, extra}``; optional stdout JSON ``{"decision"|"action": "block"|"modify", ...}`` / ``{"context": ...}`` via ``_parse_response``. -Exit code 2 blocks a ``pre_tool_call`` even without JSON (Claude-Code / Cursor). Fail open unless ``fail_closed``.""" +Exit code 2 blocks a blocking-capable event (``pre_tool_call``, ``kanban_worktree_created``) even without JSON +(Claude-Code / Cursor). Fail open unless ``fail_closed``.""" from __future__ import annotations @@ -41,7 +42,8 @@ # Exit code that signals "block this action" independent of stdout (Claude Code / Cursor). BLOCK_EXIT_CODE = 2 # Events whose block directive is honored downstream; exit-2 blocking and fail_closed only apply here. -_BLOCKING_EVENTS = frozenset({"pre_tool_call"}) +# kanban_worktree_created: the dispatcher removes the just-created worktree and fails the card. +_BLOCKING_EVENTS = frozenset({"pre_tool_call", "kanban_worktree_created"}) _TOOL_EVENTS = frozenset({"pre_tool_call", "post_tool_call"}) _STDERR_MESSAGE_LIMIT = 400 _TRUTHY = {"1", "true", "yes", "on"} @@ -438,10 +440,18 @@ def _block_message(primary: Any, secondary: Any) -> str: _PRE_TOOL_DIALECTS = (("action", "message", "reason", "args"), ("decision", "reason", "message", "tool_input")) -def _parse_pre_tool_call(data: Dict[str, Any]) -> Optional[Dict[str, Any]]: +def _parse_block_directive(data: Dict[str, Any]) -> Optional[Dict[str, Any]]: + """The block half of the dialects — the only directive ``kanban_worktree_created`` consumes.""" for verb, primary, secondary, _ in _PRE_TOOL_DIALECTS: if data.get(verb) == "block": return {"action": "block", "message": _block_message(data.get(primary), data.get(secondary))} + return None + + +def _parse_pre_tool_call(data: Dict[str, Any]) -> Optional[Dict[str, Any]]: + block = _parse_block_directive(data) + if block is not None: + return block for verb, _, _, payload in _PRE_TOOL_DIALECTS: if data.get(verb) == "modify" and isinstance(data.get(payload), dict): return {"action": "modify", "args": data[payload]} @@ -471,7 +481,10 @@ def _parse_context(data: Dict[str, Any]) -> Optional[Dict[str, Any]]: return {"context": context} if isinstance(context, str) and context.strip() else None -_RESPONSE_PARSERS: Dict[str, Callable[[Dict[str, Any]], Optional[Dict[str, Any]]]] = {"pre_tool_call": _parse_pre_tool_call, "pre_verify": _parse_pre_verify} +_RESPONSE_PARSERS: Dict[str, Callable[[Dict[str, Any]], Optional[Dict[str, Any]]]] = { + "pre_tool_call": _parse_pre_tool_call, "pre_verify": _parse_pre_verify, + "kanban_worktree_created": _parse_block_directive, +} def _parse_response(event: str, stdout: str) -> Optional[Dict[str, Any]]: diff --git a/hermes_cli/hooks.py b/hermes_cli/hooks.py index baa5feea4dac7..abd8d5f025a08 100644 --- a/hermes_cli/hooks.py +++ b/hermes_cli/hooks.py @@ -169,6 +169,11 @@ def _mtime_drift(shell_hooks, spec, entry) -> tuple[bool | None, str, str]: "response_model": "claude-haiku-4-5", "usage": {"input_tokens": 256, "output_tokens": 12}, "assistant_content_chars": 40, "assistant_tool_call_count": 0, "error": None, "error_type": None, }, + "kanban_worktree_created": { + "task_id": "test-task", "board": "default", "profile_name": "default", + "worktree_path": "/tmp/repo/.worktrees/test-task", "repo_root": "/tmp/repo", + "branch": "wt/test-task", + }, "subagent_stop": { "parent_session_id": "parent-sess", "child_role": None, "child_summary": "Synthetic summary for hooks test", "child_status": "completed", @@ -332,7 +337,15 @@ def _doctor_one(spec, shell_hooks) -> int: rc = result.get("returncode") elapsed = result.get("elapsed_seconds", 0) stdout = (result.get("stdout") or "").strip() - if not stdout: + parsed = result.get("parsed") + if isinstance(parsed, dict) and parsed.get("action") == "block": + # Exit 2 (or block JSON) on a blocking-capable event: the gate fired. Expected for a + # script that checks real paths the synthetic payload cannot provide (e.g. a + # kanban_worktree_created seeder looking for /tmp/repo/.env); not an error, but not + # "observer-only" either — say what the dispatcher would have received. + print(f" ✓ returned a block on the synthetic payload (exit={rc}, {elapsed}s): " + f"{_truncate(str(parsed.get('message') or ''), 120)}") + elif not stdout: print(f" ✓ ran clean with empty stdout " f"(exit={rc}, {elapsed}s) — hook is observer-only") else: diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 3f756f2fa9e0d..c907d8cdd76c1 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -30,6 +30,7 @@ from hermes_cli.kanban_boards import _dispatch_boards from hermes_cli.kanban_ops import ( _cmd_daemon, _kanban_config, _cmd_dispatch, _cmd_gc, _cmd_repair, _cmd_tail, _cmd_watch, + _register_config_hooks, ) from hermes_cli.kanban_parser import build_parser # noqa: F401 (re-exported: hermes_cli.main, run_slash) @@ -740,6 +741,9 @@ def _cmd_claim(args: argparse.Namespace) -> int: return _err(f"no such task: {args.task_id}") return _err(f"cannot claim {args.task_id}: status={existing.status} " f"lock={existing.claim_lock or '(none)'}") + # A worktree card fires kanban_worktree_created from resolve_workspace; the + # hooks: block has to be wired first or a configured seed script never runs. + _register_config_hooks(args) workspace = kbw.resolve_workspace(task) kbw.set_workspace_path(conn, task.id, str(workspace)) print(f"Claimed {task.id}\nWorkspace: {workspace}") diff --git a/hermes_cli/kanban_db_workspace.py b/hermes_cli/kanban_db_workspace.py index 84afc9f5093c9..2633172806bda 100644 --- a/hermes_cli/kanban_db_workspace.py +++ b/hermes_cli/kanban_db_workspace.py @@ -482,8 +482,15 @@ def _repo_root_for_worktree_target(path: Path) -> Optional[Path]: current = current.parent -def _ensure_git_worktree(repo_root: Path, target: Path, branch_name: str) -> None: - """Materialize ``target`` as a linked git worktree under ``repo_root``.""" +def _ensure_git_worktree( + repo_root: Path, target: Path, branch_name: str, *, + task_id: Optional[str] = None, board: Optional[str] = None, +) -> None: + """Materialize ``target`` as a linked git worktree under ``repo_root``. + + A worktree created here fires ``kanban_worktree_created`` before this + returns (see :func:`_fire_worktree_created_hook`); the idempotent early + return for an already-materialized checkout does not.""" target = target.expanduser() repo_common = _git_common_dir(repo_root) if target.exists() and repo_common is not None and _path_key(_git_common_dir(target)) == _path_key(repo_common): @@ -499,12 +506,107 @@ def _ensure_git_worktree(repo_root: Path, target: Path, branch_name: str) -> Non raise RuntimeError( f"git worktree add failed for {target} on branch {branch_name}: {stderr}" ) + _fire_worktree_created_hook(repo_root, target, branch_name, task_id=task_id, board=board) + + +#: The card reason travels through ``_record_task_failure`` +#: (hermes_cli/kanban_db_dispatch.py), which keeps ``error[:500]``. The sentence +#: saying whether a retry is safe is therefore placed BEFORE the hook's own text +#: and before git's removal error; these caps only bound the tail (a hook that +#: dumps its whole stderr, git's full refusal) so the exception stays readable. +_HOOK_MESSAGE_LIMIT = 300 +_REMOVAL_DETAIL_LIMIT = 120 + + +def _discard_unseeded_worktree(repo_root: Path, target: Path) -> str: + """Remove a worktree whose ``kanban_worktree_created`` hook blocked; describe the outcome. + + Removal is part of the failure, not a courtesy. The raise it accompanies + lands in ``dispatch_once``'s workspace guard, which records a ``spawn_failed`` + and RELEASES the claim, so the card is retried on the next tick. Left in + place, the half-seeded tree would make ``_ensure_git_worktree``'s idempotent + early return fire on that retry: no ``git worktree add``, no hook, and a + worker spawned into the unseeded checkout the block existed to prevent. + ``--force`` is required: a hook that failed halfway leaves untracked files + and git refuses to remove a dirty worktree. The branch is kept — it may + have existed before this call, and the retry's ``worktree add + `` works on it once the tree is gone. + + If the removal itself fails the tree stays and the next retry WILL spawn + into it; nothing here can prevent that, so the sentence says to remove it + by hand and names why git refused. + """ + try: + result = _git(repo_root, "worktree", "remove", "--force", str(target), timeout=60) + except Exception as exc: # git missing, timeout, ... + detail = str(exc) + else: + if result.returncode == 0: + return "the unseeded worktree was removed, so a retry re-creates and re-seeds it" + detail = (result.stderr or result.stdout or "").strip() + return ( + "the unseeded worktree could NOT be removed — remove it by hand before the retry " + f"({detail[:_REMOVAL_DETAIL_LIMIT]})" + ) + + +def _fire_worktree_created_hook( + repo_root: Path, target: Path, branch_name: str, *, + task_id: Optional[str], board: Optional[str], +) -> None: + """Fire ``kanban_worktree_created`` for a worktree created just now; a block fails the card. + + A linked worktree is a fresh checkout: tracked files and nothing else, so + everything git ignores — for most real projects the ``.env`` the application + cannot start without — is absent by definition. Which secrets a project + needs, and where they live, is deployment knowledge, so this is the seam for + it: a plugin (``ctx.register_hook``) or a ``hooks:`` entry in config.yaml + (the shell bridge in ``agent.shell_hooks``) seeds the checkout before any + worker sees it. + + Unlike the other ``kanban_*`` hooks this one is a gate, not an observer. It + runs synchronously on the caller's thread (``invoke_hook`` does not bound + ``kanban_*`` callbacks; a shell hook enforces its own ``timeout``), and a + ``{"action": "block", "message"}`` result — the directive shape + ``pre_tool_call`` uses, which the shell bridge derives from exit code 2 or + ``fail_closed`` — discards the worktree and raises. Raising is deliberate: + the hook exists only because somebody installed it, so its failure is a + configuration error they want on the card now, not a confusing application + failure later inside a checkout missing exactly what the hook was there to + provide. ``invoke_hook`` logs and skips a callback that RAISES, so plugins + must return the directive. With no subscriber nothing changes. + """ + from hermes_cli.lifecycle import invoke_hook + + try: + results = invoke_hook( + "kanban_worktree_created", task_id=task_id, board=board or _kb.get_current_board(), + profile_name=_kb._hook_profile_name(), worktree_path=str(target), + repo_root=str(repo_root), branch=branch_name, + ) + except Exception as exc: + discarded = _discard_unseeded_worktree(repo_root, target) + raise RuntimeError( + f"kanban_worktree_created hook could not run for {target}; {discarded}; " + f"cause: {str(exc)[:_HOOK_MESSAGE_LIMIT]}" + ) from exc + block = next((r for r in results if isinstance(r, dict) and r.get("action") == "block"), None) + if block is None: + return + discarded = _discard_unseeded_worktree(repo_root, target) + message = str(block.get("message") or "hook returned a block directive without a message") + raise RuntimeError( + f"kanban_worktree_created hook blocked the worktree {target}; {discarded}; " + f"hook: {message[:_HOOK_MESSAGE_LIMIT]}" + ) -def _anchored_worktree(repo_root: Path, task_id: str, branch_name: str) -> tuple[Path, str]: +def _anchored_worktree( + repo_root: Path, task_id: str, branch_name: str, *, board: Optional[str] = None, +) -> tuple[Path, str]: """Materialize the canonical ``/.worktrees/`` worktree.""" target = repo_root / ".worktrees" / task_id - _ensure_git_worktree(repo_root, target, branch_name) + _ensure_git_worktree(repo_root, target, branch_name, task_id=task_id, board=board) return target, branch_name @@ -537,7 +639,7 @@ def _resolve_worktree_workspace(task: Task, *, board: Optional[str] = None) -> t f"task {task.id} has workspace_kind=worktree but board " f"{board_slug!r} default_workdir {board_default!r} is not inside a git repo" ) - return _anchored_worktree(repo_root, task.id, branch_name) + return _anchored_worktree(repo_root, task.id, branch_name, board=board) requested = Path(task.workspace_path).expanduser() if not requested.is_absolute(): @@ -560,7 +662,7 @@ def _resolve_worktree_workspace(task: Task, *, board: Optional[str] = None) -> t if fallback_root is not None: fallback = fallback_root / ".worktrees" / task.id if _path_key(fallback.resolve(strict=False)) != _path_key(requested_resolved): - _ensure_git_worktree(fallback_root, fallback, branch_name) + _ensure_git_worktree(fallback_root, fallback, branch_name, task_id=task.id, board=board) return fallback.resolve(strict=False), branch_name # No repo to anchor a fallback on (or the occupied path IS this task's # own canonical worktree): keep the legacy reuse rather than fail dispatch. @@ -568,7 +670,7 @@ def _resolve_worktree_workspace(task: Task, *, board: Optional[str] = None) -> t repo_root = _git_toplevel(requested) if repo_root is not None and _path_key(requested_resolved) == _path_key(repo_root): - return _anchored_worktree(repo_root, task.id, branch_name) + return _anchored_worktree(repo_root, task.id, branch_name, board=board) repo_root = _repo_root_for_worktree_target(requested.parent) if repo_root is None: @@ -576,7 +678,7 @@ def _resolve_worktree_workspace(task: Task, *, board: Optional[str] = None) -> t f"task {task.id} worktree path {task.workspace_path!r} is not inside a git repo " "and does not point at a git repo root" ) - _ensure_git_worktree(repo_root, requested, branch_name) + _ensure_git_worktree(repo_root, requested, branch_name, task_id=task.id, board=board) return requested, branch_name diff --git a/hermes_cli/kanban_ops.py b/hermes_cli/kanban_ops.py index 3f3b5fb46738c..85c7fe520dc28 100644 --- a/hermes_cli/kanban_ops.py +++ b/hermes_cli/kanban_ops.py @@ -7,6 +7,7 @@ import argparse import json +import logging import os import sys import time @@ -57,6 +58,29 @@ def tick(): return _poll_loop(args.interval, tick) +def _register_config_hooks(args: argparse.Namespace) -> None: + """Wire config.yaml's ``hooks:`` block onto the plugin manager for a standalone kanban entry point. + + ``hermes chat`` / ``gateway run`` / ``cron run`` do this in ``_prepare_agent_startup`` + (hermes_cli/main.py) and the gateway-hosted dispatcher in + ``GatewayStartupMixin._register_config_hooks``; ``hermes kanban`` is not an agent + command, so ``claim``, ``dispatch`` and ``daemon --force`` must do it themselves before + they resolve a workspace — otherwise a ``kanban_worktree_created`` shell hook the + operator configured never sees the worktree these paths create, and the unseeded tree + is then reused by every later tick. Python plugin hooks need nothing here + (``invoke_hook`` discovers plugins lazily). Consent is the bridge's own: the allowlist, + ``--accept-hooks``, ``HERMES_ACCEPT_HOOKS``, ``hooks_auto_accept``, or the TTY prompt. + Never raises: a broken hooks block must not take the dispatcher down. + """ + try: + from hermes_cli.config import load_config + from agent.shell_hooks import register_from_config + + register_from_config(load_config(), accept_hooks=bool(getattr(args, "accept_hooks", False))) + except Exception: + logging.getLogger(__name__).debug("shell-hook registration failed for hermes kanban", exc_info=True) + + def _cmd_dispatch(args: argparse.Namespace) -> int: # Honour kanban.default_assignee, kanban.max_in_progress, # kanban.max_in_progress_per_profile and kanban.max_spawn with the same @@ -81,6 +105,8 @@ def _cmd_dispatch(args: argparse.Namespace) -> int: except Exception: default_assignee = max_in_progress_per_profile = max_in_progress = None max_spawn = getattr(args, "max", None) + if not args.dry_run: # a dry run creates no workspace, so there is nothing for a hook to see + _register_config_hooks(args) with kbc.connect_closing() as conn: res = kbd.dispatch_once( conn, @@ -181,6 +207,7 @@ def _cmd_daemon(args: argparse.Namespace) -> int: # Init before printing "started" so the DB path is right and init errors # surface immediately. kb.init_db() + _register_config_hooks(args) pidfile = getattr(args, "pidfile", None) if pidfile: diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index bba82a3b4b0a3..73920119b9f2a 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -160,6 +160,14 @@ def _install_plugin_debug_handler(force: bool = False) -> None: # spawn; completed/blocked fire in the WORKER (or whichever process drove it). Kwargs: task_id, # board, assignee, run_id, profile_name; completed adds summary, blocked adds reason. "kanban_task_claimed", "kanban_task_completed", "kanban_task_blocked", + # kanban_worktree_created: the one kanban GATE (hermes_cli.kanban_db_workspace). Fires synchronously + # right after `git worktree add` succeeds and BEFORE the worker spawns (dispatcher, or `hermes kanban + # claim`) so an operator can seed what a fresh checkout lacks (gitignored .env files, caches). Kwargs: + # task_id, board, profile_name, worktree_path, repo_root, branch. Return None to accept; {"action": + # "block", "message"} (shell bridge: exit 2 / fail_closed) makes the caller remove the worktree and + # fail the card with the message; the retry re-creates and re-fires. A RAISING callback is logged and + # skipped like any other hook, so plugins must return the directive. Not timeout-bounded (kanban_*). + "kanban_worktree_created", # Kanban worker/mutation/tick observers; returns ignored; fire sites short-circuit on # has_hook(). Kwargs: task_id, profile_name, board, assignee, run_id plus, per hook: # worker_spawned (DISPATCHER, after PID persisted, inside the dispatch lock — stay fast): diff --git a/tests/hermes_cli/test_hooks_cli.py b/tests/hermes_cli/test_hooks_cli.py index 930c5c9ea08c8..8f5adb80c36cd 100644 --- a/tests/hermes_cli/test_hooks_cli.py +++ b/tests/hermes_cli/test_hooks_cli.py @@ -201,6 +201,21 @@ def test_unallowlisted_script_is_not_executed(self, tmp_path): assert "not allowlisted" in out.lower() assert "skipped JSON smoke test" in out + def test_block_on_synthetic_payload_is_reported_as_a_block(self, tmp_path): + """An allowlisted gate that exits 2 on doctor's synthetic payload (the documented + ``kanban_worktree_created`` seeder finds no ``/tmp/repo/.env``) must be reported as the + block the dispatcher would receive, not as a clean observer-only run.""" + script = _hook_script( + tmp_path, "#!/usr/bin/env bash\necho 'no .env in /tmp/repo' >&2\nexit 2\n", + ) + shell_hooks._record_approval("kanban_worktree_created", str(script)) + cfg = {"hooks": {"kanban_worktree_created": [{"command": str(script)}]}} + with patch("hermes_cli.config.load_config", return_value=cfg): + out = _run(SimpleNamespace(hooks_action="doctor")) + + assert "returned a block on the synthetic payload (exit=2" in out + assert "no .env in /tmp/repo" in out + assert "observer-only" not in out def test_print_run_result_shows_decision_for_error_and_timeout(): """A failing hook's decision must be printed, not hidden by early returns (#115968). diff --git a/tests/hermes_cli/test_kanban_worktree_created_hook.py b/tests/hermes_cli/test_kanban_worktree_created_hook.py new file mode 100644 index 0000000000000..1f0ede955a3c7 --- /dev/null +++ b/tests/hermes_cli/test_kanban_worktree_created_hook.py @@ -0,0 +1,502 @@ +"""The ``kanban_worktree_created`` lifecycle hook fired on kanban worktree creation. + +A linked worktree is a fresh checkout: it holds tracked files and nothing else, +so every gitignored file is absent by definition — including the ``.env`` most +projects cannot start without. Which secrets a project needs is deployment +knowledge no code in this repo can hold, so ``_ensure_git_worktree`` fires a +plugin hook right after ``git worktree add`` succeeds and BEFORE the worker is +spawned. A Python plugin subscribes with ``ctx.register_hook``; a shell script +subscribes through the ``hooks:`` block in ``config.yaml`` (the same bridge +every other plugin-hook event uses). + +Two things here are contracts, not implementation details, and neither may +drift silently: + +* the payload external hooks are written against — ``worktree_path``, + ``repo_root``, ``branch``, ``task_id`` — and the fact that the checkout + already exists when the hook runs; +* the atomicity of failure. ``dispatch_once`` turns the raise into a released + claim and retries the card, so a blocked hook MUST leave no worktree behind: + otherwise the retry takes ``_ensure_git_worktree``'s idempotent early return, + never re-fires the hook, and spawns the worker into the unseeded checkout the + block existed to prevent. + +The plugin-side cases register a callback on the real plugin manager; the +shell-side cases go through ``agent.shell_hooks.register_from_config`` with a +real script; the CLI cases run ``hermes kanban claim/dispatch/daemon`` bodies +against a real kanban DB and a ``hooks:`` block in a real ``config.yaml``. No +``subprocess`` or ``invoke_hook`` mocks — the one stand-in is the +removal-failure case, which stubs the module's ``_git`` wrapper for +``worktree remove`` only, and the CLI cases, which stub the worker spawn. +""" + +from __future__ import annotations + +import argparse +import contextlib +import json +import subprocess +import sys +import textwrap +import time +from pathlib import Path +from typing import Any, Callable + +import psutil +import pytest + +from agent import shell_hooks +from hermes_cli import kanban as kanban_cli +from hermes_cli import kanban_db as kb +from hermes_cli import kanban_db_connect as kbc +from hermes_cli import kanban_db_dispatch as kbd +from hermes_cli import kanban_db_workspace as kbw +from hermes_cli.plugins import get_plugin_manager + +HOOK = "kanban_worktree_created" + +# Mirrors ``_record_task_failure`` in hermes_cli/kanban_db_dispatch.py: the exception text is +# prefixed with ``workspace: `` by dispatch_once and cut to 500 chars before it lands on the card. +_CARD_REASON_LIMIT = 500 + + +# --------------------------------------------------------------------------- +# Fixtures / helpers +# --------------------------------------------------------------------------- + + +@pytest.fixture +def kanban_home(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """Isolated HERMES_HOME (no kanban DB needed — this is worktree plumbing).""" + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + return home + + +@pytest.fixture +def hook_registry(kanban_home: Path): + """Snapshot the plugin manager's hook table and the shell-hook idempotence set, restore both.""" + mgr = get_plugin_manager() + saved = {k: list(v) for k, v in mgr._hooks.items()} + shell_hooks.reset_for_tests() + try: + yield mgr + finally: + mgr._hooks = saved + shell_hooks.reset_for_tests() + + +def _subscribe(mgr, callback: Callable[..., Any]) -> None: + mgr._hooks.setdefault(HOOK, []).append(callback) + + +def _git(cwd: Path, *args: str) -> None: + subprocess.run( + [ + "git", "-C", str(cwd), + "-c", "user.name=Test User", + "-c", "user.email=test@example.com", + "-c", "commit.gpgsign=false", + *args, + ], + check=True, capture_output=True, text=True, timeout=60, + ) + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + """A one-commit git repo to hang linked worktrees off.""" + root = tmp_path / "repo" + root.mkdir() + subprocess.run( + ["git", "init", "-b", "main", str(root)], + check=True, capture_output=True, text=True, timeout=60, + ) + (root / "README.md").write_text("base\n", encoding="utf-8") + _git(root, "add", "README.md") + _git(root, "commit", "-m", "init") + return root + + +def _worktrees(repo_root: Path) -> list[str]: + out = subprocess.run( + ["git", "-C", str(repo_root), "worktree", "list", "--porcelain"], + capture_output=True, text=True, timeout=60, check=True, + ).stdout + return [ln.split(" ", 1)[1] for ln in out.splitlines() if ln.startswith("worktree ")] + + +def _install_shell_hook(tmp_path: Path, body: str, **entry: Any) -> Path: + """Write a Python hook script and register it exactly as ``config.yaml`` would. + + Python rather than ``/bin/sh`` so the same script runs on every host; the bridge splits the + command line itself and never goes through a shell. + """ + script = tmp_path / "worktree-created-hook.py" + script.write_text(textwrap.dedent(body), encoding="utf-8") + cfg = {"hooks": {HOOK: [{"command": f'"{sys.executable}" "{script}"', **entry}]}} + registered = shell_hooks.register_from_config(cfg, accept_hooks=True) + assert [s.event for s in registered] == [HOOK], "the hooks: entry must wire onto the event" + return script + + +def _card_reason(exc: BaseException) -> str: + return f"workspace: {exc}"[:_CARD_REASON_LIMIT] + + +# --------------------------------------------------------------------------- +# Plugin hook: payload, timing, idempotence +# --------------------------------------------------------------------------- + + +def test_hook_fires_once_after_checkout_with_worktree_repo_branch_and_task( + kanban_home: Path, repo: Path, hook_registry, +) -> None: + """Payload names the new worktree, its repo, its branch and the card; the checkout exists already.""" + seen: list[dict] = [] + + def on_created(**kw): + seen.append(dict(kw, readme_present=Path(kw["worktree_path"], "README.md").is_file())) + + _subscribe(hook_registry, on_created) + target = repo / ".worktrees" / "t_abc" + + kbw._ensure_git_worktree(repo, target, "wt/t_abc", task_id="t_abc") + + assert target.is_dir() + assert len(seen) == 1, f"hook fired {len(seen)} times, expected exactly once" + kw = seen[0] + assert Path(kw["worktree_path"]) == target + assert Path(kw["repo_root"]) == repo + assert kw["branch"] == "wt/t_abc" + assert kw["task_id"] == "t_abc" + assert kw["readme_present"], "the hook must run after the checkout is materialised, not before" + + +def test_existing_worktree_does_not_re_fire(kanban_home: Path, repo: Path, hook_registry) -> None: + """A *created* event: re-resolving an existing worktree takes the idempotent path in silence.""" + calls: list[str] = [] + _subscribe(hook_registry, lambda **kw: calls.append(kw["worktree_path"])) + target = repo / ".worktrees" / "t_once" + + kbw._ensure_git_worktree(repo, target, "wt/t_once", task_id="t_once") + kbw._ensure_git_worktree(repo, target, "wt/t_once", task_id="t_once") + + assert calls == [str(target)] + + +def test_no_subscriber_is_a_no_op(kanban_home: Path, repo: Path, hook_registry) -> None: + target = repo / ".worktrees" / "t_nohook" + + kbw._ensure_git_worktree(repo, target, "wt/t_nohook", task_id="t_nohook") + + assert (target / "README.md").is_file() + + +# --------------------------------------------------------------------------- +# Plugin hook: a block directive fails the card, atomically +# --------------------------------------------------------------------------- + + +def test_block_directive_raises_with_its_message_and_discards_the_worktree( + kanban_home: Path, repo: Path, hook_registry, +) -> None: + """``{"action": "block", "message"}`` raises with the message and leaves no worktree behind.""" + + def half_seed_then_block(worktree_path: str, **kw): + # Seed something first, so removal has to cope with a dirty/untracked tree — + # which is what a hook that failed halfway actually leaves. + Path(worktree_path, ".env").write_text("half-written\n", encoding="utf-8") + return {"action": "block", "message": "no .env for this project"} + + _subscribe(hook_registry, half_seed_then_block) + target = repo / ".worktrees" / "t_fail" + + with pytest.raises(RuntimeError) as excinfo: + kbw._ensure_git_worktree(repo, target, "wt/t_fail", task_id="t_fail") + + message = str(excinfo.value) + assert HOOK in message + assert "no .env for this project" in message + # The raise is about the seeding, not about git; it must not blame git. + assert "git worktree add failed" not in message + # Whether a retry is safe comes BEFORE the hook's own text (see the budget tests below). + assert message.index("was removed") < message.index("no .env for this project") + # Atomic: the unseeded tree is gone, and git no longer tracks it. + assert not target.exists() + assert str(target) not in _worktrees(repo) + + +def test_retry_after_a_block_re_creates_and_re_fires(kanban_home: Path, repo: Path, hook_registry) -> None: + """The whole point of discarding the tree: the next attempt re-fires the hook. + + ``dispatch_once`` records the raise as a spawn failure and releases the + claim, so the card comes back. If the blocked attempt had left its worktree + in place, this second call would take the idempotent early return, skip the + hook, and hand a worker the unseeded checkout with no error attached. + """ + calls: list[str] = [] + + def fail_once_then_seed(worktree_path: str, **kw): + calls.append(worktree_path) + if len(calls) == 1: + return {"action": "block", "message": "boom"} + Path(worktree_path, ".seeded").write_text("ok\n", encoding="utf-8") + return None + + _subscribe(hook_registry, fail_once_then_seed) + target = repo / ".worktrees" / "t_retry" + + with pytest.raises(RuntimeError, match=HOOK): + kbw._ensure_git_worktree(repo, target, "wt/t_retry", task_id="t_retry") + assert not target.exists() + + kbw._ensure_git_worktree(repo, target, "wt/t_retry", task_id="t_retry") + + assert calls == [str(target), str(target)] + assert (target / ".seeded").is_file() + assert (target / "README.md").is_file() # a real checkout, not a leftover shell + + +# --------------------------------------------------------------------------- +# The card reason: the retry verdict survives the 500-char cut in both branches +# --------------------------------------------------------------------------- + + +def test_card_reason_keeps_the_retry_verdict_ahead_of_a_chatty_hook( + kanban_home: Path, repo: Path, hook_registry, +) -> None: + """A 5000-char hook message cannot push "the worktree was removed" off the card.""" + _subscribe(hook_registry, lambda **kw: {"action": "block", "message": "x" * 5000}) + target = repo / ".worktrees" / "t_chatty" + + with pytest.raises(RuntimeError) as excinfo: + kbw._ensure_git_worktree(repo, target, "wt/t_chatty", task_id="t_chatty") + + reason = _card_reason(excinfo.value) + assert "was removed" in reason + assert "re-seeds it" in reason + assert not target.exists() + + +def test_card_reason_keeps_remove_by_hand_when_removal_itself_fails( + kanban_home: Path, repo: Path, hook_registry, monkeypatch: pytest.MonkeyPatch, +) -> None: + """If git cannot remove the tree, the operator instruction survives both the git noise and the hook's.""" + real_git = kbw._git + + def git_with_broken_remove(repo_root: Path, *args: str, timeout: int): + if args[:2] == ("worktree", "remove"): + return subprocess.CompletedProcess(args, 128, stdout="", stderr="fatal: " + "y" * 5000) + return real_git(repo_root, *args, timeout=timeout) + + monkeypatch.setattr(kbw, "_git", git_with_broken_remove) + _subscribe(hook_registry, lambda **kw: {"action": "block", "message": "x" * 5000}) + target = repo / ".worktrees" / "t_stuck" + + with pytest.raises(RuntimeError) as excinfo: + kbw._ensure_git_worktree(repo, target, "wt/t_stuck", task_id="t_stuck") + + reason = _card_reason(excinfo.value) + assert "could NOT be removed" in reason + assert "remove it by hand" in reason + assert str(target) in reason + assert target.is_dir() # the residual the message warns about is real + + +# --------------------------------------------------------------------------- +# Shell hook through the hooks: bridge — the same script an operator would install +# --------------------------------------------------------------------------- + + +def test_shell_hook_seeds_the_worktree_from_the_stdin_payload( + kanban_home: Path, repo: Path, hook_registry, tmp_path: Path, +) -> None: + """A ``hooks:`` entry gets the worktree in ``extra`` and can seed it; exit 0 lets the card proceed.""" + log = tmp_path / "payload.json" + _install_shell_hook( + tmp_path, + f""" + import json, pathlib, sys + payload = json.load(sys.stdin) + pathlib.Path({str(log)!r}).write_text(json.dumps(payload), encoding="utf-8") + pathlib.Path(payload["extra"]["worktree_path"], ".env").write_text("SECRET=1\\n", encoding="utf-8") + """, + timeout=30, + ) + target = repo / ".worktrees" / "t_shell" + + kbw._ensure_git_worktree(repo, target, "wt/t_shell", task_id="t_shell") + + assert (target / ".env").read_text(encoding="utf-8") == "SECRET=1\n" + payload = json.loads(log.read_text(encoding="utf-8")) + assert payload["hook_event_name"] == HOOK + extra = payload["extra"] + assert Path(extra["worktree_path"]) == target + assert Path(extra["repo_root"]) == repo + assert extra["branch"] == "wt/t_shell" + assert extra["task_id"] == "t_shell" + + +def test_shell_hook_exit_2_fails_the_card_with_its_stderr_and_discards_the_worktree( + kanban_home: Path, repo: Path, hook_registry, tmp_path: Path, +) -> None: + """Exit 2 is the bridge's block verb: the stderr line becomes the card reason, the tree is gone.""" + _install_shell_hook( + tmp_path, + """ + import sys + sys.stdin.read() + print("no .env for this project", file=sys.stderr) + sys.exit(2) + """, + timeout=30, + ) + target = repo / ".worktrees" / "t_shell_fail" + + with pytest.raises(RuntimeError) as excinfo: + kbw._ensure_git_worktree(repo, target, "wt/t_shell_fail", task_id="t_shell_fail") + + assert "no .env for this project" in str(excinfo.value) + assert not target.exists() + assert str(target) not in _worktrees(repo) + + +def test_shell_hook_timeout_with_fail_closed_fails_the_card_and_kills_the_hook_tree( + kanban_home: Path, repo: Path, hook_registry, tmp_path: Path, +) -> None: + """A hung hook fails the card instead of wedging the dispatcher, and its children die with it. + + The hook spawns a grandchild (an ``npm ci`` stand-in) and then hangs. Killing only the hook + would leave that grandchild writing into a worktree git has just force-removed, and the retry's + ``git worktree add`` would then fail on "already exists". + """ + pid_file = tmp_path / "grandchild.pid" + _install_shell_hook( + tmp_path, + f""" + import pathlib, subprocess, sys, time + sys.stdin.read() + child = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(60)"]) + pathlib.Path({str(pid_file)!r}).write_text(str(child.pid), encoding="utf-8") + time.sleep(60) + """, + # Floor is 1s; 3s leaves headroom for two CPython start-ups on a loaded runner before + # the pid file is read below. + timeout=3, + fail_closed=True, + ) + target = repo / ".worktrees" / "t_hang" + + with pytest.raises(RuntimeError) as excinfo: + kbw._ensure_git_worktree(repo, target, "wt/t_hang", task_id="t_hang") + + assert "timed out" in str(excinfo.value) + assert not target.exists() + assert str(target) not in _worktrees(repo) + grandchild = int(pid_file.read_text(encoding="utf-8")) + deadline = time.monotonic() + 5 + while psutil.pid_exists(grandchild) and time.monotonic() < deadline: + time.sleep(0.05) + assert not psutil.pid_exists(grandchild), "the hook's grandchild survived the timeout kill" + + +# --------------------------------------------------------------------------- +# CLI entry points: the hooks: block is wired before any of them creates a worktree +# --------------------------------------------------------------------------- +# +# ``hermes chat`` / ``gateway run`` / ``cron run`` register config.yaml's ``hooks:`` in +# ``_prepare_agent_startup`` (hermes_cli/main.py) and the gateway-hosted dispatcher in +# ``GatewayStartupMixin._register_config_hooks``. ``hermes kanban`` is not an agent command, +# so ``claim``, ``dispatch`` and ``daemon --force`` must wire the block themselves — otherwise a +# configured seed script silently never runs and the unseeded tree is reused by the next tick. + + +@pytest.fixture +def cli_home(kanban_home: Path, hook_registry, repo: Path, tmp_path: Path) -> Path: + """Real kanban DB plus a config.yaml whose hooks: block seeds ``.env`` into a new worktree.""" + kb.init_db() + script = tmp_path / "seed-worktree.py" + script.write_text(textwrap.dedent( + """ + import json, pathlib, sys + payload = json.load(sys.stdin) + pathlib.Path(payload["extra"]["worktree_path"], ".env").write_text("SEEDED=1\\n", encoding="utf-8") + """ + ), encoding="utf-8") + (kanban_home / "config.yaml").write_text(json.dumps({ + "hooks_auto_accept": True, # the headless consent channel the docs name + "hooks": {HOOK: [{"command": f'"{sys.executable}" "{script}"', "timeout": 30}]}, + }), encoding="utf-8") + return kanban_home + + +def _create_worktree_card(repo: Path, *, assignee: str | None = None) -> str: + with kbc.connect_closing() as conn: + return kb.create_task( + conn, title="needs .env", assignee=assignee, + workspace_kind="worktree", workspace_path=str(repo), + ) + + +def test_hermes_kanban_claim_registers_config_hooks_before_resolving_the_workspace( + cli_home: Path, repo: Path, capsys: pytest.CaptureFixture[str], +) -> None: + """``hermes kanban claim`` seeds through the ``hooks:`` block, not only through Python plugins.""" + tid = _create_worktree_card(repo) + + rc = kanban_cli._cmd_claim(argparse.Namespace(task_id=tid, ttl=600, accept_hooks=False)) + + assert rc == 0, capsys.readouterr() + worktree = repo / ".worktrees" / tid + assert (worktree / ".env").read_text(encoding="utf-8") == "SEEDED=1\n" + with kbc.connect_closing() as conn: + assert Path(kb.get_task(conn, tid).workspace_path) == worktree + + +def _stub_worker_spawn(monkeypatch: pytest.MonkeyPatch) -> list[str]: + """``hermes kanban dispatch/daemon`` spawn a real ``hermes chat`` worker; record the workspace instead.""" + spawned: list[str] = [] + monkeypatch.setattr(kbd, "_default_spawn", lambda task, workspace, *, board=None: spawned.append(workspace)) + return spawned + + +def test_hermes_kanban_dispatch_registers_config_hooks_before_dispatch_once( + cli_home: Path, repo: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], +) -> None: + """A one-shot ``hermes kanban dispatch`` hands the worker a worktree the ``hooks:`` script has seeded.""" + tid = _create_worktree_card(repo, assignee="default") + spawned = _stub_worker_spawn(monkeypatch) + + rc = kanban_cli._cmd_dispatch( + argparse.Namespace(dry_run=False, max=None, failure_limit=2, json=False, accept_hooks=False), + ) + + assert rc == 0, capsys.readouterr() + assert [Path(p) for p in spawned] == [repo / ".worktrees" / tid] + assert (repo / ".worktrees" / tid / ".env").read_text(encoding="utf-8") == "SEEDED=1\n" + + +def test_hermes_kanban_daemon_force_registers_config_hooks_before_the_loop( + cli_home: Path, repo: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], +) -> None: + """The standalone loop (``daemon --force``) ticks with the ``hooks:`` block already wired.""" + tid = _create_worktree_card(repo, assignee="default") + spawned = _stub_worker_spawn(monkeypatch) + + def one_tick(*, interval, max_spawn, failure_limit, on_tick): + # The body of ``kbd.run_daemon``'s loop, once, instead of sleeping until a signal. + with contextlib.closing(kbc.connect()) as conn: + on_tick(kbd.dispatch_once(conn, max_spawn=max_spawn, failure_limit=failure_limit)) + + monkeypatch.setattr(kbd, "run_daemon", one_tick) + + rc = kanban_cli._cmd_daemon(argparse.Namespace( + force=True, pidfile=None, interval=5, max=None, failure_limit=2, verbose=False, accept_hooks=False, + )) + + assert rc == 0, capsys.readouterr() + assert [Path(p) for p in spawned] == [repo / ".worktrees" / tid] + assert (repo / ".worktrees" / tid / ".env").read_text(encoding="utf-8") == "SEEDED=1\n" diff --git a/website/docs/developer-guide/plugins/index.md b/website/docs/developer-guide/plugins/index.md index 2833d5e32dc4b..27285ddeec229 100644 --- a/website/docs/developer-guide/plugins/index.md +++ b/website/docs/developer-guide/plugins/index.md @@ -1096,8 +1096,9 @@ Each hook is documented in full on the **[Event Hooks reference](../../user-guid | `kanban_task_claimed` | A kanban task is claimed (dispatcher process, before the worker spawns) | `task_id: str, board: str \| None, assignee: str \| None, run_id: int \| None, profile_name: str` | ignored | | `kanban_task_completed` | A kanban task completes (worker process) | `task_id, board, assignee, run_id, profile_name, summary: str \| None` | ignored | | `kanban_task_blocked` | A kanban task is blocked (worker process) | `task_id, board, assignee, run_id, profile_name, reason: str \| None` | ignored | +| [`kanban_worktree_created`](/user-guide/features/hooks#kanban_worktree_created) | A kanban worktree was just created, before the worker spawns (dispatcher process, or `hermes kanban claim`) | `task_id: str, board: str \| None, profile_name: str, worktree_path: str, repo_root: str, branch: str` | `{"action": "block", "message"}` removes the worktree and fails the card | -Most hooks are fire-and-forget observers — their return values are ignored. The exceptions are `pre_llm_call`, which can inject context into the conversation, and `pre_tool_call`, which can return a block/approve directive. +Most hooks are fire-and-forget observers — their return values are ignored. The exceptions are `pre_llm_call`, which can inject context into the conversation, `pre_tool_call`, which can return a block/approve directive, and `kanban_worktree_created`, whose block directive fails the card. All callbacks should accept `**kwargs` for forward compatibility. If a hook callback crashes, it's logged and skipped. Other hooks and the agent continue normally. diff --git a/website/docs/user-guide/features/hooks.md b/website/docs/user-guide/features/hooks.md index 59975d14edf93..855774f5b2fdd 100644 --- a/website/docs/user-guide/features/hooks.md +++ b/website/docs/user-guide/features/hooks.md @@ -484,6 +484,7 @@ Payload fields below are the exact event-specific fields supplied by each call s | `kanban_task_claimed` | Observer | After claim commit, in dispatcher process before worker spawn; return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id` | Board/task/profile/assignee identifiers. | | `kanban_task_completed` | Observer | After completion and cleanup, usually in worker process; return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `summary` | Summary may contain project/user content. | | `kanban_task_blocked` | Observer | After a blocked transition; the dependency-wait path fires before its transaction exits. Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `reason` | Reason may contain project/user content. | +| [`kanban_worktree_created`](#kanban_worktree_created) | Directive/control | Synchronously after a kanban `git worktree add` succeeds and before the worker spawns (dispatcher or `hermes kanban claim`); the first `{"action": "block", "message"}` removes the worktree and fails the card with the message. Not timeout-bounded. | `task_id`, `profile_name`, `board`, `worktree_path`, `repo_root`, `branch` | `worktree_path` / `repo_root` are filesystem paths and may reveal project layout or usernames. | | `on_kanban_worker_spawned` | Observer | After `spawn_fn` returns and the worker PID is persisted; runs inside the dispatch lock, keep callbacks fast. Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `worker_pid`, `workspace_path` | `workspace_path` is a filesystem path and may reveal project layout or usernames. | | `on_kanban_worker_exited` | Observer | Tick-derived: after `detect_crashed_workers` reclaims a dead-PID task and the reclaim commits. Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `worker_pid`, `exit_kind`, `exit_code`, `outcome`, `retry_status` | Identifiers and exit metadata only. | | `on_kanban_worker_stale_claim` | Observer | After a TTL-expired claim is reclaimed; live-PID extensions don't fire. Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `worker_pid`, `heartbeat_stale`, `retry_status` | Identifiers and claim metadata only. | @@ -1645,7 +1646,7 @@ Auxiliary LLM calls — session titling, context compression, MoA advisors and t Fires after an authoritative skill-usage state change. It is observer-only and exposes the local `skill_name`, provenance, correlation IDs, usage count, and reuse flags. -### Kanban lifecycle observers +### Kanban lifecycle hooks #### `kanban_task_claimed` @@ -1659,7 +1660,11 @@ Fires after completion and cleanup, usually in the worker process. Its `summary` Fires after a normal blocked transition. The dependency-wait path invokes it before that write transaction exits. Its `reason` can contain project or user content. -All three kanban hooks are observer-only and carry `task_id`, `profile_name`, `board`, `assignee`, and `run_id`; completed adds `summary`, and blocked adds `reason`. +The three hooks above are observer-only and carry `task_id`, `profile_name`, `board`, `assignee`, and `run_id`; completed adds `summary`, and blocked adds `reason`. The fourth, below, is a gate with its own payload. + +#### `kanban_worktree_created` + +The one kanban hook that is a **gate** rather than an observer. Fires synchronously — on the caller's thread, not timeout-bounded — right after the dispatcher (or `hermes kanban claim`) has run `git worktree add` for a `worktree` workspace and before any worker is spawned. Payload: `task_id`, `profile_name`, `board`, `worktree_path`, `repo_root`, `branch`. Return `None` to accept; return `{"action": "block", "message": "..."}` to have the caller remove the just-created worktree (`git worktree remove --force`, branch kept) and fail the card with the message — the dispatcher then releases the claim and the retry re-creates the tree and fires again. A callback that raises is logged and skipped like any other hook, so return the directive. From a shell hook the block is exit code **2** (stderr becomes the message), or a timeout / spawn failure under `fail_closed: true`. Use case and a worked script: [seeding a worktree workspace](/user-guide/features/kanban#kanban-worktree-created-hook). ### Kanban worker-lifecycle, task-mutation, and dispatch observers @@ -1686,7 +1691,7 @@ Use shell hooks when you want a drop-in, single-file script (Bash, Python, anyth - **Inject context into the next LLM turn** — prepend `git status` output, the current weekday, or retrieved documents to the user message (see [`pre_llm_call`](#pre_llm_call)). - **Observe lifecycle events** — write a log line when a subagent completes (`subagent_stop`) or a session starts (`on_session_start`). -Shell hooks are registered by calling `agent.shell_hooks.register_from_config(cfg)` at both CLI startup (`hermes_cli/main.py`) and gateway startup (`gateway/run.py`). They compose naturally with Python plugin hooks — both flow through the same dispatcher. +Shell hooks are registered by calling `agent.shell_hooks.register_from_config(cfg)` at CLI startup (`hermes_cli/main.py`, for the commands that run an agent turn), at gateway startup (`gateway/run.py`, which also hosts the kanban dispatcher), and by the standalone kanban entry points that create workspaces — `hermes kanban claim`, `hermes kanban dispatch` and `hermes kanban daemon --force` (`hermes_cli/kanban_ops.py`). Other `hermes` subcommands do not register them. They compose naturally with Python plugin hooks — both flow through the same dispatcher. ### Comparison at a glance @@ -1710,13 +1715,14 @@ hooks: - matcher: "" # Optional; used for pre/post_tool_call only command: "" # Required; runs via shlex.split, shell=False timeout: # Optional; default 60, capped at 300 - fail_closed: # Optional; default false. pre_tool_call only. + fail_closed: # Optional; default false. Blocking-capable events only + # (pre_tool_call, kanban_worktree_created). # `failClosed` also accepted (Cursor/Claude Code compat) hooks_auto_accept: false # See "Consent model" below ``` -Event names must be one of the [plugin hook events](#plugin-hooks); typos produce a "Did you mean X?" warning and are skipped. Unknown keys inside a single entry are ignored; missing `command` is a skip-with-warning. `timeout > 300` is clamped with a warning. `fail_closed: true` on an event other than `pre_tool_call` warns and is ignored (only blocking-capable events can fail closed). +Event names must be one of the [plugin hook events](#plugin-hooks); typos produce a "Did you mean X?" warning and are skipped. Unknown keys inside a single entry are ignored; missing `command` is a skip-with-warning. `timeout > 300` is clamped with a warning. `fail_closed: true` on an event that cannot block (anything but `pre_tool_call` / `kanban_worktree_created`) warns and is ignored. On Windows, a `command` that starts with an existing script file — the `~/.hermes/agent-hooks/x.sh` shape the examples below use — is spawned through that file's own interpreter (Git Bash for `.sh`/`.bash`, the running Hermes Python for `.py`), because `CreateProcess` has no shebang support and rejects a bare script with `WinError 193`. Every other command, and every POSIX platform, passes `argv` straight to `Popen`, where the kernel already honours the shebang. @@ -1771,7 +1777,7 @@ Malformed JSON, non-zero exit codes, and timeouts log a warning but never abort ### Exit code 2 = block (Claude Code / Cursor compatible) -A `pre_tool_call` hook that exits with code **2** blocks the tool call even when its stdout carries no block JSON. The block message is resolved in priority order: +A hook on a blocking-capable event (`pre_tool_call`, `kanban_worktree_created`) that exits with code **2** blocks the action — the tool call, or the just-created kanban worktree — even when its stdout carries no block JSON. The block message is resolved in priority order: 1. stdout block JSON (`reason` / `message`), when present; 2. the first 400 characters of stderr; @@ -1785,7 +1791,7 @@ echo "policy violation: rm -rf is not permitted" >&2 exit 2 ``` -For events whose block directive is not honored (everything except `pre_tool_call`), exit 2 is treated like any other non-zero exit: a warning is logged and stdout is still parsed. +For events whose block directive is not honored (everything except `pre_tool_call` and `kanban_worktree_created`), exit 2 is treated like any other non-zero exit: a warning is logged and stdout is still parsed. ### Fail-open vs fail-closed @@ -1802,7 +1808,7 @@ hooks: fail_closed: true ``` -With `fail_closed: true`, each of these now **blocks** the tool call with `hook failed closed: `: +With `fail_closed: true`, each of these now **blocks** the action with `hook failed closed: `: | Failure | Fail-open (default) | `fail_closed: true` | |---------|--------------------|--------------------| @@ -1811,7 +1817,7 @@ With `fail_closed: true`, each of these now **blocks** the tool call with `hook | Non-JSON stdout (e.g. a stack trace) | warn, proceed | **block** | | Clean exit, valid no-op JSON (`{}`) | proceed | proceed | -`fail_closed` only applies to blocking-capable events (`pre_tool_call` today); setting it on any other event logs a warning at config-parse time and is ignored. `hermes hooks test` reflects these semantics — the `parsed` line shows exactly the block shape the dispatcher would receive. +`fail_closed` only applies to blocking-capable events (`pre_tool_call` and `kanban_worktree_created`); setting it on any other event logs a warning at config-parse time and is ignored. `hermes hooks test` reflects these semantics — the `parsed` line shows exactly the block shape the dispatcher would receive. ### Worked examples diff --git a/website/docs/user-guide/features/kanban.md b/website/docs/user-guide/features/kanban.md index 0363bbd6c8d5c..ed22bb3e18ca6 100644 --- a/website/docs/user-guide/features/kanban.md +++ b/website/docs/user-guide/features/kanban.md @@ -122,7 +122,7 @@ They coexist: a kanban worker may call `delegate_task` internally during its run - **Workspace** — the directory a worker operates in. Three kinds: - `scratch` (default) — fresh tmp dir under `~/.hermes/kanban/workspaces//` (or `~/.hermes/kanban/boards//workspaces//` on non-default boards). **Deleted when the task completes** — scratch is ephemeral by design. Files explicitly declared through `kanban_complete(artifacts=[...])` or `kanban_request_review(artifacts=[...])` are copied into durable per-task attachment storage before cleanup (a review handoff stages them at handoff time, since the reviewer's later completion is what deletes the scratch workspace); existing deliverable paths in legacy completion summaries receive the same treatment. Other scratch files are removed. A missing declared scratch artifact keeps the task in-flight so the worker can correct the path and retry. Use `worktree:` or `dir:` when the whole workspace should remain available. The first time a scratch workspace is created on an install, the dispatcher logs a warning and emits a `tip_scratch_workspace` event on the task (visible via `hermes kanban show `). - `dir:` — an existing shared directory (Obsidian vault, mail ops dir, per-account folder). **Must be an absolute path.** Relative paths like `dir:../tenants/foo/` are rejected at dispatch because they'd resolve against whatever CWD the dispatcher happens to be in, which is ambiguous and a confused-deputy escape vector. The path is otherwise trusted — it's your box, your filesystem, the worker runs with your uid. This is the trusted-local-user threat model; kanban is single-host by design. **Preserved on completion.** - - `worktree` — a git worktree under `.worktrees//` for coding tasks. Use `worktree:` to pin the exact target path. Worker-side `git worktree add` creates it, using `--branch` when provided. **Preserved on completion.** + - `worktree` — a git worktree under `.worktrees//` for coding tasks. Use `worktree:` to pin the exact target path. The dispatcher's `git worktree add` creates it, using `--branch` when provided. **Preserved on completion.** A fresh checkout carries no gitignored files, so a project that needs its `.env` should subscribe to the [`kanban_worktree_created` hook](#kanban-worktree-created-hook). - **Dispatcher** — a long-lived loop that, every N seconds (default 60): reclaims stale claims, reclaims crashed workers (PID gone but TTL not yet expired), reaps workers that outlived their finished run (a worker still alive after its own `kanban_complete`/`kanban_block` is terminated once its run has been closed for two minutes, leaving it time to finish its final turn — matched by PID *and* spawn-time fingerprint, so a recycled PID is never signalled; recorded as a `terminal_worker_reaped` event), promotes ready tasks, atomically claims, spawns assigned profiles. Runs **inside the gateway** by default (`kanban.dispatch_in_gateway: true`). One dispatcher sweeps all boards per tick; workers are spawned with `HERMES_KANBAN_BOARD` pinned so they can't see other boards. After `kanban.failure_limit` consecutive spawn failures on the same task (default: 2) the dispatcher auto-blocks it with the last error as the reason — prevents thrashing on tasks whose profile doesn't exist, workspace can't mount, etc. - **Tenant** — optional string namespace *within* a board. One specialist fleet can serve multiple businesses (`--tenant business-a`) with data isolation by workspace path and memory key prefix. Tenants are a soft filter; boards are the hard isolation boundary. @@ -699,6 +699,47 @@ def register(ctx): ctx.register_hook("kanban_task_blocked", on_blocked) ``` +### Seeding a worktree workspace (`kanban_worktree_created`) {#kanban-worktree-created-hook} + +A linked worktree is a fresh checkout: it holds **tracked files and nothing else**, so everything git ignores is absent by definition — and for most real projects that includes the one file the project cannot start without (`.env`, `.env.dev`, `local.settings.json`). The worker lands in a repo that looks complete and fails at the first command that reads configuration, with an error about the *application* rather than about the *checkout*. + +Which secrets a project needs, and where they are kept, is deployment knowledge, so there is a hook for it. Right after `git worktree add` succeeds — and before any worker is spawned — the dispatcher (or `hermes kanban claim`) fires the `kanban_worktree_created` [plugin hook](/user-guide/features/hooks#plugin-hooks) with `task_id`, `board`, `profile_name`, `worktree_path`, `repo_root` and `branch`. Unlike the observers above it is a **gate**: return `None` to accept the worktree, or `{"action": "block", "message": "..."}` to fail the card. + +The shortest way to use it is a shell script wired through the same [`hooks:` block](/user-guide/features/hooks#shell-hooks) every other event uses — no plugin needed: + +```yaml +# ~/.hermes/config.yaml (the dispatcher's profile) +hooks: + kanban_worktree_created: + - command: "~/.hermes/agent-hooks/seed-worktree.sh" + timeout: 120 # npm ci or decrypting a bundle takes a while; max 300 + fail_closed: true # a hook that hangs or cannot start must fail the card, not pass it +``` + +```bash +#!/usr/bin/env bash +# ~/.hermes/agent-hooks/seed-worktree.sh — the payload arrives as JSON on stdin +payload="$(cat -)" +worktree=$(jq -r '.extra.worktree_path' <<<"$payload") +repo=$(jq -r '.extra.repo_root' <<<"$payload") +found=0 +for f in "$repo"/.env*; do + [ -e "$f" ] || continue + ln -s "$f" "$worktree/$(basename "$f")" && found=1 +done +[ "$found" = 1 ] || { echo "no .env* in $repo — nothing to seed" >&2; exit 2; } +``` + +Approve the `(event, command)` pair once — at the TTY prompt the first time you run `hermes` (or `hermes kanban claim`), or with `hooks_auto_accept: true` / `HERMES_ACCEPT_HOOKS=1` for a gateway that has no TTY — and inspect it with `hermes hooks list`, `hermes hooks doctor` or `hermes hooks test kanban_worktree_created`, which all know the event and its payload. `doctor` and `test` run the script against a synthetic payload (`repo_root: /tmp/repo`), so the script above exits 2 there and doctor reports the block it returned — that is the gate working, not a broken hook. + +- **No subscriber → nothing changes.** Without a plugin or a `hooks:` entry the worktree is created exactly as before. +- **It fires once per creation**, not on every dispatch: re-resolving an existing worktree takes the idempotent path and does not fire. +- **A block fails the card, atomically.** On `{"action": "block"}` — for a shell hook: exit **2**, or with `fail_closed: true` a timeout or an unstartable command — the just-created worktree is **removed** (`git worktree remove --force`; the branch is kept) and the card fails with the hook's message (for exit 2: its stderr) in the reason. Removal is the point, not tidiness: the dispatcher releases the claim and retries a card whose workspace failed, and a worktree left behind would make that retry skip both `git worktree add` and the hook, handing the worker the unseeded checkout with no error attached. Because the tree is gone, the retry re-creates it and re-fires the hook. If the removal itself fails, the reason says so and names the worktree path — remove it by hand before the card retries, or that retry will spawn into the unseeded tree. +- **Only a block fails the card.** A shell hook that exits with any other non-zero code is logged as a warning and the card proceeds (the bridge's normal fail-open rule), so make the failing path `exit 2`, and set `fail_closed: true` so a hang or a missing script cannot pass an unseeded worktree either. A Python callback that *raises* is likewise logged and skipped — return the directive instead. If a step is genuinely optional, exit 0 and write the warning to stderr (kept in the debug log). +- **A timeout kills the whole hook process tree**, so an `npm ci` the hook started cannot keep writing into a worktree that has just been removed. +- **Every path that creates the worktree wires the `hooks:` block first.** The gateway-hosted dispatcher registers it at gateway startup; `hermes kanban dispatch`, `hermes kanban daemon --force` and `hermes kanban claim` register it themselves before resolving the workspace, with the same consent rules. Python plugin hooks fire on all of these regardless. +- **`hermes kanban claim`** resolves the workspace the same way and fires the same hook, but there is no dispatcher to release the claim: a block there leaves the card claimed with no workspace until the claim TTL expires. + ### Goal-mode cards (`--goal`) By default each worker gets **one shot** at its card — do the work, call `kanban_complete`/`kanban_block`, exit. Pass `--goal` (CLI) or `goal_mode=True` (the `kanban_create` tool / dashboard) to instead run that worker in a **goal loop**, the same Ralph-style engine behind the `/goal` slash command: after every turn an auxiliary judge checks the worker's output against the card's title + body (treated as the acceptance criteria), and if the work isn't done — and the turn budget remains — the worker keeps going **in the same session** until the judge agrees, the worker terminates the task itself, or the budget runs out (which **blocks** the card for human review rather than exiting silently). If the judge rules the goal **unachievable** as written, the card is blocked immediately with the judge's reason — an impossible card is never marked done, and `kanban complete` / `kanban request-review` on such a card are rejected with a pointer to `kanban block` or re-scoping. diff --git a/website/docs/user-guide/features/plugins.md b/website/docs/user-guide/features/plugins.md index 902b6f35b95bf..7c601cd57c089 100644 --- a/website/docs/user-guide/features/plugins.md +++ b/website/docs/user-guide/features/plugins.md @@ -317,11 +317,11 @@ When you upgrade to a version of Hermes that has opt-in plugins (config schema v ## Available hooks -Plugins can register the 27 lifecycle events currently accepted by `hermes_cli.plugins.VALID_HOOKS`. The **[Event Hooks catalog](./hooks.md#shipped-plugin-hook-catalog)** is canonical for exact timing, return handling, payload fields, and privacy notes. +Plugins can register the lifecycle events currently accepted by `hermes_cli.plugins.VALID_HOOKS`. The **[Event Hooks catalog](./hooks.md#shipped-plugin-hook-catalog)** is canonical for exact timing, return handling, payload fields, and privacy notes. | Descriptive category | Shipped hooks | |---|---| -| **Directive/control** | `pre_tool_call`, `pre_llm_call`, `pre_verify`, `pre_gateway_dispatch` | +| **Directive/control** | `pre_tool_call`, `pre_llm_call`, `pre_verify`, `pre_gateway_dispatch`, `kanban_worktree_created` | | **Transform** | `transform_tool_result`, `transform_terminal_output`, `transform_llm_output`, `pre_transcription` | | **Observer** | `post_tool_call`, `post_llm_call`, `pre_api_request`, `post_api_request`, `api_request_error`, `pre_auxiliary_call`, `post_auxiliary_call`, `on_stream_start`, `on_stream_delta`, `on_stream_end`, `on_interim_message`, `on_session_start`, `on_session_end`, `on_session_finalize`, `on_session_reset`, `agent_loop_stopped`, `on_skill_lifecycle`, `subagent_start`, `subagent_stop`, `pre_approval_request`, `post_approval_response`, `pre_command`, `kanban_task_claimed`, `kanban_task_completed`, `kanban_task_blocked` |