fix(tests): centralize test config dir lifecycle to prevent env var pollution

Bun runs test files sequentially in a single thread. Several test files
unconditionally deleted process.env.SENTRY_CONFIG_DIR in afterEach hooks,
causing the next file's module-level code to read undefined and crash with
TypeError. A runner image upgrade changed file discovery order, exposing
this latent bug.

Introduces useTestConfigDir() helper in test/helpers.ts that creates a
unique temp directory in beforeEach and restores (never deletes) the env
var in afterEach. Migrates all 11 affected test files to use it.

Author: BYK

AGENTS.md

@@ -383,6 +383,37 @@ Minimal comments, maximum clarity. Comments explain **intent and reasoning**, no
 | Unit tests | `*.test.ts` | `test/` (mirrors `src/`) |
 | E2E tests | `*.test.ts` | `test/e2e/` |
 
+### Test Environment Isolation (CRITICAL)
+
+Tests that need a database or config directory **must** use `useTestConfigDir()` from `test/helpers.ts`. This helper:
+- Creates a unique temp directory in `beforeEach`
+- Sets `SENTRY_CONFIG_DIR` to point at it
+- **Restores** (never deletes) the env var in `afterEach`
+- Closes the database and cleans up temp files
+
+**NEVER** do any of these in test files:
+- `delete process.env.SENTRY_CONFIG_DIR` — This pollutes other test files that load after yours
+- `const baseDir = process.env[CONFIG_DIR_ENV_VAR]!` at module scope — This captures a value that may be stale
+- Manual `beforeEach`/`afterEach` that sets/deletes `SENTRY_CONFIG_DIR`
+
+**Why**: Bun runs test files **sequentially in one thread** (load → run all tests → load next file). If your `afterEach` deletes the env var, the next file's module-level code reads `undefined`, causing `TypeError: The "paths[0]" property must be of type string`.
+
+```typescript
+// CORRECT: Use the helper
+import { useTestConfigDir } from "../helpers.js";
+
+const getConfigDir = useTestConfigDir("my-test-prefix-");
+
+// If you need the directory path in a test:
+test("example", () => {
+  const dir = getConfigDir();
+});
+
+// WRONG: Manual env var management
+beforeEach(() => { process.env.SENTRY_CONFIG_DIR = tmpDir; });
+afterEach(() => { delete process.env.SENTRY_CONFIG_DIR; }); // BUG!
+```
+
 ### Property-Based Testing
 
 Use property-based tests when verifying invariants that should hold for **any valid input**.

test/commands/cli/fix.test.ts

@@ -3,19 +3,16 @@
  */
 
 import { Database } from "bun:sqlite";
-import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
-import { mkdirSync, rmSync } from "node:fs";
+import { describe, expect, mock, test } from "bun:test";
 import { join } from "node:path";
 import { fixCommand } from "../../../src/commands/cli/fix.js";
-import {
-  CONFIG_DIR_ENV_VAR,
-  closeDatabase,
-} from "../../../src/lib/db/index.js";
+import { closeDatabase } from "../../../src/lib/db/index.js";
 import {
   EXPECTED_TABLES,
   generatePreMigrationTableDDL,
   initSchema,
 } from "../../../src/lib/db/schema.js";
