Merge randomized test-order isolation fixes

Author: TheGreatAxios

.github/workflows/ci.yml

@@ -5,6 +5,11 @@ on:
     branches: [main]
   pull_request:
   workflow_dispatch:
+  schedule:
+    # Nightly, a fresh seed each run -- the fixed-seed step above only ever
+    # exercises one shuffle of the suite, so a leak that this particular
+    # order does not disturb would otherwise stay invisible forever.
+    - cron: "17 7 * * *"
 
 jobs:
   check:
@@ -41,3 +46,40 @@ jobs:
       # inference package (vendor/) is out of scope for this repo's CI.
       - name: Test
         run: bun run test
+
+      # Catches tests that only pass because of the default file order (shared
+      # module-level state, an unrestored global mock, a leaked env var). The
+      # seed is fixed so a failure here reproduces locally with the same flag.
+      - name: Test (randomized order)
+        run: bun test ./src ./tests ./evals --randomize --seed 424242
+
+  randomize-nightly:
+    if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Setup Node
+        uses: actions/setup-node@v4
+        with:
+          node-version: "24"
+
+      - name: Setup Bun
+        uses: oven-sh/setup-bun@v2
+        with:
+          bun-version: "1.3.14"
+
+      - name: Install ripgrep
+        run: sudo apt-get install -y ripgrep
+
+      - name: Install dependencies
+        run: bun install --frozen-lockfile
+
+      # A fresh seed every run, printed up front so a failure here reproduces
+      # locally with the exact same `--seed` regardless of which shuffle hit it.
+      - name: Test (fresh random seed)
+        run: |
+          seed=$RANDOM$RANDOM
+          echo "seed=$seed"
+          bun test ./src ./tests ./evals --randomize --seed "$seed"

AGENTS.md

@@ -33,6 +33,7 @@ When refactoring replaces an old path, delete the old one. No back-compat shims,
 - Add or update tests with every behavior change.
 - 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.
 
 ## Build & Validation
 

src/pricing-metadata.test.ts

@@ -1,4 +1,4 @@
-import { afterEach, describe, expect, test } from "bun:test";
+import { afterEach, beforeEach, describe, expect, test } from "bun:test";
 import { mkdtemp, rm } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
