From cbe8635f460c16e9c4ce3f60a2e01d38d9e6173f Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:21:26 -0700 Subject: [PATCH] fix(worktrees): prevent deletion from blocking Orca (#11233) * fix(worktrees): prevent deletion from blocking Orca * test(worktrees): loosen async history-delete event-loop bound for CI The main-thread safety check failed on a loaded runner when a single timer gap hit ~48ms under the prior 30ms threshold. Keep the bound well below a recursive sync-rm stall without treating CI jitter as a block. * test(worktrees): measure history-delete critical path, not timer gaps setInterval gaps during async rm of thousands of files still flake under CI scheduling. deleteWorktreeHistoryDir is sync and must only rename, so assert that critical-path wall time stays well below a recursive walk. * fix(worktrees): prevent deletion from blocking Orca Add timeout-based draining of watcher closes so SSH round-trip delays don't indefinitely block the worktree removal path. Also: order durable temp-file sweeps ahead of writes to reclaim orphans before accumulation, skip own-process temps to avoid deleting live writes, swallow persistence errors so disk failures don't cascade to query callers, and measure history-deletion progress by loop turns rather than timer gaps to detect blocking on CI runners. * fix(worktrees): prevent deletion from blocking Orca Worktree deletion can now proceed even if filesystem watchers or history cleanup operations hang, preventing Orca from freezing. Changes: - Fence install slots with tokens instead of counters so removals can abandon wedged installs without corrupting later removals - Timeout-bound watcher unsubscribe operations with a shared drain budget - Move JSON serialization of large usage caches from queue-time to write-time to avoid blocking main thread - Async tombstone + schedule history tree deletion instead of blocking recursive rmSync during GC, preventing main-thread stalls ~10s after startup * Extract usage cache writer into reusable durable snapshot class Consolidates serialized durable-write and generation-veto logic from three usage stores into UsageCacheSnapshotWriter. Eliminates duplication, centralizes multi-MB JSON serialization on the main thread via write-queue serialization, and vetoes superseded snapshots to avoid wasted rewrites. * fix(worktrees): prevent deletion from blocking Orca Worktree deletion used to recursively delete large session trees (hundreds of MB) on the critical path, stalling the event loop. Instead, rename trees into a `.pending-delete` tombstone queue and reclaim them asynchronously off the removal's critical path. Extracted host tree removal into a reusable helper (`removeHostTree`) that centralizes Windows retry logic. Added usage-cache flush on quit to prevent data loss when scans complete right before shutdown. Improved watcher removal deadline management with reserved tail slices for the final unsubscribe, and added retry logic for tombstone removals that fail once under transient Windows locking. * fix(history): retry failed session tree removals Tombstoned session trees whose removal fails transiently (e.g., EBUSY under Windows AV) are now re-queued in-process with bounded exponential backoff instead of sitting until the next HistoryManager construction. Prevents a single stuck tree from blocking the entire Orca process. --- package.json | 1 + src/main/claude-usage/store.test.ts | 169 +++- src/main/claude-usage/store.ts | 41 +- src/main/codex-usage/store.test.ts | 96 +- src/main/codex-usage/store.ts | 36 +- src/main/daemon/history-manager.test.ts | 45 + src/main/daemon/history-manager.ts | 22 +- src/main/daemon/history-reader.ts | 6 +- .../terminal-history-recovery-quarantine.ts | 8 - ...al-history-session-tombstone-retry.test.ts | 77 ++ .../terminal-history-session-tombstone.ts | 141 +++ src/main/durable-file-write.ts | 74 +- src/main/git/worktree.ts | 14 + src/main/host-tree-removal.ts | 67 ++ src/main/index.ts | 14 +- ...ilesystem-watcher-removal-deadline.test.ts | 273 +++++ src/main/ipc/filesystem-watcher.ts | 78 +- src/main/ipc/watcher-removal-drain.ts | 67 ++ src/main/ipc/watcher-removal-gate.test.ts | 41 + src/main/ipc/watcher-removal-gate.ts | 61 +- src/main/ipc/worktrees-windows.test.ts | 6 +- src/main/ipc/worktrees.test.ts | 78 +- src/main/ipc/worktrees.ts | 945 +++++++++--------- src/main/local-worktree-filesystem.ts | 67 +- src/main/observability/instrumentation.ts | 21 + src/main/opencode-usage/store.test.ts | 183 +++- src/main/opencode-usage/store.ts | 36 +- src/main/runtime/orca-runtime.test.ts | 62 +- src/main/runtime/orca-runtime.ts | 920 +++++++++-------- .../terminal-history-async-delete.test.ts | 87 ++ src/main/terminal-history-deletion.ts | 171 ++++ src/main/terminal-history-gc.ts | 154 +++ src/main/terminal-history-paths.ts | 35 + .../terminal-history-tombstone-retry.test.ts | 82 ++ src/main/terminal-history.test.ts | 130 ++- src/main/terminal-history.ts | 211 +--- src/main/usage-cache-snapshot-writer.ts | 61 ++ .../window/attach-main-window-services.ts | 2 +- .../worktree-deletion-dev-bench.mjs | 641 ++++++++++++ 39 files changed, 3922 insertions(+), 1301 deletions(-) create mode 100644 src/main/daemon/terminal-history-session-tombstone-retry.test.ts create mode 100644 src/main/daemon/terminal-history-session-tombstone.ts create mode 100644 src/main/host-tree-removal.ts create mode 100644 src/main/ipc/filesystem-watcher-removal-deadline.test.ts create mode 100644 src/main/ipc/watcher-removal-drain.ts create mode 100644 src/main/terminal-history-async-delete.test.ts create mode 100644 src/main/terminal-history-deletion.ts create mode 100644 src/main/terminal-history-gc.ts create mode 100644 src/main/terminal-history-paths.ts create mode 100644 src/main/terminal-history-tombstone-retry.test.ts create mode 100644 src/main/usage-cache-snapshot-writer.ts create mode 100644 tools/benchmarks/worktree-deletion-dev-bench.mjs diff --git a/package.json b/package.json index 792cf4aff..fe23f1091 100644 --- a/package.json +++ b/package.json @@ -106,6 +106,7 @@ "bench:startup": "pnpm run ensure:electron-runtime && node tools/benchmarks/startup-time-bench.mjs", "bench:daemon-coldstart": "pnpm run ensure:electron-runtime && node tools/benchmarks/daemon-coldstart-bench.mjs", "bench:main-thread-jank": "pnpm run ensure:electron-runtime && node tools/benchmarks/main-thread-jank-bench.mjs", + "bench:worktree-deletion": "node tools/benchmarks/worktree-deletion-dev-bench.mjs", "bench:zustand-selector-fanout": "node config/scripts/zustand-selector-fanout-benchmark.mjs", "bench:multi-workspace-typing": "pnpm run ensure:electron-runtime && node config/scripts/run-multi-workspace-typing-bench.mjs", "bench:cold-park-reveal": "pnpm run ensure:electron-runtime && node tools/benchmarks/terminal-cold-park-reveal-bench.mjs", diff --git a/src/main/claude-usage/store.test.ts b/src/main/claude-usage/store.test.ts index 85d85d8ba..751ee0c0d 100644 --- a/src/main/claude-usage/store.test.ts +++ b/src/main/claude-usage/store.test.ts @@ -1,17 +1,63 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import type * as FsPromises from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { ClaudeUsagePersistedState } from './types' +import type * as Scanner from './scanner' -vi.mock('electron', () => ({ - app: { - getPath: vi.fn(() => '/tmp/orca-test-userdata') +const { getPathMock, writeOpens, writeGate } = vi.hoisted(() => ({ + getPathMock: vi.fn(() => '/tmp/orca-test-userdata'), + // Why only mode 'w': the durable write also opens the directory read-only to fsync it, so counting + // every open would hide a regression back to multiple full-cache rewrites per scan. + writeOpens: { value: 0, inFlight: 0, maxConcurrent: 0 }, + writeGate: { + blocked: false, + waiters: [] as (() => void)[] } })) -import { ClaudeUsageStore } from './store' +vi.mock('electron', () => ({ + app: { + getPath: getPathMock + } +})) + +vi.mock('node:fs/promises', async () => { + const actual = await vi.importActual('node:fs/promises') + return { + ...actual, + open: (async (...args: Parameters) => { + if (args[1] !== 'w') { + return actual.open(...args) + } + writeOpens.value += 1 + writeOpens.inFlight += 1 + writeOpens.maxConcurrent = Math.max(writeOpens.maxConcurrent, writeOpens.inFlight) + try { + if (writeGate.blocked) { + await new Promise((resolve) => writeGate.waiters.push(resolve)) + } + return await actual.open(...args) + } finally { + writeOpens.inFlight -= 1 + } + }) as typeof actual.open + } +}) + +vi.mock('./scanner', async (importOriginal) => ({ + ...(await importOriginal()), + scanClaudeUsageFiles: vi.fn() +})) + +import { ClaudeUsageStore, initClaudeUsagePath } from './store' +import { scanClaudeUsageFiles } from './scanner' function createStoreWithState(state: Partial): ClaudeUsageStore { const store = new ClaudeUsageStore({ getRepos: () => [], + getAllWorktreeMeta: () => ({}), getWorktreeMeta: () => undefined } as never) @@ -34,11 +80,32 @@ function createStoreWithState(state: Partial): Claude } describe('ClaudeUsageStore', () => { + let tempUserData: string + beforeEach(() => { + tempUserData = mkdtempSync(join(tmpdir(), 'orca-claude-usage-store-')) + getPathMock.mockReturnValue(tempUserData) + initClaudeUsagePath() + writeOpens.value = 0 + writeOpens.inFlight = 0 + writeOpens.maxConcurrent = 0 + writeGate.blocked = false + writeGate.waiters = [] + vi.mocked(scanClaudeUsageFiles).mockReset() + vi.mocked(scanClaudeUsageFiles).mockResolvedValue({ + processedFiles: [], + sessions: [], + dailyAggregates: [] + }) vi.useFakeTimers() vi.setSystemTime(new Date('2026-04-09T12:00:00.000-04:00')) }) + afterEach(() => { + vi.useRealTimers() + rmSync(tempUserData, { recursive: true, force: true }) + }) + it('reports no data for Orca scope when only non-Orca usage exists', async () => { const store = createStoreWithState({ sessions: [ @@ -549,4 +616,96 @@ describe('ClaudeUsageStore', () => { expect(refreshMock).toHaveBeenCalledWith(false) }) + + it('persists setEnabled via async durable write without leaving tmp files', async () => { + const store = createStoreWithState({ + schemaVersion: 5, + scanState: { + enabled: false, + lastScanStartedAt: null, + lastScanCompletedAt: null, + lastScanError: null + } + }) + + await store.setEnabled(true) + + expect(writeOpens.value).toBe(1) + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + const persisted = JSON.parse( + readFileSync(join(tempUserData, 'orca-claude-usage.json'), 'utf-8') + ) + expect(persisted.scanState.enabled).toBe(true) + // Pretty-print preserved for human inspection of the analytics cache. + expect(readFileSync(join(tempUserData, 'orca-claude-usage.json'), 'utf-8')).toContain('\n') + }) + + it('vetoes a stale concurrent async write so the newer snapshot wins', async () => { + const store = createStoreWithState({ + schemaVersion: 5, + scanState: { + enabled: true, + lastScanStartedAt: null, + lastScanCompletedAt: null, + lastScanError: null + } + }) + const internals = store as unknown as { + writeToDisk: () => Promise + state: ClaudeUsagePersistedState + } + + writeGate.blocked = true + const first = internals.writeToDisk() + await vi.waitFor(() => expect(writeGate.waiters.length).toBe(1)) + + internals.state.scanState.enabled = false + writeGate.blocked = false + const second = internals.writeToDisk() + writeGate.waiters.splice(0).forEach((resolve) => resolve()) + await Promise.all([first, second]) + + expect( + JSON.parse(readFileSync(join(tempUserData, 'orca-claude-usage.json'), 'utf-8')).scanState + .enabled + ).toBe(false) + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + // Serialized, so the superseded write can be skipped safely rather than racing the newer one. + expect(writeOpens.maxConcurrent).toBe(1) + }) + + it('persists a successful refresh with one full-cache write', async () => { + const store = createStoreWithState({ + schemaVersion: 5, + scanState: { + enabled: true, + lastScanStartedAt: null, + lastScanCompletedAt: null, + lastScanError: null + } + }) + + await store.refresh(true) + + // Why exactly one: scan start used to rewrite the whole 20 MB cache before any result changed. + expect(writeOpens.value).toBe(1) + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + expect( + JSON.parse(readFileSync(join(tempUserData, 'orca-claude-usage.json'), 'utf-8')).scanState + ).toMatchObject({ + lastScanStartedAt: new Date('2026-04-09T12:00:00.000-04:00').getTime(), + lastScanCompletedAt: new Date('2026-04-09T12:00:00.000-04:00').getTime(), + lastScanError: null + }) + }) + + it('sweeps a usage temp file orphaned by a crash between write and rename', async () => { + const orphan = join(tempUserData, 'orca-claude-usage.json.999.1.abc.tmp') + writeFileSync(orphan, '{}') + + createStoreWithState({}) + await vi.waitFor(() => + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + ) + }) }) diff --git a/src/main/claude-usage/store.ts b/src/main/claude-usage/store.ts index 549758fc6..e21329282 100644 --- a/src/main/claude-usage/store.ts +++ b/src/main/claude-usage/store.ts @@ -1,7 +1,8 @@ /* eslint-disable max-lines -- Why: this store is the single main-process owner for Claude usage persistence, scan gating, and query semantics. Keeping those policy decisions together avoids split-brain range/scope logic across multiple files. */ import { app } from 'electron' -import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs' -import { dirname, join } from 'node:path' +import { existsSync, readFileSync } from 'node:fs' +import { join } from 'node:path' +import { UsageCacheSnapshotWriter } from '../usage-cache-snapshot-writer' import type { ClaudeUsageBreakdownKind, ClaudeUsageBreakdownRow, @@ -331,6 +332,9 @@ export class ClaudeUsageStore { private state: ClaudeUsagePersistedState private readonly store: Store private scanPromise: Promise | null = null + // Why: the 20 MB usage JSON must not block the Electron main thread; the writer serializes writes + // and vetoes superseded renames. + private readonly writer = new UsageCacheSnapshotWriter('[claude-usage]', getClaudeUsageFile) constructor(store: Store) { this.store = store @@ -375,23 +379,19 @@ export class ClaudeUsageStore { } } - private writeToDisk(): void { - const usageFile = getClaudeUsageFile() - const dir = dirname(usageFile) - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }) - } - // Why: scans can refresh while the app is in active use. Use the same - // atomic temp-file pattern as the main store so a crash or concurrent write - // cannot leave a truncated analytics file as the common failure mode. - const tmpFile = `${usageFile}.${process.pid}.${Date.now()}.${Math.random().toString(16).slice(2)}.tmp` - writeFileSync(tmpFile, JSON.stringify(this.state, null, 2), 'utf-8') - renameSync(tmpFile, usageFile) + private writeToDisk(): Promise { + // Pretty-print preserved: humans inspect this analytics cache on disk. + return this.writer.write(() => JSON.stringify(this.state, null, 2)) + } + + /** Await queued cache writes so quit does not drop the final snapshot. */ + flush(): Promise { + return this.writer.flush() } async setEnabled(enabled: boolean): Promise { this.state.scanState.enabled = enabled - this.writeToDisk() + await this.writeToDisk() return this.getScanState() } @@ -441,8 +441,11 @@ export class ClaudeUsageStore { this.state.scanState.lastScanStartedAt = Date.now() this.state.scanState.lastScanError = null - this.writeToDisk() + // Why no write here: persisting scan-start would rewrite the whole multi-MB cache before a single + // result changed. The completion/failure write below persists the same fields. + + // Why: assign scanPromise before any await so concurrent refresh shares one scan. this.scanPromise = (async () => { try { const repos = this.store.getRepos() @@ -458,10 +461,12 @@ export class ClaudeUsageStore { this.state.worktreeFingerprint = worktreeFingerprint this.state.scanState.lastScanCompletedAt = Date.now() this.state.scanState.lastScanError = null - this.writeToDisk() + // Why swallow: persistence is a cache concern. A disk failure must not turn a good scan into + // a scan error and reject refresh() for every query caller; writeToDisk already logs it. + await this.writeToDisk().catch(() => {}) } catch (error) { this.state.scanState.lastScanError = error instanceof Error ? error.message : String(error) - this.writeToDisk() + await this.writeToDisk().catch(() => {}) } finally { this.scanPromise = null } diff --git a/src/main/codex-usage/store.test.ts b/src/main/codex-usage/store.test.ts index a20472abd..11be8c514 100644 --- a/src/main/codex-usage/store.test.ts +++ b/src/main/codex-usage/store.test.ts @@ -1,9 +1,9 @@ /* eslint-disable max-lines */ -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs' +import type * as FsPromises from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import type * as Fs from 'node:fs' import type { CodexUsageDailyAggregate, CodexUsagePersistedFile, @@ -11,8 +11,15 @@ import type { CodexUsageSession } from './types' -const { getPathMock } = vi.hoisted(() => ({ - getPathMock: vi.fn(() => '/tmp/orca-test-userdata') +const { getPathMock, writeOpens, writeGate } = vi.hoisted(() => ({ + getPathMock: vi.fn(() => '/tmp/orca-test-userdata'), + // Why only mode 'w': the durable write also opens the directory read-only to fsync it, so counting + // every open would hide a regression back to multiple full-cache rewrites per scan. + writeOpens: { value: 0, inFlight: 0, maxConcurrent: 0 }, + writeGate: { + blocked: false, + waiters: [] as (() => void)[] + } })) vi.mock('electron', () => ({ @@ -21,11 +28,26 @@ vi.mock('electron', () => ({ } })) -vi.mock('fs', async () => { - const actual = await vi.importActual('fs') +vi.mock('node:fs/promises', async () => { + const actual = await vi.importActual('node:fs/promises') return { ...actual, - writeFileSync: vi.fn(actual.writeFileSync) + open: (async (...args: Parameters) => { + if (args[1] !== 'w') { + return actual.open(...args) + } + writeOpens.value += 1 + writeOpens.inFlight += 1 + writeOpens.maxConcurrent = Math.max(writeOpens.maxConcurrent, writeOpens.inFlight) + try { + if (writeGate.blocked) { + await new Promise((resolve) => writeGate.waiters.push(resolve)) + } + return await actual.open(...args) + } finally { + writeOpens.inFlight -= 1 + } + }) as typeof actual.open } }) @@ -97,7 +119,11 @@ describe('CodexUsageStore', () => { tempUserData = mkdtempSync(join(tmpdir(), 'orca-codex-usage-store-')) getPathMock.mockReturnValue(tempUserData) initCodexUsagePath() - vi.mocked(writeFileSync).mockClear() + writeOpens.value = 0 + writeOpens.inFlight = 0 + writeOpens.maxConcurrent = 0 + writeGate.blocked = false + writeGate.waiters = [] vi.mocked(scanCodexUsageFiles).mockReset() vi.mocked(scanCodexUsageFiles).mockResolvedValue(createEmptyScanResult()) vi.useFakeTimers() @@ -109,7 +135,7 @@ describe('CodexUsageStore', () => { rmSync(tempUserData, { recursive: true, force: true }) }) - it('persists a successful refresh with one compact disk write', async () => { + it('persists a successful refresh with one compact async disk write', async () => { const store = createStoreWithState({ schemaVersion: 5, scanState: { @@ -122,7 +148,9 @@ describe('CodexUsageStore', () => { await store.refresh(true) - expect(writeFileSync).toHaveBeenCalledTimes(1) + // Why exactly one: a refresh that rewrites the whole 60 MB cache twice is the regression this guards. + expect(writeOpens.value).toBe(1) + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) const persistedJson = readFileSync(join(tempUserData, 'orca-codex-usage.json'), 'utf-8') expect(persistedJson).toBe(JSON.stringify(JSON.parse(persistedJson))) expect(persistedJson).not.toContain('\n') @@ -155,13 +183,57 @@ describe('CodexUsageStore', () => { lastScanStartedAt: new Date('2026-04-10T12:00:00.000-04:00').getTime(), lastScanError: null }) - expect(writeFileSync).not.toHaveBeenCalled() + expect(writeOpens.value).toBe(0) pendingScan.resolve(createEmptyScanResult()) await refreshPromise expect(store.getScanState().isScanning).toBe(false) - expect(writeFileSync).toHaveBeenCalledTimes(1) + expect(writeOpens.value).toBe(1) + }) + + it('vetoes a stale concurrent async write so the newer snapshot wins without leaking tmp files', async () => { + const store = createStoreWithState({ + schemaVersion: 5, + scanState: { + enabled: true, + lastScanStartedAt: null, + lastScanCompletedAt: null, + lastScanError: null + } + }) + const internals = store as unknown as { + writeToDisk: () => Promise + state: CodexUsagePersistedState + } + + writeGate.blocked = true + const first = internals.writeToDisk() + await vi.waitFor(() => expect(writeGate.waiters.length).toBe(1)) + + internals.state.scanState.enabled = false + writeGate.blocked = false + const second = internals.writeToDisk() + writeGate.waiters.splice(0).forEach((resolve) => resolve()) + await Promise.all([first, second]) + + expect( + JSON.parse(readFileSync(join(tempUserData, 'orca-codex-usage.json'), 'utf-8')).scanState + .enabled + ).toBe(false) + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + // Serialized, so the superseded write can be skipped safely rather than racing the newer one. + expect(writeOpens.maxConcurrent).toBe(1) + }) + + it('sweeps a usage temp file orphaned by a crash between write and rename', async () => { + const orphan = join(tempUserData, 'orca-codex-usage.json.999.1.abc.tmp') + writeFileSync(orphan, '{}') + + createStoreWithState({}) + await vi.waitFor(() => + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + ) }) it('reports no data for Orca scope when only non-Orca Codex usage exists', async () => { diff --git a/src/main/codex-usage/store.ts b/src/main/codex-usage/store.ts index f3c62fb89..540d8f23e 100644 --- a/src/main/codex-usage/store.ts +++ b/src/main/codex-usage/store.ts @@ -1,7 +1,8 @@ /* eslint-disable max-lines -- Why: this store owns Codex analytics persistence, scan policy, and renderer query semantics. Keeping them together prevents the Codex range/scope rules from drifting away from the scanner’s event model. */ import { app } from 'electron' -import { dirname, join } from 'node:path' -import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs' +import { join } from 'node:path' +import { existsSync, readFileSync } from 'node:fs' +import { UsageCacheSnapshotWriter } from '../usage-cache-snapshot-writer' import type { CodexUsageBreakdownKind, CodexUsageBreakdownRow, @@ -366,6 +367,9 @@ export class CodexUsageStore { private state: CodexUsagePersistedState private readonly store: Store private scanPromise: Promise | null = null + // Why: the 60 MB usage JSON must not block the Electron main thread; the writer serializes writes + // and vetoes superseded renames. + private readonly writer = new UsageCacheSnapshotWriter('[codex-usage]', getCodexUsageFile) constructor(store: Store) { this.store = store @@ -393,20 +397,19 @@ export class CodexUsageStore { } } - private writeToDisk(): void { - const usageFile = getCodexUsageFile() - const dir = dirname(usageFile) - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }) - } - const tmpFile = `${usageFile}.${process.pid}.${Date.now()}.${Math.random().toString(16).slice(2)}.tmp` - writeFileSync(tmpFile, JSON.stringify(this.state), 'utf-8') - renameSync(tmpFile, usageFile) + private writeToDisk(): Promise { + // Compact: this cache reaches 60 MB, and pretty-printing it costs main-thread time per scan. + return this.writer.write(() => JSON.stringify(this.state)) + } + + /** Await queued cache writes so quit does not drop the final snapshot. */ + flush(): Promise { + return this.writer.flush() } async setEnabled(enabled: boolean): Promise { this.state.scanState.enabled = enabled - this.writeToDisk() + await this.writeToDisk() return this.getScanState() } @@ -456,7 +459,8 @@ export class CodexUsageStore { this.state.scanState.lastScanStartedAt = Date.now() this.state.scanState.lastScanError = null - // Why: start-only writes rewrite the full usage cache before scan results change. + // Why no write here: persisting scan-start would rewrite the whole multi-MB cache before a single + // result changed. The completion/failure write below persists the same fields. this.scanPromise = (async () => { try { @@ -473,10 +477,12 @@ export class CodexUsageStore { this.state.worktreeFingerprint = worktreeFingerprint this.state.scanState.lastScanCompletedAt = Date.now() this.state.scanState.lastScanError = null - this.writeToDisk() + // Why swallow: persistence is a cache concern. A disk failure must not turn a good scan into + // a scan error and reject refresh() for every query caller; writeToDisk already logs it. + await this.writeToDisk().catch(() => {}) } catch (error) { this.state.scanState.lastScanError = error instanceof Error ? error.message : String(error) - this.writeToDisk() + await this.writeToDisk().catch(() => {}) } finally { this.scanPromise = null } diff --git a/src/main/daemon/history-manager.test.ts b/src/main/daemon/history-manager.test.ts index 57c74896a..ff8e9d355 100644 --- a/src/main/daemon/history-manager.test.ts +++ b/src/main/daemon/history-manager.test.ts @@ -4,6 +4,7 @@ import { join } from 'node:path' import { chmodSync, existsSync, + mkdirSync, mkdtempSync, readFileSync, readdirSync, @@ -11,6 +12,7 @@ import { writeFileSync } from 'node:fs' import { HistoryManager } from './history-manager' +import { flushPendingSessionTreeRemovals } from './terminal-history-session-tombstone' import type { TerminalSnapshot, TerminalModes } from './types' import { getHistorySessionDirName } from './history-paths' import { @@ -58,6 +60,8 @@ describe('HistoryManager', () => { afterEach(async () => { await mgr.dispose() + // Detached tombstone reclaims outlive the test that queued them; settle before the fixture goes. + await flushPendingSessionTreeRemovals() rmSync(dir, { recursive: true, force: true }) }) @@ -486,4 +490,45 @@ describe('HistoryManager', () => { } ) }) + + describe('large session cleanup responsiveness', () => { + it('tombstones a large session tree instead of walking it on the teardown path', async () => { + const sessionId = 'bulky' + await mgr.openSession(sessionId, { cwd: '/tmp', cols: 80, rows: 24 }) + const sessionDir = join(dir, getHistorySessionDirName(sessionId)) + // Enough entries that a recursive walk would dominate removeSession's duration. + for (let i = 0; i < 3_000; i++) { + writeFileSync(join(sessionDir, `chunk-${i}.log`), `payload-${i}`) + } + + // Why structural and not a turn count: worktree teardown awaits removeSession once per terminal, + // so the contract is that the awaited half only renames. The tree surviving under .pending-delete + // right after the await can only happen if the reclaim was detached. + await mgr.removeSession(sessionId) + + expect(existsSync(sessionDir)).toBe(false) + const tombstones = readdirSync(join(dir, '.pending-delete')) + expect(tombstones).toHaveLength(1) + expect(readdirSync(join(dir, '.pending-delete', tombstones[0])).length).toBeGreaterThan(0) + + await flushPendingSessionTreeRemovals() + expect(readdirSync(join(dir, '.pending-delete'))).toHaveLength(0) + }) + + it('reclaims tombstones left by a quit mid-removal on the next construction', async () => { + const sessionId = 'leftover' + await mgr.openSession(sessionId, { cwd: '/tmp', cols: 80, rows: 24 }) + await mgr.removeSession(sessionId) + await flushPendingSessionTreeRemovals() + + const orphan = join(dir, '.pending-delete', 'orphaned-tombstone') + mkdirSync(orphan, { recursive: true }) + writeFileSync(join(orphan, 'output.log'), 'stranded') + + new HistoryManager(dir) + await flushPendingSessionTreeRemovals() + + expect(existsSync(orphan)).toBe(false) + }) + }) }) diff --git a/src/main/daemon/history-manager.ts b/src/main/daemon/history-manager.ts index 7aa786063..0a8e29d79 100644 --- a/src/main/daemon/history-manager.ts +++ b/src/main/daemon/history-manager.ts @@ -1,15 +1,18 @@ import { join } from 'node:path' import { randomUUID } from 'node:crypto' -import { mkdirSync, writeFileSync, existsSync, rmSync, unlinkSync } from 'node:fs' +import { mkdirSync, writeFileSync, existsSync, unlinkSync } from 'node:fs' import { getHistorySessionDirName } from './history-paths' import { fingerprintTerminalHistorySession, hasTerminalHistoryRecoveryProtection, quarantineTerminalHistorySession, - removeTerminalHistoryQuarantines, type ActiveHistoryRecoveryFreeze, type HistoryRecoveryFreeze } from './terminal-history-recovery-quarantine' +import { + removeTerminalHistorySessionTrees, + schedulePendingSessionTreeRemovals +} from './terminal-history-session-tombstone' import { TerminalHistorySessionWriter } from './terminal-history-session-writer' import { readTerminalHistoryMetaFromDir, @@ -43,6 +46,8 @@ export class HistoryManager { ) { this.onWriteError = opts?.onWriteError this.checkpointMaxBytes = opts?.checkpointMaxBytes ?? TERMINAL_HISTORY_CHECKPOINT_MAX_BYTES + // Why: a quit between tombstone and reclaim leaves the tree on disk; nothing else rescans the queue. + schedulePendingSessionTreeRemovals(this.basePath) } async openSession(sessionId: string, opts: OpenSessionOptions): Promise { @@ -270,16 +275,11 @@ export class HistoryManager { async removeSession(sessionId: string): Promise { this.writers.delete(sessionId) this.disabledSessions.delete(sessionId) - const activeFreeze = this.recoveryFreezes.get(sessionId) - if (activeFreeze) { - this.recoveryFreezes.delete(sessionId) - } + this.recoveryFreezes.delete(sessionId) await this.mutations.wait(sessionId) - rmSync(join(this.basePath, getHistorySessionDirName(sessionId)), { - recursive: true, - force: true - }) - removeTerminalHistoryQuarantines(this.basePath, sessionId) + // Why tombstoned: writer handles are closed by here, so the trees only have to become unreachable — + // they reach hundreds of MB and every terminal a worktree delete tears down awaits this. + await removeTerminalHistorySessionTrees(this.basePath, sessionId) } isSessionDisabled(sessionId: string): boolean { diff --git a/src/main/daemon/history-reader.ts b/src/main/daemon/history-reader.ts index f950c550d..58cce506c 100644 --- a/src/main/daemon/history-reader.ts +++ b/src/main/daemon/history-reader.ts @@ -18,6 +18,7 @@ import { hasTerminalHistoryRecoveryProtection, isTerminalHistoryQuarantineEntry } from './terminal-history-recovery-quarantine' +import { isTerminalHistoryPendingDeleteEntry } from './terminal-history-session-tombstone' import { readTerminalHistoryMeta, type SessionMeta, @@ -195,7 +196,10 @@ export class HistoryReader { if (!entry.isDirectory()) { continue } - if (isTerminalHistoryQuarantineEntry(entry.name)) { + if ( + isTerminalHistoryQuarantineEntry(entry.name) || + isTerminalHistoryPendingDeleteEntry(entry.name) + ) { continue } let sessionId: string diff --git a/src/main/daemon/terminal-history-recovery-quarantine.ts b/src/main/daemon/terminal-history-recovery-quarantine.ts index df7605b55..414fe4f58 100644 --- a/src/main/daemon/terminal-history-recovery-quarantine.ts +++ b/src/main/daemon/terminal-history-recovery-quarantine.ts @@ -5,7 +5,6 @@ import { mkdirSync, readdirSync, renameSync, - rmSync, unlinkSync, writeFileSync } from 'node:fs' @@ -90,10 +89,3 @@ export function quarantineTerminalHistorySession( renameSync(sessionDir, quarantineDir) return quarantineDir } - -export function removeTerminalHistoryQuarantines(basePath: string, sessionId: string): void { - rmSync(getTerminalHistoryQuarantineOwnerDir(basePath, sessionId), { - recursive: true, - force: true - }) -} diff --git a/src/main/daemon/terminal-history-session-tombstone-retry.test.ts b/src/main/daemon/terminal-history-session-tombstone-retry.test.ts new file mode 100644 index 000000000..93a5d134a --- /dev/null +++ b/src/main/daemon/terminal-history-session-tombstone-retry.test.ts @@ -0,0 +1,77 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { removeHostTreeMock } = vi.hoisted(() => ({ + removeHostTreeMock: vi.fn<(dir: string) => Promise>() +})) + +vi.mock('../host-tree-removal', () => ({ + removeHostTree: removeHostTreeMock +})) + +import { getHistorySessionDirName } from './history-paths' +import { + cancelPendingSessionTreeRemovalRetries, + removeTerminalHistorySessionTrees, + SESSION_TREE_REMOVAL_RETRY_DELAYS_MS +} from './terminal-history-session-tombstone' + +/** Mirrors the worktree-level tombstone retry: a session tree whose rm fails must be re-queued + * in-process instead of waiting for the next HistoryManager construction. */ +describe('tombstoned session tree removal retries', () => { + let basePath: string + + beforeEach(() => { + basePath = mkdtempSync(join(tmpdir(), 'orca-session-tombstone-retry-')) + removeHostTreeMock.mockReset() + vi.useFakeTimers() + }) + + afterEach(() => { + cancelPendingSessionTreeRemovalRetries() + vi.useRealTimers() + rmSync(basePath, { recursive: true, force: true }) + }) + + function seedSession(sessionId: string): void { + const dir = join(basePath, getHistorySessionDirName(sessionId)) + mkdirSync(dir, { recursive: true }) + writeFileSync(join(dir, 'segment-0.log'), 'x') + } + + it('re-queues a tombstone whose removal failed, then stops after the last attempt', async () => { + seedSession('session-busy') + removeHostTreeMock.mockRejectedValue( + Object.assign(new Error('resource busy'), { code: 'EBUSY' }) + ) + + await removeTerminalHistorySessionTrees(basePath, 'session-busy') + await vi.advanceTimersByTimeAsync(0) + expect(removeHostTreeMock).toHaveBeenCalledTimes(1) + + for (const [index, retryDelayMs] of SESSION_TREE_REMOVAL_RETRY_DELAYS_MS.entries()) { + await vi.advanceTimersByTimeAsync(retryDelayMs) + expect(removeHostTreeMock).toHaveBeenCalledTimes(index + 2) + } + + // Bounded: a permanently wedged tree waits for the next construction's drain. + await vi.advanceTimersByTimeAsync(60 * 60_000) + expect(removeHostTreeMock).toHaveBeenCalledTimes( + SESSION_TREE_REMOVAL_RETRY_DELAYS_MS.length + 1 + ) + expect(removeHostTreeMock).toHaveBeenLastCalledWith(expect.stringContaining('.pending-delete')) + }) + + it('does not re-arm a retry after the removal succeeds', async () => { + seedSession('session-clean') + removeHostTreeMock.mockResolvedValue(undefined) + + await removeTerminalHistorySessionTrees(basePath, 'session-clean') + await vi.advanceTimersByTimeAsync(0) + await vi.advanceTimersByTimeAsync(SESSION_TREE_REMOVAL_RETRY_DELAYS_MS[0]) + + expect(removeHostTreeMock).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/daemon/terminal-history-session-tombstone.ts b/src/main/daemon/terminal-history-session-tombstone.ts new file mode 100644 index 000000000..d72f9e8f7 --- /dev/null +++ b/src/main/daemon/terminal-history-session-tombstone.ts @@ -0,0 +1,141 @@ +// Why: worktree teardown stops every terminal one by one, and each stop used to await a full recursive +// delete of that session's history tree (hundreds of MB). Rename the tree into a tombstone queue instead +// so the stop-and-wait path is metadata-only, and drain the queue off the critical path. + +import { randomUUID } from 'node:crypto' +import { existsSync, mkdirSync, readdirSync, renameSync } from 'node:fs' +import { join } from 'node:path' +import { removeHostTree } from '../host-tree-removal' +import { getHistorySessionDirName } from './history-paths' +import { getTerminalHistoryQuarantineOwnerDir } from './terminal-history-recovery-quarantine' + +const PENDING_DELETE_DIR_NAME = '.pending-delete' + +const pendingSessionTreeRemovals = new Map>() +// Why: a tombstone whose rm fails once (Windows EBUSY under AV) would otherwise sit on disk until the +// next HistoryManager construction. Bounded so a genuinely wedged tree stops burning timers. +export const SESSION_TREE_REMOVAL_RETRY_DELAYS_MS = [30_000, 120_000] +const sessionTreeRemovalAttempts = new Map() +const sessionTreeRemovalRetryTimers = new Map>() + +export function isTerminalHistoryPendingDeleteEntry(name: string): boolean { + return name === PENDING_DELETE_DIR_NAME +} + +function getPendingDeleteRoot(basePath: string): string { + return join(basePath, PENDING_DELETE_DIR_NAME) +} + +function tombstoneSessionTree(basePath: string, dir: string): string | null { + const pendingRoot = getPendingDeleteRoot(basePath) + try { + mkdirSync(pendingRoot, { recursive: true }) + const tombstone = join(pendingRoot, randomUUID()) + renameSync(dir, tombstone) + return tombstone + } catch { + return null + } +} + +function scheduleSessionTreeRemovalRetry(dir: string): void { + const attempt = sessionTreeRemovalAttempts.get(dir) ?? 0 + const retryDelayMs = SESSION_TREE_REMOVAL_RETRY_DELAYS_MS[attempt] + if (retryDelayMs === undefined) { + // Out of in-process attempts: the tombstone stays queued for the next construction's drain. + sessionTreeRemovalAttempts.delete(dir) + return + } + sessionTreeRemovalAttempts.set(dir, attempt + 1) + const timer = setTimeout(() => { + sessionTreeRemovalRetryTimers.delete(dir) + scheduleSessionTreeRemoval(dir) + }, retryDelayMs) + timer.unref?.() + sessionTreeRemovalRetryTimers.set(dir, timer) +} + +function scheduleSessionTreeRemoval(dir: string): void { + if (pendingSessionTreeRemovals.has(dir)) { + return + } + // A rescan (startup drain) hitting the same tombstone supersedes its pending retry. + const pendingRetry = sessionTreeRemovalRetryTimers.get(dir) + if (pendingRetry) { + clearTimeout(pendingRetry) + sessionTreeRemovalRetryTimers.delete(dir) + } + const removal = removeHostTree(dir) + .then(() => { + sessionTreeRemovalAttempts.delete(dir) + }) + .catch((err: unknown) => { + console.warn( + `[history] Failed to delete tombstoned session tree: ${err instanceof Error ? err.message : String(err)}` + ) + scheduleSessionTreeRemovalRetry(dir) + }) + .finally(() => { + if (pendingSessionTreeRemovals.get(dir) === removal) { + pendingSessionTreeRemovals.delete(dir) + } + }) + pendingSessionTreeRemovals.set(dir, removal) +} + +/** Remove everything a session owns on disk — its history tree and any quarantined generations — + * without holding the caller for the recursive walks. Both are unreachable once this resolves; + * only the reclaim is detached. */ +export async function removeTerminalHistorySessionTrees( + basePath: string, + sessionId: string +): Promise { + for (const dir of [ + join(basePath, getHistorySessionDirName(sessionId)), + getTerminalHistoryQuarantineOwnerDir(basePath, sessionId) + ]) { + await removeSessionOwnedTree(basePath, dir) + } +} + +async function removeSessionOwnedTree(basePath: string, dir: string): Promise { + if (!existsSync(dir)) { + return + } + const tombstone = tombstoneSessionTree(basePath, dir) + if (tombstone) { + scheduleSessionTreeRemoval(tombstone) + return + } + // Rename blocked (open handles under Windows AV); remove in place so the tree is still gone on return. + await removeHostTree(dir) +} + +/** Queue tombstones left by a crash or quit mid-removal. Cheap no-op when the queue never formed. */ +export function schedulePendingSessionTreeRemovals(basePath: string): void { + const pendingRoot = getPendingDeleteRoot(basePath) + try { + for (const entry of readdirSync(pendingRoot)) { + scheduleSessionTreeRemoval(join(pendingRoot, entry)) + } + } catch { + // Missing or unreadable queue is non-fatal; the next construction rescans. + } +} + +/** Drop every armed retry timer so a fixture teardown cannot resurrect a removal. Tests only. */ +export function cancelPendingSessionTreeRemovalRetries(): void { + for (const timer of sessionTreeRemovalRetryTimers.values()) { + clearTimeout(timer) + } + sessionTreeRemovalRetryTimers.clear() + sessionTreeRemovalAttempts.clear() +} + +/** Await the in-flight tombstone removals. Tests only — production reclaims off the critical path and + * re-queues whatever a quit interrupted on the next HistoryManager construction. */ +export async function flushPendingSessionTreeRemovals(): Promise { + while (pendingSessionTreeRemovals.size > 0) { + await Promise.all(pendingSessionTreeRemovals.values()) + } +} diff --git a/src/main/durable-file-write.ts b/src/main/durable-file-write.ts index 7f827ba8a..ac57e7d46 100644 --- a/src/main/durable-file-write.ts +++ b/src/main/durable-file-write.ts @@ -4,8 +4,8 @@ // hour's loss; fsync stops it from happening. import { closeSync, fsyncSync, openSync, renameSync, writeFileSync } from 'node:fs' -import { open, rename } from 'node:fs/promises' -import { dirname } from 'node:path' +import { open, readdir, rename, rm } from 'node:fs/promises' +import { basename, dirname, join } from 'node:path' /** * fsync a directory so a rename within it is durable. Best-effort by design: Windows cannot open a @@ -57,16 +57,72 @@ export async function writeFileDurable( finalPath: string, payload: string ): Promise { - const handle = await open(tmpPath, 'w') + await writeFileDurableIfCurrent(tmpPath, finalPath, payload, () => true) +} + +/** + * `writeFileDurable` with a commit veto: `isCurrent` is consulted after the fsync and before the + * rename so a writer that was superseded mid-write doesn't publish a stale snapshot. Returns whether + * the rename happened; the temp file is removed on every path that doesn't commit, so a multi-MB + * payload can't orphan itself. + */ +export async function writeFileDurableIfCurrent( + tmpPath: string, + finalPath: string, + payload: string, + isCurrent: () => boolean +): Promise { + let renamed = false try { - await handle.writeFile(payload, 'utf-8') - // Why: fsync BEFORE rename. A rename that lands first can expose a zero-length file. - await handle.sync() + const handle = await open(tmpPath, 'w') + try { + await handle.writeFile(payload, 'utf-8') + // Why: fsync BEFORE rename. A rename that lands first can expose a zero-length file. + await handle.sync() + } finally { + await handle.close() + } + if (!isCurrent()) { + return false + } + await rename(tmpPath, finalPath) + renamed = true + await syncDirectory(dirname(finalPath)) + return true } finally { - await handle.close() + if (!renamed) { + await rm(tmpPath, { force: true }).catch(() => {}) + } + } +} + +/** Temp path for a durable write. Shared shape so `removeStaleDurableWriteTempFiles` can reclaim orphans. */ +export function durableWriteTempPath(finalPath: string): string { + return `${finalPath}.${process.pid}.${Date.now()}.${Math.random().toString(16).slice(2)}.tmp` +} + +/** + * Sweep temp files orphaned by a death between write and rename — for multi-MB payloads they would + * otherwise accumulate forever. Racing another instance's in-flight save at worst loses that save, + * the trade already accepted for rename-based atomicity. This process's own temps are skipped: a + * `..*.tmp` seen during a sweep is a live write, and deleting it fails its rename. + */ +export async function removeStaleDurableWriteTempFiles(finalPath: string): Promise { + const directory = dirname(finalPath) + const prefix = `${basename(finalPath)}.` + const ownPrefix = `${prefix}${process.pid}.` + try { + const names = await readdir(directory) + await Promise.all( + names + .filter( + (name) => name.startsWith(prefix) && name.endsWith('.tmp') && !name.startsWith(ownPrefix) + ) + .map((name) => rm(join(directory, name), { force: true }).catch(() => {})) + ) + } catch { + // Directory missing or unreadable — nothing to sweep. } - await rename(tmpPath, finalPath) - await syncDirectory(dirname(finalPath)) } /** Synchronous counterpart for quit and crash paths that cannot await. */ diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index 1754b70bf..f2f883f0d 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -7,6 +7,7 @@ import { refreshBranchCleanupTargetRefs } from '../../shared/git-branch-cleanup' import { resolveWorktreeAddBaseRef } from '../../shared/worktree-base-ref' +import { withSpan } from '../observability/tracer' import type { GitWorktreeInfo, LocalBaseRefRefreshResult, @@ -1157,6 +1158,19 @@ async function performRemoveWorktree( return {} } + // Why its own span: branch cleanup can reach the network (`fetch --prune`), so a stall here reads as + // `git worktree remove` being slow unless it is timed separately. + return withSpan('worktree.remove.branch_delete', () => + deleteBranchAfterWorktreeRemoval(repoPath, branchName, branchHead, options) + ) +} + +async function deleteBranchAfterWorktreeRemoval( + repoPath: string, + branchName: string, + branchHead: string, + options: RemoveWorktreeOptions +): Promise { try { // Why: also drop the now-orphaned branch so delete-worktree leaves none; `-d` (not `-D`) preserves // unmerged work, and forceBranchDelete opts into `-D` for failed-creation rollback. diff --git a/src/main/host-tree-removal.ts b/src/main/host-tree-removal.ts new file mode 100644 index 000000000..dcc8a2f2a --- /dev/null +++ b/src/main/host-tree-removal.ts @@ -0,0 +1,67 @@ +// 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. + +import type { RmOptions } from 'node:fs' +import { rm } from 'node:fs/promises' +import { win32 } from 'node:path' +import { setTimeout as delay } from 'node:timers/promises' + +const WINDOWS_REMOVE_RETRY_DELAYS_MS = [250, 500, 1_000, 2_000] +const WINDOWS_RM_MAX_RETRIES = 8 +const WINDOWS_RM_RETRY_DELAY_MS = 150 + +export function toHostRemovalPath(targetPath: string): string { + // Why: Git for Windows can fail long recursive deletes even after Orca has + // proven the worktree target; Node's host deletion should use Win32 long paths. + return process.platform === 'win32' ? win32.toNamespacedPath(targetPath) : targetPath +} + +function getHostRemovalOptions(): RmOptions { + const base = { recursive: true, force: true } + if (process.platform !== 'win32') { + return base + } + return { + ...base, + // Why: large Windows trees commonly surface transient ENOTEMPTY/EPERM while + // Node walks and removes nested directories. + maxRetries: WINDOWS_RM_MAX_RETRIES, + retryDelay: WINDOWS_RM_RETRY_DELAY_MS + } +} + +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. */ +export async function removeHostTree(targetPath: string): Promise { + const removalPath = toHostRemovalPath(targetPath) + const retryDelays = process.platform === 'win32' ? WINDOWS_REMOVE_RETRY_DELAYS_MS : [] + const rmOptions = getHostRemovalOptions() + let attempt = 0 + + while (true) { + try { + await rm(removalPath, rmOptions) + return + } catch (error) { + if (attempt >= retryDelays.length || !isTransientWindowsRemovalError(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]) + attempt += 1 + } + } +} diff --git a/src/main/index.ts b/src/main/index.ts index 748a2acc2..1bc941032 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -80,6 +80,7 @@ import { DesktopRelayService } from './runtime/relay/desktop-relay-service' import type { RelayBrokerStatus } from './runtime/relay/relay-session-broker' import { awaitRuntimeFileWatcherUnsubscribes } from './runtime/orca-runtime-files' import { clearRuntimeMetadataIfOwned } from './runtime/runtime-metadata' +import { scheduleAllPendingHistoryTreeRemovals } from './terminal-history-deletion' import { ensureMainI18n, setMainPluginLanguagePacks, setMainUiLanguage } from './i18n/main-i18n' import { getNextDefaultOnAppearanceSettingValue, @@ -2791,6 +2792,9 @@ void app.whenReady().then(async () => { } // Why: headless serve never opens a renderer, so arm scheduled automation dispatch here. automations.start() + // Why: serve deletes worktrees too, and the history GC that normally drains delete tombstones is + // armed from the main window — without this, a quit mid-removal leaks the tree until a desktop launch. + scheduleAllPendingHistoryTreeRemovals() await printServeReady(serveOptions) return } @@ -2917,6 +2921,13 @@ app.on('will-quit', (e) => { killAllPty() const watcherShutdown = shutdownWatchersOnce() store?.flush() + // Why: usage-cache writes are queued off the main thread, so a quit right after setEnabled or a + // scan completion would drop the final snapshot. Captured before any await; joins the barrier below. + const usageCacheFlush = Promise.all([ + claudeUsage?.flush(), + codexUsage?.flush(), + openCodeUsage?.flush() + ]).then(() => {}) // Why: preventDefault to await disconnectDaemon's async checkpoint writes (else data lost); guard prevents an infinite quit loop on the re-fired will-quit. if (!daemonDisconnectDone) { @@ -2950,7 +2961,8 @@ app.on('will-quit', (e) => { { name: 'runtime-rpc', promise: rpcStopAndClear }, { name: 'watchers', promise: watcherShutdown }, { name: 'emulator', promise: emulatorShutdown }, - { name: 'plugin-hosts', promise: pluginHostShutdown } + { name: 'plugin-hosts', promise: pluginHostShutdown }, + { name: 'usage-cache', promise: usageCacheFlush } ]) .then((pendingTeardowns) => { if (pendingTeardowns.length > 0) { diff --git a/src/main/ipc/filesystem-watcher-removal-deadline.test.ts b/src/main/ipc/filesystem-watcher-removal-deadline.test.ts new file mode 100644 index 000000000..52a48e9a0 --- /dev/null +++ b/src/main/ipc/filesystem-watcher-removal-deadline.test.ts @@ -0,0 +1,273 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as ParcelWatcherProcess from './parcel-watcher-process' + +const { handleMock } = vi.hoisted(() => ({ + handleMock: vi.fn() +})) + +vi.mock('electron', () => ({ + ipcMain: { + handle: handleMock + } +})) + +vi.mock('fs/promises', () => ({ + stat: vi.fn() +})) + +vi.mock('@parcel/watcher', () => ({ + subscribe: vi.fn() +})) + +vi.mock('./filesystem-watcher-wsl', () => ({ + createWslWatcher: vi.fn() +})) + +vi.mock('./parcel-watcher-process', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + subscribeViaWatcherProcess: vi.fn(actual.subscribeViaWatcherProcess) + } +}) + +vi.mock('../providers/ssh-filesystem-dispatch', () => ({ + getSshFilesystemProvider: vi.fn(), + onSshFilesystemProviderRegistered: () => () => {} +})) + +import { + closeAllWatchers, + closeLocalWatcherForWorktreePath, + registerFilesystemWatcherHandlers +} from './filesystem-watcher' +import { + createWatcherRemovalDeadline, + drainBeforeWatcherRemoval, + WATCHER_REMOVAL_DRAIN_BUDGET_MS, + WATCHER_REMOVAL_FINAL_DRAIN_RESERVE_MS +} from './watcher-removal-drain' +import { WatcherProcessFailure } from './parcel-watcher-process-failure' +import { stat } from 'node:fs/promises' +import { subscribe as subscribeParcelWatcher } from '@parcel/watcher' +import { subscribeViaWatcherProcess } from './parcel-watcher-process' + +type HandlerMap = Record Promise | unknown> + +describe('local filesystem watcher removal deadline', () => { + const handlers: HandlerMap = {} + + beforeEach(async () => { + handleMock.mockReset() + vi.mocked(stat).mockReset() + vi.mocked(subscribeParcelWatcher).mockReset() + vi.mocked(subscribeViaWatcherProcess).mockClear() + for (const key of Object.keys(handlers)) { + delete handlers[key] + } + handleMock.mockImplementation((channel, handler) => { + handlers[channel] = handler + }) + registerFilesystemWatcherHandlers() + await closeAllWatchers() + }) + + afterEach(async () => { + await closeAllWatchers() + }) + + it('bounds a wedged watcher install so worktree deletion cannot hang forever', async () => { + vi.useFakeTimers() + try { + vi.mocked(stat).mockResolvedValue({ isDirectory: () => true } as never) + // Why mock the process-backed subscribe: the in-process fallback rejects on abort, so only a + // subscribe that ignores the abort signal exercises the deadline rather than the cancel path. + // Once, so the wedge cannot leak into the next test. + vi.mocked(subscribeViaWatcherProcess).mockImplementationOnce(() => new Promise(() => {})) + const sender = { + isDestroyed: () => false, + send: vi.fn(), + once: vi.fn(), + id: 1 + } + + const watchPromise = handlers['fs:watchWorktree']( + { sender }, + { worktreePath: '/tmp/repo' } + ) as Promise + await vi.waitFor(() => { + expect(subscribeViaWatcherProcess).toHaveBeenCalled() + }) + + let closed = false + const closePromise = closeLocalWatcherForWorktreePath('/tmp/repo').then(() => { + closed = true + }) + + // The install drain leaves the final unsubscribe its reserved tail slice. + await vi.advanceTimersByTimeAsync( + WATCHER_REMOVAL_DRAIN_BUDGET_MS - WATCHER_REMOVAL_FINAL_DRAIN_RESERVE_MS - 1 + ) + expect(closed).toBe(false) + + await vi.advanceTimersByTimeAsync(1) + await closePromise + expect(closed).toBe(true) + + void watchPromise + } finally { + vi.useRealTimers() + } + }) + + it('bounds a wedged live unsubscribe so worktree deletion cannot hang forever', async () => { + vi.mocked(stat).mockResolvedValue({ isDirectory: () => true } as never) + // The in-process Parcel fallback has no unsubscribe timeout of its own, so only the shared + // removal deadline can stop this from hanging delete forever. + let resolveUnsubscribe: () => void = () => {} + const unsubscribeMock = vi.fn( + () => + new Promise((resolve) => { + resolveUnsubscribe = resolve + }) + ) + vi.mocked(subscribeParcelWatcher).mockResolvedValue({ unsubscribe: unsubscribeMock } as never) + const sender = { isDestroyed: () => false, send: vi.fn(), once: vi.fn(), id: 1 } + + await handlers['fs:watchWorktree']({ sender }, { worktreePath: '/tmp/repo' }) + + vi.useFakeTimers() + try { + let closed = false + const closePromise = closeLocalWatcherForWorktreePath('/tmp/repo').then(() => { + closed = true + }) + await vi.advanceTimersByTimeAsync(WATCHER_REMOVAL_DRAIN_BUDGET_MS - 1) + expect(unsubscribeMock).toHaveBeenCalledTimes(1) + expect(closed).toBe(false) + + await vi.advanceTimersByTimeAsync(1) + await closePromise + expect(closed).toBe(true) + } finally { + resolveUnsubscribe() + vi.useRealTimers() + } + }) + + it('spends one shared budget across both drains instead of one per await', async () => { + vi.useFakeTimers() + try { + vi.mocked(stat).mockResolvedValue({ isDirectory: () => true } as never) + vi.mocked(subscribeViaWatcherProcess).mockImplementationOnce(() => new Promise(() => {})) + const sender = { + isDestroyed: () => false, + send: vi.fn(), + once: vi.fn(), + id: 1 + } + + const watchPromise = handlers['fs:watchWorktree']( + { sender }, + { worktreePath: '/tmp/repo' } + ) as Promise + await vi.waitFor(() => { + expect(subscribeViaWatcherProcess).toHaveBeenCalled() + }) + + // Half the budget is already gone before the close starts; the drain may only spend the rest. + const deadline = createWatcherRemovalDeadline() + await vi.advanceTimersByTimeAsync(WATCHER_REMOVAL_DRAIN_BUDGET_MS / 2) + + let closed = false + const closePromise = closeLocalWatcherForWorktreePath('/tmp/repo', deadline).then(() => { + closed = true + }) + // Why assert mid-drain: without this a fresh (unshared) budget would also pass the final check. + await vi.advanceTimersByTimeAsync( + WATCHER_REMOVAL_DRAIN_BUDGET_MS / 2 - WATCHER_REMOVAL_FINAL_DRAIN_RESERVE_MS - 1 + ) + expect(closed).toBe(false) + + await vi.advanceTimersByTimeAsync(1) + await closePromise + expect(closed).toBe(true) + + void watchPromise + } finally { + vi.useRealTimers() + } + }) + + it('does not let an abandoned unsubscribe poison a later close of the same root', async () => { + vi.mocked(stat).mockResolvedValue({ isDirectory: () => true } as never) + let rejectUnsubscribe: (error: unknown) => void = () => {} + const unsubscribeMock = vi.fn( + () => + new Promise((_resolve, reject) => { + rejectUnsubscribe = reject + }) + ) + vi.mocked(subscribeParcelWatcher).mockResolvedValue({ unsubscribe: unsubscribeMock } as never) + const sender = { isDestroyed: () => false, send: vi.fn(), once: vi.fn(), id: 1 } + await handlers['fs:watchWorktree']({ sender }, { worktreePath: '/tmp/repo' }) + + vi.useFakeTimers() + try { + const closePromise = closeLocalWatcherForWorktreePath('/tmp/repo') + await vi.advanceTimersByTimeAsync(WATCHER_REMOVAL_DRAIN_BUDGET_MS) + await expect(closePromise).resolves.toBeUndefined() + } finally { + vi.useRealTimers() + } + + // The delete this drain guarded already finished; a late native failure is stale news, and + // retaining it would leave this root permanently undeletable until the watcher process exits. + rejectUnsubscribe( + new WatcherProcessFailure( + 'file watcher process did not exit after termination deadline', + 'supervisor', + 'process_unavailable', + new Promise(() => {}) + ) + ) + await vi.waitFor(() => expect(unsubscribeMock).toHaveBeenCalledTimes(1)) + + await expect(closeLocalWatcherForWorktreePath('/tmp/repo')).resolves.toBeUndefined() + }) +}) + +describe('watcher removal drain budget', () => { + it('reserves a tail slice so a slow final unsubscribe is not abandoned at zero', async () => { + vi.useFakeTimers() + try { + const deadline = createWatcherRemovalDeadline() + const earlyDrain = drainBeforeWatcherRemoval( + new Promise(() => {}), + deadline, + 'wedged early drain', + { reserveMs: WATCHER_REMOVAL_FINAL_DRAIN_RESERVE_MS } + ) + await vi.advanceTimersByTimeAsync( + WATCHER_REMOVAL_DRAIN_BUDGET_MS - WATCHER_REMOVAL_FINAL_DRAIN_RESERVE_MS + ) + await expect(earlyDrain).resolves.toBe('timeout') + expect(deadline.remainingMs()).toBe(WATCHER_REMOVAL_FINAL_DRAIN_RESERVE_MS) + + let finishFinalUnsubscribe: () => void = () => {} + const finalDrain = drainBeforeWatcherRemoval( + new Promise((resolve) => { + finishFinalUnsubscribe = resolve + }), + deadline, + 'slow final unsubscribe' + ) + await vi.advanceTimersByTimeAsync(WATCHER_REMOVAL_FINAL_DRAIN_RESERVE_MS - 1) + finishFinalUnsubscribe() + + await expect(finalDrain).resolves.toBe('settled') + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/main/ipc/filesystem-watcher.ts b/src/main/ipc/filesystem-watcher.ts index b65a85c96..584017224 100644 --- a/src/main/ipc/filesystem-watcher.ts +++ b/src/main/ipc/filesystem-watcher.ts @@ -23,6 +23,12 @@ import { WatcherChildCapacityError } from './parcel-watcher-child-registry' import { beginWatcherInstall, isWatcherRemovalInProgressError } from './watcher-removal-gate' +import { + createWatcherRemovalDeadline, + drainBeforeWatcherRemoval, + WATCHER_REMOVAL_FINAL_DRAIN_RESERVE_MS, + type WatcherRemovalDeadline +} from './watcher-removal-drain' // Why: suppress high-churn dirs at the watcher level (separate from the File Explorer display filter, which only hides rows). import { WATCHER_IGNORE_DIRS, buildParcelWatcherIgnoreOptions } from './filesystem-watcher-ignore' @@ -72,6 +78,10 @@ const suspendedLocalWatcherListeners = new Map< let localWatchersClosed = false let localWatcherLifecycleGeneration = 0 const failedLocalUnsubscribes = new Map() +// Why: a drain that timed out no longer gates the delete — Git removal already proceeded past it. Its +// late failure must not fail-close a *later* close of the same root, which would leave that path +// undeletable until the watcher process physically exits. +const abandonedLocalUnsubscribes = new WeakSet>() type LocalWatcherInstallToken = { cancelled: boolean listeners: Map @@ -478,12 +488,21 @@ function trackLocalUnsubscribe(rootKey: string, root: WatchedRoot): Promise { - retainLocalWatcherPhysicalFailure(rootKey, error) + if (!abandonedLocalUnsubscribes.has(unsubscribePromise)) { + retainLocalWatcherPhysicalFailure(rootKey, error) + } console.error(`[filesystem-watcher] unsubscribe error for ${rootKey}:`, error) }) return unsubscribePromise } +/** Mark unsubscribes whose drain timed out so their late failures stay out of failedLocalUnsubscribes. */ +function abandonLocalUnsubscribes(promises: Iterable>): void { + for (const promise of promises) { + abandonedLocalUnsubscribes.add(promise) + } +} + function retainLocalWatcherPhysicalFailure(rootKey: string, error: unknown): void { if (!isWatcherProcessFailure(error) || !error.physicalExit) { return @@ -780,7 +799,10 @@ function unsubscribe(worktreePath: string, senderId: number): void { } } -export async function closeLocalWatcherForWorktreePath(worktreePath: string): Promise { +export async function closeLocalWatcherForWorktreePath( + worktreePath: string, + deadline: WatcherRemovalDeadline = createWatcherRemovalDeadline() +): Promise { const { key: rootKey } = localWatcherRoot(worktreePath) const suspended = suspendedLocalWatcherListeners.get(rootKey) ?? { worktreePath, @@ -814,10 +836,45 @@ export async function closeLocalWatcherForWorktreePath(worktreePath: string): Pr inFlight.cancelled = true inFlight.abortController.abort() } - await pendingLocalInstallPromises.get(rootKey) + // Why: abort alone is not enough if the native subscribe never settles; bound so delete cannot hang the app. + const pendingInstall = pendingLocalInstallPromises.get(rootKey) + const installDrain = await drainBeforeWatcherRemoval( + pendingInstall, + deadline, + `local watcher install for ${rootKey}`, + { reserveMs: WATCHER_REMOVAL_FINAL_DRAIN_RESERVE_MS } + ) + if (installDrain === 'timeout') { + // Why: an abandoned install never runs its own cleanup, so leaving these entries would make every + // later watch of this root queue behind the same wedged promise. Identity-checked so a late settle + // can't evict a newer install. + if (pendingLocalInstallPromises.get(rootKey) === pendingInstall) { + pendingLocalInstallPromises.delete(rootKey) + } + if (inFlight && inFlightLocalInstalls.get(rootKey) === inFlight) { + inFlightLocalInstalls.delete(rootKey) + } + } const pendingUnsubscribes = pendingLocalUnsubscribesByRoot.get(rootKey) if (pendingUnsubscribes) { - await Promise.all(Array.from(pendingUnsubscribes)) + const draining = Array.from(pendingUnsubscribes) + const unsubscribeDrain = await drainBeforeWatcherRemoval( + // Why the per-promise catch: an already-abandoned unsubscribe belongs to a delete that finished + // without it; re-raising its rejection here would fail a later close on stale news. + Promise.all( + draining.map((unsubscribe) => + abandonedLocalUnsubscribes.has(unsubscribe) + ? unsubscribe.catch(() => undefined) + : unsubscribe + ) + ), + deadline, + `local watcher unsubscribe for ${rootKey}`, + { reserveMs: WATCHER_REMOVAL_FINAL_DRAIN_RESERVE_MS } + ) + if (unsubscribeDrain === 'timeout') { + abandonLocalUnsubscribes(draining) + } } if (failedLocalUnsubscribes.has(rootKey)) { throw failedLocalUnsubscribes.get(rootKey) @@ -831,7 +888,18 @@ export async function closeLocalWatcherForWorktreePath(worktreePath: string): Pr clearTimeout(root.batch.timer) } watchedRoots.delete(rootKey) - await trackLocalUnsubscribe(rootKey, root) + // Why: the in-process Parcel fallback has no unsubscribe timeout of its own, so an unbounded await + // here would hang delete forever and hold the removal gate. The promise stays tracked in + // pendingLocalUnsubscribesByRoot, so a later close still observes its failure. + const finalUnsubscribe = trackLocalUnsubscribe(rootKey, root) + const finalDrain = await drainBeforeWatcherRemoval( + finalUnsubscribe, + deadline, + `local watcher unsubscribe for ${rootKey}` + ) + if (finalDrain === 'timeout') { + abandonLocalUnsubscribes([finalUnsubscribe]) + } } export async function restoreLocalWatcherAfterFailedRemoval(worktreePath: string): Promise { diff --git a/src/main/ipc/watcher-removal-drain.ts b/src/main/ipc/watcher-removal-drain.ts new file mode 100644 index 000000000..f926f367c --- /dev/null +++ b/src/main/ipc/watcher-removal-drain.ts @@ -0,0 +1,67 @@ +// Why: worktree deletion has to drain in-flight watcher installs/unsubscribes before Git removes the +// tree, and a wedged native subscribe can leave those promises unsettled forever. Independent per-await +// timeouts would compose (two close passes x two drains), so one removal shares one absolute deadline. + +export const WATCHER_REMOVAL_DRAIN_BUDGET_MS = 60_000 +// Why: the final live unsubscribe is the drain that actually releases the native handle. Earlier drains +// leave it this slice so a slow-but-finishing unsubscribe on Windows/WSL isn't abandoned at ~0ms left. +export const WATCHER_REMOVAL_FINAL_DRAIN_RESERVE_MS = 10_000 + +export type WatcherRemovalDeadline = { + remainingMs(reserveMs?: number): number +} + +export function createWatcherRemovalDeadline( + budgetMs: number = WATCHER_REMOVAL_DRAIN_BUDGET_MS +): WatcherRemovalDeadline { + const expiresAt = Date.now() + budgetMs + return { + remainingMs: (reserveMs = 0) => Math.max(0, expiresAt - Date.now() - reserveMs) + } +} + +export type WatcherRemovalDrainOutcome = 'settled' | 'timeout' | 'skipped' + +export type WatcherRemovalDrainOptions = { + /** Budget this drain must leave behind for the removal's final unsubscribe. */ + reserveMs?: number +} + +/** Await `promise` until the removal deadline expires. Rejections still propagate so genuine + * teardown failures keep failing the delete closed; only an unsettled wait is abandoned. */ +export async function drainBeforeWatcherRemoval( + promise: Promise | undefined, + deadline: WatcherRemovalDeadline, + label: string, + options: WatcherRemovalDrainOptions = {} +): Promise { + if (!promise) { + return 'skipped' + } + const waitMs = deadline.remainingMs(options.reserveMs) + let timer: ReturnType | undefined + try { + const settled = promise.then(() => 'settled' as const) + const outcome = await Promise.race([ + settled, + new Promise<'timeout'>((resolve) => { + timer = setTimeout(() => resolve('timeout'), waitMs) + }) + ]) + if (outcome === 'timeout') { + // Why: nobody awaits the abandoned promise anymore, so a late rejection (an aborted native + // subscribe finally reporting) would be an unhandled rejection — fatal in the main process. + void settled.catch(() => {}) + // Why log the budget split: production traces need "slow but finishing" separable from "wedged". + console.warn( + `[watcher-removal] Timed out waiting for ${label} after ${waitMs}ms ` + + `(${deadline.remainingMs()}ms of the removal budget left); continuing removal` + ) + } + return outcome + } finally { + if (timer !== undefined) { + clearTimeout(timer) + } + } +} diff --git a/src/main/ipc/watcher-removal-gate.test.ts b/src/main/ipc/watcher-removal-gate.test.ts index 520bdd55d..221f5f724 100644 --- a/src/main/ipc/watcher-removal-gate.test.ts +++ b/src/main/ipc/watcher-removal-gate.test.ts @@ -29,6 +29,47 @@ describe('watcher removal gate', () => { finishLaterInstall() }) + it('drops abandoned install fences so a later removal is not fenced by a wedged install', async () => { + beginWatcherInstall('/repo/nested') + const removal = acquireWatcherRemovalGate('/repo') + let ready = false + void removal.ready.then(() => { + ready = true + }) + await Promise.resolve() + expect(ready).toBe(false) + + removal.abandonPendingInstalls() + await removal.ready + removal.release() + + const retry = acquireWatcherRemovalGate('/repo') + await retry.ready + retry.release() + }) + + it('keeps a fresh install fenced after an unrelated install was abandoned', async () => { + const wedged = beginWatcherInstall('/repo') + const removal = acquireWatcherRemovalGate('/repo') + removal.abandonPendingInstalls() + removal.release() + // Why: a late finishInstall from the abandoned slot must not release a newer install's fence. + const finishFresh = beginWatcherInstall('/repo') + wedged() + + const retry = acquireWatcherRemovalGate('/repo') + let ready = false + void retry.ready.then(() => { + ready = true + }) + await Promise.resolve() + expect(ready).toBe(false) + + finishFresh() + await retry.ready + retry.release() + }) + it('scopes identical roots to their execution host', async () => { const removal = acquireWatcherRemovalGate('/repo', 'ssh-a') await removal.ready diff --git a/src/main/ipc/watcher-removal-gate.ts b/src/main/ipc/watcher-removal-gate.ts index 4ae9c2d39..de19acfdf 100644 --- a/src/main/ipc/watcher-removal-gate.ts +++ b/src/main/ipc/watcher-removal-gate.ts @@ -7,16 +7,25 @@ import { WATCHER_REMOVAL_IN_PROGRESS_MESSAGE } from '../../shared/worktree-removal-fence-error' +// Why: fence slots are identities, not a counter, so a removal that gives up waiting can drop exactly +// the installs it waited on without a late finishInstall corrupting the count for a newer install. +type WatcherInstallToken = symbol + type WatcherRemovalGateState = { + key: string connectionId: string | null rootPath: string - installCount: number + installs: Set removalCount: number installDrainWaiters: Set<() => void> } export type WatcherRemovalGate = { ready: Promise + /** Drop the install fence slots this removal waited on. Call only after `ready` timed out: the + * installs are presumed wedged, and leaving them counted makes every later removal of this root + * wait out the full drain budget again. */ + abandonPendingInstalls(): void release(): void } @@ -76,20 +85,11 @@ function beginRemovalSensitiveInstall( } const key = watcherRemovalGateKey(normalizedRoot, connectionId) const state = states.get(key) ?? createState(key, normalizedRoot, connectionId) - state.installCount++ - let released = false + const token: WatcherInstallToken = Symbol(normalizedRoot) + state.installs.add(token) return () => { - if (released) { - return - } - released = true - state.installCount-- - if (state.installCount === 0) { - for (const resolve of state.installDrainWaiters) { - resolve() - } - state.installDrainWaiters.clear() - deleteIdleState(key, state) + if (state.installs.delete(token) && state.installs.size === 0) { + resolveInstallDrain(state) } } } @@ -117,16 +117,30 @@ export function acquireWatcherRemovalGate( state.removalCount++ // Why: deleting a parent root must wait for native installs already admitted // under that root, not only installs keyed to the exact same spelling. - const drains = matchingHostStates(connectionId) + const fenced = matchingHostStates(connectionId) .filter( (candidate) => - candidate.installCount > 0 && isPathInsideOrEqual(normalizedRoot, candidate.rootPath) + candidate.installs.size > 0 && isPathInsideOrEqual(normalizedRoot, candidate.rootPath) ) - .map((candidate) => new Promise((resolve) => candidate.installDrainWaiters.add(resolve))) + .map((candidate) => ({ state: candidate, tokens: new Set(candidate.installs) })) + const drains = fenced.map( + ({ state: candidate }) => + new Promise((resolve) => candidate.installDrainWaiters.add(resolve)) + ) const ready = drains.length === 0 ? Promise.resolve() : Promise.all(drains).then(() => undefined) let released = false return { ready, + abandonPendingInstalls: () => { + for (const { state: candidate, tokens } of fenced) { + for (const token of tokens) { + candidate.installs.delete(token) + } + if (candidate.installs.size === 0) { + resolveInstallDrain(candidate) + } + } + }, release: () => { if (released) { return @@ -161,9 +175,10 @@ function createState( connectionId?: string ): WatcherRemovalGateState { const state = { + key, connectionId: connectionId ?? null, rootPath, - installCount: 0, + installs: new Set(), removalCount: 0, installDrainWaiters: new Set<() => void>() } @@ -171,8 +186,16 @@ function createState( return state } +function resolveInstallDrain(state: WatcherRemovalGateState): void { + for (const resolve of state.installDrainWaiters) { + resolve() + } + state.installDrainWaiters.clear() + deleteIdleState(state.key, state) +} + function deleteIdleState(key: string, state: WatcherRemovalGateState): void { - if (state.installCount === 0 && state.removalCount === 0 && states.get(key) === state) { + if (state.installs.size === 0 && state.removalCount === 0 && states.get(key) === state) { states.delete(key) } } diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts index d7745a132..5a7c27892 100644 --- a/src/main/ipc/worktrees-windows.test.ts +++ b/src/main/ipc/worktrees-windows.test.ts @@ -122,7 +122,7 @@ vi.mock('./pty', () => ({ getLocalPtyProvider: getLocalPtyProviderMock })) -vi.mock('../terminal-history', () => ({ +vi.mock('../terminal-history-deletion', () => ({ deleteWorktreeHistoryDir: deleteWorktreeHistoryDirMock })) @@ -428,6 +428,10 @@ describe('registerWorktreeHandlers – Windows path handling', () => { }) ) expect(store.removeWorktreeMeta).toHaveBeenCalledWith('repo-1::C:/workspaces/improve-dashboard') + // Windows history lives under a path-derived hash, so scheduling must not regress on this path only. + expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith( + 'repo-1::C:/workspaces/improve-dashboard' + ) expect(mainWindow.webContents.send).toHaveBeenCalledWith('worktrees:changed', { repoId: 'repo-1' }) diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 646b5cc3a..d1e5925f2 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -217,7 +217,7 @@ const { deleteWorktreeHistoryDirMock } = vi.hoisted(() => ({ deleteWorktreeHistoryDirMock: vi.fn() })) -vi.mock('../terminal-history', () => ({ +vi.mock('../terminal-history-deletion', () => ({ deleteWorktreeHistoryDir: deleteWorktreeHistoryDirMock })) @@ -258,6 +258,8 @@ import { notifyWorktreesChanged } from './worktree-remote' import { invalidateAuthorizedRootsCache, resolveRegisteredWorktreePath } from './filesystem-auth' +import { _resetTracerForTests, setActiveSink } from '../observability/tracer' +import type { RedactableSpan } from '../observability/redactor' import { reviewHeadRemoteRefComponent, REVIEW_HEAD_FETCH_TIMEOUT_MS @@ -7858,6 +7860,73 @@ describe('registerWorktreeHandlers', () => { }) }) + it('traces the removal as worktree.remove with a stage sub-span tree', async () => { + const records: RedactableSpan[] = [] + setActiveSink({ + push: (record) => records.push(record as RedactableSpan), + flush: () => {}, + close: () => {} + }) + try { + mockKnownFeatureWorktree() + getEffectiveHooksMock.mockReturnValue(null) + removeWorktreeMock.mockResolvedValue({}) + + await handlers['worktrees:remove'](null, { worktreeId: 'repo-1::/workspace/feature-wt' }) + + const parent = records.find((record) => record.name === 'worktree.remove') + expect(parent).toBeDefined() + expect(parent?.attributes).toMatchObject({ + kind: 'worktree', + 'worktree.stage': 'remove', + 'worktree.path': '/workspace/feature-wt' + }) + const stages = records.filter((record) => record.name.startsWith('worktree.remove.')) + expect(stages.map((record) => record.name)).toEqual( + expect.arrayContaining([ + 'worktree.remove.watcher_gate', + 'worktree.remove.pty_sweep', + 'worktree.remove.git_remove', + 'worktree.remove.metadata_purge', + 'worktree.remove.cache_invalidation' + ]) + ) + // Stages must hang off the removal span, not float as roots, or a freeze can't be attributed. + for (const stage of stages) { + expect(stage.parentSpanId).toBe(parent?.spanId) + expect(stage.attributes).toMatchObject({ kind: 'worktree', 'worktree.flow': 'local' }) + } + } finally { + _resetTracerForTests() + } + }) + + it('traces a local archive hook as flow local, not remote', async () => { + const records: RedactableSpan[] = [] + setActiveSink({ + push: (record) => records.push(record as RedactableSpan), + flush: () => {}, + close: () => {} + }) + try { + mockKnownFeatureWorktree() + // The archive hook block is shared by both flows, so a local repo must not land under 'remote'. + getEffectiveHooksMock.mockReturnValue({ scripts: { archive: 'pnpm worktree:archive' } }) + runHookMock.mockResolvedValue({ success: true, output: '' }) + removeWorktreeMock.mockResolvedValue({}) + + await handlers['worktrees:remove'](null, { worktreeId: 'repo-1::/workspace/feature-wt' }) + + const archiveStage = records.find((record) => record.name === 'worktree.remove.archive_hook') + expect(archiveStage?.attributes).toMatchObject({ + kind: 'worktree', + 'worktree.flow': 'local' + }) + } finally { + _resetTracerForTests() + } + }) + it('prunes git worktree tracking when removing an orphaned worktree', async () => { mockKnownFeatureWorktree() const orphanError = Object.assign(new Error('git worktree remove failed'), { @@ -8174,6 +8243,13 @@ describe('registerWorktreeHandlers', () => { expect(removeWorktreeLinkedPathsMock).toHaveBeenCalledWith('/workspace/feature-wt', [ 'node_modules' ]) + // Why order matters: linked-path deletion is destructive, so PTYs must release every handle + // before Windows or WSL filesystem cleanup starts (mirrors the runtime removal path). + expect(killAllProcessesForWorktreeMock).toHaveBeenCalled() + // Latest PTY sweep vs earliest deletion: a later sweep would mean handles were still open. + expect(Math.max(...killAllProcessesForWorktreeMock.mock.invocationCallOrder)).toBeLessThan( + Math.min(...removeWorktreeLinkedPathsMock.mock.invocationCallOrder) + ) }) it('does not remove a worktree when watcher teardown cannot release it', async () => { diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 64ffdf257..73eab28da 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -15,7 +15,7 @@ import { getProjectHostSetupWorktreeMeta } from '../../shared/project-host-setup import { getProjectGroupSubtreeIds } from '../../shared/project-groups' import { projectResolvedWorktreeLineage } from '../../shared/resolved-worktree-lineage' import { isPathInsideOrEqual, isWindowsAbsolutePathLike } from '../../shared/cross-platform-path' -import { deleteWorktreeHistoryDir } from '../terminal-history' +import { deleteWorktreeHistoryDir } from '../terminal-history-deletion' import type { AutomationWorkspaceProvenance, CliWorkspaceProvenance, @@ -69,7 +69,7 @@ import { removeWorktree } from '../git/worktree' import { gitExecFileAsync } from '../git/runner' -import { withWorktreeSpan } from '../observability/instrumentation' +import { withWorktreeRemoveStageSpan, withWorktreeSpan } from '../observability/instrumentation' import { resolveGitHubPrStartPoint } from '../github/pr-start-point' import { fetchGitHubPullRequestHeadRef, @@ -260,6 +260,7 @@ function removeWorktreeMetadataAndTransientState(store: Store, worktreeId: strin advertisedUrlWatcher.forgetWorktree(worktreeId) // Why: drop this worktree's localhost label routes so they don't accumulate in the proxy's route maps all session. localhostWorktreeLabelProxy.unregisterWorktree(worktreeId) + // Why: schedule async history tree removal — never recursive-rmSync on the delete critical path. deleteWorktreeHistoryDir(worktreeId) // Why: release the removed worktree's PR-refresh aliases so coalesced queue entries don't retain it all session (memory creep). pruneWorktreePRRefreshAliases(worktreeId) @@ -2204,237 +2205,522 @@ export function registerWorktreeHandlers( // Why: concurrent stale-toast/double-click/sidebar races can hit the same worktree; share the op so only one path touches Git and disk. const removal = (async (): Promise => { - if (isFolderRepo(repo)) { - if (args.worktreeId === getFolderWorkspaceRootId(repo)) { - throw new Error( - 'Cannot delete the project root workspace. Remove the folder project instead.' - ) - } - // Why: folder workspaces share one root, so there's no Git remove step to close shells; sweep PTYs before dropping metadata. - await killAllProcessesForWorktree(args.worktreeId, { - runtime, - localProvider: getLocalPtyProvider(), - onPtyStopped: clearProviderPtyState - }).catch((err) => { - console.warn(`[worktree-teardown] failed for ${args.worktreeId}:`, err) - }) - removeWorktreeMetadataAndTransientState(store, args.worktreeId) - preservedBranchCleanupByWorktreeId.delete(args.worktreeId) - notifyWorktreesChanged(mainWindow, repoId) - return {} - } - - // Why: renderer-supplied worktreeId embeds a path; re-derive the canonical path from git before any destructive action. - const provider = repo.connectionId ? requireSshGitProvider(repo.connectionId) : null - const localWorktreeGitOptions = repo.connectionId - ? {} - : getLocalProjectWorktreeGitOptions(store, repo) - const hasLocalWorktreeGitOptions = Object.keys(localWorktreeGitOptions).length > 0 - const registeredWorktrees = repo.connectionId - ? await provider!.listWorktrees(repo.path) - : hasLocalWorktreeGitOptions - ? await listGitWorktreesStrict(repo.path, localWorktreeGitOptions) - : await listGitWorktreesStrict(repo.path) - const removedMeta = store.getWorktreeMeta(args.worktreeId) - const removedPushTarget = removedMeta?.pushTarget - const registeredWorktree = findRegisteredDeletableWorktree( - repo.path, - worktreePath, - registeredWorktrees - ) - if (!registeredWorktree) { - const fsProvider = repo.connectionId ? getSshFilesystemProvider(repo.connectionId) : null - let canCleanOrphanedDirectory = false - if ( - canCleanupUnregisteredOrcaWorktreeDirectory({ - meta: removedMeta + // Why: worktree.create is traced; delete freezes were invisible without a matching worktree.remove parent span. + return withWorktreeSpan({ stage: 'remove', path: worktreePath }, async () => { + if (isFolderRepo(repo)) { + if (args.worktreeId === getFolderWorkspaceRootId(repo)) { + throw new Error( + 'Cannot delete the project root workspace. Remove the folder project instead.' + ) + } + // Why: folder workspaces share one root, so there's no Git remove step to close shells; sweep PTYs before dropping metadata. + await withWorktreeRemoveStageSpan('pty_sweep', 'folder', async () => { + await killAllProcessesForWorktree(args.worktreeId, { + runtime, + localProvider: getLocalPtyProvider(), + onPtyStopped: clearProviderPtyState + }).catch((err) => { + console.warn(`[worktree-teardown] failed for ${args.worktreeId}:`, err) + }) + }) + await withWorktreeRemoveStageSpan('metadata_purge', 'folder', async () => { + removeWorktreeMetadataAndTransientState(store, args.worktreeId) }) - ) { - if (repo.connectionId) { - if (!fsProvider) { - throw new Error('SSH filesystem provider unavailable') - } - if (!fsProvider.lstat) { - throw new Error('SSH filesystem provider lstat unavailable') - } - canCleanOrphanedDirectory = await canSafelyRemoveOrphanedWorktreeDirectory( - worktreePath, - repo.path, - (path) => fsProvider.lstat!(path), - (path) => fsProvider.readFile(path) - ) - } else { - const access = getLocalWorktreePathAccess(localWorktreeGitOptions) - canCleanOrphanedDirectory = - !isDangerousWorktreeRemovalPath(worktreePath, repo.path) && - (await canSafelyRemoveOrphanedWorktreeDirectory( - toLocalWorktreeRuntimePath(worktreePath, localWorktreeGitOptions), - toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), - access.statPath, - access.readPath - )) - } - } - if (canCleanOrphanedDirectory) { - assertWorktreeDoesNotContainRegisteredWorktree(worktreePath, registeredWorktrees) - if (!args.force) { - throw new Error(ORPHANED_WORKTREE_DIRECTORY_MESSAGE) - } - if (repo.connectionId) { - const removalGate = await runtime.acquireFileWatcherRemoval( - worktreePath, - repo.connectionId - ) - let removalCompleted = false - try { - await stopPtysForDestructiveWorktreeRemoval( - runtime, - args.worktreeId, - repo.connectionId - ) - await fsProvider!.deletePath(worktreePath, true) - removalCompleted = true - } finally { - await removalGate.finish(removalCompleted) - } - await cleanupUnusedWorktreePushTargetRemoteSsh( - provider!, - repo.path, - args.worktreeId, - removedPushTarget, - store - ) - } else { - const removalGate = await runtime.acquireFileWatcherRemoval(worktreePath) - let removalCompleted = false - try { - await stopPtysForDestructiveWorktreeRemoval(runtime, args.worktreeId) - await removeLocalWorktreePath(worktreePath, localWorktreeGitOptions) - removalCompleted = true - } finally { - await removalGate.finish(removalCompleted) - } - await cleanupUnusedWorktreePushTargetRemote( - repo.path, - args.worktreeId, - removedPushTarget, - store, - localWorktreeGitOptions - ) - invalidateAuthorizedRootsCache() - } - runtime.clearOptimisticReconcileToken(args.worktreeId) - removeWorktreeMetadataAndTransientState(store, args.worktreeId) preservedBranchCleanupByWorktreeId.delete(args.worktreeId) notifyWorktreesChanged(mainWindow, repoId) return {} } - if (!repo.connectionId) { - const access = getLocalWorktreePathAccess(localWorktreeGitOptions) - const runtimeWorktreePath = toLocalWorktreeRuntimePath( - worktreePath, - localWorktreeGitOptions - ) + + // Why: renderer-supplied worktreeId embeds a path; re-derive the canonical path from git before any destructive action. + const provider = repo.connectionId ? requireSshGitProvider(repo.connectionId) : null + const localWorktreeGitOptions = repo.connectionId + ? {} + : getLocalProjectWorktreeGitOptions(store, repo) + const hasLocalWorktreeGitOptions = Object.keys(localWorktreeGitOptions).length > 0 + const registeredWorktrees = repo.connectionId + ? await provider!.listWorktrees(repo.path) + : hasLocalWorktreeGitOptions + ? await listGitWorktreesStrict(repo.path, localWorktreeGitOptions) + : await listGitWorktreesStrict(repo.path) + const removedMeta = store.getWorktreeMeta(args.worktreeId) + const removedPushTarget = removedMeta?.pushTarget + const registeredWorktree = findRegisteredDeletableWorktree( + repo.path, + worktreePath, + registeredWorktrees + ) + if (!registeredWorktree) { + const fsProvider = repo.connectionId + ? getSshFilesystemProvider(repo.connectionId) + : null + let canCleanOrphanedDirectory = false if ( - await canCleanupUnregisteredOrcaLeftoverDirectory({ - meta: removedMeta, - worktreePath, - runtimeWorktreePath, - repo, - runtimeRepoPath: toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), - registeredWorktrees, - statPath: access.statPath, - isGitRepository: (path) => isLocalGitRepository(path, localWorktreeGitOptions) + canCleanupUnregisteredOrcaWorktreeDirectory({ + meta: removedMeta }) ) { + if (repo.connectionId) { + if (!fsProvider) { + throw new Error('SSH filesystem provider unavailable') + } + if (!fsProvider.lstat) { + throw new Error('SSH filesystem provider lstat unavailable') + } + canCleanOrphanedDirectory = await canSafelyRemoveOrphanedWorktreeDirectory( + worktreePath, + repo.path, + (path) => fsProvider.lstat!(path), + (path) => fsProvider.readFile(path) + ) + } else { + const access = getLocalWorktreePathAccess(localWorktreeGitOptions) + canCleanOrphanedDirectory = + !isDangerousWorktreeRemovalPath(worktreePath, repo.path) && + (await canSafelyRemoveOrphanedWorktreeDirectory( + toLocalWorktreeRuntimePath(worktreePath, localWorktreeGitOptions), + toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), + access.statPath, + access.readPath + )) + } + } + if (canCleanOrphanedDirectory) { + assertWorktreeDoesNotContainRegisteredWorktree(worktreePath, registeredWorktrees) if (!args.force) { throw new Error(ORPHANED_WORKTREE_DIRECTORY_MESSAGE) } - const removalGate = await runtime.acquireFileWatcherRemoval(worktreePath) - let removalCompleted = false - try { - await stopPtysForDestructiveWorktreeRemoval(runtime, args.worktreeId) - await removeLocalWorktreePath(worktreePath, localWorktreeGitOptions) - removalCompleted = true - } finally { - await removalGate.finish(removalCompleted) + if (repo.connectionId) { + const removalGate = await runtime.acquireFileWatcherRemoval( + worktreePath, + repo.connectionId + ) + let removalCompleted = false + try { + await stopPtysForDestructiveWorktreeRemoval( + runtime, + args.worktreeId, + repo.connectionId + ) + await fsProvider!.deletePath(worktreePath, true) + removalCompleted = true + } finally { + await removalGate.finish(removalCompleted) + } + await cleanupUnusedWorktreePushTargetRemoteSsh( + provider!, + repo.path, + args.worktreeId, + removedPushTarget, + store + ) + } else { + const removalGate = await runtime.acquireFileWatcherRemoval(worktreePath) + let removalCompleted = false + try { + await stopPtysForDestructiveWorktreeRemoval(runtime, args.worktreeId) + await removeLocalWorktreePath(worktreePath, localWorktreeGitOptions) + removalCompleted = true + } finally { + await removalGate.finish(removalCompleted) + } + await cleanupUnusedWorktreePushTargetRemote( + repo.path, + args.worktreeId, + removedPushTarget, + store, + localWorktreeGitOptions + ) + invalidateAuthorizedRootsCache() } - await cleanupUnusedWorktreePushTargetRemote( - repo.path, - args.worktreeId, - removedPushTarget, - store, - localWorktreeGitOptions - ) runtime.clearOptimisticReconcileToken(args.worktreeId) removeWorktreeMetadataAndTransientState(store, args.worktreeId) preservedBranchCleanupByWorktreeId.delete(args.worktreeId) - invalidateAuthorizedRootsCache() notifyWorktreesChanged(mainWindow, repoId) return {} } - } - if (await isAlreadyRemovedWorktreePath(repo, worktreePath, localWorktreeGitOptions)) { - if (!args.force && !removedMeta) { - // Why: without persisted metadata, require the renderer recovery path before deleting Orca-only state for an unregistered path. - throw new Error(UNREGISTERED_MISSING_WORKTREE_MESSAGE) - } - // Why: a manually deleted worktree is already gone; persisted metadata proves it was an Orca-known row, so no force is needed. - if (repo.connectionId) { - await cleanupUnusedWorktreePushTargetRemoteSsh( - provider!, - repo.path, - args.worktreeId, - removedPushTarget, - store - ) - } else { - await cleanupUnusedWorktreePushTargetRemote( - repo.path, - args.worktreeId, - removedPushTarget, - store, + if (!repo.connectionId) { + const access = getLocalWorktreePathAccess(localWorktreeGitOptions) + const runtimeWorktreePath = toLocalWorktreeRuntimePath( + worktreePath, localWorktreeGitOptions ) - invalidateAuthorizedRootsCache() + if ( + await canCleanupUnregisteredOrcaLeftoverDirectory({ + meta: removedMeta, + worktreePath, + runtimeWorktreePath, + repo, + runtimeRepoPath: toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), + registeredWorktrees, + statPath: access.statPath, + isGitRepository: (path) => isLocalGitRepository(path, localWorktreeGitOptions) + }) + ) { + if (!args.force) { + throw new Error(ORPHANED_WORKTREE_DIRECTORY_MESSAGE) + } + const removalGate = await runtime.acquireFileWatcherRemoval(worktreePath) + let removalCompleted = false + try { + await stopPtysForDestructiveWorktreeRemoval(runtime, args.worktreeId) + await removeLocalWorktreePath(worktreePath, localWorktreeGitOptions) + removalCompleted = true + } finally { + await removalGate.finish(removalCompleted) + } + await cleanupUnusedWorktreePushTargetRemote( + repo.path, + args.worktreeId, + removedPushTarget, + store, + localWorktreeGitOptions + ) + runtime.clearOptimisticReconcileToken(args.worktreeId) + removeWorktreeMetadataAndTransientState(store, args.worktreeId) + preservedBranchCleanupByWorktreeId.delete(args.worktreeId) + invalidateAuthorizedRootsCache() + notifyWorktreesChanged(mainWindow, repoId) + return {} + } } + if (await isAlreadyRemovedWorktreePath(repo, worktreePath, localWorktreeGitOptions)) { + if (!args.force && !removedMeta) { + // Why: without persisted metadata, require the renderer recovery path before deleting Orca-only state for an unregistered path. + throw new Error(UNREGISTERED_MISSING_WORKTREE_MESSAGE) + } + // Why: a manually deleted worktree is already gone; persisted metadata proves it was an Orca-known row, so no force is needed. + if (repo.connectionId) { + await cleanupUnusedWorktreePushTargetRemoteSsh( + provider!, + repo.path, + args.worktreeId, + removedPushTarget, + store + ) + } else { + await cleanupUnusedWorktreePushTargetRemote( + repo.path, + args.worktreeId, + removedPushTarget, + store, + localWorktreeGitOptions + ) + invalidateAuthorizedRootsCache() + } + runtime.clearOptimisticReconcileToken(args.worktreeId) + removeWorktreeMetadataAndTransientState(store, args.worktreeId) + preservedBranchCleanupByWorktreeId.delete(args.worktreeId) + notifyWorktreesChanged(mainWindow, repoId) + return {} + } + throw new Error(`Refusing to delete unregistered worktree path: ${worktreePath}`) + } + const canonicalWorktreePath = registeredWorktree.path + const deleteBranch = removedMeta?.preserveBranchOnDelete !== true + + // Why: a Git lock must block before archive hooks or linked-path cleanup mutate the workspace; dirty-file force is separate. + try { + assertWorktreeUnlockedForRemoval(registeredWorktree) + } catch (error) { + throw new Error( + formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false) + ) + } + + // Why: a prior forced Windows recovery can delete the dir but leave a stale Git registration; verify before clearing metadata. + if ( + !repo.connectionId && + args.force === true && + process.platform === 'win32' && + (isWindowsAbsolutePathLike(canonicalWorktreePath) || + !!localWorktreeGitOptions.wslDistro) && + removedMeta && + (await isAlreadyRemovedWorktreePath( + repo, + canonicalWorktreePath, + localWorktreeGitOptions + )) + ) { + const removalResult = await removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({ + canonicalWorktreePath, + repoPath: repo.path, + localWorktreeGitOptions, + registeredWorktree, + deleteBranch + }) + await cleanupUnusedWorktreePushTargetRemote( + repo.path, + args.worktreeId, + removedPushTarget, + store, + localWorktreeGitOptions + ) + rememberPreservedBranchCleanupTarget( + args.worktreeId, + removalResult, + registeredWorktree.head, + removedPushTarget + ) runtime.clearOptimisticReconcileToken(args.worktreeId) removeWorktreeMetadataAndTransientState(store, args.worktreeId) - preservedBranchCleanupByWorktreeId.delete(args.worktreeId) + invalidateAuthorizedRootsCache() notifyWorktreesChanged(mainWindow, repoId) - return {} + return removalResult ?? {} } - throw new Error(`Refusing to delete unregistered worktree path: ${worktreePath}`) - } - const canonicalWorktreePath = registeredWorktree.path - const deleteBranch = removedMeta?.preserveBranchOnDelete !== true - // Why: a Git lock must block before archive hooks or linked-path cleanup mutate the workspace; dirty-file force is separate. - try { - assertWorktreeUnlockedForRemoval(registeredWorktree) - } catch (error) { - throw new Error( - formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false) - ) - } + // Run archive hook before removal so teardown scripts still see the worktree directory. + const hooks = await getArchiveHooksForRemoval(repo) + const archiveScript = hooks?.scripts.archive + if (archiveScript && !args.skipArchive) { + // Why the branch on connectionId: this block is shared by both flows, so a hardcoded + // 'remote' would file every local archive hook under the SSH breakdown. + await withWorktreeRemoveStageSpan( + 'archive_hook', + repo.connectionId ? 'remote' : 'local', + async () => { + const result = repo.connectionId + ? await runRemoteArchiveHook(repo, canonicalWorktreePath, archiveScript) + : await runHook( + 'archive', + canonicalWorktreePath, + repo, + undefined, + localWorktreeGitOptions + ) + if (!result.success) { + console.error( + `[hooks] archive hook failed for ${canonicalWorktreePath}:`, + result.output + ) + } + } + ) + } - // Why: a prior forced Windows recovery can delete the dir but leave a stale Git registration; verify before clearing metadata. - if ( - !repo.connectionId && - args.force === true && - process.platform === 'win32' && - (isWindowsAbsolutePathLike(canonicalWorktreePath) || - !!localWorktreeGitOptions.wslDistro) && - removedMeta && - (await isAlreadyRemovedWorktreePath(repo, canonicalWorktreePath, localWorktreeGitOptions)) - ) { - const removalResult = await removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({ + const remoteConnectionId = repo.connectionId ?? undefined + if (remoteConnectionId) { + // Why: SSH deletion mirrors the local flow — hooks run while the directory is intact, then the clean check guards removal. + if (!args.force) { + const { clean, stdout } = await provider!.worktreeIsClean(canonicalWorktreePath) + if (!clean) { + const error = new Error('Worktree has uncommitted or untracked changes.') + ;(error as Error & { stdout?: string }).stdout = stdout + throw error + } + } + + const remoteRemoveOptions = !deleteBranch ? { deleteBranch } : {} + const removalGate = await withWorktreeRemoveStageSpan( + 'watcher_gate', + 'remote', + async () => + runtime.acquireFileWatcherRemoval(canonicalWorktreePath, remoteConnectionId) + ) + let rawRemovalResult: RemoveWorktreeResult | undefined + let removalCompleted = false + try { + await withWorktreeRemoveStageSpan('pty_sweep', 'remote', async () => { + await stopPtysForDestructiveWorktreeRemoval( + runtime, + args.worktreeId, + remoteConnectionId + ) + }) + rawRemovalResult = await withWorktreeRemoveStageSpan( + 'git_remove', + 'remote', + async () => + Object.keys(remoteRemoveOptions).length > 0 + ? provider!.removeWorktree( + canonicalWorktreePath, + args.force, + remoteRemoveOptions + ) + : provider!.removeWorktree(canonicalWorktreePath, args.force) + ) + removalCompleted = true + } finally { + await removalGate.finish(removalCompleted) + } + const removalResult = preserveBranchHeadFallback( + rawRemovalResult, + registeredWorktree.head + ) + await cleanupUnusedWorktreePushTargetRemoteSsh( + provider!, + repo.path, + args.worktreeId, + removedPushTarget, + store + ) + rememberPreservedBranchCleanupTarget( + args.worktreeId, + removalResult, + registeredWorktree.head, + removedPushTarget + ) + runtime.clearOptimisticReconcileToken(args.worktreeId) + await withWorktreeRemoveStageSpan('metadata_purge', 'remote', async () => { + removeWorktreeMetadataAndTransientState(store, args.worktreeId) + }) + notifyWorktreesChanged(mainWindow, repoId) + return removalResult ?? {} + } + + const refreshedWorktrees = hasLocalWorktreeGitOptions + ? await listGitWorktreesStrict(repo.path, localWorktreeGitOptions) + : await listGitWorktreesStrict(repo.path) + const refreshedRegisteredWorktree = findRegisteredDeletableWorktree( + repo.path, canonicalWorktreePath, - repoPath: repo.path, - localWorktreeGitOptions, - registeredWorktree, - deleteBranch - }) + refreshedWorktrees + ) + if (!refreshedRegisteredWorktree) { + throw new Error( + `Worktree registration changed during deletion: ${canonicalWorktreePath}. Retry deletion.` + ) + } + try { + // Why: an archive hook can race another Git client that locks the row; recheck before linked-path/watcher/terminal teardown. + assertWorktreeUnlockedForRemoval(refreshedRegisteredWorktree) + } catch (error) { + throw new Error( + formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false) + ) + } + + // Why: `orca.yaml` shared directories are symlinked in too, and a + // directory-only ignore rule leaves those links untracked, so removal must + // tolerate and unlink them exactly like the per-user shared paths. + const linkedPaths = getWorktreeSharedLinkPaths(repo) + const ignoredLinkedPaths = args.force + ? [] + : await findExistingWorktreeSymlinkPaths(canonicalWorktreePath, linkedPaths) + try { + await (hasLocalWorktreeGitOptions + ? assertWorktreeCleanForRemoval(canonicalWorktreePath, args.force ?? false, { + ...localWorktreeGitOptions, + ...(ignoredLinkedPaths.length > 0 + ? { ignoredUntrackedPaths: ignoredLinkedPaths } + : {}) + }) + : ignoredLinkedPaths.length > 0 + ? assertWorktreeCleanForRemoval(canonicalWorktreePath, args.force ?? false, { + ignoredUntrackedPaths: ignoredLinkedPaths + }) + : assertWorktreeCleanForRemoval(canonicalWorktreePath, args.force ?? false)) + } catch (error) { + if (!isOrphanCompatiblePreflightError(error)) { + throw new Error( + formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false) + ) + } + // Why: Git can still classify this as an orphan after preflight; keep strict PTY teardown before any recursive fallback deletion. + } + + let removalResult: RemoveWorktreeResult | undefined + const removalGate = await withWorktreeRemoveStageSpan('watcher_gate', 'local', async () => + runtime.acquireFileWatcherRemoval(canonicalWorktreePath) + ) + let removalCompleted = false + try { + // Why: hold the watcher/terminal gate through Git and any recursive fallback so no late spawn recreates a native handle. + // Linked-path deletion is destructive too, so PTYs must release every handle before Windows or WSL filesystem cleanup starts. + await withWorktreeRemoveStageSpan('pty_sweep', 'local', async () => { + await stopPtysForDestructiveWorktreeRemoval(runtime, args.worktreeId) + }) + + // Why: preflight only ignored these paths, not mutated them; keep watcher installs fenced through Git removal. + if (linkedPaths.length > 0) { + await removeWorktreeLinkedPaths(canonicalWorktreePath, linkedPaths) + } + + try { + const removeOptions = { + ...(!deleteBranch ? { deleteBranch } : {}), + // Why: reuse the authoritative worktree list already computed here instead of rescanning siblings on the hot delete path. + knownRemovedWorktree: refreshedRegisteredWorktree, + ...(hasLocalWorktreeGitOptions ? localWorktreeGitOptions : {}) + } + removalResult = preserveBranchHeadFallback( + await withWorktreeRemoveStageSpan('git_remove', 'local', async () => + removeWorktree( + repo.path, + canonicalWorktreePath, + args.force ?? false, + removeOptions + ) + ), + refreshedRegisteredWorktree.head + ) + } catch (error) { + // Why: Git for Windows can deregister a clean worktree before its recursive filesystem deletion fails transiently. + const recoveredRemovalResult = await recoverLocalWindowsWorktreeRemoval({ + error, + force: args.force ?? false, + canonicalWorktreePath, + repoPath: repo.path, + localWorktreeGitOptions, + registeredWorktree: refreshedRegisteredWorktree, + deleteBranch, + closeWatcher: (worktreePath) => runtime.closeFileWatchersForRemoval(worktreePath) + }) + if (recoveredRemovalResult) { + removalResult = recoveredRemovalResult + removalCompleted = true + } else if (isOrphanedWorktreeError(error)) { + // If git no longer tracks this worktree, clean up the directory and metadata + console.warn( + `[worktrees] Orphaned worktree detected at ${canonicalWorktreePath}, cleaning up` + ) + const access = getLocalWorktreePathAccess(localWorktreeGitOptions) + if ( + await canSafelyRemoveOrphanedWorktreeDirectory( + toLocalWorktreeRuntimePath(canonicalWorktreePath, localWorktreeGitOptions), + toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), + access.statPath, + access.readPath + ) + ) { + await runtime.closeFileWatchersForRemoval(canonicalWorktreePath) + await removeLocalWorktreePath( + canonicalWorktreePath, + localWorktreeGitOptions + ).catch(() => {}) + } else { + console.warn( + `[worktrees] Refusing recursive cleanup for unproven worktree directory: ${canonicalWorktreePath}` + ) + } + // Why: remove failed so git still tracks it (.git/worktrees/); prune or the stale entry keeps its branch locked. + await gitExecFileAsync(['worktree', 'prune'], { + cwd: repo.path, + ...localWorktreeGitOptions + }).catch(() => {}) + await cleanupUnusedWorktreePushTargetRemote( + repo.path, + args.worktreeId, + removedPushTarget, + store, + localWorktreeGitOptions + ) + runtime.clearOptimisticReconcileToken(args.worktreeId) + removeWorktreeMetadataAndTransientState(store, args.worktreeId) + preservedBranchCleanupByWorktreeId.delete(args.worktreeId) + invalidateAuthorizedRootsCache() + notifyWorktreesChanged(mainWindow, repoId) + removalCompleted = true + return {} + } else { + throw new Error( + formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false) + ) + } + } + removalCompleted = true + } finally { + await removalGate.finish(removalCompleted) + } await cleanupUnusedWorktreePushTargetRemote( repo.path, args.worktreeId, @@ -2445,251 +2731,20 @@ export function registerWorktreeHandlers( rememberPreservedBranchCleanupTarget( args.worktreeId, removalResult, - registeredWorktree.head, + refreshedRegisteredWorktree.head, removedPushTarget ) runtime.clearOptimisticReconcileToken(args.worktreeId) - removeWorktreeMetadataAndTransientState(store, args.worktreeId) - invalidateAuthorizedRootsCache() + await withWorktreeRemoveStageSpan('metadata_purge', 'local', async () => { + removeWorktreeMetadataAndTransientState(store, args.worktreeId) + }) + await withWorktreeRemoveStageSpan('cache_invalidation', 'local', async () => { + invalidateAuthorizedRootsCache() + }) + notifyWorktreesChanged(mainWindow, repoId) return removalResult ?? {} - } - - // Run archive hook before removal so teardown scripts still see the worktree directory. - const hooks = await getArchiveHooksForRemoval(repo) - if (hooks?.scripts.archive && !args.skipArchive) { - const result = repo.connectionId - ? await runRemoteArchiveHook(repo, canonicalWorktreePath, hooks.scripts.archive) - : await runHook( - 'archive', - canonicalWorktreePath, - repo, - undefined, - localWorktreeGitOptions - ) - if (!result.success) { - console.error( - `[hooks] archive hook failed for ${canonicalWorktreePath}:`, - result.output - ) - } - } - - if (repo.connectionId) { - // Why: SSH deletion mirrors the local flow — hooks run while the directory is intact, then the clean check guards removal. - if (!args.force) { - const { clean, stdout } = await provider!.worktreeIsClean(canonicalWorktreePath) - if (!clean) { - const error = new Error('Worktree has uncommitted or untracked changes.') - ;(error as Error & { stdout?: string }).stdout = stdout - throw error - } - } - - const remoteRemoveOptions = !deleteBranch ? { deleteBranch } : {} - const removalGate = await runtime.acquireFileWatcherRemoval( - canonicalWorktreePath, - repo.connectionId - ) - let rawRemovalResult: RemoveWorktreeResult | undefined - let removalCompleted = false - try { - await stopPtysForDestructiveWorktreeRemoval(runtime, args.worktreeId, repo.connectionId) - rawRemovalResult = await (Object.keys(remoteRemoveOptions).length > 0 - ? provider!.removeWorktree(canonicalWorktreePath, args.force, remoteRemoveOptions) - : provider!.removeWorktree(canonicalWorktreePath, args.force)) - removalCompleted = true - } finally { - await removalGate.finish(removalCompleted) - } - const removalResult = preserveBranchHeadFallback( - rawRemovalResult, - registeredWorktree.head - ) - await cleanupUnusedWorktreePushTargetRemoteSsh( - provider!, - repo.path, - args.worktreeId, - removedPushTarget, - store - ) - rememberPreservedBranchCleanupTarget( - args.worktreeId, - removalResult, - registeredWorktree.head, - removedPushTarget - ) - runtime.clearOptimisticReconcileToken(args.worktreeId) - removeWorktreeMetadataAndTransientState(store, args.worktreeId) - notifyWorktreesChanged(mainWindow, repoId) - return removalResult ?? {} - } - - const refreshedWorktrees = hasLocalWorktreeGitOptions - ? await listGitWorktreesStrict(repo.path, localWorktreeGitOptions) - : await listGitWorktreesStrict(repo.path) - const refreshedRegisteredWorktree = findRegisteredDeletableWorktree( - repo.path, - canonicalWorktreePath, - refreshedWorktrees - ) - if (!refreshedRegisteredWorktree) { - throw new Error( - `Worktree registration changed during deletion: ${canonicalWorktreePath}. Retry deletion.` - ) - } - try { - // Why: an archive hook can race another Git client that locks the row; recheck before linked-path/watcher/terminal teardown. - assertWorktreeUnlockedForRemoval(refreshedRegisteredWorktree) - } catch (error) { - throw new Error( - formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false) - ) - } - - // Why: `orca.yaml` shared directories are symlinked in too, and a - // directory-only ignore rule leaves those links untracked, so removal must - // tolerate and unlink them exactly like the per-user shared paths. - const linkedPaths = getWorktreeSharedLinkPaths(repo) - const ignoredLinkedPaths = args.force - ? [] - : await findExistingWorktreeSymlinkPaths(canonicalWorktreePath, linkedPaths) - try { - await (hasLocalWorktreeGitOptions - ? assertWorktreeCleanForRemoval(canonicalWorktreePath, args.force ?? false, { - ...localWorktreeGitOptions, - ...(ignoredLinkedPaths.length > 0 - ? { ignoredUntrackedPaths: ignoredLinkedPaths } - : {}) - }) - : ignoredLinkedPaths.length > 0 - ? assertWorktreeCleanForRemoval(canonicalWorktreePath, args.force ?? false, { - ignoredUntrackedPaths: ignoredLinkedPaths - }) - : assertWorktreeCleanForRemoval(canonicalWorktreePath, args.force ?? false)) - } catch (error) { - if (!isOrphanCompatiblePreflightError(error)) { - throw new Error( - formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false) - ) - } - // Why: Git can still classify this as an orphan after preflight; keep strict PTY teardown before any recursive fallback deletion. - } - - let removalResult: RemoveWorktreeResult | undefined - const removalGate = await runtime.acquireFileWatcherRemoval(canonicalWorktreePath) - let removalCompleted = false - try { - // Why: preflight only ignored these paths, not mutated them; keep watcher installs fenced through Git removal. - if (linkedPaths.length > 0) { - await removeWorktreeLinkedPaths(canonicalWorktreePath, linkedPaths) - } - - // Why: hold the watcher/terminal gate through Git and any recursive fallback so no late spawn recreates a native handle. - await stopPtysForDestructiveWorktreeRemoval(runtime, args.worktreeId) - - try { - const removeOptions = { - ...(!deleteBranch ? { deleteBranch } : {}), - // Why: reuse the authoritative worktree list already computed here instead of rescanning siblings on the hot delete path. - knownRemovedWorktree: refreshedRegisteredWorktree, - ...(hasLocalWorktreeGitOptions ? localWorktreeGitOptions : {}) - } - removalResult = preserveBranchHeadFallback( - await removeWorktree( - repo.path, - canonicalWorktreePath, - args.force ?? false, - removeOptions - ), - refreshedRegisteredWorktree.head - ) - } catch (error) { - // Why: Git for Windows can deregister a clean worktree before its recursive filesystem deletion fails transiently. - const recoveredRemovalResult = await recoverLocalWindowsWorktreeRemoval({ - error, - force: args.force ?? false, - canonicalWorktreePath, - repoPath: repo.path, - localWorktreeGitOptions, - registeredWorktree: refreshedRegisteredWorktree, - deleteBranch, - closeWatcher: (worktreePath) => runtime.closeFileWatchersForRemoval(worktreePath) - }) - if (recoveredRemovalResult) { - removalResult = recoveredRemovalResult - removalCompleted = true - } else if (isOrphanedWorktreeError(error)) { - // If git no longer tracks this worktree, clean up the directory and metadata - console.warn( - `[worktrees] Orphaned worktree detected at ${canonicalWorktreePath}, cleaning up` - ) - const access = getLocalWorktreePathAccess(localWorktreeGitOptions) - if ( - await canSafelyRemoveOrphanedWorktreeDirectory( - toLocalWorktreeRuntimePath(canonicalWorktreePath, localWorktreeGitOptions), - toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), - access.statPath, - access.readPath - ) - ) { - await runtime.closeFileWatchersForRemoval(canonicalWorktreePath) - await removeLocalWorktreePath(canonicalWorktreePath, localWorktreeGitOptions).catch( - () => {} - ) - } else { - console.warn( - `[worktrees] Refusing recursive cleanup for unproven worktree directory: ${canonicalWorktreePath}` - ) - } - // Why: remove failed so git still tracks it (.git/worktrees/); prune or the stale entry keeps its branch locked. - await gitExecFileAsync(['worktree', 'prune'], { - cwd: repo.path, - ...localWorktreeGitOptions - }).catch(() => {}) - await cleanupUnusedWorktreePushTargetRemote( - repo.path, - args.worktreeId, - removedPushTarget, - store, - localWorktreeGitOptions - ) - runtime.clearOptimisticReconcileToken(args.worktreeId) - removeWorktreeMetadataAndTransientState(store, args.worktreeId) - preservedBranchCleanupByWorktreeId.delete(args.worktreeId) - invalidateAuthorizedRootsCache() - notifyWorktreesChanged(mainWindow, repoId) - removalCompleted = true - return {} - } else { - throw new Error( - formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false) - ) - } - } - removalCompleted = true - } finally { - await removalGate.finish(removalCompleted) - } - await cleanupUnusedWorktreePushTargetRemote( - repo.path, - args.worktreeId, - removedPushTarget, - store, - localWorktreeGitOptions - ) - rememberPreservedBranchCleanupTarget( - args.worktreeId, - removalResult, - refreshedRegisteredWorktree.head, - removedPushTarget - ) - runtime.clearOptimisticReconcileToken(args.worktreeId) - removeWorktreeMetadataAndTransientState(store, args.worktreeId) - invalidateAuthorizedRootsCache() - - notifyWorktreesChanged(mainWindow, repoId) - return removalResult ?? {} + }) })() worktreeRemovalsInFlight.set(inFlightKey, { optionsKey, promise: removal }) try { diff --git a/src/main/local-worktree-filesystem.ts b/src/main/local-worktree-filesystem.ts index aafb5ea93..6cf50ed1f 100644 --- a/src/main/local-worktree-filesystem.ts +++ b/src/main/local-worktree-filesystem.ts @@ -1,16 +1,16 @@ import { execFile } from 'node:child_process' -import type { RmOptions } from 'node:fs' -import { lstat, readFile, rm } from 'node:fs/promises' -import { win32 } from 'node:path' -import { setTimeout as delay } from 'node:timers/promises' +import { lstat, readFile } from 'node:fs/promises' import { buildWslLoginShellCommand, escapeWslShCommandForWindows, quotePosixShell } from '../shared/wsl-login-shell-command' +import { removeHostTree } from './host-tree-removal' import { toLinuxPath } from './wsl' import type { ReadPath, StatPath } from './worktree-orphan-gitdir-proof' +export { toHostRemovalPath } from './host-tree-removal' + export type LocalWorktreeFilesystemOptions = { wslDistro?: string } @@ -26,9 +26,6 @@ type ExecFileTextResult = { } const WSL_FILE_OPERATION_TIMEOUT_MS = 30_000 -const WINDOWS_REMOVE_RETRY_DELAYS_MS = [250, 500, 1_000, 2_000] -const WINDOWS_RM_MAX_RETRIES = 8 -const WINDOWS_RM_RETRY_DELAY_MS = 150 function shouldUseWslFilesystem(options: LocalWorktreeFilesystemOptions): boolean { return process.platform === 'win32' && !!options.wslDistro?.trim() @@ -132,7 +129,7 @@ export async function removeLocalWorktreePath( ): Promise { const distro = options.wslDistro?.trim() if (!shouldUseWslFilesystem(options) || !distro) { - await removeHostWorktreePath(targetPath) + await removeHostTree(targetPath) return } @@ -140,57 +137,3 @@ export async function removeLocalWorktreePath( // Windows cannot delete safely. Run the deletion inside the selected distro. await runWslLoginShellCommand(distro, `rm -rf -- ${quotePosixShell(toLinuxPath(targetPath))}`) } - -async function removeHostWorktreePath(targetPath: string): Promise { - const removalPath = toHostRemovalPath(targetPath) - const retryDelays = process.platform === 'win32' ? WINDOWS_REMOVE_RETRY_DELAYS_MS : [] - const rmOptions = getHostRemovalOptions() - let attempt = 0 - - while (true) { - try { - await rm(removalPath, rmOptions) - return - } catch (error) { - if (attempt >= retryDelays.length || !isTransientWindowsRemovalError(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]) - attempt += 1 - } - } -} - -function getHostRemovalOptions(): RmOptions { - const base = { recursive: true, force: true } - if (process.platform !== 'win32') { - return base - } - return { - ...base, - // Why: large Windows dependency trees commonly surface transient - // ENOTEMPTY/EPERM while Node walks and removes nested directories. - maxRetries: WINDOWS_RM_MAX_RETRIES, - retryDelay: WINDOWS_RM_RETRY_DELAY_MS - } -} - -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) -} - -export function toHostRemovalPath(targetPath: string): string { - // Why: Git for Windows can fail long recursive deletes even after Orca has - // proven the worktree target; Node's host deletion should use Win32 long paths. - return process.platform === 'win32' ? win32.toNamespacedPath(targetPath) : targetPath -} diff --git a/src/main/observability/instrumentation.ts b/src/main/observability/instrumentation.ts index b419c046e..3e944b238 100644 --- a/src/main/observability/instrumentation.ts +++ b/src/main/observability/instrumentation.ts @@ -207,6 +207,27 @@ export async function withWorktreeSpan( ) } +/** Closed set so a typo can't silently mint an orphan span name. */ +export type WorktreeRemoveStage = + | 'archive_hook' + | 'cache_invalidation' + | 'git_remove' + | 'metadata_purge' + | 'pty_sweep' + | 'watcher_gate' + +/** Wrap one stage of a worktree removal. Children share the parent's `kind` so `kind`-filtered + * views keep the whole tree, and `worktree.flow` separates the folder/remote/local removal paths. */ +export async function withWorktreeRemoveStageSpan( + stage: WorktreeRemoveStage, + flow: 'folder' | 'remote' | 'local', + fn: () => Promise +): Promise { + return withSpan(`worktree.remove.${stage}`, fn, { + attributes: { kind: 'worktree', 'worktree.flow': flow } + }) +} + export type PtySpanArgs = { readonly stage: 'spawn' | 'exit' | 'recover' readonly shell?: string diff --git a/src/main/opencode-usage/store.test.ts b/src/main/opencode-usage/store.test.ts index aea35628f..44b03eec4 100644 --- a/src/main/opencode-usage/store.test.ts +++ b/src/main/opencode-usage/store.test.ts @@ -1,12 +1,24 @@ +import { mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs' +import type * as FsPromises from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { OpenCodeUsageDailyAggregate, + OpenCodeUsagePersistedDatabase, OpenCodeUsagePersistedState, OpenCodeUsageSession } from './types' -const { getPathMock } = vi.hoisted(() => ({ - getPathMock: vi.fn(() => '/tmp/orca-test-userdata') +const { getPathMock, writeOpens, writeGate } = vi.hoisted(() => ({ + getPathMock: vi.fn(() => '/tmp/orca-test-userdata'), + // Why only mode 'w': the durable write also opens the directory read-only to fsync it, so counting + // every open would hide a regression back to multiple full-cache rewrites per scan. + writeOpens: { value: 0, inFlight: 0, maxConcurrent: 0 }, + writeGate: { + blocked: false, + waiters: [] as (() => void)[] + } })) vi.mock('electron', () => ({ @@ -15,7 +27,61 @@ vi.mock('electron', () => ({ } })) -import { OpenCodeUsageStore, normalizePersistedState } from './store' +vi.mock('node:fs/promises', async () => { + const actual = await vi.importActual('node:fs/promises') + return { + ...actual, + open: (async (...args: Parameters) => { + if (args[1] !== 'w') { + return actual.open(...args) + } + writeOpens.value += 1 + writeOpens.inFlight += 1 + writeOpens.maxConcurrent = Math.max(writeOpens.maxConcurrent, writeOpens.inFlight) + try { + if (writeGate.blocked) { + await new Promise((resolve) => writeGate.waiters.push(resolve)) + } + return await actual.open(...args) + } finally { + writeOpens.inFlight -= 1 + } + }) as typeof actual.open + } +}) + +vi.mock('./scanner', () => ({ + createWorktreeRefs: vi.fn(() => []), + scanOpenCodeUsageDatabases: vi.fn() +})) + +import { OpenCodeUsageStore, initOpenCodeUsagePath, normalizePersistedState } from './store' +import { scanOpenCodeUsageDatabases } from './scanner' + +type ScanResult = { + processedDatabases: OpenCodeUsagePersistedDatabase[] + sessions: OpenCodeUsageSession[] + dailyAggregates: OpenCodeUsageDailyAggregate[] +} + +function createDeferred(): { + promise: Promise + resolve: (value: T) => void +} { + let resolve!: (value: T) => void + const promise = new Promise((promiseResolve) => { + resolve = promiseResolve + }) + return { promise, resolve } +} + +function createEmptyScanResult(): ScanResult { + return { + processedDatabases: [], + sessions: [], + dailyAggregates: [] + } +} function getDefaultState(): OpenCodeUsagePersistedState { return { @@ -36,6 +102,7 @@ function getDefaultState(): OpenCodeUsagePersistedState { function createStoreWithState(state: Partial): OpenCodeUsageStore { const store = new OpenCodeUsageStore({ getRepos: () => [], + getAllWorktreeMeta: () => ({}), getWorktreeMeta: () => undefined } as never) @@ -140,13 +207,123 @@ function makeDaily( } describe('OpenCodeUsageStore', () => { + let tempUserData: string + beforeEach(() => { + tempUserData = mkdtempSync(join(tmpdir(), 'orca-opencode-usage-store-')) + getPathMock.mockReturnValue(tempUserData) + initOpenCodeUsagePath() + writeOpens.value = 0 + writeOpens.inFlight = 0 + writeOpens.maxConcurrent = 0 + writeGate.blocked = false + writeGate.waiters = [] + vi.mocked(scanOpenCodeUsageDatabases).mockReset() + vi.mocked(scanOpenCodeUsageDatabases).mockResolvedValue(createEmptyScanResult()) vi.useFakeTimers() vi.setSystemTime(new Date('2026-04-10T12:00:00.000-04:00')) }) afterEach(() => { vi.useRealTimers() + rmSync(tempUserData, { recursive: true, force: true }) + }) + + it('persists a successful refresh with one full-cache write', async () => { + const store = createStoreWithState({ + scanState: { + enabled: true, + lastScanStartedAt: null, + lastScanCompletedAt: null, + lastScanError: null + } + }) + + await store.refresh(true) + + // Why exactly one: a refresh that rewrites the whole multi-MB cache twice is the regression this guards. + expect(writeOpens.value).toBe(1) + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + const persistedJson = readFileSync(join(tempUserData, 'orca-opencode-usage.json'), 'utf-8') + expect(persistedJson).toContain('\n') + expect(JSON.parse(persistedJson).scanState).toMatchObject({ + enabled: true, + lastScanStartedAt: new Date('2026-04-10T12:00:00.000-04:00').getTime(), + lastScanCompletedAt: new Date('2026-04-10T12:00:00.000-04:00').getTime(), + lastScanError: null + }) + }) + + it('keeps scan start visible in memory while scan-start persistence is skipped', async () => { + const pendingScan = createDeferred() + vi.mocked(scanOpenCodeUsageDatabases).mockReturnValueOnce(pendingScan.promise) + const store = createStoreWithState({ + scanState: { + enabled: true, + lastScanStartedAt: null, + lastScanCompletedAt: null, + lastScanError: 'previous failure' + } + }) + + const refreshPromise = store.refresh(true) + await Promise.resolve() + + expect(store.getScanState()).toMatchObject({ + isScanning: true, + lastScanStartedAt: new Date('2026-04-10T12:00:00.000-04:00').getTime(), + lastScanError: null + }) + expect(writeOpens.value).toBe(0) + + pendingScan.resolve(createEmptyScanResult()) + await refreshPromise + + expect(store.getScanState().isScanning).toBe(false) + expect(writeOpens.value).toBe(1) + }) + + it('vetoes a stale concurrent async write so the newer snapshot wins without leaking tmp files', async () => { + const store = createStoreWithState({ + scanState: { + enabled: true, + lastScanStartedAt: null, + lastScanCompletedAt: null, + lastScanError: null + } + }) + const internals = store as unknown as { + writeToDisk: () => Promise + state: OpenCodeUsagePersistedState + } + + writeGate.blocked = true + const first = internals.writeToDisk() + await vi.waitFor(() => expect(writeGate.waiters.length).toBe(1)) + + internals.state.scanState.enabled = false + writeGate.blocked = false + const second = internals.writeToDisk() + writeGate.waiters.splice(0).forEach((resolve) => resolve()) + await Promise.all([first, second]) + + expect( + JSON.parse(readFileSync(join(tempUserData, 'orca-opencode-usage.json'), 'utf-8')).scanState + .enabled + ).toBe(false) + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + // Serialized, so the superseded write can be skipped safely rather than racing the newer one. + expect(writeOpens.maxConcurrent).toBe(1) + }) + + it('sweeps a usage temp file orphaned by a crash between write and rename', async () => { + const orphan = join(tempUserData, 'orca-opencode-usage.json.999.1.abc.tmp') + writeFileSync(orphan, '{}') + + createStoreWithState({}) + await vi.waitFor(() => + expect(readdirSync(tempUserData).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + ) }) it('reports no data for Orca scope when only non-Orca OpenCode usage exists', async () => { diff --git a/src/main/opencode-usage/store.ts b/src/main/opencode-usage/store.ts index b926f3265..67080d6db 100644 --- a/src/main/opencode-usage/store.ts +++ b/src/main/opencode-usage/store.ts @@ -1,7 +1,8 @@ /* eslint-disable max-lines -- Why: this store owns OpenCode analytics persistence, scan policy, and renderer query semantics. Keeping range/scope queries next to scan persistence prevents UI totals from drifting from the SQLite projection. */ import { app } from 'electron' -import { dirname, join } from 'node:path' -import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs' +import { join } from 'node:path' +import { existsSync, readFileSync } from 'node:fs' +import { UsageCacheSnapshotWriter } from '../usage-cache-snapshot-writer' import type { OpenCodeUsageBreakdownKind, OpenCodeUsageBreakdownRow, @@ -152,6 +153,9 @@ export class OpenCodeUsageStore { private state: OpenCodeUsagePersistedState private readonly store: Store private scanPromise: Promise | null = null + // Why: the multi-MB usage JSON must not block the Electron main thread; the writer serializes + // writes and vetoes superseded renames. + private readonly writer = new UsageCacheSnapshotWriter('[opencode-usage]', getOpenCodeUsageFile) constructor(store: Store) { this.store = store @@ -179,20 +183,19 @@ export class OpenCodeUsageStore { } } - private writeToDisk(): void { - const usageFile = getOpenCodeUsageFile() - const dir = dirname(usageFile) - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }) - } - const tmpFile = `${usageFile}.${process.pid}.${Date.now()}.${Math.random().toString(16).slice(2)}.tmp` - writeFileSync(tmpFile, JSON.stringify(this.state, null, 2), 'utf-8') - renameSync(tmpFile, usageFile) + private writeToDisk(): Promise { + // Pretty-print preserved: humans inspect this analytics cache on disk. + return this.writer.write(() => JSON.stringify(this.state, null, 2)) + } + + /** Await queued cache writes so quit does not drop the final snapshot. */ + flush(): Promise { + return this.writer.flush() } async setEnabled(enabled: boolean): Promise { this.state.scanState.enabled = enabled - this.writeToDisk() + await this.writeToDisk() return this.getScanState() } @@ -242,7 +245,8 @@ export class OpenCodeUsageStore { this.state.scanState.lastScanStartedAt = Date.now() this.state.scanState.lastScanError = null - this.writeToDisk() + // Why no write here: persisting scan-start would rewrite the whole cache before a single result + // changed. The completion/failure write below persists the same fields. this.scanPromise = (async () => { try { @@ -261,10 +265,12 @@ export class OpenCodeUsageStore { this.state.worktreeFingerprint = worktreeFingerprint this.state.scanState.lastScanCompletedAt = Date.now() this.state.scanState.lastScanError = null - this.writeToDisk() + // Why swallow: persistence is a cache concern. A disk failure must not turn a good scan into + // a scan error and reject refresh() for every query caller; writeToDisk already logs it. + await this.writeToDisk().catch(() => {}) } catch (error) { this.state.scanState.lastScanError = error instanceof Error ? error.message : String(error) - this.writeToDisk() + await this.writeToDisk().catch(() => {}) } finally { this.scanPromise = null } diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index b44dfc5c5..7515b63e3 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -115,6 +115,7 @@ import { RpcDispatcher } from './rpc/dispatcher' import type { RpcRequest } from './rpc/core' import { TERMINAL_METHODS } from './rpc/methods/terminal' import { beginWatcherInstall } from '../ipc/watcher-removal-gate' +import { WATCHER_REMOVAL_DRAIN_BUDGET_MS } from '../ipc/watcher-removal-drain' import { _resetTerminalViewAttributesForTest, setTerminalViewAttributes @@ -400,7 +401,7 @@ vi.mock('../git/worktree', () => ({ forceDeleteLocalBranch: forceDeleteLocalBranchMock })) -vi.mock('../terminal-history', () => ({ +vi.mock('../terminal-history-deletion', () => ({ deleteWorktreeHistoryDir: deleteWorktreeHistoryDirMock })) @@ -39223,6 +39224,60 @@ describe('OrcaRuntimeService', () => { finishRetry() }) + it('proceeds when a wedged install never releases the removal fence', async () => { + vi.useFakeTimers() + // Held across the whole acquire and never released — models a native subscribe that ignores abort + // and never settles. The removal must abandon the fence slot rather than leak it into later suites. + beginWatcherInstall(TEST_WORKTREE_PATH) + try { + const runtime = createRuntime() + + let acquired = false + const acquiring = runtime.acquireFileWatcherRemoval(TEST_WORKTREE_PATH).then((gate) => { + acquired = true + return gate + }) + await vi.advanceTimersByTimeAsync(WATCHER_REMOVAL_DRAIN_BUDGET_MS - 1) + expect(acquired).toBe(false) + + await vi.advanceTimersByTimeAsync(1) + const gate = await acquiring + expect(acquired).toBe(true) + expect(closeLocalWatcherForWorktreePathMock).toHaveBeenCalledTimes(2) + + // The fence must not stay armed: releasing the gate re-admits installs under this root. + await gate.finish(true) + const finishRetry = beginWatcherInstall(TEST_WORKTREE_PATH) + finishRetry() + } finally { + vi.useRealTimers() + } + }) + + it('does not re-spend the drain budget on a removal after a wedged install was abandoned', async () => { + vi.useFakeTimers() + beginWatcherInstall(TEST_WORKTREE_PATH) + try { + const runtime = createRuntime() + + const firstAcquiring = runtime.acquireFileWatcherRemoval(TEST_WORKTREE_PATH) + await vi.advanceTimersByTimeAsync(WATCHER_REMOVAL_DRAIN_BUDGET_MS) + await (await firstAcquiring).finish(true) + + let secondAcquired = false + const secondAcquiring = runtime.acquireFileWatcherRemoval(TEST_WORKTREE_PATH).then((gate) => { + secondAcquired = true + return gate + }) + await vi.advanceTimersByTimeAsync(1) + + expect(secondAcquired).toBe(true) + await (await secondAcquiring).finish(true) + } finally { + vi.useRealTimers() + } + }) + it('recovers forced Windows runtime long-path removal and keeps skipped-hook warnings', async () => { setPlatform('win32') const runtime = createWorktreeRemovalRuntime() @@ -39812,7 +39867,10 @@ describe('OrcaRuntimeService', () => { await expect(runtime.removeManagedWorktree(worktreeId, true)).resolves.toEqual({}) await expect(lstat(orphanPath)).rejects.toMatchObject({ code: 'ENOENT' }) - expect(closeLocalWatcherForWorktreePathMock).toHaveBeenCalledWith(orphanPath) + expect(closeLocalWatcherForWorktreePathMock).toHaveBeenCalledWith( + orphanPath, + expect.objectContaining({ remainingMs: expect.any(Function) }) + ) expect(removeWorktree).not.toHaveBeenCalled() expect(removeWorktreeMeta).toHaveBeenCalledWith(worktreeId) expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(worktreeId) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 9cee18d16..90eeb149f 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -844,7 +844,7 @@ import { getWorktreeSharedLinkPaths, resolveWorktreeSharedDirectories } from '../git/worktree-shared-directories' -import { deleteWorktreeHistoryDir } from '../terminal-history' +import { deleteWorktreeHistoryDir } from '../terminal-history-deletion' import { cleanupUnusedWorktreePushTargetRemote, cleanupUnusedWorktreePushTargetRemoteSsh, @@ -901,6 +901,12 @@ import { restoreRemoteWatcherAfterFailedRemoval } from '../ipc/filesystem-watcher' import { acquireWatcherRemovalGate } from '../ipc/watcher-removal-gate' +import { + createWatcherRemovalDeadline, + drainBeforeWatcherRemoval, + type WatcherRemovalDeadline +} from '../ipc/watcher-removal-drain' +import { withWorktreeSpan } from '../observability/instrumentation' import { HeadlessEmulator } from '../daemon/headless-emulator' import { isNativeWindowsConptyPty, @@ -8035,13 +8041,24 @@ export class OrcaRuntimeService { this.fileCommands.watchFileExplorer.bind(this.fileCommands) closeFileWatchersForRemoval = async ( worktreePath: string, - connectionId?: string + connectionId?: string, + deadline: WatcherRemovalDeadline = createWatcherRemovalDeadline() ): Promise => { + // Why drain the remote/explorer closes: they await SSH round trips and lease suspends that a dead + // link never answers. The local close bounds its own awaits against the same deadline internally. const results = await Promise.allSettled([ connectionId - ? closeRemoteWatcherForWorktreePath(connectionId, worktreePath) - : closeLocalWatcherForWorktreePath(worktreePath), - this.fileCommands.closeFileExplorerWatchersForPath(worktreePath, connectionId) + ? drainBeforeWatcherRemoval( + closeRemoteWatcherForWorktreePath(connectionId, worktreePath), + deadline, + `remote watcher close for ${worktreePath}` + ) + : closeLocalWatcherForWorktreePath(worktreePath, deadline), + drainBeforeWatcherRemoval( + this.fileCommands.closeFileExplorerWatchersForPath(worktreePath, connectionId), + deadline, + `file explorer watcher close for ${worktreePath}` + ) ]) const failure = results.find((result): result is PromiseRejectedResult => { return result.status === 'rejected' @@ -8076,12 +8093,25 @@ export class OrcaRuntimeService { connectionId?: string ): Promise<{ finish(removed: boolean): Promise }> => { const gate = acquireWatcherRemovalGate(worktreePath, connectionId) + // Why: one budget for the whole preparation — independent per-await timeouts would compose into minutes. + const deadline = createWatcherRemovalDeadline() try { // Why: the first pass aborts desktop setup immediately; the second catches // any pre-gate runtime install that published after the first snapshot. - await this.closeFileWatchersForRemoval(worktreePath, connectionId) - await gate.ready - await this.closeFileWatchersForRemoval(worktreePath, connectionId) + await this.closeFileWatchersForRemoval(worktreePath, connectionId, deadline) + // Why: a wedged install never releases its fence slot, so gate.ready can hang forever; the delete + // must proceed instead, or the gate stays held and every later install under this root is rejected. + const fenceDrain = await drainBeforeWatcherRemoval( + gate.ready, + deadline, + `watcher install fence for ${worktreePath}` + ) + if (fenceDrain === 'timeout') { + // Why: a wedged install holds its fence slot for the process lifetime, so leaving it counted + // makes every later removal of this root burn the whole drain budget again. + gate.abandonPendingInstalls() + } + await this.closeFileWatchersForRemoval(worktreePath, connectionId, deadline) let finished = false return { finish: async (removed) => { @@ -22525,173 +22555,133 @@ export class OrcaRuntimeService { // Why: runtime callers can race the same workspace through CLI/mobile // retries. Share one destructive Git/filesystem operation per worktree ID. const removal = (async (): Promise => { - const repo = store.getRepo(removalTarget.repoId) - if (!repo) { - throw new Error('repo_not_found') - } - if (isFolderRepo(repo)) { - if (removalTarget.id === getRuntimeFolderWorkspaceRootId(repo)) { - throw new Error( - 'Cannot delete the project root workspace. Remove the folder project instead.' - ) + // Why: CLI, mobile and headless serve delete through here rather than the IPC handler; without + // this span their freezes are as invisible as desktop deletes were before `worktree.remove`. + return withWorktreeSpan({ stage: 'remove', path: removalTarget.path }, async () => { + const repo = store.getRepo(removalTarget.repoId) + if (!repo) { + throw new Error('repo_not_found') } - const localProvider = this.getLocalProvider() - if (localProvider) { - // Why: folder workspace deletion has no Git removal phase where PTYs - // would otherwise be swept; tear them down before hiding the workspace. - await killAllProcessesForWorktree(removalTarget.id, { - runtime: this, - localProvider, - onPtyStopped: this.onPtyStopped ?? undefined - }).catch((err) => { - console.warn(`[worktree-teardown] failed for ${removalTarget.id}:`, err) - }) - } - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) - this.preservedBranchCleanupByWorktreeId.delete(removalTarget.id) - this.invalidateResolvedWorktreeCache() - this.notifyWorktreesChanged(repo.id) - return {} - } - const provider = repo.connectionId ? requireSshGitProvider(repo.connectionId) : null - const fsProvider = repo.connectionId ? getSshFilesystemProvider(repo.connectionId) : null - const localWorktreeGitOptions = repo.connectionId - ? {} - : getLocalProjectWorktreeGitOptions(this.requireStore(), repo) - const hasLocalWorktreeGitOptions = Object.keys(localWorktreeGitOptions).length > 0 - const registeredWorktrees = repo.connectionId - ? await provider!.listWorktrees(repo.path) - : hasLocalWorktreeGitOptions - ? await listWorktreesStrict(repo.path, localWorktreeGitOptions) - : await listWorktreesStrict(repo.path) - const removedMeta = store.getWorktreeMeta(removalTarget.id) - const removedPushTarget = removedMeta?.pushTarget ?? removalTarget.pushTarget - const registeredWorktree = findRegisteredDeletableWorktree( - repo.path, - removalTarget.path, - registeredWorktrees - ) - if (!registeredWorktree) { - let canCleanOrphanedDirectory = false - if ( - canCleanupUnregisteredOrcaWorktreeDirectory({ - meta: removedMeta - }) - ) { - if (repo.connectionId) { - if (!fsProvider) { - throw new Error('SSH filesystem provider unavailable') - } - if (!fsProvider.lstat) { - throw new Error('SSH filesystem provider lstat unavailable') - } - canCleanOrphanedDirectory = await canSafelyRemoveOrphanedWorktreeDirectory( - removalTarget.path, - repo.path, - (path) => fsProvider.lstat!(path), - (path) => fsProvider.readFile(path) - ) - } else { - const access = getLocalWorktreePathAccess(localWorktreeGitOptions) - canCleanOrphanedDirectory = - !isDangerousWorktreeRemovalPath(removalTarget.path, repo.path) && - (await canSafelyRemoveOrphanedWorktreeDirectory( - toLocalWorktreeRuntimePath(removalTarget.path, localWorktreeGitOptions), - toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), - access.statPath, - access.readPath - )) - } - } - if (canCleanOrphanedDirectory) { - assertWorktreeDoesNotContainRegisteredWorktree(removalTarget.path, registeredWorktrees) - if (!force) { - throw new Error(ORPHANED_WORKTREE_DIRECTORY_MESSAGE) - } - if (repo.connectionId) { - const removalGate = await this.acquireFileWatcherRemoval( - removalTarget.path, - repo.connectionId - ) - let removalCompleted = false - try { - await this.stopPtysForDestructiveWorktreeRemoval(removalTarget.id, repo.connectionId) - await fsProvider!.deletePath(removalTarget.path, true) - removalCompleted = true - } finally { - await removalGate.finish(removalCompleted) - } - await cleanupUnusedWorktreePushTargetRemoteSsh( - provider!, - repo.path, - removalTarget.id, - removedPushTarget, - store - ) - } else { - const removalGate = await this.acquireFileWatcherRemoval(removalTarget.path) - let removalCompleted = false - try { - await this.stopPtysForDestructiveWorktreeRemoval(removalTarget.id) - await removeLocalWorktreePath(removalTarget.path, localWorktreeGitOptions) - removalCompleted = true - } finally { - await removalGate.finish(removalCompleted) - } - await cleanupUnusedWorktreePushTargetRemote( - repo.path, - removalTarget.id, - removedPushTarget, - store, - localWorktreeGitOptions + if (isFolderRepo(repo)) { + if (removalTarget.id === getRuntimeFolderWorkspaceRootId(repo)) { + throw new Error( + 'Cannot delete the project root workspace. Remove the folder project instead.' ) } - this.clearOptimisticReconcileToken(removalTarget.id) + const localProvider = this.getLocalProvider() + if (localProvider) { + // Why: folder workspace deletion has no Git removal phase where PTYs + // would otherwise be swept; tear them down before hiding the workspace. + await killAllProcessesForWorktree(removalTarget.id, { + runtime: this, + localProvider, + onPtyStopped: this.onPtyStopped ?? undefined + }).catch((err) => { + console.warn(`[worktree-teardown] failed for ${removalTarget.id}:`, err) + }) + } this.removeWorktreeMetadataAndHistory(store, removalTarget.id) this.preservedBranchCleanupByWorktreeId.delete(removalTarget.id) this.invalidateResolvedWorktreeCache() - this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) - invalidateAuthorizedRootsCache() this.notifyWorktreesChanged(repo.id) return {} } - if (!repo.connectionId) { - const access = getLocalWorktreePathAccess(localWorktreeGitOptions) - const runtimeWorktreePath = toLocalWorktreeRuntimePath( - removalTarget.path, - localWorktreeGitOptions - ) + const provider = repo.connectionId ? requireSshGitProvider(repo.connectionId) : null + const fsProvider = repo.connectionId ? getSshFilesystemProvider(repo.connectionId) : null + const localWorktreeGitOptions = repo.connectionId + ? {} + : getLocalProjectWorktreeGitOptions(this.requireStore(), repo) + const hasLocalWorktreeGitOptions = Object.keys(localWorktreeGitOptions).length > 0 + const registeredWorktrees = repo.connectionId + ? await provider!.listWorktrees(repo.path) + : hasLocalWorktreeGitOptions + ? await listWorktreesStrict(repo.path, localWorktreeGitOptions) + : await listWorktreesStrict(repo.path) + const removedMeta = store.getWorktreeMeta(removalTarget.id) + const removedPushTarget = removedMeta?.pushTarget ?? removalTarget.pushTarget + const registeredWorktree = findRegisteredDeletableWorktree( + repo.path, + removalTarget.path, + registeredWorktrees + ) + if (!registeredWorktree) { + let canCleanOrphanedDirectory = false if ( - await canCleanupUnregisteredOrcaLeftoverDirectory({ - meta: removedMeta, - worktreePath: removalTarget.path, - runtimeWorktreePath, - repo, - runtimeRepoPath: toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), - registeredWorktrees, - statPath: access.statPath, - isGitRepository: (path) => isLocalRuntimeGitRepository(path, localWorktreeGitOptions) + canCleanupUnregisteredOrcaWorktreeDirectory({ + meta: removedMeta }) ) { + if (repo.connectionId) { + if (!fsProvider) { + throw new Error('SSH filesystem provider unavailable') + } + if (!fsProvider.lstat) { + throw new Error('SSH filesystem provider lstat unavailable') + } + canCleanOrphanedDirectory = await canSafelyRemoveOrphanedWorktreeDirectory( + removalTarget.path, + repo.path, + (path) => fsProvider.lstat!(path), + (path) => fsProvider.readFile(path) + ) + } else { + const access = getLocalWorktreePathAccess(localWorktreeGitOptions) + canCleanOrphanedDirectory = + !isDangerousWorktreeRemovalPath(removalTarget.path, repo.path) && + (await canSafelyRemoveOrphanedWorktreeDirectory( + toLocalWorktreeRuntimePath(removalTarget.path, localWorktreeGitOptions), + toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), + access.statPath, + access.readPath + )) + } + } + if (canCleanOrphanedDirectory) { + assertWorktreeDoesNotContainRegisteredWorktree(removalTarget.path, registeredWorktrees) if (!force) { throw new Error(ORPHANED_WORKTREE_DIRECTORY_MESSAGE) } - const removalGate = await this.acquireFileWatcherRemoval(removalTarget.path) - let removalCompleted = false - try { - await this.stopPtysForDestructiveWorktreeRemoval(removalTarget.id) - await removeLocalWorktreePath(removalTarget.path, localWorktreeGitOptions) - removalCompleted = true - } finally { - await removalGate.finish(removalCompleted) + if (repo.connectionId) { + const removalGate = await this.acquireFileWatcherRemoval( + removalTarget.path, + repo.connectionId + ) + let removalCompleted = false + try { + await this.stopPtysForDestructiveWorktreeRemoval( + removalTarget.id, + repo.connectionId + ) + await fsProvider!.deletePath(removalTarget.path, true) + removalCompleted = true + } finally { + await removalGate.finish(removalCompleted) + } + await cleanupUnusedWorktreePushTargetRemoteSsh( + provider!, + repo.path, + removalTarget.id, + removedPushTarget, + store + ) + } else { + const removalGate = await this.acquireFileWatcherRemoval(removalTarget.path) + let removalCompleted = false + try { + await this.stopPtysForDestructiveWorktreeRemoval(removalTarget.id) + await removeLocalWorktreePath(removalTarget.path, localWorktreeGitOptions) + removalCompleted = true + } finally { + await removalGate.finish(removalCompleted) + } + await cleanupUnusedWorktreePushTargetRemote( + repo.path, + removalTarget.id, + removedPushTarget, + store, + localWorktreeGitOptions + ) } - await cleanupUnusedWorktreePushTargetRemote( - repo.path, - removalTarget.id, - removedPushTarget, - store, - localWorktreeGitOptions - ) this.clearOptimisticReconcileToken(removalTarget.id) this.removeWorktreeMetadataAndHistory(store, removalTarget.id) this.preservedBranchCleanupByWorktreeId.delete(removalTarget.id) @@ -22701,70 +22691,349 @@ export class OrcaRuntimeService { this.notifyWorktreesChanged(repo.id) return {} } - } - if (await isRuntimeWorktreePathMissing(repo, removalTarget.path, localWorktreeGitOptions)) { - if (!force && !removedMeta) { - // Why: without persisted metadata, require the renderer recovery - // path before deleting Orca-only state for an unregistered path. - throw new Error(UNREGISTERED_MISSING_WORKTREE_MESSAGE) - } - // Why: a manually deleted worktree is already gone from Git and disk. - // Finish runtime metadata cleanup without requiring force or touching - // any unregistered path that still exists. - await (repo.connectionId - ? cleanupUnusedWorktreePushTargetRemoteSsh( - provider!, - repo.path, - removalTarget.id, - removedPushTarget, - store - ) - : cleanupUnusedWorktreePushTargetRemote( + if (!repo.connectionId) { + const access = getLocalWorktreePathAccess(localWorktreeGitOptions) + const runtimeWorktreePath = toLocalWorktreeRuntimePath( + removalTarget.path, + localWorktreeGitOptions + ) + if ( + await canCleanupUnregisteredOrcaLeftoverDirectory({ + meta: removedMeta, + worktreePath: removalTarget.path, + runtimeWorktreePath, + repo, + runtimeRepoPath: toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), + registeredWorktrees, + statPath: access.statPath, + isGitRepository: (path) => + isLocalRuntimeGitRepository(path, localWorktreeGitOptions) + }) + ) { + if (!force) { + throw new Error(ORPHANED_WORKTREE_DIRECTORY_MESSAGE) + } + const removalGate = await this.acquireFileWatcherRemoval(removalTarget.path) + let removalCompleted = false + try { + await this.stopPtysForDestructiveWorktreeRemoval(removalTarget.id) + await removeLocalWorktreePath(removalTarget.path, localWorktreeGitOptions) + removalCompleted = true + } finally { + await removalGate.finish(removalCompleted) + } + await cleanupUnusedWorktreePushTargetRemote( repo.path, removalTarget.id, removedPushTarget, store, localWorktreeGitOptions - )) + ) + this.clearOptimisticReconcileToken(removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.preservedBranchCleanupByWorktreeId.delete(removalTarget.id) + this.invalidateResolvedWorktreeCache() + this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) + invalidateAuthorizedRootsCache() + this.notifyWorktreesChanged(repo.id) + return {} + } + } + if ( + await isRuntimeWorktreePathMissing(repo, removalTarget.path, localWorktreeGitOptions) + ) { + if (!force && !removedMeta) { + // Why: without persisted metadata, require the renderer recovery + // path before deleting Orca-only state for an unregistered path. + throw new Error(UNREGISTERED_MISSING_WORKTREE_MESSAGE) + } + // Why: a manually deleted worktree is already gone from Git and disk. + // Finish runtime metadata cleanup without requiring force or touching + // any unregistered path that still exists. + await (repo.connectionId + ? cleanupUnusedWorktreePushTargetRemoteSsh( + provider!, + repo.path, + removalTarget.id, + removedPushTarget, + store + ) + : cleanupUnusedWorktreePushTargetRemote( + repo.path, + removalTarget.id, + removedPushTarget, + store, + localWorktreeGitOptions + )) + this.clearOptimisticReconcileToken(removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.preservedBranchCleanupByWorktreeId.delete(removalTarget.id) + this.invalidateResolvedWorktreeCache() + this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) + invalidateAuthorizedRootsCache() + this.notifyWorktreesChanged(repo.id) + return {} + } + throw new Error(`Refusing to delete unregistered worktree path: ${removalTarget.path}`) + } + const canonicalWorktreePath = registeredWorktree.path + const deleteBranch = removedMeta?.preserveBranchOnDelete !== true + + // Why: a Git lock must block before archive hooks or linked-path cleanup + // mutate the workspace; dirty-file force is a separate permission. + try { + assertWorktreeUnlockedForRemoval(registeredWorktree) + } catch (error) { + throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, force)) + } + + // Why: a prior forced Windows recovery can delete the directory but leave + // Git's stale registration; recover and verify it before clearing metadata. + if ( + !repo.connectionId && + force === true && + process.platform === 'win32' && + (isWindowsAbsolutePathLike(canonicalWorktreePath) || + !!localWorktreeGitOptions.wslDistro) && + removedMeta && + (await isRuntimeWorktreePathMissing(repo, canonicalWorktreePath, localWorktreeGitOptions)) + ) { + const removalResult = await removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({ + canonicalWorktreePath, + repoPath: repo.path, + localWorktreeGitOptions, + registeredWorktree, + deleteBranch + }) + await cleanupUnusedWorktreePushTargetRemote( + repo.path, + removalTarget.id, + removedPushTarget, + store, + localWorktreeGitOptions + ) + this.rememberPreservedBranchCleanupTarget( + removalTarget.id, + removalResult, + registeredWorktree.head, + removedPushTarget + ) this.clearOptimisticReconcileToken(removalTarget.id) this.removeWorktreeMetadataAndHistory(store, removalTarget.id) - this.preservedBranchCleanupByWorktreeId.delete(removalTarget.id) this.invalidateResolvedWorktreeCache() this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) invalidateAuthorizedRootsCache() this.notifyWorktreesChanged(repo.id) - return {} + return removalResult ?? {} + } + if (repo.connectionId) { + const remoteRemoveOptions = !deleteBranch ? { deleteBranch } : {} + const removalGate = await this.acquireFileWatcherRemoval( + canonicalWorktreePath, + repo.connectionId + ) + let rawRemovalResult: RemoveWorktreeResult | undefined + let removalCompleted = false + try { + await this.stopPtysForDestructiveWorktreeRemoval(removalTarget.id, repo.connectionId) + rawRemovalResult = await (Object.keys(remoteRemoveOptions).length > 0 + ? provider!.removeWorktree(canonicalWorktreePath, force, remoteRemoveOptions) + : provider!.removeWorktree(canonicalWorktreePath, force)) + removalCompleted = true + } finally { + await removalGate.finish(removalCompleted) + } + const removalResult = this.preserveBranchHeadFallback( + rawRemovalResult, + registeredWorktree.head + ) + await cleanupUnusedWorktreePushTargetRemoteSsh( + provider!, + repo.path, + removalTarget.id, + removedPushTarget, + store + ) + this.rememberPreservedBranchCleanupTarget( + removalTarget.id, + removalResult, + registeredWorktree.head, + removedPushTarget + ) + this.clearOptimisticReconcileToken(removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.invalidateResolvedWorktreeCache() + this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) + invalidateAuthorizedRootsCache() + this.notifyWorktreesChanged(repo.id) + return removalResult ?? {} } - throw new Error(`Refusing to delete unregistered worktree path: ${removalTarget.path}`) - } - const canonicalWorktreePath = registeredWorktree.path - const deleteBranch = removedMeta?.preserveBranchOnDelete !== true - // Why: a Git lock must block before archive hooks or linked-path cleanup - // mutate the workspace; dirty-file force is a separate permission. - try { - assertWorktreeUnlockedForRemoval(registeredWorktree) - } catch (error) { - throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, force)) - } + const hooks = getEffectiveHooks(repo) + let warning: string | undefined + if (hooks?.scripts.archive && runHooks) { + const result = await runHook( + 'archive', + canonicalWorktreePath, + repo, + undefined, + hasLocalWorktreeGitOptions ? localWorktreeGitOptions : undefined + ) + if (!result.success) { + console.error( + `[hooks] archive hook failed for ${canonicalWorktreePath}:`, + result.output + ) + } + } else if (hooks?.scripts.archive) { + // Runtime RPC calls have no renderer trust prompt, so hooks require explicit CLI opt-in. + warning = `orca.yaml archive hook skipped for ${canonicalWorktreePath}; pass --run-hooks to run it.` + console.warn(`[hooks] ${warning}`) + } - // Why: a prior forced Windows recovery can delete the directory but leave - // Git's stale registration; recover and verify it before clearing metadata. - if ( - !repo.connectionId && - force === true && - process.platform === 'win32' && - (isWindowsAbsolutePathLike(canonicalWorktreePath) || !!localWorktreeGitOptions.wslDistro) && - removedMeta && - (await isRuntimeWorktreePathMissing(repo, canonicalWorktreePath, localWorktreeGitOptions)) - ) { - const removalResult = await removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({ + const refreshedWorktrees = hasLocalWorktreeGitOptions + ? await listWorktreesStrict(repo.path, localWorktreeGitOptions) + : await listWorktreesStrict(repo.path) + const refreshedRegisteredWorktree = findRegisteredDeletableWorktree( + repo.path, canonicalWorktreePath, - repoPath: repo.path, - localWorktreeGitOptions, - registeredWorktree, - deleteBranch - }) + refreshedWorktrees + ) + if (!refreshedRegisteredWorktree) { + throw new Error( + `Worktree registration changed during deletion: ${canonicalWorktreePath}. Retry deletion.` + ) + } + try { + // Why: an archive hook can race another Git client that locks the row; + // recheck before linked-path, watcher, or terminal teardown side effects. + assertWorktreeUnlockedForRemoval(refreshedRegisteredWorktree) + } catch (error) { + throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, force)) + } + + // Why: `orca.yaml` shared directories are symlinked in too, and a + // directory-only ignore rule leaves those links untracked, so removal must + // tolerate and unlink them exactly like the per-user shared paths. + const linkedPaths = getWorktreeSharedLinkPaths(repo) + const ignoredLinkedPaths = force + ? [] + : await findExistingWorktreeSymlinkPaths(canonicalWorktreePath, linkedPaths) + try { + await (hasLocalWorktreeGitOptions + ? assertWorktreeCleanForRemoval(canonicalWorktreePath, force, { + ...localWorktreeGitOptions, + ...(ignoredLinkedPaths.length > 0 + ? { ignoredUntrackedPaths: ignoredLinkedPaths } + : {}) + }) + : ignoredLinkedPaths.length > 0 + ? assertWorktreeCleanForRemoval(canonicalWorktreePath, force, { + ignoredUntrackedPaths: ignoredLinkedPaths + }) + : assertWorktreeCleanForRemoval(canonicalWorktreePath, force)) + } catch (error) { + if (!isOrphanCompatiblePreflightError(error)) { + throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, force)) + } + // Why: Git can still classify this as an orphan after preflight; + // retain strict PTY teardown before any recursive fallback deletion. + } + + let removalResult: RemoveWorktreeResult | undefined + const removalGate = await this.acquireFileWatcherRemoval(canonicalWorktreePath) + let removalCompleted = false + try { + // Why: linked-path deletion is destructive too; PTYs must release every + // handle before Windows or WSL filesystem cleanup starts. + await this.stopPtysForDestructiveWorktreeRemoval(removalTarget.id) + + if (linkedPaths.length > 0) { + await removeWorktreeLinkedPaths(canonicalWorktreePath, linkedPaths) + } + + try { + const removeOptions = { + ...(!deleteBranch ? { deleteBranch } : {}), + // Why: removal already validated the Git row under the selected + // project runtime; keep branch cleanup on that same canonical row. + knownRemovedWorktree: refreshedRegisteredWorktree, + ...localWorktreeGitOptions + } + removalResult = this.preserveBranchHeadFallback( + await removeWorktree(repo.path, canonicalWorktreePath, force, removeOptions), + refreshedRegisteredWorktree.head + ) + } catch (error) { + // Why: Git for Windows can deregister a clean worktree before its + // recursive filesystem deletion fails transiently. + const recoveredRemovalResult = await recoverLocalWindowsWorktreeRemoval({ + error, + force, + canonicalWorktreePath, + repoPath: repo.path, + localWorktreeGitOptions, + registeredWorktree: refreshedRegisteredWorktree, + deleteBranch, + closeWatcher: (worktreePath) => this.closeFileWatchersForRemoval(worktreePath) + }) + if (recoveredRemovalResult) { + removalResult = recoveredRemovalResult + removalCompleted = true + } else if (isOrphanedWorktreeError(error)) { + const access = getLocalWorktreePathAccess(localWorktreeGitOptions) + if ( + await canSafelyRemoveOrphanedWorktreeDirectory( + toLocalWorktreeRuntimePath(canonicalWorktreePath, localWorktreeGitOptions), + toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), + access.statPath, + access.readPath + ) + ) { + await this.closeFileWatchersForRemoval(canonicalWorktreePath) + await removeLocalWorktreePath(canonicalWorktreePath, localWorktreeGitOptions).catch( + () => {} + ) + } else { + console.warn( + `[worktrees] Refusing recursive cleanup for unproven worktree directory: ${canonicalWorktreePath}` + ) + } + // Why: `git worktree remove` failed, so git's internal worktree tracking + // (`.git/worktrees/`) is still intact. Without pruning, `git worktree + // list` continues to show the stale entry and the branch it had checked out + // remains locked — other worktrees cannot check it out. + await gitExecFileAsync(['worktree', 'prune'], { + cwd: repo.path, + ...localWorktreeGitOptions + }).catch(() => {}) + await cleanupUnusedWorktreePushTargetRemote( + repo.path, + removalTarget.id, + removedPushTarget, + store, + localWorktreeGitOptions + ) + this.clearOptimisticReconcileToken(removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.preservedBranchCleanupByWorktreeId.delete(removalTarget.id) + this.invalidateResolvedWorktreeCache() + this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) + invalidateAuthorizedRootsCache() + this.notifyWorktreesChanged(repo.id) + removalCompleted = true + return { + ...(warning ? { warning } : {}) + } + } else { + throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, force)) + } + } + removalCompleted = true + } finally { + await removalGate.finish(removalCompleted) + } + await cleanupUnusedWorktreePushTargetRemote( repo.path, removalTarget.id, @@ -22775,7 +23044,7 @@ export class OrcaRuntimeService { this.rememberPreservedBranchCleanupTarget( removalTarget.id, removalResult, - registeredWorktree.head, + refreshedRegisteredWorktree.head, removedPushTarget ) this.clearOptimisticReconcileToken(removalTarget.id) @@ -22784,236 +23053,11 @@ export class OrcaRuntimeService { this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) invalidateAuthorizedRootsCache() this.notifyWorktreesChanged(repo.id) - return removalResult ?? {} - } - if (repo.connectionId) { - const remoteRemoveOptions = !deleteBranch ? { deleteBranch } : {} - const removalGate = await this.acquireFileWatcherRemoval( - canonicalWorktreePath, - repo.connectionId - ) - let rawRemovalResult: RemoveWorktreeResult | undefined - let removalCompleted = false - try { - await this.stopPtysForDestructiveWorktreeRemoval(removalTarget.id, repo.connectionId) - rawRemovalResult = await (Object.keys(remoteRemoveOptions).length > 0 - ? provider!.removeWorktree(canonicalWorktreePath, force, remoteRemoveOptions) - : provider!.removeWorktree(canonicalWorktreePath, force)) - removalCompleted = true - } finally { - await removalGate.finish(removalCompleted) + return { + ...removalResult, + ...(warning ? { warning } : {}) } - const removalResult = this.preserveBranchHeadFallback( - rawRemovalResult, - registeredWorktree.head - ) - await cleanupUnusedWorktreePushTargetRemoteSsh( - provider!, - repo.path, - removalTarget.id, - removedPushTarget, - store - ) - this.rememberPreservedBranchCleanupTarget( - removalTarget.id, - removalResult, - registeredWorktree.head, - removedPushTarget - ) - this.clearOptimisticReconcileToken(removalTarget.id) - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) - this.invalidateResolvedWorktreeCache() - this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) - invalidateAuthorizedRootsCache() - this.notifyWorktreesChanged(repo.id) - return removalResult ?? {} - } - - const hooks = getEffectiveHooks(repo) - let warning: string | undefined - if (hooks?.scripts.archive && runHooks) { - const result = await runHook( - 'archive', - canonicalWorktreePath, - repo, - undefined, - hasLocalWorktreeGitOptions ? localWorktreeGitOptions : undefined - ) - if (!result.success) { - console.error(`[hooks] archive hook failed for ${canonicalWorktreePath}:`, result.output) - } - } else if (hooks?.scripts.archive) { - // Runtime RPC calls have no renderer trust prompt, so hooks require explicit CLI opt-in. - warning = `orca.yaml archive hook skipped for ${canonicalWorktreePath}; pass --run-hooks to run it.` - console.warn(`[hooks] ${warning}`) - } - - const refreshedWorktrees = hasLocalWorktreeGitOptions - ? await listWorktreesStrict(repo.path, localWorktreeGitOptions) - : await listWorktreesStrict(repo.path) - const refreshedRegisteredWorktree = findRegisteredDeletableWorktree( - repo.path, - canonicalWorktreePath, - refreshedWorktrees - ) - if (!refreshedRegisteredWorktree) { - throw new Error( - `Worktree registration changed during deletion: ${canonicalWorktreePath}. Retry deletion.` - ) - } - try { - // Why: an archive hook can race another Git client that locks the row; - // recheck before linked-path, watcher, or terminal teardown side effects. - assertWorktreeUnlockedForRemoval(refreshedRegisteredWorktree) - } catch (error) { - throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, force)) - } - - // Why: `orca.yaml` shared directories are symlinked in too, and a - // directory-only ignore rule leaves those links untracked, so removal must - // tolerate and unlink them exactly like the per-user shared paths. - const linkedPaths = getWorktreeSharedLinkPaths(repo) - const ignoredLinkedPaths = force - ? [] - : await findExistingWorktreeSymlinkPaths(canonicalWorktreePath, linkedPaths) - try { - await (hasLocalWorktreeGitOptions - ? assertWorktreeCleanForRemoval(canonicalWorktreePath, force, { - ...localWorktreeGitOptions, - ...(ignoredLinkedPaths.length > 0 - ? { ignoredUntrackedPaths: ignoredLinkedPaths } - : {}) - }) - : ignoredLinkedPaths.length > 0 - ? assertWorktreeCleanForRemoval(canonicalWorktreePath, force, { - ignoredUntrackedPaths: ignoredLinkedPaths - }) - : assertWorktreeCleanForRemoval(canonicalWorktreePath, force)) - } catch (error) { - if (!isOrphanCompatiblePreflightError(error)) { - throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, force)) - } - // Why: Git can still classify this as an orphan after preflight; - // retain strict PTY teardown before any recursive fallback deletion. - } - - let removalResult: RemoveWorktreeResult | undefined - const removalGate = await this.acquireFileWatcherRemoval(canonicalWorktreePath) - let removalCompleted = false - try { - // Why: linked-path deletion is destructive too; PTYs must release every - // handle before Windows or WSL filesystem cleanup starts. - await this.stopPtysForDestructiveWorktreeRemoval(removalTarget.id) - - if (linkedPaths.length > 0) { - await removeWorktreeLinkedPaths(canonicalWorktreePath, linkedPaths) - } - - try { - const removeOptions = { - ...(!deleteBranch ? { deleteBranch } : {}), - // Why: removal already validated the Git row under the selected - // project runtime; keep branch cleanup on that same canonical row. - knownRemovedWorktree: refreshedRegisteredWorktree, - ...localWorktreeGitOptions - } - removalResult = this.preserveBranchHeadFallback( - await removeWorktree(repo.path, canonicalWorktreePath, force, removeOptions), - refreshedRegisteredWorktree.head - ) - } catch (error) { - // Why: Git for Windows can deregister a clean worktree before its - // recursive filesystem deletion fails transiently. - const recoveredRemovalResult = await recoverLocalWindowsWorktreeRemoval({ - error, - force, - canonicalWorktreePath, - repoPath: repo.path, - localWorktreeGitOptions, - registeredWorktree: refreshedRegisteredWorktree, - deleteBranch, - closeWatcher: (worktreePath) => this.closeFileWatchersForRemoval(worktreePath) - }) - if (recoveredRemovalResult) { - removalResult = recoveredRemovalResult - removalCompleted = true - } else if (isOrphanedWorktreeError(error)) { - const access = getLocalWorktreePathAccess(localWorktreeGitOptions) - if ( - await canSafelyRemoveOrphanedWorktreeDirectory( - toLocalWorktreeRuntimePath(canonicalWorktreePath, localWorktreeGitOptions), - toLocalWorktreeRuntimePath(repo.path, localWorktreeGitOptions), - access.statPath, - access.readPath - ) - ) { - await this.closeFileWatchersForRemoval(canonicalWorktreePath) - await removeLocalWorktreePath(canonicalWorktreePath, localWorktreeGitOptions).catch( - () => {} - ) - } else { - console.warn( - `[worktrees] Refusing recursive cleanup for unproven worktree directory: ${canonicalWorktreePath}` - ) - } - // Why: `git worktree remove` failed, so git's internal worktree tracking - // (`.git/worktrees/`) is still intact. Without pruning, `git worktree - // list` continues to show the stale entry and the branch it had checked out - // remains locked — other worktrees cannot check it out. - await gitExecFileAsync(['worktree', 'prune'], { - cwd: repo.path, - ...localWorktreeGitOptions - }).catch(() => {}) - await cleanupUnusedWorktreePushTargetRemote( - repo.path, - removalTarget.id, - removedPushTarget, - store, - localWorktreeGitOptions - ) - this.clearOptimisticReconcileToken(removalTarget.id) - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) - this.preservedBranchCleanupByWorktreeId.delete(removalTarget.id) - this.invalidateResolvedWorktreeCache() - this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) - invalidateAuthorizedRootsCache() - this.notifyWorktreesChanged(repo.id) - removalCompleted = true - return { - ...(warning ? { warning } : {}) - } - } else { - throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, force)) - } - } - removalCompleted = true - } finally { - await removalGate.finish(removalCompleted) - } - - await cleanupUnusedWorktreePushTargetRemote( - repo.path, - removalTarget.id, - removedPushTarget, - store, - localWorktreeGitOptions - ) - this.rememberPreservedBranchCleanupTarget( - removalTarget.id, - removalResult, - refreshedRegisteredWorktree.head, - removedPushTarget - ) - this.clearOptimisticReconcileToken(removalTarget.id) - this.removeWorktreeMetadataAndHistory(store, removalTarget.id) - this.invalidateResolvedWorktreeCache() - this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) - invalidateAuthorizedRootsCache() - this.notifyWorktreesChanged(repo.id) - return { - ...removalResult, - ...(warning ? { warning } : {}) - } + }) })() this.removeManagedWorktreeInFlight.set(removalTarget.id, { optionsKey, promise: removal }) try { diff --git a/src/main/terminal-history-async-delete.test.ts b/src/main/terminal-history-async-delete.test.ts new file mode 100644 index 000000000..15eab3015 --- /dev/null +++ b/src/main/terminal-history-async-delete.test.ts @@ -0,0 +1,87 @@ +import { mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +let userDataDir: string + +vi.mock('electron', () => ({ + app: { + getPath: () => userDataDir + } +})) + +import { hashWorktreeId } from './terminal-history-paths' +import { + deleteWorktreeHistoryDir, + flushPendingWorktreeHistoryDeletions +} from './terminal-history-deletion' + +/** + * Prove worktree history delete stays off the main-thread recursive-rm path: the critical path only + * tombstones, and the async rm finishes afterwards. + */ +describe('deleteWorktreeHistoryDir main-thread safety', () => { + beforeEach(() => { + userDataDir = mkdtempSync(join(tmpdir(), 'orca-history-async-')) + }) + + afterEach(async () => { + await flushPendingWorktreeHistoryDeletions() + rmSync(userDataDir, { recursive: true, force: true }) + }) + + it('tombstones without blocking the event loop, then removes via async rm', async () => { + const worktreeId = 'repo-1::/path/heavy-wt' + const hash = hashWorktreeId(worktreeId) + const historyDir = join(userDataDir, 'terminal-history', hash) + mkdirSync(historyDir, { recursive: true }) + // Enough files that a recursive sync walk would dominate the critical-path duration. + for (let i = 0; i < 3_000; i++) { + writeFileSync(join(historyDir, `file-${i}.txt`), `payload-${i}`) + } + + // Why critical-path wall time, not setInterval gaps: deleteWorktreeHistoryDir is sync and must + // only rename. Interval gaps during the later async rm spike under CI scheduling (~50ms) even + // when the critical path is fine; a recursive sync walk of 3k files is still hundreds of ms. + const criticalPathStartedAt = performance.now() + deleteWorktreeHistoryDir(worktreeId) + const criticalPathMs = performance.now() - criticalPathStartedAt + + // Why a looser CI/Windows bound: a rename is O(1) metadata everywhere, but AV and shared CI runners + // stall even that. The structural assertions below are the real proof; this only catches a sync walk. + expect(criticalPathMs).toBeLessThan( + process.env.CI || process.platform === 'win32' ? 1_000 : 100 + ) + expect(readdirSync(join(userDataDir, 'terminal-history'))).not.toContain(hash) + expect( + readdirSync(join(userDataDir, 'terminal-history', '.pending-delete')).length + ).toBeGreaterThan(0) + + await flushPendingWorktreeHistoryDeletions() + expect(readdirSync(join(userDataDir, 'terminal-history', '.pending-delete'))).toHaveLength(0) + }) + + it('drains a deletion scheduled after the flush snapshotted its batch', async () => { + const seedDir = join(userDataDir, 'terminal-history', hashWorktreeId('repo-1::/path/seed-wt')) + mkdirSync(seedDir, { recursive: true }) + writeFileSync(join(seedDir, 'seed.txt'), 'seed') + + const lateWorktreeId = 'repo-1::/path/late-wt' + const lateDir = join(userDataDir, 'terminal-history', hashWorktreeId(lateWorktreeId)) + mkdirSync(lateDir, { recursive: true }) + // Big enough that its rm is still in flight when the snapshotted seed removal settles. + for (let i = 0; i < 3_000; i++) { + writeFileSync(join(lateDir, `file-${i}.txt`), `payload-${i}`) + } + + deleteWorktreeHistoryDir('repo-1::/path/seed-wt') + // Why no await before the second delete: the flush snapshots the pending map synchronously, so + // this schedules the late removal outside that batch — exactly the race the drain loop covers. + const flushed = flushPendingWorktreeHistoryDeletions() + deleteWorktreeHistoryDir(lateWorktreeId) + await flushed + + expect(readdirSync(join(userDataDir, 'terminal-history', '.pending-delete'))).toHaveLength(0) + }) +}) diff --git a/src/main/terminal-history-deletion.ts b/src/main/terminal-history-deletion.ts new file mode 100644 index 000000000..b17eaa7ba --- /dev/null +++ b/src/main/terminal-history-deletion.ts @@ -0,0 +1,171 @@ +import { basename, join } from 'node:path' +import { existsSync, mkdirSync, readdirSync, renameSync } from 'node:fs' +import { removeHostTree } from './host-tree-removal' +import { + getHistoryRoot, + hashWorktreeId, + listWslHistoryRoots, + PENDING_DELETE_DIR_NAME +} from './terminal-history-paths' + +const pendingHistoryTreeRemovals = new Map>() +// Why: a tombstone that fails once (Windows EBUSY under AV) would otherwise sit on disk for the whole +// desktop session — only the next launch re-queues it. Bounded so a genuinely stuck tree stops retrying. +export const HISTORY_TREE_REMOVAL_RETRY_DELAYS_MS = [30_000, 120_000] +const historyTreeRemovalAttempts = new Map() +const historyTreeRemovalRetryTimers = new Map>() + +function getPendingDeleteRoot(historyRoot: string): string { + return join(historyRoot, PENDING_DELETE_DIR_NAME) +} + +/** Move a history tree to a pending-delete tombstone (metadata-only) so the critical path never walks it. */ +function tombstoneHistoryTree(dir: string, historyRoot: string): string | null { + if (!existsSync(dir)) { + return null + } + const pendingRoot = getPendingDeleteRoot(historyRoot) + try { + if (!existsSync(pendingRoot)) { + mkdirSync(pendingRoot, { recursive: true }) + } + const tombstone = join( + pendingRoot, + `${basename(dir)}.${Date.now()}.${Math.random().toString(16).slice(2)}` + ) + renameSync(dir, tombstone) + return tombstone + } catch (err) { + console.warn( + `[pty:history] Failed to tombstone history dir: ${err instanceof Error ? err.message : String(err)}` + ) + // Why: never schedule an async rm of the live path — worktree IDs are path-derived, so a recreated + // worktree can own this directory again before the rm lands. GC reclaims it by meta.worktreeId instead. + return null + } +} + +function scheduleHistoryTreeRemovalRetry(dir: string): void { + const attempt = historyTreeRemovalAttempts.get(dir) ?? 0 + const retryDelayMs = HISTORY_TREE_REMOVAL_RETRY_DELAYS_MS[attempt] + if (retryDelayMs === undefined) { + // Out of in-process attempts: the tombstone stays on disk and the next startup drain re-queues it. + historyTreeRemovalAttempts.delete(dir) + return + } + historyTreeRemovalAttempts.set(dir, attempt + 1) + const timer = setTimeout(() => { + historyTreeRemovalRetryTimers.delete(dir) + scheduleHistoryTreeRemoval(dir) + }, retryDelayMs) + timer.unref?.() + historyTreeRemovalRetryTimers.set(dir, timer) +} + +function scheduleHistoryTreeRemoval(dir: string): void { + if (pendingHistoryTreeRemovals.has(dir)) { + return + } + // A rescan (GC / startup drain) hitting the same tombstone supersedes its pending retry. + const pendingRetry = historyTreeRemovalRetryTimers.get(dir) + if (pendingRetry) { + clearTimeout(pendingRetry) + historyTreeRemovalRetryTimers.delete(dir) + } + const removal = removeHostTree(dir) + .then(() => { + historyTreeRemovalAttempts.delete(dir) + }) + .catch((err: unknown) => { + console.warn( + `[pty:history] Failed to delete history dir: ${err instanceof Error ? err.message : String(err)}` + ) + scheduleHistoryTreeRemovalRetry(dir) + }) + .finally(() => { + if (pendingHistoryTreeRemovals.get(dir) === removal) { + pendingHistoryTreeRemovals.delete(dir) + } + }) + pendingHistoryTreeRemovals.set(dir, removal) +} + +/** Tombstone one history tree and queue its recursive removal off the caller's critical path. + * Returns false when the rename failed, leaving the tree for a later GC pass to reclaim. */ +export function scheduleWorktreeHistoryTreeDeletion(dir: string, historyRoot: string): boolean { + const tombstone = tombstoneHistoryTree(dir, historyRoot) + if (!tombstone) { + return false + } + scheduleHistoryTreeRemoval(tombstone) + return true +} + +/** Schedule tombstoned trees under one history root for async removal — the retry after a quit mid-rm. */ +export function schedulePendingHistoryTreeRemovals(historyRoot: string): void { + const pendingRoot = getPendingDeleteRoot(historyRoot) + if (!existsSync(pendingRoot)) { + return + } + try { + for (const entry of readdirSync(pendingRoot)) { + scheduleHistoryTreeRemoval(join(pendingRoot, entry)) + } + } catch { + // Non-fatal. + } +} + +/** Schedule tombstoned trees under every history root, native and WSL. */ +export function scheduleAllPendingHistoryTreeRemovals(): void { + schedulePendingHistoryTreeRemovals(getHistoryRoot()) + for (const distroRoot of listWslHistoryRoots()) { + schedulePendingHistoryTreeRemovals(distroRoot) + } +} + +/** Drop every armed retry timer so a fixture teardown cannot resurrect a removal. Tests only. */ +export function cancelPendingHistoryTreeRemovalRetries(): void { + for (const timer of historyTreeRemovalRetryTimers.values()) { + clearTimeout(timer) + } + historyTreeRemovalRetryTimers.clear() + historyTreeRemovalAttempts.clear() +} + +/** Drain every history root's tombstones and await the in-flight removals. Tests only: production + * schedules the same drain from startup GC and headless serve without ever blocking on it. */ +export async function flushPendingWorktreeHistoryDeletions(): Promise { + scheduleAllPendingHistoryTreeRemovals() + // Why loop: awaiting one snapshot of the map would return with a removal scheduled mid-batch still + // in flight. Each pass settles its batch and drains whatever was added while it ran. + while (pendingHistoryTreeRemovals.size > 0) { + await Promise.all(pendingHistoryTreeRemovals.values()) + } +} + +/** Delete the history directory for a removed worktree. Non-fatal; never blocks on recursive rm. */ +export function deleteWorktreeHistoryDir(worktreeId: string): void { + const worktreeHash = hashWorktreeId(worktreeId) + const historyRoot = getHistoryRoot() + try { + if (scheduleWorktreeHistoryTreeDeletion(join(historyRoot, worktreeHash), historyRoot)) { + console.log(`[pty:history] Scheduled history delete for worktree ${worktreeId}`) + } + } catch (err) { + console.warn( + `[pty:history] Failed to schedule history delete: ${err instanceof Error ? err.message : String(err)}` + ) + } + + // Also clean up WSL history for this worktree; listWslHistoryRoots is empty where WSL never ran. + try { + for (const distroRoot of listWslHistoryRoots()) { + scheduleWorktreeHistoryTreeDeletion(join(distroRoot, worktreeHash), distroRoot) + } + } catch (err) { + console.warn( + `[pty:history] Failed to schedule WSL history delete: ${err instanceof Error ? err.message : String(err)}` + ) + } +} diff --git a/src/main/terminal-history-gc.ts b/src/main/terminal-history-gc.ts new file mode 100644 index 000000000..2a6d22809 --- /dev/null +++ b/src/main/terminal-history-gc.ts @@ -0,0 +1,154 @@ +import { join } from 'node:path' +import { existsSync, readFileSync, readdirSync, statSync } from 'node:fs' +import { + getHistoryRoot, + listWslHistoryRoots, + PENDING_DELETE_DIR_NAME +} from './terminal-history-paths' +import { + schedulePendingHistoryTreeRemovals, + scheduleWorktreeHistoryTreeDeletion +} from './terminal-history-deletion' + +// Why 5 minutes: GC runs ~10s after startup, and the live-worktree snapshot is +// taken just before. A worktree created between the snapshot and GC execution +// won't appear in liveWorktreeIds, so without an age guard GC would delete its +// freshly-created history directory (TOCTOU race). 5 minutes is generous enough +// to cover any realistic snapshot-to-scan delay. +const GC_MIN_AGE_MS = 5 * 60 * 1000 + +let scheduledHistoryGcTimer: ReturnType | null = null +let historyGcRunning = false + +/** Scan a single history root directory, pruning orphaned entries. + * Returns { totalDirs, orphaned, pruned, totalSizeKB }. */ +function gcScanRoot( + root: string, + liveWorktreeIds: Set +): { totalDirs: number; orphaned: number; pruned: number; totalSizeKB: number } { + const result = { totalDirs: 0, orphaned: 0, pruned: 0, totalSizeKB: 0 } + if (!existsSync(root)) { + return result + } + + const now = Date.now() + + for (const entry of readdirSync(root)) { + // Why: pending-delete is a tombstone queue drained asynchronously, not a live worktree hash. + if (entry === PENDING_DELETE_DIR_NAME) { + continue + } + const entryPath = join(root, entry) + try { + const stat = statSync(entryPath) + if (!stat.isDirectory()) { + continue + } + result.totalDirs++ + + // Estimate directory size from meta.json + history files. + try { + for (const file of readdirSync(entryPath)) { + result.totalSizeKB += Math.ceil(statSync(join(entryPath, file)).size / 1024) + } + } catch { + // Skip size estimation on error. + } + + const metaPath = join(entryPath, 'meta.json') + if (!existsSync(metaPath)) { + // No meta.json — can't determine ownership, skip. + continue + } + + const meta = JSON.parse(readFileSync(metaPath, 'utf-8')) as { + worktreeId?: string + createdAt?: string + } + if (!meta.worktreeId) { + continue + } + + if (!liveWorktreeIds.has(meta.worktreeId)) { + // Why: avoid a TOCTOU race where a worktree is created after the + // live-ID snapshot but before GC runs. Directories younger than + // GC_MIN_AGE_MS are presumed still live and skipped. + if (meta.createdAt) { + const ageMs = now - new Date(meta.createdAt).getTime() + if (ageMs < GC_MIN_AGE_MS) { + continue + } + } + + result.orphaned++ + // Why: a large orphaned tree recursive-rm'd here would stall the main process ~10s after + // launch — the same freeze the explicit-delete path already tombstones its way out of. + if (scheduleWorktreeHistoryTreeDeletion(entryPath, root)) { + result.pruned++ + console.log(`[pty:history:gc] Pruned orphaned history: ${meta.worktreeId}`) + } + } + } catch { + // Skip individual entries that fail. + } + } + return result +} + +/** Run background GC to prune history directories for worktrees that are no + * longer in Orca's known live-worktree set. */ +export function runHistoryGc(liveWorktreeIds: Set): void { + try { + // Why: finish tombstones left by quit mid-rm before scanning live worktree hashes. + schedulePendingHistoryTreeRemovals(getHistoryRoot()) + const main = gcScanRoot(getHistoryRoot(), liveWorktreeIds) + + // Also scan WSL history directories (each distro has its own subdirectory). + const wslTotals = { totalDirs: 0, orphaned: 0, pruned: 0, totalSizeKB: 0 } + for (const distroRoot of listWslHistoryRoots()) { + schedulePendingHistoryTreeRemovals(distroRoot) + const r = gcScanRoot(distroRoot, liveWorktreeIds) + wslTotals.totalDirs += r.totalDirs + wslTotals.orphaned += r.orphaned + wslTotals.pruned += r.pruned + wslTotals.totalSizeKB += r.totalSizeKB + } + + const totalDirs = main.totalDirs + wslTotals.totalDirs + const orphaned = main.orphaned + wslTotals.orphaned + const pruned = main.pruned + wslTotals.pruned + const totalSizeKB = main.totalSizeKB + wslTotals.totalSizeKB + + console.log( + `[pty:history:gc] totalDirs=${totalDirs} orphaned=${orphaned} pruned=${pruned} totalSizeKB=${totalSizeKB}` + ) + } catch (err) { + console.warn(`[pty:history:gc] GC failed: ${err instanceof Error ? err.message : String(err)}`) + } +} + +/** Schedule GC after a delay so it runs after workspace hydration completes. + * `getLiveWorktreeIds` should use already-known IDs, not probe repo paths. */ +export function scheduleHistoryGc(getLiveWorktreeIds: () => Promise>): void { + // Why: main-window services can reattach during reload/reactivation; one + // pending/running disk GC is enough and avoids duplicate startup I/O. + if (scheduledHistoryGcTimer !== null || historyGcRunning) { + return + } + // Why 10s: avoids competing with startup-critical I/O while still running + // early enough to clean up before the user notices disk usage (§7.6). + scheduledHistoryGcTimer = setTimeout(async () => { + scheduledHistoryGcTimer = null + historyGcRunning = true + try { + const liveIds = await getLiveWorktreeIds() + runHistoryGc(liveIds) + } catch (err) { + console.warn( + `[pty:history:gc] Failed to enumerate live worktrees for GC: ${err instanceof Error ? err.message : String(err)}` + ) + } finally { + historyGcRunning = false + } + }, 10_000) +} diff --git a/src/main/terminal-history-paths.ts b/src/main/terminal-history-paths.ts new file mode 100644 index 000000000..e942dd8bc --- /dev/null +++ b/src/main/terminal-history-paths.ts @@ -0,0 +1,35 @@ +import { createHash } from 'node:crypto' +import { existsSync, readdirSync } from 'node:fs' +import { join } from 'node:path' +import { app } from 'electron' + +const HISTORY_DIR_NAME = 'terminal-history' +const HISTORY_DIR_NAME_WSL = 'terminal-history-wsl' +// Why: rename live history out of the way first so a quit mid-rm still leaves a durable tombstone GC can finish. +export const PENDING_DELETE_DIR_NAME = '.pending-delete' + +/** First 16 hex chars of SHA-256 of the worktreeId. */ +export function hashWorktreeId(worktreeId: string): string { + return createHash('sha256').update(worktreeId).digest('hex').slice(0, 16) +} + +export function getHistoryRoot(): string { + return join(app.getPath('userData'), HISTORY_DIR_NAME) +} + +export function getHistoryRootWsl(distro: string): string { + return join(app.getPath('userData'), HISTORY_DIR_NAME_WSL, distro) +} + +/** Every per-distro WSL history root that exists on disk; empty when WSL history was never written. */ +export function listWslHistoryRoots(): string[] { + const wslRoot = join(app.getPath('userData'), HISTORY_DIR_NAME_WSL) + if (!existsSync(wslRoot)) { + return [] + } + try { + return readdirSync(wslRoot).map((distro) => join(wslRoot, distro)) + } catch { + return [] + } +} diff --git a/src/main/terminal-history-tombstone-retry.test.ts b/src/main/terminal-history-tombstone-retry.test.ts new file mode 100644 index 000000000..195ee82d9 --- /dev/null +++ b/src/main/terminal-history-tombstone-retry.test.ts @@ -0,0 +1,82 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +let userDataDir: string + +const { removeHostTreeMock } = vi.hoisted(() => ({ + removeHostTreeMock: vi.fn<(dir: string) => Promise>() +})) + +vi.mock('electron', () => ({ + app: { + getPath: () => userDataDir + } +})) + +vi.mock('./host-tree-removal', () => ({ + removeHostTree: removeHostTreeMock +})) + +import { hashWorktreeId } from './terminal-history-paths' +import { + cancelPendingHistoryTreeRemovalRetries, + deleteWorktreeHistoryDir, + HISTORY_TREE_REMOVAL_RETRY_DELAYS_MS +} from './terminal-history-deletion' + +/** A tombstone whose rm fails once used to sit on disk for the rest of the session — only the next + * process start re-queued it. Prove the failure re-arms in-process, and that it stays bounded. */ +describe('tombstoned history removal retries', () => { + beforeEach(() => { + userDataDir = mkdtempSync(join(tmpdir(), 'orca-history-retry-')) + removeHostTreeMock.mockReset() + vi.useFakeTimers() + }) + + afterEach(() => { + cancelPendingHistoryTreeRemovalRetries() + vi.useRealTimers() + rmSync(userDataDir, { recursive: true, force: true }) + }) + + function seedWorktreeHistory(worktreeId: string): void { + const dir = join(userDataDir, 'terminal-history', hashWorktreeId(worktreeId)) + mkdirSync(dir, { recursive: true }) + writeFileSync(join(dir, 'meta.json'), '{}') + } + + it('re-queues a tombstone whose removal failed, then stops after the last attempt', async () => { + seedWorktreeHistory('repo-1::/path/busy-wt') + const busy = Object.assign(new Error('resource busy'), { code: 'EBUSY' }) + removeHostTreeMock.mockRejectedValue(busy) + + deleteWorktreeHistoryDir('repo-1::/path/busy-wt') + await vi.advanceTimersByTimeAsync(0) + expect(removeHostTreeMock).toHaveBeenCalledTimes(1) + + for (const [index, retryDelayMs] of HISTORY_TREE_REMOVAL_RETRY_DELAYS_MS.entries()) { + await vi.advanceTimersByTimeAsync(retryDelayMs) + expect(removeHostTreeMock).toHaveBeenCalledTimes(index + 2) + } + + // Bounded: a permanently wedged tree stops burning timers and waits for the next startup drain. + await vi.advanceTimersByTimeAsync(60 * 60_000) + expect(removeHostTreeMock).toHaveBeenCalledTimes( + HISTORY_TREE_REMOVAL_RETRY_DELAYS_MS.length + 1 + ) + expect(removeHostTreeMock).toHaveBeenLastCalledWith(expect.stringContaining('.pending-delete')) + }) + + it('does not re-arm a retry after the removal succeeds', async () => { + seedWorktreeHistory('repo-1::/path/clean-wt') + removeHostTreeMock.mockResolvedValue(undefined) + + deleteWorktreeHistoryDir('repo-1::/path/clean-wt') + await vi.advanceTimersByTimeAsync(0) + await vi.advanceTimersByTimeAsync(HISTORY_TREE_REMOVAL_RETRY_DELAYS_MS[0]) + + expect(removeHostTreeMock).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/terminal-history.test.ts b/src/main/terminal-history.test.ts index a9750bb60..c1900e3d0 100644 --- a/src/main/terminal-history.test.ts +++ b/src/main/terminal-history.test.ts @@ -1,3 +1,5 @@ +import type * as FsPromises from 'node:fs/promises' +import { sep } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { @@ -6,18 +8,22 @@ const { writeFileSyncMock, readFileSyncMock, rmSyncMock, + renameSyncMock, readdirSyncMock, statSyncMock, - getPathMock + getPathMock, + rmAsyncMock } = vi.hoisted(() => ({ existsSyncMock: vi.fn(), mkdirSyncMock: vi.fn(), writeFileSyncMock: vi.fn(), readFileSyncMock: vi.fn(), rmSyncMock: vi.fn(), + renameSyncMock: vi.fn(), readdirSyncMock: vi.fn(), statSyncMock: vi.fn(), - getPathMock: vi.fn() + getPathMock: vi.fn(), + rmAsyncMock: vi.fn(async () => undefined) })) vi.mock('fs', () => ({ @@ -26,10 +32,18 @@ vi.mock('fs', () => ({ writeFileSync: writeFileSyncMock, readFileSync: readFileSyncMock, rmSync: rmSyncMock, + renameSync: renameSyncMock, readdirSync: readdirSyncMock, statSync: statSyncMock })) +// Spread the real module: this factory replaces node:fs/promises for the whole import graph, so a +// transitive readFile/mkdir would otherwise resolve to undefined. +vi.mock('node:fs/promises', async () => ({ + ...(await vi.importActual('node:fs/promises')), + rm: rmAsyncMock +})) + vi.mock('electron', () => ({ app: { getPath: getPathMock @@ -48,14 +62,16 @@ vi.mock('./wsl', () => ({ import { resolveShellKind, - hashWorktreeId, ensureHistoryDir, injectHistoryEnv, - updateHistFileForFallback, - deleteWorktreeHistoryDir, - runHistoryGc, - scheduleHistoryGc + updateHistFileForFallback } from './terminal-history' +import { hashWorktreeId } from './terminal-history-paths' +import { + deleteWorktreeHistoryDir, + flushPendingWorktreeHistoryDeletions +} from './terminal-history-deletion' +import { runHistoryGc, scheduleHistoryGc } from './terminal-history-gc' describe('terminal-history', () => { afterEach(() => { @@ -64,6 +80,8 @@ describe('terminal-history', () => { beforeEach(() => { vi.clearAllMocks() + // Why: clearAllMocks keeps implementations, so a throwing rename from one test would leak forward. + renameSyncMock.mockReset() getPathMock.mockReturnValue('/fake/userData') existsSyncMock.mockReturnValue(true) statSyncMock.mockReturnValue({ isDirectory: () => true, size: 100 }) @@ -256,21 +274,63 @@ describe('terminal-history', () => { }) describe('deleteWorktreeHistoryDir', () => { - it('removes the history directory for a worktree', () => { + it('tombstones then async-removes the history directory without recursive rmSync', async () => { existsSyncMock.mockReturnValue(true) deleteWorktreeHistoryDir('repo-1::/path/wt') - expect(rmSyncMock).toHaveBeenCalledWith(expect.stringContaining('terminal-history'), { - recursive: true, - force: true - }) + expect(renameSyncMock).toHaveBeenCalled() + expect(rmSyncMock).not.toHaveBeenCalled() + expect(rmAsyncMock).toHaveBeenCalledWith( + expect.stringContaining('.pending-delete'), + expect.objectContaining({ recursive: true, force: true }) + ) + await flushPendingWorktreeHistoryDeletions() }) - it('does not throw on deletion failure', () => { + it('leaves the live directory alone when the tombstone rename fails', async () => { existsSyncMock.mockReturnValue(true) - rmSyncMock.mockImplementation(() => { + renameSyncMock.mockImplementation(() => { throw new Error('permission denied') }) + expect(() => deleteWorktreeHistoryDir('repo-1::/path/wt')).not.toThrow() + + // Why: a path-derived worktree ID can be recreated at the same path, so an async rm aimed at the + // live directory could delete a freshly recreated worktree's history. GC reclaims it instead. + expect(rmAsyncMock).not.toHaveBeenCalled() + expect(rmSyncMock).not.toHaveBeenCalled() + await flushPendingWorktreeHistoryDeletions() + }) + + it('does not throw when the async removal itself fails', async () => { + existsSyncMock.mockReturnValue(true) + rmAsyncMock.mockRejectedValueOnce(new Error('async fail')) + expect(() => deleteWorktreeHistoryDir('repo-1::/path/wt')).not.toThrow() + await expect(flushPendingWorktreeHistoryDeletions()).resolves.toBeUndefined() + }) + + it('retries pending tombstones on flush after a failed async rm (app-quit durability)', async () => { + existsSyncMock.mockImplementation((p: string) => { + const path = String(p) + if (path.includes('.pending-delete') && !path.endsWith('.pending-delete')) { + return true + } + if (path.endsWith('terminal-history') || path.endsWith('.pending-delete')) { + return true + } + return path.includes(hashWorktreeId('repo-1::/path/wt')) + }) + readdirSyncMock.mockImplementation((p: string) => { + if (String(p).endsWith('.pending-delete')) { + return ['leftover-tombstone'] + } + return [] + }) + rmAsyncMock.mockResolvedValue(undefined) + await flushPendingWorktreeHistoryDeletions() + expect(rmAsyncMock).toHaveBeenCalledWith( + expect.stringContaining('leftover-tombstone'), + expect.objectContaining({ recursive: true, force: true }) + ) }) }) @@ -315,12 +375,17 @@ describe('terminal-history', () => { const liveIds = new Set(['live-wt']) runHistoryGc(liveIds) - // Should only prune dir2 (dead-wt), not dir1 (live-wt) - expect(rmSyncMock).toHaveBeenCalledTimes(1) - expect(rmSyncMock).toHaveBeenCalledWith(expect.stringContaining('dir2'), { - recursive: true, - force: true - }) + // Should only prune dir2 (dead-wt), not dir1 (live-wt), and never recursive-rm on the main thread. + expect(rmSyncMock).not.toHaveBeenCalled() + expect(renameSyncMock).toHaveBeenCalledTimes(1) + expect(renameSyncMock).toHaveBeenCalledWith( + expect.stringContaining('dir2'), + expect.stringContaining(`.pending-delete${sep}dir2.`) + ) + expect(rmAsyncMock).toHaveBeenCalledWith( + expect.stringContaining(`.pending-delete${sep}dir2.`), + expect.objectContaining({ recursive: true, force: true }) + ) }) it('skips recently-created directories to avoid TOCTOU race', () => { @@ -346,6 +411,7 @@ describe('terminal-history', () => { // Should NOT prune because the directory is too young expect(rmSyncMock).not.toHaveBeenCalled() + expect(renameSyncMock).not.toHaveBeenCalled() }) it('does not throw when history root does not exist', () => { @@ -353,6 +419,30 @@ describe('terminal-history', () => { expect(() => runHistoryGc(new Set())).not.toThrow() expect(readdirSyncMock).not.toHaveBeenCalled() }) + + it('drains delete tombstones asynchronously instead of scanning them as worktrees', async () => { + existsSyncMock.mockImplementation((p: string) => !String(p).includes('terminal-history-wsl')) + readdirSyncMock.mockImplementation((dir: string) => { + if (String(dir).endsWith('.pending-delete')) { + return ['abc123.1700000000000.deadbeef'] + } + if (String(dir).endsWith('terminal-history')) { + return ['.pending-delete'] + } + return ['meta.json'] + }) + statSyncMock.mockReturnValue({ isDirectory: () => true, size: 100 }) + + runHistoryGc(new Set()) + + // The tombstone queue is drained off-thread; GC must never rmSync it or count it as a worktree. + expect(rmSyncMock).not.toHaveBeenCalled() + expect(rmAsyncMock).toHaveBeenCalledWith( + expect.stringContaining('abc123.1700000000000.deadbeef'), + expect.objectContaining({ recursive: true, force: true }) + ) + await flushPendingWorktreeHistoryDeletions() + }) }) describe('WSL path conversion', () => { diff --git a/src/main/terminal-history.ts b/src/main/terminal-history.ts index a6d09cfca..38dd79682 100644 --- a/src/main/terminal-history.ts +++ b/src/main/terminal-history.ts @@ -1,27 +1,10 @@ -import { createHash } from 'node:crypto' import { join, basename } from 'node:path' -import { - mkdirSync, - existsSync, - readFileSync, - writeFileSync, - readdirSync, - rmSync, - statSync -} from 'node:fs' -import { app } from 'electron' +import { mkdirSync, existsSync, writeFileSync } from 'node:fs' import { parseWslPath, toLinuxPath } from './wsl' - -// ─── Constants ───────────────────────────────────────────────────── - -const HISTORY_DIR_NAME = 'terminal-history' -const HISTORY_DIR_NAME_WSL = 'terminal-history-wsl' +import { getHistoryRoot, getHistoryRootWsl, hashWorktreeId } from './terminal-history-paths' type ShellKind = 'zsh' | 'bash' | 'fish' | 'pwsh' | 'powershell' | 'cmd' | 'unknown' -let scheduledHistoryGcTimer: ReturnType | null = null -let historyGcRunning = false - // ─── Shell Detection ─────────────────────────────────────────────── /** Resolve the shell kind from a shell binary path. @@ -50,13 +33,6 @@ export function resolveShellKind(shellPath: string): ShellKind { return 'unknown' } -// ─── Hash & Path Helpers ─────────────────────────────────────────── - -/** First 16 hex chars of SHA-256 of the worktreeId. */ -export function hashWorktreeId(worktreeId: string): string { - return createHash('sha256').update(worktreeId).digest('hex').slice(0, 16) -} - /** Map shell kind to the filename used inside the history directory. */ function historyFilename(shell: ShellKind): string | null { switch (shell) { @@ -76,14 +52,6 @@ function historyFilename(shell: ShellKind): string | null { // ─── Directory Management ────────────────────────────────────────── -function getHistoryRoot(): string { - return join(app.getPath('userData'), HISTORY_DIR_NAME) -} - -function getHistoryRootWsl(distro: string): string { - return join(app.getPath('userData'), HISTORY_DIR_NAME_WSL, distro) -} - /** Ensure the history directory exists for a given worktree hash. * Returns the directory path, or null if creation failed. */ export function ensureHistoryDir(worktreeHash: string, wslDistro?: string): string | null { @@ -205,178 +173,3 @@ export function logHistoryInjection(worktreeId: string, result: HistoryInjection `[pty:history] worktreeId=${truncatedId} shell=${result.shell} histFile=${result.histFile ?? 'none'}` ) } - -// ─── Cleanup ─────────────────────────────────────────────────────── - -/** Delete the history directory for a removed worktree. Non-fatal. */ -export function deleteWorktreeHistoryDir(worktreeId: string): void { - const worktreeHash = hashWorktreeId(worktreeId) - const dir = join(getHistoryRoot(), worktreeHash) - try { - if (existsSync(dir)) { - rmSync(dir, { recursive: true, force: true }) - console.log(`[pty:history] Deleted history for worktree ${worktreeId}`) - } - } catch (err) { - console.warn( - `[pty:history] Failed to delete history dir: ${err instanceof Error ? err.message : String(err)}` - ) - } - - // Also clean up WSL directories if any exist. - if (process.platform === 'win32') { - try { - const wslRoot = join(app.getPath('userData'), HISTORY_DIR_NAME_WSL) - if (existsSync(wslRoot)) { - for (const distro of readdirSync(wslRoot)) { - const wslDir = join(wslRoot, distro, worktreeHash) - if (existsSync(wslDir)) { - rmSync(wslDir, { recursive: true, force: true }) - } - } - } - } catch { - // Non-fatal. - } - } -} - -// ─── Garbage Collection ──────────────────────────────────────────── - -// Why 5 minutes: GC runs ~10s after startup, and the live-worktree snapshot is -// taken just before. A worktree created between the snapshot and GC execution -// won't appear in liveWorktreeIds, so without an age guard GC would delete its -// freshly-created history directory (TOCTOU race). 5 minutes is generous enough -// to cover any realistic snapshot-to-scan delay. -const GC_MIN_AGE_MS = 5 * 60 * 1000 - -/** Scan a single history root directory, pruning orphaned entries. - * Returns { totalDirs, orphaned, pruned, totalSizeKB }. */ -function gcScanRoot( - root: string, - liveWorktreeIds: Set -): { totalDirs: number; orphaned: number; pruned: number; totalSizeKB: number } { - const result = { totalDirs: 0, orphaned: 0, pruned: 0, totalSizeKB: 0 } - if (!existsSync(root)) { - return result - } - - const now = Date.now() - - for (const entry of readdirSync(root)) { - const entryPath = join(root, entry) - try { - const stat = statSync(entryPath) - if (!stat.isDirectory()) { - continue - } - result.totalDirs++ - - // Estimate directory size from meta.json + history files. - try { - for (const file of readdirSync(entryPath)) { - result.totalSizeKB += Math.ceil(statSync(join(entryPath, file)).size / 1024) - } - } catch { - // Skip size estimation on error. - } - - const metaPath = join(entryPath, 'meta.json') - if (!existsSync(metaPath)) { - // No meta.json — can't determine ownership, skip. - continue - } - - const meta = JSON.parse(readFileSync(metaPath, 'utf-8')) as { - worktreeId?: string - createdAt?: string - } - if (!meta.worktreeId) { - continue - } - - if (!liveWorktreeIds.has(meta.worktreeId)) { - // Why: avoid a TOCTOU race where a worktree is created after the - // live-ID snapshot but before GC runs. Directories younger than - // GC_MIN_AGE_MS are presumed still live and skipped. - if (meta.createdAt) { - const ageMs = now - new Date(meta.createdAt).getTime() - if (ageMs < GC_MIN_AGE_MS) { - continue - } - } - - result.orphaned++ - rmSync(entryPath, { recursive: true, force: true }) - result.pruned++ - console.log(`[pty:history:gc] Pruned orphaned history: ${meta.worktreeId}`) - } - } catch { - // Skip individual entries that fail. - } - } - return result -} - -/** Run background GC to prune history directories for worktrees that are no - * longer in Orca's known live-worktree set. */ -export function runHistoryGc(liveWorktreeIds: Set): void { - try { - const main = gcScanRoot(getHistoryRoot(), liveWorktreeIds) - - // Also scan WSL history directories (each distro has its own subdirectory). - const wslRoot = join(app.getPath('userData'), HISTORY_DIR_NAME_WSL) - let wslTotals = { totalDirs: 0, orphaned: 0, pruned: 0, totalSizeKB: 0 } - if (existsSync(wslRoot)) { - try { - for (const distro of readdirSync(wslRoot)) { - const distroRoot = join(wslRoot, distro) - const r = gcScanRoot(distroRoot, liveWorktreeIds) - wslTotals.totalDirs += r.totalDirs - wslTotals.orphaned += r.orphaned - wslTotals.pruned += r.pruned - wslTotals.totalSizeKB += r.totalSizeKB - } - } catch { - // Non-fatal. - } - } - - const totalDirs = main.totalDirs + wslTotals.totalDirs - const orphaned = main.orphaned + wslTotals.orphaned - const pruned = main.pruned + wslTotals.pruned - const totalSizeKB = main.totalSizeKB + wslTotals.totalSizeKB - - console.log( - `[pty:history:gc] totalDirs=${totalDirs} orphaned=${orphaned} pruned=${pruned} totalSizeKB=${totalSizeKB}` - ) - } catch (err) { - console.warn(`[pty:history:gc] GC failed: ${err instanceof Error ? err.message : String(err)}`) - } -} - -/** Schedule GC after a delay so it runs after workspace hydration completes. - * `getLiveWorktreeIds` should use already-known IDs, not probe repo paths. */ -export function scheduleHistoryGc(getLiveWorktreeIds: () => Promise>): void { - // Why: main-window services can reattach during reload/reactivation; one - // pending/running disk GC is enough and avoids duplicate startup I/O. - if (scheduledHistoryGcTimer !== null || historyGcRunning) { - return - } - // Why 10s: avoids competing with startup-critical I/O while still running - // early enough to clean up before the user notices disk usage (§7.6). - scheduledHistoryGcTimer = setTimeout(async () => { - scheduledHistoryGcTimer = null - historyGcRunning = true - try { - const liveIds = await getLiveWorktreeIds() - runHistoryGc(liveIds) - } catch (err) { - console.warn( - `[pty:history:gc] Failed to enumerate live worktrees for GC: ${err instanceof Error ? err.message : String(err)}` - ) - } finally { - historyGcRunning = false - } - }, 10_000) -} diff --git a/src/main/usage-cache-snapshot-writer.ts b/src/main/usage-cache-snapshot-writer.ts new file mode 100644 index 000000000..f38c48de1 --- /dev/null +++ b/src/main/usage-cache-snapshot-writer.ts @@ -0,0 +1,61 @@ +import { mkdir } from 'node:fs/promises' +import { dirname } from 'node:path' +import { + durableWriteTempPath, + removeStaleDurableWriteTempFiles, + writeFileDurableIfCurrent +} from './durable-file-write' + +/** + * Serialized durable writer for the per-provider usage caches. One per store: the multi-MB JSON must + * not block the Electron main thread, writes are queued so two overlapping renames can't both veto + * themselves, and a snapshot superseded before its turn is dropped instead of paying for a rewrite. + */ +export class UsageCacheSnapshotWriter { + private generation = 0 + private pending: Promise + + constructor( + private readonly logTag: string, + private readonly resolveFile: () => string + ) { + // Why: a crash between write and rename orphans a multi-MB temp file; reclaim once per launch. + // Seeding the write queue with it also orders the sweep ahead of the first write of this launch. + this.pending = removeStaleDurableWriteTempFiles(resolveFile()) + } + + /** `serialize` runs only if this snapshot is still the newest one when its turn comes. */ + write(serialize: () => string): Promise { + const generation = ++this.generation + const write = this.pending.then(() => this.commit(generation, serialize)) + // Why: keep the queue usable after a failure — the awaiting caller still sees the rejection. + this.pending = write.catch((error: unknown) => { + console.error(`${this.logTag} Failed to persist usage cache:`, error) + }) + return write + } + + /** Await every write queued so far. Never rejects — the queue already logs its own failures. */ + flush(): Promise { + return this.pending + } + + private async commit(generation: number, serialize: () => string): Promise { + // Why: a newer snapshot is already queued behind this one, so rewriting the cache here is wasted. + if (generation !== this.generation) { + return + } + // Why serialize here and not at queue time: it blocks the main process for a multi-MB cache, and + // superseded generations must not pay for it. Synchronous, so no mutation can tear the JSON. + const payload = serialize() + const usageFile = this.resolveFile() + await mkdir(dirname(usageFile), { recursive: true }).catch(() => {}) + // Why: atomic temp-file + durable rename so a crash cannot leave a truncated analytics file. + await writeFileDurableIfCurrent( + durableWriteTempPath(usageFile), + usageFile, + payload, + () => generation === this.generation + ) + } +} diff --git a/src/main/window/attach-main-window-services.ts b/src/main/window/attach-main-window-services.ts index 4d4448c04..05790e88a 100644 --- a/src/main/window/attach-main-window-services.ts +++ b/src/main/window/attach-main-window-services.ts @@ -40,7 +40,7 @@ import { dismissNudge, type UpdateInstallMode } from '../updater' -import { scheduleHistoryGc } from '../terminal-history' +import { scheduleHistoryGc } from '../terminal-history-gc' import { hydrateLocalPtyRegistryAtBoot } from '../memory/hydrate-local-pty-registry' import type { ClaudeRuntimeAuthPreparation } from '../claude-accounts/runtime-auth-service' import { getKnownWorktreeIdsForHistoryGc } from './history-gc-worktree-ids' diff --git a/tools/benchmarks/worktree-deletion-dev-bench.mjs b/tools/benchmarks/worktree-deletion-dev-bench.mjs new file mode 100644 index 000000000..e00e5423e --- /dev/null +++ b/tools/benchmarks/worktree-deletion-dev-bench.mjs @@ -0,0 +1,641 @@ +#!/usr/bin/env node +/** + * A/B benchmark for worktree deletion against real Orca dev instances. + * + * Usage: + * pnpm bench:worktree-deletion -- --instance baseline=/path/to/main \ + * --instance candidate=. --iterations 3 --history-files 10000 + */ +import { spawn, spawnSync } from 'node:child_process' +import { createHash } from 'node:crypto' +import { existsSync, mkdirSync, mkdtempSync, writeFileSync } from 'node:fs' +import { rm } from 'node:fs/promises' +import net from 'node:net' +import os from 'node:os' +import path from 'node:path' +// Why @playwright/test, not playwright: only the former is a declared devDependency; it re-exports +// the same browser types, so the benchmark resolves without relying on a hoisted transitive install. +import { chromium } from '@playwright/test' + +const DEFAULT_ITERATIONS = 3 +const DEFAULT_HISTORY_FILES = 10_000 +const CDP_START_PORT = 9_700 +const START_TIMEOUT_MS = 180_000 +const IPC_TIMEOUT_MS = 90_000 +const resultsRoot = path.resolve(import.meta.dirname, 'results') + +function parseArgs(argv) { + const options = { + instances: [], + iterations: DEFAULT_ITERATIONS, + historyFiles: DEFAULT_HISTORY_FILES, + keepFixture: false + } + for (let index = 2; index < argv.length; index += 1) { + const value = argv[index] + if (value === '--') { + continue + } + if (value === '--help' || value === '-h') { + printHelp() + process.exit(0) + } + if (value === '--keep-fixture') { + options.keepFixture = true + continue + } + const next = () => { + const result = argv[++index] + if (!result) { + throw new Error(`${value} needs a value`) + } + return result + } + if (value === '--instance') { + const entry = next() + const separator = entry.indexOf('=') + if (separator <= 0) { + throw new Error('--instance must use label=/absolute/or/relative/path') + } + options.instances.push({ + label: entry.slice(0, separator), + repoRoot: path.resolve(entry.slice(separator + 1)) + }) + } else if (value === '--iterations') { + options.iterations = readPositiveInteger(value, next()) + } else if (value === '--history-files') { + options.historyFiles = readPositiveInteger(value, next()) + } else { + throw new Error(`Unknown argument: ${value}`) + } + } + if (options.instances.length === 0) { + options.instances.push({ label: 'candidate', repoRoot: process.cwd() }) + } + return options +} + +function readPositiveInteger(flag, value) { + const number = Number(value) + if (!Number.isInteger(number) || number < 1) { + throw new Error(`${flag} must be a positive integer`) + } + return number +} + +function printHelp() { + console.log(`Usage: + pnpm bench:worktree-deletion -- [options] + +Options: + --instance Dev checkout to launch; repeat for A/B comparison + --iterations Deletions per instance (default: ${DEFAULT_ITERATIONS}) + --history-files Files seeded in each worktree history (default: ${DEFAULT_HISTORY_FILES}) + --keep-fixture Keep disposable profiles and repos for inspection`) +} + +function run(command, args, cwd) { + const result = spawnSync(command, args, { cwd, encoding: 'utf8' }) + if (result.status !== 0) { + throw new Error( + `${command} ${args.join(' ')} failed (${result.status})\n${result.stderr || result.stdout}` + ) + } +} + +function createFixture(instanceLabel) { + const root = mkdtempSync(path.join(os.tmpdir(), `orca-delete-bench-${instanceLabel}-`)) + const repoPath = path.join(root, 'repo') + const userDataPath = path.join(root, 'user-data') + mkdirSync(repoPath) + mkdirSync(userDataPath) + // Git 2.25 lacks `git init --initial-branch`; rename after the first commit below. + run('git', ['init'], repoPath) + run('git', ['config', 'user.email', 'worktree-delete-bench@orca.invalid'], repoPath) + run('git', ['config', 'user.name', 'Orca Worktree Delete Bench'], repoPath) + writeFileSync(path.join(repoPath, 'README.md'), '# Orca worktree deletion benchmark\n') + run('git', ['add', 'README.md'], repoPath) + run('git', ['commit', '-m', 'Initialize benchmark fixture', '--no-gpg-sign'], repoPath) + run('git', ['branch', '-m', 'main'], repoPath) + return { root, repoPath, userDataPath } +} + +function launchDevInstance({ label, repoRoot }, fixture, port) { + if (!existsSync(path.join(repoRoot, 'package.json'))) { + throw new Error(`${label}: package.json not found under ${repoRoot}`) + } + const env = { + ...process.env, + ORCA_DEV_USER_DATA_PATH: fixture.userDataPath, + REMOTE_DEBUGGING_PORT: String(port) + } + delete env.ELECTRON_RUN_AS_NODE + const child = spawn(process.execPath, ['config/scripts/run-electron-vite-dev.mjs'], { + cwd: repoRoot, + env, + stdio: ['ignore', 'pipe', 'pipe'], + // Why: own process group so teardown can signal the Electron/Vite grandchildren, not just the launcher. + detached: process.platform !== 'win32', + windowsHide: true + }) + const log = { stdout: '', stderr: '' } + child.stdout?.on('data', (chunk) => { + log.stdout = appendLog(log.stdout, chunk) + }) + child.stderr?.on('data', (chunk) => { + log.stderr = appendLog(log.stderr, chunk) + }) + return { child, endpoint: `http://127.0.0.1:${port}`, label, log } +} + +function appendLog(current, chunk) { + return `${current}${String(chunk)}`.slice(-30_000) +} + +async function connectToOrca(instance) { + const deadline = Date.now() + START_TIMEOUT_MS + let lastError = null + while (Date.now() < deadline) { + if (instance.child.exitCode !== null) { + throw new Error( + `${instance.label}: dev process exited (${instance.child.exitCode})\n${instance.log.stderr}` + ) + } + try { + const browser = await chromium.connectOverCDP(instance.endpoint) + try { + // CDP answers long before the renderer exposes window.__store; without this, every + // findOrcaPage timeout drops a live browser handle and leaks a connection per retry. + const page = await findOrcaPage(browser) + return { browser, page } + } catch (error) { + await browser.close().catch(() => undefined) + throw error + } + } catch (error) { + lastError = error + await delay(500) + } + } + throw new Error( + `${instance.label}: CDP did not become ready: ${String(lastError)}\n${instance.log.stderr}` + ) +} + +async function findOrcaPage(browser) { + const deadline = Date.now() + 30_000 + while (Date.now() < deadline) { + for (const context of browser.contexts()) { + for (const page of context.pages()) { + const ready = await page + .evaluate(() => Boolean(window.__store && window.api?.repos?.list)) + .catch(() => false) + if (ready) { + return page + } + } + } + await delay(250) + } + throw new Error('Orca renderer with window.__store was not found') +} + +async function addFixtureRepo(page, repoPath) { + const addedRepo = await page.evaluate(async (fixturePath) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is unavailable') + } + const repo = await store.getState().addRepoPath(fixturePath) + if (!repo) { + throw new Error(`Could not add fixture repo ${fixturePath}`) + } + return { id: repo.id, path: repo.path } + }, repoPath) + return page.evaluate( + async ({ fixtureRepoId, fixturePath }) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is unavailable') + } + let rootWorktree + for (let attempt = 0; attempt < 60 && !rootWorktree; attempt += 1) { + await store.getState().fetchWorktrees(fixtureRepoId) + await new Promise((resolve) => window.setTimeout(resolve, 250)) + rootWorktree = store + .getState() + .worktreesByRepo[fixtureRepoId]?.find((worktree) => worktree.path === fixturePath) + } + if (!rootWorktree) { + throw new Error('Fixture root worktree did not load') + } + const state = store.getState() + state.setSidebarOpen(true) + state.setShowActiveOnly(false) + state.setShowSleepingWorkspaces(true) + state.setHideDefaultBranchWorkspace(false) + state.setFilterRepoIds([]) + state.setActiveRepo(fixtureRepoId) + state.setActiveWorktree(rootWorktree.id) + return { repoId: fixtureRepoId, rootWorktreeId: rootWorktree.id } + }, + { fixtureRepoId: addedRepo.id, fixturePath: addedRepo.path } + ) +} + +async function createMeasuredWorktree(page, repoId, iteration) { + return page.evaluate( + async ({ fixtureRepoId, sequence }) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is unavailable') + } + const result = await store + .getState() + .createWorktree(fixtureRepoId, `delete-bench-${sequence}-${Date.now()}`) + await store.getState().fetchWorktrees(fixtureRepoId) + const state = store.getState() + const tab = state.createTab(result.worktree.id) + state.createBrowserTab(result.worktree.id, 'about:blank', { + title: 'worktree deletion regression probe', + activate: false + }) + state.setActiveView('terminal') + state.setActiveWorktree(result.worktree.id) + state.setActiveTab(tab.id) + state.setRightSidebarTab('explorer') + state.setRightSidebarOpen(true) + state.revealWorktreeInSidebar(result.worktree.id, { behavior: 'auto' }) + // Let the terminal and explorer install their real PTY/watcher resources before deletion. + await new Promise((resolve) => window.setTimeout(resolve, 500)) + return { id: result.worktree.id, path: result.worktree.path } + }, + { fixtureRepoId: repoId, sequence: iteration } + ) +} + +function seedTerminalHistory(userDataPath, worktreeId, fileCount) { + const hash = createHash('sha256').update(worktreeId).digest('hex').slice(0, 16) + const historyPath = path.join(userDataPath, 'terminal-history', hash) + const payload = 'worktree deletion benchmark history\n'.repeat(4) + for (let index = 0; index < fileCount; index += 1) { + const bucket = path.join(historyPath, String(Math.floor(index / 250))) + if (index % 250 === 0) { + mkdirSync(bucket, { recursive: true }) + } + writeFileSync(path.join(bucket, `${index}.history`), payload) + } + return historyPath +} + +async function assertWorktreeRowVisible(page, worktreeId) { + await page.waitForFunction( + (id) => + [...document.querySelectorAll('[data-worktree-id]')].some( + (element) => element.getAttribute('data-worktree-id') === id + ), + worktreeId + ) +} + +async function measureDeletion(page, worktreeId, rootWorktreeId) { + return page.evaluate( + async ({ targetId, fallbackId, ipcTimeoutMs }) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is unavailable') + } + store.getState().setActiveWorktree(fallbackId) + let settled = false + let failure = null + const startedAt = performance.now() + const deletion = store + .getState() + .removeWorktree(targetId, true) + .then((result) => { + settled = true + return result + }) + .catch((error) => { + // Why: without this the poll loop below spins forever on a rejected delete — the exact + // failure this benchmark exists to surface would read as a hang. + settled = true + failure = error + return null + }) + const ipcSamplesMs = [] + let maxRendererTimerDriftMs = 0 + let expectedTimerAt = performance.now() + 10 + while (!settled) { + await new Promise((resolve) => window.setTimeout(resolve, 10)) + const now = performance.now() + maxRendererTimerDriftMs = Math.max(maxRendererTimerDriftMs, now - expectedTimerAt) + const ipcStartedAt = performance.now() + await Promise.race([ + window.api.repos.list(), + new Promise((_, reject) => + window.setTimeout(() => reject(new Error('repos:list IPC timed out')), ipcTimeoutMs) + ) + ]) + ipcSamplesMs.push(performance.now() - ipcStartedAt) + // Why re-read the clock here: anchoring on `now` would fold the IPC round trip that + // ipcSamplesMs already records into the next iteration's drift. + expectedTimerAt = performance.now() + 10 + } + const result = await deletion + if (failure) { + throw new Error(`removeWorktree rejected: ${String(failure)}`) + } + const postDeleteIpcStartedAt = performance.now() + await window.api.repos.list() + const finalState = store.getState() + return { + result, + totalMs: performance.now() - startedAt, + ipcSamplesMs, + postDeleteIpcMs: performance.now() - postDeleteIpcStartedAt, + maxRendererTimerDriftMs, + worktreeStillInStore: Object.values(finalState.worktreesByRepo) + .flat() + .some((worktree) => worktree.id === targetId), + residualTabCount: finalState.tabsByWorktree[targetId]?.length ?? 0, + residualBrowserTabCount: finalState.browserTabsByWorktree[targetId]?.length ?? 0, + residualOpenFileCount: finalState.openFiles.filter((file) => file.worktreeId === targetId) + .length + } + }, + { targetId: worktreeId, fallbackId: rootWorktreeId, ipcTimeoutMs: IPC_TIMEOUT_MS } + ) +} + +async function verifyDeletion(page, worktree, historyPath, measurement) { + if (!measurement.result?.ok) { + throw new Error(`removeWorktree failed: ${measurement.result?.error ?? 'unknown error'}`) + } + if (measurement.worktreeStillInStore) { + throw new Error('Deleted worktree remains in the renderer store') + } + if ( + measurement.residualTabCount || + measurement.residualBrowserTabCount || + measurement.residualOpenFileCount + ) { + throw new Error( + `Deleted worktree retained UI state: ${JSON.stringify({ + tabs: measurement.residualTabCount, + browserTabs: measurement.residualBrowserTabCount, + openFiles: measurement.residualOpenFileCount + })}` + ) + } + if (existsSync(worktree.path)) { + throw new Error(`Deleted worktree directory remains at ${worktree.path}`) + } + if (existsSync(historyPath)) { + throw new Error(`Live terminal history remains at ${historyPath}`) + } + await page.waitForFunction( + (id) => + ![...document.querySelectorAll('[data-worktree-id]')].some( + (element) => element.getAttribute('data-worktree-id') === id + ), + worktree.id + ) +} + +async function runIteration(page, fixture, repoState, iteration, historyFiles) { + const worktree = await createMeasuredWorktree(page, repoState.repoId, iteration) + await assertWorktreeRowVisible(page, worktree.id) + const historyPath = seedTerminalHistory(fixture.userDataPath, worktree.id, historyFiles) + const measurement = await measureDeletion(page, worktree.id, repoState.rootWorktreeId) + await verifyDeletion(page, worktree, historyPath, measurement) + return { + iteration, + totalMs: round(measurement.totalMs), + ipcLatencyMs: summarize(measurement.ipcSamplesMs), + postDeleteIpcMs: round(measurement.postDeleteIpcMs), + maxRendererTimerDriftMs: round(measurement.maxRendererTimerDriftMs) + } +} + +async function verifyRestart(instanceConfig, fixture, port, repoId) { + const relaunched = launchDevInstance(instanceConfig, fixture, port) + let browser = null + try { + const connection = await connectToOrca(relaunched) + browser = connection.browser + const { page } = connection + const state = await page.evaluate(async (fixtureRepoId) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is unavailable after restart') + } + await window.api.repos.list() + let recovery = { repoPresent: false, worktreeCount: 0 } + for (let attempt = 0; attempt < 60; attempt += 1) { + await store.getState().fetchRepos() + if (store.getState().repos.some((repo) => repo.id === fixtureRepoId)) { + await store.getState().fetchWorktrees(fixtureRepoId) + } + await new Promise((resolve) => window.setTimeout(resolve, 250)) + recovery = { + repoPresent: store.getState().repos.some((repo) => repo.id === fixtureRepoId), + worktreeCount: store.getState().worktreesByRepo[fixtureRepoId]?.length ?? 0 + } + if (recovery.repoPresent && recovery.worktreeCount > 0) { + break + } + } + return recovery + }, repoId) + if (!state.repoPresent || state.worktreeCount < 1) { + throw new Error(`Fixture repo did not recover after restart: ${JSON.stringify(state)}`) + } + await page.evaluate(async (fixtureRepoId) => { + await window.__store?.getState().removeProject(fixtureRepoId) + }, repoId) + } finally { + await browser?.close().catch(() => undefined) + await stopDevInstance(relaunched) + } +} + +async function benchmarkInstance(instanceConfig, index, options) { + run( + process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm', + ['run', 'ensure:electron-runtime'], + instanceConfig.repoRoot + ) + const fixture = createFixture(instanceConfig.label) + const port = await findAvailablePort(CDP_START_PORT + index) + const instance = launchDevInstance(instanceConfig, fixture, port) + let browser = null + console.log(`[${instanceConfig.label}] launching ${instanceConfig.repoRoot}`) + try { + const connection = await connectToOrca(instance) + browser = connection.browser + const { page } = connection + const repoState = await addFixtureRepo(page, fixture.repoPath) + const iterations = [] + for (let iteration = 1; iteration <= options.iterations; iteration += 1) { + const result = await runIteration(page, fixture, repoState, iteration, options.historyFiles) + iterations.push(result) + console.log( + `[${instanceConfig.label}] ${iteration}/${options.iterations}: ${result.totalMs}ms total, ` + + `${result.ipcLatencyMs.max}ms max main IPC` + ) + } + await browser.close() + browser = null + await stopDevInstance(instance) + // Why a fresh port: the debug listener may still hold the old one, so reusing it can attach to + // the dying endpoint or burn the whole start timeout. + await verifyRestart( + instanceConfig, + fixture, + await findAvailablePort(port + 1), + repoState.repoId + ) + return { + label: instanceConfig.label, + repoRoot: instanceConfig.repoRoot, + historyFiles: options.historyFiles, + iterations, + summary: summarize(iterations.map((entry) => entry.totalMs)), + ipcSummary: summarize(iterations.flatMap((entry) => entry.ipcLatencyMs.samples)), + restartPassed: true, + fixtureRoot: options.keepFixture ? fixture.root : undefined + } + } finally { + await browser?.close().catch(() => undefined) + await stopDevInstance(instance) + if (!options.keepFixture) { + await rm(fixture.root, { + recursive: true, + force: true, + maxRetries: 10, + retryDelay: 250 + }) + } + } +} + +async function findAvailablePort(startPort) { + for (let port = startPort; port < startPort + 100; port += 1) { + if (await isPortAvailable(port)) { + return port + } + } + throw new Error(`No free CDP port found in ${startPort}-${startPort + 99}`) +} + +function isPortAvailable(port) { + return new Promise((resolve) => { + const server = net.createServer() + server.once('error', () => resolve(false)) + server.once('listening', () => server.close(() => resolve(true))) + server.listen(port, '127.0.0.1') + }) +} + +async function stopDevInstance(instance) { + // Why signalCode too: a signal-terminated child leaves exitCode null, so an exitCode-only guard + // re-enters the kill path for a dead process and burns the full 8s race on every teardown. + if (instance.child.exitCode !== null || instance.child.signalCode !== null) { + return + } + if (process.platform === 'win32') { + spawnSync('taskkill.exe', ['/PID', String(instance.child.pid), '/T', '/F'], { stdio: 'ignore' }) + return + } + killDevInstanceTree(instance.child, 'SIGINT') + const exited = await Promise.race([ + new Promise((resolve) => instance.child.once('exit', () => resolve(true))), + delay(8_000).then(() => false) + ]) + if (!exited) { + killDevInstanceTree(instance.child, 'SIGKILL') + } +} + +/** POSIX: signal the whole group, or the launcher's Electron/Vite grandchildren survive and keep the + * CDP port and fixture directory busy. */ +function killDevInstanceTree(child, signal) { + try { + process.kill(-child.pid, signal) + } catch { + child.kill(signal) + } +} + +function summarize(values) { + const samples = values.map((value) => round(value)).sort((a, b) => a - b) + if (samples.length === 0) { + return { count: 0, median: 0, p95: 0, max: 0, samples } + } + return { + count: samples.length, + median: percentile(samples, 0.5), + p95: percentile(samples, 0.95), + max: samples.at(-1), + samples + } +} + +function percentile(sorted, fraction) { + return sorted[Math.min(sorted.length - 1, Math.ceil(sorted.length * fraction) - 1)] +} + +function round(value) { + return Math.round(value * 100) / 100 +} + +function delay(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)) +} + +function printComparison(results) { + console.log('\nWorktree deletion results') + console.table( + results.map((result) => ({ + instance: result.label, + 'delete median ms': result.summary.median, + 'delete p95 ms': result.summary.p95, + 'main IPC p95 ms': result.ipcSummary.p95, + 'main IPC max ms': result.ipcSummary.max, + restart: result.restartPassed ? 'pass' : 'fail' + })) + ) + if (results.length === 2) { + const [baseline, candidate] = results + const percent = round( + ((baseline.summary.median - candidate.summary.median) / baseline.summary.median) * 100 + ) + console.log( + `${candidate.label} median deletion is ${Math.abs(percent)}% ${ + percent >= 0 ? 'faster' : 'slower' + } than ${baseline.label}.` + ) + } +} + +const options = parseArgs(process.argv) +mkdirSync(resultsRoot, { recursive: true }) +const results = [] +for (const [index, instance] of options.instances.entries()) { + results.push(await benchmarkInstance(instance, index, options)) +} +const artifact = { + benchmark: 'worktree-deletion-dev', + createdAt: new Date().toISOString(), + platform: process.platform, + arch: process.arch, + results +} +const artifactPath = path.join( + resultsRoot, + `worktree-deletion-${new Date().toISOString().replaceAll(/[:.]/g, '-')}.json` +) +writeFileSync(artifactPath, `${JSON.stringify(artifact, null, 2)}\n`) +printComparison(results) +console.log(`Artifact: ${artifactPath}`)