Fix grouped workspaces in All Computers - #9509
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe PR adds Mac-scoped workspace group aggregation, structured secondary workspace snapshots, remote group ID mapping for mutations, owner-scoped collapse persistence, and grouped-section rendering for multi-Mac selection. ChangesWorkspace group state
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SecondaryMac
participant MobileShellComposite
participant MobileWorkspaceAggregation
participant WorkspaceListView
SecondaryMac->>MobileShellComposite: return workspace and group snapshot
MobileShellComposite->>MobileWorkspaceAggregation: derive Mac-scoped workspace groups
MobileWorkspaceAggregation-->>WorkspaceListView: provide grouped workspace sections
WorkspaceListView->>MobileShellComposite: request foreground group mutation
MobileShellComposite->>SecondaryMac: send remote group identifier
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (21 passed)
✨ Finishing Touches 💡 1📝 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 |
![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/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceGroupCollapseStore.swift`:
- Around line 77-84: Update the migration logic around the owner-scoped group
handling to migrate a legacy raw rpcGroupID key only when exactly one live group
matches that ID. When multiple owners match, remove or ignore the ambiguous
legacy key and preserve each group’s existing isCollapsed value; also fail
closed when authoritative identity is unavailable. Add a regression test
covering duplicate rpcGroupID values during upgrade.
🪄 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 Plus
Run ID: 88092490-815f-4665-934b-1a4ea6d7d5f8
📒 Files selected for processing (17)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+SecondaryPromotion.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceActions.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceCreateRequest.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/SecondaryWorkspaceFetchAttempt.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/SecondaryWorkspaceSnapshot.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileShellWorkspaceCapabilityTests.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceAggregation.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceGroupCollapseStore.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceGroupPreview.swiftPackages/iOS/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/MobileWorkspaceAggregationTests.swiftPackages/iOS/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/MobileWorkspaceGroupCollapseStoreTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+DragDrop.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+MacSelection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceMacSelectionScope.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceMacSelectionTests.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
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)
Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceAggregation.swift (1)
126-132: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winBuild group membership from the same IDs used by headers.
derivedWorkspacesscopesworkspace.groupID, butderivedGroupsscopes the header withgroup.remoteGroupID ?? group.id. When those values differ,MobileWorkspaceListItem.itemscannot find the workspace’s group in its lookup table, so the grouped list breaks down in larger Mac-local groupings. Use one source-of-truth group key for both workspaces and groups, or update the derived group header to the same RPC/local ID that member workspaces reference. Also add a coverage case whereMobileWorkspaceGroupPreview.idandremoteGroupIDdiffer.🤖 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/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceAggregation.swift` around lines 126 - 132, The workspace membership key in MobileWorkspaceAggregation must match the group header key used by derivedGroups: align workspace.groupID and the group.remoteGroupID ?? group.id source-of-truth so MobileWorkspaceListItem.items can resolve memberships. Update MobileWorkspaceAggregation.swift lines 126-132 accordingly, and add coverage in MobileWorkspaceAggregationTests.swift lines 214-275 where MobileWorkspaceGroupPreview.id differs from remoteGroupID.Source: Path instructions
🤖 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/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceAggregation.swift`:
- Around line 126-132: The workspace membership key in
MobileWorkspaceAggregation must match the group header key used by
derivedGroups: align workspace.groupID and the group.remoteGroupID ?? group.id
source-of-truth so MobileWorkspaceListItem.items can resolve memberships. Update
MobileWorkspaceAggregation.swift lines 126-132 accordingly, and add coverage in
MobileWorkspaceAggregationTests.swift lines 214-275 where
MobileWorkspaceGroupPreview.id differs from remoteGroupID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5a0f0d60-ffaa-4cb1-8c56-c475d0f51d6f
📒 Files selected for processing (5)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceAggregation.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceGroupCollapseStore.swiftPackages/iOS/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/MobileWorkspaceAggregationTests.swiftPackages/iOS/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/MobileWorkspaceGroupCollapseStoreTests.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
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 `@ios/cmuxUITests/cmuxUITests.swift`:
- Around line 347-372: Strengthen
testWorkspaceGroupsStayVisibleForAllComputersAcrossMultipleMacs by selecting and
asserting the All Computers scope explicitly, then verify each group and anchor
workspace through authoritative structured Mac/instance identifiers:
seed-group-0 under preview-macbook-pro/nightly and seed-group-1 under
preview-studio/stable. Keep the screenshot capture after these ownership
assertions.
🪄 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 Plus
Run ID: cb57cfc3-7983-48b9-9cc2-300fb18fa9ab
📒 Files selected for processing (2)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swiftios/cmuxUITests/cmuxUITests.swift
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.
Fixes the All Computers workspace list flattening when a secondary Mac appears.
Tests: red/green aggregation regression, All Computers view regression, 197 model tests, shell build, and scoped group-mutation test.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes grouped workspaces in All Computers by rendering groups from every Mac and keeping same‑ID groups distinct. Preserves device‑local collapse state and translates aggregate group IDs to Mac‑local IDs for all RPC mutations.
Bug Fixes
collapseStateID; migrate legacy raw‑ID entries only when uniquely attributable, ignore ambiguous cases.group_idvalues to Mac‑local IDs for create, move, and rename RPC calls.Refactors
MobileWorkspaceGroupPreview(remoteGroupID,macDeviceID,macInstanceTag) and drive workspace/group order from precomputed Mac display order.SecondaryWorkspaceSnapshotand shared group resolution; accept empty or missing group lists and preserve prior groups when omitted.canMutateForegroundGroupsForSelection; grouped sections remain when unfiltered.Written for commit 5c4587f. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes