From c252f00b4c3c6037f9adf34e6a936653c7221c9f Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 29 Aug 2026 04:44:45 +0000 Subject: [PATCH] fix(test): scope GSTACK_HOME to each file's execution window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five files assigned process.env.GSTACK_HOME at module scope. Shard processes evaluate sibling modules before running their tests, so the assignment leaked into every other file in the shard — the damage was already visible in defensive workarounds (relink.test.ts:28 'fresh install test saw a neighbor's skill_prefix'; cdp-e2e's own comment documents a sibling's temp dir baked into artifacts). Pattern: save original, assign in beforeAll, restore in afterAll (cdp-e2e already restored but still assigned at load — its window now matches the others). GSTACK_TELEMETRY_OFF and GSTACK_PROJECT_SLUG get the same treatment where they rode along. Victim files' defenses stay in place (cheap insurance). Co-Authored-By: Claude Fable 5 --- browse/test/cdp-e2e.test.ts | 7 +++++-- browse/test/domain-skills-e2e.test.ts | 14 ++++++++++++-- browse/test/domain-skills-storage.test.ts | 16 ++++++++++++++-- browse/test/telemetry.test.ts | 19 ++++++++++++++++--- test/helpers/budget-override.test.ts | 16 ++++++++++++++-- 5 files changed, 61 insertions(+), 11 deletions(-) diff --git a/browse/test/cdp-e2e.test.ts b/browse/test/cdp-e2e.test.ts index c2d731350..d5d2e26c1 100644 --- a/browse/test/cdp-e2e.test.ts +++ b/browse/test/cdp-e2e.test.ts @@ -24,14 +24,15 @@ const TMP_HOME = path.join(os.tmpdir(), `gstack-cdp-e2e-${process.pid}-${Date.no // which then got baked into artifacts that outlived it (dangling symlinks // into a deleted render dir). Save + restore in afterAll. const ORIGINAL_GSTACK_HOME = process.env.GSTACK_HOME; -process.env.GSTACK_HOME = TMP_HOME; -process.env.GSTACK_TELEMETRY_OFF = '1'; // don't pollute analytics during tests +const ORIGINAL_TELEMETRY_OFF = process.env.GSTACK_TELEMETRY_OFF; let testServer: ReturnType; let bm: BrowserManager; let baseUrl: string; beforeAll(async () => { + process.env.GSTACK_HOME = TMP_HOME; + process.env.GSTACK_TELEMETRY_OFF = '1'; // don't pollute analytics during tests await fs.rm(TMP_HOME, { recursive: true, force: true }); await fs.mkdir(TMP_HOME, { recursive: true }); testServer = startTestServer(0); @@ -44,6 +45,8 @@ beforeAll(async () => { afterAll(async () => { if (ORIGINAL_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME; else process.env.GSTACK_HOME = ORIGINAL_GSTACK_HOME; + if (ORIGINAL_TELEMETRY_OFF === undefined) delete process.env.GSTACK_TELEMETRY_OFF; + else process.env.GSTACK_TELEMETRY_OFF = ORIGINAL_TELEMETRY_OFF; 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..4347e6c16 100644 --- a/browse/test/domain-skills-e2e.test.ts +++ b/browse/test/domain-skills-e2e.test.ts @@ -17,8 +17,12 @@ 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()}`); -process.env.GSTACK_HOME = TMP_HOME; -process.env.GSTACK_PROJECT_SLUG = 'e2e-test-slug'; + +// Scoped to this file's execution window — module-scope env assignment +// leaks into sibling files in the shard process (see +// test/gstack-home-module-scope.test.ts). +const ORIGINAL_GSTACK_HOME = process.env.GSTACK_HOME; +const ORIGINAL_PROJECT_SLUG = process.env.GSTACK_PROJECT_SLUG; let testServer: ReturnType; let bm: BrowserManager; @@ -32,6 +36,8 @@ async function fakeBodyPipe(body: string): Promise { } beforeAll(async () => { + process.env.GSTACK_HOME = TMP_HOME; + process.env.GSTACK_PROJECT_SLUG = 'e2e-test-slug'; await fs.rm(TMP_HOME, { recursive: true, force: true }); await fs.mkdir(path.join(TMP_HOME, 'projects', 'e2e-test-slug'), { recursive: true }); testServer = startTestServer(0); @@ -41,6 +47,10 @@ beforeAll(async () => { }); afterAll(async () => { + if (ORIGINAL_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME; + else process.env.GSTACK_HOME = ORIGINAL_GSTACK_HOME; + if (ORIGINAL_PROJECT_SLUG === undefined) delete process.env.GSTACK_PROJECT_SLUG; + else process.env.GSTACK_PROJECT_SLUG = ORIGINAL_PROJECT_SLUG; 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..07a0a3d4c 100644 --- a/browse/test/domain-skills-storage.test.ts +++ b/browse/test/domain-skills-storage.test.ts @@ -1,10 +1,22 @@ -import { describe, it, expect, beforeEach } from 'bun:test'; +import { describe, it, expect, beforeAll, 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()}`); -process.env.GSTACK_HOME = TMP_HOME; + +// Scoped to this file's execution window — module-scope env assignment +// leaks into sibling files in the shard process (see +// test/gstack-home-module-scope.test.ts). freshImport() below runs inside +// tests, so the beforeAll value is what ../src/domain-skills reads. +const ORIGINAL_GSTACK_HOME = process.env.GSTACK_HOME; +beforeAll(() => { + process.env.GSTACK_HOME = TMP_HOME; +}); +afterAll(() => { + if (ORIGINAL_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME; + else process.env.GSTACK_HOME = ORIGINAL_GSTACK_HOME; +}); // Re-import after env var set so module reads updated GSTACK_HOME async function freshImport() { diff --git a/browse/test/telemetry.test.ts b/browse/test/telemetry.test.ts index d3cd3219c..71a182eee 100644 --- a/browse/test/telemetry.test.ts +++ b/browse/test/telemetry.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterAll } from 'bun:test'; +import { describe, it, expect, beforeAll, beforeEach, afterAll } from 'bun:test'; import { promises as fs } from 'fs'; import * as path from 'path'; import * as os from 'os'; @@ -8,8 +8,21 @@ 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). -process.env.GSTACK_HOME = TMP_HOME; -process.env.GSTACK_TELEMETRY_OFF = '0'; +// Scoped to this file's execution window — module-scope env assignment +// leaks into sibling files in the shard process (see +// test/gstack-home-module-scope.test.ts). +const ORIGINAL_GSTACK_HOME = process.env.GSTACK_HOME; +const ORIGINAL_TELEMETRY_OFF = process.env.GSTACK_TELEMETRY_OFF; +beforeAll(() => { + process.env.GSTACK_HOME = TMP_HOME; + process.env.GSTACK_TELEMETRY_OFF = '0'; +}); +afterAll(() => { + if (ORIGINAL_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME; + else process.env.GSTACK_HOME = ORIGINAL_GSTACK_HOME; + if (ORIGINAL_TELEMETRY_OFF === undefined) delete process.env.GSTACK_TELEMETRY_OFF; + else process.env.GSTACK_TELEMETRY_OFF = ORIGINAL_TELEMETRY_OFF; +}); beforeEach(async () => { await fs.rm(TMP_HOME, { recursive: true, force: true }); diff --git a/test/helpers/budget-override.test.ts b/test/helpers/budget-override.test.ts index e420c3892..7785880ce 100644 --- a/test/helpers/budget-override.test.ts +++ b/test/helpers/budget-override.test.ts @@ -8,16 +8,28 @@ * timestamp + scope + reason + CI provenance. */ -import { describe, test, expect, beforeEach } from 'bun:test'; +import { describe, test, expect, beforeAll, beforeEach, afterAll } from 'bun:test'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { logBudgetOverride } from './budget-override'; const TMP_HOME = fs.mkdtempSync(path.join(os.tmpdir(), 'budget-override-test-')); -process.env.GSTACK_HOME = TMP_HOME; const AUDIT_PATH = path.join(TMP_HOME, 'analytics', 'spend-overrides.jsonl'); +// GSTACK_HOME is scoped to this file's execution window (beforeAll/afterAll), +// never set at module load: bun evaluates sibling modules before running +// their tests, so a module-scope assignment leaks into every other file in +// the shard process (pinned by test/gstack-home-module-scope.test.ts). +const ORIGINAL_GSTACK_HOME = process.env.GSTACK_HOME; +beforeAll(() => { + process.env.GSTACK_HOME = TMP_HOME; +}); +afterAll(() => { + if (ORIGINAL_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME; + else process.env.GSTACK_HOME = ORIGINAL_GSTACK_HOME; +}); + describe('logBudgetOverride', () => { beforeEach(() => { // Start each test with a clean audit file