Browse Source

ExtensionStateContext does not correctly merge state

cte 1 year ago
parent
commit
b0f1eb6d62

+ 5 - 0
.changeset/healthy-buckets-attack.md

@@ -0,0 +1,5 @@
+---
+"roo-cline": patch
+---
+
+ExtensionStateContext does not correctly merge state

+ 9 - 1
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": {

+ 2 - 0
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",

+ 7 - 7
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)

+ 47 - 2
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<ExperimentId, boolean>,
+			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,
+		})
+	})
+})