test: reap worker children and move fixture off window boundaries

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.

Upstream 2a4b8f2 has 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.
This commit is contained in:
ozymandiashh 2026-08-05 04:22:49 +03:00
parent f3f5814b84
commit 913d0bd019
2 changed files with 46 additions and 2 deletions

View file

@ -8,6 +8,24 @@ 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
@ -37,14 +55,28 @@ function waitForExit(child: ChildProcess): Promise<void> {
}
function worker(cacheDir: string, barriers: string, id: string, source: string, bypass = false): ChildProcess {
return spawn(process.execPath, ['--import', 'tsx', join(process.cwd(), 'tests/fixtures/cache-refresh-worker.ts'), cacheDir, barriers, id, source, String(bypass)], {
// 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 })))
})

View file

@ -146,7 +146,19 @@ async function createJsonlSession(
// Dated relative to now so the session stays inside the 90-day retention
// window whenever the suite runs; a fixed literal silently ages out (these
// events were `2026-05-01`, which prunes to zero once now is 90 days past it).
const base = Date.now() - 2 * 24 * 60 * 60 * 1000
// The offset is 6h rather than 48h and is clamped into the current month on
// purpose: exactly now-minus-2d sits ON the 48h 'recent' cutoff in
// optimize.ts (RECENT_WINDOW_MS — recent iff ts >= now-48h), so any clock
// skew between this helper and the 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. Six hours keeps the
// events unambiguously recent (42h clear of the cutoff) and the clamp keeps
// them inside any current-month range in any timezone.
const now = Date.now()
const monthStart = new Date(now)
monthStart.setDate(1)
monthStart.setHours(0, 0, 0, 0)
const base = Math.max(monthStart.getTime(), now - 6 * 60 * 60 * 1000)
const ts = (offsetSec: number) => new Date(base + offsetSec * 1000).toISOString()
const lines = [
JSON.stringify({ type: 'session.model_change', timestamp: ts(0), data: { newModel: 'gpt-4.1' } }),