fix: resolve Windows Electron build failures for missing native modules - #8959
Merged
diegosouzapw merged 2 commits intoAug 6, 2026
Conversation
- audit.ts: replace import('better-sqlite3') with createRequire() to avoid Webpack static resolution failure when the optional dependency is absent
- omp.ts: replace top-level import with lazy getDatabaseClass() wrapped in try/catch — returns null gracefully instead of crashing the process at module load
- electron/package.json: set buildDependenciesFromSource=false and npmRebuild=false to skip MSBuild/Windows SDK requirement during packaging
diegosouzapw
added a commit
that referenced
this pull request
Aug 6, 2026
…e 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>
10 tasks
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…es (diegosouzapw#8959) Validated in local merge-train (devbox-vm-06-dev002) @ combined-tip (FAST gates green: static + changed tests + vitest — only pre-existing audit.test.ts flake). Evidence: /home/diegosouzapw/dev/proxys/OmniRoute/.claude/worktrees/merge-train-20260805-213228-suite.log
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…e from (diegosouzapw#9559) * fix(mcp): give the audit tests a loader seam createRequire cannot hide from Since diegosouzapw#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 (diegosouzapw#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
2 participants
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.
audit.ts: replace import('better-sqlite3') with createRequire() to avoid Webpack static resolution failure when the optional dependency is absent
omp.ts: replace top-level import with lazy getDatabaseClass() wrapped in try/catch — returns null gracefully instead of crashing the process at module load
electron/package.json: set buildDependenciesFromSource=false and npmRebuild=false to skip MSBuild/Windows SDK requirement during packaging
Summary
Related Issues
Validation
Run only the focused loop for what you changed — the full unit suite, Vitest, the
60% coverage gate, and the production build all run in CI on this PR (#8329):
node --import tsx/esm --test tests/unit/<file>.test.tsnpm run lintTests Added Or Updated
Coverage Notes
src/,open-sse/,electron/, orbin/, explain which tests cover the change.Reviewer Notes