Skip to content

source-control:worktree cleanup classifies by cleanliness, not liveness — a clean worktree with a rebase in flight is removed without --force, and the 'locked' flag it honors is never armed #2257

Description

@kyle-sexton

Found by two lanes colliding on a live machine during the #2163/#2171 drain, then reproduced from first principles in a disposable repo.

The gap

plugins/source-control/skills/worktree/context/cleanup.md:23-27 classifies every removal candidate by Active / Stale / Stranded / Orphaned. Every input to that classification is a point-in-time property of the tree: last-commit age, open-PR presence, git status --porcelain, landed-work.sh risk, the locked flag.

None of them answers a different question: is an agent operating in this worktree right now?

That is not visible in git status, and a worktree can be perfectly clean while an operation is mid-flight.

Reproduced — a clean worktree with a rebase in progress, removed without --force

Disposable repo, interactive rebase stopped at a break so no conflict exists and the tree stays clean:

rebase in progress?   YES
status --porcelain: []
status long says:     interactive rebase in progress; onto ead523f
worktree remove without --force: <no error>
directory still exists? NO — DELETED

git status --porcelain is completely empty. The long form knows — interactive rebase in progress — but porcelain v1 does not report it, and git worktree remove does not consult it. Plain removal, no --force, deleted a worktree with an in-flight rebase and printed nothing.

For contrast, in the same probe git does refuse plain removal for staged changes and for untracked files alike:

case A: STAGED new file    -> fatal: '../wA' contains modified or untracked files, use --force to delete it
case B: UNTRACKED file     -> fatal: '../wB' contains modified or untracked files, use --force to delete it
case C: CLEAN worktree     -> removed

So git's own guard covers dirty trees and nothing else. The skill inherits exactly that blind spot.

Two live incidents

  1. D:/worktrees/fix-babysit-g — swept while clean; the removal killed another lane's in-flight rebase. No commits lost (they were already pushed), but the operation died halfway and cost that lane real time.
  2. D:/worktrees/rn2171 — one lane observed it at ~23:09Z holding another agent's staged work plus UU plugins/source-control/CHANGELOG.md. By ~23:30Z it was clean and I removed it. Nothing was lost, and plain removal proves the tree was clean — but the sweep had no way to know an agent had been live in there twenty minutes earlier. It was safe by timing, not by any guard.

The locked flag is read but never armed

cleanup.md:26 qualifies Stale with "no locked flag", and both cleanup.md:103 and create.md:63 correctly document that a locked worktree needs --force --force. So the classifier already has the right signal wired in.

But a repo-wide grep finds no git worktree lock invocation anywhere. The only hit is a comment in babysit-prs/scripts/prune_babysit_worktrees.py:373 describing that prune preserves an existing lock — nothing sets one.

The locked input is therefore structurally always absent: a guard the classifier reads and no lane ever arms.

Suggested direction

Not prescribing the design, but the mechanical parts are cheap and reliable:

  1. Detect in-progress operations before removal. All are directory probes via git -C <path> rev-parse --git-path <name>: rebase-merge, rebase-apply, MERGE_HEAD, CHERRY_PICK_HEAD, REVERT_HEAD, BISECT_LOG. Any present means "an operation is mid-flight here" and the candidate is not disposable, whatever --porcelain says. This alone would have prevented incident 1.
  2. Arm the lock. Have lanes that take a worktree run git worktree lock --reason "<lane> <pid> <started>" on entry and unlock on exit, so the locked input the classifier already honors carries real signal. A stale lock then becomes its own detectable condition (reason string with a dead PID) rather than the current state of no locks at all.
  3. Say so in the classifier table. Add liveness as an explicit non-candidate row next to Stranded, which is already there precisely because it is "the row most easily mistaken for Stale".

Framing

The existing Stranded guard exists because old and quiet does not mean disposable. This is the same lesson one step further: clean does not mean idle. Cleanliness is a point-in-time property; "an agent is mid-operation here" is a liveness property, and the sweep currently measures only the first.

Related worktree-reaping issues, none of which cover liveness: #1776, #1774.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions