Repository navigation
iOS: show per-build entries with build tags in computer pickers - #8816
Conversation
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. |
📝 WalkthroughWalkthroughThe iOS Mac-selection and task-composer flows now support optional pairing instance tags. Routing, workspace context validation, picker identity, switching, build labels, callbacks, previews, persistence, and related tests were updated to preserve device-level compatibility while distinguishing paired instances. ChangesInstance-aware Mac pairing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant WorkspaceListView
participant WorkspaceMacSelectionScope
participant WorkspaceShellView
participant MobileShellComposite
User->>WorkspaceListView: select pairing entry
WorkspaceListView->>WorkspaceMacSelectionScope: resolve switch target
WorkspaceMacSelectionScope-->>WorkspaceListView: macDeviceID and instanceTag
WorkspaceListView->>WorkspaceShellView: request Mac switch
WorkspaceShellView->>MobileShellComposite: switchToMac(macDeviceID, instanceTag)
MobileShellComposite-->>WorkspaceShellView: updated foreground pairing
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📝 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 makes iOS Mac pickers pairing-scoped: each paired build (Nightly, Stable, RC) gets its own labeled row, selection carries the
Confidence Score: 4/5Safe to merge after verifying or fixing the recovery submit path — the rest of the pairing-scoped picker plumbing is consistent and well-tested. The pairing-scoped picker, alias index, selection scope, draft/snapshot model, search, and list paths are all internally consistent and covered by new tests. One gap exists in reconcileCompletedOperation: it uses the current UI selection's instance tag rather than the snapshot's, so a recovery retry after a Mac-picker change could silently submit to the wrong app instance or fail with a misleading not-connected error. The main submit path handles this correctly, making the inconsistency easy to spot and fix. Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+CompletedOperationRecovery.swift — the reconcileCompletedOperation function uses selectedMachine?.instanceTag instead of snapshot.macInstanceTag. Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as WorkspaceMacTitlePicker
participant Scope as WorkspaceMacSelectionScope
participant List as WorkspaceListView
participant Store as MobileShellComposite
participant Comp as TaskComposerSheet
UI->>Scope: shouldSwitch(to: pairingID)
Scope-->>UI: true (active.id ≠ target.id)
UI->>List: switchMac(macDeviceID, instanceTag)
List->>Store: switchToMac(macDeviceID:instanceTag:)
Store-->>List: true (connected)
Note over Comp: Mac picker (composer)
Comp->>Comp: selectMachine(macDeviceID, instanceTag)
Comp->>Comp: "selectedMacInstanceTag = instanceTag"
Note over Comp: Submit
Comp->>Comp: captureSnapshot() → MobileTaskSubmissionSnapshot(macInstanceTag:)
Comp->>Store: submitTaskComposer(macDeviceID, snapshot.macInstanceTag, spec)
Store->>Store: matchesForegroundPairing → switchToMac if needed
Store->>Store: captureWorkspaceCreateContext (validates instanceTag)
Store-->>Comp: .success
Note over Comp: Recovery path (bug)
Comp->>Store: submitTaskComposer(snapshot.macDeviceID, selectedMachine?.instanceTag ⚠️, spec)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| #if DEBUG | ||
| /// Pairing rows supplied by the store-free workspace-list simulator fixture. | ||
| var previewDisplayPairedMacs: [MobilePairedMac] = [] | ||
| #endif |
There was a problem hiding this comment.
previewDisplayPairedMacs is a #if DEBUG-guarded stored property added to the production WorkspaceListView struct with no production caller — the only reach path is inside the #if DEBUG branch of displayPairedMacsForPicker. The canonical fix is to move the property into the dedicated debug file (WorkspaceListLayoutPreviewView.swift, which is already fully guarded by #if canImport(UIKit) && DEBUG) or into a new WorkspaceListView+Debug.swift extension, rather than polluting the main struct's surface.
Rule Used: Do not add new test/debug seams (ForTesting-styl... (source)
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!
There was a problem hiding this comment.
Fixed in 76c812d: the fixture rows moved into the DEBUG-only WorkspaceListLayoutPreviewView.swift behind UITestConfig.workspaceListLayoutPreviewEnabled; the production struct no longer carries fixture storage.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+TaskComposer.swift:
- Around line 260-263: The foreground-pairing condition is duplicated instead of
shared. In
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TaskComposer.swift#L260-L263,
make matchesForegroundPairing(macDeviceID:instanceTag:) module-internal by
removing private; in
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TaskDirectoryList.swift#L35-L44,
replace both inline foregroundMacDeviceID/activeMacInstanceTag checks with calls
to the shared helper.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swift`:
- Around line 566-570: Update the task-composer draft persistence and
restoration flow around submitTaskComposer so the selected machine’s instanceTag
is stored alongside macDeviceID. Read selectedMachine once and derive both
submission identity components from that same value, preserving the exact
selected pairing across relaunches instead of resolving only by Mac device.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView`+MacSelection.swift:
- Around line 79-99: Extract the shared Mac display-name and build-label
assembly used by macDisplayNamesByID() and macBuildLabelsByID() into a reusable
helper, then update both WorkspaceListView+MacSelection and WorkspaceShellView’s
workspaceShellRenderPresentation to use it. Preserve the existing macDeviceID/id
mappings and presenceSummary/build-label fallback behavior.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView.swift`:
- Around line 576-596: Replace the duplicated name and build-label assembly in
the workspace snapshot construction with the shared macDisplayNamesByID() and
macBuildLabelsByID() helpers from WorkspaceListView+MacSelection.swift. Preserve
the existing build-scope name transformation and pass the consolidated results
into WorkspaceMachineSnapshots.
🪄 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: 5f11ea0d-8c31-424d-a60b-6a878a1498f6
📒 Files selected for processing (25)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TaskComposer.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TaskDirectoryList.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceCreateRequest.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/WorkspaceCreatePinnedContext.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileTaskComposerSubmitTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/Debug/TaskComposer/TaskComposerAccessibilityPreviewView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerContextSection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerMachineMenu.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerMachineMenuActions.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerMachineMenuValue.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerRoutePicker.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+CompletedOperationRecovery.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceFilterMachine.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+MacSelection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceMacPickerAliasIndex.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceMacSelectionScope.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceMachineSnapshots.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView+WorkspaceActions.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceMacSelectionTests.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceMachineSnapshotsTests.swift
…elected pairing UIMenu bridging drops any Text wrapped in a stack inside a menu button label, so the build labels never rendered; menu rows are now bare Text/Text/Image tuples (title, subtitle, icon) in both the workspace title picker and the task composer machine menu. The collapsed picker title appends the build label when sibling builds share a name. Review fixes: the workspace-list preview pairing fixture moves out of the production view into the DEBUG-only fixture file behind UITestConfig; matchesForegroundPairing is shared with the directory list instead of duplicated; build-label derivation lives in one store helper; submission snapshots and composer drafts persist macInstanceTag (legacy drafts decode with nil) and submit reads the tag from the captured snapshot instead of live state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 (2)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swift (2)
111-122: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not retarget an unavailable tagged draft to a sibling build.
If the draft requested
nightlybut that pairing no longer exists, the active/first fallback silently selects another instance on the same Mac. Preserve the requested tag with no selected machine, or clear the selection, until the user explicitly chooses a valid pairing.As per coding guidelines and path instructions, correctness-critical pairing identity must use a structured source and fail closed when it is unavailable.
🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swift` around lines 111 - 122, Update the selectedMac resolution around draftInstanceTag so a draft with a requested instanceTag only selects the matching available pairing and otherwise remains unselected. Do not continue to the active or first same-Mac fallbacks when the requested tag is unavailable; retain those fallbacks only for drafts without a specific tag, using the structured pairing identity and fail-closed behavior.Sources: Coding guidelines, Path instructions
148-152: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInclude the instance tag when restoring the idempotency key.
isRequestEquivalent(to:)now treatsmacInstanceTagas request-defining, but this predicate can reuse a draft operation ID after the selected tag changed (including legacynil→ active tagged pairing). Require tag equality here and incanRestoreCompletedOperationbefore restoring either ID.🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swift` around lines 148 - 152, Update the draft restoration predicate around restoredOperationID to also require equality between the selected mac instance tag and the draft’s macInstanceTag, preserving nil-to-active-tag changes as non-equivalent. Apply the same tag-equality requirement in canRestoreCompletedOperation before restoring either operation ID.
🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swift`:
- Around line 111-122: Update the selectedMac resolution around draftInstanceTag
so a draft with a requested instanceTag only selects the matching available
pairing and otherwise remains unselected. Do not continue to the active or first
same-Mac fallbacks when the requested tag is unavailable; retain those fallbacks
only for drafts without a specific tag, using the structured pairing identity
and fail-closed behavior.
- Around line 148-152: Update the draft restoration predicate around
restoredOperationID to also require equality between the selected mac instance
tag and the draft’s macInstanceTag, preserving nil-to-active-tag changes as
non-equivalent. Apply the same tag-equality requirement in
canRestoreCompletedOperation before restoring either operation ID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e43a0361-a6b0-42a0-9b24-18a7c548f018
📒 Files selected for processing (16)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+PairedMacAliases.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TaskComposer.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+TaskDirectoryList.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileTaskComposerDraft.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileTaskSubmissionSnapshot.swiftPackages/iOS/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/MobileTaskSubmissionSnapshotTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerMachineMenu.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet+DraftState.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TaskComposer/TaskComposerSheet.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+MacSelection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceMachineSnapshots.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceMachineSnapshotsTests.swiftios/cmux/Resources/Localizable.xcstrings
💤 Files with no reviewable changes (1)
- Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swift
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceMacSelectionTests.swift (1)
39-42: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the pairing instance tag in these stubs.
Each updated
switchMacclosure accepts the newString?argument but discards it as_. These tests still pass if routing drops or misroutestarget.instanceTag. Add a sibling-build fixture sharing onemacDeviceID, capture both arguments, and assert the exact(macDeviceID, instanceTag)pair.Also applies to: 63-66, 96-99, 160-166, 240-246, 323-332, 416-426, 493-499, 579-584
🤖 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/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceMacSelectionTests.swift` around lines 39 - 42, The switchMac test stubs currently discard the pairing instance tag, so they do not verify routing. Update each referenced closure to capture both macDeviceID and instanceTag, add a sibling-build fixture using the same macDeviceID where needed, and assert the exact expected (macDeviceID, instanceTag) pair in the associated tests.
🤖 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/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceMacSelectionTests.swift`:
- Around line 39-42: The switchMac test stubs currently discard the pairing
instance tag, so they do not verify routing. Update each referenced closure to
capture both macDeviceID and instanceTag, add a sibling-build fixture using the
same macDeviceID where needed, and assert the exact expected (macDeviceID,
instanceTag) pair in the associated tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2a149765-0431-41da-8654-d7ffb572eb8a
📒 Files selected for processing (3)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceMacSelectionTests.swiftios/cmux/Resources/Localizable.xcstrings
1 participant
A Mac paired with two builds (e.g. Nightly and Stable) shows two labeled rows in the Computers sheet, but the workspace-list "Choose Computer" picker collapsed both into one unlabeled entry, and the task composer machine menu showed two identical rows that both read as selected. You could not tell which build you were picking, and picking could not target a specific build.
Picker entries are now pairing-scoped: the entry identity is the same
macDeviceID + instanceTagpairing ID the Computers sheet uses, so each build is its own row. Each row shows the build-channel label ("Nightly", "Stable", "RC", "DEV · tag") as a menu subtitle, resolved with the same priority as the sheet badge: live presencebuildLabel, falling back toMacBuildChannelon the stored instance tag when offline. Selecting a row switches the foreground connection to that exact app instance via the existingswitchToMac(macDeviceID:instanceTag:), including switching between sibling builds on the same physical Mac, which previously was treated as "already there". The task composer menu gets the same labels, carries the instance tag through directory search, pinned-context validation, and submit, and its double-checkmark is fixed by comparing pairing IDs.Workspace filtering intentionally stays device-scoped: workspace previews carry no instance tag, so both sibling rows scope the list to the same physical Mac, and the filter menu keeps one unlabeled entry per device.
taskTemplateStore.setLastMacDeviceIDandpendingMacSwitchIDalso stay device-scoped.A DEBUG workspace-list preview fixture (
CMUX_UITEST_WORKSPACE_LIST_PREVIEW=1) now seeds one device with Nightly + Stable pairings so the picker rows are verifiable in a simulator without live pairings.Tests: pairing-scoped snapshot/alias/switch coverage in
WorkspaceMachineSnapshotsTests(first commit adds them red, second turns them green), plus composer submit coverage. NoteCmuxMobileShellUIpackage tests do not run on a macOS host (pre-existing platform-declaration gap, also not in CI's package list); the composer suites inCmuxMobileShellpass on host (25/25), and behavior was verified in a simulator via the fixture.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Show each paired Mac build as its own entry with a build label in iOS computer pickers, and route actions to the exact app instance. The composer and workspace flows remember and use the selected pairing.
New Features
instanceTag) and show build subtitles; the collapsed title appends “Name · Build” when sibling builds share a name. Build labels resolve from live presence or fall back toMacBuildChannelviapairedMacBuildLabelsByEntryID.instanceTag; drafts and submission snapshots persistmacInstanceTag(legacy drafts decode as nil) and restore the exact pairing. The workspace title picker lists per-build rows and switches only for paired entries; unpaired, workspace-only devices remain single rows. The workspace-list preview pairing fixture moved to a DEBUG-only file behindUITestConfig.Bug Fixes
matchesForegroundPairingis shared across calls.Written for commit 5254bcf. Summary will update on new commits.
Summary by CodeRabbit