From 284500193b7544af4a770edef980101be2422aa2 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Wed, 15 Jul 2026 23:30:13 -0700 Subject: [PATCH] fix(natives): resolve better-sqlite3 by fresh fs walk, not cached resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ak sync` intermittently ended with a false "[natives] 1/1 agentdb location(s) on WASM fallback (data-loss writes)" even though the heal step had just reported the location native — and a fresh process confirmed it native. Root cause: bsq3Root() used createRequire().resolve(), whose process-wide resolution cache (Module._pathCache/_realpathCache) goes stale after an in-process `npm install` (the aidefence heal / a version upgrade) dedupes the tree. The cached root pointed at a nested better-sqlite3 that npm had just removed, so the binding check on that dead path failed — a false negative in sync's convergence proof. agentdbLocations() reads with plain fs and was already correct; only the resolver-backed lookup lied. Fix: bsq3Root() now walks up the node_modules chain with fs.existsSync on every call — the node-resolution equivalent, but reading disk fresh, immune to in-process tree mutation. Same semantics for the resolvable/native/WASM cases (existing tests unchanged); adds coverage for hoisted resolution and the nested→hoisted mid-process move that triggered the bug. --- src/lib/natives.mjs | 23 ++++++++++++++--------- tests/kit/natives.test.mjs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/lib/natives.mjs b/src/lib/natives.mjs index d2b7d52e..37764f8c 100644 --- a/src/lib/natives.mjs +++ b/src/lib/natives.mjs @@ -5,7 +5,6 @@ // `security defend` needs (dropped from the 3.28 tree — ruvnet/ruflo#2670). import fs from 'node:fs'; import path from 'node:path'; -import { createRequire } from 'node:module'; import { rufloNodeModules, aqeRoot } from './paths.mjs'; /** agentdb locations under the global ruflo tree (mirrors ruflo-patch-native). */ @@ -16,15 +15,21 @@ export function agentdbLocations() { .filter((p) => fs.existsSync(p)); } -/** Package root of better-sqlite3 as resolved from `fromDir` (real Node - * resolution), or null if not resolvable. */ +/** Package root of better-sqlite3 as resolved from `fromDir`, or null if not + * found. Walks up the node_modules chain reading disk fresh on every call — + * the node resolution equivalent, but WITHOUT createRequire().resolve(), whose + * process-wide cache (Module._pathCache/_realpathCache) goes stale after an + * in-process `npm install` reshapes the tree. That staleness made `sync`'s + * final convergence proof report a false WASM fallback on a location the + * earlier heal (and a fresh process) both saw as native. */ export function bsq3Root(fromDir) { - try { - const req = createRequire(path.join(fromDir, 'noop.js')); - const entry = req.resolve('better-sqlite3'); // …/better-sqlite3/lib/index.js - return path.join(entry.slice(0, entry.lastIndexOf(`${path.sep}better-sqlite3${path.sep}`)), 'better-sqlite3'); - } catch { - return null; + let dir = path.resolve(fromDir); + for (;;) { + const cand = path.join(dir, 'node_modules', 'better-sqlite3'); + if (fs.existsSync(path.join(cand, 'package.json'))) return cand; + const parent = path.dirname(dir); + if (parent === dir) return null; // reached filesystem root + dir = parent; } } diff --git a/tests/kit/natives.test.mjs b/tests/kit/natives.test.mjs index 6bd6fcac..1538fffe 100644 --- a/tests/kit/natives.test.mjs +++ b/tests/kit/natives.test.mjs @@ -45,3 +45,35 @@ test('bsq3IsNative is false for a WASM-fallback install (no binding file)', () = assert.equal(bsq3IsNative(dir), false); fs.rmSync(dir, { recursive: true, force: true }); }); + +test('bsq3Root finds better-sqlite3 hoisted up the node_modules chain', () => { + // Real layout: ruflo/node_modules/agentdb resolves better-sqlite3 from the + // hoisted ruflo/node_modules/better-sqlite3 — walk up, don't only look nested. + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-natives-hoist-')); + const fromDir = path.join(root, 'node_modules', 'agentdb'); + const hoisted = path.join(root, 'node_modules', 'better-sqlite3'); + fs.mkdirSync(fromDir, { recursive: true }); + fs.mkdirSync(hoisted, { recursive: true }); + fs.writeFileSync(path.join(hoisted, 'package.json'), JSON.stringify({ name: 'better-sqlite3' })); + assert.equal(bsq3Root(fromDir), hoisted); + fs.rmSync(root, { recursive: true, force: true }); +}); + +test('bsq3Root follows a nested→hoisted move in-process (sync convergence bug)', () => { + // The false-negative: healNatives resolves better-sqlite3 to a NESTED copy, + // then an in-process `npm install` (aidefence) dedupes it away, leaving only a + // HOISTED copy. The final proof must resolve to the hoisted copy — reading disk + // fresh — not a stale cached root pointing at the removed nested path. + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-natives-move-')); + const fromDir = path.join(root, 'node_modules', 'agentdb'); + const nested = path.join(fromDir, 'node_modules', 'better-sqlite3'); + const hoisted = path.join(root, 'node_modules', 'better-sqlite3'); + for (const p of [nested, hoisted]) { + fs.mkdirSync(p, { recursive: true }); + fs.writeFileSync(path.join(p, 'package.json'), JSON.stringify({ name: 'better-sqlite3' })); + } + assert.equal(bsq3Root(fromDir), nested, 'prefers the nested copy while it exists'); + fs.rmSync(nested, { recursive: true, force: true }); // simulate npm dedupe mid-sync + assert.equal(bsq3Root(fromDir), hoisted, 'resolves to the hoisted copy after the move'); + fs.rmSync(root, { recursive: true, force: true }); +});