fix(main): big background deletes no longer stall chat sends and other file I/O

.github/workflows/pr.yml

@@ -1080,6 +1080,8 @@ jobs:
           src/main/windows/windows-process-table-native-addon.win32.test.ts
           src/main/persistence/profile-state/profile-state-access-windows-native.win32.test.ts
           src/main/windows-live-tree-kill.win32.test.ts
+          src/main/tree-removal-walk.test.ts
+          src/main/host-tree-removal.test.ts
           src/main/wsl/wsl-runner.test.ts
           src/main/wsl/wsl-guest-environment.test.ts
           src/main/wsl/wsl-invocation-boundary.test.ts

config/scripts/pr-code-change-scope.mjs

@@ -288,6 +288,8 @@ const WINDOWS_PACKAGE_TESTS = [
   'src/main/windows/windows-process-table-native-addon.win32.test.ts',
   'src/main/persistence/profile-state/profile-state-access-windows-native.win32.test.ts',
   'src/main/windows-live-tree-kill.win32.test.ts',
+  'src/main/tree-removal-walk.test.ts',
+  'src/main/host-tree-removal.test.ts',
   'src/main/wsl/wsl-runner.test.ts',
   'src/main/wsl/wsl-guest-environment.test.ts',
   'src/main/wsl/wsl-invocation-boundary.test.ts',

src/main/ai-vault-search/session-search-index-test-fixture.ts

@@ -1,7 +1,7 @@
 import { mkdtemp } from 'node:fs/promises'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
-import { removeTree } from '../../shared/windows-transient-lock-removal'
+import { removeTreeSync } from '../../shared/windows-transient-lock-removal'
 import type { AiVaultSession } from '../../shared/ai-vault-types'
 import type { SessionFileCandidate } from '../ai-vault/session-scanner-types'
 import { TranscriptMessageChannel } from '../ai-vault/session-transcript-channel'
@@ -118,7 +118,7 @@ export async function openSessionSearchIndexFile(name: string): Promise<SessionS
         open = false
         db.close()
       }
-      await removeTree(root)
+      removeTreeSync(root)
     }
   }
 }

src/main/asar-transparent-fs.test.ts

@@ -1,24 +1,30 @@
-import { mkdir, mkdtemp, writeFile } from 'node:fs/promises'
+import { mkdir, mkdtemp, readdir, writeFile } from 'node:fs/promises'
 import { existsSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { afterAll, describe, expect, it } from 'vitest'
-import { rm } from './asar-transparent-fs'
+import { asarTransparentFs } from './asar-transparent-fs'
 
 // Why not an asar fixture here: plain Node has no asar shim to see through, so the archive case can
 // only be settled by the real binary — `host-tree-removal-asar.electron.test.ts` does that. What
 // this pins is the other half: outside Electron `original-fs` does not resolve, and the helper has
 // to degrade to `node:fs/promises` rather than throw at first use.
 const roots: string[] = []
 
+const { rm } = asarTransparentFs()
+
 afterAll(async () => {
   for (const root of roots) {
     await rm(root, { recursive: true, force: true }).catch(() => {})
   }
 })
 
-describe('asar-transparent rm', () => {
-  it('removes a tree recursively where `original-fs` is unresolvable', async () => {
+describe('asar-transparent fs', () => {
+  it('falls back to `node:fs/promises` where `original-fs` is unresolvable', () => {
+    expect(asarTransparentFs().readdir).toBe(readdir)
+  })
+
+  it('removes a tree recursively outside Electron', async () => {
     expect(process.versions.electron).toBeUndefined()
     const root = await mkdtemp(join(tmpdir(), 'orca-asar-transparent-'))
     roots.push(root)

src/main/asar-transparent-fs.ts

@@ -1,35 +1,37 @@
-// Why: Electron patches `fs` so a `*.asar` file reports `isDirectory() === true`, so Node's
-// recursive `rm` descends into the archive, tries to `rmdir` a real file, and fails the parent with
-// ENOTEMPTY. Every worktree that has ever run `pnpm install` carries at least one
-// (`node_modules/.pnpm/electron@…/…/Electron.app/Contents/Resources/default_app.asar`), so a
+// Why: Electron patches `fs` so a `*.asar` file reports `isDirectory() === true` and lists the
+// archive's contents, so a tree delete descends into the archive, tries to `rmdir` a real file, and
+// fails the parent with ENOTEMPTY. Every worktree that has ever run `pnpm install` carries at least
+// one (`node_modules/.pnpm/electron@…/…/Electron.app/Contents/Resources/default_app.asar`), so a
 // worktree removal aborts there deterministically — the residue is not a concurrent-writer race and
 // no amount of retrying clears it. `original-fs` is Electron's unpatched `fs`; unlike
-// `process.noAsar` it is scoped to this call rather than to the whole process, which matters because
-// a multi-GB removal runs for seconds while the main process may still be loading modules out of
-// `app.asar`. See `cli/appimage-payload-removal.ts` for the same bug at a call site short enough to
-// use the process-global flag.
+// `process.noAsar` it is scoped to these calls rather than to the whole process, which matters
+// because a multi-GB removal runs for seconds while the main process may still be loading modules
+// out of `app.asar`.
 
-import { rm as nodeRm } from 'node:fs/promises'
+import * as nodeFsPromises from 'node:fs/promises'
 import { createRequire } from 'node:module'
 
-type Rm = typeof nodeRm
+export type AsarTransparentFs = Pick<
+  typeof nodeFsPromises,
+  'lstat' | 'readdir' | 'rm' | 'rmdir' | 'unlink'
+>
 
-let resolvedRm: Rm | undefined
+let resolved: AsarTransparentFs | undefined
 
-function resolveRm(): Rm {
+function resolveFs(): AsarTransparentFs {
   try {
-    // Why require and not an import: `original-fs` only exists inside Electron, so vitest, the
-    // `orca` CLI and the plain-node entrypoints must resolve `node:fs/promises` instead — and there
-    // the shim does not exist either, so plain `fs` is already asar-transparent.
-    const originalFs = createRequire(__filename)('original-fs') as { promises?: { rm?: Rm } }
-    return typeof originalFs.promises?.rm === 'function' ? originalFs.promises.rm : nodeRm
+    // Why require and not an import: `original-fs` only exists inside Electron (main and the
+    // run-as-node daemon), so vitest, the `orca` CLI, plain-node entrypoints and Bun must resolve
+    // `node:fs/promises` instead — and there the shim does not exist, so plain `fs` already is.
+    const originalFs: { promises?: AsarTransparentFs } = createRequire(__filename)('original-fs')
+    return originalFs.promises ?? nodeFsPromises
   } catch {
-    return nodeRm
+    return nodeFsPromises
   }
 }
 
-/** `fs.promises.rm` that sees a `*.asar` as the file it is rather than as a directory. */
-export const rm: Rm = (path, options) => {
-  resolvedRm ??= resolveRm()
-  return resolvedRm(path, options)
+/** `fs.promises` calls that see a `*.asar` as the file it is rather than as a directory. */
+export function asarTransparentFs(): AsarTransparentFs {
+  resolved ??= resolveFs()
+  return resolved
 }

src/main/browser/browser-client-download-relay.ts

@@ -1,6 +1,6 @@
 import { randomUUID } from 'node:crypto'
 import { mkdirSync, rmSync } from 'node:fs'
-import { open, rm, stat } from 'node:fs/promises'
+import { open, stat } from 'node:fs/promises'
 import path from 'node:path'
 
 import {
@@ -11,6 +11,7 @@ import {
 } from '../../shared/browser-client-file-channel-protocol'
 import type { BrowserClientHostedPageInventory } from '../../shared/browser-client-host-protocol'
 import type { BrowserClientFileChannelTransport } from './browser-client-file-channel-transport'
+import { removeHostTree } from '../host-tree-removal'
 
 export const BROWSER_CLIENT_FILE_CHANNEL_WRITE_METHOD = 'browser.clientHost.fileChannel.write'
 export const BROWSER_CLIENT_FILE_CHANNEL_ABORT_METHOD = 'browser.clientHost.fileChannel.abort'
@@ -73,7 +74,7 @@ const nodeRelayFilesystem: RelayFilesystem = {
   },
   size: async (filePath) => (await stat(filePath)).size,
   removeDirectory: async (directory) => {
-    await rm(directory, { recursive: true, force: true })
+    await removeHostTree(directory)
   }
 }
 

src/main/browser/browser-client-upload-staging.test.ts

@@ -1,25 +1,25 @@
 import { existsSync } from 'node:fs'
-import type * as NodeFsPromises from 'node:fs/promises'
 import { mkdir, mkdtemp, readFile, readdir, realpath, rm, writeFile } from 'node:fs/promises'
 import { tmpdir } from 'node:os'
 import path from 'node:path'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import type * as HostTreeRemoval from '../host-tree-removal'
 
 import {
   BROWSER_CLIENT_UPLOAD_STAGING_MAX_BYTES_PER_PAGE,
   BROWSER_CLIENT_UPLOAD_STAGING_MAX_COMMANDS_PER_PAGE,
   BrowserClientUploadStaging
 } from './browser-client-upload-staging'
 
-const nodeRemovals = vi.hoisted(() => ({ rm: [] as { target: unknown; options: unknown }[] }))
+const hostTreeRemovals = vi.hoisted((): { targets: string[] } => ({ targets: [] }))
 
-vi.mock('node:fs/promises', async (importOriginal) => {
-  const actual = await importOriginal<typeof NodeFsPromises>()
+vi.mock('../host-tree-removal', async (importOriginal) => {
+  const actual = await importOriginal<typeof HostTreeRemoval>()
   return {
     ...actual,
-    rm: (target: unknown, options: unknown) => {
-      nodeRemovals.rm.push({ target, options })
-      return (actual.rm as (t: unknown, o: unknown) => Promise<void>)(target, options)
+    removeHostTree: (target: string) => {
+      hostTreeRemovals.targets.push(target)
+      return actual.removeHostTree(target)
     }
   }
 })
@@ -310,23 +310,22 @@ describe('BrowserClientUploadStaging', () => {
     expect(staging.activeStagingCount()).toBe(1)
   })
 
-  it('retries the staged removal so a briefly held file is not orphaned', async () => {
+  it('removes through the retrying host tree removal so a briefly held file is not orphaned', async () => {
     const staging = new BrowserClientUploadStaging(stagingRoot)
     const staged = await staging.stage({
       browserPageId: 'page-1',
       pageHostGeneration: 1,
       files: [{ remotePath: 'a.txt', contents: Buffer.from('a') }]
     })
     const directory = staging.stagedDirectory(staged.stagingId)
-    nodeRemovals.rm.length = 0
+    if (directory === undefined) {
+      throw new Error('expected a staged directory')
+    }
+    hostTreeRemovals.targets.length = 0
 
     expect(await staging.release(staged.stagingId)).toBe(true)
 
-    expect(nodeRemovals.rm).toEqual([
-      {
-        target: directory,
-        options: { recursive: true, force: true, maxRetries: 3, retryDelay: 50 }
-      }
-    ])
+    expect(hostTreeRemovals.targets).toEqual([directory])
+    expect(existsSync(directory)).toBe(false)
   })
 })

src/main/browser/browser-client-upload-staging.ts

@@ -1,13 +1,14 @@
 import { randomUUID } from 'node:crypto'
 import { rmSync } from 'node:fs'
-import { mkdir, rm, writeFile } from 'node:fs/promises'
+import { mkdir, writeFile } from 'node:fs/promises'
 import path from 'node:path'
 
 import {
   BROWSER_CLIENT_FILE_CHANNEL_MAX_FILES_PER_COMMAND,
   BROWSER_CLIENT_FILE_CHANNEL_TRANSFER_MAX_BYTES
 } from '../../shared/browser-client-file-channel-protocol'
 import { normalizeBrowserDownloadFilename } from '../../shared/browser-download-filename'
+import { removeHostTree } from '../host-tree-removal'
 
 export type BrowserClientStagedUpload = {
   stagingId: string
@@ -53,7 +54,7 @@ const nodeStagingFilesystem: StagingFilesystem = {
     await writeFile(filePath, contents, { mode: 0o600 })
   },
   removeDirectory: async (directory) => {
-    await rm(directory, stagingRemovalOptions)
+    await removeHostTree(directory)
   },
   removeDirectorySync: (directory) => {
     rmSync(directory, stagingRemovalOptions)

src/main/browser/browser-route-partition-storage-dependencies.ts

@@ -1,5 +1,4 @@
 import { session } from 'electron'
-import { rm } from 'node:fs/promises'
 import {
   releaseBrowserRoutePartitionStorage,
   type BrowserRoutePartitionStorageDependencies
@@ -9,6 +8,7 @@ import {
   routePartitionDataRoot
 } from './browser-route-partition-binding-runtime'
 import { browserSessionRegistry } from './browser-session-registry'
+import { removeHostTree } from '../host-tree-removal'
 
 export function browserRoutePartitionStorageDependencies(
   isPartitionLive: (partition: string) => boolean
@@ -24,7 +24,7 @@ export function browserRoutePartitionStorageDependencies(
       browserSessionRegistry.clearRoutePartitionPolicies(partition)
     },
     // Why: force ignores an already-absent directory, keeping the sweep idempotent.
-    removePartitionDirectory: (directory) => rm(directory, { recursive: true, force: true })
+    removePartitionDirectory: (directory) => removeHostTree(directory)
   }
 }
 

src/main/cli/appimage-payload-removal.reentrancy.test.ts

@@ -1,9 +1,9 @@
 import { afterEach, describe, expect, it, vi } from 'vitest'
 
-// Why a mocked rm: the property under test is the ORDER in which overlapping removals settle, which
-// real filesystem timing cannot pin down. Deferreds make the interleaving exact.
-const { rmMock } = vi.hoisted(() => ({ rmMock: vi.fn() }))
-vi.mock('node:fs/promises', () => ({ rm: rmMock }))
+// Why a mocked removal: the property under test is the ORDER in which overlapping removals settle,
+// which real filesystem timing cannot pin down. Deferreds make the interleaving exact.
+const { removeHostTreeMock } = vi.hoisted(() => ({ removeHostTreeMock: vi.fn() }))
+vi.mock('../host-tree-removal', () => ({ removeHostTree: removeHostTreeMock }))
 
 const originalNoAsar = process.noAsar
 
@@ -21,45 +21,47 @@ function deferred(): { promise: Promise<void>; resolve: () => void } {
 }
 
 describe('removeExtractedAppImagePayload reentrancy', () => {
-  // The hazard is the FIRST removal settling while a later one is still running: a naive
-  // save/restore would hand the shim back and silently leak the rest of the second removal.
-  it('keeps asar interception disabled while a later removal is still running', async () => {
+  // The old hazard was a process-wide asar toggle handed back while a later removal still ran; the
+  // shared removal holds no such flag, so overlapping removals must leave it untouched throughout.
+  it('leaves asar interception untouched while overlapping removals settle', async () => {
     process.noAsar = false
     const first = deferred()
     const second = deferred()
-    rmMock.mockReset()
-    rmMock.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise)
+    removeHostTreeMock.mockReset()
+    removeHostTreeMock.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise)
     const { removeExtractedAppImagePayload } = await import('./appimage-payload-removal')
 
     const firstCall = removeExtractedAppImagePayload('/cache/gen-a')
     const secondCall = removeExtractedAppImagePayload('/cache/gen-b')
-    expect(process.noAsar).toBe(true)
+    expect(removeHostTreeMock.mock.calls).toEqual([['/cache/gen-a'], ['/cache/gen-b']])
+    expect(process.noAsar).toBe(false)
 
     first.resolve()
     await firstCall
-    // gen-b is still being removed: handing the shim back here is exactly the leak.
-    expect(process.noAsar).toBe(true)
+    expect(process.noAsar).toBe(false)
 
     second.resolve()
     await secondCall
     expect(process.noAsar).toBe(false)
   })
 
-  it('keeps the flag held when the first removal rejects mid-overlap', async () => {
+  it('rejects only the failed removal when the first rejects mid-overlap', async () => {
     process.noAsar = false
     const second = deferred()
-    rmMock.mockReset()
-    rmMock.mockRejectedValueOnce(new Error('EACCES')).mockReturnValueOnce(second.promise)
+    removeHostTreeMock.mockReset()
+    removeHostTreeMock
+      .mockRejectedValueOnce(new Error('EACCES'))
+      .mockReturnValueOnce(second.promise)
     const { removeExtractedAppImagePayload } = await import('./appimage-payload-removal')
 
     const firstCall = removeExtractedAppImagePayload('/cache/gen-a')
     const secondCall = removeExtractedAppImagePayload('/cache/gen-b')
 
     await expect(firstCall).rejects.toThrow('EACCES')
-    expect(process.noAsar).toBe(true)
+    expect(process.noAsar).toBe(false)
 
     second.resolve()
-    await secondCall
+    await expect(secondCall).resolves.toBeUndefined()
     expect(process.noAsar).toBe(false)
   })
 })

src/main/cli/appimage-payload-removal.test.ts

@@ -27,15 +27,17 @@ describe('removeExtractedAppImagePayload', () => {
     expect(existsSync(root)).toBe(false)
   })
 
-  it('disables asar interception for the removal', async () => {
+  // Why: `process.noAsar` is process-wide; flipping it mid-removal would change asar semantics for
+  // every concurrent reader in the process, and the unpatched-fs walk makes the toggle unnecessary.
+  it('removes the payload without toggling process-wide asar interception', async () => {
     const root = await makePayloadTree()
-    let observed: boolean | undefined
-    const originalRealpath = process.noAsar
+    const assigned: boolean[] = []
+    const noAsarBefore = process.noAsar
     Object.defineProperty(process, 'noAsar', {
       configurable: true,
-      get: () => observed ?? originalRealpath,
+      get: () => noAsarBefore,
       set: (value: boolean) => {
-        observed ??= value
+        assigned.push(value)
       }
     })
     try {
@@ -44,13 +46,14 @@ describe('removeExtractedAppImagePayload', () => {
       Object.defineProperty(process, 'noAsar', {
         configurable: true,
         writable: true,
-        value: originalRealpath
+        value: noAsarBefore
       })
     }
-    expect(observed).toBe(true)
+    expect(assigned).toEqual([])
+    expect(existsSync(root)).toBe(false)
   })
 
-  it('restores the previous asar setting after a failure', async () => {
+  it('surfaces a removal failure and leaves the asar setting untouched', async () => {
     process.noAsar = false
     await expect(
       removeExtractedAppImagePayload(join(tmpdir(), 'orca-missing', 'nested', '\0invalid'))