diff --git a/src/main/codex-cli/codex-home-process-lock.test.ts b/src/main/codex-cli/codex-home-process-lock.test.ts index edb27bc83b3..e5a7eee7976 100644 --- a/src/main/codex-cli/codex-home-process-lock.test.ts +++ b/src/main/codex-cli/codex-home-process-lock.test.ts @@ -2,7 +2,6 @@ import { join } from 'node:path' import { homedir } from 'node:os' import { describe, expect, it, vi } from 'vitest' import { - CODEX_HOME_PROCESS_LOCK_MAX_HOLD_MS, resolveCodexHomeProcessLockKey, resolveCodexHomeProcessLockKeyForSpawnEnv, withCodexHomeProcessLock @@ -71,52 +70,25 @@ describe('withCodexHomeProcessLock', () => { await expect(withCodexHomeProcessLock('home-a', async () => 'after')).resolves.toBe('after') }) - it('releases the lock when a hold never settles', async () => { - vi.useFakeTimers() - try { - const events: string[] = [] - // A codex child whose surviving descendant keeps its stdio open never - // reports completion, so this hold never settles on its own. - void withCodexHomeProcessLock('home-wedged', () => new Promise(() => {})) - const queued = withCodexHomeProcessLock('home-wedged', async () => { - events.push('queued:start') - }) - - await vi.advanceTimersByTimeAsync(CODEX_HOME_PROCESS_LOCK_MAX_HOLD_MS - 1) - expect(events).toEqual([]) - - await vi.advanceTimersByTimeAsync(1) - await queued - expect(events).toEqual(['queued:start']) - } finally { - vi.useRealTimers() - } - }) - - it('measures the release cap from when a run starts, not when it is queued', async () => { + it('does not release a running lock based on elapsed time', async () => { vi.useFakeTimers() try { const events: string[] = [] const gate = deferred() - const first = withCodexHomeProcessLock('home-queued', async () => { + const first = withCodexHomeProcessLock('home-long-running', async () => { events.push('first:start') await gate.promise }) - void withCodexHomeProcessLock('home-queued', () => new Promise(() => {})) - const third = withCodexHomeProcessLock('home-queued', async () => { - events.push('third:start') + const second = withCodexHomeProcessLock('home-long-running', async () => { + events.push('second:start') }) - // Time spent waiting behind a healthy run must not burn the wedged run's cap. - await vi.advanceTimersByTimeAsync(CODEX_HOME_PROCESS_LOCK_MAX_HOLD_MS - 1_000) - gate.resolve() - await first - await vi.advanceTimersByTimeAsync(CODEX_HOME_PROCESS_LOCK_MAX_HOLD_MS - 1) + await vi.advanceTimersByTimeAsync(60 * 60_000) expect(events).toEqual(['first:start']) - await vi.advanceTimersByTimeAsync(1) - await third - expect(events).toEqual(['first:start', 'third:start']) + gate.resolve() + await Promise.all([first, second]) + expect(events).toEqual(['first:start', 'second:start']) } finally { vi.useRealTimers() } diff --git a/src/main/codex-cli/codex-home-process-lock.ts b/src/main/codex-cli/codex-home-process-lock.ts index 0d1f1224f8b..ef91a5705d9 100644 --- a/src/main/codex-cli/codex-home-process-lock.ts +++ b/src/main/codex-cli/codex-home-process-lock.ts @@ -8,13 +8,7 @@ import { normalizeRuntimePathForComparison } from '../../shared/cross-platform-p // so Orca's own spawns (quota probes, commit-message runs) serialize per home. // User terminal panes are intentionally not serialized here. -type LockTail = { - settled: Promise - /** Bounds the running hold; only needed once someone is queued behind it. */ - armReleaseCap: () => void -} - -const lockTails = new Map() +const lockTails = new Map>() export function resolveCodexHomeProcessLockKey(codexHomePath?: string | null): string { const home = codexHomePath ?? process.env.CODEX_HOME ?? join(homedir(), '.codex') @@ -43,73 +37,19 @@ export function resolveCodexHomeProcessLockKeyForSpawnEnv( return normalizeRuntimePathForComparison(codexHome ?? join(homedir(), '.codex')) } -// Why: a hold that never settles (a killed codex child whose descendant keeps -// the inherited stdio open never reports close) would kill this home for the -// rest of the session. Far above the 60s generation timeout so healthy runs and -// their process teardown never trip it. -export const CODEX_HOME_PROCESS_LOCK_MAX_HOLD_MS = 5 * 60_000 - export function withCodexHomeProcessLock(lockKey: string, fn: () => Promise): Promise { - const prior = lockTails.get(lockKey) - prior?.armReleaseCap() - const started = prior?.settled ?? Promise.resolve() - const run = started.then(fn) - const tail = createLockTail(started, run, lockKey) + const prior = lockTails.get(lockKey) ?? Promise.resolve() + const run = prior.then(fn) + // Why: keep the queue alive past a failed run so later entrants still start. + const tail = run.then( + () => undefined, + () => undefined + ) lockTails.set(lockKey, tail) - void tail.settled.then(() => { + void tail.then(() => { if (lockTails.get(lockKey) === tail) { lockTails.delete(lockKey) } }) return run } - -function createLockTail( - started: Promise, - run: Promise, - lockKey: string -): LockTail { - let capTimer: ReturnType | null = null - let running = false - let queuedBehind = false - let release!: () => void - const settled = new Promise((resolve) => { - release = () => { - running = false - if (capTimer) { - clearTimeout(capTimer) - capTimer = null - } - resolve() - } - }) - // The cap only counts time this entry actually holds the lock, so waiting - // behind a slow predecessor never shortens a run's own budget. - const armIfHolding = (): void => { - if (!running || !queuedBehind || capTimer) { - return - } - capTimer = setTimeout(() => { - console.warn( - `[codex-home-lock] releasing ${lockKey} after ${CODEX_HOME_PROCESS_LOCK_MAX_HOLD_MS}ms without completion` - ) - release() - }, CODEX_HOME_PROCESS_LOCK_MAX_HOLD_MS) - if (typeof capTimer === 'object' && 'unref' in capTimer) { - capTimer.unref() - } - } - void started.then(() => { - running = true - armIfHolding() - }) - // Why: keep the queue alive past a failed run so later entrants still start. - void run.then(release, release) - return { - settled, - armReleaseCap: () => { - queuedBehind = true - armIfHolding() - } - } -}