[brennanb2025]

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 in the background.

Scope: every Orca user on macOS, Linux and Windows whose app deletes a large folder in the background or on request: worktree removal (the trash delete), plugin, skill and speech-model staging cleanup, failed-clone and import rollback, folder deletes from the remote and mobile file explorer, and terminal-history cleanup. While such a delete ran, anything else in the main process that reads or writes a file (chat sends and chat creation through the agent-session store, journals, settings) waited behind it. Git commands and terminals were not affected, because they run in their own processes. For worktree removal this started with #12416, which moved the checkout delete out of git's own process (git worktree remove) into Orca's main process so removal returns immediately; the other folder deletes always worked this way.

ELI5

Node gives each app 4 shared helper threads for all file reads and writes. Deleting a folder the old way put one job per file on that shared line, and a worktree has about 100,000 files, so the line filled up for minutes and every other file read or write, including the one that delivers a chat message, waited at the back. Now a big delete may only use two of those helper threads at a time, so everything else keeps moving while it runs. A test stops new code from going around this.

What Changed

Before: fs.promises.rm(dir, { recursive: true }) walks the tree and queues one request per file and folder on Node's 4 shared file threads, first in, first out. Anything else that asked for file I/O afterwards (the agent-session store's lock, read, write and fsync, the journal files, settings) waited behind the whole tree. Worktree removal renames the checkout into a trash folder and deletes it in the background this way, and about 40 other places in the main process deleted folders the same way.

Observed on a real profile: two worktree removals at 14:00:53–14:01:14 were followed by a 4-minute window in which no chat send on the machine was accepted. Sends clicked at 14:01:54, 14:02:18 and 14:03:17 reached the agent only at 14:06:13, 14:06:15 and 14:06:18, and another chat's action clicked at 14:01:33 was not admitted until 14:06:13. Git commands kept completing throughout, because child processes do not use the shared threads, which is the signature of thread starvation rather than a frozen process.

After:

• removeHostTree is the one way the main process and the terminal daemon delete a folder tree. It walks the tree itself (src/main/tree-removal-walk.ts), and every file call any tree delete makes has to hold one of two process-wide slots, so two of the four shared file threads always stay free for everything else.
• A delete someone is waiting for (a file-explorer folder delete, a rollback) gets the next free slot ahead of background deletes (worktree trash, terminal-history cleanup), so it never queues behind a long trash delete.
• The walk checks each entry with lstat and only goes into real folders. Symbolic links and Windows junctions are removed as links and never followed out of the tree.
• Each entry is removed with one unlink or rmdir. Only when that fails does it fall back to Node's own rm for that entry, which clears a read-only flag and retries a briefly locked file on Windows.
• If one entry cannot be removed, the rest of the tree is still deleted and the call fails naming that entry, as before.
• It uses Electron's unpatched file system (asar-transparent-fs.ts), so a .asar archive inside a tree (every pnpm-installed checkout has one) is removed as the file it is.
• Every async recursive delete in src/main and src/shared now calls removeHostTree. The file-explorer delete keeps a plain single-file rm for files and uses removeHostTree only for folders.
• src/main/host-tree-removal-boundary.test.ts scans src/main and src/shared and fails on any async recursive rm/rmdir, whether the options are inline, split across lines, held in a constant, or built by a call. There is no allowlist.
• The AppImage payload removal's process-wide process.noAsar toggle is removed; it uses removeHostTree like everything else.
• The walk tests now also run on the Windows CI lane, where they use real junctions.

For the user: removing worktrees, clearing plugin or skill staging, or deleting a folder no longer makes chat sends, chat creation or other file-backed actions wait. Large deletes themselves can take somewhat longer, because they no longer take over every file thread.

Why

The failure class is "a bulk background job and latency-critical persistence share one first-in-first-out resource". Capping the bulk job's share of that resource removes it for every folder delete. A timeout or a priority around the send path would only have hidden it, and the next bulk delete would still have stalled everything else that touches a file. It does not remove every possible bulk user of those threads (for example, a large recursive copy would behave the same way); none is known to stall the app today.

