diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 6d82071512..c2ce513e24 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -1,4 +1,13 @@ -import React, { forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState } from "react" +import React, { + forwardRef, + useCallback, + useEffect, + useImperativeHandle, + useMemo, + useReducer, + useRef, + useState, +} from "react" import { useDeepCompareEffect, useEvent } from "react-use" import debounce from "debounce" import { Virtuoso, type VirtuosoHandle } from "react-virtuoso" @@ -62,6 +71,64 @@ export interface ChatViewRef { export const MAX_IMAGES_PER_MESSAGE = 20 // This is the Anthropic limit. +// --- Button state reducer (replaces 5 separate useState calls) --- + +interface ButtonState { + sendingDisabled: boolean + clineAsk: ClineAsk | undefined + enableButtons: boolean + primaryButtonText: string | undefined + secondaryButtonText: string | undefined +} + +type ButtonStateAction = + | { + type: "SET_ASK_STATE" + sendingDisabled: boolean + clineAsk: ClineAsk | undefined + enableButtons: boolean + primaryButtonText: string | undefined + secondaryButtonText: string | undefined + } + | { type: "SET_SENDING_DISABLED"; sendingDisabled: boolean } + | { type: "SET_BUTTON_TEXT"; primaryButtonText: string | undefined; secondaryButtonText: string | undefined } + | { type: "RESET_WITHOUT_BUTTON_TEXT" } + +const initialButtonState: ButtonState = { + sendingDisabled: false, + clineAsk: undefined, + enableButtons: false, + primaryButtonText: undefined, + secondaryButtonText: undefined, +} + +function buttonStateReducer(state: ButtonState, action: ButtonStateAction): ButtonState { + switch (action.type) { + case "SET_ASK_STATE": + return { + sendingDisabled: action.sendingDisabled, + clineAsk: action.clineAsk, + enableButtons: action.enableButtons, + primaryButtonText: action.primaryButtonText, + secondaryButtonText: action.secondaryButtonText, + } + case "SET_SENDING_DISABLED": + return { ...state, sendingDisabled: action.sendingDisabled } + case "SET_BUTTON_TEXT": + return { + ...state, + primaryButtonText: action.primaryButtonText, + secondaryButtonText: action.secondaryButtonText, + } + case "RESET_WITHOUT_BUTTON_TEXT": + return { ...state, sendingDisabled: true, clineAsk: undefined, enableButtons: false } + default: + return state + } +} + +// --- End button state reducer --- + const isMac = navigator.platform.toUpperCase().indexOf("MAC") >= 0 const ChatViewComponent: React.ForwardRefRenderFunction = ( @@ -140,17 +207,14 @@ const ChatViewComponent: React.ForwardRefRenderFunction(null) - const [sendingDisabled, setSendingDisabled] = useState(false) - const [selectedImages, setSelectedImages] = useState([]) - + // Button state managed by a single reducer to eliminate partial-update race conditions. // We need to hold on to the ask because useEffect > lastMessage will always // let us know when an ask comes in and handle it, but by the time // handleMessage is called, the last message might not be the ask anymore // (it could be a say that followed). - const [clineAsk, setClineAsk] = useState(undefined) - const [enableButtons, setEnableButtons] = useState(false) - const [primaryButtonText, setPrimaryButtonText] = useState(undefined) - const [secondaryButtonText, setSecondaryButtonText] = useState(undefined) + const [buttonState, dispatch] = useReducer(buttonStateReducer, initialButtonState) + const { clineAsk, enableButtons, sendingDisabled, primaryButtonText, secondaryButtonText } = buttonState + const [selectedImages, setSelectedImages] = useState([]) const [_didClickCancel, setDidClickCancel] = useState(false) const virtuosoRef = useRef(null) const [expandedRows, setExpandedRows] = useState>({}) @@ -292,101 +356,127 @@ const ChatViewComponent: React.ForwardRefRenderFunction msg.ask === "completion_result" || msg.say === "completion_result", ) if (isCompletedSubtask) { - setPrimaryButtonText(t("chat:startNewTask.title")) - setSecondaryButtonText(undefined) + dispatch({ + type: "SET_ASK_STATE", + sendingDisabled: false, + clineAsk: "resume_task", + enableButtons: true, + primaryButtonText: t("chat:startNewTask.title"), + secondaryButtonText: undefined, + }) } else { - setPrimaryButtonText(t("chat:resumeTask.title")) - setSecondaryButtonText(t("chat:terminate.title")) + dispatch({ + type: "SET_ASK_STATE", + sendingDisabled: false, + clineAsk: "resume_task", + enableButtons: true, + primaryButtonText: t("chat:resumeTask.title"), + secondaryButtonText: t("chat:terminate.title"), + }) } setDidClickCancel(false) // special case where we reset the cancel button state break + } case "resume_completed_task": - setSendingDisabled(false) - setClineAsk("resume_completed_task") - setEnableButtons(true) - setPrimaryButtonText(t("chat:startNewTask.title")) - setSecondaryButtonText(undefined) + dispatch({ + type: "SET_ASK_STATE", + sendingDisabled: false, + clineAsk: "resume_completed_task", + enableButtons: true, + primaryButtonText: t("chat:startNewTask.title"), + secondaryButtonText: undefined, + }) setDidClickCancel(false) break } @@ -438,21 +544,24 @@ const ChatViewComponent: React.ForwardRefRenderFunction msg.ask === "completion_result" || msg.say === "completion_result", ) if (hasCompletionResult) { - setPrimaryButtonText(t("chat:startNewTask.title")) - setSecondaryButtonText(undefined) + dispatch({ + type: "SET_BUTTON_TEXT", + primaryButtonText: t("chat:startNewTask.title"), + secondaryButtonText: undefined, + }) } } }, [clineAsk, currentTaskItem?.parentTaskId, messages, t]) useEffect(() => { if (messages.length === 0) { - setSendingDisabled(false) - setClineAsk(undefined) - setEnableButtons(false) - setPrimaryButtonText(undefined) - setSecondaryButtonText(undefined) + dispatch({ + type: "SET_ASK_STATE", + sendingDisabled: false, + clineAsk: undefined, + enableButtons: false, + primaryButtonText: undefined, + secondaryButtonText: undefined, + }) } }, [messages.length]) @@ -634,13 +749,10 @@ const ChatViewComponent: React.ForwardRefRenderFunction { ) }) }) + +describe("ChatView - Button Click Handler Tests", () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(vscode.postMessage).mockClear() + }) + + it("calls startNewTask (clearTask) when secondary button clicked with api_req_failed and stale isStreaming true", async () => { + const { getByText } = renderChatView() + + const baseTsA = Date.now() + + // Step 1: Post messages with api_req_failed as the last message. + // useDeepCompareEffect fires and sets clineAsk = "api_req_failed", + // enableButtons = true, primaryButtonText = "chat:retry.title", + // secondaryButtonText = "chat:startNewTask.title". + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: baseTsA - 4000, + text: "Initial task", + }, + { + type: "say", + say: "api_req_started", + ts: baseTsA - 3000, + text: JSON.stringify({ apiProtocol: "anthropic" }), // No cost + }, + { + type: "ask", + ask: "api_req_failed", + ts: baseTsA - 2000, + text: "API request failed", + }, + ], + }) + + // Wait for the secondary button to appear with "Start New Task" text + await waitFor(() => { + expect(getByText("chat:startNewTask.title")).toBeInTheDocument() + }) + + // Step 2: Append api_req_retry_delayed as the last message. + // This makes isLastAsk = false so isToolCurrentlyAsking = false. + // Since the last api_req_started has no cost, isStreaming becomes true. + // clineAsk remains "api_req_failed" because api_req_retry_delayed only + // sets setSendingDisabled(true) without clearing button state. + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: baseTsA - 4000, + text: "Initial task", + }, + { + type: "say", + say: "api_req_started", + ts: baseTsA - 3000, + text: JSON.stringify({ apiProtocol: "anthropic" }), // No cost → isStreaming = true + }, + { + type: "ask", + ask: "api_req_failed", + ts: baseTsA - 2000, + text: "API request failed", + }, + { + type: "say", + say: "api_req_retry_delayed", + ts: baseTsA - 1000, + text: "Retrying in 5s", + }, + ], + }) + + // Allow React effects to fully propagate + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 50)) + }) + + // The secondary button should still be visible with "Start New Task" + await waitFor(() => { + expect(getByText("chat:startNewTask.title")).toBeInTheDocument() + }) + + // Clear previous postMessage calls + vi.mocked(vscode.postMessage).mockClear() + + // Click the secondary button (shows "chat:startNewTask.title"). + // The early-return path in handleSecondaryButtonClick for api_req_failed + // should call startNewTask() before the isStreaming guard, preventing + // cancelTask from being called. + await act(async () => { + fireEvent.click(getByText("chat:startNewTask.title")) + }) + + // Verify startNewTask was called (posts clearTask), not cancelTask + await waitFor(() => { + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "clearTask" }) + }) + expect(vscode.postMessage).not.toHaveBeenCalledWith({ type: "cancelTask" }) + }) + + it("calls startNewTask (clearTask) when primary button clicked with Start New Task text on completion_result", async () => { + const { getByText } = renderChatView() + + // Set up completion_result ask (non-partial) which sets: + // - clineAsk = "completion_result" + // - primaryButtonText = t("chat:startNewTask.title") → "chat:startNewTask.title" + // - enableButtons = true + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2000, + text: "Initial task", + }, + { + type: "say", + say: "api_req_started", + ts: Date.now() - 1500, + text: JSON.stringify({ + apiProtocol: "anthropic", + cost: 0.05, + tokensIn: 100, + tokensOut: 50, + }), + }, + { + type: "ask", + ask: "completion_result", + ts: Date.now(), + text: "Task completed successfully", + partial: false, + }, + ], + }) + + // Wait for the primary button with "Start New Task" text to appear + await waitFor(() => { + expect(getByText("chat:startNewTask.title")).toBeInTheDocument() + }) + + // Allow React effects to fully propagate + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 50)) + }) + + // Clear previous postMessage calls + vi.mocked(vscode.postMessage).mockClear() + + // Click the primary button. With clineAsk = "completion_result", the + // handler's case "completion_result" calls startNewTask(). The default + // case fallback also checks primaryButtonText against + // t("chat:startNewTask.title") for safety when clineAsk becomes undefined + // due to race conditions. + await act(async () => { + fireEvent.click(getByText("chat:startNewTask.title")) + }) + + // Verify startNewTask was called (posts clearTask) + await waitFor(() => { + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "clearTask" }) + }) + }) +})