Skip to content

Commit 4d78171

Browse files
committed
Implement array comparison utility and update global state conditionally in extension activation
- Added `areArraysEqual` utility function to compare two arrays of strings for equality, handling null/undefined inputs. - Updated the `activate` function to only update the global state for allowed commands if the current commands differ from the defaults. - Introduced tests for the new utility function and modified tests for the extension activation to ensure correct behavior. - Enhanced `AutoApproveSettings` component to conditionally update settings based on the comparison with default commands.
1 parent 302dc2d commit 4d78171

File tree

7 files changed

+245
-9
lines changed

7 files changed

+245
-9
lines changed

src/__tests__/activate.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import * as vscode from "vscode"
2+
import { activate } from "../extension"
3+
import { areArraysEqual } from "../core/settings/utils"
4+
5+
jest.mock("vscode")
6+
jest.mock("../core/settings/utils")
7+
8+
describe("Extension Activation", () => {
9+
let mockContext: vscode.ExtensionContext
10+
11+
beforeEach(() => {
12+
jest.clearAllMocks()
13+
14+
mockContext = {
15+
subscriptions: [],
16+
globalState: {
17+
get: jest.fn(),
18+
update: jest.fn(),
19+
},
20+
} as unknown as vscode.ExtensionContext
21+
22+
// Mock vscode.workspace.getConfiguration
23+
const mockGetConfiguration = jest.fn().mockReturnValue({
24+
get: jest.fn().mockReturnValue(["npm test", "npm install", "tsc", "git log", "git diff", "git show"]),
25+
})
26+
;(vscode.workspace.getConfiguration as jest.Mock) = mockGetConfiguration
27+
})
28+
29+
it("does not update globalState when current commands match defaults", async () => {
30+
const currentCommands = ["npm test", "npm install", "tsc", "git log", "git diff", "git show"]
31+
;(mockContext.globalState.get as jest.Mock).mockReturnValue(currentCommands)
32+
;(areArraysEqual as jest.Mock).mockReturnValue(true)
33+
34+
await activate(mockContext)
35+
36+
expect(mockContext.globalState.update).not.toHaveBeenCalled()
37+
})
38+
39+
it("updates globalState when current commands differ from defaults", async () => {
40+
const currentCommands = ["custom-command"]
41+
;(mockContext.globalState.get as jest.Mock).mockReturnValue(currentCommands)
42+
;(areArraysEqual as jest.Mock).mockReturnValue(false)
43+
44+
await activate(mockContext)
45+
46+
expect(mockContext.globalState.update).toHaveBeenCalledWith("allowedCommands", expect.any(Array))
47+
})
48+
49+
it("handles undefined current commands", async () => {
50+
;(mockContext.globalState.get as jest.Mock).mockReturnValue(undefined)
51+
;(areArraysEqual as jest.Mock).mockReturnValue(false)
52+
53+
await activate(mockContext)
54+
55+
expect(mockContext.globalState.update).toHaveBeenCalledWith("allowedCommands", expect.any(Array))
56+
})
57+
})

src/activate/__tests__/CodeActionProvider.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ jest.mock("vscode", () => ({
2626
Information: 2,
2727
Hint: 3,
2828
},
29+
languages: {
30+
getDiagnostics: jest.fn().mockReturnValue([]),
31+
},
32+
window: {
33+
activeTextEditor: undefined,
34+
},
2935
}))
3036

