From eeb4012a175264ab6f1cb4e26d96c6d7e9896bb1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 05:59:44 +0000 Subject: [PATCH] fix(scripts): refuse a PR that modifies or deletes a changeset it did not add (#17712) A changeset filename is content-free, so a collision looks like nothing: the changesets default word-pair names were designed for one human running the CLI at a time, and this repository runs many agents in parallel drawing from the same small name space. Overwriting somebody else's changeset produces a perfectly valid changeset file, so every parse-shaped gate stays green on both sides -- the sibling PR's release note is silently replaced, its own CI never re-runs, and the loss surfaces at release time in the generated CHANGELOG with the authoring PR long merged. check-empty-changeset.mjs now answers a second question in the same run: a `.changeset/*.md` that exists on the merge base and was not added by this PR may be neither modified nor deleted. "Added by this PR" is the file's absence on the merge base, which is what git's status letters already say, so the rule is content-blind and legacy names are untouched. Rename detection is off for this pass (the opposite of rule 1's `AMR`): renaming somebody else's changeset deletes their release note at its path, and with detection on that deletion is folded into an `R` row and disappears. Wired into check-empty-changeset.mjs rather than a new script so it inherits the `changeset-check` job's merge base, its `changeset-release/main` exemption -- `changeset version` deletes every changeset, so the release PR would be structurally unsatisfiable otherwise -- and its self-test's place in lint.yml, with no workflow diff at all. Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU Co-authored-by: Claude --- scripts/check-empty-changeset.mjs | 475 +++++++++++++++++++++++++++++- 1 file changed, 469 insertions(+), 6 deletions(-) diff --git a/scripts/check-empty-changeset.mjs b/scripts/check-empty-changeset.mjs index 1c5638ace2..24739e0fad 100644 --- a/scripts/check-empty-changeset.mjs +++ b/scripts/check-empty-changeset.mjs @@ -1,7 +1,23 @@ #!/usr/bin/env node // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. // -// check-empty-changeset -- a PR may not ADD an empty-frontmatter changeset. +// check-empty-changeset -- two rules about what a PR may do to a +// `.changeset/*.md`, both read off the diff's STATUS LETTERS against the merge +// base, answered in one run: +// +// 1. EMPTY FRONTMATTER (#5471) -- a PR may not ADD an empty-frontmatter +// changeset. Everything from "The rule (#5471)" down to "The foreign +// changeset rule" below is this one. +// 2. THE FOREIGN CHANGESET RULE (#17712) -- a PR may not MODIFY or DELETE a +// `.changeset/*.md` that exists on the merge base and was not added by +// this PR. Read "The foreign changeset rule (#17712)" below. +// +// The run exits with the WORSE of the two verdicts and prints BOTH, because +// they are independent facts about one changeset set and an author who fixes +// the first should not have to push again to discover the second. The FILE NAME +// names rule 1 only: renaming the file would mean editing the workflow steps +// that spawn it, and the wiring is the expensive half here -- see "The second +// consumer" battery for what a step's shape is already pinned to. // // node scripts/check-empty-changeset.mjs --base [--head ] // node scripts/check-empty-changeset.mjs # base defaults to origin/main @@ -100,6 +116,65 @@ // (#7005 / PR #7048); this file and `check-adr-0087-registration.mjs` followed in // #7045 because each of the three owns its own fixtures and its own messages. // +// ## The foreign changeset rule (#17712) +// +// 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, with one remedy: +// +// rename yours; restore theirs from base +// +// Ruled 2026-09-13 (director seat, decision batch #130 item 3) on #17712, as +// option A'. The census the ruling rests on, taken on `origin/main` at +// `9bd4344e4b`: 209 changesets, 39 card-scoped (`-.md`), 170 with no +// card scope at all. So the collision surface is 81% of the directory, not an +// occasional generated name. +// +// Why a gate and not a naming convention. A changeset filename is content-free +// -- changesets' random word-pair default was designed for ONE human running the +// CLI at a time, and this repository runs many agents in parallel drawing from +// the same small name space. Overwriting somebody else's changeset produces a +// perfectly VALID changeset file, so the parse-shaped gates stay green: the +// sibling PR's release note is silently replaced by this PR's, the sibling's own +// CI never re-runs, and the loss surfaces at release time in the generated +// CHANGELOG, by which point the authoring PR is merged. A naming rule alone is +// the class of protection that just failed -- the round that collided was +// following its dispatch faithfully; the dispatch simply had not told it to +// scope the name -- so the durable protection has to be mechanical and +// CONTENT-BLIND: "you may not change someone else's release note". Legacy names +// are untouched by this rule; it reads diff shape only, never the filename. +// +// "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: +// +// 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 +// +// Three consequences worth stating, because each is a case somebody will ask +// about: +// +// - A PR editing ITS OWN changeset across commits is unaffected. Added in +// commit 1 and edited in commit 2, the file is an `M` row against the +// previous commit but an `A` row against the merge base, and only the +// second reading is the rule. +// - Rename detection is turned OFF here (`--no-renames`), the opposite of the +// `AMR` choice rule 1 makes. Renaming somebody else's changeset DELETES +// their release note at its path, and that is the harm; with detection on, +// the deletion is folded into an `R` row and disappears. With it off the +// same edit reports `D ` + `A ` and the `D` is refused. It +// costs nothing in the other direction: a PR renaming its OWN changeset +// across commits still shows `A ` alone, because the old path was +// never on the merge base either. +// - `.changeset/README.md` is documentation, not a release note, and is +// exempt by the same `isChangesetFile` predicate rule 1 uses. +// +// Where the diff starts matters MORE for this rule than for rule 1, and in a +// direction rule 1 cannot show: run two-dot against a MOVING base tip instead of +// the merge base and every changeset main gained while the PR sat open is +// reported as a `D` on this branch -- an author refused by name for deleting +// files they never touched. See "Where the diff starts (#6129)" directly below; +// the self-test pins that false red as a firing control beside the real reading. +// // ## Where the diff starts (#6129) // // "What the PR introduces" is a claim about ONE SIDE of a fork, so the scan @@ -324,6 +399,66 @@ export function scan({ cwd, base, head = 'HEAD' }) { return { violations, exempt, ok, base: from }; } +// ── The foreign-changeset scan (#17712) ────────────────────────────────────── + +/** + * The changesets this diff MODIFIES or DELETES that it did not add. + * + * A separate pass over the same fork, deliberately, rather than another branch + * inside `scan()`: the two rules want DIFFERENT diff options. Rule 1 wants + * rename detection ON (`AMR`), because a rename is where its bypass reappears; + * this rule wants it OFF, because a rename is how a deletion HIDES. One `git + * diff` invocation cannot hold both settings, and a scan that quietly answered + * one rule's question with the other's options would be the more expensive + * mistake -- see "The foreign changeset rule (#17712)" in the header. + * + * There is no content reading here at all. The verdict is the status letter and + * nothing else, which is the ruling's content-blind half: the filename, the + * frontmatter and the prose are all irrelevant to whether this PR is entitled + * to change the file. + * + * `base` is the branch point to judge against; the diff starts at + * `merge-base(base, head)` for the #6129 reason, which bites HARDER here (a + * two-dot diff against a moved base tip turns main's own new changesets into + * `D` rows on this branch). Resolving it HERE rather than in the caller is the + * same decision `scan()` documents: this is the function the self-test drives. + * + * @param {{ cwd: string, base: string, head?: string }} opts + * @returns {{ foreign: { file: string, status: 'M'|'D' }[], base: string }} + * @throws when `base` and `head` have no merge base (#4690: not a pass) + */ +export function scanForeign({ cwd, base, head = 'HEAD' }) { + const from = mergeBase(base, head, cwd); + if (!from) { + throw new Error( + `no merge base between '${base}' and '${head}' -- the diff has no trustworthy starting point. ` + + 'Refusing to fall back to the raw base, which is the #6129 defect.', + ); + } + // `--no-renames` is load-bearing, not tidiness: see the header. `MD` is the + // whole rule -- an `A` row is by definition a path absent at `from`, which is + // the definition of "added by this PR", so it is never even listed. + const out = git( + ['diff', '--name-status', '--no-renames', '--diff-filter=MD', from, head, '--', '.changeset/*.md'], + cwd, + ); + + const foreign = []; + for (const line of out.split('\n')) { + if (!line.trim()) continue; + const fields = line.split('\t'); + // Read one character wide for the same reason `scan()` does. `M` and `D` + // carry no similarity score today, but the narrow read costs nothing and + // does not become wrong if a future option adds one. + const status = fields[0][0]; + const file = fields[1]; + if (!file || !isChangesetFile(file)) continue; + foreign.push({ file, status }); + } + + return { foreign, base: from }; +} + // ── Reporting ──────────────────────────────────────────────────────────────── const KIND_NOTE = { @@ -375,6 +510,61 @@ function report(violations) { } } +/** + * The one remedy the #17712 ruling names, verbatim. A constant because the + * self-test asserts the rendered report carries it: a refusal that names the + * offending file but not the way out sends an author to read this script. + */ +export const FOREIGN_REMEDY = 'rename yours; restore theirs from base'; + +const FOREIGN_NOTE = { + M: 'present on the merge base and CHANGED by this PR -- this is somebody else\'s release note', + D: 'present on the merge base and DELETED by this PR -- this is somebody else\'s release note', +}; + +function reportForeign(rows) { + console.error('This PR changes a changeset it did not add:\n'); + for (const { file, status } of rows) { + console.error(` ${file}\n ${FOREIGN_NOTE[status] ?? `status ${status} against the merge base`}`); + } + console.error( + [ + '', + `Remedy: ${FOREIGN_REMEDY}.`, + '', + 'A changeset filename carries no meaning, so a collision looks like nothing: the', + 'default word-pair names were designed for one human running the CLI at a time, and', + 'this repository runs many agents in parallel drawing from the same small name space.', + 'Overwriting an existing changeset produces a perfectly VALID changeset file, so every', + 'parse-shaped gate stays green on BOTH sides -- the other PR\'s release note is simply', + 'replaced by yours, its own CI never re-runs, and the loss surfaces at release time in', + 'the generated CHANGELOG, with the authoring PR long merged (#17712).', + '', + 'Concretely:', + '', + ' 1. Restore their file exactly as it stands on the merge base:', + ' git checkout -- ', + ' (that command STAGES what it retrieves -- read `git status --porcelain`', + ' before committing, and diff the restored path against the merge base.)', + ' 2. Give YOUR changeset an issue-scoped name that cannot collide:', + ' .changeset/-.md', + '', + 'Deleting a changeset is the same act one step further: `changeset version` is the', + 'only thing that consumes them, and it runs on the release PR, which this gate never', + 'judges.', + '', + 'If you are deliberately correcting somebody else\'s release note, that is a decision', + 'about a release, not a refactor -- say so on the PR and get it confirmed, rather than', + 'routing around this gate.', + ].join('\n'), + ); + for (const { file } of rows) { + console.error( + `::error file=${file}::${file} exists on the merge base and was not added by this PR, so changing or deleting it silently replaces somebody else's release note (#17712). Remedy: ${FOREIGN_REMEDY}.`, + ); + } +} + /** `--list`: the whole `.changeset` directory, empty vs declaring. */ function list() { const dir = join(REPO_ROOT, '.changeset'); @@ -443,6 +633,7 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'RED 5: an `R` row whose BASE side is README.md is not "inherited"': 3, '#6129: main drift must not move the verdict, in EITHER direction': 6, '#6129, the other half: a base branch that DELETES': 2, + "A' (#17712): a changeset the PR did not add is neither modified nor deleted": 29, '#4690, one step later: no merge base at all is a failure': 1, 'The consumer: this gate\'s own CI step (#6129)': 23, 'The second consumer: where THIS SELF-TEST runs (#6509)': 12, @@ -454,7 +645,7 @@ const SELF_TEST_BATTERIES = Object.freeze({ // DELETING an entry silences that battery's floor exactly as effectively as // zeroing it, so the roster's own size is pinned too. -const SELF_TEST_BATTERY_FLOOR = 21; +const SELF_TEST_BATTERY_FLOOR = 22; // The key an assertion is filed under when no battery is open. It is not a // declared battery, so it reds by the same set difference rather than silently @@ -519,6 +710,48 @@ function selfTest() { return { dir, base }; }; + /** + * A base commit plus N further commits on one branch (#17712). + * + * `makeRepo` is this with a single head step, and is left exactly as it is + * rather than rewritten in terms of this: every fixture above is pinned to its + * behaviour, and a shared builder that drifted would move verdicts in batteries + * that never changed. What needs more than one head commit is the case where + * "what this PR did" is only visible ACROSS commits -- a changeset added in + * one commit and edited in the next. + * + * @param {Record} baseFiles files committed as the base + * @param {...Record} steps one commit each (null = delete) + * @returns {{ dir: string, base: string }} + */ + const makeRepoSteps = (baseFiles, ...steps) => { + const dir = mkdtempSync(join(tmpdir(), 'check-empty-changeset-steps-')); + repos.push(dir); + const apply = (files) => { + for (const [rel, contents] of Object.entries(files)) { + const full = join(dir, rel); + if (contents === null) rmSync(full); + else { + mkdirSync(dirname(full), { recursive: true }); + writeFileSync(full, contents); + } + } + git(['add', '-A'], dir); + }; + git(['init', '-q', '-b', 'main'], dir); + git(['config', 'user.email', 'selftest@example.invalid'], dir); + git(['config', 'user.name', 'self test'], dir); + git(['config', 'commit.gpgsign', 'false'], dir); + apply(baseFiles); + git(['commit', '-q', '-m', 'base', '--allow-empty', '--no-gpg-sign'], dir); + const base = git(['rev-parse', 'HEAD'], dir).trim(); + steps.forEach((files, i) => { + apply(files); + git(['commit', '-q', '-m', `pr commit ${i + 1}`, '--allow-empty', '--no-gpg-sign'], dir); + }); + return { dir, base }; + }; + /** * The CI shape, built for real (#6129): a base branch that KEEPS MOVING after * the PR forks off it, and the `refs/pull/N/merge` commit GitHub builds from @@ -903,6 +1136,218 @@ function selfTest() { ); } + // ── A' (#17712): a changeset the PR did not add ────────────────────────── + // The four cases the ruling names (foreign `M` red, foreign `D` red, own `M` + // green, new `A` green), each with the control that proves the fixture did + // what it claims -- a `0` from a diff that was empty for some unrelated + // reason is not a green, it is a reading taken against nothing. + battery("A' (#17712): a changeset the PR did not add is neither modified nor deleted"); + { + const THEIRS = '.changeset/plain-donkeys-repeat.md'; + const MINE = '.changeset/17712-foreign-changeset-guard.md'; + const OTHER_DECLARING = '---\n"@objectstack/cli": minor\n---\n\nfeat(cli): somebody else\n'; + /** The raw diff rows, so a fixture that did nothing cannot read as a pass. */ + const rows = (dir, base, head = 'HEAD', extra = []) => + git(['diff', '--name-status', ...extra, base, head, '--', '.changeset/*.md'], dir); + + // RULED CASE 1 -- foreign `M` is RED. The #17712 incident verbatim: a + // round writes its changeset under the changesets default word-pair name + // and that file is already a sibling PR's `minor` changeset on main. + { + const { dir, base } = makeRepo( + { '.changeset/README.md': '# Changesets\n', [THEIRS]: OTHER_DECLARING }, + { [THEIRS]: DECLARING, 'src/app.ts': 'export const v = 2;\n' }, + ); + const r = scanForeign({ cwd: dir, base }); + assert(r.foreign.length === 1, "A' foreign M: overwriting a changeset that exists on the base must produce exactly one refusal"); + assert(r.foreign[0]?.file === THEIRS, "A' foreign M: the refusal must NAME the foreign file"); + assert(r.foreign[0]?.status === 'M', "A' foreign M: the status letter carried into the report must be M"); + } + + // RULED CASE 2 -- foreign `D` is RED. `scan()` cannot see this one at all: + // its `--diff-filter=AMR` has no `D`, so deleting somebody's release note + // is invisible to every other member of this family. + { + const { dir, base } = makeRepo( + { '.changeset/README.md': '# Changesets\n', [THEIRS]: OTHER_DECLARING }, + { [THEIRS]: null, [MINE]: DECLARING }, + ); + const r = scanForeign({ cwd: dir, base }); + assert(r.foreign.length === 1, "A' foreign D: deleting a changeset that exists on the base must produce exactly one refusal"); + assert(r.foreign[0]?.file === THEIRS, "A' foreign D: the refusal must NAME the deleted file"); + assert(r.foreign[0]?.status === 'D', "A' foreign D: the status letter carried into the report must be D"); + assert( + scan({ cwd: dir, base }).violations.length === 0, + "A' foreign D: rule 1 sees nothing here (`--diff-filter=AMR` has no D) -- the control that this rule is not redundant", + ); + } + + // RULED CASE 3 -- the PR's OWN changeset, edited across commits, is GREEN. + // Added in commit 1 and rewritten in commit 2: an `M` row against the + // previous commit, an `A` row against the merge base, and only the second + // reading is the rule. + { + const { dir, base } = makeRepoSteps( + { '.changeset/README.md': '# Changesets\n', [THEIRS]: OTHER_DECLARING }, + { [MINE]: '---\n"@objectstack/spec": patch\n---\n\nfix(spec): first go\n' }, + { [MINE]: DECLARING }, + ); + assert( + /^M\t\.changeset\/17712-foreign-changeset-guard\.md$/m.test(rows(dir, 'HEAD~1')), + "A' own M: CONTROL -- against the PREVIOUS COMMIT the file really is an M row, so the green below is about the merge base and not about an empty diff", + ); + assert( + /^A\t\.changeset\/17712-foreign-changeset-guard\.md$/m.test(rows(dir, base)), + "A' own M: CONTROL -- against the MERGE BASE the same file is an A row", + ); + assert(scanForeign({ cwd: dir, base }).foreign.length === 0, "A' own M: a PR editing its own changeset across commits must stay green"); + } + + // RULED CASE 4 -- a brand-new changeset is GREEN, with the stock present + // and untouched beside it. + { + const { dir, base } = makeRepo( + { '.changeset/README.md': '# Changesets\n', [THEIRS]: OTHER_DECLARING }, + { [MINE]: DECLARING }, + ); + assert( + /^A\t\.changeset\/17712-foreign-changeset-guard\.md$/m.test(rows(dir, base)), + "A' new A: CONTROL -- the fixture really added a changeset", + ); + assert(scanForeign({ cwd: dir, base }).foreign.length === 0, "A' new A: adding a changeset of your own must stay green"); + } + + // RENAMING somebody else's changeset. This is the case `--no-renames` + // exists for: with git's default detection ON the deletion is folded into + // an `R` row and vanishes, which is how row 2 of rule 1's table reopened + // under another letter in #7045. + { + const { dir, base } = makeRepo( + { '.changeset/README.md': '# Changesets\n', [THEIRS]: OTHER_DECLARING }, + { [THEIRS]: null, [MINE]: OTHER_DECLARING }, + ); + const detected = rows(dir, base, 'HEAD', ['--find-renames']); + assert(/^R\d*\t/m.test(detected), "A' rename: CONTROL -- with rename detection ON this diff really does collapse to an R row"); + assert(!/^D\t/m.test(detected), "A' rename: CONTROL -- and that R row leaves NO D row for a `D`-only filter to find"); + const r = scanForeign({ cwd: dir, base }); + assert(r.foreign.length === 1 && r.foreign[0]?.status === 'D', "A' rename: with --no-renames the deletion reappears and is refused"); + assert(r.foreign[0]?.file === THEIRS, "A' rename: the refusal names the path their release note stood at"); + } + + // ...and the other direction of the same flag: renaming YOUR OWN + // changeset across commits stays green, because the old path was never on + // the merge base either. + { + const { dir, base } = makeRepoSteps( + { '.changeset/README.md': '# Changesets\n', [THEIRS]: OTHER_DECLARING }, + { '.changeset/wobbly-pandas-shout.md': DECLARING }, + { '.changeset/wobbly-pandas-shout.md': null, [MINE]: DECLARING }, + ); + assert( + /^A\t\.changeset\/17712-foreign-changeset-guard\.md$/m.test(rows(dir, base)) && + !/^D\t/m.test(rows(dir, base)), + "A' own rename: CONTROL -- against the merge base this is one A row and no D row", + ); + assert(scanForeign({ cwd: dir, base }).foreign.length === 0, "A' own rename: renaming your own changeset across commits must stay green"); + } + + // `.changeset/README.md` is documentation, by the same predicate rule 1 + // uses. Editing it is ordinary work. + { + const { dir, base } = makeRepo( + { '.changeset/README.md': '# Changesets\n', [THEIRS]: OTHER_DECLARING }, + { '.changeset/README.md': '# Changesets\n\nHow to write one.\n' }, + ); + assert(/^M\t\.changeset\/README\.md$/m.test(rows(dir, base)), "A' README: CONTROL -- the fixture really modified README.md"); + assert(scanForeign({ cwd: dir, base }).foreign.length === 0, "A' README: .changeset/README.md is documentation, never a release note"); + } + + // The nonsense control: a PR that modifies files with nothing to do with + // `.changeset/*.md` reads zero, and the unrestricted diff proves the + // fixture was not simply empty. + { + const { dir, base } = makeRepo( + { '.changeset/README.md': '# Changesets\n', [THEIRS]: OTHER_DECLARING, 'docs/guide.md': 'v1\n' }, + { 'docs/guide.md': 'v2\n' }, + ); + assert( + git(['diff', '--name-status', base, 'HEAD'], dir).includes('docs/guide.md'), + "A' nonsense control: CONTROL -- this PR really did change a file", + ); + assert(scanForeign({ cwd: dir, base }).foreign.length === 0, "A' nonsense control: a diff that touches no changeset reads zero"); + } + + // #6129 in THIS rule's direction, and it is sharper here than for rule 1: + // run two-dot against the moved base TIP and every changeset main gained + // while the PR sat open is reported as a deletion on this branch. The + // firing control is the false red itself. + { + const { dir, mainTip } = makeMergeRefRepo({ + baseFiles: { '.changeset/README.md': '# Changesets\n' }, + prFiles: { [MINE]: DECLARING }, + driftFiles: { [THEIRS]: OTHER_DECLARING }, + }); + const twoDot = git( + ['diff', '--name-status', '--no-renames', '--diff-filter=MD', mainTip, 'pr', '--', '.changeset/*.md'], + dir, + ); + assert( + /^D\t\.changeset\/plain-donkeys-repeat\.md$/m.test(twoDot), + "A' #6129: FIRING CONTROL -- a two-dot diff against the MOVED base tip really does report main's new changeset as a deletion on this branch", + ); + const r = scanForeign({ cwd: dir, base: mainTip, head: 'pr' }); + assert(r.foreign.length === 0, "A' #6129: from the MERGE BASE the same branch is clean -- main's drift is not this PR's deletion"); + assert( + /^A\t\.changeset\/17712-foreign-changeset-guard\.md$/m.test(rows(dir, r.base, 'pr')), + "A' #6129: CONTROL -- and the PR's own changeset is still visible from that merge base, so the zero above is not a zero from a wrong range", + ); + } + + // Missing input is a failure, never a pass (#4690) -- the same rule + // `scan()` follows, restated for this scan because it has its own throw. + { + const { dir } = makeRepo({}, { 'a.txt': 'x\n' }); + const other = mkdtempSync(join(tmpdir(), 'check-empty-changeset-foreign-unrelated-')); + repos.push(other); + git(['init', '-q', '-b', 'main'], other); + git(['config', 'user.email', 'selftest@example.invalid'], other); + git(['config', 'user.name', 'self test'], other); + git(['config', 'commit.gpgsign', 'false'], other); + writeFileSync(join(other, 'b.txt'), 'y\n'); + git(['add', '-A'], other); + git(['commit', '-q', '-m', 'unrelated', '--no-gpg-sign'], other); + git(['fetch', '-q', other, 'main:unrelated'], dir); + let threw = false; + try { + scanForeign({ cwd: dir, base: 'unrelated' }); + } catch { + threw = true; + } + assert(threw, "A' #4690: no merge base at all must THROW, not fall back to the raw base"); + } + + // The refusal has to be actionable: the report names the file and carries + // the ruling's remedy verbatim, in the human body AND in the annotation a + // reviewer sees on the diff. + { + const captured = []; + const realError = console.error; + console.error = (...args) => captured.push(args.join(' ')); + try { + reportForeign([{ file: THEIRS, status: 'M' }]); + } finally { + console.error = realError; + } + const text = captured.join('\n'); + assert(text.includes(THEIRS), "A' report: the refusal must name the offending file"); + assert(text.includes(FOREIGN_REMEDY), "A' report: the refusal must carry the ruling's remedy verbatim"); + assert( + captured.some((line) => line.startsWith('::error file=') && line.includes(FOREIGN_REMEDY)), + "A' report: the GitHub annotation must carry the remedy too -- an annotation that only accuses sends the author to read this script", + ); + } + } + // ── #4690, one step later: no merge base at all is a failure ───────────── // Falling back to the raw base here would restore exactly the bug above, so // the scan throws and the CLI turns that into exit 1. @@ -1639,24 +2084,42 @@ if (!invokedDirectly) { } let result; + let foreignResult; try { result = scan({ cwd: REPO_ROOT, base, head }); + foreignResult = scanForeign({ cwd: REPO_ROOT, base, head }); } catch (error) { console.error(`⛔ check-empty-changeset: ${error instanceof Error ? error.message : String(error)}`); console.error(' Missing input is a failure, never a pass (#4690).'); process.exit(1); } const { violations, exempt, ok, base: from } = result; + const { foreign } = foreignResult; // The starting commit is printed on both verdicts, and it is not decoration: // #6129 hid for as long as it did because nothing in any log said where the // diff began, so a gate reading the wrong side of a fork looked exactly like a // gate reading the right one. console.log(`Diffing ${head} from ${from.slice(0, 9)} (merge base with ${baseLabel}).`); + + // Both rules are reported before either exits. An author who has done two + // things wrong should learn both from one run: exiting on the first would + // spend a push per rule, and this gate's whole subject is a diff that is + // expensive to re-push. + let refused = false; if (violations.length) { report(violations); - process.exit(1); + refused = true; + } else { + const parts = [`${ok.length} declaring changeset(s) added`]; + if (exempt.length) parts.push(`${exempt.length} pre-existing empty changeset(s) touched but exempt`); + console.log(`✓ No empty-frontmatter changeset introduced by this diff (${parts.join(', ')}).`); + } + if (foreign.length) { + if (refused) console.error(''); + reportForeign(foreign); + refused = true; + } else { + console.log('✓ No changeset from the merge base modified or deleted by this diff (#17712).'); } - const parts = [`${ok.length} declaring changeset(s) added`]; - if (exempt.length) parts.push(`${exempt.length} pre-existing empty changeset(s) touched but exempt`); - console.log(`✓ No empty-frontmatter changeset introduced by this diff (${parts.join(', ')}).`); + if (refused) process.exit(1); }