Skip to content

Fix: Add placeholder property for parameterless ability schemas #174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lbbms wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Fix: Add placeholder property for parameterless ability schemas #174

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 41 additions & 4 deletions includes/Domain/Utils/SchemaTransformer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@
* @package McpAdapter
*/
class SchemaTransformer {
/**
* No-op property placeholder for tools without input fields.
*
* @var string
*/
private const NOOP_PROPERTY = '_mcp_noop';

/**
* Transform a schema to MCP-compatible object format.
*
Expand All @@ -39,8 +46,10 @@ public static function transform_to_object_schema( ?array $schema, string $wrapp
// Handle null or empty schema - return minimal valid MCP object schema.
if ( empty( $schema ) ) {
return array(
'schema' => array(
'type' => 'object',
'schema' => self::ensure_object_properties(
array(
'type' => 'object',
)
),
Comment on lines +49 to 53

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

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

This changes the contract for null/empty schemas: instead of returning the minimal { type: 'object' }, it now injects placeholder properties (and required). The existing PHPUnit expectations in tests/Unit/Domain/Utils/SchemaTransformerTest.php for null/empty schemas (and empty properties stripping) will fail unless updated to match the new output shape.

Copilot uses AI. Check for mistakes.
'was_transformed' => false,
'wrapper_property' => null,
Comment on lines 46 to 55

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

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

The comment above normalize() ("strip empty properties" because PHP encodes empty arrays as []) is now misleading given this branch returns a schema with injected placeholder properties via ensure_object_properties(). Consider updating that earlier comment to reflect the new placeholder-injection strategy so future readers don’t assume properties are always removed from the final schema.

Copilot uses AI. Check for mistakes.
Expand All @@ -52,7 +61,7 @@ public static function transform_to_object_schema( ?array $schema, string $wrapp
$schema['type'] = 'object';

return array(
'schema' => $schema,
'schema' => self::ensure_object_properties( $schema ),
'was_transformed' => false,
'wrapper_property' => null,
);
Expand All @@ -61,7 +70,7 @@ public static function transform_to_object_schema( ?array $schema, string $wrapp
// If already an object type, return as-is
if ( 'object' === $schema['type'] ) {
return array(
'schema' => $schema,
'schema' => self::ensure_object_properties( $schema ),
'was_transformed' => false,
'wrapper_property' => null,
);
Expand All @@ -75,6 +84,34 @@ public static function transform_to_object_schema( ?array $schema, string $wrapp
);
}

/**
* Ensure an object schema has properties.
*
* If the schema already has non-empty properties, it is returned unchanged.
* Otherwise, it injects a no-op placeholder property to ensure the schema
* is a valid JSON object with at least one property.
Comment on lines +90 to +92

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

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

ensure_object_properties() PHPDoc says schemas with non-empty properties are returned “unchanged”, but the method will still mutate the schema by adding required: [] when required is missing or non-array. Either update the docstring to mention required normalization, or avoid injecting an empty required field unless it’s strictly necessary to preserve the “unchanged” contract for valid object schemas.

Suggested change
* If the schema already has non-empty properties, it is returned unchanged.
* Otherwise, it injects a no-op placeholder property to ensure the schema
* is a valid JSON object with at least one property.
* If the schema already has non-empty properties, they are preserved.
* Otherwise, it injects a no-op placeholder property to ensure the schema
* is a valid JSON object with at least one property. This method also
* normalizes the `required` field to an array when it is missing or invalid.

Copilot uses AI. Check for mistakes.
*
* @param array<string,mixed> $schema The object schema.
*
* @return array<string,mixed> The schema with properties ensured.
*/
private static function ensure_object_properties( array $schema ): array {
if ( ! isset( $schema['properties'] ) || ! is_array( $schema['properties'] ) || empty( $schema['properties'] ) ) {
$schema['properties'] = array(
self::NOOP_PROPERTY => array(
'type' => 'string',
'description' => 'Optional no-op placeholder for parameterless tools.',
),
Comment on lines +99 to +104

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

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

SchemaTransformer is also used by RegisterAbilityAsMcpPrompt to derive prompt arguments from input_schema. Injecting a _mcp_noop property here means abilities with an otherwise-parameterless object schema (e.g. { type: 'object' } / empty properties) will now surface a _mcp_noop prompt argument, which is a user-visible API change. Consider making placeholder injection opt-in (e.g., a flag on transform_to_object_schema), or have prompt argument conversion explicitly ignore the NOOP_PROPERTY key so prompts remain argument-less.

Copilot uses AI. Check for mistakes.
);
}

if ( ! isset( $schema['required'] ) || ! is_array( $schema['required'] ) ) {

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

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

ensure_object_properties() currently normalizes required by overwriting any non-array value with an empty array. This can silently mask invalid schemas (e.g., a mis-specified string/object required) that the existing MCP validation layer would otherwise report. Consider only defaulting required when it is missing, and leaving invalid types untouched so validators can surface a clear error.

Suggested change
if ( ! isset( $schema['required'] ) || ! is_array( $schema['required'] ) ) {
if ( ! array_key_exists( 'required', $schema ) ) {

Copilot uses AI. Check for mistakes.
$schema['required'] = array();
}

return $schema;
}

/**
* Wrap a flattened schema in an object structure.
*
Expand Down
Loading