Settle account ties and stop discarding caches on every launch

Two devices editing an account in the same instant — or migrating one,
since every migrated account is stamped at the epoch — each kept its own
label and considered the matter closed. Equal timestamps are now broken
by the label, on the incoming side and in the conflict resolver alike,
the same rule the context already uses.

Cached feeds were thrown away on every ordinary launch: recomputing
statuses posts a notification, and the observer ran before the value it
compares against had been seeded. The seed now happens first.

Cache ownership is checked once at launch and compared against a
recorded owner afterwards. An unresolved identity at launch means the
owner cannot be established at all — a switch to an unseen Apple account
while offline looks exactly like that — so the caches are discarded;
mid-session it is a transient, and with iCloud Drive off it happens on
every activation, so it no longer clears anything.

Whether a Keychain item syncs is fixed when it is written, so a login
stored while sync was off stayed on this device while its account record
travelled, leaving every other device with an account whose credential
never arrives. Turning sync on rewrites them.

Reseeding a deleted zone marks the account records dirty even while the
category is off, so re-enabling it restores them instead of finding an
empty outbox.
This commit is contained in:
Arkadiusz Fal
2026-08-26 23:53:29 +02:00
parent d9244cf0d7
commit b37ca40d37
11 changed files with 158 additions and 43 deletions

View File

