diff --git a/.claude/commands/qa.md b/.claude/commands/qa.md index 648170f9a..deb67b5f5 100644 --- a/.claude/commands/qa.md +++ b/.claude/commands/qa.md @@ -122,7 +122,7 @@ The canonical order (from `qa/SKILL.md` "Run all tests") is: → 16 → 17 → 20 → 27 → 28 → 19 → 15 → 34 → 18 ``` -plus secondary tests `14, 22, 23, 23b, 24, 25, 26, 28b, 29, 30, 31, 32, 33, 43, 44b` slotted where they don't conflict with destructive steps (09, 18). Test 43 (long-message rendering) explodes the shared conversation in teardown, so slot it late in the sequence, after other tests that reuse `shared_conversation_id`. Test 44b briefly shuts down the primary simulator to clone it (like 03/04), so don't overlap it with tests running on the primary; it otherwise runs entirely on its own disposable clone. +plus secondary tests `14, 22, 23, 23b, 24, 25, 26, 28b, 29, 30, 31, 32, 33, 43, 44b, 45` slotted where they don't conflict with destructive steps (09, 18). Test 43 (long-message rendering) explodes the shared conversation in teardown, so slot it late in the sequence, after other tests that reuse `shared_conversation_id`. Tests 44b and 45 briefly shut down the primary simulator to clone it (like 03/04), so don't overlap them with tests running on the primary; they otherwise run entirely on their own disposable clones. **What can actually run in parallel:** diff --git a/.github/workflows/keychain-identity-store-tests.yml b/.github/workflows/keychain-identity-store-tests.yml index 18d169951..ae757d351 100644 --- a/.github/workflows/keychain-identity-store-tests.yml +++ b/.github/workflows/keychain-identity-store-tests.yml @@ -25,7 +25,11 @@ jobs: test: name: Run KeychainIdentityStore Tests runs-on: macos-26 - timeout-minutes: 15 + # A warm-cache run finishes in ~9 minutes, but a Swift package cache + # miss forces a cold dependency build that exceeds 15 - the job then + # dies mid-compile without running a single test (observed repeatedly + # on PR #1151). Sized for the cold path. + timeout-minutes: 30 steps: - name: Checkout code diff --git a/ConvosCore/Sources/ConvosCore/Auth/Keychain/KeychainIdentityStore.swift b/ConvosCore/Sources/ConvosCore/Auth/Keychain/KeychainIdentityStore.swift index a19029e22..8d53c8336 100644 --- a/ConvosCore/Sources/ConvosCore/Auth/Keychain/KeychainIdentityStore.swift +++ b/ConvosCore/Sources/ConvosCore/Auth/Keychain/KeychainIdentityStore.swift @@ -235,6 +235,22 @@ public protocol KeychainIdentityStoreProtocol: Actor { /// flow after the user loses their device. func loadSyncedBackups() throws -> [KeychainIdentityBackup] + /// Reads this device's installation marker (see `InstallationMarker`). + /// Device-local like the primary slot, so it survives app deletion + /// and never syncs to other devices. + func loadInstallationMarker() throws -> InstallationMarker? + + /// Writes (or overwrites) this device's installation marker. + func saveInstallationMarker(_ marker: InstallationMarker) throws + + /// Reads this device's consent backup (see `ConsentBackup`). + /// Device-local like the primary slot, so it survives app deletion + /// and never syncs to other devices. + func loadConsentBackup() throws -> ConsentBackup? + + /// Writes (or overwrites) this device's consent backup. + func saveConsentBackup(_ backup: ConsentBackup) throws + /// Mirrors the primary identity into the synced backup slot when its /// backup is missing. Installs that registered before the backup /// slot existed only ever wrote the primary slot; calling this on @@ -282,6 +298,10 @@ public final actor KeychainIdentityStore: KeychainIdentityStoreProtocol { private let keychainAccessGroup: String private let syncedBackupEnabled: Bool private let deviceNameProvider: (@Sendable () -> String?)? + /// Set by `delete()` and cleared by the next identity `save`. While + /// set, the device-local slot writes (installation marker, consent + /// backup) are no-ops - see the comment in `delete()`. + private var deviceSlotsSwept: Bool = false public static let defaultService: String = "org.convos.ios.KeychainIdentityStore.v3" @@ -295,6 +315,14 @@ public final actor KeychainIdentityStore: KeychainIdentityStoreProtocol { /// Fixed account key for the identity in the primary slot. public static let identityAccount: String = "convos-identity" + /// Fixed account key for this device's installation marker, stored + /// device-local in `defaultService` alongside the identity. + public static let installationMarkerAccount: String = "convos-installation-marker" + + /// Fixed account key for this device's consent backup, stored + /// device-local in `defaultService` alongside the identity. + public static let consentBackupAccount: String = "convos-consent-backup" + // MARK: - Initialization /// - Parameters: @@ -339,6 +367,7 @@ public final actor KeychainIdentityStore: KeychainIdentityStoreProtocol { removeSyncedBackup(inboxId: displacedInboxId) } try saveData(data, with: identityQuery) + deviceSlotsSwept = false mirrorToSyncedBackup(identity) return identity } @@ -372,6 +401,36 @@ public final actor KeychainIdentityStore: KeychainIdentityStoreProtocol { } } + public func loadInstallationMarker() throws -> InstallationMarker? { + do { + let data = try Self.loadKeychainData(with: installationMarkerQuery.toReadDictionary()) + return try JSONDecoder().decode(InstallationMarker.self, from: data) + } catch KeychainIdentityStoreError.identityNotFound { + return nil + } + } + + public func saveInstallationMarker(_ marker: InstallationMarker) throws { + guard !deviceSlotsSwept else { return } + let data = try JSONEncoder().encode(marker) + try saveData(data, with: installationMarkerQuery) + } + + public func loadConsentBackup() throws -> ConsentBackup? { + do { + let data = try Self.loadKeychainData(with: consentBackupQuery.toReadDictionary()) + return try JSONDecoder().decode(ConsentBackup.self, from: data) + } catch KeychainIdentityStoreError.identityNotFound { + return nil + } + } + + public func saveConsentBackup(_ backup: ConsentBackup) throws { + guard !deviceSlotsSwept else { return } + let data = try JSONEncoder().encode(backup) + try saveData(data, with: consentBackupQuery) + } + public func backfillSyncedBackupIfNeeded() { guard syncedBackupEnabled else { return } do { @@ -405,6 +464,22 @@ public final actor KeychainIdentityStore: KeychainIdentityStoreProtocol { if let inboxId = primary?.inboxId { try deleteData(with: syncedBackupQuery(inboxId: inboxId)) } + // Sweep the device-local slots too: leaving the installation + // marker or consent backup behind would let a later sign-in on + // this device reconcile against - or restore consent from - state + // the user explicitly wiped. Best-effort: a failure here only + // leaves data the next identity ignores via its inboxId check. + // + // The swept flag closes the teardown race: an in-flight mirror or + // reconcile task already past its cancellation check can still be + // queued behind this delete on the actor, and its save would + // otherwise re-create the slot after the sweep. All device-local + // slot writes go through this actor, so turning them into no-ops + // until the next identity save serializes the race at its only + // choke point. + deviceSlotsSwept = true + try? deleteData(with: installationMarkerQuery) + try? deleteData(with: consentBackupQuery) try deleteData(with: identityQuery) } @@ -418,6 +493,22 @@ public final actor KeychainIdentityStore: KeychainIdentityStoreProtocol { ) } + private nonisolated var installationMarkerQuery: KeychainQuery { + KeychainQuery( + account: Self.installationMarkerAccount, + service: keychainService, + accessGroup: keychainAccessGroup + ) + } + + private nonisolated var consentBackupQuery: KeychainQuery { + KeychainQuery( + account: Self.consentBackupAccount, + service: keychainService, + accessGroup: keychainAccessGroup + ) + } + private nonisolated func syncedBackupQuery(inboxId: String) -> KeychainQuery { KeychainQuery( account: inboxId, diff --git a/ConvosCore/Sources/ConvosCore/Auth/Keychain/MockKeychainIdentityStore.swift b/ConvosCore/Sources/ConvosCore/Auth/Keychain/MockKeychainIdentityStore.swift index cd32e4aad..a9b80612c 100644 --- a/ConvosCore/Sources/ConvosCore/Auth/Keychain/MockKeychainIdentityStore.swift +++ b/ConvosCore/Sources/ConvosCore/Auth/Keychain/MockKeychainIdentityStore.swift @@ -14,6 +14,14 @@ actor MockKeychainIdentityStore: KeychainIdentityStoreProtocol { /// inboxId — one entry per backed-up identity, like the real store's /// per-identity accounts. private let backupState: OSAllocatedUnfairLock<[String: KeychainIdentityBackup]> = .init(initialState: [:]) + /// In-memory stand-in for the device-local installation marker slot. + private let markerState: OSAllocatedUnfairLock = .init(initialState: nil) + /// In-memory stand-in for the device-local consent backup slot. + private let consentBackupState: OSAllocatedUnfairLock = .init(initialState: nil) + /// Mirrors the real store's swept flag: set by delete(), cleared by save. + private let deviceSlotsSwept: OSAllocatedUnfairLock = .init(initialState: false) + /// Optional error injection for saveConsentBackup. + private let consentBackupSaveError: OSAllocatedUnfairLock<(any Error)?> = .init(initialState: nil) /// Optional error injection for the load path. Tests simulating a /// transient keychain daemon failure set this to a non-nil `Error`; /// `loadSync` and `load` both throw it until the test clears it. @@ -26,6 +34,7 @@ actor MockKeychainIdentityStore: KeychainIdentityStoreProtocol { func save(inboxId: String, clientId: String, keys: KeychainIdentityKeys) throws -> KeychainIdentity { let identity = KeychainIdentity(inboxId: inboxId, clientId: clientId, keys: keys) let displacedInboxId = state.withLock { $0?.inboxId } + deviceSlotsSwept.withLock { $0 = false } state.withLock { $0 = identity } backupState.withLock { backups in if let displacedInboxId, displacedInboxId != inboxId { @@ -74,6 +83,36 @@ actor MockKeychainIdentityStore: KeychainIdentityStoreProtocol { if let inboxId { backupState.withLock { $0.removeValue(forKey: inboxId) } } + deviceSlotsSwept.withLock { $0 = true } + markerState.withLock { $0 = nil } + consentBackupState.withLock { $0 = nil } + } + + func loadInstallationMarker() throws -> InstallationMarker? { + markerState.withLock { $0 } + } + + func saveInstallationMarker(_ marker: InstallationMarker) throws { + guard !deviceSlotsSwept.withLock({ $0 }) else { return } + markerState.withLock { $0 = marker } + } + + func loadConsentBackup() throws -> ConsentBackup? { + consentBackupState.withLock { $0 } + } + + func saveConsentBackup(_ backup: ConsentBackup) throws { + guard !deviceSlotsSwept.withLock({ $0 }) else { return } + if let error = consentBackupSaveError.withLock({ $0 }) { + throw error + } + consentBackupState.withLock { $0 = backup } + } + + /// Test-only - inject an error for subsequent `saveConsentBackup` + /// calls. Pass `nil` to clear. + nonisolated func _setConsentBackupSaveError(_ error: (any Error)?) { + consentBackupSaveError.withLock { $0 = error } } /// Test-only — inject an error for the next `loadSync`/`load` calls. diff --git a/ConvosCore/Sources/ConvosCore/Device/ConsentBackup.swift b/ConvosCore/Sources/ConvosCore/Device/ConsentBackup.swift new file mode 100644 index 000000000..abe30d66b --- /dev/null +++ b/ConvosCore/Sources/ConvosCore/Device/ConsentBackup.swift @@ -0,0 +1,85 @@ +import Foundation +import GRDB + +/// The set of conversations this device's user has allowed, mirrored into +/// the device-local keychain so it survives app deletion (the consent +/// records themselves live in the app container's database and do not). +/// +/// Consent cannot be recovered from the network after a reinstall: XMTP +/// propagates consent records as messages inside the device-sync MLS +/// group, which a new installation cannot decrypt (they predate its +/// membership), and the only installation that could re-send them died +/// with the deleted app container. Without this backup, conversations +/// recovered by re-welcome land as consent=unknown and never surface in +/// the UI. Restoring from the backup is spam-safe: it re-allows exactly +/// the conversations this device's user had allowed, nothing else. +public struct ConsentBackup: Codable, Sendable, Equatable { + public let inboxId: String + public let allowedConversationIds: [String] + + public init(inboxId: String, allowedConversationIds: [String]) { + self.inboxId = inboxId + self.allowedConversationIds = allowedConversationIds + } +} + +extension ConsentBackup { + /// The ids worth backing up: every non-draft conversation the user has + /// allowed. Drafts are client-local placeholders that don't exist on + /// the network, so restoring consent for them is meaningless. Sorted + /// so snapshots compare stably. Static and pure for unit testing. + static func allowedConversationIds(db: Database) throws -> [String] { + let ids = try DBConversation + .filter(DBConversation.Columns.consent == Consent.allowed) + .select(DBConversation.Columns.id, as: String.self) + .fetchAll(db) + return ids + .filter { !DBConversation.isDraft(id: $0) } + .sorted() + } +} + +/// Restore helpers for backed-up consent after a reinstall resume, +/// invoked from the stale-installation reconcile (the reliable reinstall +/// signal: same inbox, different installation id). Split into Sendable- +/// only helpers because the XMTP client is not Sendable and must stay in +/// the caller's actor region - the caller sequences: load ids here, apply +/// them via `client.setConsentStates`, then flip stored rows here. The +/// keychain backup is never consumed by a restore attempt. +enum ConsentBackupRestorer { + /// The backed-up allowed conversation ids applicable to `inboxId`, + /// empty when there is no backup, the backup belongs to a replaced + /// identity, or nothing was allowed. + static func idsToRestore( + identityStore: any KeychainIdentityStoreProtocol, + inboxId: String + ) async throws -> [String] { + guard let backup = try await identityStore.loadConsentBackup(), + backup.inboxId == inboxId else { + return [] + } + return backup.allowedConversationIds + } + + /// Flips already-stored rows for the restored ids from `.unknown` to + /// `.allowed` so the list updates without waiting for a re-sync. + /// Rows in other states are left alone - `.denied` means the user + /// deleted the conversation and it must stay hidden. Rows that don't + /// exist yet need nothing: their welcomes land already-allowed via + /// the consent records written before this. + static func flipStoredUnknownRows( + ids: [String], + databaseWriter: any DatabaseWriter + ) async throws { + guard !ids.isEmpty else { return } + try await databaseWriter.write { db in + let stored = try DBConversation + .filter(ids.contains(DBConversation.Columns.id)) + .filter(DBConversation.Columns.consent == Consent.unknown) + .fetchAll(db) + for conversation in stored { + try conversation.with(consent: .allowed).save(db) + } + } + } +} diff --git a/ConvosCore/Sources/ConvosCore/Device/StaleInstallationReconciler.swift b/ConvosCore/Sources/ConvosCore/Device/StaleInstallationReconciler.swift new file mode 100644 index 000000000..7aadac2cc --- /dev/null +++ b/ConvosCore/Sources/ConvosCore/Device/StaleInstallationReconciler.swift @@ -0,0 +1,77 @@ +import Foundation + +/// Record of which XMTP installation this device most recently ran for an +/// inbox, kept in the device-local keychain so it survives app deletion +/// (the installation keys themselves live in the app container's database +/// and do not). +/// +/// The iOS keychain outliving the app is what makes reinstall churn +/// possible: a reinstall resumes the identity from the keychain but must +/// mint a new installation, permanently orphaning the previous one - its +/// keys died with the deleted database, yet it stays registered on the +/// inbox and shows up as a ghost "Device " row in the devices list. +/// Comparing this marker against the live installation on launch is the +/// only way to prove an installation was this device's own dead one (and +/// not a legitimately paired device), which makes it safe to auto-revoke. +public struct InstallationMarker: Codable, Sendable, Equatable { + public let inboxId: String + public let installationId: String + /// Installations this device orphaned but has not yet successfully + /// revoked - carried across launches so an offline launch retries + /// later instead of leaking the ghost forever. Growth is bounded in + /// practice by requiring a full reinstall cycle per entry while every + /// revoke attempt keeps failing; the first launch with network + /// drains the whole list in one revoke call. + public let staleInstallationIds: [String] + + public init(inboxId: String, installationId: String, staleInstallationIds: [String]) { + self.inboxId = inboxId + self.installationId = installationId + self.staleInstallationIds = staleInstallationIds + } +} + +/// Pure planning logic for reconciling the installation marker against +/// the live session, split from the keychain and network plumbing so it +/// can be unit tested. +enum StaleInstallationReconciler { + struct Plan: Equatable { + /// Marker to persist for this launch (records any newly-orphaned + /// installation before the revoke attempt, so a crash or network + /// failure retries on the next launch). + let marker: InstallationMarker + /// This device's own dead installations, candidates for + /// revocation once filtered against the live installation list. + let candidateStaleIds: [String] + } + + /// A nil or foreign-inbox marker carries no history worth acting on: + /// first run of a build with marker support, or the identity was + /// replaced wholesale (pairing adoption, delete-all) - in both cases + /// any previous installation belongs to an abandoned inbox that the + /// devices list never shows, so revoking it is pointless. Start + /// fresh. Otherwise every installation id this marker saw that isn't + /// the current one is provably this device's own dead installation. + static func plan( + marker: InstallationMarker?, + inboxId: String, + installationId: String + ) -> Plan { + guard let marker, marker.inboxId == inboxId else { + return Plan( + marker: InstallationMarker(inboxId: inboxId, installationId: installationId, staleInstallationIds: []), + candidateStaleIds: [] + ) + } + var stale = marker.staleInstallationIds + if marker.installationId != installationId { + stale.append(marker.installationId) + } + var seen = Set() + let filtered = stale.filter { $0 != installationId && seen.insert($0).inserted } + return Plan( + marker: InstallationMarker(inboxId: inboxId, installationId: installationId, staleInstallationIds: filtered), + candidateStaleIds: filtered + ) + } +} diff --git a/ConvosCore/Sources/ConvosCore/Inboxes/SessionStateMachine.swift b/ConvosCore/Sources/ConvosCore/Inboxes/SessionStateMachine.swift index 1aa0fe5af..2a913bb8f 100644 --- a/ConvosCore/Sources/ConvosCore/Inboxes/SessionStateMachine.swift +++ b/ConvosCore/Sources/ConvosCore/Inboxes/SessionStateMachine.swift @@ -110,6 +110,10 @@ public actor SessionStateMachine: SessionStateManagerProtocol { private let xmtpClientFactory: XMTPClientFactory private var currentTask: Task? + /// The stale-installation reconcile in flight, if any. Cancelled on + /// stop and delete so a teardown racing the reconcile can't issue + /// revocations or rewrite keychain state for a session being removed. + private var staleReconcileTask: Task? private var actionQueue: [Action] = [] private var isProcessing: Bool = false private var networkMonitorTask: Task? @@ -565,6 +569,8 @@ public actor SessionStateMachine: SessionStateManagerProtocol { try await inboxWriter.save(inboxId: client.inboxId, clientId: identity.clientId) Log.info("Saved inbox to database: \(client.inboxId)") + await seedInstallationMarker(client: client) + enqueueAction(.clientAuthorized(client)) } @@ -621,9 +627,40 @@ public actor SessionStateMachine: SessionStateManagerProtocol { throw error } + await seedInstallationMarker(client: client) + enqueueAction(.clientRegistered(client)) } + /// Records the just-minted installation in the marker as soon as the + /// client exists, before backend auth can fail - a launch killed + /// between minting the installation and reaching `.ready` must still + /// leave the marker behind, or a later reinstall can't prove the + /// orphaned installation was this device's own. Cheap (one keychain + /// read, at most one write) and non-throwing, so it can sit inline + /// on the authorize and register paths. Deliberately not gated on + /// `syncingManager` (unlike the reconcile): the notification service + /// extension can also mint an installation when it authorizes against + /// a wiped database after a reinstall, and one the marker never + /// records becomes a permanently unrevokable ghost. Seeding makes no + /// network calls, so it is safe in every session mode; only the + /// revocation half stays app-only. + private func seedInstallationMarker(client: any XMTPClientProvider) async { + do { + let marker = try await identityStore.loadInstallationMarker() + let plan = StaleInstallationReconciler.plan( + marker: marker, + inboxId: client.inboxId, + installationId: client.installationId + ) + if plan.marker != marker { + try await identityStore.saveInstallationMarker(plan.marker) + } + } catch { + Log.warning("Failed to seed installation marker: \(error)") + } + } + private func handleClientAuthorized(client: any XMTPClientProvider) async throws { try Task.checkCancellation() @@ -636,9 +673,122 @@ public actor SessionStateMachine: SessionStateManagerProtocol { try await assertInstallationActive(client: client) + reconcileStaleOwnInstallations() + enqueueAction(.authorized(.init(client: client, apiClient: apiClient))) } + /// Reinstalls resume the identity from the device keychain but mint a + /// new XMTP installation (the previous one's keys died with the + /// deleted app container), leaving a permanently-dead installation + /// registered on the inbox - a ghost "Device " row in the + /// devices list that reappears after every reinstall cycle. The + /// device-local installation marker records which installation this + /// device ran last, which proves the orphan was this device's own + /// dead installation (and not a legitimately paired device), making + /// it safe to revoke automatically. + /// + /// Best-effort and detached from the authorize flow: a failed revoke + /// leaves the stale id in the marker and retries on the next launch. + /// Skipped when streaming services are off (the notification service + /// extension) - the NSE has no business making revocation calls. + private func reconcileStaleOwnInstallations() { + guard syncingManager != nil else { return } + staleReconcileTask?.cancel() + staleReconcileTask = Task { [weak self] in + await self?.performStaleOwnInstallationReconcile() + } + } + + private func performStaleOwnInstallationReconcile() async { + do { + // Awaits the ready state rather than capturing the client at + // the call site - the client is not Sendable, so it must be + // obtained inside this actor-isolated task. + let client = try await waitForInboxReadyResult().client + let marker = try await identityStore.loadInstallationMarker() + let plan = StaleInstallationReconciler.plan( + marker: marker, + inboxId: client.inboxId, + installationId: client.installationId + ) + if plan.marker != marker { + try await identityStore.saveInstallationMarker(plan.marker) + } + guard !plan.candidateStaleIds.isEmpty else { return } + // A stale own installation means this launch is a reinstall + // resume: the app container (and with it every consent record) + // was wiped, so re-welcomed conversations would land as + // consent=unknown and never surface. The live installation + // list decides how consent comes back before anything else + // runs. Inlined (not a helper taking the client) so the + // non-Sendable client stays a disconnected value used only in + // sequential awaits. A restore failure aborts the reconcile + // before the revoke: the marker keeps its stale ids, so both + // retry on the next launch (letting the revoke clear the + // marker after a failed restore would end the retries and + // leave the conversations hidden). + let liveIds = try await client.listInstallations(refreshFromNetwork: true).map(\.id) + let idsToRevoke = plan.candidateStaleIds.filter(Set(liveIds).contains) + let livePeerIds = Set(liveIds) + .subtracting(idsToRevoke) + .subtracting([client.installationId]) + if livePeerIds.isEmpty { + // Sole surviving installation: the device-local backup is + // the only consent source, and "denied from elsewhere" is + // impossible with no other installation, so restoring the + // backed-up allowed set is safe. Records written now also + // cover welcomes that haven't arrived yet; the backup is + // never consumed. + let consentIds = try await ConsentBackupRestorer.idsToRestore( + identityStore: identityStore, + inboxId: client.inboxId + ) + if !consentIds.isEmpty { + try await client.setConsentStates(conversationIds: consentIds, consent: .allowed) + try await ConsentBackupRestorer.flipStoredUnknownRows( + ids: consentIds, + databaseWriter: databaseWriter + ) + Log.info("Restored consent for \(consentIds.count) conversation(s) from keychain backup after reinstall") + QAEvent.emit(.conversation, "consent_backup_restored", ["count": "\(consentIds.count)"]) + } + } else { + // Live paired installations exist: they hold the inbox's + // current consent, including denies made while this device + // was uninstalled. Writing fresh-timestamp allowed records + // from the backup would override those denies via + // last-writer-wins, so request preference sync instead - + // peers replay consent records with their original + // timestamps. Best-effort: the peers also push updates on + // their own sync cadence. + try? await client.syncPreferences() + Log.info("Deferred reinstall consent restore to \(livePeerIds.count) live installation(s)") + QAEvent.emit(.conversation, "consent_restore_deferred_to_peers", ["peers": "\(livePeerIds.count)"]) + } + try Task.checkCancellation() + if !idsToRevoke.isEmpty { + guard let identity = try await identityStore.load() else { return } + try await client.revokeInstallations( + signingKey: identity.keys.signingKey, + installationIds: idsToRevoke + ) + } + try Task.checkCancellation() + try await identityStore.saveInstallationMarker( + InstallationMarker( + inboxId: client.inboxId, + installationId: client.installationId, + staleInstallationIds: [] + ) + ) + Log.info("Reconciled stale own installations after reinstall (revoked \(idsToRevoke.count))") + QAEvent.emit(.pairing, "stale_own_installations_revoked", ["count": "\(idsToRevoke.count)"]) + } catch { + Log.warning("Stale own-installation reconcile failed (marker retries next launch): \(error)") + } + } + private func handleClientRegistered(client: any XMTPClientProvider) async throws { try Task.checkCancellation() @@ -646,6 +796,12 @@ public actor SessionStateMachine: SessionStateManagerProtocol { Log.info("Authenticating with backend...") try await authenticateBackend() + // The marker was already seeded in handleRegister; a fresh + // registration has no stale installations of its own, so this + // only matters as the retry path for stales an earlier launch + // failed to revoke. + reconcileStaleOwnInstallations() + try Task.checkCancellation() enqueueAction(.authorized(.init(client: client, apiClient: apiClient))) @@ -687,6 +843,8 @@ public actor SessionStateMachine: SessionStateManagerProtocol { try Task.checkCancellation() Log.info("Deleting inbox with clientId: \(initialClientId)...") + staleReconcileTask?.cancel() + staleReconcileTask = nil defer { enqueueAction(.stop) } let liveContext: (client: any XMTPClientProvider, apiClient: any ConvosAPIClientProtocol)? @@ -753,6 +911,8 @@ public actor SessionStateMachine: SessionStateManagerProtocol { private func handleStop() async throws { Log.info("Stopping inbox with clientId \(initialClientId)...") + staleReconcileTask?.cancel() + staleReconcileTask = nil stopRevocationObserver() let clientToClose: (any XMTPClientProvider)? diff --git a/ConvosCore/Sources/ConvosCore/Messaging/XMTPClientProvider.swift b/ConvosCore/Sources/ConvosCore/Messaging/XMTPClientProvider.swift index a097475c5..e2e787ef2 100644 --- a/ConvosCore/Sources/ConvosCore/Messaging/XMTPClientProvider.swift +++ b/ConvosCore/Sources/ConvosCore/Messaging/XMTPClientProvider.swift @@ -124,6 +124,17 @@ public protocol XMTPClientProvider: AnyObject { func conversation(with id: String) async throws -> XMTPiOS.Conversation? func inboxId(for ethereumAddress: String) async throws -> String? func update(consent: Consent, for conversationId: String) async throws + /// Writes consent records directly at the preferences layer, without + /// requiring the conversations to exist locally - records written + /// before a conversation's welcome arrives still apply to it. Used to + /// restore backed-up consent after a reinstall. + func setConsentStates(conversationIds: [String], consent: Consent) async throws + /// Requests preference (consent) sync from the inbox's other live + /// installations via the device-sync group. Used after a reinstall + /// when live peers exist: their replayed consent records carry the + /// original timestamps, so denies made while this device was + /// uninstalled are preserved instead of overridden. + func syncPreferences() async throws func revokeInstallations( signingKey: SigningKey, installationIds: [String] ) async throws @@ -237,6 +248,22 @@ extension XMTPiOS.Client: XMTPClientProvider { try await foundConversation.updateConsentState(state: consent.consentState) } + public func setConsentStates(conversationIds: [String], consent: Consent) async throws { + guard !conversationIds.isEmpty else { return } + let entries = conversationIds.map { (conversationId: String) -> ConsentRecord in + ConsentRecord( + value: conversationId, + entryType: .conversation_id, + consentType: consent.consentState + ) + } + try await preferences.setConsentState(entries: entries) + } + + public func syncPreferences() async throws { + try await preferences.sync() + } + public func listInstallations(refreshFromNetwork: Bool) async throws -> [InstallationInfo] { let state = try await inboxState(refreshFromNetwork: refreshFromNetwork) return state.installations.map { InstallationInfo(id: $0.id, createdAt: $0.createdAt) } diff --git a/ConvosCore/Sources/ConvosCore/Mocks/MockXMTPClientProvider.swift b/ConvosCore/Sources/ConvosCore/Mocks/MockXMTPClientProvider.swift index 76624499d..f8311faba 100644 --- a/ConvosCore/Sources/ConvosCore/Mocks/MockXMTPClientProvider.swift +++ b/ConvosCore/Sources/ConvosCore/Mocks/MockXMTPClientProvider.swift @@ -65,6 +65,19 @@ public final class MockXMTPClientProvider: XMTPClientProvider, @unchecked Sendab // No-op for mock } + /// Recorded for assertions: every batch passed to `setConsentStates`. + public private(set) var setConsentStatesCalls: [(conversationIds: [String], consent: Consent)] = [] + + public func setConsentStates(conversationIds: [String], consent: Consent) async throws { + setConsentStatesCalls.append((conversationIds: conversationIds, consent: consent)) + } + + public private(set) var syncPreferencesCallCount: Int = 0 + + public func syncPreferences() async throws { + syncPreferencesCallCount += 1 + } + public func revokeInstallations(signingKey: any SigningKey, installationIds: [String]) async throws { // No-op for mock } diff --git a/ConvosCore/Sources/ConvosCore/Syncing/ConsentBackupMirror.swift b/ConvosCore/Sources/ConvosCore/Syncing/ConsentBackupMirror.swift new file mode 100644 index 000000000..196c441a1 --- /dev/null +++ b/ConvosCore/Sources/ConvosCore/Syncing/ConsentBackupMirror.swift @@ -0,0 +1,161 @@ +import Foundation +import GRDB +import os + +/// Session-scoped observer that mirrors the allowed-conversation set into +/// the device-local keychain (`ConsentBackup`) whenever it changes, so a +/// reinstall can restore the user's consent (see `ConsentBackupRestorer` +/// for why the network can't). Owned by `SyncingManager` alongside +/// `ConversationConsentReconciler`, so it only runs in the full app +/// session, never in the notification service extension. +/// +/// Mirroring follows one carry rule, bounded by a settling window: ids +/// present in the observer's initial backup stay in every written +/// snapshot until the database has shown them once or the window expires. +/// On a reinstall launch the database starts empty and refills over the +/// first minute as welcomes and syncs land; mirroring those partial +/// emissions directly would shrink the very backup the reconcile's +/// restore is reading. An id observed and later gone was denied by the +/// user and drops out immediately. An id never observed by the end of the +/// window is not refilling - it was denied from another device or its +/// conversation died network-side - and carrying it forever would +/// resurrect it on the next reinstall, so the window's expiry flush +/// drops it from the backup. +/// +/// `@unchecked Sendable`: all mutable state lives behind unfair locks. +final class ConsentBackupMirror: @unchecked Sendable { + private let databaseReader: any DatabaseReader + private let identityStore: any KeychainIdentityStoreProtocol + private let carryWindow: Duration + + private let observationTask: OSAllocatedUnfairLock?> = .init(initialState: nil) + private let flushTask: OSAllocatedUnfairLock?> = .init(initialState: nil) + private let state: OSAllocatedUnfairLock = .init(initialState: MirrorState()) + + private struct MirrorState { + var everObservedIds: Set = [] + var carriedBackupIds: Set? + var lastObservedIds: [String] = [] + var carryWindowExpired: Bool = false + /// Set when a keychain write failed so the next attempt skips the + /// unchanged-backup early return and retries the write. + var lastSaveFailed: Bool = false + } + + init( + databaseReader: any DatabaseReader, + identityStore: any KeychainIdentityStoreProtocol, + carryWindow: Duration = .seconds(300) + ) { + self.databaseReader = databaseReader + self.identityStore = identityStore + self.carryWindow = carryWindow + } + + /// Begin observing. Safe to call repeatedly - the previous tasks are + /// cancelled and replaced, and the carry state resets. + func start() { + state.withLock { $0 = MirrorState() } + observationTask.withLock { existing in + existing?.cancel() + existing = Task { [weak self] in + await self?.observe() + } + } + flushTask.withLock { [carryWindow] existing in + existing?.cancel() + existing = Task { [weak self] in + try? await Task.sleep(for: carryWindow) + await self?.flushAfterCarryWindow() + } + } + } + + func stop() { + observationTask.withLock { existing in + existing?.cancel() + existing = nil + } + flushTask.withLock { existing in + existing?.cancel() + existing = nil + } + } + + private func observe() async { + let stream = ValueObservation + .tracking { db in + try ConsentBackup.allowedConversationIds(db: db) + } + .removeDuplicates() + .values(in: databaseReader) + do { + for try await ids in stream { + if Task.isCancelled { return } + // Bookkeeping happens here, unconditionally, not inside + // the fallible keychain path: an emission whose mirror + // attempt fails must still count as observed, or a later + // shrink would be misread as refill and resurrect a + // user's deny into the backup. + state.withLock { mirrorState in + mirrorState.everObservedIds.formUnion(ids) + mirrorState.lastObservedIds = ids + } + await mirror(allowedConversationIds: ids) + } + } catch { + Log.error("ConsentBackupMirror: stream failed: \(error.localizedDescription)") + } + } + + /// Ends the carry window and re-mirrors the last observed set so ids + /// that never appeared actually leave the backup - the observation + /// only emits on database changes, so without this flush a quiet + /// session would never drop them. + private func flushAfterCarryWindow() async { + if Task.isCancelled { return } + let lastObserved = state.withLock { mirrorState in + mirrorState.carryWindowExpired = true + return mirrorState.lastObservedIds + } + await mirror(allowedConversationIds: lastObserved) + } + + /// Writes the snapshot when the resulting backup differs from what's + /// already in the keychain (or the previous write failed). The + /// identity read failing (or no identity yet) skips the write; the + /// observation re-fires on the next change and converges. + private func mirror(allowedConversationIds: [String]) async { + do { + guard let inboxId = try identityStore.loadSync()?.inboxId else { return } + let existing = try await identityStore.loadConsentBackup() + // Cancellation is cooperative and the loop's check only runs + // between emissions - a task cancelled while suspended on the + // load above must not write, or a replaced observer's stale + // snapshot could land after its successor's. (A delete-all + // racing this write is additionally blocked by the store's + // swept flag.) + guard !Task.isCancelled else { return } + let (pendingRefillIds, retrying) = state.withLock { mirrorState -> (Set, Bool) in + if mirrorState.carriedBackupIds == nil { + mirrorState.carriedBackupIds = existing?.inboxId == inboxId + ? Set(existing?.allowedConversationIds ?? []) + : [] + } + let pending = mirrorState.carryWindowExpired + ? [] + : (mirrorState.carriedBackupIds ?? []).subtracting(mirrorState.everObservedIds) + return (pending.subtracting(allowedConversationIds), mirrorState.lastSaveFailed) + } + let target = Set(allowedConversationIds).union(pendingRefillIds).sorted() + let backup = ConsentBackup(inboxId: inboxId, allowedConversationIds: target) + guard backup != existing || retrying else { return } + try await identityStore.saveConsentBackup(backup) + state.withLock { $0.lastSaveFailed = false } + Log.info("ConsentBackupMirror: mirrored \(target.count) allowed conversation(s) to keychain (\(pendingRefillIds.count) carried)") + } catch { + state.withLock { $0.lastSaveFailed = true } + Log.warning("ConsentBackupMirror: failed to mirror consent backup (will retry on next change or flush): \(error)") + } + } +} diff --git a/ConvosCore/Sources/ConvosCore/Syncing/SyncingManager.swift b/ConvosCore/Sources/ConvosCore/Syncing/SyncingManager.swift index a5f24db96..dc1577e5f 100644 --- a/ConvosCore/Sources/ConvosCore/Syncing/SyncingManager.swift +++ b/ConvosCore/Sources/ConvosCore/Syncing/SyncingManager.swift @@ -70,6 +70,7 @@ enum SyncingError: Error { case streamRetriesExhausted } +// swiftlint:disable:next type_body_length actor SyncingManager: SyncingManagerProtocol { // MARK: - State Machine @@ -135,6 +136,7 @@ actor SyncingManager: SyncingManagerProtocol { /// source of truth for feed visibility). Reconstructed per session /// start because it captures the live XMTP client. private var consentReconciler: ConversationConsentReconciler? + private var consentBackupMirror: ConsentBackupMirror? /// Populates the agent-template read-through cache for template-backed /// agent contacts. Reconstructed per start because it captures the @@ -214,6 +216,7 @@ actor SyncingManager: SyncingManagerProtocol { deinit { // Clean up tasks consentReconciler?.stop() + consentBackupMirror?.stop() agentTemplateCacheCoordinator?.stop() syncTask?.cancel() notificationTask?.cancel() @@ -1115,17 +1118,21 @@ extension SyncingManager { installPushTokenObserver() } - /// Stop and clear the session-scoped observers (consent reconciler and - /// agent-template cache coordinator) on pause / teardown. + /// Stop and clear the session-scoped observers (consent reconciler, + /// consent backup mirror, and agent-template cache coordinator) on + /// pause / teardown. fileprivate func stopSessionScopedObservers() { consentReconciler?.stop() consentReconciler = nil + consentBackupMirror?.stop() + consentBackupMirror = nil agentTemplateCacheCoordinator?.stop() agentTemplateCacheCoordinator = nil } - /// (Re)start the consent reconciler with the live client. Safe to call - /// on every start / resume - the previous instance is stopped first. + /// (Re)start the consent reconciler and the consent backup mirror with + /// the live client. Safe to call on every start / resume - the + /// previous instances are stopped first. fileprivate func restartConsentReconciler(client: AnyClientProvider) { consentReconciler?.stop() let reconciler = ConversationConsentReconciler( @@ -1135,6 +1142,14 @@ extension SyncingManager { ) reconciler.start() consentReconciler = reconciler + + consentBackupMirror?.stop() + let mirror = ConsentBackupMirror( + databaseReader: databaseReader, + identityStore: identityStore + ) + mirror.start() + consentBackupMirror = mirror } /// (Re)start the agent-template cache coordinator with the live API diff --git a/ConvosCore/Tests/ConvosCoreTests/ConsentBackupTests.swift b/ConvosCore/Tests/ConvosCoreTests/ConsentBackupTests.swift new file mode 100644 index 000000000..e34d13334 --- /dev/null +++ b/ConvosCore/Tests/ConvosCoreTests/ConsentBackupTests.swift @@ -0,0 +1,310 @@ +@testable import ConvosCore +import Foundation +import GRDB +import Testing + +/// Covers the reinstall consent-restore plumbing: the allowed-set query +/// the mirror observes, and the restorer that re-applies a backed-up +/// allowed set after a reinstall resume (consent records died with the +/// wiped app container and cannot be recovered from the network). +@Suite("Consent Backup", .serialized) +struct ConsentBackupTests { + @Test("Allowed-set query returns sorted allowed ids, excluding drafts and other consent states") + func allowedSetQuery() throws { + let dbManager = MockDatabaseManager.makeTestDatabase() + try dbManager.dbWriter.write { db in + try Self.seedConversation(db: db, id: "convo-b", consent: .allowed) + try Self.seedConversation(db: db, id: "convo-a", consent: .allowed) + try Self.seedConversation(db: db, id: "convo-unknown", consent: .unknown) + try Self.seedConversation(db: db, id: "convo-denied", consent: .denied) + try Self.seedConversation(db: db, id: "draft-123", consent: .allowed) + } + + let ids = try dbManager.dbReader.read { db in + try ConsentBackup.allowedConversationIds(db: db) + } + + #expect(ids == ["convo-a", "convo-b"]) + } + + @Test("Ids to restore come from a matching-inbox backup only") + func idsToRestoreMatchesInbox() async throws { + let identityStore = MockKeychainIdentityStore() + + // No backup at all. + #expect(try await ConsentBackupRestorer.idsToRestore( + identityStore: identityStore, inboxId: "inbox-1" + ) == []) + + // Backup for a different inbox (identity was replaced wholesale). + try await identityStore.saveConsentBackup( + ConsentBackup(inboxId: "other-inbox", allowedConversationIds: ["convo-x"]) + ) + #expect(try await ConsentBackupRestorer.idsToRestore( + identityStore: identityStore, inboxId: "inbox-1" + ) == []) + + // Matching inbox. + try await identityStore.saveConsentBackup( + ConsentBackup(inboxId: "inbox-1", allowedConversationIds: ["convo-a", "convo-b"]) + ) + #expect(try await ConsentBackupRestorer.idsToRestore( + identityStore: identityStore, inboxId: "inbox-1" + ) == ["convo-a", "convo-b"]) + } + + @Test("Row flip promotes unknown rows, leaves denied rows and missing rows alone") + func flipStoredUnknownRows() async throws { + let dbManager = MockDatabaseManager.makeTestDatabase() + try await dbManager.dbWriter.write { db in + // Re-welcomed before the restore ran (stored as unknown). + try Self.seedConversation(db: db, id: "convo-a", consent: .unknown) + // Deleted by the user (denied) - the restore must not + // resurrect it. + try Self.seedConversation(db: db, id: "convo-denied", consent: .denied) + // convo-b has no row yet (welcome not arrived) - must no-op. + } + + try await ConsentBackupRestorer.flipStoredUnknownRows( + ids: ["convo-a", "convo-b", "convo-denied"], + databaseWriter: dbManager.dbWriter + ) + + let consents = try await dbManager.dbReader.read { db in + try DBConversation.fetchAll(db).reduce(into: [String: Consent]()) { acc, row in + acc[row.id] = row.consent + } + } + #expect(consents["convo-a"] == .allowed) + #expect(consents["convo-denied"] == .denied) + #expect(consents["convo-b"] == nil) + } + + @Test("Mirror carries unseen backup ids through a reinstall refill, never shrinking the backup") + func mirrorCarriesUnseenBackupIdsThroughRefill() async throws { + let dbManager = MockDatabaseManager.makeTestDatabase() + let identityStore = MockKeychainIdentityStore() + let keys = try KeychainIdentityKeys.generate() + _ = try await identityStore.save(inboxId: "inbox-1", clientId: "client-1", keys: keys) + // The backup a previous install wrote; the fresh database refills + // gradually and no intermediate snapshot may shrink the backup. + let previous = ConsentBackup(inboxId: "inbox-1", allowedConversationIds: ["convo-a", "convo-b"]) + try await identityStore.saveConsentBackup(previous) + + let mirror = ConsentBackupMirror( + databaseReader: dbManager.dbReader, + identityStore: identityStore + ) + mirror.start() + defer { mirror.stop() } + + // Initial empty emission, then a partial refill (only convo-a + // arrived) - both would previously shrink the backup. + try await Task.sleep(nanoseconds: 300_000_000) + #expect(try await identityStore.loadConsentBackup() == previous) + try await dbManager.dbWriter.write { db in + try Self.seedConversation(db: db, id: "convo-a", consent: .allowed) + } + try await Task.sleep(nanoseconds: 300_000_000) + #expect(try await identityStore.loadConsentBackup() == previous) + + // A new conversation the backup didn't know about joins the set; + // convo-b is still refilling and must be carried. + try await dbManager.dbWriter.write { db in + try Self.seedConversation(db: db, id: "convo-new", consent: .allowed) + } + try await Self.waitUntil { + try await identityStore.loadConsentBackup()?.allowedConversationIds == ["convo-a", "convo-b", "convo-new"] + } + + // Once convo-b has been observed, denying it is a real user + // action and the shrink must be written. + try await dbManager.dbWriter.write { db in + try Self.seedConversation(db: db, id: "convo-b", consent: .allowed) + } + try await dbManager.dbWriter.write { db in + guard let conversation = try DBConversation + .filter(DBConversation.Columns.id == "convo-b") + .fetchOne(db) else { return } + try conversation.with(consent: .denied).save(db) + } + try await Self.waitUntil { + try await identityStore.loadConsentBackup()?.allowedConversationIds == ["convo-a", "convo-new"] + } + } + + @Test("Mirror drops an id denied during the refill window") + func mirrorDropsIdDeniedDuringRefill() async throws { + let dbManager = MockDatabaseManager.makeTestDatabase() + let identityStore = MockKeychainIdentityStore() + let keys = try KeychainIdentityKeys.generate() + _ = try await identityStore.save(inboxId: "inbox-1", clientId: "client-1", keys: keys) + try await identityStore.saveConsentBackup( + ConsentBackup(inboxId: "inbox-1", allowedConversationIds: ["convo-a", "convo-b"]) + ) + + let mirror = ConsentBackupMirror( + databaseReader: dbManager.dbReader, + identityStore: identityStore + ) + mirror.start() + defer { mirror.stop() } + + // convo-a refills, then the user denies it while convo-b is still + // pending: the write must drop convo-a (observed, then denied) + // but keep carrying convo-b (never observed). + try await dbManager.dbWriter.write { db in + try Self.seedConversation(db: db, id: "convo-a", consent: .allowed) + } + try await Self.waitUntil { + try await identityStore.loadConsentBackup()?.allowedConversationIds == ["convo-a", "convo-b"] + } + try await dbManager.dbWriter.write { db in + guard let conversation = try DBConversation + .filter(DBConversation.Columns.id == "convo-a") + .fetchOne(db) else { return } + try conversation.with(consent: .denied).save(db) + } + try await Self.waitUntil { + try await identityStore.loadConsentBackup()?.allowedConversationIds == ["convo-b"] + } + } + + /// Polls `condition` until it holds or a 5s deadline expires (the + /// mirror reacts to GRDB observation emissions asynchronously). + private static func waitUntil(_ condition: () async throws -> Bool) async throws { + let deadline = Date().addingTimeInterval(5) + while Date() < deadline { + if try await condition() { return } + try await Task.sleep(nanoseconds: 50_000_000) + } + Issue.record("Condition not met within deadline") + } + + @Test("Carry window expiry drops ids the session never observed") + func carryWindowExpiryDropsUnobservedIds() async throws { + let dbManager = MockDatabaseManager.makeTestDatabase() + let identityStore = MockKeychainIdentityStore() + let keys = try KeychainIdentityKeys.generate() + _ = try await identityStore.save(inboxId: "inbox-1", clientId: "client-1", keys: keys) + // convo-gone was denied from another device while this app was + // closed: it will never appear in this database's allowed set, + // and carrying it past the settling window would resurrect it on + // the next reinstall. + try await identityStore.saveConsentBackup( + ConsentBackup(inboxId: "inbox-1", allowedConversationIds: ["convo-a", "convo-gone"]) + ) + try await dbManager.dbWriter.write { db in + try Self.seedConversation(db: db, id: "convo-a", consent: .allowed) + } + + let mirror = ConsentBackupMirror( + databaseReader: dbManager.dbReader, + identityStore: identityStore, + carryWindow: .milliseconds(600) + ) + mirror.start() + defer { mirror.stop() } + + // Inside the window the unobserved id is carried. + try await Self.waitUntil { + try await identityStore.loadConsentBackup()?.allowedConversationIds == ["convo-a", "convo-gone"] + } + // After the window's flush it is dropped, even with no further + // database changes to re-fire the observation. + try await Self.waitUntil { + try await identityStore.loadConsentBackup()?.allowedConversationIds == ["convo-a"] + } + } + + @Test("A failed keychain write is retried on the next emission") + func failedSaveRetriesOnNextChange() async throws { + struct TransientKeychainError: Error {} + let dbManager = MockDatabaseManager.makeTestDatabase() + let identityStore = MockKeychainIdentityStore() + let keys = try KeychainIdentityKeys.generate() + _ = try await identityStore.save(inboxId: "inbox-1", clientId: "client-1", keys: keys) + + let mirror = ConsentBackupMirror( + databaseReader: dbManager.dbReader, + identityStore: identityStore + ) + identityStore._setConsentBackupSaveError(TransientKeychainError()) + mirror.start() + defer { mirror.stop() } + + // This emission's write fails; the ids must still count as + // observed and the write must be retried later. + try await dbManager.dbWriter.write { db in + try Self.seedConversation(db: db, id: "convo-a", consent: .allowed) + } + try await Task.sleep(nanoseconds: 300_000_000) + #expect(try await identityStore.loadConsentBackup() == nil) + + identityStore._setConsentBackupSaveError(nil) + try await dbManager.dbWriter.write { db in + try Self.seedConversation(db: db, id: "convo-b", consent: .allowed) + } + try await Self.waitUntil { + try await identityStore.loadConsentBackup()?.allowedConversationIds == ["convo-a", "convo-b"] + } + } + + @Test("Device-local slot writes are no-ops after delete until a new identity is saved") + func sweptSlotsRejectWrites() async throws { + let identityStore = MockKeychainIdentityStore() + let keys = try KeychainIdentityKeys.generate() + _ = try await identityStore.save(inboxId: "inbox-1", clientId: "client-1", keys: keys) + try await identityStore.delete() + + // A racing task that lost the teardown race must not resurrect + // the wiped slots. + try await identityStore.saveConsentBackup( + ConsentBackup(inboxId: "inbox-1", allowedConversationIds: ["convo-a"]) + ) + try await identityStore.saveInstallationMarker( + InstallationMarker(inboxId: "inbox-1", installationId: "i1", staleInstallationIds: []) + ) + #expect(try await identityStore.loadConsentBackup() == nil) + #expect(try await identityStore.loadInstallationMarker() == nil) + + // A new identity re-enables the slots. + _ = try await identityStore.save(inboxId: "inbox-2", clientId: "client-2", keys: keys) + try await identityStore.saveConsentBackup( + ConsentBackup(inboxId: "inbox-2", allowedConversationIds: ["convo-b"]) + ) + #expect(try await identityStore.loadConsentBackup()?.allowedConversationIds == ["convo-b"]) + } + + private static func seedConversation( + db: Database, + id: String, + consent: Consent + ) throws { + try DBMember(inboxId: "creator-\(id)").save(db, onConflict: .ignore) + try DBConversation( + id: id, + clientConversationId: "client-\(id)", + inviteTag: "tag-\(id)", + creatorId: "creator-\(id)", + kind: .group, + consent: consent, + createdAt: Date(), + name: nil, + description: nil, + imageURLString: nil, + publicImageURLString: nil, + includeInfoInPublicPreview: false, + expiresAt: nil, + debugInfo: .empty, + isLocked: false, + imageSalt: nil, + imageNonce: nil, + imageEncryptionKey: nil, + conversationEmoji: nil, + imageLastRenewed: nil, + isUnused: false, + hasHadVerifiedAgent: false + ).insert(db) + } +} diff --git a/ConvosCore/Tests/ConvosCoreTests/StaleInstallationReconcilerTests.swift b/ConvosCore/Tests/ConvosCoreTests/StaleInstallationReconcilerTests.swift new file mode 100644 index 000000000..39c33e77e --- /dev/null +++ b/ConvosCore/Tests/ConvosCoreTests/StaleInstallationReconcilerTests.swift @@ -0,0 +1,125 @@ +@testable import ConvosCore +import Foundation +import Testing + +/// Covers the pure planning logic that decides which of this device's own +/// previous installations are safe to revoke after a reinstall (the +/// keychain identity survives app deletion, the installation keys don't). +@Suite("Stale Installation Reconciler") +struct StaleInstallationReconcilerTests { + @Test("First run seeds the marker without revoking anything") + func firstRunSeedsMarker() { + let plan = StaleInstallationReconciler.plan( + marker: nil, + inboxId: "inbox-1", + installationId: "install-a" + ) + + #expect(plan.candidateStaleIds.isEmpty) + #expect(plan.marker == InstallationMarker( + inboxId: "inbox-1", + installationId: "install-a", + staleInstallationIds: [] + )) + } + + @Test("Reinstall marks the previous installation stale") + func reinstallMarksPreviousInstallationStale() { + let marker = InstallationMarker( + inboxId: "inbox-1", + installationId: "install-a", + staleInstallationIds: [] + ) + + let plan = StaleInstallationReconciler.plan( + marker: marker, + inboxId: "inbox-1", + installationId: "install-b" + ) + + #expect(plan.candidateStaleIds == ["install-a"]) + #expect(plan.marker.staleInstallationIds == ["install-a"]) + #expect(plan.marker.installationId == "install-b") + } + + @Test("Unrevoked stales accumulate across reinstalls and dedupe") + func stalesAccumulateAcrossReinstalls() { + // Two reinstalls happened while offline: install-a's revoke never + // succeeded, then install-b was orphaned too. + let marker = InstallationMarker( + inboxId: "inbox-1", + installationId: "install-b", + staleInstallationIds: ["install-a", "install-a"] + ) + + let plan = StaleInstallationReconciler.plan( + marker: marker, + inboxId: "inbox-1", + installationId: "install-c" + ) + + #expect(plan.candidateStaleIds == ["install-a", "install-b"]) + } + + @Test("Same installation keeps retrying carried stales") + func sameInstallationRetriesCarriedStales() { + let marker = InstallationMarker( + inboxId: "inbox-1", + installationId: "install-b", + staleInstallationIds: ["install-a"] + ) + + let plan = StaleInstallationReconciler.plan( + marker: marker, + inboxId: "inbox-1", + installationId: "install-b" + ) + + #expect(plan.candidateStaleIds == ["install-a"]) + #expect(plan.marker == marker) + } + + @Test("Inbox change resets the marker without revoking") + func inboxChangeResetsMarker() { + // Pairing adoption or delete-all replaced the identity wholesale; + // the old inbox's installations are abandoned with it and the + // devices list never shows them - nothing to revoke. + let marker = InstallationMarker( + inboxId: "old-inbox", + installationId: "install-a", + staleInstallationIds: ["install-z"] + ) + + let plan = StaleInstallationReconciler.plan( + marker: marker, + inboxId: "new-inbox", + installationId: "install-b" + ) + + #expect(plan.candidateStaleIds.isEmpty) + #expect(plan.marker == InstallationMarker( + inboxId: "new-inbox", + installationId: "install-b", + staleInstallationIds: [] + )) + } + + @Test("Current installation is never a revocation candidate") + func currentInstallationNeverRevoked() { + // A marker corrupted into listing the live installation as stale + // must not produce a self-revoke. + let marker = InstallationMarker( + inboxId: "inbox-1", + installationId: "install-b", + staleInstallationIds: ["install-b", "install-a"] + ) + + let plan = StaleInstallationReconciler.plan( + marker: marker, + inboxId: "inbox-1", + installationId: "install-b" + ) + + #expect(plan.candidateStaleIds == ["install-a"]) + } +} diff --git a/ConvosCore/Tests/ConvosCoreTests/SyncingManagerTests.swift b/ConvosCore/Tests/ConvosCoreTests/SyncingManagerTests.swift index cc03a4d3a..5f6d046cd 100644 --- a/ConvosCore/Tests/ConvosCoreTests/SyncingManagerTests.swift +++ b/ConvosCore/Tests/ConvosCoreTests/SyncingManagerTests.swift @@ -82,6 +82,12 @@ class TestableMockClient: XMTPClientProvider, @unchecked Sendable { func update(consent: Consent, for conversationId: String) async throws { } + func setConsentStates(conversationIds: [String], consent: Consent) async throws { + } + + func syncPreferences() async throws { + } + func revokeInstallations(signingKey: any SigningKey, installationIds: [String]) async throws { } diff --git a/qa/RULES.md b/qa/RULES.md index 696e3ff53..313838cc4 100644 --- a/qa/RULES.md +++ b/qa/RULES.md @@ -109,14 +109,19 @@ test's `screen` prerequisite or a navigation step doesn't match the screen. ### Tabs -- Two tabs in the system tab bar: **Chats** (`message.fill`) and **Things** - (`square.grid.2x2.fill`). Select a tab by tapping its label ("Chats" / - "Things"). +- Three tabs in the system tab bar: **Convos**, **Things**, and + **Contacts**. Select a tab by tapping its label. Steps that reference a + "Chats" tab are stale - the conversations list lives in the Convos tab. - The **Search** tab was removed. Any step that taps a search tab or `search-tab` is stale - there is no search entry point right now. - On iPhone the tab bar is at the bottom; on iPad it is at the top. +- An empty Convos tab on a fresh account renders a full-screen + agent-marketing hero ("Make little agents"), not an empty list - don't + interpret it as a broken conversations list, but if conversations are + known to exist and the hero persists, suspect they're being filtered + out (e.g. consent=unknown rows never surface; see test 45). -### Conversations list (Chats tab) +### Conversations list (Convos tab) - Verified by `compose-button` being present. - `compose-button` opens the new-conversation flow (`NewConversationView`). @@ -622,11 +627,11 @@ The invite flow is multi-step and requires coordination between the inviting sid 3. Wait 2-3 seconds for the app to process the deep link and send the join request. 4. **Then** run `process-join-requests` from the CLI. -**Always use `--watch` with `--timeout`** when running `process-join-requests`. The join request may take a moment to arrive over the network. Example: +**Always use `--watch`** when running `process-join-requests`, and bound it externally — the CLI has no `--timeout` flag. The join request may take a moment to arrive over the network. The correct command form (validated against convos CLI 0.10.x; the old `convos conversation ... ` positional form is not a valid command): ```bash -convos conversation process-join-requests --watch --timeout 30 +timeout 45 convos conversations process-join-requests --conversation --watch ``` -Never run `process-join-requests` without `--timeout` — it can hang indefinitely with `--watch`, or miss the request without `--watch`. A 30-second timeout is a safe default. +Do not kill the watcher as soon as it prints its "Adding ..." line — that consumes the join request without completing the add-member commit and the joiner never lands. Wait for the "Sent ProfileSnapshot" line before stopping it. Running without `--watch` can miss a request that hasn't arrived yet and silently succeed. ### Test Results diff --git a/qa/SKILL.md b/qa/SKILL.md index b60a1bf92..20ab3715a 100644 --- a/qa/SKILL.md +++ b/qa/SKILL.md @@ -54,6 +54,7 @@ Run end-to-end QA tests for the Convos iOS app using the iOS simulator tools and | 43 | `qa/tests/43-long-message-rendering.md` | Long-message UX: short renders unchanged, long shows Read More + inline expand, pathological pushes MessageDetailView (Back/Copy/Reply), no-hang guard | | 44 | `qa/tests/44-icloud-pairing-prompt.md` | First-install iCloud pairing prompt: Pair ? sheet from a synced keychain backup, Skip persistence, and the full no-QR pair handshake (two simulators; Device B is a clone of Device A) | | 44b | `qa/tests/44b-pairing-prompt-ordering.md` | Pairable-backup ordering rule: backups newer than this install's own key are never offered; verified both directions via the restamp launch hook (single disposable clone) | +| 45 | `qa/tests/45-reinstall-message-continuity.md` | Delete + reinstall while actively chatting: same inbox resumes from the keychain, old installation auto-revoked, and messages flow both ways with the CLI in every prior conversation (erased disposable clone) | ## Running Tests diff --git a/qa/tests/45-reinstall-message-continuity.md b/qa/tests/45-reinstall-message-continuity.md new file mode 100644 index 000000000..cf28718d0 --- /dev/null +++ b/qa/tests/45-reinstall-message-continuity.md @@ -0,0 +1,50 @@ +# Test: Reinstall Message Continuity + +Verify that deleting and reinstalling the app does not break an actively chatting identity: the device keychain outlives the app, so a reinstall resumes the same inbox with a new XMTP installation. After the reinstall, messages must flow in both directions in every conversation the identity was in, the same inboxId must resume, and the previous (dead) installation must be auto-revoked. + +## Prerequisites + +- One simulator: the branch primary (used only as a clone source; its app state is never touched). +- The convos CLI initialized against the dev network (User B). +- A built non-production Convos.app. + +## Setup + +1. Shut down the primary (`simctl clone` requires it), clone it as `convos-qa-reinstall`, **erase the clone** (a fresh account is the point of this test), boot both, relaunch the app on the primary. Install the built app on the clone. +2. Every launch on the clone needs the App Check debug token for registration: `source .env` and pass `SIMCTL_CHILD_FIRAAppCheckDebugToken="$FIREBASE_APP_CHECK_DEBUG_TOKEN"` on each `simctl launch`. +3. Launch the app (fresh identity registers). Create two conversations shared with the CLI: for each, the CLI creates the conversation and generates an invite, the app joins via the invite deep link, and the CLI processes the join request (per invite ordering rules in RULES.md). Record the app's inboxId from the authorization log line as `inbox_id_before`. + +## Steps + +### Baseline + +1. In conversation 1: CLI sends "pre-reinstall from B (1)" and it appears in the app; the app sends "pre-reinstall from A (1)" and the CLI reads it. +2. In conversation 2: same exchange with the "(2)" texts. This is the baseline the reinstall must preserve. + +### Reinstall + +3. Terminate the app, `xcrun simctl uninstall $clone org.convos.ios-preview` (deletes app and app-group containers; the keychain survives), reinstall the same built app, launch with the App Check token. +4. The app resumes the same identity from the keychain and mints a new installation; on reaching ready it revokes the previous one - `pairing.stale_own_installations_revoked count=1` in the app log. Note the group-container path changed with the reinstall; re-resolve it before grepping. Confirm the authorization log line carries the same inboxId (`inbox_id_after`). + +### Post-reinstall delivery + +5. CLI sends "post-reinstall from B (1)" and "(2)" into both conversations. The reinstalled app starts with an empty local database; the conversations must reappear once the new installation is welcomed back into the groups - give the first appearance a generous timeout (the CLI's send is also what prompts B's client to commit A's new installation). If nothing appears, foreground-cycle the app once before failing. Both messages must appear in the app. +6. The app sends "post-reinstall from A (1)" and "(2)" in the respective conversations; the CLI must read both. +7. Observational: note whether the pre-reinstall messages are visible again and record it as `history_visible_after_reinstall` - either outcome passes; the finding informs the reinstall-history product decision. + +## Teardown + +CLI explodes both conversations. Shut down and delete the `convos-qa-reinstall` clone; the primary simulator was never modified. + +## Pass/Fail Criteria + +- [ ] Baseline: both directions deliver in both conversations before the reinstall +- [ ] The reinstall launch emits `pairing.stale_own_installations_revoked` with count >= 1 +- [ ] The inboxId after the reinstall equals the inboxId before (resumed, not re-registered) +- [ ] CLI messages sent after the reinstall appear in the app, in both conversations +- [ ] App messages sent after the reinstall are readable via CLI, in both conversations +- [ ] `history_visible_after_reinstall` recorded (observational, either outcome passes) + +## Accessibility Improvements Needed + +None known - the test reuses message-field and send-button identifiers from test 02 and log events end-to-end. diff --git a/qa/tests/structured/45-reinstall-message-continuity.yaml b/qa/tests/structured/45-reinstall-message-continuity.yaml new file mode 100644 index 000000000..413593705 --- /dev/null +++ b/qa/tests/structured/45-reinstall-message-continuity.yaml @@ -0,0 +1,223 @@ +id: "45" +name: "Reinstall Message Continuity" +description: > + Verify that deleting and reinstalling the app does not break an actively + chatting identity: the device keychain outlives the app, so a reinstall + resumes the same inbox with a new XMTP installation. After the reinstall, + the other participant's messages must reach the app, the app's messages + must reach the other participant, and this must hold across every + conversation the identity was in - not just one group. Also covers the + reinstall side effects: the same inboxId resumes, and the previous + (dead) installation is auto-revoked via the installation marker. +tags: [messaging, reinstall, keychain, installations, core] +depends_on: ["01"] +estimated_duration_s: 720 + +prerequisites: + app_running: false + cli_initialized: true + shared_conversation: false + # Runs entirely on a disposable erased clone with a fresh account; the + # primary simulator's app state is never touched. + screen: none + +state: + clone_udid: null + inbox_id_before: null + inbox_id_after: null + conversation_1_id: null + conversation_2_id: null + history_visible_after_reinstall: null + +setup: + - action: clone_primary_as_reinstall_sim + save: + clone_udid: "from clone" + note: > + `xcrun simctl shutdown $PRIMARY`, `xcrun simctl clone $PRIMARY + convos-qa-reinstall`, `xcrun simctl erase` the clone (a fresh + account, not a copied one, is the point of this test), boot both, + relaunch the app on the primary. Install the built Convos.app on + the clone. Every launch on the clone needs the App Check debug + token for registration: source .env and pass + SIMCTL_CHILD_FIRAAppCheckDebugToken="$FIREBASE_APP_CHECK_DEBUG_TOKEN" + on each simctl launch. + + - action: create_two_shared_conversations + save: + conversation_1_id: "first conversation id" + conversation_2_id: "second conversation id" + note: > + Launch the app on the clone (fresh identity registers). Create two + conversations shared with the CLI: for each, CLI creates the + conversation and generates an invite, the app joins via the invite + deep link, and the CLI processes the join request (per invite + ordering rules in RULES.md). Start `process-join-requests + --watch` in the background before opening the invite deep link: + a join request that lands in the same second the watch stream + starts is missed, and a later non-watch `process-join-requests` + finds 0 pending (validated run 1). If that happens, re-open the + same invite URL with the watcher already running - the app + re-sends the join request. Record the app's inboxId as + inbox_id_before - it appears in the app log's authorization lines + ("Started authorization flow for inbox: ..."), or via CLI member + listing on either conversation. + +steps: + - id: pre_reinstall_chat + name: "Both directions deliver in both conversations before the reinstall" + actions: + - cli_send_text: { conversation: "$conversation_1_id", text: "pre-reinstall from B (1)" } + - wait_for_element: { label_contains: "pre-reinstall from B (1)", timeout: 15 } + - type_in_field: { id: "message-text-field", text: "pre-reinstall from A (1)" } + - tap: { id: "send-message-button" } + - cli_read_messages: { conversation: "$conversation_1_id", sync: true, grep: "pre-reinstall from A (1)" } + - cli_send_text: { conversation: "$conversation_2_id", text: "pre-reinstall from B (2)" } + - type_in_field_in_conversation: + conversation: "$conversation_2_id" + id: "message-text-field" + text: "pre-reinstall from A (2)" + - cli_read_messages: { conversation: "$conversation_2_id", sync: true, grep: "pre-reinstall from A (2)" } + verify: + - element_exists: { label_contains: "pre-reinstall from B (1)" } + - cli_output_contains: "pre-reinstall from A (1)" + - cli_output_contains: "pre-reinstall from A (2)" + criteria: baseline_chat_works + note: > + This is the baseline the reinstall must preserve. Navigate between + the two conversations from the chats list as needed - the exact + navigation is up to the runner. The iOS keyboard auto-capitalizes + the first typed letter ("pre-..." arrives as "Pre-..."), so all + cli_read_messages greps for app-typed text must be + case-insensitive. + + - id: reinstall + name: "Delete and reinstall the app" + actions: + - terminate_app: {} + - uninstall_app: {} + - install_app: {} + - launch_app: {} + verify: + - event_exists: { name: "pairing.stale_own_installations_revoked" } + expect_event: "pairing.stale_own_installations_revoked" + criteria: old_installation_revoked + note: > + `xcrun simctl uninstall $clone org.convos.ios-preview` deletes the + app and app-group containers but the keychain survives; reinstall + the same built Convos.app and launch (with the App Check token). + The app must resume the same identity from the keychain and mint a + new installation; on reaching ready it revokes the previous one + (pairing.stale_own_installations_revoked count=1 in the app log, + which lives at /Logs/convos.log - note the group + container path changed with the reinstall). Also confirm the + authorization log line carries the same inboxId as before; save it + as inbox_id_after. + + - id: identity_resumed + name: "The same inbox resumed after reinstall" + actions: + - compare_state: { left: "$inbox_id_before", right: "$inbox_id_after" } + verify: + - state_equals: { left: "$inbox_id_before", right: "$inbox_id_after" } + criteria: identity_resumed + + - id: b_to_a_after_reinstall + name: "CLI's post-reinstall messages reach the app in both conversations" + actions: + - cli_send_text: { conversation: "$conversation_1_id", text: "post-reinstall from B (1)" } + - cli_send_text: { conversation: "$conversation_2_id", text: "post-reinstall from B (2)" } + - wait_for_element: { label_contains: "post-reinstall from B (1)", timeout: 90 } + - screenshot: {} + verify: + - element_exists: { label_contains: "post-reinstall from B (1)" } + - element_exists: { label_contains: "post-reinstall from B (2)" } + criteria: b_to_a_delivery + note: > + The reinstalled app starts with an empty local database; the + conversations must reappear once the new installation is welcomed + back into the groups. Give the first delivery a generous timeout - + the CLI's send is also what prompts B's client to commit A's new + installation into the group. While waiting, record whether the + pre-reinstall messages are visible again as + history_visible_after_reinstall (true/false) - both outcomes are + observational, not pass/fail; what must work is new-message flow. + If nothing appears within the timeout, foreground-cycle the app + once (background then foreground triggers a sync) before failing. + Check the app log and local DB before concluding delivery failed: + in run 1 the messages were received and stored (message.received + events fired) but the recovered conversations were saved with + consent=unknown, so the list showed the first-run empty state and + nothing was visible in the UI - a UI-visibility failure, not a + network delivery failure. Fixed since: the allowed set is mirrored + into the device-local keychain (ConsentBackupMirror) and restored + on the reinstall launch - expect + conversation.consent_backup_restored count>=1 shortly after the + stale-installation revoke, and the conversations to surface as + allowed without any workaround (verified in run c4ba3927cf16). + + - id: a_to_b_after_reinstall + name: "The app's post-reinstall messages reach the CLI in both conversations" + note: > + Reach each conversation from the Convos list only. Do not use the + invite deep link to reach an invisible conversation - the rejoin + flow flips consent to allowed as a side effect, which would let + this step pass during a consent-restore regression and mask the + exact defect the feature prevents. If the conversations are not in + the list, b_to_a_after_reinstall has already failed; record this + step as blocked by that failure instead of working around it. + actions: + - type_in_field_in_conversation: + conversation: "$conversation_1_id" + id: "message-text-field" + text: "post-reinstall from A (1)" + - cli_read_messages: { conversation: "$conversation_1_id", sync: true, grep: "post-reinstall from A (1)" } + - type_in_field_in_conversation: + conversation: "$conversation_2_id" + id: "message-text-field" + text: "post-reinstall from A (2)" + - cli_read_messages: { conversation: "$conversation_2_id", sync: true, grep: "post-reinstall from A (2)" } + verify: + - cli_output_contains: "post-reinstall from A (1)" + - cli_output_contains: "post-reinstall from A (2)" + criteria: a_to_b_delivery + + - id: history_observation + name: "Record whether pre-reinstall history is visible" + actions: + - record_state: { key: "history_visible_after_reinstall" } + - screenshot: {} + verify: + - state_recorded: { key: "history_visible_after_reinstall" } + criteria: history_state_documented + note: > + Observational: scroll conversation 1 and note whether the + "pre-reinstall" messages are present. Pass with the observation + recorded either way; the finding feeds the product decision about + reinstall history restore, it is not itself a defect verdict. + +teardown: + - action: cli_explode_conversations + args: { ids: ["$conversation_1_id", "$conversation_2_id"] } + optional: true + - action: delete_reinstall_sim + args: { udid: "$clone_udid" } + optional: true + note: > + `xcrun simctl shutdown $clone_udid && xcrun simctl delete + $clone_udid`. The clone held a throwaway identity; the primary + simulator was never modified. + +criteria: + baseline_chat_works: + description: "Before the reinstall, messages deliver in both directions in both conversations (app UI shows CLI's texts; CLI reads the app's texts)" + old_installation_revoked: + description: "The reinstall launch emits pairing.stale_own_installations_revoked with count>=1 (the previous installation was auto-revoked)" + identity_resumed: + description: "The inboxId after reinstall equals the inboxId before (identity resumed from the device keychain, not re-registered)" + b_to_a_delivery: + description: "Messages sent by the CLI after the reinstall appear in the app in both conversations" + a_to_b_delivery: + description: "Messages sent from the app after the reinstall are readable via CLI in both conversations" + history_state_documented: + description: "Whether pre-reinstall messages are visible after the reinstall is recorded in test state (observational - either outcome passes)"