From 21dc35687c4078e55ddd16ed7e69c070acd457fa Mon Sep 17 00:00:00 2001 From: "Zeyi (Rice) Fan" Date: Wed, 26 Aug 2026 14:12:42 -0700 Subject: [PATCH] =?UTF-8?q?fix(web):=20always=20bind=20the=20=E2=8C=98K=20?= =?UTF-8?q?command=20palette=20hotkey,=20including=20embedded=20mode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Related issue Linear: OMNI-5473 (cmd + k doesn't work in managed for desktop app) ## Summary - The ⌘K palette hotkey was gated off in embedded mode on the theory "⌘K belongs to the host page" (introduced with the palette in #1386). That host ⌘K doesn't exist, so on managed servers the chord was dead everywhere — most visibly in the desktop app, where the embed build is the whole window. - Drop the `isEmbedded` gate: the hotkey now binds in every mode. In the embedded build the host page is still loaded (CSS-hidden in the desktop shell), so the listener binds in the capture phase and claims the chord ahead of any host-page listener; the Monaco/terminal early-return still lets owned chords through. ## Test Plan - `npx vitest run src/hooks/useCommandPaletteHotkey.test.tsx` — 12 pass, incl. capture-phase claiming the chord ahead of document-level listeners, and focused-Monaco chords still propagating to them. - `npx vitest run src/shell/AppShell.test.tsx src/shell/AppShell.subagent-nav.test.tsx src/shell/CommandPalette.test.tsx` — 129 pass. - `npm run type-check`, `oxlint`, `prettier --check` — clean. - Manual: desktop app on a managed server URL, press ⌘K → palette opens; same URL in a browser tab, ⌘K now opens the Omnigent palette there too. ## Demo N/A — hotkey behavior fix, no visual change. ## Type of change - [x] Bug fix - [ ] Feature - [ ] UI / frontend change - [ ] Refactor / chore - [ ] Docs - [ ] Test / CI - [ ] Breaking change ## Test coverage - [x] Unit tests added / updated - [ ] Integration tests added / updated - [ ] E2E tests added / updated - [ ] Manual verification completed - [x] Existing tests cover this change - [ ] Not applicable ## Changelog ⌘K now opens the command palette everywhere, including the desktop app on managed servers Signed-off-by: Zeyi (Rice) Fan --- .../hooks/useCommandPaletteHotkey.test.tsx | 38 +++++++++++++++++++ web/src/hooks/useCommandPaletteHotkey.ts | 13 +++++-- web/src/shell/AppShell.tsx | 7 ++-- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/web/src/hooks/useCommandPaletteHotkey.test.tsx b/web/src/hooks/useCommandPaletteHotkey.test.tsx index 8dade611947..e8e608f6a8b 100644 --- a/web/src/hooks/useCommandPaletteHotkey.test.tsx +++ b/web/src/hooks/useCommandPaletteHotkey.test.tsx @@ -90,6 +90,44 @@ describe("useCommandPaletteHotkey", () => { expect(document.activeElement).toBe(input); } + it("claims the chord in the capture phase, ahead of host-page listeners", () => { + // The desktop shell renders the embed build over a CSS-hidden host page + // with its own ⌘K listener; the chord must reach us, not them. Dispatch + // from a deep node so the event actually propagates (dispatching on + // window would never reach document listeners regardless). + const onToggle = vi.fn(); + const hostListener = vi.fn(); + renderHook(() => useCommandPaletteHotkey(onToggle)); + document.addEventListener("keydown", hostListener); + const deep = document.createElement("div"); + document.body.appendChild(deep); + + deep.dispatchEvent( + new KeyboardEvent("keydown", { key: "k", metaKey: true, bubbles: true, cancelable: true }), + ); + + expect(onToggle).toHaveBeenCalledTimes(1); + expect(hostListener).not.toHaveBeenCalled(); + document.removeEventListener("keydown", hostListener); + }); + + it("lets the chord through to host-page listeners when a focused surface owns it", () => { + const onToggle = vi.fn(); + const hostListener = vi.fn(); + renderHook(() => useCommandPaletteHotkey(onToggle)); + document.addEventListener("keydown", hostListener); + focusInside("monaco-editor"); + + // Dispatch from the focused input so the event propagates up to document. + document.activeElement?.dispatchEvent( + new KeyboardEvent("keydown", { key: "k", metaKey: true, bubbles: true, cancelable: true }), + ); + + expect(onToggle).not.toHaveBeenCalled(); + expect(hostListener).toHaveBeenCalledTimes(1); + document.removeEventListener("keydown", hostListener); + }); + it("bails on Ctrl+K in a terminal — xterm sends it to the PTY as ^K", () => { const onToggle = vi.fn(); renderHook(() => useCommandPaletteHotkey(onToggle)); diff --git a/web/src/hooks/useCommandPaletteHotkey.ts b/web/src/hooks/useCommandPaletteHotkey.ts index 8b8b986942f..2fdfb8fb0c2 100644 --- a/web/src/hooks/useCommandPaletteHotkey.ts +++ b/web/src/hooks/useCommandPaletteHotkey.ts @@ -47,8 +47,8 @@ function focusOwnsHotkey(e: globalThis.KeyboardEvent): boolean { * 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. + * @param enabled Pass `false` to disable the hotkey (e.g. embedded in a real + * host page, where ⌘K belongs to the host). Defaults to enabled. */ export function useCommandPaletteHotkey(onToggle: () => void, enabled = true): void { // Held in a ref so the bound handler always calls the latest closure without @@ -71,7 +71,12 @@ export function useCommandPaletteHotkey(onToggle: () => void, enabled = true): v e.stopPropagation(); latest.current(); }; - window.addEventListener("keydown", handler); - return () => window.removeEventListener("keydown", handler); + // Capture phase: the desktop shell renders the embed build over a + // CSS-hidden host page whose own ⌘K listener would otherwise also fire. + // Binding at the propagation root lets stopPropagation keep the chord + // from reaching those listeners; focusOwnsHotkey bails BEFORE stopping + // propagation, so Monaco/terminal chords still flow to their surfaces. + window.addEventListener("keydown", handler, true); + return () => window.removeEventListener("keydown", handler, true); }, [enabled]); } diff --git a/web/src/shell/AppShell.tsx b/web/src/shell/AppShell.tsx index e9d3c3086e3..174e4e5439d 100644 --- a/web/src/shell/AppShell.tsx +++ b/web/src/shell/AppShell.tsx @@ -1304,11 +1304,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. + // ⌘K (Ctrl+K) toggles the command palette. Bound capture-phase, so in the + // embedded build we claim the chord ahead of any host-page ⌘K listener. + // Bound here where the palette's open-state lives. const [commandPaletteOpen, setCommandPaletteOpen] = useState(false); const isEmbedded = useIsEmbedded(); - useCommandPaletteHotkey(() => setCommandPaletteOpen((prev) => !prev), !isEmbedded); + useCommandPaletteHotkey(() => setCommandPaletteOpen((prev) => !prev)); useNewSessionHotkey(!isEmbedded); // Mobile back button: close the open file and return to the files/changes