Skip to content

fix: inaccessible pull_request number in workflow_run#45

Merged
typeslint-cli[bot] merged 3 commits intoTypeslint:developmentfrom
Muunatic:development
Dec 5, 2023
Merged

fix: inaccessible pull_request number in workflow_run#45
typeslint-cli[bot] merged 3 commits intoTypeslint:developmentfrom
Muunatic:development

Conversation

@Muunatic
Copy link
Copy Markdown
Member

automaton encountered an error where the pull_request[0].number wasn't accessible within the workflow_run. pull_request would return an empty array when pull requests are made from forked repositories (outside main repository).

image

@Muunatic Muunatic added the Patch label Nov 20, 2023
@Muunatic Muunatic added this to the v3.0.0 milestone Nov 20, 2023
@typeslint-cli
Copy link
Copy Markdown
Contributor

typeslint-cli Bot commented Nov 20, 2023

Hello @Muunatic Thank you for submitting Pull Request, please wait for next notification after we review your Pull Request

@typeslint-cli typeslint-cli Bot added Core Pending Unread Issues/PRs labels Nov 20, 2023
@Muunatic
Copy link
Copy Markdown
Member Author

this fix is temporary and has not been tested to confirm its completely resolves the issue.

@HarunamiYaki
Copy link
Copy Markdown
Member

It seems the issue stems from the API indicating an empty pull request array https://docs.github.com/en/rest/checks/runs?apiVersion=2022-11-28. Even though this isn't from codebase issue itself but rather than from the GitHub API.

@Muunatic
Copy link
Copy Markdown
Member Author

It seems the issue stems from the API indicating an empty pull request array https://docs.github.com/en/rest/checks/runs?apiVersion=2022-11-28. Even though this isn't from codebase issue itself but rather than from the GitHub API.

indeed, checks_run are returning empty pull_request array data if pr comes from a forked repo. that is why i need to get pr author pulls list from the head_branch to get the exact pull_request data. for now it only works on the public repository and i need to test if the payload can still get pull_request data if pr comes from a private forked repo.

@Muunatic
Copy link
Copy Markdown
Member Author

i'm going to skip the review for now. i'll convert this prs to draft to double-check everything is good.

@Muunatic Muunatic marked this pull request as draft November 21, 2023 15:26
@HarunamiYaki
Copy link
Copy Markdown
Member

I noticed the code might be pulling the wrong pull request if there are multiple open PRs. When you're fetching the PR number using res.data[0].number, it might not always grab the right one if there are multiple open PRs. If the workflow gets triggered on Pull Request #1 but you fetch res.data[0], it could mistakenly refer to Pull Request #2.

A possible fix could be filtering the data by head_sha. This tweak should help automaton to get the precise pull request data matching the triggered commit.

res.data.find((a) => a.head.sha === this.context.payload.workflow_run.head_sha)?.number as number;

@Muunatic
Copy link
Copy Markdown
Member Author

I noticed the code might be pulling the wrong pull request if there are multiple open PRs. When you're fetching the PR number using res.data[0].number, it might not always grab the right one if there are multiple open PRs. If the workflow gets triggered on Pull Request #1 but you fetch res.data[0], it could mistakenly refer to Pull Request #2.

A possible fix could be filtering the data by head_sha. This tweak should help automaton to get the precise pull request data matching the triggered commit.

res.data.find((a) => a.head.sha === this.context.payload.workflow_run.head_sha)?.number as number;

sounds good.

@HarunamiYaki
Copy link
Copy Markdown
Member

Hey there! Just checking in on your pull request – any updates on this? Is it good to merge or are the tests still in progress?

@Muunatic Muunatic marked this pull request as ready for review December 4, 2023 02:59
@Muunatic
Copy link
Copy Markdown
Member Author

Muunatic commented Dec 4, 2023

Hey there! Just checking in on your pull request – any updates on this? Is it good to merge or are the tests still in progress?

ready for review @HarunamiYaki

Copy link
Copy Markdown
Contributor

@typeslint-cli typeslint-cli Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Muunatic your pull request has been approved by [MAINTAINER]@HarunamiYaki, please type Ready to merge for merging

@typeslint-cli typeslint-cli Bot added Approved Approved PRs and removed Pending Unread Issues/PRs labels Dec 4, 2023
@Muunatic
Copy link
Copy Markdown
Member Author

Muunatic commented Dec 5, 2023

ready to merge

@typeslint-cli typeslint-cli Bot merged commit 50e1ebe into Typeslint:development Dec 5, 2023
@typeslint-cli
Copy link
Copy Markdown
Contributor

typeslint-cli Bot commented Dec 5, 2023

Merged by Muunatic!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] workflow_run.completed returning empty pull request array

2 participants