Script the Local Half of Post-Merge Cleanup and Sweep Finished Worktrees in pr_review.py

[ptr727]

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

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3d9789dd-8be2-46c2-abbc-ed53352bdf62

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

🟡 Changes recommended

Unresolved safety issues remain in cleanup ordering, stale-worktree pruning, concurrent operations, and dry-run accuracy.

12 open issues

• Preflight local state before remote deletion · New
• Preflight untracked changes before switching primary · New
• Validate stale entries before pruning · New
• Document containment in any origin ref · New
• Treat concurrently removed worktrees as complete · New
• Re-read branches after failed deletion · New
• Run remote deletion checks during dry runs · New
• Show primary base updates in dry-run output · New
• Preserve positional arguments after CLI options · New
• Correct primary-checkout claim about base ref updates · New
• Document primary fast-forward during apply · New
• Assert sweep continues after unread items · New

What changed in this PR

This PR adds scripted local post-merge cleanup and stale worktree sweeping to pr_review.py.

Changes:

• Adds local branch, worktree, and base-branch cleanup.
• Adds dry-run and apply sweep modes with safety checks.
• Adds documentation and integration tests.

File	Description
tests/​test_pr_review.py	Tests cleanup, sweep, and CLI behavior.
scripts/​README.md	Documents cleanup and sweep workflows.
scripts/​pr_review.py	Implements cleanup, sweep, validation, and CLI handling.

_{Review effort: Lite}

[ptr727]

A recorded local strict-review pass covers head 9744ababcc97f721058eae6284153a031bcdd7c5, the content this pull request carries at that commit against develop, and it recorded 7 findings.

[ptr727]

A recorded local strict-review pass covers head 04b731f443938a844902e1b637b22ebbcd7a4075, the content this pull request carries at that commit against develop, and it recorded 8 findings.

[codecov]

Codecov Report

❌ Patch coverage is 88.22526% with 69 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.46%. Comparing base (1a511c8) to head (235b035).

Files with missing lines	Patch %	Lines
scripts/pr_review.py	88.22%	69 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3186      +/-   ##
===========================================
+ Coverage    64.14%   65.46%   +1.31%     
===========================================
  Files           17       17              
  Lines         9198     9749     +551     
===========================================
+ Hits          5900     6382     +482     
- Misses        3298     3367      +69     

Flag	Coverage Δ
python-3.13	65.46% <88.22%> (+1.31%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ptr727]

A recorded local strict-review pass covers head 9463d0eaf93542750e84122824e33d0735971f33, the content this pull request carries at that commit against develop, and it recorded 5 findings.

[ptr727]

A recorded local strict-review pass covers head 235b035feea777eba74e32a57924f5e2d86d8ff0, the content this pull request carries at that commit against develop, and it recorded 0 findings.