-
-
Notifications
You must be signed in to change notification settings - Fork 103
Open
Labels
BugSomething isn't workingSomething isn't workingMCP BundleIssues & PRs about the MCP SDK integration bundleIssues & PRs about the MCP SDK integration bundleStatus: Needs Review
Description
Description
Two issues with MCP tool dependency injection:
- Design decision needed: Bundle currently only supports invokable pattern, but SDK supports both patterns (invokable + methods).
- Bug:
McpPass::process()
passes tag arrays instead of References toServiceLocatorTagPass
Current Situation
MCP SDK supports two patterns:
- Invokable:
#[McpTool]
on class +__invoke()
method - Methods:
#[McpTool]
on public methods
MCP Bundle currently:
- Uses
registerAttributeForAutoconfiguration()
- Only detects class-level attributes (invokable pattern)
- Method pattern not supported with auto-configuration
- Tools with dependencies crash due to ServiceLocator bug
Pattern Support Decision
Option 1: Keep invokable only
Uses registerAttributeForAutoconfiguration()
which only detects class-level attributes. Users need to use #[McpTool]
on class with __invoke()
method.
Option 2: Support both patterns
Find a way to detect and auto-tag methods with #[McpTool]
attributes. Would provide full compatibility with SDK examples and support multiple tools per class.
ServiceLocator Bug (must fix regardless)
Current code in McpPass::process()
(line 36):
public function process(ContainerBuilder $container): void
{
// ... collect services ...
$allMcpServices = [
'App\MCP\Tools\WeatherTool' => [['priority' => 0]], // Tag metadata array
// ...
];
// BUG: Passes tag arrays instead of References
$serviceLocatorRef = ServiceLocatorTagPass::register($container, $allMcpServices);
}
What ServiceLocatorTagPass::register()
expects:
[
'App\MCP\Tools\WeatherTool' => new Reference('App\MCP\Tools\WeatherTool'),
// ...
]
Fix (required for invokable pattern to work):
// src/mcp-bundle/src/DependencyInjection/McpPass.php
use Symfony\Component\DependencyInjection\Reference; // Add import
public function process(ContainerBuilder $container): void
{
if (!$container->hasDefinition('mcp.server.builder')) {
return;
}
$allMcpServices = [];
$mcpTags = ['mcp.tool', 'mcp.prompt', 'mcp.resource', 'mcp.resource_template'];
foreach ($mcpTags as $tag) {
$taggedServices = $container->findTaggedServiceIds($tag);
$allMcpServices = array_merge($allMcpServices, $taggedServices);
}
if ([] === $allMcpServices) {
return;
}
// Transform service IDs to References
$serviceReferences = [];
foreach (array_keys($allMcpServices) as $serviceId) {
$serviceReferences[$serviceId] = new Reference($serviceId);
}
$serviceLocatorRef = ServiceLocatorTagPass::register($container, $serviceReferences);
$container->getDefinition('mcp.server.builder')
->addMethodCall('setContainer', [$serviceLocatorRef]);
}
Example: Method Pattern Not Supported
// This does NOT work with current Bundle
namespace App\MCP\Tools;
use App\Service\WeatherService;
use Mcp\Capability\Attribute\McpTool;
class WeatherTool
{
public function __construct(
private readonly WeatherService $weatherService,
) {
}
#[McpTool(name: 'get-weather')]
public function getWeather(string $city): string
{
return $this->weatherService->getCurrentWeather($city);
}
}
Result:
$ php bin/console debug:container --tag=mcp.tool
No tags found that match "mcp.tool"
Tool is NOT auto-tagged.
Example: Invokable Pattern (Works)
// This DOES work with current Bundle
namespace App\MCP\Tools;
use App\Service\WeatherService;
use Mcp\Capability\Attribute\McpTool;
#[McpTool(name: 'get-weather')]
class WeatherTool
{
public function __construct(
private readonly WeatherService $weatherService,
) {
}
public function __invoke(string $city): string
{
$weather = $this->weatherService->getCurrentWeather($city);
return sprintf('Weather in %s: %s, %d°C',
$weather['city'],
$weather['condition'],
$weather['temp']
);
}
}
Result (after ServiceLocator fix):
$ php bin/console debug:container --tag=mcp.tool
App\MCP\Tools\WeatherTool
$ php bin/console debug:container WeatherTool
Arguments: Service(App\Service\WeatherService)
Tool IS auto-tagged, DI works.
Error Without ServiceLocator Fix
Even with correct invokable pattern:
ERROR [mcp] Tool execution failed
"exception" => "Failed to resolve dependency 'App\Service\WeatherService' for 'App\MCP\Tools\WeatherTool'
parameter $weatherService: Failed to resolve dependency 'Psr\Log\LoggerInterface' for 'App\Service\WeatherService'
parameter $logger: Class 'Psr\Log\LoggerInterface' is not instantiable"
Action Items
Must do:
- Fix
McpPass::process()
to use References instead of tag arrays
Decide:
- Keep invokable-only (document limitation) OR
- Implement method pattern support
Environment
- MCP Bundle version:
dev-main
- Symfony version: 7.3+
- PHP version: 8.4
Additional Notes
- This affects all MCP capabilities:
#[McpTool]
,#[McpPrompt]
,#[McpResource]
,#[McpResourceTemplate]
- Tools with zero dependencies work by accident (SDK can instantiate with
new $className()
) - Tools requiring any service dependency crash with current code
Metadata
Metadata
Assignees
Labels
BugSomething isn't workingSomething isn't workingMCP BundleIssues & PRs about the MCP SDK integration bundleIssues & PRs about the MCP SDK integration bundleStatus: Needs Review