mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-29 06:31:32 +00:00
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 <name> flag
- kanban show output includes 'Branch: <name>' 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.
This commit is contained in:
aqilaziz
Teknium
parent
53cf82a1ea
commit
1733cb3a13
6 changed files with 145 additions and 18 deletions
|
|
@ -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:<path>``.
|
||||
Accepts: ``scratch``, ``worktree``, ``worktree:<path>``, ``dir:<path>``.
|
||||
"""
|
||||
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:<path>"
|
||||
f"unknown --workspace value {value!r}: use scratch, worktree, "
|
||||
"worktree:<path>, or dir:<path>"
|
||||
)
|
||||
|
||||
|
||||
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:<path> (default: scratch)")
|
||||
help="scratch | worktree | worktree:<path> | dir:<path> "
|
||||
"(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:
|
||||
|
|
|
|||
|
|
@ -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():
|
||||
|
|
|
|||
|
|
@ -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:<path>` | 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 <path> <branch>` 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 <path> ${HERMES_KANBAN_BRANCH:-wt/$HERMES_KANBAN_TASK}` from the main repo first, then cd and work normally. Commit work here. |
|
||||
|
||||
## Tenant isolation
|
||||
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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/<id>/` (or `~/.hermes/kanban/boards/<slug>/workspaces/<id>/` on non-default boards).
|
||||
- `dir:<path>` — 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/<id>/` for coding tasks. Worker-side `git worktree add` creates it.
|
||||
- `worktree` — a git worktree under `.worktrees/<id>/` for coding tasks. Use `worktree:<path>` 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 "<title>" [--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]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue