v4.2.6 fix(doctor,hooks): resolve better-sqlite3 via require.resolve (follow npm hoisting)#47
Merged
Merged
Conversation
…ow npm hoisting (v4.2.6)
The v4.2.5 native-binding probe regressed every fresh `npm install
@pcircle/memesh` install: it pre-checked
`<packageRoot>/node_modules/better-sqlite3` literally, but npm hoists
dependencies to the consumer's top-level node_modules. So when memesh
is installed as a dependency the directory at the literal path doesn't
exist even though `require('better-sqlite3')` works correctly via
Node's normal resolution.
Found this by verifying the v4.2.5 release with a clean-room install:
$ mkdir /tmp/verify && cd /tmp/verify
$ npm install @pcircle/memesh@4.2.5
$ ./node_modules/.bin/memesh doctor
Overall: FAIL
[FAIL] Native SQLite binding ← false negative on a working install
The fix in both surfaces (doctor.ts and _shared.js) is to drop the
existence-precheck and call `require.resolve('better-sqlite3',
{ paths: [pkgRoot] })`. Node's resolver walks the same path the
runtime uses (current dir → parent → ancestor's node_modules) and
returns the resolved module path, which we walk back to the package
root. A MODULE_NOT_FOUND error from the probe surfaces as a clean
"run `npm install`" hint; a `bindings file` error still surfaces as
the rebuild hint from v4.2.5.
The hook self-heal also retargets `npm rebuild`'s cwd to the project
that owns the hoisted node_modules tree (walking up until a
non-node_modules package.json appears). Running rebuild from memesh's
own pkgRoot would no-op when the binary is hoisted elsewhere.
Validation:
- Local clean-room install: smoke test confirms 4.2.6 doctor PASSes
on a hoisted install
- npm test --run: 1036/1038 (same 2 pre-existing failures as v4.2.5)
- memesh doctor: Overall: PASS_WITH_CONCERNS, native-binding PASS
- npm run lint: clean
Version bumped 4.2.5 -> 4.2.6 across all 7 anchor files. The CodeQL
config and TOCTOU fix from v4.2.5 carry forward unchanged.
Multi-Model SynthesisBoth Claude and Codex reviews above are independent. The most Reviewer responsibility: read both, surface non-overlapping |
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
1 participant
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
v4.2.5 shipped a regression: every fresh
npm install @pcircle/memeshsaw[FAIL] Native SQLite bindingfrommemesh doctor, even though the binding worked. The new check pre-existence-tested<packageRoot>/node_modules/better-sqlite3literally, but npm hoists deps to the consumer's top-levelnode_modules/.Found by clean-room verifying v4.2.5:
Both surfaces (
src/core/doctor.tsandscripts/hooks/_shared.js) now resolve viarequire.resolve('better-sqlite3', { paths: [pkgRoot] }), which uses Node's normal resolution algorithm and finds hoisted packages. The hook'snpm rebuildself-heal also retargets cwd to the project that owns the hoisted tree.Docs synced
Test plan
mktemp -d && npm install @pcircle/memesh@<staging>confirms native-binding check PASSes on hoisted installnpm test --run: 1036/1038 (same 2 pre-existing failures from main —tests/transports/http.test.tsrecall-empty,tests/tools.test.tsauto-archive)npm run lint: cleannpx tsc --noEmit: cleannpm run build: smoke 6/6reports FAIL with npm install hint when better-sqlite3 is not resolvable) exercises MODULE_NOT_FOUND probe response