Fix workspace digit shortcut routing

[austinywang]

Fixes #5588

Summary

• What changed? Keep Select Workspace 1…9 shortcuts (Cmd+1…9) owned by the AppDelegate shortcut dispatcher instead of static SwiftUI menu key equivalents. menuShortcut(for: .selectWorkspaceByNumber) now always returns .unbound, and the per-digit .keyboardShortcut bindings were removed from the Workspace 1…9 menu items.
• Why? SwiftUI menu key equivalents could claim Cmd+1…9 after focus churn without invoking their command closure, silently breaking workspace switching (selectWorkspaceByNumber: default cmd+1..9 silently doesn't switch workspaces (0.64.13 and 0.64.14) #5588). Routing the entire digit family through the dispatcher keeps the dispatcher the single owner and fixes the regression.
• Added a regression test (WorkspaceNumberShortcutMenuRoutingTests) that locks in the invariant: the dispatcher keeps the default digit shortcut while the menu stays unbound.

Testing

• ./scripts/lint-pbxproj-test-wiring.sh — confirms the new test file is wired into the cmuxTests target.
• git diff --check
• CI on this HEAD is green, including the full app-host unit test suite, swift-package-tests, and release-build; the new WorkspaceNumberShortcutMenuRoutingTests regression test runs there.
• Local dev builds and bare xcodebuild were intentionally not run per task instructions (no-build constraint).

Demo Video

Not applicable — this is a non-visual keyboard-shortcut routing fix with no change to UI appearance. Behavior is verified by the WorkspaceNumberShortcutMenuRoutingTests regression test plus the full CI suite; manual/dogfood builds were intentionally not run per task instructions.

Review Trigger (Copy/Paste as PR comment)

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

Checklist

• I added or updated tests for behavior changes (WorkspaceNumberShortcutMenuRoutingTests)
• I updated docs/changelog if needed — N/A: internal shortcut-routing fix with no user-facing surface change
• I requested bot reviews after my latest commit
• All code review bot comments are resolved (CodeRabbit: "No actionable comments"; Greptile: 5/5 "safe to merge")
• All human review comments are resolved — none have been posted yet
• I tested the change locally — builds intentionally not run per task instructions; verified via green CI instead

---

Summary by cubic

Keep Cmd+1…9 workspace shortcuts owned by the AppDelegate dispatcher so SwiftUI menus can’t intercept them, ensuring consistent workspace switching. Fixes #5588.

• Bug Fixes
  • menuShortcut(for: .selectWorkspaceByNumber) now returns .unbound; removed all per-digit .keyboardShortcuts from Workspace 1…9 menu items so Cmd+1…9 always routes through the dispatcher.
  • Added WorkspaceNumberShortcutMenuRoutingTests to verify the dispatcher uses the default digit shortcut and the menu stays unbound.

^{Written for commit cfa515e. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Corrected workspace number shortcut routing so Cmd+1–9 continues to be handled by the app’s shortcut dispatcher without menu-triggered key-equivalent churn.
  • Updated the 1–9 workspace menu entries so they no longer claim Cmd+1–9 as menu key equivalents, improving shortcut consistency.
• Tests
  • Added unit tests covering that the workspace-number shortcut remains dispatcher-owned while the corresponding menu shortcut stays unbound.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	Jul 5, 2026 5:06am
cmux-staging	Building	Preview, Comment	Jul 5, 2026 5:06am

[coderabbitai]

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)

• .github/swift-file-length-budget.tsv is excluded by !**/*.tsv

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1e629152-82f4-49f2-85cb-fee020f5bf25

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Cmd+1…9 workspace selection now stays with the shortcut dispatcher: the menu shortcut for selectWorkspaceByNumber is unbound, the numbered workspace menu no longer installs a SwiftUI key equivalent, and a test bundle entry plus test verify the routing.

Changes

Workspace number shortcut routing

Layer / File(s)	Summary
Menu shortcut ownership Sources/KeyboardShortcutSettingsLookup.swift, Sources/cmuxApp.swift	selectWorkspaceByNumber now resolves to an unbound menu shortcut, and the numbered workspace menu button no longer conditionally attaches a SwiftUI keyboard shortcut.
Routing test coverage cmuxTests/WorkspaceNumberShortcutMenuRoutingTests.swift, cmux.xcodeproj/project.pbxproj	A serialized test suite installs an isolated settings store, checks dispatcher-owned shortcut ownership against the action default, asserts the menu shortcut is unbound, and adds the new test file to cmuxTests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• manaflow-ai/cmux#5196: Both PRs address the same numbered workspace “1–9” shortcut routing/rebinding behavior.
• manaflow-ai/cmux#6472: Both PRs adjust .selectWorkspaceByNumber routing for workspace selection behavior.

Suggested reviewers

• lawrencecchen

Poem

🐰 Hop, hop — the digits found their track,
and SwiftUI gave the shortcut back.
The dispatcher munches Cmd+one to nine,
while workspaces leap in a tidy line.
Thump! says the rabbit, carrot-pleased.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (24 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address #5588 by unbinding menu shortcuts, keeping Cmd+1...9 on the dispatcher path, and adding a regression test.
Out of Scope Changes check	✅ Passed	The diff stays focused on shortcut routing and its regression test without introducing unrelated scope.
Cmux Swift Actor Isolation	✅ Passed	PASS: The diff only adjusts UI shortcut routing and a serialized @MainActor test; it չի introduce new implicit MainActor models or unsafe shared mutable Sendable types.
Cmux Swift Blocking Runtime	✅ Passed	The touched Swift hunks only reroute Cmd+1…9 menu handling and add a main-actor test; they introduce no waits, sleeps, sync dispatch, polling, or new locks.
Cmux Browser Automation Off-Main	✅ Passed	No browser socket automation code changed; PR only adjusts workspace shortcut/menu routing and adds a regression test.
Cmux Expensive Synchronous Load	✅ Passed	Diff only changes shortcut routing and a main-actor test; no RestorableAgentSessionIndex.load()/agent-history JSONL/transcript load is added to menu/UI paths.
Cmux Cache Substitution Correctness	✅ Passed	Changed code only disables SwiftUI menu key equivalents for Cmd+1...9 and routes digits through AppDelegate; no persistence/history/snapshot cache swap or stale-cache risk.
Cmux No Hacky Sleeps	✅ Passed	PR only changes Swift sources, a pbxproj, and a Swift test; no non-Swift runtime code adds sleeps, timers, or polling.
Cmux Algorithmic Complexity	✅ Passed	Cmd+1…9 menu routing is fixed-size (1...9) and the mapper is O(1); no scalable rescans or repeated filters were introduced.
Cmux Swift Concurrency	✅ Passed	The new shortcut-routing code and regression test are synchronous; no new DispatchQueue, Task, Combine, or completion-handler patterns were added in the changed hunks.
Cmux Swift @Concurrent	✅ Passed	Changed Swift code is synchronous UI/menu routing and a main-actor test; no added/changed @concurrent or nonisolated async work, so the rule isn’t violated.
Cmux Swift File And Package Boundaries	✅ Passed	Focused app/UI glue in existing oversized executable code plus a small test; no new mixed-responsibility file or misplaced package logic.
Cmux Swiftpm Lockfiles	✅ Passed	Diff only adds test file wiring in cmux.xcodeproj; no Package.resolved, .gitignore, workflow, or SwiftPM package-reference pin changes were present.
Cmux Swift Logging	✅ Passed	PASS: The changed hunks add no print/debugPrint/dump/NSLog/Logger calls; the only NSLog in cmuxApp.swift is preexisting and outside the diff.
Cmux User-Facing Error Privacy	✅ Passed	PR only changes shortcut routing and adds a test; no user-facing errors, alerts, or command output were added or modified.
Cmux Full Internationalization	✅ Passed	The PR only changes shortcut-routing logic, comments, test code, and Xcode wiring; no new user-facing production text or locale assets were added.
Cmux Swiftui State Layout	✅ Passed	Changed code only reroutes Cmd+1…9 menu shortcuts and adds a test; no new ObservableObject, GeometryReader, lazy-row store refs, or render-time state writes.
Cmux Architecture Rethink	✅ Passed	The PR removes the SwiftUI key equivalent and keeps Cmd+1…9 routed solely through the existing AppDelegate shortcut handler; no timing, observers, or split lifecycle ownership were added.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PR only changes keyboard shortcut routing and a test; no NSWindow/NSPanel/WindowGroup code or cmuxAuxiliaryWindowIdentifiers changes, so the rule doesn’t apply.
Cmux Source Artifacts	✅ Passed	Changed paths are hand-written source/test/project files; no logs, temp dirs, caches, build output, or other artifact paths appear.
Cmux No Test Or Debug Seam In Production Source	✅ Passed	The PR only unbounds menuShortcut(.selectWorkspaceByNumber) in production; the regression test lives under Tests/, and no new debug/test seam was added in Sources/.
Cmux No Ambient Global State	✅ Passed	The PR only alters existing methods on KeyboardShortcutSettings/cmuxApp and adds a test; no new file-scope funcs, globals, or singletons appear.
Title check	✅ Passed	The title clearly summarizes the main change: fixing workspace digit shortcut routing.
Description check	✅ Passed	The description matches the required template and includes summary, testing, demo-video note, review trigger, and checklist.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-5588-selectworkspacebynumber-default-cmd-1-9-silen

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR fixes workspace switching regressions (#5588) by making menuShortcut(for: .selectWorkspaceByNumber) unconditionally return .unbound, preventing SwiftUI menu key equivalents from silently claiming Cmd+1…9 after focus churn. The AppDelegate shortcut dispatcher remains the sole owner of the digit family.

• KeyboardShortcutSettingsLookup.swift: Added a dedicated case .selectWorkspaceByNumber that short-circuits to .unbound before the default return shortcut path, establishing the invariant in one authoritative place.
• cmuxApp.swift: Removed the now-dead if/else branch that conditionally attached .keyboardShortcut to Workspace 1…9 menu items; the ForEach now always renders plain Button elements with no key equivalent.
• WorkspaceNumberShortcutMenuRoutingTests.swift: Regression test locked in via the cmuxTests target asserting the dispatcher retains the default shortcut while the menu remains unbound.

Confidence Score: 5/5

Safe to merge — the change narrows shortcut ownership to a single well-tested path with no new risk surface.

The fix is tightly scoped: one new case in menuShortcut(for:) returns .unbound, the corresponding dead branch in cmuxApp.swift is cleaned up, and a regression test locks in both invariants. No new concurrency, global state, or test seams are introduced. The dead else branch flagged by the previous review is fully removed in this version.

No files require special attention.

Important Files Changed

Filename	Overview
Sources/KeyboardShortcutSettingsLookup.swift	Added .selectWorkspaceByNumber case that unconditionally returns .unbound from menuShortcut(for:), ensuring Cmd+1…9 is never owned by SwiftUI menu key equivalents.
Sources/cmuxApp.swift	Removed the if/else branch in the Workspace 1…9 ForEach that conditionally attached .keyboardShortcut; now always renders plain Button with no menu key equivalent, consistent with the new .unbound guarantee in menuShortcut.
cmuxTests/WorkspaceNumberShortcutMenuRoutingTests.swift	New regression test asserting shortcut(for: .selectWorkspaceByNumber) equals the default (dispatcher still owns it) and menuShortcut(for: .selectWorkspaceByNumber) is unbound; lives in the test target, uses only pre-existing test helpers.
cmux.xcodeproj/project.pbxproj	Wires WorkspaceNumberShortcutMenuRoutingTests.swift into both the file references and the cmuxTests build phase; IDs are consistent between the two additions.
.github/swift-file-length-budget.tsv	Updates line-count budgets for cmuxApp.swift (4483→4467, matches ~16 lines removed) and GhosttySurfaceView.swift; the latter change reflects a pre-existing reduction unrelated to this PR's diff.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User as User (Cmd+1…9)
    participant SwiftUIMenu as SwiftUI Menu
    participant AppDelegate as AppDelegate Dispatcher
    participant TabManager as TabManager

    Note over SwiftUIMenu: Before fix: could silently consume Cmd+1…9 after focus churn without invoking command closure

    Note over SwiftUIMenu: After fix: menuShortcut(for: .selectWorkspaceByNumber) returns .unbound — no menu key equivalent registered

    User->>AppDelegate: Cmd+digit keyDown
    AppDelegate->>AppDelegate: shortcut(for: .selectWorkspaceByNumber) returns default digit shortcut
    AppDelegate->>TabManager: selectWorkspaceByNumber(digit)
    TabManager-->>User: workspace switched

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"}}}%%
sequenceDiagram
    participant User as User (Cmd+1…9)
    participant SwiftUIMenu as SwiftUI Menu
    participant AppDelegate as AppDelegate Dispatcher
    participant TabManager as TabManager

    Note over SwiftUIMenu: Before fix: could silently consume Cmd+1…9 after focus churn without invoking command closure

    Note over SwiftUIMenu: After fix: menuShortcut(for: .selectWorkspaceByNumber) returns .unbound — no menu key equivalent registered

    User->>AppDelegate: Cmd+digit keyDown
    AppDelegate->>AppDelegate: shortcut(for: .selectWorkspaceByNumber) returns default digit shortcut
    AppDelegate->>TabManager: selectWorkspaceByNumber(digit)
    TabManager-->>User: workspace switched

Loading

_{Reviews (7): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile}