How to integrate nixpkgs-merge-bot better with GHA CI?

[wolfgangwalther]

In NixOS/nixpkgs#378214, there is discussion around how to better integrate nixpkgs-merge-bot in GitHub Actions CI with the main focus on advertising to maintainers that they can use the bot.

This requires one of two things:

• The ability of nixpkgs-merge-bot itself to go through PRs on its own, without reacting to a comment, or
• The ability of nixpkgs GHA to tell which PRs are eligible for the merge bot.

The first option potentially requires a lot of additional architecture in the merge bot itself.

The second option requires us to re-implement the different merge-strategies in nixpkgs-ci to accurately determine when the bot is eligible.

In NixOS/nixpkgs#378214 (comment), I also had an idea to give the merge bot a CLI interface, so that it could be called from within GHA, essentially asking it to evaluate this one specific PR right on the spot, without firing up a server or so.

Here, I'd like to bring up a different idea: Why don't we re-implement nixpkgs-merge-bot directly in nixpkgs-ci as a GHA workflow?

This could significantly reduce the overall complexity.

There is a downside, though: The issue_comment, pull_request_review and pull_request_review_comment events always trigger without permissions or secrets. This means that CI could not merge directly in reaction to that event.

Instead, we'd have to go through all PRs periodically, look for comments and then act on them. We already do this for labeling, updated PRs will be reacted to within 10 minutes. If we're OK with "I'll post a comment and the first reaction from the bot might be up to 10 minutes later", then this should make things significantly simpler to implement. We could potentially bring this 10 minute interval down to 5 minutes, too, if we thought this was worth it.

Imho, this is only doable, because we now have auto-merge enabled. This allows us to enable auto-merge at the time of going through the comments - and when CI passes, it will be done. Without auto-merge, we would have to keep coming back to a PR that was not recently updated (CI success doesn't count as update). We would have to figure out how to nicely integrate OfBorg into that, but given that it's currently not checked anyway, that shouldn't be a blocker.

One upside I see: We can more easily go towards actually auto-merging stuff via CI. For example these could be enabled for automated updates via a meta/passthru attribute for certain conditions (enough tests, ofborg passing, etc.). I think these things are also ideas discussed in the RFC. Doing this in GHA should be simpler than here, when dealing with all the events etc.

cc @NixOS/nixpkgs-ci

[philiptaron]

Maybe there's a way to do this that keeps the reactivity but still has everything meaningful in GitHub actions checked into the repository.

1. We spin up a small server or a lambda function that has no inherent state. It conforms to the webhook protocol.
2. On request, the webhook then turns around and initiates a workflow in the repository. Since the workflow can have secrets, variables, and so forth, it doesn't fall afoul of these restrictions.

[wolfgangwalther]

We can also get reactivity with the same kind of hack that we use for the review-dismissed.yml / dismissed-review.yml workflows in Nixpkgs. The first one triggers an unprivileged no-op job. The second one triggers a privileged job on the workflow_run event, which then finds the related PR and acts on it. This would work the same way, without any webhook-receiving server involved.

There's one downside to that, though: It will not work for PRs from first-time contributors. Now, that's not really a problem, because we certainly don't want to auto-merge these. But there will be that "some workflows need approval to run" button in their check-list. This already happens now, when a first-time contributor has a PR in which a "requested changes" review is dismissed. So maybe it's not a big issue, because it doesn't happen too much anyway.

[philiptaron]

That's neat. I support that.

[wolfgangwalther]

One thing that would be better with the "scheduled" approach similar to the labeler: Error recovery. If something didn't work, the next scheduled run will just have another look and possibly try again.

Both the current implementation and the workflow_run based solution suffer from this: If the first try didn't work, there won't be another one without user interaction.

I think I'm favoring the scheduled approach more - even if it introduces delays of a few minutes. Users will learn quickly that this is how it works. If not, we could also do both scheduled for the actual processing - and the workflow_run hack just to acknowledge the receipt of the comment via 🚀.

[MattSturgeon]

If not, we could also do both scheduled for the actual processing - and the workflow_run hack just to acknowledge the receipt of the comment via 🚀.

This sounds like a reasonable middle ground. I think a reactive response (reaction, comment, ...) is important, especially if the actual merge itself isn't instant.

[philiptaron]

I noticed that @dependabot does this. It really makes quite a difference in making the bot feel "live" even if it's powered by some asynchronous process.

[wolfgangwalther]

One other important improvement from doing this in Nixpkgs via GHA: We can test these things much better. Currently, despite multiple requests, we still don't have access to the staging deployment. I don't enjoy testing things on production. But no matter whether staging or production: The fact that all of this is event based means that any testing always requires new events - per definition. This makes it much harder than needed to reproduce and iterate. The proposed scheduled approach of going through open PRs can use the existing testing infrastructure in Nixpkgs' ci/github-script - it allows to easily run the respective scripts directly on a specific PR, both with and without dry mode. This makes iteration much faster than what we currently have.