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 }}" 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/);