Fix Pair iPhone window: Cmd+W, sizing, layout, QR padding, channel scheme, failure copy#6038
Open
lawrencecchen wants to merge 8 commits into
Open
Fix Pair iPhone window: Cmd+W, sizing, layout, QR padding, channel scheme, failure copy#6038lawrencecchen wants to merge 8 commits into
lawrencecchen wants to merge 8 commits into
Conversation
…he window, not a terminal tab The pairing window's identifier is missing from cmuxAuxiliaryWindowIdentifiers, so the Close Tab shortcut falls through to the main window's tab manager and closes the active terminal tab behind the pairing window. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add cmux.mobilePairingWindow to cmuxAuxiliaryWindowIdentifiers so the configured Close Tab shortcut performs performClose on the pairing window (the same scoping Settings uses) instead of falling through to the main window's tab manager. Harden the auxiliary-window close-shortcut lint to resolve identifiers assigned through named constants (window.identifier = NSUserInterfaceItemIdentifier(Self.windowIdentifier)); the literal-only regex is exactly why the pairing window escaped the lint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Taller default (560x800) and bigger minimum (480x600) so the heading is not clipped. Manual entry (Copy IP / Copy Port) moves above the QR so it is reachable without scrolling. The QR loses the white card's 12pt padding: the bitmap already bakes the spec 4-module quiet zone, so the card doubled the visible margin; the code is capped at 380pt so the manual block, the full QR, and the waiting indicator fit the default window. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
All build variants registered and emitted the same cmux-ios scheme, so a beta/prod pairing QR scanned with the system Camera app could open a dev build that also claimed the scheme (the OS picks an arbitrary registrant). The scheme is now derived per channel from a single source of truth, CmxPairingURLScheme: development (DEBUG/tagged) builds register + emit cmux-ios-dev, Release (TestFlight beta + App Store) register + emit cmux-ios. The iOS registered scheme comes from CMUX_IOS_URL_SCHEME in Config/Shared.xcconfig (dev default) and Config/Release.xcconfig (release), interpolated into CFBundleURLSchemes in Info.plist. Emitters (the Mac building a pairing/attach URL: CmxPairingQRCode.encode, MobileSyncPairingPayload.encodedURL, the v1 fallback in MobileAttachTicketStore) use CmxPairingURLScheme.current so the system camera routes each channel's QR to its matching build. Parsers and prefix-checks (the in-app scanner, the deep-link gate, paste handling: MobilePairingScannerPolicy, MobileRootAuthGate, CmxAttachTicketInput, MobileShellComposite, PairingView) accept any channel's scheme via isPairingScheme/hasPairingScheme, so cross-channel pairing from inside the app still works. Adds CmxPairingURLSchemeTests (pure channel-to-scheme derivation) and extends the scanner-policy and auth-gate tests with the dev scheme. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… path There is no pairing-code secret anymore: the v2 QR carries bare routes and the host authorizes by Stack account, so "Invalid pairing code." was both wrong and unhelpful. It now reads "This isn't a cmux pairing QR. Scan the code shown in the Pair iPhone window on your Mac." Adds a distinct unrecognized-version path. When an attach URL declares a grammar version (v=) newer than this build understands, the decoder throws MobileSyncPairingPayloadError.unrecognizedURLVersion(version) instead of the generic invalidURL, and pairing surfaces the new .unrecognizedVersion failure category: "This QR needs a newer version of cmux. Update the app and try again." This is the real cause of the field report (beta 1.0.2 predated the v2 QR a newer Mac emitted); the wording and version-mismatch path ship even though 1.0.3 already understands v2. CmxPairingQRCode.attachURLVersion reads the v query item so a newer grammar is told apart from a malformed code. en + ja localized; adds MobilePairingFailure wording tests and CmxAttachTicketInput version tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR centralizes iOS pairing/attach URL scheme handling into CmxPairingURLScheme (dev vs release), replaces hard-coded scheme strings across encoding/decoding/UI, adds attach URL version parsing and an unrecognizedURLVersion error path, and parametrizes the scheme via build configuration and tests. ChangesMulti-channel pairing URL scheme centralization with version validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (18 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 |
Address autoreview: scripts/lib/attach-url.mjs (the debug-CLI QR renderer in mobile-attach-qr.sh and the headless dev-setup auto-pair mint) still hardcoded the release cmux-ios scheme, so a dev-rendered pairing QR scanned with the system Camera could route to an installed TestFlight/App Store build instead of the dev iOS build. buildAttachURL now takes a channel scheme defaulting to cmux-ios-dev (both callers are dev-only), mirroring CmxPairingURLScheme's default-to-dev safety; exported DEV_URL_SCHEME / RELEASE_URL_SCHEME and added test coverage for both. Also convert CmxPairingURLScheme from a caseless namespace enum to a struct with a private init, matching the sibling helpers in CMUXMobileCore (CmxMobileDefaults, CmxLoopbackHost, L10n) per the package-design policy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Cmd+W regression test adds 69 lines to AppDelegateShortcutRoutingTests and the pairing-window aux identifier adds 1 line to cmuxApp; both are required for the fix (a behavior-level regression test and the close-shortcut scoping). Rebaselined the checked-in budget so workflow-guard-tests passes; the other deltas track the rebase onto origin/main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address autoreview: - ios/cmuxPackage cmuxFeatureTests asserted the old "Invalid pairing code." literal in two places (the expired pair?v=1 path and the loopback-rejected negative check). Now assert against MobilePairingFailureCategory.invalidCode.message so the tests track the copy instead of breaking on every reword. - scripts/mobile-stability-soak/mobile-soak.py minted cmux-ios://attach and fed it to `simctl openurl`, which routes by the simulator app's registered CFBundleURLSchemes. Dev builds now register cmux-ios-dev, so in MOBILE_REATTACH_MODE=openurl the link would miss the tagged dev app (or open an installed release/beta build). The soak now mints the dev scheme by default (override with MOBILE_ATTACH_URL_SCHEME for release soaks). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Greptile SummaryThis PR bundles six related fixes to the Pair iPhone surface: Cmd+W now closes the pairing window (not a terminal tab) by registering
Confidence Score: 5/5All six fixes are self-contained, well-tested, and leave no bad state representable; the channel-scheme model has a single source of truth, and parsers stay cross-channel so no pairing path is broken. The changes touch deep-link routing and iOS URL-scheme registration — the highest-risk areas — but every emitter, parser, and xcconfig entry is consistently updated, and the test suite covers scheme derivation, cross-channel decoding, version-mismatch error paths, and the Cmd+W regression end-to-end. Localization is complete for all new strings in both supported locales. No actor-isolation, blocking-runtime, or SwiftUI state issues introduced. No files require special attention; the xcconfig chain (Shared → Debug/Release) and the Swift CmxPairingURLScheme constants should be kept in sync if a new build configuration is added in the future. Important Files Changed
Sequence DiagramsequenceDiagram
participant Mac as macOS App
participant Scheme as CmxPairingURLScheme
participant QR as QR Code / Camera
participant iOS as iOS App
Note over Mac,Scheme: Emitter path (channel-specific)
Mac->>Scheme: CmxPairingURLScheme.current
Scheme-->>Mac: cmux-ios-dev (DEBUG) or cmux-ios (Release)
Mac->>QR: "scheme://attach?v=2&r=..."
Note over QR,iOS: Camera scan (system routes by registered scheme)
QR->>iOS: Opens matching channel's app via CFBundleURLSchemes
Note over iOS,Scheme: Parser path (accepts all channels)
iOS->>Scheme: CmxPairingURLScheme.isPairingScheme(url.scheme)
Scheme-->>iOS: true if cmux-ios OR cmux-ios-dev
alt "version > CmxPairingQRCode.version"
iOS-->>iOS: unrecognizedURLVersion - Update the app
else valid v2 grammar
iOS-->>iOS: Decode routes, connect
else not a cmux QR
iOS-->>iOS: invalidCode - Scan the Pair iPhone window QR
end
Reviews (1): Last reviewed commit: "Pair iPhone: fix stale invalid-code test..." | Re-trigger Greptile |
![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
🤖 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 `@scripts/mobile-stability-soak/mobile-soak.py`:
- Around line 146-147: Read and validate the ATTACH_URL_SCHEME value right after
it's read (ATTACH_URL_SCHEME = os.environ.get(...).strip()) by checking it is
one of the allowed schemes ("cmux-ios-dev" or "cmux-ios"); if it's empty or not
in that allowlist, fail fast with a clear error (raise ValueError or log error
and sys.exit(1)). Also apply the same guard where the script builds attach/open
URLs (any code using ATTACH_URL_SCHEME at the later URL construction) so
invalid/typoed overrides cannot produce bad openurl targets.
🪄 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: d2981ccc-3d8e-477a-a4c6-91239dbcabd3
📒 Files selected for processing (2)
ios/cmuxPackage/Tests/cmuxFeatureTests/cmuxFeatureTests.swiftscripts/mobile-stability-soak/mobile-soak.py
Comment on lines
+146
to
+147
| ATTACH_URL_SCHEME = os.environ.get("MOBILE_ATTACH_URL_SCHEME", "cmux-ios-dev").strip() | ||
|
|
There was a problem hiding this comment.
Validate MOBILE_ATTACH_URL_SCHEME before building attach URLs.
At Line 146 and Line 153, an empty/typoed override can silently produce invalid openurl targets. Guard to the known schemes (cmux-ios-dev, cmux-ios) and fail fast with a clear error.
Proposed fix
-ATTACH_URL_SCHEME = os.environ.get("MOBILE_ATTACH_URL_SCHEME", "cmux-ios-dev").strip()
+ATTACH_URL_SCHEME = os.environ.get("MOBILE_ATTACH_URL_SCHEME", "cmux-ios-dev").strip()
+VALID_ATTACH_URL_SCHEMES = {"cmux-ios-dev", "cmux-ios"}
+if ATTACH_URL_SCHEME not in VALID_ATTACH_URL_SCHEMES:
+ raise RuntimeError(
+ "MOBILE_ATTACH_URL_SCHEME must be one of "
+ f"{sorted(VALID_ATTACH_URL_SCHEMES)}; got {ATTACH_URL_SCHEME!r}"
+ )Also applies to: 153-153
🤖 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 `@scripts/mobile-stability-soak/mobile-soak.py` around lines 146 - 147, Read
and validate the ATTACH_URL_SCHEME value right after it's read
(ATTACH_URL_SCHEME = os.environ.get(...).strip()) by checking it is one of the
allowed schemes ("cmux-ios-dev" or "cmux-ios"); if it's empty or not in that
allowlist, fail fast with a clear error (raise ValueError or log error and
sys.exit(1)). Also apply the same guard where the script builds attach/open URLs
(any code using ATTACH_URL_SCHEME at the later URL construction) so
invalid/typoed overrides cannot produce bad openurl targets.
lawrencecchen
added a commit
that referenced
this pull request
Jun 13, 2026
…dding/channel-scheme/failure-copy, #6038) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Fixes the cluster of Pair iPhone window bugs Lawrence hit while dogfooding. All six live on the same surface, so they ship in one branch.
The six fixes
1. Cmd+W closed the active terminal tab instead of the pairing window. The pairing window is an auxiliary
NSWindow, but its identifier was missing fromcmuxAuxiliaryWindowIdentifiers, so the configured Close Tab shortcut fell through to the main window's tab manager. Addedcmux.mobilePairingWindowto that set (same scoping Settings uses), so Cmd+W now performsperformCloseon the pairing window. Also hardenedscripts/lint_auxiliary_window_close_shortcuts.pyto resolve identifiers assigned through named constants (window.identifier = NSUserInterfaceItemIdentifier(Self.windowIdentifier)) — the literal-only regex is exactly why the pairing window escaped the lint. Two-commit red/green regression testtestCmdWClosesMobilePairingWindowInsteadOfTerminalTabproves Cmd+W closes the window while the main window's tab survives.2. Bigger default window size. "Pair your iPhone" was clipped and Copy IP/Port sat below the fold. Default is now 560x800 (was 540x720) with a 480x600 minimum.
3. Manual-add block moved above the QR. "Can't scan? Add this Mac manually" plus IP:port and Copy IP / Copy Port now render above the QR, so the copy controls are reachable without scrolling.
4. QR quiet zone was doubled. The bitmap bakes the spec 4-module quiet zone, and the white card added its own 12pt margin on top, doubling the visible quiet zone. Dropped the card padding so the visible quiet zone is the spec 4 modules. Still pure black/white, nearest-neighbor (
.interpolation(.none)), spec quiet zone baked in, so it still scans.5. iOS URL-scheme hijack: scheme is now channel-specific. Every build registered and emitted the same
cmux-iosscheme, so a beta/prod pairing QR scanned with the system Camera app could open a dev build that also claimed the scheme (the OS picks an arbitrary registrant). The scheme is now derived per channel from one source of truth,CmxPairingURLScheme:reload.shtagged) builds register + emitcmux-ios-dev.cmux-ios.The iOS registered scheme comes from
CMUX_IOS_URL_SCHEMEinConfig/Shared.xcconfig(dev default — an unknown config can never hijack release links) andConfig/Release.xcconfig(release), interpolated intoCFBundleURLSchemesinInfo.plist. Emitters (CmxPairingQRCode.encode,MobileSyncPairingPayload.encodedURL, the v1 fallback inMobileAttachTicketStore) useCmxPairingURLScheme.currentso the system camera routes each channel's QR to its matching build. Parsers and prefix-checks (MobilePairingScannerPolicy,MobileRootAuthGate,CmxAttachTicketInput,MobileShellComposite,PairingView) accept any channel's scheme viaisPairingScheme/hasPairingScheme, so cross-channel pairing from inside the app still works. The predecessor's first pass updated the v2 emitter and two parsers; this completes it (the v1 fallback emitter inMobileAttachTicketStorestill hardcodedcmux-ios, and five parser/prefix-check sites still hardcoded the scheme).6. "Invalid pairing code" wording was wrong; added a version-mismatch path. There is no pairing-code secret anymore (the v2 QR carries bare routes and the host authorizes by Stack account, confirmed in
CmxPairingQRCodev=2 +MobileCoreRPCClient)..invalidCodenow reads "This isn't a cmux pairing QR. Scan the code shown in the Pair iPhone window on your Mac." A new.unrecognizedVersionfailure category covers an attach URL whose grammar version (v=) is newer than this build understands — the decoder throwsMobileSyncPairingPayloadError.unrecognizedURLVersion(version)(distinct from the genericinvalidURL) and the UI says "This QR needs a newer version of cmux. Update the app and try again." That was the real field report (beta 1.0.2 predated the v2 QR a newer Mac emitted, fixed in 1.0.3); the wording and version-mismatch path still ship.Channel-scheme model
One source of truth,
CmxPairingURLScheme, maps the compile channel to a scheme:currentreturnscmux-ios-devin DEBUG andcmux-iosin Release. Emitters usecurrent; parsers acceptallchannels' schemes. The iOS bundle registers the matching scheme via the xcconfig var so the system camera routes each channel's QR to its build, while the in-app scanner stays cross-channel.Tests
CmxPairingURLSchemeTests(new): pure channel-to-scheme derivation, cross-channel parsing, foreign-scheme rejection.CmxAttachTicketInputTests(new cases): decodes a pairing code from any channel scheme; newer grammar version throwsunrecognizedURLVersion; known version does not.MobilePairingFailureTests(new cases):.invalidCodeno longer mentions a "pairing code";.unrecognizedVersiontells the user to update; exhaustive non-empty-message coverage includes the new category.MobilePairingScannerPolicyTests/MobileRootAuthGateTests: extended with the dev scheme.CmxPairingQRCodeTests: emitter round-trip assertions now trackCmxPairingURLScheme.currentso they are correct in both Debug and Release runs.AppDelegateShortcutRoutingTests.testCmdWClosesMobilePairingWindowInsteadOfTerminalTab(regression, two-commit red/green).Localization audit
The failure-message strings resolve from the iOS app catalog (
ios/cmux/Resources/Localizable.xcstrings, viaL10n/Bundle.main). Added/updatedmobile.pairing.invalidCode,mobile.pairing.unrecognizedVersion, andmobile.pairing.guidance.updateAppwith en + ja. The macOS pairing-window UI strings resolve fromResources/Localizable.xcstrings; audited all 31 referencedmobile.pairing.*keys for en + ja coverage (all present). Both catalogs validated as JSON.Dogfood
macOS: Open the Pair iPhone window. Confirm "Pair your iPhone" is fully visible and Copy IP / Copy Port are above the fold without scrolling. Press Cmd+W in the pairing window: it should close the window, leaving the terminal tab behind it intact (previously Cmd+W closed the active tab). QR should still scan from a phone.
iOS scheme: A dev Mac's QR opens the dev iOS build; a beta/prod Mac's QR opens the release build (previously a beta QR scanned with the Camera app could open a dev build).
Failure wording: Scanning a non-cmux QR shows "This isn't a cmux pairing QR…" not "Invalid pairing code."
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches deep-link registration, QR emission, and attach decoding across Mac/iOS and dev tooling—wrong scheme or parser gaps would break pairing; macOS Cmd+W routing is localized to auxiliary window IDs with lint and tests.
Overview
Pair iPhone (macOS) — Registers the pairing window in
cmuxAuxiliaryWindowIdentifiersso Cmd+W closes that window instead of a terminal tab; adds a regression test and extends the auxiliary-window lint to resolve constant-based window identifiers. Layout/sizing: larger default window (560×800), manual IP/port block above the QR, and less extra padding around the QR so the spec quiet zone isn’t doubled.Channel-specific pairing URLs — Introduces
CmxPairingURLScheme(cmux-iosrelease,cmux-ios-devdebug): emitters usecurrentfor QRs/attach links; parsers accept all schemes for in-app cross-channel pairing. iOS registers the scheme viaCMUX_IOS_URL_SCHEMEin xcconfig/Info.plist. Hardcodedcmux-ioschecks are replaced across mobile core, RPC, shell, workspace, Mac attach ticket store, and dev scripts (attach-url.mjs, soak, QR shell).Pairing failures (iOS) — New
unrecognizedURLVersionwhen attachv=is newer than the app understands; updated invalid QR copy (no “pairing code” wording). Shell maps the new error to.unrecognizedVersionwith update guidance; strings added inLocalizable.xcstrings.Reviewed by Cursor Bugbot for commit f417fc3. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes the Pair iPhone window and makes pairing links channel-specific across Mac, iOS, and dev tooling so Camera scans open the right build and errors are clearer.
cmux-ios-dev, release usescmux-ios. Emitters useCmxPairingURLScheme.current; parsers accept all schemes for in-app cross-channel pairing.Info.plistreads$(CMUX_IOS_URL_SCHEME)from xcconfig.scripts/lib/attach-url.mjsemitscmux-ios-devby default (override viaRELEASE_URL_SCHEME);scripts/mobile-attach-qr.shcopy updated;scripts/mobile-stability-soak/mobile-soak.pymints dev-scheme URLs by default (overrideMOBILE_ATTACH_URL_SCHEME).MobilePairingFailureCategory.invalidCode.messageand cover the new scheme/version logic.Written for commit f417fc3. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX