Skip to content

Commit b3f97f7

Browse files
CodeWithKyrianCodeWithKyrian
committed
fix: use class short name for invokable classes with Mcp Elements
1 parent 9cee8c3 commit b3f97f7

File tree

7 files changed

+67
-36
lines changed

7 files changed

+67
-36
lines changed

‎src/Definitions/PromptDefinition.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ private function validate(): void
4444
{
4545
if (! preg_match(self::PROMPT_NAME_PATTERN, $this->promptName)) {
4646
throw new \InvalidArgumentException(
47-
"Prompt name '{$this->promptName}' is invalid. Prompt names must match the pattern ".self::PROMPT_NAME_PATTERN
48-
.' (alphanumeric characters, underscores, and hyphens only).'
47+
"Prompt name '{$this->promptName}' is invalid. Prompt names must match the pattern " . self::PROMPT_NAME_PATTERN
48+
. ' (alphanumeric characters, underscores, and hyphens only).'
4949
);
5050
}
5151
}
@@ -98,7 +98,7 @@ public function toArray(): array
9898
}
9999
if (! empty($this->arguments)) {
100100
$data['arguments'] = array_map(
101-
fn (PromptArgumentDefinition $arg) => $arg->toArray(),
101+
fn(PromptArgumentDefinition $arg) => $arg->toArray(),
102102
$this->arguments
103103
);
104104
}
@@ -141,6 +141,7 @@ public static function fromReflection(
141141
): self {
142142
$className = $method->getDeclaringClass()->getName();
143143
$methodName = $method->getName();
144+
$promptName = $overrideName ?? ($methodName === '__invoke' ? $method->getDeclaringClass()->getShortName() : $methodName);
144145
$docBlock = $docBlockParser->parseDocBlock($method->getDocComment() ?: null);
145146
$description = $overrideDescription ?? $docBlockParser->getSummary($docBlock) ?? null;
146147

@@ -155,14 +156,14 @@ public static function fromReflection(
155156
}
156157

157158
// Correctly get the specific Param tag using the '$' prefix
158-
$paramTag = $paramTags['$'.$param->getName()] ?? null;
159+
$paramTag = $paramTags['$' . $param->getName()] ?? null;
159160
$arguments[] = PromptArgumentDefinition::fromReflection($param, $paramTag);
160161
}
161162

162163
return new self(
163164
className: $className,
164165
methodName: $methodName,
165-
promptName: $overrideName ?? $methodName,
166+
promptName: $promptName,
166167
description: $description,
167168
arguments: $arguments
168169
);

‎src/Definitions/ResourceDefinition.php‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,15 @@ private function validate(): void
5858
{
5959
if (! preg_match(self::URI_PATTERN, $this->uri)) {
6060
throw new \InvalidArgumentException(
61-
"Resource URI '{$this->uri}' is invalid. URIs must match the pattern ".self::URI_PATTERN
62-
.' (valid scheme followed by :// and optional path).'
61+
"Resource URI '{$this->uri}' is invalid. URIs must match the pattern " . self::URI_PATTERN
62+
. ' (valid scheme followed by :// and optional path).'
6363
);
6464
}
6565

6666
if (! preg_match(self::RESOURCE_NAME_PATTERN, $this->name)) {
6767
throw new \InvalidArgumentException(
68-
"Resource name '{$this->name}' is invalid. Resource names must match the pattern ".self::RESOURCE_NAME_PATTERN
69-
.' (alphanumeric characters, underscores, and hyphens only).'
68+
"Resource name '{$this->name}' is invalid. Resource names must match the pattern " . self::RESOURCE_NAME_PATTERN
69+
. ' (alphanumeric characters, underscores, and hyphens only).'
7070
);
7171
}
7272
}
@@ -178,11 +178,15 @@ public static function fromReflection(
178178
$docBlock = $docBlockParser->parseDocBlock($method->getDocComment() ?: null);
179179
$description = $overrideDescription ?? $docBlockParser->getSummary($docBlock) ?? null;
180180

181+
$name = $overrideName ?? ($method->getName() === '__invoke'
182+
? $method->getDeclaringClass()->getShortName()
183+
: $method->getName());
184+
181185
return new self(
182186
className: $method->getDeclaringClass()->getName(),
183187
methodName: $method->getName(),
184188
uri: $uri,
185-
name: $overrideName ?? $method->getName(),
189+
name: $name,
186190
description: $description,
187191
mimeType: $mimeType,
188192
size: $size,

‎src/Definitions/ResourceTemplateDefinition.php‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,14 @@ private function validate(): void
5656
if (! preg_match(self::URI_TEMPLATE_PATTERN, $this->uriTemplate)) {
5757
throw new \InvalidArgumentException(
5858
"Resource URI template '{$this->uriTemplate}' is invalid. URI templates must match the pattern "
59-
.self::URI_TEMPLATE_PATTERN.' (valid scheme followed by :// and path with placeholder(s) in curly braces).'
59+
. self::URI_TEMPLATE_PATTERN . ' (valid scheme followed by :// and path with placeholder(s) in curly braces).'
6060
);
6161
}
6262

6363
if (! preg_match(self::RESOURCE_NAME_PATTERN, $this->name)) {
6464
throw new \InvalidArgumentException(
65-
"Resource name '{$this->name}' is invalid. Resource names must match the pattern ".self::RESOURCE_NAME_PATTERN
66-
.' (alphanumeric characters, underscores, and hyphens only).'
65+
"Resource name '{$this->name}' is invalid. Resource names must match the pattern " . self::RESOURCE_NAME_PATTERN
66+
. ' (alphanumeric characters, underscores, and hyphens only).'
6767
);
6868
}
6969
}
@@ -169,11 +169,15 @@ public static function fromReflection(
169169
$docBlock = $docBlockParser->parseDocBlock($method->getDocComment() ?: null);
170170
$description = $overrideDescription ?? $docBlockParser->getSummary($docBlock) ?? null;
171171

172+
$name = $overrideName ?? ($method->getName() === '__invoke'
173+
? $method->getDeclaringClass()->getShortName()
174+
: $method->getName());
175+
172176
return new self(
173177
className: $method->getDeclaringClass()->getName(),
174178
methodName: $method->getName(),
175179
uriTemplate: $uriTemplate,
176-
name: $overrideName ?? $method->getName(),
180+
name: $name,
177181
description: $description,
178182
mimeType: $mimeType,
179183
annotations: $annotations

‎src/Definitions/ToolDefinition.php‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ private function validate(): void
4545
{
4646
if (! preg_match(self::TOOL_NAME_PATTERN, $this->toolName)) {
4747
throw new \InvalidArgumentException(
48-
"Tool name '{$this->toolName}' is invalid. Tool names must match the pattern ".self::TOOL_NAME_PATTERN
49-
.' (alphanumeric characters, underscores, and hyphens only).'
48+
"Tool name '{$this->toolName}' is invalid. Tool names must match the pattern " . self::TOOL_NAME_PATTERN
49+
. ' (alphanumeric characters, underscores, and hyphens only).'
5050
);
5151
}
5252
}
@@ -137,10 +137,14 @@ public static function fromReflection(
137137
$description = $overrideDescription ?? $docBlockParser->getSummary($docBlock) ?? null;
138138
$inputSchema = $schemaGenerator->fromMethodParameters($method);
139139

140+
$toolName = $overrideName ?? ($method->getName() === '__invoke'
141+
? $method->getDeclaringClass()->getShortName()
142+
: $method->getName());
143+
140144
return new self(
141145
className: $method->getDeclaringClass()->getName(),
142146
methodName: $method->getName(),
143-
toolName: $overrideName ?? $method->getName(),
147+
toolName: $toolName,
144148
description: $description,
145149
inputSchema: $inputSchema,
146150
);

‎src/Support/Discoverer.php‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
use PhpMcp\Server\Definitions\ResourceDefinition;
1313
use PhpMcp\Server\Definitions\ResourceTemplateDefinition;
1414
use PhpMcp\Server\Definitions\ToolDefinition;
15-
use PhpMcp\Server\Exceptions\McpException;
15+
use PhpMcp\Server\Exception\McpServerException;
1616
use PhpMcp\Server\Registry;
1717
use Psr\Log\LoggerInterface;
1818
use ReflectionAttribute;
@@ -64,7 +64,7 @@ public function discover(string $basePath, array $directories, array $excludeDir
6464
$finder = new Finder();
6565
$absolutePaths = [];
6666
foreach ($directories as $dir) {
67-
$path = rtrim($basePath, '/').'/'.ltrim($dir, '/');
67+
$path = rtrim($basePath, '/') . '/' . ltrim($dir, '/');
6868
if (is_dir($path)) {
6969
$absolutePaths[] = $path;
7070
}
@@ -84,7 +84,6 @@ public function discover(string $basePath, array $directories, array $excludeDir
8484
foreach ($finder as $file) {
8585
$this->processFile($file, $discoveredCount);
8686
}
87-
8887
} catch (Throwable $e) {
8988
$this->logger->error('Error during file finding process for MCP discovery', [
9089
'exception' => $e->getMessage(),
@@ -149,8 +148,10 @@ private function processFile(SplFileInfo $file, array &$discoveredCount): void
149148

150149
if (! $processedViaClassAttribute) {
151150
foreach ($reflectionClass->getMethods(ReflectionMethod::IS_PUBLIC) as $method) {
152-
if ($method->getDeclaringClass()->getName() !== $reflectionClass->getName() ||
153-
$method->isStatic() || $method->isAbstract() || $method->isConstructor() || $method->isDestructor() || $method->getName() === '__invoke') {
151+
if (
152+
$method->getDeclaringClass()->getName() !== $reflectionClass->getName() ||
153+
$method->isStatic() || $method->isAbstract() || $method->isConstructor() || $method->isDestructor() || $method->getName() === '__invoke'
154+
) {
154155
continue;
155156
}
156157
$attributeTypes = [McpTool::class, McpResource::class, McpPrompt::class, McpResourceTemplate::class];
@@ -166,7 +167,6 @@ private function processFile(SplFileInfo $file, array &$discoveredCount): void
166167
}
167168
}
168169
}
169-
170170
} catch (ReflectionException $e) {
171171
$this->logger->error('Reflection error processing file for MCP discovery', ['file' => $filePath, 'class' => $className, 'exception' => $e->getMessage()]);
172172
} catch (Throwable $e) {
@@ -211,7 +211,7 @@ private function processMethod(ReflectionMethod $method, array &$discoveredCount
211211

212212
case McpResource::class:
213213
if (! isset($instance->uri)) {
214-
throw new McpException("McpResource attribute on {$className}::{$methodName} requires a 'uri'.");
214+
throw new McpServerException("McpResource attribute on {$className}::{$methodName} requires a 'uri'.");
215215
}
216216
$definition = ResourceDefinition::fromReflection(
217217
$method,
@@ -240,7 +240,7 @@ private function processMethod(ReflectionMethod $method, array &$discoveredCount
240240

241241
case McpResourceTemplate::class:
242242
if (! isset($instance->uriTemplate)) {
243-
throw new McpException("McpResourceTemplate attribute on {$className}::{$methodName} requires a 'uriTemplate'.");
243+
throw new McpServerException("McpResourceTemplate attribute on {$className}::{$methodName} requires a 'uriTemplate'.");
244244
}
245245
$definition = ResourceTemplateDefinition::fromReflection(
246246
$method,
@@ -255,8 +255,7 @@ private function processMethod(ReflectionMethod $method, array &$discoveredCount
255255
$discoveredCount['resourceTemplates']++;
256256
break;
257257
}
258-
259-
} catch (McpException $e) {
258+
} catch (McpServerException $e) {
260259
$this->logger->error("Failed to process MCP attribute on {$className}::{$methodName}", ['attribute' => $attributeClassName, 'exception' => $e->getMessage(), 'trace' => $e->getPrevious() ? $e->getPrevious()->getTraceAsString() : $e->getTraceAsString()]);
261260
} catch (Throwable $e) {
262261
$this->logger->error("Unexpected error processing attribute on {$className}::{$methodName}", ['attribute' => $attributeClassName, 'exception' => $e->getMessage(), 'trace' => $e->getTraceAsString()]);
@@ -344,7 +343,7 @@ private function getClassFromFile(string $filePath): ?string
344343
for ($j = $i + 1; $j < $tokenCount; $j++) {
345344
if (is_array($tokens[$j]) && $tokens[$j][0] === T_STRING) {
346345
$className = $tokens[$j][1];
347-
$potentialClasses[] = $namespace ? $namespace.'\\'.$className : $className;
346+
$potentialClasses[] = $namespace ? $namespace . '\\' . $className : $className;
348347
$i = $j;
349348
break;
350349
}

‎tests/Mocks/DiscoveryStubs/ToolOnlyStub.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66

77
class ToolOnlyStub
88
{
9+
public function __invoke(): void {}
10+
911
#[McpTool(name: 'tool-from-file1')]
10-
public function tool1(): void
11-
{
12-
}
12+
public function tool1(): void {}
1313
}

‎tests/Unit/Definitions/ToolDefinitionTest.php‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,8 @@
1111
use PhpMcp\Server\Tests\Mocks\DiscoveryStubs\ToolOnlyStub;
1212
use ReflectionMethod;
1313

14-
// --- Constructor Validation Tests ---
15-
1614
test('constructor validates tool name pattern', function (string $toolName, bool $shouldFail) {
17-
$action = fn () => new ToolDefinition(
15+
$action = fn() => new ToolDefinition(
1816
className: AllElementsStub::class,
1917
methodName: 'templateMethod',
2018
toolName: $toolName,
@@ -46,7 +44,6 @@ className: AllElementsStub::class,
4644
// Arrange
4745
$reflectionMethod = new ReflectionMethod(AllElementsStub::class, 'templateMethod');
4846
$attribute = new McpTool(name: 'explicit-tool-name', description: 'Explicit Description');
49-
// Schema needs to match AllElementsStub::templateMethod parameters
5047
$expectedSchema = ['type' => 'object', 'properties' => ['id' => ['type' => 'string']]];
5148
$docComment = $reflectionMethod->getDocComment() ?: null;
5249

@@ -79,7 +76,7 @@ className: AllElementsStub::class,
7976
$docComment = $reflectionMethod->getDocComment() ?: null;
8077

8178
// Read the actual summary from the stub file to make the test robust
82-
$stubContent = file_get_contents(__DIR__.'/../../Mocks/DiscoveryStubs/AllElementsStub.php');
79+
$stubContent = file_get_contents(__DIR__ . '/../../Mocks/DiscoveryStubs/AllElementsStub.php');
8380
preg_match('/\/\*\*(.*?)\*\/\s+public function templateMethod/s', $stubContent, $matches);
8481
$actualDocComment = isset($matches[1]) ? trim(preg_replace('/^\s*\*\s?/?m', '', $matches[1])) : '';
8582
$expectedSummary = explode("\n", $actualDocComment)[0] ?? null; // First line is summary
@@ -105,6 +102,28 @@ className: AllElementsStub::class,
105102
expect($definition->getInputSchema())->toBe($expectedSchema);
106103
});
107104

105+
test('fromReflection uses class short name as default tool name for invokable classes', function () {
106+
$reflectionMethod = new ReflectionMethod(ToolOnlyStub::class, '__invoke');
107+
108+
$docComment = $reflectionMethod->getDocComment() ?: null;
109+
110+
$this->docBlockParser->shouldReceive('parseDocBlock')->once()->with($docComment)->andReturn(null);
111+
$this->schemaGenerator->shouldReceive('fromMethodParameters')->once()->with($reflectionMethod)->andReturn(['type' => 'object']);
112+
113+
$definition = ToolDefinition::fromReflection(
114+
$reflectionMethod,
115+
null,
116+
"Some description",
117+
$this->docBlockParser,
118+
$this->schemaGenerator
119+
);
120+
121+
expect($definition->getName())->toBe('ToolOnlyStub');
122+
expect($definition->getClassName())->toBe(ToolOnlyStub::class);
123+
expect($definition->getMethodName())->toBe('__invoke');
124+
expect($definition->getInputSchema())->toBe(['type' => 'object']);
125+
});
126+
108127
test('fromReflection handles missing docblock summary', function () {
109128
// Arrange
110129
$reflectionMethod = new ReflectionMethod(ToolOnlyStub::class, 'tool1');

0 commit comments

Comments
 (0)