From a53f89b154ec023c7895e18693f838a264f86928 Mon Sep 17 00:00:00 2001 From: shuv Date: Tue, 23 Dec 2025 17:29:09 -0800 Subject: [PATCH 1/2] refactor: remove broken ask tool in favor of askquestion The ask tool used Bus events that never reached the tool's subscription due to Instance.state scoping issues. The askquestion tool (upstream PR #5958) uses HTTP endpoints and works correctly. Removed: - ask tool registration and implementation - Question namespace schemas - DialogQuestion UI components - TuiEvent.QuestionRequest/Response event types - ask.test.ts tests Updated plan.txt to reference askquestion instead of ask. --- packages/opencode/src/cli/cmd/tui/app.tsx | 51 --- .../dialog-question-confirm.tsx | 61 --- .../dialog-question-multi-select.tsx | 203 --------- .../dialog-question-select.tsx | 157 ------- .../dialog-question/dialog-question-text.tsx | 45 -- .../dialog-question/dialog-question.tsx | 300 -------------- .../tui/component/dialog-question/index.ts | 5 - .../tui/component/dialog-question/types.ts | 14 - packages/opencode/src/cli/cmd/tui/event.ts | 3 - packages/opencode/src/question/index.ts | 108 ----- packages/opencode/src/session/prompt/plan.txt | 2 +- packages/opencode/src/tool/ask.ts | 380 ----------------- packages/opencode/src/tool/registry.ts | 2 - packages/opencode/test/tool/ask.test.ts | 387 ------------------ 14 files changed, 1 insertion(+), 1717 deletions(-) delete mode 100644 packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-confirm.tsx delete mode 100644 packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-multi-select.tsx delete mode 100644 packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-select.tsx delete mode 100644 packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-text.tsx delete mode 100644 packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question.tsx delete mode 100644 packages/opencode/src/cli/cmd/tui/component/dialog-question/index.ts delete mode 100644 packages/opencode/src/cli/cmd/tui/component/dialog-question/types.ts delete mode 100644 packages/opencode/src/question/index.ts delete mode 100644 packages/opencode/src/tool/ask.ts delete mode 100644 packages/opencode/test/tool/ask.test.ts diff --git a/packages/opencode/src/cli/cmd/tui/app.tsx b/packages/opencode/src/cli/cmd/tui/app.tsx index 247e853a6881..7e21329c72a6 100644 --- a/packages/opencode/src/cli/cmd/tui/app.tsx +++ b/packages/opencode/src/cli/cmd/tui/app.tsx @@ -31,8 +31,6 @@ import { ToastProvider, useToast } from "./ui/toast" import { ExitProvider, useExit } from "./context/exit" import { Session as SessionApi } from "@/session" import { TuiEvent } from "./event" -import { DialogQuestion } from "@tui/component/dialog-question" -import type { Question } from "@/question" import { KVProvider, useKV } from "./context/kv" import { Provider } from "@/provider/provider" import { ArgsProvider, useArgs, type Args } from "./context/args" @@ -547,55 +545,6 @@ function App() { }) }) - // Handle question requests from the Ask tool - sdk.event.on(TuiEvent.QuestionRequest.type as any, (evt: any) => { - const request = evt.properties as Question.Request - dialog.replace( - () => ( - { - sdk.client.tui.publish({ - body: { - type: TuiEvent.QuestionResponse.type, - properties: { - questionID: request.questionID, - status: "ok", - answers, - }, - }, - } as any) - }} - onCancel={() => { - sdk.client.tui.publish({ - body: { - type: TuiEvent.QuestionResponse.type, - properties: { - questionID: request.questionID, - status: "cancel", - answers: [], - }, - }, - } as any) - }} - /> - ), - () => { - // On escape/close, send cancel response - sdk.client.tui.publish({ - body: { - type: TuiEvent.QuestionResponse.type, - properties: { - questionID: request.questionID, - status: "cancel", - answers: [], - }, - }, - } as any) - }, - ) - }) - sdk.event.on(SessionApi.Event.Deleted.type, (evt) => { if (route.data.type === "session" && route.data.sessionID === evt.properties.info.id) { route.navigate({ type: "home" }) diff --git a/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-confirm.tsx b/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-confirm.tsx deleted file mode 100644 index 7242cd017491..000000000000 --- a/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-confirm.tsx +++ /dev/null @@ -1,61 +0,0 @@ -import { TextAttributes } from "@opentui/core" -import { useTheme, selectedForeground } from "@tui/context/theme" -import { createMemo, For } from "solid-js" -import { useKeyboard } from "@opentui/solid" -import type { Question } from "@/question" - -export interface DialogQuestionConfirmProps { - question: Question.ConfirmQuestion - value: boolean | undefined - onChange: (value: boolean) => void - active: boolean -} - -export function DialogQuestionConfirm(props: DialogQuestionConfirmProps) { - const { theme } = useTheme() - const fg = selectedForeground(theme) - - const selected = createMemo(() => props.value ?? props.question.defaultValue ?? false) - - useKeyboard((evt) => { - if (!props.active) return - - if (evt.name === "left" || evt.name === "right") { - props.onChange(!selected()) - evt.preventDefault() - } - if (evt.name === "y") { - props.onChange(true) - evt.preventDefault() - } - if (evt.name === "n") { - props.onChange(false) - evt.preventDefault() - } - }) - - return ( - - - {props.question.message} - - - - {(val) => { - const active = createMemo(() => selected() === val) - return ( - props.onChange(val)} - > - {val ? "Yes" : "No"} - - ) - }} - - - - ) -} diff --git a/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-multi-select.tsx b/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-multi-select.tsx deleted file mode 100644 index 837016d8d7ca..000000000000 --- a/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-multi-select.tsx +++ /dev/null @@ -1,203 +0,0 @@ -import { ScrollBoxRenderable, TextAttributes } from "@opentui/core" -import { useTheme, selectedForeground } from "@tui/context/theme" -import { For, Show, createEffect, createMemo, createSignal } from "solid-js" -import { useKeyboard, useTerminalDimensions } from "@opentui/solid" -import type { Question } from "@/question" - -export interface DialogQuestionMultiSelectProps { - question: Question.MultiSelectQuestion - value: string[] - onChange: (value: string[]) => void - active: boolean -} - -export function DialogQuestionMultiSelect(props: DialogQuestionMultiSelectProps) { - const { theme } = useTheme() - const fg = selectedForeground(theme) - - const [focusedIndex, setFocusedIndex] = createSignal(0) - - const dimensions = useTerminalDimensions() - const maxHeight = createMemo(() => Math.max(4, Math.floor(dimensions().height * 0.4))) - - let scroll: ScrollBoxRenderable | undefined - const [canScrollUp, setCanScrollUp] = createSignal(false) - const [canScrollDown, setCanScrollDown] = createSignal(false) - - function updateScrollIndicators() { - if (!scroll) return - const y = scroll.y - const height = scroll.height - const total = scroll.scrollHeight - - setCanScrollUp(y > 0) - setCanScrollDown(y + height < total) - } - - createEffect(() => { - const optionsLength = props.question.options.length - const current = focusedIndex() - if (optionsLength === 0) return - if (current >= optionsLength) setFocusedIndex(optionsLength - 1) - }) - - // Track selected values - const selectedValues = createMemo(() => new Set(props.value)) - - function moveTo(index: number) { - const optionsLength = props.question.options.length - if (optionsLength === 0) return - - let next = index - if (next < 0) next = optionsLength - 1 - if (next >= optionsLength) next = 0 - - setFocusedIndex(next) - } - - function toggle(value: string) { - const current = new Set(props.value) - - if (current.has(value)) { - current.delete(value) - props.onChange(Array.from(current)) - return - } - - // Check max constraint - if (props.question.max && current.size >= props.question.max) { - return - } - - current.add(value) - props.onChange(Array.from(current)) - } - - createEffect(() => { - if (!scroll) return - if (!props.active) return - - const option = props.question.options[focusedIndex()] - if (!option) return - - const target = scroll.getChildren().find((child) => child.id === option.value) - if (!target) return - - const y = target.y - scroll.y - if (y >= scroll.height) { - scroll.scrollBy(y - scroll.height + 1) - } - if (y < 0) { - scroll.scrollBy(y) - if (focusedIndex() === 0) scroll.scrollTo(0) - } - - updateScrollIndicators() - }) - - useKeyboard((evt) => { - if (!props.active) return - - const optionsLength = props.question.options.length - if (optionsLength === 0) return - - if (evt.name === "up" || (evt.ctrl && evt.name === "p")) { - moveTo(focusedIndex() - 1) - evt.preventDefault() - } - - if (evt.name === "down" || (evt.ctrl && evt.name === "n")) { - moveTo(focusedIndex() + 1) - evt.preventDefault() - } - - if (evt.name === "pageup") { - moveTo(focusedIndex() - 10) - evt.preventDefault() - } - - if (evt.name === "pagedown") { - moveTo(focusedIndex() + 10) - evt.preventDefault() - } - - if (evt.name === "home") { - moveTo(0) - evt.preventDefault() - } - - if (evt.name === "end") { - moveTo(optionsLength - 1) - evt.preventDefault() - } - - if (evt.name === "space") { - const option = props.question.options[focusedIndex()] - if (!option) return - toggle(option.value) - evt.preventDefault() - } - }) - - return ( - - - {props.question.message} - - - (↑/↓ move, space toggle{props.question.min ? `, min: ${props.question.min}` : ""} - {props.question.max ? `, max: ${props.question.max}` : ""}) - - - - ↑ more options - - - { - scroll = r - setTimeout(() => updateScrollIndicators(), 0) - }} - maxHeight={maxHeight()} - > - - {(option, index) => { - const focused = createMemo(() => index() === focusedIndex()) - const selected = createMemo(() => selectedValues().has(option.value)) - return ( - { - moveTo(index()) - toggle(option.value) - }} - > - {focused() ? "›" : " "} - - {selected() ? "☑" : "☐"} - - {option.label} - {option.hint && {option.hint}} - - ) - }} - - - - - ↓ more options - - - - pgup/pgdn jump • home/end - - - ) -} diff --git a/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-select.tsx b/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-select.tsx deleted file mode 100644 index 8112bdafaa65..000000000000 --- a/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-select.tsx +++ /dev/null @@ -1,157 +0,0 @@ -import { ScrollBoxRenderable, TextAttributes } from "@opentui/core" -import { useTheme, selectedForeground } from "@tui/context/theme" -import { For, Show, createEffect, createMemo, createSignal } from "solid-js" -import { useKeyboard, useTerminalDimensions } from "@opentui/solid" -import type { Question } from "@/question" - -export interface DialogQuestionSelectProps { - question: Question.SelectQuestion - value: string | undefined - onChange: (value: string) => void - active: boolean -} - -export function DialogQuestionSelect(props: DialogQuestionSelectProps) { - const { theme } = useTheme() - const fg = selectedForeground(theme) - - const selectedIndex = createMemo(() => { - const val = props.value ?? props.question.defaultValue - const idx = props.question.options.findIndex((o) => o.value === val) - return idx >= 0 ? idx : 0 - }) - - const dimensions = useTerminalDimensions() - const maxHeight = createMemo(() => Math.max(4, Math.floor(dimensions().height * 0.4))) - - let scroll: ScrollBoxRenderable | undefined - const [canScrollUp, setCanScrollUp] = createSignal(false) - const [canScrollDown, setCanScrollDown] = createSignal(false) - - function updateScrollIndicators() { - if (!scroll) return - const y = scroll.y - const height = scroll.height - const total = scroll.scrollHeight - - setCanScrollUp(y > 0) - setCanScrollDown(y + height < total) - } - - createEffect(() => { - if (!scroll) return - if (!props.active) return - - const option = props.question.options[selectedIndex()] - if (!option) return - - const target = scroll.getChildren().find((child) => child.id === option.value) - if (!target) return - - const y = target.y - scroll.y - if (y >= scroll.height) { - scroll.scrollBy(y - scroll.height + 1) - } - if (y < 0) { - scroll.scrollBy(y) - if (selectedIndex() === 0) scroll.scrollTo(0) - } - - updateScrollIndicators() - }) - - function moveTo(index: number) { - const options = props.question.options - if (options.length === 0) return - - let next = index - if (next < 0) next = options.length - 1 - if (next >= options.length) next = 0 - - props.onChange(options[next].value) - } - - useKeyboard((evt) => { - if (!props.active) return - - if (evt.name === "up" || (evt.ctrl && evt.name === "p")) { - moveTo(selectedIndex() - 1) - evt.preventDefault() - } - - if (evt.name === "down" || (evt.ctrl && evt.name === "n")) { - moveTo(selectedIndex() + 1) - evt.preventDefault() - } - - if (evt.name === "pageup") { - moveTo(selectedIndex() - 10) - evt.preventDefault() - } - - if (evt.name === "pagedown") { - moveTo(selectedIndex() + 10) - evt.preventDefault() - } - - if (evt.name === "home") { - moveTo(0) - evt.preventDefault() - } - - if (evt.name === "end") { - moveTo(props.question.options.length - 1) - evt.preventDefault() - } - }) - - return ( - - - {props.question.message} - - - ↑ more options - - - { - scroll = r - setTimeout(() => updateScrollIndicators(), 0) - }} - maxHeight={maxHeight()} - > - - {(option, index) => { - const selected = createMemo(() => index() === selectedIndex()) - return ( - props.onChange(option.value)} - > - {selected() ? "●" : "○"} - {option.label} - {option.hint && {option.hint}} - - ) - }} - - - - - ↓ more options - - - - ↑/↓ navigate • pgup/pgdn jump - - - ) -} diff --git a/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-text.tsx b/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-text.tsx deleted file mode 100644 index 1e4d6ea5d14f..000000000000 --- a/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-text.tsx +++ /dev/null @@ -1,45 +0,0 @@ -import { TextAttributes, type InputRenderable } from "@opentui/core" -import { useTheme } from "@tui/context/theme" -import { onMount } from "solid-js" -import type { Question } from "@/question" - -export interface DialogQuestionTextProps { - question: Question.TextQuestion - value: string | undefined - onChange: (value: string) => void - active: boolean -} - -export function DialogQuestionText(props: DialogQuestionTextProps) { - const { theme } = useTheme() - let input: InputRenderable - - onMount(() => { - if (props.active) { - setTimeout(() => input?.focus(), 1) - } - // Set default value if provided - if (props.question.defaultValue && !props.value) { - props.onChange(props.question.defaultValue) - } - }) - - return ( - - - {props.question.message} - - - (input = r)} - value={props.value ?? ""} - onInput={(e) => props.onChange(e)} - focusedBackgroundColor={theme.backgroundPanel} - cursorColor={theme.primary} - focusedTextColor={theme.text} - placeholder={props.question.placeholder ?? ""} - /> - - - ) -} diff --git a/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question.tsx b/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question.tsx deleted file mode 100644 index 7752255afeaf..000000000000 --- a/packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question.tsx +++ /dev/null @@ -1,300 +0,0 @@ -import { ScrollBoxRenderable, TextAttributes } from "@opentui/core" -import { useTheme, selectedForeground } from "@tui/context/theme" -import { For, Show, Switch, Match, createEffect, createMemo, onMount } from "solid-js" -import { createStore } from "solid-js/store" -import { useKeyboard, useTerminalDimensions } from "@opentui/solid" -import { useDialog } from "@tui/ui/dialog" -import type { Question } from "@/question" -import { DialogQuestionSelect } from "./dialog-question-select" -import { DialogQuestionMultiSelect } from "./dialog-question-multi-select" -import { DialogQuestionConfirm } from "./dialog-question-confirm" -import { DialogQuestionText } from "./dialog-question-text" - -export interface DialogQuestionProps { - request: Question.Request - onSubmit: (answers: Question.Answer[]) => void - onCancel: () => void -} - -export function DialogQuestion(props: DialogQuestionProps) { - const dialog = useDialog() - const { theme } = useTheme() - const fg = selectedForeground(theme) - - // Initialize answers with default values - const initialAnswers: Record = {} - for (const q of props.request.questions) { - if (q.type === "select" && q.defaultValue) { - initialAnswers[q.id] = q.defaultValue - } else if (q.type === "multi-select" && q.defaultValue) { - initialAnswers[q.id] = q.defaultValue - } else if (q.type === "confirm" && q.defaultValue !== undefined) { - initialAnswers[q.id] = q.defaultValue - } else if (q.type === "text" && q.defaultValue) { - initialAnswers[q.id] = q.defaultValue - } - } - - const [store, setStore] = createStore({ - currentIndex: 0, - answers: initialAnswers, - activeButton: "submit" as "submit" | "cancel", - }) - - const dimensions = useTerminalDimensions() - let scroll: ScrollBoxRenderable | undefined - - onMount(() => { - dialog.setSize("large") - }) - - const bodyMaxHeight = createMemo(() => { - // Dialog is rendered starting at ~1/4 screen height, so keep content <= ~3/4. - const maxDialogHeight = Math.max(10, Math.floor(dimensions().height * 0.75) - 2) - const chromeHeight = 4 // header + footer - return Math.max(6, maxDialogHeight - chromeHeight) - }) - - const summaryWidth = createMemo(() => Math.max(20, Math.min(56, dimensions().width - 10))) - - function truncate(text: string, max: number) { - if (text.length <= max) return text - return text.slice(0, Math.max(0, max - 3)) + "..." - } - - function formatSummaryAnswer(question: Question.Item, value: unknown): string { - switch (question.type) { - case "select": { - const v = typeof value === "string" ? value : question.defaultValue - if (!v) return "(not answered)" - return question.options.find((o) => o.value === v)?.label ?? v - } - case "multi-select": { - const values = Array.isArray(value) ? value : (question.defaultValue ?? []) - if (values.length === 0) return "(none selected)" - const labels = values - .map((v) => question.options.find((o) => o.value === v)?.label ?? v) - .slice(0, 2) - .join(", ") - return values.length > 2 ? `${labels} +${values.length - 2}` : labels - } - case "confirm": { - const v = typeof value === "boolean" ? value : question.defaultValue - if (typeof v !== "boolean") return "(not answered)" - return v ? "Yes" : "No" - } - case "text": { - const v = typeof value === "string" ? value : question.defaultValue - const trimmed = (v ?? "").trim() - if (!trimmed) return "(empty)" - return truncate(trimmed, Math.min(summaryWidth(), 40)) - } - } - } - - // Unused but kept for future use - const _currentQuestion = createMemo(() => props.request.questions[store.currentIndex]) - - createEffect(() => { - const current = props.request.questions[store.currentIndex] - if (!current) return - if (!scroll) return - - const target = scroll.getChildren().find((child) => child.id === current.id) - if (!target) return - - const y = target.y - scroll.y - if (y >= scroll.height) { - scroll.scrollBy(y - scroll.height + 1) - } - if (y < 0) { - scroll.scrollBy(y) - } - }) - - function setAnswer(id: string, value: unknown) { - setStore("answers", id, value) - } - - function buildAnswers(): Question.Answer[] { - const result: Question.Answer[] = [] - for (const q of props.request.questions) { - const value = store.answers[q.id] - switch (q.type) { - case "select": - result.push({ - type: "select", - id: q.id, - value: (value as string) ?? q.defaultValue ?? "", - }) - break - case "multi-select": - result.push({ - type: "multi-select", - id: q.id, - values: (value as string[]) ?? q.defaultValue ?? [], - }) - break - case "confirm": - result.push({ - type: "confirm", - id: q.id, - value: (value as boolean) ?? q.defaultValue ?? false, - }) - break - case "text": - result.push({ - type: "text", - id: q.id, - value: (value as string) ?? q.defaultValue ?? "", - }) - break - } - } - return result - } - - useKeyboard((evt) => { - // Tab to move between questions - if (evt.name === "tab") { - if (evt.shift) { - setStore("currentIndex", Math.max(0, store.currentIndex - 1)) - } else { - setStore("currentIndex", Math.min(props.request.questions.length - 1, store.currentIndex + 1)) - } - evt.preventDefault() - } - - if (evt.ctrl && evt.name === "u") { - scroll?.scrollBy(-Math.max(1, Math.floor((scroll?.height ?? 0) / 2))) - evt.preventDefault() - } - - if (evt.ctrl && evt.name === "d") { - scroll?.scrollBy(Math.max(1, Math.floor((scroll?.height ?? 0) / 2))) - evt.preventDefault() - } - - // Enter to submit (when on last question or button focused) - if (evt.name === "return") { - if (store.activeButton === "cancel") { - props.onCancel() - dialog.clear() - } else { - props.onSubmit(buildAnswers()) - dialog.clear() - } - evt.preventDefault() - } - }) - - return ( - - - - {props.request.title ?? "Question"} - - esc - - - { - scroll = r - }} - maxHeight={bodyMaxHeight()} - > - - {(question, index) => { - const active = createMemo(() => index() === store.currentIndex) - const answerSummary = createMemo(() => formatSummaryAnswer(question, store.answers[question.id])) - - return ( - setStore("currentIndex", index())}> - - ▸ {index() + 1}. - {truncate(question.message, summaryWidth())} - - {truncate(answerSummary(), 24)} - - } - > - - - setAnswer(question.id, v)} - active={true} - /> - - - setAnswer(question.id, v)} - active={true} - /> - - - setAnswer(question.id, v)} - active={true} - /> - - - setAnswer(question.id, v)} - active={true} - /> - - - - - ) - }} - - - - - 1}> - - {store.currentIndex + 1}/{props.request.questions.length} tab next - - - ctrl+u/d scroll - enter submit - - - {(key) => ( - { - if (key === "submit") { - props.onSubmit(buildAnswers()) - } else { - props.onCancel() - } - dialog.clear() - }} - onMouseOver={() => setStore("activeButton", key as "submit" | "cancel")} - > - - {key === "submit" ? "Submit" : "Cancel"} - - - )} - - - - ) -} diff --git a/packages/opencode/src/cli/cmd/tui/component/dialog-question/index.ts b/packages/opencode/src/cli/cmd/tui/component/dialog-question/index.ts deleted file mode 100644 index 32fd1b750d04..000000000000 --- a/packages/opencode/src/cli/cmd/tui/component/dialog-question/index.ts +++ /dev/null @@ -1,5 +0,0 @@ -export { DialogQuestion, type DialogQuestionProps } from "./dialog-question" -export { DialogQuestionSelect, type DialogQuestionSelectProps } from "./dialog-question-select" -export { DialogQuestionMultiSelect, type DialogQuestionMultiSelectProps } from "./dialog-question-multi-select" -export { DialogQuestionConfirm, type DialogQuestionConfirmProps } from "./dialog-question-confirm" -export { DialogQuestionText, type DialogQuestionTextProps } from "./dialog-question-text" diff --git a/packages/opencode/src/cli/cmd/tui/component/dialog-question/types.ts b/packages/opencode/src/cli/cmd/tui/component/dialog-question/types.ts deleted file mode 100644 index 85504004958b..000000000000 --- a/packages/opencode/src/cli/cmd/tui/component/dialog-question/types.ts +++ /dev/null @@ -1,14 +0,0 @@ -import type { Question } from "@/question" - -export interface QuestionDialogProps { - request: Question.Request - onSubmit: (answers: Question.Answer[], comment?: string) => void - onCancel: () => void -} - -export interface QuestionComponentProps { - question: Q - value: unknown - onChange: (value: unknown) => void - active: boolean -} diff --git a/packages/opencode/src/cli/cmd/tui/event.ts b/packages/opencode/src/cli/cmd/tui/event.ts index ee72409c1ac4..9fc3d69df48e 100644 --- a/packages/opencode/src/cli/cmd/tui/event.ts +++ b/packages/opencode/src/cli/cmd/tui/event.ts @@ -1,6 +1,5 @@ import { BusEvent } from "@/bus/bus-event" import z from "zod" -import { Question } from "@/question" export const TuiEvent = { PromptAppend: BusEvent.define("tui.prompt.append", z.object({ text: z.string() })), @@ -37,6 +36,4 @@ export const TuiEvent = { duration: z.number().default(5000).optional().describe("Duration in milliseconds"), }), ), - QuestionRequest: BusEvent.define("tui.question.request", Question.Request), - QuestionResponse: BusEvent.define("tui.question.response", Question.Response), } diff --git a/packages/opencode/src/question/index.ts b/packages/opencode/src/question/index.ts deleted file mode 100644 index fbfb2e864181..000000000000 --- a/packages/opencode/src/question/index.ts +++ /dev/null @@ -1,108 +0,0 @@ -import z from "zod" -import { Identifier } from "../id/id" - -export namespace Question { - // Question types - export const SelectOption = z.object({ - value: z.string(), - label: z.string(), - hint: z.string().optional(), - }) - export type SelectOption = z.infer - - export const SelectQuestion = z.object({ - type: z.literal("select"), - id: z.string(), - message: z.string(), - options: z.array(SelectOption), - defaultValue: z.string().optional(), - }) - export type SelectQuestion = z.infer - - export const MultiSelectQuestion = z.object({ - type: z.literal("multi-select"), - id: z.string(), - message: z.string(), - options: z.array(SelectOption), - defaultValue: z.array(z.string()).optional(), - min: z.number().optional(), - max: z.number().optional(), - }) - export type MultiSelectQuestion = z.infer - - export const ConfirmQuestion = z.object({ - type: z.literal("confirm"), - id: z.string(), - message: z.string(), - defaultValue: z.boolean().optional(), - }) - export type ConfirmQuestion = z.infer - - export const TextQuestion = z.object({ - type: z.literal("text"), - id: z.string(), - message: z.string(), - placeholder: z.string().optional(), - defaultValue: z.string().optional(), - validate: z.string().optional(), // regex pattern for validation - }) - export type TextQuestion = z.infer - - export const Item = z.discriminatedUnion("type", [SelectQuestion, MultiSelectQuestion, ConfirmQuestion, TextQuestion]) - export type Item = z.infer - - // Answer types - export const SelectAnswer = z.object({ - type: z.literal("select"), - id: z.string(), - value: z.string(), - }) - export type SelectAnswer = z.infer - - export const MultiSelectAnswer = z.object({ - type: z.literal("multi-select"), - id: z.string(), - values: z.array(z.string()), - }) - export type MultiSelectAnswer = z.infer - - export const ConfirmAnswer = z.object({ - type: z.literal("confirm"), - id: z.string(), - value: z.boolean(), - }) - export type ConfirmAnswer = z.infer - - export const TextAnswer = z.object({ - type: z.literal("text"), - id: z.string(), - value: z.string(), - }) - export type TextAnswer = z.infer - - export const Answer = z.discriminatedUnion("type", [SelectAnswer, MultiSelectAnswer, ConfirmAnswer, TextAnswer]) - export type Answer = z.infer - - // Request/Response schemas for TUI events - export const Request = z.object({ - questionID: Identifier.schema("question"), - sessionID: Identifier.schema("session"), - messageID: Identifier.schema("message"), - callID: z.string(), - questions: z.array(Item), - title: z.string().optional(), - timeout: z.number().optional(), // timeout in ms - }) - export type Request = z.infer - - export const ResponseStatus = z.enum(["ok", "cancel", "timeout"]) - export type ResponseStatus = z.infer - - export const Response = z.object({ - questionID: Identifier.schema("question"), - status: ResponseStatus, - answers: z.array(Answer).optional(), - comment: z.string().optional(), - }) - export type Response = z.infer -} diff --git a/packages/opencode/src/session/prompt/plan.txt b/packages/opencode/src/session/prompt/plan.txt index 1fb6d1780799..422056265685 100644 --- a/packages/opencode/src/session/prompt/plan.txt +++ b/packages/opencode/src/session/prompt/plan.txt @@ -24,7 +24,7 @@ Use the `askquestion` tool to gather requirements before finalizing your plan. T Ask the user clarifying questions or ask for their opinion when weighing tradeoffs. -If you need a user decision or missing information and the `ask` tool is available, prefer using it instead of asking in plain chat text. +If you need a user decision or missing information, use the `askquestion` tool to gather requirements through a wizard-style interface with labeled question tabs. **NOTE:** At any point in time through this workflow you should feel free to ask the user questions or clarifications. Don't make large assumptions about user intent. The goal is to present a well researched plan to the user, and tie any loose ends before implementation begins. diff --git a/packages/opencode/src/tool/ask.ts b/packages/opencode/src/tool/ask.ts deleted file mode 100644 index 856f58c8d763..000000000000 --- a/packages/opencode/src/tool/ask.ts +++ /dev/null @@ -1,380 +0,0 @@ -import z from "zod" -import { Tool } from "./tool" -import { Question } from "../question" -import { Bus } from "../bus" -import { TuiEvent } from "../cli/cmd/tui/event" -import { Identifier } from "../id/id" -import { Instance } from "../project/instance" -import { Permission } from "../permission" - -const DEFAULT_TIMEOUT = 5 * 60 * 1000 // 5 minutes - -// ============================================================================= -// Normalization helpers for permissive input parsing -// ============================================================================= - -/** - * Normalize a single option: string -> {value, label} or fill missing value/label - */ -function normalizeOption(opt: unknown): { value: string; label: string; hint?: string } { - if (typeof opt === "string") { - return { value: opt, label: opt } - } - if (opt && typeof opt === "object") { - const obj = opt as Record - const value = typeof obj.value === "string" ? obj.value : undefined - const label = typeof obj.label === "string" ? obj.label : undefined - const hint = typeof obj.hint === "string" ? obj.hint : undefined - // Fill missing value/label from the other - const resolvedValue = value ?? label ?? "" - const resolvedLabel = label ?? value ?? "" - return { value: resolvedValue, label: resolvedLabel, ...(hint ? { hint } : {}) } - } - return { value: String(opt), label: String(opt) } -} - -/** - * Normalize options array: handle string arrays or objects with partial fields - */ -function normalizeOptions(options: unknown): { value: string; label: string; hint?: string }[] { - if (!Array.isArray(options)) return [] - return options.map(normalizeOption) -} - -/** - * Extract message from common alias keys: text, prompt, question - */ -function extractMessage(q: Record): string | undefined { - if (typeof q.message === "string") return q.message - if (typeof q.text === "string") return q.text - if (typeof q.prompt === "string") return q.prompt - if (typeof q.question === "string") return q.question - return undefined -} - -/** - * Normalize type field to canonical values - */ -function normalizeType(type: unknown): string { - if (typeof type !== "string") return "text" - const lower = type.toLowerCase().replace(/[_-]/g, "") - if (lower === "multiselect" || lower === "multi") return "multi-select" - if (lower === "select" || lower === "multiselect" || lower === "confirm" || lower === "text") { - // return as-is for canonical types - } - // Map common variations - if (type === "multi_select" || type === "multiselect") return "multi-select" - return type -} - -/** - * Normalize defaultValue for multi-select (wrap string in array) - */ -function normalizeMultiSelectDefault(val: unknown): string[] | undefined { - if (val === undefined || val === null) return undefined - if (Array.isArray(val)) return val.map((v) => String(v)) - if (typeof val === "string") return [val] - return undefined -} - -/** - * Normalize a single question object - */ -function normalizeQuestion(q: unknown): Record { - if (!q || typeof q !== "object") return { type: "text", id: "", message: "" } - const raw = q as Record - - const type = normalizeType(raw.type) - const message = extractMessage(raw) - const id = typeof raw.id === "string" ? raw.id : `q-${Math.random().toString(36).slice(2, 9)}` - - const base: Record = { type, id, message } - - if (type === "select" || type === "multi-select") { - base.options = normalizeOptions(raw.options) - } - - if (type === "select") { - if (typeof raw.defaultValue === "string") base.defaultValue = raw.defaultValue - } - - if (type === "multi-select") { - base.defaultValue = normalizeMultiSelectDefault(raw.defaultValue) - if (typeof raw.min === "number") base.min = raw.min - if (typeof raw.max === "number") base.max = raw.max - } - - if (type === "confirm") { - if (typeof raw.defaultValue === "boolean") base.defaultValue = raw.defaultValue - } - - if (type === "text") { - if (typeof raw.placeholder === "string") base.placeholder = raw.placeholder - if (typeof raw.defaultValue === "string") base.defaultValue = raw.defaultValue - if (typeof raw.validate === "string") base.validate = raw.validate - } - - return base -} - -/** - * Normalize the entire questions array - */ -function normalizeQuestions(questions: unknown): unknown[] { - if (!Array.isArray(questions)) return [] - return questions.map(normalizeQuestion) -} - -/** - * Normalize timeout: accept string containing digits - */ -function normalizeTimeout(timeout: unknown): number | undefined { - if (typeof timeout === "number") return timeout - if (typeof timeout === "string") { - const parsed = parseInt(timeout, 10) - if (!isNaN(parsed) && parsed > 0) return parsed - } - return undefined -} - -/** - * Format validation errors with actionable guidance for models - */ -function formatAskValidationError(error: z.ZodError): string { - const issues = error.issues.map((issue) => { - const path = issue.path.join(".") - return `- ${path}: ${issue.message}` - }) - - return [ - "The ask tool was called with invalid arguments.", - "", - "Issues found:", - ...issues, - "", - "Required format:", - '- questions[].type: "select" | "multi-select" | "confirm" | "text"', - "- questions[].id: unique string identifier", - "- questions[].message: the question text (required)", - "- questions[].options (for select/multi-select): array of {value: string, label: string}", - "", - "Example valid call:", - JSON.stringify( - { - questions: [ - { - type: "select", - id: "choice", - message: "Which option do you prefer?", - options: [ - { value: "a", label: "Option A" }, - { value: "b", label: "Option B" }, - ], - }, - ], - }, - null, - 2, - ), - ].join("\n") -} - -// State for pending questions - uses Instance.state for proper cleanup on disposal -const state = Instance.state( - () => { - // Subscribe to question responses when state is initialized - Bus.subscribe(TuiEvent.QuestionResponse, async (response) => { - const s = await state() - const pending = s.pending[response.properties.questionID] - if (pending) { - pending.resolve(response.properties) - delete s.pending[response.properties.questionID] - } - }) - - return { - pending: {} as Record< - string, - { - resolve: (response: Question.Response) => void - reject: (error: Error) => void - } - >, - } - }, - async (s) => { - // On instance disposal, reject all pending questions - for (const [questionID, item] of Object.entries(s.pending)) { - item.reject(new Permission.RejectedError("", "ask", "", { questionID }, "Instance disposed")) - delete s.pending[questionID] - } - }, -) - -// Permissive parameters schema with preprocessing -// The JSON schema sent to models remains strict (guiding correct output) -// But at runtime, we normalize common deviations before validation -const AskParameters = z.preprocess( - (input) => { - if (!input || typeof input !== "object") return input - const raw = input as Record - return { - ...raw, - questions: normalizeQuestions(raw.questions), - timeout: normalizeTimeout(raw.timeout), - } - }, - z.object({ - questions: z - .array( - z.discriminatedUnion("type", [ - z.object({ - type: z.literal("select"), - id: z.string().describe("Unique identifier for this question"), - message: z.string().describe("The question to ask"), - options: z - .array( - z.object({ - value: z.string().describe("The value returned if this option is selected"), - label: z.string().describe("The display label for this option"), - hint: z.string().optional().describe("Optional hint text"), - }), - ) - .describe("The options to choose from"), - defaultValue: z.string().optional().describe("Default selected value"), - }), - z.object({ - type: z.literal("multi-select"), - id: z.string().describe("Unique identifier for this question"), - message: z.string().describe("The question to ask"), - options: z - .array( - z.object({ - value: z.string().describe("The value returned if this option is selected"), - label: z.string().describe("The display label for this option"), - hint: z.string().optional().describe("Optional hint text"), - }), - ) - .describe("The options to choose from"), - defaultValue: z.array(z.string()).optional().describe("Default selected values"), - min: z.number().optional().describe("Minimum selections required"), - max: z.number().optional().describe("Maximum selections allowed"), - }), - z.object({ - type: z.literal("confirm"), - id: z.string().describe("Unique identifier for this question"), - message: z.string().describe("The yes/no question to ask"), - defaultValue: z.boolean().optional().describe("Default value"), - }), - z.object({ - type: z.literal("text"), - id: z.string().describe("Unique identifier for this question"), - message: z.string().describe("The question to ask"), - placeholder: z.string().optional().describe("Placeholder text"), - defaultValue: z.string().optional().describe("Default value"), - validate: z.string().optional().describe("Regex pattern for validation"), - }), - ]), - ) - .describe("Array of questions to ask the user"), - title: z.string().optional().describe("Optional title for the question dialog"), - timeout: z.number().optional().describe("Timeout in milliseconds (default: 5 minutes)"), - }), -) - -export const AskTool = Tool.define("ask", { - description: - "Ask the user a question and wait for their response. Supports select (single choice), multi-select (multiple choices), confirm (yes/no), and text (free-form input) question types. Use this when you need clarification or input from the user to proceed.", - parameters: AskParameters, - formatValidationError: formatAskValidationError, - async execute(params, ctx) { - const questionID = Identifier.ascending("question") - const s = await state() - - // Create a promise that will be resolved when the user responds - const responsePromise = new Promise((resolve, reject) => { - s.pending[questionID] = { resolve, reject } - }) - - // Create timeout promise - const timeout = params.timeout ?? DEFAULT_TIMEOUT - const timeoutPromise = new Promise((_resolve, reject) => { - setTimeout(() => { - const pending = s.pending[questionID] - if (pending) { - delete s.pending[questionID] - reject(new Error(`Question timed out after ${timeout}ms`)) - } - }, timeout) - }) - - // Publish the question request to the TUI - await Bus.publish(TuiEvent.QuestionRequest, { - questionID, - sessionID: ctx.sessionID, - messageID: ctx.messageID, - callID: ctx.callID ?? "", - questions: params.questions as Question.Item[], - title: params.title, - timeout, - }) - - // Wait for response or timeout - const response = await Promise.race([responsePromise, timeoutPromise]) - - // Handle response status - if (response.status === "cancel") { - return { - title: "Question cancelled", - output: "The user cancelled the question dialog.", - metadata: { status: "cancel" } as Record, - } - } - - if (response.status === "timeout") { - return { - title: "Question timed out", - output: "The question timed out waiting for user response.", - metadata: { status: "timeout" } as Record, - } - } - - // Format successful response - const answers = response.answers ?? [] - const lines: string[] = ["User responses:"] - - for (const answer of answers) { - const question = params.questions.find((q) => q.id === answer.id) - const message = question?.message ?? answer.id - - switch (answer.type) { - case "select": - lines.push(`- ${message}: ${answer.value}`) - break - case "multi-select": - lines.push(`- ${message}: ${answer.values.join(", ")}`) - break - case "confirm": - lines.push(`- ${message}: ${answer.value ? "Yes" : "No"}`) - break - case "text": - lines.push(`- ${message}: ${answer.value}`) - break - } - } - - if (response.comment) { - lines.push(`\nAdditional comment: ${response.comment}`) - } - - return { - title: "User response received", - output: lines.join("\n"), - metadata: { - status: "ok", - answers: answers as unknown, - comment: response.comment ?? "", - } as Record, - } - }, -}) diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index bc8cb36750b2..056c283507ca 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -1,4 +1,3 @@ -import { AskTool } from "./ask" import { BashTool } from "./bash" import { EditTool } from "./edit" import { GlobTool } from "./glob" @@ -93,7 +92,6 @@ export namespace ToolRegistry { return [ InvalidTool, - AskTool, BashTool, ReadTool, GlobTool, diff --git a/packages/opencode/test/tool/ask.test.ts b/packages/opencode/test/tool/ask.test.ts deleted file mode 100644 index b27ba3a10678..000000000000 --- a/packages/opencode/test/tool/ask.test.ts +++ /dev/null @@ -1,387 +0,0 @@ -import { describe, expect, test } from "bun:test" -import { AskTool } from "../../src/tool/ask" - -describe("tool.ask normalization", () => { - // Get the schema from the tool for testing - const getSchema = async () => { - const tool = await AskTool.init() - return tool.parameters - } - - describe("options normalization", () => { - test("accepts string array options and normalizes to {value, label} objects", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "select", - id: "test", - message: "Pick one", - options: ["Option A", "Option B", "Option C"], - }, - ], - } - const result = schema.parse(input) - const q = result.questions[0] as { type: "select"; options: { value: string; label: string }[] } - expect(q.options).toEqual([ - { value: "Option A", label: "Option A" }, - { value: "Option B", label: "Option B" }, - { value: "Option C", label: "Option C" }, - ]) - }) - - test("accepts options with only value and fills label", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "select", - id: "test", - message: "Pick one", - options: [{ value: "a" }, { value: "b" }], - }, - ], - } - const result = schema.parse(input) - const q = result.questions[0] as { type: "select"; options: { value: string; label: string }[] } - expect(q.options).toEqual([ - { value: "a", label: "a" }, - { value: "b", label: "b" }, - ]) - }) - - test("accepts options with only label and fills value", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "select", - id: "test", - message: "Pick one", - options: [{ label: "First" }, { label: "Second" }], - }, - ], - } - const result = schema.parse(input) - const q = result.questions[0] as { type: "select"; options: { value: string; label: string }[] } - expect(q.options).toEqual([ - { value: "First", label: "First" }, - { value: "Second", label: "Second" }, - ]) - }) - - test("preserves hint in options", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "select", - id: "test", - message: "Pick one", - options: [{ value: "a", label: "A", hint: "Choose this for X" }], - }, - ], - } - const result = schema.parse(input) - const q = result.questions[0] as { type: "select"; options: { value: string; label: string; hint?: string }[] } - expect(q.options[0].hint).toBe("Choose this for X") - }) - }) - - describe("message alias normalization", () => { - test("accepts 'text' as alias for 'message'", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "confirm", - id: "test", - text: "Do you want to proceed?", - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].message).toBe("Do you want to proceed?") - }) - - test("accepts 'prompt' as alias for 'message'", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "confirm", - id: "test", - prompt: "Continue?", - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].message).toBe("Continue?") - }) - - test("accepts 'question' as alias for 'message'", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "text", - id: "test", - question: "What is your name?", - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].message).toBe("What is your name?") - }) - - test("prefers 'message' over aliases when present", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "confirm", - id: "test", - message: "Canonical message", - text: "Alias text", - prompt: "Alias prompt", - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].message).toBe("Canonical message") - }) - }) - - describe("type normalization", () => { - test("normalizes 'multiselect' to 'multi-select'", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "multiselect", - id: "test", - message: "Select multiple", - options: ["A", "B"], - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].type).toBe("multi-select") - }) - - test("normalizes 'multi_select' to 'multi-select'", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "multi_select", - id: "test", - message: "Select multiple", - options: ["A", "B"], - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].type).toBe("multi-select") - }) - - test("normalizes 'multi' to 'multi-select'", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "multi", - id: "test", - message: "Select multiple", - options: ["A", "B"], - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].type).toBe("multi-select") - }) - }) - - describe("multi-select defaultValue normalization", () => { - test("wraps string defaultValue into array for multi-select", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "multi-select", - id: "test", - message: "Select options", - options: ["A", "B", "C"], - defaultValue: "A", - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].defaultValue).toEqual(["A"]) - }) - - test("accepts array defaultValue for multi-select as-is", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "multi-select", - id: "test", - message: "Select options", - options: ["A", "B", "C"], - defaultValue: ["A", "B"], - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].defaultValue).toEqual(["A", "B"]) - }) - }) - - describe("timeout normalization", () => { - test("accepts string timeout and parses to number", async () => { - const schema = await getSchema() - const input = { - questions: [{ type: "confirm", id: "test", message: "OK?" }], - timeout: "60000", - } - const result = schema.parse(input) - expect(result.timeout).toBe(60000) - }) - - test("accepts number timeout as-is", async () => { - const schema = await getSchema() - const input = { - questions: [{ type: "confirm", id: "test", message: "OK?" }], - timeout: 30000, - } - const result = schema.parse(input) - expect(result.timeout).toBe(30000) - }) - }) - - describe("id auto-generation", () => { - test("generates id if missing", async () => { - const schema = await getSchema() - const input = { - questions: [{ type: "confirm", message: "OK?" }], - } - const result = schema.parse(input) - expect(result.questions[0].id).toBeDefined() - expect(result.questions[0].id.startsWith("q-")).toBe(true) - }) - }) - - describe("canonical question types", () => { - test("accepts valid select question", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "select", - id: "choice", - message: "Pick one", - options: [ - { value: "a", label: "Option A" }, - { value: "b", label: "Option B" }, - ], - defaultValue: "a", - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].type).toBe("select") - }) - - test("accepts valid multi-select question", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "multi-select", - id: "choices", - message: "Pick many", - options: [ - { value: "a", label: "Option A" }, - { value: "b", label: "Option B" }, - ], - defaultValue: ["a"], - min: 1, - max: 2, - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].type).toBe("multi-select") - }) - - test("accepts valid confirm question", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "confirm", - id: "confirm", - message: "Are you sure?", - defaultValue: true, - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].type).toBe("confirm") - }) - - test("accepts valid text question", async () => { - const schema = await getSchema() - const input = { - questions: [ - { - type: "text", - id: "name", - message: "Enter your name", - placeholder: "John Doe", - defaultValue: "", - validate: "^[a-zA-Z ]+$", - }, - ], - } - const result = schema.parse(input) - expect(result.questions[0].type).toBe("text") - }) - }) - - describe("multi-question support", () => { - test("accepts multiple questions of different types", async () => { - const schema = await getSchema() - const input = { - questions: [ - { type: "confirm", id: "q1", message: "Continue?" }, - { type: "select", id: "q2", message: "Pick one", options: ["A", "B"] }, - { type: "text", id: "q3", message: "Enter value" }, - ], - title: "Setup Wizard", - } - const result = schema.parse(input) - expect(result.questions.length).toBe(3) - expect(result.title).toBe("Setup Wizard") - }) - }) - - describe("error handling", () => { - test("fails on truly missing message (no aliases)", async () => { - const schema = await getSchema() - const input = { - questions: [{ type: "confirm", id: "test" }], - } - expect(() => schema.parse(input)).toThrow() - }) - - test("fails on invalid question type after normalization", async () => { - const schema = await getSchema() - const input = { - questions: [{ type: "invalid_type", id: "test", message: "Test" }], - } - expect(() => schema.parse(input)).toThrow() - }) - }) -}) From cb41db723ee8930f7c7042b5f69c61c518b86941 Mon Sep 17 00:00:00 2001 From: shuv Date: Tue, 23 Dec 2025 17:29:16 -0800 Subject: [PATCH 2/2] docs: add implementation plan for ask tool consolidation --- ...ate-ask-tool-implementations-2025-12-23.md | 297 ++++++++++++++++++ 1 file changed, 297 insertions(+) create mode 100644 CONTEXT/PLAN-197-consolidate-ask-tool-implementations-2025-12-23.md diff --git a/CONTEXT/PLAN-197-consolidate-ask-tool-implementations-2025-12-23.md b/CONTEXT/PLAN-197-consolidate-ask-tool-implementations-2025-12-23.md new file mode 100644 index 000000000000..6fcb1508883e --- /dev/null +++ b/CONTEXT/PLAN-197-consolidate-ask-tool-implementations-2025-12-23.md @@ -0,0 +1,297 @@ +# PLAN-197: Consolidate Ask Tool Implementations + +**Issue:** [#197 - Review and consolidate 'ask' tool implementations - response handler mismatch causes spinner hang](https://github.com/Latitudes-Dev/shuvcode/issues/197) + +**Created:** 2025-12-23 + +**Status:** Implementation Complete (pending manual TUI verification) + +--- + +## Problem Summary + +When using the `ask` tool with a freeform text prompt, submitting a response does not complete the tool - the active spinner continues indefinitely until the user aborts. + +This is caused by having **two separate `ask` tool implementations** with different response mechanisms: + +1. **`ask` tool** - Uses Bus events (`TuiEvent.QuestionRequest/QuestionResponse`) +2. **`askquestion` tool** - Uses HTTP endpoints (`AskQuestion.register/respond`) + +The TUI has handlers for both systems, but they use different UI components and response flows, causing the mismatch. + +--- + +## Decision + +**Keep `askquestion` and remove the `ask` tool entirely.** + +Rationale: + +- `askquestion` is the upstream implementation (PR #5958) and is actively maintained +- `askquestion` uses a more reliable HTTP-based response mechanism +- The `ask` tool's Bus event approach appears to suffer from a scoping/routing mismatch (suspected: Instance state scoping prevents events from reaching the tool's subscription). This is a contributing factor but is not required to be fully proven before removal. +- Removing `ask` eliminates the duplicate code and confusion + +**Note:** We are NOT merging features from `ask` into `askquestion`. The existing `askquestion` functionality (select, multi-select with custom text option) is sufficient. The `confirm` and `text` question types from `ask` will be removed along with the tool. + +--- + +## Compatibility & Migration Strategy + +- **Breaking change:** The `ask` tool will no longer be registered, and external callers using `ask` will receive "unknown tool" errors. +- **Schema migration:** `askquestion` is not a drop-in replacement. Update call sites to use `{ id, label, question, options, multiSelect? }` with 1-6 questions and 2-8 options; remove `message`, `title`, `timeout`, `defaultValue`, `min/max`, `placeholder`, and `validate`. Use explicit Yes/No options or the custom text option to emulate confirm/text flows. +- **Output/metadata migration:** `askquestion` answers are shaped as `{ questionId, values, customText? }` and omit `type`/`value(s)` fields used by `ask`; update any consumers that parse tool output/metadata. +- **Prompt/doc migration:** Update any internal prompts/docs/configs that refer to `ask` to use `askquestion` and the new schema (for example `packages/opencode/src/session/prompt/plan.txt`). +- **User-facing behavior:** `/tools` and any tool listing will no longer include `ask` after removal. + +--- + +## Current Architecture Analysis + +### Tool 1: `ask` (TO BE REMOVED) + +**File:** `packages/opencode/src/tool/ask.ts` + +**Purpose:** Fork-specific implementation for collecting user input via dialogs. + +**Question Types Supported:** + +- `select` - Single choice from options +- `multi-select` - Multiple choices from options +- `confirm` - Yes/no boolean +- `text` - Free-form text input + +**Response Flow (BROKEN):** + +``` +Tool execute() + └─> Bus.publish(TuiEvent.QuestionRequest) + └─> TUI app.tsx handler receives event + └─> Opens DialogQuestion component (overlay dialog) + └─> User submits response + └─> sdk.client.tui.publish() sends TuiEvent.QuestionResponse + └─> Server /tui/publish endpoint + └─> Bus.publish(TuiEvent.QuestionResponse) + └─> Tool's Bus.subscribe() never receives response + (suspected: Instance.state() scoping issue) +``` + +**Key Files (TO BE DELETED):** + +- `packages/opencode/src/tool/ask.ts` - Tool implementation +- `packages/opencode/src/question/index.ts` - Question/Answer schemas +- `packages/opencode/src/cli/cmd/tui/component/dialog-question/` - UI components + +### Tool 2: `askquestion` (TO KEEP) + +**File:** `packages/opencode/src/tool/askquestion.ts` + +**Purpose:** Upstream implementation for wizard-style multi-question flows. + +**Question Types Supported:** + +- Single-select with custom text option +- Multi-select with custom text option +- All questions displayed as tabs in wizard UI + +**Response Flow (WORKING):** + +``` +Tool execute() + └─> AskQuestion.register() creates pending promise + └─> ctx.metadata() updates tool state to "waiting" + └─> TUI detects pending via pendingAskQuestionFromSync() + └─> Opens DialogAskQuestion component (inline in session) + └─> User submits response + └─> HTTP POST to /askquestion/respond + └─> AskQuestion.respond() resolves promise + └─> Tool returns result +``` + +**Key Files (TO KEEP):** + +- `packages/opencode/src/tool/askquestion.ts` - Tool implementation +- `packages/opencode/src/tool/askquestion.txt` - Description +- `packages/opencode/src/askquestion/index.ts` - AskQuestion namespace +- `packages/opencode/src/server/server.ts:1538-1605` - HTTP endpoints +- `packages/opencode/src/cli/cmd/tui/routes/session/index.tsx:395-426,1426-1461` - Sync detection + UI +- `packages/opencode/src/cli/cmd/tui/ui/dialog-askquestion.tsx` - Wizard UI component + +--- + +## Implementation Plan + +### Phase 0: Pre-Removal Audit (New) + +- [x] **Task 0.1:** Search for `ask` references in prompts/docs/configs + - Verify no internal docs, prompts, or scripts reference `ask` + - Update any found references to `askquestion` with the new schema + +- [x] **Task 0.2:** Update plan-mode prompt to avoid `ask` + - File: `packages/opencode/src/session/prompt/plan.txt` + - Replace the "prefer using `ask`" guidance with `askquestion` + +- [x] **Task 0.3:** Audit any `ask` tool call sites in code/tests/fixtures + - Migrate calls to the `askquestion` schema (label/question/options, 1-6 questions, 2-8 options) + - Update any consumers of `ask` output/metadata to the `askquestion` answer shape + - Confirm no remaining `ask` tool invocations exist + +### Phase 1: Remove `ask` Tool Registration + +- [x] **Task 1.1:** Remove `AskTool` import and registration from registry + - File: `packages/opencode/src/tool/registry.ts` + - Remove line 1: `import { AskTool } from "./ask"` + - Remove line 96: `AskTool,` from the tools array + +### Phase 2: Remove `ask` Tool Event Handlers from TUI + +- [x] **Task 2.1:** Remove QuestionRequest event handler from app.tsx + - File: `packages/opencode/src/cli/cmd/tui/app.tsx` + - Remove lines 550-597 (the `sdk.event.on(TuiEvent.QuestionRequest.type, ...)` handler) + - Remove line 34: `import { DialogQuestion } from "@tui/component/dialog-question"` + - Remove line 35: `import type { Question } from "@/question"` + +- [x] **Task 2.2:** Remove Question events from TuiEvent + - File: `packages/opencode/src/cli/cmd/tui/event.ts` + - Remove line 3: `import { Question } from "@/question"` + - Remove lines 40-41: `QuestionRequest` and `QuestionResponse` definitions + +### Phase 3: Delete `ask` Tool Files + +- [x] **Task 3.1:** Delete the ask tool implementation + - Delete: `packages/opencode/src/tool/ask.ts` + +- [x] **Task 3.2:** Delete the Question namespace + - Delete: `packages/opencode/src/question/index.ts` + +- [x] **Task 3.3:** Delete the DialogQuestion component directory + - Delete: `packages/opencode/src/cli/cmd/tui/component/dialog-question/` (entire directory) + - `dialog-question.tsx` + - `dialog-question-select.tsx` + - `dialog-question-multi-select.tsx` + - `dialog-question-confirm.tsx` + - `dialog-question-text.tsx` + - `types.ts` + - `index.ts` + +### Phase 4: Update/Remove Tests + +- [x] **Task 4.1:** Decide and execute test strategy + - Option A (executed): Delete `packages/opencode/test/tool/ask.test.ts` and explicitly accept reduced coverage + - Option B (preferred): Replace with a minimal `askquestion` schema test to keep some coverage + +### Phase 5: Verification + +- [x] **Task 5.1:** Build verification + + ```bash + cd packages/opencode && bun run build + ``` + + - Ensure no TypeScript errors + - Ensure no missing imports + +- [x] **Task 5.2:** Test verification + + ```bash + cd packages/opencode && bun test + ``` + + - Ensure all remaining tests pass (340 pass, 1 skip, 0 fail) + +- [ ] **Task 5.3:** Manual TUI testing + - Start the TUI: `bun dev` + - Verify `askquestion` tool still works correctly + - Verify no errors related to missing `ask` tool + - Verify `/tools` (or any tool listing) no longer includes `ask` + +--- + +## Files to Modify + +| File | Action | Changes | +| ----------------------------------------------- | ------ | ------------------------------------------------------------------- | +| `packages/opencode/src/tool/registry.ts` | Modify | Remove AskTool import and registration | +| `packages/opencode/src/cli/cmd/tui/app.tsx` | Modify | Remove QuestionRequest handler, DialogQuestion, and Question import | +| `packages/opencode/src/cli/cmd/tui/event.ts` | Modify | Remove Question import and event definitions | +| `packages/opencode/src/session/prompt/plan.txt` | Modify | Replace `ask` guidance with `askquestion` usage | + +## Files to Delete + +| File/Directory | Reason | +| -------------------------------------------------------------- | ------------------------------------------- | +| `packages/opencode/src/tool/ask.ts` | Deprecated tool implementation | +| `packages/opencode/src/question/index.ts` | Only used by ask tool | +| `packages/opencode/src/cli/cmd/tui/component/dialog-question/` | UI for deprecated ask tool | +| `packages/opencode/test/tool/ask.test.ts` | Tests for deprecated tool (unless replaced) | + +## Files to Keep (No Changes) + +| File | Reason | +| ------------------------------------------------------------- | -------------------------- | +| `packages/opencode/src/tool/askquestion.ts` | Working implementation | +| `packages/opencode/src/tool/askquestion.txt` | Tool description | +| `packages/opencode/src/askquestion/index.ts` | AskQuestion namespace | +| `packages/opencode/src/server/server.ts` | HTTP endpoints (unchanged) | +| `packages/opencode/src/cli/cmd/tui/routes/session/index.tsx` | Sync detection (unchanged) | +| `packages/opencode/src/cli/cmd/tui/ui/dialog-askquestion.tsx` | Wizard UI (unchanged) | + +--- + +## Validation Criteria + +### Implementation Complete When: + +- [x] `AskTool` is not registered in `registry.ts` +- [x] No imports of `@/question` or `../question` exist in the codebase +- [x] No `TuiEvent.QuestionRequest` or `TuiEvent.QuestionResponse` references exist +- [x] `packages/opencode/src/tool/ask.ts` is deleted +- [x] `packages/opencode/src/question/` directory is deleted +- [x] `packages/opencode/src/cli/cmd/tui/component/dialog-question/` directory is deleted +- [x] `ask` references in docs/prompts/configs are migrated to `askquestion` +- [x] Any migrated call sites use the `askquestion` schema (label/question/options, 1-6 questions, 2-8 options) +- [x] Build passes with no TypeScript errors +- [x] All tests pass +- [ ] `askquestion` tool works correctly in TUI (pending manual verification) +- [ ] Tool listings no longer show `ask` (pending manual verification) + +--- + +## Risk Assessment + +| Risk | Likelihood | Impact | Mitigation | +| --------------------------------------------- | ---------- | ------ | -------------------------------------------------------------------------------------------- | +| Breaking upstream sync | Low | Medium | Only removing fork-specific code; askquestion is upstream | +| LLM still tries to use `ask` tool | Low | Low | Remove internal references; external callers will need migration note | +| Schema mismatch for existing `ask` call sites | Medium | Medium | Audit/migrate call sites to askquestion schema; consider explicit adapter or migration notes | +| Missing functionality | Low | Low | askquestion covers main use cases; confirm/text can be added later if needed | + +--- + +## Appendix: Files Using `@/question` Import + +These files import from `@/question` and need the import removed: + +1. `packages/opencode/src/tool/ask.ts` - TO BE DELETED +2. `packages/opencode/src/cli/cmd/tui/event.ts` - MODIFY +3. `packages/opencode/src/cli/cmd/tui/app.tsx` - MODIFY +4. `packages/opencode/src/cli/cmd/tui/component/dialog-question/types.ts` - TO BE DELETED +5. `packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-multi-select.tsx` - TO BE DELETED +6. `packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question.tsx` - TO BE DELETED +7. `packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-confirm.tsx` - TO BE DELETED +8. `packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-text.tsx` - TO BE DELETED +9. `packages/opencode/src/cli/cmd/tui/component/dialog-question/dialog-question-select.tsx` - TO BE DELETED + +--- + +## Appendix: Git History + +``` +a1c9fec1c feat: merge upstream PR #5958 askquestion tool (replaces #5563) +19ad1ee52 fix(askquestion): fix memory leaks, race conditions, and TUI UX bugs +3a23fec31 feat(askquestion): fix race condition and improve TUI UX +7ff537213 fix: ask tool schema validation and bash carriage return handling +36a1f210e feat: add Ask tool for collecting user input via dialogs +``` + +The commit message "replaces #5563" explicitly indicates `askquestion` was intended to supersede the original `ask` tool implementation.