From 01c991dc7e323e1325bfef371a8944c2034637b7 Mon Sep 17 00:00:00 2001 From: epsilonode <40526619+epsilonode@users.noreply.github.com> Date: Thu, 6 Aug 2026 09:41:12 +0900 Subject: [PATCH] feat(db): add node sqlite adapter parity (#8871) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validated in local merge-train (devbox-vm-06-dev002) @ combined-tip (FAST gates green: static + changed tests + vitest — only pre-existing audit.test.ts flake). Evidence: /home/diegosouzapw/dev/proxys/OmniRoute/.claude/worktrees/merge-train-20260805-213228-suite.log --- .../8870-node-sqlite-adapter-parity.md | 1 + src/lib/db/adapters/nodeSqliteShared.ts | 39 ++++++- tests/unit/db-adapters/driverFactory.test.ts | 100 +++++++++++++++++- .../unit/db-adapters/nodeSqliteShared.test.ts | 40 +++++++ 4 files changed, 176 insertions(+), 4 deletions(-) create mode 100644 changelog.d/features/8870-node-sqlite-adapter-parity.md diff --git a/changelog.d/features/8870-node-sqlite-adapter-parity.md b/changelog.d/features/8870-node-sqlite-adapter-parity.md new file mode 100644 index 0000000000..b912939d2e --- /dev/null +++ b/changelog.d/features/8870-node-sqlite-adapter-parity.md @@ -0,0 +1 @@ +- **Database**: The `node:sqlite` fallback now uses SQLite's native backup API and real immediate write transactions, improving backup consistency and concurrent-write behavior when `better-sqlite3` is unavailable diff --git a/src/lib/db/adapters/nodeSqliteShared.ts b/src/lib/db/adapters/nodeSqliteShared.ts index e301318f15..93b0811440 100644 --- a/src/lib/db/adapters/nodeSqliteShared.ts +++ b/src/lib/db/adapters/nodeSqliteShared.ts @@ -19,6 +19,7 @@ export function createNodeSqliteAdapterFromDatabase( onClose?: () => void ): SqliteAdapter { let _isOpen = true; + let transactionDepth = 0; type NodeSqliteStatement = ReturnType; interface CachedStatement { stmt: NodeSqliteStatement; @@ -69,6 +70,27 @@ export function createNodeSqliteAdapterFromDatabase( } } + function runImmediate(fn: () => void): void { + if (transactionDepth > 0) { + runSavepoint(fn); + return; + } + + db.exec("BEGIN IMMEDIATE"); + transactionDepth += 1; + try { + fn(); + db.exec("COMMIT"); + } catch (error) { + try { + db.exec("ROLLBACK"); + } catch {} // The failed transaction may already have released its write lock. + throw error; + } finally { + transactionDepth -= 1; + } + } + function close() { try { onClose?.(); @@ -124,12 +146,25 @@ export function createNodeSqliteAdapterFromDatabase( return db.prepare(sql).all(); }, transaction(fn: (...args: unknown[]) => T): (...args: unknown[]) => T { - return (...args: unknown[]) => runSavepoint(fn, ...args); + return (...args: unknown[]) => { + transactionDepth += 1; + try { + return runSavepoint(fn, ...args); + } finally { + transactionDepth -= 1; + } + }; }, immediate(fn: () => void): void { - runSavepoint(() => fn()); + runImmediate(fn); }, async backup(destination: string): Promise { + const { backup } = await import("node:sqlite"); + if (typeof backup === "function") { + await backup(db as never, destination); + return; + } + try { db.exec("PRAGMA wal_checkpoint(TRUNCATE)"); } catch {} diff --git a/tests/unit/db-adapters/driverFactory.test.ts b/tests/unit/db-adapters/driverFactory.test.ts index 1750ba52ff..5727eb0fd0 100644 --- a/tests/unit/db-adapters/driverFactory.test.ts +++ b/tests/unit/db-adapters/driverFactory.test.ts @@ -1,4 +1,4 @@ -import { test, describe } from "node:test"; +import { test, describe, type TestContext } from "node:test"; import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; @@ -26,7 +26,7 @@ function forceNodeSqlite() { }); } -function createTempDatabasePath(t: Parameters[1]) { +function createTempDatabasePath(t: TestContext) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-node-sqlite-")); const databasePath = path.join(dir, "database.sqlite"); t.after(() => fs.rmSync(dir, { recursive: true, force: true })); @@ -134,6 +134,7 @@ describe("driverFactory", () => { DatabaseSync: new (filePath: string) => { close(): void; exec(sql: string): void; + prepare(sql: string): { get(): unknown }; }; }; const seed = new DatabaseSync(databasePath); @@ -167,6 +168,101 @@ describe("driverFactory", () => { }); adapter.close(); }); + + test("forced node:sqlite backup preserves WAL-backed data", async (t) => { + const sourcePath = createTempDatabasePath(t); + const destinationPath = path.join(path.dirname(sourcePath), "backup.sqlite"); + const openNodeSqlite = forceNodeSqlite(); + const source = openNodeSqlite(sourcePath); + assert.ok(source); + assert.equal(source.driver, "node:sqlite"); + source.exec("PRAGMA journal_mode = WAL; CREATE TABLE items (value TEXT);"); + source.prepare("INSERT INTO items VALUES (?)").run("backup value"); + + await source.backup(destinationPath); + source.close(); + + const destination = openNodeSqlite(destinationPath, { fileMustExist: true }); + assert.ok(destination); + assert.equal(destination.driver, "node:sqlite"); + assert.equal( + (destination.prepare("SELECT value FROM items").get() as { value: string }).value, + "backup value" + ); + destination.close(); + }); + + test("forced node:sqlite immediate commits, rolls back, and nests savepoints", (t) => { + const databasePath = createTempDatabasePath(t); + const adapter = forceNodeSqlite()(databasePath); + assert.ok(adapter); + assert.equal(adapter.driver, "node:sqlite"); + adapter.exec("CREATE TABLE items (value TEXT)"); + + adapter.immediate(() => { + adapter.prepare("INSERT INTO items VALUES (?)").run("committed"); + }); + assert.throws(() => + adapter.immediate(() => { + adapter.prepare("INSERT INTO items VALUES (?)").run("rolled back"); + throw new Error("rollback"); + }) + ); + adapter.immediate(() => { + adapter.prepare("INSERT INTO items VALUES (?)").run("outer before"); + const nested = adapter.transaction(() => { + adapter.prepare("INSERT INTO items VALUES (?)").run("inner rolled back"); + throw new Error("nested rollback"); + }); + assert.throws(() => nested()); + adapter.prepare("INSERT INTO items VALUES (?)").run("outer after"); + }); + + const rows = adapter.prepare("SELECT value FROM items ORDER BY rowid").all() as Array<{ + value: string; + }>; + assert.deepEqual( + rows.map((row) => row.value), + ["committed", "outer before", "outer after"] + ); + adapter.close(); + }); + + test("forced node:sqlite immediate blocks a competing writer", (t) => { + const databasePath = createTempDatabasePath(t); + const openNodeSqlite = forceNodeSqlite(); + const first = openNodeSqlite(databasePath); + assert.ok(first); + first.exec("CREATE TABLE items (value TEXT)"); + + const { DatabaseSync } = require("node:sqlite") as { + DatabaseSync: new ( + filePath: string, + options: { timeout: number } + ) => { + close(): void; + exec(sql: string): void; + }; + }; + const second = new DatabaseSync(databasePath, { timeout: 50 }); + try { + first.immediate(() => { + assert.throws(() => second.exec("INSERT INTO items VALUES ('competing writer')"), { + code: "ERR_SQLITE_ERROR", + }); + first.prepare("INSERT INTO items VALUES (?)").run("owner"); + }); + + const rows = first.prepare("SELECT value FROM items").all() as Array<{ value: string }>; + assert.deepEqual( + rows.map((row) => row.value), + ["owner"] + ); + } finally { + second.close(); + first.close(); + } + }); } test("retains the existing cascade when native drivers are unavailable", () => { diff --git a/tests/unit/db-adapters/nodeSqliteShared.test.ts b/tests/unit/db-adapters/nodeSqliteShared.test.ts index fe9d653e75..22c1bc7bc2 100644 --- a/tests/unit/db-adapters/nodeSqliteShared.test.ts +++ b/tests/unit/db-adapters/nodeSqliteShared.test.ts @@ -80,6 +80,46 @@ test("createNodeSqliteAdapterFromDatabase uses savepoints for transactions", () assert.equal(db.execCalls[1].startsWith("RELEASE "), true); }); +test("createNodeSqliteAdapterFromDatabase uses BEGIN IMMEDIATE outside transactions", () => { + const db = new FakeDb(); + const adapter = createNodeSqliteAdapterFromDatabase(db, ":memory:"); + + adapter.immediate(() => {}); + + assert.deepEqual(db.execCalls, ["BEGIN IMMEDIATE", "COMMIT"]); +}); + +test("createNodeSqliteAdapterFromDatabase rolls back failed immediate transactions", () => { + const db = new FakeDb(); + const adapter = createNodeSqliteAdapterFromDatabase(db, ":memory:"); + + assert.throws(() => + adapter.immediate(() => { + throw new Error("fail"); + }) + ); + + assert.deepEqual(db.execCalls, ["BEGIN IMMEDIATE", "ROLLBACK"]); +}); + +test("createNodeSqliteAdapterFromDatabase nests transactions in immediate savepoints", () => { + const db = new FakeDb(); + const adapter = createNodeSqliteAdapterFromDatabase(db, ":memory:"); + + adapter.immediate(() => { + const nested = adapter.transaction(() => { + throw new Error("inner failure"); + }); + assert.throws(() => nested()); + }); + + assert.equal(db.execCalls[0], "BEGIN IMMEDIATE"); + assert.match(db.execCalls[1], /^SAVEPOINT /); + assert.match(db.execCalls[2], /^ROLLBACK TO /); + assert.match(db.execCalls[3], /^RELEASE /); + assert.equal(db.execCalls[4], "COMMIT"); +}); + test("createNodeSqliteAdapterFromDatabase finalizes cached statements on close", () => { const db = new FakeDb(); let closedHookCalls = 0;