From ae2fee2d4c960f1b95f5d9ffb6400baa8f9af319 Mon Sep 17 00:00:00 2001 From: Arkadiusz Fal Date: Wed, 26 Aug 2026 22:59:52 +0200 Subject: [PATCH] Make a second login on an instance reachable and orderable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An additional login could be created but never selected: the legacy picker lists one entry per instance and always resolved to whichever account signed in first. Account rows gain a "Use This Account" action, and the subscriptions view reloads on the data context rather than on the legacy setting — two logins on the same instance map onto the same legacy value, so switching between them changed nothing on screen while writes went to the other account. Two devices migrating different legacy selections produced contexts with identical epoch timestamps, and each refused the other's: a permanent, silent divergence that both sides considered settled. Equal timestamps are now broken by the selected account's ID, on the incoming side and in the conflict resolver alike — a record that wins the upload must not be refused on arrival. Credential cleanup falls back to both managers whenever the backend cannot be resolved, not only when the account is unknown here: an instance deletion can arrive before the tombstone, and the credential was then left in the Keychain. --- Yattee/Core/AccountsManager.swift | 4 ++-- Yattee/Core/AppEnvironment.swift | 20 ++++++++--------- Yattee/Localizable.xcstrings | 10 +++++++++ Yattee/Models/DataContext.swift | 20 +++++++++++++++++ .../CloudKit/CloudKitConflictResolver.swift | 19 ++++++++++++++-- .../Settings/InstanceAccountsSection.swift | 17 ++++++++++++++ .../Subscriptions/SubscriptionsView.swift | 8 +++++-- YatteeTests/AccountSyncTests.swift | 22 +++++++++++++++++++ 8 files changed, 104 insertions(+), 16 deletions(-) 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()