From f76c469fa7160f60d9f294fc403e06062829a6eb Mon Sep 17 00:00:00 2001 From: Yann Roberto <1922498827@qq.com> Date: Fri, 9 Oct 2026 16:28:30 +0800 Subject: [PATCH] fix(state-store): persist writes from prepared result queries Compare the completed snapshot after get/all queries so RETURNING and prepared PRAGMA writes survive reloads and reopening. Export only after the outer operation or transaction succeeds; unchanged read snapshots still avoid disk writes. This adds one export/byte comparison per read snapshot without relying on SQL parsing or private sql.js APIs. Add 18 regression cases covering multi-row INSERT/UPDATE/DELETE RETURNING, standalone and transactional get/all, no-row PRAGMA writes, and rollback. The unfixed adapter fails 16 of these cases. Strengthen read-only checks. Validation: 148 focused tests pass on WSL Node 22 and Windows Node 24; the general state-store suite passes 20/20 on WSL and 19/20 on Windows, where the existing file-symlink fixture is denied by Windows permissions. Scoped adapter coverage is 90.13% lines, 82.73% branches, 100% functions. ESLint, Unicode safety, and git diff --check pass. --- scripts/lib/state-store/index.js | 20 ++-- tests/lib/state-store-persistence.test.js | 108 ++++++++++++++++++++++ 2 files changed, 119 insertions(+), 9 deletions(-) diff --git a/scripts/lib/state-store/index.js b/scripts/lib/state-store/index.js index 8c0573a6a..6bb4fc256 100644 --- a/scripts/lib/state-store/index.js +++ b/scripts/lib/state-store/index.js @@ -208,7 +208,7 @@ function wrapSqlJsDatabase(SQL, dbPath) { let inSnapshot = false; let dirty = false; let inTransaction = false; - let pragmaExecuted = false; + let needsSnapshotComparison = false; let snapshotBytes = null; function reload() { @@ -245,17 +245,18 @@ function wrapSqlJsDatabase(SQL, dbPath) { reload(); inSnapshot = true; dirty = false; - pragmaExecuted = false; + needsSnapshotComparison = false; try { const result = callback(); if (result && typeof result.then === 'function') { throw new Error('State-store operations must be synchronous'); } - if ((dirty || pragmaExecuted) && dbPath !== ':memory:') { - // SQLite PRAGMAs include reads, connection settings and persisted - // changes. Compare the resulting database instead of parsing their - // SQL syntax. Export only here: exporting inside a transaction would - // implicitly end it before our commit/rollback boundary. + if ((dirty || needsSnapshotComparison) && dbPath !== ':memory:') { + // Prepared get/all statements and PRAGMAs can also write. Compare + // database bytes instead of guessing from SQL syntax, while keeping + // read-only queries from rewriting the file. Export only here: + // exporting inside a transaction would implicitly end it before + // our commit/rollback boundary. const data = Buffer.from(rawDb.export()); if (dirty || !snapshotBytes || !data.equals(snapshotBytes)) { writeDatabaseFileAtomic(dbPath, data); @@ -265,7 +266,7 @@ function wrapSqlJsDatabase(SQL, dbPath) { } finally { inSnapshot = false; dirty = false; - pragmaExecuted = false; + needsSnapshotComparison = false; } }; return dbPath === ':memory:' ? execute() : withStateStoreLock(dbPath, execute); @@ -280,6 +281,7 @@ function wrapSqlJsDatabase(SQL, dbPath) { } else if (positionalArgs.length > 1) { stmt.bind(positionalArgs); } + needsSnapshotComparison = true; if (firstOnly) return stmt.step() ? stmt.getAsObject() : null; const rows = []; while (stmt.step()) rows.push(stmt.getAsObject()); @@ -347,7 +349,7 @@ function wrapSqlJsDatabase(SQL, dbPath) { pragma(pragmaStr) { return withSnapshot(() => { rawDb.run(`PRAGMA ${pragmaStr}`); - pragmaExecuted = true; + needsSnapshotComparison = true; }); }, diff --git a/tests/lib/state-store-persistence.test.js b/tests/lib/state-store-persistence.test.js index 38798b307..ab1bea6a0 100644 --- a/tests/lib/state-store-persistence.test.js +++ b/tests/lib/state-store-persistence.test.js @@ -426,12 +426,107 @@ async function run() { reader.listWorkItems(); reader.getStatus(); assert.strictEqual(reader.getAppliedMigrations().length, 2); + for (const method of ['get', 'all']) { + const read = () => reader._database.prepare('SELECT id FROM work_items')[method](); + assert.deepStrictEqual(read(), method === 'get' ? { id: 'saved' } : [{ id: 'saved' }]); + reader._database.transaction(read)(); + } } finally { reader.close(); } } finally { fs.renameSync = rename; } assert.strictEqual(writes, 0); assert.deepStrictEqual(fs.readFileSync(dbPath), before); }); + const returningMutations = [ + { + action: 'INSERT', + sql: "INSERT INTO returning_probe VALUES (3, 'new'), (4, 'new') RETURNING id, value", + returned: [{ id: 3, value: 'new' }, { id: 4, value: 'new' }], + saved: [{ id: 1, value: 'original' }, { id: 2, value: 'original' }, { id: 3, value: 'new' }, { id: 4, value: 'new' }], + }, + { + action: 'UPDATE', + sql: "UPDATE returning_probe SET value = 'updated' RETURNING id, value", + returned: [{ id: 1, value: 'updated' }, { id: 2, value: 'updated' }], + saved: [{ id: 1, value: 'updated' }, { id: 2, value: 'updated' }], + }, + { + action: 'DELETE', + sql: 'DELETE FROM returning_probe RETURNING id, value', + returned: [{ id: 1, value: 'original' }, { id: 2, value: 'original' }], + saved: [], + }, + ]; + for (const method of ['get', 'all']) { + for (const transactional of [false, true]) { + for (const mutation of returningMutations) { + await test(`prepared ${method} persists ${mutation.action} RETURNING (${transactional ? 'query-only transaction' : 'standalone'})`, async dbPath => { + const store = await createStateStore({ dbPath }); + try { + store._database.exec("CREATE TABLE returning_probe (id INTEGER PRIMARY KEY, value TEXT NOT NULL); INSERT INTO returning_probe VALUES (1, 'original'), (2, 'original');"); + const before = fs.readFileSync(dbPath); + const mutate = () => { + const result = store._database.prepare(mutation.sql)[method](); + if (method === 'get') assert.ok(mutation.returned.some(row => row.id === result.id && row.value === result.value)); + else assert.deepStrictEqual(result.sort((a, b) => a.id - b.id), mutation.returned); + if (transactional) assert.deepStrictEqual(fs.readFileSync(dbPath), before, 'RETURNING must not publish before COMMIT'); + }; + if (transactional) store._database.transaction(mutate)(); + else mutate(); + assert.deepStrictEqual(store._database.prepare('SELECT id, value FROM returning_probe ORDER BY id').all(), mutation.saved, + 'the next snapshot must retain the mutation'); + } finally { store.close(); } + const reopened = await createStateStore({ dbPath }); + try { + assert.deepStrictEqual(reopened._database.prepare('SELECT id, value FROM returning_probe ORDER BY id').all(), mutation.saved); + } finally { reopened.close(); } + }); + } + + await test(`prepared ${method} persists no-row PRAGMA writes (${transactional ? 'query-only transaction' : 'standalone'})`, async dbPath => { + const store = await createStateStore({ dbPath }); + try { + const before = fs.readFileSync(dbPath); + const mutate = () => { + for (const pragma of ['user_version = 3252', 'application_id(2468)']) { + const result = store._database.prepare(`PRAGMA ${pragma}`)[method](); + assert.deepStrictEqual(result, method === 'get' ? null : []); + } + if (transactional) assert.deepStrictEqual(fs.readFileSync(dbPath), before, 'prepared PRAGMAs must not publish before COMMIT'); + }; + if (transactional) store._database.transaction(mutate)(); + else mutate(); + assert.strictEqual(store._database.prepare('PRAGMA user_version').get().user_version, 3252); + assert.strictEqual(store._database.prepare('PRAGMA application_id').get().application_id, 2468); + } finally { store.close(); } + const reopened = await createStateStore({ dbPath }); + try { + assert.strictEqual(reopened._database.prepare('PRAGMA user_version').get().user_version, 3252); + assert.strictEqual(reopened._database.prepare('PRAGMA application_id').get().application_id, 2468); + } finally { reopened.close(); } + }); + } + + await test(`prepared ${method} RETURNING and PRAGMA writes roll back without changing disk`, async dbPath => { + const store = await createStateStore({ dbPath }); + try { + store._database.exec("CREATE TABLE returning_probe (id INTEGER PRIMARY KEY, value TEXT NOT NULL); INSERT INTO returning_probe VALUES (1, 'original'), (2, 'original');"); + const before = fs.readFileSync(dbPath); + const failure = new Error('abort prepared writes'); + assert.throws(() => store._database.transaction(() => { + for (const mutation of returningMutations) store._database.prepare(mutation.sql)[method](); + store._database.prepare('PRAGMA user_version = 3252')[method](); + assert.deepStrictEqual(fs.readFileSync(dbPath), before); + throw failure; + })(), error => error === failure); + assert.deepStrictEqual(fs.readFileSync(dbPath), before); + assert.deepStrictEqual(store._database.prepare('SELECT id, value FROM returning_probe ORDER BY id').all(), + [{ id: 1, value: 'original' }, { id: 2, value: 'original' }]); + assert.strictEqual(store._database.prepare('PRAGMA user_version').get().user_version, 0); + } finally { store.close(); } + }); + } + await test('persistent PRAGMA changes survive snapshot reload and reopening', async dbPath => { const store = await createStateStore({ dbPath }); try { @@ -459,6 +554,19 @@ async function run() { store._database.pragma('user_version'); store._database.pragma('table_info(work_items)'); store._database.pragma('integrity_check'); + for (const method of ['get', 'all']) { + const read = () => { + for (const pragma of ['user_version', 'table_info(work_items)', 'integrity_check']) { + store._database.prepare(`PRAGMA ${pragma}`)[method](); + } + }; + read(); + store._database.transaction(read)(); + store._database.withSnapshot(() => { + store._database.prepare('PRAGMA cache_size = 512')[method](); + assert.strictEqual(store._database.prepare('PRAGMA cache_size').get().cache_size, 512); + }); + } store._database.transaction(() => { store._database.pragma('user_version'); store._database.pragma('table_info(work_items)');