diff --git a/.changeset/fuzzy-agents-guide.md b/.changeset/fuzzy-agents-guide.md new file mode 100644 index 000000000..54cf4e668 --- /dev/null +++ b/.changeset/fuzzy-agents-guide.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": minor +--- + +Add custom React/OpenTUI dialog surfaces to the extension API and run Hunk's Agent Skill onboarding as a bundled extension. diff --git a/docs/extension-architecture.md b/docs/extension-architecture.md index edd7c28d4..5e637a04f 100644 --- a/docs/extension-architecture.md +++ b/docs/extension-architecture.md @@ -236,12 +236,14 @@ modal keys also remain outside the table and therefore outside the event. `ctx.dialogs` is the one place extension code can interrupt the user, so its ordering and settlement live outside React in `src/ui/lib/extensionDialogs.ts` — one FIFO queue per App instance, minting a -per-extension `dialogs` object, normalizing (and sanitizing) extension-authored -text into a request the host draws, and answering by request id so a duplicated +per-extension `dialogs` object, normalizing host-rendered prompts or retaining a +custom component request, and answering by request id so a duplicated Enter cannot spill onto whatever was queued behind. App subscribes with `useSyncExternalStore`, renders the current request through `src/ui/components/chrome/ExtensionDialog.tsx` (confirm reuses `ConfirmDialog`; -select and input are `ModalFrame` surfaces), and unmount calls `shutdown()` so +select and input are `ModalFrame` surfaces; `open` mounts a guarded public +React/OpenTUI component inside exact clamped bounds), and +unmount calls `shutdown()` so every pending and queued dialog resolves its cancel value instead of leaving a handler awaiting forever. Key precedence in `useAppKeyboardShortcuts` places dialogs below Hunk's own app-critical prompts (repo trust, save-on-quit) and @@ -254,8 +256,9 @@ must not be able to impersonate Hunk. The host derives the extension's trusted bundled origin from registry metadata and omits the redundant marker only for Hunk-owned bundled UI. `src/ui/lib/modalGeometry.ts` clamps the frame before extension text is wrapped or windowed, so measurement and rendering use the -same terminal width; body/options yield rows to a pinned mouse-clickable action -footer on short terminals. +same terminal width. Custom components receive the remaining exact rectangle +after required attribution and own layout within it; Hunk retains Escape, +clipboard mediation, queue settlement, and render-failure containment. Lifecycle and bus handlers receive that same attributed dialog queue plus the same guarded live navigation commands use. `App` installs both through the diff --git a/docs/extensions.md b/docs/extensions.md index 1c74845d6..9e38ecaf5 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -280,8 +280,9 @@ new instances and run that shutdown/startup pair around the replacement. ### `hunk.apiVersion` -The API generation this Hunk speaks (currently `16`). Branch on it if you want -one file to support several Hunk versions. Version 16 adds temporary application +The API generation this Hunk speaks (currently `17`). Branch on it if you want +one file to support several Hunk versions. Version 17 adds custom React/OpenTUI +dialog surfaces; version 16 added temporary application handoffs and on-disk location resolution to command handlers; version 15 added `{ side, line }` to opted-in pane `currentLine` paint; version 14 added structured `rangeEndpoints` @@ -1624,12 +1625,13 @@ your extension. #### Asking the user -`ctx.dialogs` puts a question on screen and waits for the answer. Three shapes, -all promise-returning: +`ctx.dialogs` puts a modal surface on screen and waits for it to settle. Four +shapes, all promise-returning: - `confirm({ title, body?, confirmLabel?, cancelLabel? })` → `true` or `false` - `select({ title, options })` → the chosen string, or `null` - `input({ title, placeholder?, initial? })` → the typed string, or `null` +- `open({ title, width?, height?, component })` → `void` when closed ```ts hunk.registerCommand( @@ -1673,18 +1675,74 @@ hunk.registerCommand({ id: "pick-hunk", title: "Pick a hunk", key: "ctrl+k" }, a }); ``` -Hunk draws the dialog, not you: your text fills the title, body, and choices, -and dialogs from installed extensions carry an `ext ` attribution line -— the same marker `notify` toasts use — so a third-party prompt can never present -itself as Hunk asking. Hunk's own bundled extensions omit that redundant marker. +`open` mounts a React/OpenTUI component in an exact host-owned rectangle, like +`registerPane` inside modal chrome. `width` and `height` request the preferred +component size (defaults `64×12`, maximum `240×100`); Hunk clamps both to the +terminal before passing the resulting dimensions, semantic theme, +`copySupported`, and guarded `actions` to the component. Escape stays +host-owned. Other keys reach the component, and `actions.close()` resolves the +promise. + +```tsx +import { useKeyboard } from "@opentui/react"; +import { matchesKey, type ExtensionDialogProps } from "hunkdiff/extension"; + +const prompt = "Review the current Hunk session. Focus on correctness."; + +function AgentSetupDialog({ actions, copySupported, theme }: ExtensionDialogProps) { + const copy = () => { + actions.notify(actions.copy(prompt) ? "Copied agent prompt" : "Clipboard copy failed"); + }; + useKeyboard((key) => { + if (!copySupported || !matchesKey("c", key)) return; + key.preventDefault(); + key.stopPropagation(); + copy(); + }); + + return ( + + {prompt} + + + {copySupported ? "Copy prompt" : "Copy unavailable"} + + + + ); +} + +hunk.registerCommand({ id: "agent-setup", title: "Agent setup" }, async (ctx) => { + await ctx.dialogs.open({ + title: "Agent setup", + width: 64, + height: 6, + component: AgentSetupDialog, + }); +}); +``` + +`actions.copy(text)` uses Hunk's OSC 52 integration, strips terminal control +sequences, expands tabs to four spaces, and returns whether the renderer accepted +the bounded payload (maximum 16,384 JavaScript string code units). +`actions.notify(message)` shows a short host status message, and +`actions.close()` dismisses the modal. A render failure is contained to the +component and leaves a dismissible fallback. + +Component dialogs are trusted extension code, just like pane components: Hunk +cannot verify that an arbitrary surface visually discloses what it passes to +`actions.copy`. Hunk owns the frame, title, bounds, Escape handling, and an +`ext ` attribution line for installed extensions. Hunk's bundled UI +omits that redundant marker. One dialog is on screen at a time. Concurrent requests queue in call order, -across extensions too, so a second question waits its turn instead of replacing -the first. While a dialog is up it owns the keyboard: Escape cancels (`false`, -or `null`), Enter accepts — the confirm action, the highlighted option, or the -typed text — and review shortcuts stay suppressed underneath. Confirm dialogs -also answer to `y`/`n`, select dialogs to `↑`/`↓`, and every dialog's actions -and rows are clickable. +across extensions too, so a second modal waits its turn instead of replacing +the first. While a dialog is up it owns the keyboard: Escape cancels (`false` +or `null`) or closes a component dialog, Enter accepts the confirm action, +highlighted option, or typed text, and review shortcuts stay suppressed +underneath. Component-dialog keys other than Escape reach the mounted surface. +Confirm dialogs also answer to `y`/`n`, +select dialogs to `↑`/`↓`, and every dialog's actions and rows are clickable. Two things resolve a dialog without the user: the session moving on, and bad arguments. A session reload — the refresh key, a watch-triggered reload, an diff --git a/skills/hunk-extensions/SKILL.md b/skills/hunk-extensions/SKILL.md index a024daf5e..6cb56abd5 100644 --- a/skills/hunk-extensions/SKILL.md +++ b/skills/hunk-extensions/SKILL.md @@ -110,7 +110,7 @@ bad or duplicate id is skipped with a startup notice. | Coordinate with another loaded extension | `hunk.events.emit` / `hunk.events.on` | | Read user-supplied settings | `hunk.config` (`[extension.]` table) | | Snapshot stable files and every saved review note | `ctx.review.snapshot()` in a command | -| Branch on the API generation (currently `16`) | `hunk.apiVersion` | +| Branch on the API generation (currently `17`) | `hunk.apiVersion` | Registration is only valid while the factory runs — Hunk seals the API object afterwards. @@ -160,13 +160,16 @@ transform — gets `ctx.cwd` and `ctx.notify(message, type?)`. A file view's (`isEnabled`/`execute` for public semantic `hunk.*` commands), `ctx.keyboardModes` (enter/exit/probe this extension's session modes), `ctx.review` (deeply immutable snapshots of stable files and complete saved store notes), - `ctx.dialogs` (`confirm`/`select`/`input`, queued and attributed), + `ctx.dialogs` (`confirm`/`select`/`input` plus `open` for a custom OpenTUI component, + queued and attributed), `ctx.openInApp` (temporary terminal ownership around extension-run applications), and `ctx.workspace` (`readDocument`, `resolveLocation`, `canWriteDocument`, `writeDocument` with consent). - **Pane components** get frozen `files`, selection, placement, exact dimensions, optional `currentLine` paint (with `{ side, line }` when opted in), semantic `theme`, resolved `keybindings`, and guarded navigation/notification `actions`. +- **Dialog components** get exact clamped dimensions, semantic `theme`, clipboard + availability, and guarded `close`/`copy`/`notify` actions. Escape remains host-owned. - **File-view `layout`** gets `file`, `width`, `signal`, `changes`, and a lazy `readDocument(side)`. - **File-view `mode` handlers** get `ctx.file` and `ctx.fileViews`. `onKey`, diff --git a/src/extension-api/index.ts b/src/extension-api/index.ts index f3f6d11a0..73bb76fe4 100644 --- a/src/extension-api/index.ts +++ b/src/extension-api/index.ts @@ -95,6 +95,10 @@ export type { ExtensionReviewSnapshotNote, ExtensionReviewSnapshotNoteAnchor, ExtensionConfirmOptions, + ExtensionDialogActions, + ExtensionDialogComponent, + ExtensionDialogOptions, + ExtensionDialogProps, ExtensionDialogs, ExtensionInputOptions, ExtensionSelectOptions, diff --git a/src/extension-api/types.ts b/src/extension-api/types.ts index 3268f97d6..53acaabd8 100644 --- a/src/extension-api/types.ts +++ b/src/extension-api/types.ts @@ -21,7 +21,7 @@ * Extensions can branch on `hunk.apiVersion` so a newer Hunk can keep loading * older extensions without guessing at their expectations. */ -export const HUNK_EXTENSION_API_VERSION = 16; +export const HUNK_EXTENSION_API_VERSION = 17; export type HunkExtensionApiVersion = typeof HUNK_EXTENSION_API_VERSION; export type ExtensionNotifyType = "info" | "warning" | "error"; @@ -1631,18 +1631,61 @@ export interface ExtensionInputOptions { initial?: string; } +/** Actions available while an extension-owned dialog component is mounted. */ +export interface ExtensionDialogActions { + /** Close this dialog and resolve its `open` promise. */ + close(): void; + /** + * Copy terminal-safe text through Hunk's OSC 52 integration. + * + * Hunk strips terminal control sequences, expands tabs to four spaces, and + * rejects empty or oversized payloads. Returns false when copying is + * unsupported, refused, or no longer belongs to the mounted dialog. + */ + copy(text: string): boolean; + /** Show one short host status message while this dialog remains current. */ + notify(message: string): void; +} + +/** Everything an extension-owned dialog component receives. */ +export interface ExtensionDialogProps { + /** Exact host-owned component width after terminal clamping. */ + readonly width: number; + /** Exact host-owned component height after terminal clamping and attribution. */ + readonly height: number; + readonly theme: ExtensionPaintTheme; + /** Whether Hunk's renderer currently supports clipboard writes. */ + readonly copySupported: boolean; + readonly actions: ExtensionDialogActions; +} + +/** A React/OpenTUI component mounted inside a host-owned modal frame. */ +export type ExtensionDialogComponent = (props: ExtensionDialogProps) => unknown; + +/** One extension-owned modal surface opened from a command or event handler. */ +export interface ExtensionDialogOptions { + title: string; + /** Preferred component width in terminal cells. Defaults to 64; maximum 240. */ + width?: number; + /** Preferred component height in terminal rows. Defaults to 12; maximum 100. */ + height?: number; + component: ExtensionDialogComponent; +} + /** - * Ask the user questions from a command handler, one modal at a time. + * Present modal interactions from a command handler, one at a time. * - * Every dialog is drawn by Hunk, not by the extension. Dialogs from installed - * extensions carry an attribution line naming their source, so a third-party - * prompt cannot present itself as Hunk asking; Hunk-owned bundled extensions + * Hunk draws every frame and every confirm/select/input surface; `open` mounts + * extension-owned content inside that frame. Dialogs from installed extensions + * carry an attribution line naming their source; Hunk-owned bundled extensions * omit that redundant marker. Only one dialog is on screen at a time: * concurrent requests queue in call order (FIFO), including across extensions, * so a second question waits for the first to be answered rather than replacing it. * - * Escape always cancels, resolving the cancel value (`false`, or `null`). - * Enter accepts: the confirm action, the highlighted option, or the typed text. + * Escape always dismisses, resolving the cancel value (`false`, `null`, or + * `undefined`). Enter accepts: the confirm action, the highlighted option, or + * the typed text. Open component dialogs remain mounted until they call + * `actions.close()` or the user presses Escape. * A session reload — the refresh key, a watch-triggered reload, an agent * command — cancels open and queued dialogs the same way: the review they * asked about is being replaced. A dialog raised while the app is tearing @@ -1650,8 +1693,9 @@ export interface ExtensionInputOptions { * never left hanging. * * Bad arguments are a programming error rather than a user answer, so they - * reject instead of resolving: a missing or blank `title`, or a `select` with - * no options. Because a dialog call is only useful awaited, the rejection + * reject instead of resolving: a missing or blank `title`, a `select` with no + * options, or invalid component-dialog dimensions. Because a dialog + * call is only useful awaited, the rejection * surfaces through the same path as any other handler failure — a warning toast * naming the extension. */ @@ -1662,6 +1706,8 @@ export interface ExtensionDialogs { select(options: ExtensionSelectOptions): Promise; /** Resolves the submitted text, or null on cancel/escape. */ input(options: ExtensionInputOptions): Promise; + /** Mount an extension-owned React/OpenTUI surface inside a host-owned modal. */ + open(options: ExtensionDialogOptions): Promise; } /** One whole-document replacement an extension asks the host to write. */ diff --git a/src/extensions/default/ui/agentSkill/index.test.ts b/src/extensions/default/ui/agentSkill/index.test.ts new file mode 100644 index 000000000..556b3fa77 --- /dev/null +++ b/src/extensions/default/ui/agentSkill/index.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, mock, test } from "bun:test"; +import type { ExtensionCommandContext } from "hunkdiff/extension"; +import { getBundledUIRegistry } from ".."; +import { AgentSkillDialog, BUNDLED_AGENT_SKILL_COMMAND_FULL_ID } from "."; + +/** Return the agent-skill registration from the process-static bundled UI registry. */ +function getBundledAgentSkillCommand() { + const registered = getBundledUIRegistry().commands.find( + ({ extensionId, command }) => + `${extensionId}.${command.id}` === BUNDLED_AGENT_SKILL_COMMAND_FULL_ID, + ); + if (!registered) throw new Error("Bundled agent skill command is missing."); + return registered; +} + +describe("bundled agent skill extension", () => { + test("registers the shared Hunk command identity without owning its host menu shell", () => { + const registered = getBundledAgentSkillCommand(); + + expect(registered.extensionId).toBe("hunk"); + expect(registered.command).toEqual({ + id: "app.openAgentSkill", + title: "Show setup guidance for reviewing with an agent", + }); + }); + + test("opens its onboarding through the public component dialog", async () => { + const open = mock(async () => {}); + const context = { dialogs: { open } } as unknown as ExtensionCommandContext; + + await getBundledAgentSkillCommand().handler(context); + + expect(open).toHaveBeenCalledWith({ + title: "Agent skill", + width: 80, + height: 9, + component: AgentSkillDialog, + }); + }); +}); diff --git a/src/extensions/default/ui/agentSkill/index.tsx b/src/extensions/default/ui/agentSkill/index.tsx new file mode 100644 index 000000000..2859e8775 --- /dev/null +++ b/src/extensions/default/ui/agentSkill/index.tsx @@ -0,0 +1,136 @@ +import type { MouseEvent as TuiMouseEvent } from "@opentui/core"; +import { useKeyboard } from "@opentui/react"; +import { matchesKey, type ExtensionDialogProps, type ExtensionFactory } from "hunkdiff/extension"; + +export const AGENT_SKILL_COMMAND = "hunk skill path"; +export const AGENT_SKILL_PROMPT_ROWS = [ + "Load the Hunk skill and use it for this review.", + "Run `hunk skill path` to get the skill path.", +] as const; +export const AGENT_SKILL_PROMPT = AGENT_SKILL_PROMPT_ROWS.join(" "); +export const BUNDLED_AGENT_SKILL_COMMAND_ID = "app.openAgentSkill"; +export const BUNDLED_AGENT_SKILL_COMMAND_FULL_ID = `hunk.${BUNDLED_AGENT_SKILL_COMMAND_ID}`; + +const AGENT_SKILL_BODY = "Teach your agent how to review this Hunk session."; +const AGENT_SKILL_DIALOG_WIDTH = 80; +const AGENT_SKILL_DIALOG_HEIGHT = 9; + +/** Wrap Hunk-owned ASCII prose to one component rectangle. */ +function wrapWords(text: string, width: number) { + const safeWidth = Math.max(1, width); + const lines: string[] = []; + let current = ""; + for (const word of text.split(" ")) { + if (word.length > safeWidth) { + if (current) lines.push(current); + for (let offset = 0; offset < word.length; offset += safeWidth) { + lines.push(word.slice(offset, offset + safeWidth)); + } + current = ""; + continue; + } + const next = current ? `${current} ${word}` : word; + if (next.length <= safeWidth) { + current = next; + } else { + lines.push(current); + current = word; + } + } + if (current) lines.push(current); + return lines; +} + +/** Render Agent Skill onboarding through the public custom-dialog contract. */ +export function AgentSkillDialog({ + actions, + copySupported, + height, + theme, + width, +}: ExtensionDialogProps) { + const bodyLines = wrapWords(AGENT_SKILL_BODY, width); + const promptWidth = Math.max(1, width - 4); + const promptLines = AGENT_SKILL_PROMPT_ROWS.flatMap((line) => wrapWords(line, promptWidth)); + const requiredHeight = bodyLines.length + promptLines.length + 6; + const copyExposed = width >= 5 && height >= requiredHeight; + + const copyPrompt = () => { + const copied = actions.copy(AGENT_SKILL_PROMPT); + actions.notify(copied ? "Copied agent skill prompt to clipboard" : "Clipboard copy failed"); + }; + + useKeyboard((key) => { + if (!copySupported || !copyExposed || !matchesKey("c", key)) return; + key.preventDefault(); + key.stopPropagation(); + copyPrompt(); + }); + + return ( + + {bodyLines.map((line, index) => ( + + {line} + + ))} + + + Prompt + + + + {promptLines.map((line, index) => ( + + {line} + + ))} + + + + + { + event.stopPropagation(); + if (copySupported && copyExposed) copyPrompt(); + }} + > + + {copySupported ? " ⧉ Copy prompt " : " Copy unavailable "} + + + + + ); +} + +/** Register Hunk's agent onboarding guidance through the public dialog contract. */ +const registerBundledAgentSkill: ExtensionFactory = (hunk) => { + hunk.registerCommand( + { + id: BUNDLED_AGENT_SKILL_COMMAND_ID, + title: "Show setup guidance for reviewing with an agent", + }, + async (ctx) => { + await ctx.dialogs.open({ + title: "Agent skill", + width: AGENT_SKILL_DIALOG_WIDTH, + height: AGENT_SKILL_DIALOG_HEIGHT, + component: AgentSkillDialog, + }); + }, + ); +}; + +export default registerBundledAgentSkill; diff --git a/src/extensions/default/ui/index.test.ts b/src/extensions/default/ui/index.test.ts index f9cced6b4..f69f76117 100644 --- a/src/extensions/default/ui/index.test.ts +++ b/src/extensions/default/ui/index.test.ts @@ -1,16 +1,17 @@ import { describe, expect, test } from "bun:test"; import { getBundledUIRegistry } from "."; import { paneKey } from "../../apply"; +import { BUNDLED_AGENT_SKILL_COMMAND_FULL_ID } from "./agentSkill"; import { BUNDLED_EDITOR_COMMAND_FULL_ID } from "./editor"; describe("bundled UI registry", () => { - test("registers the built-in files pane and editor command", () => { + test("registers the built-in files pane and workflow commands", () => { const registry = getBundledUIRegistry(); const panes = registry.panes; expect(panes.map(paneKey)).toEqual(["hunk:files"]); expect( registry.commands.map(({ extensionId, command }) => `${extensionId}.${command.id}`), - ).toEqual([BUNDLED_EDITOR_COMMAND_FULL_ID]); + ).toEqual([BUNDLED_EDITOR_COMMAND_FULL_ID, BUNDLED_AGENT_SKILL_COMMAND_FULL_ID]); expect(registry.extensions).toHaveLength(1); expect(registry.extensions[0]?.origin).toBe("bundled"); }); diff --git a/src/extensions/default/ui/index.ts b/src/extensions/default/ui/index.ts index 8e86df792..f6616b6a0 100644 --- a/src/extensions/default/ui/index.ts +++ b/src/extensions/default/ui/index.ts @@ -7,6 +7,7 @@ import { type ExtensionRegistry, } from "../../types"; import registerBundledEditor, { BUNDLED_EDITOR_COMMAND_FULL_ID } from "./editor"; +import registerBundledAgentSkill, { BUNDLED_AGENT_SKILL_COMMAND_FULL_ID } from "./agentSkill"; import registerBundledSidebar from "./sidebar"; let cachedRegistry: ExtensionRegistry | undefined; @@ -15,6 +16,7 @@ let cachedRegistry: ExtensionRegistry | undefined; const registerBundledUI: ExtensionFactory = (hunk) => { registerBundledSidebar(hunk); registerBundledEditor(hunk); + registerBundledAgentSkill(hunk); }; /** Load bundled UI registrations through the public factory path, once per process. */ @@ -38,7 +40,16 @@ export function getBundledUIRegistry(): ExtensionRegistry { const editorCommandRegistered = registry.commands.some( ({ extensionId, command }) => `${extensionId}.${command.id}` === BUNDLED_EDITOR_COMMAND_FULL_ID, ); - if (issues.length > 0 || !filesPaneRegistered || !editorCommandRegistered) { + const agentSkillCommandRegistered = registry.commands.some( + ({ extensionId, command }) => + `${extensionId}.${command.id}` === BUNDLED_AGENT_SKILL_COMMAND_FULL_ID, + ); + if ( + issues.length > 0 || + !filesPaneRegistered || + !editorCommandRegistered || + !agentSkillCommandRegistered + ) { throw new Error( `Bundled UI failed to register: ${issues[0]?.message ?? "missing required contribution"}`, ); diff --git a/src/extensions/events.test.ts b/src/extensions/events.test.ts index f0ae2bad4..ff0823b15 100644 --- a/src/extensions/events.test.ts +++ b/src/extensions/events.test.ts @@ -195,6 +195,7 @@ describe("extension event dispatch", () => { confirm: async () => false, select: async () => null, input: async () => null, + open: async () => {}, }, events: { emit: () => {} }, }; diff --git a/src/extensions/events.ts b/src/extensions/events.ts index 4d3bfa636..97be5f1eb 100644 --- a/src/extensions/events.ts +++ b/src/extensions/events.ts @@ -343,6 +343,9 @@ function unavailableDialogs(result: ExtensionLoadResult, extensionId: string): E unavailable(); return null; }, + open: async () => { + unavailable(); + }, }; } diff --git a/src/extensions/types.ts b/src/extensions/types.ts index 9f9523288..cdb4bb4f3 100644 --- a/src/extensions/types.ts +++ b/src/extensions/types.ts @@ -44,6 +44,10 @@ export type { ExtensionContext, ExtensionCustomEventHandler, ExtensionDiffFile, + ExtensionDialogActions, + ExtensionDialogComponent, + ExtensionDialogOptions, + ExtensionDialogProps, ExtensionEventBus, ExtensionEventContext, ExtensionEventHandler, diff --git a/src/ui/App.tsx b/src/ui/App.tsx index b1076cfdb..dea80ef85 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -1,6 +1,7 @@ import type { BoxRenderable, MouseEvent as TuiMouseEvent, + Renderable, ScrollBoxRenderable, } from "@opentui/core"; import { useRenderer, useTerminalDimensions } from "@opentui/react"; @@ -35,6 +36,7 @@ import { import { projectExtensionReviewNotes } from "../extensions/reviewSnapshot"; import type { ExtensionNotifyType, ExtensionLoadResult } from "../extensions/types"; import { getBundledUIRegistry } from "../extensions/default/ui"; +import { BUNDLED_AGENT_SKILL_COMMAND_FULL_ID } from "../extensions/default/ui/agentSkill"; import { BUNDLED_EDITOR_COMMAND_FULL_ID } from "../extensions/default/ui/editor"; import type { ReviewProducer } from "../app/review/producer"; import type { HunkSessionBrokerClient } from "../session/broker/brokerClient"; @@ -88,6 +90,7 @@ import { } from "./lib/appCommands"; import { buildAppMenus } from "./lib/appMenus"; import { buildExtensionAppCommands, extensionCommandKeyDefaults } from "./lib/extensionCommands"; +import { normalizeExtensionDialogClipboardText } from "./lib/extensionDialogs"; import type { CurrentLineAlignment } from "./lib/hunkScroll"; import type { LineCursor } from "./lib/lineCursors"; import { useFilePresentationController } from "./fileViews/useFilePresentationController"; @@ -112,9 +115,6 @@ type FocusArea = "files" | "filter" | "note"; const FAST_CODE_HORIZONTAL_SCROLL_COLUMNS = 8; -const LazyAgentSkillDialog = lazy(async () => ({ - default: (await import("./components/chrome/AgentSkillDialog")).AgentSkillDialog, -})); const LazyHelpDialog = lazy(async () => ({ default: (await import("./components/chrome/HelpDialog")).HelpDialog, })); @@ -205,6 +205,17 @@ export function App({ const wrapToggleScrollTopRef = useRef(null); const layoutToggleScrollTopRef = useRef(null); const cancelCopySelectionRef = useRef<(() => void) | null>(null); + const activeReviewGenerationRef = useRef(bootstrap); + const renderedReviewGenerationRef = useRef(bootstrap); + renderedReviewGenerationRef.current = bootstrap; + const bundledDialogsLiveRef = useRef(false); + useLayoutEffect(() => { + activeReviewGenerationRef.current = bootstrap; + bundledDialogsLiveRef.current = true; + return () => { + bundledDialogsLiveRef.current = false; + }; + }, [bootstrap]); const [layoutToggleRequestId, setLayoutToggleRequestId] = useState(0); const [scrollEdgeRequest, setScrollEdgeRequest] = useState<{ id: number; @@ -224,7 +235,6 @@ export function App({ const [showHunkHeaders, setShowHunkHeaders] = useState(bootstrap.initialShowHunkHeaders ?? true); const [showMenuBar, setShowMenuBar] = useState(bootstrap.initialShowMenuBar ?? true); const [showHelp, setShowHelp] = useState(false); - const [showAgentSkill, setShowAgentSkill] = useState(false); const [focusArea, setFocusArea] = useState("files"); const { text: sessionNoticeText, show: showSessionNotice } = useTimedNotice(4_000); const extensions = bootstrap.extensions as ExtensionLoadResult | undefined; @@ -482,9 +492,13 @@ export function App({ const { accept: acceptExtensionDialog, + acceptRequest: acceptExtensionDialogRequest, cancel: cancelExtensionDialog, + cancelRequest: cancelExtensionDialogRequest, cancelAll: cancelAllExtensionDialogs, createDialogs: createQueuedExtensionDialogs, + getCurrentRequest: getCurrentExtensionDialogRequest, + isCurrentRequestLive: isCurrentExtensionDialogRequestLive, inputValue: extensionDialogInputValue, moveSelection: moveExtensionDialogSelection, pickOption: setExtensionDialogSelectedIndex, @@ -503,15 +517,30 @@ export function App({ const createExtensionDialogs = useCallback( (extensionId: string) => { const lease = createReviewCapabilityLease(); - const bundled = extensions?.registry.extensions.some( - (metadata) => metadata.id === extensionId && metadata.origin === "bundled", - ); + const bundled = [ + ...getBundledUIRegistry().extensions, + ...(extensions?.registry.extensions ?? []), + ].some((metadata) => metadata.id === extensionId && metadata.origin === "bundled"); return createQueuedExtensionDialogs(extensionId, { - isLive: () => lease.isLive() && !extensionAppController.isAppActive(), + // Bundled registrations are process-static rather than owned by the + // reloadable user-extension registry, but their review-scoped controls + // still retire when the mounted review changes. + isLive: () => + renderedReviewGenerationRef.current === bootstrap && + !extensionAppController.isAppActive() && + (bundled + ? bundledDialogsLiveRef.current && activeReviewGenerationRef.current === bootstrap + : lease.isLive()), showAttribution: !bundled, }); }, - [createQueuedExtensionDialogs, createReviewCapabilityLease, extensionAppController, extensions], + [ + bootstrap, + createQueuedExtensionDialogs, + createReviewCapabilityLease, + extensionAppController, + extensions, + ], ); const extensionWorkspaceController = useExtensionWorkspaceControls({ @@ -556,6 +585,15 @@ export function App({ return command; }, []); + const bundledAgentSkillCommand = useMemo(() => { + const command = resolveExtensionCommands(getBundledUIRegistry()).commands.find( + ({ extensionId, command: registration }) => + `${extensionId}.${registration.id}` === BUNDLED_AGENT_SKILL_COMMAND_FULL_ID, + ); + if (!command) throw new Error("Bundled agent skill command is not registered."); + return command; + }, []); + /** Delegate the shared host command shell to the bundled editor extension. */ const triggerEditSelectedFile = useCallback(() => { runExtensionCommand(bundledEditorCommand); @@ -907,27 +945,105 @@ export function App({ showNotice: showSessionNotice, }); - /** Close the agent skill setup overlay. */ - const closeAgentSkill = useCallback(() => { - setShowAgentSkill(false); - }, []); - - /** Open the agent skill setup overlay. */ + /** Delegate the shared host command shell to the bundled agent skill extension. */ const openAgentSkill = useCallback(() => { - setShowAgentSkill(true); - }, []); - - /** Copy the agent skill prompt through the terminal clipboard integration. */ - const copyAgentSkillPrompt = useCallback(async () => { - const { AGENT_SKILL_PROMPT } = await import("./components/chrome/AgentSkillDialog"); - if (renderer.isOsc52Supported?.() && typeof renderer.copyToClipboardOSC52 === "function") { - renderer.copyToClipboardOSC52(AGENT_SKILL_PROMPT); - showTransientNotice("Copied agent skill prompt to clipboard"); - return; + runExtensionCommand(bundledAgentSkillCommand); + }, [bundledAgentSkillCommand, runExtensionCommand]); + + const extensionDialogCopySupported = + (renderer.isOsc52Supported?.() ?? false) && typeof renderer.copyToClipboardOSC52 === "function"; + const extensionDialogActive = extensionDialog !== null; + const extensionOpenDialogActive = extensionDialog?.kind === "open"; + const extensionOpenDialogFocusLeaseRef = useRef<{ + active: boolean; + previous: Renderable | null; + }>({ active: false, previous: null }); + if (extensionOpenDialogActive && !extensionOpenDialogFocusLeaseRef.current.active) { + // Capture before the custom tree commits and takes focus. OpenTUI exposes + // one process-wide focus owner, so restoring this exact renderable also + // preserves imperative review-scroll focus outside pager mode. + extensionOpenDialogFocusLeaseRef.current = { + active: true, + previous: renderer.currentFocusedRenderable, + }; + } + useLayoutEffect(() => { + const lease = extensionOpenDialogFocusLeaseRef.current; + if (!extensionDialog && lease.active) { + extensionOpenDialogFocusLeaseRef.current = { active: false, previous: null }; + // `focus()` is inert when teardown already destroyed the old owner. + lease.previous?.focus(); } + }, [extensionDialog]); + + /** Copy text only for the custom dialog that still owns the mounted component. */ + const copyExtensionDialogText = useCallback( + (requestId: number, text: string) => { + const current = getCurrentExtensionDialogRequest(); + if ( + current?.kind !== "open" || + current.id !== requestId || + !isCurrentExtensionDialogRequestLive(requestId) || + !(renderer.isOsc52Supported?.() ?? false) || + typeof renderer.copyToClipboardOSC52 !== "function" + ) { + return false; + } - showTransientNotice("Clipboard copy unsupported in this terminal (enable OSC 52)"); - }, [renderer, showTransientNotice]); + const normalized = normalizeExtensionDialogClipboardText(text); + return normalized !== null && renderer.copyToClipboardOSC52(normalized); + }, + [getCurrentExtensionDialogRequest, isCurrentExtensionDialogRequestLive, renderer], + ); + + /** Show status only for the custom dialog that still owns the mounted component. */ + const notifyExtensionDialog = useCallback( + (requestId: number, message: string) => { + const current = getCurrentExtensionDialogRequest(); + if ( + current?.kind !== "open" || + current.id !== requestId || + !isCurrentExtensionDialogRequestLive(requestId) + ) { + return; + } + const safeMessage = sanitizeTerminalLine(message).trim(); + if (!safeMessage) return; + showTransientNotice( + current.showAttribution ? `Extension ${current.extensionId}: ${safeMessage}` : safeMessage, + ); + }, + [getCurrentExtensionDialogRequest, isCurrentExtensionDialogRequestLive, showTransientNotice], + ); + + /** Close only the custom dialog whose mounted component owns this action. */ + const closeExtensionDialogComponent = useCallback( + (requestId: number) => { + if (isCurrentExtensionDialogRequestLive(requestId)) { + cancelExtensionDialogRequest(requestId); + } + }, + [cancelExtensionDialogRequest, isCurrentExtensionDialogRequestLive], + ); + + /** Contain one custom component failure and leave its host frame dismissible. */ + const reportExtensionDialogRenderFailure = useCallback( + (requestId: number, error: unknown) => { + const current = getCurrentExtensionDialogRequest(); + if ( + current?.kind !== "open" || + current.id !== requestId || + !isCurrentExtensionDialogRequestLive(requestId) + ) { + return; + } + const detail = error instanceof Error ? error.message || error.name : String(error); + showSessionNotice( + `Extension ${current.extensionId} dialog failed rendering • ${sanitizeTerminalLine(detail)}`, + ); + }, + [getCurrentExtensionDialogRequest, isCurrentExtensionDialogRequestLive, showSessionNotice], + ); /** Toggle the modal keyboard help overlay. */ const toggleHelp = useCallback(() => { @@ -1112,7 +1228,6 @@ export function App({ useAppKeyboardShortcuts({ activeMenuId, activateCurrentMenuItem, - closeAgentSkill, closeHelp, closeMenu, acceptThemeSelector, @@ -1121,7 +1236,7 @@ export function App({ closeExtensionTrustPrompt, commands: appCommands, denyRepoExtensions, - extensionDialog, + getExtensionDialog: getCurrentExtensionDialogRequest, acceptExtensionDialog, cancelExtensionDialog, moveExtensionDialogSelection, @@ -1143,7 +1258,6 @@ export function App({ neverAskToSaveViewPreferencesAndQuit, closeSaveConfigPrompt, saveDraftNote, - showAgentSkill, showHelp, switchMenu, toggleFocusArea, @@ -1329,7 +1443,8 @@ export function App({ selectedHunkIndex={selectedHunkIndex} scrollToNote={review.scrollToNote} draftNote={review.draftNote} - draftNoteFocused={focusArea === "note"} + draftNoteFocused={focusArea === "note" && !extensionDialogActive} + keyboardFocusBlocked={extensionDialogActive} separatorWidth={diffSeparatorWidth} showAgentNotes={showAgentNotes} showLineNumbers={showLineNumbers} @@ -1392,7 +1507,7 @@ export function App({ {statusBarVisible ? ( ) : null} - {showAgentSkill ? ( - - - - ) : null} - {showHelp ? ( ) : null} diff --git a/src/ui/AppHost.extension-dialogs.test.tsx b/src/ui/AppHost.extension-dialogs.test.tsx index 84da7f025..a7330184d 100644 --- a/src/ui/AppHost.extension-dialogs.test.tsx +++ b/src/ui/AppHost.extension-dialogs.test.tsx @@ -3,6 +3,7 @@ import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, test } from "bun:test"; +import { KeyEvent, type ParsedKey } from "@opentui/core"; import { testRender } from "@opentui/react/test-utils"; import { act } from "react"; import { removeTestDirectory } from "../../test/helpers/filesystem"; @@ -77,6 +78,23 @@ async function flush(setup: Awaited>) { }); } +/** Publish one key synchronously, used for same-input-flush coverage. */ +function testKeyEvent(fields: Partial) { + return new KeyEvent({ + name: "", + sequence: "", + raw: "", + ctrl: false, + meta: false, + option: false, + shift: false, + number: false, + eventType: "press", + source: "raw", + ...fields, + }); +} + /** Render frames until a condition holds, and fail loudly when it never does. */ async function flushUntil( setup: Awaited>, @@ -204,6 +222,31 @@ function writeDialogFixture(extPath: string, logPath: string, askSource: string) writeFileSync( extPath, `import { appendFileSync } from "node:fs";\n` + + `import { createElement, useState } from "react";\n` + + `import { useKeyboard } from "@opentui/react";\n` + + `import { matchesKey } from "hunkdiff/extension";\n` + + `function createCopyDialog(body, label, text) {\n` + + ` return function CopyDialog({ actions, copySupported, height, theme, width }) {\n` + + ` const copyExposed = width >= 5 && height >= 4;\n` + + ` const copy = () => {\n` + + ` const copied = actions.copy(text);\n` + + ` actions.notify(copied ? "Copied custom content to clipboard" : "Clipboard copy failed");\n` + + ` };\n` + + ` useKeyboard((key) => {\n` + + ` if (!copySupported || !copyExposed || !matchesKey("c", key)) return;\n` + + ` key.preventDefault();\n` + + ` key.stopPropagation();\n` + + ` copy();\n` + + ` });\n` + + ` return createElement("box", { style: { width, height, flexDirection: "column", overflow: "hidden" } },\n` + + ` createElement("box", { style: { width: "100%", height: 1 } }, createElement("text", { fg: theme.text }, body)),\n` + + ` createElement("box", { style: { width: "100%", height: 1 } }, createElement("text", { fg: theme.badgeNeutral }, label)),\n` + + ` createElement("box", { style: { width: "100%", height: 1 } }, createElement("text", { fg: theme.text }, text)),\n` + + ` createElement("box", { style: { width: "100%", height: 1, backgroundColor: copySupported ? theme.accentMuted : theme.panelAlt }, onMouseUp: (event) => { event.stopPropagation(); if (copySupported && copyExposed) copy(); } },\n` + + ` createElement("text", { fg: copySupported ? theme.text : theme.muted }, copySupported ? " ⧉ Copy " + label.toLowerCase() + " " : " Copy unavailable "))\n` + + ` );\n` + + ` };\n` + + `}\n` + `export default function (hunk) {\n` + ` hunk.registerCommand({ id: "ask", title: "Ask", key: "y" }, async (ctx) => {\n` + ` const answer = await ${askSource};\n` + @@ -317,6 +360,628 @@ describe("extension dialogs", () => { }); }); + test("owns later keys when a command opens and cancels a dialog in one input flush", async () => { + const repo = createTestRepo("hunk-ext-dialog-same-flush-"); + const extDir = createTempDir("hunk-ext-dialog-same-flush-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture(extPath, logPath, `ctx.dialogs.confirm({ title: "Same flush?" })`); + + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost(bootstrap, async (setup, quits) => { + await act(async () => { + setup.renderer.keyInput.emit( + "keypress", + testKeyEvent({ name: "y", sequence: "y", raw: "y" }), + ); + setup.renderer.keyInput.emit( + "keypress", + testKeyEvent({ name: "q", sequence: "q", raw: "q" }), + ); + setup.renderer.keyInput.emit( + "keypress", + testKeyEvent({ name: "escape", sequence: "\u001b", raw: "\u001b" }), + ); + }); + + await flushUntil( + setup, + () => readProbeLog(logPath).includes("answer false"), + "the same-flush Escape to cancel the queued dialog", + ); + expect(quits()).toBe(0); + expect(setup.captureCharFrame()).not.toContain("Same flush?"); + }); + }); + + test("moves and accepts a newly opened select dialog in one input flush", async () => { + const repo = createTestRepo("hunk-ext-dialog-select-same-flush-"); + const extDir = createTempDir("hunk-ext-dialog-select-same-flush-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.select({ title: "Same flush choice", options: ["one", "two"] })`, + ); + + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost(bootstrap, async (setup) => { + await act(async () => { + setup.renderer.keyInput.emit( + "keypress", + testKeyEvent({ name: "y", sequence: "y", raw: "y" }), + ); + setup.renderer.keyInput.emit("keypress", testKeyEvent({ name: "down" })); + setup.renderer.keyInput.emit("keypress", testKeyEvent({ name: "return" })); + }); + + await flushUntil( + setup, + () => readProbeLog(logPath).includes("answer two"), + "the same-flush selection to resolve", + ); + expect(setup.captureCharFrame()).not.toContain("Same flush choice"); + }); + }); + + test("a component dialog renders an OpenTUI surface and closes only on escape", async () => { + const repo = createTestRepo("hunk-ext-dialog-open-"); + const extDir = createTempDir("hunk-ext-dialog-open-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.open({ title: "Agent setup", width: 46, height: 6, component: createCopyDialog("Teach your agent how to review this Hunk session.", "Prompt", "Load the Hunk skill and use it for this review. Run hunk skill path to get the skill path.") })`, + ); + + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost( + bootstrap, + async (setup) => { + const copied: string[] = []; + setup.renderer.isOsc52Supported = () => true; + setup.renderer.copyToClipboardOSC52 = (text: string) => { + copied.push(text); + return true; + }; + + await act(async () => { + await setup.mockInput.typeText("y"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("Agent setup"), + "the component dialog to open", + ); + + const frame = setup.captureCharFrame(); + expect(frame).toContain("Teach your agent"); + expect(frame).toContain("Prompt"); + expect(frame).toContain("Load the Hunk skill"); + expect(frame).toContain("ext ext"); + + await act(async () => { + await setup.mockInput.pressEnter(); + await setup.mockInput.typeText("c"); + }); + await flush(setup); + expect(setup.captureCharFrame()).toContain("Agent setup"); + expect(copied).toEqual([ + "Load the Hunk skill and use it for this review. Run hunk skill path to get the skill path.", + ]); + + const copyAction = findTextPosition(setup.captureCharFrame(), "Copy prompt"); + expect(copyAction).not.toBeNull(); + await act(async () => { + await setup.mockMouse.click(copyAction!.x, copyAction!.y); + }); + expect(copied).toHaveLength(2); + + await act(async () => { + await setup.mockInput.pressEscape(); + }); + await flushUntil( + setup, + () => readProbeLog(logPath).includes("answer undefined"), + "the component-dialog handler to finish", + ); + }, + undefined, + { width: 50, height: 20 }, + ); + }); + + test("preserves focus owned by an input inside a component dialog", async () => { + const repo = createTestRepo("hunk-ext-dialog-open-input-"); + const extDir = createTempDir("hunk-ext-dialog-open-input-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.open({ title: "Custom input", width: 36, height: 4, component: function InputDialog({ theme, width }) { const [value, setValue] = useState(""); return createElement("input", { focused: true, width, value, onInput: setValue, textColor: theme.text }); } })`, + ); + + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost(bootstrap, async (setup, quits) => { + await act(async () => { + await setup.mockInput.typeText("y"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("Custom input"), + "the custom input dialog to open", + ); + + await act(async () => { + await setup.mockInput.typeText("quick-fix"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("quick-fix"), + "typing to reach the extension-owned input", + ); + expect(quits()).toBe(0); + + await act(async () => { + await setup.mockInput.pressEscape(); + }); + await flushUntil( + setup, + () => + readProbeLog(logPath).includes("answer undefined") && + !setup.captureCharFrame().includes("Custom input"), + "the custom input dialog to settle and close", + ); + }); + }); + + test("keeps a promoted input focused after a component dialog closes", async () => { + const repo = createTestRepo("hunk-ext-dialog-open-input-promotion-"); + const extDir = createTempDir("hunk-ext-dialog-open-input-promotion-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeFileSync( + extPath, + `import { appendFileSync } from "node:fs";\n` + + `import { createElement } from "react";\n` + + `import { useKeyboard } from "@opentui/react";\n` + + `import { matchesKey } from "hunkdiff/extension";\n` + + `function FirstDialog({ actions, theme }) {\n` + + ` useKeyboard((key) => { if (matchesKey("x", key)) actions.close(); });\n` + + ` return createElement("text", { fg: theme.text }, "Press x for input");\n` + + `}\n` + + `export default function (hunk) {\n` + + ` hunk.registerCommand({ id: "ask", title: "Ask", key: "y" }, async (ctx) => {\n` + + ` const opened = ctx.dialogs.open({ title: "First component", component: FirstDialog });\n` + + ` const typed = ctx.dialogs.input({ title: "Promoted input" });\n` + + ` const results = await Promise.all([opened, typed]);\n` + + ` appendFileSync(${JSON.stringify(logPath)}, "answer " + String(results[1]) + "\\n");\n` + + ` });\n` + + `}\n`, + ); + + const bootstrap = await launchWithExtension(repo, extPath); + bootstrap.input.options.pager = true; + await withAppHost(bootstrap, async (setup) => { + await act(async () => { + await setup.mockInput.typeText("y"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("Press x for input"), + "the first component dialog to open", + ); + + await act(async () => { + await setup.mockInput.typeText("x"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("Promoted input"), + "the queued input dialog to be promoted", + ); + expect(setup.renderer.currentFocusedRenderable?.constructor.name).toBe("InputRenderable"); + + await act(async () => { + await setup.mockInput.typeText("quick-fix"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("quick-fix"), + "typing to remain with the promoted input", + ); + await act(async () => { + await setup.mockInput.pressEnter(); + }); + await flushUntil( + setup, + () => readProbeLog(logPath).includes("answer quick-fix"), + "the promoted input to resolve its typed value", + ); + }); + }); + + test("clips a custom component to the bounded rectangle beneath attribution", async () => { + const repo = createTestRepo("hunk-ext-dialog-open-short-"); + const extDir = createTempDir("hunk-ext-dialog-open-short-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.open({ title: "Agent setup", width: 46, height: 6, component: createCopyDialog("Teach your agent how to review this Hunk session.", "Prompt", "Load the Hunk skill and use it for this review. Run hunk skill path to get the skill path.") })`, + ); + + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost( + bootstrap, + async (setup) => { + const copied: string[] = []; + setup.renderer.isOsc52Supported = () => true; + setup.renderer.copyToClipboardOSC52 = (text: string) => { + copied.push(text); + return true; + }; + + await act(async () => { + await setup.mockInput.typeText("y"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("Agent setup"), + "the constrained component dialog to open", + ); + + const frame = setup.captureCharFrame(); + expect(frame).toContain("ext ext"); + expect(frame).not.toContain("Copy prompt"); + + await act(async () => { + await setup.mockInput.typeText("c"); + }); + await flush(setup); + expect(copied).toEqual([]); + expect(setup.captureCharFrame()).toContain("Agent setup"); + }, + undefined, + { width: 50, height: 12 }, + ); + }); + + test("an unavailable component copy action is visible but inert", async () => { + const repo = createTestRepo("hunk-ext-dialog-open-unavailable-"); + const extDir = createTempDir("hunk-ext-dialog-open-unavailable-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.open({ title: "Copy setup", width: 46, height: 6, component: createCopyDialog("Copy this text.", "Prompt", "copy me") })`, + ); + + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost(bootstrap, async (setup) => { + const copied: string[] = []; + setup.renderer.isOsc52Supported = () => false; + setup.renderer.copyToClipboardOSC52 = (text: string) => { + copied.push(text); + return true; + }; + + await act(async () => { + await setup.mockInput.typeText("y"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("Copy unavailable"), + "the unavailable copy state to render", + ); + + const unavailable = findTextPosition(setup.captureCharFrame(), "Copy unavailable"); + expect(unavailable).not.toBeNull(); + await act(async () => { + await setup.mockInput.typeText("c"); + await setup.mockMouse.click(unavailable!.x, unavailable!.y); + }); + await flush(setup); + + expect(copied).toEqual([]); + expect(setup.captureCharFrame()).toContain("Copy setup"); + + await act(async () => { + await setup.mockInput.pressEscape(); + }); + await flushUntil( + setup, + () => readProbeLog(logPath).includes("answer undefined"), + "the unavailable-copy component dialog to close", + ); + }); + }); + + test("reports a clipboard write rejected by the renderer as a failure", async () => { + const repo = createTestRepo("hunk-ext-dialog-open-copy-failure-"); + const extDir = createTempDir("hunk-ext-dialog-open-copy-failure-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.open({ title: "Copy setup", width: 46, height: 6, component: createCopyDialog("Copy this text.", "Prompt", "copy me") })`, + ); + + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost(bootstrap, async (setup) => { + let attempts = 0; + setup.renderer.isOsc52Supported = () => true; + setup.renderer.copyToClipboardOSC52 = () => { + attempts += 1; + return false; + }; + + await act(async () => { + await setup.mockInput.typeText("y"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("Copy prompt"), + "the copy action to render", + ); + + await act(async () => { + await setup.mockInput.typeText("c"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("Clipboard copy failed"), + "the copy failure notice", + ); + + expect(attempts).toBe(1); + expect(setup.captureCharFrame()).not.toContain("Copied custom content to clipboard"); + }); + }); + + test("contains a custom component render failure inside its dismissible frame", async () => { + const repo = createTestRepo("hunk-ext-dialog-open-render-failure-"); + const extDir = createTempDir("hunk-ext-dialog-open-render-failure-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.open({ title: "Broken surface", width: 30, height: 4, component: ({ actions }) => { const retained = actions; setTimeout(() => { appendFileSync(${JSON.stringify(logPath)}, "failed-copy " + String(retained.copy("stale")) + "\\n"); retained.notify("stale failure notice"); retained.close(); appendFileSync(${JSON.stringify(logPath)}, "failed-actions-called\\n"); }, 10); throw new Error("surface exploded"); } })`, + ); + + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost(bootstrap, async (setup) => { + const copied: string[] = []; + setup.renderer.isOsc52Supported = () => true; + setup.renderer.copyToClipboardOSC52 = (text: string) => { + copied.push(text); + return true; + }; + await act(async () => { + await setup.mockInput.typeText("y"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("Dialog unavailable"), + "the custom-dialog fallback to render", + ); + + const failed = setup.captureCharFrame(); + expect(failed).toContain("Broken surface"); + expect(failed).toContain("surface exploded"); + await flushUntil( + setup, + () => readProbeLog(logPath).includes("failed-actions-called"), + "the retained failed-component actions to run", + ); + expect(readProbeLog(logPath)).toContain("failed-copy false"); + expect(copied).toEqual([]); + expect(setup.captureCharFrame()).toContain("Dialog unavailable"); + expect(setup.captureCharFrame()).not.toContain("stale failure notice"); + + await act(async () => { + await setup.mockInput.pressEscape(); + }); + await flushUntil( + setup, + () => readProbeLog(logPath).includes("answer undefined"), + "the failed component dialog to close", + ); + }); + }); + + test("keeps a custom component mounted through a zero-row terminal allocation", async () => { + const repo = createTestRepo("hunk-ext-dialog-open-zero-height-"); + const extDir = createTempDir("hunk-ext-dialog-open-zero-height-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.open({ title: "Resize surface", width: 30, height: 4, component: function StatefulDialog({ height, theme }) { const [value, setValue] = useState("initial"); useKeyboard((key) => { if (matchesKey("x", key)) setValue("preserved"); }); return createElement("text", { fg: theme.text }, value + " at " + height); } })`, + ); + + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost(bootstrap, async (setup) => { + await act(async () => { + await setup.mockInput.typeText("y"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("initial at 4"), + "the stateful component to open", + ); + + await act(async () => { + await setup.mockInput.typeText("x"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("preserved at 4"), + "the stateful component to update before shrinking", + ); + + await act(async () => { + await setup.resize(80, 8); + }); + await flush(setup); + expect(setup.captureCharFrame()).not.toContain("preserved at"); + + await act(async () => { + await setup.resize(80, 20); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("preserved at 4"), + "the zero-row component state to survive the resize", + ); + + await act(async () => { + await setup.mockInput.pressEscape(); + }); + await flushUntil( + setup, + () => readProbeLog(logPath).includes("answer undefined"), + "the resized component dialog to close", + ); + }); + }); + + test("routes component keys without exposing the review and honors guarded close", async () => { + const repo = createTestRepo("hunk-ext-dialog-open-close-"); + const extDir = createTempDir("hunk-ext-dialog-open-close-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.open({ title: "Component keys", width: 30, height: 4, component: function KeyDialog({ actions, theme }) { useKeyboard((key) => { if (matchesKey("x", key)) actions.close(); }); return createElement("text", { fg: theme.text }, "Press x to close"); } })`, + ); + + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost(bootstrap, async (setup) => { + await act(async () => { + await setup.mockInput.typeText("y"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("Press x to close"), + "the keyed component dialog to open", + ); + + await act(async () => { + await setup.mockInput.typeText("x"); + }); + await flushUntil( + setup, + () => + readProbeLog(logPath).includes("answer undefined") && + !setup.captureCharFrame().includes("Component keys"), + "the component action to settle and close its dialog", + ); + }); + }); + + test("remounts reused components and retires actions when the queue advances", async () => { + const repo = createTestRepo("hunk-ext-dialog-open-lease-"); + const extDir = createTempDir("hunk-ext-dialog-open-lease-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeFileSync( + extPath, + `import { appendFileSync } from "node:fs";\n` + + `import { createElement, useState } from "react";\n` + + `import { useKeyboard } from "@opentui/react";\n` + + `import { matchesKey } from "hunkdiff/extension";\n` + + `let mounts = 0;\n` + + `let firstActions;\n` + + `function SharedDialog({ actions, theme }) {\n` + + ` const [mount] = useState(() => ++mounts);\n` + + ` if (mount === 1) firstActions = actions;\n` + + ` useKeyboard((key) => {\n` + + ` if (matchesKey("x", key)) actions.close();\n` + + ` if (matchesKey("z", key) && firstActions) {\n` + + ` appendFileSync(${JSON.stringify(logPath)}, "stale-copy " + String(firstActions.copy("stale")) + "\\n");\n` + + ` firstActions.notify("stale notification");\n` + + ` firstActions.close();\n` + + ` }\n` + + ` });\n` + + ` return createElement("text", { fg: theme.text }, "mount " + mount);\n` + + `}\n` + + `export default function (hunk) {\n` + + ` hunk.registerCommand({ id: "ask", title: "Ask", key: "y" }, async (ctx) => {\n` + + ` await Promise.all([\n` + + ` ctx.dialogs.open({ title: "First surface", component: SharedDialog }),\n` + + ` ctx.dialogs.open({ title: "Second surface", component: SharedDialog }),\n` + + ` ]);\n` + + ` appendFileSync(${JSON.stringify(logPath)}, "settled\\n");\n` + + ` });\n` + + `}\n`, + ); + + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost(bootstrap, async (setup) => { + const copied: string[] = []; + setup.renderer.isOsc52Supported = () => true; + setup.renderer.copyToClipboardOSC52 = (text: string) => { + copied.push(text); + return true; + }; + + await act(async () => { + await setup.mockInput.typeText("y"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("mount 1"), + "the first reused component to mount", + ); + + await act(async () => { + await setup.mockInput.typeText("x"); + }); + await flushUntil( + setup, + () => { + const frame = setup.captureCharFrame(); + return frame.includes("Second surface") && frame.includes("mount 2"); + }, + "the reused component to remount for the promoted request", + ); + + await act(async () => { + await setup.mockInput.typeText("z"); + }); + await flushUntil( + setup, + () => readProbeLog(logPath).includes("stale-copy false"), + "the retired copy action to report that it is inert", + ); + const promotedFrame = setup.captureCharFrame(); + expect(promotedFrame).toContain("Second surface"); + expect(promotedFrame).not.toContain("stale notification"); + expect(copied).toEqual([]); + + await act(async () => { + await setup.mockInput.pressEscape(); + }); + await flushUntil( + setup, + () => + readProbeLog(logPath).includes("settled") && + !setup.captureCharFrame().includes("Second surface"), + "the promoted component dialog to settle and close", + ); + }); + }); + test("keeps confirm actions visible when wrapped prose exceeds a short terminal", async () => { const repo = createTestRepo("hunk-ext-dialog-short-confirm-"); const extDir = createTempDir("hunk-ext-dialog-short-confirm-ext-"); @@ -570,6 +1235,68 @@ describe("extension dialogs", () => { ); }); + test("retires component actions before a soft-reload layout cleanup", async () => { + const repo = createTestRepo("hunk-ext-dialog-reload-action-lease-"); + const extDir = createTempDir("hunk-ext-dialog-reload-action-lease-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeFileSync( + extPath, + `import { appendFileSync } from "node:fs";\n` + + `import { createElement, useLayoutEffect } from "react";\n` + + `function CleanupDialog({ actions, theme }) {\n` + + ` useLayoutEffect(() => () => {\n` + + ` appendFileSync(${JSON.stringify(logPath)}, "cleanup-copy " + String(actions.copy("stale")) + "\\n");\n` + + ` actions.notify("stale cleanup notice");\n` + + ` actions.close();\n` + + ` }, []);\n` + + ` return createElement("text", { fg: theme.text }, "Reload cleanup probe");\n` + + `}\n` + + `export default function (hunk) {\n` + + ` hunk.registerCommand({ id: "ask", title: "Ask", key: "y" }, async (ctx) => {\n` + + ` await ctx.dialogs.open({ title: "Reload action lease", component: CleanupDialog });\n` + + ` appendFileSync(${JSON.stringify(logPath)}, "settled\\n");\n` + + ` });\n` + + `}\n`, + ); + + const broker = createTestBrokerClient(); + const bootstrap = await launchWithExtension(repo, extPath); + await withAppHost( + bootstrap, + async (setup) => { + const copied: string[] = []; + setup.renderer.isOsc52Supported = () => true; + setup.renderer.copyToClipboardOSC52 = (text: string) => { + copied.push(text); + return true; + }; + await act(async () => { + await setup.mockInput.typeText("y"); + }); + await flushUntil( + setup, + () => setup.captureCharFrame().includes("Reload cleanup probe"), + "the cleanup-probe component dialog to open", + ); + + await broker.reload({ kind: "vcs", staged: false, options: {} }); + await flushUntil( + setup, + () => { + const events = readProbeLog(logPath); + return events.includes("cleanup-copy false") && events.includes("settled"); + }, + "the stale cleanup actions to retire before reload teardown", + ); + + expect(copied).toEqual([]); + expect(setup.captureCharFrame()).not.toContain("stale cleanup notice"); + }, + broker.client, + ); + }); + test("keeps a dialog opened by the replacement generation's reload lifecycle", async () => { const repo = createTestRepo("hunk-ext-dialog-reload-lifecycle-"); const extDir = createTempDir("hunk-ext-dialog-reload-lifecycle-ext-"); diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index e7c707496..4b5f3862d 100644 --- a/src/ui/AppHost.interactions.test.tsx +++ b/src/ui/AppHost.interactions.test.tsx @@ -18,7 +18,7 @@ import { createTestVcsAppBootstrap } from "../../test/helpers/app-bootstrap"; import { capturedTestColorToHex } from "../../test/helpers/test-color-helpers"; import { createTestDiffFile as buildTestDiffFile, lines } from "../../test/helpers/diff-helpers"; import { createEmptyExtensionLoadResult } from "../extensions/types"; -import { AGENT_SKILL_COMMAND, AGENT_SKILL_PROMPT } from "./components/chrome/AgentSkillDialog"; +import { AGENT_SKILL_COMMAND, AGENT_SKILL_PROMPT } from "../extensions/default/ui/agentSkill"; import { App } from "./App"; import { availableThemes, resolveTheme } from "./themes"; @@ -1932,6 +1932,12 @@ describe("App interactions", () => { width: 120, height: 24, }); + const copied: string[] = []; + setup.renderer.isOsc52Supported = () => true; + setup.renderer.copyToClipboardOSC52 = (text: string) => { + copied.push(text); + return true; + }; try { await flush(setup); @@ -1975,6 +1981,17 @@ describe("App interactions", () => { expect(frame).toContain(AGENT_SKILL_COMMAND); expect(frame).toContain("Copy"); + await act(async () => { + await setup.mockInput.typeText("c"); + }); + frame = await waitForFrame( + setup, + (currentFrame) => currentFrame.includes("Copied agent skill prompt to clipboard"), + 12, + ); + expect(copied).toEqual([AGENT_SKILL_PROMPT]); + expect(frame).toContain("Copied agent skill prompt to clipboard"); + await act(async () => { await setup.mockInput.pressEscape(); }); diff --git a/src/ui/AppHost.key-routing.test.tsx b/src/ui/AppHost.key-routing.test.tsx index 3cc571e23..9a9e4a707 100644 --- a/src/ui/AppHost.key-routing.test.tsx +++ b/src/ui/AppHost.key-routing.test.tsx @@ -278,4 +278,72 @@ describe("UI key routing with a focused scroll box", () => { rmSync(root, { recursive: true, force: true }); } }); + + test("an open component dialog isolates unhandled keys from the focused review", async () => { + const root = mkdtempSync(join(tmpdir(), "hunk-key-routing-dialog-")); + const extension = join(root, "custom-dialog"); + mkdirSync(extension, { recursive: true }); + writeFileSync( + join(extension, "package.json"), + JSON.stringify({ + name: "custom-dialog", + private: true, + hunk: { extensions: ["./index.ts"] }, + }), + ); + writeFileSync( + join(extension, "index.ts"), + `import { createElement } from "react"; +export default function (hunk) { + hunk.registerCommand({ id: "open", title: "Open", key: "y" }, (ctx) => + ctx.dialogs.open({ + title: "Focused custom surface", + component: ({ theme }) => createElement("text", { fg: theme.text }, "Unhandled keys stay here"), + }), + ); +} +`, + ); + const extensions = await loadStartupExtensions({ + cliExtensionPaths: [extension], + cwd: root, + env: { XDG_CONFIG_HOME: root } as NodeJS.ProcessEnv, + extensions: { enabled: true, extensionConfigs: {}, paths: [], repoPaths: [] }, + }); + const bootstrap = createScrollableBootstrap(); + bootstrap.extensions = extensions; + const setup = await testRender( {}} />, { + width: 120, + height: 24, + }); + + try { + await waitForFrame(setup, (frame) => frame.includes("big.ts"), 12); + const scrollBox = findReviewScrollBox(setup.renderer.root); + if (!scrollBox) { + throw new Error("No scrollable review scroll box found in the rendered app."); + } + await act(async () => { + scrollBox.focus(); + await setup.mockInput.typeText("y"); + }); + await waitForFrame(setup, (frame) => frame.includes("Focused custom surface"), 12); + const scrollTopBefore = scrollBox.scrollTop; + + await act(async () => setup.mockInput.typeText("j")); + await flush(setup); + + expect(scrollBox.scrollTop).toBe(scrollTopBefore); + expect(setup.captureCharFrame()).toContain("Focused custom surface"); + + await act(async () => setup.mockInput.pressEscape()); + await waitForFrame(setup, (frame) => !frame.includes("Focused custom surface"), 12); + expect(setup.renderer.currentFocusedRenderable).toBe(scrollBox); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + rmSync(root, { recursive: true, force: true }); + } + }); }); diff --git a/src/ui/components/chrome/AgentSkillDialog.tsx b/src/ui/components/chrome/AgentSkillDialog.tsx deleted file mode 100644 index 52ccb038a..000000000 --- a/src/ui/components/chrome/AgentSkillDialog.tsx +++ /dev/null @@ -1,96 +0,0 @@ -import type { MouseEvent as TuiMouseEvent } from "@opentui/core"; -import { fitText, padText } from "../../lib/text"; -import type { AppTheme } from "../../themes"; -import { ModalFrame } from "./ModalFrame"; - -export const AGENT_SKILL_COMMAND = "hunk skill path"; -export const AGENT_SKILL_PROMPT_ROWS = [ - "Load the Hunk skill and use it for this review.", - "Run `hunk skill path` to get the skill path.", -]; -export const AGENT_SKILL_PROMPT = AGENT_SKILL_PROMPT_ROWS.join(" "); - -/** Render copyable setup guidance for connecting an agent to the live Hunk session. */ -export function AgentSkillDialog({ - copySupported, - terminalHeight, - terminalWidth, - theme, - onClose, - onCopyPrompt, -}: { - copySupported: boolean; - terminalHeight: number; - terminalWidth: number; - theme: AppTheme; - onClose: () => void; - onCopyPrompt: () => void; -}) { - const width = Math.min(84, Math.max(58, terminalWidth - 8)); - const bodyWidth = Math.max(1, width - 4); - const promptWidth = Math.max(1, bodyWidth - 4); - const promptRows = AGENT_SKILL_PROMPT_ROWS; - const cardWidth = Math.max(1, bodyWidth - 4); - const cardTextWidth = Math.max(1, cardWidth - 4); - const requiredModalHeight = promptRows.length + 11; - const modalHeight = Math.min(requiredModalHeight, Math.max(10, terminalHeight - 2)); - - const copyLabel = copySupported ? " ⧉ Copy prompt " : " Copy unavailable "; - return ( - - - - - {fitText("Teach your agent how to review this Hunk session.", bodyWidth)} - - - - - {fitText("Prompt", promptWidth)} - - - - {promptRows.map((line, index) => ( - - {fitText(line, cardTextWidth)} - - ))} - - - - - { - event.stopPropagation(); - if (copySupported) { - onCopyPrompt(); - } - }} - > - {copyLabel} - - {padText("", Math.max(1, bodyWidth - copyLabel.length))} - - - - ); -} diff --git a/src/ui/components/chrome/ExtensionDialog.tsx b/src/ui/components/chrome/ExtensionDialog.tsx index 5a79b6b79..2898f950e 100644 --- a/src/ui/components/chrome/ExtensionDialog.tsx +++ b/src/ui/components/chrome/ExtensionDialog.tsx @@ -1,13 +1,18 @@ -import type { MouseEvent as TuiMouseEvent } from "@opentui/core"; +import type { BoxRenderable, MouseEvent as TuiMouseEvent, Renderable } from "@opentui/core"; +import { useRenderer } from "@opentui/react"; +import { Component, useLayoutEffect, useMemo, useRef, type ReactNode } from "react"; +import type { ExtensionDialogActions, ExtensionDialogProps } from "../../../extension-api/types"; import type { ExtensionDialogRequest, ExtensionInputDialogRequest, + ExtensionOpenDialogRequest, ExtensionSelectDialogRequest, } from "../../lib/extensionDialogs"; +import { planExtensionOpenDialog, windowDialogText } from "../../lib/extensionDialogGeometry"; import { extensionToastPrefix } from "../../lib/extensionNotifications"; -import { windowDialogText } from "../../lib/extensionDialogGeometry"; import { listWindowStart } from "../../lib/listWindow"; import { MODAL_FRAME_CHROME_ROWS, resolveModalGeometry } from "../../lib/modalGeometry"; +import { toExtensionPaintTheme } from "../../lib/extensionPaintTheme"; import { fitText, padText } from "../../lib/text"; import type { AppTheme } from "../../themes"; import { ConfirmDialog, confirmDialogHeight, DialogActionRow } from "./ConfirmDialog"; @@ -36,37 +41,74 @@ function attributionText(extensionId: string, width: number) { return fitText(`${extensionToastPrefix()} ${extensionId}`, width); } +/** Report whether one focused renderable belongs to a custom dialog's bounded root. */ +function isWithinRenderable(root: Renderable, candidate: Renderable | null) { + let current = candidate; + while (current) { + if (current === root) return true; + current = current.parent; + } + return false; +} + export function ExtensionDialog({ + copySupported, inputValue, - onAccept, - onCancel, - onChangeInput, - onPickOption, + onAcceptRequest, + onCancelRequest, + onChangeInputRequest, + onClose, + onCopy, + onNotify, + onPickOptionRequest, + onRenderFailure, request, selectedIndex, terminalHeight, terminalWidth, theme, }: { + copySupported: boolean; /** Live text of an input dialog's field; ignored by the other kinds. */ inputValue: string; - onAccept: (selectedIndexOverride?: number) => void; - onCancel: () => void; - onChangeInput: (value: string) => void; + onAcceptRequest: (requestId: number, selectedIndexOverride?: number) => void; + onCancelRequest: (requestId: number) => void; + onChangeInputRequest: (requestId: number, value: string) => void; + onClose: (requestId: number) => void; + onCopy: (requestId: number, text: string) => boolean; + onNotify: (requestId: number, message: string) => void; /** Highlight one option row without accepting it, mirroring the theme selector. */ - onPickOption: (index: number) => void; + onPickOptionRequest: (requestId: number, index: number) => void; + onRenderFailure: (requestId: number, error: unknown) => void; request: ExtensionDialogRequest; selectedIndex: number; terminalHeight: number; terminalWidth: number; theme: AppTheme; }) { + if (request.kind === "open") { + return ( + onCancelRequest(request.id)} + onClose={onClose} + onCopy={onCopy} + onNotify={onNotify} + onRenderFailure={onRenderFailure} + request={request} + terminalHeight={terminalHeight} + terminalWidth={terminalWidth} + theme={theme} + /> + ); + } + if (request.kind === "select") { return ( onAcceptRequest(request.id, selectedIndexOverride)} + onCancel={() => onCancelRequest(request.id)} + onPickOption={(index) => onPickOptionRequest(request.id, index)} request={request} selectedIndex={selectedIndex} terminalHeight={terminalHeight} @@ -80,9 +122,9 @@ export function ExtensionDialog({ return ( onAcceptRequest(request.id)} + onCancel={() => onCancelRequest(request.id)} + onChangeInput={(value) => onChangeInputRequest(request.id, value)} request={request} terminalHeight={terminalHeight} terminalWidth={terminalWidth} @@ -109,8 +151,16 @@ export function ExtensionDialog({ return ( onAcceptRequest(request.id), + }, + { + keyLabel: "esc/n", + label: request.cancelLabel, + run: () => onCancelRequest(request.id), + }, ]} height={confirmDialogHeight(visibleBody.lines.length + attributionRows + attributionGapRows)} terminalHeight={terminalHeight} @@ -118,7 +168,7 @@ export function ExtensionDialog({ theme={theme} title={request.title} width={frame.width} - onClose={onCancel} + onClose={() => onCancelRequest(request.id)} > {attributionRows > 0 ? ( @@ -136,6 +186,151 @@ export function ExtensionDialog({ ); } +/** Contain a custom dialog's render failure to its request identity. */ +class ExtensionDialogErrorBoundary extends Component< + { + request: ExtensionOpenDialogRequest; + fallback: ReactNode; + onError: (error: unknown) => void; + retireActions: () => void; + children: ReactNode; + }, + { failed: boolean; request: ExtensionOpenDialogRequest | null } +> { + override state = { failed: false, request: null as ExtensionOpenDialogRequest | null }; + static getDerivedStateFromError() { + return { failed: true }; + } + static getDerivedStateFromProps( + props: { request: ExtensionOpenDialogRequest }, + state: { failed: boolean; request: ExtensionOpenDialogRequest | null }, + ) { + return props.request !== state.request ? { request: props.request, failed: false } : null; + } + override componentDidCatch(error: unknown) { + this.props.retireActions(); + this.props.onError(error); + } + override componentWillUnmount() { + this.props.retireActions(); + } + override render() { + return this.state.failed ? this.props.fallback : this.props.children; + } +} + +/** Mount an extension-owned component inside host-controlled modal chrome. */ +function ExtensionOpenDialog({ + copySupported, + onCancel, + onClose, + onCopy, + onNotify, + onRenderFailure, + request, + terminalHeight, + terminalWidth, + theme, +}: { + copySupported: boolean; + onCancel: () => void; + onClose: (requestId: number) => void; + onCopy: (requestId: number, text: string) => boolean; + onNotify: (requestId: number, message: string) => void; + onRenderFailure: (requestId: number, error: unknown) => void; + request: ExtensionOpenDialogRequest; + terminalHeight: number; + terminalWidth: number; + theme: AppTheme; +}) { + const renderer = useRenderer(); + const componentRootRef = useRef(null); + const layout = planExtensionOpenDialog(request, terminalWidth, terminalHeight); + const { attributionGapRows, attributionRows, bodyWidth, componentHeight, frame } = layout; + const publicTheme = useMemo(() => toExtensionPaintTheme(theme), [theme]); + const actionLease = request.actionLease; + const actions = useMemo( + () => + Object.freeze({ + close: () => { + if (actionLease.active) onClose(request.id); + }, + copy: (text: string) => actionLease.active && onCopy(request.id, text), + notify: (message: string) => { + if (actionLease.active) onNotify(request.id, message); + }, + }), + [actionLease, onClose, onCopy, onNotify, request.id], + ); + const viewProps: ExtensionDialogProps = { + width: bodyWidth, + height: componentHeight, + theme: publicTheme, + copySupported, + actions, + }; + const View = request.component as (props: ExtensionDialogProps) => ReactNode; + const componentBox = (children: ReactNode, fallback = false) => ( + 0} + style={{ + width: bodyWidth, + height: componentHeight, + flexShrink: 0, + overflow: "hidden", + flexDirection: "column", + backgroundColor: theme.panel, + }} + > + {children} + + ); + + useLayoutEffect(() => { + const root = componentRootRef.current; + if (root && !isWithinRenderable(root, renderer.currentFocusedRenderable)) { + // A nested input that focused itself during mount wins. Otherwise the + // bounded root traps unhandled keys before they reach the review. + root.focus(); + } + }, [renderer, request.id]); + + return ( + + {attributionRows > 0 ? ( + + {layout.attributionText} + + ) : null} + {attributionGapRows > 0 ? : null} + Dialog unavailable, true)} + retireActions={() => { + actionLease.active = false; + }} + onError={(error) => { + onRenderFailure(request.id, error); + }} + > + {componentBox()} + + + ); +} + /** Render a select dialog as a keyboard- and mouse-driven option list. */ function ExtensionSelectDialog({ onAccept, diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index fe8b6f44c..bd105ac92 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -318,6 +318,7 @@ export function DiffPane({ draftNoteFocused = false, separatorWidth, pagerMode = false, + keyboardFocusBlocked = false, copyDecorations = false, screenTop = 0, showTopChrome, @@ -387,6 +388,8 @@ export function DiffPane({ draftNoteFocused?: boolean; separatorWidth: number; pagerMode?: boolean; + /** Prevent a modal custom surface from forwarding unhandled keys to the review stream. */ + keyboardFocusBlocked?: boolean; copyDecorations?: boolean; screenTop?: number; showTopChrome?: boolean; @@ -2710,7 +2713,7 @@ export function DiffPane({ height="100%" scrollY={true} viewportCulling={true} - focused={pagerMode} + focused={pagerMode && !keyboardFocusBlocked} onMouseDown={beginCopySelection} onMouseDrag={updateCopySelection} onMouseDragEnd={endCopySelection} diff --git a/src/ui/hooks/useAppKeyboardShortcuts.ts b/src/ui/hooks/useAppKeyboardShortcuts.ts index ac288555c..79bad6d6f 100644 --- a/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/src/ui/hooks/useAppKeyboardShortcuts.ts @@ -23,7 +23,6 @@ type FocusArea = "files" | "filter" | "note"; export interface UseAppKeyboardShortcutsOptions { activeMenuId: MenuId | null; activateCurrentMenuItem: () => void; - closeAgentSkill: () => void; closeHelp: () => void; closeMenu: () => void; acceptThemeSelector: () => void; @@ -36,8 +35,8 @@ export interface UseAppKeyboardShortcutsOptions { */ commands: readonly AppCommand[]; denyRepoExtensions: () => void; - /** The extension dialog currently on screen, or `null` when none is. */ - extensionDialog: ExtensionDialogRequest | null; + /** Read the live queued dialog; several keys can arrive before React renders it. */ + getExtensionDialog: () => ExtensionDialogRequest | null; acceptExtensionDialog: () => void; cancelExtensionDialog: () => void; moveExtensionDialogSelection: (delta: number) => void; @@ -69,7 +68,6 @@ export interface UseAppKeyboardShortcutsOptions { neverAskToSaveViewPreferencesAndQuit: () => void; closeSaveConfigPrompt: () => void; saveDraftNote: () => void; - showAgentSkill: boolean; showHelp: boolean; switchMenu: (delta: number) => void; toggleFocusArea: () => void; @@ -98,7 +96,6 @@ export interface UseAppKeyboardShortcutsOptions { export function useAppKeyboardShortcuts({ activeMenuId, activateCurrentMenuItem, - closeAgentSkill, closeHelp, closeMenu, acceptThemeSelector, @@ -107,7 +104,7 @@ export function useAppKeyboardShortcuts({ closeExtensionTrustPrompt, commands, denyRepoExtensions, - extensionDialog, + getExtensionDialog, acceptExtensionDialog, cancelExtensionDialog, moveExtensionDialogSelection, @@ -129,7 +126,6 @@ export function useAppKeyboardShortcuts({ neverAskToSaveViewPreferencesAndQuit, closeSaveConfigPrompt, saveDraftNote, - showAgentSkill, showHelp, switchMenu, toggleFocusArea, @@ -138,12 +134,11 @@ export function useAppKeyboardShortcuts({ const activeMenuIdRef = useRef(activeMenuId); const commandsRef = useRef(commands); const focusAreaRef = useRef(focusArea); - const showAgentSkillRef = useRef(showAgentSkill); const showHelpRef = useRef(showHelp); const saveConfigPromptOpenRef = useRef(saveConfigPromptOpen); const themeSelectorOpenRef = useRef(themeSelectorOpen); const extensionTrustPromptOpenRef = useRef(extensionTrustPromptOpen); - const extensionDialogRef = useRef(extensionDialog); + const getExtensionDialogRef = useRef(getExtensionDialog); // The mode callbacks read live App state (which mode is running, its context), // so they are reached through refs rather than captured when the chain is built. const isFileViewModeActiveRef = useRef(isFileViewModeActive); @@ -152,7 +147,7 @@ export function useAppKeyboardShortcuts({ const isKeyboardModeActiveRef = useRef(isKeyboardModeActive); const exitKeyboardModeRef = useRef(exitKeyboardMode); const sendKeyboardModeKeyRef = useRef(sendKeyboardModeKey); - // These three close over live dialog state (the highlighted option, the typed + // These callbacks close over live dialog state (the highlighted option, the typed // text), so they are read through refs rather than captured once. const acceptExtensionDialogRef = useRef(acceptExtensionDialog); const cancelExtensionDialogRef = useRef(cancelExtensionDialog); @@ -161,12 +156,11 @@ export function useAppKeyboardShortcuts({ activeMenuIdRef.current = activeMenuId; commandsRef.current = commands; focusAreaRef.current = focusArea; - showAgentSkillRef.current = showAgentSkill; showHelpRef.current = showHelp; saveConfigPromptOpenRef.current = saveConfigPromptOpen; themeSelectorOpenRef.current = themeSelectorOpen; extensionTrustPromptOpenRef.current = extensionTrustPromptOpen; - extensionDialogRef.current = extensionDialog; + getExtensionDialogRef.current = getExtensionDialog; isFileViewModeActiveRef.current = isFileViewModeActive; exitFileViewModeRef.current = exitFileViewMode; sendFileViewModeKeyRef.current = sendFileViewModeKey; @@ -229,17 +223,12 @@ export function useAppKeyboardShortcuts({ return "mine"; }; - /** Escape closes the topmost open overlay (agent skill, then help). */ + /** Escape closes Hunk's help overlay. */ const handleDialogShortcut = (key: KeyEvent): KeyOwner => { if (!isEscapeKey(key)) { return "notMine"; } - if (showAgentSkillRef.current) { - closeAgentSkill(); - return "mine"; - } - if (showHelpRef.current) { closeHelp(); return "mine"; @@ -322,12 +311,12 @@ export function useAppKeyboardShortcuts({ * extension may not outrank them — and above menus, help, and the command * table. * - * The input kind is the one non-modal-shaped answer: keys it does not act on - * are the text the user is typing into the dialog's focused field, so they - * are the focused widget's, not swallowed. + * Input and open dialogs leave unclaimed keys for their focused surface. The + * open dialog's bounded root takes focus even when its component has no input, + * so those keys cannot reach a previously focused review widget behind it. */ const handleExtensionDialogShortcut = (key: KeyEvent): KeyOwner => { - const dialog = extensionDialogRef.current; + const dialog = getExtensionDialogRef.current(); if (!dialog) { return "notMine"; } @@ -338,8 +327,11 @@ export function useAppKeyboardShortcuts({ } if (key.name === "return" || key.name === "enter") { - acceptExtensionDialogRef.current(); - return "mine"; + if (dialog.kind !== "open") { + acceptExtensionDialogRef.current(); + return "mine"; + } + return "focused"; } if (dialog.kind === "select") { @@ -370,7 +362,7 @@ export function useAppKeyboardShortcuts({ } } - return dialog.kind === "input" ? "focused" : "mine"; + return dialog.kind === "input" || dialog.kind === "open" ? "focused" : "mine"; }; /** Own every key while the theme selector is up; it is a modal surface. */ diff --git a/src/ui/hooks/useExtensionDialogController.test.tsx b/src/ui/hooks/useExtensionDialogController.test.tsx index 1f12e4a11..d95232d7b 100644 --- a/src/ui/hooks/useExtensionDialogController.test.tsx +++ b/src/ui/hooks/useExtensionDialogController.test.tsx @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { testRender } from "@opentui/react/test-utils"; -import { act, useState } from "react"; +import { act, useLayoutEffect, useState } from "react"; import { useExtensionDialogController } from "./useExtensionDialogController"; /** Mount the controller with a replaceable review-generation token. */ @@ -57,7 +57,9 @@ describe("useExtensionDialogController", () => { expect(harness.controller().selectedIndex).toBe(0); expect(harness.controller().inputValue).toBe("feature/base"); - await act(async () => harness.controller().updateInput("feature/typed")); + await act(async () => + harness.controller().updateInput(harness.controller().request!.id, "feature/typed"), + ); await act(async () => harness.controller().accept()); expect(await typed).toBe("feature/typed"); await flush(harness.setup); @@ -67,6 +69,43 @@ describe("useExtensionDialogController", () => { } }); + test("ignores controls retained from a rendered request after promotion", async () => { + const harness = await renderController(); + const dialogs = harness.controller().createDialogs("probe"); + let selected!: Promise; + let typed!: Promise; + + try { + await act(async () => { + selected = dialogs.select({ title: "First", options: ["one", "two"] }); + typed = dialogs.input({ title: "Second", initial: "initial" }); + }); + await flush(harness.setup); + const firstId = harness.controller().request!.id; + + await act(async () => { + harness.controller().pickOption(firstId, 1); + harness.controller().acceptRequest(firstId); + harness.controller().pickOption(firstId, 0); + harness.controller().updateInput(firstId, "stale"); + harness.controller().acceptRequest(firstId); + }); + + expect(await selected).toBe("two"); + expect(harness.controller().getCurrentRequest()).toMatchObject({ + kind: "input", + title: "Second", + }); + await flush(harness.setup); + expect(harness.controller().inputValue).toBe("initial"); + + await act(async () => harness.controller().cancel()); + expect(await typed).toBeNull(); + } finally { + await act(async () => harness.setup.renderer.destroy()); + } + }); + test("cancels pending requests when a soft reload replaces the review", async () => { const harness = await renderController(); const dialogs = harness.controller().createDialogs("probe"); @@ -117,4 +156,41 @@ describe("useExtensionDialogController", () => { expect(await selected).toBeNull(); expect(await dialogs.input({ title: "Too late?" })).toBeNull(); }); + + test("retires the current request before child layout cleanup", async () => { + let controller!: ReturnType; + let requestDuringCleanup: unknown = "not cleaned"; + + function CleanupProbe() { + useLayoutEffect( + () => () => { + requestDuringCleanup = controller.getCurrentRequest(); + }, + [], + ); + return null; + } + + function Harness() { + controller = useExtensionDialogController({ reviewGeneration: "review" }); + return ; + } + + const setup = await testRender(, { width: 40, height: 4 }); + await act(async () => setup.renderOnce()); + let pending!: Promise; + await act(async () => { + pending = controller.createDialogs("probe").open({ + title: "Open", + component: () => null, + }); + }); + await act(async () => setup.renderOnce()); + expect(controller.getCurrentRequest()).toMatchObject({ kind: "open" }); + + await act(async () => setup.renderer.destroy()); + + expect(requestDuringCleanup).toBeNull(); + expect(await pending).toBeUndefined(); + }); }); diff --git a/src/ui/hooks/useExtensionDialogController.ts b/src/ui/hooks/useExtensionDialogController.ts index 6fd1b925c..265fa4c2a 100644 --- a/src/ui/hooks/useExtensionDialogController.ts +++ b/src/ui/hooks/useExtensionDialogController.ts @@ -1,4 +1,11 @@ -import { useEffect, useLayoutEffect, useRef, useState, useSyncExternalStore } from "react"; +import { + useCallback, + useEffect, + useLayoutEffect, + useRef, + useState, + useSyncExternalStore, +} from "react"; import { createExtensionDialogQueue, type ExtensionDialogQueue, @@ -8,17 +15,25 @@ import { export interface ExtensionDialogController { /** Build the dialog capability one extension command receives. */ createDialogs: ExtensionDialogQueue["createDialogs"]; + /** Read the queue's live request rather than a render-time snapshot. */ + getCurrentRequest: ExtensionDialogQueue["current"]; + /** Check request identity and generation liveness at the moment an action runs. */ + isCurrentRequestLive: ExtensionDialogQueue["isCurrentLive"]; + /** Cancel one request only while it remains at the front of the queue. */ + cancelRequest: ExtensionDialogQueue["cancel"]; /** Request currently visible to the user. */ request: ExtensionDialogRequest | null; selectedIndex: number; inputValue: string; accept: (selectedIndexOverride?: number) => void; + /** Accept only the rendered request with this id; stale controls are ignored. */ + acceptRequest: (requestId: number, selectedIndexOverride?: number) => void; cancel: () => void; /** Cancel the visible request and every queued request with their kind-specific values. */ cancelAll: () => void; moveSelection: (delta: number) => void; - pickOption: (index: number) => void; - updateInput: (value: string) => void; + pickOption: (requestId: number, index: number) => void; + updateInput: (requestId: number, value: string) => void; } /** Own the React state and lifetime of one App instance's extension-dialog queue. */ @@ -32,14 +47,27 @@ export function useExtensionDialogController({ const request = useSyncExternalStore(queue.subscribe, queue.current, queue.current); const [selectedIndex, setSelectedIndex] = useState(0); const [inputValue, setInputValue] = useState(""); - const requestId = request?.id ?? null; - const initialInput = request?.kind === "input" ? request.initial : ""; + const selectedIndexRef = useRef(0); + const inputValueRef = useRef(""); + const answerRequestIdRef = useRef(null); + /** Align mutable answer state before another key can arrive ahead of React. */ + const alignAnswerState = useCallback((active: ExtensionDialogRequest | null) => { + const activeId = active?.id ?? null; + if (answerRequestIdRef.current === activeId) return; + + answerRequestIdRef.current = activeId; + selectedIndexRef.current = 0; + inputValueRef.current = active?.kind === "input" ? active.initial : ""; + }, []); + + alignAnswerState(request); useEffect(() => { // A promoted queued request must never inherit the previous request's answer state. - setSelectedIndex(0); - setInputValue(initialInput); - }, [initialInput, requestId]); + alignAnswerState(request); + setSelectedIndex(selectedIndexRef.current); + setInputValue(inputValueRef.current); + }, [alignAnswerState, request]); const previousReviewGenerationRef = useRef(reviewGeneration); useLayoutEffect(() => { @@ -52,46 +80,105 @@ export function useExtensionDialogController({ } }, [queue, reviewGeneration]); - useEffect(() => { - // Settle every pending handler when this App instance leaves the review tree. + useLayoutEffect(() => { + // Retire capabilities before custom component layout cleanups can retain or + // invoke actions from a dialog whose App is already leaving the tree. return () => queue.shutdown(); }, [queue]); - /** Answer the visible request with the state appropriate to its dialog kind. */ - const accept = (selectedIndexOverride?: number) => { - if (!request) return; + /** Answer one rendered request only while it remains current. */ + const acceptRequest = useCallback( + (requestId: number, selectedIndexOverride?: number) => { + const active = queue.current(); + alignAnswerState(active); + if (!active || active.id !== requestId) return; - if (request.kind === "select") { - queue.accept(request.id, request.options[selectedIndexOverride ?? selectedIndex]); - return; - } + if (active.kind === "select") { + queue.accept(active.id, active.options[selectedIndexOverride ?? selectedIndexRef.current]); + alignAnswerState(queue.current()); + return; + } - queue.accept(request.id, request.kind === "input" ? inputValue : undefined); + queue.accept(active.id, active.kind === "input" ? inputValueRef.current : undefined); + alignAnswerState(queue.current()); + }, + [alignAnswerState, queue], + ); + + /** Answer the live request with the state appropriate to its dialog kind. */ + const accept = (selectedIndexOverride?: number) => { + const active = queue.current(); + if (active) acceptRequest(active.id, selectedIndexOverride); }; /** Dismiss the visible request with its kind-specific cancel value. */ const cancel = () => { - if (request) queue.cancel(request.id); + const active = queue.current(); + if (active) queue.cancel(active.id); + alignAnswerState(queue.current()); }; /** Move a select request's highlight, wrapping at both ends. */ const moveSelection = (delta: number) => { - if (request?.kind !== "select") return; + const active = queue.current(); + alignAnswerState(active); + if (active?.kind !== "select") return; + + const optionCount = active.options.length; + const next = (selectedIndexRef.current + delta + optionCount) % optionCount; + selectedIndexRef.current = next; + setSelectedIndex(next); + }; + + /** Cancel one live request and synchronously prepare any promoted answer state. */ + const cancelRequest = useCallback( + (id: number) => { + queue.cancel(id); + alignAnswerState(queue.current()); + }, + [alignAnswerState, queue], + ); + + /** Drain every request and synchronously clear its mutable answer state. */ + const cancelAll = useCallback(() => { + queue.cancelAll(); + alignAnswerState(queue.current()); + }, [alignAnswerState, queue]); + + /** Select one option while keeping the same-flush answer state current. */ + const pickOption = (requestId: number, index: number) => { + const active = queue.current(); + alignAnswerState(active); + if (active?.kind !== "select" || active.id !== requestId) return; + + selectedIndexRef.current = index; + setSelectedIndex(index); + }; + + /** Update input text in both React and same-flush answer state. */ + const updateInput = (requestId: number, value: string) => { + const active = queue.current(); + alignAnswerState(active); + if (active?.kind !== "input" || active.id !== requestId) return; - const optionCount = request.options.length; - setSelectedIndex((current) => (current + delta + optionCount) % optionCount); + inputValueRef.current = value; + setInputValue(value); }; return { createDialogs: queue.createDialogs, + getCurrentRequest: queue.current, + isCurrentRequestLive: queue.isCurrentLive, + cancelRequest, request, selectedIndex, inputValue, accept, + acceptRequest, cancel, - cancelAll: queue.cancelAll, + cancelAll, moveSelection, - pickOption: setSelectedIndex, - updateInput: setInputValue, + pickOption, + updateInput, }; } diff --git a/src/ui/lib/extensionDialogGeometry.test.ts b/src/ui/lib/extensionDialogGeometry.test.ts index c7ed4ba19..c96001017 100644 --- a/src/ui/lib/extensionDialogGeometry.test.ts +++ b/src/ui/lib/extensionDialogGeometry.test.ts @@ -1,5 +1,19 @@ import { describe, expect, test } from "bun:test"; -import { windowDialogText } from "./extensionDialogGeometry"; +import type { ExtensionOpenDialogRequest } from "./extensionDialogs"; +import { planExtensionOpenDialog, windowDialogText } from "./extensionDialogGeometry"; + +const TestDialog = () => null; +const openRequest = { + id: 1, + kind: "open", + extensionId: "example", + showAttribution: true, + title: "Custom surface", + width: 64, + height: 12, + component: TestDialog, + actionLease: { active: true }, +} satisfies ExtensionOpenDialogRequest; describe("windowDialogText", () => { test("wraps prose within the available terminal-cell rows", () => { @@ -17,3 +31,35 @@ describe("windowDialogText", () => { expect(windowDialogText(["overflow"], 3, 0)).toEqual({ lines: [], truncated: true }); }); }); + +describe("planExtensionOpenDialog", () => { + test("gives the component its preferred rectangle after host attribution", () => { + const layout = planExtensionOpenDialog(openRequest, 120, 40); + + expect(layout.frame).toMatchObject({ width: 68, height: 19 }); + expect(layout.bodyWidth).toBe(64); + expect(layout.componentHeight).toBe(12); + expect(layout.attributionRows).toBe(1); + expect(layout.attributionGapRows).toBe(1); + expect(layout.attributionText).toBe("ext example"); + }); + + test("clamps the component rectangle while preserving attribution first", () => { + const layout = planExtensionOpenDialog(openRequest, 50, 12); + + expect(layout.frame).toMatchObject({ width: 48, height: 10 }); + expect(layout.bodyWidth).toBe(44); + expect(layout.attributionRows).toBe(1); + expect(layout.attributionGapRows).toBe(1); + expect(layout.componentHeight).toBe(3); + }); + + test("gives bundled components the attribution rows they do not need", () => { + const layout = planExtensionOpenDialog({ ...openRequest, showAttribution: false }, 120, 40); + + expect(layout.frame.height).toBe(17); + expect(layout.attributionRows).toBe(0); + expect(layout.attributionGapRows).toBe(0); + expect(layout.componentHeight).toBe(12); + }); +}); diff --git a/src/ui/lib/extensionDialogGeometry.ts b/src/ui/lib/extensionDialogGeometry.ts index 795b8c66c..cd1bb710e 100644 --- a/src/ui/lib/extensionDialogGeometry.ts +++ b/src/ui/lib/extensionDialogGeometry.ts @@ -1,4 +1,7 @@ -import { wrapText } from "./text"; +import { fitText, wrapText } from "./text"; +import { extensionToastPrefix } from "./extensionNotifications"; +import { MODAL_FRAME_CHROME_ROWS, resolveModalGeometry } from "./modalGeometry"; +import type { ExtensionOpenDialogRequest } from "./extensionDialogs"; /** Wrapped body rows that fit one modal body allocation. */ export interface WindowedDialogText { @@ -25,3 +28,42 @@ export function windowDialogText( truncated: true, }; } + +/** Concrete host frame and component rectangle for one open dialog. */ +export interface ExtensionOpenDialogLayout { + frame: { width: number; height: number }; + bodyWidth: number; + componentHeight: number; + attributionText: string; + attributionRows: number; + attributionGapRows: number; +} + +/** Clamp an extension-owned component while preserving host attribution above it. */ +export function planExtensionOpenDialog( + request: ExtensionOpenDialogRequest, + terminalWidth: number, + terminalHeight: number, +): ExtensionOpenDialogLayout { + const attributionRequestRows = request.showAttribution ? 2 : 0; + const frame = resolveModalGeometry({ + width: request.width + 4, + height: request.height + MODAL_FRAME_CHROME_ROWS + attributionRequestRows, + terminalWidth, + terminalHeight, + }); + const bodyWidth = Math.max(1, frame.width - 4); + const contentRows = Math.max(0, frame.height - MODAL_FRAME_CHROME_ROWS); + const attributionRows = request.showAttribution && contentRows > 0 ? 1 : 0; + const attributionGapRows = attributionRows > 0 && contentRows > 1 ? 1 : 0; + const componentHeight = Math.max(0, contentRows - attributionRows - attributionGapRows); + + return { + frame, + bodyWidth, + componentHeight, + attributionText: fitText(`${extensionToastPrefix()} ${request.extensionId}`, bodyWidth), + attributionRows, + attributionGapRows, + }; +} diff --git a/src/ui/lib/extensionDialogs.test.ts b/src/ui/lib/extensionDialogs.test.ts index 2b5b4ab8c..4a37ebeb1 100644 --- a/src/ui/lib/extensionDialogs.test.ts +++ b/src/ui/lib/extensionDialogs.test.ts @@ -1,5 +1,10 @@ import { describe, expect, test } from "bun:test"; -import { createExtensionDialogQueue } from "./extensionDialogs"; +import { + createExtensionDialogQueue, + normalizeExtensionDialogClipboardText, +} from "./extensionDialogs"; + +const TestDialog = () => null; describe("createExtensionDialogQueue", () => { test("shows one dialog at a time and queues the rest in call order", async () => { @@ -50,6 +55,12 @@ describe("createExtensionDialogQueue", () => { const valueless = dialogs.select({ title: "Which?", options: ["a"] }); queue.accept(queue.current()!.id); expect(await valueless).toBeNull(); + + const opened = dialogs.open({ title: "Guide", component: TestDialog }); + queue.accept(queue.current()!.id); + expect(queue.current()).toMatchObject({ kind: "open", title: "Guide" }); + queue.cancel(queue.current()!.id); + expect(await opened).toBeUndefined(); }); test("ignores an answer aimed at a dialog that is no longer current", async () => { @@ -115,6 +126,32 @@ describe("createExtensionDialogQueue", () => { expect(queue.current()).toMatchObject({ title: "Pick", options: ["opt"] }); }); + test("carries a custom component and default rectangle into the request", () => { + const queue = createExtensionDialogQueue(); + const dialogs = queue.createDialogs("guide"); + + void dialogs.open({ + title: "Setup", + component: TestDialog, + }); + + expect(queue.current()).toMatchObject({ + kind: "open", + width: 64, + height: 12, + component: TestDialog, + }); + }); + + test("normalizes bounded custom-dialog clipboard payloads", () => { + expect(normalizeExtensionDialogClipboardText("copy\t\u001b[31mexactly\u001b[0m")).toBe( + "copy exactly", + ); + expect(normalizeExtensionDialogClipboardText("")).toBeNull(); + expect(normalizeExtensionDialogClipboardText("x".repeat(16_385))).toBeNull(); + expect(normalizeExtensionDialogClipboardText("\t".repeat(4_097))).toBeNull(); + }); + test("sanitizes an input dialog's starting text without trimming it", () => { const queue = createExtensionDialogQueue(); const dialogs = queue.createDialogs("hostile"); @@ -206,7 +243,7 @@ describe("createExtensionDialogQueue", () => { expect(await second).toBe(false); }); - test("rejects a blank title and a select with no options", async () => { + test("rejects blank sanitized titles and malformed select options", async () => { const queue = createExtensionDialogQueue(); const dialogs = queue.createDialogs("probe"); @@ -221,6 +258,36 @@ describe("createExtensionDialogQueue", () => { await expect(dialogs.select({ title: "Which?", options: [] })).rejects.toThrow( /at least one option/, ); + await expect(dialogs.confirm({ title: "\u001b[31m\u001b[0m" })).rejects.toThrow( + /non-empty title after terminal sanitization/, + ); + await expect( + dialogs.select({ title: "Which?", options: ["\u001b]0;pwned\u0007"] }), + ).rejects.toThrow(/remain non-empty after sanitization/); + await expect(dialogs.select({ title: "Which?", options: [" "] })).rejects.toThrow( + /remain non-empty after sanitization/, + ); + const sparseOptions = Array.from({ length: 2 }, () => "one"); + delete sparseOptions[0]; + sparseOptions[1] = "one"; + await expect(dialogs.select({ title: "Which?", options: sparseOptions })).rejects.toThrow( + /dense array of strings/, + ); + await expect(dialogs.open({ title: "No component", component: null as never })).rejects.toThrow( + /component function/, + ); + await expect( + dialogs.open({ title: "Bad width", width: 0, component: TestDialog }), + ).rejects.toThrow(/width must be an integer from 1 to 240/); + await expect( + dialogs.open({ title: "Wide", width: 241, component: TestDialog }), + ).rejects.toThrow(/width must be an integer from 1 to 240/); + await expect( + dialogs.open({ title: "Bad height", height: 1.5, component: TestDialog }), + ).rejects.toThrow(/height must be an integer from 1 to 100/); + await expect( + dialogs.open({ title: "Tall", height: 101, component: TestDialog }), + ).rejects.toThrow(/height must be an integer from 1 to 100/); await expect( dialogs.select({ title: "Which?", options: [1 as unknown as string] }), ).rejects.toThrow(/must all be strings/); diff --git a/src/ui/lib/extensionDialogs.ts b/src/ui/lib/extensionDialogs.ts index d97d5b9ae..6c9a77050 100644 --- a/src/ui/lib/extensionDialogs.ts +++ b/src/ui/lib/extensionDialogs.ts @@ -1,7 +1,7 @@ /** * The queue behind `ctx.dialogs`, kept free of React on purpose. * - * Extensions ask questions from async handlers, so the interesting behavior is + * Extensions open modal surfaces from async handlers, so the interesting behavior is * ordering and settlement — one dialog on screen at a time, later requests * waiting their turn, everything still waiting resolving its cancel value when * the session goes away. None of that is rendering, so it lives here as plain @@ -11,11 +11,12 @@ import type { ExtensionConfirmOptions, + ExtensionDialogOptions, ExtensionDialogs, ExtensionInputOptions, ExtensionSelectOptions, } from "../../extension-api/types"; -import { sanitizeTerminalLine } from "../../lib/terminalText"; +import { sanitizeTerminalLine, sanitizeTerminalText } from "../../lib/terminalText"; /** Default label for the accepting action of a confirm dialog. */ const DEFAULT_CONFIRM_LABEL = "ok"; @@ -26,6 +27,17 @@ const DEFAULT_CANCEL_LABEL = "cancel"; /** Body lines one confirm dialog may show; beyond this the modal stops being a prompt. */ const MAX_CONFIRM_BODY_LINES = 6; +/** Default extension-owned component rectangle. */ +const DEFAULT_OPEN_DIALOG_WIDTH = 64; +const DEFAULT_OPEN_DIALOG_HEIGHT = 12; + +/** Bounds keep one request from retaining absurd off-screen geometry. */ +const MAX_OPEN_DIALOG_WIDTH = 240; +const MAX_OPEN_DIALOG_HEIGHT = 100; + +/** Clipboard text is bounded before it reaches the terminal's OSC 52 channel. */ +const MAX_DIALOG_COPY_TEXT_LENGTH = 16_384; + /** What every queued dialog carries, whatever kind it is. */ interface ExtensionDialogRequestBase { /** @@ -61,14 +73,25 @@ export interface ExtensionInputDialogRequest extends ExtensionDialogRequestBase initial: string; } +/** One extension-owned component the host should mount in a modal frame. */ +export interface ExtensionOpenDialogRequest extends ExtensionDialogRequestBase { + kind: "open"; + width: number; + height: number; + component: ExtensionDialogOptions["component"]; + /** Shared across React render retries so every retained action can be retired together. */ + actionLease: { active: boolean }; +} + /** One dialog the host should draw, normalized from what an extension asked for. */ export type ExtensionDialogRequest = | ExtensionConfirmDialogRequest | ExtensionSelectDialogRequest - | ExtensionInputDialogRequest; + | ExtensionInputDialogRequest + | ExtensionOpenDialogRequest; /** What a dialog hands back to the awaiting handler. */ -type ExtensionDialogResult = boolean | string | null; +type ExtensionDialogResult = boolean | string | null | undefined; /** The host-side controller for every extension dialog in one session. */ export interface ExtensionDialogQueue { @@ -79,11 +102,14 @@ export interface ExtensionDialogQueue { ): ExtensionDialogs; /** The dialog that should be on screen, or `null` when none is. */ current(): ExtensionDialogRequest | null; + /** Whether this id is still the current request and its owning capability remains live. */ + isCurrentLive(id: number): boolean; /** * Accept the dialog with this id. * * A confirm resolves `true`. A select or input resolves `value`; without one - * there is nothing to hand back, so it settles as a cancel instead. + * there is nothing to hand back, so it settles as a cancel instead. Open + * component dialogs ignore acceptance and remain visible until cancelled. * * Answering anything but the current dialog is ignored: an answer computed * for a dialog the queue has already moved past — a repeated key, a late @@ -123,7 +149,12 @@ function normalizeTitle(method: string, title: unknown) { invalid(method, "requires a non-empty title."); } - return sanitizeTerminalLine(title.trim()); + const normalized = sanitizeTerminalLine(title.trim()).trim(); + if (normalized.length === 0) { + invalid(method, "requires a non-empty title after terminal sanitization."); + } + + return normalized; } /** Normalize an optional extension-authored label, falling back to Hunk's own. */ @@ -132,7 +163,7 @@ function normalizeLabel(label: unknown, fallback: string) { return fallback; } - return sanitizeTerminalLine(label.trim()); + return sanitizeTerminalLine(label.trim()).trim() || fallback; } /** @@ -143,30 +174,68 @@ function normalizeLabel(label: unknown, fallback: string) { * dialog text is third-party and routinely carries repo-controlled fragments, * exactly like toast text. */ -function normalizeBodyLines(body: unknown) { +function normalizeBodyLines(body: unknown, maxLines = MAX_CONFIRM_BODY_LINES) { if (typeof body !== "string" || body.length === 0) { return []; } return body .split("\n") - .slice(0, MAX_CONFIRM_BODY_LINES) + .slice(0, maxLines) .map((line) => sanitizeTerminalLine(line)); } +/** Normalize one preferred component dimension, or reject it. */ +function normalizeOpenDialogDimension( + name: "width" | "height", + value: unknown, + fallback: number, + maximum: number, +) { + if (value === undefined) return fallback; + if (!Number.isInteger(value) || (value as number) < 1 || (value as number) > maximum) { + invalid("open", `${name} must be an integer from 1 to ${maximum}.`); + } + return value as number; +} + +/** Normalize one custom-dialog clipboard payload, or reject it without throwing. */ +export function normalizeExtensionDialogClipboardText(text: unknown): string | null { + if (typeof text !== "string" || text.length === 0 || text.length > MAX_DIALOG_COPY_TEXT_LENGTH) { + return null; + } + + const normalized = sanitizeTerminalText(text).replaceAll("\t", " "); + return normalized.length > 0 && normalized.length <= MAX_DIALOG_COPY_TEXT_LENGTH + ? normalized + : null; +} + /** Normalize the choices of a select dialog, or reject them. */ function normalizeOptions(options: unknown) { if (!Array.isArray(options) || options.length === 0) { invalid("select", "requires at least one option."); } - return options.map((option) => { + const normalizedOptions: string[] = []; + for (let index = 0; index < options.length; index += 1) { + if (!Object.hasOwn(options, index)) { + invalid("select", "options must be a dense array of strings."); + } + const option = options[index]; if (typeof option !== "string") { - invalid("select", "options must all be strings."); + invalid("select", "options must all be strings that remain non-empty after sanitization."); } - return sanitizeTerminalLine(option); - }); + const normalized = sanitizeTerminalLine(option).trim(); + if (normalized.length === 0) { + invalid("select", "options must all be strings that remain non-empty after sanitization."); + } + + normalizedOptions.push(normalized); + } + + return normalizedOptions; } /** @@ -194,9 +263,14 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { } }; - /** The cancel value one request resolves with: `false` for confirm, `null` otherwise. */ + /** The cancel value one request resolves with. */ const cancelValueFor = (request: ExtensionDialogRequest): ExtensionDialogResult => - request.kind === "confirm" ? false : null; + request.kind === "confirm" ? false : request.kind === "open" ? undefined : null; + + /** Retire component actions before settling or removing their request. */ + const retireActions = (request: ExtensionDialogRequest) => { + if (request.kind === "open") request.actionLease.active = false; + }; /** * Queue one request and hand back the promise its handler awaits. @@ -231,6 +305,7 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { const drainPending = () => { const drained = pending.splice(0); for (const entry of drained) { + retireActions(entry.request); entry.settle(cancelValueFor(entry.request)); } @@ -246,6 +321,7 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { return; } + retireActions(active.request); active.settle(value); notify(); }; @@ -310,6 +386,39 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { isLive, ); }, + async open(options: ExtensionDialogOptions) { + const title = normalizeTitle("open", options?.title); + if (typeof options?.component !== "function") { + invalid("open", "requires a component function."); + } + const width = normalizeOpenDialogDimension( + "width", + options.width, + DEFAULT_OPEN_DIALOG_WIDTH, + MAX_OPEN_DIALOG_WIDTH, + ); + const height = normalizeOpenDialogDimension( + "height", + options.height, + DEFAULT_OPEN_DIALOG_HEIGHT, + MAX_OPEN_DIALOG_HEIGHT, + ); + await enqueue( + (id) => ({ + kind: "open", + id, + extensionId, + showAttribution, + title, + width, + height, + component: options.component, + actionLease: { active: true }, + }), + undefined, + isLive, + ); + }, }; }, @@ -317,6 +426,11 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { return pending[0]?.request ?? null; }, + isCurrentLive(id: number) { + const active = pending[0]; + return active?.request.id === id && active.isLive(); + }, + accept(id: number, value?: string) { const active = pending[0]; if (!active || active.request.id !== id) { @@ -333,6 +447,10 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { return; } + if (active.request.kind === "open") { + return; + } + settleCurrent(value ?? null); }, diff --git a/test/pty/chrome.test.ts b/test/pty/chrome.test.ts index 0eea9d935..7271c1907 100644 --- a/test/pty/chrome.test.ts +++ b/test/pty/chrome.test.ts @@ -84,6 +84,48 @@ describe("PTY chrome", () => { } }); + test("the Agent menu opens bundled skill guidance as a component dialog", async () => { + const fixture = harness.createTwoFileRepoFixture(); + const session = await harness.launchHunk({ + args: ["diff", "--mode", "split"], + cwd: fixture.dir, + cols: 120, + rows: 24, + }); + + try { + await session.waitForText(/View\s+Navigate\s+Agent\s+Help/, { timeout: 15_000 }); + await session.click(/Agent/, { first: true }); + const menu = await session.waitForText(/Agent skill/, { timeout: 5_000 }); + expect(menu).toContain("Next annotated file"); + + await session.click(/Agent skill/); + const info = await harness.waitForSnapshot( + session, + (text) => + text.includes("Teach your agent how to review this Hunk session.") && + text.includes("hunk skill path") && + text.includes("⧉ Copy prompt"), + 5_000, + ); + expect(info).not.toContain("ext hunk"); + + await session.press("enter"); + const stillOpen = await session.text({ immediate: true }); + expect(stillOpen).toContain("Teach your agent how to review this Hunk session."); + + await session.press("escape"); + const closed = await harness.waitForSnapshot( + session, + (text) => !text.includes("Teach your agent how to review this Hunk session."), + 5_000, + ); + expect(closed).toContain("alpha.ts"); + } finally { + session.close(); + } + }); + test("rapid theme preview key repeats keep the selector responsive", async () => { const initialThemeId = "github-dark-default"; const themes = availableThemes(); diff --git a/website/src/content/docs/docs/extend/extension-api.md b/website/src/content/docs/docs/extend/extension-api.md index f37846ef1..786d4a7a5 100644 --- a/website/src/content/docs/docs/extend/extension-api.md +++ b/website/src/content/docs/docs/extend/extension-api.md @@ -7,8 +7,9 @@ The extension factory receives one API object. Registration calls are only valid ## `hunk.apiVersion` -The API generation this Hunk speaks (currently `16`). Branch on it if you want -one file to support several Hunk versions. Version 16 adds temporary application +The API generation this Hunk speaks (currently `17`). Branch on it if you want +one file to support several Hunk versions. Version 17 adds custom React/OpenTUI +dialog surfaces; version 16 added temporary application handoffs and on-disk location resolution to command handlers; version 15 added `{ side, line }` to opted-in pane `currentLine` paint; version 14 added structured two-revision @@ -284,11 +285,12 @@ A handler may be async; a failure becomes a warning naming your extension. ### Asking the user -`ctx.dialogs` puts a question on screen and waits for the answer. Three methods, all return promises: +`ctx.dialogs` puts a modal surface on screen and waits for it to settle. Four methods return promises: - `confirm({ title, body?, confirmLabel?, cancelLabel? })` → `true` or `false` - `select({ title, options })` → the chosen string, or `null` - `input({ title, placeholder?, initial? })` → the typed string, or `null` +- `open({ title, width?, height?, component })` → `void` when closed ```ts hunk.registerCommand( @@ -310,6 +312,47 @@ hunk.registerCommand( ); ``` +`open` mounts a React/OpenTUI component in an exact host-owned rectangle, like +`registerPane` inside modal chrome. Preferred `width` and `height` default to +`64×12` and are clamped to the terminal. The component receives the resulting +dimensions, semantic theme, clipboard availability, and guarded `close`, `copy`, +and `notify` actions. Escape stays host-owned; other keys reach the component. + +```tsx +import type { ExtensionDialogProps } from "hunkdiff/extension"; + +const prompt = "Review the current Hunk session. Focus on correctness."; + +function AgentSetupDialog({ actions, copySupported, theme }: ExtensionDialogProps) { + const copy = () => { + actions.notify(actions.copy(prompt) ? "Copied agent prompt" : "Clipboard copy failed"); + }; + return ( + + {prompt} + + Copy prompt + + + ); +} + +hunk.registerCommand({ id: "agent-setup", title: "Agent setup" }, async (ctx) => { + await ctx.dialogs.open({ + title: "Agent setup", + width: 64, + height: 6, + component: AgentSetupDialog, + }); +}); +``` + +Component dialogs are trusted extension code like panes. Hunk cannot verify +what an arbitrary component visibly discloses before it calls `actions.copy`. +Hunk still owns bounds, frame chrome, attribution, Escape handling, queueing, +and render-failure containment. Clipboard payloads are sanitized and limited to +16,384 JavaScript string code units. + `select` fits acting on part of the selection — asking which hunk to jump to, then navigating there: ```ts @@ -331,9 +374,9 @@ hunk.registerCommand({ id: "pick-hunk", title: "Pick a hunk", key: "ctrl+k" }, a }); ``` -Hunk draws the dialog; your text fills the title, body, and choices. Dialogs from installed extensions carry an `ext ` attribution line — the same marker `notify` toasts use — so a third-party prompt cannot present itself as Hunk asking. Hunk's own bundled extensions omit that redundant marker. +Hunk draws every frame and every confirm/select/input surface. Dialogs from installed extensions carry an `ext ` attribution line — the same marker `notify` toasts use. Hunk's own bundled extensions omit that redundant marker. -One dialog shows at a time; concurrent requests queue in call order, across extensions. Escape cancels (`false` or `null`), Enter accepts; confirm dialogs also answer to `y`/`n`, select dialogs to `↑`/`↓`, and everything is clickable. A session reload cancels open and queued dialogs, and a dialog pending at shutdown resolves its cancel value. +One dialog shows at a time; concurrent requests queue in call order, across extensions. Escape cancels (`false` or `null`) or closes a component dialog. Enter accepts host-rendered interactive dialogs; component-dialog keys other than Escape reach the mounted surface. Confirm dialogs also answer to `y`/`n`, select dialogs to `↑`/`↓`, and host-rendered actions are clickable. A session reload cancels open and queued dialogs, and a dialog pending at shutdown resolves its cancel value. ### Temporary applications