Skip to content

Add first-install iCloud pairing prompt with main-device auto-surface#983

Merged
yewreeka merged 6 commits into
devfrom
jarod/icloud-pairing-prompt
Jul 7, 2026
Merged

Add first-install iCloud pairing prompt with main-device auto-surface#983
yewreeka merged 6 commits into
devfrom
jarod/icloud-pairing-prompt

Conversation

@yewreeka

@yewreeka yewreeka commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

What

When the app is installed on a new device sharing the user's iCloud account, it now finds the other device's identity in the iCloud-synced keychain backup and offers to pair with it:

  • "Pair <device>?" info sheet on first install ("Your <device> was found in iCloud, if you have it nearby, you can pair it now") with Pair / Skip. Skip persists across launches; the check re-runs each launch to cover iCloud Keychain sync latency.
  • Pair mints a pairing invite locally, signed by the backed-up private key, and runs the standard joiner handshake (PIN + emoji + identity share) with the found inbox id as the main device - no QR scan on either side. The joiner re-sends its join request every 5s while connecting since the main device may not be listening yet.
  • The main device auto-surfaces the request: StreamProcessor verifies the join request's embedded slug against its own identity key (only a device holding the key - i.e. sharing the iCloud keychain - can mint a valid one, so strangers can't trigger this) and presents the initiator PIN sheet directly when foregrounded, or posts a local notification ("<device> is requesting to pair") otherwise.

How it stays safe

  • Key material never leaves ConvosCore; the app layer only sees inboxId/deviceName and the resulting slug.
  • The joiner's existing slug-signature and identity-share address verification hold unchanged.
  • Pairing still requires physical confirmation on the main device (PIN + emoji fingerprint) - iCloud access alone can't clone an identity.
  • Established installs keep the joiner flow's hold-to-erase guard before anything destructive.

Notes

  • The prompt is deliberately not gated on local "engagement" heuristics: fresh-install bootstrap commits a visible conversation with zero user interaction, which poisons hasAnyUsedConversations and the conversation counts (see the comment in checkForPairableDeviceIfNeeded). That pre-existing clean-install bug (visible "New Convo" rows on fresh installs) deserves its own ticket; it also makes the erase guard fire for effectively-fresh devices (affects test 38 too).
  • Remote push for the killed-app case (NSE handling of pairing join requests) is a documented follow-up; the local notification covers backgrounded-but-running.

QA

  • New two-simulator test 42-icloud-pairing-prompt (structured YAML + md, registered in the suite). Simulators don't sync iCloud Keychain, so Device B is a clone of Device A reset via the new non-production CONVOS_QA_WIPE_PRIMARY_IDENTITY launch hook (deletes only the device-local identity slot; the synced backup item survives).
  • Validated end-to-end on two simulators: prompt -> Skip persistence -> re-prompt -> Pair -> main device auto-presents the PIN in ~4s -> matching emoji fingerprints -> identity adopted (both devices report the same inboxId and the conversation list mirrors over).
  • Unit tests: invite signing round-trip/expiry/tamper, backup filtering, coordinator respond entry. Full ConvosCore suite: 1110 tests in 159 suites, all passing.

🤖 Generated with Claude Code

Note

Add first-install iCloud pairing prompt with automatic main-device detection and no-QR pairing flow

  • Adds a first-install prompt (PairFoundDeviceInfoSheet) that surfaces when another device's identity is found in the iCloud-synced keychain backup, allowing the user to pair or skip without scanning a QR code.
  • Introduces PairingNonceLedger for nonce-to-joiner binding and replay prevention, and PendingPairRequestStore for cross-process handoff of verified join requests from the Notification Service Extension to the main app.
  • StreamProcessor now fast-paths incoming PairingJoinRequestContent messages: verified requests post a .pairingDidReceiveVerifiedJoinRequest notification and bypass normal conversation processing.
  • MessagingService push handling detects verified pairing join requests from welcome pushes, constructs deduplicated pairing notifications with nonce-replay protection, and stashes requests for the main app to present.
  • PairingSheetViewModel gains a respondToJoinRequest mode that skips QR display and sends the PIN directly to the joiner; a single-active-flow gate prevents concurrent initiator sessions.
  • JoinerPairingSheetViewModel gains a periodic join-request resend loop while in .connecting, stopping when a PIN arrives or the flow ends.
  • Risk: The nonce ledger and pending-request store use UserDefaults app-group storage; if the app group suite is unavailable, the ledger falls back to in-memory only and the store falls back to standard defaults with a warning, which could cause missed deduplication across processes.

Macroscope summarized 91ba6d5.

yewreeka commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

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 Bot commented Jun 4, 2026

Copy link
Copy Markdown

Claude finished @yewreeka's task —— View job


Code Review Complete ✅

I've completed a thorough review of PR #983, examining ~2500 lines of code across 31 files. This PR introduces a significant new feature enabling iCloud-based device pairing. Here's my assessment:


Summary Assessment

Overall Quality: Strong

The PR demonstrates:

  • Well-structured security model with cryptographic verification
  • Comprehensive test coverage (37 new pairing tests + integration tests)
  • Thoughtful handling of edge cases through multiple review iterations
  • Good separation of concerns (ConvosCore business logic vs. app UI)

The feature has already undergone extensive review (Macroscope + author responses through 6 commits), and most critical issues have been addressed.


Detailed Findings

Security - Well Designed

Strengths:

  • Cryptographic verification: PairingJoinRequestDetector properly validates that join request slugs are signed by the device's own identity key (PairingJoinRequestDetectorTests.swift:29-82)
  • Replay protection: PairingNonceLedger binds nonces to first joiner, preventing slug replay attacks (PairingNonceLedger.swift:68-79)
  • Key isolation: Private keys never leave ConvosCore; app layer only sees inboxId/deviceName/slug
  • Cross-process security: App-group storage properly isolates bindings between app and NSE

Minor Concern:
The NSE scans up to 10 recent DMs with 5 messages each on every welcome push when unknown-consent DMs exist (MessagingService+PushNotifications.swift:1259-1280). This is documented in Macroscope's summary as a latency risk, but it's bounded and acceptable for the feature's value.


Code Quality & Best Practices

Adherence to CLAUDE.md conventions:

  • ✅ ConvosCore remains platform-independent (no UIKit imports)
  • ✅ No force unwrapping or implicitly unwrapped optionals
  • ✅ Proper use of guard for early returns
  • ✅ Comments are minimal and contextual (not excessive)
  • ✅ No all-caps emphasis in comments
  • ✅ Proper constant naming (Constant enum at bottom of scopes)

Architecture:

  • Clean separation: discovery logic in ConvosCore, UI in main app
  • Proper protocol usage for testability
  • Good reuse of existing pairing infrastructure

Specific observations:

  • PairableDeviceBackup.swift:36 - Excellent guard against self-pairing with nil currentInboxId (addressed in review)
  • QALaunchHooks.swift - Well-documented QA hook pattern, properly gated for non-production
  • Test coverage is comprehensive with unit + integration tests

⚠️ Potential Issues (Minor)

1. Profile Onboarding Edge Case (Convos/Conversations List/ConversationsViewModel.swift:848-855)

Already flagged by Macroscope and fixed in follow-up push: markCompletedForPairedDevice() now properly gates on non-nil displayName. Good resolution.

2. SwiftUI View Complexity (ConversationsViewModel.swift)

The ViewModel is ~1150 lines, which approaches the 625-line guideline in CLAUDE.md (line 198). While acceptable for a coordinator-style ViewModel, consider extracting pairing-specific logic to a dedicated PairingCoordinatorViewModel in a future refactor for better maintainability.

3. Notification Suppression Logic (MessagingService+PushNotifications.swift:153-180)

The pairing request notification takes precedence over welcome-outcome notifications when both occur in the same push. The code comment (lines 162-168) explains this is deliberate and state writes still complete, but this could be surprising in logs. Consider adding a metric or log line when a welcome notification is suppressed by pairing.


Performance

No significant concerns:

  • NSE DM scanning is bounded (10 DMs × 5 messages, lines 1342-1343)
  • Nonce ledger pruning is efficient (PairingNonceLedger.swift:130-132)
  • UserDefaults access for app-group storage is appropriate for small data

Good patterns:

  • Proper latch usage (didCheckForPairableDevice) prevents repeated checks
  • Dedupe window (60s) prevents notification spam (line 1341)

Test Coverage - Excellent

37 new tests across 4 suites:

  1. PairableDeviceBackupTests (3 tests) - Backup filtering, nil-current-identity guard
  2. PairingInviteSigningTests (3 tests) - Round-trip validation, expiry, tampering
  3. PairingNonceLedgerTests (5 tests) - Binding, replay protection, app-group sharing
  4. PairingJoinRequestDetectorTests (6 tests) - Signature verification, all rejection paths
  5. PendingPairRequestStoreTests (3 tests) - Stash/consume, corruption handling, replacement
  6. PairingPushDetectionIntegrationTests (1 integration test) - End-to-end NSE detection with real XMTP node

