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
77 changes: 77 additions & 0 deletions tests/e2e_ui/sessions/test_command_palette.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
"""E2E: ⌘/Ctrl+K opens the command palette and jumps to a session.

Covers the command palette added in ``ap-web/src/shell/CommandPalette.tsx`` and
its global hotkey (``useCommandPaletteHotkey``, ⌘/Ctrl+K, bound in
``AppShell``). The palette lists sessions from the same server-search source as
the sidebar and navigates to the picked one.

The flow: open the palette from a focused composer (proving the window-level
hotkey fires regardless of focus, like the session-switch hotkey), then select
the *other* seeded session from the palette's list and assert the route changes
to it.

No LLM turn is needed — this is pure client-side keyboard + routing — so it
skips the nightly/real-agent markers the approval suites carry. Two runner-bound
sessions come from the ``seeded_session_pair`` fixture; both are recent and
non-archived, so both appear in the palette's default (empty-query) list.

Server-side search-query *filtering* is left to the Vitest unit tests
(``CommandPalette.test.tsx``): the server's search reindex is asynchronous (see
``useConversations.ts``), which would make a "type then expect filtered" e2e
assertion timing-dependent. Selecting from the listed sessions exercises the
same open → select → navigate path deterministically.
"""

from __future__ import annotations

import httpx
from playwright.sync_api import Page, expect

_COMPOSER = "Ask the agent anything…"


def _set_title(base_url: str, session_id: str, title: str) -> None:
"""Title a session via ``PATCH /v1/sessions/{id}`` so its row is legible."""
resp = httpx.patch(
f"{base_url}/v1/sessions/{session_id}",
json={"title": title},
timeout=10.0,
)
resp.raise_for_status()


def test_command_palette_opens_and_switches_session(
page: Page,
seeded_session_pair: tuple[str, str, str],
) -> None:
"""⌘/Ctrl+K opens the palette; picking session B navigates to it."""
base_url, session_a, session_b = seeded_session_pair
_set_title(base_url, session_a, "e2e-palette-a")
_set_title(base_url, session_b, "e2e-palette-b")

page.goto(f"{base_url}/c/{session_a}")

# Both sessions must be loaded so the palette's session list holds them.
expect(page.locator(f'a[href="/c/{session_a}"]')).to_be_visible(timeout=30_000)
expect(page.locator(f'a[href="/c/{session_b}"]')).to_be_visible()

# Focus the composer first — the hotkey is window-level and must fire even
# from a focused text field (same contract as the session-switch hotkey).
composer = page.get_by_placeholder(_COMPOSER)
expect(composer).to_be_visible()
composer.click()

# Open the palette. CI runs Linux chromium → Control; the hook also accepts
# Cmd via metaKey on macOS.
page.keyboard.press("Control+k")

dialog = page.get_by_role("dialog")
expect(dialog).to_be_visible(timeout=10_000)
expect(page.get_by_test_id("command-palette-input")).to_be_focused()

# Pick the other session from inside the palette and assert we navigate to it.
dialog.get_by_text("e2e-palette-b").click()

expect(page).to_have_url(f"{base_url}/c/{session_b}", timeout=10_000)
# The palette closes on select.
expect(page.get_by_test_id("command-palette-input")).to_have_count(0)
1 change: 1 addition & 0 deletions web/src/components/KeyboardShortcutsDialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe("KeyboardShortcutsDialog", () => {

expect(screen.getByText("Keyboard shortcuts")).toBeTruthy();
// General / In chats / Navigation / View / Slash commands — one each.
expect(screen.getByText("Open command palette")).toBeTruthy();
expect(screen.getByText("Show keyboard shortcuts")).toBeTruthy();
expect(screen.getByText("Send message")).toBeTruthy();
expect(screen.getByText("Recall previous prompt")).toBeTruthy();
Expand Down
5 changes: 4 additions & 1 deletion web/src/components/KeyboardShortcutsDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ interface ShortcutGroup {
const SHORTCUT_GROUPS: ShortcutGroup[] = [
{
title: "General",
items: [{ label: "Show keyboard shortcuts", keys: [MOD_KEY, "/"] }],
items: [
{ label: "Open command palette", keys: [MOD_KEY, "K"] },
{ label: "Show keyboard shortcuts", keys: [MOD_KEY, "/"] },
],
},
{
title: "In chats",
Expand Down
108 changes: 108 additions & 0 deletions web/src/hooks/useCommandPaletteHotkey.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import { cleanup, renderHook } from "@testing-library/react";
import { afterEach, describe, expect, it, vi } from "vitest";

import { isCommandPaletteHotkey, useCommandPaletteHotkey } from "./useCommandPaletteHotkey";

afterEach(() => {
cleanup();
document.body.innerHTML = "";
});

function press(init: KeyboardEventInit): KeyboardEvent {
const e = new KeyboardEvent("keydown", { bubbles: true, cancelable: true, ...init });
window.dispatchEvent(e);
return e;
}

describe("isCommandPaletteHotkey", () => {
it("matches Cmd+K and Ctrl+K", () => {
expect(isCommandPaletteHotkey(new KeyboardEvent("keydown", { key: "k", metaKey: true }))).toBe(
true,
);
expect(isCommandPaletteHotkey(new KeyboardEvent("keydown", { key: "k", ctrlKey: true }))).toBe(
true,
);
// Uppercase (some layouts report "K" with the modifier).
expect(isCommandPaletteHotkey(new KeyboardEvent("keydown", { key: "K", metaKey: true }))).toBe(
true,
);
});

it("rejects plain k, and k with Alt or Shift held", () => {
expect(isCommandPaletteHotkey(new KeyboardEvent("keydown", { key: "k" }))).toBe(false);
expect(
isCommandPaletteHotkey(
new KeyboardEvent("keydown", { key: "k", metaKey: true, altKey: true }),
),
).toBe(false);
expect(
isCommandPaletteHotkey(
new KeyboardEvent("keydown", { key: "k", ctrlKey: true, shiftKey: true }),
),
).toBe(false);
});

it("rejects other keys with the modifier", () => {
expect(isCommandPaletteHotkey(new KeyboardEvent("keydown", { key: "j", metaKey: true }))).toBe(
false,
);
});
});

describe("useCommandPaletteHotkey", () => {
it("toggles on Cmd+K and prevents the browser default", () => {
const onToggle = vi.fn();
renderHook(() => useCommandPaletteHotkey(onToggle));

const e = press({ key: "k", metaKey: true });

expect(onToggle).toHaveBeenCalledTimes(1);
expect(e.defaultPrevented).toBe(true);
});

it("ignores auto-repeat", () => {
const onToggle = vi.fn();
renderHook(() => useCommandPaletteHotkey(onToggle));

press({ key: "k", metaKey: true, repeat: true });

expect(onToggle).not.toHaveBeenCalled();
});

it("does nothing when disabled", () => {
const onToggle = vi.fn();
renderHook(() => useCommandPaletteHotkey(onToggle, false));

const e = press({ key: "k", metaKey: true });

expect(onToggle).not.toHaveBeenCalled();
expect(e.defaultPrevented).toBe(false);
});

it("bails when focus sits inside a terminal or code editor", () => {
const onToggle = vi.fn();
renderHook(() => useCommandPaletteHotkey(onToggle));

const term = document.createElement("div");
term.className = "xterm";
const input = document.createElement("input");
term.appendChild(input);
document.body.appendChild(term);
input.focus();
expect(document.activeElement).toBe(input);

press({ key: "k", metaKey: true });

expect(onToggle).not.toHaveBeenCalled();
});

it("unbinds on unmount", () => {
const onToggle = vi.fn();
const { unmount } = renderHook(() => useCommandPaletteHotkey(onToggle));
unmount();

press({ key: "k", metaKey: true });

expect(onToggle).not.toHaveBeenCalled();
});
});
66 changes: 66 additions & 0 deletions web/src/hooks/useCommandPaletteHotkey.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// ⌘K (Ctrl+K on Win/Linux) toggles the global command palette. Sibling to the
// session-switch (⌘↑/↓) and sidebar-toggle (⌘⌥[ / ⌘⌥]) hotkeys; like them it's
// bound ONCE at the app shell, where the palette's open-state lives.
//
// Why ⌘K: it's the de-facto command-palette key across developer tools, and
// issue #1059 / PR #1064 deliberately reserved it for this (PR #1064 took ⌘⇧F
// for sidebar search precisely to leave ⌘K free). The browser binds Ctrl+K to
// the address bar, so we preventDefault to claim it.
//
// Two surfaces own ⌘K themselves and must keep it: xterm terminals (forward it
// to the PTY) and the Monaco editor (⌘K is a chord prefix). When focus sits in
// one of those, we bail and let the keystroke through.

import { useEffect, useRef } from "react";

/** Selector for surfaces that own ⌘K and must keep it (terminals, code editor). */
const HOTKEY_OWNING_SURFACES = ".xterm, .monaco-editor";

/** True when the event is the command-palette chord: Cmd/Ctrl+K, no Alt/Shift. */
export function isCommandPaletteHotkey(e: globalThis.KeyboardEvent): boolean {
if (!(e.metaKey || e.ctrlKey) || e.altKey || e.shiftKey) return false;
// AltGr reports as Ctrl+Alt on some layouts; the altKey check above already
// rejects it, but guard explicitly so intl typing never triggers the palette.
if (e.getModifierState("AltGraph")) return false;
// Match the letter, not a physical code — ⌘ doesn't remap "k" across layouts.
return e.key === "k" || e.key === "K";
}

/** Does focus sit inside a surface that owns ⌘K (xterm / Monaco)? */
function focusOwnsHotkey(): boolean {
const el = document.activeElement;
return el instanceof Element && el.closest(HOTKEY_OWNING_SURFACES) !== null;
}

/**
* Bind ⌘/Ctrl+K to toggle the command palette. Bind ONCE.
*
* @param onToggle Flip the palette open/closed.
* @param enabled Pass `false` to disable the hotkey (e.g. embedded mode, where
* ⌘K belongs to the host page). Defaults to enabled.
*/
export function useCommandPaletteHotkey(onToggle: () => void, enabled: boolean = true): void {
// Held in a ref so the bound handler always calls the latest closure without
// re-registering on every render.
const latest = useRef(onToggle);
latest.current = onToggle;

useEffect(() => {
if (!enabled) return;
const handler = (e: globalThis.KeyboardEvent): void => {
// Ignore auto-repeat: holding the chord would flap the palette.
if (e.repeat) return;
if (!isCommandPaletteHotkey(e)) return;
// Leave ⌘K to terminals/editors that bind it themselves.
if (focusOwnsHotkey()) return;
// Claim the chord: preventDefault drops the browser default (Ctrl+K
// focuses the address bar). stopPropagation mirrors the sibling hotkey
// hooks; no other listener binds ⌘K, so it's belt-and-suspenders.
e.preventDefault();
e.stopPropagation();
latest.current();
};
window.addEventListener("keydown", handler);
return () => window.removeEventListener("keydown", handler);
}, [enabled]);
}
19 changes: 19 additions & 0 deletions web/src/shell/AppShell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { useConversations } from "@/hooks/useConversations";
import { useSessionAgent } from "@/hooks/useAgents";
import { useApproveHotkey } from "@/hooks/useApproveHotkey";
import { useSidebarToggleHotkeys } from "@/hooks/useSidebarToggleHotkeys";
import { useCommandPaletteHotkey } from "@/hooks/useCommandPaletteHotkey";
import { useIsEmbedded } from "@/lib/embedded";
import { AgentInfoContent, agentHasInfo } from "@/components/AgentInfo";
import { useIdleNotifications } from "@/hooks/useIdleNotifications";
import { useSeedReadState } from "@/hooks/useUnseenConversations";
Expand Down Expand Up @@ -70,6 +72,7 @@ import { TerminalsPanel } from "./TerminalsPanel";
import { TodoPanel } from "./TodoPanel";
import { PermissionsModal } from "@/components/PermissionsModal";
import { KeyboardShortcutsDialog } from "@/components/KeyboardShortcutsDialog";
import { CommandPalette } from "./CommandPalette";
import { Toaster } from "@/components/ui/toast";
import { ForkSessionDialog } from "./ForkSessionDialog";
import { ForkDialogContextProvider, type ForkDialogContextValue } from "./ForkDialogContext";
Expand Down Expand Up @@ -777,6 +780,12 @@ export function AppShell() {
onToggleRight: toggleRightPanel,
});

// ⌘K (Ctrl+K) toggles the command palette. Disabled embedded, where ⌘K is the
// host page's. Bound here where the palette's open-state lives.
const [commandPaletteOpen, setCommandPaletteOpen] = useState(false);
const isEmbedded = useIsEmbedded();
useCommandPaletteHotkey(() => setCommandPaletteOpen((prev) => !prev), !isEmbedded);

// Mobile back button: close the open file and return to the files/changes
// list. On mobile the tab strip is hidden, so a "back" should fully drop the
// file (remove it from openFiles) rather than leaving an orphan tab the user
Expand Down Expand Up @@ -1334,6 +1343,16 @@ export function AppShell() {
{/* Keyboard-shortcuts reference. Self-contained (owns its open state +
⌘/Ctrl+/ opener); ungated so it works on every route. */}
<KeyboardShortcutsDialog />
{/* Global command palette (⌘K). Ungated so it works on every route;
the hotkey itself is disabled in embedded mode. */}
{!isEmbedded && (
<CommandPalette
open={commandPaletteOpen}
onOpenChange={setCommandPaletteOpen}
onToggleLeftSidebar={() => setSidebarOpen((prev) => !prev)}
onToggleRightSidebar={toggleRightPanel}
/>
)}
{/* Transient toasts (e.g. "session archived"). Mounted once here so
any surface can fire one via showToast(). */}
<Toaster />
Expand Down
Loading
Loading