From 0ae073ca425f52747f0cd3f869545ef752d79bbc Mon Sep 17 00:00:00 2001
From: Johannes Millan <johannes.millan@gmail.com>
Date: Wed, 15 Jul 2026 14:53:46 +0200
Subject: [PATCH] fix(sync): only reclaim orphaned snapshot on confirmed
 rev-mismatch (#9040)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The loser-orphan cleanup deleted the just-written immutable snapshot on ANY
commit failure. A rev-mismatch is a confirmed server rejection (ops did not
commit), but a network/5xx error is ambiguous — the ops PUT may have landed and
committed, in which case the snapshot is still referenced and deleting it would
strand a reader. Restrict the cleanup to UploadRevToMatchMismatchAPIError.

Adds a test asserting the immutable snapshot survives a non-mismatch commit
failure. Full op-log suite (3428) green.
---
 .../file-based-sync-adapter.service.ts        | 18 ++++++++++-----
 .../concurrent-compaction.integration.spec.ts | 23 +++++++++++++++++++
 2 files changed, 35 insertions(+), 6 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 6c90036662..ab16d88233 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
@@ -2163,12 +2163,18 @@ export class FileBasedSyncAdapterService {
         opsRev,
       ));
     } catch (e) {
-      // #9040: our ops never committed — a concurrent compactor won the ops race.
-      // The immutable snapshot we wrote this compaction is now an orphan no
-      // committed ops file references, so reclaim it before bubbling up. (Only
-      // when we compacted; otherwise snapshotRef.file is the still-referenced
-      // predecessor and must NOT be deleted.)
-      if (needsCompaction && snapshotRef.file) {
+      // #9040: reclaim the immutable snapshot we just wrote — but ONLY on a
+      // confirmed rev-mismatch rejection, which proves the server refused our ops
+      // PUT, so a concurrent compactor won and our snapshot is a true orphan. Any
+      // other error (network/5xx) is AMBIGUOUS: the PUT may have landed and
+      // committed, in which case the snapshot is still referenced and MUST survive
+      // (readers would otherwise strand on it). Deleting only applies when we
+      // compacted; otherwise snapshotRef.file is the still-referenced predecessor.
+      if (
+        e instanceof UploadRevToMatchMismatchAPIError &&
+        needsCompaction &&
+        snapshotRef.file
+      ) {
         await this._removeGenStateFile(provider, snapshotRef.file);
       }
       throw e;
diff --git a/src/app/op-log/testing/integration/file-based-sync/concurrent-compaction.integration.spec.ts b/src/app/op-log/testing/integration/file-based-sync/concurrent-compaction.integration.spec.ts
index e7956c07c2..dde7c28d89 100644
--- a/src/app/op-log/testing/integration/file-based-sync/concurrent-compaction.integration.spec.ts
+++ b/src/app/op-log/testing/integration/file-based-sync/concurrent-compaction.integration.spec.ts
@@ -183,4 +183,27 @@ describe('File-Based Sync Integration - Concurrent Split Compaction (#9040)', ()
     expect(provider.hasFile('sync-state__1__winner-client.json')).toBe(true);
     expect(provider.hasFile('sync-state__1__loser-client.json')).toBe(false);
   });
+
+  it('ambiguous (non-mismatch) commit failure keeps the immutable snapshot', async () => {
+    const provider = harness.getProvider();
+    await seedFolderAtCap();
+
+    // A compactor writes its immutable snapshot, then its ops commit fails with a
+    // NON-mismatch error (e.g. a dropped connection). That is ambiguous — the PUT
+    // may have landed and committed — so the snapshot MUST NOT be reclaimed, or a
+    // reader of the committed ops file would strand on it.
+    const client = harness.createClient('netfail-client');
+    const realUploadFile = provider.uploadFile.bind(provider);
+    spyOn(provider, 'uploadFile').and.callFake(
+      async (path: string, data: string, rev: string | null, isForce?: boolean) => {
+        if (path === C.OPS_FILE) throw new Error('simulated network failure');
+        return realUploadFile(path, data, rev, isForce);
+      },
+    );
+
+    await expectAsync(client.uploadOps([addTaskOp(client, 'netfail-op')])).toBeRejected();
+
+    // syncVersion 3 (seed 1 → fill 2 → this compaction 3): snapshot must remain.
+    expect(provider.hasFile('sync-state__3__netfail-client.json')).toBe(true);
+  });
 });