Measured: during a 102,000-entry delete, a store-style transaction (read 2 MB, write, fsync, rename) took about 9 ms median and 14–19 ms at the 95th percentile; with the old fs.promises.rm a single transaction waited 3.9–5.1 s, the length of the whole delete. A two-file delete issued during the bulk delete finished in under 1 ms median.

It works the same in Electron's main process, the terminal daemon (Electron running as Node), plain Node and Bun, keeps each error's code, and names the entry that could not be removed. Quitting while a delete runs is not held up: there is no thread to wait for and at most two file calls are in flight. The Windows transient-lock retry loop in removeHostTree is unchanged.

Alternatives rejected:

• Node's native rmSync on a worker thread (this PR's first version): on Windows, Node 24's native recursive delete follows directory junctions and deletes files outside the tree, can report success while leaving most of a pnpm checkout on disk (77,000 of 86,000 files remained on a real checkout), and fails on read-only files such as git objects. Quitting the app also had to wait for the whole tree, and each delete cost a separate JavaScript engine instance.
• Moving fs.promises.rm into a worker thread: a worker's async file calls use the same process-wide threads; the store still waited about 3 s.
• A child process per delete: 180–250 ms to start each time, and a burst of short-lived interpreter processes is a pattern that endpoint security software on Windows scores as suspicious.
• Letting git delete the checkout (git worktree remove --force): Orca renames the checkout into trash first so removal returns immediately instead of holding a spinner for 8–35 s, and the trash folder is no longer a git worktree. It also would not cover the other ~40 folder deletes.
• Raising UV_THREADPOOL_SIZE: the threads are first in, first out, so a larger pool still runs every queued entry of the tree before later requests.
• Guarding only the worktree trash delete: the same pattern exists on user-sized folders elsewhere, and without a tree-level test the next call site reintroduces it.

Differences from the common pattern

• For worktrees, the common approach lets git worktree remove --force delete the checkout inside git's own process and then removes any leftovers with an async recursive delete. Orca keeps its rename-to-trash step for fast removal and deletes the trash folder itself.
• Apps that avoid this stall usually do it by keeping their latency-critical storage off the shared file threads (a synchronous database, or a separate process per area), not by limiting the delete. Orca's agent-session store still uses those threads, so this PR limits the bulk deletes instead; moving the store to SQLite is planned separately.

Known weaknesses

• Large deletes can be slower than before because they use two file threads instead of all four: roughly 1.3–4x on a loaded Mac in synthetic runs; on Windows a real 86,000-file pnpm checkout took 26.7 s against 27.1 s before. The worktree trash delete happens in the background; a very large folder deleted from the file explorer can take longer.
• Deletes are not cancellable. A delete in progress when the app quits stops with the process; worktree trash left behind is swept on the next start.
• Two Orca-owned staging folders (browser upload staging, remote clipboard staging) used to retry a failed delete up to 3 times on every OS; removeHostTree retries only on Windows, where held handles are the known cause.
• This does not change the agent-session store itself, which still rewrites its whole JSON file on every transaction. Moving it to SQLite is already planned after the per-host journal database; this PR only stops bulk deletes from queueing in front of it.
• Synchronous recursive deletes on the main thread (rmSync) block the event loop rather than the shared threads. That is a different failure and is not changed here.

Linked Issue

No ticket. Found while diagnosing a chat that stopped responding for about four minutes.

Visual Proof

Live Electron app on the M4Air Mac, 138,993-entry workspace removed from the sidebar, file reads (fs:readDir, fs:readFile) timed from the app window every ~250 ms. main = bd5dca4, PR = fbce1b2 (0af34fd only changes what happens when an entry cannot be deleted).

Testing

• Regression test (src/main/host-tree-removal.test.ts): a chain of 20 async stat calls issued while a 2,000-file tree is deleted must take less than half the delete's time. With the old fs.promises.rm swapped back in it fails every run.