Coverage highlights:

  • ✅ Security verification paths thoroughly tested
  • ✅ Edge cases covered (nil identity, expired slugs, tampering, replay)
  • ✅ Cross-process storage validated
  • ✅ Integration test validates real-world NSE scenario

🎯 Recommendations

High Priority

None - all critical issues have been addressed through review iterations.

Medium Priority (Future Improvements)

  1. Refactor ConversationsViewModel: Extract pairing coordination to dedicated ViewModel (~200 lines of pairing-specific code could move)
  2. Add metrics: Track pairing prompt show/skip/success rates for analytics
  3. Multi-installation fan-out: Track as follow-up (author noted in review comments)

Low Priority

  1. Log welcome suppression: When pairing banner takes precedence, log which welcome outcome was suppressed for debugging
  2. Consider debounce on keychain checks: The check re-runs every activation; a short debounce could reduce redundant work

Test Execution

Per CLAUDE.md guidelines, the full test suite must pass before pushing:

Required: Run ./dev/test to validate all 1110+ tests in ConvosCore suite.

The PR description confirms: "Full ConvosCore suite: 1110 tests in 159 suites, all passing."


Final Verdict

✅ Recommend Approval with minor future improvements tracked separately.

This is a well-engineered feature with:

  • Strong security model (cryptographic verification, replay protection)
  • Excellent test coverage (37 new tests + integration coverage)
  • Thoughtful edge case handling through multiple review iterations
  • Good adherence to project conventions

The multi-round review process (Macroscope → author → deep review) has effectively caught and resolved issues. The remaining observations are minor refactoring opportunities that don't block merge.


@yewreeka yewreeka marked this pull request as ready for review June 4, 2026 16:22
Comment thread Convos/Devices/PairingSheetViewModel.swift
@macroscopeapp

macroscopeapp Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This PR introduces a new iCloud-based pairing feature with significant new user-facing workflow, security-sensitive keychain/identity handling, signature verification logic, and push notification changes. The scope and security implications warrant careful human review.

You can customize Macroscope's approvability policy. Learn more.

@yewreeka

Copy link
Copy Markdown
Contributor Author

Pushed 582df555 addressing the review findings:

🟡 Macroscope Medium — pairing service left running after a failed flow (PairingSheetViewModel.startRespondFlow)
Fixed — the catch block (and the pairingInboxId() guard) now call await pairingService.stop() before parking in .failed, mirroring confirmEmoji's handling. Also applied the same fix to startInviteFlow's catch, which had the identical unflagged gap.

🟡 Macroscope (second location) / Claude #2pairWithFoundDevice error path strands the user
Fixed, two parts:

  • On mint failure, didCheckForPairableDevice is re-armed so a later chats-list appearance offers the prompt again (previously a transient keychain/signing failure dismissed silently with the once-per-launch latch set, leaving no retry until relaunch).
  • The mid-mint dismissal race is handled: if the user swipes the prompt away while the invite is minting, assigning nil again won't re-fire the sheet's onDismiss, so the prepared joiner flow is now promoted directly instead of leaking.

Claude #1 (Critical) — "NotificationCenter post without foreground check"
Already handled, one layer up: ConversationsViewModel observes .pairingDidReceiveVerifiedJoinRequest for the app's lifetime, and handleVerifiedJoinRequest branches on applicationState — active → present the PIN sheet; otherwise → stash the request and post a local " is requesting to pair" notification, redeemed on next activation. StreamProcessor intentionally only posts; presentation policy belongs to the consumer (and ConvosCore can't reference UIKit per project rules).

Claude #3 — reviewer self-resolved (timer restart is correct as written). #7 — no force-unwrap exists; guard let generatedPin shadows the optional and all later uses are the unwrapped binding. #4 (keychain backoff), #6 (log autoclosures), #8 (typing-indicator constant) — pre-existing code/patterns outside this PR's scope. #10 — resends already stop early: onPinReceived cancels the resend task the moment the initiator's PIN arrives. #11 — manual emoji confirmation is deliberate (it's the MITM check). #12 — the 12-minute estimate is dominated by simulator cloning + reinstall in setup, matching observed runs.

Verified: SwiftLint clean, app+NSE build green, pairing unit suites pass (32 tests). #984 restacked on the amended commit with no conflicts; CI re-running on both PRs.

