Skip to content

Commit f85cd12

Browse files
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
1 parent c08f463 commit f85cd12

2 files changed

Lines changed: 93 additions & 5 deletions

File tree

workspace/src/major-change-check.js

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,18 +93,32 @@ async function defaultRunGit(args) {
9393
// 1. Check changesets for major bumps
9494
// ---------------------------------------------------------------------------
9595

96-
async function checkChangesets() {
96+
export async function checkChangesets({changedFiles, cwd = currentDirectory} = {}) {
9797
logSection('Checking changesets for major bumps')
9898

9999
const changesetFiles = await fg('.changeset/*.md', {
100-
cwd: currentDirectory,
100+
cwd,
101101
absolute: true,
102102
ignore: ['**/README.md'],
103103
})
104104

105+
// Scope to changesets the PR actually touched. Without this, an
106+
// in-flight major changeset already merged to `main` (e.g. staged for
107+
// the next major release) re-flags every unrelated PR opened against
108+
// `main`. Mirrors the same scoping applied to the manifest and schema
109+
// scans below.
110+
const filesToCheck = changedFiles
111+
? changesetFiles.filter((file) => changedFiles.has(path.relative(cwd, file)))
112+
: changesetFiles
113+
114+
if (changedFiles && filesToCheck.length === 0) {
115+
logMessage('No changeset files changed in this PR, skipping')
116+
return []
117+
}
118+
105119
const majorChangesets = []
106120

107-
for (const file of changesetFiles) {
121+
for (const file of filesToCheck) {
108122
const content = (await fs.readFile(file, 'utf-8')).trim()
109123
// Changeset format: YAML frontmatter between --- markers, with 'package: major'
110124
const frontmatterMatch = content.match(/^---\n([\s\S]*?)\n---/)
@@ -662,7 +676,7 @@ async function runMain() {
662676
ref: context.baselineRef,
663677
})
664678

665-
const majorChangesets = await checkChangesets()
679+
const majorChangesets = await checkChangesets({changedFiles: context.changedFiles})
666680
const manifestChanges = await checkManifest(baselineDirectory, {changedFiles: context.changedFiles})
667681
const schemaChanges = await checkSchemas(baselineDirectory, {changedFiles: context.changedFiles})
668682

workspace/src/major-change-check.test.js

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@
1313
import {test} from 'node:test'
1414
import assert from 'node:assert/strict'
1515

16-
import {extractSchemaFields, resolveContext, stripStringsAndComments} from './major-change-check.js'
16+
import {mkdtemp, rm, writeFile, mkdir} from 'node:fs/promises'
17+
import os from 'node:os'
18+
import * as path from 'pathe'
19+
20+
import {checkChangesets, extractSchemaFields, resolveContext, stripStringsAndComments} from './major-change-check.js'
1721

1822
test('extracts top-level keys from a flat .object({...})', () => {
1923
const src = `
@@ -185,6 +189,76 @@ test('resolveContext: git failure degrades to scanning everything against main',
185189
assert.equal(ctx.changedFiles, null, 'git failure must NOT collapse to an empty diff set')
186190
})
187191

192+
// ---------------------------------------------------------------------------
193+
// checkChangesets() — only flag changesets the PR actually touched
194+
// ---------------------------------------------------------------------------
195+
196+
test('checkChangesets: ignores major changesets that were not added by this PR', async () => {
197+
// Stand up a fake repo containing two changesets on disk — one already
198+
// on main (not in the PR diff) and one introduced by this PR. Only the
199+
// latter should be reported. This is the regression for PR #7532, where
200+
// an in-flight major changeset (`thin-webs-notice.md`) on `main` was
201+
// failing the breaking-change check on every unrelated PR.
202+
const tmp = await mkdtemp(path.join(os.tmpdir(), 'changeset-scope-'))
203+
try {
204+
await mkdir(path.join(tmp, '.changeset'), {recursive: true})
205+
await writeFile(
206+
path.join(tmp, '.changeset', 'preexisting-major.md'),
207+
`---\n'@shopify/cli': major\n---\n\nStaged for next major.\n`,
208+
)
209+
await writeFile(
210+
path.join(tmp, '.changeset', 'pr-introduced-major.md'),
211+
`---\n'@shopify/app': major\n---\n\nIntroduced by this PR.\n`,
212+
)
213+
214+
const result = await checkChangesets({
215+
cwd: tmp,
216+
changedFiles: new Set(['.changeset/pr-introduced-major.md']),
217+
})
218+
assert.equal(result.length, 1, 'only the PR-introduced changeset is flagged')
219+
assert.equal(result[0].file, 'pr-introduced-major.md')
220+
} finally {
221+
await rm(tmp, {recursive: true, force: true})
222+
}
223+
})
224+
225+
test('checkChangesets: with no changedFiles set, scans every changeset (legacy local mode)', async () => {
226+
const tmp = await mkdtemp(path.join(os.tmpdir(), 'changeset-scope-'))
227+
try {
228+
await mkdir(path.join(tmp, '.changeset'), {recursive: true})
229+
await writeFile(
230+
path.join(tmp, '.changeset', 'a.md'),
231+
`---\n'@shopify/cli': major\n---\n`,
232+
)
233+
await writeFile(
234+
path.join(tmp, '.changeset', 'b.md'),
235+
`---\n'@shopify/app': major\n---\n`,
236+
)
237+
const result = await checkChangesets({cwd: tmp})
238+
assert.equal(result.length, 2)
239+
} finally {
240+
await rm(tmp, {recursive: true, force: true})
241+
}
242+
})
243+
244+
test('checkChangesets: returns empty when none of the changesets were touched by the PR', async () => {
245+
const tmp = await mkdtemp(path.join(os.tmpdir(), 'changeset-scope-'))
246+
try {
247+
await mkdir(path.join(tmp, '.changeset'), {recursive: true})
248+
await writeFile(
249+
path.join(tmp, '.changeset', 'preexisting.md'),
250+
`---\n'@shopify/cli': major\n---\n`,
251+
)
252+
const result = await checkChangesets({
253+
cwd: tmp,
254+
changedFiles: new Set(['packages/app/src/foo.ts']),
255+
})
256+
assert.equal(result.length, 0)
257+
} finally {
258+
await rm(tmp, {recursive: true, force: true})
259+
}
260+
})
261+
188262
test('resolveContext: no GITHUB_BASE_REF falls back to scanning main (local invocation)', async () => {
189263
let called = false
190264
const runGit = async () => {

0 commit comments

Comments
 (0)