fix(sqlite): correct node:sqlite driver semantics, pin the Bun toolchain, and cover the seam with tests

[sroussey]

Stacked on #710 — this PR targets claude/better-sqlite3-node-sqlite-0ydg8g, not main. Review #710 first; merge this into it (or land them together).

Every finding below was reproduced by running the driver before fixing it, and each fix is pinned by a test that fails without it.

What was wrong

transaction() silently committed an async body

better-sqlite3 threw TypeError("Transaction function cannot return a promise"). The replacement ignored the body's return value, so:

db.transaction(async () => {
  insert(1);
  await sleep(10);
  throw new Error("late");
})();

returned without throwing, the INSERT was already COMMITted, and the throw became an unhandled rejection with nothing left to roll back. TypeScript does not catch it: async () => {} is assignable to (...args: T) => void.

Both in-repo hand-rolled transaction paths document that they avoid the wrapper because it rejects async bodies — SqliteTabularStorage.withTransaction and SqliteMigrationRunner — so the guarantee they rely on was gone.

assertSyncTransactionBody restores the TypeError on the BEGIN path and in #runInSavepoint. It attaches a no-op catch to the abandoned promise first: without that, the body's own eventual rejection surfaces as an unhandled rejection on top of the TypeError and takes the process down.

Transaction nesting was detected by regex

#execTx was set only when the whole exec() string matched a BEGIN regex. Verified failures:

• db.exec("BEGIN;") (trailing semicolon) then db.transaction(fn)() → throws "cannot start a transaction within a transaction"; better-sqlite3 opened a SAVEPOINT.
• exec("BEGIN; …; COMMIT;") and prepare("BEGIN").run() were invisible the same way.
• Inverse desync: the reset branch sat after the rethrow, so a COMMIT that threw left #execTx stuck true.

SQLite already answers this: DatabaseSync.isTransaction. Deleted BEGIN_RE, END_RE, #execTx, #txDepth, the #inTransaction getter, the tracking block in exec() and the close() resets. #savepointSeq stays — a monotonic name can never collide with a savepoint left open by a failed RELEASE. The constructor throws if the runtime predates the getter.

Foreign keys silently flipped ON — behavior change for external consumers

node:sqlite defaults enableForeignKeyConstraints to true; better-sqlite3 and bun:sqlite both left SQLite's own default OFF. Defaulted back to false, exposed on NodeSqliteOptions as documented opt-in.

Every schema in libs and in the three consumers (sec, builder, embarc-data) was created and mutated under FK-off semantics on both prior drivers, so existing DB files legitimately contain dangling references and delete orders that were legal when they were written. Turning enforcement on as a side effect of a driver swap converts historical data into runtime SQLITE_CONSTRAINT_FOREIGNKEY failures at write time in production — not at open time, where a doc note would have helped.

better-sqlite3 options were silently dropped — behavior change for external consumers

readonly → readOnly was renamed; fileMustExist / verbose / nativeBinding have no equivalent; and ...options was spread into DatabaseSync, which ignores unknown keys. better-sqlite3 threw TypeError('Misspelled option …'). Verified: {readonly: true, fileMustExist: true} opened the database read-write and created the missing file, then successfully CREATE TABLEd.

assertKnownOptions now throws a TypeError for each legacy name (with migration guidance) and for anything outside the supported set. All 21 in-repo call sites are zero- or one-arg, so this is safe in-tree; it is a breaking change for an external consumer still passing better-sqlite3 option names — which is the point, since those options were being ignored.

Rows were null-prototype and escaped as entities

node:sqlite builds rows with Object.create(null); narrowRow mutated in place and the storage layer handed the same object back. Verified: Object.getPrototypeOf(await storage.get(key)) was null, entity.hasOwnProperty("x") threw, and toStrictEqual failed — diverging SQLite from bun:sqlite, browser-WASM and Postgres, with no assertion anywhere catching it. narrowRow now returns a plain-prototype copy; the storage layer keeps mutating the copy, so no change was needed there.

The pinned Bun toolchain could not load node:sqlite

engines.bun is ^1.4.0-canary.1 but packageManager was bun@1.3.11, which has no node:sqlite (verified: No such built-in module). bun run test drives both runners, so SQLite-backed Bun suites fail on the repo's own declared package manager; CI escaped it only because the Bun-runner jobs are commented out.

bun-version: latest was no better — verified that npm's latest (1.3.14) and its canary channel both lack the module. Nor could a concrete version be pinned: npm publishes nothing above 1.3.14, and the bun-v1.4.0 GitHub release 404s. The rolling canary asset (self-reporting 1.4.0-canary.1+52bf09cb1) is the only fetchable build with node:sqlite, so packageManager is now bun@1.4.0-canary.1 and all 17 setup-bun steps use bun-version: canary, with a comment in each workflow saying to replace it once a tagged release ships. The fixed driver was smoke-tested against that canary binary and behaves identically to Node.

Docs still described a bun:sqlite build

docs/technical/19-build-system.md and 18-multi-runtime-abstraction.md still listed @workglow/sqlite's ./storage as a --target=bun entry point. The two entries that still earn a Bun build are @workglow/util's "." and "./worker".

Also fixed (low)

