Fix CODEOWNERS review request attribution using comment metadata

[bimakw]

Problem

When CODEOWNERS automatically assigns reviewers to a pull request, the timeline incorrectly shows the PR author as the one who requested the review (e.g., "PR_AUTHOR requested review from CODE_OWNER"). This is misleading since the action was triggered automatically by CODEOWNERS rules, not by the PR author.

Fixes #36333

Solution

Store CODEOWNERS attribution in comment metadata instead of changing the doer user:

• Add SpecialDoerName field to CommentMetaData struct (value: "CODEOWNERS" for CODEOWNERS-triggered requests)
• Pass isCodeOwners=true to AddReviewRequest and AddTeamReviewRequest functions
• Template can check this metadata to show appropriate attribution message

This approach:

• Keeps the actual doer (PR author) intact for permission/audit purposes
• Allows UI to display "CODEOWNERS requested review" when metadata is present
• More general design that can support other special doer names in the future

Changes

• models/issues/comment.go: Add SpecialDoerName field to CommentMetaData and CreateCommentOptions
• models/issues/review.go: Update AddReviewRequest and AddTeamReviewRequest to set metadata when isCodeOwners=true
• models/issues/review_test.go: Add test case for metadata verification

Testing

• go build passes
• go test passes for TestAddReviewRequest

[wxiaoguang]

It is an abuse of "Ghost" user. "Ghost" user means that there was a user, but it has been deleted.

[bimakw]

Makes sense. Was thinking of adding something like CodeOwnersUser (similar to ActionsUser), would that work?

[wxiaoguang]

Makes sense. Was thinking of adding something like CodeOwnersUser (similar to ActionsUser), would that work?

I believe it needs a general "gitea system account" at the moment (#30205 (comment))

Keeping adding new "system accounts" causes problems for end users who might already have users with the same names (#30205 (comment))

But other maintainers might not agree with me.

[bimakw]

Thanks for the feedback @wxiaoguang. I understand the concern about Ghost user semantics - you're right that Ghost user represents deleted accounts, not system automation.

Regarding the general system account approach, I looked at the discussion in #30205. As @lunny pointed out, system bot accounts with [bot] prefix and no profile page are visually distinct from regular users, so name collision shouldn't be an issue in practice.

I'd like to propose following the exact same pattern as ActionsUser:

const (
    CodeOwnersUserID    int64 = -3
    CodeOwnersUserName        = "gitea-codeowners"
    CodeOwnersUserEmail       = "codeowners@gitea.io"
)

func NewCodeOwnersUser() *User {
    return &User{
        ID:               CodeOwnersUserID,
        Name:             CodeOwnersUserName,
        LowerName:        CodeOwnersUserName,
        IsActive:         true,
        FullName:         "Gitea CODEOWNERS",
        Email:            CodeOwnersUserEmail,
        KeepEmailPrivate: true,
        LoginName:        CodeOwnersUserName,
        Type:             UserTypeBot,
        Visibility:       structs.VisibleTypePublic,
    }
}

Plus add "gitea-codeowners" to reservedUsernames.

This approach is:

• Consistent with existing patterns (ActionsUser)
• Semantically correct (CODEOWNERS attribution, not Actions)
• Minimal in scope

Would this approach work, or would you prefer to wait for the broader general system account discussion to conclude first?

[wxiaoguang]

so name collision shouldn't be an issue in practice.

It doesn't answer the question: what if the user's Gitea instance already has a user named gitea-codeowners

---

I am against for "keeping adding a lot of reserved usernames", the more you add, the more potential conflicts will be.

[wxiaoguang]

As @lunny pointed out, system bot accounts with [bot] prefix and no profile page are visually distinct from regular users, so name collision shouldn't be an issue in practice.

He doesn't answer the question either. As long as you added a new reserved username, there will be conflict. No matter [bot] prefix is added or not.

In practice, GitHub's "dependabot" occupies that name too.

[lunny]

As @lunny pointed out, system bot accounts with [bot] prefix and no profile page are visually distinct from regular users, so name collision shouldn't be an issue in practice.

He doesn't answer the question either. As long as you added a new reserved username, there will be conflict. No matter [bot] prefix is added or not.

In practice, GitHub's "dependabot" occupies that name too.

In GitHub’s implementation, https://github.com/dependabot is an organization, while the dependabot[bot] account is a separate entity that represents the GitHub App. These two are distinct and do not conflict with each other.

[wxiaoguang]

In GitHub’s implementation, https://github.com/dependabot is an organization, while the dependabot[bot] account is a separate entity that represents the GitHub App. These two are distinct and do not conflict with each other.

Because dependabot[bot] works for dependabot[org], literally they are the same thing for the same purpose.

But your design: it only introduces conflicts:

• User has its project-workflows user or org.
• Gitea has its project-workflows reserved bot name.

They are different things, and it only makes the system more messy when you introduce more potential conflicting names.

[wxiaoguang]

Again: OK, that's your choice to keep introducing fragile designs and confusing end users. If you believe it's right, I won't block.

You can use your power to push and merge.

[lunny]

In GitHub’s implementation, @dependabot is an organization, while the dependabot[bot] account is a separate entity that represents the GitHub App. These two are distinct and do not conflict with each other.

Because dependabot[bot] works for dependabot[org], literally they are the same thing for the same purpose.

But your design: it only introduces conflicts:

• User has its project-workflows user or org.
• Gitea has its project-workflows reserved bot name.

They are different things, and it only makes the system more messy when you introduce more potential conflicting names.

In GitHub’s implementation, the dependabot[bot] account is automatically created when the app is installed and it seems all APP instances created by the same APP share the same name. It is completely unrelated to the dependabot organization.

The dependabot organization is optional—users may choose to create it or not—and there is no functional relationship between the two beyond the similarity in their names.

I’m simply pointing out how GitHub has implemented this.

[wxiaoguang]

I’m simply pointing out how GitHub has implemented this.

What if a user would like to search the PRs created by the real "dependabot" user? And how to search the PRs created by "dependabot" bot?

GitHub never uses "dependabot" as the "user name"

[lunny]

I’m simply pointing out how GitHub has implemented this.

What if a user would like to search the PRs created by the real "dependabot" user? And how to search the PRs created by "dependabot" bot?

GitHub never uses "dependabot" as the "user name"

Github never use dependabot as a user name because dependabot was registered as an organization. If another APP's name was registered as a user name, there will be a confusing. There will be xxx [bot] and xxx at the same time and they are unrelated.

I guess there are two tables in Github's implementation, one is user, another is apps. All names in the apps have a label [bot] and names from user will not. And both kinds of users could be listed in the pull request authors list or other necessary places.

[wxiaoguang]

I guess there are two tables in Github's implementation ...

Please propose a complete and feasible design in Gitea, that's what I am always asking for.

And show how to search PRs for real "dependabot" user, search PRs for bot "dependabot" app.

[bimakw]

Replaced GhostUser with dedicated CodeOwnersUser following the same pattern as ActionsUser:

• ID: -3
• Name: gitea-codeowners
• Type: UserTypeBot

This addresses the concern that "Ghost user means deleted user" while maintaining proper semantic attribution for CODEOWNERS-triggered review requests.

Changes:

• Added CodeOwnersUser constants and functions in models/user/user_system.go
• Added to reservedUsernames and systemUserNewFuncs
• Updated services/issue/pull.go to use NewCodeOwnersUser()
• Updated template to check IsCodeOwners() instead of IsGhost()
• Updated tests

[bimakw]

@lunny Ready for review when you have time.

[lunny]

I don’t think we need a system bot user here. It’s only used for comments and doesn’t involve any permission design. The comment itself can simply be adjusted to something like:
xxx was requested to review due to changes in xxx file.

[wxiaoguang]

Oh no, please don't force push. https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md#maintaining-open-prs

I made many necessary changes, please revert to my commits