fix(kanban): prevent reuse of unfinished worktree checkouts

[Wenfengcheng]

Problem

Part of #126004. A failed/timed-out Kanban git worktree add can leave valid Git metadata before tracked files have been checked out. A retry then returns that partial directory as a usable worker workspace.

Root cause

_ensure_git_worktree accepts a matching common directory as success; _resolve_worktree_workspace independently accepts an explicit linked checkout. Neither records whether the creator completed.

Change

• Reserve a sibling creation marker exclusively before invoking Git, removing it only after successful checkout or verified rollback.
• Fence both existing-checkout acceptance paths while creation is unfinished.
• Best-effort rollback only the new directory reserved by this attempt, using target-specific Git removal. Preserve branches, existing directories, replaced targets and unrelated registrations; no repository-wide prune or recursive deletion fallback.
• If cleanup fails, keep the marker and fail closed on subsequent resolution with a repair instruction.

Verification

Frozen main base: 83597d9b712741bf427b01c26e1b02b4a1435a94.

Native Windows, isolated HOME/USERPROFILE/HERMES_HOME/APPDATA/LOCALAPPDATA before imports, official scripts/run_tests.sh with the existing test interpreter:

• RED on base: 4 assertion failures, real temporary Git repos and public resolve_workspace, timeout/nonzero × repo-anchor/explicit-target. Git --no-checkout provides deterministic early metadata; only the interrupted command outcome is injected.
• Focused GREEN: 10 passed. Also covers failed removal, reentrant resolution during creation, preservation of existing user files, branch retention and successful dirty-checkout reuse.
• Final focused/isolation/Unicode-path run: 13 passed, 2 skipped, zero failures; skips are macOS-only cases on Windows.
• Teardown suite: 13 passed, 1 failed at scratch.rmdir() on Windows (in-use cwd); the same failure was reproduced on the frozen base. Not claiming a full-suite PASS.
• git diff --check: clean.

Broader DB/board/isolation run: 58 passed, 3 failed. All three failures reproduce on the frozen base in the same full DB file (51 passed, 3 failed on each head). Official Claude Code 2.1.280 / Opus 5.5 review is blocked by upstream HTTP 503 auth_unavailable; not PASS. Codex 0.153.4 / gpt-5.6-sol independent exact-head static review: STATIC PASS, no introduced blocker found (a bounded retry completed after the first attempt timed out). This does not replace the blocked Claude review. No real 41k-file/Defender benchmark is claimed.

Scope / remaining acceptance

Delivered here: failed new checkout cleanup + rejection of unfinished marked checkouts through both reuse paths.

Remaining: timeout policy (pre-existing #98754 owns that contract), DB stderr retention, and recovery of legacy unmarked partial trees. Existing empty directories may still be used, but are never deleted on failure. An unresolved marker requires stopping the creator and verifying/repairing/removing the target before deleting the marker; no automatic stale-marker reclamation.

Overlap checked against actual diffs: #98754 changes only checkout budgets; #104226 covers hooks after a successful checkout; #80046 was closed unmerged at its author's request and its registry/lifecycle design is not resurrected here. No new tool/config/dependency, lifecycle registry, branch realignment, remote-tip policy or post-success hook changes.

Continuation decision

Continue safety/verification follow-through on this PR; do not claim full issue resolution. After this PR was published, #126020 appeared with overlapping cleanup and the remaining stderr-budget change. Its actual diff has been inspected. Keep this earlier independently verified contribution open; do not duplicate the newly owned stderr change. Timeout policy already overlaps #98754. Legacy unmarked partial recovery needs a separate proof that legitimate dirty user worktrees cannot be mistaken for incomplete checkouts.

[Enough1122]

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

The fence design is sound — exclusive marker.open("x"), (st_dev, st_ino) identity check, refusing to bless on cleanup uncertainty. Two reachable states make it misfire, both measured by driving the real post-image functions (AST-extracted from 1ecbe826, blob 620f1b51, byte-verified) against real git fixtures.

1. A dead creator's fence permanently bricks a complete, correct workspace. marker.unlink() at :576 is the last statement with no try, so any death between git worktree add returning 0 and that line leaves the fence behind while the checkout is whole. Probe: creator subprocess os._exit(9) immediately after add returns 0 → ['.git','payload.txt'] present; next resolve_workspace() raises unfinished forever. Same end state when :576's unlink itself fails (Windows AV/indexer holding the dotfile). The fence body written at :542 is repo_root\ntarget\nbranch\n — the probe confirms 3 lines, no pid, no timestamp — so :493's bare os.path.lexists can never age out or test liveness. git worktree prune won't help either: that removes the registration, and kanban-creating has 0 hits repo-wide, so no gc path knows the filename. A 100-day-old fence still refuses. Recovery is rm of a hidden dotfile the operator is never shown; the error text names only the path. Suggest a pid/creation-time in the fence plus a staleness bound (this repo has _pid_alive precedents), and drop the fence when add already returned 0 and the tree is clean.

2. Pre-existing #126004 damage is still handed out as a workspace. The fence only exists for checkouts this code created, so a partial checkout left by the pre-merge version — or by a hand-run git worktree add --no-checkout, byte-for-byte the state above minus the fence — passes straight through resolve_workspace() with zero tracked files. Probe returns that directory as the task workspace, no warning. Worth a liveness/content check on the reuse fast path at :535-537.

tests/hermes_cli/test_kanban_worktree_creation_failure.py covers cleanup-refused | in-progress | existing-directory; in-progress re-enters while creation has not returned, so no test exercises a creator that dies after add succeeded — exactly the state in (1).

[Wenfengcheng]

@Enough1122 Thanks for the concrete failure cases. I checked the exact PR head (1ecbe82613f4d4f214ccc8503d2ac627de1a47a4) with real imports and temporary Git repositories on native Windows, through scripts/run_tests.sh, rather than extracted functions.

The interrupted-publication case is reproducible: a creator subprocess exiting immediately after successful worktree add, or an injected PermissionError on the final marker unlink, leaves a complete checkout fenced. Both repo-anchor and explicit-target resolution reject it. This is the intentional fail-closed boundary of this partial fix, not automatic recovery. One correction: _check_worktree_published() already includes the exact marker path and the stop/repair/remove instructions in its error (lines 495–497). After joining the creator, verifying the checkout, and removing that marker, both paths reuse the checkout and preserve local user files. The new temporary probes cover those steps.

I also reproduced the unmarked --no-checkout case. It remains a real limitation explicitly excluded in the PR body, not fixed here. Git reports its contents as staged deletions, which are also a legitimate user state; a content/cleanliness heuristic alone is not enough to justify resetting or deleting such a checkout. PID age alone likewise does not establish publication or exclusive ownership. I am therefore retaining the conservative boundary instead of adding automatic stale-marker reclamation without that proof.

Validation: 15 passed, 0 failed (five temporary feedback probes plus the existing ten creation-failure tests), isolated HOME/USERPROFILE/HERMES_HOME/APPDATA/LOCALAPPDATA. No product code change and no full-issue-resolution claim. Continuation decision: retain this independently useful partial fix; legacy unmarked recovery needs a safe ownership/publication contract, while timeout policy and stderr retention have existing owners as documented above.

[Enough1122]

Correction accepted — the unfinished-fence error raised at _check_worktree_published() (hermes_cli/kanban_db_workspace.py:491-498) does name the marker path and the stop/repair/remove instructions; my "the error text names only the path" was wrong. The main finding (no liveness/ageout on a dead creator's fence) stands, and retaining the conservative boundary on the unmarked --no-checkout case is a defensible call. Thanks for verifying on the exact head.