Fix CREATE operation race condition that overwrote user typing

packages/desktop-app/src/lib/services/shared-node-store.svelte.ts

@@ -58,13 +58,19 @@ interface PendingOperation {
 
 const coordLog = createLogger('PersistenceCoordinator');
 
+/** Pending operation to run after current execution completes */
+interface QueuedOperation {
+  operation: () => Promise<void>;
+  options: { mode: 'immediate' | 'debounce'; dependencies?: Array<string | (() => Promise<void>)> };
+}
+
 class SimplePersistenceCoordinator {
   private static instance: SimplePersistenceCoordinator | null = null;
   private pendingOperations = new Map<string, PendingOperation>();
   private executingOperations = new Set<string>(); // Track in-flight operations
   // Track nodes that need re-persistence after current operation completes
-  // This prevents scheduling multiple operations while one is executing
-  private needsRerun = new Set<string>();
+  // Stores the QUEUED operation so DELETE isn't overwritten by UPDATE re-run
+  private queuedOperations = new Map<string, QueuedOperation>();
   private readonly DEBOUNCE_MS = 500;
   private operationCounter = 0; // For tracking operation IDs
 
@@ -96,15 +102,17 @@ class SimplePersistenceCoordinator {
       `hasPending=${hasPending}, isExecuting=${isExecuting}`
     );
 
-    // If an operation is already executing for this node, just mark it for re-run
+    // If an operation is already executing for this node, queue the new operation
     // This prevents multiple concurrent database operations and OCC conflicts
+    // CRITICAL: We store the NEW operation so DELETE isn't lost when UPDATE is executing
     if (isExecuting) {
-      this.needsRerun.add(nodeId);
+      // Queue the new operation - it will supersede any previously queued operation
+      this.queuedOperations.set(nodeId, { operation, options });
       coordLog.debug(
-        `[op#${opId}] operation already executing for ${shortNodeId}, marked for re-run`
+        `[op#${opId}] operation already executing for ${shortNodeId}, queued new operation (mode=${options.mode})`
       );
       // Return a promise that resolves when the current operation completes
-      // The re-run will happen automatically after completion
+      // The queued operation will run automatically after completion
       const existingPending = this.pendingOperations.get(nodeId);
       if (existingPending) {
         return { promise: existingPending.promise };
@@ -154,14 +162,17 @@ class SimplePersistenceCoordinator {
         this.pendingOperations.delete(nodeId);
         this.executingOperations.delete(nodeId);
 
-        // Check if we need to re-run for this node (content changed while executing)
-        if (this.needsRerun.has(nodeId)) {
-          this.needsRerun.delete(nodeId);
-          coordLog.debug(`[op#${opId}] re-running persistence for ${shortNodeId} (content changed during execution)`);
-          // Schedule a new debounced operation - it will read current state
-          // Use setTimeout to avoid stack overflow from immediate recursion
+        // Check if there's a queued operation for this node (e.g., DELETE queued during UPDATE)
+        const queued = this.queuedOperations.get(nodeId);
+        if (queued) {
+          this.queuedOperations.delete(nodeId);
+          coordLog.debug(
+            `[op#${opId}] executing queued operation for ${shortNodeId} (mode=${queued.options.mode})`
+          );
+          // Execute the queued operation - use setTimeout to avoid stack overflow
+          // CRITICAL: Use the QUEUED operation, not the original one
           setTimeout(() => {
-            this.persist(nodeId, operation, { mode: 'debounce' });
+            this.persist(nodeId, queued.operation, queued.options);
           }, 0);
         }
       }
@@ -1154,8 +1165,14 @@ export class SharedNodeStore {
                 // This prevents version conflicts on subsequent updates
                 const createdNode = await tauriCommands.getNode(nodeId);
                 if (createdNode) {
-                  currentNode.version = createdNode.version;
-                  this.nodes.set(nodeId, currentNode); // Update local node with backend version
+                  // BUG FIX: Only update the VERSION, not the entire node!
+                  // The user may have continued typing while createNode was in flight.
+                  // We must preserve their local changes and only take the version from backend.
+                  const latestLocalNode = this.nodes.get(nodeId);
+                  if (latestLocalNode) {
+                    latestLocalNode.version = createdNode.version;
+                    this.nodes.set(nodeId, latestLocalNode);
+                  }
                 }
               }
             } catch (dbError) {
@@ -2494,8 +2511,14 @@ export class SharedNodeStore {
               // This prevents version conflicts on subsequent updates
               const createdNode = await tauriCommands.getNode(nodeId);
               if (createdNode) {
-                finalNode.version = createdNode.version;
-                this.nodes.set(nodeId, finalNode); // Update local node with backend version
+                // BUG FIX: Only update the VERSION, not the entire node!
+                // The user may have continued typing while createNode was in flight.
+                // We must preserve their local changes and only take the version from backend.
+                const latestLocalNode = this.nodes.get(nodeId);
+                if (latestLocalNode) {
+                  latestLocalNode.version = createdNode.version;
+                  this.nodes.set(nodeId, latestLocalNode);
+                }
               }
             } catch (createError) {
               // If CREATE fails (node already exists from race), try UPDATE with batched changes

packages/desktop-app/src/tests/services/shared-node-store-coverage.test.ts

@@ -621,6 +621,195 @@ describe('SharedNodeStore - Coverage Completion', () => {
     });
   });
 
+  // ========================================================================
+  // CREATE Operation Race Condition (Issue: content overwritten mid-typing)
+  // ========================================================================
+
+  describe('CREATE Operation - User Typing Race Condition', () => {
+    /**
+     * REGRESSION TEST: Content overwritten during CREATE operation
+     *
+     * Bug scenario:
+     * 1. User creates new node with content "## " (header)
+     * 2. User deletes "## " (becomes text node)
+     * 3. User presses Tab to indent
+     * 4. User starts typing "Hello World"
+     * 5. CREATE operation completes and fetches version from backend
+     * 6. BUG: nodes.set() was called with stale `currentNode` captured at step 1
+     * 7. User's typing ("Hello World") was overwritten with stale content ("## ")
+     *
+     * Fix: Only update the VERSION from backend, preserve local content
+     */
+    it('should preserve user typing when CREATE completes (setNode path)', async () => {
+      let createNodeResolve: (value: string) => void;
+      const createNodePromise = new Promise<string>((resolve) => {
+        createNodeResolve = resolve;
+      });
+
+      // Mock createNode to delay so we can simulate user typing during the operation
+      vi.spyOn(tauriCommands, 'createNode').mockImplementation(async () => {
+        return createNodePromise;
+      });
+
+      vi.spyOn(tauriCommands, 'getNode').mockResolvedValue({
+        ...mockNode,
+        id: 'typing-race-node',
+        content: '## ', // Backend has OLD content
+        nodeType: 'header',
+        version: 1
+      });
+
+      // Step 1: Create new node with initial content (NOT persisted yet)
+      // The node is new because persistedNodeIds doesn't have it
+      const newNode = {
+        ...mockNode,
+        id: 'typing-race-node',
+        content: '## ',
+        nodeType: 'header' as const,
+        version: 0
+      };
+      store.setNode(newNode, viewerSource); // Will trigger CREATE persistence
+
+      // Step 2: Simulate user typing WHILE createNode is in flight
+      // This updates the store directly (as the UI would via updateNode)
+      // The store update happens immediately, but CREATE is still pending
+      await new Promise(resolve => setTimeout(resolve, 10));
+
+      // Directly modify the store's node to simulate what the UI does
+      // (updateNode would queue its own persistence which complicates the test)
+      const nodeInStore = store.getNode('typing-race-node');
+      if (nodeInStore) {
+        nodeInStore.content = 'Hello World';
+        nodeInStore.nodeType = 'text';
+        store.nodes.set('typing-race-node', nodeInStore);
+      }
+
+      // Verify store has user's typed content
+      const beforeCreate = store.getNode('typing-race-node');
+      expect(beforeCreate?.content).toBe('Hello World');
+      expect(beforeCreate?.nodeType).toBe('text');
+
+      // Step 3: Now complete the CREATE operation
+      createNodeResolve!('typing-race-node');
+
+      // Wait for persistence to complete (debounce is 500ms for new viewer nodes)
+      await new Promise(resolve => setTimeout(resolve, 700));
+
+      // Step 4: CRITICAL CHECK - User's content MUST be preserved!
+      const afterCreate = store.getNode('typing-race-node');
+      expect(afterCreate?.content).toBe('Hello World'); // NOT '## '!
+      expect(afterCreate?.nodeType).toBe('text'); // NOT 'header'!
+      expect(afterCreate?.version).toBe(1); // Version updated from backend
+    });
+
+    it('should preserve user typing when batch CREATE completes (commitBatch path)', async () => {
+      let createNodeResolve: (value: string) => void;
+      const createNodePromise = new Promise<string>((resolve) => {
+        createNodeResolve = resolve;
+      });
+
+      vi.spyOn(tauriCommands, 'createNode').mockImplementation(async () => {
+        return createNodePromise;
+      });
+
+      vi.spyOn(tauriCommands, 'getNode').mockResolvedValue({
+        ...mockNode,
+        id: 'batch-typing-race',
+        content: '> Quote', // Backend has OLD content from batch
+        nodeType: 'quote-block',
+        version: 1
+      });
+
+      // Step 1: Create node with batch mode (quote-block uses batching)
+      const quoteNode = {
+        ...mockNode,
+        id: 'batch-typing-race',
+        content: '> Quote',
+        nodeType: 'quote-block' as const,
+        version: 0
+      };
+      store.setNode(quoteNode, viewerSource, true);
+
+      // Start a batch operation
+      store.startBatch('batch-typing-race');
+      store.addToBatch('batch-typing-race', { content: '> Original batch content' });
+      store.commitBatch('batch-typing-race');
+
+      // Step 2: User types more content while batch is processing
+      await new Promise(resolve => setTimeout(resolve, 10));
+      store.updateNode('batch-typing-race', {
+        content: '> User typed this after batch'
+      }, viewerSource);
+
+      // Verify store has user's typed content
+      const beforeComplete = store.getNode('batch-typing-race');
+      expect(beforeComplete?.content).toBe('> User typed this after batch');
+
+      // Step 3: Complete the CREATE operation
+      createNodeResolve!('batch-typing-race');
+
+      // Wait for persistence to complete
+      await new Promise(resolve => setTimeout(resolve, 100));
+
+      // Step 4: CRITICAL CHECK - User's content MUST be preserved!
+      const afterComplete = store.getNode('batch-typing-race');
+      expect(afterComplete?.content).toBe('> User typed this after batch');
+      expect(afterComplete?.version).toBe(1);
+    });
+
+    it('should handle rapid typing with multiple content changes during CREATE', async () => {
+      let createNodeResolve: (value: string) => void;
+      const createNodePromise = new Promise<string>((resolve) => {
+        createNodeResolve = resolve;
+      });
+
+      vi.spyOn(tauriCommands, 'createNode').mockImplementation(async () => {
+        return createNodePromise;
+      });
+
+      vi.spyOn(tauriCommands, 'getNode').mockResolvedValue({
+        ...mockNode,
+        id: 'rapid-typing',
+        content: 'A',
+        version: 1
+      });
+
+      // Create initial node (will trigger CREATE persistence)
+      const node = { ...mockNode, id: 'rapid-typing', content: 'A', version: 0 };
+      store.setNode(node, viewerSource);
+
+      // Simulate rapid typing - directly modify store content
+      // This simulates what happens when user types faster than persistence
+      await new Promise(resolve => setTimeout(resolve, 5));
+      const nodeInStore = store.getNode('rapid-typing');
+      if (nodeInStore) {
+        nodeInStore.content = 'AB';
+        store.nodes.set('rapid-typing', nodeInStore);
+        await new Promise(resolve => setTimeout(resolve, 5));
+        nodeInStore.content = 'ABC';
+        store.nodes.set('rapid-typing', nodeInStore);
+        await new Promise(resolve => setTimeout(resolve, 5));
+        nodeInStore.content = 'ABCD';
+        store.nodes.set('rapid-typing', nodeInStore);
+        await new Promise(resolve => setTimeout(resolve, 5));
+        nodeInStore.content = 'ABCDE';
+        store.nodes.set('rapid-typing', nodeInStore);
+      }
+
+      // Verify latest content
+      expect(store.getNode('rapid-typing')?.content).toBe('ABCDE');
+
+      // Complete CREATE
+      createNodeResolve!('rapid-typing');
+      await new Promise(resolve => setTimeout(resolve, 700)); // debounce is 500ms
+
+      // Final content must be the LATEST user typed, not initial 'A'
+      const final = store.getNode('rapid-typing');
+      expect(final?.content).toBe('ABCDE');
+      expect(final?.version).toBe(1);
+    });
+  });
+
   // ========================================================================
   // Concurrent Resync Prevention
   // ========================================================================