Add iCloud backup and restore flow

[lourou]

Add iCloud backup plan: vault-centric architecture

Backup plan stacked on jarod/convos-vault-impl:

• Vault is single source of truth for conversation keys
• iCloud Keychain only stores vault key (one key, not N)
• Backup bundle encrypted with vault key
• Phased: vault key sync → installation revocation → backup bundle → restore → scheduling → UI

Add PR briefings for iCloud backup implementation stack

Note

Add iCloud backup and restore flow with stale device detection

• Introduces BackupManager and RestoreManager actors that encrypt/decrypt a TAR-based bundle using AES-GCM, archive vault and per-conversation XMTP data, copy the app database, and write/read from an iCloud container.
• Adds ICloudIdentityStore, a dual-write keychain actor that saves vault keys to both a local ThisDeviceOnly store and a synchronizable iCloud AfterFirstUnlock store, with read-through caching and selective deletion.
• Adds StaleDeviceState (healthy/partialStale/fullStale) derived from per-inbox isStale flags; InboxStateMachine sets staleness after checking network installation activity and revokes superseded installations on sign-in.
• Conversations restored from backup are marked inactive (isActive = false); the UI shows an 'Awaiting' status, blocks sending/reacting, and displays an InactiveConversationBanner; StreamProcessor reactivates conversations and flags recent messages as reconnections when new messages arrive.
• ConversationsView renders a StaleDeviceBanner with 'Learn more' and 'Reset device' actions; full-stale state clears conversation selection and triggers an auto-reset confirmation flow.
• DatabaseManager gains replaceDatabase(with:) with rollback-on-failure and post-restore migration; SessionManager implements RestoreLifecycleControlling to pause and resume sync services around a restore.
• Adds a BackupRestoreSettingsView in App Settings and BackupDebugView/VaultKeySyncDebugView in the Debug screen for QA and testing.
• Risk: replaceDatabase(with:) is destructive; if both the replacement and the rollback fail, a DatabaseManagerError.rollbackFailed is thrown and the database is left in an undefined state.

^{Macroscope summarized e60255d.}

[lourou]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• Add iCloud backup and restore flow #602 : 2 dependent PRs (#591 , #698 ) 👈 (View in Graphite)
• Convos Vault: Multi-device key sync implementation #564
• dev

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

Claude finished @lourou's task —— View job

---

PR Review: iCloud Backup and Restore Flow

Comprehensive code review completed. This is a large, well-architected PR that implements iCloud backup/restore with encryption, stale device detection, and inactive conversation handling. Below is my feedback organized by category.

---

🏗️ Architecture & Design

✅ Strengths

1. Excellent separation of concerns: BackupManager handles creation, RestoreManager handles restoration, with clear protocols (BackupArchiveProvider, RestoreArchiveImporter, VaultArchiveImporter)

2. Vault-centric architecture is sound: one vault key in iCloud Keychain instead of N conversation keys, reducing sync complexity

3. Dual-write keychain strategy (ICloudIdentityStore) with local-first reads and iCloud fallback is well-designed for both performance and disaster recovery

4. Commit-point pattern in RestoreManager:161 prevents partial failures - smart rollback strategy

5. Protocol-based dependency injection throughout enables testability

🟡 Suggestions

1. BackupManager could benefit from state tracking like RestoreManager has. Currently it's fire-and-forget, making it hard to track long-running backup progress in UI

2. Consider making backup scheduling more explicit - the PR adds the infra but doesn't implement automatic periodic backups (though this may be intentional for phased rollout)

---

⚠️ Error Handling & Data Loss Scenarios

🟠 Critical Issues

1. DatabaseManager.replaceDatabase double-fault scenario (DatabaseManager.swift:64-81)

   • If restore fails AND rollback fails, database is in inconsistent state
   • Risk: DatabaseManagerError.rollbackFailed is thrown but recovery requires app reinstall
   • Recommendation: Add pre-flight validation of backup DB (e.g., open read-only, verify schema version) before starting destructive operations

