Skip to content

Conversation

@mrubens
Copy link
Collaborator

@mrubens mrubens commented Jun 22, 2025

A new allowedMcpServers option was added to the mode configuration, enabling restrictions on MCP server access.

  • The groupOptionsSchema in packages/types/src/mode.ts was updated to include an optional allowedMcpServers array of strings.
  • A new McpServerRestrictionError class was introduced in src/shared/modes.ts to provide specific error messages when restrictions are violated.
  • The isToolAllowedForMode function in src/shared/modes.ts was modified to enforce these restrictions for use_mcp_tool and access_mcp_resource calls.
    • If a server_name is provided and not in the allowed list, an McpServerRestrictionError is thrown.
    • Requests without a server_name (e.g., partial streaming requests) are permitted.
  • Comprehensive tests were added to src/shared/__tests__/modes.spec.ts to validate the restriction logic, covering permitted/rejected servers, unrestricted modes, and error messages.
  • Schema validation tests were added to src/core/config/__tests__/ModeConfig.spec.ts to ensure the allowedMcpServers field is correctly parsed and validated.

This change provides granular control over which MCP servers a mode can interact with.


Important

Adds allowedMcpServers option to restrict MCP server access in modes, with error handling and comprehensive tests.

  • Behavior:
    • Adds allowedMcpServers option to groupOptionsSchema in mode.ts for restricting MCP server access.
    • Updates isToolAllowedForMode in modes.ts to enforce MCP server restrictions, throwing McpServerRestrictionError for disallowed servers.
    • Allows requests without server_name (e.g., partial streaming requests).
  • Error Handling:
    • Introduces McpServerRestrictionError in modes.ts for specific error messaging when MCP server restrictions are violated.
  • Testing:
    • Adds tests in modes.spec.ts to validate MCP server restriction logic, including allowed/rejected servers and error messages.
    • Adds schema validation tests in ModeConfig.spec.ts for allowedMcpServers field parsing and validation.

This description was created by Ellipsis for cecacba. You can customize this summary. It will automatically update as commits are pushed.

@mrubens mrubens requested review from cte and jr as code owners June 22, 2025 21:41
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Jun 22, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jun 22, 2025
@daniel-lxs
Copy link
Member

This PR is also trying to achieve this #4917, although I requested some changes since having two separate list seems redundant.

But so far I think this cleaner approach might be better.

@daniel-lxs daniel-lxs moved this from Triage to PR [Draft / In Progress] in Roo Code Roadmap Jun 22, 2025
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 22, 2025
@daniel-lxs daniel-lxs moved this from PR [Draft / In Progress] to PR [Needs Prelim Review] in Roo Code Roadmap Jun 22, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Jun 22, 2025
@hannesrudolph hannesrudolph added PR - Needs Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jun 22, 2025
@mrubens
Copy link
Collaborator Author

mrubens commented Jun 23, 2025

This PR is also trying to achieve this #4917, although I requested some changes since having two separate list seems redundant.

But so far I think this cleaner approach might be better.

Oh I didn't see that one! I agree that the two separate lists is probably a little much. One nice thing in the other PR though is that it filters the other MCP servers out of the prompt.

@mrubens
Copy link
Collaborator Author

mrubens commented Jun 25, 2025

@cursoragent can you also make sure that excluded MCP servers don't show up in the system prompt?

@daniel-lxs daniel-lxs moved this from PR [Needs Review] to PR [Draft / In Progress] in Roo Code Roadmap Jun 30, 2025
@Evyatar108
Copy link

Evyatar108 commented Jul 3, 2025

Hey I've also been working on a comprehensive implementation of a similar functionality in my PR #5364 that includes additional features and UI.

Would be great if we combine forces on this effort

@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 7, 2025
@github-project-automation github-project-automation bot moved this from PR [Draft / In Progress] to Done in Roo Code Roadmap Jul 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer PR - Draft / In Progress size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants