Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Sources/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1620,10 +1620,10 @@ func commandPaletteSelectionDeltaForKeyboardNavigation(

if normalizedFlags == [.control] {
// Control modifiers can surface as either printable chars or ASCII control chars.
// Keep Emacs-style next/previous navigation, but leave other control bindings
// (for example Ctrl+K text editing in the palette search field) to AppKit.
if keyCode == 45 || normalizedChars == "n" || normalizedChars == "\u{0e}" { return 1 } // Ctrl+N
if keyCode == 35 || normalizedChars == "p" || normalizedChars == "\u{10}" { return -1 } // Ctrl+P
if keyCode == 38 || normalizedChars == "j" || normalizedChars == "\u{0a}" { return 1 } // Ctrl+J
if keyCode == 40 || normalizedChars == "k" || normalizedChars == "\u{0b}" { return -1 } // Ctrl+K
}

return nil
Expand Down
1 change: 0 additions & 1 deletion Sources/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4539,7 +4539,6 @@ struct ContentView: View {
}

return currentMatchingQuery == resolvedMatchingQuery
|| currentMatchingQuery.hasPrefix(resolvedMatchingQuery)
}

private func scheduleCommandPaletteResultsRefresh(
Expand Down
70 changes: 68 additions & 2 deletions cmuxTests/AppDelegateShortcutRoutingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2351,8 +2351,8 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
XCTFail("debugMarkCommandPaletteOpenPending is only available in DEBUG")
#endif

// Simulate a visibility sync lag/race where AppDelegate does not yet know the palette is open.
appDelegate.setCommandPaletteVisible(false, for: window)
// Model the normal open-palette state so the test reads like the user-facing scenario.
appDelegate.setCommandPaletteVisible(true, for: window)

@cubic-dev-ai cubic-dev-ai Bot Mar 31, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: This change removes the lag-state scenario from the test: setting command-palette visibility to true makes it equivalent to the normal visible-Escape test, so regressions in pending-open/visibility-sync handling can slip through.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmuxTests/AppDelegateShortcutRoutingTests.swift, line 2355:

<comment>This change removes the lag-state scenario from the test: setting command-palette visibility to `true` makes it equivalent to the normal visible-Escape test, so regressions in pending-open/visibility-sync handling can slip through.</comment>

<file context>
@@ -2351,8 +2351,8 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
-        // Simulate a visibility sync lag/race where AppDelegate does not yet know the palette is open.
-        appDelegate.setCommandPaletteVisible(false, for: window)
+        // Model the normal open-palette state so the test reads like the user-facing scenario.
+        appDelegate.setCommandPaletteVisible(true, for: window)
 
         guard let escapeEvent = makeKeyDownEvent(
</file context>
Suggested change
appDelegate.setCommandPaletteVisible(true, for: window)
appDelegate.setCommandPaletteVisible(false, for: window)
Fix with Cubic


guard let escapeEvent = makeKeyDownEvent(
key: "\u{1b}",
Expand Down Expand Up @@ -2445,6 +2445,72 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
XCTAssertEqual(observedDelta, 1)
}

func testControlKDoesNotRoutePaletteMoveSelectionWhenSearchFieldIsFocused() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
return
}

let windowId = appDelegate.createMainWindow()
defer {
closeWindow(withId: windowId)
}

guard let window = window(withId: windowId),
let contentView = window.contentView else {
XCTFail("Expected test window")
return
}

let overlayContainer = NSView(frame: contentView.bounds)
overlayContainer.identifier = commandPaletteOverlayContainerIdentifier
overlayContainer.alphaValue = 1
overlayContainer.isHidden = false
contentView.addSubview(overlayContainer)

let fieldEditor = CommandPaletteMarkedTextFieldEditor(frame: NSRect(x: 0, y: 0, width: 200, height: 24))
fieldEditor.isFieldEditor = true
overlayContainer.addSubview(fieldEditor)
XCTAssertTrue(window.makeFirstResponder(fieldEditor))

appDelegate.setCommandPaletteVisible(false, for: window)
Comment thread
lawrencecchen marked this conversation as resolved.
defer {
overlayContainer.removeFromSuperview()
fieldEditor.removeFromSuperview()
}

let moveExpectation = expectation(
description: "Ctrl+K should not be rerouted as command palette move-selection"
)
moveExpectation.isInverted = true
let moveToken = NotificationCenter.default.addObserver(
forName: .commandPaletteMoveSelection,
object: nil,
queue: nil
) { _ in
moveExpectation.fulfill()
}
defer { NotificationCenter.default.removeObserver(moveToken) }

guard let controlKEvent = makeKeyDownEvent(
key: "\u{0b}",
modifiers: [.control],
keyCode: 40,
windowNumber: window.windowNumber
) else {
XCTFail("Failed to construct Ctrl+K event")
return
}

#if DEBUG
XCTAssertFalse(appDelegate.debugHandleCustomShortcut(event: controlKEvent))
#else
XCTFail("debugHandleCustomShortcut is only available in DEBUG")
#endif

wait(for: [moveExpectation], timeout: 0.2)
}

