Fixes #4888: Fix spinner persisting in completed tasks UI

webview-ui/src/components/chat/BrowserSessionRow.tsx

@@ -20,6 +20,7 @@ interface BrowserSessionRowProps {
 	isExpanded: (messageTs: number) => boolean
 	onToggleExpand: (messageTs: number) => void
 	lastModifiedMessage?: ClineMessage
+	modifiedMessages: ClineMessage[]
 	isLast: boolean
 	onHeightChange: (isTaller: boolean) => void
 	isStreaming: boolean
@@ -423,6 +424,7 @@ const BrowserSessionRowContent = ({
 	isExpanded,
 	onToggleExpand,
 	lastModifiedMessage,
+	modifiedMessages,
 	isLast,
 	setMaxActionHeight,
 	isStreaming,
@@ -453,6 +455,7 @@ const BrowserSessionRowContent = ({
 									onToggleExpand(message.ts)
 								}}
 								lastModifiedMessage={lastModifiedMessage}
+								modifiedMessages={modifiedMessages}
 								isLast={isLast}
 								isStreaming={isStreaming}
 							/>

webview-ui/src/components/chat/ChatRow.tsx

@@ -43,6 +43,7 @@ import CodebaseSearchResultsDisplay from "./CodebaseSearchResultsDisplay"
 interface ChatRowProps {
 	message: ClineMessage
 	lastModifiedMessage?: ClineMessage
+	modifiedMessages: ClineMessage[]
 	isExpanded: boolean
 	isLast: boolean
 	isStreaming: boolean
@@ -93,6 +94,7 @@ export default ChatRow
 export const ChatRowContent = ({
 	message,
 	lastModifiedMessage,
+	modifiedMessages,
 	isExpanded,
 	isLast,
 	isStreaming,
@@ -129,10 +131,43 @@ export const ChatRowContent = ({
 			? lastModifiedMessage?.text
 			: undefined
 
-	const isCommandExecuting =
-		isLast && lastModifiedMessage?.ask === "command" && lastModifiedMessage?.text?.includes(COMMAND_OUTPUT_STRING)
+	const isCommandExecuting = useMemo(() => {
+		if (
+			!isLast ||
+			lastModifiedMessage?.ask !== "command" ||
+			!lastModifiedMessage?.text?.includes(COMMAND_OUTPUT_STRING)
+		) {
+			return false
+		}
+
+		// Check if there's a subsequent message indicating command completion
+		// Look for any message after the command that would indicate it's finished
+		const commandTs = lastModifiedMessage.ts
+		const hasCompletionMessage = modifiedMessages.some(
+			(msg) =>
+				msg.ts > commandTs &&
+				(msg.say === "api_req_finished" ||
+					msg.say === "completion_result" ||
+					msg.ask === "completion_result" ||
+					(msg.say === "command_output" && !msg.partial)),
+		)
+
+		return !hasCompletionMessage
+	}, [isLast, lastModifiedMessage, modifiedMessages])
+
+	const isMcpServerResponding = useMemo(() => {
+		if (!isLast || lastModifiedMessage?.say !== "mcp_server_request_started") {
+			return false
+		}
+
+		// Check if there's a subsequent MCP response message
+		const mcpRequestTs = lastModifiedMessage.ts
+		const hasMcpResponse = modifiedMessages.some(
+			(msg) => msg.ts > mcpRequestTs && msg.say === "mcp_server_response",
+		)
 
-	const isMcpServerResponding = isLast && lastModifiedMessage?.say === "mcp_server_request_started"
+		return !hasMcpResponse
+	}, [isLast, lastModifiedMessage, modifiedMessages])
 
 	const type = message.type === "ask" ? message.ask : message.say
 
@@ -212,21 +247,31 @@ export const ChatRowContent = ({
 						/>
 					</div>
 				)
+				// Check if this API request has actually finished by looking for completion indicators
+				const apiRequestFinished = cost !== null && cost !== undefined
+				const apiRequestCancelled = apiReqCancelReason !== null && apiReqCancelReason !== undefined
+				const apiRequestFailed = apiRequestFailedMessage !== undefined
+
+				// Only show spinner if the request is truly still in progress
+				const shouldShowSpinner = !apiRequestFinished && !apiRequestCancelled && !apiRequestFailed
+
 				return [
-					apiReqCancelReason !== null && apiReqCancelReason !== undefined ? (
+					apiRequestCancelled ? (
 						apiReqCancelReason === "user_cancelled" ? (
 							getIconSpan("error", cancelledColor)
 						) : (
 							getIconSpan("error", errorColor)
 						)
-					) : cost !== null && cost !== undefined ? (
+					) : apiRequestFinished ? (
 						getIconSpan("check", successColor)
-					) : apiRequestFailedMessage ? (
+					) : apiRequestFailed ? (
 						getIconSpan("error", errorColor)
-					) : (
+					) : shouldShowSpinner ? (
 						<ProgressIndicator />
+					) : (
+						getIconSpan("check", successColor)
 					),
-					apiReqCancelReason !== null && apiReqCancelReason !== undefined ? (
+					apiRequestCancelled ? (
 						apiReqCancelReason === "user_cancelled" ? (
 							<span style={{ color: normalColor, fontWeight: "bold" }}>
 								{t("chat:apiRequest.cancelled")}
@@ -236,12 +281,14 @@ export const ChatRowContent = ({
 								{t("chat:apiRequest.streamingFailed")}
 							</span>
 						)
-					) : cost !== null && cost !== undefined ? (
+					) : apiRequestFinished ? (
 						<span style={{ color: normalColor, fontWeight: "bold" }}>{t("chat:apiRequest.title")}</span>
-					) : apiRequestFailedMessage ? (
+					) : apiRequestFailed ? (
 						<span style={{ color: errorColor, fontWeight: "bold" }}>{t("chat:apiRequest.failed")}</span>
-					) : (
+					) : shouldShowSpinner ? (
 						<span style={{ color: normalColor, fontWeight: "bold" }}>{t("chat:apiRequest.streaming")}</span>
+					) : (
+						<span style={{ color: normalColor, fontWeight: "bold" }}>{t("chat:apiRequest.title")}</span>
 					),
 				]
 			case "followup":

webview-ui/src/components/chat/ChatView.tsx

@@ -1197,6 +1197,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 						messages={messageOrGroup}
 						isLast={index === groupedMessages.length - 1}
 						lastModifiedMessage={modifiedMessages.at(-1)}
+						modifiedMessages={modifiedMessages}
 						onHeightChange={handleRowHeightChange}
 						isStreaming={isStreaming}
 						isExpanded={(messageTs: number) => expandedRows[messageTs] ?? false}
@@ -1218,6 +1219,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 					isExpanded={expandedRows[messageOrGroup.ts] || false}
 					onToggleExpand={toggleRowExpansion} // This was already stabilized
 					lastModifiedMessage={modifiedMessages.at(-1)} // Original direct access
+					modifiedMessages={modifiedMessages}
 					isLast={index === groupedMessages.length - 1} // Original direct access
 					onHeightChange={handleRowHeightChange}
 					isStreaming={isStreaming}

webview-ui/src/components/chat/__tests__/ChatRow.test.tsx

@@ -0,0 +1,203 @@
+import React from "react"
+import { render, screen } from "@testing-library/react"
+import { ChatRowContent } from "../ChatRow"
+import type { ClineMessage } from "@roo-code/types"
+
+// Mock the entire i18n setup
+jest.mock("@src/i18n/TranslationContext", () => ({
+	useAppTranslation: () => ({
+		t: (key: string) => key,
+	}),
+}))
+
+// Mock the translation hook
+jest.mock("react-i18next", () => ({
+	useTranslation: () => ({
+		t: (key: string) => key,
+	}),
+	Trans: ({ children }: { children: React.ReactNode }) => <>{children}</>,
+}))
+
+// Mock the extension state context
+jest.mock("@src/context/ExtensionStateContext", () => ({
+	useExtensionState: () => ({
+		mcpServers: [],
+		alwaysAllowMcp: false,
+		currentCheckpoint: null,
+	}),
+}))
+
+// Mock the clipboard hook
+jest.mock("@src/utils/clipboard", () => ({
+	useCopyToClipboard: () => ({
+		copyWithFeedback: jest.fn(),
+	}),
+}))
+
+// Mock the ProgressIndicator component
+jest.mock("../ProgressIndicator", () => ({
+	ProgressIndicator: () => <div data-testid="progress-indicator">Loading...</div>,
+}))
+
+describe("ChatRow Spinner Logic", () => {
+	const baseMessage: ClineMessage = {
+		ts: 1000,
+		type: "say",
+		say: "api_req_started",
+		text: JSON.stringify({ cost: undefined, cancelReason: undefined }),
+	}
+
+	const baseProps = {
+		isExpanded: false,
+		isLast: true,
+		isStreaming: false,
+		onToggleExpand: jest.fn(),
+		onSuggestionClick: jest.fn(),
+		onBatchFileResponse: jest.fn(),
+	}
+
+	it("should show spinner for ongoing API request", () => {
+		const modifiedMessages: ClineMessage[] = [baseMessage]
+
+		render(
+			<ChatRowContent
+				{...baseProps}
+				message={baseMessage}
+				lastModifiedMessage={baseMessage}
+				modifiedMessages={modifiedMessages}
+			/>,
+		)
+
+		expect(screen.getByTestId("progress-indicator")).toBeInTheDocument()
+	})
+
+	it("should not show spinner when API request is finished", () => {
+		const finishedMessage: ClineMessage = {
+			...baseMessage,
+			text: JSON.stringify({ cost: 0.001, cancelReason: undefined }),
+		}
+		const modifiedMessages: ClineMessage[] = [finishedMessage]
+
+		render(
+			<ChatRowContent
+				{...baseProps}
+				message={finishedMessage}
+				lastModifiedMessage={finishedMessage}
+				modifiedMessages={modifiedMessages}
+			/>,
+		)
+
+		expect(screen.queryByTestId("progress-indicator")).not.toBeInTheDocument()
+	})
+
+	it("should show spinner for ongoing command execution", () => {
+		const commandMessage: ClineMessage = {
+			ts: 1000,
+			type: "ask",
+			ask: "command",
+			text: "echo 'test'\n\n--- COMMAND OUTPUT ---\n",
+		}
+		const modifiedMessages: ClineMessage[] = [commandMessage]
+
+		render(
+			<ChatRowContent
+				{...baseProps}
+				message={commandMessage}
+				lastModifiedMessage={commandMessage}
+				modifiedMessages={modifiedMessages}
+			/>,
+		)
+
+		expect(screen.getByTestId("progress-indicator")).toBeInTheDocument()
+	})
+
+	it("should not show spinner when command execution is completed", () => {
+		const commandMessage: ClineMessage = {
+			ts: 1000,
+			type: "ask",
+			ask: "command",
+			text: "echo 'test'\n\n--- COMMAND OUTPUT ---\n",
+		}
+		const completionMessage: ClineMessage = {
+			ts: 1001,
+			type: "say",
+			say: "api_req_finished",
+		}
+		const modifiedMessages: ClineMessage[] = [commandMessage, completionMessage]
+
+		render(
+			<ChatRowContent
+				{...baseProps}
+				message={commandMessage}
+				lastModifiedMessage={commandMessage}
+				modifiedMessages={modifiedMessages}
+			/>,
+		)
+
+		expect(screen.queryByTestId("progress-indicator")).not.toBeInTheDocument()
+	})
+
+	it("should show spinner for ongoing MCP server request", () => {
+		const mcpMessage: ClineMessage = {
+			ts: 1000,
+			type: "say",
+			say: "mcp_server_request_started",
+		}
+		const modifiedMessages: ClineMessage[] = [mcpMessage]
+
+		render(
+			<ChatRowContent
+				{...baseProps}
+				message={mcpMessage}
+				lastModifiedMessage={mcpMessage}
+				modifiedMessages={modifiedMessages}
+			/>,
+		)
+
+		expect(screen.getByTestId("progress-indicator")).toBeInTheDocument()
+	})
+
+	it("should not show spinner when MCP server request is completed", () => {
+		const mcpRequestMessage: ClineMessage = {
+			ts: 1000,
+			type: "say",
+			say: "mcp_server_request_started",
+		}
+		const mcpResponseMessage: ClineMessage = {
+			ts: 1001,
+			type: "say",
+			say: "mcp_server_response",
+		}
+		const modifiedMessages: ClineMessage[] = [mcpRequestMessage, mcpResponseMessage]
+
+		render(
+			<ChatRowContent
+				{...baseProps}
+				message={mcpRequestMessage}
+				lastModifiedMessage={mcpRequestMessage}
+				modifiedMessages={modifiedMessages}
+			/>,
+		)
+
+		expect(screen.queryByTestId("progress-indicator")).not.toBeInTheDocument()
+	})
+
+	it("should not show spinner when API request is cancelled", () => {
+		const cancelledMessage: ClineMessage = {
+			...baseMessage,
+			text: JSON.stringify({ cost: undefined, cancelReason: "user_cancelled" }),
+		}
+		const modifiedMessages: ClineMessage[] = [cancelledMessage]
+
+		render(
+			<ChatRowContent
+				{...baseProps}
+				message={cancelledMessage}
+				lastModifiedMessage={cancelledMessage}
+				modifiedMessages={modifiedMessages}
+			/>,
+		)
+
+		expect(screen.queryByTestId("progress-indicator")).not.toBeInTheDocument()
+	})
+})