commit 319a83e4193321e598fba3f6f3323ef209d93f81
parent 6fea79d46fb4a783437051ee0fa603e36bfb68fa
Author: Michael Camilleri <[email protected]>
Date: Sun, 26 Apr 2026 11:23:17 +0900
Defer snapshot move pruning until upload succeeds
Snapshot compaction was deleting covered MoveEntity rows as soon as the local
SnapshotEntity was created. This commit keeps those moves locally until
CKSyncEngine confirms the snapshot record was saved, then prunes the covered
moves and enqueues their CloudKit deletions.
A local needsMovePruning flag tracks compaction snapshots that still need
cleanup, including recovery for snapshots that were saved before a crash but
not yet pruned. Remote snapshots do not prune local moves.
Co-Authored-By: Codex GPT 5.5 <[email protected]>
Diffstat:
6 files changed, 226 insertions(+), 13 deletions(-)
diff --git a/Crossmate.xcodeproj/project.pbxproj b/Crossmate.xcodeproj/project.pbxproj
@@ -35,6 +35,7 @@
82918A74836E5076CBFA1592 /* SyncEngine.swift in Sources */ = {isa = PBXBuildFile; fileRef = 73DDDED719CFFDD6035C3B48 /* SyncEngine.swift */; };
8478F0BC0CA624C78DC0A3B5 /* ImportedBrowseView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 87B1BB8AB6309AF111671CB5 /* ImportedBrowseView.swift */; };
8F5CB2F94E083D06D7E04280 /* PlayerSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20B331CC55827FEF3420ABCE /* PlayerSession.swift */; };
+ 905D79CFE454D2E4374544C7 /* GameStoreSnapshotPruningTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1F61F35F4B4AEF51C02276A3 /* GameStoreSnapshotPruningTests.swift */; };
9789150602A3321D2E1E7E81 /* Media.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 0BF60C84D92A9024AC1A53FC /* Media.xcassets */; };
978F91DBAE94BC5DA1D94705 /* DriveMonitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 70AD1A006E6D03E4429E3BF0 /* DriveMonitor.swift */; };
97D77230A98330DCB757FA81 /* sample.xd in Resources */ = {isa = PBXBuildFile; fileRef = 5C63A148D98E2D37EABF2CF5 /* sample.xd */; };
@@ -89,6 +90,7 @@
0C0A7348E1283E7CD2486E2A /* RecordSerializer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RecordSerializer.swift; sourceTree = "<group>"; };
14F2AC5C3B50F4178859E9AC /* CrossmateApp.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CrossmateApp.swift; sourceTree = "<group>"; };
1F2BE43E18B1CC6AAD27DC6D /* NYTBrowseView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NYTBrowseView.swift; sourceTree = "<group>"; };
+ 1F61F35F4B4AEF51C02276A3 /* GameStoreSnapshotPruningTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GameStoreSnapshotPruningTests.swift; sourceTree = "<group>"; };
20B331CC55827FEF3420ABCE /* PlayerSession.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PlayerSession.swift; sourceTree = "<group>"; };
26397B9DBC57DCF7B58899D4 /* BuildNumber.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = BuildNumber.xcconfig; sourceTree = "<group>"; };
283C5C97180C805B6C5BF622 /* PerGameZoneTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PerGameZoneTests.swift; sourceTree = "<group>"; };
@@ -192,6 +194,7 @@
children = (
BFC1C59A30FB2571598273E4 /* GameMutatorTests.swift */,
EBC4C0246B2BCE686A3516DB /* GamePlayerColorStoreTests.swift */,
+ 1F61F35F4B4AEF51C02276A3 /* GameStoreSnapshotPruningTests.swift */,
BAC1B64755AE15CF45350DBB /* MoveBufferTests.swift */,
543481AA9FA32BF14076EB1C /* MoveLogTests.swift */,
C54223FED97577A593B7964E /* NYTToXDConverterTests.swift */,
@@ -426,6 +429,7 @@
A98382E7659991FAF0F4ED0A /* AuthorIdentityTests.swift in Sources */,
2C0DFC182240A2519ED1FA6A /* GameMutatorTests.swift in Sources */,
170D481E47CE5CB17BB8619E /* GamePlayerColorStoreTests.swift in Sources */,
+ 905D79CFE454D2E4374544C7 /* GameStoreSnapshotPruningTests.swift in Sources */,
3D407AF18566F6BA5261DF55 /* MoveBufferTests.swift in Sources */,
24B460FECF10A5BCC29E204E /* MoveLogTests.swift in Sources */,
AA38A51862FC0AB8F7D34899 /* NYTToXDConverterTests.swift in Sources */,
diff --git a/Crossmate/Models/CrossmateModel.xcdatamodeld/CrossmateModel.xcdatamodel/contents b/Crossmate/Models/CrossmateModel.xcdatamodeld/CrossmateModel.xcdatamodel/contents
@@ -55,6 +55,7 @@
<attribute name="ckSystemFields" optional="YES" attributeType="Binary"/>
<attribute name="createdAt" attributeType="Date" usesScalarValueType="NO"/>
<attribute name="gridState" attributeType="Binary"/>
+ <attribute name="needsMovePruning" attributeType="Boolean" defaultValueString="NO" usesScalarValueType="YES"/>
<attribute name="upToLamport" attributeType="Integer 64" defaultValueString="0" usesScalarValueType="YES"/>
<relationship name="game" maxCount="1" deletionRule="Nullify" destinationEntity="GameEntity" inverseName="snapshots" inverseEntity="GameEntity"/>
<fetchIndex name="byGameAndLamport">
diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift
@@ -179,14 +179,16 @@ 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. 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.
+ /// yet covered by an existing snapshot. Moves folded into a local
+ /// compaction snapshot are pruned only after CloudKit has confirmed that
+ /// snapshot save. Returns the `ckRecordName` of each new snapshot for
+ /// enqueueing, plus any move deletions made for previously-durable
+ /// snapshots.
func createSnapshotsIfNeeded(
for gameIDs: Set<UUID>
) -> (snapshotNames: [String], prunedMoveNames: [String]) {
var snapshotNames: [String] = []
- var prunedMoveNames: [String] = []
+ var prunedMoveNames = pruneMovesCoveredByDurableSnapshots()
for gameID in gameIDs {
let req = NSFetchRequest<GameEntity>(entityName: "GameEntity")
@@ -236,13 +238,7 @@ final class GameStore {
let ckName = RecordSerializer.recordName(forSnapshotInGame: gameID, upToLamport: highWater)
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)
- }
+ snapshotEntity.needsMovePruning = true
if context.hasChanges {
try? context.save()
@@ -252,6 +248,52 @@ final class GameStore {
return (snapshotNames, prunedMoveNames)
}
+ /// Deletes moves covered by local compaction snapshots after there is
+ /// durable evidence that the snapshot exists in CloudKit. Passing names
+ /// is used directly from CKSyncEngine's saved-record callback; omitting
+ /// names performs crash recovery by looking for saved snapshots with
+ /// written-back system fields.
+ func pruneMovesCoveredByDurableSnapshots(
+ ckRecordNames: Set<String>? = nil
+ ) -> [String] {
+ let req = NSFetchRequest<SnapshotEntity>(entityName: "SnapshotEntity")
+ if let ckRecordNames {
+ guard !ckRecordNames.isEmpty else { return [] }
+ req.predicate = NSPredicate(
+ format: "needsMovePruning == YES AND ckRecordName IN %@",
+ Array(ckRecordNames)
+ )
+ } else {
+ req.predicate = NSPredicate(
+ format: "needsMovePruning == YES AND ckSystemFields != nil"
+ )
+ }
+
+ let snapshots = (try? context.fetch(req)) ?? []
+ var prunedMoveNames: [String] = []
+ for snapshot in snapshots {
+ guard let game = snapshot.game else {
+ snapshot.needsMovePruning = false
+ continue
+ }
+
+ let covered = ((game.moves as? Set<MoveEntity>) ?? [])
+ .filter { $0.lamport <= snapshot.upToLamport }
+ for move in covered {
+ if let name = move.ckRecordName {
+ prunedMoveNames.append(name)
+ }
+ context.delete(move)
+ }
+ snapshot.needsMovePruning = false
+ }
+
+ if context.hasChanges {
+ try? context.save()
+ }
+ return prunedMoveNames
+ }
+
// MARK: - Load a specific game
/// Loads a game by its entity ID. Sets it as the current game.
diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift
@@ -100,6 +100,13 @@ final class AppServices {
store.markAccessRevoked(gameID: gameID)
}
+ await syncEngine.setOnSnapshotsSaved { [store, syncEngine] names in
+ let prunedMoveNames = store.pruneMovesCoveredByDurableSnapshots(
+ ckRecordNames: Set(names)
+ )
+ await syncEngine.enqueueDeleteRecords(prunedMoveNames)
+ }
+
// Fetch identity before starting engines so first moves get an authorID.
await identity.refresh(using: ckContainer)
await syncEngine.setLocalAuthorID(identity.currentID)
diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift
@@ -39,6 +39,7 @@ actor SyncEngine {
private var onRemoteMoves: (@MainActor @Sendable ([Move]) async -> Void)?
private var onAccountChange: (@MainActor @Sendable () async -> Void)?
private var onGameAccessRevoked: (@MainActor @Sendable (UUID) async -> Void)?
+ private var onSnapshotsSaved: (@MainActor @Sendable ([String]) async -> Void)?
private var tracer: (@MainActor @Sendable (String) -> Void)?
// Cached local identity — written from the main actor, read from
@@ -61,6 +62,10 @@ actor SyncEngine {
onGameAccessRevoked = cb
}
+ func setOnSnapshotsSaved(_ cb: @MainActor @Sendable @escaping ([String]) async -> Void) {
+ onSnapshotsSaved = cb
+ }
+
func setLocalAuthorID(_ id: String?) {
cachedLocalAuthorID.withLock { $0 = id }
}
@@ -830,10 +835,15 @@ actor SyncEngine {
_ event: CKSyncEngine.Event.SentRecordZoneChanges
) async {
let ctx = persistence.container.newBackgroundContext()
- let failureMessages: [String] = ctx.performAndWait {
+ let (savedSnapshotNames, failureMessages): ([String], [String]) = ctx.performAndWait {
+ var snapshotNames: [String] = []
var messages: [String] = []
for record in event.savedRecords {
self.writeBackSystemFields(record: record, in: ctx)
+ let name = record.recordID.recordName
+ if name.hasPrefix("snapshot-") {
+ snapshotNames.append(name)
+ }
}
for failure in event.failedRecordSaves {
let name = failure.record.recordID.recordName
@@ -846,7 +856,10 @@ actor SyncEngine {
)
}
if ctx.hasChanges { try? ctx.save() }
- return messages
+ return (snapshotNames, messages)
+ }
+ if !savedSnapshotNames.isEmpty, let onSnapshotsSaved {
+ await onSnapshotsSaved(savedSnapshotNames)
}
for message in failureMessages {
await trace(message)
diff --git a/Tests/Unit/GameStoreSnapshotPruningTests.swift b/Tests/Unit/GameStoreSnapshotPruningTests.swift
@@ -0,0 +1,146 @@
+import CoreData
+import Foundation
+import Testing
+
+@testable import Crossmate
+
+@Suite("GameStore snapshot pruning", .serialized)
+@MainActor
+struct GameStoreSnapshotPruningTests {
+ @Test("Creating a compaction snapshot keeps covered moves until the snapshot is saved")
+ func snapshotCreationDoesNotImmediatelyPruneMoves() throws {
+ let (store, gameID) = try makeStoreWithCompletedGame(moveCount: 2)
+
+ let result = store.createSnapshotsIfNeeded(for: [gameID])
+
+ #expect(result.snapshotNames.count == 1)
+ #expect(result.prunedMoveNames.isEmpty)
+ #expect(fetchMoveNames(store: store, gameID: gameID).count == 2)
+ #expect(fetchPendingPruneSnapshotNames(store: store) == result.snapshotNames)
+ }
+
+ @Test("Saved local compaction snapshots prune their covered moves")
+ func savedSnapshotPrunesCoveredMoves() throws {
+ let (store, gameID) = try makeStoreWithCompletedGame(moveCount: 2)
+ let result = store.createSnapshotsIfNeeded(for: [gameID])
+
+ let pruned = store.pruneMovesCoveredByDurableSnapshots(
+ ckRecordNames: Set(result.snapshotNames)
+ )
+
+ #expect(Set(pruned) == Set([
+ RecordSerializer.recordName(forMoveInGame: gameID, lamport: 1),
+ RecordSerializer.recordName(forMoveInGame: gameID, lamport: 2)
+ ]))
+ #expect(fetchMoveNames(store: store, gameID: gameID).isEmpty)
+ #expect(fetchPendingPruneSnapshotNames(store: store).isEmpty)
+ }
+
+ @Test("Durable pending snapshots are pruned on a later pass")
+ func durablePendingSnapshotPrunesOnRecoveryPass() throws {
+ let (store, gameID) = try makeStoreWithCompletedGame(moveCount: 1)
+ let result = store.createSnapshotsIfNeeded(for: [gameID])
+ try markSnapshotSaved(store: store, ckRecordName: result.snapshotNames[0])
+
+ let pruned = store.pruneMovesCoveredByDurableSnapshots()
+
+ #expect(pruned == [
+ RecordSerializer.recordName(forMoveInGame: gameID, lamport: 1)
+ ])
+ #expect(fetchMoveNames(store: store, gameID: gameID).isEmpty)
+ #expect(fetchPendingPruneSnapshotNames(store: store).isEmpty)
+ }
+
+ @Test("Remote snapshots are not used to prune local moves")
+ func remoteSnapshotDoesNotPruneLocalMoves() throws {
+ let (store, gameID) = try makeStoreWithCompletedGame(moveCount: 1)
+ let context = store.persistence.viewContext
+ let game = try #require(try fetchGame(store: store, gameID: gameID))
+ let snapshot = SnapshotEntity(context: context)
+ snapshot.game = game
+ snapshot.ckRecordName = "snapshot-\(gameID.uuidString)-1-remote"
+ snapshot.ckSystemFields = Data([1])
+ snapshot.createdAt = Date()
+ snapshot.gridState = try MoveLog.encodeGridState([:])
+ snapshot.upToLamport = 1
+ snapshot.needsMovePruning = false
+ try context.save()
+
+ let pruned = store.pruneMovesCoveredByDurableSnapshots()
+
+ #expect(pruned.isEmpty)
+ #expect(fetchMoveNames(store: store, gameID: gameID).count == 1)
+ }
+
+ private func makeStoreWithCompletedGame(
+ moveCount: Int
+ ) throws -> (GameStore, UUID) {
+ let persistence = makeTestPersistence()
+ let store = GameStore(persistence: persistence)
+ let context = persistence.viewContext
+ let gameID = UUID()
+ let game = GameEntity(context: context)
+ game.id = gameID
+ game.title = "Test"
+ game.puzzleSource = ""
+ game.createdAt = Date()
+ game.updatedAt = Date()
+ game.completedAt = Date()
+ game.ckRecordName = "game-\(gameID.uuidString)"
+ game.lamportHighWater = Int64(moveCount)
+
+ for lamport in 1...moveCount {
+ let move = MoveEntity(context: context)
+ move.game = game
+ move.lamport = Int64(lamport)
+ move.row = 0
+ move.col = Int16(lamport - 1)
+ move.letter = "\(lamport)"
+ move.markKind = 0
+ move.checkedWrong = false
+ move.createdAt = Date()
+ move.ckRecordName = RecordSerializer.recordName(
+ forMoveInGame: gameID,
+ lamport: Int64(lamport)
+ )
+ }
+
+ try context.save()
+ return (store, gameID)
+ }
+
+ private func fetchGame(store: GameStore, gameID: UUID) throws -> GameEntity? {
+ let request = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ request.predicate = NSPredicate(format: "id == %@", gameID as CVarArg)
+ request.fetchLimit = 1
+ return try store.persistence.viewContext.fetch(request).first
+ }
+
+ private func fetchMoveNames(store: GameStore, gameID: UUID) -> [String] {
+ let request = NSFetchRequest<MoveEntity>(entityName: "MoveEntity")
+ request.predicate = NSPredicate(format: "game.id == %@", gameID as CVarArg)
+ request.sortDescriptors = [NSSortDescriptor(key: "lamport", ascending: true)]
+ return ((try? store.persistence.viewContext.fetch(request)) ?? [])
+ .compactMap(\.ckRecordName)
+ }
+
+ private func fetchPendingPruneSnapshotNames(store: GameStore) -> [String] {
+ let request = NSFetchRequest<SnapshotEntity>(entityName: "SnapshotEntity")
+ request.predicate = NSPredicate(format: "needsMovePruning == YES")
+ return ((try? store.persistence.viewContext.fetch(request)) ?? [])
+ .compactMap(\.ckRecordName)
+ .sorted()
+ }
+
+ private func markSnapshotSaved(
+ store: GameStore,
+ ckRecordName: String
+ ) throws {
+ let request = NSFetchRequest<SnapshotEntity>(entityName: "SnapshotEntity")
+ request.predicate = NSPredicate(format: "ckRecordName == %@", ckRecordName)
+ request.fetchLimit = 1
+ let snapshot = try #require(store.persistence.viewContext.fetch(request).first)
+ snapshot.ckSystemFields = Data([1])
+ try store.persistence.viewContext.save()
+ }
+}
