diff --git a/scripts/lib/shard-engine.ts b/scripts/lib/shard-engine.ts index a0b3bfa03..341d8a3b6 100644 --- a/scripts/lib/shard-engine.ts +++ b/scripts/lib/shard-engine.ts @@ -12,6 +12,8 @@ import { spawn, type ChildProcess } from 'node:child_process'; import { StringDecoder } from 'node:string_decoder'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; const ROOT = path.resolve(import.meta.dir, '..', '..'); @@ -402,6 +404,18 @@ export interface RunShardChildOptions { * return reports incomplete capture instead of treating the prefix as final. */ hookStreams: (child: ChildProcess) => Array>; + /** + * Lane-owned companions of the child (the free lane's detached-browser + * tracker). Created right after spawn; `signal` runs after every forwarded + * group kill, and `settle` runs after the final group kill while parent + * signals are still forwarded. + */ + attach?: (child: ChildProcess) => ShardChildCompanion; +} + +export interface ShardChildCompanion { + signal(force: boolean): void; + settle(): Promise; } export interface ShardChildResult { @@ -436,6 +450,8 @@ export interface ShardChildResult { * call: a spawn 'error' event THROWS from here after the finally block runs, * preserving the runners' existing could-not-run handling. */ +const REAP_GRACE_MS = 250; + export async function runShardChild(options: RunShardChildOptions): Promise { const deadlineMs = Math.min(options.deadlineMs ?? Infinity, Date.now() + options.timeoutMs); if (!Number.isFinite(deadlineMs)) throw new Error('Shard deadline must be finite'); @@ -451,10 +467,12 @@ export async function runShardChild(options: RunShardChildOptions): Promise { killProcessGroup(child, (signal as NodeJS.Signals) ?? 'SIGTERM'); + companion?.signal(signal === 'SIGKILL'); return true; }, }); @@ -477,8 +495,10 @@ export async function runShardChild(options: RunShardChildOptions): Promise void; const closed = new Promise(resolve => { close = resolve; }); - const onExit = (code: number | null) => { exitCode = code; }; - const onClose = (code: number | null) => { exitCode = code; childClosed = true; close(); }; + let reap!: () => void; + const reaped = new Promise(resolve => { reap = resolve; }); + const onExit = (code: number | null) => { exitCode = code; reap(); }; + const onClose = (code: number | null) => { exitCode = code; childClosed = true; close(); reap(); }; child.once('exit', onExit); child.once('close', onClose); child.on('error', rememberError); @@ -506,10 +526,19 @@ export async function runShardChild(options: RunShardChildOptions): Promise= deadlineMs) timedOut = true; + // A wall-killed child is normally reaped within milliseconds; wait that + // long (bounded) so callers never observe a live pid after a timeout. + let reapTimer: ReturnType | undefined; + await Promise.race([reaped, new Promise(resolve => { reapTimer = setTimeout(resolve, REAP_GRACE_MS); })]); + clearTimeout(reapTimer); } finally { clearTimeout(killTimer); - forwarding.dispose(); kill(); + if (companion) { + try { await companion.settle(); } + catch (error) { rememberError(error); } + } + forwarding.dispose(); child.off('exit', onExit); child.off('close', onClose); if (!childClosed || pendingStreams > 0) { @@ -530,3 +559,157 @@ export async function runShardChild(options: RunShardChildOptions): Promise /private/var) for lanes that compare paths. + */ +export function createShardSandbox( + prefix: string, + baseEnv: NodeJS.ProcessEnv, + options: { realpath?: boolean } = {}, +): { stateDir: string; tmp: string; env: NodeJS.ProcessEnv } { + const created = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + const stateDir = options.realpath ? fs.realpathSync(created) : created; + const tmp = path.join(stateDir, 'tmp'); + fs.mkdirSync(tmp); + const env: NodeJS.ProcessEnv = { + ...baseEnv, TMPDIR: tmp, TEMP: tmp, TMP: tmp, + CHROMIUM_PROFILE: path.join(stateDir, 'chromium-profile'), + }; + return { stateDir, tmp, env }; +} + +/** + * Asynchronous best-effort backstop removal: a SIGKILLed shard can leave a + * full git workspace plus a Chromium profile, and a synchronous recursive + * delete would stall every sibling shard's classification and timers. + */ +export async function removeShardSandbox(stateDir: string): Promise { + try { await fs.promises.rm(stateDir, { recursive: true, force: true }); } + catch { /* a locked file must not turn a real verdict into an exception */ } +} + +let shardLogSequence = 0; + +/** Timestamped log path; pid + sequence defeat same-millisecond collisions. */ +export function nextShardLogPath(directory: string, stem: string): string { + const stamp = new Date().toISOString().replace(/[:.]/g, '-'); + shardLogSequence += 1; + return path.join(directory, `${stem}-${stamp}-${process.pid}-${shardLogSequence}.log`); +} + +export interface ShardLog { + readonly path: string; + readonly stream: fs.WriteStream; + /** Set on the first write error; later chunks are dropped, never thrown. */ + failed: boolean; + write(chunk: Buffer | string): void; +} + +/** Full-stream capture: every child byte is spooled to disk, never held in RAM. */ +export function openShardLog(logPath: string, label: string, mode?: number): ShardLog { + const stream = fs.createWriteStream(logPath, mode === undefined ? undefined : { mode }); + const log: ShardLog = { + path: logPath, stream, failed: false, + write(chunk) { if (!log.failed) stream.write(chunk); }, + }; + stream.on('error', (err) => { + if (log.failed) return; + log.failed = true; + console.error(`${label} could not write the full log at ${logPath}: ${err.message}`); + }); + return log; +} + +export type DurationSeedRead = + | { status: 'missing' } + | { status: 'corrupt'; error: Error } + | { status: 'ok'; durations: Record }; + +/** One reader for `{ durations: { file: ms } }` seeds; the lane decides which values count. */ +export function readDurationSeed(file: string, accepts: (ms: number) => boolean): DurationSeedRead { + let raw: string; + try { raw = fs.readFileSync(file, 'utf-8'); } + catch { return { status: 'missing' }; } + try { + const parsed = JSON.parse(raw) as { durations?: Record }; + return { status: 'ok', durations: Object.fromEntries(Object.entries(parsed.durations ?? {}) + .filter((entry): entry is [string, number] => + typeof entry[1] === 'number' && Number.isFinite(entry[1]) && accepts(entry[1]))) }; + } catch (error) { + return { status: 'corrupt', error: error as Error }; + } +} + +/** Atomic temp+rename: a killed writer never leaves a truncated seed behind. */ +export function writeDurationSeed(file: string, durations: Record): void { + const payload = { + version: 1, + recordedAt: new Date().toISOString(), + durations: Object.fromEntries(Object.entries(durations).sort(([a], [b]) => (a < b ? -1 : 1))), + }; + const temporary = `${file}.tmp-${process.pid}`; + fs.writeFileSync(temporary, `${JSON.stringify(payload, null, 2)}\n`); + fs.renameSync(temporary, file); +} + +/** What a strictly passed shard that executed zero tests becomes. */ +export type ZeroExecutionVerdict = 'passed' | 'passed-with-warning' | 'passed-empty'; + +/** Classification rules each lane injects; the engine applies them, never decides them. */ +export interface LanePolicy { + /** Which recorded seed durations the lane trusts (free >= 0, paid > 0). */ + acceptsSeedDuration(ms: number): boolean; + /** `promisedAll`: the run promised every test (EVALS_ALL) rather than a selection. */ + zeroExecution(run: { promisedAll: boolean }): ZeroExecutionVerdict; +} + +export function zeroExecutionVerdict( + executedTests: number | null, + policy: LanePolicy, + run: { promisedAll: boolean }, +): ZeroExecutionVerdict { + return executedTests === 0 ? policy.zeroExecution(run) : 'passed'; +} + +export type StrictShardStatus = 'passed' | 'failed' | 'timed-out'; + +/** + * The verdict both lanes share: a wall timeout is its own status; otherwise + * a shard passes only with complete evidence (log, capture, cleanup) AND a + * strict exit of zero, which requires bun's summary to count every planned file. + */ +export function strictShardStatus(input: { + timedOut: boolean; + evidenceComplete: boolean; + exitCode: number | null; + summary: BunTestOutputSummary; + expectedFiles: number; +}): StrictShardStatus { + if (input.timedOut) return 'timed-out'; + return input.evidenceComplete && strictTestExitCode(input.exitCode ?? 1, input.summary, input.expectedFiles) === 0 + ? 'passed' : 'failed'; +} + +/** + * Shared flag loop. Each lane declares its flags; a handler that takes a + * value calls `next()` (the following argv entry, or undefined) and owns its + * own validation message. Any undeclared flag is `Unknown argument: `. + */ +export function parseCliFlags( + argv: string[], + handlers: Record string | undefined) => void>, +): void { + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index]; + const handler = Object.hasOwn(handlers, arg) ? handlers[arg] : undefined; + if (!handler) throw new Error(`Unknown argument: ${arg}`); + handler(() => argv[++index]); + } +} diff --git a/scripts/test-free-shards.ts b/scripts/test-free-shards.ts index 1c6dcd233..445114291 100755 --- a/scripts/test-free-shards.ts +++ b/scripts/test-free-shards.ts @@ -11,9 +11,9 @@ * patterns (`/bin/bash`, `sh -c`, raw `/tmp/`, `chmod`, `xargs`). Files * that match are excluded from the Windows-safe subset — they would fail * on `windows-latest` no matter how the runner shards them. - * 4. Execution. Spawn `bun test` children and refuse to trust their exit - * code alone: every byte of output is classified through - * scripts/test-strict-output.ts, so a child that exits 0 without bun's + * 4. Execution. Run `bun test` children through the shared shard engine + * (scripts/lib/shard-engine.ts runShardChild) and refuse to trust their + * exit code alone: every byte of output is classified strictly, so a child that exits 0 without bun's * terminal summary (a mid-suite process.exit truncation), with `(fail)` * result lines, or with fewer files run than planned is a FAILURE. An * external wall-clock timeout SIGKILLs the child's process group and @@ -87,18 +87,38 @@ import { createHash, randomUUID } from 'node:crypto'; import { isPaidTestFile } from '../test/helpers/paid-test-set'; import { BunTestOutputClassifier, + createShardSandbox, exactTestFileSelectors, - installChildSignalForwarding, isTerminationRequested, killProcessGroup, BunFailureSummaryParser, + nextShardLogPath, + openShardLog, parseBunFailureResult, + parseCliFlags, normalizeRelativePath, - strictTestExitCode, + readDurationSeed, + runShardChild, + strictShardStatus, stripAnsiLine, -} from './test-strict-output'; + writeDurationSeed, + zeroExecutionVerdict, + type LanePolicy, + type ShardChildResult, +} from './lib/shard-engine'; -export { normalizeRelativePath } from './test-strict-output'; +export { normalizeRelativePath } from './lib/shard-engine'; + +/** + * Free-lane classification policy. Seeds accept zero-duration files (fast + * files are real measurements). A shard with zero executed tests passes when + * bun's summary still counted every planned file; missing or short file + * counts already fail through the strict verdict. + */ +export const FREE_LANE_POLICY: LanePolicy = { + acceptsSeedDuration: (ms) => ms >= 0, + zeroExecution: () => 'passed', +}; const ROOT = path.resolve(import.meta.dir, '..'); // design/test was silently absent from BOTH the package.json test script and @@ -669,24 +689,15 @@ export const FREE_TEST_DURATIONS_FILE = 'scripts/free-test-durations.json'; export function loadFreeTestDurations(rootDir = ROOT): Record | null { const file = process.env.GSTACK_FREE_TEST_DURATIONS ?? path.join(rootDir, FREE_TEST_DURATIONS_FILE); - let raw: string; - try { - raw = fs.readFileSync(file, 'utf-8'); - } catch { - return null; // no seed — hash sharding, silently (fresh checkouts are normal) - } - try { - const parsed = JSON.parse(raw) as { durations?: Record }; - const entries = Object.entries(parsed.durations ?? {}) - .filter((entry): entry is [string, number] => - typeof entry[1] === 'number' && Number.isFinite(entry[1]) && entry[1] >= 0); - if (entries.length === 0) return null; - return Object.fromEntries(entries); - } catch (error) { + const seed = readDurationSeed(file, FREE_LANE_POLICY.acceptsSeedDuration); + // No seed — hash sharding, silently (fresh checkouts are normal). + if (seed.status === 'missing') return null; + if (seed.status === 'corrupt') { // A corrupt seed (bad merge) must cost a warning, never the suite. - console.error(`[test:free] WARNING: corrupt durations seed ${file} (${(error as Error).message}) — falling back to hash sharding`); + console.error(`[test:free] WARNING: corrupt durations seed ${file} (${seed.error.message}) — falling back to hash sharding`); return null; } + return Object.keys(seed.durations).length === 0 ? null : seed.durations; } export interface PackedShards { @@ -889,7 +900,7 @@ type CliOptions = { results: string | null; }; -function parseCliOptions(argv: string[]): CliOptions { +export function parseCliOptions(argv: string[]): CliOptions { let dryRun = false; let listOnly = false; let recordDurations = false; @@ -903,46 +914,41 @@ function parseCliOptions(argv: string[]): CliOptions { const paths: Record<'ciPlan' | 'ciRun' | 'ciVerify' | 'result' | 'results', string | null> = { ciPlan: null, ciRun: null, ciVerify: null, result: null, results: null, }; + const pathFlag = (flag: string, key: keyof typeof paths) => (next: () => string | undefined) => { + const value = next(); + if (!value || value.startsWith('--')) throw new Error(`Missing path for ${flag}`); + paths[key] = value; + }; - for (let index = 0; index < argv.length; index += 1) { - const arg = argv[index]; - if (arg === '--dry-run') { dryRun = true; continue; } - if (arg === '--list') { listOnly = true; continue; } - if (arg === '--record-durations') { recordDurations = true; continue; } - if (arg === '--windows-only') { windowsOnly = true; continue; } - if (arg === '--verbose') { verbose = true; continue; } - if (arg === '--quick') { quick = true; continue; } - const pathKey = ({ '--ci-plan': 'ciPlan', '--ci-run': 'ciRun', '--ci-verify': 'ciVerify', '--result': 'result', '--results': 'results' } as const)[arg]; - if (pathKey) { - const value = argv[++index]; - if (!value || value.startsWith('--')) throw new Error(`Missing path for ${arg}`); - paths[pathKey] = value; - continue; - } - if (arg === '--shards') { - const value = argv[index + 1]; + parseCliFlags(argv, { + '--dry-run': () => { dryRun = true; }, + '--list': () => { listOnly = true; }, + '--record-durations': () => { recordDurations = true; }, + '--windows-only': () => { windowsOnly = true; }, + '--verbose': () => { verbose = true; }, + '--quick': () => { quick = true; }, + '--ci-plan': pathFlag('--ci-plan', 'ciPlan'), + '--ci-run': pathFlag('--ci-run', 'ciRun'), + '--ci-verify': pathFlag('--ci-verify', 'ciVerify'), + '--result': pathFlag('--result', 'result'), + '--results': pathFlag('--results', 'results'), + '--shards': (next) => { + const value = next(); if (!value) throw new Error('Missing value for --shards'); shardCount = Number.parseInt(value, 10); - index += 1; - continue; - } - if (arg === '--shard') { - const value = argv[index + 1]; + }, + '--shard': (next) => { + const value = next(); if (!value) throw new Error('Missing value for --shard'); shardIndex = Number.parseInt(value, 10); - index += 1; - continue; - } - if (arg === '--wall-timeout') { - const value = Number.parseInt(argv[index + 1] ?? '', 10); + }, + '--wall-timeout': (next) => { + const value = Number.parseInt(next() ?? '', 10); if (!Number.isInteger(value) || value <= 0) throw new Error('--wall-timeout needs a positive integer (seconds)'); wallTimeoutMs = value * 1000; wallTimeoutExplicit = true; - index += 1; - continue; - } - throw new Error(`Unknown argument: ${arg}`); - } + }, + }); const ciModes = [paths.ciPlan, paths.ciRun, paths.ciVerify].filter(Boolean).length; if (ciModes > 1 || (ciModes && (quick || listOnly || dryRun || recordDurations || windowsOnly))) throw new Error('CI modes cannot be combined with other selection modes'); @@ -1807,63 +1813,23 @@ export async function runFreeShard( // shows. Printed once at start so a wedged or noisy run is inspectable // without a re-run. const logPath = options.logFilePath ?? nextDefaultLogPath(rootDir); - const logStream = fs.createWriteStream(logPath, { mode: 0o600 }); - let logWriteFailed = false; - logStream.on('error', (err) => { - if (logWriteFailed) return; - logWriteFailed = true; - console.error(`${label} could not write the full log at ${logPath}: ${err.message}`); - }); + const shardLog = openShardLog(logPath, label, 0o600); log(`[test:free] full log: ${logPath}`); const { command, args } = options.commandFor ? options.commandFor(files) : { command: process.execPath, args: buildShardArgs(files, { parallel: options.parallel, rootDir }) }; - const env = { ...(options.env ?? process.env) }; - const stateDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-free-shard-'))); - const childTmp = path.join(stateDir, 'tmp'); - fs.mkdirSync(childTmp); - env.TMPDIR = childTmp; - env.TEMP = childTmp; - env.TMP = childTmp; + // realpath: the browser tracker refuses a state dir whose path resolves elsewhere. + const { stateDir, env } = createShardSandbox('gstack-free-shard-', options.env ?? process.env, { realpath: true }); // CLI renders otherwise attach to the repo's shared .gstack/browse.json, // even with distinct Chromium profiles. Concurrent shards and surviving // daemons from prior runs can then replace or remove each other's state. // Override inherited state too; the shard owns this directory's cleanup. env.BROWSE_STATE_FILE = path.join(stateDir, '.gstack', 'browse.json'); env.GSTACK_FREE_SHARD_ID = randomUUID(); - // Per-shard Chromium profile (same isolation idea as TMPDIR): nine test - // files launch in-process persistent contexts or daemons that default to - // the SHARED ~/.gstack/chromium-profile, and two concurrent shards on one - // profile dir kill each other's browser — observed live on CI once - // duration packing recomposed shards (handoff's launchPersistentContext - // died "Target page, context or browser has been closed" while a sibling - // shard's daemon logged "Chromium process crashed"). Hash sharding had - // masked the collision by chance placement. Within a shard, files run - // serially, so sharing the per-shard profile is safe; config tests that - // assert resolution order save/restore this env around their assertions. - env.CHROMIUM_PROFILE = path.join(stateDir, 'chromium-profile'); const startedAt = Date.now(); - const child = spawn(command, args, { - cwd: rootDir, - env, - stdio: ['ignore', 'pipe', 'pipe'], - detached: process.platform !== 'win32', - windowsHide: true, - }); - const groupPid = child.pid ?? null; - const browser = trackShardBrowser(stateDir, env); - // Group-kill on parent SIGINT/SIGTERM too, not just on timeout. - const forwarding = installChildSignalForwarding({ - kill: (signal?: NodeJS.Signals | number) => { - killProcessGroup(child, (signal as NodeJS.Signals) ?? 'SIGTERM'); - browser.signal(signal === 'SIGKILL'); - return true; - }, - }); - const classifier = new BunTestOutputClassifier(); // Console policy: quiet => nothing; verbose => the raw firehose; default => @@ -1881,14 +1847,16 @@ export async function runFreeShard( const reporter = new FreeRunReporter(files, options.verbose ? undefined : emitToConsole); const captureFailures = new Map(); - const consumeStream = (stream: typeof child.stdout, origin: StreamOrigin): Promise => - new Promise((resolve) => { + const drained = new Set(); + const consumeStream = (stream: NodeJS.ReadableStream | null, origin: StreamOrigin): Promise => + new Promise((resolve) => { if (!stream) { captureFailures.set(origin, new Error('configured pipe is missing')); resolve(); return; } - let ended = stream.readableEnded; + const readable = stream as NodeJS.ReadableStream & { readableEnded: boolean; destroyed: boolean; errored: Error | null }; + let ended = readable.readableEnded; const incomplete = (error?: Error | null): void => { // A delayed error replaces the initial destroyed-stream diagnostic // with its original cause. Failures are data, never early rejections @@ -1901,47 +1869,49 @@ export async function runFreeShard( stream.on('error', incomplete); stream.once('end', () => { ended = true; resolve(); }); stream.once('close', () => { - if (!ended) incomplete(stream.errored); + if (!ended) incomplete(readable.errored); else resolve(); }); stream.on('data', (chunk: Buffer | string) => { classifier.write(chunk, origin); // strict verdict ALWAYS sees the full stream - if (!logWriteFailed) logStream.write(chunk); + shardLog.write(chunk); reporter.write(chunk, origin); if (options.verbose) emitToConsole(typeof chunk === 'string' ? chunk : chunk.toString('utf8'), origin); }); if (ended) resolve(); - else if (stream.destroyed) incomplete(stream.errored); - }); + else if (readable.destroyed) incomplete(readable.errored); + }).then(() => { drained.add(origin); }); - let timedOut = false; - const killTimer = setTimeout(() => { - timedOut = true; - killProcessGroup(child, 'SIGKILL'); - }, wallTimeoutMs); - - let exitCode: number | null = null; - let cleanupError: string | null = null; + let child: ShardChildResult = { exitCode: null, timedOut: false, groupPid: null }; + let cleanupError = null as string | null; try { - const streams = [consumeStream(child.stdout, 'stdout'), consumeStream(child.stderr, 'stderr')]; - exitCode = await new Promise((resolve, reject) => { - child.once('error', reject); - child.once('close', (code) => resolve(code)); + // Shared spawn/detached/group-kill/wall-timer/reap lifecycle. The browser + // tracker rides along: forwarded signals reach it, and it settles after + // the final group kill. + child = await runShardChild({ + command, args, cwd: rootDir, env, timeoutMs: wallTimeoutMs, + attach: () => { + const browser = trackShardBrowser(stateDir, env); + return { signal: (force) => browser.signal(force), settle: async () => { cleanupError = await browser.settle(); } }; + }, + hookStreams: (spawned) => [consumeStream(spawned.stdout, 'stdout'), consumeStream(spawned.stderr, 'stderr')], }); - await Promise.all(streams); + } catch (error) { + child = (error as { shardResult?: ShardChildResult } | null)?.shardResult ?? child; + throw error; } finally { - clearTimeout(killTimer); - // Reap survivors of this shard even on the clean path. - killProcessGroup(child, 'SIGKILL'); - cleanupError = await browser.settle(); + // A wall-expired child is not drained: its unread tail is lost evidence. + for (const origin of ['stdout', 'stderr'] as const) { + if (!drained.has(origin) && !captureFailures.has(origin)) captureFailures.set(origin, new Error('stream did not drain before the wall deadline')); + } reporter.end(); for (const [origin, error] of captureFailures) { const diagnostic = `${label} ${origin} capture incomplete: ${error.message} ` - + `(child exit ${exitCode ?? 'signal'}). Full log: ${logPath}`; + + `(child exit ${child.exitCode ?? 'signal'}). Full log: ${logPath}`; console.error(diagnostic); - if (!logWriteFailed) logStream.write(diagnostic + '\n'); + shardLog.write(diagnostic + '\n'); } - await new Promise((resolve) => logStream.end(() => resolve())); + await new Promise((resolve) => shardLog.stream.end(() => resolve())); try { if (!cleanupError) fs.rmSync(stateDir, { recursive: true, force: true }); } catch { @@ -1949,13 +1919,18 @@ export async function runFreeShard( // Windows must not turn a real verdict into an exception. if (process.platform !== 'win32') cleanupError = 'could not remove the owned shard directory'; } - forwarding.dispose(); } + const { exitCode, timedOut, groupPid } = child; + const logWriteFailed = shardLog.failed; const summary = classifier.end(); - const status: FreeShardStatus = timedOut - ? 'timed-out' - : !cleanupError && !logWriteFailed && captureFailures.size === 0 && strictTestExitCode(exitCode ?? 1, summary, files.length) === 0 ? 'passed' : 'failed'; + let status: FreeShardStatus = strictShardStatus({ + timedOut, exitCode, summary, expectedFiles: files.length, + evidenceComplete: !cleanupError && !logWriteFailed && captureFailures.size === 0, + }); + if (status === 'passed' && zeroExecutionVerdict(reporter.report().testsRan, FREE_LANE_POLICY, { promisedAll: true }) === 'passed-empty') { + status = 'failed'; + } if (cleanupError) console.error(`${label} browser cleanup failed: ${cleanupError}; retained ${stateDir}`); @@ -2011,9 +1986,7 @@ export async function runFreeShard( return outcome; } -let logPathSequence = 0; - -/** Timestamped retained log file; pid+sequence defeat same-ms collisions. */ +/** Retained private log under .context/free-test-logs; never through a link. */ function nextDefaultLogPath(rootDir: string): string { let directory = fs.realpathSync(rootDir); for (const part of ['.context', 'free-test-logs']) { @@ -2023,12 +1996,10 @@ function nextDefaultLogPath(rootDir: string): string { if (!existing) fs.mkdirSync(directory, { mode: 0o700 }); } fs.chmodSync(directory, 0o700); - const stamp = new Date().toISOString().replace(/[:.]/g, '-'); - logPathSequence += 1; - return path.join(directory, `gstack-free-test-${stamp}-${process.pid}-${logPathSequence}.log`); + return nextShardLogPath(directory, 'gstack-free-test'); } -function exitCodeFor(status: FreeShardStatus): number { +export function exitCodeFor(status: FreeShardStatus): number { if (status === 'passed') return 0; return status === 'timed-out' ? 124 : 1; } @@ -2072,16 +2043,8 @@ async function recordFreeTestDurations(files: string[], jobs: number): Promise (a < b ? -1 : 1))), - }; - // Atomic temp+rename (capture-context-budget's pattern): a killed recorder - // must never leave a truncated seed for loadFreeTestDurations to warn on. - const tmp = `${target}.tmp-${process.pid}`; - fs.writeFileSync(tmp, `${JSON.stringify(payload, null, 2)}\n`); - fs.renameSync(tmp, target); + // Atomic: a killed recorder never leaves a truncated seed behind. + writeDurationSeed(target, durations); console.log(`[test:free] wrote ${Object.keys(durations).length} durations to ${path.relative(ROOT, target)}`); if (failed.length > 0) { // Failures still recorded (a red file's duration is still a real cost), diff --git a/scripts/test-paid-shards.ts b/scripts/test-paid-shards.ts index cd1688c2e..eb80b1887 100644 --- a/scripts/test-paid-shards.ts +++ b/scripts/test-paid-shards.ts @@ -38,8 +38,9 @@ * * Enumeration matches package.json's `test:gate` globs (via the shared * test/helpers/paid-test-set.ts) and honors EVALS_TIER against the E2E_TIERS - * map in test/helpers/touchfiles.ts. Output classification reuses - * scripts/test-strict-output.ts rather than reimplementing it. + * map in test/helpers/touchfiles.ts. Spawn, kill, sandbox, logs, seeds and + * output classification come from the shared shard engine + * (scripts/lib/shard-engine.ts); this file keeps only paid-lane policy. * * Parallelism now lives ACROSS shards (--jobs), not inside one Bun process, so * each shard runs its own file sequentially and can be killed independently. @@ -57,14 +58,23 @@ import { spawnSync } from 'node:child_process'; import { createBootstrapRetentionScope } from '../test/helpers/bootstrap-retention'; import { BunTestOutputClassifier, + createShardSandbox, exactTestFileSelectors, forwardAndClassify, isTerminationRequested, + nextShardLogPath, normalizeRelativePath, + openShardLog, + parseCliFlags, + readDurationSeed, + removeShardSandbox, runShardChild, - strictTestExitCode, + strictShardStatus, + writeDurationSeed, + zeroExecutionVerdict, + type LanePolicy, type ShardChildResult, -} from './test-strict-output'; +} from './lib/shard-engine'; import { PAID_TEST_GLOBS, isPaidTestFile } from '../test/helpers/paid-test-set'; import { PERIODIC_CI_EXCLUDE } from '../test/helpers/periodic-exclude-data'; import { FILE_RETRY_BUDGETS, STRICT_RETRY_CASE_BUDGETS } from '../test/helpers/eval-budgets'; @@ -110,6 +120,17 @@ export const DEFAULT_MAX_FILES_PER_SHARD = 1; export const DEFAULT_JOBS = 8; export const DEFAULT_WITHIN_SHARD_CONCURRENCY = 2; +/** + * Paid-lane classification policy. Seeds keep only positive walls (a zero + * is not a real paid-shard measurement). A shard that passed with zero + * executed tests is legitimate under selection (in-file diff/tier + * self-skips) and only warns; under EVALS_ALL it is hollow: 'passed-empty'. + */ +export const PAID_LANE_POLICY: LanePolicy = { + acceptsSeedDuration: (ms) => ms > 0, + zeroExecution: ({ promisedAll }) => (promisedAll ? 'passed-empty' : 'passed-with-warning'), +}; + /** One overlay process preserves the original process-wide SDK semaphore. */ export const OVERLAY_MAX_ACTIVE_SHARDS = 1; @@ -654,15 +675,6 @@ export interface RunShardsOptions { casePatterns?: Record; } -let shardLogSequence = 0; - -/** Per-shard log path: slug + timestamp; pid + sequence defeat same-ms collisions. */ -function nextShardLogPath(files: string[], logDir: string): string { - const stamp = new Date().toISOString().replace(/[:.]/g, '-'); - shardLogSequence += 1; - return path.join(logDir, `gstack-paid-shard-${shardSlug(files)}-${stamp}-${process.pid}-${shardLogSequence}.log`); -} - /** On-failure console excerpt budget: the last N bytes of the shard's log. */ export const FAILURE_TAIL_BYTES = 64 * 1024; @@ -712,15 +724,15 @@ export async function runPaidShard( ), ...(options.casePatterns ? ['--test-name-pattern', options.casePatterns[files[0]]] : [])], }; - const env = { ...(options.env ?? process.env) }; + const baseEnv = { ...(options.env ?? process.env) }; if (options.evalDirBase) { - env.GSTACK_EVAL_DIR = path.join(options.evalDirBase, 'shards', shardSlug(files)); + baseEnv.GSTACK_EVAL_DIR = path.join(options.evalDirBase, 'shards', shardSlug(files)); } // Resolve `claude --version` ONCE in the parent (cached across shards) and // hand it to every child: eval-store's fallback is a synchronous spawn on // the same thread that polls PTY sessions, so children must never pay it. - if (!env.GSTACK_CLAUDE_CLI_VERSION) { - env.GSTACK_CLAUDE_CLI_VERSION = getClaudeCliVersion(); + if (!baseEnv.GSTACK_CLAUDE_CLI_VERSION) { + baseEnv.GSTACK_CLAUDE_CLI_VERSION = getClaudeCliVersion(); } // Per-shard temp + Chromium-profile isolation — the free runner treats // this as mandatory (test-free-shards.ts: two concurrent shards on one @@ -731,13 +743,8 @@ export async function runPaidShard( // stopping wedged runs from accumulating full git-repo workspaces in the // shared tmpdir forever. Prerequisite for raising EVALS_JOBS (more // concurrency on shared state amplifies exactly the opus-47 race class). - const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-paid-shard-')); - const childTmp = path.join(stateDir, 'tmp'); - fs.mkdirSync(childTmp); - env.TMPDIR = childTmp; - env.TEMP = childTmp; - env.TMP = childTmp; - env.CHROMIUM_PROFILE = path.join(stateDir, 'chromium-profile'); + const sandbox = createShardSandbox('gstack-paid-shard-', baseEnv); + const { stateDir, tmp: childTmp, env } = sandbox; const bootstrapFile = files.some(file => normalizeRelativePath(file) === 'test/skill-e2e-qa-workflow.test.ts'); delete env.GSTACK_BOOTSTRAP_RETENTION; if (bootstrapFile && process.platform !== 'linux') log(`${label} bootstrap dependency retention unavailable on ${process.platform}; native behavior still runs without retained-dependency qualification`); @@ -754,14 +761,9 @@ export async function runPaidShard( // model), never in a whole-run Buffer[] — non-live shards used to hold // their entire 30-min stream-json stdout+stderr in RAM, × concurrent jobs. // Printed at START so a wedged shard is inspectable live, mid-run. - const logPath = nextShardLogPath(files, options.logDir ?? os.tmpdir()); - const logStream = fs.createWriteStream(logPath); - let logWriteFailed = false; - logStream.on('error', (err) => { - if (logWriteFailed) return; - logWriteFailed = true; - console.error(`${label} could not write the full log at ${logPath}: ${err.message}`); - }); + const logPath = nextShardLogPath(options.logDir ?? os.tmpdir(), `gstack-paid-shard-${shardSlug(files)}`); + const spool = openShardLog(logPath, label); + const logStream = spool.stream; log(`${label} full log: ${logPath}`); const classifier = new BunTestOutputClassifier(); @@ -770,7 +772,7 @@ export async function runPaidShard( // verdict path is unchanged by where the bytes land afterwards. const sink = (destination: NodeJS.WriteStream): NodeJS.WriteStream => ({ write: (chunk: Buffer | string): boolean => { - if (!logWriteFailed) logStream.write(chunk); + spool.write(chunk); if (streamLive) destination.write(chunk); return true; }, @@ -822,21 +824,21 @@ export async function runPaidShard( logStream.off('error', onError); logStream.off('close', onClose); if (!complete) { - logWriteFailed = true; + spool.failed = true; logStream.destroy(); log(`${label} incomplete log capture; retained prefix: ${logPath}`); } resolve(); }; const onError = () => finish(false); - const onClose = () => finish(logStream.writableFinished && !logWriteFailed); + const onClose = () => finish(logStream.writableFinished && !spool.failed); const expire = () => { timedOut = true; finish(false); }; logStream.once('error', onError); logStream.once('close', onClose); if (Date.now() >= shardDeadline) { expire(); return; } - if (logWriteFailed || logStream.destroyed) { finish(false); return; } + if (spool.failed || logStream.destroyed) { finish(false); return; } timer = setTimeout(expire, shardDeadline - Date.now()); - try { logStream.end(() => finish(logStream.writableFinished && !logWriteFailed)); } + try { logStream.end(() => finish(logStream.writableFinished && !spool.failed)); } catch { finish(false); } }); let retentionRemovable = true; @@ -852,16 +854,8 @@ export async function runPaidShard( log(`${label} bootstrap retention acknowledgment failed; preserving shard state`); } } - try { - // async rm: a SIGKILLed shard can leave a full git workspace + Chromium - // profile here; a synchronous recursive delete on the parent's event - // loop would stall every sibling shard's stream classification and - // wall timers for seconds (review finding). - if (retentionRemovable) await fs.promises.rm(stateDir, { recursive: true, force: true }); - } catch { - // Best-effort: a locked file must not turn a real verdict into an - // exception (same posture as the free runner's cleanup). - } + // Async, best-effort backstop: group-SIGKILLed tests never clean up. + if (retentionRemovable) await removeShardSandbox(stateDir); } const summary = classifier.end(); @@ -875,9 +869,10 @@ export async function runPaidShard( // commands must print a synthetic `Ran N tests across M files. [Xms]` line, // so tests can pin the summary-missing => failure backstop. const expectedFiles = files.length; - let status: ShardStatus = timedOut - ? 'timed-out' - : !retentionFailed && !logWriteFailed && !incompleteCapture && strictTestExitCode(exitCode ?? 1, summary, expectedFiles) === 0 ? 'passed' : 'failed'; + let status: ShardStatus = strictShardStatus({ + timedOut, exitCode, summary, expectedFiles, + evidenceComplete: !retentionFailed && !spool.failed && !incompleteCapture, + }); if (status === 'passed' && options.expectedCases) { const expected = files.reduce((count, file) => count + (options.expectedCases![file] ?? 0), 0); const actual = summary.terminalTestCounts.reduce((count, value) => count + value, 0) - summary.skippedTests; @@ -950,12 +945,13 @@ export function applyHollowShardGuard( (outcome.executedTests === null || outcome.executedTests === 0 || isAllSkippedPass(outcome))) { return { ...outcome, status: 'passed-empty' }; } - if (outcome.status !== 'passed' || outcome.executedTests !== 0) return outcome; - if (!opts.evalsAll) { + if (outcome.status !== 'passed') return outcome; + const verdict = zeroExecutionVerdict(outcome.executedTests, PAID_LANE_POLICY, { promisedAll: opts.evalsAll }); + if (verdict === 'passed-with-warning') { warn(`[test:paid] WARNING: shard ${outcome.shard} passed with 0 executed tests (${outcome.files.join(' ')}) — legitimate under selection, hollow under EVALS_ALL`); return outcome; } - return { ...outcome, status: 'passed-empty' }; + return verdict === 'passed-empty' ? { ...outcome, status: 'passed-empty' } : outcome; }); } @@ -1114,13 +1110,8 @@ export const PAID_TEST_DURATIONS_FILE = 'scripts/paid-test-durations.json'; * missing or corrupt seed keeps the supervision-budget allocation. */ export function loadPaidTestDurations(rootDir = ROOT): Record { - try { - const parsed = JSON.parse(fs.readFileSync(path.join(rootDir, PAID_TEST_DURATIONS_FILE), 'utf8')) as { durations?: Record }; - return Object.fromEntries(Object.entries(parsed.durations ?? {}) - .filter((entry): entry is [string, number] => typeof entry[1] === 'number' && Number.isFinite(entry[1]) && entry[1] > 0)); - } catch { - return {}; - } + const seed = readDurationSeed(path.join(rootDir, PAID_TEST_DURATIONS_FILE), PAID_LANE_POLICY.acceptsSeedDuration); + return seed.status === 'ok' ? seed.durations : {}; } /** Merge a report's executed single-file outcomes into the seed; all-skipped shards carry no cost signal. */ @@ -1556,44 +1547,32 @@ export function parseCliOptions(argv: string[], env: NodeJS.ProcessEnv = process writeDurations: false, }; - for (let index = 0; index < argv.length; index += 1) { - const arg = argv[index]; - if (arg === '--list') { options.listOnly = true; continue; } - if (arg === '--tier') { - const value = argv[index += 1]; + const pathValue = (message: string, assign: (value: string) => void) => (next: () => string | undefined) => { + const value = next(); + if (!value) throw new Error(message); + assign(value); + }; + parseCliFlags(argv, { + '--list': () => { options.listOnly = true; }, + '--tier': (next) => { + const value = next(); if (value !== 'gate' && value !== 'periodic') throw new Error(`--tier must be gate or periodic. Received: ${value}`); options.tier = value; - continue; - } - if (arg === '--profile') { - const value = argv[index += 1]; - if (!value) throw new Error('--profile needs pr or full'); - options.profile = validatedProfile(value, '--profile'); options.profileExplicit = true; continue; - } - if (arg === '--timeout') { options.timeoutMs = parsePositiveInt(argv[index += 1], '--timeout') * 1000; options.timeoutExplicit = true; continue; } - if (arg === '--jobs') { options.jobs = parsePositiveInt(argv[index += 1], '--jobs'); continue; } - if (arg === '--files-per-shard') { options.maxFilesPerShard = parsePositiveInt(argv[index += 1], '--files-per-shard'); continue; } - if (arg === '--emit-plan') { - const value = argv[index += 1]; - if (!value) throw new Error('--emit-plan needs a file path'); - options.emitPlanPath = value; continue; - } - if (arg === '--skip-judges') { options.skipJudges = true; continue; } - if (arg === '--slices') { options.slices = parsePositiveInt(argv[index += 1], '--slices'); continue; } - if (arg === '--plan') { - const value = argv[index += 1]; - if (!value) throw new Error('--plan needs a manifest path'); - options.planPath = value; continue; - } - if (arg === '--slice') { options.sliceIndex = parsePositiveInt(argv[index += 1], '--slice'); continue; } - if (arg === '--report') { - const value = argv[index += 1]; - if (!value) throw new Error('--report needs a directory'); - options.reportDir = value; continue; - } - if (arg === '--write-durations') { options.writeDurations = true; continue; } - throw new Error(`Unknown argument: ${arg}`); - } + }, + '--profile': pathValue('--profile needs pr or full', (value) => { + options.profile = validatedProfile(value, '--profile'); options.profileExplicit = true; + }), + '--timeout': (next) => { options.timeoutMs = parsePositiveInt(next(), '--timeout') * 1000; options.timeoutExplicit = true; }, + '--jobs': (next) => { options.jobs = parsePositiveInt(next(), '--jobs'); }, + '--files-per-shard': (next) => { options.maxFilesPerShard = parsePositiveInt(next(), '--files-per-shard'); }, + '--emit-plan': pathValue('--emit-plan needs a file path', (value) => { options.emitPlanPath = value; }), + '--skip-judges': () => { options.skipJudges = true; }, + '--slices': (next) => { options.slices = parsePositiveInt(next(), '--slices'); }, + '--plan': pathValue('--plan needs a manifest path', (value) => { options.planPath = value; }), + '--slice': (next) => { options.sliceIndex = parsePositiveInt(next(), '--slice'); }, + '--report': pathValue('--report needs a directory', (value) => { options.reportDir = value; }), + '--write-durations': () => { options.writeDurations = true; }, + }); if (options.writeDurations && !options.reportDir) throw new Error('--write-durations requires --report'); if (options.skipJudges && (!options.emitPlanPath || options.tier !== 'gate')) throw new Error('--skip-judges applies only to an emitted gate census plan'); if (options.profile === 'pr' && options.tier !== 'gate') throw new Error('PR profile requires gate tier'); @@ -1648,10 +1627,7 @@ async function main(): Promise { } if (options.writeDurations) { const durations = mergePaidTestDurations(loadPaidTestDurations(), results); - const target = path.join(ROOT, PAID_TEST_DURATIONS_FILE); - const temporary = `${target}.tmp-${process.pid}`; - fs.writeFileSync(temporary, `${JSON.stringify({ version: 1, recordedAt: new Date().toISOString(), durations }, null, 2)}\n`); - fs.renameSync(temporary, target); + writeDurationSeed(path.join(ROOT, PAID_TEST_DURATIONS_FILE), durations); console.log(`[test:paid] wrote ${Object.keys(durations).length} durations to ${PAID_TEST_DURATIONS_FILE}`); } // Historical flaky_retries includes every case with multiple attempts, diff --git a/test/paid-free-boundary.test.ts b/test/paid-free-boundary.test.ts index 71a5350d3..3186a23f1 100644 --- a/test/paid-free-boundary.test.ts +++ b/test/paid-free-boundary.test.ts @@ -64,7 +64,7 @@ describe('paid/free dependency boundary', () => { expect(paid.length).toBeGreaterThan(0); const all = workflowJudgeDependencies(ROOT, paid); for (const dependencies of [runner, all]) { - expect(dependencies).toContain('scripts/test-strict-output.ts'); + expect(dependencies).toContain('scripts/lib/shard-engine.ts'); expect(dependencies).toContain('test/helpers/test-selection.ts'); expect(dependencies).not.toContain('scripts/eval-flake-rank.ts'); for (const freeOnly of FREE_ONLY_PR_FILES) expect(dependencies).not.toContain(freeOnly);