From 6a67a4e952942bbe0a9f3d14de917dfc5e22e4e7 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Tue, 28 Jul 2026 15:04:14 -0500 Subject: [PATCH] fix(desktop): resolve contributed keybinds through the fallback chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit $bindings is seeded at module init from the actions known then, so an action a plugin contributes later is absent from it. Both hotkey-hint call sites did a raw bindings[id] lookup, so a plugin command rendered with no combo in the palette and no hint on its tooltip even though the dispatcher (which goes through $comboIndex → bindingsFor) fired it fine. Route both through bindingsFor, the resolver that already falls back to the stored override and the action's shipped defaults, and subscribe the hint hook to the registry version so a late registration repaints. Covered by behavior tests over the contributed-action contract: dispatch, combo resolution, panel row, teardown, and the no-shadowing guard. --- .../desktop/src/app/command-palette/index.tsx | 6 +- .../lib/keybinds/contributed-actions.test.ts | 66 +++++++++++++++++++ .../src/lib/keybinds/use-keybind-hint.ts | 12 +++- 3 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 apps/desktop/src/lib/keybinds/contributed-actions.test.ts diff --git a/apps/desktop/src/app/command-palette/index.tsx b/apps/desktop/src/app/command-palette/index.tsx index e643efef513..1192b93a0ee 100644 --- a/apps/desktop/src/app/command-palette/index.tsx +++ b/apps/desktop/src/app/command-palette/index.tsx @@ -67,7 +67,7 @@ import { closeCommandPalette, setCommandPaletteOpen } from '@/store/command-palette' -import { $bindings } from '@/store/keybinds' +import { $bindings, bindingsFor } from '@/store/keybinds' import { $dismissedAutoProjectIds, filterVisibleProjects } from '@/store/layout' import { openPetGenerate } from '@/store/pet-generate' import { $projectTree, goToProject, openFolderAsProject, requestStartWorkSession } from '@/store/projects' @@ -328,7 +328,9 @@ const PaletteRow = memo(function PaletteRow({ const Icon = item.icon // The row's live keybind, else a static modifier-variant hint (⌘↵). One slot, // so every downstream `ml-auto` fallback below keeps working unchanged. - const combo = (item.action ? bindings[item.action]?.[0] : undefined) ?? item.comboHint + // `bindingsFor`, not a raw lookup: a plugin's action is contributed after + // $bindings was seeded, so its combo only resolves through the fallback chain. + const combo = (item.action ? bindingsFor(item.action, bindings)[0] : undefined) ?? item.comboHint // While ⌘/⌃ is held, a row with a modifier variant previews it: the label // swaps to the variant's copy so Enter reads as what it will actually do. const modPreview = modHeld && Boolean(item.modLabel) diff --git a/apps/desktop/src/lib/keybinds/contributed-actions.test.ts b/apps/desktop/src/lib/keybinds/contributed-actions.test.ts new file mode 100644 index 00000000000..e5eb72b149d --- /dev/null +++ b/apps/desktop/src/lib/keybinds/contributed-actions.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from 'vitest' + +import { registry } from '@/contrib/registry' +import { createPluginContext } from '@/contrib/plugin' +import { allKeybindActions, contributedKeybindHandler, KEYBINDS_AREA } from '@/lib/keybinds/actions' +import { bindingsFor } from '@/store/keybinds' + +// The plugin-command contract: a plugin ships a hotkey through the `keybinds` +// area and it behaves like a built-in — it dispatches, it resolves a combo for +// the palette hint, and it survives the plugin being unloaded. These assert the +// relationship between the pieces, not the specific chord any one plugin picks. +describe('contributed keybind actions', () => { + it('dispatches, resolves its default combo, and disappears on unload', () => { + const ctx = createPluginContext('demo') + let ran = 0 + + const dispose = ctx.register({ + id: 'new-thing', + area: KEYBINDS_AREA, + data: { + id: 'demo.newThing', + category: 'view', + defaults: ['mod+alt+n'], + label: 'Demo: New thing', + run: () => void (ran += 1) + } + }) + + // Dispatch path: use-keybinds looks the handler up by action id. + contributedKeybindHandler('demo.newThing')?.() + expect(ran).toBe(1) + + // Hint path: $bindings was seeded before this action existed, so only the + // resolver (default fallback) finds the combo — a raw store lookup can't. + expect(bindingsFor('demo.newThing')).toEqual(['mod+alt+n']) + + // Panel path: it shows up as a rebindable row alongside the built-ins. + expect(allKeybindActions().find(a => a.id === 'demo.newThing')?.label).toBe('Demo: New thing') + + dispose() + + expect(contributedKeybindHandler('demo.newThing')).toBeUndefined() + expect(allKeybindActions().some(a => a.id === 'demo.newThing')).toBe(false) + }) + + it('cannot shadow a built-in action id', () => { + const ctx = createPluginContext('demo') + + const dispose = ctx.register({ + id: 'steal-new-session', + area: KEYBINDS_AREA, + data: { id: 'session.new', defaults: ['mod+alt+n'], label: 'Demo: hijack', run: () => undefined } + }) + + // The built-in keeps its own combo and its own (i18n) label — the + // contribution is filtered out rather than overriding core. + expect(bindingsFor('session.new')).toEqual(['mod+n', 'shift+n']) + expect(allKeybindActions().filter(a => a.id === 'session.new')).toHaveLength(1) + + dispose() + }) + + it('leaves no registry residue between plugin loads', () => { + expect(registry.getArea(KEYBINDS_AREA).filter(c => c.source === 'plugin:demo')).toHaveLength(0) + }) +}) diff --git a/apps/desktop/src/lib/keybinds/use-keybind-hint.ts b/apps/desktop/src/lib/keybinds/use-keybind-hint.ts index ba6f0425be0..263c169c25a 100644 --- a/apps/desktop/src/lib/keybinds/use-keybind-hint.ts +++ b/apps/desktop/src/lib/keybinds/use-keybind-hint.ts @@ -1,6 +1,7 @@ import { useStore } from '@nanostores/react' -import { $bindings } from '@/store/keybinds' +import { $registryVersion } from '@/contrib/registry' +import { $bindings, bindingsFor } from '@/store/keybinds' import { KEYBIND_READONLY } from './actions' import { formatCombo } from './combo' @@ -12,7 +13,14 @@ import { formatCombo } from './combo' export function useKeybindHint(actionId: string): string | null { const bindings = useStore($bindings) - const rebindable = bindings[actionId]?.[0] + // `bindingsFor`, not a raw `bindings[id]`: $bindings is seeded at module init + // from the actions known THEN, so a plugin action contributed later isn't in + // it and a raw lookup renders no hint at all. The resolver falls through to + // the stored override and the action's own defaults. Subscribing to the + // registry version repaints the hint when that late registration lands. + useStore($registryVersion) + + const rebindable = bindingsFor(actionId, bindings)[0] if (rebindable) { return formatCombo(rebindable)