diff --git a/packages/shared-schema/src/vector-clock.ts b/packages/shared-schema/src/vector-clock.ts index 6d823cc17b..907bbf6598 100644 --- a/packages/shared-schema/src/vector-clock.ts +++ b/packages/shared-schema/src/vector-clock.ts @@ -72,11 +72,15 @@ export const compareVectorClocks = ( aKeys.length >= MAX_VECTOR_CLOCK_SIZE && bKeys.length >= MAX_VECTOR_CLOCK_SIZE; let keysToCompare: Set; + let aOnlyCount = 0; + let bOnlyCount = 0; if (bothPossiblyPruned) { const bKeySet = new Set(bKeys); keysToCompare = new Set(aKeys.filter((k) => bKeySet.has(k))); // If no shared keys at all, clocks are from independent client populations → CONCURRENT if (keysToCompare.size === 0) return 'CONCURRENT'; + aOnlyCount = aKeys.length - keysToCompare.size; + bOnlyCount = bKeys.length - keysToCompare.size; } else { keysToCompare = new Set([...aKeys, ...bKeys]); } @@ -95,6 +99,12 @@ export const compareVectorClocks = ( if (aGreater && bGreater) return 'CONCURRENT'; if (aGreater) return 'GREATER_THAN'; if (bGreater) return 'LESS_THAN'; + + // In pruning-aware mode, if shared keys are equal but both sides have + // non-shared keys, the clocks have genuinely different causal histories. + // Returning EQUAL here would cause silent data loss (skip as duplicate). + // Returning CONCURRENT triggers LWW conflict resolution (safe). + if (bothPossiblyPruned && aOnlyCount > 0 && bOnlyCount > 0) return 'CONCURRENT'; return 'EQUAL'; }; diff --git a/packages/shared-schema/tests/vector-clock.spec.ts b/packages/shared-schema/tests/vector-clock.spec.ts index a78d1e6d6c..d701a79405 100644 --- a/packages/shared-schema/tests/vector-clock.spec.ts +++ b/packages/shared-schema/tests/vector-clock.spec.ts @@ -80,7 +80,7 @@ describe('compareVectorClocks', () => { expect(compareVectorClocks(a, b)).toBe('GREATER_THAN'); }); - it('should return EQUAL when shared keys are equal and both at max size', () => { + it('should return CONCURRENT when shared keys are equal but both sides have non-shared keys', () => { const a: Record = {}; const b: Record = {}; @@ -93,7 +93,8 @@ describe('compareVectorClocks', () => { b[`b_only_${i}`] = 50; } - expect(compareVectorClocks(a, b)).toBe('EQUAL'); + // Non-shared keys on both sides prove genuinely different causal histories + expect(compareVectorClocks(a, b)).toBe('CONCURRENT'); }); it('should return CONCURRENT when shared keys are genuinely concurrent', () => { @@ -192,6 +193,60 @@ describe('compareVectorClocks', () => { expect(Object.keys(b).length).toBeGreaterThan(MAX_VECTOR_CLOCK_SIZE); expect(compareVectorClocks(a, b)).toBe('GREATER_THAN'); }); + + it('should return EQUAL when fully-shared keys at MAX size are identical', () => { + // Both clocks have the exact same keys — no non-shared keys + const a: Record = {}; + const b: Record = {}; + for (let i = 0; i < MAX_VECTOR_CLOCK_SIZE; i++) { + a[`client_${i}`] = 10; + b[`client_${i}`] = 10; + } + + expect(Object.keys(a).length).toBe(MAX_VECTOR_CLOCK_SIZE); + expect(Object.keys(b).length).toBe(MAX_VECTOR_CLOCK_SIZE); + expect(compareVectorClocks(a, b)).toBe('EQUAL'); + }); + + it('should preserve shared-key result when only one side has non-shared keys', () => { + // a has all of b's keys plus extras, shared keys show a dominates + const a: Record = {}; + const b: Record = {}; + for (let i = 0; i < MAX_VECTOR_CLOCK_SIZE; i++) { + a[`shared_${i}`] = 10; + b[`shared_${i}`] = 5; + } + // Only a has extra keys (b has none unique) + for (let i = 0; i < 3; i++) { + a[`a_only_${i}`] = 100; + } + // b needs to reach MAX size too for pruning-aware mode + // Since b already has MAX_VECTOR_CLOCK_SIZE shared keys, it's at MAX + // a has MAX + 3 keys + + expect(Object.keys(a).length).toBeGreaterThan(MAX_VECTOR_CLOCK_SIZE); + expect(Object.keys(b).length).toBe(MAX_VECTOR_CLOCK_SIZE); + // Both >= MAX → pruning-aware mode. Shared keys: a dominates. + // Only a has non-shared keys (bOnlyCount=0), so no escalation to CONCURRENT. + expect(compareVectorClocks(a, b)).toBe('GREATER_THAN'); + }); + + it('should return EQUAL when only one side has non-shared keys and shared keys are equal', () => { + const a: Record = {}; + const b: Record = {}; + for (let i = 0; i < MAX_VECTOR_CLOCK_SIZE; i++) { + a[`shared_${i}`] = 10; + b[`shared_${i}`] = 10; + } + // Only a has extra keys + for (let i = 0; i < 3; i++) { + a[`a_only_${i}`] = 100; + } + + // Both >= MAX → pruning-aware. Shared keys equal. + // Only one side has non-shared keys → not escalated to CONCURRENT. + expect(compareVectorClocks(a, b)).toBe('EQUAL'); + }); }); }); diff --git a/packages/super-sync-server/src/sync/services/operation-download.service.ts b/packages/super-sync-server/src/sync/services/operation-download.service.ts index 7f36f2fd28..43866a40b6 100644 --- a/packages/super-sync-server/src/sync/services/operation-download.service.ts +++ b/packages/super-sync-server/src/sync/services/operation-download.service.ts @@ -85,7 +85,7 @@ export class OperationDownloadService { opType: { in: ['SYNC_IMPORT', 'BACKUP_IMPORT', 'REPAIR'] }, }, orderBy: { serverSeq: 'desc' }, - select: { serverSeq: true }, + select: { serverSeq: true, clientId: true }, }); const latestSnapshotSeq = latestFullStateOp?.serverSeq ?? undefined; @@ -130,9 +130,12 @@ export class OperationDownloadService { } // Limit snapshot clock to MAX_VECTOR_CLOCK_SIZE to prevent oversized - // clocks from being sent to clients. No specific client to preserve, - // so we keep the most active entries. - snapshotVectorClock = limitVectorClockSize(snapshotVectorClock); + // clocks from being sent to clients. Preserve the requesting client's ID + // and the snapshot author's ID to avoid false EQUAL in pruning-aware comparison. + const preserveIds: string[] = []; + if (excludeClient) preserveIds.push(excludeClient); + if (latestFullStateOp?.clientId) preserveIds.push(latestFullStateOp.clientId); + snapshotVectorClock = limitVectorClockSize(snapshotVectorClock, preserveIds); Logger.info( `[user:${userId}] Computed snapshotVectorClock from ${skippedOps.length} ops: ${JSON.stringify(snapshotVectorClock)}`, diff --git a/src/app/core/util/vector-clock.spec.ts b/src/app/core/util/vector-clock.spec.ts index 10b0b37da2..4bc82a40a7 100644 --- a/src/app/core/util/vector-clock.spec.ts +++ b/src/app/core/util/vector-clock.spec.ts @@ -297,5 +297,49 @@ describe('vector-clock', () => { expect(Object.keys(b).length).toBe(MAX_VECTOR_CLOCK_SIZE - 1); expect(compareVectorClocks(a, b)).toBe(VectorClockComparison.CONCURRENT); }); + + it('should return CONCURRENT when shared keys equal but both sides have non-shared keys', () => { + const a: Record = {}; + const b: Record = {}; + for (let i = 0; i < 5; i++) { + a[`shared_${i}`] = 10; + b[`shared_${i}`] = 10; + } + for (let i = 0; i < MAX_VECTOR_CLOCK_SIZE - 5; i++) { + a[`a_only_${i}`] = 50; + b[`b_only_${i}`] = 50; + } + + // Non-shared keys on both sides → genuinely different causal histories → CONCURRENT + expect(compareVectorClocks(a, b)).toBe(VectorClockComparison.CONCURRENT); + }); + + it('should return EQUAL when fully-shared keys at MAX size are identical', () => { + const a: Record = {}; + const b: Record = {}; + for (let i = 0; i < MAX_VECTOR_CLOCK_SIZE; i++) { + a[`client_${i}`] = 10; + b[`client_${i}`] = 10; + } + + expect(compareVectorClocks(a, b)).toBe(VectorClockComparison.EQUAL); + }); + + it('should preserve shared-key result when only one side has non-shared keys', () => { + const a: Record = {}; + const b: Record = {}; + for (let i = 0; i < MAX_VECTOR_CLOCK_SIZE; i++) { + a[`shared_${i}`] = 10; + b[`shared_${i}`] = 10; + } + // Only a has extra keys + for (let i = 0; i < 3; i++) { + a[`a_only_${i}`] = 100; + } + + // Both >= MAX → pruning-aware. Shared keys equal. + // Only one side has non-shared keys → stays EQUAL (not escalated). + expect(compareVectorClocks(a, b)).toBe(VectorClockComparison.EQUAL); + }); }); });