fix(undo): hot-exit + Revert reconstruct exact undo/redo state (closes #425, 1.5.1)

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 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
 
 ### Features

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": {

src/core/restore-reconciler.ts

@@ -0,0 +1,99 @@
+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 checked between revert/replay steps
+ */
+export async function reconcileRestoredDatabase(
+  databaseOps: DatabaseOperations,
+  tracker: ModificationTracker<LabeledModification>,
+  engineKind: 'wasm' | 'native',
+  signal?: AbortSignal
+): Promise<void> {
+  if (engineKind === 'native') {
+    return;
+  }
+
+  const revertSeq = tracker.getCheckpointRevertSequence();
+  const forward = tracker.getUncommittedEntries();
+  if (revertSeq.length === 0 && forward.length === 0) {
+    return;
+  }
+
+  // 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);
+  }
+}
+
+/**
+ * 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 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
+ * @param signal - Optional cancellation signal used by replay/discard helpers
+ */
+export async function revertDatabaseToSaved(
+  databaseOps: DatabaseOperations,
+  tracker: ModificationTracker<LabeledModification>,
+  signal?: AbortSignal
+): Promise<void> {
+  const forward = tracker.getUncommittedEntries();
+  const redo = [...tracker.getCheckpointRevertSequence()].reverse();
+
+  if (forward.length === 0 && redo.length === 0) {
+    tracker.rollbackToCheckpoint();
+    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);
+  }
+  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();
+}

src/core/undo-history.ts

@@ -147,6 +147,19 @@ export class ModificationTracker<T extends LabeledModification = LabeledModifica
 
   private futureStack: T[] = [];
   private futureStackSizes: number[] = [];
+  /**
+   * Saved entries that were undone and then removed from redo history by a new
+   * branch edit. They are stored in database revert order so hot-exit restore
+   * can move checkpoint bytes back to the common-prefix state before replaying
+   * live forward edits.
+   */
+  private revertOnRestore: T[] = [];
+  /**
+   * Memory sizes for entries stored in revertOnRestore. These entries remain
+   * counted in currentSize because they are still required for restore/revert
+   * correctness even after they leave futureStack.
+   */
+  private revertOnRestoreSizes: number[] = [];
 
   private checkpointIndex: number = 0;
   private maxEntries: number;
@@ -202,6 +215,30 @@ export class ModificationTracker<T extends LabeledModification = LabeledModifica
     // Calculate size of new entry
     const entrySize = calculateSize(entry);
 
