Skip to content

Conversation

@qdaxb
Copy link
Contributor

@qdaxb qdaxb commented Jun 20, 2025

Related GitHub Issue

Discussion: #639
Closes: #5346

Description

Added whitelist and blacklist lists. When sending an MCP prompt, the system will read the MCP server list configured for the current mode, add or filter MCP servers based on the whitelist/blacklist, and then assemble the prompt. If both lists are empty, all MCP servers will be used.

Test Procedure

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • [ x] ✨ New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (npm run lint).
    • All debug code (e.g., console.log) has been removed.
  • Testing:
    • New and/or updated tests have been added to cover my changes.
    • All tests pass locally (npm test).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

image
image

Documentation Updates

Additional Notes

Get in Touch


Important

Adds support for specifying available/unavailable MCP servers in custom mode, with UI and logic updates for server selection.

  • Behavior:
    • Adds whitelist and blacklist for MCP servers in custom mode, affecting prompt assembly.
    • If both lists are empty, all MCP servers are used.
  • UI Components:
    • Adds McpSelector component in McpSelector.tsx for selecting MCP servers.
    • Updates ModesView.tsx to integrate McpSelector for custom modes.
  • Logic:
    • Modifies getMcpServersSection() in mcp-servers.ts to filter servers based on whitelist/blacklist.
    • Implements memoization for server filtering in mcp-servers.ts.
  • Tests:
    • Updates tests in add-custom-instructions.spec.ts and system-prompt.spec.ts to reflect new server selection logic.
  • Localization:
    • Updates localization files for new MCP server selection strings.

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

@qdaxb qdaxb requested review from cte, jr and mrubens as code owners June 20, 2025 03:25
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. enhancement New feature or request labels Jun 20, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jun 20, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 20, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jun 20, 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.

Thanks for working on this! I've left some inline comments on specific implementation details.

The main thing that stands out is that having both allowed and denied lists seems unnecessarily complex. A single exclusion list would be simpler - just let users uncheck the MCP servers they don't want for that mode. This would avoid the confusing precedence logic and make the UI more straightforward.

The i18n support looks good and the search functionality is a nice touch. With the simplification to a single list, this would be a solid addition to the custom modes feature.

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jun 22, 2025
@qdaxb
Copy link
Contributor Author

qdaxb commented Jun 23, 2025

Thanks for working on this! I've left some inline comments on specific implementation details.

The main thing that stands out is that having both allowed and denied lists seems unnecessarily complex. A single exclusion list would be simpler - just let users uncheck the MCP servers they don't want for that mode. This would avoid the confusing precedence logic and make the UI more straightforward.

The i18n support looks good and the search functionality is a nice touch. With the simplification to a single list, this would be a solid addition to the custom modes feature.

thanks for reviewing @daniel-lxs

The variable and translation names I used for the two lists when encoding might be a bit ambiguous. My bad.I would like to reiterate the original intention of the design

The current implementation considers two scenarios:
A certain mode requires a certain mcp
A certain mcp can be blocked in certain scenarios

In my opinion, if only one list must be kept, it should be the "require list" rather than the "block list".

mcp tools should be used as a means of "achieving goals" like system tools:

(an agent that can achieve certain requirements) = prompt + tools = prompt + (system tools + mcp tools)

On this basis, the custom mode in the marketplace can be generalized into a "custom agent": when users install the mode, in addition to adding a custom prompt, they will also install the dependent mcp server, which can better expand the capabilities of the custom mode.

Therefore, if we think the current dual list is too complicated, perhaps we can just keep the require list, or make some adjustments to the UI (such as using tab to switch between require list/blocklist)?

@qdaxb qdaxb force-pushed the select_mcp_server_in_custom_mode branch 4 times, most recently from db38346 to 23ecea6 Compare June 24, 2025 08:13
@qdaxb
Copy link
Contributor Author

qdaxb commented Jun 24, 2025

update new version:

  • leave only one list(required list)
  • code refactor
  • UI fix

image

@qdaxb
Copy link
Contributor Author

qdaxb commented Jul 2, 2025

@daniel-lxs Another look? Thanks!

@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Jul 2, 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.

Thank you @qdaxb

I left a couple of suggestions, let me know what you think!

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jul 2, 2025
@jmolinski
Copy link

It'd be absolutely fantastic to have this feature merged. @qdaxb are you still interested in working on this?

@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Aug 5, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Draft / In Progress] in Roo Code Roadmap Aug 5, 2025
@daniel-lxs daniel-lxs marked this pull request as draft August 5, 2025 21:32
@daniel-lxs daniel-lxs force-pushed the select_mcp_server_in_custom_mode branch from 895f5ca to 1aa8274 Compare August 5, 2025 21:59
- Add status: 'connected' to mock MCP server
- Add proper config object with transport details
- Update test snapshots to reflect connected MCP server state
- Fixes failing tests after MCP server filtering implementation
@daniel-lxs daniel-lxs marked this pull request as ready for review August 5, 2025 22:48
@daniel-lxs daniel-lxs moved this from PR [Draft / In Progress] to PR [Needs Review] in Roo Code Roadmap Aug 5, 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.

Thank you @qdaxb and sorry for the delay!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 5, 2025
@mrubens
Copy link
Collaborator

mrubens commented Aug 6, 2025

I think there might be an issue with the yaml generation. By checking and unchecking boxes a few times I got my yaml to look like this:

    groups:
      - read
      - edit
      - command
      - - mcp
        - mcp:
            included:
              - github
              - context7

What is the yaml supposed to look like for this?

- Updated schema to accept { mcp: { included: [...] } } directly as a GroupEntry
- Modified McpSelector to create the direct object format instead of tuple
- Updated getGroupName and getGroupOptions helpers to handle the new format
- Updated MCP server section to properly extract included list from both formats

This resolves the nested array issue in YAML generation when toggling MCP
checkbox and selecting specific servers. The YAML now correctly generates:
  - mcp:
      included: [...]
Instead of the previous nested structure.
- Fixed useEffect to properly detect MCP object format { mcp: { included: [...] } }
- Fixed empty selection behavior to enable all MCP servers instead of disabling MCP
- Removed tuple format handling since this feature hasn't been released yet
- Simplified code to only handle the new object format for MCP configuration
- Removed unused GroupOptions import

When no specific servers are selected, MCP is added as a string (enables all servers).
When specific servers are selected, MCP is added as an object with the included list.

This ensures the MCP selector properly loads configurations and maintains
the expected behavior where empty selection means all servers are enabled.
@Evyatar108
Copy link

hey is this planned on being merged?

@daniel-lxs daniel-lxs moved this from PR [Needs Review] to PR [Needs Prelim Review] in Roo Code Roadmap Sep 16, 2025
@daniel-lxs
Copy link
Member

Proposing to close this PR and follow up with a version that stabilizes the schema. Two blocking issues remain:

  1. Canonical YAML shape is unstable
  • Current schema accepts both tuple and object forms, which leads to outputs like:
groups:
  - read
  - edit
  - command
  - - mcp
    - mcp:
        included:
          - github
          - context7
  • Action: choose one canonical representation for serialization and always emit that form (parser can remain lenient for backward compatibility). Prefer the plain object form:
- mcp:
    included:
      - github
      - context7

Touchpoint: packages/types/src/mode.ts (groupEntrySchema)
https://github.com/RooCodeInc/Roo-Code/blob/1f4d3c3e7741c6787b06e5482c24cd52d8122477/packages/types/src/mode.ts

  1. Empty "included" semantics must be explicit and enforced end-to-end

Once these are addressed with a stable, canonical output and explicit empty semantics, we can proceed on a cleaner foundation.

@daniel-lxs daniel-lxs closed this Sep 22, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Sep 22, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 22, 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 - Needs Preliminary Review size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

MCP Server Restrictions Per Mode

6 participants