diff --git a/docs/docs/configure/skills.md b/docs/docs/configure/skills.md index 9978116285..a6d3e8df84 100644 --- a/docs/docs/configure/skills.md +++ b/docs/docs/configure/skills.md @@ -201,7 +201,7 @@ altimate-code skill publish my-tool # upload every file in the skill dir ### TUI -Open the skill browser with `ctrl+i` when no other dialog is open, or type `/skills` in the prompt: +Open the skill browser by typing `/skills` in the prompt (or `k`): ![Skill Browser](../assets/images/skills/tui-skill-browser.png) @@ -209,17 +209,18 @@ Open the skill browser with `ctrl+i` when no other dialog is open, or type `/ski | Key | Action | |-----|--------| -| `ctrl+i` | Open skill browser (when no dialog is open) / Install skill (when inside browser) | | Enter | Use — inserts `/` into the prompt | | `ctrl+a` | Actions — show, edit, test, remove, or publish the selected skill to the linked workspace (the publish row appears only with `ALTIMATE_WORKSPACE=1`) | -| `ctrl+n` | New — scaffold a new skill + CLI tool | +| `ctrl+e` | New — scaffold a new skill + CLI tool (`ctrl+n` moves down the list, as in every dialog) | +| `ctrl+g` | Install a skill from a GitHub repo, URL, or local path (`ctrl+i` is Tab in most terminals, so it cannot be the chord) | +| Tab / Shift+Tab | Move between the **Actions · New · Install** buttons in the footer, then Enter — the same three without a chord | | Esc | Back — returns to previous screen | -**Create skill** (`ctrl+n`): +**Create skill** (`ctrl+e`, or the **New** footer button): ![Create Skill Dialog](../assets/images/skills/tui-skill-create.png) -**Install skill** (`ctrl+i` inside browser): +**Install skill** (`ctrl+g`, or the **Install** footer button): ![Install Skill Dialog](../assets/images/skills/tui-skill-install.png) diff --git a/docs/docs/configure/tools/custom.md b/docs/docs/configure/tools/custom.md index c7b70ea276..c960986bef 100644 --- a/docs/docs/configure/tools/custom.md +++ b/docs/docs/configure/tools/custom.md @@ -73,7 +73,7 @@ altimate-code skill install https://github.com/owner/repo/tree/main/skills/my-sk altimate-code skill remove my-skill ``` -Or use the TUI: type `/skills`, then `ctrl+i` to install or `ctrl+a` → Remove to delete. +Or use the TUI: type `/skills`, then `ctrl+g` (or the **Install** footer button) to install, or `ctrl+a` → Remove to delete. ### Output Conventions diff --git a/packages/opencode/src/plugin/tui/altimate/skill-ops.tsx b/packages/opencode/src/plugin/tui/altimate/skill-ops.tsx index 3421e0e88b..856c113060 100644 --- a/packages/opencode/src/plugin/tui/altimate/skill-ops.tsx +++ b/packages/opencode/src/plugin/tui/altimate/skill-ops.tsx @@ -781,9 +781,61 @@ function DialogSkillList(props: { api: TuiPluginApi; onCurrent: (skill: string | } // altimate_change end props.onCurrent(item.value) - // Selecting a skill opens its action picker (the pre-merge default action was the picker). + // altimate_change start — Enter USES the skill: it inserts `/ ` into the + // prompt, as the docs say and as the core selector this dialog now replaces did + // (#1328). The action picker moved to ctrl+a and the "Actions" footer button. + const ref = api.prompt.active() + if (ref) { + ref.set({ ...ref.current, input: `/${item.value} `, parts: [] }) + api.ui.dialog.clear() + ref.focus() + return + } + // No prompt to write into (no session mounted): fall back to the picker. openActionPicker(api, skillMap().get(item.value), item.value, () => showList(api)) + // altimate_change end }} + // altimate_change start — the picker, create and install as DIALOG actions (#1328). + // The plugin's global keymap layer registers ctrl+a / ctrl+n / ctrl+i too, but while + // this dialog is open its own layer outranks that one: ctrl+a went to the filter + // input's line-home and ctrl+n to `dialog.select.next`. Declared here they are bound + // inside the dialog (the model dialog binds ctrl+a the same way) and rendered as + // footer buttons reachable with Tab, so the picker no longer depends on a chord at + // all. ctrl+n stays the dialog's own "next"; New is ctrl+e in here. Install is + // ctrl+g, not ctrl+i: most terminals send ctrl+i as byte 0x09, which is Tab — the + // footer's own key (bot review). + actions={[ + { + command: "altimate.skill.list.actions", + title: "Actions", + disabled: (option) => option === undefined || option.value === INSTALL_ACTION_VALUE, + onTrigger: (item) => { + if (!item || item.value === INSTALL_ACTION_VALUE) return + props.onCurrent(item.value) + openActionPicker(api, skillMap().get(item.value), item.value, () => showList(api)) + }, + }, + // New and Install need no highlighted row: typing a name that matches no + // installed skill and pressing ctrl+e is the create-from-filter flow. + { + command: "altimate.skill.list.create", + title: "New", + standalone: true, + onTrigger: () => showCreate(api, filter().trim() || undefined), + }, + { + command: "altimate.skill.list.install", + title: "Install", + standalone: true, + onTrigger: () => showInstall(api, filter().trim() || undefined), + }, + ]} + bindings={[ + { key: "ctrl+a", cmd: "altimate.skill.list.actions" }, + { key: "ctrl+e", cmd: "altimate.skill.list.create" }, + { key: "ctrl+g", cmd: "altimate.skill.list.install" }, + ]} + // altimate_change end /> ) } @@ -884,11 +936,13 @@ const tui: TuiPlugin = async (api) => { // ctrl+a -> actions · ctrl+n -> create · ctrl+i -> install. // altimate_change start — restore a default key to OPEN the skills list (pre-merge skill_list // was ctrl+i, which now collides with tab/agent-cycle; use a collision-free k instead). + // Install has no global chord: ctrl+i is Tab on the wire for most terminals, and + // ctrl+g is the session route's "first message". Inside the browser it is ctrl+g + // (a dialog-local binding, see DialogSkillList); from anywhere else, the palette. bindings: [ { key: "k", cmd: "altimate.skill.list" }, { key: "ctrl+a", cmd: "altimate.skill.actions" }, { key: "ctrl+n", cmd: "altimate.skill.create" }, - { key: "ctrl+i", cmd: "altimate.skill.install" }, ], // altimate_change end }) diff --git a/packages/plugin/src/tui.ts b/packages/plugin/src/tui.ts index 70c15b8f46..917be70b12 100644 --- a/packages/plugin/src/tui.ts +++ b/packages/plugin/src/tui.ts @@ -183,9 +183,32 @@ export type TuiDialogSelectProps = { // altimate_change start — a fixed-option dialog can hide the filter box entirely renderFilter?: boolean // altimate_change end + // altimate_change start — dialog-level actions: footer buttons (Tab-reachable) with + // keybinds that are live INSIDE the dialog, where the dialog's own layer outranks a + // plugin's global keymap layer. Mirrors the host DialogSelect `actions`/`bindings`. + actions?: TuiDialogSelectAction[] + bindings?: { key: string; cmd: string }[] + // altimate_change end current?: Value } +// altimate_change start +export type TuiDialogSelectAction = { + /** Command name the `bindings` entries refer to. */ + command: string + title: string + side?: "left" | "right" + hidden?: boolean + disabled?: boolean | ((option: TuiDialogSelectOption | undefined) => boolean) + /** Called with the highlighted option — or with `undefined` when `standalone` is set + * and no row is highlighted (empty list, nothing matches the filter). */ + onTrigger: (option: TuiDialogSelectOption | undefined) => void + /** The action needs no highlighted row (create, install): it fires even when the + * list is empty or the filter matches nothing. */ + standalone?: boolean +} +// altimate_change end + export type TuiPromptInfo = { input: string mode?: "normal" | "shell" diff --git a/packages/tui/src/component/prompt/index.tsx b/packages/tui/src/component/prompt/index.tsx index 7c8c5594bf..64e99b58cf 100644 --- a/packages/tui/src/component/prompt/index.tsx +++ b/packages/tui/src/component/prompt/index.tsx @@ -593,8 +593,21 @@ export function Prompt(props: PromptProps) { title: "Skills", name: "prompt.skills", category: "Prompt", - slashName: "skills", + // altimate_change start — `/skills` belongs to the Altimate skills browser + // (`altimate.skill.list`: browse, actions, create, install). This command kept the + // same slash name, so autocomplete listed two `/skills` rows and Enter took this + // one — the plain selector with no actions — which is why ctrl+a never opened the + // picker (#1328). It has no slash name now, and its other two entry points — the + // palette row and a configured `prompt_skills` keybind — hand over to the browser + // when it is registered, so no route lands on the plain selector while a better + // one exists. Hidden from the palette then, too: two "Skills" rows invite the + // wrong one. + get hidden() { + return keymap.getCommands({ visibility: "registered", filter: { name: "altimate.skill.list" } }).length > 0 + }, run: () => { + if (keymap.dispatchCommand("altimate.skill.list").ok) return + // altimate_change end dialog.replace(() => ( { diff --git a/packages/tui/src/plugin/adapters.tsx b/packages/tui/src/plugin/adapters.tsx index 5fb7b7dd02..19a94cc4a5 100644 --- a/packages/tui/src/plugin/adapters.tsx +++ b/packages/tui/src/plugin/adapters.tsx @@ -1,4 +1,6 @@ -import type { TuiDialogSelectOption, TuiPluginApi, TuiPromptRef, TuiSlotProps } from "@opencode-ai/plugin/tui" +// altimate_change start — TuiDialogSelectProps for the generic DialogSelect adapter +import type { TuiDialogSelectOption, TuiDialogSelectProps, TuiPluginApi, TuiPromptRef, TuiSlotProps } from "@opencode-ai/plugin/tui" +// altimate_change end import type { TuiConfig } from "../config" import type { useEvent } from "../context/event" import type { usePromptRef } from "../context/prompt" @@ -234,7 +236,10 @@ export function createTuiApiAdapters(input: Input): Omit }, - DialogSelect(props) { + // altimate_change start — generic over the option value so the dialog-level + // actions below can be typed against it + DialogSelect(props: TuiDialogSelectProps) { + // altimate_change end return ( ({ + command: action.command, + title: action.title, + side: action.side, + hidden: action.hidden, + disabled: + typeof action.disabled === "function" + ? (option: SelectOption | undefined) => + (action.disabled as (o: TuiDialogSelectOption | undefined) => boolean)( + option ? pickOption(option) : undefined, + ) + : action.disabled, + standalone: true as const, + onTrigger: (option: SelectOption | undefined) => { + // The plugin API's shape is the row-bound one unless `standalone`; the + // core gate is applied here so a plugin action without a row is not called. + if (!option && !action.standalone) return + action.onTrigger(option ? pickOption(option) : undefined) + }, + }))} + bindings={props.bindings} + // altimate_change end current={props.current} /> ) diff --git a/packages/tui/src/ui/dialog-select.tsx b/packages/tui/src/ui/dialog-select.tsx index 99299f4e22..17f11ca1c6 100644 --- a/packages/tui/src/ui/dialog-select.tsx +++ b/packages/tui/src/ui/dialog-select.tsx @@ -20,6 +20,19 @@ import { getScrollAcceleration } from "../util/scroll" import { useTuiConfig } from "../config" import { formatKeyBindings, useBindings, useKeymapSelector } from "../keymap" +// altimate_change start — see `actions` +type DialogSelectActionBase = { + command: string + title: string + side?: "left" | "right" + hidden?: boolean + disabled?: boolean | ((option: DialogSelectOption | undefined) => boolean) +} +export type DialogSelectAction = + | (DialogSelectActionBase & { standalone?: false; onTrigger: (option: DialogSelectOption) => void }) + | (DialogSelectActionBase & { standalone: true; onTrigger: (option: DialogSelectOption | undefined) => void }) +// altimate_change end + export interface DialogSelectProps { title: string titleView?: JSX.Element @@ -35,14 +48,12 @@ export interface DialogSelectProps { skipFilter?: boolean renderFilter?: boolean locked?: boolean - actions?: { - command: string - title: string - side?: "left" | "right" - hidden?: boolean - disabled?: boolean | ((option: DialogSelectOption | undefined) => boolean) - onTrigger: (option: DialogSelectOption) => void - }[] + // altimate_change start — a `standalone` action needs no highlighted row (create, + // install): it fires with `undefined` when the list is empty or nothing matches the + // filter. The default keeps the row-bound contract every existing caller relies on, + // as a discriminated union so those callers' `onTrigger` still types as row-bound. + actions?: DialogSelectAction[] + // altimate_change end footerHints?: { title: string label: string @@ -135,11 +146,15 @@ export function DialogSelect(props: DialogSelectProps) { .filter((item) => item.label), ...(props.footerHints ?? []), ]) - const actionItems = createMemo(() => + // altimate_change start — evaluated lazily rather than as an eager memo: `isActionDisabled` + // reads `selected()`, which is declared further down, so a function-valued `disabled` + // (the Skills browser's, #1328) threw "Cannot access 'selected' before initialization" + // during setup. Every existing caller passed a boolean, which never touched `selected`. + const actionItems = () => visibleActions() .filter(isActionItem) - .filter((item) => !isActionDisabled(item)), - ) + .filter((item) => !isActionDisabled(item)) + // altimate_change end createEffect(() => { const index = focusedAction() @@ -371,8 +386,10 @@ export function DialogSelect(props: DialogSelectProps) { if (isActionDisabled(item)) return setStore("input", "keyboard") const option = selected() - if (!option) return - item.onTrigger(option) + // altimate_change start — see `standalone` + if (item.standalone) item.onTrigger(option) + else if (option) item.onTrigger(option) + // altimate_change end }, })), ], @@ -434,8 +451,10 @@ export function DialogSelect(props: DialogSelectProps) { if (!item || !isActionItem(item) || isActionDisabled(item)) return setStore("input", "keyboard") const option = selected() - if (!option) return - item.onTrigger(option) + // altimate_change start — see `standalone` + if (item.standalone) item.onTrigger(option) + else if (option) item.onTrigger(option) + // altimate_change end } function isActionItem(item: VisibleAction): item is Action & { label: string } { diff --git a/packages/tui/test/ui/dialog-select-actions.test.tsx b/packages/tui/test/ui/dialog-select-actions.test.tsx new file mode 100644 index 0000000000..d5c0587748 --- /dev/null +++ b/packages/tui/test/ui/dialog-select-actions.test.tsx @@ -0,0 +1,289 @@ +/** @jsxImportSource @opentui/solid */ +// altimate_change — dialog-level actions with in-dialog keybinds (#1328). +// +// The Skills browser's "Actions" picker was reachable only through a plugin-registered +// global keymap layer, which the open dialog's own layer (and its focused filter input) +// outranked, so ctrl+a never opened it. Declared as DialogSelect `actions` with `bindings` +// the chord is handled inside the dialog. This mounts a real DialogSelect in the provider +// stack and presses the key. +import { createDefaultOpenTuiKeymap } from "@opentui/keymap/opentui" +import { testRender, useRenderer } from "@opentui/solid" +import { expect, test } from "bun:test" +import { onCleanup } from "solid-js" +import { createTuiResolvedConfig } from "../fixture/tui-runtime" +import { TestTuiContexts } from "../fixture/tui-environment" + +async function wait(fn: () => boolean, timeout = 2000) { + const start = Date.now() + while (!fn()) { + if (Date.now() - start > timeout) throw new Error("timed out waiting for condition") + await Bun.sleep(10) + } +} + +async function mount( + opts: { bindings?: { key: string; cmd: string }[]; globalLayer?: boolean; via?: "core" | "adapter" } = {}, +) { + const [ + { DialogProvider, useDialog }, + { DialogSelect: CoreDialogSelect }, + { createTuiApiAdapters }, + { ThemeProvider }, + { TuiConfigProvider }, + { OpencodeKeymapProvider, registerOpencodeKeymap }, + { KVProvider }, + { ArgsProvider }, + { ToastProvider }, + ] = await Promise.all([ + import("../../src/ui/dialog"), + import("../../src/ui/dialog-select"), + import("../../src/plugin/adapters"), + import("../../src/context/theme"), + import("../../src/config"), + import("../../src/keymap"), + import("../../src/context/kv"), + import("../../src/context/args"), + import("../../src/ui/toast"), + ]) + const triggered: string[] = [] + + // The plugin-API shape, exactly as skill-ops.tsx declares it: a function-valued + // `disabled` (the synthetic Install row must not open the picker), and New / Install + // `standalone` so they fire with no highlighted row. + const actions = [ + { + command: "altimate.skill.list.actions", + title: "Actions", + disabled: (o: { value: string } | undefined) => o === undefined || o.value === "__install__", + onTrigger: (o: { value: string } | undefined) => triggered.push(`actions:${o?.value}`), + }, + { command: "altimate.skill.list.create", title: "New", standalone: true, onTrigger: () => triggered.push("create") }, + { command: "altimate.skill.list.install", title: "Install", standalone: true, onTrigger: () => triggered.push("install") }, + // Row-bound with NO `disabled` function: the shape the adapter's own gate exists + // for (the core cannot refuse it before `onTrigger`). (bot review) + { command: "altimate.skill.list.plain", title: "Plain", onTrigger: (o: { value: string } | undefined) => triggered.push(`plain:${o?.value}`) }, + ] + const bindings = opts.bindings ?? [ + { key: "ctrl+a", cmd: "altimate.skill.list.actions" }, + { key: "ctrl+e", cmd: "altimate.skill.list.create" }, + { key: "ctrl+g", cmd: "altimate.skill.list.install" }, + { key: "ctrl+o", cmd: "altimate.skill.list.plain" }, + ] + const options = [ + { title: "alpha", value: "alpha" }, + { title: "beta", value: "beta" }, + ] + + function Opener() { + const dialog = useDialog() + if (opts.via === "adapter") { + // Through the plugin API adapter — the seam skill-ops.tsx really goes through — + // so a dropped `actions`/`bindings`/`standalone` forward fails here. The adapter's + // DialogSelect reads only its props, so the rest of the input is not needed. + const api = createTuiApiAdapters({ + version: "0", + tuiConfig: { keybinds: { gather: () => [], get: () => [] } }, + keymap: { registerLayer: () => () => {} }, + dialog, + } as never) + dialog.replace(() => ) + return + } + dialog.replace(() => ( + ({ ...a, standalone: a.standalone === true }))} + bindings={bindings} + /> + )) + return + } + + function Harness() { + const renderer = useRenderer() + const keymap = createDefaultOpenTuiKeymap(renderer) + const resolvedConfig = createTuiResolvedConfig({ leader_timeout: 1000 }) + const off = registerOpencodeKeymap(keymap, renderer, resolvedConfig) + onCleanup(off) + // The plugin's global layer, as skill-ops.tsx registers it at plugin init: the same + // command name, bound to the same chord, active everywhere. + if (opts.globalLayer) { + const offGlobal = keymap.registerLayer({ + commands: [ + { + name: "altimate.skill.actions", + title: "Skill actions", + run() { + triggered.push("global") + }, + }, + ], + bindings: [{ key: "ctrl+a", cmd: "altimate.skill.actions" }], + }) + onCleanup(offGlobal) + } + return ( + + + + + + + + + + + + + + + + + + ) + } + + const app = await testRender(() => ) + await Bun.sleep(50) + return { app, triggered } +} + +test("ctrl+a inside an open DialogSelect triggers the declared action for the highlighted row", async () => { + const { app, triggered } = await mount() + try { + app.mockInput.pressKey("a", { ctrl: true }) + await wait(() => triggered.length > 0) + expect(triggered).toEqual(["actions:alpha"]) + } finally { + app.renderer.destroy() + } +}) + +test("the action follows the highlight: Down then ctrl+a names the second row", async () => { + const { app, triggered } = await mount() + try { + app.mockInput.pressKey("ARROW_DOWN") + await Bun.sleep(20) + app.mockInput.pressKey("a", { ctrl: true }) + await wait(() => triggered.length > 0) + expect(triggered).toEqual(["actions:beta"]) + } finally { + app.renderer.destroy() + } +}) + +test("a second action with its own chord fires independently", async () => { + const { app, triggered } = await mount() + try { + app.mockInput.pressKey("e", { ctrl: true }) + await wait(() => triggered.length > 0) + expect(triggered).toEqual(["create"]) + } finally { + app.renderer.destroy() + } +}) + +test("without a binding the chord does nothing — the test proves the binding is what carries it", async () => { + const { app, triggered } = await mount({ bindings: [] }) + try { + app.mockInput.pressKey("a", { ctrl: true }) + await Bun.sleep(150) + expect(triggered).toEqual([]) + } finally { + app.renderer.destroy() + } +}) + +test("REPRO: with the plugin's global layer registering the same command name, the dialog action is what fires", async () => { + const { app, triggered } = await mount({ globalLayer: true }) + try { + app.mockInput.pressKey("a", { ctrl: true }) + await Bun.sleep(200) + expect(triggered).toEqual(["actions:alpha"]) + } finally { + app.renderer.destroy() + } +}) + +test("the actions render as footer buttons with their chords, so the picker is discoverable without one", async () => { + const { app } = await mount({ globalLayer: true }) + try { + await app.renderOnce() + const frame = app.captureCharFrame() + expect(frame).toContain("Actions") + expect(frame).toContain("New") + expect(frame).toMatch(/ctrl\+a|\^a/i) + } finally { + app.renderer.destroy() + } +}) + +// Install is ctrl+g because ctrl+i is Tab on the wire for most terminals (byte 0x09), and +// Tab is the footer's own key. A Tab press must move footer focus, not run Install; the +// chord that runs it must be one no terminal folds into Tab. (bot review on #1342) +test("Tab walks the footer (Enter then activates the focused button) and does not run Install; ctrl+g does", async () => { + const { app, triggered } = await mount() + try { + app.mockInput.pressKey("TAB") + await Bun.sleep(150) + expect(triggered).toEqual([]) + // Tab moved focus to the first footer button (Actions); Enter activates it. + app.mockInput.pressKey("RETURN") + await wait(() => triggered.length > 0) + expect(triggered).toEqual(["actions:alpha"]) + app.mockInput.pressKey("g", { ctrl: true }) + await wait(() => triggered.length > 1) + expect(triggered).toEqual(["actions:alpha", "install"]) + } finally { + app.renderer.destroy() + } +}) + +// codex on #1342: New and Install need no highlighted row. Typing a name that matches no +// installed skill and pressing ctrl+e is the create-from-filter flow, and it did nothing. +test("with nothing matching the filter, ctrl+e still creates and ctrl+a (row-bound) does nothing", async () => { + const { app, triggered } = await mount() + try { + for (const ch of "zzz") app.mockInput.pressKey(ch) + await Bun.sleep(50) + app.mockInput.pressKey("a", { ctrl: true }) + await Bun.sleep(100) + expect(triggered).toEqual([]) + app.mockInput.pressKey("e", { ctrl: true }) + await wait(() => triggered.length > 0) + expect(triggered).toEqual(["create"]) + } finally { + app.renderer.destroy() + } +}) + +test("through the plugin API adapter: chords fire, standalone survives the mapping, row-bound gate holds", async () => { + const { app, triggered } = await mount({ via: "adapter" }) + try { + app.mockInput.pressKey("a", { ctrl: true }) + await wait(() => triggered.length > 0) + expect(triggered).toEqual(["actions:alpha"]) + for (const ch of "zzz") app.mockInput.pressKey(ch) + await Bun.sleep(50) + app.mockInput.pressKey("o", { ctrl: true }) // row-bound, no `disabled`: the adapter gate alone stops it + await Bun.sleep(100) + expect(triggered).toEqual(["actions:alpha"]) + app.mockInput.pressKey("g", { ctrl: true }) + await wait(() => triggered.length > 1) + expect(triggered).toEqual(["actions:alpha", "install"]) + } finally { + app.renderer.destroy() + } +}) + +test("the plain row-bound action does fire with a row (so the no-row assertion above is not vacuous)", async () => { + const { app, triggered } = await mount({ via: "adapter" }) + try { + app.mockInput.pressKey("o", { ctrl: true }) + await wait(() => triggered.length > 0) + expect(triggered).toEqual(["plain:alpha"]) + } finally { + app.renderer.destroy() + } +})