Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/queries_issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ func IssueCreate(client *Client, repo *Repository, params map[string]interface{}
}
issue := &result.CreateIssue.Issue

// Assign users using login-based mutation when ActorAssignees is true (github.com).
// Assign users using login-based mutation when ApiActorsSupported is true (github.com).
if assigneeLogins, ok := params["assigneeLogins"].([]string); ok && len(assigneeLogins) > 0 {
err := ReplaceActorsForAssignableByLogin(client, repo, issue.ID, assigneeLogins)
if err != nil {
Expand Down
10 changes: 8 additions & 2 deletions api/queries_pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -524,15 +524,15 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter
}
}

// Assign users using login-based mutation when ActorAssignees is true (github.com).
// Assign users using login-based mutation when ApiActorsSupported is true (github.com).
if assigneeLogins, ok := params["assigneeLogins"].([]string); ok && len(assigneeLogins) > 0 {
err := ReplaceActorsForAssignableByLogin(client, repo, pr.ID, assigneeLogins)
if err != nil {
return pr, err
}
}

// TODO requestReviewsByLoginCleanup
// TODO ApiActorsSupported
// Request reviewers using either login-based (github.com) or ID-based (GHES) mutation.
// The ID-based path can be removed once GHES supports requestReviewsByLogin.
userLogins, hasUserLogins := params["userReviewerLogins"].([]string)
Expand Down Expand Up @@ -599,6 +599,12 @@ func ReplaceActorsForAssignableByLogin(client *Client, repo ghrepo.Interface, as

actorLogins := make([]githubv4.String, len(logins))
for i, l := range logins {
// The replaceActorsForAssignable mutation requires the [bot] suffix
// for bot actor logins (e.g. "copilot-swe-agent[bot]"), unlike
// requestReviewsByLogin which has a separate botLogins field.
if l == CopilotAssigneeLogin {
l = l + "[bot]"
}
actorLogins[i] = githubv4.String(l)
}

Expand Down
20 changes: 11 additions & 9 deletions api/queries_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -922,14 +922,15 @@ func (m *RepoMetadataResult) Merge(m2 *RepoMetadataResult) {
}

type RepoMetadataInput struct {
Assignees bool
ActorAssignees bool
Reviewers bool
TeamReviewers bool
Labels bool
ProjectsV1 bool
ProjectsV2 bool
Milestones bool
Assignees bool
Reviewers bool
TeamReviewers bool
// TODO ApiActorsSupported
ApiActorsSupported bool
Labels bool
ProjectsV1 bool
ProjectsV2 bool
Milestones bool
}

// RepoMetadata pre-fetches the metadata for attaching to issues and pull requests
Expand All @@ -938,7 +939,8 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput
var g errgroup.Group

if input.Assignees || input.Reviewers {
if input.ActorAssignees {
// TODO ApiActorsSupported
if input.ApiActorsSupported {
g.Go(func() error {
actors, err := RepoAssignableActors(client, repo)
if err != nil {
Expand Down
30 changes: 27 additions & 3 deletions internal/featuredetection/feature_detection.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,35 @@ type Detector interface {
}

type IssueFeatures struct {
ActorIsAssignable bool
// TODO ApiActorsSupported

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great comment.

// ApiActorsSupported indicates the host supports actor-based APIs. True for
// github.com and ghe.com, false for GHES.
//
// The GitHub API has two generations of assignee/reviewer types:
//
// Legacy (GHES): Uses AssignableUser (users only) and node-ID-based mutations.
// - assignableUsers query returns []AssignableUser
// - Mutations take node IDs (assigneeIds, userReviewerIds, teamReviewerIds)
//
// Actor-based (github.com): Uses AssignableActor (User + Bot union) and
// login-based mutations, enabling assignment of non-user actors like Copilot.
// - suggestedActors query returns []AssignableActor (User | Bot)
// - suggestedReviewerActors returns []ReviewerCandidate (User | Bot | Team)
// - Mutations take logins (replaceActorsForAssignable, requestReviewsByLogin)
//
// When GHES adds support for the actor-based types and mutations, this flag
// can be removed and all // TODO ApiActorsSupported sites collapsed to the
// actor-only path. To verify GHES support, check whether the GHES GraphQL
// schema includes:
// - The suggestedActors field on Repository (assignee search)
// - The suggestedReviewerActors field on PullRequest (reviewer search)
// - The replaceActorsForAssignable mutation
// - The requestReviewsByLogin mutation
ApiActorsSupported bool
}

var allIssueFeatures = IssueFeatures{
ActorIsAssignable: true,
ApiActorsSupported: true,
}

type PullRequestFeatures struct {
Expand Down Expand Up @@ -136,7 +160,7 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) {
}

return IssueFeatures{
ActorIsAssignable: false, // replaceActorsForAssignable GraphQL mutation unavailable on GHES
ApiActorsSupported: false, // TODO ApiActorsSupported — actor-based mutations unavailable on GHES
}, nil
}

Expand Down
6 changes: 3 additions & 3 deletions internal/featuredetection/feature_detection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,23 @@ func TestIssueFeatures(t *testing.T) {
name: "github.com",
hostname: "github.com",
wantFeatures: IssueFeatures{
ActorIsAssignable: true,
ApiActorsSupported: true,
},
wantErr: false,
},
{
name: "ghec data residency (ghe.com)",
hostname: "stampname.ghe.com",
wantFeatures: IssueFeatures{
ActorIsAssignable: true,
ApiActorsSupported: true,
},
wantErr: false,
},
{
name: "GHE",
hostname: "git.my.org",
wantFeatures: IssueFeatures{
ActorIsAssignable: false,
ApiActorsSupported: false,
},
wantErr: false,
},
Expand Down
20 changes: 10 additions & 10 deletions pkg/cmd/issue/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func createRun(opts *CreateOptions) (err error) {

// Replace special values in assignees
// For web mode, @copilot should be replaced by name; otherwise, login.
assigneeReplacer := prShared.NewSpecialAssigneeReplacer(apiClient, baseRepo.RepoHost(), issueFeatures.ActorIsAssignable, !opts.WebMode)
assigneeReplacer := prShared.NewSpecialAssigneeReplacer(apiClient, baseRepo.RepoHost(), issueFeatures.ApiActorsSupported, !opts.WebMode)
assignees, err := assigneeReplacer.ReplaceSlice(opts.Assignees)
if err != nil {
return err
Expand All @@ -188,14 +188,14 @@ func createRun(opts *CreateOptions) (err error) {
assigneeSet.AddValues(assignees)

tb := prShared.IssueMetadataState{
Type: prShared.IssueMetadata,
ActorAssignees: issueFeatures.ActorIsAssignable,
Assignees: assigneeSet.ToSlice(),
Labels: opts.Labels,
ProjectTitles: opts.Projects,
Milestones: milestones,
Title: opts.Title,
Body: opts.Body,
Type: prShared.IssueMetadata,
ApiActorsSupported: issueFeatures.ApiActorsSupported, // TODO ApiActorsSupported
Assignees: assigneeSet.ToSlice(),
Labels: opts.Labels,
ProjectTitles: opts.Projects,
Milestones: milestones,
Title: opts.Title,
Body: opts.Body,
}

if opts.RecoverFile != "" {
Expand Down Expand Up @@ -309,7 +309,7 @@ func createRun(opts *CreateOptions) (err error) {
State: &tb,
}
var assigneeSearchFunc func(string) prompter.MultiSelectSearchResult
if issueFeatures.ActorIsAssignable {
if issueFeatures.ApiActorsSupported {
assigneeSearchFunc = prShared.RepoAssigneeSearchFunc(apiClient, baseRepo)
}
err = prShared.MetadataSurvey(opts.Prompter, opts.IO, baseRepo, fetcher, &tb, projectsV1Support, nil, assigneeSearchFunc)
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/issue/create/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ func Test_createRun(t *testing.T) {
{ "data": { "replaceActorsForAssignable": { "__typename": "" } } }
`, func(inputs map[string]interface{}) {
assert.Equal(t, "ISSUEID", inputs["assignableId"])
assert.Equal(t, []interface{}{"copilot-swe-agent", "MonaLisa"}, inputs["actorLogins"])
assert.Equal(t, []interface{}{"copilot-swe-agent[bot]", "MonaLisa"}, inputs["actorLogins"])
}))
},
wantsStdout: "https://github.com/OWNER/REPO/issues/12\n",
Expand Down Expand Up @@ -1161,7 +1161,7 @@ func TestIssueCreate_AtCopilotAssignee(t *testing.T) {
{ "data": { "replaceActorsForAssignable": { "__typename": "" } } }
`, func(inputs map[string]interface{}) {
assert.Equal(t, "NEWISSUEID", inputs["assignableId"])
assert.Equal(t, []interface{}{"copilot-swe-agent"}, inputs["actorLogins"])
assert.Equal(t, []interface{}{"copilot-swe-agent[bot]"}, inputs["actorLogins"])
}))

output, err := runCommand(http, true, `-a @copilot -t hello -b "cash rules everything around me"`, nil)
Expand Down
13 changes: 7 additions & 6 deletions pkg/cmd/issue/edit/edit.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,9 @@ func editRun(opts *EditOptions) error {

lookupFields := []string{"id", "number", "title", "body", "url"}
if editable.Assignees.Edited {
// TODO actorIsAssignableCleanup
if issueFeatures.ActorIsAssignable {
editable.Assignees.ActorAssignees = true
// TODO ApiActorsSupported
if issueFeatures.ApiActorsSupported {
editable.ApiActorsSupported = true
lookupFields = append(lookupFields, "assignedActors")
} else {
lookupFields = append(lookupFields, "assignees")
Expand Down Expand Up @@ -249,9 +249,9 @@ func editRun(opts *EditOptions) error {
// Fetch editable shared fields once for all issues.
apiClient := api.NewClientFromHTTP(httpClient)

// Wire up search function for assignees when ActorIsAssignable is available.
// Wire up search function for assignees when ApiActorsSupported is available.
// Interactive mode only supports a single issue, so we use its ID for the search query.
if issueFeatures.ActorIsAssignable && opts.Interactive && len(issues) == 1 {
if issueFeatures.ApiActorsSupported && opts.Interactive && len(issues) == 1 {
editable.AssigneeSearchFunc = prShared.AssigneeSearchFunc(apiClient, baseRepo, issues[0].ID)
}

Expand Down Expand Up @@ -280,7 +280,8 @@ func editRun(opts *EditOptions) error {
editable.Body.Default = issue.Body
// We use Actors as the default assignees if Actors are assignable
// on this GitHub host.
if editable.Assignees.ActorAssignees {
// TODO ApiActorsSupported
if editable.ApiActorsSupported {
editable.Assignees.Default = issue.AssignedActors.DisplayNames()
editable.Assignees.DefaultLogins = issue.AssignedActors.Logins()
} else {
Expand Down
12 changes: 6 additions & 6 deletions pkg/cmd/issue/edit/edit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ func Test_editRun(t *testing.T) {
mockIssueProjectItemsGet(t, reg)
mockRepoMetadata(t, reg)
mockIssueUpdate(t, reg)
mockIssueUpdateActorAssignees(t, reg)
mockIssueUpdateApiActors(t, reg)
mockIssueUpdateLabels(t, reg)
mockProjectV2ItemUpdate(t, reg)
},
Expand Down Expand Up @@ -444,8 +444,8 @@ func Test_editRun(t *testing.T) {
mockIssueProjectItemsGet(t, reg)
mockIssueUpdate(t, reg)
mockIssueUpdate(t, reg)
mockIssueUpdateActorAssignees(t, reg)
mockIssueUpdateActorAssignees(t, reg)
mockIssueUpdateApiActors(t, reg)
mockIssueUpdateApiActors(t, reg)
mockIssueUpdateLabels(t, reg)
mockIssueUpdateLabels(t, reg)
mockProjectV2ItemUpdate(t, reg)
Expand Down Expand Up @@ -608,7 +608,7 @@ func Test_editRun(t *testing.T) {
mockIssueProjectItemsGet(t, reg)
mockRepoMetadata(t, reg)
mockIssueUpdate(t, reg)
mockIssueUpdateActorAssignees(t, reg)
mockIssueUpdateApiActors(t, reg)
mockIssueUpdateLabels(t, reg)
mockProjectV2ItemUpdate(t, reg)
},
Expand Down Expand Up @@ -902,7 +902,7 @@ func mockIssueUpdate(t *testing.T, reg *httpmock.Registry) {
)
}

func mockIssueUpdateActorAssignees(t *testing.T, reg *httpmock.Registry) {
func mockIssueUpdateApiActors(t *testing.T, reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`),
httpmock.GraphQLMutation(`
Expand Down Expand Up @@ -935,7 +935,7 @@ func mockProjectV2ItemUpdate(t *testing.T, reg *httpmock.Registry) {
)
}

func TestActorIsAssignable(t *testing.T) {
func TestApiActorsSupported(t *testing.T) {
t.Run("when actors are assignable, query includes assignedActors", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()

Expand Down
18 changes: 9 additions & 9 deletions pkg/cmd/pr/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,15 +399,15 @@ func createRun(opts *CreateOptions) error {

client := ctx.Client

// Detect ActorIsAssignable feature to determine if we can use search-based
// Detect ApiActorsSupported feature to determine if we can use search-based
// reviewer selection (github.com) or need to use legacy ID-based selection (GHES)
issueFeatures, err := opts.Detector.IssueFeatures()
if err != nil {
return err
}
var reviewerSearchFunc func(string) prompter.MultiSelectSearchResult
var assigneeSearchFunc func(string) prompter.MultiSelectSearchResult
if issueFeatures.ActorIsAssignable {
if issueFeatures.ApiActorsSupported {
reviewerSearchFunc = func(query string) prompter.MultiSelectSearchResult {
candidates, moreResults, err := api.SuggestedReviewerActorsForRepo(client, ctx.PRRefs.BaseRepo(), query)
if err != nil {
Expand All @@ -424,14 +424,14 @@ func createRun(opts *CreateOptions) error {
assigneeSearchFunc = shared.RepoAssigneeSearchFunc(client, ctx.PRRefs.BaseRepo())
}

state, err := NewIssueState(*ctx, *opts)
state, err := NewIssueState(*ctx, *opts, issueFeatures.ApiActorsSupported)
if err != nil {
return err
}

if issueFeatures.ActorIsAssignable {
state.ActorReviewers = true
state.ActorAssignees = true
// TODO ApiActorsSupported
if issueFeatures.ApiActorsSupported {
state.ApiActorsSupported = true
}

var openURL string
Expand Down Expand Up @@ -672,14 +672,14 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
return nil
}

func NewIssueState(ctx CreateContext, opts CreateOptions) (*shared.IssueMetadataState, error) {
func NewIssueState(ctx CreateContext, opts CreateOptions, apiActorsSupported bool) (*shared.IssueMetadataState, error) {
var milestoneTitles []string
if opts.Milestone != "" {
milestoneTitles = []string{opts.Milestone}
}

meReplacer := shared.NewMeReplacer(ctx.Client, ctx.PRRefs.BaseRepo().RepoHost())
assignees, err := meReplacer.ReplaceSlice(opts.Assignees)
assigneeReplacer := shared.NewSpecialAssigneeReplacer(ctx.Client, ctx.PRRefs.BaseRepo().RepoHost(), apiActorsSupported, !opts.WebMode)
assignees, err := assigneeReplacer.ReplaceSlice(opts.Assignees)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/pr/create/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,7 @@ func Test_createRun(t *testing.T) {
`, func(inputs map[string]interface{}) {
assert.Equal(t, "NEWPULLID", inputs["pullRequestId"])
if _, ok := inputs["assigneeIds"]; ok {
t.Error("did not expect assigneeIds in updatePullRequest when ActorAssignees is true")
t.Error("did not expect assigneeIds in updatePullRequest when ApiActorsSupported is true")
}
assert.Equal(t, []interface{}{"BUGID", "TODOID"}, inputs["labelIds"])
assert.Equal(t, []interface{}{"ROADMAPID"}, inputs["projectIds"])
Expand Down
Loading
Loading