Skip to content

fix: load bun:sqlite at runtime with node:sqlite fallback - #347

Open
thatdudealso wants to merge 3 commits into
superagent-ai:mainfrom
thatdudealso:fm/grok-sqlite-compat-p1
Open

fix: load bun:sqlite at runtime with node:sqlite fallback#347
thatdudealso wants to merge 3 commits into
superagent-ai:mainfrom
thatdudealso:fm/grok-sqlite-compat-p1

Conversation

@thatdudealso

@thatdudealso thatdudealso commented Jul 22, 2026

Copy link
Copy Markdown

Summary

  • src/storage/db.ts statically imported bun:sqlite, which only resolves under the Bun runtime. This blocked running storage tests under plain Node (e.g. node node_modules/.bin/vitest run), since bunx vitest run itself still executes under Bun.
  • Loads bun:sqlite lazily via createRequire and falls back to Node's built-in node:sqlite (DatabaseSync) when Bun isn't available. Both backends satisfy the same internal Database shape, so the public SQLiteDatabase interface and all callers are unchanged.
  • Adds src/storage/db.test.ts, a regression test that initializes the database and round-trips a write/read (including inside withTransaction), proving the storage layer now works under Node/Vitest.

Fixes #346

Test plan

  • bun run typecheck
  • bun run lint
  • bun run format
  • bunx vitest run (Bun runtime) — 250/250 tests pass
  • node node_modules/.bin/vitest run (plain Node runtime) — 250/250 tests pass, confirming the node:sqlite fallback path works end-to-end

Note

Medium Risk
Touches core persistence and parameter binding on experimental node:sqlite; behavior must stay aligned with Bun’s strict mode for all existing callers.

Overview
Storage SQLite no longer statically imports bun:sqlite, so Vitest and other tooling can run under plain Node. db.ts loads Bun’s driver via createRequire when available and otherwise uses node:sqlite (DatabaseSync) behind the same internal Database shape, leaving the public SQLiteDatabase API unchanged.

The Node path enables bare named parameter binding ({ id }@id) via setAllowBareNamedParameters(true) and implements transactions with explicit BEGIN/COMMIT/ROLLBACK. New src/storage/db.test.ts covers init/read-write, named params, the Node-only spy on setAllowBareNamedParameters, and withTransaction. AGENTS.md documents the dual-runtime behavior and test commands; CLAUDE.md points agents at AGENTS.md.

Reviewed by Cursor Bugbot for commit e221bce. Configure here.

Vitest runs the storage layer under Node, but src/storage/db.ts had
a static `import { Database } from "bun:sqlite"`, which Node cannot
resolve. Load bun:sqlite via createRequire at runtime and fall back
to node:sqlite's DatabaseSync when Bun is unavailable, so the
storage tests can run under Node/Vitest as well as Bun.
Add a self-governance section and note that src/storage/db.ts now
supports Node via a node:sqlite fallback, and that bunx vitest still
runs under Bun so the Node path requires an explicit node invocation.
@open-cla

open-cla Bot commented Jul 22, 2026

Copy link
Copy Markdown

Contributor License Agreement

All contributors are covered by a CLA.

Comment thread src/storage/db.ts
The node:sqlite DatabaseSync path prepared statements without calling
setAllowBareNamedParameters(true), so callers binding objects like
{ id } against @id placeholders (as workspaces.ts and sessions.ts do)
could fail to bind under Node, unlike Bun's strict: true mode. Add a
regression test that exercises the exact fallback path and verifies
both persistence and that the API is actually invoked.
@thatdudealso

Copy link
Copy Markdown
Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit e221bce. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Storage layer cannot run under Node/Vitest due to static bun:sqlite import

1 participant