fix(web): harden web edit against data loss — 3 P1 fixes (#424 review)

Codex review found 3 confirmed data-integrity bugs that made web editing
unsafe. All fixed:

P1-A — WAL pages dropped on save. sql.js opens only the main DB image and
cannot merge a separate -wal, so saving a WAL-mode DB would overwrite the main
file and drop committed WAL pages. Now: when walContent is non-empty, the
browser connection opens read-only (viewable, not silently corruptible).

P1-B — undo/redo/revert/hot-exit were no-ops. The in-process facade wired
applyModifications/undoModification/redoModification/flushChanges/
discardModifications as async()=>{}, so VS Code Undo/Revert/backup-restore
marked document state handled while the in-memory DB was unchanged (zombie/lost
edits). createWorkerEndpoint() now exposes these ops, and the facade delegates
to the already-implemented WasmDatabaseEngine methods.

P1-C — checkpoint raced an in-flight save. createCheckpoint() ran after the
async writeFile, marking clean any edit that arrived during the write (dropped
on reload). ModificationTracker gains getCurrentPosition()/createCheckpointAt();
save() snapshots the position right after serializeDatabase() and only commits
that snapshot after a successful write. Position is absolute (timelineOffset)
so it survives front-eviction during the write.

Tests +3, each revert-proof-verified (fail when its fix is undone):
WAL-read-only, undo-delegation, save-race-stays-dirty.

Verified: build OK (extension-browser.js exports activate, worker-free);
tsc --noEmit clean; npm test 341/341. Desktop path unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: zknpr

src/core/sqlite-db.ts

@@ -21,7 +21,8 @@ import type {
   TableCountOptions,
   SchemaSnapshot,
   ColumnMetadata,
-  ColumnDefinition
+  ColumnDefinition,
+  ModificationEntry
 } from './types';
 import { getNodeFs } from './platform/fs';
 import {
@@ -174,6 +175,28 @@ export function createWorkerEndpoint() {
       return requireEngine().serializeDatabase(name);
     },
 
+    // Expose undo/history operations for the browser in-process facade, which
+    // calls this endpoint directly instead of going through worker RPC.
+    async applyModifications(mods: ModificationEntry[], signal?: AbortSignal): Promise<void> {
+      return requireEngine().applyModifications(mods, signal);
+    },
+
+    async undoModification(mod: ModificationEntry): Promise<void> {
+      return requireEngine().undoModification(mod);
+    },
+
+    async redoModification(mod: ModificationEntry): Promise<void> {
+      return requireEngine().redoModification(mod);
+    },
+
+    async flushChanges(signal?: AbortSignal): Promise<void> {
+      return requireEngine().flushChanges(signal);
+    },
+
+    async discardModifications(mods: ModificationEntry[], signal?: AbortSignal): Promise<void> {
+      return requireEngine().discardModifications(mods, signal);
+    },
+
     async updateCell(table: string, rowId: RecordId, column: string, value: CellValue): Promise<void> {
       return requireEngine().updateCell(table, rowId, column, value);
     },

src/core/undo-history.ts

@@ -142,6 +142,8 @@ interface TrackerState<T> {
 export class ModificationTracker<T extends LabeledModification = LabeledModification> {
   private timeline: T[] = [];
   private timelineSizes: number[] = [];
+  /** Number of entries that were evicted from the front of the retained timeline. */
+  private timelineOffset: number = 0;
 
   private futureStack: T[] = [];
   private futureStackSizes: number[] = [];
@@ -208,6 +210,7 @@ export class ModificationTracker<T extends LabeledModification = LabeledModifica
 
       // Adjust checkpoint index since we shifted the array
       this.checkpointIndex = Math.max(0, this.checkpointIndex - 1);
+      this.timelineOffset++;
     }
   }
 
@@ -261,6 +264,27 @@ export class ModificationTracker<T extends LabeledModification = LabeledModifica
     this.checkpointIndex = this.timeline.length;
   }
 
+  /**
+   * Return the absolute timeline position after the latest retained entry.
+   *
+   * This position is stable across later front-eviction because it includes the
+   * count of entries already removed from the retained timeline.
+   */
+  getCurrentPosition(): number {
+    return this.timelineOffset + this.timeline.length;
+  }
+
+  /**
+   * Mark a previously captured absolute timeline position as the saved state.
+   *
+   * The position is translated back into the retained timeline, clamped when
+   * old entries were evicted or when callers provide a future position.
+   */
+  async createCheckpointAt(position: number): Promise<void> {
+    const relativePosition = position - this.timelineOffset;
+    this.checkpointIndex = Math.max(0, Math.min(this.timeline.length, relativePosition));
+  }
+
   /**
    * Get all modifications since last checkpoint.
    *

src/databaseModel.ts

@@ -357,6 +357,10 @@ export class DatabaseDocument extends Disposable implements vsc.CustomDocument {
 
     const { filename } = this.fileParts;
     const binaryContent = await this.databaseOperations.serializeDatabase(filename);
+    // Capture the tracker position immediately after serialization. The bytes
+    // below represent edits up to this position only; edits recorded while the
+    // asynchronous workspace write is pending must remain dirty.
+    const serializedCheckpoint = this.#modificationTracker.getCurrentPosition();
     try {
       await vsc.workspace.fs.writeFile(this.uri, binaryContent);
     } catch (err) {
@@ -365,7 +369,9 @@ export class DatabaseDocument extends Disposable implements vsc.CustomDocument {
     }
     // Only mark the tracker clean after bytes are persisted. If a web filesystem
     // rejects writeFile, the edit history remains uncommitted for backup/retry.
-    await this.#modificationTracker.createCheckpoint();
+    // The saved checkpoint is limited to the serialized snapshot so concurrent
+    // edits are not acknowledged before their bytes reach storage.
+    await this.#modificationTracker.createCheckpointAt(serializedCheckpoint);
   }
 
   /**

src/workerFactory.ts

@@ -214,6 +214,11 @@ async function createInProcessWasmDatabaseConnection(
       // There is no file-path fast path because the web extension host cannot
       // access local disk paths directly.
       const [dbContent, walContent] = await loadDatabaseFiles(fileUri);
+      const hasActiveWal = !!walContent && walContent.byteLength > 0;
+      // sql.js opens one main database image and cannot merge a separate WAL
+      // file, so browser editing is disabled when committed WAL pages may be
+      // absent from the main database bytes that save() would later overwrite.
+      const readOnlyMode = (forceReadOnly ?? false) || hasActiveWal;
 
       // Preload sql.js WASM bytes from the extension assets directory so
       // WebAssembly instantiation does not depend on worker-relative URLs.
@@ -226,7 +231,7 @@ async function createInProcessWasmDatabaseConnection(
         maxSize: getMaximumFileSizeBytes(),
         resourceMap: {},
         wasmBinary: wasmContent,
-        readOnlyMode: forceReadOnly ?? false,
+        readOnlyMode,
         queryTimeout: getQueryTimeout()
       };
 
@@ -237,11 +242,16 @@ async function createInProcessWasmDatabaseConnection(
         executeQuery: (sql: string, params?: CellValue[]) =>
           endpoint.runQuery(sql, params),
         serializeDatabase: (name: string) => endpoint.exportDatabase(name),
-        applyModifications: async () => {},
-        undoModification: async () => {},
-        redoModification: async () => {},
-        flushChanges: async () => {},
-        discardModifications: async () => {},
+        applyModifications: (mods: ModificationEntry[], signal?: AbortSignal) =>
+          endpoint.applyModifications(mods, signal),
+        undoModification: (mod: ModificationEntry) =>
+          endpoint.undoModification(mod),
+        redoModification: (mod: ModificationEntry) =>
+          endpoint.redoModification(mod),
+        flushChanges: (signal?: AbortSignal) =>
+          endpoint.flushChanges(signal),
+        discardModifications: (mods: ModificationEntry[], signal?: AbortSignal) =>
+          endpoint.discardModifications(mods, signal),
         updateCell: (table: string, rowId: string | number, column: string, value: CellValue) =>
           endpoint.updateCell(table, rowId, column, value),
         insertRow: (table: string, data: Record<string, CellValue>) =>

tests/unit/databaseModel.test.ts

@@ -491,6 +491,78 @@ describe('DatabaseDocument save/saveAs fallback', () => {
             mockVscode.workspace.fs = originalFs;
         }
     });
+
+    it('save: does not checkpoint edits recorded while writeFile is still pending', async () => {
+        const sourceUri = createUri('vscode-vfs', '/github/user/repo/test.db');
+        let resolveWrite: () => void = () => {};
+        let markWriteStarted: () => void = () => {};
+        const writeMayFinish = new Promise<void>(resolve => {
+            resolveWrite = resolve;
+        });
+        const writeStarted = new Promise<void>(resolve => {
+            markWriteStarted = resolve;
+        });
+
+        const firstModification = {
+            label: 'First Update',
+            description: 'Update first item',
+            modificationType: 'cell_update' as const,
+            targetTable: 'items',
+            targetRowId: 1,
+            targetColumn: 'name',
+            priorValue: 'before',
+            newValue: 'after'
+        };
+        const concurrentModification = {
+            label: 'Concurrent Update',
+            description: 'Update concurrent item',
+            modificationType: 'cell_update' as const,
+            targetTable: 'items',
+            targetRowId: 2,
+            targetColumn: 'name',
+            priorValue: 'old',
+            newValue: 'new'
+        };
+        let discardedModifications: unknown[] | undefined;
+
+        const dbOps = {
+            engineKind: Promise.resolve('wasm'),
+            serializeDatabase: async () => new Uint8Array([7, 8, 9]),
+            discardModifications: async (mods: unknown[]) => {
+                discardedModifications = mods;
+            }
+        };
+
+        const doc = createDocBypassingFactory(dbOps, sourceUri);
+        doc.recordModification(firstModification);
+
+        const originalFs = mockVscode.workspace.fs;
+        mockVscode.workspace.fs = {
+            ...originalFs,
+            writeFile: async (uri: any, content: any) => {
+                assert.strictEqual(uri, sourceUri);
+                assert.deepStrictEqual(content, new Uint8Array([7, 8, 9]));
+                markWriteStarted();
+                await writeMayFinish;
+            },
+            readFile: async () => new Uint8Array([])
+        } as any;
+
+        try {
+            const savePromise = doc.save();
+            await writeStarted;
+
+            doc.recordModification(concurrentModification);
+            resolveWrite();
+            await savePromise;
+
+            await doc.revert(undefined);
+
+            assert.deepStrictEqual(discardedModifications, [concurrentModification]);
+        } finally {
+            mockVscode.workspace.fs = originalFs;
+        }
+    });
 });
 
 

tests/unit/workerFactory_browser.test.ts

@@ -7,7 +7,7 @@ import fs from 'node:fs';
 import Module from 'node:module';
 import esbuild from 'esbuild';
 import { mockVscode } from './mocks/vscode';
-import type { CellUpdate, CellValue, DatabaseInitConfig } from '../../src/core/types';
+import type { CellUpdate, CellValue, DatabaseInitConfig, ModificationEntry } from '../../src/core/types';
 
 const workerFactoryPath = path.resolve(__dirname, '../../src/workerFactory.ts');
 const workerFactorySource = fs.readFileSync(workerFactoryPath, 'utf8');
@@ -16,6 +16,11 @@ interface FakeEndpoint {
   initializeDatabase(filename: string, config: DatabaseInitConfig): Promise<{ isReadOnly: boolean }>;
   runQuery(sql: string, params?: CellValue[]): Promise<unknown[]>;
   exportDatabase(name: string): Promise<Uint8Array>;
+  applyModifications?(mods: ModificationEntry[], signal?: AbortSignal): Promise<void>;
+  undoModification?(mod: ModificationEntry): Promise<void>;
+  redoModification?(mod: ModificationEntry): Promise<void>;
+  flushChanges?(signal?: AbortSignal): Promise<void>;
+  discardModifications?(mods: ModificationEntry[], signal?: AbortSignal): Promise<void>;
   updateCell(table: string, rowId: string | number, column: string, value: CellValue): Promise<void>;
   insertRow(table: string, data: Record<string, CellValue>): Promise<string | number | undefined>;
   updateCellBatch(table: string, updates: CellUpdate[]): Promise<void>;
@@ -167,4 +172,133 @@ describe('workerFactory browser WASM connection', () => {
     assert.strictEqual(insertRowValue, blobValue);
     assert.strictEqual(updateBatchValue, blobValue);
   });
+
+  it('opens browser WAL databases read-only instead of silently writing without WAL pages', async () => {
+    const dbContent = new Uint8Array([1, 2, 3]);
+    const walContent = new Uint8Array([9, 9, 9]);
+    const wasmContent = new Uint8Array([4, 5, 6]);
+    let initConfig: DatabaseInitConfig | undefined;
+
+    Object.defineProperty(mockVscode.workspace, 'fs', {
+      value: {
+        stat: async () => ({ size: dbContent.byteLength }),
+        readFile: async (uri: { path?: string; fsPath?: string }) => {
+          const pathValue = uri.path ?? uri.fsPath ?? '';
+          if (pathValue.endsWith('-wal')) {
+            return walContent;
+          }
+          if (pathValue.endsWith('sqlite3.wasm')) {
+            return wasmContent;
+          }
+          return dbContent;
+        }
+      },
+      writable: true,
+      configurable: true
+    });
+
+    const endpoint: FakeEndpoint = {
+      initializeDatabase: async (_filename, config) => {
+        initConfig = config;
+        return { isReadOnly: config.readOnlyMode ?? false };
+      },
+      runQuery: async () => [],
+      exportDatabase: async () => new Uint8Array(),
+      updateCell: async () => {},
+      insertRow: async () => 1,
+      updateCellBatch: async () => {},
+      ping: async () => true
+    };
+
+    const workerFactory = loadBrowserWorkerFactory(endpoint);
+    const extensionUri = { scheme: 'vscode-vfs', fsPath: '/ext', path: '/ext' } as any;
+    const fileUri = {
+      scheme: 'vscode-vfs',
+      fsPath: '/workspace/test.db',
+      path: '/workspace/test.db',
+      with: ({ path: nextPath }: { path: string }) => ({
+        scheme: 'vscode-vfs',
+        fsPath: nextPath,
+        path: nextPath
+      })
+    } as any;
+
+    const bundle = await workerFactory.createDatabaseConnection(extensionUri, null as any);
+    const connection = await bundle.establishConnection(fileUri, 'test.db');
+
+    assert.strictEqual(initConfig?.walContent, walContent);
+    assert.strictEqual(initConfig?.readOnlyMode, true);
+    assert.strictEqual(connection.isReadOnly, true);
+  });
+
+  it('delegates in-process modification operations through the endpoint', async () => {
+    const calls: string[] = [];
+    const mod = {
+      label: 'Update',
+      description: 'Update item',
+      modificationType: 'cell_update' as const,
+      targetTable: 'items',
+      targetRowId: 1,
+      targetColumn: 'name',
+      priorValue: 'before',
+      newValue: 'after'
+    };
+    const abortController = new AbortController();
+
+    const endpoint: FakeEndpoint = {
+      initializeDatabase: async () => ({ isReadOnly: false }),
+      runQuery: async () => [],
+      exportDatabase: async () => new Uint8Array(),
+      applyModifications: async (mods, signal) => {
+        assert.deepStrictEqual(mods, [mod]);
+        assert.strictEqual(signal, abortController.signal);
+        calls.push('apply');
+      },
+      undoModification: async (entry) => {
+        assert.strictEqual(entry, mod);
+        calls.push('undo');
+      },
+      redoModification: async (entry) => {
+        assert.strictEqual(entry, mod);
+        calls.push('redo');
+      },
+      flushChanges: async (signal) => {
+        assert.strictEqual(signal, abortController.signal);
+        calls.push('flush');
+      },
+      discardModifications: async (mods, signal) => {
+        assert.deepStrictEqual(mods, [mod]);
+        assert.strictEqual(signal, abortController.signal);
+        calls.push('discard');
+      },
+      updateCell: async () => {},
+      insertRow: async () => 1,
+      updateCellBatch: async () => {},
+      ping: async () => true
+    };
+
+    const workerFactory = loadBrowserWorkerFactory(endpoint);
+    const extensionUri = { scheme: 'vscode-vfs', fsPath: '/ext', path: '/ext' } as any;
+    const fileUri = {
+      scheme: 'vscode-vfs',
+      fsPath: '/workspace/test.db',
+      path: '/workspace/test.db',
+      with: ({ path: nextPath }: { path: string }) => ({
+        scheme: 'vscode-vfs',
+        fsPath: nextPath,
+        path: nextPath
+      })
+    } as any;
+
+    const bundle = await workerFactory.createDatabaseConnection(extensionUri, null as any);
+    const connection = await bundle.establishConnection(fileUri, 'test.db');
+
+    await connection.databaseOps.applyModifications([mod], abortController.signal);
+    await connection.databaseOps.undoModification(mod);
+    await connection.databaseOps.redoModification(mod);
+    await connection.databaseOps.flushChanges(abortController.signal);
+    await connection.databaseOps.discardModifications([mod], abortController.signal);
+
+    assert.deepStrictEqual(calls, ['apply', 'undo', 'redo', 'flush', 'discard']);
+  });
 });