From d158325b1f8a0edc477576ca51d02b67dcf0cfbe Mon Sep 17 00:00:00 2001
From: Roo Code <roomote@roocode.com>
Date: Thu, 7 Aug 2025 13:54:00 +0000
Subject: [PATCH] fix: address review comments for mode switch race condition

- Extract magic numbers as named constants (MODE_SWITCH_DEBOUNCE_MS, MODE_SWITCH_RESET_DELAY_MS)
- Align timing between frontend and backend (both use 150ms)
- Add error handling to frontend switchToMode function
- Improve consistency in delay management between components
---
 src/core/webview/ClineProvider.ts             | 13 +++++-
 webview-ui/src/components/chat/ChatView.tsx   | 40 ++++++++++++-------
 .../__tests__/ChatView.keyboard-fix.spec.tsx  |  3 ++
 3 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts
index cebd97c581..a210ba9e9d 100644
--- a/src/core/webview/ClineProvider.ts
+++ b/src/core/webview/ClineProvider.ts
@@ -114,6 +114,9 @@ export class ClineProvider
 	private mdmService?: MdmService
 	private isModeSwitching = false
 
+	// Constants for mode switching delays
+	private static readonly MODE_SWITCH_TIMEOUT_MS = 150
+
 	public isViewLaunched = false
 	public settingsImportedAt?: number
 	public readonly latestAnnouncementId = "jul-29-2025-3-25-0" // Update for v3.25.0 announcement
@@ -967,9 +970,15 @@ export class ClineProvider
 
 		try {
 			await this.performModeSwitch(newMode)
+		} catch (error) {
+			// Log the error and re-throw to maintain existing behavior
+			this.log(`Error during mode switch: ${error instanceof Error ? error.message : String(error)}`)
+			throw error
 		} finally {
-			// Always reset the flag, even if an error occurs
-			this.isModeSwitching = false
+			// Reset the flag after a delay to ensure synchronization with frontend
+			setTimeout(() => {
+				this.isModeSwitching = false
+			}, ClineProvider.MODE_SWITCH_TIMEOUT_MS)
 		}
 	}
 
diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx
index cc374ae7c1..dfa86cfca8 100644
--- a/webview-ui/src/components/chat/ChatView.tsx
+++ b/webview-ui/src/components/chat/ChatView.tsx
@@ -1,6 +1,10 @@
 import React, { forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState } from "react"
 import { useDeepCompareEffect, useEvent, useMount } from "react-use"
 import debounce from "debounce"
+
+// Constants for mode switching delays
+const MODE_SWITCH_DEBOUNCE_MS = 150
+const MODE_SWITCH_RESET_DELAY_MS = 150 // Aligned with debounce for consistency
 import { Virtuoso, type VirtuosoHandle } from "react-virtuoso"
 import removeMd from "remove-markdown"
 import { VSCodeButton } from "@vscode/webview-ui-toolkit/react"
@@ -1462,22 +1466,30 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 				return
 			}
 
-			// Set flag to prevent concurrent switches
-			setIsModeSwitching(true)
+			try {
+				// Set flag to prevent concurrent switches
+				setIsModeSwitching(true)
 
-			// Update local state and notify extension to sync mode change
-			setMode(modeSlug)
+				// Update local state and notify extension to sync mode change
+				setMode(modeSlug)
 
-			// Send the mode switch message
-			vscode.postMessage({
-				type: "mode",
-				text: modeSlug,
-			})
+				// Send the mode switch message
+				vscode.postMessage({
+					type: "mode",
+					text: modeSlug,
+				})
 
-			// Reset the flag after a short delay to allow the mode switch to complete
-			setTimeout(() => {
+				// Reset the flag after a short delay to allow the mode switch to complete
+				setTimeout(() => {
+					setIsModeSwitching(false)
+				}, MODE_SWITCH_RESET_DELAY_MS)
+			} catch (error) {
+				// Reset the flag on error to allow retry
 				setIsModeSwitching(false)
-			}, 300)
+				console.error("Failed to switch mode:", error)
+				// Optionally show user-friendly error message
+				// You could add a toast notification here if available
+			}
 		},
 		[setMode, isModeSwitching],
 	)
@@ -1737,7 +1749,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 				const nextModeIndex = (currentModeIndex + 1) % allModes.length
 				// Update local state and notify extension to sync mode change
 				switchToMode(allModes[nextModeIndex].slug)
-			}, 150),
+			}, MODE_SWITCH_DEBOUNCE_MS),
 		[mode, customModes, switchToMode],
 	)
 
@@ -1750,7 +1762,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 				const previousModeIndex = (currentModeIndex - 1 + allModes.length) % allModes.length
 				// Update local state and notify extension to sync mode change
 				switchToMode(allModes[previousModeIndex].slug)
-			}, 150),
+			}, MODE_SWITCH_DEBOUNCE_MS),
 		[mode, customModes, switchToMode],
 	)
 
diff --git a/webview-ui/src/components/chat/__tests__/ChatView.keyboard-fix.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.keyboard-fix.spec.tsx
index f91603d2a8..6c3aeb0494 100644
--- a/webview-ui/src/components/chat/__tests__/ChatView.keyboard-fix.spec.tsx
+++ b/webview-ui/src/components/chat/__tests__/ChatView.keyboard-fix.spec.tsx
@@ -163,6 +163,7 @@ describe("ChatView - Keyboard Shortcut Fix for Dvorak", () => {
 		})
 
 		// Wait for event to be processed and debounce delay (150ms)
+		// Adding extra buffer to ensure debounce completes
 		await new Promise((resolve) => setTimeout(resolve, 200))
 
 		// Check if mode switch was triggered
@@ -245,6 +246,7 @@ describe("ChatView - Keyboard Shortcut Fix for Dvorak", () => {
 		})
 
 		// Wait for event to be processed and debounce delay (150ms)
+		// Adding extra buffer to ensure debounce completes
 		await new Promise((resolve) => setTimeout(resolve, 200))
 
 		// Check if mode switch was triggered
@@ -278,6 +280,7 @@ describe("ChatView - Keyboard Shortcut Fix for Dvorak", () => {
 		})
 
 		// Wait for event to be processed and debounce delay (150ms)
+		// Adding extra buffer to ensure debounce completes
 		await new Promise((resolve) => setTimeout(resolve, 200))
 
 		// Check if mode switch was triggered