Skip to content

fix(kanban): isolate and protect linked worktree anchors - #90529

Closed
roycepersonalassistant wants to merge 4 commits into
NousResearch:mainfrom
roycepersonalassistant:fix/kanban-linked-anchor-worktree-20260819-v3
Closed

roycepersonalassistant wants to merge 4 commits into
NousResearch:mainfrom
roycepersonalassistant:fix/kanban-linked-anchor-worktree-20260819-v3

Conversation

@roycepersonalassistant

@roycepersonalassistant roycepersonalassistant commented Aug 20, 2026 •

Copy link
Copy Markdown

Summary

  • treat a persistent linked checkout as an exact repository anchor and materialize each task at <anchor>/.worktrees/<task-id> on its requested branch
  • derive repository identity from Git's verified common directory, never from a filesystem repository that merely surrounds the anchor
  • reuse existing task targets only when the target itself is an exact linked-worktree toplevel with matching Git common directory and branch ownership; fail closed otherwise
  • allow deferred completion cleanup to remove only exact task-owned worktrees, preserving canonical and unrelated linked checkouts
  • add real-Git regression coverage for resolver isolation, cross-repository nesting, wrong-branch reuse, deferred cleanup, and legacy stale task rows

Root cause

The earlier resolver-only repair covered one branch-mismatch path but was never merged/installed, so the live runtime repeated the old behavior. Independently, _cleanup_worktree_workspace treated any clean linked checkout outside the repository's primary checkout as removable. A stale task row pointing at a canonical linked checkout therefore remained eligible for deferred cleanup.

Review found three sibling ownership gaps: matching-branch linked anchors could still be reused directly, an existing target on the wrong branch could be silently accepted, and the surrounding-path fallback could select an unrelated repository when a linked anchor lived inside another checkout. This replacement closes all three gaps with exact path, branch, and Git-common-dir identity checks.

Validation

  • RED/GREEN: deferred cleanup deleted the canonical linked checkout before the ownership guard, then preserved it afterward
  • RED/GREEN: matching-branch reuse, wrong-branch target reuse, and a plain nested directory masquerading as a matching task target failed before resolver hardening, then passed; the plain target fixture proves linked-checkout, common-dir, and branch predicates all match while exact Git toplevel identity does not
  • RED/GREEN: source-nested and unrelated-repository anchor tests selected the wrong target/repository before the common-dir fix, then passed
  • python -m pytest tests/hermes_cli/test_kanban_worktree_isolation.py tests/hermes_cli/test_kanban_worktree_teardown.py tests/hermes_cli/test_worktree_command.py -o addopts= -q — 30 passed
  • python -m ruff check hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_worktree_isolation.py tests/hermes_cli/test_kanban_worktree_teardown.py — passed
  • python -m py_compile hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_worktree_isolation.py tests/hermes_cli/test_kanban_worktree_teardown.py — passed
  • git diff --check — passed
  • Kanban artifact review reproduced the plain-directory ownership gap at the prior head; commit de39c1889c06e1d6ec172210f0af1939d58aff13 adds the requested exact-toplevel guard and real-Git regression for the next reviewer lane

Risk / rollback

Cleanup is intentionally conservative: an explicit/custom target that is not exact .worktrees/<task-id> ownership, or whose branch drifted, is retained rather than deleted. This can leave an orphan for manual review but cannot remove an ambiguously owned checkout. A linked anchor requesting the branch already checked out at the anchor fails closed because Git cannot attach that branch to a second nested worktree; callers must use a distinct task branch.

No live Hermes runtime, DailyGameHints content, cron, Vercel, environment, schema, GSC, or indexing state is changed. Before merge, rollback is closing the PR. After merge, revert the PR commit through normal review.

Supersedes #89734 without rewriting that closed PR's published history. The corrective commit on this open branch was amended before re-review to use GitHub noreply author metadata after the attribution gate identified the local Gmail author address.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have labels Aug 20, 2026
@roycepersonalassistant
roycepersonalassistant force-pushed the fix/kanban-linked-anchor-worktree-20260819-v3 branch from 9f838b6 to 05f4b60 Compare August 20, 2026 05:24
@roycepersonalassistant
roycepersonalassistant force-pushed the fix/kanban-linked-anchor-worktree-20260819-v3 branch from 05f4b60 to de39c18 Compare August 20, 2026 05:38
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

Careful, well-attacked hardening: ownership is now proven (path shape and checked-out branch) before any removal, ambiguous targets fail closed instead of silently running one task on another task's branch, and the cross-repo anchor trap (a linked checkout living inside an unrelated repo) is resolved through the verified common dir rather than filesystem adjacency. The integration tests — including the genuinely nasty "anchor inside source repo" and "plain dir masquerading as worktree" cases — are exactly right. Items:

  • hermes_cli/kanban_db.py:7858 — issue — materializing .worktrees/<task-id> inside an anchored linked checkout leaves an untracked directory in the anchor's working tree for the task's whole lifetime — why it matters — every git status/_worktree_is_dirty style check on that anchor (other tasks' guards, cli._prune_stale_worktrees, user tooling) now reports it dirty, which can suppress legitimate cleanups or trip dirty-guards far away from this code — suggestion — either teach the dirty/untracked checks to ignore the reserved .worktrees/ name, or set --git-dir-style exclusion via .git/info/exclude when materializing under an anchor, and pin it with a test asserting _worktree_is_dirty(anchor) stays False while a nested task worktree exists.

  • hermes_cli/kanban_db.py:7762 — issue (verification) — several previously-tolerated shapes now raise RuntimeError; please confirm every dispatch caller wraps _resolve_worktree_workspace so the error becomes a per-task failure/block event rather than killing the dispatch tick — why it matters — fail-closed at the resolver only helps if the failure lands on the task, not the scheduler loop; a test asserting dispatch records the failure for an ambiguous workspace would close the loop.

No blocking issues found — item 2 is a quick trace-and-confirm.

— reviewer-b (automated review)

Copy link
Copy Markdown

Addressed both automated-review follow-ups in dca9cfef2.

  • _worktree_is_dirty() now ignores only untracked entries under the reserved top-level .worktrees/ container. Tracked or staged changes there still count as dirty. A real-Git regression proves a nested task worktree leaves its persistent anchor clean while an unrelated untracked file still protects the anchor.
  • Both ready and review dispatch callers already contained workspace-resolution exceptions. A parameterized integration test now proves an ambiguous target becomes a task-scoped spawn_failed result, restores the original lane, records the error, and never reaches the spawn callback.

Validation:

  • scripts/run_tests.sh tests/cli/test_worktree.py tests/hermes_cli/test_kanban_worktree_isolation.py tests/hermes_cli/test_kanban_worktree_teardown.py tests/hermes_cli/test_worktree_command.py — 83 passed
  • Ruff on the affected source and tests — passed
  • py_compile and git diff --check — passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants