Uh oh!
There was an error while loading. Please reload this page.
feat(scripts): guard a package's published contract fields, not only its source - #6999
Merged
Merged
Conversation
…its source `check-changeset-presence.mjs` derived its guarded surface from paths alone, so a change to `sideEffects`, `exports`, `files`, `peerDependencies` or `engines` in a released package's `package.json` was owed no declaration — measured on PR #6735, whose `sideEffects` array got the gate's own `no changeset is owed`. The reading is by FIELD, over both sides of the diff, and that is what makes the two named exclusions expressible: a `version` bump written by `changeset version` and a Dependabot `devDependencies` bump are excluded BY CONSTRUCTION, because they are not in the allowlist — no branch to forget and no bot identity to sniff. `files` shares one normalisation with clause (c), so the two readings of that field cannot contradict each other inside a single run. The header's stated exclusion is rewritten rather than left standing: what is kept from that trade (`dependencies`, `scripts`, `version`) is now stated as kept, so the manifest is not read as guarded.
zhuangjianguo
marked this pull request as ready for review
August 31, 2026 08:25
This was referenced Aug 31, 2026
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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#6736
scripts/check-changeset-presence.mjsderived its guarded surface from paths alone, so a change to a released package's published contract —sideEffects,exports/main/module/types,files,peerDependencies/engines— was owed no declaration. The live instance is PR #6735, which gave@object-ui/app-shellasideEffectsARRAY and got the gate's ownNo source of a released package changed in this range, so no changeset is owed. The changeset in that PR was there because its author decided it was owed.The false-positive measurement — the ruling's precondition
Triage made measuring the false-positive population a condition of the change, on #11457 discipline: "一个被绕过的门禁比没有门禁更糟,因为它还占着「已覆盖」的名分". Method: extract the pre-change gate from
origin/main, import both versions, and runanalyze+verdictover each merged commit onorigin/mainassha^..sha(these are squash merges, so that range is the PR's diff).Zero false positives. All three field moves in the 200-PR window already carried a changeset:
The two named exclusions, measured against real history rather than fixtures
The card names two changes that must NOT demand a declaration. Both were run through the old gate, the new gate, and a file-level counterfactual ("any edit to a released package's
package.json"):package.jsonedited59f61cfb8chore: release packages(#4655) — whatchangeset versionwrites590dd6356chore(deps-dev): bump the dev-dependencies group … 11 updates(#4948) — DependabotThis is the whole argument for reading FIELDS rather than the file. A release commit rewrites 40 manifests and adds no changeset — it empties
.changeset/. A file-level gate would go red on the very commit that answers it, and red on every Dependabot bump. Neither exclusion is a branch in the code: they hold because the reading is an allowlist of eight fields andversion/devDependenciesare not in it. There is no bot identity to sniff and no commit message to parse.Is the new green on the live instance earned, or vacuous?
The old gate passed #6735 while having looked at nothing, so "still green" is not evidence. Ablating the declarations from the analysis separates the two:
What changed
A second question asked of the manifest, not a fourth clause in the population.
isPublishedSourcestays path-only and still answersfalseforpackage.json— pinned. A newcontractChangesreads both sides of the diff out of git, parses them, and compares the eight guarded fields by value.versionabovenameis green. Comparison is order-preserving serialisation, which is correct rather than convenient:exportscondition order is resolution order in Node, so a reorder that moves no key and no value is a behaviour change and goes red.files, shared with clause (c).publishedEntriesis what clause (c) compares changed paths against, and it is now what the field diff compares too. Without that the gate could demand a declaration for afilesrespelling (disttodist/) that its own clause (c) reports as changing nothing shipped — code contradicting itself inside one run.The docblock said this exclusion was deliberate — reconciled, not overwritten
The card frames the gap as an omission. It is not: line 137 stated it as a boundary — "
package.jsonitself never counts. Clause (c) reads that file; it does not match it. A dependency bump can be just as user-visible, and this gate still does not see it. That was the first draft's trade and it is kept."Read against the code, that is a scoping trade against noise, not a mechanism-level obstacle — the same population discipline the header states two paragraphs earlier ("too wide and every incidental file in a package directory demands a declaration … which is how a gate stops being read at all"). A field-level reading is the resolution of exactly that trade rather than its reversal, so this is not a fork.
The bullet is rewritten, and the part of the trade that is kept is now stated as kept:
dependenciesstays out, and a runtime dependency bump is still invisible to this gate. Nobody should read "the manifest is guarded now" off this change.Tests
scripts/__tests__/check-changeset-presence.test.tsgains section 2c — 14 cases, both directions:sideEffectsarray with no changeset goes red naming the file and the field; the same change with a changeset is green because of the declaration.CONTRACT_FIELDSare the same set, so a field with no case cannot exist.version-only bump and adevDependencies-only bump are green.scripts,dependencies,publishConfig,description) moving together: green. Reformatting: green. Root manifest: green.filesrespelling: green.exportscondition reorder: red.CONTRACT_FIELDSis pinned as a list, which is what makes the ruling mechanical: adding or dropping a field cannot happen without a deliberate edit here.Verification
All on the final commit
354653832.Ablation — replacing
contract: contract.guardedwithcontract: []inanalyze, with the mutation confirmed on disk (anchor 1 to 0, marker 0 to 1, blob hash moved) before reading any result: 4 failed / 49 passed, and the four are the positive legs. The restore leg was confirmed the same way —git checkout HEAD --on an absolute path, then blob hash equal toHEAD, marker gone, anchor back,git diff HEADempty.No changeset: this diff touches
scripts/only, which is no package's published source. Theskip-changesetlabel is deliberately not applied — in this repository that label object exists from a historical mis-attachment, is read by no workflow, and exempts nothing.Filed, not fixed here
Two pre-existing findings, both measured on
origin/mainat4357ec754and both outside this card's declared file surface:src/only — already narrower than the gate since #5733, and narrower again after #6736 #6997 — AGENTS.md line 161 states the changeset rule assrc/only. Already narrower than the gate since objectui#5733, and narrower again after this PR. Agent-facing text, so a rule stated too narrowly is paid by every reader.js-comment-mask.mjscites two line numbers as evidence and both point at the wrong line (86 and 107 lines off) #6998 —js-comment-mask.mjscites two line numbers as evidence for a live mutation-blind shape, and both point at the wrong line (86 and 107 off). This PR moves one of the two targets further, which is the point: a line number into another file has nothing keeping it true.Generated by Claude Code
Generated by Claude Code