diff --git a/browse/src/server.ts b/browse/src/server.ts index 4c9103edc..b247c5084 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 3d904a1a9..0a52e1eaf 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 9382cb27e..48f0f6424 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 0a453a43a..36c09f706 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 8c760460d..01710c108 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 e6754637f..ab4eaf74d 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 c75a115d3..07855f843 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 0cfc73126..cd2d9fc18 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 17b26c3d4..e08b340a6 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 e8d63db23..be952dbc2 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 ─────────────