[BUGFIX] Don't automerge a closed PR

[dabrady]

When automerge is triggered, it first asks a bunch of questions to determine if it should even bother trying to merge. If those checks pass, it starts polling the Github API to determine if the PR is "mergeable".

After the initial run through of what I'll call the "skip factors", it never looks at them again. This is undesirable and problematic for a number of reasons, but particularly for long-running configurations (i.e. when automerge is configured to wait several minutes or even hours before giving up). In particular:

• If a required PR label is added or removed after the automerge job initially starts, it won't know, and may merge the PR when it shouldn't.
• If the PR is closed after the automerge job initially starts, it won't know, and may merge the PR after it has been closed (a totally valid API action if the head of the branch is "mergeable," just not an available action through the Github UI).

We at ProdPerfect have observed several instances where the automerge action is triggered when a pull request is created, but the PR is not immediately mergeable and so it begins its polling; only to miss the addition of labels that should block the merge, or even worse, to miss the closure of the PR itself. There was one incident where the PR was merged a full 30min after it was closed! (Some of our regression test suites take a long time, and we have very long automerge timeouts configured on those projects.)

This PR addresses the problem by adding the "should automerge be skipped?" check to the "is this PR ready to be automerged?" polling mechanism. We have been leveraging this modification for nearly a month at this point, and this simple fix resolves the observed issues without any noticeable latency overhead.

How to test

Testing this necessarily involves consuming it. While first iterating on this enhancement, I created a test repository equipped with a CI integration that simply sleep'd for 60 seconds, and a branch protection rule which required this CI job to pass before allowing commits to be merged into main. It also had a Github workflow configured to use this fork of the automerge action, with config similar to what our team uses in production:

on:
  pull_request:
    types:
      - labeled
      - unlabeled
      - opened
      - edited
      - reopened
      - synchronize
env:
  # Adding the `needs review` label will block automerging.
  MERGE_LABELS: "!needs review"
  # Automerged PRs squish into a single commit before merging.
  MERGE_METHOD: "squash"
  # The entire PR description will be used as the automerge commit message.
  MERGE_COMMIT_MESSAGE: "pull-request-description"
  # Poll for mergeability up to 25 times, every 3 seconds.
  MERGE_RETRIES: "25"
  MERGE_RETRY_SLEEP: "20000"
jobs:
  automerge:
    runs-on: ubuntu-latest
    steps:
      - name: automerge
        uses: "ProdPerfect/automerge-action@v0.15.0"
        env:
          GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"

(For testing purposes, the retries and "retry sleep" config were modified.)

Test scenario: automerge canceled when needs review label is attached to PR

To confirm that this PR addresses this scenario, first create a commit that points the automerge workflow to the official automerge Github action: pascalgn/automerge-action@v0.14.2

Then open a PR, and wait about 20 seconds for the automerge action to be triggered and start polling the PR for mergeability. Add the needs review label after confirming automerge has started running: you should expect it to have no effect, and that the PR is automerged when the CI build passes.

That's the baseline. Now, point the workflow at this fork of the action, and do the same thing. This time, you should expect to see the automerge check finish successfully and gracefully soon after you add the label, without actually merging your PR. Now, remove the label; the automerge action should trigger again, and this time it should merge your PR as soon as the CI job finishes successfully.

Test scenario: automerge canceled on PR closure

To confirm that this PR addresses this scenario, first create a commit that points the automerge workflow to the official automerge Github action: pascalgn/automerge-action@v0.14.2

Then open a PR, and wait about 20 seconds for the automerge action to be triggered and start polling the PR for mergeability. At this point, close the PR, ensuring you do so before the CI build finishes, but after the automerge job has started. Remain on the PR page: you should observe the PR go from "closed" to "merged" within a few seconds, when CI finishes and the still-running automerge job does its thing.

That's the baseline. Now, point the workflow at this fork of the action, and do the same thing. This time, you should expect to see the automerge check finish successfully and gracefully soon after you close the PR, without actually merging your PR; you can view this in the "Checks" tab of the PR (closing the PR takes away the convenient link on the main PR page).

[dabrady]

Looking for feedback from you @pascalgn 🙂 No rush, we're getting along fine with our fork; just want to make sure this surfaces.

[pascalgn]

Sorry for the delay and thanks for the PR, it's merged now! 🎉