From b715e0161f9380d7895498a51d1fd7b3ebfdeba7 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sun, 31 Aug 2025 10:21:48 +0000 Subject: [PATCH] fix: respect auto-approval flags for IPC automation (#7565) - Add auto-approval logic to Task.ask method to check autoApprovalEnabled flag - Auto-approve MCP server requests when alwaysAllowMcp is true - Auto-approve TODO list updates when alwaysAllowUpdateTodoList is true - Auto-approve followup questions when alwaysAllowFollowupQuestions is true - Add comprehensive unit tests for auto-approval functionality - Fix p-wait-for mock in tests to properly simulate async waiting This allows complete automation via IPC sockets without user intervention when the appropriate allow flags are set, fixing the issue where RooCode would still ask for user input despite all flags being enabled. Fixes #7565 --- src/core/task/Task.ts | 43 +++++ src/core/task/__tests__/Task.spec.ts | 233 ++++++++++++++++++++++++++- 2 files changed, 275 insertions(+), 1 deletion(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 655983db20..c32427d481 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -680,6 +680,49 @@ export class Task extends EventEmitter implements TaskLike { throw new Error(`[RooCode#ask] task ${this.taskId}.${this.instanceId} aborted`) } + // Check for auto-approval based on the ask type and settings + // This allows IPC automation to work without user intervention + if (!partial && !isProtected) { + const state = await this.providerRef.deref()?.getState() + const { autoApprovalEnabled, alwaysAllowMcp, alwaysAllowUpdateTodoList, alwaysAllowFollowupQuestions } = + state ?? {} + + // Only auto-approve if autoApprovalEnabled is true + if (autoApprovalEnabled) { + // Check specific auto-approval flags based on ask type + let shouldAutoApprove = false + + switch (type) { + case "use_mcp_server": + // Auto-approve MCP server/resource requests if flag is set + shouldAutoApprove = alwaysAllowMcp ?? false + break + case "tool": + // Check if this is a todo list update by examining the text + if (text) { + try { + const parsed = JSON.parse(text) + if (parsed.tool === "updateTodoList") { + shouldAutoApprove = alwaysAllowUpdateTodoList ?? false + } + } catch { + // Not a JSON tool request, don't auto-approve + } + } + break + case "followup": + // Auto-approve followup questions if flag is set + shouldAutoApprove = alwaysAllowFollowupQuestions ?? false + break + } + + if (shouldAutoApprove) { + // Return auto-approved response immediately + return { response: "yesButtonClicked", text: undefined, images: undefined } + } + } + } + let askTs: number if (partial !== undefined) { diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 116c78d760..bb17b55fbe 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -69,7 +69,13 @@ vi.mock("fs/promises", async (importOriginal) => { }) vi.mock("p-wait-for", () => ({ - default: vi.fn().mockImplementation(async () => Promise.resolve()), + default: vi.fn().mockImplementation(async (condition, options) => { + // Actually wait for the condition to be true + const interval = options?.interval || 100 + while (!(await condition())) { + await new Promise((resolve) => setTimeout(resolve, interval)) + } + }), })) vi.mock("vscode", () => { @@ -1776,4 +1782,229 @@ describe("Cline", () => { consoleErrorSpy.mockRestore() }) }) + + describe("Task.ask auto-approval", () => { + it("should auto-approve MCP server requests when alwaysAllowMcp is true", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Mock provider state with auto-approval enabled + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: true, + alwaysAllowMcp: true, + }) + + // Call ask with use_mcp_server type + const result = await task.ask("use_mcp_server", "test MCP request") + + // Should auto-approve without waiting + expect(result.response).toBe("yesButtonClicked") + expect(result.text).toBeUndefined() + expect(result.images).toBeUndefined() + }) + + it("should auto-approve TODO list updates when alwaysAllowUpdateTodoList is true", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Mock provider state with auto-approval enabled + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: true, + alwaysAllowUpdateTodoList: true, + }) + + // Call ask with tool type for updateTodoList + const toolMessage = JSON.stringify({ tool: "updateTodoList" }) + const result = await task.ask("tool", toolMessage) + + // Should auto-approve without waiting + expect(result.response).toBe("yesButtonClicked") + expect(result.text).toBeUndefined() + expect(result.images).toBeUndefined() + }) + + it("should auto-approve followup questions when alwaysAllowFollowupQuestions is true", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Mock provider state with auto-approval enabled + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: true, + alwaysAllowFollowupQuestions: true, + }) + + // Call ask with followup type + const result = await task.ask("followup", "test followup question") + + // Should auto-approve without waiting + expect(result.response).toBe("yesButtonClicked") + expect(result.text).toBeUndefined() + expect(result.images).toBeUndefined() + }) + + it("should not auto-approve when autoApprovalEnabled is false", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Mock provider state with auto-approval disabled + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: false, + alwaysAllowMcp: true, + }) + + // Mock postStateToWebview to avoid errors + mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined) + + // Start the ask operation + const askPromise = task.ask("use_mcp_server", "test MCP request") + + // Give the ask method time to set up + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Simulate user response + task.handleWebviewAskResponse("yesButtonClicked") + + // Wait for the ask promise to resolve + const result = await askPromise + + // Should wait for user response + expect(result.response).toBe("yesButtonClicked") + }) + + it("should not auto-approve protected requests", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Mock provider state with auto-approval enabled + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: true, + alwaysAllowMcp: true, + }) + + // Mock postStateToWebview to avoid errors + mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined) + + // Start the ask operation with isProtected flag + const askPromise = task.ask("use_mcp_server", "test MCP request", false, undefined, true) + + // Give the ask method time to set up + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Simulate user response + task.handleWebviewAskResponse("yesButtonClicked") + + // Wait for the ask promise to resolve + const result = await askPromise + + // Should wait for user response even with auto-approval enabled + expect(result.response).toBe("yesButtonClicked") + }) + + it("should not auto-approve partial messages", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Mock provider state with auto-approval enabled + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: true, + alwaysAllowMcp: true, + }) + + // Call ask with partial flag - should throw error for partial + await expect(task.ask("use_mcp_server", "test MCP request", true)).rejects.toThrow( + "Current ask promise was ignored", + ) + }) + + it("should not auto-approve non-tool 'tool' type requests", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Mock provider state with auto-approval enabled + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: true, + alwaysAllowUpdateTodoList: true, + }) + + // Mock postStateToWebview to avoid errors + mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined) + + // Start the ask operation with a different tool + const toolMessage = JSON.stringify({ tool: "someOtherTool" }) + const askPromise = task.ask("tool", toolMessage) + + // Give the ask method time to set up + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Simulate user response + task.handleWebviewAskResponse("yesButtonClicked") + + // Wait for the ask promise to resolve + const result = await askPromise + + // Should wait for user response + expect(result.response).toBe("yesButtonClicked") + }) + + it("should handle invalid JSON in tool messages gracefully", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Mock provider state with auto-approval enabled + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: true, + alwaysAllowUpdateTodoList: true, + }) + + // Mock postStateToWebview to avoid errors + mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined) + + // Start the ask operation with invalid JSON + const askPromise = task.ask("tool", "not valid JSON") + + // Give the ask method time to set up + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Simulate user response + task.handleWebviewAskResponse("yesButtonClicked") + + // Wait for the ask promise to resolve + const result = await askPromise + + // Should wait for user response + expect(result.response).toBe("yesButtonClicked") + }) + }) })