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 00000000000..93a5d134ac2 --- /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 index 7046b1fa335..d72f9e8f798 100644 --- a/src/main/daemon/terminal-history-session-tombstone.ts +++ b/src/main/daemon/terminal-history-session-tombstone.ts @@ -12,6 +12,11 @@ import { getTerminalHistoryQuarantineOwnerDir } from './terminal-history-recover 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 @@ -33,15 +38,42 @@ function tombstoneSessionTree(basePath: string, dir: string): string | 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) { @@ -91,6 +123,15 @@ export function schedulePendingSessionTreeRemovals(basePath: string): void { } } +/** 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 { diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index e9565007d08..d1e5925f259 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -8245,8 +8245,10 @@ describe('registerWorktreeHandlers', () => { ]) // 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.mock.invocationCallOrder[0]).toBeLessThan( - removeWorktreeLinkedPathsMock.mock.invocationCallOrder[0] + 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) ) })