From fb30855ce391a398774bd5885f0821fdf3d8da6b Mon Sep 17 00:00:00 2001
From: garrytan <garrytan@gmail.com>
Date: Thu, 29 Oct 2026 16:22:44 +0000
Subject: [PATCH] fix(flaky): stream output, keep unreported runner failures
 red, fail-closed duplicates and nonzero reruns

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
---
 bin/gstack-flaky       |  3 +-
 lib/flaky-run.ts       | 73 +++++++++++++++++++++++++++++++++---------
 test/flaky-run.test.ts | 70 ++++++++++++++++++++++++----------------
 3 files changed, 103 insertions(+), 43 deletions(-)

diff --git a/bin/gstack-flaky b/bin/gstack-flaky
index 7bec3b7ba..60039be8c 100755
--- a/bin/gstack-flaky
+++ b/bin/gstack-flaky
@@ -41,6 +41,7 @@ const FIX: Record<string, string> = {
   FLAKY_RETRIES_HIDDEN: 'set PLAYWRIGHT_JUNIT_INCLUDE_RETRIES=1 where the report is written',
   FLAKY_COMMAND_FAILED: 'read the command output above: the suite crashed or a non-test step failed; that is a real red',
   FLAKY_COMMAND_NOT_STARTED: 'check the command after -- (it runs without a shell; wrap it as bash -c "<script>" when it needs one)',
+  FLAKY_UNREPORTED_FAILURES: 'the runner counted failures its JUnit omits (a test file that failed to load, an error outside any test); fix that first, it is a real red',
   FLAKY_RERUN_UNSUPPORTED: 'pass --runner playwright|bun, or --rerun-arg templates that select one test (for example --rerun-arg {file} --rerun-arg -k --rerun-arg {title})',
 };
 
@@ -94,7 +95,7 @@ if (!o.junit) die('run needs --junit <file>');
 if (!o.command.length) die('run needs the test command after --');
 let result;
 try {
-  result = runFlaky({ command: o.command, junit: o.junit, runner: o.runner, maxReruns: o.maxReruns, rerunArgs: o.rerunArgs });
+  result = await runFlaky({ command: o.command, junit: o.junit, runner: o.runner, maxReruns: o.maxReruns, rerunArgs: o.rerunArgs });
 } catch (e) { if (e instanceof JunitError) coded(e); throw e; }
 const ledger = ledgerPath(o.ledger);
 appendLedger(result.ledger, ledger, s => process.stderr.write(`gstack-flaky: ${s}\n`));
diff --git a/lib/flaky-run.ts b/lib/flaky-run.ts
index 33118a199..e0982523c 100644
--- a/lib/flaky-run.ts
+++ b/lib/flaky-run.ts
@@ -16,11 +16,13 @@
  *   a pass only when its command also exited 0.
  * - The JUnit file is removed before every attempt; a report that attempt did
  *   not write is missing. Missing, unreadable or empty JUnit is never green, and
- *   a non-zero exit with no failed test is a crash, never green.
+ *   a non-zero exit with no failed test is a crash, never green. When the
+ *   runner's own summary counts failures or errors the JUnit omits (bun files
+ *   that fail to load), the run is red and nothing is rerun.
  */
 import * as fs from 'fs';
 import * as path from 'path';
-import { spawnSync } from 'child_process';
+import { spawn as spawnChild, spawnSync } from 'child_process';
 import { finalAttempt, JunitError, readJunit, type JunitReport, type JunitRunner, type JunitTest } from './junit';
 import { redactFindingSpans } from './redact-engine';
 import { resolveStateRoot } from './state-root';
@@ -52,7 +54,8 @@ export interface RunOptions {
   rerunArgs?: string[];
   cwd?: string;
   env?: NodeJS.ProcessEnv;
-  spawn?: (argv: string[], env: NodeJS.ProcessEnv, cwd: string) => number;
+  /** Runs one attempt; resolves with its exit status and the tail of its combined output. */
+  spawn?: (argv: string[], env: NodeJS.ProcessEnv, cwd: string) => Promise<Attempted>;
   now?: () => Date;
 }
 
@@ -63,6 +66,8 @@ export interface TestOutcome {
   reason?: string;
 }
 
+export interface Attempted { exit: number; output: string }
+
 export interface RunResult {
   exitCode: 0 | 1 | 3;
   code?: string;
@@ -91,10 +96,42 @@ export function rerunSelection(runner: JunitRunner, t: JunitTest, generic: strin
   return generic.map(a => a.replaceAll('{file}', t.file).replaceAll('{title}', title).replaceAll('{pattern}', pattern));
 }
 
-function defaultSpawn(argv: string[], env: NodeJS.ProcessEnv, cwd: string): number {
-  const r = spawnSync(argv[0]!, argv.slice(1), { stdio: 'inherit', env, cwd });
-  if (r.error) throw new JunitError('FLAKY_COMMAND_NOT_STARTED', `could not start ${argv[0]}: ${r.error.message}`);
-  return r.status ?? (r.signal ? 128 : 1);
+const OUTPUT_TAIL = 256 * 1024;
+
+/** Stream the child's output live (a quiet runner trips CI idle watchdogs) and keep its tail for reconciliation. */
+function defaultSpawn(argv: string[], env: NodeJS.ProcessEnv, cwd: string): Promise<Attempted> {
+  return new Promise((resolve, reject) => {
+    let tail = '';
+    const keep = (chunk: Buffer) => { tail = (tail + chunk.toString('utf-8')).slice(-OUTPUT_TAIL); };
+    const child = spawnChild(argv[0]!, argv.slice(1), { stdio: ['inherit', 'pipe', 'pipe'], env, cwd });
+    child.stdout!.on('data', (c: Buffer) => { process.stdout.write(c); keep(c); });
+    child.stderr!.on('data', (c: Buffer) => { process.stderr.write(c); keep(c); });
+    child.on('error', e => reject(new JunitError('FLAKY_COMMAND_NOT_STARTED', `could not start ${argv[0]}: ${e.message}`)));
+    child.on('close', (code, signal) => resolve({ exit: code ?? (signal ? 128 : 1), output: tail }));
+  });
+}
+
+/**
+ * Failures a runner counted that its JUnit omits: bun leaves files that fail to
+ * load out of the report ("N errors"), Playwright reports errors outside any
+ * test. A rerun cannot clear those, so they keep the run red.
+ */
+export function unreportedFailures(runner: JunitRunner, output: string, reportedFailed: number): string | null {
+  const text = output.replace(/\x1b\[[0-9;]*m/g, '');
+  const last = (re: RegExp) => { const m = [...text.matchAll(re)].pop(); return m ? Number(m[1]) : 0; };
+  if (runner === 'bun') {
+    const errors = last(/^\s*(\d+) errors?\s*$/gm);
+    const fails = last(/^\s*(\d+) fail\s*$/gm);
+    if (errors > 0) return `bun reported ${errors} error(s) outside the JUnit report (a file failed to load or threw between tests)`;
+    if (fails > reportedFailed) return `bun counted ${fails} failing tests, the JUnit report ${reportedFailed}`;
+  }
+  if (runner === 'playwright') {
+    const outside = last(/(\d+) errors? (?:was|were) not a part of any test/g);
+    const fails = last(/^\s*(\d+) failed\s*$/gm);
+    if (outside > 0) return `Playwright reported ${outside} error(s) outside any test`;
+    if (fails > reportedFailed) return `Playwright counted ${fails} failed tests, the JUnit report ${reportedFailed}`;
+  }
+  return null;
 }
 
 function git(args: string[], cwd: string): string | undefined {
@@ -122,23 +159,23 @@ export function safeExcerpt(lines: string[]): string[] {
 }
 
 /** One attempt: remove the report, run, and read a report this attempt wrote. */
-function attempt(opts: RunOptions, argv: string[], cwd: string, env: NodeJS.ProcessEnv): { exit: number; report?: JunitReport; error?: JunitError } {
+async function attempt(opts: RunOptions, argv: string[], cwd: string, env: NodeJS.ProcessEnv): Promise<{ exit: number; output: string; report?: JunitReport; error?: JunitError }> {
   const junit = path.resolve(cwd, opts.junit);
   fs.rmSync(junit, { force: true });
-  const exit = (opts.spawn ?? defaultSpawn)(argv, env, cwd);
-  if (!fs.existsSync(junit)) return { exit, error: new JunitError('FLAKY_NO_RESULTS', `the command wrote no JUnit report at ${opts.junit} (exit ${exit})`) };
-  try { return { exit, report: readJunit(junit, opts.runner) }; } catch (e) {
-    if (e instanceof JunitError) return { exit, error: e };
+  const { exit, output } = await (opts.spawn ?? defaultSpawn)(argv, env, cwd);
+  if (!fs.existsSync(junit)) return { exit, output, error: new JunitError('FLAKY_NO_RESULTS', `the command wrote no JUnit report at ${opts.junit} (exit ${exit})`) };
+  try { return { exit, output, report: readJunit(junit, opts.runner) }; } catch (e) {
+    if (e instanceof JunitError) return { exit, output, error: e };
     throw e;
   }
 }
 
-export function runFlaky(opts: RunOptions): RunResult {
+export async function runFlaky(opts: RunOptions): Promise<RunResult> {
   const cwd = opts.cwd ?? process.cwd();
   const env = { ...(opts.env ?? process.env), PLAYWRIGHT_JUNIT_INCLUDE_RETRIES: '1' };
   const now = opts.now ?? (() => new Date());
   const empty = { tests: 0, outcomes: [], reruns: 0, ledger: [] };
-  const first = attempt(opts, opts.command, cwd, env);
+  const first = await attempt(opts, opts.command, cwd, env);
   if (!first.report) return { exitCode: 3, code: first.error!.code, message: first.error!.message, commandExit: first.exit, ...empty };
   const report = first.report;
   if (report.retriesHidden) {
@@ -151,6 +188,12 @@ export function runFlaky(opts: RunOptions): RunResult {
     return { exitCode: 3, code: 'FLAKY_COMMAND_FAILED', commandExit: first.exit, ...empty, tests: report.tests.length,
       message: `the command exited ${first.exit} but its JUnit reports no failed test; a crash or a non-test step failed` };
   }
+  const unreported = unreportedFailures(report.runner, first.output, failed.length);
+  if (unreported) {
+    return { exitCode: 1, code: 'FLAKY_UNREPORTED_FAILURES', commandExit: first.exit, ...empty, tests: report.tests.length,
+      outcomes: failed.map(t => ({ test: t, status: 'fail' as const, attempts: t.attempts, reason: 'not rerun: the runner reported failures outside the JUnit report' })),
+      message: `${unreported}; nothing was rerun` };
+  }
   const prov = provenance(cwd, env);
   const line = (t: JunitTest, kind: LedgerKind, by: LedgerLine['rerun_by'], attempts: string[]): LedgerLine =>
     ({ ts: now().toISOString(), runner: 'junit', kind, id: t.id, file: t.file, rerun_by: by, attempts, ...prov, excerpt: safeExcerpt(t.excerpt) });
@@ -164,7 +207,7 @@ export function runFlaky(opts: RunOptions): RunResult {
     if (!rerunnable) { outcomes.push({ test: t, status: 'fail', attempts: t.attempts, reason: `${failed.length} tests failed, more than --max-reruns ${opts.maxReruns}; not rerun` }); continue; }
     const argv = [...opts.command, ...rerunSelection(report.runner, t, opts.rerunArgs)];
     reruns++;
-    const again = attempt(opts, argv, cwd, env);
+    const again = await attempt(opts, argv, cwd, env);
     const match = again.report?.tests.find(x => x.id === t.id);
     if (!match) {
       outcomes.push({ test: t, status: 'rerun-fail', attempts: [...t.attempts, 'fail'], reason: again.error ? `rerun: ${again.error.message}` : 'rerun: the report does not contain this test' });
diff --git a/test/flaky-run.test.ts b/test/flaky-run.test.ts
index 4dd293816..e05bcf7e5 100644
--- a/test/flaky-run.test.ts
+++ b/test/flaky-run.test.ts
@@ -9,7 +9,7 @@ import * as os from 'os';
 import * as path from 'path';
 import { spawnSync } from 'child_process';
 import { JunitError, parseJunit, readJunit, testId } from '../lib/junit';
-import { renderResult, rerunSelection, runFlaky, safeExcerpt, stepSummary } from '../lib/flaky-run';
+import { renderResult, rerunSelection, runFlaky, safeExcerpt, stepSummary, unreportedFailures } from '../lib/flaky-run';
 
 const ROOT = path.resolve(import.meta.dir, '..');
 const FIX = path.join(ROOT, 'test', 'fixtures', 'junit');
@@ -85,16 +85,16 @@ describe('rerun selection', () => {
 });
 
 /** A fake runner: argv selection decides which canned report the attempt writes. */
-function fakeRun(reports: (argv: string[], n: number) => string | null, exits: (argv: string[], n: number) => number) {
+function fakeRun(reports: (argv: string[], n: number) => string | null, exits: (argv: string[], n: number) => number, outputs: (argv: string[], n: number) => string = () => '') {
   const calls: string[][] = [];
   return {
     calls,
     spawn(junit: string) {
-      return (argv: string[], _env: NodeJS.ProcessEnv, cwd: string) => {
+      return async (argv: string[], _env: NodeJS.ProcessEnv, cwd: string) => {
         calls.push(argv);
         const xml = reports(argv, calls.length);
         if (xml !== null) fs.writeFileSync(path.resolve(cwd, junit), xml);
-        return exits(argv, calls.length);
+        return { exit: exits(argv, calls.length), output: outputs(argv, calls.length) };
       };
     },
   };
@@ -108,10 +108,10 @@ describe('runFlaky', () => {
   const base = (spawn: ReturnType<typeof fakeRun>, dir: string, extra: Partial<Parameters<typeof runFlaky>[0]> = {}) =>
     runFlaky({ command: ['bun', 'test'], junit: 'r.xml', maxReruns: 3, cwd: dir, spawn: spawn.spawn('r.xml'), env: {}, now: () => new Date('2026-10-29T00:00:00Z'), ...extra });
 
-  test('a failed test that passes on rerun is a flaky pass and goes on the ledger', () => {
+  test('a failed test that passes on rerun is a flaky pass and goes on the ledger', async () => {
     const dir = tmp();
     const f = fakeRun((argv, n) => n === 1 ? suite(pass('a') + fail('b')) : suite(pass('b')), (_a, n) => n === 1 ? 1 : 0);
-    const r = base(f, dir);
+    const r = await base(f, dir);
     expect(r.exitCode).toBe(0);
     expect(f.calls[1]).toEqual(['bun', 'test', 'a.test.ts', '--test-name-pattern', '^b$']);
     expect(r.outcomes.map(o => [o.status, o.attempts.join(',')])).toEqual([['flaky-pass', 'fail,pass']]);
@@ -121,19 +121,19 @@ describe('runFlaky', () => {
     expect(stepSummary(r)).toContain('a.test.ts::::b');
   });
 
-  test('a test that fails its rerun is red and is ledgered as rerun-fail', () => {
+  test('a test that fails its rerun is red and is ledgered as rerun-fail', async () => {
     const dir = tmp();
     const f = fakeRun(() => suite(fail('b')), () => 1);
-    const r = base(f, dir);
+    const r = await base(f, dir);
     expect(r.exitCode).toBe(1);
     expect(r.outcomes[0]).toMatchObject({ status: 'rerun-fail', attempts: ['fail', 'fail'] });
     expect(r.ledger[0]!.kind).toBe('rerun-fail');
   });
 
-  test('more failures than --max-reruns: a real red, nothing rerun', () => {
+  test('more failures than --max-reruns: a real red, nothing rerun', async () => {
     const dir = tmp();
     const f = fakeRun(() => suite(fail('a') + fail('b') + fail('c') + fail('d')), () => 1);
-    const r = base(f, dir);
+    const r = await base(f, dir);
     expect(r.exitCode).toBe(1);
     expect(f.calls).toHaveLength(1);
     expect(r.reruns).toBe(0);
@@ -141,10 +141,10 @@ describe('runFlaky', () => {
     expect(r.ledger).toEqual([]);
   });
 
-  test('runner retries stand: no second rerun, and a runner flaky pass is ledgered', () => {
+  test('runner retries stand: no second rerun, and a runner flaky pass is ledgered', async () => {
     const dir = tmp();
     const f = fakeRun(() => fixture('playwright-r2-inc.xml'), () => 1);
-    const r = base(f, dir);
+    const r = await base(f, dir);
     expect(f.calls).toHaveLength(1);
     expect(r.exitCode).toBe(1);
     expect(r.ledger.filter(l => l.kind === 'flaky-pass').map(l => [l.id, l.rerun_by])).toEqual([
@@ -153,55 +153,71 @@ describe('runFlaky', () => {
     expect(r.outcomes.filter(o => o.status === 'fail').every(o => /already retried \(3 attempts\)/.test(o.reason!))).toBe(true);
   });
 
-  test('hidden Playwright retries refuse with a code instead of a guess', () => {
+  test('hidden Playwright retries refuse with a code instead of a guess', async () => {
     const dir = tmp();
-    const r = base(fakeRun(() => fixture('playwright-r2.xml'), () => 1), dir);
+    const r = await base(fakeRun(() => fixture('playwright-r2.xml'), () => 1), dir);
     expect([r.exitCode, r.code]).toEqual([3, 'FLAKY_RETRIES_HIDDEN']);
   });
 
-  test('no report, a stale report, and a crash with no failed test are never green', () => {
+  test('no report, a stale report, and a crash with no failed test are never green', async () => {
     const dir = tmp();
-    expect(base(fakeRun(() => null, () => 0), dir)).toMatchObject({ exitCode: 3, code: 'FLAKY_NO_RESULTS' });
+    expect(await base(fakeRun(() => null, () => 0), dir)).toMatchObject({ exitCode: 3, code: 'FLAKY_NO_RESULTS' });
     fs.writeFileSync(path.join(dir, 'r.xml'), suite(pass('stale')));
-    expect(base(fakeRun(() => null, () => 0), dir)).toMatchObject({ exitCode: 3, code: 'FLAKY_NO_RESULTS' });
-    expect(base(fakeRun(() => suite(pass('a')), () => 2), dir)).toMatchObject({ exitCode: 3, code: 'FLAKY_COMMAND_FAILED', commandExit: 2 });
+    expect(await base(fakeRun(() => null, () => 0), dir)).toMatchObject({ exitCode: 3, code: 'FLAKY_NO_RESULTS' });
+    expect(await base(fakeRun(() => suite(pass('a')), () => 2), dir)).toMatchObject({ exitCode: 3, code: 'FLAKY_COMMAND_FAILED', commandExit: 2 });
   });
 
-  test('a rerun that does not report the targeted test fails it; other tests in the rerun are ignored', () => {
+  test('a rerun that does not report the targeted test fails it; other tests in the rerun are ignored', async () => {
     const dir = tmp();
     const f = fakeRun((_a, n) => n === 1 ? suite(fail('b')) : suite(pass('other')), (_a, n) => n === 1 ? 1 : 0);
-    const r = base(f, dir);
+    const r = await base(f, dir);
     expect(r.exitCode).toBe(1);
     expect(r.outcomes[0]).toMatchObject({ status: 'rerun-fail', reason: 'rerun: the report does not contain this test' });
   });
 
-  test('a rerun whose command exits non-zero is not a flaky pass, even with a passing report', () => {
+  test('a rerun whose command exits non-zero is not a flaky pass, even with a passing report', async () => {
     const dir = tmp();
     const f = fakeRun((_a, n) => n === 1 ? suite(fail('b')) : suite(pass('b')), () => 1);
-    expect(base(f, dir).outcomes[0]).toMatchObject({ status: 'rerun-fail', reason: 'rerun: the command exited 1' });
+    expect((await base(f, dir)).outcomes[0]).toMatchObject({ status: 'rerun-fail', reason: 'rerun: the command exited 1' });
   });
 
-  test('a test reported twice with one failure is red and not rerun', () => {
+  test('a test reported twice with one failure is red and not rerun', async () => {
     const dir = tmp();
     const r = parseJunit(suite(pass('b') + fail('b')));
     expect(r.tests[0]).toMatchObject({ executions: 2, attempts: ['pass', 'fail'] });
     const f = fakeRun(() => suite(fail('b') + pass('b') + fail('c')), () => 1);
-    const out = base(f, dir);
+    const out = await base(f, dir);
     expect(out.outcomes.find(o => o.test.id.endsWith('::b'))).toMatchObject({ status: 'fail', reason: 'it ran 2 times in this report and one failed; not rerun' });
     expect(f.calls).toHaveLength(2);
   });
 
-  test('the Playwright environment flag reaches every attempt', () => {
+  test('the Playwright environment flag reaches every attempt', async () => {
     const dir = tmp();
     const seen: string[] = [];
-    runFlaky({ command: ['x'], junit: 'r.xml', maxReruns: 3, cwd: dir, env: {}, spawn: (_a, env, cwd) => {
+    await runFlaky({ command: ['x'], junit: 'r.xml', maxReruns: 3, cwd: dir, env: {}, spawn: async (_a, env, cwd) => {
       seen.push(env.PLAYWRIGHT_JUNIT_INCLUDE_RETRIES ?? '');
       fs.writeFileSync(path.join(cwd, 'r.xml'), suite(seen.length === 1 ? fail('b') : pass('b')));
-      return seen.length === 1 ? 1 : 0;
+      return { exit: seen.length === 1 ? 1 : 0, output: '' };
     } });
     expect(seen).toEqual(['1', '1']);
   });
 
+  test('bun load errors the JUnit omits keep the run red and nothing is rerun', async () => {
+    const dir = tmp();
+    const f = fakeRun(() => suite(fail('b')), () => 1, () => '\n 0 pass\n 3 fail\n 2 errors\nRan 3 tests across 3 files.\n');
+    const r = await base(f, dir);
+    expect(r).toMatchObject({ exitCode: 1, code: 'FLAKY_UNREPORTED_FAILURES', reruns: 0 });
+    expect(f.calls).toHaveLength(1);
+  });
+
+  test('runner summaries: Playwright errors outside tests and count mismatches are unreported failures', () => {
+    expect(unreportedFailures('playwright', '  1 error was not a part of any test, see above for details\n  1 failed\n', 1)).toContain('outside any test');
+    expect(unreportedFailures('playwright', '  2 failed\n  10 passed\n', 1)).toContain('counted 2 failed');
+    expect(unreportedFailures('playwright', '  1 failed\n  [chromium] x\n  10 passed\n', 1)).toBeNull();
+    expect(unreportedFailures('bun', ' 4 pass\n 1 fail\n', 1)).toBeNull();
+    expect(unreportedFailures('generic', '5 failed', 0)).toBeNull();
+  });
+
   test('credentials in failure text never reach the ledger', () => {
     const key = ['sk', 'ant', 'api03', 'A'.repeat(93) + 'AA'].join('-');
     const out = safeExcerpt([`Error: request failed with key ${key}`]);