fix(desktop): keep the clarify card until it is answered, skipped, or expired

apps/desktop/src/app/session/hooks/use-message-stream/clarify-hydration.test.tsx

@@ -5,7 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
 import type { ClientSessionState } from '@/app/types'
 import { createClientSessionState } from '@/lib/chat-runtime'
-import { clearClarifyRequest } from '@/store/clarify'
+import { $clarifyRequests, clearClarifyRequest } from '@/store/clarify'
 import type { RpcEvent } from '@/types/hermes'
 
 import { useMessageStream } from './index'
@@ -57,6 +57,9 @@ async function mountStream() {
 const clarifyRequest = (payload: Record<string, unknown>) =>
   act(() => handleEvent!({ payload, session_id: SID, type: 'clarify.request' }))
 
+const clarifyExpire = (payload: Record<string, unknown>) =>
+  act(() => handleEvent!({ payload, session_id: SID, type: 'clarify.expire' }))
+
 const toolStart = (payload: Record<string, unknown>) =>
   act(() => handleEvent!({ payload, session_id: SID, type: 'tool.start' }))
 
@@ -105,6 +108,34 @@ describe('clarify.request stream hydration', () => {
     expect(clarifyParts()).toHaveLength(1)
   })
 
+  it('drops the live request and the needs-input flag when the clarify expires', async () => {
+    await mountStream()
+
+    clarifyRequest({ choices: ['a'], question: 'Pick', request_id: 'req-exp' })
+
+    expect($clarifyRequests.get()[SID]).toBeTruthy()
+    expect(stateRef?.current.get(SID)?.needsInput).toBe(true)
+
+    // The server-side wait elapsed: the request is gone from the gateway's
+    // `_pending`, so the card must stop pretending to be answerable.
+    clarifyExpire({ request_id: 'req-exp' })
+
+    expect($clarifyRequests.get()[SID]).toBeUndefined()
+    expect(stateRef?.current.get(SID)?.needsInput).toBe(false)
+  })
+
+  it('leaves a newer clarify alone when a stale expire arrives', async () => {
+    await mountStream()
+
+    clarifyRequest({ choices: ['a'], question: 'Pick', request_id: 'req-new' })
+    clarifyExpire({ request_id: 'req-old' })
+
+    expect($clarifyRequests.get()[SID]?.requestId).toBe('req-new')
+    // The session is still blocked on the newer question, so the sidebar's
+    // attention indicator must survive a no-op expire.
+    expect(stateRef?.current.get(SID)?.needsInput).toBe(true)
+  })
+
   it('does not duplicate when clarify.request arrives before the tool.start row', async () => {
     await mountStream()
 

apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts

@@ -739,6 +739,24 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) {
             title: translateNow('notifications.native.inputTitle')
           })
         }
+      } else if (event.type === 'clarify.expire') {
+        // The server-side wait elapsed (agent.clarify_timeout) and the tool
+        // returned empty. Without this the card kept offering choices that
+        // could no longer resolve anything — the request is gone from the
+        // gateway's `_pending`. Drop the live request so the row settles into
+        // the "Skipped" card (which keeps the options actionable as a drafted
+        // follow-up) instead of pretending to still be answerable.
+        const expiredId = typeof payload?.request_id === 'string' ? payload.request_id : ''
+
+        const cleared = clearClarifyRequest(expiredId || undefined, sessionId ?? undefined)
+
+        // Only when THIS session's live request was the one that expired. A
+        // stale expire (the request was already superseded or answered) is a
+        // no-op, and must not drop the sidebar indicator on a session that is
+        // still blocked on a newer question.
+        if (cleared && sessionId) {
+          updateSessionState(sessionId, state => (state.needsInput ? { ...state, needsInput: false } : state))
+        }
       } else if (event.type === 'approval.request') {
         // Dangerous-command / execute_code approval. The Python side is blocked
         // in _await_gateway_decision() until approval.respond lands; without

apps/desktop/src/app/session/hooks/use-session-actions/index.ts

@@ -8,6 +8,7 @@ import { useI18n } from '@/i18n'
 import { type ChatMessage, preserveLocalAssistantErrors, toChatMessages } from '@/lib/chat-messages'
 import { isMissingRpcMethod } from '@/lib/gateway-rpc'
 import { setSessionYolo } from '@/lib/yolo-session'
+import { rearmPendingPrompts } from '@/store/clarify'
 import { migrateSessionDraft } from '@/store/composer'
 import { clearQueuedPrompts, migrateQueuedPrompts } from '@/store/composer-queue'
 import { $pinnedSessionIds } from '@/store/layout'
@@ -733,6 +734,11 @@ export function useSessionActions({
               sessionStateByRuntimeIdRef.current.delete(cachedRuntimeId)
               dropSessionState(cachedRuntimeId)
             } else {
+              // Same re-arm as the cold path: session.activate is how a
+              // reopened window attaches to an already-live session, which is
+              // exactly the case where the one-shot clarify.request was missed.
+              rearmPendingPrompts(cachedRuntimeId, activated.pending_prompts)
+
               const runtimeInfo = applyRuntimeInfo(activated.info)
 
               let activatedMessages =
@@ -951,6 +957,13 @@ export function useSessionActions({
 
         setActiveSessionId(resumed.session_id)
         activeSessionIdRef.current = resumed.session_id
+        // Re-arm anything the session is still blocked on. `clarify.request` is
+        // a one-shot event, so a window that opened after the tool blocked has
+        // no live request to answer with — the inline card would render the
+        // stored question and then be unable to resolve it. A cold resume mints
+        // a fresh runtime id and so normally carries nothing here; the payload
+        // is populated on the reuse-live path a genuinely blocked session takes.
+        rearmPendingPrompts(resumed.session_id, resumed.pending_prompts)
         const runtimeInfo = applyRuntimeInfo(resumed.info)
 
         patchSessionWorkspace(storedSessionId, runtimeInfo?.cwd)

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

@@ -0,0 +1,140 @@
+import type { ToolCallMessagePartProps } from '@assistant-ui/react'
+import { cleanup, render, screen } from '@testing-library/react'
+import type { ReactNode } from 'react'
+import { afterEach, describe, expect, it, vi } from 'vitest'
+
+import { I18nProvider } from '@/i18n'
+import { clearClarifyRequest, setClarifyRequest } from '@/store/clarify'
+import { $gateway } from '@/store/gateway'
+import { $activeSessionId } from '@/store/session'
+
+import { clarifyRequestMatchesRow, ClarifyTool } from './clarify-tool'
+
+// The turn is NOT running — the state a `session.resume` leaves behind while a
+// clarify is still parked on the session (resume sets busy true on entry, then
+// back to the resumed `running`, which the deferred-resume path reports as
+// false). The card must survive that.
+vi.mock('@assistant-ui/react', () => ({
+  useAuiState: () => false
+}))
+
+afterEach(() => {
+  cleanup()
+  clearClarifyRequest()
+  $activeSessionId.set(null)
+  $gateway.set(null)
+  vi.clearAllMocks()
+})
+
+function renderClarify(ui: ReactNode) {
+  return render(
+    <I18nProvider configClient={null} initialLocale="en">
+      {ui}
+    </I18nProvider>
+  )
+}
+
+function clarifyProps(): ToolCallMessagePartProps {
+  const args = { choices: ['staging', 'production'], 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'
+  }
+}
+
+describe('ClarifyTool lifetime is owned by the request, not the running turn', () => {
+  it('stays answerable while a request is parked even though the turn reads not-running', () => {
+    $activeSessionId.set('session-1')
+    $gateway.set({ request: vi.fn().mockResolvedValue({ ok: true }) } as never)
+    setClarifyRequest({
+      choices: ['staging', 'production'],
+      question: 'Which deployment target?',
+      requestId: 'req-live',
+      sessionId: 'session-1'
+    })
+
+    renderClarify(<ClarifyTool {...clarifyProps()} />)
+
+    // The interactive panel, not the inert ToolFallback row.
+    expect(screen.getByText('Which deployment target?')).toBeTruthy()
+    expect(screen.getAllByRole('button', { name: /staging/ }).length).toBeGreaterThan(0)
+  })
+
+  it('falls back to the inert row once the request is gone and the turn is not running', () => {
+    $activeSessionId.set('session-1')
+
+    renderClarify(<ClarifyTool {...clarifyProps()} />)
+
+    // No live request → nothing to answer with; the panel must not offer
+    // choices that can no longer resolve anything.
+    expect(screen.queryByRole('button', { name: /staging/ })).toBeNull()
+  })
+
+  it('ignores a request parked on a different session', () => {
+    $activeSessionId.set('session-1')
+    setClarifyRequest({
+      choices: ['staging', 'production'],
+      question: 'Which deployment target?',
+      requestId: 'req-other',
+      sessionId: 'session-2'
+    })
+
+    renderClarify(<ClarifyTool {...clarifyProps()} />)
+
+    expect(screen.queryByRole('button', { name: /staging/ })).toBeNull()
+  })
+
+  it('does not claim a NEWER request belonging to a different row', () => {
+    // Two clarifies in one transcript: this row is the older, un-resulted one
+    // (interrupted or expired), and a different question is now live. The old
+    // row must settle inertly — claiming the live request would leave it stuck
+    // on a spinner forever, since the pending panel rejects the mismatch.
+    $activeSessionId.set('session-1')
+    setClarifyRequest({
+      choices: ['yes', 'no'],
+      question: 'A completely different question?',
+      requestId: 'req-new',
+      sessionId: 'session-1'
+    })
+
+    const { container } = renderClarify(<ClarifyTool {...clarifyProps()} />)
+
+    expect(screen.queryByRole('button', { name: /staging/ })).toBeNull()
+    expect(container.querySelector('[role="status"]')).toBeNull()
+  })
+})
+
+describe('clarifyRequestMatchesRow', () => {
+  const request = { question: 'Which deployment target?', requestId: 'req-1' }
+
+  it('matches on request id even when the questions differ', () => {
+    // A row hydrated straight from clarify.request carries the request id as
+    // its tool call id; the args may not have landed yet.
+    expect(clarifyRequestMatchesRow(request, 'req-1', undefined)).toBe(true)
+    expect(clarifyRequestMatchesRow(request, 'req-1', 'stale text')).toBe(true)
+  })
+
+  it('matches on question when the ids differ', () => {
+    // The tool.start row carries the model's own tool_call_id, so ids never
+    // line up and the question is the only correlator.
+    expect(clarifyRequestMatchesRow(request, 'call-abc', 'Which deployment target?')).toBe(true)
+  })
+
+  it('rejects a different question on a different id', () => {
+    expect(clarifyRequestMatchesRow(request, 'call-abc', 'Some other question?')).toBe(false)
+  })
+
+  it('rejects when there is no request at all', () => {
+    expect(clarifyRequestMatchesRow(null, 'req-1', 'Which deployment target?')).toBe(false)
+  })
+})

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

@@ -197,10 +197,65 @@ export const ClarifyTool = (props: ToolCallMessagePartProps) => {
   return <ClarifyToolLive {...props} />
 }
 
+/**
+ * Whether `request` is the one THIS row is showing.
+ *
+ * A transcript can hold several clarify rows, so "the session has a clarify
+ * parked" is not enough to decide a given row is live — an older, un-resulted
+ * row would otherwise claim a newer request and then reject it downstream,
+ * leaving a spinner with no exit. Correlate on the request id first (a row
+ * hydrated from `clarify.request` carries it as its tool call id), then on the
+ * question text (the `tool.start` row carries the model's own tool_call_id, so
+ * ids differ and only the args can match them up).
+ */
+export function clarifyRequestMatchesRow(
+  request: { question: string; requestId: string } | null,
+  toolCallId: string | undefined,
+  argsQuestion: string | undefined
+): boolean {
+  if (!request) {
+    return false
+  }
+
+  if (toolCallId && request.requestId && toolCallId === request.requestId) {
+    return true
+  }
+
+  // No question on either side to compare — fall back to accepting, which
+  // preserves the pre-correlation behaviour for rows whose args never arrived.
+  if (!argsQuestion || !request.question) {
+    return true
+  }
+
+  return argsQuestion === request.question
+}
+
 function ClarifyToolLive(props: ToolCallMessagePartProps) {
   const messageRunning = useAuiState(selectMessageRunning)
+  // The clarify parked on the session this row belongs to. Its presence — not
+  // the thread's running flag — is what makes the card answerable.
+  const sessionId = useStore(useSessionView().$runtimeId)
+  const $request = useMemo(() => sessionClarifyRequest(sessionId), [sessionId])
+  const request = useStore($request)
+  const fromArgs = useMemo(() => readClarifyArgs(props.args), [props.args])
+
+  // A live request for THIS row outranks `messageRunning`. The thread's running
+  // flag is a renderer-side turn indicator, and a `session.resume` deliberately
+  // clears it (use-session-actions sets busy=true on entry, then back to the
+  // resumed `running`, which the deferred-resume path reports as false). Gating
+  // the panel on it tore a still-live question off the screen the moment the
+  // user clicked into the chat — the box appeared, then vanished, with the agent
+  // still blocked on `clarify.respond`. The card now lives and dies with its own
+  // request: answered, skipped, expired, or interrupted.
+  if (clarifyRequestMatchesRow(request, props.toolCallId, fromArgs.question)) {
+    return <ClarifyToolPending {...props} />
+  }
 
-  // Stopped mid-prompt with no result — don't leave a dead interactive panel.
+  // No live request of our own — either this row is a stale earlier clarify, or
+  // the turn was stopped mid-prompt. Don't leave a dead interactive panel.
+  // `messageRunning` still covers the tool.start → clarify.request race, where
+  // the row mounts a tick before the request lands (ClarifyToolPending holds a
+  // spinner until it does).
   if (!messageRunning) {
     return <ToolFallback {...props} />
   }

apps/desktop/src/lib/gateway-events.ts

@@ -21,6 +21,10 @@ const UNSCOPED_STREAM_EVENT_TYPES = new Set([
   'approval.request',
   'browser.progress',
   'clarify.request',
+  // Pairs with clarify.request: the expiry must reach the same pinned session
+  // even after the user switches chats mid-turn, or a background chat's dead
+  // card never settles.
+  'clarify.expire',
   'error',
   'message.complete',
   'message.delta',

apps/desktop/src/store/clarify-rearm.test.ts

@@ -0,0 +1,63 @@
+import { afterEach, describe, expect, it, vi } from 'vitest'
+
+import { $clarifyRequests, clearClarifyRequest, rearmPendingPrompts } from './clarify'
+
+afterEach(() => {
+  clearClarifyRequest()
+  vi.restoreAllMocks()
+})
+
+const parked = (sessionId: string) => Boolean($clarifyRequests.get()[sessionId])
+
+const clarifyPrompt = (payload: Record<string, unknown>) => ({ event: 'clarify.request', payload })
+
+describe('rearmPendingPrompts', () => {
+  it('restores a clarify the window never saw announced', () => {
+    rearmPendingPrompts(
+      'session-1',
+      [clarifyPrompt({ choices: ['a', 'b'], question: 'Ship it?', request_id: 'req-1' })]
+    )
+
+    const restored = $clarifyRequests.get()['session-1']
+
+    expect(restored).toMatchObject({
+      choices: ['a', 'b'],
+      question: 'Ship it?',
+      requestId: 'req-1',
+      sessionId: 'session-1'
+    })
+  })
+
+  it('is a no-op for an empty or absent list', () => {
+    rearmPendingPrompts('session-1', undefined)
+    rearmPendingPrompts('session-1', [])
+
+    expect(parked('session-1')).toBe(false)
+  })
+
+  it('ignores blocking prompts it does not own', () => {
+    rearmPendingPrompts('session-1', [
+      { event: 'sudo.request', payload: { question: 'password?', request_id: 'req-sudo' } },
+      { event: 'secret.request', payload: { question: 'token?', request_id: 'req-secret' } }
+    ])
+
+    expect(parked('session-1')).toBe(false)
+  })
+
+  it('skips a payload missing the request id or question — an unanswerable card is worse than none', () => {
+    rearmPendingPrompts('session-1', [
+      clarifyPrompt({ question: 'no id', request_id: '' }),
+      clarifyPrompt({ question: '', request_id: 'req-2' })
+    ])
+
+    expect(parked('session-1')).toBe(false)
+  })
+
+  it('falls back to free text when every choice normalizes away', () => {
+    vi.spyOn(console, 'warn').mockImplementation(() => {})
+
+    rearmPendingPrompts('session-1', [clarifyPrompt({ choices: ['', '   '], question: 'Ship it?', request_id: 'req-3' })])
+
+    expect($clarifyRequests.get()['session-1'].choices).toBeNull()
+  })
+})