Fix remapped Cmd+W close shortcuts

[austinywang]

Fixes #1488

Summary

• Treat stale default close shortcuts as stale even when their key has been reassigned to another configured action.
• Route configured shortcut handling before stale close menu fallback at the application and window key-equivalent boundaries.
• Add regression coverage for Close Tab, Close Workspace, and Close Window defaults, plus Cmd+W reassignment before a stale Close Tab menu item.

Reproduction / evidence

• Reviewed issue Cmd+W shortcut cannot be reconfigured in Settings — hardcoded to close tab while all other shortcuts are configurable #1488, umbrella All keyboard shortcuts should be remappable in Settings (no hardcoded combos) #3726, and stale PRs Fix #1488: make close-tab shortcut configurable #1490 and Fix close-tab shortcut remapping #2512.
• Attempted local reproduction against an already-running tagged debug app without launching or rebuilding; that build already suppressed the simple remap-away case, so the regression targets the remaining split-owner fallback path where stale close defaults could still win after reassignment.

Test structure

• Commit 1 adds the regression tests only.
• Commit 2 adds the fix.

Testing

• CI: all required checks green on 2026-05-20.
• Tagged dev build: CMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag issue-1488-cmd-w-remap-broken --launch built successfully locally.
• Launch: reload.sh first socket check timed out, then the built tagged app was opened directly; /tmp/cmux-debug-issue-1488-cmd-w-remap-broken.sock was created and the app stayed running.

---

^{Need help on this PR? Tag @codesmith with what you need.}

• Let Codesmith autofix CI failures and bot reviews

---

Note

Medium Risk
Changes event/shortcut routing for Command-key equivalents (including Cmd+W) and stale-menu suppression logic, which can subtly impact global keyboard handling across windows. Added regression tests reduce risk but behavior changes may affect edge-case shortcuts and AppKit menu interactions.

Overview
Fixes cases where remapped close shortcuts (e.g., Cmd+W) could still trigger stale AppKit menu items.

Configured shortcut handling is now dispatched earlier (via new handleConfiguredShortcutKeyEquivalent) from both NSApplication.sendEvent and window key-equivalent paths, before any stale menu fallback runs.

Refines shouldSuppressStaleCmuxMenuShortcut to (1) treat default close shortcuts as stale even when their key is reassigned, and (2) avoid suppressing events owned by current numbered-digit shortcuts. Adds regression tests covering these scenarios, including an end-to-end Cmd+W reassignment against a mocked stale Close Tab menu item.

^{Reviewed by Cursor Bugbot for commit e9e396c. Bugbot is set up for automated code reviews on this repo. Configure here.}

---

Summary by cubic

Fixes #1488: remapped Cmd+W and other Close shortcuts no longer lose to stale menu items, and numbered-digit shortcuts (e.g., Cmd+2) keep working. Configured shortcuts now run before any menu fallback; Close defaults are considered stale once reassigned.

• Bug Fixes
  • Route configured shortcuts before menu fallbacks in NSApplication.sendEvent and NSWindow.performKeyEquivalent via a shared handleConfiguredShortcutKeyEquivalent.
  • Treat .closeTab, .closeWorkspace, and .closeWindow defaults as stale after reassignment; do not suppress current numbered-digit shortcuts.
  • Add isolated regression tests: reassigned Cmd+W beats a stale Close Tab menu via sendEvent; stale Close defaults; numbered-digit shortcuts.

^{Written for commit e9e396c. Summary will update on new commits. Review in cubic}

Summary by CodeRabbit

• Bug Fixes

  • Configured keyboard shortcuts are now routed before stale menu fallbacks across application and window event paths.
  • Stale menu-shortcut suppression refined with explicit handling for close-related actions and more accurate suppression decisions.

• Tests

  • Added regression tests covering reassigned and temporarily remapped shortcuts, ensuring correct routing and suppression behavior for close and numbered-digit actions.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	May 20, 2026 1:34am
cmux-staging	Building	Preview, Comment	May 20, 2026 1:34am

[coderabbitai]

📝 Walkthrough

Walkthrough

Centralizes configured-shortcut dispatch and checks configured shortcuts before suppressing stale AppKit menu-backed shortcuts; refactors stale-suppression logic; updates application and window event overrides to consult the new routing; adds regression tests for close-related and numbered-digit shortcut reassignment.

Changes

Keyboard Shortcut Priority Routing

Layer / File(s)	Summary
Testing swizzles initialization Sources/AppDelegate.swift	installWindowResponderSwizzlesForTesting() initializes the application-level send-event swizzle for tests.
Configured shortcut routing helper Sources/AppDelegate.swift	handleBrowserSurfaceKeyEquivalent(_:) now delegates to handleConfiguredShortcutKeyEquivalent(_:), which centralizes configured-shortcut dispatch and forwards to handleCustomShortcut(_:) for keyDown behavior.
Stale menu shortcut suppression refactor Sources/AppDelegate.swift	shouldSuppressStaleCmuxMenuShortcut(_:) now computes menu-backed actions with matching default shortcuts, disables suppression when current shortcuts match, immediately suppresses for close-related defaults, otherwise suppresses only if no current shortcut matches.
AppKit event routing integration Sources/AppDelegate.swift	NSApplication.cmux_applicationSendEvent(_:) and NSWindow.cmux_performKeyEquivalent(with:) call handleConfiguredShortcutKeyEquivalent(_:) first and return early when it handles the event (before stale-suppression or menu fallback logic).
Tests and assertion helper cmuxTests/AppDelegateShortcutRoutingTests.swift	Adds tests: testCurrentNumberedDigitShortcutIsNotSuppressedAsStaleMenuShortcut, testStaleCloseDefaultShortcutsSuppressMenuFallbackAfterReassignment, and testApplicationSendEventRoutesReassignedCmdWBeforeStaleCloseTabMenuEquivalent; adds assertStaleCloseDefaultShortcutSuppressesMenuFallback(...) helper.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• manaflow-ai/cmux#4268: Modifies Sources/AppDelegate.swift key-equivalent routing and IME/field-editor forwarding; related to shortcut-routing control flow.

Poem

🐰 I hopped through keys where shortcuts hide,
Cmd+W now listens to what users decide.
Stale menus hush, configured commands sing,
A nimble rabbit nudged the routing string. 🥕✨

🚥 Pre-merge checks | ✅ 16 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (16 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix remapped Cmd+W close shortcuts' clearly and concisely summarizes the main change: addressing issues with remapped Cmd+W and close shortcuts being suppressed by stale menu items.
Linked Issues check	✅ Passed	The PR adequately addresses issue #1488 requirements: configured shortcuts now route before stale menu suppression, Close shortcuts are treated as stale when reassigned, and regression tests cover the specified scenarios.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing issue #1488: AppDelegate shortcut routing, stale menu suppression logic, and corresponding regression tests. No unrelated modifications detected.
Cmux Swift Actor Isolation	✅ Passed	AppDelegate is @MainActor; new handleConfiguredShortcutKeyEquivalent inherits proper actor isolation. Takes MainActor-safe parameters. No isolation debt introduced or worsened.
Cmux Swift Blocking Runtime	✅ Passed	All new production code is purely synchronous with no blocking or timing-based primitives. Test code uses only standard XCTest patterns, which are allowed.
Cmux No Hacky Sleeps	✅ Passed	Code changes are Swift-only. Rule explicitly excludes Swift, covering it separately. Check applies only to TypeScript, JavaScript, shell, and non-Swift runtime scripts.
Cmux Swift Concurrency	✅ Passed	New code adds synchronous event routing with no async/await, Task, DispatchQueue, Combine, or completion-handler patterns. Functions route shortcuts at AppKit boundaries, an allowed use case.
Cmux Swift @Concurrent	✅ Passed	New synchronous method is properly contained in @MainActor class, calls only synchronous functions. No async work, missing @concurrent, or isolation violations.
Cmux Swift File And Package Boundaries	✅ Passed	Adds 56 lines to keyboard routing in AppDelegate, under 250-line threshold. AppDelegate is allowed (app delegate/UI glue). Focused bug fix preserving extraction path.
Cmux Swift Logging	✅ Passed	All logging added by this PR uses DEBUG-guarded cmuxDebugLog calls with no secrets/personal data, no print/NSLog/debugPrint/dump in modified code, and no ad-hoc file logging.
Cmux User-Facing Error Privacy	✅ Passed	PR changes contain no user-facing error messages, sensitive data, or privacy violations. Debug logs are properly wrapped in #if DEBUG blocks (developer-only).
Cmux Full Internationalization	✅ Passed	Production code adds methods with only logic and no new user-facing strings. Existing strings use String(localized:defaultValue:). Test file is exempt from i18n requirements.
Cmux Swiftui State Layout	✅ Passed	AppKit keyboard event routing PR with no SwiftUI state patterns: no new @Published/@Observable/@StateObject/@EnvironmentObject/GeometryReader or lazy list store references.
Cmux Architecture Rethink	✅ Passed	Changes follow allowed patterns: small correctness fix with clear ownership, platform boundary integration, no timing anti-patterns, no state duplication or split UI lifecycle issues.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PR only modifies keyboard shortcut routing and stale menu suppression logic. No new user-visible windows (NSWindow, NSPanel, NSWindowController, SwiftUI Window, WindowGroup) are created or registered.
Description check	✅ Passed	The PR description provides a clear summary of changes, references the fixed issue, includes reproduction evidence, testing approach, and commit structure.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-1488-cmd-w-remap-broken

---

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 and usage tips.}

[greptile-apps]

Greptile Summary

This PR fixes the case where stale AppKit menu shortcuts for close actions (.closeTab, .closeWorkspace, .closeWindow) could still fire after a user remapped their key equivalents to another configured action. The core fix is a two-part change: shouldSuppressStaleCmuxMenuShortcut now treats any close-type action as always stale once its default shortcut key has been reassigned, and both the NSApplication.sendEvent and NSWindow.performKeyEquivalent swizzles now route the event through handleConfiguredShortcutKeyEquivalent before falling back to terminal forwarding when suppression is active.

