Skip to content

Fix workspace group header shortcuts - #6162

Open
leegisang wants to merge 10 commits into
manaflow-ai:mainfrom
leegisang:fix/workspace-group-header-shortcuts
Open

leegisang wants to merge 10 commits into
manaflow-ai:mainfrom
leegisang:fix/workspace-group-header-shortcuts

Conversation

@leegisang

@leegisang leegisang commented Jun 15, 2026 •

Copy link
Copy Markdown

Summary

  • stop creating a new terminal workspace just to serve as a workspace group header
  • keep group headers out of Cmd-number workspace selection and number only visible workspace rows
  • update Mac sidebar, mobile mirror semantics, tests, and workspace-group docs for promoted-anchor groups

Root cause

Workspace groups used a fresh anchor workspace as the group header representation. That hidden/virtual-looking terminal workspace still lived in the workspace list, so Cmd+number navigation counted the group header and hidden grouped members instead of the visible open workspace rows.

Validation

  • git diff --check
  • jq empty web/messages/en.json web/messages/ja.json

Not run

  • swift test --package-path Packages/CmuxWorkspaces and swift test --package-path Packages/CmuxMobileShellModel: local Command Line Tools Swift cannot parse the package manifests (.swiftLanguageMode(.v6) / iOS 18 platform).
  • xcodebuild / app build: active developer directory is /Library/Developer/CommandLineTools, not Xcode.
  • ./scripts/reload.sh --tag fix-workspace-groups: failed because zig is not installed.

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Fixes Cmd+number workspace selection to target only visible workspace rows and keeps digits consistent across sidebar badges and View → Workspace N. Groups now promote an existing anchor; when expanded the anchor shows as a normal row under the header.

  • Bug Fixes

    • Cmd+number, View → Workspace N, and sidebar digit badges share one TabManager-backed visible-row mapping; headers and collapsed members are skipped, expanded anchors are included, and 9 selects the last workspace.
    • View → Workspace N calls TabManager so menu and shortcuts stay in sync.
    • Header unread dot aggregates member unread only when collapsed; shows none when expanded.
    • Header selection reflects the anchor only when collapsed; when expanded the anchor is a normal selectable row.
    • Removed anchor-only title fallback now that the anchor renders as a row when expanded.
  • Migration

    • Group creation no longer spawns a new terminal; the first eligible selected workspace becomes the anchor.
    • workspace.group.create with an explicit empty children list now fails; header-only groups are not created.
    • Legacy socket cwd for group creation is accepted for compatibility but ignored; docs and localized text updated to “anchor positions the group.”

Written for commit 7326d00. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

Release Notes

  • New Features
    • Workspace group creation now promotes the first eligible selected workspace as the anchor (no extra header workspace); when expanded, the anchor appears as its own row.
    • Numbered workspace shortcuts (1–9) now follow the sidebar’s visible ordering.
    • Workspace group header interactions were aligned: the chevron collapses/expands, while selecting the header targets the anchor.
  • Bug Fixes
    • Updated unread badge behavior to match expanded vs. collapsed group states.
  • Documentation
    • Updated workspace-groups docs and localized text to reflect the new anchoring and shortcut behavior.
  • Tests
    • Expanded coverage for rendering, ordering, unread badges, and shortcut selection.

@vercel

vercel Bot commented Jun 15, 2026

Copy link
Copy Markdown

@leegisang is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Workspace group creation no longer creates a fresh terminal workspace as the group header anchor; instead, the first eligible selected child workspace is promoted as the anchor. This change cascades through the WorkspaceGroupCoordinator, TabManager, SidebarWorkspaceRenderItem, mobile shell rendering, numbered digit shortcuts (now UUID-based), control socket docs, tests, and user-facing documentation in 22 languages.

Changes

Anchor-Promoted Workspace Groups

