mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-18 13:23:33 +08:00
fix: improve AskUserQuestion desktop rendering
Desktop AskUserQuestion could show duplicate unresolved cards when restored history and a live permission request overlapped. The custom response field was also single-line, making long restored-context answers hard to read or edit. This keeps the active unresolved AskUserQuestion card aligned with the live permission request, preserves resolved history, and switches custom responses to a multiline editor with Ctrl/Cmd+Enter submission. Constraint: Desktop issue reports #542 and #543 both hit the AskUserQuestion chat surface. Rejected: Hide all AskUserQuestion history while pending | resolved answers should remain visible in the transcript. Confidence: high Scope-risk: narrow Tested: cd desktop && bun run test src/components/chat/AskUserQuestion.test.tsx Tested: cd desktop && bun run test src/components/chat/MessageList.test.tsx Tested: cd desktop && bun run lint Tested: cd desktop && bun run build Tested: bun run check:desktop Not-tested: Live Windows desktop manual restore flow from the issue attachment.
This commit is contained in:
parent
a159ee3511
commit
861d88225c
@ -296,12 +296,12 @@ describe('AskUserQuestion', () => {
|
||||
})
|
||||
fireEvent.click(screen.getByRole('button', { name: /Q2$/ }))
|
||||
|
||||
expect((screen.getByPlaceholderText('Type your answer...') as HTMLInputElement).value).toBe('')
|
||||
expect((screen.getByPlaceholderText('Type your answer...') as HTMLTextAreaElement).value).toBe('')
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: /^A2$/ }))
|
||||
fireEvent.click(screen.getByRole('button', { name: /Q1$/ }))
|
||||
|
||||
expect((screen.getByPlaceholderText('Type your answer...') as HTMLInputElement).value).toBe('custom-q1')
|
||||
expect((screen.getByPlaceholderText('Type your answer...') as HTMLTextAreaElement).value).toBe('custom-q1')
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: /submit/i }))
|
||||
|
||||
@ -318,4 +318,49 @@ describe('AskUserQuestion', () => {
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
it('uses a multiline custom response box and submits it with Ctrl+Enter', () => {
|
||||
render(
|
||||
<AskUserQuestion
|
||||
toolUseId="tool-1"
|
||||
input={{
|
||||
questions: [
|
||||
{
|
||||
question: 'What context should we restore?',
|
||||
options: [{ label: 'Skip' }],
|
||||
},
|
||||
],
|
||||
}}
|
||||
/>,
|
||||
)
|
||||
|
||||
const textarea = screen.getByPlaceholderText('Type your answer...')
|
||||
expect(textarea.tagName).toBe('TEXTAREA')
|
||||
expect(textarea.getAttribute('rows')).toBe('3')
|
||||
|
||||
fireEvent.change(textarea, {
|
||||
target: { value: 'First restored context line\nSecond restored context line' },
|
||||
})
|
||||
fireEvent.keyDown(textarea, { key: 'Enter' })
|
||||
expect(sendMock).not.toHaveBeenCalled()
|
||||
|
||||
fireEvent.keyDown(textarea, { key: 'Enter', ctrlKey: true })
|
||||
|
||||
expect(sendMock).toHaveBeenCalledWith(ACTIVE_TAB, {
|
||||
type: 'permission_response',
|
||||
requestId: 'perm-1',
|
||||
allowed: true,
|
||||
updatedInput: {
|
||||
questions: [
|
||||
{
|
||||
question: 'What context should we restore?',
|
||||
options: [{ label: 'Skip' }],
|
||||
},
|
||||
],
|
||||
answers: {
|
||||
'What context should we restore?': 'First restored context line\nSecond restored context line',
|
||||
},
|
||||
},
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@ -320,18 +320,22 @@ export function AskUserQuestion({ sessionId, toolUseId, input, result }: Props)
|
||||
<label className="text-xs text-[var(--color-text-tertiary)] mb-1.5 block">
|
||||
{t('question.customResponse')}
|
||||
</label>
|
||||
<input
|
||||
type="text"
|
||||
<textarea
|
||||
value={freeTexts[safeActiveTab] ?? ''}
|
||||
onChange={(e) => handleFreeTextChange(safeActiveTab, e.target.value)}
|
||||
onCompositionStart={() => { composingRef.current = true }}
|
||||
onCompositionEnd={() => { composingRef.current = false }}
|
||||
onKeyDown={(e) => {
|
||||
if (composingRef.current || e.nativeEvent.isComposing || e.keyCode === 229) return
|
||||
if (e.key === 'Enter' && allAnswered) handleSubmit()
|
||||
if (e.key === 'Enter' && (e.ctrlKey || e.metaKey) && allAnswered) {
|
||||
e.preventDefault()
|
||||
handleSubmit()
|
||||
}
|
||||
}}
|
||||
placeholder={t('question.typePlaceholder')}
|
||||
className="w-full px-3 py-2 text-sm bg-[var(--color-surface)] border border-[var(--color-outline-variant)]/40 rounded-[var(--radius-md)] text-[var(--color-text-primary)] placeholder:text-[var(--color-text-tertiary)] focus:outline-none focus:border-[var(--color-secondary)] focus:ring-1 focus:ring-[var(--color-secondary)]/30"
|
||||
rows={3}
|
||||
wrap="soft"
|
||||
className="max-h-48 min-h-[84px] w-full resize-y rounded-[var(--radius-md)] border border-[var(--color-outline-variant)]/40 bg-[var(--color-surface)] px-3 py-2 text-sm leading-relaxed text-[var(--color-text-primary)] placeholder:text-[var(--color-text-tertiary)] focus:border-[var(--color-secondary)] focus:outline-none focus:ring-1 focus:ring-[var(--color-secondary)]/30"
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
|
||||
@ -153,6 +153,104 @@ describe('MessageList nested tool calls', () => {
|
||||
expect(container.querySelector('[data-virtual-message-item]')).toBeNull()
|
||||
})
|
||||
|
||||
it('filters duplicate unresolved AskUserQuestion cards while a matching permission is pending', () => {
|
||||
const messages: UIMessage[] = [
|
||||
{
|
||||
id: 'stale-ask',
|
||||
type: 'tool_use',
|
||||
toolName: 'AskUserQuestion',
|
||||
toolUseId: 'stale-tool',
|
||||
input: {
|
||||
questions: [
|
||||
{
|
||||
question: 'Restore this context?',
|
||||
options: [{ label: 'No' }, { label: 'Yes' }],
|
||||
},
|
||||
],
|
||||
},
|
||||
timestamp: 1,
|
||||
},
|
||||
{
|
||||
id: 'active-ask',
|
||||
type: 'tool_use',
|
||||
toolName: 'AskUserQuestion',
|
||||
toolUseId: 'active-tool',
|
||||
input: {
|
||||
questions: [
|
||||
{
|
||||
question: 'Restore this context?',
|
||||
options: [{ label: 'No' }, { label: 'Yes' }],
|
||||
},
|
||||
],
|
||||
},
|
||||
timestamp: 2,
|
||||
},
|
||||
]
|
||||
|
||||
const { renderItems } = buildRenderModel(messages, 'active-tool')
|
||||
|
||||
expect(renderItems).toHaveLength(1)
|
||||
expect(renderItems[0]).toMatchObject({
|
||||
kind: 'message',
|
||||
message: {
|
||||
type: 'tool_use',
|
||||
toolName: 'AskUserQuestion',
|
||||
toolUseId: 'active-tool',
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps resolved AskUserQuestion history visible when filtering active duplicates', () => {
|
||||
const messages: UIMessage[] = [
|
||||
{
|
||||
id: 'answered-ask',
|
||||
type: 'tool_use',
|
||||
toolName: 'AskUserQuestion',
|
||||
toolUseId: 'answered-tool',
|
||||
input: {
|
||||
questions: [
|
||||
{
|
||||
question: 'Already answered?',
|
||||
options: [{ label: 'No' }, { label: 'Yes' }],
|
||||
},
|
||||
],
|
||||
},
|
||||
timestamp: 1,
|
||||
},
|
||||
{
|
||||
id: 'answered-result',
|
||||
type: 'tool_result',
|
||||
toolUseId: 'answered-tool',
|
||||
content: { answers: { 'Already answered?': 'Yes' } },
|
||||
isError: false,
|
||||
timestamp: 2,
|
||||
},
|
||||
{
|
||||
id: 'active-ask',
|
||||
type: 'tool_use',
|
||||
toolName: 'AskUserQuestion',
|
||||
toolUseId: 'active-tool',
|
||||
input: {
|
||||
questions: [
|
||||
{
|
||||
question: 'Restore this context?',
|
||||
options: [{ label: 'No' }, { label: 'Yes' }],
|
||||
},
|
||||
],
|
||||
},
|
||||
timestamp: 3,
|
||||
},
|
||||
]
|
||||
|
||||
const { renderItems } = buildRenderModel(messages, 'active-tool')
|
||||
|
||||
expect(renderItems).toHaveLength(2)
|
||||
expect(renderItems.map((item) => item.kind === 'message' && item.message.type === 'tool_use'
|
||||
? item.message.toolUseId
|
||||
: null,
|
||||
)).toEqual(['answered-tool', 'active-tool'])
|
||||
})
|
||||
|
||||
it('renders goal events as visible status cards', () => {
|
||||
useChatStore.setState({
|
||||
sessions: {
|
||||
|
||||
@ -447,11 +447,12 @@ function appendChildToolCall(
|
||||
}
|
||||
}
|
||||
|
||||
export function buildRenderModel(messages: UIMessage[]): RenderModel {
|
||||
export function buildRenderModel(messages: UIMessage[], activeAskUserQuestionToolUseId?: string | null): RenderModel {
|
||||
const items: RenderItem[] = []
|
||||
const toolResultMap = new Map<string, ToolResult>()
|
||||
const childToolCallsByParent = new Map<string, ToolCall[]>()
|
||||
const toolUseIds = new Set<string>()
|
||||
const lastUnresolvedAskUserQuestionIndexByToolUseId = new Map<string, number>()
|
||||
let pendingToolCalls: ToolCall[] = []
|
||||
|
||||
const flushGroup = () => {
|
||||
@ -482,6 +483,15 @@ export function buildRenderModel(messages: UIMessage[]): RenderModel {
|
||||
toolResultMap.set(msg.toolUseId, msg)
|
||||
}
|
||||
}
|
||||
messages.forEach((msg, index) => {
|
||||
if (
|
||||
msg.type === 'tool_use' &&
|
||||
msg.toolName === 'AskUserQuestion' &&
|
||||
!toolResultMap.has(msg.toolUseId)
|
||||
) {
|
||||
lastUnresolvedAskUserQuestionIndexByToolUseId.set(msg.toolUseId, index)
|
||||
}
|
||||
})
|
||||
|
||||
for (const msg of messages) {
|
||||
if (msg.type === 'assistant_text' && !msg.content.trim()) {
|
||||
@ -505,6 +515,18 @@ export function buildRenderModel(messages: UIMessage[]): RenderModel {
|
||||
continue
|
||||
}
|
||||
if (msg.toolName === 'AskUserQuestion') {
|
||||
const isResolved = toolResultMap.has(msg.toolUseId)
|
||||
const lastUnresolvedIndex = lastUnresolvedAskUserQuestionIndexByToolUseId.get(msg.toolUseId)
|
||||
if (!isResolved && lastUnresolvedIndex !== undefined && messages[lastUnresolvedIndex] !== msg) {
|
||||
continue
|
||||
}
|
||||
if (
|
||||
!isResolved &&
|
||||
activeAskUserQuestionToolUseId &&
|
||||
msg.toolUseId !== activeAskUserQuestionToolUseId
|
||||
) {
|
||||
continue
|
||||
}
|
||||
flushGroup()
|
||||
items.push({ kind: 'message', message: msg })
|
||||
} else {
|
||||
@ -822,6 +844,10 @@ export function MessageList({ sessionId, compact = false }: MessageListProps = {
|
||||
const streamingText = sessionState?.streamingText ?? ''
|
||||
const activeThinkingId = sessionState?.activeThinkingId ?? null
|
||||
const agentTaskNotifications = sessionState?.agentTaskNotifications ?? {}
|
||||
const activeAskUserQuestionToolUseId =
|
||||
sessionState?.pendingPermission?.toolName === 'AskUserQuestion'
|
||||
? sessionState.pendingPermission.toolUseId
|
||||
: null
|
||||
const shouldFollowContentResize =
|
||||
streamingText.trim().length > 0 ||
|
||||
chatState === 'streaming' ||
|
||||
@ -975,8 +1001,8 @@ export function MessageList({ sessionId, compact = false }: MessageListProps = {
|
||||
}, [scrollToBottom, shouldFollowContentResize])
|
||||
|
||||
const { toolResultMap, childToolCallsByParent, renderItems } = useMemo(
|
||||
() => buildRenderModel(messages),
|
||||
[messages],
|
||||
() => buildRenderModel(messages, activeAskUserQuestionToolUseId),
|
||||
[activeAskUserQuestionToolUseId, messages],
|
||||
)
|
||||
const branchableMessageTargets = useMemo(
|
||||
() => branchActionsDisabled ? new Map<string, BranchableMessageTarget>() : getBranchableMessageTargets(messages),
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user