Repository navigation
iOS: rebuild the disconnected Your Computers screen as a real list - #7156
Conversation
The disconnected screen rendered every stored paired-Mac record as a centered bordered pill, so a device that re-paired across dev builds showed a wall of identical "MacBook Pro" rows with no status, no way to remove one, and no feedback when a tap failed. It now shares the Computers screen's data path: rows come from the coalesced displayPairedMacs snapshots (one row per logical Mac) via a shared MacComputerSnapshot.snapshots(from:) builder, rendered with MacComputerRow in a new .reconnect style that leads with presence (Online / Last seen) instead of the phone connection, shows a spinner while a reconnect attempt is in flight, and alerts on failure. Rows get the same swipe/context-menu Remove (forgetMac, alias-wide) with confirmation. Presence and last-seen stay fresh with the same gentle 10s refresh the Computers screen uses (no offline re-dial storm), plus pull-to-refresh. The empty state keeps the previous ContentUnavailableView layout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds Mac switch in-flight state, shared Mac snapshot construction, a reconnect-capable computer row, and a rebuilt iOS disconnected workspace screen with refresh, reconnect, and removal flows. Updates changelog entries and localized strings. ChangesReconnect flow rebuild
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (4 errors)
✅ Passed checks (21 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0d4a7f9. Configure here.
Greptile SummaryRebuilds the iOS disconnected "Your Computers" screen from a stacked pill layout into a proper
Confidence Score: 5/5Safe to merge — the reconnect and remove flows reuse proven store methods, all edge cases (superseded switch, re-entry, cancellation) are explicitly guarded, and the shared snapshot builder is a pure extraction from DeviceTreeView with no behavioral change to the Computers screen. The reconnect triple-check (!connected, connectionState != .connected, !isMacSwitchInFlight) correctly handles every supersession scenario. The connectingMacID re-entry guard is synchronous on MainActor so there are no races. The Task.sleep polling loop is wired to the .task lifecycle for cancellation. Localization covers the catalog's two locales completely. macOS surface is unchanged. No new globals, no blocking primitives, no test seams. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant DisconnectedView
participant MacComputerRow
participant Store as CMUXMobileShellStore
DisconnectedView->>Store: loadPairedMacs()
Store-->>DisconnectedView: pairedMacs
alt pairedMacs empty
DisconnectedView->>User: show pairing sheet
else has Macs
DisconnectedView->>Store: loadRegistryDevices()
loop every 10s (Task.sleep)
DisconnectedView->>Store: refreshComputersScreen()
Store-->>DisconnectedView: updated presence/lastSeen
end
end
User->>MacComputerRow: tap row (.reconnect style)
MacComputerRow->>DisconnectedView: connect(macDeviceID)
DisconnectedView->>DisconnectedView: "guard connectingMacID == nil"
DisconnectedView->>DisconnectedView: "connectingMacID = macDeviceID"
DisconnectedView->>Store: switchToMac(macDeviceID)
Store-->>DisconnectedView: connected: Bool
DisconnectedView->>DisconnectedView: "connectingMacID = nil"
alt "!connected && .notConnected && !isMacSwitchInFlight"
DisconnectedView->>User: show failure alert
end
User->>MacComputerRow: swipe to remove
MacComputerRow->>DisconnectedView: requestRemove(deviceId)
DisconnectedView->>DisconnectedView: "computerPendingRemovalID = deviceId"
MacComputerRow->>User: show confirmation dialog
User->>MacComputerRow: confirm
MacComputerRow->>DisconnectedView: confirmComputerRemoval()
DisconnectedView->>Store: forgetMac(macDeviceID)
DisconnectedView->>Store: loadPairedMacs()
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant DisconnectedView
participant MacComputerRow
participant Store as CMUXMobileShellStore
DisconnectedView->>Store: loadPairedMacs()
Store-->>DisconnectedView: pairedMacs
alt pairedMacs empty
DisconnectedView->>User: show pairing sheet
else has Macs
DisconnectedView->>Store: loadRegistryDevices()
loop every 10s (Task.sleep)
DisconnectedView->>Store: refreshComputersScreen()
Store-->>DisconnectedView: updated presence/lastSeen
end
end
User->>MacComputerRow: tap row (.reconnect style)
MacComputerRow->>DisconnectedView: connect(macDeviceID)
DisconnectedView->>DisconnectedView: "guard connectingMacID == nil"
DisconnectedView->>DisconnectedView: "connectingMacID = macDeviceID"
DisconnectedView->>Store: switchToMac(macDeviceID)
Store-->>DisconnectedView: connected: Bool
DisconnectedView->>DisconnectedView: "connectingMacID = nil"
alt "!connected && .notConnected && !isMacSwitchInFlight"
DisconnectedView->>User: show failure alert
end
User->>MacComputerRow: swipe to remove
MacComputerRow->>DisconnectedView: requestRemove(deviceId)
DisconnectedView->>DisconnectedView: "computerPendingRemovalID = deviceId"
MacComputerRow->>User: show confirmation dialog
User->>MacComputerRow: confirm
MacComputerRow->>DisconnectedView: confirmComputerRemoval()
DisconnectedView->>Store: forgetMac(macDeviceID)
DisconnectedView->>Store: loadPairedMacs()
Reviews (4): Last reviewed commit: "Label stale same-named pairing records a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| .refreshable { | ||
| await store?.loadPairedMacs() | ||
| await store?.loadRegistryDevices() | ||
| } |
There was a problem hiding this comment.
The .refreshable handler calls loadPairedMacs() and loadRegistryDevices() but not refreshComputersScreen(), which is what the 10-second timer uses to keep "Online" / "Last seen …" fresh. A user who pulls down expecting up-to-date status will see the spinner complete but the Online/Last-seen values on each row stay stale until the next timer tick (up to 10 s later). Since presence is the primary signal the .reconnect row leads with, this is a visible UX inconsistency — the affordance implies "refresh everything shown" but the handler only half-does it.
| for await _ in Timer.publish(every: 10, on: .main, in: .common).autoconnect().values { | ||
| await store?.refreshComputersScreen() | ||
| } |
There was a problem hiding this comment.
Timer.publish(every: 10, on: .main, in: .common).autoconnect().values introduces a Combine publisher bridged to AsyncSequence in new iOS-only code. The cmux modernization rule flags new Combine app patterns in Swift where a concurrency-native alternative is available. An AsyncStream-based ticker or a for/while loop using Task.sleep(for: .seconds(10)) (or a Clock) would keep this in the Swift concurrency domain and not require a Combine import at all.
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!
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 6 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…tus id switchToMac returns false both for a genuine failure and for an attempt superseded by a newer switch; the reconnect list now suppresses its failure alert when another switch is in flight or already connected, via a new public isMacSwitchInFlight on the shell composite (Bugbot, cubic). Pull-to-refresh runs the same refreshComputersScreen() the 10s loop uses so presence/last-seen actually update on pull (Greptile P1). The Combine timer becomes a Task.sleep loop cancelled by the .task lifecycle (Greptile P2). The status dot's automation identifier suffix now derives from the same style-specific signal as its color: online/offline on reconnect rows (cubic P3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DisconnectedWorkspaceShellView.swift">
<violation number="1" location="Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DisconnectedWorkspaceShellView.swift:266">
P2: A superseded reconnect can still show a stale “couldn’t connect” alert after the newer Settings-sheet switch has already failed, because this check only suppresses while a newer switch is currently in flight or connected. Consider having `switchToMac` return a result reason/attempt token so the disconnected screen only alerts for its own non-superseded failure.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| connectingMacID = nil | ||
| if !connected, | ||
| store.connectionState != .connected, | ||
| !store.isMacSwitchInFlight { |
There was a problem hiding this comment.
P2: A superseded reconnect can still show a stale “couldn’t connect” alert after the newer Settings-sheet switch has already failed, because this check only suppresses while a newer switch is currently in flight or connected. Consider having switchToMac return a result reason/attempt token so the disconnected screen only alerts for its own non-superseded failure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DisconnectedWorkspaceShellView.swift, line 266:
<comment>A superseded reconnect can still show a stale “couldn’t connect” alert after the newer Settings-sheet switch has already failed, because this check only suppresses while a newer switch is currently in flight or connected. Consider having `switchToMac` return a result reason/attempt token so the disconnected screen only alerts for its own non-superseded failure.</comment>
<file context>
@@ -245,14 +251,19 @@ struct DisconnectedWorkspaceShellView: View {
- if !connected {
+ if !connected,
+ store.connectionState != .connected,
+ !store.isMacSwitchInFlight {
connectFailedComputerName = name
}
</file context>
![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: 1fdad2b74a
ℹ️ 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".
| // Cancellation is wired to this `.task`'s lifecycle. | ||
| await store?.loadRegistryDevices() | ||
| while !Task.isCancelled { | ||
| try? await Task.sleep(for: .seconds(10)) |
There was a problem hiding this comment.
When the disconnected screen has saved computers, this .task now stays alive by sleeping every 10 seconds and polling refreshComputersScreen(). The repo AGENTS.md Modern Swift concurrency rules explicitly forbid Task.sleep/Clock.sleep used to poll, because these loops are not the testable/cancellable signal path the codebase expects; use a timer/AsyncSequence-style refresh like the Computers screen instead.
Useful? React with 👍 / 👎.
…udget workflow-guard-tests failed: the 9-line accessor pushed MobileShellComposite.swift past its Swift file length budget. The accessor moves to MobileShellComposite+MacSwitchState.swift with macSwitchAttemptID widened to internal; the god file nets one line smaller than main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 099a4b70f4
ℹ️ 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".
| ForEach(computers) { computer in | ||
| MacComputerRow( | ||
| computer: computer, | ||
| requestRemove: { computerPendingRemovalID = $0 }, |
There was a problem hiding this comment.
When a user taps a reconnect row and then swipes or long-presses that same row before switchToMac finishes, this unconditional remove handler still allows forgetMac to run while the reconnect task from connect(to:) is active. forgetMac does not cancel the in-flight mac switch, so a late successful switch can leave the app connected to the computer the user just removed; either hide/disable removal for connectingMacID or cancel the switch before forgetting the row.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DisconnectedWorkspaceShellView.swift`:
- Around line 282-304: The removal-confirmation flow is duplicated between
DisconnectedWorkspaceShellView and DeviceTreeView, so extract the shared state
machine into one reusable component instead of keeping separate copies. Move the
computerPendingRemovalID/removalConfirmationBinding(for:)/confirmComputerRemoval()
behavior into a shared type or view modifier with a consistent API such as
request(_:), binding(for:), and confirm(), then have both views own and use that
shared implementation. Ensure both DisconnectedWorkspaceShellView and
DeviceTreeView call the same shared logic so future changes to removal behavior
only need to be made once.
- Around line 82-95: The polling loop in DisconnectedWorkspaceShellView’s iOS
`.task` still uses `Task.sleep`, which violates the cancellation-aware
scheduling guideline. Replace the `while !Task.isCancelled` / `Task.sleep(for:)`
refresh loop with the same timer-based async sequence pattern already used in
`DeviceTreeView` (for example, `Timer.publish(...).autoconnect().values`) so
cancellation is handled by the task lifecycle and both screens share the same
refresh mechanism.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MacComputerRow.swift`:
- Around line 188-203: The reconnect badge in MacComputerRow.badge is missing a
stable accessibility identifier on the ProgressView branch. Add an
accessibilityIdentifier to the spinner that matches the existing
MobileComputerStatus-<id>-<suffix> pattern used by the status dot, so automation
can target it consistently during isConnecting states. Keep the identifier logic
aligned with the current statusIdentifierSuffix/computer.deviceId formatting in
badge.
🪄 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: 57d90bc1-637e-4d7f-8d25-8285e6826a95
📒 Files selected for processing (8)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+MacSwitchState.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DeviceTreeView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DisconnectedWorkspaceShellView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MacComputerRow.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MacComputerSnapshot+Store.swiftios/CHANGELOG.mdios/cmux/Resources/Localizable.xcstrings
| #if os(iOS) | ||
| // Registry + presence enrich the rows (online dots, build | ||
| // labels). The loop then keeps presence and last-seen fresh | ||
| // while the app is parked on this screen; like the Computers | ||
| // screen it deliberately does NOT dial offline Macs (see | ||
| // `refreshComputersScreen()`), so no reconnect storm. | ||
| // Cancellation is wired to this `.task`'s lifecycle. | ||
| await store?.loadRegistryDevices() | ||
| while !Task.isCancelled { | ||
| try? await Task.sleep(for: .seconds(10)) | ||
| guard !Task.isCancelled else { break } | ||
| await store?.refreshComputersScreen() | ||
| } | ||
| #endif |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Replace the new Task.sleep polling loop with the same timer abstraction the Computers screen already uses.
This loop polls every 10s using Task.sleep, but the coding guidelines explicitly call this out: sleep/poll-based coordination must use "a real cancellation-aware scheduler, timer abstraction, async sequence, callback, notification, or state transition." DeviceTreeView.swift's .task already solves the identical "refresh every 10s while visible, cancel on dismiss" problem with Timer.publish(every: 10, on: .main, in: .common).autoconnect().values, which is exactly the timer-abstraction/async-sequence alternative the guideline prescribes. Introducing a second, non-compliant mechanism for the same job is both a guideline violation and a source of behavioral drift between the two screens.
🔧 Proposed fix — reuse the Timer.publish pattern
await store?.loadRegistryDevices()
- while !Task.isCancelled {
- try? await Task.sleep(for: .seconds(10))
- guard !Task.isCancelled else { break }
- await store?.refreshComputersScreen()
- }
+ for await _ in Timer.publish(every: 10, on: .main, in: .common).autoconnect().values {
+ await store?.refreshComputersScreen()
+ }As per coding guidelines, "Do not use Task.sleep in production Swift code just because it is inside an async function, including for animation timing; retry backoff, keepalive loops, readiness waits, delayed dispatch, and polling must use a real cancellation-aware scheduler, timer abstraction, async sequence, callback, notification, or state transition."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #if os(iOS) | |
| // Registry + presence enrich the rows (online dots, build | |
| // labels). The loop then keeps presence and last-seen fresh | |
| // while the app is parked on this screen; like the Computers | |
| // screen it deliberately does NOT dial offline Macs (see | |
| // `refreshComputersScreen()`), so no reconnect storm. | |
| // Cancellation is wired to this `.task`'s lifecycle. | |
| await store?.loadRegistryDevices() | |
| while !Task.isCancelled { | |
| try? await Task.sleep(for: .seconds(10)) | |
| guard !Task.isCancelled else { break } | |
| await store?.refreshComputersScreen() | |
| } | |
| #endif | |
| `#if` os(iOS) | |
| // Registry + presence enrich the rows (online dots, build | |
| // labels). The loop then keeps presence and last-seen fresh | |
| // while the app is parked on this screen; like the Computers | |
| // screen it deliberately does NOT dial offline Macs (see | |
| // `refreshComputersScreen()`), so no reconnect storm. | |
| // Cancellation is wired to this `.task`'s lifecycle. | |
| await store?.loadRegistryDevices() | |
| for await _ in Timer.publish(every: 10, on: .main, in: .common).autoconnect().values { | |
| await store?.refreshComputersScreen() | |
| } | |
| `#endif` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DisconnectedWorkspaceShellView.swift`
around lines 82 - 95, The polling loop in DisconnectedWorkspaceShellView’s iOS
`.task` still uses `Task.sleep`, which violates the cancellation-aware
scheduling guideline. Replace the `while !Task.isCancelled` / `Task.sleep(for:)`
refresh loop with the same timer-based async sequence pattern already used in
`DeviceTreeView` (for example, `Timer.publish(...).autoconnect().values`) so
cancellation is handled by the task lifecycle and both screens share the same
refresh mechanism.
Source: Coding guidelines
| private func removalConfirmationBinding(for deviceID: String) -> Binding<Bool> { | ||
| Binding( | ||
| get: { computerPendingRemovalID == deviceID }, | ||
| set: { isPresented in | ||
| if isPresented { | ||
| computerPendingRemovalID = deviceID | ||
| } else if computerPendingRemovalID == deviceID { | ||
| computerPendingRemovalID = nil | ||
| } | ||
| } | ||
| ) | ||
| } | ||
|
|
||
| private func confirmComputerRemoval() { | ||
| guard let deviceID = computerPendingRemovalID else { | ||
| return | ||
| } | ||
| computerPendingRemovalID = nil | ||
| Task { | ||
| await store?.forgetMac(macDeviceID: deviceID) | ||
| await store?.loadPairedMacs() | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Duplicate removal-confirmation state machine — extract a shared component instead of copy-pasting per screen.
computerPendingRemovalID + removalConfirmationBinding(for:) + confirmComputerRemoval() here are a line-for-line duplicate of the same trio in DeviceTreeView.swift (lines 146-172). Both screens implement independent copies of the same "confirm, then forget the alias-wide computer" flow. A future change to the removal semantics (e.g., an additional confirmation step, telemetry, or error handling) has to be applied twice and can silently diverge.
Consider extracting this into a small reusable type (e.g., an @Observable ComputerRemovalController or a shared view modifier) that both DeviceTreeView and DisconnectedWorkspaceShellView own via @State, exposing request(_:), binding(for:), and confirm().
As per the repo's shared-behavior policy: "When a behavior is exposed through multiple entrypoints ... implement one shared action/model path and verify every entrypoint that should invoke it. Do not patch one surface while leaving the others with duplicated logic."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DisconnectedWorkspaceShellView.swift`
around lines 282 - 304, The removal-confirmation flow is duplicated between
DisconnectedWorkspaceShellView and DeviceTreeView, so extract the shared state
machine into one reusable component instead of keeping separate copies. Move the
computerPendingRemovalID/removalConfirmationBinding(for:)/confirmComputerRemoval()
behavior into a shared type or view modifier with a consistent API such as
request(_:), binding(for:), and confirm(), then have both views own and use that
shared implementation. Ensure both DisconnectedWorkspaceShellView and
DeviceTreeView call the same shared logic so future changes to removal behavior
only need to be made once.
Source: Coding guidelines
| /// `.reconnect` rows show a spinner while their connect attempt is in flight. | ||
| @ViewBuilder | ||
| private var badge: some View { | ||
| Image(systemName: "circle.fill") | ||
| .font(.caption2) | ||
| .foregroundStyle(dotColor) | ||
| .accessibilityLabel(connectionPhrase) | ||
| .accessibilityIdentifier("MobileComputerStatus-\(computer.deviceId)-\(isConnected ? "connected" : "disconnected")") | ||
| if isConnecting { | ||
| ProgressView() | ||
| .controlSize(.small) | ||
| .accessibilityLabel( | ||
| L10n.string("mobile.deviceTree.reconnecting", defaultValue: "Reconnecting…")) | ||
| } else { | ||
| Image(systemName: "circle.fill") | ||
| .font(.caption2) | ||
| .foregroundStyle(dotColor) | ||
| .accessibilityLabel(primaryStatusPhrase) | ||
| .accessibilityIdentifier("MobileComputerStatus-\(computer.deviceId)-\(statusIdentifierSuffix)") | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Connecting spinner has no accessibility identifier.
The status dot exposes a stable MobileComputerStatus-<id>-<suffix> identifier for automation, but the ProgressView shown while isConnecting has none. Any UI test asserting on this identifier during a reconnect attempt loses its anchor.
🔧 Proposed fix
ProgressView()
.controlSize(.small)
.accessibilityLabel(
L10n.string("mobile.deviceTree.reconnecting", defaultValue: "Reconnecting…"))
+ .accessibilityIdentifier("MobileComputerStatus-\(computer.deviceId)-connecting")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// `.reconnect` rows show a spinner while their connect attempt is in flight. | |
| @ViewBuilder | |
| private var badge: some View { | |
| Image(systemName: "circle.fill") | |
| .font(.caption2) | |
| .foregroundStyle(dotColor) | |
| .accessibilityLabel(connectionPhrase) | |
| .accessibilityIdentifier("MobileComputerStatus-\(computer.deviceId)-\(isConnected ? "connected" : "disconnected")") | |
| if isConnecting { | |
| ProgressView() | |
| .controlSize(.small) | |
| .accessibilityLabel( | |
| L10n.string("mobile.deviceTree.reconnecting", defaultValue: "Reconnecting…")) | |
| } else { | |
| Image(systemName: "circle.fill") | |
| .font(.caption2) | |
| .foregroundStyle(dotColor) | |
| .accessibilityLabel(primaryStatusPhrase) | |
| .accessibilityIdentifier("MobileComputerStatus-\(computer.deviceId)-\(statusIdentifierSuffix)") | |
| } | |
| } | |
| /// `.reconnect` rows show a spinner while their connect attempt is in flight. | |
| `@ViewBuilder` | |
| private var badge: some View { | |
| if isConnecting { | |
| ProgressView() | |
| .controlSize(.small) | |
| .accessibilityLabel( | |
| L10n.string("mobile.deviceTree.reconnecting", defaultValue: "Reconnecting…")) | |
| .accessibilityIdentifier("MobileComputerStatus-\(computer.deviceId)-connecting") | |
| } else { | |
| Image(systemName: "circle.fill") | |
| .font(.caption2) | |
| .foregroundStyle(dotColor) | |
| .accessibilityLabel(primaryStatusPhrase) | |
| .accessibilityIdentifier("MobileComputerStatus-\(computer.deviceId)-statusIdentifierSuffix") | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MacComputerRow.swift`
around lines 188 - 203, The reconnect badge in MacComputerRow.badge is missing a
stable accessibility identifier on the ProgressView branch. Add an
accessibilityIdentifier to the spinner that matches the existing
MobileComputerStatus-<id>-<suffix> pattern used by the status dot, so automation
can target it consistently during isConnecting states. Keep the identifier logic
aligned with the current statusIdentifierSuffix/computer.deviceId formatting in
badge.
Before the shared Mac device id (PR #6772, 2026-06-25) every tagged Mac dev build minted its own device UUID, so a dogfooder's account backup holds many identically named records that do not coalesce (each dials a different port). Rows whose name matches a fresher row and that are not online now carry an "Older pairing" prefix on the diagnostic line, on both the Computers screen and the disconnected reconnect list (shared snapshot builder), so the entries stop looking interchangeable and the stale ones are obvious removal candidates. en + ja localized. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This branch was successfully deployed
1 participant
Before: the disconnected screen rendered every stored paired-Mac record as a centered bordered pill inside a ContentUnavailableView. A device that re-paired across dev builds showed dozens of identical "lawrence's MacBook Pro (2)" rows with no online status, no last-seen time, no way to remove a stale entry, and no feedback when a tap failed.
Now the screen shares the Computers screen's data path. Rows come from the coalesced
displayPairedMacssnapshots (one row per logical Mac) through a sharedMacComputerSnapshot.snapshots(from:)builder, and render withMacComputerRowin a new.reconnectstyle: machine-colored avatar, presence-led status line (Online / Last seen 2 hours ago), route diagnostic line, a green dot for Macs that are actually online, a spinner while a reconnect attempt is in flight, and an alert when the attempt fails instead of a silently ignored tap. Swipe or long-press removes the computer with the same alias-wideforgetMacconfirmation the Computers screen uses. Presence and last-seen refresh on the same gentle 10-secondrefreshComputersScreen()cadence (no offline re-dial storm), plus pull-to-refresh. The never-paired empty state keeps the previous ContentUnavailableView layout and auto-presented pairing sheet.New localized strings (
mobile.common.ok,mobile.disconnected.*) have en + ja entries; all other row strings reuse existing Computers-screen keys.ios/CHANGELOG.mdtop entry updated.Compile-verified with
swift build --triple arm64-apple-ios18.0-simulatoronCmuxMobileShellUI.🤖 Generated with Claude Code
Note
Medium Risk
Touches foreground Mac switching UX and reconnect error handling; changes are UI-focused but
switchToMacfailure semantics must stay correct to avoid missed or false alerts.Overview
Replaces the disconnected screen’s centered paired-Mac pills with the same coalesced computer list as the Computers screen: shared
MacComputerSnapshot.snapshots(from:),MacComputerRowin a new.reconnectstyle (presence-led status, tap-to-reconnect, spinner, swipe/remove), plus 10s presence refresh, pull-to-refresh, and a failure alert when reconnect truly fails.Adds
isMacSwitchInFlightonMobileShellCompositeso reconnect UI does not show “couldn’t connect” whenswitchToMacreturns false because a newer switch is still in flight. Marks older duplicate same-named offline pairings as “Older pairing” on row diagnostics. macOS disconnected shell keeps the prior ContentUnavailable layout.Reviewed by Cursor Bugbot for commit 18792a4. Bugbot is set up for automated code reviews on this repo. Configure here.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Rebuilt the iOS disconnected “Your Computers” screen into a real list that shows one row per Mac, leads with presence/last-seen, and adds clear reconnect/remove feedback. Stale same‑named records now show an “Older pairing” label so duplicates are obvious to clean up.
New Features
MacComputerSnapshot.snapshots(from:); flags stale same‑named entries as “Older pairing”..reconnectstyle inMacComputerRow: presence-led status and dot, spinner while connecting, supersession‑aware failure alert.forgetMacconfirmation.refreshComputersScreen(), plus pull‑to‑refresh; no offline re‑dial storm.mobile.common.ok,mobile.disconnected.*, andmobile.computers.olderPairing(en + ja).Refactors
isMacSwitchInFlighttoMobileShellComposite+MacSwitchState.swift(widenedmacSwitchAttemptIDto internal); no behavior change.Written for commit 18792a4. Summary will update on new commits.
Summary by CodeRabbit