Required actions workflows from branch protections should only be required if run

[daltonv]

If you have branch protections set so that a GitHub Action workflow must be run and pass before you can merge, and this workflow has path filtering in the on: clause then you sometimes can't ever merge because the workflow doesn't run.

This GitHub docs talk about this issue here: https://docs.github.com/en/enterprise-cloud@latest/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks#handling-skipped-but-required-checks

There is a workaround suggested in the doc article, but it involves making a "dummy" workflow of the same name of the required workflow. This dummy workflow always passes and is triggered by the inverse path filtering of the required workflow.

This isn't the best solution in my mind because:

• devs now see two workflows with the same name in the Actions tab of a repo. That's confusing and too much clutter
• The "dummy" workflow runs along with the required workflow sometimes. For example if the required workflow only runs when *.c files are changed, the "dummy" workflow and the required workflow run if BOTH *.c files and non *.c files are changed.

Instead it should be changed so local workflows are only required if they run in the first place.

[ethomson]

Thanks @daltonv. I agree that this is not the ideal solution. I'll talk with the team that owns branch protection to give them your feedback. I'll also talk to the docs team to suggest some improvements for this article in the meantime.

Thanks again for taking the time to leave feedback.

[gustavovalverde]

Hi @ethomson, while looking for more info I just realized I duplicated this discussion. But I'm sharing mine to add more context on other scenarios which make the actual workaround very messy https://github.com/orgs/community/discussions/44490#discussion-4761945

I'm specifically bringing this link out, just so you (and others) can realize how much burden the recommended workaround from the documentation adds to the pipelines: https://github.com/ZcashFoundation/zebra/tree/main/.github/workflows

[lhriley]

This should be a generalized change as it affects users who don't use GitHub Actions, but instead have third party CI/CD integrations triggered via webhooks.

The option should simply be Require triggered checks to pass rather than specifying a list of checks as being required.

[kousu]

This is the behaviour I expect too! And having to explicitly manage the checks to pass in the GUI is redundant and bug-prone. I want to manage everything by code.

[joshuat]

Just chiming in and it's kind of shocking there's no way to do this - if a task runs I expect it to either pass or at least have the option of blocking merges without having to itemize every task.

Having to itemize them creates a security gap and as mentioned above is problematic when dealing with conditional tasks.

[p0fi]

This is the only sane behavior any human could think of. How come it's implemented otherwise? Who would want to specify those checks ahead of time in the repo settings? Why? Just why? This makes this feature utterly useless I believe.

[Scotchester]

See also the most active discussion in ye olde GitHub Community Forum on this topic: https://github.community/t/feature-request-conditional-required-checks/16761

[steinybot]

Link seems to be broken

[antoniogamizbadger]

https://github.com/orgs/community/discussions/44490

[antoniogamizbadger]

I think is either the same one or one equivalent.

[solarmosaic-kflorence]

See also https://github.community/t/feature-request-required-checks-conditioned-on-path/2042

[andrelaszlo]

Where did this page go? I've found it in a lot of places, but now it just redirects to the GitHub Community landing page.

[betaorbust]

Ran into it as well. Having something like this would also let us implement more straightforward skipping. For instance, I don't need to to spend the time running all our test, visual regressions, etc. on a PR from an internal bot that edits changelogs, but I absolutely want those to be required checks for us humans so we can use really helpful things like auto-merging, etc.

[joegaudet]

Very much want this, we spend piles of cash on GitHub because of this shortcoming.

[DGuhr]

+1. We have the same problem over at Keycloak: Multiple workflows / jobs are running for e.g. a simple typo in our documentation, or a bump in npm dependencies. Would be very nice to have better support than the dummy workflow approach.

[kimyu92]

Running into similar problem when using conditional workflow via circle-ci.

It should not be waiting infinitely if the workflow doesn't run at all

[KevinDukelow5]

+1, it's surprising that GitHub and disappointing that GitHub doesn't fully support a code first approach for the definition of requirements. The dummy workaround is hacky to say the least.

[dodomood]

+1
can we plz haz?

[courentin]

+1
The workaround does not work with commit message skipping (ie. [skip ci] in commit message) as the dummy workflow is never run because of the [skip ci].

[OneCyrus]

this is a real pain. any update on this? @ethomson

[joegaudet]

Hard not to see this as a cash grab, we've been asking for it for 3 years now, the only way to safely protect branches in this scenario is to run all checks always.

