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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----
Expand Down
8 changes: 4 additions & 4 deletions examples/server/complex-tool-schema/Model/EventPriority.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
2 changes: 1 addition & 1 deletion src/Capability/Discovery/SchemaValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ private function formatValidationError(ValidationError $error): string
break;
case 'type':
$expected = implode('|', (array) ($args['expected'] ?? []));
$used = $args['used'] ?? 'unknown';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the opis/json-schema package, I cannot find that $args['used'] is ever set.

$used = $error->data()->type() ?? 'unknown';
$message = "Invalid type. Expected `{$expected}`, but received `{$used}`.";
break;
case 'enum':
Expand Down
4 changes: 2 additions & 2 deletions src/Schema/JsonRpc/Error.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 32 additions & 6 deletions src/Server/Handler/Request/CallToolHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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;
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tests/Inspector/Http/HttpDiscoveryUserProfileTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -13,7 +13,7 @@
"date": "2024-12-15",
"type": "reminder",
"time": "All day",
"priority": "Normal",
"priority": "High",
"attendees": [],
"invites_will_be_sent": false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -13,7 +13,7 @@
"date": "2024-12-02",
"type": "call",
"time": "14:30",
"priority": "Normal",
"priority": "High",
"attendees": [
"client@example.com"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -13,7 +13,7 @@
"date": "2024-12-20",
"type": "other",
"time": "18:00",
"priority": "Normal",
"priority": "Low",
"attendees": [
"team@company.com"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
4 changes: 2 additions & 2 deletions tests/Inspector/InspectorSnapshotTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
49 changes: 48 additions & 1 deletion tests/Unit/Server/Handler/Request/CallToolHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, mixed> $arguments
*/
Expand All @@ -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]);
Expand Down