commit 6776ddfd1f165fdb636c690baef5deff18f03218
parent 138801f76599506863675c7225ab4018d72b83cf
Author: Michael Camilleri <[email protected]>
Date: Tue, 19 May 2026 20:37:54 +0900
Replace the eternal win ping with directed, self-cleaning pings
The win notification was a broadcast .win ping emitted on the .solved
transition. Server-side cleanup deliberately kept every .win ping forever while
dedup was per-device and FIFO-capped, so the record re-notified on sibling
devices and after cache eviction — the puzzle reporting itself solved long
after the user had watched it solved.
Pings are now directed and owned by their recipient. Ping gains an addressee
via AppServices.enqueueDirectedPings. presentPings ignores a ping addressed to
someone else and, once it has handled one addressed to us (shown, suppressed,
or a duplicate), deletes the record via SyncEngine.deletePing. That deletion is
the cross-device acknowledgement: the new onPingDeleted callback fires from
both inbound-deletion paths and withdrawDeliveredNotifications (split out of
dismissDeliveredNotifications, without the .opened side effect) clears any copy
a sibling device already showed.
With every ping self-cleaning, the central sweep is gone: deleteNonWinPings,
removePendingNonWinPings and cleanupPingsAfterCompletion (and its call site)
are removed, along with the dead lastWinCompletionState and the .win
onPlayerEvent emission. presentSessions no longer takes supersedingPings — a
session is suppressed off the durable Game record (completedAt set and
completedBy == that author) rather than a transient win ping.
.opened/.closed/.friend/.invite are unchanged. Tests cover the addressee and
completedBy round-trips and the distinct .win/.resign bodies.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Diffstat:
5 files changed, 222 insertions(+), 102 deletions(-)
diff --git a/Crossmate/CrossmateApp.swift b/Crossmate/CrossmateApp.swift
@@ -388,7 +388,6 @@ private struct PuzzleDisplayView: View {
onComplete: {
store.markCompleted(id: gameID)
Task {
- await services.cleanupPingsAfterCompletion(gameID: gameID)
await services.sendCompletionPings(gameID: gameID, resigned: false)
}
},
@@ -575,22 +574,14 @@ private struct PuzzleDisplayView: View {
session.onSelectionChanged = { selection in
Task { await selectionPublisher.publish(selection) }
}
- let syncEngine = services.syncEngine
- let identity = services.identity
- let preferences = self.preferences
+ let services = self.services
let eventGameID = gameID
session.onPlayerEvent = { kind, scope in
- guard preferences.isICloudSyncEnabled,
- let authorID = identity.currentID
- else { return }
- let playerName = preferences.name
Task {
- await syncEngine.enqueuePing(
+ await services.sendPlayerEventPings(
kind: kind,
scope: scope,
- gameID: eventGameID,
- authorID: authorID,
- playerName: playerName
+ gameID: eventGameID
)
}
}
diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift
@@ -265,22 +265,23 @@ final class AppServices {
store.handleRemoteRemoval(gameID: gameID)
}
+ // A sibling device consumed (deleted) a directed ping; withdraw any
+ // copy of that game's notification we delivered before the deletion
+ // reached us.
+ await syncEngine.setOnPingDeleted { [weak self] gameIDs in
+ guard let self else { return }
+ for gameID in gameIDs {
+ await self.withdrawDeliveredNotifications(for: gameID)
+ }
+ }
+
cloudService.onShareJoined = { [weak self] gameID in
guard let self else { return }
// Register the app for notifications now that the user has joined
// a collaboration. Mirrors the owner path in `onShareSaved` so the
// app is in Settings > Notifications before any inbound moves.
await AppDelegate.requestNotificationAuthorizationIfNeeded()
- guard self.preferences.isICloudSyncEnabled,
- let authorID = self.identity.currentID
- else { return }
- await self.syncEngine.enqueuePing(
- kind: .join,
- scope: nil,
- gameID: gameID,
- authorID: authorID,
- playerName: self.preferences.name
- )
+ await self.enqueueDirectedPings(kind: .join, scope: nil, gameID: gameID)
}
// PlayerNamePublisher fans out name changes to active shared/joined
@@ -355,19 +356,17 @@ final class AppServices {
await movesUpdater.flush()
}
- func cleanupPingsAfterCompletion(gameID: UUID) async {
- guard await ensureICloudSyncStarted() else { return }
- await syncMonitor.run("completed ping cleanup") {
- try await syncEngine.deleteNonWinPings(forCompletedGame: gameID)
- }
- }
-
- /// Fans out the directed completion ping. The completing device has
- /// already written `completedAt`/`completedBy` to its own Game record;
- /// this tells every *other* roster player exactly once — `.win` for a
- /// solve, `.resign` when the player gave up. Addressed per-recipient so
- /// each device deletes its own copy on consumption (no central cleanup).
- func sendCompletionPings(gameID: UUID, resigned: Bool) async {
+ /// Fans a player-facing ping out to every *other* roster player, one
+ /// directed copy each (`addressee` = that authorID). Every player-facing
+ /// kind goes through here so each recipient consume-deletes its own copy
+ /// (see `presentPings`) — there is no central ping cleanup. nil roster
+ /// (e.g. a brand-new joiner before the owner's Player record syncs) simply
+ /// sends nothing; the lost notice is an accepted eventual-consistency cost.
+ private func enqueueDirectedPings(
+ kind: PingKind,
+ scope: PingScope?,
+ gameID: UUID
+ ) async {
guard preferences.isICloudSyncEnabled,
let localAuthorID = identity.currentID,
!localAuthorID.isEmpty
@@ -395,11 +394,10 @@ final class AppServices {
guard !recipients.isEmpty else { return }
let playerName = preferences.name
- let kind: PingKind = resigned ? .resign : .win
for recipient in recipients {
await syncEngine.enqueuePing(
kind: kind,
- scope: nil,
+ scope: scope,
gameID: gameID,
authorID: localAuthorID,
playerName: playerName,
@@ -408,6 +406,23 @@ final class AppServices {
}
}
+ /// Completion fan-out: `.win` on a solve, `.resign` when the player gave
+ /// up. The completing device has already written `completedAt`/
+ /// `completedBy` to its own Game record.
+ func sendCompletionPings(gameID: UUID, resigned: Bool) async {
+ await enqueueDirectedPings(
+ kind: resigned ? .resign : .win,
+ scope: nil,
+ gameID: gameID
+ )
+ }
+
+ /// Activity fan-out for `.join`/`.check`/`.reveal` — same directed,
+ /// self-cleaning model as completion.
+ func sendPlayerEventPings(kind: PingKind, scope: PingScope?, gameID: UUID) async {
+ await enqueueDirectedPings(kind: kind, scope: scope, gameID: gameID)
+ }
+
/// Pull-to-refresh action for the library. Discovers any zones the
/// device hasn't seen yet on both database scopes, then runs the normal
/// engine fetch so any in-flight changes also catch up. Bypasses
@@ -700,10 +715,7 @@ final class AppServices {
}
if let result {
await presentPings(result.0)
- await presentSessions(
- result.1,
- supersedingPings: result.0
- )
+ await presentSessions(result.1)
}
scheduleBackgroundPushCatchUp(scope: scope)
await refreshSnapshot()
@@ -1158,22 +1170,40 @@ final class AppServices {
let center = UNUserNotificationCenter.current()
for ping in playerFacingPings {
+ // A directed ping (`addressee` set) targets one player by
+ // authorID. Ignore one addressed to someone else — another user's
+ // device receives and consumes it. nil ⇒ broadcast (legacy /
+ // mixed-version peers): every recipient still acts on it.
+ if let addressee = ping.addressee, addressee != identity.currentID {
+ continue
+ }
if ping.authorID == identity.currentID {
syncMonitor.note("ping(\(ping.kind.rawValue)): skipped self-authored record \(ping.recordName)")
continue
}
+ // A directed ping addressed to us is consumed by this account:
+ // once handled — shown, suppressed, or a duplicate — delete it so
+ // it stops re-notifying and the deletion withdraws any copy our
+ // sibling devices showed. Broadcast pings are left as-is.
+ let consume = ping.addressee != nil
+ func consumeIfDirected() async {
+ guard consume else { return }
+ await syncEngine.deletePing(recordName: ping.recordName, gameID: ping.gameID)
+ }
// The push fast path and the CKSyncEngine catch-up both surface
// the same Ping records, so dedup by record name. We do this
// check first and short-circuit; every other path below ends by
// recording the name via the defer.
if NotificationState.wasShown(pingRecordName: ping.recordName) {
syncMonitor.note("ping(\(ping.kind.rawValue)): already-shown record \(ping.recordName)")
+ await consumeIfDirected()
continue
}
defer { NotificationState.recordShown(pingRecordName: ping.recordName) }
if NotificationState.isSuppressed(gameID: ping.gameID) {
syncMonitor.note("ping(\(ping.kind.rawValue)): suppressed — puzzle is active for \(ping.gameID.uuidString)")
+ await consumeIfDirected()
continue
}
@@ -1194,6 +1224,7 @@ final class AppServices {
do {
try await center.add(request)
syncMonitor.note("ping(\(ping.kind.rawValue)): queued local notification for \(ping.gameID.uuidString)")
+ await consumeIfDirected()
} catch {
syncMonitor.note("ping(\(ping.kind.rawValue)): local notification failed — \(error.localizedDescription)")
}
@@ -1201,8 +1232,7 @@ final class AppServices {
}
private func presentSessions(
- _ sessions: [Session],
- supersedingPings: [Ping]
+ _ sessions: [Session]
) async {
guard !sessions.isEmpty else { return }
guard await canPresentNotifications() else {
@@ -1210,10 +1240,26 @@ final class AppServices {
return
}
- let completedByAuthor = Set(supersedingPings.compactMap { ping -> String? in
- guard ping.kind == .win else { return nil }
- return "\(ping.gameID.uuidString)|\(ping.authorID)"
- })
+ // A session ("X is playing") is stale once X has completed the puzzle.
+ // The durable signal is the Game record's `completedBy` (the solver of
+ // a win; nil for a resignation, which therefore never supersedes), not
+ // a transient win ping. Keyed gameID|authorID as before.
+ let sessionGameIDs = Set(sessions.map { $0.gameID })
+ let ctx = persistence.container.newBackgroundContext()
+ let completedByAuthor: Set<String> = ctx.performAndWait {
+ var keys = Set<String>()
+ for gid in sessionGameIDs {
+ let req = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ req.predicate = NSPredicate(format: "id == %@", gid as CVarArg)
+ req.fetchLimit = 1
+ guard let g = try? ctx.fetch(req).first,
+ g.completedAt != nil,
+ let by = g.completedBy, !by.isEmpty
+ else { continue }
+ keys.insert("\(gid.uuidString)|\(by)")
+ }
+ return keys
+ }
let center = UNUserNotificationCenter.current()
for session in sessions {
if session.authorID == identity.currentID {
@@ -1221,7 +1267,7 @@ final class AppServices {
continue
}
if completedByAuthor.contains("\(session.gameID.uuidString)|\(session.authorID)") {
- syncMonitor.note("session: suppressed by win ping for \(session.gameID.uuidString)")
+ syncMonitor.note("session: suppressed — author completed \(session.gameID.uuidString)")
continue
}
if NotificationState.isSuppressed(gameID: session.gameID) {
@@ -1395,6 +1441,15 @@ final class AppServices {
/// The ping fires unconditionally — a sibling device may have a
/// notification we don't, and we can't see its Notification Center.
func dismissDeliveredNotifications(for gameID: UUID) async {
+ await withdrawDeliveredNotifications(for: gameID)
+ await broadcastOpenedPing(for: gameID)
+ }
+
+ /// Removes this device's delivered notifications for `gameID` without the
+ /// `.opened` lease side effect — used both by `dismissDeliveredNotifications`
+ /// and by the inbound ping-deletion path, where a sibling has already
+ /// consumed (and deleted) the directed ping, so any copy we showed is stale.
+ private func withdrawDeliveredNotifications(for gameID: UUID) async {
let center = UNUserNotificationCenter.current()
let delivered = await center.deliveredNotifications()
let identifiers = delivered.compactMap { notification -> String? in
@@ -1408,7 +1463,6 @@ final class AppServices {
center.removeDeliveredNotifications(withIdentifiers: identifiers)
syncMonitor.note("notif: dismissed \(identifiers.count) delivered for \(gameID.uuidString)")
}
- await broadcastOpenedPing(for: gameID)
}
/// Applies an inbound lease ping (`.opened`/`.closed`) from another device
diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift
@@ -191,6 +191,10 @@ actor SyncEngine {
private var onAccountChange: (@MainActor @Sendable () async -> Void)?
private var onGameAccessRevoked: (@MainActor @Sendable (UUID) async -> Void)?
private var onGameRemoved: (@MainActor @Sendable (UUID) async -> Void)?
+ /// Fires with the game IDs whose Ping record(s) were just deleted on the
+ /// server (a sibling device consumed a directed ping). Drives cross-device
+ /// withdrawal of the notification this device may have shown for it.
+ private var onPingDeleted: (@MainActor @Sendable (Set<UUID>) async -> Void)?
private var localAuthorIDProvider: (@MainActor @Sendable () -> String?)?
private var tracer: (@MainActor @Sendable (String) -> Void)?
private var liveQueryCheckpoints: [String: Date] = [:]
@@ -239,6 +243,10 @@ actor SyncEngine {
onGameRemoved = cb
}
+ func setOnPingDeleted(_ cb: @MainActor @Sendable @escaping (Set<UUID>) async -> Void) {
+ onPingDeleted = cb
+ }
+
func setLocalAuthorIDProvider(_ cb: @MainActor @Sendable @escaping () -> String?) {
localAuthorIDProvider = cb
}
@@ -690,58 +698,21 @@ actor SyncEngine {
Task { try? await engine.sendChanges() }
}
- /// Deletes transient Ping records for a completed owned game while keeping
- /// every `.win` ping. Participants see the owner's zone through the share,
- /// so the owner-side deletion removes the records from the cooperative
- /// game without risking the final win notification.
- func deleteNonWinPings(forCompletedGame gameID: UUID) async throws {
+ /// Consume-deletes a single Ping the local account has handled (shown,
+ /// suppressed, or a duplicate). The deletion syncs through the game zone so
+ /// this user's other devices withdraw any notification they showed for it.
+ /// Deleting an absent record is benign (CloudKit reports it gone; the send
+ /// path does not retry-loop it). `sendChanges` is deferred via `Task` so
+ /// this is safe to call from a delegate callback (friend-invite re-entrancy).
+ func deletePing(recordName: String, gameID: UUID) {
let ctx = persistence.container.newBackgroundContext()
- guard let info = zoneInfo(forGameID: gameID, in: ctx),
- info.scope == 0
- else { return }
-
- removePendingNonWinPings(for: gameID, zoneID: info.zoneID)
-
- let records = try await queryLiveRecords(
- type: "Ping",
- database: container.privateCloudDatabase,
- zoneID: info.zoneID,
- since: nil,
- desiredKeys: ["kind"]
- )
- let recordIDsToDelete = records.compactMap { record -> CKRecord.ID? in
- guard (record["kind"] as? String) != PingKind.win.rawValue else { return nil }
- return record.recordID
- }
- try await deleteRecords(withIDs: recordIDsToDelete, in: container.privateCloudDatabase)
- if !recordIDsToDelete.isEmpty {
- await trace("ping cleanup: deleted \(recordIDsToDelete.count) non-win ping(s) for \(gameID.uuidString)")
- }
- }
-
- private func removePendingNonWinPings(for gameID: UUID, zoneID: CKRecordZone.ID) {
- let namesToRemove = Set(pendingPings.compactMap { name, payload in
- payload.gameID == gameID && payload.kind != .win ? name : nil
- })
- guard !namesToRemove.isEmpty else { return }
-
- for name in namesToRemove {
- pendingPings.removeValue(forKey: name)
- }
- guard let privateEngine else { return }
- let changesToRemove = privateEngine.state.pendingRecordZoneChanges.filter { change in
- switch change {
- case .saveRecord(let id):
- return id.zoneID == zoneID && namesToRemove.contains(id.recordName)
- case .deleteRecord:
- return false
- @unknown default:
- return false
- }
- }
- if !changesToRemove.isEmpty {
- privateEngine.state.remove(pendingRecordZoneChanges: changesToRemove)
- }
+ guard let info = zoneInfo(forGameID: gameID, in: ctx) else { return }
+ let engine = info.scope == 1 ? sharedEngine : privateEngine
+ guard let engine else { return }
+ pendingPings.removeValue(forKey: recordName)
+ let recordID = CKRecord.ID(recordName: recordName, zoneID: info.zoneID)
+ engine.state.add(pendingRecordZoneChanges: [.deleteRecord(recordID)])
+ Task { try? await engine.sendChanges() }
}
private nonisolated static func notificationTitle(for entity: GameEntity?) -> String {
@@ -1673,6 +1644,13 @@ actor SyncEngine {
if let onRemotePlayersUpdated, !playersUpdatedGameIDs.isEmpty {
await onRemotePlayersUpdated(playersUpdatedGameIDs)
}
+ let pingDeletedGameIDs = Set(deletions.compactMap { deletion -> UUID? in
+ deletion.0.recordName.hasPrefix("ping-")
+ ? gameID(fromRecordName: deletion.0.recordName) : nil
+ })
+ if let onPingDeleted, !pingDeletedGameIDs.isEmpty {
+ await onPingDeleted(pingDeletedGameIDs)
+ }
if !affectedGameIDs.isEmpty {
NotificationCenter.default.post(
name: .playerRosterShouldRefresh,
@@ -2556,6 +2534,13 @@ actor SyncEngine {
if let onPings, !pings.isEmpty {
await onPings(pings)
}
+ let pingDeletedGameIDs = Set(event.deletions.compactMap { deletion -> UUID? in
+ deletion.recordID.recordName.hasPrefix("ping-")
+ ? gameID(fromRecordName: deletion.recordID.recordName) : nil
+ })
+ if let onPingDeleted, !pingDeletedGameIDs.isEmpty {
+ await onPingDeleted(pingDeletedGameIDs)
+ }
if !affectedGameIDs.isEmpty {
NotificationCenter.default.post(
name: .playerRosterShouldRefresh,
diff --git a/Tests/Unit/PuzzleNotificationTextTests.swift b/Tests/Unit/PuzzleNotificationTextTests.swift
@@ -26,7 +26,8 @@ struct PuzzleNotificationTextTests {
puzzleTitle: "Saturday Puzzle – 1 January 2001",
kind: .join,
scope: nil,
- payload: nil
+ payload: nil,
+ addressee: nil
)
#expect(AppServices.bodyText(for: ping) == "Alice joined the puzzle 'Saturday Puzzle – 1 January 2001'")
@@ -43,12 +44,36 @@ struct PuzzleNotificationTextTests {
puzzleTitle: "Saturday Puzzle – 1 January 2001",
kind: .check,
scope: .puzzle,
- payload: nil
+ payload: nil,
+ addressee: nil
)
#expect(AppServices.bodyText(for: ping) == "Alice checked all of the puzzle 'Saturday Puzzle – 1 January 2001'")
}
+ @Test("Win and resign render distinct bodies")
+ func winAndResignBodies() {
+ func ping(_ kind: PingKind) -> Ping {
+ Ping(
+ recordName: "ping-test-\(kind.rawValue)",
+ gameID: UUID(),
+ authorID: "alice",
+ deviceID: "device-a",
+ playerName: "Alice",
+ puzzleTitle: "Saturday Puzzle – 1 January 2001",
+ kind: kind,
+ scope: nil,
+ payload: nil,
+ addressee: "bob"
+ )
+ }
+
+ #expect(AppServices.bodyText(for: ping(.win))
+ == "Alice solved the puzzle 'Saturday Puzzle – 1 January 2001'")
+ #expect(AppServices.bodyText(for: ping(.resign))
+ == "Alice gave up on the puzzle 'Saturday Puzzle – 1 January 2001'")
+ }
+
@Test("Session activity says player is solving puzzle")
func sessionActivitySaysPlayerIsSolvingPuzzle() {
let session = Session(
diff --git a/Tests/Unit/RecordSerializerTests.swift b/Tests/Unit/RecordSerializerTests.swift
@@ -134,6 +134,38 @@ struct RecordSerializerTests {
#expect(withoutPayload["payload"] == nil)
}
+ @Test("pingRecord writes addressee when directed and omits it when nil")
+ func pingRecordAddresseeRoundTrip() {
+ let zone = CKRecordZone.ID(zoneName: "z", ownerName: CKCurrentUserDefaultName)
+ let directed = RecordSerializer.pingRecord(
+ gameID: UUID(),
+ authorID: "alice",
+ deviceID: "deviceA",
+ playerName: "Alice",
+ puzzleTitle: "Puzzle",
+ eventTimestampMs: 1700000000000,
+ kind: .win,
+ scope: nil,
+ addressee: "bob",
+ zone: zone
+ )
+ #expect(directed["addressee"] as? String == "bob")
+ #expect(directed["kind"] as? String == "win")
+
+ let broadcast = RecordSerializer.pingRecord(
+ gameID: UUID(),
+ authorID: "alice",
+ deviceID: "deviceA",
+ playerName: "Alice",
+ puzzleTitle: "Puzzle",
+ eventTimestampMs: 1700000000000,
+ kind: .check,
+ scope: .square,
+ zone: zone
+ )
+ #expect(broadcast["addressee"] == nil)
+ }
+
@Test("check/reveal scope is folded into payload, not the legacy field")
func pingRecordFoldsScopeIntoPayload() throws {
let zone = CKRecordZone.ID(zoneName: "z", ownerName: CKCurrentUserDefaultName)
@@ -196,6 +228,39 @@ struct RecordSerializerTests {
#expect(entity.ckRecordName == recordName)
}
+ @Test("applyGameRecord round-trips completedBy and clears it when absent")
+ @MainActor func applyGameRecordCompletedBy() throws {
+ let persistence = makeTestPersistence()
+ let ctx = persistence.viewContext
+ let gameID = UUID()
+ let recordID = CKRecord.ID(
+ recordName: RecordSerializer.recordName(forGameID: gameID),
+ zoneID: RecordSerializer.zoneID(for: gameID)
+ )
+
+ // A win carries the solver's authorID.
+ let (asset, tmpURL) = try makePuzzleAsset()
+ defer { try? FileManager.default.removeItem(at: tmpURL) }
+ let win = CKRecord(recordType: "Game", recordID: recordID)
+ win["title"] = "T" as CKRecordValue
+ win["completedAt"] = Date() as CKRecordValue
+ win["completedBy"] = "alice" as CKRecordValue
+ win["puzzleSource"] = asset as CKRecordValue
+ let entity = RecordSerializer.applyGameRecord(win, to: ctx)
+ try ctx.save()
+ #expect(entity.completedBy == "alice")
+
+ // A later record without completedBy (a resignation) clears it, so
+ // wins stay distinguishable from resignations.
+ let resign = CKRecord(recordType: "Game", recordID: recordID)
+ resign["title"] = "T" as CKRecordValue
+ resign["completedAt"] = Date() as CKRecordValue
+ let merged = RecordSerializer.applyGameRecord(resign, to: ctx)
+ try ctx.save()
+ #expect(merged === entity)
+ #expect(merged.completedBy == nil)
+ }
+
@Test("applyGameRecord preserves id and createdAt on second apply, updates title")
@MainActor func applyGameRecordMergesOnServerRecordChanged() throws {
let persistence = makeTestPersistence()
