From 4a9c4c54d2510586edb31b9040537514acbef94f Mon Sep 17 00:00:00 2001 From: whd4 Date: Mon, 13 Jul 2026 03:22:51 -0500 Subject: [PATCH 1/2] fix(test): un-leak GSTACK_HOME across files, file-scoped process.exit guard + idle-timer stop, closeTab zero-tab invariant, align config tests to DEFAULTS contract Co-Authored-By: Claude Fable 5 --- browse/src/browser-manager.ts | 15 +++++---- browse/src/server.ts | 5 +++ browse/test/cdp-e2e.test.ts | 6 ++++ browse/test/domain-skills-e2e.test.ts | 6 ++++ browse/test/domain-skills-storage.test.ts | 10 +++++- browse/test/gstack-config.test.ts | 5 +++ browse/test/server-factory.test.ts | 38 +++++++++++++++++++---- browse/test/telemetry.test.ts | 5 +++ 8 files changed, 75 insertions(+), 15 deletions(-) diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index f9f3317b51..9d63511935 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -803,15 +803,14 @@ export class BrowserManager { this.tabSessions.delete(tabId); this.tabOwnership.delete(tabId); - // Switch to another tab if we closed the active one - if (tabId === this.activeTabId) { + // Never leave zero tabs, regardless of which tab was closed — activeTabId + // can point at an already-removed id, and the old active-only guard then + // skipped auto-create when the last tab closed. + if (this.pages.size === 0) { + await this.newTab(); + } else if (tabId === this.activeTabId) { const remaining = [...this.pages.keys()]; - if (remaining.length > 0) { - this.activeTabId = remaining[remaining.length - 1]; - } else { - // No tabs left — create a new blank one - await this.newTab(); - } + this.activeTabId = remaining[remaining.length - 1]; } } diff --git a/browse/src/server.ts b/browse/src/server.ts index 301781acce..4c9103edcf 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -664,6 +664,11 @@ export const __testInternals__ = { // need shutdown to fire. Without this, the second test's shutdown // returns early at the `if (isShuttingDown) return;` guard. resetShutdownState: () => { isShuttingDown = false; }, + // Stop the module-level 60s idle interval. In-process test runs import this + // module once for the whole suite; the interval otherwise outlives the test + // file and can fire a real process.exit(0) mid-suite, silently truncating + // the run with a green exit code. + stopIdleTimer: () => { clearInterval(idleCheckInterval); }, }; // ─── Parent-Process Watchdog ──────────────────────────────────────── diff --git a/browse/test/cdp-e2e.test.ts b/browse/test/cdp-e2e.test.ts index c6b2c8a8af..2bcd66dfa5 100644 --- a/browse/test/cdp-e2e.test.ts +++ b/browse/test/cdp-e2e.test.ts @@ -18,6 +18,7 @@ import { startTestServer } from './test-server'; import { BrowserManager } from '../src/browser-manager'; const TMP_HOME = path.join(os.tmpdir(), `gstack-cdp-e2e-${process.pid}-${Date.now()}`); +const ORIG_GSTACK_HOME = process.env.GSTACK_HOME; process.env.GSTACK_HOME = TMP_HOME; process.env.GSTACK_TELEMETRY_OFF = '1'; // don't pollute analytics during tests @@ -36,6 +37,11 @@ beforeAll(async () => { }); afterAll(async () => { + // Un-leak GSTACK_HOME: it outranks GSTACK_STATE_DIR in bin/gstack-config, + // so leaving it set breaks later test files in-process. + if (ORIG_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME; + else process.env.GSTACK_HOME = ORIG_GSTACK_HOME; + try { await bm.cleanup?.(); } catch {} try { testServer.server.stop(); } catch {} await fs.rm(TMP_HOME, { recursive: true, force: true }); diff --git a/browse/test/domain-skills-e2e.test.ts b/browse/test/domain-skills-e2e.test.ts index 29d33c4bc8..598d7bdc0b 100644 --- a/browse/test/domain-skills-e2e.test.ts +++ b/browse/test/domain-skills-e2e.test.ts @@ -17,6 +17,7 @@ import { startTestServer } from './test-server'; import { BrowserManager } from '../src/browser-manager'; const TMP_HOME = path.join(os.tmpdir(), `gstack-domain-e2e-${process.pid}-${Date.now()}`); +const ORIG_GSTACK_HOME = process.env.GSTACK_HOME; process.env.GSTACK_HOME = TMP_HOME; process.env.GSTACK_PROJECT_SLUG = 'e2e-test-slug'; @@ -41,6 +42,11 @@ beforeAll(async () => { }); afterAll(async () => { + // Un-leak GSTACK_HOME: it outranks GSTACK_STATE_DIR in bin/gstack-config, + // so leaving it set breaks later test files in-process. + if (ORIG_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME; + else process.env.GSTACK_HOME = ORIG_GSTACK_HOME; + try { await bm.cleanup?.(); } catch {} try { testServer.server.stop(); } catch {} await fs.rm(TMP_HOME, { recursive: true, force: true }); diff --git a/browse/test/domain-skills-storage.test.ts b/browse/test/domain-skills-storage.test.ts index df53d8bc92..c81a0c9f53 100644 --- a/browse/test/domain-skills-storage.test.ts +++ b/browse/test/domain-skills-storage.test.ts @@ -1,11 +1,19 @@ -import { describe, it, expect, beforeEach } from 'bun:test'; +import { describe, it, expect, beforeEach, afterAll } from 'bun:test'; import { promises as fs } from 'fs'; import * as path from 'path'; import * as os from 'os'; const TMP_HOME = path.join(os.tmpdir(), `gstack-test-${process.pid}-${Date.now()}`); +const ORIG_GSTACK_HOME = process.env.GSTACK_HOME; process.env.GSTACK_HOME = TMP_HOME; +afterAll(() => { + // Un-leak GSTACK_HOME: it outranks GSTACK_STATE_DIR in bin/gstack-config, + // so leaving it set breaks later test files in-process. + if (ORIG_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME; + else process.env.GSTACK_HOME = ORIG_GSTACK_HOME; +}); + // Re-import after env var set so module reads updated GSTACK_HOME async function freshImport() { // Bun caches modules; force reload by appending a query-string-like hack via dynamic import URL diff --git a/browse/test/gstack-config.test.ts b/browse/test/gstack-config.test.ts index e342a50b74..676fa55bc9 100644 --- a/browse/test/gstack-config.test.ts +++ b/browse/test/gstack-config.test.ts @@ -18,7 +18,12 @@ function run(args: string[] = [], extraEnv: Record = {}) { const result = Bun.spawnSync(['bash', SCRIPT, ...args], { env: { ...process.env, + // Set all three state-dir vars: other test files leak GSTACK_HOME / + // GSTACK_STATE_ROOT into process.env at module scope, and those + // outrank GSTACK_STATE_DIR in the script's resolution order. GSTACK_STATE_DIR: stateDir, + GSTACK_STATE_ROOT: stateDir, + GSTACK_HOME: stateDir, ...extraEnv, }, stdout: 'pipe', diff --git a/browse/test/server-factory.test.ts b/browse/test/server-factory.test.ts index 6b5feb2641..0cfc731265 100644 --- a/browse/test/server-factory.test.ts +++ b/browse/test/server-factory.test.ts @@ -411,6 +411,27 @@ function makeMockBrowserManager(mode: 'launched' | 'headed') { } describe('idle timer + onDisconnect dual-instance fix', () => { + // File-scoped process.exit guard. The shutdown path is fire-and-forget + // async: a test's per-call exit stub can be restored while a second + // shutdown invocation is still in flight, and that straggler then hits the + // REAL process.exit(0) — truncating the whole bun test run mid-suite with + // a green exit code and no failure tally. Per-test stubs installed below + // capture-and-restore against this file stub, never the real exit. + const fileExitStub = mock((_code?: number) => {}); + let realProcessExit: typeof process.exit; + beforeAll(() => { + realProcessExit = process.exit; + (process as any).exit = fileExitStub; + }); + afterAll(async () => { + // Kill the leaked module-level 60s interval so it can't fire shutdown + // (with the real process.exit) during a later test file. + __testInternals__.stopIdleTimer(); + // Grace period: let any straggler async shutdown land on the stub. + await new Promise(r => setTimeout(r, 300)); + (process as any).exit = realProcessExit; + }); + beforeEach(() => { __resetRegistry(); // Reset module state every test. Bun memoizes the server.ts module @@ -453,14 +474,19 @@ describe('idle timer + onDisconnect dual-instance fix', () => { buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any })); __testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000)); __testInternals__.idleCheckTick(); - // Drain microtasks: shutdown awaits flushBuffers + cfgBrowserManager.close - // before reaching process.exit. - await Promise.resolve(); - await Promise.resolve(); - await new Promise(r => setImmediate(r)); - await new Promise(r => setImmediate(r)); + // Poll until the fire-and-forget shutdown reaches process.exit. A fixed + // microtask drain can lose the race on slower hosts — and then the REAL + // process.exit(0) fires after the finally restores it, killing the whole + // bun test run mid-suite with a green exit code and no failure tally. + for (let i = 0; i < 500 && exitMock.mock.calls.length === 0; i++) { + await new Promise(r => setTimeout(r, 10)); + } expect(exitMock).toHaveBeenCalled(); } finally { + // Reset activity so the module-level 60s idle interval (which outlives + // this file) cannot re-fire shutdown with the real process.exit later + // in the suite. + __testInternals__.setLastActivity(Date.now()); (process as any).exit = originalExit; } }); diff --git a/browse/test/telemetry.test.ts b/browse/test/telemetry.test.ts index d3cd3219c9..f041ea1f4f 100644 --- a/browse/test/telemetry.test.ts +++ b/browse/test/telemetry.test.ts @@ -8,6 +8,7 @@ const TELEMETRY_FILE = path.join(TMP_HOME, 'analytics', 'browse-telemetry.jsonl' // Use GSTACK_HOME env to redirect telemetry writes (read each call, // not cached at module-load). +const ORIG_GSTACK_HOME = process.env.GSTACK_HOME; process.env.GSTACK_HOME = TMP_HOME; process.env.GSTACK_TELEMETRY_OFF = '0'; @@ -17,6 +18,10 @@ beforeEach(async () => { afterAll(async () => { await fs.rm(TMP_HOME, { recursive: true, force: true }); + // Un-leak GSTACK_HOME: it outranks GSTACK_STATE_DIR in bin/gstack-config's + // resolution order, so leaving it set breaks later test files in-process. + if (ORIG_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME; + else process.env.GSTACK_HOME = ORIG_GSTACK_HOME; }); async function readEvents(): Promise { From 94b7895ab34464f6a11807dd45e09aaa476e21c2 Mon Sep 17 00:00:00 2001 From: whd4 Date: Mon, 13 Jul 2026 04:04:22 -0500 Subject: [PATCH 2/2] fix(test): remove delayed process.exit(0) time bombs from 8 browser test files + exitFn seam in ServerConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each afterAll scheduled setTimeout(() => process.exit(0), 500) which fired ~500ms into the NEXT test file during full-suite runs, killing bun with a green exit code and no tally — silently truncating the suite and hiding every failure in later files. Co-Authored-By: Claude Fable 5 --- browse/src/server.ts | 13 ++- browse/test/batch.test.ts | 11 +- browse/test/commands.test.ts | 11 +- browse/test/compare-board.test.ts | 11 +- browse/test/content-security.test.ts | 11 +- browse/test/handoff.test.ts | 11 +- browse/test/security-live-playwright.test.ts | 11 +- browse/test/server-factory.test.ts | 110 ++++++++----------- browse/test/snapshot.test.ts | 8 +- design/test/feedback-roundtrip.test.ts | 9 +- 10 files changed, 122 insertions(+), 84 deletions(-) diff --git a/browse/src/server.ts b/browse/src/server.ts index 4c9103edcf..b247c5084b 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -243,6 +243,14 @@ export interface ServerConfig { * reference is the PID record + the file paths. */ ownsTerminalAgent?: boolean; + /** + * Process-exit seam. shutdown() calls this at the very end of its async + * teardown; defaults to process.exit. In-process test runs inject a + * recording stub here — the fire-and-forget shutdown path otherwise races + * per-test process.exit stubs, and a straggler real exit(0) truncates the + * whole bun test run with a green exit code and no failure tally. + */ + exitFn?: (code?: number) => void; } /** @@ -1494,6 +1502,9 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { // to gstack-owns. Matches the "default-true preserves CLI bit-for-bit" // premise even under malformed cfg. const ownsTerminalAgent = cfg.ownsTerminalAgent === false ? false : true; + // Exit seam: production defaults to process.exit; tests inject a stub so + // in-process shutdowns can never terminate the test runner itself. + const exitFn = cfg.exitFn ?? ((code?: number) => process.exit(code)); // ─── Terminal-Agent Watchdog (v1.44+) ───────────────────────────── // @@ -1619,7 +1630,7 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { cleanSingletonLocks(resolveChromiumProfile()); safeUnlinkQuiet(config.stateFile); - process.exit(exitCode); + exitFn(exitCode); } // Named lifecycle helper (matches closeTunnel style). Logs failures so diff --git a/browse/test/batch.test.ts b/browse/test/batch.test.ts index 3d904a1a97..0a52e1eafe 100644 --- a/browse/test/batch.test.ts +++ b/browse/test/batch.test.ts @@ -42,9 +42,16 @@ 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 the real browser (capped at 5s — close can hang) instead of a + // delayed process.exit(0): that timer fired ~500ms into the NEXT test + // file in a full-suite run and killed the whole bun process with a green + // exit code and no tally, silently truncating the run. + await Promise.race([ + bm.close().catch(() => {}), + new Promise(r => setTimeout(r, 5000)), + ]); }); // We need a running browse server for HTTP tests. diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index 9382cb27ed..48f0f6424b 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -94,11 +94,18 @@ beforeAll(async () => { await bm.launch(); }); -afterAll(() => { +afterAll(async () => { // Force kill browser instead of graceful close (avoids hang) try { testServer.server.stop(); } catch {} // bm.close() can hang — just let process exit handle it - setTimeout(() => process.exit(0), 500); + // Close the real browser (capped at 5s — close can hang) instead of a + // delayed process.exit(0): that timer fired ~500ms into the NEXT test + // file in a full-suite run and killed the whole bun process with a green + // exit code and no tally, silently truncating the run. + await Promise.race([ + bm.close().catch(() => {}), + new Promise(r => setTimeout(r, 5000)), + ]); }); // ─── Navigation ───────────────────────────────────────────────── diff --git a/browse/test/compare-board.test.ts b/browse/test/compare-board.test.ts index 0a453a43a8..36c09f706a 100644 --- a/browse/test/compare-board.test.ts +++ b/browse/test/compare-board.test.ts @@ -69,10 +69,17 @@ 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 the real browser (capped at 5s — close can hang) instead of a + // delayed process.exit(0): that timer fired ~500ms into the NEXT test + // file in a full-suite run and killed the whole bun process with a green + // exit code and no tally, silently truncating the run. + await Promise.race([ + bm.close().catch(() => {}), + new Promise(r => setTimeout(r, 5000)), + ]); }); // ─── DOM Structure ────────────────────────────────────────────── diff --git a/browse/test/content-security.test.ts b/browse/test/content-security.test.ts index 8c760460dc..01710c108c 100644 --- a/browse/test/content-security.test.ts +++ b/browse/test/content-security.test.ts @@ -422,9 +422,16 @@ describe('Hidden element stripping', () => { await bm.launch(); }); - afterAll(() => { + afterAll(async () => { try { testServer.server.stop(); } catch {} - setTimeout(() => process.exit(0), 500); + // Close the real browser (capped at 5s — close can hang) instead of a + // delayed process.exit(0): that timer fired ~500ms into the NEXT test + // file in a full-suite run and killed the whole bun process with a green + // exit code and no tally, silently truncating the run. + await Promise.race([ + bm.close().catch(() => {}), + new Promise(r => setTimeout(r, 5000)), + ]); }); test('detects CSS-hidden elements on injection-hidden page', async () => { diff --git a/browse/test/handoff.test.ts b/browse/test/handoff.test.ts index e6754637f5..ab4eaf74dd 100644 --- a/browse/test/handoff.test.ts +++ b/browse/test/handoff.test.ts @@ -26,9 +26,16 @@ beforeAll(async () => { await bm.launch(); }); -afterAll(() => { +afterAll(async () => { try { testServer.server.stop(); } catch {} - setTimeout(() => process.exit(0), 500); + // Close the real browser (capped at 5s — close can hang) instead of a + // delayed process.exit(0): that timer fired ~500ms into the NEXT test + // file in a full-suite run and killed the whole bun process with a green + // exit code and no tally, silently truncating the run. + await Promise.race([ + bm.close().catch(() => {}), + new Promise(r => setTimeout(r, 5000)), + ]); }); // ─── Unit Tests: Failure Tracking (no browser needed) ──────────── diff --git a/browse/test/security-live-playwright.test.ts b/browse/test/security-live-playwright.test.ts index c75a115d30..07855f8432 100644 --- a/browse/test/security-live-playwright.test.ts +++ b/browse/test/security-live-playwright.test.ts @@ -56,9 +56,16 @@ describe('defense-in-depth — live Playwright fixture', () => { await bm.launch(); }); - afterAll(() => { + afterAll(async () => { try { testServer.server.stop(); } catch {} - setTimeout(() => process.exit(0), 500); + // Close the real browser (capped at 5s — close can hang) instead of a + // delayed process.exit(0): that timer fired ~500ms into the NEXT test + // file in a full-suite run and killed the whole bun process with a green + // exit code and no tally, silently truncating the run. + await Promise.race([ + bm.close().catch(() => {}), + new Promise(r => setTimeout(r, 5000)), + ]); }); test('L2 — content-security.ts hidden-element stripper detects the .sneaky div', async () => { diff --git a/browse/test/server-factory.test.ts b/browse/test/server-factory.test.ts index 0cfc731265..cd2d9fc188 100644 --- a/browse/test/server-factory.test.ts +++ b/browse/test/server-factory.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, beforeEach, mock } from 'bun:test'; +import { describe, test, expect, beforeAll, beforeEach, afterAll, mock } from 'bun:test'; import { resolveConfigFromEnv, buildFetchHandler, @@ -445,92 +445,68 @@ describe('idle timer + onDisconnect dual-instance fix', () => { }); test('CRITICAL — REGRESSION: headed embedder does not auto-shutdown at idle', () => { - const exitMock = mock((_code?: number) => { throw new Error('process.exit called'); }); - const originalExit = process.exit; - (process as any).exit = exitMock; - try { - const mockBM = makeMockBrowserManager('headed'); - buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any })); - // Drive lastActivity past the idle threshold via the test seam instead - // of mutating Date.now — the leaked module-level setInterval would - // see fake-time and could fire shutdown if the timing aligned. - __testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000)); - __testInternals__.idleCheckTick(); - expect(exitMock).not.toHaveBeenCalled(); - } finally { - (process as any).exit = originalExit; - } + const exitSpy = mock((_code?: number) => {}); + const mockBM = makeMockBrowserManager('headed'); + buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any, exitFn: exitSpy })); + // Drive lastActivity past the idle threshold via the test seam instead + // of mutating Date.now — the leaked module-level setInterval would + // see fake-time and could fire shutdown if the timing aligned. + __testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000)); + __testInternals__.idleCheckTick(); + expect(exitSpy).not.toHaveBeenCalled(); }); test('headless still auto-shuts down at idle (paired defensive)', async () => { - // Non-throwing mock: idleCheckTick fires shutdown as a fire-and-forget - // async call. Throwing from process.exit becomes an unhandled rejection - // that the test runner catches. Recording the call is enough. - const exitMock = mock((_code?: number) => {}); - const originalExit = process.exit; - (process as any).exit = exitMock; - try { - const mockBM = makeMockBrowserManager('launched'); - buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any })); - __testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000)); - __testInternals__.idleCheckTick(); - // Poll until the fire-and-forget shutdown reaches process.exit. A fixed - // microtask drain can lose the race on slower hosts — and then the REAL - // process.exit(0) fires after the finally restores it, killing the whole - // bun test run mid-suite with a green exit code and no failure tally. - for (let i = 0; i < 500 && exitMock.mock.calls.length === 0; i++) { - await new Promise(r => setTimeout(r, 10)); - } - expect(exitMock).toHaveBeenCalled(); - } finally { - // Reset activity so the module-level 60s idle interval (which outlives - // this file) cannot re-fire shutdown with the real process.exit later - // in the suite. - __testInternals__.setLastActivity(Date.now()); - (process as any).exit = originalExit; + // Shutdown is fire-and-forget async and hard-calling process.exit from it + // is what this seam exists to avoid: a per-test process.exit stub can be + // restored while a straggler shutdown is still in flight, and the real + // exit(0) then kills the whole bun run mid-suite with a green exit code. + // The injected exitFn spy makes the assertion race-free and harmless. + const exitSpy = mock((_code?: number) => {}); + const mockBM = makeMockBrowserManager('launched'); + buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any, exitFn: exitSpy })); + __testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000)); + __testInternals__.idleCheckTick(); + // Poll until the async shutdown reaches the exit seam. + for (let i = 0; i < 500 && exitSpy.mock.calls.length === 0; i++) { + await new Promise(r => setTimeout(r, 10)); } + expect(exitSpy).toHaveBeenCalled(); + // Reset activity so the module-level 60s idle interval cannot re-fire + // shutdown later in the suite. + __testInternals__.setLastActivity(Date.now()); }); test('buildFetchHandler chains cfgBrowserManager.onDisconnect, preserving caller-set handler', async () => { const mockBM = makeMockBrowserManager('headed'); const callerCb = mock(async (_code?: number) => {}); mockBM.onDisconnect = callerCb; - buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any })); + const exitSpy = mock((_code?: number) => {}); + buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any, exitFn: exitSpy })); // gstack should have wrapped the caller-installed handler instead of // clobbering it (Codex finding: BrowserManager.onDisconnect is a public // field; gbrowser may set it before calling buildFetchHandler). expect(typeof mockBM.onDisconnect).toBe('function'); expect(mockBM.onDisconnect).not.toBe(callerCb); // Verify the chain: invoking the wrapped handler runs the caller - // callback AND reaches activeShutdown (which calls process.exit at the - // very end of its async path). Stubbing process.exit to throw aborts - // the chain before isShuttingDown can leak into later tests. - const exitMock = mock((_code?: number) => { throw new Error('process.exit called'); }); - const originalExit = process.exit; - (process as any).exit = exitMock; - try { - await expect((mockBM.onDisconnect as any)(0)).rejects.toThrow('process.exit called'); - expect(callerCb).toHaveBeenCalledWith(0); - expect(exitMock).toHaveBeenCalledWith(0); - } finally { - (process as any).exit = originalExit; + // callback AND reaches activeShutdown, whose teardown ends at the + // injected exit seam instead of the real process.exit. + await (mockBM.onDisconnect as any)(0); + for (let i = 0; i < 500 && exitSpy.mock.calls.length === 0; i++) { + await new Promise(r => setTimeout(r, 10)); } + expect(callerCb).toHaveBeenCalledWith(0); + expect(exitSpy).toHaveBeenCalledWith(0); }); test('tunnelActive blocks idle-shutdown even in headless mode', () => { - const exitMock = mock((_code?: number) => { throw new Error('process.exit called'); }); - const originalExit = process.exit; - (process as any).exit = exitMock; - try { - const mockBM = makeMockBrowserManager('launched'); - buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any })); - __testInternals__.setTunnelActive(true); - __testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000)); - __testInternals__.idleCheckTick(); - expect(exitMock).not.toHaveBeenCalled(); - } finally { - (process as any).exit = originalExit; - } + const exitSpy = mock((_code?: number) => {}); + const mockBM = makeMockBrowserManager('launched'); + buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any, exitFn: exitSpy })); + __testInternals__.setTunnelActive(true); + __testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000)); + __testInternals__.idleCheckTick(); + expect(exitSpy).not.toHaveBeenCalled(); }); test('lifecycle handlers (idleCheckTick + parent watchdog + SIGTERM) read activeBrowserManager, not module-level browserManager', () => { diff --git a/browse/test/snapshot.test.ts b/browse/test/snapshot.test.ts index 17b26c3d4e..e08b340a60 100644 --- a/browse/test/snapshot.test.ts +++ b/browse/test/snapshot.test.ts @@ -31,9 +31,13 @@ beforeAll(async () => { await bm.launch(); }); -afterAll(() => { +afterAll(async () => { try { testServer.server.stop(); } catch {} - setTimeout(() => process.exit(0), 500); + // Close the real browser instead of scheduling a delayed process.exit(0): + // in a full-suite run that timer fired ~500ms into the NEXT test file and + // killed the entire bun process with a green exit code and no tally — + // silently truncating the run and hiding every failure after this file. + await bm.close().catch(() => {}); }); // ─── Snapshot Output ──────────────────────────────────────────── diff --git a/design/test/feedback-roundtrip.test.ts b/design/test/feedback-roundtrip.test.ts index e8d63db233..be952dbc2d 100644 --- a/design/test/feedback-roundtrip.test.ts +++ b/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 the browser (capped) instead of a delayed process.exit(0): that + // timer killed the whole bun process mid-suite with a green exit code. + await Promise.race([ + bm.close().catch(() => {}), + new Promise(r => setTimeout(r, 5000)), + ]); }); // ─── The critical test: browser click → file on disk ─────────────