fix(mcp): give the audit tests a loader seam createRequire cannot hide from (#9559)

* fix(mcp): give the audit tests a loader seam createRequire cannot hide from

Since #8959 the audit DB loads better-sqlite3 via createRequire() (so
Electron/global-install resolution works) — which vi.doMock cannot
intercept: it only patches Vitest's ESM module graph. The audit.test.ts
better-sqlite3 mock therefore never engaged; the tests opened a REAL
empty sqlite file in the temp DATA_DIR ('no such table: mcp_tool_audit'
on stderr) and every mock assertion counted zero calls. The 3 failures
are deterministic (reproduced 3/3 locally), redding Vitest (fast-path)
for the entire PR queue — long misdiagnosed as a flake (#9095 merge
notes call it 'pre-existing audit.test.ts flake').

- Shutdown tests inject the mock through the audit connection cache
  (globalThis.__omnirouteMcpAuditDb) — the module's own seam.
- The node:sqlite fallback test drives __setBetterSqliteLoaderForTests,
  a test-only loader override; the production createRequire path is
  untouched (node:sqlite itself is import()'d, so its doMock still
  works).

3/3 red -> 3/3 green; full open-sse/mcp-server vitest suite 88/88.

* chore: align changelog slug with the PR number (9559)

---------

Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>

changelog.d/fixes/9559-mcp-audit-vitest-seam.md

@@ -0,0 +1 @@
+- **fix(mcp):** the 3 `audit.test.ts` shutdown/fallback tests fail deterministically since #8959 switched the audit DB loader to `createRequire("better-sqlite3")` — `vi.doMock` only patches Vitest's ESM module graph, so the old better-sqlite3 mock never engaged and the tests hit a real empty sqlite file ("no such table: mcp_tool_audit"), redding the `Vitest (fast-path)` job on every PR (long misdiagnosed as a flake). Shutdown tests now inject the mock through the audit connection cache (`globalThis.__omnirouteMcpAuditDb`), and the node:sqlite fallback test drives a new test-only loader seam (`__setBetterSqliteLoaderForTests`) — the production `createRequire` path is unchanged. 3/3 red → 3/3 green; full `open-sse/mcp-server` vitest suite 88/88.

open-sse/mcp-server/__tests__/audit.test.ts

@@ -18,6 +18,13 @@ function createStatementMock() {
   };
 }
 
