This is an automated email from the ASF dual-hosted git repository.
klesh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/devlake.git
The following commit(s) were added to refs/heads/main by this push:
new 4bc679efd fix(github_graphql): resolve Bot actors so bot-authored PRs
keep their author (#9086)
4bc679efd is described below
commit 4bc679efdd332f9a3d1a1e8afb7476d6a4fa9f06
Author: Nikita Bolotov <[email protected]>
AuthorDate: Tue Sep 1 06:53:29 2026 +0300
fix(github_graphql): resolve Bot actors so bot-authored PRs keep their
author (#9086)
`author`, `mergedBy` and review authors are typed `Actor` in the GitHub
schema, which `Bot` implements alongside `User`. The inline query spread
only `... on User`, so every pull request, issue and review opened by a
GitHub App, Dependabot or Renovate came back with an empty selection set
and was stored with no author at all.
`... on Bot` cannot simply be added to the existing query: GitHub rejects
the whole request with "Fragment on Bot can't be spread inside User" when
it is spread on a `User`-typed field such as assignees or a commit author.
Actor fields therefore get their own GraphqlInlineActorQuery, and both
types resolve through an Account() accessor.
Bot logins are normalized to the `[bot]` suffix the REST collector stores,
so an account keeps one identity across collectors, and the accounts
lookup skips bot logins because `user(login:)` never resolves them.
Fixes #9085
Co-authored-by: Claude Opus 5 <[email protected]>
---
.../github_graphql/tasks/account_collector.go | 9 +-
.../tasks/account_graphql_pre_extractor.go | 73 ++++++++++++-
.../tasks/account_graphql_pre_extractor_test.go | 121 +++++++++++++++++++++
.../github_graphql/tasks/issue_collector.go | 2 +-
.../github_graphql/tasks/issue_extractor.go | 10 +-
.../plugins/github_graphql/tasks/pr_collector.go | 6 +-
.../plugins/github_graphql/tasks/pr_extractor.go | 28 ++---
7 files changed, 221 insertions(+), 28 deletions(-)
diff --git a/backend/plugins/github_graphql/tasks/account_collector.go
b/backend/plugins/github_graphql/tasks/account_collector.go
index a75013d0d..37ecb9716 100644
--- a/backend/plugins/github_graphql/tasks/account_collector.go
+++ b/backend/plugins/github_graphql/tasks/account_collector.go
@@ -75,10 +75,17 @@ func CollectAccount(taskCtx plugin.SubTaskContext)
errors.Error {
db := taskCtx.GetDal()
data := taskCtx.GetData().(*githubTasks.GithubTaskData)
+ // `user(login:)` resolves Users only, a bot login always comes back as
+ // "Could not resolve to a User", so bots are left out of this lookup.
They are
+ // still converted to accounts: ConvertAccounts reads
_tool_github_repo_accounts
+ // and only enriches it from _tool_github_accounts when a row is there.
cursor, err := db.Cursor(
dal.Select("login"),
dal.From(models.GithubRepoAccount{}.TableName()),
- dal.Where("repo_github_id = ? and connection_id=?",
data.Options.GithubId, data.Options.ConnectionId),
+ dal.Where(
+ "repo_github_id = ? and connection_id = ? and login not
like ?",
+ data.Options.GithubId, data.Options.ConnectionId,
"%"+botLoginSuffix,
+ ),
)
if err != nil {
return err
diff --git
a/backend/plugins/github_graphql/tasks/account_graphql_pre_extractor.go
b/backend/plugins/github_graphql/tasks/account_graphql_pre_extractor.go
index e4b47e475..54bc44a70 100644
--- a/backend/plugins/github_graphql/tasks/account_graphql_pre_extractor.go
+++ b/backend/plugins/github_graphql/tasks/account_graphql_pre_extractor.go
@@ -18,9 +18,16 @@ limitations under the License.
package tasks
import (
+ "strings"
+
"github.com/apache/incubator-devlake/plugins/github/models"
)
+// botLoginSuffix is how the REST API reports a bot login: `dependabot[bot]`.
+// GraphQL's `Bot.login` omits it, so it is appended back to give a bot the
same
+// identity no matter which collector produced it.
+const botLoginSuffix = `[bot]`
+
type GithubAccountEdge struct {
Login string
Id int `graphql:"databaseId"`
@@ -30,18 +37,76 @@ type GithubAccountEdge struct {
AvatarUrl string
HtmlUrl string `graphql:"url"`
}
+
+// GithubBotEdge holds what a `Bot` actor exposes. The GitHub schema gives
`Bot`
+// a narrower field set than `User`: no name, company or email.
+type GithubBotEdge struct {
+ Login string
+ Id int `graphql:"databaseId"`
+ AvatarUrl string
+ HtmlUrl string `graphql:"url"`
+}
+
+// GraphqlInlineAccountQuery is for fields the schema types as `User`, such as
+// assignees and commit authors.
type GraphqlInlineAccountQuery struct {
GithubAccountEdge `graphql:"... on User"`
}
-func extractGraphqlPreAccount(result *[]interface{}, res
*GraphqlInlineAccountQuery, repoId int, connId uint64) {
- if res == nil || res.Id == 0 {
+// Account returns the account this query resolved to, or a zero value.
+func (q *GraphqlInlineAccountQuery) Account() GithubAccountEdge {
+ if q == nil {
+ return GithubAccountEdge{}
+ }
+ return q.GithubAccountEdge
+}
+
+// GraphqlInlineActorQuery is for fields the schema types as `Actor`, such as a
+// pull request author or the user who merged it. `Actor` is implemented by
+// `User` and `Bot` among others, so spreading only `... on User` leaves every
+// GitHub App, Dependabot or Renovate author empty.
+//
+// The two inline queries cannot be merged into one: spreading `... on Bot` on
a
+// `User`-typed field makes GitHub reject the whole query with
+// "Fragment on Bot can't be spread inside User".
+type GraphqlInlineActorQuery struct {
+ GithubAccountEdge `graphql:"... on User"`
+ Bot GithubBotEdge `graphql:"... on Bot"`
+}
+
+// Account returns the actor as an account, whichever type it resolved to.
+func (q *GraphqlInlineActorQuery) Account() GithubAccountEdge {
+ if q == nil {
+ return GithubAccountEdge{}
+ }
+ if q.GithubAccountEdge.Id != 0 {
+ return q.GithubAccountEdge
+ }
+ if q.Bot.Id == 0 {
+ return GithubAccountEdge{}
+ }
+ login := q.Bot.Login
+ if !strings.HasSuffix(login, botLoginSuffix) {
+ login += botLoginSuffix
+ }
+ // Name, Company and Email stay empty: that is what the REST collector
stores
+ // for a bot as well.
+ return GithubAccountEdge{
+ Login: login,
+ Id: q.Bot.Id,
+ AvatarUrl: q.Bot.AvatarUrl,
+ HtmlUrl: q.Bot.HtmlUrl,
+ }
+}
+
+func extractGraphqlPreAccount(result *[]interface{}, account
GithubAccountEdge, repoId int, connId uint64) {
+ if account.Id == 0 {
return
}
*result = append(*result, &models.GithubRepoAccount{
ConnectionId: connId,
RepoGithubId: repoId,
- Login: res.Login,
- AccountId: res.Id,
+ Login: account.Login,
+ AccountId: account.Id,
})
}
diff --git
a/backend/plugins/github_graphql/tasks/account_graphql_pre_extractor_test.go
b/backend/plugins/github_graphql/tasks/account_graphql_pre_extractor_test.go
new file mode 100644
index 000000000..4979e5523
--- /dev/null
+++ b/backend/plugins/github_graphql/tasks/account_graphql_pre_extractor_test.go
@@ -0,0 +1,121 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package tasks
+
+import (
+ "encoding/json"
+ "strings"
+ "testing"
+
+ "github.com/merico-ai/graphql"
+ "github.com/stretchr/testify/assert"
+)
+
+func TestInlineAccountQueryAccount(t *testing.T) {
+ var nilQuery *GraphqlInlineAccountQuery
+ assert.Equal(t, 0, nilQuery.Account().Id)
+
+ user := &GraphqlInlineAccountQuery{
+ GithubAccountEdge: GithubAccountEdge{Login: `octocat`, Id:
583231, Name: `The Octocat`},
+ }
+ assert.Equal(t, `octocat`, user.Account().Login)
+ assert.Equal(t, 583231, user.Account().Id)
+}
+
+func TestInlineActorQueryAccount(t *testing.T) {
+ var nilQuery *GraphqlInlineActorQuery
+ assert.Equal(t, GithubAccountEdge{}, nilQuery.Account())
+
+ // an actor that resolved to neither User nor Bot, e.g. a deleted
account
+ empty := &GraphqlInlineActorQuery{}
+ assert.Equal(t, GithubAccountEdge{}, empty.Account())
+
+ user := &GraphqlInlineActorQuery{
+ GithubAccountEdge: GithubAccountEdge{Login: `octocat`, Id:
583231, Email: `[email protected]`},
+ }
+ assert.Equal(t, `octocat`, user.Account().Login)
+ assert.Equal(t, `[email protected]`, user.Account().Email)
+
+ // Bot.login has no `[bot]` suffix, the REST collector stores one
+ bot := &GraphqlInlineActorQuery{
+ Bot: GithubBotEdge{Login: `dependabot`, Id: 49699333, HtmlUrl:
`https://github.com/apps/dependabot`},
+ }
+ assert.Equal(t, `dependabot[bot]`, bot.Account().Login)
+ assert.Equal(t, 49699333, bot.Account().Id)
+ assert.Equal(t, `https://github.com/apps/dependabot`,
bot.Account().HtmlUrl)
+ assert.Empty(t, bot.Account().Name)
+
+ // a login that already carries the suffix is left alone
+ suffixed := &GraphqlInlineActorQuery{
+ Bot: GithubBotEdge{Login: `renovate[bot]`, Id: 29139614},
+ }
+ assert.Equal(t, `renovate[bot]`, suffixed.Account().Login)
+}
+
+// the raw layer holds the marshalled query result, so a bot author has to
survive
+// the round trip the extractors read it back through
+func TestInlineActorQueryUnmarshalBotAuthor(t *testing.T) {
+ raw :=
`{"Login":"","Id":0,"Name":"","Company":"","Email":"","AvatarUrl":"","HtmlUrl":"",
+
"Bot":{"Login":"dependabot","Id":49699333,"AvatarUrl":"","HtmlUrl":""}}`
+ actor := &GraphqlInlineActorQuery{}
+ assert.Nil(t, json.Unmarshal([]byte(raw), actor))
+ assert.Equal(t, `dependabot[bot]`, actor.Account().Login)
+ assert.Equal(t, 49699333, actor.Account().Id)
+
+ // rows collected before the Bot fragment existed still extract as
before
+ legacy := `{"Login":"octocat","Id":583231,"Name":"The
Octocat","Company":"","Email":"","AvatarUrl":"","HtmlUrl":""}`
+ actor = &GraphqlInlineActorQuery{}
+ assert.Nil(t, json.Unmarshal([]byte(legacy), actor))
+ assert.Equal(t, `octocat`, actor.Account().Login)
+}
+
+func TestExtractGraphqlPreAccountSkipsUnresolvedActors(t *testing.T) {
+ var results []interface{}
+ var missing *GraphqlInlineActorQuery
+ extractGraphqlPreAccount(&results, missing.Account(), 1, 1)
+ assert.Empty(t, results)
+
+ bot := &GraphqlInlineActorQuery{Bot: GithubBotEdge{Login: `dependabot`,
Id: 49699333}}
+ extractGraphqlPreAccount(&results, bot.Account(), 1, 1)
+ assert.Len(t, results, 1)
+}
+
+// `... on Bot` may only be spread where the schema says `Actor`; on a `User`
field
+// GitHub rejects the whole query with "Fragment on Bot can't be spread inside
User"
+func TestBotFragmentOnlySpreadOnActorFields(t *testing.T) {
+ prQuery, _ := graphql.ConstructQuery(&GraphqlQueryPrWrapper{},
map[string]interface{}{})
+ issueQuery, _ := graphql.ConstructQuery(&GraphqlQueryIssueWrapper{},
map[string]interface{}{})
+
+ assert.Contains(t, prQuery, `author{... on User{`)
+ assert.Contains(t, prQuery, `... on Bot{`)
+ assert.Contains(t, issueQuery, `... on Bot{`)
+
+ // assignees and commit authors are typed `User`, they must stay
User-only
+ for _, userField := range []string{`assignees(first: 1){nodes{`,
`assignees(first: 100){nodes{`} {
+ if idx := strings.Index(prQuery+issueQuery, userField); idx >=
0 {
+ tail := (prQuery + issueQuery)[idx : idx+120]
+ assert.NotContains(t, tail, `... on Bot`)
+ }
+ }
+ commitAuthor := `commits(first: 100){`
+ if idx := strings.Index(prQuery, commitAuthor); idx >= 0 {
+ tail := prQuery[idx : idx+400]
+ assert.Contains(t, tail, `user{... on User{`)
+ assert.NotContains(t, tail, `user{... on
User{login,databaseId,name,company,email,avatarUrl,url},... on Bot`)
+ }
+}
diff --git a/backend/plugins/github_graphql/tasks/issue_collector.go
b/backend/plugins/github_graphql/tasks/issue_collector.go
index 36c33584c..9069a1c72 100644
--- a/backend/plugins/github_graphql/tasks/issue_collector.go
+++ b/backend/plugins/github_graphql/tasks/issue_collector.go
@@ -68,7 +68,7 @@ type GraphqlQueryIssue struct {
StateReason string
Title string
Body string
- Author *GraphqlInlineAccountQuery
+ Author *GraphqlInlineActorQuery
Url string
ClosedAt *time.Time
CreatedAt time.Time
diff --git a/backend/plugins/github_graphql/tasks/issue_extractor.go
b/backend/plugins/github_graphql/tasks/issue_extractor.go
index e27679c83..a825c6a89 100644
--- a/backend/plugins/github_graphql/tasks/issue_extractor.go
+++ b/backend/plugins/github_graphql/tasks/issue_extractor.go
@@ -83,9 +83,9 @@ func ExtractIssues(taskCtx plugin.SubTaskContext)
errors.Error {
results = append(results, githubLabels...)
results = append(results, githubIssue)
if len(issue.AssigneeList.Assignees) > 0 {
- extractGraphqlPreAccount(&results,
&issue.AssigneeList.Assignees[0], data.Options.GithubId,
data.Options.ConnectionId)
+ extractGraphqlPreAccount(&results,
issue.AssigneeList.Assignees[0].Account(), data.Options.GithubId,
data.Options.ConnectionId)
}
- extractGraphqlPreAccount(&results, issue.Author,
data.Options.GithubId, data.Options.ConnectionId)
+ extractGraphqlPreAccount(&results,
issue.Author.Account(), data.Options.GithubId, data.Options.ConnectionId)
for _, assignee := range issue.AssigneeList.Assignees {
issueAssignee := &models.GithubIssueAssignee{
ConnectionId: githubIssue.ConnectionId,
@@ -147,9 +147,9 @@ func convertGithubIssue(milestoneMap map[int]int, issue
*GraphqlQueryIssue, conn
githubIssue.AssigneeId = issue.AssigneeList.Assignees[0].Id
githubIssue.AssigneeName = issue.AssigneeList.Assignees[0].Login
}
- if issue.Author != nil {
- githubIssue.AuthorId = issue.Author.Id
- githubIssue.AuthorName = issue.Author.Login
+ if author := issue.Author.Account(); author.Id != 0 {
+ githubIssue.AuthorId = author.Id
+ githubIssue.AuthorName = author.Login
}
if issue.ClosedAt != nil {
temp := uint(issue.ClosedAt.Sub(issue.CreatedAt).Minutes())
diff --git a/backend/plugins/github_graphql/tasks/pr_collector.go
b/backend/plugins/github_graphql/tasks/pr_collector.go
index 669839385..c05cac2ef 100644
--- a/backend/plugins/github_graphql/tasks/pr_collector.go
+++ b/backend/plugins/github_graphql/tasks/pr_collector.go
@@ -72,7 +72,7 @@ type GraphqlQueryPr struct {
Name string
}
} `graphql:"labels(first: 100)"`
- Author *GraphqlInlineAccountQuery
+ Author *GraphqlInlineActorQuery
Assignees struct {
// FIXME now domain layer just support one assignee
Assignees []GraphqlInlineAccountQuery `graphql:"nodes"`
@@ -99,7 +99,7 @@ type GraphqlQueryPr struct {
} `graphql:"reviews(first: 100)"`
Additions int
Deletions int
- MergedBy *GraphqlInlineAccountQuery
+ MergedBy *GraphqlInlineActorQuery
ReviewRequests struct {
Nodes []ReviewRequestNode `graphql:"nodes"`
} `graphql:"reviewRequests(first: 10)"`
@@ -128,7 +128,7 @@ type Team struct {
type GraphqlQueryReview struct {
Body string
- Author *GraphqlInlineAccountQuery
+ Author *GraphqlInlineActorQuery
State string `json:"state"`
DatabaseId int `json:"databaseId"`
Commit struct {
diff --git a/backend/plugins/github_graphql/tasks/pr_extractor.go
b/backend/plugins/github_graphql/tasks/pr_extractor.go
index 59bc72cdb..8d6fd863b 100644
--- a/backend/plugins/github_graphql/tasks/pr_extractor.go
+++ b/backend/plugins/github_graphql/tasks/pr_extractor.go
@@ -78,7 +78,7 @@ func ExtractPrs(taskCtx plugin.SubTaskContext) errors.Error {
if err != nil {
return nil, err
}
- extractGraphqlPreAccount(&results, rawL.Author,
data.Options.GithubId, data.Options.ConnectionId)
+ extractGraphqlPreAccount(&results,
rawL.Author.Account(), data.Options.GithubId, data.Options.ConnectionId)
for _, label := range rawL.Labels.Nodes {
results = append(results, &models.GithubPrLabel{
ConnectionId: data.Options.ConnectionId,
@@ -110,10 +110,10 @@ func ExtractPrs(taskCtx plugin.SubTaskContext)
errors.Error {
PullRequestId:
githubPr.GithubId,
}
- if apiPullRequestReview.Author != nil {
- githubPrReview.AuthorUserId =
apiPullRequestReview.Author.Id
- githubPrReview.AuthorUsername =
apiPullRequestReview.Author.Login
-
extractGraphqlPreAccount(&results, apiPullRequestReview.Author,
data.Options.GithubId, data.Options.ConnectionId)
+ if author :=
apiPullRequestReview.Author.Account(); author.Id != 0 {
+ githubPrReview.AuthorUserId =
author.Id
+ githubPrReview.AuthorUsername =
author.Login
+
extractGraphqlPreAccount(&results, author, data.Options.GithubId,
data.Options.ConnectionId)
}
results = append(results,
githubPrReview)
@@ -145,7 +145,7 @@ func ExtractPrs(taskCtx plugin.SubTaskContext) errors.Error
{
CommitAuthoredDate:
githubCommit.AuthoredDate,
}
results = append(results,
githubPullRequestCommit)
- extractGraphqlPreAccount(&results,
apiPullRequestCommit.Commit.Author.User, data.Options.GithubId,
data.Options.ConnectionId)
+ extractGraphqlPreAccount(&results,
apiPullRequestCommit.Commit.Author.User.Account(), data.Options.GithubId,
data.Options.ConnectionId)
}
return results, nil
},
@@ -180,16 +180,16 @@ func convertGithubPullRequest(pull *GraphqlQueryPr,
connId uint64, repoId int) (
Deletions: pull.Deletions,
IsDraft: pull.IsDraft,
}
- if pull.MergedBy != nil {
- githubPull.MergedByName = pull.MergedBy.Login
- githubPull.MergedById = pull.MergedBy.Id
+ if mergedBy := pull.MergedBy.Account(); mergedBy.Id != 0 {
+ githubPull.MergedByName = mergedBy.Login
+ githubPull.MergedById = mergedBy.Id
}
if pull.MergeCommit != nil {
githubPull.MergeCommitSha = pull.MergeCommit.Oid
}
- if pull.Author != nil {
- githubPull.AuthorName = pull.Author.Login
- githubPull.AuthorId = pull.Author.Id
+ if author := pull.Author.Account(); author.Id != 0 {
+ githubPull.AuthorName = author.Login
+ githubPull.AuthorId = author.Id
}
return githubPull, nil
}
@@ -206,8 +206,8 @@ func convertPullRequestCommit(prCommit GraphqlQueryCommit)
(*models.GithubCommit
CommittedDate: prCommit.Commit.Committer.Date,
Url: prCommit.Url,
}
- if prCommit.Commit.Author.User != nil {
- githubCommit.AuthorId = prCommit.Commit.Author.User.Id
+ if user := prCommit.Commit.Author.User.Account(); user.Id != 0 {
+ githubCommit.AuthorId = user.Id
}
return githubCommit, nil
}
