From 0d3933c2c94a41a5971a0eba01c08b28b303b0b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Mon, 20 Apr 2026 22:27:00 +0800 Subject: [PATCH] Keep desktop AskUserQuestion flows from stalling in plan mode Desktop chat was rendering AskUserQuestion twice: once as an inline question card and again as a generic permission request. That split also dropped the structured answers on the floor because the websocket permission_response shape only carried allow/deny state. This change keeps AskUserQuestion on the permission pipeline end-to-end. The desktop websocket contract now carries toolUseId and updatedInput, AskUserQuestion submits answers through permission_response, and the generic permission card is suppressed for that tool so the user sees a single question flow. Constraint: AskUserQuestion answers must round-trip through updatedInput.answers for the CLI tool contract to complete Rejected: Leave AskUserQuestion as a plain chat reply in desktop | the tool never receives structured answers and the pending approval UI remains stuck Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep AskUserQuestion bound to the permission-response path unless the desktop protocol grows a separate structured elicitation channel Tested: desktop lint; vitest src/components/chat/AskUserQuestion.test.tsx src/stores/chatStore.test.ts; bun test src/server/__tests__/conversations.test.ts Not-tested: Manual desktop click-through against a live plan-mode session after bundling --- .../components/chat/AskUserQuestion.test.tsx | 109 ++++++++++++++++++ .../src/components/chat/AskUserQuestion.tsx | 56 +++++++-- desktop/src/components/chat/MessageList.tsx | 1 + .../src/components/chat/PermissionDialog.tsx | 2 +- desktop/src/stores/chatStore.test.ts | 68 +++++++++++ desktop/src/stores/chatStore.ts | 46 ++++++-- desktop/src/types/chat.ts | 28 ++++- src/server/services/conversationService.ts | 23 +++- src/server/ws/events.ts | 17 ++- src/server/ws/handler.ts | 5 + 10 files changed, 327 insertions(+), 28 deletions(-) create mode 100644 desktop/src/components/chat/AskUserQuestion.test.tsx diff --git a/desktop/src/components/chat/AskUserQuestion.test.tsx b/desktop/src/components/chat/AskUserQuestion.test.tsx new file mode 100644 index 00000000..4a8686f1 --- /dev/null +++ b/desktop/src/components/chat/AskUserQuestion.test.tsx @@ -0,0 +1,109 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { fireEvent, render, screen } from '@testing-library/react' + +const { sendMock } = vi.hoisted(() => ({ + sendMock: vi.fn(), +})) + +vi.mock('../../api/websocket', () => ({ + wsManager: { + connect: vi.fn(), + disconnect: vi.fn(), + onMessage: vi.fn(() => () => {}), + clearHandlers: vi.fn(), + send: sendMock, + }, +})) + +vi.mock('../../api/sessions', () => ({ + sessionsApi: { + getMessages: vi.fn(async () => ({ messages: [] })), + getSlashCommands: vi.fn(async () => ({ commands: [] })), + }, +})) + +import { AskUserQuestion } from './AskUserQuestion' +import { useChatStore } from '../../stores/chatStore' +import { useTabStore } from '../../stores/tabStore' + +const ACTIVE_TAB = 'active-tab' + +describe('AskUserQuestion', () => { + beforeEach(() => { + sendMock.mockReset() + useTabStore.setState({ + activeTabId: ACTIVE_TAB, + tabs: [{ sessionId: ACTIVE_TAB, title: 'Test', type: 'session', status: 'idle' }], + }) + useChatStore.setState({ + sessions: { + [ACTIVE_TAB]: { + messages: [], + chatState: 'permission_pending', + connectionState: 'connected', + streamingText: '', + streamingToolInput: '', + activeToolUseId: null, + activeToolName: null, + activeThinkingId: null, + pendingPermission: { + requestId: 'perm-1', + toolName: 'AskUserQuestion', + toolUseId: 'tool-1', + input: { + questions: [ + { + question: 'Should we persist data?', + options: [{ label: 'No' }, { label: 'Yes' }], + }, + ], + }, + }, + pendingComputerUsePermission: null, + tokenUsage: { input_tokens: 0, output_tokens: 0 }, + elapsedSeconds: 0, + statusVerb: '', + slashCommands: [], + agentTaskNotifications: {}, + elapsedTimer: null, + }, + }, + }) + }) + + it('submits answers through permission_response updatedInput instead of sending a chat message', () => { + render( + , + ) + + fireEvent.click(screen.getByRole('button', { name: /^No$/ })) + fireEvent.click(screen.getByRole('button', { name: /submit/i })) + + expect(sendMock).toHaveBeenCalledWith(ACTIVE_TAB, { + type: 'permission_response', + requestId: 'perm-1', + allowed: true, + updatedInput: { + questions: [ + { + question: 'Should we persist data?', + options: [{ label: 'No' }, { label: 'Yes' }], + }, + ], + answers: { + 'Should we persist data?': 'No', + }, + }, + }) + }) +}) diff --git a/desktop/src/components/chat/AskUserQuestion.tsx b/desktop/src/components/chat/AskUserQuestion.tsx index 1e0fd0ad..67006926 100644 --- a/desktop/src/components/chat/AskUserQuestion.tsx +++ b/desktop/src/components/chat/AskUserQuestion.tsx @@ -1,4 +1,4 @@ -import { useState, useRef } from 'react' +import { useMemo, useRef, useState } from 'react' import { useChatStore } from '../../stores/chatStore' import { useTabStore } from '../../stores/tabStore' import { useTranslation } from '../../i18n' @@ -24,6 +24,7 @@ type AskUserInput = { type Props = { toolUseId: string input: unknown + result?: unknown } /** @@ -46,19 +47,41 @@ function parseInput(input: unknown): Question[] { return [] } -export function AskUserQuestion({ toolUseId: _toolUseId, input }: Props) { - const { sendMessage } = useChatStore() +export function AskUserQuestion({ toolUseId, input, result }: Props) { + const { respondToPermission } = useChatStore() const activeTabId = useTabStore((s) => s.activeTabId) + const pendingPermission = useChatStore((s) => activeTabId ? s.sessions[activeTabId]?.pendingPermission : undefined) const t = useTranslation() const questions = parseInput(input) + const inputObject = (input && typeof input === 'object') ? input as Record : {} const [activeTab, setActiveTab] = useState(0) const [selections, setSelections] = useState>({}) const [freeText, setFreeText] = useState('') - const [submitted, setSubmitted] = useState(false) + const [hasSubmitted, setHasSubmitted] = useState(false) const composingRef = useRef(false) if (questions.length === 0) return null + const resultAnswers = useMemo(() => { + if (!result || typeof result !== 'object') return {} + const answers = (result as { answers?: unknown }).answers + return answers && typeof answers === 'object' + ? answers as Record + : {} + }, [result]) + + const pendingRequest = pendingPermission?.toolUseId === toolUseId ? pendingPermission : null + const answeredText = useMemo(() => { + if (Object.keys(resultAnswers).length > 0) { + return questions + .map((question) => resultAnswers[question.question]) + .filter((answer): answer is string => typeof answer === 'string' && answer.trim().length > 0) + .join(', ') + } + return freeText.trim() || Object.values(selections).join(', ') + }, [freeText, questions, resultAnswers, selections]) + const submitted = Object.keys(resultAnswers).length > 0 || hasSubmitted + const handleSelect = (qIndex: number, label: string) => { if (submitted) return setSelections((prev) => { @@ -84,9 +107,24 @@ export function AskUserQuestion({ toolUseId: _toolUseId, input }: Props) { const response = freeText.trim() || parts.join('; ') || '' if (!response) return - setSubmitted(true) - if (!activeTabId) return - sendMessage(activeTabId, response) + if (!activeTabId || !pendingRequest) return + + const answers = questions.reduce>((acc, question, index) => { + if (freeText.trim()) { + acc[question.question] = freeText.trim() + } else if (selections[index]) { + acc[question.question] = selections[index]! + } + return acc + }, {}) + + setHasSubmitted(true) + respondToPermission(activeTabId, pendingRequest.requestId, true, { + updatedInput: { + ...inputObject, + answers, + }, + }) } // All questions must be answered (via selection or free text) to enable submit @@ -241,7 +279,7 @@ export function AskUserQuestion({ toolUseId: _toolUseId, input }: Props) {
check_circle - {t('question.answeredPrefix')}{freeText.trim() || Object.values(selections).join(', ')} + {t('question.answeredPrefix')}{answeredText}
)} @@ -253,7 +291,7 @@ export function AskUserQuestion({ toolUseId: _toolUseId, input }: Props) {