@@ -15,6 +15,14 @@ import { contextWindowFor } from "./provider/context-window.js";
 import { writePricingCache } from "./cost/pricing-fetcher.js";
 
 describe("pricing-metadata", () => {
+  // refreshScheduled is a module-level one-shot latch shared with every other
+  // file in this process; another file's real loadConfig() call can leave it
+  // set before this file's first test ever runs. Reset on both sides so this
+  // suite's outcome does not depend on what ran before it.
+  beforeEach(() => {
+    resetPricingMetadataRefreshForTests();
+  });
+
   afterEach(() => {
     resetPricingMetadataRefreshForTests();
     applyPricingCacheMetadata(null);

src/session/state.test.ts

@@ -1,9 +1,14 @@
-import { afterEach, beforeEach, expect, mock, test } from "bun:test";
-import * as realFs from "node:fs/promises";
+import { afterAll, afterEach, beforeEach, expect, mock, 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")) };
+
 // 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.
@@ -20,6 +25,10 @@ mock.module("node:fs/promises", () => ({
   },
 }));
 
+afterAll(() => {
+  mock.module("node:fs/promises", () => realFs);
+});
+
 const { loadState, saveState } = await import("./state.js");
 type RunState = Awaited<ReturnType<typeof loadState>>;
 

src/tools/web-search.test.ts

@@ -1,9 +1,19 @@
-import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
+import { afterAll, afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
 
 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 {
@@ -21,6 +31,10 @@ mock.module("../mcp/client.js", () => ({
   },
 }));
 
+afterAll(() => {
+  mock.module("../mcp/client.js", () => realClient);
+});
+
 const {
   createWebSearchTool,
   disposeWebSearchClients,

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

@@ -22,7 +22,12 @@ const mountedHarnesses: Harness[] = []
 // 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.
-const realCore = await import("@opentui/core")
+//
+// 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,

tests/unit/config.test.ts

@@ -1,6 +1,12 @@
 import { test, expect, mock } from "bun:test";
 import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises";
 import * as nodeOs from "node:os";
+
+// Bun mutates the imported namespace object in place when a module is
+// mocked, so `nodeOs` itself is not safe to hold onto across a mock.module
+// call -- capture a shallow copy now, before anything mocks node:os, so the
+// snapshot below still reads "real" after the mock/restore round-trip.
+const realNodeOs = { ...nodeOs };
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { loadConfig } from "../../src/config/index.js";
@@ -226,7 +232,7 @@ test("loadConfig resolves an OAuth-profile provider absent from any settings fil
     // parameter — so the only way to point it at a synthetic auth store
     // without touching the real one is to stub node:os for the duration of
     // this call.
-    mock.module("node:os", () => ({ ...nodeOs, homedir: () => fakeHome }));
+    mock.module("node:os", () => ({ ...realNodeOs, homedir: () => fakeHome }));
     try {
       const { impl } = offlineFetch();
       const config = await loadConfig(
@@ -240,7 +246,7 @@ test("loadConfig resolves an OAuth-profile provider absent from any settings fil
         expect(config.providers.some((p) => p.name === "xai/synthetic")).toBe(true);
       }
     } finally {
-      mock.module("node:os", () => nodeOs);
+      mock.module("node:os", () => realNodeOs);
     }
   } finally {
     await rm(fakeHome, { recursive: true, force: true });

tests/unit/tui/agent-tools.test.ts

@@ -1,4 +1,4 @@
-import { test, expect, mock } from "bun:test";
+import { afterAll, test, expect, mock } from "bun:test";
 import type { ToolDefinition, ToolCall } from "@intx/types/runtime";
 import { TOOL_NAMES } from "@intx/tools-posix";
 
@@ -17,6 +17,27 @@ const mockPosixTools = {
   dispose: mockDispose,
 };
 
+// mock.module replaces the shared module cache for the whole test process, so
+// every other file that imports these modules runs against the mock until it
+// is put back. Capture the real modules up front and restore them in
+// afterAll so this file's mocking is invisible outside its own tests. Bun
+// mutates the imported namespace object in place when a module is mocked, so
+// each capture is shallow-copied immediately -- holding onto the live
+// namespace instead would silently turn into the mocked exports as soon as
+// mock.module below runs, making the "restore" a no-op.
+const realToolsPosix = { ...(await import("@intx/tools-posix")) };
+const realPosixToolPlugins = { ...(await import("../../../src/agent/posix-tool-plugins.js")) };
+const realMcpPlugin = { ...(await import("../../../src/mcp/plugin.js")) };
+const realPathEscapePlugin = { ...(await import("../../../src/plugins/path-escape-plugin.js")) };
+const realAuthzPlugin = { ...(await import("../../../src/plugins/authz-plugin.js")) };
+const realVerifyPlugin = { ...(await import("../../../src/plugins/verify-plugin.js")) };
+const realPermissionPlugin = { ...(await import("../../../src/plugins/permission-plugin.js")) };
+const realSecretGuardPlugin = { ...(await import("../../../src/plugins/secret-guard-plugin.js")) };
+const realShellGuardPlugin = { ...(await import("../../../src/plugins/shell-guard-plugin.js")) };
+const realReadFileGuardPlugin = { ...(await import("../../../src/plugins/read-file-guard-plugin.js")) };
+const realEditFileLineRange = { ...(await import("../../../src/plugins/edit-file-line-range.js")) };
+const realDirector = { ...(await import("../../../src/agent/director.js")) };
+
 mock.module("@intx/tools-posix", () => ({
   createPosixTools: () => mockPosixTools,
   TOOL_NAMES,
@@ -70,10 +91,6 @@ mock.module("../../../src/plugins/edit-file-line-range.js", () => ({
   advertiseEditFileLineRange: (defs: ToolDefinition[]) => defs,
 }));
 
-mock.module("../../../src/web/plugin.js", () => ({
-  webToolsPlugin: () => ({}),
-}));
-
 mock.module("../../../src/agent/director.js", () => ({
   askOperatorDefinition: {
     name: "ask_operator",
@@ -93,6 +110,21 @@ mock.module("../../../src/agent/director.js", () => ({
   createChatDirector: mock(() => ({})),
 }));
 
+afterAll(() => {
+  mock.module("@intx/tools-posix", () => realToolsPosix);
+  mock.module("../../../src/agent/posix-tool-plugins.js", () => realPosixToolPlugins);
+  mock.module("../../../src/mcp/plugin.js", () => realMcpPlugin);
+  mock.module("../../../src/plugins/path-escape-plugin.js", () => realPathEscapePlugin);
+  mock.module("../../../src/plugins/authz-plugin.js", () => realAuthzPlugin);
+  mock.module("../../../src/plugins/verify-plugin.js", () => realVerifyPlugin);
+  mock.module("../../../src/plugins/permission-plugin.js", () => realPermissionPlugin);
+  mock.module("../../../src/plugins/secret-guard-plugin.js", () => realSecretGuardPlugin);
+  mock.module("../../../src/plugins/shell-guard-plugin.js", () => realShellGuardPlugin);
+  mock.module("../../../src/plugins/read-file-guard-plugin.js", () => realReadFileGuardPlugin);
+  mock.module("../../../src/plugins/edit-file-line-range.js", () => realEditFileLineRange);
+  mock.module("../../../src/agent/director.js", () => realDirector);
+});
+
 const { createAgentToolset } = await import("../../../src/agent/tools.js");
 
 const fakePermissionGate = {