From 7b2fbeba053af5e357dc7a57c0c0d9096c6dcead Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 10:23:02 +0000 Subject: [PATCH] =?UTF-8?q?fix(ci):=20Check=20Changeset=20=E4=BB=A5=20merg?= =?UTF-8?q?e-base=20=E4=B8=BA=E5=B7=AE=E5=BC=82=E8=B5=B7=E7=82=B9,main=20?= =?UTF-8?q?=E6=BC=82=E7=A7=BB=E4=B8=8D=E5=86=8D=E7=AE=97=E4=BD=9C=E6=9C=AC?= =?UTF-8?q?=20PR=20=E6=96=B0=E5=A2=9E?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pr-automation.yml` 的 Check Changeset 用 `github.event.pull_request.base.sha` 作差异起点。这个 sha 在 PR **创建时**被冻结,而 `actions/checkout@v7` 在 `pull_request` 事件上签出的是 merge ref(`refs/pull/N/merge`)—— 一个 parent^1 为**当前** main tip 的合并提交。两者之间的 main 漂移,整段被 `--diff-filter=A` 记成「本 PR 新增的文件」:别人 PR 合进 main 的 changeset,在本 PR 眼里就是本 PR 加的。PR #6117 同一份 diff、零 changeset,02:22Z 判红、02:39Z 判绿,只因为这 17 分钟里 main 多了两个带 changeset 的 PR。这是发版安全门禁上的**假绿**,后续没有 任何一步会把它纠回来。 改法:差异起点改为 `git merge-base origin/$BASE_REF HEAD`。在 merge ref 上它恰好 落在 parent^1,于是 diff 只剩本 PR 自己那侧 —— 早跑晚跑同判。三个 BASE_SHA 消费 者一起改(计数步骤、`check-empty-changeset.mjs --base`、以及脚本自己的 base 语义), checkout 保持不变。 `scripts/check-empty-changeset.mjs`:`scan()` 内部把 base 解析为 `merge-base(base, head)`。CI 路径上这一步是幂等的,它修的是另一半 —— 默认 `--base origin/main` 的本地路径:`changeset pre exit` 从 main 删掉已消费的 changeset 之后,两点 diff 会把未 rebase 分支上仍带着的存量空 changeset 全部读成 「本分支新增」(实测 2 个 fixture → 2 条假红,merge-base → 0 条)。 self-test 21 → 36 条断言,新增两个方向 + 消费者断言:main 漂移**不得**改变判定 (merge ref fixture,真实两父提交)、本 PR 自己新增的空 changeset **仍须**判红、 以及直接读 `pr-automation.yml` 断言计数与 `--base` 都吃 `$MERGE_BASE` —— 少了最 后这组,workflow 明天被改回冻结 sha 也不会有任何一条断言变红,那正是 #6129 要堵 的「换个形状回来」。 `check-changeset-no-major.mjs` 实测未受影响:它不读 `--base`、不做 git diff, `readdirSync('.changeset')` 扫整个目录,不存在 base 选取问题。#5620 的 allow-major 步骤逐字未动。 Refs #6129 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01BDmDsu2575gDxeMCxXhDE3 --- .github/workflows/pr-automation.yml | 89 +++++++- scripts/check-empty-changeset.mjs | 337 +++++++++++++++++++++++++++- 2 files changed, 411 insertions(+), 15 deletions(-) diff --git a/.github/workflows/pr-automation.yml b/.github/workflows/pr-automation.yml index 5d44d59b3e..afe1bb6230 100644 --- a/.github/workflows/pr-automation.yml +++ b/.github/workflows/pr-automation.yml @@ -217,8 +217,76 @@ jobs: if: steps.labels.outputs.skip != 'true' uses: actions/checkout@v7 with: + # `fetch-depth: 0` is load-bearing for the step below, not just nice to + # have: it is what makes actions/checkout fetch + # `+refs/heads/*:refs/remotes/origin/*` (getRefSpecForAllHistory) on top + # of the PR merge ref, so `origin/` exists locally and a + # merge base can be computed at all. A shallow checkout here would take + # the base resolution below straight to its #4690 failure branch. fetch-depth: 0 + # #6129: every diff below starts HERE, and the one thing it must never be + # is `github.event.pull_request.base.sha`. + # + # The payload's `base.sha` is frozen when the PR is OPENED and does not + # move on `synchronize`. HEAD, meanwhile, is the merge ref + # (`refs/pull/N/merge`) that the checkout above resolves by default on a + # `pull_request` event -- a merge commit whose parent^1 is whatever main + # tipped at when the ref was built. So `diff base.sha HEAD` reports + # EVERYTHING main gained in between as "added by this PR", and with ~18 + # merges a day that is a lot. Measured on PR #6117: identical diff, zero + # changesets of its own, `failure` at 02:22Z and `success` at 02:39Z -- + # main had merged two other PRs' changesets into the merge ref and the + # counting step below took them for this PR's. A release-safety gate that + # goes GREEN because someone ELSE released something is the one direction + # nothing downstream corrects. + # + # The merge base fixes it because on a merge-ref HEAD it lands exactly on + # parent^1 -- verified on a real merge commit, not assumed -- so the diff + # is this PR's own side and nothing else. Same diff, same verdict, however + # long the PR sits and however far main runs ahead. + # + # Two spellings that look like fixes and are not: + # - `git diff base.sha...HEAD` (three dots). Three-dot means + # `merge-base(base.sha, HEAD)..HEAD`, and `base.sha` is ALREADY an + # ancestor of HEAD, so the merge base is `base.sha` itself and the + # count does not move. Measured: still 2 impostors in the #6117 repro. + # - `HEAD^1`. Correct on a merge ref and silently catastrophic the day + # someone gives the checkout a `ref:`, where parent^1 becomes the PR's + # previous commit. `merge-base` is right under BOTH checkouts, which is + # why it is the one written here. + - name: Resolve the diff base (merge base with the base branch) + id: diffbase + if: steps.labels.outputs.skip != 'true' + env: + BASE_REF: ${{ github.event.pull_request.base.ref }} + PINNED_BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: | + if [ -z "$BASE_REF" ]; then + echo "::error::This event carries no base branch, so the changeset diff base cannot be computed. A gate that cannot read its input has verified nothing, so this is a failure rather than a pass (#4690)." + exit 1 + fi + if ! git rev-parse --verify --quiet "refs/remotes/origin/$BASE_REF^{commit}" >/dev/null; then + git fetch --no-tags --quiet origin "+refs/heads/$BASE_REF:refs/remotes/origin/$BASE_REF" \ + || echo "::warning::Could not fetch origin/$BASE_REF; the merge-base resolution below will decide." + fi + # `if !` rather than a bare assignment on purpose: these steps run under + # `bash -e` (no `shell:` key anywhere in this file), where a failing + # command substitution kills the step with no message at all. The gate + # is allowed to fail here -- it is NOT allowed to fail unexplained. + if ! MERGE_BASE=$(git merge-base "refs/remotes/origin/$BASE_REF" HEAD); then + echo "::error::Could not compute merge-base(origin/$BASE_REF, HEAD), so the changeset diff has no trustworthy starting point. Failing rather than falling back to the frozen base.sha, which is the #6129 defect itself." + exit 1 + fi + echo "merge_base=$MERGE_BASE" >> "$GITHUB_OUTPUT" + # The drift is printed, not just corrected. #6129 was invisible for as + # long as it was because nothing in the log ever said which commit the + # diff started from; this line is what makes the next occurrence of the + # family readable straight off the step output. + DRIFT=$(git rev-list --count "$PINNED_BASE_SHA..$MERGE_BASE" 2>/dev/null || echo '?') + echo "Diff base: $MERGE_BASE (merge-base of origin/$BASE_REF and HEAD)" + echo "Frozen payload base.sha: $PINNED_BASE_SHA -- $BASE_REF has moved $DRIFT commit(s) since it was frozen, and that drift is exactly what this gate used to count as this PR's own." + - name: Setup Node.js if: steps.labels.outputs.skip != 'true' uses: actions/setup-node@v7 @@ -236,7 +304,7 @@ jobs: - name: Check for a changeset added by this PR if: steps.labels.outputs.skip != 'true' env: - BASE_SHA: ${{ github.event.pull_request.base.sha }} + MERGE_BASE: ${{ steps.diffbase.outputs.merge_base }} run: | if [ ! -d ".changeset" ]; then echo "::warning::.changeset directory not found. Skipping changeset check." @@ -248,8 +316,11 @@ jobs: # .md file, so the directory is permanently non-empty and the gate can # never go red. #3373 merged a real spec/api-surface fix with no # changeset while this step happily reported "Found 104 changeset(s)". - # Diffing against BASE_SHA ignores that residue and sees only what the - # PR itself introduced. + # Diffing against the merge base ignores that residue and sees only + # what the PR itself introduced. It has to be the MERGE BASE and not + # the payload's frozen `base.sha` -- see the base-resolution step above + # (#6129); with the frozen sha this count silently included every + # changeset main gained while the PR was open. # # An empty-frontmatter changeset still COUNTS here — this step counts # files, and that is deliberately unchanged. What has changed is that @@ -268,7 +339,7 @@ jobs: # for new files. Splitting it across two steps is what keeps THIS # step's failure mode ("no changeset at all") distinct from that one's # ("the changeset you added declares nothing"). - ADDED=$(git diff --name-only --diff-filter=A "$BASE_SHA" HEAD -- '.changeset/*.md' \ + ADDED=$(git diff --name-only --diff-filter=A "$MERGE_BASE" HEAD -- '.changeset/*.md' \ | grep -v '/README\.md$' | wc -l | tr -d '[:space:]') if [ "$ADDED" -eq 0 ]; then # The full comparison goes to the job log — that is what an author @@ -362,13 +433,19 @@ jobs: # one. A consistent exemption beats a nondeterministic gate, and the case # is empty of motive anyway: an author who already has the label gains # nothing by adding the file. + # + # `--base` takes the same merge base the counting step uses, for the same + # #6129 reason: fed the payload's frozen `base.sha`, this script reads every + # empty changeset main gained while the PR was open as one this PR added, + # and reports it against an author who never touched the file. Same defect, + # opposite direction (a false RED here, a false GREEN up there), one base. - name: Reject an empty-frontmatter changeset added by this PR if: steps.labels.outputs.skip != 'true' env: - BASE_SHA: ${{ github.event.pull_request.base.sha }} + MERGE_BASE: ${{ steps.diffbase.outputs.merge_base }} run: | node scripts/check-empty-changeset.mjs --self-test - node scripts/check-empty-changeset.mjs --base "$BASE_SHA" + node scripts/check-empty-changeset.mjs --base "$MERGE_BASE" - name: Guard against accidental major bumps (launch window) # Every publishable package is in one Changesets "fixed" (lockstep) group, diff --git a/scripts/check-empty-changeset.mjs b/scripts/check-empty-changeset.mjs index b489c138da..804a0fc408 100644 --- a/scripts/check-empty-changeset.mjs +++ b/scripts/check-empty-changeset.mjs @@ -8,6 +8,10 @@ // node scripts/check-empty-changeset.mjs --self-test # verify the checker itself // node scripts/check-empty-changeset.mjs --list # audit the whole .changeset dir // +// `--base` names the BRANCH POINT to judge against, not the first commit of the +// diff: the scan always starts at `merge-base(, )`. See "Where the +// diff starts" below -- getting this wrong is #6129, and it is worth reading. +// // ## The rule (#5471) // // An empty-frontmatter changeset -- a `.changeset/*.md` whose frontmatter block @@ -73,11 +77,40 @@ // nothing. Row 3 is what keeps the stock exempt even when a PR edits an existing // empty file's prose, which is a legitimate thing to do and releases nothing new. // +// ## Where the diff starts (#6129) +// +// "What the PR introduces" is a claim about ONE SIDE of a fork, so the scan +// starts at `merge-base(base, head)` and never at `base` itself. Two things go +// wrong when it starts at `base`, and they were measured in temp repos rather +// than reasoned about: +// +// - Fed a FROZEN commit (CI used to hand this script +// `github.event.pull_request.base.sha`, pinned when the PR was opened), +// every changeset main gained while the PR sat open reads as `A` -- added by +// this PR. A merged PR's empty changeset then goes red against an author who +// never touched the file. Note the merge base does NOT rescue that spelling: +// a frozen base.sha is already an ANCESTOR of head, so it IS its own merge +// base. The caller has to stop pinning; that is the workflow's half of #6129. +// - Fed a moving BRANCH (`origin/main`, this script's own default), a two-dot +// diff misreads DELETIONS on the base branch as additions on this one. The +// live trigger is queued: `changeset pre exit` deletes every consumed +// changeset from main, and the next `pnpm check:empty-changeset` on any +// branch that has not rebased then reports all ~182 stock empties as brand +// new. Measured on a temp repo: 2 fixtures deleted on main -> 2 violations +// two-dot, 0 from the merge base. +// +// One rule covers both, and it is idempotent: `merge-base(X, head)` is `X` again +// whenever `X` is already the branch point, so a caller that hands over an +// exact merge base loses nothing by this. +// // ## Missing input is a failure, never a pass (#4690) // -// An unresolvable base ref exits 1 rather than 0. A gate that cannot read its -// input has verified nothing, and exiting 0 there is the #4690 anti-pattern -- -// a check that skips silently and reads as "no violations" in every checks list. +// An unresolvable base ref exits 1 rather than 0. So does a base with no merge +// base against head at all (unrelated histories) -- it is the same fact one step +// later, and falling back to the raw base there would quietly restore the bug +// above. A gate that cannot read its input has verified nothing, and exiting 0 +// there is the #4690 anti-pattern -- a check that skips silently and reads as +// "no violations" in every checks list. // // Zero third-party dependencies, so it can run in a minimal CI environment. @@ -152,16 +185,48 @@ function resolveCommit(ref, cwd) { } } +/** + * The commit the diff actually starts at: the merge base of `base` and `head`. + * `null` when the two have no common ancestor -- the caller fails on that rather + * than falling back to `base`, see "Where the diff starts" (#6129). + * + * @param {string} base + * @param {string} head + * @param {string} cwd + * @returns {string|null} + */ +export function mergeBase(base, head, cwd) { + try { + return git(['merge-base', base, head], cwd).trim() || null; + } catch { + return null; + } +} + // ── The scan ───────────────────────────────────────────────────────────────── /** * Judge the changesets this diff introduces. * + * `base` is the branch point to judge against; the diff itself starts at + * `merge-base(base, head)`, which is what makes the verdict a function of THIS + * side of the fork alone (#6129). Resolving it HERE rather than in the caller is + * deliberate: this is the function the self-test drives, and a correction that + * lived in the CLI could be dropped from it without a single fixture noticing. + * * @param {{ cwd: string, base: string, head?: string }} opts - * @returns {{ violations: {file: string, kind: string}[], exempt: string[], ok: string[] }} + * @returns {{ violations: {file: string, kind: string}[], exempt: string[], ok: string[], base: string }} + * @throws when `base` and `head` have no merge base (#4690: not a pass) */ export function scan({ cwd, base, head = 'HEAD' }) { - const out = git(['diff', '--name-status', '--diff-filter=AM', base, head, '--', '.changeset/*.md'], cwd); + 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.', + ); + } + const out = git(['diff', '--name-status', '--diff-filter=AM', from, head, '--', '.changeset/*.md'], cwd); const violations = []; const exempt = []; @@ -186,12 +251,12 @@ export function scan({ cwd, base, head = 'HEAD' }) { // Modified. Exempt only if it was ALREADY an empty declaration at base -- // i.e. this PR did not create the empty declaration, it inherited it. - const baseText = showOrNull(base, file, cwd); + const baseText = showOrNull(from, file, cwd); if (baseText !== null && isEmptyDeclaration(baseText)) exempt.push(file); else violations.push({ file, kind: 'emptied' }); } - return { violations, exempt, ok }; + return { violations, exempt, ok, base: from }; } // ── Reporting ──────────────────────────────────────────────────────────────── @@ -312,6 +377,60 @@ function selfTest() { 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 + * the two -- the very thing `actions/checkout` puts at HEAD on a + * `pull_request` event when no `ref:` is given. + * + * Faking this with two linear commits would test an imitation of the code path + * that ships: the whole defect lives in the difference between a merge commit's + * two parents, so the fixture has to have two parents. + * + * @param {{ baseFiles?: Record, prFiles?: Record, + * driftFiles?: Record }} opts + * @returns {{ dir: string, pinned: string, mainTip: string }} + * `pinned` is what the event payload freezes as `base.sha` at PR-open time. + */ + const makeMergeRefRepo = ({ baseFiles = {}, prFiles = {}, driftFiles = {} }) => { + const dir = mkdtempSync(join(tmpdir(), 'check-empty-changeset-mergeref-')); + 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 pinned = git(['rev-parse', 'HEAD'], dir).trim(); // the frozen base.sha + + git(['checkout', '-q', '-b', 'pr'], dir); + apply(prFiles); + git(['commit', '-q', '-m', 'pr: this PR own side', '--allow-empty', '--no-gpg-sign'], dir); + + git(['checkout', '-q', 'main'], dir); + apply(driftFiles); + git(['commit', '-q', '-m', "main: somebody else's PR merged", '--allow-empty', '--no-gpg-sign'], dir); + const mainTip = git(['rev-parse', 'HEAD'], dir).trim(); + + // GitHub builds the merge ref exactly this way: base branch tip, merge the + // PR head with --no-ff. Then detach, because CI stands ON the merge commit. + git(['checkout', '-q', '-b', 'merge-ref', 'main'], dir); + git(['merge', '-q', '--no-ff', '--no-gpg-sign', '-m', 'Merge pr into main', 'pr'], dir); + git(['checkout', '-q', '--detach', 'HEAD'], dir); + return { dir, pinned, mainTip }; + }; + try { // ── RED 1: a PR that ADDS an empty-frontmatter changeset ───────────────── // The #5799 shape verbatim: a skills/** change declaring nothing, via a new @@ -411,6 +530,189 @@ function selfTest() { assert(r.violations.length === 0, 'GREEN 5: .changeset/README.md must never be judged as a changeset'); } + // ── #6129: main drift must not move the verdict, in EITHER direction ───── + // + // The gate's contract is "same diff, same verdict" -- what a PR introduces + // cannot depend on what OTHER PRs merged while it sat open. Two fixtures, + // deliberately identical except for which side of the fork the offending + // changeset is on, because a drift assertion on its own is satisfied by a + // gate that has simply stopped looking. + { + const OFFENDER = '.changeset/an-empty-one.md'; + + // DRIFT, must NOT fire: the empty changeset arrives on MAIN, from someone + // else's merged PR. This PR touched nothing but source. Judged from the + // base BRANCH the file is on both sides of the diff and invisible, which + // is the point. + const drift = makeMergeRefRepo({ + baseFiles: { '.changeset/README.md': '# Changesets\n', 'src/app.ts': 'export const v = 1;\n' }, + prFiles: { 'src/app.ts': 'export const v = 2;\n' }, + driftFiles: { [OFFENDER]: EMPTY }, + }); + const drifted = scan({ cwd: drift.dir, base: 'main' }); + assert( + drifted.violations.length === 0, + "#6129 DRIFT: an empty changeset that MAIN gained after this PR forked must not be reported against this PR", + ); + + // The same repo judged from the FROZEN base.sha -- what CI used to pass. + // Asserted rather than merely described: this is the whole defect, and the + // fixture is here to stop anyone re-pinning the base "because it is the + // obvious commit to diff against". Note it stays wrong even now that the + // scan takes a merge base, because a frozen ancestor IS its own merge base. + const frozen = scan({ cwd: drift.dir, base: drift.pinned }); + assert( + frozen.violations.length === 1 && frozen.violations[0]?.file === OFFENDER, + "#6129 DRIFT: judged from the frozen base.sha the same repo blames this PR for main's file -- the defect, pinned", + ); + assert( + frozen.base === drift.pinned, + '#6129 DRIFT: a frozen ancestor is its own merge base, so no merge base can rescue that spelling -- the CALLER must stop pinning', + ); + + // MUST fire: byte-identical drift on main, but this time the PR itself is + // the one adding the empty changeset. A fix that made the drift case green + // by looking at less would take this one green too. + const own = makeMergeRefRepo({ + baseFiles: { '.changeset/README.md': '# Changesets\n', 'src/app.ts': 'export const v = 1;\n' }, + prFiles: { [OFFENDER]: EMPTY }, + driftFiles: { '.changeset/somebody-elses.md': DECLARING }, + }); + const owned = scan({ cwd: own.dir, base: 'main' }); + assert( + owned.violations.length === 1 && owned.violations[0]?.file === OFFENDER, + '#6129 OWN SIDE: an empty changeset this PR really adds must still go red, drift or no drift', + ); + assert( + owned.ok.length === 0, + "#6129 OWN SIDE: main's own changeset must not be counted as introduced by this PR either", + ); + + // Same diff, same verdict -- stated as one assertion rather than left to be + // inferred from the two above. `mainTip` advancing is exactly what turned + // PR #6117 from red at 02:22Z into green at 02:39Z. + assert( + scan({ cwd: drift.dir, base: 'main' }).violations.length === + scan({ cwd: drift.dir, base: drift.mainTip }).violations.length, + '#6129: the verdict must not depend on how far the base branch has run ahead', + ); + } + + // ── #6129, the other half: a base branch that DELETES ──────────────────── + // Not the CI shape -- an ordinary branch and the default `--base origin/main` + // of `pnpm check:empty-changeset`. `changeset pre exit` deletes every consumed + // changeset from main, and a two-dot diff then reads each one still sitting on + // an un-rebased branch as newly added AND empty. This is the fixture that goes + // red if the merge base is taken back out of scan() itself. + { + const dir = mkdtempSync(join(tmpdir(), 'check-empty-changeset-deleted-')); + repos.push(dir); + const write = (rel, contents) => { + mkdirSync(dirname(join(dir, rel)), { recursive: true }); + writeFileSync(join(dir, rel), contents); + }; + 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); + write('.changeset/stock-empty-a.md', EMPTY); + write('.changeset/stock-empty-b.md', EMPTY); + write('src/app.ts', 'export const v = 1;\n'); + git(['add', '-A'], dir); + git(['commit', '-q', '-m', 'base', '--no-gpg-sign'], dir); + const fork = git(['rev-parse', 'HEAD'], dir).trim(); + + git(['checkout', '-q', '-b', 'feature'], dir); + write('src/app.ts', 'export const v = 2;\n'); + git(['add', '-A'], dir); + git(['commit', '-q', '-m', 'feature: source only', '--no-gpg-sign'], dir); + + git(['checkout', '-q', 'main'], dir); + rmSync(join(dir, '.changeset/stock-empty-a.md')); + rmSync(join(dir, '.changeset/stock-empty-b.md')); + git(['add', '-A'], dir); + git(['commit', '-q', '-m', 'main: changeset pre exit, consumed changesets deleted', '--no-gpg-sign'], dir); + git(['checkout', '-q', 'feature'], dir); + + const r = scan({ cwd: dir, base: 'main' }); + assert( + r.violations.length === 0, + '#6129 DELETED-ON-MAIN: stock empty changesets deleted on main must not read as added by a branch that merely still carries them', + ); + assert( + r.base === fork, + '#6129 DELETED-ON-MAIN: the scan must start at the fork point, not at the moved branch tip', + ); + } + + // ── #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. + { + const { dir } = makeRepo({}, { 'a.txt': 'x\n' }); + const other = mkdtempSync(join(tmpdir(), 'check-empty-changeset-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); + git(['commit', '-q', '-m', 'unrelated', '--allow-empty', '--no-gpg-sign'], other); + const unrelated = git(['rev-parse', 'HEAD'], other).trim(); + git(['fetch', '-q', other, 'main'], dir); + let threw = false; + try { + scan({ cwd: dir, base: unrelated }); + } catch { + threw = true; + } + assert(threw, '#4690: unrelated histories have no merge base, and that is a failure rather than a silent pass'); + } + + // ── The consumer: this gate's own CI step (#6129) ──────────────────────── + // + // A self-test that only ever drives scan() cannot see the half of #6129 that + // lives in YAML -- and that half is where the false GREEN was. The count that + // let PR #6117 through is a shell line in pr-automation.yml, so the fixture + // for it has to read that file. Without this block the workflow could be + // reverted to the frozen `base.sha` tomorrow with every assertion above still + // green, which is precisely the "returns in a different shape" #6129 rules out. + { + const workflow = join(REPO_ROOT, '.github/workflows/pr-automation.yml'); + const present = existsSync(workflow); + assert(present, 'consumer: .github/workflows/pr-automation.yml must exist -- it is the step this gate runs in'); + const yaml = present ? readFileSync(workflow, 'utf8') : ''; + + assert( + /git merge-base "refs\/remotes\/origin\/\$BASE_REF" HEAD/.test(yaml), + 'consumer: the Check Changeset job must derive its diff base from `git merge-base origin/ HEAD`', + ); + assert( + /--diff-filter=A "\$MERGE_BASE" HEAD -- '\.changeset\/\*\.md'/.test(yaml), + 'consumer: the changeset COUNT must diff from $MERGE_BASE (never the frozen base.sha) -- that count going green on main drift is #6129', + ); + // Every `--base` handed to this script, not just the one that exists today: + // a second call site added later with a pinned sha is the same bug again. + const bases = [...yaml.matchAll(/check-empty-changeset\.mjs --base (\S+)/g)].map((m) => m[1]); + assert(bases.length === 1, 'consumer: exactly one `check-empty-changeset.mjs --base` call site is expected in the workflow'); + assert( + bases.every((b) => b === '"$MERGE_BASE"'), + 'consumer: every `check-empty-changeset.mjs --base` in the workflow must be handed $MERGE_BASE', + ); + // The endpoint rule itself, independent of the spellings above: no diff in + // this workflow may start at the payload's frozen base.sha. Comment lines + // are excluded because the rule is about what RUNS -- and because the + // workflow's own note on why `git diff base.sha...HEAD` is not a fix would + // otherwise be the first thing this catches (it was). + const pinnedDiffs = yaml + .split('\n') + .filter((line) => !/^\s*#/.test(line)) + .filter((line) => /\bgit diff\b/.test(line) && /BASE_SHA|base\.sha/.test(line)); + assert( + pinnedDiffs.length === 0, + `consumer: no \`git diff\` in the workflow may use the frozen base.sha as an endpoint (found ${pinnedDiffs.length})`, + ); + } + // ── Parser unit rows ───────────────────────────────────────────────────── assert(isEmptyDeclaration('---\n---\n\nbody\n'), 'parser: the canonical empty shape is empty'); assert(isEmptyDeclaration('\n---\n\n---\n\nbody\n'), 'parser: blank lines around/inside the fence stay empty'); @@ -459,6 +761,7 @@ if (argv.includes('--self-test')) { const requested = readFlag('--base'); let base = null; + let baseLabel = requested; if (requested) { base = resolveCommit(requested, REPO_ROOT); if (!base) { @@ -469,7 +772,10 @@ if (argv.includes('--self-test')) { } else { for (const candidate of ['origin/main', 'main']) { base = resolveCommit(candidate, REPO_ROOT); - if (base) break; + if (base) { + baseLabel = candidate; + break; + } } if (!base) { console.error('⛔ check-empty-changeset: no base to diff against (tried origin/main, main).'); @@ -478,7 +784,20 @@ if (argv.includes('--self-test')) { } } - const { violations, exempt, ok } = scan({ cwd: REPO_ROOT, base, head }); + let result; + try { + result = scan({ 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; + // 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}).`); if (violations.length) { report(violations); process.exit(1);