From 99dfbbde58d480bc50fbbd924062d32d069b2691 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Fri, 24 Jul 2026 11:45:28 +0100 Subject: [PATCH] feat(plugins): add keybindings and modal window capabilities (#1895) * feat(plugins): add keybindings and modal window capabilities Add two independent plugin UI capabilities. Keybindings: plugins declare keybindings in the manifest under ui.keybindings ({id, default, description}), validated backend-side (combo grammar + unique ids, requires a UI bundle) and exposed to the frontend. Plugin JS binds a handler via registry.registerKeybinding(id, handler); a global dispatcher fires it. Combos are user-overridable in Settings > Keyboard Shortcuts (namespaced plugin:{pluginId}:{id}) with conflict detection against core and other plugins. Modals: host.openModal({title?, content, size?, dismissible?}) returns a { close() } handle and renders in a single host-owned Dialog isolated by PluginErrorBoundary, cleaned up on plugin unload. Fixture plugin, PLUGIN-API.md / spec, and a Playwright e2e demonstrate a keybinding handler opening a modal. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(plugins): address review feedback on keybinding dispatch and validation Core shortcuts now take precedence over plugin keybindings on a colliding combo (defaultPrevented cooperation between the two capture-phase dispatchers), a throwing plugin handler no longer aborts dispatch to other plugins, and the keybinding combo grammar is tightened: the backend validator and frontend parser both drop the undocumented control/super modifier aliases and reject shift+digit/symbol combos that can never actually fire (KeyboardEvent.key reports the shifted character, not the token). Co-Authored-By: Claude Opus 4.8 (1M context) * fix(plugins): stop conflicting core shortcut from double-firing with plugin keybindings The earlier defaultPrevented guard only covered the central useAppShortcuts dispatcher. Per-component core shortcuts registered via useKeyboardShortcut run in the bubble phase, after the capture-phase plugin keybinding dispatcher, so a plugin-handled-and-prevented combo still triggered the core action too. Guard useKeyboardShortcut (and use-editor-keybinds, another capture-phase core listener) on event.defaultPrevented so exactly one action fires per combo. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(plugins): make core shortcuts always win over plugin keybindings Panel-search, task-switcher, and other core listeners never checked event.defaultPrevented, so a plugin bound to the same combo (e.g. Cmd/Ctrl+F) could still fire alongside them. Instead of patching every core listener, usePluginShortcuts now computes the set of effective core shortcut combos (registry, resolved through user overrides, plus FIND_IN_PANEL/SAVE) and skips invoking any plugin handler whose combo is in that set, so the matching core listener always handles the event exactly once. Also adds a hook-level regression test for useEditorKeybinds' existing defaultPrevented guard, and fixes a leaked capture-phase window listener in use-keyboard-shortcut.test.ts that could bleed into later tests. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(web): keep Cmd+K closing the command panel after shortcut-precedence guard cmdk's built-in vim bindings intercept Ctrl/Cmd+K and +P as list-navigation shortcuts and call preventDefault() before the event reaches our window-level useKeyboardShortcut listeners. Once that hook started bailing on event.defaultPrevented (added to fix core/plugin shortcut double-firing), the palette's own Cmd+K close/reopen toggle got silently suppressed whenever focus was inside the open panel. Disable cmdk's vim bindings on the command palette so its Ctrl/Cmd+K and +P always reach the app-level toggle. Co-Authored-By: Claude Opus 4.8 (1M context) * docs(plugins): document keybindings and modal window plugin APIs Fill in the public author-facing docs that the internal plugin-keybindings and modal-window contract (PLUGIN-API.md, spec.md) already covers: the registry.registerKeybinding / host.openModal JS surface, the ui.keybindings manifest field with its combo grammar, and a capability mention on the Plugins overview page. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Kandev Agent Co-authored-by: Claude Opus 4.8 (1M context) --- .../fixture-package/manifest.yaml | 4 + .../fixture-package/ui/bundle.js | 21 +- .../backend/internal/plugins/handlers_test.go | 69 ++++ .../internal/plugins/manifest/manifest.go | 19 +- .../plugins/manifest/manifest_test.go | 149 +++++++ .../internal/plugins/manifest/validate.go | 168 ++++++++ apps/web/components/command-panel-footer.tsx | 12 + apps/web/components/global-commands.tsx | 5 + .../plugins/plugin-modal-host.test.tsx | 47 +++ .../components/plugins/plugin-modal-host.tsx | 73 ++++ .../components/settings/general-settings.tsx | 5 + .../settings/keyboard-shortcuts-card.tsx | 191 +++++++-- .../settings/voice-mode-settings.tsx | 2 + apps/web/e2e/tests/plugins/plugins.spec.ts | 33 ++ apps/web/hooks/use-app-shortcuts.ts | 26 +- apps/web/hooks/use-editor-keybinds.test.ts | 101 +++++ apps/web/hooks/use-editor-keybinds.ts | 6 + apps/web/hooks/use-keyboard-shortcut.test.ts | 51 +++ apps/web/hooks/use-keyboard-shortcut.ts | 7 + apps/web/hooks/use-plugin-shortcuts.test.ts | 388 ++++++++++++++++++ apps/web/hooks/use-plugin-shortcuts.ts | 194 +++++++++ apps/web/lib/keyboard/parse-combo.test.ts | 94 +++++ apps/web/lib/keyboard/parse-combo.ts | 156 +++++++ .../web/lib/keyboard/plugin-shortcuts.test.ts | 109 +++++ apps/web/lib/keyboard/plugin-shortcuts.ts | 109 +++++ .../lib/keyboard/shortcut-conflicts.test.ts | 103 +++++ apps/web/lib/keyboard/shortcut-conflicts.ts | 62 +++ apps/web/lib/keyboard/utils.ts | 15 + apps/web/lib/plugins/host-api.test.ts | 18 + apps/web/lib/plugins/host-api.ts | 2 + apps/web/lib/plugins/host.test.ts | 36 ++ apps/web/lib/plugins/host.ts | 2 + apps/web/lib/plugins/modal-manager.test.ts | 87 ++++ apps/web/lib/plugins/modal-manager.ts | 71 ++++ apps/web/lib/plugins/registry.test.ts | 69 +++- apps/web/lib/plugins/registry.ts | 59 ++- apps/web/lib/plugins/types.ts | 34 ++ apps/web/lib/types/plugins.ts | 14 + apps/web/src/main.tsx | 2 + docs/plans/plugins/PLUGIN-API.md | 34 ++ docs/public/plugins-authoring.md | 71 ++++ docs/public/plugins-manifest.md | 8 + docs/public/plugins.md | 6 +- docs/specs/plugins/spec.md | 14 + 44 files changed, 2692 insertions(+), 54 deletions(-) create mode 100644 apps/web/components/plugins/plugin-modal-host.test.tsx create mode 100644 apps/web/components/plugins/plugin-modal-host.tsx create mode 100644 apps/web/hooks/use-editor-keybinds.test.ts create mode 100644 apps/web/hooks/use-keyboard-shortcut.test.ts create mode 100644 apps/web/hooks/use-plugin-shortcuts.test.ts create mode 100644 apps/web/hooks/use-plugin-shortcuts.ts create mode 100644 apps/web/lib/keyboard/parse-combo.test.ts create mode 100644 apps/web/lib/keyboard/parse-combo.ts create mode 100644 apps/web/lib/keyboard/plugin-shortcuts.test.ts create mode 100644 apps/web/lib/keyboard/plugin-shortcuts.ts create mode 100644 apps/web/lib/keyboard/shortcut-conflicts.test.ts create mode 100644 apps/web/lib/keyboard/shortcut-conflicts.ts create mode 100644 apps/web/lib/plugins/modal-manager.test.ts create mode 100644 apps/web/lib/plugins/modal-manager.ts diff --git a/apps/backend/cmd/plugin-fixture/fixture-package/manifest.yaml b/apps/backend/cmd/plugin-fixture/fixture-package/manifest.yaml index 847687dc8d..86e7799bd7 100644 --- a/apps/backend/cmd/plugin-fixture/fixture-package/manifest.yaml +++ b/apps/backend/cmd/plugin-fixture/fixture-package/manifest.yaml @@ -34,6 +34,10 @@ webhooks: ui: bundle: "/ui/bundle.js" + keybindings: + - id: open-demo + default: mod+shift+j + description: Open the demo modal config_schema: type: object diff --git a/apps/backend/cmd/plugin-fixture/fixture-package/ui/bundle.js b/apps/backend/cmd/plugin-fixture/fixture-package/ui/bundle.js index 425b1da8e5..dbd25a45e9 100644 --- a/apps/backend/cmd/plugin-fixture/fixture-package/ui/bundle.js +++ b/apps/backend/cmd/plugin-fixture/fixture-package/ui/bundle.js @@ -4,8 +4,10 @@ * imports, no bundled React — that calls `window.registerKandevPlugin(id, * plugin)` at evaluation time and, on `initialize(registry, host)`, * registers a nav item, a top-level route, a `task-sidebar` slot component, - * a `main-top-bar` slot component, and a `task.created` WS handler. Uses - * only host.React/host.jsx. + * a `main-top-bar` slot component, a `task.created` WS handler, and the + * `open-demo` keybinding (declared in manifest.yaml's `ui.keybindings`, + * default `mod+shift+k`) which opens a `host.openModal(...)` demo modal. + * Uses only host.React/host.jsx. * * The task-created counter lives in module scope (not component state) with * a tiny listener set, so it survives across route navigations (the page @@ -91,6 +93,21 @@ registry.registerWsHandler("task.created", function () { incrementCount(); }); + + registry.registerKeybinding("open-demo", function () { + function DemoModalContent() { + return jsx( + "div", + { id: "hello-demo-modal", "data-testid": "hello-demo-modal" }, + "Hello from the plugin modal", + ); + } + host.openModal({ + title: "Demo Modal", + content: DemoModalContent, + size: "md", + }); + }); }, }); })(); diff --git a/apps/backend/internal/plugins/handlers_test.go b/apps/backend/internal/plugins/handlers_test.go index 00f2c46e1b..cdd49cb2de 100644 --- a/apps/backend/internal/plugins/handlers_test.go +++ b/apps/backend/internal/plugins/handlers_test.go @@ -2,6 +2,7 @@ package plugins import ( "bytes" + "context" "encoding/json" "fmt" "io" @@ -173,6 +174,74 @@ func TestListHandlerReturnsInstalledPlugins(t *testing.T) { } } +// TestListHandlerExposesUIKeybindings confirms that ui.keybindings declared +// in the manifest round-trips through GET /api/plugins into the JSON each +// plugin entry carries for the frontend (record.ui.keybindings, since +// store.Record embeds manifest.Manifest inline). +func TestListHandlerExposesUIKeybindings(t *testing.T) { + router, svc := newTestRouter(t) + platformKey := goruntime.GOOS + "-" + goruntime.GOARCH + manifestYAML := fmt.Sprintf(` +id: kandev-plugin-keybound +api_version: 1 +version: "1.0.0" +display_name: Test Plugin +capabilities: + events: ["task.*"] +runtime: + type: binary + executables: + %s: server/plugin +ui: + bundle: "/ui/bundle.js" + keybindings: + - id: open-demo + default: mod+shift+k + description: Open the demo modal +`, platformKey) + var buf bytes.Buffer + if err := pkgtartest.WritePackage(&buf, map[string][]byte{ + "manifest.yaml": []byte(manifestYAML), + "server/plugin": []byte("#!/bin/sh\necho fake\n"), + "ui/bundle.js": []byte("export default {};"), + }); err != nil { + t.Fatalf("WritePackage: %v", err) + } + if _, err := svc.Install(context.Background(), &buf); err != nil { + t.Fatalf("Install: %v", err) + } + + rec := doRequest(router, http.MethodGet, "/api/plugins", "", nil) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } + + var body struct { + Plugins []struct { + UI struct { + Keybindings []struct { + ID string `json:"id"` + Default string `json:"default"` + Description string `json:"description"` + } `json:"keybindings"` + } `json:"ui"` + } `json:"plugins"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(body.Plugins) != 1 { + t.Fatalf("len(plugins) = %d, want 1", len(body.Plugins)) + } + kbs := body.Plugins[0].UI.Keybindings + if len(kbs) != 1 { + t.Fatalf("len(ui.keybindings) = %d, want 1", len(kbs)) + } + if kbs[0].ID != "open-demo" || kbs[0].Default != "mod+shift+k" || kbs[0].Description != "Open the demo modal" { + t.Fatalf("ui.keybindings[0] = %+v, want id=open-demo default=mod+shift+k description=%q", kbs[0], "Open the demo modal") + } +} + func TestGetHandlerMissingReturns404(t *testing.T) { router, _ := newTestRouter(t) rec := doRequest(router, http.MethodGet, "/api/plugins/missing", "", nil) diff --git a/apps/backend/internal/plugins/manifest/manifest.go b/apps/backend/internal/plugins/manifest/manifest.go index 5188333495..628f5d9c30 100644 --- a/apps/backend/internal/plugins/manifest/manifest.go +++ b/apps/backend/internal/plugins/manifest/manifest.go @@ -92,9 +92,10 @@ type Webhook struct { // optional/secondary when a bundle is declared: native routes come from the // bundle at runtime. type UISection struct { - Pages []UIPage `yaml:"pages,omitempty" json:"pages,omitempty"` - Bundle string `yaml:"bundle,omitempty" json:"bundle,omitempty"` - Styles []string `yaml:"styles,omitempty" json:"styles,omitempty"` + Pages []UIPage `yaml:"pages,omitempty" json:"pages,omitempty"` + Bundle string `yaml:"bundle,omitempty" json:"bundle,omitempty"` + Styles []string `yaml:"styles,omitempty" json:"styles,omitempty"` + Keybindings []UIKeybinding `yaml:"keybindings,omitempty" json:"keybindings,omitempty"` } // UIPage is a single UI page contributed by the plugin. @@ -105,6 +106,18 @@ type UIPage struct { Surface string `yaml:"surface" json:"surface"` } +// UIKeybinding is a single keyboard shortcut a plugin's UI bundle wants +// kandev to register and dispatch on its behalf. ID is plugin-local (unique +// within the plugin, not globally); Default is a combo like "mod+shift+k" +// (see parseKeybindingCombo in validate.go for the accepted grammar). +// Keybindings are only meaningful alongside ui.bundle, since the plugin's JS +// bundle is what handles the dispatched event. +type UIKeybinding struct { + ID string `yaml:"id" json:"id"` + Default string `yaml:"default" json:"default"` + Description string `yaml:"description" json:"description"` +} + // Parse decodes a plugin manifest from YAML bytes. It does not validate the // result; callers should call (*Manifest).Validate() afterward. func Parse(data []byte) (*Manifest, error) { diff --git a/apps/backend/internal/plugins/manifest/manifest_test.go b/apps/backend/internal/plugins/manifest/manifest_test.go index 2c860d9dbb..a2f831ba3f 100644 --- a/apps/backend/internal/plugins/manifest/manifest_test.go +++ b/apps/backend/internal/plugins/manifest/manifest_test.go @@ -335,6 +335,155 @@ func TestValidate_ManifestWithBundleAndNoPagesPasses(t *testing.T) { } } +func TestValidate_ValidKeybindingPasses(t *testing.T) { + m := validManifest(t) + m.UI.Bundle = "/ui/bundle.js" + m.UI.Keybindings = []UIKeybinding{ + {ID: "open-demo", Default: "mod+shift+k", Description: "Open the demo modal"}, + } + + if err := m.Validate(); err != nil { + t.Fatalf("Validate() unexpected error: %v", err) + } +} + +func TestValidate_RejectsDuplicateKeybindingID(t *testing.T) { + m := validManifest(t) + m.UI.Bundle = "/ui/bundle.js" + m.UI.Keybindings = []UIKeybinding{ + {ID: "open-demo", Default: "mod+k", Description: "Open the demo modal"}, + {ID: "open-demo", Default: "mod+j", Description: "Open a different modal"}, + } + + err := m.Validate() + if err == nil { + t.Fatal("Validate() expected error for duplicate ui.keybindings id, got nil") + } + if !strings.Contains(err.Error(), "duplicate") { + t.Fatalf("Validate() error = %q, want substring %q", err.Error(), "duplicate") + } +} + +func TestValidate_RejectsEmptyKeybindingDescription(t *testing.T) { + m := validManifest(t) + m.UI.Bundle = "/ui/bundle.js" + m.UI.Keybindings = []UIKeybinding{ + {ID: "open-demo", Default: "mod+k", Description: ""}, + } + + err := m.Validate() + if err == nil { + t.Fatal("Validate() expected error for empty ui.keybindings description, got nil") + } + if !strings.Contains(err.Error(), "description") { + t.Fatalf("Validate() error = %q, want substring %q", err.Error(), "description") + } +} + +func TestValidate_RejectsKeybindingWithoutBundle(t *testing.T) { + m := validManifest(t) + m.UI.Bundle = "" + m.UI.Keybindings = []UIKeybinding{ + {ID: "open-demo", Default: "mod+k", Description: "Open the demo modal"}, + } + + err := m.Validate() + if err == nil { + t.Fatal("Validate() expected error for ui.keybindings without ui.bundle, got nil") + } + if !strings.Contains(err.Error(), "ui.bundle") { + t.Fatalf("Validate() error = %q, want substring %q", err.Error(), "ui.bundle") + } +} + +func TestParseKeybindingCombo_ValidCombos(t *testing.T) { + valid := []string{"k", "mod+k", "mod+shift+k", "ctrl+alt+f1", "arrowup", "cmd+enter", " mod + K "} + for _, combo := range valid { + if err := parseKeybindingCombo(combo); err != nil { + t.Errorf("parseKeybindingCombo(%q) unexpected error: %v", combo, err) + } + } +} + +func TestParseKeybindingCombo_RejectsEmpty(t *testing.T) { + if err := parseKeybindingCombo(""); err == nil { + t.Fatal("parseKeybindingCombo(\"\") expected error, got nil") + } +} + +func TestParseKeybindingCombo_RejectsTwoNonModifierKeys(t *testing.T) { + err := parseKeybindingCombo("mod+k+j") + if err == nil { + t.Fatal("parseKeybindingCombo(\"mod+k+j\") expected error, got nil") + } + if !strings.Contains(err.Error(), "exactly one") { + t.Fatalf("parseKeybindingCombo() error = %q, want substring %q", err.Error(), "exactly one") + } +} + +func TestParseKeybindingCombo_RejectsNoNonModifierKey(t *testing.T) { + err := parseKeybindingCombo("mod+shift") + if err == nil { + t.Fatal("parseKeybindingCombo(\"mod+shift\") expected error, got nil") + } + if !strings.Contains(err.Error(), "exactly one") { + t.Fatalf("parseKeybindingCombo() error = %q, want substring %q", err.Error(), "exactly one") + } +} + +func TestParseKeybindingCombo_RejectsUnknownModifier(t *testing.T) { + err := parseKeybindingCombo("hyper+k") + if err == nil { + t.Fatal("parseKeybindingCombo(\"hyper+k\") expected error, got nil") + } + if !strings.Contains(err.Error(), "unknown token") { + t.Fatalf("parseKeybindingCombo() error = %q, want substring %q", err.Error(), "unknown token") + } +} + +// "control" and "super" are undocumented aliases for ctrl/cmd that the +// backend used to accept even though docs/plans/plugins/PLUGIN-API.md only +// documents mod|ctrl|cmd|meta|alt|option|shift. Dropped for consistency with +// the public contract and the frontend parser. +func TestParseKeybindingCombo_RejectsUndocumentedModifierAliases(t *testing.T) { + for _, combo := range []string{"control+k", "super+k"} { + err := parseKeybindingCombo(combo) + if err == nil { + t.Fatalf("parseKeybindingCombo(%q) expected error, got nil", combo) + } + if !strings.Contains(err.Error(), "unknown token") { + t.Fatalf("parseKeybindingCombo(%q) error = %q, want substring %q", combo, err.Error(), "unknown token") + } + } +} + +// A manifest default like "mod+shift+slash" or "shift+1" is stored/compared +// against KeyboardEvent.key, but a physically-held Shift means the browser +// reports the shifted character ("?", "!") instead of the token validated +// here — so the combo could never actually dispatch. Reject it at +// registration time instead. +func TestParseKeybindingCombo_RejectsShiftWithAlteredKey(t *testing.T) { + for _, combo := range []string{"shift+1", "mod+shift+slash", "shift+9", "shift+minus"} { + err := parseKeybindingCombo(combo) + if err == nil { + t.Fatalf("parseKeybindingCombo(%q) expected error, got nil", combo) + } + if !strings.Contains(err.Error(), "shift") { + t.Fatalf("parseKeybindingCombo(%q) error = %q, want substring %q", combo, err.Error(), "shift") + } + } +} + +// Shift is fine with letters (case is normalized before matching), named +// keys, and function keys, since Shift doesn't change their KeyboardEvent.key. +func TestParseKeybindingCombo_AllowsShiftWithUnaffectedKeys(t *testing.T) { + for _, combo := range []string{"shift+k", "mod+shift+tab", "shift+arrowup", "ctrl+shift+f1"} { + if err := parseKeybindingCombo(combo); err != nil { + t.Errorf("parseKeybindingCombo(%q) unexpected error: %v", combo, err) + } + } +} + func TestHasUIBundle_TrueWhenBundleSet(t *testing.T) { m := validManifest(t) m.UI.Bundle = "/ui/bundle.js" diff --git a/apps/backend/internal/plugins/manifest/validate.go b/apps/backend/internal/plugins/manifest/validate.go index 5f0f2c1c7f..ab4d6a3e09 100644 --- a/apps/backend/internal/plugins/manifest/validate.go +++ b/apps/backend/internal/plugins/manifest/validate.go @@ -30,6 +30,87 @@ var validUISurfaces = map[string]bool{ "main-nav": true, } +// keybindingIDPattern matches the required UIKeybinding.ID shape: lowercase +// alphanumerics and hyphens, starting with a lowercase letter or digit. IDs +// are plugin-local (unique within the plugin's own keybindings list, not +// globally), so this is deliberately simpler than idPattern. +var keybindingIDPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*$`) + +// keybindingModifiers are the accepted modifier tokens in a keybinding combo +// (case-insensitive; matched after lowercasing). "mod" is the +// platform-agnostic primary modifier (Cmd on macOS, Ctrl elsewhere) that the +// frontend dispatcher resolves at runtime; the rest are explicit aliases. +// This set must match the public contract in docs/plans/plugins/PLUGIN-API.md +// and the frontend parser (apps/web/lib/keyboard/parse-combo.ts) exactly — +// "control" and "super" are deliberately NOT accepted here even though they'd +// be easy aliases, because the documented grammar only lists +// mod|ctrl|cmd|meta|alt|option|shift. +var keybindingModifiers = map[string]bool{ + "mod": true, + "ctrl": true, + "cmd": true, + "meta": true, + "alt": true, + "option": true, + "shift": true, +} + +// keybindingKeys are the accepted non-modifier key tokens in a keybinding +// combo (case-insensitive; matched after lowercasing). This is a +// conservative, documented vocabulary — not an exhaustive KeyboardEvent.key +// mirror — covering: single alphanumeric characters, function keys F1-F12, +// and the common named keys plugins are expected to bind. +var keybindingKeys = buildKeybindingKeySet() + +func buildKeybindingKeySet() map[string]bool { + keys := map[string]bool{ + "enter": true, "escape": true, "esc": true, "tab": true, "space": true, + "backspace": true, "delete": true, "insert": true, + "arrowup": true, "arrowdown": true, "arrowleft": true, "arrowright": true, + "up": true, "down": true, "left": true, "right": true, + "home": true, "end": true, "pageup": true, "pagedown": true, + "comma": true, "period": true, "slash": true, "backslash": true, + "semicolon": true, "quote": true, "minus": true, "equal": true, + "bracketleft": true, "bracketright": true, "backquote": true, + } + for c := 'a'; c <= 'z'; c++ { + keys[string(c)] = true + } + for c := '0'; c <= '9'; c++ { + keys[string(c)] = true + } + for i := 1; i <= 12; i++ { + keys[fmt.Sprintf("f%d", i)] = true + } + return keys +} + +// shiftAlteredKeys are non-modifier key tokens whose printable character +// changes when Shift is physically held on a standard keyboard layout (e.g. +// "1" reports as "!", "slash" reports as "?"). The frontend dispatcher +// matches a combo against `KeyboardEvent.key`, so a stored default like +// "shift+1" or "mod+shift+slash" can never fire: Shift held down means the +// browser reports the shifted character, not the token validated here. +// Rejected at registration time so plugin authors get an actionable error +// instead of a keybinding that silently never dispatches. Letters are exempt +// because matching lowercases both sides; named/function keys are exempt +// because Shift doesn't change their `KeyboardEvent.key`. Must be kept in +// sync with the frontend's SHIFT_ALTERED_KEY_TOKENS in +// apps/web/lib/keyboard/parse-combo.ts. +var shiftAlteredKeys = buildShiftAlteredKeySet() + +func buildShiftAlteredKeySet() map[string]bool { + keys := map[string]bool{ + "comma": true, "period": true, "slash": true, "backslash": true, + "semicolon": true, "quote": true, "minus": true, "equal": true, + "bracketleft": true, "bracketright": true, "backquote": true, + } + for c := '0'; c <= '9'; c++ { + keys[string(c)] = true + } + return keys +} + // Validate checks the manifest against the plugin registration rules // described in docs/specs/plugins/spec.md. It returns nil if the manifest // is well-formed, or a joined error describing every violation found. @@ -47,6 +128,7 @@ func (m *Manifest) Validate() error { errs = append(errs, m.validateRepoURL()...) errs = append(errs, m.validateUIPages()...) errs = append(errs, m.validateUIBundle()...) + errs = append(errs, m.validateUIKeybindings()...) errs = append(errs, m.validateWebhooks()...) return errors.Join(errs...) } @@ -222,6 +304,92 @@ func (m *Manifest) validateUIBundle() []error { return errs } +// validateUIKeybindings checks each declared ui.keybindings entry: id is a +// non-empty slug unique within the plugin, description is non-empty, and +// default parses as a valid combo (see parseKeybindingCombo). Keybindings +// are optional, but when present they require ui.bundle — kandev dispatches +// the combo as an event the plugin's JS bundle handles, so a keybinding +// without a bundle can never do anything. +func (m *Manifest) validateUIKeybindings() []error { + var errs []error + if len(m.UI.Keybindings) > 0 && m.UI.Bundle == "" { + errs = append(errs, errors.New("ui.keybindings requires ui.bundle to be set")) + } + seen := make(map[string]bool, len(m.UI.Keybindings)) + for _, kb := range m.UI.Keybindings { + errs = append(errs, validateKeybindingID(kb.ID, seen)...) + if strings.TrimSpace(kb.Description) == "" { + errs = append(errs, fmt.Errorf("ui.keybindings[%q]: description must not be empty", kb.ID)) + } + if err := parseKeybindingCombo(kb.Default); err != nil { + errs = append(errs, fmt.Errorf("ui.keybindings[%q]: %w", kb.ID, err)) + } + } + return errs +} + +// validateKeybindingID checks a single keybinding id against the slug +// pattern and records it in seen, reporting a duplicate-id error on repeat. +func validateKeybindingID(id string, seen map[string]bool) []error { + var errs []error + if !keybindingIDPattern.MatchString(id) { + errs = append(errs, fmt.Errorf("invalid ui.keybindings id %q: must match %s", id, keybindingIDPattern.String())) + return errs + } + if seen[id] { + errs = append(errs, fmt.Errorf("duplicate ui.keybindings id %q", id)) + return errs + } + seen[id] = true + return errs +} + +// parseKeybindingCombo validates a keybinding combo string such as +// "mod+shift+k": tokens are split on "+", trimmed of surrounding whitespace, +// and lowercased. Every token but exactly one must be a recognized modifier +// (keybindingModifiers); the remaining single token must be a recognized key +// (keybindingKeys). It also rejects combos pairing the shift modifier with a +// key whose reported character shift would alter (see shiftAlteredKeys) since +// such a combo can never actually dispatch. It returns an error describing +// the first problem found; it does not return a normalized struct because +// validation is currently the only caller. +func parseKeybindingCombo(combo string) error { + if strings.TrimSpace(combo) == "" { + return errors.New("default combo must not be empty") + } + rawTokens := strings.Split(combo, "+") + var keyToken string + keyCount := 0 + hasShift := false + for _, raw := range rawTokens { + token := strings.ToLower(strings.TrimSpace(raw)) + if token == "" { + return fmt.Errorf("default combo %q has an empty token", combo) + } + if keybindingModifiers[token] { + if token == "shift" { + hasShift = true + } + continue + } + if !keybindingKeys[token] { + return fmt.Errorf("default combo %q: unknown token %q (not a recognized modifier or key)", combo, token) + } + keyToken = token + keyCount++ + } + if keyCount == 0 { + return fmt.Errorf("default combo %q must include exactly one non-modifier key", combo) + } + if keyCount > 1 { + return fmt.Errorf("default combo %q must include exactly one non-modifier key, found more than one (last: %q)", combo, keyToken) + } + if hasShift && shiftAlteredKeys[keyToken] { + return fmt.Errorf("default combo %q: shift+%q is not supported because Shift changes the character reported for this key; remove shift or bind a different key", combo, keyToken) + } + return nil +} + // validateWebhooks checks for duplicate webhook keys. func (m *Manifest) validateWebhooks() []error { seen := make(map[string]bool, len(m.Webhooks)) diff --git a/apps/web/components/command-panel-footer.tsx b/apps/web/components/command-panel-footer.tsx index 2623512ce9..22db973663 100644 --- a/apps/web/components/command-panel-footer.tsx +++ b/apps/web/components/command-panel-footer.tsx @@ -356,6 +356,18 @@ export function CommandPanelView({ filter={scoreCommandSearch} shouldFilter={mode === MODE_COMMANDS} loop + // cmdk's built-in vim bindings intercept Ctrl/Cmd+K, +P, +J, +N as + // list-navigation shortcuts and call `event.preventDefault()` on the + // React (bubble) dispatch, which runs before this event reaches our + // `window`-level `useKeyboardShortcut` listeners. That collided with + // the app's own COMMAND_PANEL-open (Ctrl/Cmd+K) and COMMAND_PANEL + // (Ctrl/Cmd+P) shortcuts: once `useKeyboardShortcut` started bailing + // on `event.defaultPrevented` (see that hook's core-vs-plugin + // precedence guard), cmdk's vim-binding preventDefault silently + // suppressed the panel's own close/reopen toggle. Disable vim + // bindings here so this palette's Ctrl/Cmd+K and +P always reach the + // app-level toggle instead of cmdk's internal navigation. + vimBindings={false} value={selectedValue} onValueChange={setSelectedValue} > diff --git a/apps/web/components/global-commands.tsx b/apps/web/components/global-commands.tsx index 82accb1421..26e506e030 100644 --- a/apps/web/components/global-commands.tsx +++ b/apps/web/components/global-commands.tsx @@ -20,6 +20,7 @@ import { import { useRegisterCommands } from "@/hooks/use-register-commands"; import { useKeyboardShortcut } from "@/hooks/use-keyboard-shortcut"; import { useAppShortcuts } from "@/hooks/use-app-shortcuts"; +import { usePluginShortcuts } from "@/hooks/use-plugin-shortcuts"; import { useAppStore } from "@/components/state-provider"; import { useQuickChatLauncher } from "@/hooks/use-quick-chat-launcher"; import { getShortcut } from "@/lib/keyboard/shortcut-overrides"; @@ -186,7 +187,11 @@ export function GlobalCommands() { useRegisterCommands(commands); useKeyboardShortcut(quickChatShortcut, handleOpenQuickChat); + // Order matters: useAppShortcuts (core) must register its capture-phase + // keydown listener before usePluginShortcuts so core shortcuts win when a + // combo matches both — see the precedence note on each hook. useAppShortcuts(); + usePluginShortcuts(); return null; } diff --git a/apps/web/components/plugins/plugin-modal-host.test.tsx b/apps/web/components/plugins/plugin-modal-host.test.tsx new file mode 100644 index 0000000000..0bced99e69 --- /dev/null +++ b/apps/web/components/plugins/plugin-modal-host.test.tsx @@ -0,0 +1,47 @@ +import { act, cleanup, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it } from "vitest"; +import { pluginModalManager } from "@/lib/plugins/modal-manager"; +import { PluginModalHost } from "./plugin-modal-host"; + +function cleanupModals(pluginId: string) { + pluginModalManager.closeAllForPlugin(pluginId); +} + +describe("PluginModalHost", () => { + afterEach(() => { + cleanup(); + cleanupModals("plugin-a"); + }); + + it("renders nothing when no plugin has an open modal", () => { + const { container } = render(); + expect(container.innerHTML).toBe(""); + }); + + it("renders an open modal's title and content", () => { + pluginModalManager.openModal("plugin-a", { + title: "My Modal", + content: () =>
Hello
, + }); + + render(); + + expect(screen.getByText("My Modal")).not.toBeNull(); + expect(screen.getByTestId("modal-content")).not.toBeNull(); + }); + + it("removes the modal from the DOM once its handle is closed", () => { + const handle = pluginModalManager.openModal("plugin-a", { + content: () =>
Hello
, + }); + + render(); + expect(screen.getByTestId("modal-content")).not.toBeNull(); + + act(() => { + handle.close(); + }); + + expect(screen.queryByTestId("modal-content")).toBeNull(); + }); +}); diff --git a/apps/web/components/plugins/plugin-modal-host.tsx b/apps/web/components/plugins/plugin-modal-host.tsx new file mode 100644 index 0000000000..5cf0bdb2d7 --- /dev/null +++ b/apps/web/components/plugins/plugin-modal-host.tsx @@ -0,0 +1,73 @@ +"use client"; + +import { Dialog, DialogContent, DialogHeader, DialogTitle } from "@kandev/ui/dialog"; +import { + pluginModalManager, + usePluginModals, + type OpenPluginModal, +} from "@/lib/plugins/modal-manager"; +import type { PluginModalOptions } from "@/lib/plugins/types"; +import { PluginErrorBoundary } from "./plugin-error-boundary"; + +/** Maps `PluginModalOptions.size` to the host's Dialog width classes. */ +const SIZE_CLASSES: Record, string> = { + sm: "sm:max-w-sm", + md: "sm:max-w-xl", + lg: "sm:max-w-3xl", + xl: "sm:max-w-5xl", +}; + +function preventWhenNotDismissible(dismissible: boolean) { + return (event: Event) => { + if (!dismissible) event.preventDefault(); + }; +} + +function PluginModalInstance({ modal }: { modal: OpenPluginModal }) { + const { instanceId, pluginId, options } = modal; + const dismissible = options.dismissible ?? true; + const Content = options.content; + const guardClose = preventWhenNotDismissible(dismissible); + + const handleOpenChange = (open: boolean) => { + if (open || !dismissible) return; + pluginModalManager.close(instanceId); + }; + + return ( + + + {options.title && ( + + {options.title} + + )} + + + + + + ); +} + +/** + * Renders every open plugin modal (`host.openModal(...)`) in a `@kandev/ui` + * `Dialog`, each isolated behind its own `PluginErrorBoundary`. Mounted once + * at the app root, alongside ``. + */ +export function PluginModalHost() { + const modals = usePluginModals(); + if (modals.length === 0) return null; + return ( + <> + {modals.map((modal) => ( + + ))} + + ); +} diff --git a/apps/web/components/settings/general-settings.tsx b/apps/web/components/settings/general-settings.tsx index 83e0cfae61..df7327fa6f 100644 --- a/apps/web/components/settings/general-settings.tsx +++ b/apps/web/components/settings/general-settings.tsx @@ -28,6 +28,8 @@ import { ArchiveConfirmationSettings } from "@/components/settings/archive-confi import { MCPTaskAgentProfileDefaultSettings } from "@/components/settings/mcp-task-agent-profile-default-settings"; import { useSettingsSaveContributor } from "@/components/settings/settings-save-provider"; import type { StoredShortcutOverrides } from "@/lib/keyboard/shortcut-overrides"; +import { buildPluginShortcutEntries } from "@/lib/keyboard/plugin-shortcuts"; +import { usePlugins } from "@/hooks/domains/plugins/use-plugins"; function ThemeSettingsCard({ theme, @@ -300,6 +302,8 @@ export function KeyboardShortcutsSettings() { const userSettings = useAppStore((state) => state.userSettings); const setUserSettings = useAppStore((state) => state.setUserSettings); const storeApi = useAppStoreApi(); + const { items: pluginItems } = usePlugins(); + const pluginShortcutEntries = buildPluginShortcutEntries(pluginItems); const [saved, setSaved] = useState(() => ({ chatSubmitKey: userSettings.chatSubmitKey, keyboardShortcuts: userSettings.keyboardShortcuts as StoredShortcutOverrides, @@ -353,6 +357,7 @@ export function KeyboardShortcutsSettings() { onChange={(keyboardShortcuts) => setDraft((current) => ({ ...current, keyboardShortcuts })) } + pluginEntries={pluginShortcutEntries} /> diff --git a/apps/web/components/settings/keyboard-shortcuts-card.tsx b/apps/web/components/settings/keyboard-shortcuts-card.tsx index c2db289e90..fb94e90f3f 100644 --- a/apps/web/components/settings/keyboard-shortcuts-card.tsx +++ b/apps/web/components/settings/keyboard-shortcuts-card.tsx @@ -1,12 +1,12 @@ "use client"; -import { useCallback, useEffect, useState } from "react"; -import { IconRotate, IconX } from "@tabler/icons-react"; +import { useCallback, useEffect, useMemo, useState } from "react"; +import { IconAlertTriangle, IconRotate, IconX } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; import { CardContent, CardHeader, CardTitle } from "@kandev/ui/card"; import { Kbd } from "@kandev/ui/kbd"; import type { Key, KeyboardShortcut } from "@/lib/keyboard/constants"; -import { formatShortcut } from "@/lib/keyboard/utils"; +import { formatShortcut, isMac } from "@/lib/keyboard/utils"; import { CONFIGURABLE_SHORTCUTS, UNBOUND_SHORTCUT, @@ -15,29 +15,43 @@ import { type ConfigurableShortcutId, type StoredShortcutOverrides, } from "@/lib/keyboard/shortcut-overrides"; +import { + coreShortcutEntries, + resolveShortcutEntry, + type ShortcutEntry, +} from "@/lib/keyboard/plugin-shortcuts"; +import { + findShortcutConflicts, + type ShortcutConflictGroup, +} from "@/lib/keyboard/shortcut-conflicts"; import { SettingsCard } from "./settings-card"; type ShortcutRecorderProps = { - shortcutId: ConfigurableShortcutId; + shortcutId: string; + label: string; + defaultShortcut: KeyboardShortcut; current: KeyboardShortcut; - onChange: (id: ConfigurableShortcutId, shortcut: KeyboardShortcut) => void; - onReset: (id: ConfigurableShortcutId) => void; + onChange: (id: string, shortcut: KeyboardShortcut) => void; + onReset: (id: string) => void; // Optional: callers that don't support an explicit "unbind" (e.g. the voice // settings recorder) omit this, and the Clear button is hidden for them. - onClear?: (id: ConfigurableShortcutId) => void; + onClear?: (id: string) => void; isDirty?: boolean; + conflictsWith?: string[]; }; export function ShortcutRecorder({ shortcutId, + label, + defaultShortcut, current, onChange, onReset, onClear, isDirty = false, + conflictsWith, }: ShortcutRecorderProps) { const [recording, setRecording] = useState(false); - const defaultShortcut = CONFIGURABLE_SHORTCUTS[shortcutId].default; const isDefault = JSON.stringify(current) === JSON.stringify(defaultShortcut); const isUnbound = isUnboundShortcut(current); const defaultIsUnbound = isUnboundShortcut(defaultShortcut); @@ -84,7 +98,7 @@ export function ShortcutRecorder({ return (
- {CONFIGURABLE_SHORTCUTS[shortcutId].label} +
- {onClear && !isUnbound && !defaultIsUnbound && ( - - )} - {!isDefault && ( - - )} +
); } +function ShortcutRecorderLabel({ + label, + conflictsWith, +}: { + label: string; + conflictsWith?: string[]; +}) { + return ( + + {label} + {conflictsWith && conflictsWith.length > 0 && ( + + + + )} + + ); +} + +function ShortcutRecorderActions({ + shortcutId, + isDefault, + isUnbound, + defaultIsUnbound, + onReset, + onClear, +}: { + shortcutId: string; + isDefault: boolean; + isUnbound: boolean; + defaultIsUnbound: boolean; + onReset: (id: string) => void; + onClear?: (id: string) => void; +}) { + return ( + <> + {onClear && !isUnbound && !defaultIsUnbound && ( + + )} + {!isDefault && ( + + )} + + ); +} + function renderRecorderLabel({ recording, current, @@ -141,27 +206,54 @@ function renderRecorderLabel({ return {formatShortcut(current)}; } +/** Builds a `shortcutId -> conflicting labels` lookup from conflict groups. */ +function buildConflictLabels(groups: ShortcutConflictGroup[]): Map { + const labels = new Map(); + for (const group of groups) { + for (const entry of group.entries) { + labels.set( + entry.id, + group.entries.filter((other) => other.id !== entry.id).map((other) => other.label), + ); + } + } + return labels; +} + export function KeyboardShortcutsCard({ overrides, baselineOverrides = {}, onChange, + pluginEntries = [], }: { overrides: StoredShortcutOverrides; baselineOverrides?: StoredShortcutOverrides; onChange: (overrides: StoredShortcutOverrides) => void; + /** Dynamic plugin-declared shortcuts (see `lib/keyboard/plugin-shortcuts.ts`). */ + pluginEntries?: ShortcutEntry[]; }) { const shortcuts = resolveAllShortcuts(overrides); const baselineShortcuts = resolveAllShortcuts(baselineOverrides); + const allEntries = useMemo(() => [...coreShortcutEntries(), ...pluginEntries], [pluginEntries]); + + const conflictLabels = useMemo(() => { + const resolved = allEntries.map((entry) => ({ + entry, + shortcut: resolveShortcutEntry(entry, overrides), + })); + return buildConflictLabels(findShortcutConflicts(resolved, isMac())); + }, [allEntries, overrides]); + const handleChange = useCallback( - (id: ConfigurableShortcutId, shortcut: KeyboardShortcut) => { + (id: string, shortcut: KeyboardShortcut) => { onChange({ ...overrides, [id]: shortcut }); }, [onChange, overrides], ); const handleReset = useCallback( - (id: ConfigurableShortcutId) => { + (id: string) => { const next = { ...overrides }; delete next[id]; onChange(next); @@ -170,7 +262,7 @@ export function KeyboardShortcutsCard({ ); const handleClear = useCallback( - (id: ConfigurableShortcutId) => { + (id: string) => { onChange({ ...overrides, [id]: UNBOUND_SHORTCUT }); }, [onChange, overrides], @@ -189,14 +281,41 @@ export function KeyboardShortcutsCard({ ))} + {pluginEntries.length > 0 && ( +
+

Plugin Shortcuts

+
+ {pluginEntries.map((entry) => ( + + ))} +
+
+ )}

Click a shortcut to record a new key combination.

diff --git a/apps/web/components/settings/voice-mode-settings.tsx b/apps/web/components/settings/voice-mode-settings.tsx index 0cf55e991f..473360f00c 100644 --- a/apps/web/components/settings/voice-mode-settings.tsx +++ b/apps/web/components/settings/voice-mode-settings.tsx @@ -525,6 +525,8 @@ function VoiceShortcutCard() { fs.existsSync(pluginDir), { timeout: 10_000 }).toBe(false); }); + test("keybinding declared in the manifest opens a host.openModal demo modal", async ({ + testPage, + }) => { + test.setTimeout(60_000); + + // --- Install via the upload UI, then reload so the boot payload (and + // the manifest-driven keybinding registration) is live. --- + await openInstallDialog(testPage); + await uploadPackage(testPage, PACKAGE_PATH); + const pluginRow = testPage.getByTestId(`plugin-row-${PLUGIN_ID}`); + await expect(pluginRow).toBeVisible({ timeout: 15_000 }); + + await testPage.goto("/"); + await testPage.reload(); + await expect(testPage.getByTestId(`plugin-nav-item-${NAV_ITEM_ID}`)).toBeVisible({ + timeout: 15_000, + }); + + // --- manifest.yaml declares `ui.keybindings: [{ id: open-demo, default: + // mod+shift+j }]`; bundle.js binds it to host.openModal(...). "mod" + // resolves to Ctrl/Cmd per-platform, matching Playwright's + // "ControlOrMeta" pseudo-modifier. --- + await testPage.keyboard.press("ControlOrMeta+Shift+J"); + const modal = testPage.getByTestId("hello-demo-modal"); + await expect(modal).toBeVisible(); + await expect(modal).toHaveText("Hello from the plugin modal"); + + // --- The host Dialog's built-in close button dismisses the (dismissible + // by default) modal. --- + await testPage.getByRole("button", { name: "Close" }).click(); + await expect(modal).not.toBeVisible(); + }); + test("settings page: schema-driven form, secret masking, and Host GetConfig delivery", async ({ testPage, apiClient, diff --git a/apps/web/hooks/use-app-shortcuts.ts b/apps/web/hooks/use-app-shortcuts.ts index 6812c0efac..340df90302 100644 --- a/apps/web/hooks/use-app-shortcuts.ts +++ b/apps/web/hooks/use-app-shortcuts.ts @@ -2,17 +2,9 @@ import { useEffect } from "react"; import { useAppStoreApi } from "@/components/state-provider"; -import { matchesShortcut } from "@/lib/keyboard/utils"; +import { isEditableKeydownTarget, matchesShortcut } from "@/lib/keyboard/utils"; import { getShortcut } from "@/lib/keyboard/shortcut-overrides"; -/** Returns true if the active element is a text input or contenteditable. */ -function isEditableTarget(e: KeyboardEvent): boolean { - const tag = (e.target as HTMLElement)?.tagName; - return ( - tag === "INPUT" || tag === "TEXTAREA" || (e.target as HTMLElement)?.isContentEditable === true - ); -} - /** * App-root keyboard shortcuts that must fire on every route — not just inside * the dockview session editor. @@ -25,13 +17,27 @@ function isEditableTarget(e: KeyboardEvent): boolean { * * The AppSidebar is hidden below the `md` breakpoint; on mobile the toggle still * flips store state but has no visible effect, which is fine. + * + * Core-vs-plugin precedence: must be mounted (in `components/global-commands.tsx`) + * before `usePluginShortcuts()` so this capture-phase listener registers — and + * therefore runs — first. `usePluginShortcuts` bails out on + * `event.defaultPrevented`, so a combo bound to both a core shortcut here and a + * plugin keybinding fires only the core action. + * + * Full precedence chain (a matching combo fires exactly one action): + * central `useAppShortcuts` (this hook, capture phase, mounted first) wins + * over plugin keybindings (`usePluginShortcuts`, capture phase, mounted + * second), which win over per-component core shortcuts registered via + * `useKeyboardShortcut` (bubble phase) — that hook also bails out on + * `event.defaultPrevented`. */ export function useAppShortcuts() { const appStore = useAppStoreApi(); useEffect(() => { const handler = (e: KeyboardEvent) => { - if (isEditableTarget(e)) return; + if (e.defaultPrevented) return; + if (isEditableKeydownTarget(e)) return; const overrides = appStore.getState().userSettings.keyboardShortcuts; if (matchesShortcut(e, getShortcut("TOGGLE_SIDEBAR", overrides))) { diff --git a/apps/web/hooks/use-editor-keybinds.test.ts b/apps/web/hooks/use-editor-keybinds.test.ts new file mode 100644 index 0000000000..2e6580cd85 --- /dev/null +++ b/apps/web/hooks/use-editor-keybinds.test.ts @@ -0,0 +1,101 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { renderHook, cleanup } from "@testing-library/react"; +import type { DockviewApi } from "dockview-react"; + +const mockToggleAppSidebar = vi.fn(); +const mockToggleBottomTerminal = vi.fn(); + +vi.mock("@/components/state-provider", () => ({ + useAppStoreApi: () => ({ + getState: () => ({ + userSettings: { keyboardShortcuts: {} }, + bottomTerminal: { isOpen: false }, + toggleAppSidebar: mockToggleAppSidebar, + toggleBottomTerminal: mockToggleBottomTerminal, + tasks: { activeTaskId: null, activeSessionId: null }, + environmentIdBySessionId: {}, + }), + }), +})); + +import { useEditorKeybinds } from "./use-editor-keybinds"; +import { useDockviewStore } from "@/lib/state/dockview-store"; + +function pressKey( + key: string, + init: KeyboardEventInit = {}, + target: EventTarget = window, +): KeyboardEvent { + const event = new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true, ...init }); + target.dispatchEvent(event); + return event; +} + +/** + * Minimal fake `DockviewApi` exercising only the tab-navigation branch of + * `useEditorKeybinds`: two panels in the active panel's group, so + * `handleTabNavigation` can find the "next" panel and call its + * `api.setActive()`. + */ +function makeFakeApi(activateSpy: (id: string) => void): DockviewApi { + type FakePanel = { id: string; api: { setActive: () => void }; group?: { panels: FakePanel[] } }; + const panelA: FakePanel = { id: "panel-a", api: { setActive: () => activateSpy("panel-a") } }; + const panelB: FakePanel = { id: "panel-b", api: { setActive: () => activateSpy("panel-b") } }; + const group = { panels: [panelA, panelB] }; + panelA.group = group; + panelB.group = group; + + return { + activePanel: panelA, + } as unknown as DockviewApi; +} + +describe("useEditorKeybinds", () => { + afterEach(() => { + cleanup(); + useDockviewStore.setState({ api: null }); + mockToggleAppSidebar.mockClear(); + mockToggleBottomTerminal.mockClear(); + }); + + it("does not run the editor action (tab navigation) when the event is already defaultPrevented", () => { + // Regression: `useEditorKeybinds`'s capture-phase handler guards on + // `event.defaultPrevented` so a combo already handled by another + // capture-phase core dispatcher or a plugin keybinding doesn't also run + // the editor's tab-navigation/terminal-toggle logic (double action for a + // single keypress). This exercises that guard directly, at the hook + // level, rather than only through `useKeyboardShortcut`. + const activateSpy = vi.fn(); + useDockviewStore.setState({ api: makeFakeApi(activateSpy) }); + + // Register this capture-phase listener BEFORE mounting the hook so it + // runs first (capture-phase listeners on the same target fire in + // registration order) — mirroring a real prior capture dispatcher (a + // core shortcut or plugin keybinding) that already handled the event. + const preventDefaultListener = (event: KeyboardEvent) => { + event.preventDefault(); + }; + window.addEventListener("keydown", preventDefaultListener, { capture: true }); + + renderHook(() => useEditorKeybinds()); + + try { + pressKey("]", { ctrlKey: true, shiftKey: true, code: "BracketRight" }); + + expect(activateSpy).not.toHaveBeenCalled(); + } finally { + window.removeEventListener("keydown", preventDefaultListener, { capture: true }); + } + }); + + it("runs the editor action (tab navigation) for a non-prevented matching event", () => { + const activateSpy = vi.fn(); + useDockviewStore.setState({ api: makeFakeApi(activateSpy) }); + + renderHook(() => useEditorKeybinds()); + + pressKey("]", { ctrlKey: true, shiftKey: true, code: "BracketRight" }); + + expect(activateSpy).toHaveBeenCalledWith("panel-b"); + }); +}); diff --git a/apps/web/hooks/use-editor-keybinds.ts b/apps/web/hooks/use-editor-keybinds.ts index c4a39c9473..e6e96d5cd8 100644 --- a/apps/web/hooks/use-editor-keybinds.ts +++ b/apps/web/hooks/use-editor-keybinds.ts @@ -136,6 +136,12 @@ export function useEditorKeybinds() { useEffect(() => { const handler = (e: KeyboardEvent) => { + // Another capture-phase core dispatcher (`useAppShortcuts`) or a + // plugin keybinding (`usePluginShortcuts`) may have already handled + // and prevented this event — bail out so a combo shared with tab + // navigation, terminal toggle, or BOTTOM_TERMINAL fires only once. + if (e.defaultPrevented) return; + const api = useDockviewStore.getState().api; if (!api) return; diff --git a/apps/web/hooks/use-keyboard-shortcut.test.ts b/apps/web/hooks/use-keyboard-shortcut.test.ts new file mode 100644 index 0000000000..838a1e80a3 --- /dev/null +++ b/apps/web/hooks/use-keyboard-shortcut.test.ts @@ -0,0 +1,51 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { renderHook, cleanup } from "@testing-library/react"; +import { useKeyboardShortcut } from "./use-keyboard-shortcut"; +import type { KeyboardShortcut } from "@/lib/keyboard/constants"; + +const SHORTCUT: KeyboardShortcut = { key: "k", modifiers: { ctrlOrCmd: true, shift: true } }; + +function pressKey( + key: string, + init: KeyboardEventInit = {}, + target: EventTarget = window, +): KeyboardEvent { + const event = new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true, ...init }); + target.dispatchEvent(event); + return event; +} + +describe("useKeyboardShortcut", () => { + afterEach(() => cleanup()); + + it("invokes the callback when the combo matches", () => { + const callback = vi.fn(); + renderHook(() => useKeyboardShortcut(SHORTCUT, callback)); + + pressKey("k", { ctrlKey: true, shiftKey: true }); + + expect(callback).toHaveBeenCalledTimes(1); + }); + + it("does not invoke the callback when the event was already handled (defaultPrevented)", () => { + // Regression: a bubble-phase useKeyboardShortcut listener must not act on + // an event a capture-phase dispatcher (e.g. a plugin keybinding handler) + // already handled and called preventDefault() on — otherwise the same + // combo fires two actions instead of one. + const callback = vi.fn(); + renderHook(() => useKeyboardShortcut(SHORTCUT, callback)); + + const preventDefaultListener = (event: KeyboardEvent) => { + event.preventDefault(); + }; + window.addEventListener("keydown", preventDefaultListener, { capture: true }); + + try { + pressKey("k", { ctrlKey: true, shiftKey: true }); + + expect(callback).not.toHaveBeenCalled(); + } finally { + window.removeEventListener("keydown", preventDefaultListener, { capture: true }); + } + }); +}); diff --git a/apps/web/hooks/use-keyboard-shortcut.ts b/apps/web/hooks/use-keyboard-shortcut.ts index be10ea6255..50afc14a72 100644 --- a/apps/web/hooks/use-keyboard-shortcut.ts +++ b/apps/web/hooks/use-keyboard-shortcut.ts @@ -47,6 +47,13 @@ export function useKeyboardShortcut( const handleKeyDown = useCallback( (event: KeyboardEvent) => { if (!enabled) return; + // Precedence: a capture-phase core dispatcher (`useAppShortcuts`) or a + // plugin keybinding (`usePluginShortcuts`) may have already handled and + // prevented this event before this bubble-phase, per-component listener + // runs. Bail out so the same combo fires exactly one action instead of + // both. See `useAppShortcuts`/`usePluginShortcuts` for the full + // core-vs-plugin precedence chain. + if (event.defaultPrevented) return; if (matchesShortcut(event, shortcut)) { if (preventDefault) { diff --git a/apps/web/hooks/use-plugin-shortcuts.test.ts b/apps/web/hooks/use-plugin-shortcuts.test.ts new file mode 100644 index 0000000000..9b651f3024 --- /dev/null +++ b/apps/web/hooks/use-plugin-shortcuts.test.ts @@ -0,0 +1,388 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderHook, cleanup } from "@testing-library/react"; +import type { StoredShortcutOverrides } from "@/lib/keyboard/shortcut-overrides"; +import type { PluginRecord } from "@/lib/types/plugins"; +import { pluginRegistry } from "@/lib/plugins/registry"; + +let mockOverrides: StoredShortcutOverrides = {}; +let mockItems: PluginRecord[] = []; +const mockToggleAppSidebar = vi.fn(); + +vi.mock("@/components/state-provider", () => ({ + useAppStoreApi: () => ({ + getState: () => ({ + userSettings: { keyboardShortcuts: mockOverrides }, + toggleAppSidebar: mockToggleAppSidebar, + }), + }), +})); + +vi.mock("@/hooks/domains/plugins/use-plugins", () => ({ + usePlugins: () => ({ items: mockItems, loaded: true, loading: false, error: null }), +})); + +import { usePluginShortcuts } from "./use-plugin-shortcuts"; +import { useAppShortcuts } from "./use-app-shortcuts"; +import { useKeyboardShortcut } from "./use-keyboard-shortcut"; +import type { KeyboardShortcut } from "@/lib/keyboard/constants"; + +const PLUGIN_ID = "session-cost"; + +function makePlugin(overrides: Partial = {}): PluginRecord { + return { + id: PLUGIN_ID, + api_version: 1, + version: "1.0.0", + display_name: "Session Cost", + description: "", + author: "", + categories: [], + capabilities: {}, + status: "active", + install_path: "", + signed: true, + installed_at: "", + restart_count: 0, + ...overrides, + }; +} + +function pressKey( + key: string, + init: KeyboardEventInit = {}, + target: EventTarget = window, +): KeyboardEvent { + const event = new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true, ...init }); + target.dispatchEvent(event); + return event; +} + +const TOGGLE_KEYBINDING = { id: "toggle", default: "mod+shift+e", description: "Toggle" }; +/** A free combo (not used by any core shortcut) shared by two plugins bound to the same "open" id. */ +const OPEN_COMBO = "mod+shift+o"; + +function withToggleKeybinding(overrides: Partial = {}): PluginRecord { + return makePlugin({ ui: { keybindings: [TOGGLE_KEYBINDING] }, ...overrides }); +} + +describe("usePluginShortcuts", () => { + beforeEach(() => { + mockOverrides = {}; + mockItems = []; + mockToggleAppSidebar.mockClear(); + }); + + afterEach(() => { + cleanup(); + pluginRegistry.unregisterPlugin(PLUGIN_ID); + pluginRegistry.unregisterPlugin("plugin-a"); + pluginRegistry.unregisterPlugin("plugin-b"); + }); + + it("invokes the bound handler when the manifest default combo is pressed", () => { + mockItems = [withToggleKeybinding()]; + const handler = vi.fn(); + pluginRegistry.forPlugin(PLUGIN_ID).registerKeybinding("toggle", handler); + + renderHook(() => usePluginShortcuts()); + const event = pressKey("e", { ctrlKey: true, shiftKey: true }); + + expect(handler).toHaveBeenCalledTimes(1); + expect(event.defaultPrevented).toBe(true); + }); + + it("does not invoke the handler when no key matches", () => { + mockItems = [withToggleKeybinding()]; + const handler = vi.fn(); + pluginRegistry.forPlugin(PLUGIN_ID).registerKeybinding("toggle", handler); + + renderHook(() => usePluginShortcuts()); + pressKey("j", { ctrlKey: true }); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("skips dispatch while typing in an editable element", () => { + mockItems = [withToggleKeybinding()]; + const handler = vi.fn(); + pluginRegistry.forPlugin(PLUGIN_ID).registerKeybinding("toggle", handler); + + renderHook(() => usePluginShortcuts()); + const input = document.createElement("input"); + document.body.appendChild(input); + pressKey("e", { ctrlKey: true, shiftKey: true }, input); + input.remove(); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("does nothing when the plugin bundle has not registered a handler yet", () => { + mockItems = [withToggleKeybinding()]; + + renderHook(() => usePluginShortcuts()); + expect(() => pressKey("e", { ctrlKey: true, shiftKey: true })).not.toThrow(); + }); + + it("respects a user override combo instead of the manifest default", () => { + mockItems = [withToggleKeybinding()]; + mockOverrides = { + [`plugin:${PLUGIN_ID}:toggle`]: { key: "u", modifiers: { ctrlOrCmd: true } }, + }; + const handler = vi.fn(); + pluginRegistry.forPlugin(PLUGIN_ID).registerKeybinding("toggle", handler); + + renderHook(() => usePluginShortcuts()); + pressKey("e", { ctrlKey: true, shiftKey: true }); // manifest default — no longer bound + expect(handler).not.toHaveBeenCalled(); + + pressKey("u", { ctrlKey: true }); // user override — now bound + expect(handler).toHaveBeenCalledTimes(1); + }); + + it("dispatches to every plugin bound to the same combo, in registration order", () => { + mockItems = [ + makePlugin({ + id: "plugin-a", + display_name: "Plugin A", + ui: { keybindings: [{ id: "open", default: OPEN_COMBO, description: "Open" }] }, + }), + makePlugin({ + id: "plugin-b", + display_name: "Plugin B", + ui: { keybindings: [{ id: "open", default: OPEN_COMBO, description: "Open" }] }, + }), + ]; + const callOrder: string[] = []; + pluginRegistry.forPlugin("plugin-a").registerKeybinding("open", () => callOrder.push("a")); + pluginRegistry.forPlugin("plugin-b").registerKeybinding("open", () => callOrder.push("b")); + + renderHook(() => usePluginShortcuts()); + pressKey("o", { ctrlKey: true, shiftKey: true }); + + expect(callOrder).toEqual(["a", "b"]); + }); + + it("removes its listener on unmount", () => { + mockItems = [withToggleKeybinding()]; + const handler = vi.fn(); + pluginRegistry.forPlugin(PLUGIN_ID).registerKeybinding("toggle", handler); + + const { unmount } = renderHook(() => usePluginShortcuts()); + unmount(); + pressKey("e", { ctrlKey: true, shiftKey: true }); + + expect(handler).not.toHaveBeenCalled(); + }); +}); + +describe("usePluginShortcuts handler exceptions", () => { + beforeEach(() => { + mockOverrides = {}; + mockItems = []; + mockToggleAppSidebar.mockClear(); + }); + + afterEach(() => { + cleanup(); + pluginRegistry.unregisterPlugin("plugin-a"); + pluginRegistry.unregisterPlugin("plugin-b"); + }); + + it("logs and continues past a throwing handler so later handlers still run", () => { + mockItems = [ + makePlugin({ + id: "plugin-a", + display_name: "Plugin A", + ui: { keybindings: [{ id: "open", default: OPEN_COMBO, description: "Open" }] }, + }), + makePlugin({ + id: "plugin-b", + display_name: "Plugin B", + ui: { keybindings: [{ id: "open", default: OPEN_COMBO, description: "Open" }] }, + }), + ]; + const secondHandler = vi.fn(); + pluginRegistry.forPlugin("plugin-a").registerKeybinding("open", () => { + throw new Error("boom"); + }); + pluginRegistry.forPlugin("plugin-b").registerKeybinding("open", secondHandler); + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + renderHook(() => usePluginShortcuts()); + expect(() => pressKey("o", { ctrlKey: true, shiftKey: true })).not.toThrow(); + + expect(secondHandler).toHaveBeenCalledTimes(1); + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining("plugin-a"), + expect.any(Error), + ); + + consoleErrorSpy.mockRestore(); + }); +}); + +describe("core vs plugin shortcut precedence", () => { + beforeEach(() => { + mockOverrides = {}; + mockItems = []; + mockToggleAppSidebar.mockClear(); + }); + + afterEach(() => { + cleanup(); + pluginRegistry.unregisterPlugin(PLUGIN_ID); + }); + + it("fires only the core action when a combo matches both a core shortcut and a plugin keybinding", () => { + mockOverrides = { TOGGLE_SIDEBAR: { key: "b", modifiers: { ctrlOrCmd: true } } }; + mockItems = [ + makePlugin({ + ui: { keybindings: [{ id: "open", default: "mod+b", description: "Open" }] }, + }), + ]; + const pluginHandler = vi.fn(); + pluginRegistry.forPlugin(PLUGIN_ID).registerKeybinding("open", pluginHandler); + + renderHook(() => { + useAppShortcuts(); + usePluginShortcuts(); + }); + const event = pressKey("b", { ctrlKey: true }); + + expect(mockToggleAppSidebar).toHaveBeenCalledTimes(1); + expect(pluginHandler).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(true); + }); + + it("still fires the plugin handler for a plugin-only combo (no core shortcut match)", () => { + mockItems = [withToggleKeybinding()]; + const handler = vi.fn(); + pluginRegistry.forPlugin(PLUGIN_ID).registerKeybinding("toggle", handler); + + renderHook(() => { + useAppShortcuts(); + usePluginShortcuts(); + }); + pressKey("e", { ctrlKey: true, shiftKey: true }); + + expect(handler).toHaveBeenCalledTimes(1); + expect(mockToggleAppSidebar).not.toHaveBeenCalled(); + }); + + it("fires only the plugin action when a combo matches both a plugin keybinding and a per-component core shortcut bound via useKeyboardShortcut", () => { + // Regression for Greptile P1 (PR #1895, comment 3638620657): a + // per-component core shortcut registered through `useKeyboardShortcut` + // runs in the bubble phase, after this capture-phase plugin dispatcher. + // Without `useKeyboardShortcut` respecting `event.defaultPrevented`, the + // plugin handler AND the core action would both fire for the same combo. + mockItems = [withToggleKeybinding()]; + const pluginHandler = vi.fn(); + pluginRegistry.forPlugin(PLUGIN_ID).registerKeybinding("toggle", pluginHandler); + + const coreShortcut: KeyboardShortcut = { + key: "e", + modifiers: { ctrlOrCmd: true, shift: true }, + }; + const coreCallback = vi.fn(); + + renderHook(() => { + // usePluginShortcuts registers its capture-phase listener on mount, + // which — like the real app (`components/global-commands.tsx`) — + // fires before the bubble-phase useKeyboardShortcut listener below. + usePluginShortcuts(); + useKeyboardShortcut(coreShortcut, coreCallback); + }); + + const event = pressKey("e", { ctrlKey: true, shiftKey: true }); + + expect(pluginHandler).toHaveBeenCalledTimes(1); + expect(coreCallback).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(true); + }); +}); + +describe("core shortcut combo shadowing (robust, non-registry-aware core shortcuts)", () => { + beforeEach(() => { + mockOverrides = {}; + mockItems = []; + mockToggleAppSidebar.mockClear(); + }); + + afterEach(() => { + cleanup(); + pluginRegistry.unregisterPlugin(PLUGIN_ID); + }); + + it("does not invoke or preventDefault a plugin bound to FIND_IN_PANEL's combo, leaving core panel-search to handle it", () => { + // Regression for Greptile P1 (PR #1895, comment 3638902528): FIND_IN_PANEL + // (Cmd/Ctrl+F) is handled by independent per-panel capture-phase + // listeners (`use-panel-search.ts`) that never call `preventDefault` up + // front and don't check it either. The old "bail on defaultPrevented" + // guard couldn't protect this combo at all — only skipping the plugin + // handler outright (by combo, not by event state) fixes it. + mockItems = [ + makePlugin({ + ui: { keybindings: [{ id: "find", default: "mod+f", description: "Find" }] }, + }), + ]; + const pluginHandler = vi.fn(); + pluginRegistry.forPlugin(PLUGIN_ID).registerKeybinding("find", pluginHandler); + + renderHook(() => usePluginShortcuts()); + const event = pressKey("f", { ctrlKey: true }); + + expect(pluginHandler).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(false); + }); + + it("still fires a plugin bound to a genuinely free combo (no core shortcut uses it)", () => { + mockItems = [ + makePlugin({ + ui: { keybindings: [{ id: "open", default: "mod+shift+u", description: "Open" }] }, + }), + ]; + const pluginHandler = vi.fn(); + pluginRegistry.forPlugin(PLUGIN_ID).registerKeybinding("open", pluginHandler); + + renderHook(() => usePluginShortcuts()); + const event = pressKey("u", { ctrlKey: true, shiftKey: true }); + + expect(pluginHandler).toHaveBeenCalledTimes(1); + expect(event.defaultPrevented).toBe(true); + }); + + it("shadows the plugin by a user's REMAPPED core combo, not the old default", () => { + // A user remaps TASK_SWITCHER (default Cmd/Ctrl+Space) to Cmd/Ctrl+U. A + // plugin bound to the OLD default (mod+space) should now fire freely, + // and a plugin bound to the NEW combo (mod+u) should be shadowed. + mockOverrides = { TASK_SWITCHER: { key: "u", modifiers: { ctrlOrCmd: true } } }; + mockItems = [ + makePlugin({ + id: "plugin-old-combo", + display_name: "Old Combo Plugin", + ui: { keybindings: [{ id: "open", default: "mod+space", description: "Open" }] }, + }), + makePlugin({ + id: "plugin-new-combo", + display_name: "New Combo Plugin", + ui: { keybindings: [{ id: "open", default: "mod+u", description: "Open" }] }, + }), + ]; + const oldComboHandler = vi.fn(); + const newComboHandler = vi.fn(); + pluginRegistry.forPlugin("plugin-old-combo").registerKeybinding("open", oldComboHandler); + pluginRegistry.forPlugin("plugin-new-combo").registerKeybinding("open", newComboHandler); + + renderHook(() => usePluginShortcuts()); + + pressKey(" ", { ctrlKey: true }); // old TASK_SWITCHER default — no longer core, plugin fires + expect(oldComboHandler).toHaveBeenCalledTimes(1); + + const event = pressKey("u", { ctrlKey: true }); // new TASK_SWITCHER combo — shadows the plugin + expect(newComboHandler).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(false); + + pluginRegistry.unregisterPlugin("plugin-old-combo"); + pluginRegistry.unregisterPlugin("plugin-new-combo"); + }); +}); diff --git a/apps/web/hooks/use-plugin-shortcuts.ts b/apps/web/hooks/use-plugin-shortcuts.ts new file mode 100644 index 0000000000..3ec718059f --- /dev/null +++ b/apps/web/hooks/use-plugin-shortcuts.ts @@ -0,0 +1,194 @@ +"use client"; + +import { useEffect } from "react"; +import { useAppStoreApi } from "@/components/state-provider"; +import { usePlugins } from "@/hooks/domains/plugins/use-plugins"; +import { pluginRegistry, usePluginRegistry } from "@/lib/plugins/registry"; +import { isEditableKeydownTarget, isMac, matchesShortcut } from "@/lib/keyboard/utils"; +import type { StoredShortcutOverrides } from "@/lib/keyboard/shortcut-overrides"; +import { + buildConfigurableShortcutEntries, + coreShortcutEntries, + resolveShortcutEntry, +} from "@/lib/keyboard/plugin-shortcuts"; +import { comboKey } from "@/lib/keyboard/shortcut-conflicts"; +import { SHORTCUTS } from "@/lib/keyboard/constants"; + +/** + * Central shortcuts that are not in `CONFIGURABLE_SHORTCUTS` (so they have no + * user override to resolve) but are still global, always-on core behavior + * that must win over a plugin keybinding bound to the same combo: + * - `FIND_IN_PANEL` (Cmd/Ctrl+F) — per-panel capture-phase listeners + * (`use-panel-search.ts`, terminal find) that don't check + * `event.defaultPrevented`. + * - `SAVE` (Cmd/Ctrl+S) — reserved for editor save; must not be hijacked by a + * plugin even before a save listener exists for every surface. + * + * Other `SHORTCUTS` entries not in this list (`SUBMIT`, `SUBMIT_ENTER`, + * `CANCEL`, `COMMAND_PANEL_SHIFT`) are contextual/per-component shortcuts — + * typically registered via `useKeyboardShortcut`, which already yields to + * plugin keybindings — so they're intentionally excluded here. + */ +const NON_CONFIGURABLE_CORE_SHORTCUT_IDS = [ + "FIND_IN_PANEL", + "SAVE", +] as const satisfies ReadonlyArray; + +/** + * Global dispatcher for plugin-declared keybindings (`ui.keybindings`), + * mirroring `useAppShortcuts`'s capture-phase, editable-skip pattern. Mount + * once at the app root (alongside `useAppShortcuts`, see + * `components/global-commands.tsx`). + * + * Also syncs each active plugin's declared keybinding ids into + * `pluginRegistry` (`setDeclaredKeybindingIds`) so `registerKeybinding` can + * warn on an id the manifest never declared — this is the one call site with + * both the plugin records (`usePlugins`) and the registry. + * + * Dispatch order when two active plugins bind the same effective combo: + * registration order (`pluginRegistry.getKeybindingHandlers()` iteration + * order) — the first plugin to call `registerKeybinding` for that combo + * wins; later handlers for the same event still run since this dispatches + * to every match, not just the first. + * + * Core-vs-plugin precedence: effective CORE shortcuts always win over plugin + * keybindings. There is no single core keydown dispatcher — panel-search, + * task-switcher, save, editor keybinds, and other per-component listeners + * are all independent, and most of them don't (and shouldn't have to) check + * `event.defaultPrevented`. So instead of relying on every core listener + * cooperating, this hook computes the set of effective CORE shortcut combos + * (the central `SHORTCUTS`/`CONFIGURABLE_SHORTCUTS` registry, resolved + * through any user overrides — see `buildCoreComboKeySet` below) and simply + * never invokes a plugin handler whose effective combo is in that set. The + * plugin handler is skipped entirely — no invocation, no `preventDefault()` + * — so whichever core listener owns that combo runs normally, exactly once. + * This makes panel-search/task-switcher/save/find-in-panel/etc. win over + * plugins without editing any of those listeners, and it still works when a + * user remaps a core shortcut's combo (the override is read fresh on every + * keydown). + * + * Among non-core listeners, this capture-phase plugin dispatcher wins over + * per-component core shortcuts registered via `useKeyboardShortcut` (bubble + * phase) and `useEditorKeybinds`'s capture-phase handler: this hook calls + * `event.preventDefault()` whenever it invokes a plugin handler for a + * non-core combo, and those listeners bail out when `event.defaultPrevented` + * is already true. So the full chain — effective core shortcuts (always win) + * > plugin keybindings > per-component core shortcuts — resolves to exactly + * one action per combo. + */ +export function usePluginShortcuts(): void { + const appStore = useAppStoreApi(); + const { items } = usePlugins(); + usePluginRegistry(); + + useEffect(() => { + for (const plugin of items) { + pluginRegistry.setDeclaredKeybindingIds( + plugin.id, + (plugin.ui?.keybindings ?? []).map((kb) => kb.id), + ); + } + }, [items]); + + useEffect(() => { + const handler = (event: KeyboardEvent) => { + // A core dispatcher already handled this combo (and called + // preventDefault) — core wins, so bail out without invoking any + // plugin handler for the same keypress. + if (event.defaultPrevented) return; + if (isEditableKeydownTarget(event)) return; + + const overrides = appStore.getState().userSettings + .keyboardShortcuts as StoredShortcutOverrides; + dispatchMatchingPluginShortcuts(event, items, overrides); + }; + + // Capture phase so plugin shortcuts win before focus-trapped surfaces + // (e.g. xterm.js) can swallow the event — mirrors useAppShortcuts. + window.addEventListener("keydown", handler, true); + return () => window.removeEventListener("keydown", handler, true); + }, [appStore, items]); +} + +/** + * Builds the set of effective CORE shortcut combo-keys (registry defaults, + * resolved through any user override), so the dispatcher can recognize a + * plugin combo that shadows a core one. Recomputed on every keydown from the + * `overrides` snapshot read in the handler, so a user remapping a core + * shortcut's combo takes effect immediately — no separate memoization + * lifecycle to keep in sync with the override store. + */ +function buildCoreComboKeySet( + overrides: StoredShortcutOverrides, + isMacPlatform: boolean, +): Set { + const keys = new Set(); + for (const entry of coreShortcutEntries()) { + const shortcut = resolveShortcutEntry(entry, overrides); + const key = comboKey(shortcut, isMacPlatform); + if (key !== null) keys.add(key); + } + for (const id of NON_CONFIGURABLE_CORE_SHORTCUT_IDS) { + const key = comboKey(SHORTCUTS[id], isMacPlatform); + if (key !== null) keys.add(key); + } + return keys; +} + +/** + * Dispatches `event` to every registered plugin keybinding handler whose + * effective combo matches. Iterates in `pluginRegistry.getKeybindingHandlers()` + * order — registration order — so when two plugins bind the same combo, the + * plugin that called `registerKeybinding` first runs first (both still run; + * this only fixes the order, it does not stop at the first match). + * + * Before invoking a handler, checks whether that plugin combo's `comboKey` + * matches an effective CORE shortcut (see `buildCoreComboKeySet`). If it + * does, the plugin handler is skipped entirely — core always wins, and the + * corresponding core listener (panel-search, task-switcher, save, etc.) + * handles the event normally. + * + * Each handler invocation is isolated in its own try/catch: a throwing + * handler is logged (with the offending plugin/keybinding id) and does not + * abort the loop, so one broken plugin can't prevent other plugins bound to + * the same combo from running. + */ +function dispatchMatchingPluginShortcuts( + event: KeyboardEvent, + plugins: Parameters[0], + overrides: StoredShortcutOverrides, +): void { + const entryById = new Map( + buildConfigurableShortcutEntries(plugins) + .filter((entry) => entry.source === "plugin") + .map((entry) => [entry.id, entry] as const), + ); + + const isMacPlatform = isMac(); + const coreComboKeys = buildCoreComboKeySet(overrides, isMacPlatform); + + for (const { pluginId, id, handler } of pluginRegistry.getKeybindingHandlers()) { + const entry = entryById.get(`plugin:${pluginId}:${id}`); + if (!entry) continue; + + const shortcut = resolveShortcutEntry(entry, overrides); + if (!matchesShortcut(event, shortcut)) continue; + + const pluginComboKey = comboKey(shortcut, isMacPlatform); + if (pluginComboKey !== null && coreComboKeys.has(pluginComboKey)) { + if (process.env.NODE_ENV !== "production") { + console.warn( + `[plugin:${pluginId}] keybinding "${id}" is shadowed by a core shortcut bound to the same combo and will not fire.`, + ); + } + continue; + } + + event.preventDefault(); + try { + handler(event); + } catch (err) { + console.error(`[plugin:${pluginId}] keybinding "${id}" handler threw an error:`, err); + } + } +} diff --git a/apps/web/lib/keyboard/parse-combo.test.ts b/apps/web/lib/keyboard/parse-combo.test.ts new file mode 100644 index 0000000000..e791b57b56 --- /dev/null +++ b/apps/web/lib/keyboard/parse-combo.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect } from "vitest"; +import { parseCombo } from "./parse-combo"; + +describe("parseCombo", () => { + it("parses a plain single-key combo", () => { + expect(parseCombo("k")).toEqual({ key: "k" }); + }); + + it("parses mod+key as ctrlOrCmd (platform resolved by matchesShortcut)", () => { + expect(parseCombo("mod+k")).toEqual({ key: "k", modifiers: { ctrlOrCmd: true } }); + }); + + it("parses multiple modifiers plus one key", () => { + expect(parseCombo("mod+shift+k")).toEqual({ + key: "k", + modifiers: { ctrlOrCmd: true, shift: true }, + }); + }); + + it("treats ctrl as an explicit ctrl modifier (not ctrlOrCmd)", () => { + expect(parseCombo("ctrl+r")).toEqual({ key: "r", modifiers: { ctrl: true } }); + }); + + it("treats cmd/meta as an explicit cmd modifier", () => { + expect(parseCombo("cmd+k")).toEqual({ key: "k", modifiers: { cmd: true } }); + expect(parseCombo("meta+k")).toEqual({ key: "k", modifiers: { cmd: true } }); + }); + + it("returns null for undocumented modifier aliases (control/super are not in the accepted grammar)", () => { + expect(parseCombo("control+r")).toBeNull(); + expect(parseCombo("super+k")).toBeNull(); + }); + + it("treats alt/option as an explicit alt modifier", () => { + expect(parseCombo("alt+k")).toEqual({ key: "k", modifiers: { alt: true } }); + expect(parseCombo("option+k")).toEqual({ key: "k", modifiers: { alt: true } }); + }); + + it("resolves named key tokens to KeyboardEvent.key equivalents", () => { + expect(parseCombo("mod+enter")).toEqual({ key: "Enter", modifiers: { ctrlOrCmd: true } }); + expect(parseCombo("shift+arrowup")).toEqual({ key: "ArrowUp", modifiers: { shift: true } }); + }); + + it("resolves a symbol key token when shift is not involved", () => { + expect(parseCombo("mod+slash")).toEqual({ key: "/", modifiers: { ctrlOrCmd: true } }); + }); + + it("returns null when shift is combined with a key whose character shift would alter", () => { + // Physically holding Shift means the browser reports "?"/"!" for + // KeyboardEvent.key, not "/"/"1" — so these combos could never dispatch. + expect(parseCombo("shift+slash")).toBeNull(); + expect(parseCombo("mod+shift+slash")).toBeNull(); + expect(parseCombo("shift+1")).toBeNull(); + expect(parseCombo("shift+minus")).toBeNull(); + }); + + it("still allows shift with keys Shift doesn't alter (letters, named/function keys)", () => { + expect(parseCombo("shift+k")).toEqual({ key: "k", modifiers: { shift: true } }); + expect(parseCombo("shift+tab")).toEqual({ key: "Tab", modifiers: { shift: true } }); + expect(parseCombo("shift+f1")).toEqual({ key: "f1", modifiers: { shift: true } }); + }); + + it("resolves function keys", () => { + expect(parseCombo("mod+f5")).toEqual({ key: "f5", modifiers: { ctrlOrCmd: true } }); + }); + + it("is case-insensitive on tokens", () => { + expect(parseCombo("MOD+SHIFT+K")).toEqual({ + key: "k", + modifiers: { ctrlOrCmd: true, shift: true }, + }); + }); + + it("returns null for an empty combo", () => { + expect(parseCombo("")).toBeNull(); + expect(parseCombo(" ")).toBeNull(); + }); + + it("returns null for an unknown token", () => { + expect(parseCombo("mod+banana")).toBeNull(); + }); + + it("returns null when the combo has more than one non-modifier key", () => { + expect(parseCombo("k+j")).toBeNull(); + }); + + it("returns null when the combo has only modifiers and no key", () => { + expect(parseCombo("mod+shift")).toBeNull(); + }); + + it("returns null for a token with an empty segment", () => { + expect(parseCombo("mod++k")).toBeNull(); + }); +}); diff --git a/apps/web/lib/keyboard/parse-combo.ts b/apps/web/lib/keyboard/parse-combo.ts new file mode 100644 index 0000000000..559b02f950 --- /dev/null +++ b/apps/web/lib/keyboard/parse-combo.ts @@ -0,0 +1,156 @@ +/** + * Parses a plugin manifest keybinding combo string (e.g. "mod+shift+k") into + * the core `KeyboardShortcut` shape consumed by `matchesShortcut`. + * + * Grammar mirrors the backend validator + * (`apps/backend/internal/plugins/manifest/validate.go` `parseKeybindingCombo`), + * which already rejected malformed combos at plugin registration time: + * `+`-separated tokens, modifiers from a fixed vocabulary, and exactly one + * non-modifier key token. This parser is defensive (returns `null` instead of + * throwing) since it also runs against values that already passed backend + * validation. + * + * The accepted modifier vocabulary — `mod|ctrl|cmd|meta|alt|option|shift` — + * matches the public contract in `docs/plans/plugins/PLUGIN-API.md` exactly. + * "control"/"super" are deliberately NOT accepted (kept in lockstep with the + * backend validator, which also rejects them). + * + * `mod` maps to `ctrlOrCmd` in the `KeyboardShortcut` shape — `matchesShortcut` + * already resolves `ctrlOrCmd` to Cmd on macOS / Ctrl elsewhere, so no + * separate platform branch is needed here. + * + * Combos pairing `shift` with a key whose printable character Shift would + * alter (digits, symbols — see `SHIFT_ALTERED_KEY_TOKENS`) are rejected: the + * combo is matched against `KeyboardEvent.key`, but a physically-held Shift + * means the browser reports the shifted character ("!", "?") instead of the + * unshifted token, so the combo could never actually dispatch. This mirrors + * the backend validator's `shiftAlteredKeys` rejection. + */ +import type { Key, KeyboardShortcut } from "./constants"; + +type ModifierToken = "mod" | "ctrl" | "cmd" | "meta" | "alt" | "option" | "shift"; + +const MODIFIER_TOKENS: Record> = { + mod: "ctrlOrCmd", + ctrl: "ctrl", + cmd: "cmd", + meta: "cmd", + alt: "alt", + option: "alt", + shift: "shift", +}; + +/** + * Non-modifier key tokens whose reported `KeyboardEvent.key` changes when + * Shift is held (e.g. "1" -> "!", "slash" -> "?"). Must stay in sync with the + * backend's `shiftAlteredKeys` in + * `apps/backend/internal/plugins/manifest/validate.go`. + */ +const SHIFT_ALTERED_KEY_TOKENS = new Set([ + "comma", + "period", + "slash", + "backslash", + "semicolon", + "quote", + "minus", + "equal", + "bracketleft", + "bracketright", + "backquote", + "0", + "1", + "2", + "3", + "4", + "5", + "6", + "7", + "8", + "9", +]); + +/** Named-key tokens mapped to their `KeyboardEvent.key` equivalent. */ +const NAMED_KEY_TOKENS: Record = { + enter: "Enter", + escape: "Escape", + esc: "Escape", + tab: "Tab", + space: " ", + backspace: "Backspace", + delete: "Delete", + insert: "Insert", + arrowup: "ArrowUp", + arrowdown: "ArrowDown", + arrowleft: "ArrowLeft", + arrowright: "ArrowRight", + up: "ArrowUp", + down: "ArrowDown", + left: "ArrowLeft", + right: "ArrowRight", + home: "Home", + end: "End", + pageup: "PageUp", + pagedown: "PageDown", + comma: ",", + period: ".", + slash: "/", + backslash: "\\", + semicolon: ";", + quote: "'", + minus: "-", + equal: "=", + bracketleft: "[", + bracketright: "]", + backquote: "`", +}; + +function isSingleAlphanumeric(token: string): boolean { + return /^[a-z0-9]$/.test(token); +} + +function isFunctionKey(token: string): boolean { + return /^f([1-9]|1[0-2])$/.test(token); +} + +/** Resolves a single non-modifier token to a `KeyboardShortcut["key"]`, or null if unrecognized. */ +function resolveKeyToken(token: string): Key | null { + if (token in NAMED_KEY_TOKENS) return NAMED_KEY_TOKENS[token] as Key; + if (isSingleAlphanumeric(token) || isFunctionKey(token)) return token as Key; + return null; +} + +/** + * Parses a combo string like "mod+shift+k" into a `KeyboardShortcut`. + * Returns null when the combo is empty, contains an unknown token, does not + * contain exactly one non-modifier key token, or pairs `shift` with a key + * whose reported character shift would alter (see `SHIFT_ALTERED_KEY_TOKENS`). + */ +export function parseCombo(combo: string): KeyboardShortcut | null { + if (!combo || !combo.trim()) return null; + + const tokens = combo.split("+").map((token) => token.trim().toLowerCase()); + const modifiers: NonNullable = {}; + let key: Key | null = null; + let keyToken: string | null = null; + + for (const token of tokens) { + if (!token) return null; + + if (token in MODIFIER_TOKENS) { + modifiers[MODIFIER_TOKENS[token as ModifierToken]] = true; + continue; + } + + const resolvedKey = resolveKeyToken(token); + if (!resolvedKey) return null; + if (key !== null) return null; // more than one non-modifier key token + key = resolvedKey; + keyToken = token; + } + + if (key === null || keyToken === null) return null; + if (modifiers.shift && SHIFT_ALTERED_KEY_TOKENS.has(keyToken)) return null; + + return Object.keys(modifiers).length > 0 ? { key, modifiers } : { key }; +} diff --git a/apps/web/lib/keyboard/plugin-shortcuts.test.ts b/apps/web/lib/keyboard/plugin-shortcuts.test.ts new file mode 100644 index 0000000000..3be222a773 --- /dev/null +++ b/apps/web/lib/keyboard/plugin-shortcuts.test.ts @@ -0,0 +1,109 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import type { PluginRecord } from "@/lib/types/plugins"; +import { + buildConfigurableShortcutEntries, + pluginShortcutId, + resolveShortcutEntry, + type ShortcutEntry, +} from "./plugin-shortcuts"; + +const PLUGIN_ID = "session-cost"; +const KEYBINDING_ID = "toggle-panel"; +const NAMESPACED_ID = pluginShortcutId(PLUGIN_ID, KEYBINDING_ID); + +function makePlugin(overrides: Partial = {}): PluginRecord { + return { + id: PLUGIN_ID, + api_version: 1, + version: "1.0.0", + display_name: "Session Cost", + description: "", + author: "", + categories: [], + capabilities: {}, + status: "active", + install_path: "", + signed: true, + installed_at: "", + restart_count: 0, + ...overrides, + }; +} + +function makeTogglePanelEntry(): ShortcutEntry { + return { + source: "plugin", + id: NAMESPACED_ID, + label: "Session Cost: Toggle panel", + default: { key: "k", modifiers: { ctrlOrCmd: true } }, + pluginId: PLUGIN_ID, + keybindingId: KEYBINDING_ID, + }; +} + +describe("pluginShortcutId", () => { + it("namespaces the id with the plugin id", () => { + expect(pluginShortcutId(PLUGIN_ID, KEYBINDING_ID)).toBe(NAMESPACED_ID); + }); +}); + +describe("buildConfigurableShortcutEntries", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("includes core entries followed by plugin entries", () => { + const plugin = makePlugin({ + ui: { + keybindings: [{ id: KEYBINDING_ID, default: "mod+shift+k", description: "Toggle panel" }], + }, + }); + + const entries = buildConfigurableShortcutEntries([plugin]); + const coreEntry = entries.find((e) => e.source === "core" && e.id === "SEARCH"); + const pluginEntry = entries.find((e) => e.source === "plugin" && e.id === NAMESPACED_ID); + + expect(coreEntry).toBeDefined(); + expect(pluginEntry).toMatchObject({ + source: "plugin", + id: NAMESPACED_ID, + label: "Session Cost: Toggle panel", + default: { key: "k", modifiers: { ctrlOrCmd: true, shift: true } }, + pluginId: PLUGIN_ID, + keybindingId: KEYBINDING_ID, + }); + }); + + it("skips a plugin with no declared keybindings", () => { + const plugin = makePlugin({ ui: { bundle: "ui/bundle.js" } }); + const entries = buildConfigurableShortcutEntries([plugin]); + expect(entries.some((e) => e.source === "plugin")).toBe(false); + }); + + it("warns and skips a keybinding with an unparseable default combo", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const plugin = makePlugin({ + ui: { keybindings: [{ id: "bad", default: "banana", description: "Bad" }] }, + }); + + const entries = buildConfigurableShortcutEntries([plugin]); + + expect(entries.some((e) => e.source === "plugin")).toBe(false); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("bad")); + }); +}); + +describe("resolveShortcutEntry", () => { + it("returns the default when no override is set", () => { + const entry = makeTogglePanelEntry(); + + expect(resolveShortcutEntry(entry)).toEqual(entry.default); + }); + + it("returns the user override when set", () => { + const entry = makeTogglePanelEntry(); + const override = { key: "j", modifiers: { shift: true } }; + + expect(resolveShortcutEntry(entry, { [entry.id]: override })).toEqual(override); + }); +}); diff --git a/apps/web/lib/keyboard/plugin-shortcuts.ts b/apps/web/lib/keyboard/plugin-shortcuts.ts new file mode 100644 index 0000000000..5d26de5ac6 --- /dev/null +++ b/apps/web/lib/keyboard/plugin-shortcuts.ts @@ -0,0 +1,109 @@ +/** + * Dynamic layer over `CONFIGURABLE_SHORTCUTS`: builds the same + * label/default shape for plugin-declared keybindings (`ui.keybindings` on a + * `PluginRecord`), so Settings > Keyboard Shortcuts can render and + * user-override plugin shortcuts alongside the static core list without + * widening `ConfigurableShortcutId` to an open string type. + * + * Namespacing: a plugin keybinding's effective override id is + * `plugin:{pluginId}:{keybindingId}` — this is what gets stored under + * `userSettings.keyboardShortcuts` (the same `StoredShortcutOverrides` map + * core shortcuts already use). + */ +import type { PluginRecord } from "@/lib/types/plugins"; +import type { KeyboardShortcut } from "./constants"; +import { parseCombo } from "./parse-combo"; +import { + CONFIGURABLE_SHORTCUTS, + type ConfigurableShortcutId, + type StoredShortcutOverrides, +} from "./shortcut-overrides"; + +/** Prefix marking a namespaced id as belonging to a plugin, not core. */ +export const PLUGIN_SHORTCUT_PREFIX = "plugin:"; + +export function pluginShortcutId(pluginId: string, keybindingId: string): string { + return `${PLUGIN_SHORTCUT_PREFIX}${pluginId}:${keybindingId}`; +} + +/** One configurable shortcut entry, from either the core static list or a plugin manifest. */ +export type ShortcutEntry = + | { source: "core"; id: ConfigurableShortcutId; label: string; default: KeyboardShortcut } + | { + source: "plugin"; + id: string; + label: string; + default: KeyboardShortcut; + pluginId: string; + keybindingId: string; + }; + +/** + * The static core shortcuts, in `ShortcutEntry` shape — exported so callers + * that already hold a `ShortcutEntry[]` for plugins (e.g. + * `KeyboardShortcutsCard`, which receives plugin entries as a prop rather + * than raw `PluginRecord[]`) can merge core + plugin entries without + * re-deriving the core list themselves. + */ +export function coreShortcutEntries(): ShortcutEntry[] { + return (Object.keys(CONFIGURABLE_SHORTCUTS) as ConfigurableShortcutId[]).map((id) => ({ + source: "core", + id, + label: CONFIGURABLE_SHORTCUTS[id].label, + default: CONFIGURABLE_SHORTCUTS[id].default, + })); +} + +/** + * Builds the plugin-sourced configurable shortcut entries for every active + * plugin's declared `ui.keybindings`. A combo that fails to parse (should + * never happen — the backend validates manifests at registration) is + * skipped with a console warning rather than throwing. + */ +function buildPluginEntries(plugins: PluginRecord[]): ShortcutEntry[] { + const entries: ShortcutEntry[] = []; + for (const plugin of plugins) { + for (const keybinding of plugin.ui?.keybindings ?? []) { + const shortcut = parseCombo(keybinding.default); + if (!shortcut) { + console.warn( + `[plugins] "${plugin.id}" keybinding "${keybinding.id}" has an unparseable default combo "${keybinding.default}"`, + ); + continue; + } + entries.push({ + source: "plugin", + id: pluginShortcutId(plugin.id, keybinding.id), + label: `${plugin.display_name}: ${keybinding.description}`, + default: shortcut, + pluginId: plugin.id, + keybindingId: keybinding.id, + }); + } + } + return entries; +} + +/** + * The full list of configurable shortcuts — core (static) entries followed + * by plugin (dynamic) entries — for rendering in Settings > Keyboard + * Shortcuts. Core behavior/order is unchanged from `CONFIGURABLE_SHORTCUTS`. + */ +export function buildConfigurableShortcutEntries(plugins: PluginRecord[]): ShortcutEntry[] { + return [...coreShortcutEntries(), ...buildPluginEntries(plugins)]; +} + +/** Builds only the plugin-sourced entries — used by callers that already have core entries. */ +export function buildPluginShortcutEntries(plugins: PluginRecord[]): ShortcutEntry[] { + return buildPluginEntries(plugins); +} + +/** Resolves an entry's effective shortcut: the user override if set, else its default. */ +export function resolveShortcutEntry( + entry: ShortcutEntry, + overrides?: StoredShortcutOverrides, +): KeyboardShortcut { + const override = overrides?.[entry.id]; + if (override) return override as KeyboardShortcut; + return entry.default; +} diff --git a/apps/web/lib/keyboard/shortcut-conflicts.test.ts b/apps/web/lib/keyboard/shortcut-conflicts.test.ts new file mode 100644 index 0000000000..83d6b9fe36 --- /dev/null +++ b/apps/web/lib/keyboard/shortcut-conflicts.test.ts @@ -0,0 +1,103 @@ +import { describe, it, expect } from "vitest"; +import { comboKey, findShortcutConflicts } from "./shortcut-conflicts"; +import type { ShortcutEntry } from "./plugin-shortcuts"; + +function coreEntry(id: string, shortcut: ShortcutEntry["default"]): ShortcutEntry { + return { source: "core", id: id as never, label: id, default: shortcut }; +} + +function pluginEntry( + pluginId: string, + keybindingId: string, + shortcut: ShortcutEntry["default"], +): ShortcutEntry { + return { + source: "plugin", + id: `plugin:${pluginId}:${keybindingId}`, + label: `${pluginId}: ${keybindingId}`, + default: shortcut, + pluginId, + keybindingId, + }; +} + +describe("comboKey", () => { + it("resolves ctrlOrCmd to cmd on mac and ctrl elsewhere", () => { + const shortcut = { key: "k" as const, modifiers: { ctrlOrCmd: true } }; + expect(comboKey(shortcut, true)).toBe(comboKey({ key: "k", modifiers: { cmd: true } }, true)); + expect(comboKey(shortcut, false)).toBe( + comboKey({ key: "k", modifiers: { ctrl: true } }, false), + ); + }); + + it("returns null for an unbound shortcut", () => { + expect(comboKey({ key: "" as never }, true)).toBeNull(); + }); +}); + +describe("findShortcutConflicts", () => { + it("detects a plugin-vs-core conflict", () => { + const search = coreEntry("SEARCH", { key: "k", modifiers: { ctrlOrCmd: true } }); + const plugin = pluginEntry("session-cost", "open", { + key: "k", + modifiers: { ctrlOrCmd: true }, + }); + + const conflicts = findShortcutConflicts( + [ + { entry: search, shortcut: search.default }, + { entry: plugin, shortcut: plugin.default }, + ], + true, + ); + + expect(conflicts).toHaveLength(1); + expect(conflicts[0].entries).toEqual([search, plugin]); + }); + + it("detects a plugin-vs-plugin conflict", () => { + const pluginA = pluginEntry("plugin-a", "open", { key: "j", modifiers: { shift: true } }); + const pluginB = pluginEntry("plugin-b", "open", { key: "j", modifiers: { shift: true } }); + + const conflicts = findShortcutConflicts( + [ + { entry: pluginA, shortcut: pluginA.default }, + { entry: pluginB, shortcut: pluginB.default }, + ], + true, + ); + + expect(conflicts).toHaveLength(1); + expect(conflicts[0].entries).toEqual([pluginA, pluginB]); + }); + + it("reports no conflicts when combos differ", () => { + const pluginA = pluginEntry("plugin-a", "open", { key: "j" }); + const pluginB = pluginEntry("plugin-b", "open", { key: "k" }); + + const conflicts = findShortcutConflicts( + [ + { entry: pluginA, shortcut: pluginA.default }, + { entry: pluginB, shortcut: pluginB.default }, + ], + true, + ); + + expect(conflicts).toEqual([]); + }); + + it("does not flag two unbound shortcuts as conflicting", () => { + const pluginA = pluginEntry("plugin-a", "open", { key: "" as never }); + const pluginB = pluginEntry("plugin-b", "open", { key: "" as never }); + + const conflicts = findShortcutConflicts( + [ + { entry: pluginA, shortcut: pluginA.default }, + { entry: pluginB, shortcut: pluginB.default }, + ], + true, + ); + + expect(conflicts).toEqual([]); + }); +}); diff --git a/apps/web/lib/keyboard/shortcut-conflicts.ts b/apps/web/lib/keyboard/shortcut-conflicts.ts new file mode 100644 index 0000000000..a0b2c00898 --- /dev/null +++ b/apps/web/lib/keyboard/shortcut-conflicts.ts @@ -0,0 +1,62 @@ +/** + * Pure conflict detection over a list of resolved (effective) keyboard + * shortcuts — used to warn about core-vs-plugin and plugin-vs-plugin combo + * collisions in Settings > Keyboard Shortcuts. Detection only; UI surfacing + * is a lightweight per-row indicator (see `keyboard-shortcuts-card.tsx`). + */ +import type { KeyboardShortcut } from "./constants"; +import { isUnboundShortcut } from "./shortcut-overrides"; +import type { ShortcutEntry } from "./plugin-shortcuts"; + +/** + * Canonicalizes a shortcut to a comparable string, resolving `ctrlOrCmd` to + * the concrete modifier the given platform would fire (mirrors + * `matchesShortcut`'s runtime behavior). Returns null for an unbound + * shortcut — unbound entries never conflict with anything. + */ +export function comboKey(shortcut: KeyboardShortcut, isMacPlatform: boolean): string | null { + if (isUnboundShortcut(shortcut)) return null; + + const modifiers = shortcut.modifiers; + const ctrl = !!modifiers?.ctrl || !!(modifiers?.ctrlOrCmd && !isMacPlatform); + const cmd = !!modifiers?.cmd || !!(modifiers?.ctrlOrCmd && isMacPlatform); + const alt = !!modifiers?.alt; + const shift = !!modifiers?.shift; + + return [shortcut.key.toLowerCase(), ctrl, cmd, alt, shift].join("|"); +} + +/** A group of two or more entries whose effective combo collides. */ +export interface ShortcutConflictGroup { + comboKey: string; + entries: ShortcutEntry[]; +} + +/** + * Groups `entries` (already resolved to their effective shortcut, e.g. via + * `resolveShortcutEntry`) by collision. Only groups with 2+ members are + * returned. Dispatch order for a colliding combo is registration order (see + * `hooks/use-plugin-shortcuts.ts`) — this helper only detects and reports, + * it doesn't resolve precedence. + */ +export function findShortcutConflicts( + resolved: { entry: ShortcutEntry; shortcut: KeyboardShortcut }[], + isMacPlatform: boolean, +): ShortcutConflictGroup[] { + const groups = new Map(); + + for (const { entry, shortcut } of resolved) { + const key = comboKey(shortcut, isMacPlatform); + if (key === null) continue; + const group = groups.get(key); + if (group) { + group.push(entry); + } else { + groups.set(key, [entry]); + } + } + + return Array.from(groups.entries()) + .filter(([, list]) => list.length > 1) + .map(([key, list]) => ({ comboKey: key, entries: list })); +} diff --git a/apps/web/lib/keyboard/utils.ts b/apps/web/lib/keyboard/utils.ts index a1c643d4d1..5ff220e379 100644 --- a/apps/web/lib/keyboard/utils.ts +++ b/apps/web/lib/keyboard/utils.ts @@ -4,6 +4,21 @@ import type { KeyboardShortcut, Platform } from "./constants"; +/** + * Returns true if the keydown event's target is a text input, textarea, or + * contenteditable element. Shared by every global keydown dispatcher + * (`useAppShortcuts`, `usePluginShortcuts`) so typing in a field never + * triggers a bound shortcut. + */ +export function isEditableKeydownTarget(event: KeyboardEvent): boolean { + const tag = (event.target as HTMLElement)?.tagName; + return ( + tag === "INPUT" || + tag === "TEXTAREA" || + (event.target as HTMLElement)?.isContentEditable === true + ); +} + /** * Detect the current platform */ diff --git a/apps/web/lib/plugins/host-api.test.ts b/apps/web/lib/plugins/host-api.test.ts index 37104b330c..74f5561ae8 100644 --- a/apps/web/lib/plugins/host-api.test.ts +++ b/apps/web/lib/plugins/host-api.test.ts @@ -140,4 +140,22 @@ describe("buildHostApi", () => { const host = buildHostApi("jira", createAppStore(), "light"); expect(host.pluginId).toBe("jira"); }); + + it("routes openModal to the modal manager, scoped to this plugin's id", async () => { + const { pluginModalManager } = await import("./modal-manager"); + const host = buildHostApi("jira", createAppStore(), "light"); + + const before = pluginModalManager.getSnapshot().length; + const handle = host.openModal({ content: () => null, title: "Test" }); + + const snapshot = pluginModalManager.getSnapshot(); + expect(snapshot).toHaveLength(before + 1); + expect(snapshot[snapshot.length - 1]).toMatchObject({ + pluginId: "jira", + options: { title: "Test" }, + }); + + handle.close(); + expect(pluginModalManager.getSnapshot()).toHaveLength(before); + }); }); diff --git a/apps/web/lib/plugins/host-api.ts b/apps/web/lib/plugins/host-api.ts index 8322e124c2..c19d22c20b 100644 --- a/apps/web/lib/plugins/host-api.ts +++ b/apps/web/lib/plugins/host-api.ts @@ -71,6 +71,7 @@ import { TaskCreateDialog } from "@/components/task-create-dialog"; import { getBackendConfig } from "@/lib/config"; import { softNavigate } from "@/lib/routing/client-router"; import type { AppState } from "@/lib/state/store"; +import { pluginModalManager } from "./modal-manager"; import type { PluginHostApi } from "./types"; /** @@ -189,6 +190,7 @@ export function buildHostApi( ui: PLUGIN_UI, theme, navigate: (href, options) => softNavigate(href, options?.replace ? "replace" : "push"), + openModal: (options) => pluginModalManager.openModal(pluginId, options), }; } diff --git a/apps/web/lib/plugins/host.test.ts b/apps/web/lib/plugins/host.test.ts index 10ee5c22b9..b27a469eb9 100644 --- a/apps/web/lib/plugins/host.test.ts +++ b/apps/web/lib/plugins/host.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, afterEach } from "vitest"; import type { Window as HappyDOMWindow } from "happy-dom"; import { loadPlugins, unloadPlugin } from "./host"; +import { pluginModalManager } from "./modal-manager"; import { pluginRegistry } from "./registry"; import type { ActivePlugin, PluginHostApi, PluginRegistry } from "./types"; @@ -34,6 +35,7 @@ function makeHostFactory(pluginId: string): PluginHostApi { ui: {}, theme: "light", navigate: () => {}, + openModal: () => ({ close: () => {} }), }; } @@ -331,6 +333,40 @@ describe("unloadPlugin", () => { }); }); +describe("unloadPlugin — plugin modal cleanup", () => { + afterEach(() => { + pluginRegistry.unregisterPlugin(PLUGIN_UNLOAD_A_ID); + }); + + it("closes every modal opened by the unloaded plugin", async () => { + const importer = fakeImporterFor({ + [BUNDLE_JS_URL]: (win) => + (win as unknown as FakeWindow).registerKandevPlugin(PLUGIN_UNLOAD_A_ID, { + initialize: () => {}, + }), + }); + await loadPlugins( + [activePlugin({ id: PLUGIN_UNLOAD_A_ID, bundleUrl: BUNDLE_JS_URL })], + makeHostFactory, + importer, + ); + + const before = pluginModalManager.getSnapshot().length; + pluginModalManager.openModal(PLUGIN_UNLOAD_A_ID, { content: () => null }); + const otherHandle = pluginModalManager.openModal("some-other-plugin", { content: () => null }); + expect(pluginModalManager.getSnapshot()).toHaveLength(before + 2); + + unloadPlugin(PLUGIN_UNLOAD_A_ID); + + const snapshot = pluginModalManager.getSnapshot(); + expect(snapshot).toHaveLength(before + 1); + expect(snapshot.some((m) => m.pluginId === PLUGIN_UNLOAD_A_ID)).toBe(false); + expect(snapshot.some((m) => m.pluginId === "some-other-plugin")).toBe(true); + + otherHandle.close(); + }); +}); + describe("loadPlugins — initialize() timeout isolation", () => { afterEach(() => { pluginRegistry.unregisterPlugin(PLUGIN_HANG_A_ID); diff --git a/apps/web/lib/plugins/host.ts b/apps/web/lib/plugins/host.ts index d2d0d197e4..20feabd776 100644 --- a/apps/web/lib/plugins/host.ts +++ b/apps/web/lib/plugins/host.ts @@ -16,6 +16,7 @@ * tab must reuse the cached registration instead of relying on re-import. */ import { getBackendConfig } from "@/lib/config"; +import { pluginModalManager } from "./modal-manager"; import { pluginRegistry } from "./registry"; import type { ActivePlugin, KandevPlugin, PluginHostApi } from "./types"; @@ -195,6 +196,7 @@ export function unloadPlugin(id: string, options?: { evictCache?: boolean }): vo console.error(`[plugins] error destroying plugin "${id}"`, error); } finally { pluginRegistry.unregisterPlugin(id); + pluginModalManager.closeAllForPlugin(id); removeStyles(id); if (options?.evictCache) { registeredPlugins.delete(id); diff --git a/apps/web/lib/plugins/modal-manager.test.ts b/apps/web/lib/plugins/modal-manager.test.ts new file mode 100644 index 0000000000..9164a812a4 --- /dev/null +++ b/apps/web/lib/plugins/modal-manager.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect } from "vitest"; +import { pluginModalManager } from "./modal-manager"; +import type { PluginModalOptions } from "./types"; + +function noopContent() { + return null; +} + +const baseOptions: PluginModalOptions = { content: noopContent }; + +describe("pluginModalManager", () => { + it("open() returns a handle and adds the modal to the snapshot", () => { + const before = pluginModalManager.getSnapshot().length; + const handle = pluginModalManager.openModal("jira", baseOptions); + + const snapshot = pluginModalManager.getSnapshot(); + expect(snapshot).toHaveLength(before + 1); + expect(snapshot[snapshot.length - 1].pluginId).toBe("jira"); + + handle.close(); + }); + + it("close() removes only the closed modal instance", () => { + const handleA = pluginModalManager.openModal("jira", baseOptions); + const handleB = pluginModalManager.openModal("jira", baseOptions); + const before = pluginModalManager.getSnapshot().length; + + handleA.close(); + + const snapshot = pluginModalManager.getSnapshot(); + expect(snapshot).toHaveLength(before - 1); + expect(snapshot.some((m) => m.pluginId === "jira")).toBe(true); + + handleB.close(); + }); + + it("supports multiple concurrently open modals", () => { + const before = pluginModalManager.getSnapshot().length; + const handleA = pluginModalManager.openModal("jira", baseOptions); + const handleB = pluginModalManager.openModal("linear", baseOptions); + + expect(pluginModalManager.getSnapshot()).toHaveLength(before + 2); + + handleA.close(); + handleB.close(); + expect(pluginModalManager.getSnapshot()).toHaveLength(before); + }); + + it("closeAllForPlugin removes only that plugin's modals", () => { + const jiraHandle = pluginModalManager.openModal("jira", baseOptions); + const linearHandle = pluginModalManager.openModal("linear", baseOptions); + const before = pluginModalManager.getSnapshot().length; + + pluginModalManager.closeAllForPlugin("jira"); + + const snapshot = pluginModalManager.getSnapshot(); + expect(snapshot).toHaveLength(before - 1); + expect(snapshot.some((m) => m.pluginId === "jira")).toBe(false); + expect(snapshot.some((m) => m.pluginId === "linear")).toBe(true); + + void jiraHandle; + linearHandle.close(); + }); + + it("generates monotonically increasing, unique instance ids", () => { + const handleA = pluginModalManager.openModal("jira", baseOptions); + const snapshotA = pluginModalManager.getSnapshot(); + const idA = snapshotA[snapshotA.length - 1].instanceId; + + const handleB = pluginModalManager.openModal("jira", baseOptions); + const snapshotB = pluginModalManager.getSnapshot(); + const idB = snapshotB[snapshotB.length - 1].instanceId; + + expect(idA).not.toBe(idB); + + handleA.close(); + handleB.close(); + }); + + it("close() is a no-op when the modal is already closed", () => { + const handle = pluginModalManager.openModal("jira", baseOptions); + handle.close(); + const before = pluginModalManager.getSnapshot().length; + expect(() => handle.close()).not.toThrow(); + expect(pluginModalManager.getSnapshot()).toHaveLength(before); + }); +}); diff --git a/apps/web/lib/plugins/modal-manager.ts b/apps/web/lib/plugins/modal-manager.ts new file mode 100644 index 0000000000..936a086e56 --- /dev/null +++ b/apps/web/lib/plugins/modal-manager.ts @@ -0,0 +1,71 @@ +/** + * Reactive singleton backing `host.openModal(...)` (docs/plans/plugins/PLUGIN-API.md). + * + * Tracks every open plugin modal as a list of `{ instanceId, pluginId, options }` + * and exposes a tiny external-store subscription so `` + * re-renders whenever a modal opens or closes — same pattern as + * `lib/plugins/registry.ts`'s `pluginRegistry` singleton. + */ +import { useSyncExternalStore } from "react"; +import type { PluginModalHandle, PluginModalOptions } from "./types"; + +export interface OpenPluginModal { + instanceId: string; + pluginId: string; + options: PluginModalOptions; +} + +let nextInstanceId = 0; + +class PluginModalManager { + private modals: OpenPluginModal[] = []; + private listeners = new Set<() => void>(); + private snapshot: OpenPluginModal[] = []; + + subscribe = (listener: () => void): (() => void) => { + this.listeners.add(listener); + return () => { + this.listeners.delete(listener); + }; + }; + + getSnapshot = (): OpenPluginModal[] => this.snapshot; + + /** Opens a modal owned by `pluginId`; returns a handle to close it. */ + openModal(pluginId: string, options: PluginModalOptions): PluginModalHandle { + const instanceId = `plugin-modal-${(nextInstanceId += 1)}`; + this.modals = [...this.modals, { instanceId, pluginId, options }]; + this.notify(); + return { close: () => this.close(instanceId) }; + } + + /** Removes every modal owned by `pluginId` (called on plugin disable/unload). */ + closeAllForPlugin(pluginId: string): void { + const before = this.modals.length; + this.modals = this.modals.filter((modal) => modal.pluginId !== pluginId); + if (this.modals.length !== before) this.notify(); + } + + /** Removes a single modal by `instanceId`, e.g. from the host's Dialog `onOpenChange`. */ + close(instanceId: string): void { + const before = this.modals.length; + this.modals = this.modals.filter((modal) => modal.instanceId !== instanceId); + if (this.modals.length !== before) this.notify(); + } + + private notify(): void { + this.snapshot = this.modals; + this.listeners.forEach((listener) => listener()); + } +} + +export const pluginModalManager = new PluginModalManager(); + +/** Snapshot hook: re-renders the caller whenever any plugin modal opens/closes. */ +export function usePluginModals(): OpenPluginModal[] { + return useSyncExternalStore( + pluginModalManager.subscribe, + pluginModalManager.getSnapshot, + pluginModalManager.getSnapshot, + ); +} diff --git a/apps/web/lib/plugins/registry.test.ts b/apps/web/lib/plugins/registry.test.ts index 814c01ba7d..10b71bd921 100644 --- a/apps/web/lib/plugins/registry.test.ts +++ b/apps/web/lib/plugins/registry.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, afterEach } from "vitest"; +import { describe, it, expect, afterEach, vi } from "vitest"; import { pluginRegistry } from "./registry"; const TASK_SIDEBAR_SLOT = "task-sidebar"; @@ -214,3 +214,70 @@ describe("pluginRegistry — route options and plugin names", () => { expect(pluginRegistry.getPluginName("plugin-a")).toBeUndefined(); }); }); + +describe("pluginRegistry — keybinding handlers", () => { + afterEach(() => { + cleanup("plugin-a", "plugin-b"); + }); + + it("registers a keybinding handler scoped to the owning plugin", () => { + const scopedA = pluginRegistry.forPlugin("plugin-a"); + const scopedB = pluginRegistry.forPlugin("plugin-b"); + const handlerA = () => {}; + const handlerB = () => {}; + + scopedA.registerKeybinding("open", handlerA); + scopedB.registerKeybinding("open", handlerB); + + expect(pluginRegistry.getKeybindingHandler("plugin-a", "open")).toBe(handlerA); + expect(pluginRegistry.getKeybindingHandler("plugin-b", "open")).toBe(handlerB); + expect(pluginRegistry.getKeybindingHandlers()).toEqual([ + { id: "open", handler: handlerA, pluginId: "plugin-a" }, + { id: "open", handler: handlerB, pluginId: "plugin-b" }, + ]); + }); + + it("revokes keybinding handlers on unregisterPlugin", () => { + const scoped = pluginRegistry.forPlugin("plugin-a"); + scoped.registerKeybinding("open", () => {}); + + pluginRegistry.unregisterPlugin("plugin-a"); + + expect(pluginRegistry.getKeybindingHandlers()).toEqual([]); + expect(pluginRegistry.getKeybindingHandler("plugin-a", "open")).toBeUndefined(); + }); + + it("warns when registering a handler for an id not declared in the manifest", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + pluginRegistry.setDeclaredKeybindingIds("plugin-a", ["open"]); + const scoped = pluginRegistry.forPlugin("plugin-a"); + + scoped.registerKeybinding("not-declared", () => {}); + + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("not-declared")); + // The handler is still stored despite the warning. + expect(pluginRegistry.getKeybindingHandler("plugin-a", "not-declared")).toBeDefined(); + warnSpy.mockRestore(); + }); + + it("does not warn when the declared id list has not been synced yet", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const scoped = pluginRegistry.forPlugin("plugin-a"); + + scoped.registerKeybinding("anything", () => {}); + + expect(warnSpy).not.toHaveBeenCalled(); + warnSpy.mockRestore(); + }); + + it("does not warn when the id is declared in the manifest", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + pluginRegistry.setDeclaredKeybindingIds("plugin-a", ["open"]); + const scoped = pluginRegistry.forPlugin("plugin-a"); + + scoped.registerKeybinding("open", () => {}); + + expect(warnSpy).not.toHaveBeenCalled(); + warnSpy.mockRestore(); + }); +}); diff --git a/apps/web/lib/plugins/registry.ts b/apps/web/lib/plugins/registry.ts index 4d6ab5b1a7..b00b2625ea 100644 --- a/apps/web/lib/plugins/registry.ts +++ b/apps/web/lib/plugins/registry.ts @@ -25,6 +25,13 @@ interface Owned { value: T; } +/** A handler bound via `PluginRegistry.registerKeybinding`. */ +export interface PluginKeybindingHandler { + /** Plugin-local keybinding id (matches `ui.keybindings[].id`). */ + id: string; + handler: (event: KeyboardEvent) => void; +} + export interface RouteRegistration { path: string; Component: ComponentType; @@ -66,9 +73,18 @@ class PluginRegistryStore { private navItems: Owned[] = []; private slotComponents: Owned[] = []; private wsHandlers: Owned[] = []; + private keybindingHandlers: Owned[] = []; private nextSlotRegistrationId = 0; /** Display names from the boot payload, used for derived page-chrome titles. */ private pluginNames = new Map(); + /** + * Keybinding ids declared in each plugin's `ui.keybindings` manifest, + * synced by the shortcut dispatcher (`hooks/use-plugin-shortcuts.ts`) from + * the plugin records store. Used only to warn on `registerKeybinding` + * calls for an id the manifest never declared — an empty/missing entry + * (descriptors not loaded yet) skips the check rather than false-warning. + */ + private declaredKeybindingIds = new Map>(); private listeners = new Set<() => void>(); private version = 0; @@ -122,6 +138,26 @@ class PluginRegistryStore { this.notify(); } + registerKeybinding(pluginId: string, id: string, handler: (event: KeyboardEvent) => void): void { + const declared = this.declaredKeybindingIds.get(pluginId); + if (declared && !declared.has(id)) { + console.warn( + `[plugins] "${pluginId}" registered a keybinding handler for id "${id}", which is not declared in its ui.keybindings manifest`, + ); + } + this.keybindingHandlers.push({ pluginId, value: { id, handler } }); + this.notify(); + } + + /** + * Records the keybinding ids declared in `pluginId`'s `ui.keybindings` + * manifest, so `registerKeybinding` can warn on an undeclared id. Safe to + * call repeatedly (e.g. every time the plugin records store refreshes). + */ + setDeclaredKeybindingIds(pluginId: string, ids: string[]): void { + this.declaredKeybindingIds.set(pluginId, new Set(ids)); + } + /** Bulk-revoke every registration owned by `pluginId` (disable/uninstall). */ unregisterPlugin(pluginId: string): void { const before = this.totalCount(); @@ -130,7 +166,9 @@ class PluginRegistryStore { this.navItems = removeByPlugin(this.navItems, pluginId); this.slotComponents = removeByPlugin(this.slotComponents, pluginId); this.wsHandlers = removeByPlugin(this.wsHandlers, pluginId); + this.keybindingHandlers = removeByPlugin(this.keybindingHandlers, pluginId); this.pluginNames.delete(pluginId); + this.declaredKeybindingIds.delete(pluginId); if (this.totalCount() !== before) this.notify(); } @@ -185,6 +223,23 @@ class PluginRegistryStore { .map((entry) => entry.value.handler); } + /** + * All registered keybinding handlers plus their owning pluginId, in + * registration order. Registration order is the dispatch-order tiebreaker + * when two plugins bind the same effective combo (see + * `hooks/use-plugin-shortcuts.ts`). + */ + getKeybindingHandlers(): (PluginKeybindingHandler & { pluginId: string })[] { + return this.keybindingHandlers.map((entry) => ({ ...entry.value, pluginId: entry.pluginId })); + } + + /** The `pluginId`'s bound handler for `id`, if any (first match wins). */ + getKeybindingHandler(pluginId: string, id: string): ((event: KeyboardEvent) => void) | undefined { + return this.keybindingHandlers.find( + (entry) => entry.pluginId === pluginId && entry.value.id === id, + )?.value.handler; + } + /** Registry view scoped to one plugin — matches the frozen `PluginRegistry` contract. */ forPlugin(pluginId: string, pluginName?: string): PluginRegistry { if (pluginName) this.pluginNames.set(pluginId, pluginName); @@ -196,6 +251,7 @@ class PluginRegistryStore { this.registerSettingsRoute(pluginId, path, Component), registerComponent: (slot, Component) => this.registerComponent(pluginId, slot, Component), registerWsHandler: (action, handler) => this.registerWsHandler(pluginId, action, handler), + registerKeybinding: (id, handler) => this.registerKeybinding(pluginId, id, handler), }; } @@ -205,7 +261,8 @@ class PluginRegistryStore { this.settingsRoutes.length + this.navItems.length + this.slotComponents.length + - this.wsHandlers.length + this.wsHandlers.length + + this.keybindingHandlers.length ); } diff --git a/apps/web/lib/plugins/types.ts b/apps/web/lib/plugins/types.ts index 14a17524d5..717e9c7ea6 100644 --- a/apps/web/lib/plugins/types.ts +++ b/apps/web/lib/plugins/types.ts @@ -112,6 +112,24 @@ export type SlotComponent = ReactType.ComponentType<{ slotProps?: unknown }>; /** WS action payload handler registered by a plugin. */ export type WsHandler = (payload: unknown) => void; +/** Options accepted by `host.openModal(...)`. */ +export interface PluginModalOptions { + /** Modal title, rendered in a `DialogHeader`/`DialogTitle`. Omit to render no header title. */ + title?: string; + /** Component rendered inside the modal body — reuses the slot-component contract. */ + content: SlotComponent; + /** Modal width, mapped to the host's Dialog size classes. Default: "md". */ + size?: "sm" | "md" | "lg" | "xl"; + /** Whether the modal can be dismissed via overlay click or Escape. Default: true. */ + dismissible?: boolean; +} + +/** Handle returned by `host.openModal(...)`, used to close that modal instance. */ +export interface PluginModalHandle { + /** Closes this modal instance. No-op if already closed. */ + close(): void; +} + /** * API surface passed as the second argument to `KandevPlugin.initialize`. * Plugins must render with `host.React` / `host.jsx` — no bundled React. @@ -143,6 +161,12 @@ export interface PluginHostApi { theme: "light" | "dark"; /** Soft SPA navigation (history push/replace + re-render), same as the app's router. */ navigate(href: string, options?: { replace?: boolean }): void; + /** + * Imperatively opens a modal window rendered by the host's `` + * (mounted once at the app root). Independent of keybindings — a keybinding + * handler may call it, but it works from any plugin code path. + */ + openModal(options: PluginModalOptions): PluginModalHandle; } /** @@ -174,6 +198,16 @@ export interface PluginRegistry { registerComponent(slot: PluginSlotName, Component: SlotComponent): void; /** WS action handler, bridged into the existing `lib/ws` dispatch. */ registerWsHandler(action: string, handler: WsHandler): void; + /** + * Bind a handler to a keybinding declared in this plugin's manifest + * (`ui.keybindings[].id`). The host resolves the effective combo (user + * override if set, else the manifest `default`) and dispatches globally, + * skipping editable targets the same way core app shortcuts do. Binding an + * `id` the manifest didn't declare still stores the handler (a console + * warning is logged) since the dispatcher's effective-shortcut resolution + * still keys off the manifest list. + */ + registerKeybinding(id: string, handler: (event: KeyboardEvent) => void): void; } /** Shape every plugin bundle registers via `window.registerKandevPlugin(id, plugin)`. */ diff --git a/apps/web/lib/types/plugins.ts b/apps/web/lib/types/plugins.ts index bc84589515..7b3e934348 100644 --- a/apps/web/lib/types/plugins.ts +++ b/apps/web/lib/types/plugins.ts @@ -29,10 +29,24 @@ export interface PluginUIPage { surface: string; } +/** + * A single keyboard shortcut a plugin's UI bundle wants kandev to register + * and dispatch on its behalf. `id` is plugin-local (unique within the + * plugin, not globally); `default` is a combo like "mod+shift+k" ("mod" + * resolves to Cmd on macOS, Ctrl elsewhere). Only meaningful alongside + * `bundle` — the backend rejects keybindings declared without one. + */ +export interface PluginKeybinding { + id: string; + default: string; + description: string; +} + export interface PluginUISection { pages?: PluginUIPage[]; bundle?: string; styles?: string[]; + keybindings?: PluginKeybinding[]; } /** diff --git a/apps/web/src/main.tsx b/apps/web/src/main.tsx index d94d26f06f..f183ee6793 100644 --- a/apps/web/src/main.tsx +++ b/apps/web/src/main.tsx @@ -1,6 +1,7 @@ import { StrictMode } from "react"; import { createRoot } from "react-dom/client"; import "@/app/globals.css"; +import { PluginModalHost } from "@/components/plugins/plugin-modal-host"; import { StateProvider } from "@/components/state-provider"; import { PluginBootBridge } from "@/lib/plugins/plugin-boot-bridge"; import { AppShell } from "./app-shell"; @@ -12,6 +13,7 @@ function App({ payload }: { payload: BootPayload }) { return ( + diff --git a/docs/plans/plugins/PLUGIN-API.md b/docs/plans/plugins/PLUGIN-API.md index b1ce1eff33..76645e8882 100644 --- a/docs/plans/plugins/PLUGIN-API.md +++ b/docs/plans/plugins/PLUGIN-API.md @@ -59,6 +59,21 @@ interface PluginHostApi { // path as the app router, so plugin pages can link into native routes // (e.g. /t/{taskId}) without a full reload. navigate(href: string, options?: { replace?: boolean }): void; + // Imperatively opens a modal window rendered by the host's + // (mounted once at the app root, isolated behind its own error boundary). + // Independent of keybindings — any plugin code path may call it. + openModal(options: PluginModalOptions): PluginModalHandle; +} + +interface PluginModalOptions { + title?: string; // rendered in a DialogHeader/DialogTitle; omit for no header title + content: React.ComponentType<{ slotProps?: unknown }>; // reuses the slot-component contract + size?: "sm" | "md" | "lg" | "xl"; // maps to the host's Dialog width classes; default "md" + dismissible?: boolean; // overlay click / Escape close the modal; default true +} + +interface PluginModalHandle { + close(): void; // closes this modal instance; no-op if already closed } ``` @@ -153,6 +168,25 @@ interface PluginRegistry { // WS action handler. Bridged into the existing lib/ws dispatch; called with the // decoded message payload for that action string. registerWsHandler(action: string, handler: (payload: unknown) => void): void; + + // Binds a handler to a keybinding declared in this plugin's manifest + // (ui.keybindings[].id — { id, default, description }, see manifest schema). + // The host resolves the effective combo (user override if the user + // rebound it, else the manifest default) and dispatches globally, skipping + // editable targets the same way core app shortcuts do. Combos are + // user-overridable in Settings > Keyboard Shortcuts, namespaced + // `plugin:{pluginId}:{id}`. Binding an id the manifest didn't declare still + // stores the handler (a console warning is logged) since the dispatcher's + // effective-shortcut resolution keys off the manifest list. + // + // Combo grammar (manifest `default` and any user override): `+`-separated + // tokens, one of the modifiers `mod|ctrl|cmd|meta|alt|option|shift` + // (repeatable) plus exactly one key token. `mod` resolves to Cmd on macOS + // and Ctrl elsewhere (⌘/Ctrl). `shift` may not be combined with a digit or + // symbol key (e.g. `shift+1`, `shift+slash`) — Shift changes the character + // a browser reports for those keys, so the combo could never dispatch; both + // the manifest validator and the frontend parser reject it. + registerKeybinding(id: string, handler: (event: KeyboardEvent) => void): void; } ``` diff --git a/docs/public/plugins-authoring.md b/docs/public/plugins-authoring.md index 547dfd14b7..7d019af3ec 100644 --- a/docs/public/plugins-authoring.md +++ b/docs/public/plugins-authoring.md @@ -308,6 +308,9 @@ interface PluginRegistry { registerComponent(slot: string, Component: React.ComponentType<{ slotProps?: unknown }>): void; // WS action handler, bridged into the existing lib/ws dispatch. registerWsHandler(action: string, handler: (payload: unknown) => void): void; + // Bind a handler to a keybinding declared in this plugin's manifest + // (ui.keybindings[].id). See "Keybindings" below. + registerKeybinding(id: string, handler: (event: KeyboardEvent) => void): void; } interface NavItem { @@ -364,6 +367,19 @@ interface PluginHostApi { theme: "light" | "dark"; // Soft SPA navigation (history push/replace), same as the app's own router. navigate(href: string, options?: { replace?: boolean }): void; + // Imperatively opens a modal window. See "Modal windows" below. + openModal(options: PluginModalOptions): PluginModalHandle; +} + +interface PluginModalOptions { + title?: string; // rendered in DialogHeader/DialogTitle; omit for no title + content: React.ComponentType; // reuses the slot-component contract + size?: "sm" | "md" | "lg" | "xl"; // default "md" + dismissible?: boolean; // overlay click / Escape; default true +} + +interface PluginModalHandle { + close(): void; // no-op if already closed } ``` @@ -381,6 +397,28 @@ here for routes that opt out and render their own chrome), and create-task flow (repo/branch/agent pickers, validation) instead of POSTing directly. See `apps/web/lib/plugins/host-api.ts` for the exact current list. +### Modal windows + +`host.openModal(options)` imperatively opens a host-owned Dialog — rendered +by a `` mounted once at the app root, isolated behind its +own error boundary, and auto-closed if the plugin is disabled or uninstalled +while it's open. It's independent of keybindings: call it from a keybinding +handler, a nav route, a slot component, or a WS handler. It complements the +declarative `host.ui.Dialog` — reach for `host.ui.Dialog` when a dialog is +embedded in a slot's own render tree, and `host.openModal` when you need to +pop one open imperatively from anywhere in your plugin's code. + +```js +const handle = host.openModal({ + title: "Acme settings", + content: SettingsPanel, + size: "lg", +}); + +// later, e.g. after the panel calls back on save: +handle.close(); +``` + ## Named slots `registerComponent(slot, Component)` injects a component into a host-defined @@ -594,6 +632,39 @@ reload, and each contribution has its own error boundary. A full-bleed route (`topbar: false`) owns its own chrome; mount the host Status trigger there if that route should expose Status. +## Keybindings + +Declare each keybinding in `manifest.yaml`'s `ui.keybindings` (requires +`ui.bundle`), then bind a handler at runtime with `registry.registerKeybinding`: + +```yaml +ui: + bundle: "/ui/bundle.js" + keybindings: + - id: "open-panel" # plugin-local slug: ^[a-z0-9][a-z0-9-]*$ + default: "mod+shift+j" # combo grammar, see below + description: "Open the Acme panel" +``` + +```js +registry.registerKeybinding("open-panel", () => { + host.openModal({ title: "Acme panel", content: AcmePanel }); +}); +``` + +The `default` combo is `+`-separated: zero or more modifiers from +`mod | ctrl | cmd | meta | alt | option | shift` (`mod` resolves to ⌘ on +macOS and Ctrl elsewhere) plus exactly one non-modifier key. `shift` may not +combine with a digit or symbol key — the browser reports the shifted glyph +for those keys (e.g. `shift+1` reports `!`), so the combo could never match. + +A plugin keybinding is **user-overridable** at **Settings > Keyboard +Shortcuts**, namespaced `plugin:{pluginId}:{id}` so two plugins can each +declare their own `open-panel` id without colliding. If a plugin's effective +combo conflicts with a core kandev shortcut, the core shortcut always wins — +the plugin handler is skipped for that combo — and the conflict is surfaced +in that same settings page so the user can rebind it. + ## Three integration patterns Both example repos are full, working plugins — read them rather than diff --git a/docs/public/plugins-manifest.md b/docs/public/plugins-manifest.md index ac9f6d947a..998b227d0e 100644 --- a/docs/public/plugins-manifest.md +++ b/docs/public/plugins-manifest.md @@ -61,6 +61,10 @@ config_schema: ui: # optional native frontend plugin bundle: "/ui/bundle.js" # root-relative styles: ["/ui/plugin.css"] # optional, root-relative + keybindings: # optional, requires ui.bundle + - id: "open-panel" # plugin-local: ^[a-z0-9][a-z0-9-]*$ + default: "mod+shift+j" # combo grammar, see field reference + description: "Open the Acme panel" ``` ## Field reference @@ -96,6 +100,10 @@ ui: # optional native frontend plugin | `ui.pages[].title` | yes* | string | Display title. | | `ui.pages[].path` | yes* | string | Route path for the page. | | `ui.pages[].surface` | yes* | string | Where the page mounts. Enum, one of: `settings` · `task-panel` · `main-nav`. Any other value is a validation error. | +| `ui.keybindings` | no | object[] | Declares plugin keybindings bound at runtime via `registerKeybinding`. Requires `ui.bundle`. | +| `ui.keybindings[].id` | yes* | string | Stable, plugin-local slug (*required when a keybinding entry is present). Must match `^[a-z0-9][a-z0-9-]*$` and be unique within this plugin's own `ui.keybindings` list — not globally; the effective shortcut is namespaced `plugin:{pluginId}:{id}`. | +| `ui.keybindings[].default` | yes* | string | Default combo string. `+`-separated tokens: zero or more modifiers from `mod`, `ctrl`, `cmd`, `meta`, `alt`, `option`, `shift` (`mod` = ⌘ on macOS, Ctrl elsewhere) plus exactly one non-modifier key. `shift` may not combine with a digit or symbol key — the browser reports the shifted glyph for those keys, so the combo could never match. | +| `ui.keybindings[].description` | yes* | string | Non-empty, human-readable label shown in **Settings > Keyboard Shortcuts**. | `ui.pages` is declarative manifest metadata only. A native bundle's runtime nav items, icons, and per-route title-bar chrome (`registerNavItem`, diff --git a/docs/public/plugins.md b/docs/public/plugins.md index 1d8875876f..4fe18706a3 100644 --- a/docs/public/plugins.md +++ b/docs/public/plugins.md @@ -38,7 +38,11 @@ alongside kandev's first-party integration links in the main sidebar's plugin is installed and active. Bundles can also inject components into host-defined slots, including **icon buttons in the chat composer toolbar** (beside the model picker, mic, and send button), so an active plugin can add -its own action right where you message an agent. +its own action right where you message an agent. A bundle can also declare +**keybindings** (user-overridable at **Settings > Keyboard Shortcuts**, with +core shortcuts always winning on a conflict) and open host-owned **modal +windows** from anywhere in its code — see [Authoring a +plugin](plugins-authoring.md) for both. ### Global Status contributions diff --git a/docs/specs/plugins/spec.md b/docs/specs/plugins/spec.md index f89d279e25..440d88f5cd 100644 --- a/docs/specs/plugins/spec.md +++ b/docs/specs/plugins/spec.md @@ -500,6 +500,20 @@ Mattermost-webapp model), not iframes. The full contract lives in bundle/`initialize` is caught and never breaks boot; slot components render behind error boundaries. Plugin JS otherwise runs with full in-origin store access — hard sandboxing (workers/realms) is future work (see Out of scope). +- **Keybindings:** a plugin declares `ui.keybindings: [{ id, default, description }]` + in its manifest (`default` is a combo string like `mod+shift+k`, validated at + registration time). `registry.registerKeybinding(id, handler)` binds a JS handler + to a declared id; the host resolves the effective combo (a user override from + Settings > Keyboard Shortcuts if set, else the manifest `default`) and dispatches + it globally, skipping editable targets the same way core app shortcuts do. User + overrides are namespaced `plugin:{pluginId}:{id}` so they survive independently + per plugin. +- **Modals:** `host.openModal({ title?, content, size?, dismissible? })` imperatively + opens a modal rendered by the host's `` (mounted once at the app + root, isolated behind its own error boundary) and returns `{ close() }` to close + that instance. `content` reuses the slot-component contract (rendered with the + host React instance). Independent of keybindings — any plugin code path may call + it, including a keybinding handler. ## State machine