Skip to content

fix: emit empty tool schema properties as JSON object#7

Merged
galatanovidiu merged 2 commits into
trunkfrom
fix/tool-schema-empty-properties
Jun 3, 2026
Merged

fix: emit empty tool schema properties as JSON object#7
galatanovidiu merged 2 commits into
trunkfrom
fix/tool-schema-empty-properties

Conversation

@galatanovidiu
Copy link
Copy Markdown
Contributor

Summary

  • ToolInputSchema::toArray() and ToolOutputSchema::toArray() now always emit the properties key on the wire, encoded as a JSON object ({}) when the tool declares no parameters.
  • Mirrored in the TypeScript generator so a clean regen reproduces the patched output.
  • generator/package-lock.json was refreshed by a clean npm install during the regen.

Why

Parameter-less tools previously produced one of two invalid wire shapes:

  • properties = null → key omitted entirely → {"type":"object"}
  • properties = [] → emitted as JSON array → "properties":[]

Strict JSON Schema validators reject both. The most visible failure is OpenAI strict function-calling mode:

Invalid schema for function '<tool>': In context=(), object schema missing properties.

Tools with real parameters serialize unchanged — only the empty/null case changes.

Implementation notes

In toArray(), the previous if ($this->properties !== null) gate was replaced with an unconditional emit:

$result['properties'] = !empty($this->properties)
    ? $this->properties
    : new \stdClass();

stdClass is used for the empty case so json_encode() produces {} rather than []. Field types (?array $properties) are unchanged; constructor and fromArray() are unchanged.

The TypeScript generator (generator/src/generators/dto.ts) emits this shape only for ToolInputSchema and ToolOutputSchema — every other DTO uses the original conditional emit.

Test plan

  • composer analyse (PHPStan level max) — clean
  • Manual: new ToolInputSchema() and new ToolInputSchema(null, []) both produce "properties":{} under json_encode(toArray())
  • Manual: new ToolInputSchema(null, ['name' => $obj], ['name']) round-trips with "properties":{"name":...}
  • Same checks for ToolOutputSchema
  • Generator regenerated cleanly (npm install && npm run build && node dist/cli/index.js generate -c config/2025-11-25.json); generated PHP matches the hand-edit

ToolInputSchema::toArray() and ToolOutputSchema::toArray() now always
emit the `properties` key, encoded as a JSON object even when the tool
declares no parameters.

Previously, parameter-less tools produced one of two invalid wire
shapes for strict JSON Schema validators:

  - properties = null  → key omitted entirely  ({"type":"object"})
  - properties = []    → emitted as JSON array ([])

Strict validators (notably OpenAI strict function-calling mode) reject
both with: `object schema missing properties`. Tools with real
parameters serialize unchanged.

The same fix is mirrored in the TypeScript generator
(generator/src/generators/dto.ts) so a clean regen reproduces the
patched output. generator/package-lock.json was refreshed by a clean
`npm install` during the regen.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2026

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.

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

Co-authored-by: galatanovidiu <ovidiu-galatan@git.wordpress.org>
Co-authored-by: gziolo <gziolo@git.wordpress.org>
Co-authored-by: JasonTheAdams <jason_the_adams@git.wordpress.org>

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

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the JSON Schema wire output for parameter-less tools by ensuring ToolInputSchema and ToolOutputSchema always serialize a properties key as an object ({}) instead of omitting it or emitting an empty array, which strict JSON Schema validators reject.

Changes:

  • Updated ToolInputSchema::toArray() and ToolOutputSchema::toArray() to always emit properties, using new \stdClass() for the empty case.
  • Updated the TypeScript DTO generator to mirror this behavior specifically for ToolInputSchema and ToolOutputSchema regenerations.
  • Refreshed generator/package-lock.json via a clean install (includes correcting the root package license metadata to match package.json).

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File Description
src/Server/Tools/DTO/ToolOutputSchema.php Always emits properties as {} when empty to satisfy strict JSON Schema validators.
src/Server/Tools/DTO/ToolInputSchema.php Same as above, for tool input schemas.
generator/src/generators/dto.ts Generator now forces properties to serialize as an object for ToolInputSchema/ToolOutputSchema only.
generator/package-lock.json Lockfile refreshed; root license metadata aligns with generator/package.json.
Files not reviewed (1)
  • generator/package-lock.json: Language not supported

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

@galatanovidiu galatanovidiu requested a review from gziolo May 4, 2026 07:01
Copy link
Copy Markdown
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Makes perfect sense 👍🏻

Comment thread generator/src/generators/dto.ts Outdated
Comment on lines +1162 to +1169
/**
* Tool input/output JSON Schemas must always emit an object-typed
* `properties` key on the wire (e.g. `{}`), not omit it or emit `[]`.
* Strict JSON-Schema validators (OpenAI strict function-calling) reject
* tools whose root object schema is missing `properties`.
*/
const forcePropertiesAsObject =
interfaceName === 'ToolInputSchema' || interfaceName === 'ToolOutputSchema';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@galatanovidiu I'm a little nervous about just having a hard-coded list like this tucked away in here. Do you think serializeAsObject or something like that could be a part of the PhpProperty interface or something like that, so it can be defined on a higher level?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did try the PhpProperty direction, and I also tried pushing it into configuration, but I think both versions overcomplicate this a bit

We would still be hardcoding the same two cases either way. The only real change is where that hardcoding lives. For something this small and specific, I’d rather keep it simple and explicit than hide it behind extra abstraction.

If you prefer, I can propose the configuration-based version instead.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I understand that it will be hard-coded somewhere, but at least it's in the context of the property, right? You have your head wrapped around this more than me, so take my feedback with a grain of salt. If there's a sane, configurable way this could work that puts the serialization decision closer to the thing that needs special treatment, then great. If not, this does work and isn't likely to change often.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@JasonTheAdams I finally had time to get back to this problem.

That makes sense. I moved this onto the property metadata as serializeEmptyAsObject, so renderToArray() no longer knows about ToolInputSchema / ToolOutputSchema directly.

The only hardcoded part now is where we resolve the PHP property and tag exactly ToolInputSchema.properties and ToolOutputSchema.properties. That felt like the smallest way to bring the decision closer to the property without adding a wider config layer.

Does that match what you had in mind?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like this better. Thank you! 😄

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • generator/package-lock.json: Language not supported

@galatanovidiu galatanovidiu merged commit 1d1936f into trunk Jun 3, 2026
10 of 12 checks passed
@galatanovidiu galatanovidiu deleted the fix/tool-schema-empty-properties branch June 3, 2026 16:10
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.

4 participants