fix(host-service): make v2 worktree removal idempotent#5075
fix(host-service): make v2 worktree removal idempotent#5075saddlepaddle wants to merge 1 commit into
Conversation
v2 workspace deletion failed >50% of the time with `fatal: '<path>' is not a working tree`. git emits that error from `git worktree remove` when the path is no longer a registered worktree (its `.git/worktrees/<id>` metadata is gone) even though the directory still exists on disk. The destroy saga's fallback only treated `!existsSync(path)` as success, so this state re-threw — and because git can never re-remove an unregistered worktree, every retry failed permanently. The metadata gets deregistered by the unconditional `git worktree prune` that `workspaces.create` runs on every call (including the idempotent "open"/refresh fired on window events) racing a partially-completed prior delete that left the directory behind. That matches the reported "feels like a race condition based on when I close windows." Fix: when `git worktree remove` reports the path is not a working tree and the directory still exists, treat it as already-deregistered — `git worktree prune` any stale metadata, then remove the leftover directory directly and continue the saga. Genuine failures (the directory persists for any other reason) still throw. Regression tests: a unit test for the deregistered-but-lingering branch, and a real-git integration test that prunes the metadata while the directory lingers and asserts the delete now completes cleanly.
📝 WalkthroughWalkthroughThe PR adds graceful recovery for workspace cleanup when git worktree removal fails due to missing worktree metadata. When ChangesDeregistered Worktree Recovery in Workspace Cleanup
Sequence DiagramEstimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 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 |
|
Ready to review this PR? Stage has broken it down into 3 individual chapters for you:
Chapters generated by Stage for commit da27d3b on Jun 3, 2026 3:24pm UTC. |
![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)
packages/host-service/src/trpc/router/workspace-cleanup/workspace-cleanup.ts (1)
328-354:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHarden
NOT_A_WORKING_TREEmatching against localized git error text (packages/host-service/src/trpc/router/workspace-cleanup/workspace-cleanup.ts, lines 328-354)The recovery path matches
err.messageagainstNOT_A_WORKING_TREE = /is not a working tree/i. Git diagnostic strings can be translated via gettext, and the git execution path here doesn’t pinLC_ALL/LANG/LC_MESSAGES(credential providers only setGIT_TERMINAL_PROMPT/GIT_ASKPASS, and shared simple-git options only set “unsafe” flags). If the message is localized, the regex won’t match and you’ll fall back to theINTERNAL_SERVER_ERRORthrow instead of pruning/removing the leftover worktree.Set a stable locale (e.g. run the
worktree remove/recovery call withLC_ALL=Cviagit.env({ ... , LC_ALL: "C" }), or equivalent) so the recovery doesn’t depend on runtime locale.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d219e0f-d16b-4c43-b463-864b9be412a8
📒 Files selected for processing (3)
packages/host-service/src/trpc/router/workspace-cleanup/workspace-cleanup.tspackages/host-service/test/integration/workspace-cleanup.integration.test.tspackages/host-service/test/workspace-cleanup.test.ts
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 3 files
Architecture diagram
sequenceDiagram
participant UI as Client / Window
participant API as TRPC Router
participant Cleanup as workspace-cleanup.ts runDestroy
participant Git as git CLI (worktree)
participant FS as File System
participant DB as SQLite (workspaces)
participant Cloud as Cloud API (v2Workspace.delete)
Note over UI,Cloud: v2 Workspace Deletion Flow
UI->>API: mutate(workspaceCleanup.destroy)
API->>Cleanup: runDestroy(workspaceId)
Cleanup->>Git: raw(["worktree", "remove", path])
alt Worktree remove succeeds (normal case)
Git-->>Cleanup: exit 0
Cleanup->>FS: directory already gone or git removed it
Cleanup->>Cloud: mutate(v2Workspace.delete.mutate)
Cloud-->>Cleanup: result
Cleanup->>DB: delete workspace row
Cleanup-->>API: success
API-->>UI: { success: true }
else Git throws "is not a working tree" (deregistered + directory lingers)
Git-->>Cleanup: exit 128, error message
Cleanup->>FS: existsSync(path) = true
alt Message matches /is not a working tree/i
Cleanup->>Git: raw(["worktree", "prune"])
Git-->>Cleanup: ok (stale metadata removed)
Cleanup->>FS: rm(path, { recursive: true, force: true })
alt rm succeeds
Cleanup->>Cloud: mutate(v2Workspace.delete.mutate) — continues saga
Cloud-->>Cleanup: result
Cleanup->>DB: delete workspace row
Cleanup-->>API: success
API-->>UI: { success: true }
else rm fails (permissions, locked, etc.)
Cleanup-->>API: TRPCError INTERNAL_SERVER_ERROR
API-->>UI: error
end
else Other git error (genuine failure, e.g. permissions)
Cleanup-->>API: TRPCError INTERNAL_SERVER_ERROR
API-->>UI: error
end
else Git throws but directory already gone
Git-->>Cleanup: exit error
Cleanup->>FS: existsSync(path) = false
Cleanup-->>Cleanup: worktreeRemoved = true
Cleanup->>Cloud: mutate(v2Workspace.delete.mutate)
Cloud-->>Cleanup: result
Cleanup->>DB: delete workspace row
Cleanup-->>API: success
API-->>UI: { success: true }
end
1 participant
Problem
v2 workspace deletion fails >50% of the time with:
Reported by Mike McQuaid — "feels like perhaps a race condition based on when I close windows."
Root cause
git worktree removeemitsfatal: '<path>' is not a working treewhen the path is no longer a registered worktree (its.git/worktrees/<id>metadata is gone) — even though the directory still exists on disk. I confirmed this against real git:git worktree remove --force --forcefatal: '…' is not a working tree(exit 128)The destroy saga in
workspace-cleanup.tsonly treated!existsSync(path)as the success-equivalent fallback. In the deregistered-but-lingering caseexistsSyncistrue, so it re-threw — and since git can never re-remove an unregistered worktree, every retry failed permanently, producing the >50% rate.How the worktree gets deregistered while its directory lingers:
workspaces.createruns an unconditionalgit worktree pruneon every call — including the idempotent "open"/refresh that fires on window events. When that prune races a partially-completed prior delete (directory left behind, or momentarily moved), it strips the metadata, and the workspace becomes permanently undeletable via the git path. This matches the "based on when I close windows" symptom.Fix
Make worktree removal idempotent. When
git worktree removereports the path is not a working tree and the directory still exists, treat it as already-deregistered:git worktree pruneany stale metadata, remove the leftover directory directly, and continue the deletion saga (cloud delete → branch delete → sqlite cleanup). The end state — worktree gone from git, directory gone from disk, cloud/sqlite rows gone — is exactly what deletion wants, so this converges to the correct result regardless of which path deregistered the worktree first.Genuine failures (directory persists for any other reason — permissions, locked, broken repo) still throw as before.
Tests
workspace-cleanup.test.ts):git worktree removethrows "is not a working tree" with the directory present → asserts prune runs, the leftover directory is removed, and the delete proceeds to cloud delete + success.workspace-cleanup.integration.test.ts): prunes the worktree metadata while the directory is briefly moved aside, restores the directory to reproduce the exact deregistered-but-lingering state, asserts git no longer lists the path but it exists on disk, then assertsdestroyremoves everything cleanly with no warnings. This test fails against the old code.All host-service tests pass; typecheck and biome clean.
Summary by cubic
Make v2 workspace deletion idempotent to stop frequent failures when a Git worktree is deregistered but its directory still exists. This removes the >50% "is not a working tree" error caused by a race with
git worktree prune.git worktree removefailure with "is not a working tree" and the directory still present, treat it as already deregistered.git worktree prune, remove the leftover directory directly, then continue deletion (cloud, branch, DB).Written for commit da27d3b. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests