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.

[natethelen]

Agreed. I can't believe this isn't the first thing that was implemented around requiring github actions to pass in branch protections. There 100% should be a checkbox that allows you to require any triggered workflow to pass.

[connorstorer-kbxcom]

THANK YOU ALL FOR VALIDATING MY THOUGHTS! I've been scratching my head on why this isn't possible - just came across this. Frustrating - but not surprising.

[Scotchester]

See also the most active discussion in ye olde GitHub Community Forum on this topic: https://github.521000.bestmunity/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.521000.bestmunity/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.

[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

[connorstorer-kbxcom]

I think the Require triggered checks to pass is a great idea. I also think that this should just be the default functionality when setting Require status checks to pass. Right now it requires that you define at least 1 status check :( . And then also allowing the explicitly set status checks to be [required, optional, disabled, etc.]

[mbarr-hrp]

We've used https://github.com/dorny/paths-filter for exactly this. Much happier.

[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/[email protected]
        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]

@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]

[connorstorer-kbxcom]

@patrick-knight any update on the internal item that may address this? We're working on migrating 200+ repositories from ADO to GitHub but this is a blocker for us. Any response would be appreciated!

[EmilMunksoe]

Keeping this thread alive since this is really a big issue.

Not to compare DevOps platforms, but to give some insights into what some might want in this feature. GitLab has the feature "Pipeline must succeed" - https://docs.gitlab.com/ee/user/project/merge_requests/auto_merge.html#require-a-successful-pipeline-for-merge which basically makes it so that if no pipelines are run, then if this setting is enabled the check will fail and block the PR. If a pipeline exists (in GitLab you can only have a single), then that pipeline must succeed for the latest commit in order to meet the criteria of merge.

Above translated into GitHub:
I think it would be awesome to support a setting called "Require all available checks to be succeeded in order to merge", which basically just validates if a single or none success checks are on a Pull Request, then if this setting is enabled it will block the availability to merge until a single check succeeds or all checks are succeeds.

[antoniogamizbadger]

That would be amaziiiiing.

[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 :(

[mbarr-hrp]

We've used https://github.com/dorny/paths-filter for exactly this. Much happier.

[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

[timos-flwls]

I think there's still a big issue with this script, if commit 1 triggers X and Y workflows, both of which fail, and then commit 2 triggers Z workflow, this status check will pass.

[wechuli]

@timos-flwls , that's an interesting scenario. I am writing a GitHub Actions for the workaround and that is a scenario I had not thought about. So, in your case, would you want to walk through all the commits in the specific pull request from latest to earliest and confirm that the check passed? I am curious on how you would end up with commit 1 triggering X and Y and the same workflows are not triggered on commit 2 given how GitHub determines the diff comparison for the paths filters:

https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#git-diff-comparisons

GitHub generates the list of changed files using two-dot diffs for pushes and three-dot diffs for pull requests:

• Pull requests: Three-dot diffs are a comparison between the most recent version of the topic branch and the commit where the topic branch was last synced with the base branch.

In your, case, are you considering a scenario where the base branch is merged to the pull request branch (topic branch)?

[danieltroger]

I think there's still a big issue with this script, if commit 1 triggers X and Y workflows, both of which fail, and then commit 2 triggers Z workflow, this status check will pass.

Ah that's interesting and might explain once when a MR got merged before all workflows had passed. So you're saying that the second commit doesn't trigger the "check_commit_status"-workflow again? Why not?

[wechuli]

I've created a GitHub Action that wraps the logic of what @danieltroger described in a nicer package. Check it out at https://github.com/wechuli/allcheckspassed. It provides some additional options such as excluding certain checks from the evaluation and choosing what exactly constitutes a passing check (e.g. skipped, neutral). The action also creates a summary of the checks that were evaluated. Additional documentation is on the README.

Ultimately this is only a workaround for a missing feature, the main downside it has is that you need to configure it on its own workflow and it will consume your Actions minutes, I have tried to document this under limitations section. Let me know what you think, if you want some features it doesn't have, please open an issue (or a pr 😉 ).

[mccrearyp]

This is great, thanks for making this!

[stevoland]

Just a note that https://github.com/palantir/policy-bot is a great option until github support this. Only real downside is the duplication of conditions between the policies and the conditional jobs imo.

[momirjalili]

the way I solved this issue is by adding a job like the following to all the workflows.

check-for-successful-tests:
    needs: [test-job-1, test-job-2, test-job-3]
    if: ${{ success() || failure() }}
    steps:
      - name: Check outcomes
        if: ${{ needs.test-job-1.result == 'failure' || needs.test-job-2.result == 'failure' || needs.test-job-3.result == 'failure' }}
        run: exit 1

and a dummy workflow that with the same job name that runs exit 0. and marked check-for-successful-tests as required. this way i'm sure that the job runs 100 percent of the time and also if a required check is failing it blocks the merge.

[mbarr-hrp]

We've actually used https://github.com/dorny/paths-filter to create conditional actions, based on paths, that still succeed in the end.

Workflow runs, it decides if it needs to do the work, and then passes/fails based on the work.

If it doesn't need to do the work, it skips the conditional job, and passes.

[hankhsu1996]

I tried the path filter action approach, and it’s been incredibly frustrating. The configuration goes like this:

1. Don’t set any path filter in the trigger condition.
2. Add a job to handle path filtering using an action like this one: https://github.com/tj-actions/changed-files.
3. Use the output of the path filter in the actual job by specifying the if condition to force the job to pass.

Initially, this seemed like a reasonable solution, but it completely breaks down when you have even a slightly complex setup with job dependencies.

For example, my CI has a build phase and a test phase, where the test phase has a matrix of runtime arguments. All these test jobs with different matrix values are marked as "required checks to pass."

The issue arises when the file changes aren’t related to this CI. In such cases, I need to skip the build and test phases. If I use if in the build phase based on the path filter output, the build is skipped, but the test jobs don’t trigger. If I use if in the test jobs, then none of the matrix entries are triggered. Either way, the result is the same: the PR merge is blocked indefinitely because the required jobs are never run.

To hack around this, you can’t rely on path filtering in the workflow or job scope. Instead, you have to move the path filtering logic into steps. You check the output of the path filter job as a global variable, and in every job, the first step checks this variable. If the job needs to be skipped, you force a pass in the step itself, not in the job’s if condition. This ensures that all jobs, including matrix jobs, are triggered and avoid blocking the workflow.

This approach is far more complicated than it should be. Something as basic as skipping jobs based on file changes shouldn’t require this level of convoluted hacking. It’s frustrating and feels like an overengineered solution for what should be a straightforward problem.

[echeran]

Have you tried https://github.com/poseidon/wait-for-status-checks ? It was mentioned above. It works really well for plugging the hole that Github has not yet fixed. I don't know why it isn't more upvoted and commented on in this thread.

[antoniogamizbadger]

Because you need to spend more minutes of Github Actions. In expensive workflow runs like the ones using MacOS, the cost can add up pretty quickly.

[echeran]

I see. Tradeoffs indeed. I respect that situation & constraints. For my situation, it happens to be that the value of the pros outweigh the cons enough to use that action. (Of course, it would be nice in the future for the 2 different features of Github Actions to natively play nice together so that we wouldn't have to choose among workarounds.)

[hankhsu1996]

@echeran Thank you, I'll try it