feat(mobile): publish richer remote workspace state - #8112
austinywang wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe mobile RPC layer now supports versioned workspace remote state and attached-view presence. The host derives agent, Git, pull-request, notification, and connection data, emits related workspace updates, includes the data in workspace-list payloads, and adds decoding and integration coverage. ChangesRicher mobile workspace state
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Workspace
participant MobileWorkspaceRemoteStateSnapshot
participant MobileHostService
participant TerminalController
participant MobileClient
Workspace->>MobileWorkspaceRemoteStateSnapshot: capture semantic workspace state
MobileWorkspaceRemoteStateSnapshot->>TerminalController: provide remote_state payload
MobileHostService->>TerminalController: provide view_presence payload
TerminalController->>MobileClient: return workspace list response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 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 |
Greptile SummaryThis PR adds richer mobile workspace state and attached-view presence. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "fix(mobile): harden attached view presen..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // plumbing. Newer clients identify themselves on their first | ||
| // `mobile.events.subscribe`, before terminal input/viewport traffic, | ||
| // so presence becomes visible as soon as the view attaches. | ||
| await onAuthorizedRequest(request) |
There was a problem hiding this comment.
When an unauthenticated mobile.host.status request includes client_id, this new early onAuthorizedRequest call can record that arbitrary ID and recordClientID now emits workspace.updated. Authenticated subscribers can then receive a presence update for a forged attached view until that connection closes, so presence recording should be limited to requests that passed the authenticated path or to the authorized subscribe flow.
There was a problem hiding this comment.
Fixed in 80ceab4: Stack-bearer mobile.host.status is explicitly excluded from presence registration, client IDs are bounded to one per transport and 128 UTF-8 bytes, and focused coverage verifies public status cannot add a view.
— Claude Code
| // plumbing. Newer clients identify themselves on their first | ||
| // `mobile.events.subscribe`, before terminal input/viewport traffic, | ||
| // so presence becomes visible as soon as the view attaches. | ||
| await onAuthorizedRequest(request) |
There was a problem hiding this comment.
This call still runs for mobile.host.status, which is the one RPC method that skips authorization. When an unauthenticated status request includes client_id, this path records that ID, emits workspace.updated, and lets authenticated subscribers see a forged attached-view row until the connection closes. Please keep presence registration on the authenticated request path only, or explicitly skip the public status method here.
There was a problem hiding this comment.
Fixed in 80ceab4: the shared presence owner rejects Stack-bearer public-status registration before any client ID is stored or event emitted. The current diff also bounds identity storage per transport.
— Claude Code
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileAttachedView.swift`:
- Line 2: Mark the pure value-model structs MobileAttachedView in
Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileAttachedView.swift:2-2
and MobileWorkspaceGitState in
Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspaceGitState.swift:2-2
as nonisolated, preserving their existing Codable, Sendable, and Equatable
declarations.
🪄 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: 807333de-1636-490e-8b01-eb614e447da8
📒 Files selected for processing (21)
Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileAttachedView.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileSyncWorkspaceListResponse.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileViewPresence.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspaceAgentLifecycle.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspaceAgentStatus.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspaceCIStatus.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspaceGitState.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspaceNotificationState.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspacePullRequestLifecycle.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspacePullRequestState.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspaceRemoteState.swiftPackages/iOS/CmuxMobileRPC/Tests/CmuxMobileRPCTests/MobileWorkspaceRemoteStateDecodeTests.swiftSources/Mobile/MobileHostService+Capabilities.swiftSources/Mobile/MobileHostService.swiftSources/Mobile/MobileWorkspaceAgentStatusSnapshot.swiftSources/Mobile/MobileWorkspaceListObserver.swiftSources/Mobile/MobileWorkspaceRemoteStateSnapshot.swiftSources/TerminalController+MobileWorkspaceList.swiftcmux.xcodeproj/project.pbxprojcmuxTests/MobileHostAuthorizationTests.swiftcmuxTests/MobileRicherRemoteStateTests.swift
| @@ -0,0 +1,18 @@ | |||
| /// One attached view represented in a host's runtime-presence snapshot. | |||
| public struct MobileAttachedView: Decodable, Sendable, Equatable { | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Mark pure value-model structs as nonisolated.
As per coding guidelines, please mark Codable, Identifiable, Sendable, and pure value-model structs as nonisolated to prevent them from implicitly inheriting @MainActor isolation under Swift 6 strict concurrency defaults.
Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileAttachedView.swift#L2-L2: Add thenonisolatedmodifier toMobileAttachedView.Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspaceGitState.swift#L2-L2: Add thenonisolatedmodifier toMobileWorkspaceGitState.
📍 Affects 2 files
Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileAttachedView.swift#L2-L2(this comment)Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspaceGitState.swift#L2-L2
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileAttachedView.swift` at
line 2, Mark the pure value-model structs MobileAttachedView in
Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileAttachedView.swift:2-2
and MobileWorkspaceGitState in
Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileWorkspaceGitState.swift:2-2
as nonisolated, preserving their existing Codable, Sendable, and Equatable
declarations.
Source: Coding guidelines
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 (1)
cmuxTests/MobileRicherRemoteStateTests.swift (1)
112-121: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winResolve conflicting test logic for
spoofed-client.The test calls
service.recordClientID("spoofed-client", for: secondConnectionID)immediately after recording"phone-1"for the same connection ID.If the presence state maps each
connectionIDto a singleclientID(replacing the previous entry),"phone-1"will have a connection count of 1, causing the assertion on line 120 (connection_count == 2) to fail. If the presence state supports multiple clients per connection, theviewsarray will contain"spoofed-client", causing the exact array assertion on line 119 to fail.Consider removing the
"spoofed-client"line if it was accidentally copied from the invalid ID test below, or adjust the assertions to match the intended state.🐛 Proposed fix (assuming `spoofed-client` is unintentional here)
service.recordClientID("phone-1", for: firstConnectionID) service.recordClientID("phone-1", for: firstConnectionID) service.recordClientID("phone-1", for: secondConnectionID) - service.recordClientID("spoofed-client", for: secondConnectionID) service.recordClientID("mac-2", for: thirdConnectionID) let payload = service.viewPresencePayload() `#expect`(payload["version"] as? Int == 1) let views = try `#require`(payload["views"] as? [[String: Any]]) - `#expect`(views.map { $0["client_id"] as? String } == ["mac-2", "phone-1"]) + `#expect`(Set(views.compactMap { $0["client_id"] as? String }) == ["mac-2", "phone-1"]) `#expect`(views.first { $0["client_id"] as? String == "phone-1" }?["connection_count"] as? Int == 2)🤖 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 `@cmuxTests/MobileRicherRemoteStateTests.swift` around lines 112 - 121, Resolve the conflicting test setup in the presence payload test by removing the unintended service.recordClientID call for "spoofed-client" on secondConnectionID. Keep the existing views ordering and connection-count assertions for "phone-1" and "mac-2" unchanged, leaving spoofed-client coverage to the appropriate invalid-ID test.
🤖 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 `@cmuxTests/MobileRicherRemoteStateTests.swift`:
- Around line 112-121: Resolve the conflicting test setup in the presence
payload test by removing the unintended service.recordClientID call for
"spoofed-client" on secondConnectionID. Keep the existing views ordering and
connection-count assertions for "phone-1" and "mac-2" unchanged, leaving
spoofed-client coverage to the appropriate invalid-ID test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b51707d1-62ac-47a7-a659-590812810f19
📒 Files selected for processing (4)
Sources/Mobile/MobileHostService.swiftSources/Mobile/MobileViewPresenceState.swiftcmux.xcodeproj/project.pbxprojcmuxTests/MobileRicherRemoteStateTests.swift
lawrencecchen
added
the
stale-revisit
This branch had an error being deployed
3 participants
Summary
remote_statev1 data to each mobile workspace row, covering aggregated agent lifecycle, git branch/dirty state, pull-request lifecycle/URL/branch/staleness, CI status, and notification unread state.view_presencev1 data, grouping active transports by stable client ID and publishing presence changes through the existing workspace event path.workspace.remote_state.v1andview.presence.v1; keep older payloads decodable and map unknown future lifecycle/CI values to.unknown.LatestWinsBatcher; this adds no per-keystroke/per-frame work and no new polling.unknown: the sidebar intentionally removed GitHub status polling in Coalesce sidebar PR polling per-repo, drop checks fetch, state-machine the probe queue #2662, and there is no cheap authoritative host-side CI source. This PR does not restore that polling.Packages/macOS/CmuxHiveuntouched.Fixes #8082
Testing
arch -arm64 swift testinPackages/iOS/CmuxMobileRPC: 81 passed.cmux-unitrun forMobileRicherRemoteStateTests,MobileWorkspaceObserverMetricsTests, andInvalidationBatchingBehaviorTests: 11 passed, 0 failed/skipped../scripts/reload.sh --tag issue-8082-richer-state: succeeded on pushed HEAD60196b84ca.scripts/check-pbxproj.sh,scripts/lint-pbxproj-test-wiring.sh(494 files),scripts/check-package-resolved-policy.py, andgit diff --check.python3 scripts/swift_file_length_budget.pystill reports pre-existingorigin/mainbudget debt; the branch-specific observer crossing was removed (MobileWorkspaceListObserver.swiftis 491 lines).Manual verification is protocol-only in this PR; the tagged app is available for attached-view payload/presence dogfood.
Demo Video
Not applicable: no UI surface changed.
Checklist
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Publish richer, versioned remote workspace state and view presence to mobile so the app can show agent, git, PR, CI, and notification details without extra polling. Adds
workspace.remote_state.v1andview.presence.v1with forward-compatible decoding, capability flags, and hardened presence tracking; addresses #8082.New Features
workspace.remote_state.v1per workspace: agents (lifecycle + panels), git (branch/dirty), pull request (lifecycle/URL/branch/staleness), CI (unknownif not authoritative), and notifications (unread count/dot, latest id).view.presence.v1: groups transports by stableclient_idwithconnection_count; presence changes emit viaworkspace.updated. iOS decoders accept optionaldisplay_nameandkindwhen supplied.workspace.remote_state.v1andview.presence.v1; older clients keep decoding legacy fields; unknown lifecycle/CI values map tounknown.Packages/iOS/CmuxMobileRPC; no new polling or per-keystroke work (reuses existing stores and batching).Bug Fixes
client_id(max 128 bytes), de-dupe per connection, and emit updates on subscribe and disconnect for catch-up without polling.Written for commit 80ceab4. Summary will update on new commits.
Summary by CodeRabbit