feat: support custom tools via Model Context Protocol (MCP)#60
Merged
Conversation
Introduce MCP support in Cassandra, allowing the reviewer to utilize custom tools during the review process. This includes: - A new CLI flag '--mcp-config' to provide an 'mcp.json' configuration file. - Support for local 'stdio' servers and remote 'sse' (HTTP) servers. - Automatic environment variable expansion (os.ExpandEnv) for all configuration values. - Discovery and registration of tools from MCP servers. - Integration into the GitHub Action with a new 'mcp_config' input. - Detailed documentation and usage examples in README.md. - Unit tests for configuration expansion and tool registration.
|
This is an exceptionally well-executed revision. The extraction of main logic into run(ctx) robustly resolves the unexecuted deferred calls issue. Correctly cloning the http.Request in the RoundTripper, applying a strict timeout, capturing error outputs, and expanding test coverage significantly improve stability and reliability. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| res, err := session.CallTool(ctx, callParams) | ||
| if err != nil { | ||
| return "", fmt.Errorf("MCP tool call failed: %w", err) |
Owner
Author
There was a problem hiding this comment.
MCP configuration should have timeout (in seconds) configuration (with 30 seconds default).
menny
commented
Apr 21, 2026
|
|
||
| res, err := session.CallTool(ctx, callParams) | ||
| if err != nil { | ||
| return "", fmt.Errorf("MCP tool call failed: %w", err) |
Owner
Author
There was a problem hiding this comment.
MCP configuration should have timeout (in seconds) configuration (with 30 seconds default).
menny
commented
Apr 21, 2026
- Fix http.RoundTripper contract violation by cloning the request in headerRoundTripper. - Prevent indefinite blocking by adding a configurable timeout (default 30s) to MCP tool calls. - Improve error visibility by returning the MCP tool's error content to the model for self-correction. - Added TimeoutSeconds to ServerConfig in mcp.json schema.
Owner
Author
|
I've addressed the resilience and safety issues identified in the review:
Verified with |
… guidelines - Use exec.CommandContext for MCP subprocesses to ensure they are reaped on context cancellation. - Prevent goroutine leaks in tests by using cancelable contexts for mock servers. - Use t.Setenv in tests for isolated and safe environment variable setup. - Update RegisterServers to return errors if no servers could be registered, making the error check in main reachable. - Fix unsafe exit in main.go by explicitly closing the MCP manager before os.Exit on failure. - Document the map[string]any exception for MCP tool arguments in client.go and AGENTS.md. - Add goroutine leak prevention guideline to AGENTS.md.
Owner
Author
|
I've addressed the second round of review feedback:
Verified with |
Introduce support for the Model Context Protocol (MCP) to allow the AI reviewer to utilize custom external tools. This includes: - A new 'tools/mcp' package supporting stdio and SSE transports. - Integration into the main review loop via the --mcp-config flag. - Detailed configuration support with environment variable expansion. Refactor the tool registry and core agent to support context forwarding. This ensures that all tool executions, including local git operations and remote MCP calls, respect application-level timeouts and cancellation signals by using exec.CommandContext and proper context chaining. Fixes several PR feedback items: - SSE transport now correctly follows http.RoundTripper contracts. - Improved error reporting for MCP server initialization failures. - Ensured subprocesses are reaped on context cancellation.
- Extract registerServerWithTransport to enable testing of the production MCP tool handler. - Update tools/mcp/client_test.go to use the production handler and verify error handling. - Add TestAgent_ExecuteToolCalls_ContextPropagation to core/agent_test.go to confirm that canceled contexts are correctly forwarded to tool dispatchers.
Refactor cmd/ai_reviewer/main.go to move its core logic into a run() function. This ensures that all deferred statements, such as mcpManager.Close(), are executed before the process terminates via os.Exit. This prevents orphan MCP subprocess issues on fatal errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces support for the Model Context Protocol (MCP) in Cassandra, allowing the reviewer to connect to custom local or remote tools to extend its capabilities.
Features
--mcp-configto provide anmcp.jsonconfiguration file.stdioservers (subprocesses) and remotesse(HTTP) servers.os.ExpandEnv) for all configuration values (commands, URLs, headers).mcp_configinput.README.md.Fixes #43