• Walk tests (src/main/tree-removal-walk.test.ts), each confirmed to fail when its protection is removed: never more than two file calls in flight across ten concurrent deletes; an awaited delete gets the next free slot ahead of queued background calls; a link that the directory listing reports as a folder is unlinked, not followed; a target that is itself a link removes only the link; a link loop completes; a refused unlink/rmdir falls back to rm; a permanent failure removes everything else and rejects with its code and the stuck entry.

• Real Electron binary (host-tree-removal-asar.electron.test.ts): removes a tree holding a real default_app.asar, also when every entry has to go through the rm fallback; that second case fails with Electron's patched file system.

• Boundary test (src/main/host-tree-removal-boundary.test.ts): putting one old recursive rm back fails it and names the file.

• Existing tests: every test that imports a changed module or stubs the file-system module passes, including the CLI bundle-parity and plain-Node entry guards.

• Windows (real machine, Windows 11, Electron 43 as Node): the bundled removeHostTree removed junctions pointing outside the tree without touching their targets (target files byte-identical before and after), a target that is itself a junction, junction loops, dangling junctions, read-only files and folders, a fresh git repo, 633-character paths, and a real pnpm checkout with 86,428 files and 4,010 junctions (26.7 s, nothing left). A read-only file held open by another process is retried and then reported as EBUSY naming that file; a second call after release removes the rest. The walk tests pass on Windows.

• Live Electron app on macOS: on the M4Air, a 138,993-entry workspace removed from the sidebar, two runs per side (main bd5dca4, PR fbce1b2). While the trash folder was deleted, the worst file read took 8.1 s and 9.0 s on main (only 4-5 reads finished in about 20 s) and 7 ms and 17 ms on the PR (median about 0.5 ms, the same as idle). The workspace left the sidebar at once, the trash folder was empty at the end, and git worktree list was clean. Quitting with app.quit() right after the delete started: the app exited in about 155 ms on the PR (914 ms on main), and the next launch swept the leftover trash. A folder delete from the file explorer still works. Chat sends were not timed because that needs a logged-in agent; the file reads above go through the same shared file threads.

• Runtime checks: Electron 43 as Node, plain Node 26 and Bun 1.3.14.

• I manually tested these changes (live Electron QA on macOS, removeHostTree on a real Windows 11 machine)

• Automated tests added/updated

Review

