crossmate

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

commit fd641654d80bffe4a48f2cafe9cc554340113732
parent 722eb163cd45325b3f6fd69c215558c15a0356a1
Author: Michael Camilleri <[email protected]>
Date:   Sun, 17 May 2026 05:48:41 +0900

Improve friend-related features on multiple devices

Crossmate's Core Data store is a plain NSPersistentContainer with no CloudKit
mirroring. Two friend-feature paths quietly depended on that state being shared
and broke when a single iCloud user had more than one device. First, when a new
collaborator was first-sighted on two of the user's devices, both ran
establishIfOwner; the second device's saveZoneWideShare collided with the share
the first had already created, threw, and left that device with no FriendEntity
and no way to re-invite. Second, blocking a collaborator only set isBlocked in
local Core Data, so the block never reached the user's other devices and an
unblocked sibling could even re-bootstrap the torn-down friend zone.

To address the first issue, establishIfOwner is now idempotent. After ensuring
the zone it fetches the existing zone-wide share (existingZoneWideShare); if
one is present a sibling existing zone-wide share (existingZoneWideShare); if
one is present a sibling device already established the friendship, so it
records the friendship locally from the deterministic pair values and skips
both the share creation and the .friend Ping. The narrow create race between
the check and the save is caught as serverRecordChanged and resolved down the
same path.

To address the second issue, a block is made authoritative across the user's
own devices with a new durable Decision record. Decision is the persistent
counterpart to the transient Ping: the identity (kind + key) is encoded in the
record name (decision-<kind>-<key>) so every write is an idempotent upsert with
record-level conflict isolation, kind is the discriminator, and payload is the
generic kind-specific slot (empty for block, mirroring Ping.payload). It lives
in the private-DB account zone, rides the existing CKSyncEngine path that
already carries .opened Pings there, and survives an app kill because
CKSyncEngine persists the pending change and the record is rebuilt
deterministically from its name. As a result, the CloudKit schema is updated to
include a Decision record type (kind, payload, createdAt).

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

Diffstat:
MCrossmate/Sync/FriendController.swift | 75++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
MCrossmate/Sync/RecordSerializer.swift | 91+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
MCrossmate/Sync/SyncEngine.swift | 50++++++++++++++++++++++++++++++++++++++++++++++++--
MTests/Unit/RecordSerializerTests.swift | 128+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 333 insertions(+), 11 deletions(-)

diff --git a/Crossmate/Sync/FriendController.swift b/Crossmate/Sync/FriendController.swift @@ -68,18 +68,50 @@ final class FriendController { let zoneName = FriendZone.zoneName(pairKey: pairKey) let zoneID = CKRecordZone.ID(zoneName: zoneName, ownerName: CKCurrentUserDefaultName) + // Every field of the local friendship is deterministic or already + // in hand, so any owner-side device can record it without reading + // anything back from the share. + let recordLocalFriendship = { + self.persistFriend( + authorID: remoteAuthorID, + displayName: remoteDisplayName, + pairKey: pairKey, + zoneName: zoneName, + zoneOwnerName: CKCurrentUserDefaultName, + databaseScope: 0 + ) + } + try await createZone(zoneID) - let share = try await saveZoneWideShare(zoneID: zoneID, addingParticipant: remoteAuthorID) + + // A sibling device on this same iCloud account may have already + // created the zone-wide share. `FriendEntity` is local-only, so + // the `friendExists` check above can't see its work and we still + // reach here. If the share exists we adopt the friendship locally + // *without* re-creating the share or re-enqueuing the `.friend` + // Ping — the sibling already delivered it to the friend. + if try await existingZoneWideShare(zoneID: zoneID) != nil { + recordLocalFriendship() + syncMonitor?.recordSuccess("establish friendship") + return + } + + let share: CKShare + do { + share = try await saveZoneWideShare( + zoneID: zoneID, + addingParticipant: remoteAuthorID + ) + } catch let error as CKError where error.code == .serverRecordChanged { + // Lost the create race to a sibling between the check above + // and this save. The share now exists; adopt it as above. + recordLocalFriendship() + syncMonitor?.recordSuccess("establish friendship") + return + } guard let url = share.url else { throw FriendError.missingShareURL } - persistFriend( - authorID: remoteAuthorID, - displayName: remoteDisplayName, - pairKey: pairKey, - zoneName: zoneName, - zoneOwnerName: CKCurrentUserDefaultName, - databaseScope: 0 - ) + recordLocalFriendship() let payload = FriendZone.BootstrapPayload( friendShareURL: url.absoluteString, @@ -198,6 +230,12 @@ final class FriendController { friend.isBlocked = true try? ctx.save() + // Make the block authoritative across the user's own devices. Written + // before the channel teardown so the durable fact survives even a + // partial teardown; `applyDecisionRecord` projects it onto a blocked + // `FriendEntity` on every other device. + await syncEngine.enqueueDecision(kind: "block", key: friendAuthorID) + guard let zoneName, let ownerName else { return } let zoneID = CKRecordZone.ID(zoneName: zoneName, ownerName: ownerName) syncMonitor?.recordStart("block friend") @@ -270,6 +308,25 @@ final class FriendController { return savedShare } + /// The zone-wide `CKShare` for `zoneID` in our private database, or `nil` + /// if it doesn't exist yet. Lets an owner-side device detect a friend zone + /// a sibling device on the same iCloud account already shared. + private func existingZoneWideShare( + zoneID: CKRecordZone.ID + ) async throws -> CKShare? { + let shareID = CKRecord.ID( + recordName: CKRecordNameZoneWideShare, + zoneID: zoneID + ) + do { + let record = try await container.privateCloudDatabase.record(for: shareID) + return record as? CKShare + } catch let error as CKError + where error.code == .unknownItem || error.code == .zoneNotFound { + return nil + } + } + private func fetchParticipant( forUserRecordName recordName: String ) async throws -> CKShare.Participant { diff --git a/Crossmate/Sync/RecordSerializer.swift b/Crossmate/Sync/RecordSerializer.swift @@ -63,6 +63,29 @@ enum RecordSerializer { "ping-\(gameID.uuidString)-\(authorID)-\(deviceID)-\(eventTimestampMs)" } + /// One `Decision` record per `(kind, key)`. A durable, per-user fact that + /// must agree across a single iCloud user's own devices — the durable + /// counterpart to the transient `Ping`. Lives in the account zone; the + /// deterministic name makes every write an idempotent upsert. `kind` + /// carries no dashes (so the first dash after the prefix splits cleanly); + /// `key` may contain dashes. + static func decisionRecordName(kind: String, key: String) -> String { + "decision-\(kind)-\(key)" + } + + /// Parses `decision-<kind>-<key>`. `kind` is the segment up to the first + /// dash after the prefix; `key` is the remainder. + static func parseDecisionRecordName(_ name: String) -> (kind: String, key: String)? { + let prefix = "decision-" + guard name.hasPrefix(prefix) else { return nil } + let rest = name.dropFirst(prefix.count) + guard let dash = rest.firstIndex(of: "-") else { return nil } + let kind = String(rest[rest.startIndex..<dash]) + let key = String(rest[rest.index(after: dash)...]) + guard !kind.isEmpty, !key.isEmpty else { return nil } + return (kind, key) + } + // MARK: - Zone /// Zone ID for a per-game zone. `ownerName` defaults to the current user @@ -188,6 +211,30 @@ enum RecordSerializer { return record } + /// Builds a `Decision` record. The identity (`kind` + `key`) lives in the + /// record name, which keeps every write an idempotent upsert; `key` is not + /// duplicated as a field. `payload` is the generic, kind-specific extra + /// slot — empty for `block`, where presence alone is the fact — mirroring + /// `Ping.payload`. Write-once and immutable, so there is no Core Data + /// equivalent and no system-fields archive; a re-send of an existing + /// decision is a benign conflict the send path drops. + static func decisionRecord( + kind: String, + key: String, + payload: String? = nil, + zone: CKRecordZone.ID + ) -> CKRecord { + let name = decisionRecordName(kind: kind, key: key) + let recordID = CKRecord.ID(recordName: name, zoneID: zone) + let record = CKRecord(recordType: "Decision", recordID: recordID) + record["kind"] = kind as CKRecordValue + if let payload { + record["payload"] = payload as CKRecordValue + } + record["createdAt"] = Date() as CKRecordValue + return record + } + static func playerRecord( gameID: UUID, authorID: String, @@ -482,6 +529,50 @@ enum RecordSerializer { return true } + /// Projects an inbound `Decision` record onto local Core Data. For + /// `kind == "block"` this upserts a `FriendEntity` tombstone keyed by the + /// blocked author so the block becomes authoritative across the user's own + /// devices: `applyInvitePings` (authorID-keyed) and the friendship + /// bootstrap's `friendExists` (pairKey-keyed) both then suppress the + /// blocked collaborator everywhere. A device that never befriended the + /// author still gets a minimal blocked row. Returns `true` when a row was + /// written. `localAuthorID` lets the pairKey be derived for the bootstrap + /// short-circuit; it's deterministic from the unordered author pair. + @discardableResult + static func applyDecisionRecord( + _ record: CKRecord, + to ctx: NSManagedObjectContext, + localAuthorID: String? + ) -> Bool { + guard record.recordType == "Decision" else { return false } + // Identity comes from the record name (always present, immutable); + // `kind` is also mirrored as a field, name-parse as the fallback. + let parsed = parseDecisionRecordName(record.recordID.recordName) + guard let kind = (record["kind"] as? String) ?? parsed?.kind, + let key = parsed?.key, + !kind.isEmpty, !key.isEmpty + else { return false } + + switch kind { + case "block": + let req = NSFetchRequest<FriendEntity>(entityName: "FriendEntity") + req.predicate = NSPredicate(format: "authorID == %@", key) + req.fetchLimit = 1 + let friend = (try? ctx.fetch(req).first) ?? FriendEntity(context: ctx) + friend.authorID = key + friend.isBlocked = true + if friend.pairKey == nil, + let localAuthorID, !localAuthorID.isEmpty { + friend.pairKey = FriendZone.pairKey(localAuthorID, key) + } + if friend.createdAt == nil { friend.createdAt = Date() } + return true + default: + // Unknown kind from a newer build — ignore rather than guess. + return false + } + } + // MARK: - System fields encode/decode static func encodeSystemFields(of record: CKRecord) -> Data? { diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift @@ -486,6 +486,24 @@ actor SyncEngine { Task { try? await engine.sendChanges() } } + /// 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. + func enqueueDecision(kind: String, key: String) { + guard let engine = privateEngine else { return } + let zoneID = RecordSerializer.accountZoneID + // CKSyncEngine dedupes redundant saveZone requests, so it's safe to + // repeat — block may be the first thing ever written to this zone. + engine.state.add(pendingDatabaseChanges: [.saveZone(CKRecordZone(zoneID: zoneID))]) + let name = RecordSerializer.decisionRecordName(kind: kind, key: key) + let recordID = CKRecord.ID(recordName: name, zoneID: zoneID) + engine.state.add(pendingRecordZoneChanges: [.saveRecord(recordID)]) + 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 @@ -1827,6 +1845,12 @@ actor SyncEngine { zone: zoneID ) } + if name.hasPrefix("decision-") { + guard let (kind, key) = RecordSerializer.parseDecisionRecordName(name) else { + return nil + } + return RecordSerializer.decisionRecord(kind: kind, key: key, zone: zoneID) + } let ctx = persistence.container.newBackgroundContext() return ctx.performAndWait { if name.hasPrefix("game-") { @@ -2193,6 +2217,12 @@ actor SyncEngine { if let ping = Self.parsePingRecord(record) { pings.append(ping) } + case "Decision": + RecordSerializer.applyDecisionRecord( + record, + to: ctx, + localAuthorID: localAuthorID + ) default: break } @@ -2352,9 +2382,11 @@ actor SyncEngine { } let ctx = persistence.container.newBackgroundContext() ctx.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump - let (failureMessages, orphanedZones): ([String], Set<CKRecordZone.ID>) = ctx.performAndWait { + let (failureMessages, orphanedZones, resolvedDecisions): + ([String], Set<CKRecordZone.ID>, Set<CKRecord.ID>) = ctx.performAndWait { var messages: [String] = [] var orphaned = Set<CKRecordZone.ID>() + var settledDecisions = Set<CKRecord.ID>() for record in event.savedRecords { self.writeBackSystemFields(record: record, in: ctx) if record.recordID.recordName.hasPrefix("game-") { @@ -2370,6 +2402,15 @@ actor SyncEngine { } else if !isPrivate, self.isInvalidSharedZoneOwnerError(err) { orphaned.insert(failure.record.recordID.zoneID) + } else if name.hasPrefix("decision-"), + err.domain == CKErrorDomain, + err.code == CKError.serverRecordChanged.rawValue { + // The durable fact already exists server-side (a re-block + // or an echo of our own write). Decisions are write-once, + // so this is success — drop the pending change so the + // immutable record doesn't retry-loop forever. + settledDecisions.insert(failure.record.recordID) + messages.append("send: decision \(name) already present — settled") } else if self.recoverServerChangedSave(failure.error, failedRecordName: name, in: ctx) { messages.append( "send: recovered stale system fields for \(name) from CloudKit server record" @@ -2392,11 +2433,16 @@ actor SyncEngine { ) } } - return (messages, orphaned) + return (messages, orphaned, settledDecisions) } if !orphanedZones.isEmpty { await applyZoneOrphaning(orphanedZones, isPrivate: isPrivate) } + if !resolvedDecisions.isEmpty, let privateEngine { + privateEngine.state.remove( + pendingRecordZoneChanges: resolvedDecisions.map { .saveRecord($0) } + ) + } for message in failureMessages { await trace(message) } diff --git a/Tests/Unit/RecordSerializerTests.swift b/Tests/Unit/RecordSerializerTests.swift @@ -296,4 +296,132 @@ struct RecordSerializerTests { #expect(decoded?.recordID.zoneID.zoneName == "game-\(gameID.uuidString)") #expect(decoded?.recordID.recordName == "test-record") } + + // MARK: - Decision records + + @Test("Decision record name uses decision-<kind>-<key> format") + func decisionRecordNameFormat() { + let name = RecordSerializer.decisionRecordName(kind: "block", key: "_bob") + #expect(name == "decision-block-_bob") + } + + @Test("parseDecisionRecordName round-trips, preserving dashes in the key") + func decisionNameRoundTrip() { + let name = RecordSerializer.decisionRecordName(kind: "block", key: "_b-o-b") + let parsed = RecordSerializer.parseDecisionRecordName(name) + #expect(parsed?.kind == "block") + #expect(parsed?.key == "_b-o-b") + } + + @Test("parseDecisionRecordName rejects non-decision and malformed names") + func decisionNameRejectsOthers() { + #expect(RecordSerializer.parseDecisionRecordName("ping-1234") == nil) + #expect(RecordSerializer.parseDecisionRecordName("decision-block") == nil) + #expect(RecordSerializer.parseDecisionRecordName("decision--k") == nil) + } + + @Test("decisionRecord keeps identity in the name, not a key field") + func decisionRecordFields() { + let record = RecordSerializer.decisionRecord( + kind: "block", + key: "_bob", + zone: RecordSerializer.accountZoneID + ) + #expect(record.recordType == "Decision") + // Identity (kind + key) lives in the record name. + #expect(record.recordID.recordName == "decision-block-_bob") + #expect(record.recordID.zoneID.zoneName == "account") + #expect(record["kind"] as? String == "block") + // `key` is not duplicated as a field; `payload` is unused for block. + #expect(record["key"] == nil) + #expect(record["payload"] == nil) + #expect(record["createdAt"] as? Date != nil) + } + + @Test("decisionRecord carries an optional payload when provided") + func decisionRecordPayload() { + let record = RecordSerializer.decisionRecord( + kind: "snooze", + key: "_bob", + payload: "{\"until\":1}", + zone: RecordSerializer.accountZoneID + ) + #expect(record.recordID.recordName == "decision-snooze-_bob") + #expect(record["payload"] as? String == "{\"until\":1}") + } + + @Test("applyDecisionRecord(.block) creates a blocked FriendEntity with derived pairKey") + @MainActor func applyDecisionBlockCreatesTombstone() throws { + let persistence = makeTestPersistence() + let ctx = persistence.viewContext + let record = RecordSerializer.decisionRecord( + kind: "block", + key: "_bob", + zone: RecordSerializer.accountZoneID + ) + + let wrote = RecordSerializer.applyDecisionRecord( + record, + to: ctx, + localAuthorID: "_alice" + ) + #expect(wrote) + + let req = NSFetchRequest<FriendEntity>(entityName: "FriendEntity") + req.predicate = NSPredicate(format: "authorID == %@", "_bob") + let friend = try ctx.fetch(req).first + #expect(friend?.isBlocked == true) + #expect(friend?.pairKey == FriendZone.pairKey("_alice", "_bob")) + } + + @Test("applyDecisionRecord(.block) flips an existing active friend to blocked") + @MainActor func applyDecisionBlockMarksExistingFriend() throws { + let persistence = makeTestPersistence() + let ctx = persistence.viewContext + + let existing = FriendEntity(context: ctx) + existing.authorID = "_bob" + existing.pairKey = "k-existing" + existing.friendZoneName = "friend-k-existing" + existing.friendZoneOwnerName = CKCurrentUserDefaultName + existing.databaseScope = 0 + existing.isBlocked = false + existing.createdAt = Date() + try ctx.save() + + let record = RecordSerializer.decisionRecord( + kind: "block", + key: "_bob", + zone: RecordSerializer.accountZoneID + ) + RecordSerializer.applyDecisionRecord(record, to: ctx, localAuthorID: "_alice") + + let req = NSFetchRequest<FriendEntity>(entityName: "FriendEntity") + req.predicate = NSPredicate(format: "authorID == %@", "_bob") + let rows = try ctx.fetch(req) + // Upsert, not insert: the existing row flips rather than duplicating, + // and its original pairKey/zone are left intact. + #expect(rows.count == 1) + #expect(rows.first?.isBlocked == true) + #expect(rows.first?.pairKey == "k-existing") + } + + @Test("applyDecisionRecord ignores unknown kinds") + @MainActor func applyDecisionIgnoresUnknownKind() throws { + let persistence = makeTestPersistence() + let ctx = persistence.viewContext + let record = RecordSerializer.decisionRecord( + kind: "future", + key: "_bob", + zone: RecordSerializer.accountZoneID + ) + let wrote = RecordSerializer.applyDecisionRecord( + record, + to: ctx, + localAuthorID: "_alice" + ) + #expect(!wrote) + let req = NSFetchRequest<FriendEntity>(entityName: "FriendEntity") + #expect(try ctx.count(for: req) == 0) + } }