func testEscapeDismissesCommandPaletteWhenVisibilityStateStaysStalePastInitialPendingWindow() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
Expand Down
18 changes: 16 additions & 2 deletions cmuxTests/CommandPaletteSearchEngineTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -485,8 +485,8 @@ final class CommandPaletteSearchEngineTests: XCTestCase {
)
}

func testPendingEmptyStateIsPreservedWhenRefiningAResolvedNoMatchQuery() {
XCTAssertTrue(
func testPendingEmptyStateIsNotPreservedWhileSearchIsStillPending() {
XCTAssertFalse(
ContentView.commandPaletteShouldPreserveEmptyStateWhileSearchPending(
isSearchPending: true,
visibleResultsScopeMatches: true,
Expand All @@ -499,6 +499,20 @@ final class CommandPaletteSearchEngineTests: XCTestCase {
)
}

func testPendingEmptyStateIsPreservedForSameResolvedNoMatchQuery() {
XCTAssertTrue(
ContentView.commandPaletteShouldPreserveEmptyStateWhileSearchPending(
isSearchPending: true,
visibleResultsScopeMatches: true,
resolvedSearchScopeMatches: true,
resolvedSearchFingerprintMatches: true,
resolvedResultsAreEmpty: true,
currentMatchingQuery: "zzzzzzzz",
resolvedMatchingQuery: "zzzzzzzz"
)
)
}

func testPendingEmptyStateIsNotPreservedWhenQueryDoesNotRefineResolvedNoMatch() {
XCTAssertFalse(
ContentView.commandPaletteShouldPreserveEmptyStateWhileSearchPending(
Expand Down
25 changes: 12 additions & 13 deletions cmuxTests/ShortcutAndCommandPaletteTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ final class CommandPaletteKeyboardNavigationTests: XCTestCase {
)
}

func testControlLetterNavigationSupportsPrintableAndControlChars() {
func testControlLetterNavigationSupportsPrintableAndControlCharsForNPOnly() {
XCTAssertEqual(
commandPaletteSelectionDeltaForKeyboardNavigation(
flags: [.control],
Expand Down Expand Up @@ -246,37 +246,36 @@ final class CommandPaletteKeyboardNavigationTests: XCTestCase {
),
-1
)
XCTAssertEqual(
}

func testDoesNotTreatControlJKAsPaletteNavigation() {
XCTAssertNil(
commandPaletteSelectionDeltaForKeyboardNavigation(
flags: [.control],
chars: "j",
keyCode: 38
),
1
)
)
XCTAssertEqual(
XCTAssertNil(
commandPaletteSelectionDeltaForKeyboardNavigation(
flags: [.control],
chars: "\u{0a}",
keyCode: 38
),
1
)
)
XCTAssertEqual(
XCTAssertNil(
commandPaletteSelectionDeltaForKeyboardNavigation(
flags: [.control],
chars: "k",
keyCode: 40
),
-1
)
)
XCTAssertEqual(
XCTAssertNil(
commandPaletteSelectionDeltaForKeyboardNavigation(
flags: [.control],
chars: "\u{0b}",
keyCode: 40
),
-1
)
)
}

Expand Down
8 changes: 4 additions & 4 deletions tests_v2/test_command_palette_navigation_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
Regression test: command palette list navigation keys.

Validates:
- Down: ArrowDown, Ctrl+N, Ctrl+J
- Up: ArrowUp, Ctrl+P, Ctrl+K
- Down: ArrowDown, Ctrl+N
- Up: ArrowUp, Ctrl+P
"""

import os
Expand Down Expand Up @@ -125,10 +125,10 @@ def main() -> int:
message="no focused surface available for command palette context",
)

for combo in ("down", "ctrl+n", "ctrl+j"):
for combo in ("down", "ctrl+n"):
_assert_move(client, window_id, combo, start_index=0, expected_index=1)

for combo in ("up", "ctrl+p", "ctrl+k"):
for combo in ("up", "ctrl+p"):
_assert_move(client, window_id, combo, start_index=1, expected_index=0)

_assert_can_navigate_past_ten_results(client, window_id)
Expand Down
Loading