🚨 CRITICAL: Fix data loss bug when lastSyncAt is null

lib/sync/engine.ts

@@ -276,13 +276,18 @@ export class SyncEngine {
           console.log(`[SYNC DEBUG] Found local task: ${localTask.title} (updated: ${localTask.updatedAt})`);
           console.log(`[SYNC DEBUG] Local vector clock:`, localTask.vectorClock);
 
-          // FIX #3: Only trigger conflict if local task was actually modified since last sync
-          const localModifiedAfterSync = config.lastSyncAt &&
-            new Date(localTask.updatedAt).getTime() > config.lastSyncAt;
-
-          console.log(`[SYNC DEBUG] Local modified after sync: ${localModifiedAfterSync}`);
-
-          if (localModifiedAfterSync) {
+          // FIX #3 (SECURITY): Check if we can safely skip conflict detection
+          // Only skip if we KNOW the task was NOT modified locally (i.e., lastSyncAt exists AND task is older)
+          // If lastSyncAt is null (reinstall/reset), we MUST check vector clocks to prevent data loss
+          const taskNotModifiedSinceLastSync = config.lastSyncAt &&
+            new Date(localTask.updatedAt).getTime() <= config.lastSyncAt;
+
+          console.log(`[SYNC DEBUG] Task not modified since last sync: ${taskNotModifiedSinceLastSync}`);
+          console.log(`[SYNC DEBUG] Will check vector clocks: ${!taskNotModifiedSinceLastSync}`);
+
+          // If we don't know when last sync was (null), OR task was modified after sync,
+          // check vector clocks to prevent data loss
+          if (!taskNotModifiedSinceLastSync) {
             const comparison = compareVectorClocks(
               localTask.vectorClock || {},
               encTask.vectorClock
@@ -301,6 +306,8 @@ export class SyncEngine {
               });
               continue;
             }
+          } else {
+            console.log(`[SYNC DEBUG] Skipping conflict check - task confirmed unmodified since last sync`);
           }
         } else if (localTask) {
           console.log(`[SYNC DEBUG] Local task exists but is completed, applying remote`);

tests/sync/sync-conflict-resolution.test.ts

@@ -305,15 +305,42 @@ describe('Sync Conflict Resolution Fixes', () => {
       expect(merged).toEqual({ device1: 5, device2: 3, device3: 2 });
     });
 
-    it('should handle missing lastSyncAt (first sync)', () => {
+    it('should handle missing lastSyncAt (first sync) - SECURITY FIX', () => {
       const lastSyncAt = null;
 
       // Task was modified at any time
       const taskUpdatedAt = Date.now();
 
-      // Should not trigger conflict detection (no previous sync to compare to)
-      const shouldCheckConflict = lastSyncAt && taskUpdatedAt > lastSyncAt;
-      expect(shouldCheckConflict).toBeFalsy(); // null is falsy
+      // SECURITY: When lastSyncAt is null (reinstall/reset), we MUST check vector clocks
+      // to prevent data loss from unsynced local changes
+      const taskNotModifiedSinceLastSync = lastSyncAt &&
+        taskUpdatedAt <= lastSyncAt;
+
+      // Should be false (null && ...) = false
+      expect(taskNotModifiedSinceLastSync).toBeFalsy();
+
+      // Therefore, we WILL check vector clocks (security-safe behavior)
+      const shouldCheckVectorClocks = !taskNotModifiedSinceLastSync;
+      expect(shouldCheckVectorClocks).toBe(true); // MUST check to prevent data loss
+    });
+
+    it('should check vector clocks when lastSyncAt is missing to prevent data loss', () => {
+      // Scenario: User reinstalls app, has local edits, syncs
+      const lastSyncAt = null; // Missing after reinstall
+
+      const localClock = { device1: 5 };
+      const remoteClock = { device2: 3 };
+
+      // With the security fix, we check if task was NOT modified
+      const taskNotModifiedSinceLastSync = lastSyncAt && false; // Always false when lastSyncAt is null
+      expect(taskNotModifiedSinceLastSync).toBeFalsy();
+
+      // Since we can't confirm it's unmodified, we MUST check vector clocks
+      if (!taskNotModifiedSinceLastSync) {
+        const comparison = compareVectorClocks(localClock, remoteClock);
+        // Concurrent clocks detected - prevents data loss!
+        expect(comparison).toBe('concurrent');
+      }
     });
   });
 });