From cc54b40a6fa31148aedb4f36c58402a8593a2af4 Mon Sep 17 00:00:00 2001 From: Pelmoggian Date: Sat, 19 Sep 2026 10:49:39 +0100 Subject: [PATCH 1/2] fix: use N-API sqlite binding across Node versions --- .github/workflows/ci.yml | 7 +- CHANGELOG.md | 6 ++ package.json | 4 +- scripts/postinstall.js | 129 +++++++++++++++---------------------- test/allow-scripts.test.ts | 20 ++++-- test/mcp-server.test.ts | 55 ++++++++++++++++ 6 files changed, 131 insertions(+), 90 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eaa89887..88d9f2a6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,10 +38,9 @@ jobs: with: node-version: ${{ matrix.node }} # No `cache: npm`: that needs a lockfile, and this repo gitignores one. - # Measured, caching ~/.npm would buy nothing anyway - install time is - # node-gyp compiling better-sqlite3, not downloads (see the PR body). - # Installing cold every run also keeps the native-install path that - # issues #100/#102/#105/#125/#135/#162 are about under real test. + # Installing cold every run keeps the native dependency path that + # issues #100/#102/#105/#125/#135/#162 are about under real test, + # including the bundled better-sqlite3 N-API platform binary. # No lockfile is committed, so `npm ci` cannot be used. Dependency ranges # resolve fresh on every run; see the PR body for the caveat this implies. diff --git a/CHANGELOG.md b/CHANGELOG.md index 511074ff..5347f567 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- Native database installs now use the N-API-based `better-sqlite3` 13.x line, so switching Node versions no longer leaves an ABI-specific binding behind. This also avoids a Node 24.19-24.21 cleanup-hook crash caused by rebuilding the old 12.x addon from source. Install verification now performs a real sqlite-vec write and nearest-neighbor query, and the MCP integration test covers text search, vector search, and reading the returned archive. + ## [1.6.0] - 2026-09-08 Adds a fifth conversation source, an off switch for automatic syncing, and two fixes for real-world resource problems. diff --git a/package.json b/package.json index 58e050c9..8c43e5b4 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "./package.json": "./package.json" }, "allowScripts": { - "better-sqlite3": true, + "better-sqlite3": false, "onnxruntime-node": true, "sharp": false }, @@ -60,7 +60,7 @@ "@anthropic-ai/claude-agent-sdk": "^0.2.126", "@huggingface/transformers": "^4.2.0", "@modelcontextprotocol/sdk": "^1.20.0", - "better-sqlite3": "^12.4.1", + "better-sqlite3": "^13.0.3", "marked": "^16.4.0", "proper-lockfile": "^4.1.2", "sqlite-vec": "^0.1.7-alpha.2", diff --git a/scripts/postinstall.js b/scripts/postinstall.js index b60431bb..a45a6ce4 100644 --- a/scripts/postinstall.js +++ b/scripts/postinstall.js @@ -1,7 +1,7 @@ #!/usr/bin/env node /** - * Cross-platform postinstall: get better-sqlite3's native binding into a state - * where it actually loads, and refuse to report success until it does. + * Cross-platform postinstall: verify that better-sqlite3 and sqlite-vec work + * together, and refuse to report success until they do. * * Replaces the unix-only shell idiom that lived in package.json: * @@ -11,59 +11,43 @@ * and `|| true` doesn't behave the same — which makes `npm install` exit * non-zero even when every dependency installed correctly (#95). * - * Why the exit status of `npm rebuild` cannot be trusted (#100) - * ------------------------------------------------------------ - * `npm rebuild better-sqlite3` does not reliably run better-sqlite3's own - * install script (`prebuild-install || node-gyp rebuild --release`). On the npm - * shipped with recent Node it prints "rebuilt dependencies successfully", - * exits 0, and builds nothing. Separately, better-sqlite3 publishes prebuilds - * only for the Node versions in its `engines` range, so on a newer host Node - * `prebuild-install` can leave a binary compiled for an older ABI in place. + * Why this no longer rebuilds better-sqlite3 (#100) + * ------------------------------------------------- + * better-sqlite3 13.x ships N-API platform binaries in the npm package. They + * are not tied to a single Node module ABI and do not need an install script. + * Rebuilding the old 12.x addon was both unreliable and unsafe: a rebuild with + * Node 24.19-24.21 headers could produce a binding that loaded successfully but + * later aborted during statement garbage collection. * - * Both failures look identical from the outside: a clean install, and a plugin - * whose search silently returns nothing because the MCP server dies loading the - * binding — in a log nobody reads. + * A broken or partial install still looks like a clean install from the + * outside, while the MCP server dies loading the binding in a log nobody reads. * - * So this script does three things `status !== 0` cannot: + * So this script verifies behavior rather than trusting package-manager status: * * 1. Verifies by INSTANTIATING a database, not by requiring the module. * `require('better-sqlite3')` succeeds with no binding at all, because the * addon is loaded lazily inside `new Database()` (#100). - * 2. Verifies in a CHILD process. A native addon cannot be un-loaded, so a - * failed load poisons the module cache and an in-process re-check after a - * repair attempt would be meaningless. - * 3. On failure, falls back to better-sqlite3's real build path - * (`npm run build-release` = `node-gyp rebuild --release`) and re-verifies, - * rather than printing a recovery hint naming the command that just failed. + * 2. Loads sqlite-vec and performs a real vec0 insert plus nearest-neighbor + * query, rather than stopping at `vec_version()`. + * 3. Verifies in a CHILD process so native failures cannot poison the + * postinstall process. * - * Exit code reflects the binding, not the tooling: a noisy `npm rebuild` whose - * binding nevertheless loads is NOT fatal (that is the #95 false alarm), and a - * clean `npm rebuild` that leaves an unloadable binding IS. + * There is deliberately no automatic source-build fallback. If the bundled + * N-API binary cannot run, failing closed preserves the original evidence and + * avoids replacing it with a runtime-specific local build. */ import { spawnSync } from 'child_process'; -import { existsSync } from 'fs'; import { dirname, join } from 'path'; import { fileURLToPath } from 'url'; const PLUGIN_ROOT = dirname(dirname(fileURLToPath(import.meta.url))); -const BETTER_SQLITE3_DIR = join(PLUGIN_ROOT, 'node_modules', 'better-sqlite3'); - -const isWindows = process.platform === 'win32'; -const npmBin = isWindows ? 'npm.cmd' : 'npm'; - -function npm(args, cwd) { - return spawnSync(npmBin, args, { - cwd, - stdio: ['ignore', 'inherit', 'inherit'], - shell: isWindows, - }); -} /** - * Load the native stack the way the MCP server will — instantiate a database, - * then load sqlite-vec into it — in a throwaway child process. + * Load the native stack the way the MCP server will: instantiate a database, + * load sqlite-vec, write a vector, and run a nearest-neighbor query. * - * Returns null on success, or { stderr } describing the failure. + * Returns the installed better-sqlite3 version on success, or a failure with + * stderr on error. * * sqlite-vec is only fatal when it is actually installed; during some install * orderings it is not resolvable yet, which is a dependency problem the @@ -74,12 +58,23 @@ function verifyNativeStack() { const script = ` const { createRequire } = require('module'); const req = createRequire(${pkgJson}); + const version = req('better-sqlite3/package.json').version; const Database = req('better-sqlite3'); const db = new Database(':memory:'); let vec = null; try { vec = req('sqlite-vec'); } catch (e) { vec = null; } - if (vec) { vec.load(db); db.prepare('select vec_version()').get(); } + if (vec) { + vec.load(db); + db.exec('CREATE VIRTUAL TABLE native_probe USING vec0(id TEXT PRIMARY KEY, embedding FLOAT[2])'); + const embedding = Buffer.from(new Float32Array([1, 0]).buffer); + db.prepare('INSERT INTO native_probe (id, embedding) VALUES (?, ?)').run('probe', embedding); + const row = db.prepare( + 'SELECT id FROM native_probe WHERE embedding MATCH ? AND k = 1' + ).get(embedding); + if (!row || row.id !== 'probe') throw new Error('sqlite-vec query returned the wrong row'); + } db.close(); + process.stdout.write(version); `; const result = spawnSync(process.execPath, ['-e', script], { @@ -87,39 +82,23 @@ function verifyNativeStack() { encoding: 'utf-8', }); - if (result.status === 0) return null; - return { stderr: (result.stderr || '').trim() || `child exited ${result.status}` }; + if (result.status === 0) { + return { version: (result.stdout || '').trim() || 'unknown', failure: null }; + } + return { + version: null, + failure: (result.stderr || '').trim() || `child exited ${result.status}`, + }; } -// Attempt 1: the cheap path that works on most hosts. -const rebuild = npm(['rebuild', 'better-sqlite3'], PLUGIN_ROOT); -let failure = verifyNativeStack(); +const verification = verifyNativeStack(); +const failure = verification.failure; -// Attempt 2: better-sqlite3's real build path. `npm rebuild` frequently does -// not run it, and it is the step that actually compiles for this Node's ABI. -let fallback = null; -if (failure && existsSync(BETTER_SQLITE3_DIR)) { +if (!failure) { console.error( - 'episodic-memory: better-sqlite3 binding did not load after `npm rebuild`; ' + - 'compiling from source with `npm run build-release`...' + `episodic-memory: native stack verified (better-sqlite3 ${verification.version}, ` + + `${process.version}, N-API ${process.versions.napi}).` ); - fallback = npm(['run', 'build-release'], BETTER_SQLITE3_DIR); - failure = verifyNativeStack(); - if (!failure) { - console.error('episodic-memory: source build succeeded — native binding loads.'); - } -} - -if (!failure) { - if (rebuild.status !== 0 && !fallback) { - // Rebuild complained but the binding loads — the #95 false-alarm case. - console.error( - `episodic-memory: 'npm rebuild better-sqlite3' exited ${rebuild.status}, ` + - 'but the native binding loads correctly for this Node ' + - `(${process.version}, NODE_MODULE_VERSION ${process.versions.modules}). ` + - 'Continuing.' - ); - } process.exit(0); } @@ -137,21 +116,15 @@ if (compiledFor) { if (noBindings) { console.error(' binary : missing entirely (nothing was compiled)'); } -console.error(` npm rebuild exit : ${rebuild.status}`); -if (fallback) { - console.error(` source build exit: ${fallback.status}`); -} console.error(''); console.error(' underlying error:'); -for (const line of failure.stderr.split('\n').slice(0, 12)) { +for (const line of failure.split('\n').slice(0, 12)) { console.error(` ${line}`); } console.error(''); -console.error(' A source build needs a working toolchain: Xcode Command Line Tools on'); -console.error(' macOS, build-essential + python3 on Linux, VS Build Tools on Windows.'); -console.error(''); -console.error(' Recover with:'); -console.error(` cd "${BETTER_SQLITE3_DIR}" && npm run build-release`); +console.error(' better-sqlite3 13.x includes N-API platform binaries; do not compile'); +console.error(' the old 12.x addon as a fallback. Reinstall episodic-memory with the'); +console.error(' same package manifest, then rerun this check.'); console.error(''); console.error(' Failing the install deliberately: passing silently here is what lets'); console.error(' search disappear without anyone noticing.'); diff --git a/test/allow-scripts.test.ts b/test/allow-scripts.test.ts index 6f5c2e83..a1154d9c 100644 --- a/test/allow-scripts.test.ts +++ b/test/allow-scripts.test.ts @@ -5,17 +5,16 @@ import { join } from 'path'; const REPO_ROOT = join(import.meta.dirname, '..'); describe('package.json allowScripts (npm 12 install-script gating, #162)', () => { - it('approves the native-binding installs indexing depends on', () => { + it('allows only the native install scripts indexing still needs', () => { const pkg = JSON.parse(readFileSync(join(REPO_ROOT, 'package.json'), 'utf-8')); // Under npm 12 (and npm 11.16+ with blocking opted in), a dependency's // install/postinstall script is skipped unless the ROOT package's - // allowScripts explicitly permits it. Without this, `npm install` - // exits 0 while better-sqlite3 and onnxruntime-node silently ship - // with no native binding, and indexing/search fail forever with - // nothing surfacing the problem to the user. + // allowScripts explicitly permits it. onnxruntime-node still needs that + // permission; better-sqlite3 13.x bundles N-API binaries and must not fall + // back to a runtime-specific source build. expect(pkg.allowScripts).toBeDefined(); - expect(pkg.allowScripts['better-sqlite3']).toBe(true); + expect(pkg.allowScripts['better-sqlite3']).toBe(false); expect(pkg.allowScripts['onnxruntime-node']).toBe(true); // Bare package names, not pinned versions: a pinned key (e.g. @@ -26,6 +25,15 @@ describe('package.json allowScripts (npm 12 install-script gating, #162)', () => } }); + it('uses the N-API better-sqlite3 line that works across Node ABIs', () => { + const pkg = JSON.parse(readFileSync(join(REPO_ROOT, 'package.json'), 'utf-8')); + const postinstall = readFileSync(join(REPO_ROOT, 'scripts', 'postinstall.js'), 'utf-8'); + + expect(pkg.dependencies['better-sqlite3']).toBe('^13.0.3'); + expect(postinstall).not.toContain("npm(['rebuild', 'better-sqlite3']"); + expect(postinstall).not.toContain("npm(['run', 'build-release']"); + }); + it('explicitly denies sharp\'s postinstall (#102)', () => { const pkg = JSON.parse(readFileSync(join(REPO_ROOT, 'package.json'), 'utf-8')); diff --git a/test/mcp-server.test.ts b/test/mcp-server.test.ts index 47382c96..566decfb 100644 --- a/test/mcp-server.test.ts +++ b/test/mcp-server.test.ts @@ -5,6 +5,8 @@ import { existsSync, mkdirSync, mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { indexTestFiles } from './test-indexer.js'; +import { getFixturePath } from './test-utils.js'; const serverPath = fileURLToPath(new URL('../dist/mcp-server.js', import.meta.url)); @@ -65,6 +67,18 @@ describe('MCP search tool', () => { mkdirSync(join(testDir, 'archive'), { recursive: true }); mkdirSync(join(testDir, 'projects'), { recursive: true }); + const previousDbPath = process.env.TEST_DB_PATH; + process.env.TEST_DB_PATH = testDbPath; + try { + await indexTestFiles([getFixturePath('short-conversation.jsonl')]); + } finally { + if (previousDbPath === undefined) { + delete process.env.TEST_DB_PATH; + } else { + process.env.TEST_DB_PATH = previousDbPath; + } + } + client = new Client({ name: 'episodic-memory-test', version: '1.0.0' }, { capabilities: {} }); transport = new StdioClientTransport({ command: process.execPath, @@ -121,4 +135,45 @@ describe('MCP search tool', () => { }); expect(existsSync(testDbPath)).toBe(true); }); + + it('completes the real MCP text, vector, and archive-read path', async () => { + const tools = await client.listTools(); + expect(tools.tools.map((tool) => tool.name)).toEqual(expect.arrayContaining(['search', 'read'])); + + const textResult = await client.callTool({ + name: 'search', + arguments: { + query: 'Employee class', + mode: 'text', + limit: 1, + response_format: 'json', + }, + }); + expect(textResult.isError).toBeFalsy(); + const textPayload = JSON.parse(getTextContent(textResult.content as ToolContent[])); + expect(textPayload.count).toBeGreaterThan(0); + + const vectorResult = await client.callTool({ + name: 'search', + arguments: { + query: 'Python employee data design', + mode: 'vector', + limit: 1, + response_format: 'json', + }, + }); + expect(vectorResult.isError).toBeFalsy(); + + const vectorPayload = JSON.parse(getTextContent(vectorResult.content as ToolContent[])); + expect(vectorPayload.count).toBeGreaterThan(0); + const archivePath = vectorPayload.results[0].exchange.archivePath; + expect(archivePath).toBe(getFixturePath('short-conversation.jsonl')); + + const readResult = await client.callTool({ + name: 'read', + arguments: { path: archivePath, startLine: 1, endLine: 3 }, + }); + expect(readResult.isError).toBeFalsy(); + expect(getTextContent(readResult.content as ToolContent[])).toBeTruthy(); + }); }); From 92f490c5f2cfb1545cc52baa3d79b66c1c679df5 Mon Sep 17 00:00:00 2001 From: Pelmoggian Date: Sat, 19 Sep 2026 11:21:18 +0100 Subject: [PATCH 2/2] test: exercise native install failure reporting --- scripts/postinstall.js | 4 +-- test/allow-scripts.test.ts | 1 + test/postinstall-native-check.test.ts | 47 +++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 test/postinstall-native-check.test.ts diff --git a/scripts/postinstall.js b/scripts/postinstall.js index a45a6ce4..f9dc2347 100644 --- a/scripts/postinstall.js +++ b/scripts/postinstall.js @@ -102,8 +102,8 @@ if (!failure) { process.exit(0); } -const compiledFor = /NODE_MODULE_VERSION (\d+)/.exec(failure.stderr); -const noBindings = /Could not locate the bindings file/.test(failure.stderr); +const compiledFor = /NODE_MODULE_VERSION (\d+)/.exec(failure); +const noBindings = /Could not locate the bindings file/.test(failure); console.error(''); console.error('='.repeat(72)); diff --git a/test/allow-scripts.test.ts b/test/allow-scripts.test.ts index a1154d9c..7c4cc942 100644 --- a/test/allow-scripts.test.ts +++ b/test/allow-scripts.test.ts @@ -32,6 +32,7 @@ describe('package.json allowScripts (npm 12 install-script gating, #162)', () => expect(pkg.dependencies['better-sqlite3']).toBe('^13.0.3'); expect(postinstall).not.toContain("npm(['rebuild', 'better-sqlite3']"); expect(postinstall).not.toContain("npm(['run', 'build-release']"); + expect(postinstall).not.toContain('failure.stderr'); }); it('explicitly denies sharp\'s postinstall (#102)', () => { diff --git a/test/postinstall-native-check.test.ts b/test/postinstall-native-check.test.ts new file mode 100644 index 00000000..9718c173 --- /dev/null +++ b/test/postinstall-native-check.test.ts @@ -0,0 +1,47 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { copyFileSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { spawnSync } from 'node:child_process'; + +const REPO_ROOT = join(import.meta.dirname, '..'); + +describe('postinstall native-stack verification', () => { + let testDir: string | undefined; + + afterEach(() => { + if (testDir) rmSync(testDir, { recursive: true, force: true }); + }); + + it('reports an ABI mismatch without attempting a source rebuild', () => { + testDir = mkdtempSync(join(tmpdir(), 'episodic-memory-postinstall-')); + const scriptsDir = join(testDir, 'scripts'); + const packageDir = join(testDir, 'node_modules', 'better-sqlite3'); + mkdirSync(scriptsDir, { recursive: true }); + mkdirSync(packageDir, { recursive: true }); + copyFileSync(join(REPO_ROOT, 'scripts', 'postinstall.js'), join(scriptsDir, 'postinstall.js')); + writeFileSync(join(testDir, 'package.json'), JSON.stringify({ type: 'module' })); + writeFileSync( + join(packageDir, 'package.json'), + JSON.stringify({ name: 'better-sqlite3', version: '13.0.3', main: 'index.js' }) + ); + writeFileSync( + join(packageDir, 'index.js'), + `module.exports = class Database { + constructor() { + throw new Error('compiled using NODE_MODULE_VERSION 147; this Node requires NODE_MODULE_VERSION 137'); + } + };` + ); + + const result = spawnSync(process.execPath, [join(scriptsDir, 'postinstall.js')], { + encoding: 'utf-8', + }); + + expect(result.status).toBe(1); + expect(result.stderr).toContain('NATIVE BINDING IS BROKEN'); + expect(result.stderr).toContain('binary built for : NODE_MODULE_VERSION 147'); + expect(result.stderr).not.toContain('TypeError'); + expect(result.stderr).not.toContain('build-release'); + }); +});