diff --git a/desktop/src/components/chat/AskUserQuestion.test.tsx b/desktop/src/components/chat/AskUserQuestion.test.tsx index e4b8855a..0dee4aa0 100644 --- a/desktop/src/components/chat/AskUserQuestion.test.tsx +++ b/desktop/src/components/chat/AskUserQuestion.test.tsx @@ -400,4 +400,88 @@ describe('AskUserQuestion', () => { expect(screen.queryByRole('button', { name: /submit/i })).toBeNull() expect(screen.getByText(/Tool permission request failed: AbortError/)).toBeTruthy() }) + + describe('chat about this', () => { + const SCOPE_INPUT = { + questions: [ + { + question: 'Which scope?', + options: [{ label: 'Single page' }, { label: 'Tabs' }], + }, + ], + } + + // The whole point of the button: you reach for it precisely when none of + // the options fit, which is when nothing is selected. Gating it on + // `allAnswered` like Submit would make it unreachable in its own use case. + it('stays enabled with nothing selected, unlike submit', () => { + render() + + expect(screen.getByRole('button', { name: /submit/i })).toHaveProperty('disabled', true) + expect(screen.getByRole('button', { name: /chat about this/i })).toHaveProperty( + 'disabled', + false, + ) + }) + + it('denies the permission so the text reaches the model, rather than answering', () => { + render() + + fireEvent.click(screen.getByRole('button', { name: /chat about this/i })) + + expect(sendMock).toHaveBeenCalledWith(ACTIVE_TAB, { + type: 'permission_response', + requestId: 'perm-1', + allowed: false, + denyMessage: '- "Which scope?"\n (No answer provided)', + }) + }) + + it('carries answers already filled in so the handoff does not discard them', () => { + render() + + fireEvent.click(screen.getByRole('button', { name: /^Tabs$/ })) + fireEvent.click(screen.getByRole('button', { name: /chat about this/i })) + + expect(sendMock).toHaveBeenCalledWith(ACTIVE_TAB, { + type: 'permission_response', + requestId: 'perm-1', + allowed: false, + denyMessage: '- "Which scope?"\n Answer: Tabs', + }) + }) + + it('reports the handoff instead of claiming the question was answered', () => { + render() + + fireEvent.click(screen.getByRole('button', { name: /^Tabs$/ })) + fireEvent.click(screen.getByRole('button', { name: /chat about this/i })) + + expect(screen.getByText(/Handed back to Claude/)).toBeTruthy() + expect(screen.queryByText(/Answered:/)).toBeNull() + expect(screen.queryByRole('button', { name: /chat about this/i })).toBeNull() + }) + + // Regression: the status badge is rendered from its own branch, so it kept + // reading "Answered" after a handoff even while the body said otherwise. + it('does not badge the handoff as answered', () => { + render() + + fireEvent.click(screen.getByRole('button', { name: /^Tabs$/ })) + fireEvent.click(screen.getByRole('button', { name: /chat about this/i })) + + expect(screen.getByText('Handed off')).toBeTruthy() + expect(screen.queryByText('Answered')).toBeNull() + }) + + it('ignores a second click once the handoff is sent', () => { + render() + + const chatButton = screen.getByRole('button', { name: /chat about this/i }) + fireEvent.click(chatButton) + fireEvent.click(chatButton) + + expect(sendMock).toHaveBeenCalledTimes(1) + }) + }) }) diff --git a/desktop/src/components/chat/AskUserQuestion.tsx b/desktop/src/components/chat/AskUserQuestion.tsx index 59125419..430bb46c 100644 --- a/desktop/src/components/chat/AskUserQuestion.tsx +++ b/desktop/src/components/chat/AskUserQuestion.tsx @@ -79,6 +79,7 @@ export function AskUserQuestion({ sessionId, toolUseId, input, result }: Props) const [selections, setSelections] = useState({}) const [freeTexts, setFreeTexts] = useState({}) const [hasSubmitted, setHasSubmitted] = useState(false) + const [hasRequestedChat, setHasRequestedChat] = useState(false) const composingRef = useRef(false) if (questions.length === 0) return null @@ -109,7 +110,7 @@ export function AskUserQuestion({ sessionId, toolUseId, input, result }: Props) .filter(Boolean) .join('; ') }, [freeTexts, hasStructuredAnswers, questions, resultAnswers, resultText, selections]) - const submitted = hasTerminalResult || hasSubmitted + const submitted = hasTerminalResult || hasSubmitted || hasRequestedChat const terminalWithoutAnswers = submitted && !hasStructuredAnswers && resultText.length > 0 const handleSelect = (qIndex: number, label: string) => { @@ -198,6 +199,37 @@ export function AskUserQuestion({ sessionId, toolUseId, input, result }: Props) }) } + /** + * Hands the questions back to the model as a conversation instead of an + * answer — the user doesn't think any option fits and wants to talk first. + * + * Travels as a denial because that's the only channel that carries free text + * back to the model, but the server rewrites it (buildDenyMessage) into + * "ask them what they'd like to clarify" rather than the usual "STOP and + * wait". Deliberately not gated on `allAnswered`: not recognising your own + * question in any of the options is exactly when nothing is filled in. + */ + const handleChatAboutThis = () => { + if (submitted) return + if (!targetSessionId || !pendingRequest) return + + // Carry whatever was already picked, so switching to a conversation isn't + // punished by losing the partial answers. + const questionsWithAnswers = questions + .map((question, index) => { + const answer = freeTexts[index]?.trim() || getSelectedAnswer(question, selections[index]) + return answer + ? `- "${question.question}"\n Answer: ${answer}` + : `- "${question.question}"\n (No answer provided)` + }) + .join('\n') + + setHasRequestedChat(true) + respondToPermission(targetSessionId, pendingRequest.requestId, false, { + denyMessage: questionsWithAnswers, + }) + } + // All questions must be answered (via selection or free text) to enable submit const allAnswered = questions.every((_, i) => Boolean(freeTexts[i]?.trim()) || (selections[i]?.length ?? 0) > 0, @@ -228,7 +260,11 @@ export function AskUserQuestion({ sessionId, toolUseId, input, result }: Props) {submitted && ( - {t(terminalWithoutAnswers ? 'question.completed' : 'question.answered')} + {/* handing the question back is not an answer — saying "answered" + there misreports what the user did */} + {t(hasRequestedChat + ? 'question.chatBadge' + : terminalWithoutAnswers ? 'question.completed' : 'question.answered')} )} @@ -347,20 +383,27 @@ export function AskUserQuestion({ sessionId, toolUseId, input, result }: Props) )} - {/* Submitted answer display */} - {submitted && ( + {/* Submitted answer display — the chat handoff wins over any terminal + result, whose text is the deny payload and not worth showing. */} + {submitted && (hasRequestedChat ? ( +
+ forum + {t('question.chatRequested')} +
+ ) : (
check_circle {t(terminalWithoutAnswers ? 'question.resultPrefix' : 'question.answeredPrefix')}{answeredText}
- )} + ))} - {/* Submit button */} + {/* Action bar. Wraps rather than overflows: two buttons plus a translated + label (kr/jp run long) can outgrow a narrow side-by-side pane. */} {!submitted && ( -
+
+
)}
diff --git a/desktop/src/i18n/locales/en.ts b/desktop/src/i18n/locales/en.ts index ca5f5102..f42b80e9 100644 --- a/desktop/src/i18n/locales/en.ts +++ b/desktop/src/i18n/locales/en.ts @@ -1839,6 +1839,10 @@ Row 9, all 8 cells: continuing from straight down, turning left through lower-le 'question.submit': 'Submit', 'question.answeredPrefix': 'Answered: ', 'question.resultPrefix': 'Result: ', + 'question.chatAboutThis': 'Chat about this', + 'question.chatAboutThisHint': 'None of these fit? Hand it back to Claude to follow up — anything already filled in comes along.', + 'question.chatRequested': 'Handed back to Claude, which will follow up', + 'question.chatBadge': 'Handed off', // ─── Thinking Block ────────────────────────────────────── 'thinking.label': 'Thinking', diff --git a/desktop/src/i18n/locales/jp.ts b/desktop/src/i18n/locales/jp.ts index 7f1776ea..d9e944a4 100644 --- a/desktop/src/i18n/locales/jp.ts +++ b/desktop/src/i18n/locales/jp.ts @@ -1841,6 +1841,10 @@ export const jp: Record = { 'question.submit': '送信', 'question.answeredPrefix': '回答: ', 'question.resultPrefix': '結果: ', + 'question.chatAboutThis': 'Claude と相談する', + 'question.chatAboutThisHint': '選択肢が合わない場合は Claude に聞き返してもらいます。入力済みの回答も一緒に渡されます', + 'question.chatRequested': '相談に切り替えました。Claude が確認します', + 'question.chatBadge': '相談中', // ─── Thinking Block ────────────────────────────────────── 'thinking.label': '思考中', diff --git a/desktop/src/i18n/locales/kr.ts b/desktop/src/i18n/locales/kr.ts index 36951047..740f669b 100644 --- a/desktop/src/i18n/locales/kr.ts +++ b/desktop/src/i18n/locales/kr.ts @@ -1841,6 +1841,10 @@ export const kr: Record = { 'question.submit': '제출', 'question.answeredPrefix': '응답: ', 'question.resultPrefix': '결과: ', + 'question.chatAboutThis': 'Claude와 상의하기', + 'question.chatAboutThisHint': '적합한 선택지가 없나요? Claude가 다시 물어보도록 넘깁니다. 이미 입력한 답변도 함께 전달됩니다', + 'question.chatRequested': '대화로 전환했습니다. Claude가 이어서 물어봅니다', + 'question.chatBadge': '상의 중', // ─── Thinking Block ────────────────────────────────────── 'thinking.label': '사고 중', diff --git a/desktop/src/i18n/locales/zh-TW.ts b/desktop/src/i18n/locales/zh-TW.ts index d262e0d2..e90ac5bf 100644 --- a/desktop/src/i18n/locales/zh-TW.ts +++ b/desktop/src/i18n/locales/zh-TW.ts @@ -1840,6 +1840,10 @@ export const zh: Record = { 'question.submit': '提交', 'question.answeredPrefix': '已回答: ', 'question.resultPrefix': '結果: ', + 'question.chatAboutThis': '和 Claude 聊聊', + 'question.chatAboutThisHint': '選項都不合適？交給 Claude 追問，已填的答案會一併帶上', + 'question.chatRequested': '已轉為對話，Claude 會接著追問', + 'question.chatBadge': '已轉對話', // ─── Thinking Block ────────────────────────────────────── 'thinking.label': '思考中', diff --git a/desktop/src/i18n/locales/zh.ts b/desktop/src/i18n/locales/zh.ts index 41e5416c..ea1c22ae 100644 --- a/desktop/src/i18n/locales/zh.ts +++ b/desktop/src/i18n/locales/zh.ts @@ -1840,6 +1840,10 @@ export const zh: Record = { 'question.submit': '提交', 'question.answeredPrefix': '已回答: ', 'question.resultPrefix': '结果: ', + 'question.chatAboutThis': '和 Claude 聊聊', + 'question.chatAboutThisHint': '选项都不合适？交给 Claude 追问，已填的答案会一并带上', + 'question.chatRequested': '已转为对话，Claude 会接着追问', + 'question.chatBadge': '已转对话', // ─── Thinking Block ────────────────────────────────────── 'thinking.label': '思考中', diff --git a/src/components/permissions/AskUserQuestionPermissionRequest/AskUserQuestionPermissionRequest.tsx b/src/components/permissions/AskUserQuestionPermissionRequest/AskUserQuestionPermissionRequest.tsx index 38719156..3e50675e 100644 --- a/src/components/permissions/AskUserQuestionPermissionRequest/AskUserQuestionPermissionRequest.tsx +++ b/src/components/permissions/AskUserQuestionPermissionRequest/AskUserQuestionPermissionRequest.tsx @@ -1,6 +1,7 @@ import { c as _c } from "react/compiler-runtime"; import type { Base64ImageSource, ImageBlockParam } from '@anthropic-ai/sdk/resources/messages.mjs'; import React, { Suspense, use, useCallback, useMemo, useRef, useState } from 'react'; +import { ASK_USER_QUESTION_CLARIFY_WITH_QUESTIONS_PREFIX } from '../../../constants/messages.js'; import { useSettings } from '../../../hooks/useSettings.js'; import { useTerminalSize } from '../../../hooks/useTerminalSize.js'; import { stringWidth } from '../../../ink/stringWidth.js'; @@ -298,12 +299,7 @@ function AskUserQuestionPermissionRequestBody(t0) { } return `- "${q_1.question}"\n (No answer provided)`; }).join("\n"); - const feedback = `The user wants to clarify these questions. - This means they may have additional information, context or questions for you. - Take their response into account and then reformulate the questions if appropriate. - Start by asking them what they would like to clarify. - - Questions asked:\n${questionsWithAnswers}`; + const feedback = `${ASK_USER_QUESTION_CLARIFY_WITH_QUESTIONS_PREFIX}${questionsWithAnswers}`; if (metadataSource) { logEvent("tengu_ask_user_question_respond_to_claude", { source: metadataSource as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, diff --git a/src/constants/messages.ts b/src/constants/messages.ts index eaeeee97..8dfb86ac 100644 --- a/src/constants/messages.ts +++ b/src/constants/messages.ts @@ -19,3 +19,21 @@ export const PLAN_REJECTION_MESSAGE = 'The user rejected this plan and chose to stay in plan mode. Do not start implementing. Revise the plan and present it again for approval.' export const PLAN_REJECTION_WITH_REASON_PREFIX = 'The user rejected this plan and chose to stay in plan mode. Do not start implementing. Revise the plan to address this feedback and present it again for approval. The user said:\n' + +// AskUserQuestion's "chat about this" is the third denial shape: the user isn't +// rejecting the tool, they want to talk the questions over before answering. +// Both of the messages above send the wrong instruction here — REJECT_MESSAGE +// tells the model to stop and wait, PLAN_REJECTION tells it to revise a plan — +// so the model has to be told to open the conversation itself. Without that, +// the button hands the user a silent turn and they have to prompt twice. +// The wording is shared with the CLI's AskUserQuestionPermissionRequest so both +// surfaces put the same instruction in front of the model. +export const ASK_USER_QUESTION_CLARIFY_MESSAGE = `The user wants to clarify these questions. + This means they may have additional information, context or questions for you. + Take their response into account and then reformulate the questions if appropriate. + Start by asking them what they would like to clarify.` +// Suffixed with the questions asked so far (and any answers already filled in), +// so switching to a conversation doesn't throw away the partial answers. +export const ASK_USER_QUESTION_CLARIFY_WITH_QUESTIONS_PREFIX = `${ASK_USER_QUESTION_CLARIFY_MESSAGE} + + Questions asked:\n` diff --git a/src/server/__tests__/conversations.test.ts b/src/server/__tests__/conversations.test.ts index c40d8a4e..8e5b66ba 100644 --- a/src/server/__tests__/conversations.test.ts +++ b/src/server/__tests__/conversations.test.ts @@ -21,6 +21,7 @@ import { conversationService, } from '../services/conversationService.js' import { + ASK_USER_QUESTION_CLARIFY_MESSAGE, PLAN_REJECTION_MESSAGE, PLAN_REJECTION_WITH_REASON_PREFIX, REJECT_MESSAGE, @@ -470,6 +471,32 @@ describe('ConversationService', () => { 'Do not start implementing', ) }) + + // "Chat about this" rides the deny channel but means the opposite of a + // denial: the model has to open the conversation, not stop and wait. + it('tells the model to ask what needs clarifying when a question is handed back', () => { + const message = buildDenyMessage('AskUserQuestion', undefined) + + expect(message).toBe(ASK_USER_QUESTION_CLARIFY_MESSAGE) + expect(message).toContain('ask') + expect(message).not.toContain('STOP what you are doing') + }) + + it('carries the partial answers back so handing off does not discard them', () => { + const answered = '- "Persist data?"\n Answer: Yes\n- "Which store?"\n (No answer provided)' + const message = buildDenyMessage('AskUserQuestion', answered) + + expect(message).toContain('Questions asked:') + expect(message).toContain('Answer: Yes') + expect(message).toContain('(No answer provided)') + expect(message).not.toContain('STOP what you are doing') + }) + + it('treats whitespace-only clarify feedback as no feedback', () => { + expect(buildDenyMessage('AskUserQuestion', ' \n ')).toBe( + ASK_USER_QUESTION_CLARIFY_MESSAGE, + ) + }) }) it('should let the model finish the turn when desktop denies a tool permission', () => { diff --git a/src/server/services/conversationService.ts b/src/server/services/conversationService.ts index e05f2de8..7b9197d6 100644 --- a/src/server/services/conversationService.ts +++ b/src/server/services/conversationService.ts @@ -36,6 +36,8 @@ import { resolveClaudeCliLauncher, } from '../../utils/desktopBundledCli.js' import { + ASK_USER_QUESTION_CLARIFY_MESSAGE, + ASK_USER_QUESTION_CLARIFY_WITH_QUESTIONS_PREFIX, PLAN_REJECTION_MESSAGE, PLAN_REJECTION_WITH_REASON_PREFIX, REJECT_MESSAGE, @@ -89,8 +91,9 @@ export function cliExitSeverity(code: number | null): 'info' | 'error' { * Builds the denial text the CLI hands to the model as tool_result content. * * The model reads this verbatim, so it has to carry the instruction the desktop - * UI can't: a plain tool denial means "stop and wait for me", while a rejected - * plan means "keep planning". Both renderers (the CLI's + * UI can't: a plain tool denial means "stop and wait for me", a rejected plan + * means "keep planning", and a question the user wants to talk over means "ask + * them what needs clarifying". Both plan renderers (the CLI's * renderToolUseRejectedMessage, the desktop's extractPlanPreview) read the plan * from the tool input, so nothing here needs to echo the plan back. */ @@ -104,6 +107,14 @@ export function buildDenyMessage( ? `${PLAN_REJECTION_WITH_REASON_PREFIX}${feedback}` : PLAN_REJECTION_MESSAGE } + // "Chat about this" is a denial only in transport terms — the user wants to + // keep talking, not to stop the turn. REJECT_MESSAGE's "STOP and wait" would + // contradict that and leave them staring at a silent turn. + if (toolName === 'AskUserQuestion') { + return feedback + ? `${ASK_USER_QUESTION_CLARIFY_WITH_QUESTIONS_PREFIX}${feedback}` + : ASK_USER_QUESTION_CLARIFY_MESSAGE + } return feedback ? `${REJECT_MESSAGE_WITH_REASON_PREFIX}${feedback}` : REJECT_MESSAGE