-
Notifications
You must be signed in to change notification settings - Fork 54
feat: Add --merged flag to clean command for squash-merged PRs #64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Adds support for detecting and removing worktrees with merged PRs. This handles squash-merged branches which don't show as merged via `git branch --merged`. New flags for `git gtr clean`: - --merged: remove worktrees with merged PRs (uses gh CLI) - --yes, -y: skip confirmation prompts - --dry-run, -n: show what would be removed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughAdds a merged-mode to Changes
Sequence DiagramsequenceDiagram
actor User
participant gtr as gtr (script)
participant Git as Local Git
participant gh as GitHub CLI
participant GitHub as GitHub API
User->>gtr: gtr clean --merged [--dry-run] [--yes]
rect rgb(235,245,255)
Note over gtr: parse flags & validate environment
gtr->>gh: check gh availability
gtr->>Git: git fetch --prune origin
end
rect rgb(246,255,242)
Note over gtr: iterate worktrees and evaluate each branch
gtr->>Git: git worktree list
loop for each worktree
gtr->>gtr: skip detached HEAD or main branch
gtr->>Git: check uncommitted/untracked/empty
alt eligible
gtr->>gh: gh pr list --state merged --head <branch>
gh->>GitHub: query merged PRs
GitHub-->>gh: merged PRs response
gh-->>gtr: merged? (yes/no)
alt merged
opt not dry-run
gtr->>Git: git worktree remove <path>
gtr->>Git: git branch -d <branch>
end
end
end
end
end
gtr->>User: report summary (removed / skipped) or dry-run report
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
bin/gtr
🧰 Additional context used
📓 Path-based instructions (2)
bin/gtr
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
bin/gtr: Dispatch commands throughcmd_*functions inbin/gtr(case block lines 36‑77)
UpdateGTR_VERSIONon line 8 ofbin/gtrwhen releasing; this affectsgtr version/--versionoutputGlobal
set -einbin/gtr: guard non-critical commands with|| true
list --porcelainoutput must remain stable for scripting purposesUpdate version number in
bin/gtr(line 8,GTR_VERSIONconstant) when releasing a new version
Files:
bin/gtr
{bin/gtr,lib/**/*.sh,adapters/**/*.sh}
📄 CodeRabbit inference engine (.github/instructions/testing.instructions.md)
{bin/gtr,lib/**/*.sh,adapters/**/*.sh}: All commands must exit 0 (except intentional failures) and produce expected side-effects
No unquoted path errors; spaces must be handled in file paths
Hooks must run only once per creation/removal event
Files:
bin/gtr
🔇 Additional comments (10)
bin/gtr (10)
672-699: LGTM! Flag parsing is well-structured.The flag parsing correctly handles the new
--merged,--yes/-y, and--dry-run/-noptions with proper unknown flag validation. The structure is consistent with other commands in the script.
752-754: Good practice: Fetching with prune before PR checks.Fetching from origin with
--pruneensures the remote state is current before checking PR merge status. The graceful error handling with|| log_warnis appropriate.
781-794: Excellent safety checks to prevent data loss.The implementation correctly checks for both uncommitted changes (staged and unstaged) and untracked files before removing worktrees. This prevents accidental loss of work.
808-808: Good branch deletion strategy.The implementation correctly tries a safe delete (
-d) first, then falls back to force delete (-D) for squash-merged branches that appear unmerged locally. The|| trueguard properly handles failures due toset -e.
764-765: Proper handling of glob expansion.The glob pattern expansion with
"$base_dir/${prefix}"*is correctly implemented, and the-dcheck on line 765 safely handles cases where no directories match the pattern.
801-803: Clean dry-run implementation.The dry-run mode correctly simulates removal without performing actual changes, with clear
[dry-run]prefixes in the output. The counter tracking enables accurate summary reporting.
824-829: Clear and informative reporting.The summary output clearly distinguishes between dry-run and actual execution, providing useful feedback with both removed and skipped counts.
1200-1204: Help text accurately documents the new functionality.The help text clearly documents the new
--merged,--yes/-y, and--dry-run/-noptions with appropriate descriptions. The gh CLI requirement is properly mentioned.
8-8: Verify if the version number should be updated.The coding guidelines specify updating
GTR_VERSIONwhen releasing. This PR adds new functionality (--mergedflag with--yesand--dry-runoptions). If this PR is intended for release, the version should be bumped (e.g., to2.1.0for a minor feature addition).Based on coding guidelines, which state: "Update
GTR_VERSIONon line 8 ofbin/gtrwhen releasing; this affectsgtr version/--versionoutput"
796-821: Correct the line numbers and clarify the gh command's limitations in the--mergedmode logic.The review identifies real concerns, but references incorrect line numbers. The code in question is at lines 938-963 (in the
cmd_clean --mergedmode), not lines 796-821.The valid concerns are:
gh pr list --headlimitations: The GitHub CLI does not support the<owner>:<branch>syntax. For worktrees created from forked repositories where the PR exists in the upstream repo but the local branch name differs, this command will fail to find the PR.Ambiguous warning message (line 961): "Skipping $branch (no merged PR found)" conflates multiple failure modes:
- No PR exists for this branch
- PR exists but isn't merged
- The
ghcommand failed silently (due to|| trueon line 940)The
|| trueguard on line 940 is correct per coding guidelines and properly prevents script exit. However, the ambiguous message could be improved to clarify thatghauthentication and a GitHub-hosted repository are required:log_warn "Skipping $branch (no merged PR or not GitHub-hosted)".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
bin/gtr (1)
889-892: Platform-specific installation message.This issue was already identified in a previous review comment. The error message suggests
brew install gh, which is macOS-specific.
🧹 Nitpick comments (2)
bin/gtr (2)
939-963: Enhance error feedback whengh pr listfails.The
gh pr listcommand can fail for multiple reasons beyond "no merged PR exists" (e.g., not authenticated, network issues, repository not on GitHub, rate limiting). Currently, all failure modes result in an empty$pr_stateand the generic message "no merged PR found" (line 961), which could mislead users about the actual problem.Consider enhancing error handling to distinguish between different failure scenarios and provide actionable feedback.
🔎 Suggested improvement
# Check if branch has a merged PR on GitHub - local pr_state - pr_state=$(gh pr list --head "$branch" --state merged --json state --jq '.[0].state' 2>/dev/null || true) + local pr_state gh_error + if pr_state=$(gh pr list --head "$branch" --state merged --json state --jq '.[0].state' 2>&1); then + : # success + else + gh_error=$? + if echo "$pr_state" | grep -q "authentication\|not logged in"; then + log_warn "Skipping $branch (gh authentication required: run 'gh auth login')" + skipped=$((skipped + 1)) + continue + fi + pr_state="" + fi
946-952: Clarify that branch deletion is included in the removal.The confirmation prompt on line 946 asks "Remove worktree '$branch'?" but doesn't mention that the local branch will also be deleted (line 950). This could surprise users who expect only the worktree to be removed.
🔎 Suggested improvement
- elif [ "$yes_mode" -eq 1 ] || prompt_yes_no "Remove worktree '$branch'?"; then + elif [ "$yes_mode" -eq 1 ] || prompt_yes_no "Remove worktree and branch '$branch'?"; then
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
bin/gtr
🧰 Additional context used
📓 Path-based instructions (2)
bin/gtr
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
bin/gtr: Dispatch commands throughcmd_*functions inbin/gtr(case block lines 36‑77)
UpdateGTR_VERSIONon line 8 ofbin/gtrwhen releasing; this affectsgtr version/--versionoutputGlobal
set -einbin/gtr: guard non-critical commands with|| true
list --porcelainoutput must remain stable for scripting purposesUpdate version number in
bin/gtr(line 8,GTR_VERSIONconstant) when releasing a new version
Files:
bin/gtr
{bin/gtr,lib/**/*.sh,adapters/**/*.sh}
📄 CodeRabbit inference engine (.github/instructions/testing.instructions.md)
{bin/gtr,lib/**/*.sh,adapters/**/*.sh}: All commands must exit 0 (except intentional failures) and produce expected side-effects
No unquoted path errors; spaces must be handled in file paths
Hooks must run only once per creation/removal event
Files:
bin/gtr
🔇 Additional comments (4)
bin/gtr (4)
814-841: LGTM! Clean flag parsing implementation.The flag parsing follows established patterns in the codebase and properly handles both long and short forms (
--yes|-y,--dry-run|-n). The unknown flag guard ensures invalid options are caught early.
906-921: LGTM! Proper filtering of worktrees.The logic correctly:
- Iterates through worktree directories using the configured prefix
- Skips detached HEAD states (lines 912-916)
- Skips the main repository branch (lines 918-921)
These checks prevent unintended modifications to the primary repository.
924-936: LGTM! Comprehensive safety checks before removal.The implementation correctly prevents removal of worktrees with:
- Uncommitted changes (staged or unstaged)
- Untracked files
These safety checks protect users from accidental data loss and align with best practices for worktree management.
1356-1360: Clear documentation of new flags.The help text effectively documents the new
--mergedflag and its dependencies. The mention of "uses gh CLI" informs users of the external dependency.Consider adding a note about authentication requirements (e.g., "requires gh auth login") to help users troubleshoot, but this is optional since the runtime error message covers it.
helizaga
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks sound, left some comments
- Use platform-agnostic gh install message (https://cli.github.com/) - Add gh repo view check upfront to catch auth/network issues - Clarify prompt mentions branch deletion along with worktree removal 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
bin/gtr (1)
900-902: Add upfront validation for GitHub CLI authentication and repository.As suggested in the previous review, adding an early check for
gh repo viewwould catch auth/network issues before iterating worktrees, providing clearer error messages.🔎 Proposed fix
# Ensure gh CLI is available if ! command -v gh >/dev/null 2>&1; then log_error "GitHub CLI (gh) not found. Install from: https://cli.github.com/" exit 1 fi + # Verify gh is authenticated and this is a GitHub repo + if ! gh repo view >/dev/null 2>&1; then + log_error "Not a GitHub repo or gh not authenticated (run: gh auth status)" + exit 1 + fi + # Fetch latest from origin log_step "Fetching from origin..."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
bin/gtr
🧰 Additional context used
📓 Path-based instructions (2)
bin/gtr
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
bin/gtr: Dispatch commands throughcmd_*functions inbin/gtr(case block lines 36‑77)
UpdateGTR_VERSIONon line 8 ofbin/gtrwhen releasing; this affectsgtr version/--versionoutputGlobal
set -einbin/gtr: guard non-critical commands with|| true
list --porcelainoutput must remain stable for scripting purposesUpdate version number in
bin/gtr(line 8,GTR_VERSIONconstant) when releasing a new version
Files:
bin/gtr
{bin/gtr,lib/**/*.sh,adapters/**/*.sh}
📄 CodeRabbit inference engine (.github/instructions/testing.instructions.md)
{bin/gtr,lib/**/*.sh,adapters/**/*.sh}: All commands must exit 0 (except intentional failures) and produce expected side-effects
No unquoted path errors; spaces must be handled in file paths
Hooks must run only once per creation/removal event
Files:
bin/gtr
🧠 Learnings (1)
📚 Learning: 2025-12-29T18:46:36.983Z
Learnt from: helizaga
Repo: coderabbitai/git-worktree-runner PR: 64
File: bin/gtr:746-750
Timestamp: 2025-12-29T18:46:36.983Z
Learning: When error messages reference missing CLI tools (such as gh for GitHub CLI), use platform-agnostic installation guidance by directing users to the official website (e.g., https://cli.github.com/) instead of platform-specific package managers (brew, apt, choco) to ensure the guidance works across all operating systems. Apply this messaging to the bin/gtr script where applicable.
Applied to files:
bin/gtr
🔇 Additional comments (3)
bin/gtr (3)
814-841: LGTM!Flag parsing follows the established patterns in other commands (
cmd_create,cmd_remove) with proper unknown flag handling and short aliases.
944-969: LGTM!The merged PR detection and removal logic handles edge cases well:
- Dry-run mode previews without changes
- Interactive confirmation shows the branch name
- Branch deletion uses
-dthen-Dfallback, appropriate for squash-merged branches- Errors are logged without aborting the loop
1362-1366: LGTM!Help text accurately documents the new options and notes the
ghCLI dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
bin/gtr (2)
960-963: Consider reducing noise for branches without merged PRs.The
log_warn "Skipping $branch (no merged PR found)"will trigger for every worktree that has an active (unmerged) PR or no PR at all. This could be noisy in repos with many active feature branches.🔎 Potential improvement: only warn when verbose or debugging
You could silently skip branches without merged PRs and only count them, then report the total at the end:
else - log_warn "Skipping $branch (no merged PR found)" skipped=$((skipped + 1)) fiThe final summary already shows the skipped count, which may be sufficient.
948-955: Consider preserving stderr for troubleshooting failed removals.Suppressing stderr with
2>/dev/nullongit worktree removehides the reason for failure. When the removal fails, users only see "Failed to remove worktree" without context.🔎 Proposed improvement
- if git worktree remove "$dir" 2>/dev/null; then + if git worktree remove "$dir"; thenAlternatively, capture and display the error:
local remove_err if remove_err=$(git worktree remove "$dir" 2>&1); then # success path else log_error "Failed to remove worktree: $branch" log_error "$remove_err" fi
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
bin/gtr
🧰 Additional context used
📓 Path-based instructions (2)
bin/gtr
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
bin/gtr: Dispatch commands throughcmd_*functions inbin/gtr(case block lines 36‑77)
UpdateGTR_VERSIONon line 8 ofbin/gtrwhen releasing; this affectsgtr version/--versionoutputGlobal
set -einbin/gtr: guard non-critical commands with|| true
list --porcelainoutput must remain stable for scripting purposesUpdate version number in
bin/gtr(line 8,GTR_VERSIONconstant) when releasing a new version
Files:
bin/gtr
{bin/gtr,lib/**/*.sh,adapters/**/*.sh}
📄 CodeRabbit inference engine (.github/instructions/testing.instructions.md)
{bin/gtr,lib/**/*.sh,adapters/**/*.sh}: All commands must exit 0 (except intentional failures) and produce expected side-effects
No unquoted path errors; spaces must be handled in file paths
Hooks must run only once per creation/removal event
Files:
bin/gtr
🧠 Learnings (1)
📚 Learning: 2025-12-29T18:46:36.983Z
Learnt from: helizaga
Repo: coderabbitai/git-worktree-runner PR: 64
File: bin/gtr:746-750
Timestamp: 2025-12-29T18:46:36.983Z
Learning: When error messages reference missing CLI tools (such as gh for GitHub CLI), use platform-agnostic installation guidance by directing users to the official website (e.g., https://cli.github.com/) instead of platform-specific package managers (brew, apt, choco) to ensure the guidance works across all operating systems. Apply this messaging to the bin/gtr script where applicable.
Applied to files:
bin/gtr
🔇 Additional comments (4)
bin/gtr (4)
814-841: LGTM!Flag parsing is well-structured, consistent with other commands in the file, and correctly handles unknown flags.
923-936: Good safeguards for uncommitted changes and untracked files.The checks for uncommitted changes (staged and unstaged) and untracked files properly protect against accidental data loss.
894-896: Good: Fetch with proper error handling.The
git fetch origin --prunewith|| log_warnproperly guards this non-critical command per the coding guidelines, ensuring the script continues even if fetch fails.
1356-1360: LGTM!Help text is clear, accurately describes the new flags, and properly notes the
ghCLI dependency.
- Add gh repo view check to catch auth/network issues upfront - Capture stderr on worktree removal to show failure reasons - Remove noisy warning for branches without merged PRs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
bin/gtr (2)
837-839: Consider warning or erroring on unexpected positional arguments.The
cmd_cleancommand doesn't expect positional arguments, but line 838 silently ignores them. This could hide user mistakes (e.g.,git gtr clean my-branchwould silently ignoremy-branch). Consider adding a warning or error message for unexpected arguments to improve user experience.🔎 Optional improvement
*) - shift + log_warn "Unexpected argument ignored: $1" + shift ;;
913-914: Add safeguard for empty glob expansion.The glob pattern
"$base_dir/${prefix}"*at line 913 will cause the loop to iterate once with the literal pattern string if no directories match. While the[ -d "$dir" ]check at line 914 catches this, it's more idiomatic to prevent the issue upfront.🔎 Recommended safeguard
# Iterate through worktree directories + shopt -s nullglob for dir in "$base_dir/${prefix}"*; do [ -d "$dir" ] || continueThis sets the
nullgloboption to make the glob expand to nothing when there are no matches, avoiding the unnecessary iteration with the literal pattern.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
bin/gtr
🧰 Additional context used
📓 Path-based instructions (2)
bin/gtr
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
bin/gtr: Dispatch commands throughcmd_*functions inbin/gtr(case block lines 36‑77)
UpdateGTR_VERSIONon line 8 ofbin/gtrwhen releasing; this affectsgtr version/--versionoutputGlobal
set -einbin/gtr: guard non-critical commands with|| true
list --porcelainoutput must remain stable for scripting purposes
bin/gtr: Updatebin/gtrversion constant (GTR_VERSION) when releasing a new version
Update help text inbin/gtrby searching for 'Available editors:' incmd_helpfunction andload_editor_adapterfunction when adding a new editor adapter
Update help text inbin/gtrby searching for 'Available AI tools:' incmd_helpfunction andload_ai_adapterfunction when adding a new AI tool adapter
Files:
bin/gtr
{bin/gtr,lib/**/*.sh,adapters/**/*.sh}
📄 CodeRabbit inference engine (.github/instructions/testing.instructions.md)
{bin/gtr,lib/**/*.sh,adapters/**/*.sh}: All commands must exit 0 (except intentional failures) and produce expected side-effects
No unquoted path errors; spaces must be handled in file paths
Hooks must run only once per creation/removal event
Files:
bin/gtr
🧠 Learnings (1)
📚 Learning: 2025-12-29T18:46:36.983Z
Learnt from: helizaga
Repo: coderabbitai/git-worktree-runner PR: 64
File: bin/gtr:746-750
Timestamp: 2025-12-29T18:46:36.983Z
Learning: When error messages reference missing CLI tools (such as gh for GitHub CLI), use platform-agnostic installation guidance by directing users to the official website (e.g., https://cli.github.com/) instead of platform-specific package managers (brew, apt, choco) to ensure the guidance works across all operating systems. Apply this messaging to the bin/gtr script where applicable.
Applied to files:
bin/gtr
🔇 Additional comments (2)
bin/gtr (2)
888-982: Excellent safety checks and error handling throughout the merged-mode cleanup.The implementation includes comprehensive safeguards:
- Early validation of
ghCLI availability and authentication (lines 888-899)- Exclusion of detached HEAD and main branch worktrees (lines 919-928)
- Protection for worktrees with uncommitted changes or untracked files (lines 930-943)
- Captured stderr from worktree removal for better diagnostics (lines 955-967)
- Appropriate use of dry-run and confirmation prompts (lines 950-971)
The error handling follows coding guidelines by guarding non-critical commands with
|| trueor|| log_warn.
1366-1370: Clear and well-documented help text for new flags.The help text clearly describes the new
--merged,--yes/-y, and--dry-run/-nflags, and appropriately mentions the GitHub CLI requirement for the--mergedfunctionality.
Summary
Adds support for detecting and removing worktrees with merged PRs, handling squash-merged branches which don't show as merged via
git branch --merged.Problem
When using squash merge (common in GitHub workflows),
git branch --mergeddoesn't detect these branches as merged because the commit SHAs differ. This leaves stale worktrees that can't be cleaned up with the existinggit gtr cleancommand.Solution
New flags for
git gtr clean:--merged: remove worktrees with merged PRs (uses GitHub CLI)--yes, -y: skip confirmation prompts--dry-run, -n: show what would be removed without removingThe
--mergedflag usesgh pr list --head <branch> --state mergedto detect if a branch was merged via PR, regardless of merge strategy.Usage
Requirements
gh) must be installed and authenticatedTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.