Repository navigation
chore: cleanup-branches task + gitignored mise task split - #267
Merged
Merged
Conversation
…nstall flow Tasks now live in .mise.toml + .mise/tasks/*.toml (split via task_config.includes), gitignored since they're personal dev-workflow shortcuts, not shared project config. refresh-install now pulls master and runs 'agentflare dev-install' instead of the old worktree-detection + cargo install --path --force script.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds branch and worktree cleanup automation with protected-branch checks, merge-base safety verification, dry-run support, and optional remote deletion. Removes obsolete mise tasks and the refresh-install script, and ignores local mise configuration. ChangesBranch cleanup automation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CleanupScript
participant LocalGit
participant RemoteGit
User->>CleanupScript: Run cleanup options
CleanupScript->>LocalGit: Fetch and inspect branches and worktrees
LocalGit-->>CleanupScript: Return cleanup candidates
CleanupScript->>LocalGit: Verify and remove eligible worktrees and branches
CleanupScript->>RemoteGit: Delete merged remote branches when --remote is set
CleanupScript-->>User: Report cleanup results
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/cleanup-branches.sh (1)
47-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueawk
$2truncates worktree paths containing spaces.
$2in awk splits on whitespace, so a worktree path like/Users/foo/my projectswould be captured as/Users/foo/my. Usesubstr($0, 10)to capture the full path after theworktreeprefix.♻️ Proposed fix for paths with spaces
done < <(git worktree list --porcelain | awk ' - /^worktree /{p=$2} + /^worktree /{p=substr($0, 10)} /^branch /{print p; print $2} ')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/cleanup-branches.sh` around lines 47 - 50, Update the awk processing in the worktree-list input to extract worktree paths with substr($0, 10) when handling /^worktree /, rather than using $2; keep branch extraction unchanged so paths containing spaces are passed intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/cleanup-branches.sh`:
- Around line 96-107: Update the remote-branch loop in the DO_REMOTE cleanup
block to call is_protected with each normalized branch name after removing the
origin/ prefix. Skip every protected branch, including DEFAULT_BRANCH, main, and
CURRENT_BRANCH, while preserving the existing HEAD exclusion and deletion
behavior for unprotected branches.
---
Nitpick comments:
In `@scripts/cleanup-branches.sh`:
- Around line 47-50: Update the awk processing in the worktree-list input to
extract worktree paths with substr($0, 10) when handling /^worktree /, rather
than using $2; keep branch extraction unchanged so paths containing spaces are
passed intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f59e4a8b-2ea6-45e9-b1f4-a57351ec0e02
📒 Files selected for processing (5)
.gitignoremise.local.tomlmise.tomlscripts/cleanup-branches.shscripts/refresh-install.ps1
💤 Files with no reviewable changes (3)
- mise.local.toml
- mise.toml
- scripts/refresh-install.ps1
…branch or its worktree git branch -D has no merge check, and the [gone] heuristic (needed since squash-merges break plain ancestry checks) doesn't guarantee the local branch's content actually matches what got merged -- a branch with an extra unpushed commit past what was squash-merged would be silently destroyed. Compare the branch's touched files against origin/ before removing either its worktree or its ref; skip with a warning if they differ. Verified against a sandboxed repo: a clean squash-merged branch still deletes, one with an unpushed follow-up commit is now left alone (ref and worktree both).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/cleanup-branches.sh`:
- Around line 79-96: Move the merge-base and touched-file safety verification
using merge_base and touched before the --dry-run reporting in
scripts/cleanup-branches.sh. Ensure dry-run outputs “would remove” only for
candidates that pass these checks, while preserving skip messages and leaving
verified branches untouched during dry-run.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d0e84fb-60ad-4f06-a7b9-cd1b5b889e05
📒 Files selected for processing (1)
scripts/cleanup-branches.sh
…g too --dry-run was reporting every candidate as removable before the content-diff safety check ran, so it could claim 'would delete' for a branch the real run would actually skip for possible unpushed work. Move the check ahead of the DRY_RUN branch so the preview matches real behavior.
5 tasks done
getappz
added a commit
that referenced
this pull request
Jul 25, 2026
…#322) remove_worktree_dir's existing retry loop and cmd /c rmdir fallback both only clear a transient in-use lock (e.g. item #302's rust-analyzer case). Neither touches a genuine ACL denial, which is item #267's actual failure mode: cargo's own target/*/.fingerprint/* files can end up ACL-restricted, not merely open, and no amount of retrying clears that. Adds one more fallback before giving up: icacls /grant <user>:F /T resets ownership access recursively, then one final remove_dir_all attempt. No-op (and thus never destructive) when the real problem was actually an in-use lock the earlier retries already cleared. Considered switching to git-parsec's shared_cache symlink strategy instead (share target/ across worktrees so there's nothing per-worktree to get ACL-locked in the first place) -- rejected: this codebase already hit and fixed the exact correctness bug that would reintroduce (item #139, cargo #12516/#14053/#7740 -- a shared CARGO_TARGET_DIR's fingerprint hash omits the worktree path, so two worktrees of different branches silently reuse each other's stale local crate artifacts). isolate_worktree_target_dir's per-worktree isolation is deliberate, not an oversight; sccache (item #133) already covers the safe part of cross-worktree cache sharing (registry deps, hash-keyed). Agentflare-Agent: claude-code_2-1-219_agent Agentflare-Branch: task/267 Agentflare-Item: 267
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
cleanup:branchesmise task +scripts/cleanup-branches.shto delete local branches/worktrees already merged intoorigin/mastermise.toml/mise.local.tomlinto a gitignored.mise.toml(root) +.mise/tasks/*.toml(split viatask_config.includes), since these are personal dev-workflow shortcuts, not shared project configrefresh-installflow (worktree detection +cargo install --path --force) with: pullmaster, then runagentflare dev-installTest plan
mise taskslists all 4 tasks correctly from the split included filesmise tasks info releaseresolves to.mise/tasks/release.tomlagentflare dev-installend-to-end against updated master, confirmed viaagentflare --versionSummary by CodeRabbit