From bbcee9ead15528c221ee1b39024e0facb8613fb3 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Sun, 30 Aug 2026 23:26:16 -0400 Subject: [PATCH 1/9] feat(extensions): move agent skill into bundled extension --- .changeset/fuzzy-agents-guide.md | 5 + docs/extension-architecture.md | 2 +- docs/extensions.md | 38 +++- skills/hunk-extensions/SKILL.md | 4 +- src/extension-api/index.ts | 2 + src/extension-api/types.ts | 28 ++- .../default/ui/agentSkill/index.test.ts | 39 ++++ src/extensions/default/ui/agentSkill/index.ts | 29 +++ src/extensions/default/ui/index.test.ts | 5 +- src/extensions/default/ui/index.ts | 13 +- src/extensions/events.test.ts | 1 + src/extensions/events.ts | 3 + src/extensions/types.ts | 2 + src/ui/App.tsx | 72 ++++--- src/ui/AppHost.extension-dialogs.test.tsx | 68 +++++++ src/ui/AppHost.interactions.test.tsx | 2 +- src/ui/components/chrome/AgentSkillDialog.tsx | 96 ---------- src/ui/components/chrome/ExtensionDialog.tsx | 181 ++++++++++++++++++ src/ui/hooks/useAppKeyboardShortcuts.ts | 28 +-- src/ui/lib/extensionDialogs.test.ts | 31 +++ src/ui/lib/extensionDialogs.ts | 93 ++++++++- test/pty/chrome.test.ts | 42 ++++ .../content/docs/docs/extend/extension-api.md | 12 +- 23 files changed, 615 insertions(+), 181 deletions(-) create mode 100644 .changeset/fuzzy-agents-guide.md create mode 100644 src/extensions/default/ui/agentSkill/index.test.ts create mode 100644 src/extensions/default/ui/agentSkill/index.ts delete mode 100644 src/ui/components/chrome/AgentSkillDialog.tsx diff --git a/.changeset/fuzzy-agents-guide.md b/.changeset/fuzzy-agents-guide.md new file mode 100644 index 000000000..f6132df0f --- /dev/null +++ b/.changeset/fuzzy-agents-guide.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": minor +--- + +Add copyable document dialogs 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 79baacd19..a338927db 100644 --- a/docs/extension-architecture.md +++ b/docs/extension-architecture.md @@ -241,7 +241,7 @@ text into a request the host draws, 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, input, and read-only copyable documents are `ModalFrame` surfaces), 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 diff --git a/docs/extensions.md b/docs/extensions.md index 120e3ddb2..ff936bd60 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 host-mediated editor +The API generation this Hunk speaks (currently `17`). Branch on it if you want +one file to support several Hunk versions. Version 17 adds read-only document +dialogs with optional clipboard actions; version 16 added host-mediated editor launches for reviewed files; version 15 added `{ side, line }` to opted-in pane `currentLine` paint; version 14 added structured `rangeEndpoints` to two-revision VCS diff requests; version 13 added saved-note parent identities and @@ -1609,12 +1610,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` +- `document({ title, body?, copy?: { label?, text } })` → `void` when closed ```ts hunk.registerCommand( @@ -1658,18 +1660,34 @@ hunk.registerCommand({ id: "pick-hunk", title: "Pick a hunk", key: "ctrl+k" }, a }); ``` +`document` presents read-only guidance rather than asking for an answer. At +least `body` or `copy` must be present. When `copy` is provided, `c` and the +clickable copy action send its `text` to the terminal clipboard while the host +removes terminal control sequences and renders the same safe value under +`label` (default `Content`): + +```ts +hunk.registerCommand({ id: "agent-setup", title: "Agent setup" }, async (ctx) => { + await ctx.dialogs.document({ + title: "Agent setup", + body: "Give this prompt to your coding agent.", + copy: { label: "Prompt", text: "Review the current Hunk session." }, + }); +}); +``` + 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. 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 document, Enter accepts the confirm action, highlighted +option, or typed text, and review shortcuts stay suppressed underneath. +Documents ignore Enter and remain open. 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 a8f2188b1..3eef11fe9 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,7 +160,7 @@ 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), and + `ctx.dialogs` (`confirm`/`select`/`input`/`document`, queued and attributed), and `ctx.workspace` (`readDocument`, host-mediated `openInEditor`, `canWriteDocument`, `writeDocument` with consent). - **Pane components** get frozen `files`, selection, placement, exact dimensions, diff --git a/src/extension-api/index.ts b/src/extension-api/index.ts index b588f18e6..1787c1235 100644 --- a/src/extension-api/index.ts +++ b/src/extension-api/index.ts @@ -95,6 +95,8 @@ export type { ExtensionReviewSnapshotNote, ExtensionReviewSnapshotNoteAnchor, ExtensionConfirmOptions, + ExtensionDocumentCopyOptions, + ExtensionDocumentOptions, ExtensionDialogs, ExtensionInputOptions, ExtensionSelectOptions, diff --git a/src/extension-api/types.ts b/src/extension-api/types.ts index 5ac231f00..7d9cf191c 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"; @@ -1621,8 +1621,25 @@ export interface ExtensionInputOptions { initial?: string; } +/** Copyable text shown inside a document dialog. */ +export interface ExtensionDocumentCopyOptions { + /** Short heading shown above the copyable text. Defaults to "Content". */ + label?: string; + /** Text copied to the terminal clipboard after Hunk removes terminal control sequences. */ + text: string; +} + +/** Read-only guidance shown to the user as a modal document. */ +export interface ExtensionDocumentOptions { + title: string; + /** Optional prose shown above the copyable text. */ + body?: string; + /** Optional text card the user can copy with `c` or the mouse. */ + copy?: ExtensionDocumentCopyOptions; +} + /** - * 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 @@ -1631,8 +1648,9 @@ export interface ExtensionInputOptions { * 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. Documents are read-only and remain open until dismissed. * 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 @@ -1652,6 +1670,8 @@ export interface ExtensionDialogs { select(options: ExtensionSelectOptions): Promise; /** Resolves the submitted text, or null on cancel/escape. */ input(options: ExtensionInputOptions): Promise; + /** Show read-only guidance until the user dismisses it. */ + document(options: ExtensionDocumentOptions): 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..d934188b3 --- /dev/null +++ b/src/extensions/default/ui/agentSkill/index.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, mock, test } from "bun:test"; +import type { ExtensionCommandContext } from "hunkdiff/extension"; +import { getBundledUIRegistry } from ".."; +import { AGENT_SKILL_PROMPT, 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 document dialog", async () => { + const document = mock(async () => {}); + const context = { dialogs: { document } } as unknown as ExtensionCommandContext; + + await getBundledAgentSkillCommand().handler(context); + + expect(document).toHaveBeenCalledWith({ + title: "Agent skill", + body: "Teach your agent how to review this Hunk session.", + copy: { label: "Prompt", text: AGENT_SKILL_PROMPT }, + }); + }); +}); diff --git a/src/extensions/default/ui/agentSkill/index.ts b/src/extensions/default/ui/agentSkill/index.ts new file mode 100644 index 000000000..7fa36a48e --- /dev/null +++ b/src/extensions/default/ui/agentSkill/index.ts @@ -0,0 +1,29 @@ +import type { ExtensionFactory } from "hunkdiff/extension"; + +export const AGENT_SKILL_COMMAND = "hunk skill path"; +export const AGENT_SKILL_PROMPT = + "Load the Hunk skill and use it for this review. Run `hunk skill path` to get the skill path."; +export const BUNDLED_AGENT_SKILL_COMMAND_ID = "app.openAgentSkill"; +export const BUNDLED_AGENT_SKILL_COMMAND_FULL_ID = `hunk.${BUNDLED_AGENT_SKILL_COMMAND_ID}`; + +/** 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.document({ + title: "Agent skill", + body: "Teach your agent how to review this Hunk session.", + copy: { + label: "Prompt", + text: AGENT_SKILL_PROMPT, + }, + }); + }, + ); +}; + +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..b3ecafbe8 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, + document: async () => {}, }, events: { emit: () => {} }, }; diff --git a/src/extensions/events.ts b/src/extensions/events.ts index 4d3bfa636..427197e59 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; }, + document: async () => { + unavailable(); + }, }; } diff --git a/src/extensions/types.ts b/src/extensions/types.ts index ab07601d2..2594a1cde 100644 --- a/src/extensions/types.ts +++ b/src/extensions/types.ts @@ -44,6 +44,8 @@ export type { ExtensionContext, ExtensionCustomEventHandler, ExtensionDiffFile, + ExtensionDocumentCopyOptions, + ExtensionDocumentOptions, ExtensionEventBus, ExtensionEventContext, ExtensionEventHandler, diff --git a/src/ui/App.tsx b/src/ui/App.tsx index e7ded3d47..cfb15323f 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -36,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"; @@ -112,9 +113,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 +203,10 @@ export function App({ const wrapToggleScrollTopRef = useRef(null); const layoutToggleScrollTopRef = useRef(null); const cancelCopySelectionRef = useRef<(() => void) | null>(null); + const activeReviewGenerationRef = useRef(bootstrap); + useLayoutEffect(() => { + activeReviewGenerationRef.current = bootstrap; + }, [bootstrap]); const [layoutToggleRequestId, setLayoutToggleRequestId] = useState(0); const [scrollEdgeRequest, setScrollEdgeRequest] = useState<{ id: number; @@ -224,7 +226,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; @@ -496,11 +497,15 @@ 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, + // 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: bundled ? () => activeReviewGenerationRef.current === bootstrap : lease.isLive, showAttribution: !bundled, }); }, @@ -552,6 +557,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); @@ -903,27 +917,23 @@ 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); - }, []); + runExtensionCommand(bundledAgentSkillCommand); + }, [bundledAgentSkillCommand, runExtensionCommand]); + + /** Copy a document dialog's normalized payload through the terminal clipboard integration. */ + const copyExtensionDialogDocument = useCallback(() => { + if (extensionDialog?.kind !== "document" || !extensionDialog.copy) return; - /** 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"); + renderer.copyToClipboardOSC52(extensionDialog.copy.text); + showTransientNotice(`Copied ${extensionDialog.copy.label.toLowerCase()} to clipboard`); return; } showTransientNotice("Clipboard copy unsupported in this terminal (enable OSC 52)"); - }, [renderer, showTransientNotice]); + }, [extensionDialog, renderer, showTransientNotice]); /** Toggle the modal keyboard help overlay. */ const toggleHelp = useCallback(() => { @@ -1108,7 +1118,6 @@ export function App({ useAppKeyboardShortcuts({ activeMenuId, activateCurrentMenuItem, - closeAgentSkill, closeHelp, closeMenu, acceptThemeSelector, @@ -1120,6 +1129,7 @@ export function App({ extensionDialog, acceptExtensionDialog, cancelExtensionDialog, + copyExtensionDialogDocument, moveExtensionDialogSelection, extensionTrustPromptOpen, trustRepoExtensions, @@ -1139,7 +1149,6 @@ export function App({ neverAskToSaveViewPreferencesAndQuit, closeSaveConfigPrompt, saveDraftNote, - showAgentSkill, showHelp, switchMenu, toggleFocusArea, @@ -1422,19 +1431,6 @@ export function App({ ) : null} - {showAgentSkill ? ( - - - - ) : null} - {showHelp ? ( copyExtensionDialogDocument()} onPickOption={setExtensionDialogSelectedIndex} /> ) : null} diff --git a/src/ui/AppHost.extension-dialogs.test.tsx b/src/ui/AppHost.extension-dialogs.test.tsx index 7df60410d..4c46e0975 100644 --- a/src/ui/AppHost.extension-dialogs.test.tsx +++ b/src/ui/AppHost.extension-dialogs.test.tsx @@ -275,6 +275,74 @@ describe("extension dialogs", () => { }); }); + test("a document dialog renders copyable guidance and closes only on escape", async () => { + const repo = createTestRepo("hunk-ext-dialog-document-"); + const extDir = createTempDir("hunk-ext-dialog-document-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.document({ title: "Agent setup", body: "Teach your agent how to review this Hunk session.", copy: { label: "Prompt", text: "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 document 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(), "c Copy"); + 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 document handler to finish", + ); + }, + undefined, + { width: 50, height: 12 }, + ); + }); + 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-"); diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index e7c707496..74943d443 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"; 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..3b688bb90 100644 --- a/src/ui/components/chrome/ExtensionDialog.tsx +++ b/src/ui/components/chrome/ExtensionDialog.tsx @@ -1,6 +1,8 @@ import type { MouseEvent as TuiMouseEvent } from "@opentui/core"; import type { ExtensionDialogRequest, + ExtensionDocumentCopyRequest, + ExtensionDocumentDialogRequest, ExtensionInputDialogRequest, ExtensionSelectDialogRequest, } from "../../lib/extensionDialogs"; @@ -36,11 +38,22 @@ function attributionText(extensionId: string, width: number) { return fitText(`${extensionToastPrefix()} ${extensionId}`, width); } +/** Preserve meaningful text when a constrained document has only one row. */ +function windowDocumentText(sourceLines: readonly string[], width: number, maxRows: number) { + const windowed = windowDialogText(sourceLines, width, maxRows); + if (maxRows !== 1 || !windowed.truncated) return windowed; + + const firstLine = windowDialogText(sourceLines, width, Number.MAX_SAFE_INTEGER).lines[0] ?? ""; + return { lines: [fitText(`${firstLine}…`, width, "…")], truncated: true }; +} + export function ExtensionDialog({ + copySupported, inputValue, onAccept, onCancel, onChangeInput, + onCopyDocument, onPickOption, request, selectedIndex, @@ -48,11 +61,13 @@ export function ExtensionDialog({ 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; + onCopyDocument: (copy: ExtensionDocumentCopyRequest) => void; /** Highlight one option row without accepting it, mirroring the theme selector. */ onPickOption: (index: number) => void; request: ExtensionDialogRequest; @@ -61,6 +76,20 @@ export function ExtensionDialog({ terminalWidth: number; theme: AppTheme; }) { + if (request.kind === "document") { + return ( + + ); + } + if (request.kind === "select") { return ( void; + onCopyDocument: (copy: ExtensionDocumentCopyRequest) => void; + request: ExtensionDocumentDialogRequest; + terminalHeight: number; + terminalWidth: number; + theme: AppTheme; +}) { + const width = Math.min(84, Math.max(40, terminalWidth - 8)); + const measuredFrame = resolveModalGeometry({ + width, + height: Number.MAX_SAFE_INTEGER, + terminalWidth, + terminalHeight, + }); + const bodyWidth = Math.max(1, measuredFrame.width - 4); + const cardTextWidth = Math.max(1, bodyWidth - 4); + const idealBodyRows = windowDialogText(request.bodyLines, bodyWidth, Number.MAX_SAFE_INTEGER) + .lines.length; + const copy = request.copy; + const idealCopyRows = copy + ? windowDialogText(copy.displayLines, cardTextWidth, Number.MAX_SAFE_INTEGER).lines.length + : 0; + const hasBody = idealBodyRows > 0; + const hasCopy = copy !== null; + const idealContentRows = + (request.showAttribution ? 2 : 0) + + idealBodyRows + + (hasBody && hasCopy ? 1 : 0) + + (hasCopy ? 1 + idealCopyRows + 2 : 0) + + 2; + const frame = resolveModalGeometry({ + width, + height: idealContentRows + MODAL_FRAME_CHROME_ROWS, + terminalWidth, + terminalHeight, + }); + const contentRows = Math.max(0, frame.height - MODAL_FRAME_CHROME_ROWS); + const actionRows = contentRows > 0 ? 1 : 0; + const minimumCopyRows = hasCopy ? 2 : 0; + const minimumContentRows = + (request.showAttribution ? 1 : 0) + (hasBody ? 1 : 0) + minimumCopyRows + actionRows; + const actionGapRows = contentRows > minimumContentRows ? 1 : 0; + let remainingRows = Math.max(0, contentRows - actionRows - actionGapRows); + const attributionRows = request.showAttribution && remainingRows > 0 ? 1 : 0; + remainingRows -= attributionRows; + const minimumVisibleDocumentRows = (hasBody ? 1 : 0) + minimumCopyRows; + const attributionGapRows = + attributionRows > 0 && remainingRows > minimumVisibleDocumentRows ? 1 : 0; + remainingRows -= attributionGapRows; + const copyReserve = hasCopy ? Math.min(minimumCopyRows, remainingRows) : 0; + const bodyCopyGapReserve = hasBody && hasCopy && remainingRows > copyReserve + 1 ? 1 : 0; + const bodyRows = Math.min( + idealBodyRows, + Math.max(0, remainingRows - copyReserve - bodyCopyGapReserve), + ); + remainingRows -= bodyRows; + const bodyCopyGapRows = bodyRows > 0 && hasCopy && remainingRows > 3 ? 1 : 0; + remainingRows -= bodyCopyGapRows; + const copyLabelRows = hasCopy && remainingRows > 1 ? 1 : 0; + remainingRows -= copyLabelRows; + const copyCardRows = hasCopy ? remainingRows : 0; + const visibleBody = windowDocumentText(request.bodyLines, bodyWidth, bodyRows); + const visibleCopy = copy + ? windowDocumentText( + copy.displayLines, + cardTextWidth, + copyCardRows >= 3 ? copyCardRows - 2 : copyCardRows, + ) + : { lines: [], truncated: false }; + + return ( + + {attributionRows > 0 ? ( + + {attributionText(request.extensionId, bodyWidth)} + + ) : null} + {attributionGapRows > 0 ? : null} + {visibleBody.lines.map((line, index) => ( + + {fitText(line, bodyWidth)} + + ))} + {bodyCopyGapRows > 0 ? : null} + {copy && copyLabelRows > 0 ? ( + + {fitText(copy.label, bodyWidth - 1)} + + ) : null} + {copy && copyCardRows > 0 ? ( + = 3 + ? { + border: true, + borderColor: theme.border, + paddingLeft: 1, + paddingRight: 1, + } + : {}), + }} + > + {visibleCopy.lines.map((line, index) => ( + + {fitText(line, cardTextWidth)} + + ))} + + ) : null} + {actionGapRows > 0 ? : null} + {actionRows > 0 ? ( + onCopyDocument(copy), + }, + ] + : [{ keyLabel: "esc", label: "close", run: onCancel }] + } + theme={theme} + /> + ) : null} + + ); +} + /** Render a select dialog as a keyboard- and mouse-driven option list. */ function ExtensionSelectDialog({ onAccept, diff --git a/src/ui/hooks/useAppKeyboardShortcuts.ts b/src/ui/hooks/useAppKeyboardShortcuts.ts index ac288555c..f0c078338 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; @@ -40,6 +39,7 @@ export interface UseAppKeyboardShortcutsOptions { extensionDialog: ExtensionDialogRequest | null; acceptExtensionDialog: () => void; cancelExtensionDialog: () => void; + copyExtensionDialogDocument: () => void; moveExtensionDialogSelection: (delta: number) => void; extensionTrustPromptOpen: boolean; trustRepoExtensions: () => void; @@ -69,7 +69,6 @@ export interface UseAppKeyboardShortcutsOptions { neverAskToSaveViewPreferencesAndQuit: () => void; closeSaveConfigPrompt: () => void; saveDraftNote: () => void; - showAgentSkill: boolean; showHelp: boolean; switchMenu: (delta: number) => void; toggleFocusArea: () => void; @@ -98,7 +97,6 @@ export interface UseAppKeyboardShortcutsOptions { export function useAppKeyboardShortcuts({ activeMenuId, activateCurrentMenuItem, - closeAgentSkill, closeHelp, closeMenu, acceptThemeSelector, @@ -110,6 +108,7 @@ export function useAppKeyboardShortcuts({ extensionDialog, acceptExtensionDialog, cancelExtensionDialog, + copyExtensionDialogDocument, moveExtensionDialogSelection, extensionTrustPromptOpen, trustRepoExtensions, @@ -129,7 +128,6 @@ export function useAppKeyboardShortcuts({ neverAskToSaveViewPreferencesAndQuit, closeSaveConfigPrompt, saveDraftNote, - showAgentSkill, showHelp, switchMenu, toggleFocusArea, @@ -138,7 +136,6 @@ 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); @@ -152,16 +149,16 @@ 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); + const copyExtensionDialogDocumentRef = useRef(copyExtensionDialogDocument); const moveExtensionDialogSelectionRef = useRef(moveExtensionDialogSelection); activeMenuIdRef.current = activeMenuId; commandsRef.current = commands; focusAreaRef.current = focusArea; - showAgentSkillRef.current = showAgentSkill; showHelpRef.current = showHelp; saveConfigPromptOpenRef.current = saveConfigPromptOpen; themeSelectorOpenRef.current = themeSelectorOpen; @@ -175,6 +172,7 @@ export function useAppKeyboardShortcuts({ sendKeyboardModeKeyRef.current = sendKeyboardModeKey; acceptExtensionDialogRef.current = acceptExtensionDialog; cancelExtensionDialogRef.current = cancelExtensionDialog; + copyExtensionDialogDocumentRef.current = copyExtensionDialogDocument; moveExtensionDialogSelectionRef.current = moveExtensionDialogSelection; /** @@ -229,17 +227,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"; @@ -338,7 +331,14 @@ export function useAppKeyboardShortcuts({ } if (key.name === "return" || key.name === "enter") { - acceptExtensionDialogRef.current(); + if (dialog.kind !== "document") { + acceptExtensionDialogRef.current(); + } + return "mine"; + } + + if (dialog.kind === "document" && dialog.copy && (key.name === "c" || key.sequence === "c")) { + copyExtensionDialogDocumentRef.current(); return "mine"; } diff --git a/src/ui/lib/extensionDialogs.test.ts b/src/ui/lib/extensionDialogs.test.ts index 2b5b4ab8c..40036e8e8 100644 --- a/src/ui/lib/extensionDialogs.test.ts +++ b/src/ui/lib/extensionDialogs.test.ts @@ -50,6 +50,12 @@ describe("createExtensionDialogQueue", () => { const valueless = dialogs.select({ title: "Which?", options: ["a"] }); queue.accept(queue.current()!.id); expect(await valueless).toBeNull(); + + const document = dialogs.document({ title: "Guide", body: "Read this." }); + queue.accept(queue.current()!.id); + expect(queue.current()).toMatchObject({ kind: "document", title: "Guide" }); + queue.cancel(queue.current()!.id); + expect(await document).toBeUndefined(); }); test("ignores an answer aimed at a dialog that is no longer current", async () => { @@ -115,6 +121,27 @@ describe("createExtensionDialogQueue", () => { expect(queue.current()).toMatchObject({ title: "Pick", options: ["opt"] }); }); + test("uses the same terminal-safe text for document display and clipboard payloads", () => { + const queue = createExtensionDialogQueue(); + const dialogs = queue.createDialogs("guide"); + + void dialogs.document({ + title: "Setup", + body: "one\n\u001b[31mtwo\u001b[0m", + copy: { label: "Prompt", text: "copy \u001b[31mexactly\u001b[0m" }, + }); + + expect(queue.current()).toMatchObject({ + kind: "document", + bodyLines: ["one", "two"], + copy: { + label: "Prompt", + text: "copy exactly", + displayLines: ["copy exactly"], + }, + }); + }); + test("sanitizes an input dialog's starting text without trimming it", () => { const queue = createExtensionDialogQueue(); const dialogs = queue.createDialogs("hostile"); @@ -221,6 +248,10 @@ describe("createExtensionDialogQueue", () => { await expect(dialogs.select({ title: "Which?", options: [] })).rejects.toThrow( /at least one option/, ); + await expect(dialogs.document({ title: "Empty" })).rejects.toThrow(/body or copy content/); + await expect(dialogs.document({ title: "Bad copy", copy: { text: "" } })).rejects.toThrow( + /non-empty string/, + ); 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..f71234e5e 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 @@ -12,10 +12,11 @@ import type { ExtensionConfirmOptions, ExtensionDialogs, + ExtensionDocumentOptions, 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,15 @@ 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; +/** Body lines one read-only document may retain before host-side windowing. */ +const MAX_DOCUMENT_BODY_LINES = 100; + +/** Clipboard text is bounded before it reaches the terminal's OSC 52 channel. */ +const MAX_DOCUMENT_COPY_TEXT_LENGTH = 16_384; + +/** Default heading for a document's copyable text card. */ +const DEFAULT_DOCUMENT_COPY_LABEL = "Content"; + /** What every queued dialog carries, whatever kind it is. */ interface ExtensionDialogRequestBase { /** @@ -61,14 +71,29 @@ export interface ExtensionInputDialogRequest extends ExtensionDialogRequestBase initial: string; } +/** Clipboard and display forms of one document's normalized copyable text. */ +export interface ExtensionDocumentCopyRequest { + label: string; + text: string; + displayLines: string[]; +} + +/** One normalized read-only document the host should draw. */ +export interface ExtensionDocumentDialogRequest extends ExtensionDialogRequestBase { + kind: "document"; + bodyLines: string[]; + copy: ExtensionDocumentCopyRequest | null; +} + /** One dialog the host should draw, normalized from what an extension asked for. */ export type ExtensionDialogRequest = | ExtensionConfirmDialogRequest | ExtensionSelectDialogRequest - | ExtensionInputDialogRequest; + | ExtensionInputDialogRequest + | ExtensionDocumentDialogRequest; /** 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 { @@ -83,7 +108,8 @@ export interface ExtensionDialogQueue { * 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. Documents + * 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 @@ -143,17 +169,41 @@ 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)); } +/** Validate and normalize a document's optional clipboard card. */ +function normalizeDocumentCopy( + copy: ExtensionDocumentOptions["copy"], +): ExtensionDocumentCopyRequest | null { + if (copy === undefined) return null; + if (!copy || typeof copy.text !== "string" || copy.text.length === 0) { + invalid("document", "copy.text must be a non-empty string."); + } + if (copy.text.length > MAX_DOCUMENT_COPY_TEXT_LENGTH) { + invalid("document", `copy.text must be at most ${MAX_DOCUMENT_COPY_TEXT_LENGTH} characters.`); + } + + const text = sanitizeTerminalText(copy.text); + if (text.length === 0) { + invalid("document", "copy.text must contain visible or whitespace content."); + } + + return { + label: normalizeLabel(copy.label, DEFAULT_DOCUMENT_COPY_LABEL), + text, + displayLines: text.split("\n").map((line) => sanitizeTerminalLine(line)), + }; +} + /** Normalize the choices of a select dialog, or reject them. */ function normalizeOptions(options: unknown) { if (!Array.isArray(options) || options.length === 0) { @@ -194,9 +244,9 @@ 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 === "document" ? undefined : null; /** * Queue one request and hand back the promise its handler awaits. @@ -310,6 +360,27 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { isLive, ); }, + async document(options: ExtensionDocumentOptions) { + const title = normalizeTitle("document", options?.title); + const bodyLines = normalizeBodyLines(options.body, MAX_DOCUMENT_BODY_LINES); + const copy = normalizeDocumentCopy(options.copy); + if (bodyLines.length === 0 && copy === null) { + invalid("document", "requires body or copy content."); + } + await enqueue( + (id) => ({ + kind: "document", + id, + extensionId, + showAttribution, + title, + bodyLines, + copy, + }), + undefined, + isLive, + ); + }, }; }, @@ -333,6 +404,10 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { return; } + if (active.request.kind === "document") { + return; + } + settleCurrent(value ?? null); }, diff --git a/test/pty/chrome.test.ts b/test/pty/chrome.test.ts index 0eea9d935..b6c5fd9dd 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 modal document", 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 document = await harness.waitForSnapshot( + session, + (text) => + text.includes("Teach your agent how to review this Hunk session.") && + text.includes("hunk skill path") && + text.includes("c Copy"), + 5_000, + ); + expect(document).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 1d17e1427..a9b3dc80f 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 host-mediated editor +The API generation this Hunk speaks (currently `17`). Branch on it if you want +one file to support several Hunk versions. Version 17 adds read-only document +dialogs with optional clipboard actions; version 16 added host-mediated editor launches for reviewed files; version 15 added `{ side, line }` to opted-in pane `currentLine` paint; version 14 added structured two-revision VCS diff endpoints; version 13 added saved-note parent identities @@ -283,11 +284,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` +- `document({ title, body?, copy?: { label?, text } })` → `void` when closed ```ts hunk.registerCommand( @@ -309,6 +311,8 @@ hunk.registerCommand( ); ``` +`document` presents read-only guidance rather than asking for an answer. At least `body` or `copy` must be present. When `copy` is provided, `c` and the clickable copy action send its `text` to the terminal clipboard after Hunk removes terminal control sequences, and Hunk renders the same safe value under `label` (default `Content`). + `select` fits acting on part of the selection — asking which hunk to jump to, then navigating there: ```ts @@ -332,7 +336,7 @@ 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. -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 document. Enter accepts interactive dialogs but leaves read-only documents open; 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. ### Workspace documents From c206852eee2dd7624d08daecb4a54cd0ec7ff7ff Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Sun, 30 Aug 2026 23:47:05 -0400 Subject: [PATCH 2/9] fix(extensions): harden document dialog behavior --- docs/extension-architecture.md | 3 +- docs/extensions.md | 10 ++-- src/extension-api/types.ts | 12 +++-- src/ui/App.tsx | 11 ++-- src/ui/AppHost.extension-dialogs.test.tsx | 51 +++++++++++++++++++ src/ui/components/chrome/ExtensionDialog.tsx | 15 ++++-- src/ui/lib/extensionDialogGeometry.test.ts | 18 ++++++- src/ui/lib/extensionDialogGeometry.ts | 39 +++++++++++++- src/ui/lib/extensionDialogs.test.ts | 15 ++++-- src/ui/lib/extensionDialogs.ts | 20 +++++++- .../content/docs/docs/extend/extension-api.md | 8 ++- 11 files changed, 177 insertions(+), 25 deletions(-) diff --git a/docs/extension-architecture.md b/docs/extension-architecture.md index a338927db..a1ae8a047 100644 --- a/docs/extension-architecture.md +++ b/docs/extension-architecture.md @@ -241,7 +241,8 @@ text into a request the host draws, 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, input, and read-only copyable documents are `ModalFrame` surfaces), and unmount calls `shutdown()` so +select, input, and read-only copyable documents are `ModalFrame` surfaces), 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 diff --git a/docs/extensions.md b/docs/extensions.md index ff936bd60..cd6e3f43a 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -1661,10 +1661,12 @@ hunk.registerCommand({ id: "pick-hunk", title: "Pick a hunk", key: "ctrl+k" }, a ``` `document` presents read-only guidance rather than asking for an answer. At -least `body` or `copy` must be present. When `copy` is provided, `c` and the -clickable copy action send its `text` to the terminal clipboard while the host -removes terminal control sequences and renders the same safe value under -`label` (default `Content`): +least `body` or `copy` must be present. A body may contain up to 100 source +lines, and copy text may contain up to 16,384 JavaScript string code units. +When `copy` is provided, `c` and the clickable copy action send its `text` to +the terminal clipboard while the host removes terminal control sequences, +expands tabs to four spaces, and renders the same safe value under `label` +(default `Content`): ```ts hunk.registerCommand({ id: "agent-setup", title: "Agent setup" }, async (ctx) => { diff --git a/src/extension-api/types.ts b/src/extension-api/types.ts index 7d9cf191c..7f1d3d7db 100644 --- a/src/extension-api/types.ts +++ b/src/extension-api/types.ts @@ -1625,14 +1625,17 @@ export interface ExtensionInputOptions { export interface ExtensionDocumentCopyOptions { /** Short heading shown above the copyable text. Defaults to "Content". */ label?: string; - /** Text copied to the terminal clipboard after Hunk removes terminal control sequences. */ + /** + * Text copied after Hunk removes terminal control sequences and expands tabs + * to four spaces. Limited to 16,384 JavaScript string code units. + */ text: string; } /** Read-only guidance shown to the user as a modal document. */ export interface ExtensionDocumentOptions { title: string; - /** Optional prose shown above the copyable text. */ + /** Optional prose shown above the copyable text. Limited to 100 source lines. */ body?: string; /** Optional text card the user can copy with `c` or the mouse. */ copy?: ExtensionDocumentCopyOptions; @@ -1658,8 +1661,9 @@ export interface ExtensionDocumentOptions { * 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 document content outside its documented bounds. 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. */ diff --git a/src/ui/App.tsx b/src/ui/App.tsx index cfb15323f..b68acd75f 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -204,8 +204,13 @@ export function App({ const layoutToggleScrollTopRef = useRef(null); const cancelCopySelectionRef = useRef<(() => void) | null>(null); const activeReviewGenerationRef = useRef(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<{ @@ -505,7 +510,9 @@ export function App({ // 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: bundled ? () => activeReviewGenerationRef.current === bootstrap : lease.isLive, + isLive: bundled + ? () => bundledDialogsLiveRef.current && activeReviewGenerationRef.current === bootstrap + : lease.isLive, showAttribution: !bundled, }); }, @@ -931,8 +938,6 @@ export function App({ showTransientNotice(`Copied ${extensionDialog.copy.label.toLowerCase()} to clipboard`); return; } - - showTransientNotice("Clipboard copy unsupported in this terminal (enable OSC 52)"); }, [extensionDialog, renderer, showTransientNotice]); /** Toggle the modal keyboard help overlay. */ diff --git a/src/ui/AppHost.extension-dialogs.test.tsx b/src/ui/AppHost.extension-dialogs.test.tsx index 4c46e0975..4dd274f0a 100644 --- a/src/ui/AppHost.extension-dialogs.test.tsx +++ b/src/ui/AppHost.extension-dialogs.test.tsx @@ -343,6 +343,57 @@ describe("extension dialogs", () => { ); }); + test("an unavailable document copy action is visible but inert", async () => { + const repo = createTestRepo("hunk-ext-dialog-document-unavailable-"); + const extDir = createTempDir("hunk-ext-dialog-document-unavailable-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.document({ title: "Copy setup", copy: { label: "Prompt", text: "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 document to 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-"); diff --git a/src/ui/components/chrome/ExtensionDialog.tsx b/src/ui/components/chrome/ExtensionDialog.tsx index 3b688bb90..f7dc6c0f7 100644 --- a/src/ui/components/chrome/ExtensionDialog.tsx +++ b/src/ui/components/chrome/ExtensionDialog.tsx @@ -7,7 +7,7 @@ import type { ExtensionSelectDialogRequest, } from "../../lib/extensionDialogs"; import { extensionToastPrefix } from "../../lib/extensionNotifications"; -import { windowDialogText } from "../../lib/extensionDialogGeometry"; +import { windowDialogLiteralText, windowDialogText } from "../../lib/extensionDialogGeometry"; import { listWindowStart } from "../../lib/listWindow"; import { MODAL_FRAME_CHROME_ROWS, resolveModalGeometry } from "../../lib/modalGeometry"; import { fitText, padText } from "../../lib/text"; @@ -196,7 +196,8 @@ function ExtensionDocumentDialog({ .lines.length; const copy = request.copy; const idealCopyRows = copy - ? windowDialogText(copy.displayLines, cardTextWidth, Number.MAX_SAFE_INTEGER).lines.length + ? windowDialogLiteralText(copy.displayLines, cardTextWidth, Number.MAX_SAFE_INTEGER).lines + .length : 0; const hasBody = idealBodyRows > 0; const hasCopy = copy !== null; @@ -239,7 +240,7 @@ function ExtensionDocumentDialog({ const copyCardRows = hasCopy ? remainingRows : 0; const visibleBody = windowDocumentText(request.bodyLines, bodyWidth, bodyRows); const visibleCopy = copy - ? windowDocumentText( + ? windowDialogLiteralText( copy.displayLines, cardTextWidth, copyCardRows >= 3 ? copyCardRows - 2 : copyCardRows, @@ -297,14 +298,18 @@ function ExtensionDocumentDialog({ ) : null} {actionGapRows > 0 ? : null} - {actionRows > 0 ? ( + {actionRows > 0 && copy && !copySupported ? ( + + Copy unavailable + + ) : actionRows > 0 ? ( onCopyDocument(copy), }, ] diff --git a/src/ui/lib/extensionDialogGeometry.test.ts b/src/ui/lib/extensionDialogGeometry.test.ts index c7ed4ba19..467e93391 100644 --- a/src/ui/lib/extensionDialogGeometry.test.ts +++ b/src/ui/lib/extensionDialogGeometry.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { windowDialogText } from "./extensionDialogGeometry"; +import { windowDialogLiteralText, windowDialogText } from "./extensionDialogGeometry"; describe("windowDialogText", () => { test("wraps prose within the available terminal-cell rows", () => { @@ -17,3 +17,19 @@ describe("windowDialogText", () => { expect(windowDialogText(["overflow"], 3, 0)).toEqual({ lines: [], truncated: true }); }); }); + +describe("windowDialogLiteralText", () => { + test("wraps copyable text without collapsing whitespace", () => { + expect(windowDialogLiteralText([" one two", " three"], 7, 4)).toEqual({ + lines: [" one ", "two", " thr", "ee"], + truncated: false, + }); + }); + + test("keeps meaningful literal text in a one-row window", () => { + expect(windowDialogLiteralText(["one two"], 5, 1)).toEqual({ + lines: ["one …"], + truncated: true, + }); + }); +}); diff --git a/src/ui/lib/extensionDialogGeometry.ts b/src/ui/lib/extensionDialogGeometry.ts index 795b8c66c..95d9b4b06 100644 --- a/src/ui/lib/extensionDialogGeometry.ts +++ b/src/ui/lib/extensionDialogGeometry.ts @@ -1,4 +1,4 @@ -import { wrapText } from "./text"; +import { fitText, measureTextWidth, sliceTextByWidth, wrapText } from "./text"; /** Wrapped body rows that fit one modal body allocation. */ export interface WindowedDialogText { @@ -25,3 +25,40 @@ export function windowDialogText( truncated: true, }; } + +/** Wrap copyable text by terminal cells without normalizing its whitespace. */ +export function windowDialogLiteralText( + sourceLines: readonly string[], + width: number, + maxRows: number, +): WindowedDialogText { + const safeWidth = Math.max(1, width); + const wrapped = sourceLines.flatMap((line) => { + const lineWidth = measureTextWidth(line); + if (lineWidth === 0) return [line]; + + const lines: string[] = []; + for (let offset = 0; offset < lineWidth; ) { + const chunk = sliceTextByWidth(line, offset, safeWidth); + if (chunk.width > 0) { + lines.push(chunk.text); + offset += chunk.width; + continue; + } + + // A cluster wider than the whole viewport cannot render intact. Show an + // overflow marker and advance past that cluster instead of looping. + const wideChunk = sliceTextByWidth(line, offset, safeWidth + 1); + lines.push(fitText(wideChunk.text, safeWidth, "…")); + offset += Math.max(1, wideChunk.width); + } + return lines; + }); + + if (wrapped.length <= maxRows) return { lines: wrapped, truncated: false }; + if (maxRows <= 0) return { lines: [], truncated: wrapped.length > 0 }; + if (maxRows === 1) { + return { lines: [fitText(`${wrapped[0] ?? ""}…`, safeWidth, "…")], truncated: true }; + } + return { lines: [...wrapped.slice(0, maxRows - 1), "…"], truncated: true }; +} diff --git a/src/ui/lib/extensionDialogs.test.ts b/src/ui/lib/extensionDialogs.test.ts index 40036e8e8..c6cf76724 100644 --- a/src/ui/lib/extensionDialogs.test.ts +++ b/src/ui/lib/extensionDialogs.test.ts @@ -128,7 +128,7 @@ describe("createExtensionDialogQueue", () => { void dialogs.document({ title: "Setup", body: "one\n\u001b[31mtwo\u001b[0m", - copy: { label: "Prompt", text: "copy \u001b[31mexactly\u001b[0m" }, + copy: { label: "Prompt", text: "copy\t\u001b[31mexactly\u001b[0m" }, }); expect(queue.current()).toMatchObject({ @@ -136,8 +136,8 @@ describe("createExtensionDialogQueue", () => { bodyLines: ["one", "two"], copy: { label: "Prompt", - text: "copy exactly", - displayLines: ["copy exactly"], + text: "copy exactly", + displayLines: ["copy exactly"], }, }); }); @@ -252,6 +252,15 @@ describe("createExtensionDialogQueue", () => { await expect(dialogs.document({ title: "Bad copy", copy: { text: "" } })).rejects.toThrow( /non-empty string/, ); + await expect( + dialogs.document({ title: "Long body", body: Array(101).fill("line").join("\n") }), + ).rejects.toThrow(/at most 100 lines/); + await expect( + dialogs.document({ title: "Long copy", copy: { text: "x".repeat(16_385) } }), + ).rejects.toThrow(/at most 16384 characters/); + await expect( + dialogs.document({ title: "Expanded copy", copy: { text: "\t".repeat(4_097) } }), + ).rejects.toThrow(/normalized copy.text.*at most 16384 characters/); 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 f71234e5e..b86e448c3 100644 --- a/src/ui/lib/extensionDialogs.ts +++ b/src/ui/lib/extensionDialogs.ts @@ -180,6 +180,16 @@ function normalizeBodyLines(body: unknown, maxLines = MAX_CONFIRM_BODY_LINES) { .map((line) => sanitizeTerminalLine(line)); } +/** Normalize a document body while rejecting content the host would have to discard. */ +function normalizeDocumentBodyLines(body: unknown) { + if (typeof body !== "string" || body.length === 0) return []; + const lines = body.split("\n"); + if (lines.length > MAX_DOCUMENT_BODY_LINES) { + invalid("document", `body must contain at most ${MAX_DOCUMENT_BODY_LINES} lines.`); + } + return lines.map((line) => sanitizeTerminalLine(line)); +} + /** Validate and normalize a document's optional clipboard card. */ function normalizeDocumentCopy( copy: ExtensionDocumentOptions["copy"], @@ -192,7 +202,13 @@ function normalizeDocumentCopy( invalid("document", `copy.text must be at most ${MAX_DOCUMENT_COPY_TEXT_LENGTH} characters.`); } - const text = sanitizeTerminalText(copy.text); + const text = sanitizeTerminalText(copy.text).replaceAll("\t", " "); + if (text.length > MAX_DOCUMENT_COPY_TEXT_LENGTH) { + invalid( + "document", + `normalized copy.text must be at most ${MAX_DOCUMENT_COPY_TEXT_LENGTH} characters.`, + ); + } if (text.length === 0) { invalid("document", "copy.text must contain visible or whitespace content."); } @@ -362,7 +378,7 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { }, async document(options: ExtensionDocumentOptions) { const title = normalizeTitle("document", options?.title); - const bodyLines = normalizeBodyLines(options.body, MAX_DOCUMENT_BODY_LINES); + const bodyLines = normalizeDocumentBodyLines(options.body); const copy = normalizeDocumentCopy(options.copy); if (bodyLines.length === 0 && copy === null) { invalid("document", "requires body or copy content."); diff --git a/website/src/content/docs/docs/extend/extension-api.md b/website/src/content/docs/docs/extend/extension-api.md index a9b3dc80f..e8f741670 100644 --- a/website/src/content/docs/docs/extend/extension-api.md +++ b/website/src/content/docs/docs/extend/extension-api.md @@ -311,7 +311,13 @@ hunk.registerCommand( ); ``` -`document` presents read-only guidance rather than asking for an answer. At least `body` or `copy` must be present. When `copy` is provided, `c` and the clickable copy action send its `text` to the terminal clipboard after Hunk removes terminal control sequences, and Hunk renders the same safe value under `label` (default `Content`). +`document` presents read-only guidance rather than asking for an answer. At +least `body` or `copy` must be present. A body may contain up to 100 source +lines, and copy text may contain up to 16,384 JavaScript string code units. +When `copy` is provided, `c` and the clickable copy action send its `text` to +the terminal clipboard after Hunk removes terminal control sequences and +expands tabs to four spaces, and Hunk renders the same safe value under `label` +(default `Content`). `select` fits acting on part of the selection — asking which hunk to jump to, then navigating there: From c2a35a1e4dbfca8461df1cd7b7375106f6e45c07 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 31 Aug 2026 00:19:41 -0400 Subject: [PATCH 3/9] fix(extensions): preserve agent skill dialog presentation --- src/extensions/default/ui/agentSkill/index.ts | 6 +- src/ui/AppHost.extension-dialogs.test.tsx | 2 +- src/ui/components/chrome/ExtensionDialog.tsx | 158 +++++++++++------- test/pty/chrome.test.ts | 2 +- 4 files changed, 101 insertions(+), 67 deletions(-) diff --git a/src/extensions/default/ui/agentSkill/index.ts b/src/extensions/default/ui/agentSkill/index.ts index 7fa36a48e..4b7b0a381 100644 --- a/src/extensions/default/ui/agentSkill/index.ts +++ b/src/extensions/default/ui/agentSkill/index.ts @@ -1,8 +1,10 @@ import type { ExtensionFactory } from "hunkdiff/extension"; export const AGENT_SKILL_COMMAND = "hunk skill path"; -export const AGENT_SKILL_PROMPT = - "Load the Hunk skill and use it for this review. Run `hunk skill path` to get the skill path."; +export const AGENT_SKILL_PROMPT = [ + "Load the Hunk skill and use it for this review.", + "Run `hunk skill path` to get the skill path.", +].join("\n"); export const BUNDLED_AGENT_SKILL_COMMAND_ID = "app.openAgentSkill"; export const BUNDLED_AGENT_SKILL_COMMAND_FULL_ID = `hunk.${BUNDLED_AGENT_SKILL_COMMAND_ID}`; diff --git a/src/ui/AppHost.extension-dialogs.test.tsx b/src/ui/AppHost.extension-dialogs.test.tsx index 4dd274f0a..cb6857b2a 100644 --- a/src/ui/AppHost.extension-dialogs.test.tsx +++ b/src/ui/AppHost.extension-dialogs.test.tsx @@ -322,7 +322,7 @@ describe("extension dialogs", () => { "Load the Hunk skill and use it for this review. Run hunk skill path to get the skill path.", ]); - const copyAction = findTextPosition(setup.captureCharFrame(), "c Copy"); + const copyAction = findTextPosition(setup.captureCharFrame(), "Copy prompt"); expect(copyAction).not.toBeNull(); await act(async () => { await setup.mockMouse.click(copyAction!.x, copyAction!.y); diff --git a/src/ui/components/chrome/ExtensionDialog.tsx b/src/ui/components/chrome/ExtensionDialog.tsx index f7dc6c0f7..4ecec5ff1 100644 --- a/src/ui/components/chrome/ExtensionDialog.tsx +++ b/src/ui/components/chrome/ExtensionDialog.tsx @@ -191,7 +191,8 @@ function ExtensionDocumentDialog({ terminalHeight, }); const bodyWidth = Math.max(1, measuredFrame.width - 4); - const cardTextWidth = Math.max(1, bodyWidth - 4); + const cardWidth = Math.max(1, bodyWidth - 4); + const cardTextWidth = Math.max(1, cardWidth - 4); const idealBodyRows = windowDialogText(request.bodyLines, bodyWidth, Number.MAX_SAFE_INTEGER) .lines.length; const copy = request.copy; @@ -209,11 +210,13 @@ function ExtensionDocumentDialog({ 2; const frame = resolveModalGeometry({ width, - height: idealContentRows + MODAL_FRAME_CHROME_ROWS, + // The inner flex column lets the final action use the last + // chrome-adjacent row without adding an empty footer row. + height: idealContentRows + MODAL_FRAME_CHROME_ROWS - 1, terminalWidth, terminalHeight, }); - const contentRows = Math.max(0, frame.height - MODAL_FRAME_CHROME_ROWS); + const contentRows = Math.max(0, frame.height - MODAL_FRAME_CHROME_ROWS + 1); const actionRows = contentRows > 0 ? 1 : 0; const minimumCopyRows = hasCopy ? 2 : 0; const minimumContentRows = @@ -257,71 +260,100 @@ function ExtensionDocumentDialog({ width={frame.width} onClose={onCancel} > - {attributionRows > 0 ? ( - - {attributionText(request.extensionId, bodyWidth)} - - ) : null} - {attributionGapRows > 0 ? : null} - {visibleBody.lines.map((line, index) => ( - - {fitText(line, bodyWidth)} - - ))} - {bodyCopyGapRows > 0 ? : null} - {copy && copyLabelRows > 0 ? ( - - {fitText(copy.label, bodyWidth - 1)} - - ) : null} - {copy && copyCardRows > 0 ? ( - = 3 - ? { - border: true, - borderColor: theme.border, - paddingLeft: 1, - paddingRight: 1, - } - : {}), - }} - > - {visibleCopy.lines.map((line, index) => ( - - {fitText(line, cardTextWidth)} + + {attributionRows > 0 ? ( + + {attributionText(request.extensionId, bodyWidth)} + + ) : null} + {attributionGapRows > 0 ? : null} + {visibleBody.lines.map((line, index) => ( + + {fitText(line, bodyWidth)} + + ))} + {bodyCopyGapRows > 0 ? : null} + {copy && copyLabelRows > 0 ? ( + + {fitText(copy.label, bodyWidth - 1)} + + ) : null} + {copy && copyCardRows > 0 ? ( + + = 3 + ? { + border: true, + borderColor: theme.border, + paddingLeft: 1, + paddingRight: 1, + } + : {}), + }} + > + {visibleCopy.lines.map((line, index) => ( + + {fitText(line, cardTextWidth)} + + ))} - ))} - - ) : null} - {actionGapRows > 0 ? : null} - {actionRows > 0 && copy && !copySupported ? ( - - Copy unavailable - - ) : actionRows > 0 ? ( - onCopyDocument(copy), - }, - ] - : [{ keyLabel: "esc", label: "close", run: onCancel }] - } - theme={theme} - /> - ) : null} + + ) : null} + {actionGapRows > 0 ? : null} + {actionRows > 0 && copy ? ( + + ) : actionRows > 0 ? ( + + ) : null} + ); } +/** Render the compact copy affordance beneath a document card. */ +function DocumentCopyAction({ + copy, + copySupported, + onCopyDocument, + theme, + width, +}: { + copy: ExtensionDocumentCopyRequest; + copySupported: boolean; + onCopyDocument: (copy: ExtensionDocumentCopyRequest) => void; + theme: AppTheme; + width: number; +}) { + const label = copySupported ? ` ⧉ Copy ${copy.label.toLowerCase()} ` : " Copy unavailable "; + return ( + + { + event.stopPropagation(); + if (copySupported) onCopyDocument(copy); + }} + > + {label} + + {padText("", Math.max(1, width - label.length))} + + ); +} + /** Render a select dialog as a keyboard- and mouse-driven option list. */ function ExtensionSelectDialog({ onAccept, diff --git a/test/pty/chrome.test.ts b/test/pty/chrome.test.ts index b6c5fd9dd..5f2b0b989 100644 --- a/test/pty/chrome.test.ts +++ b/test/pty/chrome.test.ts @@ -105,7 +105,7 @@ describe("PTY chrome", () => { (text) => text.includes("Teach your agent how to review this Hunk session.") && text.includes("hunk skill path") && - text.includes("c Copy"), + text.includes("⧉ Copy prompt"), 5_000, ); expect(document).not.toContain("ext hunk"); From 07cc9595806fb6970e4bf086677a6e5b5202519a Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 31 Aug 2026 00:25:20 -0400 Subject: [PATCH 4/9] fix(extensions): preserve agent prompt clipboard text --- docs/extensions.md | 10 +++++++-- src/extension-api/types.ts | 6 ++++++ .../default/ui/agentSkill/index.test.ts | 12 +++++++++-- src/extensions/default/ui/agentSkill/index.ts | 6 ++++-- src/ui/lib/extensionDialogs.test.ts | 21 +++++++++++++++++++ src/ui/lib/extensionDialogs.ts | 18 +++++++++++++++- 6 files changed, 66 insertions(+), 7 deletions(-) diff --git a/docs/extensions.md b/docs/extensions.md index cd6e3f43a..db4a2987c 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -1666,14 +1666,20 @@ lines, and copy text may contain up to 16,384 JavaScript string code units. When `copy` is provided, `c` and the clickable copy action send its `text` to the terminal clipboard while the host removes terminal control sequences, expands tabs to four spaces, and renders the same safe value under `label` -(default `Content`): +(default `Content`). Optional `displayLines` can add authored visual breaks; +after sanitizing, those lines must rejoin with spaces or newlines to exactly the +clipboard `text`, so a preview cannot disguise what the action copies: ```ts hunk.registerCommand({ id: "agent-setup", title: "Agent setup" }, async (ctx) => { await ctx.dialogs.document({ title: "Agent setup", body: "Give this prompt to your coding agent.", - copy: { label: "Prompt", text: "Review the current Hunk session." }, + copy: { + label: "Prompt", + text: "Review the current Hunk session. Focus on correctness.", + displayLines: ["Review the current Hunk session.", "Focus on correctness."], + }, }); }); ``` diff --git a/src/extension-api/types.ts b/src/extension-api/types.ts index 7f1d3d7db..d6fcc69ab 100644 --- a/src/extension-api/types.ts +++ b/src/extension-api/types.ts @@ -1630,6 +1630,12 @@ export interface ExtensionDocumentCopyOptions { * to four spaces. Limited to 16,384 JavaScript string code units. */ text: string; + /** + * Optional authored display rows for `text`. Their sanitized contents must + * rejoin with spaces or newlines to the sanitized clipboard text, so they + * can control wrapping without presenting different content. + */ + displayLines?: readonly string[]; } /** Read-only guidance shown to the user as a modal document. */ diff --git a/src/extensions/default/ui/agentSkill/index.test.ts b/src/extensions/default/ui/agentSkill/index.test.ts index d934188b3..558f18283 100644 --- a/src/extensions/default/ui/agentSkill/index.test.ts +++ b/src/extensions/default/ui/agentSkill/index.test.ts @@ -1,7 +1,11 @@ import { describe, expect, mock, test } from "bun:test"; import type { ExtensionCommandContext } from "hunkdiff/extension"; import { getBundledUIRegistry } from ".."; -import { AGENT_SKILL_PROMPT, BUNDLED_AGENT_SKILL_COMMAND_FULL_ID } from "."; +import { + AGENT_SKILL_PROMPT, + AGENT_SKILL_PROMPT_ROWS, + BUNDLED_AGENT_SKILL_COMMAND_FULL_ID, +} from "."; /** Return the agent-skill registration from the process-static bundled UI registry. */ function getBundledAgentSkillCommand() { @@ -33,7 +37,11 @@ describe("bundled agent skill extension", () => { expect(document).toHaveBeenCalledWith({ title: "Agent skill", body: "Teach your agent how to review this Hunk session.", - copy: { label: "Prompt", text: AGENT_SKILL_PROMPT }, + copy: { + label: "Prompt", + text: AGENT_SKILL_PROMPT, + displayLines: AGENT_SKILL_PROMPT_ROWS, + }, }); }); }); diff --git a/src/extensions/default/ui/agentSkill/index.ts b/src/extensions/default/ui/agentSkill/index.ts index 4b7b0a381..bddde318d 100644 --- a/src/extensions/default/ui/agentSkill/index.ts +++ b/src/extensions/default/ui/agentSkill/index.ts @@ -1,10 +1,11 @@ import type { ExtensionFactory } from "hunkdiff/extension"; export const AGENT_SKILL_COMMAND = "hunk skill path"; -export const AGENT_SKILL_PROMPT = [ +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.", -].join("\n"); +] 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}`; @@ -22,6 +23,7 @@ const registerBundledAgentSkill: ExtensionFactory = (hunk) => { copy: { label: "Prompt", text: AGENT_SKILL_PROMPT, + displayLines: AGENT_SKILL_PROMPT_ROWS, }, }); }, diff --git a/src/ui/lib/extensionDialogs.test.ts b/src/ui/lib/extensionDialogs.test.ts index c6cf76724..2b22c825d 100644 --- a/src/ui/lib/extensionDialogs.test.ts +++ b/src/ui/lib/extensionDialogs.test.ts @@ -142,6 +142,27 @@ describe("createExtensionDialogQueue", () => { }); }); + test("accepts authored display rows only when they preserve the clipboard text", async () => { + const queue = createExtensionDialogQueue(); + const dialogs = queue.createDialogs("guide"); + + void dialogs.document({ + title: "Setup", + copy: { text: "copy this exactly", displayLines: ["copy this", "exactly"] }, + }); + expect(queue.current()).toMatchObject({ + copy: { text: "copy this exactly", displayLines: ["copy this", "exactly"] }, + }); + queue.cancelAll(); + + await expect( + dialogs.document({ + title: "Setup", + copy: { text: "safe text", displayLines: ["different text"] }, + }), + ).rejects.toThrow("copy.displayLines must contain the same text as copy.text"); + }); + test("sanitizes an input dialog's starting text without trimming it", () => { const queue = createExtensionDialogQueue(); const dialogs = queue.createDialogs("hostile"); diff --git a/src/ui/lib/extensionDialogs.ts b/src/ui/lib/extensionDialogs.ts index b86e448c3..db8f09a7c 100644 --- a/src/ui/lib/extensionDialogs.ts +++ b/src/ui/lib/extensionDialogs.ts @@ -213,10 +213,26 @@ function normalizeDocumentCopy( invalid("document", "copy.text must contain visible or whitespace content."); } + let displayLines = text.split("\n").map((line) => sanitizeTerminalLine(line)); + if (copy.displayLines !== undefined) { + if (!Array.isArray(copy.displayLines) || copy.displayLines.length === 0) { + invalid("document", "copy.displayLines must be a non-empty string array."); + } + displayLines = copy.displayLines.map((line) => { + if (typeof line !== "string" || line.includes("\n")) { + invalid("document", "copy.displayLines must contain single-line strings."); + } + return sanitizeTerminalLine(line).replaceAll("\t", " "); + }); + if (displayLines.join(" ") !== text && displayLines.join("\n") !== text) { + invalid("document", "copy.displayLines must contain the same text as copy.text."); + } + } + return { label: normalizeLabel(copy.label, DEFAULT_DOCUMENT_COPY_LABEL), text, - displayLines: text.split("\n").map((line) => sanitizeTerminalLine(line)), + displayLines, }; } From 5f1fe64da7c42d2cdbdfd81f060d2385104e9527 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 31 Aug 2026 00:26:09 -0400 Subject: [PATCH 5/9] fix(ui): preserve document dialog narrow width --- src/ui/components/chrome/ExtensionDialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/components/chrome/ExtensionDialog.tsx b/src/ui/components/chrome/ExtensionDialog.tsx index 4ecec5ff1..d70f05f6d 100644 --- a/src/ui/components/chrome/ExtensionDialog.tsx +++ b/src/ui/components/chrome/ExtensionDialog.tsx @@ -183,7 +183,7 @@ function ExtensionDocumentDialog({ terminalWidth: number; theme: AppTheme; }) { - const width = Math.min(84, Math.max(40, terminalWidth - 8)); + const width = Math.min(84, Math.max(58, terminalWidth - 8)); const measuredFrame = resolveModalGeometry({ width, height: Number.MAX_SAFE_INTEGER, From 72b6e3f4a2121ba8a430a782f0e30850f6ad0f2e Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 31 Aug 2026 17:46:58 -0400 Subject: [PATCH 6/9] fix(ui): require full document copy disclosure --- .changeset/fuzzy-agents-guide.md | 2 +- docs/extensions.md | 4 +- src/ui/App.tsx | 40 ++++- src/ui/AppHost.extension-dialogs.test.tsx | 91 +++++++++++ src/ui/AppHost.interactions.test.tsx | 17 ++ src/ui/components/chrome/ExtensionDialog.tsx | 107 ++++--------- src/ui/hooks/useAppKeyboardShortcuts.ts | 14 +- src/ui/lib/extensionDialogGeometry.test.ts | 92 ++++++++++- src/ui/lib/extensionDialogGeometry.ts | 147 +++++++++++++++++- src/ui/lib/keyboard.ts | 15 ++ src/ui/lib/ui-lib.test.ts | 15 +- .../content/docs/docs/extend/extension-api.md | 20 ++- 12 files changed, 470 insertions(+), 94 deletions(-) diff --git a/.changeset/fuzzy-agents-guide.md b/.changeset/fuzzy-agents-guide.md index f6132df0f..002c8b6a5 100644 --- a/.changeset/fuzzy-agents-guide.md +++ b/.changeset/fuzzy-agents-guide.md @@ -2,4 +2,4 @@ "hunkdiff": minor --- -Add copyable document dialogs to the extension API and run Hunk's Agent Skill onboarding as a bundled extension. +Add fully disclosed copyable document dialogs to the extension API and run Hunk's Agent Skill onboarding as a bundled extension. diff --git a/docs/extensions.md b/docs/extensions.md index a35d54eeb..dc743169a 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -1681,7 +1681,9 @@ lines, and copy text may contain up to 16,384 JavaScript string code units. When `copy` is provided, `c` and the clickable copy action send its `text` to the terminal clipboard while the host removes terminal control sequences, expands tabs to four spaces, and renders the same safe value under `label` -(default `Content`). Optional `displayLines` can add authored visual breaks; +(default `Content`). Hunk exposes those actions only while the complete payload +and any required extension attribution are visible. Optional `displayLines` can +add authored visual breaks; after sanitizing, those lines must rejoin with spaces or newlines to exactly the clipboard `text`, so a preview cannot disguise what the action copies: diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 2af21688a..768a6aa50 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -89,6 +89,7 @@ import { } from "./lib/appCommands"; import { buildAppMenus } from "./lib/appMenus"; import { buildExtensionAppCommands, extensionCommandKeyDefaults } from "./lib/extensionCommands"; +import { planExtensionDocumentDialog } from "./lib/extensionDialogGeometry"; import type { CurrentLineAlignment } from "./lib/hunkScroll"; import type { LineCursor } from "./lib/lineCursors"; import { useFilePresentationController } from "./fileViews/useFilePresentationController"; @@ -935,16 +936,42 @@ export function App({ runExtensionCommand(bundledAgentSkillCommand); }, [bundledAgentSkillCommand, runExtensionCommand]); + const extensionDocumentLayout = + extensionDialog?.kind === "document" + ? planExtensionDocumentDialog(extensionDialog, terminal.width, terminal.height) + : null; + const extensionDialogCopySupported = + (renderer.isOsc52Supported?.() ?? false) && typeof renderer.copyToClipboardOSC52 === "function"; + const extensionDocumentCopyExposed = extensionDocumentLayout?.copyActionExposed ?? false; + /** Copy a document dialog's normalized payload through the terminal clipboard integration. */ const copyExtensionDialogDocument = useCallback(() => { - if (extensionDialog?.kind !== "document" || !extensionDialog.copy) return; + if ( + extensionDialog?.kind !== "document" || + !extensionDialog.copy || + !extensionDocumentCopyExposed + ) { + return; + } - if (renderer.isOsc52Supported?.() && typeof renderer.copyToClipboardOSC52 === "function") { - renderer.copyToClipboardOSC52(extensionDialog.copy.text); - showTransientNotice(`Copied ${extensionDialog.copy.label.toLowerCase()} to clipboard`); + if (extensionDialogCopySupported) { + const copied = renderer.copyToClipboardOSC52!(extensionDialog.copy.text); + if (!copied) { + showTransientNotice("Clipboard copy failed"); + return; + } + showTransientNotice( + `Copied ${extensionDialog.title.toLowerCase()} ${extensionDialog.copy.label.toLowerCase()} to clipboard`, + ); return; } - }, [extensionDialog, renderer, showTransientNotice]); + }, [ + extensionDialog, + extensionDialogCopySupported, + extensionDocumentCopyExposed, + renderer, + showTransientNotice, + ]); /** Toggle the modal keyboard help overlay. */ const toggleHelp = useCallback(() => { @@ -1140,6 +1167,7 @@ export function App({ extensionDialog, acceptExtensionDialog, cancelExtensionDialog, + extensionDocumentCopyEnabled: extensionDocumentCopyExposed && extensionDialogCopySupported, copyExtensionDialogDocument, moveExtensionDialogSelection, extensionTrustPromptOpen, @@ -1456,7 +1484,7 @@ export function App({ {extensionDialog ? ( { ); }, undefined, + { width: 50, height: 20 }, + ); + }); + + test("withholds copy when a short terminal cannot disclose the complete payload", async () => { + const repo = createTestRepo("hunk-ext-dialog-document-short-"); + const extDir = createTempDir("hunk-ext-dialog-document-short-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.document({ title: "Agent setup", body: "Teach your agent how to review this Hunk session.", copy: { label: "Prompt", text: "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 document dialog to open", + ); + + const frame = setup.captureCharFrame(); + expect(frame).toContain("ext ext"); + expect(frame).toContain("…"); + 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 }, ); }); @@ -436,6 +484,49 @@ describe("extension dialogs", () => { }); }); + test("reports a clipboard write rejected by the renderer as a failure", async () => { + const repo = createTestRepo("hunk-ext-dialog-document-copy-failure-"); + const extDir = createTempDir("hunk-ext-dialog-document-copy-failure-ext-"); + const logPath = join(extDir, "probe.log"); + const extPath = join(extDir, "ext.ts"); + writeDialogFixture( + extPath, + logPath, + `ctx.dialogs.document({ title: "Copy setup", copy: { label: "Prompt", text: "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 copy setup prompt to clipboard"); + }); + }); + 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-"); diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index 74943d443..4b5f3862d 100644 --- a/src/ui/AppHost.interactions.test.tsx +++ b/src/ui/AppHost.interactions.test.tsx @@ -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/components/chrome/ExtensionDialog.tsx b/src/ui/components/chrome/ExtensionDialog.tsx index d70f05f6d..98cfcb0b9 100644 --- a/src/ui/components/chrome/ExtensionDialog.tsx +++ b/src/ui/components/chrome/ExtensionDialog.tsx @@ -6,11 +6,11 @@ import type { ExtensionInputDialogRequest, ExtensionSelectDialogRequest, } from "../../lib/extensionDialogs"; +import { planExtensionDocumentDialog, windowDialogText } from "../../lib/extensionDialogGeometry"; import { extensionToastPrefix } from "../../lib/extensionNotifications"; -import { windowDialogLiteralText, windowDialogText } from "../../lib/extensionDialogGeometry"; import { listWindowStart } from "../../lib/listWindow"; import { MODAL_FRAME_CHROME_ROWS, resolveModalGeometry } from "../../lib/modalGeometry"; -import { fitText, padText } from "../../lib/text"; +import { fitText, measureTextWidth, padText } from "../../lib/text"; import type { AppTheme } from "../../themes"; import { ConfirmDialog, confirmDialogHeight, DialogActionRow } from "./ConfirmDialog"; import { ModalFrame } from "./ModalFrame"; @@ -38,15 +38,6 @@ function attributionText(extensionId: string, width: number) { return fitText(`${extensionToastPrefix()} ${extensionId}`, width); } -/** Preserve meaningful text when a constrained document has only one row. */ -function windowDocumentText(sourceLines: readonly string[], width: number, maxRows: number) { - const windowed = windowDialogText(sourceLines, width, maxRows); - if (maxRows !== 1 || !windowed.truncated) return windowed; - - const firstLine = windowDialogText(sourceLines, width, Number.MAX_SAFE_INTEGER).lines[0] ?? ""; - return { lines: [fitText(`${firstLine}…`, width, "…")], truncated: true }; -} - export function ExtensionDialog({ copySupported, inputValue, @@ -183,72 +174,24 @@ function ExtensionDocumentDialog({ terminalWidth: number; theme: AppTheme; }) { - const width = Math.min(84, Math.max(58, terminalWidth - 8)); - const measuredFrame = resolveModalGeometry({ - width, - height: Number.MAX_SAFE_INTEGER, - terminalWidth, - terminalHeight, - }); - const bodyWidth = Math.max(1, measuredFrame.width - 4); - const cardWidth = Math.max(1, bodyWidth - 4); - const cardTextWidth = Math.max(1, cardWidth - 4); - const idealBodyRows = windowDialogText(request.bodyLines, bodyWidth, Number.MAX_SAFE_INTEGER) - .lines.length; const copy = request.copy; - const idealCopyRows = copy - ? windowDialogLiteralText(copy.displayLines, cardTextWidth, Number.MAX_SAFE_INTEGER).lines - .length - : 0; - const hasBody = idealBodyRows > 0; - const hasCopy = copy !== null; - const idealContentRows = - (request.showAttribution ? 2 : 0) + - idealBodyRows + - (hasBody && hasCopy ? 1 : 0) + - (hasCopy ? 1 + idealCopyRows + 2 : 0) + - 2; - const frame = resolveModalGeometry({ - width, - // The inner flex column lets the final action use the last - // chrome-adjacent row without adding an empty footer row. - height: idealContentRows + MODAL_FRAME_CHROME_ROWS - 1, - terminalWidth, - terminalHeight, - }); - const contentRows = Math.max(0, frame.height - MODAL_FRAME_CHROME_ROWS + 1); - const actionRows = contentRows > 0 ? 1 : 0; - const minimumCopyRows = hasCopy ? 2 : 0; - const minimumContentRows = - (request.showAttribution ? 1 : 0) + (hasBody ? 1 : 0) + minimumCopyRows + actionRows; - const actionGapRows = contentRows > minimumContentRows ? 1 : 0; - let remainingRows = Math.max(0, contentRows - actionRows - actionGapRows); - const attributionRows = request.showAttribution && remainingRows > 0 ? 1 : 0; - remainingRows -= attributionRows; - const minimumVisibleDocumentRows = (hasBody ? 1 : 0) + minimumCopyRows; - const attributionGapRows = - attributionRows > 0 && remainingRows > minimumVisibleDocumentRows ? 1 : 0; - remainingRows -= attributionGapRows; - const copyReserve = hasCopy ? Math.min(minimumCopyRows, remainingRows) : 0; - const bodyCopyGapReserve = hasBody && hasCopy && remainingRows > copyReserve + 1 ? 1 : 0; - const bodyRows = Math.min( - idealBodyRows, - Math.max(0, remainingRows - copyReserve - bodyCopyGapReserve), - ); - remainingRows -= bodyRows; - const bodyCopyGapRows = bodyRows > 0 && hasCopy && remainingRows > 3 ? 1 : 0; - remainingRows -= bodyCopyGapRows; - const copyLabelRows = hasCopy && remainingRows > 1 ? 1 : 0; - remainingRows -= copyLabelRows; - const copyCardRows = hasCopy ? remainingRows : 0; - const visibleBody = windowDocumentText(request.bodyLines, bodyWidth, bodyRows); - const visibleCopy = copy - ? windowDialogLiteralText( - copy.displayLines, - cardTextWidth, - copyCardRows >= 3 ? copyCardRows - 2 : copyCardRows, - ) - : { lines: [], truncated: false }; + const layout = planExtensionDocumentDialog(request, terminalWidth, terminalHeight); + const { + actionGapRows, + actionRows, + attributionGapRows, + attributionRows, + bodyCopyGapRows, + bodyWidth, + cardTextWidth, + cardWidth, + copyActionExposed, + copyCardRows, + copyLabelRows, + frame, + visibleBody, + visibleCopy, + } = layout; return ( {attributionRows > 0 ? ( - {attributionText(request.extensionId, bodyWidth)} + {layout.attributionText} ) : null} {attributionGapRows > 0 ? : null} @@ -304,7 +247,7 @@ function ExtensionDocumentDialog({ ) : null} {actionGapRows > 0 ? : null} - {actionRows > 0 && copy ? ( + {actionRows > 0 && copy && copyActionExposed ? ( {label} - {padText("", Math.max(1, width - label.length))} + {padText("", Math.max(0, width - measureTextWidth(label)))} ); } diff --git a/src/ui/hooks/useAppKeyboardShortcuts.ts b/src/ui/hooks/useAppKeyboardShortcuts.ts index f0c078338..fedc48879 100644 --- a/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/src/ui/hooks/useAppKeyboardShortcuts.ts @@ -15,7 +15,7 @@ import { } from "../lib/appCommands"; import type { ExtensionDialogRequest } from "../lib/extensionDialogs"; import { toExtensionKeyEvent } from "../lib/extensionKeyEvent"; -import { isEscapeKey, isSaveDraftNoteKey } from "../lib/keyboard"; +import { isEscapeKey, isSaveDraftNoteKey, isUnmodifiedKey } from "../lib/keyboard"; import { routeKeyOwnership, type KeyOwner } from "../lib/keyRouting"; type FocusArea = "files" | "filter" | "note"; @@ -39,6 +39,8 @@ export interface UseAppKeyboardShortcutsOptions { extensionDialog: ExtensionDialogRequest | null; acceptExtensionDialog: () => void; cancelExtensionDialog: () => void; + /** Whether the visible document's fully disclosed copy action is usable. */ + extensionDocumentCopyEnabled: boolean; copyExtensionDialogDocument: () => void; moveExtensionDialogSelection: (delta: number) => void; extensionTrustPromptOpen: boolean; @@ -108,6 +110,7 @@ export function useAppKeyboardShortcuts({ extensionDialog, acceptExtensionDialog, cancelExtensionDialog, + extensionDocumentCopyEnabled, copyExtensionDialogDocument, moveExtensionDialogSelection, extensionTrustPromptOpen, @@ -141,6 +144,7 @@ export function useAppKeyboardShortcuts({ const themeSelectorOpenRef = useRef(themeSelectorOpen); const extensionTrustPromptOpenRef = useRef(extensionTrustPromptOpen); const extensionDialogRef = useRef(extensionDialog); + const extensionDocumentCopyEnabledRef = useRef(extensionDocumentCopyEnabled); // 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); @@ -164,6 +168,7 @@ export function useAppKeyboardShortcuts({ themeSelectorOpenRef.current = themeSelectorOpen; extensionTrustPromptOpenRef.current = extensionTrustPromptOpen; extensionDialogRef.current = extensionDialog; + extensionDocumentCopyEnabledRef.current = extensionDocumentCopyEnabled; isFileViewModeActiveRef.current = isFileViewModeActive; exitFileViewModeRef.current = exitFileViewMode; sendFileViewModeKeyRef.current = sendFileViewModeKey; @@ -337,7 +342,12 @@ export function useAppKeyboardShortcuts({ return "mine"; } - if (dialog.kind === "document" && dialog.copy && (key.name === "c" || key.sequence === "c")) { + if ( + dialog.kind === "document" && + dialog.copy && + extensionDocumentCopyEnabledRef.current && + isUnmodifiedKey(key, "c") + ) { copyExtensionDialogDocumentRef.current(); return "mine"; } diff --git a/src/ui/lib/extensionDialogGeometry.test.ts b/src/ui/lib/extensionDialogGeometry.test.ts index 467e93391..166e4c82f 100644 --- a/src/ui/lib/extensionDialogGeometry.test.ts +++ b/src/ui/lib/extensionDialogGeometry.test.ts @@ -1,5 +1,24 @@ import { describe, expect, test } from "bun:test"; -import { windowDialogLiteralText, windowDialogText } from "./extensionDialogGeometry"; +import type { ExtensionDocumentDialogRequest } from "./extensionDialogs"; +import { + planExtensionDocumentDialog, + windowDialogLiteralText, + windowDialogText, +} from "./extensionDialogGeometry"; + +const documentRequest = { + id: 1, + kind: "document", + extensionId: "example", + showAttribution: true, + title: "Agent setup", + bodyLines: ["Teach your agent how to review this Hunk session."], + copy: { + label: "Prompt", + text: "Load the Hunk skill and use it for this review. Run hunk skill path.", + displayLines: ["Load the Hunk skill and use it for this review. Run hunk skill path."], + }, +} satisfies ExtensionDocumentDialogRequest; describe("windowDialogText", () => { test("wraps prose within the available terminal-cell rows", () => { @@ -32,4 +51,75 @@ describe("windowDialogLiteralText", () => { truncated: true, }); }); + + test("marks a substituted cluster as incomplete disclosure", () => { + expect(windowDialogLiteralText(["界"], 1, 1)).toEqual({ + lines: ["…"], + truncated: true, + }); + }); + + test("marks invisible clusters as incomplete disclosure", () => { + expect(windowDialogLiteralText(["\u200b"], 4, 1)).toEqual({ + lines: ["\u200b"], + truncated: true, + }); + }); + + test("marks invisible scalars inside a visible grapheme as incomplete disclosure", () => { + const taggedFlag = "\u{1F3F4}\u{E0061}\u{E0062}\u{E007F}"; + expect(windowDialogLiteralText([taggedFlag], 4, 1)).toEqual({ + lines: [taggedFlag], + truncated: true, + }); + }); +}); + +describe("planExtensionDocumentDialog", () => { + test("exposes copy only when its complete payload and attribution fit", () => { + const complete = planExtensionDocumentDialog(documentRequest, 50, 20); + const constrained = planExtensionDocumentDialog(documentRequest, 50, 12); + + expect(complete.visibleCopy.truncated).toBe(false); + expect(complete.copyActionExposed).toBe(true); + expect(constrained.visibleCopy.truncated).toBe(true); + expect(constrained.copyActionExposed).toBe(false); + }); + + test("prioritizes required attribution over document actions", () => { + const layout = planExtensionDocumentDialog(documentRequest, 50, 7); + + expect(layout.attributionRows).toBe(1); + expect(layout.attributionText).toBe("ext example"); + expect(layout.actionRows).toBe(0); + expect(layout.copyActionExposed).toBe(false); + }); + + test("withholds copy when required attribution is truncated", () => { + const layout = planExtensionDocumentDialog( + { ...documentRequest, extensionId: "x".repeat(80) }, + 50, + 30, + ); + + expect(layout.attributionText).not.toBe(`ext ${"x".repeat(80)}`); + expect(layout.visibleCopy.truncated).toBe(false); + expect(layout.copyActionExposed).toBe(false); + }); + + test("withholds copy when frame chrome leaves no real card width", () => { + const layout = planExtensionDocumentDialog( + { + ...documentRequest, + showAttribution: false, + bodyLines: [], + copy: { label: "Content", text: "x", displayLines: ["x"] }, + }, + 6, + 30, + ); + + expect(layout.visibleCopy.truncated).toBe(false); + expect(layout.copyActionExposed).toBe(false); + }); }); diff --git a/src/ui/lib/extensionDialogGeometry.ts b/src/ui/lib/extensionDialogGeometry.ts index 95d9b4b06..e2805f80e 100644 --- a/src/ui/lib/extensionDialogGeometry.ts +++ b/src/ui/lib/extensionDialogGeometry.ts @@ -1,4 +1,7 @@ import { fitText, measureTextWidth, sliceTextByWidth, wrapText } from "./text"; +import { extensionToastPrefix } from "./extensionNotifications"; +import { MODAL_FRAME_CHROME_ROWS, resolveModalGeometry } from "./modalGeometry"; +import type { ExtensionDocumentDialogRequest } from "./extensionDialogs"; /** Wrapped body rows that fit one modal body allocation. */ export interface WindowedDialogText { @@ -33,6 +36,9 @@ export function windowDialogLiteralText( maxRows: number, ): WindowedDialogText { const safeWidth = Math.max(1, width); + let incompleteDisclosure = sourceLines.some((line) => + Array.from(line).some((scalar) => measureTextWidth(scalar) === 0), + ); const wrapped = sourceLines.flatMap((line) => { const lineWidth = measureTextWidth(line); if (lineWidth === 0) return [line]; @@ -50,15 +56,154 @@ export function windowDialogLiteralText( // overflow marker and advance past that cluster instead of looping. const wideChunk = sliceTextByWidth(line, offset, safeWidth + 1); lines.push(fitText(wideChunk.text, safeWidth, "…")); + incompleteDisclosure = true; offset += Math.max(1, wideChunk.width); } return lines; }); - if (wrapped.length <= maxRows) return { lines: wrapped, truncated: false }; + if (wrapped.length <= maxRows) return { lines: wrapped, truncated: incompleteDisclosure }; if (maxRows <= 0) return { lines: [], truncated: wrapped.length > 0 }; if (maxRows === 1) { return { lines: [fitText(`${wrapped[0] ?? ""}…`, safeWidth, "…")], truncated: true }; } return { lines: [...wrapped.slice(0, maxRows - 1), "…"], truncated: true }; } + +/** Concrete row allocation shared by document rendering and copy authorization. */ +export interface ExtensionDocumentDialogLayout { + frame: { width: number; height: number }; + bodyWidth: number; + cardWidth: number; + cardTextWidth: number; + attributionText: string; + attributionRows: number; + attributionGapRows: number; + bodyCopyGapRows: number; + copyLabelRows: number; + copyCardRows: number; + actionGapRows: number; + actionRows: number; + visibleBody: WindowedDialogText; + visibleCopy: WindowedDialogText; + /** Whether the complete payload and required attribution are visible beside the action. */ + copyActionExposed: boolean; +} + +/** Preserve meaningful text when a constrained document has only one body row. */ +function windowDocumentText(sourceLines: readonly string[], width: number, maxRows: number) { + const windowed = windowDialogText(sourceLines, width, maxRows); + if (maxRows !== 1 || !windowed.truncated) return windowed; + + const firstLine = windowDialogText(sourceLines, width, Number.MAX_SAFE_INTEGER).lines[0] ?? ""; + return { lines: [fitText(`${firstLine}…`, width, "…")], truncated: true }; +} + +/** Plan a read-only document so rendering and keyboard copy use identical disclosure facts. */ +export function planExtensionDocumentDialog( + request: ExtensionDocumentDialogRequest, + terminalWidth: number, + terminalHeight: number, +): ExtensionDocumentDialogLayout { + const width = Math.min(84, Math.max(58, terminalWidth - 8)); + const measuredFrame = resolveModalGeometry({ + width, + height: Number.MAX_SAFE_INTEGER, + terminalWidth, + terminalHeight, + }); + const bodyWidth = Math.max(1, measuredFrame.width - 4); + const cardWidth = Math.max(1, bodyWidth - 4); + const cardTextWidth = Math.max(1, cardWidth - 4); + const availableBodyWidth = measuredFrame.width - 4; + const availableCardWidth = availableBodyWidth - 4; + const idealBodyRows = windowDialogText(request.bodyLines, bodyWidth, Number.MAX_SAFE_INTEGER) + .lines.length; + const copy = request.copy; + const idealCopyRows = copy + ? windowDialogLiteralText(copy.displayLines, cardTextWidth, Number.MAX_SAFE_INTEGER).lines + .length + : 0; + const hasBody = idealBodyRows > 0; + const hasCopy = copy !== null; + const idealContentRows = + (request.showAttribution ? 2 : 0) + + idealBodyRows + + (hasBody && hasCopy ? 1 : 0) + + (hasCopy ? 1 + idealCopyRows + 2 : 0) + + 2; + const frame = resolveModalGeometry({ + width, + // The inner flex column lets the final action use the last + // chrome-adjacent row without adding an empty footer row. + height: idealContentRows + MODAL_FRAME_CHROME_ROWS - 1, + terminalWidth, + terminalHeight, + }); + const contentRows = Math.max(0, frame.height - MODAL_FRAME_CHROME_ROWS + 1); + let remainingRows = contentRows; + // Attribution wins the first available row so third-party copy UI never hides its owner. + const attributionRows = request.showAttribution && remainingRows > 0 ? 1 : 0; + remainingRows -= attributionRows; + const actionRows = remainingRows > 0 ? 1 : 0; + remainingRows -= actionRows; + const minimumCopyRows = hasCopy ? 2 : 0; + const minimumVisibleDocumentRows = (hasBody ? 1 : 0) + minimumCopyRows; + const attributionGapRows = + attributionRows > 0 && remainingRows > minimumVisibleDocumentRows ? 1 : 0; + remainingRows -= attributionGapRows; + const minimumContentRows = (hasBody ? 1 : 0) + minimumCopyRows; + const actionGapRows = remainingRows > minimumContentRows ? 1 : 0; + remainingRows -= actionGapRows; + const copyReserve = hasCopy ? Math.min(minimumCopyRows, remainingRows) : 0; + const bodyCopyGapReserve = hasBody && hasCopy && remainingRows > copyReserve + 1 ? 1 : 0; + const bodyRows = Math.min( + idealBodyRows, + Math.max(0, remainingRows - copyReserve - bodyCopyGapReserve), + ); + remainingRows -= bodyRows; + const bodyCopyGapRows = bodyRows > 0 && hasCopy && remainingRows > 3 ? 1 : 0; + remainingRows -= bodyCopyGapRows; + const copyLabelRows = hasCopy && remainingRows > 1 ? 1 : 0; + remainingRows -= copyLabelRows; + const copyCardRows = hasCopy ? remainingRows : 0; + const visibleBody = windowDocumentText(request.bodyLines, bodyWidth, bodyRows); + const visibleCopy = copy + ? windowDialogLiteralText( + copy.displayLines, + cardTextWidth, + copyCardRows >= 3 ? copyCardRows - 2 : copyCardRows, + ) + : { lines: [], truncated: false }; + const fullAttributionText = `${extensionToastPrefix()} ${request.extensionId}`; + const attributionComplete = + !request.showAttribution || + (attributionRows === 1 && measureTextWidth(fullAttributionText) <= bodyWidth); + const copyTextHasRealWidth = + availableBodyWidth > 0 && + (copyCardRows >= 3 ? availableCardWidth >= 5 : availableCardWidth >= 1); + + return { + frame, + bodyWidth, + cardWidth, + cardTextWidth, + attributionText: fitText(fullAttributionText, bodyWidth), + attributionRows, + attributionGapRows, + bodyCopyGapRows, + copyLabelRows, + copyCardRows, + actionGapRows, + actionRows, + visibleBody, + visibleCopy, + copyActionExposed: + hasCopy && + actionRows === 1 && + copyLabelRows === 1 && + copyTextHasRealWidth && + !visibleCopy.truncated && + attributionComplete, + }; +} diff --git a/src/ui/lib/keyboard.ts b/src/ui/lib/keyboard.ts index 73d66afab..98ea34f24 100644 --- a/src/ui/lib/keyboard.ts +++ b/src/ui/lib/keyboard.ts @@ -24,6 +24,21 @@ export function isEscapeKey(key: KeyEvent) { ); } +/** Match one literal key only when no modifier changes its meaning. */ +export function isUnmodifiedKey(key: KeyEvent, value: string) { + return ( + !key.ctrl && + !key.meta && + !key.option && + !key.shift && + !key.super && + !key.hyper && + !key.capsLock && + !key.numLock && + (key.name === value || key.sequence === value) + ); +} + /** * Match Ctrl-S across raw, Kitty/CSI-u, and tmux control-mode encodings. * diff --git a/src/ui/lib/ui-lib.test.ts b/src/ui/lib/ui-lib.test.ts index 1292f255d..5e8b85256 100644 --- a/src/ui/lib/ui-lib.test.ts +++ b/src/ui/lib/ui-lib.test.ts @@ -13,7 +13,7 @@ import { } from "../components/chrome/menu"; import { createVisibleAgentNote } from "./agentAnnotations"; import { buildAgentPopoverContent, resolveAgentPopoverPlacement } from "./agentPopover"; -import { isEscapeKey, isSaveDraftNoteKey } from "./keyboard"; +import { isEscapeKey, isSaveDraftNoteKey, isUnmodifiedKey } from "./keyboard"; import { BoundedClusterWidthCache, CLUSTER_WIDTH_CACHE_MAX_ENTRIES, @@ -194,6 +194,19 @@ describe("ui helpers", () => { expect(isEscapeKey(createKeyEvent({ name: "q" }))).toBe(false); }); + test("literal modal keys reject every modified form", () => { + expect(isUnmodifiedKey(createKeyEvent({ name: "c" }), "c")).toBe(true); + expect(isUnmodifiedKey(createKeyEvent({ sequence: "c" }), "c")).toBe(true); + expect(isUnmodifiedKey(createKeyEvent({ name: "c", ctrl: true }), "c")).toBe(false); + expect(isUnmodifiedKey(createKeyEvent({ name: "c", meta: true }), "c")).toBe(false); + expect(isUnmodifiedKey(createKeyEvent({ name: "c", option: true }), "c")).toBe(false); + expect(isUnmodifiedKey(createKeyEvent({ name: "c", shift: true }), "c")).toBe(false); + expect(isUnmodifiedKey(createKeyEvent({ name: "c", super: true }), "c")).toBe(false); + expect(isUnmodifiedKey(createKeyEvent({ name: "c", hyper: true }), "c")).toBe(false); + expect(isUnmodifiedKey(createKeyEvent({ name: "c", capsLock: true }), "c")).toBe(false); + expect(isUnmodifiedKey(createKeyEvent({ name: "c", numLock: true }), "c")).toBe(false); + }); + test("save-draft-note shortcut matches Ctrl-S across raw, CSI-u, and tmux encodings", () => { const CTRL_S = "\u0013"; const CTRL_S_CSI_U = "\u001b[115;5u"; diff --git a/website/src/content/docs/docs/extend/extension-api.md b/website/src/content/docs/docs/extend/extension-api.md index d815890d2..8aea478cc 100644 --- a/website/src/content/docs/docs/extend/extension-api.md +++ b/website/src/content/docs/docs/extend/extension-api.md @@ -318,7 +318,25 @@ lines, and copy text may contain up to 16,384 JavaScript string code units. When `copy` is provided, `c` and the clickable copy action send its `text` to the terminal clipboard after Hunk removes terminal control sequences and expands tabs to four spaces, and Hunk renders the same safe value under `label` -(default `Content`). +(default `Content`). Hunk exposes those actions only while the complete payload +and any required extension attribution are visible. Optional `displayLines` can +add authored visual breaks; after sanitizing, those lines must rejoin with +spaces or newlines to exactly the clipboard `text`, so a preview cannot disguise +what the action copies: + +```ts +hunk.registerCommand({ id: "agent-setup", title: "Agent setup" }, async (ctx) => { + await ctx.dialogs.document({ + title: "Agent setup", + body: "Give this prompt to your coding agent.", + copy: { + label: "Prompt", + text: "Review the current Hunk session. Focus on correctness.", + displayLines: ["Review the current Hunk session.", "Focus on correctness."], + }, + }); +}); +``` `select` fits acting on part of the selection — asking which hunk to jump to, then navigating there: From 14df7e1fb2c6cb534d5894cfb445b2650a19560e Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 31 Aug 2026 20:39:20 -0400 Subject: [PATCH 7/9] refactor(extensions): rename document dialogs to info --- .changeset/fuzzy-agents-guide.md | 2 +- docs/extensions.md | 10 +-- skills/hunk-extensions/SKILL.md | 2 +- src/extension-api/index.ts | 4 +- src/extension-api/types.ts | 16 ++-- .../default/ui/agentSkill/index.test.ts | 8 +- src/extensions/default/ui/agentSkill/index.ts | 2 +- src/extensions/events.test.ts | 2 +- src/extensions/events.ts | 2 +- src/extensions/types.ts | 4 +- src/ui/App.tsx | 30 +++---- src/ui/AppHost.extension-dialogs.test.tsx | 36 ++++----- src/ui/components/chrome/ExtensionDialog.tsx | 42 +++++----- src/ui/hooks/useAppKeyboardShortcuts.ts | 26 +++--- src/ui/lib/extensionDialogGeometry.test.ts | 30 +++---- src/ui/lib/extensionDialogGeometry.ts | 20 ++--- src/ui/lib/extensionDialogs.test.ts | 26 +++--- src/ui/lib/extensionDialogs.ts | 80 +++++++++---------- test/pty/chrome.test.ts | 6 +- .../content/docs/docs/extend/extension-api.md | 8 +- 20 files changed, 173 insertions(+), 183 deletions(-) diff --git a/.changeset/fuzzy-agents-guide.md b/.changeset/fuzzy-agents-guide.md index 002c8b6a5..63683f74c 100644 --- a/.changeset/fuzzy-agents-guide.md +++ b/.changeset/fuzzy-agents-guide.md @@ -2,4 +2,4 @@ "hunkdiff": minor --- -Add fully disclosed copyable document dialogs to the extension API and run Hunk's Agent Skill onboarding as a bundled extension. +Add fully disclosed copyable info dialogs to the extension API and run Hunk's Agent Skill onboarding as a bundled extension. diff --git a/docs/extensions.md b/docs/extensions.md index dc743169a..f3a86fa68 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -281,7 +281,7 @@ new instances and run that shutdown/startup pair around the replacement. ### `hunk.apiVersion` The API generation this Hunk speaks (currently `17`). Branch on it if you want -one file to support several Hunk versions. Version 17 adds read-only document +one file to support several Hunk versions. Version 17 adds read-only info dialogs with optional clipboard actions; version 16 added temporary application handoffs and on-disk location resolution to command handlers; version 15 added `{ side, line }` to opted-in pane @@ -1675,7 +1675,7 @@ hunk.registerCommand({ id: "pick-hunk", title: "Pick a hunk", key: "ctrl+k" }, a }); ``` -`document` presents read-only guidance rather than asking for an answer. At +`info` presents read-only guidance rather than asking for an answer. At least `body` or `copy` must be present. A body may contain up to 100 source lines, and copy text may contain up to 16,384 JavaScript string code units. When `copy` is provided, `c` and the clickable copy action send its `text` to @@ -1689,7 +1689,7 @@ clipboard `text`, so a preview cannot disguise what the action copies: ```ts hunk.registerCommand({ id: "agent-setup", title: "Agent setup" }, async (ctx) => { - await ctx.dialogs.document({ + await ctx.dialogs.info({ title: "Agent setup", body: "Give this prompt to your coding agent.", copy: { @@ -1709,9 +1709,9 @@ itself as Hunk asking. Hunk's own bundled extensions omit that redundant marker. One dialog is on screen at a time. Concurrent requests queue in call order, 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 document, Enter accepts the confirm action, highlighted +or `null`) or closes an info dialog, Enter accepts the confirm action, highlighted option, or typed text, and review shortcuts stay suppressed underneath. -Documents ignore Enter and remain open. Confirm dialogs also answer to `y`/`n`, +Info dialogs ignore Enter and remain open. 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 diff --git a/skills/hunk-extensions/SKILL.md b/skills/hunk-extensions/SKILL.md index 377ed6397..1c14e9548 100644 --- a/skills/hunk-extensions/SKILL.md +++ b/skills/hunk-extensions/SKILL.md @@ -160,7 +160,7 @@ 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`/`document`, queued and attributed), + `ctx.dialogs` (`confirm`/`select`/`input`/`info`, queued and attributed), `ctx.openInApp` (temporary terminal ownership around extension-run applications), and `ctx.workspace` (`readDocument`, `resolveLocation`, `canWriteDocument`, `writeDocument` with consent). diff --git a/src/extension-api/index.ts b/src/extension-api/index.ts index f6791303e..43df735bb 100644 --- a/src/extension-api/index.ts +++ b/src/extension-api/index.ts @@ -95,8 +95,8 @@ export type { ExtensionReviewSnapshotNote, ExtensionReviewSnapshotNoteAnchor, ExtensionConfirmOptions, - ExtensionDocumentCopyOptions, - ExtensionDocumentOptions, + ExtensionInfoCopyOptions, + ExtensionInfoOptions, ExtensionDialogs, ExtensionInputOptions, ExtensionSelectOptions, diff --git a/src/extension-api/types.ts b/src/extension-api/types.ts index 9283fbf20..b962282d1 100644 --- a/src/extension-api/types.ts +++ b/src/extension-api/types.ts @@ -1631,8 +1631,8 @@ export interface ExtensionInputOptions { initial?: string; } -/** Copyable text shown inside a document dialog. */ -export interface ExtensionDocumentCopyOptions { +/** Copyable text shown inside an info dialog. */ +export interface ExtensionInfoCopyOptions { /** Short heading shown above the copyable text. Defaults to "Content". */ label?: string; /** @@ -1648,13 +1648,13 @@ export interface ExtensionDocumentCopyOptions { displayLines?: readonly string[]; } -/** Read-only guidance shown to the user as a modal document. */ -export interface ExtensionDocumentOptions { +/** Read-only information shown to the user in a modal. */ +export interface ExtensionInfoOptions { title: string; /** Optional prose shown above the copyable text. Limited to 100 source lines. */ body?: string; /** Optional text card the user can copy with `c` or the mouse. */ - copy?: ExtensionDocumentCopyOptions; + copy?: ExtensionInfoCopyOptions; } /** @@ -1669,7 +1669,7 @@ export interface ExtensionDocumentOptions { * * Escape always dismisses, resolving the cancel value (`false`, `null`, or * `undefined`). Enter accepts: the confirm action, the highlighted option, or - * the typed text. Documents are read-only and remain open until dismissed. + * the typed text. Info dialogs are read-only and remain open until dismissed. * 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 @@ -1678,7 +1678,7 @@ export interface ExtensionDocumentOptions { * * Bad arguments are a programming error rather than a user answer, so they * reject instead of resolving: a missing or blank `title`, a `select` with no - * options, or document content outside its documented bounds. Because a dialog + * options, or info content outside its documented bounds. 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. @@ -1691,7 +1691,7 @@ export interface ExtensionDialogs { /** Resolves the submitted text, or null on cancel/escape. */ input(options: ExtensionInputOptions): Promise; /** Show read-only guidance until the user dismisses it. */ - document(options: ExtensionDocumentOptions): Promise; + info(options: ExtensionInfoOptions): 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 index 558f18283..bfe576bf5 100644 --- a/src/extensions/default/ui/agentSkill/index.test.ts +++ b/src/extensions/default/ui/agentSkill/index.test.ts @@ -28,13 +28,13 @@ describe("bundled agent skill extension", () => { }); }); - test("opens its onboarding through the public document dialog", async () => { - const document = mock(async () => {}); - const context = { dialogs: { document } } as unknown as ExtensionCommandContext; + test("opens its onboarding through the public info dialog", async () => { + const info = mock(async () => {}); + const context = { dialogs: { info } } as unknown as ExtensionCommandContext; await getBundledAgentSkillCommand().handler(context); - expect(document).toHaveBeenCalledWith({ + expect(info).toHaveBeenCalledWith({ title: "Agent skill", body: "Teach your agent how to review this Hunk session.", copy: { diff --git a/src/extensions/default/ui/agentSkill/index.ts b/src/extensions/default/ui/agentSkill/index.ts index bddde318d..e4118d645 100644 --- a/src/extensions/default/ui/agentSkill/index.ts +++ b/src/extensions/default/ui/agentSkill/index.ts @@ -17,7 +17,7 @@ const registerBundledAgentSkill: ExtensionFactory = (hunk) => { title: "Show setup guidance for reviewing with an agent", }, async (ctx) => { - await ctx.dialogs.document({ + await ctx.dialogs.info({ title: "Agent skill", body: "Teach your agent how to review this Hunk session.", copy: { diff --git a/src/extensions/events.test.ts b/src/extensions/events.test.ts index b3ecafbe8..fd147580f 100644 --- a/src/extensions/events.test.ts +++ b/src/extensions/events.test.ts @@ -195,7 +195,7 @@ describe("extension event dispatch", () => { confirm: async () => false, select: async () => null, input: async () => null, - document: async () => {}, + info: async () => {}, }, events: { emit: () => {} }, }; diff --git a/src/extensions/events.ts b/src/extensions/events.ts index 427197e59..908c4fcd5 100644 --- a/src/extensions/events.ts +++ b/src/extensions/events.ts @@ -343,7 +343,7 @@ function unavailableDialogs(result: ExtensionLoadResult, extensionId: string): E unavailable(); return null; }, - document: async () => { + info: async () => { unavailable(); }, }; diff --git a/src/extensions/types.ts b/src/extensions/types.ts index 6dfc2ba95..c9aba24e7 100644 --- a/src/extensions/types.ts +++ b/src/extensions/types.ts @@ -44,8 +44,8 @@ export type { ExtensionContext, ExtensionCustomEventHandler, ExtensionDiffFile, - ExtensionDocumentCopyOptions, - ExtensionDocumentOptions, + ExtensionInfoCopyOptions, + ExtensionInfoOptions, ExtensionEventBus, ExtensionEventContext, ExtensionEventHandler, diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 768a6aa50..4bed8f3e5 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -89,7 +89,7 @@ import { } from "./lib/appCommands"; import { buildAppMenus } from "./lib/appMenus"; import { buildExtensionAppCommands, extensionCommandKeyDefaults } from "./lib/extensionCommands"; -import { planExtensionDocumentDialog } from "./lib/extensionDialogGeometry"; +import { planExtensionInfoDialog } from "./lib/extensionDialogGeometry"; import type { CurrentLineAlignment } from "./lib/hunkScroll"; import type { LineCursor } from "./lib/lineCursors"; import { useFilePresentationController } from "./fileViews/useFilePresentationController"; @@ -936,21 +936,17 @@ export function App({ runExtensionCommand(bundledAgentSkillCommand); }, [bundledAgentSkillCommand, runExtensionCommand]); - const extensionDocumentLayout = - extensionDialog?.kind === "document" - ? planExtensionDocumentDialog(extensionDialog, terminal.width, terminal.height) + const extensionInfoLayout = + extensionDialog?.kind === "info" + ? planExtensionInfoDialog(extensionDialog, terminal.width, terminal.height) : null; const extensionDialogCopySupported = (renderer.isOsc52Supported?.() ?? false) && typeof renderer.copyToClipboardOSC52 === "function"; - const extensionDocumentCopyExposed = extensionDocumentLayout?.copyActionExposed ?? false; - - /** Copy a document dialog's normalized payload through the terminal clipboard integration. */ - const copyExtensionDialogDocument = useCallback(() => { - if ( - extensionDialog?.kind !== "document" || - !extensionDialog.copy || - !extensionDocumentCopyExposed - ) { + const extensionInfoCopyExposed = extensionInfoLayout?.copyActionExposed ?? false; + + /** Copy an info dialog's normalized payload through the terminal clipboard integration. */ + const copyExtensionDialogInfo = useCallback(() => { + if (extensionDialog?.kind !== "info" || !extensionDialog.copy || !extensionInfoCopyExposed) { return; } @@ -968,7 +964,7 @@ export function App({ }, [ extensionDialog, extensionDialogCopySupported, - extensionDocumentCopyExposed, + extensionInfoCopyExposed, renderer, showTransientNotice, ]); @@ -1167,8 +1163,8 @@ export function App({ extensionDialog, acceptExtensionDialog, cancelExtensionDialog, - extensionDocumentCopyEnabled: extensionDocumentCopyExposed && extensionDialogCopySupported, - copyExtensionDialogDocument, + extensionInfoCopyEnabled: extensionInfoCopyExposed && extensionDialogCopySupported, + copyExtensionDialogInfo, moveExtensionDialogSelection, extensionTrustPromptOpen, trustRepoExtensions, @@ -1494,7 +1490,7 @@ export function App({ onAccept={acceptExtensionDialog} onCancel={cancelExtensionDialog} onChangeInput={setExtensionDialogInputValue} - onCopyDocument={() => copyExtensionDialogDocument()} + onCopyInfo={() => copyExtensionDialogInfo()} onPickOption={setExtensionDialogSelectedIndex} /> ) : null} diff --git a/src/ui/AppHost.extension-dialogs.test.tsx b/src/ui/AppHost.extension-dialogs.test.tsx index a665382b6..d7e19bd02 100644 --- a/src/ui/AppHost.extension-dialogs.test.tsx +++ b/src/ui/AppHost.extension-dialogs.test.tsx @@ -317,15 +317,15 @@ describe("extension dialogs", () => { }); }); - test("a document dialog renders copyable guidance and closes only on escape", async () => { - const repo = createTestRepo("hunk-ext-dialog-document-"); - const extDir = createTempDir("hunk-ext-dialog-document-ext-"); + test("an info dialog renders copyable guidance and closes only on escape", async () => { + const repo = createTestRepo("hunk-ext-dialog-info-"); + const extDir = createTempDir("hunk-ext-dialog-info-ext-"); const logPath = join(extDir, "probe.log"); const extPath = join(extDir, "ext.ts"); writeDialogFixture( extPath, logPath, - `ctx.dialogs.document({ title: "Agent setup", body: "Teach your agent how to review this Hunk session.", copy: { label: "Prompt", text: "Load the Hunk skill and use it for this review. Run hunk skill path to get the skill path." } })`, + `ctx.dialogs.info({ title: "Agent setup", body: "Teach your agent how to review this Hunk session.", copy: { label: "Prompt", text: "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); @@ -345,7 +345,7 @@ describe("extension dialogs", () => { await flushUntil( setup, () => setup.captureCharFrame().includes("Agent setup"), - "the document dialog to open", + "the info dialog to open", ); const frame = setup.captureCharFrame(); @@ -377,7 +377,7 @@ describe("extension dialogs", () => { await flushUntil( setup, () => readProbeLog(logPath).includes("answer undefined"), - "the document handler to finish", + "the info handler to finish", ); }, undefined, @@ -386,14 +386,14 @@ describe("extension dialogs", () => { }); test("withholds copy when a short terminal cannot disclose the complete payload", async () => { - const repo = createTestRepo("hunk-ext-dialog-document-short-"); - const extDir = createTempDir("hunk-ext-dialog-document-short-ext-"); + const repo = createTestRepo("hunk-ext-dialog-info-short-"); + const extDir = createTempDir("hunk-ext-dialog-info-short-ext-"); const logPath = join(extDir, "probe.log"); const extPath = join(extDir, "ext.ts"); writeDialogFixture( extPath, logPath, - `ctx.dialogs.document({ title: "Agent setup", body: "Teach your agent how to review this Hunk session.", copy: { label: "Prompt", text: "Load the Hunk skill and use it for this review. Run hunk skill path to get the skill path." } })`, + `ctx.dialogs.info({ title: "Agent setup", body: "Teach your agent how to review this Hunk session.", copy: { label: "Prompt", text: "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); @@ -413,7 +413,7 @@ describe("extension dialogs", () => { await flushUntil( setup, () => setup.captureCharFrame().includes("Agent setup"), - "the constrained document dialog to open", + "the constrained info dialog to open", ); const frame = setup.captureCharFrame(); @@ -433,15 +433,15 @@ describe("extension dialogs", () => { ); }); - test("an unavailable document copy action is visible but inert", async () => { - const repo = createTestRepo("hunk-ext-dialog-document-unavailable-"); - const extDir = createTempDir("hunk-ext-dialog-document-unavailable-ext-"); + test("an unavailable info copy action is visible but inert", async () => { + const repo = createTestRepo("hunk-ext-dialog-info-unavailable-"); + const extDir = createTempDir("hunk-ext-dialog-info-unavailable-ext-"); const logPath = join(extDir, "probe.log"); const extPath = join(extDir, "ext.ts"); writeDialogFixture( extPath, logPath, - `ctx.dialogs.document({ title: "Copy setup", copy: { label: "Prompt", text: "copy me" } })`, + `ctx.dialogs.info({ title: "Copy setup", copy: { label: "Prompt", text: "copy me" } })`, ); const bootstrap = await launchWithExtension(repo, extPath); @@ -479,20 +479,20 @@ describe("extension dialogs", () => { await flushUntil( setup, () => readProbeLog(logPath).includes("answer undefined"), - "the unavailable-copy document to close", + "the unavailable-copy info dialog to close", ); }); }); test("reports a clipboard write rejected by the renderer as a failure", async () => { - const repo = createTestRepo("hunk-ext-dialog-document-copy-failure-"); - const extDir = createTempDir("hunk-ext-dialog-document-copy-failure-ext-"); + const repo = createTestRepo("hunk-ext-dialog-info-copy-failure-"); + const extDir = createTempDir("hunk-ext-dialog-info-copy-failure-ext-"); const logPath = join(extDir, "probe.log"); const extPath = join(extDir, "ext.ts"); writeDialogFixture( extPath, logPath, - `ctx.dialogs.document({ title: "Copy setup", copy: { label: "Prompt", text: "copy me" } })`, + `ctx.dialogs.info({ title: "Copy setup", copy: { label: "Prompt", text: "copy me" } })`, ); const bootstrap = await launchWithExtension(repo, extPath); diff --git a/src/ui/components/chrome/ExtensionDialog.tsx b/src/ui/components/chrome/ExtensionDialog.tsx index 98cfcb0b9..e8623f9a6 100644 --- a/src/ui/components/chrome/ExtensionDialog.tsx +++ b/src/ui/components/chrome/ExtensionDialog.tsx @@ -1,12 +1,12 @@ import type { MouseEvent as TuiMouseEvent } from "@opentui/core"; import type { ExtensionDialogRequest, - ExtensionDocumentCopyRequest, - ExtensionDocumentDialogRequest, + ExtensionInfoCopyRequest, + ExtensionInfoDialogRequest, ExtensionInputDialogRequest, ExtensionSelectDialogRequest, } from "../../lib/extensionDialogs"; -import { planExtensionDocumentDialog, windowDialogText } from "../../lib/extensionDialogGeometry"; +import { planExtensionInfoDialog, windowDialogText } from "../../lib/extensionDialogGeometry"; import { extensionToastPrefix } from "../../lib/extensionNotifications"; import { listWindowStart } from "../../lib/listWindow"; import { MODAL_FRAME_CHROME_ROWS, resolveModalGeometry } from "../../lib/modalGeometry"; @@ -44,7 +44,7 @@ export function ExtensionDialog({ onAccept, onCancel, onChangeInput, - onCopyDocument, + onCopyInfo, onPickOption, request, selectedIndex, @@ -58,7 +58,7 @@ export function ExtensionDialog({ onAccept: (selectedIndexOverride?: number) => void; onCancel: () => void; onChangeInput: (value: string) => void; - onCopyDocument: (copy: ExtensionDocumentCopyRequest) => void; + onCopyInfo: (copy: ExtensionInfoCopyRequest) => void; /** Highlight one option row without accepting it, mirroring the theme selector. */ onPickOption: (index: number) => void; request: ExtensionDialogRequest; @@ -67,12 +67,12 @@ export function ExtensionDialog({ terminalWidth: number; theme: AppTheme; }) { - if (request.kind === "document") { + if (request.kind === "info") { return ( - void; - onCopyDocument: (copy: ExtensionDocumentCopyRequest) => void; - request: ExtensionDocumentDialogRequest; + onCopyInfo: (copy: ExtensionInfoCopyRequest) => void; + request: ExtensionInfoDialogRequest; terminalHeight: number; terminalWidth: number; theme: AppTheme; }) { const copy = request.copy; - const layout = planExtensionDocumentDialog(request, terminalWidth, terminalHeight); + const layout = planExtensionInfoDialog(request, terminalWidth, terminalHeight); const { actionGapRows, actionRows, @@ -248,10 +248,10 @@ function ExtensionDocumentDialog({ ) : null} {actionGapRows > 0 ? : null} {actionRows > 0 && copy && copyActionExposed ? ( - @@ -266,17 +266,17 @@ function ExtensionDocumentDialog({ ); } -/** Render the compact copy affordance beneath a document card. */ -function DocumentCopyAction({ +/** Render the compact copy affordance beneath an info card. */ +function InfoCopyAction({ copy, copySupported, - onCopyDocument, + onCopyInfo, theme, width, }: { - copy: ExtensionDocumentCopyRequest; + copy: ExtensionInfoCopyRequest; copySupported: boolean; - onCopyDocument: (copy: ExtensionDocumentCopyRequest) => void; + onCopyInfo: (copy: ExtensionInfoCopyRequest) => void; theme: AppTheme; width: number; }) { @@ -291,7 +291,7 @@ function DocumentCopyAction({ style={{ backgroundColor: copySupported ? theme.accentMuted : theme.panelAlt }} onMouseUp={(event: TuiMouseEvent) => { event.stopPropagation(); - if (copySupported) onCopyDocument(copy); + if (copySupported) onCopyInfo(copy); }} > {label} diff --git a/src/ui/hooks/useAppKeyboardShortcuts.ts b/src/ui/hooks/useAppKeyboardShortcuts.ts index fedc48879..50f38f3b1 100644 --- a/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/src/ui/hooks/useAppKeyboardShortcuts.ts @@ -39,9 +39,9 @@ export interface UseAppKeyboardShortcutsOptions { extensionDialog: ExtensionDialogRequest | null; acceptExtensionDialog: () => void; cancelExtensionDialog: () => void; - /** Whether the visible document's fully disclosed copy action is usable. */ - extensionDocumentCopyEnabled: boolean; - copyExtensionDialogDocument: () => void; + /** Whether the visible info dialog's fully disclosed copy action is usable. */ + extensionInfoCopyEnabled: boolean; + copyExtensionDialogInfo: () => void; moveExtensionDialogSelection: (delta: number) => void; extensionTrustPromptOpen: boolean; trustRepoExtensions: () => void; @@ -110,8 +110,8 @@ export function useAppKeyboardShortcuts({ extensionDialog, acceptExtensionDialog, cancelExtensionDialog, - extensionDocumentCopyEnabled, - copyExtensionDialogDocument, + extensionInfoCopyEnabled, + copyExtensionDialogInfo, moveExtensionDialogSelection, extensionTrustPromptOpen, trustRepoExtensions, @@ -144,7 +144,7 @@ export function useAppKeyboardShortcuts({ const themeSelectorOpenRef = useRef(themeSelectorOpen); const extensionTrustPromptOpenRef = useRef(extensionTrustPromptOpen); const extensionDialogRef = useRef(extensionDialog); - const extensionDocumentCopyEnabledRef = useRef(extensionDocumentCopyEnabled); + const extensionInfoCopyEnabledRef = useRef(extensionInfoCopyEnabled); // 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); @@ -157,7 +157,7 @@ export function useAppKeyboardShortcuts({ // text), so they are read through refs rather than captured once. const acceptExtensionDialogRef = useRef(acceptExtensionDialog); const cancelExtensionDialogRef = useRef(cancelExtensionDialog); - const copyExtensionDialogDocumentRef = useRef(copyExtensionDialogDocument); + const copyExtensionDialogInfoRef = useRef(copyExtensionDialogInfo); const moveExtensionDialogSelectionRef = useRef(moveExtensionDialogSelection); activeMenuIdRef.current = activeMenuId; @@ -168,7 +168,7 @@ export function useAppKeyboardShortcuts({ themeSelectorOpenRef.current = themeSelectorOpen; extensionTrustPromptOpenRef.current = extensionTrustPromptOpen; extensionDialogRef.current = extensionDialog; - extensionDocumentCopyEnabledRef.current = extensionDocumentCopyEnabled; + extensionInfoCopyEnabledRef.current = extensionInfoCopyEnabled; isFileViewModeActiveRef.current = isFileViewModeActive; exitFileViewModeRef.current = exitFileViewMode; sendFileViewModeKeyRef.current = sendFileViewModeKey; @@ -177,7 +177,7 @@ export function useAppKeyboardShortcuts({ sendKeyboardModeKeyRef.current = sendKeyboardModeKey; acceptExtensionDialogRef.current = acceptExtensionDialog; cancelExtensionDialogRef.current = cancelExtensionDialog; - copyExtensionDialogDocumentRef.current = copyExtensionDialogDocument; + copyExtensionDialogInfoRef.current = copyExtensionDialogInfo; moveExtensionDialogSelectionRef.current = moveExtensionDialogSelection; /** @@ -336,19 +336,19 @@ export function useAppKeyboardShortcuts({ } if (key.name === "return" || key.name === "enter") { - if (dialog.kind !== "document") { + if (dialog.kind !== "info") { acceptExtensionDialogRef.current(); } return "mine"; } if ( - dialog.kind === "document" && + dialog.kind === "info" && dialog.copy && - extensionDocumentCopyEnabledRef.current && + extensionInfoCopyEnabledRef.current && isUnmodifiedKey(key, "c") ) { - copyExtensionDialogDocumentRef.current(); + copyExtensionDialogInfoRef.current(); return "mine"; } diff --git a/src/ui/lib/extensionDialogGeometry.test.ts b/src/ui/lib/extensionDialogGeometry.test.ts index 166e4c82f..004c1eefd 100644 --- a/src/ui/lib/extensionDialogGeometry.test.ts +++ b/src/ui/lib/extensionDialogGeometry.test.ts @@ -1,14 +1,14 @@ import { describe, expect, test } from "bun:test"; -import type { ExtensionDocumentDialogRequest } from "./extensionDialogs"; +import type { ExtensionInfoDialogRequest } from "./extensionDialogs"; import { - planExtensionDocumentDialog, + planExtensionInfoDialog, windowDialogLiteralText, windowDialogText, } from "./extensionDialogGeometry"; -const documentRequest = { +const infoRequest = { id: 1, - kind: "document", + kind: "info", extensionId: "example", showAttribution: true, title: "Agent setup", @@ -18,7 +18,7 @@ const documentRequest = { text: "Load the Hunk skill and use it for this review. Run hunk skill path.", displayLines: ["Load the Hunk skill and use it for this review. Run hunk skill path."], }, -} satisfies ExtensionDocumentDialogRequest; +} satisfies ExtensionInfoDialogRequest; describe("windowDialogText", () => { test("wraps prose within the available terminal-cell rows", () => { @@ -75,10 +75,10 @@ describe("windowDialogLiteralText", () => { }); }); -describe("planExtensionDocumentDialog", () => { +describe("planExtensionInfoDialog", () => { test("exposes copy only when its complete payload and attribution fit", () => { - const complete = planExtensionDocumentDialog(documentRequest, 50, 20); - const constrained = planExtensionDocumentDialog(documentRequest, 50, 12); + const complete = planExtensionInfoDialog(infoRequest, 50, 20); + const constrained = planExtensionInfoDialog(infoRequest, 50, 12); expect(complete.visibleCopy.truncated).toBe(false); expect(complete.copyActionExposed).toBe(true); @@ -86,8 +86,8 @@ describe("planExtensionDocumentDialog", () => { expect(constrained.copyActionExposed).toBe(false); }); - test("prioritizes required attribution over document actions", () => { - const layout = planExtensionDocumentDialog(documentRequest, 50, 7); + test("prioritizes required attribution over info actions", () => { + const layout = planExtensionInfoDialog(infoRequest, 50, 7); expect(layout.attributionRows).toBe(1); expect(layout.attributionText).toBe("ext example"); @@ -96,11 +96,7 @@ describe("planExtensionDocumentDialog", () => { }); test("withholds copy when required attribution is truncated", () => { - const layout = planExtensionDocumentDialog( - { ...documentRequest, extensionId: "x".repeat(80) }, - 50, - 30, - ); + const layout = planExtensionInfoDialog({ ...infoRequest, extensionId: "x".repeat(80) }, 50, 30); expect(layout.attributionText).not.toBe(`ext ${"x".repeat(80)}`); expect(layout.visibleCopy.truncated).toBe(false); @@ -108,9 +104,9 @@ describe("planExtensionDocumentDialog", () => { }); test("withholds copy when frame chrome leaves no real card width", () => { - const layout = planExtensionDocumentDialog( + const layout = planExtensionInfoDialog( { - ...documentRequest, + ...infoRequest, showAttribution: false, bodyLines: [], copy: { label: "Content", text: "x", displayLines: ["x"] }, diff --git a/src/ui/lib/extensionDialogGeometry.ts b/src/ui/lib/extensionDialogGeometry.ts index e2805f80e..dcc83b5a5 100644 --- a/src/ui/lib/extensionDialogGeometry.ts +++ b/src/ui/lib/extensionDialogGeometry.ts @@ -1,7 +1,7 @@ import { fitText, measureTextWidth, sliceTextByWidth, wrapText } from "./text"; import { extensionToastPrefix } from "./extensionNotifications"; import { MODAL_FRAME_CHROME_ROWS, resolveModalGeometry } from "./modalGeometry"; -import type { ExtensionDocumentDialogRequest } from "./extensionDialogs"; +import type { ExtensionInfoDialogRequest } from "./extensionDialogs"; /** Wrapped body rows that fit one modal body allocation. */ export interface WindowedDialogText { @@ -70,8 +70,8 @@ export function windowDialogLiteralText( return { lines: [...wrapped.slice(0, maxRows - 1), "…"], truncated: true }; } -/** Concrete row allocation shared by document rendering and copy authorization. */ -export interface ExtensionDocumentDialogLayout { +/** Concrete row allocation shared by info rendering and copy authorization. */ +export interface ExtensionInfoDialogLayout { frame: { width: number; height: number }; bodyWidth: number; cardWidth: number; @@ -90,8 +90,8 @@ export interface ExtensionDocumentDialogLayout { copyActionExposed: boolean; } -/** Preserve meaningful text when a constrained document has only one body row. */ -function windowDocumentText(sourceLines: readonly string[], width: number, maxRows: number) { +/** Preserve meaningful text when constrained info has only one body row. */ +function windowInfoText(sourceLines: readonly string[], width: number, maxRows: number) { const windowed = windowDialogText(sourceLines, width, maxRows); if (maxRows !== 1 || !windowed.truncated) return windowed; @@ -99,12 +99,12 @@ function windowDocumentText(sourceLines: readonly string[], width: number, maxRo return { lines: [fitText(`${firstLine}…`, width, "…")], truncated: true }; } -/** Plan a read-only document so rendering and keyboard copy use identical disclosure facts. */ -export function planExtensionDocumentDialog( - request: ExtensionDocumentDialogRequest, +/** Plan read-only info so rendering and keyboard copy use identical disclosure facts. */ +export function planExtensionInfoDialog( + request: ExtensionInfoDialogRequest, terminalWidth: number, terminalHeight: number, -): ExtensionDocumentDialogLayout { +): ExtensionInfoDialogLayout { const width = Math.min(84, Math.max(58, terminalWidth - 8)); const measuredFrame = resolveModalGeometry({ width, @@ -167,7 +167,7 @@ export function planExtensionDocumentDialog( const copyLabelRows = hasCopy && remainingRows > 1 ? 1 : 0; remainingRows -= copyLabelRows; const copyCardRows = hasCopy ? remainingRows : 0; - const visibleBody = windowDocumentText(request.bodyLines, bodyWidth, bodyRows); + const visibleBody = windowInfoText(request.bodyLines, bodyWidth, bodyRows); const visibleCopy = copy ? windowDialogLiteralText( copy.displayLines, diff --git a/src/ui/lib/extensionDialogs.test.ts b/src/ui/lib/extensionDialogs.test.ts index 2b22c825d..cf1909e26 100644 --- a/src/ui/lib/extensionDialogs.test.ts +++ b/src/ui/lib/extensionDialogs.test.ts @@ -51,11 +51,11 @@ describe("createExtensionDialogQueue", () => { queue.accept(queue.current()!.id); expect(await valueless).toBeNull(); - const document = dialogs.document({ title: "Guide", body: "Read this." }); + const info = dialogs.info({ title: "Guide", body: "Read this." }); queue.accept(queue.current()!.id); - expect(queue.current()).toMatchObject({ kind: "document", title: "Guide" }); + expect(queue.current()).toMatchObject({ kind: "info", title: "Guide" }); queue.cancel(queue.current()!.id); - expect(await document).toBeUndefined(); + expect(await info).toBeUndefined(); }); test("ignores an answer aimed at a dialog that is no longer current", async () => { @@ -121,18 +121,18 @@ describe("createExtensionDialogQueue", () => { expect(queue.current()).toMatchObject({ title: "Pick", options: ["opt"] }); }); - test("uses the same terminal-safe text for document display and clipboard payloads", () => { + test("uses the same terminal-safe text for info display and clipboard payloads", () => { const queue = createExtensionDialogQueue(); const dialogs = queue.createDialogs("guide"); - void dialogs.document({ + void dialogs.info({ title: "Setup", body: "one\n\u001b[31mtwo\u001b[0m", copy: { label: "Prompt", text: "copy\t\u001b[31mexactly\u001b[0m" }, }); expect(queue.current()).toMatchObject({ - kind: "document", + kind: "info", bodyLines: ["one", "two"], copy: { label: "Prompt", @@ -146,7 +146,7 @@ describe("createExtensionDialogQueue", () => { const queue = createExtensionDialogQueue(); const dialogs = queue.createDialogs("guide"); - void dialogs.document({ + void dialogs.info({ title: "Setup", copy: { text: "copy this exactly", displayLines: ["copy this", "exactly"] }, }); @@ -156,7 +156,7 @@ describe("createExtensionDialogQueue", () => { queue.cancelAll(); await expect( - dialogs.document({ + dialogs.info({ title: "Setup", copy: { text: "safe text", displayLines: ["different text"] }, }), @@ -269,18 +269,18 @@ describe("createExtensionDialogQueue", () => { await expect(dialogs.select({ title: "Which?", options: [] })).rejects.toThrow( /at least one option/, ); - await expect(dialogs.document({ title: "Empty" })).rejects.toThrow(/body or copy content/); - await expect(dialogs.document({ title: "Bad copy", copy: { text: "" } })).rejects.toThrow( + await expect(dialogs.info({ title: "Empty" })).rejects.toThrow(/body or copy content/); + await expect(dialogs.info({ title: "Bad copy", copy: { text: "" } })).rejects.toThrow( /non-empty string/, ); await expect( - dialogs.document({ title: "Long body", body: Array(101).fill("line").join("\n") }), + dialogs.info({ title: "Long body", body: Array(101).fill("line").join("\n") }), ).rejects.toThrow(/at most 100 lines/); await expect( - dialogs.document({ title: "Long copy", copy: { text: "x".repeat(16_385) } }), + dialogs.info({ title: "Long copy", copy: { text: "x".repeat(16_385) } }), ).rejects.toThrow(/at most 16384 characters/); await expect( - dialogs.document({ title: "Expanded copy", copy: { text: "\t".repeat(4_097) } }), + dialogs.info({ title: "Expanded copy", copy: { text: "\t".repeat(4_097) } }), ).rejects.toThrow(/normalized copy.text.*at most 16384 characters/); await expect( dialogs.select({ title: "Which?", options: [1 as unknown as string] }), diff --git a/src/ui/lib/extensionDialogs.ts b/src/ui/lib/extensionDialogs.ts index db8f09a7c..92266ebb7 100644 --- a/src/ui/lib/extensionDialogs.ts +++ b/src/ui/lib/extensionDialogs.ts @@ -12,7 +12,7 @@ import type { ExtensionConfirmOptions, ExtensionDialogs, - ExtensionDocumentOptions, + ExtensionInfoOptions, ExtensionInputOptions, ExtensionSelectOptions, } from "../../extension-api/types"; @@ -27,14 +27,14 @@ 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; -/** Body lines one read-only document may retain before host-side windowing. */ -const MAX_DOCUMENT_BODY_LINES = 100; +/** Body lines one read-only info dialog may retain before host-side windowing. */ +const MAX_INFO_BODY_LINES = 100; /** Clipboard text is bounded before it reaches the terminal's OSC 52 channel. */ -const MAX_DOCUMENT_COPY_TEXT_LENGTH = 16_384; +const MAX_INFO_COPY_TEXT_LENGTH = 16_384; -/** Default heading for a document's copyable text card. */ -const DEFAULT_DOCUMENT_COPY_LABEL = "Content"; +/** Default heading for an info dialog's copyable text card. */ +const DEFAULT_INFO_COPY_LABEL = "Content"; /** What every queued dialog carries, whatever kind it is. */ interface ExtensionDialogRequestBase { @@ -71,18 +71,18 @@ export interface ExtensionInputDialogRequest extends ExtensionDialogRequestBase initial: string; } -/** Clipboard and display forms of one document's normalized copyable text. */ -export interface ExtensionDocumentCopyRequest { +/** Clipboard and display forms of one info dialog's normalized copyable text. */ +export interface ExtensionInfoCopyRequest { label: string; text: string; displayLines: string[]; } -/** One normalized read-only document the host should draw. */ -export interface ExtensionDocumentDialogRequest extends ExtensionDialogRequestBase { - kind: "document"; +/** One normalized read-only info dialog the host should draw. */ +export interface ExtensionInfoDialogRequest extends ExtensionDialogRequestBase { + kind: "info"; bodyLines: string[]; - copy: ExtensionDocumentCopyRequest | null; + copy: ExtensionInfoCopyRequest | null; } /** One dialog the host should draw, normalized from what an extension asked for. */ @@ -90,7 +90,7 @@ export type ExtensionDialogRequest = | ExtensionConfirmDialogRequest | ExtensionSelectDialogRequest | ExtensionInputDialogRequest - | ExtensionDocumentDialogRequest; + | ExtensionInfoDialogRequest; /** What a dialog hands back to the awaiting handler. */ type ExtensionDialogResult = boolean | string | null | undefined; @@ -108,7 +108,7 @@ export interface ExtensionDialogQueue { * 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. Documents + * there is nothing to hand back, so it settles as a cancel instead. Info dialogs * ignore acceptance and remain visible until cancelled. * * Answering anything but the current dialog is ignored: an answer computed @@ -180,57 +180,55 @@ function normalizeBodyLines(body: unknown, maxLines = MAX_CONFIRM_BODY_LINES) { .map((line) => sanitizeTerminalLine(line)); } -/** Normalize a document body while rejecting content the host would have to discard. */ -function normalizeDocumentBodyLines(body: unknown) { +/** Normalize an info body while rejecting content the host would have to discard. */ +function normalizeInfoBodyLines(body: unknown) { if (typeof body !== "string" || body.length === 0) return []; const lines = body.split("\n"); - if (lines.length > MAX_DOCUMENT_BODY_LINES) { - invalid("document", `body must contain at most ${MAX_DOCUMENT_BODY_LINES} lines.`); + if (lines.length > MAX_INFO_BODY_LINES) { + invalid("info", `body must contain at most ${MAX_INFO_BODY_LINES} lines.`); } return lines.map((line) => sanitizeTerminalLine(line)); } -/** Validate and normalize a document's optional clipboard card. */ -function normalizeDocumentCopy( - copy: ExtensionDocumentOptions["copy"], -): ExtensionDocumentCopyRequest | null { +/** Validate and normalize an info dialog's optional clipboard card. */ +function normalizeInfoCopy(copy: ExtensionInfoOptions["copy"]): ExtensionInfoCopyRequest | null { if (copy === undefined) return null; if (!copy || typeof copy.text !== "string" || copy.text.length === 0) { - invalid("document", "copy.text must be a non-empty string."); + invalid("info", "copy.text must be a non-empty string."); } - if (copy.text.length > MAX_DOCUMENT_COPY_TEXT_LENGTH) { - invalid("document", `copy.text must be at most ${MAX_DOCUMENT_COPY_TEXT_LENGTH} characters.`); + if (copy.text.length > MAX_INFO_COPY_TEXT_LENGTH) { + invalid("info", `copy.text must be at most ${MAX_INFO_COPY_TEXT_LENGTH} characters.`); } const text = sanitizeTerminalText(copy.text).replaceAll("\t", " "); - if (text.length > MAX_DOCUMENT_COPY_TEXT_LENGTH) { + if (text.length > MAX_INFO_COPY_TEXT_LENGTH) { invalid( - "document", - `normalized copy.text must be at most ${MAX_DOCUMENT_COPY_TEXT_LENGTH} characters.`, + "info", + `normalized copy.text must be at most ${MAX_INFO_COPY_TEXT_LENGTH} characters.`, ); } if (text.length === 0) { - invalid("document", "copy.text must contain visible or whitespace content."); + invalid("info", "copy.text must contain visible or whitespace content."); } let displayLines = text.split("\n").map((line) => sanitizeTerminalLine(line)); if (copy.displayLines !== undefined) { if (!Array.isArray(copy.displayLines) || copy.displayLines.length === 0) { - invalid("document", "copy.displayLines must be a non-empty string array."); + invalid("info", "copy.displayLines must be a non-empty string array."); } displayLines = copy.displayLines.map((line) => { if (typeof line !== "string" || line.includes("\n")) { - invalid("document", "copy.displayLines must contain single-line strings."); + invalid("info", "copy.displayLines must contain single-line strings."); } return sanitizeTerminalLine(line).replaceAll("\t", " "); }); if (displayLines.join(" ") !== text && displayLines.join("\n") !== text) { - invalid("document", "copy.displayLines must contain the same text as copy.text."); + invalid("info", "copy.displayLines must contain the same text as copy.text."); } } return { - label: normalizeLabel(copy.label, DEFAULT_DOCUMENT_COPY_LABEL), + label: normalizeLabel(copy.label, DEFAULT_INFO_COPY_LABEL), text, displayLines, }; @@ -278,7 +276,7 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { /** The cancel value one request resolves with. */ const cancelValueFor = (request: ExtensionDialogRequest): ExtensionDialogResult => - request.kind === "confirm" ? false : request.kind === "document" ? undefined : null; + request.kind === "confirm" ? false : request.kind === "info" ? undefined : null; /** * Queue one request and hand back the promise its handler awaits. @@ -392,16 +390,16 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { isLive, ); }, - async document(options: ExtensionDocumentOptions) { - const title = normalizeTitle("document", options?.title); - const bodyLines = normalizeDocumentBodyLines(options.body); - const copy = normalizeDocumentCopy(options.copy); + async info(options: ExtensionInfoOptions) { + const title = normalizeTitle("info", options?.title); + const bodyLines = normalizeInfoBodyLines(options.body); + const copy = normalizeInfoCopy(options.copy); if (bodyLines.length === 0 && copy === null) { - invalid("document", "requires body or copy content."); + invalid("info", "requires body or copy content."); } await enqueue( (id) => ({ - kind: "document", + kind: "info", id, extensionId, showAttribution, @@ -436,7 +434,7 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { return; } - if (active.request.kind === "document") { + if (active.request.kind === "info") { return; } diff --git a/test/pty/chrome.test.ts b/test/pty/chrome.test.ts index 5f2b0b989..3ef45a381 100644 --- a/test/pty/chrome.test.ts +++ b/test/pty/chrome.test.ts @@ -84,7 +84,7 @@ describe("PTY chrome", () => { } }); - test("the Agent menu opens bundled skill guidance as a modal document", async () => { + test("the Agent menu opens bundled skill guidance as an info dialog", async () => { const fixture = harness.createTwoFileRepoFixture(); const session = await harness.launchHunk({ args: ["diff", "--mode", "split"], @@ -100,7 +100,7 @@ describe("PTY chrome", () => { expect(menu).toContain("Next annotated file"); await session.click(/Agent skill/); - const document = await harness.waitForSnapshot( + const info = await harness.waitForSnapshot( session, (text) => text.includes("Teach your agent how to review this Hunk session.") && @@ -108,7 +108,7 @@ describe("PTY chrome", () => { text.includes("⧉ Copy prompt"), 5_000, ); - expect(document).not.toContain("ext hunk"); + expect(info).not.toContain("ext hunk"); await session.press("enter"); const stillOpen = await session.text({ immediate: true }); diff --git a/website/src/content/docs/docs/extend/extension-api.md b/website/src/content/docs/docs/extend/extension-api.md index 8aea478cc..45b368a74 100644 --- a/website/src/content/docs/docs/extend/extension-api.md +++ b/website/src/content/docs/docs/extend/extension-api.md @@ -8,7 +8,7 @@ The extension factory receives one API object. Registration calls are only valid ## `hunk.apiVersion` The API generation this Hunk speaks (currently `17`). Branch on it if you want -one file to support several Hunk versions. Version 17 adds read-only document +one file to support several Hunk versions. Version 17 adds read-only info dialogs with optional clipboard actions; version 16 added temporary application handoffs and on-disk location resolution to command handlers; version 15 added `{ side, line }` to opted-in pane @@ -312,7 +312,7 @@ hunk.registerCommand( ); ``` -`document` presents read-only guidance rather than asking for an answer. At +`info` presents read-only guidance rather than asking for an answer. At least `body` or `copy` must be present. A body may contain up to 100 source lines, and copy text may contain up to 16,384 JavaScript string code units. When `copy` is provided, `c` and the clickable copy action send its `text` to @@ -326,7 +326,7 @@ what the action copies: ```ts hunk.registerCommand({ id: "agent-setup", title: "Agent setup" }, async (ctx) => { - await ctx.dialogs.document({ + await ctx.dialogs.info({ title: "Agent setup", body: "Give this prompt to your coding agent.", copy: { @@ -361,7 +361,7 @@ 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. -One dialog shows at a time; concurrent requests queue in call order, across extensions. Escape cancels (`false` or `null`) or closes a document. Enter accepts interactive dialogs but leaves read-only documents open; 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 an info dialog. Enter accepts interactive dialogs but leaves read-only info dialogs open; 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. ### Temporary applications From 7c92edb88a7327aec00f281548e0d842b23cb531 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Tue, 1 Sep 2026 01:12:44 -0400 Subject: [PATCH 8/9] feat(extensions): add custom dialog surfaces --- .changeset/fuzzy-agents-guide.md | 2 +- docs/extension-architecture.md | 12 +- docs/extensions.md | 88 ++-- skills/hunk-extensions/SKILL.md | 5 +- src/extension-api/index.ts | 6 +- src/extension-api/types.ts | 68 +-- .../default/ui/agentSkill/index.test.ts | 23 +- src/extensions/default/ui/agentSkill/index.ts | 33 -- .../default/ui/agentSkill/index.tsx | 136 ++++++ src/extensions/events.test.ts | 2 +- src/extensions/events.ts | 2 +- src/extensions/types.ts | 6 +- src/ui/App.tsx | 139 ++++-- src/ui/AppHost.extension-dialogs.test.tsx | 404 +++++++++++++++++- src/ui/AppHost.key-routing.test.tsx | 68 +++ src/ui/components/chrome/ExtensionDialog.tsx | 266 ++++++------ src/ui/components/panes/DiffPane.tsx | 5 +- src/ui/hooks/useAppKeyboardShortcuts.ts | 34 +- .../useExtensionDialogController.test.tsx | 39 +- src/ui/hooks/useExtensionDialogController.ts | 14 +- src/ui/lib/extensionDialogGeometry.test.ts | 117 ++--- src/ui/lib/extensionDialogGeometry.ts | 178 +------- src/ui/lib/extensionDialogs.test.ts | 77 ++-- src/ui/lib/extensionDialogs.ts | 146 +++---- src/ui/lib/keyboard.ts | 15 - src/ui/lib/ui-lib.test.ts | 15 +- test/pty/chrome.test.ts | 2 +- .../content/docs/docs/extend/extension-api.md | 63 +-- 28 files changed, 1217 insertions(+), 748 deletions(-) delete mode 100644 src/extensions/default/ui/agentSkill/index.ts create mode 100644 src/extensions/default/ui/agentSkill/index.tsx diff --git a/.changeset/fuzzy-agents-guide.md b/.changeset/fuzzy-agents-guide.md index 63683f74c..54cf4e668 100644 --- a/.changeset/fuzzy-agents-guide.md +++ b/.changeset/fuzzy-agents-guide.md @@ -2,4 +2,4 @@ "hunkdiff": minor --- -Add fully disclosed copyable info dialogs to the extension API and run Hunk's Agent Skill onboarding as a bundled extension. +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 9d00a6423..5e637a04f 100644 --- a/docs/extension-architecture.md +++ b/docs/extension-architecture.md @@ -236,12 +236,13 @@ 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, input, and read-only copyable documents are `ModalFrame` surfaces), and +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 @@ -255,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 f3a86fa68..9e38ecaf5 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -281,8 +281,8 @@ new instances and run that shutdown/startup pair around the replacement. ### `hunk.apiVersion` The API generation this Hunk speaks (currently `17`). Branch on it if you want -one file to support several Hunk versions. Version 17 adds read-only info -dialogs with optional clipboard actions; version 16 added temporary application +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` @@ -1631,7 +1631,7 @@ 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` -- `document({ title, body?, copy?: { label?, text } })` → `void` when closed +- `open({ title, width?, height?, component })` → `void` when closed ```ts hunk.registerCommand( @@ -1675,43 +1675,73 @@ hunk.registerCommand({ id: "pick-hunk", title: "Pick a hunk", key: "ctrl+k" }, a }); ``` -`info` presents read-only guidance rather than asking for an answer. At -least `body` or `copy` must be present. A body may contain up to 100 source -lines, and copy text may contain up to 16,384 JavaScript string code units. -When `copy` is provided, `c` and the clickable copy action send its `text` to -the terminal clipboard while the host removes terminal control sequences, -expands tabs to four spaces, and renders the same safe value under `label` -(default `Content`). Hunk exposes those actions only while the complete payload -and any required extension attribution are visible. Optional `displayLines` can -add authored visual breaks; -after sanitizing, those lines must rejoin with spaces or newlines to exactly the -clipboard `text`, so a preview cannot disguise what the action copies: +`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"} + + + + ); +} -```ts hunk.registerCommand({ id: "agent-setup", title: "Agent setup" }, async (ctx) => { - await ctx.dialogs.info({ + await ctx.dialogs.open({ title: "Agent setup", - body: "Give this prompt to your coding agent.", - copy: { - label: "Prompt", - text: "Review the current Hunk session. Focus on correctness.", - displayLines: ["Review the current Hunk session.", "Focus on correctness."], - }, + width: 64, + height: 6, + component: AgentSetupDialog, }); }); ``` -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. +`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 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 an info dialog, Enter accepts the confirm action, highlighted -option, or typed text, and review shortcuts stay suppressed underneath. -Info dialogs ignore Enter and remain open. Confirm dialogs also answer to `y`/`n`, +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 diff --git a/skills/hunk-extensions/SKILL.md b/skills/hunk-extensions/SKILL.md index 1c14e9548..6cb56abd5 100644 --- a/skills/hunk-extensions/SKILL.md +++ b/skills/hunk-extensions/SKILL.md @@ -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`/`info`, 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 43df735bb..73bb76fe4 100644 --- a/src/extension-api/index.ts +++ b/src/extension-api/index.ts @@ -95,8 +95,10 @@ export type { ExtensionReviewSnapshotNote, ExtensionReviewSnapshotNoteAnchor, ExtensionConfirmOptions, - ExtensionInfoCopyOptions, - ExtensionInfoOptions, + ExtensionDialogActions, + ExtensionDialogComponent, + ExtensionDialogOptions, + ExtensionDialogProps, ExtensionDialogs, ExtensionInputOptions, ExtensionSelectOptions, diff --git a/src/extension-api/types.ts b/src/extension-api/types.ts index b962282d1..53acaabd8 100644 --- a/src/extension-api/types.ts +++ b/src/extension-api/types.ts @@ -1631,45 +1631,61 @@ export interface ExtensionInputOptions { initial?: string; } -/** Copyable text shown inside an info dialog. */ -export interface ExtensionInfoCopyOptions { - /** Short heading shown above the copyable text. Defaults to "Content". */ - label?: string; - /** - * Text copied after Hunk removes terminal control sequences and expands tabs - * to four spaces. Limited to 16,384 JavaScript string code units. - */ - text: string; +/** Actions available while an extension-owned dialog component is mounted. */ +export interface ExtensionDialogActions { + /** Close this dialog and resolve its `open` promise. */ + close(): void; /** - * Optional authored display rows for `text`. Their sanitized contents must - * rejoin with spaces or newlines to the sanitized clipboard text, so they - * can control wrapping without presenting different content. + * 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. */ - displayLines?: readonly string[]; + 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; } -/** Read-only information shown to the user in a modal. */ -export interface ExtensionInfoOptions { +/** 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; - /** Optional prose shown above the copyable text. Limited to 100 source lines. */ - body?: string; - /** Optional text card the user can copy with `c` or the mouse. */ - copy?: ExtensionInfoCopyOptions; + /** 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; } /** * 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 dismisses, resolving the cancel value (`false`, `null`, or * `undefined`). Enter accepts: the confirm action, the highlighted option, or - * the typed text. Info dialogs are read-only and remain open until dismissed. + * 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 @@ -1678,7 +1694,7 @@ export interface ExtensionInfoOptions { * * Bad arguments are a programming error rather than a user answer, so they * reject instead of resolving: a missing or blank `title`, a `select` with no - * options, or info content outside its documented bounds. Because a dialog + * 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. @@ -1690,8 +1706,8 @@ export interface ExtensionDialogs { select(options: ExtensionSelectOptions): Promise; /** Resolves the submitted text, or null on cancel/escape. */ input(options: ExtensionInputOptions): Promise; - /** Show read-only guidance until the user dismisses it. */ - info(options: ExtensionInfoOptions): 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 index bfe576bf5..556b3fa77 100644 --- a/src/extensions/default/ui/agentSkill/index.test.ts +++ b/src/extensions/default/ui/agentSkill/index.test.ts @@ -1,11 +1,7 @@ import { describe, expect, mock, test } from "bun:test"; import type { ExtensionCommandContext } from "hunkdiff/extension"; import { getBundledUIRegistry } from ".."; -import { - AGENT_SKILL_PROMPT, - AGENT_SKILL_PROMPT_ROWS, - BUNDLED_AGENT_SKILL_COMMAND_FULL_ID, -} from "."; +import { AgentSkillDialog, BUNDLED_AGENT_SKILL_COMMAND_FULL_ID } from "."; /** Return the agent-skill registration from the process-static bundled UI registry. */ function getBundledAgentSkillCommand() { @@ -28,20 +24,17 @@ describe("bundled agent skill extension", () => { }); }); - test("opens its onboarding through the public info dialog", async () => { - const info = mock(async () => {}); - const context = { dialogs: { info } } as unknown as ExtensionCommandContext; + 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(info).toHaveBeenCalledWith({ + expect(open).toHaveBeenCalledWith({ title: "Agent skill", - body: "Teach your agent how to review this Hunk session.", - copy: { - label: "Prompt", - text: AGENT_SKILL_PROMPT, - displayLines: AGENT_SKILL_PROMPT_ROWS, - }, + width: 80, + height: 9, + component: AgentSkillDialog, }); }); }); diff --git a/src/extensions/default/ui/agentSkill/index.ts b/src/extensions/default/ui/agentSkill/index.ts deleted file mode 100644 index e4118d645..000000000 --- a/src/extensions/default/ui/agentSkill/index.ts +++ /dev/null @@ -1,33 +0,0 @@ -import 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}`; - -/** 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.info({ - title: "Agent skill", - body: "Teach your agent how to review this Hunk session.", - copy: { - label: "Prompt", - text: AGENT_SKILL_PROMPT, - displayLines: AGENT_SKILL_PROMPT_ROWS, - }, - }); - }, - ); -}; - -export default registerBundledAgentSkill; 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/events.test.ts b/src/extensions/events.test.ts index fd147580f..ff0823b15 100644 --- a/src/extensions/events.test.ts +++ b/src/extensions/events.test.ts @@ -195,7 +195,7 @@ describe("extension event dispatch", () => { confirm: async () => false, select: async () => null, input: async () => null, - info: async () => {}, + open: async () => {}, }, events: { emit: () => {} }, }; diff --git a/src/extensions/events.ts b/src/extensions/events.ts index 908c4fcd5..97be5f1eb 100644 --- a/src/extensions/events.ts +++ b/src/extensions/events.ts @@ -343,7 +343,7 @@ function unavailableDialogs(result: ExtensionLoadResult, extensionId: string): E unavailable(); return null; }, - info: async () => { + open: async () => { unavailable(); }, }; diff --git a/src/extensions/types.ts b/src/extensions/types.ts index c9aba24e7..cdb4bb4f3 100644 --- a/src/extensions/types.ts +++ b/src/extensions/types.ts @@ -44,8 +44,10 @@ export type { ExtensionContext, ExtensionCustomEventHandler, ExtensionDiffFile, - ExtensionInfoCopyOptions, - ExtensionInfoOptions, + ExtensionDialogActions, + ExtensionDialogComponent, + ExtensionDialogOptions, + ExtensionDialogProps, ExtensionEventBus, ExtensionEventContext, ExtensionEventHandler, diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 4bed8f3e5..734525421 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"; @@ -89,7 +90,7 @@ import { } from "./lib/appCommands"; import { buildAppMenus } from "./lib/appMenus"; import { buildExtensionAppCommands, extensionCommandKeyDefaults } from "./lib/extensionCommands"; -import { planExtensionInfoDialog } from "./lib/extensionDialogGeometry"; +import { normalizeExtensionDialogClipboardText } from "./lib/extensionDialogs"; import type { CurrentLineAlignment } from "./lib/hunkScroll"; import type { LineCursor } from "./lib/lineCursors"; import { useFilePresentationController } from "./fileViews/useFilePresentationController"; @@ -205,6 +206,8 @@ export function App({ 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; @@ -490,8 +493,11 @@ export function App({ const { accept: acceptExtensionDialog, cancel: cancelExtensionDialog, + cancelRequest: cancelExtensionDialogRequest, cancelAll: cancelAllExtensionDialogs, createDialogs: createQueuedExtensionDialogs, + getCurrentRequest: getCurrentExtensionDialogRequest, + isCurrentRequestLive: isCurrentExtensionDialogRequestLive, inputValue: extensionDialogInputValue, moveSelection: moveExtensionDialogSelection, pickOption: setExtensionDialogSelectedIndex, @@ -519,6 +525,7 @@ export function App({ // 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 @@ -526,7 +533,13 @@ export function App({ showAttribution: !bundled, }); }, - [createQueuedExtensionDialogs, createReviewCapabilityLease, extensionAppController, extensions], + [ + bootstrap, + createQueuedExtensionDialogs, + createReviewCapabilityLease, + extensionAppController, + extensions, + ], ); const extensionWorkspaceController = useExtensionWorkspaceControls({ @@ -936,38 +949,100 @@ export function App({ runExtensionCommand(bundledAgentSkillCommand); }, [bundledAgentSkillCommand, runExtensionCommand]); - const extensionInfoLayout = - extensionDialog?.kind === "info" - ? planExtensionInfoDialog(extensionDialog, terminal.width, terminal.height) - : null; const extensionDialogCopySupported = (renderer.isOsc52Supported?.() ?? false) && typeof renderer.copyToClipboardOSC52 === "function"; - const extensionInfoCopyExposed = extensionInfoLayout?.copyActionExposed ?? false; - - /** Copy an info dialog's normalized payload through the terminal clipboard integration. */ - const copyExtensionDialogInfo = useCallback(() => { - if (extensionDialog?.kind !== "info" || !extensionDialog.copy || !extensionInfoCopyExposed) { - return; + 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; + } + + const normalized = normalizeExtensionDialogClipboardText(text); + return normalized !== null && renderer.copyToClipboardOSC52(normalized); + }, + [getCurrentExtensionDialogRequest, isCurrentExtensionDialogRequestLive, renderer], + ); - if (extensionDialogCopySupported) { - const copied = renderer.copyToClipboardOSC52!(extensionDialog.copy.text); - if (!copied) { - showTransientNotice("Clipboard copy failed"); + /** 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( - `Copied ${extensionDialog.title.toLowerCase()} ${extensionDialog.copy.label.toLowerCase()} to clipboard`, + current.showAttribution ? `Extension ${current.extensionId}: ${safeMessage}` : safeMessage, ); - return; - } - }, [ - extensionDialog, - extensionDialogCopySupported, - extensionInfoCopyExposed, - renderer, - showTransientNotice, - ]); + }, + [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(() => { @@ -1163,8 +1238,6 @@ export function App({ extensionDialog, acceptExtensionDialog, cancelExtensionDialog, - extensionInfoCopyEnabled: extensionInfoCopyExposed && extensionDialogCopySupported, - copyExtensionDialogInfo, moveExtensionDialogSelection, extensionTrustPromptOpen, trustRepoExtensions, @@ -1369,7 +1442,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} @@ -1432,7 +1506,7 @@ export function App({ {statusBarVisible ? ( copyExtensionDialogInfo()} + onClose={closeExtensionDialogComponent} + onCopy={copyExtensionDialogText} + onNotify={notifyExtensionDialog} onPickOption={setExtensionDialogSelectedIndex} + onRenderFailure={reportExtensionDialogRenderFailure} /> ) : null} diff --git a/src/ui/AppHost.extension-dialogs.test.tsx b/src/ui/AppHost.extension-dialogs.test.tsx index d7e19bd02..5aff4c829 100644 --- a/src/ui/AppHost.extension-dialogs.test.tsx +++ b/src/ui/AppHost.extension-dialogs.test.tsx @@ -204,6 +204,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,15 +342,15 @@ describe("extension dialogs", () => { }); }); - test("an info dialog renders copyable guidance and closes only on escape", async () => { - const repo = createTestRepo("hunk-ext-dialog-info-"); - const extDir = createTempDir("hunk-ext-dialog-info-ext-"); + 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.info({ title: "Agent setup", body: "Teach your agent how to review this Hunk session.", copy: { label: "Prompt", text: "Load the Hunk skill and use it for this review. Run hunk skill path to get the skill path." } })`, + `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); @@ -345,7 +370,7 @@ describe("extension dialogs", () => { await flushUntil( setup, () => setup.captureCharFrame().includes("Agent setup"), - "the info dialog to open", + "the component dialog to open", ); const frame = setup.captureCharFrame(); @@ -377,7 +402,7 @@ describe("extension dialogs", () => { await flushUntil( setup, () => readProbeLog(logPath).includes("answer undefined"), - "the info handler to finish", + "the component-dialog handler to finish", ); }, undefined, @@ -385,15 +410,126 @@ describe("extension dialogs", () => { ); }); - test("withholds copy when a short terminal cannot disclose the complete payload", async () => { - const repo = createTestRepo("hunk-ext-dialog-info-short-"); - const extDir = createTempDir("hunk-ext-dialog-info-short-ext-"); + 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.info({ title: "Agent setup", body: "Teach your agent how to review this Hunk session.", copy: { label: "Prompt", text: "Load the Hunk skill and use it for this review. Run hunk skill path to get the skill path." } })`, + `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); @@ -413,12 +549,11 @@ describe("extension dialogs", () => { await flushUntil( setup, () => setup.captureCharFrame().includes("Agent setup"), - "the constrained info dialog to open", + "the constrained component dialog to open", ); const frame = setup.captureCharFrame(); expect(frame).toContain("ext ext"); - expect(frame).toContain("…"); expect(frame).not.toContain("Copy prompt"); await act(async () => { @@ -433,15 +568,15 @@ describe("extension dialogs", () => { ); }); - test("an unavailable info copy action is visible but inert", async () => { - const repo = createTestRepo("hunk-ext-dialog-info-unavailable-"); - const extDir = createTempDir("hunk-ext-dialog-info-unavailable-ext-"); + 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.info({ title: "Copy setup", copy: { label: "Prompt", text: "copy me" } })`, + `ctx.dialogs.open({ title: "Copy setup", width: 46, height: 6, component: createCopyDialog("Copy this text.", "Prompt", "copy me") })`, ); const bootstrap = await launchWithExtension(repo, extPath); @@ -479,20 +614,20 @@ describe("extension dialogs", () => { await flushUntil( setup, () => readProbeLog(logPath).includes("answer undefined"), - "the unavailable-copy info dialog to close", + "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-info-copy-failure-"); - const extDir = createTempDir("hunk-ext-dialog-info-copy-failure-ext-"); + 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.info({ title: "Copy setup", copy: { label: "Prompt", text: "copy me" } })`, + `ctx.dialogs.open({ title: "Copy setup", width: 46, height: 6, component: createCopyDialog("Copy this text.", "Prompt", "copy me") })`, ); const bootstrap = await launchWithExtension(repo, extPath); @@ -523,7 +658,172 @@ describe("extension dialogs", () => { ); expect(attempts).toBe(1); - expect(setup.captureCharFrame()).not.toContain("Copied copy setup prompt to clipboard"); + 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: () => { throw new Error("surface exploded"); } })`, + ); + + 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("Dialog unavailable"), + "the custom-dialog fallback to render", + ); + + const failed = setup.captureCharFrame(); + expect(failed).toContain("Broken surface"); + expect(failed).toContain("surface exploded"); + + await act(async () => { + await setup.mockInput.pressEscape(); + }); + await flushUntil( + setup, + () => readProbeLog(logPath).includes("answer undefined"), + "the failed 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", + ); }); }); @@ -780,6 +1080,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.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/ExtensionDialog.tsx b/src/ui/components/chrome/ExtensionDialog.tsx index e8623f9a6..addfa214a 100644 --- a/src/ui/components/chrome/ExtensionDialog.tsx +++ b/src/ui/components/chrome/ExtensionDialog.tsx @@ -1,16 +1,19 @@ -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, - ExtensionInfoCopyRequest, - ExtensionInfoDialogRequest, ExtensionInputDialogRequest, + ExtensionOpenDialogRequest, ExtensionSelectDialogRequest, } from "../../lib/extensionDialogs"; -import { planExtensionInfoDialog, windowDialogText } from "../../lib/extensionDialogGeometry"; +import { planExtensionOpenDialog, windowDialogText } from "../../lib/extensionDialogGeometry"; import { extensionToastPrefix } from "../../lib/extensionNotifications"; import { listWindowStart } from "../../lib/listWindow"; import { MODAL_FRAME_CHROME_ROWS, resolveModalGeometry } from "../../lib/modalGeometry"; -import { fitText, measureTextWidth, padText } from "../../lib/text"; +import { toExtensionPaintTheme } from "../../lib/extensionPaintTheme"; +import { fitText, padText } from "../../lib/text"; import type { AppTheme } from "../../themes"; import { ConfirmDialog, confirmDialogHeight, DialogActionRow } from "./ConfirmDialog"; import { ModalFrame } from "./ModalFrame"; @@ -38,14 +41,27 @@ 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, - onCopyInfo, + onClose, + onCopy, + onNotify, onPickOption, + onRenderFailure, request, selectedIndex, terminalHeight, @@ -58,21 +74,27 @@ export function ExtensionDialog({ onAccept: (selectedIndexOverride?: number) => void; onCancel: () => void; onChangeInput: (value: string) => void; - onCopyInfo: (copy: ExtensionInfoCopyRequest) => 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; + onRenderFailure: (requestId: number, error: unknown) => void; request: ExtensionDialogRequest; selectedIndex: number; terminalHeight: number; terminalWidth: number; theme: AppTheme; }) { - if (request.kind === "info") { + if (request.kind === "open") { return ( - 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.onError(error); + } + 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, - onCopyInfo, + onClose, + onCopy, + onNotify, + onRenderFailure, request, terminalHeight, terminalWidth, @@ -168,30 +221,63 @@ function ExtensionInfoDialog({ }: { copySupported: boolean; onCancel: () => void; - onCopyInfo: (copy: ExtensionInfoCopyRequest) => void; - request: ExtensionInfoDialogRequest; + 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 copy = request.copy; - const layout = planExtensionInfoDialog(request, terminalWidth, terminalHeight); - const { - actionGapRows, - actionRows, - attributionGapRows, - attributionRows, - bodyCopyGapRows, - bodyWidth, - cardTextWidth, - cardWidth, - copyActionExposed, - copyCardRows, - copyLabelRows, - frame, - visibleBody, - visibleCopy, - } = layout; + 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 actions = useMemo( + () => + Object.freeze({ + close: () => onClose(request.id), + copy: (text: string) => onCopy(request.id, text), + notify: (message: string) => onNotify(request.id, message), + }), + [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) => ( + + {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} - {visibleBody.lines.map((line, index) => ( - - {fitText(line, bodyWidth)} - - ))} - {bodyCopyGapRows > 0 ? : null} - {copy && copyLabelRows > 0 ? ( - - {fitText(copy.label, bodyWidth - 1)} - - ) : null} - {copy && copyCardRows > 0 ? ( - - = 3 - ? { - border: true, - borderColor: theme.border, - paddingLeft: 1, - paddingRight: 1, - } - : {}), - }} - > - {visibleCopy.lines.map((line, index) => ( - - {fitText(line, cardTextWidth)} - - ))} - - - ) : null} - {actionGapRows > 0 ? : null} - {actionRows > 0 && copy && copyActionExposed ? ( - - ) : actionRows > 0 ? ( - - ) : null} - + {attributionRows > 0 ? ( + + {layout.attributionText} + + ) : null} + {attributionGapRows > 0 ? : null} + {componentHeight > 0 ? ( + Dialog unavailable, true)} + onError={(error) => { + onRenderFailure(request.id, error); + }} + > + {componentBox()} + + ) : ( + + )} ); } -/** Render the compact copy affordance beneath an info card. */ -function InfoCopyAction({ - copy, - copySupported, - onCopyInfo, - theme, - width, -}: { - copy: ExtensionInfoCopyRequest; - copySupported: boolean; - onCopyInfo: (copy: ExtensionInfoCopyRequest) => void; - theme: AppTheme; - width: number; -}) { - const label = fitText( - copySupported ? ` ⧉ Copy ${copy.label.toLowerCase()} ` : " Copy unavailable ", - width, - "…", - ); - return ( - - { - event.stopPropagation(); - if (copySupported) onCopyInfo(copy); - }} - > - {label} - - {padText("", Math.max(0, width - measureTextWidth(label)))} - - ); -} - /** 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 50f38f3b1..89f6ba095 100644 --- a/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/src/ui/hooks/useAppKeyboardShortcuts.ts @@ -15,7 +15,7 @@ import { } from "../lib/appCommands"; import type { ExtensionDialogRequest } from "../lib/extensionDialogs"; import { toExtensionKeyEvent } from "../lib/extensionKeyEvent"; -import { isEscapeKey, isSaveDraftNoteKey, isUnmodifiedKey } from "../lib/keyboard"; +import { isEscapeKey, isSaveDraftNoteKey } from "../lib/keyboard"; import { routeKeyOwnership, type KeyOwner } from "../lib/keyRouting"; type FocusArea = "files" | "filter" | "note"; @@ -39,9 +39,6 @@ export interface UseAppKeyboardShortcutsOptions { extensionDialog: ExtensionDialogRequest | null; acceptExtensionDialog: () => void; cancelExtensionDialog: () => void; - /** Whether the visible info dialog's fully disclosed copy action is usable. */ - extensionInfoCopyEnabled: boolean; - copyExtensionDialogInfo: () => void; moveExtensionDialogSelection: (delta: number) => void; extensionTrustPromptOpen: boolean; trustRepoExtensions: () => void; @@ -110,8 +107,6 @@ export function useAppKeyboardShortcuts({ extensionDialog, acceptExtensionDialog, cancelExtensionDialog, - extensionInfoCopyEnabled, - copyExtensionDialogInfo, moveExtensionDialogSelection, extensionTrustPromptOpen, trustRepoExtensions, @@ -144,7 +139,6 @@ export function useAppKeyboardShortcuts({ const themeSelectorOpenRef = useRef(themeSelectorOpen); const extensionTrustPromptOpenRef = useRef(extensionTrustPromptOpen); const extensionDialogRef = useRef(extensionDialog); - const extensionInfoCopyEnabledRef = useRef(extensionInfoCopyEnabled); // 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); @@ -157,7 +151,6 @@ export function useAppKeyboardShortcuts({ // text), so they are read through refs rather than captured once. const acceptExtensionDialogRef = useRef(acceptExtensionDialog); const cancelExtensionDialogRef = useRef(cancelExtensionDialog); - const copyExtensionDialogInfoRef = useRef(copyExtensionDialogInfo); const moveExtensionDialogSelectionRef = useRef(moveExtensionDialogSelection); activeMenuIdRef.current = activeMenuId; @@ -168,7 +161,6 @@ export function useAppKeyboardShortcuts({ themeSelectorOpenRef.current = themeSelectorOpen; extensionTrustPromptOpenRef.current = extensionTrustPromptOpen; extensionDialogRef.current = extensionDialog; - extensionInfoCopyEnabledRef.current = extensionInfoCopyEnabled; isFileViewModeActiveRef.current = isFileViewModeActive; exitFileViewModeRef.current = exitFileViewMode; sendFileViewModeKeyRef.current = sendFileViewModeKey; @@ -177,7 +169,6 @@ export function useAppKeyboardShortcuts({ sendKeyboardModeKeyRef.current = sendKeyboardModeKey; acceptExtensionDialogRef.current = acceptExtensionDialog; cancelExtensionDialogRef.current = cancelExtensionDialog; - copyExtensionDialogInfoRef.current = copyExtensionDialogInfo; moveExtensionDialogSelectionRef.current = moveExtensionDialogSelection; /** @@ -320,9 +311,9 @@ 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; @@ -336,20 +327,11 @@ export function useAppKeyboardShortcuts({ } if (key.name === "return" || key.name === "enter") { - if (dialog.kind !== "info") { + if (dialog.kind !== "open") { acceptExtensionDialogRef.current(); + return "mine"; } - return "mine"; - } - - if ( - dialog.kind === "info" && - dialog.copy && - extensionInfoCopyEnabledRef.current && - isUnmodifiedKey(key, "c") - ) { - copyExtensionDialogInfoRef.current(); - return "mine"; + return "focused"; } if (dialog.kind === "select") { @@ -380,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..d1a0319d3 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. */ @@ -117,4 +117,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..1c9f769a6 100644 --- a/src/ui/hooks/useExtensionDialogController.ts +++ b/src/ui/hooks/useExtensionDialogController.ts @@ -8,6 +8,12 @@ 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; @@ -52,8 +58,9 @@ 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]); @@ -84,6 +91,9 @@ export function useExtensionDialogController({ return { createDialogs: queue.createDialogs, + getCurrentRequest: queue.current, + isCurrentRequestLive: queue.isCurrentLive, + cancelRequest: queue.cancel, request, selectedIndex, inputValue, diff --git a/src/ui/lib/extensionDialogGeometry.test.ts b/src/ui/lib/extensionDialogGeometry.test.ts index 004c1eefd..aba0430bc 100644 --- a/src/ui/lib/extensionDialogGeometry.test.ts +++ b/src/ui/lib/extensionDialogGeometry.test.ts @@ -1,24 +1,18 @@ import { describe, expect, test } from "bun:test"; -import type { ExtensionInfoDialogRequest } from "./extensionDialogs"; -import { - planExtensionInfoDialog, - windowDialogLiteralText, - windowDialogText, -} from "./extensionDialogGeometry"; +import type { ExtensionOpenDialogRequest } from "./extensionDialogs"; +import { planExtensionOpenDialog, windowDialogText } from "./extensionDialogGeometry"; -const infoRequest = { +const TestDialog = () => null; +const openRequest = { id: 1, - kind: "info", + kind: "open", extensionId: "example", showAttribution: true, - title: "Agent setup", - bodyLines: ["Teach your agent how to review this Hunk session."], - copy: { - label: "Prompt", - text: "Load the Hunk skill and use it for this review. Run hunk skill path.", - displayLines: ["Load the Hunk skill and use it for this review. Run hunk skill path."], - }, -} satisfies ExtensionInfoDialogRequest; + title: "Custom surface", + width: 64, + height: 12, + component: TestDialog, +} satisfies ExtensionOpenDialogRequest; describe("windowDialogText", () => { test("wraps prose within the available terminal-cell rows", () => { @@ -37,85 +31,34 @@ describe("windowDialogText", () => { }); }); -describe("windowDialogLiteralText", () => { - test("wraps copyable text without collapsing whitespace", () => { - expect(windowDialogLiteralText([" one two", " three"], 7, 4)).toEqual({ - lines: [" one ", "two", " thr", "ee"], - truncated: false, - }); - }); - - test("keeps meaningful literal text in a one-row window", () => { - expect(windowDialogLiteralText(["one two"], 5, 1)).toEqual({ - lines: ["one …"], - truncated: true, - }); - }); - - test("marks a substituted cluster as incomplete disclosure", () => { - expect(windowDialogLiteralText(["界"], 1, 1)).toEqual({ - lines: ["…"], - truncated: true, - }); - }); - - test("marks invisible clusters as incomplete disclosure", () => { - expect(windowDialogLiteralText(["\u200b"], 4, 1)).toEqual({ - lines: ["\u200b"], - truncated: true, - }); - }); - - test("marks invisible scalars inside a visible grapheme as incomplete disclosure", () => { - const taggedFlag = "\u{1F3F4}\u{E0061}\u{E0062}\u{E007F}"; - expect(windowDialogLiteralText([taggedFlag], 4, 1)).toEqual({ - lines: [taggedFlag], - truncated: true, - }); - }); -}); - -describe("planExtensionInfoDialog", () => { - test("exposes copy only when its complete payload and attribution fit", () => { - const complete = planExtensionInfoDialog(infoRequest, 50, 20); - const constrained = planExtensionInfoDialog(infoRequest, 50, 12); - - expect(complete.visibleCopy.truncated).toBe(false); - expect(complete.copyActionExposed).toBe(true); - expect(constrained.visibleCopy.truncated).toBe(true); - expect(constrained.copyActionExposed).toBe(false); - }); - - test("prioritizes required attribution over info actions", () => { - const layout = planExtensionInfoDialog(infoRequest, 50, 7); +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"); - expect(layout.actionRows).toBe(0); - expect(layout.copyActionExposed).toBe(false); }); - test("withholds copy when required attribution is truncated", () => { - const layout = planExtensionInfoDialog({ ...infoRequest, extensionId: "x".repeat(80) }, 50, 30); + test("clamps the component rectangle while preserving attribution first", () => { + const layout = planExtensionOpenDialog(openRequest, 50, 12); - expect(layout.attributionText).not.toBe(`ext ${"x".repeat(80)}`); - expect(layout.visibleCopy.truncated).toBe(false); - expect(layout.copyActionExposed).toBe(false); + 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("withholds copy when frame chrome leaves no real card width", () => { - const layout = planExtensionInfoDialog( - { - ...infoRequest, - showAttribution: false, - bodyLines: [], - copy: { label: "Content", text: "x", displayLines: ["x"] }, - }, - 6, - 30, - ); + test("gives bundled components the attribution rows they do not need", () => { + const layout = planExtensionOpenDialog({ ...openRequest, showAttribution: false }, 120, 40); - expect(layout.visibleCopy.truncated).toBe(false); - expect(layout.copyActionExposed).toBe(false); + 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 dcc83b5a5..cd1bb710e 100644 --- a/src/ui/lib/extensionDialogGeometry.ts +++ b/src/ui/lib/extensionDialogGeometry.ts @@ -1,7 +1,7 @@ -import { fitText, measureTextWidth, sliceTextByWidth, wrapText } from "./text"; +import { fitText, wrapText } from "./text"; import { extensionToastPrefix } from "./extensionNotifications"; import { MODAL_FRAME_CHROME_ROWS, resolveModalGeometry } from "./modalGeometry"; -import type { ExtensionInfoDialogRequest } from "./extensionDialogs"; +import type { ExtensionOpenDialogRequest } from "./extensionDialogs"; /** Wrapped body rows that fit one modal body allocation. */ export interface WindowedDialogText { @@ -29,181 +29,41 @@ export function windowDialogText( }; } -/** Wrap copyable text by terminal cells without normalizing its whitespace. */ -export function windowDialogLiteralText( - sourceLines: readonly string[], - width: number, - maxRows: number, -): WindowedDialogText { - const safeWidth = Math.max(1, width); - let incompleteDisclosure = sourceLines.some((line) => - Array.from(line).some((scalar) => measureTextWidth(scalar) === 0), - ); - const wrapped = sourceLines.flatMap((line) => { - const lineWidth = measureTextWidth(line); - if (lineWidth === 0) return [line]; - - const lines: string[] = []; - for (let offset = 0; offset < lineWidth; ) { - const chunk = sliceTextByWidth(line, offset, safeWidth); - if (chunk.width > 0) { - lines.push(chunk.text); - offset += chunk.width; - continue; - } - - // A cluster wider than the whole viewport cannot render intact. Show an - // overflow marker and advance past that cluster instead of looping. - const wideChunk = sliceTextByWidth(line, offset, safeWidth + 1); - lines.push(fitText(wideChunk.text, safeWidth, "…")); - incompleteDisclosure = true; - offset += Math.max(1, wideChunk.width); - } - return lines; - }); - - if (wrapped.length <= maxRows) return { lines: wrapped, truncated: incompleteDisclosure }; - if (maxRows <= 0) return { lines: [], truncated: wrapped.length > 0 }; - if (maxRows === 1) { - return { lines: [fitText(`${wrapped[0] ?? ""}…`, safeWidth, "…")], truncated: true }; - } - return { lines: [...wrapped.slice(0, maxRows - 1), "…"], truncated: true }; -} - -/** Concrete row allocation shared by info rendering and copy authorization. */ -export interface ExtensionInfoDialogLayout { +/** Concrete host frame and component rectangle for one open dialog. */ +export interface ExtensionOpenDialogLayout { frame: { width: number; height: number }; bodyWidth: number; - cardWidth: number; - cardTextWidth: number; + componentHeight: number; attributionText: string; attributionRows: number; attributionGapRows: number; - bodyCopyGapRows: number; - copyLabelRows: number; - copyCardRows: number; - actionGapRows: number; - actionRows: number; - visibleBody: WindowedDialogText; - visibleCopy: WindowedDialogText; - /** Whether the complete payload and required attribution are visible beside the action. */ - copyActionExposed: boolean; -} - -/** Preserve meaningful text when constrained info has only one body row. */ -function windowInfoText(sourceLines: readonly string[], width: number, maxRows: number) { - const windowed = windowDialogText(sourceLines, width, maxRows); - if (maxRows !== 1 || !windowed.truncated) return windowed; - - const firstLine = windowDialogText(sourceLines, width, Number.MAX_SAFE_INTEGER).lines[0] ?? ""; - return { lines: [fitText(`${firstLine}…`, width, "…")], truncated: true }; } -/** Plan read-only info so rendering and keyboard copy use identical disclosure facts. */ -export function planExtensionInfoDialog( - request: ExtensionInfoDialogRequest, +/** Clamp an extension-owned component while preserving host attribution above it. */ +export function planExtensionOpenDialog( + request: ExtensionOpenDialogRequest, terminalWidth: number, terminalHeight: number, -): ExtensionInfoDialogLayout { - const width = Math.min(84, Math.max(58, terminalWidth - 8)); - const measuredFrame = resolveModalGeometry({ - width, - height: Number.MAX_SAFE_INTEGER, - terminalWidth, - terminalHeight, - }); - const bodyWidth = Math.max(1, measuredFrame.width - 4); - const cardWidth = Math.max(1, bodyWidth - 4); - const cardTextWidth = Math.max(1, cardWidth - 4); - const availableBodyWidth = measuredFrame.width - 4; - const availableCardWidth = availableBodyWidth - 4; - const idealBodyRows = windowDialogText(request.bodyLines, bodyWidth, Number.MAX_SAFE_INTEGER) - .lines.length; - const copy = request.copy; - const idealCopyRows = copy - ? windowDialogLiteralText(copy.displayLines, cardTextWidth, Number.MAX_SAFE_INTEGER).lines - .length - : 0; - const hasBody = idealBodyRows > 0; - const hasCopy = copy !== null; - const idealContentRows = - (request.showAttribution ? 2 : 0) + - idealBodyRows + - (hasBody && hasCopy ? 1 : 0) + - (hasCopy ? 1 + idealCopyRows + 2 : 0) + - 2; +): ExtensionOpenDialogLayout { + const attributionRequestRows = request.showAttribution ? 2 : 0; const frame = resolveModalGeometry({ - width, - // The inner flex column lets the final action use the last - // chrome-adjacent row without adding an empty footer row. - height: idealContentRows + MODAL_FRAME_CHROME_ROWS - 1, + width: request.width + 4, + height: request.height + MODAL_FRAME_CHROME_ROWS + attributionRequestRows, terminalWidth, terminalHeight, }); - const contentRows = Math.max(0, frame.height - MODAL_FRAME_CHROME_ROWS + 1); - let remainingRows = contentRows; - // Attribution wins the first available row so third-party copy UI never hides its owner. - const attributionRows = request.showAttribution && remainingRows > 0 ? 1 : 0; - remainingRows -= attributionRows; - const actionRows = remainingRows > 0 ? 1 : 0; - remainingRows -= actionRows; - const minimumCopyRows = hasCopy ? 2 : 0; - const minimumVisibleDocumentRows = (hasBody ? 1 : 0) + minimumCopyRows; - const attributionGapRows = - attributionRows > 0 && remainingRows > minimumVisibleDocumentRows ? 1 : 0; - remainingRows -= attributionGapRows; - const minimumContentRows = (hasBody ? 1 : 0) + minimumCopyRows; - const actionGapRows = remainingRows > minimumContentRows ? 1 : 0; - remainingRows -= actionGapRows; - const copyReserve = hasCopy ? Math.min(minimumCopyRows, remainingRows) : 0; - const bodyCopyGapReserve = hasBody && hasCopy && remainingRows > copyReserve + 1 ? 1 : 0; - const bodyRows = Math.min( - idealBodyRows, - Math.max(0, remainingRows - copyReserve - bodyCopyGapReserve), - ); - remainingRows -= bodyRows; - const bodyCopyGapRows = bodyRows > 0 && hasCopy && remainingRows > 3 ? 1 : 0; - remainingRows -= bodyCopyGapRows; - const copyLabelRows = hasCopy && remainingRows > 1 ? 1 : 0; - remainingRows -= copyLabelRows; - const copyCardRows = hasCopy ? remainingRows : 0; - const visibleBody = windowInfoText(request.bodyLines, bodyWidth, bodyRows); - const visibleCopy = copy - ? windowDialogLiteralText( - copy.displayLines, - cardTextWidth, - copyCardRows >= 3 ? copyCardRows - 2 : copyCardRows, - ) - : { lines: [], truncated: false }; - const fullAttributionText = `${extensionToastPrefix()} ${request.extensionId}`; - const attributionComplete = - !request.showAttribution || - (attributionRows === 1 && measureTextWidth(fullAttributionText) <= bodyWidth); - const copyTextHasRealWidth = - availableBodyWidth > 0 && - (copyCardRows >= 3 ? availableCardWidth >= 5 : availableCardWidth >= 1); + 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, - cardWidth, - cardTextWidth, - attributionText: fitText(fullAttributionText, bodyWidth), + componentHeight, + attributionText: fitText(`${extensionToastPrefix()} ${request.extensionId}`, bodyWidth), attributionRows, attributionGapRows, - bodyCopyGapRows, - copyLabelRows, - copyCardRows, - actionGapRows, - actionRows, - visibleBody, - visibleCopy, - copyActionExposed: - hasCopy && - actionRows === 1 && - copyLabelRows === 1 && - copyTextHasRealWidth && - !visibleCopy.truncated && - attributionComplete, }; } diff --git a/src/ui/lib/extensionDialogs.test.ts b/src/ui/lib/extensionDialogs.test.ts index cf1909e26..5b326ade1 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 () => { @@ -51,11 +56,11 @@ describe("createExtensionDialogQueue", () => { queue.accept(queue.current()!.id); expect(await valueless).toBeNull(); - const info = dialogs.info({ title: "Guide", body: "Read this." }); + const opened = dialogs.open({ title: "Guide", component: TestDialog }); queue.accept(queue.current()!.id); - expect(queue.current()).toMatchObject({ kind: "info", title: "Guide" }); + expect(queue.current()).toMatchObject({ kind: "open", title: "Guide" }); queue.cancel(queue.current()!.id); - expect(await info).toBeUndefined(); + expect(await opened).toBeUndefined(); }); test("ignores an answer aimed at a dialog that is no longer current", async () => { @@ -121,46 +126,30 @@ describe("createExtensionDialogQueue", () => { expect(queue.current()).toMatchObject({ title: "Pick", options: ["opt"] }); }); - test("uses the same terminal-safe text for info display and clipboard payloads", () => { + test("carries a custom component and default rectangle into the request", () => { const queue = createExtensionDialogQueue(); const dialogs = queue.createDialogs("guide"); - void dialogs.info({ + void dialogs.open({ title: "Setup", - body: "one\n\u001b[31mtwo\u001b[0m", - copy: { label: "Prompt", text: "copy\t\u001b[31mexactly\u001b[0m" }, + component: TestDialog, }); expect(queue.current()).toMatchObject({ - kind: "info", - bodyLines: ["one", "two"], - copy: { - label: "Prompt", - text: "copy exactly", - displayLines: ["copy exactly"], - }, + kind: "open", + width: 64, + height: 12, + component: TestDialog, }); }); - test("accepts authored display rows only when they preserve the clipboard text", async () => { - const queue = createExtensionDialogQueue(); - const dialogs = queue.createDialogs("guide"); - - void dialogs.info({ - title: "Setup", - copy: { text: "copy this exactly", displayLines: ["copy this", "exactly"] }, - }); - expect(queue.current()).toMatchObject({ - copy: { text: "copy this exactly", displayLines: ["copy this", "exactly"] }, - }); - queue.cancelAll(); - - await expect( - dialogs.info({ - title: "Setup", - copy: { text: "safe text", displayLines: ["different text"] }, - }), - ).rejects.toThrow("copy.displayLines must contain the same text as copy.text"); + 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", () => { @@ -269,19 +258,21 @@ describe("createExtensionDialogQueue", () => { await expect(dialogs.select({ title: "Which?", options: [] })).rejects.toThrow( /at least one option/, ); - await expect(dialogs.info({ title: "Empty" })).rejects.toThrow(/body or copy content/); - await expect(dialogs.info({ title: "Bad copy", copy: { text: "" } })).rejects.toThrow( - /non-empty string/, + await expect(dialogs.open({ title: "No component", component: null as never })).rejects.toThrow( + /component function/, ); await expect( - dialogs.info({ title: "Long body", body: Array(101).fill("line").join("\n") }), - ).rejects.toThrow(/at most 100 lines/); + 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.info({ title: "Long copy", copy: { text: "x".repeat(16_385) } }), - ).rejects.toThrow(/at most 16384 characters/); + dialogs.open({ title: "Bad height", height: 1.5, component: TestDialog }), + ).rejects.toThrow(/height must be an integer from 1 to 100/); await expect( - dialogs.info({ title: "Expanded copy", copy: { text: "\t".repeat(4_097) } }), - ).rejects.toThrow(/normalized copy.text.*at most 16384 characters/); + 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 92266ebb7..d6dd33ff0 100644 --- a/src/ui/lib/extensionDialogs.ts +++ b/src/ui/lib/extensionDialogs.ts @@ -11,8 +11,8 @@ import type { ExtensionConfirmOptions, + ExtensionDialogOptions, ExtensionDialogs, - ExtensionInfoOptions, ExtensionInputOptions, ExtensionSelectOptions, } from "../../extension-api/types"; @@ -27,14 +27,16 @@ 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; -/** Body lines one read-only info dialog may retain before host-side windowing. */ -const MAX_INFO_BODY_LINES = 100; +/** Default extension-owned component rectangle. */ +const DEFAULT_OPEN_DIALOG_WIDTH = 64; +const DEFAULT_OPEN_DIALOG_HEIGHT = 12; -/** Clipboard text is bounded before it reaches the terminal's OSC 52 channel. */ -const MAX_INFO_COPY_TEXT_LENGTH = 16_384; +/** Bounds keep one request from retaining absurd off-screen geometry. */ +const MAX_OPEN_DIALOG_WIDTH = 240; +const MAX_OPEN_DIALOG_HEIGHT = 100; -/** Default heading for an info dialog's copyable text card. */ -const DEFAULT_INFO_COPY_LABEL = "Content"; +/** 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 { @@ -71,18 +73,12 @@ export interface ExtensionInputDialogRequest extends ExtensionDialogRequestBase initial: string; } -/** Clipboard and display forms of one info dialog's normalized copyable text. */ -export interface ExtensionInfoCopyRequest { - label: string; - text: string; - displayLines: string[]; -} - -/** One normalized read-only info dialog the host should draw. */ -export interface ExtensionInfoDialogRequest extends ExtensionDialogRequestBase { - kind: "info"; - bodyLines: string[]; - copy: ExtensionInfoCopyRequest | null; +/** 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"]; } /** One dialog the host should draw, normalized from what an extension asked for. */ @@ -90,7 +86,7 @@ export type ExtensionDialogRequest = | ExtensionConfirmDialogRequest | ExtensionSelectDialogRequest | ExtensionInputDialogRequest - | ExtensionInfoDialogRequest; + | ExtensionOpenDialogRequest; /** What a dialog hands back to the awaiting handler. */ type ExtensionDialogResult = boolean | string | null | undefined; @@ -104,12 +100,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. Info dialogs - * ignore acceptance and remain visible until cancelled. + * 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 @@ -180,58 +178,30 @@ function normalizeBodyLines(body: unknown, maxLines = MAX_CONFIRM_BODY_LINES) { .map((line) => sanitizeTerminalLine(line)); } -/** Normalize an info body while rejecting content the host would have to discard. */ -function normalizeInfoBodyLines(body: unknown) { - if (typeof body !== "string" || body.length === 0) return []; - const lines = body.split("\n"); - if (lines.length > MAX_INFO_BODY_LINES) { - invalid("info", `body must contain at most ${MAX_INFO_BODY_LINES} lines.`); +/** 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 lines.map((line) => sanitizeTerminalLine(line)); + return value as number; } -/** Validate and normalize an info dialog's optional clipboard card. */ -function normalizeInfoCopy(copy: ExtensionInfoOptions["copy"]): ExtensionInfoCopyRequest | null { - if (copy === undefined) return null; - if (!copy || typeof copy.text !== "string" || copy.text.length === 0) { - invalid("info", "copy.text must be a non-empty string."); - } - if (copy.text.length > MAX_INFO_COPY_TEXT_LENGTH) { - invalid("info", `copy.text must be at most ${MAX_INFO_COPY_TEXT_LENGTH} characters.`); - } - - const text = sanitizeTerminalText(copy.text).replaceAll("\t", " "); - if (text.length > MAX_INFO_COPY_TEXT_LENGTH) { - invalid( - "info", - `normalized copy.text must be at most ${MAX_INFO_COPY_TEXT_LENGTH} characters.`, - ); - } - if (text.length === 0) { - invalid("info", "copy.text must contain visible or whitespace content."); - } - - let displayLines = text.split("\n").map((line) => sanitizeTerminalLine(line)); - if (copy.displayLines !== undefined) { - if (!Array.isArray(copy.displayLines) || copy.displayLines.length === 0) { - invalid("info", "copy.displayLines must be a non-empty string array."); - } - displayLines = copy.displayLines.map((line) => { - if (typeof line !== "string" || line.includes("\n")) { - invalid("info", "copy.displayLines must contain single-line strings."); - } - return sanitizeTerminalLine(line).replaceAll("\t", " "); - }); - if (displayLines.join(" ") !== text && displayLines.join("\n") !== text) { - invalid("info", "copy.displayLines must contain the same text as copy.text."); - } +/** 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; } - return { - label: normalizeLabel(copy.label, DEFAULT_INFO_COPY_LABEL), - text, - displayLines, - }; + 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. */ @@ -276,7 +246,7 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { /** The cancel value one request resolves with. */ const cancelValueFor = (request: ExtensionDialogRequest): ExtensionDialogResult => - request.kind === "confirm" ? false : request.kind === "info" ? undefined : null; + request.kind === "confirm" ? false : request.kind === "open" ? undefined : null; /** * Queue one request and hand back the promise its handler awaits. @@ -390,22 +360,33 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { isLive, ); }, - async info(options: ExtensionInfoOptions) { - const title = normalizeTitle("info", options?.title); - const bodyLines = normalizeInfoBodyLines(options.body); - const copy = normalizeInfoCopy(options.copy); - if (bodyLines.length === 0 && copy === null) { - invalid("info", "requires body or copy content."); + 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: "info", + kind: "open", id, extensionId, showAttribution, title, - bodyLines, - copy, + width, + height, + component: options.component, }), undefined, isLive, @@ -418,6 +399,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) { @@ -434,7 +420,7 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { return; } - if (active.request.kind === "info") { + if (active.request.kind === "open") { return; } diff --git a/src/ui/lib/keyboard.ts b/src/ui/lib/keyboard.ts index 98ea34f24..73d66afab 100644 --- a/src/ui/lib/keyboard.ts +++ b/src/ui/lib/keyboard.ts @@ -24,21 +24,6 @@ export function isEscapeKey(key: KeyEvent) { ); } -/** Match one literal key only when no modifier changes its meaning. */ -export function isUnmodifiedKey(key: KeyEvent, value: string) { - return ( - !key.ctrl && - !key.meta && - !key.option && - !key.shift && - !key.super && - !key.hyper && - !key.capsLock && - !key.numLock && - (key.name === value || key.sequence === value) - ); -} - /** * Match Ctrl-S across raw, Kitty/CSI-u, and tmux control-mode encodings. * diff --git a/src/ui/lib/ui-lib.test.ts b/src/ui/lib/ui-lib.test.ts index 5e8b85256..1292f255d 100644 --- a/src/ui/lib/ui-lib.test.ts +++ b/src/ui/lib/ui-lib.test.ts @@ -13,7 +13,7 @@ import { } from "../components/chrome/menu"; import { createVisibleAgentNote } from "./agentAnnotations"; import { buildAgentPopoverContent, resolveAgentPopoverPlacement } from "./agentPopover"; -import { isEscapeKey, isSaveDraftNoteKey, isUnmodifiedKey } from "./keyboard"; +import { isEscapeKey, isSaveDraftNoteKey } from "./keyboard"; import { BoundedClusterWidthCache, CLUSTER_WIDTH_CACHE_MAX_ENTRIES, @@ -194,19 +194,6 @@ describe("ui helpers", () => { expect(isEscapeKey(createKeyEvent({ name: "q" }))).toBe(false); }); - test("literal modal keys reject every modified form", () => { - expect(isUnmodifiedKey(createKeyEvent({ name: "c" }), "c")).toBe(true); - expect(isUnmodifiedKey(createKeyEvent({ sequence: "c" }), "c")).toBe(true); - expect(isUnmodifiedKey(createKeyEvent({ name: "c", ctrl: true }), "c")).toBe(false); - expect(isUnmodifiedKey(createKeyEvent({ name: "c", meta: true }), "c")).toBe(false); - expect(isUnmodifiedKey(createKeyEvent({ name: "c", option: true }), "c")).toBe(false); - expect(isUnmodifiedKey(createKeyEvent({ name: "c", shift: true }), "c")).toBe(false); - expect(isUnmodifiedKey(createKeyEvent({ name: "c", super: true }), "c")).toBe(false); - expect(isUnmodifiedKey(createKeyEvent({ name: "c", hyper: true }), "c")).toBe(false); - expect(isUnmodifiedKey(createKeyEvent({ name: "c", capsLock: true }), "c")).toBe(false); - expect(isUnmodifiedKey(createKeyEvent({ name: "c", numLock: true }), "c")).toBe(false); - }); - test("save-draft-note shortcut matches Ctrl-S across raw, CSI-u, and tmux encodings", () => { const CTRL_S = "\u0013"; const CTRL_S_CSI_U = "\u001b[115;5u"; diff --git a/test/pty/chrome.test.ts b/test/pty/chrome.test.ts index 3ef45a381..7271c1907 100644 --- a/test/pty/chrome.test.ts +++ b/test/pty/chrome.test.ts @@ -84,7 +84,7 @@ describe("PTY chrome", () => { } }); - test("the Agent menu opens bundled skill guidance as an info dialog", async () => { + 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"], diff --git a/website/src/content/docs/docs/extend/extension-api.md b/website/src/content/docs/docs/extend/extension-api.md index 45b368a74..786d4a7a5 100644 --- a/website/src/content/docs/docs/extend/extension-api.md +++ b/website/src/content/docs/docs/extend/extension-api.md @@ -8,8 +8,8 @@ The extension factory receives one API object. Registration calls are only valid ## `hunk.apiVersion` The API generation this Hunk speaks (currently `17`). Branch on it if you want -one file to support several Hunk versions. Version 17 adds read-only info -dialogs with optional clipboard actions; version 16 added temporary application +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 @@ -290,7 +290,7 @@ A handler may be async; a failure becomes a warning naming your extension. - `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` -- `document({ title, body?, copy?: { label?, text } })` → `void` when closed +- `open({ title, width?, height?, component })` → `void` when closed ```ts hunk.registerCommand( @@ -312,32 +312,47 @@ hunk.registerCommand( ); ``` -`info` presents read-only guidance rather than asking for an answer. At -least `body` or `copy` must be present. A body may contain up to 100 source -lines, and copy text may contain up to 16,384 JavaScript string code units. -When `copy` is provided, `c` and the clickable copy action send its `text` to -the terminal clipboard after Hunk removes terminal control sequences and -expands tabs to four spaces, and Hunk renders the same safe value under `label` -(default `Content`). Hunk exposes those actions only while the complete payload -and any required extension attribution are visible. Optional `displayLines` can -add authored visual breaks; after sanitizing, those lines must rejoin with -spaces or newlines to exactly the clipboard `text`, so a preview cannot disguise -what the action copies: +`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 + + + ); +} -```ts hunk.registerCommand({ id: "agent-setup", title: "Agent setup" }, async (ctx) => { - await ctx.dialogs.info({ + await ctx.dialogs.open({ title: "Agent setup", - body: "Give this prompt to your coding agent.", - copy: { - label: "Prompt", - text: "Review the current Hunk session. Focus on correctness.", - displayLines: ["Review the current Hunk session.", "Focus on correctness."], - }, + 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 @@ -359,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`) or closes an info dialog. Enter accepts interactive dialogs but leaves read-only info dialogs open; 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 From a1cc59e9c220711c4014ec6025b24b75f0ac89cf Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Thu, 3 Sep 2026 09:28:47 -0400 Subject: [PATCH 9/9] fix(extensions): harden dialog lifecycles --- src/ui/App.tsx | 11 +- src/ui/AppHost.extension-dialogs.test.tsx | 157 +++++++++++++++++- src/ui/components/chrome/ExtensionDialog.tsx | 90 ++++++---- src/ui/hooks/useAppKeyboardShortcuts.ts | 12 +- .../useExtensionDialogController.test.tsx | 41 ++++- src/ui/hooks/useExtensionDialogController.ts | 125 +++++++++++--- src/ui/lib/extensionDialogGeometry.test.ts | 1 + src/ui/lib/extensionDialogs.test.ts | 17 +- src/ui/lib/extensionDialogs.ts | 39 ++++- 9 files changed, 413 insertions(+), 80 deletions(-) diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 734525421..dea80ef85 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -492,6 +492,7 @@ export function App({ const { accept: acceptExtensionDialog, + acceptRequest: acceptExtensionDialogRequest, cancel: cancelExtensionDialog, cancelRequest: cancelExtensionDialogRequest, cancelAll: cancelAllExtensionDialogs, @@ -1235,7 +1236,7 @@ export function App({ closeExtensionTrustPrompt, commands: appCommands, denyRepoExtensions, - extensionDialog, + getExtensionDialog: getCurrentExtensionDialogRequest, acceptExtensionDialog, cancelExtensionDialog, moveExtensionDialogSelection, @@ -1561,13 +1562,13 @@ export function App({ terminalHeight={terminal.height} terminalWidth={terminal.width} theme={baseTheme} - onAccept={acceptExtensionDialog} - onCancel={cancelExtensionDialog} - onChangeInput={setExtensionDialogInputValue} + onAcceptRequest={acceptExtensionDialogRequest} + onCancelRequest={cancelExtensionDialogRequest} + onChangeInputRequest={setExtensionDialogInputValue} onClose={closeExtensionDialogComponent} onCopy={copyExtensionDialogText} onNotify={notifyExtensionDialog} - onPickOption={setExtensionDialogSelectedIndex} + onPickOptionRequest={setExtensionDialogSelectedIndex} onRenderFailure={reportExtensionDialogRenderFailure} /> ) : null} diff --git a/src/ui/AppHost.extension-dialogs.test.tsx b/src/ui/AppHost.extension-dialogs.test.tsx index 5aff4c829..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>, @@ -342,6 +360,71 @@ 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-"); @@ -670,11 +753,17 @@ describe("extension dialogs", () => { writeDialogFixture( extPath, logPath, - `ctx.dialogs.open({ title: "Broken surface", width: 30, height: 4, component: () => { throw new Error("surface exploded"); } })`, + `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"); }); @@ -687,6 +776,15 @@ describe("extension dialogs", () => { 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(); @@ -699,6 +797,63 @@ describe("extension dialogs", () => { }); }); + 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-"); diff --git a/src/ui/components/chrome/ExtensionDialog.tsx b/src/ui/components/chrome/ExtensionDialog.tsx index addfa214a..2898f950e 100644 --- a/src/ui/components/chrome/ExtensionDialog.tsx +++ b/src/ui/components/chrome/ExtensionDialog.tsx @@ -54,13 +54,13 @@ function isWithinRenderable(root: Renderable, candidate: Renderable | null) { export function ExtensionDialog({ copySupported, inputValue, - onAccept, - onCancel, - onChangeInput, + onAcceptRequest, + onCancelRequest, + onChangeInputRequest, onClose, onCopy, onNotify, - onPickOption, + onPickOptionRequest, onRenderFailure, request, selectedIndex, @@ -71,14 +71,14 @@ export function ExtensionDialog({ 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; @@ -90,7 +90,7 @@ export function ExtensionDialog({ return ( onCancelRequest(request.id)} onClose={onClose} onCopy={onCopy} onNotify={onNotify} @@ -106,9 +106,9 @@ export function ExtensionDialog({ 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} @@ -122,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} @@ -151,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} @@ -160,7 +168,7 @@ export function ExtensionDialog({ theme={theme} title={request.title} width={frame.width} - onClose={onCancel} + onClose={() => onCancelRequest(request.id)} > {attributionRows > 0 ? ( @@ -184,6 +192,7 @@ class ExtensionDialogErrorBoundary extends Component< request: ExtensionOpenDialogRequest; fallback: ReactNode; onError: (error: unknown) => void; + retireActions: () => void; children: ReactNode; }, { failed: boolean; request: ExtensionOpenDialogRequest | null } @@ -199,8 +208,12 @@ class ExtensionDialogErrorBoundary extends Component< 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; } @@ -235,14 +248,19 @@ function ExtensionOpenDialog({ 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: () => onClose(request.id), - copy: (text: string) => onCopy(request.id, text), - notify: (message: string) => onNotify(request.id, message), + 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); + }, }), - [onClose, onCopy, onNotify, request.id], + [actionLease, onClose, onCopy, onNotify, request.id], ); const viewProps: ExtensionDialogProps = { width: bodyWidth, @@ -257,6 +275,7 @@ function ExtensionOpenDialog({ ref={fallback ? undefined : componentRootRef} focusable={true} focused={fallback} + visible={componentHeight > 0} style={{ width: bodyWidth, height: componentHeight, @@ -295,20 +314,19 @@ function ExtensionOpenDialog({ ) : null} {attributionGapRows > 0 ? : null} - {componentHeight > 0 ? ( - Dialog unavailable, true)} - onError={(error) => { - onRenderFailure(request.id, error); - }} - > - {componentBox()} - - ) : ( - - )} + Dialog unavailable, true)} + retireActions={() => { + actionLease.active = false; + }} + onError={(error) => { + onRenderFailure(request.id, error); + }} + > + {componentBox()} + ); } diff --git a/src/ui/hooks/useAppKeyboardShortcuts.ts b/src/ui/hooks/useAppKeyboardShortcuts.ts index 89f6ba095..79bad6d6f 100644 --- a/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/src/ui/hooks/useAppKeyboardShortcuts.ts @@ -35,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; @@ -104,7 +104,7 @@ export function useAppKeyboardShortcuts({ closeExtensionTrustPrompt, commands, denyRepoExtensions, - extensionDialog, + getExtensionDialog, acceptExtensionDialog, cancelExtensionDialog, moveExtensionDialogSelection, @@ -138,7 +138,7 @@ export function useAppKeyboardShortcuts({ 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); @@ -160,7 +160,7 @@ export function useAppKeyboardShortcuts({ saveConfigPromptOpenRef.current = saveConfigPromptOpen; themeSelectorOpenRef.current = themeSelectorOpen; extensionTrustPromptOpenRef.current = extensionTrustPromptOpen; - extensionDialogRef.current = extensionDialog; + getExtensionDialogRef.current = getExtensionDialog; isFileViewModeActiveRef.current = isFileViewModeActive; exitFileViewModeRef.current = exitFileViewMode; sendFileViewModeKeyRef.current = sendFileViewModeKey; @@ -316,7 +316,7 @@ export function useAppKeyboardShortcuts({ * 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"; } diff --git a/src/ui/hooks/useExtensionDialogController.test.tsx b/src/ui/hooks/useExtensionDialogController.test.tsx index d1a0319d3..d95232d7b 100644 --- a/src/ui/hooks/useExtensionDialogController.test.tsx +++ b/src/ui/hooks/useExtensionDialogController.test.tsx @@ -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"); diff --git a/src/ui/hooks/useExtensionDialogController.ts b/src/ui/hooks/useExtensionDialogController.ts index 1c9f769a6..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, @@ -19,12 +26,14 @@ export interface ExtensionDialogController { 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. */ @@ -38,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(() => { @@ -64,44 +86,99 @@ export function useExtensionDialogController({ 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(active.id, active.kind === "input" ? inputValueRef.current : undefined); + alignAnswerState(queue.current()); + }, + [alignAnswerState, queue], + ); - queue.accept(request.id, request.kind === "input" ? inputValue : undefined); + /** 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: queue.cancel, + 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 aba0430bc..c96001017 100644 --- a/src/ui/lib/extensionDialogGeometry.test.ts +++ b/src/ui/lib/extensionDialogGeometry.test.ts @@ -12,6 +12,7 @@ const openRequest = { width: 64, height: 12, component: TestDialog, + actionLease: { active: true }, } satisfies ExtensionOpenDialogRequest; describe("windowDialogText", () => { diff --git a/src/ui/lib/extensionDialogs.test.ts b/src/ui/lib/extensionDialogs.test.ts index 5b326ade1..4a37ebeb1 100644 --- a/src/ui/lib/extensionDialogs.test.ts +++ b/src/ui/lib/extensionDialogs.test.ts @@ -243,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"); @@ -258,6 +258,21 @@ 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/, ); diff --git a/src/ui/lib/extensionDialogs.ts b/src/ui/lib/extensionDialogs.ts index d6dd33ff0..6c9a77050 100644 --- a/src/ui/lib/extensionDialogs.ts +++ b/src/ui/lib/extensionDialogs.ts @@ -79,6 +79,8 @@ export interface ExtensionOpenDialogRequest extends ExtensionDialogRequestBase { 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. */ @@ -147,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. */ @@ -156,7 +163,7 @@ function normalizeLabel(label: unknown, fallback: string) { return fallback; } - return sanitizeTerminalLine(label.trim()); + return sanitizeTerminalLine(label.trim()).trim() || fallback; } /** @@ -210,13 +217,25 @@ function normalizeOptions(options: unknown) { 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."); + } + + const normalized = sanitizeTerminalLine(option).trim(); + if (normalized.length === 0) { + invalid("select", "options must all be strings that remain non-empty after sanitization."); } - return sanitizeTerminalLine(option); - }); + normalizedOptions.push(normalized); + } + + return normalizedOptions; } /** @@ -248,6 +267,11 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { const cancelValueFor = (request: ExtensionDialogRequest): ExtensionDialogResult => 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. * @@ -281,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)); } @@ -296,6 +321,7 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { return; } + retireActions(active.request); active.settle(value); notify(); }; @@ -387,6 +413,7 @@ export function createExtensionDialogQueue(): ExtensionDialogQueue { width, height, component: options.component, + actionLease: { active: true }, }), undefined, isLive,