diff --git a/mobile/src/session/MobileNativeChatQuestion.test.tsx b/mobile/src/session/MobileNativeChatQuestion.test.tsx new file mode 100644 index 00000000000..be9777a0b69 --- /dev/null +++ b/mobile/src/session/MobileNativeChatQuestion.test.tsx @@ -0,0 +1,106 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { MobileNativeChatQuestion } from './MobileNativeChatQuestion' + +vi.mock('react-native', () => ({ + Pressable: 'Pressable', + StyleSheet: { create: (styles: unknown) => styles, hairlineWidth: 1 }, + Text: 'Text', + TextInput: 'TextInput', + View: 'View' +})) + +vi.mock('lucide-react-native', () => ({ + ArrowUp: 'ArrowUp', + Check: 'Check', + CircleHelp: 'CircleHelp' +})) + +describe('MobileNativeChatQuestion', () => { + let renderer: ReactTestRenderer | null = null + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + }) + + it('submits the selected duplicate-label row by position', async () => { + const onAnswer = vi.fn(async () => true) + + await act(async () => { + renderer = create( + createElement(MobileNativeChatQuestion, { + question: { + question: 'Pick regions', + options: ['Region', 'Region'], + multiSelect: true, + allowOther: false, + optionTokens: ['first-token', 'second-token'] + }, + onAnswer + }) + ) + }) + + const choices = renderer.root.findAllByProps({ accessibilityRole: 'checkbox' }) + await act(async () => choices[1]!.props.onPress()) + const submit = renderer.root.findByProps({ accessibilityLabel: 'Submit selected options' }) + await act(async () => submit.props.onPress()) + + expect(onAnswer).toHaveBeenCalledWith('second-token') + }) + + it('submits a tokenless duplicate-label row by position', async () => { + const onAnswer = vi.fn(async () => true) + + await act(async () => { + renderer = create( + createElement(MobileNativeChatQuestion, { + question: { + question: 'Pick one', + options: ['Choice', 'Choice'], + multiSelect: false, + allowOther: false, + optionTokens: ['first-token', null] + }, + onAnswer + }) + ) + }) + + const choices = renderer.root.findAllByProps({ accessibilityRole: 'button' }) + await act(async () => choices[1]!.props.onPress()) + + expect(onAnswer).toHaveBeenCalledWith('Choice') + }) + + it('submits structured multi-select choices together with other text', async () => { + const onAnswer = vi.fn(async () => true) + + await act(async () => { + renderer = create( + createElement(MobileNativeChatQuestion, { + question: { + question: 'Pick regions', + options: ['us-east', 'eu-west'], + multiSelect: true, + allowOther: true, + optionTokens: ['east-token', 'west-token'], + freeTextToken: 'other-token' + }, + onAnswer + }) + ) + }) + + const choices = renderer.root.findAllByProps({ accessibilityRole: 'checkbox' }) + await act(async () => choices[0]!.props.onPress()) + const input = renderer.root.findByType('TextInput') + await act(async () => input.props.onChangeText('ap-south')) + const submit = renderer.root.findByProps({ accessibilityLabel: 'Submit selected options' }) + await act(async () => submit.props.onPress()) + + expect(onAnswer).toHaveBeenCalledWith('east-token, other-token:ap-south') + }) +}) diff --git a/mobile/src/session/MobileNativeChatQuestion.tsx b/mobile/src/session/MobileNativeChatQuestion.tsx index f4a34494328..9eae7210bc8 100644 --- a/mobile/src/session/MobileNativeChatQuestion.tsx +++ b/mobile/src/session/MobileNativeChatQuestion.tsx @@ -3,7 +3,8 @@ import { Pressable, StyleSheet, Text, TextInput, View } from 'react-native' import { ArrowUp, Check, CircleHelp } from 'lucide-react-native' import { colors, radii, spacing, typography } from '../theme/mobile-theme' import { - formatQuestionAnswer, + formatQuestionAnswerByIndexes, + formatQuestionAnswerWithOtherByIndexes, formatQuestionFreeTextAnswer, type MobileChatQuestion } from './mobile-native-chat-question' @@ -18,7 +19,7 @@ type Props = { * the user answer freely (the escape hatch) when the heuristic misreads the * options or none apply. */ export function MobileNativeChatQuestion({ question, onAnswer }: Props): React.JSX.Element { - const [selected, setSelected] = useState([]) + const [selectedOptionIndexes, setSelectedOptionIndexes] = useState([]) const [freeText, setFreeText] = useState('') const [sending, setSending] = useState(false) const sendingRef = useRef(false) @@ -27,9 +28,11 @@ export function MobileNativeChatQuestion({ question, onAnswer }: Props): React.J const hasOptions = question.options.length > 0 const trimmedFreeText = freeText.trim() - const toggle = (option: string): void => { - setSelected((prev) => - prev.includes(option) ? prev.filter((o) => o !== option) : [...prev, option] + const toggle = (optionIndex: number): void => { + setSelectedOptionIndexes((prev) => + prev.includes(optionIndex) + ? prev.filter((index) => index !== optionIndex) + : [...prev, optionIndex] ) } @@ -47,34 +50,51 @@ export function MobileNativeChatQuestion({ question, onAnswer }: Props): React.J } } - const answerSingle = async (option: string, optionIndex: number): Promise => { + const answerSingle = async (optionIndex: number): Promise => { const token = question.optionTokens[optionIndex] - await sendAnswer(token && token.length > 0 ? token : formatQuestionAnswer(question, [option])) + await sendAnswer( + token && token.length > 0 ? token : formatQuestionAnswerByIndexes(question, [optionIndex]) + ) } const submitMulti = async (): Promise => { - if (selected.length === 0) { + if (selectedOptionIndexes.length === 0) { return } - await sendAnswer(formatQuestionAnswer(question, selected)) + const answer = + question.freeTextToken && trimmedFreeText.length > 0 + ? formatQuestionAnswerWithOtherByIndexes(question, selectedOptionIndexes, trimmedFreeText) + : formatQuestionAnswerByIndexes(question, selectedOptionIndexes) + if (await sendAnswer(answer)) { + setFreeText('') + } } const submitFreeText = async (): Promise => { if (trimmedFreeText.length === 0) { return } - if (await sendAnswer(formatQuestionFreeTextAnswer(question, trimmedFreeText))) { + const answer = + question.multiSelect && question.freeTextToken && selectedOptionIndexes.length > 0 + ? formatQuestionAnswerWithOtherByIndexes(question, selectedOptionIndexes, trimmedFreeText) + : formatQuestionFreeTextAnswer(question, trimmedFreeText) + if (await sendAnswer(answer)) { setFreeText('') } } - const canSubmitMulti = selected.length > 0 && !sending + const canSubmitMulti = selectedOptionIndexes.length > 0 && !sending const canSendFreeText = allowOther && trimmedFreeText.length > 0 && !sending // Stable keys for option rows even if an agent repeats a label. const optionRows = useMemo( - () => question.options.map((label, index) => ({ label, key: `${index}:${label}` })), - [question.options] + () => + question.options.map((label, index) => ({ + label, + description: question.optionDescriptions?.[index], + key: `${index}:${label}` + })), + [question.optionDescriptions, question.options] ) return ( @@ -86,8 +106,8 @@ export function MobileNativeChatQuestion({ question, onAnswer }: Props): React.J {hasOptions ? ( - {optionRows.map(({ label, key }, optIndex) => { - const isSelected = selected.includes(label) + {optionRows.map(({ label, description, key }, optIndex) => { + const isSelected = selectedOptionIndexes.includes(optIndex) return ( - question.multiSelect ? toggle(label) : answerSingle(label, optIndex) - } + onPress={() => (question.multiSelect ? toggle(optIndex) : answerSingle(optIndex))} > {question.multiSelect ? ( {isSelected ? : null} ) : null} - {label} + + {label} + {description ? ( + + {description} + + ) : null} + ) })} @@ -126,7 +151,7 @@ export function MobileNativeChatQuestion({ question, onAnswer }: Props): React.J disabled={!canSubmitMulti} > - Submit{selected.length > 0 ? ` (${selected.length})` : ''} + Submit{selectedOptionIndexes.length > 0 ? ` (${selectedOptionIndexes.length})` : ''} ) : null} @@ -207,11 +232,19 @@ const styles = StyleSheet.create({ optionSelected: { borderColor: colors.accentBlue }, - optionText: { + optionBody: { flex: 1, + gap: 2 + }, + optionText: { color: colors.textPrimary, fontSize: typography.bodySize + 1 }, + optionDescription: { + color: colors.textMuted, + fontSize: typography.metaSize, + lineHeight: typography.metaSize + 5 + }, checkbox: { width: 20, height: 20, diff --git a/mobile/src/session/mobile-native-chat-eligibility.test.ts b/mobile/src/session/mobile-native-chat-eligibility.test.ts index 3a42b8780af..62aeb49a477 100644 --- a/mobile/src/session/mobile-native-chat-eligibility.test.ts +++ b/mobile/src/session/mobile-native-chat-eligibility.test.ts @@ -155,13 +155,27 @@ describe('resolveMobileNativeChat', () => { }) }) - it('rejects non-Codex structured agent-session tabs', () => { + it('resolves Claude structured agent-session tabs on the same journal path', () => { expect( resolveMobileNativeChat({ type: 'agent-session', sessionId: 'structured-1', agent: 'claude' - } as never) + }) + ).toEqual({ + agent: 'claude', + sessionId: 'structured-1', + transcriptPath: null + }) + }) + + it('rejects structured agent-session tabs whose provider the reducer cannot replay', () => { + expect( + resolveMobileNativeChat({ + type: 'agent-session', + sessionId: 'structured-1', + agent: 'grok' + }) ).toBeNull() }) diff --git a/mobile/src/session/mobile-native-chat-eligibility.ts b/mobile/src/session/mobile-native-chat-eligibility.ts index f91a98fb2b3..892e7d7cfe4 100644 --- a/mobile/src/session/mobile-native-chat-eligibility.ts +++ b/mobile/src/session/mobile-native-chat-eligibility.ts @@ -1,3 +1,4 @@ +import { isAgentSessionHandleProvider } from '../../../src/shared/agent-session-provider-handle' import type { AgentWorkingMode } from '../../../src/shared/agent-status-types' import type { MobileWebNativeChatAgentStatus } from '../../../src/shared/mobile-web/native-chat-operation-contract' import { isRuntimeOwnedSshTargetId } from '../../../src/shared/execution-host' @@ -59,7 +60,9 @@ export function resolveMobileNativeChat( return null } if (tab.type === 'agent-session') { - return tab.sessionId && tab.agent === 'codex' + // Structured tabs are journal-backed, so any provider the shared reducer can + // replay renders here — there is no per-agent transcript layout to know. + return tab.sessionId && isAgentSessionHandleProvider(tab.agent) ? { agent: tab.agent, sessionId: tab.sessionId, transcriptPath: null } : null } diff --git a/mobile/src/session/mobile-native-chat-question.ts b/mobile/src/session/mobile-native-chat-question.ts index 5d4e65a46ff..59ba3d72aba 100644 --- a/mobile/src/session/mobile-native-chat-question.ts +++ b/mobile/src/session/mobile-native-chat-question.ts @@ -13,6 +13,8 @@ export type MobileChatQuestion = { * parallel to `options`. Null where the option was a plain bullet. Used to * echo the exact choice the agent listed back to the terminal. */ optionTokens: (string | null)[] + /** Per-option secondary text from structured prompts, parallel to `options`. */ + optionDescriptions?: (string | undefined)[] /** Opaque prefix used when free-text answers must target a specific prompt. */ freeTextToken?: string } @@ -130,6 +132,48 @@ export function parseAgentQuestion(text: string): MobileChatQuestion | null { } } +function formatQuestionOptionAtIndex(question: MobileChatQuestion, index: number): string | null { + if (!Number.isInteger(index) || index < 0 || index >= question.options.length) { + return null + } + const label = question.options[index] + if (label == null || label.trim().length === 0) { + return null + } + const token = question.optionTokens[index] + return token != null && token.length > 0 ? token : label +} + +function formatQuestionAnswerPartsByIndexes( + question: MobileChatQuestion, + selectedIndexes: number[] +): string[] { + return selectedIndexes + .map((index) => formatQuestionOptionAtIndex(question, index)) + .filter((part): part is string => part != null && part.trim().length > 0) +} + +export function formatQuestionAnswerByIndexes( + question: MobileChatQuestion, + selectedIndexes: number[] +): string { + const parts = formatQuestionAnswerPartsByIndexes(question, selectedIndexes) + return parts.join(question.multiSelect ? ', ' : ' ') +} + +export function formatQuestionAnswerWithOtherByIndexes( + question: MobileChatQuestion, + selectedIndexes: number[], + text: string +): string { + const parts = formatQuestionAnswerPartsByIndexes(question, selectedIndexes) + const other = formatQuestionFreeTextAnswer(question, text) + if (other.length > 0) { + parts.push(other) + } + return parts.join(question.multiSelect ? ', ' : ' ') +} + /** * Build the text to send to the agent terminal for the selected option(s). * Convention: echo the option's leading marker (number/letter) when the list had @@ -150,8 +194,7 @@ export function formatQuestionAnswer(question: MobileChatQuestion, selected: str // Free-text / unknown entry: pass the user's text straight through. return label } - const token = question.optionTokens[index] - return token != null && token.length > 0 ? token : label + return formatQuestionOptionAtIndex(question, index) ?? label }) return parts.join(question.multiSelect ? ', ' : ' ') diff --git a/mobile/src/session/mobile-session-route-parity.test.ts b/mobile/src/session/mobile-session-route-parity.test.ts index 27659b25e04..679f35d2f76 100644 --- a/mobile/src/session/mobile-session-route-parity.test.ts +++ b/mobile/src/session/mobile-session-route-parity.test.ts @@ -74,8 +74,10 @@ const HEAD_CALLBACK_IDENTITY_SHA256 = const HEAD_CALLBACK_BODY_SHA256 = '596f85a60076e49fb198028f8fede0e122fe18c5cc7aaad09562a514cc7c2e29' const HEAD_EFFECT_SHA256 = 'f81ef4b4794875643dd429e9dfb6cffab037feb68a334e260c0045a258c07d51' const HEAD_CONTENT_HOOK_SHA256 = 'd74431115b27c22dd38c29a510604554ca767cdd2585beaa73ec2e2dae0c5de4' +// Re-frozen when main's structured Claude chat (#18741) generalized the bare-launch gate +// from an `agent === 'codex'` literal to isAgentSessionHandleProvider. const HEAD_NESTED_FUNCTION_SHA256 = - '778091a23e090f4d9b512e369fdd8a703dd76b0caa08e1fe9b1d8bf46223df46' + 'b2473556b97f3b8f41fbce728e1a0e5a2bb3808e17a9e022515af1fa7dd97ecc' const HEAD_NATIVE_REGISTRATION_SHA256 = '482c1b9df56a02236e8efcc56fab41de0ea525aa5a03785dc5ac4af8f694c457' const HEAD_NATIVE_REMOVAL_SHA256 = @@ -83,10 +85,10 @@ const HEAD_NATIVE_REMOVAL_SHA256 = const HEAD_TIMER_CREATION_SHA256 = '688342d48a1b4a46cdffbf0d8953bac245fb6d3c4fe1b5698a1ea6e1e1929bed' const HEAD_TIMER_CLEANUP_SHA256 = '8a45ae3c8a01a639a40ffaf3c0fc89a2e0b610623306818c86bad4ef9195b824' -// Re-frozen when main's iPad hardware-keyboard fix (#12772) added the -// reopenFocusedInputWhenKeyboardHidden argument: one more runtime string, same JSX. +// Re-frozen when main's structured Claude chat (#18741) dropped the 'codex' literal +// from the bare-launch gate: one fewer runtime string, same JSX. const HEAD_RUNTIME_STRING_SHA256 = - '4be6fc665f971e9eb294814b6d985f7405add51adf3c1bf9b06a8af948fa746e' + '12405d53b411f1849d183e018f027a5ffe0150ecc01a069d96a3e6973d922a37' // Re-frozen when the repeatable accessory key's press handler dropped its duplicate // handleAccessoryKey call: startAccessoryRepeat already sends at press time, so every // tap emitted the key twice. Same element count, one attribute body changed. @@ -527,7 +529,7 @@ describe('mobile session route extraction parity', () => { it('preserves runtime strings, styles, and the expanded JSX tree', () => { const strings = readRuntimeStrings() - expect(strings).toHaveLength(476) + expect(strings).toHaveLength(475) expect(hash(strings)).toBe(HEAD_RUNTIME_STRING_SHA256) const jsx = readJsxFacts(readDefinitions()) expect(jsx.host).toHaveLength(126) diff --git a/mobile/src/session/mobile-session-route-types.ts b/mobile/src/session/mobile-session-route-types.ts index 2ef2d58010a..fc8b46db5e3 100644 --- a/mobile/src/session/mobile-session-route-types.ts +++ b/mobile/src/session/mobile-session-route-types.ts @@ -1,3 +1,4 @@ +import type { AgentSessionHandleProvider } from '../../../src/shared/agent-session-provider-handle' import type { DiffComment } from '../../../src/shared/diff-comment-types' import type { MobileBrowserTab } from '../browser/MobileBrowserPane' import type { MobileTerminalTheme } from '../terminal/terminal-webview-contract' @@ -34,7 +35,7 @@ export type MobileSessionTab = id: string title: string sessionId: string - agent: 'codex' + agent: AgentSessionHandleProvider isActive: boolean } | { diff --git a/mobile/src/session/mobile-structured-agent-prompts-grouped.test.ts b/mobile/src/session/mobile-structured-agent-prompts-grouped.test.ts new file mode 100644 index 00000000000..6818d8e92f7 --- /dev/null +++ b/mobile/src/session/mobile-structured-agent-prompts-grouped.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest' +import type { AgentJournalRenderItem } from '../../../src/shared/agent-session-journal-types' +import { + projectStructuredQuestion, + type StructuredQuestionItem +} from './mobile-structured-agent-prompts' + +/** The shape the host emits for a Claude AskUserQuestion carrying more than one question: + * the flat `question`/`options` pair is a placeholder and the real content is in `questions`. */ +function groupedPrompt(): StructuredQuestionItem { + return { + itemId: 'item-1', + revision: 1, + body: { + kind: 'question', + question: '2 grouped questions from Claude', + options: [], + questions: [ + { + id: 'q1', + question: 'Which database?', + multiSelect: false, + options: [{ id: 'q1:choice-1', label: 'Postgres', description: 'Durable server' }], + freeTextQuestionId: 'q1' + }, + { + id: 'q2', + question: 'Which regions?', + multiSelect: true, + options: [{ id: 'q2:choice-1', label: 'us-east' }], + freeTextQuestionId: 'q2' + } + ], + resolution: { state: 'pending' } + } + } as unknown as AgentJournalRenderItem as StructuredQuestionItem +} + +describe('structured question projection for grouped Claude prompts', () => { + it('renders an answerable question instead of the empty placeholder card', () => { + const projected = projectStructuredQuestion(groupedPrompt()) + + expect(projected?.question).not.toBe('2 grouped questions from Claude') + expect(projected?.options).toEqual(['Postgres']) + expect(projected?.optionDescriptions).toEqual(['Durable server']) + expect(projected?.optionTokens.filter(Boolean)).toHaveLength(1) + }) +}) diff --git a/mobile/src/session/mobile-structured-agent-prompts.ts b/mobile/src/session/mobile-structured-agent-prompts.ts index 84cb7033d30..61425597721 100644 --- a/mobile/src/session/mobile-structured-agent-prompts.ts +++ b/mobile/src/session/mobile-structured-agent-prompts.ts @@ -1,6 +1,11 @@ import type { AgentJournalRenderItem } from '../../../src/shared/agent-session-journal-types' import type { MobileChatPermission } from './mobile-native-chat-permission' import type { MobileChatQuestion } from './mobile-native-chat-question' +import { + groupedQuestionPromptKey, + projectGroupedQuestion, + type GroupedQuestionDraft +} from './mobile-structured-grouped-question' export type StructuredApprovalItem = AgentJournalRenderItem & { body: Extract @@ -143,14 +148,24 @@ export function projectStructuredPermission( } export function projectStructuredQuestion( - prompt: StructuredQuestionItem | null + prompt: StructuredQuestionItem | null, + groupedDraft: GroupedQuestionDraft | null = null ): MobileChatQuestion | null { if (prompt?.body.kind !== 'question') { return null } + if (prompt.body.questions) { + return projectGroupedQuestion( + prompt.body.questions, + groupedDraft, + groupedQuestionPromptKey(prompt.itemId, prompt.revision) + ) + } + const optionDescriptions = prompt.body.options.map((option) => option.description) return { question: prompt.body.question, options: prompt.body.options.map((option) => option.label), + ...(optionDescriptions.some(Boolean) ? { optionDescriptions } : {}), multiSelect: false, allowOther: Boolean(prompt.body.freeTextQuestionId), optionTokens: prompt.body.options.map((option) => diff --git a/mobile/src/session/mobile-structured-agent-session-launch.test.ts b/mobile/src/session/mobile-structured-agent-session-launch.test.ts index f575d5ac6d4..8a020d2eea9 100644 --- a/mobile/src/session/mobile-structured-agent-session-launch.test.ts +++ b/mobile/src/session/mobile-structured-agent-session-launch.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it, vi } from 'vitest' import type { RpcClient } from '../transport/rpc-client' import { markRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity' -import { createMobileStructuredCodexSession } from './mobile-structured-agent-session-launch' +import { createMobileStructuredAgentSession } from './mobile-structured-agent-session-launch' function clientReturning( ...responses: unknown[] @@ -36,11 +36,13 @@ const acceptedCreateResult = { } const acceptedCreate = { ok: true, result: acceptedCreateResult } -describe('mobile structured Codex launch', () => { +describe('mobile structured agent-session launch', () => { it('creates through the structured agent-session intent after support is confirmed', async () => { const client = clientReturning({ ok: true, result: { supported: true } }, acceptedCreate) - await expect(createMobileStructuredCodexSession(client, 'workspace-1')).resolves.toMatchObject({ + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'codex') + ).resolves.toMatchObject({ kind: 'created', sessionId: expect.stringMatching(/^codex_[A-Za-z0-9_]{8,128}$/) }) @@ -67,16 +69,94 @@ describe('mobile structured Codex launch', () => { expect(params.envelope.sessionId).toMatch(/^codex_[A-Za-z0-9_]{8,128}$/) }) + it('creates a Claude session through the same envelope, keyed to the claude provider', async () => { + const client = clientReturning( + { ok: true, result: { supported: true } }, + { + ok: true, + result: { + ...acceptedCreateResult, + value: { ...acceptedCreateResult.value, sessionId: 'claude_session_1' } + } + } + ) + + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'claude') + ).resolves.toMatchObject({ kind: 'created', sessionId: 'claude_session_1' }) + expect(client.sendRequest).toHaveBeenNthCalledWith(1, 'agentSession.createSupport', { + worktree: 'id:workspace-1', + agent: 'claude' + }) + const params = client.sendRequest.mock.calls[1]?.[1] as { + envelope: { sessionId: string; payloadFingerprint: string } + agent: string + } + expect(params.agent).toBe('claude') + expect(params.envelope.sessionId).toMatch(/^claude_[A-Za-z0-9_]{8,128}$/) + expect(params.envelope.payloadFingerprint).toMatch(/^[0-9a-f]{64}$/) + }) + + it('names the refusing agent in the failure copy rather than always saying Codex', async () => { + const client = clientReturning( + { ok: true, result: { supported: true } }, + // A definitive refusal is the only path that reaches the failure copy; anything else + // stays unknown and never renders a message. + { ok: false, error: { code: 'method_not_found', message: '' } } + ) + + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'claude') + ).resolves.toEqual({ kind: 'failed', message: 'Could not open Claude chat.' }) + }) + it('reports unsupported without creating a terminal when the structured path is unavailable', async () => { const client = clientReturning({ ok: true, result: { supported: false, reason: 'remote' } }) - await expect(createMobileStructuredCodexSession(client, 'workspace-1')).resolves.toEqual({ + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'codex') + ).resolves.toEqual({ kind: 'unsupported', reason: 'remote' }) expect(client.sendRequest).toHaveBeenCalledTimes(1) }) + it('retries a transient unresolved worktree before deciding structured support', async () => { + vi.useFakeTimers() + const client = clientReturning( + { ok: false, error: { code: 'selector_not_found', message: 'Selector not found' } }, + { ok: true, result: { supported: true } }, + acceptedCreate + ) + + try { + const result = createMobileStructuredAgentSession(client, 'workspace-1', 'claude') + await vi.runAllTimersAsync() + + await expect(result).resolves.toMatchObject({ kind: 'created' }) + expect(client.sendRequest.mock.calls.map(([method]) => method)).toEqual([ + 'agentSession.createSupport', + 'agentSession.createSupport', + 'agentSession.create' + ]) + } finally { + vi.useRealTimers() + } + }) + + it('does not retry a support failure unrelated to worktree resolution', async () => { + const client = clientReturning({ + ok: false, + error: { code: 'runtime_busy', message: 'Runtime busy' } + }) + + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'claude') + ).resolves.toEqual({ kind: 'unsupported' }) + expect(client.sendRequest).toHaveBeenCalledTimes(1) + }) + it('keeps an unknown create outcome distinct so callers do not create a duplicate terminal', async () => { const client = clientReturning({ ok: true, result: { supported: true } }) client.sendRequest.mockImplementationOnce(async () => ({ @@ -85,7 +165,9 @@ describe('mobile structured Codex launch', () => { })) client.sendRequest.mockRejectedValue(markRpcDeliveryUnknown(new Error('response lost'))) - await expect(createMobileStructuredCodexSession(client, 'workspace-1')).resolves.toMatchObject({ + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'codex') + ).resolves.toMatchObject({ kind: 'unknown' }) expect(client.sendRequest.mock.calls.map(([method]) => method)).toEqual([ @@ -105,7 +187,9 @@ describe('mobile structured Codex launch', () => { client.sendRequest.mockRejectedValueOnce(markRpcDeliveryUnknown(new Error('response lost'))) client.sendRequest.mockRejectedValueOnce(new Error('connection interrupted')) - await expect(createMobileStructuredCodexSession(client, 'workspace-1')).resolves.toMatchObject({ + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'codex') + ).resolves.toMatchObject({ kind: 'unknown' }) }) @@ -118,7 +202,9 @@ describe('mobile structured Codex launch', () => { })) client.sendRequest.mockRejectedValue(new Error('internal error after commit')) - await expect(createMobileStructuredCodexSession(client, 'workspace-1')).resolves.toMatchObject({ + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'codex') + ).resolves.toMatchObject({ kind: 'unknown' }) expect(client.sendRequest.mock.calls.map(([method]) => method)).toEqual([ @@ -135,7 +221,9 @@ describe('mobile structured Codex launch', () => { { ok: true, result: { ok: true, value: { sessionId: '' } } } ) - await expect(createMobileStructuredCodexSession(client, 'workspace-1')).resolves.toMatchObject({ + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'codex') + ).resolves.toMatchObject({ kind: 'unknown' }) }) @@ -148,7 +236,9 @@ describe('mobile structured Codex launch', () => { { ok: false, error: { code, message: 'structured create unavailable' } } ) - await expect(createMobileStructuredCodexSession(client, 'workspace-1')).resolves.toEqual({ + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'codex') + ).resolves.toEqual({ kind: 'failed', message: 'structured create unavailable' }) @@ -163,7 +253,9 @@ describe('mobile structured Codex launch', () => { { ok: false, error: { code, message: 'create outcome ambiguous' } } ) - await expect(createMobileStructuredCodexSession(client, 'workspace-1')).resolves.toEqual({ + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'codex') + ).resolves.toEqual({ kind: 'unknown', message: 'create outcome ambiguous' }) @@ -185,7 +277,9 @@ describe('mobile structured Codex launch', () => { } ) - await expect(createMobileStructuredCodexSession(client, 'workspace-1')).resolves.toEqual({ + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'codex') + ).resolves.toEqual({ kind: 'failed', message: 'structured create unavailable' }) @@ -205,7 +299,9 @@ describe('mobile structured Codex launch', () => { } ) - await expect(createMobileStructuredCodexSession(client, 'workspace-1')).resolves.toEqual({ + await expect( + createMobileStructuredAgentSession(client, 'workspace-1', 'codex') + ).resolves.toEqual({ kind: 'unknown', message: 'create outcome ambiguous' }) diff --git a/mobile/src/session/mobile-structured-agent-session-launch.ts b/mobile/src/session/mobile-structured-agent-session-launch.ts index b7eb8289e84..9e26eaab91e 100644 --- a/mobile/src/session/mobile-structured-agent-session-launch.ts +++ b/mobile/src/session/mobile-structured-agent-session-launch.ts @@ -1,93 +1,108 @@ +import type { AgentSessionHandleProvider } from '../../../src/shared/agent-session-provider-handle' import type { AgentSessionAttachResult, AgentSessionMutationResult } from '../../../src/shared/agent-session-wire' import { isDefinitiveAgentSessionCreateRefusal } from '../../../src/shared/agent-session-definitive-refusal' -import { structuredAgentSessionPayloadFingerprint } from '../../../src/shared/structured-agent-session-mutation' +import { + createStructuredAgentSessionId, + structuredAgentSessionCreateParams, + type StructuredAgentSessionCreateParams +} from '../../../src/shared/structured-agent-session-create' +import { TUI_AGENT_DISPLAY_NAMES } from '../../../src/shared/tui-agent-display-names' +import { hasRuntimeRpcErrorCode } from '../../../src/shared/runtime-rpc-error-code' import type { RpcClient } from '../transport/rpc-client' -import { structuredSessionOperationId } from './mobile-structured-agent-session-rpc' +import { structuredSessionRandomUuid } from './mobile-structured-agent-session-rpc' type StructuredCreateSupport = { supported?: boolean reason?: 'agent' | 'remote' | 'wsl' } -export type MobileStructuredCodexLaunchResult = +const SELECTOR_NOT_RESOLVABLE_CODE = 'selector_not_found' +const CREATE_SUPPORT_RETRY_DELAYS_MS: readonly number[] = [50, 150, 300] + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)) +} + +export type MobileStructuredAgentLaunchResult = | { kind: 'created'; sessionId: string } | { kind: 'unsupported'; reason?: StructuredCreateSupport['reason'] } | { kind: 'failed'; message: string } | { kind: 'unknown'; message: string } -type StructuredCreateParams = { - envelope: { - sessionId: string - clientOperationId: string - expectedRuntimeFence: null - payloadFingerprint: string - } +function createParamsFor( + agent: AgentSessionHandleProvider, worktree: string - agent: 'codex' +): StructuredAgentSessionCreateParams { + return structuredAgentSessionCreateParams({ + sessionId: createStructuredAgentSessionId(agent, structuredSessionRandomUuid), + worktree, + agent, + randomUuid: structuredSessionRandomUuid + }) } -function createStructuredCodexSessionId(): string { - return `codex_${createRandomUuid().replaceAll('-', '_')}` -} - -function createRandomUuid(): string { - if (typeof globalThis.crypto?.randomUUID === 'function') { - return globalThis.crypto.randomUUID() - } - return Array.from({ length: 32 }, () => Math.floor(Math.random() * 16).toString(16)).join('') -} - -function createStructuredCodexSessionParams(worktreeId: string): StructuredCreateParams { - const sessionId = createStructuredCodexSessionId() - const worktree = `id:${worktreeId}` - const fields = { worktree, agent: 'codex' as const } - return { - envelope: { - sessionId, - clientOperationId: structuredSessionOperationId(), - expectedRuntimeFence: null, - payloadFingerprint: structuredAgentSessionPayloadFingerprint({ - method: 'agentSession.create', - sessionId, - fields - }) - }, - ...fields - } -} - -function unknownCreateResult(error: unknown): MobileStructuredCodexLaunchResult { +function unknownCreateResult( + agent: AgentSessionHandleProvider, + error: unknown +): MobileStructuredAgentLaunchResult { const message = error instanceof Error ? error.message.trim() : '' - return { - kind: 'unknown', - message: message || 'The Codex chat result could not be confirmed.' - } + return { kind: 'unknown', message: message || unconfirmedMessage(agent) } } -function classifyCreateRefusal(code: string, message: string): MobileStructuredCodexLaunchResult { +function unconfirmedMessage(agent: AgentSessionHandleProvider): string { + return `The ${TUI_AGENT_DISPLAY_NAMES[agent]} chat result could not be confirmed.` +} + +function failedMessage(agent: AgentSessionHandleProvider): string { + return `Could not open ${TUI_AGENT_DISPLAY_NAMES[agent]} chat.` +} + +/** Only a refusal the host names as definitive may become `failed`; anything else keeps the + * outcome unknown so no legacy sibling terminal is created for a session that may exist. */ +function classifyCreateRefusal( + agent: AgentSessionHandleProvider, + code: string, + message: string +): MobileStructuredAgentLaunchResult { if (!isDefinitiveAgentSessionCreateRefusal(code)) { - return unknownCreateResult(new Error(message)) + return unknownCreateResult(agent, new Error(message)) } - return { kind: 'failed', message: message || 'Could not open Codex chat.' } + return { kind: 'failed', message: message || failedMessage(agent) } } -export async function createMobileStructuredCodexSession( +export async function createMobileStructuredAgentSession( client: RpcClient, - worktreeId: string -): Promise { + worktreeId: string, + agent: AgentSessionHandleProvider +): Promise { const worktree = `id:${worktreeId}` let supportResponse - try { - supportResponse = await client.sendRequest('agentSession.createSupport', { - worktree, - agent: 'codex' - }) - } catch { - // A support probe has no side effect; an unavailable probe safely degrades to terminal chat. - return { kind: 'unsupported' } + for (let attempt = 0; ; attempt += 1) { + try { + supportResponse = await client.sendRequest('agentSession.createSupport', { worktree, agent }) + } catch (error) { + const retryDelayMs = CREATE_SUPPORT_RETRY_DELAYS_MS[attempt] + if ( + retryDelayMs === undefined || + !hasRuntimeRpcErrorCode(error, SELECTOR_NOT_RESOLVABLE_CODE) + ) { + return { kind: 'unsupported' } + } + await delay(retryDelayMs) + continue + } + const retryDelayMs = CREATE_SUPPORT_RETRY_DELAYS_MS[attempt] + if ( + retryDelayMs !== undefined && + hasRuntimeRpcErrorCode(supportResponse, SELECTOR_NOT_RESOLVABLE_CODE) + ) { + await delay(retryDelayMs) + continue + } + break } if ( !supportResponse || @@ -102,7 +117,7 @@ export async function createMobileStructuredCodexSession( return { kind: 'unsupported', reason: support?.reason } } - const params = createStructuredCodexSessionParams(worktreeId) + const params = createParamsFor(agent, worktree) let response try { response = await client.sendRequest('agentSession.create', params, { @@ -118,12 +133,12 @@ export async function createMobileStructuredCodexSession( }) } catch (retryError) { // A second transport error cannot disprove the first attempt committed. - return unknownCreateResult(retryError) + return unknownCreateResult(agent, retryError) } } if (!response || typeof response !== 'object' || typeof response.ok !== 'boolean') { - return unknownCreateResult(new Error('The Codex chat result could not be confirmed.')) + return unknownCreateResult(agent, new Error(unconfirmedMessage(agent))) } if (!response.ok) { if ( @@ -131,13 +146,13 @@ export async function createMobileStructuredCodexSession( typeof response.error !== 'object' || typeof response.error.code !== 'string' ) { - return unknownCreateResult(new Error('The Codex chat result could not be confirmed.')) + return unknownCreateResult(agent, new Error(unconfirmedMessage(agent))) } - return classifyCreateRefusal(response.error.code, response.error.message) + return classifyCreateRefusal(agent, response.error.code, response.error.message) } const result = response.result as AgentSessionMutationResult if (!result || typeof result !== 'object' || typeof result.ok !== 'boolean') { - return unknownCreateResult(new Error('The Codex chat result could not be confirmed.')) + return unknownCreateResult(agent, new Error(unconfirmedMessage(agent))) } if (!result.ok) { if ( @@ -145,16 +160,16 @@ export async function createMobileStructuredCodexSession( typeof result.refusal !== 'object' || typeof result.refusal.code !== 'string' ) { - return unknownCreateResult(new Error('The Codex chat result could not be confirmed.')) + return unknownCreateResult(agent, new Error(unconfirmedMessage(agent))) } - return classifyCreateRefusal(result.refusal.code, result.refusal.message) + return classifyCreateRefusal(agent, result.refusal.code, result.refusal.message) } if ( !result.value || typeof result.value.sessionId !== 'string' || !result.value.sessionId.trim() ) { - return unknownCreateResult(new Error('The Codex chat result could not be confirmed.')) + return unknownCreateResult(agent, new Error(unconfirmedMessage(agent))) } return { kind: 'created', sessionId: result.value.sessionId } } diff --git a/mobile/src/session/mobile-structured-agent-session-rpc.ts b/mobile/src/session/mobile-structured-agent-session-rpc.ts index a602122978e..bd5dd80ded3 100644 --- a/mobile/src/session/mobile-structured-agent-session-rpc.ts +++ b/mobile/src/session/mobile-structured-agent-session-rpc.ts @@ -49,16 +49,16 @@ export async function callAgentSession( return response.result as TResult } +/** React Native has no guaranteed `crypto.randomUUID`; the fallback keeps the same + * 32-hex entropy shape the durable id and fingerprint helpers validate. */ +export function structuredSessionRandomUuid(): string { + return typeof globalThis.crypto?.randomUUID === 'function' + ? globalThis.crypto.randomUUID() + : Array.from({ length: 32 }, () => Math.floor(Math.random() * 16).toString(16)).join('') +} + export function structuredSessionOperationId(): string { - const randomUuid = - typeof globalThis.crypto?.randomUUID === 'function' - ? () => globalThis.crypto.randomUUID() - : () => { - return Array.from({ length: 32 }, () => Math.floor(Math.random() * 16).toString(16)).join( - '' - ) - } - return createStructuredAgentSessionOperationId(randomUuid) + return createStructuredAgentSessionOperationId(structuredSessionRandomUuid) } /** diff --git a/mobile/src/session/mobile-structured-grouped-question.test.ts b/mobile/src/session/mobile-structured-grouped-question.test.ts new file mode 100644 index 00000000000..f45c922c6cb --- /dev/null +++ b/mobile/src/session/mobile-structured-grouped-question.test.ts @@ -0,0 +1,256 @@ +import { describe, expect, it } from 'vitest' +import type { AgentJournalQuestion } from '../../../src/shared/agent-session-journal-types' +import { decodeAgentSessionQuestionAnswers } from '../../../src/shared/agent-session-question-answer' +import { + formatQuestionAnswer, + formatQuestionFreeTextAnswer, + mobileChatQuestionKey +} from './mobile-native-chat-question' +import { + advanceGroupedQuestion, + groupedQuestionPromptKey, + projectGroupedQuestion, + type GroupedQuestionDraft +} from './mobile-structured-grouped-question' + +const PROMPT_KEY = groupedQuestionPromptKey('item-1', 3) + +function question(overrides: Partial = {}): AgentJournalQuestion { + return { + id: 'q1', + question: 'Which database?', + multiSelect: false, + options: [ + { id: 'q1:choice-1', label: 'Postgres' }, + { id: 'q1:choice-2', label: 'SQLite' } + ], + freeTextQuestionId: 'q1', + ...overrides + } +} + +const SECOND = question({ + id: 'q2', + question: 'Which regions?', + multiSelect: true, + options: [ + { id: 'q2:choice-1', label: 'us-east' }, + { id: 'q2:choice-2', label: 'eu-west' } + ], + freeTextQuestionId: 'q2' +}) + +/** Mirrors what the question card sends back for a single-select tap. */ +function tapOption(projected: NonNullable>, at: number) { + return projected.optionTokens[at] ?? '' +} + +describe('mobile structured grouped questions', () => { + it('projects the first question with real options instead of the empty flat shape', () => { + const projected = projectGroupedQuestion([question(), SECOND], null, PROMPT_KEY) + + expect(projected).toMatchObject({ + question: 'Which database? (1 of 2)', + options: ['Postgres', 'SQLite'], + multiSelect: false, + allowOther: true + }) + expect(projected?.optionTokens.every((token) => Boolean(token))).toBe(true) + expect(projected?.freeTextToken).toBeTruthy() + }) + + it('steps to the next question once the first is answered, without sending anything', () => { + const questions = [question(), SECOND] + const first = projectGroupedQuestion(questions, null, PROMPT_KEY)! + + const advance = advanceGroupedQuestion({ + response: tapOption(first, 0), + questions, + draft: null, + promptKey: PROMPT_KEY + }) + + expect(advance).toEqual({ + kind: 'advance', + draft: { promptKey: PROMPT_KEY, answers: [{ questionId: 'q1', optionIds: ['q1:choice-1'] }] } + }) + const second = projectGroupedQuestion( + questions, + advance!.kind === 'advance' ? advance.draft : null, + PROMPT_KEY + ) + expect(second).toMatchObject({ question: 'Which regions? (2 of 2)', multiSelect: true }) + }) + + it('submits the whole group as one encoded answer on the last step', () => { + const questions = [question(), SECOND] + const draft: GroupedQuestionDraft = { + promptKey: PROMPT_KEY, + answers: [{ questionId: 'q1', optionIds: ['q1:choice-1'] }] + } + const second = projectGroupedQuestion(questions, draft, PROMPT_KEY)! + + const result = advanceGroupedQuestion({ + // Multi-select joins its selected option tokens the way the card does. + response: formatQuestionAnswer(second, ['us-east', 'eu-west']), + questions, + draft, + promptKey: PROMPT_KEY + }) + + expect(result?.kind).toBe('submit') + expect( + decodeAgentSessionQuestionAnswers(result?.kind === 'submit' ? result.optionId : '') + ).toEqual([ + { questionId: 'q1', optionIds: ['q1:choice-1'] }, + { questionId: 'q2', optionIds: ['q2:choice-1', 'q2:choice-2'] } + ]) + }) + + it('carries a free-text answer as `other` for the question it was typed against', () => { + const questions = [question()] + const only = projectGroupedQuestion(questions, null, PROMPT_KEY)! + + const result = advanceGroupedQuestion({ + response: formatQuestionFreeTextAnswer(only, ' DuckDB '), + questions, + draft: null, + promptKey: PROMPT_KEY + }) + + expect( + decodeAgentSessionQuestionAnswers(result?.kind === 'submit' ? result.optionId : '') + ).toEqual([{ questionId: 'q1', optionIds: [], other: 'DuckDB' }]) + }) + + it('keeps selected options and other text for grouped multi-select answers', () => { + const questions = [SECOND] + const only = projectGroupedQuestion(questions, null, PROMPT_KEY)! + + const result = advanceGroupedQuestion({ + response: `${tapOption(only, 0)}, ${formatQuestionFreeTextAnswer(only, 'ap-south')}`, + questions, + draft: null, + promptKey: PROMPT_KEY + }) + + expect( + decodeAgentSessionQuestionAnswers(result?.kind === 'submit' ? result.optionId : '') + ).toEqual([{ questionId: 'q2', optionIds: ['q2:choice-1'], other: 'ap-south' }]) + }) + + it('gives each step a distinct card key so a selection cannot carry into the next question', () => { + // The view keys MobileNativeChatQuestion by this value; an identical key would reuse the + // mounted card and submit step 1's checkboxes as step 2's answer. Claude can legitimately ask + // the SAME text twice in one group (once per file, say), so identical wording must still key + // apart on the question id and step counter. + const questions = [ + question({ id: 'q1', question: 'Approve?' }), + question({ id: 'q2', question: 'Approve?' }) + ] + const first = projectGroupedQuestion(questions, null, PROMPT_KEY)! + const second = projectGroupedQuestion( + questions, + { promptKey: PROMPT_KEY, answers: [{ questionId: 'q1', optionIds: ['q1:choice-1'] }] }, + PROMPT_KEY + )! + + expect(first.question).toBe('Approve? (1 of 2)') + expect(second.question).toBe('Approve? (2 of 2)') + expect(mobileChatQuestionKey(first)).not.toBe(mobileChatQuestionKey(second)) + }) + + it('discards a draft collected against a superseded prompt revision', () => { + const questions = [question(), SECOND] + const stale: GroupedQuestionDraft = { + promptKey: groupedQuestionPromptKey('item-1', 2), + answers: [{ questionId: 'q1', optionIds: ['q1:choice-1'] }] + } + + expect(projectGroupedQuestion(questions, stale, PROMPT_KEY)).toMatchObject({ + question: 'Which database? (1 of 2)' + }) + }) + + it('refuses a response that does not answer the current step', () => { + const questions = [question(), SECOND] + + expect( + advanceGroupedQuestion({ + response: 'Postgres', + questions, + draft: null, + promptKey: PROMPT_KEY + }) + ).toBeNull() + }) + + it('refuses an option token rendered for a superseded prompt revision', () => { + const stale = projectGroupedQuestion([question()], null, groupedQuestionPromptKey('item-1', 2))! + + expect( + advanceGroupedQuestion({ + response: tapOption(stale, 0), + questions: [question()], + draft: null, + promptKey: PROMPT_KEY + }) + ).toBeNull() + }) + + it('refuses free text rendered for a superseded prompt revision', () => { + const stale = projectGroupedQuestion([question()], null, groupedQuestionPromptKey('item-1', 2))! + + expect( + advanceGroupedQuestion({ + response: formatQuestionFreeTextAnswer(stale, 'stale answer'), + questions: [question()], + draft: null, + promptKey: PROMPT_KEY + }) + ).toBeNull() + }) + + it('rejects a multi-select response when one selected token is malformed', () => { + const questions = [SECOND] + const only = projectGroupedQuestion(questions, null, PROMPT_KEY)! + + expect( + advanceGroupedQuestion({ + response: `${tapOption(only, 0)}, not-a-grouped-token`, + questions, + draft: null, + promptKey: PROMPT_KEY + }) + ).toBeNull() + }) + + it('rejects a multi-select response when one selected token belongs to another prompt', () => { + const questions = [SECOND] + const current = projectGroupedQuestion(questions, null, PROMPT_KEY)! + const stale = projectGroupedQuestion(questions, null, groupedQuestionPromptKey('item-1', 2))! + + expect( + advanceGroupedQuestion({ + response: `${tapOption(current, 0)}, ${tapOption(stale, 1)}`, + questions, + draft: null, + promptKey: PROMPT_KEY + }) + ).toBeNull() + }) + + it('refuses an empty multi-select rather than sending a group the host would reject', () => { + const questions = [SECOND] + const only = projectGroupedQuestion(questions, null, PROMPT_KEY)! + + expect( + advanceGroupedQuestion({ + response: formatQuestionAnswer(only, []), + questions, + draft: null, + promptKey: PROMPT_KEY + }) + ).toBeNull() + }) +}) diff --git a/mobile/src/session/mobile-structured-grouped-question.ts b/mobile/src/session/mobile-structured-grouped-question.ts new file mode 100644 index 00000000000..17a716cd032 --- /dev/null +++ b/mobile/src/session/mobile-structured-grouped-question.ts @@ -0,0 +1,221 @@ +import type { AgentJournalQuestion } from '../../../src/shared/agent-session-journal-types' +import { + encodeAgentSessionQuestionAnswers, + isValidAgentSessionQuestionAnswers, + type AgentSessionQuestionAnswer +} from '../../../src/shared/agent-session-question-answer' +import type { MobileChatQuestion } from './mobile-native-chat-question' + +/** + * Claude's AskUserQuestion can carry several questions, or one multi-select question, in a single + * prompt. The host then leaves the flat `question.options` EMPTY and puts the real content in + * `questions`, so a client that reads only the flat shape renders an unanswerable card and the turn + * stalls. The phone has room for one question at a time, so the group is answered as steps and + * submitted once — the host accepts the whole group as one encoded option id. + */ +export type GroupedQuestionDraft = { + /** Identifies the exact prompt revision these answers belong to; a revised prompt discards them. */ + promptKey: string + answers: AgentSessionQuestionAnswer[] +} + +export type GroupedQuestionAdvance = + | { kind: 'advance'; draft: GroupedQuestionDraft } + | { kind: 'submit'; optionId: string } + +const GROUPED_TOKEN_PREFIX = 'structured-grouped-question:' + +type GroupedTokenPayload = + | { kind: 'option'; promptKey: string; questionId: string; optionId: string } + | { kind: 'free-text'; promptKey: string; questionId: string } + +export function groupedQuestionPromptKey(itemId: string, revision: number): string { + return `${itemId}:${revision}` +} + +function encodeGroupedToken(payload: GroupedTokenPayload): string { + return `${GROUPED_TOKEN_PREFIX}${encodeURIComponent(JSON.stringify(payload))}` +} + +function decodeGroupedToken(value: string): GroupedTokenPayload | null { + if (!value.startsWith(GROUPED_TOKEN_PREFIX)) { + return null + } + try { + const decoded = JSON.parse( + decodeURIComponent(value.slice(GROUPED_TOKEN_PREFIX.length)) + ) as Record + if (typeof decoded.promptKey !== 'string' || typeof decoded.questionId !== 'string') { + return null + } + if (decoded.kind === 'option' && typeof decoded.optionId === 'string') { + return { + kind: 'option', + promptKey: decoded.promptKey, + questionId: decoded.questionId, + optionId: decoded.optionId + } + } + if (decoded.kind === 'free-text') { + return { kind: 'free-text', promptKey: decoded.promptKey, questionId: decoded.questionId } + } + } catch { + return null + } + return null +} + +function decodeGroupedFreeTextAnswer(value: string): { + promptKey: string + questionId: string + answer: string +} | null { + if (!value.startsWith(GROUPED_TOKEN_PREFIX)) { + return null + } + // The payload is percent-encoded, so the first `:` after the prefix is the answer separator. + const separator = value.indexOf(':', GROUPED_TOKEN_PREFIX.length) + if (separator === -1) { + return null + } + const payload = decodeGroupedToken(value.slice(0, separator)) + if (payload?.kind !== 'free-text') { + return null + } + try { + return { + promptKey: payload.promptKey, + questionId: payload.questionId, + answer: decodeURIComponent(value.slice(separator + 1)) + } + } catch { + return null + } +} + +/** Answers already collected for this exact prompt revision; a stale draft counts as none. */ +function answersFor( + draft: GroupedQuestionDraft | null, + promptKey: string +): AgentSessionQuestionAnswer[] { + return draft && draft.promptKey === promptKey ? draft.answers : [] +} + +/** The step to show now, or null once every question has an answer. */ +export function projectGroupedQuestion( + questions: readonly AgentJournalQuestion[], + draft: GroupedQuestionDraft | null, + promptKey: string +): MobileChatQuestion | null { + const answered = answersFor(draft, promptKey).length + const question = questions[answered] + if (!question) { + return null + } + const heading = question.header ? `${question.header}: ${question.question}` : question.question + const optionDescriptions = question.options.map((option) => option.description) + return { + question: + questions.length > 1 ? `${heading} (${answered + 1} of ${questions.length})` : heading, + options: question.options.map((option) => option.label), + ...(optionDescriptions.some(Boolean) ? { optionDescriptions } : {}), + multiSelect: question.multiSelect, + allowOther: Boolean(question.freeTextQuestionId), + optionTokens: question.options.map((option) => + encodeGroupedToken({ + kind: 'option', + promptKey, + questionId: question.id, + optionId: option.id + }) + ), + ...(question.freeTextQuestionId + ? { + freeTextToken: encodeGroupedToken({ + kind: 'free-text', + promptKey, + questionId: question.id + }) + } + : {}) + } +} + +/** Read one step's answer out of what the question card sent back. */ +function answerFromResponse( + response: string, + question: AgentJournalQuestion, + promptKey: string +): AgentSessionQuestionAnswer | null { + // Multi-select submits comma-joined parts; tokens and free text are encoded, so the separator is stable. + const optionIds: string[] = [] + let other: string | undefined + for (const part of response.split(', ')) { + const trimmed = part.trim() + const freeText = decodeGroupedFreeTextAnswer(trimmed) + if (freeText) { + const answer = freeText.answer.trim() + if ( + freeText.promptKey !== promptKey || + freeText.questionId !== question.id || + answer.length === 0 || + other !== undefined + ) { + return null + } + other = answer + continue + } + + const payload = decodeGroupedToken(trimmed) + if ( + payload?.kind !== 'option' || + payload.promptKey !== promptKey || + payload.questionId !== question.id + ) { + return null + } + optionIds.push(payload.optionId) + } + const offered = new Set(question.options.map((option) => option.id)) + if (optionIds.some((optionId) => !offered.has(optionId))) { + return null + } + if (other && !question.freeTextQuestionId) { + return null + } + const answerCount = optionIds.length + (other ? 1 : 0) + if (answerCount === 0 || (!question.multiSelect && answerCount !== 1)) { + return null + } + return { questionId: question.id, optionIds, ...(other ? { other } : {}) } +} + +/** + * Fold one answer into the draft. Returns `advance` while questions remain and `submit` with the + * encoded group once the last one lands; null when the response does not answer this prompt step. + */ +export function advanceGroupedQuestion(args: { + response: string + questions: readonly AgentJournalQuestion[] + draft: GroupedQuestionDraft | null + promptKey: string +}): GroupedQuestionAdvance | null { + const collected = answersFor(args.draft, args.promptKey) + const question = args.questions[collected.length] + if (!question) { + return null + } + const answer = answerFromResponse(args.response, question, args.promptKey) + if (!answer) { + return null + } + const answers = [...collected, answer] + if (answers.length < args.questions.length) { + return { kind: 'advance', draft: { promptKey: args.promptKey, answers } } + } + // Never send a group the host would refuse — the user would see a silent failure with no way back. + return isValidAgentSessionQuestionAnswers(args.questions, answers) + ? { kind: 'submit', optionId: encodeAgentSessionQuestionAnswers(answers) } + : null +} diff --git a/mobile/src/session/use-mobile-session-terminal-create-actions.ts b/mobile/src/session/use-mobile-session-terminal-create-actions.ts index 672919e4635..82271eee60f 100644 --- a/mobile/src/session/use-mobile-session-terminal-create-actions.ts +++ b/mobile/src/session/use-mobile-session-terminal-create-actions.ts @@ -4,12 +4,13 @@ import { terminalRecordsEqual } from './mobile-terminal-records' import type { MobileNewTabAgentOption } from './mobile-new-tab-agent-options' import type { Terminal, TerminalCreateResult } from './mobile-session-route-types' import type { MobileSessionAttachmentsModel } from './use-mobile-session-attachments' +import { isAgentSessionHandleProvider } from '../../../src/shared/agent-session-provider-handle' import { buildTerminalSendParams, TERMINAL_INPUT_SEND_OPTIONS } from '../terminal/terminal-send-request' import { createSessionQuickCommandLauncher } from './session-quick-command-launch' -import { createMobileStructuredCodexSession } from './mobile-structured-agent-session-launch' +import { createMobileStructuredAgentSession } from './mobile-structured-agent-session-launch' export function useMobileSessionTerminalCreateActions(scope: MobileSessionAttachmentsModel) { const { @@ -79,9 +80,9 @@ export function useMobileSessionTerminalCreateActions(scope: MobileSessionAttach if (!client) { return } - // Bare Codex launches follow structured support; prompted launches keep their startup semantics. - if (agent === 'codex' && options === undefined) { - const structured = await createMobileStructuredCodexSession(client, worktreeId) + // Bare structured-provider launches follow host createSupport; prompted launches keep their startup semantics. + if (isAgentSessionHandleProvider(agent) && options === undefined) { + const structured = await createMobileStructuredAgentSession(client, worktreeId, agent) if (structured.kind === 'created') { const previous = activeHandleRef.current if (previous) { diff --git a/mobile/src/session/use-mobile-structured-agent-session.ts b/mobile/src/session/use-mobile-structured-agent-session.ts index d9cabf1f2d0..4cf5adea98f 100644 --- a/mobile/src/session/use-mobile-structured-agent-session.ts +++ b/mobile/src/session/use-mobile-structured-agent-session.ts @@ -1,7 +1,6 @@ import { useCallback, useEffect, useMemo, useRef } from 'react' import type { AgentSessionCancelResult, - AgentSessionPromptResult, AgentSessionSendResult } from '../../../src/shared/agent-session-wire' import type { @@ -21,9 +20,7 @@ import { pendingStructuredApproval, pendingStructuredQuestion, projectStructuredPermission, - projectStructuredQuestion, - structuredApprovalResponseTarget, - structuredQuestionResponseTarget + projectStructuredQuestion } from './mobile-structured-agent-prompts' import { requestStructuredAgentSessionMutation, @@ -36,6 +33,7 @@ import type { MobileChatPermission } from './mobile-native-chat-permission' import type { MobileChatQuestion } from './mobile-native-chat-question' import type { MobileNativeChatSession } from './use-mobile-native-chat-session' import { useMobileStructuredAgentState } from './use-mobile-structured-agent-state' +import { useMobileStructuredPromptResponses } from './use-mobile-structured-prompt-responses' import { useMobileStructuredAgentOptions } from './use-mobile-structured-agent-options' type StructuredMobileAttachment = StructuredAgentSessionAttachment & { id?: string } @@ -196,51 +194,12 @@ export function useMobileStructuredAgentSession(args: { [client, enabled, onSendError, sessionId, sessionKey] ) - const respondPermission = useCallback( - async (optionId: string): Promise => { - const target = structuredApprovalResponseTarget( - optionId, - stateRef.current.items.find(pendingStructuredApproval) ?? null - ) - if (!target) { - return false - } - const result = await mutate( - 'agentSession.respondToApproval', - 'agentSession.respondTo:approval', - target - ) - if (result.status === 'unknown') { - onSendError('Response unconfirmed — check chat before retrying') - return false - } - return result.status === 'accepted' - }, - [mutate, onSendError] - ) - - const respondQuestion = useCallback( - async (answer: string): Promise => { - const target = structuredQuestionResponseTarget( - answer, - stateRef.current.items.find(pendingStructuredQuestion) ?? null - ) - if (!target) { - return false - } - const result = await mutate( - 'agentSession.respondToQuestion', - 'agentSession.respondTo:question', - target - ) - if (result.status === 'unknown') { - onSendError('Answer unconfirmed — check chat before retrying') - return false - } - return result.status === 'accepted' - }, - [mutate, onSendError] - ) + const { groupedDraft, respondPermission, respondQuestion } = useMobileStructuredPromptResponses({ + stateRef, + sessionKey, + mutate, + onSendError + }) const cancel = useCallback(() => { const current = stateRef.current @@ -303,7 +262,7 @@ export function useMobileStructuredAgentSession(args: { sendWithOutcome, cancel, permission: projectStructuredPermission(approvalPrompt), - question: projectStructuredQuestion(questionPrompt), + question: projectStructuredQuestion(questionPrompt, groupedDraft), optionSnapshot, optionSurface, pendingOptionId, diff --git a/mobile/src/session/use-mobile-structured-prompt-responses.test.tsx b/mobile/src/session/use-mobile-structured-prompt-responses.test.tsx new file mode 100644 index 00000000000..05a2b7fc380 --- /dev/null +++ b/mobile/src/session/use-mobile-structured-prompt-responses.test.tsx @@ -0,0 +1,175 @@ +import { createElement, useRef } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { AgentSessionPromptResult } from '../../../src/shared/agent-session-wire' +import type { AgentJournalRenderItem } from '../../../src/shared/agent-session-journal-types' +import { + EMPTY_STRUCTURED_AGENT_SESSION, + type StructuredAgentSessionState +} from '../../../src/shared/structured-agent-session-reducer' +import { projectStructuredQuestion } from './mobile-structured-agent-prompts' +import type { + StructuredAgentSessionMutate, + StructuredAgentSessionMutationResult +} from './mobile-structured-agent-session-rpc' +import { groupedQuestionPromptKey } from './mobile-structured-grouped-question' +import { useMobileStructuredPromptResponses } from './use-mobile-structured-prompt-responses' + +type PromptResponses = ReturnType + +let currentHook: PromptResponses | null = null +let renderer: ReactTestRenderer | null = null + +function groupedPrompt(itemId: string, revision: number): AgentJournalRenderItem { + return { + itemId, + revision, + sequence: 1, + observedAt: 1, + body: { + kind: 'question', + question: '2 grouped questions from Claude', + options: [], + questions: [ + { + id: 'q1', + question: 'First?', + multiSelect: false, + options: [ + { id: 'q1:choice-1', label: 'One' }, + { id: 'q1:choice-2', label: 'Another one' } + ] + }, + { + id: 'q2', + question: 'Second?', + multiSelect: false, + options: [ + { id: 'q2:choice-1', label: 'Two' }, + { id: 'q2:choice-2', label: 'Another two' } + ] + } + ], + resolution: { state: 'pending', selectedOptionId: null, resolvedBy: null, resolvedAt: null } + } + } +} + +function sessionState(prompt: AgentJournalRenderItem): StructuredAgentSessionState { + return { ...EMPTY_STRUCTURED_AGENT_SESSION, status: 'ready', items: [prompt] } +} + +function projectedResponse(prompt: AgentJournalRenderItem, draft: PromptResponses['groupedDraft']) { + const projected = projectStructuredQuestion(prompt, draft) + const response = projected?.optionTokens[0] + if (!response) { + throw new Error('Grouped question did not project an option response') + } + return response +} + +function Probe(props: { + sessionKey: string + state: StructuredAgentSessionState + mutate: StructuredAgentSessionMutate +}) { + const stateRef = useRef(props.state) + stateRef.current = props.state + currentHook = useMobileStructuredPromptResponses({ + stateRef, + sessionKey: props.sessionKey, + mutate: props.mutate, + onSendError: vi.fn() + }) + return null +} + +function hook(): PromptResponses { + if (!currentHook) { + throw new Error('Hook probe is not mounted') + } + return currentHook +} + +afterEach(() => { + act(() => renderer?.unmount()) + currentHook = null + renderer = null +}) + +describe('useMobileStructuredPromptResponses', () => { + it.each([ + ['another session', 'session-b', groupedPrompt('item-b', 1)], + ['a newer prompt revision', 'session-a', groupedPrompt('item-a', 2)] + ])( + 'does not let a completed grouped response clear %s draft', + async (_, nextSession, nextPrompt) => { + const firstPrompt = groupedPrompt('item-a', 1) + let resolveMutation!: ( + value: StructuredAgentSessionMutationResult + ) => void + const pendingMutation = new Promise< + StructuredAgentSessionMutationResult + >((resolve) => { + resolveMutation = resolve + }) + const mutate = vi.fn(() => pendingMutation) as unknown as StructuredAgentSessionMutate + + act(() => { + renderer = create( + createElement(Probe, { + sessionKey: 'session-a', + state: sessionState(firstPrompt), + mutate + }) + ) + }) + await act(async () => { + await hook().respondQuestion(projectedResponse(firstPrompt, null)) + }) + let firstSubmission!: Promise + act(() => { + firstSubmission = hook().respondQuestion( + projectedResponse(firstPrompt, hook().groupedDraft) + ) + }) + + act(() => { + renderer?.update( + createElement(Probe, { + sessionKey: nextSession, + state: sessionState(nextPrompt), + mutate + }) + ) + }) + await act(async () => { + await hook().respondQuestion(projectedResponse(nextPrompt, null)) + }) + expect(hook().groupedDraft?.answers).toHaveLength(1) + + await act(async () => { + resolveMutation({ + status: 'accepted', + value: { + itemId: firstPrompt.itemId, + revision: firstPrompt.revision, + resolution: { + state: 'resolved', + selectedOptionId: 'q2:choice-1', + resolvedBy: 'mobile', + resolvedAt: 2 + } + }, + sameFence: true + }) + await firstSubmission + }) + + expect(hook().groupedDraft?.promptKey).toBe( + groupedQuestionPromptKey(nextPrompt.itemId, nextPrompt.revision) + ) + expect(hook().groupedDraft?.answers).toHaveLength(1) + } + ) +}) diff --git a/mobile/src/session/use-mobile-structured-prompt-responses.ts b/mobile/src/session/use-mobile-structured-prompt-responses.ts new file mode 100644 index 00000000000..8340b7edee8 --- /dev/null +++ b/mobile/src/session/use-mobile-structured-prompt-responses.ts @@ -0,0 +1,121 @@ +import { useCallback, useState } from 'react' +import type { AgentSessionPromptResult } from '../../../src/shared/agent-session-wire' +import type { StructuredAgentSessionState } from '../../../src/shared/structured-agent-session-reducer' +import { + pendingStructuredApproval, + pendingStructuredQuestion, + structuredApprovalResponseTarget, + structuredQuestionResponseTarget +} from './mobile-structured-agent-prompts' +import type { StructuredAgentSessionMutate } from './mobile-structured-agent-session-rpc' +import { + advanceGroupedQuestion, + groupedQuestionPromptKey, + type GroupedQuestionDraft +} from './mobile-structured-grouped-question' + +/** + * Answering the two durable prompt kinds. Kept beside the session hook rather than inside it + * because grouped questions carry their own multi-step draft, which is state the rest of the + * session does not touch. + */ +export function useMobileStructuredPromptResponses(args: { + stateRef: { readonly current: StructuredAgentSessionState } + sessionKey: string + mutate: StructuredAgentSessionMutate + onSendError: (message: string) => void +}): { + groupedDraft: GroupedQuestionDraft | null + respondPermission: (optionId: string) => Promise + respondQuestion: (answer: string) => Promise +} { + const { mutate, onSendError, sessionKey, stateRef } = args + // Partially answered grouped question, held only until its last step is submitted. The session it + // was collected in is stored with it and checked on read, so switching sessions drops the draft + // without an effect that would render the stale one for a frame first. + const [collected, setCollected] = useState<{ + sessionKey: string + draft: GroupedQuestionDraft + } | null>(null) + const groupedDraft = collected?.sessionKey === sessionKey ? collected.draft : null + + const respondPermission = useCallback( + async (optionId: string): Promise => { + const target = structuredApprovalResponseTarget( + optionId, + stateRef.current.items.find(pendingStructuredApproval) ?? null + ) + if (!target) { + return false + } + const result = await mutate( + 'agentSession.respondToApproval', + 'agentSession.respondTo:approval', + target + ) + if (result.status === 'unknown') { + onSendError('Response unconfirmed — check chat before retrying') + return false + } + return result.status === 'accepted' + }, + [mutate, onSendError, stateRef] + ) + + const respondQuestion = useCallback( + async (answer: string): Promise => { + const prompt = stateRef.current.items.find(pendingStructuredQuestion) ?? null + if (prompt?.body.questions) { + const promptKey = groupedQuestionPromptKey(prompt.itemId, prompt.revision) + const grouped = advanceGroupedQuestion({ + response: answer, + questions: prompt.body.questions, + draft: groupedDraft, + promptKey + }) + if (!grouped) { + return false + } + if (grouped.kind === 'advance') { + setCollected({ sessionKey, draft: grouped.draft }) + return true + } + const result = await mutate( + 'agentSession.respondToQuestion', + 'agentSession.respondTo:question', + { itemId: prompt.itemId, expectedRevision: prompt.revision, optionId: grouped.optionId } + ) + if (result.status !== 'rejected') { + // The group left the phone; a retry must start from the first question, not a stale tail. + setCollected((current) => + current?.sessionKey === sessionKey && current.draft.promptKey === promptKey + ? null + : current + ) + } + if (result.status === 'unknown') { + onSendError('Answer unconfirmed — check chat before retrying') + return false + } + return result.status === 'accepted' + } + const target = structuredQuestionResponseTarget(answer, prompt) + if (!target) { + return false + } + const result = await mutate( + 'agentSession.respondToQuestion', + 'agentSession.respondTo:question', + target + ) + if (result.status === 'unknown') { + onSendError('Answer unconfirmed — check chat before retrying') + return false + } + return result.status === 'accepted' + }, + [groupedDraft, mutate, onSendError, sessionKey, stateRef] + ) + + return { groupedDraft, respondPermission, respondQuestion } +} diff --git a/mobile/src/transport/mobile-runtime-client-capabilities.test.ts b/mobile/src/transport/mobile-runtime-client-capabilities.test.ts new file mode 100644 index 00000000000..7a9b2d841ce --- /dev/null +++ b/mobile/src/transport/mobile-runtime-client-capabilities.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from 'vitest' +import { + CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, + STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY, + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY +} from '../../../src/shared/protocol-version' +import { MOBILE_RUNTIME_CLIENT_CAPABILITIES } from './mobile-runtime-client-capabilities' + +/** Mirrors the host's `parseRuntimeClientCapabilities`, which returns an EMPTY list — silently + * dropping every capability, not just the excess — when the array is longer than this or any + * entry is longer than 128 chars. Growing past it would look exactly like an old client. */ +const HOST_CAPABILITY_LIMIT = 64 +const HOST_CAPABILITY_NAME_LIMIT = 128 + +describe('mobile runtime client capabilities', () => { + it('advertises structured agent sessions including the Claude lane', () => { + expect(MOBILE_RUNTIME_CLIENT_CAPABILITIES).toEqual( + expect.arrayContaining([ + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, + STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY, + CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY + ]) + ) + }) + + it('stays inside the bounds the host parses, which fail closed to no capabilities at all', () => { + expect(MOBILE_RUNTIME_CLIENT_CAPABILITIES.length).toBeLessThanOrEqual(HOST_CAPABILITY_LIMIT) + for (const capability of MOBILE_RUNTIME_CLIENT_CAPABILITIES) { + expect(capability.length).toBeGreaterThan(0) + expect(capability.length).toBeLessThanOrEqual(HOST_CAPABILITY_NAME_LIMIT) + } + }) + + it('advertises each capability once so duplicates cannot consume the budget', () => { + expect(new Set(MOBILE_RUNTIME_CLIENT_CAPABILITIES).size).toBe( + MOBILE_RUNTIME_CLIENT_CAPABILITIES.length + ) + }) +}) diff --git a/mobile/src/transport/mobile-runtime-client-capabilities.ts b/mobile/src/transport/mobile-runtime-client-capabilities.ts index 5b3dc977240..29a9e93b527 100644 --- a/mobile/src/transport/mobile-runtime-client-capabilities.ts +++ b/mobile/src/transport/mobile-runtime-client-capabilities.ts @@ -1,4 +1,5 @@ import { + CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' @@ -6,7 +7,8 @@ import { remoteRuntimeClientCapabilities } from '../../../src/shared/remote-runt export const MOBILE_RUNTIME_CLIENT_CAPABILITIES = remoteRuntimeClientCapabilities([ STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, - STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY + STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY, + CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY ]) export const MOBILE_RUNTIME_CLIENT_CAPABILITY_UPDATE_METHOD = diff --git a/mobile/src/transport/rpc-client-capabilities.test.ts b/mobile/src/transport/rpc-client-capabilities.test.ts index 7107ae6717e..41bb4091a0b 100644 --- a/mobile/src/transport/rpc-client-capabilities.test.ts +++ b/mobile/src/transport/rpc-client-capabilities.test.ts @@ -90,7 +90,10 @@ describe('mobile rpc-client capabilities', () => { const capabilityRequest = sentRequest(socket, 'runtime.clientCapabilities.update') expect(capabilityRequest.params).toMatchObject({ - clientCapabilities: expect.arrayContaining(['agent-session.structured.v1']) + clientCapabilities: expect.arrayContaining([ + 'agent-session.structured.v1', + 'agent-session.structured.claude.v1' + ]) }) expect(socket.sent.some((payload) => payload.includes('session.tabs.subscribe'))).toBe(false) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-schemas.ts b/src/main/runtime/rpc/methods/structured-agent-session-schemas.ts index 58dece2256c..233809d40ff 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-schemas.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-schemas.ts @@ -12,6 +12,8 @@ import { import { normalizeExecutionHostId } from '../../../../shared/execution-host' const MAX_ID_LENGTH = 512 +// Four Claude questions with all four generated choices occupy 610 chars when fully percent-encoded. +const MAX_RESPONSE_OPTION_ID_LENGTH = 1024 const MAX_PROMPT_BYTES = 256 * 1024 const MAX_BLOCKS = 64 const MAX_OPTION_LABEL = 512 @@ -21,11 +23,11 @@ export const SessionId = z .max(MAX_ID_LENGTH) .refine(isAgentSessionId, 'Invalid agent session id') -const Identifier = (message: string) => +const Identifier = (message: string, maxLength = MAX_ID_LENGTH) => z .string() .min(1, message) - .max(MAX_ID_LENGTH, message) + .max(maxLength, message) .refine((value) => value === value.trim(), message) export const JournalCursor = z @@ -166,7 +168,7 @@ export const RespondParams = z itemId: Identifier('Invalid item id'), /** Compare-and-set: the revision the client had on screen. */ expectedRevision: z.number().int().positive(), - optionId: Identifier('Invalid option id') + optionId: Identifier('Invalid option id', MAX_RESPONSE_OPTION_ID_LENGTH) }) .strict() diff --git a/src/main/runtime/rpc/methods/structured-agent-session.test.ts b/src/main/runtime/rpc/methods/structured-agent-session.test.ts index c43c82d03ee..8defafb4433 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session.test.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session.test.ts @@ -732,6 +732,41 @@ describe('parameter validation', () => { }) }) + it('accepts the maximum fully encoded Claude choice group and retains a finite bound', async () => { + const maximumSelections = Array.from({ length: 4 }, (_, questionIndex) => ({ + questionId: `q${questionIndex + 1}`, + optionIds: Array.from( + { length: 4 }, + (_, optionIndex) => `q${questionIndex + 1}:choice-${optionIndex + 1}` + ) + })) + const optionId = `question-group:${encodeURIComponent(JSON.stringify(maximumSelections))}` + expect(optionId.length).toBe(610) + + const response = await call( + 'agentSession.respondToQuestion', + { + envelope: envelope(), + itemId: 'item-1', + expectedRevision: 1, + optionId + }, + STRUCTURED_CLIENT + ) + expect(response).toMatchObject({ ok: true }) + expect(hostCalls.respondToPrompt).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ optionId }) + ) + + await rejects('agentSession.respondToQuestion', { + envelope: envelope(), + itemId: 'item-1', + expectedRevision: 1, + optionId: 'x'.repeat(1025) + }) + }) + it('bounds a history page and validates its cursor', async () => { await rejects('agentSession.history', { sessionId: SESSION, diff --git a/src/renderer/src/lib/launch-structured-agent-session.ts b/src/renderer/src/lib/launch-structured-agent-session.ts index 3d7f94a1135..ae85117b7e6 100644 --- a/src/renderer/src/lib/launch-structured-agent-session.ts +++ b/src/renderer/src/lib/launch-structured-agent-session.ts @@ -1,13 +1,13 @@ import type { AgentSessionHandleProvider } from '../../../shared/agent-session-provider-handle' import type { AgentSessionAttachResult, - AgentSessionMutationEnvelope, AgentSessionMutationResult } from '../../../shared/agent-session-wire' import { - createStructuredAgentSessionOperationId, - structuredAgentSessionPayloadFingerprint -} from '../../../shared/structured-agent-session-mutation' + createStructuredAgentSessionId, + structuredAgentSessionCreateParams, + type StructuredAgentSessionCreateParams +} from '../../../shared/structured-agent-session-create' import { hasRuntimeRpcErrorCode } from '../../../shared/runtime-rpc-error-code' import { isDefinitiveAgentSessionCreateRefusal } from '../../../shared/agent-session-definitive-refusal' import { callStructuredAgentSession } from '@/runtime/structured-agent-session-client' @@ -20,12 +20,6 @@ import { } from '@/runtime/web-session-focus-intent' import { LOCAL_STRUCTURED_SESSION_OWNER } from '@/runtime/local-structured-session-tabs-sync' -type StructuredAgentSessionCreateParams = { - envelope: AgentSessionMutationEnvelope - worktree: string - agent: AgentSessionHandleProvider -} - export type StructuredAgentSessionLaunchIntent = { sessionId: string worktreeId: string @@ -96,8 +90,7 @@ export function createStructuredAgentSessionLaunchIntent( worktreeId: string, agent: AgentSessionHandleProvider ): StructuredAgentSessionLaunchIntent { - const sessionId = `${agent}_${crypto.randomUUID().replaceAll('-', '_')}` - const fields = { worktree: toRuntimeWorktreeSelector(worktreeId), agent } + const sessionId = createStructuredAgentSessionId(agent, () => crypto.randomUUID()) const state = useAppStore.getState() recordWebSessionFocusIntent( { environmentId: LOCAL_STRUCTURED_SESSION_OWNER }, @@ -110,19 +103,12 @@ export function createStructuredAgentSessionLaunchIntent( sessionId, worktreeId, agent, - params: { - envelope: { - sessionId, - clientOperationId: createStructuredAgentSessionOperationId(() => crypto.randomUUID()), - expectedRuntimeFence: null, - payloadFingerprint: structuredAgentSessionPayloadFingerprint({ - method: 'agentSession.create', - sessionId, - fields - }) - }, - ...fields - } + params: structuredAgentSessionCreateParams({ + sessionId, + worktree: toRuntimeWorktreeSelector(worktreeId), + agent, + randomUuid: () => crypto.randomUUID() + }) } } diff --git a/src/shared/agent-session-question-answer.test.ts b/src/shared/agent-session-question-answer.test.ts index 529bfdfd72d..3f8820d3013 100644 --- a/src/shared/agent-session-question-answer.test.ts +++ b/src/shared/agent-session-question-answer.test.ts @@ -6,6 +6,12 @@ import { type AgentSessionQuestionAnswer } from './agent-session-question-answer' +const GROUP_ANSWER_PREFIX = 'question-group:' + +function decodeWithOriginalPercentDecoder(encoded: string): unknown { + return JSON.parse(decodeURIComponent(encoded.slice(GROUP_ANSWER_PREFIX.length))) +} + describe('agent-session grouped question answers', () => { const answers: AgentSessionQuestionAnswer[] = [ { questionId: 'q1', optionIds: ['target-web', 'target-mobile'] }, @@ -18,6 +24,44 @@ describe('agent-session grouped question answers', () => { ) }) + it('keeps compact answers readable by the original percent-decoding contract', () => { + expect(decodeWithOriginalPercentDecoder(encodeAgentSessionQuestionAnswers(answers))).toEqual( + answers + ) + }) + + it('accepts the previous fully percent-encoded representation', () => { + const encoded = `${GROUP_ANSWER_PREFIX}${encodeURIComponent(JSON.stringify(answers))}` + + expect(decodeAgentSessionQuestionAnswers(encoded)).toEqual(answers) + }) + + it('round-trips percent signs and Unicode through the compact representation', () => { + const unicodeAnswers: AgentSessionQuestionAnswer[] = [ + { + questionId: '進捗%', + optionIds: ['100%:完了', '🚀'], + other: 'café 東京 50%' + } + ] + + expect( + decodeAgentSessionQuestionAnswers(encodeAgentSessionQuestionAnswers(unicodeAnswers)) + ).toEqual(unicodeAnswers) + }) + + it("fits Claude's maximum choice group within a 512-character host response bound", () => { + const maximumSelections = Array.from({ length: 4 }, (_, questionIndex) => ({ + questionId: `q${questionIndex + 1}`, + optionIds: Array.from( + { length: 4 }, + (_, optionIndex) => `q${questionIndex + 1}:choice-${optionIndex + 1}` + ) + })) + + expect(encodeAgentSessionQuestionAnswers(maximumSelections).length).toBeLessThanOrEqual(512) + }) + it('validates each grouped answer against its question shape', () => { const questions = [ { diff --git a/src/shared/agent-session-question-answer.ts b/src/shared/agent-session-question-answer.ts index f90df30ddff..072dfce200a 100644 --- a/src/shared/agent-session-question-answer.ts +++ b/src/shared/agent-session-question-answer.ts @@ -11,7 +11,8 @@ export type AgentSessionQuestionAnswer = { export function encodeAgentSessionQuestionAnswers( answers: readonly AgentSessionQuestionAnswer[] ): string { - return `${GROUP_ANSWER_PREFIX}${encodeURIComponent(JSON.stringify(answers))}` + // RPC already JSON-frames this value; escaping `%` alone preserves decodeURIComponent readers. + return `${GROUP_ANSWER_PREFIX}${JSON.stringify(answers).replaceAll('%', '%25')}` } export function decodeAgentSessionQuestionAnswers( diff --git a/src/shared/structured-agent-session-create.ts b/src/shared/structured-agent-session-create.ts new file mode 100644 index 00000000000..13c7b4fe29a --- /dev/null +++ b/src/shared/structured-agent-session-create.ts @@ -0,0 +1,48 @@ +import type { AgentSessionHandleProvider } from './agent-session-provider-handle' +import type { AgentSessionMutationEnvelope } from './agent-session-wire' +import { + createStructuredAgentSessionOperationId, + structuredAgentSessionCreateFingerprint +} from './structured-agent-session-mutation' + +export type StructuredAgentSessionCreateParams = { + envelope: AgentSessionMutationEnvelope + worktree: string + agent: AgentSessionHandleProvider +} + +/** Provider-prefixed so a session id names its lane on sight, and underscore-only + * so the id stays a single token everywhere it is embedded (tab ids, log keys). */ +export function createStructuredAgentSessionId( + agent: AgentSessionHandleProvider, + randomUuid: () => string +): string { + return `${agent}_${randomUuid().replaceAll('-', '_')}` +} + +/** + * The durable `agentSession.create` envelope every client replays on an ambiguous + * transport failure. The fingerprint must be computed over the same fields the host + * recomputes, so both clients build it here rather than each assembling their own. + */ +export function structuredAgentSessionCreateParams(args: { + sessionId: string + worktree: string + agent: AgentSessionHandleProvider + randomUuid: () => string + now?: number +}): StructuredAgentSessionCreateParams { + const fields = { worktree: args.worktree, agent: args.agent } + return { + envelope: { + sessionId: args.sessionId, + clientOperationId: createStructuredAgentSessionOperationId(args.randomUuid, args.now), + expectedRuntimeFence: null, + payloadFingerprint: structuredAgentSessionCreateFingerprint({ + sessionId: args.sessionId, + ...fields + }) + }, + ...fields + } +}