+import { useTestConfigDir } from "../../helpers.js";
 
 /**
  * Generate DDL for creating a database with pre-migration tables.
@@ -64,46 +61,12 @@ function createDatabaseWithMissingTables(
   ).run();
 }
 
-let testDir: string;
-let originalConfigDir: string | undefined;
-
-beforeEach(() => {
-  // Save original config dir
-  originalConfigDir = process.env[CONFIG_DIR_ENV_VAR];
-
-  // Close any existing database connection
-  closeDatabase();
-
-  // Create unique test directory
-  const baseDir = originalConfigDir ?? "/tmp/sentry-cli-test";
-  testDir = join(
-    baseDir,
-    `fix-test-${Date.now()}-${Math.random().toString(36).slice(2)}`
-  );
-  mkdirSync(testDir, { recursive: true });
-  process.env[CONFIG_DIR_ENV_VAR] = testDir;
-});
-
-afterEach(() => {
-  closeDatabase();
-  // Restore original config dir
-  if (originalConfigDir) {
-    process.env[CONFIG_DIR_ENV_VAR] = originalConfigDir;
-  } else {
-    delete process.env[CONFIG_DIR_ENV_VAR];
-  }
-  // Clean up test directory
-  try {
-    rmSync(testDir, { recursive: true, force: true });
-  } catch {
-    // Ignore cleanup errors
-  }
-});
+const getTestDir = useTestConfigDir("fix-test-");
 
 describe("sentry cli fix", () => {
   test("reports no issues for healthy database", async () => {
     // Create healthy database
-    const db = new Database(join(testDir, "cli.db"));
+    const db = new Database(join(getTestDir(), "cli.db"));
     initSchema(db);
     db.close();
 
@@ -124,7 +87,7 @@ describe("sentry cli fix", () => {
 
   test("detects and reports missing columns in dry-run mode", async () => {
     // Create database with pre-migration tables (missing v4 columns)
-    const db = new Database(join(testDir, "cli.db"));
+    const db = new Database(join(getTestDir(), "cli.db"));
     createPreMigrationDatabase(db);
     db.close();
 
@@ -148,7 +111,7 @@ describe("sentry cli fix", () => {
 
   test("fixes missing columns when not in dry-run mode", async () => {
     // Create database with pre-migration tables (missing v4 columns)
-    const db = new Database(join(testDir, "cli.db"));
+    const db = new Database(join(getTestDir(), "cli.db"));
     createPreMigrationDatabase(db);
     db.close();
 
@@ -169,7 +132,7 @@ describe("sentry cli fix", () => {
 
     // Verify the column was actually added
     closeDatabase();
-    const verifyDb = new Database(join(testDir, "cli.db"));
+    const verifyDb = new Database(join(getTestDir(), "cli.db"));
     const cols = verifyDb.query("PRAGMA table_info(dsn_cache)").all() as Array<{
       name: string;
     }>;
@@ -188,7 +151,7 @@ describe("sentry cli fix", () => {
   // that was previously missing tables (now fixed by auto-repair at startup).
   test("handles database that was auto-repaired at startup", async () => {
     // Create database missing dsn_cache - initSchema will create it when command runs
-    const db = new Database(join(testDir, "cli.db"));
+    const db = new Database(join(getTestDir(), "cli.db"));
     createDatabaseWithMissingTables(db, ["dsn_cache"]);
     db.close();
 
@@ -210,7 +173,7 @@ describe("sentry cli fix", () => {
 
     // Verify the table was created (by initSchema auto-repair)
     closeDatabase();
-    const verifyDb = new Database(join(testDir, "cli.db"));
+    const verifyDb = new Database(join(getTestDir(), "cli.db"));
     const tables = verifyDb
       .query(
         "SELECT name FROM sqlite_master WHERE type='table' AND name='dsn_cache'"
@@ -221,7 +184,7 @@ describe("sentry cli fix", () => {
   });
 
   test("shows database path in output", async () => {
-    const db = new Database(join(testDir, "cli.db"));
+    const db = new Database(join(getTestDir(), "cli.db"));
     initSchema(db);
     db.close();
 
@@ -237,6 +200,6 @@ describe("sentry cli fix", () => {
 
     const output = stdoutWrite.mock.calls.map((c) => c[0]).join("");
     expect(output).toContain("Database:");
-    expect(output).toContain(testDir);
+    expect(output).toContain(getTestDir());
   });
 });

test/commands/issue/utils.test.ts

@@ -13,9 +13,8 @@ import {
 } from "../../../src/commands/issue/utils.js";
 import { DEFAULT_SENTRY_URL } from "../../../src/lib/constants.js";
 import { setAuthToken } from "../../../src/lib/db/auth.js";
-import { CONFIG_DIR_ENV_VAR } from "../../../src/lib/db/index.js";
 import { setOrgRegion } from "../../../src/lib/db/regions.js";
-import { cleanupTestDir, createTestConfigDir } from "../../helpers.js";
+import { useTestConfigDir } from "../../helpers.js";
 
 describe("buildCommandHint", () => {
   test("suggests <org>/ID for numeric IDs", () => {
@@ -47,15 +46,13 @@ describe("buildCommandHint", () => {
   });
 });
 
-let testConfigDir: string;
+const getConfigDir = useTestConfigDir("test-issue-utils-", {
+  isolateProjectRoot: true,
+});
+
 let originalFetch: typeof globalThis.fetch;
 
 beforeEach(async () => {
-  // Use isolateProjectRoot to prevent DSN detection from scanning the real project
-  testConfigDir = await createTestConfigDir("test-issue-utils-", {
-    isolateProjectRoot: true,
-  });
-  process.env[CONFIG_DIR_ENV_VAR] = testConfigDir;
   originalFetch = globalThis.fetch;
   await setAuthToken("test-token");
   // Pre-populate region cache for orgs used in tests to avoid region resolution API calls
@@ -65,9 +62,8 @@ beforeEach(async () => {
   await setOrgRegion("org1", DEFAULT_SENTRY_URL);
 });
 
-afterEach(async () => {
+afterEach(() => {
   globalThis.fetch = originalFetch;
-  await cleanupTestDir(testConfigDir);
 });
 
 describe("resolveOrgAndIssueId", () => {
@@ -106,7 +102,7 @@ describe("resolveOrgAndIssueId", () => {
     await expect(
       resolveOrgAndIssueId({
         issueArg: "123456789",
-        cwd: testConfigDir,
+        cwd: getConfigDir(),
         command: "explain",
       })
     ).rejects.toThrow("Organization");
@@ -144,7 +140,7 @@ describe("resolveOrgAndIssueId", () => {
 
     const result = await resolveOrgAndIssueId({
       issueArg: "my-org/PROJECT-ABC",
-      cwd: testConfigDir,
+      cwd: getConfigDir(),
       command: "explain",
     });
 
@@ -196,7 +192,7 @@ describe("resolveOrgAndIssueId", () => {
 
     const result = await resolveOrgAndIssueId({
       issueArg: "f-g",
-      cwd: testConfigDir,
+      cwd: getConfigDir(),
       command: "explain",
     });
 
@@ -237,7 +233,7 @@ describe("resolveOrgAndIssueId", () => {
 
     const result = await resolveOrgAndIssueId({
       issueArg: "org1/dashboard-4y",
-      cwd: testConfigDir,
+      cwd: getConfigDir(),
       command: "explain",
     });
 
@@ -280,7 +276,7 @@ describe("resolveOrgAndIssueId", () => {
 
     const result = await resolveOrgAndIssueId({
       issueArg: "G",
-      cwd: testConfigDir,
+      cwd: getConfigDir(),
       command: "explain",
     });
 
@@ -298,7 +294,7 @@ describe("resolveOrgAndIssueId", () => {
     await expect(
       resolveOrgAndIssueId({
         issueArg: "G",
-        cwd: testConfigDir,
+        cwd: getConfigDir(),
         command: "explain",
       })
     ).rejects.toThrow("Cannot resolve issue suffix");
@@ -375,7 +371,7 @@ describe("resolveOrgAndIssueId", () => {
 
     const result = await resolveOrgAndIssueId({
       issueArg: "craft-g",
-      cwd: testConfigDir,
+      cwd: getConfigDir(),
       command: "explain",
     });
 
@@ -435,7 +431,7 @@ describe("resolveOrgAndIssueId", () => {
     await expect(
       resolveOrgAndIssueId({
         issueArg: "nonexistent-g",
-        cwd: testConfigDir,
+        cwd: getConfigDir(),
         command: "explain",
       })
     ).rejects.toThrow("not found");
@@ -511,7 +507,7 @@ describe("resolveOrgAndIssueId", () => {
     await expect(
       resolveOrgAndIssueId({
         issueArg: "common-g",
-        cwd: testConfigDir,
+        cwd: getConfigDir(),
         command: "explain",
       })
     ).rejects.toThrow("multiple organizations");
@@ -531,7 +527,7 @@ describe("resolveOrgAndIssueId", () => {
     await expect(
       resolveOrgAndIssueId({
         issueArg: "G",
-        cwd: testConfigDir,
+        cwd: getConfigDir(),
         command: "explain",
       })
     ).rejects.toThrow();
@@ -551,7 +547,7 @@ describe("resolveOrgAndIssueId", () => {
     await expect(
       resolveOrgAndIssueId({
         issueArg: "G",
-        cwd: testConfigDir,
+        cwd: getConfigDir(),
         command: "explain",
       })
     ).rejects.toThrow("500");

test/helpers.ts

@@ -4,9 +4,11 @@
  * Shared utilities for test setup and teardown.
  */
 
