fix: prevent notification sound on attempt_completion with queued messages (#8540)

Co-authored-by: Roo Code <[email protected]>

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

@@ -380,7 +380,8 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 							break
 						case "completion_result":
 							// extension waiting for feedback. but we can just present a new task button
-							if (!isPartial) {
+							// Only play celebration sound if there are no queued messages
+							if (!isPartial && messageQueue.length === 0) {
 								playSound("celebration")
 							}
 							setSendingDisabled(isPartial)

webview-ui/src/components/chat/__tests__/ChatView.notification-sound.spec.tsx

@@ -0,0 +1,522 @@
+// npx vitest run src/components/chat/__tests__/ChatView.notification-sound.spec.tsx
+
+import React from "react"
+import { render, waitFor } from "@/utils/test-utils"
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query"
+
+import { ExtensionStateContextProvider } from "@src/context/ExtensionStateContext"
+
+import ChatView, { ChatViewProps } from "../ChatView"
+
+// Define minimal types needed for testing
+interface ClineMessage {
+	type: "say" | "ask"
+	say?: string
+	ask?: string
+	ts: number
+	text?: string
+	partial?: boolean
+}
+
+interface QueuedMessage {
+	id: string
+	text: string
+	images?: string[]
+}
+
+interface ExtensionState {
+	version: string
+	clineMessages: ClineMessage[]
+	taskHistory: any[]
+	shouldShowAnnouncement: boolean
+	messageQueue?: QueuedMessage[]
+	[key: string]: any
+}
+
+// Mock vscode API
+vi.mock("@src/utils/vscode", () => ({
+	vscode: {
+		postMessage: vi.fn(),
+	},
+}))
+
+// Mock use-sound hook
+const mockPlayFunction = vi.fn()
+vi.mock("use-sound", () => ({
+	default: vi.fn().mockImplementation(() => {
+		return [mockPlayFunction]
+	}),
+}))
+
+// Mock components that use ESM dependencies
+vi.mock("../BrowserSessionRow", () => ({
+	default: function MockBrowserSessionRow({ messages }: { messages: ClineMessage[] }) {
+		return <div data-testid="browser-session">{JSON.stringify(messages)}</div>
+	},
+}))
+
+vi.mock("../ChatRow", () => ({
+	default: function MockChatRow({ message }: { message: ClineMessage }) {
+		return <div data-testid="chat-row">{JSON.stringify(message)}</div>
+	},
+}))
+
+vi.mock("../AutoApproveMenu", () => ({
+	default: () => null,
+}))
+
+// Mock VersionIndicator
+vi.mock("../../common/VersionIndicator", () => ({
+	default: vi.fn(() => null),
+}))
+
+vi.mock("../Announcement", () => ({
+	default: function MockAnnouncement({ hideAnnouncement }: { hideAnnouncement: () => void }) {
+		// eslint-disable-next-line @typescript-eslint/no-require-imports
+		const React = require("react")
+		return React.createElement(
+			"div",
+			{ "data-testid": "announcement-modal" },
+			React.createElement("div", null, "What's New"),
+			React.createElement("button", { onClick: hideAnnouncement }, "Close"),
+		)
+	},
+}))
+
+// Mock DismissibleUpsell component
+vi.mock("@/components/common/DismissibleUpsell", () => ({
+	default: function MockDismissibleUpsell({ children }: { children: React.ReactNode }) {
+		return <div data-testid="dismissible-upsell">{children}</div>
+	},
+}))
+
+// Mock QueuedMessages component
+vi.mock("../QueuedMessages", () => ({
+	QueuedMessages: function MockQueuedMessages({
+		queue = [],
+		onRemove,
+	}: {
+		queue?: Array<{ id: string; text: string; images?: string[] }>
+		onRemove?: (index: number) => void
+		onUpdate?: (index: number, newText: string) => void
+	}) {
+		if (!queue || queue.length === 0) {
+			return null
+		}
+		return (
+			<div data-testid="queued-messages">
+				{queue.map((msg, index) => (
+					<div key={msg.id}>
+						<span>{msg.text}</span>
+						<button aria-label="Remove message" onClick={() => onRemove?.(index)}>
+							Remove
+						</button>
+					</div>
+				))}
+			</div>
+		)
+	},
+}))
+
+// Mock RooTips component
+vi.mock("@src/components/welcome/RooTips", () => ({
+	default: function MockRooTips() {
+		return <div data-testid="roo-tips">Tips content</div>
+	},
+}))
+
+// Mock RooHero component
+vi.mock("@src/components/welcome/RooHero", () => ({
+	default: function MockRooHero() {
+		return <div data-testid="roo-hero">Hero content</div>
+	},
+}))
+
+// Mock TelemetryBanner component
+vi.mock("../common/TelemetryBanner", () => ({
+	default: function MockTelemetryBanner() {
+		return null // Don't render anything to avoid interference
+	},
+}))
+
+// Mock i18n
+vi.mock("react-i18next", () => ({
+	useTranslation: () => ({
+		t: (key: string, options?: any) => {
+			if (key === "chat:versionIndicator.ariaLabel" && options?.version) {
+				return `Version ${options.version}`
+			}
+			return key
+		},
+	}),
+	initReactI18next: {
+		type: "3rdParty",
+		init: () => {},
+	},
+	Trans: ({ i18nKey, children }: { i18nKey: string; children?: React.ReactNode }) => {
+		return <>{children || i18nKey}</>
+	},
+}))
+
+interface ChatTextAreaProps {
+	onSend: (value: string) => void
+	inputValue?: string
+	sendingDisabled?: boolean
+	placeholderText?: string
+	selectedImages?: string[]
+	shouldDisableImages?: boolean
+}
+
+const mockInputRef = React.createRef<HTMLInputElement>()
+const mockFocus = vi.fn()
+
+vi.mock("../ChatTextArea", () => {
+	// eslint-disable-next-line @typescript-eslint/no-require-imports
+	const mockReact = require("react")
+
+	const ChatTextAreaComponent = mockReact.forwardRef(function MockChatTextArea(
+		props: ChatTextAreaProps,
+		ref: React.ForwardedRef<{ focus: () => void }>,
+	) {
+		// Use useImperativeHandle to expose the mock focus method
+		mockReact.useImperativeHandle(ref, () => ({
+			focus: mockFocus,
+		}))
+
+		return (
+			<div data-testid="chat-textarea">
+				<input
+					ref={mockInputRef}
+					type="text"
+					onChange={(e) => {
+						// With message queueing, onSend is always called (it handles queueing internally)
+						props.onSend(e.target.value)
+					}}
+					data-sending-disabled={props.sendingDisabled}
+				/>
+			</div>
+		)
+	})
+
+	return {
+		default: ChatTextAreaComponent,
+		ChatTextArea: ChatTextAreaComponent, // Export as named export too
+	}
+})
+
+// Mock VSCode components
+vi.mock("@vscode/webview-ui-toolkit/react", () => ({
+	VSCodeButton: function MockVSCodeButton({
+		children,
+		onClick,
+		appearance,
+	}: {
+		children: React.ReactNode
+		onClick?: () => void
+		appearance?: string
+	}) {
+		return (
+			<button onClick={onClick} data-appearance={appearance}>
+				{children}
+			</button>
+		)
+	},
+	VSCodeTextField: function MockVSCodeTextField({
+		value,
+		onInput,
+		placeholder,
+	}: {
+		value?: string
+		onInput?: (e: { target: { value: string } }) => void
+		placeholder?: string
+	}) {
+		return (
+			<input
+				type="text"
+				value={value}
+				onChange={(e) => onInput?.({ target: { value: e.target.value } })}
+				placeholder={placeholder}
+			/>
+		)
+	},
+	VSCodeLink: function MockVSCodeLink({ children, href }: { children: React.ReactNode; href?: string }) {
+		return <a href={href}>{children}</a>
+	},
+}))
+
+// Mock window.postMessage to trigger state hydration
+const mockPostMessage = (state: Partial<ExtensionState>) => {
+	window.postMessage(
+		{
+			type: "state",
+			state: {
+				version: "1.0.0",
+				clineMessages: [],
+				taskHistory: [],
+				shouldShowAnnouncement: false,
+				cloudIsAuthenticated: false,
+				telemetrySetting: "enabled",
+				messageQueue: [],
+				...state,
+			},
+		},
+		"*",
+	)
+}
+
+const defaultProps: ChatViewProps = {
+	isHidden: false,
+	showAnnouncement: false,
+	hideAnnouncement: () => {},
+}
+
+const queryClient = new QueryClient()
+
+const renderChatView = (props: Partial<ChatViewProps> = {}) => {
+	return render(
+		<ExtensionStateContextProvider>
+			<QueryClientProvider client={queryClient}>
+				<ChatView {...defaultProps} {...props} />
+			</QueryClientProvider>
+		</ExtensionStateContextProvider>,
+	)
+}
+
+describe("ChatView - Notification Sound with Queued Messages", () => {
+	beforeEach(() => vi.clearAllMocks())
+
+	it("does not play celebration sound when completion_result is received with queued messages", async () => {
+		renderChatView()
+
+		// First hydrate state with initial task
+		mockPostMessage({
+			soundEnabled: true, // Enable sound
+			messageQueue: [
+				{
+					id: "msg-1",
+					text: "This is a queued message",
+					images: [],
+				},
+			],
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: Date.now() - 2000,
+					text: "Initial task",
+				},
+			],
+		})
+
+		// Clear any initial calls
+		mockPlayFunction.mockClear()
+
+		// Add completion result with queued messages present
+		mockPostMessage({
+			soundEnabled: true, // Enable sound
+			messageQueue: [
+				{
+					id: "msg-1",
+					text: "This is a queued message",
+					images: [],
+				},
+			],
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: Date.now() - 2000,
+					text: "Initial task",
+				},
+				{
+					type: "ask",
+					ask: "completion_result",
+					ts: Date.now(),
+					text: "Task completed successfully",
+					partial: false, // Ensure it's not partial
+				},
+			],
+		})
+
+		// Wait a bit to ensure the effect would have run
+		await waitFor(
+			() => {
+				// Should NOT play sound when there are queued messages
+				expect(mockPlayFunction).not.toHaveBeenCalled()
+			},
+			{ timeout: 1000 },
+		)
+	})
+
+	it("plays celebration sound when completion_result is received with empty message queue", async () => {
+		renderChatView()
+
+		// First hydrate state with initial task
+		mockPostMessage({
+			soundEnabled: true, // Enable sound
+			messageQueue: [], // Empty queue
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: Date.now() - 2000,
+					text: "Initial task",
+				},
+			],
+		})
+
+		// Clear any initial calls
+		mockPlayFunction.mockClear()
+
+		// Add completion result with empty message queue
+		mockPostMessage({
+			soundEnabled: true, // Enable sound
+			messageQueue: [], // Empty queue
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: Date.now() - 2000,
+					text: "Initial task",
+				},
+				{
+					type: "ask",
+					ask: "completion_result",
+					ts: Date.now(),
+					text: "Task completed successfully",
+					partial: false, // Ensure it's not partial
+				},
+			],
+		})
+
+		// Wait for sound to be played
+		await waitFor(() => {
+			// Should play sound when message queue is empty
+			expect(mockPlayFunction).toHaveBeenCalled()
+		})
+	})
+
+	it("does not play celebration sound when completion_result is received with multiple queued messages", async () => {
+		renderChatView()
+
+		// First hydrate state with initial task
+		mockPostMessage({
+			soundEnabled: true, // Enable sound
+			messageQueue: [
+				{
+					id: "msg-1",
+					text: "First queued message",
+					images: [],
+				},
+				{
+					id: "msg-2",
+					text: "Second queued message",
+					images: [],
+				},
+			],
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: Date.now() - 2000,
+					text: "Initial task",
+				},
+			],
+		})
+
+		// Clear any initial calls
+		mockPlayFunction.mockClear()
+
+		// Add completion result with multiple queued messages
+		mockPostMessage({
+			soundEnabled: true, // Enable sound
+			messageQueue: [
+				{
+					id: "msg-1",
+					text: "First queued message",
+					images: [],
+				},
+				{
+					id: "msg-2",
+					text: "Second queued message",
+					images: [],
+				},
+			],
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: Date.now() - 2000,
+					text: "Initial task",
+				},
+				{
+					type: "ask",
+					ask: "completion_result",
+					ts: Date.now(),
+					text: "Task completed successfully",
+					partial: false, // Ensure it's not partial
+				},
+			],
+		})
+
+		// Wait a bit to ensure the effect would have run
+		await waitFor(
+			() => {
+				// Should NOT play sound when there are multiple queued messages
+				expect(mockPlayFunction).not.toHaveBeenCalled()
+			},
+			{ timeout: 1000 },
+		)
+	})
+
+	it("does not play celebration sound when completion_result is partial", async () => {
+		renderChatView()
+
+		// First hydrate state with initial task
+		mockPostMessage({
+			soundEnabled: true, // Enable sound
+			messageQueue: [], // Empty queue
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: Date.now() - 2000,
+					text: "Initial task",
+				},
+			],
+		})
+
+		// Clear any initial calls
+		mockPlayFunction.mockClear()
+
+		// Add partial completion result
+		mockPostMessage({
+			soundEnabled: true, // Enable sound
+			messageQueue: [], // Empty queue
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: Date.now() - 2000,
+					text: "Initial task",
+				},
+				{
+					type: "ask",
+					ask: "completion_result",
+					ts: Date.now(),
+					text: "Task completed successfully",
+					partial: true, // Partial message
+				},
+			],
+		})
+
+		// Wait a bit to ensure the effect would have run
+		await waitFor(
+			() => {
+				// Should NOT play sound when message is partial
+				expect(mockPlayFunction).not.toHaveBeenCalled()
+			},
+			{ timeout: 1000 },
+		)
+	})
+})