You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Promote Scripted Post-Merge Cleanup and Its Skill Wiring to Main - #3189
Script the Local Half of Post-Merge Cleanup and Sweep Finished Worktrees in pr_review.py #3186 adds the local half of post-merge cleanup to pr_review.py cleanup. With --checkout <base-clone>, it removes the merged branch's worktrees and local branch after the remote delete and fast-forwards the base clone's develop. cleanup --sweep reports, and with --apply removes, the worktrees and branches an interrupted or abandoned session left behind.
Point Every Worktree-Finishing Skill at pr_review.py cleanup #3188 points every skill that finishes a worktree at those commands, in place of the raw git worktree remove, git branch -D, and remote delete, which the host-safety allowlist does not cover. The skills are repo-worktree, drive-pr, merge-and-release, backlog-burndown, pr-review-conduct, unattended-handoff, resync-a-repo, and audit-a-repo. git-commit-conventions's post-squash -D exception now admits a tip behind the verified head.
…ees in pr_review.py (#3186)
## Summary
Part of #3184. Skill changes follow in a second pull request.
Agents leave their local cleanup to the maintainer. The guard does not
block the raw git steps. They are simply not on the permission
allowlist, while `python3 scripts/pr_review.py:*` is. So an attended
agent prompts for each step, and an unattended one cannot run them. This
change moves the whole cleanup into the allowlisted script.
- `cleanup <n> --repo O/R --checkout <primary>` runs the local half
after the remote half:
- removes the merged branch's worktrees, never forced;
- deletes the local branch only at the merged head, using `git branch
-D` after re-reading the tip;
- switches the primary to `develop` only when it held that branch;
- fast-forwards `develop`.
Every refusal is decided before the first removal. The base clone's
outcome is reported and never fails a run whose removals landed.
- `cleanup --sweep --repo O/R --checkout <primary> [--apply]
[--min-age-hours N]` lists what crashed or abandoned sessions left
behind, and removes it only with `--apply`.
- An item qualifies when it is clean, idle at least N hours (default
24), and not mid-operation. It must also be merged by a pull request
from `O/R` at its exact tip, or be contained in `origin`.
- The sweep also deletes the merged pull request's remote branch,
through the same checks as `cleanup <n>`.
- It never touches permanent branches, the primary's branch, locked
worktrees, or submodule worktrees.
- `scripts/README.md` describes both modes.
## Verification
- `python3 tests/test_pr_review.py`: 906 tests, all pass, and the full
`unittest discover` suite passes. Each new refusal test was confirmed to
fail with its guard disabled.
- `ruff check`, `ruff format --check`, `mypy`, and the commit hooks'
prose and EOL gates are clean.
- A dry run against a real clone with a long backlog of leftovers
listed:
- 39 worktrees, matched to merged pull requests or contained in
`origin`;
- 11 loose branches;
- 4 entries whose directories are gone.
It kept a worktree touched 20 minutes earlier by a live session in
another repository, which is the case the age rule exists for.
- A recorded local strict-review pass on every push. The last one, on
the final head, raised no findings. The first raised 31 findings (14 of
them sentence length), the second 10, the third 7. Two rounds of fixes
answered the first two passes. Three consolidations were declined with
evidence:
- `cleanup_git` vs the existing `_git`: `_git` is documented read-only,
swallows a failure into rc 1, and uses a 30s timeout.
- `GITHUB_REMOTE` vs `origin_owner`: `origin_owner` reads only the owner
of the script's own clone, through `insteadOf`. This parser checks the
full name of `--checkout`, raw.
- `GITHUB_REMOTE` vs `carry.normalized_origin`: `normalized_origin` maps
two URL forms to an https URL for comparison and parses no owner or
name.
## Open findings
None. The local pass on 235b035 raised no findings.
## Findings from the local pass on 9463d0e
Each was fixed in 235b035:
- The preflight never checks the primary for a paused merge, cherry-pick
or revert when the primary holds the merged branch, so `checkout
develop` can clear that paused state.
- `loose()` ages a branch by its reflog alone and reads a missing reflog
as infinitely old, so with `core.logAllRefUpdates=false` or the reftable
backend a just-created branch passes the age rule.
- A failed `git worktree prune` under `--apply` stops the whole sweep,
while the docstrings say the other items are still tried.
- `test_a_stale_remote_ref_for_a_deleted_branch_is_not_containment` runs
`--apply` after the report already pruned the stale ref, so it never
proves that `--apply` prunes on its own.
- `finished()` repeats the ancestor test inline, which the existing
`cleanup_behind` helper already provides.
## Findings from the local passes on 04b731f and 3dbc9d7
Each was fixed in 3dbc9d7 or
9463d0e:
- The preflight checks for a rebase or bisect naming the branch only in
linked worktrees, never in the primary. A primary paused mid-rebase on
the merged branch passes the preflight, and `git branch -D` refuses
after the remote delete.
- The docs say a refusal means nothing was deleted. But the re-run
preflight and the pre-delete tip re-read can still refuse after the
remote delete when a concurrent session changes something in between.
- The docs say the local branch is deleted only while it names the
merged head. It is also deleted when its tip is an ancestor of that
head.
- With `--checkout`, the remote delete uses a ref read taken before the
fetch, the preflight and the checkout, which widens the window in which
a push to the merged branch is deleted without the `TIP_MOVED` refusal.
- A report-only sweep runs `git fetch --prune origin` in the primary,
which the docs do not list.
- The closing `git status` and `worktree list` reads in `cleanup_local`
can raise after every removal landed, turning success into exit 2.
- The paused-rebase test uses GNU `sed -i` syntax, which BSD sed on
macOS reads differently.
- `test_a_primary_holding_the_branch_moves_to_develop_before_the_delete`
asserts only the end state, not the order.
- The README's re-check sentences sat between "Every refusal is decided
before anything is deleted" and "That includes", so the list read as
things that already happened.
- A report-only sweep without `--prune` judged containment against stale
refs that `--apply` would prune.
- No sweep test covered a rebase paused in the primary, and the closing
`worktree list` read's guard was untested.
- One docstring line ran to 125 characters.
## Findings from the local pass on 9744aba
Each was fixed in 6defb7b:
- A failed `git worktree remove` is not read back. When a concurrent
cleanup already removed the worktree, the run exits 2 instead of reading
the list and continuing.
- A branch deleted by a concurrent run between the first tip read and
the delete is reported as `LOCAL_TIP_MOVED` at exit 1, not as already
gone. In `loose()` the same case is labelled `checked-out-elsewhere`.
- The docs say the sweep never touches the primary checkout, but
`--apply` fast-forwards its `develop`, and the dry run does not list
that.
- The dry run's `would-remove remote branch` line runs none of the
remote half's checks, so it can name a branch that `--apply` keeps or
finds already gone.
- The docstring says "contained in `origin/develop` or `origin/main`",
while a detached worktree qualifies when contained in any `origin` ref,
and the README says "contained in `origin`".
- `test_a_read_failure_continues_and_exits_2` asserts nothing processed
after the failing item.
- With `number` now optional, Python 3.11's argparse rejects a number
placed after the options, such as `status --repo O/R 7`. Confirmed on
3.11; 3.12 and later accept it. No caller in the tree uses that order.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
## Summary
PR 2 for #3184. Every skill that finishes a worktree now names
`pr_review.py cleanup <n> --repo <owner>/<repo> --checkout <base-clone>`
for a pull request merged into `develop`, and `cleanup --sweep` for what
an interrupted or abandoned session left behind. These replace the raw
`git worktree remove`, `git branch -D`, and remote delete steps, which
the host-safety permission allowlist does not cover.
- `repo-worktree` "Listing and Cleanup": the command, plus hand removal
for trees no `develop` pull request covers (a promotion's worktree, a
detached helper, a standalone clone). A separate base-clone step handles
a `base=kept` outcome. A sweep bullet runs `--apply` only on the
maintainer's go-ahead.
- `drive-pr` step 4, `pr-review-conduct`'s post-merge line, and
`backlog-burndown` "Cleanup Is the Orchestrator's" now name the command.
- `merge-and-release` step 8's hand-written defensive sweep is now one
merged-PR lookup per branch, followed by the command and a `--sweep`
report.
- `unattended-handoff` step 7, `resync-a-repo`, and `audit-a-repo` gain
the cleanup step they lacked.
- `git-commit-conventions`'s post-squash `-D` exception now admits a
branch tip behind the verified head, which is what `cleanup` deletes.
Generated trees are rebuilt with `scripts/build_dist.py`. The closing
reference for #3184 belongs on the promotion pull request.
## Local review
Five recorded `local-strict-review` passes ran at the `frontier` tier,
raising 13 `introduced` findings, all fixed. The last one, the `-d`
upstream claim that Copilot also raised, was fixed by leaving a
promotion branch to `cleanup --sweep`, which reads it as finished once
`origin/main` contains it. `pr-review-conduct`'s loop-completion rule
now names that exception. Nothing remains open.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
❌ Patch coverage is 88.24532% with 69 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.44%. Comparing base (1a511c8) to head (0b5954f). ⚠️ Report is 393 commits behind head on main.
#3190)
## Summary
This fixes a finding raised on promotion #3189. `cleanup --sweep` read
the branches held by worktrees before pruning the entries whose
directory is gone. A finished branch held only by a pruned entry was
therefore skipped by the loose-branch scan until the next run.
`CleanupSweep.prune` now returns the entries it pruned, or under a
report the ones it would prune, and those entries no longer hold their
branch. An entry kept as too recent, or still listed after a failed
prune, keeps holding its branch.
## Tests
- `test_a_pruned_entry_s_branch_is_swept_in_the_same_run` and
`test_a_dry_run_reports_a_prunable_entry_s_branch` both failed before
the fix.
- `test_a_too_recent_prunable_entry_still_holds_its_branch` fails when
`held` drops every prunable entry.
- The failed-prune test now carries a branch-holding entry. It fails
when the failed path returns the entries anyway.
- `python3 -m unittest tests.test_pr_review` ran 917+ tests, all OK.
## Local review
Three recorded `local-strict-review` passes ran at the `frontier` tier.
Both findings were test-coverage gaps, now closed. Nothing remains open.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Correct cleanup status claim when cleanup remains unresolved
.agents/skills/unattended-handoff/SKILL.md:332
[P2] Do not claim the lane has no leftover cleanup when the next sentence permits it. A nonzero cleanup is explicitly only recorded in the close-out comment and is not finished by hand, so the finished worktree or branch can still outlive the lane. Reword the first sentence to say cleanup was attempted and its result was recorded, or keep the lane open until the reported cleanup is resolved.
## Summary
This fixes a previously-missed finding raised on promotion #3189. Step 7
of `unattended-handoff` said that because each merged pull request had
`drive-pr` step 4's cleanup run, "no finished worktree or branch
outlives the lane". That is false when the cleanup could not run or
exited non-zero.
The step is now the bare instruction. Each merged pull request is
cleaned up per `drive-pr` step 4. A cleanup that could not run, or
exited other than `0`, is named in the close-out comment with its
output, rather than finished by hand.
## Local review
Two recorded `local-strict-review` passes ran at the `frontier` tier.
The first found that the rewrite added two claims of the same kind: what
a failed cleanup leaves, and that cleanup ran at all. Both were cut.
Nothing remains open.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Previously missed finding (1): Fixed - "Correct cleanup status claim when cleanup remains unresolved" (.agents/skills/unattended-handoff/SKILL.md:332). Fixed in 0b5954f (#3191). Step 7 no longer claims that no finished worktree or branch outlives the lane. It now says each merged pull request is cleaned up per drive-pr step 4, and that a cleanup that could not run, or exited other than 0, is named in the close-out comment with its output rather than finished by hand.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
An unresolved P1 finding affects unreadable base results, and four nit findings require base=kept follow-up.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Keep lane open when base=kept is reported
.agents/skills/unattended-handoff/SKILL.md:332
A successful cleanup can still report base=kept with exit 0, so the current nonzero-only condition closes the lane while the base clone remains on another branch, dirty, or unable to fast-forward. Treat base=kept as unfinished and record it in the close-out comment instead of closing the lane as complete.
Previously missed finding (1): Declined with evidence - "Keep lane open when base=kept is reported" (.agents/skills/unattended-handoff/SKILL.md:332). A base=kept exit 0 is already handled where the cleanup runs, so step 7 does not need to hold the lane for it.
Step 7 cleans each merged pull request up "per drive-pr step 4". That step says repo-worktree "Listing and Cleanup" covers "what a refusal after it and a base=kept line each leave to do".
repo-worktree's "Return the base clone to current develop" bullet says the base=kept line leaves that step to run: fetch and prune, switch to develop, and fast-forward. It also says to stop and report a dirty base clone or a non-fast-forward rather than reconcile it. The worker runs that step inside drive-pr step 4, before step 7 is reached.
A base=kept base clone holds no lane work. The lane's merged branch, worktree, and remote branch were all removed by the exit 0 cleanup. Holding the lane open for the shared base clone's own state would hold every later lane open behind it.
Restating the base=kept handling in step 7 would make a fourth surface for one rule, which is how these statements drift apart.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Script the Local Half of Post-Merge Cleanup and Sweep Finished Worktrees in pr_review.py #3186 adds the local half of post-merge cleanup to
pr_review.py cleanup. With--checkout <base-clone>, it removes the merged branch's worktrees and local branch after the remote delete and fast-forwards the base clone'sdevelop.cleanup --sweepreports, and with--applyremoves, the worktrees and branches an interrupted or abandoned session left behind.Point Every Worktree-Finishing Skill at pr_review.py cleanup #3188 points every skill that finishes a worktree at those commands, in place of the raw
git worktree remove,git branch -D, and remote delete, which the host-safety allowlist does not cover. The skills arerepo-worktree,drive-pr,merge-and-release,backlog-burndown,pr-review-conduct,unattended-handoff,resync-a-repo, andaudit-a-repo.git-commit-conventions's post-squash-Dexception now admits a tip behind the verified head.Sweep a Pruned Worktree Entry's Branch in the Same Run in pr_review.py #3190 fixes a promotion-review finding.
cleanup --sweepnow judges a branch held only by a pruned worktree entry in the same run, rather than skipping it until the next.Drop the Claim That No Worktree Outlives an Unattended Lane #3191 fixes a second promotion-review finding.
unattended-handoffstep 7 no longer claims that no worktree outlives the lane, and it names a cleanup that could not run or that failed.Installed Skills change at release.
Closes #3184
Closes #1682
Closes #1693
🤖 Generated with Claude Code