Allow code-owner approval to override breaking-change check

[alfonso-noriega]

WHY are these changes introduced?

The breaking-change check fails hard (exit 1) whenever the script detects a removed Zod field, command, flag, or major changeset. There's currently no way to override it short of force-merging or admin-bypassing branch protection — and after PR #7468 in this stack, the previous (broken) "approval" path is gone too.

In practice, breaking changes are sometimes intentional: removing a deprecated field at a major-version boundary, restructuring a schema after a migration, etc. The team needs a way to acknowledge "yes, we know this is breaking, ship it" that's auditable and lightweight.

The simplest, most natural mechanism is the one already in place for everything else: a code-owner approval on the PR.

WHAT is this pull request doing?

Adds a code-owner-approval override to the breaking-change check. When a code owner of the changed files approves the PR, the check re-runs automatically (via a new pull_request_review trigger) and turns green.

.github/workflows/breaking-change-check.yml (new)

The breaking-change job moved into its own workflow file so we can trigger it on pull_request_review events without re-running the entire test suite (unit tests, e2e, type-diff, etc.) every time someone clicks Approve. Triggers:

• pull_request — same as before
• pull_request_review: types: [submitted, dismissed, edited] — re-runs when an approval is added or withdrawn so the check turns green/red automatically
• merge_group — same as before

Concurrency is keyed per PR (or per merge-queue head) so the latest review event always wins.

.github/workflows/tests-pr.yml

• Removes the now-relocated major-change-check job.

workspace/src/major-change-check.js

When breaking changes are detected, the script now:

1. Fetches the PR's reviews via GET /repos/{owner}/{repo}/pulls/{N}/reviews.
2. Computes the latest review per author (matches GitHub's "latest review wins" semantics — a later CHANGES_REQUESTED cancels an earlier APPROVED, exactly like dismiss_stale_reviews).
3. For each approver, checks GET /repos/{owner}/{repo}/collaborators/{user}/permission. If the permission is admin, maintain, or write, treat the approval as a code-owner override.
4. If override is granted: emit has_breaking_changes=false, log the approver, append an "Override: approved by @user" footer to the sticky comment, and exit cleanly.
5. Otherwise: behave exactly as before — sticky comment + exit 1.

The script also reads CODEOWNERS (.github/CODEOWNERS) and logs which teams own the changed paths for transparency in the run output. The override decision uses the repo-permission check (not direct team-membership lookup) because the default GITHUB_TOKEN can't list arbitrary org-team memberships across Shopify, but it can read repo-level collaborator permissions. Anyone with write on this repo got that access via a code-owning team grant — same security posture as branch protection's "require code-owner review", evaluated earlier so the check can flip without manual CI re-runs.

The override does not auto-approve when the GitHub API fails — every degradation path returns approved: false. We'd rather over-flag than silently grant.

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

10 new tests:

• parseCodeowners — comments and blank lines stripped, owners preserved
• codeownersPatternToRegExp — *, **, anchored, and unanchored patterns
• ownersForFiles — last-matching-rule-wins (matches CODEOWNERS semantics, including theme overriding the catch-all)
• findCodeownerApproval:
  • Approver with write access overrides (✓)
  • Latest review per author wins (a later CHANGES_REQUESTED cancels an earlier APPROVED, sticky-as-dismiss_stale_reviews)
  • No approving reviewer with write access → not approved
  • Missing PR number bails immediately
  • Reviews API failure bails (does NOT silently grant)

Stickiness behaviour

Per the requirement: matches dismiss_stale_reviews. Concretely:

• If the same author submits a later CHANGES_REQUESTED or DISMISSED, the override is gone (we always look at latest review per author).
• After a git push, the PR's existing approving reviews remain (since dismiss_stale_reviews: false on main's branch protection), so the override survives a force-push. If branch-protection ever flips dismiss_stale_reviews to true, GitHub will dismiss the reviews and our check will correctly go red again on the next event.

How to test your changes?

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

All 21 tests pass locally (11 pre-existing + 10 new). End-to-end testing requires a PR that introduces a real or fake breaking change; smoke-test plan:

1. Open a PR that removes a Zod field. The check should be red with the existing sticky-comment.
2. A code owner clicks Approve.
3. The workflow re-triggers (visible in Actions → Breaking change check). The check turns green and the sticky-comment gains the "Override: approved by @user" footer.
4. The reviewer requests changes. The check turns red again.

Post-release steps

None — the workflow takes effect immediately on first run on main.

Checklist

• I've considered possible cross-platform impacts (Mac, Linux, Windows) — pure Node + workflow YAML, no platform calls
• I've considered possible documentation changes
• I've considered analytics changes to measure impact — N/A (CI tooling)
• The change is user-facing — N/A (internal CI tooling, no changeset needed)

[alfonso-noriega]

• Allow code-owner approval to override breaking-change check #7469 👈 (View in Graphite)
• Remove unused 'Breaking change approval' job #7468
• Fix breaking-change check: compare against merge-base, scope to PR diff #7467
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Testing ci checks behavior

[github-actions]

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

• Some seemingly private modules might be re-exported through public modules.
• If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/private/node/conf-store.d.ts

@@ -128,11 +128,10 @@ interface RunWithRateLimitOptions {
 export declare function runWithRateLimit(options: RunWithRateLimitOptions, config?: LocalStorage<ConfSchema>): Promise<boolean>;
 /**
  * Get auto-upgrade preference.
- * Defaults to true if the preference has never been explicitly set.
  *
- * @returns Whether auto-upgrade is enabled.
+ * @returns Whether auto-upgrade is enabled, or undefined if never set.
  */
-export declare function getAutoUpgradeEnabled(config?: LocalStorage<ConfSchema>): boolean;
+export declare function getAutoUpgradeEnabled(config?: LocalStorage<ConfSchema>): boolean | undefined;
 /**
  * Set auto-upgrade preference.
  *

packages/cli-kit/dist/private/node/constants.d.ts

@@ -8,7 +8,6 @@ export declare const environmentVariables: {
     env: string;
     firstPartyDev: string;
     noAnalytics: string;
-    optOutInstrumentation: string;
     appAutomationToken: string;
     partnersToken: string;
     runAsUser: string;

packages/cli-kit/dist/public/node/metadata.d.ts

@@ -34,7 +34,7 @@ export type SensitiveSchema<T> = T extends RuntimeMetadataManager<infer _TPublic
  * @returns A container for the metadata.
  */
 export declare function createRuntimeMetadataContainer<TPublic extends AnyJson, TSensitive extends AnyJson = Record<string, never>>(defaultPublicMetadata?: Partial<TPublic>): RuntimeMetadataManager<TPublic, TSensitive>;
-type CmdFieldsFromMonorail = PickByPrefix<MonorailEventPublic, 'cmd_all_'> & PickByPrefix<MonorailEventPublic, 'cmd_app_'> & PickByPrefix<MonorailEventPublic, 'cmd_create_app_'> & PickByPrefix<MonorailEventPublic, 'cmd_theme_'> & PickByPrefix<MonorailEventPublic, 'store_'> & PickByPrefix<MonorailEventPublic, 'env_auto_upgrade_'>;
+type CmdFieldsFromMonorail = Pick<MonorailEventPublic, 'shop_id'> & PickByPrefix<MonorailEventPublic, 'cmd_all_'> & PickByPrefix<MonorailEventPublic, 'cmd_app_'> & PickByPrefix<MonorailEventPublic, 'cmd_create_app_'> & PickByPrefix<MonorailEventPublic, 'cmd_theme_'> & PickByPrefix<MonorailEventPublic, 'store_'> & PickByPrefix<MonorailEventPublic, 'env_auto_upgrade_'>;
 declare const coreData: RuntimeMetadataManager<CmdFieldsFromMonorail, {
     commandStartOptions: {
         startTime: number;

packages/cli-kit/dist/public/node/monorail.d.ts

@@ -2,7 +2,7 @@ import { JsonMap } from '../../private/common/json.js';
 import { DeepRequired } from '../common/ts/deep-required.js';
 export { DeepRequired };
 type Optional<T> = T | null;
-export declare const MONORAIL_COMMAND_TOPIC = "app_cli3_command/1.24";
+export declare const MONORAIL_COMMAND_TOPIC = "app_cli3_command/1.23";
 export interface Schemas {
     [MONORAIL_COMMAND_TOPIC]: {
         sensitive: {
@@ -32,7 +32,7 @@ export interface Schemas {
             node_version: string;
             is_employee: boolean;
             store_fqdn_hash?: Optional<string>;
-            store_fqdn_validated?: Optional<boolean>;
+            shop_id?: Optional<number>;
             user_id: string;
             cmd_all_alias_used?: Optional<string>;
             cmd_all_launcher?: Optional<string>;

packages/cli-kit/dist/public/node/node-package-manager.d.ts

@@ -13,8 +13,8 @@ export declare const bunLockfile = "bun.lockb";
 export declare const pnpmWorkspaceFile = "pnpm-workspace.yaml";
 /** An array containing the lockfiles from all the package managers */
 export declare const lockfiles: Lockfile[];
-export declare const lockfilesByManager: Record<PackageManager, Lockfile[]>;
-export type Lockfile = 'yarn.lock' | 'package-lock.json' | 'pnpm-lock.yaml' | 'bun.lockb' | 'bun.lock';
+export declare const lockfilesByManager: Record<PackageManager, Lockfile | undefined>;
+export type Lockfile = 'yarn.lock' | 'package-lock.json' | 'pnpm-lock.yaml' | 'bun.lockb';
 /**
  * A union type that represents the type of dependencies in the package.json
  * - dev: devDependencies

packages/cli-kit/dist/public/node/upgrade.d.ts

 import { getAutoUpgradeEnabled, setAutoUpgradeEnabled } from '../../private/node/conf-store.js';
 export { getAutoUpgradeEnabled, setAutoUpgradeEnabled };
 /**
  * Utility function for generating an install command for the user to run
  * to install an updated version of Shopify CLI.
  *
  * @returns A string with the command to run, or undefined if the package manager cannot be determined.
  */
 export declare function cliInstallCommand(): string | undefined;
 /**
- * Options for {@link runCLIUpgrade}.
- */
-export interface RunCLIUpgradeOptions {
-    /**
-     * `true` when the upgrade is being triggered by the automatic postrun hook.
-     * In that case we skip project-local upgrades — those should only happen when the
-     * user explicitly runs `shopify upgrade` so we don't surprise them by mutating
-     * their `package.json` / lockfile in the background.
-     */
-    autoupgrade?: boolean;
-}
-/**
  * Runs the CLI upgrade using the appropriate package manager.
  * Determines the install command and executes it.
  *
- * @param options - See {@link RunCLIUpgradeOptions}.
  * @throws AbortError if the package manager or command cannot be determined.
  */
-export declare function runCLIUpgrade(options?: RunCLIUpgradeOptions): Promise<void>;
+export declare function runCLIUpgrade(): Promise<void>;
 /**
  * Returns the version to auto-upgrade to, or undefined if auto-upgrade should be skipped.
- * Auto-upgrade is enabled by default and can be disabled via `setAutoUpgradeEnabled(false)`.
+ * Auto-upgrade is disabled by default and must be enabled via `shopify upgrade`.
  * Also skips for CI, pre-release versions, or when no newer version is available.
  *
  * @returns The version string to upgrade to, or undefined if no upgrade should happen.
  */
 export declare function versionToAutoUpgrade(): string | undefined;
 /**
  * Checks the freshly fetched notifications feed for a kill-switch notification that
  * disables auto-upgrade. A blocking notification is one with `surface: "autoupgrade"`,
  * `type: "error"`, and matching version/date ranges for the current CLI.
  *
  * Fails open: any error fetching or parsing the feed results in `false`, so a broken
  * notifications endpoint never prevents users from auto-upgrading. Intentionally silent
  * (no logs) — this is invoked on the auto-upgrade hot path.
  *
  * @returns `true` when an active blocking notification is found, `false` otherwise.
  */
 export declare function hasBlockingAutoUpgradeNotification(): Promise<boolean>;
 /**
  * Shows a daily upgrade-available warning for users who have not enabled auto-upgrade.
  * Skipped in CI and for pre-release versions. When auto-upgrade is enabled this is a no-op
  * because the postrun hook will handle the upgrade directly.
  */
 export declare function warnIfUpgradeAvailable(): Promise<void>;
 /**
  * Generates a message to remind the user to update the CLI.
  * For major version bumps, appends a link to the GitHub release notes so users
  * can review breaking changes before deciding to upgrade.
  *
  * @param version - The version to update to.
  * @param isMajor - Whether the version bump is a major version change.
  * @returns The message to remind the user to update the CLI.
  */
 export declare function getOutputUpdateCLIReminder(version: string, isMajor?: boolean): string;
+/**
+ * Prompts the user to enable or disable automatic upgrades, then persists their choice.
+ *
+ * @returns Whether the user chose to enable auto-upgrade.
+ */
+export declare function promptAutoUpgrade(): Promise<boolean>;

packages/cli-kit/dist/public/node/context/local.d.ts

@@ -53,7 +53,7 @@ export declare function isUnitTest(env?: NodeJS.ProcessEnv): boolean;
  * Returns true if reporting analytics is enabled.
  *
  * @param env - The environment variables from the environment of the current process.
- * @returns True unless SHOPIFY_CLI_NO_ANALYTICS or OPT_OUT_INSTRUMENTATION is truthy, or debug mode is enabled.
+ * @returns True unless SHOPIFY_CLI_NO_ANALYTICS is truthy or debug mode is enabled.
  */
 export declare function analyticsDisabled(env?: NodeJS.ProcessEnv): boolean;
 /**

[isaacroldan]

---

Review assisted by pair-review

[isaacroldan]

---

Review assisted by pair-review

[isaacroldan]

💬 Suggestion: The edited type fires when a review's body text is edited, not when its state changes. Editing a review comment doesn't change whether the PR is approved, so this trigger re-runs the full breaking-change check (clone, diff, scan) for zero functional effect. Each unnecessary run costs a full repo clone + schema diff scan. The submitted and dismissed types already cover all state-changing review actions.

Suggestion:

Suggested change

	pull_request_review:
	types: [submitted, dismissed]

[isaacroldan]

💡 Improvement: The breaking-change-check workflow pins DEFAULT_NODE_VERSION: '24.1.0' while tests-pr.yml, tests-main.yml, and tests-manual.yml all use '26.1.0'. This means the script runs on a different Node version than the rest of the CI suite. Likely an oversight from when the job was extracted from tests-pr.yml.

Suggestion:

Suggested change

	DEFAULT_NODE_VERSION: '24.1.0'
	DEFAULT_NODE_VERSION: '26.1.0'

[isaacroldan]

📐 Design: The parseCodeowners, codeownersPatternToRegExp, and ownersForFiles functions (~80 lines of exported, tested code) are only consumed at line 295 for a logMessage call — they never gate the approval decision. The doc-comment at line 248–258 explains why (token limitations prevent resolving team membership), and the transparency logging is a reasonable design choice.

However, the exported/tested surface area creates an expectation that ownership is being enforced. The PR description says 'code-owner-approval override' but the implementation is really 'write-access-approval override'. This isn't wrong — just worth noting that future readers may be surprised by the gap between the CODEOWNERS machinery and its actual role.

Suggestion: Consider either (a) making the CODEOWNERS functions private (unexport) and reducing test coverage to match their log-only role, or (b) adding a brief note in the function doc-comment clarifying that CODEOWNERS matching is informational-only. The current state is correct but potentially misleading.

[isaacroldan]

🎨 Code Style: defaultFetchCompare (lines 134–145) has zero callers anywhere in the codebase. It was likely scaffolded for a compare-API-based approach that was replaced by the git-based diff in resolveContext. All three analysis passes confirmed no references exist.

Suggestion: Delete the function and its doc-comment (lines 134–145).

Suggested change

async function defaultFetchCompare(repo, base, head) {

[isaacroldan]

🐛 Bug: findCodeownerApproval guards on !prNumber but not on !repo. If repo is null and prNumber is truthy, defaultFetchReviews(null, prNumber) constructs ${repo.owner} which throws TypeError: Cannot read properties of null (reading 'owner'). In CI this is unlikely (GITHUB_REPOSITORY is always set), but runMain at line 935 passes context.repo through without a guard, and the script can be invoked locally. The crash is accidentally fail-safe (non-zero exit keeps the check red), but the error message would be baffling.

Suggestion: Add a repo guard alongside the prNumber one:

Suggested change

	if (!prNumber) return {approved: false, approver: null, reason: 'no PR number in event'}
	if (!repo) return {approved: false, approver: null, reason: 'no repo context'}
	if (!prNumber) return {approved: false, approver: null, reason: 'no PR number in event'}