Found by the independent verifier on #2315 / #2352 (verdicts posted at pull/2315#issuecomment-5263441478 and pull/2352#issuecomment-5263443101). Both PRs verified SOUND AS MERGED; this is a gap the verification surfaced, not a defect in what shipped.
The gap
#2257 existed because git worktree lock was a signal the cleanup classifier honoured and nothing in the repo ever set. #2315 fixed that by arming it at creation — plugins/source-control/scripts/worktree-create.sh L658-661:
if ! git -C "$toplevel" worktree lock --reason "$lock_reason" "$worktree_path" >&2; then
printf '%s: warning: could not lock the new worktree — cleanup sweeps will not see it as claimed\n' "$PROG" >&2
fi
The script runs under set -uo pipefail with no -e. So a lock failure produces one stderr warning, no exit, and the script continues to completion returning 0. The caller receives a usable worktree path and no machine-readable signal that the guard is unarmed.
That reproduces the original #2257 condition — a worktree that cleanup will treat as unclaimed — except now it happens silently and only under failure, which is harder to notice than the old unconditional absence.
Why the inconsistency is the sharpest part
A few lines later in the same script, a .worktreeinclude copy failure hard-exits 4.
So the script currently treats copying convenience files as fatal and arming the guard that stops another session deleting this worktree mid-rebase as advisory. That ordering is backwards relative to the harm: a missing gitignored config is an inconvenience; an unarmed lock is how a live rebase got deleted twice on this machine, which is what #2257 was filed about.
What to decide
Not obviously "make it fatal" — there is a real argument for best-effort, since a worktree that exists but is unlocked is still more useful than no worktree, and some environments may refuse locks. But the current shape gives the caller no way to know, which is the part worth fixing regardless of which way the decision goes:
- Fail closed — exit non-zero on lock failure, matching the
.worktreeinclude precedent. Simplest, consistent with how the script already treats a lesser failure.
- Stay best-effort, but signal it — return a distinct exit code, or emit the lock state in the script's machine-readable output, so an orchestrator can decide. Preserves the usable-worktree case.
- Best-effort with a retry, then (1) or (2). Most lock failures on Windows are transient handle contention.
Option (2) at minimum. A guard whose arming can fail invisibly is a guard the classifier will trust and the operator cannot audit.
Scope note carried from the verifier
The verifier states plainly that it verified the guard only at the arming end. It did not check the cleanup skill's side of the contract — that cleanup actually reads locked, and that anything ever unlocks. Both are worth a separate look: an armed lock that nothing removes turns every worktree into a permanent non-candidate, which fails in the opposite direction and would show up as worktrees accumulating unreaped (cf. #1776).
Found by the independent verifier on #2315 / #2352 (verdicts posted at pull/2315#issuecomment-5263441478 and pull/2352#issuecomment-5263443101). Both PRs verified SOUND AS MERGED; this is a gap the verification surfaced, not a defect in what shipped.
The gap
#2257 existed because
git worktree lockwas a signal the cleanup classifier honoured and nothing in the repo ever set. #2315 fixed that by arming it at creation —plugins/source-control/scripts/worktree-create.shL658-661:The script runs under
set -uo pipefailwith no-e. So a lock failure produces one stderr warning, no exit, and the script continues to completion returning 0. The caller receives a usable worktree path and no machine-readable signal that the guard is unarmed.That reproduces the original #2257 condition — a worktree that cleanup will treat as unclaimed — except now it happens silently and only under failure, which is harder to notice than the old unconditional absence.
Why the inconsistency is the sharpest part
A few lines later in the same script, a
.worktreeincludecopy failure hard-exits 4.So the script currently treats copying convenience files as fatal and arming the guard that stops another session deleting this worktree mid-rebase as advisory. That ordering is backwards relative to the harm: a missing gitignored config is an inconvenience; an unarmed lock is how a live rebase got deleted twice on this machine, which is what #2257 was filed about.
What to decide
Not obviously "make it fatal" — there is a real argument for best-effort, since a worktree that exists but is unlocked is still more useful than no worktree, and some environments may refuse locks. But the current shape gives the caller no way to know, which is the part worth fixing regardless of which way the decision goes:
.worktreeincludeprecedent. Simplest, consistent with how the script already treats a lesser failure.Option (2) at minimum. A guard whose arming can fail invisibly is a guard the classifier will trust and the operator cannot audit.
Scope note carried from the verifier
The verifier states plainly that it verified the guard only at the arming end. It did not check the cleanup skill's side of the contract — that cleanup actually reads
locked, and that anything ever unlocks. Both are worth a separate look: an armed lock that nothing removes turns every worktree into a permanent non-candidate, which fails in the opposite direction and would show up as worktrees accumulating unreaped (cf. #1776).