From 8df58c63ceac1018a1facad435bd5b0c664b44ee Mon Sep 17 00:00:00 2001 From: "Liu, Minsheng" Date: Thu, 23 Jul 2026 21:32:31 +0800 Subject: [PATCH 1/2] Fix permanent sync divergence on equal userModificationTime stamps Two devices writing the same field with identical microsecond stamps could diverge permanently: the merge guard's <= let each replica's base value overwrite the incoming committed server value on a tie (both sides conclude "mine wins"), and the fetch path never enqueued an upload for locally preserved values, so the losing device silently self-certified as synced while disagreeing with the server forever. - Inbound merges now require the base field stamp to be strictly newer to overrule an incoming committed server value; on a tie the server's committed value wins, using CloudKit's CAS commit order as the deterministic tie-breaker shared by all replicas. Outgoing record construction keeps <= so local edits whose stamp equals the base stamp still apply. - When a fetched record's merge overrules incoming committed field values (base field strictly newer, e.g. clock skew between writers), a save of that record is now enqueued, mirroring the serverRecordChanged conflict path, so the surviving value is re-asserted to the server rather than silently retained. The enqueue is suppressed for stale replays (incoming record older than the stored base) and does not fire for row-level preservation of pending local edits, whose upload is already owned by the change triggers. Co-Authored-By: Claude Fable 5 --- .../CloudKit/CloudKit+StructuredQueries.swift | 54 ++++++++++++++----- Sources/SQLiteData/CloudKit/SyncEngine.swift | 51 ++++++++++++++---- 2 files changed, 82 insertions(+), 23 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/CloudKit+StructuredQueries.swift b/Sources/SQLiteData/CloudKit/CloudKit+StructuredQueries.swift index d2f0726e..dd9abee3 100644 --- a/Sources/SQLiteData/CloudKit/CloudKit+StructuredQueries.swift +++ b/Sources/SQLiteData/CloudKit/CloudKit+StructuredQueries.swift @@ -156,10 +156,13 @@ package func setValue( _ newValue: some CKRecordValueProtocol & Equatable, forKey key: CKRecord.FieldKey, - at userModificationTime: Int64 + at userModificationTime: Int64, + requireStrictlyNewer: Bool = false ) -> Bool { guard - encryptedValues[at: key] <= userModificationTime, + requireStrictlyNewer + ? encryptedValues[at: key] < userModificationTime + : encryptedValues[at: key] <= userModificationTime, encryptedValues[key] != newValue else { return false } encryptedValues[key] = newValue @@ -172,7 +175,8 @@ package func setAsset( _ newValue: CKAsset, forKey key: CKRecord.FieldKey, - at userModificationTime: Int64 + at userModificationTime: Int64, + requireStrictlyNewer: Bool = false ) -> Bool { @Dependency(\.dataManager) var dataManager guard @@ -180,7 +184,9 @@ let hash = dataManager.sha256(of: fileURL) else { return false } guard - encryptedValues[at: key] <= userModificationTime, + requireStrictlyNewer + ? encryptedValues[at: key] < userModificationTime + : encryptedValues[at: key] <= userModificationTime, encryptedValues[hash: key] != hash else { return false } @@ -223,9 +229,13 @@ @discardableResult package func removeValue( forKey key: CKRecord.FieldKey, - at userModificationTime: Int64 + at userModificationTime: Int64, + requireStrictlyNewer: Bool = false ) -> Bool { - guard encryptedValues[at: key] <= userModificationTime + guard + requireStrictlyNewer + ? encryptedValues[at: key] < userModificationTime + : encryptedValues[at: key] <= userModificationTime else { return false } @@ -283,26 +293,42 @@ } } + @discardableResult func update( with other: CKRecord, row: T, columnNames: inout [String], parentForeignKey: ForeignKey? - ) { + ) -> Bool { typealias EquatableCKRecordValueProtocol = CKRecordValueProtocol & Equatable self.userModificationTime = other.userModificationTime + var didPreserveLocalValues = false for column in T.TableColumns.writableColumns { func open(_ column: some WritableTableColumnExpression) { let key = column.name let keyPath = column.keyPath as! KeyPath let didSet: Bool if let value = other[key] as? CKAsset { - didSet = setAsset(value, forKey: key, at: other.encryptedValues[at: key]) + didSet = setAsset( + value, + forKey: key, + at: other.encryptedValues[at: key], + requireStrictlyNewer: true + ) } else if let value = other.encryptedValues[key] as? any EquatableCKRecordValueProtocol { - didSet = setValue(value, forKey: key, at: other.encryptedValues[at: key]) + didSet = setValue( + value, + forKey: key, + at: other.encryptedValues[at: key], + requireStrictlyNewer: true + ) } else if other.encryptedValues[key] == nil { - didSet = removeValue(forKey: key, at: other.encryptedValues[at: key]) + didSet = removeValue( + forKey: key, + at: other.encryptedValues[at: key], + requireStrictlyNewer: true + ) } else { didSet = false } @@ -334,13 +360,17 @@ } if didSet || isRowValueModified { columnNames.removeAll(where: { $0 == key }) - if didSet, let parentForeignKey, key == parentForeignKey.from { - self.parent = other.parent + if didSet { + didPreserveLocalValues = true + if let parentForeignKey, key == parentForeignKey.from { + self.parent = other.parent + } } } } open(column) } + return didPreserveLocalValues } package var userModificationTime: Int64 { diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index ddcb3f32..6897ec77 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -1591,24 +1591,33 @@ case share(CKShare) case reference(CKShare.Reference) } - let shares: [ShareOrReference] = + let (shares, preservedRecordIDs): ([ShareOrReference], [CKRecord.ID]) = await withErrorReporting(.sqliteDataCloudKitFailure) { try await userDatabase.write { db in var shares: [ShareOrReference] = [] + var preservedRecordIDs: [CKRecord.ID] = [] for record in modifications { if let share = record as? CKShare { shares.append(.share(share)) } else { - upsertFromServerRecord(record, db: db) + if upsertFromServerRecord(record, db: db) { + preservedRecordIDs.append(record.recordID) + } if let shareReference = record.share { shares.append(.reference(shareReference)) } } } - return shares + return (shares, preservedRecordIDs) } } - ?? [] + ?? ([], []) + + if !preservedRecordIDs.isEmpty { + syncEngine.state.add( + pendingRecordZoneChanges: preservedRecordIDs.map { .saveRecord($0) } + ) + } await withTaskGroup(of: Void.self) { group in for share in shares { @@ -1897,28 +1906,31 @@ } } + @discardableResult private func upsertFromServerRecord( _ serverRecord: CKRecord, force: Bool = false - ) async { + ) async -> Bool { await withErrorReporting(.sqliteDataCloudKitFailure) { try await userDatabase.write { db in upsertFromServerRecord(serverRecord, force: force, db: db) } } + ?? false } + @discardableResult private func upsertFromServerRecord( _ serverRecord: CKRecord, force: Bool = false, db: Database - ) { + ) -> Bool { withErrorReporting(.sqliteDataCloudKitFailure) { guard let recordPrimaryKey = serverRecord.recordID.recordPrimaryKey, serverRecord.encryptedValues[CKRecord.userModificationTimeKey] != nil else { - return + return false } try SyncMetadata.insert { @@ -1950,18 +1962,19 @@ let metadata = try SyncMetadata.find(serverRecord.recordID).fetchOne(db), let table = tablesByName[serverRecord.recordType] else { - return + return false } serverRecord.userModificationTime = metadata.userModificationTime - func open(_ table: some SynchronizableTable) throws { + func open(_ table: some SynchronizableTable) throws -> Bool { var columnNames: [String] = T.TableColumns.writableColumns.map(\.name) + var didPreserveLocalValues = false if !force, let allFields = metadata._lastKnownServerRecordAllFields, let row = try T.unscoped.find(#sql("\(bind: metadata.recordPrimaryKey)")).fetchOne(db) { - serverRecord.update( + didPreserveLocalValues = serverRecord.update( with: allFields, row: T(queryOutput: row), columnNames: &columnNames, @@ -1994,9 +2007,25 @@ } .execute(db) } + if didPreserveLocalValues, let lastKnownServerRecord = metadata.lastKnownServerRecord { + if let lastKnownDate = lastKnownServerRecord.modificationDate, + let serverDate = serverRecord.modificationDate + { + if serverDate < lastKnownDate { + didPreserveLocalValues = false + } + } else if let lastKnownTag = lastKnownServerRecord._recordChangeTag, + let serverTag = serverRecord._recordChangeTag, + serverTag < lastKnownTag + { + didPreserveLocalValues = false + } + } + return didPreserveLocalValues } - try open(table) + return try open(table) } + ?? false } private func refreshLastKnownServerRecord(_ record: CKRecord) async { From 6d4c4d05f55117b9bfa4414d58822cc95bd4d8fe Mon Sep 17 00:00:00 2001 From: "Liu, Minsheng" Date: Thu, 23 Jul 2026 22:06:35 +0800 Subject: [PATCH 2/2] Add regression tests for equal-stamp and older-stamp inbound merges equalTimestampConflictConvergesToServerValue reproduces the original divergence: a fetched record carrying a different value at an equal field stamp must be adopted, leaving device, server, and merge base converged on the committed value. olderTimestampFetchReassertsLocalWinToServer covers the enqueue-on- override path: a later commit carrying an older field stamp loses the merge, and the surviving local value must be pushed back to the server so replicas that never saw it (or fresh syncs) converge. Co-Authored-By: Claude Fable 5 --- .../CloudKitTests/MergeConflictTests.swift | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) diff --git a/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift b/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift index b9840148..df443f8f 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift @@ -637,6 +637,170 @@ } } + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func equalTimestampConflictConvergesToServerValue() async throws { + try await userDatabase.userWrite { db in + try db.seed { + RemindersList(id: 1, title: "") + Reminder(id: 1, title: "", remindersListID: 1) + } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + try await withDependencies { + $0.currentTime.now += 30 + } operation: { + try await userDatabase.userWrite { db in + try Reminder.find(1).update { $0.title = "Mine" }.execute(db) + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + let record = try syncEngine.private.database.record(for: Reminder.recordID(for: 1)) + record.setValue("Theirs", forKey: "title", at: now) + let modificationCallback = try { + try syncEngine.modifyRecords(scope: .private, saving: [record]) + }() + await modificationCallback.notify() + } + + assertQuery(Reminder.select(\.title), database: userDatabase.database) { + """ + ┌──────────┐ + │ "Theirs" │ + └──────────┘ + """ + } + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(1:reminders/zone/__defaultOwner__), + recordType: "reminders", + parent: CKReference(recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__)), + share: nil, + dueDate🗓️: 0, + id: 1, + id🗓️: 0, + isCompleted: 0, + isCompleted🗓️: 0, + priority🗓️: 0, + remindersListID: 1, + remindersListID🗓️: 0, + title: "Theirs", + title🗓️: 30, + 🗓️: 30 + ), + [1]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__), + recordType: "remindersLists", + parent: nil, + share: nil, + id: 1, + id🗓️: 0, + title: "", + title🗓️: 0, + 🗓️: 0 + ) + ] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [] + ) + ) + """ + } + } + + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func olderTimestampFetchReassertsLocalWinToServer() async throws { + try await userDatabase.userWrite { db in + try db.seed { + RemindersList(id: 1, title: "") + Reminder(id: 1, title: "", remindersListID: 1) + } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + try await withDependencies { + $0.currentTime.now += 100 + } operation: { + try await userDatabase.userWrite { db in + try Reminder.find(1).update { $0.title = "Fast" }.execute(db) + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + let record = try syncEngine.private.database.record(for: Reminder.recordID(for: 1)) + record.encryptedValues["title"] = "Slow" + record.encryptedValues["\(CKRecord.userModificationTimeKey)_title"] = Int64(50) + let modificationCallback = try { + try syncEngine.modifyRecords(scope: .private, saving: [record]) + }() + await modificationCallback.notify() + syncEngine.private.state.assertPendingRecordZoneChanges([ + .saveRecord(Reminder.recordID(for: 1)) + ]) + syncEngine.private.state.add( + pendingRecordZoneChanges: [.saveRecord(Reminder.recordID(for: 1))] + ) + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + } + + assertQuery(Reminder.select(\.title), database: userDatabase.database) { + """ + ┌────────┐ + │ "Fast" │ + └────────┘ + """ + } + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(1:reminders/zone/__defaultOwner__), + recordType: "reminders", + parent: CKReference(recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__)), + share: nil, + dueDate🗓️: 0, + id: 1, + id🗓️: 0, + isCompleted: 0, + isCompleted🗓️: 0, + priority🗓️: 0, + remindersListID: 1, + remindersListID🗓️: 0, + title: "Fast", + title🗓️: 100, + 🗓️: 100 + ), + [1]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__), + recordType: "remindersLists", + parent: nil, + share: nil, + id: 1, + id🗓️: 0, + title: "", + title🗓️: 0, + 🗓️: 0 + ) + ] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [] + ) + ) + """ + } + } + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) @Test func mergeWithNullableFields() async throws { try await userDatabase.userWrite { db in