commit 39eb714f6676e7a1cfc230ceaae7341fe4e1c100
parent c3f601eb3ebb0df25ca9317e12ed2de612f5bd0a
Author: Michael Camilleri <[email protected]>
Date: Sat, 15 Aug 2026 21:27:06 +0900
Coalesce Chronicle handovers across navigation
A Chronicle handover could race the Game List sweep when the user backed
out while the revocation callback was fetching the archive. The
competing Core Data transactions could make the callback report failure
and restore the sticky 'Puzzle Not Shared' banner even though another
path had materialised the Chronicle.
This commit coalesces promotion and retirement per game so every caller
observes the same result. Promotion now materialises the Chronicle and
supersedes the live row without deleting it. AppServices counts mounted
puzzle destinations and retires the row only after the last destination
disappears, while retirement verifies that a complete Chronicle is
durable before removing the live copy.
Co-Authored-By: Codex GPT 5.6 Sol <[email protected]>
Diffstat:
4 files changed, 269 insertions(+), 37 deletions(-)
diff --git a/Crossmate/CrossmateApp.swift b/Crossmate/CrossmateApp.swift
@@ -1091,9 +1091,16 @@ private struct PuzzleDisplayView: View {
break
}
}
+ .onAppear {
+ services.puzzleAppeared(gameID: gameID)
+ }
.onDisappear {
openPuzzleFollowUpTask?.cancel()
openPuzzleFollowUpTask = nil
+ services.puzzleDisappeared(
+ gameID: gameID,
+ wasAccessRevoked: session?.mutator.isAccessRevoked == true
+ )
guard session?.mutator.isArchived == false else { return }
let selectionPublisher = services.playerSelectionPublisher
let movesUpdater = services.movesUpdater
diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift
@@ -454,6 +454,11 @@ final class AppServices {
/// debounced.
private let remotePuzzleGridFreshenDebounce: TimeInterval = 5
private var isGameListVisible = false
+ /// Puzzle destinations currently mounted by SwiftUI. Unlike
+ /// `GameStore.currentEntity`, this is cleared synchronously on disappearance,
+ /// so Chronicle cleanup can distinguish a visible or transitioning puzzle
+ /// from a stale last-loaded entity.
+ private var mountedPuzzleCounts: [UUID: Int] = [:]
/// Whether the app is foreground-active — the single source of truth for
/// "the user is actively using the app." `publishReadCursor(.activeLease)`
/// consults it so a background CKSyncEngine wake can never re-arm our
@@ -1265,16 +1270,16 @@ final class AppServices {
// than deleted, so the mounted view isn't left on a dead
// `GameEntity`; the open puzzle is then handed over to the Chronicle,
// which shows its own load rather than a banner over a dead game.
- let isOnScreen = self?.isPuzzleOnScreen(gameID: gameID) == true
- let chronicleID = await gameArchiver.promoteRevoked(
- gameID: gameID,
- retiringLiveRow: !isOnScreen
- )
- if let chronicleID, isOnScreen {
- NotificationNavigationBroker.shared.replaceOpenGame(
- gameID,
- with: chronicleID
- )
+ let chronicleID = await gameArchiver.promoteRevoked(gameID: gameID)
+ if let chronicleID {
+ if self?.isPuzzleOnScreen(gameID: gameID) == true {
+ NotificationNavigationBroker.shared.replaceOpenGame(
+ gameID,
+ with: chronicleID
+ )
+ } else {
+ await gameArchiver.retireRevokedLiveRow(gameID: gameID)
+ }
}
// Surface the revocation as a sticky, input-blocking banner on
// the open puzzle, replacing the former AccessRevokedBanner
@@ -1518,14 +1523,11 @@ final class AppServices {
}
}
- /// Whether `gameID` is the puzzle currently pushed on screen. `currentEntity`
- /// alone is stale after a close (nothing clears it), so it is paired with the
- /// Game List's own visibility: the list is showing exactly when no puzzle is.
- /// Backgrounding doesn't fire either view's appearance callbacks, so a puzzle
- /// left open while the app is away still reads as on screen — which is the
- /// case that matters here.
+ /// Whether `gameID` still has a mounted puzzle destination. This stays true
+ /// while backgrounded and through a pop transition, until `onDisappear`
+ /// confirms that deleting its live Core Data row is safe.
private func isPuzzleOnScreen(gameID: UUID) -> Bool {
- !isGameListVisible && store.currentEntity?.id == gameID
+ mountedPuzzleCounts[gameID, default: 0] > 0
}
func gameListAppeared() async {
@@ -1534,7 +1536,10 @@ final class AppServices {
// screen, before the list renders, so the row goes straight from live
// game to Chronicle without a revoked state in between. Also the
// cold-launch backstop for a revocation that arrived while terminated.
- await gameArchiver.promoteRevokedCompleted()
+ let promoted = await gameArchiver.promoteRevokedCompleted()
+ for gameID in promoted where !isPuzzleOnScreen(gameID: gameID) {
+ await gameArchiver.retireRevokedLiveRow(gameID: gameID)
+ }
await freshenGameList(reason: .appeared)
}
@@ -1542,6 +1547,24 @@ final class AppServices {
isGameListVisible = false
}
+ func puzzleAppeared(gameID: UUID) {
+ mountedPuzzleCounts[gameID, default: 0] += 1
+ }
+
+ /// Clears the mounted destination before starting Chronicle cleanup. If
+ /// promotion is still fetching, the archiver joins that work before
+ /// deciding whether a complete Chronicle exists.
+ func puzzleDisappeared(gameID: UUID, wasAccessRevoked: Bool) {
+ let remaining = max(0, mountedPuzzleCounts[gameID, default: 0] - 1)
+ if remaining == 0 {
+ mountedPuzzleCounts[gameID] = nil
+ } else {
+ mountedPuzzleCounts[gameID] = remaining
+ }
+ guard wasAccessRevoked, remaining == 0 else { return }
+ Task { await gameArchiver.retireRevokedLiveRow(gameID: gameID) }
+ }
+
func loadRecentCompleted(since cutoff: Date) async -> GameArchiver.CompletedPage {
guard await ensureICloudSyncStarted() else {
return .init(oldestCompletedAt: nil, hasMore: false)
diff --git a/Crossmate/Sync/GameArchiver.swift b/Crossmate/Sync/GameArchiver.swift
@@ -2,6 +2,38 @@ import CloudKit
import CoreData
import Foundation
+/// Shares one asynchronous operation among overlapping callers for the same
+/// key. Main-actor isolation makes the lookup-and-install atomic even though
+/// the operation itself can suspend.
+@MainActor
+final class AsyncTaskCoalescer<Key: Hashable, Value: Sendable> {
+ private var tasks: [Key: Task<Value, Never>] = [:]
+
+ func run(
+ for key: Key,
+ operation: @MainActor @Sendable @escaping () async -> Value
+ ) async -> Value {
+ if let task = tasks[key] {
+ return await task.value
+ }
+
+ let task = Task { @MainActor in
+ await operation()
+ }
+ tasks[key] = task
+ let value = await task.value
+ tasks[key] = nil
+ return value
+ }
+
+ /// Waits only when work for `key` is already under way. Used by cleanup
+ /// that must not race the materialization it follows.
+ func waitForCurrentTask(for key: Key) async {
+ guard let task = tasks[key] else { return }
+ _ = await task.value
+ }
+}
+
enum CompletedMetadataPageWalker {
struct Result<Item, Cursor> {
let selected: [Item]
@@ -106,6 +138,8 @@ final class GameArchiver {
private var ensuredArchiveZone = false
private var chronicleCursor: CKQueryOperation.Cursor?
private var bufferedCompleted: [CompletedMetadata] = []
+ private let revokedPromotionCoalescer = AsyncTaskCoalescer<UUID, UUID?>()
+ private let revokedRetirementCoalescer = AsyncTaskCoalescer<UUID, Bool>()
init(
container: CKContainer,
@@ -430,11 +464,10 @@ final class GameArchiver {
}
/// Sweeps every finished game whose shared zone has gone, promoting each to
- /// its Chronicle and retiring the live row behind it. Catches up the rows
- /// `promoteRevoked` left standing because their puzzle was on screen, and
- /// backstops both a revocation the app never got to act on because it was
+ /// its Chronicle and returning the live rows now eligible for retirement.
+ /// Catches up both a revocation the app never got to act on because it was
/// terminated and one whose Chronicle could not be read at the time.
- func promoteRevokedCompleted() async {
+ func promoteRevokedCompleted() async -> [UUID] {
let ctx = persistence.container.newBackgroundContext()
let ids: [UUID] = await ctx.perform {
let req = NSFetchRequest<GameEntity>(entityName: "GameEntity")
@@ -445,9 +478,13 @@ final class GameArchiver {
)
return ((try? ctx.fetch(req)) ?? []).compactMap(\.id)
}
+ var promoted: [UUID] = []
for id in ids {
- await promoteRevoked(gameID: id)
+ if await promoteRevoked(gameID: id) != nil {
+ promoted.append(id)
+ }
}
+ return promoted
}
/// Promotes a participant's private archive when the owner retires the live
@@ -464,13 +501,22 @@ final class GameArchiver {
/// puzzle, with the zone gone and no way back. Those stay revoked rows, a
/// state the user is actually shown.
///
- /// `retiringLiveRow` is false while that puzzle is on screen. The Chronicle
- /// is materialized either way — the caller hands the open view over to it —
- /// but deleting the row underneath a mounted `PuzzleView` would strand it on
- /// a dead `GameEntity`, so the live row is hidden now and swept later by
- /// `promoteRevokedCompleted`.
+ /// Overlapping callers share one promotion. This matters when the revocation
+ /// callback is suspended in CloudKit and the user returns to the Game List,
+ /// whose catch-up sweep asks for the same promotion.
+ ///
+ /// Promotion never deletes the live row. It is hidden behind the Chronicle
+ /// here, then retired separately once no mounted `PuzzleView` can still hold
+ /// its `GameEntity`.
@discardableResult
- func promoteRevoked(gameID: UUID, retiringLiveRow: Bool = true) async -> UUID? {
+ func promoteRevoked(gameID: UUID) async -> UUID? {
+ await revokedPromotionCoalescer.run(for: gameID) { [weak self] in
+ guard let self else { return nil }
+ return await self.performPromoteRevoked(gameID: gameID)
+ }
+ }
+
+ private func performPromoteRevoked(gameID: UUID) async -> UUID? {
// A game this device never saw finish is a genuine mid-play revocation,
// whatever the account's Chronicle says; leave it as a revoked row.
let ctx = persistence.container.newBackgroundContext()
@@ -508,14 +554,10 @@ final class GameArchiver {
req.predicate = NSPredicate(format: "id == %@", gameID as CVarArg)
req.fetchLimit = 1
if let original = try? promoteCtx.fetch(req).first {
- if retiringLiveRow {
- promoteCtx.delete(original)
- } else {
- // Keep the row for the mounted view to finish with, but take
- // it out of the library now: the Chronicle is already the
- // visible representation, and they share a list identity.
- original.isSupersededByChronicle = true
- }
+ // Keep the row for any mounted view to finish with, but take it
+ // out of the library now: the Chronicle is already the visible
+ // representation, and they share a list identity.
+ original.isSupersededByChronicle = true
}
if promoteCtx.hasChanges {
do {
@@ -528,6 +570,49 @@ final class GameArchiver {
}
}
+ /// Deletes a revoked live row after its Chronicle is durable and any
+ /// in-flight promotion has finished. Calling this from `PuzzleView`'s
+ /// disappearance is what makes deletion safe for an open-puzzle handover;
+ /// off-screen callers can invoke it immediately after promotion.
+ @discardableResult
+ func retireRevokedLiveRow(gameID: UUID) async -> Bool {
+ await revokedPromotionCoalescer.waitForCurrentTask(for: gameID)
+ return await revokedRetirementCoalescer.run(for: gameID) { [weak self] in
+ guard let self else { return false }
+ return await self.performRetireRevokedLiveRow(gameID: gameID)
+ }
+ }
+
+ private func performRetireRevokedLiveRow(gameID: UUID) async -> Bool {
+ let ctx = persistence.container.newBackgroundContext()
+ return await ctx.perform {
+ let chronicleReq = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ chronicleReq.predicate = NSPredicate(
+ format: "id == %@ AND replayCacheComplete == YES",
+ Archive.archiveGameID(for: gameID) as CVarArg
+ )
+ chronicleReq.fetchLimit = 1
+ guard (try? ctx.fetch(chronicleReq).first) != nil else { return false }
+
+ let liveReq = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ liveReq.predicate = NSPredicate(
+ format: "id == %@ AND isAccessRevoked == YES " +
+ "AND ckRecordName BEGINSWITH %@",
+ gameID as CVarArg,
+ "game-"
+ )
+ liveReq.fetchLimit = 1
+ guard let live = try? ctx.fetch(liveReq).first else { return true }
+ ctx.delete(live)
+ do {
+ try ctx.save()
+ return true
+ } catch {
+ return false
+ }
+ }
+ }
+
private func migrateMaterializedLegacyArchives() async {
let ctx = persistence.container.newBackgroundContext()
let candidates: [(localID: UUID, originalID: UUID)] = await ctx.perform {
diff --git a/Tests/Unit/ArchiveTests.swift b/Tests/Unit/ArchiveTests.swift
@@ -33,6 +33,123 @@ struct ArchiveTests {
D3. Down 3 ~ CEH
"""
+ @Test("overlapping work for one key is coalesced")
+ func overlappingWorkIsCoalesced() async {
+ let coalescer = AsyncTaskCoalescer<UUID, Int>()
+ let key = UUID()
+ var starts = 0
+
+ let first = Task { @MainActor in
+ await coalescer.run(for: key) {
+ starts += 1
+ try? await Task.sleep(for: .milliseconds(20))
+ return 42
+ }
+ }
+ while starts == 0 { await Task.yield() }
+ let second = Task { @MainActor in
+ await coalescer.run(for: key) {
+ starts += 1
+ return 99
+ }
+ }
+
+ #expect(await first.value == 42)
+ #expect(await second.value == 42)
+ #expect(starts == 1)
+
+ let later = await coalescer.run(for: key) {
+ starts += 1
+ return 7
+ }
+ #expect(later == 7)
+ #expect(starts == 2)
+ }
+
+ @Test("retirement requires a complete materialized Chronicle")
+ func retirementRequiresCompleteChronicle() async throws {
+ let persistence = makeTestPersistence()
+ let engine = try makeSyncEngine(persistence)
+ let archiver = GameArchiver(
+ container: CloudContainer.container,
+ persistence: persistence,
+ syncEngine: engine
+ )
+ let original = UUID()
+ let ctx = persistence.viewContext
+ let live = GameEntity(context: ctx)
+ live.id = original
+ live.title = "Live"
+ live.puzzleSource = source
+ live.createdAt = Date()
+ live.updatedAt = Date()
+ live.completedAt = Date()
+ live.databaseScope = 1
+ live.isAccessRevoked = true
+ live.ckRecordName = "game-\(original.uuidString)"
+
+ _ = Archive.materialize(
+ Archive.payload(
+ from: sampleSnapshot(originalGameID: original),
+ replayState: .waiting(missing: 1)
+ ),
+ in: ctx
+ )
+ try ctx.save()
+
+ #expect(await archiver.retireRevokedLiveRow(gameID: original) == false)
+ let liveRequest = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ liveRequest.predicate = NSPredicate(format: "id == %@", original as CVarArg)
+ #expect(try ctx.count(for: liveRequest) == 1)
+ }
+
+ @Test("retirement deletes the live row behind a complete Chronicle")
+ func retirementDeletesLiveRow() async throws {
+ let persistence = makeTestPersistence()
+ let engine = try makeSyncEngine(persistence)
+ let archiver = GameArchiver(
+ container: CloudContainer.container,
+ persistence: persistence,
+ syncEngine: engine
+ )
+ let original = UUID()
+ let ctx = persistence.viewContext
+ let live = GameEntity(context: ctx)
+ live.id = original
+ live.title = "Live"
+ live.puzzleSource = source
+ live.createdAt = Date()
+ live.updatedAt = Date()
+ live.completedAt = Date()
+ live.databaseScope = 1
+ live.isAccessRevoked = true
+ live.ckRecordName = "game-\(original.uuidString)"
+
+ _ = Archive.materialize(
+ Archive.payload(from: sampleSnapshot(originalGameID: original)),
+ in: ctx
+ )
+ try ctx.save()
+
+ #expect(await archiver.retireRevokedLiveRow(gameID: original))
+ let verifyCtx = persistence.container.newBackgroundContext()
+ let counts = await verifyCtx.perform {
+ let liveRequest = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ liveRequest.predicate = NSPredicate(format: "id == %@", original as CVarArg)
+ let chronicleRequest = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ chronicleRequest.predicate = NSPredicate(
+ format: "id == %@",
+ Archive.archiveGameID(for: original) as CVarArg
+ )
+ return (
+ live: (try? verifyCtx.count(for: liveRequest)) ?? -1,
+ chronicle: (try? verifyCtx.count(for: chronicleRequest)) ?? -1
+ )
+ }
+ #expect(counts.live == 0)
+ #expect(counts.chronicle == 1)
+ }
+
@Test("recent Chronicle paging advances through the returned cursor")
func recentPagingAdvancesCursor() async throws {
let now = Date()
