Mirrored_Repos/yattee
1
0
Fork 0
mirror of https://github.com/yattee/yattee.git synced 2026-10-23 21:22:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

Stop a fresh login from synthesizing a phantom migrated account

Browse Source 
Signing in wrote `usesAccountLogin`, which syncs through key-value
storage independently of the account records. A device receiving that
flag before the account synthesized a migrated account for a login that
was never legacy — and the marker arriving later deliberately retracts
nothing, so the credential-less duplicate was permanent and uploaded.
The flag is pre-revamp state that migration only reads; accounts
describe logins now. `needsCredentials` reads the account list for any
instance that has one, so the badge it drives is unaffected.

A tombstone now dominates contexts as well as accounts: an offline
device can select an account another device deleted and publish that
choice with a newer timestamp, which would have left every device
pointing at an account that is never coming back.

Cached private feeds survived an Apple-account switch two ways. A scope
this device has seen before resolves while `CredentialIdentityScope` is
still being constructed, so the change notification fires before any
observer exists — startup now compares the persisted cache owner. And a
purge asked for before the disk cache is read was undone by the load,
which now carries it.

The need to re-bootstrap after dropping records for want of a resolved
identity is persisted. The engine may already have moved its token past
those records, so a launch that lost the in-memory flag would neither
replay nor bootstrap them.
This commit is contained in:
Arkadiusz Fal
2026-08-26 08:06:52 +02:00
parent ff78cc0121
commit 923e13565f
7 changed files with 183 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
Yattee/Core/AccountsManager.swift
View File

@@ -448,6 +448,24 @@ final class AccountsManager {
/// pushed back over the stale server copy.
@discardableResult
func applySyncedContext(_ incoming: StoredDataContext) -> Bool {
var incoming = incoming
// A tombstone dominates the account everywhere, contexts included. An
// offline device can select an account another device has deleted and
// publish that choice with a newer timestamp; adopting it would leave
// every device pointing at an account that is never coming back. The
// choice's own timestamp is kept, so a later real selection still wins,
// and every device that holds the tombstone repairs it identically.
if let accountID = incoming.context.accountID, deletedAccountIDs.contains(accountID) {
incoming = StoredDataContext(context: .local, changedAt: incoming.changedAt)
LoggingService.shared.info(
"Ignoring a synced context that names a deleted account",
category: .general,
details: "accountID=\(accountID)"
)
}
// Before the user has ever chosen here there is nothing to order
// against: the local value is a default stamped when this manager was
// constructed, so a synced choice made yesterday would lose to it and a

47
Yattee/Core/AppEnvironment.swift
View File

@@ -460,10 +460,15 @@ final class AppEnvironment {
// 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)
}
self.mirrorContextToLegacySelection(resetWhenUnresolvable: true)
}
}
discardCachesFromAnotherIdentityIfNeeded()
// Turn any pre-revamp single-instance logins into accounts. Repeatable:
// it runs again whenever instances arrive from iCloud.
accounts.reconcileLegacyAccounts()
@@ -650,6 +655,33 @@ final class AppEnvironment {
HomeInstanceCache.shared.clearAllFeeds()
}
private static let cacheOwnerKey = "accountScopedCacheOwner"
/// Discards cached private data left behind by a different Apple account.
///
/// The identity-change notification cannot cover this: a scope this device
/// has seen before resolves synchronously while `CredentialIdentityScope` is
/// 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 }
let defaults = UserDefaults.standard
let previous = defaults.string(forKey: Self.cacheOwnerKey)
defaults.set(owner, forKey: Self.cacheOwnerKey)
guard let previous, previous != owner else { return }
clearAccountScopedCaches()
LoggingService.shared.info(
"Apple account changed while the app was closed — discarded cached account data",
category: .general
)
}
/// Deletes an instance together with the accounts that live on it.
///
/// The cascade has to happen here, while the instance is still resolvable:
@@ -812,6 +844,21 @@ final class AppEnvironment {
return true
}
// Accounts describe the logins on an instance once there are any, so the
// pre-revamp flag is consulted only for instances that have none —
// fresh logins deliberately no longer set it.
let accounts = accountsManager.accounts(forInstance: instance.id)
if !accounts.isEmpty {
// An unresolved identity scope means "cannot tell yet". Reporting a
// missing credential on it would badge every account on every
// launch until the scope lands.
guard credentialIdentityScope.isResolved else { return false }
return !accounts.contains { account in
accountCredentialsManager(for: account)?.isLoggedIn(for: account) ?? false
}
}
if instance.usesAccountLogin,
let manager = credentialsManager(for: instance),
!manager.isLoggedIn(for: instance) {

59
Yattee/Services/CloudKit/AccountSyncCoordinator.swift
View File

@@ -78,10 +78,6 @@ final class AccountSyncCoordinator {
/// no identity was resolved.
private var lastResolvedNamespace: String?
/// Set when account records had to be dropped because no identity was
/// resolved yet. The engine's token has already moved past them, so the
/// identity that resolves next has to bootstrap again to recover them.
private var droppedRecordsWhileUnresolved = false
private var isActivating = false
private var activationRequested = false
@@ -182,29 +178,7 @@ final class AccountSyncCoordinator {
func identityDidChange() {
identityGeneration += 1
guard let current = namespace else { return }
let previouslyResolved = lastResolvedNamespace
lastResolvedNamespace = current
// Records dropped for want of a resolved identity belonged to whichever
// Apple account was signed in at the time, and that cannot be
// reconstructed afterwards: a first launch and a sign-out/sign-in look
// identical from here. Rather than guess, every identity that could have
// owned them bootstraps again and recovers its own from its own zone. A
// redundant full fetch is the whole cost of that; guessing wrongly would
// show one Apple account's logins under another's.
if droppedRecordsWhileUnresolved {
droppedRecordsWhileUnresolved = false
for candidate in Set([current, previouslyResolved].compactMap { $0 }) {
store.clearBootstrapCompletion(in: candidate)
}
LoggingService.shared.logCloudKit(
"Account records arrived before the identity resolved — bootstrapping again to recover them"
)
}
guard namespace != nil else { return }
Task { await activate() }
}
@@ -239,7 +213,7 @@ final class AccountSyncCoordinator {
// Staging under a guessed identity is the one thing worse than
// losing the records: they would be applied to, and uploaded from,
// an account they never belonged to.
droppedRecordsWhileUnresolved = true
store.hasPendingIdentityRecovery = true
LoggingService.shared.logCloudKit(
"Dropping \(accountRecords.count + accountDeletions.count) account records fetched before the identity resolved"
)
@@ -306,6 +280,9 @@ final class AccountSyncCoordinator {
return
}
recoverDroppedRecordsIfNeeded(in: namespace)
lastResolvedNamespace = namespace
await runBootstrapIfNeeded(in: namespace)
guard store.isBootstrapCompleted(in: namespace) else { return }
@@ -315,6 +292,32 @@ final class AccountSyncCoordinator {
enqueueDirtyRecords()
}
/// Makes every identity that could have owned dropped records fetch its zone
/// again.
///
/// Which identity they belonged to cannot be reconstructed — a first launch
/// and a sign-out followed by a different sign-in look identical from here —
/// so instead of guessing, each candidate recovers its own records from its
/// own zone. A redundant full fetch is the whole cost; guessing wrongly
/// would show one Apple account's logins under another's.
///
/// Only the identity signed in now and the last one seen in this process are
/// candidates. An identity that owned dropped records, is not signed in now,
/// and was last seen in an earlier process keeps its completed flag; nothing
/// here knows it exists.
private func recoverDroppedRecordsIfNeeded(in namespace: String) {
guard store.hasPendingIdentityRecovery else { return }
store.hasPendingIdentityRecovery = false
for candidate in Set([namespace, lastResolvedNamespace].compactMap { $0 }) {
store.clearBootstrapCompletion(in: candidate)
}
LoggingService.shared.logCloudKit(
"Account records arrived before the identity resolved — bootstrapping again to recover them"
)
}
private func applyStagedChanges(in namespace: String) {
let staged = store.stagedChanges(in: namespace)
guard !staged.isEmpty else { return }

15
Yattee/Services/CloudKit/AccountSyncStore.swift
View File

@@ -154,6 +154,21 @@ final class AccountSyncStore {
defaults.removeObject(forKey: key("schemaVersion", in: namespace))
}
// MARK: - Identity recovery
/// Whether account records were dropped because no identity was resolved to
/// stage them under.
///
/// Deliberately not namespaced — the identity they belonged to is the one
/// thing that could not be established — and persisted, because the engine
/// may already have moved its change token past those records: a crash
/// before the identity resolved would otherwise leave them unreachable by
/// any later fetch.
var hasPendingIdentityRecovery: Bool {
get { defaults.bool(forKey: "accountSync.pendingIdentityRecovery") }
set { defaults.set(newValue, forKey: "accountSync.pendingIdentityRecovery") }
}
/// The account-category schema this build understands. Raising it makes
/// every identity bootstrap once more, which is how a future release picks
/// up record types this one ignores.

22
Yattee/Services/HomeInstanceCache.swift
View File

@@ -24,6 +24,10 @@ final class HomeInstanceCache {
/// Whether the disk cache has been loaded.
private var diskCacheLoaded = false
/// Set when feeds had to be dropped before the disk cache was read, so the
/// entries that load carry the same purge.
private var pendingFeedPurge = false
private init() {}
// MARK: - Cache Key Management
@@ -71,6 +75,11 @@ final class HomeInstanceCache {
} else {
LoggingService.shared.debug("No home instance cache found on disk", category: .general)
}
if pendingFeedPurge {
pendingFeedPurge = false
clearAllFeeds()
}
}
/// Refreshes content from network for a specific instance and content type.
@@ -184,6 +193,19 @@ final class HomeInstanceCache {
/// otherwise be served each other's. Popular and trending are public and
/// stay.
func clearAllFeeds() {
// A purge can be asked for before the disk cache has been read — the
// identity check runs at startup, the load is async. Dropping only what
// is in memory would then be undone by the load, so the requirement is
// remembered and applied to whatever it brings in.
guard diskCacheLoaded else {
pendingFeedPurge = true
LoggingService.shared.debug(
"HomeInstanceCache.clearAllFeeds: Deferred until the disk cache is loaded",
category: .general
)
return
}
let keysToRemove = cache.keys.filter { $0.hasSuffix("_\(InstanceContentType.feed.rawValue)") }
guard !keysToRemove.isEmpty else { return }

9
Yattee/Views/Settings/EditSourceView.swift
View File

@@ -427,7 +427,14 @@ private struct EditRemoteServerContent: View {
}
appEnvironment.accountsManager.setStatus(.signedIn, for: account.id)
appEnvironment.instancesManager.setUsesAccountLogin(true, for: instance)
// `usesAccountLogin` is deliberately not set. It is pre-revamp state
// that migration only reads, and it syncs through iCloud key-value
// storage independently of the account records — a device receiving the
// flag before the account would synthesize a migrated account for a
// login that was never legacy, and the marker that arrives later does
// not retract it. Accounts describe logins now; the flag only describes
// the world before they existed.
// Signing in again keeps the same account, so nothing downstream sees a
// change of context — but the credential behind it may now belong to

49
YatteeTests/AccountSyncTests.swift
View File

@@ -612,8 +612,32 @@ struct AccountSyncTests {
#expect(identityA.accounts.account(id: account.id) != nil)
}
@Test("A context naming a deleted account is never adopted")
func tombstoneDominatesASyncedContext() async {
let harness = Self.makeHarness()
let account = harness.accounts.addAccount(instanceID: UUID())
harness.accounts.removeAccount(account.id)
// An offline device can select an account another device has already
// deleted, and publish that choice with a newer timestamp.
let stale = StoredDataContext(
context: .account(account.id),
changedAt: Date().addingTimeInterval(3_600)
)
harness.coordinator.stageFetched(
records: [harness.mapper.toCKRecord(dataContext: stale)],
deletions: []
)
await harness.coordinator.activate()
#expect(harness.accounts.currentContext == .local)
// The choice's own timestamp is kept, so a later real selection still
// outranks this repair on every device.
#expect(harness.accounts.storedContext.changedAt == stale.changedAt)
}
@Test("Records fetched before the identity resolves are recovered by a bootstrap")
func unresolvedIdentityForcesAnotherBootstrap() {
func unresolvedIdentityForcesAnotherBootstrap() async {
let defaults = Self.makeDefaults()
let store = AccountSyncStore(defaults: defaults)
store.markBootstrapCompleted(in: "identity-A")
@@ -640,14 +664,27 @@ struct AccountSyncTests {
#expect(store.stagedChanges(in: "unresolved").isEmpty)
// The engine's token has moved past those records, so the identity that
// resolves has to fetch the zone in full again to see them.
// resolves has to fetch the zone in full again to see them. The
// requirement is persisted, not held in memory: the token may already
// have advanced, and a launch that lost it would neither replay nor
// bootstrap.
#expect(store.hasPendingIdentityRecovery)
identity.value = "identity-A"
coordinator.identityDidChange()
let relaunched = AccountSyncCoordinator(
accountsManager: accounts,
namespaceProvider: { identity.value },
store: AccountSyncStore(defaults: defaults),
isCategoryEnabled: { true }
)
await relaunched.activate()
#expect(!store.isBootstrapCompleted(in: "identity-A"))
#expect(!store.hasPendingIdentityRecovery)
}
@Test("A switch of Apple account never claims records dropped while unresolved")
func switchedIdentityDoesNotClaimDroppedRecords() {
func switchedIdentityDoesNotClaimDroppedRecords() async {
let defaults = Self.makeDefaults()
let store = AccountSyncStore(defaults: defaults)
store.markBootstrapCompleted(in: "identity-A")
@@ -672,10 +709,8 @@ struct AccountSyncTests {
records: [AccountSyncTests.makeMapper().toCKRecord(account: Account(instanceID: UUID()))],
deletions: []
)
coordinator.identityDidChange()
identity.value = "identity-B"
coordinator.identityDidChange()
await coordinator.activate()
// Neither identity is handed the other's records; both recover their
// own from their own zone instead.