diff --git a/.github/workflows/breaking-change-check.yml b/.github/workflows/breaking-change-check.yml
new file mode 100644
index 0000000000..05b1b7f879
--- /dev/null
+++ b/.github/workflows/breaking-change-check.yml
@@ -0,0 +1,97 @@
+name: Breaking change check
+
+# Runs the breaking-change detector on PR opens, updates, AND on review
+# submissions. Re-running on review submissions lets the script flip from
+# red to green when a code-owner approves the PR — see the override logic
+# in workspace/src/major-change-check.js.
+#
+# Lives in a dedicated workflow so review events don't re-trigger the full
+# unit/e2e/type-diff suite.
+on:
+ pull_request:
+ pull_request_review:
+ types: [submitted, dismissed, edited]
+ merge_group:
+
+concurrency:
+ group: shopify-cli-breaking-change-${{ github.event.pull_request.number || github.event.merge_group.head_sha || github.run_id }}
+ cancel-in-progress: true
+
+env:
+ DEBUG: '1'
+ SHOPIFY_CLI_ENV: development
+ SHOPIFY_CONFIG: debug
+ PNPM_VERSION: '10.11.1'
+ BUNDLE_WITHOUT: 'test:development'
+ GH_TOKEN: ${{ secrets.SHOPIFY_GH_READ_CONTENT_TOKEN }}
+ GH_TOKEN_SHOP: ${{ secrets.SHOP_GH_READ_CONTENT_TOKEN }}
+ DEFAULT_NODE_VERSION: '24.1.0'
+
+jobs:
+ major-change-check:
+ # Skip fork PRs — fork tokens are read-only and the codeowner-override
+ # API calls would fail.
+ if: github.event.pull_request.head.repo.full_name == github.repository || github.event_name == 'merge_group'
+ name: 'Breaking change detection'
+ runs-on: ubuntu-latest
+ timeout-minutes: 30
+ permissions:
+ contents: read
+ pull-requests: write
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ repository: ${{ github.event.pull_request.head.repo.full_name || github.event.repository.full_name }}
+ ref: ${{ github.event.pull_request.head.sha || github.event.merge_group.head_sha }}
+ # Full history so `git merge-base origin/ HEAD` (used by
+ # major-change-check.js to scope the diff to PR-only changes) can
+ # reach the divergence point. With fetch-depth: 1 the merge-base
+ # call fails and the script falls back to scanning everything,
+ # surfacing every pre-existing major changeset as "new".
+ fetch-depth: 0
+ - name: Setup deps
+ uses: ./.github/actions/setup-cli-deps
+ with:
+ node-version: ${{ env.DEFAULT_NODE_VERSION }}
+ # No build step: major-change-check.js operates on git-tracked
+ # source files (.ts, oclif.manifest.json, .changeset/*.md) and
+ # `git diff` output only — it never imports built artefacts. The
+ # script itself explicitly opts out of installing/building the
+ # baseline checkout for the same reason (see runMain in
+ # workspace/src/major-change-check.js). Skipping the build keeps
+ # approval-triggered re-runs fast (~30s instead of ~5–8min) so the
+ # check can flip green quickly when a code owner approves.
+ - name: Check for breaking changes
+ id: check
+ env:
+ # The default GITHUB_TOKEN can read PR reviews and the repo's
+ # CODEOWNERS file, which is all the override needs.
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ run: node workspace/src/major-change-check.js
+ - uses: marocchino/sticky-pull-request-comment@fcf6fe9e4a0409cd9316a5011435be0f3327f1e1 # v2.3.1
+ if: steps.check.outputs.has_breaking_changes == 'true'
+ with:
+ header: Breaking-change-detection
+ message: ${{ steps.check.outputs.report }}
+ recreate: true
+ # Override granted: breaking changes were detected but a code-owner
+ # approved, so we keep the sticky comment up to record the override
+ # footer ("✅ Override: approved by @user") emitted by the script.
+ - uses: marocchino/sticky-pull-request-comment@fcf6fe9e4a0409cd9316a5011435be0f3327f1e1 # v2.3.1
+ if: steps.check.outputs.has_breaking_changes != 'true' && steps.check.outputs.report != ''
+ with:
+ header: Breaking-change-detection
+ message: ${{ steps.check.outputs.report }}
+ recreate: true
+ # No breaking changes at all: delete any stale sticky comment from a
+ # previous push.
+ - uses: marocchino/sticky-pull-request-comment@fcf6fe9e4a0409cd9316a5011435be0f3327f1e1 # v2.3.1
+ if: steps.check.outputs.has_breaking_changes != 'true' && steps.check.outputs.report == ''
+ with:
+ header: Breaking-change-detection
+ delete: true
+ - name: Fail if breaking changes detected
+ if: steps.check.outputs.has_breaking_changes == 'true'
+ run: |
+ echo '::error::Breaking changes detected. See the sticky comment on the PR for details. A code-owner approval on this PR will turn this check green.'
+ exit 1
diff --git a/.github/workflows/tests-pr.yml b/.github/workflows/tests-pr.yml
index f2fac95275..a5a3dd7e70 100644
--- a/.github/workflows/tests-pr.yml
+++ b/.github/workflows/tests-pr.yml
@@ -295,43 +295,6 @@ jobs:
message: ${{ steps.type-diff.outputs.report }}
recreate: true
- major-change-check:
- if: github.event.pull_request.head.repo.full_name == github.repository
- name: 'Breaking change detection'
- runs-on: ubuntu-latest
- timeout-minutes: 30
- outputs:
- has_breaking_changes: ${{ steps.check.outputs.has_breaking_changes }}
- steps:
- - uses: actions/checkout@v3
- 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 }}
- # 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:
- node-version: ${{ env.DEFAULT_NODE_VERSION }}
- - name: Build
- run: pnpm nx run-many --all --skip-nx-cache --target=build --output-style=stream
- - name: Check for breaking changes
- id: check
- run: node workspace/src/major-change-check.js
- - uses: marocchino/sticky-pull-request-comment@fcf6fe9e4a0409cd9316a5011435be0f3327f1e1 # v2.3.1
- if: steps.check.outputs.has_breaking_changes == 'true'
- with:
- header: Breaking-change-detection
- message: ${{ steps.check.outputs.report }}
- recreate: true
- - uses: marocchino/sticky-pull-request-comment@fcf6fe9e4a0409cd9316a5011435be0f3327f1e1 # v2.3.1
- if: steps.check.outputs.has_breaking_changes != 'true'
- with:
- header: Breaking-change-detection
- delete: true
- - name: Fail if breaking changes detected
- if: steps.check.outputs.has_breaking_changes == 'true'
- run: |
- echo '::error::Breaking changes detected. See the sticky comment on the PR for details.'
- exit 1
+ # The breaking-change check moved to .github/workflows/breaking-change-check.yml
+ # so it can re-run on `pull_request_review` events without re-triggering
+ # the rest of this workflow.
diff --git a/workspace/src/major-change-check.js b/workspace/src/major-change-check.js
index 9691a8dd34..eca5a8c36a 100644
--- a/workspace/src/major-change-check.js
+++ b/workspace/src/major-change-check.js
@@ -54,41 +54,278 @@ const currentDirectory = path.join(url.fileURLToPath(new URL('.', import.meta.ur
* {
* baselineRef: string, // SHA (or 'main' in fallback)
* changedFiles: Set | null, // null = scan everything (fallback)
+ * repo: {owner, name} | null, // null = couldn't parse GITHUB_REPOSITORY
+ * prNumber: number | null, // null = no PR context (merge_group / local)
* }
*
+ * `repo` / `prNumber` are needed by the code-owner approval override
+ * (see `findCodeownerApproval`). They're populated from the standard
+ * GitHub Actions env vars (`GITHUB_REPOSITORY`, `GITHUB_EVENT_PATH`).
+ * On `merge_group` events the payload doesn't carry a PR number, and
+ * locally there's no event file at all — in both cases `prNumber` is
+ * `null` and the override check short-circuits cleanly.
+ *
* 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,
+ repository = process.env.GITHUB_REPOSITORY,
+ eventPath = process.env.GITHUB_EVENT_PATH,
+ readEvent = defaultReadEvent,
runGit = defaultRunGit,
} = {}) {
+ const repo = parseRepository(repository)
+ const prNumber = await resolvePrNumber(eventPath, readEvent)
+
// 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}
+ return {baselineRef: 'main', changedFiles: null, repo, prNumber}
}
try {
const baselineRef = (await runGit(['merge-base', `origin/${baseRef}`, 'HEAD'])).trim()
if (!baselineRef) {
- return {baselineRef: 'main', changedFiles: null}
+ return {baselineRef: 'main', changedFiles: null, repo, prNumber}
}
const diff = await runGit(['diff', '--name-only', `${baselineRef}...HEAD`])
const changedFiles = new Set(diff.split('\n').filter(Boolean))
- return {baselineRef, changedFiles}
+ return {baselineRef, changedFiles, repo, prNumber}
} catch (error) {
logMessage(`git merge-base/diff failed: ${error.message} — falling back to full scan against main`)
- return {baselineRef: 'main', changedFiles: null}
+ return {baselineRef: 'main', changedFiles: null, repo, prNumber}
+ }
+}
+
+function parseRepository(repository) {
+ if (!repository) return null
+ const [owner, name] = repository.split('/')
+ if (!owner || !name) return null
+ return {owner, name}
+}
+
+async function resolvePrNumber(eventPath, readEvent) {
+ if (!eventPath) return null
+ try {
+ const event = await readEvent(eventPath)
+ if (!event) return null
+ // `pull_request` event => event.pull_request.number
+ // `pull_request_review` event => event.pull_request.number (same shape)
+ // `merge_group` event => no PR number, returns null
+ return event.pull_request?.number ?? event.number ?? null
+ } catch (error) {
+ logMessage(`Failed to read GITHUB_EVENT_PATH (${eventPath}): ${error.message}`)
+ return null
}
}
+async function defaultReadEvent(eventPath) {
+ const raw = await fs.readFile(eventPath, 'utf-8')
+ return JSON.parse(raw)
+}
+
async function defaultRunGit(args) {
const {stdout} = await execa('git', args)
return stdout
}
+/**
+ * Default GitHub compare-API fetcher. Uses the standard `GITHUB_TOKEN` so
+ * it works on first-party PRs; falls back to unauthenticated for forks
+ * (still works for public-repo compare). On any failure we return null and
+ * the caller falls back to scanning everything — i.e. degrades to legacy
+ * behavior, never silently ignores potential breaking changes.
+ */
+async function defaultFetchCompare(repo, base, head) {
+ if (!base || !head) return null
+ const url = `https://api.github.com/repos/${repo.owner}/${repo.name}/compare/${base}...${head}`
+ return githubGet(url)
+}
+
+async function githubGet(url) {
+ const headers = {Accept: 'application/vnd.github+json'}
+ const token = process.env.GITHUB_TOKEN
+ if (token) headers.Authorization = `Bearer ${token}`
+ try {
+ const res = await fetch(url, {headers})
+ if (!res.ok) {
+ logMessage(`GitHub API ${url} returned ${res.status}`)
+ return null
+ }
+ return await res.json()
+ } catch (error) {
+ logMessage(`GitHub API ${url} failed: ${error.message}`)
+ return null
+ }
+}
+
+// ---------------------------------------------------------------------------
+// Code-owner approval override
+// ---------------------------------------------------------------------------
+
+/**
+ * Parses a CODEOWNERS file body and returns an array of
+ * `{pattern, owners: ["@org/team", "@user", ...]}`.
+ *
+ * Comments and blank lines are skipped. Order is preserved (the last
+ * matching rule wins, per CODEOWNERS semantics).
+ */
+export function parseCodeowners(content) {
+ const rules = []
+ for (const rawLine of content.split(/\r?\n/)) {
+ const line = rawLine.replace(/#.*$/, '').trim()
+ if (!line) continue
+ const tokens = line.split(/\s+/)
+ if (tokens.length < 2) continue
+ const [pattern, ...owners] = tokens
+ rules.push({pattern, owners})
+ }
+ return rules
+}
+
+/**
+ * Convert a CODEOWNERS pattern into a RegExp.
+ *
+ * This is a deliberately conservative translation: it covers the patterns
+ * actually used in this repo (leading `*`, leading `/`, directory globs)
+ * but does not aim for full gitignore compatibility. CODEOWNERS
+ * mis-matches here only affect whether the override is *granted*; the
+ * detection itself is unchanged, so a too-narrow regex just falls back to
+ * "no override" rather than silently approving the wrong thing.
+ */
+export function codeownersPatternToRegExp(pattern) {
+ let p = pattern
+ // A pattern that doesn't start with `/` matches anywhere in the tree.
+ const anchored = p.startsWith('/')
+ if (anchored) p = p.slice(1)
+ // A trailing `/` means "this directory and everything inside it".
+ if (p.endsWith('/')) p = `${p}**`
+ // `**` => any path segments, `*` => anything except `/`.
+ let regex = ''
+ let i = 0
+ while (i < p.length) {
+ const ch = p[i]
+ if (ch === '*' && p[i + 1] === '*') {
+ regex += '.*'
+ i += 2
+ if (p[i] === '/') i++ // consume the `/` after `**`
+ } else if (ch === '*') {
+ regex += '[^/]*'
+ i++
+ } else if ('.+?^$()[]{}|\\'.includes(ch)) {
+ regex += `\\${ch}`
+ i++
+ } else {
+ regex += ch
+ i++
+ }
+ }
+ return new RegExp(anchored ? `^${regex}$` : `(^|/)${regex}$`)
+}
+
+/**
+ * Returns the set of CODEOWNERS owner handles (teams and users, with the
+ * leading `@`) responsible for any of the given changed files. Per
+ * CODEOWNERS semantics the *last* matching rule wins.
+ */
+export function ownersForFiles(rules, files) {
+ const compiled = rules.map((r) => ({...r, regex: codeownersPatternToRegExp(r.pattern)}))
+ const owners = new Set()
+ for (const file of files) {
+ let match = null
+ for (const rule of compiled) {
+ if (rule.regex.test(file)) match = rule
+ }
+ if (match) match.owners.forEach((o) => owners.add(o))
+ }
+ return owners
+}
+
+/**
+ * Checks whether the PR has been approved by anyone who can plausibly
+ * count as a code-owner approval. Returns `{approved: bool, approver: string | null}`.
+ *
+ * "Plausibly" intentionally means "a member of the org with write access
+ * to this repo". The default GITHUB_TOKEN can't list arbitrary team
+ * memberships across the org, but it can read repo-level permissions, and
+ * for Shopify/cli the CODEOWNERS file is `* @shopify/dev_experience`
+ * top-level, with team-only overrides for a couple of paths. Any human
+ * with write access on this repo is in one of those teams (the team grant
+ * is what gives them write access). This is the same security posture as
+ * branch protection's "require code-owner review" rule, just evaluated
+ * earlier so the check can flip green without manually re-running CI.
+ */
+export async function findCodeownerApproval({
+ repo,
+ prNumber,
+ changedFiles,
+ fetchReviews = defaultFetchReviews,
+ fetchCodeowners = defaultFetchCodeowners,
+ fetchPermission = defaultFetchPermission,
+} = {}) {
+ if (!prNumber) return {approved: false, approver: null, reason: 'no PR number in event'}
+
+ const reviews = await fetchReviews(repo, prNumber)
+ if (!reviews) return {approved: false, approver: null, reason: 'reviews API failed'}
+
+ // Last *actionable* review per author wins (matches GitHub's own
+ // branch-protection semantics). GitHub returns a `COMMENTED` entry for
+ // every inline review comment, so without this filter a reviewer who
+ // approves and then leaves a line comment would have their `APPROVED`
+ // silently overwritten by the later `COMMENTED` entry.
+ const ACTIONABLE_REVIEW_STATES = new Set(['APPROVED', 'CHANGES_REQUESTED', 'DISMISSED'])
+ const latestByUser = new Map()
+ for (const review of reviews) {
+ if (!review.user?.login) continue
+ if (!ACTIONABLE_REVIEW_STATES.has(review.state)) continue
+ latestByUser.set(review.user.login, review)
+ }
+ const approvers = [...latestByUser.values()].filter((r) => r.state === 'APPROVED').map((r) => r.user.login)
+ if (approvers.length === 0) return {approved: false, approver: null, reason: 'no approving reviews'}
+
+ // Optional refinement: if we have the CODEOWNERS file and the changed
+ // files, only count approvers whose teams own a path in the diff. We
+ // log it for transparency but don't gate on it — see the function
+ // doc-comment for why.
+ const codeowners = await fetchCodeowners(repo)
+ if (codeowners && changedFiles && changedFiles.size > 0) {
+ const owners = ownersForFiles(parseCodeowners(codeowners), [...changedFiles])
+ if (owners.size > 0) logMessage(`Code owners for changed files: ${[...owners].join(', ')}`)
+ }
+
+ for (const approver of approvers) {
+ const permission = await fetchPermission(repo, approver)
+ if (permission === 'admin' || permission === 'write' || permission === 'maintain') {
+ return {approved: true, approver, reason: `${approver} has ${permission} access`}
+ }
+ }
+ return {approved: false, approver: null, reason: 'no approving reviewer has write access'}
+}
+
+async function defaultFetchReviews(repo, prNumber) {
+ return githubGet(`https://api.github.com/repos/${repo.owner}/${repo.name}/pulls/${prNumber}/reviews?per_page=100`)
+}
+
+async function defaultFetchCodeowners(repo) {
+ // Try the canonical locations (root, .github, docs).
+ for (const candidate of ['.github/CODEOWNERS', 'CODEOWNERS', 'docs/CODEOWNERS']) {
+ const data = await githubGet(
+ `https://api.github.com/repos/${repo.owner}/${repo.name}/contents/${candidate}`,
+ )
+ if (data?.content) return Buffer.from(data.content, 'base64').toString('utf-8')
+ }
+ return null
+}
+
+async function defaultFetchPermission(repo, username) {
+ const data = await githubGet(
+ `https://api.github.com/repos/${repo.owner}/${repo.name}/collaborators/${username}/permission`,
+ )
+ return data?.permission || null
+}
+
// ---------------------------------------------------------------------------
// 1. Check changesets for major bumps
// ---------------------------------------------------------------------------
@@ -571,7 +808,7 @@ function buildReport({majorChangesets, manifestChanges, schemaChanges}) {
This PR contains changes that may break the existing contract.
-**@shopify/dev_experience** — this PR contains breaking changes that require coordination for the next major release.
+**@shopify/dev_experience** — this PR contains breaking changes that require coordination for the next major release. To unblock this check, an approving review from a code owner of the changed files (typically a \`@shopify/dev_experience\` member) is sufficient — the check will re-run automatically when the review is submitted and turn green.
`
@@ -663,6 +900,11 @@ async function runMain() {
if (context.changedFiles) {
logMessage(`PR touched ${context.changedFiles.size} file(s); scoping schema/manifest scan to those`)
}
+ if (context.repo && context.prNumber) {
+ logMessage(`PR context: ${context.repo.owner}/${context.repo.name}#${context.prNumber} (override eligible)`)
+ } else {
+ logMessage(`PR context unavailable (repo=${Boolean(context.repo)}, prNumber=${context.prNumber ?? 'null'}); override check will be skipped`)
+ }
// This script consumes only git-tracked files (oclif.manifest.json + .ts
// sources). It does not need the baseline's node_modules or dist output,
@@ -680,14 +922,32 @@ async function runMain() {
const report = buildReport({majorChangesets, manifestChanges, schemaChanges})
- if (report) {
- logSection('\n⚠️ Breaking changes detected!')
- setOutput('has_breaking_changes', 'true')
- setOutput('report', report)
- } else {
+ if (!report) {
logSection('\n✅ No breaking changes detected')
setOutput('has_breaking_changes', 'false')
+ return
}
+
+ // Breaking changes were detected. If a code-owner has already approved
+ // the PR, treat that as sign-off and turn the check green. The script
+ // re-runs on `pull_request_review` events (see breaking-change-check.yml)
+ // so a fresh approval will flip this check without manual CI re-runs.
+ const override = await findCodeownerApproval({
+ repo: context.repo,
+ prNumber: context.prNumber,
+ changedFiles: context.changedFiles,
+ })
+ if (override.approved) {
+ logSection(`\n✅ Breaking changes detected, but overridden by ${override.approver}'s approval`)
+ setOutput('has_breaking_changes', 'false')
+ setOutput('report', `${report}\n---\n✅ Override: approved by @${override.approver}.\n`)
+ return
+ }
+
+ logSection('\n⚠️ Breaking changes detected!')
+ if (override.reason) logMessage(`Override not granted: ${override.reason}`)
+ setOutput('has_breaking_changes', 'true')
+ setOutput('report', report)
} 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 0af5762bd1..01ae378e42 100644
--- a/workspace/src/major-change-check.test.js
+++ b/workspace/src/major-change-check.test.js
@@ -17,7 +17,16 @@ 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'
+import {
+ checkChangesets,
+ codeownersPatternToRegExp,
+ extractSchemaFields,
+ findCodeownerApproval,
+ ownersForFiles,
+ parseCodeowners,
+ resolveContext,
+ stripStringsAndComments,
+} from './major-change-check.js'
test('extracts top-level keys from a flat .object({...})', () => {
const src = `
@@ -189,6 +198,87 @@ 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')
})
+test('resolveContext: populates repo + prNumber from GITHUB_REPOSITORY and the event payload', async () => {
+ // Integration-shaped test: this is exactly the path that ships in CI.
+ // Without this, `runMain` calls findCodeownerApproval with undefined
+ // repo/prNumber and the override silently no-ops.
+ const runGit = async (args) => (args[0] === 'merge-base' ? 'abc123\n' : '')
+ const ctx = await resolveContext({
+ baseRef: 'main',
+ repository: 'Shopify/cli',
+ eventPath: '/fake/path/event.json',
+ readEvent: async () => ({pull_request: {number: 7469}}),
+ runGit,
+ })
+ assert.deepEqual(ctx.repo, {owner: 'Shopify', name: 'cli'})
+ assert.equal(ctx.prNumber, 7469)
+})
+
+test('resolveContext: pull_request_review event also yields a PR number', async () => {
+ // The whole reason this workflow exists is to re-run on review events,
+ // so this shape must work.
+ const ctx = await resolveContext({
+ baseRef: 'main',
+ repository: 'Shopify/cli',
+ eventPath: '/fake/path/event.json',
+ readEvent: async () => ({
+ action: 'submitted',
+ pull_request: {number: 7469},
+ review: {state: 'approved', user: {login: 'isaac'}},
+ }),
+ runGit: async () => '',
+ })
+ assert.equal(ctx.prNumber, 7469)
+})
+
+test('resolveContext: merge_group event has no PR number, repo still resolves', async () => {
+ const ctx = await resolveContext({
+ baseRef: 'main',
+ repository: 'Shopify/cli',
+ eventPath: '/fake/path/event.json',
+ readEvent: async () => ({merge_group: {head_sha: 'deadbeef'}}),
+ runGit: async () => '',
+ })
+ assert.deepEqual(ctx.repo, {owner: 'Shopify', name: 'cli'})
+ assert.equal(ctx.prNumber, null, 'merge_group has no PR number; override is skipped cleanly')
+})
+
+test('resolveContext: missing env vars degrade to null (local invocation)', async () => {
+ const ctx = await resolveContext({
+ baseRef: undefined,
+ repository: undefined,
+ eventPath: undefined,
+ runGit: async () => '',
+ })
+ assert.equal(ctx.repo, null)
+ assert.equal(ctx.prNumber, null)
+ assert.equal(ctx.baselineRef, 'main')
+ assert.equal(ctx.changedFiles, null)
+})
+
+test('resolveContext: malformed GITHUB_REPOSITORY returns null repo', async () => {
+ const ctx = await resolveContext({
+ baseRef: undefined,
+ repository: 'not-a-valid-slug',
+ runGit: async () => '',
+ })
+ assert.equal(ctx.repo, null, 'a repository slug without a slash must not produce undefined-named URLs')
+})
+
+test('resolveContext: unreadable event file degrades to null prNumber (does not throw)', async () => {
+ const ctx = await resolveContext({
+ baseRef: undefined,
+ repository: 'Shopify/cli',
+ eventPath: '/fake/path/event.json',
+ readEvent: async () => {
+ throw new Error('ENOENT: no such file or directory')
+ },
+ runGit: async () => '',
+ })
+ assert.deepEqual(ctx.repo, {owner: 'Shopify', name: 'cli'})
+ assert.equal(ctx.prNumber, null)
+})
+
// ---------------------------------------------------------------------------
// checkChangesets() — only flag changesets the PR actually touched
// ---------------------------------------------------------------------------
@@ -270,3 +360,138 @@ test('resolveContext: no GITHUB_BASE_REF falls back to scanning main (local invo
assert.equal(ctx.changedFiles, null)
assert.equal(called, false, 'must not shell out to git when no base ref is known')
})
+
+// ---------------------------------------------------------------------------
+// CODEOWNERS parsing & matching
+// ---------------------------------------------------------------------------
+
+test('parseCodeowners strips comments and blank lines', () => {
+ const content = `
+# top of file
+* @shopify/dev_experience
+
+# section
+/.github/CODEOWNERS @shopify/developer-platforms @shopify/dev_experience
+packages/theme/** @shopify/theme # inline comment
+`
+ const rules = parseCodeowners(content)
+ assert.deepEqual(rules, [
+ {pattern: '*', owners: ['@shopify/dev_experience']},
+ {pattern: '/.github/CODEOWNERS', owners: ['@shopify/developer-platforms', '@shopify/dev_experience']},
+ {pattern: 'packages/theme/**', owners: ['@shopify/theme']},
+ ])
+})
+
+test('codeownersPatternToRegExp matches like CODEOWNERS does', () => {
+ // `*` covers everything
+ assert.match('packages/app/foo.ts', codeownersPatternToRegExp('*'))
+
+ // Anchored path
+ const cliRule = codeownersPatternToRegExp('/.github/CODEOWNERS')
+ assert.match('.github/CODEOWNERS', cliRule)
+ assert.doesNotMatch('foo/.github/CODEOWNERS', cliRule)
+
+ // Directory glob
+ const themeRule = codeownersPatternToRegExp('packages/theme/**')
+ assert.match('packages/theme/index.ts', themeRule)
+ assert.match('packages/theme/sub/dir/file.ts', themeRule)
+ assert.doesNotMatch('packages/app/foo.ts', themeRule)
+})
+
+test('ownersForFiles: last matching rule wins (CODEOWNERS semantics)', () => {
+ const rules = parseCodeowners(`
+* @shopify/dev_experience
+packages/theme/** @shopify/theme
+`)
+ const themeOwners = ownersForFiles(rules, ['packages/theme/foo.ts'])
+ assert.deepEqual([...themeOwners], ['@shopify/theme'], 'theme rule overrides catch-all')
+
+ const appOwners = ownersForFiles(rules, ['packages/app/foo.ts'])
+ assert.deepEqual([...appOwners], ['@shopify/dev_experience'], 'falls through to catch-all')
+})
+
+// ---------------------------------------------------------------------------
+// findCodeownerApproval()
+// ---------------------------------------------------------------------------
+
+const fakeRepo = {owner: 'Shopify', name: 'cli'}
+
+test('findCodeownerApproval: approver with write access overrides', async () => {
+ const result = await findCodeownerApproval({
+ repo: fakeRepo,
+ prNumber: 1,
+ changedFiles: new Set(['packages/app/foo.ts']),
+ fetchReviews: async () => [
+ {state: 'COMMENTED', user: {login: 'someone'}},
+ {state: 'APPROVED', user: {login: 'isaac'}},
+ ],
+ fetchCodeowners: async () => '* @shopify/dev_experience',
+ fetchPermission: async (_repo, user) => (user === 'isaac' ? 'write' : 'read'),
+ })
+ assert.equal(result.approved, true)
+ assert.equal(result.approver, 'isaac')
+})
+
+test('findCodeownerApproval: latest review per author wins (changes_requested cancels earlier approval)', async () => {
+ const result = await findCodeownerApproval({
+ repo: fakeRepo,
+ prNumber: 2,
+ changedFiles: new Set(['packages/app/foo.ts']),
+ fetchReviews: async () => [
+ {state: 'APPROVED', user: {login: 'isaac'}, submitted_at: '2025-01-01'},
+ {state: 'CHANGES_REQUESTED', user: {login: 'isaac'}, submitted_at: '2025-01-02'},
+ ],
+ fetchCodeowners: async () => '* @shopify/dev_experience',
+ fetchPermission: async () => 'write',
+ })
+ assert.equal(result.approved, false, "withdrawn approval should not count")
+})
+
+test('findCodeownerApproval: later COMMENTED review does not overwrite an earlier APPROVED', async () => {
+ // Common workflow: approve a PR, then leave an inline review comment.
+ // GitHub records the comment as a `COMMENTED` review entry. We must
+ // ignore non-actionable states when computing "latest review per author",
+ // otherwise the approval is silently lost.
+ const result = await findCodeownerApproval({
+ repo: fakeRepo,
+ prNumber: 5,
+ changedFiles: new Set(['packages/app/foo.ts']),
+ fetchReviews: async () => [
+ {state: 'APPROVED', user: {login: 'isaac'}, submitted_at: '2025-01-01'},
+ {state: 'COMMENTED', user: {login: 'isaac'}, submitted_at: '2025-01-02'},
+ ],
+ fetchCodeowners: async () => '* @shopify/dev_experience',
+ fetchPermission: async () => 'write',
+ })
+ assert.equal(result.approved, true, 'a later COMMENTED entry must not cancel an APPROVED review')
+ assert.equal(result.approver, 'isaac')
+})
+
+test('findCodeownerApproval: no approving reviewer with write access => not approved', async () => {
+ const result = await findCodeownerApproval({
+ repo: fakeRepo,
+ prNumber: 3,
+ changedFiles: new Set(['packages/app/foo.ts']),
+ fetchReviews: async () => [{state: 'APPROVED', user: {login: 'external'}}],
+ fetchCodeowners: async () => '* @shopify/dev_experience',
+ fetchPermission: async () => 'read',
+ })
+ assert.equal(result.approved, false)
+ assert.equal(result.approver, null)
+})
+
+test('findCodeownerApproval: missing PR number bails immediately', async () => {
+ const result = await findCodeownerApproval({repo: fakeRepo, prNumber: null})
+ assert.equal(result.approved, false)
+ assert.match(result.reason, /no PR number/)
+})
+
+test('findCodeownerApproval: reviews API failure bails (does not auto-approve)', async () => {
+ const result = await findCodeownerApproval({
+ repo: fakeRepo,
+ prNumber: 4,
+ changedFiles: new Set(['x']),
+ fetchReviews: async () => null,
+ })
+ assert.equal(result.approved, false, 'API failure must NOT silently grant override')
+})