Automatically close invalid PRs using GitHub Actions

[iBug]

This is an enhancement or feature.

Summary

Use a GitHub Actions to close invalid PRs that would have been done manually.

This PR adds two aspects of content:

1. An "identification token" for detection, based on the fact that most (if not all) invalid PRs don't even bother reading the PULL_REQUEST_TEMPLATE, let alone properly filling them. This provides us with a nice simple heuristic that should "just work" for 99% of the cases.

   This PR adds something at the end of the PR template with the hope that a reasonable user would have removed it should they want to submit some real thing, and checks if the "must remove" content is left intact.

   Update 1: Shortly after the creation of this PR there came 3314 that included an empty body (the user just deleted everything pre-filled in the input box). Considering that this could also be a pattern, I updated the workflow file to include a check for this case.

2. A workflow file that runs on every opened or reopened PR, checking the "identification token", and closes it if found.

   This workflow runs on pull_request_target because the intuitive pull_request event does not trigger a workflow run if the PR has merge conflicts, which is often the case for invalid PRs. There's a reply suggesting this alternative trigger that according to my tests, works.

Unfortunately, I have yet to discover an easy way to perform these actions (comment + close) using only GitHub-provided workflow steps. While I certainly could craft API calls using curl or another command-line tool (or use more sophisticated things like Python, altogether), I chose not to overcomplicate things and use a Marketplace Actions step. If external dependencies are a concern to you, I'm happy to do this myself with scripts. Please let me know if this is the case.

Since we're not loading any content from the submitted PR (or any code at all), we should be safe from (malicious) code. The only input we're consuming is a string sent as part of the event payload. The only operation performed on it is contains() which is a substring match. TBH I would have been shocked if there were any security concerns on this simple thing.

Demo

Please see this PR and this workflow run on my repo.

Context

Recent influx of invalid PRs that submit personal site content instead of something intended to be included into the project.

P.S. I've been watching this repository for years and I too am tired of unsubscribing every invalid PR and cleaning up my email inbox.

[mmistakes]

I'm fine with using a GH action to filter out these bad PRs, but think this only solves part of the problem.
Sure it will close the pull request, but I believe we'll still get notified when someone mistakenly opens up a pull request since this workflow doesn't intercept them before we're emailed.

If there's a solution to do that, I'd be all for it as the noise caused to my (and your's) inboxes is a nuisance. Only option I've found is to unsubcribe from all notifications on this repo, which I'd prefer not to do.

[iBug]

You're right that this doesn't automatically unsubscribe us or otherwise cleans up our inboxes, but I do find it occasionally useful when the author continues to push commits after opening the PR. It's also nice that pushes to the HEAD branch will no longer be considered "updates" once a PR is closed, so I'd say it helps more or less. After all it saves you some time to close each PR after your inbox gets cluttered up so you'll only have to delete the emails.

The ultimate solution, unfortunately, is to unsubscribe from the repo (issues/PRs at least) and only subscribe to specific threads we're interested in. There certainly are more elaborate methods to build complex stuff to achieve this but I'd doubt it'd be worth the efforts (I've already made something alike). I'd say this is as far as we can get without stepping too deep into this swamp.

[iBug]

@mmistakes Does this PR deserve more attention or higher priority?

[antenore]

On Gitlab there are more controls over Merge Requests (aka PR), like who can submit a MR, and you can limit it to Project Members. So at the end you can require to send the patches with a different workflow...

This plus the @iBug solution would help a lot...

BTW, I'm a long time user and following everything is going on since a long time. @mmistakes, you know me from the Remmina project maybe and via PayPal ;-) if you need triaging issues and PRs, I can help you a bit.

Beside asking for more information, I could simply add specific comments, like (@mmistakes close, or attention, or merge, or whatever), or if you can give me a little of powers, I can also close those PRs myself, like #3322

[iBug]

Part of the reason I'm making this automated thing is my reminiscence of Michael's not willing to give write access to other people. GitHub so far doesn't allow "Triage" access for personal repositories, only "collaborators" that includes push access, so I "had to" implement something that doesn't depend on me having power over the repository.

Adding to @antenore's comment, I'm also glad to help with triaging Issues, PRs and Discussions if Michael'd grant me the privileges. (Pretty sure I expressed this before)

[mmistakes]

I appreciate the support and willingness to jump in and help. But as stated before, I feel more comfortable doing it myself.

Most of the issues/PRs/discussions that lack action or feedback from me are usually ones that I feel are "thorny" and require a defense for something I have no interest in acting on. Sometimes it's just easier to remain silent. That's a failing on my part, but hey... open source is hard.

I suppose I could just close issues/PRs, but that would likely rub people the wrong way.

RE: this PR. My hesitation is it's a partial solution. I'd rather do something that fixes the entire problem or nothing at all. Closing a pull request doesn't really bother me. It's the notifications... and those repeat ones when someone constantly pushes commits.

Are you certain that once a pull request is closed... the notifications stop? Because I could have sworn that in the past I would still get emails even after closing them. I had to unsubscribe from each to get that to take affect. But maybe something has changed in recent years.

[iBug]

Thanks for the heads up! Once a PR is closed, new pushes to the head (source) branch won't trigger additional notifications. However if anyone comments on the PR it still shows up (which happens rarely). If you want to completely halt it as-is you'll have to lock them as well, which I'm not confident in pushing this as the detection heuristics is too simple at this point (I can do it if you want).

If it were my repo I could've built more sophisticated solutions using webhooks and GitHub API (with my Personal Access Token), writing automated stuff that cleans up and unsubscribes myself from these mistaken PRs.

Unfortunately without going "too intrusive" it's only possible to create a partial solution. This is as far as I can get so that in most cases you only have to delete your emails and take no more actions. For you this relieves some tedious tasks, and for me this mostly solves the case where the author continues to add extra activities before either of us reacts that stacks up in our inboxes. So I'd classify this PR as "nice-to-haves" for everyone watching the repository.

[iBug]

BTW, I just figured out that I could unwatch from the repo and watch again using a bot account. Then I could parse notifications for the bot account and only subscribe myself to the ones (Issues/PRs) I'm interested in. Maybe I could make some prototype or proof-of-concept when my exam-season is over.

[mmistakes]

OK. I'll merge this in and see if it helps anything. I'd rather not over engineer things and add more complexity to this repo.

[iBug]

There's been 3 bad PRs handled by this new "facility". The response time is typically within 20 seconds - looks decent to me.

[mmistakes]

Thanks @iBug! It's working great. Big improvement.

[iBug]

So far, we have 100% precision and ~97% recall (179 / 185) over the past three months, and PR 3592 is the sixth false negative. It's interesting that they did bother reading the template in whole and removed just the last comment, and without thinking carefully about what they're submitting.

One good point is the first 5 FNs are closed by their authors after realizing they've submitted undesired stuff, and this is the first that stayed up for longer than necessary.

I've always had this in mind: A new (additional) heuristic that can maintain a 99%+ recall over the past samples.

If _config.yml is modified and url: is changed, then it's most probably not meant to be submitted.

It's more likely this is a one-of-a-kind occurrence or a "tolerable error", so I don't think it's urgent to get this implemented and running. Anyway, I'm leaving this idea here for future reference, and I'd be glad to put it up if you want.