MCP Demo #132
MCP Demo #132
Conversation
TomHart
commented
Jul 15, 2025
TomHart
commented
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | no |
| Docs? | no |
| Issues | Demo (not working) code for MCP |
| License | MIT |
|
@OskarStark Thanks for your feedback, I've addressed the comments and also fixed the code standards failure |
There was a problem hiding this comment.
Thanks @TomHart for working on this - that was def missing 👍
I left a few comments, mostly minor stuff - please make sure to remove the security bundle - didn't comment on every changed caused by that change
Thanks @chr-hertel. I've addressed the comments you left |
|
Looks good on the first glance - how did you test this? which the inspector? let's add information about that so users can check it out themselves? |
|
I used the CLI to paste in some JSON to list the tools. I've added the steps in the README. Is that what you were thinking, or something else? |
| Symfony\AI\McpSdk\Capability\Tool\ToolExecutorInterface: | ||
| class: Symfony\AI\McpSdk\Capability\ToolChain | ||
| arguments: | ||
| - ['@App\MCP\Tools\CurrentTimeTool'] |
There was a problem hiding this comment.
Off topic, but it would be nice if this is configurable via the bundle config
cc @Nyholm
There was a problem hiding this comment.
Can you pass a service reference in bundle config? E.g.
mcp:
tools:
- '@App\MCP\Tools\CurrentTimeTool'Would a tag + compiler pass perhaps be a cleaner way of doing this?
App\MCP\Tools\CurrentTimeTool:
tags:
- { name: mcp_tool_executor }
- { name: mcp_tool_metadata }or just name: mcp_tool
|
@TomHart please squash the commits to a single one |
Addressed comments lint Apply suggestion from @chr-hertel Co-authored-by: Christopher Hertel <[email protected]> revert .gitignore Addressed comments revert README for testing Update demo/README.md Co-authored-by: Oskar Stark <[email protected]> Update demo/README.md Co-authored-by: Oskar Stark <[email protected]> Update demo/README.md Co-authored-by: Christopher Hertel <[email protected]>
|
Done! Do you have any thoughts on #132 (comment)? I'd be happy to take a stab at implementing |
|
Thank you @TomHart. |
Hmm, on first thought, I think this would already be an improvement for now: mcp:
tools:
- '@App\MCP\Tools\CurrentTimeTool' |
|
@chr-hertel I couldn't get it to work referencing services manually but added a compiler pass. Happy to give the list another go if you'd prefer #168 |