@@ -344,8 +344,8 @@ final class AccountsManager {
if let index = accounts.firstIndex(where: { $0.id == incoming.id }) {
let existing = accounts[index]
guard incoming.updatedAt > existing.updatedAt else {
guard existing.updatedAt > incoming.updatedAt else { return false }
guard existing.isOutranked(by: incoming) else {
guard incoming.isOutranked(by: existing) else { return false }
// The server holds the older copy. Marked dirty rather than
// merely re-registered with the engine, so the correction
// survives a crash before it is sent.

View File

@@ -389,28 +389,6 @@ final class AppEnvironment {
// is global rather than account-keyed, so it would otherwise keep
// serving the previous account's videos.
//
// Accounts and instances arriving are triggers too, not redundant ones:
// a context naming an account that has not synced yet — or one whose
// instance has not — cannot be mapped onto the legacy shape, and these
// are what restore the mapping once it can be made rather than leaving
// that moment permanent. The same arrivals are what make a synced
// account's credential findable, so statuses are recomputed with them:
// an account that appears mid-session would otherwise sit at "sign in
// required" until the app was relaunched.
for name: Notification.Name in [.dataContextDidChange, .accountsDidChange, .instancesDidChange] {
contextMirrorObservers.append(
NotificationCenter.default.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in
MainActor.assumeIsolated {
guard let self else { return }
if name != .dataContextDidChange {
self.refreshAccountStatuses()
}
self.mirrorContextToLegacySelection()
}
}
)
}
// A deletion that arrives from another device takes the credential with
// it. Only a deletion does this — an account merely missing from a
// fetch keeps everything, because a fetch is not an enumeration.
@@ -450,19 +428,16 @@ final class AppEnvironment {
guard let self else { return }
self.refreshAccountStatuses()
// Cleared unconditionally, because the identity moving means
// everything cached belongs to the Apple account that just went
// away — even when the new namespace happens to select the same
// context and the change comparison would find nothing.
self.clearAccountScopedCaches()
if let owner = self.credentialIdentityScope.current {
UserDefaults.standard.set(owner, forKey: Self.cacheOwnerKey)
}
// Compared against the recorded owner rather than cleared
// outright: a scope that merely dropped to unresolved and came
// back is the same Apple account, and with iCloud Drive off
// that happens on every activation.
self.discardCachesFromAnotherIdentityIfNeeded()
self.mirrorContextToLegacySelection(resetWhenUnresolvable: true)
}
}
discardCachesFromAnotherIdentityIfNeeded()
discardCachesFromAnotherIdentityIfNeeded(atLaunch: true)
// Turn any pre-revamp single-instance logins into accounts. Repeatable:
// it runs again whenever instances arrive from iCloud.
@@ -474,6 +449,28 @@ final class AppEnvironment {
// not look like a change and throw away caches that are still right.
lastCachedContext = dataContextValidator.effectiveContext
// Accounts and instances arriving are triggers too, not redundant ones:
// a context naming an account that has not synced yet — or one whose
// instance has not — cannot be mapped onto the legacy shape, and these
// are what restore the mapping once it can be made rather than leaving
// that moment permanent. The same arrivals are what make a synced
// account's credential findable, so statuses are recomputed with them:
// an account that appears mid-session would otherwise sit at "sign in
// required" until the app was relaunched.
for name: Notification.Name in [.dataContextDidChange, .accountsDidChange, .instancesDidChange] {
contextMirrorObservers.append(
NotificationCenter.default.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in
MainActor.assumeIsolated {
guard let self else { return }
if name != .dataContextDidChange {
self.refreshAccountStatuses()
}
self.mirrorContextToLegacySelection()
}
}
)
}
// Resolve the credential identity scope if this Apple account has never
// been seen on this device. Until it lands, account credentials report
// "can't tell yet" rather than "signed out".
@@ -663,12 +660,28 @@ final class AppEnvironment {
/// being constructed, which is before any observer here exists. So an Apple
/// account switched while the app was closed would otherwise reach the new
/// user with the previous one's feeds still on disk.
private func discardCachesFromAnotherIdentityIfNeeded() {
// Unresolved: no conclusion either way, and the observer covers it once
// the scope lands.
guard let owner = credentialIdentityScope.current else { return }
/// - Parameter atLaunch: whether this is the startup check. It decides what
/// an *unresolved* identity means. At launch it means the cached data's
/// owner cannot be confirmed at all — which is exactly the state a switch
/// to an unseen Apple account while offline produces — so the safe answer
/// is to discard. Mid-session it is a transient: with iCloud Drive off the
/// scope drops to unresolved and re-resolves on every activation, and
/// clearing on that would throw the caches away several times a day.
private func discardCachesFromAnotherIdentityIfNeeded(atLaunch: Bool = false) {
let defaults = UserDefaults.standard
guard let owner = credentialIdentityScope.current else {
guard atLaunch, defaults.string(forKey: Self.cacheOwnerKey) != nil else { return }
clearAccountScopedCaches()
defaults.removeObject(forKey: Self.cacheOwnerKey)
LoggingService.shared.info(
"Cannot confirm who the cached account data belongs to — discarded it",
category: .general
)
return
}
let previous = defaults.string(forKey: Self.cacheOwnerKey)
defaults.set(owner, forKey: Self.cacheOwnerKey)
@@ -676,7 +689,7 @@ final class AppEnvironment {
clearAccountScopedCaches()
LoggingService.shared.info(
"Apple account changed while the app was closed — discarded cached account data",
"Apple account changed — discarded the previous account's cached data",
category: .general
)
}

View File

@@ -71,4 +71,16 @@ struct Account: Identifiable, Codable, Hashable, Sendable {
/// Whether this account was produced by the legacy migration rather than an
/// explicit sign-in.
var isMigrated: Bool { id == instanceID }
/// Whether `other` should replace this copy.
///
/// Ordered by the app-managed `updatedAt`, with the label settling an exact
/// tie. The tie is reachable — two devices can migrate the same account in
/// the same instant, and every migrated account is stamped at the epoch — and
/// without a tie-break each device would keep its own label and consider the
/// matter closed, which is a divergence neither would ever notice.
func isOutranked(by other: Account) -> Bool {
if other.updatedAt != updatedAt { return other.updatedAt > updatedAt }
return (other.displayName ?? "") > (displayName ?? "")
}
}

View File

@@ -342,8 +342,11 @@ actor CloudKitConflictResolver {
/// modification date: an edit made offline has to stay orderable against
/// a concurrent edit elsewhere, and a migrated account carries an
/// epoch-old date precisely so any real edit outranks it.
/// - `instanceID` and `dateAdded` are immutable identity, taken from
/// whichever side wins only so a malformed record cannot strip them.
/// - An exact tie is settled by the label, so two devices that edited in the
/// same instant — or migrated the same account, all of which are stamped
/// at the epoch — converge instead of each keeping its own. This has to
/// match `Account.isOutranked`, or a record could win the upload and still
/// be refused on arrival.
func resolveAccountConflict(
local: CKRecord,
server: CKRecord
@@ -351,10 +354,17 @@ actor CloudKitConflictResolver {
let localUpdated = (local["updatedAt"] as? Date) ?? Date.distantPast
let serverUpdated = (server["updatedAt"] as? Date) ?? Date.distantPast
let localWins: Bool
if localUpdated != serverUpdated {
localWins = localUpdated > serverUpdated
} else {
localWins = ((local["displayName"] as? String) ?? "") > ((server["displayName"] as? String) ?? "")
}
// IMPORTANT: Always start with server record to preserve recordChangeTag
let resolved = server
if localUpdated > serverUpdated {
if localWins {
resolved["displayName"] = local["displayName"]
resolved["updatedAt"] = local["updatedAt"]
}

View File

@@ -1565,8 +1565,11 @@ final class CloudKitSyncEngine: @unchecked Sendable {
/// recreating the zone after it was deleted elsewhere. The account outbox
/// normally tracks only what changed, which is not enough when the server
/// has nothing at all.
/// Deliberately not gated on the category being enabled: the zone can be
/// deleted while account sync is off, and marking the records dirty is what
/// lets re-enabling restore them. The outbox is durable and nothing is sent
/// while the category is off, so priming it costs nothing.
func uploadAllAccounts() {
guard canSyncAccounts else { return }
uploadProgress?.currentCategory = "Accounts"
accountSync?.uploadEverything()
}

View File

@@ -41,6 +41,12 @@ protocol AccountCredentialsManager: AnyObject, Observable {
/// key needs only the account's ID.
func deleteCredential(forAccountID accountID: UUID)
/// Rewrites stored credentials so they carry the current iCloud sync
/// setting. Whether a Keychain item syncs is decided when it is written, so
/// enabling sync later would otherwise leave existing logins local — and
/// every other device with an account whose credential never arrives.
func resyncCredentialsWithCurrentSetting()
/// The login identifier the user typed. Kept in the Keychain beside the
/// credential so it never enters synced metadata.
func setUsername(_ username: String, for account: Account)

View File

@@ -111,6 +111,35 @@ final class AccountKeychainCredentials {
HomeInstanceCache.shared.clear(instanceID: account.instanceID, contentType: .feed)
}
/// Rewrites the stored credentials so they carry the current sync setting.
///
/// `kSecAttrSynchronizable` is fixed when an item is written, so a login
/// stored while iCloud sync was off stays local forever. Enabling sync
/// afterwards uploads the account metadata, and every other device would
/// then show an account whose credential never arrives. Called when the
/// user turns sync on.
func resyncCredentialsWithCurrentSetting() {
guard let scope = identityScope.current else { return }
for accountID in loginsByScope[scope] ?? [] {
let credentialKey = KeychainCredentialStore.credentialKey(scope: scope, accountID: accountID)
let usernameKey = KeychainCredentialStore.usernameKey(scope: scope, accountID: accountID)
let syncEnabled = shouldSyncToiCloud
if let credential = store.value(forKey: credentialKey) {
_ = store.set(credential, forKey: credentialKey, synchronizable: syncEnabled)
}
if let username = store.value(forKey: usernameKey) {
_ = store.set(username, forKey: usernameKey, synchronizable: syncEnabled)
}
}
LoggingService.shared.info(
"Rewrote stored \(credentialNoun)s with the current iCloud sync setting",
category: .keychain
)
}
func deleteCredential(forAccountID accountID: UUID) {
guard let scope = currentScope() else { return }

View File

@@ -289,6 +289,10 @@ extension InvidiousCredentialsManager: AccountCredentialsManager {
accountCredentials.deleteCredential(forAccountID: accountID)
}
func resyncCredentialsWithCurrentSetting() {
accountCredentials.resyncCredentialsWithCurrentSetting()
}
func setUsername(_ username: String, for account: Account) {
accountCredentials.setUsername(username, for: account)
}

View File

@@ -218,6 +218,10 @@ extension PipedCredentialsManager: AccountCredentialsManager {
accountCredentials.deleteCredential(forAccountID: accountID)
}
func resyncCredentialsWithCurrentSetting() {
accountCredentials.resyncCredentialsWithCurrentSetting()
}
func setUsername(_ username: String, for account: Account) {
accountCredentials.setUsername(username, for: account)
}

View File

@@ -528,6 +528,18 @@ struct iCloudSettingsView: View {
settingsManager?.replaceWithiCloudData()
instancesManager?.replaceWithiCloudData()
mediaSourcesManager?.replaceWithiCloudData()
resyncStoredLogins()
}
/// Rewrites stored logins so they carry the sync setting that has just
/// changed. Whether a Keychain item syncs is fixed when it is written, so a
/// login stored while sync was off would stay on this device while its
/// account record travelled — every other device showing an account whose
/// credential never arrives.
private func resyncStoredLogins() {
appEnvironment?.invidiousCredentialsManager.resyncCredentialsWithCurrentSetting()
appEnvironment?.pipedCredentialsManager.resyncCredentialsWithCurrentSetting()
}
private func disableiCloudSync() {
@@ -547,6 +559,7 @@ struct iCloudSettingsView: View {
Task {
await appEnvironment?.accountSyncCoordinator.activate()
}
resyncStoredLogins()
case .subscriptions:
settingsManager?.syncSubscriptions = true
// Upload existing subscriptions to CloudKit

View File

@@ -267,6 +267,27 @@ struct AccountSyncTests {
#expect(try mapper.toStoredDataContext(from: serverWins).context == .account(accountID))
}
@Test("Two accounts edited in the same instant converge")
func equalAccountTimestampsBreakTheTieTheSameWayEverywhere() async throws {
let mapper = AccountSyncTests.makeMapper()
let resolver = CloudKitConflictResolver()
let id = UUID()
let instanceID = UUID()
let sameMoment = Date(timeIntervalSince1970: 500)
let lower = Account(id: id, instanceID: instanceID, displayName: "A", updatedAt: sameMoment)
let higher = Account(id: id, instanceID: instanceID, displayName: "B", updatedAt: sameMoment)
#expect(lower.isOutranked(by: higher))
#expect(!higher.isOutranked(by: lower))
let resolved = await resolver.resolveAccountConflict(
local: mapper.toCKRecord(account: higher),
server: mapper.toCKRecord(account: lower)
)
#expect(try mapper.toAccount(from: resolved).displayName == "B")
}
@Test("Two migrated contexts converge instead of diverging")
func equalTimestampsBreakTheTieTheSameWayEverywhere() async throws {
let mapper = AccountSyncTests.makeMapper()