Fleabag515 added a commit to Fleabag515/Pleiades that referenced this pull request
Blocker found while scoping Phase 2 of the vendoring plan (cross-platform native-dep packaging, docs/specs/2026-07-25-anamnesis-vendoring-and- installer-plan.md): better-sqlite3 has no prebuilt binary for any currently-supported Node version. Verified directly against WiseLibs/better-sqlite3's GitHub releases -- v9.6.0 (the pinned version) tops out at node-v120 (Node 21); v11.x/v12.x never got past node-v115 (Node 20); v13.0.1 (latest) dropped prebuilt distribution entirely, install script is now bare `node-gyp rebuild`. Every currently-supported Node LTS (22, 24) has zero prebuild coverage -- confirmed why `npm install` on this machine fell back to a genuine local node-gyp compile (no prebuilds/ dir, build/Release/obj.target/ intermediate files present, live daemon process had build/Release/better_sqlite3.node mapped, not a prebuilds/ path). That's the one dependency standing between "download the installer, done" and "needs a C++ toolchain" for anyone who isn't on this exact dev machine -- unacceptable for the vendoring effort's own goal. Considered and rejected: self-building+vendoring better-sqlite3's binary per platform (keeps the per-ABI treadmill forever, adds ongoing release-cutting burden); libsql (better-sqlite3-compatible API, upstream-published ABI-stable binary, but it's a SQLite fork -- worse bet for a data-critical store than paying a small migration cost to stay on stock SQLite). node:sqlite ships inside Node itself (RC stability as of v25.7.0/v24.15.0, unflagged since v22.13.0/v23.4.0 -- nodejs/node#61262), so pinning the bundled Node runtime (Phase 2b) permanently ends the native-addon-per- platform problem for this dependency. Migration, verified end to end: - New src/lib/sqlite-tx.js: runInTransaction(db, fn) -- node:sqlite's DatabaseSync has no .transaction() sugar. BEGIN/fn()/COMMIT, rollback + rethrow on any throw. Replaces the 3 real .transaction() call sites (history.js:352 updateDecayScores, history.js:564 mergeSessions -- atomically repoints session_key across 5 tables, the corruption-critical one -- and importers/index.js's bulk import). - .pragma('x = y') -> .exec('PRAGMA x = y') at all 4 call sites (history.js x2, cli.js, importers/index.js). - Two DatabaseSync constructor defaults explicitly overridden everywhere a connection opens, because getting either wrong changes real behavior silently: enableForeignKeyConstraints:false (node defaults this true; better-sqlite3 left it off matching SQLite's own default, and nothing here has ever run `PRAGMA foreign_keys` -- the schema's `ON DELETE CASCADE` refs have been inert in production the whole time. Migrating onto node's default would make prune() start cascade-deleting a pruned turn's engrams/foresights, a real change to what memory survives a prune, not something to flip on as a migration side effect) and timeout:5000 + a matching `PRAGMA busy_timeout=5000` belt-and- suspenders (node defaults busy-timeout to 0 -- immediate SQLITE_BUSY on any lock contention -- vs better-sqlite3's 5000ms default; cli.js opens its own separate handle for repair commands like reembed/status while the daemon may be writing, so cross-process WAL contention is real; independently confirmed via nodejs/node#57597, a user hitting exactly this "database is locked" default). - readonly:true -> readOnly:true (cli.js's status-check handle) -- verified the correct camelCase option name rather than assuming better-sqlite3's own casing carried over. - Verified everything empirically against real Node 22.22.2 before editing any source: .run() returns {lastInsertRowid, changes} matching better- sqlite3's shape; BLOB columns round-trip as Uint8Array (history.js's toFloat32() already decodes via .buffer/.byteOffset/.byteLength, which Buffer and Uint8Array both expose -- no change needed there); a runInTransaction() probe commits on success and rolls back cleanly with zero partial rows on a thrown error; readOnly:true genuinely rejects writes. - 2 new targeted tests in test/history.test.js: mergeSessions rolls back atomically on a mid-merge throw (drops character_observations mid-loop to force one), and commits all 5 tables together on success. Replaced a 3rd planned test (cross-handle busy-timeout wait via a same-thread setTimeout racing a blocking DatabaseSync call) after recognizing it would deadlock the single JS thread instead of testing anything -- kept a simpler, correct assertion that busy_timeout is actually configured to 5000 instead. - Removed the test suite's better-sqlite3-native-binding-unavailable skip guard (test/history.test.js) -- node:sqlite can't fail to load the way a native addon could, the guard no longer has a reason to exist. - package.json: removed the now-unused better-sqlite3 dependency (30 packages dropped from node_modules on reinstall; the real size -- 1.6GB -- is unchanged, that's node-llama-cpp's 6 platform variants + onnxruntime-node, untouched by this change and still Phase 2's actual remaining size problem). Full existing suite (213 tests) + the 3 new ones: 216/216 pass. Also booted the real daemon.js end-to-end against an isolated scratch HOME (separate control port, no contact with production data) and confirmed a clean health-check response with zero errors beyond the expected ExperimentalWarning. Advisory research for this decision done via a single Agent(model:"opus") consult per the current council-agent-choice policy; every specific claim it made (file:line citations, node:sqlite default values, ABI/version history) was independently re-verified against the real source and Node's own docs/issue tracker before acting on it, not taken on faith.