feat(clean): support closed PR cleanup#183
Merged
Merged
Conversation
Assisted-by: pi:gpt-5.5 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: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughExtends ChangesClosed PR/MR cleanup feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
🧹 Nitpick comments (2)
lib/commands/clean.sh (1)
186-188: ⚡ Quick winShort-circuit local skip cases before hitting the provider.
_clean_branch_matches_pr_stateruns before_clean_should_skip, so active, detached, or dirty worktrees still pay forgh/glablookups even though they cannot be removed. Pushing the cheap local eligibility check ahead of the provider query would reduce unnecessary API traffic and auth/rate-limit churn in larger cleanup runs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/commands/clean.sh` around lines 186 - 188, The local eligibility check `_clean_should_skip` is currently performed after the provider query `_clean_branch_matches_pr_state`, causing unnecessary API calls; swap the order so `_clean_should_skip "$dir" "$branch" "$force" "$active_worktree_path"` runs first and returns early when true, and only then call `_clean_branch_matches_pr_state "$provider" "$branch" "$target_ref" "$branch_tip" "$merged_mode" "$closed_mode"` when local checks pass; ensure any surrounding conditionals/returns are adjusted so the skip path behaves identically (no provider call) and the original branch-matching logic still executes when not skipped.lib/provider.sh (1)
176-185: ⚡ Quick winVerify the GitLab SHA guard against a single MR field.
The reused-branch-name guard is doing a raw substring search across the entire JSON payload. Any matching
"sha"or"head_sha"anywhere in that response can satisfy the check, which makes this destructive path depend on an imprecise match. Please verify which exact fieldglab mr list --output jsonexposes for the source-head commit and compare that field structurally instead of grepping the whole blob.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/provider.sh` around lines 176 - 185, The current substring match against mr_result using branch_tip is unsafe; update the check to parse the JSON structurally (e.g. with jq) instead of grepping the whole blob: read mr_result, select the MR object that matches the source branch (or MR iid) and extract its explicit commit field (the exact field name returned by glab, e.g. .head_sha or .sha—verify with glab mr list --output json), then compare that extracted value to branch_tip and return 0/1 accordingly; replace the case block that inspects compact_result with this jq-based extraction and a direct string comparison against branch_tip (referencing variables branch_tip and mr_result and the current case block for location).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/commands/clean.sh`:
- Around line 186-188: The local eligibility check `_clean_should_skip` is
currently performed after the provider query `_clean_branch_matches_pr_state`,
causing unnecessary API calls; swap the order so `_clean_should_skip "$dir"
"$branch" "$force" "$active_worktree_path"` runs first and returns early when
true, and only then call `_clean_branch_matches_pr_state "$provider" "$branch"
"$target_ref" "$branch_tip" "$merged_mode" "$closed_mode"` when local checks
pass; ensure any surrounding conditionals/returns are adjusted so the skip path
behaves identically (no provider call) and the original branch-matching logic
still executes when not skipped.
In `@lib/provider.sh`:
- Around line 176-185: The current substring match against mr_result using
branch_tip is unsafe; update the check to parse the JSON structurally (e.g. with
jq) instead of grepping the whole blob: read mr_result, select the MR object
that matches the source branch (or MR iid) and extract its explicit commit field
(the exact field name returned by glab, e.g. .head_sha or .sha—verify with glab
mr list --output json), then compare that extracted value to branch_tip and
return 0/1 accordingly; replace the case block that inspects compact_result with
this jq-based extraction and a direct string comparison against branch_tip
(referencing variables branch_tip and mr_result and the current case block for
location).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4abc4c8f-101f-4138-870c-19901675002c
📒 Files selected for processing (13)
README.mdcompletions/_git-gtrcompletions/git-gtr.fishcompletions/gtr.bashdocs/configuration.mdlib/commands/clean.shlib/commands/doctor.shlib/commands/help.shlib/provider.shscripts/generate-completions.shtests/cmd_clean.batstests/cmd_help.batstests/provider.bats
Assisted-by: pi:gpt-5.5 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/provider.sh`:
- Around line 138-158: The no-jq fallback currently extracts only "sha" and
"diff_refs.head_sha" but misses a top-level "head_sha"; update the while-loop
parsing in provider.sh to also extract and compare a top-level "head_sha" (e.g.,
add a head_sha_field from "$object" similar to sha_field and test [
"$head_sha_field" = "$branch_tip" ]), checking it before inspecting diff_refs,
so objects exposing only "head_sha" are correctly matched; ensure you reference
the existing variables (object, sha_field, diff_refs, branch_tip) and return 0
on match to mirror the jq branch behavior.
🪄 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: Enterprise
Run ID: fb6959cf-67f0-4006-a577-3eeebefdb6bb
📒 Files selected for processing (4)
lib/commands/clean.shlib/provider.shtests/cmd_clean.batstests/provider.bats
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/commands/clean.sh
Assisted-by: pi:gpt-5.5 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
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
git gtr clean --closedfor closed PR/MR worktree cleanup.--merged --closedas merged OR closed.Motivation
Clean up worktrees for rejected/closed PRs as well as merged PRs.
Type of Change
Testing
npm exec --yes bats -- tests/provider.bats tests/cmd_clean.bats tests/cmd_help.bats tests/completion.bats./scripts/generate-completions.sh --checkbash -n lib/provider.sh lib/commands/clean.sh lib/commands/help.sh lib/commands/doctor.sh scripts/generate-completions.sh completions/gtr.bash completions/_git-gtr completions/git-gtr.fish tests/provider.bats tests/cmd_clean.bats tests/cmd_help.batstmp_global=$(mktemp) && GIT_CONFIG_GLOBAL=$tmp_global npm exec --yes bats -- tests/; rc=$?; rm -f $tmp_global; exit $rcManual 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 --closed,git gtr clean --merged --closed --to mainTest Steps
Expected behavior:
--closedremoves worktrees with closed PRs/MRs;--merged --closedremoves merged OR closed matches.Actual behavior: Matches expected behavior in targeted and full-suite tests.
Breaking Changes
Checklist
Before submitting this PR, please check:
git gtr(production) and./bin/gtr(development)Additional Context
--to <ref>applies to both--mergedand--closedcleanup.License Acknowledgment
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache License 2.0.
PR opened by gpt-5.5 high on pi
Summary by CodeRabbit
New Features
Documentation
Tests
Chores