From e019ef0b71f2f22d5ee090bd2df7e94b220273c2 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Sat, 11 Jul 2026 18:01:14 +0200 Subject: [PATCH] fix(supersync): widen conflict lookups to aliased and unioned entity ids Two conflict-detection blind spots let concurrent edits slip through without a conflict: - An op carrying both a scalar entityId and an entityIds array only checked the array; the scalar and array sets are now unioned for detection while getStoredEntityIds keeps the historical storage normalization (scalar in entity_id, arrays in entity_ids). - Histories written before schema v2 keep migrated task settings under GLOBAL_CONFIG:misc. Incoming GLOBAL_CONFIG:tasks writes now consult the legacy misc row as an alias (newest of the two wins, compared by serverSeq), and legacy misc ops check the tasks key per-entity. Works for encrypted payloads since only row metadata is consulted. --- .../super-sync-server/src/sync/conflict.ts | 107 +++++++++++++++--- .../super-sync-server/src/sync/sync.types.ts | 1 + .../super-sync-server/tests/conflict.spec.ts | 23 ++++ 3 files changed, 116 insertions(+), 15 deletions(-) diff --git a/packages/super-sync-server/src/sync/conflict.ts b/packages/super-sync-server/src/sync/conflict.ts index 008733516a..6924586f0f 100644 --- a/packages/super-sync-server/src/sync/conflict.ts +++ b/packages/super-sync-server/src/sync/conflict.ts @@ -1,4 +1,5 @@ import { Prisma } from '@prisma/client'; +import { CURRENT_SCHEMA_VERSION } from '@sp/shared-schema'; import { Logger } from '../logger'; import { BatchUploadCandidate, @@ -34,11 +35,7 @@ export const detectConflict = async ( // Build list of entity IDs to check for conflicts. // Operations may have either entityId (singular) or entityIds (batch operations). - const rawEntityIdsToCheck = op.entityIds?.length - ? op.entityIds - : op.entityId - ? [op.entityId] - : []; + const rawEntityIdsToCheck = getConflictEntityIds(op); // Skip if no entity IDs (can't have entity-level conflicts) if (rawEntityIdsToCheck.length === 0) { @@ -49,6 +46,18 @@ export const detectConflict = async ( return detectConflictForEntity(userId, op, rawEntityIdsToCheck[0], tx); } + // v1 GLOBAL_CONFIG:misc contains fields that became GLOBAL_CONFIG:tasks in + // v2. This compatibility path is intentionally per-key: it is rare, keeps + // the normal multi-entity SQL unchanged, and works for encrypted legacy + // payloads whose contents the server cannot inspect. + if (isLegacyMiscConfigOperation(op)) { + for (const entityId of rawEntityIdsToCheck) { + const result = await detectConflictForEntity(userId, op, entityId, tx); + if (result.hasConflict) return result; + } + return { hasConflict: false }; + } + const entityIdsToCheck = Array.from(new Set(rawEntityIdsToCheck)); return detectConflictForEntities(userId, op, entityIdsToCheck, tx); }; @@ -210,16 +219,40 @@ export const detectConflictForEntity = async ( entityType: op.entityType, OR: [{ entityId }, { entityIds: { has: entityId } }], }, - select: { clientId: true, vectorClock: true }, + select: { clientId: true, vectorClock: true, serverSeq: true }, orderBy: { serverSeq: 'desc' }, }); + // Histories written before schema v2 persist migrated task settings under + // the raw `GLOBAL_CONFIG:misc` key. Consult that key as an alias when the + // incoming write targets `tasks`; no backfill (and no payload decryption) is + // required. Pick the newer of the canonical and legacy rows. + const legacyMiscOp = + op.entityType === 'GLOBAL_CONFIG' && entityId === 'tasks' + ? await tx.operation.findFirst({ + where: { + userId, + entityType: 'GLOBAL_CONFIG', + entityId: 'misc', + schemaVersion: { lt: CURRENT_SCHEMA_VERSION }, + }, + select: { clientId: true, vectorClock: true, serverSeq: true }, + orderBy: { serverSeq: 'desc' }, + }) + : null; + + const latestExistingOp = + legacyMiscOp && + (!existingOp || (legacyMiscOp.serverSeq ?? -1) > (existingOp.serverSeq ?? -1)) + ? legacyMiscOp + : existingOp; + // No existing operation = no conflict - if (!existingOp) { + if (!latestExistingOp) { return { hasConflict: false }; } - return resolveConflictForExistingOp(op, entityId, existingOp); + return resolveConflictForExistingOp(op, entityId, latestExistingOp); }; export const isSameDuplicateOperation = ( @@ -350,14 +383,21 @@ export const toStableJsonValue = (value: unknown): unknown => { * `entity_ids` column, which uses {@link getStoredEntityIds} (multi-entity only). */ export const getConflictEntityIds = (op: Operation): string[] => { - const rawEntityIds = op.entityIds?.length - ? op.entityIds - : op.entityId - ? [op.entityId] - : []; + const rawEntityIds = [ + ...(op.entityId ? [op.entityId] : []), + ...(op.entityIds?.length ? op.entityIds : []), + ]; + if (isLegacyMiscConfigOperation(op)) { + rawEntityIds.push('tasks'); + } return Array.from(new Set(rawEntityIds)); }; +const isLegacyMiscConfigOperation = (op: Operation): boolean => + op.schemaVersion < CURRENT_SCHEMA_VERSION && + op.entityType === 'GLOBAL_CONFIG' && + op.entityId === 'misc'; + /** * The entity_ids array to persist with an op. Ops whose touched-entity set is * already covered by the scalar entity_id store an empty array, so single-entity @@ -371,7 +411,12 @@ export const getConflictEntityIds = (op: Operation): string[] => { * would be invisible to conflict lookups (#8334). */ export const getStoredEntityIds = (op: Operation): string[] => { - const ids = getConflictEntityIds(op); + // Preserve the historical storage normalization: the scalar is persisted in + // entity_id and only the declared multi-entity set belongs in entity_ids. + // Conflict detection unions both fields via getConflictEntityIds above. + const ids = Array.from( + new Set(op.entityIds?.length ? op.entityIds : op.entityId ? [op.entityId] : []), + ); if (ids.length <= 1 && ids[0] === op.entityId) { return []; } @@ -436,7 +481,8 @@ export const prefetchLatestEntityOpsForBatch = async ( o.entity_type AS "entityType", eid AS "entityId", o.client_id AS "clientId", - o.vector_clock AS "vectorClock" + o.vector_clock AS "vectorClock", + o.server_seq AS "serverSeq" FROM operations o CROSS JOIN LATERAL unnest( o.entity_ids || CASE WHEN o.entity_id IS NULL THEN '{}'::text[] ELSE ARRAY[o.entity_id] END @@ -457,6 +503,37 @@ export const prefetchLatestEntityOpsForBatch = async ( } } + if ( + entityPairs.some( + ({ entityType, entityId }) => + entityType === 'GLOBAL_CONFIG' && entityId === 'tasks', + ) + ) { + const legacyMiscOp = await tx.operation.findFirst({ + where: { + userId, + entityType: 'GLOBAL_CONFIG', + entityId: 'misc', + schemaVersion: { lt: CURRENT_SCHEMA_VERSION }, + }, + select: { clientId: true, vectorClock: true, serverSeq: true }, + orderBy: { serverSeq: 'desc' }, + }); + const tasksKey = getEntityConflictKey('GLOBAL_CONFIG', 'tasks'); + const currentTasksOp = latestByEntity.get(tasksKey); + if ( + legacyMiscOp && + (!currentTasksOp || + (legacyMiscOp.serverSeq ?? -1) > (currentTasksOp.serverSeq ?? -1)) + ) { + latestByEntity.set(tasksKey, { + entityType: 'GLOBAL_CONFIG', + entityId: 'tasks', + ...legacyMiscOp, + }); + } + } + return latestByEntity; }; diff --git a/packages/super-sync-server/src/sync/sync.types.ts b/packages/super-sync-server/src/sync/sync.types.ts index 50950b75b6..b47b1c0a3c 100644 --- a/packages/super-sync-server/src/sync/sync.types.ts +++ b/packages/super-sync-server/src/sync/sync.types.ts @@ -222,6 +222,7 @@ export interface LatestEntityOperationRow { entityId: string; clientId: string; vectorClock: unknown; + serverSeq?: number; } export interface LatestBatchEntityOperationRow extends LatestEntityOperationRow { diff --git a/packages/super-sync-server/tests/conflict.spec.ts b/packages/super-sync-server/tests/conflict.spec.ts index d0824ddbc0..dca33fff32 100644 --- a/packages/super-sync-server/tests/conflict.spec.ts +++ b/packages/super-sync-server/tests/conflict.spec.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest'; import { + getConflictEntityIds, isSameDuplicateOperation, isSameDuplicateTimestamp, pruneVectorClockForStorage, @@ -48,6 +49,28 @@ const duplicateCandidate = ( }); describe('conflict helpers', () => { + it('includes a divergent scalar entityId in the incoming conflict set', () => { + expect( + getConflictEntityIds(op({ entityId: 'task-scalar', entityIds: ['task-array'] })), + ).toEqual(['task-scalar', 'task-array']); + }); + + it.each([false, true])( + 'aliases legacy misc config writes to tasks when encrypted=%s', + (isPayloadEncrypted) => { + expect( + getConflictEntityIds( + op({ + entityType: 'GLOBAL_CONFIG', + entityId: 'misc', + schemaVersion: 1, + isPayloadEncrypted, + }), + ), + ).toEqual(['misc', 'tasks']); + }, + ); + it('accepts matching duplicate operations regardless of JSON key order', () => { const incoming = op({ payload: { title: 'A', nested: { b: 2, a: 1 } },