crossmate

A collaborative crossword app for iOS
Log | Files | Refs | LICENSE

commit 3ddfcd3cf7ffed79ce6de41a74cd7b51b4b26273
parent ed9cd0a764a4f0a062bb06d714a002decc3c8e21
Author: Michael Camilleri <[email protected]>
Date:   Thu, 28 May 2026 23:00:17 +0900

Rearchitect session summary push notifications

A pause push for a peer's session was arriving with the body 'added N
letters' but the app icon badge stayed flat. The cells the push
described had already landed in the puzzle while the user was looking at
it, so receiver-side suppression via NotificationState.isSuppressed
correctly held back the badge. The banner, however, is not something an
NSE can withhold — only the badge. The two signals disagreed, which
looked broken even though each half was self-consistent.

This commit moves the whole 'is this news for you' decision to the
sender. publishSessionEndPush now reads each peer's last-known
Player.readAt off the locally synced Player record, then counts cells in
the author's merged-across-devices Moves value whose
TimestampedCell.updatedAt is newer than that peer's readAt. Recipients
whose diff is zero are dropped from the addressee list entirely — no
banner, no badge for them. The remaining recipients each get a body
describing only what they personally haven't seen, so 'added 3 letters'
is now a statement about the recipient, not about the sender's session
boundary. The receiver-side gate is gone, and the NSE collapses to
'pause / win / resign bump the unread-games set; play stamps the current
count'; the set semantics naturally coalesce a later win into the same
badge unit as an earlier pause.

The shared Player.sessionSnapshot baseline that the previous design
relied on for the session-end diff is no longer read or written. The
CKRecord field and the PlayerEntity attribute stay; nothing touches them
in the meantime, so they round-trip as nil. anchorSessionSnapshot and
the matching sessionSnapshotStaleness constant are removed.
PushClient.publish takes [PushClient.Addressee] — each addressee can
carry a per-recipient body that overrides the top-level broadcast text,
which the worker treats as a fallback. cellsAdded / cellsCleared are
gone from both the Swift call site and the worker payload assembly. The
leave-grace machinery on NotificationState stays —
GameStore.mergeRemoteMoves and the foreground willPresent handler still
depend on it.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

Diffstat:
MCrossmate.xcodeproj/project.pbxproj | 8++++----
MCrossmate/Models/PuzzleNotificationText.swift | 31+++++++++++++++++++++++++++++++
MCrossmate/Persistence/GameStore.swift | 71++++++++++++++++++++++++-----------------------------------------------
MCrossmate/Services/AppServices.swift | 172++++++++++++++++++++++++++++++++-----------------------------------------------
MCrossmate/Services/PushClient.swift | 31+++++++++++++++++++++++--------
MNotificationService/NotificationService.swift | 49+++++++++++++------------------------------------
ATests/Unit/GameStoreMergedAuthorCellsTests.swift | 158+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
DTests/Unit/GameStoreSessionSnapshotTests.swift | 145-------------------------------------------------------------------------------
MTests/Unit/PuzzleNotificationTextTests.swift | 55+++++++++++++++++++++++++++++++++++++++++++++++++++++++
MWorker/push-worker.js | 11+++++------
10 files changed, 382 insertions(+), 349 deletions(-)

