fix(shortcuts): prevent accidental double-Shift palette activation (#1519)

src/ui/AppShell.tsx

@@ -53,7 +53,11 @@ import {
 } from "@/shell/split/EmptyPanePicker";
 import { renderTabContent } from "@/shell/tabUtils";
 import { TabBar } from "@/shell/TabBar";
-import { dispatchCtrlW, isShiftKey } from "@/lib/app-keyboard-shortcuts";
+import {
+  dispatchCtrlW,
+  createCommandPaletteShortcutMatcher,
+  isShiftKey,
+} from "@/lib/app-keyboard-shortcuts";
 import { parseCustomKeybindings } from "@/api/open-tabs-api";
 import { findMatchingKeybinding } from "@/lib/keybinding-match";
 import type {
@@ -476,7 +480,6 @@ export function AppShell({
       document.removeEventListener("fullscreenchange", handleFullscreenChange);
   }, []);
 
-  const lastShiftTime = useRef(0);
   const tabsRef = useRef(tabs);
   const activeTabIdRef = useRef(activeTabId);
   const closeActiveTabRef = useRef<() => void>(() => {});
@@ -762,26 +765,21 @@ export function AppShell({
   // Double-shift or Ctrl+K opens the command palette. Double-shift alone was
   // hard to discover.
   useEffect(() => {
+    if (!commandPaletteShortcutEnabled) return;
+    const shortcut = createCommandPaletteShortcutMatcher();
     const handleKeyDown = (e: KeyboardEvent) => {
-      if (isShiftKey(e) && !e.repeat) {
-        const now = Date.now();
-        if (now - lastShiftTime.current < 300 && commandPaletteShortcutEnabled)
-          setCommandPaletteOpen((prev) => !prev);
-        lastShiftTime.current = now;
-      }
-      if (
-        (e.ctrlKey || e.metaKey) &&
-        !e.shiftKey &&
-        !e.altKey &&
-        e.code === "KeyK" &&
-        commandPaletteShortcutEnabled
-      ) {
-        e.preventDefault();
-        setCommandPaletteOpen((prev) => !prev);
-      }
+      if (!shortcut.matches(e)) return;
+      if (!isShiftKey(e)) e.preventDefault();
+      setCommandPaletteOpen((prev) => !prev);
     };
     window.addEventListener("keydown", handleKeyDown);
-    return () => window.removeEventListener("keydown", handleKeyDown);
+    window.addEventListener("blur", shortcut.reset);
+    window.addEventListener("compositionstart", shortcut.reset);
+    return () => {
+      window.removeEventListener("keydown", handleKeyDown);
+      window.removeEventListener("blur", shortcut.reset);
+      window.removeEventListener("compositionstart", shortcut.reset);
+    };
   }, [commandPaletteShortcutEnabled]);
 
   // Ctrl+Shift+E toggles between the two most recent sidebar panels.

src/ui/lib/app-keyboard-shortcuts.ts

@@ -25,3 +25,35 @@ export function dispatchCtrlW(target: EventTarget | null) {
   });
   return !target.dispatchEvent(event);
 }
+
+/** Match two consecutive Shift presses, never a typing or IME sequence. */
+export function createCommandPaletteShortcutMatcher() {
+  let lastShift: number | null = null;
+  const reset = () => {
+    lastShift = null;
+  };
+  const matches = (event: KeyboardEvent, now = Date.now()): boolean => {
+    if (
+      event.defaultPrevented ||
+      event.isComposing ||
+      event.keyCode === 229 ||
+      event.repeat ||
+      event.altKey
+    ) {
+      reset();
+      return false;
+    }
+    if (isShiftKey(event) && !event.ctrlKey && !event.metaKey) {
+      const matched = lastShift !== null && now - lastShift < 300;
+      lastShift = matched ? null : now;
+      return matched;
+    }
+    reset();
+    return (
+      (event.ctrlKey || event.metaKey) &&
+      !event.shiftKey &&
+      event.code === "KeyK"
+    );
+  };
+  return { matches, reset };
+}

src/ui/tests/lib/app-keyboard-shortcuts.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, it, vi } from "vitest";
 import {
   dispatchCtrlW,
+  createCommandPaletteShortcutMatcher,
   getAltDigitShortcut,
   isShiftKey,
 } from "../../lib/app-keyboard-shortcuts";
@@ -40,3 +41,74 @@ describe("app keyboard shortcuts", () => {
     expect(dispatchCtrlW(document.createElement("div"))).toBe(false);
   });
 });
+
+describe("command palette shortcut matching", () => {
+  const shift = () =>
+    new KeyboardEvent("keydown", {
+      key: "Shift",
+      code: "ShiftLeft",
+      shiftKey: true,
+    });
+  it("requires a fresh pair after each double Shift", () => {
+    const { matches } = createCommandPaletteShortcutMatcher();
+    expect(matches(shift(), 1000)).toBe(false);
+    expect(matches(shift(), 1100)).toBe(true);
+    expect(matches(shift(), 1200)).toBe(false);
+    expect(matches(shift(), 1300)).toBe(true);
+  });
+  it("does not combine slow presses", () => {
+    const { matches } = createCommandPaletteShortcutMatcher();
+    expect(matches(shift(), 1000)).toBe(false);
+    expect(matches(shift(), 1300)).toBe(false);
+    expect(matches(shift(), 1400)).toBe(true);
+  });
+  it.each([
+    { key: "A", code: "KeyA", shiftKey: true },
+    { key: "Shift", code: "ShiftLeft", ctrlKey: true },
+    { key: "Shift", code: "ShiftLeft", metaKey: true },
+    { key: "Shift", code: "ShiftLeft", altKey: true },
+    { key: "Shift", code: "ShiftLeft", isComposing: true },
+    { key: "Process", keyCode: 229 },
+    { key: "Shift", code: "ShiftLeft", repeat: true },
+  ])("breaks the sequence on %j", (init) => {
+    const { matches } = createCommandPaletteShortcutMatcher();
+    matches(shift(), 1000);
+    expect(matches(new KeyboardEvent("keydown", init), 1050)).toBe(false);
+    expect(matches(shift(), 1100)).toBe(false);
+  });
+  it("resets on lost focus or composition start", () => {
+    const { matches, reset } = createCommandPaletteShortcutMatcher();
+    matches(shift(), 1000);
+    reset();
+    expect(matches(shift(), 1100)).toBe(false);
+  });
+  it("respects consumed key events", () => {
+    const { matches } = createCommandPaletteShortcutMatcher();
+    matches(shift(), 1000);
+    const event = new KeyboardEvent("keydown", {
+      key: "Shift",
+      cancelable: true,
+    });
+    event.preventDefault();
+    expect(matches(event, 1050)).toBe(false);
+    expect(matches(shift(), 1100)).toBe(false);
+  });
+  it.each([{ ctrlKey: true }, { metaKey: true }])(
+    "keeps the explicit shortcut: %j",
+    (modifier) => {
+      const { matches } = createCommandPaletteShortcutMatcher();
+      expect(
+        matches(new KeyboardEvent("keydown", { code: "KeyK", ...modifier })),
+      ).toBe(true);
+      expect(
+        matches(
+          new KeyboardEvent("keydown", {
+            code: "KeyK",
+            ...modifier,
+            isComposing: true,
+          }),
+        ),
+      ).toBe(false);
+    },
+  );
+});