diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index 2b8fc76184..3ff7615d00 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -1597,6 +1597,93 @@ describe('createGatewayEventHandler', () => { expect(getOverlayState().sudo).toBeNull() }) + // ── Batch (multi-question) clarify ───────────────────────────────── + + it('parses a batch clarify.request into a questions overlay', () => { + const onEvent = createGatewayEventHandler(buildCtx([])) + + onEvent({ + payload: { + questions: [ + { choices: ['a', 'b'], qid: 'q0', question: 'One?' }, + { choices: null, qid: 'q1', question: 'Two?' } + ], + request_id: 'req-batch' + }, + type: 'clarify.request' + } as any) + + const clarify = getOverlayState().clarify + expect(clarify?.requestId).toBe('req-batch') + expect(clarify?.questions).toHaveLength(2) + expect(clarify?.questions?.[0]?.qid).toBe('q0') + expect(clarify?.questions?.[1]?.choices).toBeNull() + expect(clarify?.answers).toEqual({}) + }) + + it('seeds locked answers from a reconnect-replay batch clarify.request', () => { + const onEvent = createGatewayEventHandler(buildCtx([])) + + onEvent({ + payload: { + answers: { q0: 'a' }, + questions: [ + { choices: ['a', 'b'], qid: 'q0', question: 'One?' }, + { choices: null, qid: 'q1', question: 'Two?' } + ], + request_id: 'req-replay' + }, + type: 'clarify.request' + } as any) + + expect(getOverlayState().clarify?.answers).toEqual({ q0: 'a' }) + }) + + it('drops malformed batch entries and falls back to single-question shape when none survive', () => { + const onEvent = createGatewayEventHandler(buildCtx([])) + + onEvent({ + payload: { + choices: ['x', 'y'], + question: 'Fallback?', + questions: [{ qid: '', question: 'no qid' }, { qid: 'q1', question: ' ' }], + request_id: 'req-bad' + }, + type: 'clarify.request' + } as any) + + const clarify = getOverlayState().clarify + expect(clarify?.questions).toBeUndefined() + expect(clarify?.question).toBe('Fallback?') + expect(clarify?.choices).toEqual(['x', 'y']) + }) + + it('persists an abandoned batch clarify with its locked partials on tool.complete', () => { + const appended: Msg[] = [] + const onEvent = createGatewayEventHandler(buildCtx(appended)) + + patchOverlayState({ + clarify: { + answers: { q0: 'alpha' }, + choices: null, + question: '', + questions: [ + { choices: ['alpha', 'beta'], qid: 'q0', question: 'One?' }, + { choices: null, qid: 'q1', question: 'Two?' } + ], + requestId: 'req-batch-timeout' + } + }) + + onEvent({ payload: { name: 'clarify', tool_id: 'clar-b' }, type: 'tool.complete' } as any) + + const record = appended.find(msg => msg.role === 'system' && msg.text.startsWith('ask (2 questions)')) + expect(record).toBeDefined() + expect(record?.text).toContain('✓ One? → alpha') + expect(record?.text).toContain('· Two? (no answer)') + expect(getOverlayState().clarify).toBeNull() + }) + // ── Credits notice (Strategy B) ────────────────────────────────────── describe('credits notice', () => { it('shows a notice immediately when idle (no turn in flight)', () => { diff --git a/ui-tui/src/app/createGatewayEventHandler.ts b/ui-tui/src/app/createGatewayEventHandler.ts index 798c6e4726..b5fef4b633 100644 --- a/ui-tui/src/app/createGatewayEventHandler.ts +++ b/ui-tui/src/app/createGatewayEventHandler.ts @@ -20,7 +20,7 @@ import { openExternalUrl } from '../lib/openExternalUrl.js' import { rpcErrorMessage } from '../lib/rpc.js' import { topLevelSubagents } from '../lib/subagentTree.js' import { isPaintableHex, setTerminalBackground, setTerminalForeground } from '../lib/terminalModes.js' -import { formatAbandonedClarify, formatToolCall, stripAnsi } from '../lib/text.js' +import { formatAbandonedClarify, formatAbandonedClarifyBatch, formatToolCall, stripAnsi } from '../lib/text.js' import { bootSeededPin, invalidateBootBackground, writeBootTheme } from '../lib/themeBoot.js' import { defaultThemeForCurrentBackground, fromSkin, skinIsLight, type Theme, themeToneHex } from '../theme.js' import type { Msg, SubagentProgress, SubagentStatus, Usage } from '../types.js' @@ -454,7 +454,9 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: persistedAbandonedClarify.add(clarify.requestId) appendMessage({ role: 'system', - text: formatAbandonedClarify(clarify.question, clarify.choices, 'timed out') + text: clarify.questions?.length + ? formatAbandonedClarifyBatch(clarify.questions, clarify.answers ?? {}, 'timed out') + : formatAbandonedClarify(clarify.question, clarify.choices, 'timed out') }) patchOverlayState({ clarify: null }) } @@ -1213,13 +1215,35 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: return } - case 'clarify.request': + case 'clarify.request': { + const batch = (ev.payload.questions ?? []) + .filter(q => typeof q?.qid === 'string' && q.qid && typeof q?.question === 'string' && q.question.trim()) + .map(q => ({ + choices: q.choices && q.choices.length > 0 ? q.choices : null, + multiSelect: q.multi_select === true, + qid: q.qid, + question: q.question.trim() + })) + patchOverlayState({ - clarify: { choices: ev.payload.choices, question: ev.payload.question, requestId: ev.payload.request_id } + clarify: batch.length + ? { + answers: ev.payload.answers ?? {}, + choices: null, + question: '', + questions: batch, + requestId: ev.payload.request_id + } + : { + choices: ev.payload.choices ?? null, + question: ev.payload.question ?? '', + requestId: ev.payload.request_id + } }) setStatus('waiting for input…') return + } case 'approval.request': { const description = String(ev.payload.description ?? 'dangerous command') // Only an explicit false (tirith warning) drops the permanent-allow option. diff --git a/ui-tui/src/app/interfaces.ts b/ui-tui/src/app/interfaces.ts index dffa3aeb50..1a4cfbc565 100644 --- a/ui-tui/src/app/interfaces.ts +++ b/ui-tui/src/app/interfaces.ts @@ -552,6 +552,7 @@ export interface SlashHandlerContext { export interface AppLayoutActions { answerApproval: (choice: string) => void answerClarify: (answer: string) => void + answerClarifyQuestion: (qid: string, answer: string) => void answerSecret: (value: string) => void answerSudo: (pw: string) => void clearSelection: () => void @@ -619,6 +620,7 @@ export interface AppOverlaysProps { completions: CompletionItem[] onApprovalChoice: (choice: string) => void onClarifyAnswer: (value: string) => void + onClarifyQuestionAnswer: (qid: string, value: string) => void onActiveSessionSelect: (sessionId: string) => void onActiveSessionClose: (sessionId: string) => Promise onModelSelect: (value: string) => void diff --git a/ui-tui/src/app/useMainApp.ts b/ui-tui/src/app/useMainApp.ts index 555d66f073..cb931e55c1 100644 --- a/ui-tui/src/app/useMainApp.ts +++ b/ui-tui/src/app/useMainApp.ts @@ -35,7 +35,7 @@ import { DEFAULT_VOICE_RECORD_KEY, isMac, type ParsedVoiceRecordKey } from '../l import { createResizeCoalescer } from '../lib/resizeCoalescer.js' import { asRpcResult, rpcErrorMessage } from '../lib/rpc.js' import { terminalParityHints } from '../lib/terminalParity.js' -import { buildToolTrailLine, formatAbandonedClarify, sameToolTrailGroup, toolTrailLabel } from '../lib/text.js' +import { buildToolTrailLine, formatAbandonedClarify, formatAbandonedClarifyBatch, sameToolTrailGroup, toolTrailLabel } from '../lib/text.js' import { estimatedMsgHeight, messageHeightKey } from '../lib/virtualHeights.js' import { onUserWidgets } from '../sdk/userWidgets.js' import type { Msg, PanelSection, SlashCatalog } from '../types.js' @@ -705,7 +705,9 @@ export function useMainApp(gw: GatewayClient) { // survives on screen as standard output, matching the timeout path. appendMessage({ role: 'system', - text: formatAbandonedClarify(clarify.question, clarify.choices, 'cancelled') + text: clarify.questions?.length + ? formatAbandonedClarifyBatch(clarify.questions, clarify.answers ?? {}, 'cancelled') + : formatAbandonedClarify(clarify.question, clarify.choices, 'cancelled') }) } @@ -715,6 +717,60 @@ export function useMainApp(gw: GatewayClient) { [appendMessage, overlay.clarify, rpc] ) + // Lock one answer of a batch clarify (clarify.respond + question_id). The + // overlay stays up until the server reports no remaining questions — the + // final lock resolves the tool and the turn continues. + const answerClarifyQuestion = useCallback( + (qid: string, answer: string) => { + const clarify = overlay.clarify + + if (!clarify?.questions?.length) { + return + } + + rpc('clarify.respond', { + answer, + question_id: qid, + request_id: clarify.requestId + }).then(r => { + if (!r) { + return + } + + const answers = { ...(clarify.answers ?? {}), [qid]: answer } + + if ((r.remaining ?? []).length > 0) { + patchOverlayState({ clarify: { ...clarify, answers } }) + + return + } + + // Batch complete: persist the whole Q&A set as one user-visible + // block (mirrors the single-question trail + answer lines). + const label = toolTrailLabel('clarify') + + turnController.turnTools = turnController.turnTools.filter(line => !sameToolTrailGroup(label, line)) + patchTurnState({ turnTrail: turnController.turnTools }) + turnController.persistedToolLabels.add(label) + appendMessage({ + kind: 'trail', + role: 'system', + text: '', + tools: [buildToolTrailLine('clarify', `${clarify.questions!.length} questions`)] + }) + appendMessage({ + role: 'user', + text: clarify + .questions!.map(q => `${q.question} → ${answers[q.qid]?.trim() ? answers[q.qid] : '(skipped)'}`) + .join('\n') + }) + patchUiState({ status: 'running…' }) + patchOverlayState({ clarify: null }) + }) + }, + [appendMessage, overlay.clarify, rpc] + ) + sysRef.current = sys const { dispatchSubmission, send, sendQueued, submit } = useSubmission({ @@ -1091,6 +1147,7 @@ export function useMainApp(gw: GatewayClient) { closeLiveSession, answerApproval, answerClarify, + answerClarifyQuestion, answerSecret, answerSudo, clearSelection, @@ -1113,6 +1170,7 @@ export function useMainApp(gw: GatewayClient) { [ answerApproval, answerClarify, + answerClarifyQuestion, answerSecret, answerSudo, clearSelection, diff --git a/ui-tui/src/components/appLayout.tsx b/ui-tui/src/components/appLayout.tsx index 3b5ef135c0..f358097699 100644 --- a/ui-tui/src/components/appLayout.tsx +++ b/ui-tui/src/components/appLayout.tsx @@ -561,6 +561,7 @@ export const AppLayout = memo(function AppLayout({ cols={composer.cols} onApprovalChoice={actions.answerApproval} onClarifyAnswer={actions.answerClarify} + onClarifyQuestionAnswer={actions.answerClarifyQuestion} onSecretSubmit={actions.answerSecret} onSudoSubmit={actions.answerSudo} /> diff --git a/ui-tui/src/components/appOverlays.tsx b/ui-tui/src/components/appOverlays.tsx index aa9fcd6547..f96be36ba4 100644 --- a/ui-tui/src/components/appOverlays.tsx +++ b/ui-tui/src/components/appOverlays.tsx @@ -59,9 +59,13 @@ export function PromptZone({ cols, onApprovalChoice, onClarifyAnswer, + onClarifyQuestionAnswer, onSecretSubmit, onSudoSubmit -}: Pick) { +}: Pick< + AppOverlaysProps, + 'cols' | 'onApprovalChoice' | 'onClarifyAnswer' | 'onClarifyQuestionAnswer' | 'onSecretSubmit' | 'onSudoSubmit' +>) { const overlay = useStore($overlayState) const theme = useStore($uiTheme) @@ -129,6 +133,7 @@ export function PromptZone({ cols={cols} onAnswer={onClarifyAnswer} onCancel={() => onClarifyAnswer('')} + onQuestionAnswer={onClarifyQuestionAnswer} req={overlay.clarify} t={theme} /> diff --git a/ui-tui/src/components/prompts.tsx b/ui-tui/src/components/prompts.tsx index 78fade0d2c..d82293260e 100644 --- a/ui-tui/src/components/prompts.tsx +++ b/ui-tui/src/components/prompts.tsx @@ -1,5 +1,5 @@ import { Box, Text, useInput, wrapAnsi } from '@hermes/ink' -import { useState } from 'react' +import { useEffect, useState } from 'react' import { isMac } from '../lib/platform.js' import type { Theme } from '../theme.js' @@ -142,27 +142,160 @@ export function ApprovalPrompt({ cols = 80, onChoice, req, t }: ApprovalPromptPr ) } -export function ClarifyPrompt({ cols = 80, onAnswer, onCancel, req, t }: ClarifyPromptProps) { +export function ClarifyPrompt({ cols = 80, onAnswer, onCancel, onQuestionAnswer, req, t }: ClarifyPromptProps) { const [sel, setSel] = useState(0) const [custom, setCustom] = useState('') const [typing, setTyping] = useState(false) const choices = req.choices ?? [] + const batch = req.questions ?? [] + const isBatch = batch.length > 0 + + // ── Batch (A-compact) state: status list + one expanded active question. + // `active` walks the QUESTION list (any order); `sel` is reused as the + // cursor within the active question's choice rows. + const answers = req.answers ?? {} + const firstUnanswered = batch.findIndex(q => answers[q.qid] === undefined) + const [active, setActive] = useState(Math.max(0, firstUnanswered)) + // Walking the status list vs. answering the expanded question. + const [browsing, setBrowsing] = useState(false) + + // After a lock the overlay is re-patched with the new answers map — jump + // the cursor to the next unanswered question (stay put when editing). + useEffect(() => { + if (!isBatch || browsing) { + return + } + + const current = batch[active] + + if (current && answers[current.qid] === undefined) { + return + } + + const next = batch.findIndex(q => answers[q.qid] === undefined) + + if (next >= 0) { + setActive(next) + setSel(0) + setCustom('') + setTyping(false) + } + // eslint-disable-next-line react-hooks/exhaustive-deps -- keyed by the answers map only + }, [req.answers]) const heading = ( ask - {req.question} + {isBatch ? `${batch.length} questions` : req.question} ) + const activeQuestion = isBatch ? batch[active] : undefined + const activeChoices = activeQuestion ? (activeQuestion.choices ?? []) : choices + const answeredCount = isBatch ? batch.filter(q => answers[q.qid] !== undefined).length : 0 + const remainingCount = isBatch ? batch.length - answeredCount : 0 + + const lockActive = (value: string) => { + if (activeQuestion) { + onQuestionAnswer?.(activeQuestion.qid, value) + setSel(0) + setCustom('') + setTyping(false) + } + } + useInput((ch, key) => { if (key.escape) { - typing && choices.length ? setTyping(false) : onCancel() + if (typing) { + setTyping(false) + + return + } + + if (isBatch && browsing) { + setBrowsing(false) + + return + } + + onCancel() return } - if (typing || !choices.length) { + if (typing) { + return + } + + if (isBatch) { + // Tab toggles between walking the question list and answering the + // expanded question — the "any order" affordance. + if (key.tab) { + setBrowsing(b => !b) + setSel(0) + + return + } + + if (browsing) { + if (key.upArrow && active > 0) { + setActive(a => a - 1) + setSel(0) + setCustom('') + } + + if (key.downArrow && active < batch.length - 1) { + setActive(a => a + 1) + setSel(0) + setCustom('') + } + + if (key.return) { + setBrowsing(false) + } + + return + } + + if (!activeQuestion) { + return + } + + if (activeChoices.length === 0) { + // Open-ended question: any keypress starts typing (TextInput below). + setTyping(true) + + return + } + + if (key.upArrow && sel > 0) { + setSel(s => s - 1) + } + + if (key.downArrow && sel < activeChoices.length) { + setSel(s => s + 1) + } + + if (key.return) { + if (sel === activeChoices.length) { + setTyping(true) + } else if (activeChoices[sel]) { + lockActive(activeChoices[sel]!) + } + + return + } + + const n = parseInt(ch) + + if (n >= 1 && n <= activeChoices.length) { + lockActive(activeChoices[n - 1]!) + } + + return + } + + if (!choices.length) { return } @@ -185,6 +318,71 @@ export function ClarifyPrompt({ cols = 80, onAnswer, onCancel, req, t }: Clarify } }) + if (isBatch) { + const hint = typing + ? `Enter ${remainingCount === 1 ? 'confirm and continue' : 'lock answer'} · Esc back` + : browsing + ? '↑/↓ pick a question · Enter/Tab answer it · Esc cancel all' + : `↑/↓ select · Enter ${remainingCount === 1 ? 'confirm and continue' : 'lock answer'} · Tab switch question · Esc/Ctrl+C cancel` + + return ( + + {heading} + + {batch.map((q, i) => { + const answer = answers[q.qid] + const isActive = i === active + const marker = answer !== undefined ? '✓' : isActive ? '▸' : '·' + const summary = answer !== undefined ? ` → ${answer || '(skipped)'}` : '' + + return ( + + + + {marker} {q.question} + {summary} + + + + {isActive && !browsing ? ( + typing || activeChoices.length === 0 ? ( + + {'> '} + + + ) : ( + + {[...activeChoices, 'Other (type your answer)'].map((c, ci) => ( + + + {sel === ci ? '▸ ' : ' '} + {ci + 1}. {c} + + + ))} + + ) + ) : null} + + ) + })} + + + {answeredCount}/{batch.length} answered · {hint} + + + ) + } + if (typing || !choices.length) { return ( @@ -300,6 +498,8 @@ interface ClarifyPromptProps { cols?: number onAnswer: (s: string) => void onCancel: () => void + /** Batch mode: lock one question's answer (clarify.respond + question_id). */ + onQuestionAnswer?: (qid: string, s: string) => void req: ClarifyReq t: Theme } diff --git a/ui-tui/src/gatewayTypes.ts b/ui-tui/src/gatewayTypes.ts index fcd634206f..427af8bade 100644 --- a/ui-tui/src/gatewayTypes.ts +++ b/ui-tui/src/gatewayTypes.ts @@ -703,7 +703,13 @@ export type GatewayEvent = type: 'tool.complete' } | { - payload: { choices: string[] | null; question: string; request_id: string } + payload: { + answers?: Record + choices?: string[] | null + question?: string + questions?: { choices?: string[] | null; multi_select?: boolean; qid: string; question: string }[] + request_id: string + } session_id?: string type: 'clarify.request' } diff --git a/ui-tui/src/lib/text.test.ts b/ui-tui/src/lib/text.test.ts index 7117e1f44a..bbbb2a382d 100644 --- a/ui-tui/src/lib/text.test.ts +++ b/ui-tui/src/lib/text.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' -import { formatAbandonedClarify, stripTrailingPasteNewlines } from './text.js' +import { formatAbandonedClarify, formatAbandonedClarifyBatch, stripTrailingPasteNewlines } from './text.js' describe('stripTrailingPasteNewlines', () => { it('removes trailing newline runs from pasted text', () => { @@ -51,3 +51,26 @@ describe('formatAbandonedClarify', () => { expect(out).not.toContain(' 0.') }) }) + +describe('formatAbandonedClarifyBatch', () => { + it('shows locked answers and marks unanswered questions', () => { + const out = formatAbandonedClarifyBatch( + [ + { qid: 'q0', question: 'One?' }, + { qid: 'q1', question: 'Two?' } + ], + { q0: 'alpha' }, + 'timed out' + ) + + expect(out).toBe( + ['ask (2 questions)', ' ✓ One? → alpha', ' · Two? (no answer)', ' (timed out)'].join('\n') + ) + }) + + it('treats an empty locked answer as unanswered in the record', () => { + const out = formatAbandonedClarifyBatch([{ qid: 'q0', question: 'One?' }], { q0: '' }, 'cancelled') + + expect(out).toContain('· One? (no answer)') + }) +}) diff --git a/ui-tui/src/lib/text.ts b/ui-tui/src/lib/text.ts index 64b5b31117..b850aa7b22 100644 --- a/ui-tui/src/lib/text.ts +++ b/ui-tui/src/lib/text.ts @@ -365,6 +365,25 @@ export const formatAbandonedClarify = (question: string, choices: string[] | nul return [head, ...opts, ` (${reason} — no selection)`].join('\n') } +/** + * Batch counterpart of `formatAbandonedClarify`: every question on its own + * line, answered ones keeping their locked answer (partials survive a + * timeout server-side, so the record must show what was actually sent). + */ +export const formatAbandonedClarifyBatch = ( + questions: { qid: string; question: string }[], + answers: Record, + reason: string +) => { + const lines = questions.map(q => { + const answer = answers[q.qid] + + return answer ? ` ✓ ${q.question} → ${answer}` : ` · ${q.question} (no answer)` + }) + + return [`ask (${questions.length} questions)`, ...lines, ` (${reason})`].join('\n') +} + export const flat = (r: Record) => Object.values(r).flat() const COMPACT_NUMBER = new Intl.NumberFormat('en-US', { maximumFractionDigits: 1, notation: 'compact' }) diff --git a/ui-tui/src/types.ts b/ui-tui/src/types.ts index f90d3b9c45..6e225363cb 100644 --- a/ui-tui/src/types.ts +++ b/ui-tui/src/types.ts @@ -107,10 +107,22 @@ export interface ConfirmReq { title: string } +export interface ClarifyBatchQuestion { + choices: string[] | null + multiSelect?: boolean + qid: string + question: string +} + export interface ClarifyReq { choices: string[] | null question: string requestId: string + /** Batch (multi-question) clarify: present instead of question/choices. */ + questions?: ClarifyBatchQuestion[] + /** Answers already locked server-side (qid → answer): seeded from the + * reconnect replay, updated as the user locks each question. */ + answers?: Record } export interface Msg {