mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
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
This commit is contained in:
parent
6884871ffc
commit
d158325b1f
3 changed files with 40 additions and 16 deletions
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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],
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue