commit 7a41b6bbe1f700d10db63d9f6d62f6ed66e299d9
parent bf1b6f354240c7e142a3823b1ceaa0c0c68ab539
Author: Michael Camilleri <[email protected]>
Date: Wed, 10 Jun 2026 16:33:18 +0900
Scope Moves enqueues to the current author's row
enqueueMoves looked up the local device's Moves row by (game, deviceID)
alone with a fetchLimit of 1. The record name embeds the authorID, so
after an iCloud account switch the same device holds distinct rows for
the old and new accounts -- and the device-only fetch could return the
old account's row, enqueuing a save of a record the current user no
longer owns. enqueueUnconfirmedMoves had the same shape, so the
crash-recovery sweep could re-push an old account's leftover unconfirmed
rows under the new account.
This commit narrows both lookups to the current authorID. Each method
resolves the local author first (becoming async for the MainActor hop;
every caller already awaited them as actor methods) and adds it to the
predicate. When no author is known yet -- a first launch before the
userRecordID lookup lands, or tests without a provider -- the lookup
falls back to the previous device-only match, which is unambiguous until
a switch has actually happened.
Two ShareRoutingTests cases pin the narrowing: a game holding rows for
two authors on the same device enqueues only the current author's
record, and the unconfirmed-recovery sweep skips a game whose only
unconfirmed row belongs to the old account.
Co-Authored-By: Claude Fable 5 <[email protected]>
Diffstat:
2 files changed, 134 insertions(+), 18 deletions(-)
diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift
@@ -417,8 +417,17 @@ actor SyncEngine {
/// `enqueueUnconfirmedMoves`): the solver's final letters must reach
/// CloudKit promptly even when no peer is on the socket, so force the
/// send rather than waiting for the next foreground.
- func enqueueMoves(gameIDs: Set<UUID>, drain: Bool = true) {
+ func enqueueMoves(gameIDs: Set<UUID>, drain: Bool = true) async {
guard !gameIDs.isEmpty else { return }
+ // The local-device row is matched by author as well as device: after
+ // an iCloud account switch this device's old rows persist under the
+ // previous authorID (the record name embeds the author, so they are
+ // distinct rows), and a device-only `fetchLimit = 1` lookup could pick
+ // the old account's record and push a save the user no longer owns.
+ // When no author is known yet (provider unset, first launch) fall back
+ // to the device-only match, which is unambiguous until a switch has
+ // happened.
+ let localAuthorID = await currentLocalAuthorID()
let ctx = persistence.container.newBackgroundContext()
let (privateRecordIDs, sharedRecordIDs): ([CKRecord.ID], [CKRecord.ID]) = ctx.performAndWait {
var privateIDs: [CKRecord.ID] = []
@@ -427,11 +436,20 @@ actor SyncEngine {
guard let info = zoneInfo(forGameID: gameID, in: ctx) else { continue }
let isShared = info.scope == 1
let req = NSFetchRequest<MovesEntity>(entityName: "MovesEntity")
- req.predicate = NSPredicate(
- format: "game.id == %@ AND deviceID == %@",
- gameID as CVarArg,
- RecordSerializer.localDeviceID
- )
+ if let localAuthorID, !localAuthorID.isEmpty {
+ req.predicate = NSPredicate(
+ format: "game.id == %@ AND deviceID == %@ AND authorID == %@",
+ gameID as CVarArg,
+ RecordSerializer.localDeviceID,
+ localAuthorID
+ )
+ } else {
+ req.predicate = NSPredicate(
+ format: "game.id == %@ AND deviceID == %@",
+ gameID as CVarArg,
+ RecordSerializer.localDeviceID
+ )
+ }
req.fetchLimit = 1
guard let entity = try? ctx.fetch(req).first,
let recordName = entity.ckRecordName
@@ -461,20 +479,32 @@ actor SyncEngine {
/// Re-enqueues locally persisted Moves rows that do not yet have CloudKit
/// system fields. Covers crash/relaunch recovery and any save that reached
- /// Core Data before CKSyncEngine recorded the pending change.
+ /// Core Data before CKSyncEngine recorded the pending change. Scoped to the
+ /// current author for the same reason as `enqueueMoves`: an account
+ /// switch leaves the old account's unconfirmed rows behind on this device,
+ /// and recovery must not push them under the new account.
@discardableResult
- func enqueueUnconfirmedMoves() -> Int {
+ func enqueueUnconfirmedMoves() async -> Int {
+ let localAuthorID = await currentLocalAuthorID()
let ctx = persistence.container.newBackgroundContext()
let gameIDs: Set<UUID> = ctx.performAndWait {
let req = NSFetchRequest<MovesEntity>(entityName: "MovesEntity")
- req.predicate = NSPredicate(
- format: "ckSystemFields == nil AND deviceID == %@",
- RecordSerializer.localDeviceID
- )
+ if let localAuthorID, !localAuthorID.isEmpty {
+ req.predicate = NSPredicate(
+ format: "ckSystemFields == nil AND deviceID == %@ AND authorID == %@",
+ RecordSerializer.localDeviceID,
+ localAuthorID
+ )
+ } else {
+ req.predicate = NSPredicate(
+ format: "ckSystemFields == nil AND deviceID == %@",
+ RecordSerializer.localDeviceID
+ )
+ }
let entities = (try? ctx.fetch(req)) ?? []
return Set(entities.compactMap { $0.game?.id })
}
- enqueueMoves(gameIDs: gameIDs)
+ await enqueueMoves(gameIDs: gameIDs)
return gameIDs.count
}
@@ -1162,7 +1192,7 @@ actor SyncEngine {
loggedFirstSharedPushPayload = false
playerSendBurstDepth = [:]
playerSendBurstPending = []
- _ = enqueueUnconfirmedMoves()
+ _ = await enqueueUnconfirmedMoves()
}
// MARK: - Private helpers
diff --git a/Tests/Unit/Sync/ShareRoutingTests.swift b/Tests/Unit/Sync/ShareRoutingTests.swift
@@ -56,22 +56,26 @@ struct ShareRoutingTests {
/// Inserts a confirmed (system-fields-bearing — simulated by setting non-nil
/// `ckSystemFields`-equivalent state) MovesEntity row for the local device.
- /// `enqueueMoves` looks up rows by `(game.id, deviceID == localDeviceID)`.
+ /// `enqueueMoves` looks up rows by `(game.id, deviceID == localDeviceID)`,
+ /// narrowed to the current authorID when the engine knows one — the
+ /// account-switch tests below seed a second author on the same device to
+ /// pin that narrowing.
private static func seedMovesEntity(
for game: GameEntity,
gameID: UUID,
in ctx: NSManagedObjectContext,
- confirmed: Bool = false
+ confirmed: Bool = false,
+ authorID: String = writerAuthorID
) {
let entity = MovesEntity(context: ctx)
entity.game = game
- entity.authorID = writerAuthorID
+ entity.authorID = authorID
entity.deviceID = RecordSerializer.localDeviceID
entity.cells = Data()
entity.updatedAt = Date()
entity.ckRecordName = RecordSerializer.recordName(
forMovesInGame: gameID,
- authorID: writerAuthorID,
+ authorID: authorID,
deviceID: RecordSerializer.localDeviceID
)
if confirmed {
@@ -269,4 +273,86 @@ struct ShareRoutingTests {
#expect(count == 1)
#expect(privateNames.contains(recordName))
}
+
+ /// Simulates the residue of an iCloud account switch: the same device
+ /// holds Moves rows under two authorIDs. With the previous device-only
+ /// lookup, the `fetchLimit = 1` fetch could return the old account's row
+ /// and enqueue a record the current user no longer owns.
+ @Test("Enqueue targets the current author's row, not an old account's")
+ func enqueueScopedToCurrentAuthorAfterAccountSwitch() async throws {
+ let persistence = makeTestPersistence()
+ let ctx = persistence.viewContext
+
+ let gameID = UUID()
+ let game = GameEntity(context: ctx)
+ game.id = gameID
+ game.title = "Private"
+ game.puzzleSource = ""
+ game.createdAt = Date()
+ game.updatedAt = Date()
+ game.ckRecordName = "game-\(gameID.uuidString)"
+ game.ckZoneName = "game-\(gameID.uuidString)"
+ game.databaseScope = 0
+ Self.seedMovesEntity(for: game, gameID: gameID, in: ctx, authorID: "old-account")
+ Self.seedMovesEntity(for: game, gameID: gameID, in: ctx)
+ try ctx.save()
+
+ let container = CKContainer(identifier: "iCloud.net.inqk.crossmate.v3")
+ let engine = SyncEngine(container: container, persistence: persistence)
+ await engine.start()
+ await engine.setLocalAuthorIDProvider { Self.writerAuthorID }
+
+ await engine.enqueueMoves(gameIDs: [gameID])
+
+ let privateNames = await engine.pendingSaveRecordNames(scope: .private)
+ #expect(privateNames.contains(movesRecordName(for: gameID)))
+ #expect(!privateNames.contains(RecordSerializer.recordName(
+ forMovesInGame: gameID,
+ authorID: "old-account",
+ deviceID: RecordSerializer.localDeviceID
+ )))
+ }
+
+ @Test("Unconfirmed recovery skips an old account's leftover rows")
+ func unconfirmedRecoveryScopedToCurrentAuthor() async throws {
+ let persistence = makeTestPersistence()
+ let ctx = persistence.viewContext
+
+ // Game A carries only the old account's unconfirmed row; game B
+ // carries the current author's. Recovery must re-enqueue B alone.
+ func makeGame(_ title: String) -> (GameEntity, UUID) {
+ let id = UUID()
+ let game = GameEntity(context: ctx)
+ game.id = id
+ game.title = title
+ game.puzzleSource = ""
+ game.createdAt = Date()
+ game.updatedAt = Date()
+ game.ckRecordName = "game-\(id.uuidString)"
+ game.ckZoneName = "game-\(id.uuidString)"
+ game.databaseScope = 0
+ return (game, id)
+ }
+ let (gameA, idA) = makeGame("Old account's")
+ Self.seedMovesEntity(for: gameA, gameID: idA, in: ctx, authorID: "old-account")
+ let (gameB, idB) = makeGame("Current author's")
+ Self.seedMovesEntity(for: gameB, gameID: idB, in: ctx)
+ try ctx.save()
+
+ let container = CKContainer(identifier: "iCloud.net.inqk.crossmate.v3")
+ let engine = SyncEngine(container: container, persistence: persistence)
+ await engine.start()
+ await engine.setLocalAuthorIDProvider { Self.writerAuthorID }
+
+ let count = await engine.enqueueUnconfirmedMoves()
+
+ let privateNames = await engine.pendingSaveRecordNames(scope: .private)
+ #expect(count == 1)
+ #expect(privateNames.contains(movesRecordName(for: idB)))
+ #expect(!privateNames.contains(RecordSerializer.recordName(
+ forMovesInGame: idA,
+ authorID: "old-account",
+ deviceID: RecordSerializer.localDeviceID
+ )))
+ }
}
