mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-07 02:58:15 +00:00
fix: MCP tool auto-approval fails when fetchToolsList() hasn't completed
When an MCP tool call is auto-approval checked before fetchToolsList()
has completed, server.tools is empty so tool.alwaysAllow is never found.
The server's config JSON (set at connection creation time) already
contains the alwaysAllow array, so fall back to parsing server.config
when the tool-level flag isn't set. Supports both wildcard ("*") and
specific tool name matching.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
00075684fd
commit
0bd9857662
2 changed files with 103 additions and 2 deletions
85
src/core/auto-approval/__tests__/mcp.spec.ts
Normal file
85
src/core/auto-approval/__tests__/mcp.spec.ts
Normal file
|
|
@ -0,0 +1,85 @@
|
|||
import { isMcpToolAlwaysAllowed } from "../mcp"
|
||||
import type { McpServerUse, McpServer, McpTool } from "@roo-code/types"
|
||||
|
||||
function makeServerUse(serverName: string, toolName: string): McpServerUse {
|
||||
return { type: "use_mcp_tool", serverName, toolName }
|
||||
}
|
||||
|
||||
function makeTool(name: string, alwaysAllow: boolean): McpTool {
|
||||
return { name, description: "test", inputSchema: {}, alwaysAllow } as McpTool
|
||||
}
|
||||
|
||||
function makeServer(name: string, tools: McpTool[], config: string): McpServer {
|
||||
return { name, config, tools } as McpServer
|
||||
}
|
||||
|
||||
describe("isMcpToolAlwaysAllowed", () => {
|
||||
describe("primary check: tool-level alwaysAllow flag", () => {
|
||||
it("returns true when tool has alwaysAllow set", () => {
|
||||
const server = makeServer("my-server", [makeTool("my-tool", true)], "{}")
|
||||
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(true)
|
||||
})
|
||||
|
||||
it("returns false when tool has alwaysAllow unset", () => {
|
||||
const server = makeServer("my-server", [makeTool("my-tool", false)], "{}")
|
||||
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(false)
|
||||
})
|
||||
|
||||
it("returns false when server not found", () => {
|
||||
expect(isMcpToolAlwaysAllowed(makeServerUse("missing-server", "my-tool"), [])).toBe(false)
|
||||
})
|
||||
|
||||
it("returns false when mcpServers is undefined", () => {
|
||||
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), undefined)).toBe(false)
|
||||
})
|
||||
|
||||
it("returns false for non use_mcp_tool type", () => {
|
||||
const serverUse = { type: "access_mcp_resource", serverName: "s", toolName: "t" } as unknown as McpServerUse
|
||||
const server = makeServer("s", [makeTool("t", true)], "{}")
|
||||
expect(isMcpToolAlwaysAllowed(serverUse, [server])).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("fallback: server config alwaysAllow", () => {
|
||||
it("returns true when tool missing alwaysAllow but server config has wildcard", () => {
|
||||
const config = JSON.stringify({ alwaysAllow: ["*"] })
|
||||
const server = makeServer("my-server", [makeTool("my-tool", false)], config)
|
||||
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(true)
|
||||
})
|
||||
|
||||
it("returns true when tool missing alwaysAllow but server config has tool name", () => {
|
||||
const config = JSON.stringify({ alwaysAllow: ["my-tool"] })
|
||||
const server = makeServer("my-server", [makeTool("my-tool", false)], config)
|
||||
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(true)
|
||||
})
|
||||
|
||||
it("returns false when tool missing alwaysAllow and config has different tools", () => {
|
||||
const config = JSON.stringify({ alwaysAllow: ["other-tool"] })
|
||||
const server = makeServer("my-server", [makeTool("my-tool", false)], config)
|
||||
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(false)
|
||||
})
|
||||
|
||||
it("returns false when config has empty alwaysAllow", () => {
|
||||
const config = JSON.stringify({ alwaysAllow: [] })
|
||||
const server = makeServer("my-server", [makeTool("my-tool", false)], config)
|
||||
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(false)
|
||||
})
|
||||
|
||||
it("returns false when config has no alwaysAllow field", () => {
|
||||
const config = JSON.stringify({ command: "node", args: ["server.js"] })
|
||||
const server = makeServer("my-server", [makeTool("my-tool", false)], config)
|
||||
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(false)
|
||||
})
|
||||
|
||||
it("returns false when config is invalid JSON", () => {
|
||||
const server = makeServer("my-server", [makeTool("my-tool", false)], "not-json")
|
||||
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "my-tool"), [server])).toBe(false)
|
||||
})
|
||||
|
||||
it("returns true with wildcard when tool is not in tools list at all", () => {
|
||||
const config = JSON.stringify({ alwaysAllow: ["*"] })
|
||||
const server = makeServer("my-server", [], config)
|
||||
expect(isMcpToolAlwaysAllowed(makeServerUse("my-server", "unlisted-tool"), [server])).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -3,8 +3,24 @@ import type { McpServerUse, McpServer, McpTool } from "@roo-code/types"
|
|||
export function isMcpToolAlwaysAllowed(mcpServerUse: McpServerUse, mcpServers: McpServer[] | undefined): boolean {
|
||||
if (mcpServerUse.type === "use_mcp_tool" && mcpServerUse.toolName) {
|
||||
const server = mcpServers?.find((s: McpServer) => s.name === mcpServerUse.serverName)
|
||||
const tool = server?.tools?.find((t: McpTool) => t.name === mcpServerUse.toolName)
|
||||
return tool?.alwaysAllow || false
|
||||
if (!server) return false
|
||||
|
||||
// Primary check: tool-level flag set by fetchToolsList()
|
||||
const tool = server.tools?.find((t: McpTool) => t.name === mcpServerUse.toolName)
|
||||
if (tool?.alwaysAllow) return true
|
||||
|
||||
// Fallback: check the server's stored config directly.
|
||||
// server.config is the JSON-stringified validated config set at connection
|
||||
// creation time. It includes the alwaysAllow array from mcp_settings.json.
|
||||
// This handles race conditions where fetchToolsList() hasn't completed or
|
||||
// its config file read failed silently.
|
||||
try {
|
||||
const config = JSON.parse(server.config)
|
||||
const alwaysAllowConfig: string[] = config.alwaysAllow || []
|
||||
return alwaysAllowConfig.includes("*") || alwaysAllowConfig.includes(mcpServerUse.toolName)
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue