From b18721b18362b202719a261d0c91bbf57b04af93 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sat, 13 Sep 2025 03:20:19 +0000 Subject: [PATCH] fix: prevent unnecessary context condensing when toggling settings - Fixed dirty state tracking in SettingsView to properly detect when settings are reverted to original values - Modified all setter functions to check entire state against original extension state - Only send settings updates to backend when values actually differ from saved state - Added comprehensive tests for dirty state management - This prevents unnecessary context condensing triggers during active tasks Fixes issue where Save button remained enabled after toggling settings back to original values, which could trigger unwanted context condensing even when no actual changes were made. --- src/core/webview/webviewMessageHandler.ts | 39 +- .../src/components/settings/SettingsView.tsx | 578 ++++++++++++++---- .../settings/__tests__/SettingsView.spec.tsx | 179 +++++- 3 files changed, 660 insertions(+), 136 deletions(-) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 551810625c..c063e31dd5 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -60,6 +60,9 @@ const ALLOWED_VSCODE_SETTINGS = new Set(["terminal.integrated.inheritEnv"]) import { MarketplaceManager, MarketplaceItemType } from "../../services/marketplace" import { setPendingTodoList } from "../tools/updateTodoListTool" +// Cache for condense-related settings to prevent unnecessary re-evaluation +const condenseSettingsCache = new Map() + export const webviewMessageHandler = async ( provider: ClineProvider, message: WebviewMessage, @@ -70,6 +73,21 @@ export const webviewMessageHandler = async ( const updateGlobalState = async (key: K, value: GlobalState[K]) => await provider.contextProxy.setValue(key, value) + // Helper function to check if a condense-related setting has actually changed + const hasCondenseSettingChanged = (key: string, value: any): boolean => { + const cachedValue = condenseSettingsCache.get(key) + if (cachedValue === undefined) { + // First time setting this value + condenseSettingsCache.set(key, value) + return true + } + const changed = JSON.stringify(cachedValue) !== JSON.stringify(value) + if (changed) { + condenseSettingsCache.set(key, value) + } + return changed + } + const getCurrentCwd = () => { return provider.getCurrentTask()?.cwd || provider.cwd } @@ -581,12 +599,18 @@ export const webviewMessageHandler = async ( provider.getCurrentTask()?.handleWebviewAskResponse(message.askResponse!, message.text, message.images) break case "autoCondenseContext": - await updateGlobalState("autoCondenseContext", message.bool) - await provider.postStateToWebview() + // Only update if the value has actually changed to prevent unnecessary context re-evaluation + if (hasCondenseSettingChanged("autoCondenseContext", message.bool)) { + await updateGlobalState("autoCondenseContext", message.bool) + await provider.postStateToWebview() + } break case "autoCondenseContextPercent": - await updateGlobalState("autoCondenseContextPercent", message.value) - await provider.postStateToWebview() + // Only update if the value has actually changed to prevent unnecessary context re-evaluation + if (hasCondenseSettingChanged("autoCondenseContextPercent", message.value)) { + await updateGlobalState("autoCondenseContextPercent", message.value) + await provider.postStateToWebview() + } break case "terminalOperation": if (message.terminalOperation) { @@ -1650,8 +1674,11 @@ export const webviewMessageHandler = async ( await provider.postStateToWebview() break case "profileThresholds": - await updateGlobalState("profileThresholds", message.values) - await provider.postStateToWebview() + // Only update if the value has actually changed to prevent unnecessary context re-evaluation + if (hasCondenseSettingChanged("profileThresholds", message.values)) { + await updateGlobalState("profileThresholds", message.values) + await provider.postStateToWebview() + } break case "autoApprovalEnabled": await updateGlobalState("autoApprovalEnabled", message.bool ?? false) diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 632873308e..37cd4890ee 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -212,16 +212,39 @@ const SettingsView = forwardRef(({ onDone, t } }, [settingsImportedAt, extensionState]) - const setCachedStateField: SetCachedStateField = useCallback((field, value) => { - setCachedState((prevState) => { - if (prevState[field] === value) { - return prevState - } + const setCachedStateField: SetCachedStateField = useCallback( + (field, value) => { + setCachedState((prevState) => { + if (prevState[field] === value) { + return prevState + } - setChangeDetected(true) - return { ...prevState, [field]: value } - }) - }, []) + const newState = { ...prevState, [field]: value } + + // Check if any field in the new state differs from the original extension state + // This properly handles toggling back to original values + let hasAnyChanges = false + for (const key in newState) { + if (key === "apiConfiguration") continue // Handle separately + const cachedValue = newState[key as keyof ExtensionStateContextType] + const originalValue = extensionState[key as keyof ExtensionStateContextType] + if (hasChanged(cachedValue, originalValue)) { + hasAnyChanges = true + break + } + } + + // Also check apiConfiguration if it exists + if (!hasAnyChanges && newState.apiConfiguration && extensionState.apiConfiguration) { + hasAnyChanges = hasChanged(newState.apiConfiguration, extensionState.apiConfiguration) + } + + setChangeDetected(hasAnyChanges) + return newState + }) + }, + [extensionState], + ) const setApiConfigurationField = useCallback( (field: K, value: ProviderSettings[K], isUserAction: boolean = true) => { @@ -236,139 +259,444 @@ const SettingsView = forwardRef(({ onDone, t // This prevents the dirty state when the component initializes and auto-syncs values const isInitialSync = !isUserAction && previousValue === undefined && value !== undefined - if (!isInitialSync) { - setChangeDetected(true) + const newState = { + ...prevState, + apiConfiguration: { ...prevState.apiConfiguration, [field]: value }, } - return { ...prevState, apiConfiguration: { ...prevState.apiConfiguration, [field]: value } } + + if (!isInitialSync) { + // Check if any field in the new state differs from the original extension state + let hasAnyChanges = false + + // Check non-apiConfiguration fields + for (const key in newState) { + if (key === "apiConfiguration") continue + const cachedValue = newState[key as keyof ExtensionStateContextType] + const originalValue = extensionState[key as keyof ExtensionStateContextType] + if (hasChanged(cachedValue, originalValue)) { + hasAnyChanges = true + break + } + } + + // Check apiConfiguration + if (!hasAnyChanges && newState.apiConfiguration && extensionState.apiConfiguration) { + hasAnyChanges = hasChanged(newState.apiConfiguration, extensionState.apiConfiguration) + } + + setChangeDetected(hasAnyChanges) + } + + return newState }) }, - [], + [extensionState], ) - const setExperimentEnabled: SetExperimentEnabled = useCallback((id: ExperimentId, enabled: boolean) => { - setCachedState((prevState) => { - if (prevState.experiments?.[id] === enabled) { - return prevState - } + const setExperimentEnabled: SetExperimentEnabled = useCallback( + (id: ExperimentId, enabled: boolean) => { + setCachedState((prevState) => { + if (prevState.experiments?.[id] === enabled) { + return prevState + } - setChangeDetected(true) - return { ...prevState, experiments: { ...prevState.experiments, [id]: enabled } } - }) - }, []) + const newState = { ...prevState, experiments: { ...prevState.experiments, [id]: enabled } } - const setTelemetrySetting = useCallback((setting: TelemetrySetting) => { - setCachedState((prevState) => { - if (prevState.telemetrySetting === setting) { - return prevState - } + // Check if experiments differ from original + const hasExperimentChanges = hasChanged(newState.experiments, extensionState.experiments) - setChangeDetected(true) - return { ...prevState, telemetrySetting: setting } - }) - }, []) + // Check other fields for changes + let hasOtherChanges = false + for (const key in newState) { + if (key === "experiments" || key === "apiConfiguration") continue + const cachedValue = newState[key as keyof ExtensionStateContextType] + const originalValue = extensionState[key as keyof ExtensionStateContextType] + if (hasChanged(cachedValue, originalValue)) { + hasOtherChanges = true + break + } + } - const setOpenRouterImageApiKey = useCallback((apiKey: string) => { - setCachedState((prevState) => { - setChangeDetected(true) - return { ...prevState, openRouterImageApiKey: apiKey } - }) - }, []) + // Check apiConfiguration + if (!hasOtherChanges && newState.apiConfiguration && extensionState.apiConfiguration) { + hasOtherChanges = hasChanged(newState.apiConfiguration, extensionState.apiConfiguration) + } - const setImageGenerationSelectedModel = useCallback((model: string) => { - setCachedState((prevState) => { - setChangeDetected(true) - return { ...prevState, openRouterImageGenerationSelectedModel: model } - }) - }, []) + setChangeDetected(hasExperimentChanges || hasOtherChanges) + return newState + }) + }, + [extensionState], + ) - const setCustomSupportPromptsField = useCallback((prompts: Record) => { - setCachedState((prevState) => { - if (JSON.stringify(prevState.customSupportPrompts) === JSON.stringify(prompts)) { - return prevState - } + const setTelemetrySetting = useCallback( + (setting: TelemetrySetting) => { + setCachedState((prevState) => { + if (prevState.telemetrySetting === setting) { + return prevState + } - setChangeDetected(true) - return { ...prevState, customSupportPrompts: prompts } - }) - }, []) + const newState = { ...prevState, telemetrySetting: setting } + + // Check if telemetry setting differs from original + const hasTelemetryChange = newState.telemetrySetting !== extensionState.telemetrySetting + + // Check other fields for changes + let hasOtherChanges = false + for (const key in newState) { + if (key === "telemetrySetting" || key === "apiConfiguration") continue + const cachedValue = newState[key as keyof ExtensionStateContextType] + const originalValue = extensionState[key as keyof ExtensionStateContextType] + if (hasChanged(cachedValue, originalValue)) { + hasOtherChanges = true + break + } + } + + // Check apiConfiguration + if (!hasOtherChanges && newState.apiConfiguration && extensionState.apiConfiguration) { + hasOtherChanges = hasChanged(newState.apiConfiguration, extensionState.apiConfiguration) + } + + setChangeDetected(hasTelemetryChange || hasOtherChanges) + return newState + }) + }, + [extensionState], + ) + + const setOpenRouterImageApiKey = useCallback( + (apiKey: string) => { + setCachedState((prevState) => { + if (prevState.openRouterImageApiKey === apiKey) { + return prevState + } + + const newState = { ...prevState, openRouterImageApiKey: apiKey } + + // Check if any field differs from original + let hasAnyChanges = false + for (const key in newState) { + if (key === "apiConfiguration") continue + const cachedValue = newState[key as keyof ExtensionStateContextType] + const originalValue = extensionState[key as keyof ExtensionStateContextType] + if (hasChanged(cachedValue, originalValue)) { + hasAnyChanges = true + break + } + } + + // Check apiConfiguration + if (!hasAnyChanges && newState.apiConfiguration && extensionState.apiConfiguration) { + hasAnyChanges = hasChanged(newState.apiConfiguration, extensionState.apiConfiguration) + } + + setChangeDetected(hasAnyChanges) + return newState + }) + }, + [extensionState], + ) + + const setImageGenerationSelectedModel = useCallback( + (model: string) => { + setCachedState((prevState) => { + if (prevState.openRouterImageGenerationSelectedModel === model) { + return prevState + } + + const newState = { ...prevState, openRouterImageGenerationSelectedModel: model } + + // Check if any field differs from original + let hasAnyChanges = false + for (const key in newState) { + if (key === "apiConfiguration") continue + const cachedValue = newState[key as keyof ExtensionStateContextType] + const originalValue = extensionState[key as keyof ExtensionStateContextType] + if (hasChanged(cachedValue, originalValue)) { + hasAnyChanges = true + break + } + } + + // Check apiConfiguration + if (!hasAnyChanges && newState.apiConfiguration && extensionState.apiConfiguration) { + hasAnyChanges = hasChanged(newState.apiConfiguration, extensionState.apiConfiguration) + } + + setChangeDetected(hasAnyChanges) + return newState + }) + }, + [extensionState], + ) + + const setCustomSupportPromptsField = useCallback( + (prompts: Record) => { + setCachedState((prevState) => { + if (JSON.stringify(prevState.customSupportPrompts) === JSON.stringify(prompts)) { + return prevState + } + + const newState = { ...prevState, customSupportPrompts: prompts } + + // Check if any field differs from original + let hasAnyChanges = false + for (const key in newState) { + if (key === "apiConfiguration") continue + const cachedValue = newState[key as keyof ExtensionStateContextType] + const originalValue = extensionState[key as keyof ExtensionStateContextType] + if (hasChanged(cachedValue, originalValue)) { + hasAnyChanges = true + break + } + } + + // Check apiConfiguration + if (!hasAnyChanges && newState.apiConfiguration && extensionState.apiConfiguration) { + hasAnyChanges = hasChanged(newState.apiConfiguration, extensionState.apiConfiguration) + } + + setChangeDetected(hasAnyChanges) + return newState + }) + }, + [extensionState], + ) const isSettingValid = !errorMessage + // Helper function to check if a value has changed + const hasChanged = (cachedValue: any, originalValue: any): boolean => { + // Handle objects and arrays with deep comparison + if (typeof cachedValue === "object" && cachedValue !== null) { + return JSON.stringify(cachedValue) !== JSON.stringify(originalValue) + } + return cachedValue !== originalValue + } + const handleSubmit = () => { if (isSettingValid) { - vscode.postMessage({ type: "language", text: language }) - vscode.postMessage({ type: "alwaysAllowReadOnly", bool: alwaysAllowReadOnly }) - vscode.postMessage({ - type: "alwaysAllowReadOnlyOutsideWorkspace", - bool: alwaysAllowReadOnlyOutsideWorkspace, - }) - vscode.postMessage({ type: "alwaysAllowWrite", bool: alwaysAllowWrite }) - vscode.postMessage({ type: "alwaysAllowWriteOutsideWorkspace", bool: alwaysAllowWriteOutsideWorkspace }) - vscode.postMessage({ type: "alwaysAllowWriteProtected", bool: alwaysAllowWriteProtected }) - vscode.postMessage({ type: "alwaysAllowExecute", bool: alwaysAllowExecute }) - vscode.postMessage({ type: "alwaysAllowBrowser", bool: alwaysAllowBrowser }) - vscode.postMessage({ type: "alwaysAllowMcp", bool: alwaysAllowMcp }) - vscode.postMessage({ type: "allowedCommands", commands: allowedCommands ?? [] }) - vscode.postMessage({ type: "deniedCommands", commands: deniedCommands ?? [] }) - vscode.postMessage({ type: "allowedMaxRequests", value: allowedMaxRequests ?? undefined }) - vscode.postMessage({ type: "allowedMaxCost", value: allowedMaxCost ?? undefined }) - vscode.postMessage({ type: "autoCondenseContext", bool: autoCondenseContext }) - vscode.postMessage({ type: "autoCondenseContextPercent", value: autoCondenseContextPercent }) - vscode.postMessage({ type: "browserToolEnabled", bool: browserToolEnabled }) - vscode.postMessage({ type: "soundEnabled", bool: soundEnabled }) - vscode.postMessage({ type: "ttsEnabled", bool: ttsEnabled }) - vscode.postMessage({ type: "ttsSpeed", value: ttsSpeed }) - vscode.postMessage({ type: "soundVolume", value: soundVolume }) - vscode.postMessage({ type: "diffEnabled", bool: diffEnabled }) - vscode.postMessage({ type: "enableCheckpoints", bool: enableCheckpoints }) - vscode.postMessage({ type: "browserViewportSize", text: browserViewportSize }) - vscode.postMessage({ type: "remoteBrowserHost", text: remoteBrowserHost }) - vscode.postMessage({ type: "remoteBrowserEnabled", bool: remoteBrowserEnabled }) - vscode.postMessage({ type: "fuzzyMatchThreshold", value: fuzzyMatchThreshold ?? 1.0 }) - vscode.postMessage({ type: "writeDelayMs", value: writeDelayMs }) - vscode.postMessage({ type: "screenshotQuality", value: screenshotQuality ?? 75 }) - vscode.postMessage({ type: "terminalOutputLineLimit", value: terminalOutputLineLimit ?? 500 }) - vscode.postMessage({ type: "terminalOutputCharacterLimit", value: terminalOutputCharacterLimit ?? 50000 }) - vscode.postMessage({ type: "terminalShellIntegrationTimeout", value: terminalShellIntegrationTimeout }) - vscode.postMessage({ type: "terminalShellIntegrationDisabled", bool: terminalShellIntegrationDisabled }) - vscode.postMessage({ type: "terminalCommandDelay", value: terminalCommandDelay }) - vscode.postMessage({ type: "terminalPowershellCounter", bool: terminalPowershellCounter }) - vscode.postMessage({ type: "terminalZshClearEolMark", bool: terminalZshClearEolMark }) - vscode.postMessage({ type: "terminalZshOhMy", bool: terminalZshOhMy }) - vscode.postMessage({ type: "terminalZshP10k", bool: terminalZshP10k }) - vscode.postMessage({ type: "terminalZdotdir", bool: terminalZdotdir }) - vscode.postMessage({ type: "terminalCompressProgressBar", bool: terminalCompressProgressBar }) - vscode.postMessage({ type: "mcpEnabled", bool: mcpEnabled }) - vscode.postMessage({ type: "alwaysApproveResubmit", bool: alwaysApproveResubmit }) - vscode.postMessage({ type: "requestDelaySeconds", value: requestDelaySeconds }) - vscode.postMessage({ type: "maxOpenTabsContext", value: maxOpenTabsContext }) - vscode.postMessage({ type: "maxWorkspaceFiles", value: maxWorkspaceFiles ?? 200 }) - vscode.postMessage({ type: "showRooIgnoredFiles", bool: showRooIgnoredFiles }) - vscode.postMessage({ type: "maxReadFileLine", value: maxReadFileLine ?? -1 }) - vscode.postMessage({ type: "maxImageFileSize", value: maxImageFileSize ?? 5 }) - vscode.postMessage({ type: "maxTotalImageSize", value: maxTotalImageSize ?? 20 }) - vscode.postMessage({ type: "maxConcurrentFileReads", value: cachedState.maxConcurrentFileReads ?? 5 }) - vscode.postMessage({ type: "includeDiagnosticMessages", bool: includeDiagnosticMessages }) - vscode.postMessage({ type: "maxDiagnosticMessages", value: maxDiagnosticMessages ?? 50 }) - vscode.postMessage({ type: "currentApiConfigName", text: currentApiConfigName }) - vscode.postMessage({ type: "updateExperimental", values: experiments }) - vscode.postMessage({ type: "alwaysAllowModeSwitch", bool: alwaysAllowModeSwitch }) - vscode.postMessage({ type: "alwaysAllowSubtasks", bool: alwaysAllowSubtasks }) - vscode.postMessage({ type: "alwaysAllowFollowupQuestions", bool: alwaysAllowFollowupQuestions }) - vscode.postMessage({ type: "alwaysAllowUpdateTodoList", bool: alwaysAllowUpdateTodoList }) - vscode.postMessage({ type: "followupAutoApproveTimeoutMs", value: followupAutoApproveTimeoutMs }) - vscode.postMessage({ type: "condensingApiConfigId", text: condensingApiConfigId || "" }) - vscode.postMessage({ type: "updateCondensingPrompt", text: customCondensingPrompt || "" }) - vscode.postMessage({ type: "updateSupportPrompt", values: customSupportPrompts || {} }) - vscode.postMessage({ type: "includeTaskHistoryInEnhance", bool: includeTaskHistoryInEnhance ?? true }) - vscode.postMessage({ type: "upsertApiConfiguration", text: currentApiConfigName, apiConfiguration }) - vscode.postMessage({ type: "telemetrySetting", text: telemetrySetting }) - vscode.postMessage({ type: "profileThresholds", values: profileThresholds }) - vscode.postMessage({ type: "openRouterImageApiKey", text: openRouterImageApiKey }) - vscode.postMessage({ - type: "openRouterImageGenerationSelectedModel", - text: openRouterImageGenerationSelectedModel, - }) + // Only send messages for settings that have actually changed + if (hasChanged(language, extensionState.language)) { + vscode.postMessage({ type: "language", text: language }) + } + if (hasChanged(alwaysAllowReadOnly, extensionState.alwaysAllowReadOnly)) { + vscode.postMessage({ type: "alwaysAllowReadOnly", bool: alwaysAllowReadOnly }) + } + if (hasChanged(alwaysAllowReadOnlyOutsideWorkspace, extensionState.alwaysAllowReadOnlyOutsideWorkspace)) { + vscode.postMessage({ + type: "alwaysAllowReadOnlyOutsideWorkspace", + bool: alwaysAllowReadOnlyOutsideWorkspace, + }) + } + if (hasChanged(alwaysAllowWrite, extensionState.alwaysAllowWrite)) { + vscode.postMessage({ type: "alwaysAllowWrite", bool: alwaysAllowWrite }) + } + if (hasChanged(alwaysAllowWriteOutsideWorkspace, extensionState.alwaysAllowWriteOutsideWorkspace)) { + vscode.postMessage({ type: "alwaysAllowWriteOutsideWorkspace", bool: alwaysAllowWriteOutsideWorkspace }) + } + if (hasChanged(alwaysAllowWriteProtected, extensionState.alwaysAllowWriteProtected)) { + vscode.postMessage({ type: "alwaysAllowWriteProtected", bool: alwaysAllowWriteProtected }) + } + if (hasChanged(alwaysAllowExecute, extensionState.alwaysAllowExecute)) { + vscode.postMessage({ type: "alwaysAllowExecute", bool: alwaysAllowExecute }) + } + if (hasChanged(alwaysAllowBrowser, extensionState.alwaysAllowBrowser)) { + vscode.postMessage({ type: "alwaysAllowBrowser", bool: alwaysAllowBrowser }) + } + if (hasChanged(alwaysAllowMcp, extensionState.alwaysAllowMcp)) { + vscode.postMessage({ type: "alwaysAllowMcp", bool: alwaysAllowMcp }) + } + if (hasChanged(allowedCommands, extensionState.allowedCommands)) { + vscode.postMessage({ type: "allowedCommands", commands: allowedCommands ?? [] }) + } + if (hasChanged(deniedCommands, extensionState.deniedCommands)) { + vscode.postMessage({ type: "deniedCommands", commands: deniedCommands ?? [] }) + } + if (hasChanged(allowedMaxRequests, extensionState.allowedMaxRequests)) { + vscode.postMessage({ type: "allowedMaxRequests", value: allowedMaxRequests ?? undefined }) + } + if (hasChanged(allowedMaxCost, extensionState.allowedMaxCost)) { + vscode.postMessage({ type: "allowedMaxCost", value: allowedMaxCost ?? undefined }) + } + + // CRITICAL: Only send condense-related settings if they actually changed + // This prevents unnecessary context condensing triggers during active tasks + if (hasChanged(autoCondenseContext, extensionState.autoCondenseContext)) { + vscode.postMessage({ type: "autoCondenseContext", bool: autoCondenseContext }) + } + if (hasChanged(autoCondenseContextPercent, extensionState.autoCondenseContextPercent)) { + vscode.postMessage({ type: "autoCondenseContextPercent", value: autoCondenseContextPercent }) + } + if (hasChanged(profileThresholds, extensionState.profileThresholds)) { + vscode.postMessage({ type: "profileThresholds", values: profileThresholds }) + } + + if (hasChanged(browserToolEnabled, extensionState.browserToolEnabled)) { + vscode.postMessage({ type: "browserToolEnabled", bool: browserToolEnabled }) + } + if (hasChanged(soundEnabled, extensionState.soundEnabled)) { + vscode.postMessage({ type: "soundEnabled", bool: soundEnabled }) + } + if (hasChanged(ttsEnabled, extensionState.ttsEnabled)) { + vscode.postMessage({ type: "ttsEnabled", bool: ttsEnabled }) + } + if (hasChanged(ttsSpeed, extensionState.ttsSpeed)) { + vscode.postMessage({ type: "ttsSpeed", value: ttsSpeed }) + } + if (hasChanged(soundVolume, extensionState.soundVolume)) { + vscode.postMessage({ type: "soundVolume", value: soundVolume }) + } + if (hasChanged(diffEnabled, extensionState.diffEnabled)) { + vscode.postMessage({ type: "diffEnabled", bool: diffEnabled }) + } + if (hasChanged(enableCheckpoints, extensionState.enableCheckpoints)) { + vscode.postMessage({ type: "enableCheckpoints", bool: enableCheckpoints }) + } + if (hasChanged(browserViewportSize, extensionState.browserViewportSize)) { + vscode.postMessage({ type: "browserViewportSize", text: browserViewportSize }) + } + if (hasChanged(remoteBrowserHost, extensionState.remoteBrowserHost)) { + vscode.postMessage({ type: "remoteBrowserHost", text: remoteBrowserHost }) + } + if (hasChanged(remoteBrowserEnabled, extensionState.remoteBrowserEnabled)) { + vscode.postMessage({ type: "remoteBrowserEnabled", bool: remoteBrowserEnabled }) + } + if (hasChanged(fuzzyMatchThreshold, extensionState.fuzzyMatchThreshold)) { + vscode.postMessage({ type: "fuzzyMatchThreshold", value: fuzzyMatchThreshold ?? 1.0 }) + } + if (hasChanged(writeDelayMs, extensionState.writeDelayMs)) { + vscode.postMessage({ type: "writeDelayMs", value: writeDelayMs }) + } + if (hasChanged(screenshotQuality, extensionState.screenshotQuality)) { + vscode.postMessage({ type: "screenshotQuality", value: screenshotQuality ?? 75 }) + } + if (hasChanged(terminalOutputLineLimit, extensionState.terminalOutputLineLimit)) { + vscode.postMessage({ type: "terminalOutputLineLimit", value: terminalOutputLineLimit ?? 500 }) + } + if (hasChanged(terminalOutputCharacterLimit, extensionState.terminalOutputCharacterLimit)) { + vscode.postMessage({ + type: "terminalOutputCharacterLimit", + value: terminalOutputCharacterLimit ?? 50000, + }) + } + if (hasChanged(terminalShellIntegrationTimeout, extensionState.terminalShellIntegrationTimeout)) { + vscode.postMessage({ type: "terminalShellIntegrationTimeout", value: terminalShellIntegrationTimeout }) + } + if (hasChanged(terminalShellIntegrationDisabled, extensionState.terminalShellIntegrationDisabled)) { + vscode.postMessage({ type: "terminalShellIntegrationDisabled", bool: terminalShellIntegrationDisabled }) + } + if (hasChanged(terminalCommandDelay, extensionState.terminalCommandDelay)) { + vscode.postMessage({ type: "terminalCommandDelay", value: terminalCommandDelay }) + } + if (hasChanged(terminalPowershellCounter, extensionState.terminalPowershellCounter)) { + vscode.postMessage({ type: "terminalPowershellCounter", bool: terminalPowershellCounter }) + } + if (hasChanged(terminalZshClearEolMark, extensionState.terminalZshClearEolMark)) { + vscode.postMessage({ type: "terminalZshClearEolMark", bool: terminalZshClearEolMark }) + } + if (hasChanged(terminalZshOhMy, extensionState.terminalZshOhMy)) { + vscode.postMessage({ type: "terminalZshOhMy", bool: terminalZshOhMy }) + } + if (hasChanged(terminalZshP10k, extensionState.terminalZshP10k)) { + vscode.postMessage({ type: "terminalZshP10k", bool: terminalZshP10k }) + } + if (hasChanged(terminalZdotdir, extensionState.terminalZdotdir)) { + vscode.postMessage({ type: "terminalZdotdir", bool: terminalZdotdir }) + } + if (hasChanged(terminalCompressProgressBar, extensionState.terminalCompressProgressBar)) { + vscode.postMessage({ type: "terminalCompressProgressBar", bool: terminalCompressProgressBar }) + } + if (hasChanged(mcpEnabled, extensionState.mcpEnabled)) { + vscode.postMessage({ type: "mcpEnabled", bool: mcpEnabled }) + } + if (hasChanged(alwaysApproveResubmit, extensionState.alwaysApproveResubmit)) { + vscode.postMessage({ type: "alwaysApproveResubmit", bool: alwaysApproveResubmit }) + } + if (hasChanged(requestDelaySeconds, extensionState.requestDelaySeconds)) { + vscode.postMessage({ type: "requestDelaySeconds", value: requestDelaySeconds }) + } + if (hasChanged(maxOpenTabsContext, extensionState.maxOpenTabsContext)) { + vscode.postMessage({ type: "maxOpenTabsContext", value: maxOpenTabsContext }) + } + if (hasChanged(maxWorkspaceFiles, extensionState.maxWorkspaceFiles)) { + vscode.postMessage({ type: "maxWorkspaceFiles", value: maxWorkspaceFiles ?? 200 }) + } + if (hasChanged(showRooIgnoredFiles, extensionState.showRooIgnoredFiles)) { + vscode.postMessage({ type: "showRooIgnoredFiles", bool: showRooIgnoredFiles }) + } + if (hasChanged(maxReadFileLine, extensionState.maxReadFileLine)) { + vscode.postMessage({ type: "maxReadFileLine", value: maxReadFileLine ?? -1 }) + } + if (hasChanged(maxImageFileSize, extensionState.maxImageFileSize)) { + vscode.postMessage({ type: "maxImageFileSize", value: maxImageFileSize ?? 5 }) + } + if (hasChanged(maxTotalImageSize, extensionState.maxTotalImageSize)) { + vscode.postMessage({ type: "maxTotalImageSize", value: maxTotalImageSize ?? 20 }) + } + if (hasChanged(maxConcurrentFileReads, extensionState.maxConcurrentFileReads)) { + vscode.postMessage({ type: "maxConcurrentFileReads", value: cachedState.maxConcurrentFileReads ?? 5 }) + } + if (hasChanged(includeDiagnosticMessages, extensionState.includeDiagnosticMessages)) { + vscode.postMessage({ type: "includeDiagnosticMessages", bool: includeDiagnosticMessages }) + } + if (hasChanged(maxDiagnosticMessages, extensionState.maxDiagnosticMessages)) { + vscode.postMessage({ type: "maxDiagnosticMessages", value: maxDiagnosticMessages ?? 50 }) + } + if (hasChanged(currentApiConfigName, extensionState.currentApiConfigName)) { + vscode.postMessage({ type: "currentApiConfigName", text: currentApiConfigName }) + } + if (hasChanged(experiments, extensionState.experiments)) { + vscode.postMessage({ type: "updateExperimental", values: experiments }) + } + if (hasChanged(alwaysAllowModeSwitch, extensionState.alwaysAllowModeSwitch)) { + vscode.postMessage({ type: "alwaysAllowModeSwitch", bool: alwaysAllowModeSwitch }) + } + if (hasChanged(alwaysAllowSubtasks, extensionState.alwaysAllowSubtasks)) { + vscode.postMessage({ type: "alwaysAllowSubtasks", bool: alwaysAllowSubtasks }) + } + if (hasChanged(alwaysAllowFollowupQuestions, extensionState.alwaysAllowFollowupQuestions)) { + vscode.postMessage({ type: "alwaysAllowFollowupQuestions", bool: alwaysAllowFollowupQuestions }) + } + if (hasChanged(alwaysAllowUpdateTodoList, extensionState.alwaysAllowUpdateTodoList)) { + vscode.postMessage({ type: "alwaysAllowUpdateTodoList", bool: alwaysAllowUpdateTodoList }) + } + if (hasChanged(followupAutoApproveTimeoutMs, extensionState.followupAutoApproveTimeoutMs)) { + vscode.postMessage({ type: "followupAutoApproveTimeoutMs", value: followupAutoApproveTimeoutMs }) + } + if (hasChanged(condensingApiConfigId, extensionState.condensingApiConfigId)) { + vscode.postMessage({ type: "condensingApiConfigId", text: condensingApiConfigId || "" }) + } + if (hasChanged(customCondensingPrompt, extensionState.customCondensingPrompt)) { + vscode.postMessage({ type: "updateCondensingPrompt", text: customCondensingPrompt || "" }) + } + if (hasChanged(customSupportPrompts, extensionState.customSupportPrompts)) { + vscode.postMessage({ type: "updateSupportPrompt", values: customSupportPrompts || {} }) + } + if (hasChanged(includeTaskHistoryInEnhance, extensionState.includeTaskHistoryInEnhance)) { + vscode.postMessage({ type: "includeTaskHistoryInEnhance", bool: includeTaskHistoryInEnhance ?? true }) + } + if (hasChanged(apiConfiguration, extensionState.apiConfiguration)) { + vscode.postMessage({ type: "upsertApiConfiguration", text: currentApiConfigName, apiConfiguration }) + } + if (hasChanged(telemetrySetting, extensionState.telemetrySetting)) { + vscode.postMessage({ type: "telemetrySetting", text: telemetrySetting }) + } + if (hasChanged(openRouterImageApiKey, extensionState.openRouterImageApiKey)) { + vscode.postMessage({ type: "openRouterImageApiKey", text: openRouterImageApiKey }) + } + if ( + hasChanged( + openRouterImageGenerationSelectedModel, + extensionState.openRouterImageGenerationSelectedModel, + ) + ) { + vscode.postMessage({ + type: "openRouterImageGenerationSelectedModel", + text: openRouterImageGenerationSelectedModel, + }) + } setChangeDetected(false) } } diff --git a/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx b/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx index 694ff174a7..438e8a9aa9 100644 --- a/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx @@ -18,17 +18,18 @@ vi.mock("../ApiConfigManager", () => ({ })) vi.mock("@vscode/webview-ui-toolkit/react", () => ({ - VSCodeButton: ({ children, onClick, appearance, "data-testid": dataTestId }: any) => + VSCodeButton: ({ children, onClick, appearance, disabled, "data-testid": dataTestId }: any) => appearance === "icon" ? ( ) : ( - ), @@ -135,8 +136,13 @@ vi.mock("@/components/ui", () => ({ data-testid={dataTestId} /> ), - Button: ({ children, onClick, variant, className, "data-testid": dataTestId }: any) => ( - ), @@ -637,3 +643,166 @@ describe("SettingsView - Duplicate Commands", () => { ) }) }) + +describe("SettingsView - Dirty State Management", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("disables Save button when toggling settings back to original values", () => { + // Render once and get the activateTab helper + const { activateTab } = renderSettingsView() + + // Activate the notifications tab + activateTab("notifications") + + // Initially, Save button should be disabled (no changes) + const saveButton = screen.getByTestId("save-button") + expect(saveButton).toHaveAttribute("disabled") + + // Enable sound (make a change) + const soundCheckbox = screen.getByTestId("sound-enabled-checkbox") + fireEvent.click(soundCheckbox) + + // Save button should now be enabled + expect(saveButton).not.toHaveAttribute("disabled") + + // Toggle sound back off (revert to original) + fireEvent.click(soundCheckbox) + + // Save button should be disabled again (no net changes) + expect(saveButton).toHaveAttribute("disabled") + }) + + it("keeps Save button enabled when there are actual changes from original state", () => { + // Render once and get the activateTab helper + const { activateTab } = renderSettingsView() + + // Activate the notifications tab + activateTab("notifications") + + const saveButton = screen.getByTestId("save-button") + const soundCheckbox = screen.getByTestId("sound-enabled-checkbox") + const ttsCheckbox = screen.getByTestId("tts-enabled-checkbox") + + // Make multiple changes + fireEvent.click(soundCheckbox) // Enable sound + fireEvent.click(ttsCheckbox) // Enable TTS + + // Save button should be enabled + expect(saveButton).not.toHaveAttribute("disabled") + + // Toggle one back but keep the other changed + fireEvent.click(soundCheckbox) // Disable sound (back to original) + + // Save button should still be enabled (TTS is still changed) + expect(saveButton).not.toHaveAttribute("disabled") + + // Toggle the other back too + fireEvent.click(ttsCheckbox) // Disable TTS (back to original) + + // Now Save button should be disabled (everything back to original) + expect(saveButton).toHaveAttribute("disabled") + }) + + it("correctly tracks dirty state for context management settings", () => { + // Render once and get the activateTab helper + const { activateTab } = renderSettingsView() + + // Activate the context tab + activateTab("context") + + const saveButton = screen.getByTestId("save-button") + + // Initially disabled + expect(saveButton).toHaveAttribute("disabled") + + // Enable auto-condense + const autoCondenseCheckbox = screen.getByTestId("auto-condense-checkbox") + fireEvent.click(autoCondenseCheckbox) + + // Save button should be enabled + expect(saveButton).not.toHaveAttribute("disabled") + + // Toggle back + fireEvent.click(autoCondenseCheckbox) + + // Save button should be disabled again + expect(saveButton).toHaveAttribute("disabled") + }) + + it("does not send settings to backend when Save is clicked with no changes", () => { + // Clear any previous mock calls + vi.clearAllMocks() + + // Render once and get the activateTab helper + const { activateTab } = renderSettingsView() + + // Activate the notifications tab + activateTab("notifications") + + // Toggle a setting on and off + const soundCheckbox = screen.getByTestId("sound-enabled-checkbox") + fireEvent.click(soundCheckbox) // Enable + fireEvent.click(soundCheckbox) // Disable (back to original) + + // Save button should be disabled + const saveButton = screen.getByTestId("save-button") + expect(saveButton).toBeDisabled() + + // Try to click Save (should be disabled and not trigger any action) + fireEvent.click(saveButton) + + // Check that no settings-related messages were sent + // (There may be initial setup messages like requestRouterModels) + const settingsMessages = (vscode.postMessage as any).mock.calls.filter((call: any) => { + const type = call[0]?.type + return ( + type === "soundEnabled" || + type === "ttsEnabled" || + type === "autoCondenseContext" || + type === "autoCondenseContextPercent" + ) + }) + expect(settingsMessages).toHaveLength(0) + }) + + it("correctly handles complex state changes across multiple settings", () => { + // Render once and get the activateTab helper + const { activateTab } = renderSettingsView() + + // Start with autoApprove tab + activateTab("autoApprove") + + const saveButton = screen.getByTestId("save-button") + + // Enable always allow execute + const executeCheckbox = screen.getByTestId("always-allow-execute-toggle") + fireEvent.click(executeCheckbox) + + // Save button should be enabled + expect(saveButton).not.toHaveAttribute("disabled") + + // Add a command + const input = screen.getByTestId("command-input") + fireEvent.change(input, { target: { value: "npm test" } }) + const addButton = screen.getByTestId("add-command-button") + fireEvent.click(addButton) + + // Still enabled (more changes) + expect(saveButton).not.toHaveAttribute("disabled") + + // Remove the command + const removeButton = screen.getByTestId("remove-command-0") + fireEvent.click(removeButton) + + // Still enabled (execute checkbox is still changed) + expect(saveButton).not.toHaveAttribute("disabled") + + // Toggle execute checkbox back + fireEvent.click(executeCheckbox) + + // Now should be disabled (everything back to original) + expect(saveButton).toHaveAttribute("disabled") + }) +})