Skip to content

Add native SQLite record adapter (node:sqlite, FTS5, WAL) — #46 native adapter#85

Merged
cuibonobo merged 2 commits into
mainfrom
claude/issue-45-design-review-1jgkcj
Jul 11, 2026
Merged

Add native SQLite record adapter (node:sqlite, FTS5, WAL) — #46 native adapter#85
cuibonobo merged 2 commits into
mainfrom
claude/issue-45-design-review-1jgkcj

Conversation

@cuibonobo

Copy link
Copy Markdown
Member

Summary

Builds the native adapter piece of #46: a new @haverstack/record-adapter-sqlite package on node:sqlite (Node ≥ 22.5, no native compilation), plus the shared-layer/core plumbing it needs. Scoped to just this package per discussion — adapter-local's swap to it, record-adapter-sqljs's browser-only cleanup, and doc updates are left for a follow-up pass.

record-adapter-sqlite:

  • NativeSQLiteRecordAdapter implements StackRecordAdapter directly against node:sqlite's DatabaseSync. No export()/rewrite-the-whole-file step at all — native SQLite writes straight to the file under WAL journaling, so page-level writes, crash safety, and file locking come from the engine itself.
  • FTS5 full-text search, with its own sanitizer (sanitizeFts5Query) reviewed against real FTS5 grammar rather than assumed from FTS4: NEAR is a function-call form here, not FTS4's infix operator, and FTS5 has real column-filter syntax that errors hard on unknown columns — both handled so they don't become new query-time failures.
  • Opening a stack file created by record-adapter-sqljs transparently migrates its FTS4 index to FTS5 (detected via sqlite_master, rebuilt once) — cross-compatibility verified against a real sql.js-written file.
  • FK enforcement (PRAGMA foreign_keys = ON, mapped to StackNotFoundError) and the same PID-lock-file storage-ownership guard as record-adapter-sqljs, now shared via sqlite-shared.
  • deleteUnreferencedAttachmentRecords implemented as a real SQL transaction, matching record-adapter-sqljs's existing fix for the same check-then-act race.
  • NativeTokenStore: a separate class/file implementing core's new StackTokenStore, storing bearer tokens outside the portable stack file by default (defaultTokenStorePath() derives a sibling path) — the fix requested in Split SQLite support: native record-adapter-sqlite for Node, browser-only record-adapter-sqljs, and a StackTokenStore interface in core #46's polish-backlog comment.

Two things I ran into and fixed along the way:

  1. A real correctness bug: FTS5 external-content tables require the special INSERT INTO fts(fts, rowid, content) VALUES('delete', ...) command to unindex a row — a plain DELETE FROM records_fts leaves stale, still-searchable entries behind. Caught via direct testing against node:sqlite before it shipped; there's a regression test for it (full-text search reflects patchContent updates).
  2. A Vitest/Vite bug where node:sqlite resolution drops the node: prefix and fails (vitest-dev/vitest#7177) — worked around by loading the module via process.getBuiltinModule() instead of a static import. That's the Node-sanctioned mechanism for exactly this class of bundler incompatibility, so it protects consumers who build with Vite-based tooling too, not just our own test suite.

sqlite-shared:

  • New: FTS5_SCHEMA_SQL, sanitizeFts5Query, PRAGMA_JOURNAL_MODE_WAL, TOKENS_SCHEMA_SQL split out of RECORD_SCHEMA_SQL (tokens now live in their own schema fragment so each adapter can place them in whatever file it wants).
  • The storage-ownership lock (acquireLock/releaseLock) moves here from record-adapter-sqljs — always pure Node fs/process logic, not sql.js-specific, so both engines share one implementation and one error message now.

core:

  • New StackTokenStore interface + TokenInfo type: bearer-token issuance/lookup for server implementations, deliberately not part of the StackRecordAdapter contract (neither Stack nor StackClient touches it) and deliberately decoupled from record storage.

record-adapter-sqljs:

  • Consumes the shared lock module instead of its own copy; TokenInfo now sourced from core. No behavior change — its 95 tests pass unmodified.

Test plan

  • pnpm build / pnpm typecheck / pnpm lint — full monorepo, clean
  • pnpm test — full monorepo, 582 tests passing, no regressions
  • New record-adapter-sqlite tests (71): CRUD, queries incl. FTS5 search, associations, versions, permissions, restoreVersion, commitMigration, deleteUnreferencedAttachmentRecords, WAL mode enabled, FK enforcement, storage lock (reject/reclaim/force/release), FTS4→FTS5 migration (idempotent), NativeTokenStore (separate file, expiry, revoke, persistence across instances)
  • New sqlite-shared tests: sanitizeFts5Query (NEAR term-preservation, column-filter stripping, parity with FTS4 sanitizer's known limitations), lock module (reject/reclaim/force/release) direct unit tests
  • Manual verification scripts (not committed) confirmed: node:sqlite JSON1/FTS5/WAL/FK support, cross-file-compatibility between sql.js and node:sqlite, and the FTS5 delete-command bug before writing the fix

Refs #46.

https://claude.ai/code/session_01RRLiGUZC344E3pgNEGAe5W


Generated by Claude Code

claude added 2 commits July 11, 2026 18:49
First real deliverable of #46's native-adapter work item: a new
@haverstack/record-adapter-sqlite package built on node:sqlite (Node
>= 22.5, no native compilation), plus the shared-layer/core plumbing
it needs. adapter-local's swap to it, sqljs's browser-only cleanup,
and doc updates are left for a follow-up pass.

record-adapter-sqlite:
- NativeSQLiteRecordAdapter implements StackRecordAdapter directly
  against node:sqlite's DatabaseSync — no export()/rewrite-the-whole-
  file step at all, since native SQLite writes straight to the file
  under WAL journaling (real page-level writes, crash safety, and
  file locking for free).
- FTS5 full-text search, with its own sanitizer (sanitizeFts5Query)
  reviewed against real FTS5 grammar rather than assumed from FTS4:
  NEAR is a function-call form here, not FTS4's infix operator, and
  FTS5 has real column-filter syntax that errors hard on unknown
  columns — both handled to avoid new query-time failures.
- Opening a stack file created by record-adapter-sqljs transparently
  migrates its FTS4 index to FTS5 (detected via sqlite_master, rebuilt
  once) — cross-compatibility verified against real sql.js-written
  files.
- FK enforcement (PRAGMA foreign_keys = ON, mapped to
  StackNotFoundError) and the same PID-lock-file storage-ownership
  guard as record-adapter-sqljs, now shared via sqlite-shared.
- deleteUnreferencedAttachmentRecords implemented as a real SQL
  transaction, matching record-adapter-sqljs's fix for the same race.
- NativeTokenStore: a separate class/file implementing core's new
  StackTokenStore, storing bearer tokens outside the portable stack
  file by default (defaultTokenStorePath() derives a sibling path) —
  the fix requested in #46's polish-backlog comment.

Along the way:
- Found and fixed a real correctness bug: FTS5 external-content
  tables require the special `('delete', rowid, oldContent)` command
  to unindex a row — a plain `DELETE FROM records_fts` leaves stale,
  still-searchable entries behind. Caught via direct testing against
  node:sqlite before it became a shipped bug; regression-tested.
- Worked around a Vitest/Vite bug where "node:sqlite" resolution
  drops the "node:" prefix and fails (vitest-dev/vitest#7177) by
  loading the module via process.getBuiltinModule() instead of a
  static import — the Node-sanctioned mechanism for exactly this class
  of bundler incompatibility, so it also protects consumers who build
  with Vite-based tooling.

sqlite-shared:
- New: FTS5_SCHEMA_SQL, sanitizeFts5Query, PRAGMA_JOURNAL_MODE_WAL,
  TOKENS_SCHEMA_SQL split out of RECORD_SCHEMA_SQL (tokens now live in
  their own schema fragment so each adapter can place them in whatever
  file it wants).
- The storage-ownership lock (acquireLock/releaseLock) moves here from
  record-adapter-sqljs — it was always pure Node fs/process logic, not
  sql.js-specific, so both engines now share one implementation and
  one error message.

core:
- New StackTokenStore interface + TokenInfo type: bearer-token
  issuance/lookup for server implementations, deliberately not part of
  the StackRecordAdapter contract (neither Stack nor StackClient
  touches it) and deliberately decoupled from record storage.

record-adapter-sqljs:
- Consumes the shared lock module instead of its own copy; TokenInfo
  now sourced from core; no behavior change (95 tests still pass
  unmodified).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RRLiGUZC344E3pgNEGAe5W
Follow-up to feedback on the native-adapter PR: record-adapter-sqljs
and record-adapter-sqlite had near-identical method bodies (same SQL
text, same parameter order) for every CRUD/query/version/type/
association/token operation, differing only in each engine's call
convention (sql.js's array-bind-then-step vs. node:sqlite's
spread-args run/get/all). That's now unified.

sqlite-shared adds:
- SqlExecutor: a small interface (exec/run/get/all) normalizing the
  two conventions, plus isForeignKeyViolation (verified identical
  error message on both engines already, just written twice before).
- FtsStrategy: the one place FTS4 and FTS5 genuinely differ (FTS5's
  external-content table needs its special 'delete' command with the
  *old* content to unindex a row; FTS4's plain DELETE is fine either
  way). fts4Strategy/fts5Strategy are the concrete strategies.
- SharedSqlRecordLogic: every StackRecordAdapter method (createRecord,
  getRecord, patchContent, deleteRecord, associate/dissociate,
  versions, types, deleteUnreferencedAttachmentRecords, ...), written
  once against a SqlExecutor + FtsStrategy instead of twice against
  two different database APIs.
- SharedTokenLogic: same treatment for StackTokenStore
  (createToken/lookupToken/listTokens/revokeToken).
- insertConfigRecord/readStackConfig: the _config@1 record read/write,
  also identical across engines modulo the call convention.

Both adapters now implement a thin per-engine SqlExecutor
(SqlJsExecutor, NativeSqliteExecutor) and construct
SharedSqlRecordLogic/SharedTokenLogic with it, an FTS strategy, and an
onWrite hook (sql.js's export+rename persist(); omitted for native,
whose writes are already durable via WAL). What's left in each
package's index.ts is genuinely engine-specific: WASM init vs.
DatabaseSync construction, pragma setup, the FTS4->FTS5 migration, and
lifecycle (flush/close).

Net effect: record-adapter-sqljs's index.ts drops from 676 to 373
lines; record-adapter-sqlite's from 614 to 310 (+ a 23-line executor +
its token-store.ts shrinking similarly). No behavior change — every
existing test in both packages passes completely unmodified (95 +
71 = 166 tests), which is the actual proof this refactor is
behavior-preserving rather than just believed to be.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RRLiGUZC344E3pgNEGAe5W

Copy link
Copy Markdown
Member Author

Follow-up commit: consolidated the duplication between record-adapter-sqljs and record-adapter-sqlite that was raised in review — the two adapters had near-identical method bodies (same SQL, same param order) for every CRUD/query/version/type/association/token operation, differing only in each engine's call convention (sql.js's array-bind-then-step vs. node:sqlite's spread-args run/get/all).

Added to @haverstack/sqlite-shared:

  • SqlExecutor — a small interface (exec/run/get/all) normalizing the two conventions
  • FtsStrategy — the one place FTS4/FTS5 genuinely differ (fts4Strategy/fts5Strategy)
  • SharedSqlRecordLogic — every StackRecordAdapter method, written once
  • SharedTokenLogic — every StackTokenStore method, written once
  • insertConfigRecord/readStackConfig — the _config@1 record read/write

Both adapters now implement a thin per-engine SqlExecutor and delegate to the shared classes. Each package's index.ts is left with only what's genuinely engine-specific (init, pragmas, the FTS4→FTS5 migration, lifecycle).

record-adapter-sqljs's index.ts: 676 → 373 lines. record-adapter-sqlite's: 614 → 310 (+ a 23-line executor). No behavior change — every existing test in both packages (95 + 71 = 166) passes completely unmodified, which is the actual evidence this is behavior-preserving rather than just believed to be.


Generated by Claude Code

@cuibonobo cuibonobo merged commit 74d606d into main Jul 11, 2026
5 checks passed
@cuibonobo cuibonobo deleted the claude/issue-45-design-review-1jgkcj branch July 11, 2026 22:52
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.

2 participants