• Statement.finalize() called stmt[Symbol.dispose]?.(), but StatementSync exposes no Symbol.dispose — a permanent no-op, while canonical-api.ts documented it as releasing the statement. The JSDoc now says plainly that it is a no-op on node:sqlite today.
• The driver's own exec("BEGIN" / "SAVEPOINT …" / "RELEASE …") bypassed translateError, so a SQLITE_BUSY on BEGIN surfaced as ERR_SQLITE_ERROR. They now go through an error-translating #exec helper.

Tests

packages/test/src/test/storage-tabular/SqliteDriver.contract.test.ts (new, 22 cases) drives the driver directly rather than through a storage class — async body rejected and rolled back (with an unhandledRejection listener asserting the swallowed rejection never surfaces); sync commit and sync throw; exec("BEGIN;") and prepare("BEGIN").run() each proven to yield a SAVEPOINT by rolling the outer transaction back; a failed COMMIT not wedging the next transaction; nested-throw isolation; every legacy/unknown option throwing; row prototypes; error-code translation; undefined → NULL; BigInt narrowing at the safe-integer boundary; FK default off / opt-in on.

Two cases use a file-backed database under os.tmpdir() — PRAGMA journal_mode is wal, PRAGMA busy_timeout is 5000, and a second connection's write under a held BEGIN IMMEDIATE waits out its timeout instead of failing instantly. Every existing SQLite test uses ":memory:", so that path was entirely unexercised.

11 of the 22 fail against the driver as it stood (verified by reverting node.ts and re-running).

SqliteTabularStorage.integration.test.ts gains prototype + toStrictEqual assertions on get() and on the RETURNING * update path — both fail before the fix.

SqliteAiVectorStorage.integration.test.ts's extension probe wrapped module resolution and extension load in one bare try {} catch {}, so a broken allowExtension would have skipped the suite and left CI green. It now skips only when the platform has no prebuilt binary, and rethrows when @sqliteai/sqlite-vector resolves but loadExtension fails.

Rebase prerequisite (for #710, not this PR)

#710 is 26 commits behind origin/main and should be rebased before merge. Doing so is not free:

• packages/test/src/test/util/BunExportConditions.test.ts exists on main but not on Migrate SQLite driver from better-sqlite3 to node:sqlite #710's base. It asserts a three-entry EXPECTED_BUN_CONDITIONS set, which Migrate SQLite driver from better-sqlite3 to node:sqlite #710's removal of @workglow/sqlite's ./storage bun condition breaks. The rebase needs EXPECTED_BUN_CONDITIONS updated to the two @workglow/util entries and the two sqliteExports assertions removed. That test also names the two docs files this PR edits as prose that must change with the condition set — already done here.
• main adds .github/workflows/nightly-bun-parity.yml, which has its own bun-version: latest (line 45 on main). It needs the same canary pin this PR applies to the three workflow files that exist on Migrate SQLite driver from better-sqlite3 to node:sqlite #710's base.
• main's scripts/test.ts has discovered sections and --check-sections; Migrate SQLite driver from better-sqlite3 to node:sqlite #710's base still has the hand-enumerated list (no storage-tabular section, no --check-sections). After the rebase, bun scripts/test.ts --check-sections should be run to confirm the new test file is reachable — it lives in the already-covered storage-tabular directory, so no section change is expected.

I did not rebase #710, per instruction.

Verification

Run on Node v22.22.2 (node:sqlite present, experimental warning) with bun run use-source, in an isolated worktree.

command	result
bunx vitest run …/SqliteDriver.contract.test.ts	22 passed (11 fail with node.ts reverted)
bunx vitest run …/SqliteTabularStorage.integration.test.ts	160 passed, 1 skipped (the 2 new prototype cases fail with node.ts reverted)
bunx vitest run over the 16 SQLite-touching files	262 passed, 2 skipped
bun scripts/test.ts storage vitest (63 files)	1856 passed, 25 skipped, 2 failed — all PGlite/Postgres timeouts, identical on the unmodified branch
bun scripts/test.ts util vitest (49 files)	722 passed, 10 skipped, 0 failed
bun run build:types (37 packages, incl. @workglow/sqlite and @workglow/test)	37/37 successful
bunx eslint + bunx prettier on changed files	clean
Bun canary 1.4.0-canary.1+52bf09cb1 smoke test of the driver	async body rejected, SAVEPOINT nesting, Object.prototype rows, foreign_keys = 0, legacy option rejected

Notes on the two Postgres failures: PostgresTabularStorage.integration.test.ts and PostgresTabular.smoke.test.ts time out under full-section load, and ScopedTabularStoragePostgres.integration.test.ts fails its beforeAll hook. Running those three files in isolation against the unmodified branch and against this branch gives byte-identical results (1 file failed, 2 passed, 159 passed / 3 skipped in both), so they are pre-existing PGlite flakiness, not a regression here.

bun scripts/test.ts storage-tabular vitest and --check-sections are main-only features; on #710's base the equivalents are the storage section and no --check-sections, which is what was run.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01K6huUY7hSkRbjun1P9HKsz

---

Generated by Claude Code

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	64.49%	31891 / 49448
🔵	Statements	64.29%	32995 / 51315
🔵	Functions	65.56%	6002 / 9154
🔵	Branches	53.52%	16458 / 30750

File Coverage

No changed files found.

Generated in workflow #2936 for commit f1f6b31 by the Vitest Coverage Report Action