feat(clean): add --to filter for git gtr clean --merged#167
Open
scarf005 wants to merge 5 commits intocoderabbitai:mainfrom
Open
feat(clean): add --to filter for git gtr clean --merged#167scarf005 wants to merge 5 commits intocoderabbitai:mainfrom
--to filter for git gtr clean --merged#167scarf005 wants to merge 5 commits intocoderabbitai:mainfrom
Conversation
Assisted-by: gpt-5.4-high on opencode Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
|
Warning Rate limit exceeded
Your PR has hit rate limit. Contact your admin to purchase credits for running PR reviews or try again in 6 minutes and 11 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as cmd_clean
participant Cleaner as _clean_merged
participant ProviderCheck as check_branch_merged
participant Provider as GitHub/GitLab
User->>CLI: git gtr clean --merged --to main
CLI->>CLI: parse flags → merged=true, target_ref="main"
CLI->>Cleaner: _clean_merged(target_ref="main")
Cleaner->>Cleaner: for each worktree: compute branch, branch_tip
Cleaner->>ProviderCheck: check_branch_merged(provider, branch, "main", branch_tip)
ProviderCheck->>Provider: query PRs/MRs filtered by base/target="main" and head==branch_tip
Provider-->>ProviderCheck: return match count / MR list
ProviderCheck-->>Cleaner: return merged? (0) or not (1)
Cleaner->>Cleaner: remove worktrees only when merged into "main"
Cleaner-->>User: report cleanup results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 2
🧹 Nitpick comments (1)
tests/cmd_clean.bats (1)
147-159: Strengthen this test with a non-matching target branch assertion.Current test validates the positive path. Add a second worktree that should not be removed to lock in filter behavior and prevent false positives.
Suggested test hardening
`@test` "cmd_clean --merged --to filters by target ref" { create_test_worktree "merged-to-main" + create_test_worktree "merged-to-develop" _clean_detect_provider() { printf "github"; } ensure_provider_cli() { return 0; } - check_branch_merged() { [ "$2" = "merged-to-main" ] && [ "$3" = "main" ]; } + check_branch_merged() { + [ "$3" = "main" ] && [ "$2" = "merged-to-main" ] + } run_hooks_in() { return 0; } run_hooks() { return 0; } run cmd_clean --merged --to main --yes [ "$status" -eq 0 ] [ ! -d "$TEST_WORKTREES_DIR/merged-to-main" ] + [ -d "$TEST_WORKTREES_DIR/merged-to-develop" ] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cmd_clean.bats` around lines 147 - 159, The test "cmd_clean --merged --to filters by target ref" currently only verifies removal of the matching worktree; add a second non-matching worktree (e.g., create_test_worktree "merged-to-feature") and stub the same helpers (_clean_detect_provider, ensure_provider_cli, check_branch_merged, run_hooks_in, run_hooks) so that check_branch_merged returns true only for branch "merged-to-main" -> "main" and false for the non-matching pair, then after running run cmd_clean --merged --to main --yes assert status is 0, assert the "merged-to-main" directory was removed and assert the "merged-to-feature" directory still exists to lock in the negative case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@completions/_git-gtr`:
- Around line 82-84: The completions for the new flags (--merged, --to, --yes)
are stale; re-run the project's completion-generation script/tool to regenerate
shell completions for the gtr command, ensure the generated artifacts for all
three completion targets (gtr.bash, _git-gtr, git-gtr.fish) include the new
flags/options, and commit those regenerated files together in the same change so
CI no longer reports stale completions.
In `@lib/commands/clean.sh`:
- Around line 149-157: After parsing args in lib/commands/clean.sh, validate
that --to (_arg_to -> target_ref) is only used when --merged (_arg_merged ->
merged_mode) is enabled: if target_ref is non-empty and merged_mode is false/0,
print a clear error message and exit non-zero; implement this check near where
merged_mode and target_ref are set (after parse_args) so the script rejects the
unsupported combination early rather than silently ignoring --to.
---
Nitpick comments:
In `@tests/cmd_clean.bats`:
- Around line 147-159: The test "cmd_clean --merged --to filters by target ref"
currently only verifies removal of the matching worktree; add a second
non-matching worktree (e.g., create_test_worktree "merged-to-feature") and stub
the same helpers (_clean_detect_provider, ensure_provider_cli,
check_branch_merged, run_hooks_in, run_hooks) so that check_branch_merged
returns true only for branch "merged-to-main" -> "main" and false for the
non-matching pair, then after running run cmd_clean --merged --to main --yes
assert status is 0, assert the "merged-to-main" directory was removed and assert
the "merged-to-feature" directory still exists to lock in the negative case.
🪄 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
Run ID: 4910c592-4cf8-4508-9cbc-4c8b29a846d6
📒 Files selected for processing (10)
README.mdcompletions/_git-gtrcompletions/git-gtr.fishcompletions/gtr.bashlib/commands/clean.shlib/commands/help.shlib/provider.shtests/cmd_clean.batstests/cmd_help.batstests/provider.bats
Avoid dirty skip noise for non-merged worktrees and reject --to without --merged. Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/provider.sh`:
- Around line 116-124: The pr_matches logic in lib/provider.sh uses gh pr list
and glab mr list but only inspects the first page, causing false negatives for
reused branch names; update each gh pr list call that assigns pr_matches (the
blocks referencing pr_matches and branch_tip) to include a sufficiently large
--limit (or use --all) and implement pagination (or loop) so all pages are
examined, and likewise change glab mr list invocations to use --all or a paging
loop; ensure the selection/filtering (the jq/map/select parts relying on
headRefOid and state) remains the same so the code still counts merged PRs/MRs
but across the complete result set.
- Around line 114-118: Normalize the target_ref variable to a plain branch name
before passing it to provider filters: detect and strip refs/heads/ and remote
prefixes (e.g., origin/) or resolve via git rev-parse --abbrev-ref to obtain the
simple branch name, then use that normalized variable in the gh pr list calls
(the target_ref used with --base and the branch_tip comparison) and in the glab
--target-branch usage mentioned around the later block (lines near the other
gh/glab calls). Ensure normalization happens once right after parsing --to and
before any use in the pr/MR queries so callers can still pass refs like
refs/heads/main or origin/main safely.
🪄 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
Run ID: 0dd3a9e3-9de4-4594-925f-765a49904572
📒 Files selected for processing (2)
lib/provider.shtests/provider.bats
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/provider.bats
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
2 participants
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.
Pull Request
Description
Adds
--to <Ref>option togit gtr clean --mergedand tightens merged cleanup so only worktrees whose current branch tip matches a merged PR/MR for the requested target ref are considered.Motivation
For teams that merge PRs into staging or release branches before
main,git gtr clean --merged --to mainmust not treat staging-only merges as eligible for deletion. This also needs to avoid false positives when branch names are reused.Type of Change
Testing
bats tests/cmd_clean.bats tests/provider.bats tests/cmd_help.bats tests/completion.batsshellcheck lib/commands/clean.sh lib/provider.sh lib/commands/help.sh./bin/gtr cleangit gtr clean./scripts/generate-completions.sh --checkManual Testing Checklist
Tested on:
Core functionality tested:
git gtr new <branch>- Create worktreegit gtr go <branch>- Navigate to worktreegit gtr editor <branch>- Open in editor (if applicable)git gtr ai <branch>- Start AI tool (if applicable)git gtr rm <branch>- Remove worktreegit gtr list- List worktreesgit gtr config- Configuration commands (if applicable)git gtr clean --merged --to <ref>Test Steps
dummy.git gtr clean --merged --to main.main.Expected behavior:
Only worktrees whose current branch tip belongs to a merged PR/MR into the specified target ref are removed.
Actual behavior:
git gtr clean --merged --to mainignores non-merged worktrees, rejects--towithout--merged, and does not emit dirty-skip warnings for branches that were never eligible for deletion.Breaking Changes
Checklist
Before submitting this PR, please check:
git gtr(production) and./bin/gtr(development)Additional Context
This also fixes the completion generator so regenerated bash/zsh/fish completions keep the
--tooption.License Acknowledgment
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache License 2.0.
Summary by CodeRabbit
New Features
--to <ref>forgit gtr clean --mergedto restrict cleanup to worktrees whose PRs/MRs were merged into the specified base ref;--tois only valid with--merged.Documentation
--to <ref>and its usage.Shell Completions
--toacross shells.Tests
--to <ref>behavior and merged-check logic.