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

META GitHub Bot #3238

Open
moul opened this issue Nov 28, 2024 · 44 comments
Open

META GitHub Bot #3238

moul opened this issue Nov 28, 2024 · 44 comments
Assignees

Comments

@moul
Copy link
Member

moul commented Nov 28, 2024

Do not hesitate to let me know if individual issues could be clearer, but I believe it's better to avoid the noise.

@moul
Copy link
Member Author

moul commented Nov 28, 2024

So, here, I clicked on "the pull request description", then the bot switched back to unchecked. Should be persisted.

CleanShot 2024-11-28 at 17 57 33

@moul
Copy link
Member Author

moul commented Nov 28, 2024

Please change the logo; it is generally poor and not suitable for small sizes.

@moul
Copy link
Member Author

moul commented Nov 28, 2024

CleanShot 2024-11-28 at 17 59 39

The introduction should be aimed at those who are not familiar with the bot. It could say something like, "I'm a bot that assists the core team in maintaining this repository.

followed by the details."

Remove the h1.

@moul
Copy link
Member Author

moul commented Nov 28, 2024

CleanShot 2024-11-28 at 18 00 59

Group the two "details" into a single "debug" at the end.

@aeddi
Copy link
Contributor

aeddi commented Nov 29, 2024

So, here, I clicked on "the pull request description", then the bot switched back to unchecked. Should be persisted.

Morgan already pinged me about this. The bot unchecks because it can't fetch the list of members from the core-contributors team, so it can't verify if you're allowed to check. It might be a token issue, I'll look into it today.

Also this one is weird: https://github.com/gnolang/gno/actions/runs/12072757244

Please change the logo; it is generally poor and not suitable for small sizes.

The introduction should be aimed at those who are not familiar with the bot. It could say something like, "I'm a bot that assists the core team in maintaining this repository."

Group the two "details" into a single "debug" at the end.

Ok 👍

@aeddi
Copy link
Contributor

aeddi commented Nov 29, 2024

Another issue: https://github.com/gnolang/gno/actions/runs/12082335167/job/33693214789?pr=3096

Error: unable to update pull request: unable to create status on PR 3096: POST https://api.github.com/repos/gnolang/gno/statuses/2516635949b9e0b19e9b6a4c04115cf6fbf877c4: 403 Resource not accessible by personal access token []

@moul
Copy link
Member Author

moul commented Dec 2, 2024

🔴 The pull request head branch must be up-to-date with its base

This is already blocked by GitHub settings.

You can keep the check if you believe the bot explains it better, but it shouldn't block the merge. It should be marked "orange" to indicate a warning.

@aeddi
Copy link
Contributor

aeddi commented Dec 2, 2024

🔴 The pull request head branch must be up-to-date with its base

This is already blocked by GitHub settings.

You can keep the check if you believe the bot explains it better, but it shouldn't block the merge.

@moul I can disable it, just added it because it was one of the rule requested initially here, I think we should probably merge the fixes here first and then fine-tune the configuration.

It should be marked "orange" to indicate a warning.

I replaced all orange / pending / warning statuses by red / failed after answering this comment (see the response), basically for Github orange means "processing" and red means "failure" or "requirements not met". The CI is red when you don't have enough review yet for example (which is not an error but just an unmet requirement).

@aeddi
Copy link
Contributor

aeddi commented Dec 2, 2024

Please change the logo; it is generally poor and not suitable for small sizes.

What about this one?

New avatar

Logo Short On White Background

@thehowl
Copy link
Member

thehowl commented Dec 2, 2024

This is already blocked by GitHub settings.

Actually, not anymore; I don't recall exactly why, but this was disabled some short while ago.

@aeddi
Copy link
Contributor

aeddi commented Dec 2, 2024

I always have this message in the checks of my PR:
Screenshot 2024-12-02 at 14 33 54

Do you think it’s not blocking / that it doesn’t put the CI in "failure" state if everything else is green?

@jefft0
Copy link
Contributor

jefft0 commented Dec 2, 2024

I agree that the "out-of-date with the base branch" message is unnecessary. In addition, it can lead the author of the PR to keep clicking the "Update branch" button, which we don't want. (See this message from Leon to a user who was doing this.)

aeddi added a commit that referenced this issue Dec 2, 2024
This PR will allow debugging errors of [this
type](https://github.com/gnolang/gno/actions/runs/12072757244) that
unfortunately cannot be tested locally since they rely on the context of
GitHub Actions.

Since I also had to add flags to the matrix subcommand, I moved the two
matrix and check subcommands into subfolders.

This PR also modify the comment to stick to moul's request and fixes
several Github Actions errors.

Related to #3238 

Changes:
-
d11ad5a
moves matrix and check subcommands to their own packages in internal
-
462ac01
5c1edda
ffdce93
adds a debug to matrix subcommand (print event input / matrix output) +
direct output of matrix to GitHub Actions using a matrix-key flag
-
6af501d
embed comment template file as a string at compile time instead of
opening it at runtime
-
59c3ad6
modifies bot comment to meet [this
requirements](#3238 (comment))
-
241a755
filter out from the matrix generation and the PR processing all issues
or closed PRs (process / list only opened PRs)

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
</details>

---------

Co-authored-by: Morgan <[email protected]>
@thehowl
Copy link
Member

thehowl commented Dec 2, 2024

I always have this message in the checks of my PR:

yeah, it's a warning, but not a requirement. maybe we can enable it back since Teritori is now submitting PRs which are modifiable by us, and we have a check to ensure that the branch is modifiable by maintainers.

@moul
Copy link
Member Author

moul commented Dec 2, 2024

#3135 -> green, but red.

CleanShot 2024-12-02 at 23 18 53
CleanShot 2024-12-02 at 23 18 58

@jefft0
Copy link
Contributor

jefft0 commented Dec 3, 2024

On PR #2597, the "stale bot" added the stale label and a comment. This triggered the meta guidelines bot to add a comment. Is this considered "activity" on the PR and does it reset the timer for the stale bot?

image

@aeddi
Copy link
Contributor

aeddi commented Dec 3, 2024

#3135 -> green, but red.

Still the same error as here, basically, the bot is unable to create a commit status like this:

Screenshot 2024-12-02 at 15 57 04 Screenshot 2024-12-02 at 15 58 38

That's the last known issue for now. I created a test Github organisation and I'm currently trying to reproduce this issue to understand what settings needs to be modified on gnolang/gno.

@aeddi
Copy link
Contributor

aeddi commented Dec 3, 2024

On PR #2597, the "stale bot" added the stale label and a comment. This triggered the meta guidelines bot to add a comment. Is this considered "activity" on the PR and does it reset the timer for the stale bot?

Mmmh yes I think it should unfortunately reset the stale timer, the stale bot will probably remove the label when it will run tonight at 1:30am.

I think there are two workarounds:

  • Run the GitHub Bot on the different open PRs to reset the timer now (rather than waiting for each of them to receive a stale label).
  • Add an if statement to the GitHub Actions bot workflow to ignore the 'labeled' event when the author happens to be the stale bot.

@aeddi
Copy link
Contributor

aeddi commented Dec 3, 2024

Okay, I finally figured out what the issue was by brute-forcing random settings on an organization + a test repo with several users. The problem is that to create a commit status on a repo that belongs to an organization, the bot's PAT must not only have the commit statuses: read & write permission but also the associated account must either:

  • have an owner role in the gnolang organization (and not just a member)
  • have a write role in the gnolang/gno repo

And this is not documented anywhere as far as I know.
For anyone who comes across this, I also tried literally checking all the highest permissions on the PAT, but that didn't work either.

Screenshot 2024-12-03 at 15 23 04

While these permissions are sufficient on the PAT as long as one of the two conditions mentioned above is met.

Screenshot 2024-12-03 at 15 25 42

I also tried lower roles on the repo, but starting from triage, it no longer works. The minimum requirement is indeed to give the bot account a write role in the gno repo.

I will ask to someone with the admin role on gnolang/gno to give this role to the bot account, and everything should work normally.

@jefft0
Copy link
Contributor

jefft0 commented Dec 4, 2024

On PR #2597, the "stale bot" added the stale label and a comment. This triggered the meta guidelines bot to add a comment. Is this considered "activity" on the PR and does it reset the timer for the stale bot?

Mmmh yes I think it should unfortunately reset the stale timer, the stale bot will probably remove the label when it will run tonight at 1:30am.

You're right, @aeddi . The stale bot removed the stale label on PR #2597 . This will cause lots of confusion. I prefer your option "Add an if statement to the GitHub Actions bot workflow to ignore the 'labeled' event when the author happens to be the stale bot." In general, I think the bots should ignore each other's actions.

zivkovicmilos pushed a commit that referenced this issue Dec 4, 2024
Simple update of the bot README to mention the necessary permissions for
the bot to operate, see this comment:
#3238 (comment)

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
</details>
@aeddi
Copy link
Contributor

aeddi commented Dec 4, 2024

@jefft0 Okay, so after digging a bit, I just saw that it wasn't the addition of the label by the stale bot that triggered the Gno bot. I think that the events emitted by the GitHub Actions account (used by the stale bot) are not forwarded to any other workflows, because even if the Gno bot is subscribed to the pull_request: labelled event, it's not fired by the stale bot applying a label.

However, what seems to have happened after analyzing the PR and this workflow run is that:

  • The stale bot, via the GitHub Actions account, applied the stale label.
  • This triggered Codecov, which is not an action or a workflow, but a GitHub app (which explains why it is triggered by events that are not forwarded to GitHub Actions workflows).
  • Codecov edited its own comment, which triggered the workflow of the Gno bot.

So I will ignore the events emitted by Codecov in the Gno bot's workflow, but I wonder if the fact that Codecov edits its own comment every time the stale bot adds the stale label generates activity on the PR that causes the stale bot to remove the label afterwards.

That could explained why the stale bot removed the label right after adding it on the PR you sent me on Signal, when the Gno bot was not running yet.

Edit: I just checked, and indeed, that seems to be the source of the problem. The Codecov comment was edited right after the label was added by the stale bot. I have no idea if we can disable this on Codecov, and I don't have access to the admin interface.

Screenshot 2024-12-04 at 12 13 52 Screenshot 2024-12-04 at 12 14 44

@aeddi
Copy link
Contributor

aeddi commented Dec 4, 2024

FYI, the bot was just added as a contributor with a write role to the gno repo, so this last issue seems to be fixed : #3238 (comment)

@jefft0
Copy link
Contributor

jefft0 commented Dec 5, 2024

FYI, the bot was just added as a contributor with a write role to the gno repo, so this last issue seems to be fixed : #3238 (comment)

Yes, good news. The stale bot just added the stale label on PR #2655, the GnoD2D bot added its comment, and the stale label is still there. :-)

@aeddi
Copy link
Contributor

aeddi commented Dec 5, 2024

@jefft0 Unfortunately, I think the bot will notice the new comment and remove the label during its next run tonight at 1:30am.

I will open a PR to ignore event from codecov this morning.

r3v4s pushed a commit to gnoswap-labs/gno that referenced this issue Dec 10, 2024
Simple update of the bot README to mention the necessary permissions for
the bot to operate, see this comment:
gnolang#3238 (comment)

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
</details>
r3v4s pushed a commit to gnoswap-labs/gno that referenced this issue Dec 10, 2024
It's a bit of a "push and pray" since it would take too long to set up
to reproduce this on my test repo/org, but I've triple-checked the
documentation, the previous runs, etc., and it should be fine.

Related to
gnolang#3238 (comment)

<details><summary>Contributors' checklist...</summary>

- [ ] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
</details>
r3v4s pushed a commit to gnoswap-labs/gno that referenced this issue Dec 10, 2024
r3v4s pushed a commit to gnoswap-labs/gno that referenced this issue Dec 10, 2024
…#3303)

This PR improves the bot on two points:
- it now handle `pull_request_review` events to address [this
concern](gnolang#3238 (comment))
gnolang@4f7b0b8
- a new condition allows to check if a PR was created from a fork to
address [this
concern](gnolang#3238 (comment))
gnolang@f491d95
thehowl pushed a commit that referenced this issue Dec 10, 2024
This PR significantly modifies the github-bot's comment and adds a
button to force the success of its CI check, even it the requirements
provided in the config are not met.

Related to
#3238 (comment)

**Edit**: I updated [the comment
below](#3311 (comment))
by running the bot on my laptop if you want to see the result (so the
skip button is not working yet).
@thehowl
Copy link
Member

thehowl commented Dec 10, 2024

Edit: I think this can be the solution https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#workflow_run

sounds good to try this out :)

@aeddi
Copy link
Contributor

aeddi commented Dec 11, 2024

@gfanton warned me that the bot failed on his fork, which makes sense since the bot needs a token to work. I will simply add a condition to skip the bot workflow if the action variables/secrets are not set on the repo. This way, there will be no more failures on forks, and people who want to can still set up the bot on their fork.

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

No branches or pull requests

5 participants