ci: fixes bot workflow and comment update (#3229)

This PR should (normally) fix the issues with the bot on this repo. 
In addition to the fixes, I also replaced the old config with a config
that has much simpler rules while we decide what to add later on, once
we've verified that everything is working properly.

Here is the current config, If nothing seems off to you, we can merge it
as it is then improve it incrementaly.

```go
auto := []automaticCheck{
  {
    description: "Maintainers must be able to edit this pull request",
    ifC:         c.Always(),
    thenR:       r.MaintainerCanModify(),
  },
  {
    description: "The pull request head branch must be up-to-date with its base",
    ifC:         c.Always(),
    thenR:       r.UpToDateWith(gh, r.PR_BASE),
  },
  {
    description: "Changes to 'docs' folder must be reviewed/authored by at least one devrel and one tech-staff",
    ifC:         c.FileChanged(gh, "^docs/"),
    thenR: r.Or(
      r.And(
        r.AuthorInTeam(gh, "devrels"),
        r.ReviewByTeamMembers(gh, "tech-staff", 1),
      ),
      r.And(
        r.AuthorInTeam(gh, "tech-staff"),
        r.ReviewByTeamMembers(gh, "devrels", 1),
      ),
    ),
  },
}

manual := []manualCheck{
  {
    description: "The pull request description provides enough details",
    ifC:         c.Not(c.AuthorInTeam(gh, "core-contributors")),
    teams:       Teams{"core-contributors"},
  },
  {
    description: "Determine if infra needs to be updated before merging",
    ifC: c.And(
      c.BaseBranch("master"),
      c.Or(
        c.FileChanged(gh, `Dockerfile`),
        c.FileChanged(gh, `^misc/deployments`),
        c.FileChanged(gh, `^misc/docker-`),
        c.FileChanged(gh, `^.github/workflows/releaser.*\.yml$`),
        c.FileChanged(gh, `^.github/workflows/portal-loop\.yml$`),
      ),
    ),
    teams: Teams{"devops"},
  },
}
```


<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>

.github/workflows/bot.yml

@@ -44,16 +44,18 @@ jobs:
     steps:
       - name: Checkout code
         uses: actions/checkout@v4
+
       - name: Install Go
         uses: actions/setup-go@v5
         with:
-          go-version-file: go.mod
+          go-version-file: contribs/github-bot/go.mod
+
       - name: Generate matrix from event
         id: pr-numbers
         working-directory: contribs/github-bot
         env:
-          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-        run: go run . matrix >> "$GITHUB_OUTPUT"
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: echo "pr-numbers=$(go run . matrix)" >> "$GITHUB_OUTPUT"
 
   # This job processes each pull request in the matrix individually while ensuring
   # that a same PR cannot be processed concurrently by mutliple runners
@@ -76,10 +78,10 @@ jobs:
       - name: Install Go
         uses: actions/setup-go@v5
         with:
-          go-version-file: go.mod
+          go-version-file: contribs/github-bot/go.mod
 
       - name: Run GitHub Bot
         working-directory: contribs/github-bot
         env:
           GITHUB_TOKEN: ${{ secrets.GH_BOT_PAT }}
-        run: go run . -pr-numbers '${{ matrix.pr-number }}' -verbose
+        run: go run . check -pr-numbers '${{ matrix.pr-number }}' -verbose

contribs/github-bot/README.md

@@ -27,14 +27,11 @@ For the bot to make requests to the GitHub API, it needs a Personal Access Token
 ## Usage
 
 ```bash
-> go install github.com/gnolang/gno/contribs/github-bot@latest
-// (go: downloading ...)
-
-> github-bot --help
+> github-bot check --help
 USAGE
-  github-bot [flags]
+  github-bot check [flags]
 
-This tool checks if the requirements for a PR to be merged are satisfied (defined in config.go) and displays PR status checks accordingly.
+This tool checks if the requirements for a pull request to be merged are satisfied (defined in config.go) and displays PR status checks accordingly.
 A valid GitHub Token must be provided by setting the GITHUB_TOKEN environment variable.
 
 FLAGS

contribs/github-bot/comment.go

@@ -175,9 +175,9 @@ func handleCommentUpdate(gh *client.GitHub, actionCtx *githubactions.GitHubConte
 
 		// If teams specified in rule, check if actor is a member of one of them.
 		if len(teams) > 0 {
-			if gh.IsUserInTeams(actionCtx.Actor, teams) {
+			if !gh.IsUserInTeams(actionCtx.Actor, teams) { // If user not allowed
 				if !gh.DryRun {
-					gh.SetBotComment(previous, int(prNum))
+					gh.SetBotComment(previous, int(prNum)) // Restore previous state
 				}
 				return errors.New("checkbox edited by a user not allowed to")
 			}

contribs/github-bot/config.go

@@ -6,6 +6,8 @@ import (
 	r "github.com/gnolang/gno/contribs/github-bot/internal/requirements"
 )
 
+type Teams []string
+
 // Automatic check that will be performed by the bot.
 type automaticCheck struct {
 	description string
@@ -17,73 +19,58 @@ type automaticCheck struct {
 type manualCheck struct {
 	description string
 	ifC         c.Condition // If the condition is met, a checkbox will be displayed on bot comment.
-	teams       []string    // Members of these teams can check the checkbox to make the check pass.
+	teams       Teams       // Members of these teams can check the checkbox to make the check pass.
 }
 
 // This function returns the configuration of the bot consisting of automatic and manual checks
 // in which the GitHub client is injected.
 func config(gh *client.GitHub) ([]automaticCheck, []manualCheck) {
 	auto := []automaticCheck{
 		{
-			description: "Changes to 'tm2' folder should be reviewed/authored by at least one member of both EU and US teams",
-			ifC: c.And(
-				c.FileChanged(gh, "tm2"),
-				c.BaseBranch("master"),
-			),
-			thenR: r.And(
-				r.Or(
-					r.ReviewByTeamMembers(gh, "eu", 1),
-					r.AuthorInTeam(gh, "eu"),
-				),
-				r.Or(
-					r.ReviewByTeamMembers(gh, "us", 1),
-					r.AuthorInTeam(gh, "us"),
-				),
-			),
-		},
-		{
-			description: "A maintainer must be able to edit this pull request",
+			description: "Maintainers must be able to edit this pull request",
 			ifC:         c.Always(),
 			thenR:       r.MaintainerCanModify(),
 		},
 		{
 			description: "The pull request head branch must be up-to-date with its base",
-			ifC:         c.Always(), // Or only if c.BaseBranch("main") ?
+			ifC:         c.Always(),
 			thenR:       r.UpToDateWith(gh, r.PR_BASE),
 		},
+		{
+			description: "Changes to 'docs' folder must be reviewed/authored by at least one devrel and one tech-staff",
+			ifC:         c.FileChanged(gh, "^docs/"),
+			thenR: r.Or(
+				r.And(
+					r.AuthorInTeam(gh, "devrels"),
+					r.ReviewByTeamMembers(gh, "tech-staff", 1),
+				),
+				r.And(
+					r.AuthorInTeam(gh, "tech-staff"),
+					r.ReviewByTeamMembers(gh, "devrels", 1),
+				),
+			),
+		},
 	}
 
 	manual := []manualCheck{
 		{
-			description: "Determine if infra needs to be updated",
-			ifC: c.And(
-				c.BaseBranch("master"),
-				c.Or(
-					c.FileChanged(gh, "misc/deployments"),
-					c.FileChanged(gh, `misc/docker-\.*`),
-					c.FileChanged(gh, "tm2/pkg/p2p"),
-				),
-			),
-			teams: []string{"tech-staff"},
+			description: "The pull request description provides enough details",
+			ifC:         c.Not(c.AuthorInTeam(gh, "core-contributors")),
+			teams:       Teams{"core-contributors"},
 		},
 		{
-			description: "Ensure the code style is satisfactory",
+			description: "Determine if infra needs to be updated before merging",
 			ifC: c.And(
 				c.BaseBranch("master"),
 				c.Or(
-					c.FileChanged(gh, `.*\.go`),
-					c.FileChanged(gh, `.*\.js`),
+					c.FileChanged(gh, `Dockerfile`),
+					c.FileChanged(gh, `^misc/deployments`),
+					c.FileChanged(gh, `^misc/docker-`),
+					c.FileChanged(gh, `^.github/workflows/releaser.*\.yml$`),
+					c.FileChanged(gh, `^.github/workflows/portal-loop\.yml$`),
 				),
 			),
-			teams: []string{"tech-staff"},
-		},
-		{
-			description: "Ensure the documentation is accurate and relevant",
-			ifC:         c.FileChanged(gh, `.*\.md`),
-			teams: []string{
-				"tech-staff",
-				"devrels",
-			},
+			teams: Teams{"devops"},
 		},
 	}
 

contribs/github-bot/internal/client/client.go

@@ -80,7 +80,7 @@ func (gh *GitHub) GetBotComment(prNum int) (*github.IssueComment, error) {
 		opts.Page = response.NextPage
 	}
 
-	return nil, errors.New("bot comment not found")
+	return nil, ErrBotCommentNotFound
 }
 
 // SetBotComment creates a bot's comment on the provided PR number

contribs/github-bot/internal/conditions/branch_test.go

@@ -22,9 +22,9 @@ func TestHeadBaseBranch(t *testing.T) {
 	}{
 		{"perfectly match", "base", "base", true},
 		{"prefix match", "^dev/", "dev/test-bot", true},
-		{"prefix doesn't match", "dev/$", "dev/test-bot", false},
+		{"prefix doesn't match", "^/test-bot", "dev/test-bot", false},
 		{"suffix match", "/test-bot$", "dev/test-bot", true},
-		{"suffix doesn't match", "^/test-bot", "dev/test-bot", false},
+		{"suffix doesn't match", "dev/$", "dev/test-bot", false},
 		{"doesn't match", "base", "notatall", false},
 	} {
 		t.Run(testCase.name, func(t *testing.T) {

contribs/github-bot/internal/conditions/draft.go

@@ -0,0 +1,21 @@
+package conditions
+
+import (
+	"github.com/gnolang/gno/contribs/github-bot/internal/utils"
+
+	"github.com/google/go-github/v64/github"
+	"github.com/xlab/treeprint"
+)
+
+// Draft Condition.
+type draft struct{}
+
+var _ Condition = &baseBranch{}
+
+func (*draft) IsMet(pr *github.PullRequest, details treeprint.Tree) bool {
+	return utils.AddStatusNode(pr.GetDraft(), "This pull request is a draft", details)
+}
+
+func Draft() Condition {
+	return &draft{}
+}

contribs/github-bot/internal/conditions/draft_test.go

@@ -0,0 +1,34 @@
+package conditions
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/gnolang/gno/contribs/github-bot/internal/utils"
+	"github.com/google/go-github/v64/github"
+	"github.com/stretchr/testify/assert"
+	"github.com/xlab/treeprint"
+)
+
+func TestDraft(t *testing.T) {
+	t.Parallel()
+
+	for _, testCase := range []struct {
+		name  string
+		isMet bool
+	}{
+		{"draft is true", true},
+		{"draft is false", false},
+	} {
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+
+			pr := &github.PullRequest{Draft: &testCase.isMet}
+			details := treeprint.New()
+			condition := Draft()
+
+			assert.Equal(t, condition.IsMet(pr, details), testCase.isMet, fmt.Sprintf("condition should have a met status: %t", testCase.isMet))
+			assert.True(t, utils.TestLastNodeStatus(t, testCase.isMet, details), fmt.Sprintf("condition details should have a status: %t", testCase.isMet))
+		})
+	}
+}

contribs/github-bot/internal/conditions/file_test.go

@@ -33,9 +33,9 @@ func TestFileChanged(t *testing.T) {
 		{"empty file list", "foo", []*github.CommitFile{}, false},
 		{"file list contains exact match", "foo", filenames, true},
 		{"file list contains prefix match", "^fo", filenames, true},
-		{"file list contains prefix doesn't match", "fo$", filenames, false},
+		{"file list contains prefix doesn't match", "^oo", filenames, false},
 		{"file list contains suffix match", "oo$", filenames, true},
-		{"file list contains suffix doesn't match", "^oo", filenames, false},
+		{"file list contains suffix doesn't match", "fo$", filenames, false},
 		{"file list doesn't contains match", "foobar", filenames, false},
 	} {
 		t.Run(testCase.name, func(t *testing.T) {

contribs/github-bot/internal/conditions/label_test.go

@@ -29,9 +29,9 @@ func TestLabel(t *testing.T) {
 		{"empty label list", "label", []*github.Label{}, false},
 		{"label list contains exact match", "label", labels, true},
 		{"label list contains prefix match", "^lab", labels, true},
-		{"label list contains prefix doesn't match", "lab$", labels, false},
+		{"label list contains prefix doesn't match", "^bel", labels, false},
 		{"label list contains suffix match", "bel$", labels, true},
-		{"label list contains suffix doesn't match", "^bel", labels, false},
+		{"label list contains suffix doesn't match", "lab$", labels, false},
 		{"label list doesn't contains match", "baleb", labels, false},
 	} {
 		t.Run(testCase.name, func(t *testing.T) {

contribs/github-bot/internal/params/prlist.go

@@ -23,7 +23,7 @@ func (p PRList) MarshalText() (text []byte, err error) {
 		prNumsStr[i] = strconv.Itoa(prNum)
 	}
 
-	return []byte(strings.Join(prNumsStr, ",")), nil
+	return []byte(strings.Join(prNumsStr, ", ")), nil
 }
 
 // UnmarshalText implements encoding.TextUnmarshaler.

contribs/github-bot/internal/requirements/assignee_test.go

@@ -64,9 +64,9 @@ func TestAssignee(t *testing.T) {
 			details := treeprint.New()
 			requirement := Assignee(gh, testCase.user)
 
-			assert.False(t, !requirement.IsSatisfied(pr, details) && !testCase.dryRun, "requirement should have a satisfied status: true")
-			assert.False(t, !utils.TestLastNodeStatus(t, true, details) && !testCase.dryRun, "requirement details should have a status: true")
-			assert.False(t, !testCase.exists && !requested && !testCase.dryRun, "requirement should have requested to create item")
+			assert.True(t, requirement.IsSatisfied(pr, details) || testCase.dryRun, "requirement should have a satisfied status: true")
+			assert.True(t, utils.TestLastNodeStatus(t, true, details) || testCase.dryRun, "requirement details should have a status: true")
+			assert.True(t, testCase.exists || requested || testCase.dryRun, "requirement should have requested to create item")
 		})
 	}
 }

contribs/github-bot/internal/requirements/branch.go

@@ -27,7 +27,12 @@ func (u *upToDateWith) IsSatisfied(pr *github.PullRequest, details treeprint.Tre
 	if u.base == PR_BASE {
 		base = pr.GetBase().GetRef()
 	}
+
 	head := pr.GetHead().GetRef()
+	// If pull request is open from a fork, prepend head ref with fork owner login
+	if pr.GetHead().GetRepo().GetFullName() != pr.GetBase().GetRepo().GetFullName() {
+		head = fmt.Sprintf("%s:%s", pr.GetHead().GetRepo().GetOwner().GetLogin(), pr.GetHead().GetRef())
+	}
 
 	cmp, _, err := u.gh.Client.Repositories.CompareCommits(u.gh.Ctx, u.gh.Owner, u.gh.Repo, base, head, nil)
 	if err != nil {

contribs/github-bot/internal/requirements/label_test.go

@@ -32,17 +32,10 @@ func TestLabel(t *testing.T) {
 		exists  bool
 	}{
 		{"empty label list", "label", []*github.Label{}, false, false},
-		{"empty label list with dry-run", "label", []*github.Label{}, true, false},
-		{"label list contains exact match", "label", labels, false, true},
-		{"label list contains prefix match", "^lab", labels, false, true},
-		{"label list contains prefix doesn't match", "lab$", labels, false, false},
-		{"label list contains prefix doesn't match with dry-run", "lab$", labels, true, false},
-		{"label list contains suffix match", "bel$", labels, false, true},
-		{"label list contains suffix match with dry-run", "bel$", labels, true, true},
-		{"label list contains suffix doesn't match", "^bel", labels, false, false},
-		{"label list contains suffix doesn't match with dry-run", "^bel", labels, true, false},
-		{"label list doesn't contains match", "baleb", labels, false, false},
-		{"label list doesn't contains match with dry-run", "baleb", labels, true, false},
+		{"empty label list with dry-run", "user", []*github.Label{}, true, false},
+		{"label list contains label", "label", labels, false, true},
+		{"label list doesn't contain label", "label2", labels, false, false},
+		{"label list doesn't contain label with dry-run", "label", labels, true, false},
 	} {
 		t.Run(testCase.name, func(t *testing.T) {
 			t.Parallel()
@@ -71,9 +64,9 @@ func TestLabel(t *testing.T) {
 			details := treeprint.New()
 			requirement := Label(gh, testCase.pattern)
 
-			assert.False(t, !requirement.IsSatisfied(pr, details) && !testCase.dryRun, "requirement should have a satisfied status: true")
-			assert.False(t, !utils.TestLastNodeStatus(t, true, details) && !testCase.dryRun, "requirement details should have a status: true")
-			assert.False(t, !testCase.exists && !requested && !testCase.dryRun, "requirement should have requested to create item")
+			assert.True(t, requirement.IsSatisfied(pr, details) || testCase.dryRun, "requirement should have a satisfied status: true")
+			assert.True(t, utils.TestLastNodeStatus(t, true, details) || testCase.dryRun, "requirement details should have a status: true")
+			assert.True(t, testCase.exists || requested || testCase.dryRun, "requirement should have requested to create item")
 		})
 	}
 }

contribs/github-bot/matrix.go

@@ -48,7 +48,13 @@ func execMatrix() error {
 		return err
 	}
 
-	fmt.Println(prList)
+	// Print PR list for GitHub Actions matrix definition
+	bytes, err := prList.MarshalText()
+	if err != nil {
+		return fmt.Errorf("unable to marshal PR list: %w", err)
+	}
+	fmt.Printf("[%s]", string(bytes))
+
 	return nil
 }