commit 037807d3be7fdfd8f063053745eaa868a5227fe1
parent 36b2e5296a3dc8cfeb3c6ab201d48f5b1d0dcced
Author: Michael Camilleri <[email protected]>
Date: Mon, 18 May 2026 08:03:41 +0900
Fix ping bugs
Production check/reveal pings were BadSyntax-failing on a 'scope' field never
deployed to the production schema, while the generic 'payload' slot (already in
production) does the same job — 'scope' was just an older, narrower field that
predates it and is set for no other kind. pingRecord now folds a check/reveal PingScope into payload as {"scope":"…"}
via a new PingScopePayload codec; the top-level Ping.scope field is no longer
written or read, and is dropped from ping desiredKeys.
Additionally, Phase 2 scene-phase wiring fired an .opened/.closed ping on every
transition including the transient .inactive, while the poll loop kept
refreshing the lease for a backgrounded puzzle whose view hadn't disappeared,
so a single lock/unlock thrashed the lease open/closed several times over. The
scene-phase handler now acts only on settled transitions (.background → closed,
.active → opened, .inactive → no-op), and the poll-loop lease refresh is gated
on NotificationState.activePuzzleID() == gameID, so a backgrounded puzzle stops
claiming the lease.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Diffstat:
5 files changed, 129 insertions(+), 23 deletions(-)
diff --git a/Crossmate/CrossmateApp.swift b/Crossmate/CrossmateApp.swift
@@ -424,16 +424,21 @@ private struct PuzzleDisplayView: View {
}
.onChange(of: scenePhase) { _, newPhase in
updateActiveNotificationPuzzleID(for: newPhase)
- // Releasing/refreshing the cross-device lease mirrors the local
- // active state: backgrounding ends it promptly so siblings stop
- // suppressing; returning re-opens it without waiting for the poll.
+ // Only act on settled transitions. `.inactive` is transient (lock
+ // animation, app switcher, Control Center, banners) — sending a
+ // ping on it thrashed the lease open/closed many times per
+ // lock/unlock. `.background` ends it promptly so siblings stop
+ // suppressing; `.active` re-opens it without waiting for the poll.
let id = gameID
- Task {
- if newPhase == .active {
- await services.refreshActiveLease(for: id)
- } else {
- await services.closeActiveLease(for: id)
- }
+ switch newPhase {
+ case .active:
+ Task { await services.refreshActiveLease(for: id) }
+ case .background:
+ Task { await services.closeActiveLease(for: id) }
+ case .inactive:
+ break
+ @unknown default:
+ break
}
}
.onDisappear {
@@ -556,7 +561,14 @@ private struct PuzzleDisplayView: View {
}
guard !Task.isCancelled else { break }
guard let scope = syncedScope else { break }
- if Date().timeIntervalSince(lastLeaseSentAt)
+ // Only refresh the lease while this puzzle is genuinely the
+ // foreground-active one. The poll loop isn't cancelled when the
+ // app backgrounds (the view stays alive), so without this gate a
+ // backgrounded puzzle kept re-broadcasting "I'm viewing" every
+ // interval. `.background` already sent `.closed`; resume on
+ // return to `.active`.
+ if NotificationState.activePuzzleID() == gameID,
+ Date().timeIntervalSince(lastLeaseSentAt)
>= NotificationState.leaseRefreshInterval {
await services.refreshActiveLease(for: gameID)
lastLeaseSentAt = Date()
diff --git a/Crossmate/Sync/RecordSerializer.swift b/Crossmate/Sync/RecordSerializer.swift
@@ -176,7 +176,10 @@ enum RecordSerializer {
/// between a single user's own devices, where authorID is identical.
/// - `playerName` and `puzzleTitle` let receivers render the alert body.
/// - `kind` distinguishes join/win/check/reveal/opened events.
- /// - `scope` is set only for check/reveal kinds.
+ /// - `scope` (check/reveal granularity) is folded into `payload` as
+ /// `{"scope":"…"}` — the legacy top-level `scope` field is no longer
+ /// written (see PingScope). `scope`/`payload` are mutually exclusive
+ /// across kinds, so there is never anything to merge.
static func pingRecord(
gameID: UUID,
authorID: String,
@@ -202,11 +205,13 @@ enum RecordSerializer {
record["playerName"] = playerName as CKRecordValue
record["puzzleTitle"] = puzzleTitle as CKRecordValue
record["kind"] = kind.rawValue as CKRecordValue
- if let scope {
- record["scope"] = scope.rawValue as CKRecordValue
- }
- if let payload {
- record["payload"] = payload as CKRecordValue
+ // `scope` and `payload` never co-occur across kinds; fold a legacy
+ // `scope` into the generic payload rather than writing the now-frozen
+ // top-level field.
+ let outPayload = payload
+ ?? scope.flatMap { PingScopePayload(scope: $0.rawValue).encoded() }
+ if let outPayload {
+ record["payload"] = outPayload as CKRecordValue
}
return record
}
diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift
@@ -46,8 +46,19 @@ enum PingKind: String, Sendable {
case invite
}
-/// Granularity of a check/reveal action. Stored as a string in the CKRecord's
-/// `scope` field; nil for kinds where it doesn't apply.
+/// Granularity of a check/reveal action; nil for kinds where it doesn't apply.
+///
+/// LEGACY SCHEMA NOTE: this used to be its own `Ping.scope` CKRecord field,
+/// predating the generic `payload` slot. It now rides in `payload` as
+/// `{"scope":"…"}` like every other kind-specific datum. The old `Ping.scope`
+/// CKRecord field is DEAD: no code writes or reads it. It still exists in the
+/// deployed production CloudKit schema only because CloudKit cannot delete
+/// fields from a deployed type.
+///
+/// ┌─────────────────────────────────────────────────────────────────┐
+/// │ When the CloudKit container/schema is next rebuilt clean, do NOT │
+/// │ recreate the `Ping.scope` field. It is intentionally gone. │
+/// └─────────────────────────────────────────────────────────────────┘
enum PingScope: String, Sendable {
case square
case word
@@ -64,7 +75,8 @@ struct Ping: Sendable {
let kind: PingKind
let scope: PingScope?
/// Kind-specific JSON. `.friend`: `{friendShareURL,pairKey,ownerAuthorID}`;
- /// `.invite`: `{gameShareURL}`. nil for join/win/check/reveal/opened.
+ /// `.invite`: `{gameShareURL}`; `.check`/`.reveal`: `{scope}` (see
+ /// PingScope); `.opened`/`.closed`: the OpenedLease. nil for join/win.
let payload: String?
}
@@ -97,6 +109,23 @@ struct OpenedLease: Codable, Sendable {
}
}
+/// `payload` JSON for `.check`/`.reveal` pings — the check/reveal granularity
+/// that used to live in the legacy `Ping.scope` field (see PingScope). The
+/// `scope` key is required, so decoding a different kind's payload (lease,
+/// friend, invite) yields nil rather than a false match.
+struct PingScopePayload: Codable, Sendable {
+ let scope: String
+
+ func encoded() -> String? {
+ (try? JSONEncoder().encode(self)).flatMap { String(data: $0, encoding: .utf8) }
+ }
+
+ static func decode(_ string: String?) -> PingScopePayload? {
+ guard let data = string?.data(using: .utf8) else { return nil }
+ return try? JSONDecoder().decode(PingScopePayload.self, from: data)
+ }
+}
+
/// Owns the CloudKit sync lifecycle via two `CKSyncEngine` instances — one for
/// the private database (owned games and shares) and one for the shared
/// database (joined games). Zone creation, subscription setup, change-token
@@ -907,7 +936,7 @@ actor SyncEngine {
database: database,
zoneID: zoneID,
since: since,
- desiredKeys: ["authorID", "deviceID", "playerName", "puzzleTitle", "kind", "scope", "payload"]
+ desiredKeys: ["authorID", "deviceID", "playerName", "puzzleTitle", "kind", "payload"]
)
return PerZonePings(records: records, orphanedZone: nil)
} catch {
@@ -1028,7 +1057,7 @@ actor SyncEngine {
database: database,
zoneID: zone.zoneID,
since: since,
- desiredKeys: ["authorID", "deviceID", "playerName", "puzzleTitle", "kind", "scope", "payload"]
+ desiredKeys: ["authorID", "deviceID", "playerName", "puzzleTitle", "kind", "payload"]
)
async let playerRecords = self.queryLiveRecords(
type: "Player",
@@ -2499,7 +2528,11 @@ actor SyncEngine {
let kindRaw = record["kind"] as? String,
let kind = PingKind(rawValue: kindRaw)
else { return nil }
- let scope: PingScope? = (record["scope"] as? String).flatMap(PingScope.init(rawValue:))
+ // Scope rides in `payload` as `{"scope":"…"}`. The old top-level
+ // `scope` field is dead — nothing reads or writes it (see PingScope).
+ let payloadString = record["payload"] as? String
+ let scope: PingScope? =
+ PingScopePayload.decode(payloadString).flatMap { PingScope(rawValue: $0.scope) }
// Legacy records written before the schema added `deviceID` won't have
// the field. Parse-tolerant: empty string can never equal a real
// localDeviceID, so the self-send filter stays safe.
@@ -2513,7 +2546,7 @@ actor SyncEngine {
puzzleTitle: (record["puzzleTitle"] as? String) ?? "",
kind: kind,
scope: scope,
- payload: record["payload"] as? String
+ payload: payloadString
)
}
diff --git a/Tests/Unit/OpenedLeaseTests.swift b/Tests/Unit/OpenedLeaseTests.swift
@@ -30,3 +30,36 @@ struct OpenedLeaseTests {
#expect(OpenedLease.decode("{\"leaseMs\":1}") == nil)
}
}
+
+@Suite("Ping scope payload")
+struct PingScopePayloadTests {
+ @Test("Round-trips through its JSON payload")
+ func roundTrip() throws {
+ for scope in [PingScope.square, .word, .puzzle] {
+ let encoded = try #require(
+ PingScopePayload(scope: scope.rawValue).encoded()
+ )
+ let decoded = try #require(PingScopePayload.decode(encoded))
+ #expect(PingScope(rawValue: decoded.scope) == scope)
+ }
+ }
+
+ @Test("decode is nil for missing or malformed payload")
+ func decodeNilOnGarbage() {
+ #expect(PingScopePayload.decode(nil) == nil)
+ #expect(PingScopePayload.decode("") == nil)
+ #expect(PingScopePayload.decode("not json") == nil)
+ }
+
+ @Test("Other kinds' payloads don't false-match as a scope payload")
+ func crossKindIsolation() {
+ // A lease payload has no `scope` key.
+ let lease = OpenedLease(leaseMs: 300_000, sentAtMs: 1).encoded()
+ #expect(PingScopePayload.decode(lease) == nil)
+ // …and a scope payload isn't a valid lease.
+ let scope = PingScopePayload(scope: "word").encoded()
+ #expect(OpenedLease.decode(scope) == nil)
+ // An invite-shaped payload also has no `scope` key.
+ #expect(PingScopePayload.decode(#"{"gameShareURL":"https://x"}"#) == nil)
+ }
+}
diff --git a/Tests/Unit/RecordSerializerTests.swift b/Tests/Unit/RecordSerializerTests.swift
@@ -134,6 +134,29 @@ struct RecordSerializerTests {
#expect(withoutPayload["payload"] == nil)
}
+ @Test("check/reveal scope is folded into payload, not the legacy field")
+ func pingRecordFoldsScopeIntoPayload() throws {
+ let zone = CKRecordZone.ID(zoneName: "z", ownerName: CKCurrentUserDefaultName)
+ let record = RecordSerializer.pingRecord(
+ gameID: UUID(),
+ authorID: "alice",
+ deviceID: "deviceA",
+ playerName: "Alice",
+ puzzleTitle: "Puzzle",
+ eventTimestampMs: 1700000000000,
+ kind: .check,
+ scope: .word,
+ zone: zone
+ )
+ // The frozen top-level field is no longer written.
+ #expect(record["scope"] == nil)
+ let decoded = try #require(
+ PingScopePayload.decode(record["payload"] as? String)
+ )
+ #expect(decoded.scope == "word")
+ #expect(PingScope(rawValue: decoded.scope) == .word)
+ }
+
@Test("accountZoneID is named 'account' in the current user's private DB")
func accountZoneIDShape() {
let zone = RecordSerializer.accountZoneID
