diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index f9f3317b5..9d6351193 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 301781acc..4c9103edc 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 c6b2c8a8a..2bcd66dfa 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 29d33c4bc..598d7bdc0 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 df53d8bc9..c81a0c9f5 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 e342a50b7..676fa55bc 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 6b5feb264..0cfc73126 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 d3cd3219c..f041ea1f4 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 {