+// #8959 made the production loader use createRequire() (Electron/global-install
+// resolution), which vi.doMock CANNOT intercept — it only patches Vitest's ESM
+// module graph. The old better-sqlite3 doMock therefore never engaged: the code
+// opened a REAL sqlite file in the temp DATA_DIR ("no such table" on stderr)
+// and every mock assertion counted 0 calls. The shutdown tests now inject the
+// mock through the audit connection cache (globalThis.__omnirouteMcpAuditDb),
+// and the fallback test uses the __setBetterSqliteLoaderForTests seam.
 describe("MCP audit shutdown", () => {
   let dataDir: string;
   let dbFile: string;
@@ -46,15 +53,10 @@ describe("MCP audit shutdown", () => {
         close: vi.fn(),
         open: true,
       };
-      const MockDatabase = vi.fn(function MockDatabase() {
-        return mockDb;
-      });
-
-      vi.doMock("better-sqlite3", () => ({
-        default: MockDatabase,
-      }));
 
       const audit = await import("../audit.ts");
+      // Inject through the connection cache — the seam the module itself uses.
+      globalThis.__omnirouteMcpAuditDb = mockDb as unknown as typeof globalThis.__omnirouteMcpAuditDb;
 
       await audit.logToolCall("omniroute_get_health", { ok: true }, { ok: true }, 12, true);
       expect(mockDb.prepare).toHaveBeenCalledTimes(1);
@@ -80,15 +82,9 @@ describe("MCP audit shutdown", () => {
       close: vi.fn(),
       open: true,
     };
-    const MockDatabase = vi.fn(function MockDatabase() {
-      return mockDb;
-    });
-
-    vi.doMock("better-sqlite3", () => ({
-      default: MockDatabase,
-    }));
 
     const audit = await import("../audit.ts");
+    globalThis.__omnirouteMcpAuditDb = mockDb as unknown as typeof globalThis.__omnirouteMcpAuditDb;
 
     await audit.logToolCall("omniroute_get_health", {}, {}, 5, true);
     expect(audit.closeAuditDb()).toBe(true);
@@ -103,26 +99,16 @@ describe("MCP audit shutdown", () => {
 
     // Simulate a global-install scenario where the bundled native binary
     // never landed in dist/node_modules/better-sqlite3/build/Release/.
+    // Thrown from the loader seam because the real load path is
+    // createRequire("better-sqlite3"), unreachable by vi.doMock.
     const bindingErr = new Error(
       "Could not locate the bindings file. Tried: …/better_sqlite3.node"
     ) as Error & { code?: string };
     bindingErr.code = "MODULE_NOT_FOUND";
-    // Simulate the binding-missing failure as the better-sqlite3 default
-    // constructor throwing — this matches reality (`new Database()` throws
-    // "Could not locate the bindings file" when the prebuilt .node is absent)
-    // and reaches the adapter's `catch (nativeErr)`. A factory that itself
-    // throws is reported by vitest as a mock-setup error and never reaches
-    // the code under test.
-    const ThrowingDatabase = vi.fn(function ThrowingDatabase() {
-      throw bindingErr;
-    });
-    vi.doMock("better-sqlite3", () => ({
-      default: ThrowingDatabase,
-    }));
 
-    // node:sqlite's DatabaseSync does not expose a boolean `open` property,
-    // so the mock intentionally omits it — the adapter tracks open state in
-    // a local closure and exposes it via a getter.
+    // node:sqlite IS loaded via dynamic import(), so doMock works for it.
+    // Its DatabaseSync does not expose a boolean `open` property — the
+    // adapter tracks open state in a local closure.
     const mockNodeDb = {
       prepare: vi.fn(() => createStatementMock()),
       exec: vi.fn(),
@@ -134,17 +120,24 @@ describe("MCP audit shutdown", () => {
     vi.doMock("node:sqlite", () => ({ DatabaseSync }));
 
     const audit = await import("../audit.ts");
+    audit.__setBetterSqliteLoaderForTests(() => {
+      throw bindingErr;
+    });
 
-    await audit.logToolCall("omniroute_get_health", { ok: true }, { ok: true }, 4, true);
-    expect(DatabaseSync).toHaveBeenCalledWith(dbFile);
-    expect(mockNodeDb.prepare).toHaveBeenCalled();
+    try {
+      await audit.logToolCall("omniroute_get_health", { ok: true }, { ok: true }, 4, true);
+      expect(DatabaseSync).toHaveBeenCalledWith(dbFile);
+      expect(mockNodeDb.prepare).toHaveBeenCalled();
 
-    expect(audit.closeAuditDb()).toBe(true);
-    expect(mockNodeDb.exec).toHaveBeenCalledWith("PRAGMA wal_checkpoint(TRUNCATE)");
-    expect(mockNodeDb.close).toHaveBeenCalledTimes(1);
+      expect(audit.closeAuditDb()).toBe(true);
+      expect(mockNodeDb.exec).toHaveBeenCalledWith("PRAGMA wal_checkpoint(TRUNCATE)");
+      expect(mockNodeDb.close).toHaveBeenCalledTimes(1);
 
-    // Cache is cleared after close, so a second close is a no-op.
-    expect(audit.closeAuditDb()).toBe(false);
-    expect(mockNodeDb.close).toHaveBeenCalledTimes(1);
+      // Cache is cleared after close, so a second close is a no-op.
+      expect(audit.closeAuditDb()).toBe(false);
+      expect(mockNodeDb.close).toHaveBeenCalledTimes(1);
+    } finally {
+      audit.__setBetterSqliteLoaderForTests(null);
+    }
   });
 });

open-sse/mcp-server/audit.ts

@@ -206,11 +206,27 @@ function toString(value: unknown): string {
   return typeof value === "string" ? value : "";
 }
 
+/**
+ * Test-only seam: the production load path uses `createRequire()` (so the
+ * Electron/global-install resolution works — #8959), which `vi.doMock` cannot
+ * intercept (it only patches Vitest's ESM module graph). Tests inject a
+ * throwing/mocked loader here to exercise the node:sqlite fallback.
+ */
+let betterSqliteLoaderForTests: (() => unknown) | null = null;
+export function __setBetterSqliteLoaderForTests(loader: (() => unknown) | null): void {
+  betterSqliteLoaderForTests = loader;
+}
+
 async function openBetterSqliteAuditDb(dbPath: string): Promise<AuditDatabase> {
-  const { createRequire } = await import("node:module");
-  const _require = createRequire(import.meta.url);
-  const mod = _require("better-sqlite3");
-  const Database = (mod?.default || mod) as unknown as new (
+  let mod: unknown;
+  if (betterSqliteLoaderForTests) {
+    mod = betterSqliteLoaderForTests();
+  } else {
+    const { createRequire } = await import("node:module");
+    const _require = createRequire(import.meta.url);
+    mod = _require("better-sqlite3");
+  }
+  const Database = ((mod as { default?: unknown })?.default || mod) as unknown as new (
     dbPath: string
   ) => AuditDatabase;
   return new Database(dbPath);