Repository navigation
Preserve connectivity across staggered Iroh upgrades - #8196
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR refactors iOS reconnect route resolution around shared registry snapshots, adds migration handling when secure Iroh routes are unavailable, surfaces classified failure copy, and preserves legacy private-network pairing through Tailscale-only attach payloads and historical listener settings. ChangesIroh reconnect migration
Legacy private-network compatibility
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MobileShellComposite
participant DeviceRegistryService
participant PairedMacStore
participant TransportFactory
MobileShellComposite->>PairedMacStore: load saved Macs
MobileShellComposite->>DeviceRegistryService: capture registry snapshot
MobileShellComposite->>TransportFactory: attempt secure reconnect
TransportFactory-->>MobileShellComposite: local connection failure
MobileShellComposite->>DeviceRegistryService: resolve refreshed routes
DeviceRegistryService-->>MobileShellComposite: refreshed routes or missing Iroh
MobileShellComposite->>PairedMacStore: apply route upgrade or migration failure
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (22 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR preserves saved Tailscale pairings during staggered Iroh rollouts by upgrading them through a single authenticated registry snapshot rather than per-candidate requests, and adds a
Confidence Score: 5/5The change is safe to merge; the migration path is fully guarded, the pairing is only preserved (never cleared) by the new code path, and all new user-facing strings are localized for both supported locales. The registry snapshot is built once from an already-parsed listDevices() result and shared across all reconnect candidates. The macUpdateRequired failure is applied only after a second fresh store read confirms no Iroh identity, eliminating the race with a concurrent Presence write. The insecureManualRoute reclassification is intentional and correctly scoped. The workspace Package.resolved matches the existing CmuxIrohTransport pin exactly. No files require special attention; the most complex logic in MobileShellComposite.swift and MobileShellComposite+ReconnectRoutes.swift is well-guarded with scope and generation checks throughout. Important Files Changed
Reviews (3): Last reviewed commit: "refactor: centralize legacy pairing code..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobilePairingFailure.swift`:
- Around line 334-338: Update the .macUpdateRequired case in
MobilePairingFailure guidance to remove the internal “Iroh route” terminology
and tell users their saved computer will reconnect automatically after updating
cmux on the Mac, while preserving the instruction that they do not need to sign
out or pair again.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 1785-1800: The reconnect flow currently infers Mac-update
requirements from missing Iroh data instead of one authoritative result. In
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift#L1785-L1800,
replace the didSuccessfullyLoadRegistrySnapshot/foundPublishedIroh inference
with a typed confirmed outcome; at `#L2227-L2242`, initialize confirmed-missing
state only after exact route resolution succeeds; and at `#L2294-L2298`, trigger
the shared failure action only for the current-scope .confirmedMissingIroh
result.
- Around line 1737-1768: Eliminate per-candidate full snapshot rescans in the
reconnect flow: in MobileShellComposite.swift lines 1737-1768, build reusable
indexes once before iterating candidates; in DeviceRegistryService.swift lines
292-301, update the registry API to accept the selected device/instance or
provide indexed snapshot lookup; and in
MobileShellComposite+ReconnectRoutes.swift lines 181-185, use keyed row
retrieval or one indexed post-request store snapshot. Preserve existing
reconnect behavior while ensuring lookups are indexed or single-pass rather than
O(candidates × collection size).
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+ReconnectRoutes.swift:
- Around line 173-180: Update the reconnect flow around the currentMac.routes
comparison so it evaluates the authoritative current row before requiring
registryRoutes. Move DeviceRegistryService.routes resolution out of the initial
guard and into the fallback branch, allowing the newer current row to be used
when the device snapshot lacks or ambiguously matches the target. Preserve the
existing scope and forgotten-device checks.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swift`:
- Around line 666-678: Convert MobileDisconnectedFailureCopy from a static-only
namespace into an instantiable value type with immutable error and guidance
properties and an instance property exposing the combined, trimmed result.
Preserve the existing filtering and joining behavior, then update both callers
to construct MobileDisconnectedFailureCopy values and read the instance result
instead of calling combine.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: be4b73e0-18de-4551-bd31-34a25e9b811a
⛔ Files ignored due to path filters (1)
ios/cmux.xcworkspace/xcshareddata/swiftpm/Package.resolvedis excluded by!**/Package.resolved
📒 Files selected for processing (14)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/DeviceRegistryService.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobilePairingFailure.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ReconnectRoutes.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/IrohReconnectRouteSelectionTests.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobilePairingFailureTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DisconnectedWorkspaceShellView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftSources/Mobile/MobileAttachTicketStore.swiftSources/Mobile/MobileHostService.swiftcmuxTests/MobileHostIrohAdmissionTests.swiftcmuxTests/MobileHostServiceSettingsTests.swiftios/cmux/Resources/Localizable.xcstringsvendor/bonsplit
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 1788-1796: Revalidate the snapshot-derived confirmedMissingIroh
result from the current structured paired-device row before consuming it. In
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift
lines 1788-1796, reload the exact device/instance row and require current
connection, reauthentication, and Iroh state before
applyStoredMacUpdateRequiredFailure(disconnect: true); in lines 2277-2287,
reload the target row and require it to remain Tailscale-only before showing
update guidance. Ensure both paths derive reconnect authority from that single
current structured source.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+ReconnectRoutes.swift:
- Around line 266-275: Update the refreshed-route condition in the
currentMac.routes branch of listDevices so it returns .refreshedRoutes only when
reconnectRoutes includes an Iroh or permitted debug-loopback route, regardless
of requiresIroh; otherwise leave Tailscale-only routes for registry
classification and preserve the existing empty-route guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7e4344b7-c7cd-419c-aad5-c2f811aa72ac
📒 Files selected for processing (10)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/DeviceRegistryService.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobilePairingFailure.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ReconnectRoutes.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/DeviceRegistryListParsingTests.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/IrohReconnectRouteSelectionTests.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobilePairingFailureTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DisconnectedWorkspaceShellView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftios/cmux/Resources/Localizable.xcstrings
Summary
Workspace.swift.Security
Compatibility
Testing
swift test --package-path Packages/iOS/CmuxMobileShell --filter IrohReconnectRouteSelectionTestsswift test --package-path Packages/iOS/CmuxMobileShell --filter MobilePairingFailureTestsswift test --package-path Packages/iOS/CmuxMobileShell --filter DeviceRegistryRouteSelectionTestsswift test --package-path Packages/iOS/CmuxMobileShell --filter TaggedBuildRegistryRouteIsolationTestsswift test --package-path vendor/bonsplit(197 tests passed)mobile.subscribesession.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Keeps connections stable during staggered Iroh rollouts by upgrading legacy Tailscale pairings from one authoritative registry snapshot and only connecting over Iroh. Adds a centralized legacy pairing encoder for old iOS, preserves saved pairings, and shows clear “Update cmux on this Mac” guidance only when revalidated.
New Features
DeviceRegistryRouteIndexper refresh and selects routes by exact device ID + normalized instance tag; ambiguous or missing authority no longer shows an update message.macUpdateRequiredfailure that preserves the pairing and shows actionable guidance in disconnected views; UI now displays the classified reason with its guidance.CmxLegacyPrivateNetworkPairingCode: emits a tokenless, Tailscale‑only v1 attach URL with far‑future expiry, dropsauthTokenandmacUserEmail, and retainsmacUserID.Dependencies
iroh-ffiinPackage.resolved.bonsplitfork API expected by currentWorkspace.swift.Written for commit 84fc14f. Summary will update on new commits.
Summary by CodeRabbit