Implement array comparison utility and update global state conditionally in extension activation bug 2625

src/__tests__/activate.test.ts

@@ -0,0 +1,57 @@
+import * as vscode from "vscode"
+import { activate } from "../extension"
+import { areArraysEqual } from "../core/settings/utils"
+
+jest.mock("vscode")
+jest.mock("../core/settings/utils")
+
+describe("Extension Activation", () => {
+	let mockContext: vscode.ExtensionContext
+
+	beforeEach(() => {
+		jest.clearAllMocks()
+
+		mockContext = {
+			subscriptions: [],
+			globalState: {
+				get: jest.fn(),
+				update: jest.fn(),
+			},
+		} as unknown as vscode.ExtensionContext
+
+		// Mock vscode.workspace.getConfiguration
+		const mockGetConfiguration = jest.fn().mockReturnValue({
+			get: jest.fn().mockReturnValue(["npm test", "npm install", "tsc", "git log", "git diff", "git show"]),
+		})
+		;(vscode.workspace.getConfiguration as jest.Mock) = mockGetConfiguration
+	})
+
+	it("does not update globalState when current commands match defaults", async () => {
+		const currentCommands = ["npm test", "npm install", "tsc", "git log", "git diff", "git show"]
+		;(mockContext.globalState.get as jest.Mock).mockReturnValue(currentCommands)
+		;(areArraysEqual as jest.Mock).mockReturnValue(true)
+
+		await activate(mockContext)
+
+		expect(mockContext.globalState.update).not.toHaveBeenCalled()
+	})
+
+	it("updates globalState when current commands differ from defaults", async () => {
+		const currentCommands = ["custom-command"]
+		;(mockContext.globalState.get as jest.Mock).mockReturnValue(currentCommands)
+		;(areArraysEqual as jest.Mock).mockReturnValue(false)
+
+		await activate(mockContext)
+
+		expect(mockContext.globalState.update).toHaveBeenCalledWith("allowedCommands", expect.any(Array))
+	})
+
+	it("handles undefined current commands", async () => {
+		;(mockContext.globalState.get as jest.Mock).mockReturnValue(undefined)
+		;(areArraysEqual as jest.Mock).mockReturnValue(false)
+
+		await activate(mockContext)
+
+		expect(mockContext.globalState.update).toHaveBeenCalledWith("allowedCommands", expect.any(Array))
+	})
+})

src/activate/__tests__/CodeActionProvider.test.ts

