From 9479932846664e8d6b583d9b876c691c233d6b6d Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Thu, 30 Jul 2026 10:54:34 +0200 Subject: [PATCH 1/2] ci: decide the changeset allowlist before validating the queue The gate ran `changeset status` before it worked out whether the PR was allowlisted. That command errors with "Some packages have been changed but no changesets were found" for any diff inside a package, and a docs-only PR is such a diff, so the check failed on exactly the PRs the allowlist exists to exempt and the exemption step never ran. PR #715 changes markdown only and fails this way today. The allowlist verdict now runs first and publishes what it decided. The Changesets queue validation still runs whenever the PR needs a changeset, and also when an allowlisted PR carries one anyway: a `.changeset/*.md` file matches the markdown allowlist, so a changeset-only PR is exempt from the gate and would otherwise ship its entry unvalidated. Co-authored-by: Claude Opus 5 --- .github/workflows/changeset-check.yml | 93 ++++++++++++++++++--------- 1 file changed, 63 insertions(+), 30 deletions(-) diff --git a/.github/workflows/changeset-check.yml b/.github/workflows/changeset-check.yml index 4100637f..5687a0c5 100644 --- a/.github/workflows/changeset-check.yml +++ b/.github/workflows/changeset-check.yml @@ -11,6 +11,11 @@ name: changeset-check # cases the allowlist doesn't anticipate). # 3. All changed files are in the path allowlist → skip. # 4. Otherwise: require at least one added .changeset/.md file. +# +# Layer 3 is decided before Changesets runs, because `changeset status` +# reports "packages have been changed but no changesets were found" for +# any diff inside a package — a docs-only PR included. Validating the +# queue first therefore failed exactly the PRs the allowlist exempts. on: pull_request: @@ -71,29 +76,8 @@ jobs: - name: Validate package and lockfile metadata run: node --test web/release-metadata.test.mjs - # Presence alone is not enough: a Changeset can reference a package that - # no longer exists after a rename. Validate the complete pending queue - # with Changesets itself so an invalid entry fails on the PR instead of - # breaking every release run after it reaches master. - - name: Setup Node for Changesets validation - if: steps.version_pr.outputs.skip != 'true' && steps.label.outputs.skip != 'true' - uses: actions/setup-node@v7 - with: - node-version: '22' - cache: npm - cache-dependency-path: package-lock.json - - - name: Install Changesets dependencies - if: steps.version_pr.outputs.skip != 'true' && steps.label.outputs.skip != 'true' - run: npm ci --prefer-offline --no-audit --no-fund - - - name: Validate pending Changesets - if: steps.version_pr.outputs.skip != 'true' && steps.label.outputs.skip != 'true' - # pull_request workflows run on a detached merge ref. Compare against - # the fetched remote base because no local `master` branch exists. - run: npx changeset status --since="origin/${{ github.base_ref }}" - - name: Verify changeset present (or PR is allowlisted) + id: gate if: steps.version_pr.outputs.skip != 'true' && steps.label.outputs.skip != 'true' env: BASE_SHA: ${{ github.event.pull_request.base.sha }} @@ -104,7 +88,26 @@ jobs: git fetch --no-tags origin "${BASE_SHA}" "${HEAD_SHA}" CHANGED="$(git diff --name-only "${BASE_SHA}"..."${HEAD_SHA}")" + + # Accept any added or modified changeset file. Exclude + # deletions (--diff-filter=d) so a PR that *deletes* a + # changeset can't satisfy the rule. Exclude the README + + # config files which are infrastructure, not real entries. + # Computed up front: the verdict below and the Changesets + # validation that follows need the same answer. + CHANGESETS="$(git diff --name-only --diff-filter=d "${BASE_SHA}"..."${HEAD_SHA}" \ + | grep -E '^\.changeset/[^/]+\.md$' \ + | grep -vE '^\.changeset/(README)\.md$' \ + || true)" + + if [ -n "${CHANGESETS}" ]; then + echo "has_changesets=true" >> "${GITHUB_OUTPUT}" + else + echo "has_changesets=false" >> "${GITHUB_OUTPUT}" + fi + if [ -z "${CHANGED}" ]; then + echo "exempt=true" >> "${GITHUB_OUTPUT}" echo "No file changes — passing." exit 0 fi @@ -131,6 +134,7 @@ jobs: || true)" if [ -z "${REQUIRES_CHANGESET}" ]; then + echo "exempt=true" >> "${GITHUB_OUTPUT}" echo "✓ PR changes only docs / repo config / CI tooling (allowlist) — changeset not required." echo "" echo "Files in this PR:" @@ -138,14 +142,7 @@ jobs: exit 0 fi - # Accept any added or modified changeset file. Exclude - # deletions (--diff-filter=d) so a PR that *deletes* a - # changeset can't satisfy the rule. Exclude the README + - # config files which are infrastructure, not real entries. - CHANGESETS="$(git diff --name-only --diff-filter=d "${BASE_SHA}"..."${HEAD_SHA}" \ - | grep -E '^\.changeset/[^/]+\.md$' \ - | grep -vE '^\.changeset/(README)\.md$' \ - || true)" + echo "exempt=false" >> "${GITHUB_OUTPUT}" if [ -n "${CHANGESETS}" ]; then echo "✓ Found changeset(s):" @@ -167,3 +164,39 @@ jobs: echo " - apply the 'no-changeset' label to skip the gate, or" echo " - expand the allowlist in .github/workflows/changeset-check.yml in the same PR." exit 1 + + # Presence alone is not enough: a Changeset can reference a package that + # no longer exists after a rename. Validate the complete pending queue + # with Changesets itself so an invalid entry fails on the PR instead of + # breaking every release run after it reaches master. + # + # Runs when the PR needs a changeset, and also when an allowlisted PR + # carries one anyway — a `.changeset/*.md` file matches the markdown + # allowlist, so a changeset-only PR is exempt from the gate and would + # otherwise ship its entry unvalidated. + - name: Setup Node for Changesets validation + if: >- + steps.version_pr.outputs.skip != 'true' + && steps.label.outputs.skip != 'true' + && (steps.gate.outputs.exempt != 'true' || steps.gate.outputs.has_changesets == 'true') + uses: actions/setup-node@v7 + with: + node-version: '22' + cache: npm + cache-dependency-path: package-lock.json + + - name: Install Changesets dependencies + if: >- + steps.version_pr.outputs.skip != 'true' + && steps.label.outputs.skip != 'true' + && (steps.gate.outputs.exempt != 'true' || steps.gate.outputs.has_changesets == 'true') + run: npm ci --prefer-offline --no-audit --no-fund + + - name: Validate pending Changesets + if: >- + steps.version_pr.outputs.skip != 'true' + && steps.label.outputs.skip != 'true' + && (steps.gate.outputs.exempt != 'true' || steps.gate.outputs.has_changesets == 'true') + # pull_request workflows run on a detached merge ref. Compare against + # the fetched remote base because no local `master` branch exists. + run: npx changeset status --since="origin/${{ github.base_ref }}" From ba8185eb44e80cf05425e8ddf9b099a6d88f714d Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Thu, 30 Jul 2026 11:01:40 +0200 Subject: [PATCH 2/2] test: pin the changeset gate's step order instead of one guard string The workflow-structure test asserted that each Changesets step carried the label guard by matching the exact concatenation of both short-circuits. Adding a third condition to a step therefore read as a removed guard. It now checks the two short-circuits separately, which is as strict about the property it names and lets a step carry a further guard of its own. Adds the assertion this change is actually about: the allowlist verdict must be resolved before Changesets validation runs. Restoring the old order fails the test, so it guards the regression rather than describing it. Co-authored-by: Claude Opus 5 --- web/release-metadata.test.mjs | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/web/release-metadata.test.mjs b/web/release-metadata.test.mjs index 4518e275..24fbb831 100644 --- a/web/release-metadata.test.mjs +++ b/web/release-metadata.test.mjs @@ -169,17 +169,32 @@ describe("release metadata", () => { const presence = changesetCheckWorkflow.indexOf( "- name: Verify changeset present (or PR is allowlisted)", ); - const guard = - "if: steps.version_pr.outputs.skip != 'true' && steps.label.outputs.skip != 'true'"; + // Checked as two separate conditions rather than one exact string: a + // step may carry a further guard of its own — the allowlist verdict + // does — and matching the concatenation would read that as a removed + // short-circuit. + const guards = [ + "steps.version_pr.outputs.skip != 'true'", + "steps.label.outputs.skip != 'true'", + ]; assert.ok(labelDecision >= 0, "workflow must resolve the label exemption"); assert.ok(labelDecision < setup && setup < install && install < validate); + // `changeset status` fails whenever a package changed and no changeset + // was added, which is true of a docs-only PR. Reaching it before the + // allowlist verdict failed exactly the PRs the allowlist exempts. + assert.ok( + presence >= 0 && presence < setup, + "the allowlist verdict must be resolved before Changesets validation runs", + ); for (const step of [setup, install, validate, presence]) { - assert.equal( - changesetCheckWorkflow.slice(step, step + 240).includes(guard), - true, - "every Changesets validation step must use the label guard", - ); + for (const guard of guards) { + assert.equal( + changesetCheckWorkflow.slice(step, step + 320).includes(guard), + true, + `every Changesets validation step must keep the guard: ${guard}`, + ); + } } assert.match(changesetCheckWorkflow, /GITHUB_STEP_SUMMARY/); assert.match(changesetCheckWorkflow, /npx changeset status/);