From 8c9122a8b41517b18a92b4efd83d77d5d3bf8bb9 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Fri, 17 Jul 2026 07:06:33 -0700 Subject: [PATCH] fix(natives): heal better-sqlite3 in place so npm can't prune the heal away MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ak sync` reported "natives: native installed" and then failed its own convergence proof with "1/1 agentdb location(s) on WASM fallback". Both reports were true, 30 seconds apart. The ruflo upgrade leaves the shared ruflo/node_modules/better-sqlite3 half-built (sqlite3.a, no .node). healNatives then installed better-sqlite3 INTO the agentdb location with --no-save, which produced a working local copy and verified true — honest at that instant. But nothing in the tree declares that copy, so when healAidefence ran `npm install` into the ruflo root, npm reconciled the tree and pruned it as extraneous. Resolution fell back to the half-built shared copy → WASM fallback. (The same prune also removed agentic-flow/node_modules/agentdb, which is why the count read 1/1 where sync had just patched two locations.) Heal the copy resolution ALREADY finds, in place, via the package's own `prebuild-install || node-gyp rebuild` — `npm run` is user-invoked and never gated by npm >=11.17 allow-scripts. Installing a copy is now the last resort, only when better-sqlite3 is not resolvable at all, so the heal lands on the declared copy and survives any later reconciliation. Also: - Move the natives step after aidefence/aqe-solver in sync, so nothing reshapes the tree after the heal. Defense in depth: with the in-place fix, ordering alone no longer decides correctness. Runs on `security` too — an aidefence install can wipe the binding even when the plan never flagged natives. - Raise the build timeout 300s → 600s. A node-gyp sqlite3 compile is slow and a half-built build/ dir is what a truncated one leaves behind. - Inject `runner` into ensureNativeBsq3 so the ladder is testable without npm or a network. Verified by reproducing the original broken state (half-built shared copy + aidefence absent): same 1/1 WASM fallback, then `ak sync` converges, the binding lands on the shared declared copy, survives a subsequent tree-reconciling `npm install`, and a second sync is a clean no-op. The regression test simulates npm's prune-the-undeclared behavior and fails on the old strategy with "a heal that a later npm install prunes away is not a heal". --- src/commands/sync.mjs | 14 +++- src/lib/heal.mjs | 60 +++++++++------ tests/kit/heal-natives.test.mjs | 131 ++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+), 26 deletions(-) create mode 100644 tests/kit/heal-natives.test.mjs diff --git a/src/commands/sync.mjs b/src/commands/sync.mjs index 3c50eccb..8187a59e 100644 --- a/src/commands/sync.mjs +++ b/src/commands/sync.mjs @@ -61,9 +61,6 @@ export async function run({ flags, pkgRoot }) { if (d.outdated || !d.installed) report(`upgrade ${d.pkg}`, await heal.upgradePackage(d.pkg)); } } - if (subsystems.has('natives') || subsystems.has('versions')) { - report('natives', await heal.healNatives()); - } // ruvnet-brain: install if absent / re-run installer to pull latest when // drifted (force bypasses the installer's skip-if-present). Not an npm pkg, so // it rides its own branch rather than the driftReport loop above. @@ -74,6 +71,17 @@ export async function run({ flags, pkgRoot }) { report('aidefence', await heal.healAidefence()); report('aqe solver', await heal.healAqeSolver()); } + // natives LAST among the npm-tree mutations. Every agentdb location resolves up + // to the single shared ruflo/node_modules/better-sqlite3, so any later `npm + // install` into the ruflo/aqe root re-resolves that copy and drops the freshly + // built binding — project-scoped installs can't pass --allow-scripts, so the + // build script never re-runs and a half-built build/ dir (obj/, sqlite3.a, no + // .node) is left behind. Healing here means nothing reshapes the tree after us. + // Runs on `security` too: an aidefence install wipes the binding even when the + // plan never flagged natives. + if (subsystems.has('natives') || subsystems.has('versions') || subsystems.has('security')) { + report('natives', await heal.healNatives()); + } if (subsystems.has('aqe')) { report('rvf', heal.healRvf(paths.projectAqeDir(cwd))); } diff --git a/src/lib/heal.mjs b/src/lib/heal.mjs index ddff5a0a..51296349 100644 --- a/src/lib/heal.mjs +++ b/src/lib/heal.mjs @@ -24,36 +24,50 @@ const ALLOW_SCRIPTS = [ // npm >=11.17) — it is a global-install flag only. Plain installs still get // native better-sqlite3 because 12.x resolves a usable prebuilt without a // lifecycle script (verified live 2026-07-14). -async function npmInstallInto(dir, spec) { - return run('npm', ['install', spec, '--no-save', '--no-audit', '--no-fund'], +async function npmInstallInto(dir, spec, runner = run) { + return runner('npm', ['install', spec, '--no-save', '--no-audit', '--no-fund'], { cwd: dir, timeout: 300_000 }); } const failTail = (r) => `FAILED (${(r.stderr || `exit ${r.code}`).trim().split('\n').slice(-2).join(' ').slice(0, 200)})`; -/** Deterministic native better-sqlite3 for one location — an escalation - * ladder, verifying after each rung, stopping at the first binding: - * 1. plain install — enough on npm <11.17, or when npm's content store - * already holds a built copy of the exact version (why plain installs - * look like they "work": it depends on cache history, not on scripts). - * 2. npm approve-scripts + rebuild — npm ≥11.17's sanctioned path for the - * blocked install script (approve-scripts pins the exact version into - * the location's package.json; harmless no-op failure on older npm). - * rebuild also recovers a stale half-built build/ dir. - * 3. run the package's own install script directly — explicit `npm run` - * is user-invoked and never gated by allow-scripts. */ -export async function ensureNativeBsq3(dir) { - await npmInstallInto(dir, 'better-sqlite3@^12'); - if (bsq3IsNative(dir)) return { ok: true, how: 'native installed' }; - await run('npm', ['approve-scripts', 'better-sqlite3'], { cwd: dir, timeout: 60_000 }); - let r = await run('npm', ['rebuild', 'better-sqlite3'], { cwd: dir, timeout: 300_000 }); - if (bsq3IsNative(dir)) return { ok: true, how: 'native rebuilt (scripts approved)' }; - const pkgRoot = bsq3Root(dir); - if (pkgRoot) { - r = await run('npm', ['run', 'install'], { cwd: pkgRoot, timeout: 300_000 }); - if (bsq3IsNative(dir)) return { ok: true, how: 'native built via package install script' }; +/** Deterministic native better-sqlite3 for one location. + * + * Heals the copy that node resolution ALREADY finds, IN PLACE. Installing + * better-sqlite3 into `dir` itself is the last resort, not the first rung: + * a `--no-save` install plants a copy no package.json in the tree declares, + * and the next `npm install` into the ruflo root (healAidefence's, say) + * reconciles the tree and PRUNES it as extraneous — silently reverting the + * heal to the shared, half-built copy underneath. That is exactly how a sync + * reported "native installed" and then failed its own convergence proof with + * a WASM fallback: both reports were true, 30 seconds apart. + * + * Ladder, verifying after each rung, stopping at the first binding: + * 1. the resolved package's own install script — `prebuild-install || + * node-gyp rebuild`, which fetches a prebuilt when one exists. Explicit + * `npm run` is user-invoked and never gated by npm >=11.17 allow-scripts, + * and it recovers a stale half-built build/ dir. + * 2. npm approve-scripts + rebuild — npm >=11.17's sanctioned path for the + * blocked install script (harmless no-op failure on older npm). + * 3. install a copy into `dir` — only when better-sqlite3 is not resolvable + * from `dir` at all, so there is nothing in place to build. + * + * `runner` is injectable so the ladder is testable without npm or a network. */ +export async function ensureNativeBsq3(dir, { runner = run } = {}) { + let pkgRoot = bsq3Root(dir); + if (!pkgRoot) { + await npmInstallInto(dir, 'better-sqlite3@^12', runner); + if (bsq3IsNative(dir)) return { ok: true, how: 'native installed' }; + pkgRoot = bsq3Root(dir); + if (!pkgRoot) return { ok: false, how: 'FAILED (better-sqlite3 not resolvable)' }; } + // node-gyp compiling sqlite3 from source is slow; 300s truncated it mid-build. + await runner('npm', ['run', 'install'], { cwd: pkgRoot, timeout: 600_000 }); + if (bsq3IsNative(dir)) return { ok: true, how: 'native built in place' }; + await runner('npm', ['approve-scripts', 'better-sqlite3'], { cwd: pkgRoot, timeout: 60_000 }); + const r = await runner('npm', ['rebuild', 'better-sqlite3'], { cwd: pkgRoot, timeout: 600_000 }); + if (bsq3IsNative(dir)) return { ok: true, how: 'native rebuilt (scripts approved)' }; return { ok: false, how: failTail(r) }; } diff --git a/tests/kit/heal-natives.test.mjs b/tests/kit/heal-natives.test.mjs new file mode 100644 index 00000000..def8ecbb --- /dev/null +++ b/tests/kit/heal-natives.test.mjs @@ -0,0 +1,131 @@ +// ensureNativeBsq3 — the natives heal ladder. Uses a synthetic node_modules +// fixture and an injected runner that simulates npm, so the test is hermetic +// (no npm, no network, no global tree) and runs on the full CI matrix. +// +// The regression under test (the sync that reported "native installed" and then +// failed its own convergence proof with a WASM fallback, 30s apart — both true): +// healing by installing better-sqlite3 INTO the agentdb location plants a copy +// no package.json declares. The next `npm install` into the ruflo root reconciles +// the tree, PRUNES that copy as extraneous, and resolution silently falls back to +// the unbuilt copy underneath. Heal must survive a reconciliation to be a heal. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { ensureNativeBsq3 } from '../../src/lib/heal.mjs'; +import { bsq3IsNative } from '../../src/lib/natives.mjs'; + +const BINDING = path.join('build', 'Release', 'better_sqlite3.node'); + +function writePkg(dir, { withBinding = false } = {}) { + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'package.json'), + JSON.stringify({ name: 'better-sqlite3', version: '12.0.0' })); + if (withBinding) addBinding(dir); +} + +function addBinding(pkgDir) { + fs.mkdirSync(path.join(pkgDir, 'build', 'Release'), { recursive: true }); + fs.writeFileSync(path.join(pkgDir, BINDING), ''); +} + +/** ruflo/node_modules/{agentdb, better-sqlite3} — better-sqlite3 hoisted and + * declared, but unbuilt: the state a `ruflo@latest` upgrade leaves behind. */ +function makeTree() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-heal-')); + const agentdb = path.join(root, 'node_modules', 'agentdb'); + const shared = path.join(root, 'node_modules', 'better-sqlite3'); + fs.mkdirSync(agentdb, { recursive: true }); + writePkg(shared); + return { root, agentdb, shared, cleanup: () => fs.rmSync(root, { recursive: true, force: true }) }; +} + +/** A runner that fakes just enough npm: `npm run install` builds the binding in + * cwd (prebuild-install succeeding), `npm install better-sqlite3` plants a + * built copy under cwd/node_modules (the old rung 1). Records every call. */ +function fakeNpm({ nested = null } = {}) { + const calls = []; + const runner = async (cmd, args, opts) => { + calls.push({ cmd, args, cwd: opts?.cwd }); + if (args[0] === 'run' && args[1] === 'install') addBinding(opts.cwd); + if (args[0] === 'install' && String(args[1]).startsWith('better-sqlite3')) { + writePkg(nested ?? path.join(opts.cwd, 'node_modules', 'better-sqlite3'), { withBinding: true }); + } + return { code: 0, stdout: '', stderr: '' }; + }; + return { runner, calls }; +} + +/** What `npm install ` into the ruflo root does to the tree: removes + * packages no package.json declares. The heal's `--no-save` copy is exactly that. */ +function reconcileTree(root) { + for (const extraneous of [path.join(root, 'node_modules', 'agentdb', 'node_modules')]) { + fs.rmSync(extraneous, { recursive: true, force: true }); + } +} + +test('heal survives an npm tree reconciliation (the sync convergence regression)', async () => { + const { root, agentdb, cleanup } = makeTree(); + const { runner } = fakeNpm(); + + const r = await ensureNativeBsq3(agentdb, { runner }); + assert.equal(r.ok, true, 'heal reports success'); + assert.equal(bsq3IsNative(agentdb), true, 'native immediately after the heal'); + + reconcileTree(root); // a later `npm install` into the ruflo root (healAidefence's) + + assert.equal(bsq3IsNative(agentdb), true, + 'STILL native after reconciliation — a heal that a later npm install prunes away is not a heal'); + cleanup(); +}); + +test('heal builds the resolved copy in place, planting no extraneous copy', async () => { + const { agentdb, shared, cleanup } = makeTree(); + const { runner, calls } = fakeNpm(); + + await ensureNativeBsq3(agentdb, { runner }); + + assert.equal(fs.existsSync(path.join(shared, BINDING)), true, 'binding built on the declared copy'); + assert.equal(fs.existsSync(path.join(agentdb, 'node_modules', 'better-sqlite3')), false, + 'no extraneous copy planted under agentdb — npm would prune it as undeclared'); + assert.equal(calls.some((c) => c.args[0] === 'install'), false, + 'never installs a copy when better-sqlite3 already resolves'); + cleanup(); +}); + +test('heal installs a copy only when better-sqlite3 is not resolvable at all', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-heal-bare-')); + const agentdb = path.join(root, 'node_modules', 'agentdb'); + fs.mkdirSync(agentdb, { recursive: true }); + const { runner, calls } = fakeNpm(); + + const r = await ensureNativeBsq3(agentdb, { runner }); + + assert.equal(r.ok, true); + assert.equal(calls[0].args[0], 'install', 'falls back to installing a copy — nothing in place to build'); + assert.equal(bsq3IsNative(agentdb), true); + fs.rmSync(root, { recursive: true, force: true }); +}); + +test('heal reports failure honestly when no rung produces a binding', async () => { + const { agentdb, cleanup } = makeTree(); + const runner = async () => ({ code: 1, stdout: '', stderr: 'node-gyp: build error\n' }); + + const r = await ensureNativeBsq3(agentdb, { runner }); + + assert.equal(r.ok, false, 'never claims success without the binding on disk'); + assert.match(r.how, /FAILED/); + cleanup(); +}); + +test('heal reports failure when better-sqlite3 stays unresolvable', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-heal-noop-')); + const runner = async () => ({ code: 0, stdout: '', stderr: '' }); // install produces nothing + + const r = await ensureNativeBsq3(root, { runner }); + + assert.equal(r.ok, false); + assert.match(r.how, /not resolvable/); + fs.rmSync(root, { recursive: true, force: true }); +});