From b76fab349faa69f448bba784ae08124a60afadba Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 30 Sep 2026 05:46:20 +0000 Subject: [PATCH 1/6] refactor(shard-engine): move scripts/test-strict-output.ts to scripts/lib/shard-engine.ts The shared shard engine grows from the existing strict-output module (runShardChild, killProcessGroup, signal forwarding, strict classifier). scripts/test-strict-output.ts stays as a re-export so existing importers, mock.module paths and the strict-output/run-shard-child tests are unchanged. The engine inherits the global touchfile entry; the free runner's CLI-routing fixture copies the new module. --- scripts/lib/shard-engine.ts | 532 ++++++++++++++++++++++++++++++++ scripts/test-strict-output.ts | 530 +------------------------------ test/helpers/touchfiles-data.ts | 1 + test/test-free-shards.test.ts | 2 +- 4 files changed, 538 insertions(+), 527 deletions(-) create mode 100644 scripts/lib/shard-engine.ts diff --git a/scripts/lib/shard-engine.ts b/scripts/lib/shard-engine.ts new file mode 100644 index 000000000..a0b3bfa03 --- /dev/null +++ b/scripts/lib/shard-engine.ts @@ -0,0 +1,532 @@ +/** + * Shard engine: the one owner of how a test shard runs — process-group spawn, + * wall timeout + group kill, signal forwarding, strict Bun output verdicts, + * per-shard tmp/Chromium sandbox, log files, duration seeds, CLI flag loop. + * Lanes (scripts/test-free-shards.ts, scripts/test-paid-shards.ts) keep only + * policy: selection, budgets, manifests, retries, and a LanePolicy, e.g. + * const LANE: LanePolicy = { acceptsSeedDuration: ms => ms > 0, zeroExecution: () => 'passed' }; + * const result = await runShardChild({ command, args, cwd, env, timeoutMs, hookStreams }); + * Enforced by ratchet (c) (test/module-size-ratchet.test.ts). Moved from + * scripts/test-strict-output.ts, which re-exports this module. + */ + +import { spawn, type ChildProcess } from 'node:child_process'; +import { StringDecoder } from 'node:string_decoder'; +import * as path from 'node:path'; + +const ROOT = path.resolve(import.meta.dir, '..', '..'); +// Strict Bun-test output classification works around a Bun test runner bug +// where failures can be printed even though the child exits successfully: +// output is forwarded byte-for-byte as it arrives, only complete Bun result +// lines and terminal summaries are classified, and `strictTestExitCode` +// refuses a zero exit when the output shows failures or fewer files ran. +const ANSI_ESCAPE = /\u001B\[[0-?]*[ -/]*[@-~]/g; +const BUN_FAIL_RESULT = /^(?:\(fail\)|✗) (.+) \[(?:\d+(?:\.\d+)?)(?:ns|us|µs|ms|s)\]$/; +const BUN_BETWEEN_TESTS_ERROR = '# Unhandled error between tests'; +const BUN_TERMINAL_SUMMARY = /^Ran (\d+) tests? across (\d+) files?\. \[(?:\d+(?:\.\d+)?)(?:ns|us|µs|ms|s)\]$/; +// The counts block bun prints just before the terminal summary (" 1 pass", +// " 2 skip", " 0 fail"). "Ran N tests" COUNTS skipped tests, so N alone +// cannot distinguish a shard that verified work from one whose every test +// self-skipped (external-service binary missing, tier mismatch) — the +// green-by-skip class. Anchored to whole-line matches; nested bun-test +// children can still contribute counts (same known limit as the terminal +// summary — see the last-summary-anchoring TODO in the audit). +const BUN_SKIP_COUNT = /^\s*(\d+) skip$/; +const BUN_PASS_COUNT = /^\s*(\d+) pass$/; +const BUN_FAIL_COUNT = /^\s*(\d+) fail$/; + +export type BunTestOutputFinding = 'failed-test' | 'unhandled-between-tests'; + +export interface BunTestOutputSummary { + failedTests: number; + unhandledBetweenTests: number; + terminalFileCounts: number[]; + /** Test counts from the same terminal lines — feeds the hollow-shard guard. */ + terminalTestCounts: number[]; + /** Sum of bun's " N skip" count lines. "Ran N tests" includes skips, so + * this is what separates verified work from green-by-skip. */ + skippedTests: number; + /** Sum of bun's " N pass" count lines. */ + passedTests: number; +} + +export type ForwardedTerminationSignal = 'SIGINT' | 'SIGTERM'; + +export interface TerminationSignalSource { + on(event: string, listener: () => void): unknown; + off(event: string, listener: () => void): unknown; +} + +export interface TerminationTimerApi { + schedule(callback: () => void, delayMs: number): unknown; + cancel(handle: unknown): void; +} + +export interface ChildSignalForwarding { + readonly receivedSignal: ForwardedTerminationSignal | null; + dispose(): void; +} + +const DEFAULT_TERMINATION_TIMER: TerminationTimerApi = { + schedule: (callback, delayMs) => setTimeout(callback, delayMs), + cancel: (handle) => clearTimeout(handle as ReturnType), +}; + +/** + * Per-source termination bookkeeping, shared across every forwarder bound to + * the same source. Installing ANY signal listener suppresses Node's default + * terminate-on-SIGINT/SIGTERM, so without this the parent runner survived + * cancellation: it killed the current child, then kept LAUNCHING new shards + * (observed: paid runs continuing to burn API spend after Ctrl-C). The first + * signal now also schedules the parent's own exit after the children's + * SIGKILL grace, and runners consult isTerminationRequested() before + * launching more work. + */ +interface SourceTerminationState { + requested: boolean; + exitScheduled: boolean; +} +const SOURCE_TERMINATION_STATE = new WeakMap(); +function terminationStateFor(source: TerminationSignalSource): SourceTerminationState { + let state = SOURCE_TERMINATION_STATE.get(source); + if (!state) { + state = { requested: false, exitScheduled: false }; + SOURCE_TERMINATION_STATE.set(source, state); + } + return state; +} +export function isTerminationRequested(source: TerminationSignalSource = process): boolean { + return SOURCE_TERMINATION_STATE.get(source)?.requested ?? false; +} +const signalExitCode = (signal: ForwardedTerminationSignal): number => + 128 + (signal === 'SIGINT' ? 2 : 15); + +/** + * Bind one active child to the parent's termination lifecycle. SIGINT and + * SIGTERM get a grace period so Bun can clean up; a repeated signal, timeout, + * or synchronous parent exit uses SIGKILL so the child cannot be orphaned. + * The parent itself exits shortly after the grace window (or immediately on + * a repeated signal) — cancellation must terminate the RUN, not just the + * currently-running children. + */ +export function installChildSignalForwarding( + child: Pick, + source: TerminationSignalSource = process, + timer: TerminationTimerApi = DEFAULT_TERMINATION_TIMER, + graceMs = 5_000, + exitImpl: (code: number) => void = (code) => process.exit(code), +): ChildSignalForwarding { + let receivedSignal: ForwardedTerminationSignal | null = null; + let forceTimer: unknown = null; + let disposed = false; + + const scheduleParentExit = (signal: ForwardedTerminationSignal, delayMs: number): void => { + const state = terminationStateFor(source); + state.requested = true; + if (state.exitScheduled) return; + state.exitScheduled = true; + // Never cancelled by dispose(): once cancellation is requested, the run + // is going down even if this particular shard finishes cleanly first. + timer.schedule(() => exitImpl(signalExitCode(signal)), delayMs); + }; + + const forward = (signal: ForwardedTerminationSignal): void => { + if (disposed) return; + if (receivedSignal !== null) { + child.kill('SIGKILL'); + scheduleParentExit(signal, 0); + return; + } + receivedSignal = signal; + child.kill(signal); + forceTimer = timer.schedule(() => { + forceTimer = null; + child.kill('SIGKILL'); + }, graceMs); + // Exit AFTER the children's SIGKILL grace so the group kills land first. + scheduleParentExit(signal, graceMs + 1_000); + }; + const onSigint = () => forward('SIGINT'); + const onSigterm = () => forward('SIGTERM'); + const onExit = () => { child.kill('SIGKILL'); }; + + source.on('SIGINT', onSigint); + source.on('SIGTERM', onSigterm); + source.on('exit', onExit); + + return { + get receivedSignal() { + return receivedSignal; + }, + dispose() { + if (disposed) return; + disposed = true; + source.off('SIGINT', onSigint); + source.off('SIGTERM', onSigterm); + source.off('exit', onExit); + if (forceTimer !== null) timer.cancel(forceTimer); + forceTimer = null; + }, + }; +} + +/** + * SIGKILL the shard's whole process group. Orphaned grandchildren (browsers, + * claude sessions) are how a stalled run once burned a core for 15.7 hours. + */ +export function killProcessGroup(child: ChildProcess, signal: NodeJS.Signals): void { + if (process.platform === 'win32' || typeof child.pid !== 'number') { + child.kill(signal); + return; + } + try { + process.kill(-child.pid, signal); + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === 'ESRCH') return; // group already gone + if (code !== 'EPERM') throw err; + // Observed on macOS after a SIGKILLed group is reaped: signalling the + // now-empty group id returns EPERM, not ESRCH. Throwing here loses the + // shard's real outcome (a timeout gets recorded as a failure) and, from + // the timeout timer, leaves the shard promise unsettled — a hang, which + // is the exact failure class this runner exists to kill. Fall back to the + // direct pid so a genuinely-live child is still signalled. + try { + child.kill(signal); + } catch { + // Best-effort reap: nothing actionable is left if this fails too. + } + } +} + +/** + * Strip ANSI escapes and a trailing CR from one output line. Every line + * matcher (here and in the free runner's console filter / failure + * attribution) MUST match against this form — a prior grep for `(fail)` + * lines missed real failures because color codes sat inside the line. + */ +export function stripAnsiLine(rawLine: string): string { + return rawLine.replace(ANSI_ESCAPE, '').replace(/\r$/, ''); +} + +export function classifyBunTestOutputLine(rawLine: string): BunTestOutputFinding | null { + const line = stripAnsiLine(rawLine); + if (parseBunFailureResult(line) !== null) return 'failed-test'; + if (line === BUN_BETWEEN_TESTS_ERROR) return 'unhandled-between-tests'; + return null; +} + +export function parseBunFailureResult(rawLine: string): string | null { + return BUN_FAIL_RESULT.exec(stripAnsiLine(rawLine))?.[1] ?? null; +} + +export function parseBunTerminalSummaryLine(rawLine: string): number | null { + return parseBunTerminalSummary(rawLine)?.files ?? null; +} + +export function parseBunTerminalSummary(rawLine: string): { tests: number; files: number } | null { + const line = stripAnsiLine(rawLine); + const match = BUN_TERMINAL_SUMMARY.exec(line); + return match + ? { tests: Number.parseInt(match[1], 10), files: Number.parseInt(match[2], 10) } + : null; +} + +/** + * Incrementally classifies output without assuming process chunks align to + * lines. Buffers are PER ORIGIN: stdout and stderr are independent pipes, so + * a chunk from one can arrive between two halves of a line from the other. + * A single shared buffer would glue those fragments into garbled lines — a + * sheared `(fail)` line goes uncounted and a sheared terminal summary reads + * as truncation. Counters are shared; only line assembly is per-stream. + */ +export type ClassifierOrigin = 'stdout' | 'stderr'; + +export class BunFailureSummaryParser { + private readonly pending: Partial> = {}; + + consume(rawLine: string, origin: ClassifierOrigin): number | null { + const line = stripAnsiLine(rawLine); + if (BUN_PASS_COUNT.test(line)) { + this.pending[origin] = { failures: null }; + return null; + } + const pending = this.pending[origin]; + if (!pending) return null; + const fail = BUN_FAIL_COUNT.exec(line); + if (fail) { + pending.failures = Math.max(pending.failures ?? 0, Number.parseInt(fail[1], 10)); + return null; + } + if (parseBunTerminalSummary(line) !== null) { + delete this.pending[origin]; + return pending.failures; + } + return null; + } +} + +export class BunTestOutputClassifier { + private readonly decoders: Record = { + stdout: new StringDecoder('utf8'), + stderr: new StringDecoder('utf8'), + }; + private pending: Record = { stdout: '', stderr: '' }; + private failedTests = 0; + private reportedFailedTests = 0; + private readonly failureSummary = new BunFailureSummaryParser(); + private unhandledBetweenTests = 0; + private terminalFileCounts: number[] = []; + private terminalTestCounts: number[] = []; + private skippedTests = 0; + private passedTests = 0; + + write(chunk: Uint8Array | string, origin: ClassifierOrigin = 'stdout'): void { + this.pending[origin] += typeof chunk === 'string' + ? chunk + : this.decoders[origin].write(Buffer.from(chunk)); + this.consumeCompleteLines(origin); + } + + end(): BunTestOutputSummary { + for (const origin of ['stdout', 'stderr'] as const) { + this.pending[origin] += this.decoders[origin].end(); + if (this.pending[origin].length > 0) this.classify(this.pending[origin], origin); + this.pending[origin] = ''; + } + return this.summary(); + } + + summary(): BunTestOutputSummary { + return { + failedTests: Math.max(this.failedTests, this.reportedFailedTests), + unhandledBetweenTests: this.unhandledBetweenTests, + terminalFileCounts: [...this.terminalFileCounts], + terminalTestCounts: [...this.terminalTestCounts], + skippedTests: this.skippedTests, + passedTests: this.passedTests, + }; + } + + private consumeCompleteLines(origin: ClassifierOrigin): void { + let newline = this.pending[origin].indexOf('\n'); + while (newline !== -1) { + this.classify(this.pending[origin].slice(0, newline), origin); + this.pending[origin] = this.pending[origin].slice(newline + 1); + newline = this.pending[origin].indexOf('\n'); + } + } + + private classify(line: string, origin: ClassifierOrigin): void { + const finding = classifyBunTestOutputLine(line); + if (finding === 'failed-test') this.failedTests += 1; + if (finding === 'unhandled-between-tests') this.unhandledBetweenTests += 1; + const stripped = stripAnsiLine(line); + const skip = BUN_SKIP_COUNT.exec(stripped); + if (skip !== null) this.skippedTests += Number.parseInt(skip[1], 10); + const pass = BUN_PASS_COUNT.exec(stripped); + if (pass !== null) this.passedTests += Number.parseInt(pass[1], 10); + const fail = this.failureSummary.consume(stripped, origin); + if (fail !== null) this.reportedFailedTests = Math.max(this.reportedFailedTests, fail); + const terminal = parseBunTerminalSummary(line); + if (terminal !== null) { + this.terminalFileCounts.push(terminal.files); + this.terminalTestCounts.push(terminal.tests); + } + } +} + +export function strictTestExitCode( + childExitCode: number, + summary: BunTestOutputSummary, + expectedFiles?: number, +): number { + if (childExitCode !== 0) return childExitCode; + if (summary.failedTests > 0 || summary.unhandledBetweenTests > 0) return 1; + if (expectedFiles !== undefined && !summary.terminalFileCounts.includes(expectedFiles)) return 1; + return 0; +} + +export function normalizeRelativePath(filePath: string): string { + return filePath.replace(/\\/g, '/'); +} + +/** + * Bun treats positional test paths as substring filters. Resolve every + * canonical relative path before spawning so `test/foo.test.ts` cannot also + * select `browse/test/foo.test.ts`. + */ +export function exactTestFileSelectors(files: string[], rootDir = ROOT): string[] { + return files.map((file) => path.isAbsolute(file) ? path.normalize(file) : path.resolve(rootDir, file)); +} + +export function forwardAndClassify( + stream: NodeJS.ReadableStream, + destination: NodeJS.WriteStream, + classifier: BunTestOutputClassifier, + origin: ClassifierOrigin = 'stdout', +): Promise { + return new Promise((resolve, reject) => { + let ended = false; + const incomplete = () => reject(new Error(`incomplete ${origin} capture: stream closed before end`)); + stream.on('data', (chunk: Buffer | string) => { + classifier.write(chunk, origin); + destination.write(chunk); + }); + stream.once('end', () => { ended = true; resolve(); }); + stream.on('error', reject); + stream.once('close', () => { if (!ended) incomplete(); }); + // Bun can return an already-destroyed pipe whose close event is past. + if ('destroyed' in stream && stream.destroyed && !ended) { + if ('errored' in stream && stream.errored) reject(stream.errored); + else incomplete(); + } + }); +} + +// --- Shared shard-child lifecycle --- + +export interface RunShardChildOptions { + command: string; + args: string[]; + cwd: string; + env: NodeJS.ProcessEnv; + /** External wall-clock deadline; on expiry the child's process GROUP is SIGKILLed. */ + timeoutMs: number; + deadlineMs?: number; + /** + * Hook the freshly-spawned child's stdout/stderr. Stream POLICY (classifier + * tees, log spooling, console forwarding, reporters) is entirely the + * caller's. Runs synchronously right after spawn; child close and every + * returned promise must settle within the same deadline. A wall-expired + * return reports incomplete capture instead of treating the prefix as final. + */ + hookStreams: (child: ChildProcess) => Array>; +} + +export interface ShardChildResult { + exitCode: number | null; + /** True when the shared deadline expired; no further child work is allowed. */ + timedOut: boolean; + /** The child's pid — the process-GROUP id on POSIX (detached spawn). */ + groupPid: number | null; + incompleteCapture?: { + childClosed: boolean; + pendingStreams: number; + failedStreams: number; + deadlineMs: number; + }; +} + +/** + * The child lifecycle both sharded runners need, extracted from + * scripts/test-paid-shards.ts runPaidShard (scripts/test-free-shards.ts + * runFreeShard duplicates the same ~35 lines verbatim today and is designed + * to migrate here in a later change): + * + * - spawn detached on POSIX so the child owns its process group, + * - forward parent SIGINT/SIGTERM to the whole group (not just the child), + * - arm an EXTERNAL wall-clock timer that SIGKILLs the group — a spinning + * child main thread never fires its own in-process timer, + * - in EVERY exit path: disarm the timer, detach the signal forwarder, and + * signal group survivors with SIGKILL. + * + * Caller-side cleanup that must run even on a spawn failure (log streams, + * reporters, temp dirs) belongs in the caller's own try/finally around this + * call: a spawn 'error' event THROWS from here after the finally block runs, + * preserving the runners' existing could-not-run handling. + */ +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'); + if (deadlineMs <= Date.now()) { + return { exitCode: null, timedOut: true, groupPid: null, + incompleteCapture: { childClosed: false, pendingStreams: 0, failedStreams: 0, deadlineMs } }; + } + const child = spawn(options.command, options.args, { + cwd: options.cwd, + env: options.env, + stdio: ['ignore', 'pipe', 'pipe'], + detached: process.platform !== 'win32', + windowsHide: true, + }); + const groupPid = child.pid ?? null; + // 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'); + return true; + }, + }); + + let timedOut = false; + let childClosed = false; + let pendingStreams = 0; + let failedStreams = 0; + let exitCode: number | null = null; + let failed = false; + let firstError: unknown; + const rememberError = (error: unknown) => { + if (failed) return; + failed = true; + firstError = error; + }; + const kill = () => { + try { killProcessGroup(child, 'SIGKILL'); } + catch (error) { rememberError(error); } + }; + let close!: () => 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(); }; + child.once('exit', onExit); + child.once('close', onClose); + child.on('error', rememberError); + let expire!: () => void; + const expired = new Promise(resolve => { expire = resolve; }); + const killTimer = setTimeout(() => { + timedOut = true; + kill(); + expire(); + }, Math.max(0, deadlineMs - Date.now())); + + try { + let streams: Array> = []; + try { streams = options.hookStreams(child); } + catch (error) { + rememberError(error); + kill(); + child.stdout?.destroy(); + child.stderr?.destroy(); + } + pendingStreams = streams.length; + const drainage = Promise.all(streams.map(stream => Promise.resolve(stream).then( + () => { pendingStreams -= 1; }, + (error: unknown) => { pendingStreams -= 1; failedStreams += 1; rememberError(error); }, + ))); + await Promise.race([Promise.all([closed, drainage]), expired]); + if (Date.now() >= deadlineMs) timedOut = true; + } finally { + clearTimeout(killTimer); + forwarding.dispose(); + kill(); + child.off('exit', onExit); + child.off('close', onClose); + if (!childClosed || pendingStreams > 0) { + child.stdout?.destroy(); + child.stderr?.destroy(); + child.unref(); + } + } + const result: ShardChildResult = { exitCode, timedOut, groupPid }; + if (!childClosed || pendingStreams > 0 || failedStreams > 0) { + result.incompleteCapture = { childClosed, pendingStreams, failedStreams, deadlineMs }; + } + if (failed) { + if (firstError instanceof Error && Object.isExtensible(firstError)) { + Reflect.defineProperty(firstError, 'shardResult', { value: result, configurable: true }); + } + throw firstError; + } + return result; +} diff --git a/scripts/test-strict-output.ts b/scripts/test-strict-output.ts index 5de584c57..01185d20d 100644 --- a/scripts/test-strict-output.ts +++ b/scripts/test-strict-output.ts @@ -1,528 +1,6 @@ /** - * Strict Bun-test output classification + child lifecycle helpers. - * - * Works around a Bun test runner bug where failures can be printed even though - * the child exits successfully: output is forwarded byte-for-byte as it - * arrives, and only complete Bun result lines and terminal summaries are - * classified. `strictTestExitCode` then refuses to trust a zero exit when the - * output shows failures (or when fewer files ran than expected). - * - * Shared by the sharded paid-tier runner (scripts/test-paid-shards.ts) and any - * future strict wrapper around `bun test`. + * Compatibility path for the shard engine, which now lives in + * scripts/lib/shard-engine.ts. Existing importers (session runners, the + * strict-output and run-shard-child tests, mock.module paths) keep working. */ - -import { spawn, type ChildProcess } from 'node:child_process'; -import { StringDecoder } from 'node:string_decoder'; -import * as path from 'node:path'; - -const ROOT = path.resolve(import.meta.dir, '..'); -const ANSI_ESCAPE = /\u001B\[[0-?]*[ -/]*[@-~]/g; -const BUN_FAIL_RESULT = /^(?:\(fail\)|✗) (.+) \[(?:\d+(?:\.\d+)?)(?:ns|us|µs|ms|s)\]$/; -const BUN_BETWEEN_TESTS_ERROR = '# Unhandled error between tests'; -const BUN_TERMINAL_SUMMARY = /^Ran (\d+) tests? across (\d+) files?\. \[(?:\d+(?:\.\d+)?)(?:ns|us|µs|ms|s)\]$/; -// The counts block bun prints just before the terminal summary (" 1 pass", -// " 2 skip", " 0 fail"). "Ran N tests" COUNTS skipped tests, so N alone -// cannot distinguish a shard that verified work from one whose every test -// self-skipped (external-service binary missing, tier mismatch) — the -// green-by-skip class. Anchored to whole-line matches; nested bun-test -// children can still contribute counts (same known limit as the terminal -// summary — see the last-summary-anchoring TODO in the audit). -const BUN_SKIP_COUNT = /^\s*(\d+) skip$/; -const BUN_PASS_COUNT = /^\s*(\d+) pass$/; -const BUN_FAIL_COUNT = /^\s*(\d+) fail$/; - -export type BunTestOutputFinding = 'failed-test' | 'unhandled-between-tests'; - -export interface BunTestOutputSummary { - failedTests: number; - unhandledBetweenTests: number; - terminalFileCounts: number[]; - /** Test counts from the same terminal lines — feeds the hollow-shard guard. */ - terminalTestCounts: number[]; - /** Sum of bun's " N skip" count lines. "Ran N tests" includes skips, so - * this is what separates verified work from green-by-skip. */ - skippedTests: number; - /** Sum of bun's " N pass" count lines. */ - passedTests: number; -} - -export type ForwardedTerminationSignal = 'SIGINT' | 'SIGTERM'; - -export interface TerminationSignalSource { - on(event: string, listener: () => void): unknown; - off(event: string, listener: () => void): unknown; -} - -export interface TerminationTimerApi { - schedule(callback: () => void, delayMs: number): unknown; - cancel(handle: unknown): void; -} - -export interface ChildSignalForwarding { - readonly receivedSignal: ForwardedTerminationSignal | null; - dispose(): void; -} - -const DEFAULT_TERMINATION_TIMER: TerminationTimerApi = { - schedule: (callback, delayMs) => setTimeout(callback, delayMs), - cancel: (handle) => clearTimeout(handle as ReturnType), -}; - -/** - * Per-source termination bookkeeping, shared across every forwarder bound to - * the same source. Installing ANY signal listener suppresses Node's default - * terminate-on-SIGINT/SIGTERM, so without this the parent runner survived - * cancellation: it killed the current child, then kept LAUNCHING new shards - * (observed: paid runs continuing to burn API spend after Ctrl-C). The first - * signal now also schedules the parent's own exit after the children's - * SIGKILL grace, and runners consult isTerminationRequested() before - * launching more work. - */ -interface SourceTerminationState { - requested: boolean; - exitScheduled: boolean; -} -const SOURCE_TERMINATION_STATE = new WeakMap(); -function terminationStateFor(source: TerminationSignalSource): SourceTerminationState { - let state = SOURCE_TERMINATION_STATE.get(source); - if (!state) { - state = { requested: false, exitScheduled: false }; - SOURCE_TERMINATION_STATE.set(source, state); - } - return state; -} -export function isTerminationRequested(source: TerminationSignalSource = process): boolean { - return SOURCE_TERMINATION_STATE.get(source)?.requested ?? false; -} -const signalExitCode = (signal: ForwardedTerminationSignal): number => - 128 + (signal === 'SIGINT' ? 2 : 15); - -/** - * Bind one active child to the parent's termination lifecycle. SIGINT and - * SIGTERM get a grace period so Bun can clean up; a repeated signal, timeout, - * or synchronous parent exit uses SIGKILL so the child cannot be orphaned. - * The parent itself exits shortly after the grace window (or immediately on - * a repeated signal) — cancellation must terminate the RUN, not just the - * currently-running children. - */ -export function installChildSignalForwarding( - child: Pick, - source: TerminationSignalSource = process, - timer: TerminationTimerApi = DEFAULT_TERMINATION_TIMER, - graceMs = 5_000, - exitImpl: (code: number) => void = (code) => process.exit(code), -): ChildSignalForwarding { - let receivedSignal: ForwardedTerminationSignal | null = null; - let forceTimer: unknown = null; - let disposed = false; - - const scheduleParentExit = (signal: ForwardedTerminationSignal, delayMs: number): void => { - const state = terminationStateFor(source); - state.requested = true; - if (state.exitScheduled) return; - state.exitScheduled = true; - // Never cancelled by dispose(): once cancellation is requested, the run - // is going down even if this particular shard finishes cleanly first. - timer.schedule(() => exitImpl(signalExitCode(signal)), delayMs); - }; - - const forward = (signal: ForwardedTerminationSignal): void => { - if (disposed) return; - if (receivedSignal !== null) { - child.kill('SIGKILL'); - scheduleParentExit(signal, 0); - return; - } - receivedSignal = signal; - child.kill(signal); - forceTimer = timer.schedule(() => { - forceTimer = null; - child.kill('SIGKILL'); - }, graceMs); - // Exit AFTER the children's SIGKILL grace so the group kills land first. - scheduleParentExit(signal, graceMs + 1_000); - }; - const onSigint = () => forward('SIGINT'); - const onSigterm = () => forward('SIGTERM'); - const onExit = () => { child.kill('SIGKILL'); }; - - source.on('SIGINT', onSigint); - source.on('SIGTERM', onSigterm); - source.on('exit', onExit); - - return { - get receivedSignal() { - return receivedSignal; - }, - dispose() { - if (disposed) return; - disposed = true; - source.off('SIGINT', onSigint); - source.off('SIGTERM', onSigterm); - source.off('exit', onExit); - if (forceTimer !== null) timer.cancel(forceTimer); - forceTimer = null; - }, - }; -} - -/** - * SIGKILL the shard's whole process group. Orphaned grandchildren (browsers, - * claude sessions) are how a stalled run once burned a core for 15.7 hours. - */ -export function killProcessGroup(child: ChildProcess, signal: NodeJS.Signals): void { - if (process.platform === 'win32' || typeof child.pid !== 'number') { - child.kill(signal); - return; - } - try { - process.kill(-child.pid, signal); - } catch (err) { - const code = (err as NodeJS.ErrnoException).code; - if (code === 'ESRCH') return; // group already gone - if (code !== 'EPERM') throw err; - // Observed on macOS after a SIGKILLed group is reaped: signalling the - // now-empty group id returns EPERM, not ESRCH. Throwing here loses the - // shard's real outcome (a timeout gets recorded as a failure) and, from - // the timeout timer, leaves the shard promise unsettled — a hang, which - // is the exact failure class this runner exists to kill. Fall back to the - // direct pid so a genuinely-live child is still signalled. - try { - child.kill(signal); - } catch { - // Best-effort reap: nothing actionable is left if this fails too. - } - } -} - -/** - * Strip ANSI escapes and a trailing CR from one output line. Every line - * matcher (here and in the free runner's console filter / failure - * attribution) MUST match against this form — a prior grep for `(fail)` - * lines missed real failures because color codes sat inside the line. - */ -export function stripAnsiLine(rawLine: string): string { - return rawLine.replace(ANSI_ESCAPE, '').replace(/\r$/, ''); -} - -export function classifyBunTestOutputLine(rawLine: string): BunTestOutputFinding | null { - const line = stripAnsiLine(rawLine); - if (parseBunFailureResult(line) !== null) return 'failed-test'; - if (line === BUN_BETWEEN_TESTS_ERROR) return 'unhandled-between-tests'; - return null; -} - -export function parseBunFailureResult(rawLine: string): string | null { - return BUN_FAIL_RESULT.exec(stripAnsiLine(rawLine))?.[1] ?? null; -} - -export function parseBunTerminalSummaryLine(rawLine: string): number | null { - return parseBunTerminalSummary(rawLine)?.files ?? null; -} - -export function parseBunTerminalSummary(rawLine: string): { tests: number; files: number } | null { - const line = stripAnsiLine(rawLine); - const match = BUN_TERMINAL_SUMMARY.exec(line); - return match - ? { tests: Number.parseInt(match[1], 10), files: Number.parseInt(match[2], 10) } - : null; -} - -/** - * Incrementally classifies output without assuming process chunks align to - * lines. Buffers are PER ORIGIN: stdout and stderr are independent pipes, so - * a chunk from one can arrive between two halves of a line from the other. - * A single shared buffer would glue those fragments into garbled lines — a - * sheared `(fail)` line goes uncounted and a sheared terminal summary reads - * as truncation. Counters are shared; only line assembly is per-stream. - */ -export type ClassifierOrigin = 'stdout' | 'stderr'; - -export class BunFailureSummaryParser { - private readonly pending: Partial> = {}; - - consume(rawLine: string, origin: ClassifierOrigin): number | null { - const line = stripAnsiLine(rawLine); - if (BUN_PASS_COUNT.test(line)) { - this.pending[origin] = { failures: null }; - return null; - } - const pending = this.pending[origin]; - if (!pending) return null; - const fail = BUN_FAIL_COUNT.exec(line); - if (fail) { - pending.failures = Math.max(pending.failures ?? 0, Number.parseInt(fail[1], 10)); - return null; - } - if (parseBunTerminalSummary(line) !== null) { - delete this.pending[origin]; - return pending.failures; - } - return null; - } -} - -export class BunTestOutputClassifier { - private readonly decoders: Record = { - stdout: new StringDecoder('utf8'), - stderr: new StringDecoder('utf8'), - }; - private pending: Record = { stdout: '', stderr: '' }; - private failedTests = 0; - private reportedFailedTests = 0; - private readonly failureSummary = new BunFailureSummaryParser(); - private unhandledBetweenTests = 0; - private terminalFileCounts: number[] = []; - private terminalTestCounts: number[] = []; - private skippedTests = 0; - private passedTests = 0; - - write(chunk: Uint8Array | string, origin: ClassifierOrigin = 'stdout'): void { - this.pending[origin] += typeof chunk === 'string' - ? chunk - : this.decoders[origin].write(Buffer.from(chunk)); - this.consumeCompleteLines(origin); - } - - end(): BunTestOutputSummary { - for (const origin of ['stdout', 'stderr'] as const) { - this.pending[origin] += this.decoders[origin].end(); - if (this.pending[origin].length > 0) this.classify(this.pending[origin], origin); - this.pending[origin] = ''; - } - return this.summary(); - } - - summary(): BunTestOutputSummary { - return { - failedTests: Math.max(this.failedTests, this.reportedFailedTests), - unhandledBetweenTests: this.unhandledBetweenTests, - terminalFileCounts: [...this.terminalFileCounts], - terminalTestCounts: [...this.terminalTestCounts], - skippedTests: this.skippedTests, - passedTests: this.passedTests, - }; - } - - private consumeCompleteLines(origin: ClassifierOrigin): void { - let newline = this.pending[origin].indexOf('\n'); - while (newline !== -1) { - this.classify(this.pending[origin].slice(0, newline), origin); - this.pending[origin] = this.pending[origin].slice(newline + 1); - newline = this.pending[origin].indexOf('\n'); - } - } - - private classify(line: string, origin: ClassifierOrigin): void { - const finding = classifyBunTestOutputLine(line); - if (finding === 'failed-test') this.failedTests += 1; - if (finding === 'unhandled-between-tests') this.unhandledBetweenTests += 1; - const stripped = stripAnsiLine(line); - const skip = BUN_SKIP_COUNT.exec(stripped); - if (skip !== null) this.skippedTests += Number.parseInt(skip[1], 10); - const pass = BUN_PASS_COUNT.exec(stripped); - if (pass !== null) this.passedTests += Number.parseInt(pass[1], 10); - const fail = this.failureSummary.consume(stripped, origin); - if (fail !== null) this.reportedFailedTests = Math.max(this.reportedFailedTests, fail); - const terminal = parseBunTerminalSummary(line); - if (terminal !== null) { - this.terminalFileCounts.push(terminal.files); - this.terminalTestCounts.push(terminal.tests); - } - } -} - -export function strictTestExitCode( - childExitCode: number, - summary: BunTestOutputSummary, - expectedFiles?: number, -): number { - if (childExitCode !== 0) return childExitCode; - if (summary.failedTests > 0 || summary.unhandledBetweenTests > 0) return 1; - if (expectedFiles !== undefined && !summary.terminalFileCounts.includes(expectedFiles)) return 1; - return 0; -} - -export function normalizeRelativePath(filePath: string): string { - return filePath.replace(/\\/g, '/'); -} - -/** - * Bun treats positional test paths as substring filters. Resolve every - * canonical relative path before spawning so `test/foo.test.ts` cannot also - * select `browse/test/foo.test.ts`. - */ -export function exactTestFileSelectors(files: string[], rootDir = ROOT): string[] { - return files.map((file) => path.isAbsolute(file) ? path.normalize(file) : path.resolve(rootDir, file)); -} - -export function forwardAndClassify( - stream: NodeJS.ReadableStream, - destination: NodeJS.WriteStream, - classifier: BunTestOutputClassifier, - origin: ClassifierOrigin = 'stdout', -): Promise { - return new Promise((resolve, reject) => { - let ended = false; - const incomplete = () => reject(new Error(`incomplete ${origin} capture: stream closed before end`)); - stream.on('data', (chunk: Buffer | string) => { - classifier.write(chunk, origin); - destination.write(chunk); - }); - stream.once('end', () => { ended = true; resolve(); }); - stream.on('error', reject); - stream.once('close', () => { if (!ended) incomplete(); }); - // Bun can return an already-destroyed pipe whose close event is past. - if ('destroyed' in stream && stream.destroyed && !ended) { - if ('errored' in stream && stream.errored) reject(stream.errored); - else incomplete(); - } - }); -} - -// --- Shared shard-child lifecycle --- - -export interface RunShardChildOptions { - command: string; - args: string[]; - cwd: string; - env: NodeJS.ProcessEnv; - /** External wall-clock deadline; on expiry the child's process GROUP is SIGKILLed. */ - timeoutMs: number; - deadlineMs?: number; - /** - * Hook the freshly-spawned child's stdout/stderr. Stream POLICY (classifier - * tees, log spooling, console forwarding, reporters) is entirely the - * caller's. Runs synchronously right after spawn; child close and every - * returned promise must settle within the same deadline. A wall-expired - * return reports incomplete capture instead of treating the prefix as final. - */ - hookStreams: (child: ChildProcess) => Array>; -} - -export interface ShardChildResult { - exitCode: number | null; - /** True when the shared deadline expired; no further child work is allowed. */ - timedOut: boolean; - /** The child's pid — the process-GROUP id on POSIX (detached spawn). */ - groupPid: number | null; - incompleteCapture?: { - childClosed: boolean; - pendingStreams: number; - failedStreams: number; - deadlineMs: number; - }; -} - -/** - * The child lifecycle both sharded runners need, extracted from - * scripts/test-paid-shards.ts runPaidShard (scripts/test-free-shards.ts - * runFreeShard duplicates the same ~35 lines verbatim today and is designed - * to migrate here in a later change): - * - * - spawn detached on POSIX so the child owns its process group, - * - forward parent SIGINT/SIGTERM to the whole group (not just the child), - * - arm an EXTERNAL wall-clock timer that SIGKILLs the group — a spinning - * child main thread never fires its own in-process timer, - * - in EVERY exit path: disarm the timer, detach the signal forwarder, and - * signal group survivors with SIGKILL. - * - * Caller-side cleanup that must run even on a spawn failure (log streams, - * reporters, temp dirs) belongs in the caller's own try/finally around this - * call: a spawn 'error' event THROWS from here after the finally block runs, - * preserving the runners' existing could-not-run handling. - */ -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'); - if (deadlineMs <= Date.now()) { - return { exitCode: null, timedOut: true, groupPid: null, - incompleteCapture: { childClosed: false, pendingStreams: 0, failedStreams: 0, deadlineMs } }; - } - const child = spawn(options.command, options.args, { - cwd: options.cwd, - env: options.env, - stdio: ['ignore', 'pipe', 'pipe'], - detached: process.platform !== 'win32', - windowsHide: true, - }); - const groupPid = child.pid ?? null; - // 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'); - return true; - }, - }); - - let timedOut = false; - let childClosed = false; - let pendingStreams = 0; - let failedStreams = 0; - let exitCode: number | null = null; - let failed = false; - let firstError: unknown; - const rememberError = (error: unknown) => { - if (failed) return; - failed = true; - firstError = error; - }; - const kill = () => { - try { killProcessGroup(child, 'SIGKILL'); } - catch (error) { rememberError(error); } - }; - let close!: () => 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(); }; - child.once('exit', onExit); - child.once('close', onClose); - child.on('error', rememberError); - let expire!: () => void; - const expired = new Promise(resolve => { expire = resolve; }); - const killTimer = setTimeout(() => { - timedOut = true; - kill(); - expire(); - }, Math.max(0, deadlineMs - Date.now())); - - try { - let streams: Array> = []; - try { streams = options.hookStreams(child); } - catch (error) { - rememberError(error); - kill(); - child.stdout?.destroy(); - child.stderr?.destroy(); - } - pendingStreams = streams.length; - const drainage = Promise.all(streams.map(stream => Promise.resolve(stream).then( - () => { pendingStreams -= 1; }, - (error: unknown) => { pendingStreams -= 1; failedStreams += 1; rememberError(error); }, - ))); - await Promise.race([Promise.all([closed, drainage]), expired]); - if (Date.now() >= deadlineMs) timedOut = true; - } finally { - clearTimeout(killTimer); - forwarding.dispose(); - kill(); - child.off('exit', onExit); - child.off('close', onClose); - if (!childClosed || pendingStreams > 0) { - child.stdout?.destroy(); - child.stderr?.destroy(); - child.unref(); - } - } - const result: ShardChildResult = { exitCode, timedOut, groupPid }; - if (!childClosed || pendingStreams > 0 || failedStreams > 0) { - result.incompleteCapture = { childClosed, pendingStreams, failedStreams, deadlineMs }; - } - if (failed) { - if (firstError instanceof Error && Object.isExtensible(firstError)) { - Reflect.defineProperty(firstError, 'shardResult', { value: result, configurable: true }); - } - throw firstError; - } - return result; -} +export * from './lib/shard-engine'; diff --git a/test/helpers/touchfiles-data.ts b/test/helpers/touchfiles-data.ts index 10acaa314..4b5f9d191 100644 --- a/test/helpers/touchfiles-data.ts +++ b/test/helpers/touchfiles-data.ts @@ -1560,6 +1560,7 @@ export const LLM_JUDGE_TOUCHFILES: Record = { */ export const GLOBAL_TOUCHFILES = [ 'scripts/test-strict-output.ts', + 'scripts/lib/shard-engine.ts', // The shard engine test-strict-output.ts re-exports (moved there in the W2 refactor) // Canonical paid execution and its shared time allocations affect every paid test. 'scripts/test-paid-shards.ts', 'scripts/test-pr-profile.ts', diff --git a/test/test-free-shards.test.ts b/test/test-free-shards.test.ts index 7195fc97b..ddce2c8f5 100644 --- a/test/test-free-shards.test.ts +++ b/test/test-free-shards.test.ts @@ -467,7 +467,7 @@ describe('test-free-shards: exclusive host-state phase', () => { let child: ReturnType | undefined; let watchdog: ReturnType | undefined; try { - for (const file of ['scripts/test-free-shards.ts', 'scripts/test-strict-output.ts', + for (const file of ['scripts/test-free-shards.ts', 'scripts/test-strict-output.ts', 'scripts/lib/shard-engine.ts', 'test/helpers/paid-test-set.ts', 'test/helpers/touchfiles.ts', 'test/helpers/touchfiles-data.ts', 'test/helpers/test-selection.ts']) { const target = path.join(directory, file); fs.mkdirSync(path.dirname(target), { recursive: true }); From cb203b298237db147fcbe1d17b4593c8c82994e3 Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 30 Sep 2026 05:56:15 +0000 Subject: [PATCH 2/6] refactor(shard-engine): run both lanes on the shared engine; lane policy injected Engine (scripts/lib/shard-engine.ts) gains the W2 primitives: per-shard tmp/Chromium sandbox + async cleanup backstop, log-path allocation and full-stream log capture, one duration-seed reader/writer with a lane predicate, LanePolicy (seed predicate + zero-execution verdict), strictShardStatus, and the shared CLI flag loop. runShardChild takes an optional companion (signal/settle) and waits a bounded 250ms to reap a wall-killed child. Free lane stops spawning shards itself: runFreeShard uses runShardChild with trackShardBrowser as the companion (win32 path unchanged: no process group, no negative-pid kill). Its sync state-dir removal stays lane policy. Paid lane uses the sandbox, log, seed, verdict and flag primitives; the hollow-shard guard applies PAID_LANE_POLICY. Lane outcomes are unchanged (free keeps >= 0 seeds and file-count zero-exec rule; paid keeps > 0 seeds, warning under selection and passed-empty under EVALS_ALL). paid-free-boundary's closure assertion now names the engine module, where the strict classifier lives. --- scripts/lib/shard-engine.ts | 189 ++++++++++++++++++++++- scripts/test-free-shards.ts | 255 ++++++++++++++------------------ scripts/test-paid-shards.ts | 176 ++++++++++------------ test/paid-free-boundary.test.ts | 2 +- 4 files changed, 372 insertions(+), 250 deletions(-) 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); From eb44a747a91e8481c31a0ec15ea4a8d8942cd36e Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 30 Sep 2026 05:56:16 +0000 Subject: [PATCH 3/6] test(shard-engine): engine unit tests, fixture-corpus equivalence, per-lane CLI parity - test/shard-engine.test.ts: failing/unhandled/module-load output fails both lanes, per-lane zero-execution and seed rules, whole-group kill on a wall timeout (both lanes), mocked-win32 path with no negative-pid kill, companion settle order, log capture, sandbox isolation, flag loop. - test/shard-engine-equivalence.test.ts + test/fixtures/shard-equivalence: seven outcome fixtures plus one real shard, run through both lanes and compared with classifications recorded from the base runners (96764e80). - test/shard-cli-parity.test.ts + test/fixtures/shard-cli-parity: flag set, defaults, validation errors and the Unknown argument error per lane match the base runners. --- test/fixtures/shard-cli-parity/cases.ts | 64 ++ test/fixtures/shard-cli-parity/results.json | 762 ++++++++++++++++++ test/fixtures/shard-equivalence/expected.json | 127 +++ .../shard-equivalence/fail.fixture.ts | 3 + .../module-load-error.fixture.ts | 5 + .../shard-equivalence/pass.fixture.ts | 3 + test/fixtures/shard-equivalence/record.ts | 67 ++ .../shard-equivalence/skip.fixture.ts | 3 + .../unhandled-between-tests.fixture.ts | 6 + .../shard-equivalence/wall-timeout.fixture.ts | 7 + .../zero-executed.fixture.ts | 2 + test/shard-cli-parity.test.ts | 33 + test/shard-engine-equivalence.test.ts | 52 ++ test/shard-engine.test.ts | 273 +++++++ 14 files changed, 1407 insertions(+) create mode 100644 test/fixtures/shard-cli-parity/cases.ts create mode 100644 test/fixtures/shard-cli-parity/results.json create mode 100644 test/fixtures/shard-equivalence/expected.json create mode 100644 test/fixtures/shard-equivalence/fail.fixture.ts create mode 100644 test/fixtures/shard-equivalence/module-load-error.fixture.ts create mode 100644 test/fixtures/shard-equivalence/pass.fixture.ts create mode 100644 test/fixtures/shard-equivalence/record.ts create mode 100644 test/fixtures/shard-equivalence/skip.fixture.ts create mode 100644 test/fixtures/shard-equivalence/unhandled-between-tests.fixture.ts create mode 100644 test/fixtures/shard-equivalence/wall-timeout.fixture.ts create mode 100644 test/fixtures/shard-equivalence/zero-executed.fixture.ts create mode 100644 test/shard-cli-parity.test.ts create mode 100644 test/shard-engine-equivalence.test.ts create mode 100644 test/shard-engine.test.ts diff --git a/test/fixtures/shard-cli-parity/cases.ts b/test/fixtures/shard-cli-parity/cases.ts new file mode 100644 index 000000000..75b2a4db0 --- /dev/null +++ b/test/fixtures/shard-cli-parity/cases.ts @@ -0,0 +1,64 @@ +/** + * Argument vectors for the per-lane CLI parity check. results.json holds the + * parse results the pre-engine runners (base 96764e80) produced for them. + */ +export const FREE_CASES: string[][] = [ + [], + ['--dry-run', '--list', '--verbose'], + ['--windows-only', '--list'], + ['--record-durations'], + ['--quick'], + ['--shards', '4', '--shard', '2'], + ['--wall-timeout', '600'], + ['--ci-plan', 'plan.json', '--shards', '20'], + ['--ci-run', 'plan.json', '--shard', '1', '--result', 'result.json'], + ['--ci-verify', 'plan.json', '--results', 'results'], + ['--bogus'], + ['--list', '--bogus'], + ['--shards'], + ['--shard'], + ['--wall-timeout', '0'], + ['--wall-timeout'], + ['--ci-plan'], + ['--ci-plan', '--list'], + ['--ci-plan', 'a.json', '--ci-run', 'b.json'], + ['--ci-plan', 'a.json', '--quick'], + ['--ci-run', 'plan.json'], + ['--ci-verify', 'plan.json'], + ['--quick', '--shard', '1'], + ['--quick', '--windows-only'], +]; + +export const PAID_CASES: Array<{ argv: string[]; env?: Record }> = [ + { argv: [] }, + { argv: ['--list', '--tier', 'periodic', '--profile', 'full'] }, + { argv: ['--tier', 'gate', '--profile', 'pr', '--list'] }, + { argv: ['--timeout', '600', '--jobs', '2', '--files-per-shard', '3'] }, + { argv: ['--emit-plan', 'manifest.json', '--slices', '4', '--skip-judges'] }, + { argv: ['--plan', 'manifest.json', '--slice', '2'] }, + { argv: ['--report', 'reports', '--write-durations'] }, + { argv: [], env: { EVALS_TIER: 'periodic', EVALS_JOBS: '3', EVALS_CONCURRENCY: '5', EVALS_SHARD_TIMEOUT_MS: '1000', EVALS_PROFILE: 'full' } }, + { argv: ['--bogus'] }, + { argv: ['--list', '--bogus'] }, + { argv: ['--tier', 'e2e'] }, + { argv: ['--tier'] }, + { argv: ['--profile'] }, + { argv: ['--profile', 'fast'] }, + { argv: ['--timeout', '0'] }, + { argv: ['--jobs'] }, + { argv: ['--emit-plan'] }, + { argv: ['--plan'] }, + { argv: ['--report'] }, + { argv: ['--write-durations'] }, + { argv: ['--skip-judges'] }, + { argv: ['--profile', 'pr', '--tier', 'periodic'] }, + { argv: ['--profile', 'pr', '--files-per-shard', '2'] }, + { argv: [], env: { EVALS_TIER: 'e2e' } }, +]; + +export type ParseResult = { ok: unknown } | { error: string }; + +export function capture(parse: () => unknown): ParseResult { + try { return { ok: parse() }; } + catch (error) { return { error: error instanceof Error ? error.message : String(error) }; } +} diff --git a/test/fixtures/shard-cli-parity/results.json b/test/fixtures/shard-cli-parity/results.json new file mode 100644 index 000000000..f2d034188 --- /dev/null +++ b/test/fixtures/shard-cli-parity/results.json @@ -0,0 +1,762 @@ +{ + "recordedFrom": "96764e80 (pre-engine base runners)", + "free": [ + { + "argv": [], + "result": { + "ok": { + "dryRun": false, + "listOnly": false, + "recordDurations": false, + "windowsOnly": false, + "verbose": false, + "shardCount": 20, + "shardIndex": null, + "wallTimeoutMs": 360000, + "wallTimeoutExplicit": false, + "quick": false, + "ciPlan": null, + "ciRun": null, + "ciVerify": null, + "result": null, + "results": null + } + } + }, + { + "argv": [ + "--dry-run", + "--list", + "--verbose" + ], + "result": { + "ok": { + "dryRun": true, + "listOnly": true, + "recordDurations": false, + "windowsOnly": false, + "verbose": true, + "shardCount": 20, + "shardIndex": null, + "wallTimeoutMs": 360000, + "wallTimeoutExplicit": false, + "quick": false, + "ciPlan": null, + "ciRun": null, + "ciVerify": null, + "result": null, + "results": null + } + } + }, + { + "argv": [ + "--windows-only", + "--list" + ], + "result": { + "ok": { + "dryRun": false, + "listOnly": true, + "recordDurations": false, + "windowsOnly": true, + "verbose": false, + "shardCount": 20, + "shardIndex": null, + "wallTimeoutMs": 360000, + "wallTimeoutExplicit": false, + "quick": false, + "ciPlan": null, + "ciRun": null, + "ciVerify": null, + "result": null, + "results": null + } + } + }, + { + "argv": [ + "--record-durations" + ], + "result": { + "ok": { + "dryRun": false, + "listOnly": false, + "recordDurations": true, + "windowsOnly": false, + "verbose": false, + "shardCount": 20, + "shardIndex": null, + "wallTimeoutMs": 360000, + "wallTimeoutExplicit": false, + "quick": false, + "ciPlan": null, + "ciRun": null, + "ciVerify": null, + "result": null, + "results": null + } + } + }, + { + "argv": [ + "--quick" + ], + "result": { + "ok": { + "dryRun": false, + "listOnly": false, + "recordDurations": false, + "windowsOnly": false, + "verbose": false, + "shardCount": 20, + "shardIndex": null, + "wallTimeoutMs": 360000, + "wallTimeoutExplicit": false, + "quick": true, + "ciPlan": null, + "ciRun": null, + "ciVerify": null, + "result": null, + "results": null + } + } + }, + { + "argv": [ + "--shards", + "4", + "--shard", + "2" + ], + "result": { + "ok": { + "dryRun": false, + "listOnly": false, + "recordDurations": false, + "windowsOnly": false, + "verbose": false, + "shardCount": 4, + "shardIndex": 2, + "wallTimeoutMs": 360000, + "wallTimeoutExplicit": false, + "quick": false, + "ciPlan": null, + "ciRun": null, + "ciVerify": null, + "result": null, + "results": null + } + } + }, + { + "argv": [ + "--wall-timeout", + "600" + ], + "result": { + "ok": { + "dryRun": false, + "listOnly": false, + "recordDurations": false, + "windowsOnly": false, + "verbose": false, + "shardCount": 20, + "shardIndex": null, + "wallTimeoutMs": 600000, + "wallTimeoutExplicit": true, + "quick": false, + "ciPlan": null, + "ciRun": null, + "ciVerify": null, + "result": null, + "results": null + } + } + }, + { + "argv": [ + "--ci-plan", + "plan.json", + "--shards", + "20" + ], + "result": { + "ok": { + "dryRun": false, + "listOnly": false, + "recordDurations": false, + "windowsOnly": false, + "verbose": false, + "shardCount": 20, + "shardIndex": null, + "wallTimeoutMs": 360000, + "wallTimeoutExplicit": false, + "quick": false, + "ciPlan": "plan.json", + "ciRun": null, + "ciVerify": null, + "result": null, + "results": null + } + } + }, + { + "argv": [ + "--ci-run", + "plan.json", + "--shard", + "1", + "--result", + "result.json" + ], + "result": { + "ok": { + "dryRun": false, + "listOnly": false, + "recordDurations": false, + "windowsOnly": false, + "verbose": false, + "shardCount": 20, + "shardIndex": 1, + "wallTimeoutMs": 360000, + "wallTimeoutExplicit": false, + "quick": false, + "ciPlan": null, + "ciRun": "plan.json", + "ciVerify": null, + "result": "result.json", + "results": null + } + } + }, + { + "argv": [ + "--ci-verify", + "plan.json", + "--results", + "results" + ], + "result": { + "ok": { + "dryRun": false, + "listOnly": false, + "recordDurations": false, + "windowsOnly": false, + "verbose": false, + "shardCount": 20, + "shardIndex": null, + "wallTimeoutMs": 360000, + "wallTimeoutExplicit": false, + "quick": false, + "ciPlan": null, + "ciRun": null, + "ciVerify": "plan.json", + "result": null, + "results": "results" + } + } + }, + { + "argv": [ + "--bogus" + ], + "result": { + "error": "Unknown argument: --bogus" + } + }, + { + "argv": [ + "--list", + "--bogus" + ], + "result": { + "error": "Unknown argument: --bogus" + } + }, + { + "argv": [ + "--shards" + ], + "result": { + "error": "Missing value for --shards" + } + }, + { + "argv": [ + "--shard" + ], + "result": { + "error": "Missing value for --shard" + } + }, + { + "argv": [ + "--wall-timeout", + "0" + ], + "result": { + "error": "--wall-timeout needs a positive integer (seconds)" + } + }, + { + "argv": [ + "--wall-timeout" + ], + "result": { + "error": "--wall-timeout needs a positive integer (seconds)" + } + }, + { + "argv": [ + "--ci-plan" + ], + "result": { + "error": "Missing path for --ci-plan" + } + }, + { + "argv": [ + "--ci-plan", + "--list" + ], + "result": { + "error": "Missing path for --ci-plan" + } + }, + { + "argv": [ + "--ci-plan", + "a.json", + "--ci-run", + "b.json" + ], + "result": { + "error": "CI modes cannot be combined with other selection modes" + } + }, + { + "argv": [ + "--ci-plan", + "a.json", + "--quick" + ], + "result": { + "error": "CI modes cannot be combined with other selection modes" + } + }, + { + "argv": [ + "--ci-run", + "plan.json" + ], + "result": { + "error": "--ci-run requires --shard and --result" + } + }, + { + "argv": [ + "--ci-verify", + "plan.json" + ], + "result": { + "error": "--ci-verify requires --results" + } + }, + { + "argv": [ + "--quick", + "--shard", + "1" + ], + "result": { + "error": "--quick cannot change recording, Windows or shard selection" + } + }, + { + "argv": [ + "--quick", + "--windows-only" + ], + "result": { + "error": "--quick cannot change recording, Windows or shard selection" + } + } + ], + "paid": [ + { + "argv": [], + "result": { + "ok": { + "tier": "gate", + "profile": "full", + "profileExplicit": false, + "listOnly": false, + "skipJudges": false, + "timeoutExplicit": false, + "timeoutMs": 1800000, + "jobs": 8, + "withinShardConcurrency": 2, + "maxFilesPerShard": 1, + "emitPlanPath": null, + "slices": 1, + "planPath": null, + "sliceIndex": null, + "reportDir": null, + "writeDurations": false + } + } + }, + { + "argv": [ + "--list", + "--tier", + "periodic", + "--profile", + "full" + ], + "result": { + "ok": { + "tier": "periodic", + "profile": "full", + "profileExplicit": true, + "listOnly": true, + "skipJudges": false, + "timeoutExplicit": false, + "timeoutMs": 1800000, + "jobs": 8, + "withinShardConcurrency": 2, + "maxFilesPerShard": 1, + "emitPlanPath": null, + "slices": 1, + "planPath": null, + "sliceIndex": null, + "reportDir": null, + "writeDurations": false + } + } + }, + { + "argv": [ + "--tier", + "gate", + "--profile", + "pr", + "--list" + ], + "result": { + "ok": { + "tier": "gate", + "profile": "pr", + "profileExplicit": true, + "listOnly": true, + "skipJudges": false, + "timeoutExplicit": false, + "timeoutMs": 1800000, + "jobs": 8, + "withinShardConcurrency": 2, + "maxFilesPerShard": 1, + "emitPlanPath": null, + "slices": 1, + "planPath": null, + "sliceIndex": null, + "reportDir": null, + "writeDurations": false + } + } + }, + { + "argv": [ + "--timeout", + "600", + "--jobs", + "2", + "--files-per-shard", + "3" + ], + "result": { + "ok": { + "tier": "gate", + "profile": "full", + "profileExplicit": false, + "listOnly": false, + "skipJudges": false, + "timeoutExplicit": true, + "timeoutMs": 600000, + "jobs": 2, + "withinShardConcurrency": 2, + "maxFilesPerShard": 3, + "emitPlanPath": null, + "slices": 1, + "planPath": null, + "sliceIndex": null, + "reportDir": null, + "writeDurations": false + } + } + }, + { + "argv": [ + "--emit-plan", + "manifest.json", + "--slices", + "4", + "--skip-judges" + ], + "result": { + "ok": { + "tier": "gate", + "profile": "full", + "profileExplicit": false, + "listOnly": false, + "skipJudges": true, + "timeoutExplicit": false, + "timeoutMs": 1800000, + "jobs": 8, + "withinShardConcurrency": 2, + "maxFilesPerShard": 1, + "emitPlanPath": "manifest.json", + "slices": 4, + "planPath": null, + "sliceIndex": null, + "reportDir": null, + "writeDurations": false + } + } + }, + { + "argv": [ + "--plan", + "manifest.json", + "--slice", + "2" + ], + "result": { + "ok": { + "tier": "gate", + "profile": "full", + "profileExplicit": false, + "listOnly": false, + "skipJudges": false, + "timeoutExplicit": false, + "timeoutMs": 1800000, + "jobs": 8, + "withinShardConcurrency": 2, + "maxFilesPerShard": 1, + "emitPlanPath": null, + "slices": 1, + "planPath": "manifest.json", + "sliceIndex": 2, + "reportDir": null, + "writeDurations": false + } + } + }, + { + "argv": [ + "--report", + "reports", + "--write-durations" + ], + "result": { + "ok": { + "tier": "gate", + "profile": "full", + "profileExplicit": false, + "listOnly": false, + "skipJudges": false, + "timeoutExplicit": false, + "timeoutMs": 1800000, + "jobs": 8, + "withinShardConcurrency": 2, + "maxFilesPerShard": 1, + "emitPlanPath": null, + "slices": 1, + "planPath": null, + "sliceIndex": null, + "reportDir": "reports", + "writeDurations": true + } + } + }, + { + "argv": [], + "env": { + "EVALS_TIER": "periodic", + "EVALS_JOBS": "3", + "EVALS_CONCURRENCY": "5", + "EVALS_SHARD_TIMEOUT_MS": "1000", + "EVALS_PROFILE": "full" + }, + "result": { + "ok": { + "tier": "periodic", + "profile": "full", + "profileExplicit": true, + "listOnly": false, + "skipJudges": false, + "timeoutExplicit": true, + "timeoutMs": 1000, + "jobs": 3, + "withinShardConcurrency": 5, + "maxFilesPerShard": 1, + "emitPlanPath": null, + "slices": 1, + "planPath": null, + "sliceIndex": null, + "reportDir": null, + "writeDurations": false + } + } + }, + { + "argv": [ + "--bogus" + ], + "result": { + "error": "Unknown argument: --bogus" + } + }, + { + "argv": [ + "--list", + "--bogus" + ], + "result": { + "error": "Unknown argument: --bogus" + } + }, + { + "argv": [ + "--tier", + "e2e" + ], + "result": { + "error": "--tier must be gate or periodic. Received: e2e" + } + }, + { + "argv": [ + "--tier" + ], + "result": { + "error": "--tier must be gate or periodic. Received: undefined" + } + }, + { + "argv": [ + "--profile" + ], + "result": { + "error": "--profile needs pr or full" + } + }, + { + "argv": [ + "--profile", + "fast" + ], + "result": { + "error": "--profile must be pr or full. Received: fast" + } + }, + { + "argv": [ + "--timeout", + "0" + ], + "result": { + "error": "--timeout needs a positive integer. Received: 0" + } + }, + { + "argv": [ + "--jobs" + ], + "result": { + "error": "--jobs needs a positive integer. Received: undefined" + } + }, + { + "argv": [ + "--emit-plan" + ], + "result": { + "error": "--emit-plan needs a file path" + } + }, + { + "argv": [ + "--plan" + ], + "result": { + "error": "--plan needs a manifest path" + } + }, + { + "argv": [ + "--report" + ], + "result": { + "error": "--report needs a directory" + } + }, + { + "argv": [ + "--write-durations" + ], + "result": { + "error": "--write-durations requires --report" + } + }, + { + "argv": [ + "--skip-judges" + ], + "result": { + "error": "--skip-judges applies only to an emitted gate census plan" + } + }, + { + "argv": [ + "--profile", + "pr", + "--tier", + "periodic" + ], + "result": { + "error": "PR profile requires gate tier" + } + }, + { + "argv": [ + "--profile", + "pr", + "--files-per-shard", + "2" + ], + "result": { + "error": "PR profile requires one file per shard to preserve case accounting" + } + }, + { + "argv": [], + "env": { + "EVALS_TIER": "e2e" + }, + "result": { + "error": "EVALS_TIER must be gate or periodic. Received: e2e" + } + } + ], + "unknownArgumentCli": { + "free": { + "status": 1, + "stderr": "[test:free] Unknown argument: --bogus" + }, + "paid": { + "status": 1, + "stderr": "[test:paid] Unknown argument: --bogus" + } + } +} diff --git a/test/fixtures/shard-equivalence/expected.json b/test/fixtures/shard-equivalence/expected.json new file mode 100644 index 000000000..934099a62 --- /dev/null +++ b/test/fixtures/shard-equivalence/expected.json @@ -0,0 +1,127 @@ +{ + "recordedFrom": "96764e80 (pre-engine base) via test/fixtures/shard-equivalence/record.ts", + "free": { + "fixtures": { + "pass": { + "status": "passed", + "exitCode": 0, + "laneExit": 0 + }, + "fail": { + "status": "failed", + "exitCode": 1, + "laneExit": 1 + }, + "skip": { + "status": "passed", + "exitCode": 0, + "laneExit": 0 + }, + "wall-timeout": { + "status": "timed-out", + "exitCode": null, + "laneExit": 124 + }, + "module-load-error": { + "status": "failed", + "exitCode": 1, + "laneExit": 1 + }, + "zero-executed": { + "status": "passed", + "exitCode": 0, + "laneExit": 0 + }, + "unhandled-between-tests": { + "status": "failed", + "exitCode": 1, + "laneExit": 1 + } + }, + "laneExit": 124 + }, + "paid": { + "fixtures": { + "pass": { + "status": "passed", + "exitCode": 0, + "executedTests": 1, + "skippedTests": 0, + "laneExit": { + "selective": 0, + "evalsAll": 0 + } + }, + "fail": { + "status": "failed", + "exitCode": 1, + "executedTests": 1, + "skippedTests": 0, + "laneExit": { + "selective": 1, + "evalsAll": 1 + } + }, + "skip": { + "status": "passed", + "exitCode": 0, + "executedTests": 1, + "skippedTests": 1, + "laneExit": { + "selective": 0, + "evalsAll": 0 + } + }, + "wall-timeout": { + "status": "timed-out", + "exitCode": null, + "executedTests": null, + "skippedTests": null, + "laneExit": { + "selective": 1, + "evalsAll": 1 + } + }, + "module-load-error": { + "status": "failed", + "exitCode": 1, + "executedTests": 1, + "skippedTests": 0, + "laneExit": { + "selective": 1, + "evalsAll": 1 + } + }, + "zero-executed": { + "status": "passed", + "exitCode": 0, + "executedTests": 0, + "skippedTests": 0, + "laneExit": { + "selective": 0, + "evalsAll": 1 + } + }, + "unhandled-between-tests": { + "status": "failed", + "exitCode": 1, + "executedTests": 0, + "skippedTests": 0, + "laneExit": { + "selective": 1, + "evalsAll": 1 + } + } + }, + "laneExit": { + "selective": 1, + "evalsAll": 1 + } + }, + "realShard": { + "file": "test/strict-output.test.ts", + "status": "passed", + "filesRan": 1, + "sawTerminalSummary": true + } +} diff --git a/test/fixtures/shard-equivalence/fail.fixture.ts b/test/fixtures/shard-equivalence/fail.fixture.ts new file mode 100644 index 000000000..1b269d356 --- /dev/null +++ b/test/fixtures/shard-equivalence/fail.fixture.ts @@ -0,0 +1,3 @@ +import { expect, test } from 'bun:test'; + +test('fails', () => { expect(1 + 1).toBe(3); }); diff --git a/test/fixtures/shard-equivalence/module-load-error.fixture.ts b/test/fixtures/shard-equivalence/module-load-error.fixture.ts new file mode 100644 index 000000000..b4813f95f --- /dev/null +++ b/test/fixtures/shard-equivalence/module-load-error.fixture.ts @@ -0,0 +1,5 @@ +import { test } from 'bun:test'; + +throw new Error('module load failure fixture'); + +test('never registered', () => {}); diff --git a/test/fixtures/shard-equivalence/pass.fixture.ts b/test/fixtures/shard-equivalence/pass.fixture.ts new file mode 100644 index 000000000..e2400d773 --- /dev/null +++ b/test/fixtures/shard-equivalence/pass.fixture.ts @@ -0,0 +1,3 @@ +import { expect, test } from 'bun:test'; + +test('passes', () => { expect(1 + 1).toBe(2); }); diff --git a/test/fixtures/shard-equivalence/record.ts b/test/fixtures/shard-equivalence/record.ts new file mode 100644 index 000000000..77e1c8707 --- /dev/null +++ b/test/fixtures/shard-equivalence/record.ts @@ -0,0 +1,67 @@ +/** + * Runs the shard-equivalence fixture corpus through one checkout's free and + * paid runners and prints their classifications as one JSON line. + * + * bun test/fixtures/shard-equivalence/record.ts + * + * expected.json was recorded by running this against the pre-engine base + * commit (96764e80); test/shard-engine-equivalence.test.ts runs it against + * the current checkout and requires identical classifications and lane exits. + * Never imported by the test runner. + */ +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +export const FIXTURES = [ + 'pass', 'fail', 'skip', 'wall-timeout', 'module-load-error', 'zero-executed', 'unhandled-between-tests', +] as const; +export const REAL_SHARD = 'test/strict-output.test.ts'; +const WALL_MS = 3_000; + +const root = path.resolve(process.argv[2] ?? path.join(import.meta.dir, '../../..')); +const fixtureDir = import.meta.dir; +const scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'shard-equivalence-')); +const free = await import(path.join(root, 'scripts/test-free-shards.ts')); +const paid = await import(path.join(root, 'scripts/test-paid-shards.ts')); +const freeExit = (status: string) => (status === 'passed' ? 0 : status === 'timed-out' ? 124 : 1); + +try { + const runFree = (file: string, index: number) => free.runFreeShard([file], index + 1, FIXTURES.length, { + rootDir: root, quiet: true, log: () => {}, wallTimeoutMs: WALL_MS, + logFilePath: path.join(scratch, `free-${index}.log`), + }); + const runPaid = (file: string, index: number) => paid.runPaidShard([file], index + 1, FIXTURES.length, { + rootDir: root, timeoutMs: WALL_MS, jobs: 2, logDir: scratch, log: () => {}, + env: { ...process.env, GSTACK_CLAUDE_CLI_VERSION: 'fixture', EVALS: '', EVALS_TIER: '', EVALS_ALL: '' }, + }); + const files = FIXTURES.map(name => path.join(fixtureDir, `${name}.fixture.ts`)); + const [freeOutcomes, paidOutcomes, real] = await Promise.all([ + Promise.all(files.map(runFree)), + Promise.all(files.map(runPaid)), + free.runFreeShard([REAL_SHARD], 1, 1, { + rootDir: root, quiet: true, log: () => {}, logFilePath: path.join(scratch, 'real.log'), + }), + ]); + const byName = (values: T[]) => Object.fromEntries(FIXTURES.map((name, index) => [name, values[index]])); + // Lane exit for a run of just that fixture, and for the whole corpus. + const paidExit = (outcomes: unknown[], evalsAll: boolean) => + paid.summaryExitCode(paid.summarize(paid.applyHollowShardGuard(outcomes, { evalsAll, warn: () => {} }))); + const record = { + free: { + fixtures: byName(freeOutcomes.map((o: any) => ({ status: o.status, exitCode: o.exitCode, laneExit: freeExit(o.status) }))), + laneExit: Math.max(...freeOutcomes.map((o: any) => freeExit(o.status))), + }, + paid: { + fixtures: byName(paidOutcomes.map((o: any) => ({ + status: o.status, exitCode: o.exitCode, executedTests: o.executedTests, skippedTests: o.skippedTests, + laneExit: { selective: paidExit([o], false), evalsAll: paidExit([o], true) }, + }))), + laneExit: { selective: paidExit(paidOutcomes, false), evalsAll: paidExit(paidOutcomes, true) }, + }, + realShard: { file: REAL_SHARD, status: real.status, ...real.summary }, + }; + console.log(`SHARD_EQUIVALENCE:${JSON.stringify(record)}`); +} finally { + fs.rmSync(scratch, { recursive: true, force: true }); +} diff --git a/test/fixtures/shard-equivalence/skip.fixture.ts b/test/fixtures/shard-equivalence/skip.fixture.ts new file mode 100644 index 000000000..c6531278e --- /dev/null +++ b/test/fixtures/shard-equivalence/skip.fixture.ts @@ -0,0 +1,3 @@ +import { test } from 'bun:test'; + +test.skip('skipped', () => {}); diff --git a/test/fixtures/shard-equivalence/unhandled-between-tests.fixture.ts b/test/fixtures/shard-equivalence/unhandled-between-tests.fixture.ts new file mode 100644 index 000000000..1ad5fa0c2 --- /dev/null +++ b/test/fixtures/shard-equivalence/unhandled-between-tests.fixture.ts @@ -0,0 +1,6 @@ +import { test } from 'bun:test'; + +test('first', () => {}); +// Rejects outside any test: bun prints "# Unhandled error between tests". +Promise.reject(new Error('unhandled between tests fixture')); +test('second', () => {}); diff --git a/test/fixtures/shard-equivalence/wall-timeout.fixture.ts b/test/fixtures/shard-equivalence/wall-timeout.fixture.ts new file mode 100644 index 000000000..6ed42c3c7 --- /dev/null +++ b/test/fixtures/shard-equivalence/wall-timeout.fixture.ts @@ -0,0 +1,7 @@ +import { test } from 'bun:test'; + +// Blocks the main thread, so no in-process timer can end it: only the +// runner's external wall-clock group kill can. +test('blocks past the wall deadline', () => { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 60_000); +}); diff --git a/test/fixtures/shard-equivalence/zero-executed.fixture.ts b/test/fixtures/shard-equivalence/zero-executed.fixture.ts new file mode 100644 index 000000000..7d1c1ce54 --- /dev/null +++ b/test/fixtures/shard-equivalence/zero-executed.fixture.ts @@ -0,0 +1,2 @@ +// Declares no tests: bun reports "Ran 0 tests across 1 file". +export const noTests = true; diff --git a/test/shard-cli-parity.test.ts b/test/shard-cli-parity.test.ts new file mode 100644 index 000000000..66246adf3 --- /dev/null +++ b/test/shard-cli-parity.test.ts @@ -0,0 +1,33 @@ +/** + * Per-lane CLI parity across the shard-engine refactor: each runner accepts + * the same flags with the same defaults and validation messages, and emits + * the same `Unknown argument:` error, as the pre-engine runners recorded in + * test/fixtures/shard-cli-parity/results.json (base 96764e80). + */ +import { describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { parseCliOptions as parseFree } from '../scripts/test-free-shards'; +import { parseCliOptions as parsePaid } from '../scripts/test-paid-shards'; +import { FREE_CASES, PAID_CASES, capture } from './fixtures/shard-cli-parity/cases'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const recorded = JSON.parse(fs.readFileSync(path.join(ROOT, 'test/fixtures/shard-cli-parity/results.json'), 'utf8')); + +describe('shard runner CLI parity with the pre-engine runners', () => { + test('free lane: same flags, defaults and validation errors', () => { + expect(FREE_CASES.map(argv => ({ argv, result: capture(() => parseFree(argv)) }))).toEqual(recorded.free); + }); + + test('paid lane: same flags, env-derived defaults and validation errors', () => { + expect(PAID_CASES.map(c => ({ ...c, result: capture(() => parsePaid(c.argv, c.env ?? {})) }))).toEqual(recorded.paid); + }); + + test.each(['free', 'paid'] as const)('%s CLI rejects an unknown flag with the same message and exit code', lane => { + const run = spawnSync(process.execPath, [path.join(ROOT, `scripts/test-${lane}-shards.ts`), '--bogus'], { + cwd: ROOT, encoding: 'utf8', timeout: 30_000, + }); + expect({ status: run.status, stderr: run.stderr.trim() }).toEqual(recorded.unknownArgumentCli[lane]); + }); +}); diff --git a/test/shard-engine-equivalence.test.ts b/test/shard-engine-equivalence.test.ts new file mode 100644 index 000000000..8788239c6 --- /dev/null +++ b/test/shard-engine-equivalence.test.ts @@ -0,0 +1,52 @@ +/** + * W2 runner equivalence: the free and paid lanes, now on the shared shard + * engine, classify a fixture corpus (pass, fail, skip, wall-timeout, + * module-load error, zero executed tests, unhandled error between tests) + * exactly as the pre-engine runners did. expected.json was recorded by the + * same recorder against base commit 96764e80; one real suite shard is run + * too. Re-record only for a deliberate, reviewed lane-policy change. + */ +import { describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { collectFreeTestFiles } from '../scripts/test-free-shards'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const CORPUS = path.join(ROOT, 'test/fixtures/shard-equivalence'); + +describe('shard engine runner equivalence (fixture corpus)', () => { + test('both lanes keep the base runners\' per-file classifications and lane exit codes', () => { + const run = spawnSync(process.execPath, [path.join(CORPUS, 'record.ts'), ROOT], { + cwd: ROOT, encoding: 'utf8', timeout: 90_000, + env: { ...process.env, EVALS: '', EVALS_TIER: '', EVALS_ALL: '' }, + }); + expect(run.error, run.stderr).toBeUndefined(); + expect(run.status, run.stderr).toBe(0); + const line = run.stdout.split('\n').find(entry => entry.startsWith('SHARD_EQUIVALENCE:')); + expect(line, run.stdout + run.stderr).toBeDefined(); + const actual = JSON.parse(line!.slice('SHARD_EQUIVALENCE:'.length)); + const expected = JSON.parse(fs.readFileSync(path.join(CORPUS, 'expected.json'), 'utf8')); + + expect(actual.free).toEqual(expected.free); + expect(actual.paid).toEqual(expected.paid); + // The real shard's test count moves as that file grows; its verdict and + // complete file accounting must not. + expect(actual.realShard).toMatchObject(expected.realShard); + expect(actual.realShard.testsRan).toBeGreaterThan(0); + }, 120_000); + + test('the corpus covers every outcome kind and stays outside the free census', () => { + const fixtures = fs.readdirSync(CORPUS).filter(name => name.endsWith('.fixture.ts')).sort(); + expect(fixtures).toEqual([ + 'fail.fixture.ts', 'module-load-error.fixture.ts', 'pass.fixture.ts', 'skip.fixture.ts', + 'unhandled-between-tests.fixture.ts', 'wall-timeout.fixture.ts', 'zero-executed.fixture.ts', + ]); + const expected = JSON.parse(fs.readFileSync(path.join(CORPUS, 'expected.json'), 'utf8')); + for (const lane of ['free', 'paid']) { + expect(Object.keys(expected[lane].fixtures).sort()).toEqual(fixtures.map(name => name.replace('.fixture.ts', ''))); + } + // A failing fixture inside the census would fail every full free run. + expect(collectFreeTestFiles(ROOT).filter(file => file.startsWith('test/fixtures/shard-equivalence/'))).toEqual([]); + }); +}); diff --git a/test/shard-engine.test.ts b/test/shard-engine.test.ts new file mode 100644 index 000000000..f625a9882 --- /dev/null +++ b/test/shard-engine.test.ts @@ -0,0 +1,273 @@ +/** + * Unit pins for the shared shard engine (scripts/lib/shard-engine.ts): the + * lane-exit consequences of failing, unhandled and module-load output, each + * lane's injected zero-execution and seed rules, whole-group kill on a wall + * timeout, the win32 path, log capture, sandbox isolation and the flag loop. + * run-shard-child and strict-output-* tests pin the moved primitives. + */ +import { afterEach, describe, expect, spyOn, test } from 'bun:test'; +import type { ChildProcess } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { + BunTestOutputClassifier, + createShardSandbox, + killProcessGroup, + nextShardLogPath, + openShardLog, + parseCliFlags, + readDurationSeed, + removeShardSandbox, + runShardChild, + strictShardStatus, + writeDurationSeed, + zeroExecutionVerdict, +} from '../scripts/lib/shard-engine'; +import { FREE_LANE_POLICY, exitCodeFor, runFreeShard } from '../scripts/test-free-shards'; +import { + PAID_LANE_POLICY, applyHollowShardGuard, runPaidShard, summarize, summaryExitCode, type ShardOutcome, +} from '../scripts/test-paid-shards'; + +const scratch: string[] = []; +const tempDir = (prefix: string) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + scratch.push(dir); + return dir; +}; +afterEach(() => { for (const dir of scratch.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); }); + +function summaryOf(output: string) { + const classifier = new BunTestOutputClassifier(); + classifier.write(output, 'stderr'); + return classifier.end(); +} + +const PASSING = 'test/a.test.ts:\n(pass) a [1.00ms]\n\n 1 pass\n 0 fail\nRan 1 test across 1 file. [2.00ms]\n'; +// Bun 1.3 output shapes observed for these outcomes (fixture corpus). +const FAILING = 'test/a.test.ts:\n(fail) a [1.00ms]\n\n 0 pass\n 1 fail\nRan 1 test across 1 file. [2.00ms]\n'; +const MODULE_LOAD = 'test/a.test.ts:\nerror: module load failure\n\n 0 pass\n 1 fail\n 1 error\nRan 1 test across 1 file. [2.00ms]\n'; +const UNHANDLED = 'test/a.test.ts:\n# Unhandled error between tests\nerror: boom\n\n 0 pass\n 0 fail\n 1 error\nRan 0 tests across 1 file. [2.00ms]\n'; + +describe('shard engine: verdicts and lane exits', () => { + const verdict = (output: string, exitCode: number | null, timedOut = false) => strictShardStatus({ + timedOut, exitCode, summary: summaryOf(output), expectedFiles: 1, evidenceComplete: true, + }); + const paidExit = (status: ShardOutcome['status']) => summaryExitCode(summarize([{ + shard: 1, files: ['test/a.test.ts'], status, exitCode: 0, elapsedMs: 1, groupPid: null, executedTests: 1, skippedTests: 0, + }])); + + test('a failing test file makes both lanes exit nonzero, even when bun exits 0', () => { + for (const exitCode of [0, 1]) { + const status = verdict(FAILING, exitCode); + expect(status).toBe('failed'); + expect(exitCodeFor(status)).toBe(1); + expect(paidExit(status)).toBe(1); + } + expect(exitCodeFor(verdict(PASSING, 0))).toBe(0); + expect(paidExit(verdict(PASSING, 0))).toBe(0); + }); + + test('unhandled between-test errors and module-load errors count as failures', () => { + expect(summaryOf(UNHANDLED).unhandledBetweenTests).toBe(1); + expect(verdict(UNHANDLED, 0)).toBe('failed'); + expect(summaryOf(MODULE_LOAD).failedTests).toBe(1); + expect(verdict(MODULE_LOAD, 0)).toBe('failed'); + expect(verdict(MODULE_LOAD, 1)).toBe('failed'); + }); + + test('missing summary, short file count or incomplete evidence never pass; a timeout is its own status', () => { + expect(verdict('(pass) a [1.00ms]\n', 0)).toBe('failed'); + expect(strictShardStatus({ timedOut: false, exitCode: 0, summary: summaryOf(PASSING), expectedFiles: 2, evidenceComplete: true })).toBe('failed'); + expect(strictShardStatus({ timedOut: false, exitCode: 0, summary: summaryOf(PASSING), expectedFiles: 1, evidenceComplete: false })).toBe('failed'); + expect(verdict(PASSING, 0, true)).toBe('timed-out'); + expect(exitCodeFor('timed-out')).toBe(124); + expect(paidExit('timed-out')).toBe(1); + }); +}); + +describe('shard engine: injected lane policy', () => { + const zero = (executedTests: number | null): ShardOutcome => ({ + shard: 1, files: ['test/a.test.ts'], status: 'passed', exitCode: 0, elapsedMs: 1, groupPid: null, executedTests, skippedTests: 0, + }); + + test('free lane: zero executed tests pass when every planned file was counted', () => { + for (const promisedAll of [false, true]) { + expect(zeroExecutionVerdict(0, FREE_LANE_POLICY, { promisedAll })).toBe('passed'); + } + const zeroTests = 'test/a.test.ts:\n\n 0 pass\n 0 fail\nRan 0 tests across 1 file. [2.00ms]\n'; + expect(strictShardStatus({ timedOut: false, exitCode: 0, summary: summaryOf(zeroTests), expectedFiles: 1, evidenceComplete: true })).toBe('passed'); + }); + + test('paid lane: zero executed tests warn under selection and are passed-empty under EVALS_ALL', () => { + expect(zeroExecutionVerdict(0, PAID_LANE_POLICY, { promisedAll: false })).toBe('passed-with-warning'); + expect(zeroExecutionVerdict(0, PAID_LANE_POLICY, { promisedAll: true })).toBe('passed-empty'); + expect(zeroExecutionVerdict(3, PAID_LANE_POLICY, { promisedAll: true })).toBe('passed'); + const warnings: string[] = []; + const selective = applyHollowShardGuard([zero(0)], { evalsAll: false, warn: line => warnings.push(line) }); + expect(selective[0].status).toBe('passed'); + expect(warnings).toHaveLength(1); + expect(summaryExitCode(summarize(selective))).toBe(0); + const all = applyHollowShardGuard([zero(0)], { evalsAll: true, warn: () => {} }); + expect(all[0].status).toBe('passed-empty'); + expect(summaryExitCode(summarize(all))).toBe(1); + expect(applyHollowShardGuard([zero(null)], { evalsAll: true, warn: () => {} })[0].status).toBe('passed'); + }); + + test('duration seeds: one reader, lane predicate decides (free >= 0, paid > 0)', () => { + const dir = tempDir('seed-'); + const file = path.join(dir, 'seed.json'); + fs.writeFileSync(file, JSON.stringify({ durations: { 'a.ts': 0, 'b.ts': 5, 'c.ts': -1, 'd.ts': 'x', 'e.ts': Infinity } })); + expect(readDurationSeed(file, FREE_LANE_POLICY.acceptsSeedDuration)).toEqual({ status: 'ok', durations: { 'a.ts': 0, 'b.ts': 5 } }); + expect(readDurationSeed(file, PAID_LANE_POLICY.acceptsSeedDuration)).toEqual({ status: 'ok', durations: { 'b.ts': 5 } }); + expect(readDurationSeed(path.join(dir, 'missing.json'), () => true).status).toBe('missing'); + for (const corrupt of ['{not json', 'null']) { + fs.writeFileSync(file, corrupt); + expect(readDurationSeed(file, () => true).status).toBe('corrupt'); + } + writeDurationSeed(file, { 'z.ts': 2, 'a.ts': 1 }); + const written = JSON.parse(fs.readFileSync(file, 'utf8')); + expect(written.version).toBe(1); + expect(Object.keys(written.durations)).toEqual(['a.ts', 'z.ts']); + expect(fs.readdirSync(dir)).toEqual(['seed.json']); + }); +}); + +describe('shard engine: process lifecycle', () => { + test.skipIf(process.platform === 'win32')('a timed-out shard\'s whole process group is killed and the lanes report it as not passed', async () => { + const dir = tempDir('group-kill-'); + const pidFile = path.join(dir, 'grandchild.pid'); + const spinner = `const { spawn } = require('node:child_process'); + const p = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: 'ignore' }); + require('node:fs').writeFileSync(${JSON.stringify(pidFile)}, String(p.pid)); + const end = Date.now() + 600000; while (Date.now() < end) {}`; + const alive = (pid: number) => { + try { + process.kill(pid, 0); + return !(process.platform === 'linux' && fs.readFileSync(`/proc/${pid}/stat`, 'utf8').split(') ')[1].startsWith('Z')); + } catch { return false; } + }; + const waitDead = async (pid: number) => { + const deadline = Date.now() + 2_000; + while (alive(pid) && Date.now() < deadline) await Bun.sleep(20); + return !alive(pid); + }; + const free = await runFreeShard(['spin.test.ts'], 1, 1, { + rootDir: dir, quiet: true, log: () => {}, wallTimeoutMs: 1_500, logFilePath: path.join(dir, 'free.log'), + commandFor: () => ({ command: process.execPath, args: ['-e', spinner] }), + }); + expect(free.status).toBe('timed-out'); + expect(exitCodeFor(free.status)).not.toBe(0); + expect(await waitDead(Number(fs.readFileSync(pidFile, 'utf8')))).toBe(true); + expect(await waitDead(free.groupPid!)).toBe(true); + + fs.rmSync(pidFile); + const paid = await runPaidShard(['spin.test.ts'], 1, 1, { + rootDir: dir, timeoutMs: 1_500, jobs: 2, logDir: dir, log: () => {}, + env: { ...process.env, GSTACK_CLAUDE_CLI_VERSION: 'fixture' }, + commandFor: () => ({ command: process.execPath, args: ['-e', spinner] }), + }); + expect(paid.status).toBe('timed-out'); + expect(summaryExitCode(summarize([paid]))).toBe(1); + expect(await waitDead(Number(fs.readFileSync(pidFile, 'utf8')))).toBe(true); + }, 30_000); + + test('win32 semantics: no process group and never a negative-pid kill', async () => { + const original = Object.getOwnPropertyDescriptor(process, 'platform')!; + const kills = spyOn(process, 'kill'); + Object.defineProperty(process, 'platform', { value: 'win32' }); + try { + const signalled: string[] = []; + killProcessGroup({ pid: 4242, kill: (signal: string) => { signalled.push(signal); return true; } } as unknown as ChildProcess, 'SIGKILL'); + expect(signalled).toEqual(['SIGKILL']); + const result = await runShardChild({ + command: process.execPath, args: ['-e', 'setInterval(() => {}, 1000)'], + cwd: process.cwd(), env: process.env, timeoutMs: 500, hookStreams: () => [], + }); + expect(result.timedOut).toBe(true); + expect(kills.mock.calls.some(([pid]) => typeof pid === 'number' && pid < 0)).toBe(false); + } finally { + Object.defineProperty(process, 'platform', original); + kills.mockRestore(); + } + }, 30_000); + + test('an attached companion settles after the final group kill', async () => { + let settledAfterExit = false; + let child: ChildProcess | undefined; + const result = await runShardChild({ + command: process.execPath, args: ['-e', 'console.log("done")'], + cwd: process.cwd(), env: process.env, timeoutMs: 10_000, + hookStreams: spawned => [new Promise(resolve => { spawned.stdout!.resume(); spawned.stdout!.once('end', resolve); })], + attach: spawned => { + child = spawned; + return { signal: () => {}, settle: async () => { settledAfterExit = spawned.exitCode !== null || spawned.signalCode !== null; } }; + }, + }); + expect(result.exitCode).toBe(0); + expect(child).toBeDefined(); + expect(settledAfterExit).toBe(true); + }, 30_000); +}); + +describe('shard engine: capture and isolation', () => { + test('log capture spools every chunk; a write error fails the log once, without throwing', async () => { + const dir = tempDir('shard-log-'); + const first = nextShardLogPath(dir, 'gstack-free-test'); + const second = nextShardLogPath(dir, 'gstack-free-test'); + expect(first).not.toBe(second); + expect(path.basename(first)).toMatch(new RegExp(`^gstack-free-test-.+-${process.pid}-\\d+\\.log$`)); + const log = openShardLog(first, '[test:unit]', 0o600); + log.write('one\n'); + log.write(Buffer.from('two\n')); + await new Promise(resolve => log.stream.end(() => resolve())); + expect(fs.readFileSync(first, 'utf8')).toBe('one\ntwo\n'); + if (process.platform !== 'win32') expect(fs.statSync(first).mode & 0o777).toBe(0o600); + expect(log.failed).toBe(false); + + const errors = spyOn(console, 'error').mockImplementation(() => {}); + try { + const broken = openShardLog(path.join(dir, 'missing', 'x.log'), '[test:unit]'); + await new Promise(resolve => broken.stream.once('error', resolve)); + broken.write('dropped'); + expect(broken.failed).toBe(true); + expect(errors.mock.calls.filter(([line]) => String(line).includes('could not write the full log'))).toHaveLength(1); + } finally { errors.mockRestore(); } + }); + + test('each shard gets its own tmp and Chromium profile, removed by the backstop', async () => { + const base = { KEEP: '1', TMPDIR: '/inherited', CHROMIUM_PROFILE: '/shared-profile' }; + const a = createShardSandbox('gstack-unit-shard-', base); + const b = createShardSandbox('gstack-unit-shard-', base, { realpath: true }); + try { + expect(a.stateDir).not.toBe(b.stateDir); + expect(b.stateDir).toBe(fs.realpathSync(b.stateDir)); + for (const box of [a, b]) { + expect(box.env.KEEP).toBe('1'); + expect(box.env.GSTACK_HOME).toBeUndefined(); + for (const key of ['TMPDIR', 'TEMP', 'TMP']) expect(box.env[key]).toBe(box.tmp); + expect(box.env.CHROMIUM_PROFILE).toBe(path.join(box.stateDir, 'chromium-profile')); + expect(fs.statSync(box.tmp).isDirectory()).toBe(true); + } + expect(base.TMPDIR).toBe('/inherited'); + fs.writeFileSync(path.join(a.tmp, 'left-behind'), 'x'); + await removeShardSandbox(a.stateDir); + expect(fs.existsSync(a.stateDir)).toBe(false); + await removeShardSandbox(a.stateDir); + } finally { + fs.rmSync(a.stateDir, { recursive: true, force: true }); + fs.rmSync(b.stateDir, { recursive: true, force: true }); + } + }); + + test('flag loop: declared handlers consume values; anything else is Unknown argument', () => { + const seen: Array<[string, string | undefined]> = []; + parseCliFlags(['--on', '--value', 'x', '--value'], { + '--on': () => { seen.push(['--on', undefined]); }, + '--value': next => { seen.push(['--value', next()]); }, + }); + expect(seen).toEqual([['--on', undefined], ['--value', 'x'], ['--value', undefined]]); + expect(() => parseCliFlags(['--nope'], {})).toThrow('Unknown argument: --nope'); + expect(() => parseCliFlags(['constructor'], {})).toThrow('Unknown argument: constructor'); + }); +}); From c7d6ca70bd4c09743552c63f2b0809debd8ade67 Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 30 Sep 2026 06:03:29 +0000 Subject: [PATCH 4/6] refactor(shard-engine): decompose runFreeShard and runPaidShard to <= 150 lines Output-neutral extraction under the fixture-corpus equivalence and runner tests: captureFreeStream, explainFreeVerdict and logFreeRecovery (free); paidShardCommand, settleShardSpool, settleBootstrapRetention and printLogTail (paid). The bootstrap scope-creation block that bootstrap-retention.test.ts evaluates stays verbatim. --- scripts/test-free-shards.ts | 158 +++++++++++++++++++++--------------- scripts/test-paid-shards.ts | 146 ++++++++++++++++++--------------- 2 files changed, 176 insertions(+), 128 deletions(-) diff --git a/scripts/test-free-shards.ts b/scripts/test-free-shards.ts index 445114291..f37989b4e 100755 --- a/scripts/test-free-shards.ts +++ b/scripts/test-free-shards.ts @@ -1757,6 +1757,86 @@ function trackShardBrowser(stateDir: string, env: NodeJS.ProcessEnv) { }; } +/** + * Drain one child pipe into `onChunk`. Capture failures are recorded as data + * in `failures`, never thrown: the caller still waits for the child's real exit. + */ +function captureFreeStream( + stream: NodeJS.ReadableStream | null, + origin: StreamOrigin, + failures: Map, + onChunk: (chunk: Buffer | string) => void, +): Promise { + return new Promise((resolve) => { + if (!stream) { + failures.set(origin, new Error('configured pipe is missing')); + resolve(); + return; + } + 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. + if (error) failures.set(origin, error); + else if (!failures.has(origin)) failures.set(origin, new Error('stream closed before end')); + resolve(); + }; + // Even an already-destroyed pipe can emit error on the next tick. + stream.on('error', incomplete); + stream.once('end', () => { ended = true; resolve(); }); + stream.once('close', () => { + if (!ended) incomplete(readable.errored); + else resolve(); + }); + stream.on('data', onChunk); + if (ended) resolve(); + else if (readable.destroyed) incomplete(readable.errored); + }); +} + +/** Why a shard did not pass, on stderr (the epilogue repeats the names). */ +function explainFreeVerdict(label: string, status: FreeShardStatus, facts: { + cleanupError: string | null; stateDir: string; evidenceComplete: boolean; exitCode: number | null; + summary: ReturnType; expectedFiles: number; wallTimeoutMs: number; +}): void { + const { summary, exitCode } = facts; + if (facts.cleanupError) console.error(`${label} browser cleanup failed: ${facts.cleanupError}; retained ${facts.stateDir}`); + if (status === 'timed-out') { + console.error( + `${label} exceeded the ${Math.round(facts.wallTimeoutMs / 1000)}s wall-clock deadline — ` + + 'killed the process group. Reporting as TIMED-OUT (distinct from failed).', + ); + } else if (status === 'failed' && facts.evidenceComplete && (exitCode ?? 1) === 0) { + const reason = summary.failedTests > 0 || summary.unhandledBetweenTests > 0 + ? `reported ${summary.failedTests} failing test(s) and ${summary.unhandledBetweenTests} unhandled error(s) between tests` + : summary.terminalFileCounts.length === 0 + ? "never printed bun's terminal summary — the run was truncated (a process.exit fired mid-suite)" + : `bun's summary reported ${summary.terminalFileCounts.join(', ')} file(s), expected ${facts.expectedFiles}`; + console.error(`${label} exited 0 but ${reason}. Treating as FAILED.`); + } else if (status === 'failed' && (exitCode ?? 1) !== 0) { + console.error(`${label} failed with exit code ${exitCode ?? 'signal'}`); + } +} + +/** The recovery step and, only when the failure scope is complete, a focused rerun. */ +function logFreeRecovery(log: (line: string) => void, outcome: FreeShardOutcome, facts: { + cleanupError: string | null; logWriteFailed: boolean; captureIncomplete: boolean; rootDir: string; +}): void { + const problem = facts.cleanupError ? 'Owned-process cleanup is unconfirmed; inspect the retained state before another run.' + : facts.logWriteFailed ? 'The evidence log could not be retained; repair the log destination before another run.' + : facts.captureIncomplete ? 'Evidence capture is incomplete; repair the stream or early exit before another run.' + : outcome.status === 'timed-out' ? 'Execution exceeded its deadline; inspect the last completed step before changing code or rerunning.' + : 'A test or module failed; the root cause is not established. Inspect the full log and repair the cause first.'; + log(`[test:free] Recovery: ${problem} See docs/TESTING_INTERNALS.md.`); + const focused = outcome.failingFiles.filter(file => outcome.files.includes(file) && fs.existsSync(path.resolve(facts.rootDir, file))); + if (!outcome.unattributedFailures && focused.length) { + log(`[test:free] After repair, focused check: bun test ${focused.map(file => `'${file.replaceAll("'", "'\\''")}'`).join(' ')}`); + } else { + log('[test:free] No complete narrower failure scope is available; do not treat a subset rerun as complete coverage.'); + } +} + /** One line per shard, printed after the run: `[test:free] shard i/N: M files, XXs, pass|fail|timed-out`. */ function shardEpilogue(outcome: FreeShardOutcome, totalShards: number): string { return `[test:free] shard ${outcome.shard}/${totalShards}: ${outcome.files.length} files, ` @@ -1822,10 +1902,8 @@ export async function runFreeShard( // 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. + // CLI renders otherwise share the repo's .gstack/browse.json, where concurrent + // shards and prior daemons replace each other's state; override inherited state. env.BROWSE_STATE_FILE = path.join(stateDir, '.gstack', 'browse.json'); env.GSTACK_FREE_SHARD_ID = randomUUID(); @@ -1849,37 +1927,11 @@ export async function runFreeShard( const captureFailures = new Map(); 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; - } - 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 - // while the caller is still waiting for the child's real exit. - if (error) captureFailures.set(origin, error); - else if (!captureFailures.has(origin)) captureFailures.set(origin, new Error('stream closed before end')); - resolve(); - }; - // Even an already-destroyed pipe can emit error on the next tick. - stream.on('error', incomplete); - stream.once('end', () => { ended = true; resolve(); }); - stream.once('close', () => { - if (!ended) incomplete(readable.errored); - else resolve(); - }); - stream.on('data', (chunk: Buffer | string) => { - classifier.write(chunk, origin); // strict verdict ALWAYS sees the full stream - shardLog.write(chunk); - reporter.write(chunk, origin); - if (options.verbose) emitToConsole(typeof chunk === 'string' ? chunk : chunk.toString('utf8'), origin); - }); - if (ended) resolve(); - else if (readable.destroyed) incomplete(readable.errored); + captureFreeStream(stream, origin, captureFailures, (chunk) => { + classifier.write(chunk, origin); // strict verdict ALWAYS sees the full stream + shardLog.write(chunk); + reporter.write(chunk, origin); + if (options.verbose) emitToConsole(typeof chunk === 'string' ? chunk : chunk.toString('utf8'), origin); }).then(() => { drained.add(origin); }); let child: ShardChildResult = { exitCode: null, timedOut: false, groupPid: null }; @@ -1932,23 +1984,10 @@ export async function runFreeShard( status = 'failed'; } - if (cleanupError) console.error(`${label} browser cleanup failed: ${cleanupError}; retained ${stateDir}`); - - if (status === 'timed-out') { - console.error( - `${label} exceeded the ${Math.round(wallTimeoutMs / 1000)}s wall-clock deadline — ` - + 'killed the process group. Reporting as TIMED-OUT (distinct from failed).', - ); - } else if (status === 'failed' && !cleanupError && !logWriteFailed && captureFailures.size === 0 && (exitCode ?? 1) === 0) { - const reason = summary.failedTests > 0 || summary.unhandledBetweenTests > 0 - ? `reported ${summary.failedTests} failing test(s) and ${summary.unhandledBetweenTests} unhandled error(s) between tests` - : summary.terminalFileCounts.length === 0 - ? "never printed bun's terminal summary — the run was truncated (a process.exit fired mid-suite)" - : `bun's summary reported ${summary.terminalFileCounts.join(', ')} file(s), expected ${files.length}`; - console.error(`${label} exited 0 but ${reason}. Treating as FAILED.`); - } else if (status === 'failed' && (exitCode ?? 1) !== 0) { - console.error(`${label} failed with exit code ${exitCode ?? 'signal'}`); - } + explainFreeVerdict(label, status, { + cleanupError, stateDir, exitCode, summary, expectedFiles: files.length, wallTimeoutMs, + evidenceComplete: !cleanupError && !logWriteFailed && captureFailures.size === 0, + }); const report = reporter.report(); const failingFiles = status === 'passed' ? [] : [...new Set([ @@ -1970,18 +2009,9 @@ export async function runFreeShard( log(shardEpilogue(outcome, totalShards)); for (const line of buildRunEpilogue(status, report, outcome.elapsedMs, logPath)) log(line); if (status !== 'passed') { - const problem = cleanupError ? 'Owned-process cleanup is unconfirmed; inspect the retained state before another run.' - : logWriteFailed ? 'The evidence log could not be retained; repair the log destination before another run.' - : captureFailures.size || !report.sawTerminalSummary ? 'Evidence capture is incomplete; repair the stream or early exit before another run.' - : status === 'timed-out' ? 'Execution exceeded its deadline; inspect the last completed step before changing code or rerunning.' - : 'A test or module failed; the root cause is not established. Inspect the full log and repair the cause first.'; - log(`[test:free] Recovery: ${problem} See docs/TESTING_INTERNALS.md.`); - const focused = failingFiles.filter(file => files.includes(file) && fs.existsSync(path.resolve(rootDir, file))); - if (!unattributedFailures && focused.length) { - log(`[test:free] After repair, focused check: bun test ${focused.map(file => `'${file.replaceAll("'", "'\\''")}'`).join(' ')}`); - } else { - log('[test:free] No complete narrower failure scope is available; do not treat a subset rerun as complete coverage.'); - } + logFreeRecovery(log, outcome, { + cleanupError, logWriteFailed, rootDir, captureIncomplete: captureFailures.size > 0 || !report.sawTerminalSummary, + }); } return outcome; } diff --git a/scripts/test-paid-shards.ts b/scripts/test-paid-shards.ts index eb80b1887..f60bf4b54 100644 --- a/scripts/test-paid-shards.ts +++ b/scripts/test-paid-shards.ts @@ -74,6 +74,7 @@ import { zeroExecutionVerdict, type LanePolicy, type ShardChildResult, + type ShardLog, } 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'; @@ -696,6 +697,80 @@ function readLogTail(logPath: string, maxBytes = FAILURE_TAIL_BYTES): string { } } +function paidShardCommand(files: string[], rootDir: string, timeoutMs: number, options: RunShardsOptions): ShardCommand { + return { + command: process.execPath, + args: [...buildPaidShardArgs( + exactTestFileSelectors(files, rootDir), + timeoutMs, + options.withinShardConcurrency ?? DEFAULT_WITHIN_SHARD_CONCURRENCY, + retriesForFiles(files), + ), ...(options.casePatterns ? ['--test-name-pattern', options.casePatterns[files[0]]] : [])], + }; +} + +/** Print the last FAILURE_TAIL_BYTES of a failed shard's log to stdout. */ +function printLogTail(label: string, logPath: string): void { + const tail = readLogTail(logPath); + if (tail.length === 0) return; + process.stdout.write(`${label} last ${Math.min(tail.length, FAILURE_TAIL_BYTES)} bytes of ${logPath}:\n`); + process.stdout.write(tail.endsWith('\n') ? tail : `${tail}\n`); +} + +/** Acknowledge bootstrap dependency retention; an unconfirmed scope keeps the shard state. */ +async function settleBootstrapRetention( + scope: NonNullable>, + deadlineMs: number, + label: string, + log: (line: string) => void, +): Promise<{ failed: boolean; removable: boolean }> { + try { + const retained = await scope.cleanup(deadlineMs); + if (!retained.complete) log(`${label} bootstrap retention incomplete; qualification failed`); + return { failed: !retained.complete, removable: retained.removable }; + } catch { + log(`${label} bootstrap retention acknowledgment failed; preserving shard state`); + return { failed: true, removable: false }; + } +} + +/** + * End the shard's log spool within the shard's own deadline. An error, a + * premature close or the deadline marks the spool failed (and logs once); + * returns true when the deadline expired first. + */ +function settleShardSpool(spool: ShardLog, deadlineMs: number, onIncomplete: () => void): Promise { + const logStream = spool.stream; + return new Promise((resolve) => { + let settled = false; + let expired = false; + let timer: ReturnType | undefined; + const finish = (complete: boolean) => { + if (settled) return; + settled = true; + clearTimeout(timer); + logStream.off('error', onError); + logStream.off('close', onClose); + if (!complete) { + spool.failed = true; + logStream.destroy(); + onIncomplete(); + } + resolve(expired); + }; + const onError = () => finish(false); + const onClose = () => finish(logStream.writableFinished && !spool.failed); + const expire = () => { expired = true; finish(false); }; + logStream.once('error', onError); + logStream.once('close', onClose); + if (Date.now() >= deadlineMs) { expire(); return; } + if (spool.failed || logStream.destroyed) { finish(false); return; } + timer = setTimeout(expire, deadlineMs - Date.now()); + try { logStream.end(() => finish(logStream.writableFinished && !spool.failed)); } + catch { finish(false); } + }); +} + export async function runPaidShard( files: string[], shardNumber: number, @@ -712,17 +787,7 @@ export async function runPaidShard( const log = options.log ?? ((line: string) => console.log(line)); const label = `[test:paid] shard ${shardNumber}/${totalShards}`; - const { command, args } = options.commandFor - ? options.commandFor(files) - : { - command: process.execPath, - args: [...buildPaidShardArgs( - exactTestFileSelectors(files, rootDir), - timeoutMs, - options.withinShardConcurrency ?? DEFAULT_WITHIN_SHARD_CONCURRENCY, - retriesForFiles(files), - ), ...(options.casePatterns ? ['--test-name-pattern', options.casePatterns[files[0]]] : [])], - }; + const { command, args } = options.commandFor ? options.commandFor(files) : paidShardCommand(files, rootDir, timeoutMs, options); const baseEnv = { ...(options.env ?? process.env) }; if (options.evalDirBase) { @@ -763,7 +828,6 @@ export async function runPaidShard( // Printed at START so a wedged shard is inspectable live, mid-run. 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(); @@ -814,45 +878,10 @@ export async function runPaidShard( throw error; } finally { if (incompleteCapture) log(`${label} incomplete child capture: ${JSON.stringify(incompleteCapture)}; retained log prefix: ${logPath}`); - await new Promise((resolve) => { - let settled = false; - let timer: ReturnType | undefined; - const finish = (complete: boolean) => { - if (settled) return; - settled = true; - clearTimeout(timer); - logStream.off('error', onError); - logStream.off('close', onClose); - if (!complete) { - spool.failed = true; - logStream.destroy(); - log(`${label} incomplete log capture; retained prefix: ${logPath}`); - } - resolve(); - }; - const onError = () => finish(false); - 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 (spool.failed || logStream.destroyed) { finish(false); return; } - timer = setTimeout(expire, shardDeadline - Date.now()); - try { logStream.end(() => finish(logStream.writableFinished && !spool.failed)); } - catch { finish(false); } - }); + if (await settleShardSpool(spool, shardDeadline, () => log(`${label} incomplete log capture; retained prefix: ${logPath}`))) timedOut = true; let retentionRemovable = true; if (bootstrapRetention) { - try { - const retained = await bootstrapRetention.cleanup(shardDeadline); - retentionFailed = !retained.complete; - retentionRemovable = retained.removable; - if (retentionFailed) log(`${label} bootstrap retention incomplete; qualification failed`); - } catch { - retentionFailed = true; - retentionRemovable = false; - log(`${label} bootstrap retention acknowledgment failed; preserving shard state`); - } + ({ failed: retentionFailed, removable: retentionRemovable } = await settleBootstrapRetention(bootstrapRetention, shardDeadline, label, log)); } // Async, best-effort backstop: group-SIGKILLed tests never clean up. if (retentionRemovable) await removeShardSandbox(stateDir); @@ -860,14 +889,9 @@ export async function runPaidShard( const summary = classifier.end(); - // Pass expectedFiles so a shard whose bun child ran fewer files than planned - // (or zero, all self-skipped) with exit 0 is NOT recorded 'passed' — the - // invisible-non-execution class this runner exists to kill. bun prints - // "Ran N tests across M files" with M = selected files even when every test - // self-skips, so terminalFileCounts must include files.length. Enforced for - // injected commandFor (tests) too, matching the free runner — fake passing - // commands must print a synthetic `Ran N tests across M files. [Xms]` line, - // so tests can pin the summary-missing => failure backstop. + // expectedFiles: a shard whose bun child ran fewer files than planned with + // exit 0 is NOT 'passed' (bun counts self-skipped files, so M = planned). + // Fake commandFor children must print a synthetic `Ran N tests across M files`. const expectedFiles = files.length; let status: ShardStatus = strictShardStatus({ timedOut, exitCode, summary, expectedFiles, @@ -885,13 +909,7 @@ export async function runPaidShard( // Failure debuggability without the RAM cost: read back only the log's // tail. Live mode already streamed everything, so no re-print there. - if (status !== 'passed' && !streamLive) { - const tail = readLogTail(logPath); - if (tail.length > 0) { - process.stdout.write(`${label} last ${Math.min(tail.length, FAILURE_TAIL_BYTES)} bytes of ${logPath}:\n`); - process.stdout.write(tail.endsWith('\n') ? tail : `${tail}\n`); - } - } + if (status !== 'passed' && !streamLive) printLogTail(label, logPath); const logSuffix = status === 'passed' ? '' : ` — full log: ${logPath}`; log(`${label} ${status.toUpperCase()} in ${Math.round(elapsedMs / 1000)}s (exit ${exitCode ?? 'signal'})${logSuffix}`); From fe0a9183f84cd5f8851d54db7b9a8f68b9345621 Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 30 Sep 2026 06:06:16 +0000 Subject: [PATCH 5/6] test(ratchet-c): enforcing module/function size ratchet and moved-code touchfile coverage Ratchet (c) ships enforcing: test/helpers/module-size.ts counts file and top-level function lengths by brace matching over masked source (strings, comments, regex literals and template text masked; ${} expressions kept), covering function declarations, arrow functions assigned to consts and route-table handler properties, with no parser dependency. Its self-test uses template literals and code-fence braces copied from scripts/resolvers/review.ts and design.ts. test/fixtures/module-size-ratchet.json binds scripts/lib/shard-engine.ts (<= 800 lines, <= 150 per function) and records the residual runner sizes (free 2352, paid 1921) as non-growth caps; allowlist entries are keyed on file plus matched text and need a reason. Failure output lists file:line, the rule, Fix: and the allowlist path. touchfiles.test.ts gains the moved-code superset check over test/fixtures/touchfile-move-goldens/ (W2 golden recorded at 96764e80: test-strict-output.ts and test-paid-shards.ts global, test-free-shards.ts none). --- test/fixtures/module-size-ratchet.json | 11 + .../module-size/resolver-snippets.ts.txt | 44 ++++ .../w2-shard-engine.json | 11 + test/helpers/module-size.ts | 231 ++++++++++++++++++ test/module-size-ratchet.test.ts | 104 ++++++++ test/test-of-test-ratchet.test.ts | 1 + test/touchfiles.test.ts | 58 +++++ 7 files changed, 460 insertions(+) create mode 100644 test/fixtures/module-size-ratchet.json create mode 100644 test/fixtures/module-size/resolver-snippets.ts.txt create mode 100644 test/fixtures/touchfile-move-goldens/w2-shard-engine.json create mode 100644 test/helpers/module-size.ts create mode 100644 test/module-size-ratchet.test.ts diff --git a/test/fixtures/module-size-ratchet.json b/test/fixtures/module-size-ratchet.json new file mode 100644 index 000000000..16c0a6d4e --- /dev/null +++ b/test/fixtures/module-size-ratchet.json @@ -0,0 +1,11 @@ +{ + "newModules": [ + { "file": "scripts/lib/shard-engine.ts", "movedFrom": "scripts/test-strict-output.ts" }, + { "file": "test/helpers/module-size.ts", "movedFrom": "(new: ratchet (c) counter)" } + ], + "residualFiles": [ + { "file": "scripts/test-free-shards.ts", "maxLines": 2352, "reason": "free lane policy after the shard engine took spawn, sandbox, logs, seeds and flags" }, + { "file": "scripts/test-paid-shards.ts", "maxLines": 1921, "reason": "paid lane policy after the shard engine took sandbox, logs, seeds, verdicts and flags" } + ], + "allowlist": [] +} diff --git a/test/fixtures/module-size/resolver-snippets.ts.txt b/test/fixtures/module-size/resolver-snippets.ts.txt new file mode 100644 index 000000000..20ab2dd9a --- /dev/null +++ b/test/fixtures/module-size/resolver-snippets.ts.txt @@ -0,0 +1,44 @@ +// Ratchet (c) counter self-test input. Template text below is copied from +// scripts/resolvers/review.ts (dashboard, design-board reload) and +// scripts/resolvers/design.ts (sketch code fences). Read as text, never run. +import { toShellPath, type TemplateContext } from './types'; + +export function generateDashboardLike(ctx: TemplateContext): string { + return `## Review dashboard +${ctx.skillName === 'ship' ? `**REVIEW READINESS DASHBOARD** + +| Review | Runs | Last run | Status | Required | +|---|---:|---|---|---| +| {row and suffix} | {count} | {timestamp or —} | {actual status and reason} | {yes/no} | + +VERDICT: {CLEARED or NOT CLEARED} — {reason}` : `\`\`\` ++====================================================================+ +| REVIEW READINESS DASHBOARD | ++====================================================================+ +\`\`\``} + + \`jq -nc --arg html "$_DESIGN_DIR/design-board.html" '{html: $html}' | curl -sS -X POST "\${BOARD_URL}api/reload" -H 'Content-Type: application/json' --data-binary @-\` +`; +} + +export function generateDesignSketchLike(ctx: TemplateContext): string { + const fence = '```'; + return `Create a private directory for it first: +\`\`\`bash +mktemp -d "\${TMPDIR:-/tmp}/gstack-sketch.XXXXXX" +\`\`\` +\`\`\`bash +bun run ${toShellPath(ctx.paths.binDir)}/gstack-render.ts /sketch.html --screenshot /sketch.png --width 1280 +\`\`\` +${fence} { unbalanced { braces in prose ${fence}`; +} + +const arrowWithRegex = (value: string): string => + value.replace(/[{}]/g, '').split(/\}/).join('{'); + +export const handlerTable = [ + { path: '/x', handler: async (req: Request) => { + const body = `{"ok": ${JSON.stringify(req.url)}}`; + return new Response(body); + } }, +]; diff --git a/test/fixtures/touchfile-move-goldens/w2-shard-engine.json b/test/fixtures/touchfile-move-goldens/w2-shard-engine.json new file mode 100644 index 000000000..42ba31849 --- /dev/null +++ b/test/fixtures/touchfile-move-goldens/w2-shard-engine.json @@ -0,0 +1,11 @@ +{ + "recordedFrom": "96764e80 (before the W2 shard-engine move)", + "moves": { + "scripts/lib/shard-engine.ts": ["scripts/test-strict-output.ts", "scripts/test-free-shards.ts", "scripts/test-paid-shards.ts"] + }, + "selections": { + "scripts/test-strict-output.ts": { "e2e": "global", "judges": "global" }, + "scripts/test-free-shards.ts": { "e2e": [], "judges": [] }, + "scripts/test-paid-shards.ts": { "e2e": "global", "judges": "global" } + } +} diff --git a/test/helpers/module-size.ts b/test/helpers/module-size.ts new file mode 100644 index 000000000..420e27c64 --- /dev/null +++ b/test/helpers/module-size.ts @@ -0,0 +1,231 @@ +/** + * Ratchet (c) counter: file and top-level function lengths by brace + * matching, with no parser dependency. Strings, comments, regex literals and + * template-literal text are masked first (template `${}` expressions stay + * code), so braces inside generated prose or code fences never count. + * Functions = function declarations and arrow/function expressions assigned + * to top-level consts, plus route-table `handler` properties at any depth. + */ +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +export const MAX_MODULE_LINES = 800; +export const MAX_FUNCTION_LINES = 150; +export const RATCHET_CONFIG = 'test/fixtures/module-size-ratchet.json'; + +export interface FunctionSpan { name: string; line: number; lines: number; match: string } + +const REGEX_AFTER_PUNCTUATION = '(,=:[!&|?{};+-*%<>~^'; +const REGEX_AFTER_WORD = new Set(['return', 'typeof', 'case', 'do', 'else', 'in', 'of', 'new', 'delete', 'void', 'throw', 'yield', 'await']); + +/** Same length as `source`, newlines kept; non-code characters become spaces. */ +export function maskSource(source: string): string { + const out = source.split(''); + const blank = (from: number, to: number) => { + for (let k = from; k < to; k += 1) if (out[k] !== '\n') out[k] = ' '; + }; + // A masked literal leaves an operand placeholder, so a following `/` reads as division. + const literal = (from: number, to: number) => { + blank(from, to); + if (to > from && out[to - 1] !== '\n') out[to - 1] = '0'; + }; + const regexAllowed = (at: number) => { + let j = at - 1; + while (j >= 0 && /\s/.test(out[j])) j -= 1; + if (j < 0) return true; + if (/[\w$]/.test(out[j])) { + let k = j; + while (k >= 0 && /[\w$]/.test(out[k])) k -= 1; + return REGEX_AFTER_WORD.has(out.slice(k + 1, j + 1).join('')); + } + return REGEX_AFTER_PUNCTUATION.includes(out[j]); + }; + const templateDepth: number[] = []; + let i = 0; + const inTemplateText = () => templateDepth.length > 0 && templateDepth[templateDepth.length - 1] < 0; + while (i < source.length) { + if (inTemplateText()) { + const start = i; + while (i < source.length && source[i] !== '`' && !(source[i] === '$' && source[i + 1] === '{')) i += source[i] === '\\' ? 2 : 1; + blank(start, i); + if (source[i] === '`') { out[i] = '0'; templateDepth.pop(); i += 1; continue; } + blank(i, i + 2); + templateDepth[templateDepth.length - 1] = 0; + i += 2; + continue; + } + const ch = source[i]; + const next = source[i + 1]; + if (ch === '/' && next === '/') { + const end = source.indexOf('\n', i); + blank(i, end < 0 ? source.length : end); + i = end < 0 ? source.length : end; + continue; + } + if (ch === '/' && next === '*') { + const end = source.indexOf('*/', i + 2); + blank(i, end < 0 ? source.length : end + 2); + i = end < 0 ? source.length : end + 2; + continue; + } + if (ch === '"' || ch === "'") { + const start = i; + i += 1; + while (i < source.length && source[i] !== ch && source[i] !== '\n') i += source[i] === '\\' ? 2 : 1; + i += 1; + literal(start, Math.min(i, source.length)); + continue; + } + if (ch === '`') { + out[i] = ' '; + templateDepth.push(-1); + i += 1; + continue; + } + if (ch === '/' && regexAllowed(i)) { + const start = i; + let inClass = false; + i += 1; + while (i < source.length && source[i] !== '\n' && (inClass || source[i] !== '/')) { + if (source[i] === '\\') i += 1; + else if (source[i] === '[') inClass = true; + else if (source[i] === ']') inClass = false; + i += 1; + } + i += 1; + while (/[a-z]/i.test(source[i] ?? '')) i += 1; + literal(start, i); + continue; + } + if (templateDepth.length > 0) { + if (ch === '{') templateDepth[templateDepth.length - 1] += 1; + if (ch === '}') { + if (templateDepth[templateDepth.length - 1] === 0) { + out[i] = ' '; + templateDepth[templateDepth.length - 1] = -1; + i += 1; + continue; + } + templateDepth[templateDepth.length - 1] -= 1; + } + } + i += 1; + } + return out.join(''); +} + +const OPEN = '([{'; +const CLOSE = ')]}'; +const CONTINUES = /^(?:[{([.?:=|&,<>+\-*/%]|=>)/; + +/** Index just past the end of the expression or statement that starts at `from` (depth-relative). */ +function spanEnd(masked: string, from: number, stopAtComma: boolean): number { + let depth = 0; + for (let k = from; k < masked.length; k += 1) { + const ch = masked[k]; + if (OPEN.includes(ch)) depth += 1; + else if (CLOSE.includes(ch)) { + depth -= 1; + if (depth < 0) return k; + if (depth === 0) { + const rest = masked.slice(k + 1).trimStart(); + if (stopAtComma ? /^[,}\]]/.test(rest) || !CONTINUES.test(rest) : !CONTINUES.test(rest)) return k + 1; + } + } else if (depth === 0 && (ch === ';' || (stopAtComma && ch === ','))) return k; + } + return masked.length; +} + +const lineAt = (source: string, index: number) => source.slice(0, index).split('\n').length; + +function looksLikeFunction(masked: string, valueStart: number): boolean { + const rest = masked.slice(valueStart).replace(/^\s*async\b/, '').trimStart(); + if (/^function\b/.test(rest) || /^[A-Za-z_$][\w$]*\s*=>/.test(rest)) return true; + const open = rest.startsWith('<') ? rest.indexOf('(') : rest.startsWith('(') ? 0 : -1; + if (open < 0) return false; + let depth = 0; + let close = open; + for (; close < rest.length; close += 1) { + if (OPEN.includes(rest[close])) depth += 1; + else if (CLOSE.includes(rest[close]) && --depth === 0) break; + } + return /^\s*(?::[^;]*?)?=>/.test(rest.slice(close + 1)); +} + +export function measureFunctions(source: string): FunctionSpan[] { + const masked = maskSource(source); + const depthAt: number[] = new Array(masked.length); + let depth = 0; + for (let k = 0; k < masked.length; k += 1) { + depthAt[k] = depth; + if (OPEN.includes(masked[k])) depth += 1; + else if (CLOSE.includes(masked[k])) depth = Math.max(0, depth - 1); + } + const spans: FunctionSpan[] = []; + const record = (name: string, start: number, end: number) => { + const line = lineAt(source, start); + spans.push({ name, line, lines: lineAt(source, Math.max(start, end - 1)) - line + 1, + match: source.slice(start, source.indexOf('\n', start) < 0 ? undefined : source.indexOf('\n', start)).trim() }); + }; + const declaration = /^[ \t]*(?:export\s+)?(?:default\s+)?(?:async\s+)?function\s*\*?\s*([A-Za-z_$][\w$]*)?/gm; + for (const m of masked.matchAll(declaration)) { + const start = m.index! + m[0].length - m[0].trimStart().length; + if (depthAt[start] === 0) record(m[1] ?? 'default', start, spanEnd(masked, start, false)); + } + const assigned = /^[ \t]*(?:export\s+)?(?:const|let|var)\s+([A-Za-z_$][\w$]*)[^=\n]*?=(?!=)/gm; + for (const m of masked.matchAll(assigned)) { + const start = m.index! + m[0].length - m[0].trimStart().length; + if (depthAt[start] === 0 && looksLikeFunction(masked, m.index! + m[0].length)) record(m[1], start, spanEnd(masked, start, false)); + } + for (const m of masked.matchAll(/\bhandler\s*(?::|(?=\())/g)) { + record('handler', m.index!, spanEnd(masked, m.index!, true)); + } + return spans.sort((a, b) => a.line - b.line); +} + +export interface RatchetConfig { + newModules: Array<{ file: string; movedFrom: string }>; + residualFiles: Array<{ file: string; maxLines: number; reason: string }>; + allowlist: Array<{ file: string; match: string; reason?: string }>; +} + +export function countLines(source: string): number { + return source.length === 0 ? 0 : source.split('\n').length - (source.endsWith('\n') ? 1 : 0); +} + +/** Every violation as `file:line `; empty when the ratchet holds. */ +export function ratchetViolations(config: RatchetConfig, read: (file: string) => string): string[] { + const violations: string[] = []; + for (const entry of config.allowlist) { + if (!entry.reason?.trim()) violations.push(`${RATCHET_CONFIG}:0 allowlist entry without a reason: ${entry.file} "${entry.match}"`); + } + for (const { file } of config.newModules) { + const source = read(file); + const lines = countLines(source); + if (lines > MAX_MODULE_LINES) violations.push(`${file}:${lines} module has ${lines} lines > ${MAX_MODULE_LINES}`); + for (const span of measureFunctions(source)) { + if (span.lines <= MAX_FUNCTION_LINES) continue; + if (config.allowlist.some(entry => entry.file === file && entry.reason?.trim() && span.match.includes(entry.match))) continue; + violations.push(`${file}:${span.line} ${span.match} (${span.name}: ${span.lines} lines > ${MAX_FUNCTION_LINES})`); + } + } + for (const { file, maxLines } of config.residualFiles) { + const lines = countLines(read(file)); + if (lines > maxLines) violations.push(`${file}:${lines} residual file grew to ${lines} lines > recorded ${maxLines}`); + } + return violations; +} + +export function formatRatchetFailure(violations: string[]): string { + return [ + `Ratchet (c) — module and function size — ${violations.length} violation(s):`, + ...violations.map(violation => ` ${violation}`), + `Rule: refactor-wave owner modules stay at or under ${MAX_MODULE_LINES} lines and ${MAX_FUNCTION_LINES} lines per top-level function, and residual files may not grow past their post-refactor size, because oversized files and functions are where this code kept drifting.`, + 'Fix: split the module or function — move a cohesive step into its own named function or a sibling module; for a residual file, put new code in its owner module instead.', + `Allowlist: ${RATCHET_CONFIG} ("allowlist": { file, match, reason }, keyed on file plus matched text) — legitimate only for a data table or generated literal whose length is not logic; every entry needs a reason.`, + ].join('\n'); +} + +export function loadRatchetConfig(root: string): RatchetConfig { + return JSON.parse(fs.readFileSync(path.join(root, RATCHET_CONFIG), 'utf8')) as RatchetConfig; +} diff --git a/test/module-size-ratchet.test.ts b/test/module-size-ratchet.test.ts new file mode 100644 index 000000000..66e146b4d --- /dev/null +++ b/test/module-size-ratchet.test.ts @@ -0,0 +1,104 @@ +/** + * Ratchet (c): refactor-wave owner modules stay <= 800 lines and <= 150 lines + * per top-level function; residual files may not grow past their recorded + * post-refactor size. Entries live in test/fixtures/module-size-ratchet.json; + * the counter is test/helpers/module-size.ts (brace matching, no parser). + */ +import { describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { + MAX_FUNCTION_LINES, + RATCHET_CONFIG, + formatRatchetFailure, + loadRatchetConfig, + maskSource, + measureFunctions, + ratchetViolations, + type RatchetConfig, +} from './helpers/module-size'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const readRepo = (file: string) => fs.readFileSync(path.join(ROOT, file), 'utf8'); + +/** A function of bodyLines + 3 lines. */ +const fn = (name: string, bodyLines: number) => + [`export function ${name}(value: number): number {`, ...Array.from({ length: bodyLines }, (_, i) => ` value += ${i};`), ' return value;', '}'].join('\n'); +const withFiles = (files: Record) => (file: string) => { + if (!(file in files)) throw new Error(`unexpected read: ${file}`); + return files[file]; +}; + +describe('ratchet (c): module and function size', () => { + test('refactor-wave modules and residual files hold their limits', () => { + const config = loadRatchetConfig(ROOT); + expect(config.newModules.length).toBeGreaterThan(0); + const violations = ratchetViolations(config, readRepo); + if (violations.length > 0) throw new Error(formatRatchetFailure(violations)); + }); + + test('counter self-test: resolver template literals with ${} and code-fence braces', () => { + const source = readRepo('test/fixtures/module-size/resolver-snippets.ts.txt'); + expect(measureFunctions(source).map(({ name, line, lines }) => ({ name, line, lines }))).toEqual([ + { name: 'generateDashboardLike', line: 6, lines: 17 }, + { name: 'generateDesignSketchLike', line: 24, lines: 11 }, + { name: 'arrowWithRegex', line: 36, lines: 2 }, + { name: 'handler', line: 40, lines: 4 }, + ]); + const masked = maskSource(source); + expect(masked.length).toBe(source.length); + expect(masked.split('\n').length).toBe(source.split('\n').length); + // Prose braces vanish; `${}` expression code stays measurable. + expect(masked).not.toContain('{row and suffix}'); + expect(masked).not.toContain('unbalanced'); + expect(masked).toContain('toShellPath(ctx.paths.binDir)'); + }); + + test('a planted 151-line function fails with file:line, the Fix and the allowlist path', () => { + const config: RatchetConfig = { newModules: [{ file: 'planted.ts', movedFrom: 'test' }], residualFiles: [], allowlist: [] }; + const planted = `// header\n${fn('small', 3)}\n\n${fn('tooLong', MAX_FUNCTION_LINES - 2)}\n`; + const violations = ratchetViolations(config, withFiles({ 'planted.ts': planted })); + expect(violations).toHaveLength(1); + expect(violations[0]).toStartWith('planted.ts:9 export function tooLong(value: number): number {'); + expect(violations[0]).toContain('151 lines > 150'); + const message = formatRatchetFailure(violations); + expect(message).toContain('planted.ts:9'); + expect(message).toContain('Fix: split the module or function'); + expect(message).toContain(`Allowlist: ${RATCHET_CONFIG}`); + expect(ratchetViolations(config, withFiles({ 'planted.ts': fn('atLimit', MAX_FUNCTION_LINES - 3) }))).toEqual([]); + }); + + test('oversized modules and grown residual files fail', () => { + const config: RatchetConfig = { + newModules: [{ file: 'big.ts', movedFrom: 'test' }], + residualFiles: [{ file: 'residual.ts', maxLines: 3, reason: 'test' }], + allowlist: [], + }; + const violations = ratchetViolations(config, withFiles({ + 'big.ts': `${'// line\n'.repeat(801)}`, + 'residual.ts': 'a\nb\nc\nd\n', + })); + expect(violations).toEqual([ + 'big.ts:801 module has 801 lines > 800', + 'residual.ts:4 residual file grew to 4 lines > recorded 3', + ]); + }); + + test('allowlist entries need a reason and are keyed on file plus matched text, not line numbers', () => { + const long = fn('generatedTable', MAX_FUNCTION_LINES + 10); + const entry = { file: 'table.ts', match: 'function generatedTable(' }; + const unreasoned: RatchetConfig = { newModules: [{ file: 'table.ts', movedFrom: 'test' }], residualFiles: [], allowlist: [entry] }; + const unreasonedViolations = ratchetViolations(unreasoned, withFiles({ 'table.ts': long })); + expect(unreasonedViolations.some(v => v.includes('allowlist entry without a reason'))).toBe(true); + expect(unreasonedViolations.some(v => v.includes('generatedTable'))).toBe(true); + + const allowed: RatchetConfig = { ...unreasoned, allowlist: [{ ...entry, reason: 'data table, not logic' }] }; + expect(ratchetViolations(allowed, withFiles({ 'table.ts': long }))).toEqual([]); + expect(ratchetViolations(allowed, withFiles({ 'table.ts': `// inserted above the match\n\n${long}` }))).toEqual([]); + }); + + test('every committed allowlist entry and residual record carries a reason', () => { + const config = loadRatchetConfig(ROOT); + for (const entry of [...config.allowlist, ...config.residualFiles]) expect(entry.reason?.trim()).toBeTruthy(); + }); +}); diff --git a/test/test-of-test-ratchet.test.ts b/test/test-of-test-ratchet.test.ts index 1f6f94152..4e1007ffe 100644 --- a/test/test-of-test-ratchet.test.ts +++ b/test/test-of-test-ratchet.test.ts @@ -158,6 +158,7 @@ const BASELINE = [ 'test/memory-cache-injection.test.ts', 'test/memory-ingest-include-gitignored.test.ts', 'test/migrations-v1.27.0.0.test.ts', + 'test/module-size-ratchet.test.ts', // ratchet (c) source scanner (W2): measures product modules as text; its counter is test tooling 'test/native-auto-decide-pty.test.ts', 'test/no-suicide-exit.test.ts', 'test/osv-config-wiring.test.ts', diff --git a/test/touchfiles.test.ts b/test/touchfiles.test.ts index b476fd45a..6cefc5c67 100644 --- a/test/touchfiles.test.ts +++ b/test/touchfiles.test.ts @@ -716,3 +716,61 @@ describe('derived touchfile closure', () => { expect(paid.filter(file => !keyed(file))).toEqual(Object.keys(KEYLESS_PAID).sort()); }); }); + +// Moved code keeps its paid-eval selection: touching a module extracted by a +// refactor must select a superset of what touching its source file selected +// before the move. Each golden in test/fixtures/touchfile-move-goldens/ maps +// new modules to their source files and records the sources' selections. +describe('touchfile coverage for moved code', () => { + const GOLDENS = 'test/fixtures/touchfile-move-goldens'; + type Selection = 'global' | string[]; + interface MoveGolden { moves: Record; selections: Record } + const MAPS = { e2e: E2E_TOUCHFILES, judges: LLM_JUDGE_TOUCHFILES } as const; + const selectionFor = (file: string, map: Record): Selection => { + const result = selectTests([file], map, GLOBAL_TOUCHFILES); + return result.reason.startsWith('global') ? 'global' : result.selected; + }; + /** One line per module/source/map whose recorded selection is no longer covered. */ + const moveSelectionGaps = (golden: MoveGolden, current = selectionFor): string[] => { + const gaps: string[] = []; + for (const [module, sources] of Object.entries(golden.moves)) { + for (const source of sources) { + const recorded = golden.selections[source]; + if (!recorded) { gaps.push(`${module} no recorded selection for source ${source}`); continue; } + for (const kind of ['e2e', 'judges'] as const) { + const now = current(module, MAPS[kind]); + if (now === 'global') continue; + const missing = recorded[kind] === 'global' ? ['(every test: global touchfile)'] + : (recorded[kind] as string[]).filter(name => name in MAPS[kind] && !now.includes(name)); + if (missing.length) gaps.push(`${module} ${kind} selection misses ${missing.join(', ')} (selected by ${source})`); + } + } + } + return gaps; + }; + const goldens = fs.readdirSync(path.join(ROOT, GOLDENS)).filter(name => name.endsWith('.json')).sort(); + + test.each(goldens)('%s: every new module selects a superset of its source files', name => { + const golden = JSON.parse(fs.readFileSync(path.join(ROOT, GOLDENS, name), 'utf8')) as MoveGolden; + const gaps = moveSelectionGaps(golden); + if (gaps.length) { + throw new Error([ + `Touchfile move coverage — ${gaps.length} gap(s):`, ...gaps.map(gap => ` ${gap}`), + 'Rule: code moved out of a file keeps that file\'s paid-eval selection, so an edit to the new module still runs the evals the old file ran.', + 'Fix: add the new module to every touchfile entry (E2E_TOUCHFILES, LLM_JUDGE_TOUCHFILES, GLOBAL_TOUCHFILES) that lists its source file in test/helpers/touchfiles-data.ts.', + `Golden: ${GOLDENS}/${name} — re-record a source's selection only for a deliberate, reviewed touchfile change.`, + ].join('\n')); + } + }); + + test('the move check fails when a new module drops its source file\'s selection', () => { + const golden: MoveGolden = { + moves: { 'scripts/new-module.ts': ['scripts/old.ts'] }, + selections: { 'scripts/old.ts': { e2e: 'global', judges: [] } }, + }; + expect(moveSelectionGaps(golden, () => [])).toEqual([ + 'scripts/new-module.ts e2e selection misses (every test: global touchfile) (selected by scripts/old.ts)', + ]); + expect(moveSelectionGaps(golden, () => 'global')).toEqual([]); + }); +}); From 48aeca9593ada097bc28467b73c502ddac1e3e80 Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 30 Sep 2026 06:07:24 +0000 Subject: [PATCH 6/6] test(shard-engine): give non-timeout corpus fixtures CI headroom; keep the POSIX golden off the Windows lane Only the wall-timeout fixture keeps a 3s wall; the rest get 60s so a loaded host cannot turn a pass into a timeout. Base and branch runners still agree on every classification under the new walls. The Windows exclusion entry moves the free runner's ratchet (c) residual cap to 2356 lines. --- scripts/test-free-shards.ts | 4 ++++ test/fixtures/module-size-ratchet.json | 2 +- test/fixtures/shard-equivalence/record.ts | 8 +++++--- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/scripts/test-free-shards.ts b/scripts/test-free-shards.ts index f37989b4e..83911f630 100755 --- a/scripts/test-free-shards.ts +++ b/scripts/test-free-shards.ts @@ -338,6 +338,10 @@ export const KNOWN_WINDOWS_INCOMPATIBLE: Array<{ file: string; reason: string }> file: 'test/cso-witness.test.ts', reason: 'tests the contained repair witness with POSIX private-directory and compiled-helper assumptions; comprehensive execution is unavailable on Windows', }, + { + file: 'test/shard-engine-equivalence.test.ts', + reason: 'its classification golden was recorded from the POSIX runners (process-group wall kill); the win32 engine path is pinned by the mocked-platform case in shard-engine.test.ts', + }, { file: 'test/cso-scanner-cli.test.ts', reason: 'drives the prebuilt POSIX CSO launcher with /usr/bin/git and a POSIX-only PATH; native Windows launcher behavior is covered by the dedicated cso-windows-launcher gate', diff --git a/test/fixtures/module-size-ratchet.json b/test/fixtures/module-size-ratchet.json index 16c0a6d4e..f99b1acdc 100644 --- a/test/fixtures/module-size-ratchet.json +++ b/test/fixtures/module-size-ratchet.json @@ -4,7 +4,7 @@ { "file": "test/helpers/module-size.ts", "movedFrom": "(new: ratchet (c) counter)" } ], "residualFiles": [ - { "file": "scripts/test-free-shards.ts", "maxLines": 2352, "reason": "free lane policy after the shard engine took spawn, sandbox, logs, seeds and flags" }, + { "file": "scripts/test-free-shards.ts", "maxLines": 2356, "reason": "free lane policy after the shard engine took spawn, sandbox, logs, seeds and flags" }, { "file": "scripts/test-paid-shards.ts", "maxLines": 1921, "reason": "paid lane policy after the shard engine took sandbox, logs, seeds, verdicts and flags" } ], "allowlist": [] diff --git a/test/fixtures/shard-equivalence/record.ts b/test/fixtures/shard-equivalence/record.ts index 77e1c8707..5738d1032 100644 --- a/test/fixtures/shard-equivalence/record.ts +++ b/test/fixtures/shard-equivalence/record.ts @@ -17,7 +17,9 @@ export const FIXTURES = [ 'pass', 'fail', 'skip', 'wall-timeout', 'module-load-error', 'zero-executed', 'unhandled-between-tests', ] as const; export const REAL_SHARD = 'test/strict-output.test.ts'; -const WALL_MS = 3_000; +// Only the wall-timeout fixture should ever reach its wall; the others get +// generous headroom so a loaded CI host cannot turn a pass into a timeout. +const wallFor = (name: string) => (name === 'wall-timeout' ? 3_000 : 60_000); const root = path.resolve(process.argv[2] ?? path.join(import.meta.dir, '../../..')); const fixtureDir = import.meta.dir; @@ -28,11 +30,11 @@ const freeExit = (status: string) => (status === 'passed' ? 0 : status === 'time try { const runFree = (file: string, index: number) => free.runFreeShard([file], index + 1, FIXTURES.length, { - rootDir: root, quiet: true, log: () => {}, wallTimeoutMs: WALL_MS, + rootDir: root, quiet: true, log: () => {}, wallTimeoutMs: wallFor(FIXTURES[index]), logFilePath: path.join(scratch, `free-${index}.log`), }); const runPaid = (file: string, index: number) => paid.runPaidShard([file], index + 1, FIXTURES.length, { - rootDir: root, timeoutMs: WALL_MS, jobs: 2, logDir: scratch, log: () => {}, + rootDir: root, timeoutMs: wallFor(FIXTURES[index]), jobs: 2, logDir: scratch, log: () => {}, env: { ...process.env, GSTACK_CLAUDE_CLI_VERSION: 'fixture', EVALS: '', EVALS_TIER: '', EVALS_ALL: '' }, }); const files = FIXTURES.map(name => path.join(fixtureDir, `${name}.fixture.ts`));