3137
jest.mock("../../integrations/editor/EditorUtils", () => ({
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { areArraysEqual } from "../utils"
2+
3+
describe("areArraysEqual", () => {
4+
it("returns true for identical arrays", () => {
5+
const arr1 = ["npm test", "git log"]
6+
const arr2 = ["npm test", "git log"]
7+
expect(areArraysEqual(arr1, arr2)).toBe(true)
8+
})
9+
10+
it("returns true for arrays with same elements in different order", () => {
11+
const arr1 = ["npm test", "git log"]
12+
const arr2 = ["git log", "npm test"]
13+
expect(areArraysEqual(arr1, arr2)).toBe(true)
14+
})
15+
16+
it("returns false for arrays with different elements", () => {
17+
const arr1 = ["npm test", "git log"]
18+
const arr2 = ["npm test", "git diff"]
19+
expect(areArraysEqual(arr1, arr2)).toBe(false)
20+
})
21+
22+
it("returns true for empty arrays", () => {
23+
expect(areArraysEqual([], [])).toBe(true)
24+
})
25+
26+
it("returns true for null/undefined inputs", () => {
27+
expect(areArraysEqual(null, null)).toBe(true)
28+
expect(areArraysEqual(undefined, undefined)).toBe(true)
29+
})
30+
31+
it("returns false when one input is null/undefined", () => {
32+
expect(areArraysEqual(null, [])).toBe(false)
33+
expect(areArraysEqual([], undefined)).toBe(false)
34+
})
35+
})

src/core/settings/utils.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* Compares two arrays of strings for equality regardless of order.
3+
* Handles null/undefined inputs and empty arrays gracefully.
4+
*/
5+
export function areArraysEqual(arr1: string[] | undefined | null, arr2: string[] | undefined | null): boolean {
6+
// Handle null/undefined cases
7+
if (!arr1 && !arr2) return true
8+
if (!arr1 || !arr2) return false
9+
10+
// Handle empty arrays
11+
if (arr1.length === 0 && arr2.length === 0) return true
12+
13+
// Sort and compare
14+
const sorted1 = [...arr1].sort()
15+
const sorted2 = [...arr2].sort()
16+
17+
return sorted1.length === sorted2.length && sorted1.every((value, index) => value === sorted2[index])
18+
}

src/extension.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { telemetryService } from "./services/telemetry/TelemetryService"
2323
import { API } from "./exports/api"
2424
import { migrateSettings } from "./utils/migrateSettings"
2525
import { formatLanguage } from "./shared/language"
26+
import { areArraysEqual } from "./core/settings/utils"
2627

2728
import {
2829
handleUri,
@@ -66,10 +67,11 @@ export async function activate(context: vscode.ExtensionContext) {
6667

6768
// Get default commands from configuration.
6869
const defaultCommands = vscode.workspace.getConfiguration("roo-cline").get<string[]>("allowedCommands") || []
70+
const currentCommands = context.globalState.get<string[]>("allowedCommands")
6971

70-
// Initialize global state if not already set.
71-
if (!context.globalState.get("allowedCommands")) {
72-
context.globalState.update("allowedCommands", defaultCommands)
72+
// Only update if current commands differ from defaults
73+
if (!areArraysEqual(currentCommands, defaultCommands)) {
74+
await context.globalState.update("allowedCommands", defaultCommands)
7375
}
7476

7577
const contextProxy = await ContextProxy.getInstance(context)

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

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import { SectionHeader } from "./SectionHeader"
1111
import { Section } from "./Section"
1212
import { AutoApproveToggle } from "./AutoApproveToggle"
1313

14+
// Default allowed commands from package.json
15+
const DEFAULT_ALLOWED_COMMANDS = ["npm test", "npm install", "tsc", "git log", "git diff", "git show"]
16+
1417
type AutoApproveSettingsProps = HTMLAttributes<HTMLDivElement> & {
1518
alwaysAllowReadOnly?: boolean
1619
alwaysAllowReadOnlyOutsideWorkspace?: boolean
@@ -67,8 +70,22 @@ export const AutoApproveSettings = ({
6770

6871
if (commandInput && !currentCommands.includes(commandInput)) {
6972
const newCommands = [...currentCommands, commandInput]
70-
setCachedStateField("allowedCommands", newCommands)
73+
74+
// Only update if the new commands list differs from defaults
75+
if (newCommands.sort().join(",") !== DEFAULT_ALLOWED_COMMANDS.sort().join(",")) {
76+
setCachedStateField("allowedCommands", newCommands)
77+
vscode.postMessage({ type: "allowedCommands", commands: newCommands })
78+
}
7179
setCommandInput("")
80+
}
81+
}
82+
83+
const handleRemoveCommand = (index: number) => {
84+
const newCommands = (allowedCommands ?? []).filter((_, i) => i !== index)
85+
86+
// Only update if the new commands list differs from defaults
87+
if (newCommands.sort().join(",") !== DEFAULT_ALLOWED_COMMANDS.sort().join(",")) {
88+
setCachedStateField("allowedCommands", newCommands)
7289
vscode.postMessage({ type: "allowedCommands", commands: newCommands })
7390
}
7491
}
@@ -227,11 +244,7 @@ export const AutoApproveSettings = ({
227244
key={index}
228245
variant="secondary"
229246
data-testid={`remove-command-${index}`}
230-
onClick={() => {
231-
const newCommands = (allowedCommands ?? []).filter((_, i) => i !== index)
232-
setCachedStateField("allowedCommands", newCommands)
233-
vscode.postMessage({ type: "allowedCommands", commands: newCommands })
234-
}}>
247+
onClick={() => handleRemoveCommand(index)}>
235248
<div className="flex flex-row items-center gap-1">
236249
<div>{cmd}</div>
237250
<X className="text-foreground scale-75" />
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
import { render, screen, fireEvent } from "@testing-library/react"
2+
import { AutoApproveSettings } from "../AutoApproveSettings"
3+
import { vscode } from "@/utils/vscode"
4+
5+
jest.mock("@/utils/vscode", () => ({
6+
vscode: {
7+
postMessage: jest.fn(),
8+
},
9+
}))
10+
11+
describe("AutoApproveSettings", () => {
12+
const mockSetCachedStateField = jest.fn()
13+
14+
beforeEach(() => {
15+
jest.clearAllMocks()
16+
})
17+
18+
it("does not update settings when adding default command", () => {
19+
render(
20+
<AutoApproveSettings
21+
allowedCommands={["npm test"]}
22+
setCachedStateField={mockSetCachedStateField}
23+
writeDelayMs={1000}
24+
requestDelaySeconds={5}
25+
alwaysAllowExecute={true}
26+
/>,
27+
)
28+
29+
const input = screen.getByTestId("command-input")
30+
fireEvent.change(input, { target: { value: "git log" } })
31+
32+
const addButton = screen.getByTestId("add-command-button")
33+
fireEvent.click(addButton)
34+
35+
// Should not call setCachedStateField or postMessage for default commands
36+
expect(mockSetCachedStateField).not.toHaveBeenCalled()
37+
expect(vscode.postMessage).not.toHaveBeenCalled()
38+
})
39+
40+
it("updates settings when adding non-default command", () => {
41+
render(
42+
<AutoApproveSettings
43+
allowedCommands={[]}
44+
setCachedStateField={mockSetCachedStateField}
45+
writeDelayMs={1000}
46+
requestDelaySeconds={5}
47+
alwaysAllowExecute={true}
48+
/>,
49+
)
50+
51+
const input = screen.getByTestId("command-input")
52+
fireEvent.change(input, { target: { value: "custom-command" } })
53+
54+
const addButton = screen.getByTestId("add-command-button")
55+
fireEvent.click(addButton)
56+
57+
expect(mockSetCachedStateField).toHaveBeenCalledWith("allowedCommands", ["custom-command"])
58+
expect(vscode.postMessage).toHaveBeenCalledWith({
59+
type: "allowedCommands",
60+
commands: ["custom-command"],
61+
})
62+
})
63+
64+
it("does not update settings when removing command results in default list", () => {
65+
render(
66+
<AutoApproveSettings
67+
allowedCommands={["npm test", "git log", "custom-command"]}
68+
setCachedStateField={mockSetCachedStateField}
69+
writeDelayMs={1000}
70+
requestDelaySeconds={5}
71+
alwaysAllowExecute={true}
72+
/>,
73+
)
74+
75+
// Remove custom command
76+
const removeButton = screen.getByTestId("remove-command-2")
77+
fireEvent.click(removeButton)
78+
79+
// Should not call setCachedStateField or postMessage when result matches defaults
80+
expect(mockSetCachedStateField).not.toHaveBeenCalled()
81+
expect(vscode.postMessage).not.toHaveBeenCalled()
82+
})
83+
84+
it("updates settings when removing command results in non-default list", () => {
85+
render(
86+
<AutoApproveSettings
87+
allowedCommands={["npm test", "custom-command"]}
88+
setCachedStateField={mockSetCachedStateField}
89+
writeDelayMs={1000}
90+
requestDelaySeconds={5}
91+
alwaysAllowExecute={true}
92+
/>,
93+
)
94+
95+
// Remove custom command
96+
const removeButton = screen.getByTestId("remove-command-1")
97+
fireEvent.click(removeButton)
98+
99+
expect(mockSetCachedStateField).toHaveBeenCalledWith("allowedCommands", ["npm test"])
100+
expect(vscode.postMessage).toHaveBeenCalledWith({
101+
type: "allowedCommands",
102+
commands: ["npm test"],
103+
})
104+
})
105+
})

0 commit comments

Comments
 (0)