mirror of
https://github.com/AgentSeal/codeburn.git
synced 2026-08-25 00:14:40 +00:00
Second-reviewer findings on top of the port (f3f5814). Test-only. cache-refresh-lock-process: afterEach removed the temp roots but never killed the spawned workers, which block on their barrier files indefinitely. The success path reaps them (waitForExit after each run), but a failed assertion or waitFor timeout leaves the blocked winner behind; with the global retry: 2, every attempt then spawns a fresh pair on top of the leaked ones, so they accumulate. Verified: a forced assertion failure left 3 stray worker processes after the run before this fix, and 0 after, with the kill path running between retry attempts. afterEach now kills every child it spawned and waits for it to actually die (SIGTERM, then SIGKILL after 1s), is robust to children that already exited (exitCode !== null short-circuits), and detaches waitForExit listeners first so a SIGTERM cannot surface an unhandled rejection on top of the real failure. Upstream2a4b8f2has the identical leaky afterEach, so this is not a regression the port introduces; it is a latent leak the retry makes reachable. Same file: worker() resolved its fixture relative to process.cwd(), so `vitest run --root packages/cli` from the repo root spawned children at a nonexistent tests/fixtures path and every one died on ENOENT before touching a barrier. Resolve the fixture relative to this file (import.meta.dirname) so the suite works from any invocation cwd. parser.test.ts createJsonlSession: the fixture dated events at exactly now minus two days. That is safely inside the 90-day retention window but sits exactly ON the 48h 'recent' cutoff in optimize.ts (RECENT_WINDOW_MS — recent iff ts >= now-48h), so any clock skew between the helper and a consumer flips the classification, and a current-month date range (cli-date.ts `month`, which starts at local midnight of the 1st) would exclude it on the 1st-2nd. Moved the offset to 6h, clamped into the current month. Six hours keeps the events unambiguously recent (42h clear of the cutoff) and inside retention with ~89 days of headroom; the clamp keeps them inside any current-month range in any timezone (it uses the same local-calendar construction as cli-date.ts). Both existing cases that use this helper, (a) and (f), pass with the new offset.
199 lines
9.4 KiB
TypeScript
199 lines
9.4 KiB
TypeScript
import { afterEach, describe, expect, it } from 'vitest'
|
|
import { spawn, type ChildProcess } from 'child_process'
|
|
import { existsSync } from 'fs'
|
|
import { mkdir, mkdtemp, readdir, rm, stat, utimes, writeFile } from 'fs/promises'
|
|
import { tmpdir } from 'os'
|
|
import { join } from 'path'
|
|
|
|
import { emptyCache, loadCache, saveCache } from '../src/session-cache.js'
|
|
|
|
const roots: string[] = []
|
|
const children: ChildProcess[] = []
|
|
|
|
async function killChild(child: ChildProcess): Promise<void> {
|
|
// Robust to a child that already exited: exitCode is set once the process
|
|
// is gone, so the early return skips it and the kill below is a no-op.
|
|
if (child.exitCode !== null) return
|
|
// A test that failed mid-flight can still hold pending waitForExit promises
|
|
// on this child; detach them so the SIGTERM exit does not reject a promise
|
|
// nobody is awaiting (unhandled-rejection noise on top of the real failure).
|
|
child.removeAllListeners('exit')
|
|
child.removeAllListeners('error')
|
|
const exited = new Promise<void>(resolve => child.once('exit', () => resolve()))
|
|
if (child.exitCode !== null) return // exited between the check and the listener
|
|
if (!child.kill()) return
|
|
await Promise.race([exited, new Promise(resolve => { setTimeout(resolve, 1_000) })])
|
|
if (child.exitCode === null) child.kill('SIGKILL')
|
|
await Promise.race([exited, new Promise(resolve => { setTimeout(resolve, 1_000) })])
|
|
}
|
|
|
|
async function waitFor(path: string, timeoutMs = 5_000): Promise<void> {
|
|
const deadline = Date.now() + timeoutMs
|
|
while (!existsSync(path)) {
|
|
if (Date.now() >= deadline) throw new Error(`timed out waiting for ${path}`)
|
|
await new Promise(resolve => { setTimeout(resolve, 5) })
|
|
}
|
|
}
|
|
|
|
async function waitForAny(dir: string, names: string[], timeoutMs = 5_000): Promise<string> {
|
|
const deadline = Date.now() + timeoutMs
|
|
while (Date.now() < deadline) {
|
|
for (const name of names) if (existsSync(join(dir, name))) return name
|
|
await new Promise(resolve => { setTimeout(resolve, 5) })
|
|
}
|
|
throw new Error(`timed out waiting for ${names.join(', ')}; saw ${(await readdir(dir)).join(', ')}`)
|
|
}
|
|
|
|
function waitForExit(child: ChildProcess): Promise<void> {
|
|
if (child.exitCode !== null) return child.exitCode === 0 ? Promise.resolve() : Promise.reject(new Error(`worker exited ${child.exitCode}`))
|
|
return new Promise((resolve, reject) => {
|
|
let stderr = ''
|
|
child.stderr?.on('data', chunk => { stderr += String(chunk) })
|
|
child.once('error', reject)
|
|
child.once('exit', code => code === 0 ? resolve() : reject(new Error(`worker exited ${code}: ${stderr}`)))
|
|
})
|
|
}
|
|
|
|
function worker(cacheDir: string, barriers: string, id: string, source: string, bypass = false): ChildProcess {
|
|
// Resolve the fixture relative to this file, not process.cwd(): vitest keeps
|
|
// the shell's cwd (e.g. the repo root under `--root packages/cli`), where
|
|
// `tests/fixtures/` does not exist and every child dies on ENOENT before
|
|
// touching a barrier.
|
|
const child = spawn(process.execPath, ['--import', 'tsx', join(import.meta.dirname, 'fixtures/cache-refresh-worker.ts'), cacheDir, barriers, id, source, String(bypass)], {
|
|
cwd: process.cwd(),
|
|
stdio: ['ignore', 'ignore', 'pipe'],
|
|
})
|
|
children.push(child)
|
|
return child
|
|
}
|
|
|
|
afterEach(async () => {
|
|
delete process.env['CODEBURN_CACHE_DIR']
|
|
// A failed assertion (or a waitFor timeout) can leave a worker blocked on
|
|
// its barrier file indefinitely; with the global retry the next attempt
|
|
// spawns a fresh pair on top of the leaked ones. Kill everything we
|
|
// spawned and wait for it to die before tearing the temp roots down.
|
|
// Upstream 2a4b8f2 has the identical leaky afterEach, so this is not a
|
|
// regression the port introduces — the retry just makes the latent leak
|
|
// reachable.
|
|
await Promise.all(children.splice(0).map(killChild))
|
|
await Promise.all(roots.splice(0).map(root => rm(root, { recursive: true, force: true })))
|
|
})
|
|
|
|
describe('warm refresh child-process regression', () => {
|
|
it('gives exactly one contender ownership of a stale lock', async () => {
|
|
const root = await mkdtemp(join(tmpdir(), 'cb-refresh-stale-'))
|
|
roots.push(root)
|
|
const cacheDir = join(root, 'cache')
|
|
const barriers = join(root, 'barriers')
|
|
await mkdir(cacheDir, { recursive: true })
|
|
await mkdir(barriers, { recursive: true })
|
|
process.env['CODEBURN_CACHE_DIR'] = cacheDir
|
|
const initial = emptyCache()
|
|
initial.complete = true
|
|
await saveCache(initial)
|
|
const stalePath = join(cacheDir, 'session-refresh.lock')
|
|
await writeFile(stalePath, JSON.stringify({ pid: 1, token: 'abandoned', at: 1 }))
|
|
await utimes(stalePath, new Date(1), new Date(1))
|
|
const source = join(root, 'changed.json')
|
|
await writeFile(source, JSON.stringify({ output: 303 }))
|
|
|
|
const a = worker(cacheDir, barriers, 'a', source)
|
|
const b = worker(cacheDir, barriers, 'b', source)
|
|
const winner = await Promise.race([
|
|
waitFor(join(barriers, 'a.parsed')).then(() => 'a'),
|
|
waitFor(join(barriers, 'b.parsed')).then(() => 'b'),
|
|
])
|
|
const loser = winner === 'a' ? 'b' : 'a'
|
|
expect(Number(existsSync(join(barriers, 'a.parsed'))) + Number(existsSync(join(barriers, 'b.parsed')))).toBe(1)
|
|
// Keep the winner alive through the loser's full waiter budget: a second
|
|
// stale contender must never publish or steal a heartbeating successor.
|
|
const loserOutcome = await waitForAny(barriers, [
|
|
`${loser}.timed-out`, `${loser}.parsed`, `${loser}.completed-by-other`, `${loser}.unavailable`,
|
|
])
|
|
expect(loserOutcome, (await readdir(barriers)).join(',')).toBe(`${loser}.timed-out`)
|
|
await writeFile(join(barriers, `${winner}.save`), '')
|
|
await Promise.all([waitForExit(a), waitForExit(b)])
|
|
await expect(stat(join(cacheDir, 'session-refresh.lock.takeover'))).rejects.toMatchObject({ code: 'ENOENT' })
|
|
})
|
|
|
|
it('serializes disjoint parsed updates so the later publication cannot drop the first', async () => {
|
|
const root = await mkdtemp(join(tmpdir(), 'cb-refresh-process-'))
|
|
roots.push(root)
|
|
const cacheDir = join(root, 'cache')
|
|
const barriers = join(root, 'barriers')
|
|
await mkdir(cacheDir, { recursive: true })
|
|
await mkdir(barriers, { recursive: true })
|
|
process.env['CODEBURN_CACHE_DIR'] = cacheDir
|
|
const initial = emptyCache()
|
|
initial.complete = true
|
|
await saveCache(initial)
|
|
|
|
const sourceA = join(root, 'changed-a.json')
|
|
const sourceB = join(root, 'changed-b.json')
|
|
await writeFile(sourceA, JSON.stringify({ output: 101 }))
|
|
await writeFile(sourceB, JSON.stringify({ output: 202 }))
|
|
|
|
const a = worker(cacheDir, barriers, 'a', sourceA)
|
|
const b = worker(cacheDir, barriers, 'b', sourceB)
|
|
|
|
// Exactly one child can cross the acquisition barrier. Let it publish and
|
|
// release; only then can the other reload the first child's update.
|
|
let retry: ChildProcess | undefined
|
|
await Promise.race([
|
|
waitFor(join(barriers, 'a.parsed')).then(() => 'a'),
|
|
waitFor(join(barriers, 'b.parsed')).then(() => 'b'),
|
|
]).then(async first => {
|
|
const second = first === 'a' ? 'b' : 'a'
|
|
const secondSource = second === 'a' ? sourceA : sourceB
|
|
await writeFile(join(barriers, `${first}.save`), '')
|
|
await waitFor(join(barriers, `${first}.published`))
|
|
// A waiter that observes the clean release correctly serves the holder's
|
|
// fresh snapshot instead of mutating. A subsequent refresh of that
|
|
// process then acquires normally and applies its independently visible
|
|
// source change on top of the holder's publication.
|
|
const outcome = await waitForAny(barriers, [`${second}.completed-by-other`, `${second}.parsed`])
|
|
if (outcome === `${second}.parsed`) {
|
|
await writeFile(join(barriers, `${second}.save`), '')
|
|
} else {
|
|
await waitForExit(second === 'a' ? a : b)
|
|
retry = worker(cacheDir, barriers, `${second}-retry`, secondSource)
|
|
await waitFor(join(barriers, `${second}-retry.parsed`))
|
|
await writeFile(join(barriers, `${second}-retry.save`), '')
|
|
}
|
|
})
|
|
|
|
await Promise.all([waitForExit(a), waitForExit(b), ...(retry ? [waitForExit(retry)] : [])])
|
|
const files = (await loadCache()).providers['regression']?.files ?? {}
|
|
expect(Object.keys(files).sort()).toEqual([sourceA, sourceB].sort())
|
|
})
|
|
|
|
it('proves the barrier reproducer loses one update when the transaction gate is bypassed', async () => {
|
|
const root = await mkdtemp(join(tmpdir(), 'cb-refresh-control-'))
|
|
roots.push(root)
|
|
const cacheDir = join(root, 'cache')
|
|
const barriers = join(root, 'barriers')
|
|
await mkdir(cacheDir, { recursive: true })
|
|
await mkdir(barriers, { recursive: true })
|
|
process.env['CODEBURN_CACHE_DIR'] = cacheDir
|
|
const initial = emptyCache()
|
|
initial.complete = true
|
|
await saveCache(initial)
|
|
|
|
const sourceA = join(root, 'changed-a.json')
|
|
const sourceB = join(root, 'changed-b.json')
|
|
await writeFile(sourceA, JSON.stringify({ output: 101 }))
|
|
await writeFile(sourceB, JSON.stringify({ output: 202 }))
|
|
const a = worker(cacheDir, barriers, 'a', sourceA, true)
|
|
const b = worker(cacheDir, barriers, 'b', sourceB, true)
|
|
await Promise.all([waitFor(join(barriers, 'a.parsed')), waitFor(join(barriers, 'b.parsed'))])
|
|
|
|
await writeFile(join(barriers, 'a.save'), '')
|
|
await waitFor(join(barriers, 'a.published'))
|
|
await writeFile(join(barriers, 'b.save'), '')
|
|
await Promise.all([waitForExit(a), waitForExit(b)])
|
|
|
|
const files = (await loadCache()).providers['regression']?.files ?? {}
|
|
expect(Object.keys(files)).toEqual([sourceB])
|
|
})
|
|
})
|