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 () => {