Repository navigation
fix(mcp): give the audit tests a loader seam createRequire cannot hide from - #9559
Merged
Merged
Conversation
…e 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.
10 tasks done
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…e from (cheaper-inference#9559) * fix(mcp): give the audit tests a loader seam createRequire cannot hide from Since cheaper-inference#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 (cheaper-inference#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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Vitest (fast-path)reds the entire PR queue on the 3audit.test.tsshutdown/fallback tests. This has been misdiagnosed as a flake (the #9095 merge notes call it a "pre-existing audit.test.ts flake") — it is deterministic: reproduced 3/3 locally on the clean tip.Root cause
#8959 switched the audit DB loader to
createRequire("better-sqlite3")so Electron/global-install resolution works.vi.doMockonly patches Vitest's ESM module graph — a CommonJScreateRequireload is invisible to it. The test's better-sqlite3 mock never engaged: the code opened a REAL empty sqlite file in the tempDATA_DIR(no such table: mcp_tool_auditon stderr) and every mock assertion counted 0 calls.Fix
globalThis.__omnirouteMcpAuditDb) — the module's own seam, no module mocking needed.__setBetterSqliteLoaderForTests, a new test-only loader override inaudit.ts; the productioncreateRequirepath is unchanged (node:sqliteitself isimport()'d, so itsdoMockstill works and the fallback is exercised end-to-end).Validation (TDD)
open-sse/mcp-servervitest suite 88/88; lint clean.Refs #8959 #9095