fix: enforce and persist workspace-scoped auto-approve command lists (#1939)

packages/types/src/global-settings.ts

@@ -54,6 +54,17 @@
 
 export const DEFAULT_DESTRUCTIVE_COMMAND_GUARD_ENABLED = false
 
+/**
+ * Whether workspace-scoped command-list layers (`zoo-code.allowedCommands` /
+ * `zoo-code.deniedCommands` values set in workspace settings) participate in
+ * command auto-approval at all. Opt-in: while off, workspace-layer entries
+ * contribute nothing to enforcement or display. The manifest declares the key
+ * with `scope: "application"`, so only user settings can change it; this
+ * constant is the single default shared by every consumer reading the
+ * setting.
+ */
+export const DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED = false
+
 /**
  * Whether commands that are not explicitly auto-approved are automatically
  * denied (with a structured reason sent to the model) instead of prompting the
@@ -180,6 +191,14 @@
 	followupAutoApproveTimeoutMs: z.number().optional(),
 	allowedCommands: z.array(z.string()).optional(),
 	deniedCommands: z.array(z.string()).optional(),
+	/**
+	 * Gate for the workspace-scoped command-list layers. While false (the
+	 * default), workspace-set `allowedCommands`/`deniedCommands` entries
+	 * contribute nothing; when true they are composed under the global layer,
+	 * which outranks them wherever it matches a command.
+	 * @default false
+	 */
+	workspaceCommandListsEnabled: z.boolean().optional(),
 	commandExecutionTimeout: z.number().optional(),
 	commandTimeoutAllowlist: z.array(z.string()).optional(),
 	preventCompletionWithOpenTodos: z.boolean().optional(),

packages/types/src/vscode-extension-host.ts

@@ -304,6 +304,7 @@ export type ExtensionState = Pick<
 	| "followupAutoApproveTimeoutMs"
 	| "allowedCommands"
 	| "deniedCommands"
+	| "workspaceCommandListsEnabled"
 	| "allowedMaxRequests"
 	| "allowedMaxCost"
 	| "ttsEnabled"
@@ -387,6 +388,18 @@ export type ExtensionState = Pick<
 	toolRequirements?: Record<string, boolean> // Map of tool names to their requirements (e.g. {"apply_diff": true})
 
 	cwd?: string // Current working directory
+
+	/**
+	 * The user-scope command lists as persisted in extension global state,
+	 * before any workspace-scoped composition. Auto-approval composes the
+	 * pair it enforces from these raw layers plus the folder the task runs
+	 * in, because the merged `allowedCommands`/`deniedCommands` pair follows
+	 * the window's first folder and would judge a task by another root's
+	 * lists. Not user settings: runtime state, written by the provider's
+	 * state bake, never by the settings dialog.
+	 */
+	globalAllowedCommands?: string[]
+	globalDeniedCommands?: string[]
 	telemetrySetting: TelemetrySetting
 	telemetryKey?: string
 	machineId?: string
@@ -666,9 +679,7 @@ export interface WebviewMessage {
 		| "getDismissedUpsells"
 		| "openMarkdownPreview"
 		| "updateSettings"
-		| "allowedCommands"
 		| "getTaskWithAggregatedCosts"
-		| "deniedCommands"
 		| "openDebugApiHistory"
 		| "openDebugUiHistory"
 		| "downloadErrorDiagnostics"
@@ -726,7 +737,6 @@ export interface WebviewMessage {
 	stepIndex?: number
 	isLaunchAction?: boolean
 	forceShow?: boolean
-	commands?: string[]
 	audioType?: AudioType
 	serverName?: string
 	toolName?: string
@@ -815,6 +825,14 @@ export interface WebviewMessage {
 		codebaseIndexOpenRouterApiKey?: string
 	}
 	updatedSettings?: RooCodeSettings
+	/**
+	 * The command lists as the settings dialog last displayed them, sent alongside the
+	 * save that rewrites them. The settings file can gain entries while the dialog is
+	 * open, and without this baseline the save path cannot tell those additions apart
+	 * from list entries the user deleted: both look like "in the file, absent from the
+	 * submitted list". Senders that do not display the lists omit it.
+	 */
+	commandListBaseline?: { allowedCommands?: string[]; deniedCommands?: string[] }
 	/** Task configuration applied via `createTask()`. */
 	taskConfiguration?: RooCodeSettings
 	// Worktree properties

src/__tests__/extension.spec.ts

@@ -22,6 +22,7 @@ vi.mock("vscode", () => ({
 		registerTextDocumentContentProvider: vi.fn(),
 		getConfiguration: vi.fn().mockReturnValue({
 			get: vi.fn().mockReturnValue([]),
+			inspect: vi.fn(),
 		}),
 		createFileSystemWatcher: vi.fn().mockReturnValue({
 			onDidCreate: vi.fn(),
@@ -228,15 +229,17 @@ vi.mock("../api/providers/fetchers/modelCache", () => ({
 
 describe("extension.ts", () => {
 	let mockContext: vscode.ExtensionContext
+	let globalStateValues: Map<string, unknown>
 	let settingsUpdatedHandler: ((data: Record<string, never>) => void | Promise<void>) | undefined
 
 	beforeEach(() => {
 		vi.clearAllMocks()
 
+		globalStateValues = new Map<string, unknown>()
 		mockContext = {
 			extensionPath: "/test/path",
 			globalState: {
-				get: vi.fn().mockReturnValue(undefined),
+				get: vi.fn((key: string) => globalStateValues.get(key)),
 				update: vi.fn(),
 			},
 			subscriptions: [],
@@ -536,6 +539,236 @@ describe("extension.ts", () => {
 		expect(dotenv.config).toHaveBeenCalledTimes(1)
 	})
 
+	describe("command-list activation seeding", () => {
+		function setConfigScope(
+			vscode: typeof import("vscode"),
+			scope: { globalValue?: string[]; workspaceValue?: string[]; deniedGlobalValue?: string[]; gate?: boolean },
+		) {
+			const { globalValue, workspaceValue, deniedGlobalValue, gate = false } = scope
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
+				// Key-scoped so proxy/other activation reads still see no value.
+				get: vi.fn((key: string) => {
+					if (key === "workspaceCommandListsEnabled") {
+						return gate
+					}
+					return key === "allowedCommands" ? [...(globalValue ?? []), ...(workspaceValue ?? [])] : undefined
+				}),
+				// Key-scoped inspect: the deny seed must not read the allow scope's
+				// values through the same fixture object.
+				inspect: vi.fn().mockImplementation((key: string) =>
+					key === "deniedCommands"
+						? { key, defaultValue: [], globalValue: deniedGlobalValue }
+						: {
+								key,
+								// Real inspect() always reports the declared package default;
+								// including it here lets the workspace-override test prove the
+								// default stays unseeded beside an override, not just absent.
+								defaultValue: ["git log", "git diff", "git show"],
+								globalValue,
+								workspaceValue,
+							},
+				),
+				update: vi.fn(),
+				// The double scripts key-scoped get() and per-scope inspect()
+				// shapes the shared helper cannot express without obscuring them.
+			} as unknown as vscode.WorkspaceConfiguration)
+		}
+
+		beforeEach(() => {
+			vi.resetModules()
+		})
+
+		afterEach(async () => {
+			const vscode = await import("vscode")
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
+				get: vi.fn().mockReturnValue([]),
+				inspect: vi.fn(),
+				// Restores the neutral default after the scope fixtures above;
+				// update is intentionally absent because no test runs after this.
+			} as unknown as vscode.WorkspaceConfiguration)
+		})
+
+		test("seeds both command lists before initializing the terminal registry", async () => {
+			// Invariant: a seed rejection must fail activation before the terminal
+			// registry is initialized, so no initialized-but-half-activated state is
+			// reachable (the registry's listeners are released only via deactivate,
+			// which the host does not guarantee after a failed activation).
+			const order: string[] = []
+			const seedCommandList = vi.fn(async (_globalState: vscode.Memento, configKey: string): Promise<void> => {
+				order.push(`seed:${configKey}`)
+			})
+			vi.doMock("../utils/seedAllowedCommands", () => ({ seedCommandList }))
+			try {
+				const { TerminalRegistry } = await import("../integrations/terminal/TerminalRegistry")
+				vi.mocked(TerminalRegistry.initialize).mockImplementation(() => void order.push("initialize"))
+
+				const { activate } = await import("../extension")
+				await activate(mockContext)
+
+				expect(order).toEqual(["seed:allowedCommands", "seed:deniedCommands", "initialize"])
+				expect(TerminalRegistry.initialize).toHaveBeenCalledTimes(1)
+			} finally {
+				vi.doUnmock("../utils/seedAllowedCommands")
+			}
+		})
+
+		test("aborts activation before the terminal registry is initialized when seeding rejects", async () => {
+			vi.doMock("../utils/seedAllowedCommands", () => ({
+				seedCommandList: vi.fn().mockRejectedValue(new Error("seed state write failed")),
+			}))
+			try {
+				const { TerminalRegistry } = await import("../integrations/terminal/TerminalRegistry")
+				const { ClineProvider } = await import("../core/webview/ClineProvider")
+
+				const { activate } = await import("../extension")
+				await expect(activate(mockContext)).rejects.toThrow("seed state write failed")
+
+				// The seed rejection must reach the host unchanged, with the registry
+				// never initialized and no downstream activation (sidebar provider) reached.
+				expect(TerminalRegistry.initialize).not.toHaveBeenCalled()
+				expect(ClineProvider).not.toHaveBeenCalled()
+			} finally {
+				vi.doUnmock("../utils/seedAllowedCommands")
+			}
+		})
+
+		test("disposes the auth secret-storage listener when seeding rejects activation", async () => {
+			// A rejected seed aborts activation after the auth initializer has
+			// registered its secret-storage listener, and the host does not guarantee
+			// `deactivate()` after a failed activation, so the failure path must
+			// release the retained listener disposable itself.
+			vi.doMock("../utils/seedAllowedCommands", () => ({
+				seedCommandList: vi.fn().mockRejectedValue(new Error("seed state write failed")),
+			}))
+			const dispose = vi.fn()
+			const authListener = { dispose }
+			vi.doMock("../services/zoo-code-auth", () => ({
+				initZooCodeAuth: vi.fn().mockResolvedValue(authListener),
+			}))
+			try {
+				const { activate } = await import("../extension")
+				await expect(activate(mockContext)).rejects.toThrow("seed state write failed")
+
+				// Retained for the success path (deactivate disposes subscriptions) and
+				// disposed on the failure path, exactly once, with the error propagated.
+				expect(mockContext.subscriptions).toContain(authListener)
+				expect(dispose).toHaveBeenCalledExactlyOnceWith()
+			} finally {
+				vi.doUnmock("../utils/seedAllowedCommands")
+				vi.doUnmock("../services/zoo-code-auth")
+			}
+		})
+
+		test("shuts down the telemetry service before propagating a seed rejection", async () => {
+			// The telemetry client is created and registered before the seed point,
+			// so a rejected seed must shut it down on the failure path: without
+			// `deactivate()` after a failed activation, an unshut-down client keeps
+			// sending events against an extension the host treats as inactive.
+			vi.doMock("../utils/seedAllowedCommands", () => ({
+				seedCommandList: vi.fn().mockRejectedValue(new Error("seed state write failed")),
+			}))
+			const teardownOrder: string[] = []
+			vi.doMock("../services/zoo-code-auth", () => ({
+				initZooCodeAuth: vi
+					.fn()
+					.mockResolvedValue({ dispose: () => void teardownOrder.push("auth-listener-disposed") }),
+			}))
+			const { TelemetryService } = await import("@roo-code/telemetry")
+			const shutdown = vi.mocked(TelemetryService.instance.shutdown)
+			// A never-settling shutdown: activation may only reject once the awaited
+			// teardown settles, pinning that the seed rejection never reaches the
+			// host ahead of it.
+			let settleShutdown: () => void = () => {}
+			shutdown.mockImplementation(() => {
+				teardownOrder.push("telemetry-shutdown")
+				return new Promise<void>((resolve) => {
+					settleShutdown = resolve
+				})
+			})
+			try {
+				const { activate } = await import("../extension")
+				const activation = activate(mockContext)
+				let rejected = false
+				void activation.catch(() => {
+					rejected = true
+				})
+
+				// Both teardown steps run on the failure path, in order, exactly once.
+				await vi.waitFor(() => expect(teardownOrder).toEqual(["auth-listener-disposed", "telemetry-shutdown"]))
+
+				// Still unrejected while the awaited shutdown is unsettled: the catch
+				// must not propagate the seed rejection before teardown completes.
+				expect(rejected).toBe(false)
+
+				settleShutdown()
+				await expect(activation).rejects.toThrow("seed state write failed")
+			} finally {
+				shutdown.mockReset()
+				vi.doUnmock("../utils/seedAllowedCommands")
+				vi.doUnmock("../services/zoo-code-auth")
+			}
+		})
+
+		test("does not shut down telemetry when activation completes", async () => {
+			// Telemetry shutdown is failure-path only: a successfully activated
+			// extension owns the telemetry client until deactivate() runs.
+			const { TelemetryService } = await import("@roo-code/telemetry")
+
+			const { activate } = await import("../extension")
+			await expect(activate(mockContext)).resolves.toBeDefined()
+
+			expect(TelemetryService.instance.shutdown).not.toHaveBeenCalled()
+		})
+
+		test("does not seed global state from an enforced workspace-scoped override", async () => {
+			const vscode = await import("vscode")
+			// The gate is on: an enforced workspace list replaces the default across
+			// scopes, so the seed must not re-grant the approvals it removed. (With
+			// the gate off the workspace layer is inert and the default is seeded —
+			// pinned in the seed module's own spec.)
+			setConfigScope(vscode, { workspaceValue: ["docker"], gate: true })
+
+			const { activate } = await import("../extension")
+			await activate(mockContext)
+
+			expect(mockContext.globalState.update).not.toHaveBeenCalledWith("allowedCommands", expect.anything())
+			expect(globalStateValues.get("allowedCommands")).toBeUndefined()
+		})
+
+		test("seeds global state from the user-scoped value when it is empty", async () => {
+			const vscode = await import("vscode")
+			setConfigScope(vscode, { globalValue: ["git"] })
+
+			const { activate } = await import("../extension")
+			await activate(mockContext)
+
+			expect(mockContext.globalState.update).toHaveBeenCalledWith("allowedCommands", ["git"])
+		})
+
+		test("does not overwrite global state that is already populated", async () => {
+			const vscode = await import("vscode")
+			setConfigScope(vscode, { globalValue: ["git"] })
+			globalStateValues.set("allowedCommands", ["npm"])
+
+			const { activate } = await import("../extension")
+			await activate(mockContext)
+
+			expect(mockContext.globalState.update).not.toHaveBeenCalledWith("allowedCommands", expect.anything())
+			expect(globalStateValues.get("allowedCommands")).toEqual(["npm"])
+		})
+
+		test("seeds both command-list keys on a fresh profile", async () => {
+			const vscode = await import("vscode")
+			setConfigScope(vscode, { globalValue: ["git"], deniedGlobalValue: ["docker"] })
+
+			const { activate } = await import("../extension")
+			await activate(mockContext)
+
+			expect(mockContext.globalState.update).toHaveBeenCalledWith("allowedCommands", ["git"])
+			expect(mockContext.globalState.update).toHaveBeenCalledWith("deniedCommands", ["docker"])
+		})
+	})
+
 	describe("cloud organization settings handling", () => {
 		beforeEach(() => {
 			vi.resetModules()