mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-24 16:54:43 +00:00
fix(kanban): stop decompose siblings sharing one worktree checkout
Some checks failed
CI / Detect affected areas (push) Waiting to run
CI / Python tests (push) Blocked by required conditions
CI / Python lints (push) Blocked by required conditions
CI / JS & TS checks (push) Blocked by required conditions
CI / Desktop E2E (push) Blocked by required conditions
CI / Docs Site (push) Blocked by required conditions
CI / Deny unrelated histories (push) Blocked by required conditions
CI / Check contributors (push) Blocked by required conditions
CI / Check uv.lock (push) Blocked by required conditions
CI / package-lock.json diff (push) Blocked by required conditions
CI / Lint Docker scripts (push) Blocked by required conditions
CI / Build&Test Docker image (push) Blocked by required conditions
CI / Supply-chain scan (push) Blocked by required conditions
CI / Review label gate (push) Blocked by required conditions
CI / OSV scan (push) Waiting to run
CI / CI review comment (live) (push) Blocked by required conditions
CI / All required checks pass (push) Blocked by required conditions
CI / CI timing report (push) Blocked by required conditions
Deploy Site / deploy-vercel (push) Waiting to run
Deploy Site / deploy-docs (push) Waiting to run
Docker Build, Test, and Publish / build (amd64, type=gha,scope=docker-amd64, type=gha,mode=max,scope=docker-amd64, linux/amd64, ubuntu-latest) (push) Waiting to run
Docker Build, Test, and Publish / build (arm64, type=gha,scope=docker-arm64, type=gha,mode=max,scope=docker-arm64, linux/arm64, ubuntu-24.04-arm) (push) Waiting to run
Docker Build, Test, and Publish / publish (amd64, type=gha,scope=docker-amd64, type=gha,mode=max,scope=docker-amd64, linux/amd64, ubuntu-latest) (push) Blocked by required conditions
Docker Build, Test, and Publish / publish (arm64, type=gha,scope=docker-arm64, type=gha,mode=max,scope=docker-arm64, linux/arm64, ubuntu-24.04-arm) (push) Blocked by required conditions
Docker Build, Test, and Publish / merge (push) Blocked by required conditions
auto-fix lint issues & formatting / Generate eslint --fix patch (push) Waiting to run
auto-fix lint issues & formatting / Apply patch (push) Blocked by required conditions
Build Skills Index / build-index (push) Has been cancelled
Build Skills Index / trigger-deploy (push) Has been cancelled
Some checks failed
CI / Detect affected areas (push) Waiting to run
CI / Python tests (push) Blocked by required conditions
CI / Python lints (push) Blocked by required conditions
CI / JS & TS checks (push) Blocked by required conditions
CI / Desktop E2E (push) Blocked by required conditions
CI / Docs Site (push) Blocked by required conditions
CI / Deny unrelated histories (push) Blocked by required conditions
CI / Check contributors (push) Blocked by required conditions
CI / Check uv.lock (push) Blocked by required conditions
CI / package-lock.json diff (push) Blocked by required conditions
CI / Lint Docker scripts (push) Blocked by required conditions
CI / Build&Test Docker image (push) Blocked by required conditions
CI / Supply-chain scan (push) Blocked by required conditions
CI / Review label gate (push) Blocked by required conditions
CI / OSV scan (push) Waiting to run
CI / CI review comment (live) (push) Blocked by required conditions
CI / All required checks pass (push) Blocked by required conditions
CI / CI timing report (push) Blocked by required conditions
Deploy Site / deploy-vercel (push) Waiting to run
Deploy Site / deploy-docs (push) Waiting to run
Docker Build, Test, and Publish / build (amd64, type=gha,scope=docker-amd64, type=gha,mode=max,scope=docker-amd64, linux/amd64, ubuntu-latest) (push) Waiting to run
Docker Build, Test, and Publish / build (arm64, type=gha,scope=docker-arm64, type=gha,mode=max,scope=docker-arm64, linux/arm64, ubuntu-24.04-arm) (push) Waiting to run
Docker Build, Test, and Publish / publish (amd64, type=gha,scope=docker-amd64, type=gha,mode=max,scope=docker-amd64, linux/amd64, ubuntu-latest) (push) Blocked by required conditions
Docker Build, Test, and Publish / publish (arm64, type=gha,scope=docker-arm64, type=gha,mode=max,scope=docker-arm64, linux/arm64, ubuntu-24.04-arm) (push) Blocked by required conditions
Docker Build, Test, and Publish / merge (push) Blocked by required conditions
auto-fix lint issues & formatting / Generate eslint --fix patch (push) Waiting to run
auto-fix lint issues & formatting / Apply patch (push) Blocked by required conditions
Build Skills Index / build-index (push) Has been cancelled
Build Skills Index / trigger-deploy (push) Has been cancelled
Decompose children inherit the root's literal workspace_path (#37172), so every sibling of a worktree-kind root points at the SAME checkout. _resolve_worktree_workspace's existing-checkout shortcut then reuses that directory on whatever branch is currently checked out, ignoring the task's own branch_name. Net effect: sibling workers — which can be promoted and dispatched concurrently — run in one directory on the first sibling's branch, with no lock. Work lands on the wrong task's branch (provenance corruption) and concurrent siblings trample each other's index/tree. Fix, two layers: - decompose_triage_task: worktree-kind children no longer inherit the root's literal path; each child materializes its own <repo>/.worktrees/<child-id> at dispatch (dir/scratch inheritance unchanged — children legitimately share those). - _resolve_worktree_workspace: when the requested path is an existing checkout of a DIFFERENT branch, fall back to a fresh <repo>/.worktrees/<task-id> instead of silently reusing it (heals rows that already carry a shared path). Same-branch reuse and the no-repo/own-path degenerate cases keep the legacy behaviour. Tests: tests/hermes_cli/test_kanban_worktree_isolation.py (5); full test_kanban_db.py + test_kanban_decompose_db.py suites pass unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
c2c2449d05
commit
65d42e35d4
2 changed files with 225 additions and 0 deletions
|
|
@ -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 <repo>/.worktrees/<child-id> 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)
|
||||
|
|
|
|||
198
tests/hermes_cli/test_kanban_worktree_isolation.py
Normal file
198
tests/hermes_cli/test_kanban_worktree_isolation.py
Normal file
|
|
@ -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 ``<repo>/.worktrees/<child-id>``.
|
||||
- ``_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 <repo>/.worktrees/<child-id> 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"
|
||||
Loading…
Add table
Add a link
Reference in a new issue