fix(test): remove all 8 delayed process.exit teardown bombs — the tier-1 gate can finally fail

bun test runs every file in ONE process, so a 500ms setTimeout(process.exit(0))
armed in afterAll fired mid-way through a LATER file and killed the entire
suite with exit 0 and no summary — only ~16 of 434 files ran, and every
downstream failure was invisible (observed live throughout this wave's
enumeration). Changes, all guarded by fault injection:

- Replace every delayed-exit teardown with a time-boxed close of the file's
  own browser (8 files across browse/ and design/); stub the daemon
  /shutdown timer instead of letting its unconditional process.exit tear
  the runner down.
- test/no-suicide-exit.test.ts: static tripwire — no *.test.ts may schedule
  a delayed process.exit again.
- test/exit-propagation.test.ts + fixtures: fault injection with REAL bun
  output proves the truncation shape (exit 0, no summary) and that
  scripts/test-free-shards.ts now detects it: a shard exiting 0 WITHOUT
  bun's final summary line is treated as FAILED (exit code alone is not
  evidence of completion).
- handoff: the three headed-mode integration tests are darwin-skipped with
  a pointer to the known macOS headed-launch breakage (#2242/#2554); they
  keep running on Linux CI. Un-skip in the browse-daemon wave.
- feedback-roundtrip: repair the handler call sites unmasked by the fix —
  handlers take (command, args, session, bm); passing the manager where a
  session belongs broke all six tests.
- user-slug-fallback: HOME isolation makes endpoint_hash deterministic.

Fixes #2421, #2435.

Contributed by @sneakygriff (PR #2172) with repairs from @time-attack
(PR #2230 feedback-roundtrip hunks); supersedes PR #2252 by @whd4 (same
defect, credited).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-08-14 20:20:51 -07:00
co-authored by Claude Fable 5
parent 3f176d2226
commit e0bfc8fff5
16 changed files with 322 additions and 77 deletions
+85
View File
@@ -0,0 +1,85 @@
import { describe, test, expect } from 'bun:test';
import { spawnSync } from 'child_process';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { shardRunLooksTruncated } from '../scripts/test-free-shards';
// Fault-injection companion to test/no-suicide-exit.test.ts.
//
// The static tripwire prevents OUR files from scheduling a delayed
// process.exit. This file proves, with real bun output, WHY that guard and
// the sharded runner's summary check both exist: `bun test` itself exits 0
// when a mid-suite process.exit(0) fires — the truncated run is
// indistinguishable from a green one by exit code alone. The sharded
// runner's shardRunLooksTruncated() predicate is the detection layer; these
// tests drive it with genuine truncated and genuine complete runs.
function runBunTest(dir: string) {
return spawnSync('bun', ['test', '.'], {
cwd: dir,
encoding: 'utf8',
timeout: 60000,
env: { ...process.env },
});
}
function withFixtureDir(files: Record<string, string>, fn: (dir: string) => void) {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'exit-prop-'));
try {
for (const [name, content] of Object.entries(files)) {
fs.writeFileSync(path.join(dir, name), content);
}
fn(dir);
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
}
// Fixture sources live as .txt (test/fixtures/exit-propagation/) and are
// copied to .test.ts names inside a temp dir at runtime — the no-suicide-exit
// static tripwire scans every *.test.ts in the repo, and inlining the suicide
// pattern here (even as a string) would rightly trip it.
const FIXTURES = path.join(import.meta.dir, 'fixtures', 'exit-propagation');
const SUICIDE_FIXTURE = fs.readFileSync(path.join(FIXTURES, 'suicide.txt'), 'utf8');
const FAILING_FIXTURE = fs.readFileSync(path.join(FIXTURES, 'failing.txt'), 'utf8');
const PASSING_FIXTURE = fs.readFileSync(path.join(FIXTURES, 'passing.txt'), 'utf8');
describe('exit-code propagation (fault injection)', () => {
test('a mid-suite process.exit(0) yields exit 0 with NO summary — and the shard predicate catches it', () => {
withFixtureDir(
{ 'a-suicide.test.ts': SUICIDE_FIXTURE, 'b-failing.test.ts': FAILING_FIXTURE },
(dir) => {
const r = runBunTest(dir);
const combined = `${r.stdout ?? ''}${r.stderr ?? ''}`;
if (r.status === 0) {
// The dangerous shape: green exit, truncated run. The predicate
// MUST flag it — this is the assertion that guards the suite.
expect(shardRunLooksTruncated(r.status, combined)).toBe(true);
} else {
// If a future bun version starts propagating the failure itself,
// even better — nothing to detect. Either way, never green+silent.
expect(r.status).not.toBe(0);
}
},
);
});
test('a complete green run is NOT flagged as truncated', () => {
withFixtureDir({ 'ok.test.ts': PASSING_FIXTURE }, (dir) => {
const r = runBunTest(dir);
const combined = `${r.stdout ?? ''}${r.stderr ?? ''}`;
expect(r.status).toBe(0);
expect(shardRunLooksTruncated(r.status, combined)).toBe(false);
});
});
test('a plain failing run propagates nonzero and is not the silent case', () => {
withFixtureDir({ 'fail.test.ts': FAILING_FIXTURE }, (dir) => {
const r = runBunTest(dir);
const combined = `${r.stdout ?? ''}${r.stderr ?? ''}`;
expect(r.status).not.toBe(0);
expect(shardRunLooksTruncated(r.status, combined)).toBe(false);
});
});
});
+2
View File
@@ -0,0 +1,2 @@
import { test, expect } from 'bun:test';
test('this failure must be visible', () => { expect(1).toBe(2); });
+2
View File
@@ -0,0 +1,2 @@
import { test, expect } from 'bun:test';
test('passes', () => { expect(1).toBe(1); });
+8
View File
@@ -0,0 +1,8 @@
import { test, expect } from 'bun:test';
test('passes then arms a delayed exit', () => {
expect(1).toBe(1);
setTimeout(() => process.exit(0), 300);
});
test('waits long enough for the timer to fire', async () => {
await new Promise((r) => setTimeout(r, 1500));
});
+49
View File
@@ -0,0 +1,49 @@
/**
* Guard: no test file may schedule a delayed process.exit().
*
* `bun test` runs EVERY test file in one process. The pattern of arming a
* 500ms timer in afterAll whose callback calls process.exit(0) — once used
* in several browse/design tests as a "bm.close() can hang" workaround —
* assumes each file gets its own process. It doesn't: the armed timer fires
* 500ms later, mid-way through a LATER test file, and kills the entire
* suite with exit code 0 and no summary. The truncated run silently masks
* every downstream failure (observed: only ~16 of 434 files ran, shell
* exit 0).
*
* This test statically scans every *.test.ts in the repo and fails if any
* schedules process.exit via setTimeout. Teardown must only release the
* file's own resources (e.g. `await bm.close()` — BrowserManager.close()
* is already time-boxed internally) — never terminate the shared runner.
*
* If a future test legitimately needs this pattern inside a child-process
* script (template literal passed to `bun -e`), split the child script
* into a fixture file instead of exempting it here.
*/
import { test, expect } from 'bun:test';
import fs from 'node:fs';
import path from 'node:path';
const repoRoot = path.resolve(import.meta.dir, '..');
// Matches a setTimeout whose arrow callback (with or without an argument)
// immediately calls process.exit. Doesn't match its own escaped source text
// (the backslashes in this regex literal prevent a literal-text match).
const DELAYED_EXIT = /setTimeout\(\s*(?:\(\s*\)|\(?\w+\)?)\s*=>\s*process\.exit\(/;
test('no test file schedules a delayed process.exit (kills the whole bun test run)', () => {
const glob = new Bun.Glob('**/*.test.ts');
const violations: string[] = [];
for (const rel of glob.scanSync({ cwd: repoRoot })) {
if (rel.includes('node_modules/')) continue;
const source = fs.readFileSync(path.join(repoRoot, rel), 'utf-8');
const lines = source.split('\n');
for (let i = 0; i < lines.length; i++) {
if (DELAYED_EXIT.test(lines[i])) {
violations.push(`${rel}:${i + 1}: ${lines[i].trim()}`);
}
}
}
expect(violations).toEqual([]);
});
+9 -1
View File
@@ -35,6 +35,12 @@ function runConfig(args: string[], extraEnv: Record<string, string> = {}): { std
encoding: 'utf-8',
env: {
...process.env,
// HOME isolation: endpoint_hash() reads $HOME/.claude.json for the
// gbrain MCP URL. Pointing HOME at the empty TMP_HOME makes it
// deterministically 'local' regardless of the developer's real
// ~/.claude.json (which would otherwise change the persisted key
// namespace to user_slug_at_<sha8-of-url>).
HOME: TMP_HOME,
...extraEnv,
},
timeout: 5000,
@@ -92,7 +98,9 @@ describe('resolve-user-slug fallback chain', () => {
const configFile = join(TMP_HOME, 'config.yaml');
expect(existsSync(configFile)).toBe(true);
const content = readFileSync(configFile, 'utf-8');
expect(content).toMatch(/^user_slug_at_(local|[a-f0-9]{8}|[a-f0-9]{16}):\s+persisttest/m);
// HOME is isolated to the empty TMP_HOME, so endpoint_hash() is
// deterministically the literal 'local' on every machine.
expect(content).toMatch(/^user_slug_at_local:\s+persisttest/m);
});
test('subsequent calls return same slug (stable across sessions)', () => {