config mcp server in config file - #761
Conversation
WalkthroughThe changes introduce support for configuring MCP servers directly in the global Holmes configuration file, update the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HolmesConfig
participant ToolsetManager
User->>HolmesConfig: Load config (may include mcp_servers)
HolmesConfig->>ToolsetManager: Instantiate with mcp_servers, toolsets, etc.
ToolsetManager->>ToolsetManager: Merge mcp_servers into toolsets (type="MCP")
User->>ToolsetManager: Request toolsets
ToolsetManager-->>User: Return toolsets including MCP servers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
holmes/core/toolset_manager.py (2)
35-39: Fix redundant assignment and approve MCP server integration logic.The MCP server processing logic correctly sets the type to string values and merges them into the toolsets dictionary. However, there's a redundant assignment to
self.toolsetson line 35.- self.toolsets = toolsets self.toolsets = toolsets or {}
444-444: Remove duplicate assignment.There's a duplicate assignment on line 444 that should be removed.
existing_toolsets_by_name[new_toolset.name] = new_toolset - existing_toolsets_by_name[new_toolset.name] = new_toolset
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/data-sources/remote-mcp-servers.md(1 hunks)holmes/config.py(2 hunks)holmes/core/toolset_manager.py(3 hunks)holmes/plugins/toolsets/__init__.py(2 hunks)tests/core/test_toolset_manager.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: Use Ruff for formatting and linting (configured in pyproject.toml)
Type hints required (mypy configuration in pyproject.toml)
Files:
holmes/config.pytests/core/test_toolset_manager.pyholmes/core/toolset_manager.pyholmes/plugins/toolsets/__init__.py
tests/**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
tests/**/*.py: All new features require unit tests
New toolsets require integration tests with mocks
Tests: Match source structure under tests/
Files:
tests/core/test_toolset_manager.py
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Toolsets: holmes/plugins/toolsets/{name}.yaml or {name}/
docs/data-sources/remote-mcp-servers.md (2)
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Toolsets: holmes/plugins/toolsets/{name}.yaml or {name}/
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Bash toolset validates commands for safety
tests/core/test_toolset_manager.py (2)
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to tests/**/*.py : New toolsets require integration tests with mocks
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to tests/**/*.py : All new features require unit tests
holmes/core/toolset_manager.py (1)
Learnt from: nherment
PR: #535
File: holmes/plugins/toolsets/bash/bash_toolset.py:207-209
Timestamp: 2025-06-24T05:51:04.543Z
Learning: The init_config method in toolsets should be idempotent - safely callable multiple times without errors. self.config should maintain consistent typing (not alternate between dict and config object types) throughout the object lifecycle.
holmes/plugins/toolsets/__init__.py (5)
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Toolsets: holmes/plugins/toolsets/{name}.yaml or {name}/
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Bash toolset validates commands for safety
Learnt from: nherment
PR: #610
File: .github/workflows/llm-evaluation.yaml:39-42
Timestamp: 2025-07-08T08:45:41.069Z
Learning: The robusta-dev/holmesgpt codebase has comprehensive existing validation for Azure environment variables (AZURE_API_BASE, AZURE_API_KEY, AZURE_API_VERSION) and MODEL in tests/llm/utils/classifiers.py, tests/llm/conftest.py, and holmes/core/llm.py. Don't suggest adding redundant validation logic.
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to holmes/plugins/prompts/**/*.jinja2 : Prompts: holmes/plugins/prompts/{name}.jinja2
Learnt from: nherment
PR: #535
File: holmes/plugins/toolsets/bash/bash_toolset.py:207-209
Timestamp: 2025-06-24T05:51:04.543Z
Learning: The init_config method in toolsets should be idempotent - safely callable multiple times without errors. self.config should maintain consistent typing (not alternate between dict and config object types) throughout the object lifecycle.
🧬 Code Graph Analysis (1)
holmes/core/toolset_manager.py (1)
holmes/core/tools.py (1)
ToolsetType(114-117)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Pre-commit checks
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
🔇 Additional comments (10)
docs/data-sources/remote-mcp-servers.md (1)
170-174: LGTM! Clear documentation of the new MCP server configuration option.The addition effectively demonstrates the new capability to configure MCP servers directly in the global Holmes configuration file, which simplifies the user experience by eliminating the need for custom toolset files in CLI commands.
holmes/config.py (2)
118-118: LGTM! Proper integration of MCP server configuration.The new
mcp_serversattribute follows the established pattern with appropriate typing and default initialization.
129-129: LGTM! Correct parameter passing to ToolsetManager.The
mcp_serversattribute is properly passed to the ToolsetManager constructor, enabling the integration of MCP servers as toolsets.holmes/plugins/toolsets/__init__.py (2)
10-31: LGTM! Import reorganization improves code structure.The imports are now better organized and grouped logically, making the code more readable and maintainable.
157-157: LGTM! Critical fix for MCP toolset type comparison.The change from identity (
is) to equality (==) comparison is essential for correct MCP toolset recognition, as the system now uses string values (ToolsetType.MCP.value) rather than enum instances for type checking.tests/core/test_toolset_manager.py (2)
307-324: LGTM! Comprehensive test for MCP servers from custom toolset files.The test properly validates MCP server loading from YAML configuration files, including correct toolset creation, naming, and type assignment. The use of temporary files and proper test data structure follows established testing patterns.
327-344: LGTM! Proper test coverage for direct MCP server configuration.The test effectively validates MCP server loading from direct configuration dictionaries, ensuring the ToolsetManager correctly processes MCP servers passed via the constructor. The assertions verify both toolset presence and correct type assignment.
holmes/core/toolset_manager.py (3)
29-29: LGTM! Proper constructor extension for MCP server support.The new
mcp_serversparameter is correctly typed and integrated into the constructor signature, enabling MCP server configuration support.
264-264: LGTM! Correct enum instantiation from cached status.The use of
.valueensures proper enum instantiation from string values stored in the cache, maintaining consistency with the string-based type representation.
364-364: LGTM! Consistent MCP server type assignment in YAML loading.The assignment of
ToolsetType.MCP.valuemaintains consistency with the string-based type representation used throughout the system for MCP servers.
2 participants
For the example, after setting up the mcp server configuration as follows
holmes gives