commit 62674e9f3aed9ba7f69b4063abfb35bb013f63e6
parent 24093ce722295b05a1d4635b87c2c2413d68e66c
Author: Michael Camilleri <[email protected]>
Date: Fri, 15 May 2026 11:59:30 +0900
Preserve in-flight Game record against clobbering
A completed shared game could land back in the In Progress section after
exit, sometimes settling there permanently. The cause: applyGameRecord
adopts whatever fields the server snapshot carries (title, completedAt,
shareRecordName, puzzleSource), and the freshness check is etag-only —
it has no notion of unsent local writes. So when a live-query fetch
resolved between markCompleted writing entity.completedAt and the queued
Game push being serialised by CKSyncEngine, the just-set Date got
overwritten with nil from the stale server record. Worse: the next
outbound push reads entity values at serialise time, so the engine then
serialised the clobbered nil and shipped it to the server, permanently
losing the completion.
GameEntity now carries a hasPendingSave flag. markCompleted, resignGame
and persistShareName set it in the same context.save() that mutates the
data, so the marker is durable, atomic with the write, and visible
across contexts the moment the save returns — no Task-hop window
between local mutation and engine pending state. applyGameRecord still
adopts the fresher etag and zone identity (so the next push doesn't
oplock-fail), then returns early when the flag is set, leaving the
mutable fields alone. handleSentRecordZoneChanges clears the flag for
each game- record in savedRecords once the push lands. The oplock-
recovery path in recoverServerChangedSave only adopts a new etag for
retry, so it deliberately does not clear the flag.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Diffstat:
6 files changed, 80 insertions(+), 8 deletions(-)
diff --git a/Crossmate/Models/CrossmateModel.xcdatamodeld/CrossmateModel.xcdatamodel/contents b/Crossmate/Models/CrossmateModel.xcdatamodeld/CrossmateModel.xcdatamodel/contents
@@ -14,6 +14,7 @@
<attribute name="databaseScope" optional="YES" attributeType="Integer 16" defaultValueString="0" usesScalarValueType="YES"/>
<attribute name="gridHeight" optional="YES" attributeType="Integer 16" defaultValueString="0" usesScalarValueType="YES"/>
<attribute name="gridWidth" optional="YES" attributeType="Integer 16" defaultValueString="0" usesScalarValueType="YES"/>
+ <attribute name="hasPendingSave" attributeType="Boolean" defaultValueString="NO" usesScalarValueType="YES"/>
<attribute name="id" attributeType="UUID" usesScalarValueType="NO"/>
<attribute name="isAccessRevoked" attributeType="Boolean" defaultValueString="NO" usesScalarValueType="YES"/>
<attribute name="lastSeenOtherMoveAt" optional="YES" attributeType="Date" usesScalarValueType="NO"/>
diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift
@@ -487,6 +487,7 @@ final class GameStore {
guard let entity = currentEntity else { return }
entity.completedAt = Date()
+ entity.hasPendingSave = true
try context.save()
if let ckName = entity.ckRecordName {
onGameUpdated(ckName)
@@ -509,6 +510,7 @@ final class GameStore {
guard let entity = try? context.fetch(request).first,
entity.completedAt == nil else { return }
entity.completedAt = Date()
+ entity.hasPendingSave = true
try? context.save()
if let ckName = entity.ckRecordName {
onGameUpdated(ckName)
diff --git a/Crossmate/Sync/RecordSerializer.swift b/Crossmate/Sync/RecordSerializer.swift
@@ -342,6 +342,23 @@ enum RecordSerializer {
return entity
}
+ // Always adopt the fresher etag and zone identity so the next outbound
+ // push uses a current change tag and routes to the right zone.
+ entity.ckRecordName = recordName
+ entity.ckSystemFields = encodeSystemFields(of: record)
+ entity.ckZoneName = record.recordID.zoneID.zoneName
+ let ownerName = record.recordID.zoneID.ownerName
+ entity.ckZoneOwnerName = ownerName == CKCurrentUserDefaultName ? nil : ownerName
+ entity.databaseScope = databaseScope
+
+ // Local mutable fields take precedence while a push is in flight.
+ // The flag is set atomically with the local write (in `markCompleted`,
+ // `resignGame`, `persistShareName`) and cleared once `SyncEngine`
+ // confirms the push landed. Writing server values here would clobber
+ // the pending change and the next outbound push would then serialise
+ // the clobbered value, permanently losing it server-side.
+ guard !entity.hasPendingSave else { return entity }
+
// Seed createdAt/updatedAt from the server record so the library
// can order newly-arrived games. After that, Game records may lag
// behind fresher Moves timestamps, so never move updatedAt backward.
@@ -356,11 +373,8 @@ enum RecordSerializer {
entity.updatedAt = Date()
}
- entity.ckRecordName = recordName
- entity.ckSystemFields = encodeSystemFields(of: record)
entity.title = record["title"] as? String ?? entity.title
entity.completedAt = record["completedAt"] as? Date
- entity.databaseScope = databaseScope
// Owner-side share marker — set on the device that created the share
// and round-tripped via the Game record so other owner-devices learn
// the game is shared. On participant devices `databaseScope == 1`
@@ -369,11 +383,6 @@ enum RecordSerializer {
entity.ckShareRecordName = shareRecordName
}
- // Persist the zone identity so outbound moves use the right zone ID.
- entity.ckZoneName = record.recordID.zoneID.zoneName
- let ownerName = record.recordID.zoneID.ownerName
- entity.ckZoneOwnerName = ownerName == CKCurrentUserDefaultName ? nil : ownerName
-
if let asset = record["puzzleSource"] as? CKAsset,
let fileURL = asset.fileURL,
let source = try? String(contentsOf: fileURL, encoding: .utf8) {
diff --git a/Crossmate/Sync/ShareController.swift b/Crossmate/Sync/ShareController.swift
@@ -185,6 +185,7 @@ final class ShareController {
guard let entity = try ctx.fetch(request).first else { return }
guard entity.ckShareRecordName != recordName else { return }
entity.ckShareRecordName = recordName
+ entity.hasPendingSave = true
try ctx.save()
if let ckRecordName = entity.ckRecordName {
await syncEngine.enqueueGame(ckRecordName: ckRecordName)
diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift
@@ -2162,6 +2162,9 @@ actor SyncEngine {
var orphaned = Set<CKRecordZone.ID>()
for record in event.savedRecords {
self.writeBackSystemFields(record: record, in: ctx)
+ if record.recordID.recordName.hasPrefix("game-") {
+ self.clearPendingSaveFlag(for: record.recordID.recordName, in: ctx)
+ }
}
for failure in event.failedRecordSaves {
let name = failure.record.recordID.recordName
@@ -2340,6 +2343,21 @@ actor SyncEngine {
}
}
+ /// Clears the `hasPendingSave` guard on `GameEntity` after a successful
+ /// push so future inbound fetches resume adopting server fields. Called
+ /// from `handleSentRecordZoneChanges` only on confirmed saves — *not* from
+ /// the oplock-recovery path, which only adopts a fresh etag for retry.
+ private nonisolated func clearPendingSaveFlag(
+ for recordName: String,
+ in ctx: NSManagedObjectContext
+ ) {
+ let req = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ req.predicate = NSPredicate(format: "ckRecordName == %@", recordName)
+ req.fetchLimit = 1
+ guard let entity = try? ctx.fetch(req).first else { return }
+ entity.hasPendingSave = false
+ }
+
// MARK: - Logging helpers
private nonisolated func trace(_ message: String) {
diff --git a/Tests/Unit/RecordSerializerTests.swift b/Tests/Unit/RecordSerializerTests.swift
@@ -135,6 +135,47 @@ struct RecordSerializerTests {
#expect(merged.title == "Updated") // mutable field updated
}
+ @Test("applyGameRecord preserves local mutable fields when a save is pending")
+ @MainActor func applyGameRecordPreservesLocalFieldsWhenSavePending() throws {
+ let persistence = makeTestPersistence()
+ let ctx = persistence.viewContext
+ let gameID = UUID()
+ let zone = RecordSerializer.zoneID(for: gameID)
+ let recordName = RecordSerializer.recordName(forGameID: gameID)
+ let recordID = CKRecord.ID(recordName: recordName, zoneID: zone)
+
+ // Local entity reflects a just-set completion: cells are solved,
+ // `markCompleted` wrote `completedAt` and `hasPendingSave` together,
+ // and a Game-record push is queued but hasn't landed yet.
+ let localCompletedAt = Date(timeIntervalSince1970: 1_700_000_500)
+ let entity = GameEntity(context: ctx)
+ entity.id = gameID
+ entity.ckRecordName = recordName
+ entity.title = "Local Title"
+ entity.completedAt = localCompletedAt
+ entity.hasPendingSave = true
+ entity.puzzleSource = ""
+ entity.createdAt = Date(timeIntervalSince1970: 1_700_000_000)
+ entity.updatedAt = Date(timeIntervalSince1970: 1_700_000_400)
+
+ // Stale server snapshot (the push hasn't landed): no completedAt,
+ // older title. Applying it without the pending-save guard would
+ // clobber the local fields and the next outbound push would then
+ // serialise the clobbered values, permanently losing them.
+ let record = CKRecord(recordType: "Game", recordID: recordID)
+ record["title"] = "Remote Stale Title" as CKRecordValue
+
+ let merged = RecordSerializer.applyGameRecord(record, to: ctx)
+ try ctx.save()
+
+ #expect(merged === entity)
+ #expect(merged.completedAt == localCompletedAt)
+ #expect(merged.title == "Local Title")
+ // The fresher etag is still adopted so the next push uses a current
+ // change tag and doesn't oplock-fail.
+ #expect(merged.ckSystemFields != nil)
+ }
+
@Test("applyGameRecord does not lower an existing updatedAt")
@MainActor func applyGameRecordPreservesFresherUpdatedAt() throws {
let persistence = makeTestPersistence()
