fix(sync): use a random suffix, not clientId, in immutable snapshot names (#9040)

Filenames are not encrypted, so embedding the clientId in
sync-state__<syncVersion>__<clientId>.json leaked device count/platform and a
per-device counter to the remote — a metadata downgrade for E2EE file-based
users. Replace the clientId with an opaque random suffix, which still gives two
concurrent compactors distinct files. A collision is astronomically unlikely and
self-heals (the reader validates loaded content against snapshotRef, so a wrong
file fails validation and falls back to sync-state.json/.bak). syncVersion stays
in the name for legible ordering and a future listFiles-prune.

Tests now assert on the count of immutable snapshot files (and a captured name)
rather than hardcoding the now-random filenames. Full op-log suite (3428) green.

packages/sync-providers/src/file-based-sync-data.ts

@@ -98,13 +98,13 @@ export interface FileBasedSnapshotRef {
   /** Optional provider rev of the referenced snapshot file (best-effort). */
   rev?: string;
   /**
-   * #9040: generation+client-unique, immutable snapshot filename this ops file
-   * was built against (e.g. `sync-state__7__clientA.json`). Present on ops files
-   * written by post-#9040 clients. Readers MUST prefer it over the fixed
-   * `sync-state.json`: a concurrent compactor force-writes its own snapshot to a
-   * DIFFERENT immutable file, so the winning ops pointer can never be stranded by
-   * a clobbered `sync-state.json`. Optional for backward compatibility — when
-   * absent (older ops files), readers fall back to `sync-state.json`/`.bak`.
+   * #9040: immutable, per-compaction snapshot filename this ops file was built
+   * against (e.g. `sync-state__7__9f3a1c8b0d2e4f6a.json` — `<syncVersion>__<random>`).
+   * Present on ops files written by post-#9040 clients. Readers MUST prefer it over
+   * the fixed `sync-state.json`: a concurrent compactor force-writes its own
+   * snapshot to a DIFFERENT immutable file, so the winning ops pointer can never be
+   * stranded by a clobbered `sync-state.json`. Optional for backward compatibility
+   * — when absent (older ops files), readers fall back to `sync-state.json`/`.bak`.
    */
   file?: string;
 }
@@ -191,8 +191,8 @@ export const FILE_BASED_SYNC_CONSTANTS = {
   // old and new clients must agree on these names forever.
   OPS_BACKUP_FILE: 'sync-ops.json.bak',
   STATE_BACKUP_FILE: 'sync-state.json.bak',
-  // #9040: prefix for the generation+client-unique immutable snapshot files
-  // (`sync-state__<syncVersion>__<clientId>.json`). Distinct from the fixed
+  // #9040: prefix for the immutable per-compaction snapshot files
+  // (`sync-state__<syncVersion>__<random>.json`). Distinct from the fixed
   // `sync-state.json` (no `__`) so the two never collide. Remote-format surface:
   // old and new clients must keep this stable forever.
   STATE_GEN_FILE_PREFIX: 'sync-state__',

src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts

@@ -1445,13 +1445,21 @@ export class FileBasedSyncAdapterService {
   }
 
   /**
-   * Builds the generation+client-unique immutable snapshot filename (#9040).
-   * `clientId` is sanitized to path-safe characters; `syncVersion` + `clientId`
-   * together guarantee two concurrent compactors never target the same file.
+   * Builds the immutable snapshot filename for a compaction (#9040):
+   * `sync-state__<syncVersion>__<random>.json`. The random suffix (not the
+   * clientId) makes two concurrent compactors target different files without
+   * leaking device identity — filenames are NOT encrypted, so an opaque suffix
+   * keeps device count/platform private from the remote for E2EE users. A
+   * collision is astronomically unlikely and self-heals anyway: the reader
+   * validates loaded content against `snapshotRef` (clock EQUAL), so a wrong
+   * file fails validation and falls back to `sync-state.json`/`.bak`. `syncVersion`
+   * stays in the name for legible ordering and a future `listFiles`-prune.
    */
-  private _genStateFileName(syncVersion: number, clientId: string): string {
-    const safeClientId = clientId.replace(/[^A-Za-z0-9_-]/g, '_');
-    return `${FILE_BASED_SYNC_CONSTANTS.STATE_GEN_FILE_PREFIX}${syncVersion}__${safeClientId}.json`;
+  private _genStateFileName(syncVersion: number): string {
+    const bytes = new Uint8Array(8);
+    crypto.getRandomValues(bytes);
+    const random = Array.from(bytes, (b) => b.toString(16).padStart(2, '0')).join('');
+    return `${FILE_BASED_SYNC_CONSTANTS.STATE_GEN_FILE_PREFIX}${syncVersion}__${random}.json`;
   }
 
   /**
@@ -2083,12 +2091,11 @@ export class FileBasedSyncAdapterService {
         schemaVersion,
         localStateSnapshot,
       );
-      // #9040: write the snapshot to a generation+client-unique IMMUTABLE file
-      // first. This is the snapshot the ops pointer references; because its name
-      // is unique per (syncVersion, clientId), a concurrent compactor writes a
-      // DIFFERENT file and can never clobber it, so the winning ops pointer can
-      // never be stranded.
-      const genStateFile = this._genStateFileName(newSyncVersion, clientId);
+      // #9040: write the snapshot to an IMMUTABLE, per-compaction file first. This
+      // is the snapshot the ops pointer references; because its name carries a
+      // random suffix, a concurrent compactor writes a DIFFERENT file and can never
+      // clobber it, so the winning ops pointer can never be stranded.
+      const genStateFile = this._genStateFileName(newSyncVersion);
       const stateRev = await this._writeStateFile(
         provider,
         cfg,

src/app/op-log/testing/integration/file-based-sync/concurrent-compaction.integration.spec.ts

@@ -97,8 +97,16 @@ describe('File-Based Sync Integration - Concurrent Split Compaction (#9040)', ()
     return (await fresh.downloadOps(0)) as FileSnapshotOpDownloadResponse;
   };
 
+  // The immutable per-compaction snapshot files currently on the remote. Their
+  // names carry a random suffix (#9040 keeps the clientId out of plaintext file
+  // names), so tests assert on their COUNT rather than exact names.
+  const genStateFiles = (): string[] =>
+    harness
+      .getProvider()
+      .getFilePaths()
+      .filter((p) => p.startsWith(C.STATE_GEN_FILE_PREFIX));
+
   it('harmful interleave: loser clobbers sync-state.json but cannot strand the immutable snapshot', async () => {
-    const provider = harness.getProvider();
     await seedFolderAtCap();
 
     // Control: the seeded folder is healthy and fully hydratable BEFORE the race,
@@ -124,12 +132,11 @@ describe('File-Based Sync Integration - Concurrent Split Compaction (#9040)', ()
       UploadRevToMatchMismatchAPIError,
     );
 
-    // The winner's immutable snapshot (syncVersion 3) survives the loser's clobber,
-    // the superseded predecessor (seed's syncVersion 1) was garbage-collected, and
-    // the loser cleaned up its own orphaned snapshot on its failed commit.
-    expect(provider.hasFile('sync-state__3__winner-client.json')).toBe(true);
-    expect(provider.hasFile('sync-state__1__seed-client.json')).toBe(false);
-    expect(provider.hasFile('sync-state__3__loser-client.json')).toBe(false);
+    // Exactly one immutable snapshot remains: the winner's. The seed's predecessor
+    // was GC'd on the winner's commit, and the loser cleaned up its own orphaned
+    // snapshot on its failed commit. A fresh client hydrating without a gap proves
+    // the survivor is the one the committed ops file references (the winner's).
+    expect(genStateFiles().length).toBe(1);
 
     const download = await downloadFresh();
     expect(download.gapDetected).toBeFalsy();
@@ -156,8 +163,6 @@ describe('File-Based Sync Integration - Concurrent Split Compaction (#9040)', ()
   });
 
   it('fresh-folder concurrent compaction: create-if-absent picks one winner, no strand', async () => {
-    const provider = harness.getProvider();
-
     // Both clients compact a fresh folder (no snapshot yet). Park the loser just
     // before it creates sync-ops.json; the winner then creates it first and wins
     // the create-if-absent gate, so the loser's create fails.
@@ -178,10 +183,9 @@ describe('File-Based Sync Integration - Concurrent Split Compaction (#9040)', ()
     const download = await downloadFresh();
     expect(download.gapDetected).toBeFalsy();
     expect(download.snapshotState).toBeDefined();
-    // The winner's immutable snapshot (syncVersion 1) survives the loser's clobber,
-    // and the loser cleaned up its own orphan on its failed commit.
-    expect(provider.hasFile('sync-state__1__winner-client.json')).toBe(true);
-    expect(provider.hasFile('sync-state__1__loser-client.json')).toBe(false);
+    // Only the winner's immutable snapshot remains — the loser cleaned up its own
+    // orphan on its failed commit (no predecessor exists on a fresh folder).
+    expect(genStateFiles().length).toBe(1);
   });
 
   it('ambiguous (non-mismatch) commit failure keeps the immutable snapshot', async () => {
@@ -194,8 +198,10 @@ describe('File-Based Sync Integration - Concurrent Split Compaction (#9040)', ()
     // reader of the committed ops file would strand on it.
     const client = harness.createClient('netfail-client');
     const realUploadFile = provider.uploadFile.bind(provider);
+    let writtenGenFile: string | undefined;
     spyOn(provider, 'uploadFile').and.callFake(
       async (path: string, data: string, rev: string | null, isForce?: boolean) => {
+        if (path.startsWith(C.STATE_GEN_FILE_PREFIX)) writtenGenFile = path;
         if (path === C.OPS_FILE) throw new Error('simulated network failure');
         return realUploadFile(path, data, rev, isForce);
       },
@@ -203,7 +209,8 @@ describe('File-Based Sync Integration - Concurrent Split Compaction (#9040)', ()
 
     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);
+    // The immutable snapshot this compaction wrote must NOT have been reclaimed.
+    expect(writtenGenFile).toBeDefined();
+    expect(provider.hasFile(writtenGenFile as string)).toBe(true);
   });
 });