fix(driver-sqlite-wasm): persist RETURNING writes — unblock cold-boot e2e (#4518) (#4564)

A file-backed sqlite-wasm database flushed its schema at boot and then
recorded nothing else: every table on disk, every subsequent row only in
the WASM heap. `bootStack({ databaseFile })` therefore could not cold-boot,
which blocked #4470's third minimal form.

Root cause is in the Knex dialect, not the harness. `_query` picked its
execution branch from "does this statement return rows" and then set the
dirty flag only on the other, row-less branch. `INSERT ... RETURNING *`
returns rows — and that is the shape ObjectQL writes with — so it executed
on the row-returning branch and never marked the database dirty. The
`on-disconnect` flush is gated on the same flag, so both persist strategies
dropped the write; `knex.raw('INSERT ...')` (no Knex `method`) was lost the
same way.

"Does this statement change the database?" is now one exported predicate,
`statementMutatesDatabase(sql, method)`, classifying by method AND SQL text
and applied at a single funnel after execution — independent of which branch
ran it. Transaction control still routes to `noteTransactionControl` so
flushes stay deferred until a transaction closes (#1494); mutating PRAGMA
assignments now count as writes. `WasmSqliteConnection.markDirty()` loses
its method argument: re-filtering there made the same decision in two places
that could disagree, which is precisely how the branches diverged.

Tests: a new driver-level suite pins every execution branch (all six fail
when the fix is reverted), and `flow-durable-suspend.dogfood.test.ts` loses
its KNOWN GAP — it now suspends, shuts the kernel down, cold-boots a second
kernel over the same file, resumes there, and proves the result survives a
third boot, plus the plain-record assertion that identified this as a driver
defect rather than a suspended-run one.


Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.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']);
+  });
+});