fix: keep plugin shortcuts behind ui:keyboard and off the keys the app and browser use

lib/__tests__/plugin-sandbox-loader.test.ts

@@ -7,6 +7,7 @@ const mocks = vi.hoisted(() => ({
   saveCode: vi.fn(),
   downloadManagedBundle: vi.fn(),
   createBackgroundInstance: vi.fn(),
+  registerShortcuts: vi.fn((..._args: unknown[]) => () => {}),
 }));
 
 vi.mock('../plugin-storage', () => ({
@@ -27,7 +28,7 @@ vi.mock('../plugin-sandbox/host-api', () => ({
   cancelPluginDialogs: vi.fn(),
 }));
 vi.mock('../plugin-sandbox/shortcuts', () => ({
-  registerShortcuts: () => () => {},
+  registerShortcuts: (...args: unknown[]) => mocks.registerShortcuts(...args),
 }));
 
 import { loadSandboxedPlugin, unloadSandboxedPlugin, setSandboxStoreAccessor } from '../plugin-sandbox/loader';
@@ -177,3 +178,30 @@ describe('loadSandboxedPlugin bundle sourcing', () => {
     expect(lastStatus()).toMatchObject({ status: 'error', error: expect.stringMatching(/integrity mismatch/) });
   });
 });
+
+describe('plugin shortcuts', () => {
+  const SHORTCUT = [{ id: 'new', keys: 'Alt+N', label: 'New' }];
+  function withShortcuts() {
+    return { ...fakeBackground(), initPromise: Promise.resolve({ hooks: [], slots: [], shortcuts: SHORTCUT }) };
+  }
+
+  it('does not install shortcuts for a plugin without ui:keyboard', async () => {
+    mocks.getCode.mockResolvedValue(CODE);
+    mocks.createBackgroundInstance.mockImplementation(withShortcuts);
+    mocks.registerShortcuts.mockClear();
+
+    await loadSandboxedPlugin(plugin());
+
+    expect(mocks.registerShortcuts).not.toHaveBeenCalled();
+  });
+
+  it('installs them with ui:keyboard', async () => {
+    mocks.getCode.mockResolvedValue(CODE);
+    mocks.createBackgroundInstance.mockImplementation(withShortcuts);
+    mocks.registerShortcuts.mockClear();
+
+    await loadSandboxedPlugin(plugin({ permissions: ['ui:keyboard'] }));
+
+    expect(mocks.registerShortcuts).toHaveBeenCalledWith(expect.anything(), SHORTCUT);
+  });
+});

lib/__tests__/plugin-shortcuts.test.ts

@@ -0,0 +1,67 @@
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { registerShortcuts, isAllowedPluginCombo } from '../plugin-sandbox/shortcuts';
+import type { SandboxInstance } from '../plugin-sandbox/host-bridge';
+
+function instance(pluginId: string) {
+  return { pluginId, invokeHook: vi.fn(async () => undefined) } as unknown as SandboxInstance & { invokeHook: ReturnType<typeof vi.fn> };
+}
+
+const disposers: Array<() => void> = [];
+afterEach(() => {
+  while (disposers.length) disposers.pop()!();
+});
+
+function press(key: string, mods: Partial<Pick<KeyboardEvent, 'ctrlKey' | 'shiftKey' | 'altKey' | 'metaKey'>> = {}) {
+  const ev = new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...mods });
+  const outer = vi.fn();
+  document.body.addEventListener('keydown', outer);
+  document.body.dispatchEvent(ev);
+  document.body.removeEventListener('keydown', outer);
+  return { defaultPrevented: ev.defaultPrevented, reachedApp: outer.mock.calls.length > 0 };
+}
+
+describe('plugin keyboard shortcuts', () => {
+  it.each(['Escape', 'c', 'Shift+G', 'Ctrl+K', 'Ctrl+F', 'Ctrl+P', 'Cmd+W', 'Ctrl+Shift+T', 'Tab', 'Alt+Tab'])(
+    'refuses %s, which belongs to the app or the browser',
+    (combo) => {
+      expect(isAllowedPluginCombo(combo)).toBe(false);
+    },
+  );
+
+  it.each(['Alt+N', 'Alt+Shift+N', 'Ctrl+Shift+L', 'Ctrl+Alt+S', 'Cmd+Shift+Y'])('allows %s', (combo) => {
+    expect(isAllowedPluginCombo(combo)).toBe(true);
+  });
+
+  it('does not take Escape away from the app', async () => {
+    const plugin = instance('grabber');
+    disposers.push(registerShortcuts(plugin, [{ id: 'esc', keys: 'Escape', label: 'x' }]));
+
+    const result = press('Escape');
+
+    expect(plugin.invokeHook).not.toHaveBeenCalled();
+    expect(result.defaultPrevented).toBe(false);
+    expect(result.reachedApp).toBe(true);
+  });
+
+  it('runs an allowed shortcut without hiding the key from the rest of the app', async () => {
+    const plugin = instance('notes');
+    disposers.push(registerShortcuts(plugin, [{ id: 'new', keys: 'Alt+N', label: 'New note' }]));
+
+    const result = press('n', { altKey: true });
+
+    expect(plugin.invokeHook).toHaveBeenCalledWith('shortcut:new', []);
+    expect(result.reachedApp).toBe(true);
+  });
+
+  it('keeps the first plugin that claimed a combo', async () => {
+    const first = instance('first');
+    const second = instance('second');
+    disposers.push(registerShortcuts(first, [{ id: 'a', keys: 'Alt+J', label: 'a' }]));
+    disposers.push(registerShortcuts(second, [{ id: 'b', keys: 'Alt+J', label: 'b' }]));
+
+    press('j', { altKey: true });
+
+    expect(first.invokeHook).toHaveBeenCalled();
+    expect(second.invokeHook).not.toHaveBeenCalled();
+  });
+});

lib/plugin-sandbox/loader.ts

@@ -21,7 +21,7 @@ import { resolvePluginTier } from './tier';
 import { register as registerActive, deregister as deregisterActive, all as allActiveEntries } from './registry';
 import { cancelPluginDialogs } from './host-api';
 import { registerShortcuts } from './shortcuts';
-import { mayOfferSlot, mayRegisterHook } from './permissions';
+import { mayOfferSlot, mayRegisterHook, pluginHasPermission } from './permissions';
 
 // ─── Hook-bus lookup (one flat map for name → bus) ────────────
 
@@ -193,9 +193,16 @@ export async function loadSandboxedPlugin(plugin: InstalledPlugin): Promise<void
       hookDisposables.push(bus.register(plugin.id, proxy as (...a: unknown[]) => unknown));
     }
 
-    // Install plugin-declared keyboard shortcuts.
-    const shortcutDispose = registerShortcuts(bg, info.shortcuts ?? []);
-    hookDisposables.push({ dispose: shortcutDispose });
+    // Install plugin-declared keyboard shortcuts. A shortcut sees every
+    // key press outside text fields, so it needs ui:keyboard like the hooks.
+    if ((info.shortcuts ?? []).length > 0) {
+      if (pluginHasPermission(plugin, 'ui:keyboard')) {
+        const shortcutDispose = registerShortcuts(bg, info.shortcuts ?? []);
+        hookDisposables.push({ dispose: shortcutDispose });
+      } else {
+        console.warn(`[plugin-sandbox] Plugin "${plugin.id}" declares shortcuts without the ui:keyboard permission; not registered`);
+      }
+    }
 
     const slotOffers = info.slots.filter((offer) => {
       if (mayOfferSlot(plugin, offer.name)) return true;

lib/plugin-sandbox/shortcuts.ts

@@ -49,6 +49,31 @@ function normaliseCombo(combo: string): NormalisedCombo | null {
   return { ctrl, shift, alt, meta, key };
 }
 
+// Keys a plugin may never bind: the app and the browser need them.
+const RESERVED_KEYS = new Set(['escape', 'tab', 'enter', 'f1', 'f5', 'f11', 'f12', 'delete', 'backspace']);
+// Ctrl/Cmd+Shift chords browsers use (reopen tab, new private window,
+// devtools, …).
+const BROWSER_SHIFT_CHORDS = new Set(['t', 'n', 'w', 'q', 'i', 'j', 'c', 'r', 'p', 'b', 'o', 'delete', 'tab', 'enter', 'k']);
+
+/**
+ * Whether a plugin may bind this combo. The app uses bare keys (j, k, c, /,
+ * Shift+G…) and Escape, and browsers own plain Ctrl/Cmd chords, so a plugin
+ * needs Alt, Ctrl/Cmd+Shift or Ctrl/Cmd+Alt.
+ */
+export function isAllowedPluginCombo(keys: string): boolean {
+  const combo = normaliseCombo(keys);
+  if (!combo) return false;
+  if (RESERVED_KEYS.has(combo.key)) return false;
+  const primary = combo.ctrl || combo.meta;
+  if (combo.alt) return true;
+  if (primary && combo.shift) return !BROWSER_SHIFT_CHORDS.has(combo.key);
+  return false;
+}
+
+function comboId(combo: NormalisedCombo): string {
+  return `${combo.ctrl || combo.meta ? 'C' : ''}${combo.shift ? 'S' : ''}${combo.alt ? 'A' : ''}+${combo.key}`;
+}
+
 function eventMatches(ev: KeyboardEvent, combo: NormalisedCombo): boolean {
   if (combo.ctrl !== (ev.ctrlKey || ev.metaKey ? ev.ctrlKey : false)) {
     // Treat Ctrl and Cmd as equivalent: a binding declaring Ctrl matches a
@@ -70,8 +95,8 @@ function onKeyDown(ev: KeyboardEvent): void {
     const combo = normaliseCombo(binding.keys);
     if (!combo) continue;
     if (eventMatches(ev, combo)) {
+      // Not stopPropagation: the app's own listeners still see the key.
       ev.preventDefault();
-      ev.stopPropagation();
       void binding.invoke();
       return;
     }
@@ -90,8 +115,22 @@ export function registerShortcuts(
 ): () => void {
   ensureListener();
   const keys: string[] = [];
+  const taken = new Set([...bindings.values()].map((b) => {
+    const c = normaliseCombo(b.keys);
+    return c ? comboId(c) : '';
+  }));
   for (const sc of shortcuts) {
     const key = `${instance.pluginId}:${sc.id}`;
+    const combo = normaliseCombo(sc.keys);
+    if (!combo || !isAllowedPluginCombo(sc.keys)) {
+      console.warn(`[plugin-shortcuts] "${instance.pluginId}" may not bind ${sc.keys}; use Alt, Ctrl+Shift or Ctrl+Alt`);
+      continue;
+    }
+    if (taken.has(comboId(combo))) {
+      console.warn(`[plugin-shortcuts] ${sc.keys} is already used by another plugin; "${instance.pluginId}" does not get it`);
+      continue;
+    }
+    taken.add(comboId(combo));
     bindings.set(key, {
       pluginId: instance.pluginId,
       shortcutId: sc.id,