crossmate

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

commit e697be1ef4f3ba8c061bb62f346836113ca347a3
parent 4ea6f4f986c9aa61f55744bb0b0666cf75c45f27
Author: Michael Camilleri <[email protected]>
Date:   Sun, 31 May 2026 05:03:36 +0900

Gate the peer cursor and engagement teardown on the readAt lease

Two signals decided 'is a peer here' and they disagreed. Engagement
teardown and the selection-publisher send gate used the active-lease
predicate readAt > now (Player.readAt, set ~10 min ahead while a puzzle
is open and collapsed to now on leave). The peer cursor instead used an
unrelated 60s freshness window on the selection's updatedAt. So a peer
who paused for think-time lost their cursor after 60s while still
sitting on the puzzle, and the cursor could vanish while the engagement
bolt — driven by the local socket — stayed lit, because the two tracked
different things.

This commit makes readAt > now the single presence rule.
PeerPresence.isPresent captures it; PlayerRoster now reads each peer's
readAt and gates both the persisted and live-engagement cursor tracks on
it, holding the last-known track regardless of selection age.
hasPresentPeer and presentPeers route their decision through the same
predicate. The bolt stays bound to the socket — but the socket's
lifecycle already keys on hasPresentPeer, so it follows.

Teardown was otherwise only as prompt as the 30s reconnect tick. For a
clean leave the lease collapses and the presence-change handler
reconciles at once; for a dirty leave (crash, lost network, force-quit)
the lease just expires. Now, a one-shot recompute is scheduled at the
soonest peer lease expiry on both sides: PlayerRoster clears the ghost
cursor, and reconcileEngagement re-reconciles — tearing down (dropping
the bolt) if no renewal landed, or rescheduling onto the renewed horizon
if one did. The 30s tick remains the coarse backstop. Both
self-terminate once no peer is present.

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

Diffstat:
MCrossmate/Models/PlayerRoster.swift | 208+++++++++++++++++++++++++++++++++++++++++++++++--------------------------------
MCrossmate/Services/AppServices.swift | 88++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
MCrossmate/Sync/Presence.swift | 11+++++++++++
MTests/Unit/PlayerRosterTests.swift | 46+++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 256 insertions(+), 97 deletions(-)

