docs: record the release-* ruleset, which now exists - #641
Conversation
The `Protect stable hotfix branches` ruleset was created after #635 merged (id 20972632: `refs/heads/release-*`, non_fast_forward, enforcement=active, empty bypass_actors). CLAUDE.md and quality-check.sh still carried the warning that `release-X.Y` was unguarded at both layers, which is no longer true -- and a doc asserting a gap that has been closed misleads exactly as much as one asserting a guard that does not exist. Adds the row to the ruleset table and replaces the warning with the reason for the one asymmetry in it: `release-*` blocks non_fast_forward and deliberately NOT deletion. The branch is pushed and tagged, so rewriting it must fail; deleting it once the release is out is ordinary cleanup and must not. The tag ruleset one row up is what makes dropping the deletion guard safe -- the published tag pins the released commit. Careful with the summary line above the table: `--delete release-X.Y` is still permitted, so "refuse every line above" would be false. It refuses every REWRITE. Written that way once in this commit's first draft and corrected before pushing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016tGBHZEAfKzP9qoJU4NdNY
|
@claude-bot review |
There was a problem hiding this comment.
Verdict: APPROVE
Fact-check
Verified against live GitHub state (gh ruleset view 20972632 --repo johanzander/bess-manager), not just the diff text: the Protect stable hotfix branches ruleset exists exactly as claimed — refs/heads/release-*, rule non_fast_forward only, enforcement=active, empty bypass list. The new table row and the replaced
Checklist
- Matches linked context: this is a follow-up to #635 recording a ruleset created after that PR merged, not an issue fix — no root-cause claim to check, and the description is honest about that.
- Minimal/no scope creep: only
CLAUDE.mdandscripts/quality-check.shtouched, both comment/doc text. No behavior change, confirmed by running the PR's version ofquality-check.shlocally — the permission-surface section (the only part this diff can affect) reports✅ Permission surface intact (76 command shapes checked, 20 require deny, 26 must stay unattended). The 4 errors in my run are pre-existing local environment gaps (missing.venv/frontend deps in this checkout), unrelated to the diff. - Rule violations: none. No code touched, so the Python/API/error-handling checklist items don't apply. Comment style matches the file's existing WHY-focused convention.
- Wording-trap check called out in the PR body: correct — the new prose says "refuse every rewrite" rather than "refuse every line," and that matches reality (
--delete release-X.Yis still permitted).
One non-blocking gap
scripts/quality-check.sh:190 (top comment block) and scripts/quality-check.sh:343 (MUST_NOT_BE_GUARDED preamble) both now document the new mapping:
# git push origin +release-9.9 -> non_fast_forward on release-*
...
# +release-X.Y (rewrite) -> refused by a ruleset
but the MUST_NOT_BE_GUARDED array itself (scripts/quality-check.sh:353-355) only carries the pre-existing "git push origin --delete release-9.9" entry — there's no new pinned string like "git push origin +release-9.9" alongside it, unlike the exactly analogous sibling "git push origin +beta-release-9.9" on the line above, which pins that ruleset-guarded rewrite case.
This file is unusually explicit about why that pinning matters ("asserts this by COMMAND STRING, not by rule name... When a new bypass spelling turns up, add the string to the right list first"). This PR introduces exactly that situation — a new ruleset-guarded push spelling — documents it in prose, but doesn't add the parallel pinned entry the other three ruleset-guarded spellings (main --force, +beta-release-*, tag force) all have. It doesn't break anything (script is green either way; verified locally), so it's not a blocker — just worth a fast follow-up so the new spelling gets the same command-string coverage as its siblings.
…ove (#653) * fix: stop the fleet prune from destroying the worktrees it cannot remove `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 * fix: measure the prune half, which exits 0 while failing 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 * docs: record that `git branch -D` warns on .git/config but strands nothing 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 * docs: reconcile worktree-remove bullets with the rest of the permissions 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> * docs: report worktree paths, not branch names, in the prune output 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> * docs: fold the branch delete into Step 11's deferred worktree removal 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> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to #635. The
Protect stable hotfix branchesruleset now exists:#635 shipped with a⚠️ in CLAUDE.md saying
release-X.Ywas unguarded at both layers, plus a matching comment inquality-check.sh. That was accurate when written and is now false. A doc asserting a gap that has been closed misleads exactly as much as one asserting a guard that does not exist — and both failure modes have already happened once each in this area today.Changes
release-*row to the ruleset table (four → five)The asymmetry, since it will look like an oversight later
release-*blocksnon_fast_forwardand deliberately notdeletion, unlike every other branch ruleset here. The branch is pushed and tagged by thereleaseskill (steps 2–6), so rewriting it must fail — but deleting it once the release is out is ordinary cleanup and must not. What makes dropping the deletion guard safe is the tag ruleset one row up: the published tag pins the released commit, so a spentrelease-*branch costs nothing to lose. Before tagging it is recoverable only from a local reflog, so delete it after, not during.One wording trap
--delete release-X.Yis still permitted, so the summary above the table cannot say the rulesets "refuse every line above" — it refuses every rewrite. I wrote the wrong version first and corrected it before pushing; flagging it because the same sentence is the natural thing to write next time the table changes.Verification
scripts/quality-check.sh— 0 errors, 0 warnings. Docs and comments only; no behaviour change.