• shouldSuppressStaleCmuxMenuShortcut refactor: Builds staleDefaultActions upfront (menu-backed actions whose default key matches), returns false if any stale action still owns the key via its current shortcut, then short-circuits with true for close-type stale actions before the general "does any action own this key" check that applies to non-close actions.
• handleConfiguredShortcutKeyEquivalent extraction: Extracts a shared entry-point for configured shortcut dispatch (wrapping handleCustomShortcut) that is now called both from handleBrowserSurfaceKeyEquivalent and from the two swizzled event-handling sites, ensuring a reassigned action fires before any stale menu item can win.
• Regression tests: Covers numbered-digit shortcut non-suppression, stale close-default suppression for all three close actions, and a full NSApp.sendEvent integration test verifying sidebar toggle fires instead of the stale Close Tab menu item after Cmd+W is reassigned.

Confidence Score: 5/5

Safe to merge. The changes are tightly scoped to the stale-menu suppression and event-routing path, the new logic correctly identifies close-type stale actions and routes configured shortcuts before any menu fallback, and the regression tests confirm correct behavior for all three close defaults and the Cmd+W reassignment scenario.

The refactored shouldSuppressStaleCmuxMenuShortcut preserves all pre-existing non-close semantics exactly (numbered-digit matching, the all-actions current-shortcut guard), and the new close-action early-return is the minimal targeted fix for the described split-owner race. The handleConfiguredShortcutKeyEquivalent extraction is a transparent delegation with no behavior change for existing callers. No actor isolation issues, no blocking primitives, no user-facing text, and no production logging changes.

No files require special attention.

Important Files Changed

Filename	Overview
Sources/AppDelegate.swift	Refactors shouldSuppressStaleCmuxMenuShortcut to treat stale close defaults as always-stale after reassignment, adds handleConfiguredShortcutKeyEquivalent routing before the stale-menu fallback in both NSApplication.sendEvent and NSWindow.performKeyEquivalent swizzles; logic is correct.
cmuxTests/AppDelegateShortcutRoutingTests.swift	Adds three regression tests: numbered-digit current shortcut is not suppressed, stale close-default shortcuts suppress menu fallback for all three close actions, and a full integration test verifying reassigned Cmd+W fires the configured action instead of the stale Close Tab menu item.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[NSApp.sendEvent / NSWindow.performKeyEquivalent\nreceives keyDown event] --> B{shouldSuppressStaleCmuxMenuShortcut?}
    B -->|No| C[Normal AppKit dispatch\nMenu item fires normally]
    B -->|Yes| D{handleConfiguredShortcutKeyEquivalent\nhandleCustomShortcut}
    D -->|true - action fired| E[Return: event consumed\nConfigured shortcut ran]
    D -->|false - nothing matched| F[Forward to Ghostty view\nor suppress stale menu]
    subgraph shouldSuppressStaleCmuxMenuShortcut
        G[Build staleDefaultActions:\nmenu-backed actions whose\ndefault shortcut matches event] --> H{staleDefaultActions\nempty?}
        H -->|Yes| I[return false]
        H -->|No| J{Any stale action still\nowns the key via\ncurrent shortcut?}
        J -->|Yes| K[return false\nnot actually stale]
        J -->|No| L{staleDefaultActions\ncontains a close action?}
        L -->|Yes - CLOSE PATH| M[return true\nalways suppress stale close default]
        L -->|No - NON-CLOSE PATH| N{Any action currently\nowns this key?}
        N -->|Yes| O[return false\nconfigured shortcut handles it via local monitor]
        N -->|No| P[return true\nsuppress orphaned stale default]
    end

Loading

_{Reviews (3): Last reviewed commit: "test: keep cmd-w send-event regression i..." | Re-trigger Greptile}

[coderabbitai]

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 `@Sources/AppDelegate.swift`:
- Around line 13554-13560: The fallback loop that currently only calls
matchesKeyboardShortcutEvent(event, action: action, shortcut: currentShortcut)
can miss actions that use numbered-digit matching; update the loop in the
consume-after-chord-mismatch path (iterating
KeyboardShortcutSettings.Action.allCases and using
KeyboardShortcutSettings.shortcut(for:)) to also check for numbered-digit
matches by calling the existing numberedShortcutDigit(event: currentShortcut)
(or equivalent helper) when the action's usesNumberedDigitMatching is true, and
treat that as a match (i.e., return false to avoid suppression) just as
matchesKeyboardShortcutEvent does before finally returning true.

🪄 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: e4876a78-d1bc-40b0-96b7-135b3a7a448b

📥 Commits

Reviewing files that changed from the base of the PR and between c6db818 and f2b257f.

📒 Files selected for processing (2)

• Sources/AppDelegate.swift
• cmuxTests/AppDelegateShortcutRoutingTests.swift

[cubic-dev-ai]

No issues found across 2 files

_{Re-trigger cubic}

Dismissed stale CodeRabbit review: its only actionable inline thread was addressed in d198a96 and is resolved; CodeRabbit check is passing.