From 3c7d50d292c6bdcbe6c73f3967d447a67e0718fd Mon Sep 17 00:00:00 2001 From: rimless-casualty Date: Mon, 1 Jun 2026 17:37:26 +0800 Subject: [PATCH] Improve ask option rendering --- packages/coding-agent/CHANGELOG.md | 8 + .../src/extensibility/extensions/types.ts | 19 +- .../coding-agent/src/modes/acp/acp-agent.ts | 8 +- .../src/modes/components/hook-selector.ts | 73 ++++++-- .../controllers/extension-ui-controller.ts | 5 +- .../src/modes/interactive-mode.ts | 3 +- .../coding-agent/src/modes/rpc/rpc-mode.ts | 23 ++- packages/coding-agent/src/modes/types.ts | 3 +- .../coding-agent/src/prompts/tools/ask.md | 3 +- packages/coding-agent/src/tools/ask.ts | 106 ++++++++---- .../test/hook-selector-overflow.test.ts | 90 ++++++++++ packages/coding-agent/test/tools/ask.test.ts | 163 ++++++++++++++++-- 12 files changed, 431 insertions(+), 73 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9e22dabb5..e2756b513 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,14 @@ ## [Unreleased] +### Added + +- Added `ask` option descriptions so agents can keep short labels and render explanatory text as separate muted rows in the selector. + +### Fixed + +- Fixed long outlined `ask` selector options wrapping instead of truncating their tails. + ## [15.7.4] - 2026-05-31 ### Removed diff --git a/packages/coding-agent/src/extensibility/extensions/types.ts b/packages/coding-agent/src/extensibility/extensions/types.ts index 332427184..8b7dc0795 100644 --- a/packages/coding-agent/src/extensibility/extensions/types.ts +++ b/packages/coding-agent/src/extensibility/extensions/types.ts @@ -90,6 +90,17 @@ export type { AgentToolResult, AgentToolUpdateCallback }; // UI Context // ============================================================================ +export interface ExtensionUISelectOption { + label: string; + description?: string; +} + +export type ExtensionUISelectItem = string | ExtensionUISelectOption; + +export function getExtensionUISelectOptionLabel(option: ExtensionUISelectItem): string { + return typeof option === "string" ? option : option.label; +} + /** * UI dialog options for extensions. */ @@ -135,8 +146,12 @@ export type ExtensionWidgetContent = string[] | ExtensionUiComponentFactory | un // and may be invoked from event handlers that have already taken the agent // loop's lock — hooks intentionally cannot. export interface ExtensionUIContext { - /** Show a selector and return the user's choice. */ - select(title: string, options: string[], dialogOptions?: ExtensionUIDialogOptions): Promise; + /** Show a selector and return the selected label, even when an option also includes a description. */ + select( + title: string, + options: ExtensionUISelectItem[], + dialogOptions?: ExtensionUIDialogOptions, + ): Promise; /** Show a confirmation dialog. */ confirm(title: string, message: string, dialogOptions?: ExtensionUIDialogOptions): Promise; diff --git a/packages/coding-agent/src/modes/acp/acp-agent.ts b/packages/coding-agent/src/modes/acp/acp-agent.ts index d558b65c2..9457e66fd 100644 --- a/packages/coding-agent/src/modes/acp/acp-agent.ts +++ b/packages/coding-agent/src/modes/acp/acp-agent.ts @@ -47,7 +47,11 @@ import { logger, VERSION } from "@oh-my-pi/pi-utils"; import { disableProvider, enableProvider, reset as resetCapabilities } from "../../capability"; import { Settings } from "../../config/settings"; import { clearPluginRootsAndCaches, resolveActiveProjectRegistryPath } from "../../discovery/helpers"; -import type { ExtensionUIContext, ExtensionUIDialogOptions } from "../../extensibility/extensions"; +import { + type ExtensionUIContext, + type ExtensionUIDialogOptions, + getExtensionUISelectOptionLabel, +} from "../../extensibility/extensions"; import { runExtensionCompact } from "../../extensibility/extensions/compact-handler"; import { getSessionSlashCommands } from "../../extensibility/extensions/get-commands-handler"; import { buildSkillPromptMessage, getSkillSlashCommandName } from "../../extensibility/skills"; @@ -302,7 +306,7 @@ export function createAcpExtensionUiContext( getSessionId(), "select", title, - { type: "string", enum: options }, + { type: "string", enum: options.map(getExtensionUISelectOptionLabel) }, dialogOptions, ); return typeof value === "string" ? value : undefined; diff --git a/packages/coding-agent/src/modes/components/hook-selector.ts b/packages/coding-agent/src/modes/components/hook-selector.ts index 19aab9400..e8a8f6067 100644 --- a/packages/coding-agent/src/modes/components/hook-selector.ts +++ b/packages/coding-agent/src/modes/components/hook-selector.ts @@ -14,8 +14,8 @@ import { Spacer, Text, type TUI, - truncateToWidth, visibleWidth, + wrapTextWithAnsi, } from "@oh-my-pi/pi-tui"; import { getMarkdownTheme, type ThemeColor, theme } from "../../modes/theme/theme"; import { @@ -69,6 +69,34 @@ export interface HookSelectorOptions { slider?: HookSelectorSlider; } +export interface HookSelectorOption { + label: string; + description?: string; +} + +export type HookSelectorOptionInput = string | HookSelectorOption; + +function normalizeHookSelectorOption(option: HookSelectorOptionInput): HookSelectorOption { + if (typeof option === "string") return { label: option }; + if (option.description?.trim()) { + return { label: option.label, description: option.description.trim() }; + } + return { label: option.label }; +} + +function splitLeadingSpacesForWrap(line: string, width: number): { indent: string; body: string } { + let indentLength = 0; + while (indentLength < line.length && line.charCodeAt(indentLength) === 32) { + indentLength += 1; + } + const maxIndentLength = Math.max(0, width - 1); + const clampedIndentLength = Math.min(indentLength, maxIndentLength); + return { + indent: line.slice(0, clampedIndentLength), + body: line.slice(indentLength), + }; +} + class OutlinedList extends Container { #lines: string[] = []; @@ -81,19 +109,26 @@ class OutlinedList extends Container { const borderColor = (text: string) => theme.fg("border", text); const horizontal = borderColor(theme.boxSharp.horizontal.repeat(Math.max(1, width))); const innerWidth = Math.max(1, width - 2); - const content = this.#lines.map(line => { + const content: string[] = []; + for (const line of this.#lines) { const normalized = replaceTabs(line); - const fitted = truncateToWidth(normalized, innerWidth); - const pad = Math.max(0, innerWidth - visibleWidth(fitted)); - return `${borderColor(theme.boxSharp.vertical)}${fitted}${padding(pad)}${borderColor(theme.boxSharp.vertical)}`; - }); + const { indent, body } = splitLeadingSpacesForWrap(normalized, innerWidth); + const wrapped = wrapTextWithAnsi(body, Math.max(1, innerWidth - visibleWidth(indent))); + for (const wrappedBody of wrapped.length > 0 ? wrapped : [""]) { + const wrappedLine = `${indent}${wrappedBody}`; + const pad = Math.max(0, innerWidth - visibleWidth(wrappedLine)); + content.push( + `${borderColor(theme.boxSharp.vertical)}${wrappedLine}${padding(pad)}${borderColor(theme.boxSharp.vertical)}`, + ); + } + } return [horizontal, ...content, horizontal]; } } export class HookSelectorComponent extends Container { - #options: string[]; - #filteredOptions: string[]; + #options: HookSelectorOption[]; + #filteredOptions: HookSelectorOption[]; #searchQuery = ""; #selectedIndex: number; #maxVisible: number; @@ -112,15 +147,15 @@ export class HookSelectorComponent extends Container { #sliderComponent: Text | undefined; constructor( title: string, - options: string[], + options: HookSelectorOptionInput[], onSelect: (option: string) => void, onCancel: () => void, opts?: HookSelectorOptions, ) { super(); - this.#options = options; - this.#filteredOptions = options; + this.#options = options.map(normalizeHookSelectorOption); + this.#filteredOptions = this.#options; this.#selectedIndex = Math.min(opts?.initialIndex ?? 0, this.#filteredOptions.length - 1); this.#maxVisible = Math.max(3, opts?.maxVisible ?? 12); this.#onSelectCallback = onSelect; @@ -156,7 +191,7 @@ export class HookSelectorComponent extends Container { opts?.onTimeout?.(); const selected = this.#filteredOptions[this.#selectedIndex]; if (selected) { - this.#onSelectCallback(selected); + this.#onSelectCallback(selected.label); } else { this.#onCancelCallback(); } @@ -195,10 +230,14 @@ export class HookSelectorComponent extends Container { if (option === undefined) continue; const isSelected = i === this.#selectedIndex; const label = isSelected - ? renderInlineMarkdown(option, mdTheme, t => theme.fg("accent", t)) - : renderInlineMarkdown(option, mdTheme, t => theme.fg("text", t)); + ? renderInlineMarkdown(option.label, mdTheme, t => theme.fg("accent", t)) + : renderInlineMarkdown(option.label, mdTheme, t => theme.fg("text", t)); const prefix = isSelected ? theme.fg("accent", `${theme.nav.cursor} `) : " "; lines.push(prefix + label); + if (option.description) { + const description = renderInlineMarkdown(option.description, mdTheme, t => theme.fg("muted", t)); + lines.push(` ${description}`); + } } if (total === 0) { @@ -273,7 +312,9 @@ export class HookSelectorComponent extends Container { #setSearchQuery(query: string): void { this.#searchQuery = query; - this.#filteredOptions = query.trim() ? fuzzyFilter(this.#options, query, option => option) : this.#options; + this.#filteredOptions = query.trim() + ? fuzzyFilter(this.#options, query, option => `${option.label} ${option.description ?? ""}`) + : this.#options; this.#selectedIndex = 0; this.#updateList(); } @@ -322,7 +363,7 @@ export class HookSelectorComponent extends Container { } } else if (matchesKey(keyData, "enter") || matchesKey(keyData, "return") || keyData === "\n") { const selected = this.#filteredOptions[this.#selectedIndex]; - if (selected) this.#onSelectCallback(selected); + if (selected) this.#onSelectCallback(selected.label); } else if (matchesKey(keyData, "left") || (this.#slider && !this.#isSearchEnabled() && keyData === "h")) { if (this.#slider) this.#moveSlider(-1); else this.#onLeftCallback?.(); diff --git a/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts b/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts index 1f7e191a2..7d2ea98fb 100644 --- a/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts +++ b/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts @@ -10,6 +10,7 @@ import type { ExtensionError, ExtensionUIContext, ExtensionUIDialogOptions, + ExtensionUISelectItem, ExtensionUiComponent, ExtensionWidgetContent, ExtensionWidgetOptions, @@ -483,7 +484,7 @@ export class ExtensionUiController { createBackgroundUiContext(): ExtensionUIContext { return { - select: async (_title: string, _options: string[], _dialogOptions) => undefined, + select: async (_title: string, _options: ExtensionUISelectItem[], _dialogOptions) => undefined, confirm: async (_title: string, _message: string, _dialogOptions) => false, input: async (_title: string, _placeholder?: string, _dialogOptions?: unknown) => undefined, notify: () => {}, @@ -581,7 +582,7 @@ export class ExtensionUiController { */ showHookSelector( title: string, - options: string[], + options: ExtensionUISelectItem[], dialogOptions?: ExtensionUIDialogOptions, extra?: { slider?: HookSelectorSlider }, ): Promise { diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index f5725f6c8..e3792dc26 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -40,6 +40,7 @@ import { isSettingsInitialized, Settings, settings } from "../config/settings"; import type { ExtensionUIContext, ExtensionUIDialogOptions, + ExtensionUISelectItem, ExtensionWidgetContent, ExtensionWidgetOptions, } from "../extensibility/extensions"; @@ -2896,7 +2897,7 @@ export class InteractiveMode implements InteractiveModeContext { showHookSelector( title: string, - options: string[], + options: ExtensionUISelectItem[], dialogOptions?: ExtensionUIDialogOptions, extra?: { slider?: HookSelectorSlider }, ): Promise { diff --git a/packages/coding-agent/src/modes/rpc/rpc-mode.ts b/packages/coding-agent/src/modes/rpc/rpc-mode.ts index 0a2f69a01..69173a5ad 100644 --- a/packages/coding-agent/src/modes/rpc/rpc-mode.ts +++ b/packages/coding-agent/src/modes/rpc/rpc-mode.ts @@ -12,10 +12,12 @@ */ import { getOAuthProviders } from "@oh-my-pi/pi-ai/utils/oauth"; import { $env, readJsonl, Snowflake } from "@oh-my-pi/pi-utils"; -import type { - ExtensionUIContext, - ExtensionUIDialogOptions, - ExtensionWidgetOptions, +import { + type ExtensionUIContext, + type ExtensionUIDialogOptions, + type ExtensionUISelectItem, + type ExtensionWidgetOptions, + getExtensionUISelectOptionLabel, } from "../../extensibility/extensions"; import { type Theme, theme } from "../../modes/theme/theme"; import type { AgentSession } from "../../session/agent-session"; @@ -256,11 +258,20 @@ export async function runRpcMode( return promise; } - select(title: string, options: string[], dialogOptions?: ExtensionUIDialogOptions): Promise { + select( + title: string, + options: ExtensionUISelectItem[], + dialogOptions?: ExtensionUIDialogOptions, + ): Promise { return this.#createDialogPromise( dialogOptions, undefined, - { method: "select", title, options, timeout: dialogOptions?.timeout }, + { + method: "select", + title, + options: options.map(getExtensionUISelectOptionLabel), + timeout: dialogOptions?.timeout, + }, response => parseValueDialogResponse(response, dialogOptions), ); } diff --git a/packages/coding-agent/src/modes/types.ts b/packages/coding-agent/src/modes/types.ts index 544ef6f91..9a6708eec 100644 --- a/packages/coding-agent/src/modes/types.ts +++ b/packages/coding-agent/src/modes/types.ts @@ -7,6 +7,7 @@ import type { Settings } from "../config/settings"; import type { ExtensionUIContext, ExtensionUIDialogOptions, + ExtensionUISelectItem, ExtensionWidgetContent, ExtensionWidgetOptions, } from "../extensibility/extensions"; @@ -297,7 +298,7 @@ export interface InteractiveModeContext { setHookStatus(key: string, text: string | undefined): void; showHookSelector( title: string, - options: string[], + options: ExtensionUISelectItem[], dialogOptions?: ExtensionUIDialogOptions, ): Promise; hideHookSelector(): void; diff --git a/packages/coding-agent/src/prompts/tools/ask.md b/packages/coding-agent/src/prompts/tools/ask.md index 631ac3356..0fc7ada1a 100644 --- a/packages/coding-agent/src/prompts/tools/ask.md +++ b/packages/coding-agent/src/prompts/tools/ask.md @@ -8,6 +8,7 @@ Asks user when you need clarification or input during task execution. - Use `recommended: ` to mark default (0-indexed); " (Recommended)" added automatically - Use `questions` for multiple related questions instead of asking one at a time - Set `multi: true` on question to allow multiple selections +- Use short option labels; put explanatory tradeoffs in `description` instead of merging them into the label @@ -22,7 +23,7 @@ Asks user when you need clarification or input during task execution. # Single question -questions: [{"id": "auth_method", "question": "Which authentication method should this API use?", "options": [{"label": "JWT"}, {"label": "OAuth2"}, {"label": "Session cookies"}], "recommended": 0}] +questions: [{"id": "auth_method", "question": "Which authentication method should this API use?", "options": [{"label": "JWT", "description": "Bearer tokens for stateless API clients."}, {"label": "OAuth2", "description": "Delegated authorization with external identity providers."}, {"label": "Session cookies", "description": "Browser-first authentication backed by server-side sessions."}], "recommended": 0}] # Multiple questions questions: [{"id": "storage_type", "question": "Which storage backend?", "options": [{"label": "SQLite"}, {"label": "PostgreSQL"}]}, {"id": "auth_method", "question": "Which auth method?", "options": [{"label": "JWT"}, {"label": "Session cookies"}]}] diff --git a/packages/coding-agent/src/tools/ask.ts b/packages/coding-agent/src/tools/ask.ts index 6c67a6e4c..8114b877b 100644 --- a/packages/coding-agent/src/tools/ask.ts +++ b/packages/coding-agent/src/tools/ask.ts @@ -20,6 +20,7 @@ import { type Component, Container, Markdown, renderInlineMarkdown, TERMINAL, Te import { prompt, untilAborted } from "@oh-my-pi/pi-utils"; import * as z from "zod/v4"; import type { RenderResultOptions } from "../extensibility/custom-tools/types"; +import type { ExtensionUISelectItem } from "../extensibility/extensions"; import { getMarkdownTheme, type Theme, theme } from "../modes/theme/theme"; import askDescription from "../prompts/tools/ask.md" with { type: "text" }; import { renderStatusLine } from "../tui"; @@ -33,6 +34,7 @@ import { ToolAbortError } from "./tool-errors"; const OptionItem = z.object({ label: z.string().describe("display label"), + description: z.string().describe("optional explanatory text displayed below the label").optional(), }); const QuestionItem = z.object({ @@ -69,6 +71,23 @@ export interface AskToolDetails { results?: QuestionResult[]; } +interface AskOption { + label: string; + description?: string; +} + +function getAskOptionLabel(option: AskOption): string { + return option.label; +} + +function getSelectOptionLabel(option: ExtensionUISelectItem): string { + return typeof option === "string" ? option : option.label; +} + +function toSelectOption(option: AskOption, label = option.label): ExtensionUISelectItem { + return option.description ? { label, description: option.description } : label; +} + // ============================================================================= // Constants // ============================================================================= @@ -81,24 +100,25 @@ function getDoneOptionLabel(): string { } /** Add "(Recommended)" suffix to the option at the given index if not already present */ -function addRecommendedSuffix(labels: string[], recommendedIndex?: number): string[] { - if (recommendedIndex === undefined || recommendedIndex < 0 || recommendedIndex >= labels.length) { - return labels; +function addRecommendedSuffix(options: AskOption[], recommendedIndex?: number): ExtensionUISelectItem[] { + if (recommendedIndex === undefined || recommendedIndex < 0 || recommendedIndex >= options.length) { + return options.map(option => toSelectOption(option)); } - return labels.map((label, i) => { - if (i === recommendedIndex && !label.endsWith(RECOMMENDED_SUFFIX)) { - return label + RECOMMENDED_SUFFIX; - } - return label; + return options.map((option, i) => { + const label = + i === recommendedIndex && !option.label.endsWith(RECOMMENDED_SUFFIX) + ? option.label + RECOMMENDED_SUFFIX + : option.label; + return toSelectOption(option, label); }); } -function getAutoSelectionOnTimeout(optionLabels: string[], recommended?: number): string[] { - if (optionLabels.length === 0) return []; - if (typeof recommended === "number" && recommended >= 0 && recommended < optionLabels.length) { - return [optionLabels[recommended]]; +function getAutoSelectionOnTimeout(options: AskOption[], recommended?: number): string[] { + if (options.length === 0) return []; + if (typeof recommended === "number" && recommended >= 0 && recommended < options.length) { + return [options[recommended]!.label]; } - return [optionLabels[0]]; + return [options[0]!.label]; } /** Strip "(Recommended)" suffix from a label */ @@ -134,7 +154,7 @@ interface AskSingleQuestionOptions { interface UIContext { select( prompt: string, - options: string[], + options: ExtensionUISelectItem[], options_?: { initialIndex?: number; timeout?: number; @@ -157,7 +177,7 @@ interface UIContext { async function askSingleQuestion( ui: UIContext, question: string, - optionLabels: string[], + questionOptions: AskOption[], multi: boolean, options: AskSingleQuestionOptions = {}, ): Promise { @@ -169,7 +189,7 @@ async function askSingleQuestion( const selectOption = async ( prompt: string, - optionsToShow: string[], + optionsToShow: ExtensionUISelectItem[], initialIndex?: number, ): Promise<{ choice: string | undefined; timedOut: boolean; navigation?: "back" | "forward" }> => { let timeoutTriggered = false; @@ -218,18 +238,19 @@ async function askSingleQuestion( const promptWithProgress = navigation?.progressText ? `${question} (${navigation.progressText})` : question; if (multi) { const selected = new Set(selectedOptions); - let cursorIndex = Math.min(Math.max(recommended ?? 0, 0), Math.max(optionLabels.length - 1, 0)); + let cursorIndex = Math.min(Math.max(recommended ?? 0, 0), Math.max(questionOptions.length - 1, 0)); const firstSelected = selectedOptions[0]; if (firstSelected) { - const selectedIndex = optionLabels.indexOf(firstSelected); + const selectedIndex = questionOptions.findIndex(option => option.label === firstSelected); if (selectedIndex >= 0) cursorIndex = selectedIndex; } while (true) { - const opts: string[] = []; + const opts: ExtensionUISelectItem[] = []; - for (const opt of optionLabels) { - const checkbox = selected.has(opt) ? theme.checkbox.checked : theme.checkbox.unchecked; - opts.push(`${checkbox} ${opt}`); + for (const opt of questionOptions) { + const checkbox = selected.has(opt.label) ? theme.checkbox.checked : theme.checkbox.unchecked; + const displayLabel = `${checkbox} ${opt.label}`; + opts.push(toSelectOption(opt, displayLabel)); } if (!navigation?.allowForward && selected.size > 0) { @@ -269,7 +290,7 @@ async function askSingleQuestion( break; } - const selectedIdx = opts.indexOf(choice); + const selectedIdx = opts.findIndex(opt => getSelectOptionLabel(opt) === choice); if (selectedIdx >= 0) { cursorIndex = selectedIdx; } @@ -297,16 +318,16 @@ async function askSingleQuestion( } selectedOptions = Array.from(selected); } else { - const displayLabels = addRecommendedSuffix(optionLabels, recommended); - const optionsWithNavigation = [...displayLabels, OTHER_OPTION]; + const displayOptions = addRecommendedSuffix(questionOptions, recommended); + const optionsWithNavigation: ExtensionUISelectItem[] = [...displayOptions, OTHER_OPTION]; let initialIndex = recommended; const previouslySelected = selectedOptions[0]; if (previouslySelected) { - const selectedIndex = optionLabels.indexOf(previouslySelected); + const selectedIndex = questionOptions.findIndex(option => option.label === previouslySelected); if (selectedIndex >= 0) initialIndex = selectedIndex; } else if (customInput !== undefined) { - initialIndex = displayLabels.length; + initialIndex = displayOptions.length; } if (initialIndex !== undefined) { const maxIndex = Math.max(optionsWithNavigation.length - 1, 0); @@ -346,7 +367,7 @@ async function askSingleQuestion( } if (timedOut && selectedOptions.length === 0 && customInput === undefined) { - selectedOptions = getAutoSelectionOnTimeout(optionLabels, recommended); + selectedOptions = getAutoSelectionOnTimeout(questionOptions, recommended); } return { selectedOptions, customInput, timedOut }; @@ -442,12 +463,16 @@ export class AskTool implements AgentTool { q: AskParams["questions"][number], options?: { previous?: QuestionResult; navigation?: NavigationControls }, ) => { - const optionLabels = q.options.map(o => o.label); + const questionOptions = q.options.map(option => ({ + label: option.label, + ...(option.description?.trim() ? { description: option.description.trim() } : {}), + })); + const optionLabels = questionOptions.map(getAskOptionLabel); try { const { selectedOptions, customInput, navigation, cancelled, timedOut } = await askSingleQuestion( ui, q.question, - optionLabels, + questionOptions, q.multi ?? false, { recommended: q.recommended, @@ -568,14 +593,19 @@ export class AskTool implements AgentTool { // TUI Renderer // ============================================================================= +interface AskRenderOption { + label: string; + description?: string; +} + interface AskRenderArgs { question?: string; - options?: Array<{ label: string }>; + options?: AskRenderOption[]; multi?: boolean; questions?: Array<{ id: string; question: string; - options: Array<{ label: string }>; + options: AskRenderOption[]; multi?: boolean; }>; } @@ -634,6 +664,13 @@ export const askToolRenderer = { const optBranch = isLastOpt ? uiTheme.tree.last : uiTheme.tree.branch; const optLabel = renderInlineMarkdown(opt.label, mdTheme, t => uiTheme.fg("muted", t)); optText += `\n ${uiTheme.fg("dim", continuation)} ${uiTheme.fg("dim", optBranch)} ${uiTheme.fg("dim", uiTheme.checkbox.unchecked)} ${optLabel}`; + if (opt.description?.trim()) { + const optContinuation = isLastOpt ? " " : uiTheme.tree.vertical; + const description = renderInlineMarkdown(opt.description.trim(), mdTheme, t => + uiTheme.fg("dim", t), + ); + optText += `\n ${uiTheme.fg("dim", continuation)} ${uiTheme.fg("dim", optContinuation)} ${uiTheme.fg("dim", "↳")} ${description}`; + } } container.addChild(new Text(optText, 0, 0)); } @@ -661,6 +698,11 @@ export const askToolRenderer = { const branch = isLast ? uiTheme.tree.last : uiTheme.tree.branch; const optLabel = renderInlineMarkdown(opt.label, mdTheme, t => uiTheme.fg("muted", t)); optText += `\n ${uiTheme.fg("dim", branch)} ${uiTheme.fg("dim", uiTheme.checkbox.unchecked)} ${optLabel}`; + if (opt.description?.trim()) { + const continuation = isLast ? " " : uiTheme.tree.vertical; + const description = renderInlineMarkdown(opt.description.trim(), mdTheme, t => uiTheme.fg("dim", t)); + optText += `\n ${uiTheme.fg("dim", continuation)} ${uiTheme.fg("dim", "↳")} ${description}`; + } } container.addChild(new Text(optText, 0, 0)); } diff --git a/packages/coding-agent/test/hook-selector-overflow.test.ts b/packages/coding-agent/test/hook-selector-overflow.test.ts index 713a17cc0..4049ffc21 100644 --- a/packages/coding-agent/test/hook-selector-overflow.test.ts +++ b/packages/coding-agent/test/hook-selector-overflow.test.ts @@ -31,4 +31,94 @@ describe("HookSelectorComponent", () => { expect(visibleWidth(Bun.stripANSI(line))).toBeLessThanOrEqual(width); } }); + + it("wraps outlined option text without omitting the tail", () => { + const options = [ + "Option A: Move to OMP-native only by migrating reusable shared AI instructions into .omp/AGENTS.md, .omp/rules, .omp/skills, and .omp/agents while deliberately not creating a root .github directory.", + "Option B: Keep dual support by migrating canonical instructions into .omp while also maintaining a root .github/copilot-instructions.md compatibility bridge for editors that do not understand OMP resources yet.", + ]; + const component = new HookSelectorComponent( + "Which migration stance should be used?", + options, + () => {}, + () => {}, + { outline: true, initialIndex: 0 }, + ); + + const width = 72; + const lines = component.render(width); + const plain = lines.map(line => Bun.stripANSI(line)).join("\n"); + const normalizedPlain = plain.replace(/[\u2500-\u257f]/g, " ").replace(/\s+/g, " "); + expect(normalizedPlain).toContain("not creating a root .github directory"); + expect(normalizedPlain).toContain("do not understand OMP resources yet"); + for (const line of lines) { + expect(visibleWidth(Bun.stripANSI(line))).toBeLessThanOrEqual(width); + } + }); + + it("renders option descriptions as separate wrapped rows", () => { + const options = [ + { + label: "Use existing local credentials", + description: + "Authenticate via the provider keys and OAuth state already configured under ~/.omp without opening a new browser-based setup flow.", + }, + { + label: "Set up Oh My Pi in terminal", + description: + "Launch the local terminal UI to add provider keys, select models, and keep the current editor session waiting for the configured credentials.", + }, + ]; + const component = new HookSelectorComponent( + "How should authentication continue?", + options, + () => {}, + () => {}, + { outline: true, initialIndex: 0 }, + ); + + const width = 76; + const lines = component.render(width); + const plainLines = lines.map(line => Bun.stripANSI(line)); + const normalizedPlain = plainLines + .join("\n") + .replace(/[\u2500-\u257f]/g, " ") + .replace(/\s+/g, " "); + const labelLineIndex = plainLines.findIndex(line => line.includes("Use existing local credentials")); + const descriptionLineIndex = plainLines.findIndex(line => line.includes("Authenticate via the provider keys")); + expect(labelLineIndex).toBeGreaterThanOrEqual(0); + expect(descriptionLineIndex).toBeGreaterThan(labelLineIndex); + expect(normalizedPlain).toContain("without opening a new browser-based setup flow"); + expect(normalizedPlain).toContain("keep the current editor session waiting for the configured credentials"); + for (const line of lines) { + expect(visibleWidth(Bun.stripANSI(line))).toBeLessThanOrEqual(width); + } + }); + + it("filters options by description text", () => { + const component = new HookSelectorComponent( + "Which setup path should be used?", + [ + { label: "Path A", description: "Reuse the credentials already available in the environment." }, + { label: "Path B", description: "Launch a browser flow to authorize a new provider account." }, + { label: "Path C", description: "Open the local settings file and edit provider keys manually." }, + { label: "Path D", description: "Skip provider setup and continue with offline-only tools." }, + ], + () => {}, + () => {}, + { outline: true, maxVisible: 3 }, + ); + + for (const key of "browser") { + component.handleInput(key); + } + + const plain = component + .render(76) + .map(line => Bun.stripANSI(line)) + .join("\n"); + expect(plain).toContain("Path B"); + expect(plain).toContain("Launch a browser flow"); + expect(plain).not.toContain("Path A"); + }); }); diff --git a/packages/coding-agent/test/tools/ask.test.ts b/packages/coding-agent/test/tools/ask.test.ts index 9b7e5db6e..21a21318f 100644 --- a/packages/coding-agent/test/tools/ask.test.ts +++ b/packages/coding-agent/test/tools/ask.test.ts @@ -2,6 +2,7 @@ import { beforeAll, describe, expect, it, vi } from "bun:test"; import { stripVTControlCharacters } from "node:util"; import type { AgentToolContext } from "@oh-my-pi/pi-agent-core"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import type { ExtensionUISelectItem } from "@oh-my-pi/pi-coding-agent/extensibility/extensions"; import { getThemeByName, initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; import { AskTool, askToolRenderer } from "@oh-my-pi/pi-coding-agent/tools/ask"; @@ -21,7 +22,7 @@ function createSession(overrides: Partial = {}): ToolSession { function createContext(args: { select: ( prompt: string, - options: string[], + options: ExtensionUISelectItem[], dialogOptions?: { initialIndex?: number; timeout?: number; @@ -60,6 +61,10 @@ function stripAnsi(text: string): string { return stripVTControlCharacters(text); } +function selectItemLabel(option: ExtensionUISelectItem | undefined): string | undefined { + return typeof option === "string" ? option : option?.label; +} + beforeAll(async () => { await initTheme(false); }); @@ -98,8 +103,11 @@ describe("AskTool cancellation", () => { // deliberate indefinitely. The dialog timeout is opt-in via the `ask.timeout` setting. const tool = new AskTool(createSession()); const select = vi.fn( - async (_prompt: string, options: string[], _dialogOptions?: { initialIndex?: number; timeout?: number }) => - options[0], + async ( + _prompt: string, + options: ExtensionUISelectItem[], + _dialogOptions?: { initialIndex?: number; timeout?: number }, + ) => (typeof options[0] === "string" ? options[0] : options[0]?.label), ); const context = createContext({ select }); @@ -164,13 +172,14 @@ describe("AskTool cancellation", () => { const select = vi.fn( async ( _prompt: string, - options: string[], + options: ExtensionUISelectItem[], dialogOptions?: { initialIndex?: number; timeout?: number; onTimeout?: () => void }, ) => { const timeout = dialogOptions?.timeout ?? 1; await Bun.sleep(timeout + 5); dialogOptions?.onTimeout?.(); - return options[dialogOptions?.initialIndex ?? 0]; + const selected = options[dialogOptions?.initialIndex ?? 0]; + return typeof selected === "string" ? selected : selected?.label; }, ); const context = createContext({ @@ -387,6 +396,140 @@ describe("AskTool cancellation", () => { }); }); +describe("AskTool option descriptions", () => { + it("passes descriptions to the selector while returning selected labels", async () => { + const tool = new AskTool(createSession()); + const select = vi.fn(async (_prompt: string, options: ExtensionUISelectItem[]) => { + expect(options[0]).toEqual({ + label: "Use local credentials", + description: "Authenticate with provider keys already configured under ~/.omp.", + }); + expect(options[1]).toEqual({ + label: "Set up in terminal", + description: "Launch the terminal setup flow to add credentials before continuing.", + }); + const selected = options[1]; + return typeof selected === "string" ? selected : selected?.label; + }); + const context = createContext({ select }); + + const result = await tool.execute( + "call-option-descriptions", + { + questions: [ + { + id: "auth", + question: "How should authentication continue?", + options: [ + { + label: "Use local credentials", + description: "Authenticate with provider keys already configured under ~/.omp.", + }, + { + label: "Set up in terminal", + description: "Launch the terminal setup flow to add credentials before continuing.", + }, + ], + }, + ], + }, + undefined, + undefined, + context, + ); + + expect(result.content[0]?.type).toBe("text"); + if (result.content[0]?.type !== "text") { + throw new Error("Expected text result"); + } + expect(result.content[0].text).toContain("User selected: Set up in terminal"); + expect(result.details?.selectedOptions).toEqual(["Set up in terminal"]); + expect(result.content[0].text).not.toContain("Launch the terminal setup flow"); + expect(result.details?.options).toEqual(["Use local credentials", "Set up in terminal"]); + }); + + it("renders descriptions under labels in ask call previews", async () => { + const theme = await getThemeByName("dark"); + expect(theme).toBeDefined(); + const rendered = askToolRenderer.renderCall( + { + question: "How should authentication continue?", + options: [ + { + label: "Use local credentials", + description: "Authenticate with provider keys already configured under ~/.omp.", + }, + { + label: "Set up in terminal", + description: "Launch the terminal setup flow to add credentials before continuing.", + }, + ], + }, + { expanded: true, isPartial: false }, + theme!, + ); + const renderedLines = stripAnsi(rendered.render(120).join("\n")).split("\n"); + const labelLine = renderedLines.findIndex(line => line.includes("Use local credentials")); + const descriptionLine = renderedLines.findIndex(line => + line.includes("Authenticate with provider keys already configured"), + ); + expect(labelLine).toBeGreaterThanOrEqual(0); + expect(descriptionLine).toBeGreaterThan(labelLine); + }); + + it("forwards descriptions through multi-select and returns bare labels", async () => { + const tool = new AskTool(createSession()); + let step = 0; + let firstOptions: ExtensionUISelectItem[] = []; + const editor = vi.fn(async () => undefined); + const context = createContext({ + select: async (_prompt, options) => { + if (step === 0) { + firstOptions = options; + step += 1; + return selectItemLabel(options.find(o => selectItemLabel(o)?.endsWith("alpha"))); + } + if (step === 1) { + step += 1; + return selectItemLabel(options.find(o => selectItemLabel(o)?.endsWith("beta"))); + } + return "Other (type your own)"; + }, + editor, + }); + + const result = await tool.execute( + "call-multi-desc", + { + questions: [ + { + id: "multi", + question: "Pick answers", + options: [ + { label: "alpha", description: "First choice detail." }, + { label: "beta", description: "Second choice detail." }, + ], + multi: true, + }, + ], + }, + undefined, + undefined, + context, + ); + + expect(result.details?.selectedOptions).toEqual(["alpha", "beta"]); + expect(result.content[0]?.type).toBe("text"); + if (result.content[0]?.type !== "text") { + throw new Error("Expected text result"); + } + expect(result.content[0].text).toContain("User selected: alpha, beta"); + expect(result.content[0].text).not.toContain("First choice detail"); + const alphaOption = firstOptions.find(o => selectItemLabel(o)?.endsWith("alpha")); + expect(typeof alphaOption === "object" ? alphaOption.description : undefined).toBe("First choice detail."); + }); +}); + describe("AskTool custom input", () => { it("routes custom input through editor and preserves raw multiline strings", async () => { const tool = new AskTool(createSession()); @@ -556,9 +699,9 @@ describe("AskTool custom input", () => { select: async (_prompt, options) => { if (step === 0) { step += 1; - const alphaOption = options.find(option => option.endsWith("alpha")); + const alphaOption = options.find(option => selectItemLabel(option)?.endsWith("alpha")); if (!alphaOption) throw new Error("Missing alpha option"); - return alphaOption; + return selectItemLabel(alphaOption); } return "Other (type your own)"; }, @@ -607,9 +750,9 @@ describe("AskTool custom input", () => { select: async (_prompt, options) => { if (step === 0) { step += 1; - const alphaOption = options.find(option => option.endsWith("alpha")); + const alphaOption = options.find(option => selectItemLabel(option)?.endsWith("alpha")); if (!alphaOption) throw new Error("Missing alpha option"); - return alphaOption; + return selectItemLabel(alphaOption); } return "Other (type your own)"; }, @@ -758,7 +901,7 @@ describe("AskTool multi-question navigation", () => { it("keeps back unavailable on the first question and supports returning from later questions", async () => { const tool = new AskTool(createSession()); - const firstQuestionOptions: string[][] = []; + const firstQuestionOptions: ExtensionUISelectItem[][] = []; let firstVisits = 0; let secondVisits = 0; const context = createContext({