From f568e60e7e345515780ec79eb4342469953faf66 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Wed, 2 Sep 2026 07:03:01 -0700 Subject: [PATCH] fix(native-chat): restore Claude grouped question handling --- CLAUDE-GROUPED-QUESTION-P1-REPORT.md | 51 ++++++ .../NativeChatQuestionCard.test.tsx | 69 +++++++- .../native-chat/NativeChatQuestionCard.tsx | 5 +- .../NativeChatStructuredSession.test.tsx | 147 +++++++++++++++++- .../NativeChatStructuredSession.tsx | 54 ++++++- .../agent-session-question-answer.test.ts | 49 ++++++ 6 files changed, 361 insertions(+), 14 deletions(-) create mode 100644 CLAUDE-GROUPED-QUESTION-P1-REPORT.md create mode 100644 src/shared/agent-session-question-answer.test.ts diff --git a/CLAUDE-GROUPED-QUESTION-P1-REPORT.md b/CLAUDE-GROUPED-QUESTION-P1-REPORT.md new file mode 100644 index 00000000000..6f89f9ff020 --- /dev/null +++ b/CLAUDE-GROUPED-QUESTION-P1-REPORT.md @@ -0,0 +1,51 @@ +# Claude grouped-question P1 + +## Outcome + +Implemented the narrow renderer/native-chat fix on top of integrated parent +`256d037d2bc73fbbc812082124e43d064213a391` (the child commit SHA is recorded +after commit). Claude structured question items now retain every grouped +question, per-question multi-select state, per-question free-text capability, +and one shared grouped-answer payload; legacy single-question journal rows keep +their existing option/free-text route. + +## Reproduction and coverage + +The first red reproduction was `NativeChatQuestionCard > applies free-text +capability per question in a grouped prompt`: the pre-fix card treated a +boolean-array capability as truthy and rendered free text for a listed-only +question. Focused coverage now includes grouped card rendering and answer +selection, shared grouped-answer encode/decode and validation, grouped +multi-select Claude callback settlement, and legacy single-question routing. + +## Verification + +- Focused Claude/native-chat/shared Vitest: 152 files passed, 1,447 tests + passed, 2 skipped. +- Node typecheck: `tsc --noEmit -p config/tsconfig.node.json` passed. +- Web typecheck: `tsc --noEmit -p config/tsconfig.tc.web.json` passed. +- Changed-file native and type-aware oxlint passed with no warnings. +- Changed-file `oxfmt --check` passed. +- `pnpm-lock.yaml` is byte-identical to the parent baseline and is not part + of the change. + +## Files + +- `src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx` +- `src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx` +- `src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx` +- `src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx` +- `src/shared/agent-session-question-answer.test.ts` + +## Readiness notes + +The change only consumes the established journal grouped-question shape and +shared answer codec; no RPC/stream opcode, lease/fence, journal identity, +provider session/leaf, stale-tail, restart/reconnect, SSH, or platform wire +contract was changed. No mobile-facing source was touched; existing mixed +version behavior remains additive through the journal's optional `questions` +field and the legacy top-level projection. + +Residual risk: a live Electron/Claude TUI run was not requested for this +implementation; the adapter and renderer contract tests cover the provider +callback and answer payload boundaries. diff --git a/src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx b/src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx index b6748fe7e82..9b1a2967682 100644 --- a/src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx @@ -28,7 +28,7 @@ afterEach(() => { function render( prompt: AskPrompt, onAnswer: (s: AskAnswerSelection[]) => void, - allowOther = true + allowOther: boolean | readonly boolean[] = true ): void { act(() => { root.render( @@ -155,4 +155,71 @@ describe('NativeChatQuestionCard', () => { expect(container.querySelector('input')).toBeNull() expect(container.textContent).not.toContain('Type your answer') }) + + it('applies free-text capability per question in a grouped prompt', () => { + render( + { + questions: [ + { + header: 'Listed', + question: 'Pick a listed value', + multiSelect: false, + options: [{ label: 'One' }] + }, + { + header: 'Custom', + question: 'Provide a custom value', + multiSelect: false, + options: [] + } + ] + }, + vi.fn(), + [false, true] + ) + + expect(container.querySelector('input')).toBeNull() + clickAction('Skip') + expect(container.querySelector('input')).not.toBeNull() + }) + + it('submits grouped multi-select and free-text answers together', () => { + const onAnswer = vi.fn() + render( + { + questions: [ + { + header: 'Targets', + question: 'Which targets?', + multiSelect: true, + options: [{ label: 'Web' }, { label: 'Mobile' }] + }, + { + header: 'Notes', + question: 'Anything else?', + multiSelect: false, + options: [] + } + ] + }, + onAnswer, + [false, true] + ) + + clickOption('Web') + clickOption('Mobile') + clickAction('Next') + const input = container.querySelector('input')! + act(() => { + const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set! + setter.call(input, 'SSH host') + input.dispatchEvent(new Event('input', { bubbles: true })) + }) + clickAction('Submit') + + expect(onAnswer).toHaveBeenCalledWith([ + { indices: [0, 1], other: '' }, + { indices: [], other: 'SSH host' } + ]) + }) }) diff --git a/src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx b/src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx index 4bc881ee3e1..1b1ce5a3547 100644 --- a/src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx +++ b/src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx @@ -10,7 +10,7 @@ export type NativeChatQuestionCardProps = { isSubmitting?: boolean /** Deliver the chosen answer (per-question option indices + free text). */ onAnswer: (selections: AskAnswerSelection[]) => void - allowOther?: boolean + allowOther?: boolean | readonly boolean[] /** Dismiss the prompt (sends Escape to the agent). */ onCancel: () => void /** Exposes the free-text row so pane-level Paste can target it while the @@ -42,6 +42,7 @@ export function NativeChatQuestionCard({ const total = prompt.questions.length const isLast = index === total - 1 const q = prompt.questions[index]! + const questionAllowsOther = Array.isArray(allowOther) ? (allowOther[index] ?? false) : allowOther const setOther = (qi: number, value: string): void => { setOtherText((prev) => { @@ -186,7 +187,7 @@ export function NativeChatQuestionCard({ /> ))}
- {allowOther ? ( + {questionAllowsOther ? ( <> diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx index bfb3dd1ef52..d3c13a48ef1 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx @@ -3,6 +3,9 @@ import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' import React, { forwardRef, useImperativeHandle } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' +import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' +import { decodeAgentSessionQuestionAnswers } from '../../../../shared/agent-session-question-answer' +import type { NativeChatQuestionCardProps } from './NativeChatQuestionCard' const mocks = vi.hoisted(() => ({ call: vi.fn(), @@ -13,6 +16,9 @@ const mocks = vi.hoisted(() => ({ onLinkClick?: (...args: unknown[]) => void }, composerProps: null as null | { structuredTransport?: Record }, + questionCardProps: null as NativeChatQuestionCardProps | null, + promptItems: [] as AgentJournalRenderItem[], + respond: vi.fn(), handlePasteEvent: vi.fn(), pasteFromClipboard: vi.fn(), submissions: [] as unknown[] @@ -53,7 +59,7 @@ vi.mock('./use-structured-agent-session', async () => { hasOlder: false, loadingOlder: false, loadOlder: vi.fn(), - prompts: [], + prompts: mocks.promptItems, outbox: outbox.outbox, blockedClientMessageId: outbox.blockedClientMessageId, send: outbox.send, @@ -61,7 +67,7 @@ vi.mock('./use-structured-agent-session', async () => { isWorking: false, turnId: null, cancel: vi.fn(), - respond: vi.fn(), + respond: mocks.respond, optionSnapshot: [ { id: 'model', @@ -125,7 +131,12 @@ vi.mock('./NativeChatComposer', () => ({ })) vi.mock('./NativeChatEmptyState', () => ({ NativeChatEmptyState: () => null })) vi.mock('./NativeChatApprovalCard', () => ({ NativeChatApprovalCard: () => null })) -vi.mock('./NativeChatQuestionCard', () => ({ NativeChatQuestionCard: () => null })) +vi.mock('./NativeChatQuestionCard', () => ({ + NativeChatQuestionCard: (props: NativeChatQuestionCardProps) => { + mocks.questionCardProps = props + return null + } +})) import { NativeChatStructuredSession } from './NativeChatStructuredSession' @@ -136,6 +147,9 @@ describe('NativeChatStructuredSession', () => { mocks.mode = 'static' mocks.messageListProps = null mocks.composerProps = null + mocks.questionCardProps = null + mocks.promptItems = [] + mocks.respond.mockReset() mocks.handlePasteEvent.mockReset() mocks.pasteFromClipboard.mockReset() mocks.submissions = [] @@ -546,4 +560,131 @@ describe('NativeChatStructuredSession', () => { vi.useRealTimers() } }, 30000) + + it('passes Claude grouped questions and one shared answer through the card', () => { + mocks.promptItems = [ + { + itemId: 'question-item', + revision: 1, + sequence: 1, + observedAt: 1, + body: { + kind: 'question', + question: '2 grouped questions from Claude', + options: [], + questions: [ + { + id: 'q1', + header: 'Targets', + question: 'Which targets?', + multiSelect: true, + options: [ + { id: 'target-web', label: 'Web' }, + { id: 'target-mobile', label: 'Mobile' } + ], + freeTextQuestionId: 'q1' + }, + { + id: 'q2', + header: 'Host', + question: 'Where should it run?', + multiSelect: false, + options: [], + freeTextQuestionId: 'q2' + } + ], + resolution: { + state: 'pending', + selectedOptionId: null, + resolvedBy: null, + resolvedAt: null + } + } + } + ] + + render( + + ) + + const card = mocks.questionCardProps + if (!card) { + throw new Error('question card was not rendered') + } + expect(card.prompt.questions).toHaveLength(2) + expect(card.prompt.questions[0]).toMatchObject({ + question: 'Which targets?', + multiSelect: true, + options: [{ label: 'Web' }, { label: 'Mobile' }] + }) + expect(card.allowOther).toEqual([true, true]) + + card.onAnswer([ + { indices: [0, 1], other: '' }, + { indices: [], other: 'SSH host' } + ]) + const encoded = mocks.respond.mock.calls[0]?.[1] + expect(decodeAgentSessionQuestionAnswers(encoded)).toEqual([ + { questionId: 'q1', optionIds: ['target-web', 'target-mobile'] }, + { questionId: 'q2', optionIds: [], other: 'SSH host' } + ]) + }) + + it('keeps legacy single-question option ids and free text behavior', () => { + mocks.promptItems = [ + { + itemId: 'legacy-question-item', + revision: 1, + sequence: 1, + observedAt: 1, + body: { + kind: 'question', + question: 'Pick a library', + options: [ + { id: 'q1:choice-1', label: 'React' }, + { id: 'q1:choice-2', label: 'Vue' } + ], + freeTextQuestionId: 'q1', + resolution: { + state: 'pending', + selectedOptionId: null, + resolvedBy: null, + resolvedAt: null + } + } + } + ] + + render( + + ) + + const card = mocks.questionCardProps + if (!card) { + throw new Error('question card was not rendered') + } + expect(card.prompt.questions).toEqual([ + { + question: 'Pick a library', + multiSelect: false, + options: [{ label: 'React' }, { label: 'Vue' }] + } + ]) + card.onAnswer([{ indices: [1], other: '' }]) + expect(mocks.respond).toHaveBeenCalledWith(mocks.promptItems[0], 'q1:choice-2') + }) }) diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx index 7a02829a05a..73a6e5c8c06 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx @@ -4,6 +4,7 @@ import type { AgentStatusOrchestrationContext, AgentType } from '../../../../shared/agent-status-types' +import { encodeAgentSessionQuestionAnswers } from '../../../../shared/agent-session-question-answer' import { dispatchStructuredAgentSessionComposerCommand } from '../../../../shared/structured-agent-session-composer' import { structuredAgentSessionPaneKey } from '../../../../shared/structured-agent-session-projection' import type { NativeChatLiveSession } from './use-native-chat-live-session' @@ -83,6 +84,21 @@ export function NativeChatStructuredSession(props: { const fileLinkClick = useNativeChatFileLinkClick(props.allowFileUriLinks ? fileLinkContext : null) const prompt = controller.prompts[0] ?? null const questionBody = prompt?.body.kind === 'question' ? prompt.body : null + const questions = + questionBody?.questions ?? + (questionBody + ? [ + { + id: questionBody.freeTextQuestionId ?? 'q1', + question: questionBody.question, + options: questionBody.options, + multiSelect: false, + ...(questionBody.freeTextQuestionId + ? { freeTextQuestionId: questionBody.freeTextQuestionId } + : {}) + } + ] + : []) const retryableOutboxEntry = controller.outbox.find((entry) => entry.state === 'unconfirmed') ?? controller.outbox.find( @@ -163,17 +179,39 @@ export function NativeChatStructuredSession(props: { ) : null} {prompt && questionBody ? ( ({ label: option.label })) - } - ] + questions: questions.map((question) => ({ + question: question.question, + ...(question.header ? { header: question.header } : {}), + multiSelect: question.multiSelect, + options: question.options.map((option) => ({ + label: option.label, + ...(option.description ? { description: option.description } : {}) + })) + })) }} - allowOther={Boolean(questionBody.freeTextQuestionId)} + allowOther={questions.map((question) => Boolean(question.freeTextQuestionId))} onAnswer={(answers) => { + if (questionBody.questions) { + const grouped = questions.map((question, questionIndex) => { + const answer = answers[questionIndex] + const other = answer?.other?.trim() + const optionIds = (answer?.indices ?? []).flatMap((optionIndex) => { + const optionId = question.options[optionIndex]?.id + return optionId ? [optionId] : [] + }) + return { + questionId: question.id, + optionIds: question.multiSelect || !other ? optionIds : [], + ...(other ? { other } : {}) + } + }) + if (grouped.every((answer) => answer.optionIds.length > 0 || answer.other)) { + void controller.respond(prompt, encodeAgentSessionQuestionAnswers(grouped)) + } + return + } const index = answers[0]?.indices[0] const other = answers[0]?.other?.trim() const optionId = diff --git a/src/shared/agent-session-question-answer.test.ts b/src/shared/agent-session-question-answer.test.ts new file mode 100644 index 00000000000..529bfdfd72d --- /dev/null +++ b/src/shared/agent-session-question-answer.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from 'vitest' +import { + decodeAgentSessionQuestionAnswers, + encodeAgentSessionQuestionAnswers, + isValidAgentSessionQuestionAnswers, + type AgentSessionQuestionAnswer +} from './agent-session-question-answer' + +describe('agent-session grouped question answers', () => { + const answers: AgentSessionQuestionAnswer[] = [ + { questionId: 'q1', optionIds: ['target-web', 'target-mobile'] }, + { questionId: 'q2', optionIds: [], other: 'SSH host' } + ] + + it('round-trips grouped multi-select and free-text answers', () => { + expect(decodeAgentSessionQuestionAnswers(encodeAgentSessionQuestionAnswers(answers))).toEqual( + answers + ) + }) + + it('validates each grouped answer against its question shape', () => { + const questions = [ + { + id: 'q1', + question: 'Targets', + multiSelect: true, + options: [ + { id: 'target-web', label: 'Web' }, + { id: 'target-mobile', label: 'Mobile' } + ] + }, + { + id: 'q2', + question: 'Host', + multiSelect: false, + options: [], + freeTextQuestionId: 'q2' + } + ] + + expect(isValidAgentSessionQuestionAnswers(questions, answers)).toBe(true) + expect( + isValidAgentSessionQuestionAnswers(questions, [ + { questionId: 'q1', optionIds: ['unknown'] }, + answers[1]! + ]) + ).toBe(false) + }) +})