diff --git a/CHANGELOG.md b/CHANGELOG.md index 66238624..cb34bbd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,11 @@ All notable changes to `mcp/sdk` will be documented in this file. -0.3 +0.3.0 ----- * Add output schema support to MCP tools +* Add validation of the input parameters given to a Tool. 0.2.2 ----- diff --git a/examples/server/complex-tool-schema/Model/EventPriority.php b/examples/server/complex-tool-schema/Model/EventPriority.php index eb046eff..97bdb70e 100644 --- a/examples/server/complex-tool-schema/Model/EventPriority.php +++ b/examples/server/complex-tool-schema/Model/EventPriority.php @@ -11,9 +11,9 @@ namespace Mcp\Example\Server\ComplexToolSchema\Model; -enum EventPriority: int +enum EventPriority: string { - case Low = 0; - case Normal = 1; - case High = 2; + case Low = 'low'; + case Normal = 'normal'; + case High = 'high'; } diff --git a/src/Capability/Discovery/SchemaValidator.php b/src/Capability/Discovery/SchemaValidator.php index cc6be11d..a6a37b35 100644 --- a/src/Capability/Discovery/SchemaValidator.php +++ b/src/Capability/Discovery/SchemaValidator.php @@ -220,7 +220,7 @@ private function formatValidationError(ValidationError $error): string break; case 'type': $expected = implode('|', (array) ($args['expected'] ?? [])); - $used = $args['used'] ?? 'unknown'; + $used = $error->data()->type() ?? 'unknown'; $message = "Invalid type. Expected `{$expected}`, but received `{$used}`."; break; case 'enum': diff --git a/src/Schema/JsonRpc/Error.php b/src/Schema/JsonRpc/Error.php index d5273eb1..422b295f 100644 --- a/src/Schema/JsonRpc/Error.php +++ b/src/Schema/JsonRpc/Error.php @@ -91,9 +91,9 @@ public static function forMethodNotFound(string $message, string|int $id = ''): return new self($id, self::METHOD_NOT_FOUND, $message); } - public static function forInvalidParams(string $message, string|int $id = ''): self + public static function forInvalidParams(string $message, string|int $id = '', mixed $data = null): self { - return new self($id, self::INVALID_PARAMS, $message); + return new self($id, self::INVALID_PARAMS, $message, $data); } public static function forInternalError(string $message, string|int $id = ''): self diff --git a/src/Server/Handler/Request/CallToolHandler.php b/src/Server/Handler/Request/CallToolHandler.php index c19b5b8c..ab92239d 100644 --- a/src/Server/Handler/Request/CallToolHandler.php +++ b/src/Server/Handler/Request/CallToolHandler.php @@ -11,6 +11,7 @@ namespace Mcp\Server\Handler\Request; +use Mcp\Capability\Discovery\SchemaValidator; use Mcp\Capability\Registry\ReferenceHandlerInterface; use Mcp\Capability\RegistryInterface; use Mcp\Exception\ToolCallException; @@ -33,11 +34,15 @@ */ final class CallToolHandler implements RequestHandlerInterface { + private SchemaValidator $schemaValidator; + public function __construct( private readonly RegistryInterface $registry, private readonly ReferenceHandlerInterface $referenceHandler, private readonly LoggerInterface $logger = new NullLogger(), + ?SchemaValidator $schemaValidator = null, ) { + $this->schemaValidator = $schemaValidator ?? new SchemaValidator($logger); } public function supports(Request $request): bool @@ -59,10 +64,35 @@ public function handle(Request $request, SessionInterface $session): Response|Er try { $reference = $this->registry->getTool($toolName); + } catch (ToolNotFoundException $e) { + $this->logger->error('Tool not found', ['name' => $toolName]); + + return new Error($request->getId(), Error::METHOD_NOT_FOUND, $e->getMessage()); + } + + $inputSchema = $reference->tool->inputSchema; + $validationErrors = $this->schemaValidator->validateAgainstJsonSchema($arguments, $inputSchema); + if (!empty($validationErrors)) { + $errorMessages = []; + + foreach ($validationErrors as $errorDetail) { + $pointer = $errorDetail['pointer'] ?? ''; + $message = $errorDetail['message'] ?? 'Unknown validation error'; + $errorMessages[] = ('/' !== $pointer && '' !== $pointer ? "Property '{$pointer}': " : '').$message; + } - $arguments['_session'] = $session; - $arguments['_request'] = $request; + $summaryMessage = "Invalid parameters for tool '{$toolName}': ".implode('; ', \array_slice($errorMessages, 0, 3)); + if (\count($errorMessages) > 3) { + $summaryMessage .= '; ...and more errors.'; + } + + return Error::forInvalidParams($summaryMessage, $request->getId(), ['validation_errors' => $validationErrors]); + } + $arguments['_session'] = $session; + $arguments['_request'] = $request; + + try { $result = $this->referenceHandler->handle($reference, $arguments); $structuredContent = null; @@ -87,10 +117,6 @@ public function handle(Request $request, SessionInterface $session): Response|Er $errorContent = [new TextContent($e->getMessage())]; return new Response($request->getId(), CallToolResult::error($errorContent)); - } catch (ToolNotFoundException $e) { - $this->logger->error('Tool not found', ['name' => $toolName]); - - return new Error($request->getId(), Error::METHOD_NOT_FOUND, $e->getMessage()); } catch (\Throwable $e) { $this->logger->error('Unhandled error during tool execution', [ 'name' => $toolName, diff --git a/tests/Inspector/Http/HttpDiscoveryUserProfileTest.php b/tests/Inspector/Http/HttpDiscoveryUserProfileTest.php index 80114396..985b01b7 100644 --- a/tests/Inspector/Http/HttpDiscoveryUserProfileTest.php +++ b/tests/Inspector/Http/HttpDiscoveryUserProfileTest.php @@ -21,7 +21,7 @@ public static function provideMethods(): array 'method' => 'tools/call', 'options' => [ 'toolName' => 'send_welcome', - 'toolArgs' => ['userId' => '101', 'customMessage' => 'Welcome to our platform!'], + 'toolArgs' => ['userId' => '"101"', 'customMessage' => 'Welcome to our platform!'], ], 'testName' => 'send_welcome', ], diff --git a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_all_day_reminder.json b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_all_day_reminder.json index 5f6e553f..26888e22 100644 --- a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_all_day_reminder.json +++ b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_all_day_reminder.json @@ -2,7 +2,7 @@ "content": [ { "type": "text", - "text": "{\n \"success\": true,\n \"message\": \"Event \\\"Project Deadline\\\" scheduled successfully for \\\"2024-12-15\\\".\",\n \"event_details\": {\n \"title\": \"Project Deadline\",\n \"date\": \"2024-12-15\",\n \"type\": \"reminder\",\n \"time\": \"All day\",\n \"priority\": \"Normal\",\n \"attendees\": [],\n \"invites_will_be_sent\": false\n }\n}" + "text": "{\n \"success\": true,\n \"message\": \"Event \\\"Project Deadline\\\" scheduled successfully for \\\"2024-12-15\\\".\",\n \"event_details\": {\n \"title\": \"Project Deadline\",\n \"date\": \"2024-12-15\",\n \"type\": \"reminder\",\n \"time\": \"All day\",\n \"priority\": \"High\",\n \"attendees\": [],\n \"invites_will_be_sent\": false\n }\n}" } ], "structuredContent": { @@ -13,7 +13,7 @@ "date": "2024-12-15", "type": "reminder", "time": "All day", - "priority": "Normal", + "priority": "High", "attendees": [], "invites_will_be_sent": false } diff --git a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_high_priority.json b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_high_priority.json index 8a702475..23ee2994 100644 --- a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_high_priority.json +++ b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_high_priority.json @@ -2,7 +2,7 @@ "content": [ { "type": "text", - "text": "{\n \"success\": true,\n \"message\": \"Event \\\"Client Call\\\" scheduled successfully for \\\"2024-12-02\\\".\",\n \"event_details\": {\n \"title\": \"Client Call\",\n \"date\": \"2024-12-02\",\n \"type\": \"call\",\n \"time\": \"14:30\",\n \"priority\": \"Normal\",\n \"attendees\": [\n \"client@example.com\"\n ],\n \"invites_will_be_sent\": false\n }\n}" + "text": "{\n \"success\": true,\n \"message\": \"Event \\\"Client Call\\\" scheduled successfully for \\\"2024-12-02\\\".\",\n \"event_details\": {\n \"title\": \"Client Call\",\n \"date\": \"2024-12-02\",\n \"type\": \"call\",\n \"time\": \"14:30\",\n \"priority\": \"High\",\n \"attendees\": [\n \"client@example.com\"\n ],\n \"invites_will_be_sent\": false\n }\n}" } ], "structuredContent": { @@ -13,7 +13,7 @@ "date": "2024-12-02", "type": "call", "time": "14:30", - "priority": "Normal", + "priority": "High", "attendees": [ "client@example.com" ], diff --git a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_low_priority.json b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_low_priority.json index 97c2340e..36980277 100644 --- a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_low_priority.json +++ b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_call-schedule_event_low_priority.json @@ -2,7 +2,7 @@ "content": [ { "type": "text", - "text": "{\n \"success\": true,\n \"message\": \"Event \\\"Office Party\\\" scheduled successfully for \\\"2024-12-20\\\".\",\n \"event_details\": {\n \"title\": \"Office Party\",\n \"date\": \"2024-12-20\",\n \"type\": \"other\",\n \"time\": \"18:00\",\n \"priority\": \"Normal\",\n \"attendees\": [\n \"team@company.com\"\n ],\n \"invites_will_be_sent\": true\n }\n}" + "text": "{\n \"success\": true,\n \"message\": \"Event \\\"Office Party\\\" scheduled successfully for \\\"2024-12-20\\\".\",\n \"event_details\": {\n \"title\": \"Office Party\",\n \"date\": \"2024-12-20\",\n \"type\": \"other\",\n \"time\": \"18:00\",\n \"priority\": \"Low\",\n \"attendees\": [\n \"team@company.com\"\n ],\n \"invites_will_be_sent\": true\n }\n}" } ], "structuredContent": { @@ -13,7 +13,7 @@ "date": "2024-12-20", "type": "other", "time": "18:00", - "priority": "Normal", + "priority": "Low", "attendees": [ "team@company.com" ], diff --git a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_list.json b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_list.json index 5f47adca..58815df7 100644 --- a/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_list.json +++ b/tests/Inspector/Http/snapshots/HttpComplexToolSchemaTest-tools_list.json @@ -33,13 +33,13 @@ "default": null }, "priority": { - "type": "integer", + "type": "string", "description": "The priority of the event. Defaults to Normal.", - "default": 1, + "default": "normal", "enum": [ - 0, - 1, - 2 + "low", + "normal", + "high" ] }, "attendees": { diff --git a/tests/Inspector/InspectorSnapshotTestCase.php b/tests/Inspector/InspectorSnapshotTestCase.php index 292670ae..18bb7f45 100644 --- a/tests/Inspector/InspectorSnapshotTestCase.php +++ b/tests/Inspector/InspectorSnapshotTestCase.php @@ -50,7 +50,7 @@ public function testOutputMatchesSnapshot( if (\is_array($value)) { $args[] = \sprintf('%s=%s', $key, json_encode($value)); } elseif (\is_bool($value)) { - $args[] = \sprintf('%s=%s', $key, $value ? '1' : '0'); + $args[] = \sprintf('%s=%s', $key, $value ? 'true' : 'false'); } else { $args[] = \sprintf('%s=%s', $key, $value); } @@ -73,7 +73,7 @@ public function testOutputMatchesSnapshot( if (\is_array($value)) { $args[] = \sprintf('%s=%s', $key, json_encode($value)); } elseif (\is_bool($value)) { - $args[] = \sprintf('%s=%s', $key, $value ? '1' : '0'); + $args[] = \sprintf('%s=%s', $key, $value ? 'true' : 'false'); } else { $args[] = \sprintf('%s=%s', $key, $value); } diff --git a/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php b/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php index 90af0c84..ce44f276 100644 --- a/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php +++ b/tests/Unit/Server/Handler/Request/CallToolHandlerTest.php @@ -466,6 +466,43 @@ public function testHandleReturnsCallToolResult(): void $this->assertArrayNotHasKey('structuredContent', $response->result->jsonSerialize()); } + public function testValidationError(): void + { + $schema = [ + 'type' => 'object', + 'properties' => [ + 'favorite_number' => [ + 'type' => 'number', + 'description' => 'Your favorite number', + ], + ], + 'required' => [ + 'favorite_number', + ], + ]; + + $request = $this->createCallToolRequest('result_tool', ['query' => 'php']); + $toolReference = $this->getMockBuilder(ToolReference::class) + ->setConstructorArgs([new Tool('simple_tool', $schema, null, null), function () {}]) + ->getMock(); + + $this->registry + ->expects($this->once()) + ->method('getTool') + ->with('result_tool') + ->willReturn($toolReference); + + $this->referenceHandler + ->expects($this->never()) + ->method('handle'); + + $response = $this->handler->handle($request, $this->session); + + $this->assertInstanceOf(Error::class, $response); + $this->assertEquals($request->getId(), $response->id); + $this->assertEquals(Error::INVALID_PARAMS, $response->code); + } + /** * @param array $arguments */ @@ -488,7 +525,17 @@ private function createToolReference( ?array $outputSchema = null, array $methodsToMock = ['formatResult'], ): ToolReference&MockObject { - $tool = new Tool($name, ['type' => 'object', 'properties' => [], 'required' => null], null, null, null, null, $outputSchema); + $schema = [ + 'type' => 'object', + 'properties' => [ + 'example' => [ + 'type' => 'string', + 'description' => 'This is just a dummy', + ], + ], + 'required' => [], + ]; + $tool = new Tool($name, $schema, null, null, null, null, $outputSchema); $builder = $this->getMockBuilder(ToolReference::class) ->setConstructorArgs([$tool, $handler]);