fix: allow retrying partial worktree removal - #3315
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe worktree removal error path now requests force confirmation for unregistered checkouts that are not valid working trees. A new test verifies the API response, state transition, cleanup, and absence of workspace changes or events. ChangesWorktree removal recovery
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Retrying a partially removed worktree now reaches explicit force confirmation while preserving safeguards against deleting registered or unrelated directories. No actionable merge-blocking risk remains after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
0b02595 to
c70825e
Compare
Greptile SummaryThis PR allows an interrupted worktree removal to be retried when Git has already unregistered the checkout but left its directory behind.
Confidence Score: 5/5The PR appears safe to merge because the new retry state remains explicitly confirmed and the existing forced-cleanup guards continue to reject registered or unrelated directories. The change only enables confirmation after a recognized partial-removal error; operation identity and path matching protect modal state, and filesystem cleanup remains gated by registration and repository-identity checks.
|
| Filename | Overview |
|---|---|
| src/app/api/worktrees/deferred.rs | Extends deferred removal error handling to request explicit force confirmation for recognized partial-removal errors while retaining existing response semantics and path/state matching. |
| src/app/api/worktrees.rs | Adds focused regression coverage verifying the response, pending-operation cleanup, modal transition, and preservation of workspace and event state. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Deferred worktree removal] --> B{Git result}
B -->|Success| C[Remove workspace and emit events]
B -->|Dirty checkout| D[Return dirty_worktree_requires_force]
B -->|Not a working tree| E[Return worktree_remove_failed]
D --> F[Show explicit force confirmation]
E --> F
F --> G[Confirmed forced retry]
G --> H{Path still registered or unrelated?}
H -->|Yes| I[Reject cleanup]
H -->|No, valid repository leftover| J[Remove leftover directory]
Reviews (1): Last reviewed commit: "fix: allow retrying partial worktree rem..." | Re-trigger Greptile
Issue
A blocked worktree removal can unregister the checkout in Git while leaving its directory behind. Retrying from Herdr then stays on a fatal error instead of allowing cleanup.
Problem
Git reports that the leftover path is no longer a working tree. The removal dialog treated that response as terminal, so it could not reach Herdr's guarded forced cleanup.
How did we fix it?
Herdr now asks for explicit force confirmation when this retry state occurs. The API still returns
worktree_remove_failedwith Git's original message. The confirmed retry continues to use the existing checks that reject registered or unrelated directories.Verification
The new deferred-removal regression failed before the change and passes afterward. All 21 targeted worktree-removal tests pass. Formatting and Clippy pass, and 3,534 of 3,535 broad tests pass. The remaining live-handoff test cannot recognize the isolated external Cargo target path in this environment, although its log shows the replacement server started.
refs #3314