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

Workflow filter incorrectly includes/cancels runs from other branches #82

Closed
mikehardy opened this issue May 4, 2021 · 12 comments · Fixed by #105
Closed

Workflow filter incorrectly includes/cancels runs from other branches #82

mikehardy opened this issue May 4, 2021 · 12 comments · Fixed by #105

Comments

@mikehardy
Copy link
Contributor

I can't adequately explain it, but currently the only explanation I have for the behavior I just saw is that this line is not filtering runs from other branches:

Specifically this happened in an "all_but_latest" self-cancelling config:

1- workflow run on a PR from a branch starts
2- I merge to master branch (triggers another run)
3- I merge again to master branch (triggers another run)

Run 3 cancelled 1 and 2 even though 1 was on another branch

It looks like the workflow_id is not a parameter that the octokit API call accepts, but branch definitely is
https://docs.github.com/en/[email protected]/rest/reference/actions#list-workflow-runs-for-a-repository--parameters

The current run ids would not match, the head_shas did not match, nothing matched so they didn't get filtered - thus my only guess is that the repo workflow list call was expected to filter on branch but clearly did not, which is unexpected.

🤔

@styfle
Copy link
Owner

styfle commented May 6, 2021

It looks like the workflow_id is not a parameter that the octokit API call accepts

That's really odd. They allow it in the octokit sdk so I wonder if it was recently removed from the API?

@styfle
Copy link
Owner

styfle commented May 6, 2021

The old API docs mention workflow_id here:

https://web.archive.org/web/20200207173916/https://developer.github.com/v3/actions/workflow_runs/#list-workflow-runs

image

@styfle
Copy link
Owner

styfle commented May 10, 2021

At the very least, it sounds like a bug in @actions/github since it accepts the branch parameter and even documents it.

Returns workflow runs associated with a branch. Use the name of the branch of the push.

image

@mikehardy
Copy link
Contributor Author

definitely agree - it hasn't risen above "notable, and a nuisance" for me yet so I haven't taken the time to run it down over on that side but I do think it's an issue with this action as collateral damage

@elocke
Copy link

elocke commented Jul 22, 2021

looks like I'm hitting this too. Just tried the action out for the first time yesterday

@mikehardy
Copy link
Contributor Author

Unfortunately I definitely won't have time to track it down, but my clearest recollection of it based on close examination of output was exactly as above, filter by branch just wasn't working, and it could be worked around here or fixed upstream in the github workflow run query (if that's open?)

@spaceface777
Copy link
Contributor

spaceface777 commented Jul 26, 2021

I've hit this myself as well...
... with this config (i.e. pretty much the Pull Requests from Forks example in the README).

The bad news is that this issue was a deal-breaker for us;
the good news is that I forked the repo and fixed the bug because of it 🙂.
The even better news is that using my fork is as simple as replacing styfle/[email protected]
with spaceface777/[email protected] in your workflow config.

Basically, as most of you probably figured out, this action assumes that if two branches have the same name, while this is not necessarily true (for example, the main branch of your repo, and a PR that also happens to use main as the name of its base branch for some reason).

This behavior can be seen in this PR, for example, which caused this cancel workflow run to start, and in turn cancelled the checks for this commit, which was pushed to the master branch of the main repo, not of the PR.


Extended description of the bug in our case (using the PR workaround):

Basically, the problem comes from the very nature of how the workflow is designed in order to be able to cancel pull request jobs. Every time the workflow listed under workflow_run is started, Actions automatically schedules the Cancel job as well.

Now, the GitHub api which repo started a workflow, so I first tried filtering out all the workflows where the initiator repo did not match that of the currently running job. This would have worked if I was not using the PR permissions workaround, but due to that it did not:

Recall that the cancel workflow is run on demand. It turns out that whenever this happens, the initiator repo inside GitHub's api payload is always the parent repo, not a fork, even if that workflow was triggered from a PR. Luckily, GitHub provides yet another api payload that does tell you which workflow caused this cancel one to be scheduled, so I simply made the filtering logic use that repo for deciding if a job should be cancelled or not.

@styfle
Copy link
Owner

styfle commented Jul 26, 2021

I'm looking at the PR in question but I don't see any workflow run with "cancel".

So perhaps the bug is only present when using events other than "push"?

@spaceface777
Copy link
Contributor

@styfle that's a github thing - if a workflow is triggered from a PR, that workflow will not appear in the workflow list for that PR.

If you think about it, it makes sense, since github treats that workflow as belonging to the parent repo, not the pr, thus causing this bug.

@styfle
Copy link
Owner

styfle commented Jul 26, 2021

The fix in 8cbb31 looks good.

I think you can use payload.workflow_run.head_repository.id instead of parsing the event again.

@spaceface777
Copy link
Contributor

Oh, thanks for the tip.

Would you be willing to accept a PR with that patch?

@styfle
Copy link
Owner

styfle commented Jul 26, 2021

Yes, that would be great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants