-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PAT tokens unable to add to merge a PR with the merge queue #8352
Comments
Not sure if setting However, I was having failures trying to set a PR to be configured from auto-merge using fine grained token. The same token successfully approves the PRs. With the help of https://github.com/orgs/community/discussions/24686 i was able to get it working by adding Contents read/write to the allowed permissions of the fine grained token. Seems excessive to me for my use case. |
Hey @mhemmings, thanks for raising the issue. That sounds pretty annoying. Could you provide the output with |
Hi @williammartin, he's the debug output:
|
Seems to be calling the incorrect API? Should be calling |
Definitely something funky about those requests and responses, they are missing fields relating to the merge queue which would result in the CLI determining the right course of action. Will look into this more tomorrow! |
@williammartin Any update? I am getting this error also for a token. Using SSH key however is okay. |
We're seeing the same issue in the context of a Github action, which does some auto merging for us. It's in a public repository -- here's a link to the failing job with error message: https://github.com/contentful/marketplace-partner-apps/actions/runs/7136977689/job/19436248905?pr=567
(EDIT: I realize that the main issue description claims this is only an issue with PATs, but we're seeing exactly the issue in the context of a Github Action with a |
Sorry folks, I was really focused on getting the multi account support out this week and everything else fell by the wayside. I'll make an effort to prioritise this. |
Here's what output looks like for a PR of mine that went through merge queue:
The relevant difference here from your output are the fields: I created a PAT here with what I think are matching scopes for yours: |
@mhemmings I'm sure you've looked at this but I would be remiss not to be absolutely certain. Can you double check:
In the meantime I'll think about what other differences there could be here. |
@jsdalton, I'm not sure but I suspect that something in your workflow permissions needs to be changed. Since @mhemmings has this working in Actions, perhaps they can share the |
@williammartin Permissions are the same as yours (see screenshot below) and apply to the repo being merged. I'm fairly confident the PAT was used correctly, but I will do a full test again today to confirm. |
Thanks, I know it feels like first line tech support help but given that it worked for me I really don't want to go down a rabbit hole only to discover it was something simple. 🙏 |
To try and recreate, can you tell me any more about the
|
|
@mhemmings, I think I've found the issue in the platform that is causing this and seem to be able to reproduce it. Unfortunately, I don't have a great workaround for you right now other than using the enqueue mutation directly. I'm in discussion with the team that owns this feature to try and get this resolved on their side. As far as I'm concerned everything you've done is correct, the CLI is also doing what it is supposed to, and the platform has some incongruent behaviour. |
## Purpose Per discussion in this thread cli/cli#8352 the issue with our dependabot approve and merge might be that it lacks correct permissions. ## Approach * Try adding pull-request permission. See here https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs ## Testing steps <!-- Do you have a happy path a user would run through to test this app or would help the reviewer get started bug testing --> ## Breaking Changes <!-- Are there any changes to be aware of that would break current production build? --> ## Dependencies and/or References <!-- Where can we get more insights about this change? (Tickets, wiki pages or links to other places/docs -- no private/internal links please) --> ## Deployment <!-- (Optional) Are there any deployment-related tasks, concerns or risks we should be mindful of? -->
Thanks @williammartin I appreciate your working on this (and totally understand not wanting to fall down the rabbit hole if it's user error). To be clear in our use case adding the permission Our current permissions are https://github.com/contentful/marketplace-partner-apps/actions/runs/7169699697/workflow#L8-L10
|
@jsdalton, given that @mhemmings has this working in GitHub Actions, I would expect that it's possible to work there. Was hoping that he could either:
|
Thanks for that clarification @williammartin . The action itself is in a private repository but in this repo it appears we are overriding the I'm talking to the team who owns the action about fixing this (the override is unneeded I believe) so hopefully this resolves the issue. If this is indeed the issue on my side then it's further confirmation of the problem with PATs you've already identified. Thanks again for your active help and feedback here. |
It's also possible that the reverse is true and @mhemmings is overriding with a PAT that does happen to work. I intend to write this up tomorrow but the gist of it is that in past the CLI had merge queue support for actors that were feature flagged to use it. This feature flag gated both the merge queue functionality and all of the GQL API. However, not all of the GQL API was stabilised so now it's possible to be using the merge queue without being in the feature flag for the unstable API. It's a bit of a mess! I have ideas on how to move this forward but being realistic, I don't think it would happen before the holidays. |
Thanks for the clarity @williammartin. If I learn anything relevant (one way or the other) I'll post my findings here. |
Workaround for cli/cli#8352 (cherry picked from commit 9328e7e)
Workaround for cli/cli#8352 (cherry picked from commit 9328e7e)
Workaround for cli/cli#8352 (cherry picked from commit 9328e7e)
Workaround for cli/cli#8352 (cherry picked from commit 9328e7e)
any update on this? |
No. Are you also running into it? Please drop a thumbs up on the original issue to help it stand out in our prioritisation discussions. |
I just started running into this just now. A GH workflow that previously was working just stopped working with this issue. I'm able to use the CLI locally successfully, but in the GH Actions workflow it fails. |
Yes, this just started happening the past hour on our side |
That's surprising. I can't think of any reason that it would stop working with no change on your end. Did you change the token in any way?
That isn't surprising based on #8352 (comment) |
Interesting. Just to check @rwong2888 and @marisbest2 are you in the same organisation? |
We are not |
Thanks all. It seems likely that a change has been shipped in the platform that has caused a regression in the CLI. We are currently investigating. |
I just set up a workflow for merging in PR's from dependabot by using a PAT from an admin user for the merge. This was working yesterday and earlier today but stopped working around 2:30 Eastern resulting in the errors reported above: GraphQL: Field 'isMergeQueueEnabled' doesn't exist on type 'PullRequest' (query PullRequestByNumber.repository.pullRequest.isMergeQueueEnabled) If I don't use the PAT we run into the branch protection rules that were set up, particularly requiring two approving reviewers. |
Let's move this conversation over to #8645 which is specifically for the platform change you are all experiencing right now. Updates will be provided over on that issue. |
@mhemmings, @jsdalton and @tyrone-anz do any of you still experience the original issue? I would expect that this has been resolved after some platform changes. |
Describe the bug
gh version 2.39.1 (2023-11-14)
The following command gives strange behaviour when using a PAT (fine-grained) token:
gh pr merge --auto "$PR_URL"
The CLI takes you through normal merge steps as if there is no merge queue, and then fails at the end.
However, running the same command in Github Actions with a
GITHUB_TOKEN
, the command succeeds as expected and the PR gets added to the merge queue.May be related to #7213, though not quite the same issue I don't think
Steps to reproduce the behavior
gh pr merge --auto "$PR_URL"
GraphQL: Changes must be made through the merge queue (mergePullRequest)
Expected vs actual behavior
As per the docs, the PR should be added to the merge queue. This works as expected when running in Github Actions with a
GITHUB_TOKEN
, but the above error happens with a PAT.The text was updated successfully, but these errors were encountered: