Skip to content

Commit 0ffb496

Browse files
baozhoutaoclaude
andauthored
fix(scripts): refuse a PR that modifies or deletes a changeset it did not add (#17712) (#18146)
Closes #17712 `Clause-②: no` — this card only makes a gate refuse MORE. It loosens no accept set and widens no published surface. Implements **item 1 (A′)** of the ruling on #17712 (director seat, decision batch #130 item 3, 2026-09-13; maintainer's verbatim reply 「同意」). Item 2 (the `ISSUE-SLUG.md` naming line in governed dev-round guidance) is #17748's and is not touched here. Item 3 — legacy names untouched, diff shape only — is honoured: nothing in this diff reads a changeset filename. ## The rule A PR may not MODIFY or DELETE a `.changeset/*.md` that exists on the merge base and was not added by this PR. Refused by name, one remedy: **rename yours; restore theirs from base**. "Added by this PR" is the file's absence on the merge base — which is exactly what git's status letters already say, so the rule needs no second reading and no filename predicate at all: | row | meaning | verdict | |:--|:--|:--| | `A` | absent at the merge base | this PR's own file, always ok | | `M` | present at the merge base, changed here | foreign, REFUSED | | `D` | present at the merge base, gone here | foreign, REFUSED | ## Where it is wired, and why Into `scripts/check-empty-changeset.mjs`, as a second axis answered in the same run — the shape `check-changeset-no-major.mjs` already uses for its two axes. **The PR touches no workflow file at all**, so it can be armed normally. Three things that come for free from that placement, each of which a standalone script would have had to re-derive: 1. **The `changeset-release/main` exemption.** `changeset version` DELETES every pending changeset, so the Version Packages PR is all-`D` rows. Outside `changeset-check`'s job-level `if:`, this gate would be structurally unsatisfiable on every release PR — the #4422 / #4894 shape, pointed at the release train. 2. **The merge base.** The `changeset-check` job already derives `MERGE_BASE` and hands it to this script. That matters more for this rule than for the others: run two-dot against a moving base tip instead of the merge base and every changeset `main` gained while the PR sat open reports as a `D` on this branch — an author refused by name for deleting files they never touched. The self-test pins that false red as a firing control beside the real reading. 3. **A self-test that cannot be skipped.** `check-empty-changeset.mjs --self-test` runs unconditionally in `lint.yml` (#6509), outside the `skip-changeset` exemption — which is where this gate's own fixtures need to be, since a PR editing a CI-internal script is the textbook `skip-changeset` case. The file name still names rule 1 only. Renaming it would mean editing the workflow steps that spawn it; the header says so rather than leaving it to be noticed. **Rename detection is OFF for this pass** (`--no-renames`), the opposite of the `AMR` choice rule 1 makes in #7045 — and for the same underlying reason, one letter over. Renaming somebody else's changeset DELETES their release note at its path; with detection on, that deletion folds into an `R` row and disappears. With it off the same edit reports `D theirs` + `A yours` and the `D` is refused. It costs nothing in the other direction: a PR renaming its OWN changeset across commits still shows one `A` row, because the old path was never on the merge base either. ## The four ruled cases, end to end on this repository Driven on a throwaway worktree at this branch's tip, against real commits (the gate reads commits, never the working tree): | case | diff rows vs merge base | exit | |:--|:--|--:| | foreign `M` — overwrite a sibling's changeset | `M .changeset/12271-published-entry-no-auto-transpile.md` | **1** | | foreign `D` — delete theirs, add mine | `D .changeset/12271-...md` + `A .changeset/17712-demo-only.md` | **1** | | own `M` across commits — add in commit 1, reword in commit 2 | `A .changeset/17712-demo-only.md` | **0** | | new `A` | `A .changeset/17712-demo-only.md` | **0** | The refusal, verbatim: ``` This PR changes a changeset it did not add: .changeset/12271-published-entry-no-auto-transpile.md present on the merge base and DELETED by this PR -- this is somebody else's release note Remedy: rename yours; restore theirs from base. ``` plus a `::error file=...` annotation carrying the same remedy, so it lands on the diff rather than only in a log. ## Ablation — the currently-false sentence this makes true Same two fixtures, with `scripts/check-empty-changeset.mjs` restored to its `origin/main` blob (`1c5638ace2`, marker count for `scanForeign` 0 on disk; restored to `24739e0fad`, marker count 12, `git diff HEAD` empty): | fixture | pre-change gate | post-change gate | |:--|--:|--:| | overwrite a sibling's changeset | **exit 0** | exit 1 | | delete theirs + add mine | **exit 0** | exit 1 | ## Reverse-read, both directions **Made false.** "`check-empty-changeset.mjs` exits 0 on any diff whose `.changeset/*.md` rows are all non-empty at head." It no longer does — case `M` above shows rule 1 printing its own green tick on the same run that exits 1. **Made true.** "A PR that changes or deletes another PR's release note is refused by name, with a remedy." Previously false in every member of the family: the only filename predicate in the changeset gates is prefix + extension, and `scan()`'s `--diff-filter=AMR` has no `D` at all, so a deletion was invisible to all three. Pinned as an assertion, not just asserted here: the foreign-`D` battery case also runs `scan()` on the same fixture and requires **zero** rule-1 violations — the control that this rule is not redundant with the one beside it. ## One premise in the card is not exactly right, measured The card says the collision would pass "with **every gate green** on both sides". Measured on real fixtures against `pr-automation.yml`'s own `--diff-filter=A` count: | shape | `ADDED` | pre-change verdict | |:--|--:|:--| | overwrite theirs, add nothing of your own | **0** | already RED today — but as *"this PR doesn't add a changeset"* | | overwrite theirs AND add your own | 1 | green | | delete theirs, add your own | 1 | green | So the narrowest shape of the incident was already refused — by the wrong gate, naming the wrong problem, and with a remedy ("add a changeset") that leaves the overwrite in place. The two shapes that actually reach `main` were green. The card's conclusion stands; its "every gate" is one row too strong, and the correction is recorded here rather than left for the next reader of that thread. ## Self-test New battery `A' (#17712): a changeset the PR did not add is neither modified nor deleted` — **29 cases**, pinned in `SELF_TEST_BATTERIES` and reached (the roster's own size floor moves 21 → 22). The battery-floor machinery (#13489) is what makes "every case held" distinguishable from "the cases never ran". Beyond the four ruled cases it covers: renaming a foreign changeset (with the rename-detection-ON control proving the `R` row really does swallow the `D`); renaming your own changeset across commits; `.changeset/README.md`; a nonsense control (a diff touching no changeset); #6129 in this rule's direction, firing control first; #4690 (no merge base throws, never falls back); and that the rendered report names the file and carries the remedy verbatim in both the body and the annotation. Every green in the battery carries a control that the fixture did what it claims — a `0` from a diff that was empty for an unrelated reason is a reading taken against nothing, not a pass. ``` ✓ check-empty-changeset --self-test: 147 assertions over real temp git repos (real scan() path) ``` ## Verification At `eeb4012a17`: - **`node scripts/pm/dispatch-gates.mjs --ran` — 34 derived families, 34 run, 0 NOT-MEASURED (derived, every entry carrying its exit code), 0 UNRUN.** All 34 exit 0. Re-derived after `git fetch origin main`; the set did not move. - **`npx eslint . --no-inline-config --format json` — exit 0 over 6751 files, 0 errors, 0 warnings.** The whole-repo run, not a narrowing: the population is eslint's own count from its `--format json` output, and this file is in it. - Control-character self scan over the edited file: 0 matches (firing control on a vertical tab: 1 match). - `node scripts/check-empty-changeset.mjs --base origin/main` on this branch: both axes green. No package is touched, so there is no dependency closure to build and no package test suite affected; `scripts/` is not TypeScript and no `*.test.ts` in the tree names this script (control: `check-nul-bytes` names three). ## No changeset — `skip-changeset` Measured rather than asserted: **0** published (non-`private`) packages have a `files[]` entry that would ship anything under `scripts/`. Firing control: **70** published packages ship `dist/`. Nothing published moves, so the label is the correct route and an empty-frontmatter changeset would be refused by rule 1 of this very script. ## 验收备注 Two cells are recorded rather than implied, both inherited from the job this step lives in and neither newly opened by it: - A PR carrying `skip-changeset` is exempt from the whole `changeset-check` job, so it is exempt from this rule too. Identical in shape to the cell `check-empty-changeset.mjs` already records for rule 1, and with the same reasoning: a consistent exemption beats a gate that reds one PR and greens an identical one. Motive is thin — a PR with the label adds no changeset of its own, so it has nothing to collide with — but deletion is still reachable under it. - `pr-automation.yml` runs on `pull_request` only, so this gate does not re-run in the merge queue. There is a recorded ⛔ in `lint.yml`'s `on:` block against adding `merge_group` to advisory workflows, so closing that would be a decision, not a wiring fix. What the ruling's "re-evaluated against the current base" needs is satisfied on the PR leg: the base is recomputed on every `synchronize`, and it is a merge base, never a branch tip. One maintainer-only question, deliberately left un-decided here: the ruling names no escape hatch for **deliberately** correcting somebody else's release note on `main`. The report tells such an author to say so on the PR and get it confirmed. If a mechanical route is wanted instead, that is a decision about release integrity and belongs to you, not to this PR. Noted, not filed: `scan()` and `scanForeign()` now hold two near-identical merge-base preambles, including the same throw text. Whoever next touches a third axis in this family will want one helper — no PR or person is queued on this file today, so there is no carrier: 承接者:无. --- _Generated by [Claude Code](https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 295eae5 commit 0ffb496

1 file changed

Lines changed: 469 additions & 6 deletions

File tree

0 commit comments

Comments
 (0)