commit 76a6790f7915798779e66a04c63c8a1efe195bc5
parent 5aad0672b4f04e8cdd2bcb66d88baf8d4c9cb02f
Author: Michael Camilleri <[email protected]>
Date: Fri, 8 May 2026 05:31:11 +0900
Recover from CloudKit etag drift on Player and Move records
Diagnostics from a shared-game session showed the owner's Player record locked
at clientEtag c5 while the server had advanced to c7, producing a stream of
OpLockFailed retries. Move records hit the same problem ('record to insert
already exists' against a local row whose ckSystemFields was still nil) which
enqueueUnconfirmedMoves then resurrected on every push.
The root cause is that PresencePublisher, NameBroadcaster, applyPlayerRecord
and writeBackSystemFields all run on background contexts with no merge policy.
The default NSErrorMergePolicy throws on conflict, and the `try? ctx.save()` at
the end of each block swallowed the error, so the system-fields writeback was
silently dropped whenever a concurrent write to the same row had landed in
between.
This commit sets mergePolicy = .mergeByPropertyObjectTrump on the four
sync-side background contexts so concurrent edits to the same row coexist and
writeback persists. The silent ctx.save inside handleSentRecordZoneChanges is
promoted to a logged failure so future drift is visible if it sneaks back in.
This commit also adds recoverServerChangedSave so that when CloudKit returns
serverRecordChanged with an attached server record, the local entity adopts
that record's system fields. The next retry then uses a fresh change tag while
still pushing the local values the user wanted to save. This covers both the
stale-etag update path and the already-exists duplicate-create path observed
for Move records.
Finally, a timestamp guard is added in applyPlayerRecord. Fetched server
records still always refresh ckSystemFields, but the name/selection/updatedAt
fields are only adopted when the incoming updatedAt is at least as recent as
the local one — otherwise an in-flight fetch could clobber the user's live
cursor with a stale server position on every poll.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-Authored-By: Codex GPT 5.5 <[email protected]>
Diffstat:
3 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/Crossmate/Services/NameBroadcaster.swift b/Crossmate/Services/NameBroadcaster.swift
@@ -78,6 +78,7 @@ final class NameBroadcaster {
guard let authorID = authorIdentity.currentID else { return }
let ctx = persistence.container.newBackgroundContext()
+ ctx.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
let touchedGameIDs = Self.upsertPlayerRecords(
in: ctx,
authorID: authorID,
diff --git a/Crossmate/Sync/PresencePublisher.swift b/Crossmate/Sync/PresencePublisher.swift
@@ -114,6 +114,7 @@ actor PresencePublisher {
selection: PlayerSelection?
) async {
let context = persistence.container.newBackgroundContext()
+ context.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
let now = Date()
let fallbackName = self.fallbackName
context.performAndWait {
diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift
@@ -750,9 +750,18 @@ actor SyncEngine {
entity.game = game
}
+ // Always adopt the server's system fields — that's etag tracking and
+ // is independent of which side has the freshest data. The value
+ // fields, however, are only adopted when the incoming record is at
+ // least as new as what we have locally; otherwise a stale-but-current
+ // server record (e.g. our own pending writes haven't landed yet)
+ // would clobber the user's live selection on every fetch.
entity.ckRecordName = ckName
entity.ckSystemFields = RecordSerializer.encodeSystemFields(of: record)
entity.authorID = authorID
+ let localUpdatedAt = entity.updatedAt
+ let incomingIsFresher = localUpdatedAt.map { updatedAt >= $0 } ?? true
+ guard incomingIsFresher else { return }
// An empty `name` is what older builds shipped from PresencePublisher
// before the fix; treat it as "no information" rather than letting it
// clobber a previously-resolved name.
@@ -900,6 +909,7 @@ actor SyncEngine {
// zones (populated fully when the Game record arrives), and mark
// games access-revoked when the owner removes us from the share.
let ctx = persistence.container.newBackgroundContext()
+ ctx.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
let revokedIDs: [UUID] = ctx.performAndWait {
var ids: [UUID] = []
for mod in event.modifications {
@@ -960,6 +970,7 @@ actor SyncEngine {
}
let ctx = persistence.container.newBackgroundContext()
+ ctx.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
let (newMoves, affectedGameIDs, pings): ([Move], Set<UUID>, [Ping]) = ctx.performAndWait {
var moves: [Move] = []
var affected = Set<UUID>()
@@ -1102,6 +1113,7 @@ actor SyncEngine {
}
}
let ctx = persistence.container.newBackgroundContext()
+ ctx.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
let (savedSnapshotNames, failureMessages): ([String], [String]) = ctx.performAndWait {
var snapshotNames: [String] = []
var messages: [String] = []
@@ -1114,6 +1126,11 @@ actor SyncEngine {
}
for failure in event.failedRecordSaves {
let name = failure.record.recordID.recordName
+ if self.recoverServerChangedSave(failure.error, failedRecordName: name, in: ctx) {
+ messages.append(
+ "send: recovered stale system fields for \(name) from CloudKit server record"
+ )
+ }
let err = failure.error as NSError
let userInfo = err.userInfo
.map { "\($0.key)=\($0.value)" }
@@ -1122,7 +1139,16 @@ actor SyncEngine {
"send: failed to save \(name) — domain=\(err.domain) code=\(err.code) \(err.localizedDescription) | userInfo: \(userInfo)"
)
}
- if ctx.hasChanges { try? ctx.save() }
+ if ctx.hasChanges {
+ do {
+ try ctx.save()
+ } catch {
+ let nsError = error as NSError
+ messages.append(
+ "send: writeBack ctx.save failed — domain=\(nsError.domain) code=\(nsError.code) \(nsError.localizedDescription)"
+ )
+ }
+ }
return (snapshotNames, messages)
}
if !savedSnapshotNames.isEmpty, let onSnapshotsSaved {
@@ -1133,6 +1159,26 @@ actor SyncEngine {
}
}
+ /// CKSyncEngine reports optimistic-lock conflicts as failed saves, but the
+ /// error payload often includes the current server record. Adopt only that
+ /// record's system fields so a retry can use the fresh change tag while
+ /// preserving the local values that caused the pending save.
+ private nonisolated func recoverServerChangedSave(
+ _ error: Error,
+ failedRecordName: String,
+ in ctx: NSManagedObjectContext
+ ) -> Bool {
+ let nsError = error as NSError
+ guard nsError.domain == CKErrorDomain,
+ nsError.code == CKError.serverRecordChanged.rawValue,
+ let serverRecord = nsError.userInfo[CKRecordChangedErrorServerRecordKey] as? CKRecord,
+ serverRecord.recordID.recordName == failedRecordName
+ else { return false }
+
+ writeBackSystemFields(record: serverRecord, in: ctx)
+ return true
+ }
+
private nonisolated func writeBackSystemFields(
record: CKRecord,
in ctx: NSManagedObjectContext
