Merge pull request #7533 from Shopify/scope-checkchangesets-to-pr-diff

Fix breaking-change check flagging pre-existing major changesets on main

Author: alfonso-noriega

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})
 

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