From c56e670d64e6050d91f05a0219bb14b97005bce1 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 15 Jul 2026 00:20:24 -0700 Subject: [PATCH] fix(native-chat): deliver AskUserQuestion answer by option number, not label Claude Code's AskUserQuestion is an arrow-navigate selector: a bare Enter commits the HIGHLIGHTED default (the first option), and pasted label text does not move the highlight. Native chat delivered the answer as the option LABEL text + Enter, so every non-first pick was silently committed as the first option (STA-1860 Bug A). Deliver the answer by each option's stable 1-based number instead (the marker the selector commits on), paced so each keystroke lands after the selector step renders. Covers single/multi-select, multi-question, and free-text ('Type something') answers. Verified live against Claude Code 2.1.210. - native-chat-interactive-prompt.ts: buildAskAnswerKeys / AskAnswerSelection / AskAnswerKeyGroup / hasAskAnswer; formatAskAnswer kept (index-based) for the non-Claude label path - native-chat-runtime-send.ts: sendNativeChatAskAnswer (paced keystroke groups) replaces the old label-line sender - card + interactive hook thread structured { indices, other } selections STA-1860 --- .../native-chat/NativeChatInteractiveCard.tsx | 4 +- .../NativeChatQuestionCard.test.tsx | 115 ++++++++++++++ .../native-chat/NativeChatQuestionCard.tsx | 27 ++-- .../native-chat-interactive-prompt.test.ts | 101 +++++++++++- .../native-chat-interactive-prompt.ts | 115 +++++++++++++- .../native-chat-runtime-send.test.ts | 144 +++++++----------- .../native-chat/native-chat-runtime-send.ts | 74 ++++----- .../use-native-chat-interactive-send.test.tsx | 56 ++++++- .../use-native-chat-interactive-send.ts | 48 +++--- 9 files changed, 499 insertions(+), 185 deletions(-) create mode 100644 src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx diff --git a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx index 947d5f65c3e..751ce98ce20 100644 --- a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx +++ b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx @@ -94,12 +94,12 @@ export function NativeChatInteractiveCard({ key={cardKey ?? 'question'} prompt={card.prompt} answerInputRef={answerInputRef} - onAnswer={(text) => { + onAnswer={(selections) => { if (submittingRef.current) { return } submittingRef.current = true - const settleMs = sendAnswer(text) + const settleMs = sendAnswer(card.prompt, selections) // Hold the card until the paced write finishes, then mark it answered // (which hides it and restores the composer). dismissTimerRef.current = setTimeout(() => { diff --git a/src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx b/src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx new file mode 100644 index 00000000000..5a9b9c90271 --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx @@ -0,0 +1,115 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { NativeChatQuestionCard } from './NativeChatQuestionCard' +import type { AskAnswerSelection, AskPrompt } from './native-chat-interactive-prompt' + +// The card resolves its own label-keyed selection state into the index-based +// answer the delivery layer needs. These tests pin that resolution — the exact +// seam of STA-1860 (a non-first pick must surface as its option INDEX, not the +// first option / the raw label). + +let container: HTMLDivElement +let root: ReturnType + +beforeEach(() => { + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(() => { + act(() => root.unmount()) + container.remove() +}) + +function render(prompt: AskPrompt, onAnswer: (s: AskAnswerSelection[]) => void): void { + act(() => { + root.render( {}} />) + }) +} + +function click(button: Element | undefined, describe: string): void { + if (!button) { + throw new Error(`button not found: ${describe}`) + } + act(() => button.dispatchEvent(new MouseEvent('click', { bubbles: true }))) +} + +// Option rows carry a badge number + label, so match them by the label they +// contain among the aria-pressed selectable rows. +function clickOption(label: string): void { + const row = [...container.querySelectorAll('button[aria-pressed]')].find((b) => + b.textContent?.includes(label) + ) + click(row, `option ${label}`) +} + +function clickAction(text: string): void { + const button = [...container.querySelectorAll('button')].find( + (b) => b.textContent?.trim() === text + ) + click(button, text) +} + +const tabsOrSpaces: AskPrompt = { + questions: [ + { + question: 'Do you prefer tabs or spaces?', + header: 'Indent', + multiSelect: false, + options: [{ label: 'Tabs' }, { label: 'Spaces' }] + } + ] +} + +describe('NativeChatQuestionCard', () => { + it('delivers the SECOND option as index 1, not the default (STA-1860)', () => { + const onAnswer = vi.fn() + render(tabsOrSpaces, onAnswer) + + clickOption('Spaces') + clickAction('Send answer') + + expect(onAnswer).toHaveBeenCalledWith([{ indices: [1], other: '' }]) + }) + + it('delivers a multi-select pick as its option indices', () => { + const onAnswer = vi.fn() + render( + { + questions: [ + { + question: 'Which fruits?', + multiSelect: true, + options: [{ label: 'Apple' }, { label: 'Banana' }, { label: 'Cherry' }] + } + ] + }, + onAnswer + ) + + clickOption('Apple') + clickOption('Cherry') + clickAction('Send answer') + + expect(onAnswer).toHaveBeenCalledWith([{ indices: [0, 2], other: '' }]) + }) + + it('carries free text through as the other answer', () => { + const onAnswer = vi.fn() + render(tabsOrSpaces, onAnswer) + + const input = container.querySelector('input')! + act(() => { + const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set! + setter.call(input, 'four spaces') + input.dispatchEvent(new Event('input', { bubbles: true })) + }) + clickAction('Send answer') + + expect(onAnswer).toHaveBeenCalledWith([{ indices: [], other: 'four spaces' }]) + }) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx b/src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx index 503f3111034..6e81d70f39a 100644 --- a/src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx +++ b/src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx @@ -2,12 +2,12 @@ import { useState, type RefObject } from 'react' import { Check, Pencil, X } from 'lucide-react' import { cn } from '@/lib/utils' import { translate } from '@/i18n/i18n' -import { formatAskAnswer, type AskPrompt } from './native-chat-interactive-prompt' +import type { AskAnswerSelection, AskPrompt } from './native-chat-interactive-prompt' export type NativeChatQuestionCardProps = { prompt: AskPrompt - /** Send the formatted answer text to the agent. */ - onAnswer: (text: string) => void + /** Deliver the chosen answer (per-question option indices + free text). */ + onAnswer: (selections: AskAnswerSelection[]) => void /** Dismiss the prompt (sends Escape to the agent). */ onCancel: () => void /** Exposes the free-text row so pane-level Paste can target it while the @@ -53,15 +53,24 @@ export function NativeChatQuestionCard({ const currentAnswered = answerFor(index).length > 0 + // Resolve the card's label-keyed selection state into per-question option + // INDICES (in option order) + trimmed free text, the index-based shape the + // answer is delivered by (the agent's selector commits by option number, not + // by label — STA-1860). const submitAll = (sel: string[][], oth: string[]): void => { - const resolved = prompt.questions.map((_, i) => { + const selections: AskAnswerSelection[] = prompt.questions.map((question, i) => { const picked = sel[i] ?? [] - const other = (oth[i] ?? '').trim() - return [...picked, ...(other ? [other] : [])] + const indices = question.options.reduce((acc, opt, oi) => { + if (picked.includes(opt.label)) { + acc.push(oi) + } + return acc + }, []) + return { indices, other: (oth[i] ?? '').trim() } }) - const text = formatAskAnswer(prompt, resolved) - if (text.length > 0) { - onAnswer(text) + const anyAnswered = selections.some((s) => s.indices.length > 0 || (s.other ?? '').length > 0) + if (anyAnswered) { + onAnswer(selections) } } diff --git a/src/renderer/src/components/native-chat/native-chat-interactive-prompt.test.ts b/src/renderer/src/components/native-chat/native-chat-interactive-prompt.test.ts index c97059bf64b..8afec9c0fac 100644 --- a/src/renderer/src/components/native-chat/native-chat-interactive-prompt.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-interactive-prompt.test.ts @@ -1,9 +1,12 @@ import { describe, expect, it } from 'vitest' import { + buildAskAnswerKeys, formatAskAnswer, + hasAskAnswer, parseApprovalFromStatus, parseAskFromStatus, - parseInteractivePrompt + parseInteractivePrompt, + type AskPrompt } from './native-chat-interactive-prompt' const ESC = String.fromCharCode(27) @@ -129,37 +132,119 @@ describe('parseInteractivePrompt', () => { }) describe('formatAskAnswer', () => { - it('joins selected labels per question, one line each', () => { - const prompt = { + it('joins selected option labels per question, one line each', () => { + const prompt: AskPrompt = { questions: [ { question: 'q1', multiSelect: true, options: [{ label: 'A' }, { label: 'B' }] }, { question: 'q2', multiSelect: false, options: [{ label: 'C' }] } ] } - expect(formatAskAnswer(prompt, [['A', 'B'], ['C']])).toBe('A, B\nC') + expect(formatAskAnswer(prompt, [{ indices: [0, 1] }, { indices: [0] }])).toBe('A, B\nC') + }) + + it('appends free-text after picked labels', () => { + const prompt: AskPrompt = { + questions: [{ question: 'q1', multiSelect: true, options: [{ label: 'A' }, { label: 'B' }] }] + } + expect(formatAskAnswer(prompt, [{ indices: [0], other: ' extra ' }])).toBe('A, extra') }) it('preserves empty answers as empty lines so N lines == N questions', () => { - const prompt = { + const prompt: AskPrompt = { questions: [ { question: 'q1', multiSelect: false, options: [{ label: 'A' }] }, { question: 'q2', multiSelect: false, options: [{ label: 'B' }] } ] } // Leading blank stays an empty line: '\nB' (2 lines), not 'B'. - expect(formatAskAnswer(prompt, [[], ['B']])).toBe('\nB') + expect(formatAskAnswer(prompt, [{ indices: [] }, { indices: [0] }])).toBe('\nB') }) it('keeps one line per question with a blank middle answer (3 questions)', () => { - const prompt = { + const prompt: AskPrompt = { questions: [ { question: 'q1', multiSelect: false, options: [{ label: 'A' }] }, { question: 'q2', multiSelect: false, options: [{ label: 'B' }] }, { question: 'q3', multiSelect: false, options: [{ label: 'C' }] } ] } - const answer = formatAskAnswer(prompt, [['A'], [], ['C']]) + const answer = formatAskAnswer(prompt, [{ indices: [0] }, { indices: [] }, { indices: [0] }]) expect(answer).toBe('A\n\nC') expect(answer.split('\n')).toHaveLength(3) }) }) + +const single = (options: string[], multiSelect = false): AskPrompt => ({ + questions: [{ question: 'q', multiSelect, options: options.map((label) => ({ label })) }] +}) + +describe('buildAskAnswerKeys', () => { + it('single-select: sends the picked option NUMBER (not the label), no trailing Enter', () => { + // STA-1860 regression: picking the 2nd option must deliver the 2nd option. + // '2' is the selector marker Claude commits on; a label + Enter committed the + // highlighted default (option 1) instead. + expect(buildAskAnswerKeys(single(['Tabs', 'Spaces']), [{ indices: [1] }])).toEqual([ + { raw: '2' } + ]) + expect(buildAskAnswerKeys(single(['Apple', 'Banana', 'Cherry']), [{ indices: [2] }])).toEqual([ + { raw: '3' } + ]) + }) + + it('single-select free text: opens "Type something" then types the answer + Enter', () => { + expect( + buildAskAnswerKeys(single(['Tabs', 'Spaces']), [{ indices: [], other: 'Zebra' }]) + ).toEqual([{ raw: '3' }, { text: 'Zebra' }, { raw: '\r' }]) + }) + + it('multi-select: toggles each option NUMBER, then steps to Submit and confirms', () => { + expect( + buildAskAnswerKeys(single(['Apple', 'Banana', 'Cherry'], true), [{ indices: [0, 2] }]) + ).toEqual([{ raw: '1' }, { raw: '3' }, { raw: '\x1b[C' }, { raw: '\r' }]) + }) + + it('multi-question single-select: option numbers auto-advance, one final submit Enter', () => { + const prompt: AskPrompt = { + questions: [ + { question: 'q1', multiSelect: false, options: [{ label: 'Tabs' }, { label: 'Spaces' }] }, + { + question: 'q2', + multiSelect: false, + options: [{ label: 'Apple' }, { label: 'Banana' }, { label: 'Cherry' }] + } + ] + } + expect(buildAskAnswerKeys(prompt, [{ indices: [1] }, { indices: [2] }])).toEqual([ + { raw: '2' }, + { raw: '3' }, + { raw: '\r' } + ]) + }) + + it('multi-question: steps past an unanswered question then submits', () => { + const prompt: AskPrompt = { + questions: [ + { question: 'q1', multiSelect: false, options: [{ label: 'Tabs' }, { label: 'Spaces' }] }, + { question: 'q2', multiSelect: false, options: [{ label: 'Apple' }, { label: 'Banana' }] } + ] + } + expect(buildAskAnswerKeys(prompt, [{ indices: [] }, { indices: [1] }])).toEqual([ + { raw: '\x1b[C' }, + { raw: '2' }, + { raw: '\r' } + ]) + }) + + it('is empty when nothing is answered', () => { + expect(buildAskAnswerKeys(single(['Tabs', 'Spaces']), [{ indices: [] }])).toEqual([]) + }) +}) + +describe('hasAskAnswer', () => { + it('is true for a picked option or typed text, false when empty', () => { + expect(hasAskAnswer(single(['A', 'B']), [{ indices: [1] }])).toBe(true) + expect(hasAskAnswer(single(['A', 'B']), [{ indices: [], other: 'x' }])).toBe(true) + expect(hasAskAnswer(single(['A', 'B']), [{ indices: [], other: ' ' }])).toBe(false) + expect(hasAskAnswer(single(['A', 'B']), [])).toBe(false) + }) +}) diff --git a/src/renderer/src/components/native-chat/native-chat-interactive-prompt.ts b/src/renderer/src/components/native-chat/native-chat-interactive-prompt.ts index 182c6b5d514..8a995a7580d 100644 --- a/src/renderer/src/components/native-chat/native-chat-interactive-prompt.ts +++ b/src/renderer/src/components/native-chat/native-chat-interactive-prompt.ts @@ -190,11 +190,112 @@ export function parseInteractivePrompt( return null } -/** Build the answer text to send: exactly one line per question, in question - * order, each line the selected option label(s) joined by ", ". Empty answers - * stay as empty lines (not dropped) so N lines always == N questions — the - * per-question Enter stepping counts one Enter per line, so dropping a blank - * middle answer would misalign the count and leave the prompt unsubmitted. */ -export function formatAskAnswer(prompt: AskPrompt, selections: string[][]): string { - return prompt.questions.map((_, i) => (selections[i] ?? []).join(', ')).join('\n') +/** One question's chosen answer, normalized for delivery: the selected option + * indices (in option order) plus any free-text "other" answer. Index-based (not + * label text) so the answer can be delivered by the selector's stable option + * number — see `buildAskAnswerKeys`. */ +export type AskAnswerSelection = { indices: number[]; other?: string } + +/** A single keystroke group to write to the agent PTY. `raw` bytes (option + * numbers, Enter, arrows) are written verbatim as keystrokes; `text` is a + * free-text answer the caller runs through its paste sanitizer before writing. */ +export type AskAnswerKeyGroup = { raw: string } | { text: string } + +/** True when this question is answered (a picked option or typed free text). */ +function isAnswered(sel: AskAnswerSelection | undefined): boolean { + return (sel?.indices.length ?? 0) > 0 || (sel?.other ?? '').trim().length > 0 +} + +/** The picked labels + trimmed free text for one question, in option order. */ +function answerLabels(question: AskQuestion, sel: AskAnswerSelection | undefined): string[] { + const labels = (sel?.indices ?? []) + .map((i) => question.options[i]?.label ?? '') + .filter((l) => l.length > 0) + const other = (sel?.other ?? '').trim() + return other ? [...labels, other] : labels +} + +/** Build the human-readable answer text: one line per question, in question + * order, each the selected label(s) + free text joined by ", ". Empty answers + * stay empty lines so N lines always == N questions. Used for agents whose + * question tool commits a pasted answer (not Claude's arrow-navigate selector). */ +export function formatAskAnswer(prompt: AskPrompt, selections: AskAnswerSelection[]): string { + return prompt.questions.map((q, i) => answerLabels(q, selections[i]).join(', ')).join('\n') +} + +// Claude's AskUserQuestion is an arrow-navigate selector: a bare Enter commits +// the HIGHLIGHTED default (the first option), and pasted label text does not move +// the highlight — so answering by label silently delivered every non-first pick +// as the first option (STA-1860). Instead we drive the selector by each option's +// stable 1-based number (which matches the card's badge), the marker it commits +// on. Right-arrow steps to the next question / the Submit tab. Verified live +// against Claude Code's TUI; groups are written spaced apart (see the desktop +// sender) because a navigation keystroke batched with Enter commits before the +// selector has applied it. +const ASK_ENTER = '\r' +const ASK_NEXT_TAB = '\x1b[C' + +/** Build the ordered keystroke groups that answer a Claude Code AskUserQuestion. + * Each group is written a step apart so the selector applies it before the next. + * + * - single-select pick → the option number (selects AND commits; in a + * multi-question prompt it auto-advances to the next question) + * - free-text answer → the "Type something" row number, the text, then Enter + * - multi-select → each option number TOGGLES its checkbox, then a step + * to the Submit tab + * - a multi-question prompt (and a lone multi-select) finishes on a Submit + * confirmation, so it ends with one Enter + * + * (Option counts are ≤ the tool's cap of a few, so single-digit numbers always + * address every row.) */ +export function buildAskAnswerKeys( + prompt: AskPrompt, + selections: AskAnswerSelection[] +): AskAnswerKeyGroup[] { + const questions = prompt.questions + const multiQuestion = questions.length > 1 + const groups: AskAnswerKeyGroup[] = [] + + questions.forEach((q, qi) => { + const sel = selections[qi] + const other = (sel?.other ?? '').trim() + const typeSomething = String(q.options.length + 1) + + if (q.multiSelect) { + for (const i of sel?.indices ?? []) { + groups.push({ raw: String(i + 1) }) + } + if (other) { + groups.push({ raw: typeSomething }, { text: other }, { raw: ASK_ENTER }) + } + // A multi-select never auto-advances; step to the next tab (the Submit tab + // when this is the last question). + groups.push({ raw: ASK_NEXT_TAB }) + } else if (other) { + // Single-select can only carry one value, so route any answer that + // includes free text through the "Type something" row as one string. + groups.push( + { raw: typeSomething }, + { text: answerLabels(q, sel).join(', ') }, + { raw: ASK_ENTER } + ) + } else if ((sel?.indices.length ?? 0) > 0) { + groups.push({ raw: String(sel!.indices[0]! + 1) }) + } else if (multiQuestion) { + // Unanswered question in a multi-question prompt: step past it. + groups.push({ raw: ASK_NEXT_TAB }) + } + }) + + const endsOnSubmitTab = + multiQuestion || (questions.length === 1 && questions[0]!.multiSelect === true) + if (endsOnSubmitTab && groups.length > 0) { + groups.push({ raw: ASK_ENTER }) + } + return groups +} + +/** Whether any question in `selections` carries an answer worth submitting. */ +export function hasAskAnswer(prompt: AskPrompt, selections: AskAnswerSelection[]): boolean { + return prompt.questions.some((_, i) => isAnswered(selections[i])) } diff --git a/src/renderer/src/components/native-chat/native-chat-runtime-send.test.ts b/src/renderer/src/components/native-chat/native-chat-runtime-send.test.ts index c2fd1b87ed6..626e126ffa5 100644 --- a/src/renderer/src/components/native-chat/native-chat-runtime-send.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-runtime-send.test.ts @@ -11,8 +11,7 @@ import { sendNativeChatMessage, sendNativeChatMessageWithImageAttachments, submitNativeChatPrompt, - sendNativeChatAnswer, - nativeChatQuestionOffsets, + sendNativeChatAskAnswer, NATIVE_CHAT_IMAGE_ATTACHMENT_SETTLE_MS, NATIVE_CHAT_SUBMIT_DELAY_MS, NATIVE_CHAT_QUESTION_STEP_MS, @@ -153,17 +152,7 @@ describe('empty prompt submit', () => { }) }) -describe('nativeChatQuestionOffsets', () => { - it('paces each question a full step apart, Enter 500ms after its body', () => { - expect(NATIVE_CHAT_QUESTION_STEP_MS).toBe(1000) - expect(NATIVE_CHAT_ADVANCE_BUFFER_MS).toBe(500) - expect(nativeChatQuestionOffsets(0)).toEqual({ bodyAt: 0, enterAt: 500 }) - expect(nativeChatQuestionOffsets(1)).toEqual({ bodyAt: 1000, enterAt: 1500 }) - expect(nativeChatQuestionOffsets(2)).toEqual({ bodyAt: 2000, enterAt: 2500 }) - }) -}) - -describe('sendNativeChatAnswer', () => { +describe('sendNativeChatAskAnswer', () => { beforeEach(() => { vi.useFakeTimers() sendRuntimePtyInput.mockClear() @@ -172,87 +161,72 @@ describe('sendNativeChatAnswer', () => { vi.useRealTimers() }) - it('single-line answer behaves exactly like sendNativeChatMessage', () => { - sendNativeChatAnswer(SETTINGS, PTY, ['only one']) - expect(sendRuntimePtyInput).toHaveBeenCalledTimes(1) - expect(sendRuntimePtyInput).toHaveBeenCalledWith( - SETTINGS, - PTY, - buildNativeChatPasteBytes('only one') - ) - vi.advanceTimersByTime(NATIVE_CHAT_SUBMIT_DELAY_MS) - expect(sendRuntimePtyInput).toHaveBeenCalledTimes(2) - expect(sendRuntimePtyInput).toHaveBeenLastCalledWith(SETTINGS, PTY, NATIVE_CHAT_SUBMIT) + it('paces each keystroke a full step apart (the proven submit gap + advance buffer)', () => { + expect(NATIVE_CHAT_QUESTION_STEP_MS).toBe(1000) + expect(NATIVE_CHAT_ADVANCE_BUFFER_MS).toBe(500) }) - it('multi-line: 3 bodies + 3 Enters in order, each Enter 500ms after its body, next body only after prior Enter+buffer', () => { - const lines = ['answer one', 'answer two', 'answer three'] - const handle = sendNativeChatAnswer(SETTINGS, PTY, lines) - - expect(handle.settleAfterMs).toBe(nativeChatQuestionOffsets(lines.length - 1).enterAt) - - // Nothing fires synchronously: even question 0's body is scheduled (setTimeout 0). + it('no writes and 0 settle for an empty keystroke list', () => { + const handle = sendNativeChatAskAnswer(SETTINGS, PTY, []) + expect(handle.settleAfterMs).toBe(0) + vi.runAllTimers() expect(sendRuntimePtyInput).toHaveBeenCalledTimes(0) + }) - // t=0: question 0 body. + it('single option-number keystroke: fires at t=0, settles a submit gap later', () => { + // The STA-1860 fix: a lone single-select pick is delivered as the option + // NUMBER, with no trailing Enter (the number both selects and commits). + const handle = sendNativeChatAskAnswer(SETTINGS, PTY, [{ raw: '2' }]) + expect(handle.settleAfterMs).toBe(NATIVE_CHAT_SUBMIT_DELAY_MS) + + // Scheduled at t=0 (setTimeout 0), not written synchronously. + expect(sendRuntimePtyInput).toHaveBeenCalledTimes(0) vi.advanceTimersByTime(0) expect(sendRuntimePtyInput).toHaveBeenCalledTimes(1) - expect(sendRuntimePtyInput).toHaveBeenLastCalledWith( - SETTINGS, - PTY, - buildNativeChatPasteBytes('answer one') - ) - - // t=500: question 0 Enter (500ms after its body); question 1 body NOT yet. - vi.advanceTimersByTime(NATIVE_CHAT_SUBMIT_DELAY_MS) - expect(sendRuntimePtyInput).toHaveBeenCalledTimes(2) - expect(sendRuntimePtyInput).toHaveBeenLastCalledWith(SETTINGS, PTY, NATIVE_CHAT_SUBMIT) - - // Question 1 body must wait the advance buffer past question 0's Enter. - vi.advanceTimersByTime(NATIVE_CHAT_ADVANCE_BUFFER_MS - 1) - expect(sendRuntimePtyInput).toHaveBeenCalledTimes(2) - - // t=800: question 1 body. - vi.advanceTimersByTime(1) - expect(sendRuntimePtyInput).toHaveBeenCalledTimes(3) - expect(sendRuntimePtyInput).toHaveBeenLastCalledWith( - SETTINGS, - PTY, - buildNativeChatPasteBytes('answer two') - ) - - // t=1300: question 1 Enter. - vi.advanceTimersByTime(NATIVE_CHAT_SUBMIT_DELAY_MS) - expect(sendRuntimePtyInput).toHaveBeenCalledTimes(4) - expect(sendRuntimePtyInput).toHaveBeenLastCalledWith(SETTINGS, PTY, NATIVE_CHAT_SUBMIT) - - // t=1600: question 2 body. - vi.advanceTimersByTime(NATIVE_CHAT_ADVANCE_BUFFER_MS) - expect(sendRuntimePtyInput).toHaveBeenCalledTimes(5) - expect(sendRuntimePtyInput).toHaveBeenLastCalledWith( - SETTINGS, - PTY, - buildNativeChatPasteBytes('answer three') - ) - - // t=2100: question 2 Enter — the final submit. No trailing writes after. - vi.advanceTimersByTime(NATIVE_CHAT_SUBMIT_DELAY_MS) - expect(sendRuntimePtyInput).toHaveBeenCalledTimes(6) - expect(sendRuntimePtyInput).toHaveBeenLastCalledWith(SETTINGS, PTY, NATIVE_CHAT_SUBMIT) - - // Exactly 3 bodies + 3 Enters; running all timers adds nothing more. + expect(sendRuntimePtyInput).toHaveBeenLastCalledWith(SETTINGS, PTY, '2') vi.runAllTimers() - expect(sendRuntimePtyInput).toHaveBeenCalledTimes(6) + expect(sendRuntimePtyInput).toHaveBeenCalledTimes(1) + }) - // Verify body/Enter ordering across the whole sequence. + it('writes each group a step apart; text groups go through the paste framer', () => { + const groups = [{ raw: '3' }, { text: 'custom answer' }, { raw: '\r' }] + const handle = sendNativeChatAskAnswer(SETTINGS, PTY, groups) + expect(handle.settleAfterMs).toBe( + 2 * NATIVE_CHAT_QUESTION_STEP_MS + NATIVE_CHAT_SUBMIT_DELAY_MS + ) + + vi.advanceTimersByTime(0) + expect(sendRuntimePtyInput).toHaveBeenLastCalledWith(SETTINGS, PTY, '3') + + // The next group must wait a full step so the "Type something" row renders. + vi.advanceTimersByTime(NATIVE_CHAT_QUESTION_STEP_MS - 1) + expect(sendRuntimePtyInput).toHaveBeenCalledTimes(1) + vi.advanceTimersByTime(1) + expect(sendRuntimePtyInput).toHaveBeenLastCalledWith( + SETTINGS, + PTY, + buildNativeChatPasteBytes('custom answer') + ) + + vi.advanceTimersByTime(NATIVE_CHAT_QUESTION_STEP_MS) + expect(sendRuntimePtyInput).toHaveBeenLastCalledWith(SETTINGS, PTY, NATIVE_CHAT_SUBMIT) + + vi.runAllTimers() const calls = sendRuntimePtyInput.mock.calls.map((c) => c[2]) - expect(calls).toEqual([ - buildNativeChatPasteBytes('answer one'), - NATIVE_CHAT_SUBMIT, - buildNativeChatPasteBytes('answer two'), - NATIVE_CHAT_SUBMIT, - buildNativeChatPasteBytes('answer three'), - NATIVE_CHAT_SUBMIT + expect(calls).toEqual(['3', buildNativeChatPasteBytes('custom answer'), NATIVE_CHAT_SUBMIT]) + }) + + it('cancel clears every pending keystroke', () => { + const handle = sendNativeChatAskAnswer(SETTINGS, PTY, [ + { raw: '1' }, + { raw: '\x1b[C' }, + { raw: '\r' } ]) + vi.advanceTimersByTime(0) + expect(sendRuntimePtyInput).toHaveBeenCalledTimes(1) + handle.cancel() + vi.runAllTimers() + // Only the first keystroke landed; the rest were cancelled. + expect(sendRuntimePtyInput).toHaveBeenCalledTimes(1) }) }) diff --git a/src/renderer/src/components/native-chat/native-chat-runtime-send.ts b/src/renderer/src/components/native-chat/native-chat-runtime-send.ts index e1bb88625f8..fa07de93486 100644 --- a/src/renderer/src/components/native-chat/native-chat-runtime-send.ts +++ b/src/renderer/src/components/native-chat/native-chat-runtime-send.ts @@ -4,6 +4,7 @@ import { sendRuntimePtyInput } from '@/runtime/runtime-terminal-inspection' import type { getSettingsForAgentTabRuntimeOwner } from '@/lib/agent-paste-draft' +import type { AskAnswerKeyGroup } from './native-chat-interactive-prompt' import { buildNativeChatImagePasteBytes, buildNativeChatPasteBytes, @@ -21,36 +22,21 @@ import { export const NATIVE_CHAT_SUBMIT_DELAY_MS = 500 export const NATIVE_CHAT_IMAGE_ATTACHMENT_SETTLE_MS = 300 -// Why: Claude Code's AskUserQuestion is a MULTI-STEP prompt — it renders one -// question at a time and each Enter advances to the next (the final Enter -// submits the whole thing). After firing a question's Enter we must let the TUI -// render the next question before writing its body, or that body lands on the -// wrong (or no) active question. This buffer is added ON TOP of the body→Enter -// gap; a previous attempt that spaced Enters only ~350ms apart fired them faster -// than the next question rendered, leaking answers as fresh prompts. Kept -// generous so the next step still renders in time on slower machines / under SSH +// Why: answering Claude's AskUserQuestion is a MULTI-STEP keystroke sequence +// (select an option, advance to the next question, land on Submit). Each step +// re-renders the selector, and a keystroke sent before the next step has +// rendered — or an Enter batched with the navigation before it — lands on the +// wrong (or no) row. This buffer pads the between-keystroke gap so each step +// renders first; kept generous so it still holds on slower machines / under SSH // round-trip latency, where a tighter cadence misfires the answer. export const NATIVE_CHAT_ADVANCE_BUFFER_MS = 500 -/** Per-question wall-clock cadence: body→Enter gap plus the advance buffer that - * lets the next AskUserQuestion step render before its body is written. */ +/** Wall-clock gap between successive AskUserQuestion keystroke groups: the proven + * pty submit gap plus the advance buffer that lets each selector step render + * before the next keystroke is written. */ export const NATIVE_CHAT_QUESTION_STEP_MS = NATIVE_CHAT_SUBMIT_DELAY_MS + NATIVE_CHAT_ADVANCE_BUFFER_MS -/** Pure scheduling math for a per-question answer sequence. For question index - * `i` (0-based) returns the offsets (ms from the start of the send) at which to - * write its framed body and its Enter. Body for question 0 fires at 0; each - * later question starts a full step after the previous, so its body is never - * written until the previous question's Enter has fired plus the advance - * buffer. Exactly one Enter per question; the last Enter submits the prompt. */ -export function nativeChatQuestionOffsets(index: number): { - bodyAt: number - enterAt: number -} { - const bodyAt = index * NATIVE_CHAT_QUESTION_STEP_MS - return { bodyAt, enterAt: bodyAt + NATIVE_CHAT_SUBMIT_DELAY_MS } -} - /** Cancels an in-flight send's pending pty writes (the delayed Enter, and any * later question bodies/Enters). Safe to call after the send completes. */ export type NativeChatSendHandle = { @@ -131,35 +117,30 @@ export function submitNativeChatPrompt( } /** - * Send an AskUserQuestion answer that may span multiple questions. Each line is - * one question's answer (exactly how `formatAskAnswer` builds it). A single line - * is just `sendNativeChatMessage` (no behavior change). For multiple lines we - * write each question's framed body then its Enter as a per-question sequence, - * paced by `NATIVE_CHAT_QUESTION_STEP_MS` so each Enter lands on its own - * rendered question and the LAST Enter submits — exactly N Enters for N lines, - * never a trailing one. Returns a cancel handle that clears every pending timer - * so a detached sequence can't keep writing PTY bytes after unmount/stop. + * Answer Claude's AskUserQuestion by writing its keystroke groups (built by + * `buildAskAnswerKeys`) to the PTY, one group per `NATIVE_CHAT_QUESTION_STEP_MS` + * step so the arrow-navigate selector applies each before the next — a + * navigation/number keystroke batched with the Enter that follows would commit + * the wrong (default) option. `raw` groups are written verbatim as keystrokes; + * `text` groups (a free-text answer) go through the composer's paste framing. + * Returns a cancel handle clearing every pending timer so a detached sequence + * can't keep writing PTY bytes after unmount/stop. */ -export function sendNativeChatAnswer( +export function sendNativeChatAskAnswer( settings: ReturnType, ptyId: string, - lines: string[] + groups: AskAnswerKeyGroup[] ): NativeChatSendHandle { - if (lines.length <= 1) { - return sendNativeChatMessage(settings, ptyId, lines[0] ?? '') + if (groups.length === 0) { + return { cancel: () => {}, settleAfterMs: 0 } } const timers: ReturnType[] = [] - lines.forEach((line, index) => { - const { bodyAt, enterAt } = nativeChatQuestionOffsets(index) + groups.forEach((group, index) => { timers.push( setTimeout(() => { - sendRuntimePtyInput(settings, ptyId, buildNativeChatPasteBytes(line)) - }, bodyAt) - ) - timers.push( - setTimeout(() => { - sendRuntimePtyInput(settings, ptyId, NATIVE_CHAT_SUBMIT) - }, enterAt) + const bytes = 'raw' in group ? group.raw : buildNativeChatPasteBytes(group.text) + sendRuntimePtyInput(settings, ptyId, bytes) + }, index * NATIVE_CHAT_QUESTION_STEP_MS) ) }) return { @@ -168,6 +149,7 @@ export function sendNativeChatAnswer( clearTimeout(timer) } }, - settleAfterMs: nativeChatQuestionOffsets(lines.length - 1).enterAt + // Hold the card until the last keystroke has fired and its submit gap passed. + settleAfterMs: (groups.length - 1) * NATIVE_CHAT_QUESTION_STEP_MS + NATIVE_CHAT_SUBMIT_DELAY_MS } } diff --git a/src/renderer/src/components/native-chat/use-native-chat-interactive-send.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-interactive-send.test.tsx index 09075d99903..76c84e42e98 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-interactive-send.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-interactive-send.test.tsx @@ -6,7 +6,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ cancel: vi.fn(), sendRuntimePtyInput: vi.fn(), - sendNativeChatAnswer: vi.fn(), + sendNativeChatAskAnswer: vi.fn(), sendNativeChatMessage: vi.fn() })) @@ -19,27 +19,73 @@ vi.mock('@/lib/agent-paste-draft', () => ({ })) vi.mock('./native-chat-runtime-send', () => ({ - sendNativeChatAnswer: (...args: unknown[]) => mocks.sendNativeChatAnswer(...args), + sendNativeChatAskAnswer: (...args: unknown[]) => mocks.sendNativeChatAskAnswer(...args), sendNativeChatMessage: (...args: unknown[]) => mocks.sendNativeChatMessage(...args) })) import { useNativeChatInteractiveSend } from './use-native-chat-interactive-send' +import type { AskPrompt } from './native-chat-interactive-prompt' + +const PROMPT: AskPrompt = { + questions: [{ question: 'q', multiSelect: false, options: [{ label: 'A' }, { label: 'B' }] }] +} describe('useNativeChatInteractiveSend', () => { beforeEach(() => { vi.clearAllMocks() const handle = { cancel: mocks.cancel, settleAfterMs: 500 } - mocks.sendNativeChatAnswer.mockReturnValue(handle) + mocks.sendNativeChatAskAnswer.mockReturnValue(handle) mocks.sendNativeChatMessage.mockReturnValue(handle) }) + it('routes a non-Claude answer through the pasted-text send path', () => { + const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'codex')) + + act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) + + // Codex commits a pasted answer: label text 'B', not option-number keystrokes. + expect(mocks.sendNativeChatMessage).toHaveBeenCalledWith( + { terminalTabId: 'tab-1' }, + 'pty-1', + 'B' + ) + expect(mocks.sendNativeChatAskAnswer).not.toHaveBeenCalled() + }) + + it('routes a Claude answer through the option-number keystroke path', () => { + const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'claude')) + + act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) + + // The 2nd option is delivered as its number '2', not the label 'B' (STA-1860). + expect(mocks.sendNativeChatAskAnswer).toHaveBeenCalledWith( + { terminalTabId: 'tab-1' }, + 'pty-1', + [{ raw: '2' }] + ) + expect(mocks.sendNativeChatMessage).not.toHaveBeenCalled() + }) + + it('does nothing when no option is answered', () => { + const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'claude')) + + let settleMs = -1 + act(() => { + settleMs = result.current.sendAnswer(PROMPT, [{ indices: [] }]) + }) + + expect(settleMs).toBe(0) + expect(mocks.sendNativeChatAskAnswer).not.toHaveBeenCalled() + expect(mocks.sendNativeChatMessage).not.toHaveBeenCalled() + }) + it('cancels delayed answer writes when the PTY target changes', () => { const { result, rerender } = renderHook( ({ targetPtyId }) => useNativeChatInteractiveSend('tab-1', targetPtyId, 'codex'), { initialProps: { targetPtyId: 'pty-1' as string | null } } ) - act(() => result.current.sendAnswer('continue')) + act(() => result.current.sendAnswer(PROMPT, [{ indices: [0] }])) rerender({ targetPtyId: 'pty-2' }) expect(mocks.cancel).toHaveBeenCalledOnce() @@ -48,7 +94,7 @@ describe('useNativeChatInteractiveSend', () => { it('cancels delayed answer writes before interrupting the active PTY', () => { const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'claude')) - act(() => result.current.sendAnswer('one\ntwo')) + act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) act(() => result.current.cancel()) expect(mocks.cancel).toHaveBeenCalledOnce() diff --git a/src/renderer/src/components/native-chat/use-native-chat-interactive-send.ts b/src/renderer/src/components/native-chat/use-native-chat-interactive-send.ts index 12e50ec0b12..b964bae58b0 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-interactive-send.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-interactive-send.ts @@ -3,7 +3,14 @@ import { sendRuntimePtyInput } from '@/runtime/runtime-terminal-inspection' import { getSettingsForAgentTabRuntimeOwner } from '@/lib/agent-paste-draft' import type { AgentType } from '../../../../shared/native-chat-types' import { - sendNativeChatAnswer, + buildAskAnswerKeys, + formatAskAnswer, + hasAskAnswer, + type AskAnswerSelection, + type AskPrompt +} from './native-chat-interactive-prompt' +import { + sendNativeChatAskAnswer, sendNativeChatMessage, type NativeChatSendHandle } from './native-chat-runtime-send' @@ -13,10 +20,10 @@ import { const ESC = '\x1b' export type NativeChatInteractiveSend = { - /** Send answer text (bracketed-paste wrapped + Enter, like the composer). - * Returns the ms after which every scheduled write has fired (0 if nothing - * was sent) so the caller can keep the card up until the send settles. */ - sendAnswer: (text: string) => number + /** Deliver the answer to an AskUserQuestion prompt. Returns the ms after which + * every scheduled write has fired (0 if nothing was sent) so the caller can + * keep the card up until the send settles. */ + sendAnswer: (prompt: AskPrompt, selections: AskAnswerSelection[]) => number /** Send a raw control string (e.g. an approval option number or ESC) as-is. */ sendRaw: (raw: string) => void /** Send ESC to interrupt — cancels a question / denies an approval. */ @@ -27,9 +34,10 @@ export type NativeChatInteractiveSend = { * Reuse the desktop composer's exact send path for the interactive cards: * resolve this tab's live ptyId + runtime owner settings, then write bytes via * `sendRuntimePtyInput` (which branches local pty:write vs remote runtime RPC, - * so SSH panes work unchanged). Answers go through `sendNativeChatMessage` - * (bracketed-paste framed body, then a separate delayed Enter); control strings - * (option digits, ESC) are written raw so the agent reads them as keystrokes. + * so SSH panes work unchanged). Claude's AskUserQuestion answers are delivered + * as selector keystrokes (by option number, `sendNativeChatAskAnswer`); other + * agents' question tools commit a pasted answer, so those still go through + * `sendNativeChatMessage`. Control strings (option digits, ESC) are written raw. */ export function useNativeChatInteractiveSend( terminalTabId: string, @@ -59,27 +67,21 @@ export function useNativeChatInteractiveSend( ) const sendAnswer = useCallback( - (text: string): number => { - if (text.trim() === '') { - return 0 - } - if (!targetPtyId) { + (prompt: AskPrompt, selections: AskAnswerSelection[]): number => { + if (!targetPtyId || !hasAskAnswer(prompt, selections)) { return 0 } // Cancel any prior in-flight answer before starting a new one. cancelInFlight() const settings = getSettingsForAgentTabRuntimeOwner(terminalTabId) - // Only Claude's AskUserQuestion is a MULTI-STEP prompt: one question per - // step, each Enter advances to the next, the final Enter submits. So a - // multi-line answer (one line per question, as `formatAskAnswer` builds - // it) is sent as a per-question sequence — body then its own Enter, paced - // so each Enter lands on its rendered question and only the last submits. - // Other agents (e.g. Codex) submit the whole answer with one Enter, so - // gate the stepping on Claude and send a single body + Enter otherwise. - const handle = + // Claude's AskUserQuestion is an arrow-navigate selector: it commits by the + // highlighted option, not a pasted label, so answer it with per-option + // keystrokes (by option number), paced so each step renders before the next. + // Other agents' question tools commit a pasted answer, so send label text. + const handle: NativeChatSendHandle = agent === 'claude' - ? sendNativeChatAnswer(settings, targetPtyId, text.split('\n')) - : sendNativeChatMessage(settings, targetPtyId, text) + ? sendNativeChatAskAnswer(settings, targetPtyId, buildAskAnswerKeys(prompt, selections)) + : sendNativeChatMessage(settings, targetPtyId, formatAskAnswer(prompt, selections)) inFlightRef.current = handle return handle.settleAfterMs },