Block breaking changes to the dev schema at pull-request time - #732
Open
MGudgin wants to merge 2 commits into
Open
Block breaking changes to the dev schema at pull-request time#732MGudgin wants to merge 2 commits into
MGudgin wants to merge 2 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
This PR adds `scripts/versioning/lib/schema-compatibility.js`, which reports
the ways a new JSON Schema can reject an instance the old one accepted. It is
the primitive the dev-schema gate enforces with.
The detector fails closed in both directions: anything it cannot model becomes
a manual-review finding rather than silence, and anything it cannot prove is a
restriction is reported as needing proof rather than asserted as breaking.
Details
* Covers roughly thirty categories of tightening: closed objects losing a
property, new `required` entries, narrowed `type`, removed `enum` values,
tightened numeric and length bounds, added `items` / `contains` /
`propertyNames`, and changed combinators.
* Normalises equivalent spellings so a generator's rendering choice never reads
as a structural change: `const` and single-valued `enum`, `{}` and `true`,
draft-04 boolean `exclusiveMinimum` / `exclusiveMaximum`, and a `oneOf` of
singleton enums against a flat `enum`.
* Preserves keywords sitting beside a `$ref`. Draft 2019-09 applies them, so
returning only the target would drop a real restriction such as an added
`required` or `additionalProperties: false`.
* Reports an unresolved reference even when both sides carry the same one:
matching text says nothing about matching content when neither target was
ever inspected. A recursion marker is treated separately, because it marks a
cycle the walk already entered, so equal markers there do mean equal
structure.
* Descends into unmatched `anyOf` branches only when exactly one remains on
each side, which admits a single possible correspondence. That is the
`anyOf: [T, null]` nullable idiom, where descending is what names a property
removed from inside `T`. Other shapes report that containment requires
manual proof, since replacing singleton-enum branches with a general `string`
branch accepts strictly more, not less.
* Compares `additionalItems` only alongside tuple-form `items`, where the
keyword has effect, and uses own-property lookups throughout so a property
legitimately named `constructor` or `toString` is not skipped via the
prototype chain.
* Bounds traversal. Normalisation memoises `$ref` targets; the diff walk and
structural equality memoise node-identity pairs; combinator branches are
bucketed by a fixed-size digest, also memoised on identity. Without these a
`$ref` graph that fans out expands exponentially, and equality that serialises
its operands materialises the tree a shared graph unfolds to. Depth and node
budgets catch what remains, including deeply nested enum data, and surface it
as a finding rather than a crash.
Tests
* 63 unit tests pass, covering each detection category, the equivalent-spelling
normalisations, `$ref` siblings, unresolved and recursive references,
prototype-named properties, tuple-only `additionalItems`, deterministic
ordering, and the traversal budgets.
* Fan-out at depth 40 completes in about a millisecond and 5,000-deep nesting
returns a budget finding instead of overflowing the stack; a 5,000-level enum
value does the same.
* Combinator matching measured 19 ms at 2,000 branches, 35 ms at 4,000 and
67 ms at 8,000.
* Regressions pin the two behaviours in tension: a removed property inside a
nullable wrapper is still named, and an equal-count branch replacement yields
one manual-proof finding rather than invented positional restrictions.
* Checked against the committed schemas: a schema against itself yields no
findings, 0.6.0-alpha to 0.7.0-alpha yields 6, and 0.7.0-alpha to the dev
schema yields 12.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: cd48fff2-bde9-487a-ab67-012e9bbc0796
Generated-with: claude-opus-5
This PR adds a CI gate that compares the dev schema at the pull-request base against the dev schema at HEAD and fails when the new one rejects an instance the old one accepted. Every other breaking-change guard compares RELEASED stable schemas, and only at release time. The surface a pull request actually edits -- the dev schema -- is unguarded, so a change can delete a stable field, regenerate the schema and the SDK types, migrate the config corpus, and merge green. PR #676 did exactly that, and was reverted by hand. Details * `scripts/versioning/check-dev-schema-compat.js` resolves the base commit with the fail-closed helper, reads both dev schemas out of git, and reports every structural restriction the compatibility detector finds. * Each side is read at its own declared `devSchemaFile`. Opening a new dev line copies the outgoing one, so the documents stay the same lineage and the comparison holds across that transition. Skipping the comparison when the line moves would let a change disable the gate by editing one line of `schemas/schema-version.json`. * A missing or unparsable schema on either side fails. The gate is only useful if it cannot succeed vacuously. * There is no per-field escape hatch. The supported-version window is what allows surface to end, so until a change moves that window, a config declaring an already-supported version has to keep parsing. * Documented in `.github/copilot-instructions.md` alongside the other schema gates, including how to make a breaking change additively, since this gate is what a contributor meets when they try to remove surface. * Runs ahead of corpus validation, because a change that removes a field also migrates the corpus; validation then passes and the removal is what needs reporting. Tests * 8 end-to-end tests drive the real CLI against throwaway repositories and assert on its exit code: unchanged and additive schemas pass; a removed property, a narrowed type, a missing schema and an unparsable schema all exit 1; a compatible new dev line passes and reports the move; and an incompatible new dev line is still blocked. * Replayed against PR #676: the gate exits 1 and names all six removed `network` fields. * Run against the repository as it stands, the gate passes, as do `check-schema-versions.js` and corpus validation across 195 configs. * Full versioning suite: 71 tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cd48fff2-bde9-487a-ab67-012e9bbc0796 Generated-with: claude-opus-5
MGudgin
force-pushed
the
user/gudge/versioning_phase7d_dev_schema_gate
branch
from
August 1, 2026 19:15
62679f7 to
8977c18
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a CI gate to prevent the development schema from becoming structurally more restrictive.
Changes:
- Adds the compatibility gate and eight integration tests.
- Runs the gate before corpus validation.
- Documents the additive schema-evolution policy.
Show a summary per file
| File | Description |
|---|---|
scripts/versioning/check-dev-schema-compat.js |
Implements base-to-HEAD comparison. |
scripts/versioning/tests/dev-schema-gate-integration.test.js |
Adds end-to-end gate coverage. |
scripts/versioning/package.json |
Exposes the gate as an npm script. |
.github/workflows/Versioning.Checks.Job.yml |
Adds the CI check. |
.github/copilot-instructions.md |
Documents compatibility requirements. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
| ? "" | ||
| : ` (dev line moved ${baseVersions.devSchemaFile} -> ${headVersions.devSchemaFile})`; | ||
|
|
||
| const findings = detectBreaking(baseSchema, headSchema); |
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.
Summary
This PR adds a CI gate that compares the dev schema at the pull-request base against the dev schema at HEAD and fails when the new one rejects an instance the old one accepted.
Every other breaking-change guard compares RELEASED stable schemas, and only at release time. The surface a pull request actually edits -- the dev schema -- is unguarded, so a change can delete a stable field, regenerate the schema and the SDK types, migrate the config corpus, and merge green. PR #676 did exactly that, and was reverted by hand.
Details
scripts/versioning/check-dev-schema-compat.jsresolves the base commit with the fail-closed helper, reads both dev schemas out of git, and reports every structural restriction the compatibility detector finds.devSchemaFile. Opening a new dev line copies the outgoing one, so the documents stay the same lineage and the comparison holds across that transition. Skipping the comparison when the line moves would let a change disable the gate by editing one line ofschemas/schema-version.json..github/copilot-instructions.mdalongside the other schema gates, including how to make a breaking change additively, since this gate is what a contributor meets when they try to remove surface.Tests
networkfields.check-schema-versions.jsand corpus validation across 195 configs.Note for reviewers
The gate blocks the dev schema accepting less than it did, which is not the same as blocking breaking changes. Because one dev schema validates configs declaring every supported version, surface a supported version can use has to remain in it. A breaking change is made additively -- keep the old fields, add the new shape alongside -- and the supported-version window (a later phase) governs which may be used at which version. Deleting becomes legitimate only once
minrises past the surface being dropped.Microsoft Reviewers: Open in CodeFlow