mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
fix(codex): preserve home lock exclusivity
This commit is contained in:
@@ -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<void>(() => {}))
|
||||
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<void>(() => {}))
|
||||
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()
|
||||
}
|
||||
|
||||
@@ -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<void>
|
||||
/** Bounds the running hold; only needed once someone is queued behind it. */
|
||||
armReleaseCap: () => void
|
||||
}
|
||||
|
||||
const lockTails = new Map<string, LockTail>()
|
||||
const lockTails = new Map<string, Promise<unknown>>()
|
||||
|
||||
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<T>(lockKey: string, fn: () => Promise<T>): Promise<T> {
|
||||
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<unknown>,
|
||||
run: Promise<unknown>,
|
||||
lockKey: string
|
||||
): LockTail {
|
||||
let capTimer: ReturnType<typeof setTimeout> | null = null
|
||||
let running = false
|
||||
let queuedBehind = false
|
||||
let release!: () => void
|
||||
const settled = new Promise<void>((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()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user