Layer / File(s) Summary
WorkspaceGroup model & coordinator: anchor promotion
Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceGroup.swift, Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Coordinators/WorkspaceGroupCoordinator.swift, Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Coordinators/WorkspaceGroupHosting.swift, Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel+GroupInvariants.swift, Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspaceTabRepresenting.swift, Sources/TabManager.swift
WorkspaceGroup anchor comments clarify positioning role; createWorkspaceGroup promotes eligibleChildren.first as anchorId, builds the group directly without calling host to create terminal, simplifies placeNewWorkspaceGroupAtCreationPosition by removing standalone anchor parameter, narrows workspaceOrderDidChange payload to moved child IDs only; TabManager removes createGroupAnchorWorkspace(...).
Control socket & RPC: group creation semantics
Packages/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/..., Packages/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileSyncWorkspaceListResponse.swift, Sources/TerminalController+ControlWorkspaceGroupContext.swift
Control workspace-group docs clarify anchor promotion instead of fresh terminal, validation distinguishes param-shape failures from live-state resolution, focus resolution returns existing anchor ref; RPC anchor property comment changed from "owns" to "positions".
Mac sidebar: anchor row emission & group header state
Sources/SidebarWorkspaceRenderItem.swift, Sources/VerticalTabsSidebar+WorkspaceGroups.swift, Sources/ContentView.swift, Sources/SidebarWorkspaceGroupHeaderView.swift, Sources/TabManager+WindowTitle.swift
SidebarWorkspaceRenderItem removes skip-anchor-row guard so anchor is emitted as regular indented row when expanded; adds numberedShortcutWorkspaceId per-row and numberedShortcutWorkspaceIds(...) helper; render context precomputes workspaceShortcutDigitById and workspaceGroupUnreadCountById; VerticalTabsSidebar gates isAnchorActive/unread on group.isCollapsed, swaps .id()/.sidebarWorkspaceFrameAnchor() between anchorWorkspaceId and group.id by collapse state.
Numbered workspace shortcuts: UUID-based digit mapping
Sources/WorkspaceShortcutMapper.swift, Sources/TabManager+WorkspaceShortcuts.swift, Sources/AppDelegate.swift, Sources/cmuxApp.swift, Sources/App/TerminalDirectoryOpenSupport.swift
WorkspaceShortcutMapper gains workspaceId(forDigit:workspaceIds:) and digitForWorkspace(at:workspaceCount:); TabManager gains numberedWorkspaceShortcutWorkspaceIds and selectWorkspaceByShortcutDigit(_:); old in-file WorkspaceShortcutMapper is removed; AppDelegate and cmuxApp replace workspaceIndex→selectTab(at:) path with selectWorkspaceByShortcutDigit.
Xcode project build wiring
cmux.xcodeproj/project.pbxproj
TabManager+WorkspaceShortcuts.swift and WorkspaceShortcutMapper.swift added with PBXBuildFile, PBXFileReference, and PBXSourcesBuildPhase entries for compilation.
Mobile shell: anchor row & unread badge semantics
Packages/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceListItem.swift, Packages/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceGroupPreview.swift, Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceGroupHeaderRow.swift, Packages/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/MobileWorkspaceListItemTests.swift
MobileWorkspaceListItem emits anchor as separate indented row when expanded and computes groupHeader unread only when collapsed from anyMemberUnreadByGroupID; MobileWorkspaceGroupPreview anchor comments change from "owns" to "positions"; tests validate expanded rendering and unread deduplication.
Coordinator & workspace-group behavior tests
Packages/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift, cmuxTests/WorkspaceGroupTests.swift, cmuxTests/MobileWorkspaceListFidelityTests.swift
StubGroupHost removes createGroupAnchorWorkspace; coordinator tests assert anchor equals first child tab and expect single closure on delete; WorkspaceGroupTests adds 25+ ordering/membership/renaming expectations; mobile test provisions explicit anchor workspace.
Docs & i18n
docs/workspace-groups.md, web/messages/en.json, web/messages/{ja,ar,bs,da,de,es,fr,it,km,ko,no,pl,pt-BR,ru,th,tr,uk,zh-CN,zh-TW}.json
Workspace Groups docs rewritten to describe anchor promotion, updated ⌘⇧G shortcut wording, removed --cwd from CLI example; all 22 language web messages updated to match anchor-promoted behavior (folder-style header, anchor as positioning member, no extra terminal workspace).

Sequence Diagram(s)

sequenceDiagram
  rect rgba(173, 216, 230, 0.5)
    Note over Caller,TabManager: Group Creation (anchor promoted from children)
  end
  participant Caller
  participant WorkspaceGroupCoordinator
  participant WorkspaceGroup
  participant TabManager

  Caller->>WorkspaceGroupCoordinator: createWorkspaceGroup(eligibleChildren)
  WorkspaceGroupCoordinator->>WorkspaceGroupCoordinator: anchorId = eligibleChildren.first
  WorkspaceGroupCoordinator->>WorkspaceGroup: init(anchorWorkspaceId: anchorId)
  WorkspaceGroupCoordinator->>WorkspaceGroupCoordinator: assign groupId to each child
  WorkspaceGroupCoordinator->>WorkspaceGroupCoordinator: placeNewWorkspaceGroupAtCreationPosition(childIds)
  WorkspaceGroupCoordinator->>TabManager: select promoted anchor (if selectAnchor)
  WorkspaceGroupCoordinator-->>Caller: workspaceOrderDidChange(movedIds: eligibleChildren)
