Skip to content

fix: stop the fleet prune from destroying the worktrees it cannot remove - #653

Merged
johanzander merged 8 commits into
mainfrom
worktree-fix-worktree-remove-sandbox
Aug 22, 2026
Merged

fix: stop the fleet prune from destroying the worktrees it cannot remove#653
johanzander merged 8 commits into
mainfrom
worktree-fix-worktree-remove-sandbox

Conversation

@johanzander

Copy link
Copy Markdown
Owner

The fleet prune reported "14 merged worktrees kept because each has uncommitted tracked changes — a real backlog of stranded edits." There were no stranded edits. Every one of those worktrees was wreckage the prune itself had created on earlier runs.

What happens

git worktree remove is sandbox-denied. Unlike git worktree add, which fails harmlessly, it fails destructively — removal deletes the working tree first and only then unlinks .git/worktrees/<name>, which is the denied write:

$ git worktree remove --force .claude/worktrees/backlogger
error: failed to delete ".../worktrees/backlogger": Operation not permitted
error: failed to delete ".git/worktrees/backlogger": Operation not permitted

By then ~393 tracked files are gone, and there is no rollback. What remains is a carcass: a registered worktree whose git status is a few hundred D lines and nothing else.

Both prune loops read that as "uncommitted tracked changes" and correctly refuse to auto-delete it — so the failure makes the worktree permanently unprunable by itself. Re-running hits the no---force refusal; --force re-hits the denial. git worktree prune performs the same unlink, so it cannot clear the wreckage either. There is no in-sandbox path to removing a worktree at all.

Evidence

13 carcasses accumulated over three sweeps (deletion timestamps cluster at Aug 15 14:30, Aug 16 23:15, Aug 18 22:13) across PRs #568, #596, #597, #600, #601, #603, #609, #612, #617, #629, #633, #634, #641 — all merged.

The missing path set is byte-identical in every one of them: core/ (270), frontend/ (111), bess_manager/ (3), plus pyproject.toml, Dockerfile, CHANGELOG.md, LICENSE, .pre-commit-config.yaml, docker-compose.ci.yml, mock-run.sh, package-addon.sh, .claude/settings.json. Same filenames means the same APFS readdir order means the same abort point. Identical damage across many worktrees is the signature of this bug, not a coincidence.

Two branches had commits past their merged PR head; both check out as superseded — #600s 9923be4d is verbatim in origin/main:scripts/request-pr-review.sh:140-146, and #568s extras are mains own squash commit pulled back via a merge. Nothing recoverable was lost.

The fix

  • Neither skill removes a worktree any more. sweep-prs Step 3 and implement-issue Step 4 report PRUNE and emit one !-prefixed command for the maintainer to run unsandboxed.
  • Both classify a carcass distinctly. A dirty set that is entirely D is reported as CARCASS (failed prune), not as stranded edits. The predicate anchors on ^ D — unstaged deletions only — so a staged deletion still reads as intentional work. Verified against 5 cases (pure carcass, edit+deletion, staged deletion, added file, clean).
  • docs/agents/local-agent-environment.md gains the remove/prune half of the .git/worktrees denial, next to the add half it already documented.

implement-issue Step 11 keeps using ExitWorktree action=remove, which is the harness doing the removal and therefore unsandboxed — that path was never affected.

scripts/verify-sandbox.sh:88 already predicted this in a parenthetical ("a stray directory when git worktree remove then fails"). Nothing acted on it.

Verification

./scripts/quality-check.sh — 0 errors, 0 warnings. Docs and skills only; no runtime code, so no CHANGELOG entry.

Cleanup of the 13 existing carcasses is a separate manual step — the command is in the skill and has to be pasted with !, since this session cannot run it.

🤖 Generated with Claude Code

https://claude.ai/code/session_014FQA38o22RkHZJ4zEHDZS3

`git worktree remove` is sandbox-denied, and unlike `git worktree add` it
fails DESTRUCTIVELY. Removal deletes the working tree first and only then
unlinks `.git/worktrees/<name>` -- and that unlink is the denied one:

    error: failed to delete '.../worktrees/backlogger': Operation not permitted
    error: failed to delete '.git/worktrees/backlogger': Operation not permitted

By then ~393 tracked files are gone. It does not roll back. What is left is a
carcass: a registered worktree whose `git status` is a few hundred ` D` lines
and nothing else. Both prune loops read that as "uncommitted tracked changes"
and correctly refuse to auto-delete it -- so the failure makes the worktree
permanently unprunable BY ITSELF. Re-running hits the no-`--force` refusal;
`--force` re-hits the denial. `git worktree prune` performs the same unlink,
so it cannot clear the wreckage either.

13 carcasses accumulated across three sweeps (#568, #596, #597, #600, #601,
#603, #609, #612, #617, #629, #633, #634, #641) before anyone read the diff.
The last sweep reported them back as "a real backlog of stranded edits worth
reviewing" -- they were its own wreckage from the previous runs, and not one
byte of real work was in them.

Because the filename set is identical in every worktree, so is APFS's readdir
order, so every carcass loses the SAME ~393 paths (`core/`, `frontend/`,
`bess_manager/`, `pyproject.toml`, ...). Identical damage across many
worktrees is the signature, not a coincidence.

- Both prune loops now report `PRUNE` and emit one `!`-prefixed command for
  the maintainer to run unsandboxed, instead of removing anything themselves.
- Both classify a dirty set that is entirely ` D` as `CARCASS`, distinct from
  real edits. The predicate anchors on `^ D ` (unstaged deletions only), so a
  STAGED deletion still reads as intentional work.
- `local-agent-environment.md` gains the `remove`/`prune` half of the
  `.git/worktrees` denial, next to the `add` half it already documented.

`verify-sandbox.sh:88` predicted this in a parenthetical ("a stray directory
when `git worktree remove` then fails"); nothing acted on it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014FQA38o22RkHZJ4zEHDZS3
@bess-product-owner

Copy link
Copy Markdown
Collaborator

Confirming the premise with a measurement across the whole fleet — it is stronger than the PR states.

Every worktree currently failing the status --porcelain -uno gate is 100% deletions, zero modifications:

backlogger:                          D=393  M=0  other=0
docs-release-ruleset:                D=397  M=0  other=0
drop-permission-hooks:               D=389  M=0  other=0
feat+issue-554-defer-tou-writes:     D=389  M=0  other=0
fix-gh-release-ask:                  D=393  M=0  other=0
fix-issue-569-huawei-solar:          D=389  M=0  other=0
fix-issue-604-signed-pair-aliases:   D=393  M=0  other=0
fix-issue-621-wizard-platform-gate:  D=397  M=0  other=0
fix-issue-624-pwl-window-bisect:     D=397  M=0  other=0
fix-issue-630-subfloor-solar-export: D=397  M=0  other=0
issue-589-divergence-gate:           D=389  M=0  other=0
review-loop:                         D=389  M=0  other=0
skill-ready-on-approval:             D=389  M=0  other=0

13 of 13. Not "mostly wreckage" — there is not a single modified file
anywhere in the set, which is what a genuine stranded edit would look like. Each
count clusters at 389/393/397, i.e. the tracked file count of main at the
moment that worktree was hit.

Two things worth folding in:

  1. It predates today. I created four of these in a sweep this evening
    (docs-release-ruleset, 621, 624, 630, all D=397). The other nine
    were already wreckage before I started, at lower counts — so the prune has
    been quietly doing this across multiple sessions, and each run's damage then
    trips the next run's uncommitted gate. That is the self-perpetuating part.

  2. A second, independent failure mode hits the same command, and the fix
    should cover it: git worktree remove also aborts when the worktree holds
    untracked files — and it aborts after unlinking. The gate uses -uno,
    so untracked content passes the check and then blocks the removal it just
    authorised. Separately, fix-issue-646-stage2-silent classified as PRUNE
    while carrying lock reason: claude session … pid 90253; the skill's table
    says locked worktrees are skipped, but the script only matches on session
    cwd. I confirmed that pid was dead before unlocking it.

So there are three ways this command destroys a worktree it cannot remove:
sandbox-denied .git/config, untracked files, and a stale lock.

johanzander and others added 3 commits August 18, 2026 23:45
The previous commit inferred that `git worktree prune` was denied because it
performs the same `.git/worktrees/<name>` unlink as `remove`. Measured now,
using a carcass as the probe -- `rm -rf` its directory, then prune:

    $ git worktree prune -v; echo "exit=$?"
    Removing worktrees/backlogger: gitdir file points to non-existent location
    error: failed to delete '.../.git/worktrees/backlogger': Operation not permitted
    exit=0

Denied as expected, but it EXITS 0. `remove` at least exits 255; prune reports
success having done nothing, so `git worktree prune && echo done` prints done.
That is the nastier of the two and was not in the inferred version.

Two further measured facts change the guidance:

- The denial is on the `.git/worktrees/<name>` unlink ONLY. `rm -rf` of the
  working tree from Bash SUCCEEDS. An agent can therefore always destroy the
  files and never the registration, and half-doing it converts a carcass into
  a `prunable` phantom -- no better, still needing the same unsandboxed fix.
- A phantom breaks both classification loops. Its directory is gone, so
  `git -C "$wt" branch --show-current` fails, `$b` comes back empty, and the
  detached-HEAD guard swallows it as "SKIP (detached)". Both loops now test
  `[ -d "$wt" ]` first and report PHANTOM. Verified over four cases: missing
  dir with and without a branch name, detached, and on a branch.

The emitted cleanup command gains a trailing `git worktree prune` (a phantom
has no directory, so `remove` has nothing to work with), uses `[ -n "$b" ]`
instead of `&&` since a phantom yields no branch name, and now says it must
run from a NON-worktree-isolated session -- an isolated one refuses the `cd`
to the shared checkout, which is why the first cleanup attempt silently did
nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014FQA38o22RkHZJ4zEHDZS3
…thing

Third instance of the `.git/config` denial, alongside `checkout -b --track`
and `push -u`, and the same exit-0-while-partially-failing shape as the prune
bullet:

    $ git branch -D worktree-backlogger; echo "exit=$?"
    error: could not lock config file .../.git/config
    warning: update of config-file failed
    Deleted branch worktree-backlogger (was 6c70a77).
    exit=0

The first draft of this bullet asserted the denial leaves a stale
`[branch "<name>"]` stanza behind. Checked before committing, and it does not:
grepping .git/config afterwards found no stanza for the deleted branch, and
the only stale one in the file is an unrelated `undefined`. The branch had no
stanza to drop in the first place -- writing one needs `push -u` or
`checkout -b --track`, both denied by the two bullets above, so branches
created under this sandbox never have one. Recorded as the general case, with
the pre-sandbox branch explicitly marked untested rather than assumed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014FQA38o22RkHZJ4zEHDZS3
@bess-agent

Copy link
Copy Markdown
Collaborator

@claude-bot review

@bess-manager-claude-bot bess-manager-claude-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Solid, well-evidenced fix for a real and serious bug (13 destroyed worktrees across 3 sweeps, byte-identical missing-path sets across all of them — good forensic work). The core mechanism change is correct: both implement-issue and sweep-prs now only report PRUNE/CARCASS/PHANTOM and emit a single !-prefixed command for the maintainer to paste, rather than ever calling git worktree remove/prune from sandboxed Bash. I traced the shell logic in both skill files line-by-line — the [ -d "$wt" ] phantom guard, the ^ D carcass predicate (correctly distinguishes unstaged deletions " D " from staged "D ", so a genuinely staged deletion still reads as real work), and the maintainer script (branch name captured via symbolic-ref before remove, --force used only where needed) are all correct.

gh api is unavailable to me this run (permission-denied on a read-only probe), so per the review instructions I'm folding the one finding below into this summary rather than posting inline.

Blocking finding — the fix leaves the same file self-contradictory

docs/agents/local-agent-environment.md gets four new bullets in this diff documenting that git worktree remove/git worktree prune are sandbox-denied and destructive (deletes ~393 tracked files before failing on the .git/worktrees/<name> unlink, no rollback). That's accurate and well-evidenced. But the same file, untouched by this diff, makes three claims that directly contradict it:

  • local-agent-environment.md:55-56: `rm`, `git reset --hard`, `rebase`, `merge`, `git branch -D`, `git worktree remove` all run unattended. — lists it as safe to run without supervision.
  • local-agent-environment.md:325: "The OS sandbox is what makes the unattended list safe, and it is on." — this is the thesis sentence immediately preceding this PR's own insertion point (the new bullets land right after it, under the same - list at line ~344). The bullets that follow it in this very diff disprove it for git worktree remove: the sandbox is not what makes that command safe, it's what makes it destructively unsafe.
  • local-agent-environment.md:571-574: "Don't add a prompt where git already refuses." `git branch -D` and plain `git worktree remove` are deliberately not in the `ask` list: git itself blocks the dangerous case (it won't delete a branch checked out in another worktree, and won't remove a worktree holding uncommitted or untracked files). — this reasoning doesn't cover the case that actually caused the damage: a clean, merged worktree, where git has no reason to refuse and happily starts the removal, which is exactly when the sandbox corrupts it mid-operation.

This isn't a nitpick — it's the "fitness of approach" checklist's item 2 (does it hold for all valid inputs, not just the triggering case): a future agent or maintainer who reads only the ### Permissions section (which is the section that governs whether a command needs a confirmation prompt at all) has no reason to believe git worktree remove is dangerous, and could reintroduce a direct call to it outside the two skills this PR fixed. Given this file's entire purpose is to be the single source of truth an autonomous agent consults before acting unattended, and given the new content sits ~20-250 lines away from the claims it contradicts, this should be reconciled before merge — e.g. carve out git worktree remove/prune as an explicit exception at lines 55-56 and 325, and narrow the "git already refuses" reasoning at 571-574 to the cases it actually covers (uncommitted/locked worktrees), not clean ones.

Checklist

  • Root-cause match: no linked issue (self-diagnosed from operational evidence pasted directly in the PR body — git worktree remove/prune output, byte-identical missing-path sets across 13 carcasses). The evidence is concrete and directly supports the stated root cause.
  • Regression test: none, and none is really possible here — this is agent-instruction markdown with embedded shell snippets, not code under the test suite. Acceptable given the nature of the change.
  • Minimal, no scope creep: yes. The added git branch -D / .git/config gotcha bullet is tangential to the headline bug but directly relevant to verifying the maintainer script this PR asks the maintainer to run (it calls git branch -D), so it's in scope.
  • Rule violations: the contradiction above. No architecture/error-handling/security rules apply (no Python/TS touched).

Verdict: REQUEST_CHANGES

The mechanism fix is right and should ship, but please reconcile the ### Permissions section's "runs unattended" / "sandbox makes it safe" / "git already refuses" claims with the new destructive-failure bullets in the same file before merging — otherwise the fix and the doc it lives in tell an agent two different things about the same command.

johanzander and others added 2 commits August 22, 2026 21:39
…ons doc

The new destructive-failure bullets for `git worktree remove`/`prune`
contradicted three claims in the same file: the unattended list (which still
named `git worktree remove`), the "sandbox makes the unattended list safe"
thesis, and the "git already refuses the dangerous case" reasoning for the ask
list. A reader of only the Permissions section could conclude the command was
safe to call from sandboxed Bash — the exact bug the skills no longer call.
Carve the two verbs out as explicit exceptions and scope the git-refuses
reasoning to the cases git actually covers.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@bess-agent

Copy link
Copy Markdown
Collaborator

@claude-bot review

@bess-manager-claude-bot bess-manager-claude-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

This is a docs/skills-only change (no runtime code) that stops sweep-prs and implement-issue from calling git worktree remove/prune from sandboxed Bash, and instead has them report PRUNE/CARCASS/PHANTOM and hand a !-prefixed command to the maintainer. Root cause and evidence in the PR body are solid and independently verifiable against docs/agents/local-agent-environment.md's existing "What stays denied" section (the .git/worktrees/<name> unlink denial, git worktree prune exiting 0 while failing). No linked issue — this is a self-diagnosed infra bug, well documented with byte-identical carcass evidence across 13 worktrees.

Root-cause match: N/A (no issue), but the stated root cause (destructive partial failure of git worktree remove under the sandbox, -uno porcelain misread as real edits) is demonstrated with real command output, not asserted.

Regression coverage: N/A — this is process/documentation, not testable code; no test is expected or claimed.

Scope: Touches exactly the three files needed (two SKILL.md, one docs file) and follows the existing gh api-write precedent (report + emit !-prefixed command for the maintainer) rather than inventing a new pattern. No CHANGELOG entry, consistent with prior docs-only PR #651 which also skipped it.

Finding: the emitted maintainer command in sweep-prs Step 3 can't locate PRUNE/CARCASS worktrees it just found

.claude/skills/sweep-prs/SKILL.md:

Step 2's classify loop reports PRUNE and CARCASS by branch name only:

117:      echo "PRUNE: $b"; continue
...
110:      echo "CARCASS (failed prune, $(printf '%s\n' "$dirty" | grep -c .) deletions): $b"

But Step 3's emitted maintainer command reconstructs the worktree path from <names> as a bare directory name under .claude/worktrees/:

157:cd /Users/johanzander/GitHub/bess-manager && for wt in <names>; do
158:  b=$(git -C ".claude/worktrees/$wt" symbolic-ref --short HEAD 2>/dev/null)
159:  git worktree remove --force ".claude/worktrees/$wt"

Nothing in Step 2 ever captures the actual worktree directory name for a PRUNE/CARCASS entry — only the branch name — and nothing in this repo's docs states that the worktree directory name equals the branch name (I checked docs/agents/local-agent-environment.md and docs/agents/rules.md for this; EnterWorktree naming isn't documented anywhere). Worse, git worktree list — what Step 2 iterates over — also includes sibling-folder worktrees (../bess-manager-feature/, documented as "first-class" in local-agent-environment.md's Worktree Conventions), which don't live under .claude/worktrees/ at all.

Practical effect: an agent following this doc literally has only a branch name to work with when composing <names>, and .claude/worktrees/<branch-name> will frequently not be the real path — the git worktree remove --force in the pasted command then fails to find the worktree and the carcass/prune survives, silently defeating the exact cleanup this PR exists to enable. In the unlucky case where a same-named directory happens to exist under .claude/worktrees/, it removes the wrong worktree instead.

Contrast with implement-issue's version of the same fix, which gets this right — it reports the actual path:

.claude/skills/implement-issue/SKILL.md:337:

echo "PRUNE: $wt  ($b)"                                   # report; do NOT remove here

and its CARCASS line (331) also carries $wt, not just $b. sweep-prs should do the same — report $wt (or make it explicit in the "Act" section that the maintainer must re-derive the path via git worktree list, since the branch name alone is not sufficient and doesn't cover sibling worktrees).

Minor: implement-issue Step 4's "emit one command" line drops PHANTOM

.claude/skills/implement-issue/SKILL.md:346:

Emit one `!`-prefixed command covering every `PRUNE` and `CARCASS` for the
maintainer to paste, exactly as `sweep-prs` Step 3 does.

PHANTOM is defined two lines earlier (322-324) as a real, distinct state, and sweep-prs's own Step 3 text explicitly says "Collect every PRUNE, CARCASS and PHANTOM" — so "exactly as sweep-prs Step 3 does" is itself inaccurate here, since this sentence omits PHANTOM. Low practical impact (the trailing git worktree prune in the emitted command cleans up phantoms regardless of what's in the loop), but worth fixing for consistency between the two skills.

Rule compliance

No violations found against docs/agents/rules.md (no runtime code touched, no Optional/hasattr/new classes/sensor access applicable). Comments added are WHY-focused, matching the Comments rule. No CHANGELOG entry needed — consistent with prior docs-only PR #651.

The sweep's emitted maintainer command reconstructed `.claude/worktrees/<name>`
from branch names, but `git worktree list` yields paths and includes sibling
worktrees outside `.claude/worktrees/` — the branch name alone cannot locate
the worktree, so removal would silently miss its target. Report `$wt` in the
PRUNE/CARCASS lines and drive the command from those paths. Also count PHANTOM
in implement-issue's emit-one-command line, matching sweep-prs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@bess-agent

Copy link
Copy Markdown
Collaborator

@claude-bot review

@bess-manager-claude-bot bess-manager-claude-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Scope: docs/skills only (.claude/skills/implement-issue/SKILL.md, .claude/skills/sweep-prs/SKILL.md, docs/agents/local-agent-environment.md) — no runtime code, matches the PR description's claim.

Root cause match: No linked GitHub issue — this is a self-diagnosed tooling bug (the PR body itself is the evidence sheet: measured destructive failure of git worktree remove/prune under the sandbox, with reproduction output and a count of 13 real carcasses across past sweeps). The fix matches the diagnosis: stop calling git worktree remove/prune from sandboxed Bash entirely, classify existing damage (CARCASS/PHANTOM) instead of misreading it as stranded edits, and hand removal off to the maintainer via a single !-prefixed command. docs/agents/local-agent-environment.md is updated consistently — I checked all four touched sections (the unattended-list claim, the "git already refuses" claim, the new "What stays denied" bullets, and the "don't add a prompt" section) and the previously-contradictory claims about git worktree remove being unattended-safe are now correctly carved out as exceptions.

Tests: None possible/applicable — this is markdown instructions for agents, not executable code with a test harness. ./scripts/quality-check.sh (0 errors/warnings) is the only available gate and the PR states it passed.

Minimality: Changes are scoped to the three files that actually needed updating for this bug; no unrelated cleanup.

Correctness bug found.claude/skills/implement-issue/SKILL.md:769-780 (Step 11, "After Merge"):

2. Remove the worktree — via `ExitWorktree action=remove discard_changes=true`
   if the session is still in it. That is the harness doing it, so it is not
   sandboxed and it works. If the session has already left, **hand the
   `git worktree remove --force <path>` to the maintainer to paste with `!`**
   rather than running it: from a sandboxed Bash it half-deletes the worktree
   and then fails (see Step 4).

3. Force-delete the local branch and prune stale remote refs:

   ```bash
   git branch -D <branch-name>
   git fetch origin --prune

Item 2's "session has already left" branch is new in this PR and correctly defers worktree removal to the maintainer (async — it won't happen in this session). But item 3 still runs `git branch -D <branch-name>` immediately afterward, in the same session. Git refuses to force-delete a branch that is checked out in an existing worktree ("cannot delete branch '<branch>' checked out at '<path>'") — and the worktree registration (`.git/worktrees/<name>`) is exactly what's still present in the deferred case, since only the maintainer's not-yet-run command removes it. So step 3 will fail whenever step 2 takes its new deferred path.

This PR's own fix to `sweep-prs/SKILL.md` (the "Act" section, ~line 157-161) avoids this exact race by bundling `git worktree remove --force "$wt"` and `git branch -D "$b"` into the **same** deferred maintainer command, in the right order. Step 11 wasn't given the equivalent treatment — it still splits "maybe-deferred worktree removal" from "immediate branch delete" as two independent steps with no ordering guard between them. Confirmed this is not a pre-existing issue: on `main`, item 2 assumed the removal always happened synchronously in the current session (that assumption *was* the destructive bug this PR fixes), so item 3 was safe under the old text; the new deferred branch was added without updating item 3 to match.

Suggested fix: either fold the branch delete into the same emitted maintainer command when the worktree removal is deferred (matching `sweep-prs`), or explicitly note in item 3 that it only applies when item 2 completed in-session.

**Minor (not blocking on its own):** `sweep-prs/SKILL.md`'s emitted maintainer command, `for wt in <paths>; do ...`, word-splits `<paths>` on whitespace. Low risk given `.claude/worktrees/<name>` naming, but the surrounding text now explicitly extends this to sibling checkouts too (`../bess-manager-feature/`), which widens the space of possible paths.

**Verdict: REQUEST_CHANGES** — one concrete correctness bug in the exact area this PR is fixing (Step 11 branch-delete ordering), everything else is sound and well-evidenced.

The After-Merge cleanup splits in two: `ExitWorktree action=remove` clears the
worktree in-session, or removal is handed to the maintainer when the session
has already left. The old item 3 ran `git branch -D` right after either path,
but git refuses to force-delete a branch while its worktree registration
persists — the exact state the deferred path leaves behind, since only the
maintainer's not-yet-run command clears the registration. Emit the branch
delete as part of that same deferred command (remove first, then delete, as
sweep-prs does), and scope item 3 to the in-session path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@bess-agent

Copy link
Copy Markdown
Collaborator

@claude-bot review

@bess-manager-claude-bot bess-manager-claude-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: PR #653 — stop the fleet prune from destroying worktrees it cannot remove

Scope: .claude/skills/implement-issue/SKILL.md, .claude/skills/sweep-prs/SKILL.md, docs/agents/local-agent-environment.md. Docs/skill-instruction only — no application code, so the Python/TS architecture checklist (Optional[x], hasattr, sensor mapping, camelCase, etc.) doesn't apply here.

Root cause match: Yes. The PR body's evidence is concrete and checks out: git worktree remove deletes the working tree before hitting the denied .git/worktrees/<name> unlink, and 13 carcasses across PRs #568#641 have byte-identical missing path sets (core/, frontend/, bess_manager/, ...), which is the signature of one deterministic failure point, not 13 independent incidents of stranded work. The fix directly addresses that: neither skill calls git worktree remove/prune from sandboxed Bash anymore: both now only report PRUNE/CARCASS/PHANTOM and emit one !-prefixed command for the maintainer to run unsandboxed.

Carcass detection logic (sweep-prs/SKILL.md and implement-issue/SKILL.md, the grep -v '^ D ' check): verified this is sound. git status --porcelain -uno reports unstaged deletions as " D <path>", so a dirty set that is entirely that pattern is unambiguously "every tracked file vanished," not intentional work; a staged deletion (AD, MD, ...) still fails the all-^ D -lines test and correctly falls through to KEEP/SKIP. I did not find a case this misclassifies.

Would the change catch a regression? There's no automated test here (these are markdown-embedded shell recipes an agent/maintainer runs, not application code under pytest), and the PR is honest about that — ./scripts/quality-check.sh is the only stated verification, and the description doesn't claim more. That's consistent with how other skill files in this repo are "tested" (by the agents that follow them), not a gap specific to this PR.

Minimal / no scope creep: Yes. All three touched files are directly load-bearing for the bug (the two prune loops plus the doc section describing the sandbox denial), and the diff is proportional to a real, evidenced multi-incident bug.

One nit — an unverified rationale, not a functional bug:

.claude/skills/sweep-prs/SKILL.md:170-171:

The trailing git worktree prune is what clears any PHANTOM, whose directory is already gone so remove has nothing to work with.

I measured this against real (unsandboxed) git and it doesn't hold: git worktree remove --force <path> on an already-missing directory (a prunable entry) succeeds by itself, exit=0, and fully clears the .git/worktrees/<name> registration — no trailing prune needed.

$ git worktree list
/tmp/.../wt-test2 183ccb8 [wtbranch2] prunable
$ git worktree remove --force ../wt-test2; echo "exit=$?"
exit=0
$ git worktree list
(entry gone)

This doesn't change the emitted command's behavior — it already calls remove --force on every path and prune as a catch-all, so PHANTOM entries get cleared either way — but given this PR's own standard of "measured, not assumed" for every other claim about git's destructive-failure modes (and it gets that standard right everywhere else I checked: the prune exit-0-while-failing behavior, the .git/config warning on branch -D, the carcass byte-identical path sets), this one line is the odd one out. Worth either re-measuring in your own environment or softening to something like "and prune catches anything remove leaves behind" so it doesn't read as a verified fact it isn't.

Not a blocker — approving.

@johanzander
johanzander marked this pull request as ready for review August 22, 2026 21:27
@johanzander
johanzander merged commit e9a4baa into main Aug 22, 2026
8 checks passed
@johanzander
johanzander deleted the worktree-fix-worktree-remove-sandbox branch August 22, 2026 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants