Check not present on a PR when it should be

[filmaj]

PR in question: https://github.com/adobe/aem-spa-project-archetype/pull/37

Relevant deliveries:

The bottom delivery in the above screencap was for when the PR was opened. It looks to have delivered a proper payload of 'adobe employee' on the PR originally. Since then, there were 3 ‘pull request edited’ events, 1 ‘pull request synchronized’ event, and 1 ‘check suite requested’ event, all of which return HTTP 400 - by design, as the code in its current form ignores any event that is not a PR being opened or reopened.

The issue is that the check is now not set. It seems to have disappeared since the PR was opened. Not sure why! That's the bug.

@lydiapuric if you have any additional information as to what went on in that PR that would help us debug this situation, please share :)

[lydiapuric]

@filmaj I am the only one who pushed to this PR. I did one change before the PR on the repository (removing the workflow check) and after that no changes were taken in the repository itself. I made changes in the description of the PR, but that's it...

[filmaj]

Thanks for the input and clarification @lydiapuric ! We will try to reproduce on our end and try to get to the bottom of it...

[filmaj]

Interestingly, the next PR opened on the repo set the check successfully https://github.com/adobe/aem-spa-project-archetype/pull/38.

The webhook events that fired for this PR, so far, were, in order:

1. Pull request opened (and this set the check on the PR)
2. Pull request review requested (and this got ignored / HTTP 400)
3. Pull request edited (and this got ignored / HTTP 400)
4. Check suite requested (and this got ignored / HTTP 400

As far as I can tell, the only difference between this set of events, and the one from @lydiapuric's PR, was that it had two less PR-edited events, and no "PR synchronized" event.

🤔🤔🤔

[filmaj]

I think I'm getting closer to identifying this bug.

I issued a pull request to this repo (https://github.com/adobe/cla-bot/pull/32). Currently, it has two commits. However, when I initially sent the PR, there was only 1 commit in the branch. After opening the pull request, the CLA check was applied correctly to the PR.

However, when I pushed the second commit to the repo, the CLA check disappeared.

Pushing the second commit triggered a "PR synchronized" webhook event, which our bot ignored (and responded with a HTTP 202 code).

We may need to have the bot respond to synchronize events in order to persist the check. Possibly also (and this needs more investigation) to other PR events too (like review requested, edited, etc).

[hirenoble]

I would say check should get fired for all PR events except PR closed. This way way we can cover all the use case which are important but we might be missing. It's safer to cover larger set of use cases.

[filmaj]

I agree. Is there a reason for us to try to limit checker invocations? Are we subject to rate limits from Adobe Docs API? I don't think we have to worry about GitHub rate limits at this point.

[stevengill]

We aren't subject to rate limit APIs. We could do the check on every PR related event (except close). I was just thinking it would be a lot of extra computation for no reason. Once an approve has been sent, we don't really want to run the check again. But if these other events are removing the check, it would be better to just run it on all of the PR events (except close)

[filmaj]

I think we should test which PR events clear out the check. It looks like synchronize does. I want to also test review_requested and edited.

[filmaj]

I believe I have proved that responding with no content/action (i.e. HTTP 202) to a PR synchronize event will wipe out the check. See https://github.com/adobe/cla-bot-playground/pull/37

[filmaj]

OK, and I have confirmed that review_requested and edited events do not affect the state of the GitHub Check.

Therefore I recommend we tweak the bot so that it triggers on PR open, reopen, and synchronize.