Skip to content

Commit b715e01

Browse files
roomoteroomote
committed
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
1 parent 63b71d8 commit b715e01

File tree

2 files changed

+275
-1
lines changed

2 files changed

+275
-1
lines changed

‎src/core/task/Task.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,49 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
680680
throw new Error(`[RooCode#ask] task ${this.taskId}.${this.instanceId} aborted`)
681681
}
682682

683+
// Check for auto-approval based on the ask type and settings
684+
// This allows IPC automation to work without user intervention
685+
if (!partial && !isProtected) {
686+
const state = await this.providerRef.deref()?.getState()
687+
const { autoApprovalEnabled, alwaysAllowMcp, alwaysAllowUpdateTodoList, alwaysAllowFollowupQuestions } =
688+
state ?? {}
689+
690+
// Only auto-approve if autoApprovalEnabled is true
691+
if (autoApprovalEnabled) {
692+
// Check specific auto-approval flags based on ask type
693+
let shouldAutoApprove = false
694+
695+
switch (type) {
696+
case "use_mcp_server":
697+
// Auto-approve MCP server/resource requests if flag is set
698+
shouldAutoApprove = alwaysAllowMcp ?? false
699+
break
700+
case "tool":
701+
// Check if this is a todo list update by examining the text
702+
if (text) {
703+
try {
704+
const parsed = JSON.parse(text)
705+
if (parsed.tool === "updateTodoList") {
706+
shouldAutoApprove = alwaysAllowUpdateTodoList ?? false
707+
}
708+
} catch {
709+
// Not a JSON tool request, don't auto-approve
710+
}
711+
}
712+
break
713+
case "followup":
714+
// Auto-approve followup questions if flag is set
715+
shouldAutoApprove = alwaysAllowFollowupQuestions ?? false
716+
break
717+
}
718+
719+
if (shouldAutoApprove) {
720+
// Return auto-approved response immediately
721+
return { response: "yesButtonClicked", text: undefined, images: undefined }
722+
}
723+
}
724+
}
725+
683726
let askTs: number
684727

685728
if (partial !== undefined) {

‎src/core/task/__tests__/Task.spec.ts‎

Lines changed: 232 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,13 @@ vi.mock("fs/promises", async (importOriginal) => {
6969
})
7070

7171
vi.mock("p-wait-for", () => ({
72-
default: vi.fn().mockImplementation(async () => Promise.resolve()),
72+
default: vi.fn().mockImplementation(async (condition, options) => {
73+
// Actually wait for the condition to be true
74+
const interval = options?.interval || 100
75+
while (!(await condition())) {
76+
await new Promise((resolve) => setTimeout(resolve, interval))
77+
}
78+
}),
7379
}))
7480

7581
vi.mock("vscode", () => {
@@ -1776,4 +1782,229 @@ describe("Cline", () => {
17761782
consoleErrorSpy.mockRestore()
17771783
})
17781784
})
1785+
1786+
describe("Task.ask auto-approval", () => {
1787+
it("should auto-approve MCP server requests when alwaysAllowMcp is true", async () => {
1788+
const task = new Task({
1789+
provider: mockProvider,
1790+
apiConfiguration: mockApiConfig,
1791+
task: "test task",
1792+
startTask: false,
1793+
})
1794+
1795+
// Mock provider state with auto-approval enabled
1796+
mockProvider.getState = vi.fn().mockResolvedValue({
1797+
autoApprovalEnabled: true,
1798+
alwaysAllowMcp: true,
1799+
})
1800+
1801+
// Call ask with use_mcp_server type
1802+
const result = await task.ask("use_mcp_server", "test MCP request")
1803+
1804+
// Should auto-approve without waiting
1805+
expect(result.response).toBe("yesButtonClicked")
1806+
expect(result.text).toBeUndefined()
1807+
expect(result.images).toBeUndefined()
1808+
})
1809+
1810+
it("should auto-approve TODO list updates when alwaysAllowUpdateTodoList is true", async () => {
1811+
const task = new Task({
1812+
provider: mockProvider,
1813+
apiConfiguration: mockApiConfig,
1814+
task: "test task",
1815+
startTask: false,
1816+
})
1817+
1818+
// Mock provider state with auto-approval enabled
1819+
mockProvider.getState = vi.fn().mockResolvedValue({
1820+
autoApprovalEnabled: true,
1821+
alwaysAllowUpdateTodoList: true,
1822+
})
1823+
1824+
// Call ask with tool type for updateTodoList
1825+
const toolMessage = JSON.stringify({ tool: "updateTodoList" })
1826+
const result = await task.ask("tool", toolMessage)
1827+
1828+
// Should auto-approve without waiting
1829+
expect(result.response).toBe("yesButtonClicked")
1830+
expect(result.text).toBeUndefined()
1831+
expect(result.images).toBeUndefined()
1832+
})
1833+
1834+
it("should auto-approve followup questions when alwaysAllowFollowupQuestions is true", async () => {
1835+
const task = new Task({
1836+
provider: mockProvider,
1837+
apiConfiguration: mockApiConfig,
1838+
task: "test task",
1839+
startTask: false,
1840+
})
1841+
1842+
// Mock provider state with auto-approval enabled
1843+
mockProvider.getState = vi.fn().mockResolvedValue({
1844+
autoApprovalEnabled: true,
1845+
alwaysAllowFollowupQuestions: true,
1846+
})
1847+
1848+
// Call ask with followup type
1849+
const result = await task.ask("followup", "test followup question")
1850+
1851+
// Should auto-approve without waiting
1852+
expect(result.response).toBe("yesButtonClicked")
1853+
expect(result.text).toBeUndefined()
1854+
expect(result.images).toBeUndefined()
1855+
})
1856+
1857+
it("should not auto-approve when autoApprovalEnabled is false", async () => {
1858+
const task = new Task({
1859+
provider: mockProvider,
1860+
apiConfiguration: mockApiConfig,
1861+
task: "test task",
1862+
startTask: false,
1863+
})
1864+
1865+
// Mock provider state with auto-approval disabled
1866+
mockProvider.getState = vi.fn().mockResolvedValue({
1867+
autoApprovalEnabled: false,
1868+
alwaysAllowMcp: true,
1869+
})
1870+
1871+
// Mock postStateToWebview to avoid errors
1872+
mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined)
1873+
1874+
// Start the ask operation
1875+
const askPromise = task.ask("use_mcp_server", "test MCP request")
1876+
1877+
// Give the ask method time to set up
1878+
await new Promise((resolve) => setTimeout(resolve, 50))
1879+
1880+
// Simulate user response
1881+
task.handleWebviewAskResponse("yesButtonClicked")
1882+
1883+
// Wait for the ask promise to resolve
1884+
const result = await askPromise
1885+
1886+
// Should wait for user response
1887+
expect(result.response).toBe("yesButtonClicked")
1888+
})
1889+
1890+
it("should not auto-approve protected requests", async () => {
1891+
const task = new Task({
1892+
provider: mockProvider,
1893+
apiConfiguration: mockApiConfig,
1894+
task: "test task",
1895+
startTask: false,
1896+
})
1897+
1898+
// Mock provider state with auto-approval enabled
1899+
mockProvider.getState = vi.fn().mockResolvedValue({
1900+
autoApprovalEnabled: true,
1901+
alwaysAllowMcp: true,
1902+
})
1903+
1904+
// Mock postStateToWebview to avoid errors
1905+
mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined)
1906+
1907+
// Start the ask operation with isProtected flag
1908+
const askPromise = task.ask("use_mcp_server", "test MCP request", false, undefined, true)
1909+
1910+
// Give the ask method time to set up
1911+
await new Promise((resolve) => setTimeout(resolve, 50))
1912+
1913+
// Simulate user response
1914+
task.handleWebviewAskResponse("yesButtonClicked")
1915+
1916+
// Wait for the ask promise to resolve
1917+
const result = await askPromise
1918+
1919+
// Should wait for user response even with auto-approval enabled
1920+
expect(result.response).toBe("yesButtonClicked")
1921+
})
1922+
1923+
it("should not auto-approve partial messages", async () => {
1924+
const task = new Task({
1925+
provider: mockProvider,
1926+
apiConfiguration: mockApiConfig,
1927+
task: "test task",
1928+
startTask: false,
1929+
})
1930+
1931+
// Mock provider state with auto-approval enabled
1932+
mockProvider.getState = vi.fn().mockResolvedValue({
1933+
autoApprovalEnabled: true,
1934+
alwaysAllowMcp: true,
1935+
})
1936+
1937+
// Call ask with partial flag - should throw error for partial
1938+
await expect(task.ask("use_mcp_server", "test MCP request", true)).rejects.toThrow(
1939+
"Current ask promise was ignored",
1940+
)
1941+
})
1942+
1943+
it("should not auto-approve non-tool 'tool' type requests", async () => {
1944+
const task = new Task({
1945+
provider: mockProvider,
1946+
apiConfiguration: mockApiConfig,
1947+
task: "test task",
1948+
startTask: false,
1949+
})
1950+
1951+
// Mock provider state with auto-approval enabled
1952+
mockProvider.getState = vi.fn().mockResolvedValue({
1953+
autoApprovalEnabled: true,
1954+
alwaysAllowUpdateTodoList: true,
1955+
})
1956+
1957+
// Mock postStateToWebview to avoid errors
1958+
mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined)
1959+
1960+
// Start the ask operation with a different tool
1961+
const toolMessage = JSON.stringify({ tool: "someOtherTool" })
1962+
const askPromise = task.ask("tool", toolMessage)
1963+
1964+
// Give the ask method time to set up
1965+
await new Promise((resolve) => setTimeout(resolve, 50))
1966+
1967+
// Simulate user response
1968+
task.handleWebviewAskResponse("yesButtonClicked")
1969+
1970+
// Wait for the ask promise to resolve
1971+
const result = await askPromise
1972+
1973+
// Should wait for user response
1974+
expect(result.response).toBe("yesButtonClicked")
1975+
})
1976+
1977+
it("should handle invalid JSON in tool messages gracefully", async () => {
1978+
const task = new Task({
1979+
provider: mockProvider,
1980+
apiConfiguration: mockApiConfig,
1981+
task: "test task",
1982+
startTask: false,
1983+
})
1984+
1985+
// Mock provider state with auto-approval enabled
1986+
mockProvider.getState = vi.fn().mockResolvedValue({
1987+
autoApprovalEnabled: true,
1988+
alwaysAllowUpdateTodoList: true,
1989+
})
1990+
1991+
// Mock postStateToWebview to avoid errors
1992+
mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined)
1993+
1994+
// Start the ask operation with invalid JSON
1995+
const askPromise = task.ask("tool", "not valid JSON")
1996+
1997+
// Give the ask method time to set up
1998+
await new Promise((resolve) => setTimeout(resolve, 50))
1999+
2000+
// Simulate user response
2001+
task.handleWebviewAskResponse("yesButtonClicked")
2002+
2003+
// Wait for the ask promise to resolve
2004+
const result = await askPromise
2005+
2006+
// Should wait for user response
2007+
expect(result.response).toBe("yesButtonClicked")
2008+
})
2009+
})
17792010
})

0 commit comments

Comments
 (0)