From 0511024c78c9151eea76f7cb351b8ec83ea073c8 Mon Sep 17 00:00:00 2001 From: KristofersOzolinsMagebit Date: Wed, 19 Aug 2026 12:51:32 +0300 Subject: [PATCH] fix(mcp): fold non-string enums into descriptions for tools/list --- Model/JsonRpc/Handler/ToolsListHandler.php | 2 +- Model/Tool/SchemaSanitizer.php | 64 ++++++- Test/Unit/Model/Tool/SchemaSanitizerTest.php | 173 +++++++++++++++++++ 3 files changed, 235 insertions(+), 4 deletions(-) diff --git a/Model/JsonRpc/Handler/ToolsListHandler.php b/Model/JsonRpc/Handler/ToolsListHandler.php index 1e479a7..14699a5 100644 --- a/Model/JsonRpc/Handler/ToolsListHandler.php +++ b/Model/JsonRpc/Handler/ToolsListHandler.php @@ -88,7 +88,7 @@ public function handle(Request $request, AuthenticatedContext $context): Respons 'name' => str_replace('.', '_', $tool->getName()), 'title' => $displayTitle, 'description' => $tool->getDescription(), - 'inputSchema' => $this->schemaSanitizer->sanitize( + 'inputSchema' => $this->schemaSanitizer->sanitizeForClient( $tool->getName(), $tool->getInputSchema() ), diff --git a/Model/Tool/SchemaSanitizer.php b/Model/Tool/SchemaSanitizer.php index 7c3ef74..ca88df8 100644 --- a/Model/Tool/SchemaSanitizer.php +++ b/Model/Tool/SchemaSanitizer.php @@ -38,7 +38,22 @@ public function __construct( public function sanitize(string $toolName, array $schema): array { /** @var array $walked */ - $walked = $this->walk($toolName, $schema, ''); + $walked = $this->walk($toolName, $schema, '', false); + return $walked; + } + + /** + * Same as {@see sanitize()} plus the rewrites clients need. Only for what + * `tools/list` advertises — argument validation keeps the stricter schema. + * + * @param string $toolName + * @param array $schema + * @return array + */ + public function sanitizeForClient(string $toolName, array $schema): array + { + /** @var array $walked */ + $walked = $this->walk($toolName, $schema, '', true); return $walked; } @@ -46,9 +61,10 @@ public function sanitize(string $toolName, array $schema): array * @param string $toolName * @param mixed $node * @param string $path + * @param bool $forClient * @return mixed */ - private function walk(string $toolName, mixed $node, string $path): mixed + private function walk(string $toolName, mixed $node, string $path, bool $forClient): mixed { if (!is_array($node)) { return $node; @@ -68,6 +84,10 @@ private function walk(string $toolName, mixed $node, string $path): mixed } } + if ($forClient) { + $node = $this->foldNonStringEnum($node); + } + $cleaned = []; foreach ($node as $key => $value) { $childPath = $path === '' ? (string) $key : $path . '.' . $key; @@ -75,8 +95,46 @@ private function walk(string $toolName, mixed $node, string $path): mixed $cleaned[$key] = new stdClass(); continue; } - $cleaned[$key] = $this->walk($toolName, $value, $childPath); + $cleaned[$key] = $this->walk($toolName, $value, $childPath, $forClient); } return $cleaned; } + + /** + * Replaces an `enum` holding non-string values with a description of them. + * + * @param array $node + * @return array + */ + private function foldNonStringEnum(array $node): array + { + $values = $node['enum'] ?? null; + if (!is_array($values) || !array_is_list($values) || $values === []) { + return $node; + } + foreach ($values as $value) { + if (!is_string($value)) { + unset($node['enum']); + $node['description'] = $this->describeValues($node['description'] ?? null, $values); + return $node; + } + } + return $node; + } + + /** + * @param mixed $description + * @param array $values + * @return string + */ + private function describeValues(mixed $description, array $values): string + { + $encoded = array_map( + static fn (mixed $value): string => (string) json_encode($value, JSON_UNESCAPED_SLASHES), + $values + ); + $sentence = sprintf('Allowed values: %s.', implode(', ', $encoded)); + $existing = is_string($description) ? trim($description) : ''; + return $existing === '' ? $sentence : $existing . ' ' . $sentence; + } } diff --git a/Test/Unit/Model/Tool/SchemaSanitizerTest.php b/Test/Unit/Model/Tool/SchemaSanitizerTest.php index 4a158ed..4dbb2f3 100644 --- a/Test/Unit/Model/Tool/SchemaSanitizerTest.php +++ b/Test/Unit/Model/Tool/SchemaSanitizerTest.php @@ -174,4 +174,177 @@ public function testEmptyPropertiesNormalizedAtNestedDepth(): void self::assertIsArray($address); self::assertInstanceOf(\stdClass::class, $address['properties']); } + + public function testDropsIntegerEnumAndFoldsValuesIntoDescription(): void + { + $schema = [ + 'type' => 'object', + 'properties' => [ + 'status' => [ + 'type' => 'integer', + 'enum' => [1, 2], + 'description' => 'Product status.', + ], + ], + 'required' => ['status'], + ]; + + $expected = [ + 'type' => 'object', + 'properties' => [ + 'status' => [ + 'type' => 'integer', + 'description' => 'Product status. Allowed values: 1, 2.', + ], + ], + 'required' => ['status'], + ]; + + self::assertSame($expected, $this->sanitizer->sanitizeForClient('test.tool', $schema)); + } + + public function testValidationSchemaKeepsIntegerEnum(): void + { + $schema = [ + 'type' => 'object', + 'properties' => [ + 'status' => ['type' => 'integer', 'enum' => [1, 2]], + ], + ]; + + self::assertSame($schema, $this->sanitizer->sanitize('test.tool', $schema)); + } + + public function testDroppedEnumWithoutDescriptionGetsOne(): void + { + $schema = [ + 'type' => 'object', + 'properties' => [ + 'visibility' => ['type' => 'integer', 'enum' => [1, 2, 3, 4]], + ], + ]; + + $expected = [ + 'type' => 'object', + 'properties' => [ + 'visibility' => [ + 'type' => 'integer', + 'description' => 'Allowed values: 1, 2, 3, 4.', + ], + ], + ]; + + self::assertSame($expected, $this->sanitizer->sanitizeForClient('test.tool', $schema)); + } + + public function testKeepsStringEnumUntouched(): void + { + $schema = [ + 'type' => 'object', + 'properties' => [ + 'sort_dir' => [ + 'type' => 'string', + 'enum' => ['asc', 'desc'], + 'description' => 'Sort direction.', + ], + ], + ]; + + self::assertSame($schema, $this->sanitizer->sanitizeForClient('test.tool', $schema)); + } + + public function testDropsEnumNestedInsideArrayItems(): void + { + $schema = [ + 'type' => 'object', + 'properties' => [ + 'items' => [ + 'type' => 'array', + 'items' => [ + 'type' => 'object', + 'properties' => [ + 'sku' => ['type' => 'string'], + 'backorders' => [ + 'type' => 'integer', + 'enum' => [0, 1, 2], + 'description' => '0 = no, 1 = allow.', + ], + ], + 'required' => ['sku'], + ], + ], + ], + 'required' => ['items'], + ]; + + $expected = [ + 'type' => 'object', + 'properties' => [ + 'items' => [ + 'type' => 'array', + 'items' => [ + 'type' => 'object', + 'properties' => [ + 'sku' => ['type' => 'string'], + 'backorders' => [ + 'type' => 'integer', + 'description' => '0 = no, 1 = allow. Allowed values: 0, 1, 2.', + ], + ], + 'required' => ['sku'], + ], + ], + ], + 'required' => ['items'], + ]; + + self::assertSame($expected, $this->sanitizer->sanitizeForClient('test.tool', $schema)); + } + + public function testDropsEnumMixingStringsAndNumbers(): void + { + $schema = [ + 'type' => 'object', + 'properties' => [ + 'mode' => ['type' => 'string', 'enum' => ['all', 0]], + ], + ]; + + $expected = [ + 'type' => 'object', + 'properties' => [ + 'mode' => [ + 'type' => 'string', + 'description' => 'Allowed values: "all", 0.', + ], + ], + ]; + + self::assertSame($expected, $this->sanitizer->sanitizeForClient('test.tool', $schema)); + } + + public function testLeavesPropertyNamedEnumAlone(): void + { + $schema = [ + 'type' => 'object', + 'properties' => [ + 'enum' => ['type' => 'string', 'description' => 'A field named enum.'], + ], + 'required' => ['enum'], + ]; + + self::assertSame($schema, $this->sanitizer->sanitizeForClient('test.tool', $schema)); + } + + public function testLeavesPropertyNamedEnumWithNumericKeywordsAlone(): void + { + $schema = [ + 'type' => 'object', + 'properties' => [ + 'enum' => ['type' => 'integer', 'minimum' => 1], + ], + ]; + + self::assertSame($schema, $this->sanitizer->sanitizeForClient('test.tool', $schema)); + } }