Skip to content

Commit 5b86a53

Browse files
committed
bug #757 [MCP Bundle] Fix dependency injection and method-based / invokable support (camilleislasse)
This PR was squashed before being merged into the main branch. Discussion ---------- [MCP Bundle] Fix dependency injection and method-based / invokable support | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Docs? | yes | Issues | symfony/ai#755 | License | MIT ** [MCP Bundle] Fix dependency injection and method-based / invokable 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 Commits ------- bdd050c6 [MCP Bundle] Fix dependency injection and method-based / invokable support
2 parents 67f70a0 + 3a8d759 commit 5b86a53

File tree

3 files changed

+30
-2
lines changed

3 files changed

+30
-2
lines changed

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/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
);

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)