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}`)