crossmate

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

commit 40592e1aca08b08191ed151b330300133d68271f
parent d110bbfeae5cf00a8938170d10bac1da2c1b663d
Author: Michael Camilleri <[email protected]>
Date:   Fri,  5 Jun 2026 20:18:08 +0900

Make account decision payloads durable and adopt on conflict

The account push secret (and the push address before it) is published as
a write-once Decision whose payload lived only in the in-memory
pendingDecisionPayloads map, while CKSyncEngine persists the pending
.saveRecord across launches. If the app died between enqueue and a
completed send, the relaunched engine still held the pending change but
the payload was gone, so buildRecord emitted a payload-less Decision
(e.g. an empty pushSecret) and uploaded it. Other devices parse that
record, find no payload, and never learn the secret; the minting device
sees its UserDefaults value and never republishes. Each device then
mints its own secret and derives divergent per-game push addresses — the
exact divergence the derivation scheme exists to remove, and one that
does not self-heal.

Make the payload as durable as the pending change it belongs to: mirror
pendingDecisionPayloads to UserDefaults on every mutation and restore it
in start(), alongside CKSyncEngine reloading its serialized pending
changes. buildRecord now rebuilds the Decision with its payload after a
relaunch instead of as a poison record. resetSyncState clears the
durable copy with the rest of the wiped state. This covers both
payload-carrying decisions — the push secret and the pre-existing push
address.

Separately, the send-conflict settlement path adopted the winner's push
address straight off the serverRecordChanged error but ignored the
secret, leaving a losing device on its own divergent secret until a
later fetch happened to deliver the winner. Parse the secret off the
same server record and apply it via onAccountPushSecret, which caches it
and re-reconciles the derived addresses — closing the divergence window
the same way the address path already does.

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

Diffstat:
MCrossmate/Sync/SyncEngine.swift | 81++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 70 insertions(+), 11 deletions(-)

diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift @@ -59,8 +59,18 @@ actor SyncEngine { /// backing — they're write-once-and-forget — so we stash the minimal data /// here keyed by record name and look it up in `buildRecord`. private var pendingPings: [String: PingPayload] = [:] + /// Payloads for `Decision` records pending send, keyed by record name. + /// Unlike a ping body, these must survive an app kill: CKSyncEngine persists + /// the pending `.saveRecord`, so on relaunch `buildRecord` would otherwise + /// emit a payload-less Decision (e.g. an empty `pushSecret`) that uploads as + /// a poison record other devices can't parse and this device never + /// republishes. Mirrored to UserDefaults on every mutation and restored in + /// `start()`. private var pendingDecisionPayloads: [String: String] = [:] + private static let pendingDecisionPayloadsDefaultsKey = + "SyncEngine.pendingDecisionPayloads" + struct PingPayload { let gameID: UUID let authorID: String @@ -232,6 +242,11 @@ actor SyncEngine { func start() async { guard privateEngine == nil, sharedEngine == nil else { return } + // CKSyncEngine restores its pending `.saveRecord` changes from the + // serialized state below; restore the matching Decision payloads so a + // pending decision rebuilds with its body instead of as a poison record. + restorePendingDecisionPayloads() + let bgCtx = persistence.container.newBackgroundContext() let privateData: Data? = bgCtx.performAndWait { @@ -806,11 +821,11 @@ actor SyncEngine { } /// Registers a durable `Decision` record into the account zone so the fact - /// reaches the user's own other devices. Unlike Pings there is no in-memory - /// payload stash: the record is fully reconstructable from its name, so it - /// survives an app kill (CKSyncEngine persists the pending change and - /// `buildRecord` rebuilds it deterministically). Idempotent — a re-send of - /// an existing decision is a benign conflict the send path drops. + /// reaches the user's own other devices. Decisions without payloads are + /// reconstructable from their names; payload-bearing decisions mirror their + /// body to UserDefaults so CKSyncEngine's persisted pending save survives an + /// app kill without rebuilding as a payload-less record. Idempotent — a + /// re-send of an existing decision is a benign conflict the send path drops. func enqueueDecision(kind: String, key: String, payload: String? = nil) { guard let engine = privateEngine else { return } let zoneID = RecordSerializer.accountZoneID @@ -820,6 +835,7 @@ actor SyncEngine { let name = RecordSerializer.decisionRecordName(kind: kind, key: key) if let payload { pendingDecisionPayloads[name] = payload + persistPendingDecisionPayloads() } let recordID = CKRecord.ID(recordName: name, zoneID: zoneID) engine.state.add(pendingRecordZoneChanges: [.saveRecord(recordID)]) @@ -838,6 +854,28 @@ actor SyncEngine { sendChangesDetached(on: engine) } + /// Mirrors `pendingDecisionPayloads` to durable storage. CKSyncEngine + /// persists the pending `.saveRecord` on its own, so the payload must be + /// equally durable or a relaunch sends a payload-less Decision. + private func persistPendingDecisionPayloads() { + if pendingDecisionPayloads.isEmpty { + UserDefaults.standard.removeObject( + forKey: Self.pendingDecisionPayloadsDefaultsKey + ) + } else { + UserDefaults.standard.set( + pendingDecisionPayloads, + forKey: Self.pendingDecisionPayloadsDefaultsKey + ) + } + } + + private func restorePendingDecisionPayloads() { + pendingDecisionPayloads = (UserDefaults.standard.dictionary( + forKey: Self.pendingDecisionPayloadsDefaultsKey + ) as? [String: String]) ?? [:] + } + /// 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. @@ -1039,6 +1077,7 @@ actor SyncEngine { )) pendingPings = [:] pendingDecisionPayloads = [:] + persistPendingDecisionPayloads() pingPushCheckpoints = [:] seenPingRecords = [:] liveQueryCheckpoints = [:] @@ -1462,17 +1501,21 @@ actor SyncEngine { pendingPings.removeValue(forKey: name) } else if name.hasPrefix("decision-") { pendingDecisionPayloads.removeValue(forKey: name) + persistPendingDecisionPayloads() } } let ctx = persistence.container.newBackgroundContext() ctx.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump - let (failureMessages, orphanedZones, resolvedDecisions, settledJournals, resolvedAccountAddresses): - ([String], Set<CKRecordZone.ID>, Set<CKRecord.ID>, Set<CKRecord.ID>, [String]) = ctx.performAndWait { + let (failureMessages, orphanedZones, resolvedDecisions, settledJournals, + resolvedAccountAddresses, resolvedAccountSecrets): + ([String], Set<CKRecordZone.ID>, Set<CKRecord.ID>, Set<CKRecord.ID>, + [String], [String]) = ctx.performAndWait { var messages: [String] = [] var orphaned = Set<CKRecordZone.ID>() var settledDecisions = Set<CKRecord.ID>() var settledJournals = Set<CKRecord.ID>() var accountAddresses: [String] = [] + var accountSecrets: [String] = [] for record in event.savedRecords { self.writeBackSystemFields(record: record, in: ctx) let savedName = record.recordID.recordName @@ -1508,9 +1551,18 @@ actor SyncEngine { // so this is success — drop the pending change so the // immutable record doesn't retry-loop forever. settledDecisions.insert(failure.record.recordID) - if let serverRecord = err.userInfo[CKRecordChangedErrorServerRecordKey] as? CKRecord, - let address = RecordSerializer.parseAccountPushAddressDecision(serverRecord) { - accountAddresses.append(address) + // We lost the write-once race; adopt the winner's payload + // straight off the conflict error instead of waiting for a + // later fetch to deliver it. For the push secret that closes + // the window in which this device would keep deriving + // divergent per-game addresses from its own minted secret. + if let serverRecord = err.userInfo[CKRecordChangedErrorServerRecordKey] as? CKRecord { + if let address = RecordSerializer.parseAccountPushAddressDecision(serverRecord) { + accountAddresses.append(address) + } + if let secret = RecordSerializer.parseAccountPushSecretDecision(serverRecord) { + accountSecrets.append(secret) + } } settled = true messages.append("send: decision \(name) already present — settled") @@ -1555,7 +1607,8 @@ actor SyncEngine { ) } } - return (messages, orphaned, settledDecisions, settledJournals, accountAddresses) + return (messages, orphaned, settledDecisions, settledJournals, + accountAddresses, accountSecrets) } if !orphanedZones.isEmpty { await applyZoneOrphaning(orphanedZones, isPrivate: isPrivate) @@ -1567,12 +1620,18 @@ actor SyncEngine { for recordID in resolvedDecisions { pendingDecisionPayloads.removeValue(forKey: recordID.recordName) } + persistPendingDecisionPayloads() } if let onAccountPushAddress { for address in resolvedAccountAddresses { await onAccountPushAddress(address) } } + if let onAccountPushSecret { + for secret in resolvedAccountSecrets { + await onAccountPushSecret(secret) + } + } if !settledJournals.isEmpty { // Drop from whichever engine owns the zone (private for solo games, // shared for collaborations) — unlike decisions, journals ride both.