Skip to content

Commit 42b5261

Browse files
okxaasX2NX
okxaas
and
X2NX
authored
fix: because the cached discoveryState does not use ->setDiscoveryState(), it is found that the discoveryState cannot be used (#120)
Co-authored-by: X2NX <[email protected]>
1 parent f1b471a commit 42b5261

File tree

4 files changed

+90
-121
lines changed

4 files changed

+90
-121
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 & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use Mcp\Capability\Discovery\CachedDiscoverer;
1515
use Mcp\Capability\Discovery\Discoverer;
1616
use Mcp\Capability\Discovery\DiscoveryState;
17-
use Mcp\Capability\Registry;
1817
use PHPUnit\Framework\TestCase;
1918
use Psr\Log\NullLogger;
2019
use Psr\SimpleCache\CacheInterface;
@@ -23,8 +22,7 @@ class CachedDiscovererTest extends TestCase
2322
{
2423
public function testCachedDiscovererUsesCacheOnSecondCall(): void
2524
{
26-
$registry = new Registry(null, new NullLogger());
27-
$discoverer = new Discoverer($registry, new NullLogger());
25+
$discoverer = new Discoverer();
2826

2927
$cache = $this->createMock(CacheInterface::class);
3028
$cache->expects($this->once())
@@ -47,8 +45,7 @@ public function testCachedDiscovererUsesCacheOnSecondCall(): void
4745

4846
public function testCachedDiscovererReturnsCachedResults(): void
4947
{
50-
$registry = new Registry(null, new NullLogger());
51-
$discoverer = new Discoverer($registry, new NullLogger());
48+
$discoverer = new Discoverer();
5249

5350
$cache = $this->createMock(CacheInterface::class);
5451
$cachedState = new DiscoveryState();
@@ -71,8 +68,7 @@ public function testCachedDiscovererReturnsCachedResults(): void
7168

7269
public function testCacheKeyGeneration(): void
7370
{
74-
$registry = new Registry(null, new NullLogger());
75-
$discoverer = new Discoverer($registry, new NullLogger());
71+
$discoverer = new Discoverer();
7672

7773
$cache = $this->createMock(CacheInterface::class);
7874

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

Lines changed: 81 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,6 @@
1414
use Mcp\Capability\Completion\EnumCompletionProvider;
1515
use Mcp\Capability\Completion\ListCompletionProvider;
1616
use Mcp\Capability\Discovery\Discoverer;
17-
use Mcp\Capability\Registry;
18-
use Mcp\Capability\Registry\PromptReference;
19-
use Mcp\Capability\Registry\ResourceReference;
20-
use Mcp\Capability\Registry\ResourceTemplateReference;
2117
use Mcp\Capability\Registry\ToolReference;
2218
use Mcp\Tests\Unit\Capability\Attribute\CompletionProviderFixture;
2319
use Mcp\Tests\Unit\Capability\Discovery\Fixtures\DiscoverableToolHandler;
@@ -29,155 +25,139 @@
2925

3026
class DiscoveryTest extends TestCase
3127
{
32-
private Registry $registry;
3328
private Discoverer $discoverer;
3429

3530
protected function setUp(): void
3631
{
37-
$this->registry = new Registry();
38-
$this->discoverer = new Discoverer($this->registry);
32+
$this->discoverer = new Discoverer();
3933
}
4034

4135
public function testDiscoversAllElementTypesCorrectlyFromFixtureFiles()
4236
{
43-
$this->discoverer->discover(__DIR__, ['Fixtures']);
37+
$discovery = $this->discoverer->discover(__DIR__, ['Fixtures']);
4438

45-
$tools = $this->registry->getTools();
39+
$tools = $discovery->getTools();
4640
$this->assertCount(4, $tools);
4741

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();
42+
$this->assertArrayHasKey('greet_user', $tools);
43+
$this->assertFalse($tools['greet_user']->isManual);
44+
$this->assertEquals('greet_user', $tools['greet_user']->tool->name);
45+
$this->assertEquals('Greets a user by name.', $tools['greet_user']->tool->description);
46+
$this->assertEquals([DiscoverableToolHandler::class, 'greet'], $tools['greet_user']->handler);
47+
$this->assertArrayHasKey('name', $tools['greet_user']->tool->inputSchema['properties'] ?? []);
48+
49+
$this->assertArrayHasKey('repeatAction', $tools);
50+
$this->assertEquals('A tool with more complex parameters and inferred name/description.', $tools['repeatAction']->tool->description);
51+
$this->assertTrue($tools['repeatAction']->tool->annotations->readOnlyHint);
52+
$this->assertEquals(['count', 'loudly', 'mode'], array_keys($tools['repeatAction']->tool->inputSchema['properties'] ?? []));
53+
54+
$this->assertArrayHasKey('InvokableCalculator', $tools);
55+
$this->assertInstanceOf(ToolReference::class, $tools['InvokableCalculator']);
56+
$this->assertFalse($tools['InvokableCalculator']->isManual);
57+
$this->assertEquals([InvocableToolFixture::class, '__invoke'], $tools['InvokableCalculator']->handler);
58+
59+
$this->assertArrayNotHasKey('private_tool_should_be_ignored', $tools);
60+
$this->assertArrayNotHasKey('protected_tool_should_be_ignored', $tools);
61+
$this->assertArrayNotHasKey('static_tool_should_be_ignored', $tools);
62+
63+
$resources = $discovery->getResources();
7264
$this->assertCount(3, $resources);
7365

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);
66+
$this->assertArrayHasKey('app://info/version', $resources);
67+
$this->assertFalse($resources['app://info/version']->isManual);
68+
$this->assertEquals('app_version', $resources['app://info/version']->schema->name);
69+
$this->assertEquals('text/plain', $resources['app://info/version']->schema->mimeType);
7970

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);
71+
$this->assertArrayHasKey('invokable://config/status', $resources);
72+
$this->assertFalse($resources['invokable://config/status']->isManual);
73+
$this->assertEquals([InvocableResourceFixture::class, '__invoke'], $resources['invokable://config/status']->handler);
8474

85-
$prompts = $this->registry->getPrompts();
75+
$prompts = $discovery->getPrompts();
8676
$this->assertCount(4, $prompts);
8777

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']);
78+
$this->assertArrayHasKey('creative_story_prompt', $prompts);
79+
$this->assertFalse($prompts['creative_story_prompt']->isManual);
80+
$this->assertCount(2, $prompts['creative_story_prompt']->prompt->arguments);
81+
$this->assertEquals(CompletionProviderFixture::class, $prompts['creative_story_prompt']->completionProviders['genre']);
9382

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

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);
86+
$this->assertArrayHasKey('InvokableGreeterPrompt', $prompts);
87+
$this->assertFalse($prompts['InvokableGreeterPrompt']->isManual);
88+
$this->assertEquals([InvocablePromptFixture::class, '__invoke'], $prompts['InvokableGreeterPrompt']->handler);
10289

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

108-
$templates = $this->registry->getResourceTemplates();
94+
$templates = $discovery->getResourceTemplates();
10995
$this->assertCount(4, $templates);
11096

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);
97+
$this->assertArrayHasKey('product://{region}/details/{productId}', $templates);
98+
$this->assertFalse($templates['product://{region}/details/{productId}']->isManual);
99+
$this->assertEquals('product_details_template', $templates['product://{region}/details/{productId}']->resourceTemplate->name);
100+
$this->assertEquals(CompletionProviderFixture::class, $templates['product://{region}/details/{productId}']->completionProviders['region']);
101+
$this->assertEqualsCanonicalizing(['region', 'productId'], $templates['product://{region}/details/{productId}']->getVariableNames());
102+
103+
$this->assertArrayHasKey('invokable://user-profile/{userId}', $templates);
104+
$this->assertFalse($templates['invokable://user-profile/{userId}']->isManual);
105+
$this->assertEquals([InvocableResourceTemplateFixture::class, '__invoke'], $templates['invokable://user-profile/{userId}']->handler);
122106
}
123107

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

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

135117
public function testHandlesEmptyDirectoriesOrDirectoriesWithNoPhpFiles()
136118
{
137-
$this->discoverer->discover(__DIR__, ['EmptyDir']);
138-
$tools = $this->registry->getTools();
139-
$this->assertEmpty($tools->references);
119+
$discovery = $this->discoverer->discover(__DIR__, ['EmptyDir']);
120+
121+
$this->assertTrue($discovery->isEmpty());
140122
}
141123

142124
public function testCorrectlyInfersNamesAndDescriptionsFromMethodsOrClassesIfNotSetInAttribute()
143125
{
144-
$this->discoverer->discover(__DIR__, ['Fixtures']);
126+
$discovery = $this->discoverer->discover(__DIR__, ['Fixtures']);
145127

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);
128+
$this->assertArrayHasKey('repeatAction', $tools = $discovery->getTools());
129+
$this->assertEquals('repeatAction', $tools['repeatAction']->tool->name);
130+
$this->assertEquals('A tool with more complex parameters and inferred name/description.', $tools['repeatAction']->tool->description);
149131

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

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

159141
public function testDiscoversEnhancedCompletionProvidersWithValuesAndEnumAttributes()
160142
{
161-
$this->discoverer->discover(__DIR__, ['Fixtures']);
143+
$discovery = $this->discoverer->discover(__DIR__, ['Fixtures']);
162144

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

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

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

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

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

180-
$categoryProvider = $contentTemplate->completionProviders['category'];
160+
$categoryProvider = $templates['content://{category}/{slug}']->completionProviders['category'];
181161
$this->assertInstanceOf(ListCompletionProvider::class, $categoryProvider);
182162
}
183163
}

0 commit comments

Comments
 (0)