Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions workspace/src/major-change-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -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---/)
Expand Down Expand Up @@ -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})

Expand Down
76 changes: 75 additions & 1 deletion workspace/src/major-change-check.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = `
Expand Down Expand Up @@ -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 () => {
Expand Down
Loading