mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 16:03:26 +00:00
fix(history): retry failed session tree removals
Tombstoned session trees whose removal fails transiently (e.g., EBUSY under Windows AV) are now re-queued in-process with bounded exponential backoff instead of sitting until the next HistoryManager construction. Prevents a single stuck tree from blocking the entire Orca process.
This commit is contained in:
@@ -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<void>>()
|
||||
}))
|
||||
|
||||
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)
|
||||
})
|
||||
})
|
||||
@@ -12,6 +12,11 @@ import { getTerminalHistoryQuarantineOwnerDir } from './terminal-history-recover
|
||||
const PENDING_DELETE_DIR_NAME = '.pending-delete'
|
||||
|
||||
const pendingSessionTreeRemovals = new Map<string, Promise<void>>()
|
||||
// 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<string, number>()
|
||||
const sessionTreeRemovalRetryTimers = new Map<string, ReturnType<typeof setTimeout>>()
|
||||
|
||||
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<void> {
|
||||
|
||||
@@ -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)
|
||||
)
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user