diff --git a/Yattee/Core/AccountsManager.swift b/Yattee/Core/AccountsManager.swift index 36dd21c1..f699c92d 100644 --- a/Yattee/Core/AccountsManager.swift +++ b/Yattee/Core/AccountsManager.swift @@ -478,8 +478,8 @@ final class AccountsManager { return false } - guard incoming.changedAt > storedContext.changedAt else { - guard storedContext.changedAt > incoming.changedAt else { return false } + guard storedContext.isOutranked(by: incoming) else { + guard incoming.isOutranked(by: storedContext) else { return false } pendingContextUpload = true save() return true diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index c37f8ea1..b753b19e 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -417,21 +417,21 @@ final class AppEnvironment { accountSync.onAccountDeleted = { [weak self] accountID, account in guard let self else { return } - guard let account else { - // No metadata for this account here, so its backend is unknown - // and the right credential manager cannot be picked. The scoped - // key is the account ID either way, so both are asked to drop - // it; at most one of them holds anything. The unscoped legacy - // item is left alone — deleting it needs the instance, and its - // ownership check is bound to one. + // The backend is unknown whenever the account itself never arrived + // here, and equally when its instance is already gone — an instance + // deletion can reach this device before the tombstone does. The + // scoped key is the account ID either way, so both managers are + // asked to drop it; at most one of them holds anything. The + // unscoped legacy item is left alone in that case: deleting it + // needs the instance, and its ownership check is bound to one. + guard let account, let credentials = self.accountCredentialsManager(for: account) else { self.invidiousCredentialsManager.deleteCredential(forAccountID: accountID) self.pipedCredentialsManager.deleteCredential(forAccountID: accountID) return } - let credentials = self.accountCredentialsManager(for: account) - credentials?.deleteCredential(for: account) - credentials?.deleteLegacyCredentialIfOwned(for: account) + credentials.deleteCredential(for: account) + credentials.deleteLegacyCredentialIfOwned(for: account) } // Statuses are cleared whenever the identity namespace reloads, so they diff --git a/Yattee/Localizable.xcstrings b/Yattee/Localizable.xcstrings index 8b80b619..92bac8ec 100644 --- a/Yattee/Localizable.xcstrings +++ b/Yattee/Localizable.xcstrings @@ -233,6 +233,16 @@ } } }, + "accounts.useThisAccount" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Use This Account" + } + } + } + }, "accounts.useSavedLogin" : { "localizations" : { "en" : { diff --git a/Yattee/Models/DataContext.swift b/Yattee/Models/DataContext.swift index a6c83214..27c7e4e1 100644 --- a/Yattee/Models/DataContext.swift +++ b/Yattee/Models/DataContext.swift @@ -24,6 +24,16 @@ enum DataContext: Codable, Hashable, Sendable { } var isLocal: Bool { self == .local } + + /// Total order used **only** to break a tie between two contexts chosen at + /// the same instant. + /// + /// Migrated contexts are all deliberately stamped at the epoch, so two + /// devices migrating different legacy selections produce genuinely equal + /// timestamps. Without a tie-break each would keep its own and consider the + /// matter settled — a permanent, silent divergence. The value is arbitrary; + /// what matters is that every device computes the same one. + var tieBreakKey: String { accountID?.uuidString ?? "" } } /// A context together with the moment it was chosen. @@ -51,6 +61,16 @@ struct StoredDataContext: Codable, Hashable, Sendable { } var isMigrated: Bool { changedAt == Date(timeIntervalSince1970: 0) } + + /// Whether `other` should replace this selection. + /// + /// Ordered by the app-managed timestamp, with `DataContext.tieBreakKey` + /// settling an exact tie so that every device converges on the same answer + /// rather than each keeping its own. + func isOutranked(by other: StoredDataContext) -> Bool { + if other.changedAt != changedAt { return other.changedAt > changedAt } + return other.context.tieBreakKey > context.tieBreakKey + } } // MARK: - Legacy migration diff --git a/Yattee/Services/CloudKit/CloudKitConflictResolver.swift b/Yattee/Services/CloudKit/CloudKitConflictResolver.swift index f0f4dcbc..540f4079 100644 --- a/Yattee/Services/CloudKit/CloudKitConflictResolver.swift +++ b/Yattee/Services/CloudKit/CloudKitConflictResolver.swift @@ -367,7 +367,13 @@ actor CloudKitConflictResolver { /// Resolves a conflict between local and server data-context records. /// /// Strategy: last write wins on the app-managed `changedAt` — the moment - /// the user chose, not the moment a record happened to be written. + /// the user chose, not the moment a record happened to be written — with + /// an exact tie settled by the selected account's ID. + /// + /// The tie is not hypothetical: every migrated context is stamped at the + /// epoch, so two devices migrating different legacy selections produce + /// identical timestamps. This has to match `StoredDataContext.isOutranked`, + /// or a record could win the upload and still be refused on arrival. func resolveDataContextConflict( local: CKRecord, server: CKRecord @@ -375,9 +381,18 @@ actor CloudKitConflictResolver { let localChanged = (local["changedAt"] as? Date) ?? Date.distantPast let serverChanged = (server["changedAt"] as? Date) ?? Date.distantPast + let localWins: Bool + if localChanged != serverChanged { + localWins = localChanged > serverChanged + } else { + let localKey = (local["accountID"] as? String) ?? "" + let serverKey = (server["accountID"] as? String) ?? "" + localWins = localKey > serverKey + } + let resolved = server - if localChanged > serverChanged { + if localWins { resolved["contextKind"] = local["contextKind"] resolved["accountID"] = local["accountID"] resolved["changedAt"] = local["changedAt"] diff --git a/Yattee/Views/Settings/InstanceAccountsSection.swift b/Yattee/Views/Settings/InstanceAccountsSection.swift index 0a4eda07..fdb2dc50 100644 --- a/Yattee/Views/Settings/InstanceAccountsSection.swift +++ b/Yattee/Views/Settings/InstanceAccountsSection.swift @@ -72,6 +72,19 @@ struct InstanceAccountsSection: View { } .frame(maxWidth: .infinity, alignment: .leading) .contextMenu { + // Without this a second login on an instance cannot be reached: the + // legacy picker lists one entry per instance, so it always resolves + // to whichever account was signed in first. The switcher that makes + // this a first-class choice everywhere comes later; until then this + // is what makes a second login usable at all. + if !isCurrentContext(account) { + Button { + appEnvironment?.accountsManager.setContext(.account(account.id)) + } label: { + Label(String(localized: "accounts.useThisAccount"), systemImage: "checkmark.circle") + } + } + Button { loginTarget = .existing(account) } label: { @@ -152,6 +165,10 @@ struct InstanceAccountsSection: View { } } + private func isCurrentContext(_ account: Account) -> Bool { + appEnvironment?.accountsManager.currentContext == .account(account.id) + } + private func statusColor(for account: Account) -> Color { switch status(for: account) { case .signedIn: .secondary diff --git a/Yattee/Views/Subscriptions/SubscriptionsView.swift b/Yattee/Views/Subscriptions/SubscriptionsView.swift index d765d831..50ac60e7 100644 --- a/Yattee/Views/Subscriptions/SubscriptionsView.swift +++ b/Yattee/Views/Subscriptions/SubscriptionsView.swift @@ -502,8 +502,12 @@ struct SubscriptionsView: View { .onReceive(NotificationCenter.default.publisher(for: .watchHistoryDidChange)) { _ in loadWatchEntries() } - .onChange(of: appEnvironment?.settingsManager.subscriptionAccount) { _, _ in - // Clear cache and refresh when subscription account changes + // The data context, not the legacy setting: two logins on + // the same instance map onto the same legacy value, so + // switching between them would change nothing here and this + // view would keep showing the first account's subscriptions + // while writes went to the second. + .onReceive(NotificationCenter.default.publisher(for: .dataContextDidChange)) { _ in feedCache.handleAccountChange() subscriptions = [] subscriptionsLoaded = false diff --git a/YatteeTests/AccountSyncTests.swift b/YatteeTests/AccountSyncTests.swift index a9280ad0..b49ca4dc 100644 --- a/YatteeTests/AccountSyncTests.swift +++ b/YatteeTests/AccountSyncTests.swift @@ -267,6 +267,28 @@ struct AccountSyncTests { #expect(try mapper.toStoredDataContext(from: serverWins).context == .account(accountID)) } + @Test("Two migrated contexts converge instead of diverging") + func equalTimestampsBreakTheTieTheSameWayEverywhere() async throws { + let mapper = AccountSyncTests.makeMapper() + let resolver = CloudKitConflictResolver() + + // Every migrated context is stamped at the epoch, so two devices + // migrating different legacy selections tie exactly. + let lower = StoredDataContext.migrated(.account(UUID(uuidString: "00000000-0000-0000-0000-000000000001")!)) + let higher = StoredDataContext.migrated(.account(UUID(uuidString: "FFFFFFFF-0000-0000-0000-000000000001")!)) + + #expect(lower.isOutranked(by: higher)) + #expect(!higher.isOutranked(by: lower)) + + // The upload side has to reach the same verdict, or a record could + // win the conflict and then be refused when it arrives. + let resolved = await resolver.resolveDataContextConflict( + local: mapper.toCKRecord(dataContext: higher), + server: mapper.toCKRecord(dataContext: lower) + ) + #expect(try mapper.toStoredDataContext(from: resolved).context == higher.context) + } + @Test("Write-once records resolve to the server copy") func writeOnceResolvesToServer() async { let mapper = AccountSyncTests.makeMapper()