@@ -26,6 +26,12 @@ jest.mock("vscode", () => ({
 		Information: 2,
 		Hint: 3,
 	},
+	languages: {
+		getDiagnostics: jest.fn().mockReturnValue([]),
+	},
+	window: {
+		activeTextEditor: undefined,
+	},
 }))
 
 jest.mock("../../integrations/editor/EditorUtils", () => ({

src/core/settings/__tests__/utils.test.ts

@@ -0,0 +1,35 @@
+import { areArraysEqual } from "../utils"
+
+describe("areArraysEqual", () => {
+	it("returns true for identical arrays", () => {
+		const arr1 = ["npm test", "git log"]
+		const arr2 = ["npm test", "git log"]
+		expect(areArraysEqual(arr1, arr2)).toBe(true)
+	})
+
+	it("returns true for arrays with same elements in different order", () => {
+		const arr1 = ["npm test", "git log"]
+		const arr2 = ["git log", "npm test"]
+		expect(areArraysEqual(arr1, arr2)).toBe(true)
+	})
+
+	it("returns false for arrays with different elements", () => {
+		const arr1 = ["npm test", "git log"]
+		const arr2 = ["npm test", "git diff"]
+		expect(areArraysEqual(arr1, arr2)).toBe(false)
+	})
+
+	it("returns true for empty arrays", () => {
+		expect(areArraysEqual([], [])).toBe(true)
+	})
+
+	it("returns true for null/undefined inputs", () => {
+		expect(areArraysEqual(null, null)).toBe(true)
+		expect(areArraysEqual(undefined, undefined)).toBe(true)
+	})
+
+	it("returns false when one input is null/undefined", () => {
+		expect(areArraysEqual(null, [])).toBe(false)
+		expect(areArraysEqual([], undefined)).toBe(false)
+	})
+})

src/core/settings/utils.ts

@@ -0,0 +1,18 @@
+/**
+ * Compares two arrays of strings for equality regardless of order.
+ * Handles null/undefined inputs and empty arrays gracefully.
+ */
+export function areArraysEqual(arr1: string[] | undefined | null, arr2: string[] | undefined | null): boolean {
+	// Handle null/undefined cases
+	if (!arr1 && !arr2) return true
+	if (!arr1 || !arr2) return false
+
+	// Handle empty arrays
+	if (arr1.length === 0 && arr2.length === 0) return true
+
+	// Sort and compare
+	const sorted1 = [...arr1].sort()
+	const sorted2 = [...arr2].sort()
+
+	return sorted1.length === sorted2.length && sorted1.every((value, index) => value === sorted2[index])
+}

src/extension.ts

@@ -23,6 +23,7 @@ import { telemetryService } from "./services/telemetry/TelemetryService"
 import { API } from "./exports/api"
 import { migrateSettings } from "./utils/migrateSettings"
 import { formatLanguage } from "./shared/language"
+import { areArraysEqual } from "./core/settings/utils"
 
 import {
 	handleUri,
@@ -66,10 +67,11 @@ export async function activate(context: vscode.ExtensionContext) {
 
 	// Get default commands from configuration.
 	const defaultCommands = vscode.workspace.getConfiguration("roo-cline").get<string[]>("allowedCommands") || []
+	const currentCommands = context.globalState.get<string[]>("allowedCommands")
 
-	// Initialize global state if not already set.
-	if (!context.globalState.get("allowedCommands")) {
-		context.globalState.update("allowedCommands", defaultCommands)
+	// Only update if current commands differ from defaults
+	if (!areArraysEqual(currentCommands, defaultCommands)) {
+		await context.globalState.update("allowedCommands", defaultCommands)
 	}
 
 	const contextProxy = await ContextProxy.getInstance(context)

webview-ui/src/components/settings/AutoApproveSettings.tsx

@@ -11,6 +11,9 @@ import { SectionHeader } from "./SectionHeader"
 import { Section } from "./Section"
 import { AutoApproveToggle } from "./AutoApproveToggle"
 
+// Default allowed commands from package.json
+const DEFAULT_ALLOWED_COMMANDS = ["npm test", "npm install", "tsc", "git log", "git diff", "git show"]
+
 type AutoApproveSettingsProps = HTMLAttributes<HTMLDivElement> & {
 	alwaysAllowReadOnly?: boolean
 	alwaysAllowReadOnlyOutsideWorkspace?: boolean
@@ -67,8 +70,22 @@ export const AutoApproveSettings = ({
 
 		if (commandInput && !currentCommands.includes(commandInput)) {
 			const newCommands = [...currentCommands, commandInput]
-			setCachedStateField("allowedCommands", newCommands)
+
+			// Only update if the new commands list differs from defaults
+			if (newCommands.sort().join(",") !== DEFAULT_ALLOWED_COMMANDS.sort().join(",")) {
+				setCachedStateField("allowedCommands", newCommands)
+				vscode.postMessage({ type: "allowedCommands", commands: newCommands })
+			}
 			setCommandInput("")
+		}
+	}
+
+	const handleRemoveCommand = (index: number) => {
+		const newCommands = (allowedCommands ?? []).filter((_, i) => i !== index)
+
+		// Only update if the new commands list differs from defaults
+		if (newCommands.sort().join(",") !== DEFAULT_ALLOWED_COMMANDS.sort().join(",")) {
+			setCachedStateField("allowedCommands", newCommands)
 			vscode.postMessage({ type: "allowedCommands", commands: newCommands })
 		}
 	}
@@ -227,11 +244,7 @@ export const AutoApproveSettings = ({
 									key={index}
 									variant="secondary"
 									data-testid={`remove-command-${index}`}
-									onClick={() => {
-										const newCommands = (allowedCommands ?? []).filter((_, i) => i !== index)
-										setCachedStateField("allowedCommands", newCommands)
-										vscode.postMessage({ type: "allowedCommands", commands: newCommands })
-									}}>
+									onClick={() => handleRemoveCommand(index)}>
 									<div className="flex flex-row items-center gap-1">
 										<div>{cmd}</div>
 										<X className="text-foreground scale-75" />

webview-ui/src/components/settings/__tests__/AutoApproveSettings.test.tsx

@@ -0,0 +1,105 @@
+import { render, screen, fireEvent } from "@testing-library/react"
+import { AutoApproveSettings } from "../AutoApproveSettings"
+import { vscode } from "@/utils/vscode"
+
+jest.mock("@/utils/vscode", () => ({
+	vscode: {
+		postMessage: jest.fn(),
+	},
+}))
+
+describe("AutoApproveSettings", () => {
+	const mockSetCachedStateField = jest.fn()
+
+	beforeEach(() => {
+		jest.clearAllMocks()
+	})
+
+	it("does not update settings when adding default command", () => {
+		render(
+			<AutoApproveSettings
+				allowedCommands={["npm test"]}
+				setCachedStateField={mockSetCachedStateField}
+				writeDelayMs={1000}
+				requestDelaySeconds={5}
+				alwaysAllowExecute={true}
+			/>,
+		)
+
+		const input = screen.getByTestId("command-input")
+		fireEvent.change(input, { target: { value: "git log" } })
+
+		const addButton = screen.getByTestId("add-command-button")
+		fireEvent.click(addButton)
+
+		// Should not call setCachedStateField or postMessage for default commands
+		expect(mockSetCachedStateField).not.toHaveBeenCalled()
+		expect(vscode.postMessage).not.toHaveBeenCalled()
+	})
+
+	it("updates settings when adding non-default command", () => {
+		render(
+			<AutoApproveSettings
+				allowedCommands={[]}
+				setCachedStateField={mockSetCachedStateField}
+				writeDelayMs={1000}
+				requestDelaySeconds={5}
+				alwaysAllowExecute={true}
+			/>,
+		)
+
+		const input = screen.getByTestId("command-input")
+		fireEvent.change(input, { target: { value: "custom-command" } })
+
+		const addButton = screen.getByTestId("add-command-button")
+		fireEvent.click(addButton)
+
+		expect(mockSetCachedStateField).toHaveBeenCalledWith("allowedCommands", ["custom-command"])
+		expect(vscode.postMessage).toHaveBeenCalledWith({
+			type: "allowedCommands",
+			commands: ["custom-command"],
+		})
+	})
+
+	it("does not update settings when removing command results in default list", () => {
+		render(
+			<AutoApproveSettings
+				allowedCommands={["npm test", "git log", "custom-command"]}
+				setCachedStateField={mockSetCachedStateField}
+				writeDelayMs={1000}
+				requestDelaySeconds={5}
+				alwaysAllowExecute={true}
+			/>,
+		)
+
+		// Remove custom command
+		const removeButton = screen.getByTestId("remove-command-2")
+		fireEvent.click(removeButton)
+
+		// Should not call setCachedStateField or postMessage when result matches defaults
+		expect(mockSetCachedStateField).not.toHaveBeenCalled()
+		expect(vscode.postMessage).not.toHaveBeenCalled()
+	})
+
+	it("updates settings when removing command results in non-default list", () => {
+		render(
+			<AutoApproveSettings
+				allowedCommands={["npm test", "custom-command"]}
+				setCachedStateField={mockSetCachedStateField}
+				writeDelayMs={1000}
+				requestDelaySeconds={5}
+				alwaysAllowExecute={true}
+			/>,
+		)
+
+		// Remove custom command
+		const removeButton = screen.getByTestId("remove-command-1")
+		fireEvent.click(removeButton)
+
+		expect(mockSetCachedStateField).toHaveBeenCalledWith("allowedCommands", ["npm test"])
+		expect(vscode.postMessage).toHaveBeenCalledWith({
+			type: "allowedCommands",
+			commands: ["npm test"],
+		})
+	})
+})