fix: make generated JSON Schemas self-contained and strict-oneOf-safe#305
Open
joshmouch wants to merge 1 commit into
Open
fix: make generated JSON Schemas self-contained and strict-oneOf-safe#305joshmouch wants to merge 1 commit into
joshmouch wants to merge 1 commit into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #302.
Both bugs are fixed generator-wide, plus one scope correction and a latent crash surfaced along the way.
Bug 1: empty
{}oneOf branchA union type alias's printed type text carries a leading
|(type X =\n | A\n | B), sosplitUnionTypeyielded an empty first element that fell throughtypeTextToSchemato{}. Every union alias'soneOftherefore led with a matches-anything branch, so every instance matched two or more branches and strictoneOfvalidation 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;splitUnionTypecorrectly 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:$refsAnd 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 intypeTextToSchemaand was emitted as a$refto a$defthat 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:
{ "const": <member value> }(sotype: ActionType.Foobecomes{ "const": "root/agentsChanged" }rather than a dangling ref).resolveMissingDefspass runs after each schema is built and backfills every referenced$defthat is not yet present, resolving interfaces, type aliases (includingURI = string), and whole enums (SessionStatusbecomes{ "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.nullunion 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 intonpm 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 testpasses (290 tests, typecheck, lint, changelog + release-metadata gates). All fiveschema/*.jsonfiles are regenerated in this PR.