test: fix all 182 failures unmasked by #2172 + closeTab last-tab race

browse/src/browser-manager.ts

@@ -798,19 +798,31 @@ export class BrowserManager {
     const page = this.pages.get(tabId);
     if (!page) throw new Error(`Tab ${tabId} not found`);
 
+    // Capture BEFORE close(): the page 'close' event handler wired in
+    // wirePageEvents() can fire while page.close() is awaited. It removes
+    // the tab from the maps and reassigns activeTabId (to 0 when no tabs
+    // remain), so a post-close `tabId === this.activeTabId` check is
+    // order-dependent — whether the event dispatches before or after
+    // close() resolves varies across Playwright/Chromium versions and
+    // machines, and losing the race means the last-tab auto-create below
+    // never runs, leaving the manager with zero tabs.
+    const wasActive = tabId === this.activeTabId;
+
     await page.close();
     this.pages.delete(tabId);
     this.tabSessions.delete(tabId);
     this.tabOwnership.delete(tabId);
 
     // Switch to another tab if we closed the active one
-    if (tabId === this.activeTabId) {
+    if (wasActive) {
       const remaining = [...this.pages.keys()];
-      if (remaining.length > 0) {
-        this.activeTabId = remaining[remaining.length - 1];
-      } else {
+      if (remaining.length === 0) {
         // No tabs left — create a new blank one
         await this.newTab();
+      } else if (!this.pages.has(this.activeTabId)) {
+        // The 'close' handler may have already switched to a valid tab;
+        // only reassign when activeTabId no longer points at a live tab.
+        this.activeTabId = remaining[remaining.length - 1];
       }
     }
   }

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

@@ -5,7 +5,7 @@
  * A real browse server is started and commands are sent via the CLI HTTP interface.
  */
 
-import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
+import { describe, test, expect, beforeAll, beforeEach, afterAll } from 'bun:test';
 import { startTestServer } from './test-server';
 import { BrowserManager } from '../src/browser-manager';
 import { resolveServerScript } from '../src/cli';
@@ -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 ─────────────────────────────────────────────────
@@ -135,7 +138,12 @@ describe('Navigation', () => {
 // ─── Content Extraction ─────────────────────────────────────────
 
 describe('Content extraction', () => {
-  beforeAll(async () => {
+  // beforeEach, NOT beforeAll: bun <1.3 runs every describe-scoped beforeAll
+  // eagerly at file start (in file order), so a beforeAll goto here is
+  // clobbered by later describes' beforeAll gotos before any test runs.
+  // beforeEach is scoped correctly on all bun versions, and a goto against
+  // the local fixture server costs only a few ms per test.
+  beforeEach(async () => {
     await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm);
   });
 
@@ -193,7 +201,9 @@ describe('Content extraction', () => {
 // ─── JavaScript / CSS / Attrs ───────────────────────────────────
 
 describe('Inspection', () => {
-  beforeAll(async () => {
+  // beforeEach, NOT beforeAll — see 'Content extraction' note (bun <1.3
+  // runs describe-scoped beforeAll hooks eagerly at file start).
+  beforeEach(async () => {
     await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm);
   });
 
@@ -1071,7 +1081,9 @@ describe('Dialog handling', () => {
 // ─── Element State Checks (is) ─────────────────────────────────
 
 describe('Element state checks', () => {
-  beforeAll(async () => {
+  // beforeEach, NOT beforeAll — see 'Content extraction' note (bun <1.3
+  // runs describe-scoped beforeAll hooks eagerly at file start).
+  beforeEach(async () => {
     await handleWriteCommand('goto', [baseUrl + '/states.html'], bm);
   });
 

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

@@ -124,6 +124,17 @@ describe('Content filter hooks', () => {
     clearContentFilters();
   });
 
+  // Restore module-load state when this describe is done. bun test runs
+  // every file in one process, so the content-security module instance is
+  // SHARED across test files — leaving the registry empty here breaks
+  // security-integration.test.ts, whose pipeline test pins that the
+  // built-in urlBlocklistFilter is registered (content-security.ts
+  // registers it at module load).
+  afterAll(() => {
+    clearContentFilters();
+    registerContentFilter(urlBlocklistFilter);
+  });
+
   test('URL blocklist detects requestbin', () => {
     const result = urlBlocklistFilter('', 'https://requestbin.com/r/abc', 'text');
     expect(result.safe).toBe(false);
@@ -422,9 +433,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/dual-listener.test.ts

@@ -220,7 +220,9 @@ describe('/command tunnel command allowlist', () => {
       'return handleCommand(body, tokenInfo)'
     );
     expect(commandBlock).toContain("surface === 'tunnel'");
-    expect(commandBlock).toContain('canDispatchOverTunnel(body?.command)');
+    // Args-aware since the --out (disk write) tunnel ban: the dispatch gate
+    // takes both the command and its args.
+    expect(commandBlock).toContain('canDispatchOverTunnel(body?.command, body?.args)');
     expect(commandBlock).toContain('disallowed_command');
     expect(commandBlock).toContain('is not allowed over the tunnel surface');
     expect(commandBlock).toContain('status: 403');

browse/test/gstack-config.test.ts

@@ -15,12 +15,20 @@ const SCRIPT = join(import.meta.dir, '..', '..', 'bin', 'gstack-config');
 let stateDir: string;
 
 function run(args: string[] = [], extraEnv: Record<string, string> = {}) {
+  // The script resolves GSTACK_STATE_ROOT > GSTACK_HOME > GSTACK_STATE_DIR.
+  // Scrub the two higher-priority overrides from the inherited env so a
+  // leaked GSTACK_HOME/GSTACK_STATE_ROOT from another test file (or the
+  // operator shell) can never redirect reads/writes away from this test's
+  // temp stateDir.
+  const env: Record<string, string | undefined> = {
+    ...process.env,
+    GSTACK_STATE_DIR: stateDir,
+    ...extraEnv,
+  };
+  if (!('GSTACK_STATE_ROOT' in extraEnv)) delete env.GSTACK_STATE_ROOT;
+  if (!('GSTACK_HOME' in extraEnv)) delete env.GSTACK_HOME;
   const result = Bun.spawnSync(['bash', SCRIPT, ...args], {
-    env: {
-      ...process.env,
-      GSTACK_STATE_DIR: stateDir,
-      ...extraEnv,
-    },
+    env,
     stdout: 'pipe',
     stderr: 'pipe',
   });

browse/test/gstack-update-check.test.ts

@@ -22,6 +22,14 @@ function run(extraEnv: Record<string, string> = {}, args: string[] = []) {
       ...process.env,
       GSTACK_DIR: gstackDir,
       GSTACK_STATE_DIR: stateDir,
+      // gstack-config resolves its state dir as GSTACK_STATE_ROOT >
+      // GSTACK_HOME > GSTACK_STATE_DIR. Other test files in the same bun
+      // process set process.env.GSTACK_HOME mid-suite (and operator shells
+      // may export it too); a leaked value would win over GSTACK_STATE_DIR
+      // above and point gstack-config at the wrong config.yaml, making the
+      // update_check config tests order-dependent. Pin all three.
+      GSTACK_HOME: stateDir,
+      GSTACK_STATE_ROOT: stateDir,
       GSTACK_REMOTE_URL: `file://${join(gstackDir, 'REMOTE_VERSION')}`,
       ...extraEnv,
     },

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 () => {