fix(native): validate ABI cache via child-process probe before trusting it#238
Open
husniadil wants to merge 1 commit intomksglu:nextfrom
Open
fix(native): validate ABI cache via child-process probe before trusting it#238husniadil wants to merge 1 commit intomksglu:nextfrom
husniadil wants to merge 1 commit intomksglu:nextfrom
Conversation
…ng it
Cached binaries were swapped in based solely on filename (e.g. abi137.node)
without verifying the binary actually matches the current Node ABI. This
caused persistent FTS5 failures when a binary compiled for one Node version
was incorrectly cached under another ABI label.
Two issues in the original ensureNativeCompat:
1. No validation after swapping in a cached binary — if the cache was
stale/corrupt, every subsequent session would fail silently.
2. In-process require() can't detect on-disk binary changes because
native .node modules are cached at the dlopen level. Additionally,
require('better-sqlite3') only loads the JS wrapper — the native
binary is lazy-loaded on first Database instantiation.
Fix: probe via child process (`node -e "new (require('better-sqlite3'))
(':memory:').close()"`) which gets a fresh dlopen cache and triggers
actual native binary loading. Falls through to npm rebuild if the
cached binary is invalid.
Also removes dead createRequire import and adds codesignBinary call
after rebuild in the non-cache path.
Adds 6 Vitest cases: corrupted cache recovery, valid cache fast path,
missing cache with compatible/incompatible binary, missing binary edge
case, and graceful degradation when both probe and rebuild fail.
Refs: mksglu#148
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 tasks
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.
Summary
Follow-up to #148. The ABI caching mechanism in
ensureNativeCompattrusts cached binaries based solely on filename label (e.g.abi137.node) without verifying the binary actually matches the current Node ABI. This causes persistent FTS5 failures that survive session restarts.Root cause
Two issues in the original probe logic:
No validation after cache swap — if a binary was incorrectly cached under the wrong ABI label (e.g. Node 20 session caches ABI 115 binary as
abi137.node), every subsequent Node 24 session silently fails.In-process
require()can't detect swapped binaries — native.nodemodules are cached at the dlopen level (per-process). Additionally,require('better-sqlite3')only loads the JS wrapper; the native binary is lazy-loaded on firstDatabaseinstantiation, so the original probe never actually triggered dlopen.Fix
Probe via child process (
node -e "new (require('better-sqlite3'))(':memory:').close()") which gets a fresh dlopen cache and triggers actual native binary loading. Falls through tonpm rebuildif the cached binary is invalid.Also:
createRequireimportcodesignBinary()after rebuild in the non-cache path (prevents SIGKILL on macOS hardened runtime)Behavioral change
The old probe only triggered rebuild on
NODE_MODULE_VERSIONerrors specifically. The new child-process probe treats any failure as needing rebuild — intentionally broader to handle edge cases (corrupt binaries, missing deps) at the cost of occasional unnecessary rebuilds (~5-30s).Test plan
Unit tests
tests/hooks/ensure-deps.test.ts:npx vitest run tests/hooks/ensure-deps.test.ts— 14/14 passmise exec node@20.19.4 -- npx vitest run tests/hooks/ensure-deps.test.tsReal-world validation
Patched the installed plugin (
~/.claude/plugins/cache/context-mode/context-mode/1.0.75/hooks/ensure-deps.mjs) and tested across live Claude Code sessions withmise-managed Node versions:abi137.nodecachedabi115.nodecachedabi137.nodecacheAfter test, both
abi115.nodeandabi137.nodecoexist inbuild/Release/. Switching Node versions is seamless — no rebuild needed after first cache.🤖 Generated with Claude Code