Repository navigation
iOS: hierarchical device tree (device → tags → workspaces) over the device registry - #5648
Merged
Merged
Conversation
…evice registry Render the merged #5626 device registry as a hierarchical tree: each registered device (Mac/host) expands to its cmux app instances (tags), and a tag expands to that build's workspaces; tapping a workspace opens it via the existing path. Surfaces the registry list to the UI (DeviceRegistryRefreshing.listDevices), adds a RegistryDevice/RegistryAppInstance value model, store.registryDevices + loadRegistryDevices + connectToRegistryInstance (connect-on-tap a non-connected tag via its routes), and a DeviceTreeView reachable from Settings. Keeps the flat workspace list and the multi-Mac switcher as the fallback paths. Online state: the connected device shows live macConnectionStatus; others show registry last-seen (best-effort, no per-host ping yet; the attach ticket carries no tag, so per-tag liveness is a TODO). Expansion persists via @AppStorage. Localized en+ja. Pure decode + expansion-codec tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… (autoreview P1s) P1-1: loadRegistryDevices now captures the requesting user id and discards a result that lands after a sign-out + different-user sign-in, so a slow registry load can't leak a previous user's team devices into the new user's tree (mirrors loadPairedMacs's user guard). P1-2: attribute live workspaces to the ONE instance whose route matches the live connection (instanceMatchesActiveRoute), not every tag on the connected device. A multi-tag Mac now shows workspaces only under the connected build; the other tags offer Connect instead of mirroring the wrong build's workspaces, so a workspace can no longer be opened under the wrong tag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…(autoreview) P1: connectToRegistryInstance now captures the previously-active Mac and, when the destructive connect fails to land on the target route, reconnects it (mirrors switchToMac). Tapping a stale/offline registry tag no longer drops a healthy live session; the user is left where they were. P2: add store.deviceTreeDevices, which honors the documented best-effort fallback: the registry list when loaded, otherwise the locally paired Macs synthesized into the same device→instance shape. The tree now sources from it and loads paired Macs first, so the Devices sheet stays usable (and connectable) during a registry outage instead of showing the empty state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…itch (autoreview P1) The previous rollback excluded the tapped device id (copied from switchToMac), which is wrong here: a Mac runs multiple tagged builds, so tapping another tag on the currently-connected device must still be able to reconnect that device's active route when the new tag is stale/offline. Capture the active paired Mac regardless of device id so a same-device tag-switch failure restores the live session instead of stranding the user disconnected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eview P1) listDevices() now returns a 3-way outcome (ok / authRejected / transientFailure) instead of an optional, so the store can distinguish a transient blip (keep the tree) from a 401/403 auth/scope rejection (clear it). The registry is team-scoped, so a token/scope change must not leave a previous scope's team-device names/tags/ routes visible; on authRejected the store clears registryDevices and the tree falls back to local paired Macs. Transient failures (5xx, network, malformed body) keep the current tree to avoid blip-blanking. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a team-scoped device-registry API and outcome type, registry data models and parsing, composite orchestration for loading/connecting, iOS device-tree UI with expansion persistence and sheet integration, localization, and unit tests. ChangesDevice Tree Feature
Possibly related PRs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (18 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}
![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: ab9bf3ddc4
ℹ️ 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".
Comment on lines
+1121
to
+1127
| case .authRejected: | ||
| // The registry is team-scoped and rejected the call on auth/scope | ||
| // grounds (401/403): the cached list may be another scope's data, so | ||
| // clear it. The tree falls back to local paired Macs via | ||
| // `deviceTreeDevices`, so the sheet stays usable. | ||
| registryDevices = [] | ||
| return |
There was a problem hiding this comment.
When an in-flight registry load was started for a previous account/team and the user signs into another account before it returns, a late 401/403 from the old request still clears registryDevices for the current account. The success path has a requestingUserID guard for this exact account-switch race, but the auth-rejected branch mutates state before that guard, so a new user's freshly loaded device tree can be blanked and fall back to local pairings until another refresh.
Useful? React with 👍 / 👎.
…anual-ticket active device (autoreview P2) P2 (sheet stack): move the device tree to a top-level sheet on the workspace list (a Devices toolbar button) instead of nesting it under the Settings sheet, so selecting a workspace dismisses straight back to the workspace shell and reveals the opened workspace rather than leaving Settings covering it. Removes the duplicate Settings 'Devices' entry. P2 (manual-ticket active): connectedMacDeviceID now falls back to the active paired Mac's real device id when the live ticket is a synthetic manual one (host without mobile.attach_ticket.create). The registry connect path persists the real device as active, so the tree now marks it connected and shows its live workspaces instead of hiding them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Greptile SummaryIntroduces a hierarchical Devices sheet for iOS (device → cmux build tags → workspaces), built on the merged device registry.
Confidence Score: 5/5Safe to merge; the account-switch guard is on both outcome branches and the failed-connect rollback is route-verified. The auth-rejection clear is now guarded on No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as DeviceTreeView
participant Store as MobileShellComposite
participant Reg as DeviceRegistryService
participant API as GET /api/devices
UI->>Store: .task → loadPairedMacs()
Store-->>UI: pairedMacs updated (fallback)
UI->>Store: loadRegistryDevices()
Store->>Reg: listDevices()
Reg->>API: GET /api/devices
alt 2xx
API-->>Reg: device list JSON
Reg-->>Store: .ok([RegistryDevice])
Store-->>UI: registryDevices updated (sorted)
else 401/403
API-->>Reg: auth error
Reg-->>Store: .authRejected
Store-->>UI: "registryDevices = [] (fallback to pairedMacs)"
else network/5xx
API-->>Reg: error
Reg-->>Store: .transientFailure
Store-->>UI: registryDevices unchanged
end
UI->>Store: connectToRegistryInstance(device, instance)
Store->>Store: capture previousActive
Store->>Store: connectManualHost(host, port)
alt connect succeeded on this route
Store->>Store: pairedMacStore.upsert(markActive: true)
Store->>Store: loadPairedMacs() + loadRegistryDevices()
Store-->>UI: workspaces appear under active instance
else connect failed / wrong route
Store->>Store: reconnectActiveMacIfAvailable (rollback)
Store-->>UI: connectionError surfaced
end
Reviews (8): Last reviewed commit: "chore: refresh file-length budget for th..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+206
to
+209
| let store = store | ||
| return { | ||
| Task { | ||
| await store.connectToRegistryInstance( |
Contributor
There was a problem hiding this comment.
Task with meaningful lifecycle — flagged by the cmux Swift concurrency rule. The task is unstructured (no stored handle, no cancellation), so if the user dismisses the sheet mid-connect or triggers a second tap before the first finishes, multiple concurrent connectToRegistryInstance calls can race, each potentially invoking the rollback path independently. The captured store reference is fine, but storing the returned Task value allows callers (or a future .onDisappear) to cancel in-flight connects when they are no longer relevant.
Suggested change
| let store = store | |
| return { | |
| Task { | |
| await store.connectToRegistryInstance( | |
| let store = store | |
| return { | |
| Task { [weak store] in | |
| await store?.connectToRegistryInstance( |
Comment on lines
+253
to
+257
| private func setExpanded(_ id: String, _ expanded: Bool) { | ||
| var store = expansion | ||
| store.setExpanded(id, expanded) | ||
| expandedStorage = store.storage | ||
| } |
Contributor
There was a problem hiding this comment.
var store = expansion shadows the self.store: CMUXMobileShellStore property. Swift resolves the type correctly here (the local is a DeviceTreeExpansionStore), but a reader scanning store.setExpanded or store.storage would naturally expect the shell store. Using a distinct name eliminates the ambiguity.
Suggested change
| private func setExpanded(_ id: String, _ expanded: Bool) { | |
| var store = expansion | |
| store.setExpanded(id, expanded) | |
| expandedStorage = store.storage | |
| } | |
| private func setExpanded(_ id: String, _ expanded: Bool) { | |
| var expansionStore = expansion | |
| expansionStore.setExpanded(id, expanded) | |
| expandedStorage = expansionStore.storage | |
| } |
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!
Comment on lines
+31
to
+34
| @AppStorage("cmux.mobile.deviceTree.expanded") private var expandedStorage = "" | ||
| @State private var isRefreshing = false | ||
|
|
||
| private var expansion: DeviceTreeExpansionStore { |
Contributor
There was a problem hiding this comment.
isRefreshing is declared but never read or mutated anywhere in the view — it's dead @State. While it won't cause stale renders (it's never set), it's clutter that could mislead a future reader into thinking a loading spinner or in-progress guard is wired.
Suggested change
| @AppStorage("cmux.mobile.deviceTree.expanded") private var expandedStorage = "" | |
| @State private var isRefreshing = false | |
| private var expansion: DeviceTreeExpansionStore { | |
| @AppStorage("cmux.mobile.deviceTree.expanded") private var expandedStorage = "" | |
| private var expansion: DeviceTreeExpansionStore { |
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!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/CmuxMobileShell/Sources/CmuxMobileShell/DeviceRegistryService.swift`:
- Around line 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.
In `@Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DeviceTreeView.swift`:
- Around line 150-155: The DeviceTreeInstanceCapture construction and subsequent
connect payloads are overwriting the original registry platform by hardcoding
platform: "mac"; update the DeviceTreeInstanceCapture (where captured is
created) to include the device's platform (e.g., add a platform: device.platform
field) and ensure any place that builds the connect payload (e.g., the code path
that calls connectToRegistryInstance and the payload construction around lines
creating captured) passes captured.platform through instead of the literal
"mac"; make sure the symbol DeviceTreeInstanceCapture and the
connectToRegistryInstance call use the captured.platform value so Linux/Windows
platforms are preserved end-to-end.
🪄 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: 69a48339-b8ec-4323-a3d7-b18d4dc12b7a
📒 Files selected for processing (13)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/DeviceRegistryRefreshing.swiftPackages/CmuxMobileShell/Sources/CmuxMobileShell/DeviceRegistryService.swiftPackages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/CmuxMobileShell/Tests/CmuxMobileShellTests/DeviceRegistryListParsingTests.swiftPackages/CmuxMobileShellModel/Sources/CmuxMobileShellModel/RegistryDevice.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DeviceTreeExpansionStore.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DeviceTreeRows.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DeviceTreeView.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileSettingsView.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftPackages/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/DeviceTreeExpansionStoreTests.swiftPackages/CmuxMobileSupport/Sources/CmuxMobileSupport/L10n.swiftios/cmux/Resources/Localizable.xcstrings
Comment on lines
+245
to
+246
| platform: device.platform?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == false | ||
| ? device.platform! : "mac", |
There was a problem hiding this comment.
🧹 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.
Comment on lines
+150
to
+155
| let captured = DeviceTreeInstanceCapture( | ||
| deviceId: device.deviceId, | ||
| displayName: device.displayName, | ||
| tag: instance.tag, | ||
| routes: instance.routes | ||
| ) |
There was a problem hiding this comment.
Preserve the registry platform when building connect payloads.
Line 212 hardcodes platform: "mac", which rewrites Linux/Windows devices before connectToRegistryInstance receives them. Pass through the captured platform to keep model contracts intact across layers.
Proposed fix
@@
let captured = DeviceTreeInstanceCapture(
deviceId: device.deviceId,
+ platform: device.platform,
displayName: device.displayName,
tag: instance.tag,
routes: instance.routes
)
@@
device: RegistryDevice(
deviceId: capture.deviceId,
- platform: "mac",
+ platform: capture.platform,
displayName: capture.displayName,
lastSeenAt: .distantPast,
instances: []
),
@@
private struct DeviceTreeInstanceCapture {
let deviceId: String
+ let platform: String
let displayName: String?
let tag: String
let routes: [CmxAttachRoute]Also applies to: 204-222, 263-269
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/DeviceTreeView.swift`
around lines 150 - 155, The DeviceTreeInstanceCapture construction and
subsequent connect payloads are overwriting the original registry platform by
hardcoding platform: "mac"; update the DeviceTreeInstanceCapture (where captured
is created) to include the device's platform (e.g., add a platform:
device.platform field) and ensure any place that builds the connect payload
(e.g., the code path that calls connectToRegistryInstance and the payload
construction around lines creating captured) passes captured.platform through
instead of the literal "mac"; make sure the symbol DeviceTreeInstanceCapture and
the connectToRegistryInstance call use the captured.platform value so
Linux/Windows platforms are preserved end-to-end.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7c8039906
ℹ️ 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".
| return nil | ||
| } | ||
| return decoded.devices.compactMap { device -> RegistryDevice? in | ||
| let deviceId = device.deviceId.trimmingCharacters(in: .whitespacesAndNewlines) |
There was a problem hiding this comment.
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 👍 / 👎.
Comment on lines
+1133
to
+1139
| case .authRejected: | ||
| // The registry is team-scoped and rejected the call on auth/scope | ||
| // grounds (401/403): the cached list may be another scope's data, so | ||
| // clear it. The tree falls back to local paired Macs via | ||
| // `deviceTreeDevices`, so the sheet stays usable. | ||
| registryDevices = [] | ||
| return |
Contributor
There was a problem hiding this comment.
authRejected path
The .ok path correctly checks identityProvider?.currentUserID == requestingUserID before writing to registryDevices, but the .authRejected path skips that guard entirely. If User A's request lands after User A signs out and User B has already loaded their own device list, the unconditional registryDevices = [] blanks User B's tree — exactly the account-switch leak the .ok guard was designed to prevent. A 401 on a stale session token is the most likely post-sign-out outcome, making this race realistic rather than theoretical.
Suggested change
| case .authRejected: | |
| // The registry is team-scoped and rejected the call on auth/scope | |
| // grounds (401/403): the cached list may be another scope's data, so | |
| // clear it. The tree falls back to local paired Macs via | |
| // `deviceTreeDevices`, so the sheet stays usable. | |
| registryDevices = [] | |
| return | |
| case .authRejected: | |
| // The registry is team-scoped and rejected the call on auth/scope | |
| // grounds (401/403): the cached list may be another scope's data, so | |
| // clear it. The tree falls back to local paired Macs via | |
| // `deviceTreeDevices`, so the sheet stays usable. | |
| // Apply the same account-switch guard as the .ok path: a stale | |
| // request that returned 401 after a different user signed in must | |
| // not blank the new user's device list. | |
| guard identityProvider?.currentUserID == requestingUserID else { return } | |
| registryDevices = [] | |
| return |
Contributor
Author
There was a problem hiding this comment.
Fixed in cb188c5: the .authRejected blanking is now guarded on identityProvider?.currentUserID == requestingUserID, mirroring the .ok path, so a stale 401 landing after an account switch no longer blanks the new user's tree.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift (1)
1127-1150:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStale registry loads should no-op before mutating state.
This captures
requestingUserID, but stale completions still writeregistryDevices:.authRejectedclears immediately, and the final mismatch guard also clears on user mismatch. If user A’s request finishes after user B has already signed in and loaded their tree, the older task can wipe B’s device list. Re-check the user right afterawait deviceRegistry.listDevices()and return on mismatch before any branch mutates state.Suggested fix
let requestingUserID = identityProvider?.currentUserID let outcome = await deviceRegistry.listDevices() + guard isSignedIn, identityProvider?.currentUserID == requestingUserID else { + return + } let loaded: [RegistryDevice] switch outcome { case .ok(let devices): loaded = devices case .authRejected: @@ case .transientFailure: // Network blip / 5xx / malformed body: keep what we have rather than // blanking a populated tree on a transient failure. return } - // The await above suspended the main actor; discard the result unless we - // are still the same signed-in user, so a slow load can never repopulate - // another user's team devices after sign-out or an account switch. - guard isSignedIn, identityProvider?.currentUserID == requestingUserID else { - registryDevices = [] - return - } let connectedID = connectedMacDeviceID registryDevices = loaded.sorted { lhs, rhs in🤖 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 1127 - 1150, The code captures requestingUserID then awaits deviceRegistry.listDevices(), but branches (especially the .authRejected case and the final guard) mutate registryDevices even if the current signed-in user changed; after the await and before any mutation, re-check identityProvider?.currentUserID against requestingUserID (and isSignedIn) and return early on mismatch so stale completions no-op; update the .authRejected and other branches to only clear or set registryDevices after that check (use the existing symbols: requestingUserID, identityProvider?.currentUserID, isSignedIn, deviceRegistry.listDevices(), registryDevices).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 1095-1106: The fallback that returns a paired Mac's macDeviceID
when activeTicket.macDeviceID is synthetic should be gated by confirming that
that paired Mac's stored routes include the current activeRoute; in the block
that currently finds activeMacID via pairedMacs.first(where: { $0.isActive }) in
MobileShellComposite (check symbols activeTicket, pairedMacs, activeMacID,
activeRoute, macDeviceID), verify the matched paired Mac’s routes (e.g., its
recorded connect path or routes collection) contains/equals activeRoute before
returning activeMacID — if the routes do not match, return nil instead of
falling back to that macDeviceID.
---
Outside diff comments:
In `@Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 1127-1150: The code captures requestingUserID then awaits
deviceRegistry.listDevices(), but branches (especially the .authRejected case
and the final guard) mutate registryDevices even if the current signed-in user
changed; after the await and before any mutation, re-check
identityProvider?.currentUserID against requestingUserID (and isSignedIn) and
return early on mismatch so stale completions no-op; update the .authRejected
and other branches to only clear or set registryDevices after that check (use
the existing symbols: requestingUserID, identityProvider?.currentUserID,
isSignedIn, deviceRegistry.listDevices(), registryDevices).
🪄 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: 56e7eb53-d910-4d07-9294-52b16576d03f
📒 Files selected for processing (2)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swift
Comment on lines
+1095
to
+1106
| if let macDeviceID = activeTicket?.macDeviceID, | ||
| !macDeviceID.isEmpty, | ||
| !macDeviceID.hasPrefix("manual-") { | ||
| return macDeviceID | ||
| } | ||
| // Manual/synthetic ticket but a live connection: correlate via the active | ||
| // paired Mac the connect path persisted (its id is the real device id). | ||
| if let activeMacID = pairedMacs.first(where: { $0.isActive })?.macDeviceID, | ||
| !activeMacID.isEmpty, | ||
| !activeMacID.hasPrefix("manual-") { | ||
| return activeMacID | ||
| } |
There was a problem hiding this comment.
Only fall back to the active paired Mac when the live route matches it.
A synthetic manual-* ticket is not unique to the registry-connect path. The generic manual-host flow can also leave activeTicket.macDeviceID synthetic while pairedMacs.first(where: { $0.isActive }) still points at a previously paired Mac. In that state this property reports the wrong device as connected, so the device tree can highlight another host and attach the live-workspace branch to the wrong row. Gate the fallback on the active paired Mac’s stored routes matching activeRoute, otherwise return nil.
Suggested fix
if let macDeviceID = activeTicket?.macDeviceID,
!macDeviceID.isEmpty,
!macDeviceID.hasPrefix("manual-") {
return macDeviceID
}
- // Manual/synthetic ticket but a live connection: correlate via the active
- // paired Mac the connect path persisted (its id is the real device id).
- if let activeMacID = pairedMacs.first(where: { $0.isActive })?.macDeviceID,
- !activeMacID.isEmpty,
- !activeMacID.hasPrefix("manual-") {
- return activeMacID
+ // Only trust the active paired Mac fallback when its stored route matches
+ // the live connection; a generic manual-host connect can also produce a
+ // synthetic `manual-*` ticket while some unrelated paired Mac remains active.
+ if case let .hostPort(liveHost, livePort)? = activeRoute?.endpoint,
+ let activeMac = pairedMacs.first(where: { mac in
+ mac.isActive && mac.routes.contains { route in
+ if case let .hostPort(host, port) = route.endpoint {
+ return host == liveHost && port == livePort
+ }
+ return false
+ }
+ }),
+ !activeMac.macDeviceID.isEmpty,
+ !activeMac.macDeviceID.hasPrefix("manual-") {
+ return activeMac.macDeviceID
}
return nil
}🤖 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 1095 - 1106, The fallback that returns a paired Mac's macDeviceID
when activeTicket.macDeviceID is synthetic should be gated by confirming that
that paired Mac's stored routes include the current activeRoute; in the block
that currently finds activeMacID via pairedMacs.first(where: { $0.isActive }) in
MobileShellComposite (check symbols activeTicket, pairedMacs, activeMacID,
activeRoute, macDeviceID), verify the matched paired Mac’s routes (e.g., its
recorded connect path or routes collection) contains/equals activeRoute before
returning activeMacID — if the routes do not match, return nil instead of
falling back to that macDeviceID.
lawrencecchen
added a commit
that referenced
this pull request
Jun 9, 2026
…nt stub Mac: PresenceHeartbeatClient follows the DeviceRegistryClient pattern (same device UUID and tag, best-effort, auth-gated), default OFF behind the presenceHeartbeatEnabled + presenceServiceURL defaults keys, with a server-owned cadence and a clean-quit goodbye. iOS: PresenceWire typed models + WebSocket subscribe stub in CmuxMobileShell, the seam for the device tree (#5648). 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: 24ae20ad04
ℹ️ 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".
| // are still the same signed-in user, so a slow load can never repopulate | ||
| // another user's team devices after sign-out or an account switch. | ||
| guard isSignedIn, identityProvider?.currentUserID == requestingUserID else { | ||
| registryDevices = [] |
There was a problem hiding this comment.
When a registry load for user A succeeds after the phone has switched to user B, this guard correctly detects the stale result, but it clears registryDevices instead of just returning. If user B's own load has already populated the tree, the late success from user A blanks B's devices until another refresh, so the account-switch race is still user-visible even on the 2xx path.
Useful? React with 👍 / 👎.
# Conflicts: # .github/swift-file-length-budget.tsv # ios/cmux/Resources/Localizable.xcstrings
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4177d5a799
ℹ️ 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".
| /// (and its connect-on-tap) keeps working with the cloud down. The connected | ||
| /// device sorts first, then most-recently-seen. | ||
| public var deviceTreeDevices: [RegistryDevice] { | ||
| if !registryDevices.isEmpty { return registryDevices } |
There was a problem hiding this comment.
When /api/devices returns any device, this bypasses the paired-Mac fallback entirely, so a user whose active/paired Mac has not registered yet (for example an older Mac build or a failed POST) loses that Mac from the new Devices sheet as soon as the team registry contains some other device. The fallback only helps when the registry list is empty; merging missing pairedMacs into the registry result would keep the locally usable Mac visible while still showing team devices.
Useful? React with 👍 / 👎.
…ng current Mirrors the .ok path's account-switch guard: a stale 401 from a signed-out session that lands after a different user signed in no longer blanks the new user's device tree. Addresses the Greptile P1 on the PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cb188c5. Configure here.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift (2)
1089-1302: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit the device-tree orchestration out of
MobileShellComposite.This file is already a multi-thousand-line façade that owns connection lifecycle, paired-Mac persistence, workspace/terminal state, transport, and analytics. Adding registry tree state/loading/connect logic here deepens the same god-object boundary the repo asks us to avoid. Please move this device-tree orchestration into a dedicated coordinator/model and keep
MobileShellCompositeas the composed surface. As per coding guidelines:Flag Swift production files that exceed 400 lines without a clear single responsibility, or exceed 800 lines even with mostly coherent responsibilityandFlag files that mix UI rendering, state ownership, persistence, networking, parsing, subprocess/socket protocol, and platform bridge code in one place.🤖 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 1089 - 1302, The device-tree logic (registryDevices, connectedMacDeviceID, loadRegistryDevices(), deviceTreeDevices, connectToRegistryInstance(), and related helpers) should be extracted from MobileShellComposite into a new DeviceTreeCoordinator (or DeviceRegistryModel) responsible for registry state, loading, sorting, and connect orchestration; move the properties (registryDevices, deviceTreeDevices), the loadRegistryDevices() implementation, connectedMacDeviceID computed logic, and connectToRegistryInstance() into that coordinator, inject dependencies currently used (deviceRegistry, identityProvider, pairedMacStore, runtime, mobileShellLog, reconnect helpers, etc.) via initializer, and expose minimal API/async methods the composite will call (e.g., loadRegistryDevices(), connectToRegistryInstance(device:instance:), and a publisher/closure or read-only properties for registryDevices/deviceTreeDevices/connectionError), update MobileShellComposite to forward calls and subscribe to the coordinator instead of owning the logic, and keep existing behavior (race/user-checks, fallback, sorting, persistence, and reconnect-on-failure) and tests intact.Source: Coding guidelines
1141-1173:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake
loadRegistryDevices()latest-request-wins.The new
requestingUserIDcheck only fixes one stale-result path. A slow response from an older load can still mutate current state: on Lines 1171-1173 a stale.okfrom the previous session will clear the current user's already-loaded tree instead of being ignored, and older completions can still beat newer ones because this method has no load generation/token.Suggested fix
+ private var registryLoadGeneration = 0 + public func loadRegistryDevices() async { guard isSignedIn, let deviceRegistry else { registryDevices = [] return } + registryLoadGeneration &+= 1 + let generation = registryLoadGeneration let requestingUserID = identityProvider?.currentUserID let outcome = await deviceRegistry.listDevices() + guard generation == registryLoadGeneration else { return } let loaded: [RegistryDevice] switch outcome { case .ok(let devices): loaded = devices case .authRejected: - if identityProvider?.currentUserID == requestingUserID { + if identityProvider?.currentUserID == requestingUserID { registryDevices = [] } return case .transientFailure: return } - guard isSignedIn, identityProvider?.currentUserID == requestingUserID else { - registryDevices = [] - return - } + guard isSignedIn, identityProvider?.currentUserID == requestingUserID else { return } let connectedID = connectedMacDeviceID registryDevices = loaded.sorted { lhs, rhs in🤖 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 1141 - 1173, loadRegistryDevices() can still let slow, older completions mutate state; make it latest-request-wins by adding a load generation/token that is incremented at the start of each invocation, captured in a local variable before awaiting deviceRegistry.listDevices(), and checked after each await/early-return to ensure only the newest load mutates registryDevices. Concretely: add a property like registryLoadGeneration (Int) on the actor/actor-isolated type, increment it at the start of loadRegistryDevices(), capture into a local let currentLoad = registryLoadGeneration, then after awaiting deviceRegistry.listDevices() and before any assignment/clearing of registryDevices (including in the .ok, .authRejected, .transientFailure and the final guard block that checks identityProvider?.currentUserID and isSignedIn) bail out if registryLoadGeneration != currentLoad so stale responses are ignored. Use the existing symbols loadRegistryDevices, registryDevices, identityProvider?.currentUserID, and deviceRegistry.listDevices() when applying this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 1089-1302: The device-tree logic (registryDevices,
connectedMacDeviceID, loadRegistryDevices(), deviceTreeDevices,
connectToRegistryInstance(), and related helpers) should be extracted from
MobileShellComposite into a new DeviceTreeCoordinator (or DeviceRegistryModel)
responsible for registry state, loading, sorting, and connect orchestration;
move the properties (registryDevices, deviceTreeDevices), the
loadRegistryDevices() implementation, connectedMacDeviceID computed logic, and
connectToRegistryInstance() into that coordinator, inject dependencies currently
used (deviceRegistry, identityProvider, pairedMacStore, runtime, mobileShellLog,
reconnect helpers, etc.) via initializer, and expose minimal API/async methods
the composite will call (e.g., loadRegistryDevices(),
connectToRegistryInstance(device:instance:), and a publisher/closure or
read-only properties for registryDevices/deviceTreeDevices/connectionError),
update MobileShellComposite to forward calls and subscribe to the coordinator
instead of owning the logic, and keep existing behavior (race/user-checks,
fallback, sorting, persistence, and reconnect-on-failure) and tests intact.
- Around line 1141-1173: loadRegistryDevices() can still let slow, older
completions mutate state; make it latest-request-wins by adding a load
generation/token that is incremented at the start of each invocation, captured
in a local variable before awaiting deviceRegistry.listDevices(), and checked
after each await/early-return to ensure only the newest load mutates
registryDevices. Concretely: add a property like registryLoadGeneration (Int) on
the actor/actor-isolated type, increment it at the start of
loadRegistryDevices(), capture into a local let currentLoad =
registryLoadGeneration, then after awaiting deviceRegistry.listDevices() and
before any assignment/clearing of registryDevices (including in the .ok,
.authRejected, .transientFailure and the final guard block that checks
identityProvider?.currentUserID and isSignedIn) bail out if
registryLoadGeneration != currentLoad so stale responses are ignored. Use the
existing symbols loadRegistryDevices, registryDevices,
identityProvider?.currentUserID, and deviceRegistry.listDevices() when applying
this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 96e4a99b-55f1-4c60-968e-1f94971d5617
📒 Files selected for processing (1)
Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift
# Conflicts: # ios/cmux/Resources/Localizable.xcstrings
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59acfbecc6
ℹ️ 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".
Comment on lines
+1171
to
+1172
| guard isSignedIn, identityProvider?.currentUserID == requestingUserID else { | ||
| registryDevices = [] |
There was a problem hiding this comment.
When the same signed-in user changes the selected team while a /api/devices load is in flight, this user-only stale-result guard still accepts the old response and assigns that previous team's registryDevices into the current Devices sheet. The registry client reads coordinator.resolvedTeamID live for each request, so the request can already have been sent with the old X-Cmux-Team-Id; capture the requested team (or validate the response teamId) before applying or clearing the cached team-scoped tree.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 12, 2026
…nt stub Mac: PresenceHeartbeatClient follows the DeviceRegistryClient pattern (same device UUID and tag, best-effort, auth-gated), default OFF behind the presenceHeartbeatEnabled + presenceServiceURL defaults keys, with a server-owned cadence and a clean-quit goodbye. iOS: PresenceWire typed models + WebSocket subscribe stub in CmuxMobileShell, the seam for the device tree (#5648). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 12, 2026
…nt stub Mac: PresenceHeartbeatClient follows the DeviceRegistryClient pattern (same device UUID and tag, best-effort, auth-gated), default OFF behind the presenceHeartbeatEnabled + presenceServiceURL defaults keys, with a server-owned cadence and a clean-quit goodbye. iOS: PresenceWire typed models + WebSocket subscribe stub in CmuxMobileShell, the seam for the device tree (#5648). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 12, 2026
…nt stub Mac: PresenceHeartbeatClient follows the DeviceRegistryClient pattern (same device UUID and tag, best-effort, auth-gated), default OFF behind the presenceHeartbeatEnabled + presenceServiceURL defaults keys, with a server-owned cadence and a clean-quit goodbye. iOS: PresenceWire typed models + WebSocket subscribe stub in CmuxMobileShell, the seam for the device tree (#5648). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 12, 2026
…nt stub Mac: PresenceHeartbeatClient follows the DeviceRegistryClient pattern (same device UUID and tag, best-effort, auth-gated), default OFF behind the presenceHeartbeatEnabled + presenceServiceURL defaults keys, with a server-owned cadence and a clean-quit goodbye. iOS: PresenceWire typed models + WebSocket subscribe stub in CmuxMobileShell, the seam for the device tree (#5648). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 12, 2026
…nt stub Mac: PresenceHeartbeatClient follows the DeviceRegistryClient pattern (same device UUID and tag, best-effort, auth-gated), default OFF behind the presenceHeartbeatEnabled + presenceServiceURL defaults keys, with a server-owned cadence and a clean-quit goodbye. iOS: PresenceWire typed models + WebSocket subscribe stub in CmuxMobileShell, the seam for the device tree (#5648). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 12, 2026
…nt stub Mac: PresenceHeartbeatClient follows the DeviceRegistryClient pattern (same device UUID and tag, best-effort, auth-gated), default OFF behind the presenceHeartbeatEnabled + presenceServiceURL defaults keys, with a server-owned cadence and a clean-quit goodbye. iOS: PresenceWire typed models + WebSocket subscribe stub in CmuxMobileShell, the seam for the device tree (#5648). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 13, 2026
…nt stub Mac: PresenceHeartbeatClient follows the DeviceRegistryClient pattern (same device UUID and tag, best-effort, auth-gated), default OFF behind the presenceHeartbeatEnabled + presenceServiceURL defaults keys, with a server-owned cadence and a clean-quit goodbye. iOS: PresenceWire typed models + WebSocket subscribe stub in CmuxMobileShell, the seam for the device tree (#5648). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 14, 2026
…nt stub Mac: PresenceHeartbeatClient follows the DeviceRegistryClient pattern (same device UUID and tag, best-effort, auth-gated), default OFF behind the presenceHeartbeatEnabled + presenceServiceURL defaults keys, with a server-owned cadence and a clean-quit goodbye. iOS: PresenceWire typed models + WebSocket subscribe stub in CmuxMobileShell, the seam for the device tree (#5648). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 14, 2026
…nt stub Mac: PresenceHeartbeatClient follows the DeviceRegistryClient pattern (same device UUID and tag, best-effort, auth-gated), default OFF behind the presenceHeartbeatEnabled + presenceServiceURL defaults keys, with a server-owned cadence and a clean-quit goodbye. iOS: PresenceWire typed models + WebSocket subscribe stub in CmuxMobileShell, the seam for the device tree (#5648). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 14, 2026
…er the device registry (#5792) * Add cmux-presence Cloudflare Worker: per-team Durable Object presence service Realtime device presence (online/offline) layered over the durable devices/device_app_instances registry. POST /v1/presence/heartbeat, GET /v1/presence/snapshot, GET /v1/presence/subscribe (WebSocket or SSE), Stack bearer auth mirroring web/services/vms/auth.ts, alarm-driven timeout-offline transitions (15s heartbeat / 45s timeout), 24h prune. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add presence worker local end-to-end proof script Drives wrangler dev with real dev-Stack credentials through the full lifecycle: 401 unauthenticated, heartbeat online, SSE + WebSocket subscribe, seen tick, goodbye offline, and alarm-driven timeout offline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add presence deploy-on-push workflow and service docs Path-filtered GitHub Actions: typecheck + unit tests + wrangler dry-run on PRs, wrangler deploy on push to main (DO migrations applied atomically with the deploy). docs/presence-service.md carries the DO-vs-RivetKit decision memo and the ephemeral-presence migration story. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add flagged Mac presence heartbeat sender and iOS typed presence client stub Mac: PresenceHeartbeatClient follows the DeviceRegistryClient pattern (same device UUID and tag, best-effort, auth-gated), default OFF behind the presenceHeartbeatEnabled + presenceServiceURL defaults keys, with a server-owned cadence and a clean-quit goodbye. iOS: PresenceWire typed models + WebSocket subscribe stub in CmuxMobileShell, the seam for the device tree (#5648). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bound subscribe streams to token expiry and harden request reading Subscribe streams now carry a worker-computed deadline (verified token expiry, capped at 15 minutes) enforced by the DO at delivery and via the alarm, so a revoked token or removed team member cannot keep an old stream alive; clients reconnect with a fresh token and get a fresh snapshot. Adds a per-team subscriber cap (64) and drops stalled SSE readers instead of buffering unboundedly. readBoundedJson now reads the body incrementally and aborts the moment it crosses the 16 KiB cap, so a chunked or lying-Content-Length body can never over-buffer; covered by new unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bind presence devices to their first authenticated owner Mirrors the device-registry ownership guard (a device row pins the registering userId and rejects other users' writes): the first authenticated team member to announce a deviceId owns it in DO storage, and a co-member's heartbeat for that device is rejected with 403 device_owner_mismatch, so presence cannot be forged online or force-cleared offline by another member who learned the device id from snapshots or the registry. Owner pins are pruned with the same 24h alarm pass that bounds the instance map. The local proof now exercises the guard with a real second Stack account in a shared team. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Make device-owner pins durable and harden the local proof script Owner pins are no longer pruned with the 24h presence tail (an idle device could be re-claimed by a co-member through the prune window), and new pins are bounded by MAX_OWNERS_PER_TEAM. The proof script keeps secrets off argv via curl config files and skips the owner-guard step with an explanation when both accounts resolve to the same Stack user instead of mistaking a legitimate same-owner 200 for a guard failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Split presence Swift types one-per-file with DocC coverage Flatten the PresenceWire namespace into top-level PresenceInstance / PresenceDevice / PresenceSnapshot / PresenceOfflineReason / PresenceUpdate / PresenceClientError / PresenceTokenSource files, each holding one documented major type, and decode the tagged wire frame via PresenceUpdate's custom Decodable (CodingKeys) instead of function-local payload structs. The Mac client moves PresenceSettings to its own pbxproj-wired file and reads the server interval with JSONSerialization to keep PresenceHeartbeatClient single-type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Republish attach routes on network path changes The mobile-host listener stays bound when the Mac moves networks or Tailscale flips, and .mobileHostStatusDidChange only fired on listener and connection transitions, so the advertised route set (and the team device registry DeviceRegistryClient mirrors from statusUpdates()) kept the old network's routes until the next listener restart. An NWPathMonitor now runs for the listener's lifetime: a changed path signature (status + interfaces + gateways, order-insensitive) invalidates the resolved-Tailscale-host cache and republishes routes through the same two-phase publish the listener-ready handler uses. A generation guard in MobileRouteResolver discards a resolution that raced the invalidation, so old-path hosts can never land late in the cache. Route-level dedup downstream means path flaps that do not change the route set produce no registry write. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Document the live cmux-presence-dev staging instance cmux-presence-dev is deployed on the team Cloudflare account with dev Stack Worker secrets provisioned; record its URL, the manual redeploy command, and how to point a dev Mac build at it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Mirror the registry's real per-team caps in the presence DO The DO capped instances and owner pins at a flat 5000 per team, so one authenticated member could mint thousands of fake deviceIds or tags, bloat every snapshot, and starve legitimate devices out of the budget. checkPresenceCaps (pure, unit-tested) now mirrors the registry route's actual limits: 200 devices per team (owner pins) and 25 instances per device, which structurally bounds the instance map at 5000 without an aggregate check since every stored instance's device holds a pin. Counts are fetched lazily with bounded list() calls only on new-device or new-instance heartbeats. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Reject expired subscribe deadlines and republish on first path observation Two review findings. The DO treated a forwarded x-presence-expires-at that was already past as missing and minted a fresh 15-minute window, so a token that expired between worker verification and DO handling could keep a stream open; resolveSubscribeDeadline (pure, unit-tested) now rejects missing/garbled/past deadlines with 401 and defensively re-caps the rest. The Mac path monitor treated its initial callback as a silent baseline, which swallowed a path change that landed between the listener-ready route publish and the monitor's first observation; the first observation now republishes too (deduped downstream), and only duplicate consecutive observations are skipped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bound the iOS presence stream buffer and scrub proof-script tokens The subscribe AsyncThrowingStream used the default unbounded buffering while the receive loop yields every frame (including the team's 15s seen ticks), so a stalled consumer would grow memory without limit; bufferingNewest(256) bounds it, and a dropped frame at worst leaves the map stale until the snapshot the deadline-bounded resubscribe protocol already guarantees. The local proof script kept $WORK for transcript logs but its curl configs carry live Stack bearer tokens; the cleanup trap now scrubs every token-bearing file on all exit paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * End the presence stream on buffer overflow instead of dropping silently Presence is a stateful snapshot+delta protocol, so a silently dropped transition frame could render wrong live state until the next reconnect (up to the 15-minute deadline). The receive loop now checks the yield result: a .dropped frame finishes the stream with the new PresenceClientError.updatesDropped, so the consumer's reconnect delivers a fresh snapshot first, and .terminated stops the loop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Deterministic handshake in the stale-resolution race test async let does not guarantee the child task entered the resolver and captured the old cache generation before the invalidation runs, so the test could nondeterministically exercise the wrong interleaving. A started semaphore now proves the resolution is in flight before the invalidation, and the gate holds it there until after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Extract network path observation into MobileHostNetworkPathMonitor MobileHostService owned both the republish action and the raw NWPathMonitor observation (signature computation, duplicate suppression, baseline state). The observation concerns now live in a small dedicated type with the same tested pure functions, so the service keeps a single responsibility: deciding what to do when the path changes. Behavior is unchanged; the existing path-refresh tests now target the monitor type directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Push route changes through presence: heartbeat routes + routes event Heartbeats now carry the instance's attach routes (tri-state: absent = unchanged, [] = no routes), the DO stores them on the presence record as a live cache of the registry row, and a changed set on an online instance broadcasts a 'routes' event so subscribed phones reconnect on the fresh port/IP without polling the registry. Entry filtering and the 16-route bound mirror the registry route; a non-array routes value is rejected rather than coerced so a client bug can never silently wipe pushed routes. * Mac presence heartbeats carry attach routes and beat immediately on change The heartbeat is the realtime twin of the registry write-through: every beat states the full current route set from MobileHostService (empty means pairing off), and a route-set change observed via statusUpdates() fires one immediate out-of-cadence beat so the presence DO can push the fresh port/IP to subscribed phones within a round trip. Debug builds now default the gate on against the dev/staging worker (dev Stack identity matches what cmux-presence-dev verifies), keeping Release default off; both stay explicitly overridable via defaults/env. * Phone subscribes to live presence: device tree online/offline + pushed-route reconnect The phone-side half of the presence service. MobileShellComposite owns one presence subscription (PresenceSubscribing seam, PresenceClient transport) that follows the session: starts on sign-in, tears down with a blanked map on sign-out, restarts from foreground refresh. Stream frames reduce into a pure PresenceMap (snapshot replaces, events upsert) that the device tree overlays on registry rows as live Online/Offline instead of last-seen guesses (en+ja). Route pushes (routes/online events and reconcile snapshots) write through to the local paired-Mac store via the same selectReconnectRoutes merge the registry refresh uses, and kick a reconnect when the active Mac is online but the phone sits disconnected, so a port change reattaches without re-pairing. PresenceInstance decodes routes with per-entry leniency (unknown kinds drop, frames never fail), matching the registry contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Presence doc reflects shipped clients; deploy job names missing CF secrets explicitly Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Review fixes: offline alarm defers to prune deadline; snapshot route sync is one batch Greptile P1: ensureAlarmFor scheduled offline instances at the 45s offline timeout, so every goodbye burned one no-op DO alarm before the real 24h prune alarm. Delegate to core's nextAlarmTime so the deadline rule lives in one place. Greptile P2: the presence snapshot fanned out one Task per online instance, so a multi-tag Mac could queue duplicate recoverMobileConnection kicks (a late one lands as a spurious resync after reconnect succeeds) with nondeterministic route-upsert order. Process the snapshot's instances sequentially in one task and kick at most one reconnect per delivery. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Refresh swift file length budget for presence client growth MobileShellComposite +176 (presence subscription lifecycle), MobileHostService +49 (network path monitor wiring), AppDelegate +4 (heartbeat client). Known debt accepted; MobileHostNetworkPathMonitor was already extracted to its own file to bound the growth. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Autoreview fixes: heartbeat test asserts real wire shape; explicit empty route push clears the tree The heartbeat body test read host/port at the route's top level, but mobileHostJSONObject nests them under endpoint, so the assertions could never pass once the suite ran. Assert the nested shape (the same wire contract the registry POST and iOS parser use). applyPushedRoutes treated routes nil and [] identically and returned before touching registryDevices, so an explicit empty push (host advertises no routes) left stale Connect affordances in the device tree. nil now means "not announced" (no-op); an announced set, including [], mirrors to the tree, while the paired-Mac store still keeps last-known-good reconnect routes and only updates on non-empty pushes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Presence route sync discards stale frames after sign-out or account switch The unstructured sync task can suspend in loadPairedMacs/upsert and resume after a different user signed in. Re-check isSignedIn plus the captured requesting user after every suspension, mirroring refreshRegistryDevices' account-switch guard, so a stale frame can never write routes into or kick reconnects for the next session. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Path observation always invalidates the Tailscale host cache; PresenceMap rollups are per-device A pre-ready initial path observation advanced the monitor's dedup baseline but returned before invalidating the resolver cache, so the .ready publish could reuse TTL-fresh hosts from the previous network with no further path callback coming (toggle pairing off, move networks, toggle on). Invalidate on every observation, before the no-port early return. PresenceMap stored instances flat by deviceId:tag, so deviceSummary scanned the whole team map; the device tree recomputes every visible row's summary per heartbeat mutation, making row projection O(devices x all instances). Group storage by device so a rollup only touches that device's instances (25 max). Adds direct PresenceMap reduction tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Presence route pushes respect the registry's multi-instance ambiguity guard The paired-Mac store is device-level (no tag); the registry refresh only substitutes reconnect routes when exactly one instance advertises any, but the presence push path wrote every instance's routes through, so a tagged debug build's push could repoint the phone's persisted reconnect routes at the wrong build. Gate the store write on PresenceMap's new soleRouteAdvertisingInstance(deviceId:) (exactly one online route-bearing instance, and it is the pusher). The per-tag device-tree mirror stays unconditional. Covered in PresenceMapTests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bound cumulative serialized route bytes per heartbeat Route entries were individually unbounded (only the 16KiB request cap applied), so one authenticated member could fill the admitted 200x25 instance caps with near-16KiB route payloads (~78MiB) and blow the Workers isolate memory budget whenever snapshot/alarm materialize the team map, DoSing presence for the team. Cap cumulative serialized routes at 2KiB per instance (worst-case team state ~10MiB), dropping entries past the budget so the host's preferred-first prefix survives. Real route sets are ~100-200 bytes per entry and fit untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Scope presence service-resolution statics onto PresenceClient (conventions lint) The caseless namespace enum tripped the package-conventions namespace-enum rule; the members now live directly on the owning type. Covariant Self in the default argument replaced with the concrete type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Serve production presence at presence.cmux.dev custom_domain route in wrangler.toml (cmux.dev zone is on the same Cloudflare account, so the deploy provisions DNS + TLS). Release clients keep a nil default service URL; flipping them to this domain is a follow-up gated on the first production deploy and dogfood. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Serialize presence route deliveries on the paired-Mac write chain Greptile P1: the per-delivery fire-and-forget task raced on reconnect (snapshot immediately followed by online/routes for the same device), producing concurrent pairedMacStore upserts for one Mac and a possible double reconnect kick. Deliveries now run through performSerializedPairedMacWrite, which appends synchronously on the main actor, so they execute strictly in arrival order; userIsCurrent doubles as the chain's ifStillCurrent entry check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Negative-cache rejected presence auth tokens (security audit MED) An opaque (non-JWT) bearer token skips the client-side expiry short-circuit, so every request carrying a bad token forced an outbound Stack /users/me subrequest — an unauthenticated amplification vector against Stack's rate limits and CF subrequest budget. Rejected tokens are now cached for 10s (bounded by the token's own exp), keyed by token hash like the positive cache. Test asserts 3 rejected requests cost 1 Stack call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix test fetch cast for typecheck * Refresh swift file length budget after rebase onto main Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Path signature includes local IPv4 addresses so same-gateway network moves republish routes Codex review (P2) on the presence PR: two networks can present the same interface name and gateway (two LANs both en0 + 192.168.1.1) while assigning a different local address; the old signature deduped that move and never invalidated/republished routes. The signature now includes the machine's local IPv4 addresses (getifaddrs, up non-loopback interfaces), injectable for tests. IPv6 is excluded deliberately: temporary-address rotation would cause spurious republish churn. Also corrects the reconnect-kick comment in MobileShellComposite: under the multi-instance ambiguity guard, pushed routes are deliberately not persisted and the reconnect uses stored last-known-good routes (cursor bot flagged the old comment's claim that routes were always persisted). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(presence): how to upgrade running Durable Objects safely Class migrations vs data-schema: class migrations manage the DO class registry (append-only, atomic with deploy); they do not migrate the shape of stored data. Running objects keep old code until evicted, then hydrate new code against persisted storage, so upgrades = make new code read old data (additive fields, schemaVersion + lazy upgrade, rollout-window tolerance). For presence only the never-pruned owner pins need that care; the live map self-heals via 15s re-announce. * Refresh swift file length budget for post-rebase file sizes --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jun 16, 2026
This was referenced Jun 26, 2026
ShubhamPatilsd
pushed a commit
to emergent-inc/mosaic
that referenced
this pull request
Jul 9, 2026
…er the device registry (#5792) * Add cmux-presence Cloudflare Worker: per-team Durable Object presence service Realtime device presence (online/offline) layered over the durable devices/device_app_instances registry. POST /v1/presence/heartbeat, GET /v1/presence/snapshot, GET /v1/presence/subscribe (WebSocket or SSE), Stack bearer auth mirroring web/services/vms/auth.ts, alarm-driven timeout-offline transitions (15s heartbeat / 45s timeout), 24h prune. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add presence worker local end-to-end proof script Drives wrangler dev with real dev-Stack credentials through the full lifecycle: 401 unauthenticated, heartbeat online, SSE + WebSocket subscribe, seen tick, goodbye offline, and alarm-driven timeout offline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add presence deploy-on-push workflow and service docs Path-filtered GitHub Actions: typecheck + unit tests + wrangler dry-run on PRs, wrangler deploy on push to main (DO migrations applied atomically with the deploy). docs/presence-service.md carries the DO-vs-RivetKit decision memo and the ephemeral-presence migration story. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add flagged Mac presence heartbeat sender and iOS typed presence client stub Mac: PresenceHeartbeatClient follows the DeviceRegistryClient pattern (same device UUID and tag, best-effort, auth-gated), default OFF behind the presenceHeartbeatEnabled + presenceServiceURL defaults keys, with a server-owned cadence and a clean-quit goodbye. iOS: PresenceWire typed models + WebSocket subscribe stub in CmuxMobileShell, the seam for the device tree (manaflow-ai/cmux#5648). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bound subscribe streams to token expiry and harden request reading Subscribe streams now carry a worker-computed deadline (verified token expiry, capped at 15 minutes) enforced by the DO at delivery and via the alarm, so a revoked token or removed team member cannot keep an old stream alive; clients reconnect with a fresh token and get a fresh snapshot. Adds a per-team subscriber cap (64) and drops stalled SSE readers instead of buffering unboundedly. readBoundedJson now reads the body incrementally and aborts the moment it crosses the 16 KiB cap, so a chunked or lying-Content-Length body can never over-buffer; covered by new unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bind presence devices to their first authenticated owner Mirrors the device-registry ownership guard (a device row pins the registering userId and rejects other users' writes): the first authenticated team member to announce a deviceId owns it in DO storage, and a co-member's heartbeat for that device is rejected with 403 device_owner_mismatch, so presence cannot be forged online or force-cleared offline by another member who learned the device id from snapshots or the registry. Owner pins are pruned with the same 24h alarm pass that bounds the instance map. The local proof now exercises the guard with a real second Stack account in a shared team. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Make device-owner pins durable and harden the local proof script Owner pins are no longer pruned with the 24h presence tail (an idle device could be re-claimed by a co-member through the prune window), and new pins are bounded by MAX_OWNERS_PER_TEAM. The proof script keeps secrets off argv via curl config files and skips the owner-guard step with an explanation when both accounts resolve to the same Stack user instead of mistaking a legitimate same-owner 200 for a guard failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Split presence Swift types one-per-file with DocC coverage Flatten the PresenceWire namespace into top-level PresenceInstance / PresenceDevice / PresenceSnapshot / PresenceOfflineReason / PresenceUpdate / PresenceClientError / PresenceTokenSource files, each holding one documented major type, and decode the tagged wire frame via PresenceUpdate's custom Decodable (CodingKeys) instead of function-local payload structs. The Mac client moves PresenceSettings to its own pbxproj-wired file and reads the server interval with JSONSerialization to keep PresenceHeartbeatClient single-type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Republish attach routes on network path changes The mobile-host listener stays bound when the Mac moves networks or Tailscale flips, and .mobileHostStatusDidChange only fired on listener and connection transitions, so the advertised route set (and the team device registry DeviceRegistryClient mirrors from statusUpdates()) kept the old network's routes until the next listener restart. An NWPathMonitor now runs for the listener's lifetime: a changed path signature (status + interfaces + gateways, order-insensitive) invalidates the resolved-Tailscale-host cache and republishes routes through the same two-phase publish the listener-ready handler uses. A generation guard in MobileRouteResolver discards a resolution that raced the invalidation, so old-path hosts can never land late in the cache. Route-level dedup downstream means path flaps that do not change the route set produce no registry write. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Document the live cmux-presence-dev staging instance cmux-presence-dev is deployed on the team Cloudflare account with dev Stack Worker secrets provisioned; record its URL, the manual redeploy command, and how to point a dev Mac build at it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Mirror the registry's real per-team caps in the presence DO The DO capped instances and owner pins at a flat 5000 per team, so one authenticated member could mint thousands of fake deviceIds or tags, bloat every snapshot, and starve legitimate devices out of the budget. checkPresenceCaps (pure, unit-tested) now mirrors the registry route's actual limits: 200 devices per team (owner pins) and 25 instances per device, which structurally bounds the instance map at 5000 without an aggregate check since every stored instance's device holds a pin. Counts are fetched lazily with bounded list() calls only on new-device or new-instance heartbeats. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Reject expired subscribe deadlines and republish on first path observation Two review findings. The DO treated a forwarded x-presence-expires-at that was already past as missing and minted a fresh 15-minute window, so a token that expired between worker verification and DO handling could keep a stream open; resolveSubscribeDeadline (pure, unit-tested) now rejects missing/garbled/past deadlines with 401 and defensively re-caps the rest. The Mac path monitor treated its initial callback as a silent baseline, which swallowed a path change that landed between the listener-ready route publish and the monitor's first observation; the first observation now republishes too (deduped downstream), and only duplicate consecutive observations are skipped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bound the iOS presence stream buffer and scrub proof-script tokens The subscribe AsyncThrowingStream used the default unbounded buffering while the receive loop yields every frame (including the team's 15s seen ticks), so a stalled consumer would grow memory without limit; bufferingNewest(256) bounds it, and a dropped frame at worst leaves the map stale until the snapshot the deadline-bounded resubscribe protocol already guarantees. The local proof script kept $WORK for transcript logs but its curl configs carry live Stack bearer tokens; the cleanup trap now scrubs every token-bearing file on all exit paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * End the presence stream on buffer overflow instead of dropping silently Presence is a stateful snapshot+delta protocol, so a silently dropped transition frame could render wrong live state until the next reconnect (up to the 15-minute deadline). The receive loop now checks the yield result: a .dropped frame finishes the stream with the new PresenceClientError.updatesDropped, so the consumer's reconnect delivers a fresh snapshot first, and .terminated stops the loop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Deterministic handshake in the stale-resolution race test async let does not guarantee the child task entered the resolver and captured the old cache generation before the invalidation runs, so the test could nondeterministically exercise the wrong interleaving. A started semaphore now proves the resolution is in flight before the invalidation, and the gate holds it there until after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Extract network path observation into MobileHostNetworkPathMonitor MobileHostService owned both the republish action and the raw NWPathMonitor observation (signature computation, duplicate suppression, baseline state). The observation concerns now live in a small dedicated type with the same tested pure functions, so the service keeps a single responsibility: deciding what to do when the path changes. Behavior is unchanged; the existing path-refresh tests now target the monitor type directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Push route changes through presence: heartbeat routes + routes event Heartbeats now carry the instance's attach routes (tri-state: absent = unchanged, [] = no routes), the DO stores them on the presence record as a live cache of the registry row, and a changed set on an online instance broadcasts a 'routes' event so subscribed phones reconnect on the fresh port/IP without polling the registry. Entry filtering and the 16-route bound mirror the registry route; a non-array routes value is rejected rather than coerced so a client bug can never silently wipe pushed routes. * Mac presence heartbeats carry attach routes and beat immediately on change The heartbeat is the realtime twin of the registry write-through: every beat states the full current route set from MobileHostService (empty means pairing off), and a route-set change observed via statusUpdates() fires one immediate out-of-cadence beat so the presence DO can push the fresh port/IP to subscribed phones within a round trip. Debug builds now default the gate on against the dev/staging worker (dev Stack identity matches what cmux-presence-dev verifies), keeping Release default off; both stay explicitly overridable via defaults/env. * Phone subscribes to live presence: device tree online/offline + pushed-route reconnect The phone-side half of the presence service. MobileShellComposite owns one presence subscription (PresenceSubscribing seam, PresenceClient transport) that follows the session: starts on sign-in, tears down with a blanked map on sign-out, restarts from foreground refresh. Stream frames reduce into a pure PresenceMap (snapshot replaces, events upsert) that the device tree overlays on registry rows as live Online/Offline instead of last-seen guesses (en+ja). Route pushes (routes/online events and reconcile snapshots) write through to the local paired-Mac store via the same selectReconnectRoutes merge the registry refresh uses, and kick a reconnect when the active Mac is online but the phone sits disconnected, so a port change reattaches without re-pairing. PresenceInstance decodes routes with per-entry leniency (unknown kinds drop, frames never fail), matching the registry contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Presence doc reflects shipped clients; deploy job names missing CF secrets explicitly Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Review fixes: offline alarm defers to prune deadline; snapshot route sync is one batch Greptile P1: ensureAlarmFor scheduled offline instances at the 45s offline timeout, so every goodbye burned one no-op DO alarm before the real 24h prune alarm. Delegate to core's nextAlarmTime so the deadline rule lives in one place. Greptile P2: the presence snapshot fanned out one Task per online instance, so a multi-tag Mac could queue duplicate recoverMobileConnection kicks (a late one lands as a spurious resync after reconnect succeeds) with nondeterministic route-upsert order. Process the snapshot's instances sequentially in one task and kick at most one reconnect per delivery. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Refresh swift file length budget for presence client growth MobileShellComposite +176 (presence subscription lifecycle), MobileHostService +49 (network path monitor wiring), AppDelegate +4 (heartbeat client). Known debt accepted; MobileHostNetworkPathMonitor was already extracted to its own file to bound the growth. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Autoreview fixes: heartbeat test asserts real wire shape; explicit empty route push clears the tree The heartbeat body test read host/port at the route's top level, but mobileHostJSONObject nests them under endpoint, so the assertions could never pass once the suite ran. Assert the nested shape (the same wire contract the registry POST and iOS parser use). applyPushedRoutes treated routes nil and [] identically and returned before touching registryDevices, so an explicit empty push (host advertises no routes) left stale Connect affordances in the device tree. nil now means "not announced" (no-op); an announced set, including [], mirrors to the tree, while the paired-Mac store still keeps last-known-good reconnect routes and only updates on non-empty pushes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Presence route sync discards stale frames after sign-out or account switch The unstructured sync task can suspend in loadPairedMacs/upsert and resume after a different user signed in. Re-check isSignedIn plus the captured requesting user after every suspension, mirroring refreshRegistryDevices' account-switch guard, so a stale frame can never write routes into or kick reconnects for the next session. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Path observation always invalidates the Tailscale host cache; PresenceMap rollups are per-device A pre-ready initial path observation advanced the monitor's dedup baseline but returned before invalidating the resolver cache, so the .ready publish could reuse TTL-fresh hosts from the previous network with no further path callback coming (toggle pairing off, move networks, toggle on). Invalidate on every observation, before the no-port early return. PresenceMap stored instances flat by deviceId:tag, so deviceSummary scanned the whole team map; the device tree recomputes every visible row's summary per heartbeat mutation, making row projection O(devices x all instances). Group storage by device so a rollup only touches that device's instances (25 max). Adds direct PresenceMap reduction tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Presence route pushes respect the registry's multi-instance ambiguity guard The paired-Mac store is device-level (no tag); the registry refresh only substitutes reconnect routes when exactly one instance advertises any, but the presence push path wrote every instance's routes through, so a tagged debug build's push could repoint the phone's persisted reconnect routes at the wrong build. Gate the store write on PresenceMap's new soleRouteAdvertisingInstance(deviceId:) (exactly one online route-bearing instance, and it is the pusher). The per-tag device-tree mirror stays unconditional. Covered in PresenceMapTests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bound cumulative serialized route bytes per heartbeat Route entries were individually unbounded (only the 16KiB request cap applied), so one authenticated member could fill the admitted 200x25 instance caps with near-16KiB route payloads (~78MiB) and blow the Workers isolate memory budget whenever snapshot/alarm materialize the team map, DoSing presence for the team. Cap cumulative serialized routes at 2KiB per instance (worst-case team state ~10MiB), dropping entries past the budget so the host's preferred-first prefix survives. Real route sets are ~100-200 bytes per entry and fit untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Scope presence service-resolution statics onto PresenceClient (conventions lint) The caseless namespace enum tripped the package-conventions namespace-enum rule; the members now live directly on the owning type. Covariant Self in the default argument replaced with the concrete type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Serve production presence at presence.cmux.dev custom_domain route in wrangler.toml (cmux.dev zone is on the same Cloudflare account, so the deploy provisions DNS + TLS). Release clients keep a nil default service URL; flipping them to this domain is a follow-up gated on the first production deploy and dogfood. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Serialize presence route deliveries on the paired-Mac write chain Greptile P1: the per-delivery fire-and-forget task raced on reconnect (snapshot immediately followed by online/routes for the same device), producing concurrent pairedMacStore upserts for one Mac and a possible double reconnect kick. Deliveries now run through performSerializedPairedMacWrite, which appends synchronously on the main actor, so they execute strictly in arrival order; userIsCurrent doubles as the chain's ifStillCurrent entry check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Negative-cache rejected presence auth tokens (security audit MED) An opaque (non-JWT) bearer token skips the client-side expiry short-circuit, so every request carrying a bad token forced an outbound Stack /users/me subrequest — an unauthenticated amplification vector against Stack's rate limits and CF subrequest budget. Rejected tokens are now cached for 10s (bounded by the token's own exp), keyed by token hash like the positive cache. Test asserts 3 rejected requests cost 1 Stack call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix test fetch cast for typecheck * Refresh swift file length budget after rebase onto main Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Path signature includes local IPv4 addresses so same-gateway network moves republish routes Codex review (P2) on the presence PR: two networks can present the same interface name and gateway (two LANs both en0 + 192.168.1.1) while assigning a different local address; the old signature deduped that move and never invalidated/republished routes. The signature now includes the machine's local IPv4 addresses (getifaddrs, up non-loopback interfaces), injectable for tests. IPv6 is excluded deliberately: temporary-address rotation would cause spurious republish churn. Also corrects the reconnect-kick comment in MobileShellComposite: under the multi-instance ambiguity guard, pushed routes are deliberately not persisted and the reconnect uses stored last-known-good routes (cursor bot flagged the old comment's claim that routes were always persisted). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(presence): how to upgrade running Durable Objects safely Class migrations vs data-schema: class migrations manage the DO class registry (append-only, atomic with deploy); they do not migrate the shape of stored data. Running objects keep old code until evicted, then hydrate new code against persisted storage, so upgrades = make new code read old data (additive fields, schemaVersion + lazy upgrade, rollout-window tolerance). For presence only the never-pruned owner pins need that care; the live map self-heals via 15s re-announce. * Refresh swift file length budget for post-rebase file sizes --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A hierarchical device tree for iOS, the primary multi-device nav built on the device registry that just merged (#5626). Top level is each registered device (Mac/host); expand a device to its cmux app instances (tags: stable / dev builds); expand a tag to that build's workspaces; tap a workspace to open it via the existing path.
Reached from a Devices button in the workspace-list toolbar (a single top-level sheet, so selecting a workspace dismisses straight back and opens it). The old flat workspace list and the multi-Mac switcher (Settings → Switch Mac) stay as fallback paths, nothing is removed.
How it reads #5626
The merged registry exposes the two-level model the tree renders:
devices+device_app_instances(tag, routes)tables —web/db/schema.ts:190(devices),web/db/schema.ts:234(instances).GET /api/devicesreturns{ devices: [{ deviceId, platform, displayName, lastSeenAt, instances: [{ tag, routes, lastSeenAt }] }] }—web/app/api/devices/route.ts:289.DeviceRegistryServicealready fetched this for reconnect routes (freshRoutes); this PR addslistDevices()to surface the whole list to the UI, generalizing the existing failable-per-route decode intoparseDeviceList—Packages/CmuxMobileShell/Sources/CmuxMobileShell/DeviceRegistryService.swift.Tree UI + tap-to-open
RegistryDevice/RegistryAppInstance—Packages/CmuxMobileShellModel/Sources/CmuxMobileShellModel/RegistryDevice.swift.DeviceTreeView/DeviceTreeRows— device → tag → workspace, rendered with a strict snapshot boundary (rows take immutable value snapshots + closure action bundles, no@Observablestore crosses theListboundary, per AGENTS.md).store.connectToRegistryInstance), then its workspaces appear. Tapping a workspace uses the existing open path.@AppStorage(DeviceTreeExpansionStore).Online / ping derivation
macConnectionStatus. The live tag on a multi-tag device is identified by route match (instanceMatchesActiveRoute), so only the actually-connected build shows workspaces.lastSeenAt("last seen N ago"), best-effort. TODO: there is no active per-host reachability ping yet, so a non-connected device's dot is last-seen-only.Fallback / safety
deviceTreeDevices), so the sheet stays usable with the cloud down.What it supersedes
The flat workspace list (single connected Mac) and the Switch Mac picker, as the new primary nav for multi-Mac + multi-tag. Both are kept behind it for safety. Note: this renders the registry list directly; it does not depend on the still-open cross-Mac workspace aggregate (#5566), which is not on main.
Tests
Pure decode tests (
GET /api/devicesbody → tree model, forward-compat route drop, ios-not-controllable, ISO8601 last-seen) and the expansion-codec round-trip.Known limitation
registryDevicesis cached in-memory, guarded against account switches (capturedcurrentUserIDrecheck) and auth/scope rejection (a 401/403 clears it). It is not yet keyed by team. The mobile app has no in-app team switcher, so there is no reachable trigger to change the active team mid-session; if the selected team changes via another surface, the next successful/api/devicesload replaces the list. Keying the cache by(userID, teamID)— the response envelope already returnsteamId(web/app/api/devices/route.ts:349) — is the follow-up for when a mobile team switcher lands. Worst case today is a user briefly seeing their own other team's device names, never another account's.Verification
iOS simulator build (arm64, build-only) green. Autoreview run iteratively and fixed (account-switch race, multi-tag wrong-tag workspaces, failed-connect rollback incl. same-device tag switch, paired-Mac fallback, team-scope auth-rejection clear).
Needs device dogfood (folds into a dog round + beta).
🤖 Generated with Claude Code
Note
Medium Risk
Touches live mobile connection switching, team-scoped registry caching, and auth-rejection handling; mistakes could strand sessions or briefly show wrong-scope device names, though rollback and account-switch guards mitigate the worst cases.
Overview
Adds a hierarchical Devices sheet on iOS (device → tagged cmux builds → workspaces) as the primary multi-host navigation, opened from a new toolbar button on the workspace list.
Registry & shell:
DeviceRegistryRefreshinggainslistDevices()with a three-way outcome (ok/authRejected/transientFailure).DeviceRegistryServicedecodes the fullGET /api/devicespayload viaparseDeviceList(failable per-route, lenientlastSeenAt).MobileShellCompositecachesregistryDevices, exposesdeviceTreeDevices(registry or synthesized paired Macs),connectedMacDeviceID,loadRegistryDevices(), andconnectToRegistryInstance()with failed-connect rollback to the previous active Mac. Registry data clears on sign-out; 401/403 clears cache (team-scope leak guard); transient errors keep the current tree.UI: New
RegistryDevice/RegistryAppInstancemodels,DeviceTreeViewwith snapshot-bound rows, persisted expand/collapse (DeviceTreeExpansionStore), route-matched active tag for workspaces, and Connect for non-live tags. Workspaces only list under the live route-matched instance.Tests & strings: Parsing and expansion-store tests; device-tree localization keys.
Reviewed by Cursor Bugbot for commit a0d6660. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit