ci: make Chromatic visual check a real approval gate (and unblock non-UI/fork PRs)#18809
Draft
pettinarip wants to merge 1 commit into
Draft
ci: make Chromatic visual check a real approval gate (and unblock non-UI/fork PRs)#18809pettinarip wants to merge 1 commit into
pettinarip wants to merge 1 commit into
Conversation
Set exitZeroOnChanges: false on the required visual-tests job so PRs with unaccepted visual changes fail the check until a maintainer accepts them in Chromatic and re-runs the job. Add a pull_request_target reporter that posts the required "Visual regression (Chromatic)" status as success for dev PRs that touch no UI code. This unblocks content-only PRs -- including fork PRs whose CI workflow is held for manual approval and therefore never reports the check (e.g. #18806).
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.

Problem
PR #18806 (a content-only change from an external fork) is stuck on the required check "Visual regression (Chromatic)", which never reports. Root cause is two-fold:
visual-testsjob (whose name is the required context) only runs for dev PRs that touch UI paths. Content-only PRs skip it. For internal PRs a skipped job reportsskipped= success, so they merge fine.action_required(they carry secrets). Until a maintainer approves the run, the job never executes, so the required context is never posted — the PR sits in "Expected — waiting for status to be reported" forever.Separately, the check enforces nothing today:
exitZeroOnChanges: truemakes it go green even when there are unaccepted visual changes.Changes
1.
ci.yml— make the check a real approval gate.exitZeroOnChanges: true→falseon the requiredvisual-testsjob. Now a PR with unaccepted visual changes fails the check and stays blocked until a maintainer accepts the diffs in Chromatic and re-runs the job. (autoAcceptChanges: {dev,staging,master}still auto-accepts release-train head branches, so only feature PRs require manual acceptance.)2. New
chromatic-required-status.yml— unblock non-UI PRs, including forks.A
pull_request_targetreporter that posts the requiredVisual regression (Chromatic)status as success for dev PRs that touch no UI code. It runs in base-repo context (no approval needed) and only reads the PR's changed-file list — it never checks out or executes fork code, so there's no secret-exfiltration surface. Its path filter mirrors thechromatic-gatefilter inci.yml.Behavior after this PR
Notes / limitations
branches: [dev]because that's the only base wherevisual-testsis skipped for content PRs; master/staging/test always run the real job.page-visual-tests(not a required check).Immediate unblock for #18806
Independent of this PR, a maintainer can unblock #18806 now by clicking "Approve and run workflows" (the skipped job then reports success), or by admin-merging (
enforce_adminsis off).