fix(driver-sqlite-wasm): persist RETURNING writes — stop() 返回即落盘,冷启动 e2e 解封 (#4518)

.changeset/wasm-sqlite-returning-writes-persist.md

@@ -0,0 +1,44 @@
+---
+"@objectstack/driver-sqlite-wasm": patch
+---
+
+fix(driver-sqlite-wasm): a `RETURNING` write is a write — persist it (#4518)
+
+A file-backed `sqlite-wasm` database flushed its schema at boot and then
+recorded nothing else. Every table was on disk; every row written after schema
+sync lived only in the WASM heap and died with the process. Reopening the file
+found a complete, empty database.
+
+**Cause.** The Knex dialect picked its execution branch from *"does this
+statement return rows"* — and then marked the database dirty only on the other,
+row-less branch. `INSERT … RETURNING *` returns rows, so it executed on the
+row-returning branch and never set the flag. Since the `on-disconnect` flush is
+gated on the same flag, nothing rescued it afterwards either: **both** persist
+strategies dropped the write. ObjectQL writes through `RETURNING *` (it hands
+the stored row back to the caller), so this covered essentially all business
+data, along with `knex.raw('INSERT …')` and any other mutation arriving without
+a Knex `method`.
+
+**Fix.** "Does this statement change the database?" is now one exported
+predicate — `statementMutatesDatabase(sql, method)` — classifying by Knex method
+*and* SQL text, applied at a single funnel after execution. It is independent of
+which branch executed the statement, so a mutation can no longer slip through by
+returning rows, by arriving without a method, or by taking a branch that forgot
+to say so. Transaction control still routes to `noteTransactionControl`, which
+keeps deferring flushes until the transaction closes (#1494), and mutating
+`PRAGMA` assignments (`auto_vacuum`, `user_version`) now count as writes too.
+
+**What changes for you.** Nothing to author. File-backed wasm SQLite now
+actually persists under `on-write` / `debounced:*`, and `disconnect()` is a real
+durability boundary: when it returns, committed data is on disk. This is what
+`bootStack({ databaseFile })` in `@objectstack/verify` needed to make `stop()` →
+second `bootStack` a genuine cold boot — the suspended-run restart proof
+ADR-0019 promises is now asserted end to end in the dogfood gate. Expect more
+disk writes than before on a file-backed dev database, because previously there
+were almost none.
+
+**One internal signature moved.** `WasmSqliteConnection.markDirty(method?)` is
+now `markDirty()`. It used to re-filter the caller's Knex method against its own
+allowlist, which made "did this mutate?" a decision taken in two places that
+could — and did — disagree. If you call it directly, drop the argument; the
+dialect classifies, the connection obeys.

packages/plugins/driver-sqlite-wasm/src/knex-wasm-dialect.ts

@@ -92,13 +92,69 @@ function formatBindings(bindings: unknown[] | undefined): unknown[] {
  * Everything else — `select`, `first`, `pluck`, `columnInfo`, raw PRAGMA,
  * DDL with no `method` — is read with `all`/row iteration so Knex sees the
  * same response shape it would from better-sqlite3.
+ *
+ * ⚠️ This answers "how do I EXECUTE this statement", never "does this statement
+ * change the database" — an `INSERT … RETURNING *` is executed down the
+ * row-returning branch and mutates. Persistence is classified separately by
+ * {@link statementMutatesDatabase}; conflating the two is #4518.
  */
-function isReadMethod(method?: string, returning?: unknown): boolean {
+function isRowReturningExecution(method?: string, returning?: unknown): boolean {
  if (method === 'insert' || method === 'update') return !!returning ? true : false;
  if (method === 'counter' || method === 'del') return false;
  return true;
 }
 
+/** Knex `method` values that always denote a mutation. */
+const MUTATING_METHODS = new Set(['insert', 'update', 'del', 'counter']);
+
+/** Statement-control forms whose persistence is owned by the transaction lifecycle. */
+const TRANSACTION_CONTROL_RE = /^\s*(BEGIN|COMMIT|END|ROLLBACK|SAVEPOINT|RELEASE)\b/i;
+
+/**
+ * DDL / schema statements. `BEGIN…RELEASE` share this prefix set in SQLite's
+ * grammar but are transaction control, so they are matched (and routed) first.
+ */
+const DDL_RE =
+ /^\s*(CREATE|ALTER|DROP|BEGIN|COMMIT|ROLLBACK|SAVEPOINT|RELEASE|REINDEX|VACUUM|ATTACH|DETACH|TRUNCATE)\b/i;
+
+/** DML that changes rows, whatever execution branch it happens to run down. */
+const MUTATING_DML_RE = /^\s*(INSERT|UPDATE|DELETE|REPLACE|UPSERT)\b/i;
+
+/**
+ * PRAGMA forms that change bytes in the database file: any assignment
+ * (`PRAGMA auto_vacuum = INCREMENTAL`, `PRAGMA user_version = 3` — both
+ * persistent header state) and `incremental_vacuum`, which actually moves
+ * pages. Introspection PRAGMAs (`table_info`, `index_list`, …) are reads.
+ */
+const MUTATING_PRAGMA_RE = /^\s*PRAGMA\b(?:[^;]*=|\s+incremental_vacuum\b)/i;
+
+/**
+ * THE single answer to "did this statement change the database, so that the
+ * in-memory image must eventually be written back to disk?"
+ *
+ * It is deliberately independent of which execution branch {@link
+ * isRowReturningExecution} picks, because those are different questions and
+ * answering them with one predicate is what broke persistence in #4518: the
+ * ObjectQL engine writes through `INSERT … RETURNING *` / `UPDATE … RETURNING *`
+ * (it needs the stored row back), those run down the row-returning branch, and
+ * the dirty flag was only ever set on the other branch. The result was a
+ * file-backed database that flushed its schema and then silently stopped
+ * recording anything — a cold boot found every table present and every row
+ * gone, and `on-disconnect` did not save it either, because the final flush
+ * also keys off the same flag.
+ *
+ * Classifying by BOTH the Knex `method` and the SQL text means a mutation
+ * cannot slip through by arriving without a method (`knex.raw('INSERT …')`,
+ * seed/migration SQL) or by taking an unexpected branch.
+ */
+export function statementMutatesDatabase(sql: string, method?: string): boolean {
+ if (TRANSACTION_CONTROL_RE.test(sql)) return false; // owned by noteTransactionControl
+ if (method && MUTATING_METHODS.has(method)) return true;
+ if (DDL_RE.test(sql)) return true;
+ if (MUTATING_DML_RE.test(sql)) return true;
+ return MUTATING_PRAGMA_RE.test(sql);
+}
+
 /**
  * Resolve the upstream `knex/lib/dialects/sqlite3` class at runtime.
  *
@@ -179,35 +235,27 @@ export function getClient_WasmSqlite(): any {
  const db = connection.raw;
  const bindings = formatBindings(obj.bindings);
 
- // DDL / transactional control statements have no Knex `method`. sql.js's
+ // ── 1. EXECUTE ────────────────────────────────────────────────────────
+ // Three execution shapes. None of them decides persistence: that is
+ // settled once, below, so a statement cannot mutate the database on a
+ // branch that forgot to say so (#4518).
+
+ // DDL / transaction control have no Knex `method`. sql.js's
  // `prepare`+`step` silently no-ops on many of these (e.g. CREATE TABLE),
  // so route them through `run` which is implemented via `exec` and
  // actually mutates the database. PRAGMA is intentionally excluded — many
  // PRAGMA forms (e.g. `PRAGMA table_info(...)`, `foreign_key_list(...)`)
  // return rows used by Knex's schema introspection/columnInfo, and
  // `db.run` discards those rows.
- const isDdl =
- /^\s*(CREATE|ALTER|DROP|BEGIN|COMMIT|ROLLBACK|SAVEPOINT|RELEASE|REINDEX|VACUUM|ATTACH|DETACH|TRUNCATE)\b/i.test(
- obj.sql,
- );
- if (isDdl) {
+ if (DDL_RE.test(obj.sql)) {
  db.run(obj.sql, bindings as any);
  obj.response = [];
- // Transaction-control statements are routed through
- // `noteTransactionControl`, which owns flushing for the transaction
- // lifecycle: it suppresses flushes while a transaction is open (sql.js
- // `export()` closes+reopens the db, which would abort the txn) and
- // performs a single flush once the transaction fully closes. Routing
- // them away from `markDirty` avoids a second, racing flush on COMMIT.
- if (/^\s*(BEGIN|COMMIT|END|ROLLBACK|SAVEPOINT|RELEASE)\b/i.test(obj.sql)) {
- connection.noteTransactionControl(obj.sql);
- } else {
- connection.markDirty('run');
- }
- return obj;
- }
-
- if (isReadMethod(obj.method, obj.returning) || /^\s*PRAGMA\b/i.test(obj.sql)) {
+ } else if (
+ isRowReturningExecution(obj.method, obj.returning) ||
+ /^\s*PRAGMA\b/i.test(obj.sql)
+ ) {
+ // Row-returning branch. NOTE this is also where `INSERT … RETURNING *`
+ // and `UPDATE … RETURNING *` land — statements that very much write.
  const stmt = db.prepare(obj.sql);
  try {
  if (bindings.length) stmt.bind(bindings as any);
@@ -219,21 +267,34 @@ export function getClient_WasmSqlite(): any {
  } finally {
  stmt.free();
  }
- return obj;
+ } else {
+ // Row-less write path: execute via `run` and capture SQLite's
+ // per-connection lastID / changes counters.
+ db.run(obj.sql, bindings as any);
+ const changes = db.getRowsModified();
+ let lastID: number | bigint = 0;
+ if (obj.method === 'insert') {
+ const r = db.exec('SELECT last_insert_rowid() AS id');
+ lastID = (r?.[0]?.values?.[0]?.[0] as number) ?? 0;
+ }
+ obj.response = [];
+ obj.context = { lastID, changes };
  }
 
- // Write path: execute via `run` (no row iteration needed) and capture
- // SQLite's per-connection lastID / changes counters.
- db.run(obj.sql, bindings as any);
- const changes = db.getRowsModified();
- let lastID: number | bigint = 0;
- if (obj.method === 'insert') {
- const r = db.exec('SELECT last_insert_rowid() AS id');
- lastID = (r?.[0]?.values?.[0]?.[0] as number) ?? 0;
+ // ── 2. PERSIST ────────────────────────────────────────────────────────
+ // Exactly one place decides whether the on-disk image is now stale.
+ //
+ // Transaction-control statements are routed to `noteTransactionControl`,
+ // which owns flushing across the transaction lifecycle: it suppresses
+ // flushes while a transaction is open (sql.js `export()` closes+reopens
+ // the db, which would abort the txn) and performs a single flush once the
+ // transaction fully closes. Routing them away from `markDirty` avoids a
+ // second, racing flush on COMMIT.
+ if (TRANSACTION_CONTROL_RE.test(obj.sql)) {
+ connection.noteTransactionControl(obj.sql);
+ } else if (statementMutatesDatabase(obj.sql, obj.method)) {
+ connection.markDirty();
  }
- obj.response = [];
- obj.context = { lastID, changes };
- connection.markDirty(obj.method);
  return obj;
  }
  }

packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-returning-persist.test.ts

@@ -0,0 +1,161 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect, afterEach } from 'vitest';
+import { mkdtempSync, rmSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+
+import { SqliteWasmDriver } from '../src/index.js';
+import { statementMutatesDatabase } from './knex-wasm-dialect.js';
+
+/**
+ * #4518 — a mutation must mark the database dirty on EVERY execution branch.
+ *
+ * The dialect picks its execution branch from "does this statement return
+ * rows", and `INSERT … RETURNING *` returns rows — so it ran down the branch
+ * that never called `markDirty`. Since the final `on-disconnect` flush keys off
+ * the same flag, nothing rescued it either: a file-backed database flushed its
+ * schema at boot and then recorded nothing for the rest of its life. Every
+ * table present, every row missing.
+ *
+ * That is not a corner case for this stack. ObjectQL writes through
+ * `RETURNING *` because it hands the stored row back to the caller, so the
+ * defect covered essentially all business data — which is why it surfaced as
+ * "`bootStack({ databaseFile })` cannot cold-boot" rather than as a driver bug.
+ *
+ * These tests read the database back through a SECOND driver over the same
+ * file, so they assert what is on disk rather than what the live WASM heap
+ * still remembers.
+ */
+describe('SqliteWasmDriver persists mutations from every execution branch (#4518)', () => {
+ const dirs: string[] = [];
+ const drivers: SqliteWasmDriver[] = [];
+
+ function newFile(): string {
+ const dir = mkdtempSync(join(tmpdir(), 'wasm-returning-'));
+ dirs.push(dir);
+ return join(dir, 'db.sqlite');
+ }
+
+ function track(d: SqliteWasmDriver): SqliteWasmDriver {
+ drivers.push(d);
+ return d;
+ }
+
+ /** Reopen the on-disk image in a fresh driver — a cold read, in miniature. */
+ async function readBack(file: string): Promise<Record<string, any>[]> {
+ const reopened = track(new SqliteWasmDriver({ filename: file, persist: 'on-disconnect' }));
+ return (reopened as any).knex('acct').orderBy('id');
+ }
+
+ afterEach(async () => {
+ await Promise.all(drivers.splice(0).map((d) => d.disconnect().catch(() => {})));
+ for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true });
+ });
+
+ it('classifies a statement by what it DOES, not by which branch executes it', () => {
+ // The row-returning branch and the mutating set overlap; that overlap is
+ // the whole defect, so it is pinned directly.
+ expect(statementMutatesDatabase('insert into "acct" ("id") values (?) returning *', 'insert')).toBe(true);
+ expect(statementMutatesDatabase('update "acct" set "name" = ? returning *', 'update')).toBe(true);
+ expect(statementMutatesDatabase("INSERT INTO acct (id) VALUES ('x')")).toBe(true);
+ expect(statementMutatesDatabase('DELETE FROM acct WHERE id = ?')).toBe(true);
+ expect(statementMutatesDatabase('CREATE TABLE acct (id text)')).toBe(true);
+ expect(statementMutatesDatabase('PRAGMA auto_vacuum = INCREMENTAL')).toBe(true);
+
+ expect(statementMutatesDatabase('select * from "acct"', 'select')).toBe(false);
+ expect(statementMutatesDatabase('PRAGMA table_info(acct)')).toBe(false);
+ // Transaction control is owned by the transaction lifecycle, which must NOT
+ // flush mid-transaction: sql.js `export()` closes and reopens the database,
+ // aborting the open transaction (#1494).
+ expect(statementMutatesDatabase('BEGIN')).toBe(false);
+ expect(statementMutatesDatabase('COMMIT')).toBe(false);
+ expect(statementMutatesDatabase('SAVEPOINT sp1')).toBe(false);
+ });
+
+ it('flushes an `INSERT … RETURNING` under on-write — the shape ObjectQL writes with', async () => {
+ const file = newFile();
+ const driver = track(new SqliteWasmDriver({ filename: file, persist: 'on-write' }));
+ await driver.initObjects([{ name: 'acct', fields: { name: { type: 'string' } } }]);
+
+ const returned = await (driver as any)
+ .knex('acct')
+ .insert({ id: 'a1', name: 'returning-insert' })
+ .returning('*');
+ expect(returned.length).toBe(1);
+
+ // `flush()` awaits the queued write; `on-write` schedules it fire-and-forget.
+ await (driver as any).flush();
+
+ const onDisk = await readBack(file);
+ expect(onDisk.map((r) => r.id)).toEqual(['a1']);
+ expect(onDisk[0].name).toBe('returning-insert');
+ });
+
+ it('flushes an `UPDATE … RETURNING` under on-write', async () => {
+ const file = newFile();
+ const driver = track(new SqliteWasmDriver({ filename: file, persist: 'on-write' }));
+ await driver.initObjects([{ name: 'acct', fields: { name: { type: 'string' } } }]);
+ await (driver as any).knex('acct').insert({ id: 'a1', name: 'before' });
+ await (driver as any).flush();
+
+ await (driver as any).knex('acct').where('id', 'a1').update({ name: 'after' }).returning('*');
+ await (driver as any).flush();
+
+ expect((await readBack(file))[0].name).toBe('after');
+ });
+
+ it('flushes a raw mutation that carries no Knex `method`', async () => {
+ const file = newFile();
+ const driver = track(new SqliteWasmDriver({ filename: file, persist: 'on-write' }));
+ await driver.initObjects([{ name: 'acct', fields: { name: { type: 'string' } } }]);
+
+ await (driver as any).knex.raw("INSERT INTO acct (id, name) VALUES ('r1', 'raw')");
+ await (driver as any).flush();
+
+ expect((await readBack(file)).map((r) => r.id)).toEqual(['r1']);
+ });
+
+ it('`on-disconnect` still saves RETURNING writes — disconnect() is the durability contract', async () => {
+ // The second half of the same defect: the final flush is gated on the same
+ // dirty flag, so an unmarked write was lost by BOTH persist strategies.
+ // This is the invariant `bootStack({ databaseFile }).stop()` rests on.
+ //
+ // The explicit `flush()` after schema creation is what makes this test
+ // DISCRIMINATING rather than incidentally green. Schema DDL did mark the
+ // database dirty even before the fix, and under `on-disconnect` that flag
+ // simply sat there until close — so the final export happened to carry the
+ // unmarked row along with it. Clearing the flag first reproduces what
+ // `on-write` did in production: every earlier flush reset it, and from then
+ // on nothing set it again.
+ const file = newFile();
+ const driver = new SqliteWasmDriver({ filename: file, persist: 'on-disconnect' });
+ drivers.push(driver);
+ await driver.initObjects([{ name: 'acct', fields: { name: { type: 'string' } } }]);
+ await (driver as any).flush();
+
+ await (driver as any).knex('acct').insert({ id: 'd1', name: 'durable' }).returning('*');
+
+ await driver.disconnect();
+
+ const onDisk = await readBack(file);
+ expect(onDisk.map((r) => r.id)).toEqual(['d1']);
+ expect(onDisk[0].name).toBe('durable');
+ });
+
+ it('persists RETURNING writes committed inside a transaction (no mid-transaction export)', async () => {
+ const file = newFile();
+ const driver = track(new SqliteWasmDriver({ filename: file, persist: 'on-write' }));
+ await driver.initObjects([{ name: 'acct', fields: { name: { type: 'string' } } }]);
+ const knex = (driver as any).knex;
+
+ await knex.transaction(async (trx: any) => {
+ await trx('acct').insert({ id: 't1', name: 'in-tx' }).returning('*');
+ await trx('acct').where('id', 't1').first();
+ await trx('acct').insert({ id: 't2', name: 'in-tx-2' }).returning('*');
+ });
+ await (driver as any).flush();
+
+ expect((await readBack(file)).map((r) => r.id)).toEqual(['t1', 't2']);
+ });
+});