Skip to content

Stop a missing C++ toolchain from breaking the install - #28

Merged
MohitBansal321 merged 1 commit into
mainfrom
feat/node-sqlite-driver
Sep 3, 2026
Merged

MohitBansal321 merged 1 commit into
mainfrom
feat/node-sqlite-driver

Conversation

@MohitBansal321

Copy link
Copy Markdown
Owner

Closes #27

Storage now uses Node's built-in node:sqlite, which needs no compiler. better-sqlite3
moves to optionalDependencies, so a failed native build warns instead of failing the
install, and it is only ever loaded on a Node too old for the built-in.

This is the fix docs/PUBLISHING.md already recommended for this exact problem.

Verified before writing any code

Against Node's own doc/api/sqlite.md history blocks and runtime probes on a real machine —
not from memory:

Question Answer
node:sqlite unflagged since? v22.13.0 / v23.4.0 (PR 55890). Added v22.5.0 behind --experimental-sqlite; RC since v24.15.0
Did the old floor (>=22.0.0) need the flag? Yes, on 22.0–22.12
Read-only connections? Yes — { readOnly: true }, since v22.5.0. Writes throw
WAL-mode reads? Yes
iterate()? v22.13.0 / v23.4.0 — the same releases that unflagged the module

iterate() landing in exactly the release that removed the flag is why the floor is one
boundary rather than two, and why the capability probe in the driver tests for iterate()
rather than sniffing versions.

Live test against a real 879 MB OpenCode database (WAL, with a 30 MB -wal) — the
"don't disturb a running OpenCode" case:

YES  open readOnly                      YES  journal_mode {"journal_mode":"wal"}
YES  count messages {"c":7791}          YES  iterate
YES  writes rejected (OpenCode undisturbed)

.db and -wal byte-identical afterwards; only -shm's mtime moved, which is SQLite's
shared-memory coordination and is what better-sqlite3 does too.

Design

Everything goes through openSqlite() in the new src/db/driver.ts — no other module
imports a SQLite driver directly
, which is what keeps the fallback honest. Recorded as an
invariant in CLAUDE.md.

node:sqlite has no .pragma() or .transaction(), so the driver supplies both. Transactions
nest via savepoints, matching better-sqlite3, because migrate() runs each migration inside a
transaction and an inner BEGIN would otherwise throw.

changes is coerced to a number: node:sqlite can return a bigint there, and the repositories
do arithmetic on it.

Invariants

verify through both drivers — identical figures, zero delta, against the same real
usage.db:

AI_USAGE_SQLITE_DRIVER=node:sqlite      -> 3x MATCH, RESULT: every client reconciles exactly
AI_USAGE_SQLITE_DRIVER=better-sqlite3   -> 3x MATCH, RESULT: every client reconciles exactly

File format untouched, no migration. The existing 12,599-record database — written by
better-sqlite3 — reads correctly under node:sqlite: schema_migrations intact,
journal_mode=wal, real rows readable. Header is plain SQLite format 3, page size 4096,
because the format is SQLite's, not the binding's.

npm run check passes: 151 tests (143 + 8 new). parity.test.ts untouched and passing.

The plan's "Done when": a clean install with no build step

npm install <tarball> --ignore-scripts   -> exit 0, no native binding built, SQLite driver: node:sqlite
npm install <tarball> --omit=optional    -> exit 0, better-sqlite3 absent entirely, SQLite driver: node:sqlite

Full MCP handshake in the --omit=optional install: initialize OK, tools/list → 7 tools,
tools/call usage_summary OK. stdout stayed clean JSON-RPC; the ExperimentalWarning goes to
stderr, as it must.

One difference deliberately not papered over

node:sqlite returns rows with a null prototype; better-sqlite3 returns ordinary objects.
Property access, spreading and JSON.stringify are identical, but row instanceof Object is
false and row.hasOwnProperty is undefined. Nothing reads rows that way today (audited), so
rather than wrap every row and pay for it, the driver documents the constraint and a test pins
the difference.

Notes on scope

  • engines.node moves >=22.0.0 → >=22.13.0. Phase 5 said not to touch the floor, but
    the stated "Done when" requires node:sqlite to be present, and below 22.13.0 it isn't —
    the native module would be mandatory again. Still "22+", just patch-precise. Flagging it
    because it narrows support to 22.13+ within the Node 22 LTS line.
  • engines.npm dropped, and CI no longer upgrades the runner's npm — so an npm 10 Windows
    install is now a genuine regression test rather than a configuration we avoid.
  • AI_USAGE_SQLITE_DRIVER forces either driver. Not in the plan; added because otherwise
    the fallback is untestable on a Node that doesn't need it, and it makes a suspected driver
    difference diagnosable.
  • ai-usage status now prints SQLite driver:. With two possible drivers, which one
    produced the numbers is provenance, and this project already reports schema version and
    pricing provenance there.
  • Windows npm notes in the README were reworded to apply to the fallback, not deleted.

`better-sqlite3` is a native addon. On Windows with npm 10 the install died
inside `node-gyp` -- and because that happens during `npx`, the user never
reached the README section explaining the fix. `engines.npm >= 11` existed
only to force an npm that avoided the bug.

Storage now uses Node's built-in `node:sqlite`, which needs no compiler.
`better-sqlite3` moves to `optionalDependencies`, so npm reports a build
failure and carries on instead of failing the install, and it is only ever
loaded on a Node too old for the built-in.

Everything goes through `openSqlite()` in the new `src/db/driver.ts`; no
other module may import a SQLite driver directly. `node:sqlite` has no
`.pragma()` or `.transaction()`, so the driver supplies both, nesting
transactions via savepoints the way better-sqlite3 does.

The floor moves to Node >=22.13.0, which is where `node:sqlite` came out
from behind `--experimental-sqlite` and where `StatementSync.iterate()`
landed -- the same release, so it is one boundary rather than two. Below it
the native module would be mandatory again, defeating the point.
`engines.npm` is gone, and CI no longer upgrades the runner's npm, which
turns an npm 10 Windows install into a real regression test.

Verified: the file format is untouched, so an existing usage.db keeps
working with no migration -- a 12,599-record database written by
better-sqlite3 reads correctly, and `verify` reports a zero delta through
either driver with byte-identical figures. A clean install with
`--ignore-scripts` and with `--omit=optional` both produce a working
server.

One difference is deliberately not papered over: `node:sqlite` returns
rows with a null prototype. Property access, spreading and JSON are
identical, but inherited methods are not there. Nothing reads rows that
way today; the driver documents it and a test pins it.

`AI_USAGE_SQLITE_DRIVER` forces either driver, which is how the fallback
stays tested on a Node that does not need it, and `ai-usage status` now
reports which driver produced the numbers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@MohitBansal321 MohitBansal321 added the bug Something isn't working label Sep 2, 2026
@MohitBansal321 MohitBansal321 mentioned this pull request Sep 3, 2026
@MohitBansal321
MohitBansal321 merged commit 833d745 into main Sep 3, 2026
9 checks passed
@MohitBansal321
MohitBansal321 deleted the feat/node-sqlite-driver branch September 3, 2026 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Install fails with node-gyp errors before the user can read the fix

1 participant