Skip to content

fix(db): preserve readonly and existing-file semantics in node:sqlite fallback - #8724

Merged
diegosouzapw merged 4 commits into
diegosouzapw:release/v3.8.49from
epsilonode:fix/db-native-sqlite-fallback
Jul 27, 2026
Merged

diegosouzapw merged 4 commits into
diegosouzapw:release/v3.8.49from
epsilonode:fix/db-native-sqlite-fallback

Conversation

@epsilonode

@epsilonode epsilonode commented Jul 26, 2026 •

Copy link
Copy Markdown
Contributor

PR 1 of issue #8707 (staged native SQLite baseline)

OmniRoute prefers better-sqlite3, then falls back to Node's built-in node:sqlite when the native addon cannot load. That fallback opened DatabaseSync(filePath) without preserving supported adapter open semantics.

This PR addresses the fallback safety gap implicated by:

Specifically, readonly was not translated to Node's readOnly option, and fileMustExist had no Node equivalent, so a missing persistent path could be created rather than rejected.

What This PR Establishes

  • better-sqlite3 remains the preferred driver when available.
  • A real forced node:sqlite fallback can create, reopen, and query a writable temporary database.
  • A missing readonly + fileMustExist path creates no database, WAL, or SHM artifacts.
  • An existing read-only database remains readable and rejects writes.
  • The raw native handle retains disabled extension loading.
  • Bun behavior is unchanged: bun:sqlite remains preferred under Bun.

The test seam only makes better-sqlite3 unavailable. It does not mock or replace node:sqlite, patch the global module loader, or expose a public test-only API.

Runtime-Specific Tests

Bun does not expose node:sqlite, so Node-only fallback tests are registered only under Node. Bun still runs the shared driver-factory checks; Node still runs the full fallback suite. This avoids reporting no-op Node tests as Bun passes.

Validation

  • Node 24 driver-factory suite: 15/15.
  • Bun DB compatibility suite: 14/14 genuinely executed tests.
  • Full lint and core typecheck.
  • check:any-budget:t11, check:tracked-artifacts, and check:changelog-integrity.

Fork CI is diagnostic-only for this branch. This PR does not claim a green full suite, build, coverage, quality-gate, or Sonar result.

Related Issues And Boundaries

This PR does not close:

It reduces one avoidable unsafe fallback transition. It does not claim to repair Electron packaging, persisted packaged startup, permissions failures, or sql.js memory behavior.

It also excludes native backup changes, transaction changes, vector loading, dependency changes, runtime repair, and any driver-preference change.

Follow-up Stack

After this PR merges, PR 2 can prove native backup and transaction parity: immediate acquisition, nested savepoints, commit/rollback behavior, contention, and old-runtime degradation, while retaining better-sqlite3 preference.

Only after PR 2 merges should PR 3 address trusted sqlite-vec lifecycle and two-cycle persisted Windows Electron package proof. Each PR requires its own evidence and a fresh branch from the current release line.

@epsilonode
epsilonode requested a review from diegosouzapw as a code owner July 26, 2026 21:36
@epsilonode epsilonode changed the title fix(db): preserve node sqlite open semantics fix(db): preserve readonly and existing-file semantics in node:sqlite fallback Jul 26, 2026
@diegosouzapw
diegosouzapw merged commit 296765f into diegosouzapw:release/v3.8.49 Jul 27, 2026
3 checks passed
diegosouzapw added a commit that referenced this pull request Jul 28, 2026
…e lost credits

Aggregates every pending changelog.d fragment into the [3.8.49] section and
regenerates the contributors table from the reconciled bullets.

Three fixes this surfaced:

- The [3.8.49] section had no `### 📝 Maintenance` heading, so the aggregator's
  findIndex matched the first one in the file — inside [3.8.47] — and would have
  filed 92 maintenance bullets under the wrong release. Added the heading to the
  living section; [3.8.47] stays at its original 234 bullets.

- 46 bullets carried no PR/issue reference. Fragments may keep the number only in
  the filename (`<N>-slug.md`), which the aggregator does not copy into the bullet,
  so the link and the credit were dropped on aggregation. Restored, scoped strictly
  to the [3.8.49] range.

- 9 external contributors lost their attribution that way and are credited again:
  @MisileLab (#8566), @MumuTW (#8619), @epsilonode (#8724), @hppsc1215 (#8835),
  @sumanxg (#8837, #8856), @TitoTFP (#8838), @HouMinXi (#8842, #8845).

Contributors table: 84 → 155 entries, no one removed. 42 i18n mirrors synced.
check:changelog-integrity green — no base bullet lost.
@diegosouzapw diegosouzapw mentioned this pull request Jul 28, 2026
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
… fallback (diegosouzapw#8724)

* fix(db): preserve node sqlite open semantics

* test(db): keep node sqlite coverage off Bun

* chore(changelog): number native sqlite fallback

* test(db): register node sqlite tests only under Node
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
… fallback (diegosouzapw#8724)

* fix(db): preserve node sqlite open semantics

* test(db): keep node sqlite coverage off Bun

* chore(changelog): number native sqlite fallback

* test(db): register node sqlite tests only under Node
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…e lost credits

Aggregates every pending changelog.d fragment into the [3.8.49] section and
regenerates the contributors table from the reconciled bullets.

Three fixes this surfaced:

- The [3.8.49] section had no `### 📝 Maintenance` heading, so the aggregator's
  findIndex matched the first one in the file — inside [3.8.47] — and would have
  filed 92 maintenance bullets under the wrong release. Added the heading to the
  living section; [3.8.47] stays at its original 234 bullets.

- 46 bullets carried no PR/issue reference. Fragments may keep the number only in
  the filename (`<N>-slug.md`), which the aggregator does not copy into the bullet,
  so the link and the credit were dropped on aggregation. Restored, scoped strictly
  to the [3.8.49] range.

- 9 external contributors lost their attribution that way and are credited again:
  @MisileLab (diegosouzapw#8566), @MumuTW (diegosouzapw#8619), @epsilonode (diegosouzapw#8724), @hppsc1215 (diegosouzapw#8835),
  @sumanxg (diegosouzapw#8837, diegosouzapw#8856), @TitoTFP (diegosouzapw#8838), @HouMinXi (diegosouzapw#8842, diegosouzapw#8845).

Contributors table: 84 → 155 entries, no one removed. 42 i18n mirrors synced.
check:changelog-integrity green — no base bullet lost.
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