From cfe5e8f570af67a4147fcfb4bdcc5835842193da Mon Sep 17 00:00:00 2001 From: Christopher Hertel Date: Tue, 18 Aug 2026 15:09:58 +0200 Subject: [PATCH] [Capability] Apply a parameter's complete schema definition (#435) * [Capability] Apply a parameter's complete schema definition Schema(definition:) is documented as taking precedence over the individual keywords, and generate() unwraps it that way at method level. At parameter level it was merged as an ordinary key instead, in both the plain and the variadic path, so the property reached clients carrying a literal "definition" member - not JSON Schema, read by nothing, and everything inside it lost. Both paths now replace the schema with the definition. The variadic one still forces type: array afterwards, since a variadic arrives as an array however the definition describes it, and the signature default survives a definition that states none - it is what the handler receives when the argument is omitted, and default is an annotation rather than a constraint. * Fix docblock description overwriting definition on variadic params * Apply suggestion from @chr-hertel --- src/Capability/Discovery/SchemaGenerator.php | 33 +++++++++--- .../Discovery/SchemaGeneratorFixture.php | 45 ++++++++++++++++ .../Discovery/SchemaGeneratorTest.php | 52 +++++++++++++++++++ 3 files changed, 122 insertions(+), 8 deletions(-) diff --git a/src/Capability/Discovery/SchemaGenerator.php b/src/Capability/Discovery/SchemaGenerator.php index 3d9f8aa..673477d 100644 --- a/src/Capability/Discovery/SchemaGenerator.php +++ b/src/Capability/Discovery/SchemaGenerator.php @@ -237,7 +237,17 @@ final class SchemaGenerator implements SchemaGeneratorInterface // Parameter-level takes highest precedence $parameterLevelSchema = $paramInfo['parameter_schema']; if (!empty($parameterLevelSchema)) { - $mergedSchema = array_merge($mergedSchema, $parameterLevelSchema); + // A complete definition replaces the schema, as it does at method level. + if (isset($parameterLevelSchema['definition']) && \is_array($parameterLevelSchema['definition'])) { + $mergedSchema = $parameterLevelSchema['definition']; + + // The default comes from the signature, not from the schema. + if (!\array_key_exists('default', $mergedSchema) && \array_key_exists('default', $inferredSchema)) { + $mergedSchema['default'] = $inferredSchema['default']; + } + } else { + $mergedSchema = array_merge($mergedSchema, $parameterLevelSchema); + } } // Run after all merges so that when a Schema attribute reshapes the parameter @@ -324,17 +334,24 @@ final class SchemaGenerator implements SchemaGeneratorInterface { $paramSchema = ['type' => 'array']; - // Apply parameter-level Schema attributes first - if (!empty($paramInfo['parameter_schema'])) { - $paramSchema = array_merge($paramSchema, $paramInfo['parameter_schema']); - // Ensure type is always array for variadic - $paramSchema['type'] = 'array'; - } - + // Add description from docblock before parameter-level schema is merged in, + // so a definition's own description takes precedence (matches buildParameterSchema). if ($paramInfo['description']) { $paramSchema['description'] = $paramInfo['description']; } + // Apply parameter-level Schema attributes first + if (!empty($paramInfo['parameter_schema'])) { + $parameterLevelSchema = $paramInfo['parameter_schema']; + + $paramSchema = isset($parameterLevelSchema['definition']) && \is_array($parameterLevelSchema['definition']) + ? $parameterLevelSchema['definition'] + : array_merge($paramSchema, $parameterLevelSchema); + + // Ensure type is always array for variadic + $paramSchema['type'] = 'array'; + } + // If no items specified by Schema attribute, infer from type if (!isset($paramSchema['items'])) { $itemJsonTypes = $this->mapPhpTypeToJsonSchemaType($paramInfo['type_string']); diff --git a/tests/Unit/Capability/Discovery/SchemaGeneratorFixture.php b/tests/Unit/Capability/Discovery/SchemaGeneratorFixture.php index 548c5d9..14f2d8f 100644 --- a/tests/Unit/Capability/Discovery/SchemaGeneratorFixture.php +++ b/tests/Unit/Capability/Discovery/SchemaGeneratorFixture.php @@ -130,6 +130,30 @@ class SchemaGeneratorFixture // ===== PARAMETER-LEVEL SCHEMA SCENARIOS ===== + /** + * Parameter-level Schema with complete definition. + */ + public function parameterLevelCompleteDefinition( + #[Schema(definition: [ + 'type' => 'string', + 'description' => 'The region to query.', + 'x-mcp-header' => 'Region', + ])] + string $region = 'eu', + #[Schema(definition: ['type' => 'integer', 'minimum' => 1, 'maximum' => 10])] + int $limit = 3, + ): void { + } + + /** + * Parameter-level definition that reshapes the parameter's type. + */ + public function definitionReshapingTheParameter( + #[Schema(definition: ['type' => 'object', 'properties' => ['id' => ['type' => 'integer']]])] + string $payload = '{}', + ): void { + } + /** * Parameter-level Schema attributes only. */ @@ -326,6 +350,27 @@ class SchemaGeneratorFixture // ===== VARIADIC SCENARIOS ===== + /** + * Variadic parameter with a complete definition. + */ + public function variadicCompleteDefinition( + #[Schema(definition: ['type' => 'array', 'description' => 'Tags to apply.', 'items' => ['type' => 'string']])] + string ...$tags, + ): void { + } + + /** + * Variadic parameter with both a docblock description and a complete definition; + * the definition's own description must win. + * + * @param string ...$tags Docblock description that must lose to the definition's own + */ + public function variadicCompleteDefinitionWithDocblockDescription( + #[Schema(definition: ['type' => 'array', 'description' => 'Tags to apply.', 'items' => ['type' => 'string']])] + string ...$tags, + ): void { + } + /** * Variadic parameter scenarios. * diff --git a/tests/Unit/Capability/Discovery/SchemaGeneratorTest.php b/tests/Unit/Capability/Discovery/SchemaGeneratorTest.php index b5dbb09..51d73d2 100644 --- a/tests/Unit/Capability/Discovery/SchemaGeneratorTest.php +++ b/tests/Unit/Capability/Discovery/SchemaGeneratorTest.php @@ -92,6 +92,58 @@ final class SchemaGeneratorTest extends TestCase ], $schema); } + public function testUsesCompleteSchemaDefinitionFromParameterLevelSchemaAttribute(): void + { + $method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'parameterLevelCompleteDefinition'); + $schema = $this->schemaGenerator->generate($method); + + $this->assertEquals([ + 'type' => 'string', + 'description' => 'The region to query.', + 'x-mcp-header' => 'Region', + 'default' => 'eu', + ], $schema['properties']['region']); + + $this->assertEquals([ + 'type' => 'integer', + 'minimum' => 1, + 'maximum' => 10, + 'default' => 3, + ], $schema['properties']['limit']); + + $this->assertArrayNotHasKey('required', $schema); + } + + public function testUsesCompleteSchemaDefinitionFromVariadicParameter(): void + { + $method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'variadicCompleteDefinition'); + $schema = $this->schemaGenerator->generate($method); + + // The array type is forced: a variadic always arrives as one. + $this->assertEquals([ + 'type' => 'array', + 'description' => 'Tags to apply.', + 'items' => ['type' => 'string'], + ], $schema['properties']['tags']); + } + + public function testDefinitionDescriptionWinsOverDocblockOnVariadicParameter(): void + { + $method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'variadicCompleteDefinitionWithDocblockDescription'); + $schema = $this->schemaGenerator->generate($method); + + $this->assertSame('Tags to apply.', $schema['properties']['tags']['description']); + } + + public function testKeepsTheSignatureDefaultWhenADefinitionReshapesTheParameter(): void + { + $method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'definitionReshapingTheParameter'); + $schema = $this->schemaGenerator->generate($method); + + $this->assertSame('{}', $schema['properties']['payload']['default']); + $this->assertSame('object', $schema['properties']['payload']['type']); + } + public function testGeneratesSchemaFromMethodLevelSchemaAttributeWithProperties(): void { $method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'methodLevelWithProperties');