diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f5ecac983..c4226318a 100644 --- a/.github/workflows/ci.yml +++ b/.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" diff --git a/AGENTS.md b/AGENTS.md index 0df98dcdb..9c445f7db 100644 --- a/AGENTS.md +++ b/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 diff --git a/src/pricing-metadata.test.ts b/src/pricing-metadata.test.ts index 25f061a30..e0f46e011 100644 --- a/src/pricing-metadata.test.ts +++ b/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); diff --git a/src/session/state.test.ts b/src/session/state.test.ts index ed28ff369..8c2479aa9 100644 --- a/src/session/state.test.ts +++ b/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>; diff --git a/src/tools/web-search.test.ts b/src/tools/web-search.test.ts index 782019c1c..544f893a4 100644 --- a/src/tools/web-search.test.ts +++ b/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 }[] = []; 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, diff --git a/src/tui-opentui/mouse-reporting-disabled.test.ts b/src/tui-opentui/mouse-reporting-disabled.test.ts index 47ef8a8c9..a3b207318 100644 --- a/src/tui-opentui/mouse-reporting-disabled.test.ts +++ b/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, diff --git a/tests/unit/config.test.ts b/tests/unit/config.test.ts index 622f3dee6..671517759 100644 --- a/tests/unit/config.test.ts +++ b/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 }); diff --git a/tests/unit/tui/agent-tools.test.ts b/tests/unit/tui/agent-tools.test.ts index 2ca97e68e..c8a04cb06 100644 --- a/tests/unit/tui/agent-tools.test.ts +++ b/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 = {