commit f81539b62cec3dc2d9707b8e6d7cb36b0e70ec82
parent 7015ff2f7af074959b4cab56de3c15b5a38a0138
Author: Michael Camilleri <[email protected]>
Date: Thu, 2 Jul 2026 22:05:55 +0900
Unregister push addresses for departed games
This commit stops a device staying reachable at the push worker for a
game it no longer holds. The previous paths never refreshed the worker
registration when a game was deleted locally, removed remotely, or had
its shared-zone access revoked, and a failed unregister was logged and
dropped — so the stale address stayed bound, and the device could keep
receiving pushes for the departed game until an unrelated lifecycle
event happened to reconcile again.
Now each departure path reconciles the registration explicitly:
GameStore.deleteGame signals AccountPushCoordinator through a new
onPushRegistrationMayNeedRefresh callback, and the sync engine's
game-removed and access-revoked handlers reconcile once their local
clean-up has settled. PushClient.unregister also throws instead of
swallowing the failure, so a failed DELETE leaves lastRegistered
unadvanced and the next reconcile retries the removal rather than
treating the old address as already gone.
Because reconciles can now overlap more often, AccountPushCoordinator
stamps each run with a generation and discards its address snapshot when
a newer reconcile started while it was suspended, so an out-of-order
resume cannot re-register an address the newer run had already dropped.
Co-Authored-By: Claude Fable 5 <[email protected]>
Diffstat:
7 files changed, 279 insertions(+), 17 deletions(-)
diff --git a/Crossmate.xcodeproj/project.pbxproj b/Crossmate.xcodeproj/project.pbxproj
@@ -173,6 +173,7 @@
C472EF02D8C7B0AC1D2284B8 /* CustomButtons.swift in Sources */ = {isa = PBXBuildFile; fileRef = F9B757D86362CD6F0500E9CB /* CustomButtons.swift */; };
C511387D9FFBCC2E2F5EF699 /* NotificationService.appex in Embed Foundation Extensions */ = {isa = PBXBuildFile; fileRef = 51318FC5DAE02D35CB005729 /* NotificationService.appex */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
C58F15CBEADA72032B54009D /* ReplayControlsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 847415468DBB1C566D18BC17 /* ReplayControlsTests.swift */; };
+ C6CC673F2D4BB8A1C0439FBB /* PushClientTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63AE7146492AD286EF13C3E7 /* PushClientTests.swift */; };
C843CADAA263CED503528A4E /* NYTBrowseView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3413F8755236FC0578AF8109 /* NYTBrowseView.swift */; };
C89A15D812E372FE1C56039B /* PUZToXDConverterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FDE193CAB325C991952D7CE5 /* PUZToXDConverterTests.swift */; };
C8ACF431021E7BEE61A99153 /* FriendController.swift in Sources */ = {isa = PBXBuildFile; fileRef = E655698481325C92EF5C348B /* FriendController.swift */; };
@@ -335,6 +336,7 @@
603E6FC55F1BD944592379D2 /* ReplayCacheTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReplayCacheTests.swift; sourceTree = "<group>"; };
605CA0FC7AF069CE3A3B38C1 /* RecentChanges.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RecentChanges.swift; sourceTree = "<group>"; };
60E818B0F4689BAD57660B7C /* GameCursorStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GameCursorStoreTests.swift; sourceTree = "<group>"; };
+ 63AE7146492AD286EF13C3E7 /* PushClientTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PushClientTests.swift; sourceTree = "<group>"; };
64C8064F04FC6177D987ACA2 /* Puzzle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Puzzle.swift; sourceTree = "<group>"; };
663934F0B7CA8BDD462DFAA4 /* GameShareItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GameShareItem.swift; sourceTree = "<group>"; };
67CFF96D54D2DE9C44EB120A /* EngagementCoordinatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EngagementCoordinatorTests.swift; sourceTree = "<group>"; };
@@ -585,6 +587,7 @@
1813630FA05C194AFF43855C /* PlayerRosterTests.swift */,
FF159746D076E051C2CB590C /* PlayerSelectionPublisherTests.swift */,
46801B570FC0B2C791ECDED3 /* PlayerSessionNavigationTests.swift */,
+ 63AE7146492AD286EF13C3E7 /* PushClientTests.swift */,
6C793B5B9684D49328E42129 /* PushPayloadCipherTests.swift */,
E87E28DC9402A4369647DE50 /* PushPayloadTests.swift */,
FDE193CAB325C991952D7CE5 /* PUZToXDConverterTests.swift */,
@@ -1033,6 +1036,7 @@
04062BCD473ED244159B1066 /* PlayerRosterTests.swift in Sources */,
309457EC2DFEC476253D54D2 /* PlayerSelectionPublisherTests.swift in Sources */,
00F2108848ADC7B4BF3AA0AE /* PlayerSessionNavigationTests.swift in Sources */,
+ C6CC673F2D4BB8A1C0439FBB /* PushClientTests.swift in Sources */,
2A273C98FE3AC5E4C9BE1D88 /* PushPayloadCipherTests.swift in Sources */,
A78FF09708EDED7ED50BB55B /* PushPayloadTests.swift in Sources */,
F34EDFD45E2F5006807DDAC7 /* PuzzleCatalogTests.swift in Sources */,
diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift
@@ -549,6 +549,8 @@ final class GameStore {
@ObservationIgnored
var onUnreadOtherMovesChanged: (() -> Void)?
@ObservationIgnored
+ var onPushRegistrationMayNeedRefresh: (@MainActor () -> Void)?
+ @ObservationIgnored
var onLocalCellEdit: (@MainActor (RealtimeCellEdit) -> Void)?
@ObservationIgnored
var onLocalCellEditBatch: (@MainActor ([RealtimeCellEdit]) -> Void)?
@@ -1303,6 +1305,7 @@ final class GameStore {
context.delete(entity)
try context.save()
onGameDeleted(deletion)
+ onPushRegistrationMayNeedRefresh?()
onUnreadOtherMovesChanged?()
}
diff --git a/Crossmate/Services/AccountPushCoordinator.swift b/Crossmate/Services/AccountPushCoordinator.swift
@@ -36,6 +36,12 @@ final class AccountPushCoordinator {
private var preferenceObservationTask: Task<Void, Never>?
private var preferenceDebounceTask: Task<Void, Never>?
private var lastAccountSeenReadAt: [UUID: Date] = [:]
+ /// Monotonic stamp for in-flight registration reconciles. A reconcile
+ /// snapshots store state, then suspends (player republish, friend-key
+ /// fetch) before handing the address set to the push client; overlapping
+ /// reconciles can therefore resume out of order, and an older snapshot
+ /// must not overwrite the fresher set a newer reconcile already pushed.
+ private var pushRegistrationGeneration = 0
init(
identity: AuthorIdentity,
@@ -169,6 +175,8 @@ final class AccountPushCoordinator {
private func updatePushRegistration(republishPlayerRows: Bool) async {
guard let pushClient, preferences.isICloudSyncEnabled else { return }
guard let authorID = identity.currentID, !authorID.isEmpty else { return }
+ pushRegistrationGeneration += 1
+ let generation = pushRegistrationGeneration
let accountAddress = ensureAccountPushAddress(authorID: authorID)
let secret = ensureAccountPushSecret(authorID: authorID)
let result = store.reconcileLocalPushAddresses(
@@ -187,6 +195,11 @@ final class AccountPushCoordinator {
var bindings = Set(result.bindings)
bindings.insert(PushAddressBinding(address: accountAddress))
bindings.formUnion(await friendEncryptionKeyBindings(localAuthorID: authorID))
+ // A newer reconcile started while this one was suspended; its snapshot
+ // supersedes this one, so let its setAddresses stand. The Player-row
+ // stamping and republish above still hold — they mutate real state
+ // rather than racing on the registration snapshot.
+ guard generation == pushRegistrationGeneration else { return }
pushClient.setAddresses(bindings)
}
diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift
@@ -548,7 +548,7 @@ final class AppServices {
pushClient: self.pushClient
)
- self.accountPush = AccountPushCoordinator(
+ let accountPush = AccountPushCoordinator(
identity: identity,
preferences: preferences,
persistence: persistence,
@@ -557,6 +557,13 @@ final class AppServices {
syncMonitor: self.syncMonitor,
pushClient: self.pushClient
)
+ self.accountPush = accountPush
+ store.onPushRegistrationMayNeedRefresh = { [weak accountPush] in
+ guard let accountPush else { return }
+ Task { @MainActor in
+ await accountPush.reconcilePushRegistration()
+ }
+ }
self.replays = ReplayLoader(
store: store,
@@ -936,7 +943,7 @@ final class AppServices {
await self.accountPush.reconcilePushRegistration()
}
- await syncEngine.setOnGameAccessRevoked { [store, gameViewedStore, announcements, gameArchiver] gameID in
+ await syncEngine.setOnGameAccessRevoked { [weak self, store, gameViewedStore, announcements, gameArchiver] gameID in
store.markAccessRevoked(gameID: gameID)
// Supersede any pending catch-up banner: advancing the view baseline
// to now leaves nothing for the next open to diff against.
@@ -950,6 +957,7 @@ final class AppServices {
// private-zone archive (and from the still-present local data);
// in-progress games are left as revoked rows.
await gameArchiver.promoteRevoked(gameID: gameID)
+ await self?.accountPush.reconcilePushRegistration()
}
await syncEngine.setOnGameRemoved { [weak self, store, gameViewedStore, announcements] gameID in
@@ -966,6 +974,7 @@ final class AppServices {
if wasOpen {
announcements.post(.gameRemoved(gameID: gameID))
}
+ await self?.accountPush.reconcilePushRegistration()
}
await syncEngine.setOnGameCompleted { [weak self] gameID in
diff --git a/Crossmate/Services/PushClient.swift b/Crossmate/Services/PushClient.swift
@@ -19,6 +19,7 @@ final class PushClient {
private let session: URLSession
private let log: (String) -> Void
private let authenticator: PushRequestAuthenticator
+ private let authorizationHeaders: (String, String, Data) async throws -> [String: String]
private var apnsToken: String?
private var authorID: String?
@@ -111,11 +112,15 @@ final class PushClient {
self.deviceID = deviceID
self.session = session
self.log = log
- self.authenticator = PushRequestAuthenticator(
+ let authenticator = PushRequestAuthenticator(
baseURL: base,
deviceID: deviceID,
session: session
)
+ self.authenticator = authenticator
+ self.authorizationHeaders = { method, path, body in
+ try await authenticator.signedHeaders(method: method, path: path, body: body)
+ }
// CrossmateAPSEnvironment carries the same APS_ENVIRONMENT build
// setting that fills the aps-environment entitlement, so the
// environment registered with the worker always matches the
@@ -131,6 +136,28 @@ final class PushClient {
}
}
+ init(
+ baseURL: URL,
+ environment: Environment,
+ deviceID: String = RecordSerializer.localDeviceID,
+ session: URLSession,
+ log: @escaping (String) -> Void = { _ in },
+ authorizationHeaders: @escaping (String, String, Data) async throws -> [String: String]
+ ) {
+ self.baseURL = baseURL
+ self.deviceID = deviceID
+ self.environment = environment
+ self.session = session
+ self.log = log
+ let authenticator = PushRequestAuthenticator(
+ baseURL: baseURL,
+ deviceID: deviceID,
+ session: session
+ )
+ self.authenticator = authenticator
+ self.authorizationHeaders = authorizationHeaders
+ }
+
func updateAPNsToken(_ data: Data) {
let hex = data.map { String(format: "%02x", $0) }.joined()
if hex == apnsToken { return }
@@ -138,6 +165,14 @@ final class PushClient {
Task { await reconcile() }
}
+ func testingSetAPNsToken(_ data: Data) {
+ apnsToken = data.map { String(format: "%02x", $0) }.joined()
+ }
+
+ func testingReconcile() async {
+ await reconcile()
+ }
+
/// Records the current account identity. Used only as the `fromAuthorID`
/// display field on outgoing pushes — the worker no longer keys anything
/// on identity, so an account switch is reflected purely through the
@@ -158,6 +193,10 @@ final class PushClient {
Task { await reconcile() }
}
+ func testingSetAddresses(_ next: Set<PushAddressBinding>) {
+ bindings = next
+ }
+
/// Sets the push kinds this device's notification settings have muted.
/// The caller (AccountPushCoordinator) mirrors this from preferences on
/// launch and on every settings change.
@@ -191,7 +230,7 @@ final class PushClient {
)
}
if !toRemove.isEmpty {
- await unregister(bindings: Array(toRemove))
+ try await unregister(bindings: Array(toRemove))
}
lastRegistered = signature
} catch {
@@ -445,7 +484,7 @@ final class PushClient {
try assert204(response)
}
- private func unregister(bindings: [PushAddressBinding]) async {
+ private func unregister(bindings: [PushAddressBinding]) async throws {
var request = URLRequest(url: baseURL.appendingPathComponent("register"))
request.httpMethod = "DELETE"
let data = (try? JSONSerialization.data(withJSONObject: [
@@ -453,16 +492,17 @@ final class PushClient {
"addresses": addressPayload(for: bindings)
])) ?? Data()
request.httpBody = data
+ let (_, response) = try await sendAuthorized(
+ request,
+ method: "DELETE",
+ path: "/register",
+ body: data
+ )
do {
- let (_, response) = try await sendAuthorized(
- request,
- method: "DELETE",
- path: "/register",
- body: data
- )
try assert204(response)
} catch {
- log("Push unregister failed: \(error.localizedDescription)")
+ log("Push unregister failed: HTTP \(response.statusCode)")
+ throw error
}
}
@@ -568,11 +608,7 @@ final class PushClient {
) async throws -> (Data, HTTPURLResponse) {
var request = request
request.setValue("application/json", forHTTPHeaderField: "Content-Type")
- let headers = try await authenticator.signedHeaders(
- method: method,
- path: path,
- body: body
- )
+ let headers = try await authorizationHeaders(method, path, body)
for (key, value) in headers {
request.setValue(value, forHTTPHeaderField: key)
}
diff --git a/Tests/Unit/GameStorePushAddressTests.swift b/Tests/Unit/GameStorePushAddressTests.swift
@@ -310,4 +310,19 @@ struct GameStorePushAddressTests {
#expect(store.ensurePushCredentials(for: localOnly) == nil)
#expect(store.ensurePushCredentials(for: UUID()) == nil)
}
+
+ @Test("deleteGame requests a push registration refresh")
+ func deleteGameRequestsPushRegistrationRefresh() throws {
+ let persistence = makeTestPersistence()
+ let store = makeTestStore(persistence: persistence)
+ let gameID = try makeGame(scope: 1, in: persistence.viewContext)
+ var refreshCount = 0
+ store.onPushRegistrationMayNeedRefresh = {
+ refreshCount += 1
+ }
+
+ try store.deleteGame(id: gameID)
+
+ #expect(refreshCount == 1)
+ }
}
diff --git a/Tests/Unit/PushClientTests.swift b/Tests/Unit/PushClientTests.swift
@@ -0,0 +1,182 @@
+import Foundation
+import Testing
+
+@testable import Crossmate
+
+@Suite("PushClient registration", .serialized)
+@MainActor
+struct PushClientTests {
+ @Test("failed unregister is retried on the next reconcile")
+ func failedUnregisterRetries() async throws {
+ PushClientURLProtocol.reset()
+ PushClientURLProtocol.setHandler { request, deleteAttempt in
+ let status: Int
+ if request.httpMethod == "DELETE" {
+ status = deleteAttempt == 1 ? 500 : 204
+ } else {
+ status = 204
+ }
+ return HTTPURLResponse(
+ url: request.url!,
+ statusCode: status,
+ httpVersion: nil,
+ headerFields: nil
+ )!
+ }
+ defer { PushClientURLProtocol.reset() }
+
+ let client = makeClient()
+ let old = PushAddressBinding(address: "old")
+ let new = PushAddressBinding(address: "new")
+
+ client.testingSetAPNsToken(Data([0x01, 0x02, 0x03]))
+ client.testingSetAddresses([old])
+ await client.testingReconcile()
+ PushClientURLProtocol.resetRequests()
+
+ client.testingSetAddresses([new])
+ await client.testingReconcile()
+
+ #expect(PushClientURLProtocol.deleteAttempts == 1)
+
+ PushClientURLProtocol.resetRequests()
+ await client.testingReconcile()
+
+ #expect(PushClientURLProtocol.deleteAttempts == 1)
+ }
+
+ @Test("successful unregister advances registration state")
+ func successfulUnregisterAdvancesState() async throws {
+ PushClientURLProtocol.reset()
+ PushClientURLProtocol.setHandler { request, _ in
+ HTTPURLResponse(
+ url: request.url!,
+ statusCode: 204,
+ httpVersion: nil,
+ headerFields: nil
+ )!
+ }
+ defer { PushClientURLProtocol.reset() }
+
+ let client = makeClient()
+ let old = PushAddressBinding(address: "old")
+ let new = PushAddressBinding(address: "new")
+
+ client.testingSetAPNsToken(Data([0x01, 0x02, 0x03]))
+ client.testingSetAddresses([old])
+ await client.testingReconcile()
+ PushClientURLProtocol.resetRequests()
+
+ client.testingSetAddresses([new])
+ await client.testingReconcile()
+
+ #expect(PushClientURLProtocol.deleteAttempts == 1)
+
+ PushClientURLProtocol.resetRequests()
+ await client.testingReconcile()
+
+ #expect(PushClientURLProtocol.deleteAttempts == 0)
+ }
+
+ private func makeClient() -> PushClient {
+ let config = URLSessionConfiguration.ephemeral
+ config.protocolClasses = [PushClientURLProtocol.self]
+ let session = URLSession(configuration: config)
+ return PushClient(
+ baseURL: URL(string: "https://push.test")!,
+ environment: .sandbox,
+ deviceID: "device",
+ session: session,
+ authorizationHeaders: { _, _, _ in [:] }
+ )
+ }
+}
+
+private final class PushClientURLProtocol: URLProtocol {
+ private static let state = PushClientURLProtocolState()
+
+ static var deleteAttempts: Int {
+ state.deleteAttempts
+ }
+
+ static func reset() {
+ state.reset()
+ }
+
+ static func resetRequests() {
+ state.resetRequests()
+ }
+
+ static func setHandler(_ handler: @escaping @Sendable (URLRequest, Int) -> HTTPURLResponse) {
+ state.setHandler(handler)
+ }
+
+ override class func canInit(with request: URLRequest) -> Bool {
+ true
+ }
+
+ override class func canonicalRequest(for request: URLRequest) -> URLRequest {
+ request
+ }
+
+ override func startLoading() {
+ let response = Self.state.response(for: request)
+ client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed)
+ client?.urlProtocol(self, didLoad: Data())
+ client?.urlProtocolDidFinishLoading(self)
+ }
+
+ override func stopLoading() {}
+}
+
+private final class PushClientURLProtocolState: @unchecked Sendable {
+ private let lock = NSLock()
+ private var handler: (@Sendable (URLRequest, Int) -> HTTPURLResponse)?
+ private var requestDeleteAttempts = 0
+
+ var deleteAttempts: Int {
+ lock.lock()
+ defer { lock.unlock() }
+ return requestDeleteAttempts
+ }
+
+ func reset() {
+ lock.lock()
+ handler = nil
+ requestDeleteAttempts = 0
+ lock.unlock()
+ }
+
+ func resetRequests() {
+ lock.lock()
+ requestDeleteAttempts = 0
+ lock.unlock()
+ }
+
+ func setHandler(_ next: @escaping @Sendable (URLRequest, Int) -> HTTPURLResponse) {
+ lock.lock()
+ handler = next
+ lock.unlock()
+ }
+
+ func response(for request: URLRequest) -> HTTPURLResponse {
+ let handler: (@Sendable (URLRequest, Int) -> HTTPURLResponse)?
+ let deleteAttempt: Int
+ lock.lock()
+ if request.httpMethod == "DELETE" {
+ requestDeleteAttempts += 1
+ deleteAttempt = requestDeleteAttempts
+ } else {
+ deleteAttempt = 0
+ }
+ handler = self.handler
+ lock.unlock()
+
+ return handler?(request, deleteAttempt) ?? HTTPURLResponse(
+ url: request.url!,
+ statusCode: 204,
+ httpVersion: nil,
+ headerFields: nil
+ )!
+ }
+}
