Demote QR pairing from primary surfaces - #8821
Merged
Merged
Conversation
Make same-account iroh discovery the primary path by hiding the macOS titlebar pairing button by default, stopping iOS from auto-presenting Add Computer, and renaming the disconnect-and-hide action to Forget This Computer.\n\nKeep QR pairing available as a manual fallback through explicit controls, and update the macOS/iOS copy and English/Japanese localizations to lead with automatic discovery.
|
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 changes replace rescan actions with “Forget This Computer,” remove automatic add-device presentation after paired-Mac loading, update connection and empty-state guidance, disable the unavailable-flag fallback, and add content-driven sizing for the macOS pairing window. ChangesMobile connection behavior
Pairing window layout
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MobileSettingsView
participant WorkspaceShellView
participant MobileShellComposite
User->>MobileSettingsView: Select Forget This Computer
MobileSettingsView->>WorkspaceShellView: Invoke forgetComputer
WorkspaceShellView->>MobileShellComposite: Call disconnectAndHideActiveMac()
MobileShellComposite-->>WorkspaceShellView: Disconnect and hide active Mac
sequenceDiagram
participant MobilePairingView
participant MobilePairingWindowController
participant NSWindow
MobilePairingView->>MobilePairingWindowController: Report measured content height
MobilePairingWindowController->>NSWindow: Resize and clamp pairing window
MobilePairingWindowController->>NSWindow: Bring window to front
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (21 passed)
✨ Finishing Touches🧪 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 |
Contributor
Greptile SummaryThis PR demotes QR pairing from primary macOS and iOS surfaces so that same-account zero-touch discovery is the first path users see. The QR path remains reachable from Settings ▸ Mobile and the command palette.
Confidence Score: 5/5Safe to merge. All changed paths are UI surface changes with no correctness-critical state affected. The window-resize mechanism is correctly wired: ReleasingWindowController.installManagedWindow sets window.delegate = self, so the live-resize guard delegate methods fire correctly. The GeometryReader-in-background PreferenceKey pattern does not disturb layout. The feature flag default and its FLAG comment's defaultWhenUnavailable field are updated consistently. All three MobileSettingsView call sites drop the removed rescanQR parameter. Both string catalogs carry EN and JA entries for every changed string. The deleted tests exactly cover the removed behavior. Files Needing Attention: No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User opens pairing window] --> B[MobilePairingWindowController.show]
B --> C{Window exists?}
C -->|No| D[makeWindow: 560x800 initial size]
C -->|Yes| E[Reuse existing window]
D --> F[resizeWindowToIdealContentHeight\nidealContentHeight nil → no-op]
E --> F2[resizeWindowToIdealContentHeight\napply cached height]
F --> G[makeKeyAndOrderFront]
F2 --> G
G --> H[SwiftUI renders MobilePairingView]
H --> I[GeometryReader in .background\nmeasures content height]
I --> J[MobilePairingContentHeightPreferenceKey\nonPreferenceChange fires]
J --> K[pairingContentHeightDidChange]
K --> L{isUserResizing?}
L -->|Yes| M[Skip resize — user is dragging]
L -->|No| N[resizeWindowToIdealContentHeight\nclamp to screen visible frame]
N --> O[window.setFrame — keep top edge,\ngrow down, shift up if clipped]
Reviews (6): Last reviewed commit: "Clamp programmatic pairing-window resize..." | Re-trigger Greptile |
The Pair iPhone window now grows to its content's ideal height (clamped to the screen's visible frame, scroll kept for short displays) so the legacy Tailscale code link below the QR is discoverable without scrolling. Settings and the workspace overflow menu lose the active-Mac-only forget item; the Computers sheet gains a destructive Forget All Computers action (confirmation dialog) backed by a new forgetAllComputers() that disconnects and hides every stored pairing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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`+HiddenMacs.swift:
- Around line 186-217: Update forgetAllComputers() in
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+HiddenMacs.swift
(lines 186-217) to compare both macDeviceID and the active instance tag in its
fallback contains check, using the available activeTicket/activeInstanceTag
state. Add coverage in
Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileShellCompositeHideMacTests.swift
(lines 195-247) for an active tag absent from stored pairings while a sibling
tag for the same Mac is present.
- Around line 193-195: Update the forget-all flow around
storedPairedMacsIncludingHidden so it always awaits loadPairedMacs() before
computing the set of pairings to disconnect and hide, rather than refreshing
only when the in-memory list is empty. Use the freshly loaded authoritative
pairing data for the entire destructive operation.
In
`@Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileShellCompositeHideMacTests.swift`:
- Around line 195-247: The test
forgetAllComputersDisconnectsAndHidesVisibleHiddenAndTaggedPairings must also
cover an active instance tag absent from storedPairedMacsIncludingHidden while
another tag for the same Mac remains stored. Add a distinct active tagged
pairing and a different stored tag for that physical Mac, then assert
forgetAllComputers() hides the active pairing ID alongside all stored pairings
and clears the displayed list.
In `@Sources/Mobile/Pairing/MobilePairingWindowController.swift`:
- Around line 97-123: Update resizeWindowToIdealContentHeight(_:) to clamp the
computed target frame dimensions against window.contentMinSize before calling
setFrame(_:display:), preserving the intended minimum 480x320 floor while still
respecting the visible screen frame and existing positioning behavior.
🪄 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 Plus
Run ID: 4177bb72-7e2f-4cfb-9ef0-c5dbbbc3761a
📒 Files selected for processing (10)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+HiddenMacs.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileShellCompositeHideMacTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DeviceTreeView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DisconnectedWorkspaceShellView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileSettingsView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView.swiftSources/Mobile/Pairing/MobilePairingView.swiftSources/Mobile/Pairing/MobilePairingWindowController.swiftios/cmux/Resources/Localizable.xcstrings
💤 Files with no reviewable changes (3)
- Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DisconnectedWorkspaceShellView.swift
- Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swift
- Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView.swift
Centered red text with no icon or card background, so the rare whole-phone action stops competing with the computer rows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
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/DeviceTreeView.swift (1)
108-116: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftSerialize and own the “Forget All Computers” path.
Task { await store.forgetAllComputers() }starts durable disconnect/hide work without retention or cancellation, and the.taskrefresh can queue another paired-Mac write while that work is in progress. Move the whole sequence into the store’s serialization boundary (or retain/cancel a store-owned operation) to avoid interleavingforgetAllComputers()with store reloads or reconnect writes.🤖 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/DeviceTreeView.swift` around lines 108 - 116, Update the “Forget All Computers” action in DeviceTreeView so the operation is owned and serialized by the store rather than launching an unretained Task. Route forgetAllComputers() through the store’s existing serialization boundary, coordinating reload and reconnect writes so they cannot interleave while the forget operation is running.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.
Outside diff comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DeviceTreeView.swift`:
- Around line 108-116: Update the “Forget All Computers” action in
DeviceTreeView so the operation is owned and serialized by the store rather than
launching an unretained Task. Route forgetAllComputers() through the store’s
existing serialization boundary, coordinating reload and reconnect writes so
they cannot interleave while the forget operation is running.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 87fee436-45d5-40f9-92ac-7fc3755c4fb3
📒 Files selected for processing (1)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DeviceTreeView.swift
Per-computer hide on the Computers sheet rows remains the way to drop a Mac; the whole-phone destructive action, its confirmation dialog, forgetAllComputers(), its test, and its strings are removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
setFrame(_:display:) bypasses contentMinSize, so a short in-flight content measurement (the loading spinner) could shrink the window below the 480x320 floor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
3 tasks
This was referenced Aug 3, 2026
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
1 participant
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.
QR pairing is iroh-first (the default QR payload is identity-only; the Tailscale
host:portcode is the legacy fallback behind "Pair an Older iPhone App"), and same-account zero-touch discovery is the primary pairing path. This PR makes every pairing surface read that way.macOS
mobileConnectButtonDefault = false); PostHog can still re-enable it remotely. Pairing stays reachable via Settings ▸ Mobile and the command palette.iOS
disconnectAndHideActiveMac()), and the workspace overflow menu loses its twin. Per-computer hide on the Computers sheet rows remains the way to drop a Mac; a whole-phone "Forget All Computers" action was built and then omitted per owner decision.All changed strings localized (EN + JA). Focused tests:
MobileShellCompositeHideMacTests16/16 green.Summary by CodeRabbit
New Features
Improvements