-
Notifications
You must be signed in to change notification settings - Fork 50
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
base: main
Are you sure you want to change the base?
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
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
gtr->>Git: git worktree list
loop per worktree
gtr->>gtr: skip if detached HEAD or main-repo branch
gtr->>Git: check uncommitted/untracked/empty
alt clean and safe
gtr->>gh: gh pr list --state merged --head <branch>
gh->>GitHub: query merged PRs
GitHub-->>gh: return results
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: 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.
| # 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 |
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.
🧩 Analysis chain
🏁 Script executed:
# Search for gh repo view or gh auth in the entire bin/gtr file
rg -n 'gh (repo view|auth)' bin/gtrRepository: coderabbitai/git-worktree-runner
Length of output: 58
🏁 Script executed:
# Also examine the merged-mode block context (lines 883-972)
sed -n '883,972p' bin/gtr | cat -nRepository: coderabbitai/git-worktree-runner
Length of output: 3689
🏁 Script executed:
# Check if there's any gh initialization or auth check earlier in the file
rg -n 'gh ' bin/gtr | head -20Repository: coderabbitai/git-worktree-runner
Length of output: 400
Add gh authentication/repository check before using gh pr list.
The code only verifies that gh CLI is installed (line 889) but doesn't check authentication or that this is a GitHub repository. When gh pr list fails on line 940 (due to auth issues or network errors), the error is silently suppressed with || true, causing the code to report "no merged PR found" (line 979) instead of the actual root cause.
Add after line 892:
if ! gh repo view >/dev/null 2>&1; then
log_error "Not authenticated with GitHub or not a GitHub repository (run: gh auth login)"
exit 1
fiThis ensures early failure with clear diagnostics rather than misleading skip messages.
🤖 Prompt for AI Agents
In bin/gtr around lines 888 to 892, after verifying gh is installed, add a check
that gh can access the repository and the user is authenticated by invoking gh
repo view and, on failure, call log_error with a message like "Not authenticated
with GitHub or not a GitHub repository (run: gh auth login)" and exit 1; this
prevents later silent failures from commands like gh pr list and surfaces
authentication or repo issues early.
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.