Reviewed in six loops. The first design (Node's native rmSync on a worker thread) was replaced after a Windows run showed it follows junctions out of the tree; the last loop was clean. CI is green.

Agent skill upstream boundary

• Not applicable.

Notes

Security: no new dependencies, no code evaluation, no network. Cross-platform: macOS live QA, Windows real-machine fixtures and the Windows CI lane, Linux CI. Remote SSH: only local deletes change; the relay is untouched. Mobile: unaffected. Backwards compatibility: no persisted state or wire changes. Performance: covered above.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[brennanb2025]

Review status: ready for review (head 0af34fd)

Verdict: the final review loop came back clean, CI is green (42 pass, 6 skipped, 0 failing), and live QA passed. Nothing a user would find surprising remains. Not merged.

What changed during review: the PR's first version deleted each tree with Node's native rmSync on a worker thread. A run on a real Windows 11 machine showed that, on Windows, Node 24's native recursive delete follows directory junctions and deletes files outside the folder being removed. It also reported success while leaving 77,000 of 86,000 files of a real pnpm checkout on disk, and it failed on read-only files such as git objects. The worker also held up app quit until the whole tree was gone. The delete engine was therefore replaced with a walk that keeps at most two file calls on Node's shared file threads at a time, never follows links, and falls back to Node's own rm per entry. The single removeHostTree entry point, the boundary test and the background/interactive split were kept. The PR description is updated to match.

Commits added in review:

• 9028159, 8bb221c, b156086: worker hardening (a concurrency cap, background/interactive lanes, quit), later superseded by the engine change.
• d6eba1b, e27d738, 2262ad7, de4901d, 5beb8e1, 654f717: the bounded walk, the unpatched file system for .asar archives, tests that fail when their protection is removed, and mocks for tests that stub the file system.
• 62f2e93: a lint fix in the walk test. fbce1b2: the walk tests now run on the Windows CI lane.
• 0af34fd: one entry that cannot be deleted no longer strands the rest of the tree, which the startup sweep would otherwise hit on every launch.

Live QA, M4Air Mac, 138,993-entry workspace, two runs per side (main bd5dca4, PR fbce1b2): while the removed workspace's trash folder was being deleted, the worst file read was 8.1 s and 9.0 s on main and 7 ms and 17 ms on the PR. The workspace left the sidebar at once and the trash was empty at the end. Quitting right after starting a delete exited in about 155 ms (914 ms on main), and the next launch swept the leftover. A folder delete from the file explorer works.

Windows (real machine, 654f717): junctions pointing outside the tree, a target that is itself a junction, junction loops, dangling junctions, read-only files and folders, a git repo and long paths were all removed with the files outside the tree byte-identical. A real pnpm checkout (86,428 files, 4,010 junctions) was removed in 26.7 s with nothing left (27.1 s before this PR). The walk tests pass on Windows.

Not verified: chat-send timing in the live app (it needs a logged-in agent; the file reads above go through the same shared threads); true symbolic links on Windows (they need admin or Developer Mode); Bun 1.4.2 (1.3.14 checked).

Accepted low risks:

• Large deletes can be slower because they use two file threads instead of four; the delete time on the M4Air and on Windows was not slower.
• A locked file on Windows holds a slot through its retries (about 5.4 s).
• A folder that can never be deleted is walked again on each startup sweep, as before.
• The boundary test does not catch aliased imports or option spreads; none exist today.

Follow-ups, not caused by this PR:

• A large recursive copy would load the shared file threads the same way.
• The "Deleting…" state stays on the workspace card for about 2.5 s after confirming (main too).
• Each removal logs a git:status AbortError (main too).

[greptile-apps]

Confidence Score: 3/5

[High risk] Refactors file deletion across 75 files to use a new shared removal function.

The PR should not merge until the tree walk prevents concurrent link replacement from redirecting deletion outside the requested tree.

Findings

1.   Deletion can escape the tree ▶
2.  Fallback bypasses deletion limit ▶

Summary

The PR replaces main-process recursive folder deletes with a shared, priority-aware bounded walk and routes existing deletion sites through it. It also adds cross-platform walk tests and a test guarding against future direct recursive deletes.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Caller requests tree removal] --> B[removeHostTree]
  B --> C[Bounded walk]
  C --> D[lstat path]
  D --> E[readdir path]
  E --> F[Remove listed children]
  C --> G[unlink or rmdir]
  G -->|failure| H[recursive rm fallback]

Loading

_{Reviews (1) · Last reviewed commit: "fix(main): keep deleting siblings after ..."}

[greptile-apps]

Deletion can escape the tree If a concurrent process replaces a directory in a worktree or file-explorer delete with a symlink or junction after lstat but before readdir, the listing follows the replacement. The walk then removes the listed children through that link, potentially deleting files outside the requested tree. How this was verified: The walk classifies a directory, reopens it by path, and deletes children through paths joined beneath it.

[greptile-apps]

Fallback bypasses deletion limit If another process adds entries after a directory is listed, rmdir can find it non-empty. The fallback then runs recursive fs.rm while holding just one semaphore slot. A large subtree can therefore queue file operations outside the per-entry limit, bringing back delays for chat persistence. Keeping this fallback bounded would preserve the protection provided by the new walk.

[pullfrog]

Important

The new walk lists a directory's children and deletes them before it attempts rmdir — the reverse of Node's fs.rm, which does rmdir first and recurses only on ENOTEMPTY/EEXIST/EPERM. On a busy mount point that empties the mounted filesystem before rmdir fails EBUSY; fs.rm would have refused without touching the contents. This PR introduces that divergence, and user-driven deletes reach it.

Reviewed changes

• New bounded walk — src/main/tree-removal-walk.ts walks a tree itself, gating every lstat/readdir/unlink/rmdir/rm call behind a 2-slot process-wide PrioritySemaphore (interactive before background), with a per-entry unlink/rmdir and an fs.rm fallback.
• removeHostTree routed through it — gains an { lane } option; every async recursive delete in src/main and src/shared now calls it. Worktree trash and terminal-history cleanup are tagged background; everything else defaults to interactive.
• Unpatched fs — asar-transparent-fs.ts now exposes the original-fs call set, so a tree containing a .asar is deleted as files. The AppImage payload removal drops its process-wide process.noAsar toggle.
• Guards — a no-allowlist boundary test scans src/main + src/shared for async recursive rm/rmdir; new walk/pool/asar tests, with the walk tests added to the Windows CI lane.
• Test mocks migrated — specs that stubbed fs/promises.rm now stub removeHostTree or removeTreeWithBoundedFsCalls.

I read the full diff (75 files) and ran the new and modified suites; all pass. src/main/terminal-history.test.ts fails locally with No "mkdtempSync" export on the "fs" mock, but that reproduces identically on origin/main, so it is pre-existing and unrelated.

⚠️ A mount point inside the deleted tree is emptied instead of refused

Node's fs.rm tries rmdir first and only enumerates on ENOTEMPTY/EEXIST/EPERM, so an EBUSY from a busy mount point aborts without touching the mounted filesystem. The walk does the opposite: any path lstat calls a directory is readdir'd and its children deleted, and only then is rmdir attempted. On POSIX a bind/overlay/network mount is an ordinary directory to lstat, and on Windows a volume mount point can be reported as one, so the walk descends and deletes the mounted volume's contents before failing. removeHostTree is reached from user-driven deletes (the file-explorer permanent delete and worktree trash), so this is a data-loss path that did not exist before this PR.

Technical details

# Mount point emptied before rmdir

## Affected sites
- `src/main/tree-removal-walk.ts:59-62` — every `lstat`-directory is `readdir`'d before any removal attempt
- `src/main/tree-removal-walk.ts:90` — `rmdir`/`unlink` runs only after the children are gone
- `src/main/host-tree-removal.ts:60` — previously delegated to Node's `fs.rm` (rmdir-first), now to the walk
- Reached from user deletes: `src/main/runtime/runtime-file-commands-create-file-explorer-dir-no-clobber.ts:164`, `src/main/worktree-trash.ts:87`

## Required outcome
A delete must not remove entries inside a mount point that Node's `fs.rm` refused to enter. Either
probe the directory removal before enumerating it, or detect a mount point (e.g. `stat(dir).dev`
vs `stat(dirname(dir)).dev`) and stop instead of descending.

## Suggested approach
Mirror Node's `_rmdir`: call `rmdir(path)` first; only on `ENOTEMPTY`/`EEXIST`/`EPERM` `readdir`
and recurse; let `EBUSY`/`EXDEV` propagate without touching contents. Costs one extra `rmdir` per
directory and restores the old guarantee. The existing per-entry `fs.rm` fallback already refuses
the mount point, so the regression is purely the pre-emptive `readdir`.

ℹ️ The relay's remote folder delete still uses an unbounded recursive rm

deleteRelayPath in src/relay/fs-path-mutation-requests.ts:32 still calls rm(targetPath, { recursive, force: true }). The PR body lists "folder deletes from the remote and mobile file explorer" in scope while also stating "the relay is untouched", and the new boundary guard scans only src/main and src/shared, so this path can neither be fixed nor caught. Because the relay is a separate process it does not share the main pool, so it is not the reported chat-send stall — but the stated scope is wider than the change, and a large remote delete still occupies the relay's own async fs work.

Technical details

# Relay recursive delete outside the bounded walk

## Affected sites
- `src/relay/fs-path-mutation-requests.ts:32` — `rm(targetPath, { recursive: !!recursive, force: true })`, the remote file-explorer folder delete
- `src/main/host-tree-removal-boundary.test.ts:14` — `SCANNED_ROOTS = ['src/main', 'src/shared']` excludes `src/relay`

## Required outcome
Either route the relay's tree deletes through the same bounded walk, or narrow the PR's stated scope
so remote file-explorer deletes are not claimed as fixed.

## Open questions for the human
Is the relay intentionally deferred to a follow-up (separate process, separate pool), or should the
boundary guard at least cover `src/relay`?

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

This enumerates and deletes a directory's children before attempting rmdir (:90), whereas Node's fs.rm calls rmdir first and recurses only on ENOTEMPTY/EEXIST/EPERM. On a busy mount point (lstat reports a POSIX bind mount, and can report a Windows volume mount point, as an ordinary directory) the walk therefore deletes the mounted filesystem's contents and only then fails with EBUSY — content fs.rm would have left untouched.

Technical details

# Mount point emptied before rmdir

## Affected sites
- `src/main/tree-removal-walk.ts:59-62` — every `lstat`-directory is `readdir`'d before any removal attempt
- `src/main/tree-removal-walk.ts:90` — `rmdir`/`unlink` runs only after the children are gone
- `src/main/host-tree-removal.ts:60` — was Node's `fs.rm` (rmdir-first), now the walk

## Required outcome
A delete must not remove entries inside a mount point that Node's `fs.rm` refused to enter.

## Suggested approach
Call `rmdir(path)` first and `readdir` only on `ENOTEMPTY`/`EEXIST`/`EPERM`, letting `EBUSY`/`EXDEV`
propagate without touching contents (Node's `_rmdir`). The existing per-entry `fs.rm` fallback
already refuses the mount point, so only the pre-emptive `readdir` needs to change.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

📝 Walkthrough

Walkthrough

The change adds a bounded tree-removal walk and a shared host-tree removal helper. Recursive directory cleanup across application workflows now uses the helper instead of direct recursive rm calls. Selected terminal-history and worktree deletions use the background lane. The change also updates cleanup tests, adds boundary and removal tests, adjusts Windows transient-lock cleanup, and includes the new tests in Windows-specific test configuration.

Priority: ⬆️ High

Merge Risk: 🔵 Low · up to 0af34

A directory changed during deletion can briefly defeat the responsiveness improvement. This is a narrow case, but the fallback should be bounded before merge if strict responsiveness is required.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 0af34

Most deletions should leave capacity for other file operations, but a folder that changes during deletion can still trigger the long-running removal behavior this change aims to prevent. The shared change affects many cleanup paths; no expanded deletion permission was established.

Retained concerns

• Medium · security · inferred: A directory that becomes non-empty after enumeration can make rmdir fail. The fallback then runs recursive removal within one scheduler slot, bypassing the new per-filesystem-call bound and potentially stalling unrelated main-process file I/O. This leaves a pre-existing availability problem incompletely mitigated rather than establishing a new deletion authority.

Security review details

Security Blast Radius

• inferred — A recursive fallback on a large mutable tree can compete with other main-process file operations; the shared host-removal boundary extends the consequence across its cleanup callers.

Security Findings and Attack Paths

• inferred — If a caller's directory gains a child between enumeration and rmdir, the resulting failure can enter recursive fs.rm despite the two-slot design. The evidence establishes this control-flow path, not a verified exploit or increased filesystem authority relative to the previous recursive removal.

Trust Boundaries and Controls

• observed — The shared remover accepts the caller's target and selects deletion mechanics; in the examined file-explorer path, authorization occurs before that call. Static links are distinguished from directories by lstat during traversal.

Resilience and Maintainability Implications

• observed — Slots are released after each awaited call, and persistent host-removal errors escape after the applicable Windows retries rather than being reported as success.

Hardening Proposals

• proposed — Keep populated-directory recovery within the bounded walk rather than invoking recursive removal inside one slot, while retaining the required Windows read-only and transient-lock recovery behavior.

🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is thorough and covers the required change summary, rationale, testing, visual proof, review, cross-platform impact, and checklist. However, the Linked Issue section states "No ticket"…	Add the linked issue number or URL. If the author is a maintainer and the no-issue exception applies, explicitly confirm that exception in the Linked Issue section and retain the rationale for not opening an issue.
Docstring Coverage	❓ Inconclusive	Docstring coverage is 29.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 44 functions across 50 files. (25 skipped…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing large background deletes from blocking chat sends and other file I/O.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description is thorough and covers the required change summary, rationale, testing, visual proof, review, cross-platform impact, and checklist. However, the Linked Issue section states "No ticket" instead of providing the required issue link.

Full details: Docstring Coverage

Explanation

Docstring coverage is 29.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 44 functions across 50 files. (25 skipped: 1 unsupported, 24 over the file limit.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 007235db-4e5a-4ee4-af81-cb01a2089e24

📥 Commits

Reviewing files that changed from the base of the PR and between bd5dca4 and 0af34fd.

📒 Files selected for processing (75)

• .github/workflows/pr.yml
• config/scripts/pr-code-change-scope.mjs
• src/main/ai-vault-search/session-search-index-test-fixture.ts
• src/main/asar-transparent-fs.test.ts
• src/main/asar-transparent-fs.ts
• src/main/browser/browser-client-download-relay.ts
• src/main/browser/browser-client-upload-staging.test.ts
• src/main/browser/browser-client-upload-staging.ts
• src/main/browser/browser-route-partition-storage-dependencies.ts
• src/main/cli/appimage-payload-removal.reentrancy.test.ts
• src/main/cli/appimage-payload-removal.test.ts
• src/main/cli/appimage-payload-removal.ts
• src/main/computer/desktop-script-provider-client.ts
• src/main/computer/desktop-script-provider-test-harness.ts
• src/main/computer/macos-computer-use-permission-status.test.ts
• src/main/computer/macos-computer-use-permission-status.ts
• src/main/computer/macos-computer-use-permissions.test.ts
• src/main/daemon/terminal-history-session-tombstone-retry.test.ts
• src/main/daemon/terminal-history-session-tombstone.ts
• src/main/git/repo-clone-path.ts
• src/main/github/client/create/create-github-pull-request.ts
• src/main/host-tree-removal-asar.electron.test.ts
• src/main/host-tree-removal-boundary.test.ts
• src/main/host-tree-removal.test.ts
• src/main/host-tree-removal.ts
• src/main/ipc/filesystem-download-folder.ts
• src/main/ipc/filesystem-download-transfers.test.ts
• src/main/ipc/filesystem-import-local-rollback.test.ts
• src/main/ipc/filesystem-import-local-tree-copy.ts
• src/main/ipc/filesystem-import.test.ts
• src/main/ipc/filesystem-test-harness.ts
• src/main/ipc/pet-bundle-import.ts
• src/main/ipc/pet.ts
• src/main/ipc/repos-create.test.ts
• src/main/ipc/repos/repo-creation-handlers.ts
• src/main/ipc/shallow-watch-delivery-probe-lifetime.test.ts
• src/main/ipc/shallow-watch-delivery-probe.ts
• src/main/ipc/terminal-render-desync-evidence.ts
• src/main/local-downloaded-folder-promotion.ts
• src/main/local-worktree-filesystem.test.ts
• src/main/orcad/electron-serve-browser-process.ts
• src/main/orcad/orcad-bun-native-preflight.test.ts
• src/main/orcad/orcad-bun-native-preflight.ts
• src/main/persistence/profile-state/profile-state-runtime-preflight.ts
• src/main/plugins/plugin-install-publication.ts
• src/main/plugins/plugin-install-staging.ts
• src/main/plugins/plugin-install.ts
• src/main/plugins/plugin-marketplace-fetch.ts
• src/main/plugins/plugin-marketplace-installer.ts
• src/main/runtime/runtime-file-commands-create-file-explorer-dir-no-clobber.ts
• src/main/runtime/runtime-repository-registration-controller.ts
• src/main/skills/skill-bundle-creation.ts
• src/main/skills/skill-bundle-extraction.ts
• src/main/skills/skill-install-filesystem.ts
• src/main/skills/skill-package-creation.ts
• src/main/skills/skill-package-download.ts
• src/main/skills/skill-package-extraction.ts
• src/main/skills/skill-share-preparation-service.ts
• src/main/skills/skill-upload-staging-ownership.ts
• src/main/speech/model-manager.ts
• src/main/ssh/orcad-artifact-materializer.ts
• src/main/ssh/orcad-bun-runtime-materializer.ts
• src/main/ssh/ssh-relay-opencode-runtime.ts
• src/main/ssh/system-ssh-file-binary-transfer.ts
• src/main/terminal-history-deletion.ts
• src/main/terminal-history-gc.test.ts
• src/main/terminal-history-tombstone-retry.test.ts
• src/main/terminal-history.test.ts
• src/main/tree-removal-walk.test.ts
• src/main/tree-removal-walk.ts
• src/main/window/clipboard-ipc-handlers.test.ts
• src/main/window/clipboard-remote-file-staging.test.ts
• src/main/window/clipboard-remote-file-staging.ts
• src/main/worktree-trash.ts
• src/shared/windows-transient-lock-removal.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

[coderabbitai]

🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

sed -n '1,100p' src/main/tree-removal-walk.ts
sed -n '1,84p' src/main/host-tree-removal.ts

Repository: stablyai/orca

Length of output: 7495

---

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- tree-removal-walk references ---'
rg -n -C 3 'removeEntry|fs\\.rmdir|fs\\.rm|ENOTEMPTY|EEXIST|EPERM|transientLockRemovalOptions' src/main/tree-removal-walk.ts src/main/asar-transparent-fs.ts src/shared/windows-transient-lock-removal.ts src/main/host-tree-removal.ts
printf '%s\n' '--- asar wrapper ---'
cat -n src/main/asar-transparent-fs.ts
printf '%s\n' '--- removal options ---'
cat -n src/shared/windows-transient-lock-removal.ts
printf '%s\n' '--- callers and tests ---'
rg -n -C 3 'removeHostTree|removeTreeWithBoundedFsCalls' src test tests 2>/dev/null || true

Repository: stablyai/orca

Length of output: 42333

---

Keep recursive fallback for recovery errors, but reject directory-not-empty errors on Windows.

This is not a deadlock. removeTree releases its slot before recursing. The actual issue is that rmdir can fail with ENOTEMPTY, causing recursive fs.rm to queue the subtree while a slot is held.

On Windows, let ENOTEMPTY and EEXIST reach removeHostTree so its finite retry loop handles transient additions. Keep recursive fs.rm for EPERM and other errors because it provides read-only recovery. Do not reject these errors on non-Windows, where removeHostTree has no retry loop.

Suggested fix

+function isDirectoryNotEmptyError(error: unknown): boolean {
+  const code = typeof error === 'object' && error !== null && 'code' in error ? error.code : null
+  return code === 'ENOTEMPTY' || code === 'EEXIST'
+}
+
-  const removeEntry = (path: string, isDirectory: boolean): Promise<void> =>
-    withFsCallSlot(lane, () =>
-      (isDirectory ? fs.rmdir(path) : fs.unlink(path)).catch(() => fs.rm(path, options))
-    )
+  const removeEntry = async (path: string, isDirectory: boolean): Promise<void> => {
+    try {
+      await withFsCallSlot(lane, () => (isDirectory ? fs.rmdir(path) : fs.unlink(path)))
+    } catch (error) {
+      if (process.platform === 'win32' && isDirectory && isDirectoryNotEmptyError(error)) {
+        throw error
+      }
+      await withFsCallSlot(lane, () => fs.rm(path, options))
+    }
+  }

Add EEXIST to the transient error codes in isTransientWindowsRemovalError.

[brennanb2025]

Superseded by #23837. This PR kept Orca deleting removed worktrees itself and limited how much of Orca's file I/O the delete could use. A run on a real Windows machine showed Node's native recursive delete follows directory junctions out of the tree, and the approach matched no comparable app. #23837 instead lets git delete the checkout in its own process, as before #12416, with the card showing Deleting until git finishes. Closing this one.