Open iOS Tailscale pairing on strict method selection - #9720
Closed
azooz2003-bit wants to merge 6 commits into
Closed
azooz2003-bit wants to merge 6 commits into
azooz2003-bit wants to merge 6 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe iOS connection flow checks active Mac Tailscale route authorization before committing a connection method. Unauthorized Tailscale selections remain selected while pairing starts. Authorized selections commit immediately. ChangesTailscale method flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MobileShellComposite
participant MobileConnectionMethodSection
participant MobileConnectionMethodStore
participant PairingScanner
MobileShellComposite->>MobileConnectionMethodSection: provide authorized Tailscale route status
MobileConnectionMethodSection->>MobileConnectionMethodStore: request selected connection method
MobileConnectionMethodStore-->>MobileConnectionMethodSection: return pairing required
MobileConnectionMethodSection->>PairingScanner: start Tailscale pairing
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (23 passed)
✨ Finishing Touches 💡 1📝 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 |
azooz2003-bit
force-pushed
the
fix-ios-tailscale-disconnect
branch
from
August 6, 2026 21:30
1363a66 to
d704a12
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift (1)
294-306: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNon-Tailscale connect while a Tailscale selection is pending leaves
pendingMethodstuck.If
connectionStatebecomes.connectedthrough a route other than Tailscale whilependingMethod == .tailscale(for example, Auto-Connect succeeds in the background while the pairing sheet is open), this handler dismisses the add-device sheet but never callscancelPendingMethod().store.methodstays.automatic, butpresentedMethodkeeps returning.tailscale, so Settings keeps showing "Tailscale Only" selected even though the app connected through a different method. The pending state stays stuck until the user manually re-selects a method.Cancel the pending selection on the same transition when the active route is not Tailscale.
🛡️ Proposed fix to keep pending state in sync with the sheet dismissal
.onChange(of: store.connectionState) { _, connectionState in if connectionState == .connected { if store.activeRoute?.kind == .tailscale { connectionMethodStore?.commitPendingTailscaleMethod() + } else { + connectionMethodStore?.cancelPendingMethod() } isShowingAddDeviceSheet = false } else { clearAttachTicketAuthenticationIfNeeded() }Add a test covering: stage an unauthorized Tailscale selection, then transition
connectionStateto.connectedvia a non-Tailscale route, and assertpresentedMethodreturns to.automatic.🤖 Prompt for 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. In `@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift` around lines 294 - 306, Update the connected branch of the connectionState change handler to cancel the pending method when store.activeRoute?.kind is not .tailscale, while retaining commitPendingTailscaleMethod() for Tailscale connections. Add a test that stages an unauthorized Tailscale selection, connects through a non-Tailscale route, and verifies presentedMethod returns to .automatic.Source: Path instructions
🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileConnectionMethodSection.swift`:
- Around line 55-71: The methodSelection setter must always call
store.request(method, hasAuthorizedTailscaleRoute:) regardless of whether
startPairingScanner exists. Remove the direct store.method assignment, pass the
optional scanner into the request result flow, and invoke startPairingScanner
only when the request requires pairing.
---
Outside diff comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift`:
- Around line 294-306: Update the connected branch of the connectionState change
handler to cancel the pending method when store.activeRoute?.kind is not
.tailscale, while retaining commitPendingTailscaleMethod() for Tailscale
connections. Add a test that stages an unauthorized Tailscale selection,
connects through a non-Tailscale route, and verifies presentedMethod returns to
.automatic.
🪄 Autofix
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 Plus
Run ID: b06b9672-6a56-4a79-b273-f0a7414f2a32
📒 Files selected for processing (8)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ConnectionMethod.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ReconnectRoutes.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileConnectionMethodStore.swiftPackages/iOS/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/MobileConnectionMethodStoreTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileConnectionMethodSection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileSettingsView.swiftios/cmuxUITests/cmuxUITests.swift
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
azooz2003-bit
changed the title
…connect # Conflicts: # ios/cmuxUITests/cmuxUITests.swift
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
This was referenced Aug 10, 2026
lawrencecchen
added
the
stale-revisit
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cause: selecting Tailscale Only without an exact device-local route grant made the strict choice active, but did not lead the user into the required pairing flow.
Fix: persist Tailscale Only immediately, disconnect non-Tailscale transports as requested, and open the QR scanner when the active Mac lacks a Tailscale grant. Cancelling or failing pairing leaves Tailscale Only selected so the failure stays visible. Existing authorized routes switch without reopening pairing.
Verification:
swift test --package-path Packages/iOS/CmuxMobileShellModel --filter MobileConnectionMethodStoreTestsNote
Medium Risk
Changes connection routing UX and persists Tailscale Only before pairing completes, which can trigger existing connection-method recovery and leave the app disconnected until pairing succeeds; scope is mobile settings/onboarding, not auth core.
Overview
Selecting Tailscale Only now persists the choice immediately and, when the active Mac has no exact device-local Tailscale grant, opens the QR pairing scanner from onboarding, Settings, and onboarding replay. If the user cancels pairing, Tailscale Only stays selected so the missing authorization remains visible instead of silently reverting.
MobileConnectionMethodStore.request(_:hasAuthorizedTailscaleRoute:)centralizes that behavior: it saves the method and returns whether the UI must present pairing. Settings and root onboarding wire the connection-method picker through a custom binding that callsrequestand launches the scanner ontrue.The shell exposes
activeMacHasAuthorizedTailscaleRoute, derived from stored reconnect routes and Tailscale grant metadata for the active paired Mac, so UI layers do not duplicate route logic. When a grant already exists, selecting Tailscale Only commits without reopening pairing.Unit tests cover unauthorized vs authorized Tailscale requests and switching back to Auto-Connect; a UI test expects the scanner after Tailscale selection and confirms selection after cancel.
Reviewed by Cursor Bugbot for commit 6edc4d1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit