fix(jsonschema): align nullability with JSON Schema 2020-12 - #1518
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR migrates JSON Schema nullability representation from OpenAPI-style ChangesJSON Schema 2020-12 Nullability Format
🎯 3 (Moderate) | ⏱️ ~25 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 docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Comment |
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.
The generator previously expressed nullable fields with the OpenAPI 3.0
keyword `"nullable": true`. Standard JSON Schema 2020-12 validators (e.g.
santhosh-tekuri/jsonschema, used by Cosmo's router) silently ignore
unknown keywords, so a payload containing `null` for an otherwise valid
optional field is rejected at the schema-validation boundary. LLMs that
legitimately emit `null` for absent optional fields (e.g. OpenAI's GPT-4o
in tool-call arguments) then have their output rejected before reaching
the resolver — same failure pattern as ENG-9631, different root cause.
Switch to JSON Schema 2020-12:
- typed schemas: "type": [<type>, "null"]
- enum schemas: null appended to "enum"
- $ref schemas: {"anyOf": [{"$ref": ...}, {"type": "null"}]}
- the "nullable" keyword is no longer emitted.
The internal Nullable field is retained for construction-time logic; only
the JSON serialization changes. Definition bodies under $defs are now
marked non-nullable explicitly, so the def carries the single canonical
type and per-use-site nullability is applied via the surrounding ref.
Adds a regression test that validates explicit-null payloads against the
generated schema for nullable scalar, enum, and recursive ref fields,
and updates the existing golden-JSON tests to assert the 2020-12 shape.
d0ad76b to
6bc6bfc
Compare
🤖 I have created a release *beep* *boop* --- ## [2.4.3](v2.4.2...v2.4.3) (2026-06-05) ### Bug Fixes * calculate costs for abstract fields without double counting ([#1521](#1521)) ([4175a9e](4175a9e)) * **jsonschema:** align nullability with JSON Schema 2020-12 ([#1518](#1518)) ([6fcdf8c](6fcdf8c)) --- 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>
…ct" (#1528) 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: ```go if len(v.schema.Required) > 0 { v.schema.Nullable = false } ``` 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's `AddTool` requires the input schema `type` to be exactly `"object"` and panics: ``` panic: AddTool "list_employees": input schema must have type "object" (got [object null]) ``` This breaks the cosmo router engine bump 2.4.2 -> 2.4.3 (wundergraph/cosmo#2925): `pkg/mcpserver`, `protocol` and `security` suites all panic. ## Fix 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. Golden tests updated (root `["object","null"]` -> `"object"`); the `root_schema_nullable_based_on_required_arguments` subtest renamed to `root 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 local `replace`: `pkg/mcpserver` no longer panics. ## Follow-up Needs a patch release (2.4.4) and a corresponding engine bump in cosmo#2925. Fixes ENG-9682
Problem
The generator emits the OpenAPI 3.0 keyword
"nullable": truefor nullable fields.Standard JSON Schema 2020-12 validators (e.g.
santhosh-tekuri/jsonschema, used by Cosmo's router) silently ignore unknown keywords, so a payload containingnullfor an otherwise-valid optional field is rejected at the schema-validation boundary.LLMs that legitimately emit
nullfor absent optional fields - OpenAI'sgpt-4odoes this routinely in tool-call arguments - then have their output rejected before reaching the resolver.Same failure pattern as #1513 (ENG-9631) but different root cause.
Discovered while building the provider-compatibility evidence for that PR.
Fix
Switch nullability to the JSON Schema 2020-12 form:
{"type": "string", "nullable": true}{"type": ["string", "null"]}{"type": "string", "enum": [...], "nullable": true}{"type": ["string", "null"], "enum": [..., null]}$ref{"$ref": "...", "nullable": true}{"anyOf": [{"$ref": "..."}, {"type": "null"}]}The
"nullable"key is no longer emitted. The internalNullablefield is retained for construction-time logic; only the JSON serialization changes. Definition bodies under$defsare now marked non-nullable explicitly so the def carries the single canonical type and per-use-site nullability is applied via the surrounding ref.Tests
nullable_2020_12_test.go: validates explicit-null payloads against the generated schema for nullable scalar, enum, and recursive ref fields (red before, green after).schema_test.goandvariables_schema_test.goupdated to assert the 2020-12 shape.All package tests pass;
gofmtclean;go vetclean.