Comment thread Convos/Conversations List/ConversationsViewModel.swift
@yewreeka yewreeka force-pushed the jarod/icloud-pairing-prompt branch from 582df55 to edd7d72 Compare June 10, 2026 04:45
Comment thread Convos/MainTabView.swift
@yewreeka yewreeka force-pushed the jarod/icloud-pairing-prompt branch from edd7d72 to 57ece2f Compare June 10, 2026 19:10
@yewreeka

Copy link
Copy Markdown
Contributor Author

Pushed 57ece2fb addressing the two new Macroscope findings:

🟡 Double-tap race in pairWithFoundDevice()
Fixed — foundDevicePairingPrompt is now nil'ed synchronously before the mint Task spawns, so a rapid second tap hits the guard and bails: only one JoinerPairingSheetViewModel (with its live notification observers) can ever be created per prompt. Note the suggested diff in the finding had an inverted condition (if preparedFoundDevicePairing == nil { onFoundDevicePromptDismissed() } promotes nothing — the handler guards on prepared != nil); the landed fix instead tracks dismissal completion explicitly: minting and the sheet's dismissal animation race in both directions, and whichever finishes second performs the promotion via a shared idempotent promotePreparedFoundDevicePairing(). Promoting while the prompt is still animating out would drop the new presentation, which is why "just promote when the Task ends" isn't safe.

🟡 Swipe-dismiss skips triggerCancel() (MainTabView)
Fixed — the sheet now uses a custom incomingPairingBinding whose setter routes every nil-assignment through dismissIncomingPairingRequest() before the reference drops, instead of relying on onDismiss (where, as the finding correctly notes, SwiftUI has already nil'ed the item on interactive dismissal). Cancelling on every dismissal path is safe: PairingSheetViewModel.cancel() only sends the joiner-facing cancellation error from mid-handshake states (showingPin / waitingForEmojiConfirmation); terminal states just stop the service — the same cleanup the QR flow performs on sheet close.

Verified: SwiftLint clean, app+NSE build green, pairing unit suites pass (32 tests). #984 restacked cleanly; CI re-running on both PRs.

Comment thread Convos/Conversations List/ConversationsViewModel.swift
@yewreeka yewreeka force-pushed the jarod/icloud-pairing-prompt branch from 57ece2f to 164c4c8 Compare June 12, 2026 17:25
@yewreeka

Copy link
Copy Markdown
Contributor Author

Pushed 164c4c87 addressing the deep-review findings (independent agent review of the full diff):

  • Slug replay guard (M) — new PairingNonceLedger binds each verified invite nonce to the first joiner inbox that used it; a different inbox replaying a captured slug (photographed QR inside its expiry window) is dropped instead of popping an unsolicited PIN sheet, while the legit joiner's 5s resend loop matches its own binding. In-memory with 10-minute pruning (slugs expire in minutes). Unit-tested (bind/no-steal/independence). This implements the singleUse behavior PairingInvite's doc comment promised.
  • Dropped-presentation watchdog (M) — if incomingPairingRequest is assigned while another sheet is up, SwiftUI drops the presentation and the stale reference used to block every resend until app restart. The sheet's .task sets PairingSheetViewModel.active on real presentation; a request that sits un-presented past a 3s grace is now reset and re-presented on the next resend.
  • Joiner-sheet swipe-dismiss teardown (L)pendingJoinerPairing now presents via a binding wrapper routing every dismissal through dismissJoinerPairing() (cancel + ephemeral-client teardown), the same fix the initiator sheet got; previously a swipe-away orphaned the joiner's streaming task and temp database.
  • Respond-mode initial state (L) — the auto-surfaced sheet starts in the spinner state instead of flashing the empty QR layout while the service bootstraps.
  • Background notification latch (L) — the local "is requesting to pair" banner posts once per joiner instead of re-alerting on every 5s resend; the stash still refreshes so staleness keys off the latest resend.
  • QA fixes (L) — test 43's yaml asserted connecting copy the app never renders (now matches "Open Convos on …"); the md no longer claims the launch hook is "compiled out" of production (it's runtime-gated).

Deferred as a follow-up: multi-installation fan-out — with 3+ paired devices, every paired installation auto-surfaces the PIN sheet and the non-matching ones end in "code does not match". Needs a cross-installation signal; tracked separately rather than bolted on here.

Verified: lint clean, app+NSE build green, 37 pairing unit tests pass (incl. the new ledger suite). Note: local Docker is mid-migration to OrbStack on this machine, so the Docker-backed integration suite runs in CI for this push (Unit + Integration both monitored).

@yewreeka yewreeka force-pushed the jarod/icloud-pairing-prompt branch from 164c4c8 to ce3928e Compare July 6, 2026 17:28
Comment thread Convos/Conversations List/ConversationsViewModel.swift
Comment thread ConvosCore/Sources/ConvosCore/Pairing/PairableDeviceBackup.swift
Comment thread ConvosCore/Sources/ConvosCore/Syncing/StreamProcessor.swift
yewreeka and others added 4 commits July 7, 2026 09:54
When a fresh install finds another device's identity in the iCloud-synced
keychain backup, prompt "Pair <device>?" and run the standard joiner
pairing handshake against it - the found inbox id becomes this device's
identity. The invite is minted locally, signed by the backed-up key, so
the joiner's slug and identity-share verification hold unchanged and no
QR is scanned.

The main device now auto-surfaces incoming requests: StreamProcessor
verifies the join request's slug against its own identity key (only a
device sharing the iCloud keychain can mint one) and presents the
initiator PIN sheet directly when foregrounded, or posts a local
"<device> is requesting to pair" notification otherwise.

Includes QA test 42 (two-simulator clone seeding via the
CONVOS_QA_WIPE_PRIMARY_IDENTITY launch hook), accessibility ids and QA
events for automation, and unit tests for invite signing, backup
filtering, and the coordinator's respond entry.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
)

A pairing join request now reaches the user even when the app isn't
running: the NSE detects PairingJoinRequestContent in both push paths
(the welcome push that delivers the brand-new pairing DM, and encrypted
message pushes for resends), verifies the slug against this device's own
identity key, and shows "<device> is requesting to pair". The verified
request is stashed in app-group defaults (PendingPairRequestStore) and
the app presents the initiator PIN sheet from the stash on its next
activation; the stash doubles as the NSE's dedupe record against the
joiner's 5s resend cadence.

Verification logic is extracted into PairingJoinRequestDetector, shared
with the main stream's fast path, with a pure core covered by unit tests
(self-signed slug accepted; foreign-key, expired, self-sender, wrong
inbox, and garbage slugs rejected).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Persist the pairing nonce ledger to app-group storage and apply the
  replay guard in the NSE welcome path, which is a fresh process per
  push and was blind to in-memory bindings
- Close the pre-presentation race window: a freshly created pairing
  sheet VM blocks a second flow for a short grace period before it
  claims PairingSheetViewModel.active
- Recover dropped sheet presentations on the activation path too, so a
  stale never-presented sheet can't strand the NSE stash
- Return no pairable backups when the current identity is unknown,
  instead of offering a self-pair
- Decode the pending pair request before clearing it so schema drift
  can't silently destroy it
- Document why the adopted profile's PhotoKit identifier is
  deliberately not persisted on the joiner

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A welcome push that carries both a pairing join request and a join
result / new group used to return the pairing notification before the
other cases' state writes ran; a later push would no longer see either
event as new, losing them entirely. The join-result / new-group
handling is factored into welcomeOutcomeNotification and always runs;
only the banner is superseded.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@yewreeka yewreeka force-pushed the jarod/icloud-pairing-prompt branch from 69127c6 to 4e3e405 Compare July 7, 2026 17:02
Comment thread ConvosCore/Sources/ConvosCore/Pairing/PairingNonceLedger.swift Outdated
Comment thread Convos/Conversations List/ConversationsViewModel.swift
Comment thread Convos/Conversations List/ConversationsViewModel.swift
Comment thread Convos/Conversations List/ConversationsViewModel.swift Outdated
- Persist nonce-ledger bindings under per-nonce keys so concurrent
  app/NSE writes can't clobber each other's replay-guard entries
- Un-latch the found-device check when it yields to an in-flight
  deep-link flow, so the prompt can re-offer once that flow ends
- Found-device promotion yields to a deep-link pairing sheet that
  claimed the slot mid-mint instead of replacing the user's session
- Cap the stashed-request surfacing window at the shortest invite
  lifetime, and ignore duplicate join-request resends once the PIN is
  showing so the VM countdown can't drift past the coordinator's expiry

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread Convos/Conversations List/ConversationsViewModel.swift
Adopting a profile-less identity (initiator never set up a profile)
used to mark onboarding complete anyway, stranding the joiner as
"Somebody" with the prompt permanently suppressed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@yewreeka yewreeka merged commit c54c99b into dev Jul 7, 2026
15 checks passed
@yewreeka yewreeka deleted the jarod/icloud-pairing-prompt branch July 7, 2026 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant