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
4 changes: 2 additions & 2 deletions .github/swift-file-length-budget.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# Format: max_lines<TAB>relative path
# Reduce counts as files shrink. CI fails if tracked files exceed this budget.
32655 CLI/cmux.swift
22074 Sources/TerminalController.swift
22073 Sources/TerminalController.swift
19820 Sources/Workspace.swift
19209 Sources/ContentView.swift
18011 Sources/AppDelegate.swift
Expand All @@ -28,9 +28,9 @@
3937 Sources/Feed/FeedPanelView.swift
3760 cmuxTests/TabManagerUnitTests.swift
3699 cmuxTests/CLIGenericHookPersistenceTests.swift
3527 Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift
3396 Sources/CmuxConfig.swift
3316 cmuxTests/TabManagerSessionSnapshotTests.swift
3309 Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift
3202 Sources/Update/UpdateTitlebarAccessory.swift
2953 Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
2877 Sources/SessionIndexView.swift
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
public import CMUXMobileCore
public import CmuxMobileShellModel

/// A best-effort lookup of fresher attach routes for a paired Mac from the
/// team-scoped device registry.
Expand All @@ -21,4 +22,32 @@ public protocol DeviceRegistryRefreshing: Sendable {
/// `nil` and `[]` are both treated as "no fresher routes" by
/// ``DeviceRegistryService/selectReconnectRoutes(local:registry:)``.
func freshRoutes(forMacDeviceID macDeviceID: String) async -> [CmxAttachRoute]?

/// List the team's registered devices and their running cmux app instances,
/// for the device tree (device → tags → workspaces).
///
/// The same team-scoped `GET /api/devices` response that backs
/// ``freshRoutes(forMacDeviceID:)``, decoded into the full two-level model
/// rather than narrowed to one Mac's routes. Returns a three-way outcome so
/// the caller can tell a transient failure (keep the current tree) from an
/// auth/scope rejection (clear it). The registry is team-scoped, so a 401/403
/// after the token/scope changed must NOT keep the previous scope's
/// team-device data visible.
func listDevices() async -> DeviceRegistryListOutcome
}

/// The outcome of a device-list registry read, distinguishing the cases that
/// must clear the cached team-device data from those that must keep it.
public enum DeviceRegistryListOutcome: Sendable {
/// A successful read; the decoded device list (possibly empty).
case ok([RegistryDevice])
/// The registry rejected the call on authorization/scope grounds (a non-2xx
/// 401/403). The cached, possibly other-scope, device data must be cleared so
/// it cannot leak into the new auth context; the UI falls back to local
/// paired Macs.
case authRejected
/// A transient failure (network error, timeout, malformed body, or any other
/// non-auth non-2xx). The current device list should be kept so a blip never
/// blanks a populated tree.
case transientFailure
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
public import CMUXMobileCore
public import CmuxMobileShellModel
public import Foundation
import os

Expand Down Expand Up @@ -154,8 +155,120 @@ public actor DeviceRegistryService: DeviceRegistryRefreshing {
return Self.routes(forMacDeviceID: macDeviceID, in: data)
}

public func listDevices() async -> DeviceRegistryListOutcome {
// No request could be built (no valid session/tokens): treat as a
// transient failure rather than an auth rejection, since this is the
// signed-out / not-yet-bootstrapped case, not the registry actively
// rejecting the caller's scope.
guard let request = await makeRequest(method: "GET", path: "/api/devices", body: nil) else {
return .transientFailure
}
let data: Data
do {
let (responseData, response) = try await session.data(for: request)
guard let http = response as? HTTPURLResponse else {
return .transientFailure
}
// An auth/scope rejection (401/403) must clear the cached team-scoped
// data; any other non-2xx (5xx, etc.) is transient and keeps it.
if http.statusCode == 401 || http.statusCode == 403 {
return .authRejected
}
guard (200...299).contains(http.statusCode) else {
return .transientFailure
}
data = responseData
} catch {
deviceRegistryLog.debug("listDevices request failed: \(String(describing: error), privacy: .public)")
return .transientFailure
}
// A 2xx with an undecodable body is a server/contract glitch, not an auth
// rejection: keep the current tree rather than blanking it.
guard let devices = Self.parseDeviceList(in: data) else {
return .transientFailure
}
return .ok(devices)
}

// MARK: - Parsing (pure, testable)

/// Decode the `/api/devices` list response into the full two-level device
/// tree (devices → app instances), for the device tree UI. Returns `nil` only
/// when the top-level envelope is undecodable; individual bad routes are
/// dropped (not fatal) so one malformed sibling can't blank the whole tree.
///
/// Each route is decoded *failably* and individually (same forward-compat
/// contract as ``routes(forMacDeviceID:in:)``): a malformed or unknown-kind
/// route is skipped rather than failing its instance, so an old client stays
/// forward-compatible when a newer build advertises a route kind it cannot
/// decode. `lastSeenAt` is parsed leniently (ISO8601, with or without
/// fractional seconds), defaulting to ``Date/distantPast`` when absent so a
/// device still renders, just sorted oldest.
static func parseDeviceList(in data: Data) -> [RegistryDevice]? {
struct FailableRoute: Decodable {
let value: CmxAttachRoute?
init(from decoder: Decoder) throws {
value = try? CmxAttachRoute(from: decoder)
}
}
struct Instance: Decodable {
let tag: String?
let routes: [FailableRoute]?
let lastSeenAt: String?
}
struct Device: Decodable {
let deviceId: String
let platform: String?
let displayName: String?
let lastSeenAt: String?
let instances: [Instance]?
}
struct ListResponse: Decodable {
let devices: [Device]
}
guard let decoded = try? JSONDecoder().decode(ListResponse.self, from: data) else {
return nil
}
return decoded.devices.compactMap { device -> RegistryDevice? in
let deviceId = device.deviceId.trimmingCharacters(in: .whitespacesAndNewlines)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Normalize registry device IDs before comparing them

When the registry-backed tree is used for a Mac whose MobileHostIdentity.deviceID() is the default uppercase UUID, this preserves the GET deviceId casing even though web/app/api/devices/route.ts lowercases it on POST and returns that stored value. The new tree then compares this lowercase device.deviceId case-sensitively against connectedMacDeviceID in sorting and DeviceTreeView, so the currently connected Mac is not marked live, its workspaces do not appear under the active tag, and tapping Connect on the already-connected instance can create a duplicate lowercase paired-Mac row.

Useful? React with 👍 / 👎.

guard !deviceId.isEmpty else { return nil }
let instances = (device.instances ?? []).map { instance in
RegistryAppInstance(
tag: instance.tag?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == false
? instance.tag! : "default",
routes: (instance.routes ?? []).compactMap(\.value),
lastSeenAt: Self.parseTimestamp(instance.lastSeenAt)
)
}
return RegistryDevice(
deviceId: deviceId,
platform: device.platform?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == false
? device.platform! : "mac",
Comment on lines +245 to +246

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Document the platform default fallback.

When platform is missing or empty, the code silently defaults to "mac", which makes the device appear controllable via isControllableHost. If the server has a bug or omits the field for an unsupported platform, the device will incorrectly show as a Mac host in the tree.

Add a comment explaining this default choice (e.g., backward compatibility, best-effort handling, or defensive programming against server bugs).

📝 Suggested documentation addition
         return RegistryDevice(
             deviceId: deviceId,
+            // Default to "mac" when platform is missing/empty for backward
+            // compatibility; isControllableHost will still filter correctly.
             platform: device.platform?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == false
                 ? device.platform! : "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/DeviceRegistryService.swift`
around lines 245 - 246, Add an inline comment next to the platform fallback
assignment in DeviceRegistryService (the line using
device.platform?.trimmingCharacters... ? device.platform! : "mac") documenting
why we default to "mac" when platform is missing or empty (e.g., for backward
compatibility / best-effort handling and to guard against server bugs) and note
the risk that this makes the device appear controllable via isControllableHost;
keep the comment concise and mention any intended behavior or future TODO to
avoid silent misclassification.

displayName: device.displayName,
lastSeenAt: Self.parseTimestamp(device.lastSeenAt),
instances: instances
)
}
}

/// Lenient ISO8601 parse for the registry's `lastSeenAt` strings. The server
/// emits `Date.toISOString()` (always fractional seconds), but tolerate the
/// non-fractional form too. An absent/unparseable value yields
/// ``Date/distantPast`` so the device still renders rather than being dropped.
///
/// The formatters are created per call rather than cached in a `static` so
/// this stays `Sendable`-clean under strict concurrency (`ISO8601DateFormatter`
/// is not `Sendable`). This runs once per `/api/devices` response, not on any
/// hot path, so the allocation is negligible.
static func parseTimestamp(_ value: String?) -> Date {
guard let value, !value.isEmpty else { return .distantPast }
let withFraction = ISO8601DateFormatter()
withFraction.formatOptions = [.withInternetDateTime, .withFractionalSeconds]
if let date = withFraction.date(from: value) { return date }
if let date = ISO8601DateFormatter().date(from: value) { return date }
return .distantPast
}

/// Decode the `/api/devices` list response and return the routes for the
/// device whose id matches `macDeviceID`, preferring its most recently seen
/// app instance. Returns `nil` when the device or routes are absent so the
Expand Down
Loading
Loading