diff --git a/src/renderer/src/components/native-chat/NativeChatView.tsx b/src/renderer/src/components/native-chat/NativeChatView.tsx index 8caf7d835c8..cb47afabe73 100644 --- a/src/renderer/src/components/native-chat/NativeChatView.tsx +++ b/src/renderer/src/components/native-chat/NativeChatView.tsx @@ -171,7 +171,7 @@ function NativeChatResolvedView({ const canSend = useNativeChatCanSend(targetPtyId) // Reuse the verified composer send path for interactive cards and composer // stop (Stop sends ESC, the agent-TUI interrupt key). - const interactiveSend = useNativeChatInteractiveSend(terminalTabId, targetPtyId, agent) + const interactiveSend = useNativeChatInteractiveSend(terminalTabId, paneKey, targetPtyId, agent) const [workingInterrupted, setWorkingInterrupted] = useState(false) // True while a question card owns the input region, so the composer is hidden. const [questionActive, setQuestionActive] = useState(false) 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 626e126ffa5..e9bb39c115c 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 @@ -30,6 +30,7 @@ describe('sendNativeChatMessage', () => { beforeEach(() => { vi.useFakeTimers() sendRuntimePtyInput.mockClear() + sendRuntimePtyInput.mockReturnValue(true) }) afterEach(() => { vi.useRealTimers() @@ -156,6 +157,7 @@ describe('sendNativeChatAskAnswer', () => { beforeEach(() => { vi.useFakeTimers() sendRuntimePtyInput.mockClear() + sendRuntimePtyInput.mockReturnValue(true) }) afterEach(() => { vi.useRealTimers() @@ -229,4 +231,19 @@ describe('sendNativeChatAskAnswer', () => { // Only the first keystroke landed; the rest were cancelled. expect(sendRuntimePtyInput).toHaveBeenCalledTimes(1) }) + + it('reports delivery only after settling and suppresses it after cancellation', () => { + const onSettled = vi.fn() + sendRuntimePtyInput.mockReturnValueOnce(true).mockReturnValueOnce(false) + const handle = sendNativeChatAskAnswer(SETTINGS, PTY, [{ raw: '1' }, { raw: '\r' }], onSettled) + + vi.advanceTimersByTime(handle.settleAfterMs) + expect(onSettled).toHaveBeenCalledExactlyOnceWith(false) + + const canceledSettled = vi.fn() + const canceled = sendNativeChatAskAnswer(SETTINGS, PTY, [{ raw: '1' }], canceledSettled) + canceled.cancel() + vi.runAllTimers() + expect(canceledSettled).not.toHaveBeenCalled() + }) }) 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 59ff443fd24..d4b1ec9f966 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 @@ -20,7 +20,6 @@ export { NATIVE_CHAT_ADVANCE_BUFFER_MS, NATIVE_CHAT_QUESTION_STEP_MS, NATIVE_CHA export const NATIVE_CHAT_IMAGE_ATTACHMENT_SETTLE_MS = 300 - /** 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 = { @@ -113,20 +112,31 @@ export function submitNativeChatPrompt( export function sendNativeChatAskAnswer( settings: ReturnType, ptyId: string, - groups: AskAnswerKeyGroup[] + groups: AskAnswerKeyGroup[], + onSettled?: (delivered: boolean) => void ): NativeChatSendHandle { if (groups.length === 0) { return { cancel: () => {}, settleAfterMs: 0 } } const timers: ReturnType[] = [] + let delivered = true groups.forEach((group, index) => { timers.push( setTimeout(() => { const bytes = 'raw' in group ? group.raw : buildNativeChatPasteBytes(group.text) - sendRuntimePtyInput(settings, ptyId, bytes) + if (!sendRuntimePtyInput(settings, ptyId, bytes)) { + delivered = false + } }, index * NATIVE_CHAT_QUESTION_STEP_MS) ) }) + const settleAfterMs = + (groups.length - 1) * NATIVE_CHAT_QUESTION_STEP_MS + NATIVE_CHAT_SUBMIT_DELAY_MS + if (onSettled) { + // Why: status inference must wait for every paced write and must not run + // after cancellation or a rejected runtime write. + timers.push(setTimeout(() => onSettled(delivered), settleAfterMs)) + } return { cancel: () => { for (const timer of timers) { @@ -134,6 +144,6 @@ export function sendNativeChatAskAnswer( } }, // 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 + settleAfterMs } } 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 5a336009c5c..9010bc01057 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 @@ -5,11 +5,30 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ cancel: vi.fn(), + inferQuestionAnswered: vi.fn(() => Promise.resolve(true)), sendRuntimePtyInput: vi.fn(), sendNativeChatAskAnswer: vi.fn(), sendNativeChatMessage: vi.fn() })) +const PANE_KEY = 'tab-1:11111111-1111-4111-8111-111111111111' +const waitingQuestion = { + state: 'waiting' as const, + prompt: 'pick one', + updatedAt: Date.now(), + stateStartedAt: Date.now(), + agentType: 'claude' as const, + paneKey: PANE_KEY, + stateHistory: [], + toolName: 'AskUserQuestion' +} + +vi.mock('../../store', () => ({ + useAppStore: { + getState: () => ({ agentStatusByPaneKey: { [PANE_KEY]: waitingQuestion } }) + } +})) + vi.mock('@/runtime/runtime-terminal-inspection', () => ({ sendRuntimePtyInput: (...args: unknown[]) => mocks.sendRuntimePtyInput(...args) })) @@ -33,13 +52,19 @@ const PROMPT: AskPrompt = { describe('useNativeChatInteractiveSend', () => { beforeEach(() => { vi.clearAllMocks() + Object.defineProperty(window, 'api', { + configurable: true, + value: { agentStatus: { inferQuestionAnswered: mocks.inferQuestionAnswered } } + }) const handle = { cancel: mocks.cancel, settleAfterMs: 500 } 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')) + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'codex') + ) act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) @@ -53,7 +78,9 @@ describe('useNativeChatInteractiveSend', () => { }) it('routes a Claude answer through the option-number keystroke path', () => { - const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'claude')) + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'claude') + ) act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) @@ -61,13 +88,16 @@ describe('useNativeChatInteractiveSend', () => { expect(mocks.sendNativeChatAskAnswer).toHaveBeenCalledWith( { terminalTabId: 'tab-1' }, 'pty-1', - [{ raw: '2' }] + [{ raw: '2' }], + expect.any(Function) ) expect(mocks.sendNativeChatMessage).not.toHaveBeenCalled() }) it('does nothing when no option is answered', () => { - const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'claude')) + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'claude') + ) let settleMs = -1 act(() => { @@ -81,7 +111,7 @@ describe('useNativeChatInteractiveSend', () => { it('cancels delayed answer writes when the PTY target changes', () => { const { result, rerender } = renderHook( - ({ targetPtyId }) => useNativeChatInteractiveSend('tab-1', targetPtyId, 'codex'), + ({ targetPtyId }) => useNativeChatInteractiveSend('tab-1', PANE_KEY, targetPtyId, 'codex'), { initialProps: { targetPtyId: 'pty-1' as string | null } } ) @@ -92,7 +122,9 @@ describe('useNativeChatInteractiveSend', () => { }) it('cancels delayed answer writes before interrupting the active PTY', () => { - const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'claude')) + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'claude') + ) act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) act(() => result.current.cancel()) @@ -106,7 +138,9 @@ describe('useNativeChatInteractiveSend', () => { }) it('can cancel delayed writes without interrupting the replacement prompt', () => { - const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', 'pty-1', 'claude')) + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'claude') + ) act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) act(() => result.current.cancelPending()) @@ -114,4 +148,27 @@ describe('useNativeChatInteractiveSend', () => { expect(mocks.cancel).toHaveBeenCalledOnce() expect(mocks.sendRuntimePtyInput).not.toHaveBeenCalled() }) + + it('infers a Claude question answer only after every runtime write was delivered', () => { + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'claude') + ) + + act(() => result.current.sendAnswer(PROMPT, [{ indices: [1] }])) + expect(mocks.inferQuestionAnswered).not.toHaveBeenCalled() + + const onSettled = mocks.sendNativeChatAskAnswer.mock.calls[0]?.[3] + expect(onSettled).toBeTypeOf('function') + onSettled?.(false) + expect(mocks.inferQuestionAnswered).not.toHaveBeenCalled() + onSettled?.(true) + + expect(mocks.inferQuestionAnswered).toHaveBeenCalledExactlyOnceWith({ + paneKey: PANE_KEY, + baselineUpdatedAt: waitingQuestion.updatedAt, + baselineStateStartedAt: waitingQuestion.stateStartedAt, + baselinePrompt: 'pick one', + baselineAgentType: 'claude' + }) + }) }) 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 cb49cd9c57d..f27d7f63be7 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 @@ -1,4 +1,5 @@ import { useCallback, useLayoutEffect, useRef } from 'react' +import { useAppStore } from '../../store' import { sendRuntimePtyInput } from '@/runtime/runtime-terminal-inspection' import { getSettingsForAgentTabRuntimeOwner } from '@/lib/agent-paste-draft' import type { AgentType } from '../../../../shared/native-chat-types' @@ -15,6 +16,7 @@ import { sendNativeChatMessage, type NativeChatSendHandle } from './native-chat-runtime-send' +import { inferQuestionAnsweredFromCurrentStatus } from '../terminal-pane/agent-question-answered-inference' // ESC is the agent-TUI interrupt/cancel key over the PTY (matches how the // composer forwards Escape). Used to cancel a question or deny an approval. @@ -44,6 +46,7 @@ export type NativeChatInteractiveSend = { */ export function useNativeChatInteractiveSend( terminalTabId: string, + paneKey: string, targetPtyId: string | null, agent: AgentType ): NativeChatInteractiveSend { @@ -83,13 +86,38 @@ export function useNativeChatInteractiveSend( // Other agents' question tools commit a pasted answer, so send label text. // Gate on the transcript agent (not `=== 'claude'`) so OpenClaude — which // runs the same selector — takes the keystroke path too. + const onSettled = + agent === 'claude' + ? (delivered: boolean): void => { + if (!delivered) { + return + } + inferQuestionAnsweredFromCurrentStatus({ + paneKey, + getStatusEntry: () => useAppStore.getState().agentStatusByPaneKey[paneKey], + inferQuestionAnswered: (request) => + window.api.agentStatus.inferQuestionAnswered(request).catch((err) => { + console.warn('[agent-question] native-chat inference failed:', err) + return false + }) + }) + } + : undefined const handle: NativeChatSendHandle = shouldStepNativeChatAskAnswer(agent) - ? sendNativeChatAskAnswer(settings, targetPtyId, buildAskAnswerKeys(prompt, selections)) + ? sendNativeChatAskAnswer( + settings, + targetPtyId, + buildAskAnswerKeys(prompt, selections), + onSettled + ) : sendNativeChatMessage(settings, targetPtyId, formatAskAnswer(prompt, selections)) + // Why: native-chat answer writes bypass xterm.onData. Infer only after + // every paced selector write has fired, so an early digit in a multi-step + // answer cannot dismiss the wait or cancel the remaining writes. inFlightRef.current = handle return handle.settleAfterMs }, - [terminalTabId, targetPtyId, agent, cancelInFlight] + [terminalTabId, paneKey, targetPtyId, agent, cancelInFlight] ) // Stop/cancel: drop any pending answer writes, then send ESC to interrupt. diff --git a/src/renderer/src/components/terminal-pane/agent-question-answered-inference.test.ts b/src/renderer/src/components/terminal-pane/agent-question-answered-inference.test.ts index 0d4d0b5ad80..1fd07583b97 100644 --- a/src/renderer/src/components/terminal-pane/agent-question-answered-inference.test.ts +++ b/src/renderer/src/components/terminal-pane/agent-question-answered-inference.test.ts @@ -3,6 +3,15 @@ import type { AgentStatusEntry } from '../../../../shared/agent-status-types' import { createAgentQuestionAnsweredInference } from './agent-question-answered-inference' const PANE_KEY = 'tab-1:11111111-1111-4111-8111-111111111111' +const SINGLE_SELECT_PROMPT = JSON.stringify({ + questions: [ + { + question: 'pick a color', + multiSelect: false, + options: [{ label: 'red' }, { label: 'blue' }] + } + ] +}) function makeWaitingQuestionEntry(overrides: Partial = {}): AgentStatusEntry { return { @@ -14,6 +23,7 @@ function makeWaitingQuestionEntry(overrides: Partial = {}): Ag paneKey: PANE_KEY, stateHistory: [], toolName: 'AskUserQuestion', + interactivePrompt: SINGLE_SELECT_PROMPT, ...overrides } } @@ -60,6 +70,59 @@ describe('agent question-answered inference', () => { expect(inferQuestionAnswered).toHaveBeenCalledOnce() }) + it('keeps waiting when a digit only advances or toggles a partial answer', () => { + const prompts = [ + JSON.stringify({ + questions: [ + { question: 'first?', multiSelect: false, options: [{ label: 'A' }] }, + { question: 'second?', multiSelect: false, options: [{ label: 'B' }] } + ] + }), + JSON.stringify({ + questions: [ + { + question: 'pick several', + multiSelect: true, + options: [{ label: 'A' }, { label: 'B' }] + } + ] + }) + ] + + for (const interactivePrompt of prompts) { + const { inference, inferQuestionAnswered } = makeInference( + makeWaitingQuestionEntry({ interactivePrompt }) + ) + inference.observeSentTerminalInput('1') + inference.observeSentTerminalInput('\r') + expect(inferQuestionAnswered).not.toHaveBeenCalled() + } + }) + + it('keeps waiting when the synthetic free-text row is selected', () => { + const { inference, inferQuestionAnswered } = makeInference(makeWaitingQuestionEntry()) + + // Two declared options means 3 opens Claude's synthetic "Type something" row. + inference.observeSentTerminalInput('3') + + expect(inferQuestionAnswered).not.toHaveBeenCalled() + }) + + it('does not read status for ordinary terminal input', () => { + const getStatusEntry = vi.fn(() => makeWaitingQuestionEntry()) + const inferQuestionAnswered = vi.fn() + const inference = createAgentQuestionAnsweredInference({ + paneKey: PANE_KEY, + getStatusEntry, + inferQuestionAnswered + }) + + inference.observeSentTerminalInput('ordinary typing') + + expect(getStatusEntry).not.toHaveBeenCalled() + expect(inferQuestionAnswered).not.toHaveBeenCalled() + }) + it('ignores non-submit input, batched keystrokes, and pastes', () => { const { inference, inferQuestionAnswered } = makeInference(makeWaitingQuestionEntry()) diff --git a/src/renderer/src/components/terminal-pane/agent-question-answered-inference.ts b/src/renderer/src/components/terminal-pane/agent-question-answered-inference.ts index f4a49b409aa..1469648be2e 100644 --- a/src/renderer/src/components/terminal-pane/agent-question-answered-inference.ts +++ b/src/renderer/src/components/terminal-pane/agent-question-answered-inference.ts @@ -4,6 +4,7 @@ import { } from '../../../../shared/agent-status-types' import { isAskUserQuestionTool, + isPotentialQuestionAnsweredSubmitInput, isQuestionAnsweredSubmitInput, type AgentQuestionAnsweredInferenceRequest } from '../../../../shared/agent-question-answered-intent' @@ -22,6 +23,39 @@ type AgentQuestionAnsweredInferenceDeps = { now?: () => number } +function inferQuestionAnsweredFromEntry( + deps: AgentQuestionAnsweredInferenceDeps, + entry: AgentStatusEntry | undefined +): boolean { + const now = deps.now ?? Date.now + if ( + !entry || + entry.state !== 'waiting' || + entry.agentType !== 'claude' || + !isAskUserQuestionTool(entry.toolName) || + !isExplicitAgentStatusFresh(entry, now(), AGENT_STATUS_STALE_AFTER_MS) + ) { + return false + } + void deps.inferQuestionAnswered({ + paneKey: deps.paneKey, + baselineUpdatedAt: entry.updatedAt, + baselineStateStartedAt: entry.stateStartedAt, + baselinePrompt: entry.prompt, + baselineAgentType: entry.agentType + }) + return true +} + +/** Completion signal for answer surfaces that write directly to the runtime + * instead of xterm (notably native chat). The same fresh-status baseline is + * used, so a real hook that wins the race prevents the fallback IPC. */ +export function inferQuestionAnsweredFromCurrentStatus( + deps: AgentQuestionAnsweredInferenceDeps +): boolean { + return inferQuestionAnsweredFromEntry(deps, deps.getStatusEntry()) +} + /** Sibling of the interrupt inference for a hook Claude never sends: answering * an AskUserQuestion emits no event, so the submit keystroke into the waiting * pane is the only "question dealt with" signal. Unlike interrupts there is @@ -35,26 +69,17 @@ export function createAgentQuestionAnsweredInference({ }: AgentQuestionAnsweredInferenceDeps): AgentQuestionAnsweredInference { return { observeSentTerminalInput(data) { - if (!isQuestionAnsweredSubmitInput(data)) { + // Why: ordinary terminal input is the hot path. Reject it with one + // constant-time membership check before reading Zustand or parsing the + // bounded interactive-prompt JSON. + if (!isPotentialQuestionAnsweredSubmitInput(data)) { return } const entry = getStatusEntry() - if ( - !entry || - entry.state !== 'waiting' || - entry.agentType !== 'claude' || - !isAskUserQuestionTool(entry.toolName) || - !isExplicitAgentStatusFresh(entry, now(), AGENT_STATUS_STALE_AFTER_MS) - ) { + if (!entry || !isQuestionAnsweredSubmitInput(data, entry.interactivePrompt)) { return } - void inferQuestionAnswered({ - paneKey, - baselineUpdatedAt: entry.updatedAt, - baselineStateStartedAt: entry.stateStartedAt, - baselinePrompt: entry.prompt, - baselineAgentType: entry.agentType - }) + inferQuestionAnsweredFromEntry({ paneKey, getStatusEntry, inferQuestionAnswered, now }, entry) } } } diff --git a/src/shared/agent-question-answered-intent.ts b/src/shared/agent-question-answered-intent.ts index f0864812283..36f357bbb9b 100644 --- a/src/shared/agent-question-answered-intent.ts +++ b/src/shared/agent-question-answered-intent.ts @@ -19,21 +19,63 @@ export function isAskUserQuestionTool(toolName: string | undefined): boolean { return toolName?.replaceAll(/[^a-z0-9]/gi, '').toLowerCase() === 'askuserquestion' } -// Why: answering an interactive question emits no hook event, so the submit -// keystroke is the only signal that the question was dealt with. Enter (plus -// kitty-keyboard Enter encodings some agent TUIs enable) confirms the -// highlighted option, and a bare digit is the selector's quick-select, which -// submits immediately without Enter. Exact single-keystroke matches only — -// batched input or pasted text must never clear a pending question card. -const QUESTION_ANSWER_SUBMIT_INPUTS = new Set([ +const QUESTION_ANSWER_ENTER_INPUTS: ReadonlySet = new Set([ '\r', '\n', '\r\n', '\x1b[13u', - '\x1b[13;1u', - ...'123456789' + '\x1b[13;1u' ]) +const QUESTION_ANSWER_DIGIT_INPUTS: ReadonlySet = new Set('123456789') -export function isQuestionAnsweredSubmitInput(data: string): boolean { - return QUESTION_ANSWER_SUBMIT_INPUTS.has(data) +export function isPotentialQuestionAnsweredSubmitInput(data: string): boolean { + return QUESTION_ANSWER_ENTER_INPUTS.has(data) || QUESTION_ANSWER_DIGIT_INPUTS.has(data) +} + +function readSingleSelectOptionCount(interactivePrompt: string | undefined): number | null { + if (!interactivePrompt) { + return null + } + try { + const parsed = JSON.parse(interactivePrompt) as { questions?: unknown } + if (!Array.isArray(parsed.questions) || parsed.questions.length !== 1) { + return -1 + } + const [question] = parsed.questions as { multiSelect?: unknown; options?: unknown }[] + if (!question || question.multiSelect === true || !Array.isArray(question.options)) { + return -1 + } + return question.options.length + } catch { + return null + } +} + +/** True only when one keystroke is enough to finish the whole prompt. + * Why: digits merely advance multi-question prompts and toggle multi-selects; + * the synthetic "Type something" row also opens an editor instead of + * submitting. Without the prompt-shape gate those partial choices clear the + * waiting indicator while Claude is still blocked on more input. */ +export function isQuestionAnsweredSubmitInput( + data: string, + interactivePrompt: string | undefined +): boolean { + if (!isPotentialQuestionAnsweredSubmitInput(data)) { + return false + } + const optionCount = readSingleSelectOptionCount(interactivePrompt) + if (optionCount === -1) { + return false + } + if (QUESTION_ANSWER_ENTER_INPUTS.has(data)) { + // Older hook payloads can omit tool input; Enter remains the conservative + // fallback because it is the ordinary submit path for a question. + return true + } + if (optionCount === null) { + return false + } + // Claude adds a final "Type something" row after the declared options. + // Only declared option numbers complete a single-select immediately. + return Number(data) <= optionCount }