Loading
sequenceDiagram
  rect rgba(255, 218, 185, 0.5)
    Note over cmuxApp,SidebarWorkspaceRenderItem: Digit shortcut → workspace selection (UUID-based)
  end
  participant cmuxApp
  participant TabManager
  participant SidebarWorkspaceRenderItem
  participant WorkspaceShortcutMapper

  cmuxApp->>TabManager: selectWorkspaceByShortcutDigit(digit)
  TabManager->>SidebarWorkspaceRenderItem: numberedShortcutWorkspaceIds(tabs, groupsById)
  Note right of SidebarWorkspaceRenderItem: skips group headers & collapsed members
  SidebarWorkspaceRenderItem-->>TabManager: [UUID]
  TabManager->>WorkspaceShortcutMapper: workspaceId(forDigit:, workspaceIds:)
  WorkspaceShortcutMapper-->>TabManager: UUID?
  TabManager->>TabManager: find tab by UUID, selectTab
  TabManager-->>cmuxApp: Bool
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Possibly related PRs

  • manaflow-ai/cmux#4989: Directly overlaps group-creation and placement logic—both adjust placeNewWorkspaceGroupAtCreationPosition and anchor promotion/ordering in WorkspaceGroupCoordinator.
  • manaflow-ai/cmux#4815: Modifies the same sidebar grouping and anchor-based group UI state rendering in TabManager, SidebarWorkspaceRenderItem, and VerticalTabsSidebar+WorkspaceGroups.
  • manaflow-ai/cmux#5196: Overlaps numbered-workspace shortcut 1–9 digit selection and validation path touched by this PR's WorkspaceShortcutMapper and TabManager integration.

Suggested reviewers

  • Ari4ka

Poem

🐰 No more fresh terminal for the folder's crown,
The first chosen workspace steps up in renown!
Promoted to anchor, it positions the group,
While collapsed or expanded—no unread badge dupe.
⌘⇧G now skips the extra tab hop,
One fewer creation—this bunny won't stop! 🎉

