From 853e3db6ecbeb9a39f333c65aa7eddc32d613d7b 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: Sat, 23 May 2026 15:58:15 +0800 Subject: [PATCH] fix: keep AskUserQuestion state aligned with CLI answers The CLI transcript already records each AskUserQuestion once, but the desktop bridge dropped the structured toolUseResult answers from both history replay and live WebSocket tool results. That made answered question cards look unresolved and allowed stale cards to reappear during the gap before the next permission request. Constraint: Desktop should render the CLI question stream directly without inventing separate question lifecycle state. Rejected: Hide all previous AskUserQuestion cards after submit | would remove resolved answer history instead of preserving transcript context. Confidence: high Scope-risk: moderate Directive: Preserve toolUseResult metadata when adapting CLI transcript entries; AskUserQuestion answer state depends on the structured answers object, not the prose tool_result string. Tested: bun test src/server/__tests__/sessions.test.ts --timeout 30000 Tested: bun test src/server/__tests__/ws-memory-events.test.ts --timeout 30000 Tested: cd desktop && bun run test src/stores/chatStore.test.ts Tested: cd desktop && bun run test src/components/chat/MessageList.test.tsx Tested: bun run check:server Tested: cd desktop && bun run lint Tested: cd desktop && bun run build Tested: git diff --check Not-tested: Full bun run verify and live desktop browser smoke. --- .../src/components/chat/MessageList.test.tsx | 47 +++++++++++++++ desktop/src/components/chat/MessageList.tsx | 10 ++++ desktop/src/stores/chatStore.test.ts | 57 +++++++++++++++++++ desktop/src/stores/chatStore.ts | 20 ++++++- desktop/src/types/session.ts | 1 + src/server/__tests__/sessions.test.ts | 36 ++++++++++++ src/server/__tests__/ws-memory-events.test.ts | 33 +++++++++++ src/server/services/sessionService.ts | 2 + src/server/ws/handler.ts | 17 +++++- 9 files changed, 221 insertions(+), 2 deletions(-) diff --git a/desktop/src/components/chat/MessageList.test.tsx b/desktop/src/components/chat/MessageList.test.tsx index f03f1c18..bdd9cf4e 100644 --- a/desktop/src/components/chat/MessageList.test.tsx +++ b/desktop/src/components/chat/MessageList.test.tsx @@ -251,6 +251,53 @@ describe('MessageList nested tool calls', () => { )).toEqual(['answered-tool', 'active-tool']) }) + it('keeps only the latest unresolved AskUserQuestion when no pending permission is active', () => { + const messages: UIMessage[] = [ + { + id: 'first-ask', + type: 'tool_use', + toolName: 'AskUserQuestion', + toolUseId: 'first-tool', + input: { + questions: [ + { + question: 'First question?', + options: [{ label: 'A' }, { label: 'B' }], + }, + ], + }, + timestamp: 1, + }, + { + id: 'second-ask', + type: 'tool_use', + toolName: 'AskUserQuestion', + toolUseId: 'second-tool', + input: { + questions: [ + { + question: 'Second question?', + options: [{ label: 'A' }, { label: 'B' }], + }, + ], + }, + timestamp: 2, + }, + ] + + const { renderItems } = buildRenderModel(messages, null) + + expect(renderItems).toHaveLength(1) + expect(renderItems[0]).toMatchObject({ + kind: 'message', + message: { + type: 'tool_use', + toolName: 'AskUserQuestion', + toolUseId: 'second-tool', + }, + }) + }) + it('renders goal events as visible status cards', () => { useChatStore.setState({ sessions: { diff --git a/desktop/src/components/chat/MessageList.tsx b/desktop/src/components/chat/MessageList.tsx index 3b9da434..8d16578a 100644 --- a/desktop/src/components/chat/MessageList.tsx +++ b/desktop/src/components/chat/MessageList.tsx @@ -453,6 +453,7 @@ export function buildRenderModel(messages: UIMessage[], activeAskUserQuestionToo const childToolCallsByParent = new Map() const toolUseIds = new Set() const lastUnresolvedAskUserQuestionIndexByToolUseId = new Map() + let lastUnresolvedAskUserQuestionIndex: number | null = null let pendingToolCalls: ToolCall[] = [] const flushGroup = () => { @@ -490,6 +491,7 @@ export function buildRenderModel(messages: UIMessage[], activeAskUserQuestionToo !toolResultMap.has(msg.toolUseId) ) { lastUnresolvedAskUserQuestionIndexByToolUseId.set(msg.toolUseId, index) + lastUnresolvedAskUserQuestionIndex = index } }) @@ -527,6 +529,14 @@ export function buildRenderModel(messages: UIMessage[], activeAskUserQuestionToo ) { continue } + if ( + !isResolved && + !activeAskUserQuestionToolUseId && + lastUnresolvedAskUserQuestionIndex !== null && + messages[lastUnresolvedAskUserQuestionIndex] !== msg + ) { + continue + } flushGroup() items.push({ kind: 'message', message: msg }) } else { diff --git a/desktop/src/stores/chatStore.test.ts b/desktop/src/stores/chatStore.test.ts index 2e82f606..129452d2 100644 --- a/desktop/src/stores/chatStore.test.ts +++ b/desktop/src/stores/chatStore.test.ts @@ -223,6 +223,63 @@ describe('chatStore history mapping', () => { expect(mapped[3]).toMatchObject({ parentToolUseId: 'agent-1' }) }) + it('maps AskUserQuestion transcript answers from toolUseResult metadata', () => { + const messages: MessageEntry[] = [ + { + id: 'assistant-ask', + type: 'assistant', + timestamp: '2026-04-06T00:00:00.000Z', + content: [ + { + type: 'tool_use', + name: 'AskUserQuestion', + id: 'ask-1', + input: { + questions: [ + { + question: 'Pick one?', + options: [{ label: 'A' }, { label: 'B' }], + }, + ], + }, + }, + ], + }, + { + id: 'user-answer', + type: 'tool_result', + timestamp: '2026-04-06T00:00:01.000Z', + content: [ + { + type: 'tool_result', + tool_use_id: 'ask-1', + content: 'User has answered your questions: "Pick one?"="A". You can now continue with the user\'s answers in mind.', + }, + ], + toolUseResult: { + questions: [ + { + question: 'Pick one?', + options: [{ label: 'A' }, { label: 'B' }], + }, + ], + answers: { 'Pick one?': 'A' }, + }, + }, + ] + + const mapped = mapHistoryMessagesToUiMessages(messages) + + expect(mapped).toHaveLength(2) + expect(mapped[1]).toMatchObject({ + type: 'tool_result', + toolUseId: 'ask-1', + content: { + answers: { 'Pick one?': 'A' }, + }, + }) + }) + it('maps compact boundary and summary history without hiding pre-compact messages', () => { const messages: MessageEntry[] = [ { diff --git a/desktop/src/stores/chatStore.ts b/desktop/src/stores/chatStore.ts index 58782872..69d6d9db 100644 --- a/desktop/src/stores/chatStore.ts +++ b/desktop/src/stores/chatStore.ts @@ -1663,6 +1663,16 @@ function readRecord(value: unknown): Record | null { return value as Record } +function normalizeHistoryToolResultContent(content: unknown, toolUseResult: unknown): unknown { + const result = readRecord(toolUseResult) + const answers = readRecord(result?.answers) + if (!result || !answers || !Array.isArray(result.questions)) return content + return { + questions: result.questions, + answers, + } +} + function parseJsonRecord(value: unknown): Record | null { const record = readRecord(value) if (record) return record @@ -2313,7 +2323,15 @@ export function mapHistoryMessagesToUiMessages( } else if (block.type === 'image') attachments.push({ type: 'image', name: block.name || 'image', data: block.source?.data, mimeType: block.mimeType || block.media_type }) else if (block.type === 'file') attachments.push({ type: 'file', name: block.name || 'file' }) - else if (block.type === 'tool_result') uiMessages.push({ id: nextId(), type: 'tool_result', toolUseId: block.tool_use_id ?? '', content: block.content, isError: !!block.is_error, timestamp, parentToolUseId: msg.parentToolUseId }) + else if (block.type === 'tool_result') uiMessages.push({ + id: nextId(), + type: 'tool_result', + toolUseId: block.tool_use_id ?? '', + content: normalizeHistoryToolResultContent(block.content, msg.toolUseResult), + isError: !!block.is_error, + timestamp, + parentToolUseId: msg.parentToolUseId, + }) } if (textParts.length > 0 || attachments.length > 0) { const parsed = extractLeadingFileReferences(textParts.join('\n')) diff --git a/desktop/src/types/session.ts b/desktop/src/types/session.ts index 0d7940a6..d094ae6e 100644 --- a/desktop/src/types/session.ts +++ b/desktop/src/types/session.ts @@ -16,6 +16,7 @@ export type MessageEntry = { id: string type: 'user' | 'assistant' | 'system' | 'tool_use' | 'tool_result' content: unknown + toolUseResult?: unknown timestamp: string model?: string parentUuid?: string diff --git a/src/server/__tests__/sessions.test.ts b/src/server/__tests__/sessions.test.ts index cfb77ba1..f5cf545a 100644 --- a/src/server/__tests__/sessions.test.ts +++ b/src/server/__tests__/sessions.test.ts @@ -643,6 +643,42 @@ describe('SessionService', () => { expect(messages).toHaveLength(2) }) + it('preserves structured toolUseResult metadata for AskUserQuestion answers', async () => { + const sessionId = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + await writeSessionFile('-tmp-project', sessionId, [ + makeSnapshotEntry(), + { + type: 'user', + message: { + role: 'user', + content: [ + { + type: 'tool_result', + tool_use_id: 'ask-1', + content: 'User has answered your questions: "Pick one?"="A". You can now continue with the user\'s answers in mind.', + }, + ], + }, + toolUseResult: { + questions: [{ question: 'Pick one?', options: [{ label: 'A' }] }], + answers: { 'Pick one?': 'A' }, + }, + uuid: crypto.randomUUID(), + timestamp: '2026-01-01T00:00:01.000Z', + }, + ]) + + const messages = await service.getSessionMessages(sessionId) + + expect(messages).toHaveLength(1) + expect(messages[0]).toMatchObject({ + type: 'tool_result', + toolUseResult: { + answers: { 'Pick one?': 'A' }, + }, + }) + }) + it('should append subagent tool calls under their parent agent tool result', async () => { const sessionId = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' const projectDir = '-tmp-project' diff --git a/src/server/__tests__/ws-memory-events.test.ts b/src/server/__tests__/ws-memory-events.test.ts index 508166d3..0b9cac46 100644 --- a/src/server/__tests__/ws-memory-events.test.ts +++ b/src/server/__tests__/ws-memory-events.test.ts @@ -39,6 +39,39 @@ describe('WebSocket memory events', () => { }) }) +describe('WebSocket AskUserQuestion events', () => { + it('forwards structured AskUserQuestion answers from CLI toolUseResult metadata', () => { + expect(translateCliMessage({ + type: 'user', + message: { + role: 'user', + content: [ + { + type: 'tool_result', + tool_use_id: 'ask-1', + content: 'User has answered your questions: "Pick one?"="A". You can now continue with the user\'s answers in mind.', + }, + ], + }, + toolUseResult: { + questions: [{ question: 'Pick one?', options: [{ label: 'A' }] }], + answers: { 'Pick one?': 'A' }, + }, + }, 'session-1')).toEqual([ + { + type: 'tool_result', + toolUseId: 'ask-1', + content: { + questions: [{ question: 'Pick one?', options: [{ label: 'A' }] }], + answers: { 'Pick one?': 'A' }, + }, + isError: false, + parentToolUseId: undefined, + }, + ]) + }) +}) + describe('WebSocket compact events', () => { it('forwards CLI compacting status to the desktop client', () => { expect(translateCliMessage({ diff --git a/src/server/services/sessionService.ts b/src/server/services/sessionService.ts index 5da15afe..3bd84a62 100644 --- a/src/server/services/sessionService.ts +++ b/src/server/services/sessionService.ts @@ -78,6 +78,7 @@ export type MessageEntry = { id: string type: 'user' | 'assistant' | 'system' | 'tool_use' | 'tool_result' content: unknown + toolUseResult?: unknown timestamp: string model?: string parentUuid?: string @@ -425,6 +426,7 @@ export class SessionService { id: entry.uuid || crypto.randomUUID(), type, content: msg.content, + ...(entry.toolUseResult !== undefined ? { toolUseResult: entry.toolUseResult } : {}), timestamp: entry.timestamp || new Date().toISOString(), model: msg.model, parentUuid: entry.parentUuid ?? undefined, diff --git a/src/server/ws/handler.ts b/src/server/ws/handler.ts index c894217c..857a653d 100644 --- a/src/server/ws/handler.ts +++ b/src/server/ws/handler.ts @@ -849,6 +849,21 @@ function extractAssistantText(cliMsg: any): string { return textBlock?.text || '' } +function readObject(value: unknown): Record | null { + if (!value || typeof value !== 'object' || Array.isArray(value)) return null + return value as Record +} + +function normalizeAskUserQuestionToolResult(content: unknown, toolUseResult: unknown): unknown { + const result = readObject(toolUseResult) + const answers = readObject(result?.answers) + if (!result || !answers || !Array.isArray(result.questions)) return content + return { + questions: result.questions, + answers, + } +} + function isDuplicateOfLastApiError( lastApiError: SessionStreamState['lastApiError'], resultMessage: string, @@ -1044,7 +1059,7 @@ export function translateCliMessage(cliMsg: any, sessionId: string): ServerMessa messages.push({ type: 'tool_result', toolUseId: block.tool_use_id, - content: block.content, + content: normalizeAskUserQuestionToolResult(block.content, cliMsg.toolUseResult), isError: !!block.is_error, parentToolUseId, })