fix(sync-core): break whole-entity LWW timestamp ties by clientId (#9035)

* fix(sync-core): break whole-entity LWW timestamp ties by clientId

An exact-millisecond timestamp tie on the same field of one entity fell
to remote-wins with no tiebreak. Because local/remote are swapped between
devices, each kept the other's value and diverged permanently.

Fall back to a deterministic larger-clientId compare on the tie (mirrors
noiseTiebreakSide), routing a local tie-win through the existing
localWinOperationKind: 'update' path so the compensating op preserves the
local value and dominates the loser's clock.

* refactor(sync-core): tidy LWW tiebreak helper + strengthen tie tests

Multi-review follow-up (no behavior change):
- drop unreachable `?? ops[0]` fallback and the unused generic in
  winningClientId; correct the doc comment's "mirrors" overstatement.
- rewrite the mislabeled service-level tie test that never exercised the
  tiebreak and add the local-clientId-larger direction (compensating op).

* test(sync): cover LWW client-ID tie-win over a concurrent remote DELETE

The tiebreak reaches _createLocalWinUpdateOp's delete-recreation branch via
a tie (not just a newer timestamp); assert the entity is still recreated.

packages/sync-core/src/conflict-resolution.ts

@@ -333,6 +333,22 @@ export const suggestConflictResolution = <TOperation extends Operation<string>>(
   return 'manual';
 };
 
+/**
+ * The clientId of the op carrying a side's winning (max) timestamp. Used only as
+ * a deterministic tiebreak when both sides share the same max timestamp: the two
+ * devices see "local" and "remote" swapped, so comparing timestamps alone makes
+ * each keep the other's value and diverge permanently. Comparing the winning
+ * clientIds instead — over the same unordered pair on both devices — makes them
+ * converge, following the same `(timestamp, clientId)` principle as the client's
+ * `noiseTiebreakSide`. A genuine cross-device tie always has exactly one op at
+ * the max timestamp per side (same-client ops on one entity are never
+ * vector-clock-concurrent, so they never reach here as a conflict).
+ */
+const winningClientId = (
+  ops: readonly Operation<string>[],
+  maxTimestamp: number,
+): string => ops.find((op) => op.timestamp === maxTimestamp)?.clientId ?? '';
+
 /**
  * Plans last-write-wins conflict resolution without looking up host state or
  * creating operations.
@@ -418,7 +434,19 @@ export const planLwwConflictResolutions = <
     const localMaxTimestamp = Math.max(...conflict.localOps.map((op) => op.timestamp));
     const remoteMaxTimestamp = Math.max(...conflict.remoteOps.map((op) => op.timestamp));
 
-    if (localMaxTimestamp > remoteMaxTimestamp) {
+    // On an exact-millisecond tie, fall back to a deterministic clientId compare
+    // (larger wins) so both devices converge instead of each keeping the other's
+    // value. A local tie-win must carry `localWinOperationKind: 'update'` exactly
+    // like a timestamp win: the host rejects the original local ops regardless of
+    // winner, and only the resulting compensating op (dominating clock) both
+    // preserves the local value and stops the loser from resurfacing.
+    const localWins =
+      localMaxTimestamp > remoteMaxTimestamp ||
+      (localMaxTimestamp === remoteMaxTimestamp &&
+        winningClientId(conflict.localOps, localMaxTimestamp) >
+          winningClientId(conflict.remoteOps, remoteMaxTimestamp));
+
+    if (localWins) {
       return {
         conflict,
         winner: 'local',

packages/sync-core/tests/conflict-resolution.spec.ts

@@ -529,10 +529,13 @@ describe('planLwwConflictResolutions', () => {
     ]);
   });
 
-  it('lets remote win timestamp ties', () => {
+  it('defaults a timestamp tie to remote when both sides share a clientId', () => {
+    // Degenerate case only: same-client ops on one entity are never
+    // vector-clock-concurrent, so a real cross-device tie never has equal
+    // clientIds. Keeping the default deterministic is enough here.
     const conflict = createConflict(
-      [createOp({ id: 'local', timestamp: 1_000 })],
-      [createOp({ id: 'remote', timestamp: 1_000 })],
+      [createOp({ id: 'local', timestamp: 1_000, clientId: 'client-1' })],
+      [createOp({ id: 'remote', timestamp: 1_000, clientId: 'client-1' })],
     );
 
     expect(planLwwConflictResolutions([conflict], { isArchiveAction })).toEqual([
@@ -545,6 +548,61 @@ describe('planLwwConflictResolutions', () => {
       },
     ]);
   });
+
+  it('breaks a timestamp tie for local when its winning clientId is larger', () => {
+    const conflict = createConflict(
+      [createOp({ id: 'local', timestamp: 1_000, clientId: 'client-z' })],
+      [createOp({ id: 'remote', timestamp: 1_000, clientId: 'client-a' })],
+    );
+
+    expect(planLwwConflictResolutions([conflict], { isArchiveAction })).toEqual([
+      {
+        conflict,
+        winner: 'local',
+        reason: 'local-timestamp',
+        localWinOperationKind: 'update',
+        localMaxTimestamp: 1_000,
+        remoteMaxTimestamp: 1_000,
+      },
+    ]);
+  });
+
+  it('breaks a timestamp tie for remote when its winning clientId is larger', () => {
+    const conflict = createConflict(
+      [createOp({ id: 'local', timestamp: 1_000, clientId: 'client-a' })],
+      [createOp({ id: 'remote', timestamp: 1_000, clientId: 'client-z' })],
+    );
+
+    expect(planLwwConflictResolutions([conflict], { isArchiveAction })).toEqual([
+      {
+        conflict,
+        winner: 'remote',
+        reason: 'remote-timestamp-or-tie',
+        localMaxTimestamp: 1_000,
+        remoteMaxTimestamp: 1_000,
+      },
+    ]);
+  });
+
+  it('resolves a timestamp tie to the same physical client when sides are swapped', () => {
+    // Commutativity: whichever side "client-b" is on, it wins — so both devices
+    // converge instead of each keeping the other's value.
+    const opA = createOp({ id: 'a', timestamp: 1_000, clientId: 'client-a' });
+    const opB = createOp({ id: 'b', timestamp: 1_000, clientId: 'client-b' });
+
+    const [deviceA] = planLwwConflictResolutions([createConflict([opA], [opB])], {
+      isArchiveAction,
+    });
+    const [deviceB] = planLwwConflictResolutions([createConflict([opB], [opA])], {
+      isArchiveAction,
+    });
+
+    // Device A sees opB as remote and adopts it; device B sees opB as local and
+    // keeps it. Different winner label, same winning op — convergent.
+    expect(deviceA.winner).toBe('remote');
+    expect(deviceB.winner).toBe('local');
+    expect(deviceB.localWinOperationKind).toBe('update');
+  });
 });
 
 describe('partitionLwwResolutions', () => {

src/app/op-log/sync/conflict-resolution.service.spec.ts

@@ -1080,6 +1080,45 @@ describe('ConflictResolutionService', () => {
         ).toBeTrue();
       });
 
+      it('should recreate a locally-winning UPDATE over a concurrent remote DELETE on a client-ID tie (#9024)', async () => {
+        const now = Date.now();
+        mockStore.select.and.returnValue(
+          of({ id: 'task-1', title: 'Local winning task' }),
+        );
+        // Exact-timestamp tie against a remote DELETE. Local's clientId
+        // (client-z) is the larger, so the deterministic tiebreak makes the
+        // local UPDATE win — reaching the SAME delete-recreation path as the
+        // "UPDATE is newer" case above, just via the tie rather than the
+        // timestamp. Guards that the #9024 tiebreak doesn't bypass entity
+        // recreation when the loser was a delete.
+        const conflicts: EntityConflict[] = [
+          createConflict(
+            'task-1',
+            [
+              {
+                ...createOpWithTimestamp('local-upd', 'client-z', now),
+                opType: OpType.Update,
+              },
+            ],
+            [
+              {
+                ...createOpWithTimestamp('remote-del', 'client-a', now),
+                opType: OpType.Delete,
+              },
+            ],
+          ),
+        ];
+
+        await service.autoResolveConflictsLWW(conflicts);
+
+        expect(mockOpLogStore.markRejected).toHaveBeenCalledWith(['local-upd']);
+        expect(mockOpLogStore.markRejected).toHaveBeenCalledWith(['remote-del']);
+        expect(
+          (getFirstMixedLocalOp().payload as { recreatesEntityAfterDelete?: boolean })
+            .recreatesEntityAfterDelete,
+        ).toBeTrue();
+      });
+
       it('should resolve DELETE vs UPDATE conflict when DELETE is older (remote UPDATE wins)', async () => {
         const now = Date.now();
         const conflicts: EntityConflict[] = [
@@ -5185,7 +5224,8 @@ describe('ConflictResolutionService', () => {
         {
           entityType: 'TASK',
           entityId: 'task-1',
-          // client-a < client-b alphabetically, but we test that remote wins on tie
+          // Remote's clientId (client-b) is lexicographically larger, so the
+          // deterministic tiebreak makes remote win the exact-timestamp tie.
           localOps: [createOpWithTimestamp('local-1', 'client-a', now)],
           remoteOps: [createOpWithTimestamp('remote-1', 'client-b', now)],
           suggestedResolution: 'remote', // Remote wins on tie
@@ -5210,6 +5250,37 @@ describe('ConflictResolutionService', () => {
       );
       expect(mockOpLogStore.markRejected).toHaveBeenCalledWith(['local-1']);
     });
+
+    it('should let local win the tie when its client ID is larger', async () => {
+      const now = Date.now();
+
+      // Same exact-millisecond tie, but now LOCAL's clientId (client-z) is the
+      // larger, so the deterministic tiebreak flips the winner to local. This is
+      // the direction the pre-existing tie tests never exercised (#9024): both
+      // devices see the sides swapped yet pick the same physical client, so they
+      // converge instead of each keeping the other's value.
+      const conflicts: EntityConflict[] = [
+        {
+          entityType: 'TASK',
+          entityId: 'task-1',
+          localOps: [createOpWithTimestamp('local-1', 'client-z', now)],
+          remoteOps: [createOpWithTimestamp('remote-1', 'client-a', now)],
+          suggestedResolution: 'remote',
+        },
+      ];
+
+      mockOpLogStore.hasOp.and.resolveTo(false);
+      mockOpLogStore.append.and.resolveTo(1);
+      mockOpLogStore.markApplied.and.resolveTo(undefined);
+      mockOpLogStore.markRejected.and.resolveTo(undefined);
+      mockOperationApplier.applyOperations.and.resolveTo({ appliedOps: [] });
+
+      await service.autoResolveConflictsLWW(conflicts);
+
+      // Local wins the tie → the remote op is rejected (mirrors the local-win
+      // timestamp cases in this block).
+      expect(mockOpLogStore.markRejected).toHaveBeenCalledWith(['remote-1']);
+    });
   });
 
   // =========================================================================