Skip to content

Commit 8f7d208

Browse files
authored
Merge pull request cli#13025 from cli/kw/refactor/reviewer-assignee-actor-symmetry
Consolidate actor-mode signals into ApiActorsSupported
2 parents 9b4a989 + 391e661 commit 8f7d208

18 files changed

Lines changed: 178 additions & 121 deletions

api/queries_issue.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ func IssueCreate(client *Client, repo *Repository, params map[string]interface{}
311311
}
312312
issue := &result.CreateIssue.Issue
313313

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

api/queries_pr.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -524,15 +524,15 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter
524524
}
525525
}
526526

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

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

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

api/queries_repo.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -963,14 +963,15 @@ func (m *RepoMetadataResult) Merge(m2 *RepoMetadataResult) {
963963
}
964964

965965
type RepoMetadataInput struct {
966-
Assignees bool
967-
ActorAssignees bool
968-
Reviewers bool
969-
TeamReviewers bool
970-
Labels bool
971-
ProjectsV1 bool
972-
ProjectsV2 bool
973-
Milestones bool
966+
Assignees bool
967+
Reviewers bool
968+
TeamReviewers bool
969+
// TODO ApiActorsSupported
970+
ApiActorsSupported bool
971+
Labels bool
972+
ProjectsV1 bool
973+
ProjectsV2 bool
974+
Milestones bool
974975
}
975976

976977
// RepoMetadata pre-fetches the metadata for attaching to issues and pull requests
@@ -979,7 +980,8 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput
979980
var g errgroup.Group
980981

