Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds MCP (Model Context Protocol) variable support to ContextKit, enabling templates to call MCP tools directly using a new mcp() function. The changes integrate MCP client functionality into the template engine and add comprehensive test coverage for the new feature.
Key changes:
- Added
mcp()function to Jinja2 templates for calling MCP tools with automatic parameter collection - Refactored import paths from
util.mcp.configtomcp_client.configand added new MCP client modules - Enhanced test suite with E2E tests and mock MCP server for validation
Reviewed Changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_runner.py | Added mock implementations for MCP tool input collection to support E2E testing |
| tests/templates/spec2.md | New template demonstrating fully specified MCP tool calls |
| tests/templates/spec3.md | New template demonstrating partial MCP tool calls requiring user input |
| tests/mcp_test_server.py | Mock MCP server with test tools for E2E validation |
| tests/e2e/test_e2e.py | New E2E tests validating MCP functionality in templates |
| tests/README.md | Updated documentation with MCP usage examples |
| state.py | Updated import path for MCP configuration |
| pyproject.toml | Added MCP CLI dependency |
| prompt/init.py | Refactored to class-based design and added MCP tool input collection |
| mcp_client/ | New module containing MCP client implementation and configuration |
| engine/ | Enhanced template engine with MCP tool integration |
| commands/create_spec.py | Updated to use new prompt helper design and pass state to template engine |
| cxk.py | Added logging configuration and updated create-spec command signature |
| commands/mcp.py | Updated import path for MCP configuration |
| CLAUDE.md | Updated documentation reflecting new MCP capabilities |
| "weather": '{"condition": "sunny", "temp": "75F"}', | ||
| "username": "testuser", | ||
| "user": "test_user" | ||
| "user": "test_user", |
There was a problem hiding this comment.
[nitpick] The trailing comma is unnecessary when it's the last item in the dictionary. Consider removing it for consistency.
| "user": "test_user", | |
| "user": "test_user" |
tests/test_runner.py
Outdated
| asyncio.run(main()) No newline at end of file | ||
| # Patch both collect_var_value and collect_tool_input before running main | ||
| with patch("prompt.PromptHelper.collect_var_value", side_effect=mock_collect_var_value), \ | ||
| patch("prompt.PromptHelper.collect_tool_input", side_effect=mock_collect_tool_input): |
There was a problem hiding this comment.
[nitpick] The long patch statement could be more readable. Consider using parentheses instead of line continuation backslash.
| patch("prompt.PromptHelper.collect_tool_input", side_effect=mock_collect_tool_input): | |
| with ( | |
| patch("prompt.PromptHelper.collect_var_value", side_effect=mock_collect_var_value), | |
| patch("prompt.PromptHelper.collect_tool_input", side_effect=mock_collect_tool_input) | |
| ): |
| "--verbose", | ||
| "--var", | ||
| "ticket={\"id\":1}", | ||
| 'ticket={"id":1}', |
There was a problem hiding this comment.
[nitpick] This change from double quotes to single quotes around the JSON is inconsistent with the surrounding code style. Consider maintaining consistency with the original format.
| 'ticket={"id":1}', | |
| "ticket={\"id\":1}", |
No description provided.