From f85cd127b952fab3597cd138add7153b655ae0e4 Mon Sep 17 00:00:00 2001 From: Alfonso Noriega Date: Tue, 12 May 2026 12:52:02 +0200 Subject: [PATCH] Scope checkChangesets() to PR-diffed files The breaking-change detection script already scopes its OCLIF manifest and Zod schema scans to files actually modified in the PR diff, but checkChangesets() was scanning every .changeset/*.md regardless. That meant a major changeset already merged to main (e.g. one staged for the next major release) would re-fail the breaking-change job on every unrelated PR. Scope checkChangesets() to changedFiles the same way as the manifest and schema scans, preserving the legacy 'scan everything' behavior when the script is invoked locally without GITHUB_BASE_REF. Adds three regression tests: - only PR-introduced major changesets are flagged - legacy local mode (no changedFiles) still scans everything - no flag when the PR doesn't touch any changeset --- workspace/src/major-change-check.js | 22 +++++-- workspace/src/major-change-check.test.js | 76 +++++++++++++++++++++++- 2 files changed, 93 insertions(+), 5 deletions(-) diff --git a/workspace/src/major-change-check.js b/workspace/src/major-change-check.js index e52053576c..80b790acfd 100644 --- a/workspace/src/major-change-check.js +++ b/workspace/src/major-change-check.js @@ -93,18 +93,32 @@ async function defaultRunGit(args) { // 1. Check changesets for major bumps // --------------------------------------------------------------------------- -async function checkChangesets() { +export async function checkChangesets({changedFiles, cwd = currentDirectory} = {}) { logSection('Checking changesets for major bumps') const changesetFiles = await fg('.changeset/*.md', { - cwd: currentDirectory, + cwd, absolute: true, ignore: ['**/README.md'], }) + // Scope to changesets the PR actually touched. Without this, an + // in-flight major changeset already merged to `main` (e.g. staged for + // the next major release) re-flags every unrelated PR opened against + // `main`. Mirrors the same scoping applied to the manifest and schema + // scans below. + const filesToCheck = changedFiles + ? changesetFiles.filter((file) => changedFiles.has(path.relative(cwd, file))) + : changesetFiles + + if (changedFiles && filesToCheck.length === 0) { + logMessage('No changeset files changed in this PR, skipping') + return [] + } + const majorChangesets = [] - for (const file of changesetFiles) { + for (const file of filesToCheck) { const content = (await fs.readFile(file, 'utf-8')).trim() // Changeset format: YAML frontmatter between --- markers, with 'package: major' const frontmatterMatch = content.match(/^---\n([\s\S]*?)\n---/) @@ -662,7 +676,7 @@ async function runMain() { ref: context.baselineRef, }) - const majorChangesets = await checkChangesets() + const majorChangesets = await checkChangesets({changedFiles: context.changedFiles}) const manifestChanges = await checkManifest(baselineDirectory, {changedFiles: context.changedFiles}) const schemaChanges = await checkSchemas(baselineDirectory, {changedFiles: context.changedFiles}) diff --git a/workspace/src/major-change-check.test.js b/workspace/src/major-change-check.test.js index 7c14748706..0af5762bd1 100644 --- a/workspace/src/major-change-check.test.js +++ b/workspace/src/major-change-check.test.js @@ -13,7 +13,11 @@ import {test} from 'node:test' import assert from 'node:assert/strict' -import {extractSchemaFields, resolveContext, stripStringsAndComments} from './major-change-check.js' +import {mkdtemp, rm, writeFile, mkdir} from 'node:fs/promises' +import os from 'node:os' +import * as path from 'pathe' + +import {checkChangesets, extractSchemaFields, resolveContext, stripStringsAndComments} from './major-change-check.js' test('extracts top-level keys from a flat .object({...})', () => { const src = ` @@ -185,6 +189,76 @@ test('resolveContext: git failure degrades to scanning everything against main', assert.equal(ctx.changedFiles, null, 'git failure must NOT collapse to an empty diff set') }) +// --------------------------------------------------------------------------- +// checkChangesets() — only flag changesets the PR actually touched +// --------------------------------------------------------------------------- + +test('checkChangesets: ignores major changesets that were not added by this PR', async () => { + // Stand up a fake repo containing two changesets on disk — one already + // on main (not in the PR diff) and one introduced by this PR. Only the + // latter should be reported. This is the regression for PR #7532, where + // an in-flight major changeset (`thin-webs-notice.md`) on `main` was + // failing the breaking-change check on every unrelated PR. + const tmp = await mkdtemp(path.join(os.tmpdir(), 'changeset-scope-')) + try { + await mkdir(path.join(tmp, '.changeset'), {recursive: true}) + await writeFile( + path.join(tmp, '.changeset', 'preexisting-major.md'), + `---\n'@shopify/cli': major\n---\n\nStaged for next major.\n`, + ) + await writeFile( + path.join(tmp, '.changeset', 'pr-introduced-major.md'), + `---\n'@shopify/app': major\n---\n\nIntroduced by this PR.\n`, + ) + + const result = await checkChangesets({ + cwd: tmp, + changedFiles: new Set(['.changeset/pr-introduced-major.md']), + }) + assert.equal(result.length, 1, 'only the PR-introduced changeset is flagged') + assert.equal(result[0].file, 'pr-introduced-major.md') + } finally { + await rm(tmp, {recursive: true, force: true}) + } +}) + +test('checkChangesets: with no changedFiles set, scans every changeset (legacy local mode)', async () => { + const tmp = await mkdtemp(path.join(os.tmpdir(), 'changeset-scope-')) + try { + await mkdir(path.join(tmp, '.changeset'), {recursive: true}) + await writeFile( + path.join(tmp, '.changeset', 'a.md'), + `---\n'@shopify/cli': major\n---\n`, + ) + await writeFile( + path.join(tmp, '.changeset', 'b.md'), + `---\n'@shopify/app': major\n---\n`, + ) + const result = await checkChangesets({cwd: tmp}) + assert.equal(result.length, 2) + } finally { + await rm(tmp, {recursive: true, force: true}) + } +}) + +test('checkChangesets: returns empty when none of the changesets were touched by the PR', async () => { + const tmp = await mkdtemp(path.join(os.tmpdir(), 'changeset-scope-')) + try { + await mkdir(path.join(tmp, '.changeset'), {recursive: true}) + await writeFile( + path.join(tmp, '.changeset', 'preexisting.md'), + `---\n'@shopify/cli': major\n---\n`, + ) + const result = await checkChangesets({ + cwd: tmp, + changedFiles: new Set(['packages/app/src/foo.ts']), + }) + assert.equal(result.length, 0) + } finally { + await rm(tmp, {recursive: true, force: true}) + } +}) + test('resolveContext: no GITHUB_BASE_REF falls back to scanning main (local invocation)', async () => { let called = false const runGit = async () => {