981982
if input.Assignees || input.Reviewers {
982-
if input.ActorAssignees {
983+
// TODO ApiActorsSupported
984+
if input.ApiActorsSupported {
983985
g.Go(func() error {
984986
actors, err := RepoAssignableActors(client, repo)
985987
if err != nil {

internal/featuredetection/feature_detection.go

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,35 @@ type Detector interface {
2323
}
2424

2525
type IssueFeatures struct {
26-
ActorIsAssignable bool
26+
// TODO ApiActorsSupported
27+
// ApiActorsSupported indicates the host supports actor-based APIs. True for
28+
// github.com and ghe.com, false for GHES.
29+
//
30+
// The GitHub API has two generations of assignee/reviewer types:
31+
//
32+
// Legacy (GHES): Uses AssignableUser (users only) and node-ID-based mutations.
33+
// - assignableUsers query returns []AssignableUser
34+
// - Mutations take node IDs (assigneeIds, userReviewerIds, teamReviewerIds)
35+
//
36+
// Actor-based (github.com): Uses AssignableActor (User + Bot union) and
37+
// login-based mutations, enabling assignment of non-user actors like Copilot.
38+
// - suggestedActors query returns []AssignableActor (User | Bot)
39+
// - suggestedReviewerActors returns []ReviewerCandidate (User | Bot | Team)
40+
// - Mutations take logins (replaceActorsForAssignable, requestReviewsByLogin)
41+
//
42+
// When GHES adds support for the actor-based types and mutations, this flag
43+
// can be removed and all // TODO ApiActorsSupported sites collapsed to the
44+
// actor-only path. To verify GHES support, check whether the GHES GraphQL
45+
// schema includes:
46+
// - The suggestedActors field on Repository (assignee search)
47+
// - The suggestedReviewerActors field on PullRequest (reviewer search)
48+
// - The replaceActorsForAssignable mutation
49+
// - The requestReviewsByLogin mutation
50+
ApiActorsSupported bool
2751
}
2852

2953
var allIssueFeatures = IssueFeatures{
30-
ActorIsAssignable: true,
54+
ApiActorsSupported: true,
3155
}
3256

3357
type PullRequestFeatures struct {
@@ -136,7 +160,7 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) {
136160
}
137161

138162
return IssueFeatures{
139-
ActorIsAssignable: false, // replaceActorsForAssignable GraphQL mutation unavailable on GHES
163+
ApiActorsSupported: false, // TODO ApiActorsSupported — actor-based mutations unavailable on GHES
140164
}, nil
141165
}
142166

internal/featuredetection/feature_detection_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,23 +23,23 @@ func TestIssueFeatures(t *testing.T) {
2323
name: "github.com",
2424
hostname: "github.com",
2525
wantFeatures: IssueFeatures{
26-
ActorIsAssignable: true,
26+
ApiActorsSupported: true,
2727
},
2828
wantErr: false,
2929
},
3030
{
3131
name: "ghec data residency (ghe.com)",
3232
hostname: "stampname.ghe.com",
3333
wantFeatures: IssueFeatures{
34-
ActorIsAssignable: true,
34+
ApiActorsSupported: true,
3535
},
3636
wantErr: false,
3737
},
3838
{
3939
name: "GHE",
4040
hostname: "git.my.org",
4141
wantFeatures: IssueFeatures{
42-
ActorIsAssignable: false,
42+
ApiActorsSupported: false,
4343
},
4444
wantErr: false,
4545
},

pkg/cmd/issue/create/create.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func createRun(opts *CreateOptions) (err error) {
179179

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

190190
tb := prShared.IssueMetadataState{
191-
Type: prShared.IssueMetadata,
192-
ActorAssignees: issueFeatures.ActorIsAssignable,
193-
Assignees: assigneeSet.ToSlice(),
194-
Labels: opts.Labels,
195-
ProjectTitles: opts.Projects,
196-
Milestones: milestones,
197-
Title: opts.Title,
198-
Body: opts.Body,
191+
Type: prShared.IssueMetadata,
192+
ApiActorsSupported: issueFeatures.ApiActorsSupported, // TODO ApiActorsSupported
193+
Assignees: assigneeSet.ToSlice(),
194+
Labels: opts.Labels,
195+
ProjectTitles: opts.Projects,
196+
Milestones: milestones,
197+
Title: opts.Title,
198+
Body: opts.Body,
199199
}
200200

201201
if opts.RecoverFile != "" {
@@ -309,7 +309,7 @@ func createRun(opts *CreateOptions) (err error) {
309309
State: &tb,
310310
}
311311
var assigneeSearchFunc func(string) prompter.MultiSelectSearchResult
312-
if issueFeatures.ActorIsAssignable {
312+
if issueFeatures.ApiActorsSupported {
313313
assigneeSearchFunc = prShared.RepoAssigneeSearchFunc(apiClient, baseRepo)
314314
}
315315
err = prShared.MetadataSurvey(opts.Prompter, opts.IO, baseRepo, fetcher, &tb, projectsV1Support, nil, assigneeSearchFunc)

pkg/cmd/issue/create/create_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,7 @@ func Test_createRun(t *testing.T) {
548548
{ "data": { "replaceActorsForAssignable": { "__typename": "" } } }
549549
`, func(inputs map[string]interface{}) {
550550
assert.Equal(t, "ISSUEID", inputs["assignableId"])
551-
assert.Equal(t, []interface{}{"copilot-swe-agent", "MonaLisa"}, inputs["actorLogins"])
551+
assert.Equal(t, []interface{}{"copilot-swe-agent[bot]", "MonaLisa"}, inputs["actorLogins"])
552552
}))
553553
},
554554
wantsStdout: "https://github.com/OWNER/REPO/issues/12\n",
@@ -1161,7 +1161,7 @@ func TestIssueCreate_AtCopilotAssignee(t *testing.T) {
11611161
{ "data": { "replaceActorsForAssignable": { "__typename": "" } } }
11621162
`, func(inputs map[string]interface{}) {
11631163
assert.Equal(t, "NEWISSUEID", inputs["assignableId"])
1164-
assert.Equal(t, []interface{}{"copilot-swe-agent"}, inputs["actorLogins"])
1164+
assert.Equal(t, []interface{}{"copilot-swe-agent[bot]"}, inputs["actorLogins"])
11651165
}))
11661166

11671167
output, err := runCommand(http, true, `-a @copilot -t hello -b "cash rules everything around me"`, nil)

pkg/cmd/issue/edit/edit.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -215,9 +215,9 @@ func editRun(opts *EditOptions) error {
215215

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

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

@@ -280,7 +280,8 @@ func editRun(opts *EditOptions) error {
280280
editable.Body.Default = issue.Body
281281
// We use Actors as the default assignees if Actors are assignable
282282
// on this GitHub host.
283-
if editable.Assignees.ActorAssignees {
283+
// TODO ApiActorsSupported
284+
if editable.ApiActorsSupported {
284285
editable.Assignees.Default = issue.AssignedActors.DisplayNames()
285286
editable.Assignees.DefaultLogins = issue.AssignedActors.Logins()
286287
} else {

pkg/cmd/issue/edit/edit_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ func Test_editRun(t *testing.T) {
395395
mockIssueProjectItemsGet(t, reg)
396396
mockRepoMetadata(t, reg)
397397
mockIssueUpdate(t, reg)
398-
mockIssueUpdateActorAssignees(t, reg)
398+
mockIssueUpdateApiActors(t, reg)
399399
mockIssueUpdateLabels(t, reg)
400400
mockProjectV2ItemUpdate(t, reg)
401401
},
@@ -444,8 +444,8 @@ func Test_editRun(t *testing.T) {
444444
mockIssueProjectItemsGet(t, reg)
445445
mockIssueUpdate(t, reg)
446446
mockIssueUpdate(t, reg)
447-
mockIssueUpdateActorAssignees(t, reg)
448-
mockIssueUpdateActorAssignees(t, reg)
447+
mockIssueUpdateApiActors(t, reg)
448+
mockIssueUpdateApiActors(t, reg)
449449
mockIssueUpdateLabels(t, reg)
450450
mockIssueUpdateLabels(t, reg)
451451
mockProjectV2ItemUpdate(t, reg)
@@ -608,7 +608,7 @@ func Test_editRun(t *testing.T) {
608608
mockIssueProjectItemsGet(t, reg)
609609
mockRepoMetadata(t, reg)
610610
mockIssueUpdate(t, reg)
611-
mockIssueUpdateActorAssignees(t, reg)
611+
mockIssueUpdateApiActors(t, reg)
612612
mockIssueUpdateLabels(t, reg)
613613
mockProjectV2ItemUpdate(t, reg)
614614
},
@@ -902,7 +902,7 @@ func mockIssueUpdate(t *testing.T, reg *httpmock.Registry) {
902902
)
903903
}
904904

905-
func mockIssueUpdateActorAssignees(t *testing.T, reg *httpmock.Registry) {
905+
func mockIssueUpdateApiActors(t *testing.T, reg *httpmock.Registry) {
906906
reg.Register(
907907
httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`),
908908
httpmock.GraphQLMutation(`
@@ -935,7 +935,7 @@ func mockProjectV2ItemUpdate(t *testing.T, reg *httpmock.Registry) {
935935
)
936936
}
937937

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

pkg/cmd/pr/create/create.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -399,15 +399,15 @@ func createRun(opts *CreateOptions) error {
399399

400400
client := ctx.Client
401401

402-
// Detect ActorIsAssignable feature to determine if we can use search-based
402+
// Detect ApiActorsSupported feature to determine if we can use search-based
403403
// reviewer selection (github.com) or need to use legacy ID-based selection (GHES)
404404
issueFeatures, err := opts.Detector.IssueFeatures()
405405
if err != nil {
406406
return err
407407
}
408408
var reviewerSearchFunc func(string) prompter.MultiSelectSearchResult
409409
var assigneeSearchFunc func(string) prompter.MultiSelectSearchResult
410-
if issueFeatures.ActorIsAssignable {
410+
if issueFeatures.ApiActorsSupported {
411411
reviewerSearchFunc = func(query string) prompter.MultiSelectSearchResult {
412412
candidates, moreResults, err := api.SuggestedReviewerActorsForRepo(client, ctx.PRRefs.BaseRepo(), query)
413413
if err != nil {
@@ -424,14 +424,14 @@ func createRun(opts *CreateOptions) error {
424424
assigneeSearchFunc = shared.RepoAssigneeSearchFunc(client, ctx.PRRefs.BaseRepo())
425425
}
426426

427-
state, err := NewIssueState(*ctx, *opts)
427+
state, err := NewIssueState(*ctx, *opts, issueFeatures.ApiActorsSupported)
428428
if err != nil {
429429
return err
430430
}
431431

432-
if issueFeatures.ActorIsAssignable {
433-
state.ActorReviewers = true
434-
state.ActorAssignees = true
432+
// TODO ApiActorsSupported
433+
if issueFeatures.ApiActorsSupported {
434+
state.ApiActorsSupported = true
435435
}
436436

437437
var openURL string
@@ -672,14 +672,14 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
672672
return nil
673673
}
674674

675-
func NewIssueState(ctx CreateContext, opts CreateOptions) (*shared.IssueMetadataState, error) {
675+
func NewIssueState(ctx CreateContext, opts CreateOptions, apiActorsSupported bool) (*shared.IssueMetadataState, error) {
676676
var milestoneTitles []string
677677
if opts.Milestone != "" {
678678
milestoneTitles = []string{opts.Milestone}
679679
}
680680

681-
meReplacer := shared.NewMeReplacer(ctx.Client, ctx.PRRefs.BaseRepo().RepoHost())
682-
assignees, err := meReplacer.ReplaceSlice(opts.Assignees)
681+
assigneeReplacer := shared.NewSpecialAssigneeReplacer(ctx.Client, ctx.PRRefs.BaseRepo().RepoHost(), apiActorsSupported, !opts.WebMode)
682+
assignees, err := assigneeReplacer.ReplaceSlice(opts.Assignees)
683683
if err != nil {
684684
return nil, err
685685
}

0 commit comments

Comments
 (0)