From e30ad63d3aae3907345e30cb83140c2a1073d80e Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Tue, 19 May 2026 17:12:28 -0700 Subject: [PATCH 1/4] test: cover remapped close shortcut fallbacks --- .../AppDelegateShortcutRoutingTests.swift | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/cmuxTests/AppDelegateShortcutRoutingTests.swift b/cmuxTests/AppDelegateShortcutRoutingTests.swift index 206aa923a6be..bc2c9ebb31c6 100644 --- a/cmuxTests/AppDelegateShortcutRoutingTests.swift +++ b/cmuxTests/AppDelegateShortcutRoutingTests.swift @@ -5656,6 +5656,97 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { } } + func testStaleCloseDefaultShortcutsSuppressMenuFallbackAfterReassignment() { + assertStaleCloseDefaultShortcutSuppressesMenuFallback( + staleAction: .closeTab, + replacementAction: .newTab, + replacementShortcut: StoredShortcut(key: "w", command: true, shift: false, option: false, control: false), + remappedStaleShortcut: StoredShortcut(key: "w", command: true, shift: false, option: true, control: false) + ) + + assertStaleCloseDefaultShortcutSuppressesMenuFallback( + staleAction: .closeWorkspace, + replacementAction: .newWindow, + replacementShortcut: StoredShortcut(key: "w", command: true, shift: true, option: false, control: false), + remappedStaleShortcut: StoredShortcut(key: "w", command: true, shift: true, option: true, control: false) + ) + + assertStaleCloseDefaultShortcutSuppressesMenuFallback( + staleAction: .closeWindow, + replacementAction: .toggleFullScreen, + replacementShortcut: StoredShortcut(key: "w", command: true, shift: false, option: false, control: true), + remappedStaleShortcut: StoredShortcut(key: "w", command: true, shift: false, option: true, control: true) + ) + } + + func testApplicationSendEventRoutesReassignedCmdWBeforeStaleCloseTabMenuEquivalent() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + AppDelegate.installWindowResponderSwizzlesForTesting() + + let windowId = appDelegate.createMainWindow() + guard let window = appDelegate.windowForMainWindowId(windowId), + let manager = appDelegate.tabManagerFor(windowId: windowId) else { + closeWindow(withId: windowId) + XCTFail("Expected a main window context") + return + } + + let previousMainMenu = NSApp.mainMenu + let menuProbe = MenuActionProbe() + + defer { + NSApp.mainMenu = previousMainMenu + closeWindow(withId: windowId) + } + + let staleMenu = NSMenu(title: "Test") + let staleCloseItem = NSMenuItem( + title: "Close Tab", + action: #selector(MenuActionProbe.perform(_:)), + keyEquivalent: "w" + ) + staleCloseItem.keyEquivalentModifierMask = [.command] + staleCloseItem.target = menuProbe + staleMenu.addItem(staleCloseItem) + NSApp.mainMenu = staleMenu + + window.makeKeyAndOrderFront(nil) + window.displayIfNeeded() + + guard let event = makeKeyDownEvent( + key: "w", + modifiers: [.command], + keyCode: 13, + windowNumber: window.windowNumber + ) else { + XCTFail("Failed to construct Cmd+W event") + return + } + + let initialWorkspaceCount = manager.tabs.count + let remappedCloseTab = StoredShortcut(key: "w", command: true, shift: false, option: true, control: false) + let reassignedNewWorkspace = StoredShortcut(key: "w", command: true, shift: false, option: false, control: false) + + withTemporaryShortcut(action: .closeTab, shortcut: remappedCloseTab) { + withTemporaryShortcut(action: .newTab, shortcut: reassignedNewWorkspace) { + NSApp.sendEvent(event) + } + } + + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05)) + + XCTAssertEqual(menuProbe.callCount, 0, "A stale Cmd+W Close Tab menu item must not run after Cmd+W is reassigned") + XCTAssertEqual( + manager.tabs.count, + initialWorkspaceCount + 1, + "The action currently assigned to Cmd+W should run before stale Close Tab menu fallback" + ) + } + func testApplicationSendEventSuppressesRemappedCmdDStaleMenuShortcut() { let previousMainMenu = NSApp.mainMenu let probeWindow = NSWindow( @@ -6209,6 +6300,35 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { body() } + private func assertStaleCloseDefaultShortcutSuppressesMenuFallback( + staleAction: KeyboardShortcutSettings.Action, + replacementAction: KeyboardShortcutSettings.Action, + replacementShortcut: StoredShortcut, + remappedStaleShortcut: StoredShortcut, + file: StaticString = #filePath, + line: UInt = #line + ) { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared", file: file, line: line) + return + } + guard let event = makeKeyDownEvent(shortcut: replacementShortcut, windowNumber: 0) else { + XCTFail("Failed to construct reassigned close-default shortcut event", file: file, line: line) + return + } + + withTemporaryShortcut(action: staleAction, shortcut: remappedStaleShortcut) { + withTemporaryShortcut(action: replacementAction, shortcut: replacementShortcut) { + XCTAssertTrue( + appDelegate.shouldSuppressStaleCmuxMenuShortcut(event: event), + "\(staleAction.rawValue) should suppress its stale default menu fallback after that key is reassigned", + file: file, + line: line + ) + } + } + } + private func assertEscapeKeyUpIsConsumedAfterCommandPaletteOpenRequest( _ openRequest: (_ appDelegate: AppDelegate, _ window: NSWindow) -> Void, file: StaticString = #filePath, From f2b257fb452b59dc674da4cb91d9ea6e97538c88 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Tue, 19 May 2026 17:13:23 -0700 Subject: [PATCH 2/4] fix: route remapped close defaults before stale menus --- Sources/AppDelegate.swift | 50 +++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index c13c8a5e7010..233d2921e47f 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -10890,6 +10890,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent static func installWindowResponderSwizzlesForTesting() { _ = didInstallApplicationAccessibilitySwizzle _ = didInstallApplicationSendActionSwizzle + _ = didInstallApplicationSendEventSwizzle _ = didInstallWindowKeyEquivalentSwizzle _ = didInstallWindowFirstResponderSwizzle _ = didInstallWindowSendEventSwizzle @@ -13098,6 +13099,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent /// through the same app-level shortcut handler used by the local key monitor. @discardableResult func handleBrowserSurfaceKeyEquivalent(_ event: NSEvent) -> Bool { + handleConfiguredShortcutKeyEquivalent(event) + } + + /// Route AppKit key-equivalent fallbacks through the same configured shortcut + /// dispatcher as the local key monitor before any stale menu item can run. + @discardableResult + func handleConfiguredShortcutKeyEquivalent(_ event: NSEvent) -> Bool { handleCustomShortcut(event: event) } @@ -13526,25 +13534,45 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent .subtracting([.numericPad, .function, .capsLock]) guard flags.contains(.command) else { return false } - for action in KeyboardShortcutSettings.Action.allCases { + let staleDefaultActions = KeyboardShortcutSettings.Action.allCases.filter { action in + isMenuBackedShortcutAction(action) && + matchesKeyboardShortcutEvent(event, action: action, shortcut: action.defaultShortcut) + } + guard !staleDefaultActions.isEmpty else { return false } + + for action in staleDefaultActions { let currentShortcut = KeyboardShortcutSettings.shortcut(for: action) if matchesKeyboardShortcutEvent(event, action: action, shortcut: currentShortcut) { return false } } - for action in KeyboardShortcutSettings.Action.allCases where isMenuBackedShortcutAction(action) { - if matchesKeyboardShortcutEvent(event, action: action, shortcut: action.defaultShortcut) { - return true + if staleDefaultActions.contains(where: isCloseShortcutAction) { + return true + } + + for action in KeyboardShortcutSettings.Action.allCases { + let currentShortcut = KeyboardShortcutSettings.shortcut(for: action) + if matchesKeyboardShortcutEvent(event, action: action, shortcut: currentShortcut) { + return false } } - return false + return true } private func isMenuBackedShortcutAction(_ action: KeyboardShortcutSettings.Action) -> Bool { action != .showHideAllWindows && action != .globalSearch } + private func isCloseShortcutAction(_ action: KeyboardShortcutSettings.Action) -> Bool { + switch action { + case .closeTab, .closeWorkspace, .closeWindow: + return true + default: + return false + } + } + private func numberedShortcutDigit(event: NSEvent, stroke: ShortcutStroke) -> Int? { let flags = event.modifierFlags.intersection(.deviceIndependentFlagsMask) .subtracting([.numericPad, .function, .capsLock]) @@ -14734,6 +14762,12 @@ private extension NSApplication { return } if AppDelegate.shared?.shouldSuppressStaleCmuxMenuShortcut(event: event) == true { + if AppDelegate.shared?.handleConfiguredShortcutKeyEquivalent(event) == true { +#if DEBUG + cmuxDebugLog("app.sendEvent routed configured shortcut before stale cmux menu shortcut") +#endif + return + } let responder = event.window?.firstResponder ?? keyWindow?.firstResponder ?? mainWindow?.firstResponder @@ -15284,6 +15318,12 @@ private extension NSWindow { return true } if AppDelegate.shared?.shouldSuppressStaleCmuxMenuShortcut(event: event) == true { + if AppDelegate.shared?.handleConfiguredShortcutKeyEquivalent(event) == true { +#if DEBUG + cmuxDebugLog(" → consumed by configured shortcut before stale cmux menu shortcut") +#endif + return true + } if let firstResponderGhosttyView { firstResponderGhosttyView.keyDown(with: event) #if DEBUG From d198a96266990062d01a6d968158e64d773bd6a2 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Tue, 19 May 2026 17:30:42 -0700 Subject: [PATCH 3/4] fix: preserve numbered shortcut stale-menu routing --- Sources/AppDelegate.swift | 17 ++++++-- .../AppDelegateShortcutRoutingTests.swift | 41 +++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 233d2921e47f..563978e7e645 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -13541,8 +13541,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent guard !staleDefaultActions.isEmpty else { return false } for action in staleDefaultActions { - let currentShortcut = KeyboardShortcutSettings.shortcut(for: action) - if matchesKeyboardShortcutEvent(event, action: action, shortcut: currentShortcut) { + if currentShortcutMatchesKeyboardShortcutEvent(event, action: action) { return false } } @@ -13552,14 +13551,24 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } for action in KeyboardShortcutSettings.Action.allCases { - let currentShortcut = KeyboardShortcutSettings.shortcut(for: action) - if matchesKeyboardShortcutEvent(event, action: action, shortcut: currentShortcut) { + if currentShortcutMatchesKeyboardShortcutEvent(event, action: action) { return false } } return true } + private func currentShortcutMatchesKeyboardShortcutEvent( + _ event: NSEvent, + action: KeyboardShortcutSettings.Action + ) -> Bool { + let currentShortcut = KeyboardShortcutSettings.shortcut(for: action) + if action.usesNumberedDigitMatching { + return numberedShortcutDigit(event: event, shortcut: currentShortcut) != nil + } + return matchesKeyboardShortcutEvent(event, action: action, shortcut: currentShortcut) + } + private func isMenuBackedShortcutAction(_ action: KeyboardShortcutSettings.Action) -> Bool { action != .showHideAllWindows && action != .globalSearch } diff --git a/cmuxTests/AppDelegateShortcutRoutingTests.swift b/cmuxTests/AppDelegateShortcutRoutingTests.swift index bc2c9ebb31c6..0b2c9a27d0b0 100644 --- a/cmuxTests/AppDelegateShortcutRoutingTests.swift +++ b/cmuxTests/AppDelegateShortcutRoutingTests.swift @@ -5656,6 +5656,47 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { } } + func testCurrentNumberedDigitShortcutIsNotSuppressedAsStaleMenuShortcut() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + guard let event = makeKeyDownEvent( + key: "2", + modifiers: [.command], + keyCode: 19, + windowNumber: 0 + ) else { + XCTFail("Failed to construct Cmd+2 event") + return + } + + let remappedWorkspaceNumber = StoredShortcut( + key: "1", + command: false, + shift: false, + option: false, + control: true + ) + let currentSurfaceNumber = StoredShortcut( + key: "1", + command: true, + shift: false, + option: false, + control: false + ) + + withTemporaryShortcut(action: .selectWorkspaceByNumber, shortcut: remappedWorkspaceNumber) { + withTemporaryShortcut(action: .selectSurfaceByNumber, shortcut: currentSurfaceNumber) { + XCTAssertFalse( + appDelegate.shouldSuppressStaleCmuxMenuShortcut(event: event), + "A current numbered-digit shortcut must own Cmd+2 before stale menu suppression" + ) + } + } + } + func testStaleCloseDefaultShortcutsSuppressMenuFallbackAfterReassignment() { assertStaleCloseDefaultShortcutSuppressesMenuFallback( staleAction: .closeTab, From e9e396cc6121284243c71528f801043b80e88e12 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Tue, 19 May 2026 18:25:26 -0700 Subject: [PATCH 4/4] test: keep cmd-w send-event regression isolated --- cmuxTests/AppDelegateShortcutRoutingTests.swift | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/cmuxTests/AppDelegateShortcutRoutingTests.swift b/cmuxTests/AppDelegateShortcutRoutingTests.swift index 0b2c9a27d0b0..3920e8074320 100644 --- a/cmuxTests/AppDelegateShortcutRoutingTests.swift +++ b/cmuxTests/AppDelegateShortcutRoutingTests.swift @@ -5730,7 +5730,8 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { let windowId = appDelegate.createMainWindow() guard let window = appDelegate.windowForMainWindowId(windowId), - let manager = appDelegate.tabManagerFor(windowId: windowId) else { + let manager = appDelegate.tabManagerFor(windowId: windowId), + let initialSidebarVisible = appDelegate.sidebarVisibility(windowId: windowId) else { closeWindow(withId: windowId) XCTFail("Expected a main window context") return @@ -5770,10 +5771,10 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { let initialWorkspaceCount = manager.tabs.count let remappedCloseTab = StoredShortcut(key: "w", command: true, shift: false, option: true, control: false) - let reassignedNewWorkspace = StoredShortcut(key: "w", command: true, shift: false, option: false, control: false) + let reassignedSidebarToggle = StoredShortcut(key: "w", command: true, shift: false, option: false, control: false) withTemporaryShortcut(action: .closeTab, shortcut: remappedCloseTab) { - withTemporaryShortcut(action: .newTab, shortcut: reassignedNewWorkspace) { + withTemporaryShortcut(action: .toggleSidebar, shortcut: reassignedSidebarToggle) { NSApp.sendEvent(event) } } @@ -5783,7 +5784,12 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { XCTAssertEqual(menuProbe.callCount, 0, "A stale Cmd+W Close Tab menu item must not run after Cmd+W is reassigned") XCTAssertEqual( manager.tabs.count, - initialWorkspaceCount + 1, + initialWorkspaceCount, + "Plain Cmd+W must not close a tab after Close Tab is remapped away" + ) + XCTAssertEqual( + appDelegate.sidebarVisibility(windowId: windowId), + !initialSidebarVisible, "The action currently assigned to Cmd+W should run before stale Close Tab menu fallback" ) }