Repository navigation
Slim the pairing QR: compact attach payload, drop vestigial auth_token, ECC M to L - #5727
Merged
Merged
Conversation
Replace the CmxAttachTicketCompactCoding namespace enum with an injectable CmxAttachTicketCompactCoder struct, move each compact DTO (CompactAttachTicket, CompactAttachRoute, CompactAttachEndpoint) into its own file, and turn the pure private static helpers into file-scope private functions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces a compact JSON wire format for attach ticket QR payloads, adds short-key Codable DTOs, a public compact coder with grammar detection, integrates compact encoding/decoding into attach URL generation and input parsing, updates QR ECC, and adds comprehensive tests. ChangesCompact Attach Ticket QR Payload
Sequence DiagramsequenceDiagram
participant Client
participant MobileAttachTicketStore
participant CompactCoder
participant CmxAttachTicketInput
participant QRImageView
Client->>MobileAttachTicketStore: attachURL(ticket)
MobileAttachTicketStore->>CompactCoder: encode(ticket)
CompactCoder-->>MobileAttachTicketStore: compact Data (auth token omitted)
MobileAttachTicketStore-->>Client: cmux-ios://attach?payload=base64url(compact)
Client->>CmxAttachTicketInput: decode(URL)
CmxAttachTicketInput->>CompactCoder: isCompactPayload(data)
CompactCoder-->>CmxAttachTicketInput: Bool
alt compact
CmxAttachTicketInput->>CompactCoder: decode(data)
CompactCoder-->>CmxAttachTicketInput: CmxAttachTicket
else legacy
CmxAttachTicketInput->>CmxAttachTicketInput: JSON decode with ISO8601
end
CmxAttachTicketInput-->>Client: ticket
Client->>QRImageView: render(qr payload)
QRImageView->>QRImageView: setECC to "L"
QRImageView-->>Client: QR image
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 20 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (20 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Greptile SummaryThis PR slims the pairing QR by switching the attach payload from a verbose full-key Codable JSON to a compact short-key grammar (unix expiry, omitted empty fields, no
Confidence Score: 5/5Safe to merge — the dual-grammar routing is correct, validation runs exactly once on both paths, and the compatibility matrix is locked by tests covering all four scan combinations. The wire-format change is backward-compatible in both directions: new decoders accept both grammars, and old decoders fail loudly on compact payloads rather than silently misreading them. Auth is unchanged (Stack token remains the sole gate). The ECC reduction from M to L is appropriate for screen-to-camera use. Tests cover round-trips, grammar discrimination, error cases, and a payload-size ceiling. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Mac as MobileAttachTicketStore (Mac)
participant QR as MobilePairingQRImageView
participant Phone as CmxAttachTicketInput (Phone)
participant CompactCoder as CmxAttachTicketCompactCoder
Mac->>CompactCoder: encode(ticket) — compact short-key JSON, no auth_token
CompactCoder-->>Mac: Data
Mac->>Mac: "base64URLEncode → cmux-ios://attach?v=1&payload=…"
Mac->>QR: render URL as QR (ECC L)
Note over Phone: Scans QR or receives URL
Phone->>Phone: base64URLDecode(payload) → Data
Phone->>CompactCoder: isCompactPayload(data) — checks top-level "v" key
alt compact grammar (new Mac QR)
CompactCoder-->>Phone: true
Phone->>CompactCoder: decode(data)
CompactCoder-->>Phone: "CmxAttachTicket (authToken = nil)"
else legacy grammar (old Mac / dev script)
CompactCoder-->>Phone: false
Phone->>Phone: JSONDecoder (iso8601) → CmxAttachTicket (authToken preserved)
end
Phone->>Phone: ticket.validate() — expiry, required fields
Phone-->>Phone: validated CmxAttachTicket
Reviews (3): Last reviewed commit: "Merge branch 'main' into feat-ios-qr-sli..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| guard let object = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else { | ||
| return false | ||
| } | ||
| return object["v"] != nil |
Contributor
There was a problem hiding this comment.
object["v"] != nil, but JSONSerialization maps a JSON null value to NSNull, so a payload like {"v":null,...} passes the check and is routed to the compact decoder — which then throws a DecodingError for the wrong reason rather than falling through to the legacy decoder. Checking for an integer value keeps the routing intent unambiguous.
Suggested change
| return object["v"] != nil | |
| return object["v"] as? Int != nil |
| return try encoder.encode(CompactAttachTicket(ticket)) | ||
| } | ||
|
|
||
| /// Decode a compact JSON payload into a validated ``CmxAttachTicket``. |
Contributor
There was a problem hiding this comment.
validate() call (expiry, required fields, etc.) is the caller's responsibility and is done in CmxAttachTicketInput.decode. A caller who reads the doc comment may reasonably skip calling validate() on the returned ticket, accepting an expired or otherwise invalid ticket.
Suggested change
| /// Decode a compact JSON payload into a validated ``CmxAttachTicket``. | |
| /// Decode a compact JSON payload into a ``CmxAttachTicket``. | |
| /// Callers are responsible for calling ``CmxAttachTicket/validate()`` before use. |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
lawrencecchen
added a commit
that referenced
this pull request
Jun 9, 2026
Every distinct way a scanned pairing code can fail must surface its own actionable walk-through message through the shared connectionError surface from #5713, and a scan performed while a live session exists must never tear that session down. Red on this commit (fix follows): - expired code says "This code expired" + how to mint a fresh one, instead of the dead-end "Invalid pairing code." - a compact short-key payload (top-level "v", the newer-Mac QR grammar from #5727) says "update cmux on this device" loudly instead of "invalid code" - a pair-grammar payload with a newer version says the same - unreadable garbage keeps an actionable refresh-code instruction - scanning a bad code while connected keeps the live session Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 10, 2026
Every distinct way a scanned pairing code can fail must surface its own actionable walk-through message through the shared connectionError surface from #5713, and a scan performed while a live session exists must never tear that session down. Red on this commit (fix follows): - expired code says "This code expired" + how to mint a fresh one, instead of the dead-end "Invalid pairing code." - a compact short-key payload (top-level "v", the newer-Mac QR grammar from #5727) says "update cmux on this device" loudly instead of "invalid code" - a pair-grammar payload with a newer version says the same - unreadable garbage keeps an actionable refresh-code instruction - scanning a bad code while connected keeps the live session Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 10, 2026
Turns the red QR-journey behavior tests green. Every distinct way a scanned pairing code can fail now maps to its own localized message + recovery action through the single classifier sink from #5713: - Decode failures get a classifier (`classify(decodeError:)`) instead of collapsing into one "Invalid pairing code.": expired payloads say the code expired and how to mint a fresh one; version mismatches split into codeFromNewerApp ("update cmux on this device", loud, because rescanning can never help) vs codeFromOlderMac ("update the Mac"). - `CmxAttachTicketInput.decode` probes undecodable payloads for a cmux format marker: a top-level "v" is the compact short-key QR grammar newer Macs mint (#5727), a "version" key or the URL's `v` query item names a long-key version this build does not speak. Markerless garbage keeps the original opaque decode error and the actionable refresh-code message. - Tailnet-off seam: `refined(tailnetHint:)` upgrades hostUnreachable / dnsFailed / handshakeTimedOut on a tailnet-shaped address (CGNAT 100.64/10, the Tailscale ULA, .ts.net) to an explicit "Tailscale is off on this device" walk-through when the injected `tailnetHintProvider` reports the tailnet inactive. Defaults to unknown, so refinement is a no-op until the composition root wires the #5722 detector. - Scans while connected go through the pure `MobilePairingScanGate` before the destructive `beginPairingAttempt`: a code for the already connected Mac shows an "Already connected" notice, an undecodable code shows its classified message as a notice, and only a decodable code for a different Mac proceeds into the re-pair path. The live session is never torn down for a code that was never going to connect. - Non-fatal outcomes (already paired, bad code while connected, post-pair store save failure) surface on a new `pairingNotice` banner with `ios_pairing_scan_rejected` analytics, separate from the fatal `connectionError` surface, so a healthy session never shows a fatal error state. - Camera permission denial in the scanner sheet explains the recovery and offers an Open Settings button; a decoded QR that is not a cmux pairing code shows a "not a pairing code" hint (once per distinct code) instead of being silently ignored. - Connect-phase failures (offline, unreachable, listener not running, account mismatch, auth) keep the verified #5713 classification and are not rebuilt here. Pure ticket/route helpers move from MobileShellComposite to MobileShellTicketHelpers to stay under the Swift file length budget. All user-facing strings are localized with en + ja entries in the iOS string catalog. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 10, 2026
Turns the red QR-journey behavior tests green. Every distinct way a scanned pairing code can fail now maps to its own localized message + recovery action through the single classifier sink from #5713: - Decode failures get a classifier (`classify(decodeError:)`) instead of collapsing into one "Invalid pairing code.": expired payloads say the code expired and how to mint a fresh one; version mismatches split into codeFromNewerApp ("update cmux on this device", loud, because rescanning can never help) vs codeFromOlderMac ("update the Mac"). - `CmxAttachTicketInput.decode` probes undecodable payloads for a cmux format marker: a top-level "v" is the compact short-key QR grammar newer Macs mint (#5727), a "version" key or the URL's `v` query item names a long-key version this build does not speak. Markerless garbage keeps the original opaque decode error and the actionable refresh-code message. - Tailnet-off seam: `refined(tailnetHint:)` upgrades hostUnreachable / dnsFailed / handshakeTimedOut on a tailnet-shaped address (CGNAT 100.64/10, the Tailscale ULA, .ts.net) to an explicit "Tailscale is off on this device" walk-through when the injected `tailnetHintProvider` reports the tailnet inactive. Defaults to unknown, so refinement is a no-op until the composition root wires the #5722 detector. - Scans while connected go through the pure `MobilePairingScanGate` before the destructive `beginPairingAttempt`: a code for the already connected Mac shows an "Already connected" notice, an undecodable code shows its classified message as a notice, and only a decodable code for a different Mac proceeds into the re-pair path. The live session is never torn down for a code that was never going to connect. - Non-fatal outcomes (already paired, bad code while connected, post-pair store save failure) surface on a new `pairingNotice` banner with `ios_pairing_scan_rejected` analytics, separate from the fatal `connectionError` surface, so a healthy session never shows a fatal error state. - Camera permission denial in the scanner sheet explains the recovery and offers an Open Settings button; a decoded QR that is not a cmux pairing code shows a "not a pairing code" hint (once per distinct code) instead of being silently ignored. - Connect-phase failures (offline, unreachable, listener not running, account mismatch, auth) keep the verified #5713 classification and are not rebuilt here. Pure ticket/route helpers move from MobileShellComposite to MobileShellTicketHelpers to stay under the Swift file length budget. All user-facing strings are localized with en + ja entries in the iOS string catalog. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 10, 2026
This was referenced Jun 11, 2026
This branch was successfully deployed
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.
Slims the pairing QR so it scans faster from the Mac screen: the attach payload moves to a compact short-key JSON grammar and the QR renders at ECC L instead of M. The envelope is unchanged (
cmux-ios://attach?v=1&payload=<base64url(JSON)>), the Mac generator staysMobileAttachTicketStore.attachURL, and the phone decode stays behindCmxAttachTicketInput.decode(String) throws -> CmxAttachTicket.Measured effect (printed from the real encoder + CIQRCodeGenerator)
Representative Mac-wide pairing ticket (UUID device id, real-length display name, 43-char minted token):
At the pairing window's fixed 220 pt render, each module grows from ~2.2 pt to ~3.0 pt (1.38x). Attribution: JSON slimming alone takes the 2-route code v21 to v16 at ECC M; ECC M to L alone takes it to v18; combined v14.
Field-by-field before/after
versionvv, legacy hasversionworkspaceIDw"")terminalIDtmacDeviceIDdmacDisplayNamenexpiresAtISO8601 stringeunix seconds (Int)routes[]r[]route.id/kind/priority/endpointi/k/p/epomitted when 0endpoint.typethost_port/peer/urlendpoint.host/porth/pendpoint.id/relay_hint/direct_addrs/relay_urli/rh/da/rudaomitted when emptyendpoint.urluauth_tokenWhy dropping auth_token from the QR is safe
Host side, Stack auth is the sole authorization gate: the live
authorizeRequestpath goes throughMobileHostService.authorizationError(for:), which only verifies the same-account Stack access token and never consults the attach token.attach_tokenis read host-side only byrecordCreatedResourcesIfNeeded(bookkeeping for tokens minted overmobile.attach_ticket.create), and the ticket scope checker is reachable only viadebugTicketAuthorizationError(tests).Phone side,
MobileCoreRPCClient.requestDataWithAuthattachesattach_tokenonly when present and always sends the Stack token for authorized requests (a token-only request is rejected host-side withmissingStackTokens). With a nil token,initialWorkspaceListRequestsfalls through to the identical unscopedworkspace.listthe Mac-wide pairing ticket produced before. ThehasActiveUnexpiredAttachTicket/ attach-as-auth UI gate only matters for deep-link attach URLs from dev tooling, and that tooling (scripts/lib/attach-url.mjsviamobile-attach-qr.sh/dev-setup.sh, andmobile-soak.py) rebuilds its URLs from the RPC payload'sticketobject in legacy full-key JSON, which still carriesauth_tokenand which the tolerant decoder still accepts. The full ticket including the token also still rides unchanged in themobile.attach_ticket.createresponse.Compatibility matrix (all pinned by tests)
vkey (decodesCompactPayloadAttachURL, round-trip tests incl. peer/url endpoints).versionpayloads keep decoding through the original Codable path, auth token preserved (decodesLegacyFullKeyPayloadAttachURL).DecodingErroron the missingversionkey, so the user sees the pairing error UI, never a silently misread ticket (compactPayloadFailsLoudlyOnPreCompactDecoder,legacyDecoderRejectsCompactPayloadLoudly).cmux-ios://pairURLs: separate branch inCmxAttachTicketInput.decode, untouched.validate()path (expiredCompactTicketIsRejected, unknown kind/endpoint tests), andcompactPayloadIsSmallerThanLegacyPayloadpins a 220 B ceiling so payload regrowth shows up in review.Coordination with #5713
That PR's pairing connect/error work sits above the
CmxAttachTicketInput.decode(String)boundary; this PR only changes internals behind it and the two diffs share no files, so merge order is flexible. Any future edits toCmxAttachTicketInputitself should keep the two-grammar routing block intact.Verification
swift test --package-path Packages/CMUXMobileCore(69 tests) andswift test --package-path Packages/CmuxMobileRPC(23 tests) pass.xcodebuild -project cmux.xcodeproj -scheme cmuxwith a tagged derivedDataPath, and iOSxcodebuild -workspace ios/cmux.xcworkspace -scheme cmux-iosagainst an arm64 iPhone 17 Pro simulator destination. Both succeed.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Changes pairing wire format and drops attach token from QR; mitigated by dual-grammar decode, loud failure on old clients, and documented reliance on Stack auth only.
Overview
Pairing QR codes scan faster from the Mac screen by shrinking the attach payload and lowering QR error correction.
Compact attach JSON replaces legacy full-key
Codablein the pairing QR: short keys (v,d,r, …), omitted empty optionals, expiry as unix seconds (rounded up), and noauth_tokenin the QR (Stack access token remains the host auth gate; RPC responses still carry the full ticket).CmxAttachTicketCompactCoderand compact DTOs handle encode/decode;isCompactPayloaddistinguishes"v"vs legacy"version".Decode routing in
CmxAttachTicketInput: compact payloads go through the new coder; everything else keeps ISO8601 legacy decoding. Old app versions fail loudly on compact QRs instead of mis-parsing.Mac side:
MobileAttachTicketStore.attachURLemits compact payloads;MobilePairingQRImageViewuses ECC L instead of M for screen-to-camera pairing.Tests cover round-trips, cross-grammar compatibility, size ceiling (~220 B), and URL-level decode paths.
Reviewed by Cursor Bugbot for commit d6732a4. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Slims the pairing QR so it scans faster: the attach payload switches to a compact short-key JSON and the QR renders at ECC L instead of M. The URL envelope and decode entry points stay the same; the QR no longer carries a vestigial attach auth token.
New Features
cmux-ios://attach?v=1&payload=<base64url(JSON)>: short keys, omit empty fields, expiry in unix seconds (rounded up), noauth_token."v"→ compact coder;"version"→ legacyCodable. Validation unchanged; legacycmux-ios://pairremains untouched.MobileAttachTicketStoreemits the compact payload;CmxAttachTicketInput.decodesupports both grammars.Refactors
CmxAttachTicketCompactCoder; split compact DTOs (CompactAttachTicket,CompactAttachRoute,CompactAttachEndpoint) into separate files and scoped helpers to owning types per package conventions.Written for commit d6732a4. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Tests