diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 2b6124a29c8d..9ab82318dcee 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -5947,6 +5947,15 @@ def decompose_triage_task( child_ws_kind = child.get("workspace_kind") or root_ws_kind if child.get("workspace_path"): child_ws_path = child.get("workspace_path") + elif child_ws_kind == "worktree": + # Never share one worktree checkout between siblings: the + # root's literal path would put every child in the same + # directory on the first-dispatched sibling's branch, with + # no lock — siblings can be promoted and dispatched + # concurrently. Leave the path unset so dispatch + # materializes a fresh /.worktrees/ per + # child from the board anchor. + child_ws_path = None elif child_ws_kind == root_ws_kind: child_ws_path = root_ws_path else: @@ -6324,6 +6333,24 @@ def _resolve_worktree_workspace( if requested.exists() and _is_linked_worktree_checkout(requested): actual_branch = _git_current_branch(requested) + if actual_branch == branch_name: + return requested_resolved, actual_branch + # The requested path is an existing checkout of a DIFFERENT + # task's branch. Decompose children inherit the root's + # workspace_path verbatim, so siblings all point here; reusing + # the checkout as-is would run this task on the other task's + # branch — silent cross-task provenance corruption, and unsafe + # when siblings run concurrently. Fall back to a fresh worktree + # of our own under the same repo. + fallback_root = _repo_root_for_worktree_target(requested.parent) + if fallback_root is not None: + fallback = fallback_root / ".worktrees" / task.id + if fallback.resolve(strict=False) != requested_resolved: + _ensure_git_worktree(fallback_root, fallback, branch_name) + 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 failing dispatch. return requested_resolved, actual_branch or branch_name repo_root = _git_toplevel(requested) diff --git a/tests/hermes_cli/test_kanban_worktree_isolation.py b/tests/hermes_cli/test_kanban_worktree_isolation.py new file mode 100644 index 000000000000..784eb11af965 --- /dev/null +++ b/tests/hermes_cli/test_kanban_worktree_isolation.py @@ -0,0 +1,198 @@ +"""Per-task worktree isolation for decompose siblings. + +Decompose children used to inherit the root's literal ``workspace_path``, +so every sibling of a worktree-kind root pointed at the SAME checkout — +and ``_resolve_worktree_workspace``'s existing-checkout shortcut reused it +on whatever branch was there, letting sibling workers run concurrently in +one directory on one branch (cross-task provenance corruption, no lock). + +Two-part fix under test: +- ``decompose_triage_task`` leaves worktree children's ``workspace_path`` + unset so each child materializes its own ``/.worktrees/``. +- ``_resolve_worktree_workspace`` falls back to a fresh per-task worktree + when the requested path is occupied by another task's branch (heals + pre-existing rows that still carry a shared path). +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from hermes_cli import kanban_db as kb + + +@pytest.fixture +def kanban_home(tmp_path, monkeypatch): + """Isolated HERMES_HOME with an empty kanban DB.""" + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + kb.init_db() + return home + + +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, + ) + + +def _make_repo(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + subprocess.run( + ["git", "init", "-b", "main", str(repo)], + check=True, capture_output=True, text=True, + ) + (repo / "README.md").write_text("base\n", encoding="utf-8") + _git(repo, "add", "README.md") + _git(repo, "commit", "-m", "init") + return repo + + +def _add_worktree(repo: Path, target: Path, branch: str) -> Path: + _git(repo, "worktree", "add", str(target), "-b", branch, "HEAD") + return target + + +def test_decompose_worktree_children_get_own_workspace(kanban_home): + with kb.connect() as conn: + root = kb.create_task(conn, title="build the feature", triage=True) + conn.execute( + "UPDATE tasks SET workspace_kind='worktree', " + "workspace_path='/repo/.worktrees/root' WHERE id = ?", + (root,), + ) + conn.commit() + + child_ids = kb.decompose_triage_task( + conn, + root, + root_assignee="orchestrator", + children=[ + {"title": "spec it", "assignee": "alice", "parents": []}, + {"title": "implement it", "assignee": "bob", "parents": [0]}, + ], + author="decomposer", + ) + assert child_ids is not None and len(child_ids) == 2 + + for cid in child_ids: + row = conn.execute( + "SELECT workspace_kind, workspace_path FROM tasks WHERE id = ?", + (cid,), + ).fetchone() + assert row["workspace_kind"] == "worktree" + # Each child resolves its own /.worktrees/ at + # dispatch; the root's literal path must never be shared. + assert row["workspace_path"] is None + + +def test_decompose_dir_children_still_inherit_path(kanban_home): + with kb.connect() as conn: + root = kb.create_task(conn, title="ops sweep", triage=True) + conn.execute( + "UPDATE tasks SET workspace_kind='dir', " + "workspace_path='/srv/ops' WHERE id = ?", + (root,), + ) + conn.commit() + + child_ids = kb.decompose_triage_task( + conn, + root, + root_assignee="orchestrator", + children=[{"title": "child", "assignee": "alice", "parents": []}], + author="decomposer", + ) + assert child_ids is not None + row = conn.execute( + "SELECT workspace_kind, workspace_path FROM tasks WHERE id = ?", + (child_ids[0],), + ).fetchone() + assert row["workspace_kind"] == "dir" + assert row["workspace_path"] == "/srv/ops" + + +def test_resolve_worktree_falls_back_when_path_occupied(kanban_home, tmp_path): + repo = _make_repo(tmp_path) + occupied = _add_worktree(repo, repo / ".worktrees" / "sibling", "wt/sibling") + + with kb.connect() as conn: + tid = kb.create_task( + conn, + title="second sibling", + workspace_kind="worktree", + workspace_path=str(occupied), # inherited shared/stale path + ) + task = kb.get_task(conn, tid) + + workspace, branch = kb._resolve_worktree_workspace(task) + assert workspace == (repo / ".worktrees" / tid).resolve() + assert branch == f"wt/{tid}" + # The sibling's checkout is untouched, still on its own branch. + assert (occupied / "README.md").exists() + head = subprocess.run( + ["git", "-C", str(occupied), "rev-parse", "--abbrev-ref", "HEAD"], + capture_output=True, text=True, check=True, + ).stdout.strip() + assert head == "wt/sibling" + + +def test_resolve_worktree_same_branch_still_reuses(kanban_home, tmp_path): + repo = _make_repo(tmp_path) + + with kb.connect() as conn: + tid = kb.create_task( + conn, + title="returning task", + workspace_kind="worktree", + ) + own = _add_worktree(repo, repo / ".worktrees" / tid, f"wt/{tid}") + conn.execute( + "UPDATE tasks SET workspace_path = ? WHERE id = ?", + (str(own), tid), + ) + conn.commit() + task = kb.get_task(conn, tid) + + workspace, branch = kb._resolve_worktree_workspace(task) + assert workspace == own.resolve() + assert branch == f"wt/{tid}" + + +def test_resolve_worktree_own_path_on_foreign_branch_keeps_legacy_reuse( + kanban_home, tmp_path +): + repo = _make_repo(tmp_path) + + with kb.connect() as conn: + tid = kb.create_task( + conn, + title="foreign-branch checkout", + workspace_kind="worktree", + ) + own = _add_worktree(repo, repo / ".worktrees" / tid, "wt/foreign") + conn.execute( + "UPDATE tasks SET workspace_path = ? WHERE id = ?", + (str(own), tid), + ) + conn.commit() + task = kb.get_task(conn, tid) + + # The fallback target would be the occupied path itself, so the + # legacy reuse applies rather than failing dispatch. + workspace, branch = kb._resolve_worktree_workspace(task) + assert workspace == own.resolve() + assert branch == "wt/foreign"