Skip to content

Fix check-for-reproducer action not adding Missing Repro comment#38531

Closed
kacperkapusciak wants to merge 6 commits intofacebook:mainfrom
kacperkapusciak:@kacperkapusciak/fix-needs-repro-issue-bot
Closed

Fix check-for-reproducer action not adding Missing Repro comment#38531
kacperkapusciak wants to merge 6 commits intofacebook:mainfrom
kacperkapusciak:@kacperkapusciak/fix-needs-repro-issue-bot

Conversation

@kacperkapusciak
Copy link
Copy Markdown
Contributor

@kacperkapusciak kacperkapusciak commented Jul 20, 2023

Summary:

This PR fixes a bug where GitHub Actions bot didn't add a comment when "Needs: Repro" label was present in an issue.

When a maintainer adds the label manually the bot comments with "Missing Reproductible Example" as normally.

It seems like the problem occurred because of a difference in a sandbox repository and the proper facebook/react-native repo environment.

My sandbox that I used to test #38338 had an "bot" account with Personal Access Token setup to reply to issues. Turns out that bots using PAT have more permissions and can trigger one action from the other.

The solution is to send the comment directly from the checkForReproducer action. This won't collide with other actions but sadly will duplicate the sending logic into two actions.

This PR also makes the bot respect when a maintainer removes and adds a label by hand and won't alter the maintainer decision.

Related to ☂️ #35591

Changelog:

[INTERNAL] [FIXED] - Fix check-for-reproducer action not adding Missing Repro comment

Test Plan:

When issue is added

  1. Bot adds comment and "Needs: Repro" label when no reproducer is present in the issue body
    image

  2. Bot adds comment and "Needs: Repro" label when a repository not connected to the user's name was submitted

image

  1. Bot doesn't trigger when Snack is present in issue body

image

  1. Bot doesn't trigger when Github Repo under the user's name is present in the issue body

image

When issue body is edited

  1. Bot removes the comment and "Needs: Repro" label when user edits the issue and adds a link to snack

image

  1. Bot removes the comment and "Needs: Repro" label when user edits the issue and adds a GitHub repo under user's name

image

  1. Bot triggers when user removes the reproduction from issue body

image

When comment is added

  1. Bot removes the comment and the Needs Repro label when user submits a repo under their name
    image

  2. Bot removes the comment and the Needs Repro label when any user submits a snack
    image

  3. Bot removes the comment and the Needs repro label when other user submits a repo under their name

image

When maintainer changes the label

  1. When maintainer changes the label it turns off the action, needs repro bot won't interfere with the issue anymore

image

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 20, 2023
@analysis-bot
Copy link
Copy Markdown

analysis-bot commented Jul 20, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,843,818 -2
android hermes armeabi-v7a 8,152,943 -1
android hermes x86 9,349,584 -4
android hermes x86_64 9,192,338 +2
android jsc arm64-v8a 9,455,584 -1
android jsc armeabi-v7a 8,636,721 +0
android jsc x86 9,538,674 +0
android jsc x86_64 9,781,970 -2

Base commit: 5705661
Branch: main

@github-actions github-actions bot added the Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. label Jul 20, 2023
@cortinico
Copy link
Copy Markdown
Contributor

My sandbox that I used to test #38338 had an "bot" account with Personal Access Token setup to reply to issues. Turns out that bots using PAT have more permissions and can trigger one action from the other.

If having a bot with a PAT can solve this, I'll gladly set up one for @react-native-bot and add the token to the setup

@kacperkapusciak
Copy link
Copy Markdown
Contributor Author

If having a bot with a PAT can solve this, I'll gladly set up one for @react-native-bot and add the token to the setup

I'm afraid of a case where there would an infinite loop somewhere I haven't tested yet (or 20 max invocations as GitHub docs says) so maybe it's better to have it that way

@cortinico
Copy link
Copy Markdown
Contributor

so maybe it's better to have it that way

What do you mean with "that way" here?

@kacperkapusciak
Copy link
Copy Markdown
Contributor Author

@cortinico sorry, I meant without the PAT so loops in GH action invocation wouldn't be possible. I think it's safer

@kacperkapusciak kacperkapusciak marked this pull request as ready for review July 21, 2023 10:15
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jul 21, 2023
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@kacperkapusciak kacperkapusciak changed the title Fix checkForReproducer action not adding Missing Repro comment Fix check-for-reproducer action not adding Missing Repro comment Jul 21, 2023
@kacperkapusciak
Copy link
Copy Markdown
Contributor Author

@cortinico I coded it so it when a non-author (a maintainer) alters the "Needs: Repro" label it turns off the action completely

Q: Do you think it should attempt to remove the "Missing Repro" comment when a maintainer removes the label before it turns off?

image

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jul 21, 2023
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@cortinico merged this pull request in 3f78fa9.

@cortinico
Copy link
Copy Markdown
Contributor

Q: Do you think it should attempt to remove the "Missing Repro" comment when a maintainer removes the label before it turns off?

Not necessarily. Let's keep it as it is for now, and we can refine it afterwards

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants