Skip to content

feat: add safe Kanban worktree lifecycle - #80046

Closed
roycepersonalassistant wants to merge 3 commits into
NousResearch:mainfrom
roycepersonalassistant:feat/worktree-lifecycle-hardening
Closed

roycepersonalassistant wants to merge 3 commits into
NousResearch:mainfrom
roycepersonalassistant:feat/worktree-lifecycle-hardening

Conversation

@roycepersonalassistant

@roycepersonalassistant roycepersonalassistant commented Aug 6, 2026 •

Copy link
Copy Markdown

Summary

  • add a shared, profile-aware SQLite registry for Kanban-owned Git worktrees
  • deterministically reuse exact compatible ownership and fail closed on duplicate, branch-collision, or concurrent reservation ambiguity
  • require explicit terminal dispositions before completing or archiving task-owned worktrees
  • add deterministic inventory, report-only pressure thresholds, protected operational exceptions, and a dry-run-only janitor
  • harden concurrent materialization with per-reservation owner tokens, lease renewal, and compare-and-swap state transitions
  • resolve external linked checkouts through Git common-directory/worktree metadata rather than filesystem ancestry
  • preserve standalone linked-root anchor behavior and keep exact registered targets composable with branch-realignment policy

Worktree lifecycle hardening

  • Only the token owner may run git worktree add, publish reserved -> active, or mark its still-reserved row creation_failed.
  • Compatible losing resolvers do not materialize and do not infer completion from early-visible Git metadata; they wait for the owner's explicit terminal reservation state.
  • The lease starts after pressure discovery, renews immediately before Git mutation, and exceeds the materialization command budget.
  • A losing caller cannot downgrade an active row or poison a successful shared reservation.
  • Detached/branchless reservations remain verifiable.
  • Existing linked checkouts outside the primary repository hierarchy are safely reused when already on the requested branch.

Integration with related PRs

Safety invariants

  • janitor remains dry-run only and has no deletion execution path
  • dirty, changed, active, primary, locked, protected, dependency-retained, duplicate-owned, out-of-root, and open/unverifiable-PR worktrees are excluded
  • protected-policy import cannot take over an active task-owned registry row
  • inventory does not advance terminal snapshots or bless post-terminal drift
  • worktree, branch, remote branch, and stash deletion remain separate operator decisions

Validation

  • focused lifecycle/isolation: 33 passed
  • all 36 tests/hermes_cli/test_kanban*.py files in isolated processes: 191 passed, zero failures
  • Ruff: passed
  • py_compile: passed
  • ty check hermes_cli/kanban_workspaces.py: passed
  • git diff --check: passed
  • static security scan: no hardcoded secrets, shell injection, dynamic eval/exec, unsafe pickle, or dynamic SQL f-strings
  • Codex CLI review found an initial detached-worktree regression; a RED/GREEN regression test and fix were added
  • independent review found two partial-materialization/lease races; owner-only publication, pre-mutation lease renewal, and a slow-owner regression addressed both
  • final Codex CLI review: no discrete correctness issues identified

Full-suite baseline comparison

Exact-base comparison at 8f2712725:

  • origin/main: 145 failed, 4198 passed, 22 skipped
  • feature before this follow-up: 145 failed, 4224 passed, 22 skipped

The same 145 order-dependent/shared-state failures occurred on both trees; the feature added passing tests and no new failures or skips. This follow-up's scoped Kanban suite is fully green.

Notes

The example exception policy is intentionally checked in under docs/examples/ even though the repository's broad examples/ ignore rule required force-adding it.

@roycepersonalassistant

Copy link
Copy Markdown
Author

Ready for maintainer review: the complete CI matrix is green after retrying one unrelated flaky temp-directory cleanup failure in test_codex_app_server_persist (the exact test passed locally on both origin/main and this branch). Independent safety review returned GO. The janitor is strictly dry-run-only and emits no executable deletion command.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 6, 2026
@roycepersonalassistant

Copy link
Copy Markdown
Author

Follow-up safety review — NO-GO on current head

I re-reviewed exact PR head 783b59e47f530a8dd33a1a371851c4d070af8e6a / tree e00c4273a46f46f6bbdedbcc39cdd634598a643e after the earlier GO note. CI is green and the focused 28-test plus broad 186-test Kanban suites pass in isolation, but adversarial fixture-repo probes through kanban_db.resolve_workspace exposed two untested blocking defects:

  1. HIGH — a concurrent compatible resolver can poison a successful shared reservation. Both callers reuse the same reserved workspace ID. If one materializes the worktree while the other loses, the loser executes mark_creation_failed() before the winner calls verify_workspace(). The observed terminal state was a real linked worktree present and listed by Git, the only registry row marked creation_failed, and both callers raising. The creation gate therefore does not guarantee that every successfully created worktree retains an authoritative row under concurrency.

  2. HIGH — existing external linked checkouts no longer resolve. For an absolute workspace_path that is an already-linked checkout outside the primary repository hierarchy and is already on the requested branch, kanban_db.py:6765-6770 derives ownership only from requested.parent. The probe preserved the valid Git worktree but resolve_workspace() raised that the primary repository could not be resolved. origin/main reused this concrete target, while overlapping PR fix(kanban): resolve linked worktree anchors by branch #70585 explicitly defines linked-root anchor behavior.

There is also an integration conflict to resolve before merge: kanban_db.py:6779-6797 rejects a registered exact-path branch mismatch before the _ensure_git_worktree seam can apply the realignment behavior proposed by #71978, while the external-parent lookup conflicts with #70585. This lifecycle PR should wrap ownership/disposition around those seams rather than independently redefine pending branch/anchor policy.

Expected fix: add deterministic concurrent and external-linked public-seam regressions, use reservation ownership/CAS or compatible live verification so a losing caller cannot downgrade a winner, derive repository identity from Git metadata rather than filesystem-parent ancestry, and reconcile #70585/#71978 semantics explicitly. I am holding this PR from merge until an amended head is independently re-reviewed.

@roycepersonalassistant

Copy link
Copy Markdown
Author

Closing at the author's request. This lifecycle hardening will remain a local/fork-only Hermes customization; please do not merge it upstream.

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 sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants