Skip to content

Fix: Add placeholder property for parameterless ability schemas#174

Open
lbbms wants to merge 1 commit into
WordPress:trunkfrom
lbbms:fix/schema-transformer-empty-properties
Open

Fix: Add placeholder property for parameterless ability schemas#174
lbbms wants to merge 1 commit into
WordPress:trunkfrom
lbbms:fix/schema-transformer-empty-properties

Conversation

@lbbms

@lbbms lbbms commented Apr 23, 2026

Copy link
Copy Markdown

Summary

This PR updates SchemaTransformer to make parameterless ability schemas compatible with stricter MCP clients (notably Anthropic integrations) that reject type: object schemas when properties is missing.

Changes

1. Added a placeholder property constant

  • Introduced private const NOOP_PROPERTY = '_mcp_noop';
  • Used as an optional, no-op placeholder key for tools that otherwise have no input fields

2. Normalized empty/object schemas to always include properties

Changed all object-schema return paths in transform_to_object_schema() to go through a new helper:

  • Empty schema case (empty($schema))
  • No explicit type case (default to type: object)
  • Already object schema case (type === 'object')

All now call: self::ensure_object_properties($schema)

3. Added ensure_object_properties() helper

New private method with the following behavior:

  • If schema already has non-empty properties (array), it is returned unchanged
  • Otherwise, injects:
    • properties._mcp_noop with:
      • type: string
      • description: "Optional no-op placeholder for parameterless tools."
  • Ensures required exists and is an array
  • Does not require _mcp_noop (so no real behavioral change for callers)

4. Why this approach (instead of stdClass in schema)

A previous approach used new \stdClass() for empty properties, which helps JSON encoding but can be risky in internal DTO/validation paths expecting arrays.

This patch keeps everything array-based and client-safe:

  • inputSchema.properties is always present
  • DTO and validator flow remains consistent
  • No-op property is optional, so tool execution semantics are unchanged

Compatibility Impact

✅ Fixes clients that require object schemas to include properties
✅ Keeps existing tools with real input schemas untouched
✅ Parameterless tools now expose one optional placeholder input key, which is harmless and ignored unless explicitly sent

Rationale

This improves interoperability with strict MCP client schema validation while preserving adapter internals and tool behavior. It specifically addresses cases where parameterless WordPress abilities were transformed into { type: "object" } without properties, causing client-side tool registration failures.

- Add NOOP_PROPERTY constant for empty schemas
- Normalize empty/object schemas to always include properties
- Add ensure_object_properties() helper method
- Improve compatibility with strict MCP clients (e.g., Anthropic)

Fixes schema validation failures when parameterless WordPress abilities
are transformed into {type: 'object'} without properties.
Copilot AI review requested due to automatic review settings April 23, 2026 11:40
@github-actions

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @brummermann@brunsdigital.de.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: brummermann@brunsdigital.de.


To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates schema transformation so parameterless (or otherwise property-less) object schemas always include a properties object, improving compatibility with strict MCP clients that reject { "type": "object" } without properties.

Changes:

  • Added a _mcp_noop placeholder property constant for parameterless object schemas.
  • Routed object-schema return paths through a new ensure_object_properties() helper.
  • Added ensure_object_properties() to inject placeholder properties (and normalize required).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 46 to 55
// 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',
)
),
'was_transformed' => false,
'wrapper_property' => null,

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.
Comment on lines +90 to +92
* 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.

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.
Comment on lines +49 to 53
'schema' => self::ensure_object_properties(
array(
'type' => 'object',
)
),

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.
Comment on lines +99 to +104
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.',
),

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants