1.0.2 - #53
Conversation
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change adds configurable worktree removal, merge detection, and ChangesWorktree and branch cleanup
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant cmd_clean
participant Git
participant WorktreeRemover
Operator->>cmd_clean: Run clean with selectors
cmd_clean->>Git: Fetch and prune origin metadata
cmd_clean->>Git: Detect gone or merged branches
cmd_clean->>WorktreeRemover: Remove registered worktrees
cmd_clean->>Git: Delete associated branches
cmd_clean-->>Operator: Report results and failure status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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 (2)
git-trees (1)
32-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the intentional unquoted expansion.
$TREES_RM_CMDis unquoted so that a value such asrm -rfsplits into command and argument. That is deliberate, but it reads like a quoting bug. The repository guideline asks for why-comments on non-obvious correctness-sensitive lines.📝 Proposed comment
_remove_worktree() { local path="$1" if [ -n "$TREES_RM_CMD" ]; then + # Unquoted on purpose: TREES_RM_CMD carries its own arguments ("rm -rf"), + # so it must word-split. "$path" stays quoted. $TREES_RM_CMD "$path" else git worktree remove "$path" fi }As per coding guidelines: "Comments should explain why, especially for non-obvious correctness-sensitive lines such as
--no-track."🤖 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 `@git-trees` around lines 32 - 39, Add a concise why-comment immediately before the unquoted `$TREES_RM_CMD` invocation in `_remove_worktree`, documenting that intentional word splitting allows configured values such as `rm -rf` to provide both a command and its arguments. Leave the command behavior unchanged.Source: Coding guidelines
tests/smoke.sh (1)
634-644: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the partial-failure exit status.
The suite tests the success path of
clean --apply, but not the documented failure contract.AGENTS.mdrequiresclean --applyto continue past a failed removal and still exit nonzero. No assertion exercises that path.Add a case that forces one removal to fail, then assert both the nonzero exit and that the remaining targets were still processed. A failing
TREES_RM_CMDmakes this simple.💚 Sketch of the missing case
# Partial failure: the remover fails, so the branch delete is skipped, the loop # continues, and the exit status is nonzero. assert_ok "add clean-fail-a" bash "$T" add clean-fail-a assert_ok "delete clean-fail-a on origin" git -C "$ORIGIN" branch -D clean-fail-a assert_fail "clean --apply exits nonzero after a failed removal" \ env TREES_RM_CMD="false" bash "$T" clean --gone --apply assert_ok "branch kept when its worktree removal failed" \ git show-ref --verify --quiet refs/heads/clean-fail-aAs per coding guidelines: "The smoke suite must cover command dispatch, repository validation, init/root/add/track/list/install/rm/clean behavior, safety cases, upstream correctness, path slugging, rollback, and custom removal commands."
🤖 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 `@tests/smoke.sh` around lines 634 - 644, Add a partial-failure case near the existing clean --apply assertions: create and remove the upstream branch for a test worktree, run clean --gone --apply with TREES_RM_CMD set to a failing command, assert the command exits nonzero, and verify the branch remains while cleanup continues for any remaining targets.Source: Coding guidelines
🤖 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 `@git-trees`:
- Around line 657-664: Update the branch-deletion path in cmd_rm so a failed git
branch -d/-D operation records failure and returns a nonzero status, while
preserving the existing error message and worktree pruning. Ensure successful
branch and worktree removals continue returning 0, and align the failure status
with the documented cmd_rm partial-failure behavior.
In `@README.md`:
- Line 260: Update the README description of cmd_rm to document detached-HEAD
worktrees: removing a worktree deletes its associated local branch only when one
exists, while a detached-HEAD worktree is removed without branch deletion.
---
Nitpick comments:
In `@git-trees`:
- Around line 32-39: Add a concise why-comment immediately before the unquoted
`$TREES_RM_CMD` invocation in `_remove_worktree`, documenting that intentional
word splitting allows configured values such as `rm -rf` to provide both a
command and its arguments. Leave the command behavior unchanged.
In `@tests/smoke.sh`:
- Around line 634-644: Add a partial-failure case near the existing clean
--apply assertions: create and remove the upstream branch for a test worktree,
run clean --gone --apply with TREES_RM_CMD set to a failing command, assert the
command exits nonzero, and verify the branch remains while cleanup continues for
any remaining targets.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0efc6909-da41-4657-8044-6ccd97792013
⛔ Files ignored due to path filters (1)
logo.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
.claude/agent-notes.mdAGENTS.mdREADME.mdgit-treestests/smoke.sh
Make cmd_rm report nonzero after best-effort cleanup, document detached-HEAD rm, and cover clean --apply failure exit status. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary by CodeRabbit
rmandcleancommands to preview or apply worktree and branch removal.