commit 2278efef23ba305647593ee47760d1aab7b7a08c
parent a36d710291e5d060b6b567b66b75182a9beb75bb
Author: Michael Camilleri <[email protected]>
Date: Fri, 24 Apr 2026 13:06:13 +0900
Fix correctness issues in new data flow
This commit addresses five issues identified in code review of the
move-log and CKSyncEngine adoption.
1. Snapshot pruning: createSnapshotsIfNeeded now deletes MoveEntity rows
covered by the new snapshot and enqueues their CloudKit deletions, so the
move log stays bounded as intended.
2. Replay tiebreak: MoveLog.replay now sorts by (lamport, createdAt,
authorID) so two devices that independently assign the same Lamport value to
the same cell converge to the same result on replay.
3. Completion snapshot: markCompleted now flushes the MoveBuffer
immediately after setting completedAt, ensuring the completion snapshot is
created without waiting for a subsequent keystroke.
4. Puzzle asset: gameRecord only attaches the puzzleSource CKAsset on the
first send (ckSystemFields == nil) and uses a stable temp path keyed to the
record name, avoiding redundant uploads and temp file leaks on retry.
5. Remote refresh guard: onRemoteMoves now checks that at least one incoming
move belongs to the current game before calling refreshCurrentGame.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Diffstat:
4 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift
@@ -159,10 +159,15 @@ final class GameStore {
/// Checks each game in `gameIDs` and writes a `SnapshotEntity` if the
/// game is complete (has `completedAt` set) or has 200 or more moves not
- /// yet covered by an existing snapshot. Returns the `ckRecordName` of
- /// each newly-created snapshot so the caller can enqueue it for CloudKit.
- func createSnapshotsIfNeeded(for gameIDs: Set<UUID>) -> [String] {
- var created: [String] = []
+ /// yet covered by an existing snapshot. Moves folded into the new snapshot
+ /// are deleted locally and their record names returned for CloudKit
+ /// deletion. Returns the `ckRecordName` of each new snapshot for enqueueing.
+ func createSnapshotsIfNeeded(
+ for gameIDs: Set<UUID>
+ ) -> (snapshotNames: [String], prunedMoveNames: [String]) {
+ var snapshotNames: [String] = []
+ var prunedMoveNames: [String] = []
+
for gameID in gameIDs {
let req = NSFetchRequest<GameEntity>(entityName: "GameEntity")
req.predicate = NSPredicate(format: "id == %@", gameID as CVarArg)
@@ -212,12 +217,19 @@ final class GameStore {
snapshotEntity.ckRecordName = ckName
snapshotEntity.gridState = try? MoveLog.encodeGridState(grid)
+ // Prune moves now folded into the snapshot.
+ let covered = allMoves.filter { $0.lamport <= highWater }
+ for move in covered {
+ if let name = move.ckRecordName { prunedMoveNames.append(name) }
+ context.delete(move)
+ }
+
if context.hasChanges {
try? context.save()
}
- created.append(ckName)
+ snapshotNames.append(ckName)
}
- return created
+ return (snapshotNames, prunedMoveNames)
}
// MARK: - Load a specific game
@@ -330,6 +342,8 @@ final class GameStore {
}
/// Marks a game as completed after a normal win. No-ops if already marked.
+ /// Triggers a buffer flush so the completion snapshot is created promptly
+ /// rather than waiting for the next keystroke or app-background event.
func markCompleted(id: UUID) {
let request = NSFetchRequest<GameEntity>(entityName: "GameEntity")
request.predicate = NSPredicate(format: "id == %@", id as CVarArg)
@@ -338,6 +352,7 @@ final class GameStore {
entity.completedAt == nil else { return }
entity.completedAt = Date()
try? context.save()
+ Task { await moveBuffer?.flush() }
}
// MARK: - Reset
diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift
@@ -35,10 +35,11 @@ final class AppServices {
},
afterFlush: { gameIDs in
await store.replayCellCaches(for: gameIDs)
- let snapshotNames = await store.createSnapshotsIfNeeded(for: gameIDs)
- for name in snapshotNames {
+ let result = await store.createSnapshotsIfNeeded(for: gameIDs)
+ for name in result.snapshotNames {
await syncEngine.enqueueSnapshot(ckRecordName: name)
}
+ await syncEngine.enqueueDeleteRecords(result.prunedMoveNames)
}
)
self.moveBuffer = moveBuffer
@@ -66,7 +67,9 @@ final class AppServices {
syncMonitor.note(message)
}
- await syncEngine.setOnRemoteMoves { [store] _ in
+ await syncEngine.setOnRemoteMoves { [store] moves in
+ guard let currentID = store.currentEntity?.id,
+ moves.contains(where: { $0.gameID == currentID }) else { return }
store.refreshCurrentGame()
}
diff --git a/Crossmate/Sync/MoveLog.swift b/Crossmate/Sync/MoveLog.swift
@@ -51,7 +51,11 @@ enum MoveLog {
let cutoff: Int64 = snapshot?.upToLamport ?? 0
let ordered = moves
.filter { $0.lamport > cutoff }
- .sorted { $0.lamport < $1.lamport }
+ .sorted {
+ if $0.lamport != $1.lamport { return $0.lamport < $1.lamport }
+ if $0.createdAt != $1.createdAt { return $0.createdAt < $1.createdAt }
+ return ($0.authorID ?? "") < ($1.authorID ?? "")
+ }
for move in ordered {
let position = GridPosition(row: move.row, col: move.col)
diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift
@@ -265,11 +265,19 @@ actor SyncEngine {
}
record["title"] = entity.title as CKRecordValue?
record["completedAt"] = entity.completedAt as CKRecordValue?
- if let source = entity.puzzleSource {
+ // puzzleSource is immutable after game creation. Only attach it when
+ // there are no saved system fields (i.e. the record has never been
+ // successfully sent). On a retry the temp file already exists at the
+ // stable path, so no re-write is needed. On a re-send of an already-
+ // sent record the asset is already on the server and we omit it so
+ // CloudKit leaves the stored asset untouched.
+ if let source = entity.puzzleSource, entity.ckSystemFields == nil {
let url = FileManager.default.temporaryDirectory
- .appendingPathComponent(UUID().uuidString)
+ .appendingPathComponent(ckName)
.appendingPathExtension("xd")
- try? source.write(to: url, atomically: true, encoding: .utf8)
+ if !FileManager.default.fileExists(atPath: url.path) {
+ try? source.write(to: url, atomically: true, encoding: .utf8)
+ }
record["puzzleSource"] = CKAsset(fileURL: url)
}
return record
