scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-10-08 03:07:53 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

respect review

Browse source
This commit is contained in:
Александр Родионов 2025-05-31 23:12:52 +03:00 • committed by Daniel Riccio
parent 6ef2c66acf
commit eb00079158
4 changed files with 101 additions and 138 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/__mocks__/fs/promises.ts
View file

@ -168,7 +168,7 @@ const mockFs = {
args: ["test.js"],
disabled: false,
alwaysAllow: ["existing-tool"],
disabledForPromptTools: [],
disabledTools: [],
},
},
}),

4
src/core/prompts/instructions/create-mcp-server.ts
View file

@ -50,7 +50,7 @@ Common configuration options for both types:
- \`disabled\`: (optional) Set to true to temporarily disable the server
- \`timeout\`: (optional) Maximum time in seconds to wait for server responses (default: 60)
- \`alwaysAllow\`: (optional) Array of tool names that don't require user confirmation
- \`disabledForPromptTools\`: (optional) Array of tool names that are not included in the system prompt and won't be used
- \`disabledTools\`: (optional) Array of tool names that are not included in the system prompt and won't be used
### Example Local MCP Server
@ -277,7 +277,7 @@ npm run build
5. Install the MCP Server by adding the MCP server configuration to the settings file located at '${await mcpHub.getMcpSettingsFilePath()}'. The settings file may have other MCP servers already configured, so you would read it first and then add your new server to the existing \`mcpServers\` object.
IMPORTANT: Regardless of what else you see in the MCP settings file, you must default any new MCP servers you create to disabled=false, alwaysAllow=[] and disabledForPromptTools=[].
IMPORTANT: Regardless of what else you see in the MCP settings file, you must default any new MCP servers you create to disabled=false, alwaysAllow=[] and disabledTools=[].
\`\`\`json
{

211
src/services/mcp/McpHub.ts
View file

@ -45,7 +45,7 @@ const BaseConfigSchema = z.object({
timeout: z.number().min(1).max(3600).optional().default(60),
alwaysAllow: z.array(z.string()).default([]),
watchPaths: z.array(z.string()).optional(), // paths to watch for changes and restart server
disabledForPromptTools: z.array(z.string()).default([]),
disabledTools: z.array(z.string()).default([]),
})
// Custom error messages for better user feedback
@ -820,7 +820,7 @@ export class McpHub {
const actualSource = connection.server.source || "global"
let configPath: string
let alwaysAllowConfig: string[] = []
let disabledForPromptToolsList: string[] = []
let disabledToolsList: string[] = []
// Read from the appropriate config file based on the actual source
try {
@ -841,7 +841,7 @@ export class McpHub {
}
if (serverConfigData) {
alwaysAllowConfig = serverConfigData.mcpServers?.[serverName]?.alwaysAllow || []
disabledForPromptToolsList = serverConfigData.mcpServers?.[serverName]?.disabledForPromptTools || []
disabledToolsList = serverConfigData.mcpServers?.[serverName]?.disabledTools || []
}
} catch (error) {
console.error(`Failed to read tool configuration for ${serverName}:`, error)
@ -852,7 +852,7 @@ export class McpHub {
const tools = (response?.tools || []).map((tool) => ({
...tool,
alwaysAllow: alwaysAllowConfig.includes(tool.name),
enabledForPrompt: !disabledForPromptToolsList.includes(tool.name),
enabledForPrompt: !disabledToolsList.includes(tool.name),
}))
return tools
@ -1481,6 +1481,84 @@ export class McpHub {
)
}
/**
* Helper method to update a specific tool list (alwaysAllow or disabledTools)
* in the appropriate settings file.
* @param serverName The name of the server to update
* @param source Whether to update the global or project config
* @param toolName The name of the tool to add or remove
* @param listName The name of the list to modify ("alwaysAllow" or "disabledTools")
* @param addTool Whether to add (true) or remove (false) the tool from the list
*/
private async updateServerToolList(
serverName: string,
source: "global" | "project",
toolName: string,
listName: "alwaysAllow" | "disabledTools",
addTool: boolean,
): Promise<void> {
// Find the connection with matching name and source
const connection = this.findConnection(serverName, source)
if (!connection) {
throw new Error(`Server ${serverName} with source ${source} not found`)
}
// Determine the correct config path based on the source
let configPath: string
if (source === "project") {
// Get project MCP config path
const projectMcpPath = await this.getProjectMcpPath()
if (!projectMcpPath) {
throw new Error("Project MCP configuration file not found")
}
configPath = projectMcpPath
} else {
// Get global MCP settings path
configPath = await this.getMcpSettingsFilePath()
}
// Normalize path for cross-platform compatibility
// Use a consistent path format for both reading and writing
const normalizedPath = process.platform === "win32" ? configPath.replace(/\\/g, "/") : configPath
// Read the appropriate config file
const content = await fs.readFile(normalizedPath, "utf-8")
const config = JSON.parse(content)
if (!config.mcpServers) {
config.mcpServers = {}
}
if (!config.mcpServers[serverName]) {
config.mcpServers[serverName] = {
type: "stdio",
command: "node",
args: [], // Default to an empty array; can be set later if needed
}
}
if (!config.mcpServers[serverName][listName]) {
config.mcpServers[serverName][listName] = []
}
const targetList = config.mcpServers[serverName][listName]
const toolIndex = targetList.indexOf(toolName)
if (addTool && toolIndex === -1) {
targetList.push(toolName)
} else if (!addTool && toolIndex !== -1) {
targetList.splice(toolIndex, 1)
}
await fs.writeFile(normalizedPath, JSON.stringify(config, null, 2))
if (connection) {
connection.server.tools = await this.fetchToolsList(serverName, source)
await this.notifyWebviewOfServerChanges()
}
}
async toggleToolAlwaysAllow(
serverName: string,
source: "global" | "project",
@ -1488,74 +1566,7 @@ export class McpHub {
shouldAllow: boolean,
): Promise<void> {
try {
// Find the connection with matching name and source
const connection = this.findConnection(serverName, source)
if (!connection) {
throw new Error(`Server ${serverName} with source ${source} not found`)
}
// Determine the correct config path based on the source
let configPath: string
if (source === "project") {
// Get project MCP config path
const projectMcpPath = await this.getProjectMcpPath()
if (!projectMcpPath) {
throw new Error("Project MCP configuration file not found")
}
configPath = projectMcpPath
} else {
// Get global MCP settings path
configPath = await this.getMcpSettingsFilePath()
}
// Normalize path for cross-platform compatibility
// Use a consistent path format for both reading and writing
const normalizedPath = process.platform === "win32" ? configPath.replace(/\\/g, "/") : configPath
// Read the appropriate config file
const content = await fs.readFile(normalizedPath, "utf-8")
const config = JSON.parse(content)
// Initialize mcpServers if it doesn't exist
if (!config.mcpServers) {
config.mcpServers = {}
}
// Initialize server config if it doesn't exist
if (!config.mcpServers[serverName]) {
config.mcpServers[serverName] = {
type: "stdio",
command: "node",
args: [], // Default to an empty array; can be set later if needed
}
}
// Initialize alwaysAllow if it doesn't exist
if (!config.mcpServers[serverName].alwaysAllow) {
config.mcpServers[serverName].alwaysAllow = []
}
const alwaysAllow = config.mcpServers[serverName].alwaysAllow
const toolIndex = alwaysAllow.indexOf(toolName)
if (shouldAllow && toolIndex === -1) {
// Add tool to always allow list
alwaysAllow.push(toolName)
} else if (!shouldAllow && toolIndex !== -1) {
// Remove tool from always allow list
alwaysAllow.splice(toolIndex, 1)
}
// Write updated config back to file
await fs.writeFile(normalizedPath, JSON.stringify(config, null, 2))
// Update the tools list to reflect the change
if (connection) {
// Explicitly pass the source to ensure we're updating the correct server's tools
connection.server.tools = await this.fetchToolsList(serverName, source)
await this.notifyWebviewOfServerChanges()
}
await this.updateServerToolList(serverName, source, toolName, "alwaysAllow", shouldAllow)
} catch (error) {
this.showErrorMessage(
`Failed to toggle always allow for tool "${toolName}" on server "${serverName}" with source "${source}"`,
@ -1572,58 +1583,10 @@ export class McpHub {
isEnabled: boolean,
): Promise<void> {
try {
const connection = this.findConnection(serverName, source)
if (!connection) {
throw new Error(`Server ${serverName} with source ${source} not found`)
}
let configPath: string
if (source === "project") {
const projectMcpPath = await this.getProjectMcpPath()
if (!projectMcpPath) {
throw new Error("Project MCP configuration file not found")
}
configPath = projectMcpPath
} else {
configPath = await this.getMcpSettingsFilePath()
}
const normalizedPath = process.platform === "win32" ? configPath.replace(/\\/g, "/") : configPath
const content = await fs.readFile(normalizedPath, "utf-8")
const config = JSON.parse(content)
if (!config.mcpServers) {
config.mcpServers = {}
}
if (!config.mcpServers[serverName]) {
// Initialize with minimal valid config if server entry doesn't exist
config.mcpServers[serverName] = {
type: "stdio",
command: "echo",
args: ["MCP server not fully configured"],
}
}
if (!config.mcpServers[serverName].disabledForPromptTools) {
config.mcpServers[serverName].disabledForPromptTools = []
}
const disabledList = config.mcpServers[serverName].disabledForPromptTools
const toolIndex = disabledList.indexOf(toolName)
if (!isEnabled && toolIndex === -1) {
// If tool should be disabled (not included in prompt) and is not in disabled list
disabledList.push(toolName)
} else if (isEnabled && toolIndex !== -1) {
// If tool should be enabled (included in prompt) and is in disabled list
disabledList.splice(toolIndex, 1)
}
await fs.writeFile(normalizedPath, JSON.stringify(config, null, 2))
if (connection) {
connection.server.tools = await this.fetchToolsList(serverName, source)
await this.notifyWebviewOfServerChanges()
}
// When isEnabled is true, we want to remove the tool from the disabledTools list.
// When isEnabled is false, we want to add the tool to the disabledTools list.
const addToolToDisabledList = !isEnabled
await this.updateServerToolList(serverName, source, toolName, "disabledTools", addToolToDisabledList)
} catch (error) {
this.showErrorMessage(`Failed to update disabledForPromptTool settings for tool ${toolName}`, error)
throw error // Re-throw to ensure the error is properly handled

22
src/services/mcp/__tests__/McpHub.spec.ts
View file

@ -100,7 +100,7 @@ describe("McpHub", () => {
command: "node",
args: ["test.js"],
alwaysAllow: ["allowed-tool"],
disabledForPromptTools: ["disabled-tool"],
disabledTools: ["disabled-tool"],
},
},
}),
@ -259,14 +259,14 @@ describe("McpHub", () => {
})
describe("toggleToolEnabledForPrompt", () => {
it("should add tool to disabledForPromptTools list when enabling", async () => {
it("should add tool to disabledTools list when enabling", async () => {
const mockConfig = {
mcpServers: {
"test-server": {
type: "stdio",
command: "node",
args: ["test.js"],
disabledForPromptTools: [],
disabledTools: [],
},
},
}
@ -290,17 +290,17 @@ describe("McpHub", () => {
expect(writtenConfig.mcpServers).toBeDefined()
expect(writtenConfig.mcpServers["test-server"]).toBeDefined()
expect(Array.isArray(writtenConfig.mcpServers["test-server"].enabledForPrompt)).toBe(false)
expect(writtenConfig.mcpServers["test-server"].disabledForPromptTools).toContain("new-tool")
expect(writtenConfig.mcpServers["test-server"].disabledTools).toContain("new-tool")
})
it("should remove tool from disabledForPromptTools list when disabling", async () => {
it("should remove tool from disabledTools list when disabling", async () => {
const mockConfig = {
mcpServers: {
"test-server": {
type: "stdio",
command: "node",
args: ["test.js"],
disabledForPromptTools: ["existing-tool"],
disabledTools: ["existing-tool"],
},
},
}
@ -324,10 +324,10 @@ describe("McpHub", () => {
expect(writtenConfig.mcpServers).toBeDefined()
expect(writtenConfig.mcpServers["test-server"]).toBeDefined()
expect(Array.isArray(writtenConfig.mcpServers["test-server"].enabledForPrompt)).toBe(false)
expect(writtenConfig.mcpServers["test-server"].disabledForPromptTools).not.toContain("existing-tool")
expect(writtenConfig.mcpServers["test-server"].disabledTools).not.toContain("existing-tool")
})
it("should initialize disabledForPromptTools if it does not exist", async () => {
it("should initialize disabledTools if it does not exist", async () => {
const mockConfig = {
mcpServers: {
"test-server": {
@ -344,7 +344,7 @@ describe("McpHub", () => {
// Call with false because of "true" is default value
await mcpHub.toggleToolEnabledForPrompt("test-server", "global", "new-tool", false)
// Verify the config was updated with initialized disabledForPromptTools
// Verify the config was updated with initialized disabledTools
// Find the write call with the normalized path
const normalizedSettingsPath = "/mock/settings/path/cline_mcp_settings.json"
const writeCalls = (fs.writeFile as jest.Mock).mock.calls
@ -354,8 +354,8 @@ describe("McpHub", () => {
const callToUse = writeCall || writeCalls[0]
const writtenConfig = JSON.parse(callToUse[1])
expect(writtenConfig.mcpServers["test-server"].disabledForPromptTools).toBeDefined()
expect(writtenConfig.mcpServers["test-server"].disabledForPromptTools).toContain("new-tool")
expect(writtenConfig.mcpServers["test-server"].disabledTools).toBeDefined()
expect(writtenConfig.mcpServers["test-server"].disabledTools).toContain("new-tool")
})
})