iOS: show Restoring session for a known paired Mac on launch - #5543
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthroughPersists a "known paired Mac" hint in UserDefaults, adds in-memory reconnect gating flags and a reconnect-generation token, refactors stored‑Mac reconnection to manage those flags and persistence, centralizes view-layer reconnect orchestration, and adds a pure gating function with unit tests to show the restoring UI during reconnect attempts. ChangesStored Mac Reconnection Flow
Sequence DiagramsequenceDiagram
participant CMUXMobileRootView
participant MobileShellComposite
participant Store
participant UserDefaults
participant NetworkRoute
CMUXMobileRootView->>MobileShellComposite: onAppear / reconnectStoredMacIfNeeded()
MobileShellComposite->>UserDefaults: read hasKnownPairedMac key
UserDefaults-->>MobileShellComposite: persisted hint or absent
MobileShellComposite->>Store: reconnectActiveMacIfAvailable(stackUserID)
Store-->>MobileShellComposite: ticket / no ticket
MobileShellComposite->>NetworkRoute: attempt connect / check route
NetworkRoute-->>MobileShellComposite: success / failure
MobileShellComposite->>MobileShellComposite: set/clear hasKnownPairedMac, toggle isReconnectingStoredMac, finishStoredMacReconnectAttempt()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning, 1 inconclusive)
✅ Passed checks (15 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb035eb6cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else if store.connectionState != .connected && shouldShowRestoringStoredMac { | ||
| RestoringSessionView() |
There was a problem hiding this comment.
When the auth restore finishes before CMUXMobileRootView is mounted, isAuthenticated is already true, so the .onChange(of: isAuthenticated) block that calls reconnectActiveMacIfAvailable never runs. With a persisted hasKnownPairedMac hint, this new branch then renders RestoringSessionView while didFinishStoredMacReconnectAttempt remains false, trapping that returning user on the restoring screen instead of either reconnecting or falling through. This can happen on cold launch when cached session validation completes during app startup; the initial-authenticated path needs to kick off the same reconnect attempt, e.g. from onAppear/an initial task.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes the iOS cold-launch flash of the empty "Add device" sheet for returning users by introducing a
Confidence Score: 4/5Safe to merge with the unguarded hasKnownPairedMac write in persistPairedMacFromTicket understood and accepted — it produces the same add-device flash this PR reduces, but only after a forget-during-upsert race. The generation-token mechanism correctly guards every hint write on the reconnect path, but persistPairedMacFromTicket writes hasKnownPairedMac = true unconditionally after awaiting the SQLite upsert. If disconnectAndForgetActiveMac runs during that await, the forget's hasKnownPairedMac = false is overwritten by the completing upsert, leaving the hint dirty and causing a RestoringSessionView flash on the next cold launch — exactly the scenario this PR is trying to prevent. Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift — specifically the hasKnownPairedMac = true write in persistPairedMacFromTicket after the async upsert. Important Files Changed
Sequence DiagramsequenceDiagram
participant View as CMUXMobileRootView
participant Gate as MobileRootAuthGate
participant Store as MobileShellComposite
participant DB as PairedMacStore
Note over View: onAppear (already authenticated)
View->>Store: reconnectActiveMacIfAvailable
Store->>Store: bump generation (G)
Store->>DB: activeMac(stackUserID)
Note over View,Gate: shouldShowRestoringStoredMac=true
alt Mac found + route
DB-->>Store: MobilePairedMac
Store->>Store: setHasKnownPairedMac(true, G)
Store->>Store: "isReconnectingStoredMac=true"
Store->>Store: connectManualHost (await)
Note over View: shows RestoringSessionView
Store-->>Store: "isReconnectingStoredMac=false"
Store-->>Store: "didFinishStoredMacReconnectAttempt=true"
else No Mac / store error
DB-->>Store: nil / error
Store->>Store: setHasKnownPairedMac(false, G)
Store->>Store: finishStoredMacReconnectAttempt(G)
Note over View: falls through to add-device
end
Note over View,Store: User taps Forget
View->>Store: disconnectAndForgetActiveMac()
Store->>Store: bump generation (G+1)
Store->>Store: "hasKnownPairedMac=false"
Store->>Store: "isReconnectingStoredMac=false"
Store->>Store: "didFinishStoredMacReconnectAttempt=false"
Store-->>View: "connectionState=.disconnected"
Reviews (5): Last reviewed commit: "Restoring gate: neutral determining stat..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| hasKnownPairedMac = true | ||
| isReconnectingStoredMac = true | ||
| await connectManualHost(name: mac.displayName ?? host, host: host, port: port) | ||
| isReconnectingStoredMac = false | ||
| didFinishStoredMacReconnectAttempt = true | ||
| return connectionState == .connected |
There was a problem hiding this comment.
isReconnectingStoredMac prematurely
reconnectActiveMacIfAvailable has no guard against concurrent invocations. recoverMobileConnection has recoveryInFlight that prevents recovery-on-recovery, but does not coordinate with the initial launch reconnect started by reconnectStoredMacIfNeeded. If a network-path event fires (e.g., Wi-Fi appears during an offline cold launch) while the launch reconnect's await connectManualHost is in flight, a second call to reconnectActiveMacIfAvailable starts: it calls beginPairingAttempt(), which rotates the attempt UUID and causes the launch call's connectManualHost to return as superseded. The launch call then executes isReconnectingStoredMac = false / didFinishStoredMacReconnectAttempt = true synchronously — while the recovery call's connectManualHost is still awaiting a response on the actor. shouldShowRestoringStoredMac now returns false (didFinishStoredMacReconnectAttempt = true, isReconnectingStoredMac = false), so the disconnected/add-device sheet flashes in for the duration of the recovery connect — exactly the flash this PR was designed to prevent.
![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 `@Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 201-203: The public initializer for MobileShellComposite currently
hardcodes UserDefaults.standard via the parameter pairingHintDefaults; remove
that default so the initializer requires an injected UserDefaults (i.e., change
the signature of init(...) to accept pairingHintDefaults: UserDefaults without a
default) and update any call sites to explicitly pass the desired UserDefaults
instance; locate the initializer in MobileShellComposite (the init that takes
reachability: ReachabilityProviding and pairingHintDefaults: UserDefaults) and
ensure no global .standard/.shared is referenced in package code.
🪄 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: 888c460e-5254-4c74-a6e3-efe9e0188633
📒 Files selected for processing (4)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swiftPackages/CmuxMobileWorkspace/Sources/CmuxMobileWorkspace/MobileRootAuthGate.swiftPackages/CmuxMobileWorkspace/Tests/CmuxMobileWorkspaceTests/MobileRootAuthGateTests.swift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bacd377f95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| guard authenticated, connectionState != .connected else { return false } | ||
| if isReconnectingStoredMac { return true } | ||
| guard !didFinishStoredMacReconnectAttempt else { return false } | ||
| return hasKnownPairedMac || pairedMacHintUndetermined |
There was a problem hiding this comment.
On a fresh install this defaults key is absent too (pairedMacHintUndetermined is set from object(forKey:) == nil), so an authenticated never-paired user now satisfies this return path and sees RestoringSessionView until the async paired-Mac lookup finishes. That contradicts the intended “never paired → Add device immediately” path and can make first-time sign-in look like a stuck restore whenever the store read is slow or fails late; the undetermined compatibility case needs a way to exclude truly new installs or avoid blocking the add-device UI for them.
Useful? React with 👍 / 👎.
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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift (1)
585-588:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard reconnect completion flags against stale post-sign-out continuation.
After awaiting
connectManualHost(...), this path always marks the reconnect attempt finished. IfsignOut()runs during that await, the stale continuation can overwrite the sign-out reset and leave reconnect-gate state inconsistent for the next auth transition.Suggested fix
hasKnownPairedMac = true isReconnectingStoredMac = true await connectManualHost(name: mac.displayName ?? host, host: host, port: port) + guard isSignedIn else { + // Sign-out/reset path owns these flags; avoid resurrecting state + // from a stale reconnect continuation. + return false + } isReconnectingStoredMac = false didFinishStoredMacReconnectAttempt = true return connectionState == .connectedAlso applies to: 279-295
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift` around lines 585 - 588, After awaiting connectManualHost(name:host:port:) avoid blindly setting isReconnectingStoredMac and didFinishStoredMacReconnectAttempt because a signOut() may have occurred during the await; instead capture the current auth/session identity (or a local reconAttemptID) before the await and after connectManualHost returns verify the identity/ID still matches the one captured, and only then set isReconnectingStoredMac = false and didFinishStoredMacReconnectAttempt = true; apply the same guard change to the other reconnect path that touches isReconnectingStoredMac/didFinishStoredMacReconnectAttempt (the block around the earlier connectManualHost call referenced in the review).
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 585-588: After awaiting connectManualHost(name:host:port:) avoid
blindly setting isReconnectingStoredMac and didFinishStoredMacReconnectAttempt
because a signOut() may have occurred during the await; instead capture the
current auth/session identity (or a local reconAttemptID) before the await and
after connectManualHost returns verify the identity/ID still matches the one
captured, and only then set isReconnectingStoredMac = false and
didFinishStoredMacReconnectAttempt = true; apply the same guard change to the
other reconnect path that touches
isReconnectingStoredMac/didFinishStoredMacReconnectAttempt (the block around the
earlier connectManualHost call referenced in the review).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b7aeb7fc-0b5a-4712-a7b2-b5d3dc555454
📒 Files selected for processing (4)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swiftPackages/CmuxMobileWorkspace/Sources/CmuxMobileWorkspace/MobileRootAuthGate.swiftPackages/CmuxMobileWorkspace/Tests/CmuxMobileWorkspaceTests/MobileRootAuthGateTests.swift
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 5 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d22db46. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d22db4646c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| RestoringSessionView() | ||
| } else if !isAuthenticated { | ||
| SignInView() | ||
| } else if store.connectionState != .connected && shouldShowRestoringStoredMac { |
There was a problem hiding this comment.
When a fresh install has no paired-Mac hint (pairedMacHintUndetermined == true), this new branch can stay active forever on auth flows that intentionally skip reconnectStoredMacIfNeeded, such as an attach/deep-link connection launched while unauthenticated or a pending pairing URL consumed immediately after sign-in. If that URL connection fails, no stored-Mac lookup ever runs to set didFinishStoredMacReconnectAttempt, so the user remains on the neutral/restoring spinner instead of falling through to the add-device UI; these URL-only paths need to either run or explicitly resolve the stored-Mac determination gate after the URL attempt completes.
Useful? React with 👍 / 👎.
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
`@Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePairedMacDeterminingView.swift`:
- Around line 14-19: Add a localized accessibility label to the ProgressView so
screen readers announce meaningful context: update the ProgressView in
MobilePairedMacDeterminingView (the ProgressView() instance) to call
.accessibilityLabel(...) with a localized string key (e.g.
"mobile.pairedMacDetermining.accessibilityLabel" and sensible default like
"Loading") while keeping the existing
.accessibilityIdentifier("MobilePairedMacDetermining"); ensure you use
String(localized:..., defaultValue:...) (or your app's localization helper) so
the label is localized for VoiceOver users.
🪄 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: bdd779c1-2622-4024-8a5f-10a392ea298c
📒 Files selected for processing (2)
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePairedMacDeterminingView.swift
| var body: some View { | ||
| ProgressView() | ||
| .controlSize(.large) | ||
| .frame(maxWidth: .infinity, maxHeight: .infinity) | ||
| .accessibilityIdentifier("MobilePairedMacDetermining") | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add a localized accessibility label for screen reader users.
The view is correctly label-free visually to avoid misleading users, but accessibility labels are not shown on screen — they provide context for assistive technologies. Without an explicit label, screen readers will announce a generic "In progress" message.
Consider adding a neutral localized accessibility label:
ProgressView()
.controlSize(.large)
.frame(maxWidth: .infinity, maxHeight: .infinity)
.accessibilityLabel(String(localized: "mobile.pairedMacDetermining.accessibilityLabel", defaultValue: "Loading"))
.accessibilityIdentifier("MobilePairedMacDetermining")This improves the experience for VoiceOver users without compromising the visual neutrality.
🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePairedMacDeterminingView.swift`
around lines 14 - 19, Add a localized accessibility label to the ProgressView so
screen readers announce meaningful context: update the ProgressView in
MobilePairedMacDeterminingView (the ProgressView() instance) to call
.accessibilityLabel(...) with a localized string key (e.g.
"mobile.pairedMacDetermining.accessibilityLabel" and sensible default like
"Loading") while keeping the existing
.accessibilityIdentifier("MobilePairedMacDetermining"); ensure you use
String(localized:..., defaultValue:...) (or your app's localization helper) so
the label is localized for VoiceOver users.
A returning (already-paired) user briefly saw the empty "Add device" sheet flash on relaunch before the session restored. The root view fell straight to DisconnectedWorkspaceShellView during the reconnect window with no "restoring known Mac" state. Adds a gate (MobileRootAuthGate.shouldShowRestoringStoredMac) backed by a persisted hasKnownPairedMac hint plus isReconnectingStoredMac / didFinishStoredMacReconnectAttempt flags on the store, so a returning user sees RestoringSessionView during reconnect and a never-paired user still gets "Add device" instantly (no flash). Gate is unit-tested. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ount Autoreview caught that the restoring gate (hasKnownPairedMac && !didFinishStoredMacReconnectAttempt) could stay on RestoringSessionView forever: the reconnect that resolves the attempt only started from onChange(of: isAuthenticated), which never fires when the view mounts already authenticated (cached session, mock/fixture launch). Extract reconnectStoredMacIfNeeded() and call it from both onAppear and onChange so the attempt always resolves and the gate can never stick. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Autoreview caught that existing installs (which have an active Mac in SQLite but never wrote the new hasKnownPairedMac key) initialize the hint as false, so their first launch after updating still flashes the add-device flow until the async reconnect kicks in -- exactly the case this PR fixes, for the existing installed base. Add a pairedMacHintUndetermined flag (key absent at launch) and treat "undetermined" like "may have a paired Mac" in the restoring gate until the first reconnect attempt resolves and writes the hint. Determined-false (genuinely never paired) still shows add-device immediately. Gate test covers the undetermined case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Autoreview caught that reconnectActiveMacIfAvailable cleared the restoring flags unconditionally after connectManualHost returns. With reconnect Tasks started from multiple SwiftUI paths (onAppear, onChange, network recovery), a superseded older attempt could clear the gate while a newer reconnect was still in progress, reintroducing the add-device flash. Each attempt now claims a monotonically-increasing generation; only the current generation may write isReconnectingStoredMac / didFinishStoredMacReconnectAttempt / hasKnownPairedMac. sign-out and disconnect-and-forget bump the generation so an in-flight reconnect is superseded and can't resolve the gate after those events. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Autoreview round 4 flagged that fresh installs (no paired Mac, hint key absent) would briefly show the labeled "Restoring session..." screen before add-device. The paired-Mac store is a Swift actor, so the fresh-vs-existing determination is inherently async and can't be made before the first frame. During the undetermined window show a neutral, label-free spinner (MobilePairedMacDeterminingView) instead of "Restoring session..."; only switch to the labeled RestoringSessionView once we actually know a Mac is being reconnected (hasKnownPairedMac or isReconnectingStoredMac). Fresh installs see a neutral spinner then add-device; existing installs see the neutral spinner then "Restoring..." then their session. No misleading label in either direction. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
d22db46 to
4d4b898
Compare
| // A real, reconnectable Mac is now the active paired Mac: record the | ||
| // persisted hint so the next launch shows RestoringSessionView during | ||
| // the reconnect window instead of the empty add-device sheet. | ||
| hasKnownPairedMac = true |
There was a problem hiding this comment.
hasKnownPairedMac = true write races with disconnectAndForgetActiveMac
persistPairedMacFromTicket writes hasKnownPairedMac = true directly after await pairedMacStore.upsert(...) returns, with no generation or pairingAttemptID check. If the user taps "Forget" while the upsert is in flight — disconnectAndForgetActiveMac bumps the generation and synchronously sets hasKnownPairedMac = false — the upsert's completion then overwrites that to true, leaving the hint dirty. Every subsequent cold launch will show RestoringSessionView, then fall through once the store read returns no active Mac — exactly the flash this PR was written to eliminate.
The reconnect path guards every hint write through setHasKnownPairedMac(_:generation:), but persistPairedMacFromTicket bypasses that guard entirely. pairingAttemptID is already rotated by disconnectLiveConnection() inside the forget path; capturing it before the upsert and checking isCurrentPairingAttempt after would prevent the stale write.
Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift (1)
934-955:⚠️ Potential issue | 🟠 Major | ⚡ Quick winForget the persisted active row, not the transient attach-ticket ID.
Line 935 uses
activeTicket?.macDeviceIDfor removal, but this file also creates syntheticmanual-*ticket IDs on the reconnect/manual-host path at Lines 984-994, andpersistPairedMacFromTicketexplicitly refuses to store those IDs at Lines 830-833. In that case, "Rescan QR" clears the UI hint but leaves the real paired-Mac row active in SQLite, so the next launch reconnects the supposedly forgotten Mac.🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift` around lines 934 - 955, The code uses activeTicket?.macDeviceID (symbol: activeTicket) to decide which persisted row to remove, but transient/manual-* ticket IDs are never persisted (see persistPairedMacFromTicket) so the real paired-Mac row remains; change disconnectAndForgetActiveMac to query the persisted paired-Mac identifier from the pairedMacStore (or the same stored field used by persistPairedMacFromTicket) and call pairedMacStore.remove(macDeviceID:) with that persisted ID (symbols: pairedMacStore.remove, persistPairedMacFromTicket); if no persisted row exists, then fall back to removing activeTicket?.macDeviceID as before, and keep the removal async with the same error logging.
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 1803-1826: sendRemoteTerminalPasteImage is doing heavy base64
encoding on the `@MainActor`, causing UI stalls; fix by capturing values you need
(client, clientID, connectionGeneration, workspaceID.rawValue,
terminalID.rawValue, format) into local constants, then perform
data.base64EncodedString() and build the params dictionary inside a background
task (e.g. Task.detached) off the main actor, await that result, then hop back
to the MainActor to verify connectionGeneration still matches and call
client.sendRequest/handle response and errors on the main actor for state
updates; reference the sendRemoteTerminalPasteImage function and the symbols
remoteClient, clientID, connectionGeneration, workspaceID, terminalID, and
MobileCoreRPCClient.requestData when making the change.
- Around line 626-648: In reconnectActiveMacIfAvailable(stackUserID:), avoid
doing an unscoped lookup when stackUserID is nil: don't call
pairedMacStore.activeMac(stackUserID:) for a nil stackUserID since
pairedMacStore.fetchAllMacs adds the stack_user_id predicate only when non-nil
and activeMac(nil) can return another user’s Mac; instead treat nil as “no
scoped lookup” by short-circuiting (call
finishStoredMacReconnectAttempt(generation:) and return false) or only invoking
pairedMacStore.activeMac when stackUserID != nil so the persisted hint is not
cleared or the reconnect prematurely finished for the wrong user. Ensure you
reference storedMacReconnectGeneration/generation and
finishStoredMacReconnectAttempt when implementing the early-return behavior.
---
Outside diff comments:
In `@Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 934-955: The code uses activeTicket?.macDeviceID (symbol:
activeTicket) to decide which persisted row to remove, but transient/manual-*
ticket IDs are never persisted (see persistPairedMacFromTicket) so the real
paired-Mac row remains; change disconnectAndForgetActiveMac to query the
persisted paired-Mac identifier from the pairedMacStore (or the same stored
field used by persistPairedMacFromTicket) and call
pairedMacStore.remove(macDeviceID:) with that persisted ID (symbols:
pairedMacStore.remove, persistPairedMacFromTicket); if no persisted row exists,
then fall back to removing activeTicket?.macDeviceID as before, and keep the
removal async with the same error logging.
🪄 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: d95c1df7-c9fc-4bd4-ae85-31122e8daa73
📒 Files selected for processing (5)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePairedMacDeterminingView.swiftPackages/CmuxMobileWorkspace/Sources/CmuxMobileWorkspace/MobileRootAuthGate.swiftPackages/CmuxMobileWorkspace/Tests/CmuxMobileWorkspaceTests/MobileRootAuthGateTests.swift
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift (1)
934-955:⚠️ Potential issue | 🟠 Major | ⚡ Quick winForget the persisted active row, not the transient attach-ticket ID.
Line 935 uses
activeTicket?.macDeviceIDfor removal, but this file also creates syntheticmanual-*ticket IDs on the reconnect/manual-host path at Lines 984-994, andpersistPairedMacFromTicketexplicitly refuses to store those IDs at Lines 830-833. In that case, "Rescan QR" clears the UI hint but leaves the real paired-Mac row active in SQLite, so the next launch reconnects the supposedly forgotten Mac.🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift` around lines 934 - 955, The code uses activeTicket?.macDeviceID (symbol: activeTicket) to decide which persisted row to remove, but transient/manual-* ticket IDs are never persisted (see persistPairedMacFromTicket) so the real paired-Mac row remains; change disconnectAndForgetActiveMac to query the persisted paired-Mac identifier from the pairedMacStore (or the same stored field used by persistPairedMacFromTicket) and call pairedMacStore.remove(macDeviceID:) with that persisted ID (symbols: pairedMacStore.remove, persistPairedMacFromTicket); if no persisted row exists, then fall back to removing activeTicket?.macDeviceID as before, and keep the removal async with the same error logging.
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 1803-1826: sendRemoteTerminalPasteImage is doing heavy base64
encoding on the `@MainActor`, causing UI stalls; fix by capturing values you need
(client, clientID, connectionGeneration, workspaceID.rawValue,
terminalID.rawValue, format) into local constants, then perform
data.base64EncodedString() and build the params dictionary inside a background
task (e.g. Task.detached) off the main actor, await that result, then hop back
to the MainActor to verify connectionGeneration still matches and call
client.sendRequest/handle response and errors on the main actor for state
updates; reference the sendRemoteTerminalPasteImage function and the symbols
remoteClient, clientID, connectionGeneration, workspaceID, terminalID, and
MobileCoreRPCClient.requestData when making the change.
- Around line 626-648: In reconnectActiveMacIfAvailable(stackUserID:), avoid
doing an unscoped lookup when stackUserID is nil: don't call
pairedMacStore.activeMac(stackUserID:) for a nil stackUserID since
pairedMacStore.fetchAllMacs adds the stack_user_id predicate only when non-nil
and activeMac(nil) can return another user’s Mac; instead treat nil as “no
scoped lookup” by short-circuiting (call
finishStoredMacReconnectAttempt(generation:) and return false) or only invoking
pairedMacStore.activeMac when stackUserID != nil so the persisted hint is not
cleared or the reconnect prematurely finished for the wrong user. Ensure you
reference storedMacReconnectGeneration/generation and
finishStoredMacReconnectAttempt when implementing the early-return behavior.
---
Outside diff comments:
In `@Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 934-955: The code uses activeTicket?.macDeviceID (symbol:
activeTicket) to decide which persisted row to remove, but transient/manual-*
ticket IDs are never persisted (see persistPairedMacFromTicket) so the real
paired-Mac row remains; change disconnectAndForgetActiveMac to query the
persisted paired-Mac identifier from the pairedMacStore (or the same stored
field used by persistPairedMacFromTicket) and call
pairedMacStore.remove(macDeviceID:) with that persisted ID (symbols:
pairedMacStore.remove, persistPairedMacFromTicket); if no persisted row exists,
then fall back to removing activeTicket?.macDeviceID as before, and keep the
removal async with the same error logging.
🪄 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: d95c1df7-c9fc-4bd4-ae85-31122e8daa73
📒 Files selected for processing (5)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePairedMacDeterminingView.swiftPackages/CmuxMobileWorkspace/Sources/CmuxMobileWorkspace/MobileRootAuthGate.swiftPackages/CmuxMobileWorkspace/Tests/CmuxMobileWorkspaceTests/MobileRootAuthGateTests.swift
🛑 Comments failed to post (2)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift (2)
626-648:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "Definitions of activeMac/loadAll:" rg -n -C3 '\bfunc\s+(activeMac|loadAll)\s*\(' --glob '*.swift' echo echo "Call sites that pass stackUserID:" rg -n -C3 '\b(activeMac|loadAll)\s*\(stackUserID:|reconnectActiveMacIfAvailable\s*\(' --glob '*.swift'Repository: manaflow-ai/cmux
Length of output: 18129
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- MobileShellComposite.swift: loadPairedMacs / reconnect gating context ----" sed -n '680,820p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -n echo echo "---- MobileShellComposite.swift: reconnectActiveMacIfAvailable core ----" sed -n '600,720p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -n echo echo "---- MobilePairedMacStore.swift: activeMac/loadAll implementation ----" sed -n '150,230p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStore.swift | cat -n echo echo "---- MobilePairedMacStoring.swift: activeMac/loadAll protocol/impl details ----" sed -n '1,120p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStoring.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 23670
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- MobileShellComposite.swift: loadPairedMacs / reconnect gating context ----" sed -n '680,820p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -n echo echo "---- MobileShellComposite.swift: reconnectActiveMacIfAvailable core ----" sed -n '600,720p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -n echo echo "---- MobilePairedMacStore.swift: activeMac/loadAll implementation ----" sed -n '150,230p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStore.swift | cat -n echo echo "---- MobilePairedMacStoring.swift: activeMac/loadAll protocol/impl details ----" sed -n '1,140p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStoring.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 23670
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- MobileShellComposite.swift: loadPairedMacs / reconnect gating context ----" sed -n '680,820p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -n echo echo "---- MobileShellComposite.swift: reconnectActiveMacIfAvailable core ----" sed -n '600,720p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -n echo echo "---- MobilePairedMacStore.swift: activeMac/loadAll implementation ----" sed -n '150,230p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStore.swift | cat -n echo echo "---- MobilePairedMacStoring.swift: activeMac/loadAll protocol/impl details ----" sed -n '1,140p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStoring.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 23670
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- MobileShellComposite.swift: loadPairedMacs / reconnect gating context ----" sed -n '680,820p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -n echo echo "---- MobileShellComposite.swift: reconnectActiveMacIfAvailable core ----" sed -n '600,720p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -n echo echo "---- MobilePairedMacStore.swift: activeMac/loadAll implementation ----" sed -n '150,230p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStore.swift | cat -n echo echo "---- MobilePairedMacStoring.swift: activeMac/loadAll protocol/impl details ----" sed -n '1,140p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStoring.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 23670
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- MobileShellComposite.swift: loadPairedMacs / reconnect gating context ----" sed -n '680,820p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -n echo echo "---- MobileShellComposite.swift: reconnectActiveMacIfAvailable core ----" sed -n '600,720p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -n echo echo "---- MobilePairedMacStore.swift: activeMac/loadAll implementation ----" sed -n '150,230p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStore.swift | cat -n echo echo "---- MobilePairedMacStoring.swift: activeMac/loadAll protocol/impl details ----" sed -n '1,140p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStoring.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 23670
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- MobileShellComposite.swift: loadPairedMacs / reconnect gating context ----" sed -n '680,820p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -n echo echo "---- MobileShellComposite.swift: reconnectActiveMacIfAvailable core ----" sed -n '600,720p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -n echo echo "---- MobilePairedMacStore.swift: activeMac/loadAll implementation ----" sed -n '150,230p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStore.swift | cat -n echo echo "---- MobilePairedMacStoring.swift: activeMac/loadAll protocol/impl details ----" sed -n '1,140p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStoring.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 23670
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- MobilePairedMacStore.swift: fetchAllMacs implementations ----" rg -n "func\s+fetchAllMacs\s*\(" Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStore.swift echo echo "---- MobilePairedMacStore.swift: fetchAllMacs (context) ----" rg -n -C3 "fetchAllMacs" Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStore.swiftRepository: manaflow-ai/cmux
Length of output: 1065
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '316,420p' Packages/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStore.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 5113
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- Call site(s) for reconnectActiveMacIfAvailable ----" rg -n "reconnectActiveMacIfAvailable\s*\(" --glob '*.swift' echo echo "---- Any tests referencing the reconnect nil stackUserID case ----" rg -n "activeMac\s*\(stackUserID:\s*nil\)|stackUserID:\s*nil|reconnectActiveMacIfAvailable" --glob '*Test*.swift' --glob '*.swift' || true echo echo "---- MobileShellComposite: definition of isSignedIn / identityProvider usage ----" rg -n "var\s+isSignedIn|isSignedIn\s*[:=]|identityProvider\?" Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftRepository: manaflow-ai/cmux
Length of output: 2539
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- MobilePairedMacStoreTests around stackUserID: nil (line ~110-160) ----" sed -n '90,170p' Packages/CmuxMobilePairedMac/Tests/CmuxMobilePairedMacTests/MobilePairedMacStoreTests.swift | cat -n echo echo "---- CMUXMobileRootView: how stackUserID is computed/passed (around line ~200-240) ----" sed -n '180,260p' Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift | cat -n echo echo "---- MobileShellComposite: init/sign-in state relationship (around where isSignedIn is set) ----" sed -n '200,360p' Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 14739
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n "struct\s+MobileRootAuthGate|enum\s+MobileRootAuthGate|class\s+MobileRootAuthGate" --glob '*.swift' echo rg -n "shouldReconnectStoredMac|shouldShowRestoringStoredMac|didFinishStoredMacReconnectAttempt|hasKnownPairedMac|pairedMacHintUndetermined" --glob '*.swift' Packages/CmuxMobileShellUI/Sources Packages/CmuxMobileShell/Sources echo # If multiple, just dump the most relevant file(s) for f in $(rg -l "shouldReconnectStoredMac" --glob '*.swift' Packages/CmuxMobileShellUI/Sources Packages/CmuxMobileShell/Sources); do echo "---- $f ----" sed -n '1,220p' "$f" | cat -n doneRepository: manaflow-ai/cmux
Length of output: 14930
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,220p' Packages/CmuxMobileWorkspace/Sources/CmuxMobileWorkspace/MobileRootAuthGate.swift | cat -n echo sed -n '1,260p' Packages/CmuxMobileWorkspace/Tests/CmuxMobileWorkspaceTests/MobileRootAuthGateTests.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 15411
Fix cross-user / non-scoped stored-Mac reconnect when
stackUserID == nil
reconnectActiveMacIfAvailable(stackUserID:)forwards its optionalstackUserIDintopairedMacStore.activeMac(stackUserID:); the store’sfetchAllMacs(activeOnly: stackUserID:)only addsstack_user_id IS ?whenstackUserIDis non-nil, soactiveMac(nil)can return an active Mac from any Stack user.- If that unscoped lookup returns
nil,reconnectActiveMacIfAvailableclears the persisted “known paired Mac” hint and setsdidFinishStoredMacReconnectAttempt, and the reconnect is only kicked off ononAppear/onChange(of: isAuthenticatedinCMUXMobileRootView—so it won’t automatically retry onceauthManager.currentUser?.idbecomes available.Expected: treat
stackUserID == nilas “no scoped lookup” (returnnil/ don’t query), or prevent callingactiveMacuntilstackUserIDis non-nil (matching the scoping behavior ofloadPairedMacs).🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift` around lines 626 - 648, In reconnectActiveMacIfAvailable(stackUserID:), avoid doing an unscoped lookup when stackUserID is nil: don't call pairedMacStore.activeMac(stackUserID:) for a nil stackUserID since pairedMacStore.fetchAllMacs adds the stack_user_id predicate only when non-nil and activeMac(nil) can return another user’s Mac; instead treat nil as “no scoped lookup” by short-circuiting (call finishStoredMacReconnectAttempt(generation:) and return false) or only invoking pairedMacStore.activeMac when stackUserID != nil so the persisted hint is not cleared or the reconnect prematurely finished for the wrong user. Ensure you reference storedMacReconnectGeneration/generation and finishStoredMacReconnectAttempt when implementing the early-return behavior.
1803-1826: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Encode pasted images off the main actor.
MobileShellCompositeis@MainActor, so Line 1818 base64-encodes the entire image and builds the JSON payload on the UI thread. A multi-MB screenshot paste will stall the shell before the RPC even starts. Pre-encode off-main and only hop back for state/error updates.♻️ Suggested direction
- let params: [String: Any] = [ + let imageBase64 = await Task.detached(priority: .userInitiated) { + data.base64EncodedString() + }.value + let params: [String: Any] = [ "workspace_id": workspaceID.rawValue, "surface_id": terminalID.rawValue, - "image_base64": data.base64EncodedString(), + "image_base64": imageBase64, "image_format": format, "client_id": clientID, ]🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift` around lines 1803 - 1826, sendRemoteTerminalPasteImage is doing heavy base64 encoding on the `@MainActor`, causing UI stalls; fix by capturing values you need (client, clientID, connectionGeneration, workspaceID.rawValue, terminalID.rawValue, format) into local constants, then perform data.base64EncodedString() and build the params dictionary inside a background task (e.g. Task.detached) off the main actor, await that result, then hop back to the MainActor to verify connectionGeneration still matches and call client.sendRequest/handle response and errors on the main actor for state updates; reference the sendRemoteTerminalPasteImage function and the symbols remoteClient, clientID, connectionGeneration, workspaceID, terminalID, and MobileCoreRPCClient.requestData when making the change.
…ign-out Address the actionable autoreview P2: signIn()'s untracked bootstrap Task could run loadPairedMacs() after a fast sign-out, take its !isSignedIn path, and set hasCompletedInitialPairedMacLoad = true after signOut() reset it to false. The next sign-in could then evaluate hasNoPairedMacs before the new user's load completed and flash/open pairing for a returning paired user. Fix with a signInBootstrapGeneration (the same pattern #5543 uses for the stored-Mac reconnect): signIn claims a generation and the bootstrap bails if it is superseded or signed out before loadPairedMacs and again before the refresh; signOut bumps the generation. So a bootstrap from a signed-out session can no longer resolve the gate for the next session. The remaining legacy-synthetic-ticket partition-key finding is the same effectively- unreachable case already documented as residual risk (persistPairedMacFromTicket rejects manual- ids, so a stored Mac always has a real macDeviceID; divergence needs a version downgrade between pair and reconnect). Not perturbing the active-path partition key for it untested. 20 CmuxMobileShell tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g it Regression from the restoring-session gate (#5543): on launch, a returning user's stored Mac whose route went stale (Tailscale address changed, or the Mac is offline) makes connectManualHost hang on a slow connect timeout. Since the gate shows RestoringSessionView while isReconnectingStoredMac is true, the user was stuck on "Restoring session..." for the whole connect timeout before it finally fell through to add-device. Add a bounded, cancellable deadline (6s) around the launch reconnect: if the stored Mac hasn't connected by then, resolve the restoring gate so the user reaches add-device quickly. The connect keeps trying in the background, so a later success still flips connectionState to .connected and shows the workspaces. Generation-guarded so a superseded attempt can't fire it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g it (#5564) Regression from the restoring-session gate (#5543): on launch, a returning user's stored Mac whose route went stale (Tailscale address changed, or the Mac is offline) makes connectManualHost hang on a slow connect timeout. Since the gate shows RestoringSessionView while isReconnectingStoredMac is true, the user was stuck on "Restoring session..." for the whole connect timeout before it finally fell through to add-device. Add a bounded, cancellable deadline (6s) around the launch reconnect: if the stored Mac hasn't connected by then, resolve the restoring gate so the user reaches add-device quickly. The connect keeps trying in the background, so a later success still flips connectionState to .connected and shows the workspaces. Generation-guarded so a superseded attempt can't fire it. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This branch was successfully deployed
1 participant
Summary
MobileRootAuthGate.shouldShowRestoringStoredMac, gated by a persistedhasKnownPairedMachint (covers the first rendered frame) plusisReconnectingStoredMac/didFinishStoredMacReconnectAttemptflags onMobileShellComposite. A returning user sees the existingRestoringSessionViewduring reconnect; a never-paired user still gets "Add device" instantly with no flash; a failed/offline reconnect falls through instead of spinning forever.Testing
MobileRootAuthGateTests.shouldShowRestoringStoredMac(6 cases) — passing locally viaswift testonCmuxMobileWorkspace(no GhosttyKit needed): 4 tests green.Notes
MobileShellComposite+CMUXMobileRootView, which overlap with iOS: multi-Mac host switcher #5513 and other in-flight iOS PRs; will rebase as those land.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Launch and reconnect UI routing changed across shell store and root view; overlapping reconnect/sign-out/forget paths rely on generation guards—wrong gating could spin forever or flash the wrong screen, but logic is covered by new auth-gate tests.
Overview
Fixes the iOS cold-launch flash where authenticated returning users briefly saw the empty Add device sheet before stored-Mac reconnect finished.
MobileShellCompositepersists ahasKnownPairedMachint inUserDefaults(sync read on init for the first frame), tracksisReconnectingStoredMac,didFinishStoredMacReconnectAttempt, andpairedMacHintUndetermined(missing key = legacy install that may already have SQLite pairings).reconnectActiveMacIfAvailablenow drives those flags, clears the hint only when there is definitively no Mac or no route, keeps the hint on store read failures, and uses a reconnect generation so sign-out, forget, and overlapping attempts cannot clobber a newer reconnect. Pairing upsert sets the hint;disconnectLiveConnectionno longer clears it—disconnectAndForgetActiveMacdoes.CMUXMobileRootViewaddsreconnectStoredMacIfNeeded()ononAppearand auth changes (fixes stuck restoring when already signed in at mount). While disconnected and gated byMobileRootAuthGate.shouldShowRestoringStoredMac, it showsRestoringSessionViewwhen a Mac is known or reconnecting, elseMobilePairedMacDeterminingViewuntil the first attempt resolves, then add-device or workspaces.MobileRootAuthGategainsshouldShowRestoringStoredMacwith unit tests.Reviewed by Cursor Bugbot for commit 4d4b898. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes the iOS cold-launch flash by showing the restoring UI for returning users and reliably starting the stored‑Mac reconnect on initial authenticated mount. Adds a neutral determining state and generation‑guarded reconnects to avoid misleading labels and racey flashes.
MobileRootAuthGate.shouldShowRestoringStoredMac, backed byhasKnownPairedMac,pairedMacHintUndetermined,isReconnectingStoredMac, anddidFinishStoredMacReconnectAttempt.CMUXMobileRootViewshowsRestoringSessionViewwhen reconnecting or hinted, and a neutralMobilePairedMacDeterminingViewspinner while the hint is undetermined.hasKnownPairedMacinUserDefaults: treat a missing key as “may have a Mac,” keep the hint on store read failures, clear it when no Mac or no usable route is found, and write true on successful reconnect/upsert. Use generation tokens so only the latest attempt can change hints/flags; bump on sign‑out and forget to prevent stale tasks from resolving the gate.reconnectStoredMacIfNeeded()and call it from bothonAppearand auth changes so cached‑session mounts always resolve the gate and never stick on restoring. Added unit tests covering the restoring‑stored‑Mac policy.Written for commit 4d4b898. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Tests