Repository navigation
gh pr merge --auto command behaviour is confusing #3514
Description
Activity
Thanks @jagobagascon!
If you run the command but the repo does not have any checks it does nothing.
Out of curiosity, why are you passing
--autoif there are no checks? Why not just usegh pr merge?If you run the command after the required checks have already passed it does nothing.
I think I agree with your proposed approach, that the PR should be merged if you pass that and all the required checks have already passed. Curious what others think.
If you run the command but the checks defined are not set as required it does nothing.
It seems like at the very least some feedback here would be appropriate.
I think generally, auto-merge is intended to be for merging after a certain set of conditions are met. It's unclear to me why you would use it if there are no conditions or if there are none required, and I think it's more likely that someone intended for the PR to meet some conditions if they're passing
--autoand therefore just merging immediately might be unexpected. So I'm hesitant to just merge as opposed to providing feedback to users that there aren't any conditions or there aren't any required conditions, and therefore completing this will just merge the PR.Out of curiosity, why are you passing --auto if there are no checks? Why not just use gh pr merge?
We use an automated release process that doesn't know if the repo it is targeting has any checks or not, so it always runs with the
--autoflag. All of our repos have checks, we just had some of them set as not required by mistake, which caused some errors.It may not make sense to use
--autoflag on a repo without checks or with non-required checks, but I believe it should merge the PR cause having no checks means all checks have passed 😄 (at least that's how I understand it).We could probably make our release"smarter" by using
gh pr statusto detect if the PR has checks or not, but I was looking for a way of telling GitHub: hey, merge this PR when everything is ok.Reacted by Mark Adamson and Ray SmetsThanks for writing in!
Agreed that:
- PR in a repo without checks should get immediately merged;
- PR that has "required" checks already passed should be immediately merged.
These feel to me like a platform (not a client) issue. We'll try to bubble it upwards as feedback to the PR team.
If you run the command but the checks defined are not set as required it does nothing.
This feels like a gray area! I think the design of auto-merge was meant to exclusively center around "required" checks. Maybe we could print a warning for the user in this case.
After talking to some folks internally, our agreement was that this is more of a client concern, i.e. that we should address this from GitHub CLI instead of on the platform side.
Here's how I envision an improved behavior:
- When a user runs
gh pr merge --auto, we use themergeStateStatusGraphQL field to determine the overall mergeability of a PR; - If the PR appears mergeable, we merge right away instead of scheduling an auto-merge. This alone would address most, if not all, scenarios described in this issue;
- If
mergeStateStatusis "BLOCKED", then the PR is waiting on things like approving reviews and/or Checks; and we should go ahead and schedule an auto-merge; - If the PR isn't mergeable due to git problems (merge conflict or the head branch not being up to date), we could print some warning about this (since a manual intervention of either the PR author or the project maintainer will be needed) but schedule the auto-merge anyway. I'm on the fence about this last bit, so I'd welcome other people's thoughts.
Reacted by Billy Griffin and Hawken Rives- When a user runs
When a user runs gh pr merge --auto, we use the mergeStateStatus GraphQL field to determine the overall mergeability of a PR;
If the PR appears mergeable, we merge right away instead of scheduling an auto-merge. This alone would address most, if not all, scenarios described in this issue;
If mergeStateStatus is "BLOCKED", then the PR is waiting on things like approving reviews and/or Checks; and we should go ahead and schedule an auto-merge;I think this may have introduced a race condition. I am setting a PR to auto-merge as soon as I have created it, which sometimes works, but sometimes it gives:
Pull request is not in the correct state to enable auto-merge
I wonder if it is getting one state from mergeStateStatus, but then when it comes to action it, the state has just changed.
As per the original request, I just want a generic way to say 'merge once everything is ok'
I guess so as I'm using it on GitHub Actions (GitHub hosted) and I think that always has the latest version
@mungojam That's true; Actions always has the latest gh installed, but it can be ~1 week delay between our release and Actions images being updated with the latest gh because Actions images are only refreshed once a week and then need a few days to propagate fully to production.
I've filed a request upstream that there should be an API that avoids the risk of hitting the race condition that you've describing. Apparently, the issue is known. In the meantime, you can tweak your Actions workflows to react to this particular failure message from
gh pr merge --autoand retry with agh pr merge. That's the best workaround I can suggest to you right now.Reacted by Mark AdamsonI discovered this change was causing all my Dependabot PRs to merge instantly, presumably because my workflow with this command was run before the other check (my tests) had spawned.
The Dependabot docs currently are recommending this command in a sample workflow for enabling auto-merge for your PR with the promise that “Dependabot will not merge the PR until your checks pass”.
I fear this means that a lot of projects out there are now merging dependencies that are not passing their tests.
Certainly this was the case for several of my projects.
Not really this project’s fault, but someone at GitHub should probably change the Dependabot docs.
I discovered this change was causing all my Dependabot PRs to merge instantly, presumably because my workflow with this command was run before the other check (my tests) had spawned.
The Dependabot docs currently are recommending this command in a sample workflow for enabling auto-merge for your PR with the promise that “Dependabot will not merge the PR until your checks pass”.
I fear this means that a lot of projects out there are now merging dependencies that are not passing their tests.
Presumably if you have branch protection with required checks on, then this isn't an issue. There isn't a watertight way to know which checks will trigger for a given PR because not all hooks will lead to a check, some may be conditional on target branch etc.
I fear this means that a lot of projects out there are now merging dependencies that are not passing their tests.
That's indeed worrisome. Thank you for raising this concern, @mxcl.
On the other hand, if
gh pr merge --autodoes not merge currently mergeable PRs, then callinggh pr merge --autoon a PR that is subject to no checks whatsoever (or on a PR where all checks already passed) will never result in a merge.I'm torn between considering this to be a bug with GitHub CLI vs. considering this to be a fault of the repository that hasn't set up branch protection rules as @mungojam pointed out. I shall consult with my colleagues. 🙇
Ahh... found this thread after posting my issue #8206 on a similar topic.
I'm in favor of @mislav's idea here -If the PR isn't mergeable due to git problems (merge conflict or the head branch not being up to date), we could print some warning about this (since a manual intervention of either the PR author or the project maintainer will be needed) but schedule the auto-merge anyway.
CLI Feedback
I'm currently running CLI version: 1.9.2
What was confusing or gave you pause?
The way
gh pr merge --autobehaves is a bit confusing. I know there's a PR #3367 to fix the fact that the command never fails, but I've found other scenarios where the command does not behave as I would expect:If you run the command but the repo does not have any checks it does nothing.
If there are no checks at all then the PR should probably be merged.
If you run the command after the required checks have already passed it does nothing.
If I'm running
gh pr merge --autoit probably means that I don't care when did the checks pass, I just want to accept the PR if they do/did, so this should merge the PR.If you run the command but the checks defined are not set as required it does nothing.
I guess this is similar to the case without checks, here too I would expect the PR to be accepted and merged.
I'm not sure if any of these is a bug or it is the intended behaviour, that's why I added it as feedback!
Thanks in advance.