feat(desktop): keyboard navigation for clarify choices

apps/desktop/src/components/assistant-ui/clarify-tool.test.tsx

@@ -1,15 +1,28 @@
 import type { ToolCallMessagePartProps } from '@assistant-ui/react'
-import { cleanup, fireEvent, render, screen } from '@testing-library/react'
+import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
 import type { ReactNode } from 'react'
 import { afterEach, describe, expect, it, vi } from 'vitest'
 
 import { onComposerInsertRequest } from '@/app/chat/composer/focus'
 import { I18nProvider } from '@/i18n'
+import { clearClarifyRequest, setClarifyRequest } from '@/store/clarify'
+import { $gateway } from '@/store/gateway'
+import { $activeSessionId } from '@/store/session'
 
 import { ClarifyTool, readClarifyResult } from './clarify-tool'
 
+// The live pending card only renders while its message is running. Force that so
+// keyboard-navigation tests can exercise ClarifyToolPending directly.
+vi.mock('@assistant-ui/react', () => ({
+  useAuiState: () => true
+}))
+
 afterEach(() => {
   cleanup()
+  clearClarifyRequest()
+  $activeSessionId.set(null)
+  $gateway.set(null)
+  vi.clearAllMocks()
 })
 
 function renderClarify(ui: ReactNode) {
@@ -40,6 +53,40 @@
   }
 }
 
+function liveClarifyProps(choices = ['staging', 'production']): ToolCallMessagePartProps {
+  const args = { choices, question: 'Which deployment target?' }
+
+  return {
+    addResult: vi.fn(),
+    args,
+    argsText: JSON.stringify(args),
+    isError: false,
+    respondToApproval: vi.fn(),
+    result: undefined,
+    resume: vi.fn(),
+    status: { type: 'running' },
+    toolCallId: 'clarify-live',
+    toolName: 'clarify',
+    type: 'tool-call'
+  }
+}
+
+function renderLiveClarify() {
+  const request = vi.fn().mockResolvedValue({ ok: true })
+
+  $activeSessionId.set('session-1')
+  $gateway.set({ request } as never)
+  setClarifyRequest({
+    choices: ['staging', 'production'],
+    question: 'Which deployment target?',
+    requestId: 'request-1',
+    sessionId: 'session-1'
+  })
+  renderClarify(<ClarifyTool {...liveClarifyProps()} />)
+
+  return request
+}
+
 describe('readClarifyResult', () => {
   it('reads question + user_response from the tool JSON payload', () => {
     expect(
@@ -97,8 +144,8 @@
 
     expect(screen.getByText('Which deployment target?')).toBeTruthy()
     expect(screen.getByText('staging')).toBeTruthy()
     expect(document.querySelector('[data-clarify-settled]')).toBeTruthy()
     expect(document.querySelector('[data-clarify-answer]')?.textContent).toBe('staging')
   })
 
   it('labels an empty response as Skipped', () => {
@@ -182,3 +229,70 @@
     expect(document.querySelector('[data-clarify-late-choices]')).toBeNull()
   })
 })
+
+describe('ClarifyTool keyboard navigation', () => {
+  it('cycles through choices and Other with the arrow keys', () => {
+    renderLiveClarify()
+
+    const staging = screen.getByRole('button', { name: /staging/ })
+    const production = screen.getByRole('button', { name: /production/ })
+    const other = screen.getByPlaceholderText(/Other/)
+
+    expect(staging.getAttribute('data-highlighted')).toBe('true')
+    expect(staging.getAttribute('aria-current')).toBe('true')
+    expect(staging.getAttribute('aria-keyshortcuts')).toBe('A 1')
+
+    fireEvent.keyDown(window, { key: 'ArrowDown' })
+    expect(production.getAttribute('data-highlighted')).toBe('true')
+
+    fireEvent.keyDown(window, { key: 'ArrowDown' })
+    expect(other.closest('label')?.getAttribute('data-highlighted')).toBe('true')
+    expect(other.getAttribute('aria-current')).toBe('true')
+    expect(other.getAttribute('aria-keyshortcuts')).toBe('C 3')
+
+    fireEvent.keyDown(window, { key: 'ArrowDown' })
+    expect(staging.getAttribute('data-highlighted')).toBe('true')
+
+    fireEvent.keyDown(window, { key: 'ArrowUp' })
+    expect(other.closest('label')?.getAttribute('data-highlighted')).toBe('true')
+  })
+
+  it('selects by number and confirms the answer with Enter', async () => {
+    const request = renderLiveClarify()
+
+    fireEvent.keyDown(window, { key: '2' })
+    fireEvent.keyDown(window, { key: 'Enter' })
+
+    await waitFor(() => {
+      expect(request).toHaveBeenCalledWith('clarify.respond', {
+        answer: 'production',
+        request_id: 'request-1'
+      })
+    })
+  })
+
+  it('focuses Other when its number is pressed and leaves typing keys alone', () => {
+    renderLiveClarify()
+
+    const other = screen.getByPlaceholderText(/Other/)
+
+    fireEvent.keyDown(window, { key: '3' })
+    expect(document.activeElement).toBe(other)
+
+    fireEvent.change(other, { target: { value: 'canary' } })
+    fireEvent.keyDown(window, { key: 'ArrowUp' })
+    expect(document.activeElement).toBe(other)
+    expect((other as HTMLTextAreaElement).value).toBe('canary')
+  })
+
+  it('does not intercept keyboard events while an action button has focus', () => {
+    const request = renderLiveClarify()
+    const skip = screen.getByRole('button', { name: 'Skip' })
+
+    skip.focus()
+
+    expect(fireEvent.keyDown(window, { key: 'Enter' })).toBe(true)
+    expect(fireEvent.keyDown(window, { key: 'ArrowDown' })).toBe(true)
+    expect(request).not.toHaveBeenCalled()
+  })
+})

apps/desktop/src/components/assistant-ui/clarify-tool.tsx

@@ -138,16 +138,20 @@ function KeyBadge({ char, preview, selected }: { char: string; preview?: boolean
  * selects an answer) and the settled skip card (where a click drafts a
  * follow-up), so both stay visually identical. */
 function ChoiceButton({
+  active = false,
   char,
   choice,
   disabled,
+  keyShortcuts,
   onClick,
   selected = false,
   title
 }: {
+  active?: boolean
   char: string
   choice: string
   disabled?: boolean
+  keyShortcuts?: string
   onClick: () => void
   selected?: boolean
   title?: string
@@ -156,20 +160,28 @@ function ChoiceButton({
   // tooltip on a <button> is banned by the no-native-title guard). It renders
   // the child untouched when `label` is falsy, so the live card (no tip) is
   // unaffected and only the settled skip card gets the hover hint.
+  //
+  // `active` is the keyboard cursor on the live card (arrow-key navigation);
+  // it highlights the row and previews its key badge. The settled skip card
+  // never passes it, so its rows stay plain.
   return (
     <Tip label={title}>
       <button
+        aria-current={active || undefined}
+        aria-keyshortcuts={keyShortcuts}
         className={cn(
           OPTION_ROW_CLASS,
           'text-(--ui-text-secondary) hover:bg-(--chrome-action-hover) hover:text-(--ui-text-primary)',
+          active && 'bg-(--chrome-action-hover) text-(--ui-text-primary)',
           selected && 'text-(--ui-text-primary)'
         )}
         data-choice
+        data-highlighted={active || undefined}
         disabled={disabled}
         onClick={onClick}
         type="button"
       >
-        <KeyBadge char={char} selected={selected} />
+        <KeyBadge char={char} preview={active} selected={selected} />
         <span className="flex-1 wrap-anywhere">{choice}</span>
       </button>
     </Tip>
@@ -298,6 +310,9 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
   const [draft, setDraft] = useState('')
   const [submitting, setSubmitting] = useState(false)
   const [selectedChoice, setSelectedChoice] = useState<string | null>(null)
+  // The keyboard cursor. Indices 0..choices.length-1 are the options; the
+  // trailing index (=== choices.length) is the "Other" free-text row.
+  const [activeIndex, setActiveIndex] = useState(0)
   const [otherFocused, setOtherFocused] = useState(false)
   const textareaRef = useRef<HTMLTextAreaElement | null>(null)
 
@@ -346,12 +361,31 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
   // confirms with Continue (or Enter from the field).
   const pendingAnswer = selectedChoice ?? (trimmedDraft || null)
 
-  const selectChoice = useCallback((choice: string) => {
+  const selectChoice = useCallback((choice: string, index: number) => {
     // Picking a choice and typing are mutually exclusive answers.
     setDraft('')
     setSelectedChoice(choice)
+    setActiveIndex(index)
   }, [])
 
+  // Keep the cursor in range when the choice set changes (never past "Other").
+  useEffect(() => {
+    setActiveIndex(index => Math.min(index, choices.length))
+  }, [choices.length])
+
+  const moveActive = useCallback(
+    (delta: number) => {
+      const itemCount = choices.length + 1
+
+      // Arrow navigation is a move, not a pick — clear any staged answer so the
+      // cursor and the selection can't disagree.
+      setDraft('')
+      setSelectedChoice(null)
+      setActiveIndex(index => (index + delta + itemCount) % itemCount)
+    },
+    [choices.length]
+  )
+
   const submitAnswer = useCallback(() => {
     if (selectedChoice !== null) {
       void respond(selectedChoice)
@@ -364,6 +398,27 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
     }
   }, [respond, selectedChoice, trimmedDraft])
 
+  const activateActive = useCallback(() => {
+    // A staged answer (picked choice or typed text) wins — confirm it.
+    if (pendingAnswer) {
+      submitAnswer()
+
+      return
+    }
+
+    // Otherwise act on the highlighted row: a choice responds immediately, and
+    // the trailing "Other" row focuses the free-text field.
+    const choice = choices[activeIndex]
+
+    if (choice) {
+      void respond(choice)
+
+      return
+    }
+
+    textareaRef.current?.focus()
+  }, [activeIndex, choices, pendingAnswer, respond, submitAnswer])
+
   const handleTextareaKey = useCallback(
     (event: KeyboardEvent<HTMLTextAreaElement>) => {
       if (event.nativeEvent.isComposing) {
@@ -386,10 +441,11 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
     [submitAnswer]
   )
 
-  // Letter shortcuts: A/B/C… pick the matching option, the trailing letter jumps
-  // into "Other", and Enter confirms the current pick. Stands down whenever a
-  // field is focused (you're typing, not navigating) so it never eats keystrokes
-  // meant for the composer or the Other box.
+  // Arrow keys move a visual cursor, 1-9 and A/B/C… pick directly, and Enter
+  // confirms the current answer (or acts on the highlighted row). Stands down
+  // whenever a focusable control (a field, a choice button, the action bar) is
+  // focused, so it never eats keystrokes meant for the composer, the Other box,
+  // or a button the user tabbed to.
   useEffect(() => {
     if (!ready || !hasChoices || submitting) {
       return
@@ -402,7 +458,32 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
 
       const active = document.activeElement as HTMLElement | null
 
-      if (active && (active.tagName === 'INPUT' || active.tagName === 'TEXTAREA' || active.isContentEditable)) {
+      if (
+        active &&
+        (active.isContentEditable || active.matches('a[href], button, input, select, textarea, [role="button"]'))
+      ) {
+        return
+      }
+
+      if (event.key === 'ArrowDown' || event.key === 'ArrowUp') {
+        event.preventDefault()
+        moveActive(event.key === 'ArrowDown' ? 1 : -1)
+
+        return
+      }
+
+      if (/^[1-9]$/.test(event.key)) {
+        const index = Number(event.key) - 1
+
+        if (index < choices.length) {
+          event.preventDefault()
+          selectChoice(choices[index], index)
+        } else if (index === choices.length) {
+          event.preventDefault()
+          setActiveIndex(index)
+          textareaRef.current?.focus()
+        }
+
         return
       }
 
@@ -413,25 +494,26 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
 
         if (index < choices.length) {
           event.preventDefault()
-          selectChoice(choices[index])
+          selectChoice(choices[index], index)
         } else if (index === choices.length) {
           event.preventDefault()
+          setActiveIndex(index)
           textareaRef.current?.focus()
         }
 
         return
       }
 
-      if (event.key === 'Enter' && pendingAnswer) {
+      if (event.key === 'Enter') {
         event.preventDefault()
-        submitAnswer()
+        activateActive()
       }
     }
 
     window.addEventListener('keydown', onKeyDown)
 
     return () => window.removeEventListener('keydown', onKeyDown)
-  }, [choices, hasChoices, pendingAnswer, ready, selectChoice, submitAnswer, submitting])
+  }, [activateActive, choices, hasChoices, moveActive, ready, selectChoice, submitting])
 
   if (loading) {
     return (
@@ -467,23 +549,39 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
           <div className="grid gap-px" role="group">
             {choices.map((choice, index) => (
               <ChoiceButton
+                active={activeIndex === index}
                 char={letterFor(index)}
                 choice={choice}
                 disabled={submitting}
                 key={`${index}-${choice}`}
-                onClick={() => selectChoice(choice)}
+                keyShortcuts={`${letterFor(index)} ${index + 1}`}
+                onClick={() => selectChoice(choice, index)}
                 selected={selectedChoice === choice}
               />
             ))}
-            <label className={cn(OPTION_ROW_CLASS, 'items-center')}>
-              <KeyBadge char={letterFor(choices.length)} preview={otherFocused} selected={Boolean(trimmedDraft)} />
+            <label
+              className={cn(
+                OPTION_ROW_CLASS,
+                'items-center',
+                activeIndex === choices.length && 'bg-(--chrome-action-hover)'
+              )}
+              data-highlighted={activeIndex === choices.length || undefined}
+            >
+              <KeyBadge
+                char={letterFor(choices.length)}
+                preview={otherFocused || activeIndex === choices.length}
+                selected={Boolean(trimmedDraft)}
+              />
               <Textarea
+                aria-current={activeIndex === choices.length || undefined}
+                aria-keyshortcuts={`${letterFor(choices.length)} ${choices.length + 1}`}
                 className={CLARIFY_TEXTAREA_CLASS}
                 disabled={submitting}
                 onBlur={() => setOtherFocused(false)}
                 onChange={event => onDraftChange(event.target.value)}
                 onFocus={() => {
                   setSelectedChoice(null)
+                  setActiveIndex(choices.length)
                   setOtherFocused(true)
                 }}
                 onKeyDown={handleTextareaKey}