From 710ed9b068f336432497aae5f2c16d0aa052d736 Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Mon, 23 Feb 2026 20:08:21 -0800 Subject: [PATCH] Fix Cmd+P/Cmd+Shift+P window routing (#413) * Fix command palette shortcuts to stay window-scoped * Fix cross-window command palette typing focus lock --- Sources/AppDelegate.swift | 22 +++- Sources/ContentView.swift | 56 +++++++- tests_v2/test_command_palette_window_scope.py | 124 +++++++++++++++++- 3 files changed, 191 insertions(+), 11 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index cc798ff7..f3c5e73d 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -2389,6 +2389,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent return nil } + private func commandPaletteWindowForShortcutEvent(_ event: NSEvent) -> NSWindow? { + if let scopedWindow = mainWindowForShortcutEvent(event) { + return scopedWindow + } + return activeCommandPaletteWindow() + } + private func contextForMainWindow(_ window: NSWindow?) -> MainWindowContext? { guard let window, isMainTerminalWindow(window) else { return nil } return mainWindowContexts[ObjectIdentifier(window)] @@ -4097,13 +4104,18 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } let normalizedFlags = flags.subtracting([.numericPad, .function, .capsLock]) + let commandPaletteTargetWindow = commandPaletteWindowForShortcutEvent(event) + let commandPaletteVisibleInTargetWindow = commandPaletteTargetWindow.map { + isCommandPaletteVisible(for: $0) + } ?? false if let delta = commandPaletteSelectionDeltaForKeyboardNavigation( flags: event.modifierFlags, chars: chars, keyCode: event.keyCode ), - let paletteWindow = activeCommandPaletteWindow() { + commandPaletteVisibleInTargetWindow, + let paletteWindow = commandPaletteTargetWindow { NotificationCenter.default.post( name: .commandPaletteMoveSelection, object: paletteWindow, @@ -4114,20 +4126,20 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent let isCommandP = normalizedFlags == [.command] && (chars == "p" || event.keyCode == 35) if isCommandP { - let targetWindow = activeCommandPaletteWindow() ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow + let targetWindow = commandPaletteTargetWindow ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow NotificationCenter.default.post(name: .commandPaletteSwitcherRequested, object: targetWindow) return true } let isCommandShiftP = normalizedFlags == [.command, .shift] && (chars == "p" || event.keyCode == 35) if isCommandShiftP { - let targetWindow = activeCommandPaletteWindow() ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow + let targetWindow = commandPaletteTargetWindow ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow NotificationCenter.default.post(name: .commandPaletteRequested, object: targetWindow) return true } if shouldConsumeShortcutWhileCommandPaletteVisible( - isCommandPaletteVisible: activeCommandPaletteWindow() != nil, + isCommandPaletteVisible: commandPaletteVisibleInTargetWindow, normalizedFlags: normalizedFlags, chars: chars, keyCode: event.keyCode @@ -4354,7 +4366,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent if tabManager?.focusedBrowserPanel != nil { return false } - let targetWindow = activeCommandPaletteWindow() ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow + let targetWindow = commandPaletteTargetWindow ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow NotificationCenter.default.post(name: .commandPaletteRenameTabRequested, object: targetWindow) return true } diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 79f17cda..d2010f74 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -760,6 +760,9 @@ private final class WindowCommandPaletteOverlayController: NSObject { private weak var installedThemeFrame: NSView? private var focusLockTimer: DispatchSourceTimer? private var scheduledFocusWorkItem: DispatchWorkItem? + private var isPaletteVisible = false + private var windowDidBecomeKeyObserver: NSObjectProtocol? + private var windowDidResignKeyObserver: NSObjectProtocol? init(window: NSWindow) { self.window = window @@ -782,6 +785,7 @@ private final class WindowCommandPaletteOverlayController: NSObject { hostingView.trailingAnchor.constraint(equalTo: containerView.trailingAnchor), ]) _ = ensureInstalled() + installWindowKeyObservers() } @discardableResult @@ -913,6 +917,52 @@ private final class WindowCommandPaletteOverlayController: NSObject { } } + private func installWindowKeyObservers() { + guard let window else { return } + windowDidBecomeKeyObserver = NotificationCenter.default.addObserver( + forName: NSWindow.didBecomeKeyNotification, + object: window, + queue: .main + ) { [weak self] _ in + Task { @MainActor [weak self] in + self?.updateFocusLockForWindowState() + } + } + windowDidResignKeyObserver = NotificationCenter.default.addObserver( + forName: NSWindow.didResignKeyNotification, + object: window, + queue: .main + ) { [weak self] _ in + Task { @MainActor [weak self] in + self?.updateFocusLockForWindowState() + } + } + } + + private func updateFocusLockForWindowState() { + guard let window else { + stopFocusLockTimer() + return + } + guard isPaletteVisible else { + stopFocusLockTimer() + return + } + + guard window.isKeyWindow else { + stopFocusLockTimer() + if isPaletteResponder(window.firstResponder) { + _ = window.makeFirstResponder(nil) + } + return + } + + startFocusLockTimer() + if !isPaletteTextInputFirstResponder(window.firstResponder) { + scheduleFocusIntoPalette(retries: 8) + } + } + private func startFocusLockTimer() { guard focusLockTimer == nil else { return } let timer = DispatchSource.makeTimerSource(queue: .main) @@ -959,6 +1009,7 @@ private final class WindowCommandPaletteOverlayController: NSObject { func update(rootView: AnyView, isVisible: Bool) { guard ensureInstalled() else { return } + isPaletteVisible = isVisible if isVisible { hostingView.rootView = rootView containerView.capturesMouseEvents = true @@ -967,10 +1018,7 @@ private final class WindowCommandPaletteOverlayController: NSObject { if let themeFrame = installedThemeFrame, themeFrame.subviews.last !== containerView { themeFrame.addSubview(containerView, positioned: .above, relativeTo: nil) } - startFocusLockTimer() - if let window, !isPaletteTextInputFirstResponder(window.firstResponder) { - scheduleFocusIntoPalette(retries: 8) - } + updateFocusLockForWindowState() } else { stopFocusLockTimer() if let window, isPaletteResponder(window.firstResponder) { diff --git a/tests_v2/test_command_palette_window_scope.py b/tests_v2/test_command_palette_window_scope.py index 63236c34..e6cfeab7 100644 --- a/tests_v2/test_command_palette_window_scope.py +++ b/tests_v2/test_command_palette_window_scope.py @@ -32,6 +32,10 @@ def _palette_visible(client: cmux, window_id: str) -> bool: return bool(res.get("visible")) +def _palette_results(client: cmux, window_id: str, limit: int = 20) -> dict: + return client.command_palette_results(window_id=window_id, limit=limit) + + def _set_palette_visible(client: cmux, window_id: str, visible: bool) -> None: if _palette_visible(client, window_id) == visible: return @@ -43,6 +47,116 @@ def _set_palette_visible(client: cmux, window_id: str, visible: bool) -> None: ) +def _focus_window(client: cmux, window_id: str) -> None: + client.focus_window(window_id) + client.activate_app() + _wait_until( + lambda: client.current_window().lower() == window_id.lower(), + timeout_s=3.0, + message=f"failed to focus window {window_id}", + ) + time.sleep(0.15) + + +def _assert_shortcut_window_scoped(client: cmux, shortcut: str, w1: str, w2: str) -> None: + _set_palette_visible(client, w1, False) + _set_palette_visible(client, w2, False) + + _focus_window(client, w1) + client.simulate_shortcut(shortcut) + _wait_until( + lambda: _palette_visible(client, w1), + timeout_s=3.0, + message=f"{shortcut} did not open palette in window1", + ) + if _palette_visible(client, w2): + raise cmuxError(f"{shortcut} in window1 incorrectly opened palette in window2") + + _focus_window(client, w2) + client.simulate_shortcut(shortcut) + _wait_until( + lambda: _palette_visible(client, w2), + timeout_s=3.0, + message=f"{shortcut} did not open palette in window2", + ) + if not _palette_visible(client, w1): + raise cmuxError( + f"{shortcut} in window2 incorrectly toggled window1 palette off " + "(cross-window routing regression)" + ) + + client.simulate_shortcut(shortcut) + _wait_until( + lambda: not _palette_visible(client, w2), + timeout_s=3.0, + message=f"second {shortcut} did not close palette in window2", + ) + if not _palette_visible(client, w1): + raise cmuxError( + f"second {shortcut} in window2 incorrectly changed window1 palette visibility" + ) + + _focus_window(client, w1) + client.simulate_shortcut(shortcut) + _wait_until( + lambda: not _palette_visible(client, w1), + timeout_s=3.0, + message=f"second {shortcut} did not close palette in window1", + ) + + +def _assert_cross_window_typing_after_mixed_shortcuts(client: cmux, w1: str, w2: str) -> None: + _set_palette_visible(client, w1, False) + _set_palette_visible(client, w2, False) + + _focus_window(client, w1) + client.simulate_shortcut("cmd+shift+p") + _wait_until( + lambda: _palette_visible(client, w1), + timeout_s=3.0, + message="cmd+shift+p did not open palette in window1", + ) + _wait_until( + lambda: str(_palette_results(client, w1).get("mode") or "") == "commands", + timeout_s=3.0, + message="window1 palette did not enter commands mode", + ) + window1_query_before = str(_palette_results(client, w1).get("query") or "") + + _focus_window(client, w2) + client.simulate_shortcut("cmd+p") + _wait_until( + lambda: _palette_visible(client, w2), + timeout_s=3.0, + message="cmd+p did not open palette in window2", + ) + _wait_until( + lambda: str(_palette_results(client, w2).get("mode") or "") == "switcher", + timeout_s=3.0, + message="window2 palette did not enter switcher mode", + ) + + typed = "" + for ch in "crosswindow": + typed += ch + client.simulate_type(ch) + _wait_until( + lambda expected=typed: str(_palette_results(client, w2).get("query") or "").lower() == expected, + timeout_s=1.8, + message=( + "typing into window2 palette did not accumulate query text " + f"(expected {typed!r})" + ), + ) + + window1_query_now = str(_palette_results(client, w1).get("query") or "") + if window1_query_now != window1_query_before: + raise cmuxError( + "typing in window2 changed window1 command-palette query " + f"(before={window1_query_before!r}, now={window1_query_now!r})" + ) + + def main() -> int: with cmux(SOCKET_PATH) as client: client.activate_app() @@ -51,8 +165,8 @@ def main() -> int: w2 = client.new_window() time.sleep(0.25) - ws1 = client.new_workspace(window_id=w1) - ws2 = client.new_workspace(window_id=w2) + _ = client.new_workspace(window_id=w1) + _ = client.new_workspace(window_id=w2) time.sleep(0.25) _set_palette_visible(client, w1, False) _set_palette_visible(client, w2, False) @@ -91,6 +205,12 @@ def main() -> int: message="window2 command palette did not close", ) + # Reproduce keyboard-shortcut window-scoping path: + # opening from window2 must not jump back and toggle window1. + _assert_shortcut_window_scoped(client, "cmd+shift+p", w1, w2) + _assert_shortcut_window_scoped(client, "cmd+p", w1, w2) + _assert_cross_window_typing_after_mixed_shortcuts(client, w1, w2) + print("PASS: command palette is scoped to active window") return 0