From c822e7e3e5b099a43f660c20b901903dd5cd8d4f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 21 Jul 2026 22:44:43 -0700 Subject: [PATCH] fix(native-chat): preserve codex question answer semantics --- ...use-mobile-native-chat-answer-send.test.ts | 23 ++++++++ .../use-mobile-native-chat-answer-send.ts | 25 ++++---- .../native-chat-interactive-prompt.test.ts | 47 +++++++++++++++ .../native-chat-interactive-prompt.ts | 2 + .../use-native-chat-interactive-send.test.tsx | 16 +++++ .../use-native-chat-interactive-send.ts | 24 ++++---- src/shared/native-chat-ask.ts | 58 +++++++++++++++++++ 7 files changed, 174 insertions(+), 21 deletions(-) diff --git a/mobile/src/session/use-mobile-native-chat-answer-send.test.ts b/mobile/src/session/use-mobile-native-chat-answer-send.test.ts index 63710ee0c5b..0f3e7275976 100644 --- a/mobile/src/session/use-mobile-native-chat-answer-send.test.ts +++ b/mobile/src/session/use-mobile-native-chat-answer-send.test.ts @@ -195,6 +195,29 @@ describe('useMobileNativeChatAnswerSend', () => { expect(sendRequest.mock.calls[0]?.[1]).toMatchObject({ text: '2', enter: false }) }) + it('does not send a trailing Enter after Codex submits a multi-question answer', async () => { + const sendRequest = vi.fn().mockResolvedValue(acceptedResponse()) + await mount({ sendRequest } as unknown as RpcClient, vi.fn(), 'codex') + const prompt: AskPrompt = { + questions: [ + { question: 'q1', multiSelect: false, options: [{ label: 'A' }, { label: 'B' }] }, + { question: 'q2', multiSelect: false, options: [{ label: 'C' }, { label: 'D' }] } + ] + } + + let result: Promise | undefined + await act(async () => { + result = answerSend?.answerAsk(prompt, [{ indices: [1] }, { indices: [0] }]) + }) + await act(async () => vi.runAllTimersAsync()) + + await expect(result).resolves.toBe(true) + expect(sendRequest.mock.calls.map((call) => call[1])).toEqual([ + expect.objectContaining({ text: '2', enter: false }), + expect.objectContaining({ text: '1', enter: false }) + ]) + }) + it('submits a non-selector answer as pasted label text with a single Enter', async () => { const sendRequest = vi.fn().mockResolvedValue(acceptedResponse()) await mount({ sendRequest } as unknown as RpcClient, vi.fn(), 'grok') diff --git a/mobile/src/session/use-mobile-native-chat-answer-send.ts b/mobile/src/session/use-mobile-native-chat-answer-send.ts index 255f60de3a7..937a3e0b21e 100644 --- a/mobile/src/session/use-mobile-native-chat-answer-send.ts +++ b/mobile/src/session/use-mobile-native-chat-answer-send.ts @@ -3,16 +3,20 @@ import type { RpcClient } from '../transport/rpc-client' import { MOBILE_NATIVE_CHAT_QUESTION_STEP_MS } from './mobile-native-chat-answer-stepping' import { buildAskAnswerKeys, + buildCodexAskAnswerKeys, formatAskAnswer, hasAskAnswer, type AskAnswerSelection, type AskPrompt } from './mobile-native-chat-ask' import { sendMobileNativeChatMessage } from './mobile-native-chat-send' -import { shouldStepNativeChatAskAnswer } from '../../../src/shared/native-chat-agent-support' +import { + resolveNativeChatTranscriptAgent, + shouldStepNativeChatAskAnswer +} from '../../../src/shared/native-chat-agent-support' -/** Sends an AskUserQuestion answer to the active chat pane. Claude's selector is - * answered by option-number keystrokes; other agents get pasted label text. +/** Sends an ask-user answer to the active chat pane. Claude and Codex selectors + * use their agent-specific keystrokes; other agents get pasted label text. * Extracted from the session route to keep that file under its line cap and to * own the pending-timer lifecycle in one place. */ export type MobileNativeChatAnswerSend = { @@ -32,8 +36,8 @@ function sanitizeAskFreeText(text: string): string { /** * Owns the ask-answer send sequence for the mobile native chat. Reads the live * pane/agent through refs (the route already keeps them current) so the returned - * callbacks stay stable. Claude answers are delivered as `buildAskAnswerKeys` - * keystroke groups written one selector-step apart over the EXISTING + * callbacks stay stable. Selector answers are delivered as keystroke groups + * written one step apart over the EXISTING * `terminal.send` passthrough (raw text, no enter) — same contract the * permission card already uses, so old runtimes replay them verbatim (no new * RPC; keystrokes are built client-side). The scheduled wait chain is cancelled @@ -136,14 +140,15 @@ export function useMobileNativeChatAnswerSend(args: { } return false } - // Non-Claude question tools commit a pasted answer, so send the label text - // with one Enter. Claude's arrow-navigate selector ignores pasted labels - // (STA-1860): drive it by option-number keystrokes instead, one group per - // selector step so each renders before the next lands. + // Grok commits pasted labels; Claude and Codex need their selector-specific + // keystrokes paced so each step renders before the next lands. if (!shouldStepNativeChatAskAnswer(agentRef.current)) { return (await sendTerminal(formatAskAnswer(prompt, selections), true)) || fail() } - const groups = buildAskAnswerKeys(prompt, selections) + const groups = + resolveNativeChatTranscriptAgent(agentRef.current) === 'codex' + ? buildCodexAskAnswerKeys(prompt, selections) + : buildAskAnswerKeys(prompt, selections) for (let index = 0; index < groups.length; index += 1) { if (generationRef.current !== generation) { return false 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 8afec9c0fac..3921d862526 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,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { buildAskAnswerKeys, + buildCodexAskAnswerKeys, formatAskAnswer, hasAskAnswer, parseApprovalFromStatus, @@ -240,6 +241,52 @@ describe('buildAskAnswerKeys', () => { }) }) +describe('buildCodexAskAnswerKeys', () => { + it("submits the final multi-question option without Claude's extra Enter", () => { + const prompt: AskPrompt = { + questions: [ + { question: 'q1', multiSelect: false, options: [{ label: 'A' }, { label: 'B' }] }, + { question: 'q2', multiSelect: false, options: [{ label: 'C' }, { label: 'D' }] } + ] + } + + expect(buildCodexAskAnswerKeys(prompt, [{ indices: [1] }, { indices: [0] }])).toEqual([ + { raw: '2' }, + { raw: '1' } + ]) + }) + + it('adds free text as notes before committing the selected row', () => { + expect( + buildCodexAskAnswerKeys(single(['Tabs', 'Spaces']), [ + { indices: [1], other: 'Keep existing files' } + ]) + ).toEqual([{ raw: '\x1b[B' }, { raw: '\t' }, { text: 'Keep existing files' }, { raw: '\r' }]) + }) + + it("targets Codex's synthetic None-of-the-above row for a custom answer", () => { + expect( + buildCodexAskAnswerKeys(single(['Tabs', 'Spaces']), [{ indices: [], other: 'Four spaces' }]) + ).toEqual([{ raw: '\x1b[A' }, { raw: '\t' }, { text: 'Four spaces' }, { raw: '\r' }]) + }) + + it('clears skipped rows and confirms the partial answer once', () => { + const prompt: AskPrompt = { + questions: [ + { question: 'q1', multiSelect: false, options: [{ label: 'A' }, { label: 'B' }] }, + { question: 'q2', multiSelect: false, options: [{ label: 'C' }, { label: 'D' }] } + ] + } + + expect(buildCodexAskAnswerKeys(prompt, [{ indices: [] }, { indices: [1] }])).toEqual([ + { raw: '\x7f' }, + { raw: '\x1b[C' }, + { raw: '2' }, + { raw: '\r' } + ]) + }) +}) + describe('hasAskAnswer', () => { it('is true for a picked option or typed text, false when empty', () => { expect(hasAskAnswer(single(['A', 'B']), [{ indices: [1] }])).toBe(true) 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 f8da5702190..1590f55e7cc 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 @@ -1,6 +1,7 @@ import { translate } from '@/i18n/i18n' import { buildAskAnswerKeys, + buildCodexAskAnswerKeys, formatAskAnswer, hasAskAnswer, parseAskFromStatus, @@ -15,6 +16,7 @@ import { export { buildAskAnswerKeys, + buildCodexAskAnswerKeys, formatAskAnswer, hasAskAnswer, parseAskFromStatus, 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 ca9176d8810..864c62f6f91 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 @@ -98,6 +98,22 @@ describe('useNativeChatInteractiveSend', () => { expect(mocks.sendNativeChatMessage).not.toHaveBeenCalled() }) + it('does not send a trailing Enter after Codex submits a multi-question answer', () => { + const prompt: AskPrompt = { + questions: [ + { question: 'q1', multiSelect: false, options: [{ label: 'A' }, { label: 'B' }] }, + { question: 'q2', multiSelect: false, options: [{ label: 'C' }, { label: 'D' }] } + ] + } + const { result } = renderHook(() => + useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', 'codex') + ) + + act(() => result.current.sendAnswer(prompt, [{ indices: [1] }, { indices: [0] }])) + + expect(mocks.sendNativeChatAskAnswer.mock.calls[0]?.[2]).toEqual([{ raw: '2' }, { raw: '1' }]) + }) + it('routes a Claude answer through the option-number keystroke path', () => { const { result } = renderHook(() => useNativeChatInteractiveSend('tab-1', PANE_KEY, 'pty-1', '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 9b22253927a..43adabae8f3 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,9 +3,13 @@ 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' -import { shouldStepNativeChatAskAnswer } from '../../../../shared/native-chat-agent-support' +import { + resolveNativeChatTranscriptAgent, + shouldStepNativeChatAskAnswer +} from '../../../../shared/native-chat-agent-support' import { buildAskAnswerKeys, + buildCodexAskAnswerKeys, formatAskAnswer, hasAskAnswer, type AskAnswerSelection, @@ -42,9 +46,8 @@ 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). 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 + * so SSH panes work unchanged). Claude and Codex answers use their respective + * selector keystrokes via `sendNativeChatAskAnswer`; other agents still go through * `sendNativeChatMessage`. Control strings (option digits, ESC) are written raw. */ export function useNativeChatInteractiveSend( @@ -90,13 +93,10 @@ export function useNativeChatInteractiveSend( // Cancel any prior in-flight answer before starting a new one. cancelInFlight() const settings = getSettingsForAgentTabRuntimeOwner(terminalTabId) - // 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. - // Gate on the transcript agent (not `=== 'claude'`) so OpenClaude — which - // runs the same selector — takes the keystroke path too. + // Claude and Codex ignore pasted labels but have different selector state + // machines; Grok commits pasted text. OpenClaude follows Claude's path. const stepsAnswer = shouldStepNativeChatAskAnswer(agent) + const buildsCodexAnswer = resolveNativeChatTranscriptAgent(agent) === 'codex' // Why: pin the answered question's baseline BEFORE delivery. A late settle // callback (paced writes + remote acceptance can span seconds on SSH) must // not read the live status and mint a fresh baseline for a replacement @@ -132,7 +132,9 @@ export function useNativeChatInteractiveSend( ? sendNativeChatAskAnswer( settings, targetPtyId, - buildAskAnswerKeys(prompt, selections), + buildsCodexAnswer + ? buildCodexAskAnswerKeys(prompt, selections) + : buildAskAnswerKeys(prompt, selections), onSettled ) : sendNativeChatMessage(settings, targetPtyId, formatAskAnswer(prompt, selections)) diff --git a/src/shared/native-chat-ask.ts b/src/shared/native-chat-ask.ts index 567ed97e0d2..b602e5918f8 100644 --- a/src/shared/native-chat-ask.ts +++ b/src/shared/native-chat-ask.ts @@ -160,6 +160,9 @@ export function formatAskAnswer(prompt: AskPrompt, selections: AskAnswerSelectio // has applied it. const ASK_ENTER = '\r' const ASK_NEXT_TAB = '\x1b[C' +const ASK_PREVIOUS_ROW = '\x1b[A' +const ASK_NEXT_ROW = '\x1b[B' +const ASK_NOTES = '\t' /** 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. @@ -221,6 +224,61 @@ export function buildAskAnswerKeys( return groups } +/** Build keystrokes for Codex's request_user_input overlay. + * + * Unlike Claude, Codex submits on the final option digit and attaches free text + * as notes to the highlighted row. The overlay starts on the first row, so note + * answers move to the target without committing, open notes with Tab, then + * submit with Enter. */ +export function buildCodexAskAnswerKeys( + prompt: AskPrompt, + selections: AskAnswerSelection[] +): AskAnswerKeyGroup[] { + const groups: AskAnswerKeyGroup[] = [] + let hasUnanswered = false + + prompt.questions.forEach((question, questionIndex) => { + const selection = selections[questionIndex] + const selectedIndex = selection?.indices[0] + const note = (selection?.other ?? '').trim() + + if (note) { + const targetIndex = selectedIndex ?? question.options.length + const rowCount = question.options.length + 1 + const nextSteps = targetIndex + const previousSteps = rowCount - targetIndex + const usePrevious = previousSteps < nextSteps + const navigationKey = usePrevious ? ASK_PREVIOUS_ROW : ASK_NEXT_ROW + const navigationSteps = usePrevious ? previousSteps : nextSteps + for (let index = 0; index < navigationSteps; index += 1) { + groups.push({ raw: navigationKey }) + } + groups.push({ raw: ASK_NOTES }, { text: note }, { raw: ASK_ENTER }) + return + } + + if (selectedIndex !== undefined) { + groups.push({ raw: String(selectedIndex + 1) }) + return + } + + hasUnanswered = true + groups.push({ raw: '\x7f' }) + if (questionIndex < prompt.questions.length - 1) { + groups.push({ raw: ASK_NEXT_TAB }) + } else { + groups.push({ raw: ASK_ENTER }) + } + }) + + // Codex opens a confirmation after the last question when any were skipped; + // Proceed is highlighted by default, so one Enter submits the partial answer. + if (hasUnanswered) { + 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]))