+    const savedUndoneCount = this.checkpointIndex - this.timeline.length;
+    if (savedUndoneCount > 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;
+
+      // Intentionally do NOT invalidate captured save positions here. Any
+      // in-flight save that predates this undo+branch was already invalidated by
+      // stepBack() (line ~285); a save that started *after* the undo is writing
+      // exactly this common-prefix state, so it must be allowed to commit its
+      // checkpoint — createCheckpointAt() then clears revertOnRestore to match
+      // the bytes now on disk. Invalidating here would make save() skip that
+      // checkpoint and leave revertOnRestore describing the pre-save state,
+      // desyncing File>Revert and hot-exit restore from the saved file
+      // (Codex P2, #434).
+    }
+
     // 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,14 +320,17 @@ export class ModificationTracker<T extends LabeledModification = LabeledModifica
    * @returns True if timeline differs from checkpoint
    */
   hasUncommittedChanges(): boolean {
-    return this.timeline.length !== this.checkpointIndex;
+    return this.timeline.length !== this.checkpointIndex || this.revertOnRestore.length > 0;
   }
 
   /**
    * Mark current position as checkpoint (saved state).
    */
   async createCheckpoint(): Promise<void> {
     this.checkpointIndex = this.timeline.length;
+    this.currentSize -= this.revertOnRestoreSizes.reduce((a, b) => a + b, 0);
+    this.revertOnRestore = [];
+    this.revertOnRestoreSizes = [];
   }
 
   /**
@@ -332,6 +372,9 @@ export class ModificationTracker<T extends LabeledModification = LabeledModifica
   createCheckpointAt(position: number): void {
     const relativePosition = position - this.timelineOffset;
     this.checkpointIndex = Math.max(0, Math.min(this.timeline.length, relativePosition));
+    this.currentSize -= this.revertOnRestoreSizes.reduce((a, b) => a + b, 0);
+    this.revertOnRestore = [];
+    this.revertOnRestoreSizes = [];
   }
 
   /**
@@ -343,18 +386,86 @@ export class ModificationTracker<T extends LabeledModification = LabeledModifica
     return this.timeline.slice(this.checkpointIndex);
   }
 
+  /**
+   * Get saved checkpoint entries that were undone after the last save.
+   *
+   * A hot-exit restore opens WASM bytes at the checkpoint state. When the live
+   * timeline is behind that checkpoint, the restored database must undo the
+   * saved entries that are now stored on the redo stack. The returned entries
+   * are ordered exactly as they must be reverted to replay the user's undo
+   * sequence against checkpoint bytes.
+   *
+   * @returns Saved-then-undone entries in database revert order
+   */
+  getEntriesUndoneSinceCheckpoint(): T[] {
+    const delta = this.checkpointIndex - this.timeline.length;
+    if (delta <= 0) {
+      return [];
+    }
+    // Clamp defensively: a corrupted or hand-edited backup could make delta exceed
+    // futureStack.length, where a negative slice start would silently drop entries.
+    return this.futureStack.slice(Math.max(0, this.futureStack.length - delta));
+  }
+
+  /**
+   * Get all saved checkpoint entries that restore/revert must undo first.
+   *
+   * The returned sequence is ordered newest-first for database undo. It combines
+   * entries already captured from abandoned branches with entries that are still
+   * available on the redo stack in the base undo-saved case.
+   *
+   * @returns Saved entries in database revert order
+   */
+  getCheckpointRevertSequence(): T[] {
+    return [...this.revertOnRestore, ...this.getEntriesUndoneSinceCheckpoint()];
+  }
+
   /**
    * Rollback to the last checkpoint.
    * Moves uncommitted modifications to redo stack.
    */
   rollbackToCheckpoint(): void {
+    const savedUndoneCount = Math.max(0, this.checkpointIndex - this.timeline.length);
+    const savedUndoneStart = Math.max(0, this.futureStack.length - savedUndoneCount);
+    const savedUndone = this.futureStack.slice(savedUndoneStart);
+    const savedUndoneSizes = this.futureStackSizes.slice(savedUndoneStart);
+    let changed = false;
+
+    if (savedUndone.length > 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);
       const uncommittedSizes = this.timelineSizes.splice(this.checkpointIndex);
 
       this.futureStack.push(...uncommitted.reverse());
       this.futureStackSizes.push(...uncommittedSizes.reverse());
+      changed = true;
+    }
+
+    const restoreSequence = [...this.revertOnRestore, ...savedUndone];
+    if (restoreSequence.length > 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();
     }
   }
@@ -368,7 +479,9 @@ export class ModificationTracker<T extends LabeledModification = LabeledModifica
   serialize(): Uint8Array {
     const payload = {
       timeline: this.timeline,
-      checkpointIndex: this.checkpointIndex
+      checkpointIndex: this.checkpointIndex,
+      futureStack: this.futureStack,
+      revertOnRestore: this.revertOnRestore
     };
     // Use binaryReplacer to properly serialize Uint8Array values
     const jsonStr = JSON.stringify(payload, binaryReplacer);
@@ -396,10 +509,19 @@ export class ModificationTracker<T extends LabeledModification = LabeledModifica
     const tracker = new ModificationTracker<T>(maxEntries, maxMemory);
     tracker.timeline = payload.timeline || [];
     tracker.checkpointIndex = payload.checkpointIndex || 0;
+    tracker.futureStack = payload.futureStack || [];
+    tracker.revertOnRestore = payload.revertOnRestore || [];
 
-    // Recalculate sizes
+    // Recalculate sizes for active timeline entries, redo entries, and captured
+    // branch-revert 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.revertOnRestoreSizes = tracker.revertOnRestore.map(calculateSize);
+    tracker.currentSize =
+      tracker.timelineSizes.reduce((a, b) => a + b, 0) +
+      tracker.futureStackSizes.reduce((a, b) => a + b, 0) +
+      tracker.revertOnRestoreSizes.reduce((a, b) => a + b, 0);
 
     return tracker;
   }

src/databaseModel.ts

@@ -22,6 +22,7 @@ import { getMaximumFileSizeBytes } from './config';
 import { GlobalOutputChannel } from './main';
 
 import { ModificationTracker } from './core/undo-history';
+import { reconcileRestoredDatabase, revertDatabaseToSaved } 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) {
@@ -439,10 +444,9 @@ export class DatabaseDocument extends Disposable implements vsc.CustomDocument {
    */
   async revert(cancellation: vsc.CancellationToken): Promise<void> {
     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({});