Give mock.module its own automatic teardown

AGENTS.md

@@ -34,6 +34,7 @@ When refactoring replaces an old path, delete the old one. No back-compat shims,
 - Bug fixes start with a failing test that reproduces the bug. Do not start by patching.
 - `tests/unit/` shared unit tests and helpers · co-located `src/**/*.test.ts` for module logic · `tests/fixtures/` fixture repos · `tests/integration/` reactor/permission harness. Planned: `tests/e2e/` (fixture-repo runs).
 - A test must not depend on another file having run, or on the default file order. It must pass under `bun test ./src ./tests ./evals --randomize`. If a test mutates module-level state or calls `mock.module`, it must restore that state itself (`afterEach`/`afterAll`), not rely on the process happening to reset it. When capturing a module's real exports to restore later, shallow-copy them (`{ ...moduleNamespace }`) at capture time, whether the namespace came from `await import(path)` or a static `import * as ns from "path"` — Bun mutates the live namespace object in place when the module is mocked, so holding a bare reference to it (either form) silently turns into the mocked exports.
+- Never call `mock.module` directly. Bun runs every test file in one process, so a `mock.module` call without its own teardown stays installed for the rest of the run and silently replaces the real module for other files — producing failures in files the change never touched, with no obvious link to the cause and no signal from `tsc` or a per-file run (CL-6967). Use `withMockedModule`/`withMockedModuleDuring` from `tests/helpers/mock-module.ts`, which capture the real module and register their own restore. An eslint rule (`no-restricted-syntax` in `eslint.config.js`) rejects bare `mock.module` calls in `*.test.ts` files.
 
 ## Build & Validation
 

eslint.config.js

@@ -56,4 +56,22 @@ export default tseslint.config(
       "no-control-regex": "off",
     },
   },
+  {
+    // A bare `mock.module` call has no teardown of its own, so a mock left
+    // installed by one test file silently replaces a real module for every
+    // other file in the same `bun test` process (see CL-6967). Route through
+    // withMockedModule/withMockedModuleDuring (tests/helpers/mock-module.ts)
+    // instead, which register their own restore.
+    files: ["**/*.test.ts"],
+    rules: {
+      "no-restricted-syntax": [
+        "error",
+        {
+          selector: "CallExpression[callee.object.name='mock'][callee.property.name='module']",
+          message:
+            "Use withMockedModule/withMockedModuleDuring from tests/helpers/mock-module.ts instead of bare mock.module — an un-restored mock.module leaks into every test file that runs after this one.",
+        },
+      ],
+    },
+  },
 );

src/auth/codex/instructions.test.ts

@@ -1,20 +1,15 @@
-import { afterAll, afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
+import { afterEach, beforeEach, describe, expect, test } from "bun:test";
+import { withMockedModule } from "../../../tests/helpers/mock-module.js";
 
 // Reused by both the TUI and exec boot paths (src/tui/runner.ts,
 // src/exec/runner.ts) to refresh the pinned Codex instructions before first
 // Codex inference. This exercises the shared refresh/fallback logic directly,
 // with disk I/O faked so tests never touch the real ~/.corbits cache.
-//
-// Shallow-copy + afterAll restore is required: Bun mutates the live module
-// namespace when mock.module runs, and CI loads ./src before ./tests — a leaked
-// node:fs mock turns later suites into ENOENT / missing-settings failures.
-
-const realFs = { ...(await import("node:fs")) };
 
 let fakeDisk = new Map<string, string>();
 
-mock.module("node:fs", () => ({
-  ...realFs,
+await withMockedModule(import.meta.resolve("node:fs"), (real: typeof import("node:fs")) => ({
+  ...real,
   readFileSync: (path: string) => {
     const contents = fakeDisk.get(path);
     if (contents === undefined) {
@@ -30,10 +25,6 @@ mock.module("node:fs", () => ({
   mkdirSync: () => undefined,
 }));
 
-afterAll(() => {
-  mock.module("node:fs", () => realFs);
-});
-
 const { refreshCodexInstructions, codexInstructions, codexInstructionsHash } =
   await import("./instructions.js");
 const { GPT_5_CODEX_PROMPT } = await import("./prompts/gpt-5-codex.js");

src/auth/oauth-scope-check.test.ts

@@ -1,27 +1,26 @@
-import { afterAll, afterEach, describe, expect, mock, test } from "bun:test";
+import { afterEach, describe, expect, test } from "bun:test";
+import { withMockedModule } from "../../tests/helpers/mock-module.js";
 
 // getValidCodexToken/getValidXaiToken hit the real home-level auth store and
 // refresh endpoints; stub the session layer so this test only exercises the
 // scope probe's own HTTP call and status classification. Other suites
 // (tests/unit/codex-session.test.ts) import the real modules directly, so the
 // mocks must be torn down after this file's tests run rather than leaking
 // into the rest of the bun test process.
-const realCodexSession = { ...(await import("./codex/session.js")) };
-const realXaiSession = { ...(await import("./xai/session.js")) };
-
-mock.module("./codex/session.js", () => ({
-  ...realCodexSession,
-  getValidCodexToken: async () => ({ access: "codex-token", accountId: "acct-1" }),
-}));
-mock.module("./xai/session.js", () => ({
-  ...realXaiSession,
-  getValidXaiToken: async () => ({ access: "xai-token" }),
-}));
-
-afterAll(() => {
-  mock.module("./codex/session.js", () => realCodexSession);
-  mock.module("./xai/session.js", () => realXaiSession);
-});
+await withMockedModule(
+  import.meta.resolve("./codex/session.js"),
+  (real: typeof import("./codex/session.js")) => ({
+    ...real,
+    getValidCodexToken: async () => ({ access: "codex-token", accountId: "acct-1" }),
+  }),
+);
+await withMockedModule(
+  import.meta.resolve("./xai/session.js"),
+  (real: typeof import("./xai/session.js")) => ({
+    ...real,
+    getValidXaiToken: async () => ({ access: "xai-token" }),
+  }),
+);
 
 const { checkOAuthProviderScope } = await import("./oauth-scope-check.js");
 

src/session/state.test.ts

@@ -1,33 +1,26 @@
-import { afterAll, afterEach, beforeEach, expect, mock, test } from "bun:test";
+import { afterEach, beforeEach, expect, test } from "bun:test";
 import { mkdir, rm } from "node:fs/promises";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
-
-// Bun mutates the imported namespace object in place when a module is
-// mocked, so the capture is shallow-copied immediately -- holding onto the
-// live namespace would turn into the mocked exports as soon as mock.module
-// below runs, making a later restore a no-op.
-const realFs = { ...(await import("node:fs/promises")) };
+import { withMockedModule } from "../../tests/helpers/mock-module.js";
 
 // Simulates the straggler write's real await point (e.g. cycleRecorder.dispose
 // during the terminal path) landing its writeFile after a later-issued
 // terminal write's writeFile, so rename-order alone would let it win.
-const realWriteFile = realFs.writeFile;
 let delayNextWrite = false;
-mock.module("node:fs/promises", () => ({
-  ...realFs,
-  writeFile: async (path: string, data: string) => {
-    if (delayNextWrite) {
-      delayNextWrite = false;
-      await new Promise((resolve) => setTimeout(resolve, 30));
-    }
-    return realWriteFile(path, data);
-  },
-}));
-
-afterAll(() => {
-  mock.module("node:fs/promises", () => realFs);
-});
+await withMockedModule(
+  import.meta.resolve("node:fs/promises"),
+  (real: typeof import("node:fs/promises")) => ({
+    ...real,
+    writeFile: async (path: string, data: string) => {
+      if (delayNextWrite) {
+        delayNextWrite = false;
+        await new Promise((resolve) => setTimeout(resolve, 30));
+      }
+      return real.writeFile(path, data);
+    },
+  }),
+);
 
 const { finalizeRunState, loadState, saveState } = await import("./state.js");
 const { getActiveRun, setActiveRun } = await import("./active-run.js");

src/tools/web-search.test.ts

@@ -1,39 +1,33 @@
-import { afterAll, afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
+import { afterEach, beforeEach, describe, expect, test } from "bun:test";
+import { withMockedModule } from "../../tests/helpers/mock-module.js";
 
 const calls: { toolName: string; args: Record<string, unknown> }[] = [];
 let connectConfigs: { name: string; url?: string }[] = [];
 
-// Bun mutates the imported namespace object in place when a module is
-// mocked, so the capture is shallow-copied immediately -- holding onto the
-// live namespace would turn into the mocked exports as soon as mock.module
-// below runs, making the afterAll restore a no-op. The mock also needs to
-// spread the real module rather than replace it outright, or any other
-// export (unwrapToolContent, connectMCPServers) disappears for the rest of
-// the process for every file that runs after this one.
-const realClient = { ...(await import("../mcp/client.js")) };
-
-mock.module("../mcp/client.js", () => ({
-  ...realClient,
-  connectMCPServer: async (config: { name: string; url?: string }) => {
-    connectConfigs.push(config);
-    return {
-      ok: true,
-      client: {
-        serverName: config.name,
-        tools: [],
-        call: async (toolName: string, args: Record<string, unknown>) => {
-          calls.push({ toolName, args });
-          return "mock result";
+// The mock needs to spread the real module rather than replace it outright,
+// or any other export (unwrapToolContent, connectMCPServers) disappears for
+// the rest of the process for every file that runs after this one.
+await withMockedModule(
+  import.meta.resolve("../mcp/client.js"),
+  (real: typeof import("../mcp/client.js")) => ({
+    ...real,
+    connectMCPServer: async (config: { name: string; url?: string }) => {
+      connectConfigs.push(config);
+      return {
+        ok: true,
+        client: {
+          serverName: config.name,
+          tools: [],
+          call: async (toolName: string, args: Record<string, unknown>) => {
+            calls.push({ toolName, args });
+            return "mock result";
+          },
+          close: async () => undefined,
         },
-        close: async () => undefined,
-      },
-    };
-  },
-}));
-
-afterAll(() => {
-  mock.module("../mcp/client.js", () => realClient);
-});
+      };
+    },
+  }),
+);
 
 const {
   createWebSearchTool,

src/tui/mouse-reporting-disabled.test.ts

@@ -7,8 +7,9 @@
  * `createCliRenderer` branch runs, and assert on the options it was
  * actually called with.
  */
-import { afterAll, afterEach, describe, expect, mock, test } from "bun:test";
+import { afterEach, describe, expect, test } from "bun:test";
 import type { Harness } from "./harness.js";
+import { withMockedModule } from "../../tests/helpers/mock-module.js";
 
 interface CapturedRendererOptions {
   readonly useMouse?: boolean;
@@ -21,33 +22,20 @@ const mountedHarnesses: Harness[] = [];
 // The mock must be registered before anything (including this file's own
 // helpers) does a real `@opentui/core` import, or that import wins the module
 // cache and the mock never takes effect. Every dependency below is loaded
-// with a dynamic `import()` after `mock.module` for that reason.
-//
-// Bun mutates the imported namespace object in place when a module is
-// mocked, so the capture is shallow-copied immediately -- holding onto the
-// live namespace would turn into the mocked exports the moment mock.module
-// below runs, making the afterAll restore below a no-op.
-const realCore = { ...(await import("@opentui/core")) };
-
-mock.module("@opentui/core", () => ({
-  ...realCore,
-  createCliRenderer: async (options: CapturedRendererOptions) => {
-    capturedOptions.push(options);
-    const { createHarness } = await import("./harness.js");
-    const harness = await createHarness({ width: 80, height: 24 });
-    mountedHarnesses.push(harness);
-    return harness.renderer;
-  },
-}));
-
-// `mock.module` replaces the shared module cache for the whole test process,
-// not just this file — every other test that imports `@opentui/core` runs in
-// the same process. Put the real module back once this file is done so a
-// later un-injected `createCliRenderer` caller does not silently get this
-// fake harness renderer instead.
-afterAll(() => {
-  mock.module("@opentui/core", () => realCore);
-});
+// with a dynamic `import()` after the mock is installed for that reason.
+await withMockedModule(
+  import.meta.resolve("@opentui/core"),
+  (real: typeof import("@opentui/core")) => ({
+    ...real,
+    createCliRenderer: async (options: CapturedRendererOptions) => {
+      capturedOptions.push(options);
+      const { createHarness } = await import("./harness.js");
+      const harness = await createHarness({ width: 80, height: 24 });
+      mountedHarnesses.push(harness);
+      return harness.renderer;
+    },
+  }),
+);
 
 afterEach(() => {
   let harness: Harness | undefined;

src/tui/provider-setup-submit.test.ts

@@ -1,25 +1,22 @@
-import { describe, test, expect, afterEach, afterAll, mock } from "bun:test";
+import { describe, test, expect, afterEach } from "bun:test";
 import { mkdtemp, rm } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 
 import type { OAuthScopeCheckResult } from "../auth/oauth-scope-check.js";
+import { withMockedModule } from "../../tests/helpers/mock-module.js";
 
 // The oauth branch probes real provider scope over the network; stub the
 // check so these tests exercise buildProviderSubmitHandler's own branching
-// (ok / insufficient-scope / unavailable) without a live call. Restored after
-// this file's tests run so a mock never leaks into another suite that
-// imports provider-setup-submit.js and expects the real probe.
-const realOAuthScopeCheck = { ...(await import("../auth/oauth-scope-check.js")) };
+// (ok / insufficient-scope / unavailable) without a live call.
 let scopeCheckResult: OAuthScopeCheckResult = { status: "ok" };
-mock.module("../auth/oauth-scope-check.js", () => ({
-  ...realOAuthScopeCheck,
-  checkOAuthProviderScope: async () => scopeCheckResult,
-}));
-
-afterAll(() => {
-  mock.module("../auth/oauth-scope-check.js", () => realOAuthScopeCheck);
-});
+await withMockedModule(
+  import.meta.resolve("../auth/oauth-scope-check.js"),
+  (real: typeof import("../auth/oauth-scope-check.js")) => ({
+    ...real,
+    checkOAuthProviderScope: async () => scopeCheckResult,
+  }),
+);
 
 const { buildProviderSubmitHandler } = await import("./provider-setup-submit.js");
 const { loadLocalSettings, loadSettings, localSettingsPath } =

tests/helpers/mock-module.ts

@@ -0,0 +1,72 @@
+import { afterAll, mock } from "bun:test";
+import { fileURLToPath } from "node:url";
+
+// `mock.module` keys its registry by filesystem path, not by `file://` URL --
+// registering under a URL string silently fails to intercept a module
+// resolved elsewhere by relative specifier, once anything else in the
+// process has already loaded the real module. `import.meta.resolve` returns
+// a `file://` URL for relative specifiers, so normalize it back to a path.
+function toModulePath(path: string): string {
+  return path.startsWith("file://") ? fileURLToPath(path) : path;
+}
+
+async function captureModule<T extends object>(path: string): Promise<T> {
+  // Bun mutates the imported namespace object in place when a module is
+  // mocked, so the capture must be a shallow copy taken before any mock
+  // installs -- holding onto the live namespace would silently turn into
+  // the mocked exports as soon as mock.module runs.
+  return { ...(await import(path)) } as T;
+}
+
+/**
+ * Mocks a module for the rest of this test file and registers its own
+ * `afterAll` restore, so correctness never depends on remembering to add
+ * one. Bun runs every test file in a single process, so an un-restored
+ * `mock.module` silently replaces the real module for every file that runs
+ * after this one -- this is the only sanctioned way to call `mock.module`
+ * at file scope.
+ *
+ * `impl` receives the captured real module so mocks can spread it
+ * (`...real`) without a separate capture line.
+ *
+ * Pass `path` as `import.meta.resolve("./relative/path.js")` from the
+ * calling file, not a bare relative specifier -- both `import()` and
+ * `mock.module` inside this helper resolve relative specifiers against
+ * this file's own location, not the caller's.
+ */
+export async function withMockedModule<T extends object>(
+  path: string,
+  impl: (real: T) => object,
+): Promise<T> {
+  const modulePath = toModulePath(path);
+  const real = await captureModule<T>(modulePath);
+  mock.module(modulePath, () => impl(real));
+  afterAll(() => {
+    mock.module(modulePath, () => real);
+  });
+  return real;
+}
+
+/**
+ * Mocks a module only for the duration of `run`, restoring it immediately
+ * afterward -- even if `run` throws -- rather than leaving it mocked for
+ * the rest of the file. Use this when a mock only needs to apply around a
+ * single call.
+ *
+ * Pass `path` as `import.meta.resolve("./relative/path.js")` -- see
+ * `withMockedModule` above.
+ */
+export async function withMockedModuleDuring<T extends object, R>(
+  path: string,
+  impl: (real: T) => object,
+  run: () => Promise<R>,
+): Promise<R> {
+  const modulePath = toModulePath(path);
+  const real = await captureModule<T>(modulePath);
+  mock.module(modulePath, () => impl(real));
+  try {
+    return await run();
+  } finally {
+    mock.module(modulePath, () => real);
+  }
+}