+import { afterEach, beforeEach } from "bun:test";
 import { mkdirSync } from "node:fs";
 import { mkdtemp, rm } from "node:fs/promises";
 import { join, resolve } from "node:path";
+import { CONFIG_DIR_ENV_VAR, closeDatabase } from "../src/lib/db/index.js";
 
 const TEST_TMP_DIR = resolve(import.meta.dir, "../.test-tmp");
 mkdirSync(TEST_TMP_DIR, { recursive: true });
@@ -73,3 +75,47 @@ type FetchMockFn =
 export function mockFetch(fn: FetchMockFn): typeof fetch {
   return fn as unknown as typeof fetch;
 }
+
+/**
+ * Sets up an isolated test config directory with proper env var lifecycle.
+ *
+ * Registers beforeEach/afterEach hooks that create a unique config directory,
+ * point SENTRY_CONFIG_DIR at it, and restore the original value on teardown.
+ * This eliminates the fragile pattern of manually managing process.env in
+ * each test file, which caused cross-file pollution when afterEach hooks
+ * deleted the env var while other files were still loading.
+ *
+ * Must be called at module scope or inside a describe() block.
+ *
+ * @param prefix - Directory name prefix for the temp directory
+ * @param options - Configuration options (e.g., isolateProjectRoot)
+ * @returns Getter function for the current test's config directory path
+ */
+export function useTestConfigDir(
+  prefix = "sentry-test-",
+  options?: TestConfigDirOptions
+): () => string {
+  let dir: string;
+  let savedConfigDir: string | undefined;
+
+  beforeEach(async () => {
+    savedConfigDir = process.env[CONFIG_DIR_ENV_VAR];
+    closeDatabase();
+    dir = await createTestConfigDir(prefix, options);
+    process.env[CONFIG_DIR_ENV_VAR] = dir;
+  });
+
+  afterEach(async () => {
+    closeDatabase();
+    // Always restore the previous value — never delete.
+    // Deleting process.env.SENTRY_CONFIG_DIR causes failures in test files
+    // that load after this afterEach runs, because their module-level code
+    // (or beforeEach hooks) may read the env var and get undefined.
+    if (savedConfigDir !== undefined) {
+      process.env[CONFIG_DIR_ENV_VAR] = savedConfigDir;
+    }
+    await cleanupTestDir(dir);
+  });
+
+  return () => dir;
+}