feat(clean): add --to filter for git gtr clean --merged#167
Open
scarf005 wants to merge 2 commits intocoderabbitai:mainfrom
Open
feat(clean): add --to filter for git gtr clean --merged#167scarf005 wants to merge 2 commits intocoderabbitai:mainfrom
--to filter for git gtr clean --merged#167scarf005 wants to merge 2 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>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
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 --merged and --to main → target_ref='main'
CLI->>Cleaner: invoke _clean_merged(target_ref='main')
Cleaner->>Cleaner: iterate worktrees (compute branch_tip)
Cleaner->>ProviderCheck: check_branch_merged(provider, branch, 'main', branch_tip)
ProviderCheck->>Provider: query PRs/MRs filtered by base/target='main' (and headRefOid==branch_tip if provided)
Provider-->>ProviderCheck: return count/status of merged PRs/MRs
ProviderCheck-->>Cleaner: return merged? (exit 0) / not merged
Cleaner->>Cleaner: remove matching worktrees (only those merged into 'main')
Cleaner-->>User: report cleanup results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
Comment on lines
82
to
84
| '--merged[Remove worktrees with merged PRs/MRs]' \ | ||
| '--to[Only remove worktrees for PRs/MRs merged into this ref]:ref:' \ | ||
| '--yes[Skip confirmation prompts]' \ |
There was a problem hiding this comment.
Generated completion artifacts are out-of-sync with CI expectations.
CI reports stale completion generation for this command surface. Please re-run completion generation and commit all produced completion files in the same change to clear the stale check.
As per coding guidelines, completions/*.{bash,fish} requires updating all three completion files (completions/gtr.bash, completions/_git-gtr, completions/git-gtr.fish) when adding commands or flags.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@completions/_git-gtr` around lines 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.
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>
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
1 participant
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.Documentation
--to <ref>option and its usage.Shell Completions
--tooption across shells.Tests
--to <ref>behavior and related merged-check logic.