mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 16:02:50 +00:00
perf(worktrees): converge the trash sweep instead of re-walking doomed trees
`transientLockRemovalOptions()` only asked for `maxRetries` on Windows, and `removeHostTree`'s retry ladder was gated on `process.platform === 'win32'`. A concurrent writer is not Windows-specific: Spotlight/`mds`, a scanner, or a live process writing under the tree surface the same EBUSY/ENOTEMPTY/EPERM on macOS and Linux. So on POSIX the startup sweep got exactly one attempt per entry, failed, and re-issued the same guaranteed-to-fail walk on every launch. - Extend the retry policy to every platform. Windows keeps its error set, its message fallback, and its delays; the message fallback stays Windows-only because POSIX always sets a code. - Persist a per-entry failure ledger in the trash root so a repeatedly failing entry is retried on a 15m/1h/6h ladder rather than on every launch. Nothing is abandoned: the ladder clamps, records are pruned when the entry goes, and a torn ledger fails open to a full sweep. - Defer the sweep behind first paint, so its recursive readdir/rm no longer competes with window creation and worktree-catalog hydration.
This commit is contained in:
@@ -0,0 +1,96 @@
|
||||
import type * as NodeFsPromises from 'node:fs/promises'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { rmMock, delayMock } = vi.hoisted(() => ({
|
||||
rmMock: vi.fn<(path: string, options?: unknown) => Promise<void>>(),
|
||||
delayMock: vi.fn(async (_ms?: number) => undefined)
|
||||
}))
|
||||
|
||||
vi.mock('node:fs/promises', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof NodeFsPromises>()
|
||||
return { ...actual, rm: rmMock }
|
||||
})
|
||||
// Keep the retry ladder's real delays out of the test clock; only the attempt count is asserted.
|
||||
vi.mock('node:timers/promises', () => ({ setTimeout: delayMock }))
|
||||
|
||||
const { removeHostTree } = await import('./host-tree-removal')
|
||||
const { WINDOWS_RM_MAX_RETRIES } = await import('../shared/windows-transient-lock-removal')
|
||||
|
||||
/** POSIX-shaped so `toHostRemovalPath` is the identity on every platform under test. */
|
||||
const TRASH_ROOT = '/workspaces/.orca-worktree-trash'
|
||||
const TRASH_ENTRY = `${TRASH_ROOT}/wt-1700000000000-abcdef01`
|
||||
|
||||
function transientError(code: string): NodeJS.ErrnoException {
|
||||
return Object.assign(new Error(`${code}: transient`), { code })
|
||||
}
|
||||
|
||||
function withPlatform(platform: NodeJS.Platform): void {
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: platform })
|
||||
}
|
||||
|
||||
const originalPlatform = process.platform
|
||||
|
||||
afterEach(() => {
|
||||
withPlatform(originalPlatform)
|
||||
rmMock.mockReset()
|
||||
delayMock.mockClear()
|
||||
})
|
||||
|
||||
describe('removeHostTree', () => {
|
||||
// Why every platform: a concurrent writer (Spotlight/`mds`, a scanner, a live process under the
|
||||
// tree) is not a Windows-only hazard, and a one-shot removal on POSIX never converged.
|
||||
for (const platform of ['darwin', 'linux', 'win32'] as const) {
|
||||
it(`retries a transient ENOTEMPTY until it succeeds on ${platform}`, async () => {
|
||||
withPlatform(platform)
|
||||
rmMock.mockRejectedValueOnce(transientError('ENOTEMPTY'))
|
||||
rmMock.mockRejectedValueOnce(transientError('ENOTEMPTY'))
|
||||
rmMock.mockResolvedValueOnce(undefined)
|
||||
|
||||
await expect(removeHostTree(TRASH_ENTRY)).resolves.toBeUndefined()
|
||||
|
||||
expect(rmMock).toHaveBeenCalledTimes(3)
|
||||
// The retry may only change how many times the SAME path is attempted.
|
||||
expect(new Set(rmMock.mock.calls.map((call) => call[0]))).toEqual(new Set([TRASH_ENTRY]))
|
||||
expect(rmMock.mock.calls[0]?.[1]).toEqual({
|
||||
recursive: true,
|
||||
force: true,
|
||||
maxRetries: WINDOWS_RM_MAX_RETRIES,
|
||||
retryDelay: expect.any(Number)
|
||||
})
|
||||
})
|
||||
|
||||
it(`gives up on a non-transient failure without retrying on ${platform}`, async () => {
|
||||
withPlatform(platform)
|
||||
rmMock.mockRejectedValue(transientError('EIO'))
|
||||
|
||||
await expect(removeHostTree(TRASH_ENTRY)).rejects.toThrow('EIO')
|
||||
expect(rmMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it(`stops after a bounded number of attempts on ${platform}`, async () => {
|
||||
withPlatform(platform)
|
||||
rmMock.mockRejectedValue(transientError('EBUSY'))
|
||||
|
||||
await expect(removeHostTree(TRASH_ENTRY)).rejects.toThrow('EBUSY')
|
||||
expect(rmMock).toHaveBeenCalledTimes(5)
|
||||
expect(new Set(rmMock.mock.calls.map((call) => call[0]))).toEqual(new Set([TRASH_ENTRY]))
|
||||
})
|
||||
}
|
||||
|
||||
it('does not retry prose-only failures off Windows, where every error carries a code', async () => {
|
||||
withPlatform('linux')
|
||||
rmMock.mockRejectedValue(new Error('directory not empty'))
|
||||
|
||||
await expect(removeHostTree(TRASH_ENTRY)).rejects.toThrow('directory not empty')
|
||||
expect(rmMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('still retries a codeless Windows failure by its message', async () => {
|
||||
withPlatform('win32')
|
||||
rmMock.mockRejectedValueOnce(new Error('directory not empty'))
|
||||
rmMock.mockResolvedValueOnce(undefined)
|
||||
|
||||
await expect(removeHostTree(TRASH_ENTRY)).resolves.toBeUndefined()
|
||||
expect(rmMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
@@ -1,15 +1,19 @@
|
||||
// Why: every recursive host delete Orca performs (worktrees, terminal history, quarantined recovery
|
||||
// generations) hits the same Windows stickiness — AV/indexers/late handle releases surface transient
|
||||
// EBUSY/ENOTEMPTY/EPERM on a tree Node just emptied. One helper so no call site forgets the retries.
|
||||
// generations) races whatever else is touching the tree — AV/indexers/late handle releases on
|
||||
// Windows, Spotlight/`mds` or a live process writing under it on macOS and Linux — and all of them
|
||||
// surface transient EBUSY/ENOTEMPTY/EPERM. One helper so no call site forgets the retries.
|
||||
|
||||
import { rm } from 'node:fs/promises'
|
||||
import { win32 } from 'node:path'
|
||||
import { setTimeout as delay } from 'node:timers/promises'
|
||||
import { isWindowsAbsolutePathLike } from '../shared/cross-platform-path'
|
||||
import { isWslUncPath } from '../shared/wsl-paths'
|
||||
import { transientLockRemovalOptions } from '../shared/windows-transient-lock-removal'
|
||||
import {
|
||||
isTransientRemovalError,
|
||||
transientLockRemovalOptions
|
||||
} from '../shared/windows-transient-lock-removal'
|
||||
|
||||
const WINDOWS_REMOVE_RETRY_DELAYS_MS = [250, 500, 1_000, 2_000]
|
||||
const REMOVE_RETRY_DELAYS_MS = [250, 500, 1_000, 2_000]
|
||||
|
||||
/** Convert a native host filesystem path to the Win32 long-path namespace. */
|
||||
export function toHostFilesystemPath(targetPath: string): string {
|
||||
@@ -28,24 +32,11 @@ export function toHostRemovalPath(targetPath: string): string {
|
||||
return toHostFilesystemPath(targetPath)
|
||||
}
|
||||
|
||||
function isTransientWindowsRemovalError(error: unknown): boolean {
|
||||
if (process.platform !== 'win32' || typeof error !== 'object' || error === null) {
|
||||
return false
|
||||
}
|
||||
const code = 'code' in error && typeof error.code === 'string' ? error.code : undefined
|
||||
if (code && ['EBUSY', 'ENOTEMPTY', 'EPERM'].includes(code)) {
|
||||
return true
|
||||
}
|
||||
const message = 'message' in error && typeof error.message === 'string' ? error.message : ''
|
||||
return /directory not empty|resource busy|operation not permitted/i.test(message)
|
||||
}
|
||||
|
||||
/** Recursively remove a host directory tree, retrying the transient Windows failures. */
|
||||
/** Recursively remove a host directory tree, retrying the transient concurrent-writer failures. */
|
||||
export async function removeHostTree(targetPath: string): Promise<void> {
|
||||
const removalPath = toHostRemovalPath(targetPath)
|
||||
const retryDelays = process.platform === 'win32' ? WINDOWS_REMOVE_RETRY_DELAYS_MS : []
|
||||
// Why: large Windows trees commonly surface transient ENOTEMPTY/EPERM while Node walks and
|
||||
// removes nested directories; Node's own retries absorb that before the loop below has to.
|
||||
// Why: large trees commonly surface transient ENOTEMPTY/EPERM while Node walks and removes nested
|
||||
// directories; Node's own retries absorb that before the loop below has to.
|
||||
const rmOptions = transientLockRemovalOptions()
|
||||
let attempt = 0
|
||||
|
||||
@@ -54,12 +45,12 @@ export async function removeHostTree(targetPath: string): Promise<void> {
|
||||
await rm(removalPath, rmOptions)
|
||||
return
|
||||
} catch (error) {
|
||||
if (attempt >= retryDelays.length || !isTransientWindowsRemovalError(error)) {
|
||||
if (attempt >= REMOVE_RETRY_DELAYS_MS.length || !isTransientRemovalError(error)) {
|
||||
throw error
|
||||
}
|
||||
// Why: Git/Node recursive deletes on Windows can observe a just-emptied
|
||||
// directory before antivirus/indexers/handles release it.
|
||||
await delay(retryDelays[attempt])
|
||||
// Why a whole second pass and not just Node's inner retries: a writer that outlives them
|
||||
// leaves directories Node already descended into, so only a fresh walk can finish the tree.
|
||||
await delay(REMOVE_RETRY_DELAYS_MS[attempt])
|
||||
attempt += 1
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { app, type BrowserWindow } from 'electron'
|
||||
import { runAfterFirstWindowShown } from '../startup/first-window-deferral'
|
||||
import { reportSecretProtectionGap } from './secret-protection-report'
|
||||
|
||||
/**
|
||||
@@ -72,21 +72,5 @@ export function scheduleSecretProtectionGapReport({
|
||||
}
|
||||
}
|
||||
|
||||
let ran = false
|
||||
const run = (): void => {
|
||||
if (ran) {
|
||||
return
|
||||
}
|
||||
ran = true
|
||||
clearTimeout(fallback)
|
||||
// Why setImmediate: keep the blocking keyring probe off the event handler that
|
||||
// reveals the window, so the reveal paints first.
|
||||
setImmediate(report)
|
||||
}
|
||||
|
||||
const fallback = setTimeout(run, REPORT_FALLBACK_MS)
|
||||
fallback.unref?.()
|
||||
app.once('browser-window-created', (_event: Electron.Event, window: BrowserWindow) => {
|
||||
window.once('ready-to-show', run)
|
||||
})
|
||||
runAfterFirstWindowShown(report, REPORT_FALLBACK_MS)
|
||||
}
|
||||
|
||||
@@ -159,9 +159,27 @@ describe('local worktree filesystem runtime access', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('does not retry host removal failures outside Windows', async () => {
|
||||
// Why not Windows-only: Spotlight/`mds`, a scanner, or a live process writing under the tree race
|
||||
// a POSIX removal the same way, and a one-shot rm left the worktree on disk for good.
|
||||
it('retries transient host removal failures outside Windows too', async () => {
|
||||
vi.useFakeTimers()
|
||||
await withPlatform('linux', async () => {
|
||||
const error = Object.assign(new Error('Directory not empty'), { code: 'ENOTEMPTY' })
|
||||
rmMock.mockRejectedValueOnce(error).mockResolvedValueOnce(undefined)
|
||||
|
||||
const removal = removeLocalWorktreePath('/repo/feature')
|
||||
await vi.advanceTimersByTimeAsync(250)
|
||||
|
||||
await expect(removal).resolves.toBeUndefined()
|
||||
expect(rmMock).toHaveBeenCalledTimes(2)
|
||||
// The retry may only change how many times the SAME path is attempted.
|
||||
expect(new Set(rmMock.mock.calls.map((call) => call[0]))).toEqual(new Set(['/repo/feature']))
|
||||
})
|
||||
})
|
||||
|
||||
it('does not retry a non-transient host removal failure outside Windows', async () => {
|
||||
await withPlatform('linux', async () => {
|
||||
const error = Object.assign(new Error('I/O error'), { code: 'EIO' })
|
||||
rmMock.mockRejectedValue(error)
|
||||
|
||||
await expect(removeLocalWorktreePath('/repo/feature')).rejects.toBe(error)
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
import { app, type BrowserWindow } from 'electron'
|
||||
|
||||
/**
|
||||
* Run `task` once the first window can paint, or after `fallbackMs` if it never does.
|
||||
*
|
||||
* For startup work nothing on the critical path consumes: a probe or a disk sweep started before the
|
||||
* window exists competes with window creation for the same main thread and libuv threadpool, and the
|
||||
* user sees that as the app being slow to open.
|
||||
*
|
||||
* Why a fallback as well as the window event: `ready-to-show` can fail to fire at all when the
|
||||
* GPU/driver cannot present (see main-window-state-lifecycle), and headless serve has no window.
|
||||
*/
|
||||
export function runAfterFirstWindowShown(task: () => void, fallbackMs: number): void {
|
||||
let ran = false
|
||||
const run = (): void => {
|
||||
if (ran) {
|
||||
return
|
||||
}
|
||||
ran = true
|
||||
clearTimeout(fallback)
|
||||
// Why setImmediate: keep the work off the event handler that reveals the window, so it paints first.
|
||||
setImmediate(task)
|
||||
}
|
||||
const fallback = setTimeout(run, fallbackMs)
|
||||
fallback.unref?.()
|
||||
app.once('browser-window-created', (_event: Electron.Event, window: BrowserWindow) => {
|
||||
window.once('ready-to-show', run)
|
||||
})
|
||||
}
|
||||
@@ -32,8 +32,12 @@ import {
|
||||
import { initializeMainProcessAutomations } from './main-process-automations'
|
||||
import { initializeMainProcessPlugins } from './main-process-plugins'
|
||||
import { collectWorktreeTrashSweepRoots, sweepStaleWorktreeTrash } from '../worktree-trash'
|
||||
import { runAfterFirstWindowShown } from './first-window-deferral'
|
||||
import { logStartupMilestone } from './startup-diagnostics'
|
||||
|
||||
// Headless serve never opens a window, so the sweep still has to run off a timer there.
|
||||
const WORKTREE_TRASH_SWEEP_FALLBACK_MS = 15_000
|
||||
|
||||
export async function initializeReadyRuntimeServices(): Promise<void> {
|
||||
const store = state.store
|
||||
if (!store) {
|
||||
@@ -74,12 +78,16 @@ export async function initializeReadyRuntimeServices(): Promise<void> {
|
||||
state.emulatorBridge = new EmulatorBridge()
|
||||
runtime.setEmulatorBridge(state.emulatorBridge)
|
||||
// Why: worktree deletion renames the checkout aside and deletes it in the background, so a quit or
|
||||
// crash mid-delete can leave the moved directory on disk.
|
||||
void sweepStaleWorktreeTrash(
|
||||
collectWorktreeTrashSweepRoots(store.getRepos(), store.getSettings())
|
||||
).catch((error) => {
|
||||
console.warn('[worktrees] Failed to sweep leftover worktree directories:', error)
|
||||
})
|
||||
// crash mid-delete can leave the moved directory on disk. Why deferred: the sweep's recursive
|
||||
// readdir/rm runs on the same libuv threadpool the window's first paint and worktree-catalog
|
||||
// hydration are reading disk on, and nothing on the startup path consumes its result.
|
||||
runAfterFirstWindowShown(() => {
|
||||
void sweepStaleWorktreeTrash(
|
||||
collectWorktreeTrashSweepRoots(store.getRepos(), store.getSettings())
|
||||
).catch((error) => {
|
||||
console.warn('[worktrees] Failed to sweep leftover worktree directories:', error)
|
||||
})
|
||||
}, WORKTREE_TRASH_SWEEP_FALLBACK_MS)
|
||||
nativeTheme.themeSource = store.getSettings().theme ?? 'system'
|
||||
// Why (#16441): the real-home grant runs a codex app-server session. It stays
|
||||
// ordered before managed-hook reconciliation — an incapable host must re-arm
|
||||
|
||||
@@ -0,0 +1,171 @@
|
||||
import { existsSync } from 'node:fs'
|
||||
import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { dirname, join, sep } from 'node:path'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { removeHostTreeMock } = vi.hoisted(() => ({
|
||||
removeHostTreeMock: vi.fn<(path: string) => Promise<void>>()
|
||||
}))
|
||||
vi.mock('./host-tree-removal', () => ({ removeHostTree: removeHostTreeMock }))
|
||||
|
||||
const { sweepStaleWorktreeTrash, WORKTREE_TRASH_DIR_NAME } = await import('./worktree-trash')
|
||||
const { TRASH_SWEEP_MAX_RETRY_DELAY_MS, TRASH_SWEEP_RETRY_DELAYS_MS, trashSweepBackoffPath } =
|
||||
await import('./worktree-trash-sweep-backoff')
|
||||
|
||||
const ENTRY = 'wt-1700000000000-abcdef01'
|
||||
const OTHER_ENTRY = 'wt-1700000000001-abcdef02'
|
||||
|
||||
let scratchDir = ''
|
||||
let trashRoot = ''
|
||||
|
||||
function transientFailure(): NodeJS.ErrnoException {
|
||||
return Object.assign(new Error('ENOTEMPTY: directory not empty'), { code: 'ENOTEMPTY' })
|
||||
}
|
||||
|
||||
async function readBackoff(): Promise<Record<string, { failures: number; nextAttemptAt: number }>> {
|
||||
return JSON.parse(await readFile(trashSweepBackoffPath(trashRoot), 'utf8'))
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
scratchDir = await mkdtemp(join(tmpdir(), 'orca-trash-backoff-'))
|
||||
trashRoot = join(scratchDir, WORKTREE_TRASH_DIR_NAME)
|
||||
await mkdir(join(trashRoot, ENTRY, 'node_modules'), { recursive: true })
|
||||
removeHostTreeMock.mockReset()
|
||||
removeHostTreeMock.mockRejectedValue(transientFailure())
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(scratchDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
describe('sweepStaleWorktreeTrash backoff', () => {
|
||||
it('does not re-walk a failing entry on the next launch', async () => {
|
||||
await sweepStaleWorktreeTrash([scratchDir])
|
||||
await sweepStaleWorktreeTrash([scratchDir])
|
||||
|
||||
// Without the persisted record, every launch re-walks the same doomed tree forever.
|
||||
expect(removeHostTreeMock).toHaveBeenCalledTimes(1)
|
||||
expect(existsSync(join(trashRoot, ENTRY))).toBe(true)
|
||||
})
|
||||
|
||||
it('reports the deferral rather than counting it as swept', async () => {
|
||||
expect(await sweepStaleWorktreeTrash([scratchDir])).toEqual({ removed: 0, deferred: 0 })
|
||||
expect(await sweepStaleWorktreeTrash([scratchDir])).toEqual({ removed: 0, deferred: 1 })
|
||||
})
|
||||
|
||||
it('persists the record before the sweep returns, so a quit mid-sweep keeps it', async () => {
|
||||
await sweepStaleWorktreeTrash([scratchDir])
|
||||
|
||||
const backoff = await readBackoff()
|
||||
expect(backoff[ENTRY].failures).toBe(1)
|
||||
expect(backoff[ENTRY].nextAttemptAt).toBeGreaterThan(Date.now())
|
||||
})
|
||||
|
||||
it('retries the entry once the backoff window has elapsed, and clears the record on success', async () => {
|
||||
await sweepStaleWorktreeTrash([scratchDir])
|
||||
await writeFile(
|
||||
trashSweepBackoffPath(trashRoot),
|
||||
JSON.stringify({ [ENTRY]: { failures: 1, nextAttemptAt: Date.now() - 1 } }),
|
||||
'utf8'
|
||||
)
|
||||
removeHostTreeMock.mockImplementation(async (path) => {
|
||||
await rm(path, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
expect(await sweepStaleWorktreeTrash([scratchDir])).toEqual({ removed: 1, deferred: 0 })
|
||||
expect(removeHostTreeMock).toHaveBeenCalledTimes(2)
|
||||
expect(existsSync(join(trashRoot, ENTRY))).toBe(false)
|
||||
// An empty ledger is deleted rather than left as a permanent file in the trash root.
|
||||
expect(existsSync(trashSweepBackoffPath(trashRoot))).toBe(false)
|
||||
})
|
||||
|
||||
it('escalates the delay with each consecutive failure', async () => {
|
||||
for (const [index, delayMs] of TRASH_SWEEP_RETRY_DELAYS_MS.entries()) {
|
||||
await writeFile(
|
||||
trashSweepBackoffPath(trashRoot),
|
||||
JSON.stringify({ [ENTRY]: { failures: index, nextAttemptAt: Date.now() - 1 } }),
|
||||
'utf8'
|
||||
)
|
||||
const before = Date.now()
|
||||
await sweepStaleWorktreeTrash([scratchDir])
|
||||
|
||||
const backoff = await readBackoff()
|
||||
expect(backoff[ENTRY].failures).toBe(index + 1)
|
||||
expect(backoff[ENTRY].nextAttemptAt).toBeGreaterThanOrEqual(before + delayMs)
|
||||
}
|
||||
})
|
||||
|
||||
it('clamps at the last ladder step rather than deferring an entry forever', async () => {
|
||||
await writeFile(
|
||||
trashSweepBackoffPath(trashRoot),
|
||||
JSON.stringify({ [ENTRY]: { failures: 99, nextAttemptAt: Date.now() - 1 } }),
|
||||
'utf8'
|
||||
)
|
||||
const before = Date.now()
|
||||
await sweepStaleWorktreeTrash([scratchDir])
|
||||
const after = Date.now()
|
||||
|
||||
const { nextAttemptAt } = (await readBackoff())[ENTRY]
|
||||
expect(nextAttemptAt).toBeGreaterThanOrEqual(before + TRASH_SWEEP_MAX_RETRY_DELAY_MS)
|
||||
expect(nextAttemptAt).toBeLessThanOrEqual(after + TRASH_SWEEP_MAX_RETRY_DELAY_MS)
|
||||
})
|
||||
|
||||
it('retries an entry parked by a clock that has since moved backwards', async () => {
|
||||
await writeFile(
|
||||
trashSweepBackoffPath(trashRoot),
|
||||
JSON.stringify({
|
||||
[ENTRY]: { failures: 1, nextAttemptAt: Date.now() + TRASH_SWEEP_MAX_RETRY_DELAY_MS * 10 }
|
||||
}),
|
||||
'utf8'
|
||||
)
|
||||
|
||||
expect(await sweepStaleWorktreeTrash([scratchDir])).toEqual({ removed: 0, deferred: 0 })
|
||||
expect(removeHostTreeMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('drops a torn ledger and sweeps everything, exactly as before it existed', async () => {
|
||||
await writeFile(
|
||||
trashSweepBackoffPath(trashRoot),
|
||||
'{"wt-1700000000000-abcdef01": {"fail',
|
||||
'utf8'
|
||||
)
|
||||
|
||||
expect(await sweepStaleWorktreeTrash([scratchDir])).toEqual({ removed: 0, deferred: 0 })
|
||||
expect(removeHostTreeMock).toHaveBeenCalledTimes(1)
|
||||
expect((await readBackoff())[ENTRY].failures).toBe(1)
|
||||
})
|
||||
|
||||
it('prunes records for entries that are no longer on disk', async () => {
|
||||
await writeFile(
|
||||
trashSweepBackoffPath(trashRoot),
|
||||
JSON.stringify({ [OTHER_ENTRY]: { failures: 4, nextAttemptAt: Date.now() + 60_000 } }),
|
||||
'utf8'
|
||||
)
|
||||
|
||||
await sweepStaleWorktreeTrash([scratchDir])
|
||||
|
||||
expect(Object.keys(await readBackoff())).toEqual([ENTRY])
|
||||
})
|
||||
|
||||
it('never offers the removal a path outside a trash root', async () => {
|
||||
const liveWorktree = join(scratchDir, 'feature')
|
||||
await mkdir(join(liveWorktree, 'src'), { recursive: true })
|
||||
await mkdir(join(trashRoot, OTHER_ENTRY), { recursive: true })
|
||||
await mkdir(join(trashRoot, 'unrelated-directory'), { recursive: true })
|
||||
await writeFile(join(trashRoot, 'wt-notes.txt'), 'keep me\n')
|
||||
|
||||
await sweepStaleWorktreeTrash([scratchDir])
|
||||
|
||||
const attempted = removeHostTreeMock.mock.calls.map((call) => call[0])
|
||||
expect(new Set(attempted)).toEqual(
|
||||
new Set([join(trashRoot, ENTRY), join(trashRoot, OTHER_ENTRY)])
|
||||
)
|
||||
for (const path of attempted) {
|
||||
expect(dirname(path).endsWith(`${sep}${WORKTREE_TRASH_DIR_NAME}`)).toBe(true)
|
||||
}
|
||||
expect(existsSync(liveWorktree)).toBe(true)
|
||||
expect(existsSync(join(trashRoot, 'unrelated-directory'))).toBe(true)
|
||||
expect(existsSync(join(trashRoot, 'wt-notes.txt'))).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,87 @@
|
||||
// Why: a trashed checkout that a live process keeps writing under fails removal on every launch, and
|
||||
// the sweep had no memory of that — measured on one machine at 267 leftover entries / 3804 inodes,
|
||||
// a 265 ms warm walk (worse cold) re-issued at every cold start against trees it is guaranteed to
|
||||
// fail. This records the failure so the retry is scheduled instead of immediate. It only ever delays
|
||||
// a retry; nothing here abandons an entry, and the disk is still reclaimed once the writer goes.
|
||||
|
||||
import { readFile, rm, writeFile } from 'node:fs/promises'
|
||||
import { join } from 'node:path'
|
||||
|
||||
/** Deliberately not a `wt-…` name, so `isWorktreeTrashEntryName` never treats this as sweepable. */
|
||||
export const TRASH_SWEEP_BACKOFF_FILE_NAME = '.orca-sweep-backoff.json'
|
||||
|
||||
// Consecutive-failure ladder, clamped at the last step. Launches a day apart still retry every
|
||||
// entry; only a burst of relaunches stops re-walking the trees that just failed.
|
||||
export const TRASH_SWEEP_RETRY_DELAYS_MS = [15 * 60_000, 60 * 60_000, 6 * 60 * 60_000]
|
||||
export const TRASH_SWEEP_MAX_RETRY_DELAY_MS = Math.max(...TRASH_SWEEP_RETRY_DELAYS_MS)
|
||||
|
||||
export type TrashSweepFailure = { readonly failures: number; readonly nextAttemptAt: number }
|
||||
export type TrashSweepFailures = Map<string, TrashSweepFailure>
|
||||
|
||||
export function trashSweepBackoffPath(trashRoot: string): string {
|
||||
return join(trashRoot, TRASH_SWEEP_BACKOFF_FILE_NAME)
|
||||
}
|
||||
|
||||
function parseFailures(parsed: unknown): TrashSweepFailures {
|
||||
const failures: TrashSweepFailures = new Map()
|
||||
if (typeof parsed !== 'object' || parsed === null) {
|
||||
return failures
|
||||
}
|
||||
for (const [entry, record] of Object.entries(parsed as Record<string, unknown>)) {
|
||||
if (typeof record !== 'object' || record === null) {
|
||||
continue
|
||||
}
|
||||
const { failures: count, nextAttemptAt } = record as Partial<TrashSweepFailure>
|
||||
if (Number.isFinite(count) && Number.isFinite(nextAttemptAt) && (count as number) > 0) {
|
||||
failures.set(entry, { failures: count as number, nextAttemptAt: nextAttemptAt as number })
|
||||
}
|
||||
}
|
||||
return failures
|
||||
}
|
||||
|
||||
export async function readTrashSweepFailures(trashRoot: string): Promise<TrashSweepFailures> {
|
||||
const path = trashSweepBackoffPath(trashRoot)
|
||||
try {
|
||||
return parseFailures(JSON.parse(await readFile(path, 'utf8')))
|
||||
} catch (error) {
|
||||
// Fail open: a torn or hand-edited file must not pin the sweep, so drop it and sweep everything.
|
||||
if ((error as NodeJS.ErrnoException).code !== 'ENOENT') {
|
||||
await rm(path, { force: true }).catch(() => {})
|
||||
}
|
||||
return new Map()
|
||||
}
|
||||
}
|
||||
|
||||
export async function writeTrashSweepFailures(
|
||||
trashRoot: string,
|
||||
failures: TrashSweepFailures
|
||||
): Promise<void> {
|
||||
const path = trashSweepBackoffPath(trashRoot)
|
||||
try {
|
||||
if (failures.size === 0) {
|
||||
await rm(path, { force: true })
|
||||
return
|
||||
}
|
||||
await writeFile(path, JSON.stringify(Object.fromEntries(failures)), 'utf8')
|
||||
} catch (error) {
|
||||
// Best effort: losing the record costs one extra walk on the next launch, nothing more.
|
||||
console.warn(`[worktrees] Failed to record trash sweep backoff at ${path}`, error)
|
||||
}
|
||||
}
|
||||
|
||||
export function isTrashSweepEntryDue(record: TrashSweepFailure | undefined, now: number): boolean {
|
||||
if (!record) {
|
||||
return true
|
||||
}
|
||||
// The second clause: a clock that has since moved backwards must not park an entry indefinitely.
|
||||
return now >= record.nextAttemptAt || record.nextAttemptAt - now > TRASH_SWEEP_MAX_RETRY_DELAY_MS
|
||||
}
|
||||
|
||||
export function nextTrashSweepFailure(
|
||||
previous: TrashSweepFailure | undefined,
|
||||
now: number
|
||||
): TrashSweepFailure {
|
||||
const failures = (previous?.failures ?? 0) + 1
|
||||
const step = Math.min(failures, TRASH_SWEEP_RETRY_DELAYS_MS.length) - 1
|
||||
return { failures, nextAttemptAt: now + TRASH_SWEEP_RETRY_DELAYS_MS[step] }
|
||||
}
|
||||
@@ -141,14 +141,17 @@ describe('sweepStaleWorktreeTrash', () => {
|
||||
})
|
||||
|
||||
it('ignores workspace roots that do not exist', async () => {
|
||||
expect(await sweepStaleWorktreeTrash([join(scratchDir, 'missing')])).toEqual({ removed: 0 })
|
||||
expect(await sweepStaleWorktreeTrash([join(scratchDir, 'missing')])).toEqual({
|
||||
removed: 0,
|
||||
deferred: 0
|
||||
})
|
||||
})
|
||||
|
||||
it('never descends past the trash roots beside worktrees', async () => {
|
||||
const deepTrashRoot = join(scratchDir, 'repo', 'feature', WORKTREE_TRASH_DIR_NAME)
|
||||
await mkdir(join(deepTrashRoot, 'wt-1700000000002-abcdef03'), { recursive: true })
|
||||
|
||||
expect(await sweepStaleWorktreeTrash([scratchDir])).toEqual({ removed: 0 })
|
||||
expect(await sweepStaleWorktreeTrash([scratchDir])).toEqual({ removed: 0, deferred: 0 })
|
||||
expect(existsSync(join(deepTrashRoot, 'wt-1700000000002-abcdef03'))).toBe(true)
|
||||
})
|
||||
|
||||
@@ -159,7 +162,7 @@ describe('sweepStaleWorktreeTrash', () => {
|
||||
await mkdir(externalEntry, { recursive: true })
|
||||
await symlink(join(scratchDir, 'external'), join(scratchDir, WORKTREE_TRASH_DIR_NAME))
|
||||
|
||||
expect(await sweepStaleWorktreeTrash([scratchDir])).toEqual({ removed: 0 })
|
||||
expect(await sweepStaleWorktreeTrash([scratchDir])).toEqual({ removed: 0, deferred: 0 })
|
||||
expect(existsSync(externalEntry)).toBe(true)
|
||||
}
|
||||
)
|
||||
|
||||
@@ -6,6 +6,13 @@ import { randomBytes } from 'node:crypto'
|
||||
import { lstat, mkdir, readdir, rename, rmdir } from 'node:fs/promises'
|
||||
import { dirname, join } from 'node:path'
|
||||
import { removeHostTree } from './host-tree-removal'
|
||||
import {
|
||||
isTrashSweepEntryDue,
|
||||
nextTrashSweepFailure,
|
||||
readTrashSweepFailures,
|
||||
writeTrashSweepFailures,
|
||||
type TrashSweepFailures
|
||||
} from './worktree-trash-sweep-backoff'
|
||||
import { isFolderRepo } from '../shared/repo-kind'
|
||||
import { computeWorkspaceRoot, getWorktreePathSettings } from './ipc/worktree-logic'
|
||||
import type { GlobalSettings } from '../shared/global-settings-types'
|
||||
@@ -94,12 +101,14 @@ export function whenWorktreeTrashDeletionsSettled(): Promise<void> {
|
||||
|
||||
/**
|
||||
* Delete trash entries left behind by a previous run (a crash or a kill during background deletion).
|
||||
* Only entries matching the generated name pattern inside a trash root are removed.
|
||||
* Only entries matching the generated name pattern inside a trash root are removed. An entry whose
|
||||
* removal keeps failing is deferred onto a backoff rather than re-walked on every single launch.
|
||||
*/
|
||||
export async function sweepStaleWorktreeTrash(
|
||||
workspaceRoots: readonly string[]
|
||||
): Promise<{ removed: number }> {
|
||||
): Promise<{ removed: number; deferred: number }> {
|
||||
let removed = 0
|
||||
let deferred = 0
|
||||
for (const trashRoot of await collectExistingTrashRoots(workspaceRoots)) {
|
||||
let entries: string[]
|
||||
try {
|
||||
@@ -111,25 +120,63 @@ export async function sweepStaleWorktreeTrash(
|
||||
} catch {
|
||||
continue
|
||||
}
|
||||
const failures = await readTrashSweepFailures(trashRoot)
|
||||
// Records for entries that are gone (removed here, or restored) must not accumulate forever.
|
||||
let changed = pruneFailuresForMissingEntries(failures, entries)
|
||||
for (const entry of entries) {
|
||||
if (!isWorktreeTrashEntryName(entry)) {
|
||||
continue
|
||||
}
|
||||
const failure = failures.get(entry)
|
||||
if (!isTrashSweepEntryDue(failure, Date.now())) {
|
||||
deferred += 1
|
||||
continue
|
||||
}
|
||||
try {
|
||||
await removeHostTree(join(trashRoot, entry))
|
||||
removed += 1
|
||||
changed = failures.delete(entry) || changed
|
||||
} catch (error) {
|
||||
failures.set(entry, nextTrashSweepFailure(failure, Date.now()))
|
||||
// Flush now: a sweep over many failing entries is long, and a quit mid-way must not throw
|
||||
// away the backoff that was just earned.
|
||||
await writeTrashSweepFailures(trashRoot, failures)
|
||||
changed = false
|
||||
console.warn(
|
||||
`[worktrees] Failed to sweep leftover worktree at ${trashRoot}/${entry}`,
|
||||
error
|
||||
)
|
||||
}
|
||||
}
|
||||
if (changed) {
|
||||
await writeTrashSweepFailures(trashRoot, failures)
|
||||
}
|
||||
}
|
||||
if (removed > 0) {
|
||||
console.log(`[worktrees] Swept ${removed} leftover worktree director(ies) from a previous run`)
|
||||
}
|
||||
return { removed }
|
||||
if (deferred > 0) {
|
||||
console.log(`[worktrees] Deferred ${deferred} leftover worktree director(ies) still in use`)
|
||||
}
|
||||
return { removed, deferred }
|
||||
}
|
||||
|
||||
function pruneFailuresForMissingEntries(
|
||||
failures: TrashSweepFailures,
|
||||
entries: readonly string[]
|
||||
): boolean {
|
||||
if (failures.size === 0) {
|
||||
return false
|
||||
}
|
||||
const present = new Set(entries)
|
||||
let pruned = false
|
||||
for (const entry of failures.keys()) {
|
||||
if (!present.has(entry)) {
|
||||
failures.delete(entry)
|
||||
pruned = true
|
||||
}
|
||||
}
|
||||
return pruned
|
||||
}
|
||||
|
||||
/** Trash roots live beside worktrees, so they sit at the workspace root (flat) or one level in (nested). */
|
||||
|
||||
@@ -47,8 +47,10 @@ describe('transient lock removal options', () => {
|
||||
rmSyncMock.mockReset()
|
||||
})
|
||||
|
||||
it('retries on Windows, where a late handle release is the whole problem', () => {
|
||||
withPlatform('win32')
|
||||
// Why every platform: Windows is the acute case, but Spotlight/`mds`, a scanner or a live process
|
||||
// writing under the tree race a POSIX removal the same way, and a one-shot rm never converged.
|
||||
it.each(['win32', 'darwin', 'linux'] as const)('retries on %s', (platform) => {
|
||||
withPlatform(platform)
|
||||
|
||||
expect(transientLockRemovalOptions()).toEqual({
|
||||
recursive: true,
|
||||
@@ -62,12 +64,28 @@ describe('transient lock removal options', () => {
|
||||
expect(WINDOWS_RM_MAX_RETRIES).toBe(8)
|
||||
})
|
||||
|
||||
it('asks for no retries where removal is not raced by the OS', () => {
|
||||
for (const platform of ['darwin', 'linux'] as const) {
|
||||
withPlatform(platform)
|
||||
expect(transientLockRemovalOptions()).toEqual({ recursive: true, force: true })
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
|
||||
}
|
||||
it('retries a transient ENOTEMPTY off Windows too', () => {
|
||||
withPlatform('darwin')
|
||||
rmSyncMock.mockImplementationOnce(() => {
|
||||
throw Object.assign(new Error('ENOTEMPTY: directory not empty'), { code: 'ENOTEMPTY' })
|
||||
})
|
||||
rmSyncMock.mockImplementationOnce(() => undefined)
|
||||
|
||||
expect(() => removeTreeSync('/tmp/orca-host-job')).not.toThrow()
|
||||
expect(rmSyncMock).toHaveBeenCalledTimes(2)
|
||||
expect(new Set(rmSyncMock.mock.calls.map((call) => call[0]))).toEqual(
|
||||
new Set(['/tmp/orca-host-job'])
|
||||
)
|
||||
})
|
||||
|
||||
it('does not read a POSIX failure out of its prose, only its code', () => {
|
||||
withPlatform('linux')
|
||||
rmSyncMock.mockImplementation(() => {
|
||||
throw new Error('operation not permitted')
|
||||
})
|
||||
|
||||
expect(() => removeTreeSync('/tmp/orca-host-job')).toThrow('operation not permitted')
|
||||
expect(rmSyncMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('retries a transient EPERM instead of treating force: true as enough', () => {
|
||||
|
||||
@@ -1,8 +1,9 @@
|
||||
// Why: Windows releases handles late. Antivirus, the search indexer, a just-exited child and a
|
||||
// freshly dlopen'd DLL all keep a tree Node has just emptied locked for a few milliseconds, which
|
||||
// surfaces as EBUSY/ENOTEMPTY/EPERM. Node's own `maxRetries` absorbs exactly that, and the repo
|
||||
// already settled on 8 attempts — but only product code was using it, so test teardown kept
|
||||
// failing tests whose assertions had already passed.
|
||||
// Why: a recursive removal races whatever else is touching the tree. Windows is the acute case —
|
||||
// antivirus, the search indexer, a just-exited child and a freshly dlopen'd DLL all keep a tree Node
|
||||
// has just emptied locked for a few milliseconds — but it is not the only one: on macOS and Linux
|
||||
// Spotlight/`mds`, a scanner, or a live process writing under the tree surface the same
|
||||
// EBUSY/ENOTEMPTY/EPERM while Node walks it. Node's own `maxRetries` absorbs exactly that, and the
|
||||
// repo settled on 8 attempts. The constant names keep the prefix the repo already ratchets on.
|
||||
|
||||
import type { RmOptions } from 'node:fs'
|
||||
import { rmSync } from 'node:fs'
|
||||
@@ -11,23 +12,32 @@ import { rm } from 'node:fs/promises'
|
||||
export const WINDOWS_RM_MAX_RETRIES = 8
|
||||
export const WINDOWS_RM_RETRY_DELAY_MS = 150
|
||||
|
||||
/** `rm`/`rmSync` options for a recursive removal that must survive a late handle release. */
|
||||
/** `rm`/`rmSync` options for a recursive removal that must survive a concurrent writer. */
|
||||
export function transientLockRemovalOptions(): RmOptions {
|
||||
const base = { recursive: true, force: true }
|
||||
if (process.platform !== 'win32') {
|
||||
return base
|
||||
// Why every platform: `maxRetries` only ever re-issues the syscall that failed, against the path
|
||||
// the caller already chose. It cannot reach a path the one-shot removal would not have touched.
|
||||
return {
|
||||
recursive: true,
|
||||
force: true,
|
||||
maxRetries: WINDOWS_RM_MAX_RETRIES,
|
||||
retryDelay: WINDOWS_RM_RETRY_DELAY_MS
|
||||
}
|
||||
return { ...base, maxRetries: WINDOWS_RM_MAX_RETRIES, retryDelay: WINDOWS_RM_RETRY_DELAY_MS }
|
||||
}
|
||||
|
||||
function isTransientWindowsLockError(error: unknown): boolean {
|
||||
if (process.platform !== 'win32' || typeof error !== 'object' || error === null) {
|
||||
/** Whether a removal failure is a concurrent writer worth re-attempting rather than a real fault. */
|
||||
export function isTransientRemovalError(error: unknown): boolean {
|
||||
if (typeof error !== 'object' || error === null) {
|
||||
return false
|
||||
}
|
||||
const code = 'code' in error && typeof error.code === 'string' ? error.code : undefined
|
||||
if (code && ['EBUSY', 'ENOTEMPTY', 'EPERM'].includes(code)) {
|
||||
return true
|
||||
}
|
||||
// Why Windows-only: there the failure can arrive with no `code` at all. POSIX always sets one, so
|
||||
// matching prose there would only retry unrelated errors that happen to read like a lock.
|
||||
if (process.platform !== 'win32') {
|
||||
return false
|
||||
}
|
||||
const message = 'message' in error && typeof error.message === 'string' ? error.message : ''
|
||||
return /directory not empty|resource busy|operation not permitted/i.test(message)
|
||||
}
|
||||
@@ -36,10 +46,9 @@ function sleepSync(ms: number): void {
|
||||
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms)
|
||||
}
|
||||
|
||||
/** Recursively remove a directory, retrying the transient Windows locks. */
|
||||
/** Recursively remove a directory, retrying a transient concurrent-writer failure. */
|
||||
export function removeTreeSync(targetPath: string): void {
|
||||
const options = transientLockRemovalOptions()
|
||||
const extraAttempts = process.platform === 'win32' ? WINDOWS_RM_MAX_RETRIES : 0
|
||||
let attempt = 0
|
||||
for (;;) {
|
||||
try {
|
||||
@@ -49,7 +58,7 @@ export function removeTreeSync(targetPath: string): void {
|
||||
// Why the outer loop: Node's `maxRetries` only runs inside a real `rmSync`. A mock, or a
|
||||
// handle that outlives those inner attempts, still surfaces EPERM. `force: true` only
|
||||
// suppresses ENOENT.
|
||||
if (attempt >= extraAttempts || !isTransientWindowsLockError(error)) {
|
||||
if (attempt >= WINDOWS_RM_MAX_RETRIES || !isTransientRemovalError(error)) {
|
||||
throw error
|
||||
}
|
||||
sleepSync(WINDOWS_RM_RETRY_DELAY_MS)
|
||||
@@ -58,7 +67,7 @@ export function removeTreeSync(targetPath: string): void {
|
||||
}
|
||||
}
|
||||
|
||||
/** Recursively remove a directory, retrying the transient Windows locks. */
|
||||
/** Recursively remove a directory, retrying a transient concurrent-writer failure. */
|
||||
export async function removeTree(targetPath: string): Promise<void> {
|
||||
await rm(targetPath, transientLockRemovalOptions())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user