Skip to content

fix: lazy-load sqlite bindings and add bun:sqlite support#1428

Open
brittianwarner wants to merge 1 commit intomainfrom
fix/lazy-sqlite-bun-compat
Open

fix: lazy-load sqlite bindings and add bun:sqlite support#1428
brittianwarner wants to merge 1 commit intomainfrom
fix/lazy-sqlite-bun-compat

Conversation

@brittianwarner
Copy link
Copy Markdown

Summary

  • Removes the eager top-level require("node:sqlite") in sqlite-bindings.ts that crashed on any runtime without node:sqlite (including Bun)
  • Lazy-loads the SQLite module on first AgentOs.create() call with promise-cached loading
  • Adds a full bun:sqlite adapter (BunDatabaseAdapter/BunStatementAdapter) that normalizes API differences to match the node:sqlite shape
  • Auto-selects the right backend via runtime detection (process.versions.bun)

Related: rivet-dev/rivet#4550 — lazy-loads agent-os-core on the RivetKit side to prevent the same crash at the actor layer.

Changes

packages/core/src/sqlite-bindings.ts — Full refactor

  • Removed the eager top-level require("node:sqlite") (the crash root cause)
  • Added SqliteDatabase, SqliteStatement, SqliteModule interfaces for runtime-agnostic abstraction
  • Added BunDatabaseAdapter / BunStatementAdapter with full API mapping:
    • db.location() maps from db.filename (normalizes :memory: to null)
    • stmt.run() passes through { changes, lastInsertRowid } directly
    • stmt.get() normalizes Bun's null-for-no-rows to undefined
    • stmt.iterate() uses native iterate() when available, falls back to all()
    • stmt.columns() maps columnNames string[] to [{ name }] objects
    • stmt.finalize() calls through to release native resources
    • setReadBigInts / setAllowBareNamedParameters / setAllowUnknownNamedParameters are documented no-ops
    • constants forwarded from bun:sqlite (e.g. SQLITE_FCNTL_PERSIST_WAL)
  • Promise-cached module loading prevents race conditions on concurrent calls
  • Explicit unsupported-runtime error for non-Node/non-Bun environments
  • Improved error messages with running version, minimum requirements, open handle diagnostics, and contextual hints (ENOENT, EACCES, corrupt DB)
  • JSDoc on all exported functions

packages/core/src/agent-os.ts — One line

  • bindings: createSqliteBindings(kernel) to bindings: await createSqliteBindings(kernel) (already inside static async create())

packages/core/src/index.ts — New exports

  • Exported SqliteModule, SqliteDatabase, SqliteStatement types for custom backends
  • Exported createSqliteBindings and createSqliteBindingsFromModule

packages/core/tests/sqlite-bindings.test.ts — 55 tests across 6 suites

  • Lazy loading (no eager import, caching, reset, concurrent safety)
  • Runtime detection
  • Bun adapter surface via mock (20 tests)
  • Binding tree lifecycle, error diagnostics, VFS sync (20 tests)
  • Real node:sqlite integration (CRUD, bigint, Uint8Array, transactions, VFS sync)
  • Public API entry point (createSqliteBindings)

Documentation

  • CLAUDE.md: Added SQLite runtime requirement to Dependencies, Bun adapter parity gaps to Known VM Limitations
  • .agent/todo/remove-sqlite-bindings.md: Updated with current state and known adapter limitations
  • .agent/notes/vm-friction.md: Created per project policy

Impact

  • Bun users: @rivet-dev/agent-os-core imports cleanly. SQLite features work via bun:sqlite.
  • Node users: Zero behavior change. node:sqlite loads lazily instead of eagerly.
  • Other runtimes: Import succeeds. Clear error on AgentOs.create() if SQLite is needed.

Testing

  • 55 new tests, 251+ total across 9 test files, 0 failures
  • Two rounds of multi-agent code review (correctness, Bun compat, test quality, DX)
  • Verified against latest bun:sqlite spec
  • Zero new type errors vs baseline

Remove the eager top-level require('node:sqlite') in sqlite-bindings.ts
that crashed on any runtime without node:sqlite (including Bun). The
module now lazy-loads on first AgentOs.create() and auto-selects the
right backend via runtime detection (process.versions.bun).

bun:sqlite adapter:
- BunDatabaseAdapter/BunStatementAdapter normalize API differences
- db.location() maps from db.filename, normalizes :memory: to null
- stmt.run() passes through { changes, lastInsertRowid } directly
- stmt.get() normalizes Bun's null-for-no-rows to undefined
- stmt.iterate() uses native iterate() when available
- stmt.columns() maps columnNames string[] to [{ name }] objects
- stmt.finalize() calls through to release native resources
- setReadBigInts/setAllowBareNamedParameters are documented no-ops
- constants forwarded from bun:sqlite (e.g. SQLITE_FCNTL_PERSIST_WAL)

Loading and caching:
- Promise-cached module loading prevents race conditions
- Explicit unsupported-runtime error path (not just Node fallthrough)
- createSqliteBindings() is now async (call site already in async create())

Error messages improved:
- Include running runtime version and minimum requirements
- Database/statement handle errors list open handles and common causes
- Database open errors include contextual hints (ENOENT, EACCES, corrupt)

API surface:
- Exported SqliteModule/SqliteDatabase/SqliteStatement types from index.ts
- Exported createSqliteBindingsFromModule for custom backends and testing
- JSDoc on all exported functions

Documentation:
- CLAUDE.md: added SQLite runtime requirement and Bun parity gaps
- .agent/todo/remove-sqlite-bindings.md: updated with current state
- .agent/notes/vm-friction.md: created per project policy

Tests: 55 tests across 6 suites covering lazy loading, runtime detection,
Bun adapter (mock), binding tree lifecycle, error diagnostics, VFS sync
behavior, real node:sqlite integration, and the public API entry point.
Copy link
Copy Markdown
Author

#4550

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.

1 participant