feat: cross-repo worktree cleanup with squash-merge detection#2616
feat: cross-repo worktree cleanup with squash-merge detection#2616marcusquinn merged 1 commit intomainfrom
Conversation
Root cause: worktrees accumulated indefinitely because: 1. pulse-wrapper only cleaned aidevops repo, not other managed repos 2. cmd_clean missed squash-merged PRs (git branch --merged only detects traditional merges, not squash merges) 3. Dirty worktrees were always skipped, even when the PR was confirmed merged (dirty state = abandoned WIP from workers) 4. No cleanup of node_modules/.next/.turbo before removal, causing git worktree remove to take minutes per worktree Fix: - pulse-wrapper.sh: cleanup_worktrees() now iterates all repos from repos.json (skipping local_only), running worktree-helper in each - worktree-helper.sh: cmd_clean() gains --force-merged flag that: - Uses gh pr list to detect squash-merged PRs - Force-removes dirty worktrees when PR is confirmed merged - Cleans node_modules/.next/.turbo before git worktree remove Evidence: aidevops had 97 stale worktrees, awardsapp had 58. Both repos use squash merge. The old cleanup only ran against aidevops and missed all squash-merged branches.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the worktree management system by addressing long-standing issues that led to worktree accumulation. It broadens the scope of cleanup operations to cover all managed repositories, introduces robust detection for GitHub squash merges, and refines the handling of dirty worktrees to ensure that abandoned development branches are properly removed. These changes aim to maintain a cleaner development environment and prevent disk space wastage by streamlining the worktree lifecycle. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
WalkthroughEnhanced worktree cleanup across multiple repositories by introducing Changes
Sequence DiagramsequenceDiagram
participant Pulse as pulse-wrapper.sh
participant Repos as repos.json
participant Helper as worktree-helper.sh
participant Git as git
participant GH as gh CLI
participant FS as File System
Pulse->>Repos: Load repos list (with fallback)
Pulse->>Pulse: Filter valid git paths
loop For each repository
Pulse->>Helper: cmd_clean --force-merged
Helper->>Git: List merged branches
Helper->>GH: gh pr list (detect squash-merged)
GH-->>Helper: Merged PR heads
Helper->>Helper: Identify merged worktrees
Helper->>Git: Check for uncommitted changes
alt Merged && Dirty && Force-Merged
Helper->>FS: Remove node_modules/.next/.turbo
Helper->>FS: Remove aidesvops runtime
Helper->>Git: git worktree remove --force
else Merged && !Dirty
Helper->>Git: git worktree remove
else Not Merged
Helper->>Helper: Skip worktree
end
Helper->>Git: Branch deletion
Helper-->>Pulse: Removal count
end
Pulse->>Pulse: Aggregate total removals
Pulse-->>Pulse: Log results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Feb 28 18:12:03 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the worktree cleanup process to operate across multiple repositories and to correctly identify squash-merged branches. However, it introduces a high-severity issue where build artifacts in the main worktree can be unintentionally deleted if its current branch is merged. A medium-severity regex injection vulnerability in the branch matching logic is also identified, with a comprehensive suggestion provided to address it, improving both security and performance. Additionally, the review suggests improving debuggability by removing several instances of 2>/dev/null suppression, aligning with repository rules.
| rm -rf "$worktree_path/node_modules" 2>/dev/null || true | ||
| rm -rf "$worktree_path/.next" 2>/dev/null || true | ||
| rm -rf "$worktree_path/.turbo" 2>/dev/null || true |
There was a problem hiding this comment.
The cmd_clean function iterates over all worktrees returned by git worktree list. It does not distinguish between the main worktree and linked worktrees. If the main worktree is currently on a branch that has been merged (and is not the default branch), the script will proceed to "clean up" heavy directories like node_modules, .next, and .turbo using rm -rf before attempting to remove the worktree. While git worktree remove will fail for the main worktree, the rm -rf commands will succeed, leading to the loss of build artifacts and dependencies in the main repository.
Remediation: The script should identify and skip the main worktree. This can be done by checking if the worktree path is the same as the base repository path or by skipping the first entry in the git worktree list --porcelain output.
| # GitHub squash merges create a new commit — the original branch is NOT | ||
| # an ancestor of the target, so git branch --merged misses it. The remote | ||
| # branch may still exist if "auto-delete head branches" is off. | ||
| elif [[ -n "$merged_pr_branches" ]] && echo "$merged_pr_branches" | grep -qx "$worktree_branch"; then |
There was a problem hiding this comment.
This line has two issues:
- Regex Injection Vulnerability (Medium Severity): The
grep -qx "$worktree_branch"command treats the branch name as a regular expression, which can lead to unintended matches and potential deletion of unrelated worktrees if the branch name contains regex metacharacters. - Performance Issue: Using
grepinside a loop is inefficient.
To address both, it's recommended to use grep -Fqx to treat the branch name as a literal string, and for performance, consider loading merged branch names into a bash associative array for O(1) lookup. The suggested code below uses an associative array lookup, which also removes 2>/dev/null from the gh command, improving debuggability.
| elif [[ -n "$merged_pr_branches" ]] && echo "$merged_pr_branches" | grep -qx "$worktree_branch"; then | |
| elif [[ -v "merged_pr_lookup[$worktree_branch]" ]]; then |
| if [[ -f "$repos_json" ]] && command -v jq &>/dev/null; then | ||
| # Iterate all repos, skip local_only (no GitHub remote for PR detection) | ||
| local repo_paths | ||
| repo_paths=$(jq -r '.[] | select(.local_only != true) | .path' "$repos_json" 2>/dev/null || echo "") |
There was a problem hiding this comment.
Avoid redirecting stderr to /dev/null. If repos.json is malformed or jq fails for another reason, the error message will be suppressed, making debugging difficult. The || echo "" already provides a safe fallback if the command fails, so stderr can be allowed to print for better diagnostics.
| repo_paths=$(jq -r '.[] | select(.local_only != true) | .path' "$repos_json" 2>/dev/null || echo "") | |
| repo_paths=$(jq -r '.[] | select(.local_only != true) | .path' "$repos_json" || echo "") |
References
- When reporting errors for failed file operations in shell scripts, such as 'jq' writes, include the file path in the error message. Avoid suppressing stderr with '2>/dev/null' to ensure that diagnostic information about malformed files or write failures is visible.
| [[ ! -d "$repo_path/.git" ]] && continue | ||
|
|
||
| local cleaned_output | ||
| cleaned_output=$(git -C "$repo_path" worktree list 2>/dev/null | wc -l) |
There was a problem hiding this comment.
Avoid redirecting stderr to /dev/null. While a check for the .git directory exists, the git command could still fail for other reasons (e.g., a corrupted repository). Suppressing stderr hides these potentially important errors, hindering debugging.
| cleaned_output=$(git -C "$repo_path" worktree list 2>/dev/null | wc -l) | |
| cleaned_output=$(git -C "$repo_path" worktree list | wc -l) |
References
- When using git commands (like 'init', 'remote') in shell scripts, use the '-q' flag to suppress standard output instead of '2>/dev/null', ensuring that error messages on stderr remain visible for debugging.
| @@ -698,6 +703,13 @@ cmd_clean() { | |||
| # Fetch to get current remote branch state (detects deleted branches) | |||
| git fetch --prune origin 2>/dev/null || true | |||
There was a problem hiding this comment.
Avoid redirecting stderr to /dev/null. If git fetch fails (e.g., due to network or authentication issues), the error message will be suppressed, making debugging difficult. The || true already prevents the script from exiting on failure, so it's safe to let error messages be displayed.
| git fetch --prune origin 2>/dev/null || true | |
| git fetch --prune origin || true |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
| remove_flag="--force" | ||
| fi | ||
| # shellcheck disable=SC2086 | ||
| if ! git worktree remove $remove_flag "$worktree_path" 2>/dev/null; then |
There was a problem hiding this comment.
Avoid redirecting stderr to /dev/null. The error message from git worktree remove can be more informative than the custom message, especially if the failure is due to something other than uncommitted changes (e.g., permissions). Allowing the original error to be displayed will aid in debugging.
| if ! git worktree remove $remove_flag "$worktree_path" 2>/dev/null; then | |
| if ! git worktree remove $remove_flag "$worktree_path"; then |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
1 participant
Summary
cleanup_worktrees()now iterates ALL repos fromrepos.json(skippinglocal_only), not just the aidevops repocmd_clean()gains--force-mergedflag that detects squash merges viagh pr listand force-removes dirty worktrees when the PR is confirmed mergednode_modules/.next/.turbobeforegit worktree removeto avoid multi-minute deletionsRoot Cause
Worktrees accumulated indefinitely (97 in aidevops, 58 in webapp) due to four compounding failures:
cleanup_worktrees()only ran against the aidevops repo — never webapp or other managed reposgit branch --mergedonly detects traditional merges. GitHub squash merges create a new commit that is NOT an ancestor of the original branch, so they were invisible to cleanupnode_moduleswith 100k+ files, makinggit worktree removetake minutes. No pre-cleanup of these directories was doneTesting
Summary by CodeRabbit
New Features
Improvements