Commit listing issues in 1.7.x and 1.8.x

[bluekeyes]

The new commit listing logic introduced in 1.7.0 also introduced several significant bugs that break various parts of the application. As a result, we don't recommend deploying the 1.7.0, 1.7.1, 1.8.0, and 1.8.1 releases.

This issue serves as a summary of the problems and a tracking issue for the final fix.

In 1.7.0, commit listing was switched to read the history of a commit from the head repository instead of from the pull request to populate the pushedDate field and fix invalidate_on_push for pull requests from forks. This relied on the commits field existing in the pull request object, which was not true for objects included in pull_request_review event payloads. The server would crash when processing these events.

This was fixed in 1.8.0 and revealed a more significant problem. By listing the history of a single commit, commits already on the target branch but included in the PR via a merge commit were also considered by the bot. This broke collaborator detection and could break file predicates, since the commit list is truncated to the number of commits GitHub reports as being part of the pull request.

Both of these issues were caused by bad assumptions made about the GitHub API that were not adequately tested before release. In addition to fixing the bug, we'll also be looking at better integration testing strategies.

[bluekeyes]

I believe this is fully fixed in the upcoming 1.9.0 release:

• For PRs that are from the same repository, commits are listed from the pull request
• For PRs that are from forks, commits are listed from the pull request and then a second request is made against the fork repository to load the pushedDate field for all commits included in the pull request