Repository navigation
fix(worktrees): let git delete removed checkouts so chat sends never wait behind them - #23837
Conversation
Local worktree removal renamed the checkout into a sibling trash root and deleted it in the background with a recursive fs.rm in the main process. That queued one request per entry on libuv's shared 4-thread file pool, so for minutes every other async fs call in the main process (the agent-session store behind chat sends, file explorer reads) waited behind the delete. `git worktree remove` now deletes the checkout inline in git's own process again, so the card stays in its Deleting state for the length of the delete while Orca's file pool stays free. No timeout applies to the call, so a large delete is never killed halfway. If git reports success but the path still exists (Git for Windows leaves junctions and their parent directories in place), the leftover is deleted with the existing removeHostTree; WSL checkouts stay with the distro. Nothing creates trash any more: the scheduling queue, rename/restore helpers and the trash_rename span are gone. The startup sweep stays to drain entries older releases left behind, and now removes each emptied trash root so the obligation ends.
Removal now always runs Git's own recursive delete, and worktree creation checks out with core.longpaths on Windows, so a deep checkout Orca created could fail to delete with "Filename too long" (#6433). The Windows recovery then finishes the delete but keeps the branch. Pass the same command-scoped core.longpaths option to `git worktree remove` so Git can delete what it created. Also point the CI shard timing entry at the renamed real-git removal suite.
…elete Git for Windows asks $GIT_ASK_YESNO whether to retry when a file stays locked during a recursive delete. Orca's git env inherits the user's environment, so an inherited value would run an arbitrary prompt program in the middle of a removal. Drop it for the removal call only.
…git admission `git worktree remove` now deletes the whole checkout in Git's own process, which takes 20-35 s on a large tree. It took a general git admission slot at status tier for that whole time, and that cap is as small as two slots on a machine with six or fewer cores, so two deletes blocked every status read. Deletes now skip general admission and queue under their own limit of two per host instead: two concurrent deletes already saturate one disk, and more only slow each other down. Leftover cleanup runs inside the same slot.
…hem removing Since the checkout is deleted by `git worktree remove` in Git's own process, a large delete takes 20-35 s. Answering the request only after that made web and mobile (30 s), paired desktop (60/180 s) and the CLI (60 s) report a failure for a delete that was still going, and mobile silently re-showed the row. The request now does everything that can refuse (lock, cleanliness, archive hook, watcher/terminal gate, terminal stop, shared-link unlink), records the removal in an in-memory table on the host and answers `removing: true`. The delete, branch cleanup and metadata purge run after it in the same order as before, and the watcher/terminal gate stays held until they finish. - Listings mark rows in the table `removing` for clients that advertise `worktree.background-removal.v1` (the desktop renderer, paired desktop and web), and leave them out for everyone else (older clients, mobile, the CLI), which already dropped the row when the request answered. - The outcome (removed, with any preserved branch, or the error) rides the existing worktrees-changed event as an optional field, sent after the row has left the table. - A repeat delete while Git runs joins it. A create at the same path or with the same branch is refused with "Cleanup is pending; try again shortly"; create's name search skips the path, so generated names move on. - Nothing is persisted: after a quit or crash Git still lists the checkout and it can be deleted again. WSL checkouts still delete inline. - `orca worktree rm` says the checkout is still being deleted.
…finishes The host now answers a local worktree delete on acceptance and deletes in the background. The renderer keeps the existing delete state set until the host publishes how it ended: - The delete that asked waits for the outcome on the worktrees-changed event (local IPC or the paired runtime's client event), then runs the same teardown, preserved-branch toast and card error an inline delete did. If that event is lost to a dropped connection, a listing that shows the row gone after it was marked removing finishes the wait, and one that shows it back without the marker fails it. - Any other renderer (a reload, a paired desktop, web) sets the same delete state from the host's `removing` marker and clears it when the marker goes. A failure the host publishes lands on that card's existing error. - Web advertises `worktree.background-removal.v1` so the host sends it the marker; paired desktop does through the Electron capability list. No new component, style or state: the card reads the delete state it always did. A host that predates this answers when done without `removing`, and the renderer takes that as finished, as before.
…uffered failure A background removal's outcome that reached this renderer with no waiter (another client's delete, a host-marked card, or one already settled from listings) was buffered for 60 s and consumed by the next delete of the same workspace, so retrying a failed delete failed at once with the old error while the host was deleting. Drop the buffered outcome before sending the request; only an outcome that arrives after it can belong to it.
…ete from listings Git unlists the checkout before the host deletes the branch, cleans the push target and purges metadata, and the worktree-directory watcher refetches within 250 ms. The renderer read the missing row as a finished delete, so the waiter resolved without the preserved branch (no toast) and a failure in those last steps showed as success; the real outcome was then dropped. The listing fallback exists only for a lost outcome event, so it now applies only after this host's event stream had a gap: a new subscription or a replay after reconnect.
…te once the host accepts the last A bulk delete ran one worktree at a time per repo (#2259, for packed-refs and ref-lock races in branch cleanup). With Git now deleting each checkout for 20-35 s before the request settles, N worktrees in one repo took N times that. The renderer now queues same-repo deletes only until the host accepts each one; a parent still waits for its nested children to finish. The host serializes the branch cleanup step per repo itself, which also covers removals started by different clients.
…o they pass on Windows Removal now passes -c core.longpaths=true on Windows, so the exact-argv assertions and command-keyed mocks never matched there (17 failures on a Windows host). Pin darwin as the add-worktree suites already do, and drive the one Windows-specific case through the same spy.
…d object The anti-slop static-analysis gate rejects `object` parameters.
…uites Merge the duplicate node:fs import, build the mock child without a cast, read worktrees:list rows through one typed helper, and give the remaining casts a SAFETY line.
…fter a quit or crash A quit mid-delete left git to finish the checkout on its own while the branch delete and metadata purge never ran; a crash left a normal-looking row. Each accepted local removal now writes a record beside the profile state before git starts, clears it on success or failure, and the host runs the same delete again for any record left at startup, re-deriving what remains from git and disk. An orderly quit stops the checkout delete without waiting for it.
… checkout's .git file
Quit stops git worktree remove mid-delete, and Git deletes the checkout's .git
file wherever it falls in directory order. Git then refuses the checkout
("validation failed ... .git does not exist") on every retry, so the startup
finish failed and the row could never be deleted from Orca. A registered
checkout this record owns that has lost its .git file now finishes like an
unregistered one: leftover files, prune, then the branch.
…a different checkout A quit or crash that stops `git worktree remove` after it deleted the checkout's .git file left a registered checkout Git refuses to remove. The previous fix deleted that leftover inside Orca's process, which is the bulk delete this change exists to avoid (and on Windows the leftover can be most of the checkout). The startup finish now rewrites the missing .git file from Git's own admin entry for that path and lets `git worktree remove --force` delete it. `git worktree repair` is not used: it also re-points every other registered path, including a checkout another repository now owns there. Orca deletes the leftover itself only when no admin entry claims the path. The startup finish forces, so it now leaves the path alone when the checkout there is not the one recorded: a registered worktree on a different branch or head, or a `.git` at a path Git already unregistered. The record is dropped and the card shows why. The record write before Git starts is now bounded (2 s, logged when exceeded) so a stalled disk cannot hold the delete, and the outcome is published before the record's clear reaches disk.
…indows lock retries Git prints forward slashes in `git worktree list` on Windows, so the real-Git removal suites never found a joined path there: positive checks failed and negative ones passed without proving anything. They now compare Git's parsed rows by value. Teardown uses the shared retrying removeTree, since Windows can hold the deleted checkout busy for a moment after Git exits. Adds a relative-path worktree case for the .git restore (skipped before Git 2.48).
…on a broadcast event A current client's delete request now waits for the host's background delete and gets its real result (removed, a preserved branch, or the error) as the reply, the way it did before the delete moved off the request. A request that arrives while the delete runs joins it and gets the same result. Every other view keeps reading the host's `removing` marker: the row leaving means the delete finished, and the row listed again without the marker shows "The delete did not finish. Try again." on a card that view had marked Deleting. A request whose reply is lost (a timeout or a dropped connection) settles the same way from a fresh listing instead of reporting a failure. Clients without the background-removal capability (mobile, the CLI, older desktops) are still answered on acceptance and have rows under removal left out of their listings. This removes the outcome on worktreesChanged and everything it needed: the renderer's outcome waiters, early-outcome buffer and TTL, per-host event-gap generations, the request pre-registration, and the accept callback bulk delete used. Bulk delete runs same-repo deletes in parallel only on this machine, whose host serializes branch cleanup per repo; SSH and paired hosts stay serialized.
…l of the same worktree replaced its record The desktop app's removal and the runtime removal (CLI, paired clients) coalesce separately, so both can be accepted for one worktree. The second replaced the first's record, and the first delete then finished without resolving the request waiting on it, leaving the desktop card on Deleting indefinitely. Each delete now settles the request it was started for.
…at a time on the host Local bulk delete now sends same-repo removals in parallel, so their archive hooks, terminal teardown and preflight ran at once; a hook that writes refs can race the repo's ref locks (#2259). The host now serializes each local removal up to acceptance per repo, for every client; Git's checkout delete still runs in parallel under the delete limit.
…d call slots worktree.rm now replies only after Git deletes the checkout (up to minutes), so on paired desktop and web each waiting delete held one of the host's 8 foreground call slots, and a bulk delete queued listing refreshes and every other foreground call behind it. Deletes now run in their own lane with the same bound; the 2-slot background lane stays for status polls.
…waited its repo turn The desktop app and the runtime (CLI, paired clients, web) check for a running delete before they queue for the repo's acceptance turn. A delete of the same worktree from the other path, accepted while this one queued, was missed: this request re-ran the archive hook, stopped the terminals again and started a second `git worktree remove` on the directory Git was deleting. The queued acceptance now re-checks and joins the running delete.
…rop rows a listing read before the delete finished A delete a quit or crash interrupted took its terminal and file-watcher gate only when the resume job ran, after the first window was shown; session restore could open a shell or watcher inside the half-deleted checkout first, and on Windows that handle can fail the resumed git delete. Loading the records now fences each recorded path, and the resumed job takes the fence over in the same tick it takes its own gate. A listing that read git's registration before a delete finished, and replied after the removal record cleared, returned the row unmarked, so other views briefly showed "The delete did not finish". Listings now capture the pending removals before reading git and leave out a row whose delete finished successfully since; a row whose delete failed stays listed as before.
…val suite CI's Git 2.55 failed the file-pool test in teardown with ENOTEMPTY on the scratch repo's objects/pack after the test body passed: the 3,000-file commit's detached auto-maintenance was still writing a pack. The scratch repo now disables auto-maintenance and auto-gc.
…ete again Local same-repo deletes now start together, so each queued its trust prompt with a state snapshot taken before the first prompt was answered; approving the first still showed the same prompt once per remaining worktree. The queued check now reads the store when its turn comes.
Review status: ready for review (head 177b7a1)Verdict: review loop 14 came back clean, the closing readiness check passed, CI is green (24 pass, 14 skipped, 0 failing), and live QA passed on macOS; Windows behaviour was checked on a real Windows 11 machine. Not merged. What this PR does now: git deletes a removed worktree's checkout in its own process, as before #12416, so Orca's shared file threads stay free and chat sends, the file explorer and settings no longer stall behind a delete. The card shows "Deleting…" for as long as git takes, in every window and client, and a delete interrupted by quitting or a crash is finished on the next start. It supersedes #23746. How it got here: #23746 tried to keep Orca deleting the files itself; a real Windows run showed Node's native delete follows directory junctions out of the tree, and that approach matched no comparable app, so it was replaced with the standard mechanism. Later rounds replaced an event-based way of reporting the result with the standard one (the delete request waits and replies with the result), added the durable record and startup finish after QA found a quit mid-delete leaked the branch, and fixed several concurrency cases (same-worktree deletes from two clients, per-repo ordering of archive hooks, a stale listing, terminals opening in a half-deleted checkout at startup, a repeated trust prompt). Live QA (M4Air, 139,000-entry workspace): file reads during the delete stayed at a median of 0.4 ms (main: stalls of 3.2–3.7 s); the card showed "Deleting…" for the whole git delete and after a reload; quit and crash mid-delete were finished on relaunch, branch and metadata included; two same-repo worktrees deleted together both showed "Deleting…" and finished. QA ran at 2d71eb9 and 52cdae9; the last three commits (joining concurrent deletes of the same worktree, the startup terminal fence, and the single trust prompt in a bulk delete) are covered by unit tests only.
Windows (Windows 11, Git for Windows 2.55): git never followed a junction and files outside the worktree were byte-identical afterwards; junction leftovers (4,010 on a real pnpm checkout) were removed; read-only files, long paths and a file held open by another process all completed. Known and documented in the body: a large checkout shows "Deleting…" for 10–35 s; the path and branch can't be reused until git finishes; paired and web same-repo bulk deletes run one after another; when git fails partway (a file it may not delete) the error is shown but the row then disappears with the files, branch and metadata left behind. That last item is labelled temporary and is fixed by a follow-up PR stacked on this one. Not caused by this PR: creating two workspaces back to back in one repo once left a locked |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Why force: Git already deleted part of the checkout, which reads as local changes. | ||
| force: true, |
There was a problem hiding this comment.
git worktree remove --force, deleting those uncommitted edits. Check whether Git actually started deleting before forcing the resumed removal.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughWorktree removal now runs Git deletion inline under a separate concurrency limit or in the background for eligible local checkouts. The change adds persisted removal records, restart recovery, request joining, and pending-removal markers in listings and replies. Client code handles progress markers, longer reply waits, and lost replies. Worktree creation checks for conflicts with pending removals. Queued hook confirmations can also read updated application state. Priority: ⬆️ High Merge Risk: 🟡 Moderate · up to Background deletion improves responsiveness, but a lost removal reply can still leave a workspace on “Deleting…” and prevent a bulk deletion from finishing. Resolve or explicitly accept that remaining limitation before merging. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Moving deletion into Git improves responsiveness and retains important deletion safeguards. However, restart recovery can mistake replacement data for the original checkout, and deletion can begin before its recovery record is safely stored. These risks are conditional and local, but can affect user data. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 29.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 37 functions across 52 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: d3e12c2c-0283-4990-8d21-ab407d012884
📒 Files selected for processing (100)
config/scripts/ci-shard-timings.jsonsrc/cli/handlers/worktree.tssrc/cli/index.test.tssrc/main/git/add-sparse-worktree.test.tssrc/main/git/command-runner/git-admission-state.tssrc/main/git/command-runner/git-exec-admission-exempt.test.tssrc/main/git/command-runner/git-exec-file.tssrc/main/git/command-runner/git-exec-options.tssrc/main/git/remove-worktree-branch-cleanup.test.tssrc/main/git/remove-worktree-clean-preflight.test.tssrc/main/git/remove-worktree-test-harness.tssrc/main/git/remove-worktree.test.tssrc/main/git/worktree-add-creation-config.test.tssrc/main/git/worktree-add-local-base-refresh.test.tssrc/main/git/worktree-add-local-base-suggestion.test.tssrc/main/git/worktree-add-pending-removal.test.tssrc/main/git/worktree-add-timeout-override.test.tssrc/main/git/worktree-add.tssrc/main/git/worktree-deferred-removal-real-git.test.tssrc/main/git/worktree-delete-limit.test.tssrc/main/git/worktree-delete-limit.tssrc/main/git/worktree-git-file-restore.tssrc/main/git/worktree-graph-listing.test.tssrc/main/git/worktree-list-porcelain.test.tssrc/main/git/worktree-move.test.tssrc/main/git/worktree-mutation-route-invalidation.test.tssrc/main/git/worktree-operation-options.tssrc/main/git/worktree-porcelain-parsing.test.tssrc/main/git/worktree-posix-path-case-sensitivity.test.tssrc/main/git/worktree-removal-large-tree.bench.test.tssrc/main/git/worktree-removal-real-git.test.tssrc/main/git/worktree-removal.tssrc/main/git/worktree-remove-branch-deletion.test.tssrc/main/git/worktree-scan-cache-sharing.test.tssrc/main/ipc/desktop-renderer-runtime-capabilities.test.tssrc/main/ipc/worktree-remote.tssrc/main/ipc/worktrees-background-removal.test.tssrc/main/ipc/worktrees-delete-pty-teardown.test.tssrc/main/ipc/worktrees-removal-recovery.test.tssrc/main/ipc/worktrees-test-harness.tssrc/main/ipc/worktrees-test-runtime-stub.tssrc/main/ipc/worktrees-windows.test.tssrc/main/ipc/worktrees/listing/detected-provider-listing.tssrc/main/ipc/worktrees/listing/register-worktree-catalog-handlers.tssrc/main/ipc/worktrees/removal/register-worktree-removal-handlers.tssrc/main/ipc/worktrees/removal/remove-registered-local-worktree.tssrc/main/observability/instrumentation.tssrc/main/repo-maintenance-idle-gate.tssrc/main/runtime/orca-runtime-background-removal-test-support.tssrc/main/runtime/orca-runtime-notify-ssh-state-changed.tssrc/main/runtime/orca-runtime-remove-managed-worktree.tssrc/main/runtime/orca-runtime-resolve-worktree-removal-target.tssrc/main/runtime/orca-runtime-test-lifecycle.spec.tssrc/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation-part-03.spec.tssrc/main/runtime/orca-runtime-tests/worktree-removal-archive-hook-gate.spec.tssrc/main/runtime/rpc/methods/worktree-catalog-methods.tssrc/main/runtime/rpc/methods/worktree-rm-background-reply.test.tssrc/main/runtime/rpc/methods/worktree.tssrc/main/runtime/rpc/worktree-removal-marker-projection.test.tssrc/main/runtime/rpc/worktree-removal-marker-projection.tssrc/main/runtime/runtime-interrupted-local-worktree-removal.test.tssrc/main/runtime/runtime-interrupted-local-worktree-removal.tssrc/main/runtime/runtime-local-worktree-create-candidate.tssrc/main/runtime/runtime-registered-local-worktree-removal.tssrc/main/runtime/runtime-worktree-selection.tssrc/main/startup/main-process-quit.tssrc/main/startup/main-process-ready-runtime.tssrc/main/worktree-background-removal-records.test.tssrc/main/worktree-background-removal.test.tssrc/main/worktree-background-removal.tssrc/main/worktree-removal-acceptance-queue.tssrc/main/worktree-removal-records.tssrc/main/worktree-trash.test.tssrc/main/worktree-trash.tssrc/relay/git-branch-delete-refusal-parity.test.tssrc/renderer/src/components/sidebar/delete-worktree-flow.test.tssrc/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.tssrc/renderer/src/components/sidebar/worktree-delete-execution.tssrc/renderer/src/hooks/ipc-events/app-lifetime-ipc-bridge.tssrc/renderer/src/hooks/ipc-events/background-worktree-removal-bridge.tssrc/renderer/src/hooks/useIpcEvents-lifecycle.test.tssrc/renderer/src/lib/ensure-hooks-confirmed.test.tssrc/renderer/src/lib/ensure-hooks-confirmed.tssrc/renderer/src/store/slices/worktrees-background-removal.test.tssrc/renderer/src/store/slices/worktrees-remote-runtime-removal.test.tssrc/renderer/src/store/slices/worktrees/teardown/dispatch-worktree-removal.tssrc/renderer/src/store/slices/worktrees/teardown/host-worktree-removal-state.tssrc/renderer/src/store/slices/worktrees/teardown/remove-worktree.tssrc/renderer/src/web/preload-api/web-worktrees-api.tssrc/renderer/src/web/web-runtime-client.test.tssrc/renderer/src/web/web-runtime-connection-frame-router.test.tssrc/renderer/src/web/web-runtime-connection-frame-router.tssrc/shared/git-binary-compatibility.test.tssrc/shared/protocol-version.tssrc/shared/runtime-rpc-call-queue.test.tssrc/shared/runtime-rpc-call-queue.tssrc/shared/runtime-worktree-contracts.tssrc/shared/worktree/archive-hook-removal-gate.tssrc/shared/worktree/create-types.tssrc/shared/worktree/types.ts
💤 Files with no reviewable changes (5)
- src/main/ipc/worktrees-removal-recovery.test.ts
- src/main/git/worktree-deferred-removal-real-git.test.ts
- src/main/observability/instrumentation.ts
- src/main/git/worktree-move.test.ts
- src/main/git/worktree-mutation-route-invalidation.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.
| // Why judge only after a refresh: the rows held when the reply was lost can predate the host | ||
| // accepting the delete, and would read as a delete that never started. | ||
| const fail = (): void => reject(args.replyError) | ||
| args.refresh().then((applied) => { | ||
| if (!applied) { | ||
| fail() | ||
| return | ||
| } | ||
| pendingJudgements.add(judge) | ||
| judge() | ||
| }, fail) | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add a deadline to the lost-reply wait.
After a successful refresh, judge returns early while the listed row still has removing. It then waits for settleHostWorktreeRemovals to call it again. Nothing ever rejects this promise on a timer.
Trigger: the host keeps listing the row as removing and never publishes an end notice. This can happen after a host crash or a permanent disconnect with retained rows.
Consequence: removeWorktree never settles, and the card stays on Deleting. runWorktreeDeletesInParallel waits on Promise.all and never finishes the batch. The judge closure also stays in pendingJudgements.
Add a bounded wait. When it expires, remove judge from pendingJudgements and reject with args.replyError.
🐛 Proposed fix
- const fail = (): void => reject(args.replyError)
+ let timer: ReturnType<typeof setTimeout> | undefined
+ const fail = (): void => {
+ clearTimeout(timer)
+ pendingJudgements.delete(judge)
+ reject(args.replyError)
+ }
args.refresh().then((applied) => {
if (!applied) {
fail()
return
}
pendingJudgements.add(judge)
+ timer = setTimeout(fail, LOST_REPLY_SETTLE_DEADLINE_MS)
judge()
}, fail)Also call clearTimeout(timer) in judge before it resolves or rejects.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Why judge only after a refresh: the rows held when the reply was lost can predate the host | |
| // accepting the delete, and would read as a delete that never started. | |
| const fail = (): void => reject(args.replyError) | |
| args.refresh().then((applied) => { | |
| if (!applied) { | |
| fail() | |
| return | |
| } | |
| pendingJudgements.add(judge) | |
| judge() | |
| }, fail) | |
| }) | |
| // Why judge only after a refresh: the rows held when the reply was lost can predate the host | |
| // accepting the delete, and would read as a delete that never started. | |
| let timer: ReturnType<typeof setTimeout> | undefined | |
| const fail = (): void => { | |
| clearTimeout(timer) | |
| pendingJudgements.delete(judge) | |
| reject(args.replyError) | |
| } | |
| args.refresh().then((applied) => { | |
| if (!applied) { | |
| fail() | |
| return | |
| } | |
| pendingJudgements.add(judge) | |
| timer = setTimeout(fail, LOST_REPLY_SETTLE_DEADLINE_MS) | |
| judge() | |
| }, fail) | |
| }) |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The interrupted-removal resume hardcodes --force and never reads the recorded force choice, so a non-force delete that a quit or crash cut short before Git started can resume as an unconditional forced delete. The "different checkout" guard is also narrower than the description claims.
Reviewed changes
- Git deletes checkouts again:
git worktree remove [--force]now runs in Git's own process under a new two-per-host limit, exempt from general git admission, replacing the rename-to-trash +fs.rmpath. Windows getscore.longpaths=true, and an inheritedGIT_ASK_YESNOis dropped. - Background removal state machine: accepted removals are recorded in memory and on disk before Git starts, Git runs detached, joiners share the delete's result, and the startup sweep only drains legacy trash.
removingmarker + capability: local IPC and runtime listings mark rows a host is deleting, or omit them for clients that do not advertiseworktree.background-removal.v1(CLI/mobile). The reply timeout is raised to 180 s (+120 s with an archive hook) andworktree.rmgets its own RPC lane.- Durable resume:
orca-worktree-removals.jsonplusruntime-interrupted-local-worktree-removal.tsfinish a quit/crash-interrupted delete at next start, restoring a missing.gitlink so Git can complete it. - Renderer/CLI: a bridge keeps the Deleting card from the host marker after a reload or on a second client, the lost-reply case settles from the listing, and the CLI reports that Git is still deleting.
ℹ️ Nitpicks
src/main/git/worktree-removal-real-git.test.ts:232-233asserts loose performance bounds (maxQueued < 32,maxStatMs < deletionMs / 4); the second can flake on a loaded CI runner. Fine as a smoke test, but consider a wider margin or a comment that it is indicative.- No test covers the ordering where the second-accepted removal of a worktree finishes before the first (the suite only covers first-finished-first,
worktree-background-removal.test.ts). I traced it by hand and it is correct, but that is the ordering that exercises the per-run settlement capture.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| repo, | ||
| localOptions, | ||
| // Why force: Git already deleted part of the checkout, which reads as local changes. | ||
| force: true, |
There was a problem hiding this comment.
The resume hardcodes force: true, while WorktreeRemovalRecord.force is persisted and never read anywhere. A non-force delete that a quit or crash interrupted before Git actually ran (including one still queued behind the two-delete limit) therefore resumes as a forced delete, discarding files written to the checkout after acceptance. Please either honor the recorded choice, or escalate to force only when the checkout shows Git already began deleting it and say why.
Technical details
# Force-resume discards the recorded force choice
## Affected sites
- `src/main/runtime/runtime-interrupted-local-worktree-removal.ts:118` — `force: true` is hardcoded on the finish args.
- `src/main/worktree-removal-records.ts:25` — `force` is part of the persisted record schema.
- Producers that persist it: `src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts` (`force: args.force ?? false`) and `src/main/runtime/runtime-registered-local-worktree-removal.ts` (`force: args.force`).
- `src/main/git/worktree-removal.ts` — `force` pushes `--force` and drives the `removeCheckoutLeftByGit` fallback.
## Why it can bite
- The record is written after the dirty preflight passes, which proves the request was *accepted*, not that it was forced. The window from acceptance to Git starting includes `RECORD_WRITE_WAIT_MS` (2 s) plus any wait in `runUnderWorktreeDeleteLimit`; a quit or crash there leaves an intact checkout.
- Git refusing a non-force delete is the only thing that protects uncommitted work; the resume bypasses it.
## Required outcome
- A resumed delete must not remove uncommitted/untracked files the original non-force request would have refused.
## Suggested approach (optional)
- Gate escalating to force on evidence Git already started (for example the `.git` link was `missing`/restored, or the recorded checkout was partially absent), otherwise re-run the non-force path.
## Open questions for the human (optional)
- Is unconditional force intended? If so, the persisted `force` field is dead schema — drop it or document why it exists.| function isRecordedCheckout(worktree: GitWorktreeInfo, record: WorktreeRemovalRecord): boolean { | ||
| return ( | ||
| normalizeLocalBranchRef(worktree.branch) === record.branch && | ||
| (!record.head || worktree.head === record.head) | ||
| ) | ||
| } |
There was a problem hiding this comment.
isRecordedCheckout matches on branch + head only, so a worktree re-created at the same path on the same branch at the same head passes this guard and is force-deleted — narrower protection than the stated "never take a different checkout". Consider a stronger identity check, and add coverage for that case (the tests only cover a different branch and a different head).
Technical details
# Re-created-checkout guard is branch + head only
## Affected sites
- `src/main/runtime/runtime-interrupted-local-worktree-removal.ts:143` — the sole guard for the registered path (`deletable ? !isRecordedCheckout(...) : gitLink === 'present'`).
- `src/main/runtime/runtime-interrupted-local-worktree-removal.ts:179-184` — `isRecordedCheckout` compares `normalizeLocalBranchRef(worktree.branch) === record.branch` and `worktree.head === record.head` (and skips the head check entirely when `record.head` is empty).
- `src/main/runtime/runtime-interrupted-local-worktree-removal.ts:159-176` — for an unregistered path the only guard is `.git` presence, so a plain directory of user files at the recorded path is recursively removed by `finishUnregisteredWorktreeRemoval`.
## Required outcome
- A worktree re-created at the recorded path with the same branch and head must not be force-deleted, or the stated guarantee must be narrowed to match the implementation.
## Suggested approach (optional)
- Distinguish the original checkout by its Git admin entry identity rather than branch + head, and add a same-branch + same-head re-creation test (the current suite has different-branch and same-branch/different-head only).…-remove-in-git # Conflicts: # src/main/git/worktree-add-timeout-override.test.ts
…-remove-in-git # Conflicts: # src/renderer/src/components/sidebar/delete-worktree-parallel-flow.test.ts
What this fixes
A structured chat stopped accepting messages for about four minutes right after two worktrees were removed: the message appeared in the chat, but the agent only received it once Orca finished deleting the removed worktrees' files.
Scope: local worktree removal on macOS, Linux and Windows, from the desktop app, paired desktops, the web client, mobile and the
orca worktree rmCLI. While Orca deleted a removed checkout (often a pnpmnode_moduleswith ~100,000 files), every other file read or write in Orca's main process waited behind it: chat sends and chat creation, the file explorer (reads measured at 3–9 s during the delete), settings. Git commands and terminals were not affected. SSH worktrees (deleted by git on the remote) and folder workspaces are unchanged; WSL worktrees are still deleted by git in the distro (see Known weaknesses for their ordering).Caused by #12416, which replaced
git worktree removewith "rename the checkout into a hidden trash folder, then delete it inside Orca's main process" so the Deleting state would clear in under a second.Supersedes #23746, which kept Orca deleting the files itself and tried to limit how much of Orca's file I/O the delete could use.
ELI5
Node gives Orca 4 shared helper threads for every file read and write. Since #12416, Orca deleted a removed worktree itself, which put one job per file on that shared line, so everything else, including delivering a chat message, waited at the back for minutes. Now git deletes the checkout in its own process, as it did before #12416, so Orca's helper threads stay free. Because git's delete takes 10–35 s on a big checkout, the card keeps showing "Deleting…" until git is done, in every window and client, and a delete interrupted by quitting Orca is finished on the next start.
What Changed
Before: removing a local worktree renamed its checkout into
<parent>/.orca-worktree-trash/, cleared git's registration, and deleted the trash folder inside Orca's main process withfs.promises.rm, which queues one request per file on Node's 4 shared file threads, first in, first out.After:
git worktree remove [--force]deletes it in git's own process; the rename-to-trash path is removed. If the folder still exists after git succeeds (Git for Windows deliberately leaves directory junctions in place), the small leftover is removed with the existingremoveHostTree, and the existing Windows recovery still covers a failed delete. Windows removal passescore.longpaths=true(creation already did), and an inheritedGIT_ASK_YESNOprompt program is not passed to git.removing, so another window, a reloaded window, or a second client keeps the existing "Deleting…" card until git finishes. If a reply is lost (a dropped connection or the time limit), the card settles from the listing: row gone means deleted; row back without the marker shows "The delete did not finish. Try again." A listing that read git just before a delete finished leaves that row out rather than showing it as unfinished.orca-worktree-removals.jsonnext to the profile's state, written atomically. Quitting Orca stops itsgit worktree removewithout waiting. On the next start the card shows "Deleting…" again and the same delete runs to completion, branch and metadata included. If git had already deleted the checkout's.gitfile, Orca restores that one-line link from git's own records so git can finish; if git no longer registers the checkout at all, Orca removes what is left itself. It never force-deletes a checkout whose branch or head no longer matches the record, or a path that now holds a different checkout. Until the resumed delete starts, no terminal or file watcher can open inside that checkout.-2suffix).removingmarker get a reply as soon as the delete is accepted and don't see rows that are being deleted. The CLI returns once the delete is accepted and prints that the checkout is still being deleted.For the user: removing a worktree no longer stalls chat sends, the file explorer or anything else that reads or writes files. The card shows "Deleting…" for as long as git takes (about 10–35 s on a large checkout, a second on a small one), instead of under a second with minutes of stalls behind it.
Why
This is how comparable apps remove worktrees:
git worktree remove --forcedoes the recursive delete in git's process, leftovers are removed afterwards, the delete request replies with the result, a delete in progress is recorded durably before git starts, and a sweep at startup finishes an interrupted one by re-checking git and the disk.Measured live on a 139,000-entry workspace (M4Air): file reads during the delete stayed at a median of 0.4 ms and at most 1.4 ms with this change, versus 3.2–3.7 s on
main. On a 100,000-file checkout, a store-style transaction (read 2 MB, write, fsync, rename, every 20 ms) completed 1,978 times with a median of 20 ms during the delete, versus 22 times with one waiting 20.3 s onmain.Alternatives rejected:
git worktree moveinto trash, then remove in the background: a plain rename with the same cross-volume and open-file failures, refused for worktrees with submodules, and the branch stays taken until the delete finishes anyway.git worktree repairto resume an interrupted delete: needs Git 2.29, exits 1 even when it succeeds, and in a test it re-pointed another repository's worktree that reused the path.Differences from the common pattern
.gitlink so git can finish an interrupted delete, where comparable apps would fail that delete. Safe because Orca only writes the link when exactly one of git's own worktree records claims that path.Known weaknesses
orca worktree rmand then immediately recreates the same path or branch is refused until then.Linked Issue
No ticket. Found while diagnosing a chat that stopped responding for about four minutes.
Visual Proof
Live QA on the M4Air Mac, 139,000-entry workspace. main = c49388c, PR = 2d71eb9 (later commits change how the result reaches clients, not the delete itself).
Testing
main.main); tests for the waiting reply and joining, the lost-reply fallback, theremovingmarker for other views and older clients, the durable record (written before git, cleared on success and failure, finished at startup in each state, never a mismatched checkout, terminals and watchers fenced from startup until the resume), restoring the.gitlink (including relative-path worktrees), the per-repo ordering of hooks and branch cleanup, the separate call lane, the delete limit, and the Windows long-path andGIT_ASK_YESNOhandling. Each fails when its protection is removed.tscfor the node, web and cli projects; oxlint, oxfmt and the changed-code quality gate.