2. Restore UI progress tracking broken (BackupRestoreSettingsView.swift:85-101)

   • Macroscope identified this: restoreState only updates once after completion, not during operation
   • RestoreManager broadcasts granular states via state property, but UI doesn't consume them
   • Fix needed: Use for await state in manager.stateSequence or observe manager.state periodically
   • Evidence: RestoreManager.swift has detailed state updates (lines 82, 387, 428, 452, 487, 510) but UI doesn't consume them

3. Conversation archive import failures are non-fatal (RestoreManager.swift:468-512)

   • Individual archive import errors are logged but don't fail the restore
   • Could silently skip conversations if archives are corrupted
   • Recommendation: Add metadata validation - if bundle claims N conversations but we only import M < threshold%, fail loudly

🟡 Medium Concerns

4. XMTP file staging cleanup on failure (RestoreManager.swift:219-276)

   • stageXMTPFiles moves files to temp dir, but if FileManager.moveItem fails mid-stream (lines 239-243), some files remain in staging
   • Line 274-275 unconditionally deletes staging dir, losing unmoved files
   • Recommendation: Track which files were successfully moved, only delete those on cleanup

5. Keychain restore partial failure handling (RestoreManager.swift:424-447)

   • If all keys fail to save (failedKeyCount == entries.count), restore aborts - good
   • But if only some fail (e.g., 3/10), restore continues with incomplete keychain
   • Those conversations will be inaccessible post-restore
   • Recommendation: Consider failing if failure rate > threshold (e.g., >20%)

6. iCloud container availability not checked before backup (BackupManager.swift:185-203)

   • Silently falls back to local storage if iCloud unavailable
   • User may think they have iCloud backup when it's actually local-only
   • Recommendation: Surface backup location to user in UI (already partially done in metadata, but could be clearer)

---

🔒 Security

✅ Strengths

1. AES-256-GCM encryption (BackupBundleCrypto) is industry-standard, well-implemented
2. Vault key stored in iCloud Keychain with proper accessibility (kSecAttrAccessibleAfterFirstUnlock)
3. No secrets in logs - checked backup/restore flows, keys are not logged
4. Path traversal protection in BackupBundle.untarData (lines 164-182) - double-checks with symlink resolution

🟡 Suggestions

7. Encryption key derivation could be more explicit

   • vaultIdentity.keys.databaseKey is used directly as AES key
   • Consider KDF (Key Derivation Function) for additional security layer, though current approach is acceptable for this use case

8. Backup bundle validation

   • No HMAC/signature to verify bundle integrity (only detects corruption via AES-GCM auth tag failure)
   • Consider adding explicit integrity check before attempting restore

---

🧪 Test Coverage

✅ Well-Tested

• BackupBundleTests.swift: 561 lines - comprehensive tar format, crypto, path traversal tests
• RestoreManagerTests.swift: 597 lines - full restore flow, DB replacement, rollback scenarios
• InboxWriterTests.swift: Added stale device tests (156 new lines)
• StaleDeviceStateTests.swift: 185 lines - comprehensive state machine tests
• ConversationLocalStateWriterTests.swift: 150 lines - inactive conversation handling

🟡 Gaps

9. No integration test for full backup → restore round-trip

   • Tests mock BackupArchiveProvider and RestoreArchiveImporter
   • Would benefit from at least one end-to-end test using real XMTP archive creation
   • (This may exist elsewhere or be covered by manual QA)

10. ICloudIdentityStore tested in KeychainIdentityStoreTests but iCloud-specific edge cases could use more coverage

    • What happens when iCloud Keychain is toggled off mid-operation?
    • Cross-device sync timing issues?

11. DatabaseManager.replaceDatabase rollback scenario needs specific test

    • Current tests verify happy path, but the double-fault scenario (restore fails + rollback fails) is critical and should be explicitly tested

---

🎨 UI/UX

✅ Strengths

1. Stale device banner is clear and actionable - good separation of partial vs. full stale states
2. Inactive conversation banner provides immediate feedback on reconnection status
3. Progress text mapping in BackupRestoreViewModel:103-114 is well thought out

🟠 Issues

12. Progress UI non-functional (already noted in error handling) - restoreProgressText always returns nil during restore because restoreState stays .idle

13. Auto-reset countdown removed from ConversationsViewModel

    • Macroscope review shows FullStaleAutoResetCountdown view exists (StaleDeviceBanner.swift:124-189)
    • But it's not used anywhere (verified in ConversationsViewModel.swift)
    • Question: Was this intentional? The view suggests it was planned as part of the stale device flow

14. No backup scheduling UI

    • Only manual "Back up now" button
    • Consider prompting user to back up after significant events (e.g., added 5+ conversations)

---

📐 Code Quality & Conventions

✅ Strengths

1. Follows CLAUDE.md conventions: No force unwraps, no implicitly unwrapped optionals, proper error handling
2. Logging is excellent - very helpful for debugging (BackupManager, RestoreManager have great log breadcrumbs)
3. Async/await usage is clean - proper task cancellation checks throughout
4. Actor isolation on BackupManager, RestoreManager, ICloudIdentityStore is correct

🟡 Minor Issues

15. Some functions are quite long

    • RestoreManager.restoreFromBackup is 134 lines (lines 81-214)
    • Consider extracting phases (decrypt, extractKeys, replaceDB, importArchives, reCreateVault) into separate methods

16. Magic numbers

    • BackupBundle tar format uses 4-byte and 8-byte headers - consider named constants
    • InboxStateMachine retry logic (line 1001) uses maxForegroundRetries = 2 - good, but similar patterns elsewhere could be centralized

17. Commented-out code in InboxStateMachine (lines 318-328)

    • XMTP v4 d14n preparation code is left in comments
    • Consider moving to a feature branch or ADR document instead of commented code

---

🔄 Stale Device Detection

✅ Implementation Quality

1. Detection logic is sound (InboxStateMachine.swift:756-779)

   • Checks isInstallationActive() after authorization and on foreground
   • Marks inbox stale in DB via InboxWriter.markStale

2. Revocation flow is correct (InboxStateMachine.swift:728-743)

   • When installationId changes, revokes all other installations
   • Prevents split-brain scenarios

3. UI filtering works (ConversationsViewModel.swift)

   • Stale conversations filtered from list
   • Banner shown appropriately

🟡 Edge Cases

18. Race condition on simultaneous multi-device foreground

    • If Device A and B both come to foreground after restore on Device C
    • Both see they're stale, both try to revoke (which fails since they're already revoked)
    • Not a critical bug, but logs may be confusing
    • Already handled gracefully with non-fatal error logging

19. Stale state persists even if installation is manually re-added

    • If user somehow re-registers the same installation (edge case), isStale flag in DB won't auto-clear
    • May need manual reset or re-authorization to clear
    • Consider TTL or re-check on successful network operations

---

📊 Performance

✅ Good Practices

1. Database backup uses GRDB's native .backup(to:) method - efficient copy-on-write
2. Streaming approach for conversation archives (one at a time, not all in memory)
3. Actor isolation prevents data races without over-locking

🟡 Potential Improvements

20. Backup could be parallelized

    • Conversation archives are created sequentially (BackupManager.swift:124-159)
    • Could use TaskGroup to parallelize, though sequential may be safer for resource constraints

21. Restore re-creates vault synchronously (RestoreManager.swift:299-337)

    • Blocks restore completion while broadcasting keys
    • Consider making this async background task post-restore

---

🔍 Cross-Reference with Macroscope Findings

• ✅ ICloudIdentityStore deletion issues (original review comments) - Fixed in c3c85e0 and c2cd5c1
• ✅ InboxWriter installationId parameter - Fixed in c3c85e0
• ✅ RestoreManager XMTP file stash cleanup - Fixed in 8165058
• ✅ BackupRestoreSettingsView stale state - Fixed in 55a451b
• ⚠️ BackupRestoreSettingsView progress tracking - Still present (comment added 2026-04-13 17:57:05)
• ⚠️ StreamProcessor reactivation error handling - Still present (comment added 2026-04-13 17:57:05)

---

✅ Summary & Recommendations

Must Fix Before Merge

1. ⚠️ Restore progress UI - Implement state observation in BackupRestoreViewModel.restore()
2. ⚠️ Database rollback double-fault - Add pre-flight validation of backup DB before destructive operations
3. ⚠️ XMTP file staging cleanup - Track moved files to avoid data loss on partial failure

Should Fix Soon (Post-Merge)

4. Add backup progress tracking (BackupManager state sequence)
5. Add integration test for full backup → restore round-trip
6. Improve conversation archive partial failure handling (threshold-based abort)
7. Clarify iCloud vs. local backup status in UI

Nice to Have

8. Extract long methods in RestoreManager for readability
9. Add explicit backup scheduling/prompting
10. Remove commented-out XMTP v4 code

Overall Assessment

This is a high-quality, well-architected PR that implements a complex feature with appropriate error handling, security, and testability. The vault-centric design is sound, encryption is industry-standard, and the rollback strategy shows careful planning.

The main concerns are:

• Restore UI progress tracking is broken (easy fix, already identified by Macroscope)
• Database replacement double-fault scenario needs hardening (add pre-flight validation)
• Partial failures in conversation archive import could be more visible (threshold-based abort)

With the three "must fix" items addressed, this PR is ready to merge. The "should fix soon" items can be follow-up work.

Test coverage is excellent (2000+ lines of new tests), and the code follows project conventions well. Great work on a complex feature! 🎉

---

[macroscopeapp]

Approvability

Verdict: Needs human review

4 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR.

^{You can customize Macroscope's approvability policy. Learn more.}

[macroscopeapp]

🟡 Medium App Settings/BackupRestoreSettingsView.swift:91

restore() only updates restoreState once after restoreFromBackup completes, so restoreState remains .idle throughout the entire restore operation. This causes restoreProgressText to always return nil, making the progress UI non-functional even though the RestoreManager broadcasts granular states during the operation.

    func restore() async {
        guard let bundleURL = availableRestoreURL, !isRestoring else { return }
        isRestoring = true
 
        do {
            let manager = try makeRestoreManager()
+            for await state in manager.stateSequence {
+                restoreState = state
+            }
-            try await manager.restoreFromBackup(bundleURL: bundleURL)
-            restoreState = await manager.state
+        } catch {
            isRestoring = false
-            restoreState = .failed(error.localizedDescription)
            alertMessage = error.localizedDescription
            showingAlert = true
        }
    }

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file Convos/App Settings/BackupRestoreSettingsView.swift around lines 91-107:

`restore()` only updates `restoreState` once after `restoreFromBackup` completes, so `restoreState` remains `.idle` throughout the entire restore operation. This causes `restoreProgressText` to always return `nil`, making the progress UI non-functional even though the `RestoreManager` broadcasts granular states during the operation.

Evidence trail:
BackupRestoreSettingsView.swift lines 91-105 (restore function), line 14 (restoreState initialization), lines 109-119 (restoreProgressText). RestoreManager.swift line 55 (state property), line 85 (state = .decrypting), lines 323, 341, 357, 385 (intermediate state updates during operation).

[macroscopeapp]

🟡 Medium Backup/RestoreManager.swift:81

finishRestore() can be invoked twice when countRestoredInboxes() throws after the success-path call on line 186. The catch block then calls it again on line 209, which starts duplicate periodic checks, schedules duplicate background tasks, and emits duplicate notifications. Consider tracking whether finishRestore was already invoked so the catch block skips the second call.

        var preparedForRestore = false
+        var finishedRestore = false

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file ConvosCore/Sources/ConvosCore/Backup/RestoreManager.swift around lines 81-91:

`finishRestore()` can be invoked twice when `countRestoredInboxes()` throws after the success-path call on line 186. The catch block then calls it again on line 209, which starts duplicate periodic checks, schedules duplicate background tasks, and emits duplicate notifications. Consider tracking whether `finishRestore` was already invoked so the catch block skips the second call.

Evidence trail:
ConvosCore/Sources/ConvosCore/Backup/RestoreManager.swift at REVIEWED_COMMIT:
- Line ~117: `preparedForRestore = true` after `prepareForRestore()` call
- Line ~186: First `finishRestore()` call inside `if preparedForRestore` in success path
- Line ~193: `try countRestoredInboxes()` which can throw
- Line ~209: Second `finishRestore()` call inside `if preparedForRestore` in catch block
- No flag to track whether `finishRestore()` was already called

[macroscopeapp]

🟠 High Backup/ConvosRestoreArchiveImporter.swift:43

If group.updateConsentState(state: .allowed) throws for any group in the loop at lines 44-46, the function exits early and subsequent groups retain unknown consent state. Per the comment at lines 38-42, StreamProcessor.shouldProcessConversation drops messages from groups without allowed consent, so these groups would silently lose their first incoming messages after restore. Consider wrapping each consent update in its own do/catch to log failures and continue processing the remaining groups.

-        for group in groups {
-            try await group.updateConsentState(state: .allowed)
-        }
+        for group in groups {
+            do {
+                try await group.updateConsentState(state: .allowed)
+            } catch {
+                Log.warning("failed to set consent for group: \(error)")
+            }
+        }

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file ConvosCore/Sources/ConvosCore/Backup/ConvosRestoreArchiveImporter.swift around lines 43-46:

If `group.updateConsentState(state: .allowed)` throws for any group in the loop at lines 44-46, the function exits early and subsequent groups retain `unknown` consent state. Per the comment at lines 38-42, `StreamProcessor.shouldProcessConversation` drops messages from groups without `allowed` consent, so these groups would silently lose their first incoming messages after restore. Consider wrapping each consent update in its own do/catch to log failures and continue processing the remaining groups.

Evidence trail:
ConvosRestoreArchiveImporter.swift lines 43-46: for loop with `try await` inside without do/catch wrapper. ConvosRestoreArchiveImporter.swift lines 38-42: comment explaining the consent state issue. StreamProcessor.swift line 673-697: `shouldProcessConversation` function returns `consentState == .allowed` at line 697. StreamProcessor.swift line 143: early return when `shouldProcessConversation` returns false. StreamProcessor.swift lines 221-223: messages skipped when `shouldProcessConversation` returns false.

[macroscopeapp]

🟠 High Conversations List/ConversationsViewModel.swift:379

In handleStaleStateTransition, the .fullStale case sets newConversationViewModel = nil and selectedConversation = nil but does not set isPendingFullStaleAutoReset = true. The doc comment states this case should "trigger an automatic local reset (countdown handled by the UI)", so the missing assignment means the countdown never starts when the device becomes fully stale.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file Convos/Conversations List/ConversationsViewModel.swift around line 379:

In `handleStaleStateTransition`, the `.fullStale` case sets `newConversationViewModel = nil` and `selectedConversation = nil` but does not set `isPendingFullStaleAutoReset = true`. The doc comment states this case should "trigger an automatic local reset (countdown handled by the UI)", so the missing assignment means the countdown never starts when the device becomes fully stale.

Evidence trail:
Convos/Conversations List/ConversationsViewModel.swift lines 344-353 (doc comment stating fullStale should trigger automatic local reset), lines 379-381 (.fullStale case implementation missing isPendingFullStaleAutoReset = true), line 385 (isPendingFullStaleAutoReset property declaration). git_grep for 'isPendingFullStaleAutoReset\s*=\s*true' returns no matches, confirming property is never set to true.