From 247eb5cc9a85db9ab219018156c1a295d46ca9ca Mon Sep 17 00:00:00 2001 From: Shoubhit Dash Date: Sun, 4 Oct 2026 15:46:34 +0530 Subject: [PATCH] fix(tui): allow plugin keymaps during setup (#53010) --- packages/plugin/src/tui/context.ts | 9 +- packages/tui/src/context/keymap.tsx | 47 +++--- packages/tui/src/plugin/api.tsx | 23 ++- packages/tui/test/fixture/app.ts | 19 ++- .../test/fixture/plugin/setup-keymap/tui.tsx | 72 ++++++++ packages/tui/test/plugin-keymap.test.tsx | 155 ++++++++++++++++++ .../src/docs/content/build/plugins/cli.mdx | 17 ++ 7 files changed, 314 insertions(+), 28 deletions(-) create mode 100644 packages/tui/test/fixture/plugin/setup-keymap/tui.tsx create mode 100644 packages/tui/test/plugin-keymap.test.tsx diff --git a/packages/plugin/src/tui/context.ts b/packages/plugin/src/tui/context.ts index f2c5792447e..9e05b669df6 100644 --- a/packages/plugin/src/tui/context.ts +++ b/packages/plugin/src/tui/context.ts @@ -438,8 +438,13 @@ export interface KeymapActive { } export interface Keymap { - /** Creates a reactive keymap layer owned by the calling component. */ - layer(input: () => KeymapLayer): void + /** + * Creates a reactive keymap layer. It is removed when the plugin deactivates, or earlier when the component or reactive + * computation that created it is disposed. The factory must be pure. It runs once untracked for initial command-shape + * validation, then reactively. Initial command-shape errors throw synchronously. OpenTUI handles binding-parse errors. + * Errors from later reactive updates follow Solid's error handling. + */ + layer(factory: () => KeymapLayer): void /** Dispatches a reachable command by ID. */ dispatch(id: string, input?: string): void /** Returns every formatted shortcut for a registered command. */ diff --git a/packages/tui/src/context/keymap.tsx b/packages/tui/src/context/keymap.tsx index bcc98dc5717..521b3aeac86 100644 --- a/packages/tui/src/context/keymap.tsx +++ b/packages/tui/src/context/keymap.tsx @@ -207,27 +207,7 @@ function createLayer(input: () => KeymapLayer) { useBindings(() => { const layer = input() const { commands, bindings, mode, ...options } = layer - const grouped = (commands ?? []).reduce( - (result, command) => { - if (command.id !== undefined) { - if (!command.id) throw new Error("Keymap command IDs cannot be empty") - if (typeof command.bind === "string" && !command.bind) - throw new Error("Keymap command bindings cannot be empty") - result.named.push({ ...command, id: command.id }) - return result - } - if (command.palette) throw new Error("Palette commands require an ID") - if (command.slash) throw new Error("Slash commands require an ID") - if (typeof command.bind !== "string") throw new Error("Inline keymap commands require bind") - if (!command.bind) throw new Error("Keymap command bindings cannot be empty") - result.inline.push({ ...command, id: undefined, bind: command.bind }) - return result - }, - { - named: [] as Array, - inline: [] as Array, - }, - ) + const grouped = groupCommands(commands) return { ...options, enabled: enabled() ? options.enabled : false, @@ -270,6 +250,30 @@ function createLayer(input: () => KeymapLayer) { }) } +function groupCommands(commands: readonly KeymapCommand[] = []) { + return commands.reduce( + (result, command) => { + if (command.id !== undefined) { + if (!command.id) throw new Error("Keymap command IDs cannot be empty") + if (typeof command.bind === "string" && !command.bind) + throw new Error("Keymap command bindings cannot be empty") + result.named.push({ ...command, id: command.id }) + return result + } + if (command.palette) throw new Error("Palette commands require an ID") + if (command.slash) throw new Error("Slash commands require an ID") + if (typeof command.bind !== "string") throw new Error("Inline keymap commands require bind") + if (!command.bind) throw new Error("Keymap command bindings cannot be empty") + result.inline.push({ ...command, id: undefined, bind: command.bind }) + return result + }, + { + named: [] as Array, + inline: [] as Array, + }, + ) +} + function useShortcuts() { const value = useValue() const shortcuts = useKeymapSelector((keymap) => { @@ -403,6 +407,7 @@ export const Keymap = { Provider, use, createLayer, + validateCommands: (commands?: readonly KeymapCommand[]) => void groupCommands(commands), useShortcuts, useShortcut, useLeaderActive, diff --git a/packages/tui/src/plugin/api.tsx b/packages/tui/src/plugin/api.tsx index 9895b1f8ab5..c26e3c25983 100644 --- a/packages/tui/src/plugin/api.tsx +++ b/packages/tui/src/plugin/api.tsx @@ -1,5 +1,5 @@ import { PluginContextProvider } from "@opencode/plugin/tui" -import type { JSX } from "solid-js" +import { createRoot, getOwner, onCleanup, runWithOwner, untrack, type JSX } from "solid-js" import type { Context, Dialog, Page, SlotClaim, SlotMap, SlotPath, Toast } from "@opencode/plugin/tui/context" import type { Placement, PlacementKind } from "./structure" import { infoStringToFiletype, type MarkdownCodeBlockRenderer } from "@opentui/core" @@ -54,6 +54,7 @@ export type Registry = { // (hooks must run during component setup) and shared by every activation. export function usePluginHost() { return { + owner: getOwner(), renderer: useRenderer(), client: useClient(), data: useData(), @@ -133,6 +134,12 @@ export function createPluginContext(input: { input.owned.push(async () => unregister()) return unregister } + let cleanups: Set<() => void> | undefined = new Set() + input.owned.push(async () => { + const active = cleanups + cleanups = undefined + active?.forEach((dispose) => dispose()) + }) context = { options: input.options ?? {}, get location() { @@ -161,7 +168,19 @@ export function createPluginContext(input: { }, }, keymap: { - layer: Keymap.createLayer, + layer(factory) { + const active = cleanups + if (!active) return + // Validate outside Solid, whose error routing would bypass the caller. + Keymap.validateCommands(untrack(factory).commands) + const caller = getOwner() + createRoot((dispose) => { + active.add(dispose) + onCleanup(() => active.delete(dispose)) + if (caller) runWithOwner(caller, () => onCleanup(dispose)) + Keymap.createLayer(factory) + }, caller ?? host.owner) + }, dispatch: host.keymap.dispatch, shortcuts: host.shortcuts.list, commands: host.keymapState.commands, diff --git a/packages/tui/test/fixture/app.ts b/packages/tui/test/fixture/app.ts index 91e2aca55bc..b76d2db1e7a 100644 --- a/packages/tui/test/fixture/app.ts +++ b/packages/tui/test/fixture/app.ts @@ -1,5 +1,6 @@ import { createTestRenderer } from "@opentui/core/testing" import { Effect, FileSystem } from "effect" +import path from "node:path" import { Global } from "@opencode/util/global" import type { TuiInput } from "../../src/app" import type { Config } from "../../src/config" @@ -17,7 +18,7 @@ export async function createAppFixture( } = {}, ) { const { run } = await import("../../src/app") - const state = input.state ? undefined : await tmpdir() + const root = await tmpdir() const setup = await createTestRenderer({ width: input.width ?? 100, height: input.height ?? 30, @@ -39,7 +40,19 @@ export async function createAppFixture( args: input.args ?? {}, log: () => {}, }).pipe( - Effect.provide(Global.layerWith({ state: input.state ?? state?.path })), + Effect.provide( + Global.layerWith({ + home: path.join(root.path, "home"), + data: path.join(root.path, "data"), + cache: path.join(root.path, "cache"), + config: path.join(root.path, "config"), + state: input.state ?? path.join(root.path, "state"), + bin: path.join(root.path, "cache", "bin"), + log: path.join(root.path, "data", "log"), + repos: path.join(root.path, "data", "repos"), + tmp: path.join(root.path, "tmp"), + }), + ), Effect.provide(FileSystem.layerNoop({})), ), ) @@ -53,7 +66,7 @@ export async function createAppFixture( await task } finally { await server.stop() - await state?.[Symbol.asyncDispose]() + await root[Symbol.asyncDispose]() } }, } diff --git a/packages/tui/test/fixture/plugin/setup-keymap/tui.tsx b/packages/tui/test/fixture/plugin/setup-keymap/tui.tsx new file mode 100644 index 00000000000..c2dfc7b6f88 --- /dev/null +++ b/packages/tui/test/fixture/plugin/setup-keymap/tui.tsx @@ -0,0 +1,72 @@ +import { Plugin } from "@opencode/plugin/tui" +import { batch, createSignal } from "solid-js" + +const generation = "generation 1" + +export default Plugin.define({ + id: "fixture.setup-keymap", + async setup(context) { + const [runs, setRuns] = createSignal(0) + let syncs = 0 + context.keymap.layer(() => ({ + commands: [ + { + id: "fixture.sync", + title: "Sync setup check", + bind: "f9", + run: () => context.ui.toast.show({ message: `Sync keymap ran ${++syncs}` }), + }, + ], + })) + await Promise.resolve() + if (context.options.fail) { + setTimeout( + () => + context.keymap.layer(() => ({ + commands: [ + { + id: "fixture.late", + title: "Late setup check", + bind: "f10", + run: () => context.ui.toast.show({ message: "Late keymap ran" }), + }, + ], + })), + 20, + ) + batch(() => context.keymap.layer(() => ({ commands: [{ title: "Invalid", palette: true, run: () => {} }] }))) + } + context.keymap.layer(() => ({ + mode: "global", + commands: [ + { + id: "fixture.setup", + title: `Setup check ${runs()}`, + palette: true, + slash: { name: "setup-check" }, + bind: "ctrl+g", + run() { + setRuns((count) => count + 1) + context.ui.toast.show({ message: `Setup keymap ran ${runs()}` }) + }, + }, + { + id: "fixture.dialog", + title: "Open fixture dialog", + bind: "f8", + run: () => context.ui.dialog.show(() => ), + }, + ], + })) + context.ui.toast.show({ message: `Fixture ready ${generation}` }) + }, +}) + +function FixtureDialog(props: { context: Plugin.Context }) { + let presses = 0 + props.context.keymap.layer(() => ({ + mode: "global", + commands: [{ bind: "f11", run: () => props.context.ui.toast.show({ message: `Dialog keymap ran ${++presses}` }) }], + })) + return Fixture dialog +} diff --git a/packages/tui/test/plugin-keymap.test.tsx b/packages/tui/test/plugin-keymap.test.tsx new file mode 100644 index 00000000000..0790e9a689d --- /dev/null +++ b/packages/tui/test/plugin-keymap.test.tsx @@ -0,0 +1,155 @@ +import { InputRenderable } from "@opentui/core" +import { beforeEach, expect, test } from "bun:test" +import { mkdir, symlink } from "node:fs/promises" +import path from "node:path" +import { takeDraft } from "../src/component/prompt/draft-stash" +import { createAppFixture } from "./fixture/app" +import { tmpdir } from "./fixture/fixture" + +type Fixture = Awaited> + +beforeEach(() => { + takeDraft(undefined) +}) + +test("setup layers before and after await reach keyboard, palette, and slash across disable and enable", async () => { + await using plugin = await copyPlugin() + await using setup = await createAppFixture({ config: { animations: false, plugins: [plugin.directory] } }) + await setup.ready + await setup.waitForFrame((frame) => frame.includes("Fixture ready generation 1")) + + await search(setup, "Setup check") + await setup.waitForFrame((frame) => frame.includes("Setup check 0")) + setup.mockInput.pressEnter() + await setup.waitForFrame((frame) => frame.includes("Setup keymap ran 1")) + await search(setup, "Setup check") + await setup.waitForFrame((frame) => frame.includes("Setup check 1")) + setup.mockInput.pressEscape() + + setup.mockInput.pressKey("F9") + await setup.waitForFrame((frame) => frame.includes("Sync keymap ran 1")) + setup.mockInput.pressKey("g", { ctrl: true }) + await setup.waitForFrame((frame) => frame.includes("Setup keymap ran 2")) + await setup.mockInput.typeText("/setup-check") + await setup.waitForFrame((frame) => frame.includes("/setup-check")) + setup.mockInput.pressEnter() + await setup.waitForFrame((frame) => frame.includes("Setup keymap ran 3")) + + await openPlugin(setup) + await setup.waitForFrame((frame) => frame.includes("disable")) + setup.mockInput.pressEnter() + await setup.waitForFrame((frame) => frame.includes("inactive")) + setup.mockInput.pressEscape() + setup.mockInput.pressKey("g", { ctrl: true }) + setup.mockInput.pressKey("F9") + await search(setup, "Setup check") + await setup.waitForFrame((frame) => frame.includes("No results found")) + expect(setup.captureCharFrame()).not.toContain("Setup keymap ran 4") + expect(setup.captureCharFrame()).not.toContain("Sync keymap ran 2") + setup.mockInput.pressEscape() + + await openPlugin(setup) + await setup.waitForFrame((frame) => frame.includes("inactive")) + setup.mockInput.pressEnter() + await setup.waitForFrame((frame) => frame.includes("Fixture ready generation 1")) + setup.mockInput.pressEscape() + setup.mockInput.pressKey("g", { ctrl: true }) + await setup.waitForFrame((frame) => frame.includes("Setup keymap ran 1")) + setup.mockInput.pressKey("F9") + await setup.waitForFrame((frame) => frame.includes("Sync keymap ran 1")) +}) + +test("a component layer ends when its component unmounts while the plugin stays active", async () => { + await using plugin = await copyPlugin() + await using setup = await createAppFixture({ config: { animations: false, plugins: [plugin.directory] } }) + await setup.ready + await setup.waitForFrame((frame) => frame.includes("Fixture ready generation 1")) + + setup.mockInput.pressKey("F8") + await setup.waitForFrame((frame) => frame.includes("Fixture dialog")) + setup.mockInput.pressKey("F11") + await setup.waitForFrame((frame) => frame.includes("Dialog keymap ran 1")) + setup.mockInput.pressEscape() + await setup.waitForFrame((frame) => !frame.includes("Fixture dialog")) + + setup.mockInput.pressKey("g", { ctrl: true }) + await setup.waitForFrame((frame) => frame.includes("Setup keymap ran 1")) + setup.mockInput.pressKey("F11") + await Bun.sleep(100) + await setup.renderOnce() + expect(setup.captureCharFrame()).toContain("Setup keymap ran 1") + expect(setup.captureCharFrame()).not.toContain("Dialog keymap ran 2") +}) + +test("a component layer ends with its activation on reload while the component stays mounted", async () => { + await using plugin = await copyPlugin() + await using setup = await createAppFixture({ config: { animations: false, plugins: [plugin.directory] } }) + await setup.ready + await setup.waitForFrame((frame) => frame.includes("Fixture ready generation 1")) + + setup.mockInput.pressKey("F8") + await setup.waitForFrame((frame) => frame.includes("Fixture dialog")) + setup.mockInput.pressKey("F11") + await setup.waitForFrame((frame) => frame.includes("Dialog keymap ran 1")) + + await Bun.write(plugin.entry, (await Bun.file(plugin.entry).text()).replace("generation 1", "generation 2")) + await setup.waitForFrame((frame) => frame.includes("Fixture ready generation 2")) + setup.mockInput.pressKey("g", { ctrl: true }) + await setup.waitForFrame((frame) => frame.includes("Setup keymap ran 1")) + setup.mockInput.pressKey("F11") + await Bun.sleep(100) + await setup.renderOnce() + expect(setup.captureCharFrame()).toContain("Setup keymap ran 1") + expect(setup.captureCharFrame()).toContain("Fixture dialog") + expect(setup.captureCharFrame()).not.toContain("Dialog keymap ran 2") +}) + +test("failed setup reports batched layer errors and disposes earlier and later setup layers", async () => { + await using plugin = await copyPlugin() + await using setup = await createAppFixture({ + config: { animations: false, plugins: [{ package: plugin.directory, options: { fail: true } }] }, + }) + await setup.ready + await setup.waitForFrame((frame) => frame.includes("ctrl+p commands")) + + await openPlugin(setup) + await setup.waitForFrame((frame) => frame.includes("view error")) + setup.mockInput.pressKey(" ") + await setup.waitForFrame((frame) => frame.includes("Palette commands require an ID")) + setup.mockInput.pressEscape() + setup.mockInput.pressEscape() + + await Bun.sleep(100) + setup.mockInput.pressKey("F9") + setup.mockInput.pressKey("F10") + await Bun.sleep(100) + await setup.renderOnce() + expect(setup.captureCharFrame()).not.toContain("Sync keymap ran") + expect(setup.captureCharFrame()).not.toContain("Late keymap ran") +}) + +async function copyPlugin() { + const root = await tmpdir() + const directory = path.join(root.path, "setup-keymap") + const entry = path.join(directory, "tui.tsx") + await mkdir(directory) + await symlink(path.join(import.meta.dir, "../node_modules"), path.join(directory, "node_modules")) + await Bun.write(entry, Bun.file(path.join(import.meta.dir, "fixture/plugin/setup-keymap/tui.tsx"))) + return { directory, entry, [Symbol.asyncDispose]: root[Symbol.asyncDispose] } +} + +async function search(setup: Fixture, query: string) { + setup.mockInput.pressKey("p", { ctrl: true }) + await setup.waitForFrame((frame) => frame.includes("Commands")) + await setup.waitFor(() => setup.renderer.currentFocusedEditor instanceof InputRenderable) + await setup.mockInput.typeText(query) +} + +async function openPlugin(setup: Fixture) { + await setup.mockInput.typeText("/plugins") + await setup.waitForFrame((frame) => frame.includes("/plugins")) + setup.mockInput.pressEnter() + await setup.waitForFrame((frame) => frame.includes("fixture.setup-keymap")) + await setup.waitFor(() => setup.renderer.currentFocusedEditor instanceof InputRenderable) + await setup.mockInput.typeText("setup-keymap") +} diff --git a/services/www/src/docs/content/build/plugins/cli.mdx b/services/www/src/docs/content/build/plugins/cli.mdx index 52f7cabd557..7d7aa52a814 100644 --- a/services/www/src/docs/content/build/plugins/cli.mdx +++ b/services/www/src/docs/content/build/plugins/cli.mdx @@ -265,6 +265,23 @@ context.keymap.layer(() => ({ })) ``` +Call `layer` from `setup`, including after an `await`, or from a component. Each layer is removed at the first of these: + +- The plugin is disabled, reloaded, or fails setup. +- The component or reactive computation that created it, such as a slot, route, or dialog, is disposed. + +The layer factory must be pure. OpenCode evaluates it once without dependency tracking to validate the initial command shape, then reactively. Initial command-shape errors, such as a palette command without an ID, throw from `layer()`. OpenTUI handles binding-parse errors. Errors from later reactive updates follow Solid's error handling. + +```ts +async setup(context) { + const settings = await loadSettings() + context.keymap.layer(() => ({ + mode: "global", + commands: [{ id: "acme.sync", title: "Sync Acme", palette: true, run: () => sync(settings) }], + })) +} +``` + A layer may target one OpenTUI renderable and can return `false` from a command to continue keyboard dispatch. ```ts