Use SchemaValidator to validate tool input (#203)
* Use SchemaValidator to validate tool input * add test * add changelog * fix cs * Improve error message * fix tests * fix tests * fix baseline * update snapshot files * Remove BC compat while switching to v0.3.0 --------- Co-authored-by: Christopher Hertel <mail@christopher-hertel.de>
This commit is contained in:
parent
c0938a19bd
commit
ed205ef793
12 changed files with 103 additions and 29 deletions
|
|
@ -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
|
||||
-----
|
||||
|
|
|
|||
|
|
@ -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';
|
||||
}
|
||||
|
|
|
|||
|
|
@ -220,7 +220,7 @@ class SchemaValidator
|
|||
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':
|
||||
|
|
|
|||
|
|
@ -91,9 +91,9 @@ class Error implements MessageInterface
|
|||
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
|
||||
|
|
|
|||
|
|
@ -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 @@ use Psr\Log\NullLogger;
|
|||
*/
|
||||
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 @@ final class CallToolHandler implements RequestHandlerInterface
|
|||
|
||||
try {
|
||||
$reference = $this->registry->getTool($toolName);
|
||||
} catch (ToolNotFoundException $e) {
|
||||
$this->logger->error('Tool not found', ['name' => $toolName]);
|
||||
|
||||
$arguments['_session'] = $session;
|
||||
$arguments['_request'] = $request;
|
||||
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;
|
||||
}
|
||||
|
||||
$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 @@ final class CallToolHandler implements RequestHandlerInterface
|
|||
$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,
|
||||
|
|
|
|||
|
|
@ -21,7 +21,7 @@ final class HttpDiscoveryUserProfileTest extends HttpInspectorSnapshotTestCase
|
|||
'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',
|
||||
],
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
],
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
],
|
||||
|
|
|
|||
|
|
@ -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": {
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@ abstract class InspectorSnapshotTestCase extends TestCase
|
|||
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 @@ abstract class InspectorSnapshotTestCase extends TestCase
|
|||
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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -466,6 +466,43 @@ class CallToolHandlerTest extends TestCase
|
|||
$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
|
||||
*/
|
||||
|
|
@ -488,7 +525,17 @@ class CallToolHandlerTest extends TestCase
|
|||
?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]);
|
||||
|
|
|
|||
Loading…
Reference in a new issue