From b0f1eb6d62b47820d0fa6dab3061c8dd0f100b39 Mon Sep 17 00:00:00 2001 From: cte Date: Sun, 2 Mar 2025 15:17:15 -0800 Subject: [PATCH 1/3] ExtensionStateContext does not correctly merge state --- .changeset/healthy-buckets-attack.md | 5 ++ webview-ui/package-lock.json | 10 +++- webview-ui/package.json | 2 + .../src/context/ExtensionStateContext.tsx | 14 +++--- .../__tests__/ExtensionStateContext.test.tsx | 49 ++++++++++++++++++- 5 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 .changeset/healthy-buckets-attack.md diff --git a/.changeset/healthy-buckets-attack.md b/.changeset/healthy-buckets-attack.md new file mode 100644 index 0000000000..8961d7151e --- /dev/null +++ b/.changeset/healthy-buckets-attack.md @@ -0,0 +1,5 @@ +--- +"roo-cline": patch +--- + +ExtensionStateContext does not correctly merge state diff --git a/webview-ui/package-lock.json b/webview-ui/package-lock.json index b614ba387e..fdf6e6d116 100644 --- a/webview-ui/package-lock.json +++ b/webview-ui/package-lock.json @@ -27,6 +27,7 @@ "debounce": "^2.1.1", "fast-deep-equal": "^3.1.3", "fzf": "^0.5.2", + "lodash": "^4.17.21", "lucide-react": "^0.475.0", "mermaid": "^11.4.1", "react": "^18.3.1", @@ -54,6 +55,7 @@ "@testing-library/react": "^16.2.0", "@testing-library/user-event": "^14.6.1", "@types/jest": "^27.5.2", + "@types/lodash": "^4.17.16", "@types/node": "^18.0.0", "@types/react": "^18.3.18", "@types/react-dom": "^18.3.5", @@ -6930,6 +6932,13 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/lodash": { + "version": "4.17.16", + "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.17.16.tgz", + "integrity": "sha512-HX7Em5NYQAXKW+1T+FiuG27NGwzJfCX3s1GjOa7ujxZa52kjJLOr4FUxT+giF6Tgxv1e+/czV/iTtBw27WTU9g==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/mdast": { "version": "3.0.15", "resolved": "https://registry.npmjs.org/@types/mdast/-/mdast-3.0.15.tgz", @@ -15312,7 +15321,6 @@ "version": "4.17.21", "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==", - "dev": true, "license": "MIT" }, "node_modules/lodash-es": { diff --git a/webview-ui/package.json b/webview-ui/package.json index 12786d1060..0209d41129 100644 --- a/webview-ui/package.json +++ b/webview-ui/package.json @@ -34,6 +34,7 @@ "debounce": "^2.1.1", "fast-deep-equal": "^3.1.3", "fzf": "^0.5.2", + "lodash": "^4.17.21", "lucide-react": "^0.475.0", "mermaid": "^11.4.1", "react": "^18.3.1", @@ -61,6 +62,7 @@ "@testing-library/react": "^16.2.0", "@testing-library/user-event": "^14.6.1", "@types/jest": "^27.5.2", + "@types/lodash": "^4.17.16", "@types/node": "^18.0.0", "@types/react": "^18.3.18", "@types/react-dom": "^18.3.5", diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 3dfc87de75..23e70ee983 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -1,5 +1,7 @@ import React, { createContext, useCallback, useContext, useEffect, useState } from "react" import { useEvent } from "react-use" +import { merge } from "lodash" + import { ApiConfigMeta, ExtensionMessage, ExtensionState } from "../../../src/shared/ExtensionMessage" import { ApiConfiguration } from "../../../src/shared/api" import { vscode } from "../utils/vscode" @@ -123,13 +125,8 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode switch (message.type) { case "state": { const newState = message.state! - setState((prevState) => ({ - ...prevState, - ...newState, - })) - const config = newState.apiConfiguration - const hasKey = checkExistKey(config) - setShowWelcome(!hasKey) + setState((prevState) => mergeExtensionState(prevState, newState)) + setShowWelcome(!checkExistKey(newState.apiConfiguration)) setDidHydrateState(true) break } @@ -256,3 +253,6 @@ export const useExtensionState = () => { } return context } + +export const mergeExtensionState = (prevState: ExtensionState, newState: ExtensionState): ExtensionState => + merge(prevState, newState) diff --git a/webview-ui/src/context/__tests__/ExtensionStateContext.test.tsx b/webview-ui/src/context/__tests__/ExtensionStateContext.test.tsx index 22ecd2a837..aca1f26bcf 100644 --- a/webview-ui/src/context/__tests__/ExtensionStateContext.test.tsx +++ b/webview-ui/src/context/__tests__/ExtensionStateContext.test.tsx @@ -1,6 +1,11 @@ -import React from "react" +// npx jest webview-ui/src/context/__tests__/ExtensionStateContext.test.tsx + import { render, screen, act } from "@testing-library/react" -import { ExtensionStateContextProvider, useExtensionState } from "../ExtensionStateContext" + +import { ExtensionState } from "../../../../src/shared/ExtensionMessage" +import { ExtensionStateContextProvider, useExtensionState, mergeExtensionState } from "../ExtensionStateContext" +import { ExperimentId } from "../../../../src/shared/experiments" +import { ApiConfiguration } from "../../../../src/shared/api" // Test component that consumes the context const TestComponent = () => { @@ -63,3 +68,43 @@ describe("ExtensionStateContext", () => { consoleSpy.mockRestore() }) }) + +describe("mergeExtensionState", () => { + it("should correctly merge extension states", () => { + const baseState: ExtensionState = { + version: "", + mcpEnabled: false, + enableMcpServerCreation: false, + clineMessages: [], + taskHistory: [], + shouldShowAnnouncement: false, + enableCheckpoints: true, + preferredLanguage: "English", + writeDelayMs: 1000, + requestDelaySeconds: 5, + rateLimitSeconds: 0, + mode: "default", + experiments: {} as Record, + customModes: [], + maxOpenTabsContext: 20, + apiConfiguration: { providerId: "openrouter" } as ApiConfiguration, + } + + const prevState: ExtensionState = { + ...baseState, + apiConfiguration: { modelMaxTokens: 1234, modelMaxThinkingTokens: 123 }, + } + const newState: ExtensionState = { + ...baseState, + apiConfiguration: { modelMaxThinkingTokens: 456, modelTemperature: 0.3 }, + } + + const result = mergeExtensionState(prevState, newState) + + expect(result.apiConfiguration).toEqual({ + modelMaxTokens: 1234, + modelMaxThinkingTokens: 456, + modelTemperature: 0.3, + }) + }) +}) From 2bd310bc925a27eb2d354c605550198d471f7157 Mon Sep 17 00:00:00 2001 From: Chris Estreich Date: Sun, 2 Mar 2025 15:20:34 -0800 Subject: [PATCH 2/3] Update webview-ui/src/context/ExtensionStateContext.tsx Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com> --- webview-ui/src/context/ExtensionStateContext.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 23e70ee983..fda47ed80f 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -255,4 +255,4 @@ export const useExtensionState = () => { } export const mergeExtensionState = (prevState: ExtensionState, newState: ExtensionState): ExtensionState => - merge(prevState, newState) + merge({}, prevState, newState) From e201ee8fdc83cb8953e0c36e56992122102d003d Mon Sep 17 00:00:00 2001 From: cte Date: Sun, 2 Mar 2025 16:37:44 -0800 Subject: [PATCH 3/3] Selectively deep merge rather than deep merging everything --- webview-ui/package-lock.json | 10 +----- webview-ui/package.json | 2 -- .../src/context/ExtensionStateContext.tsx | 31 ++++++++++++++++--- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/webview-ui/package-lock.json b/webview-ui/package-lock.json index fdf6e6d116..b614ba387e 100644 --- a/webview-ui/package-lock.json +++ b/webview-ui/package-lock.json @@ -27,7 +27,6 @@ "debounce": "^2.1.1", "fast-deep-equal": "^3.1.3", "fzf": "^0.5.2", - "lodash": "^4.17.21", "lucide-react": "^0.475.0", "mermaid": "^11.4.1", "react": "^18.3.1", @@ -55,7 +54,6 @@ "@testing-library/react": "^16.2.0", "@testing-library/user-event": "^14.6.1", "@types/jest": "^27.5.2", - "@types/lodash": "^4.17.16", "@types/node": "^18.0.0", "@types/react": "^18.3.18", "@types/react-dom": "^18.3.5", @@ -6932,13 +6930,6 @@ "dev": true, "license": "MIT" }, - "node_modules/@types/lodash": { - "version": "4.17.16", - "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.17.16.tgz", - "integrity": "sha512-HX7Em5NYQAXKW+1T+FiuG27NGwzJfCX3s1GjOa7ujxZa52kjJLOr4FUxT+giF6Tgxv1e+/czV/iTtBw27WTU9g==", - "dev": true, - "license": "MIT" - }, "node_modules/@types/mdast": { "version": "3.0.15", "resolved": "https://registry.npmjs.org/@types/mdast/-/mdast-3.0.15.tgz", @@ -15321,6 +15312,7 @@ "version": "4.17.21", "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==", + "dev": true, "license": "MIT" }, "node_modules/lodash-es": { diff --git a/webview-ui/package.json b/webview-ui/package.json index 0209d41129..12786d1060 100644 --- a/webview-ui/package.json +++ b/webview-ui/package.json @@ -34,7 +34,6 @@ "debounce": "^2.1.1", "fast-deep-equal": "^3.1.3", "fzf": "^0.5.2", - "lodash": "^4.17.21", "lucide-react": "^0.475.0", "mermaid": "^11.4.1", "react": "^18.3.1", @@ -62,7 +61,6 @@ "@testing-library/react": "^16.2.0", "@testing-library/user-event": "^14.6.1", "@types/jest": "^27.5.2", - "@types/lodash": "^4.17.16", "@types/node": "^18.0.0", "@types/react": "^18.3.18", "@types/react-dom": "^18.3.5", diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index fda47ed80f..abf921dc2b 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -1,7 +1,5 @@ import React, { createContext, useCallback, useContext, useEffect, useState } from "react" import { useEvent } from "react-use" -import { merge } from "lodash" - import { ApiConfigMeta, ExtensionMessage, ExtensionState } from "../../../src/shared/ExtensionMessage" import { ApiConfiguration } from "../../../src/shared/api" import { vscode } from "../utils/vscode" @@ -71,6 +69,32 @@ export interface ExtensionStateContextType extends ExtensionState { export const ExtensionStateContext = createContext(undefined) +export const mergeExtensionState = (prevState: ExtensionState, newState: ExtensionState) => { + const { + apiConfiguration: prevApiConfiguration, + customModePrompts: prevCustomModePrompts, + customSupportPrompts: prevCustomSupportPrompts, + experiments: prevExperiments, + ...prevRest + } = prevState + + const { + apiConfiguration: newApiConfiguration, + customModePrompts: newCustomModePrompts, + customSupportPrompts: newCustomSupportPrompts, + experiments: newExperiments, + ...newRest + } = newState + + const apiConfiguration = { ...prevApiConfiguration, ...newApiConfiguration } + const customModePrompts = { ...prevCustomModePrompts, ...newCustomModePrompts } + const customSupportPrompts = { ...prevCustomSupportPrompts, ...newCustomSupportPrompts } + const experiments = { ...prevExperiments, ...newExperiments } + const rest = { ...prevRest, ...newRest } + + return { ...rest, apiConfiguration, customModePrompts, customSupportPrompts, experiments } +} + export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode }> = ({ children }) => { const [state, setState] = useState({ version: "", @@ -253,6 +277,3 @@ export const useExtensionState = () => { } return context } - -export const mergeExtensionState = (prevState: ExtensionState, newState: ExtensionState): ExtensionState => - merge({}, prevState, newState)