Skip to content

fix: make generated JSON Schemas self-contained and strict-oneOf-safe#305

Open
joshmouch wants to merge 1 commit into
microsoft:mainfrom
joshmouch:pr/json-schema-generator-fix
Open

fix: make generated JSON Schemas self-contained and strict-oneOf-safe#305
joshmouch wants to merge 1 commit into
microsoft:mainfrom
joshmouch:pr/json-schema-generator-fix

Conversation

@joshmouch

Copy link
Copy Markdown
Contributor

Fixes #302.

Both bugs are fixed generator-wide, plus one scope correction and a latent crash surfaced along the way.

Bug 1: empty {} oneOf branch

A union type alias's printed type text carries a leading | (type X =\n | A\n | B), so splitUnionType yielded an empty first element that fell through typeTextToSchema to {}. Every union alias's oneOf therefore led with a matches-anything branch, so every instance matched two or more branches and strict oneOf validation always failed.

While fixing this I hit a pre-existing infinite recursion the union path could already trigger: an inline object with a nested union field (e.g. side?: 'before' | 'after') contains a |, so it entered the union branch; splitUnionType correctly kept it as a single part (the | is nested inside {}), and the single-part early-return re-processed the identical string forever. The union branch now fires only for a genuine top-level union (two or more parts) and otherwise falls through to the inline-object / reference handling.

Bug 2: dangling $refs (broader than the issue scopes it)

One correction on scope: the dangling-ref problem is not unique to notifications.schema.json. Every generated file has them. Counts before this PR:

file dangling $refs
state.schema.json 74
actions.schema.json 154
commands.schema.json 182
notifications.schema.json 95
errors.schema.json 103
total 608

And most of those (roughly 60-75% per file) are a second root cause the issue does not mention: the generator never walked enums. Any property typed as a bare const enum (SessionStatus, TurnState, ...) or an enum-member-literal discriminant (type: ActionType.AnnotationsEntryRemoved) hit the capitalized-identifier fallback in typeTextToSchema and was emitted as a $ref to a $def that was never defined, in any file. The remaining dangling refs were type aliases (URI, JsonPrimitive, ...) that only two of the five generators transitively collected.

The fix has three parts:

  • Enum-member accesses inline as { "const": <member value> } (so type: ActionType.Foo becomes { "const": "root/agentsChanged" } rather than a dangling ref).
  • A fixpoint resolveMissingDefs pass runs after each schema is built and backfills every referenced $def that is not yet present, resolving interfaces, type aliases (including URI = string), and whole enums (SessionStatus becomes { "type": "number", "enum": [1, 2, 8, 24, 32, 64] }, with its bitwise member values evaluated). It throws loudly if a referenced name resolves to none of those, so a future gap fails the build instead of silently emitting a dangling ref.
  • null union members now render as { "type": "null" } instead of {}.

I chose transitive inline collection (option A from the issue) rather than cross-file $refs (option B), since every existing schema file is already self-contained and I did not want to introduce a new cross-document architecture.

Result

After this PR, across all five files: 0 dangling $refs (was 608) and 0 empty {} oneOf branches (was 30).

There was no test coverage for the schema generator and no CI gate that would have caught any of this, so I added scripts/generate-json-schema.test.ts (wired into npm test) asserting that no generated schema has an empty {} oneOf branch or a dangling $ref. It is dependency-free (no new packages) and fails if either bug is reintroduced.

npm test passes (290 tests, typecheck, lint, changelog + release-metadata gates). All five schema/*.json files are regenerated in this PR.

The JSON Schema generator (scripts/generate-json-schema.ts) produced two
classes of invalid output in every schema/*.json file:

1. Every union type alias emitted a leading empty `{}` branch in its `oneOf`.
   A union alias's printed type text carries a leading `|`, so splitUnionType
   yielded an empty first element that fell through to `{}`. Because `{}`
   matches any value, every instance matched two or more branches and strict
   `oneOf` validation always failed. This also exposed a latent infinite
   recursion: an inline object with a nested `|` (e.g. `side?: 'a' | 'b'`) was
   mis-routed as a top-level union and re-expanded to itself. The union branch
   now fires only for a genuine top-level `|` (two or more parts) and falls
   through otherwise.

2. Roughly 608 dangling `$ref`s across all five files, not only
   notifications.schema.json. Two root causes: the generator never walked
   enums, so bare `const enum` types and enum-member discriminants
   (`type: ActionType.Foo`) referenced $defs that were never emitted; and four
   of the five generators did not transitively collect referenced type aliases
   (URI, JsonPrimitive, ...). A fixpoint resolver now backfills every
   referenced enum, type alias, and interface, and enum-member accesses inline
   as `{ "const": <value> }`.

Also renders `null` union members as `{ "type": "null" }`, regenerates all
five schemas, and adds a regression test asserting no empty `{}` oneOf branch
and no dangling `$ref` in any generated schema.

Closes microsoft#302
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.

JSON Schema generator emits invalid schemas: spurious {} oneOf branch for union aliases + dangling $refs in notifications.schema.json

1 participant