diff --git a/scripts/test-free-shards.ts b/scripts/test-free-shards.ts index 8c9986bfe..1cec43311 100755 --- a/scripts/test-free-shards.ts +++ b/scripts/test-free-shards.ts @@ -77,13 +77,14 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; -import { spawn } from 'child_process'; +import { spawn, spawnSync } from 'child_process'; import { StringDecoder } from 'node:string_decoder'; import { isPaidTestFile } from '../test/helpers/paid-test-set'; import { BunTestOutputClassifier, exactTestFileSelectors, installChildSignalForwarding, + isTerminationRequested, killProcessGroup, strictTestExitCode, stripAnsiLine, @@ -1081,16 +1082,38 @@ async function main(): Promise { })), ); let worst = Math.max(...outcomes.map((o) => exitCodeFor(o.status))); - if (mutators.length > 0) { + // Cancellation stops the run: don't launch the serial tree-mutating shard + // after a SIGINT/SIGTERM already killed the parallel phase. + if (mutators.length > 0 && !isTerminationRequested()) { const mutatorOutcome = await runFreeShard(mutators, totalShards, totalShards, { wallTimeoutMs: shardTimeout(mutators.length), verbose: options.verbose, }); worst = Math.max(worst, exitCodeFor(mutatorOutcome.status)); + if (mutatorOutcome.status !== 'passed') { + // Mutator safety rests on each test restoring default state itself; a + // SIGKILL at the wall deadline (or a mid-regeneration crash) defeats + // that by construction. Say so, loudly, before someone commits + // regenerated SKILL.md / .agents artifacts by accident. + const dirty = spawnSyncGitStatusGenerated(); + if (dirty.length > 0) { + console.error('[test:free] ⚠ tree-mutating shard did not finish cleanly — generated artifacts may be mid-regeneration:'); + for (const line of dirty.slice(0, 20)) console.error(`[test:free] ${line}`); + console.error('[test:free] restore with: bun run gen:skill-docs (or git checkout -- )'); + } + } } return worst; } +/** Dirty generated artifacts (SKILL.md / host outputs) after a failed mutator shard. */ +function spawnSyncGitStatusGenerated(): string[] { + const result = spawnSync('git', ['status', '--porcelain'], { cwd: ROOT, encoding: 'utf8' }); + if (result.status !== 0 || !result.stdout) return []; + return result.stdout.split('\n').filter((line) => + /SKILL\.md$/.test(line) || line.includes('.agents/') || line.includes('.factory/')); +} + if (import.meta.main) { try { process.exitCode = await main(); diff --git a/scripts/test-paid-shards.ts b/scripts/test-paid-shards.ts index 514d3d8d2..c2f3e7e3d 100644 --- a/scripts/test-paid-shards.ts +++ b/scripts/test-paid-shards.ts @@ -59,6 +59,7 @@ import { exactTestFileSelectors, forwardAndClassify, installChildSignalForwarding, + isTerminationRequested, killProcessGroup, strictTestExitCode, } from './test-strict-output'; @@ -517,6 +518,10 @@ export async function runPaidShards( let next = 0; const worker = async (): Promise => { while (true) { + // Cancellation (SIGINT/SIGTERM) must stop the RUN: the signal + // forwarders kill in-flight children, and this guard stops the pool + // from launching replacement shards that would keep burning API spend. + if (isTerminationRequested()) return; const index = next; next += 1; if (index >= shards.length) return; diff --git a/scripts/test-strict-output.ts b/scripts/test-strict-output.ts index 5fdfa5af4..4da71fd19 100644 --- a/scripts/test-strict-output.ts +++ b/scripts/test-strict-output.ts @@ -51,25 +51,69 @@ const DEFAULT_TERMINATION_TIMER: TerminationTimerApi = { 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; @@ -78,6 +122,8 @@ export function installChildSignalForwarding( 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'); diff --git a/test/strict-output.test.ts b/test/strict-output.test.ts index 149171a3e..832283081 100644 --- a/test/strict-output.test.ts +++ b/test/strict-output.test.ts @@ -8,7 +8,14 @@ */ import { describe, expect, it } from 'bun:test'; -import { BunTestOutputClassifier, strictTestExitCode } from '../scripts/test-strict-output'; +import { + BunTestOutputClassifier, + installChildSignalForwarding, + isTerminationRequested, + strictTestExitCode, + type TerminationSignalSource, + type TerminationTimerApi, +} from '../scripts/test-strict-output'; describe('strictTestExitCode', () => { it('trusts a clean zero exit when the expected file count ran', () => { @@ -83,3 +90,74 @@ describe('BunTestOutputClassifier', () => { expect(strictTestExitCode(0, summary, 2)).toBe(0); }); }); + +describe('installChildSignalForwarding — cancellation terminates the RUN', () => { + // Installing any SIGINT/SIGTERM listener suppresses Node's default + // terminate-on-signal. Pre-fix, the forwarder killed the current child and + // the parent LIVED ON — the paid worker pool kept launching API-burning + // shards after Ctrl-C. The parent must schedule its own exit and expose + // isTerminationRequested() so launch loops stop taking new work. + type Handler = () => void; + const makeFakes = () => { + const listeners = new Map(); + const source: TerminationSignalSource = { + on: (event, listener) => { + listeners.set(event, [...(listeners.get(event) ?? []), listener]); + }, + off: (event, listener) => { + listeners.set(event, (listeners.get(event) ?? []).filter((l) => l !== listener)); + }, + }; + const emit = (event: string) => (listeners.get(event) ?? []).forEach((l) => l()); + const scheduled: Array<{ callback: () => void; delayMs: number; cancelled: boolean }> = []; + const timer: TerminationTimerApi = { + schedule: (callback, delayMs) => { + const handle = { callback, delayMs, cancelled: false }; + scheduled.push(handle); + return handle; + }, + cancel: (handle) => { + (handle as { cancelled: boolean }).cancelled = true; + }, + }; + const kills: string[] = []; + const child = { kill: (sig?: unknown) => { kills.push(String(sig)); return true; } }; + const exits: number[] = []; + return { source, emit, timer, scheduled, kills, child, exits, exit: (code: number) => { exits.push(code); } }; + }; + + it('first signal kills the child, marks termination, and schedules parent exit after the grace', () => { + const f = makeFakes(); + installChildSignalForwarding(f.child, f.source, f.timer, 5_000, f.exit); + expect(isTerminationRequested(f.source)).toBe(false); + f.emit('SIGTERM'); + expect(f.kills).toEqual(['SIGTERM']); + expect(isTerminationRequested(f.source)).toBe(true); + // Two timers: child SIGKILL grace (5s) and parent exit (grace + 1s). + const delays = f.scheduled.map((s) => s.delayMs); + expect(delays).toContain(5_000); + expect(delays).toContain(6_000); + const parentExit = f.scheduled.find((s) => s.delayMs === 6_000)!; + parentExit.callback(); + expect(f.exits).toEqual([143]); + }); + + it('parent exit fires even when the shard disposes cleanly first', () => { + const f = makeFakes(); + const forwarding = installChildSignalForwarding(f.child, f.source, f.timer, 5_000, f.exit); + f.emit('SIGINT'); + forwarding.dispose(); + const parentExit = f.scheduled.find((s) => s.delayMs === 6_000)!; + expect(parentExit.cancelled).toBe(false); + parentExit.callback(); + expect(f.exits).toEqual([130]); + }); + + it('one parent exit across many concurrent forwarders on the same source', () => { + const f = makeFakes(); + installChildSignalForwarding(f.child, f.source, f.timer, 5_000, f.exit); + installChildSignalForwarding({ kill: () => true }, f.source, f.timer, 5_000, f.exit); + f.emit('SIGTERM'); + expect(f.scheduled.filter((s) => s.delayMs === 6_000).length).toBe(1); + }); +});