From f6c9714433c5a4e89bb7a5feabccda4241cd5f45 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Tue, 3 Feb 2026 12:48:14 +0100 Subject: [PATCH] fix(sync): prevent task duplication during file-based sync snapshot hydration When a fresh client first syncs with a file-based provider (WebDAV, Dropbox, LocalFile), it receives both a snapshotState and recentOps. The snapshot was hydrated correctly, but recentOps were not recorded, causing duplication through two paths: 1. Download path: On the next sync cycle, recentOps bypassed the appliedOpIds filter (not in IndexedDB) and were re-applied. 2. Piggyback path: The adapter's isForceFromZero guard skipped marking ops as processed, so they were returned as piggybacked during upload. Fix by writing recentOps to IndexedDB after snapshot hydration and always marking downloaded ops as processed for piggyback tracking. --- .../file-based-sync-adapter.service.ts | 14 +++++------ .../op-log/sync/operation-log-sync.service.ts | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts b/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts index c061caabc3..66088a9fc8 100644 --- a/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts +++ b/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts @@ -802,14 +802,14 @@ export class FileBasedSyncAdapterService { `FileBasedSyncAdapter: Downloaded ${limitedOps.length} ops (new/total: ${filteredOps.length}/${latestSeq})`, ); - // Mark downloaded ops as processed (unless it's a force-from-zero request, - // where the caller is expected to process and then call setLastServerSeq) - if (!isForceFromZero) { - for (const serverOp of limitedOps) { - this._markOpProcessed(providerKey, serverOp.op.id); - } - this._persistState(); + // Mark downloaded ops as processed for piggyback tracking. + // This must happen for ALL downloads, including force-from-zero (fresh client). + // Without marking, the upload piggyback mechanism (_collectPiggybackedOps) will + // return these same ops during the next upload, causing entity duplication. + for (const serverOp of limitedOps) { + this._markOpProcessed(providerKey, serverOp.op.id); } + this._persistState(); // NOTE: Archives are NOT written to IndexedDB here. They are included in the // snapshotState response and written to IndexedDB during hydrateFromRemoteSync() diff --git a/src/app/op-log/sync/operation-log-sync.service.ts b/src/app/op-log/sync/operation-log-sync.service.ts index a661e23cd2..f1fe260118 100644 --- a/src/app/op-log/sync/operation-log-sync.service.ts +++ b/src/app/op-log/sync/operation-log-sync.service.ts @@ -447,6 +447,19 @@ export class OperationLogSyncService { false, // Don't create SYNC_IMPORT for file-based bootstrap ); + // CRITICAL FIX: Write recentOps to IndexedDB after snapshot hydration. + // File-based providers return ALL recentOps on every download, relying on + // getAppliedOpIds() (from IndexedDB) to filter already-applied ops. + // Without writing these ops, they bypass the filter on the next sync cycle + // and get applied again, duplicating entities. + if (result.newOps.length > 0) { + await this.opLogStore.appendBatch(result.newOps, 'remote'); + OpLog.normal( + `OperationLogSyncService: Wrote ${result.newOps.length} snapshot ops to IndexedDB ` + + '(prevents duplication on next sync cycle).', + ); + } + // Persist lastServerSeq after hydration if (result.latestServerSeq !== undefined) { await syncProvider.setLastServerSeq(result.latestServerSeq); @@ -734,6 +747,17 @@ export class OperationLogSyncService { false, // Don't create SYNC_IMPORT ); + // CRITICAL FIX: Write recentOps to IndexedDB after snapshot hydration. + // Same rationale as downloadRemoteOps: file-based providers return ALL + // recentOps on every download and rely on getAppliedOpIds() to filter them. + if (result.newOps.length > 0) { + await this.opLogStore.appendBatch(result.newOps, 'remote'); + OpLog.normal( + `OperationLogSyncService: Wrote ${result.newOps.length} snapshot ops to IndexedDB ` + + 'after force-download hydration.', + ); + } + // Update lastServerSeq after hydration if (result.latestServerSeq !== undefined) { await syncProvider.setLastServerSeq(result.latestServerSeq);