[MCP Bundle] Remove internal MCP SDK and use official mcp/sdk
#460
+569
−3,991
Conversation
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
mcp/sdk packagemcp/sdk package
mcp/sdk package3929cf2 to
9c05fa5
Compare
OskarStark
commented
Sep 5, 2025
| @@ -30,7 +30,7 @@ public function __construct( | |||
| protected function execute(InputInterface $input, OutputInterface $output): int | |||
| { | |||
| $this->server->connect( | |||
| new SymfonyConsoleTransport($input, $output) | |||
| new StdioTransport(STDIN, STDOUT) | |||
There was a problem hiding this comment.
needs to be checked, I propose to move the SymfonyConsoleTransport from the MCP SDK to our bundle and use it. Is this a solution @chr-hertel or is the change as is, fine?
mcp/sdk
chr-hertel
reviewed
Sep 6, 2025
CLAUDE.md
Outdated
|
|
||
| ### Integration Bundles | ||
| - **AI Bundle** (`src/ai-bundle/`): Symfony integration for Platform, Store, and Agent components | ||
| - **MCP Bundle** (`src/mcp-bundle/`): Symfony integration for MCP SDK | ||
| - **MCP Bundle** (`src/mcp-bundle/`): Symfony integration for external MCP SDK |
There was a problem hiding this comment.
Suggested change
| - **MCP Bundle** (`src/mcp-bundle/`): Symfony integration for external MCP SDK | |
| - **MCP Bundle** (`src/mcp-bundle/`): Symfony integration for official MCP SDK |
chr-hertel
reviewed
Sep 6, 2025
CLAUDE.md
Outdated
| @@ -92,7 +91,7 @@ symfony server:start | |||
| Components are designed to work independently but have these relationships: | |||
| - Agent depends on Platform for AI communication | |||
| - AI Bundle integrates Platform, Agent, and Store | |||
| - MCP Bundle provides MCP SDK integration | |||
| - MCP Bundle provides external MCP SDK integration | |||
There was a problem hiding this comment.
Suggested change
| - MCP Bundle provides external MCP SDK integration | |
| - MCP Bundle provides official MCP SDK integration |
chr-hertel
reviewed
Sep 6, 2025
README.md
Outdated
| * **Bundles** | ||
| * **[AI Bundle](src/ai-bundle/README.md)**: Symfony integration for AI Platform, Store and Agent components. | ||
| * **[MCP Bundle](src/mcp-bundle/README.md)**: Symfony integration for MCP SDK, allowing them to act as MCP servers or clients. | ||
| * **[MCP Bundle](src/mcp-bundle/README.md)**: Symfony integration for external [mcp/sdk](https://github.com/modelcontextprotocol/php-sdk), allowing them to act as MCP servers or clients. |
There was a problem hiding this comment.
Suggested change
| * **[MCP Bundle](src/mcp-bundle/README.md)**: Symfony integration for external [mcp/sdk](https://github.com/modelcontextprotocol/php-sdk), allowing them to act as MCP servers or clients. | |
| * **[MCP Bundle](src/mcp-bundle/README.md)**: Symfony integration for official [mcp/sdk](https://github.com/modelcontextprotocol/php-sdk), allowing them to act as MCP servers or clients. |
chr-hertel
reviewed
Sep 6, 2025
There was a problem hiding this comment.
Hmm, that doesn't work, does it?
chr-hertel
reviewed
Sep 6, 2025
src/mcp-bundle/MIGRATION_NOTES.md
Outdated
There was a problem hiding this comment.
is this claude? than let's remove it.
i don't think that's helpful in the repo - for the half of it that is correct we could create issues, but yea ... could do that later as well
chr-hertel
reviewed
Sep 6, 2025
src/mcp-bundle/README.md
Outdated
| @@ -1,7 +1,7 @@ | |||
| # MCP Bundle | |||
|
|
|||
| Symfony integration bundle for [Model Context Protocol](https://modelcontextprotocol.io/) using the Symfony AI | |||
| MCP SDK [symfony/mcp-sdk](https://github.com/symfony/mcp-sdk). | |||
| Symfony integration bundle for [Model Context Protocol](https://modelcontextprotocol.io/) using the external | |||
There was a problem hiding this comment.
Suggested change
| Symfony integration bundle for [Model Context Protocol](https://modelcontextprotocol.io/) using the external | |
| Symfony integration bundle for [Model Context Protocol](https://modelcontextprotocol.io/) using the official |
chr-hertel
reviewed
Sep 6, 2025
mcp/sdkmcp/sdk
- Remove src/mcp-sdk/ directory entirely - Update MCP Bundle to use external mcp/sdk package instead of internal SDK - Update all namespace imports from Symfony\AI\McpSdk to Mcp - Update composer.json dependencies in MCP Bundle - Update documentation references - Update demo application tool to use new SDK - Add migration notes documenting API changes and current status The external mcp/sdk is still in development with significant API changes. Further work is needed to fully adapt the bundle architecture to the new SDK structure.
- Create SymfonyRegistry to bridge Symfony services with new MCP Registry system - Add McpRegistryCompilerPass to automatically register tagged tools - Update service definitions to use new SDK architecture with Handler::make() factory - Fix StdioTransport to use raw resources (STDIN/STDOUT) instead of Console I/O - Update CurrentTimeTool demo to implement new ToolExecutorInterface with CallToolRequest/CallToolResult - Update tests to reflect new architecture without separate handler services - Remove obsolete handler interface autoconfigurations All tests passing. The bundle now works with external mcp/sdk package.
Mark the migration as complete with all functionality working. Document the new components and successful test status.
Add Oskar Stark as author for newly created classes instead of generic 'Assistant' placeholder.
Since we're fully migrating to external mcp/sdk, no backward compatibility is needed. Simplify SymfonyRegistry to require ToolExecutorInterface implementation.
Add section covering: - Potential enhancements (attribute-based discovery, docs, tests, performance) - Additional capabilities (prompts, resources, discovery integration) - Clear breaking changes from previous version - Production-ready status with optional future improvements
Remove all completed migration details and only keep: - Future improvement opportunities - Breaking changes from previous version The migration is complete, this now serves as a roadmap for future enhancements.
- Add discovery configuration options (enabled, directories, exclude) - Create SymfonyRegistryFactory for configurable registry creation - Integrate with external SDK's Discoverer for attribute-based tool discovery - Registry now automatically discovers tools with #[McpTool] attributes - All tests passing with new discovery functionality
- Update terminology in README.md, CLAUDE.md, and src/mcp-bundle/README.md - Remove MIGRATION_NOTES.md as requested by @chr-hertel - Clean up formatting in CurrentTimeTool.php - Verified CurrentTimeTool implementation works correctly with new SDK
- Add newlines to README.md, mcp-bundle/README.md, CurrentTimeTool.php, and InvalidArgumentException.php - Follows project coding standards requirement for file endings
aaa7619 to
52bc4d2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follows Symfony to Provide the Official MCP SDK
Todos
mcp-sdkpackage from packagistmcp-sdkanymore