Skip to content

Commit bd87ca4

Browse files
author
camilleislasse
committed
[MCP Bundle] Fix dependency injection and method-based attributes support
This commit fixes critical issues preventing MCP tools with constructor dependencies from working and enables method-based attributes support. Changes: - Fix McpPass to use Reference objects for ServiceLocator registration Previously passed tag arrays directly, causing crashes when tools had constructor dependencies - Fix McpBundle::registerMcpAttributes() closure signature Added missing object $attribute and \Reflector $reflector parameters. Without these parameters, Symfony's AttributeAutoconfigurationPass only scans class-level attributes and skips method-level ones entirely - Add LoggerInterface to CurrentTimeTool to demonstrate DI works - Update documentation to reflect both patterns are supported Added examples showing invokable and method-based patterns both work with full DI support - Improve McpPassTest to verify ServiceLocator uses References Previous tests only checked presence, not type of values
1 parent 2c4d20d commit bd87ca4

File tree

5 files changed

+63
-2
lines changed

5 files changed

+63
-2
lines changed

demo/src/Mcp/Tools/CurrentTimeTool.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,18 @@
1212
namespace App\Mcp\Tools;
1313

1414
use Mcp\Capability\Attribute\McpTool;
15+
use Psr\Log\LoggerInterface;
1516

1617
/**
1718
* @author Tom Hart <[email protected]>
1819
*/
1920
class CurrentTimeTool
2021
{
22+
public function __construct(
23+
private readonly LoggerInterface $logger,
24+
) {
25+
}
26+
2127
/**
2228
* Returns the current time in UTC.
2329
*

docs/bundles/mcp-bundle.rst

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,33 @@ Dynamic resources with parameters:
113113

114114
All capabilities are automatically discovered in the ``src/`` directory when the server starts.
115115

116+
.. warning::
117+
118+
The MCP Bundle also ** supports the invokable pattern** where attributes are placed on classes
119+
with an ``__invoke()`` method.
120+
121+
** Invokable ** :
122+
123+
#[McpTool(name: 'my-tool')]
124+
class MyTool
125+
{
126+
public function __invoke(string $param): string
127+
{
128+
// Implementation
129+
}
130+
}
131+
132+
** Method Based ** :
133+
134+
class MyTools
135+
{
136+
#[McpTool(name: 'tool-one')]
137+
public function toolOne(): string { }
138+
139+
#[McpTool(name: 'tool-two')]
140+
public function toolTwo(): string { }
141+
}
142+
116143
Transport Types
117144
...............
118145

src/mcp-bundle/src/DependencyInjection/McpPass.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
1515
use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
1616
use Symfony\Component\DependencyInjection\ContainerBuilder;
17+
use Symfony\Component\DependencyInjection\Reference;
1718

1819
final class McpPass implements CompilerPassInterface
1920
{
@@ -35,7 +36,12 @@ public function process(ContainerBuilder $container): void
3536
return;
3637
}
3738

38-
$serviceLocatorRef = ServiceLocatorTagPass::register($container, $allMcpServices);
39+
$serviceReferences = [];
40+
foreach (array_keys($allMcpServices) as $serviceId) {
41+
$serviceReferences[$serviceId] = new Reference($serviceId);
42+
}
43+
44+
$serviceLocatorRef = ServiceLocatorTagPass::register($container, $serviceReferences);
3945

4046
$container->getDefinition('mcp.server.builder')
4147
->addMethodCall('setContainer', [$serviceLocatorRef]);

src/mcp-bundle/src/McpBundle.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ private function registerMcpAttributes(ContainerBuilder $builder): void
7575
foreach ($mcpAttributes as $attributeClass => $tag) {
7676
$builder->registerAttributeForAutoconfiguration(
7777
$attributeClass,
78-
static function (ChildDefinition $definition) use ($tag): void {
78+
static function (ChildDefinition $definition, object $attribute, \Reflector $reflector) use ($tag): void {
7979
$definition->addTag($tag);
8080
}
8181
);

src/mcp-bundle/tests/DependencyInjection/McpPassTest.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\AI\McpBundle\DependencyInjection\McpPass;
16+
use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
1617
use Symfony\Component\DependencyInjection\ContainerBuilder;
1718
use Symfony\Component\DependencyInjection\Definition;
19+
use Symfony\Component\DependencyInjection\Reference;
1820

1921
/**
2022
* @covers \Symfony\AI\McpBundle\DependencyInjection\McpPass
@@ -53,6 +55,18 @@ public function testCreatesServiceLocatorForAllMcpServices()
5355
$this->assertArrayHasKey('prompt_service', $services);
5456
$this->assertArrayHasKey('resource_service', $services);
5557
$this->assertArrayHasKey('template_service', $services);
58+
59+
// Verify services are ServiceClosureArguments wrapping References
60+
$this->assertInstanceOf(ServiceClosureArgument::class, $services['tool_service']);
61+
$this->assertInstanceOf(ServiceClosureArgument::class, $services['prompt_service']);
62+
$this->assertInstanceOf(ServiceClosureArgument::class, $services['resource_service']);
63+
$this->assertInstanceOf(ServiceClosureArgument::class, $services['template_service']);
64+
65+
// Verify the underlying values are References
66+
$this->assertInstanceOf(Reference::class, $services['tool_service']->getValues()[0]);
67+
$this->assertInstanceOf(Reference::class, $services['prompt_service']->getValues()[0]);
68+
$this->assertInstanceOf(Reference::class, $services['resource_service']->getValues()[0]);
69+
$this->assertInstanceOf(Reference::class, $services['template_service']->getValues()[0]);
5670
}
5771

5872
public function testDoesNothingWhenNoMcpServicesTagged()
@@ -115,5 +129,13 @@ public function testHandlesPartialMcpServices()
115129
$this->assertArrayHasKey('prompt_service', $services);
116130
$this->assertArrayNotHasKey('resource_service', $services);
117131
$this->assertArrayNotHasKey('template_service', $services);
132+
133+
// Verify services are ServiceClosureArguments wrapping References
134+
$this->assertInstanceOf(ServiceClosureArgument::class, $services['tool_service']);
135+
$this->assertInstanceOf(ServiceClosureArgument::class, $services['prompt_service']);
136+
137+
// Verify the underlying values are References
138+
$this->assertInstanceOf(Reference::class, $services['tool_service']->getValues()[0]);
139+
$this->assertInstanceOf(Reference::class, $services['prompt_service']->getValues()[0]);
118140
}
119141
}

0 commit comments

Comments
 (0)