diff --git a/Crossmate.xcodeproj/project.pbxproj b/Crossmate.xcodeproj/project.pbxproj @@ -22,6 +22,7 @@ 1A19D13D9B820E276C60819E /* InputMonitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6BDD06460A76D4AF31077732 /* InputMonitor.swift */; }; 1CC2D062086FDC5894BFEFA2 /* DiagnosticsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 434862125EC5C0C0F3717ECA /* DiagnosticsView.swift */; }; 1F4E5473F78A5CEDBA9719CE /* NYTAuthService.swift in Sources */ = {isa = PBXBuildFile; fileRef = A253416F4FEA271A80B22A73 /* NYTAuthService.swift */; }; + 262A9CE8C3CB93869190CFF1 /* GameStoreMergedAuthorCellsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 122BC1863D12DE06388D5DA7 /* GameStoreMergedAuthorCellsTests.swift */; }; 267ED5B329F05A30430B73A0 /* EngagementHost.swift in Sources */ = {isa = PBXBuildFile; fileRef = 18C701DAE36000DE19F7CC95 /* EngagementHost.swift */; }; 2AF2550B08CE79F8615B3076 /* FriendZone.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A4AFF292381C9B33C0F2CD6 /* FriendZone.swift */; }; 2B03A1A36AB55495ED0E8684 /* HardwareKeyboardInputView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 947102E58EFCF898D258AC3E /* HardwareKeyboardInputView.swift */; }; @@ -35,7 +36,6 @@ 3A5483EF2893AE325DF27EE8 /* GameMutator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 43DC132D49361C56DE79C13E /* GameMutator.swift */; }; 3C54AE4AA04342CCF5705B20 /* PlayerNamePublisher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 71DFD035381B6252DCD873C9 /* PlayerNamePublisher.swift */; }; 40256E08EE741F4C414B842B /* PuzzleNotificationText.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2F03A9F357672533E2A8DB0 /* PuzzleNotificationText.swift */; }; - 42D795339FFF80C3E730395B /* GameStoreSessionSnapshotTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F9E3B149B506E74AC1B89892 /* GameStoreSessionSnapshotTests.swift */; }; 449B0A09A36B276C93CFB9A4 /* GameStoreUnreadMovesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31C534911020BE4ED2E5065D /* GameStoreUnreadMovesTests.swift */; }; 47584CBEF819C2F507D06DFF /* PlayerColor.swift in Sources */ = {isa = PBXBuildFile; fileRef = DB55FC337CF72C650373210A /* PlayerColor.swift */; }; 4819D7FBB407C9D76510EA2A /* TestHelpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = F97B399E89BBB37730F2F1E9 /* TestHelpers.swift */; }; @@ -175,6 +175,7 @@ 0EB332831AB173ACF6BFEC59 /* SessionMonitor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SessionMonitor.swift; sourceTree = "<group>"; }; 0FD9A43789D0ED123F7A99B0 /* CheckResult.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CheckResult.swift; sourceTree = "<group>"; }; 11BF168D5C1CD85DAE5CAF9E /* PlayerSelectionPublisher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PlayerSelectionPublisher.swift; sourceTree = "<group>"; }; + 122BC1863D12DE06388D5DA7 /* GameStoreMergedAuthorCellsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GameStoreMergedAuthorCellsTests.swift; sourceTree = "<group>"; }; 14B05C19BD4705876B3DF0EC /* GridStateMerger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GridStateMerger.swift; sourceTree = "<group>"; }; 14F2AC5C3B50F4178859E9AC /* CrossmateApp.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CrossmateApp.swift; sourceTree = "<group>"; }; 16AAC1E8D2CB3B5117159934 /* CloudQuery.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CloudQuery.swift; sourceTree = "<group>"; }; @@ -295,7 +296,6 @@ F8E50E7BA98C88B4CAB39DC1 /* CellView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CellView.swift; sourceTree = "<group>"; }; F91707302CBA406BF7FB8F8A /* FriendAvatarView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FriendAvatarView.swift; sourceTree = "<group>"; }; F97B399E89BBB37730F2F1E9 /* TestHelpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestHelpers.swift; sourceTree = "<group>"; }; - F9E3B149B506E74AC1B89892 /* GameStoreSessionSnapshotTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GameStoreSessionSnapshotTests.swift; sourceTree = "<group>"; }; FDE193CAB325C991952D7CE5 /* PUZToXDConverterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PUZToXDConverterTests.swift; sourceTree = "<group>"; }; FEDD63AD5E33E2B0399780EF /* NotificationNavigationBrokerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationNavigationBrokerTests.swift; sourceTree = "<group>"; }; FF159746D076E051C2CB590C /* PlayerSelectionPublisherTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PlayerSelectionPublisherTests.swift; sourceTree = "<group>"; }; @@ -353,7 +353,7 @@ 978A96CC6F550ED7A73F8D96 /* AnnouncementCenterTests.swift */, 60E818B0F4689BAD57660B7C /* GameCursorStoreTests.swift */, BFC1C59A30FB2571598273E4 /* GameMutatorTests.swift */, - F9E3B149B506E74AC1B89892 /* GameStoreSessionSnapshotTests.swift */, + 122BC1863D12DE06388D5DA7 /* GameStoreMergedAuthorCellsTests.swift */, 31C534911020BE4ED2E5065D /* GameStoreUnreadMovesTests.swift */, 6C7F3A9BD7FAF81CB77032A6 /* GridStateMergerTests.swift */, 9AF6157D97271205626E207C /* MovesUpdaterTests.swift */, @@ -683,7 +683,7 @@ 712A2764596A2D17A0BBBF3B /* FriendZoneTests.swift in Sources */, 85A798525FE1DC98210A9E82 /* GameCursorStoreTests.swift in Sources */, 2C0DFC182240A2519ED1FA6A /* GameMutatorTests.swift in Sources */, - 42D795339FFF80C3E730395B /* GameStoreSessionSnapshotTests.swift in Sources */, + 262A9CE8C3CB93869190CFF1 /* GameStoreMergedAuthorCellsTests.swift in Sources */, 449B0A09A36B276C93CFB9A4 /* GameStoreUnreadMovesTests.swift in Sources */, AACC9F70AEEDCB3360FFDEFF /* GridStateMergerTests.swift in Sources */, DE90CC8BE23A0EFC4A32FFA5 /* MovesInboundTests.swift in Sources */, diff --git a/Crossmate/Models/PuzzleNotificationText.swift b/Crossmate/Models/PuzzleNotificationText.swift @@ -16,6 +16,37 @@ enum PuzzleNotificationText { ) } + /// Body for a session-end push, addressed to a single recipient. The + /// `added` / `cleared` counts describe cells *that recipient* hasn't + /// seen yet (cells in the author's merged-across-devices Moves whose + /// `updatedAt` is newer than that recipient's last-known + /// `Player.readAt`). When both counts are zero the recipient should be + /// dropped from the push entirely — the returned no-edits wording is a + /// fallback used for the worker's top-level broadcast body, which is + /// never the visible text under the per-recipient contract. + static func pauseBody( + playerName: String, + puzzleTitle: String, + added: Int, + cleared: Int + ) -> String { + let resolvedName = playerName.isEmpty ? "A player" : playerName + let puzzleSuffix = puzzleTitle.isEmpty + ? "the puzzle" + : "the puzzle '\(puzzleTitle)'" + var parts: [String] = [] + if added > 0 { + parts.append("added \(added) \(added == 1 ? "letter" : "letters")") + } + if cleared > 0 { + parts.append("cleared \(cleared) \(cleared == 1 ? "letter" : "letters")") + } + if parts.isEmpty { + return "\(resolvedName) stopped solving \(puzzleSuffix)." + } + return "\(resolvedName) \(parts.joined(separator: " and ")) in \(puzzleSuffix)" + } + private static func subtitle(publisher: String?, date: Date?) -> String? { let formattedDate = date?.formatted(date: .long, time: .omitted) if let publisher, !publisher.isEmpty, let formattedDate { diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift @@ -932,55 +932,32 @@ final class GameStore { saveContext("setLastMovesSnapshot") } - /// Returns the author's current `sessionSnapshot` (decoded) and Player - /// `updatedAt` for `(gameID, authorID)`. Both fields are nil when no - /// PlayerEntity row exists yet. The snapshot is the baseline an active - /// session writes at begin and clears at end; `updatedAt` doubles as the - /// staleness check (an old snapshot with a fresh `updatedAt` is live; - /// an old snapshot with an old `updatedAt` is a crash leftover) and as - /// the peer-device liveness probe during the pause grace window. - func sessionState( - for gameID: UUID, - by authorID: String - ) -> (snapshot: LocalMovesSnapshot?, updatedAt: Date?) { - guard let entity = fetchPlayerEntity(gameID: gameID, authorID: authorID) else { - return (nil, nil) - } - let snapshot = entity.sessionSnapshot.flatMap { try? LocalMovesSnapshot.decode($0) } - return (snapshot, entity.updatedAt) + /// Player record `updatedAt` for `(gameID, authorID)` — `nil` if no row + /// exists yet. Used as the peer-device liveness probe during the pause + /// grace window: a value newer than `pauseStart` means a sibling device + /// of the same author wrote to Player after we started pausing, i.e. + /// that device is still active and will publish its own pause later. + func playerUpdatedAt(for gameID: UUID, by authorID: String) -> Date? { + fetchPlayerEntity(gameID: gameID, authorID: authorID)?.updatedAt } - /// Writes `snapshot` (encoded) to `PlayerEntity.sessionSnapshot` and - /// bumps `updatedAt` to `now`. Pass `nil` to clear the field at session - /// end. Creates a stub PlayerEntity if none exists yet, mirroring - /// `setLastMovesSnapshot`. Unlike `setLastMovesSnapshot`, the value is - /// included in the outbound Player CKRecord — the caller is responsible - /// for calling `SyncEngine.enqueuePlayer` to ship the change. - func setSessionSnapshot( - _ snapshot: LocalMovesSnapshot?, - for gameID: UUID, - by authorID: String, - now: Date = Date() - ) { - let entity: PlayerEntity - if let existing = fetchPlayerEntity(gameID: gameID, authorID: authorID) { - entity = existing - } else { - let gameRequest = NSFetchRequest<GameEntity>(entityName: "GameEntity") - gameRequest.predicate = NSPredicate(format: "id == %@", gameID as CVarArg) - gameRequest.fetchLimit = 1 - guard let game = try? context.fetch(gameRequest).first else { return } - entity = PlayerEntity(context: context) - entity.game = game - entity.authorID = authorID - entity.ckRecordName = RecordSerializer.recordName( - forPlayerInGame: gameID, - authorID: authorID - ) - } - entity.sessionSnapshot = snapshot.flatMap { try? $0.encoded() } - entity.updatedAt = now - saveContext("setSessionSnapshot") + /// Merged-across-devices author cells for `(gameID, authorID)`. Returns + /// each touched grid position's winning `TimestampedCell` after the + /// usual LWW merge across the author's devices, including cleared cells + /// (empty `letter` with a non-default `updatedAt`). The pause-push + /// per-recipient diff iterates this list, counting cells whose + /// `updatedAt` is newer than that recipient's last-known `Player.readAt`. + func mergedAuthorCells(for gameID: UUID, by authorID: String) -> [TimestampedCell] { + let request = NSFetchRequest<MovesEntity>(entityName: "MovesEntity") + request.predicate = NSPredicate( + format: "game.id == %@ AND authorID == %@", + gameID as CVarArg, + authorID + ) + let entities = (try? context.fetch(request)) ?? [] + let values: [MovesValue] = entities.compactMap { Self.movesValue(from: $0) } + guard !values.isEmpty else { return [] } + return GridStateMerger.mergeWithProvenance(values).values.map(\.cell) } /// Distinct authorIDs that have written a `MovesEntity` for `gameID`, diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift @@ -63,14 +63,6 @@ final class AppServices { /// call) should not fan out pause/play pings to peers on every flicker — /// only a sustained absence does. static let sessionEndGrace: TimeInterval = 120 - /// Window past which a non-nil `Player.sessionSnapshot` is treated as a - /// crashed-session leftover rather than a live session belonging to - /// another device of this author. Comfortably exceeds any plausible - /// mid-session idle gap (selection writes, readAt writes, and the - /// pause-grace window are all much shorter), and well below "user came - /// back the next day" so a real abandoned baseline doesn't suppress a - /// genuine new-session begin. - static let sessionSnapshotStaleness: TimeInterval = 30 * 60 private var pendingSessionEndTasks: [UUID: Task<Void, Never>] = [:] let shareController: ShareController let friendController: FriendController @@ -614,18 +606,14 @@ final class AppServices { /// Sender-side session-begin push. Replaces the receiver-side /// `SessionMonitor.presentBegins(...)` path: the opening device owns the /// notification timing, so peers get "Alice is solving X" the instant - /// Alice opens the puzzle. Anchors the matching pause push by adopting - /// or writing a shared `Player.sessionSnapshot` so the cumulative delta - /// reported at session end spans every device of this author, not just - /// this one. Kind is `play` (rather than `join`) to avoid collision with - /// the `PingKind.join` invite-accept ping — the user-facing event is - /// "started playing", not "joined". + /// Alice opens the puzzle. Kind is `play` (rather than `join`) to avoid + /// collision with the `PingKind.join` invite-accept ping — the user-facing + /// event is "started playing", not "joined". func publishSessionBeginPush(gameID: UUID) async { guard let localAuthorID = identity.currentID, !localAuthorID.isEmpty else { syncMonitor.note("push(play): skipped (no authorID)") return } - await anchorSessionSnapshot(gameID: gameID, authorID: localAuthorID) guard let pushClient else { syncMonitor.note("push(play): skipped (no pushClient)") return @@ -652,36 +640,12 @@ final class AppServices { await pushClient.publish( kind: "play", gameID: gameID, - addressees: plan.recipients, + addressees: plan.recipients.map { PushClient.Addressee(authorID: $0.authorID) }, title: "Crossmate", body: "\(playerName) is solving \(puzzleSuffix)" ) } - /// Adopts `Player.sessionSnapshot` if a fresh one already exists (another - /// device of this author began a session within the staleness window), - /// otherwise captures the author's merged-across-devices snapshot and - /// writes it to Player. Either way, when the matching pause push fires - /// on whichever device pauses last, it diffs against this shared - /// baseline so the reported delta covers every device's contribution. - private func anchorSessionSnapshot(gameID: UUID, authorID: String) async { - let now = Date() - let state = store.sessionState(for: gameID, by: authorID) - if state.snapshot != nil, - let updatedAt = state.updatedAt, - now.timeIntervalSince(updatedAt) < Self.sessionSnapshotStaleness { - syncMonitor.note("session-snapshot: adopted") - return - } - let snapshot = store.movesSnapshot(for: gameID, by: authorID, on: nil) - store.setSessionSnapshot(snapshot, for: gameID, by: authorID, now: now) - await syncEngine.enqueuePlayer( - gameID: gameID, - authorID: authorID, - reason: "session-begin" - ) - } - /// Defer the session-end push by `seconds`. Cancels any previously /// scheduled pause for the same game. If the user resumes within the /// grace window, call `cancelPendingSessionEndPush` to drop the timer @@ -714,16 +678,16 @@ final class AppServices { return true } - /// Sender-side session-end push. Diffs the author's current - /// merged-across-devices Moves snapshot against the shared - /// `Player.sessionSnapshot` baseline and ships the summary. Suppresses - /// the push when a peer device of this author wrote to Player during - /// the grace window — that device is still playing and will report the - /// cumulative delta when it pauses. Otherwise clears the shared - /// snapshot so the next session-begin re-anchors fresh. A no-edit - /// pause still fires (peers need to know the session ended); the - /// `cellsAdded`/`cellsCleared` fields ride along so the receiver-side - /// Notification Service Extension can decide whether to bump the badge. + /// Sender-side session-end push. For each recipient, counts cells in + /// the author's merged-across-devices Moves whose `updatedAt` is newer + /// than that recipient's last-known `Player.readAt`, and ships a body + /// describing only what *that* recipient hasn't seen. Recipients whose + /// readAt already covers every author cell are dropped — they have + /// nothing unseen, so a banner-and-badge for them would be misleading. + /// + /// Suppresses the push when a peer device of this author wrote to + /// Player during the grace window — that device is still playing and + /// will publish its own pause when it stops. func publishSessionEndPush(gameID: UUID, pauseStart: Date = Date()) async { // A direct call (e.g. from `.onDisappear`) supersedes any pending // grace-window timer for this game — drop it so we don't fire a @@ -733,34 +697,17 @@ final class AppServices { syncMonitor.note("push(pause): skipped (no authorID)") return } - let state = store.sessionState(for: gameID, by: localAuthorID) // During the grace window this device wrote nothing to Player // (any local activity would have reset the timer via // `cancelPendingSessionEndPush`). A Player `updatedAt` newer than // pauseStart therefore came from another device of this author — - // that device is still active, leave the baseline intact so its - // eventual pause reports cumulatively. - if let updatedAt = state.updatedAt, updatedAt > pauseStart { + // that device is still active, so let its eventual pause cover + // the session. + if let updatedAt = store.playerUpdatedAt(for: gameID, by: localAuthorID), + updatedAt > pauseStart { syncMonitor.note("push(pause): skipped (peer device active)") return } - guard let baseline = state.snapshot else { - syncMonitor.note("push(pause): skipped (no session snapshot)") - return - } - let current = store.movesSnapshot(for: gameID, by: localAuthorID, on: nil) - let added = current.filled.subtracting(baseline.filled).count - let cleared = current.cleared.subtracting(baseline.cleared).count - // Session is over from this device's perspective. Clear the shared - // snapshot regardless of whether we actually send a push — no-edit - // sessions, no-recipient sessions, and completed games all still - // need the field cleared so a future begin re-anchors fresh. - store.setSessionSnapshot(nil, for: gameID, by: localAuthorID) - await syncEngine.enqueuePlayer( - gameID: gameID, - authorID: localAuthorID, - reason: "session-end" - ) guard let pushClient else { syncMonitor.note("push(pause): skipped (no pushClient)") return @@ -780,33 +727,43 @@ final class AppServices { syncMonitor.note("push(pause): skipped (access revoked)") return } - let playerName = preferences.name.isEmpty ? "A player" : preferences.name - let puzzleSuffix = plan.title.isEmpty ? "the puzzle" : "the puzzle '\(plan.title)'" - let body: String - if added == 0 && cleared == 0 { - // Peers still need to know the session ended — otherwise they're - // left thinking the player is still active. The badge logic on the - // receiver side keys off `cellsAdded`/`cellsCleared` so a zero - // pause doesn't tick the unread-games count. - body = "\(playerName) stopped solving \(puzzleSuffix) without making any changes." - } else { - var parts: [String] = [] - if added > 0 { - parts.append("added \(added) \(added == 1 ? "letter" : "letters")") + let mergedCells = store.mergedAuthorCells(for: gameID, by: localAuthorID) + var addressees: [PushClient.Addressee] = [] + for recipient in plan.recipients { + let readAt = recipient.readAt ?? .distantPast + var added = 0 + var cleared = 0 + for cell in mergedCells where cell.updatedAt > readAt { + if cell.letter.isEmpty { cleared += 1 } else { added += 1 } } - if cleared > 0 { - parts.append("cleared \(cleared) \(cleared == 1 ? "letter" : "letters")") - } - body = "\(playerName) \(parts.joined(separator: " and ")) in \(puzzleSuffix)" + guard added + cleared > 0 else { continue } + let body = PuzzleNotificationText.pauseBody( + playerName: preferences.name, + puzzleTitle: plan.title, + added: added, + cleared: cleared + ) + addressees.append(PushClient.Addressee(authorID: recipient.authorID, body: body)) } + guard !addressees.isEmpty else { + syncMonitor.note("push(pause): skipped (all recipients up to date)") + return + } + // Top-level broadcast body is the worker's fallback if an addressee + // carries no per-recipient body. Under the new contract every + // addressee has one, but the field is still required. + let fallbackBody = PuzzleNotificationText.pauseBody( + playerName: preferences.name, + puzzleTitle: plan.title, + added: 0, + cleared: 0 + ) await pushClient.publish( kind: "pause", gameID: gameID, - addressees: plan.recipients, + addressees: addressees, title: "Crossmate", - body: body, - cellsAdded: added, - cellsCleared: cleared + body: fallbackBody ) } @@ -839,17 +796,24 @@ final class AppServices { await pushClient.publish( kind: kind, gameID: gameID, - addressees: plan.recipients, + addressees: plan.recipients.map { PushClient.Addressee(authorID: $0.authorID) }, title: "Crossmate", body: body ) } /// Everything a sender-side push helper needs to know about a game in - /// one Core Data round-trip: the roster authors to notify, the puzzle's - /// display title, and the gating flags callers consult before emitting. + /// one Core Data round-trip: the roster authors to notify (each with the + /// last-known `Player.readAt` so the pause path can compute a + /// per-recipient diff), the puzzle's display title, and the gating flags + /// callers consult before emitting. + struct PushRecipient: Sendable, Equatable { + let authorID: String + let readAt: Date? + } + private struct PushPlan { - let recipients: [String] + let recipients: [PushRecipient] let title: String let completedAt: Date? let isAccessRevoked: Bool @@ -872,18 +836,20 @@ final class AppServices { gReq.predicate = NSPredicate(format: "id == %@", gameID as CVarArg) gReq.fetchLimit = 1 guard let game = try? ctx.fetch(gReq).first else { return .empty } - var authors = Set<String>() + var byAuthor: [String: Date?] = [:] let pReq = NSFetchRequest<PlayerEntity>(entityName: "PlayerEntity") pReq.predicate = NSPredicate(format: "game == %@", game) for p in (try? ctx.fetch(pReq)) ?? [] { - guard let a = p.authorID else { continue } - authors.insert(a) + guard let a = p.authorID, + a != localAuthorID, + a != CKCurrentUserDefaultName, + !a.isEmpty + else { continue } + byAuthor[a] = p.readAt } - authors.remove(localAuthorID) - authors.remove(CKCurrentUserDefaultName) - authors.remove("") + let recipients = byAuthor.map { PushRecipient(authorID: $0.key, readAt: $0.value) } return PushPlan( - recipients: Array(authors), + recipients: recipients, title: PuzzleNotificationText.title(for: game), completedAt: game.completedAt, isAccessRevoked: game.isAccessRevoked diff --git a/Crossmate/Services/PushClient.swift b/Crossmate/Services/PushClient.swift @@ -93,6 +93,20 @@ final class PushClient { } } + /// One push addressee. `body`, if set, overrides the top-level broadcast + /// `body` for this recipient only — the receiver-side notification text + /// can then be personalised (e.g. the pause-summary counts that depend on + /// when that specific peer last read the puzzle). + struct Addressee: Sendable, Equatable { + let authorID: String + let body: String? + + init(authorID: String, body: String? = nil) { + self.authorID = authorID + self.body = body + } + } + /// Fire-and-forget publish. The worker fans the push out to every /// addressee author's registered devices. Failures are logged but never /// surfaced — pushes are advisory and the next event will retry the @@ -100,24 +114,25 @@ final class PushClient { func publish( kind: String, gameID: UUID, - addressees: [String], + addressees: [Addressee], title: String, - body: String, - cellsAdded: Int? = nil, - cellsCleared: Int? = nil + body: String ) async { guard !addressees.isEmpty else { return } log("push(\(kind)): publishing to \(addressees.count) addressee(s)") - var payload: [String: Any] = [ + let addresseePayloads: [[String: Any]] = addressees.map { addressee in + var entry: [String: Any] = ["authorID": addressee.authorID] + if let body = addressee.body { entry["body"] = body } + return entry + } + let payload: [String: Any] = [ "kind": kind, "gameID": gameID.uuidString, "fromAuthorID": authorID ?? "", "title": title, "alertBody": body, - "addressees": addressees.map { ["authorID": $0] } + "addressees": addresseePayloads ] - if let cellsAdded { payload["cellsAdded"] = cellsAdded } - if let cellsCleared { payload["cellsCleared"] = cellsCleared } var request = URLRequest(url: baseURL.appendingPathComponent("publish")) request.httpMethod = "POST" applyAuth(&request) diff --git a/NotificationService/NotificationService.swift b/NotificationService/NotificationService.swift @@ -12,16 +12,16 @@ import UserNotifications /// notification's `badge` field. Once the main app foregrounds it unions /// Core Data ground truth into the same set and re-stamps. /// -/// The payload from the push worker carries `kind` and `gameID`. Bumping -/// rules: -/// - `play` — presence ping, never bumps. -/// - `pause` — bumps only when `cellsAdded`/`cellsCleared` indicate edits -/// (peers still need a no-edit pause to know the session ended). -/// - `win` / `resign` — always bump (completion is by definition new). -/// -/// We also honour `NotificationState.isSuppressed(gameID:)` so a peer push -/// for the puzzle the user is currently viewing (or just left, within the -/// leave-grace window) doesn't tick the badge. +/// The sender does all gating before publishing — pause pushes are diffed +/// per recipient against that recipient's last-known `Player.readAt`, and a +/// recipient with no unseen cells is dropped from the addressee list +/// entirely. Anything that reaches the NSE is therefore worth flagging: +/// - `pause` / `win` / `resign` — bump the unread set for `gameID`. The +/// set semantics make repeats idempotent (a pause followed by a win for +/// the same game is one badge unit, not two). +/// - `play` — presence ping; the grid hasn't changed yet, so there's +/// nothing to mark unread. Stamp the current count to keep the badge in +/// sync without growing it. final class NotificationService: UNNotificationServiceExtension { private var contentHandler: ((UNNotificationContent) -> Void)? @@ -43,14 +43,10 @@ final class NotificationService: UNNotificationServiceExtension { let kind = userInfo["kind"] as? String let gameID = (userInfo["gameID"] as? String).flatMap(UUID.init(uuidString:)) - if shouldBumpBadge(kind: kind, gameID: gameID, userInfo: userInfo), - let gameID { - let newCount = BadgeState.markUnread(gameID: gameID) - bestAttemptContent.badge = NSNumber(value: newCount) + if let gameID, kind == "pause" || kind == "win" || kind == "resign" { + let count = BadgeState.markUnread(gameID: gameID) + bestAttemptContent.badge = NSNumber(value: count) } else { - // Either presence-only (play), no-edit pause, suppressed, or the - // payload is malformed. Stamp the current count so the badge at - // least tracks reality, but don't grow it. bestAttemptContent.badge = NSNumber(value: BadgeState.unreadGameIDs().count) } @@ -62,23 +58,4 @@ final class NotificationService: UNNotificationServiceExtension { contentHandler(bestAttemptContent) } } - - private func shouldBumpBadge( - kind: String?, - gameID: UUID?, - userInfo: [AnyHashable: Any] - ) -> Bool { - guard let kind, let gameID else { return false } - if NotificationState.isSuppressed(gameID: gameID) { return false } - switch kind { - case "win", "resign": - return true - case "pause": - let added = (userInfo["cellsAdded"] as? NSNumber)?.intValue ?? 0 - let cleared = (userInfo["cellsCleared"] as? NSNumber)?.intValue ?? 0 - return added > 0 || cleared > 0 - default: - return false - } - } } diff --git a/Tests/Unit/GameStoreMergedAuthorCellsTests.swift b/Tests/Unit/GameStoreMergedAuthorCellsTests.swift @@ -0,0 +1,158 @@ +import CoreData +import Foundation +import Testing + +@testable import Crossmate + +/// `GameStore.mergedAuthorCells` is the data source for the per-recipient +/// pause-push diff: each touched grid position's winning `TimestampedCell` +/// after LWW-merging the author's MovesEntities across devices, including +/// cleared cells whose latest write left them empty. +@Suite("GameStore.mergedAuthorCells", .isolatedNotificationState) +@MainActor +struct GameStoreMergedAuthorCellsTests { + + private static let authorID = "alice" + private static let puzzleSource = """ + Title: Test Puzzle + Author: Test + + + AB + CD + """ + + private func makeGame(in ctx: NSManagedObjectContext) throws -> (GameEntity, UUID) { + let gameID = UUID() + let entity = GameEntity(context: ctx) + entity.id = gameID + entity.title = "Test" + entity.puzzleSource = Self.puzzleSource + entity.createdAt = Date() + entity.updatedAt = Date() + entity.ckRecordName = "game-\(gameID.uuidString)" + entity.ckZoneName = "game-\(gameID.uuidString)" + entity.databaseScope = 1 + try ctx.save() + return (entity, gameID) + } + + private func addMoves( + for entity: GameEntity, + gameID: UUID, + deviceID: String, + cells: [GridPosition: TimestampedCell], + updatedAt: Date, + in ctx: NSManagedObjectContext + ) throws { + let row = MovesEntity(context: ctx) + row.game = entity + row.authorID = Self.authorID + row.deviceID = deviceID + row.cells = try MovesCodec.encode(cells) + row.updatedAt = updatedAt + row.ckRecordName = RecordSerializer.recordName( + forMovesInGame: gameID, + authorID: Self.authorID, + deviceID: deviceID + ) + try ctx.save() + } + + private func cell( + letter: String, + at updatedAt: Date + ) -> TimestampedCell { + TimestampedCell( + letter: letter, + markKind: 0, + checkedRight: false, + checkedWrong: false, + updatedAt: updatedAt, + authorID: Self.authorID + ) + } + + @Test("Returns an empty list when the author has no Moves rows") + func emptyWhenNoMoves() throws { + let persistence = makeTestPersistence() + let store = makeTestStore(persistence: persistence) + let (_, gameID) = try makeGame(in: persistence.viewContext) + + let cells = store.mergedAuthorCells(for: gameID, by: Self.authorID) + + #expect(cells.isEmpty) + } + + @Test("Later-timestamped write across devices wins for the same position") + func laterWriteWins() throws { + let persistence = makeTestPersistence() + let store = makeTestStore(persistence: persistence) + let (entity, gameID) = try makeGame(in: persistence.viewContext) + let earlier = Date(timeIntervalSince1970: 1_000) + let later = Date(timeIntervalSince1970: 2_000) + + try addMoves( + for: entity, + gameID: gameID, + deviceID: "ipad", + cells: [GridPosition(row: 0, col: 0): cell(letter: "A", at: earlier)], + updatedAt: earlier, + in: persistence.viewContext + ) + try addMoves( + for: entity, + gameID: gameID, + deviceID: "iphone", + cells: [GridPosition(row: 0, col: 0): cell(letter: "B", at: later)], + updatedAt: later, + in: persistence.viewContext + ) + + let cells = store.mergedAuthorCells(for: gameID, by: Self.authorID) + + #expect(cells.count == 1) + let winner = try #require(cells.first) + #expect(winner.letter == "B") + #expect(winner.updatedAt == later) + } + + @Test("Cleared cells (empty letter) are retained so the diff can count them") + func clearedCellsRetained() throws { + let persistence = makeTestPersistence() + let store = makeTestStore(persistence: persistence) + let (entity, gameID) = try makeGame(in: persistence.viewContext) + let early = Date(timeIntervalSince1970: 1_000) + let later = Date(timeIntervalSince1970: 2_000) + + try addMoves( + for: entity, + gameID: gameID, + deviceID: "ipad", + cells: [ + GridPosition(row: 0, col: 0): cell(letter: "A", at: early), + GridPosition(row: 0, col: 1): cell(letter: "B", at: early) + ], + updatedAt: early, + in: persistence.viewContext + ) + // Later write on a sibling device clears (0, 1). + try addMoves( + for: entity, + gameID: gameID, + deviceID: "iphone", + cells: [GridPosition(row: 0, col: 1): cell(letter: "", at: later)], + updatedAt: later, + in: persistence.viewContext + ) + + let cells = store.mergedAuthorCells(for: gameID, by: Self.authorID) + + #expect(cells.count == 2) + let cleared = cells.first { $0.letter.isEmpty } + let filled = cells.first { !$0.letter.isEmpty } + #expect(cleared?.updatedAt == later) + #expect(filled?.letter == "A") + #expect(filled?.updatedAt == early) + } +} diff --git a/Tests/Unit/GameStoreSessionSnapshotTests.swift b/Tests/Unit/GameStoreSessionSnapshotTests.swift @@ -1,145 +0,0 @@ -import CoreData -import Foundation -import Testing - -@testable import Crossmate - -/// Pins down the shared per-author session-snapshot field on `PlayerEntity`. -/// The sender-side pause push anchors against this baseline so the cumulative -/// delta spans every device of an author, not just the device emitting the -/// push (see `AppServices.anchorSessionSnapshot` / `publishSessionEndPush`). -@Suite("GameStore session snapshot", .isolatedNotificationState) -@MainActor -struct GameStoreSessionSnapshotTests { - - private static let authorID = "author-1" - - private static let puzzleSource = """ - Title: Test Puzzle - Author: Test - - - ABC - D#E - FGH - - - A1. Across 1 ~ ABC - A4. Across 4 ~ DE - A5. Across 5 ~ FGH - D1. Down 1 ~ ADF - D2. Down 2 ~ BG - D3. Down 3 ~ CEH - """ - - private struct Fixture { - let persistence: PersistenceController - let store: GameStore - let gameID: UUID - } - - private func makeFixture() throws -> Fixture { - let persistence = makeTestPersistence() - let store = makeTestStore(persistence: persistence) - let ctx = persistence.viewContext - let gameID = UUID() - let xd = try XD.parse(Self.puzzleSource) - let entity = GameEntity(context: ctx) - entity.id = gameID - entity.title = "Test" - entity.puzzleSource = Self.puzzleSource - entity.createdAt = Date() - entity.updatedAt = Date() - entity.ckRecordName = "game-\(gameID.uuidString)" - entity.populateCachedSummaryFields(from: Puzzle(xd: xd)) - try ctx.save() - return Fixture(persistence: persistence, store: store, gameID: gameID) - } - - private func snapshot(filled: [(Int, Int)] = [], cleared: [(Int, Int)] = []) -> LocalMovesSnapshot { - LocalMovesSnapshot( - filled: Set(filled.map { GridPosition(row: $0.0, col: $0.1) }), - cleared: Set(cleared.map { GridPosition(row: $0.0, col: $0.1) }) - ) - } - - @Test("sessionState returns (nil, nil) when no PlayerEntity exists") - func sessionStateMissingRow() throws { - let fixture = try makeFixture() - let state = fixture.store.sessionState(for: fixture.gameID, by: Self.authorID) - #expect(state.snapshot == nil) - #expect(state.updatedAt == nil) - } - - @Test("setSessionSnapshot creates a stub PlayerEntity and round-trips the value") - func setSessionSnapshotCreatesStub() throws { - let fixture = try makeFixture() - let baseline = snapshot(filled: [(0, 0), (0, 1)], cleared: [(2, 2)]) - let now = Date() - - fixture.store.setSessionSnapshot(baseline, for: fixture.gameID, by: Self.authorID, now: now) - - let state = fixture.store.sessionState(for: fixture.gameID, by: Self.authorID) - #expect(state.snapshot == baseline) - #expect(state.updatedAt == now) - } - - @Test("setSessionSnapshot(nil) clears the field while leaving the row intact") - func setSessionSnapshotClears() throws { - let fixture = try makeFixture() - let baseline = snapshot(filled: [(0, 0)]) - fixture.store.setSessionSnapshot(baseline, for: fixture.gameID, by: Self.authorID) - - let clearAt = Date().addingTimeInterval(10) - fixture.store.setSessionSnapshot(nil, for: fixture.gameID, by: Self.authorID, now: clearAt) - - let state = fixture.store.sessionState(for: fixture.gameID, by: Self.authorID) - #expect(state.snapshot == nil) - // updatedAt is still bumped — the clear write is itself activity that - // peer devices' suppression checks must see. - #expect(state.updatedAt == clearAt) - } - - @Test("setSessionSnapshot bumps updatedAt on every write") - func setSessionSnapshotBumpsUpdatedAt() throws { - let fixture = try makeFixture() - let first = Date() - fixture.store.setSessionSnapshot(snapshot(filled: [(0, 0)]), for: fixture.gameID, by: Self.authorID, now: first) - let firstState = fixture.store.sessionState(for: fixture.gameID, by: Self.authorID) - #expect(firstState.updatedAt == first) - - let second = first.addingTimeInterval(60) - fixture.store.setSessionSnapshot(snapshot(filled: [(0, 0), (0, 1)]), for: fixture.gameID, by: Self.authorID, now: second) - let secondState = fixture.store.sessionState(for: fixture.gameID, by: Self.authorID) - #expect(secondState.updatedAt == second) - #expect(secondState.snapshot == snapshot(filled: [(0, 0), (0, 1)])) - } - - @Test("setSessionSnapshot reuses an existing PlayerEntity rather than creating a duplicate") - func setSessionSnapshotReusesRow() throws { - let fixture = try makeFixture() - let ctx = fixture.persistence.viewContext - let recordName = RecordSerializer.recordName( - forPlayerInGame: fixture.gameID, - authorID: Self.authorID - ) - let gameReq = NSFetchRequest<GameEntity>(entityName: "GameEntity") - gameReq.predicate = NSPredicate(format: "id == %@", fixture.gameID as CVarArg) - let game = try #require(try ctx.fetch(gameReq).first) - let existing = PlayerEntity(context: ctx) - existing.game = game - existing.authorID = Self.authorID - existing.ckRecordName = recordName - existing.name = "Original" - existing.updatedAt = Date() - try ctx.save() - - fixture.store.setSessionSnapshot(snapshot(filled: [(1, 0)]), for: fixture.gameID, by: Self.authorID) - - let req = NSFetchRequest<PlayerEntity>(entityName: "PlayerEntity") - req.predicate = NSPredicate(format: "ckRecordName == %@", recordName) - let rows = try ctx.fetch(req) - #expect(rows.count == 1) - #expect(rows.first?.name == "Original") - } -} diff --git a/Tests/Unit/PuzzleNotificationTextTests.swift b/Tests/Unit/PuzzleNotificationTextTests.swift @@ -32,6 +32,61 @@ struct PuzzleNotificationTextTests { #expect(AppServices.bodyText(for: ping) == "Alice joined the puzzle 'Saturday Puzzle – 1 January 2001'") } + @Test("pauseBody combines added and cleared counts when both are non-zero") + func pauseBodyAddedAndCleared() { + let body = PuzzleNotificationText.pauseBody( + playerName: "Alice", + puzzleTitle: "Saturday Puzzle", + added: 3, + cleared: 2 + ) + + #expect(body == "Alice added 3 letters and cleared 2 letters in the puzzle 'Saturday Puzzle'") + } + + @Test("pauseBody pluralises only multi-letter counts") + func pauseBodyPluralisation() { + let added = PuzzleNotificationText.pauseBody( + playerName: "Alice", + puzzleTitle: "X", + added: 1, + cleared: 0 + ) + let cleared = PuzzleNotificationText.pauseBody( + playerName: "Alice", + puzzleTitle: "X", + added: 0, + cleared: 1 + ) + + #expect(added == "Alice added 1 letter in the puzzle 'X'") + #expect(cleared == "Alice cleared 1 letter in the puzzle 'X'") + } + + @Test("pauseBody falls back to no-edits wording when both counts are zero") + func pauseBodyZeroCounts() { + let body = PuzzleNotificationText.pauseBody( + playerName: "Alice", + puzzleTitle: "Saturday Puzzle", + added: 0, + cleared: 0 + ) + + #expect(body == "Alice stopped solving the puzzle 'Saturday Puzzle'.") + } + + @Test("pauseBody substitutes a default name and a generic suffix when missing") + func pauseBodyMissingNameAndTitle() { + let body = PuzzleNotificationText.pauseBody( + playerName: "", + puzzleTitle: "", + added: 2, + cleared: 0 + ) + + #expect(body == "A player added 2 letters in the puzzle") + } + @Test("Invite body names the inviter and puzzle") func inviteBody() { let ping = Ping( diff --git a/Worker/push-worker.js b/Worker/push-worker.js @@ -70,7 +70,7 @@ export class PushRegistry { async handlePublish(request) { const body = await readJSON(request); if (!body) return badRequest("Body must be JSON"); - const { kind, addressees, gameID, fromAuthorID, title, alertBody, cellsAdded, cellsCleared } = body; + const { kind, addressees, gameID, fromAuthorID, title, alertBody } = body; if (!kind || !Array.isArray(addressees) || addressees.length === 0) { return badRequest("kind and non-empty addressees required"); } @@ -89,9 +89,7 @@ export class PushRegistry { gameID, fromAuthorID, title, - body: alertBody, - cellsAdded, - cellsCleared + body: target.body || alertBody }); if (result === "ok") delivered += 1; else if (result === "drop") { @@ -108,6 +106,7 @@ export class PushRegistry { const targets = []; for (const addressee of addressees) { if (!addressee || !addressee.authorID) continue; + const body = typeof addressee.body === "string" ? addressee.body : undefined; if (addressee.deviceID) { const stored = await this.state.storage.get( `token:${addressee.authorID}:${addressee.deviceID}` @@ -116,6 +115,7 @@ export class PushRegistry { targets.push({ authorID: addressee.authorID, deviceID: addressee.deviceID, + body, ...stored }); } @@ -129,6 +129,7 @@ export class PushRegistry { targets.push({ authorID: addressee.authorID, deviceID, + body, ...value }); } @@ -151,8 +152,6 @@ export class PushRegistry { }; if (message.gameID) payload.gameID = message.gameID; if (message.fromAuthorID) payload.fromAuthorID = message.fromAuthorID; - if (Number.isFinite(message.cellsAdded)) payload.cellsAdded = message.cellsAdded; - if (Number.isFinite(message.cellsCleared)) payload.cellsCleared = message.cellsCleared; const response = await fetch(`https://${host}/3/device/${target.token}`, { method: "POST",