[annabkr]

Came here to add to the chorus of programmers asking for help with this sad trombone UX.

[andrelaszlo]

I got the "dummy workflow workaround" working but it broke after about a week. I don't know if it was some internal Github change or if my setup is just fragile, but the whole situation is pretty disappointing nonetheless.

[p0fi]

I don't even know what to say. We stumbled upon this issue and we are just plain baffled how anyone could implement the required checks in such weird way. Furthermore the suggested workaround is so far from "not being the ideal solution" like the sun is from earth. This is not only not the ideal solution is this a joke.

[dodomood]

I've implemented checks as simple triggers when a PR is opened, reopened or synchronized:

on:
pull_request:
types: [opened, reopened, synchronize]
branches:
- 'main'
paths:
- 'YOUR_PATH_IF_NEEDED'

Result:

[antoniogamizbadger]

Not sure how that is relevant to the discussion.

[tim-sh]

@daltonv You wrote 'Instead it should be changed…' but it obviously they haven't yet. Perhaps you'd like to un-mark this as 'answered' if possible?

[daltonv]

I'm not quite sure what the policy is regarding what constitutes an answer, but hey if this gets more visibility by GitHub as an unanswered thread then why not.

[thorhj]

I have a workflow that does something when a PR is opened or closed. I have implemented this as a required workflow to apply it to all repositories in the organization. I arbitrarily have to also trigger it on PR 'synchronize' and 'reopened' event types and just skip all the steps.

I agree it would make more sense to have required workflows only block the PR if they actually satisfy the trigger conditions.

[antoniogamizbadger]

How does that solve the problem described in the discussion?

[austinpray-mixpanel]

I did a writeup on how we are working around this at Mixpanel: Enforcing required checks on conditional CI jobs in a GitHub monorepo

Would love to update the blog post with a big banner saying "disregard this post, everything in GitHub works as expected now!" 👍

[tylerlaprade]

This is off-topic, but it's pretty cool that your name is Austin and you live in Austin 😄

[astorrs]

+1

[pndurette]

I hate to be that guy, but: any official update on this?
This is making me want to stop using branch protection required checks altogether, so goodbye auto-merge and other great things that stern from it.

Maybe this made sense in some convoluted archaic GitFlow with 6 long-lived branches that each have their own branch protection rules, but the world is doing much less of that these days.

I looked into the new Repository Rules (in beta as of this writing) thinking GitHub might have been focusing their efforts there, but it seems (so far) that required checks work the same way there.

[pndurette]

Update: back in April 2023, in the Repository Rules Public Beta Feedback, this thread was mentioned and looks like in the works within the upcoming Repository Rules (which will eventually replace the branch protection rules), so that's good to see.

@patrick-knight on Apr 17
We are working on enhancement to rules with workflows so keep an eye for update around that in the future.
https://github.com/orgs/community/discussions/52652#discussioncomment-5641312

[dghubble]

As of today, ruleset's don't handle this any better. They still just wait forever for statuses from jobs that are triggered by workflows with path conditions. Still waiting to see if they're working toward the Require triggered checks to pass idea

[cschmatzler]

+1 for Require triggered checks to pass. That's really the only reasonable option here.
@ethomson did you have a conversation with the team about this?

[p0fi]

I really still can't get my head around how this is being implemented. Where is the Require triggered checks to pass option? 🙈

[thaysom22]

+1 is there any plan in place for this?

[dghubble]

I ended up having to write a GitHub Action to add a status check that polls other GitHub check runs (but not from 3rd parties). Then marking enforce-all-checks as required to get the "require triggered checks" behavior I wanted.

name: summary
on:
  pull_request:
jobs:
  enforce-all-checks:
    runs-on: ubuntu-latest
    permissions:
      checks: read
    steps:
      - name: GitHub Checks
        uses: poseidon/wait-for-status-checks@v0.1.0
        with:
          token: ${{ secrets.GITHUB_TOKEN }}

It works, but writing actions is fundamentally limited compared to native GitHub support. First, we have to poll GitHub which is wasteful. Next, it's possible (though unlikely) to race if no other GitHub Checks start for a whle. And finally, when a triggered check fails, developers naturally re-run it, but that can't reasonably re-trigger a job like enforce-all-checks, so you end up manually re-running. That or go write a GitHub App and handle webhooks.

[OneCyrus]

good idea 👍just tried it in our large monorepo with multiple workflows. unfortunately it looks like it doesn't wait for all workflows to complete.

most likely it's due to the API call which returns only the first 30 checks by default.

opened an issue on the repo: poseidon/wait-for-status-checks#8

[eirnym]

Nice! the perfect solution for me. Easy and gets work done.

[elasticdog]

My approach for this has been a reusable workflow named "ready to merge" that parses the needs context to ensure that all required checks have actually passed (or been skipped) as expected. That eliminates the need to poll, but you do have to explicitly list out the other jobs that you are waiting for, which may or may not be feasible based on how you structure your workflows.

[dghubble]

I think another caveat to this approach is that you can only use needs for jobs that are part of the same workflow. If you have multiple workflows, you'd have to add a job like this to each of them, and list out each job in the respective workflow.

[dghubble]

Oh, and to require "ready to merge" that workflow needs to always be triggered. Otherwise, if the whole workflow only runs for changes to foo, the "ready to merge" job won't run.

And with multiple workflows, you'd need different job names "ready to merge 1", "ready to merge 2".

I think your approach is more geared for having a finalizer job at the end of a workflow

[austinpray-mixpanel]

We ran into issues using a workflow to drive the "ready to merge" functionality. @dghubble pointed out a few that we also ran into but in addition:

• Having the status rollup run as an action means it needs to be manually retriggered when failing jobs are rerun.
• GitHub actions being cancelled or failing triggers a failure email notification. This is desirable for actual CI jobs, but bad for these meta jobs. Multiple users (more than 5) expressed confusion.
  

We ended up creating an internal GitHub App rather than using an action. There's other advantages to having full control of the lifecycle as a real app rather than a gha workflow, speed and being able to control api rate limit usage etc.

[elasticdog]

@dghubble yep, those caveats are mentioned in the link. I did start off originally having multiple workflows, but with the move over to reusable workflows it was easier just to have a single "on pull request" workflow that ties things together with "ready to merge" at the end. The need for a workaround at all isn't ideal though :-(

@austinpray-mixpanel the merge group emails are a bit of a mess across the board, but if you've got things all defined in a single workflow, the reason for failure is a bit more obvious:

...I've not had the need to re-trigger single failed jobs, but are you saying that the dependent jobs are not automatically re-triggered?

[patrick-knight]

👋 First thanks for the feedback and discussions on work arounds. This behavior is a long standing one that exists for reasons (no need to bore y'all with history on things we should fix.)

We've working on the successor to branch protections in Repository Rules and are making strides to have those interoperate with Actions with Required Workflows. This will help us close gaps between "branch protections" and actions, but not this one yet.

That said we do have an internal issue tracking this request.

A question for y'all to help us build internal context, what are the typical path filters y'all are using? Is there a specific path/file type etc? Is this limited to only actions workflows or are there areas where path filtering would be useful?

[austinpray-mixpanel]

+1 for monorepos combined with sparse checkouts

Service specific directories like "Only run marketing site build if you touch the marketing site"

  pull_request:
    paths:
      - '.github/actions/setup-node/**'
      - '.github/workflows/marketing-site.yaml'
      - '.github/workflows/marketing-site-build-and-upload.yaml'
      - 'marketing-site/**'

In our monorepo we have multiple languages floating around so we only run the respective linter if the language is touched

Python

on:
  pull_request:
    paths:
      - 'tox.ini'
      - '**.py'
      - '.github/actions/setup-gcp/**'
      - '.github/workflows/lint-python.yaml'
      - 'google-cloud/container/python-lint/pin'
      - 'tools/flake8'
      - 'tools/black'
      - 'dagger/lib/images.json'

Javascript

  pull_request:
    paths:
      - '.browserslistrc'
      - '.editorconfig'
      - '.eslintignore'
      - '.github/**'
      - '.npmrc'
      - '.prettierignore'
      - '.prettierrc'
      - '*.js'
      - '*.json'
      - 'configurations/devs/**'
      - 'dagger/**/*.json'
      - 'iron/**'
      - 'media/**'
      - 'tests/browser/**'
      - 'tools/webapp_smoke_tests/**'
      - 'webapp/app_api/**/*.ts'
      - 'webpack/**'
      - 'yarn.lock'
      - '!marketing-site/**'

[tylerlaprade]

@patrick-knight, my usecase is even simpler - I want all my workflows to run on every PR. Some of those workflows might have conditional logic in the if: that stops them. Without having to explicitly enumerate all my workflows to GitHub, I want all of the ones that make it past the if: to be required to pass for a PR to be merged.

[paulgb]

I agree with @tylerlaprade. The existing action trigger filters are more than sufficient for my use case, I just want to be able to say "allow merging only if actions have passed, but ignore actions that have been excluded due to filters".

[austinpray-mixpanel]

@paulgb yup that's the exact use case we have. I wrote up how we are working around it by implementing a GitHub app: https://engineering.mixpanel.com/enforcing-required-checks-on-conditional-ci-jobs-in-a-github-monorepo-8d4949694340

[courentin]

agree with @tylerlaprade, I wanted to add that there's not only the if: use case, but also commits with a [skip ci]

[kibertoad]

A question for y'all to help us build internal context, what are the typical path filters y'all are using? Is there a specific path/file type etc? Is this limited to only actions workflows or are there areas where path filtering would be useful?

The most typical use-case is monorepo with frontend/backend/packages, where the only subset of flows that we want to run is what was affected by the PR.

Hence paths are something like services/app/users-ui, services/api/users-backend

[antoniogamizbadger]

@patrick-knight any updates on this? almost half a year later and no news :(

[danieltroger]

We got an AMAZING workaround by Paul from GitHub enterprise support. Unfortunately he hasn't posted it here himself even though he promised to, roughly two weeks ago. He understands it better and wanted to package it nicer, but this is it:

It is a standalone workflow that polls the github API every minute for other workflows running for the same merge request and first completes once none is in progress anymore.

How to:

1. Create .github/workflows/check_commit_status.yml in your repo
2. Add this yaml into it:

name: Confirm that all checks have passed or skipped
on:
  pull_request:
    types: [opened, synchronize, reopened]
    branches:
      - main

jobs:
  check_commit_status:
    runs-on: ubuntu-latest
    steps:
      - name: Confirm Check Runs
        uses: actions/github-script@v7
        with:
          script: |
            console.log("Sleeping 10s to give other runs a chance to start");
            await sleep(10000);
            const { owner, repo } = context.repo;
            const commit_sha = context.payload.pull_request.head.sha; // Get the SHA of the head commit of the PR
            const maxRetries = 15; // Maximum number of retries
            const excludedCheckRunName = 'check_commit_status' // important you don't define name key for the job
            let retryCount = 0;
            
            async function fetchCheckRuns() {
              const response = await github.rest.checks.listForRef({
                owner,
                repo,
                ref: commit_sha,
                per_page: 100
              });
              return response.data.check_runs;
            }
            async function sleep(ms) {
              return new Promise(resolve => setTimeout(resolve, ms));
            }
            let checkRuns = await fetchCheckRuns();
            let inProgressFound = false;
            let failureFound = false;
            do {
              inProgressFound = false;
              failureFound = false;
              for (const checkRun of checkRuns) {
                // exclude this job from the evaluation
                if (checkRun.name === excludedCheckRunName) {
                  console.log(`Skipping excluded check run: ${checkRun.name}`);
                  continue;
                }
                if (checkRun.status === 'in_progress') {
                  inProgressFound = true;
                  console.log(`Check run in progress: ${checkRun.name}`);
                }
                if (checkRun.conclusion === 'failure') {
                  failureFound = true;
                  console.log(`Check run failed: ${checkRun.name}`);
                }
              }
              if (inProgressFound) {
                if (retryCount >= maxRetries) {
                  core.setFailed('Maximum retries reached with check runs still in progress.');
                  break;
                }
                console.log('Waiting for 1 minute before rechecking...');
                await sleep(60000);
                checkRuns = await fetchCheckRuns();
                retryCount++;
              }
            } while (inProgressFound);
            if (failureFound) {
              core.setFailed('There are failed check runs.');
              }

3. Set up a ruleset like this, that enforces above workflow to run before merging (make it manually bypassable as you wish)

Edit: if you want to save github actions minutes by first starting the workaround workflow after some minutes, you can add environment: delayenv to the check_commit_status section above and follow these steps

I am referencing the delayenv environment to delay this job from executing using the wait timer protection rule so that it gives the jobs to be checked time to run and finish. The delay should be around the maximum amount of time you expect your other builds to take. This avoids unnecessary polling and consumption of minutes