From 6bfc0126fb5d934f9683a6dbad5d06e04dc0fcc2 Mon Sep 17 00:00:00 2001 From: zknpr Date: Wed, 3 Jun 2026 14:16:03 +0200 Subject: [PATCH 01/11] feat(undo): persist futureStack across hot-exit serialization serialize()/deserialize() now round-trip the redo stack; add getEntriesUndoneSinceCheckpoint() returning the saved-then-undone entries in revert order. Backward-compatible with pre-1.5.1 backups (absent futureStack -> []). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/undo-history.ts | 31 +++++++++++++++++++-- tests/unit/undo-history.test.ts | 49 +++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 tests/unit/undo-history.test.ts diff --git a/src/core/undo-history.ts b/src/core/undo-history.ts index 34e0f1a..19a9409 100644 --- a/src/core/undo-history.ts +++ b/src/core/undo-history.ts @@ -343,6 +343,25 @@ export class ModificationTracker(maxEntries, maxMemory); tracker.timeline = payload.timeline || []; tracker.checkpointIndex = payload.checkpointIndex || 0; + tracker.futureStack = payload.futureStack || []; - // Recalculate sizes + // Recalculate sizes for both active timeline entries and redo entries so + // restored trackers enforce the same memory accounting as live trackers. tracker.timelineSizes = tracker.timeline.map(calculateSize); - tracker.currentSize = tracker.timelineSizes.reduce((a, b) => a + b, 0); + tracker.futureStackSizes = tracker.futureStack.map(calculateSize); + tracker.currentSize = + tracker.timelineSizes.reduce((a, b) => a + b, 0) + + tracker.futureStackSizes.reduce((a, b) => a + b, 0); return tracker; } diff --git a/tests/unit/undo-history.test.ts b/tests/unit/undo-history.test.ts new file mode 100644 index 0000000..01c48c4 --- /dev/null +++ b/tests/unit/undo-history.test.ts @@ -0,0 +1,49 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert'; +import { ModificationTracker } from '../../src/core/undo-history'; +import type { LabeledModification } from '../../src/core/types'; + +const entry = (label: string): LabeledModification => ({ + label, + description: label, + modificationType: 'cell_update' +}); + +describe('ModificationTracker hot-exit persistence', () => { + it('T1: serialize/deserialize round-trips futureStack', () => { + const t = new ModificationTracker(); + t.record(entry('e1')); + t.record(entry('e2')); + t.stepBack(); + + const restored = ModificationTracker.deserialize(t.serialize()); + + assert.strictEqual(restored.entryCount, 1); + assert.strictEqual(restored.canStepForward, true); + assert.strictEqual(restored.stepForward()?.label, 'e2'); + }); + + it('T2: a pre-1.5.1 backup (no futureStack key) yields an empty redo stack', () => { + const legacy = new TextEncoder().encode( + JSON.stringify({ timeline: [entry('e1')], checkpointIndex: 1 }) + ); + + const restored = ModificationTracker.deserialize(legacy); + + assert.strictEqual(restored.entryCount, 1); + assert.strictEqual(restored.canStepForward, false); + }); + + it('T3: getEntriesUndoneSinceCheckpoint returns the saved-then-undone entries in revert order', async () => { + const t = new ModificationTracker(); + t.record(entry('e1')); + t.record(entry('e2')); + await t.createCheckpoint(); + + assert.deepStrictEqual(t.getEntriesUndoneSinceCheckpoint().map(e => e.label), []); + t.stepBack(); + assert.deepStrictEqual(t.getEntriesUndoneSinceCheckpoint().map(e => e.label), ['e2']); + t.stepBack(); + assert.deepStrictEqual(t.getEntriesUndoneSinceCheckpoint().map(e => e.label), ['e2', 'e1']); + }); +}); From 67ee127d05c435e2095adcfd45af0f19c6af1716 Mon Sep 17 00:00:00 2001 From: zknpr Date: Wed, 3 Jun 2026 14:16:03 +0200 Subject: [PATCH 02/11] feat(undo): restore reconciler brings a hot-exit-restored DB to the live state reconcileRestoredDatabase replays forward or reverts saved-then-undone edits for WASM (the saved file is the checkpoint state); native is a no-op (on-disk file is already live). vscode-free, unit-tested against a real engine (incl. a json_patch revert exercising the #427 undo). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/restore-reconciler.ts | 37 ++++++++ tests/unit/restore-reconciler.test.ts | 119 ++++++++++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 src/core/restore-reconciler.ts create mode 100644 tests/unit/restore-reconciler.test.ts diff --git a/src/core/restore-reconciler.ts b/src/core/restore-reconciler.ts new file mode 100644 index 0000000..66b8c0d --- /dev/null +++ b/src/core/restore-reconciler.ts @@ -0,0 +1,37 @@ +import type { DatabaseOperations, LabeledModification } from './types'; +import type { ModificationTracker } from './undo-history'; + +/** + * Bring a freshly opened database to the live timeline state after hot-exit restore. + * + * WASM restore opens the database bytes from the saved checkpoint, so the database + * must either replay entries that were recorded after that checkpoint or revert + * entries that were saved and then undone before shutdown. Native SQLite writes + * edits and undos directly to the on-disk file, so the opened database is already + * at the live state and only the tracker needs to retain redo state. + * + * @param databaseOps - Database operation facade for the restored database + * @param tracker - Deserialized hot-exit modification tracker + * @param engineKind - Active database engine type + * @param signal - Optional cancellation signal used by forward replay + */ +export async function reconcileRestoredDatabase( + databaseOps: DatabaseOperations, + tracker: ModificationTracker, + engineKind: 'wasm' | 'native', + signal?: AbortSignal +): Promise { + if (engineKind === 'native') { + return; + } + + const forward = tracker.getUncommittedEntries(); + if (forward.length > 0) { + await databaseOps.applyModifications(forward, signal); + return; + } + + for (const entry of tracker.getEntriesUndoneSinceCheckpoint()) { + await databaseOps.undoModification(entry); + } +} diff --git a/tests/unit/restore-reconciler.test.ts b/tests/unit/restore-reconciler.test.ts new file mode 100644 index 0000000..1de8f91 --- /dev/null +++ b/tests/unit/restore-reconciler.test.ts @@ -0,0 +1,119 @@ +import './vscode_mock_setup'; +import { describe, it } from 'node:test'; +import assert from 'node:assert'; +import { createDatabaseEngine } from '../../src/core/sqlite-db'; +import { ModificationTracker } from '../../src/core/undo-history'; +import { reconcileRestoredDatabase } from '../../src/core/restore-reconciler'; +import type { DatabaseOperations, LabeledModification } from '../../src/core/types'; + +async function freshEngine(): Promise { + const result = await createDatabaseEngine({ content: null, maxSize: 0, readOnlyMode: false }); + const ops = result.operations!; + await ops.executeQuery('CREATE TABLE t (id INTEGER PRIMARY KEY, data TEXT)'); + return ops; +} + +const cellEdit = ( + label: string, + prior: string, + next: string, + op: 'set' | 'json_patch' = 'set' +): LabeledModification => ({ + label, + description: label, + modificationType: 'cell_update', + targetTable: 't', + targetRowId: 1, + targetColumn: 'data', + priorValue: prior, + newValue: next, + operation: op +}); + +describe('reconcileRestoredDatabase', () => { + it('W1: WASM restore reverts a saved-then-undone edit to the undone value', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: 'original' }); + await ops.updateCell('t', 1, 'data', 'edited'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e', 'original', 'edited')); + await tracker.createCheckpoint(); + tracker.stepBack(); + + await reconcileRestoredDatabase(ops, tracker, 'wasm'); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.strictEqual(r[0].rows[0][0], 'original'); + assert.strictEqual(tracker.canStepForward, true); + }); + + it('W2: reverts two saved-then-undone edits to the same cell in the correct order', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: 'v0' }); + await ops.updateCell('t', 1, 'data', 'v1'); + await ops.updateCell('t', 1, 'data', 'v2'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e1', 'v0', 'v1')); + tracker.record(cellEdit('e2', 'v1', 'v2')); + await tracker.createCheckpoint(); + tracker.stepBack(); + tracker.stepBack(); + + await reconcileRestoredDatabase(ops, tracker, 'wasm'); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.strictEqual(r[0].rows[0][0], 'v0'); + }); + + it('W3: WASM restore replays forward edits recorded after the checkpoint', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: 'v0' }); + await ops.updateCell('t', 1, 'data', 'v1'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e1', 'v0', 'v1')); + await tracker.createCheckpoint(); + tracker.record(cellEdit('e2', 'v1', 'v2')); + + await reconcileRestoredDatabase(ops, tracker, 'wasm'); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.strictEqual(r[0].rows[0][0], 'v2'); + }); + + it('W4: WASM restore reverts a saved-then-undone json_patch edit faithfully (uses #427)', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: '{"x":1,"y":2}' }); + await ops.updateCell('t', 1, 'data', null, '{"x":9}'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('jp', '{"x":1,"y":2}', '{"x":9}', 'json_patch')); + await tracker.createCheckpoint(); + tracker.stepBack(); + + await reconcileRestoredDatabase(ops, tracker, 'wasm'); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.deepStrictEqual(JSON.parse(r[0].rows[0][0] as string), { x: 1, y: 2 }); + }); + + it('native: reconcile performs no database operations and keeps redo available', async () => { + const calls: string[] = []; + const ops = { + applyModifications: async () => { calls.push('apply'); }, + undoModification: async () => { calls.push('undo'); } + } as unknown as DatabaseOperations; + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e', 'a', 'b')); + await tracker.createCheckpoint(); + tracker.stepBack(); + + await reconcileRestoredDatabase(ops, tracker, 'native'); + + assert.deepStrictEqual(calls, []); + assert.strictEqual(tracker.canStepForward, true); + }); +}); From 44eb7bdbf3dcb1214aab2e84c71e2b9b2482dd92 Mon Sep 17 00:00:00 2001 From: zknpr Date: Wed, 3 Jun 2026 14:16:04 +0200 Subject: [PATCH 03/11] feat(undo): reconcile restored database state on hot-exit open create() now calls reconcileRestoredDatabase instead of a bare forward replay, so a saved-then-undone WASM edit is reverted to the undone state on reopen and redo works. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/databaseModel.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/databaseModel.ts b/src/databaseModel.ts index fdb0368..c4305ba 100644 --- a/src/databaseModel.ts +++ b/src/databaseModel.ts @@ -22,6 +22,7 @@ import { getMaximumFileSizeBytes } from './config'; import { GlobalOutputChannel } from './main'; import { ModificationTracker } from './core/undo-history'; +import { reconcileRestoredDatabase } from './core/restore-reconciler'; import type { LabeledModification, DatabaseOperations } from './core/types'; import { LoggingDatabaseOperations } from './loggingDatabaseOperations'; @@ -151,9 +152,13 @@ export class DatabaseDocument extends Disposable implements vsc.CustomDocument { ); try { - // Replay uncommitted modifications - await databaseOps.applyModifications( - tracker.getUncommittedEntries(), + // Reconcile the opened database to the live timeline state. WASM opens + // checkpoint bytes and may need forward replay or saved-edit reverts; + // native opens the already-live on-disk database and returns unchanged. + await reconcileRestoredDatabase( + databaseOps, + tracker, + await databaseOps.engineKind, cancelTokenToAbortSignal(cancellation) ); } catch (err) { From 7341f4821ff1dfa9a5ae9523872c2e6c83590f5e Mon Sep 17 00:00:00 2001 From: zknpr Date: Wed, 3 Jun 2026 14:16:04 +0200 Subject: [PATCH 04/11] =?UTF-8?q?chore(release):=201.5.1=20=E2=80=94=20jso?= =?UTF-8?q?n=5Fpatch=20undo=20fidelity=20+=20hot-exit=20futureStack=20(#42?= =?UTF-8?q?5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 7 +++++++ package.json | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ad1225..f2833a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 1.5.1 + +### Fixes + +- **Undo of a JSON cell edit no longer clobbers concurrent changes to other keys.** When a JSON cell was edited as a merge patch, undo now restores only the keys that edit changed (reading the current value and writing the whole object back), preserving a concurrent change to a different key of the same cell. Cells whose value is not a JSON object, and JSON numbers that cannot survive a parse/serialize round-trip (large integers, overflow, high-precision decimals), fall back to an exact whole-value restore. The undo read/compute/write is serialized so a concurrent edit cannot interleave. +- **Hot exit now preserves undone and redo state.** Closing the editor with unsaved changes and reopening previously lost the redo stack, and on the web build could silently drop an undo of an already-saved edit. The redo history is now persisted in the hot-exit backup and the database is reconciled to the exact pre-close state on reopen. + ## 1.5.0 ### Features diff --git a/package.json b/package.json index 2723ab7..a0a01b4 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "name": "sqlite-explorer", "displayName": "SQLite Explorer", "description": "A powerful SQLite database viewer and editor for VS Code", - "version": "1.5.0", + "version": "1.5.1", "publisher": "zknpr", "license": "MIT", "repository": { From 6c05167311f7a52c384d763a94a4eaaa9f23821d Mon Sep 17 00:00:00 2001 From: zknpr Date: Wed, 3 Jun 2026 15:57:54 +0200 Subject: [PATCH 05/11] fix(undo): clamp getEntriesUndoneSinceCheckpoint + document branch-case limitation (review) Address PR #434 review: - Gemini (G1): Math.max(0, ...) clamps the slice start so a corrupted/hand-edited backup with delta > futureStack.length cannot silently drop entries. - CodeRabbit (CR1): document the branch case (undo a saved edit, then record a new one) as a known limitation in restore-reconciler.ts. Fully fixing it is a checkpoint-model redesign (must also fix File>Revert to avoid data loss), tracked as a dedicated follow-up PR that closes #425; this PR is scoped to the base case + redo persistence and references #425. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/restore-reconciler.ts | 6 ++++++ src/core/undo-history.ts | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/core/restore-reconciler.ts b/src/core/restore-reconciler.ts index 66b8c0d..16d4d6f 100644 --- a/src/core/restore-reconciler.ts +++ b/src/core/restore-reconciler.ts @@ -10,6 +10,12 @@ import type { ModificationTracker } from './undo-history'; * edits and undos directly to the on-disk file, so the opened database is already * at the live state and only the tracker needs to retain redo state. * + * Known limitation (tracked in #425, addressed in a dedicated follow-up): the + * "branch" case — undo a saved edit, then record a NEW edit — is not reconstructed + * here. record() clears the redo stack and the index-based checkpoint cannot + * describe the resulting divergence, so reopen yields the saved state rather than + * the branched one. Fixing it requires a checkpoint-model redesign. + * * @param databaseOps - Database operation facade for the restored database * @param tracker - Deserialized hot-exit modification tracker * @param engineKind - Active database engine type diff --git a/src/core/undo-history.ts b/src/core/undo-history.ts index 19a9409..f4f5913 100644 --- a/src/core/undo-history.ts +++ b/src/core/undo-history.ts @@ -359,7 +359,9 @@ export class ModificationTracker Date: Wed, 3 Jun 2026 17:18:56 +0200 Subject: [PATCH 06/11] =?UTF-8?q?feat(undo):=20branch-aware=20checkpoint?= =?UTF-8?q?=20=E2=80=94=20persist=20abandoned=20saved=20edits=20(revertOnR?= =?UTF-8?q?estore)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a new edit branches off an undone saved edit, record() now captures the saved-then-undone entries into a persisted revertOnRestore list and moves the checkpoint to the divergence point, instead of dropping them. hasUncommittedChanges accounts for it (the branch is correctly dirty), createCheckpoint/At clear it, and rollbackToCheckpoint re-applies it so the tracker rebuilds the saved state. serialize/deserialize round-trip it (backward-compatible). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/undo-history.ts | 96 +++++++++++++++++++++++++++++++-- tests/unit/undo-history.test.ts | 68 +++++++++++++++++++++++ 2 files changed, 159 insertions(+), 5 deletions(-) diff --git a/src/core/undo-history.ts b/src/core/undo-history.ts index f4f5913..93a659c 100644 --- a/src/core/undo-history.ts +++ b/src/core/undo-history.ts @@ -147,6 +147,19 @@ export class ModificationTracker 0) { + const start = Math.max(0, this.futureStack.length - savedUndoneCount); + this.revertOnRestore.push(...this.futureStack.slice(start)); + this.revertOnRestoreSizes.push(...this.futureStackSizes.slice(start)); + + // Captured entries remain necessary for restore, so leave their memory in + // currentSize and trim futureStack down to only redo entries that the new + // branch truly discards. + this.futureStack = this.futureStack.slice(0, start); + this.futureStackSizes = this.futureStackSizes.slice(0, start); + this.checkpointIndex = this.timeline.length; + this.invalidateCapturedCheckpointPositions(); + } + // Subtract size of redo history that we are about to discard const redoSize = this.futureStackSizes.reduce((a, b) => a + b, 0); this.currentSize -= redoSize; @@ -283,7 +311,7 @@ export class ModificationTracker 0; } /** @@ -291,6 +319,9 @@ export class ModificationTracker { this.checkpointIndex = this.timeline.length; + this.currentSize -= this.revertOnRestoreSizes.reduce((a, b) => a + b, 0); + this.revertOnRestore = []; + this.revertOnRestoreSizes = []; } /** @@ -332,6 +363,9 @@ export class ModificationTracker a + b, 0); + this.revertOnRestore = []; + this.revertOnRestoreSizes = []; } /** @@ -364,11 +398,39 @@ export class ModificationTracker 0) { + // Entries below the checkpoint that are currently undone already exist on + // futureStack. Rolling back to saved consumes them because they become part + // of the active saved timeline again. + this.futureStack = this.futureStack.slice(0, savedUndoneStart); + this.futureStackSizes = this.futureStackSizes.slice(0, savedUndoneStart); + changed = true; + } + const uncommittedCount = this.timeline.length - this.checkpointIndex; if (uncommittedCount > 0) { const uncommitted = this.timeline.splice(this.checkpointIndex); @@ -376,6 +438,25 @@ export class ModificationTracker 0) { + // Saved-undone entries are stored newest-first for undo. Rolling back to + // the saved checkpoint needs them in original application order. + const restoreSizes = [...this.revertOnRestoreSizes, ...savedUndoneSizes]; + const restored = restoreSequence.reverse(); + const restoredSizes = restoreSizes.reverse(); + this.timeline.push(...restored); + this.timelineSizes.push(...restoredSizes); + this.checkpointIndex = this.timeline.length; + this.revertOnRestore = []; + this.revertOnRestoreSizes = []; + changed = true; + } + + if (changed) { this.invalidateCapturedCheckpointPositions(); } } @@ -390,7 +471,8 @@ export class ModificationTracker a + b, 0) + - tracker.futureStackSizes.reduce((a, b) => a + b, 0); + tracker.futureStackSizes.reduce((a, b) => a + b, 0) + + tracker.revertOnRestoreSizes.reduce((a, b) => a + b, 0); return tracker; } diff --git a/tests/unit/undo-history.test.ts b/tests/unit/undo-history.test.ts index 01c48c4..b43390c 100644 --- a/tests/unit/undo-history.test.ts +++ b/tests/unit/undo-history.test.ts @@ -46,4 +46,72 @@ describe('ModificationTracker hot-exit persistence', () => { t.stepBack(); assert.deepStrictEqual(t.getEntriesUndoneSinceCheckpoint().map(e => e.label), ['e2', 'e1']); }); + + it('BC1: record after undoing a saved edit captures it and moves the checkpoint', async () => { + const t = new ModificationTracker(); + t.record(entry('e1')); + t.record(entry('e2')); + await t.createCheckpoint(); + t.stepBack(); + t.record(entry('e3')); + + assert.strictEqual(t.entryCount, 2); + assert.deepStrictEqual(t.getCheckpointRevertSequence().map(e => e.label), ['e2']); + assert.strictEqual(t.hasUncommittedChanges(), true); + }); + + it('BC2: multi-branch accumulates saved-undone in revert order', async () => { + const t = new ModificationTracker(); + t.record(entry('e1')); + t.record(entry('e2')); + t.record(entry('e3')); + await t.createCheckpoint(); + t.stepBack(); + t.stepBack(); + t.record(entry('e4')); + + assert.deepStrictEqual(t.getCheckpointRevertSequence().map(e => e.label), ['e3', 'e2']); + }); + + it('BC3: a save clears revertOnRestore (clean baseline)', async () => { + const t = new ModificationTracker(); + t.record(entry('e1')); + t.record(entry('e2')); + await t.createCheckpoint(); + t.stepBack(); + t.record(entry('e3')); + await t.createCheckpoint(); + + assert.deepStrictEqual(t.getCheckpointRevertSequence(), []); + assert.strictEqual(t.hasUncommittedChanges(), false); + }); + + it('BC4: serialize/deserialize round-trips revertOnRestore', async () => { + const t = new ModificationTracker(); + t.record(entry('e1')); + t.record(entry('e2')); + await t.createCheckpoint(); + t.stepBack(); + t.record(entry('e3')); + + const restored = ModificationTracker.deserialize(t.serialize()); + + assert.deepStrictEqual(restored.getCheckpointRevertSequence().map(e => e.label), ['e2']); + assert.strictEqual(restored.hasUncommittedChanges(), true); + }); + + it('BC5: rollbackToCheckpoint re-applies branched saved edits to reach the saved state', async () => { + const t = new ModificationTracker(); + t.record(entry('e1')); + t.record(entry('e2')); + await t.createCheckpoint(); + t.stepBack(); + t.record(entry('e3')); + + t.rollbackToCheckpoint(); + + assert.deepStrictEqual(t['timeline'].map((e: LabeledModification) => e.label), ['e1', 'e2']); + assert.deepStrictEqual(t.getCheckpointRevertSequence(), []); + assert.strictEqual(t.hasUncommittedChanges(), false); + }); }); From 9df2bdc3a752680e0706bb1ed8e807f7db4e2cbd Mon Sep 17 00:00:00 2001 From: zknpr Date: Wed, 3 Jun 2026 17:18:56 +0200 Subject: [PATCH 07/11] feat(undo): atomic branch-aware restore + revert helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reconcileRestoredDatabase reverts the saved-undone sequence then replays forward; revertDatabaseToSaved (File>Revert) undoes the live branch then re-applies the saved-undone edits — both wrapped in a SAVEPOINT (runInSavepoint) so a mid-sequence failure rolls back to the opened bytes instead of leaving a partial timeline. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/restore-reconciler.ts | 89 ++++++++++-- tests/unit/restore-reconciler.test.ts | 188 +++++++++++++++++++++++++- 2 files changed, 264 insertions(+), 13 deletions(-) diff --git a/src/core/restore-reconciler.ts b/src/core/restore-reconciler.ts index 16d4d6f..71d957b 100644 --- a/src/core/restore-reconciler.ts +++ b/src/core/restore-reconciler.ts @@ -10,12 +10,6 @@ import type { ModificationTracker } from './undo-history'; * edits and undos directly to the on-disk file, so the opened database is already * at the live state and only the tracker needs to retain redo state. * - * Known limitation (tracked in #425, addressed in a dedicated follow-up): the - * "branch" case — undo a saved edit, then record a NEW edit — is not reconstructed - * here. record() clears the redo stack and the index-based checkpoint cannot - * describe the resulting divergence, so reopen yields the saved state rather than - * the branched one. Fixing it requires a checkpoint-model redesign. - * * @param databaseOps - Database operation facade for the restored database * @param tracker - Deserialized hot-exit modification tracker * @param engineKind - Active database engine type @@ -31,13 +25,88 @@ export async function reconcileRestoredDatabase( return; } + const revertSeq = tracker.getCheckpointRevertSequence(); + const forward = tracker.getUncommittedEntries(); + if (revertSeq.length === 0 && forward.length === 0) { + return; + } + + await runInSavepoint(databaseOps, 'sp_restore', async () => { + // Revert saved entries first to move checkpoint bytes back to the + // common-prefix state, then replay live entries from that boundary. + for (const entry of revertSeq) { + await databaseOps.undoModification(entry); + } + if (forward.length > 0) { + await databaseOps.applyModifications(forward, signal); + } + }); +} + +/** + * Bring the live database and tracker back to the last saved checkpoint state. + * + * Forward entries are undone from the live timeline, then saved-undone entries + * are re-applied in original application order. The tracker is rolled back only + * after database mutations commit so a failed revert does not desynchronize the + * in-memory history from the live database. + * + * @param databaseOps - Database operation facade for the open database + * @param tracker - Modification tracker for the open document + * @param signal - Optional cancellation signal used by replay/discard helpers + */ +export async function revertDatabaseToSaved( + databaseOps: DatabaseOperations, + tracker: ModificationTracker, + signal?: AbortSignal +): Promise { const forward = tracker.getUncommittedEntries(); - if (forward.length > 0) { - await databaseOps.applyModifications(forward, signal); + const redo = [...tracker.getCheckpointRevertSequence()].reverse(); + + if (forward.length === 0 && redo.length === 0) { + tracker.rollbackToCheckpoint(); + return; + } + + await runInSavepoint(databaseOps, 'sp_revert', async () => { + // Discard live edits first, then re-apply saved entries that the user had + // undone so the final database bytes match the checkpoint exactly. + if (forward.length > 0) { + await databaseOps.discardModifications(forward, signal); + } + if (redo.length > 0) { + await databaseOps.applyModifications(redo, signal); + } + }); + + tracker.rollbackToCheckpoint(); +} + +/** + * Run database mutations inside a SAVEPOINT and roll back all mutations from + * this helper if any operation in the sequence fails. + */ +async function runInSavepoint( + databaseOps: DatabaseOperations, + prefix: string, + body: () => Promise +): Promise { + if (typeof databaseOps.executeQuery !== 'function') { + // Some unit-test doubles only implement the mutation primitive under test. + // Real DatabaseOperations implementations expose executeQuery and therefore + // take the atomic SAVEPOINT path below. + await body(); return; } - for (const entry of tracker.getEntriesUndoneSinceCheckpoint()) { - await databaseOps.undoModification(entry); + const name = `${prefix}_${Date.now()}`; + await databaseOps.executeQuery(`SAVEPOINT ${name}`); + try { + await body(); + await databaseOps.executeQuery(`RELEASE ${name}`); + } catch (err) { + await databaseOps.executeQuery(`ROLLBACK TO ${name}`).catch(() => {}); + await databaseOps.executeQuery(`RELEASE ${name}`).catch(() => {}); + throw err; } } diff --git a/tests/unit/restore-reconciler.test.ts b/tests/unit/restore-reconciler.test.ts index 1de8f91..8b64f68 100644 --- a/tests/unit/restore-reconciler.test.ts +++ b/tests/unit/restore-reconciler.test.ts @@ -3,9 +3,11 @@ import { describe, it } from 'node:test'; import assert from 'node:assert'; import { createDatabaseEngine } from '../../src/core/sqlite-db'; import { ModificationTracker } from '../../src/core/undo-history'; -import { reconcileRestoredDatabase } from '../../src/core/restore-reconciler'; +import * as restoreReconciler from '../../src/core/restore-reconciler'; import type { DatabaseOperations, LabeledModification } from '../../src/core/types'; +const { reconcileRestoredDatabase } = restoreReconciler; + async function freshEngine(): Promise { const result = await createDatabaseEngine({ content: null, maxSize: 0, readOnlyMode: false }); const ops = result.operations!; @@ -17,19 +19,25 @@ const cellEdit = ( label: string, prior: string, next: string, - op: 'set' | 'json_patch' = 'set' + op: 'set' | 'json_patch' = 'set', + column: string = 'data' ): LabeledModification => ({ label, description: label, modificationType: 'cell_update', targetTable: 't', targetRowId: 1, - targetColumn: 'data', + targetColumn: column, priorValue: prior, newValue: next, operation: op }); +const roundTripTracker = ( + tracker: ModificationTracker +): ModificationTracker => + ModificationTracker.deserialize(tracker.serialize()); + describe('reconcileRestoredDatabase', () => { it('W1: WASM restore reverts a saved-then-undone edit to the undone value', async () => { const ops = await freshEngine(); @@ -116,4 +124,178 @@ describe('reconcileRestoredDatabase', () => { assert.deepStrictEqual(calls, []); assert.strictEqual(tracker.canStepForward, true); }); + + it('BC-restore branch: reverts the saved edit and replays the branch edit', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: '{"a":0,"b":0}' }); + await ops.updateCell('t', 1, 'data', '{"a":1,"b":0}'); + await ops.updateCell('t', 1, 'data', '{"a":1,"b":2}'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e1', '{"a":0,"b":0}', '{"a":1,"b":0}')); + tracker.record(cellEdit('e2', '{"a":1,"b":0}', '{"a":1,"b":2}')); + await tracker.createCheckpoint(); + tracker.stepBack(); + tracker.record(cellEdit('e3', '{"a":1,"b":0}', '{"c":3}', 'json_patch')); + + await reconcileRestoredDatabase(ops, roundTripTracker(tracker), 'wasm'); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.deepStrictEqual(JSON.parse(r[0].rows[0][0] as string), { a: 1, b: 0, c: 3 }); + }); + + it('BC-restore multi-branch: reverts abandoned saved edits before replaying the branch', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: 'v0' }); + await ops.updateCell('t', 1, 'data', 'v1'); + await ops.updateCell('t', 1, 'data', 'v2'); + await ops.updateCell('t', 1, 'data', 'v3'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e1', 'v0', 'v1')); + tracker.record(cellEdit('e2', 'v1', 'v2')); + tracker.record(cellEdit('e3', 'v2', 'v3')); + await tracker.createCheckpoint(); + tracker.stepBack(); + tracker.stepBack(); + tracker.record(cellEdit('e4', 'v1', 'v4')); + + await reconcileRestoredDatabase(ops, roundTripTracker(tracker), 'wasm'); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.strictEqual(r[0].rows[0][0], 'v4'); + }); + + it('BC-restore base: reverts a saved edit that remains on the redo stack', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: 'v0' }); + await ops.updateCell('t', 1, 'data', 'v1'); + await ops.updateCell('t', 1, 'data', 'v2'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e1', 'v0', 'v1')); + tracker.record(cellEdit('e2', 'v1', 'v2')); + await tracker.createCheckpoint(); + tracker.stepBack(); + + await reconcileRestoredDatabase(ops, roundTripTracker(tracker), 'wasm'); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.strictEqual(r[0].rows[0][0], 'v1'); + }); + + it('BC-restore forward: replays edits recorded after the saved checkpoint', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: 'v0' }); + await ops.updateCell('t', 1, 'data', 'v1'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e1', 'v0', 'v1')); + await tracker.createCheckpoint(); + tracker.record(cellEdit('e2', 'v1', 'v2')); + + await reconcileRestoredDatabase(ops, roundTripTracker(tracker), 'wasm'); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.strictEqual(r[0].rows[0][0], 'v2'); + }); + + it('BC-restore branch then undo common: reverts abandoned saved and common-prefix edits', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: 'v0' }); + await ops.updateCell('t', 1, 'data', 'v1'); + await ops.updateCell('t', 1, 'data', 'v2'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e1', 'v0', 'v1')); + tracker.record(cellEdit('e2', 'v1', 'v2')); + await tracker.createCheckpoint(); + tracker.stepBack(); + tracker.record(cellEdit('e3', 'v1', 'v3')); + tracker.stepBack(); + tracker.stepBack(); + + await reconcileRestoredDatabase(ops, roundTripTracker(tracker), 'wasm'); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.strictEqual(r[0].rows[0][0], 'v0'); + }); + + it('BC-revert branch: File>Revert restores the saved state without losing data', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: 'v0' }); + await ops.updateCell('t', 1, 'data', 'v1'); + await ops.updateCell('t', 1, 'data', 'v3'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e1', 'v0', 'v1')); + tracker.record(cellEdit('e2', 'v1', 'v2')); + await tracker.createCheckpoint(); + tracker.stepBack(); + tracker.record(cellEdit('e3', 'v1', 'v3')); + + await restoreReconciler.revertDatabaseToSaved(ops, tracker); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.strictEqual(r[0].rows[0][0], 'v2'); + assert.strictEqual(tracker.hasUncommittedChanges(), false); + }); + + it('BC-revert base: File>Revert reapplies a saved edit that was undone', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: 'v0' }); + await ops.updateCell('t', 1, 'data', 'v1'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e1', 'v0', 'v1')); + tracker.record(cellEdit('e2', 'v1', 'v2')); + await tracker.createCheckpoint(); + tracker.stepBack(); + + await restoreReconciler.revertDatabaseToSaved(ops, tracker); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.strictEqual(r[0].rows[0][0], 'v2'); + assert.strictEqual(tracker.hasUncommittedChanges(), false); + }); + + it('BC-revert forward: File>Revert discards edits recorded after the checkpoint', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: 'v0' }); + await ops.updateCell('t', 1, 'data', 'v1'); + await ops.updateCell('t', 1, 'data', 'v2'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e1', 'v0', 'v1')); + await tracker.createCheckpoint(); + tracker.record(cellEdit('e2', 'v1', 'v2')); + + await restoreReconciler.revertDatabaseToSaved(ops, tracker); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.strictEqual(r[0].rows[0][0], 'v1'); + assert.strictEqual(tracker.hasUncommittedChanges(), false); + }); + + it('BC-restore atomicity: rolls back prior reverts when a later revert entry fails', async () => { + const ops = await freshEngine(); + await ops.insertRow('t', { id: 1, data: 'v0' }); + await ops.updateCell('t', 1, 'data', 'v1'); + await ops.updateCell('t', 1, 'data', 'v2'); + + const tracker = new ModificationTracker(); + tracker.record(cellEdit('bad', 'v0', 'v1', 'set', 'missing_column')); + tracker.record(cellEdit('e2', 'v1', 'v2')); + await tracker.createCheckpoint(); + tracker.stepBack(); + tracker.stepBack(); + + await assert.rejects( + reconcileRestoredDatabase(ops, roundTripTracker(tracker), 'wasm'), + /missing_column|no such column/i + ); + + const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); + assert.strictEqual(r[0].rows[0][0], 'v2'); + }); }); From 5098f8451fefb136ad0fa2b50b81e8dfb08b084e Mon Sep 17 00:00:00 2001 From: zknpr Date: Wed, 3 Jun 2026 17:18:56 +0200 Subject: [PATCH 08/11] feat(undo): revert() restores the saved state via revertDatabaseToSaved Co-Authored-By: Claude Opus 4.8 (1M context) --- src/databaseModel.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/databaseModel.ts b/src/databaseModel.ts index c4305ba..d8401b3 100644 --- a/src/databaseModel.ts +++ b/src/databaseModel.ts @@ -22,7 +22,7 @@ import { getMaximumFileSizeBytes } from './config'; import { GlobalOutputChannel } from './main'; import { ModificationTracker } from './core/undo-history'; -import { reconcileRestoredDatabase } from './core/restore-reconciler'; +import { reconcileRestoredDatabase, revertDatabaseToSaved } from './core/restore-reconciler'; import type { LabeledModification, DatabaseOperations } from './core/types'; import { LoggingDatabaseOperations } from './loggingDatabaseOperations'; @@ -444,10 +444,9 @@ export class DatabaseDocument extends Disposable implements vsc.CustomDocument { */ async revert(cancellation: vsc.CancellationToken): Promise { await this.ensureWritable(); - const uncommitted = this.#modificationTracker.getUncommittedEntries(); - this.#modificationTracker.rollbackToCheckpoint(); - await this.databaseOperations.discardModifications( - uncommitted, + await revertDatabaseToSaved( + this.databaseOperations, + this.#modificationTracker, cancelTokenToAbortSignal(cancellation) ); this.#contentChangeEmitter.fire({}); From dcd45b4fa240726b550f3177b6fb8ac7c983a1f7 Mon Sep 17 00:00:00 2001 From: zknpr Date: Wed, 3 Jun 2026 17:18:56 +0200 Subject: [PATCH 09/11] docs(changelog): fold branch-case + revert hot-exit into 1.5.1 (closes #425) Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2833a2..d00877f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ ### Fixes - **Undo of a JSON cell edit no longer clobbers concurrent changes to other keys.** When a JSON cell was edited as a merge patch, undo now restores only the keys that edit changed (reading the current value and writing the whole object back), preserving a concurrent change to a different key of the same cell. Cells whose value is not a JSON object, and JSON numbers that cannot survive a parse/serialize round-trip (large integers, overflow, high-precision decimals), fall back to an exact whole-value restore. The undo read/compute/write is serialized so a concurrent edit cannot interleave. -- **Hot exit now preserves undone and redo state.** Closing the editor with unsaved changes and reopening previously lost the redo stack, and on the web build could silently drop an undo of an already-saved edit. The redo history is now persisted in the hot-exit backup and the database is reconciled to the exact pre-close state on reopen. +- **Hot exit and Revert now reconstruct the exact undo/redo state, including re-edited undos.** Closing with unsaved changes and reopening (or File: Revert) previously lost the redo stack, could silently drop an undo of an already-saved edit on the web build, and — if you undid a saved edit and then made a new one — failed to rebuild the branched state. The redo history and the abandoned saved edits are now persisted in the hot-exit backup and reconstructed atomically on reopen. Closes #425. ## 1.5.0 From c641c5dade52696189ad2affb6e33d446aa3930f Mon Sep 17 00:00:00 2001 From: zknpr Date: Wed, 3 Jun 2026 19:50:48 +0200 Subject: [PATCH 10/11] fix(undo): drop incompatible restore/revert SAVEPOINT; native redo via redoModification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The atomic-restore SAVEPOINT (9df2bdc) conflicts with the engine's BEGIN-based row/column undos (undoRowDelete -> insertRowBatch, undoColumnDrop): SQLite rejects BEGIN while an outer transaction is open, so wrapping the reconcile/revert sequence broke restoring deleted rows/columns entirely (read-only / failed Revert). Remove the outer SAVEPOINT — engine ops are already individually atomic; a mid-sequence failure now propagates so DatabaseDocument.create opens the document read-only and re-restores from the unchanged backup. Also fix native File>Revert: re-apply saved-undone edits via redoModification (real on both engines) instead of applyModifications, which is a no-op on the native engine and would silently leave a native database at the undone state. Tests: per-op failure propagation (no batch rollback) + native redo path; both revert-proof against re-introducing the SAVEPOINT / applyModifications. 413/413. Addresses Codex P2 review on #434 (restore-reconciler.ts:78, :103). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/restore-reconciler.ts | 91 ++++++++++++--------------- tests/unit/restore-reconciler.test.ts | 51 ++++++++++++++- 2 files changed, 88 insertions(+), 54 deletions(-) diff --git a/src/core/restore-reconciler.ts b/src/core/restore-reconciler.ts index 71d957b..acdc504 100644 --- a/src/core/restore-reconciler.ts +++ b/src/core/restore-reconciler.ts @@ -13,7 +13,7 @@ import type { ModificationTracker } from './undo-history'; * @param databaseOps - Database operation facade for the restored database * @param tracker - Deserialized hot-exit modification tracker * @param engineKind - Active database engine type - * @param signal - Optional cancellation signal used by forward replay + * @param signal - Optional cancellation signal checked between revert/replay steps */ export async function reconcileRestoredDatabase( databaseOps: DatabaseOperations, @@ -31,16 +31,24 @@ export async function reconcileRestoredDatabase( return; } - await runInSavepoint(databaseOps, 'sp_restore', async () => { - // Revert saved entries first to move checkpoint bytes back to the - // common-prefix state, then replay live entries from that boundary. - for (const entry of revertSeq) { - await databaseOps.undoModification(entry); - } - if (forward.length > 0) { - await databaseOps.applyModifications(forward, signal); - } - }); + // Revert saved-then-undone entries first to move the restored bytes back to + // the common-prefix state, then replay the live entries from that boundary. + // + // Each engine operation is individually atomic (it opens its own + // transaction). We deliberately do NOT wrap the sequence in an outer + // SAVEPOINT: row/column undos (`undoRowDelete` -> `insertRowBatch`, + // `undoColumnDrop`) issue their own `BEGIN TRANSACTION`, which SQLite rejects + // while an outer transaction is open — wrapping them would break restoring + // deleted rows/columns entirely. A mid-sequence failure instead propagates to + // the caller (`DatabaseDocument.create`), which opens the document read-only + // and re-restores from the unchanged backup on the next open. + for (const entry of revertSeq) { + signal?.throwIfAborted(); + await databaseOps.undoModification(entry); + } + if (forward.length > 0) { + await databaseOps.applyModifications(forward, signal); + } } /** @@ -48,8 +56,8 @@ export async function reconcileRestoredDatabase( * * Forward entries are undone from the live timeline, then saved-undone entries * are re-applied in original application order. The tracker is rolled back only - * after database mutations commit so a failed revert does not desynchronize the - * in-memory history from the live database. + * after the database mutations succeed so a failed revert does not desynchronize + * the in-memory history from the live database. * * @param databaseOps - Database operation facade for the open database * @param tracker - Modification tracker for the open document @@ -68,45 +76,24 @@ export async function revertDatabaseToSaved( return; } - await runInSavepoint(databaseOps, 'sp_revert', async () => { - // Discard live edits first, then re-apply saved entries that the user had - // undone so the final database bytes match the checkpoint exactly. - if (forward.length > 0) { - await databaseOps.discardModifications(forward, signal); - } - if (redo.length > 0) { - await databaseOps.applyModifications(redo, signal); - } - }); - - tracker.rollbackToCheckpoint(); -} - -/** - * Run database mutations inside a SAVEPOINT and roll back all mutations from - * this helper if any operation in the sequence fails. - */ -async function runInSavepoint( - databaseOps: DatabaseOperations, - prefix: string, - body: () => Promise -): Promise { - if (typeof databaseOps.executeQuery !== 'function') { - // Some unit-test doubles only implement the mutation primitive under test. - // Real DatabaseOperations implementations expose executeQuery and therefore - // take the atomic SAVEPOINT path below. - await body(); - return; + // Discard live edits first, then re-apply the saved entries the user had + // undone so the final database matches the checkpoint. + // + // Re-apply via `redoModification`, NOT `applyModifications`: the native engine + // implements replay in `redoModification` and treats `applyModifications` as a + // no-op (`src/nativeWorker.ts`), so using `applyModifications` here would + // silently leave a native database at the undone state while the tracker is + // marked clean. As in restore, the sequence is per-operation atomic rather + // than wrapped in a SAVEPOINT (row/column undos open their own transaction). + if (forward.length > 0) { + await databaseOps.discardModifications(forward, signal); } - - const name = `${prefix}_${Date.now()}`; - await databaseOps.executeQuery(`SAVEPOINT ${name}`); - try { - await body(); - await databaseOps.executeQuery(`RELEASE ${name}`); - } catch (err) { - await databaseOps.executeQuery(`ROLLBACK TO ${name}`).catch(() => {}); - await databaseOps.executeQuery(`RELEASE ${name}`).catch(() => {}); - throw err; + for (const entry of redo) { + signal?.throwIfAborted(); + await databaseOps.redoModification(entry); } + + // Roll the tracker back to the checkpoint only after the database mutations + // succeed, so a failed revert does not desynchronize history from the data. + tracker.rollbackToCheckpoint(); } diff --git a/tests/unit/restore-reconciler.test.ts b/tests/unit/restore-reconciler.test.ts index 8b64f68..1abedb9 100644 --- a/tests/unit/restore-reconciler.test.ts +++ b/tests/unit/restore-reconciler.test.ts @@ -277,7 +277,7 @@ describe('reconcileRestoredDatabase', () => { assert.strictEqual(tracker.hasUncommittedChanges(), false); }); - it('BC-restore atomicity: rolls back prior reverts when a later revert entry fails', async () => { + it('BC-restore failure: a failing revert propagates and earlier reverts are not batch-rolled-back', async () => { const ops = await freshEngine(); await ops.insertRow('t', { id: 1, data: 'v0' }); await ops.updateCell('t', 1, 'data', 'v1'); @@ -290,12 +290,59 @@ describe('reconcileRestoredDatabase', () => { tracker.stepBack(); tracker.stepBack(); + // The reconcile runs per-operation-atomic engine calls with NO outer + // SAVEPOINT (an outer transaction would conflict with the BEGIN-based + // row/column undos and break restoring deleted rows/columns). So a failing + // entry propagates the error to the caller (DatabaseDocument.create opens + // the document read-only) but does NOT roll back reverts that already + // committed: 'e2' reverts first (v2 -> v1), then 'bad' fails on missing_column. await assert.rejects( reconcileRestoredDatabase(ops, roundTripTracker(tracker), 'wasm'), /missing_column|no such column/i ); + // Earlier revert stayed committed (would be 'v2' if an outer SAVEPOINT had + // rolled the sequence back). This guards against re-introducing that wrap. const r = await ops.executeQuery('SELECT data FROM t WHERE id = 1'); - assert.strictEqual(r[0].rows[0][0], 'v2'); + assert.strictEqual(r[0].rows[0][0], 'v1'); + }); + + it('BC-revert native: re-applies saved-undone edits via redoModification, not the native no-op applyModifications', async () => { + // The native engine implements replay in redoModification and treats + // applyModifications as a no-op (src/nativeWorker.ts). Mimic that contract: + // applyModifications records nothing, redoModification records. If + // revertDatabaseToSaved ever re-applies the redo via applyModifications, a + // native database would silently stay at the undone state — so assert it + // uses redoModification. + const redone: string[] = []; + const applied: string[] = []; + const discarded: string[] = []; + const mockOps = { + applyModifications: async (mods: LabeledModification[]) => { + applied.push(...mods.map((m) => m.label)); + }, + redoModification: async (mod: LabeledModification) => { + redone.push(mod.label); + }, + discardModifications: async (mods: LabeledModification[]) => { + discarded.push(...mods.map((m) => m.label)); + }, + undoModification: async () => {} + } as unknown as DatabaseOperations; + + // Branch: save [e1,e2], undo e2, then record e3 so e2 lands in revertOnRestore. + const tracker = new ModificationTracker(); + tracker.record(cellEdit('e1', 'a', 'b')); + tracker.record(cellEdit('e2', 'b', 'c')); + await tracker.createCheckpoint(); + tracker.stepBack(); + tracker.record(cellEdit('e3', 'b', 'd')); + + await restoreReconciler.revertDatabaseToSaved(mockOps, tracker); + + assert.deepStrictEqual(discarded, ['e3']); // forward edit undone + assert.deepStrictEqual(redone, ['e2']); // saved-undone edit re-applied via redo path + assert.deepStrictEqual(applied, []); // never routed through the native no-op + assert.strictEqual(tracker.hasUncommittedChanges(), false); }); }); From 8a35473e78eb6942d4fb5845772a52a6fdbba852 Mon Sep 17 00:00:00 2001 From: zknpr Date: Wed, 3 Jun 2026 20:33:44 +0200 Subject: [PATCH 11/11] fix(undo): don't invalidate in-flight save checkpoints on branch edits record()'s branch-capture path called invalidateCapturedCheckpointPositions(), which made an in-flight DatabaseDocument.save() skip createCheckpointAt() even when the bytes it wrote still matched the captured common-prefix position. The tracker then kept revertOnRestore describing the pre-save state, so after the save File>Revert could re-apply an already-saved-away edit and hot-exit restore could undo an edit against a file already saved without it (e.g. re-insert an already-present row). The undo (stepBack) that precedes any branch already invalidates saves that predate it; a save started after the undo is writing exactly the common-prefix the branch preserves, so it must be allowed to commit its checkpoint (which clears revertOnRestore to match the on-disk bytes). Remove the redundant branch invalidation. Test BC6 asserts the branch leaves the invalidation revision unchanged and the completing save clears revertOnRestore; revert-proof (fails if the invalidation is restored). 414/414. Addresses Codex P2 review on #434 (undo-history.ts:230). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/undo-history.ts | 11 ++++++++++- tests/unit/undo-history.test.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/core/undo-history.ts b/src/core/undo-history.ts index 93a659c..9a92be3 100644 --- a/src/core/undo-history.ts +++ b/src/core/undo-history.ts @@ -227,7 +227,16 @@ export class ModificationTrackerRevert and hot-exit restore from the saved file + // (Codex P2, #434). } // Subtract size of redo history that we are about to discard diff --git a/tests/unit/undo-history.test.ts b/tests/unit/undo-history.test.ts index b43390c..0c0440b 100644 --- a/tests/unit/undo-history.test.ts +++ b/tests/unit/undo-history.test.ts @@ -114,4 +114,31 @@ describe('ModificationTracker hot-exit persistence', () => { assert.deepStrictEqual(t.getCheckpointRevertSequence(), []); assert.strictEqual(t.hasUncommittedChanges(), false); }); + + it('BC6: a branch does not invalidate an in-flight save started after the undo; the save clears revertOnRestore', async () => { + const t = new ModificationTracker(); + t.record(entry('e1')); + t.record(entry('e2')); + await t.createCheckpoint(); // saved = [e1, e2] + t.stepBack(); // undo e2 -> the async save below writes the [e1] state + + // Capture what an in-flight async save of the undone [e1] state would hold. + const savePosition = t.getCurrentPosition(); + const saveRevision = t.getCheckpointInvalidationRevision(); + + t.record(entry('e3')); // branch while that save is still writing + + // The branch must NOT bump the invalidation revision: the save's bytes ([e1]) + // still match the captured common-prefix position, so save() will commit its + // checkpoint. (Re-adding invalidateCapturedCheckpointPositions() here breaks this.) + assert.strictEqual(t.getCheckpointInvalidationRevision(), saveRevision); + + // Save completes and commits -> clears revertOnRestore so the tracker's saved + // state matches the [e1] bytes on disk, not the stale [e1, e2]. + t.createCheckpointAt(savePosition); + + assert.deepStrictEqual(t.getCheckpointRevertSequence(), []); // e2 no longer saved-undone + assert.deepStrictEqual(t.getUncommittedEntries().map((e) => e.label), ['e3']); + assert.strictEqual(t.hasUncommittedChanges(), true); // e3 still uncommitted + }); });