Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,20 @@ public actor MobilePairedMacStore: MobilePairedMacStoring {
public func setActive(macDeviceID: String) throws {
try ensureReady()
try transaction {
try exec("UPDATE paired_macs SET is_active = 0;")
// Clear the active flag only within the target Mac's own Stack-user
// scope, mirroring the scoped clear in `upsert`. On a shared device
// (more than one Stack user has pairings), switching hosts for one
// signed-in user must not wipe another user's active Mac, or that
// user fails to auto-reconnect after signing back in. `IS` is
// SQLite's null-safe equality, so a NULL-scoped target clears only
// other NULL-scoped rows.
try exec("""
UPDATE paired_macs SET is_active = 0
WHERE stack_user_id IS (
SELECT stack_user_id FROM paired_macs WHERE mac_device_id = ?
);
""",
binding: [.text(macDeviceID)])
try exec("UPDATE paired_macs SET is_active = 1 WHERE mac_device_id = ?;",
binding: [.text(macDeviceID)])
}
Comment on lines 198 to 215

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Silent active-flag corruption when macDeviceID is not in the table. The inner SELECT is a scalar subquery; when no row matches, SQLite produces NULL, so WHERE stack_user_id IS NULL fires and clears every null-scoped row's active flag — then the second UPDATE matches nothing, leaving no Mac active for that scope. The untracked Tasks already flagged on MobileHostPickerView make this reachable via a race between a swipe-to-forget and a tap-to-switch on the same non-active row. Adding a WHERE EXISTS guard or checking the row count before committing prevents silent corruption.

Suggested change
try transaction {
try exec("UPDATE paired_macs SET is_active = 0;")
// Clear the active flag only within the target Mac's own Stack-user
// scope, mirroring the scoped clear in `upsert`. On a shared device
// (more than one Stack user has pairings), switching hosts for one
// signed-in user must not wipe another user's active Mac, or that
// user fails to auto-reconnect after signing back in. `IS` is
// SQLite's null-safe equality, so a NULL-scoped target clears only
// other NULL-scoped rows.
try exec("""
UPDATE paired_macs SET is_active = 0
WHERE stack_user_id IS (
SELECT stack_user_id FROM paired_macs WHERE mac_device_id = ?
);
""",
binding: [.text(macDeviceID)])
try exec("UPDATE paired_macs SET is_active = 1 WHERE mac_device_id = ?;",
binding: [.text(macDeviceID)])
}
try transaction {
// Verify the target mac exists before touching any active flags.
// If the subquery in the scoped-clear returns no rows (mac deleted
// in a concurrent operation) it becomes NULL, which would silently
// clear every null-scoped row's active flag without setting any row
// active. Failing early keeps the store in a consistent state.
let exists = try fetchMacRow(macDeviceID: macDeviceID) != nil
guard exists else {
throw MobilePairedMacStoreError.stepFailed(SQLITE_NOTFOUND, "mac_device_id not found: \(macDeviceID)")
}
// Clear the active flag only within the target Mac's own Stack-user
// scope, mirroring the scoped clear in `upsert`. On a shared device
// (more than one Stack user has pairings), switching hosts for one
// signed-in user must not wipe another user's active Mac, or that
// user fails to auto-reconnect after signing back in. `IS` is
// SQLite's null-safe equality, so a NULL-scoped target clears only
// other NULL-scoped rows.
try exec("""
UPDATE paired_macs SET is_active = 0
WHERE stack_user_id IS (
SELECT stack_user_id FROM paired_macs WHERE mac_device_id = ?
);
""",
binding: [.text(macDeviceID)])
try exec("UPDATE paired_macs SET is_active = 1 WHERE mac_device_id = ?;",
binding: [.text(macDeviceID)])
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,28 @@ import Testing
#expect(active.first?.macDeviceID == "mac-c")
}

@Test func setActiveScopesClearToTargetStackUser() async throws {
let (store, directory) = try makeStore()
defer { try? FileManager.default.removeItem(at: directory) }

let route = try CmxAttachRoute(
id: "tailscale",
kind: .tailscale,
endpoint: .hostPort(host: "100.64.0.3", port: 8443)
)
// user-1 has two macs, user-2 one; each starts active within its scope.
try await store.upsert(macDeviceID: "mac-a1", displayName: nil, routes: [route], markActive: true, stackUserID: "user-1", now: Date())
try await store.upsert(macDeviceID: "mac-a2", displayName: nil, routes: [route], markActive: true, stackUserID: "user-1", now: Date())
try await store.upsert(macDeviceID: "mac-b", displayName: nil, routes: [route], markActive: true, stackUserID: "user-2", now: Date())

// Switching user-1's active Mac must not disturb user-2's active pairing.
try await store.setActive(macDeviceID: "mac-a1")

let activeUser1 = try await store.loadAll(stackUserID: "user-1").filter(\.isActive)
#expect(activeUser1.map(\.macDeviceID) == ["mac-a1"])
#expect(try await store.activeMac(stackUserID: "user-2")?.macDeviceID == "mac-b")
}

@Test func removePersistsAcrossReopen() async throws {
let directory = FileManager.default.temporaryDirectory
.appendingPathComponent(UUID().uuidString, isDirectory: true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,9 @@ public final class MobileShellComposite: MobileTerminalOutputSinking {
connectionError = nil
activeTicket = nil
activeRoute = nil
// Drop the cached paired Macs so the next signed-in user never sees the
// previous user's hosts in the switcher.
pairedMacs = []
replaceRemoteClient(with: nil)
cancelRemoteOperationTasks()
rawTerminalInputBuffer.clear()
Expand Down Expand Up @@ -568,6 +571,108 @@ public final class MobileShellComposite: MobileTerminalOutputSinking {
return connectionState == .connected
}

// MARK: - Paired Mac switching

/// Every Mac paired with this device, for the host switcher. Refreshed via
/// ``loadPairedMacs()`` and after switch/forget. Cleared on sign-out so a
/// shared device never shows the previous user's Macs. The active row is
/// marked by each ``MobilePairedMac/isActive`` flag (the live connection's
/// attach ticket carries a transient manual id, so it is not a reliable
/// active marker on its own).
public private(set) var pairedMacs: [MobilePairedMac] = []

/// Reload ``pairedMacs`` from the store, scoped to the signed-in Stack user.
///
/// A missing current Stack user id yields no pairings rather than falling
/// back to the unscoped all-users query, so a shared device never exposes
/// another user's Macs in the switcher.
public func loadPairedMacs() async {
guard let pairedMacStore, isSignedIn,
let stackUserID = identityProvider?.currentUserID else {
pairedMacs = []
return
}
let loaded: [MobilePairedMac]
do {
loaded = try await pairedMacStore.loadAll(stackUserID: stackUserID)
} catch {
mobileShellLog.error("paired mac store loadAll failed: \(String(describing: error), privacy: .public)")
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failed reload keeps stale Mac list

Low Severity

When pairedMacStore.loadAll throws, loadPairedMacs logs the error and returns without updating pairedMacs. The host picker can keep showing an outdated list (including Macs already forgotten elsewhere) until another successful reload runs.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 861ac47. Configure here.

}
// The await above suspended the main actor; a sign-out or user switch may
// have run meanwhile. Discard the result unless we are still the same
// signed-in user, so a slow load can never repopulate another user's hosts.
guard isSignedIn, identityProvider?.currentUserID == stackUserID else {
pairedMacs = []
return
}
Comment on lines +596 to +608

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clear pairedMacs when reload fails to avoid stale/cross-account host exposure.

If loadAll throws, the previous in-memory list is kept. After account/scope changes, the UI can continue showing old pairings until a later successful reload.

Suggested fix
public func loadPairedMacs() async {
    guard let pairedMacStore else {
        pairedMacs = []
        return
    }
    do {
        pairedMacs = try await pairedMacStore.loadAll(stackUserID: identityProvider?.currentUserID)
    } catch {
+       pairedMacs = []
        mobileShellLog.error("paired mac store loadAll failed: \(String(describing: error), privacy: .public)")
    }
}
📝 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.

Suggested change
do {
pairedMacs = try await pairedMacStore.loadAll(stackUserID: identityProvider?.currentUserID)
} catch {
mobileShellLog.error("paired mac store loadAll failed: \(String(describing: error), privacy: .public)")
}
do {
pairedMacs = try await pairedMacStore.loadAll(stackUserID: identityProvider?.currentUserID)
} catch {
pairedMacs = []
mobileShellLog.error("paired mac store loadAll failed: \(String(describing: error), privacy: .public)")
}
🤖 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 525 - 529, When pairedMacStore.loadAll throws in
MobileShellComposite.swift, the old in-memory pairedMacs remains and can show
stale/cross-account data; in the catch block for the try await
pairedMacStore.loadAll(stackUserID: identityProvider?.currentUserID) set
pairedMacs to an empty collection (or nil if pairedMacs is optional) before
calling mobileShellLog.error to ensure the UI no longer shows previous pairings
after a failed reload.

pairedMacs = loaded
}

/// Switch the live connection to `macDeviceID`, persisting it as the active
/// pairing only on a successful connect.
///
/// The underlying connect path is destructive (it replaces the live client),
/// so a failed switch to an offline/stale Mac would drop the working session.
/// To avoid stranding the user, the store's active row is only updated on a
/// successful connect, and on failure the previously-active Mac (still the
/// active row) is reconnected. A no-op when already connected to that Mac.
/// - Parameter macDeviceID: The stored Mac to switch to.
public func switchToMac(macDeviceID: String) async {
guard let pairedMacStore,
let target = pairedMacs.first(where: { $0.macDeviceID == macDeviceID }) else { return }
if target.isActive, connectionState == .connected { return }
Comment thread
cursor[bot] marked this conversation as resolved.
// The currently-active Mac to fall back to if the switch fails.
let previousActive = pairedMacs.first { $0.isActive && $0.macDeviceID != macDeviceID }
let supportedKinds = runtime?.supportedRouteKinds ?? []
guard let (host, port) = Self.firstReconnectHostPortRoute(
target.routes,
supportedKinds: supportedKinds
), let normalizedHost = MobileShellRouteAuthPolicy.normalizedManualHost(host) else {
mobileShellLog.error("switchToMac: no reconnectable route mac=\(macDeviceID, privacy: .public)")
return
}
await connectManualHost(name: target.displayName ?? host, host: host, port: port)
// Persist the active row only if the live connection is to THIS Mac's
// route. A different switch tapped while this connect was in flight
// supersedes it via `beginPairingAttempt`, leaving `connectionState`
// `.connected` for the other Mac; matching the live route prevents this
// superseded task from persisting a stale active target.
if connectionState == .connected,
case let .hostPort(liveHost, livePort)? = activeRoute?.endpoint,
liveHost == normalizedHost, livePort == port {
do {
try await pairedMacStore.setActive(macDeviceID: macDeviceID)
} catch {
mobileShellLog.error("paired mac store setActive failed mac=\(macDeviceID, privacy: .public) error=\(String(describing: error), privacy: .public)")
}
} else if previousActive != nil, connectionState != .connected {
// The switch did not connect and the destructive connect path dropped
// the previous session; reconnect to the still-active previous Mac so
// the user is not left stranded on a failed switch.
_ = await reconnectActiveMacIfAvailable(stackUserID: identityProvider?.currentUserID)
}
await loadPairedMacs()
Comment thread
cursor[bot] marked this conversation as resolved.
Comment on lines +621 to +655

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 switchToMac drops the live connection before the new one is established

connectManualHost calls beginPairingAttempt() (which cancels pending remote operation tasks) then awaits manualHostTicket(...) over the network. If that server round-trip fails — Mac B offline, auth-server hiccup, or a timeout — the catch block calls clearRemoteConnectionContext() → replaceRemoteClient(with: nil), which disconnects the current Mac A RPC connection. The user ends up disconnected from both Macs.

The doc comment directly above promises the opposite: "a failed switch…leaves the previously-working Mac active and reachable." That is true of the SQLite record (is_active is not updated on failure), but false of the live socket connection, which is severed during the failed attempt. Users switching to an offline Mac lose their working session unexpectedly. The fix would be to either snapshot and restore the existing remoteClient on failure, or at minimum correct the doc comment to match actual behavior.

Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

/// Forget `macDeviceID`. Always removes the selected stored row by its real
/// id, and additionally tears down the live connection when that row is the
/// active one (the live attach ticket can carry a transient manual id, so we
/// must not rely on it to identify the row being forgotten).
/// - Parameter macDeviceID: The stored Mac to forget.
public func forgetMac(macDeviceID: String) async {
let isActiveMac = pairedMacs.first(where: { $0.macDeviceID == macDeviceID })?.isActive ?? false
if isActiveMac, connectionState == .connected {
disconnectLiveConnection()
}
Comment thread
cursor[bot] marked this conversation as resolved.
Comment on lines +665 to +667

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 forgetMac only calls disconnectLiveConnection() when connectionState == .connected, but this misses the case where a reconnection attempt is already in-flight (started by reconnectActiveMacIfAvailable on network change). In that state, connectionState is still .disconnected (no .connecting intermediate), so the guard is false, the live-connection teardown is skipped, and beginPairingAttempt's attempt ID is never invalidated. The await pairedMacStore?.remove(...) suspends the MainActor, the in-flight connectManualHost resumes past its next guard isCurrentPairingAttempt checkpoint, succeeds, and sets connectionState = .connected — leaving the device live-connected to a Mac that was just deleted from the store. Calling disconnectLiveConnection() unconditionally for the active Mac (when not connected it is a no-op for state, but it does rotate pairingAttemptID) aborts the pending attempt.

Suggested change
if isActiveMac, connectionState == .connected {
disconnectLiveConnection()
}
if isActiveMac {
disconnectLiveConnection()
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forget skips live host disconnect

Medium Severity

forgetMac only calls disconnectLiveConnection when the removed row has isActive in the store. A live session can target a paired Mac whose isActive flag is stale, so forgetting that Mac deletes it from SQLite while the RPC client keeps running.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 27b7020. Configure here.

do {
try await pairedMacStore?.remove(macDeviceID: macDeviceID)
} catch {
mobileShellLog.error("paired mac store remove failed mac=\(macDeviceID, privacy: .public) error=\(String(describing: error), privacy: .public)")
}
await loadPairedMacs()
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.
}
Comment on lines +663 to +674

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Stale list after forgetting the active Mac

When macDeviceID == activeMacDeviceID, disconnectAndForgetActiveMac() is synchronous and fires an unstructured Task internally to call pairedMacStore.remove(...). Control returns to forgetMac immediately, which then awaits loadPairedMacs(). Because the removal and the reload are now two independent tasks on the MainActor executor, loadPairedMacs() can read the store before the remove task has written its deletion — the forgotten Mac stays in pairedMacs with no subsequent refresh scheduled. The non-active path correctly awaits the removal before reloading, so that branch is clean.

The fix is to await the removal explicitly in the active-Mac path rather than relying on disconnectAndForgetActiveMac's internal fire-and-forget Task, or to call loadPairedMacs() from inside disconnectAndForgetActiveMac's remove-completion handler.

Comment thread
coderabbitai[bot] marked this conversation as resolved.

static func firstReconnectHostPortRoute(
_ routes: [CmxAttachRoute],
supportedKinds: [CmxAttachTransportKind]
Expand Down Expand Up @@ -672,14 +777,23 @@ public final class MobileShellComposite: MobileTerminalOutputSinking {
/// session starts from a fresh QR scan. Clears in-memory state and the
/// persisted active flag (other macs in SQLite stay, but none are marked
/// active so reconnect-on-launch is a no-op until the user pairs again).
public func disconnectAndForgetActiveMac() {
let staleMacID = activeTicket?.macDeviceID
/// Tear down the live connection and reset connection UI state, without
/// touching the paired-Mac store.
private func disconnectLiveConnection() {
pairingAttemptID = UUID()
connectionError = nil
connectionRequiresReauth = false
connectionState = .disconnected
macConnectionStatus = .unavailable
clearRemoteConnectionContext()
}
Comment on lines +782 to +789

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Cancel recovery when the user explicitly disconnects.

disconnectLiveConnection() clears the UI state, but it leaves recoveryTask / recoveryInFlight alive. If a retry or network-triggered reconnect is already running, it can reconnect the just-forgotten Mac after the user chose to disconnect.

Suggested fix
 private func disconnectLiveConnection() {
+    recoveryTask?.cancel()
+    recoveryTask = nil
+    recoveryInFlight = false
+    isRecoveringConnection = false
+    connectionRecoveryFailed = false
     pairingAttemptID = UUID()
     connectionError = nil
     connectionRequiresReauth = false
     connectionState = .disconnected
     macConnectionStatus = .unavailable
     clearRemoteConnectionContext()
 }
🤖 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 702 - 709, disconnectLiveConnection() currently resets UI state but
doesn't stop any ongoing reconnection work; cancel any in-progress recovery when
the user explicitly disconnects by checking and cancelling recoveryTask (and
setting it to nil) and clearing recoveryInFlight (set to false) inside
disconnectLiveConnection(), ensuring any async Task is cancelled/awaited as
appropriate so a retry or network-triggered reconnect cannot revive the
connection after explicit disconnect.


/// Disconnect the live connection and forget the currently-active paired Mac
/// (drops it from the store), returning the UI to the pairing flow. Backs the
/// "Rescan QR" action.
public func disconnectAndForgetActiveMac() {
let staleMacID = activeTicket?.macDeviceID
disconnectLiveConnection()
if let pairedMacStore, let macID = staleMacID {
// Fire-and-forget: forgetting the persisted mac is cleanup that must
// not block the synchronous disconnect UI state update above.
Expand Down
2 changes: 2 additions & 0 deletions Packages/CmuxMobileShellUI/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ let package = Package(
.package(path: "../CmuxAuthRuntime"),
.package(path: "../CmuxMobileCamera"),
.package(path: "../CmuxMobileDiagnostics"),
.package(path: "../CmuxMobilePairedMac"),
.package(path: "../CmuxMobileShell"),
.package(path: "../CmuxMobileShellModel"),
.package(path: "../CmuxMobileSupport"),
Expand All @@ -33,6 +34,7 @@ let package = Package(
"CmuxAuthRuntime",
"CmuxMobileCamera",
"CmuxMobileDiagnostics",
"CmuxMobilePairedMac",
"CmuxMobileShell",
"CmuxMobileShellModel",
"CmuxMobileSupport",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
#if os(iOS)
import CmuxMobilePairedMac
import CmuxMobileShell
import CmuxMobileSupport
import SwiftUI

/// Lets the user switch which paired Mac this device controls, and pair another.
///
/// Lists every Mac paired with this device (from the on-device store), marks the
/// one the live connection targets, switches on tap, forgets on swipe, and pairs
/// a new Mac by scanning its QR code without dropping the others.
struct MobileHostPickerView: View {
@Bindable var store: CMUXMobileShellStore
@Environment(\.dismiss) private var dismiss
@State private var showingScanner = false

var body: some View {
NavigationStack {
List {
Section {
if store.pairedMacs.isEmpty {
Text(L10n.string("mobile.hostPicker.empty", defaultValue: "No paired Macs yet."))
.foregroundStyle(.secondary)
}
ForEach(store.pairedMacs) { mac in
macRow(mac)
}
} header: {
Text(L10n.string("mobile.hostPicker.header", defaultValue: "Paired Macs"))
} footer: {
Text(L10n.string(
"mobile.hostPicker.footer",
defaultValue: "Switch which Mac this device controls. Pairing another Mac keeps the others, so you can hop between them."
))
}

Section {
Button {
showingScanner = true
} label: {
Label(
L10n.string("mobile.hostPicker.addMac", defaultValue: "Pair Another Mac"),
systemImage: "plus"
)
}
.accessibilityIdentifier("MobileHostPickerAddMac")
}
}
.navigationTitle(L10n.string("mobile.hostPicker.title", defaultValue: "Switch Mac"))
.navigationBarTitleDisplayMode(.inline)
.toolbar {
ToolbarItem(placement: .confirmationAction) {
Button(L10n.string("mobile.common.done", defaultValue: "Done")) {
dismiss()
}
.accessibilityIdentifier("MobileHostPickerDone")
}
}
.task { await store.loadPairedMacs() }
.sheet(isPresented: $showingScanner) {
MobilePairingScannerSheet { code in
showingScanner = false
Task {
_ = await store.connectPairingURL(code)
await store.loadPairedMacs()
dismiss()
}
Comment thread
cursor[bot] marked this conversation as resolved.
}
Comment on lines +61 to +68

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 QR scan severs existing session on failure, then unconditionally dismisses the picker

connectPairingURL calls beginPairingAttempt() (which rotates pairingAttemptID) and then on any failure — bad QR, decode error, network timeout — calls clearRemoteConnectionContext() before returning .failed. That tears down the live Mac A connection. After that, dismiss() fires regardless of the return value, so the user is ejected from the picker and returned to the workspace list with no active connection.

switchToMac's corresponding failure path at least attempts a fallback via reconnectActiveMacIfAvailable; this path has no fallback and no guard. The user opens "Pair Another Mac", scans an expired or wrong QR, and silently loses their working session.

The fix is to check the return value of connectPairingURL and only call dismiss() on success, and/or use store.reconnectActiveMacIfAvailable as a fallback when the pairing fails.

}
}
.accessibilityIdentifier("MobileHostPicker")
}

@ViewBuilder
private func macRow(_ mac: MobilePairedMac) -> some View {
let isActive = mac.isActive
Button {
Task { await store.switchToMac(macDeviceID: mac.macDeviceID) }
} label: {
HStack(spacing: 12) {
Image(systemName: "desktopcomputer")
.foregroundStyle(.secondary)
VStack(alignment: .leading, spacing: 2) {
Text(mac.displayName ?? mac.macDeviceID)
.foregroundStyle(.primary)
Text(mac.lastSeenAt, format: .relative(presentation: .named))
.font(.caption)
.foregroundStyle(.secondary)
}
Spacer(minLength: 8)
if isActive {
Image(systemName: "checkmark")
.foregroundStyle(Color.accentColor)
.accessibilityLabel(L10n.string("mobile.hostPicker.active", defaultValue: "Active"))
}
}
.contentShape(Rectangle())
}
.buttonStyle(.plain)
.accessibilityIdentifier("MobileHostPickerRow-\(mac.macDeviceID)")
.swipeActions(edge: .trailing) {
Button(role: .destructive) {
Task { await store.forgetMac(macDeviceID: mac.macDeviceID) }
} label: {
Label(L10n.string("mobile.hostPicker.forget", defaultValue: "Forget"), systemImage: "trash")
}
.accessibilityIdentifier("MobileHostPickerForget-\(mac.macDeviceID)")
}
Comment on lines +77 to +108

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unlifecycled fire-and-forget Tasks for switch and forget actions

Both Task { await store.switchToMac(...) } (tap) and Task { await store.forgetMac(...) } (swipe) are unstructured and untracked. A rapid double-tap or swipe queues a second operation before the first completes: switchToMac's guard (macDeviceID != activeMacDeviceID) won't catch the duplicate because activeMacDeviceID still reflects the old value during the first task's await. forgetMac has no guard at all. For actions that trigger reconnection or deletion side-effects, these should be tracked tasks (stored on the store or on a @State binding) so that in-flight operations can be cancelled or gated before a second one starts.

Rule Used: Flag new legacy async patterns in cmux-owned Swift... (source)

}
}
#endif
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#if os(iOS)
import CmuxAuthRuntime
import CmuxMobileShell
import CmuxMobileSupport
import SwiftUI

Expand All @@ -13,9 +14,13 @@ struct MobileSettingsView: View {
let connectedHostName: String
let rescanQR: (() -> Void)?
let signOut: (() -> Void)?
/// The shell store, used to drive the multi-Mac switcher. `nil` in previews,
/// where the "Switch Mac" entry is hidden.
var store: CMUXMobileShellStore?

@Environment(\.dismiss) private var dismiss
@State private var showingShortcuts = false
@State private var showingHostPicker = false

var body: some View {
NavigationStack {
Expand Down Expand Up @@ -58,6 +63,17 @@ struct MobileSettingsView: View {
value: connectedHostName
)
}
if store != nil {
Button {
showingHostPicker = true
} label: {
Label(
L10n.string("mobile.settings.switchMac", defaultValue: "Switch Mac"),
systemImage: "macbook.and.iphone"
)
}
.accessibilityIdentifier("MobileSettingsSwitchMac")
}
if let rescanQR {
Button {
rescanQR()
Expand Down Expand Up @@ -117,6 +133,11 @@ struct MobileSettingsView: View {
.sheet(isPresented: $showingShortcuts) {
TerminalShortcutsSettingsView()
}
.sheet(isPresented: $showingHostPicker) {
if let store {
MobileHostPickerView(store: store)
}
}
}
.accessibilityIdentifier("MobileSettingsView")
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import CmuxMobileShell
import CmuxMobileShellModel
import CmuxMobileSupport
import SwiftUI
Expand All @@ -20,6 +21,9 @@ struct WorkspaceListView: View {
/// previews), the menu is hidden.
var rescanQR: (() -> Void)?
var signOut: (() -> Void)?
/// The shell store, forwarded to Settings to drive the multi-Mac switcher.
/// `nil` in previews.
var store: CMUXMobileShellStore?
/// Optional: rename a workspace on the Mac. When present, each row offers a
/// Rename context-menu action.
var renameWorkspace: ((MobileWorkspacePreview.ID, String) -> Void)?
Expand Down Expand Up @@ -107,7 +111,8 @@ struct WorkspaceListView: View {
MobileSettingsView(
connectedHostName: host,
rescanQR: rescanQR,
signOut: signOut
signOut: signOut,
store: store
)
}
#endif
Expand Down
Loading
Loading