From 0a9c2b910def16839465416df123267d9b13167c Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Mon, 9 Feb 2026 10:03:45 -0700 Subject: [PATCH] fix(chat): harden responsiveness and deterministic pending transitions --- webview-ui/src/components/chat/ChatView.tsx | 510 ++++-- .../src/components/chat/FollowUpSuggest.tsx | 95 +- .../chat/__tests__/ChatView.spec.tsx | 1367 ++++++++++++++++- .../chat/__tests__/FollowUpSuggest.spec.tsx | 304 +++- .../followUpInteractionInstrumentation.ts | 45 + .../chat/usePendingActionContract.ts | 33 + 6 files changed, 2165 insertions(+), 189 deletions(-) create mode 100644 webview-ui/src/components/chat/followUpInteractionInstrumentation.ts create mode 100644 webview-ui/src/components/chat/usePendingActionContract.ts diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 90d5abf23b..b3532a00b7 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -52,6 +52,8 @@ import { WorktreeSelector } from "./WorktreeSelector" import DismissibleUpsell from "../common/DismissibleUpsell" import { useCloudUpsell } from "@src/hooks/useCloudUpsell" import { Cloud } from "lucide-react" +import { emitFollowUpInteractionMarker } from "./followUpInteractionInstrumentation" +import { usePendingActionContract } from "./usePendingActionContract" export interface ChatViewProps { isHidden: boolean @@ -156,6 +158,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction(false) const [primaryButtonText, setPrimaryButtonText] = useState(undefined) const [secondaryButtonText, setSecondaryButtonText] = useState(undefined) + const clineAskRef = useRef(undefined) const [_didClickCancel, setDidClickCancel] = useState(false) const virtuosoRef = useRef(null) const [expandedRows, setExpandedRows] = useState>({}) @@ -171,15 +174,35 @@ const ChatViewComponent: React.ForwardRefRenderFunction(undefined) const [isCondensing, setIsCondensing] = useState(false) const [showAnnouncementModal, setShowAnnouncementModal] = useState(false) + const { + isPending: isApprovalActionPending, + tryBeginPendingAction: tryBeginApprovalActionPending, + clearPendingAction: clearApprovalActionPending, + } = usePendingActionContract() + const { + isPending: isFollowUpSuggestionActionPending, + tryBeginPendingAction: tryBeginFollowUpSuggestionActionPending, + clearPendingAction: clearFollowUpSuggestionActionPending, + } = usePendingActionContract() const everVisibleMessagesTsRef = useRef>( new LRUCache({ max: 100, ttl: 1000 * 60 * 5, }), ) - const autoApproveTimeoutRef = useRef(null) + const cacheCleanupSnapshotRef = useRef<{ + currentMessageIds: Set + viewportMessageIds: Set + }>({ + currentMessageIds: new Set(), + viewportMessageIds: new Set(), + }) const userRespondedRef = useRef(false) const [currentFollowUpTs, setCurrentFollowUpTs] = useState(null) + const followUpInstrumentationRef = useRef<{ pendingTs: number | null; settledTs: number | null }>({ + pendingTs: null, + settledTs: null, + }) const [aggregatedCostsMap, setAggregatedCostsMap] = useState< Map< string, @@ -191,11 +214,6 @@ const ChatViewComponent: React.ForwardRefRenderFunction >(new Map()) - const clineAskRef = useRef(clineAsk) - useEffect(() => { - clineAskRef.current = clineAsk - }, [clineAsk]) - const { isOpen: isUpsellOpen, openUpsell, @@ -210,6 +228,10 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + clineAskRef.current = clineAsk + }, [clineAsk]) + // Compute whether auto-approval is paused (user is typing in a followup) const isFollowUpAutoApprovalPaused = useMemo(() => { return !!(inputValue && inputValue.trim().length > 0 && clineAsk === "followup") @@ -243,6 +265,44 @@ const ChatViewComponent: React.ForwardRefRenderFunction messages.at(-1), [messages]) const secondLastMessage = useMemo(() => messages.at(-2), [messages]) + const getCurrentAskForInteraction = useCallback((): ClineAsk | undefined => { + const latestMessage = messagesRef.current.at(-1) + + if (latestMessage?.type === "ask") { + return latestMessage.ask + } + + return clineAskRef.current + }, []) + + const activeFollowUpTs = useMemo(() => { + const latestMessage = messages.at(-1) + + if (latestMessage?.type !== "ask" || latestMessage.ask !== "followup") { + return null + } + + return latestMessage.ts + }, [messages]) + + const isCurrentSubtaskCompleted = useCallback((): boolean => { + return Boolean( + currentTaskItem?.parentTaskId && + messagesRef.current.some((msg) => msg.ask === "completion_result" || msg.say === "completion_result"), + ) + }, [currentTaskItem?.parentTaskId]) + + const applyResumeTaskControlState = useCallback(() => { + if (isCurrentSubtaskCompleted()) { + setPrimaryButtonText(t("chat:startNewTask.title")) + setSecondaryButtonText(undefined) + return + } + + setPrimaryButtonText(t("chat:resumeTask.title")) + setSecondaryButtonText(t("chat:terminate.title")) + }, [isCurrentSubtaskCompleted, t]) + const volume = typeof soundVolume === "number" ? soundVolume : 0.5 const [playNotification] = useSound(`${audioBaseUri}/notification.wav`, { volume, soundEnabled }) const [playCelebration] = useSound(`${audioBaseUri}/celebration.wav`, { volume, soundEnabled }) @@ -407,22 +467,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction msg.ask === "completion_result" || msg.say === "completion_result", - ) - if (isCompletedSubtask) { - setPrimaryButtonText(t("chat:startNewTask.title")) - setSecondaryButtonText(undefined) - } else { - setPrimaryButtonText(t("chat:resumeTask.title")) - setSecondaryButtonText(t("chat:terminate.title")) - } + applyResumeTaskControlState() setDidClickCancel(false) // special case where we reset the cancel button state break case "resume_completed_task": @@ -467,20 +512,22 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + if (!isApprovalActionPending) { + return + } + + clearApprovalActionPending() + }, [messages, isApprovalActionPending, clearApprovalActionPending]) // Update button text when messages change (e.g., completion_result is added) for subtasks in resume_task state useEffect(() => { - if (clineAsk === "resume_task" && currentTaskItem?.parentTaskId) { - const hasCompletionResult = messages.some( - (msg) => msg.ask === "completion_result" || msg.say === "completion_result", - ) - if (hasCompletionResult) { - setPrimaryButtonText(t("chat:startNewTask.title")) - setSecondaryButtonText(undefined) - } + if (clineAsk === "resume_task") { + applyResumeTaskControlState() } - }, [clineAsk, currentTaskItem?.parentTaskId, messages, t]) + }, [clineAsk, applyResumeTaskControlState]) useEffect(() => { if (messages.length === 0) { @@ -499,12 +546,6 @@ const ChatViewComponent: React.ForwardRefRenderFunction { const lastFollowUpMessage = messagesRef.current.findLast((msg: ClineMessage) => msg.ask === "followup") if (lastFollowUpMessage) { + if (followUpInstrumentationRef.current.pendingTs !== lastFollowUpMessage.ts) { + emitFollowUpInteractionMarker({ + stage: "pending_render", + followUpTs: lastFollowUpMessage.ts, + source: "chat_view", + }) + followUpInstrumentationRef.current.pendingTs = lastFollowUpMessage.ts + followUpInstrumentationRef.current.settledTs = null + } + setCurrentFollowUpTs(lastFollowUpMessage.ts) } }, []) - const handleChatReset = useCallback(() => { - // Clear any pending auto-approval timeout - if (autoApproveTimeoutRef.current) { - clearTimeout(autoApproveTimeoutRef.current) - autoApproveTimeoutRef.current = null + useEffect(() => { + const pendingTs = followUpInstrumentationRef.current.pendingTs + + if (pendingTs === null || currentFollowUpTs !== pendingTs) { + return } + + if (followUpInstrumentationRef.current.settledTs === pendingTs) { + return + } + + emitFollowUpInteractionMarker({ + stage: "settle", + followUpTs: pendingTs, + source: "chat_view", + }) + followUpInstrumentationRef.current.settledTs = pendingTs + }, [currentFollowUpTs]) + + useEffect(() => { + if (currentFollowUpTs !== null) { + return + } + + const { pendingTs, settledTs } = followUpInstrumentationRef.current + + if (pendingTs === null && settledTs === null) { + return + } + + emitFollowUpInteractionMarker({ + stage: "clear", + followUpTs: pendingTs, + source: "chat_view", + }) + + followUpInstrumentationRef.current.pendingTs = null + followUpInstrumentationRef.current.settledTs = null + }, [currentFollowUpTs]) + + useEffect(() => { + if (!isFollowUpSuggestionActionPending) { + return + } + + const hasFollowUpAsk = messages.some((message) => message.type === "ask" && message.ask === "followup") + + if (currentFollowUpTs !== null || !hasFollowUpAsk || getCurrentAskForInteraction() !== "followup") { + clearFollowUpSuggestionActionPending() + } + }, [ + messages, + currentFollowUpTs, + isFollowUpSuggestionActionPending, + clearFollowUpSuggestionActionPending, + getCurrentAskForInteraction, + ]) + + const handleChatReset = useCallback(() => { // Reset user response flag for new message userRespondedRef.current = false @@ -633,8 +737,15 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + ( + text: string, + images: string[], + options?: { + followUpInteractionSource?: "typed" | "suggestion" + }, + ) => { text = text.trim() + const currentAsk = getCurrentAskForInteraction() if (text || images.length > 0) { // Intercept when the active provider is retired — show a @@ -649,12 +760,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction 0 || - clineAskRef.current === "command_output" - ) { + if (sendingDisabled || isStreaming || messageQueue.length > 0 || currentAsk === "command_output") { try { console.log("queueMessage", text, images) vscode.postMessage({ type: "queueMessage", text, images }) @@ -669,20 +775,31 @@ const ChatViewComponent: React.ForwardRefRenderFunction msg.ask === "followup")?.ts ?? null, + source: "chat_view", + }) + markFollowUpAsAnswered() + } + } + // Mark that user has responded - this prevents any pending auto-approvals. userRespondedRef.current = true if (messagesRef.current.length === 0) { vscode.postMessage({ type: "newTask", text, images }) - } else if (clineAskRef.current) { - if (clineAskRef.current === "followup") { - markFollowUpAsAnswered() - } - - // Use clineAskRef.current - switch ( - clineAskRef.current // Use clineAskRef.current - ) { + } else if (currentAsk) { + switch (currentAsk) { case "followup": case "tool": case "browser_action_launch": @@ -712,11 +829,13 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + setSendingDisabled(true) + setClineAsk(undefined) + setEnableButtons(false) + setPrimaryButtonText(undefined) + setSecondaryButtonText(undefined) + }, []) + + const beginActionResolutionTransition = useCallback(() => { + transitionActionHandlersToPendingState() + userRespondedRef.current = true + }, [transitionActionHandlersToPendingState]) + + const postAskResponseWithOptionalInput = useCallback( + (askResponse: "yesButtonClicked" | "noButtonClicked", text?: string, images?: string[]) => { + if (text || (images && images.length > 0)) { + vscode.postMessage({ + type: "askResponse", + askResponse, + text, + images, + }) + setInputValue("") + setSelectedImages([]) + return + } + + vscode.postMessage({ type: "askResponse", askResponse }) + }, + [], + ) + // This logic depends on the useEffect[messages] above to set clineAsk, // after which buttons are shown and we then send an askResponse to the // extension. const handlePrimaryButtonClick = useCallback( (text?: string, images?: string[]) => { - // Mark that user has responded - userRespondedRef.current = true + if (!clineAsk) { + return + } + + if (!tryBeginApprovalActionPending()) { + return + } + + // Apply optimistic pending UI immediately on click. + beginActionResolutionTransition() const trimmedInput = text?.trim() @@ -776,46 +935,13 @@ const ChatViewComponent: React.ForwardRefRenderFunction 0)) { - vscode.postMessage({ - type: "askResponse", - askResponse: "yesButtonClicked", - text: trimmedInput, - images: images, - }) - // Clear input state after sending - setInputValue("") - setSelectedImages([]) - } else { - vscode.postMessage({ type: "askResponse", askResponse: "yesButtonClicked" }) - } + postAskResponseWithOptionalInput("yesButtonClicked", trimmedInput, images) break case "resume_task": - // For completed subtasks (tasks with a parentTaskId and a completion_result), - // start a new task instead of resuming since the subtask is done - const isCompletedSubtaskForClick = - currentTaskItem?.parentTaskId && - messagesRef.current.some( - (msg) => msg.ask === "completion_result" || msg.say === "completion_result", - ) - if (isCompletedSubtaskForClick) { + if (isCurrentSubtaskCompleted()) { startNewTask() } else { - // Only send text/images if they exist - if (trimmedInput || (images && images.length > 0)) { - vscode.postMessage({ - type: "askResponse", - askResponse: "yesButtonClicked", - text: trimmedInput, - images: images, - }) - // Clear input state after sending - setInputValue("") - setSelectedImages([]) - } else { - vscode.postMessage({ type: "askResponse", askResponse: "yesButtonClicked" }) - } + postAskResponseWithOptionalInput("yesButtonClicked", trimmedInput, images) } break case "completion_result": @@ -827,20 +953,29 @@ const ChatViewComponent: React.ForwardRefRenderFunction { - // Mark that user has responded - userRespondedRef.current = true + if (!clineAsk) { + return + } + + if (!tryBeginApprovalActionPending()) { + return + } + + // Apply optimistic pending UI immediately on click to avoid stale controls. + beginActionResolutionTransition() const trimmedInput = text?.trim() @@ -860,31 +995,22 @@ const ChatViewComponent: React.ForwardRefRenderFunction 0)) { - vscode.postMessage({ - type: "askResponse", - askResponse: "noButtonClicked", - text: trimmedInput, - images: images, - }) - // Clear input state after sending - setInputValue("") - setSelectedImages([]) - } else { - // Responds to the API with a "This operation failed" and lets it try again - vscode.postMessage({ type: "askResponse", askResponse: "noButtonClicked" }) - } + postAskResponseWithOptionalInput("noButtonClicked", trimmedInput, images) break case "command_output": vscode.postMessage({ type: "terminalOperation", terminalOperation: "abort" }) break } - setSendingDisabled(true) - setClineAsk(undefined) - setEnableButtons(false) }, - [clineAsk, startNewTask, isStreaming, setDidClickCancel], + [ + clineAsk, + startNewTask, + isStreaming, + setDidClickCancel, + tryBeginApprovalActionPending, + beginActionResolutionTransition, + postAskResponseWithOptionalInput, + ], ) const { info: model } = useSelectedModel(apiConfiguration) @@ -1094,9 +1220,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction { const cleanupInterval = setInterval(() => { const cache = everVisibleMessagesTsRef.current - const currentMessageIds = new Set(modifiedMessages.map((m: ClineMessage) => m.ts)) - const viewportMessages = visibleMessages.slice(Math.max(0, visibleMessages.length - 100)) - const viewportMessageIds = new Set(viewportMessages.map((m: ClineMessage) => m.ts)) + const { currentMessageIds, viewportMessageIds } = cacheCleanupSnapshotRef.current cache.forEach((_value: boolean, key: number) => { if (!currentMessageIds.has(key) && !viewportMessageIds.has(key)) { @@ -1106,7 +1230,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction clearInterval(cleanupInterval) - }, [modifiedMessages, visibleMessages]) + }, []) useDebounceEffect( () => { @@ -1325,15 +1449,73 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + const browserActions: ClineMessage[] = [] + const browserActionResults: ClineMessage[] = [] + + for (const message of modifiedMessages) { + if (message.type !== "say") { + continue + } + + if (message.say === "browser_action") { + browserActions.push(message) + continue + } + + if (message.say === "browser_action_result") { + browserActionResults.push(message) + } + } + + const browserActionIndexByTs = new Map() + const nextBrowserActionResultByTs = new Map() + let resultCursor = 0 + + for (let actionIndex = 0; actionIndex < browserActions.length; actionIndex++) { + const actionMessage = browserActions[actionIndex] + browserActionIndexByTs.set(actionMessage.ts, actionIndex + 1) + + while ( + resultCursor < browserActionResults.length && + browserActionResults[resultCursor].ts <= actionMessage.ts + ) { + resultCursor++ + } + + nextBrowserActionResultByTs.set(actionMessage.ts, browserActionResults[resultCursor]) + } + + return { + hasCheckpoint: modifiedMessages.some((message) => message.say === "checkpoint_saved"), + lastModifiedMessage: modifiedMessages.at(-1), + totalBrowserActions: browserActions.length, + browserActionIndexByTs, + nextBrowserActionResultByTs, + } + }, [modifiedMessages]) + + useEffect(() => { + const viewportStart = Math.max(0, visibleMessages.length - 100) + + cacheCleanupSnapshotRef.current = { + currentMessageIds: new Set(modifiedMessages.map((message: ClineMessage) => message.ts)), + viewportMessageIds: new Set( + visibleMessages.slice(viewportStart).map((message: ClineMessage) => message.ts), + ), + } + }, [modifiedMessages, visibleMessages]) // scrolling @@ -1384,7 +1566,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction scrollToBottomAuto(), 0) + scrollToBottomAuto() } } }, @@ -1439,13 +1621,20 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + const currentAsk = getCurrentAskForInteraction() + const isFollowUpSuggestionSubmit = currentAsk === "followup" && !event?.shiftKey + + if (isFollowUpSuggestionSubmit && !tryBeginFollowUpSuggestionActionPending()) { + return + } + // Mark that user has responded if this is a manual click (not auto-approval) if (event) { userRespondedRef.current = true } // Mark the current follow-up question as answered when a suggestion is clicked - if (clineAsk === "followup" && !event?.shiftKey) { + if (isFollowUpSuggestionSubmit) { markFollowUpAsAnswered() } @@ -1468,12 +1657,20 @@ const ChatViewComponent: React.ForwardRefRenderFunction { @@ -1483,19 +1680,11 @@ const ChatViewComponent: React.ForwardRefRenderFunction { - const hasCheckpoint = modifiedMessages.some((message) => message.say === "checkpoint_saved") - // Check if this is a browser action message if (messageOrGroup.type === "say" && messageOrGroup.say === "browser_action") { - // Find the corresponding result message by looking for the next browser_action_result after this action's timestamp - const nextMessage = modifiedMessages.find( - (m) => m.ts > messageOrGroup.ts && m.say === "browser_action_result", - ) - - // Calculate action index and total count - const browserActions = modifiedMessages.filter((m) => m.say === "browser_action") - const actionIndex = browserActions.findIndex((m) => m.ts === messageOrGroup.ts) + 1 - const totalActions = browserActions.length + const nextMessage = rowRenderMetadata.nextBrowserActionResultByTs.get(messageOrGroup.ts) + const actionIndex = rowRenderMetadata.browserActionIndexByTs.get(messageOrGroup.ts) ?? 1 + const totalActions = rowRenderMetadata.totalBrowserActions return ( ) }, [ expandedRows, toggleRowExpansion, - modifiedMessages, + rowRenderMetadata, groupedMessages.length, handleRowHeightChange, isStreaming, handleSuggestionClickInRow, handleBatchFileResponse, currentFollowUpTs, + activeFollowUpTs, isFollowUpAutoApprovalPaused, enableButtons, primaryButtonText, @@ -1612,10 +1808,11 @@ const ChatViewComponent: React.ForwardRefRenderFunction ({ acceptInput: () => { const hasInput = inputValue.trim() || selectedImages.length > 0 + const currentAsk = getCurrentAskForInteraction() // Special case: during command_output, queue the message instead of // triggering the primary button action (which would lose the message) - if (clineAskRef.current === "command_output" && hasInput) { + if (currentAsk === "command_output" && hasInput) { vscode.postMessage({ type: "queueMessage", text: inputValue.trim(), images: selectedImages }) setInputValue("") setSelectedImages([]) @@ -1640,6 +1837,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction - {isFirstSuggestion && countdown !== null && !suggestionSelected && !isAnswered && ( + {isFirstSuggestion && shouldShowCountdown && (

{t("chat:followUpSuggest.timerPrefix", { seconds: countdown })} @@ -143,14 +205,7 @@ export const FollowUpSuggest = ({

{ - e.stopPropagation() - // Cancel the auto-approve timer when edit button is clicked - setSuggestionSelected(true) - onCancelAutoApproval?.() - // Simulate shift-click by directly calling the handler with shiftKey=true. - onSuggestionClick?.(suggestion, { ...e, shiftKey: true }) - }}> + onClick={(event) => handleCopyToInputClick(suggestion, event)}>
diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index 1026ac86d0..aead964a76 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -8,6 +8,10 @@ import { ExtensionStateContextProvider } from "@src/context/ExtensionStateContex import { vscode } from "@src/utils/vscode" import ChatView, { ChatViewProps } from "../ChatView" +import { + setFollowUpInteractionInstrumentationSink, + type FollowUpInteractionMarker, +} from "../followUpInteractionInstrumentation" // Define minimal types needed for testing interface ClineMessage { @@ -19,6 +23,11 @@ interface ClineMessage { partial?: boolean } +interface SuggestionItem { + answer: string + mode?: string +} + interface ExtensionState { version: string clineMessages: ClineMessage[] @@ -52,8 +61,31 @@ vi.mock("../BrowserSessionRow", () => ({ })) vi.mock("../ChatRow", () => ({ - default: function MockChatRow({ message }: { message: ClineMessage }) { - return
{JSON.stringify(message)}
+ default: function MockChatRow({ + message, + isFollowUpAnswered, + onSuggestionClick, + }: { + message: ClineMessage + isFollowUpAnswered?: boolean + onSuggestionClick?: (suggestion: SuggestionItem, event?: React.MouseEvent) => void + }) { + return ( +
+ {message.ask === "followup" && ( + + )} + {JSON.stringify(message)} +
+ ) }, })) @@ -315,6 +347,19 @@ const renderChatView = (props: Partial = {}) => { ) } +const expectMonotonicMarkerTimes = (markers: FollowUpInteractionMarker[]): void => { + const markerTimes = markers.map((marker) => marker.atMs) + expect( + markerTimes.every((time, index) => { + if (index === 0) { + return true + } + + return time >= markerTimes[index - 1] + }), + ).toBe(true) +} + describe("ChatView - Sound Playing Tests", () => { beforeEach(() => vi.clearAllMocks()) @@ -1082,7 +1127,7 @@ describe("ChatView - Message Queueing Tests", () => { ) }) - it("queues messages during command_output state instead of losing them", async () => { + it("queues the first interaction immediately during command_output without effect-sync delays", async () => { const { getByTestId } = renderChatView() // Hydrate state with command_output ask (Proceed While Running state) @@ -1104,17 +1149,11 @@ describe("ChatView - Message Queueing Tests", () => { ], }) - // Wait for state to be updated - need to allow time for React effects to propagate - // (clineAsk state update -> clineAskRef.current update) + // Wait for state to be updated await waitFor(() => { expect(getByTestId("chat-textarea")).toBeInTheDocument() }) - // Allow React effects to complete (clineAsk -> clineAskRef sync) - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 50)) - }) - // Clear message calls before simulating user input vi.mocked(vscode.postMessage).mockClear() @@ -1145,6 +1184,1304 @@ describe("ChatView - Message Queueing Tests", () => { }) }) +describe("ChatView - Primary Action Responsiveness", () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(vscode.postMessage).mockClear() + }) + + it.each([ + { + name: "primary approval", + buttonLabel: "chat:runCommand.title", + invoke: "primaryButtonClick", + expectedMessage: { type: "askResponse", askResponse: "yesButtonClicked" }, + }, + { + name: "secondary rejection", + buttonLabel: "chat:reject.title", + invoke: "secondaryButtonClick", + expectedMessage: { type: "askResponse", askResponse: "noButtonClicked" }, + }, + ] as const)( + "emits only one dispatch path for $name when duplicate activation is attempted while pending", + async ({ buttonLabel, invoke, expectedMessage }) => { + const { getByRole } = renderChatView() + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2_000, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: Date.now(), + text: "", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(getByRole("button", { name: buttonLabel })).toBeInTheDocument() + }) + + vi.mocked(vscode.postMessage).mockClear() + + act(() => { + fireEvent.click(getByRole("button", { name: buttonLabel })) + }) + + await act(async () => { + window.dispatchEvent( + new MessageEvent("message", { + data: { + type: "invoke", + invoke, + }, + }), + ) + }) + + expect(vscode.postMessage).toHaveBeenCalledTimes(1) + expect(vscode.postMessage).toHaveBeenCalledWith(expectedMessage) + }, + ) + + it("applies optimistic pending UI immediately when the primary action is clicked", async () => { + const { getByRole, getByTestId, queryByRole } = renderChatView() + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2_000, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: Date.now(), + text: "", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(getByRole("button", { name: "chat:runCommand.title" })).toBeInTheDocument() + }) + + const input = getByTestId("chat-textarea").querySelector("input") as HTMLInputElement + const primaryButton = getByRole("button", { name: "chat:runCommand.title" }) + + expect(input.getAttribute("data-sending-disabled")).toBe("false") + + act(() => { + fireEvent.click(primaryButton) + }) + + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "askResponse", askResponse: "yesButtonClicked" }) + expect(input.getAttribute("data-sending-disabled")).toBe("true") + expect(queryByRole("button", { name: "chat:runCommand.title" })).not.toBeInTheDocument() + }) + + it("keeps pending state stable through async ask-resolution updates", async () => { + const { getByRole, getByTestId, queryByRole } = renderChatView() + + const taskTs = Date.now() - 2_000 + const askTs = Date.now() - 1_000 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: taskTs, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: askTs, + text: "", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(getByRole("button", { name: "chat:runCommand.title" })).toBeInTheDocument() + }) + + const input = getByTestId("chat-textarea").querySelector("input") as HTMLInputElement + + act(() => { + fireEvent.click(getByRole("button", { name: "chat:runCommand.title" })) + }) + + expect(input.getAttribute("data-sending-disabled")).toBe("true") + expect(queryByRole("button", { name: "chat:runCommand.title" })).not.toBeInTheDocument() + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: taskTs, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: askTs, + text: "", + partial: false, + }, + { + type: "say", + say: "api_req_started", + ts: Date.now(), + text: JSON.stringify({ apiProtocol: "anthropic" }), + }, + ], + }) + + await waitFor(() => { + expect(input.getAttribute("data-sending-disabled")).toBe("true") + expect(queryByRole("button", { name: "chat:runCommand.title" })).not.toBeInTheDocument() + }) + }) + + it("applies optimistic pending UI immediately when the secondary reject action is clicked", async () => { + const { getByRole, getByTestId, queryByRole } = renderChatView() + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2_000, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: Date.now(), + text: "", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(getByRole("button", { name: "chat:reject.title" })).toBeInTheDocument() + }) + + const input = getByTestId("chat-textarea").querySelector("input") as HTMLInputElement + const secondaryButton = getByRole("button", { name: "chat:reject.title" }) + + expect(input.getAttribute("data-sending-disabled")).toBe("false") + + act(() => { + fireEvent.click(secondaryButton) + }) + + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "askResponse", askResponse: "noButtonClicked" }) + expect(input.getAttribute("data-sending-disabled")).toBe("true") + expect(queryByRole("button", { name: "chat:runCommand.title" })).not.toBeInTheDocument() + expect(queryByRole("button", { name: "chat:reject.title" })).not.toBeInTheDocument() + }) + + it("restores deterministic error controls after secondary rejection settle", async () => { + const { getByRole, getByTestId, queryByRole } = renderChatView() + + const taskTs = Date.now() - 2_000 + const askTs = Date.now() - 1_000 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: taskTs, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: askTs, + text: "", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(getByRole("button", { name: "chat:reject.title" })).toBeInTheDocument() + }) + + const input = getByTestId("chat-textarea").querySelector("input") as HTMLInputElement + + act(() => { + fireEvent.click(getByRole("button", { name: "chat:reject.title" })) + }) + + expect(input.getAttribute("data-sending-disabled")).toBe("true") + expect(queryByRole("button", { name: "chat:runCommand.title" })).not.toBeInTheDocument() + expect(queryByRole("button", { name: "chat:reject.title" })).not.toBeInTheDocument() + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: taskTs, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: askTs, + text: "", + partial: false, + }, + { + type: "say", + say: "error", + ts: Date.now(), + text: "Tool rejected by user", + }, + { + type: "ask", + ask: "api_req_failed", + ts: Date.now() + 1, + text: "Tool rejected by user", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(input.getAttribute("data-sending-disabled")).toBe("true") + expect(getByRole("button", { name: "chat:retry.title" })).toBeInTheDocument() + expect(getByRole("button", { name: "chat:startNewTask.title" })).toBeInTheDocument() + }) + }) + + it("applies optimistic pending transition when primary action is invoked via extension message", async () => { + const { getByRole, getByTestId, queryByRole } = renderChatView() + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2_000, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: Date.now(), + text: "", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(getByRole("button", { name: "chat:runCommand.title" })).toBeInTheDocument() + }) + + vi.mocked(vscode.postMessage).mockClear() + + const input = getByTestId("chat-textarea").querySelector("input") as HTMLInputElement + expect(input.getAttribute("data-sending-disabled")).toBe("false") + + await act(async () => { + window.dispatchEvent( + new MessageEvent("message", { + data: { + type: "invoke", + invoke: "primaryButtonClick", + }, + }), + ) + }) + + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "askResponse", askResponse: "yesButtonClicked" }) + expect(input.getAttribute("data-sending-disabled")).toBe("true") + expect(queryByRole("button", { name: "chat:runCommand.title" })).not.toBeInTheDocument() + }) + + it("blocks duplicate primary invoke activations while approval action resolution is pending", async () => { + const { getByRole } = renderChatView() + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2_000, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: Date.now(), + text: "", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(getByRole("button", { name: "chat:runCommand.title" })).toBeInTheDocument() + }) + + vi.mocked(vscode.postMessage).mockClear() + + await act(async () => { + window.dispatchEvent( + new MessageEvent("message", { + data: { + type: "invoke", + invoke: "primaryButtonClick", + }, + }), + ) + + window.dispatchEvent( + new MessageEvent("message", { + data: { + type: "invoke", + invoke: "primaryButtonClick", + }, + }), + ) + }) + + expect(vscode.postMessage).toHaveBeenCalledTimes(1) + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "askResponse", askResponse: "yesButtonClicked" }) + }) + + it("applies optimistic pending transition when secondary action is invoked via extension message", async () => { + const { getByRole, getByTestId, queryByRole } = renderChatView() + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2_000, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: Date.now(), + text: "", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(getByRole("button", { name: "chat:reject.title" })).toBeInTheDocument() + }) + + vi.mocked(vscode.postMessage).mockClear() + + const input = getByTestId("chat-textarea").querySelector("input") as HTMLInputElement + expect(input.getAttribute("data-sending-disabled")).toBe("false") + + await act(async () => { + window.dispatchEvent( + new MessageEvent("message", { + data: { + type: "invoke", + invoke: "secondaryButtonClick", + }, + }), + ) + }) + + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "askResponse", askResponse: "noButtonClicked" }) + expect(input.getAttribute("data-sending-disabled")).toBe("true") + expect(queryByRole("button", { name: "chat:runCommand.title" })).not.toBeInTheDocument() + expect(queryByRole("button", { name: "chat:reject.title" })).not.toBeInTheDocument() + }) + + it("blocks duplicate secondary invoke activations while approval action resolution is pending", async () => { + const { getByRole } = renderChatView() + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2_000, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: Date.now(), + text: "", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(getByRole("button", { name: "chat:reject.title" })).toBeInTheDocument() + }) + + vi.mocked(vscode.postMessage).mockClear() + + await act(async () => { + window.dispatchEvent( + new MessageEvent("message", { + data: { + type: "invoke", + invoke: "secondaryButtonClick", + }, + }), + ) + + window.dispatchEvent( + new MessageEvent("message", { + data: { + type: "invoke", + invoke: "secondaryButtonClick", + }, + }), + ) + }) + + expect(vscode.postMessage).toHaveBeenCalledTimes(1) + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "askResponse", askResponse: "noButtonClicked" }) + }) + + it("keeps approval lock stable across rerenders and deterministically re-enables terminal controls after completion", async () => { + const { getByRole, queryByRole, rerender } = renderChatView() + + const taskTs = Date.now() - 2_000 + const commandAskTs = Date.now() - 1_000 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: taskTs, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: commandAskTs, + text: "", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(getByRole("button", { name: "chat:runCommand.title" })).toBeInTheDocument() + }) + + vi.mocked(vscode.postMessage).mockClear() + + act(() => { + fireEvent.click(getByRole("button", { name: "chat:runCommand.title" })) + }) + + expect(vscode.postMessage).toHaveBeenCalledTimes(1) + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "askResponse", askResponse: "yesButtonClicked" }) + expect(queryByRole("button", { name: "chat:runCommand.title" })).not.toBeInTheDocument() + + rerender( + + + + + , + ) + + await act(async () => { + window.dispatchEvent( + new MessageEvent("message", { + data: { + type: "invoke", + invoke: "primaryButtonClick", + }, + }), + ) + }) + + expect(vscode.postMessage).toHaveBeenCalledTimes(1) + expect(queryByRole("button", { name: "chat:runCommand.title" })).not.toBeInTheDocument() + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: taskTs, + text: "Initial task", + }, + { + type: "ask", + ask: "command", + ts: commandAskTs, + text: "", + partial: false, + }, + { + type: "ask", + ask: "completion_result", + ts: Date.now(), + text: "Completed", + partial: false, + }, + ], + }) + + await waitFor(() => { + const startNewTaskButton = getByRole("button", { name: "chat:startNewTask.title" }) + expect(startNewTaskButton).toBeInTheDocument() + expect(startNewTaskButton).toBeEnabled() + }) + + vi.mocked(vscode.postMessage).mockClear() + + act(() => { + fireEvent.click(getByRole("button", { name: "chat:startNewTask.title" })) + }) + + expect(vscode.postMessage).toHaveBeenCalledTimes(1) + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "clearTask" }) + }) +}) + +describe("ChatView - Follow-up Responsiveness Guards", () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(vscode.postMessage).mockClear() + }) + + afterEach(() => { + setFollowUpInteractionInstrumentationSink(undefined) + }) + + it("marks historical follow-up rows as answered while keeping only the newest follow-up actionable", async () => { + const { getByTestId } = renderChatView() + + const olderFollowUpTs = 2_000 + const newerFollowUpTs = 4_000 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: 1_000, + text: "Initial task", + }, + { + type: "ask", + ask: "followup", + ts: olderFollowUpTs, + text: "Older follow-up", + }, + { + type: "say", + say: "text", + ts: 3_000, + text: "Interleaved text", + }, + { + type: "ask", + ask: "followup", + ts: newerFollowUpTs, + text: "Newest follow-up", + }, + ], + }) + + await waitFor(() => { + const olderRow = getByTestId(`chat-row-${olderFollowUpTs}`) + const newerRow = getByTestId(`chat-row-${newerFollowUpTs}`) + + expect(olderRow).toHaveAttribute("data-message-ask", "followup") + expect(newerRow).toHaveAttribute("data-message-ask", "followup") + + expect(olderRow).toHaveAttribute("data-followup-answered", "true") + expect(newerRow).toHaveAttribute("data-followup-answered", "false") + }) + }) + + it("clears active follow-up row controls on the next render cycle after user answers", async () => { + const { getByTestId } = renderChatView() + + const followUpTs = 2_000 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: 1_000, + text: "Initial task", + }, + { + type: "ask", + ask: "followup", + ts: followUpTs, + text: "Should I continue?", + }, + ], + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "false") + }) + + vi.mocked(vscode.postMessage).mockClear() + + const chatTextArea = getByTestId("chat-textarea") + const input = chatTextArea.querySelector("input")! as HTMLInputElement + + await act(async () => { + fireEvent.change(input, { target: { value: "Proceed with this" } }) + fireEvent.keyDown(input, { key: "Enter", code: "Enter" }) + }) + + await waitFor(() => { + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: "askResponse", + askResponse: "messageResponse", + text: "Proceed with this", + images: [], + }) + + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "true") + }) + }) + + it("blocks duplicate typed follow-up submit dispatches while pending and emits a single click marker", async () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + const { getByTestId } = renderChatView() + + const followUpTs = 2_050 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: 1_000, + text: "Initial task", + }, + { + type: "ask", + ask: "followup", + ts: followUpTs, + text: "Should I continue?", + }, + ], + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "false") + }) + + vi.mocked(vscode.postMessage).mockClear() + + const chatTextArea = getByTestId("chat-textarea") + const input = chatTextArea.querySelector("input")! as HTMLInputElement + + await act(async () => { + fireEvent.change(input, { target: { value: "Proceed with this" } }) + fireEvent.keyDown(input, { key: "Enter", code: "Enter" }) + fireEvent.keyDown(input, { key: "Enter", code: "Enter" }) + }) + + const postMessages = vi.mocked(vscode.postMessage).mock.calls.map(([message]) => message) + const askResponseCalls = postMessages.filter((message) => { + if (typeof message !== "object" || message === null || !("type" in message)) { + return false + } + + return message.type === "askResponse" + }) + + expect(askResponseCalls).toHaveLength(1) + expect(askResponseCalls[0]).toEqual({ + type: "askResponse", + askResponse: "messageResponse", + text: "Proceed with this", + images: [], + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "true") + }) + + await waitFor(() => { + expect(markers.map((marker) => marker.stage)).toEqual(["click", "pending_render", "settle"]) + }) + expect(markers.map((marker) => marker.source)).toEqual(["chat_view", "chat_view", "chat_view"]) + expect(markers.map((marker) => marker.followUpTs)).toEqual([followUpTs, followUpTs, followUpTs]) + expectMonotonicMarkerTimes(markers) + }) + + it("keeps typed follow-up submit single-dispatch and forward-only markers under rerender pressure", async () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + const { getByTestId, rerender } = renderChatView() + + const followUpTs = 2_075 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: 1_000, + text: "Initial task", + }, + { + type: "ask", + ask: "followup", + ts: followUpTs, + text: "Should I continue?", + }, + ], + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "false") + }) + + vi.mocked(vscode.postMessage).mockClear() + + const firstInput = getByTestId("chat-textarea").querySelector("input") as HTMLInputElement + + await act(async () => { + fireEvent.change(firstInput, { target: { value: "Proceed with this" } }) + fireEvent.keyDown(firstInput, { key: "Enter", code: "Enter" }) + }) + + rerender( + + + + + , + ) + + const secondInput = getByTestId("chat-textarea").querySelector("input") as HTMLInputElement + + await act(async () => { + fireEvent.keyDown(secondInput, { key: "Enter", code: "Enter" }) + }) + + const postMessages = vi.mocked(vscode.postMessage).mock.calls.map(([message]) => message) + const askResponseCalls = postMessages.filter((message) => { + if (typeof message !== "object" || message === null || !("type" in message)) { + return false + } + + return message.type === "askResponse" + }) + + expect(askResponseCalls).toHaveLength(1) + expect(askResponseCalls[0]).toEqual({ + type: "askResponse", + askResponse: "messageResponse", + text: "Proceed with this", + images: [], + }) + + await waitFor(() => { + expect(markers.map((marker) => marker.stage)).toEqual(["click", "pending_render", "settle"]) + }) + expect(markers.map((marker) => marker.followUpTs)).toEqual([followUpTs, followUpTs, followUpTs]) + expectMonotonicMarkerTimes(markers) + }) + + it("marks the active follow-up as terminal in the first post-settle render cycle after auto-approval acceptance", async () => { + const { getByTestId } = renderChatView() + + const followUpTs = 2_000 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: 1_000, + text: "Initial task", + }, + { + type: "ask", + ask: "followup", + ts: followUpTs, + text: JSON.stringify({ + question: "Should I continue?", + suggest: [{ answer: "Proceed" }, { answer: "Pause" }], + }), + }, + ], + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "false") + }) + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: 1_000, + text: "Initial task", + }, + { + type: "ask", + ask: "followup", + ts: followUpTs, + text: JSON.stringify({ + question: "Should I continue?", + suggest: [{ answer: "Proceed" }, { answer: "Pause" }], + }), + }, + { + type: "say", + say: "user_feedback", + ts: 3_000, + text: "Proceed", + }, + ], + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "true") + }) + }) + + it("emits deterministic pending/settle/clear markers for follow-up answer lifecycle", async () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + const { getByTestId, queryByTestId } = renderChatView() + + const followUpTs = 2_000 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: 1_000, + text: "Initial task", + }, + { + type: "ask", + ask: "followup", + ts: followUpTs, + text: "Should I continue?", + }, + ], + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "false") + }) + + const chatTextArea = getByTestId("chat-textarea") + const input = chatTextArea.querySelector("input") as HTMLInputElement + + await act(async () => { + fireEvent.change(input, { target: { value: "Proceed" } }) + fireEvent.keyDown(input, { key: "Enter", code: "Enter" }) + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "true") + }) + + mockPostMessage({ clineMessages: [] }) + + await waitFor(() => { + expect(queryByTestId(`chat-row-${followUpTs}`)).toBeNull() + }) + + await waitFor(() => { + expect(markers.map((marker) => marker.stage)).toEqual(["click", "pending_render", "settle", "clear"]) + }) + + expect(markers.map((marker) => marker.followUpTs)).toEqual([followUpTs, followUpTs, followUpTs, followUpTs]) + expect(markers.map((marker) => marker.source)).toEqual(["chat_view", "chat_view", "chat_view", "chat_view"]) + expect(markers.every((marker) => typeof marker.atMs === "number")).toBe(true) + expectMonotonicMarkerTimes(markers) + }) + + it("uses current ask interaction state for suggestion clicks and emits pending->settle->clear once", async () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + const { getByTestId, queryByTestId } = renderChatView() + + const followUpTs = 2_100 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: 1_000, + text: "Initial task", + }, + { + type: "ask", + ask: "followup", + ts: followUpTs, + text: JSON.stringify({ + question: "Choose a follow-up path", + suggest: [{ answer: "Proceed", mode: "code" }], + }), + }, + ], + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "false") + expect(getByTestId(`mock-followup-suggestion-${followUpTs}`)).toBeInTheDocument() + }) + + vi.mocked(vscode.postMessage).mockClear() + + act(() => { + fireEvent.click(getByTestId(`mock-followup-suggestion-${followUpTs}`)) + }) + + await waitFor(() => { + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "mode", text: "code" }) + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: "askResponse", + askResponse: "messageResponse", + text: "Mock suggestion", + images: [], + }) + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "true") + }) + + await waitFor(() => { + expect(markers.map((marker) => marker.stage)).toEqual(["pending_render", "settle"]) + }) + + mockPostMessage({ clineMessages: [] }) + + await waitFor(() => { + expect(queryByTestId(`chat-row-${followUpTs}`)).toBeNull() + }) + + await waitFor(() => { + expect(markers.map((marker) => marker.stage)).toEqual(["pending_render", "settle", "clear"]) + }) + expect(markers.map((marker) => marker.followUpTs)).toEqual([followUpTs, followUpTs, followUpTs]) + expectMonotonicMarkerTimes(markers) + }) + + it("blocks duplicate follow-up suggestion dispatch while the follow-up suggestion action is pending", async () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + const { getByTestId, queryByTestId } = renderChatView() + + const followUpTs = 2_120 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: 1_000, + text: "Initial task", + }, + { + type: "ask", + ask: "followup", + ts: followUpTs, + text: JSON.stringify({ + question: "Choose a follow-up path", + suggest: [{ answer: "Proceed", mode: "code" }], + }), + }, + ], + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "false") + expect(getByTestId(`mock-followup-suggestion-${followUpTs}`)).toBeInTheDocument() + }) + + vi.mocked(vscode.postMessage).mockClear() + + const suggestionButton = getByTestId(`mock-followup-suggestion-${followUpTs}`) + + act(() => { + fireEvent.click(suggestionButton) + fireEvent.click(suggestionButton) + }) + + await waitFor(() => { + expect(vscode.postMessage).toHaveBeenCalledWith({ type: "mode", text: "code" }) + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: "askResponse", + askResponse: "messageResponse", + text: "Mock suggestion", + images: [], + }) + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "true") + }) + + const postMessages = vi.mocked(vscode.postMessage).mock.calls.map(([message]) => message) + const askResponseCalls = postMessages.filter((message) => { + if (typeof message !== "object" || message === null || !("type" in message)) { + return false + } + + return message.type === "askResponse" + }) + const modeCalls = postMessages.filter((message) => { + if (typeof message !== "object" || message === null || !("type" in message)) { + return false + } + + return message.type === "mode" + }) + + expect(askResponseCalls).toHaveLength(1) + expect(modeCalls).toHaveLength(1) + + await waitFor(() => { + expect(markers.map((marker) => marker.stage)).toEqual(["pending_render", "settle"]) + }) + + mockPostMessage({ clineMessages: [] }) + + await waitFor(() => { + expect(queryByTestId(`chat-row-${followUpTs}`)).toBeNull() + }) + + await waitFor(() => { + expect(markers.map((marker) => marker.stage)).toEqual(["pending_render", "settle", "clear"]) + }) + expect(markers.map((marker) => marker.followUpTs)).toEqual([followUpTs, followUpTs, followUpTs]) + expectMonotonicMarkerTimes(markers) + }) + + it("keeps follow-up lifecycle forward-only when clear is re-attempted across rerenders", async () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + const { getByTestId, queryByTestId, rerender } = renderChatView() + + const followUpTs = 2_130 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: 1_000, + text: "Initial task", + }, + { + type: "ask", + ask: "followup", + ts: followUpTs, + text: "Should I continue?", + }, + ], + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${followUpTs}`)).toHaveAttribute("data-followup-answered", "false") + }) + + const input = getByTestId("chat-textarea").querySelector("input") as HTMLInputElement + + await act(async () => { + fireEvent.change(input, { target: { value: "Proceed" } }) + fireEvent.keyDown(input, { key: "Enter", code: "Enter" }) + }) + + await waitFor(() => { + expect(markers.map((marker) => marker.stage)).toEqual(["click", "pending_render", "settle"]) + }) + + mockPostMessage({ clineMessages: [] }) + + await waitFor(() => { + expect(queryByTestId(`chat-row-${followUpTs}`)).toBeNull() + }) + + await waitFor(() => { + expect(markers.map((marker) => marker.stage)).toEqual(["click", "pending_render", "settle", "clear"]) + }) + + rerender( + + + + + , + ) + + mockPostMessage({ clineMessages: [] }) + + await act(async () => { + await Promise.resolve() + }) + + expect(markers.map((marker) => marker.stage)).toEqual(["click", "pending_render", "settle", "clear"]) + expect(markers.map((marker) => marker.followUpTs)).toEqual([followUpTs, followUpTs, followUpTs, followUpTs]) + expectMonotonicMarkerTimes(markers) + }) + + it("handles failed follow-up attempt then retry with deterministic per-attempt marker progression", async () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + const { getByTestId } = renderChatView() + + const firstFollowUpTs = 2_140 + const retryFollowUpTs = 2_141 + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: 1_000, + text: "Initial task", + }, + { + type: "ask", + ask: "followup", + ts: firstFollowUpTs, + text: "First follow-up", + }, + ], + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${firstFollowUpTs}`)).toHaveAttribute("data-followup-answered", "false") + }) + + const input = getByTestId("chat-textarea").querySelector("input") as HTMLInputElement + + await act(async () => { + fireEvent.change(input, { target: { value: "Attempt one" } }) + fireEvent.keyDown(input, { key: "Enter", code: "Enter" }) + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${firstFollowUpTs}`)).toHaveAttribute("data-followup-answered", "true") + expect(markers.map((marker) => marker.stage)).toEqual(["click", "pending_render", "settle"]) + }) + + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: 1_000, + text: "Initial task", + }, + { + type: "ask", + ask: "followup", + ts: firstFollowUpTs, + text: "First follow-up", + }, + { + type: "say", + say: "error", + ts: 3_000, + text: "Attempt failed", + }, + { + type: "ask", + ask: "followup", + ts: retryFollowUpTs, + text: "Retry follow-up", + }, + ], + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${firstFollowUpTs}`)).toHaveAttribute("data-followup-answered", "true") + expect(getByTestId(`chat-row-${retryFollowUpTs}`)).toHaveAttribute("data-followup-answered", "false") + }) + + await act(async () => { + fireEvent.change(input, { target: { value: "Attempt two" } }) + fireEvent.keyDown(input, { key: "Enter", code: "Enter" }) + }) + + await waitFor(() => { + expect(getByTestId(`chat-row-${retryFollowUpTs}`)).toHaveAttribute("data-followup-answered", "true") + }) + + expect(markers.map((marker) => marker.stage)).toEqual([ + "click", + "pending_render", + "settle", + "click", + "pending_render", + "settle", + ]) + expect(markers.map((marker) => marker.followUpTs)).toEqual([ + firstFollowUpTs, + firstFollowUpTs, + firstFollowUpTs, + retryFollowUpTs, + retryFollowUpTs, + retryFollowUpTs, + ]) + expectMonotonicMarkerTimes(markers) + }) +}) + describe("ChatView - Context Condensing Indicator Tests", () => { beforeEach(() => { vi.clearAllMocks() @@ -1155,6 +2492,9 @@ describe("ChatView - Context Condensing Indicator Tests", () => { // the isCondensing state is set to true and a synthetic condensing message is added // to the grouped messages list const { getByTestId, container } = renderChatView() + const taskTs = 11_000 + const apiReqStartedTs = 12_000 + const expectedSyntheticCondenseTs = -taskTs // First hydrate state with an active task mockPostMessage({ @@ -1162,13 +2502,13 @@ describe("ChatView - Context Condensing Indicator Tests", () => { { type: "say", say: "task", - ts: Date.now() - 2000, + ts: taskTs, text: "Initial task", }, { type: "say", say: "api_req_started", - ts: Date.now() - 1000, + ts: apiReqStartedTs, text: JSON.stringify({ apiProtocol: "anthropic" }), }, ], @@ -1202,7 +2542,8 @@ describe("ChatView - Context Condensing Indicator Tests", () => { // With Virtuoso mocked, items render directly and we can find the ChatRow with partial condense_context message await waitFor( () => { - const rows = container.querySelectorAll('[data-testid="chat-row"]') + const rows = container.querySelectorAll('[data-testid^="chat-row-"]') + expect(getByTestId(`chat-row-${expectedSyntheticCondenseTs}`)).toBeInTheDocument() // Check for the actual message structure: partial condense_context message const condensingRow = Array.from(rows).find((row) => { const text = row.textContent || "" diff --git a/webview-ui/src/components/chat/__tests__/FollowUpSuggest.spec.tsx b/webview-ui/src/components/chat/__tests__/FollowUpSuggest.spec.tsx index c2f2d56f34..bcec042ae8 100644 --- a/webview-ui/src/components/chat/__tests__/FollowUpSuggest.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/FollowUpSuggest.spec.tsx @@ -1,8 +1,12 @@ import React, { createContext, useContext } from "react" -import { render, screen, act } from "@testing-library/react" +import { render, screen, act, fireEvent } from "@testing-library/react" import { TooltipProvider } from "@radix-ui/react-tooltip" import { FollowUpSuggest } from "../FollowUpSuggest" +import { + setFollowUpInteractionInstrumentationSink, + type FollowUpInteractionMarker, +} from "../followUpInteractionInstrumentation" // Mock the translation hook vi.mock("@src/i18n/TranslationContext", () => ({ @@ -81,6 +85,7 @@ describe("FollowUpSuggest", () => { afterEach(() => { vi.useRealTimers() + setFollowUpInteractionInstrumentationSink(undefined) }) it("should display countdown timer when auto-approval is enabled", () => { @@ -289,6 +294,303 @@ describe("FollowUpSuggest", () => { expect(mockOnCancelAutoApproval).toHaveBeenCalled() }) + it("should hide follow-up controls immediately after accepting a suggestion", () => { + renderWithTestProviders( + , + defaultTestState, + ) + + const firstSuggestionButton = screen.getByRole("button", { name: "First suggestion" }) + fireEvent.click(firstSuggestionButton) + + expect(mockOnSuggestionClick).toHaveBeenCalledWith(mockSuggestions[0], expect.any(Object)) + expect(mockOnCancelAutoApproval).toHaveBeenCalled() + + // Terminal follow-up state should remove actionable controls on the next render cycle. + expect(screen.queryByRole("button", { name: "First suggestion" })).not.toBeInTheDocument() + expect(screen.queryByRole("button", { name: "Second suggestion" })).not.toBeInTheDocument() + expect(screen.queryByText(/Selecting in \d+s/)).not.toBeInTheDocument() + }) + + it("emits deterministic click instrumentation marker when a suggestion is clicked", () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + renderWithTestProviders( + , + defaultTestState, + ) + + const firstSuggestionButton = screen.getByRole("button", { name: "First suggestion" }) + fireEvent.click(firstSuggestionButton) + + expect(markers).toHaveLength(1) + expect(markers[0]).toMatchObject({ + stage: "click", + followUpTs: 123, + source: "follow_up_suggest", + }) + expect(typeof markers[0].atMs).toBe("number") + }) + + it("prevents duplicate non-shift clicks from re-firing handler and instrumentation after terminal transition", () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + renderWithTestProviders( + , + defaultTestState, + ) + + const firstSuggestionButton = screen.getByRole("button", { name: "First suggestion" }) + + fireEvent.click(firstSuggestionButton) + // Immediate second click before React commit should be blocked by followUpTerminalRef. + fireEvent.click(firstSuggestionButton) + + expect(mockOnSuggestionClick).toHaveBeenCalledTimes(1) + expect(mockOnCancelAutoApproval).toHaveBeenCalled() + expect(markers.map((marker) => marker.stage)).toEqual(["click"]) + expect(markers[0]).toMatchObject({ + followUpTs: 123, + source: "follow_up_suggest", + }) + }) + + /** + * Verifies state-machine style suppression when a second suggestion click is attempted + * before the first click's terminal transition has fully committed to the DOM. + */ + it("rejects rapid cross-suggestion clicks while pending, allowing only the first dispatch", () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + renderWithTestProviders( + , + defaultTestState, + ) + + const firstSuggestionButton = screen.getByRole("button", { name: "First suggestion" }) + const secondSuggestionButton = screen.getByRole("button", { name: "Second suggestion" }) + + fireEvent.click(firstSuggestionButton) + // Attempt a competing click against another option in the same interaction turn. + fireEvent.click(secondSuggestionButton) + + expect(mockOnSuggestionClick).toHaveBeenCalledTimes(1) + expect(mockOnSuggestionClick).toHaveBeenCalledWith(mockSuggestions[0], expect.any(Object)) + expect(markers.map((marker) => marker.stage)).toEqual(["click"]) + expect(screen.queryByRole("button", { name: "First suggestion" })).not.toBeInTheDocument() + expect(screen.queryByRole("button", { name: "Second suggestion" })).not.toBeInTheDocument() + }) + + it("keeps follow-up actionable for shift-click copy behavior, then transitions terminal on normal click", () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + renderWithTestProviders( + , + defaultTestState, + ) + + const firstSuggestionButton = screen.getByRole("button", { name: "First suggestion" }) + + fireEvent.click(firstSuggestionButton, { shiftKey: true }) + + expect(mockOnSuggestionClick).toHaveBeenCalledTimes(1) + expect(mockOnSuggestionClick).toHaveBeenNthCalledWith( + 1, + mockSuggestions[0], + expect.objectContaining({ shiftKey: true }), + ) + // Shift-click should not terminalize the follow-up controls. + expect(screen.getByRole("button", { name: "First suggestion" })).toBeInTheDocument() + + fireEvent.click(screen.getByRole("button", { name: "First suggestion" })) + + expect(mockOnSuggestionClick).toHaveBeenCalledTimes(2) + expect(mockOnSuggestionClick).toHaveBeenNthCalledWith( + 2, + mockSuggestions[0], + expect.objectContaining({ shiftKey: false }), + ) + expect(screen.queryByRole("button", { name: "First suggestion" })).not.toBeInTheDocument() + expect(markers.map((marker) => marker.stage)).toEqual(["click", "click"]) + }) + + /** + * Ensures once terminalized, rerenders cannot regress the component back to an actionable state. + * This guards against invalid/out-of-order lifecycle progression under parent rerenders. + */ + it("accepts only forward follow-up lifecycle progression under rerender pressure", () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + const { rerender } = renderWithTestProviders( + , + defaultTestState, + ) + + const firstSuggestionButton = screen.getByRole("button", { name: "First suggestion" }) + fireEvent.click(firstSuggestionButton) + + expect(mockOnSuggestionClick).toHaveBeenCalledTimes(1) + expect(markers.map((marker) => marker.stage)).toEqual(["click"]) + expect(screen.queryByRole("button", { name: "First suggestion" })).not.toBeInTheDocument() + + const rerenderSequence: boolean[] = [false, true, false] + for (const isAnswered of rerenderSequence) { + rerender( + + + + + , + ) + + expect(screen.queryByRole("button", { name: "First suggestion" })).not.toBeInTheDocument() + expect(screen.queryByRole("button", { name: "Second suggestion" })).not.toBeInTheDocument() + } + + expect(mockOnSuggestionClick).toHaveBeenCalledTimes(1) + }) + + it("keeps answered state forward-only for a single follow-up interaction even if parent rerenders isAnswered out-of-order", () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + const { rerender } = renderWithTestProviders( + , + defaultTestState, + ) + + expect(screen.queryByRole("button", { name: "First suggestion" })).not.toBeInTheDocument() + + rerender( + + + + + , + ) + + expect(screen.queryByRole("button", { name: "First suggestion" })).not.toBeInTheDocument() + expect(screen.queryByRole("button", { name: "Second suggestion" })).not.toBeInTheDocument() + expect(markers).toHaveLength(0) + expect(mockOnSuggestionClick).not.toHaveBeenCalled() + }) + + it("allows deterministic retry on remount without stale disabled or hidden controls", () => { + const markers: FollowUpInteractionMarker[] = [] + setFollowUpInteractionInstrumentationSink((marker) => { + markers.push(marker) + }) + + const firstRender = renderWithTestProviders( + , + defaultTestState, + ) + + fireEvent.click(screen.getByRole("button", { name: "First suggestion" })) + + expect(mockOnSuggestionClick).toHaveBeenCalledTimes(1) + expect(screen.queryByRole("button", { name: "First suggestion" })).not.toBeInTheDocument() + + firstRender.unmount() + + renderWithTestProviders( + , + defaultTestState, + ) + + const retryButton = screen.getByRole("button", { name: "First suggestion" }) + expect(retryButton).toBeEnabled() + + fireEvent.click(retryButton) + + expect(mockOnSuggestionClick).toHaveBeenCalledTimes(2) + expect(markers.map((marker) => marker.stage)).toEqual(["click", "click"]) + expect(markers.map((marker) => marker.followUpTs)).toEqual([123, 456]) + }) + it("should handle race condition when timeout fires but user has already responded", () => { // This test simulates the scenario where: // 1. Auto-approval countdown starts diff --git a/webview-ui/src/components/chat/followUpInteractionInstrumentation.ts b/webview-ui/src/components/chat/followUpInteractionInstrumentation.ts new file mode 100644 index 0000000000..cd5d129fee --- /dev/null +++ b/webview-ui/src/components/chat/followUpInteractionInstrumentation.ts @@ -0,0 +1,45 @@ +export type FollowUpInteractionStage = "click" | "pending_render" | "settle" | "clear" + +export interface FollowUpInteractionMarker { + stage: FollowUpInteractionStage + followUpTs: number | null + source: "follow_up_suggest" | "chat_view" + atMs: number +} + +type FollowUpInteractionSink = (marker: FollowUpInteractionMarker) => void + +let followUpInteractionSink: FollowUpInteractionSink | undefined +let lastMarkerAtMs = 0 + +const getMonotonicNowMs = (): number => { + const rawNow = + typeof performance !== "undefined" && typeof performance.now === "function" ? performance.now() : Date.now() + + if (!Number.isFinite(rawNow)) { + lastMarkerAtMs += 1 + return lastMarkerAtMs + } + + lastMarkerAtMs = Math.max(lastMarkerAtMs, rawNow) + return lastMarkerAtMs +} + +export const setFollowUpInteractionInstrumentationSink = (sink: FollowUpInteractionSink | undefined): void => { + followUpInteractionSink = sink + + if (!sink) { + lastMarkerAtMs = 0 + } +} + +export const emitFollowUpInteractionMarker = (marker: Omit): void => { + if (!followUpInteractionSink) { + return + } + + followUpInteractionSink({ + ...marker, + atMs: getMonotonicNowMs(), + }) +} diff --git a/webview-ui/src/components/chat/usePendingActionContract.ts b/webview-ui/src/components/chat/usePendingActionContract.ts new file mode 100644 index 0000000000..019605e0fc --- /dev/null +++ b/webview-ui/src/components/chat/usePendingActionContract.ts @@ -0,0 +1,33 @@ +import { useCallback, useRef, useState } from "react" + +export interface PendingActionContract { + isPending: boolean + tryBeginPendingAction: () => boolean + clearPendingAction: () => void +} + +export const usePendingActionContract = (): PendingActionContract => { + const pendingActionRef = useRef(false) + const [isPending, setIsPending] = useState(false) + + const tryBeginPendingAction = useCallback(() => { + if (pendingActionRef.current) { + return false + } + + pendingActionRef.current = true + setIsPending(true) + return true + }, []) + + const clearPendingAction = useCallback(() => { + pendingActionRef.current = false + setIsPending(false) + }, []) + + return { + isPending, + tryBeginPendingAction, + clearPendingAction, + } +}