support updating pull requests made from forks

[chdsbd]

Github's API doesn't support updating forks: https://developer.github.com/v3/repos/merging/#perform-a-merge

For reference, if it did, we would need to update this code to use the head/repo information of the fork:

kodiak/kodiak/pull_request.py

Lines 296 to 299 in 7375b45

	res = await self.client.merge_branch(
	head=event.pull_request.baseRefName, base=event.pull_request.headRefName
	)
	if res.status_code > 300:

To work around this API limitation I thought we might be able to clone the fork and do the update ourselves, but github app permissions are such that even if a PR is marked "Allow edits from maintainers.", our github app installation doesn't have permission to download that fork.

I believe we cannot do anything to implement this feature until github changes their API.

Related #100

[KaiSzuttor]

why don't you merge the master and the pull request and push it to a staging branch in the main repo. There is no need do change the fork.

[chdsbd]

@KaiSzuttor I think that solution would definitely work, but I don't believe that it would be an appropriate solution for Kodiak. For me Kodiak was built around a GitHub PR centered flow where Kodiak mainly automates the update and merging of PRs. I think having Kodiak create new branches for PRs from forks would not fit with this.

[KaiSzuttor]

thanks @chdsbd for the response. That's a pity to hear since that makes your tool useless to pretty much every project on github where almost all PRs come from forks. And that is probably the majority.

How do you use your tool for PRs on the project itself?

[chdsbd]

Kodiak can still merge PRs for forks, it just cannot update the branches. I mainly use Kodiak for work, so this limitation doesn’t affect me for the most part. For Kodiak itself most of the PRs are by collaborators on the repo, so we don’t hit this issue.

This limitation is definitely a blocker for using Kodiak on open source repositories, but if we want to be able to use GitHub PR features, I think creating new branches with the changes of a fork would break the regular GitHub PR flow.

I think the simplest solution is for GitHub to allow bots to update branches like regular GitHub users can.

[KaiSzuttor]

I just realized that there is the possibility to do exactly what we need via the v3 API (https://developer.github.com/v3/pulls/#update-a-pull-request-branch) but probably it's not allowed in github apps because of the preview state.

[chdsbd]

@KaiSzuttor Thanks, that endpoint looks promising. I'll test it out as a replacement for this endpoint that we use now: https://developer.github.com/v3/repos/merging/#perform-a-merge

[chdsbd]

I've tested with a GitHub app and https://developer.github.com/v3/pulls/#update-a-pull-request-branch works for updating PRs made from forks! It looks like it may be possible to implement this feature now.

I think the only changes that need to be made are removing this check against forks and refactoring the PR update logic to use the new API call.

kodiak/kodiak/pull_request.py

Lines 227 to 231 in 2f7c26d

	# PRs from forks will always appear deleted because the v4 api doesn't
	# return head information for forks like the v3 api does.
	if not self.event.pull_request.isCrossRepository and not self.event.head_exists:
	self.log.info("branch deleted")
	return MergeabilityResponse.NOT_MERGEABLE, None

kodiak/kodiak/pull_request.py

Lines 313 to 325 in 2f7c26d

	async def update(self) -> bool:
	self.log.info("update")
	event = await self.get_event()
	if event is None:
	self.log.warning("problem")
	return False
	res = await self.client.merge_branch(
	head=event.pull_request.baseRefName, base=event.pull_request.headRefName
	)
	if res.status_code > 300:
	self.log.error("could not update branch", res=res, res_json=res.json())
	return False
	return True

Thanks for bringing this API to my attention!

[KaiSzuttor]

do you plan to implement the update? we would really be interested in that feature and be willing to support the implementation if needed.

[chdsbd]

I plan to implement this and I'm hopeful I can do this in the next week or so.

[chdsbd]

#202 has been deployed

[chdsbd]

I'm very upset with GitHub at the moment.

It appears that GitHub has changed the /update-branch API to no longer work with forks.

When Kodiak (or any GitHub app) attempts to call /update-branch on a PR created from a fork, the GitHub API returns a 422 status code with the body, {"message":"user doesn't have permission to update head repository","documentation_url":"https://developer.github.com/v3"}, instead of updating the fork's branch.

Previously, as of #202's release, Kodiak was able to use this API successfully to update PRs made from forks, so it seems like a recent change to the GitHub API has impacted this update branch feature.

I'm not sure what any GitHub app can do to get around this.

I've opened a support request with GitHub, so hopefully that will go somewhere.

I think in the meantime I'll update Kodiak to warn about not being able to update branches from forks.

[KaiSzuttor]

It appears that GitHub has changed the /update-branch API to no longer work with forks.

i just tried locally with curl and it seemed to work. Are you sure that is was not only temporary?

[chdsbd]

@KaiSzuttor Sorry, I should have specified that this change only affects GitHub Apps.

Here’s a repo where I reproduced the error: https://github.com/chdsbd/test_github/pull/1

[KaiSzuttor]

That's really annoying. I contacted the github support and asked exactly about whether the beta api is enabled for github apps. The answer has been 'yes' 4 weeks ago 😠

[chdsbd]

GitHub Apps can still use the API update normal PRs, but updating PRs that depend on a fork's branch now returns an error.

As of #202's release, GitHub Apps were able to update all PRs, including ones dependent on forks. So GitHub must have change the API recently to prevent this. ☹️

[KaiSzuttor]

espressomd/espresso#3419 it still seems to work on our repository

[chdsbd]

@KaiSzuttor thanks for the link.

It appears that /update-branch does not work on a pull request from a fork if "allow edits from maintainers" is unchecked, which isn't surprising.

My problem was that I created a fork of a repository I owned to another organization I had permissions for, so when I opened the PR, I didn't have the option for "Allow edits from maintainers".

I'm going to open a new issue to track this.

[jberryman]

@chdsbd I'm glad to hear updating forks works for the common case. This is a good differentiator to include in https://kodiakhq.com/docs/prior-art-and-alternatives in my opinion (it's why (I learned after setting it up) bulldozer doesn't work for us)

[chdsbd]

@jberryman Thanks for the feedback. That table has definitely been neglected. I'll take a look at adding some more differentiators.

I've created a ticket to track this #487.