Clean up empty parent dirs when removing a worktree#116
Clean up empty parent dirs when removing a worktree#116bigs wants to merge 2 commits intosatococoa:mainfrom
Conversation
After `git worktree remove`, walk from the worktree's parent up to the resolved `base_dir`, removing each directory that is still empty. This keeps prefixed branch layouts like `feat/my-feature` from leaving behind empty `feat/` dirs under `base_dir`. Cleanup is best-effort and stops at the first non-empty directory or when it reaches `base_dir`.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 1 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. 📝 WalkthroughWalkthroughThe Changes
Sequence DiagramsequenceDiagram
participant User
participant WtpCommand
participant ConfigLoader
participant FileSystem
User->>WtpCommand: wtp remove [worktree]
WtpCommand->>FileSystem: Remove worktree
WtpCommand->>ConfigLoader: Load .wtp.yml from main worktree
ConfigLoader->>FileSystem: Read config file
ConfigLoader-->>WtpCommand: Return base_dir (absolute path)
WtpCommand->>FileSystem: Check parent directory of removed worktree
loop Traverse upward to base_dir
WtpCommand->>FileSystem: Is directory empty?
alt Directory is empty
WtpCommand->>FileSystem: Remove directory
WtpCommand->>FileSystem: Check next parent
else Directory is non-empty or is base_dir
WtpCommand->>WtpCommand: Stop cleanup
end
end
WtpCommand-->>User: Command completes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
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)
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 |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/wtp/remove.go (1)
151-158:⚠️ Potential issue | 🟡 MinorDefer parent cleanup until after branch deletion.
If the user runs
wtp remove --with-branchfrom an empty prefix directory that cleanup removes, the subsequentgit branchcommand may inherit a deleted current directory. Deferring cleanup keeps the best-effort behavior but runs it after all git operations complete.🐛 Proposed fix
- // Clean up empty parent directories between the worktree and base_dir - cleanupEmptyParentDirs(absTargetPath, worktrees) + // Clean up empty parent directories between the worktree and base_dir + // after all git operations complete. + defer cleanupEmptyParentDirs(absTargetPath, worktrees) // Remove branch if requested if withBranch && targetWorktree.Branch != "" {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/wtp/remove.go` around lines 151 - 158, Move the call to cleanupEmptyParentDirs so it runs after all git operations (specifically after removeBranchWithCommandExecutor completes) to avoid deleting the current working directory before running git branch commands; locate the cleanupEmptyParentDirs(absTargetPath, worktrees) invocation and the block that calls removeBranchWithCommandExecutor(w, executor, targetWorktree.Branch, forceBranch) (guarded by withBranch and targetWorktree.Branch) and reposition the cleanup call to execute after that branch-removal block (and any other git commands) so cleanup remains best-effort but occurs last.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@cmd/wtp/remove.go`:
- Around line 151-158: Move the call to cleanupEmptyParentDirs so it runs after
all git operations (specifically after removeBranchWithCommandExecutor
completes) to avoid deleting the current working directory before running git
branch commands; locate the cleanupEmptyParentDirs(absTargetPath, worktrees)
invocation and the block that calls removeBranchWithCommandExecutor(w, executor,
targetWorktree.Branch, forceBranch) (guarded by withBranch and
targetWorktree.Branch) and reposition the cleanup call to execute after that
branch-removal block (and any other git commands) so cleanup remains best-effort
but occurs last.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4759adf-8833-465b-b7e0-4e71bed5b04b
📒 Files selected for processing (2)
cmd/wtp/remove.gocmd/wtp/remove_test.go
When `wtp remove --with-branch` is invoked from inside a directory that cleanup removes (a prefix parent under base_dir), the subsequent `git branch -d` inherits a deleted cwd and can fail. Defer cleanup so it runs last, after all git operations.
2 participants
Heya, as always feel free to close this if you’re not interested!
I frequently use slashes in my branch names, e.g. fix/foo, feat/bar.
wtpcreates these in nested directories withinbase_dirwhich is fine, but when cleaning up a worktree, it doesn’t iteratively remove empty directories back up tobase_dir. This PR changes that to keep my worktreebase_dirnice and tidy. I’m using it locally.Details
After
git worktree remove, walk from the worktree's parent up to the resolvedbase_dir, removing each directory that is still empty. This keeps prefixed branch layouts likefeat/my-featurefrom leaving behind emptyfeat/dirs underbase_dir. Cleanup is best-effort and stops at the first non-empty directory or when it reachesbase_dir.Summary by CodeRabbit