Allow code-owner approval to override breaking-change check#7469
Draft
alfonso-noriega wants to merge 1 commit intoremove-fake-approval-gatefrom
Draft
Allow code-owner approval to override breaking-change check#7469alfonso-noriega wants to merge 1 commit intoremove-fake-approval-gatefrom
alfonso-noriega wants to merge 1 commit intoremove-fake-approval-gatefrom
Conversation
4 tasks
Contributor
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

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_reviewtrigger) 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_reviewevents without re-running the entire test suite (unit tests, e2e, type-diff, etc.) every time someone clicks Approve. Triggers:pull_request— same as beforepull_request_review: types: [submitted, dismissed, edited]— re-runs when an approval is added or withdrawn so the check turns green/red automaticallymerge_group— same as beforeConcurrency is keyed per PR (or per merge-queue head) so the latest review event always wins.
.github/workflows/tests-pr.ymlmajor-change-checkjob.workspace/src/major-change-check.jsWhen breaking changes are detected, the script now:
GET /repos/{owner}/{repo}/pulls/{N}/reviews.CHANGES_REQUESTEDcancels an earlierAPPROVED, exactly likedismiss_stale_reviews).GET /repos/{owner}/{repo}/collaborators/{user}/permission. If the permission isadmin,maintain, orwrite, treat the approval as a code-owner override.has_breaking_changes=false, log the approver, append an "Override: approved by @user" footer to the sticky comment, and exit cleanly.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 defaultGITHUB_TOKENcan't list arbitrary org-team memberships across Shopify, but it can read repo-level collaborator permissions. Anyone withwriteon 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.js10 new tests:
parseCodeowners— comments and blank lines stripped, owners preservedcodeownersPatternToRegExp—*,**, anchored, and unanchored patternsownersForFiles— last-matching-rule-wins (matches CODEOWNERS semantics, including theme overriding the catch-all)findCodeownerApproval:CHANGES_REQUESTEDcancels an earlierAPPROVED, sticky-as-dismiss_stale_reviews)Stickiness behaviour
Per the requirement: matches
dismiss_stale_reviews. Concretely:CHANGES_REQUESTEDorDISMISSED, the override is gone (we always look at latest review per author).git push, the PR's existing approving reviews remain (sincedismiss_stale_reviews: falseonmain's branch protection), so the override survives a force-push. If branch-protection ever flipsdismiss_stale_reviewsto 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.jsAll 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:
Post-release steps
None — the workflow takes effect immediately on first run on
main.Checklist