Merge commit 'refs/pull/2172/head' of https://github.com/garrytan/gstack into pr-2172-test-env

browse/test/batch.test.ts

@@ -42,9 +42,14 @@ beforeAll(async () => {
   // The test needs to start a server. Let's use the existing server infrastructure.
 });
 
-afterAll(() => {
+afterAll(async () => {
   try { testServer.server.stop(); } catch {}
-  setTimeout(() => process.exit(0), 500);
+  // Close only this file's own browser — never process.exit(): bun test runs
+  // all files in one process, so a delayed exit kills the whole suite
+  // (see test/no-suicide-exit.test.ts). close() can hang when the browser
+  // already died, and its internal 5s timeout ties bun's 5s hook timeout —
+  // so race it at 3s and abandon; the child is reaped at process exit.
+  try { await Promise.race([bm?.close(), new Promise((resolve) => setTimeout(resolve, 3000))]); } catch {}
 });
 
 // We need a running browse server for HTTP tests.

browse/test/commands.test.ts

@@ -94,11 +94,14 @@ beforeAll(async () => {
   await bm.launch();
 });
 
-afterAll(() => {
-  // Force kill browser instead of graceful close (avoids hang)
+afterAll(async () => {
   try { testServer.server.stop(); } catch {}
-  // bm.close() can hang — just let process exit handle it
-  setTimeout(() => process.exit(0), 500);
+  // Close only this file's own browser — never process.exit(): bun test runs
+  // all files in one process, so a delayed exit kills the whole suite
+  // (see test/no-suicide-exit.test.ts). close() can hang when the browser
+  // already died, and its internal 5s timeout ties bun's 5s hook timeout —
+  // so race it at 3s and abandon; the child is reaped at process exit.
+  try { await Promise.race([bm?.close(), new Promise((resolve) => setTimeout(resolve, 3000))]); } catch {}
 });
 
 // ─── Navigation ─────────────────────────────────────────────────

browse/test/compare-board.test.ts

@@ -69,10 +69,15 @@ beforeAll(async () => {
   await handleWriteCommand('goto', [boardUrl], bm);
 });
 
-afterAll(() => {
+afterAll(async () => {
   try { server.stop(); } catch {}
   fs.rmSync(tmpDir, { recursive: true, force: true });
-  setTimeout(() => process.exit(0), 500);
+  // Close only this file's own browser — never process.exit(): bun test runs
+  // all files in one process, so a delayed exit kills the whole suite
+  // (see test/no-suicide-exit.test.ts). close() can hang when the browser
+  // already died, and its internal 5s timeout ties bun's 5s hook timeout —
+  // so race it at 3s and abandon; the child is reaped at process exit.
+  try { await Promise.race([bm?.close(), new Promise((resolve) => setTimeout(resolve, 3000))]); } catch {}
 });
 
 // ─── DOM Structure ──────────────────────────────────────────────

browse/test/content-security.test.ts

@@ -422,9 +422,14 @@ describe('Hidden element stripping', () => {
     await bm.launch();
   });
 
-  afterAll(() => {
+  afterAll(async () => {
     try { testServer.server.stop(); } catch {}
-    setTimeout(() => process.exit(0), 500);
+    // Close only this file's own browser — never process.exit(): bun test
+    // runs all files in one process, so a delayed exit kills the whole suite
+    // (see test/no-suicide-exit.test.ts). close() can hang when the browser
+    // already died, and its internal 5s timeout ties bun's 5s hook timeout —
+    // so race it at 3s and abandon; the child is reaped at process exit.
+    try { await Promise.race([bm?.close(), new Promise((resolve) => setTimeout(resolve, 3000))]); } catch {}
   });
 
   test('detects CSS-hidden elements on injection-hidden page', async () => {

browse/test/handoff.test.ts

@@ -26,9 +26,14 @@ beforeAll(async () => {
   await bm.launch();
 });
 
-afterAll(() => {
+afterAll(async () => {
   try { testServer.server.stop(); } catch {}
-  setTimeout(() => process.exit(0), 500);
+  // Close only this file's own browser — never process.exit(): bun test runs
+  // all files in one process, so a delayed exit kills the whole suite
+  // (see test/no-suicide-exit.test.ts). close() can hang when the browser
+  // already died, and its internal 5s timeout ties bun's 5s hook timeout —
+  // so race it at 3s and abandon; the child is reaped at process exit.
+  try { await Promise.race([bm?.close(), new Promise((resolve) => setTimeout(resolve, 3000))]); } catch {}
 });
 
 // ─── Unit Tests: Failure Tracking (no browser needed) ────────────

browse/test/security-live-playwright.test.ts

@@ -56,9 +56,14 @@ describe('defense-in-depth — live Playwright fixture', () => {
     await bm.launch();
   });
 
-  afterAll(() => {
+  afterAll(async () => {
     try { testServer.server.stop(); } catch {}
-    setTimeout(() => process.exit(0), 500);
+    // Close only this file's own browser — never process.exit(): bun test
+    // runs all files in one process, so a delayed exit kills the whole suite
+    // (see test/no-suicide-exit.test.ts). close() can hang when the browser
+    // already died, and its internal 5s timeout ties bun's 5s hook timeout —
+    // so race it at 3s and abandon; the child is reaped at process exit.
+    try { await Promise.race([bm?.close(), new Promise((resolve) => setTimeout(resolve, 3000))]); } catch {}
   });
 
   test('L2 — content-security.ts hidden-element stripper detects the .sneaky div', async () => {

browse/test/snapshot.test.ts

@@ -31,9 +31,14 @@ beforeAll(async () => {
   await bm.launch();
 });
 
-afterAll(() => {
+afterAll(async () => {
   try { testServer.server.stop(); } catch {}
-  setTimeout(() => process.exit(0), 500);
+  // Close only this file's own browser — never process.exit(): bun test runs
+  // all files in one process, so a delayed exit kills the whole suite
+  // (see test/no-suicide-exit.test.ts). close() can hang when the browser
+  // already died, and its internal 5s timeout ties bun's 5s hook timeout —
+  // so race it at 3s and abandon; the child is reaped at process exit.
+  try { await Promise.race([bm?.close(), new Promise((resolve) => setTimeout(resolve, 3000))]); } catch {}
 });
 
 // ─── Snapshot Output ────────────────────────────────────────────

design/test/daemon.test.ts

@@ -361,16 +361,27 @@ describe("daemon /shutdown", () => {
     await fetchHandler(
       req("POST", `/boards/${board.id}/api/feedback`, { regenerated: false }),
     );
-    // Now non-done count is 0 — handler should return shuttingDown:true.
-    // We DON'T let the real gracefulShutdown timer fire (it calls process.exit
-    // after 50ms which would tear down the test runner); instead we just
-    // observe the immediate response.
-    const r = await fetchHandler(req("POST", "/shutdown"));
-    expect(r.status).toBe(200);
-    const body = (await r.json()) as any;
-    expect(body.shuttingDown).toBe(true);
-    // Reset state for subsequent tests; the shutdown timer will be a no-op
-    // because the next resetForTest flips shuttingDown back to false.
+    // The handler arms setTimeout(gracefulShutdown, 50), and gracefulShutdown
+    // arms setTimeout(process.exit, 50). bun test runs ALL files in one
+    // process, so letting that exit fire would kill the whole suite ~100ms
+    // later (exit 0, no summary — see test/no-suicide-exit.test.ts). Stub
+    // process.exit, wait past both timers so they fire harmlessly while
+    // stubbed, then restore. (resetForTest does NOT defuse the timers: the
+    // exit callback is unconditional.)
+    const origExit = process.exit;
+    (process as any).exit = (() => undefined) as any;
+    try {
+      const r = await fetchHandler(req("POST", "/shutdown"));
+      expect(r.status).toBe(200);
+      const body = (await r.json()) as any;
+      expect(body.shuttingDown).toBe(true);
+      // Let both 50ms timers (gracefulShutdown, then its process.exit) fire
+      // against the stub before restoring the real process.exit.
+      await new Promise((resolve) => setTimeout(resolve, 200));
+    } finally {
+      (process as any).exit = origExit;
+    }
+    // Reset state for subsequent tests (gracefulShutdown set shuttingDown).
     resetDaemon();
   });
 });

design/test/feedback-roundtrip.test.ts

@@ -121,10 +121,15 @@ beforeAll(async () => {
   await bm.launch();
 });
 
-afterAll(() => {
+afterAll(async () => {
   try { server.stop(); } catch {}
   fs.rmSync(tmpDir, { recursive: true, force: true });
-  setTimeout(() => process.exit(0), 500);
+  // Close only this file's own browser — never process.exit(): bun test runs
+  // all files in one process, so a delayed exit kills the whole suite
+  // (see test/no-suicide-exit.test.ts). close() can hang when the browser
+  // already died, and its internal 5s timeout ties bun's 5s hook timeout —
+  // so race it at 3s and abandon; the child is reaped at process exit.
+  try { await Promise.race([bm?.close(), new Promise((resolve) => setTimeout(resolve, 3000))]); } catch {}
 });
 
 // ─── The critical test: browser click → file on disk ─────────────

test/no-suicide-exit.test.ts

@@ -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([]);
+});

test/user-slug-fallback.test.ts

@@ -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_[a-f0-9]+:\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)', () => {