fix(fuzz): run parser cases in a worker process, not the runner's thread (#2053) (#2055)

The unit-lane corpus replay executed adversarial parser cases on worker
threads of the Vitest worker running the test file. A fault in a worker
thread ends its whole process, so a case that faulted killed the test
runner: `[vitest-pool]: Worker forks emitted error / Worker exited
unexpectedly`, with no test, file, or case named. Six of six Coverage
deaths before #1994's split were this one file out of ~1100, and the
uninstrumented second leg it created then lost the same file six more
times in three days.

Cases now run in a worker *process*. The two faults a case cannot report
about itself are both classified from outside it: a case that never
returns is a `hang` (unchanged), and one that ends the process it runs in
is a new `crash` failure carrying the exit code or signal and the tail of
the worker's stderr — the death certificate the lane used to lose. A
sixth self-check target seeds that kind, so a regression in reporting it
fails the harness self-check like every other kind.

Author: thymikee

.github/workflows/replays-nightly.yml

@@ -38,7 +38,10 @@ jobs:
   # 38,000 cases/target holds the job's wall-clock at the pre-B2 five-target/50k budget now that
   # seven targets share it: measured on a quiet host, 7x38k = 16.5s against 5x50k = 16.6-16.9s.
   # Chosen by measurement rather than rounding — 33k would have been 2s cheaper but cost the
-  # five untouched targets a third of their depth for no wall-clock reason.
+  # five untouched targets a third of their depth for no wall-clock reason. The count is
+  # unchanged by #2053; moving case execution out of process cost that run about 3s (17.8s ->
+  # 20.9s, best of three on one host), which buys a case that faults its process a named
+  # `crash` failure instead of a dead caller.
   nightly-parser-fuzz:
     name: Parser Fuzz Lane
     runs-on: ubuntu-latest

docs/agents/testing.md

@@ -115,8 +115,11 @@ another example. Keep examples for a real past bug or a named decision. Reuse `P
 budgets so property files stay inside the unit slow-test gate.
 
 Parser fuzz targets live in `scripts/fuzz/targets.ts`. Validation generators carry the invalid
-outcome they planted, so silent acceptance and wrong error codes are failures. Promote a discovered
-case with the command the harness prints — never hand-copy an unshrunk input.
+outcome they planted, so silent acceptance and wrong error codes are failures. Cases run in a
+worker process, so the two faults a case cannot report about itself — never returning, and killing
+the process it runs in — are reported as `hang` and `crash` against the exact input rather than
+taking the caller down with them. Promote a discovered case with the command the harness prints —
+never hand-copy an unshrunk input.
 
 Mutation is report-only and limited to the registry in `scripts/mutation/modules.ts`. It measures
 whether tests distinguish changed decision logic. Do not infer redundancy from line coverage alone.

scripts/fuzz/corpus-replay.test.ts

@@ -2,8 +2,10 @@
 //
 // The nightly lane finds cases; this replays every case it ever found so a regression fails in
 // seconds on a PR instead of a night later. Cases go through the same worker-backed watchdog the
-// nightly lane uses: a promoted hang case must fail this test against its per-case budget, not
-// wedge the unit job until the CI timeout.
+// nightly lane uses, and for the same reason they run in a worker *process* (#2053): a promoted
+// hang case must fail this test against its per-case budget rather than wedge the unit job, and
+// a case that faults the process it runs in must fail this test rather than kill the Vitest
+// worker running it — which reports no test, no file, and no case.
 
 import fc from 'fast-check';
 import { describe, expect, it } from 'vitest';

scripts/fuzz/execute.ts

@@ -1,129 +1,207 @@
-// Case execution with hang detection for the parser fuzz lane (#1414).
+// Case execution with hang and crash detection for the parser fuzz lane (#1414).
 //
-// Cases run one at a time in a worker thread: a synchronous parser that never returns cannot be
-// timed out from inside its own tick, so the budget is enforced from *another* thread, which can
-// terminate the wedged one and attribute the stall to the exact input. Cases go over the wire
-// individually (rather than as one batch) because fast-check drives the loop — it decides the next
-// input, including the shrink candidates it derives from a failing one.
+// Cases run one at a time in a child process, and the budget is enforced from the parent. Two
+// faults a case can produce cannot be observed from inside the context running it: a synchronous
+// parser that never returns cannot be timed out from its own tick, and a parser that faults the
+// process — a native crash, an out-of-memory abort — leaves no stack to catch. Both are
+// attributed here, to the exact input, by watching a process the parent can outlive.
+//
+// It is a process rather than a worker thread because a thread cannot contain the second fault:
+// a fault in a worker thread takes its whole process down, and that process is the caller's —
+// for the unit-lane replay, the Vitest worker running the test file, which then dies with no
+// test, file, or case named (#2053). Cases go over the wire individually (rather than as one
+// batch) because fast-check drives the loop — it decides the next input, including the shrink
+// candidates it derives from a failing one.
 
+import { fork, type ChildProcess } from 'node:child_process';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
-import { Worker } from 'node:worker_threads';
 import type { FuzzFailure } from './invariant.ts';
 import type { FuzzTarget } from './target-types.ts';
-import type { FuzzWorkerData, FuzzWorkerMessage } from './worker.ts';
+import type { FuzzWorkerMessage } from './worker.ts';
 
 const WORKER_PATH = path.join(path.dirname(fileURLToPath(import.meta.url)), 'worker.ts');
 
 /** Bound on worker startup only; a per-case budget must never be charged for it. */
 const STARTUP_BUDGET_MS = 60_000;
 
+/** How much of a dead worker's output a crash failure quotes. Enough for a V8 fatal banner. */
+const DEATH_OUTPUT_LIMIT = 2_000;
+
 export type CaseRunner = {
   /** The failure this input violates the invariant with, or `null` when it holds. */
   run: (input: string) => Promise<FuzzFailure | null>;
   close: () => Promise<void>;
 };
 
-type Session = { worker: Worker; ready: Promise<void> };
+/** A started worker: it has answered the handshake, so a budget may be charged to it. */
+type Session = {
+  child: ChildProcess;
+  /**
+   * How the process died, or `null` while it is alive — the death certificate a crash failure
+   * quotes. A reader rather than a field because the session's own listeners write it.
+   */
+  death: () => string | null;
+};
+
+/**
+ * Everything a worker can do next, and all either wait below has to classify: answer, die, or
+ * stay silent for the whole budget. The handshake and a case are the same wait against different
+ * budgets, so they share one primitive and differ only in what they make of its three outcomes.
+ */
+type WorkerOutcome =
+  | { kind: 'message'; message: FuzzWorkerMessage }
+  | { kind: 'death' }
+  | { kind: 'silence' };
+
+function awaitWorker(child: ChildProcess, budgetMs: number): Promise<WorkerOutcome> {
+  return new Promise((resolve) => {
+    const finish = (outcome: WorkerOutcome) => {
+      clearTimeout(timer);
+      child.off('message', onMessage);
+      child.off('close', onDeath);
+      child.off('error', onDeath);
+      resolve(outcome);
+    };
+    const onMessage = (message: FuzzWorkerMessage) => finish({ kind: 'message', message });
+    const onDeath = () => finish({ kind: 'death' });
+    const timer = setTimeout(() => finish({ kind: 'silence' }), budgetMs);
+    child.once('message', onMessage);
+    child.once('close', onDeath);
+    child.once('error', onDeath);
+  });
+}
+
+function describeDeath(code: number | null, signal: NodeJS.Signals | null, output: string): string {
+  const how = signal === null ? `exit code ${code}` : `signal ${signal}`;
+  const tail = output.trim();
+  return tail === '' ? how : `${how}: ${tail}`;
+}
+
+/** Why a started worker is unusable, or `null` when it handed back the handshake. */
+function startupRefusal(outcome: WorkerOutcome, death: string | null): string | null {
+  if (outcome.kind === 'silence') return `fuzz worker did not start within ${STARTUP_BUDGET_MS}ms`;
+  if (outcome.kind === 'death') return `fuzz worker died before it was ready (${death})`;
+  if (outcome.message.kind !== 'ready') {
+    return `unexpected first worker message: ${outcome.message.kind}`;
+  }
+  return null;
+}
 
-/** A worker plus the promise that settles when it has finished importing the parsers. */
-function startSession(targetName: string): Session {
-  const workerData: FuzzWorkerData = { targetName };
-  // Type stripping is requested explicitly rather than inherited: under Vitest the parent's
-  // execArgv carries no such flag, and the worker is a plain `.ts` file Node must strip itself.
-  // The warning is silenced because a restarted worker re-emits it — one per hang buries the report.
-  const worker = new Worker(WORKER_PATH, {
-    workerData,
+/** Starts a worker and resolves once it has finished importing the parsers. */
+async function startSession(targetName: string): Promise<Session> {
+  const child = fork(WORKER_PATH, [targetName], {
+    // Type stripping is requested explicitly rather than inherited: under Vitest the parent's
+    // execArgv carries no such flag, and the worker is a plain `.ts` file Node must strip itself.
+    // The warning is silenced because a restarted worker re-emits it — one per hang buries the report.
     execArgv: ['--experimental-strip-types', '--disable-warning=ExperimentalWarning'],
+    // stderr is piped rather than inherited so a fatal fault — which writes to the descriptor and
+    // so escapes the worker's own silencing — is quoted in the failure instead of the lane's log.
+    stdio: ['ignore', 'inherit', 'pipe', 'ipc'],
   });
-  const ready = new Promise<void>((resolve, reject) => {
-    const timer = setTimeout(
-      () => reject(new Error(`fuzz worker did not start within ${STARTUP_BUDGET_MS}ms`)),
-      STARTUP_BUDGET_MS,
-    );
-    worker.once('message', (message: FuzzWorkerMessage) => {
-      clearTimeout(timer);
-      if (message.kind === 'ready') resolve();
-      else reject(new Error(`unexpected first worker message: ${message.kind}`));
-    });
-    worker.once('error', (error) => {
-      clearTimeout(timer);
-      reject(error);
-    });
+  let death: string | null = null;
+  let output = '';
+  child.stderr?.setEncoding('utf8');
+  child.stderr?.on('data', (chunk: string) => {
+    output = (output + chunk).slice(-DEATH_OUTPUT_LIMIT);
+  });
+  // A ChildProcess with no `error` listener raises the error at the process, which is the fault
+  // this module exists to keep off the caller. The message is evidence rather than the
+  // certificate — `close` is the certificate whenever the process ran at all, and quotes this
+  // tail — so the fallback below only stands for a worker that could not be spawned, the one
+  // case where `close` never comes.
+  child.on('error', (error) => {
+    output = `${output}${error.message}\n`.slice(-DEATH_OUTPUT_LIMIT);
+    death ??= `spawn failed: ${error.message}`;
   });
-  return { worker, ready };
+  // `close` rather than `exit`: it fires once the piped stderr has drained too, so the death is
+  // recorded with the output that explains it.
+  child.once('close', (code: number | null, signal: NodeJS.Signals | null) => {
+    death = describeDeath(code, signal, output);
+  });
+  const session: Session = { child, death: () => death };
+  const outcome = await awaitWorker(child, STARTUP_BUDGET_MS);
+  const refusal = startupRefusal(outcome, session.death());
+  if (refusal === null) return session;
+  // Every refusing path ends the worker, including the silent one — a worker still importing
+  // holds the channel open, and nothing else would ever come back for it.
+  await endSession(session);
+  throw new Error(refusal);
+}
+
+/** Ends a worker process, and resolves once it is gone. Already-dead sessions are a no-op. */
+async function endSession(session: Session): Promise<void> {
+  if (session.death() !== null) return;
+  const gone = new Promise<void>((resolve) => session.child.once('close', () => resolve()));
+  // SIGKILL, not SIGTERM: a session is usually ended because a case wedged the worker, and a
+  // process spinning inside a synchronous parser is exactly the one a catchable signal cannot reach.
+  session.child.kill('SIGKILL');
+  await gone;
 }
 
+/** A case's verdict, plus whether the worker can take another one. */
+type CaseOutcome = { failure: FuzzFailure | null; usable: boolean };
+
 /**
  * Runs one case on a started worker. Startup is already awaited by the caller, so the budget below
  * covers parser time only — the reason a slow import can never be misreported as a parser hang.
+ *
+ * A worker that hung is wedged in this case and one that died is gone, so neither is `usable`.
  */
-function runOnSession(
+async function runCase(
   session: Session,
   target: FuzzTarget,
   input: string,
   caseTimeoutMs: number,
-): Promise<{ failure: FuzzFailure | null; hung: boolean }> {
-  return new Promise((resolve, reject) => {
-    const settle = (failure: FuzzFailure | null, hung: boolean) => {
-      clearTimeout(timer);
-      session.worker.off('message', onMessage);
-      session.worker.off('error', onError);
-      resolve({ failure, hung });
-    };
-    const onMessage = (message: FuzzWorkerMessage) => {
-      if (message.kind === 'result') settle(message.failure, false);
-    };
-    const onError = (error: Error) => {
-      clearTimeout(timer);
-      session.worker.off('message', onMessage);
-      reject(error);
-    };
-    const timer = setTimeout(() => {
-      settle(
-        {
-          target: target.name,
-          input,
-          kind: 'hang',
-          detail: `case did not finish within ${caseTimeoutMs}ms`,
-        },
-        true,
-      );
-    }, caseTimeoutMs);
-    session.worker.on('message', onMessage);
-    session.worker.once('error', onError);
-    session.worker.postMessage({ kind: 'case', input });
+): Promise<CaseOutcome> {
+  const failed = (kind: 'hang' | 'crash', detail: string): CaseOutcome => ({
+    failure: { target: target.name, input, kind, detail },
+    usable: false,
   });
+  const crashed = () => failed('crash', `case killed the worker process (${session.death()})`);
+  // A worker that died between cases is reported against the case that meets it, rather than
+  // through the ERR_IPC_CHANNEL_CLOSED that sending to it would raise: the harness classifies
+  // every death of a worker it was using, and an opaque transport error is not a classification.
+  if (session.death() !== null) return crashed();
+  // Armed before the case is sent, so an answer cannot arrive unobserved.
+  const answer = awaitWorker(session.child, caseTimeoutMs);
+  session.child.send({ kind: 'case', input });
+  const outcome = await answer;
+  if (outcome.kind === 'death') return crashed();
+  if (outcome.kind === 'silence') {
+    return failed('hang', `case did not finish within ${caseTimeoutMs}ms`);
+  }
+  if (outcome.message.kind !== 'result') {
+    throw new Error(`unexpected worker message: ${outcome.message.kind}`);
+  }
+  return { failure: outcome.message.failure, usable: true };
 }
 
 /**
- * A worker-backed runner for one target. A hang leaves the thread wedged in its own loop, so the
- * runner terminates it and starts a fresh one for the next case — otherwise a single hang would
- * silently turn every later case into a hang too.
+ * A worker-backed runner for one target. A hang leaves the worker wedged in its own loop and a
+ * crash leaves no worker at all, so the runner ends the session and starts a fresh one for the
+ * next case — otherwise a single such case would silently turn every later case into one too.
  */
 export async function createCaseRunner(
   target: FuzzTarget,
   caseTimeoutMs: number,
 ): Promise<CaseRunner> {
-  let session = startSession(target.name);
-  await session.ready;
+  let session = await startSession(target.name);
   let closed = false;
 
   return {
     run: async (input) => {
       if (closed) throw new Error('fuzz case runner is closed');
-      await session.ready;
-      const { failure, hung } = await runOnSession(session, target, input, caseTimeoutMs);
-      if (hung) {
-        await session.worker.terminate();
-        session = startSession(target.name);
-        await session.ready;
+      const { failure, usable } = await runCase(session, target, input, caseTimeoutMs);
+      if (!usable) {
+        await endSession(session);
+        session = await startSession(target.name);
       }
       return failure;
     },
     close: async () => {
       closed = true;
-      await session.worker.terminate();
+      await endSession(session);
     },
   };
 }

scripts/fuzz/harness.test.ts

@@ -14,6 +14,7 @@ import { fileURLToPath } from 'node:url';
 import { describe, expect, it } from 'vitest';
 import { LANE_ENVELOPE_SCHEMA_VERSION } from '../lib/lane-envelope.ts';
 import { CASE_GENERATION_INPUTS } from './envelope.ts';
+import { runCases } from './execute.ts';
 import { checkCase } from './invariant.ts';
 import { SELF_CHECK_TARGETS } from './self-check-targets.ts';
 
@@ -69,7 +70,7 @@ describe('fuzz invariant classifier', () => {
 });
 
 describe('fuzz harness self-check', () => {
-  // One run asserts both the report and its envelope: a second full self-check would cost five
+  // One run asserts both the report and its envelope: a second full self-check would cost six
   // more real worker startups (#1823) for no new signal.
   it('catches every seeded violation kind and writes the self-check envelope', () => {
     const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'fuzz-selfcheck-'));
@@ -83,17 +84,36 @@ describe('fuzz harness self-check', () => {
     expect(stdout).toContain('ok   self-check-untyped-throw: expected untyped-throw');
     expect(stdout).toContain('ok   self-check-empty-hint: expected empty-hint');
     expect(stdout).toContain('ok   self-check-hang: expected hang, got hang');
+    expect(stdout).toContain('ok   self-check-crash: expected crash, got crash');
     expect(stdout).toContain('ok   self-check-silent-accept: expected silent-accept');
     expect(stdout).toContain('ok   self-check-wrong-code: expected wrong-code');
     expect(status).toBe(0);
     const envelope = JSON.parse(fs.readFileSync(path.join(dir, 'run-envelope.json'), 'utf8'));
     expect(envelope.result).toBe('pass');
     expect(envelope.data.mode).toBe('self-check');
-    expect(envelope.data.targetRuns).toHaveLength(5);
+    expect(envelope.data.targetRuns).toHaveLength(6);
     fs.rmSync(dir, { recursive: true, force: true });
   }, 30_000);
 });
 
+describe('case crash containment', () => {
+  // #2053: a case that faults the process it runs in must take only the worker with it. This
+  // test runs the runner in-process on purpose — it *is* the caller the lane must protect, so a
+  // runner that executed cases on this thread would kill this Vitest worker instead of failing,
+  // and the file would vanish from the run with nothing attributed.
+  it('attributes a worker death to the case and survives it', async () => {
+    const failures = await runCases(targetNamed('self-check-crash'), ['case'], 5_000);
+    expect(failures.map((failure) => failure.kind)).toEqual(['crash']);
+    // The death certificate the lane used to lose: how the worker ended, quoted with the case.
+    expect(failures[0]?.detail).toContain('exit code 97');
+  }, 30_000);
+
+  it('keeps running cases after one kills the worker', async () => {
+    const failures = await runCases(targetNamed('self-check-crash'), ['first', 'second'], 5_000);
+    expect(failures.map((failure) => failure.input)).toEqual(['first', 'second']);
+  }, 30_000);
+});
+
 describe('worker startup budget', () => {
   // The watchdog used to start before the worker reported ready, so thread spawn plus type
   // stripping plus parser imports — hundreds of milliseconds — was charged to the first case and