mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-06 08:18:39 +00:00
Enchancements for #11971
This commit is contained in:
parent
137d3f4fd8
commit
09eb91d593
2 changed files with 199 additions and 2 deletions
|
|
@ -70,6 +70,12 @@ const BaseConfigSchema = z.object({
|
|||
alwaysAllow: z.array(z.string()).default([]),
|
||||
watchPaths: z.array(z.string()).optional(), // paths to watch for changes and restart server
|
||||
disabledTools: z.array(z.string()).default([]),
|
||||
/**
|
||||
* Allowlist of tool names. When non-empty, ONLY these tools will be enabled
|
||||
* (enabledForPrompt=true). All other tools on the server are disabled.
|
||||
* Takes precedence over disabledTools.
|
||||
*/
|
||||
onlyAllow: z.array(z.string()).optional(),
|
||||
})
|
||||
|
||||
// Custom error messages for better user feedback
|
||||
|
|
@ -993,6 +999,7 @@ export class McpHub {
|
|||
let configPath: string
|
||||
let alwaysAllowConfig: string[] = []
|
||||
let disabledToolsList: string[] = []
|
||||
let onlyAllowList: string[] | undefined = undefined
|
||||
|
||||
// Read from the appropriate config file based on the actual source
|
||||
try {
|
||||
|
|
@ -1014,6 +1021,8 @@ export class McpHub {
|
|||
if (serverConfigData) {
|
||||
alwaysAllowConfig = serverConfigData.mcpServers?.[serverName]?.alwaysAllow || []
|
||||
disabledToolsList = serverConfigData.mcpServers?.[serverName]?.disabledTools || []
|
||||
// onlyAllow is undefined when not set (distinct from empty array)
|
||||
onlyAllowList = serverConfigData.mcpServers?.[serverName]?.onlyAllow
|
||||
}
|
||||
} catch (error) {
|
||||
console.error(`Failed to read tool configuration for ${serverName}:`, error)
|
||||
|
|
@ -1023,11 +1032,18 @@ export class McpHub {
|
|||
// Check if wildcard "*" is in the alwaysAllow config
|
||||
const hasWildcard = alwaysAllowConfig.includes("*")
|
||||
|
||||
// Mark tools as always allowed and enabled for prompt based on settings
|
||||
// Determine if onlyAllow mode is active (list is defined and non-empty)
|
||||
const hasOnlyAllow = Array.isArray(onlyAllowList) && onlyAllowList.length > 0
|
||||
|
||||
// Mark tools as always allowed and enabled for prompt based on settings.
|
||||
// onlyAllow takes precedence over disabledTools: when onlyAllow is active,
|
||||
// a tool is enabled only if its name is in the onlyAllow list.
|
||||
const tools = (response?.tools || []).map((tool) => ({
|
||||
...tool,
|
||||
alwaysAllow: hasWildcard || alwaysAllowConfig.includes(tool.name),
|
||||
enabledForPrompt: !disabledToolsList.includes(tool.name),
|
||||
enabledForPrompt: hasOnlyAllow
|
||||
? onlyAllowList!.includes(tool.name)
|
||||
: !disabledToolsList.includes(tool.name),
|
||||
}))
|
||||
|
||||
return tools
|
||||
|
|
|
|||
|
|
@ -1051,6 +1051,187 @@ describe("McpHub", () => {
|
|||
expect(tools[0].alwaysAllow).toBe(true) // allowed-tool
|
||||
expect(tools[1].alwaysAllow).toBe(false) // not-allowed-tool
|
||||
})
|
||||
|
||||
it("should enable only tools listed in onlyAllow and disable all others", async () => {
|
||||
const mockConfig = {
|
||||
mcpServers: {
|
||||
"test-server": {
|
||||
type: "stdio",
|
||||
command: "node",
|
||||
args: ["test.js"],
|
||||
onlyAllow: ["tool1", "tool3"],
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(mockConfig))
|
||||
|
||||
const mockConnection: ConnectedMcpConnection = {
|
||||
type: "connected",
|
||||
server: {
|
||||
name: "test-server",
|
||||
type: "stdio",
|
||||
command: "node",
|
||||
args: ["test.js"],
|
||||
source: "global",
|
||||
} as any,
|
||||
client: {
|
||||
request: vi.fn().mockResolvedValue({
|
||||
tools: [
|
||||
{ name: "tool1", description: "Tool 1" },
|
||||
{ name: "tool2", description: "Tool 2" },
|
||||
{ name: "tool3", description: "Tool 3" },
|
||||
{ name: "tool4", description: "Tool 4" },
|
||||
],
|
||||
}),
|
||||
} as any,
|
||||
transport: {} as any,
|
||||
}
|
||||
mcpHub.connections = [mockConnection]
|
||||
|
||||
const tools = await mcpHub["fetchToolsList"]("test-server", "global")
|
||||
|
||||
expect(tools.length).toBe(4)
|
||||
expect(tools[0].enabledForPrompt).toBe(true) // tool1 – in onlyAllow
|
||||
expect(tools[1].enabledForPrompt).toBe(false) // tool2 – not in onlyAllow
|
||||
expect(tools[2].enabledForPrompt).toBe(true) // tool3 – in onlyAllow
|
||||
expect(tools[3].enabledForPrompt).toBe(false) // tool4 – not in onlyAllow
|
||||
})
|
||||
|
||||
it("should take precedence over disabledTools when onlyAllow is set", async () => {
|
||||
const mockConfig = {
|
||||
mcpServers: {
|
||||
"test-server": {
|
||||
type: "stdio",
|
||||
command: "node",
|
||||
args: ["test.js"],
|
||||
onlyAllow: ["tool1"],
|
||||
disabledTools: ["tool1"], // onlyAllow should win
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(mockConfig))
|
||||
|
||||
const mockConnection: ConnectedMcpConnection = {
|
||||
type: "connected",
|
||||
server: {
|
||||
name: "test-server",
|
||||
type: "stdio",
|
||||
command: "node",
|
||||
args: ["test.js"],
|
||||
source: "global",
|
||||
} as any,
|
||||
client: {
|
||||
request: vi.fn().mockResolvedValue({
|
||||
tools: [
|
||||
{ name: "tool1", description: "Tool 1" },
|
||||
{ name: "tool2", description: "Tool 2" },
|
||||
],
|
||||
}),
|
||||
} as any,
|
||||
transport: {} as any,
|
||||
}
|
||||
mcpHub.connections = [mockConnection]
|
||||
|
||||
const tools = await mcpHub["fetchToolsList"]("test-server", "global")
|
||||
|
||||
expect(tools.length).toBe(2)
|
||||
// onlyAllow wins: tool1 is enabled even though it's also in disabledTools
|
||||
expect(tools[0].enabledForPrompt).toBe(true)
|
||||
// tool2 is not in onlyAllow so it's disabled
|
||||
expect(tools[1].enabledForPrompt).toBe(false)
|
||||
})
|
||||
|
||||
it("should fall back to disabledTools behaviour when onlyAllow is absent", async () => {
|
||||
const mockConfig = {
|
||||
mcpServers: {
|
||||
"test-server": {
|
||||
type: "stdio",
|
||||
command: "node",
|
||||
args: ["test.js"],
|
||||
disabledTools: ["tool2"],
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(mockConfig))
|
||||
|
||||
const mockConnection: ConnectedMcpConnection = {
|
||||
type: "connected",
|
||||
server: {
|
||||
name: "test-server",
|
||||
type: "stdio",
|
||||
command: "node",
|
||||
args: ["test.js"],
|
||||
source: "global",
|
||||
} as any,
|
||||
client: {
|
||||
request: vi.fn().mockResolvedValue({
|
||||
tools: [
|
||||
{ name: "tool1", description: "Tool 1" },
|
||||
{ name: "tool2", description: "Tool 2" },
|
||||
{ name: "tool3", description: "Tool 3" },
|
||||
],
|
||||
}),
|
||||
} as any,
|
||||
transport: {} as any,
|
||||
}
|
||||
mcpHub.connections = [mockConnection]
|
||||
|
||||
const tools = await mcpHub["fetchToolsList"]("test-server", "global")
|
||||
|
||||
expect(tools.length).toBe(3)
|
||||
expect(tools[0].enabledForPrompt).toBe(true) // tool1 – not disabled
|
||||
expect(tools[1].enabledForPrompt).toBe(false) // tool2 – in disabledTools
|
||||
expect(tools[2].enabledForPrompt).toBe(true) // tool3 – not disabled
|
||||
})
|
||||
|
||||
it("should treat an empty onlyAllow array as if onlyAllow is not set", async () => {
|
||||
const mockConfig = {
|
||||
mcpServers: {
|
||||
"test-server": {
|
||||
type: "stdio",
|
||||
command: "node",
|
||||
args: ["test.js"],
|
||||
onlyAllow: [], // empty – should NOT disable all tools
|
||||
disabledTools: ["tool2"],
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(mockConfig))
|
||||
|
||||
const mockConnection: ConnectedMcpConnection = {
|
||||
type: "connected",
|
||||
server: {
|
||||
name: "test-server",
|
||||
type: "stdio",
|
||||
command: "node",
|
||||
args: ["test.js"],
|
||||
source: "global",
|
||||
} as any,
|
||||
client: {
|
||||
request: vi.fn().mockResolvedValue({
|
||||
tools: [
|
||||
{ name: "tool1", description: "Tool 1" },
|
||||
{ name: "tool2", description: "Tool 2" },
|
||||
{ name: "tool3", description: "Tool 3" },
|
||||
],
|
||||
}),
|
||||
} as any,
|
||||
transport: {} as any,
|
||||
}
|
||||
mcpHub.connections = [mockConnection]
|
||||
|
||||
const tools = await mcpHub["fetchToolsList"]("test-server", "global")
|
||||
|
||||
// Falls back to disabledTools behaviour since onlyAllow is empty
|
||||
expect(tools.length).toBe(3)
|
||||
expect(tools[0].enabledForPrompt).toBe(true) // tool1
|
||||
expect(tools[1].enabledForPrompt).toBe(false) // tool2 – in disabledTools
|
||||
expect(tools[2].enabledForPrompt).toBe(true) // tool3
|
||||
})
|
||||
})
|
||||
|
||||
describe("toggleToolEnabledForPrompt", () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue