fix(native-chat): restore Claude grouped question handling

2026-09-30 08:03:12 +00:00 · 2026-09-02 07:04:29 -07:00
parent 256d037d2b
commit f568e60e7e
6 changed files with 361 additions and 14 deletions
@@ -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.
@@ -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' }
+    ])
+  })
 })
@@ -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({
              />
            ))}
            <div className="flex items-center gap-3 px-3.5 py-2.5">
-              {allowOther ? (
+              {questionAllowsOther ? (
                <>
                  <span className="flex size-6 shrink-0 items-center justify-center rounded-md bg-muted text-muted-foreground">
                    <Pencil className="size-3.5" />
@@ -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<string, unknown> },
+  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(
+      <NativeChatStructuredSession
+        isVisible
+        tabId="structured-tab-questions"
+        sessionId="session-questions"
+        target={{ kind: 'local' }}
+        agent="claude"
+        allowFileUriLinks={false}
+      />
+    )
+
+    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(
+      <NativeChatStructuredSession
+        isVisible
+        tabId="structured-tab-legacy-question"
+        sessionId="session-legacy-question"
+        target={{ kind: 'local' }}
+        agent="claude"
+        allowFileUriLinks={false}
+      />
+    )
+
+    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')
+  })
 })
@@ -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 ? (
        <NativeChatQuestionCard
+          key={`${prompt.itemId}:${prompt.revision}`}
          prompt={{
-            questions: [
-              {
-                question: questionBody.question,
-                multiSelect: false,
-                options: questionBody.options.map((option) => ({ 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 =
@@ -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)
+  })
+})