Skip to content
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

On pull request, check run Safe-setting validator is created but never runs ("Not working on a PR, returning...") #540

Open
andi-bigswitch opened this issue Oct 20, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@andi-bigswitch
Copy link

Problem Description

I am seeing a non-deterministic issue with pull requests to the admin repo (we are using the fork model, so the pull requests come from a fork of the repo). Sometimes, the head commit on these pull requests has a pending (yellow) Safe-setting validator check run created with a description of Queued — Waiting to run this check..., but nothing more happens. The check run never completes.

What is actually happening

The check run stays yellow/pending and never completes

What is the expected behavior

The check run eventually completes.

Error output, if available

With debug logging enabled, I see that safe-settings reacts to pull_requests.opened and creates a check run and then reacts to a check_run.created event.

However, the handling stops with error message Not working on a PR, returning... here.

Looking at the Webhook Deliveries for my app, I indeed see that check_suite.pull_requests is empty (even though it was created by safe-settings. So it is not surprising that the check above is failing.

    "check_suite": {
      "id": 17454159573,
      "node_id": "CS_kwDOKfMBec8AAAAEEFlW1Q",
      "head_branch": null,
      "head_sha": "23a5560b00614263576b69dacbb80c4dd73c6dce",
      "status": "queued",
      "conclusion": null,
      "url": "https://api.github.com/repos/xxx/admin/check-suites/17454159573",
      "before": null,
      "after": null,
      "pull_requests": [

      ],

At this point I am not sure if this is

  • an issue with Github itself not populating check_suite.pull_requests, or whether
  • safe-settings is making invalid assumptions about that field being populated or whether
  • this is an artifact of me using a fork workflow? (During testing yesterday I did see the same problem though with a PR from a branch on the target repo).

Context

Are you using the hosted instance of probot/settings or running your own?

Our own.

If running your own instance, are you using it with github.com or GitHub Enterprise?

github.com

Version of probot/settings

2.0.25

Version of GitHub Enterprise

@decyjphr
Copy link
Collaborator

decyjphr commented Nov 1, 2023

Hi, I am able to recreate this. Looking into why the there are no pull_requests. Safe-settings is making that assumption that it would be there but it appears for forks the field is not populated.

This does not happen when working on a branching based PR workflow; only with forks.

Checking to see if a different way of triggering a check_run would work.

@decyjphr
Copy link
Collaborator

decyjphr commented Nov 1, 2023

Hi,
I don't believe it will work with the fork based PRs. I tried to see if there is a different way to trigger the checkrun but there isn't. It appears like a GitHub limitation that for forks, the pull_requests field is not populated. I suspect it is because the commit triggering the check_run is not in the admin repo (Base repo), but in any case, I am unable to get that information in the check_run webhook and so it is failing. I will make changes to better handle this and make the check_run as completed with resolution as failure

@andi-bigswitch
Copy link
Author

@decyjphr thanks for the investigation. I am still not sure whether this should be considered a Github bug (because the PR field isn't populated for fork-based PRs) or a limitation of safe-settings. I've also poked my Github Enterprise support ticket on this question.

If changing the Github-side behavior is not possible, a thought on a mitigation: The sequence of events that I have observed is:

  1. safe-settings gets called for pull.create, with both the PR information and the head commit
  2. safe-settings creates a check run
  3. Github calls back safe-settings with a check_run.created, which unexpectedly does not contain the pull req info.

It seems safe-settings could remember the commit -> pull_req information from (1) and then use that in (3), e.g., in a Hash Map, ideally one that is size limited and expires entries after a period. Then it would not need to rely on Github to ping-pong back the PR info in step 3, meaning this would work across both local-branch and fork-based workflows.

Is this direction something that could be considered?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants