commit d63d53b9e3d9eae6f6bb86604be2a2d8058d3576
parent 467133e1f5018ebeb561670a3b899f54f2dce13b
Author: Michael Camilleri <[email protected]>
Date: Sun, 17 May 2026 16:35:44 +0900
Reap pending sends that can no longer be built
Prior to this commit, nextRecordZoneChangeBatch handed every pending
.saveRecord to CKSyncEngine.RecordZoneChangeBatch's record provider, which
returned buildRecord's result verbatim — including nil. CKSyncEngine persists
pendingRecordZoneChanges to disk, but a ping's body lives only in the in-memory
pendingPings dictionary, which is empty after any cold start (and is cleared by
reset()). So a ping enqueued but not sent before the app was suspended or
killed came back as a persisted pending change with no payload: buildRecord
returned nil, the provider returned nil, and when that was the only pending
change the batch initializer returned nil too. A nil batch means 'nothing to
send', not 'discard' — so the change stayed queued forever.
The record provider now removes the pending change when buildRecord can't
reconstruct the record — a lost ping payload, or a deleted game/moves/player
entity — so the queue self-heals on the next send cycle instead of stalling.
This mirrors Apple's CKSyncEngine reference implementation, which reaps
un-buildable changes inside the provider. The batch build moves into an
internal makeRecordZoneChangeBatch(for:) with a scope-routed test seam,
following the applyZoneOrphaning precedent, since the delegate entry point
can't be driven directly: CKSyncEngine.SendChangesContext has no public
initializer.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Diffstat:
3 files changed, 135 insertions(+), 2 deletions(-)
diff --git a/Crossmate.xcodeproj/project.pbxproj b/Crossmate.xcodeproj/project.pbxproj
@@ -65,6 +65,7 @@
978F91DBAE94BC5DA1D94705 /* DriveMonitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 70AD1A006E6D03E4429E3BF0 /* DriveMonitor.swift */; };
98F8FBF324ED00D53FEBB1DB /* Game.swift in Sources */ = {isa = PBXBuildFile; fileRef = 465F2BB469EFE84CF3733398 /* Game.swift */; };
9CB8808193A4A106D721D767 /* XDFileType.swift in Sources */ = {isa = PBXBuildFile; fileRef = EAC61E2582D94B1E6EC67136 /* XDFileType.swift */; };
+ A458AF9CA8579AB51B695B08 /* PendingChangeReapTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BAEDA3C3765CD8D8897FE5D5 /* PendingChangeReapTests.swift */; };
A98382E7659991FAF0F4ED0A /* AuthorIdentityTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 457B06DBFDC358D213A7CE54 /* AuthorIdentityTests.swift */; };
AA28425BD26F72A9E2B58742 /* BundledBrowseView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9A4B7C6A8A23C6E4CCEC759F /* BundledBrowseView.swift */; };
AA38A51862FC0AB8F7D34899 /* NYTToXDConverterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C54223FED97577A593B7964E /* NYTToXDConverterTests.swift */; };
@@ -192,6 +193,7 @@
B8560440C548752EE93E0ED9 /* PuzzleCatalogTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PuzzleCatalogTests.swift; sourceTree = "<group>"; };
B9031A1574C21866940F6A2C /* XD.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XD.swift; sourceTree = "<group>"; };
BA67C509B467132D1B7510A4 /* Puzzles */ = {isa = PBXFileReference; lastKnownFileType = folder; path = Puzzles; sourceTree = SOURCE_ROOT; };
+ BAEDA3C3765CD8D8897FE5D5 /* PendingChangeReapTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PendingChangeReapTests.swift; sourceTree = "<group>"; };
BF6F111BE8750697C4BC7A17 /* NYTToXDConverter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NYTToXDConverter.swift; sourceTree = "<group>"; };
BFC1C59A30FB2571598273E4 /* GameMutatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GameMutatorTests.swift; sourceTree = "<group>"; };
C0CAA5E17BD406AFEEF96196 /* CalendarDayCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CalendarDayCell.swift; sourceTree = "<group>"; };
@@ -392,6 +394,7 @@
B766E872B12DC79ECCD80941 /* FriendModelTests.swift */,
800CCFBE90554F287E765755 /* FriendZoneTests.swift */,
EF1254FE7BE3672AEC1607B1 /* MovesInboundTests.swift */,
+ BAEDA3C3765CD8D8897FE5D5 /* PendingChangeReapTests.swift */,
283C5C97180C805B6C5BF622 /* PerGameZoneTests.swift */,
68072F4F3EB5D5A78E03D408 /* ShareRoutingTests.swift */,
A9A01534A21796A4EC7113A9 /* ZoneOrphaningTests.swift */,
@@ -541,6 +544,7 @@
AA38A51862FC0AB8F7D34899 /* NYTToXDConverterTests.swift in Sources */,
E632562D090D8BE907F28C53 /* NotificationStateTests.swift in Sources */,
C89A15D812E372FE1C56039B /* PUZToXDConverterTests.swift in Sources */,
+ A458AF9CA8579AB51B695B08 /* PendingChangeReapTests.swift in Sources */,
8225918652DCC822CA1C862F /* PendingEditFlagTests.swift in Sources */,
014134FB81566B5D41168260 /* PerGameZoneTests.swift in Sources */,
CEDF853009D0C367035F1F76 /* PlayerNamePublisherTests.swift in Sources */,
diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift
@@ -2749,12 +2749,45 @@ extension SyncEngine: CKSyncEngineDelegate {
_ context: CKSyncEngine.SendChangesContext,
syncEngine: CKSyncEngine
) async -> CKSyncEngine.RecordZoneChangeBatch? {
- let pending = syncEngine.state.pendingRecordZoneChanges
+ await makeRecordZoneChangeBatch(for: syncEngine)
+ }
+
+ /// Builds the next outbound batch for `engine`, reaping any pending
+ /// `.saveRecord` whose record can no longer be reconstructed. For a ping
+ /// that means its ephemeral `pendingPings` payload was lost across a
+ /// relaunch (CKSyncEngine persists pending changes; the ping body is
+ /// in-memory only); for game/moves/player it means the Core Data entity
+ /// was deleted. Either way the save can never succeed, so the change is
+ /// dropped instead of returning a nil batch that leaves it queued forever
+ /// — `Pending Changes` would never drain and no error is ever surfaced.
+ /// Mirrors Apple's CKSyncEngine reference implementation, which reaps
+ /// un-buildable changes inside the record provider. Internal-rather-than-
+ /// private so the test suite can drive it directly; the delegate entry
+ /// point can't be exercised because `CKSyncEngine.SendChangesContext` has
+ /// no public initializer.
+ func makeRecordZoneChangeBatch(
+ for engine: CKSyncEngine
+ ) async -> CKSyncEngine.RecordZoneChangeBatch? {
+ let pending = engine.state.pendingRecordZoneChanges
guard !pending.isEmpty else { return nil }
let pingSnapshot = pendingPings
return await CKSyncEngine.RecordZoneChangeBatch(pendingChanges: pending) { [weak self] recordID in
guard let self else { return nil }
- return self.buildRecord(for: recordID, pings: pingSnapshot)
+ if let record = self.buildRecord(for: recordID, pings: pingSnapshot) {
+ return record
+ }
+ engine.state.remove(pendingRecordZoneChanges: [.saveRecord(recordID)])
+ return nil
}
}
+
+ /// Test seam: drives `makeRecordZoneChangeBatch` for the given scope's
+ /// engine. Mirrors `pendingSaveRecordNames(scope:)`'s scope routing.
+ func makeRecordZoneChangeBatch(
+ forTestingScope scope: CKDatabase.Scope
+ ) async -> CKSyncEngine.RecordZoneChangeBatch? {
+ let engine = scope == .shared ? sharedEngine : privateEngine
+ guard let engine else { return nil }
+ return await makeRecordZoneChangeBatch(for: engine)
+ }
}
diff --git a/Tests/Unit/Sync/PendingChangeReapTests.swift b/Tests/Unit/Sync/PendingChangeReapTests.swift
@@ -0,0 +1,96 @@
+import CloudKit
+import CoreData
+import Foundation
+import Testing
+
+@testable import Crossmate
+
+/// Pins down the reap path in `makeRecordZoneChangeBatch`. A pending
+/// `.saveRecord` whose record can't be reconstructed (a ping whose in-memory
+/// `pendingPings` payload was lost across a relaunch, or a deleted Core Data
+/// entity) must be dropped from the engine's persisted pending changes rather
+/// than left queued forever. Without this the device showed a permanent
+/// `Pending Changes: 1` that never drained and surfaced no error — every push
+/// "succeeded" while silently sending nothing (see the stuck-ping log).
+@Suite("PendingChangeReap", .serialized)
+@MainActor
+struct PendingChangeReapTests {
+
+ private func makeEngine(
+ persistence: PersistenceController
+ ) async -> SyncEngine {
+ let container = CKContainer(identifier: "iCloud.net.inqk.crossmate.v3")
+ let engine = SyncEngine(container: container, persistence: persistence)
+ await engine.start()
+ return engine
+ }
+
+ private func makePrivateGame(
+ in ctx: NSManagedObjectContext
+ ) throws -> (UUID, String) {
+ let id = UUID()
+ let zoneName = "game-\(id.uuidString)"
+ let entity = GameEntity(context: ctx)
+ entity.id = id
+ entity.title = "Private"
+ entity.puzzleSource = ""
+ entity.createdAt = Date()
+ entity.updatedAt = Date()
+ entity.ckRecordName = zoneName
+ entity.ckZoneName = zoneName
+ entity.databaseScope = 0
+ try ctx.save()
+ return (id, zoneName)
+ }
+
+ @Test("An un-buildable pending save is reaped instead of queued forever")
+ func unbuildableSaveIsReaped() async throws {
+ let persistence = makeTestPersistence()
+ let ctx = persistence.viewContext
+ let (gameID, zoneName) = try makePrivateGame(in: ctx)
+ let engine = await makeEngine(persistence: persistence)
+
+ await engine.enqueueGame(ckRecordName: zoneName)
+ let before = await engine.pendingSaveRecordNames(scope: .private)
+ #expect(before.contains(zoneName))
+
+ // Delete the backing entity so `buildRecord` can no longer
+ // reconstruct the `game-` record — the same dead-end the engine hits
+ // for a ping whose payload didn't survive a relaunch.
+ let req = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ req.predicate = NSPredicate(format: "id == %@", gameID as CVarArg)
+ let entity = try #require(try ctx.fetch(req).first)
+ ctx.delete(entity)
+ try ctx.save()
+
+ _ = await engine.makeRecordZoneChangeBatch(forTestingScope: .private)
+
+ let after = await engine.pendingSaveRecordNames(scope: .private)
+ #expect(!after.contains(zoneName))
+ }
+
+ @Test("A buildable pending ping is preserved, not reaped")
+ func buildablePingIsPreserved() async throws {
+ let persistence = makeTestPersistence()
+ let ctx = persistence.viewContext
+ let (gameID, _) = try makePrivateGame(in: ctx)
+ let engine = await makeEngine(persistence: persistence)
+
+ await engine.enqueuePing(
+ kind: .win,
+ scope: nil,
+ gameID: gameID,
+ authorID: "_localAuthor",
+ playerName: "Local"
+ )
+ let before = await engine.pendingSaveRecordNames(scope: .private)
+ let pingName = try #require(before.first { $0.hasPrefix("ping-") })
+
+ // The payload is still in `pendingPings`, so the record builds — the
+ // reap must not fire just because a record was materialized.
+ _ = await engine.makeRecordZoneChangeBatch(forTestingScope: .private)
+
+ let after = await engine.pendingSaveRecordNames(scope: .private)
+ #expect(after.contains(pingName))
+ }
+}
