Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/Definitions/PromptDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ private function validate(): void
{
if (! preg_match(self::PROMPT_NAME_PATTERN, $this->promptName)) {
throw new \InvalidArgumentException(
"Prompt name '{$this->promptName}' is invalid. Prompt names must match the pattern ".self::PROMPT_NAME_PATTERN
.' (alphanumeric characters, underscores, and hyphens only).'
"Prompt name '{$this->promptName}' is invalid. Prompt names must match the pattern " . self::PROMPT_NAME_PATTERN
. ' (alphanumeric characters, underscores, and hyphens only).'
);
}
}
Expand Down Expand Up @@ -98,7 +98,7 @@ public function toArray(): array
}
if (! empty($this->arguments)) {
$data['arguments'] = array_map(
fn (PromptArgumentDefinition $arg) => $arg->toArray(),
fn(PromptArgumentDefinition $arg) => $arg->toArray(),
$this->arguments
);
}
Expand Down Expand Up @@ -141,6 +141,7 @@ public static function fromReflection(
): self {
$className = $method->getDeclaringClass()->getName();
$methodName = $method->getName();
$promptName = $overrideName ?? ($methodName === '__invoke' ? $method->getDeclaringClass()->getShortName() : $methodName);
$docBlock = $docBlockParser->parseDocBlock($method->getDocComment() ?: null);
$description = $overrideDescription ?? $docBlockParser->getSummary($docBlock) ?? null;

Expand All @@ -155,14 +156,14 @@ public static function fromReflection(
}

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

return new self(
className: $className,
methodName: $methodName,
promptName: $overrideName ?? $methodName,
promptName: $promptName,
description: $description,
arguments: $arguments
);
Expand Down
14 changes: 9 additions & 5 deletions src/Definitions/ResourceDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ private function validate(): void
{
if (! preg_match(self::URI_PATTERN, $this->uri)) {
throw new \InvalidArgumentException(
"Resource URI '{$this->uri}' is invalid. URIs must match the pattern ".self::URI_PATTERN
.' (valid scheme followed by :// and optional path).'
"Resource URI '{$this->uri}' is invalid. URIs must match the pattern " . self::URI_PATTERN
. ' (valid scheme followed by :// and optional path).'
);
}

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

$name = $overrideName ?? ($method->getName() === '__invoke'
? $method->getDeclaringClass()->getShortName()
: $method->getName());

return new self(
className: $method->getDeclaringClass()->getName(),
methodName: $method->getName(),
uri: $uri,
name: $overrideName ?? $method->getName(),
name: $name,
description: $description,
mimeType: $mimeType,
size: $size,
Expand Down
12 changes: 8 additions & 4 deletions src/Definitions/ResourceTemplateDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@ private function validate(): void
if (! preg_match(self::URI_TEMPLATE_PATTERN, $this->uriTemplate)) {
throw new \InvalidArgumentException(
"Resource URI template '{$this->uriTemplate}' is invalid. URI templates must match the pattern "
.self::URI_TEMPLATE_PATTERN.' (valid scheme followed by :// and path with placeholder(s) in curly braces).'
. self::URI_TEMPLATE_PATTERN . ' (valid scheme followed by :// and path with placeholder(s) in curly braces).'
);
}

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

$name = $overrideName ?? ($method->getName() === '__invoke'
? $method->getDeclaringClass()->getShortName()
: $method->getName());

return new self(
className: $method->getDeclaringClass()->getName(),
methodName: $method->getName(),
uriTemplate: $uriTemplate,
name: $overrideName ?? $method->getName(),
name: $name,
description: $description,
mimeType: $mimeType,
annotations: $annotations
Expand Down
10 changes: 7 additions & 3 deletions src/Definitions/ToolDefinition.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ private function validate(): void
{
if (! preg_match(self::TOOL_NAME_PATTERN, $this->toolName)) {
throw new \InvalidArgumentException(
"Tool name '{$this->toolName}' is invalid. Tool names must match the pattern ".self::TOOL_NAME_PATTERN
.' (alphanumeric characters, underscores, and hyphens only).'
"Tool name '{$this->toolName}' is invalid. Tool names must match the pattern " . self::TOOL_NAME_PATTERN
. ' (alphanumeric characters, underscores, and hyphens only).'
);
}
}
Expand Down Expand Up @@ -137,10 +137,14 @@ public static function fromReflection(
$description = $overrideDescription ?? $docBlockParser->getSummary($docBlock) ?? null;
$inputSchema = $schemaGenerator->fromMethodParameters($method);

$toolName = $overrideName ?? ($method->getName() === '__invoke'
? $method->getDeclaringClass()->getShortName()
: $method->getName());

return new self(
className: $method->getDeclaringClass()->getName(),
methodName: $method->getName(),
toolName: $overrideName ?? $method->getName(),
toolName: $toolName,
description: $description,
inputSchema: $inputSchema,
);
Expand Down
21 changes: 10 additions & 11 deletions src/Support/Discoverer.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
use PhpMcp\Server\Definitions\ResourceDefinition;
use PhpMcp\Server\Definitions\ResourceTemplateDefinition;
use PhpMcp\Server\Definitions\ToolDefinition;
use PhpMcp\Server\Exceptions\McpException;
use PhpMcp\Server\Exception\McpServerException;
use PhpMcp\Server\Registry;
use Psr\Log\LoggerInterface;
use ReflectionAttribute;
Expand Down Expand Up @@ -64,7 +64,7 @@ public function discover(string $basePath, array $directories, array $excludeDir
$finder = new Finder();
$absolutePaths = [];
foreach ($directories as $dir) {
$path = rtrim($basePath, '/').'/'.ltrim($dir, '/');
$path = rtrim($basePath, '/') . '/' . ltrim($dir, '/');
if (is_dir($path)) {
$absolutePaths[] = $path;
}
Expand All @@ -84,7 +84,6 @@ public function discover(string $basePath, array $directories, array $excludeDir
foreach ($finder as $file) {
$this->processFile($file, $discoveredCount);
}

} catch (Throwable $e) {
$this->logger->error('Error during file finding process for MCP discovery', [
'exception' => $e->getMessage(),
Expand Down Expand Up @@ -149,8 +148,10 @@ private function processFile(SplFileInfo $file, array &$discoveredCount): void

if (! $processedViaClassAttribute) {
foreach ($reflectionClass->getMethods(ReflectionMethod::IS_PUBLIC) as $method) {
if ($method->getDeclaringClass()->getName() !== $reflectionClass->getName() ||
$method->isStatic() || $method->isAbstract() || $method->isConstructor() || $method->isDestructor() || $method->getName() === '__invoke') {
if (
$method->getDeclaringClass()->getName() !== $reflectionClass->getName() ||
$method->isStatic() || $method->isAbstract() || $method->isConstructor() || $method->isDestructor() || $method->getName() === '__invoke'
) {
continue;
}
$attributeTypes = [McpTool::class, McpResource::class, McpPrompt::class, McpResourceTemplate::class];
Expand All @@ -166,7 +167,6 @@ private function processFile(SplFileInfo $file, array &$discoveredCount): void
}
}
}

} catch (ReflectionException $e) {
$this->logger->error('Reflection error processing file for MCP discovery', ['file' => $filePath, 'class' => $className, 'exception' => $e->getMessage()]);
} catch (Throwable $e) {
Expand Down Expand Up @@ -211,7 +211,7 @@ private function processMethod(ReflectionMethod $method, array &$discoveredCount

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

case McpResourceTemplate::class:
if (! isset($instance->uriTemplate)) {
throw new McpException("McpResourceTemplate attribute on {$className}::{$methodName} requires a 'uriTemplate'.");
throw new McpServerException("McpResourceTemplate attribute on {$className}::{$methodName} requires a 'uriTemplate'.");
}
$definition = ResourceTemplateDefinition::fromReflection(
$method,
Expand All @@ -255,8 +255,7 @@ private function processMethod(ReflectionMethod $method, array &$discoveredCount
$discoveredCount['resourceTemplates']++;
break;
}

} catch (McpException $e) {
} catch (McpServerException $e) {
$this->logger->error("Failed to process MCP attribute on {$className}::{$methodName}", ['attribute' => $attributeClassName, 'exception' => $e->getMessage(), 'trace' => $e->getPrevious() ? $e->getPrevious()->getTraceAsString() : $e->getTraceAsString()]);
} catch (Throwable $e) {
$this->logger->error("Unexpected error processing attribute on {$className}::{$methodName}", ['attribute' => $attributeClassName, 'exception' => $e->getMessage(), 'trace' => $e->getTraceAsString()]);
Expand Down Expand Up @@ -344,7 +343,7 @@ private function getClassFromFile(string $filePath): ?string
for ($j = $i + 1; $j < $tokenCount; $j++) {
if (is_array($tokens[$j]) && $tokens[$j][0] === T_STRING) {
$className = $tokens[$j][1];
$potentialClasses[] = $namespace ? $namespace.'\\'.$className : $className;
$potentialClasses[] = $namespace ? $namespace . '\\' . $className : $className;
$i = $j;
break;
}
Expand Down
6 changes: 3 additions & 3 deletions tests/Mocks/DiscoveryStubs/ToolOnlyStub.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

class ToolOnlyStub
{
public function __invoke(): void {}

#[McpTool(name: 'tool-from-file1')]
public function tool1(): void
{
}
public function tool1(): void {}
}
29 changes: 24 additions & 5 deletions tests/Unit/Definitions/ToolDefinitionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,8 @@
use PhpMcp\Server\Tests\Mocks\DiscoveryStubs\ToolOnlyStub;
use ReflectionMethod;

// --- Constructor Validation Tests ---

test('constructor validates tool name pattern', function (string $toolName, bool $shouldFail) {
$action = fn () => new ToolDefinition(
$action = fn() => new ToolDefinition(
className: AllElementsStub::class,
methodName: 'templateMethod',
toolName: $toolName,
Expand Down Expand Up @@ -46,7 +44,6 @@ className: AllElementsStub::class,
// Arrange
$reflectionMethod = new ReflectionMethod(AllElementsStub::class, 'templateMethod');
$attribute = new McpTool(name: 'explicit-tool-name', description: 'Explicit Description');
// Schema needs to match AllElementsStub::templateMethod parameters
$expectedSchema = ['type' => 'object', 'properties' => ['id' => ['type' => 'string']]];
$docComment = $reflectionMethod->getDocComment() ?: null;

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

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

test('fromReflection uses class short name as default tool name for invokable classes', function () {
$reflectionMethod = new ReflectionMethod(ToolOnlyStub::class, '__invoke');

$docComment = $reflectionMethod->getDocComment() ?: null;

$this->docBlockParser->shouldReceive('parseDocBlock')->once()->with($docComment)->andReturn(null);
$this->schemaGenerator->shouldReceive('fromMethodParameters')->once()->with($reflectionMethod)->andReturn(['type' => 'object']);

$definition = ToolDefinition::fromReflection(
$reflectionMethod,
null,
"Some description",
$this->docBlockParser,
$this->schemaGenerator
);

expect($definition->getName())->toBe('ToolOnlyStub');
expect($definition->getClassName())->toBe(ToolOnlyStub::class);
expect($definition->getMethodName())->toBe('__invoke');
expect($definition->getInputSchema())->toBe(['type' => 'object']);
});

test('fromReflection handles missing docblock summary', function () {
// Arrange
$reflectionMethod = new ReflectionMethod(ToolOnlyStub::class, 'tool1');
Expand Down