diff --git a/.github/workflows/tests-pr.yml b/.github/workflows/tests-pr.yml
index f18a9b69218..67eed3eb0a3 100644
--- a/.github/workflows/tests-pr.yml
+++ b/.github/workflows/tests-pr.yml
@@ -307,7 +307,9 @@ jobs:
with:
repository: ${{ github.event.pull_request.head.repo.full_name || github.event.repository.full_name }}
ref: ${{ github.event.pull_request.head.ref || github.event.merge_group.head_ref }}
- fetch-depth: 1
+ # Full history so `git merge-base origin/ HEAD` (used by
+ # major-change-check.js) can reach the divergence point.
+ fetch-depth: 0
- name: Setup deps
uses: ./.github/actions/setup-cli-deps
with:
diff --git a/workspace/src/major-change-check.js b/workspace/src/major-change-check.js
index 0baf21178fa..e52053576c4 100644
--- a/workspace/src/major-change-check.js
+++ b/workspace/src/major-change-check.js
@@ -7,7 +7,20 @@
* 2. OCLIF manifest changes: removed commands, removed flags, or removed flag env vars
* 3. Zod schema changes: removed or renamed fields in app/extension config schemas
*
- * Compares the current branch against the main branch baseline.
+ * Baseline selection:
+ * - In CI (`GITHUB_BASE_REF` set, by both `pull_request` and `merge_group`)
+ * the baseline is `git merge-base origin/ HEAD`. This avoids false
+ * positives when the base branch has progressed past the PR's branch
+ * point (a field added on `main` after the PR opened would otherwise
+ * show up as "removed" by the PR). Requires `fetch-depth: 0` on the
+ * workflow checkout.
+ * - Otherwise (local invocation, no base ref) falls back to `main`'s
+ * current tip with a full scan (legacy behavior).
+ *
+ * The schema scan and OCLIF manifest comparison are scoped to files that
+ * actually changed in the PR's diff so unrelated drift on `main` cannot
+ * trigger this check.
+ *
* Outputs a GitHub Actions summary and sets a `has_breaking_changes` output.
*/
@@ -17,12 +30,65 @@ import {mkdtemp, rm} from 'fs/promises'
import os from 'os'
import {promises as fs} from 'fs'
import {setOutput} from '@actions/core'
+import {execa} from 'execa'
import {cloneCLIRepository} from './utils/git.js'
import {logMessage, logSection} from './utils/log.js'
import fg from 'fast-glob'
const currentDirectory = path.join(url.fileURLToPath(new URL('.', import.meta.url)), '../..')
+// ---------------------------------------------------------------------------
+// Event context: pick the right baseline and the right diff to scan
+// ---------------------------------------------------------------------------
+
+/**
+ * Resolves the baseline + diff context for the check using local git.
+ *
+ * `GITHUB_BASE_REF` is set on both `pull_request` and `merge_group` events
+ * (and points at the target branch in both cases), so a single git
+ * `merge-base origin/ HEAD` gives us the right baseline. The
+ * surrounding workflow checks out with `fetch-depth: 0` so the merge-base
+ * is reachable.
+ *
+ * Returned shape:
+ * {
+ * baselineRef: string, // SHA (or 'main' in fallback)
+ * changedFiles: Set | null, // null = scan everything (fallback)
+ * }
+ *
+ * On any git failure we degrade *open*: `changedFiles=null` so the scan
+ * widens rather than silently skipping potential removals. We never want
+ * to mask a real breaking change because of a transient git error.
+ */
+export async function resolveContext({
+ baseRef = process.env.GITHUB_BASE_REF,
+ runGit = defaultRunGit,
+} = {}) {
+ // No base ref => running locally or in an unsupported event. Fall back
+ // to legacy behavior so this script remains usable as a manual tool.
+ if (!baseRef) {
+ return {baselineRef: 'main', changedFiles: null}
+ }
+
+ try {
+ const baselineRef = (await runGit(['merge-base', `origin/${baseRef}`, 'HEAD'])).trim()
+ if (!baselineRef) {
+ return {baselineRef: 'main', changedFiles: null}
+ }
+ const diff = await runGit(['diff', '--name-only', `${baselineRef}...HEAD`])
+ const changedFiles = new Set(diff.split('\n').filter(Boolean))
+ return {baselineRef, changedFiles}
+ } catch (error) {
+ logMessage(`git merge-base/diff failed: ${error.message} — falling back to full scan against main`)
+ return {baselineRef: 'main', changedFiles: null}
+ }
+}
+
+async function defaultRunGit(args) {
+ const {stdout} = await execa('git', args)
+ return stdout
+}
+
// ---------------------------------------------------------------------------
// 1. Check changesets for major bumps
// ---------------------------------------------------------------------------
@@ -107,9 +173,17 @@ function extractManifestSurface(manifest) {
return surface
}
-async function checkManifest(baselineDirectory) {
+async function checkManifest(baselineDirectory, {changedFiles} = {}) {
logSection('Checking OCLIF manifest for removed commands/flags')
+ // The OCLIF manifest is a generated artifact — a removed command always
+ // shows up as a `oclif.manifest.json` change. If the file isn't in the
+ // PR diff there is by definition no surface change to detect.
+ if (changedFiles && !changedFiles.has('packages/cli/oclif.manifest.json')) {
+ logMessage('oclif.manifest.json unchanged in this PR, skipping')
+ return {removedCommands: [], removedFlags: [], removedEnvVars: []}
+ }
+
const baselineManifest = await parseManifest(baselineDirectory)
const currentManifest = await parseManifest(currentDirectory)
@@ -390,7 +464,7 @@ export function extractSchemaFields(content) {
return fields
}
-async function checkSchemas(baselineDirectory) {
+async function checkSchemas(baselineDirectory, {changedFiles} = {}) {
logSection('Checking Zod schemas for removed fields')
const schemaGlob = 'packages/app/src/cli/models/**/specifications/**/*.ts'
@@ -402,9 +476,22 @@ async function checkSchemas(baselineDirectory) {
ignore: ignorePatterns,
})
+ // When we know the PR's actual diff, only inspect schema files the PR
+ // touched. Without this, drift on `main` (e.g. a field added after the
+ // PR branched) is reported as a removal. With it, removals come strictly
+ // from this PR's own changes.
+ const filesToCheck = changedFiles
+ ? baselineSchemaFiles.filter((file) => changedFiles.has(file))
+ : baselineSchemaFiles
+
+ if (changedFiles && filesToCheck.length === 0) {
+ logMessage('No schema files changed in this PR, skipping')
+ return []
+ }
+
const removedFields = []
- for (const file of baselineSchemaFiles) {
+ for (const file of filesToCheck) {
const baselinePath = path.join(baselineDirectory, file)
const currentPath = path.join(currentDirectory, file)
@@ -547,29 +634,51 @@ The following Zod schema fields were removed or their files deleted:
// Main
// ---------------------------------------------------------------------------
-const tmpDir = await mkdtemp(path.join(os.tmpdir(), 'major-change-check-'))
-
-try {
- // This script consumes only git-tracked files (oclif.manifest.json + .ts
- // sources). It does not need the baseline's node_modules or dist output,
- // so we skip pnpm install and pnpm build to save ~5–10 minutes of CI
- // per PR. type-diff.js (which diffs `dist/**/*.d.ts`) keeps the default.
- const baselineDirectory = await cloneCLIRepository(tmpDir, {install: false, build: false})
+// `import.meta.main` is only true when this file is run directly. When the
+// test file imports it as a module, we don't want to spawn a baseline clone.
+if (process.argv[1] && url.fileURLToPath(import.meta.url) === path.resolve(process.argv[1])) {
+ await runMain()
+}
- const majorChangesets = await checkChangesets()
- const manifestChanges = await checkManifest(baselineDirectory)
- const schemaChanges = await checkSchemas(baselineDirectory)
+async function runMain() {
+ const tmpDir = await mkdtemp(path.join(os.tmpdir(), 'major-change-check-'))
- const report = buildReport({majorChangesets, manifestChanges, schemaChanges})
+ try {
+ const context = await resolveContext()
+ if (context.baselineRef !== 'main') {
+ logMessage(`Resolved baseline to ${context.baselineRef.slice(0, 7)} (merge-base with base branch)`)
+ }
+ if (context.changedFiles) {
+ logMessage(`PR touched ${context.changedFiles.size} file(s); scoping schema/manifest scan to those`)
+ }
- if (report) {
- logSection('\n⚠️ Breaking changes detected!')
- setOutput('has_breaking_changes', 'true')
- setOutput('report', report)
- } else {
- logSection('\n✅ No breaking changes detected')
- setOutput('has_breaking_changes', 'false')
+ // This script consumes only git-tracked files (oclif.manifest.json + .ts
+ // sources). It does not need the baseline's node_modules or dist output,
+ // so we skip pnpm install and pnpm build to save ~5–10 minutes of CI
+ // per PR. type-diff.js (which diffs `dist/**/*.d.ts`) keeps the default.
+ const baselineDirectory = await cloneCLIRepository(tmpDir, {
+ install: false,
+ build: false,
+ ref: context.baselineRef,
+ })
+
+ const majorChangesets = await checkChangesets()
+ const manifestChanges = await checkManifest(baselineDirectory, {changedFiles: context.changedFiles})
+ const schemaChanges = await checkSchemas(baselineDirectory, {changedFiles: context.changedFiles})
+
+ const report = buildReport({majorChangesets, manifestChanges, schemaChanges})
+
+ if (report) {
+ logSection('\n⚠️ Breaking changes detected!')
+ setOutput('has_breaking_changes', 'true')
+ setOutput('report', report)
+ } else {
+ logSection('\n✅ No breaking changes detected')
+ setOutput('has_breaking_changes', 'false')
+ }
+ } finally {
+ await rm(tmpDir, {recursive: true, force: true, maxRetries: 2})
}
-} finally {
- await rm(tmpDir, {recursive: true, force: true, maxRetries: 2})
}
+
+
diff --git a/workspace/src/major-change-check.test.js b/workspace/src/major-change-check.test.js
index 1f20037e789..7c147487064 100644
--- a/workspace/src/major-change-check.test.js
+++ b/workspace/src/major-change-check.test.js
@@ -13,7 +13,7 @@
import {test} from 'node:test'
import assert from 'node:assert/strict'
-import {extractSchemaFields, stripStringsAndComments} from './major-change-check.js'
+import {extractSchemaFields, resolveContext, stripStringsAndComments} from './major-change-check.js'
test('extracts top-level keys from a flat .object({...})', () => {
const src = `
@@ -135,3 +135,64 @@ test('stripStringsAndComments preserves length and newlines', () => {
assert.equal(stripped.includes('hello'), false)
assert.equal(stripped.includes('trailing'), false)
})
+
+// ---------------------------------------------------------------------------
+// resolveContext()
+// ---------------------------------------------------------------------------
+
+test('resolveContext: derives baseline from `git merge-base origin/ HEAD`', async () => {
+ const calls = []
+ const runGit = async (args) => {
+ calls.push(args)
+ if (args[0] === 'merge-base') {
+ assert.deepEqual(args, ['merge-base', 'origin/main', 'HEAD'])
+ return 'mergebase123\n'
+ }
+ if (args[0] === 'diff') {
+ assert.deepEqual(args, ['diff', '--name-only', 'mergebase123...HEAD'])
+ return 'packages/app/foo.ts\npackages/cli/oclif.manifest.json\n'
+ }
+ throw new Error(`unexpected git call: ${args.join(' ')}`)
+ }
+ const ctx = await resolveContext({baseRef: 'main', runGit})
+ assert.equal(ctx.baselineRef, 'mergebase123', 'uses merge-base SHA, trimmed')
+ assert.deepEqual(
+ [...ctx.changedFiles].sort(),
+ ['packages/app/foo.ts', 'packages/cli/oclif.manifest.json'],
+ )
+ assert.equal(calls.length, 2, 'one merge-base + one diff call')
+})
+
+test('resolveContext: respects non-default base branches (e.g. release branches)', async () => {
+ const runGit = async (args) => {
+ if (args[0] === 'merge-base') {
+ assert.equal(args[1], 'origin/release/3.0', 'forwarded GITHUB_BASE_REF unchanged')
+ return 'releasebase\n'
+ }
+ return ''
+ }
+ const ctx = await resolveContext({baseRef: 'release/3.0', runGit})
+ assert.equal(ctx.baselineRef, 'releasebase')
+})
+
+test('resolveContext: git failure degrades to scanning everything against main', async () => {
+ const runGit = async () => {
+ throw new Error('fatal: Not a valid object name origin/main')
+ }
+ const ctx = await resolveContext({baseRef: 'main', runGit})
+ // We'd rather over-flag than silently miss a real removal.
+ assert.equal(ctx.baselineRef, 'main')
+ assert.equal(ctx.changedFiles, null, 'git failure must NOT collapse to an empty diff set')
+})
+
+test('resolveContext: no GITHUB_BASE_REF falls back to scanning main (local invocation)', async () => {
+ let called = false
+ const runGit = async () => {
+ called = true
+ return ''
+ }
+ const ctx = await resolveContext({baseRef: undefined, runGit})
+ assert.equal(ctx.baselineRef, 'main')
+ assert.equal(ctx.changedFiles, null)
+ assert.equal(called, false, 'must not shell out to git when no base ref is known')
+})
diff --git a/workspace/src/utils/git.js b/workspace/src/utils/git.js
index 6d08d7dca2f..4f93cd55c14 100644
--- a/workspace/src/utils/git.js
+++ b/workspace/src/utils/git.js
@@ -4,7 +4,13 @@ import git from 'simple-git'
import {logMessage, logSection} from './log.js'
/**
- * Clone the Shopify/cli `main` branch into `tmpDir/cli`.
+ * Clone the Shopify/cli repository into `tmpDir/cli` and check out `ref`.
+ *
+ * `ref` defaults to `main` to preserve historical behavior. Callers that
+ * want to diff against the merge-base of a PR (rather than against the
+ * current tip of `main`) should pass the merge-base SHA explicitly so the
+ * baseline reflects the actual fork point and not whatever has landed on
+ * `main` since the PR was opened.
*
* `install` and `build` default to `true` to preserve behavior for
* `type-diff.js`, which diffs `dist/**\/*.d.ts` and therefore needs the
@@ -15,14 +21,21 @@ import {logMessage, logSection} from './log.js'
* PR for those callers.
*
* @param {string} tmpDir
- * @param {{install?: boolean, build?: boolean}} [options]
+ * @param {{install?: boolean, build?: boolean, ref?: string}} [options]
* @returns {Promise} path to the cloned repository
*/
-export async function cloneCLIRepository(tmpDir, {install = true, build = true} = {}) {
- logSection('Setting up baseline: main branch')
+export async function cloneCLIRepository(tmpDir, {install = true, build = true, ref = 'main'} = {}) {
+ logSection(`Setting up baseline: ${ref}`)
const directory = path.join(tmpDir, 'cli')
logMessage('Cloning repository')
+ // Full clone (no `--depth`) so any historical SHA passed as `ref` is
+ // reachable. Shopify/cli's history is small enough that this is fast
+ // and the clone is throwaway.
await git().clone('https://github.com/Shopify/cli.git', directory)
+ if (ref !== 'main') {
+ logMessage(`Checking out ${ref}`)
+ await git(directory).checkout(ref)
+ }
if (install) {
logMessage('Installing dependencies')
await execa('pnpm', ['install'], {cwd: directory})
@@ -33,3 +46,4 @@ export async function cloneCLIRepository(tmpDir, {install = true, build = true}
}
return directory
}
+