[Schema] Always emit {} for empty tool schema properties (#407)
* [Schema] Always emit {} for empty tool schema properties
Normalize empty properties arrays (including nested object schemas and
outputSchema) in Tool::__construct so tools/list never serializes
invalid JSON Schema properties: [].
Fixes #405
* [Schema] Normalize every empty sub-schema, not only properties
Walking into `items` and `additionalProperties` without normalizing an empty
sub-schema at those positions left the `[]`-instead-of-`{}` bug intact one
level down. Most notably it regressed #151 on a `Tool::fromArray()`
round-trip: `SchemaGenerator` emits `items: {}` for untyped array parameters,
which decodes to `[]` and was re-serialized as `[]` — the exact schema strict
clients reject.
Empty sub-schemas are now replaced with a `\stdClass` at every schema
position, and the keyword list is completed to cover draft-07 through
2020-12: `$defs`, `definitions`, `dependentSchemas`, `patternProperties`,
`propertyNames`, `contains`, `not`, `if`/`then`/`else`, `additionalItems`,
and `unevaluatedItems`/`unevaluatedProperties`. Keywords holding plain JSON
arrays (`allOf: []`, `enum: []`, `dependentRequired`) keep encoding as `[]`.
---------
Co-authored-by: Christopher Hertel <mail@christopher-hertel.de>
This commit is contained in:
Björn Meyer
GitHub
parent
a987a3a248
commit
9236824b6d
1 changed files with 327 additions and 12 deletions
|
|
@ -97,4 +97,196 @@ class ToolTest extends TestCase
|
|||
$this->assertSame($original->name, $restored->name);
|
||||
$this->assertSame($original->description, $restored->description);
|
||||
}
|
||||
|
||||
public function testConstructorNormalizesEmptyInputSchemaPropertiesToObject(): void
|
||||
{
|
||||
$tool = new Tool(
|
||||
name: 'no_params',
|
||||
title: null,
|
||||
inputSchema: ['type' => 'object', 'properties' => [], 'required' => null],
|
||||
description: null,
|
||||
annotations: null,
|
||||
);
|
||||
|
||||
$this->assertInstanceOf(\stdClass::class, $tool->inputSchema['properties']);
|
||||
$this->assertSame('{"name":"no_params","inputSchema":{"type":"object","properties":{},"required":null}}', json_encode($tool));
|
||||
}
|
||||
|
||||
public function testConstructorNormalizesEmptyPropertiesAfterJsonDecodeRoundTrip(): void
|
||||
{
|
||||
/** @var array{type: 'object', properties: array<string, mixed>, required: null} $schema */
|
||||
$schema = json_decode('{"type":"object","properties":{},"required":null}', true);
|
||||
$this->assertSame([], $schema['properties']);
|
||||
|
||||
$tool = new Tool('t', null, $schema, null, null);
|
||||
|
||||
$this->assertInstanceOf(\stdClass::class, $tool->inputSchema['properties']);
|
||||
$this->assertStringContainsString('"properties":{}', (string) json_encode($tool));
|
||||
}
|
||||
|
||||
public function testFromArrayNormalizesNestedEmptyPropertiesRecursively(): void
|
||||
{
|
||||
$tool = Tool::fromArray([
|
||||
'name' => 't',
|
||||
'inputSchema' => [
|
||||
'type' => 'object',
|
||||
'properties' => [
|
||||
'filter' => ['type' => 'object', 'properties' => []],
|
||||
],
|
||||
'required' => null,
|
||||
],
|
||||
]);
|
||||
|
||||
$this->assertInstanceOf(\stdClass::class, $tool->inputSchema['properties']['filter']['properties']);
|
||||
$this->assertStringContainsString('"properties":{}', (string) json_encode($tool->inputSchema['properties']['filter']));
|
||||
}
|
||||
|
||||
public function testConstructorNormalizesEmptyOutputSchemaProperties(): void
|
||||
{
|
||||
$tool = new Tool(
|
||||
name: 't',
|
||||
title: null,
|
||||
inputSchema: ['type' => 'object', 'properties' => ['q' => ['type' => 'string']], 'required' => null],
|
||||
description: null,
|
||||
annotations: null,
|
||||
outputSchema: ['type' => 'object', 'properties' => []],
|
||||
);
|
||||
|
||||
$this->assertInstanceOf(\stdClass::class, $tool->outputSchema['properties']);
|
||||
$this->assertStringContainsString('"outputSchema":{"type":"object","properties":{}}', (string) json_encode($tool));
|
||||
}
|
||||
|
||||
public function testConstructorNormalizesEmptyPropertiesInsideArrayItems(): void
|
||||
{
|
||||
$tool = new Tool(
|
||||
name: 't',
|
||||
title: null,
|
||||
inputSchema: [
|
||||
'type' => 'object',
|
||||
'properties' => [
|
||||
'rows' => [
|
||||
'type' => 'array',
|
||||
'items' => ['type' => 'object', 'properties' => []],
|
||||
],
|
||||
],
|
||||
'required' => null,
|
||||
],
|
||||
description: null,
|
||||
annotations: null,
|
||||
);
|
||||
|
||||
$this->assertInstanceOf(\stdClass::class, $tool->inputSchema['properties']['rows']['items']['properties']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Each case is a schema that is already valid JSON: decoding it collapses every `{}`
|
||||
* to `[]`, and normalization has to restore it verbatim.
|
||||
*
|
||||
* @return iterable<string, array{string}>
|
||||
*/
|
||||
public static function emptySubSchemaProvider(): iterable
|
||||
{
|
||||
yield 'top-level properties' => ['{"type":"object","properties":{}}'];
|
||||
yield 'nested properties' => ['{"type":"object","properties":{"filter":{"type":"object","properties":{}}}}'];
|
||||
yield 'property schema' => ['{"type":"object","properties":{"anything":{}}}'];
|
||||
yield 'items schema' => ['{"type":"object","properties":{"tags":{"type":"array","items":{}}}}'];
|
||||
yield 'tuple items schema' => ['{"type":"object","properties":{"pair":{"type":"array","items":[{"type":"object","properties":{}},{}]}}}'];
|
||||
yield 'additionalItems schema' => ['{"type":"object","properties":{"tags":{"type":"array","additionalItems":{}}}}'];
|
||||
yield 'additionalProperties schema' => ['{"type":"object","properties":{"map":{"type":"object","additionalProperties":{}}}}'];
|
||||
yield 'propertyNames schema' => ['{"type":"object","properties":{"map":{"type":"object","propertyNames":{}}}}'];
|
||||
yield 'contains schema' => ['{"type":"object","properties":{"tags":{"type":"array","contains":{}}}}'];
|
||||
yield 'unevaluatedItems schema' => ['{"type":"object","properties":{"tags":{"type":"array","unevaluatedItems":{}}}}'];
|
||||
yield 'unevaluatedProperties schema' => ['{"type":"object","properties":{"map":{"type":"object","unevaluatedProperties":{}}}}'];
|
||||
yield 'not schema' => ['{"type":"object","properties":{"a":{"not":{}}}}'];
|
||||
yield 'if/then/else schemas' => ['{"type":"object","properties":{"a":{"if":{},"then":{"type":"object","properties":{}},"else":{}}}}'];
|
||||
yield '$defs entry' => ['{"type":"object","properties":{"a":{"$ref":"#/$defs/E"}},"$defs":{"E":{"type":"object","properties":{}}}}'];
|
||||
yield 'definitions entry' => ['{"type":"object","properties":{"a":{"$ref":"#/definitions/E"}},"definitions":{"E":{}}}'];
|
||||
yield 'patternProperties entry' => ['{"type":"object","properties":{"map":{"type":"object","patternProperties":{"^x":{}}}}}'];
|
||||
yield 'dependentSchemas entry' => ['{"type":"object","properties":{"a":{"type":"string"}},"dependentSchemas":{"a":{}}}'];
|
||||
yield 'combinator branches' => ['{"type":"object","properties":{"a":{"anyOf":[{},{"type":"object","properties":{}}],"oneOf":[{}],"allOf":[{}]}}}'];
|
||||
yield 'prefixItems entry' => ['{"type":"object","properties":{"a":{"type":"array","prefixItems":[{},{"type":"object","properties":{}}]}}}'];
|
||||
}
|
||||
|
||||
#[DataProvider('emptySubSchemaProvider')]
|
||||
public function testConstructorNormalizesEmptySubSchemas(string $schemaJson): void
|
||||
{
|
||||
/** @var array{type: 'object', properties: array<string, mixed>, required: string[]|null} $schema */
|
||||
$schema = json_decode($schemaJson, true, 512, \JSON_THROW_ON_ERROR);
|
||||
|
||||
$tool = new Tool(name: 't', title: null, inputSchema: $schema, description: null, annotations: null);
|
||||
|
||||
$this->assertSame($schemaJson, json_encode($tool->inputSchema, \JSON_UNESCAPED_SLASHES));
|
||||
}
|
||||
|
||||
#[DataProvider('emptySubSchemaProvider')]
|
||||
public function testConstructorNormalizesEmptySubSchemasInOutputSchema(string $schemaJson): void
|
||||
{
|
||||
/** @var array{type: 'object', properties?: array<string, mixed>} $schema */
|
||||
$schema = json_decode($schemaJson, true, 512, \JSON_THROW_ON_ERROR);
|
||||
|
||||
$tool = new Tool(
|
||||
name: 't',
|
||||
title: null,
|
||||
inputSchema: self::validInputSchema(),
|
||||
description: null,
|
||||
annotations: null,
|
||||
outputSchema: $schema,
|
||||
);
|
||||
|
||||
$this->assertSame($schemaJson, json_encode($tool->outputSchema, \JSON_UNESCAPED_SLASHES));
|
||||
}
|
||||
|
||||
/**
|
||||
* @return iterable<string, array{string}>
|
||||
*/
|
||||
public static function preservedEmptyArrayProvider(): iterable
|
||||
{
|
||||
yield 'empty combinator list' => ['{"type":"object","properties":{},"allOf":[]}'];
|
||||
yield 'empty prefixItems list' => ['{"type":"object","properties":{},"prefixItems":[]}'];
|
||||
yield 'empty required list' => ['{"type":"object","properties":{},"required":[]}'];
|
||||
yield 'empty enum list' => ['{"type":"object","properties":{"a":{"enum":[]}}}'];
|
||||
yield 'empty dependentRequired list' => ['{"type":"object","properties":{"a":{}},"dependentRequired":{"a":[]}}'];
|
||||
}
|
||||
|
||||
/**
|
||||
* Keywords that hold JSON arrays — not sub-schemas — must keep encoding as `[]`.
|
||||
*/
|
||||
#[DataProvider('preservedEmptyArrayProvider')]
|
||||
public function testConstructorLeavesNonSchemaEmptyArraysAlone(string $schemaJson): void
|
||||
{
|
||||
/** @var array{type: 'object', properties: array<string, mixed>, required: string[]|null} $schema */
|
||||
$schema = json_decode($schemaJson, true, 512, \JSON_THROW_ON_ERROR);
|
||||
|
||||
$tool = new Tool(name: 't', title: null, inputSchema: $schema, description: null, annotations: null);
|
||||
|
||||
$this->assertSame($schemaJson, json_encode($tool->inputSchema, \JSON_UNESCAPED_SLASHES));
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression test for #151: `SchemaGenerator` emits `items: {}` for untyped arrays,
|
||||
* but a client decoding that payload gets `items: []` back — re-serializing it used to
|
||||
* hand strict clients the very schema #151 fixed.
|
||||
*/
|
||||
public function testFromArrayRoundTripPreservesEmptyItemsSchema(): void
|
||||
{
|
||||
$tool = new Tool(
|
||||
name: 't',
|
||||
title: null,
|
||||
inputSchema: [
|
||||
'type' => 'object',
|
||||
'properties' => ['tags' => ['type' => 'array', 'items' => new \stdClass()]],
|
||||
'required' => null,
|
||||
],
|
||||
description: null,
|
||||
annotations: null,
|
||||
);
|
||||
|
||||
$wire = (string) json_encode($tool);
|
||||
$this->assertStringContainsString('"items":{}', $wire);
|
||||
|
||||
/** @var array{name: string, inputSchema: array{type: 'object', properties: array<string, mixed>, required: string[]|null}} $decoded */
|
||||
$decoded = json_decode($wire, true, 512, \JSON_THROW_ON_ERROR);
|
||||
|
||||
$this->assertSame($wire, json_encode(Tool::fromArray($decoded)));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue