From 1733cb3a13bb726c496412e5e404ec72c132d13d Mon Sep 17 00:00:00 2001 From: aqilaziz <46887634+aqilaziz@users.noreply.github.com> Date: Mon, 18 May 2026 21:33:02 -0700 Subject: [PATCH] feat(kanban): configure worktree paths and branches Salvages #26496 by @aqilaziz. Adds branch_name column + CLI flag so tasks with workspace_kind='worktree' can pin a target branch on create. Schema migration added to _migrate_add_optional_columns. - Task.branch_name field + DB column + migration - create_task accepts branch_name kwarg - hermes kanban create --branch flag - kanban show output includes 'Branch: ' when set Cherry-picked the substantive commit (a7558cf27); the PR's tip was an unrelated service-path-dirs commit. Resolved 2 INSERT-column-list and show-output conflicts alongside main's session_id and max_runtime_seconds additions; kept all three. --- hermes_cli/kanban.py | 59 ++++++++++++++++++---- hermes_cli/kanban_db.py | 22 ++++++-- skills/devops/kanban-worker/SKILL.md | 2 +- tests/hermes_cli/test_kanban_cli.py | 39 +++++++++++++- tests/hermes_cli/test_kanban_db.py | 36 ++++++++++++- website/docs/user-guide/features/kanban.md | 5 +- 6 files changed, 145 insertions(+), 18 deletions(-) diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 12e62c2c7db..c9346fd560c 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -66,6 +66,7 @@ def _task_to_dict(t: kb.Task) -> dict[str, Any]: "tenant": t.tenant, "workspace_kind": t.workspace_kind, "workspace_path": t.workspace_path, + "branch_name": t.branch_name, "created_by": t.created_by, "created_at": t.created_at, "started_at": t.started_at, @@ -92,25 +93,42 @@ def _run_state_kwargs(args: argparse.Namespace) -> Optional[dict[str, str]]: def _parse_workspace_flag(value: str) -> tuple[str, Optional[str]]: """Parse ``--workspace`` into ``(kind, path|None)``. - Accepts: ``scratch``, ``worktree``, ``dir:``. + Accepts: ``scratch``, ``worktree``, ``worktree:``, ``dir:``. """ if not value: return ("scratch", None) v = value.strip() if v in {"scratch", "worktree"}: return (v, None) - if v.startswith("dir:"): - path = v[len("dir:"):].strip() + for prefix, kind in (("dir:", "dir"), ("worktree:", "worktree")): + if not v.startswith(prefix): + continue + path = v[len(prefix):].strip() if not path: raise argparse.ArgumentTypeError( - "--workspace dir: requires a path after the colon" + f"--workspace {prefix} requires a path after the colon" ) - return ("dir", os.path.expanduser(path)) + return (kind, os.path.expanduser(path)) raise argparse.ArgumentTypeError( - f"unknown --workspace value {value!r}: use scratch, worktree, or dir:" + f"unknown --workspace value {value!r}: use scratch, worktree, " + "worktree:, or dir:" ) +def _parse_branch_flag(value: Optional[str]) -> Optional[str]: + """Normalize an optional branch name from ``kanban create --branch``.""" + if value is None: + return None + branch = value.strip() + if not branch: + raise argparse.ArgumentTypeError("--branch requires a non-empty name") + if branch.startswith("-"): + raise argparse.ArgumentTypeError("--branch must not start with '-'") + if any(ch.isspace() for ch in branch): + raise argparse.ArgumentTypeError("--branch must not contain whitespace") + return branch + + def _check_dispatcher_presence() -> tuple[bool, str]: """Return ``(running, message)``. @@ -290,7 +308,10 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu p_create.add_argument("--parent", action="append", default=[], help="Parent task id (repeatable)") p_create.add_argument("--workspace", default="scratch", - help="scratch | worktree | dir: (default: scratch)") + help="scratch | worktree | worktree: | dir: " + "(default: scratch)") + p_create.add_argument("--branch", default=None, + help="Branch name for worktree tasks, e.g. wt/t6-wire") p_create.add_argument("--tenant", default=None, help="Tenant namespace") p_create.add_argument("--priority", type=int, default=0, help="Priority tiebreaker") p_create.add_argument("--triage", action="store_true", @@ -1235,7 +1256,15 @@ def _cmd_assignees(args: argparse.Namespace) -> int: def _cmd_create(args: argparse.Namespace) -> int: - ws_kind, ws_path = _parse_workspace_flag(args.workspace) + try: + ws_kind, ws_path = _parse_workspace_flag(args.workspace) + branch_name = _parse_branch_flag(getattr(args, "branch", None)) + except argparse.ArgumentTypeError as exc: + print(f"kanban: {exc}", file=sys.stderr) + return 2 + if branch_name and ws_kind != "worktree": + print("kanban: --branch is only valid with --workspace worktree", file=sys.stderr) + return 2 try: max_runtime = _parse_duration(getattr(args, "max_runtime", None)) except ValueError as exc: @@ -1258,6 +1287,7 @@ def _cmd_create(args: argparse.Namespace) -> int: created_by=args.created_by or _profile_author(), workspace_kind=ws_kind, workspace_path=ws_path, + branch_name=branch_name, tenant=args.tenant, priority=args.priority, parents=tuple(args.parent or ()), @@ -1434,6 +1464,8 @@ def _cmd_show(args: argparse.Namespace) -> int: print(f" tenant: {task.tenant}") print(f" workspace: {task.workspace_kind}" + (f" @ {task.workspace_path}" if task.workspace_path else "")) + if task.branch_name: + print(f" branch: {task.branch_name}") if task.skills: print(f" skills: {', '.join(task.skills)}") if task.model_override: @@ -2572,6 +2604,15 @@ def run_slash(rest: str) -> str: _choice.prog = f"/kanban {_name}" _choice.exit_on_error = False # type: ignore[attr-defined] + def _usage_for_error() -> str: + if tokens: + for _action in kanban_parser._actions: + if isinstance(_action, argparse._SubParsersAction): + subparser = _action.choices.get(tokens[0]) + if subparser is not None: + return subparser.format_usage().rstrip() + return kanban_parser.format_usage().rstrip() + buf_out = io.StringIO() buf_err = io.StringIO() # ``-h`` / ``--help`` makes argparse print to stdout and SystemExit(0). @@ -2589,7 +2630,7 @@ def run_slash(rest: str) -> str: body = err or out return f"⚠ /kanban usage error\n{body}" if body else "⚠ /kanban usage error" except argparse.ArgumentError as exc: - return f"⚠ /kanban usage error: {exc}" + return f"⚠ /kanban usage error\n{_usage_for_error()}\n{exc}" with contextlib.redirect_stdout(buf_out), contextlib.redirect_stderr(buf_err): try: diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 80e8259a3b4..eb20e342794 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -616,6 +616,7 @@ class Task: claim_lock: Optional[str] claim_expires: Optional[int] tenant: Optional[str] + branch_name: Optional[str] = None result: Optional[str] = None idempotency_key: Optional[str] = None # Unified non-success counter. Incremented on any of: @@ -681,6 +682,7 @@ class Task: completed_at=row["completed_at"], workspace_kind=row["workspace_kind"], workspace_path=row["workspace_path"], + branch_name=row["branch_name"] if "branch_name" in keys else None, claim_lock=row["claim_lock"], claim_expires=row["claim_expires"], tenant=row["tenant"] if "tenant" in keys else None, @@ -817,6 +819,7 @@ CREATE TABLE IF NOT EXISTS tasks ( completed_at INTEGER, workspace_kind TEXT NOT NULL DEFAULT 'scratch', workspace_path TEXT, + branch_name TEXT, claim_lock TEXT, claim_expires INTEGER, tenant TEXT, @@ -1074,6 +1077,8 @@ def _migrate_add_optional_columns(conn: sqlite3.Connection) -> None: _add_column_if_missing(conn, "tasks", "tenant", "tenant TEXT") if "result" not in cols: _add_column_if_missing(conn, "tasks", "result", "result TEXT") + if "branch_name" not in cols: + _add_column_if_missing(conn, "tasks", "branch_name", "branch_name TEXT") if "idempotency_key" not in cols: _add_column_if_missing( conn, "tasks", "idempotency_key", "idempotency_key TEXT" @@ -1334,6 +1339,7 @@ def create_task( created_by: Optional[str] = None, workspace_kind: str = "scratch", workspace_path: Optional[str] = None, + branch_name: Optional[str] = None, tenant: Optional[str] = None, priority: int = 0, parents: Iterable[str] = (), @@ -1382,6 +1388,10 @@ def create_task( f"workspace_kind must be one of {sorted(VALID_WORKSPACE_KINDS)}, " f"got {workspace_kind!r}" ) + if branch_name is not None: + branch_name = str(branch_name).strip() or None + if branch_name and workspace_kind != "worktree": + raise ValueError("branch_name is only valid for worktree workspaces") parents = tuple(p for p in parents if p) # Normalise + validate skills: strip whitespace, drop empties, dedupe @@ -1497,9 +1507,9 @@ def create_task( INSERT INTO tasks ( id, title, body, assignee, status, priority, created_by, created_at, workspace_kind, workspace_path, - tenant, idempotency_key, max_runtime_seconds, skills, - max_retries, session_id - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + branch_name, tenant, idempotency_key, max_runtime_seconds, + skills, max_retries, session_id + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) """, ( task_id, @@ -1512,6 +1522,7 @@ def create_task( now, workspace_kind, workspace_path, + branch_name, tenant, idempotency_key, int(max_runtime_seconds) if max_runtime_seconds is not None else None, @@ -1534,6 +1545,7 @@ def create_task( "status": task_status, "parents": list(parents), "tenant": tenant, + "branch_name": branch_name, "skills": list(skills_list) if skills_list else None, }, ) @@ -5129,6 +5141,8 @@ def _default_spawn( env["HERMES_TENANT"] = task.tenant env["HERMES_KANBAN_TASK"] = task.id env["HERMES_KANBAN_WORKSPACE"] = workspace + if task.branch_name: + env["HERMES_KANBAN_BRANCH"] = task.branch_name if task.current_run_id is not None: env["HERMES_KANBAN_RUN_ID"] = str(task.current_run_id) if task.claim_lock: @@ -5359,6 +5373,8 @@ def build_worker_context(conn: sqlite3.Connection, task_id: str) -> str: lines.append(f"Max runtime: {task.max_runtime_seconds}s") if effective_terminal_timeout: lines.append(f"Terminal timeout: {effective_terminal_timeout}s") + if task.branch_name: + lines.append(f"Branch: {task.branch_name}") lines.append("") if task.body and task.body.strip(): diff --git a/skills/devops/kanban-worker/SKILL.md b/skills/devops/kanban-worker/SKILL.md index cdbd574ad16..4954e6dc9dd 100644 --- a/skills/devops/kanban-worker/SKILL.md +++ b/skills/devops/kanban-worker/SKILL.md @@ -21,7 +21,7 @@ Your workspace kind determines how you should behave inside `$HERMES_KANBAN_WORK |---|---|---| | `scratch` | Fresh tmp dir, yours alone | Read/write freely; it gets GC'd when the task is archived. | | `dir:` | Shared persistent directory | Other runs will read what you write. Treat it like long-lived state. Path is guaranteed absolute (the kernel rejects relative paths). | -| `worktree` | Git worktree at the resolved path | If `.git` doesn't exist, run `git worktree add ` from the main repo first, then cd and work normally. Commit work here. | +| `worktree` | Git worktree at the resolved path | If `.git` doesn't exist, run `git worktree add ${HERMES_KANBAN_BRANCH:-wt/$HERMES_KANBAN_TASK}` from the main repo first, then cd and work normally. Commit work here. | ## Tenant isolation diff --git a/tests/hermes_cli/test_kanban_cli.py b/tests/hermes_cli/test_kanban_cli.py index e67bb94bdb7..fd9b1572513 100644 --- a/tests/hermes_cli/test_kanban_cli.py +++ b/tests/hermes_cli/test_kanban_cli.py @@ -32,6 +32,7 @@ def kanban_home(tmp_path, monkeypatch): [ ("scratch", ("scratch", None)), ("worktree", ("worktree", None)), + ("worktree:/tmp/wt", ("worktree", "/tmp/wt")), ("dir:/tmp/work", ("dir", "/tmp/work")), ], ) @@ -45,8 +46,12 @@ def test_parse_workspace_flag_expands_user(): assert path.endswith("/vault") assert not path.startswith("~") + kind, path = kc._parse_workspace_flag("worktree:~/trees/t6-wire") + assert kind == "worktree" + assert path.endswith("/trees/t6-wire") + assert not path.startswith("~") -@pytest.mark.parametrize("bad", ["cloud", "dir:", "", "worktree:/x"]) +@pytest.mark.parametrize("bad", ["cloud", "dir:", "worktree:", ""]) def test_parse_workspace_flag_rejects(bad): if not bad: # Empty -> defaults; not an error. @@ -56,6 +61,17 @@ def test_parse_workspace_flag_rejects(bad): kc._parse_workspace_flag(bad) +def test_parse_branch_flag_rejects_empty_and_option_like(): + assert kc._parse_branch_flag(None) is None + assert kc._parse_branch_flag(" wt/t6-wire ") == "wt/t6-wire" + with pytest.raises(argparse.ArgumentTypeError): + kc._parse_branch_flag(" ") + with pytest.raises(argparse.ArgumentTypeError): + kc._parse_branch_flag("-bad") + with pytest.raises(argparse.ArgumentTypeError): + kc._parse_branch_flag("bad branch") + + # --------------------------------------------------------------------------- # run_slash smoke tests (end-to-end via the same entry both CLI and gateway use) # --------------------------------------------------------------------------- @@ -74,6 +90,27 @@ def test_run_slash_create_and_list(kanban_home): assert "alice" in out +def test_run_slash_create_worktree_path_and_branch(kanban_home, tmp_path): + target = tmp_path / ".worktrees" / "t6-wire" + target_arg = target.as_posix() + out = kc.run_slash( + f"create 'ship worktree' --workspace worktree:{target_arg} --branch wt/t6-wire" + ) + assert "Created" in out + + with kb.connect() as conn: + tasks = kb.list_tasks(conn) + task = tasks[0] + assert task.workspace_kind == "worktree" + assert task.workspace_path == target_arg + assert task.branch_name == "wt/t6-wire" + + +def test_run_slash_rejects_branch_without_worktree(kanban_home): + out = kc.run_slash("create 'bad branch' --workspace scratch --branch wt/bad") + assert "--branch is only valid with --workspace worktree" in out + + def test_run_slash_create_with_parent_and_cascade(kanban_home): # Parent then child via --parent out1 = kc.run_slash("create 'parent' --assignee alice") diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index 15c0767d2cc..1f405bcce4c 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -81,6 +81,35 @@ def test_workspace_kind_validation(kanban_home): kb.create_task(conn, title="bad ws", workspace_kind="cloud") +def test_create_task_persists_worktree_branch_name(kanban_home, tmp_path): + target = tmp_path / ".worktrees" / "t6-wire" + with kb.connect() as conn: + tid = kb.create_task( + conn, + title="ship worktree", + workspace_kind="worktree", + workspace_path=str(target), + branch_name=" wt/t6-wire ", + ) + task = kb.get_task(conn, tid) + events = kb.list_events(conn, tid) + context = kb.build_worker_context(conn, tid) + + assert task.branch_name == "wt/t6-wire" + assert events[0].payload["branch_name"] == "wt/t6-wire" + assert "Branch: wt/t6-wire" in context + + +def test_branch_name_requires_worktree_workspace(kanban_home): + with kb.connect() as conn, pytest.raises(ValueError, match="worktree"): + kb.create_task( + conn, + title="bad branch", + workspace_kind="scratch", + branch_name="wt/bad", + ) + + # --------------------------------------------------------------------------- # Links + dependency resolution # --------------------------------------------------------------------------- @@ -1654,11 +1683,12 @@ class TestSharedBoardPaths: created_at=0, started_at=None, completed_at=None, - workspace_kind="scratch", - workspace_path=None, + workspace_kind="worktree", + workspace_path=str(tmp_path / "ws"), claim_lock=None, claim_expires=None, tenant=None, + branch_name="wt/t_dispatch_env", ) kb._default_spawn(task, str(tmp_path / "ws")) @@ -1668,6 +1698,7 @@ class TestSharedBoardPaths: default_home / "kanban" / "workspaces" ) assert env["HERMES_KANBAN_TASK"] == "t_dispatch_env" + assert env["HERMES_KANBAN_BRANCH"] == "wt/t_dispatch_env" # --------------------------------------------------------------------------- @@ -1907,6 +1938,7 @@ def test_migrate_add_optional_columns_tolerates_concurrent_migration(kanban_home tenant TEXT, result TEXT, idempotency_key TEXT, + branch_name TEXT, consecutive_failures INTEGER NOT NULL DEFAULT 0, worker_pid INTEGER, last_failure_error TEXT, diff --git a/website/docs/user-guide/features/kanban.md b/website/docs/user-guide/features/kanban.md index 2f81b621e0f..084e096529e 100644 --- a/website/docs/user-guide/features/kanban.md +++ b/website/docs/user-guide/features/kanban.md @@ -65,7 +65,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). - `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. - - `worktree` — a git worktree under `.worktrees//` for coding tasks. Worker-side `git worktree add` creates it. + - `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. - **Dispatcher** — a long-lived loop that, every N seconds (default 60): reclaims stale claims, reclaims crashed workers (PID gone but TTL not yet expired), 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. @@ -597,7 +597,8 @@ This is the surface **you** (or scripts, cron, the dashboard) use to drive the b hermes kanban init # create kanban.db + print daemon hint hermes kanban create "" [--body ...] [--assignee <profile>] [--parent <id>]... [--tenant <name>] - [--workspace scratch|worktree|dir:<path>] + [--workspace scratch|worktree|worktree:<path>|dir:<path>] + [--branch <name>] [--priority N] [--triage] [--idempotency-key KEY] [--max-runtime 30m|2h|1d|<seconds>] [--max-retries N]