🚥 Pre-merge checks | ✅ 19 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 17.46% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description provides a clear summary of changes, root cause, and validation approach, but omits testing and demo video sections required by the template. Add the 'Testing' section with details on how the change was tested and what was verified manually. Include or note if a demo video is not applicable.
✅ Passed checks (19 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: fixing workspace group header shortcuts by removing synthetic terminal workspaces and adjusting workspace numbering.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed No Swift actor isolation violations detected. New TabManager extension properly on MainActor, WorkspaceShortcutMapper is pure utility enum, modified UI types appropriately marked, all value models...
Cmux Swift Blocking Runtime ✅ Passed PR introduces no new blocking or timing-based synchronization. New code (WorkspaceShortcutMapper, TabManager+WorkspaceShortcuts) contains only pure utilities. All pre-existing blocking patterns ver...
Cmux Expensive Synchronous Load ✅ Passed No expensive synchronous loaders added to main actor or interactive paths; new @MainActor code performs only in-memory dictionary/array operations.
Cmux Cache Substitution Correctness ✅ Passed Cache substitution correctness verified: workspaceShortcutDigitById and workspaceGroupUnreadCountById are precomputed only for transient UI display (badges/hints), not persistence. selectWorkspaceB...
Cmux No Hacky Sleeps ✅ Passed PR contains only Swift code changes; rule applies only to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. No covered file types were modified.
Cmux Algorithmic Complexity ✅ Passed All hot paths (UI rendering, body evaluation) use linear O(n) algorithms without nested collection scans. ContentView precomputes workspace shortcut mapping once per body evaluation using single-pa...
Cmux Swift Concurrency ✅ Passed PR introduces no new legacy async patterns. New files (TabManager+WorkspaceShortcuts.swift, WorkspaceShortcutMapper.swift) contain only synchronous @MainActor code and pure calculations. Pre-existi...
Cmux Swift @Concurrent ✅ Passed PR modifies workspace group handling, documentation, and workspace shortcuts without violating concurrent annotation rules: no nonisolated async functions lack @concurrent, no @concurrent is...
Cmux Swift File And Package Boundaries ✅ Passed PR introduces two small new Swift files (41 and 25 lines) with single clear responsibilities; oversized files receive minimal changes (≤22 net lines, well under 250-line limit); code extraction fro...
Cmux Swift Logging ✅ Passed Logging changes comply with Swift logging rules: sendTextWhenReady and scheduleLaunchServicesBundleRegistration changed from unconditional NSLog to #if DEBUG-guarded cmuxDebugLog, eliminating ungua...
Cmux User-Facing Error Privacy ✅ Passed PR changes don't violate user-facing error privacy rules: debug logging was moved behind #if DEBUG guards (improving privacy), error messages are generic product-level text, and documentation updat...
Cmux Full Internationalization ✅ Passed All 20 supported locales in web/messages have matching translations; no unlocalized user-facing strings added; debug logs and code comments properly excluded.
Cmux Swiftui State Layout ✅ Passed SwiftUI changes follow the rules: new values (workspaceShortcutDigitById, workspaceGroupUnreadCountById) are computed as immutable let snapshots in body; row snapshot passing is correct (TabItemV...
Cmux Architecture Rethink ✅ Passed PR eliminates synthetic anchor workspace creation and fixes Cmd-digit navigation via pure WorkspaceShortcutMapper, single TabManager owner, and immutable precomputed RenderContext—no timing, locks,...
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed No new NSWindow, NSPanel, NSWindowController, SwiftUI Window, or WindowGroup declarations were introduced. The two new helper files (TabManager+WorkspaceShortcuts.swift, WorkspaceShortcutMapper.swi...
Cmux Source Artifacts ✅ Passed All 60 changed files are intentional source artifacts: 34 hand-written Swift source files, 4 test files, 1 Xcode project configuration, 1 documentation file, and 20 localization catalogs. No prohib...

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 15, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes Cmd+number workspace selection by replacing the raw-index approach with a visible-row mapping that skips group headers and collapsed members. It also removes the "fresh anchor terminal" pattern from group creation — the first eligible selected workspace is now promoted as the anchor, eliminating a hidden workspace that previously inflated the workspace count and broke shortcut numbering.

  • Shortcut routing: WorkspaceShortcutMapper gains ID-based overloads; TabManager+WorkspaceShortcuts routes Cmd+number through SidebarWorkspaceRenderItem.renderItems so the digit targets the same row the user sees in the sidebar. The precomputed workspaceShortcutDigitById in RenderContext keeps sidebar badges in sync with the same mapping.
  • Anchor-promotion: WorkspaceGroupCoordinator.createWorkspaceGroup now promotes eligibleChildren.first as the anchor and removes the createGroupAnchorWorkspace host protocol method entirely. When a group is expanded, the anchor renders as a normal workspace row under the folder-style header; when collapsed, only the header is visible and it reflects the anchor's active/unread state.
  • Locale and docs: All 20 locale files and docs/workspace-groups.md are updated to describe the promoted-anchor model, resolving the previously flagged stale-copy issue across every supported locale.

Confidence Score: 5/5

Safe to merge; the behavioral change is coherent, well-tested, and the stale-locale issue from the previous review round is fully addressed.

The core group-creation and shortcut-routing changes are mechanically straightforward — removing one workspace creation call and replacing a raw index lookup with a visible-row mapping. All 20 locale files are updated. The new numberedWorkspaceShortcutsSkipGroupHeadersAndCollapsedMembers test exercises the end-to-end shortcut flow, and the existing coordinator and group-ordering tests were revised to match the promoted-anchor invariant. The one non-blocking concern is a dead anchorWorkingDirectory parameter left in the public coordinator API with a suppression assignment; this is documented but should be cleaned up in a follow-up.

No files require special attention. WorkspaceGroupCoordinator.swift has a dead public parameter worth tracking for future cleanup.

Important Files Changed

Filename Overview
Sources/WorkspaceShortcutMapper.swift New file: extracts WorkspaceShortcutMapper from TerminalDirectoryOpenSupport and adds two ID-based overloads; clean factoring with correct digit-9 'last workspace' semantics.
Sources/TabManager+WorkspaceShortcuts.swift New extension routing Cmd+number through visible-row IDs rather than raw tab index; correct use of SidebarWorkspaceRenderItem.numberedShortcutWorkspaceIds to skip headers and collapsed members.
Sources/ContentView.swift Precomputes workspaceShortcutDigitById and workspaceGroupUnreadCountById in RenderContext; uses sidebarUnread instead of notificationStore for collapsed-group badge aggregation — semantically consistent with the rest of the sidebar but worth confirming they delegate identically.
Sources/SidebarWorkspaceRenderItem.swift Removes anchor-skip logic so expanded group anchors appear as normal workspace rows; adds numberedShortcutWorkspaceId and the static numberedShortcutWorkspaceIds helper; logic is clean and correct.
Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Coordinators/WorkspaceGroupCoordinator.swift Core behavioral change: promotes first eligible child as anchor instead of creating a fresh terminal; placeNewWorkspaceGroupAtCreationPosition correctly includes the anchor in orderedChildIds; anchorWorkingDirectory is now silently discarded via _ = anchorWorkingDirectory leaving a dead public-API parameter.
Sources/AppDelegate.swift Shortcut handler now calls manager.selectWorkspaceByShortcutDigit(digit) instead of manager.selectTab(at: targetIndex); correctly unifies shortcut and menu routing through TabManager.
Sources/VerticalTabsSidebar+WorkspaceGroups.swift Collapsed-header isAnchorActive now guards on group.isCollapsed; unread badge delegates to precomputed renderContext dictionary; shortcutDigit/modifierSymbol correctly set to nil; .id and .sidebarWorkspaceFrameAnchor updated for expanded vs collapsed identity.
cmuxTests/WorkspaceGroupTests.swift Tests correctly updated for promoted-anchor semantics; new numberedWorkspaceShortcutsSkipGroupHeadersAndCollapsedMembers test exercises the full shortcut flow end-to-end; existing ordering, drag, and ungroup tests all revised to match the no-fresh-anchor invariant.
Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceListItem.swift Mirrors Mac changes: anchor is no longer skipped when expanded; header unread badge is false when expanded, aggregates all members when collapsed; anchorUnreadByGroupID tracking removed as no longer needed.
web/messages/en.json Updated anchorDesc, anchorNew, creatingIntro, keyboardDesc and related keys to reflect promoted-anchor semantics; all 20 locale files updated in this PR, resolving the previously flagged stale-locale issue.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User presses Cmd+N] --> B[AppDelegate shortcut handler]
    B --> C[manager.selectWorkspaceByShortcutDigit digit]
    C --> D[numberedWorkspaceShortcutWorkspaceIds]
    D --> E[SidebarWorkspaceRenderItem.renderItems]
    E --> F{For each tab in order}
    F --> G{Has groupId?}
    G -- No --> H[Emit .workspace row]
    G -- Yes --> I{Group collapsed?}
    I -- Yes --> J[Emit .groupHeader only]
    I -- No --> L[Emit .workspace row incl. anchor]
    J --> F
    H --> F
    L --> F
    F --> M[compactMap numberedShortcutWorkspaceId]
    M --> N[WorkspaceShortcutMapper.workspaceId forDigit]
    N --> O{digit == 9?}
    O -- Yes --> P[Select last visible workspace]
    O -- No --> Q[Select workspace at digit-1 index]
    P --> R[TabManager.selectWorkspace]
    Q --> R
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[User presses Cmd+N] --> B[AppDelegate shortcut handler]
    B --> C[manager.selectWorkspaceByShortcutDigit digit]
    C --> D[numberedWorkspaceShortcutWorkspaceIds]
    D --> E[SidebarWorkspaceRenderItem.renderItems]
    E --> F{For each tab in order}
    F --> G{Has groupId?}
    G -- No --> H[Emit .workspace row]
    G -- Yes --> I{Group collapsed?}
    I -- Yes --> J[Emit .groupHeader only]
    I -- No --> L[Emit .workspace row incl. anchor]
    J --> F
    H --> F
    L --> F
    F --> M[compactMap numberedShortcutWorkspaceId]
    M --> N[WorkspaceShortcutMapper.workspaceId forDigit]
    N --> O{digit == 9?}
    O -- Yes --> P[Select last visible workspace]
    O -- No --> Q[Select workspace at digit-1 index]
    P --> R[TabManager.selectWorkspace]
    Q --> R
Loading

Reviews (7): Last reviewed commit: "Merge branch 'main' into fix/workspace-g..." | Re-trigger Greptile

Comment thread Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceGroup.swift Outdated
Comment thread Sources/VerticalTabsSidebar+WorkspaceGroups.swift Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceGroupPreview.swift (1)

39-48: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align the initializer docs with the new anchor semantics.

The property comment now says the anchor workspace positions the group, but the anchorWorkspaceID parameter still says it owns the group. That leaves the generated API docs contradictory.

🤖 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/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceGroupPreview.swift`
around lines 39 - 48, The property documentation for anchorWorkspaceID states
that it positions the group, but the initializer parameter documentation for
anchorWorkspaceID states that it owns the group. Update the parameter
documentation comment for anchorWorkspaceID in the initializer to use
"positions" instead of "owns" to align with the property's semantic meaning and
ensure consistency in the generated API documentation.
🤖 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/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift`:
- Line 267: The `try! `#require`(...)` pattern at the assignment to groupId uses
force try which can crash the test process instead of reporting a proper test
failure. Replace the force try (`try!`) with a regular `try` statement in a
throwing context to allow proper error propagation and test failure reporting.
Ensure the test function containing this line is marked as `throws` so that any
errors from the `createWorkspaceGroup` call will properly fail the test instead
of hard-crashing the test process.

---

Outside diff comments:
In
`@Packages/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceGroupPreview.swift`:
- Around line 39-48: The property documentation for anchorWorkspaceID states
that it positions the group, but the initializer parameter documentation for
anchorWorkspaceID states that it owns the group. Update the parameter
documentation comment for anchorWorkspaceID in the initializer to use
"positions" instead of "owns" to align with the property's semantic meaning and
ensure consistency in the generated API documentation.
🪄 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: 8316370a-b1c6-4bd4-b40a-be986f69eac2

📥 Commits

Reviewing files that changed from the base of the PR and between 5321bec and fe111bf.

📒 Files selected for processing (29)
  • Packages/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlWorkspaceGroupContext.swift
  • Packages/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlWorkspaceGroupCreateResolution.swift
  • Packages/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/WorkspaceGroup/ControlWorkspaceGroupFocusResolution.swift
  • Packages/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileSyncWorkspaceListResponse.swift
  • Packages/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceGroupPreview.swift
  • Packages/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceListItem.swift
  • Packages/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/MobileWorkspaceListItemTests.swift
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceGroupHeaderRow.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Coordinators/WorkspaceGroupCoordinator.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Coordinators/WorkspaceGroupHosting.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspaceTabRepresenting.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel+GroupInvariants.swift
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceGroup.swift
  • Packages/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift
  • Sources/App/TerminalDirectoryOpenSupport.swift
  • Sources/AppDelegate.swift
  • Sources/ContentView.swift
  • Sources/SidebarWorkspaceGroupHeaderView.swift
  • Sources/SidebarWorkspaceRenderItem.swift
  • Sources/TabManager+WindowTitle.swift
  • Sources/TabManager.swift
  • Sources/TerminalController+ControlWorkspaceGroupContext.swift
  • Sources/VerticalTabsSidebar+WorkspaceGroups.swift
  • Sources/cmuxApp.swift
  • cmuxTests/MobileWorkspaceListFidelityTests.swift
  • cmuxTests/WorkspaceGroupTests.swift
  • docs/workspace-groups.md
  • web/messages/en.json
  • web/messages/ja.json
💤 Files with no reviewable changes (1)
  • Sources/TabManager+WindowTitle.swift

Comment thread Packages/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Sources/VerticalTabsSidebar+WorkspaceGroups.swift (1)

27-31: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid per-render full member scans for collapsed-group unread counts.

Line 27–31 recomputes unread by iterating all memberWorkspaceIds inside a sidebar render path. With scalable data (~1000 workspaces), this introduces repeated O(n) scans per render/event. Precompute group unread totals once in the render-context snapshot (e.g., [UUID: Int] keyed by group id) and read O(1) here.

As per coding guidelines, this path must avoid repeated full scans over workspace/group collections in render hot paths and should use a cached/pre-indexed snapshot.

🤖 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 `@Sources/VerticalTabsSidebar`+WorkspaceGroups.swift around lines 27 - 31, The
`anchorUnreadCount` computation at lines 27-31 iterates through all
`memberWorkspaceIds` on every render, creating an O(n) performance issue with
large workspace counts. Replace the reduce loop that scans `memberWorkspaceIds`
with a simple O(1) dictionary lookup by precomputing a snapshot of unread totals
(keyed by group ID) earlier in the render-context pipeline, then accessing the
cached value here instead of recalculating it each time.

Source: Coding guidelines

🤖 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 `@Sources/VerticalTabsSidebar`+WorkspaceGroups.swift:
- Around line 27-31: The `anchorUnreadCount` computation at lines 27-31 iterates
through all `memberWorkspaceIds` on every render, creating an O(n) performance
issue with large workspace counts. Replace the reduce loop that scans
`memberWorkspaceIds` with a simple O(1) dictionary lookup by precomputing a
snapshot of unread totals (keyed by group ID) earlier in the render-context
pipeline, then accessing the cached value here instead of recalculating it each
time.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 08b51276-b6ce-40df-9bce-7654ef2f27c0

📥 Commits

Reviewing files that changed from the base of the PR and between fe111bf and 020e8c7.

📒 Files selected for processing (20)
  • Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/Values/WorkspaceGroup.swift
  • Sources/VerticalTabsSidebar+WorkspaceGroups.swift
  • web/messages/ar.json
  • web/messages/bs.json
  • web/messages/da.json
  • web/messages/de.json
  • web/messages/es.json
  • web/messages/fr.json
  • web/messages/it.json
  • web/messages/km.json
  • web/messages/ko.json
  • web/messages/no.json
  • web/messages/pl.json
  • web/messages/pt-BR.json
  • web/messages/ru.json
  • web/messages/th.json
  • web/messages/tr.json
  • web/messages/uk.json
  • web/messages/zh-CN.json
  • web/messages/zh-TW.json

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
Packages/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift (4)

199-199: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Replace try! with throwable test flow for consistency.

Line 199 uses try! #require(...), the same pattern that was fixed at Line 267. This can crash the test process instead of reporting a proper test failure.

♻️ Proposed fix
 `@Test`
-func createWorkspaceGroupAdoptsChildrenAndKeepsSectionContiguous() {
+func createWorkspaceGroupAdoptsChildrenAndKeepsSectionContiguous() throws {
     let (model, host, groups, _) = makeWorld()
@@
-    let group = try! `#require`(model.workspaceGroups.first(where: { $0.id == groupId }))
+    let group = try `#require`(model.workspaceGroups.first(where: { $0.id == groupId }))
🤖 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/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift`
at line 199, Replace the `try! `#require`(...)` pattern on line 199 where the
workspaceGroup is retrieved from model.workspaceGroups with the proper throwable
test flow pattern that was already applied at line 267. This will ensure the
code reports proper test failures instead of crashing the test process when the
required workspace group is not found.

231-231: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Replace try! with throwable test flow for consistency.

Line 231 uses try! #require(...), the same pattern that was fixed at Line 267. This can crash the test process instead of reporting a proper test failure.

♻️ Proposed fix
 `@Test`
-func deleteWorkspaceGroupClosesMembersAndClearsLastHoldout() {
+func deleteWorkspaceGroupClosesMembersAndClearsLastHoldout() throws {
     let (model, host, groups, _) = makeWorld()
@@
-    let groupId = try! `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id, b.id]))
+    let groupId = try `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id, b.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/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift`
at line 231, Replace the `try! `#require`(...)` pattern in the
createWorkspaceGroup call with a proper throwable test flow for consistency with
the pattern already used elsewhere in the test file. Instead of using force try
with `#require` which can crash the test process, use the appropriate throwable
pattern (likely `try `#require`(...)` or a similar construct that allows the test
framework to properly report failures) to ensure errors are handled as test
failures rather than crashes.

251-251: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Replace try! with throwable test flow for consistency.

Line 251 uses try! #require(...), the same pattern that was fixed at Line 267. This can crash the test process instead of reporting a proper test failure.

♻️ Proposed fix
 `@Test`
-func ungroupKeepsMemberPositionsAndDropsMembership() {
+func ungroupKeepsMemberPositionsAndDropsMembership() throws {
     let (model, host, groups, _) = makeWorld()
@@
-    let groupId = try! `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id]))
+    let groupId = try `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.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/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift`
at line 251, Replace the `try!` keyword with `try` in the
WorkspaceCoordinatorTests.swift file at line 251 where groupId is assigned using
`#require`(groups.createWorkspaceGroup(...)). This change ensures that if the
workspace group creation fails, the test will report a proper test failure
instead of crashing the test process, consistent with the fix already applied at
line 267.

306-306: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Replace try! with throwable test flow for consistency.

Line 306 uses try! #require(...), the same pattern that was fixed at Line 267. This can crash the test process instead of reporting a proper test failure.

♻️ Proposed fix
 `@Test`
-func setWorkspaceGroupAnchorHoistsNewAnchorToSectionFront() {
+func setWorkspaceGroupAnchorHoistsNewAnchorToSectionFront() throws {
     let (model, host, groups, _) = makeWorld()
@@
-    let groupId = try! `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id, b.id]))
+    let groupId = try `#require`(groups.createWorkspaceGroup(name: "G", childWorkspaceIds: [a.id, b.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/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift`
at line 306, Replace the try! unwrap in the groupId assignment where
createWorkspaceGroup is called with a proper throwable test flow for
consistency. Instead of using try! `#require`(...), use a pattern that allows test
failures to be reported properly rather than crashing the test process. This
matches the fix that was already applied to similar code elsewhere in the test
file.
🤖 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/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift`:
- Line 199: Replace the `try! `#require`(...)` pattern on line 199 where the
workspaceGroup is retrieved from model.workspaceGroups with the proper throwable
test flow pattern that was already applied at line 267. This will ensure the
code reports proper test failures instead of crashing the test process when the
required workspace group is not found.
- Line 231: Replace the `try! `#require`(...)` pattern in the createWorkspaceGroup
call with a proper throwable test flow for consistency with the pattern already
used elsewhere in the test file. Instead of using force try with `#require` which
can crash the test process, use the appropriate throwable pattern (likely `try
`#require`(...)` or a similar construct that allows the test framework to properly
report failures) to ensure errors are handled as test failures rather than
crashes.
- Line 251: Replace the `try!` keyword with `try` in the
WorkspaceCoordinatorTests.swift file at line 251 where groupId is assigned using
`#require`(groups.createWorkspaceGroup(...)). This change ensures that if the
workspace group creation fails, the test will report a proper test failure
instead of crashing the test process, consistent with the fix already applied at
line 267.
- Line 306: Replace the try! unwrap in the groupId assignment where
createWorkspaceGroup is called with a proper throwable test flow for
consistency. Instead of using try! `#require`(...), use a pattern that allows test
failures to be reported properly rather than crashing the test process. This
matches the fix that was already applied to similar code elsewhere in the test
file.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9393eb2c-5300-4beb-b7fe-d26abadee167

📥 Commits

Reviewing files that changed from the base of the PR and between 020e8c7 and 355449b.

📒 Files selected for processing (9)
  • Packages/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceGroupPreview.swift
  • Packages/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspaceCoordinatorTests.swift
  • Sources/App/TerminalDirectoryOpenSupport.swift
  • Sources/AppDelegate.swift
  • Sources/ContentView.swift
  • Sources/TabManager+WorkspaceShortcuts.swift
  • Sources/VerticalTabsSidebar+WorkspaceGroups.swift
  • Sources/WorkspaceShortcutMapper.swift
  • cmux.xcodeproj/project.pbxproj
💤 Files with no reviewable changes (1)
  • Sources/App/TerminalDirectoryOpenSupport.swift

Comment thread Sources/WorkspaceShortcutMapper.swift
@leegisang

Copy link
Copy Markdown
Author

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews resumed.

…eader-shortcuts

# Conflicts:
#	Sources/ContentView.swift
#	cmux.xcodeproj/project.pbxproj
…eader-shortcuts

# Conflicts:
#	Sources/AppDelegate.swift
#	Sources/VerticalTabsSidebar+WorkspaceGroups.swift
@teamleaderleo teamleaderleo added area: workspaces Workspaces, sessions, restore after relaunch, worktrees needs a call Finished and held for a team design or product decision (see #13742 and the gallery in #15427) review: needs-attention Actionable automated review finding needs an author reply labels Sep 30, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator

The anchor and visible-row behavior changes a workspace-group interaction model, so this needs the product call tracked in #15427 before landing.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: workspaces Workspaces, sessions, restore after relaunch, worktrees needs a call Finished and held for a team design or product decision (see #13742 and the gallery in #15427) review: needs-attention Actionable automated review finding needs an author reply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants