Skip to content

Commit 3cbcb28

Browse files
X2NXchr-hertel
X2NX
authored and
chr-hertel
committed
fix: because the cached discoveryState does not use ->setDiscoveryState(), it is found that the discoveryState cannot be used
1 parent e5a4cb5 commit 3cbcb28

File tree

4 files changed

+90
-119
lines changed

4 files changed

+90
-119
lines changed

‎src/Capability/Discovery/Discoverer.php‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
use Mcp\Capability\Completion\ListCompletionProvider;
2121
use Mcp\Capability\Completion\ProviderInterface;
2222
use Mcp\Capability\Registry\PromptReference;
23-
use Mcp\Capability\Registry\ReferenceRegistryInterface;
2423
use Mcp\Capability\Registry\ResourceReference;
2524
use Mcp\Capability\Registry\ResourceTemplateReference;
2625
use Mcp\Capability\Registry\ToolReference;
@@ -48,7 +47,6 @@
4847
class Discoverer
4948
{
5049
public function __construct(
51-
private readonly ReferenceRegistryInterface $registry,
5250
private readonly LoggerInterface $logger = new NullLogger(),
5351
private ?DocBlockParser $docBlockParser = null,
5452
private ?SchemaGenerator $schemaGenerator = null,
@@ -95,10 +93,7 @@ public function discover(string $basePath, array $directories, array $excludeDir
9593
'base_path' => $basePath,
9694
]);
9795

98-
$emptyState = new DiscoveryState();
99-
$this->registry->setDiscoveryState($emptyState);
100-
101-
return $emptyState;
96+
return new DiscoveryState();
10297
}
10398

10499
$finder->files()
@@ -125,11 +120,7 @@ public function discover(string $basePath, array $directories, array $excludeDir
125120
'resourceTemplates' => $discoveredCount['resourceTemplates'],
126121
]);
127122

128-
$discoveryState = new DiscoveryState($tools, $resources, $prompts, $resourceTemplates);
129-
130-
$this->registry->setDiscoveryState($discoveryState);
131-
132-
return $discoveryState;
123+
return new DiscoveryState($tools, $resources, $prompts, $resourceTemplates);
133124
}
134125

135126
/**

‎src/Capability/Registry/Loader/DiscoveryLoader.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,14 @@ public function __construct(
3838
public function load(ReferenceRegistryInterface $registry): void
3939
{
4040
// This now encapsulates the discovery process
41-
$discoverer = new Discoverer($registry, $this->logger);
41+
$discoverer = new Discoverer($this->logger);
4242

4343
$cachedDiscoverer = $this->cache
4444
? new CachedDiscoverer($discoverer, $this->cache, $this->logger)
4545
: $discoverer;
4646

47-
$cachedDiscoverer->discover($this->basePath, $this->scanDirs, $this->excludeDirs);
47+
$discoveryState = $cachedDiscoverer->discover($this->basePath, $this->scanDirs, $this->excludeDirs);
48+
49+
$registry->setDiscoveryState($discoveryState);
4850
}
4951
}

‎tests/Unit/Capability/Discovery/CachedDiscovererTest.php‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ class CachedDiscovererTest extends TestCase
2323
{
2424
public function testCachedDiscovererUsesCacheOnSecondCall(): void
2525
{
26-
$registry = new Registry(null, new NullLogger());
27-
$discoverer = new Discoverer($registry, new NullLogger());
26+
$discoverer = new Discoverer();
2827

2928
$cache = $this->createMock(CacheInterface::class);
3029
$cache->expects($this->once())
@@ -47,8 +46,7 @@ public function testCachedDiscovererUsesCacheOnSecondCall(): void
4746

4847
public function testCachedDiscovererReturnsCachedResults(): void
4948
{
50-
$registry = new Registry(null, new NullLogger());
51-
$discoverer = new Discoverer($registry, new NullLogger());
49+
$discoverer = new Discoverer();
5250

5351
$cache = $this->createMock(CacheInterface::class);
5452
$cachedState = new DiscoveryState();
@@ -71,8 +69,7 @@ public function testCachedDiscovererReturnsCachedResults(): void
7169

7270
public function testCacheKeyGeneration(): void
7371
{
74-
$registry = new Registry(null, new NullLogger());
75-
$discoverer = new Discoverer($registry, new NullLogger());
72+
$discoverer = new Discoverer();
7673

7774
$cache = $this->createMock(CacheInterface::class);
7875

‎tests/Unit/Capability/Discovery/DiscoveryTest.php‎

Lines changed: 81 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@
1515
use Mcp\Capability\Completion\ListCompletionProvider;
1616
use Mcp\Capability\Discovery\Discoverer;
1717
use Mcp\Capability\Registry;
18-
use Mcp\Capability\Registry\PromptReference;
19-
use Mcp\Capability\Registry\ResourceReference;
20-
use Mcp\Capability\Registry\ResourceTemplateReference;
2118
use Mcp\Capability\Registry\ToolReference;
2219
use Mcp\Tests\Unit\Capability\Attribute\CompletionProviderFixture;
2320
use Mcp\Tests\Unit\Capability\Discovery\Fixtures\DiscoverableToolHandler;
@@ -29,155 +26,139 @@
2926

3027
class DiscoveryTest extends TestCase
3128
{
32-
private Registry $registry;
3329
private Discoverer $discoverer;
3430

3531
protected function setUp(): void
3632
{
37-
$this->registry = new Registry();
38-
$this->discoverer = new Discoverer($this->registry);
33+
$this->discoverer = new Discoverer();
3934
}
4035

4136
public function testDiscoversAllElementTypesCorrectlyFromFixtureFiles()
4237
{
43-
$this->discoverer->discover(__DIR__, ['Fixtures']);
38+
$discovery = $this->discoverer->discover(__DIR__, ['Fixtures']);
4439

45-
$tools = $this->registry->getTools();
40+
$tools = $discovery->getTools();
4641
$this->assertCount(4, $tools);
4742

48-
$greetUserTool = $this->registry->getTool('greet_user');
49-
$this->assertInstanceOf(ToolReference::class, $greetUserTool);
50-
$this->assertFalse($greetUserTool->isManual);
51-
$this->assertEquals('greet_user', $greetUserTool->tool->name);
52-
$this->assertEquals('Greets a user by name.', $greetUserTool->tool->description);
53-
$this->assertEquals([DiscoverableToolHandler::class, 'greet'], $greetUserTool->handler);
54-
$this->assertArrayHasKey('name', $greetUserTool->tool->inputSchema['properties'] ?? []);
55-
56-
$repeatActionTool = $this->registry->getTool('repeatAction');
57-
$this->assertInstanceOf(ToolReference::class, $repeatActionTool);
58-
$this->assertEquals('A tool with more complex parameters and inferred name/description.', $repeatActionTool->tool->description);
59-
$this->assertTrue($repeatActionTool->tool->annotations->readOnlyHint);
60-
$this->assertEquals(['count', 'loudly', 'mode'], array_keys($repeatActionTool->tool->inputSchema['properties'] ?? []));
61-
62-
$invokableCalcTool = $this->registry->getTool('InvokableCalculator');
63-
$this->assertInstanceOf(ToolReference::class, $invokableCalcTool);
64-
$this->assertFalse($invokableCalcTool->isManual);
65-
$this->assertEquals([InvocableToolFixture::class, '__invoke'], $invokableCalcTool->handler);
66-
67-
$this->assertNull($this->registry->getTool('private_tool_should_be_ignored'));
68-
$this->assertNull($this->registry->getTool('protected_tool_should_be_ignored'));
69-
$this->assertNull($this->registry->getTool('static_tool_should_be_ignored'));
70-
71-
$resources = $this->registry->getResources();
43+
$this->assertArrayHasKey('greet_user', $tools);
44+
$this->assertFalse($tools['greet_user']->isManual);
45+
$this->assertEquals('greet_user', $tools['greet_user']->tool->name);
46+
$this->assertEquals('Greets a user by name.', $tools['greet_user']->tool->description);
47+
$this->assertEquals([DiscoverableToolHandler::class, 'greet'], $tools['greet_user']->handler);
48+
$this->assertArrayHasKey('name', $tools['greet_user']->tool->inputSchema['properties'] ?? []);
49+
50+
$this->assertArrayHasKey('repeatAction', $tools);
51+
$this->assertEquals('A tool with more complex parameters and inferred name/description.', $tools['repeatAction']->tool->description);
52+
$this->assertTrue($tools['repeatAction']->tool->annotations->readOnlyHint);
53+
$this->assertEquals(['count', 'loudly', 'mode'], array_keys($tools['repeatAction']->tool->inputSchema['properties'] ?? []));
54+
55+
$this->assertArrayHasKey('InvokableCalculator', $tools);
56+
$this->assertInstanceOf(ToolReference::class, $tools['InvokableCalculator']);
57+
$this->assertFalse($tools['InvokableCalculator']->isManual);
58+
$this->assertEquals([InvocableToolFixture::class, '__invoke'], $tools['InvokableCalculator']->handler);
59+
60+
$this->assertArrayNotHasKey('private_tool_should_be_ignored', $tools);
61+
$this->assertArrayNotHasKey('protected_tool_should_be_ignored', $tools);
62+
$this->assertArrayNotHasKey('static_tool_should_be_ignored', $tools);
63+
64+
$resources = $discovery->getResources();
7265
$this->assertCount(3, $resources);
7366

74-
$appVersionRes = $this->registry->getResource('app://info/version');
75-
$this->assertInstanceOf(ResourceReference::class, $appVersionRes);
76-
$this->assertFalse($appVersionRes->isManual);
77-
$this->assertEquals('app_version', $appVersionRes->schema->name);
78-
$this->assertEquals('text/plain', $appVersionRes->schema->mimeType);
67+
$this->assertArrayHasKey('app://info/version', $resources);
68+
$this->assertFalse($resources['app://info/version']->isManual);
69+
$this->assertEquals('app_version', $resources['app://info/version']->schema->name);
70+
$this->assertEquals('text/plain', $resources['app://info/version']->schema->mimeType);
7971

80-
$invokableStatusRes = $this->registry->getResource('invokable://config/status');
81-
$this->assertInstanceOf(ResourceReference::class, $invokableStatusRes);
82-
$this->assertFalse($invokableStatusRes->isManual);
83-
$this->assertEquals([InvocableResourceFixture::class, '__invoke'], $invokableStatusRes->handler);
72+
$this->assertArrayHasKey('invokable://config/status', $resources);
73+
$this->assertFalse($resources['invokable://config/status']->isManual);
74+
$this->assertEquals([InvocableResourceFixture::class, '__invoke'], $resources['invokable://config/status']->handler);
8475

85-
$prompts = $this->registry->getPrompts();
76+
$prompts = $discovery->getPrompts();
8677
$this->assertCount(4, $prompts);
8778

88-
$storyPrompt = $this->registry->getPrompt('creative_story_prompt');
89-
$this->assertInstanceOf(PromptReference::class, $storyPrompt);
90-
$this->assertFalse($storyPrompt->isManual);
91-
$this->assertCount(2, $storyPrompt->prompt->arguments);
92-
$this->assertEquals(CompletionProviderFixture::class, $storyPrompt->completionProviders['genre']);
79+
$this->assertArrayHasKey('creative_story_prompt', $prompts);
80+
$this->assertFalse($prompts['creative_story_prompt']->isManual);
81+
$this->assertCount(2, $prompts['creative_story_prompt']->prompt->arguments);
82+
$this->assertEquals(CompletionProviderFixture::class, $prompts['creative_story_prompt']->completionProviders['genre']);
9383

94-
$simplePrompt = $this->registry->getPrompt('simpleQuestionPrompt');
95-
$this->assertInstanceOf(PromptReference::class, $simplePrompt);
96-
$this->assertFalse($simplePrompt->isManual);
84+
$this->assertArrayHasKey('simpleQuestionPrompt', $prompts);
85+
$this->assertFalse($prompts['simpleQuestionPrompt']->isManual);
9786

98-
$invokableGreeter = $this->registry->getPrompt('InvokableGreeterPrompt');
99-
$this->assertInstanceOf(PromptReference::class, $invokableGreeter);
100-
$this->assertFalse($invokableGreeter->isManual);
101-
$this->assertEquals([InvocablePromptFixture::class, '__invoke'], $invokableGreeter->handler);
87+
$this->assertArrayHasKey('InvokableGreeterPrompt', $prompts);
88+
$this->assertFalse($prompts['InvokableGreeterPrompt']->isManual);
89+
$this->assertEquals([InvocablePromptFixture::class, '__invoke'], $prompts['InvokableGreeterPrompt']->handler);
10290

103-
$contentCreatorPrompt = $this->registry->getPrompt('content_creator');
104-
$this->assertInstanceOf(PromptReference::class, $contentCreatorPrompt);
105-
$this->assertFalse($contentCreatorPrompt->isManual);
106-
$this->assertCount(3, $contentCreatorPrompt->completionProviders);
91+
$this->assertArrayHasKey('content_creator', $prompts);
92+
$this->assertFalse($prompts['content_creator']->isManual);
93+
$this->assertCount(3, $prompts['content_creator']->completionProviders);
10794

108-
$templates = $this->registry->getResourceTemplates();
95+
$templates = $discovery->getResourceTemplates();
10996
$this->assertCount(4, $templates);
11097

111-
$productTemplate = $this->registry->getResourceTemplate('product://{region}/details/{productId}');
112-
$this->assertInstanceOf(ResourceTemplateReference::class, $productTemplate);
113-
$this->assertFalse($productTemplate->isManual);
114-
$this->assertEquals('product_details_template', $productTemplate->resourceTemplate->name);
115-
$this->assertEquals(CompletionProviderFixture::class, $productTemplate->completionProviders['region']);
116-
$this->assertEqualsCanonicalizing(['region', 'productId'], $productTemplate->getVariableNames());
117-
118-
$invokableUserTemplate = $this->registry->getResourceTemplate('invokable://user-profile/{userId}');
119-
$this->assertInstanceOf(ResourceTemplateReference::class, $invokableUserTemplate);
120-
$this->assertFalse($invokableUserTemplate->isManual);
121-
$this->assertEquals([InvocableResourceTemplateFixture::class, '__invoke'], $invokableUserTemplate->handler);
98+
$this->assertArrayHasKey('product://{region}/details/{productId}', $templates);
99+
$this->assertFalse($templates['product://{region}/details/{productId}']->isManual);
100+
$this->assertEquals('product_details_template', $templates['product://{region}/details/{productId}']->resourceTemplate->name);
101+
$this->assertEquals(CompletionProviderFixture::class, $templates['product://{region}/details/{productId}']->completionProviders['region']);
102+
$this->assertEqualsCanonicalizing(['region', 'productId'], $templates['product://{region}/details/{productId}']->getVariableNames());
103+
104+
$this->assertArrayHasKey('invokable://user-profile/{userId}', $templates);
105+
$this->assertFalse($templates['invokable://user-profile/{userId}']->isManual);
106+
$this->assertEquals([InvocableResourceTemplateFixture::class, '__invoke'], $templates['invokable://user-profile/{userId}']->handler);
122107
}
123108

124109
public function testDoesNotDiscoverElementsFromExcludedDirectories()
125110
{
126-
$this->discoverer->discover(__DIR__, ['Fixtures']);
127-
$this->assertInstanceOf(ToolReference::class, $this->registry->getTool('hidden_subdir_tool'));
128-
129-
$this->registry->clear();
111+
$discovery = $this->discoverer->discover(__DIR__, ['Fixtures']);
112+
$this->assertArrayHasKey('hidden_subdir_tool', $discovery->getTools());
130113

131-
$this->discoverer->discover(__DIR__, ['Fixtures'], ['SubDir']);
132-
$this->assertNull($this->registry->getTool('hidden_subdir_tool'));
114+
$discovery = $this->discoverer->discover(__DIR__, ['Fixtures'], ['SubDir']);
115+
$this->assertArrayNotHasKey('hidden_subdir_tool', $discovery->getTools());
133116
}
134117

135118
public function testHandlesEmptyDirectoriesOrDirectoriesWithNoPhpFiles()
136119
{
137-
$this->discoverer->discover(__DIR__, ['EmptyDir']);
138-
$tools = $this->registry->getTools();
139-
$this->assertEmpty($tools->references);
120+
$discovery = $this->discoverer->discover(__DIR__, ['EmptyDir']);
121+
122+
$this->assertTrue($discovery->isEmpty());
140123
}
141124

142125
public function testCorrectlyInfersNamesAndDescriptionsFromMethodsOrClassesIfNotSetInAttribute()
143126
{
144-
$this->discoverer->discover(__DIR__, ['Fixtures']);
127+
$discovery = $this->discoverer->discover(__DIR__, ['Fixtures']);
145128

146-
$repeatActionTool = $this->registry->getTool('repeatAction');
147-
$this->assertEquals('repeatAction', $repeatActionTool->tool->name);
148-
$this->assertEquals('A tool with more complex parameters and inferred name/description.', $repeatActionTool->tool->description);
129+
$this->assertArrayHasKey('repeatAction', $tools = $discovery->getTools());
130+
$this->assertEquals('repeatAction', $tools['repeatAction']->tool->name);
131+
$this->assertEquals('A tool with more complex parameters and inferred name/description.', $tools['repeatAction']->tool->description);
149132

150-
$simplePrompt = $this->registry->getPrompt('simpleQuestionPrompt');
151-
$this->assertEquals('simpleQuestionPrompt', $simplePrompt->prompt->name);
152-
$this->assertNull($simplePrompt->prompt->description);
133+
$this->assertArrayHasKey('simpleQuestionPrompt', $prompts = $discovery->getPrompts());
134+
$this->assertEquals('simpleQuestionPrompt', $prompts['simpleQuestionPrompt']->prompt->name);
135+
$this->assertNull($prompts['simpleQuestionPrompt']->prompt->description);
153136

154-
$invokableCalc = $this->registry->getTool('InvokableCalculator');
155-
$this->assertEquals('InvokableCalculator', $invokableCalc->tool->name);
156-
$this->assertEquals('An invokable calculator tool.', $invokableCalc->tool->description);
137+
$this->assertArrayHasKey('InvokableCalculator', $tools);
138+
$this->assertEquals('InvokableCalculator', $tools['InvokableCalculator']->tool->name);
139+
$this->assertEquals('An invokable calculator tool.', $tools['InvokableCalculator']->tool->description);
157140
}
158141

159142
public function testDiscoversEnhancedCompletionProvidersWithValuesAndEnumAttributes()
160143
{
161-
$this->discoverer->discover(__DIR__, ['Fixtures']);
144+
$discovery = $this->discoverer->discover(__DIR__, ['Fixtures']);
162145

163-
$contentPrompt = $this->registry->getPrompt('content_creator');
164-
$this->assertInstanceOf(PromptReference::class, $contentPrompt);
165-
$this->assertCount(3, $contentPrompt->completionProviders);
146+
$this->assertArrayHasKey('content_creator', $prompts = $discovery->getPrompts());
147+
$this->assertCount(3, $prompts['content_creator']->completionProviders);
166148

167-
$typeProvider = $contentPrompt->completionProviders['type'];
149+
$typeProvider = $prompts['content_creator']->completionProviders['type'];
168150
$this->assertInstanceOf(ListCompletionProvider::class, $typeProvider);
169151

170-
$statusProvider = $contentPrompt->completionProviders['status'];
152+
$statusProvider = $prompts['content_creator']->completionProviders['status'];
171153
$this->assertInstanceOf(EnumCompletionProvider::class, $statusProvider);
172154

173-
$priorityProvider = $contentPrompt->completionProviders['priority'];
155+
$priorityProvider = $prompts['content_creator']->completionProviders['priority'];
174156
$this->assertInstanceOf(EnumCompletionProvider::class, $priorityProvider);
175157

176-
$contentTemplate = $this->registry->getResourceTemplate('content://{category}/{slug}');
177-
$this->assertInstanceOf(ResourceTemplateReference::class, $contentTemplate);
178-
$this->assertCount(1, $contentTemplate->completionProviders);
158+
$this->assertArrayHasKey('content://{category}/{slug}', $templates = $discovery->getResourceTemplates());
159+
$this->assertCount(1, $templates['content://{category}/{slug}']->completionProviders);
179160

180-
$categoryProvider = $contentTemplate->completionProviders['category'];
161+
$categoryProvider = $templates['content://{category}/{slug}']->completionProviders['category'];
181162
$this->assertInstanceOf(ListCompletionProvider::class, $categoryProvider);
182163
}
183164
}

0 commit comments

Comments
 (0)