diff --git a/Crossmate/Models/PlayerRoster.swift b/Crossmate/Models/PlayerRoster.swift @@ -34,22 +34,39 @@ final class PlayerRoster { let updatedAt: Date } - /// Peer cursor tracks keyed by `authorID`. Stale entries (older than the - /// freshness window) are dropped on each refresh. The local player is - /// never present in this map. + /// The Core Data snapshot read off the background context in `refresh()`. + /// A struct rather than a tuple so the empty-game path, the populated + /// return, and the consumers stay in sync by name rather than position. + private struct FetchedRoster { + var databaseScope: Int16 = 0 + var ckShareRecordName: String? + var ckZoneName: String? + var ckZoneOwnerName: String? + var namesMap: [String: String] = [:] + var moveAuthorIDs: [String] = [] + var rawSelections: [RawSelection] = [] + var readAtByAuthor: [String: Date] = [:] + } + + /// Last-known peer cursor tracks keyed by `authorID`, from the synced + /// Player record. Held unconditionally; visibility is decided at read time + /// by the presence gate in `remoteSelections`. The local player is never + /// present in this map. private var persistedRemoteSelections: [String: RemoteSelection] = [:] + /// Each non-local peer's active-session lease (`Player.readAt`). A cursor + /// is shown only while its peer is present (`PeerPresence.isPresent`), so a + /// peer who pauses keeps their cursor and a departed peer's cursor clears + /// when the lease lapses — the same heuristic that gates engagement. + private var remoteReadAt: [String: Date] = [:] + var remoteSelections: [String: RemoteSelection] { var merged = persistedRemoteSelections let colorByAuthor = Dictionary( uniqueKeysWithValues: entries.filter { !$0.isLocal }.map { ($0.authorID, $0.color) } ) - let now = Date() for (authorID, engagement) in engagementStore.selections(for: gameID) { guard let color = colorByAuthor[authorID] else { continue } - guard now.timeIntervalSince(engagement.updatedAt) < selectionFreshnessWindow else { - continue - } let selection = RemoteSelection( authorID: authorID, row: engagement.row, @@ -62,15 +79,10 @@ final class PlayerRoster { merged[authorID] = selection } } - return merged + let now = Date() + return merged.filter { PeerPresence.isPresent(readAt: remoteReadAt[$0.key], asOf: now) } } - /// Selections older than this are treated as stale and hidden — covers - /// the case where the peer crashed or lost connectivity without writing - /// a "cleared" record. The polling interval is 5s plus push, so 60s is - /// generous enough to ride out a brief network hiccup. - private let selectionFreshnessWindow: TimeInterval = 60 - private(set) var entries: [Entry] = [] private(set) var localAuthorID: String? @@ -85,6 +97,10 @@ final class PlayerRoster { private var cachedShare: CKShare? private var observationTasks: [Task<Void, Never>] = [] private var lastTracedSignature: String? + /// One-shot recompute scheduled for the soonest peer lease expiry, so a + /// departed peer's ghost cursor clears precisely when its lease lapses + /// rather than waiting for an unrelated record to nudge a refresh. + private var leaseExpiryTask: Task<Void, Never>? init( gameID: UUID, @@ -109,6 +125,7 @@ final class PlayerRoster { for task in observationTasks { task.cancel() } + leaseExpiryTask?.cancel() } // MARK: - Observation @@ -146,97 +163,103 @@ final class PlayerRoster { self.localAuthorID = nil entries = [] persistedRemoteSelections = [:] + remoteReadAt = [:] + leaseExpiryTask?.cancel() + leaseExpiryTask = nil return } self.localAuthorID = localAuthorID // Pull Core Data fields off a background context. let ctx = persistence.container.newBackgroundContext() - let (databaseScope, ckShareRecordName, ckZoneName, ckZoneOwnerName, namesMap, moveAuthorIDs, rawSelections) = - ctx.performAndWait { () -> (Int16, String?, String?, String?, [String: String], [String], [RawSelection]) in - let req = NSFetchRequest<GameEntity>(entityName: "GameEntity") - req.predicate = NSPredicate(format: "id == %@", gameID as CVarArg) - req.fetchLimit = 1 - guard let entity = try? ctx.fetch(req).first else { - return (0, nil, nil, nil, [:], [], []) + let fetched = ctx.performAndWait { () -> FetchedRoster in + let req = NSFetchRequest<GameEntity>(entityName: "GameEntity") + req.predicate = NSPredicate(format: "id == %@", gameID as CVarArg) + req.fetchLimit = 1 + guard let entity = try? ctx.fetch(req).first else { + return FetchedRoster() + } + let nameReq = NSFetchRequest<PlayerEntity>(entityName: "PlayerEntity") + nameReq.predicate = NSPredicate(format: "game == %@", entity) + let nameEntities = (try? ctx.fetch(nameReq)) ?? [] + var namesMap: [String: String] = [:] + var selections: [RawSelection] = [] + var readAtByAuthor: [String: Date] = [:] + for nr in nameEntities { + guard let aid = nr.authorID, !aid.isEmpty else { continue } + if let name = nr.name, !name.isEmpty { + namesMap[aid] = name } - let nameReq = NSFetchRequest<PlayerEntity>(entityName: "PlayerEntity") - nameReq.predicate = NSPredicate(format: "game == %@", entity) - let nameEntities = (try? ctx.fetch(nameReq)) ?? [] - var namesMap: [String: String] = [:] - var selections: [RawSelection] = [] - for nr in nameEntities { - guard let aid = nr.authorID, !aid.isEmpty else { continue } - if let name = nr.name, !name.isEmpty { - namesMap[aid] = name - } - if aid == localAuthorID { continue } - if let row = nr.selRow, - let col = nr.selCol, - let dir = nr.selDir, - let direction = Puzzle.Direction(rawValue: dir.intValue), - let updatedAt = nr.updatedAt { - selections.append(RawSelection( - authorID: aid, - row: row.intValue, - col: col.intValue, - direction: direction, - updatedAt: updatedAt - )) - } + if aid == localAuthorID { continue } + if let readAt = nr.readAt { + readAtByAuthor[aid] = readAt + } + if let row = nr.selRow, + let col = nr.selCol, + let dir = nr.selDir, + let direction = Puzzle.Direction(rawValue: dir.intValue), + let updatedAt = nr.updatedAt { + selections.append(RawSelection( + authorID: aid, + row: row.intValue, + col: col.intValue, + direction: direction, + updatedAt: updatedAt + )) } - let movesReq = NSFetchRequest<MovesEntity>(entityName: "MovesEntity") - movesReq.predicate = NSPredicate(format: "game == %@", entity) - let movesEntities = (try? ctx.fetch(movesReq)) ?? [] - let authorIDs = Array( - Set(movesEntities.compactMap { $0.authorID }) - .subtracting([localAuthorID, CKCurrentUserDefaultName, ""]) - ) - return ( - entity.databaseScope, - entity.ckShareRecordName, - entity.ckZoneName, - entity.ckZoneOwnerName, - namesMap, - authorIDs, - selections - ) } + let movesReq = NSFetchRequest<MovesEntity>(entityName: "MovesEntity") + movesReq.predicate = NSPredicate(format: "game == %@", entity) + let movesEntities = (try? ctx.fetch(movesReq)) ?? [] + let authorIDs = Array( + Set(movesEntities.compactMap { $0.authorID }) + .subtracting([localAuthorID, CKCurrentUserDefaultName, ""]) + ) + return FetchedRoster( + databaseScope: entity.databaseScope, + ckShareRecordName: entity.ckShareRecordName, + ckZoneName: entity.ckZoneName, + ckZoneOwnerName: entity.ckZoneOwnerName, + namesMap: namesMap, + moveAuthorIDs: authorIDs, + rawSelections: selections, + readAtByAuthor: readAtByAuthor + ) + } - applyRoster(localAuthorID: localAuthorID, namesMap: namesMap, moveAuthorIDs: moveAuthorIDs, rawSelections: rawSelections, share: nil) + applyRoster(localAuthorID: localAuthorID, fetched: fetched, share: nil) // Fetch the CKShare if not already cached. This can be noticeably // slower on device, so publish the local Core Data roster first and // then refine names/participants if share metadata arrives. let share = await fetchShare( - databaseScope: databaseScope, - ckShareRecordName: ckShareRecordName, - ckZoneName: ckZoneName, - ckZoneOwnerName: ckZoneOwnerName + databaseScope: fetched.databaseScope, + ckShareRecordName: fetched.ckShareRecordName, + ckZoneName: fetched.ckZoneName, + ckZoneOwnerName: fetched.ckZoneOwnerName ) - applyRoster(localAuthorID: localAuthorID, namesMap: namesMap, moveAuthorIDs: moveAuthorIDs, rawSelections: rawSelections, share: share) + applyRoster(localAuthorID: localAuthorID, fetched: fetched, share: share) + scheduleLeaseExpiryRecompute() // Trace only the post-fetch roster. The interim publish above always // has `share: nil` and would otherwise emit a second signature on // every refresh, defeating the dedup and producing the // `share=[]` ↔ `share=[…]` flicker seen in the logs. traceRoster( localAuthorID: localAuthorID, - namesMap: namesMap, - moveAuthorIDs: moveAuthorIDs, + namesMap: fetched.namesMap, + moveAuthorIDs: fetched.moveAuthorIDs, share: share ) } private func applyRoster( localAuthorID: String, - namesMap: [String: String], - moveAuthorIDs: [String], - rawSelections: [RawSelection], + fetched: FetchedRoster, share: CKShare? ) { // Collect all remote participant authorIDs. var otherAuthorIDs = Set<String>() - for key in namesMap.keys + for key in fetched.namesMap.keys where key != localAuthorID && key != CKCurrentUserDefaultName && !key.isEmpty { @@ -252,7 +275,7 @@ final class PlayerRoster { otherAuthorIDs.insert(recordName) } } - for authorID in moveAuthorIDs where authorID != CKCurrentUserDefaultName { + for authorID in fetched.moveAuthorIDs where authorID != CKCurrentUserDefaultName { otherAuthorIDs.insert(authorID) } @@ -267,7 +290,7 @@ final class PlayerRoster { var remoteEntries: [Entry] = [] var taken: Set<String> = [preferences.color.id] for authorID in otherAuthorIDs.sorted() { - let name = resolveName(authorID: authorID, namesMap: namesMap) + let name = resolveName(authorID: authorID, namesMap: fetched.namesMap) let color = PlayerColor.stableColor(forAuthorID: authorID, reserved: taken) taken.insert(color.id) remoteEntries.append(Entry(authorID: authorID, name: name, color: color, isLocal: false)) @@ -288,18 +311,16 @@ final class PlayerRoster { } // Map raw cursor tracks to the resolved colour from the entry list, - // dropping anything stale or with no matching entry. + // dropping anything with no matching entry. Visibility (the presence + // gate) is applied lazily in `remoteSelections`, so the last-known + // track is retained here regardless of age. let colorByAuthor = Dictionary( uniqueKeysWithValues: remoteEntries.map { ($0.authorID, $0.color) } ) - let now = Date() - var fresh: [String: RemoteSelection] = [:] - for raw in rawSelections { + var tracks: [String: RemoteSelection] = [:] + for raw in fetched.rawSelections { guard let color = colorByAuthor[raw.authorID] else { continue } - guard now.timeIntervalSince(raw.updatedAt) < selectionFreshnessWindow else { - continue - } - fresh[raw.authorID] = RemoteSelection( + tracks[raw.authorID] = RemoteSelection( authorID: raw.authorID, row: raw.row, col: raw.col, @@ -308,7 +329,26 @@ final class PlayerRoster { updatedAt: raw.updatedAt ) } - persistedRemoteSelections = fresh + persistedRemoteSelections = tracks + remoteReadAt = fetched.readAtByAuthor + } + + /// Schedules a single recompute at the soonest future peer lease expiry. + /// When it fires, `refresh()` re-reads `readAt` (reassigning `remoteReadAt`, + /// which triggers observation so the cursor and engagement icon re-evaluate) + /// and reschedules for the next-soonest lease. No-op when no peer holds a + /// live lease. + private func scheduleLeaseExpiryRecompute() { + leaseExpiryTask?.cancel() + leaseExpiryTask = nil + let now = Date() + guard let soonest = remoteReadAt.values.filter({ $0 > now }).min() else { return } + let interval = soonest.timeIntervalSince(now) + leaseExpiryTask = Task { [weak self] in + try? await Task.sleep(for: .seconds(interval)) + guard !Task.isCancelled, let self else { return } + await self.refresh() + } } // MARK: - Private helpers diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift @@ -150,6 +150,7 @@ final class AppServices { private var latestLocalSelections: [UUID: PlayerSelection] = [:] private var scheduledEngagementEndTasks: [UUID: Task<Void, Never>] = [:] private var engagementReconnectTasks: [UUID: Task<Void, Never>] = [:] + private var engagementLeaseExpiryTasks: [UUID: Task<Void, Never>] = [:] init() { let preferences = PlayerPreferences() @@ -967,16 +968,12 @@ final class AppServices { /// which mints and connects without waiting to see a present peer. func reconcileEngagement(gameID: UUID, force: Bool = false) async { guard preferences.isICloudSyncEnabled else { return } - let hasPeer: Bool - if force { - hasPeer = true - } else { - hasPeer = await Self.hasPresentPeer( - persistence: persistence, - gameID: gameID, - localAuthorID: identity.currentID - ) - } + let soonestLease = await Self.soonestPeerLease( + persistence: persistence, + gameID: gameID, + localAuthorID: identity.currentID + ) + let hasPeer = force || soonestLease != nil var creds = EngagementRoomCredentials.decode(store.engagement(for: gameID)) if creds == nil, hasPeer { if let minted = try? EngagementRoomCredentials.fresh(), @@ -989,6 +986,12 @@ final class AppServices { } } await engagementCoordinator.reconcile(gameID: gameID, creds: creds, hasPeer: hasPeer) + // When the soonest present peer's lease lapses, re-reconcile: tear down + // (dropping the bolt) if no renewal arrived, or reschedule onto the new + // horizon if one did. This makes teardown track lease expiry precisely; + // the 30s reconnect tick remains the coarse backstop. A force/no-peer + // reconcile has no lease to watch, so this just clears any prior wake. + scheduleEngagementLeaseExpiry(gameID: gameID, at: soonestLease) } func offerEngagement(gameID: UUID) async { @@ -1020,6 +1023,7 @@ final class AppServices { func endEngagement(gameID: UUID) async { cancelScheduledEngagementEnd(gameID: gameID) cancelEngagementReconnectRetry(gameID: gameID) + cancelEngagementLeaseExpiry(gameID: gameID) syncMonitor.note("engagement: ending for \(gameID.uuidString)") engagementStatus.setLive(false, gameID: gameID) latestLocalSelections[gameID] = nil @@ -1034,6 +1038,7 @@ final class AppServices { // opened but never connected would tick forever. A quick return // re-arms it via `startEngagementIfPossible`. cancelEngagementReconnectRetry(gameID: gameID) + cancelEngagementLeaseExpiry(gameID: gameID) guard engagementStatus.isLive(gameID: gameID) else { return } syncMonitor.note( "engagement: scheduled ending for \(gameID.uuidString) " + @@ -1097,6 +1102,28 @@ final class AppServices { syncMonitor.note("engagement: reconnect backstop cancelled for \(gameID.uuidString)") } + /// Arms a one-shot reconcile at `expiry` — the soonest moment a present + /// peer's lease lapses — replacing any prior wake for this game. When it + /// fires, `reconcileEngagement` tears the channel down if no renewal landed + /// (so the bolt drops at expiry, not up to a tick later) or reschedules + /// onto the renewed horizon. `nil` (force/no-peer) just clears the wake. + private func scheduleEngagementLeaseExpiry(gameID: UUID, at expiry: Date?) { + engagementLeaseExpiryTasks[gameID]?.cancel() + engagementLeaseExpiryTasks[gameID] = nil + guard let expiry else { return } + let interval = max(0, expiry.timeIntervalSinceNow) + engagementLeaseExpiryTasks[gameID] = Task { [weak self] in + try? await Task.sleep(for: .seconds(interval)) + guard !Task.isCancelled, let self else { return } + await self.reconcileEngagement(gameID: gameID) + } + } + + private func cancelEngagementLeaseExpiry(gameID: UUID) { + guard let task = engagementLeaseExpiryTasks.removeValue(forKey: gameID) else { return } + task.cancel() + } + func noteLocalSelection(_ selection: PlayerSelection, gameID: UUID) async { latestLocalSelections[gameID] = selection guard engagementStatus.isLive(gameID: gameID), @@ -2445,18 +2472,53 @@ final class AppServices { gameID as CVarArg, Date() as NSDate ) + let now = Date() let players = (try? context.fetch(req)) ?? [] let hasPeer = players.contains { player in guard let authorID = player.authorID, !authorID.isEmpty else { return false } if authorID == CKCurrentUserDefaultName { return false } if let localAuthorID, !localAuthorID.isEmpty, authorID == localAuthorID { return false } - return true + return PeerPresence.isPresent(readAt: player.readAt, asOf: now) } continuation.resume(returning: hasPeer) } } } + /// The earliest future `readAt` among non-local participants in `gameID` — + /// i.e. when the soonest peer lease lapses — or `nil` if no peer is + /// present. `nil` is the same condition `hasPresentPeer` reports as absent, + /// so callers can derive presence from this and also schedule against the + /// expiry instant. + static func soonestPeerLease( + persistence: PersistenceController, + gameID: UUID, + localAuthorID: String? + ) async -> Date? { + let context = persistence.container.newBackgroundContext() + return await withCheckedContinuation { continuation in + context.perform { + let req = NSFetchRequest<PlayerEntity>(entityName: "PlayerEntity") + req.predicate = NSPredicate( + format: "game.id == %@ AND readAt > %@", + gameID as CVarArg, + Date() as NSDate + ) + let now = Date() + var soonest: Date? + for player in (try? context.fetch(req)) ?? [] { + guard let authorID = player.authorID, !authorID.isEmpty else { continue } + if authorID == CKCurrentUserDefaultName { continue } + if let localAuthorID, !localAuthorID.isEmpty, authorID == localAuthorID { continue } + guard let readAt = player.readAt, + PeerPresence.isPresent(readAt: readAt, asOf: now) else { continue } + if soonest == nil || readAt < soonest! { soonest = readAt } + } + continuation.resume(returning: soonest) + } + } + } + static func presentPeers( persistence: PersistenceController, gameIDs: Set<UUID>?, @@ -2477,12 +2539,14 @@ final class AppServices { } req.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: predicates) + let now = Date() var result: [UUID: Set<String>] = [:] for player in (try? context.fetch(req)) ?? [] { guard let gameID = player.game?.id, let authorID = player.authorID, !authorID.isEmpty, - authorID != CKCurrentUserDefaultName else { continue } + authorID != CKCurrentUserDefaultName, + PeerPresence.isPresent(readAt: player.readAt, asOf: now) else { continue } result[gameID, default: []].insert(authorID) } continuation.resume(returning: result.mapValues { Array($0) }) diff --git a/Crossmate/Sync/Presence.swift b/Crossmate/Sync/Presence.swift @@ -1,6 +1,17 @@ import CloudKit import Foundation +/// The single rule for "is a peer present." A peer is present iff their +/// active-session lease (`Player.readAt`) is still in the future — the cursor +/// (`PlayerRoster`), the engagement icon, engagement teardown, and the +/// selection-publisher send gate all derive presence from this. +enum PeerPresence { + static func isPresent(readAt: Date?, asOf now: Date = Date()) -> Bool { + guard let readAt else { return false } + return readAt > now + } +} + /// What a Ping record represents. Stored as a string in the CKRecord's /// `kind` field. Pings now cover durable bootstrap/side-channel events that /// do not need live APN timing. User-facing play events ride on the push diff --git a/Tests/Unit/PlayerRosterTests.swift b/Tests/Unit/PlayerRosterTests.swift @@ -62,7 +62,8 @@ struct PlayerRosterTests { gameID: UUID, persistence: PersistenceController, selection: PlayerSelection? = nil, - updatedAt: Date = Date() + updatedAt: Date = Date(), + readAt: Date? = Date().addingTimeInterval(600) ) { let ctx = persistence.viewContext let req = NSFetchRequest<GameEntity>(entityName: "GameEntity") @@ -74,6 +75,7 @@ struct PlayerRosterTests { player.name = name player.ckRecordName = RecordSerializer.recordName(forPlayerInGame: gameID, authorID: authorID) player.updatedAt = updatedAt + player.readAt = readAt if let selection { player.selRow = NSNumber(value: selection.row) player.selCol = NSNumber(value: selection.col) @@ -233,4 +235,46 @@ struct PlayerRosterTests { #expect(fallback?.col == 2) #expect(fallback?.direction == .across) } + + @Test("Cursor is hidden when the peer's lease has expired") + func cursorHiddenWhenLeaseExpired() async throws { + let (persistence, gameID) = try makePersistenceWithGame() + addMoves(authorIDs: ["_B"], gameID: gameID, persistence: persistence) + addPlayerEntity( + authorID: "_B", + name: "Alice", + gameID: gameID, + persistence: persistence, + selection: PlayerSelection(row: 1, col: 2, direction: .across), + updatedAt: Date(), + readAt: Date().addingTimeInterval(-60) + ) + + let roster = makeRoster(gameID: gameID, persistence: persistence) + await roster.refresh() + + #expect(roster.remoteSelections["_B"] == nil, "an expired lease should hide the peer cursor") + } + + @Test("Cursor stays visible for a present peer with a stale selection") + func cursorVisibleWhilePresentDespiteStaleSelection() async throws { + let (persistence, gameID) = try makePersistenceWithGame() + addMoves(authorIDs: ["_B"], gameID: gameID, persistence: persistence) + addPlayerEntity( + authorID: "_B", + name: "Alice", + gameID: gameID, + persistence: persistence, + selection: PlayerSelection(row: 1, col: 2, direction: .across), + updatedAt: Date().addingTimeInterval(-300), + readAt: Date().addingTimeInterval(600) + ) + + let roster = makeRoster(gameID: gameID, persistence: persistence) + await roster.refresh() + + let selection = roster.remoteSelections["_B"] + #expect(selection?.row == 1, "a present peer keeps their cursor even when the selection is old") + #expect(selection?.col == 2) + } }