Repository navigation
Device registry P1: auto-pair the phone on reload #5626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c12d392
4efa7c2
90fc5e4
9a86cb4
536a857
cab19fa
b61ace6
7aa8606
8de2e91
17562bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| public import CMUXMobileCore | ||
|
|
||
| /// A best-effort lookup of fresher attach routes for a paired Mac from the | ||
| /// team-scoped device registry. | ||
| /// | ||
| /// The registry is a rendezvous layer, not an authority: it lets a re-launched | ||
| /// phone discover the current routes for the Mac it last paired with (e.g. when | ||
| /// the Mac moved networks or restarted on a different port). It is deliberately | ||
| /// fallible — a `nil` result means "registry unavailable, use what you have," so | ||
| /// reconnect always falls back to the locally persisted paired-Mac routes and | ||
| /// pairing survives the cloud registry being down. | ||
| /// | ||
| /// The pure reconnect route-selection policy lives on ``DeviceRegistryService`` | ||
| /// (`selectReconnectRoutes` / `shouldApplyRegistryRefresh`). | ||
| public protocol DeviceRegistryRefreshing: Sendable { | ||
| /// Fetch the registry's current routes for the given Mac device id, scoped to | ||
| /// the signed-in user's team. | ||
| /// | ||
| /// - Returns: The registry's routes for that Mac, or `nil` when the registry | ||
| /// is unreachable, the call is unauthorized, or the Mac is not registered. | ||
| /// `nil` and `[]` are both treated as "no fresher routes" by | ||
| /// ``DeviceRegistryService/selectReconnectRoutes(local:registry:)``. | ||
| func freshRoutes(forMacDeviceID macDeviceID: String) async -> [CmxAttachRoute]? | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,231 @@ | ||
| public import CMUXMobileCore | ||
| public import Foundation | ||
| import os | ||
|
|
||
| private let deviceRegistryLog = Logger(subsystem: "com.cmuxterm.app", category: "DeviceRegistry") | ||
|
|
||
| /// HTTP client for the team-scoped device registry (`/api/devices`). | ||
| /// | ||
| /// Looks up fresher attach routes for a paired Mac on reload. P1 only needs the | ||
| /// phone to *read* the team's Macs; registering the phone itself as a `device` | ||
| /// row is deferred to the key-pinning phase (a phone row only matters once it | ||
| /// anchors a pinned key for revoke). `deviceID` is already plumbed here so that | ||
| /// phase has the persisted identity ready. | ||
| /// | ||
| /// Auth mirrors ``PushRegistrationService``: native calls send | ||
| /// `Authorization: Bearer <access>` + `X-Stack-Refresh-Token: <refresh>`, plus an | ||
| /// optional `X-Cmux-Team-Id` so the server scopes to the chosen team (defaults to | ||
| /// the Stack-selected team when omitted). Tokens are supplied through injected | ||
| /// Sendable closures so this service needs no dependency on the auth package. | ||
| /// | ||
| /// Every call is best-effort and failure-tolerant: a thrown/timed-out request | ||
| /// yields `nil` so reconnect falls back to locally persisted routes and pairing | ||
| /// survives the registry being down. | ||
| public actor DeviceRegistryService: DeviceRegistryRefreshing { | ||
| /// Supplies the bearer/refresh tokens for an authenticated request, or `nil` | ||
| /// when there is no valid session. | ||
| public struct TokenSource: Sendable { | ||
| public var accessToken: @Sendable () async -> String? | ||
| public var refreshToken: @Sendable () async -> String? | ||
|
|
||
| public init( | ||
| accessToken: @escaping @Sendable () async -> String?, | ||
| refreshToken: @escaping @Sendable () async -> String? | ||
| ) { | ||
| self.accessToken = accessToken | ||
| self.refreshToken = refreshToken | ||
| } | ||
| } | ||
|
|
||
| private let apiBaseURL: String | ||
| private let deviceID: String | ||
| private let tokenSource: TokenSource | ||
| private let teamIDProvider: @Sendable () async -> String? | ||
| private let session: URLSession | ||
| private let requestTimeout: TimeInterval | ||
|
|
||
| /// - Parameters: | ||
| /// - apiBaseURL: The cmux web API base URL (no trailing slash). | ||
| /// - deviceID: This iOS device's registry id (``deviceID(defaults:)``). | ||
| /// - tokenSource: Supplies the Stack access/refresh tokens. | ||
| /// - teamIDProvider: Supplies the team id to scope to, or `nil` to let the | ||
| /// server use the Stack-selected team. | ||
| /// - session: The URLSession used for API calls. | ||
| /// - requestTimeout: Per-request deadline, bounding the worst-case latency | ||
| /// of a registry call so it never stalls the reconnect refresh. | ||
| public init( | ||
| apiBaseURL: String, | ||
| deviceID: String, | ||
| tokenSource: TokenSource, | ||
| teamIDProvider: @escaping @Sendable () async -> String? = { nil }, | ||
| session: sending URLSession = .shared, | ||
| requestTimeout: TimeInterval = 5 | ||
| ) { | ||
| self.apiBaseURL = apiBaseURL | ||
| self.deviceID = deviceID | ||
| self.tokenSource = tokenSource | ||
| self.teamIDProvider = teamIDProvider | ||
| self.session = session | ||
| self.requestTimeout = requestTimeout | ||
| } | ||
|
|
||
| // MARK: - Device identity | ||
|
|
||
| private static let deviceIDKey = "cmux.deviceRegistry.iosDeviceID" | ||
|
|
||
| /// This iOS device's stable cmux identity for the device registry. | ||
| /// | ||
| /// A cmux-GENERATED persisted UUID (NOT `identifierForVendor`, which resets | ||
| /// when the last cmux app is removed, and NOT a hardware fingerprint). | ||
| /// Persisted in `UserDefaults` so it survives relaunch and reinstall, is | ||
| /// cross-platform, and is user-renamable via its display name. Mirrors the | ||
| /// Mac side's `MobileHostIdentity.deviceID()`. The phone sends this id when | ||
| /// it registers itself as a device; the key-pinning phase will anchor a | ||
| /// pinned key to it for revoke. | ||
| /// - Parameter defaults: Persistence store (injected for tests). | ||
| public static func deviceID(defaults: UserDefaults = .standard) -> String { | ||
| if let existing = defaults.string(forKey: deviceIDKey), | ||
| !existing.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { | ||
| return existing | ||
| } | ||
| let generated = UUID().uuidString.lowercased() | ||
| defaults.set(generated, forKey: deviceIDKey) | ||
| return generated | ||
| } | ||
|
|
||
| // MARK: - Reconnect route policy (pure, testable) | ||
|
|
||
| /// Choose the routes to persist for the next reconnect. | ||
| /// | ||
| /// The reconnect path connects on `local` routes immediately (no added | ||
| /// latency on the common case) and only *replaces* the persisted routes when | ||
| /// the registry returns a usable, different set, so a stale-route Mac gets | ||
| /// rescued on the next reconnect trigger. Returns `nil` to signal "no change | ||
| /// needed" (registry unavailable, empty, or identical), letting callers skip | ||
| /// a redundant store write and fall back to the locally persisted routes. | ||
| public static func selectReconnectRoutes( | ||
| local: [CmxAttachRoute], | ||
| registry: [CmxAttachRoute]? | ||
| ) -> [CmxAttachRoute]? { | ||
| guard let registry, !registry.isEmpty else { return nil } | ||
| guard registry != local else { return nil } | ||
| return registry | ||
| } | ||
|
|
||
| /// Whether a background registry refresh may write back into the paired-Mac | ||
| /// store, re-evaluated *after* the network call. | ||
| /// | ||
| /// The refresh upserts with `markActive: true`, so it must not resurrect a | ||
| /// pairing the user removed or deactivated while the network call was in | ||
| /// flight. It is safe to apply only when the same user is still signed in and | ||
| /// the Mac it refreshed is still the active paired Mac. If the user signed | ||
| /// out, switched accounts, forgot the Mac, or switched to a different active | ||
| /// Mac, the captured user no longer matches, or the active Mac id is now | ||
| /// `nil`/different, so the write is rejected. | ||
| public static func shouldApplyRegistryRefresh( | ||
| isSignedIn: Bool, | ||
| capturedUserID: String?, | ||
| currentUserID: String?, | ||
| activeMacID: String?, | ||
| targetMacID: String | ||
| ) -> Bool { | ||
| guard isSignedIn else { return false } | ||
| guard capturedUserID == currentUserID else { return false } | ||
| return activeMacID == targetMacID | ||
| } | ||
|
|
||
| // MARK: - DeviceRegistryRefreshing | ||
|
|
||
| public func freshRoutes(forMacDeviceID macDeviceID: String) async -> [CmxAttachRoute]? { | ||
| guard let request = await makeRequest(method: "GET", path: "/api/devices", body: nil) else { | ||
| return nil | ||
| } | ||
| let data: Data | ||
| do { | ||
| let (responseData, response) = try await session.data(for: request) | ||
| guard let http = response as? HTTPURLResponse, (200...299).contains(http.statusCode) else { | ||
| return nil | ||
| } | ||
| data = responseData | ||
| } catch { | ||
| deviceRegistryLog.debug("freshRoutes request failed: \(String(describing: error), privacy: .public)") | ||
| return nil | ||
| } | ||
| return Self.routes(forMacDeviceID: macDeviceID, in: data) | ||
| } | ||
|
|
||
| // MARK: - Parsing (pure, testable) | ||
|
|
||
| /// 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 | ||
| /// caller falls back to local routes. | ||
| /// | ||
| /// Each route is decoded *failably* and individually: a malformed or | ||
| /// unknown-kind route from any instance (even another Mac's) is skipped | ||
| /// rather than failing the whole response. This keeps one bad sibling row | ||
| /// from disabling registry refresh for every Mac, and makes old clients | ||
| /// forward-compatible when a newer build advertises a route kind they cannot | ||
| /// decode. | ||
| static func routes(forMacDeviceID macDeviceID: String, in data: Data) -> [CmxAttachRoute]? { | ||
| // Decode each route element through an optional wrapper so a single bad | ||
| // element decodes to `nil` and is dropped, never throwing for the array. | ||
| struct FailableRoute: Decodable { | ||
| let value: CmxAttachRoute? | ||
| init(from decoder: Decoder) throws { | ||
| value = try? CmxAttachRoute(from: decoder) | ||
| } | ||
| } | ||
| struct Instance: Decodable { | ||
| let routes: [FailableRoute] | ||
| } | ||
| struct Device: Decodable { | ||
| let deviceId: String | ||
| let instances: [Instance] | ||
| } | ||
| struct ListResponse: Decodable { | ||
| let devices: [Device] | ||
| } | ||
| guard let decoded = try? JSONDecoder().decode(ListResponse.self, from: data) else { | ||
| return nil | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| let target = macDeviceID.lowercased() | ||
| guard let device = decoded.devices.first(where: { $0.deviceId.lowercased() == target }) else { | ||
| return nil | ||
| } | ||
| // A Mac may run multiple tagged app instances (stable + a debug build). | ||
| // The phone's stored routes have no tag to match against in P1, so only | ||
| // substitute routes when exactly one instance is advertising any (the | ||
| // single-build common case). With zero or 2+ candidate instances, return | ||
| // nil and let reconnect fall back to the locally persisted routes, rather | ||
| // than risk connecting a stable phone to a different tagged build's | ||
| // workspaces. Tag-aware matching is a follow-up (see key-pinning phase). | ||
| let nonEmpty = device.instances | ||
| .map { $0.routes.compactMap(\.value) } | ||
| .filter { !$0.isEmpty } | ||
| return nonEmpty.count == 1 ? nonEmpty[0] : nil | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // MARK: - Request building | ||
|
|
||
| private func makeRequest(method: String, path: String, body: [String: Any]?) async -> URLRequest? { | ||
| guard let accessToken = await tokenSource.accessToken(), | ||
| let refreshToken = await tokenSource.refreshToken(), | ||
| let url = URL(string: apiBaseURL + path) else { | ||
| return nil | ||
| } | ||
| var request = URLRequest(url: url) | ||
| request.httpMethod = method | ||
| request.timeoutInterval = requestTimeout | ||
| request.setValue("Bearer \(accessToken)", forHTTPHeaderField: "Authorization") | ||
| request.setValue(refreshToken, forHTTPHeaderField: "X-Stack-Refresh-Token") | ||
| if let teamID = await teamIDProvider(), !teamID.isEmpty { | ||
| request.setValue(teamID, forHTTPHeaderField: "X-Cmux-Team-Id") | ||
| } | ||
| if let body { | ||
| request.setValue("application/json", forHTTPHeaderField: "Content-Type") | ||
| request.httpBody = try? JSONSerialization.data(withJSONObject: body) | ||
| } | ||
| return request | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -200,6 +200,11 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |
|
|
||
| private let runtime: (any MobileSyncRuntime)? | ||
| private let pairedMacStore: (any MobilePairedMacStoring)? | ||
| /// Best-effort, team-scoped lookup of fresher attach routes from the device | ||
| /// registry. Optional and failure-tolerant: when `nil` or unreachable, | ||
| /// reconnect uses the locally persisted paired-Mac routes, so pairing | ||
| /// survives the cloud registry being down. | ||
| private let deviceRegistry: (any DeviceRegistryRefreshing)? | ||
| private let identityProvider: (any MobileIdentityProviding)? | ||
| private let reachability: any ReachabilityProviding | ||
| private let pairingHintDefaults: UserDefaults | ||
|
|
@@ -323,6 +328,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |
| pairingCode: String = "", | ||
| workspaces: [MobileWorkspacePreview] = [], | ||
| pairedMacStore: (any MobilePairedMacStoring)? = nil, | ||
| deviceRegistry: (any DeviceRegistryRefreshing)? = nil, | ||
| clientIDRepository: MobileClientIDRepository = MobileClientIDRepository(defaults: .standard), | ||
| identityProvider: (any MobileIdentityProviding)? = nil, | ||
| reachability: any ReachabilityProviding = ReachabilityService(), | ||
|
|
@@ -332,6 +338,7 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |
| ) { | ||
| self.runtime = runtime | ||
| self.pairedMacStore = pairedMacStore | ||
| self.deviceRegistry = deviceRegistry | ||
| self.identityProvider = identityProvider | ||
| self.reachability = reachability | ||
| self.pairingHintDefaults = pairingHintDefaults | ||
|
|
@@ -915,6 +922,12 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |
| finishStoredMacReconnectAttempt(generation: generation) | ||
| return false | ||
| } | ||
| // Kick off a best-effort registry refresh for this Mac in the background. | ||
| // It does NOT block the connect below: the common case (fresh local | ||
| // routes) reconnects immediately with no network round-trip. If the Mac | ||
| // moved networks / changed port, the refreshed routes land in the store | ||
| // and the next reconnect trigger (network change or Retry) uses them. | ||
| refreshRoutesFromRegistry(for: mac, stackUserID: stackUserID) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the locally stored route is stale but Useful? React with 👍 / 👎. |
||
| let supportedKinds = runtime?.supportedRouteKinds ?? [] | ||
| guard let (host, port) = Self.firstReconnectHostPortRoute( | ||
| mac.routes, | ||
|
|
@@ -980,6 +993,63 @@ public final class MobileShellComposite: MobileTerminalOutputSinking { | |
| didFinishStoredMacReconnectAttempt = true | ||
| } | ||
|
|
||
| /// Best-effort, non-blocking registry refresh for the active paired Mac. | ||
| /// | ||
| /// Runs detached so it never adds latency to the in-flight reconnect (which | ||
| /// connects on the locally persisted routes). When the registry returns | ||
| /// usable, *different* routes for this Mac, they are written back into the | ||
| /// store so the next reconnect trigger (network change / Retry) reaches the | ||
| /// Mac at its current address after it moved networks or changed port. A | ||
| /// missing registry, an unauthorized call, or no-change routes are no-ops, so | ||
| /// a registry outage never disturbs the locally stored routes. | ||
| private func refreshRoutesFromRegistry(for mac: MobilePairedMac, stackUserID: String?) { | ||
| guard let deviceRegistry, let pairedMacStore else { return } | ||
| let macDeviceID = mac.macDeviceID | ||
| let localRoutes = mac.routes | ||
| let displayName = mac.displayName | ||
| Task { [weak self] in | ||
| let registryRoutes = await deviceRegistry.freshRoutes(forMacDeviceID: macDeviceID) | ||
| guard let updated = DeviceRegistryService.selectReconnectRoutes( | ||
| local: localRoutes, | ||
| registry: registryRoutes | ||
| ) else { return } | ||
| guard let self else { return } | ||
| // The network await above suspended; the user may have signed out, | ||
| // switched accounts, forgotten this Mac, or switched the active Mac | ||
| // meanwhile. Re-evaluate against the *current* store/identity before | ||
| // the `markActive: true` upsert, so a stale refresh can never | ||
| // resurrect or reactivate a pairing the user removed. Mirrors the | ||
| // user-switch guard in `loadPairedMacs`. | ||
| let activeMacID: String? | ||
| do { | ||
| activeMacID = try await pairedMacStore.activeMac(stackUserID: stackUserID)?.macDeviceID | ||
| } catch { | ||
| mobileShellLog.debug("registry refresh active-mac recheck failed: \(String(describing: error), privacy: .public)") | ||
| return | ||
| } | ||
| guard DeviceRegistryService.shouldApplyRegistryRefresh( | ||
| isSignedIn: self.isSignedIn, | ||
| capturedUserID: stackUserID, | ||
| currentUserID: self.identityProvider?.currentUserID, | ||
| activeMacID: activeMacID, | ||
| targetMacID: macDeviceID | ||
| ) else { return } | ||
| do { | ||
| try await pairedMacStore.upsert( | ||
| macDeviceID: macDeviceID, | ||
| displayName: displayName, | ||
| routes: updated, | ||
| markActive: true, | ||
| stackUserID: stackUserID | ||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } catch { | ||
| mobileShellLog.debug("registry route refresh upsert failed: \(String(describing: error), privacy: .public)") | ||
| return | ||
| } | ||
| await self.loadPairedMacs() | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
| } | ||
|
Comment on lines
+1013
to
+1051
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The goal of this path is route freshness only — |
||
|
|
||
| // MARK: - Paired Mac switching | ||
|
|
||
| /// Every Mac paired with this device, for the host switcher. Refreshed via | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale registry overwrites local routes
Medium Severity
When a device row belongs to another team member, the Mac cannot POST updated routes, but GET still returns that row to everyone on the team.
selectReconnectRoutestreats any differing registry routes as fresher and the shell persists them, so phones can replace good locally paired routes with outdated registry endpoints after an account switch or stale registration.Additional Locations (1)
web/app/api/devices/route.ts#L178-L181Reviewed by Cursor Bugbot for commit 17562bc. Configure here.