DEV Community

Cover image for Your CI is testing the wrong PR first
Daniel Schroeder
Daniel Schroeder

Posted on Edited on AI-assisted

Your CI is testing the wrong PR first

I maintain a handful of open source projects that deploy real infrastructure in their tests. Not mocks. An actual AWS stack gets created, poked at, torn down. Only one copy of that stack exists, so only one test run can use it at a time.

That constraint sounds manageable until you add Renovate and release-please to the repo. Suddenly there are six open pull requests on a Tuesday morning, all of them want the stack, and GitHub Actions has no opinion about who should go first. It took me an embarrassingly long time to notice how much of my CI minutes were spent testing commits that were never going to be merged in that state.

So I wrote boarding-gate, a GitHub Action that queues workflow runs, lets you order them by PR label, and knows a few things about auto-merge and outdated branches that a plain lock doesn't. This post is about the problem first, because I think most people have it without noticing, and the action second.

What a normal morning looks like

Say the repo has these PRs open, and branch protection requires branches to be up to date with main before merging (which you want, otherwise you merge things that were never tested together):

  • Two feature PRs of your own
  • Three Renovate PRs with auto-merge enabled
  • One release-please PR that bumps the version and changelog

All six get a CI run when they're opened. Fine, that's expected. Now you merge the first feature PR. Here's what happens next:

  1. The push to main triggers a run on main. Same code that just passed on the PR, give or take a merge commit. That's one wasted run.
  2. All five remaining PRs are now behind main. release-please rebases its branch immediately. Renovate rebases its three a few minutes later. Your other feature branch waits for you to click "Update branch". Each rebase is a new commit, so each one is a new run. Five more runs.
  3. The first of those rebased runs that finishes and has auto-merge gets merged. Back to step 1 with four PRs.

For six PRs that works out to something like 6 + 1 + 5 + 1 + 4 + 1 + 3 + 1 + 2 + 1 + 1 = 26 runs to merge six changes. On a normal runner that's just money. On a shared test stack where every run takes 20 minutes and nothing else can run meanwhile, it's the whole afternoon.

And the order is wrong. The release-please PR rebases fastest, so it tends to be first in line after every merge, even though it's the one PR that explicitly must not be merged until everything else is in. It gets tested, passes, sits there, goes stale on the next merge, gets tested again. I've watched a release PR go through CI five times in a day without ever being merged.

"Just use a concurrency group"

That's the first thing everyone says, and it's what I tried first. The GitHub docs even frame concurrency as the answer to shared environments:

concurrency:
  group: integration-tests
  cancel-in-progress: false
Enter fullscreen mode Exit fullscreen mode

It does make runs go one at a time. What it doesn't do is queue. A concurrency group holds exactly one running job and one pending job. When a third run shows up, it cancels the pending one and takes its place. With six PRs, four of them get cancelled before they ever start, and nothing restarts them. You end up re-running cancelled jobs by hand, which is the thing you were trying to automate away.

Set cancel-in-progress: true and it gets worse: the newest run kills whatever is currently using the stack, mid-test, without cleanup.

And even if it did queue properly, it would be first come, first served. The release-please PR that rebased fastest is still first. There's no knob for "these runs matter less".

The same goes for the queue actions I found, like softprops/turnstyle or ben-z/gh-action-mutex. They're proper queues, so nothing gets cancelled, but they're FIFO and they don't know what a pull request is. A run holding the lock finishes, the lock is free, and the next run in line grabs it even if its branch is already behind main and will have to be retested anyway.

None of these are bad tools. They solve "one at a time". My problem was "one at a time, in a sensible order, and don't bother testing things that are about to be stale".

What about merge queue?

If you can use GitHub's merge queue, you probably should. It fixes the stale branch problem at the root: it tests each PR on top of the ones ahead of it and merges it itself, so there's no rebase-and-retest loop at all. Add a concurrency group on the merge_group workflow and you also get one at a time.

Two reasons I couldn't:

  • Merge queue is only available for repositories owned by an organization. All my projects live under my user account.
  • It orders PRs by when they were added to the queue. You can jump the queue by hand, but there's no "release PRs always go last" rule.

So if you're in an org, go read the merge queue docs instead of the rest of this post. If you're not, keep reading.

How boarding-gate works

The name is from airports: everyone waits at the gate, and the gate decides who boards next.

The lock is a git ref. The run that currently has the turn created refs/boarding-gate/<workflow>/<n>. When it's done, the next run takes the turn by creating ref <n+1>. Creating a ref that already exists fails, so two runs can't both win. There's no external service, no database, no lease table. Old refs get cleaned up.

Every run of the workflow starts with a wait job that polls until it's that run's turn. The polling is cheap on purpose: the run that's next in line checks every 30 seconds, everyone further back only every five minutes. With ten runs waiting you're not hammering the API.

The order of the waiting runs is what makes this different from a mutex:

  • Each PR label can carry a weight. A run's weight is the sum of its labels. Higher goes first, ties go by start time.
  • With renovate=-5 and autorelease: pending=-10, your own unlabelled PRs (weight 0) are tested first, Renovate updates after that, and the release PR last. Which is exactly when it should run, because by then there's nothing left to merge before it.
  • Labels are re-read while waiting. Slap an urgent label on a PR and its run moves to the front of the queue without re-triggering anything.
  • Runs without a PR, like the push to main after a merge, get their own weight. Set it to -100 and main gets tested only once the PRs are done.

The turn is released when the holder finishes, whether it passed, failed or got cancelled. A re-run of the holder keeps the turn. A run that gets cancelled halfway through the job after the wait frees the turn too, so nobody waits on a dead run.

One more thing a mutex doesn't do: if a PR gets a new commit while its old run is still queued, the old run drops out of the queue and fails as outdated. There's no point testing a commit that's no longer the head of the branch.

Setting it up

Add a wait job and make the jobs that touch the shared resource depend on it:

jobs:
  boarding-gate:
    runs-on: ubuntu-latest
    timeout-minutes: 180
    permissions:
      actions: read
      contents: write
      pull-requests: read
    concurrency:
      group: ${{ github.workflow }}-wait-${{ github.event.pull_request.number || github.ref }}
      cancel-in-progress: true
    steps:
      - uses: udondan/boarding-gate@v1
        with:
          require-up-to-date: true
          label-weights: |
            autorelease: pending=-10
            renovate=-5

  deploy:
    needs: boarding-gate
    if: ${{ !cancelled() }}
    runs-on: ubuntu-latest
    steps:
      - name: Check turn
        if: needs.boarding-gate.result != 'success'
        run: exit 1
      # ... your actual tests against the shared stack
Enter fullscreen mode Exit fullscreen mode

Two bits of that deserve a sentence each, because they look odd.

The concurrency group on the wait job is per pull request, and it's there to kill the waiting run of a commit that just got replaced by a newer push. Without it, the outdated run sits on a runner until its next poll notices. It groups by PR number rather than branch name, because a fork can have a branch with the same name as one of yours. The group is on the wait job only. Once a run has the turn, the wait job is finished, so the jobs that actually use the stack can never be cancelled by a newer push and always get to clean up.

The Check turn step exists because a skipped job counts as a passing required check on GitHub. If the wait fails (timeout, outdated branch), deploy would be skipped, and the PR would show green. So deploy runs anyway and fails itself in its first step.

All runs of the workflow have to go through the wait job. A run that skips it isn't in the queue and will happily use the stack while someone else has the turn.

If you'd rather not copy that block into every repo, there's also a reusable workflow that bundles the same job with sensible defaults (urgent=10 included):

jobs:
  boarding-gate:
    permissions:
      actions: read
      contents: write
      pull-requests: read
      statuses: write
    uses: udondan/boarding-gate/.github/workflows/boarding-gate.yml@v1
    secrets:
      update-token: ${{ secrets.UPDATE_TOKEN }}

  deploy:
    needs: boarding-gate
    # ...
Enter fullscreen mode Exit fullscreen mode

The part that took the most iterations: stale branches and auto-merge

Ordering by label was the easy part. Most of the commits in the repo went into the interaction between the queue, branch protection and auto-merge, because that's where the wasted runs actually come from.

Don't test a branch that's behind. With require-up-to-date: true, a waiting run checks whether its branch is still up to date with main, and fails immediately if it isn't. Not after the test, not after taking the turn. It's going to have to be rebased and retested anyway, so there's no point letting it hold the stack for 20 minutes first.

Rebase it if nobody else will. Some branches never get updated on their own. release-please leaves its PR alone if the merged commit didn't touch the changelog, so its run would fail as outdated forever. Pass an update-token (a PAT or App token, because pushes made with github.token don't trigger workflows) and the action rebases the branch onto main before failing. The new commit starts a fresh run that's actually worth running.

Hold the turn through the merge. This one bit me. A Renovate PR with auto-merge passes CI, and about three seconds later GitHub merges it. In those three seconds the turn is free, the next run in line takes it, checks that its branch is up to date (it is, for now), and starts testing. Then the merge lands, and it's testing a branch that's already behind. So when the holder succeeded and its PR has auto-merge enabled, the turn stays held until the PR is actually merged or closed, or it's clear the merge isn't coming: auto-merge got disabled, a required check failed, the branch fell behind or got a new commit. merge-wait-minutes caps the wait in case a review is missing. Once it's merged, the waiting runs fail as outdated right away instead of after a full test.

Let the right PR go first after a merge. After a merge, PRs get updated at different speeds. release-please rebases within seconds, Renovate takes a few minutes. So for a short while the release PR is the only run in the queue, and it takes the turn even though it has the lowest weight. The action now checks whether there are open auto-merge PRs with a higher weight whose head commit hasn't been tested yet, and yields to them for up to yield-to-pending-minutes. With update-pending-branches: true it goes one step further and rebases those higher-weight PRs itself, so the order by weight holds across merges without waiting for Renovate's schedule.

With all of that on, the morning from earlier looks different. One run per PR per merge of something ahead of it, in weight order, main tested last, and the release PR tested exactly once, right before it's the only thing left.

Seeing where you are in the queue

Composite actions don't stream their logs, you only see them once the step is done. Which is annoying for a step whose whole job is to wait. So the action can set a commit status on the PR (status-context: boarding-gate, needs statuses: write) that updates live: Position 2 of 4 (weight -5). Turn held by #123. It turns green when the run takes the turn and red if the wait failed. You can see at a glance why nothing is happening on your PR without opening a single workflow log.

Pull requests from forks and Dependabot

The first version of this post told you to skip the wait job for these. That's no longer necessary.

A run of a PR from a fork gets a read-only token, so it can't create the lock ref. It now waits in the queue anyway, ordered by its labels like any other run. When it's first in line and the turn is free, a second run takes the turn on its behalf. That run comes from a small workflow_run workflow you add next to the one that waits. workflow_run runs the workflow from your default branch with a writable token, also for runs of forks:

name: Priority boarding for forks

on:
  workflow_run: # zizmor: ignore[dangerous-triggers] no code of the pull request is run
    workflows: [Test]
    types: [in_progress]

permissions: {}

jobs:
  boarding-gate:
    if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.head_repository.full_name != github.repository
    permissions:
      actions: read
      contents: write
      pull-requests: read
      statuses: write
    uses: udondan/boarding-gate/.github/workflows/boarding-gate.yml@v1
    with:
      run-id: ${{ format('{0}', github.event.workflow_run.id) }}
Enter fullscreen mode Exit fullscreen mode

It waits in the place of the fork's run, creates the lock naming that run, sets the commit status the fork's run can't set itself, and ends. The turn belongs to the run named in the lock, not the one that created it, so the fork's run sees its name and carries on with its tests. It never checks out the PR's code, so the writable token isn't exposed to it. If you forget to add the workflow, the fork's run gives up after five minutes instead of holding up everyone behind it.

Forks still get no secrets on pull_request, so only jobs that work without them can run, and their branches are never rebased. On a self-hosted runner that gets reused, a read-only token doesn't make the fork's code safe: it can stay on the machine and read the credentials of later jobs. Run forks on GitHub-hosted or throwaway runners.

Dependabot PRs need nothing extra. Their token is read-only by default, but the permissions of the wait job raise it. They only see Dependabot secrets, so store update-token there as well. And if the token can't write after all, the action fails right away instead of blocking the queue until the timeout.

Workflows on pull_request_target work too. Their token is writable even for forks, so they take the turn themselves and need no workflow_run workflow. The usual warning applies: those runs have your secrets, so the jobs after the wait must not run the PR's code. For forks, pull_request with the workflow above is the safer choice.

Things it can't do

It can't predict a manual merge. If you click the merge button yourself while a run is between "turn is free" and "branch is up to date", that run will test a branch that's about to be stale. Auto-merge is the predictable path.

And it's not a merge queue. If you have one, use it.

Wrapping up

It's a composite action written in bash, Apache-2.0, tested with bats against a fake GitHub API and end-to-end in a real repo with bursts of parallel PRs, relabelling, Renovate auto-merges, pushes to main, cancelled and re-run runs, and PRs from a fork. The repo is at github.com/udondan/boarding-gate and it's on the Marketplace.

If your CI has a resource that only one run can use at a time, and you've got bots opening PRs, I'd be curious whether it fits your setup or where it falls over. Issues welcome.

Top comments (0)