fix(jsonschema): root variables object is always a non-nullable "object" - #1528
Conversation
#1518 was correct but incomplete. Switching nullability from the OpenAPI 3.0 "nullable" keyword to the JSON Schema 2020-12 form was right, but it exposed a latent defect: GetSchema() only forced the root variables object non-nullable when the operation had a required variable. Operations with all-optional variables kept a nullable root, which previously serialized to {"type":"object","nullable":true} and was harmless because validators ignore the unknown "nullable" keyword. After #1518 it serializes to {"type":["object","null"]}, which strict consumers reject — the MCP go-sdk's AddTool requires the input schema type to be exactly "object" and panics otherwise: panic: AddTool "list_employees": input schema must have type "object" (got [object null]) This broke the cosmo router engine bump 2.4.2 -> 2.4.3 (pkg/mcpserver, protocol and security suites). The root variables object is always a concrete object — the container is present or omitted, never the JSON literal null — so set Nullable=false unconditionally in GetSchema(). This is consistent with nested input-object variables, which are already forced non-nullable. Only individual optional fields remain nullable. Update the affected golden tests and rename the root_schema_nullable_based_on_required_arguments subtest accordingly. Fixes ENG-9682
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
Too many files changed? Review this PR in Change Stack to see how the pieces fit before you dive in. 📝 WalkthroughWalkthroughThe root variables JSON schema is now unconditionally non-nullable in the ChangesRoot Variables Schema Non-Nullable Enforcement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🤖 I have created a release *beep* *boop* --- ## [2.4.4](v2.4.3...v2.4.4) (2026-06-08) ### Bug Fixes * fallback to abstract name if no __typename was sent ([#1527](#1527)) ([2548d5a](2548d5a)) * **jsonschema:** root variables object is always a non-nullable "object" ([#1528](#1528)) ([8327b53](8327b53)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: wundergraph-bot[bot] <285992168+wundergraph-bot[bot]@users.noreply.github.com>
Follow-up to #1518. That PR was correct but incomplete and exposed an old defect.
Problem
GetSchema()only forced the root variables object non-nullable when the operation had a required variable:Operations with all-optional variables kept a nullable root. Before #1518 that serialized to
{"type":"object","nullable":true}and was harmless (validators ignore the unknown"nullable"keyword). After #1518 it serializes to{"type":["object","null"]}, which strict consumers reject - the MCP go-sdk'sAddToolrequires the input schematypeto be exactly"object"and panics:This breaks the cosmo router engine bump 2.4.2 -> 2.4.3 (wundergraph/cosmo#2925):
pkg/mcpserver,protocolandsecuritysuites all panic.Fix
The root variables object is always a concrete object - the container is present or omitted, never the JSON literal
null- so setNullable = falseunconditionally inGetSchema(). This is consistent with nested input-object variables, which are already forced non-nullable. Only individual optional fields remain nullable.Golden tests updated (root
["object","null"]->"object"); theroot_schema_nullable_based_on_required_argumentssubtest renamed toroot schema is always a non-nullable object.Tests
go test ./pkg/engine/jsonschema/passes; gofmt and go vet clean. Verified against the cosmo router via a localreplace:pkg/mcpserverno longer panics.Follow-up
Needs a patch release (2.4.4) and a corresponding engine bump in cosmo#2925.
Fixes ENG-9682