From e6d9019bc9cf73ad8b4600549215519f70fe421b Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Mon, 23 Mar 2026 15:21:20 -0600 Subject: [PATCH 1/7] fix(pr create): use login-based assignee mutation on github.com When ActorAssignees is true (github.com), pass assignee logins directly to the ReplaceActorsForAssignable mutation instead of resolving logins to node IDs. This eliminates the need to bulk fetch all assignable users/actors and fixes a bug where providing assignees via CLI flag and then interactively adding metadata would fail with 'not found' because the cached MetadataResult had no assignee data. Changes: - Set state.ActorAssignees = true in pr create (was missing) - AddMetadataToIssueParams: pass assigneeLogins when ActorAssignees is true, skip fetch and ID resolution entirely - CreatePullRequest/IssueCreate: call ReplaceActorsForAssignableByLogin after creation to assign via logins - Consolidate replaceActorsForAssignable mutation into api/ package (ReplaceActorsForAssignableByLogin + ReplaceActorsForAssignableByID) - Remove duplicate replaceActorAssigneesForEditable from editable_http.go - Add TODO replaceActorsByLoginCleanup markers on edit paths Fixes cli/cli#13000 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- api/queries_issue.go | 11 +++- api/queries_pr.go | 64 ++++++++++++++++++++++ pkg/cmd/issue/create/create_test.go | 83 +++++++++++++++++------------ pkg/cmd/pr/create/create.go | 1 + pkg/cmd/pr/create/create_test.go | 23 ++++---- pkg/cmd/pr/shared/editable.go | 9 +++- pkg/cmd/pr/shared/editable_http.go | 33 +++--------- pkg/cmd/pr/shared/params.go | 28 ++++++---- 8 files changed, 165 insertions(+), 87 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index d545ef59f..f719e52f9 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -289,7 +289,8 @@ func IssueCreate(client *Client, repo *Repository, params map[string]interface{} switch key { case "assigneeIds", "body", "issueTemplate", "labelIds", "milestoneId", "projectIds", "repositoryId", "title": inputParams[key] = val - case "projectV2Ids": + case "projectV2Ids", "assigneeLogins": + // handled after issue creation default: return nil, fmt.Errorf("invalid IssueCreate mutation parameter %s", key) } @@ -310,6 +311,14 @@ 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). + if assigneeLogins, ok := params["assigneeLogins"].([]string); ok && len(assigneeLogins) > 0 { + err := ReplaceActorsForAssignableByLogin(client, repo, issue.ID, assigneeLogins) + if err != nil { + return issue, err + } + } + // projectV2 parameters aren't supported in the `createIssue` mutation, // so add them after the issue has been created. projectV2Ids, ok := params["projectV2Ids"].([]string) diff --git a/api/queries_pr.go b/api/queries_pr.go index d0488c64b..8b0d6b179 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -524,6 +524,14 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter } } + // Assign users using login-based mutation when ActorAssignees 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 // Request reviewers using either login-based (github.com) or ID-based (GHES) mutation. // The ID-based path can be removed once GHES supports requestReviewsByLogin. @@ -581,6 +589,62 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter return pr, nil } +// ReplaceActorsForAssignableByLogin calls the replaceActorsForAssignable mutation +// using actor logins. This avoids the need to resolve logins to node IDs. +func ReplaceActorsForAssignableByLogin(client *Client, repo ghrepo.Interface, assignableID string, logins []string) error { + type ReplaceActorsForAssignableInput struct { + AssignableID githubv4.ID `json:"assignableId"` + ActorLogins []githubv4.String `json:"actorLogins"` + } + + actorLogins := make([]githubv4.String, len(logins)) + for i, l := range logins { + actorLogins[i] = githubv4.String(l) + } + + variables := map[string]interface{}{ + "input": ReplaceActorsForAssignableInput{ + AssignableID: githubv4.ID(assignableID), + ActorLogins: actorLogins, + }, + } + + return replaceActorsForAssignable(client, repo, variables) +} + +// ReplaceActorsForAssignableByID calls the replaceActorsForAssignable mutation +// using actor node IDs. Used for GHES and edit flows that resolve IDs from search results. +func ReplaceActorsForAssignableByID(client *Client, repo ghrepo.Interface, assignableID string, actorIDs []string) error { + type ReplaceActorsForAssignableInput struct { + AssignableID githubv4.ID `json:"assignableId"` + ActorIDs []githubv4.ID `json:"actorIds"` + } + + ids := make([]githubv4.ID, len(actorIDs)) + for i, id := range actorIDs { + ids[i] = githubv4.ID(id) + } + + variables := map[string]interface{}{ + "input": ReplaceActorsForAssignableInput{ + AssignableID: githubv4.ID(assignableID), + ActorIDs: ids, + }, + } + + return replaceActorsForAssignable(client, repo, variables) +} + +func replaceActorsForAssignable(client *Client, repo ghrepo.Interface, variables map[string]interface{}) error { + var mutation struct { + ReplaceActorsForAssignable struct { + TypeName string `graphql:"__typename"` + } `graphql:"replaceActorsForAssignable(input: $input)"` + } + + return client.Mutate(repo.RepoHost(), "ReplaceActorsForAssignable", &mutation, variables) +} + // SuggestedAssignableActors fetches up to 10 suggested actors for a specific assignable // (Issue or PullRequest) node ID. `assignableID` is the GraphQL node ID for the Issue/PR. // Returns the actors, the total count of available assignees in the repo, and an error. diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 80c8f76d3..e02929a45 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -539,10 +539,21 @@ func Test_createRun(t *testing.T) { httpmock.GraphQL(`mutation IssueCreate\b`), httpmock.GraphQLMutation(` { "data": { "createIssue": { "issue": { + "id": "ISSUEID", "URL": "https://github.com/OWNER/REPO/issues/12" } } } } `, func(inputs map[string]interface{}) { - assert.Equal(t, []interface{}{"COPILOTID", "MONAID"}, inputs["assigneeIds"]) + if v, ok := inputs["assigneeIds"]; ok { + t.Errorf("did not expect assigneeIds: %v", v) + } + })) + r.Register( + httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`), + httpmock.GraphQLMutation(` + { "data": { "replaceActorsForAssignable": { "__typename": "" } } } + `, func(inputs map[string]interface{}) { + assert.Equal(t, "ISSUEID", inputs["assignableId"]) + assert.Equal(t, []interface{}{"copilot-swe-agent", "MonaLisa"}, inputs["actorLogins"]) })) }, wantsStdout: "https://github.com/OWNER/REPO/issues/12\n", @@ -948,16 +959,6 @@ func TestIssueCreate_metadata(t *testing.T) { defer http.Verify(t) http.StubRepoInfoResponse("OWNER", "REPO", "main") - http.Register( - httpmock.GraphQL(`query RepositoryAssignableActors\b`), - httpmock.StringResponse(` - { "data": { "repository": { "suggestedActors": { - "nodes": [ - { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) http.Register( httpmock.GraphQL(`query RepositoryLabelList\b`), httpmock.StringResponse(` @@ -1030,12 +1031,15 @@ func TestIssueCreate_metadata(t *testing.T) { httpmock.GraphQL(`mutation IssueCreate\b`), httpmock.GraphQLMutation(` { "data": { "createIssue": { "issue": { + "id": "NEWISSUEID", "URL": "https://github.com/OWNER/REPO/issues/12" } } } } `, func(inputs map[string]interface{}) { assert.Equal(t, "TITLE", inputs["title"]) assert.Equal(t, "BODY", inputs["body"]) - assert.Equal(t, []interface{}{"MONAID"}, inputs["assigneeIds"]) + if v, ok := inputs["assigneeIds"]; ok { + t.Errorf("did not expect assigneeIds: %v", v) + } assert.Equal(t, []interface{}{"BUGID", "TODOID"}, inputs["labelIds"]) assert.Equal(t, []interface{}{"ROADMAPID"}, inputs["projectIds"]) assert.Equal(t, "BIGONEID", inputs["milestoneId"]) @@ -1043,6 +1047,14 @@ func TestIssueCreate_metadata(t *testing.T) { assert.NotContains(t, inputs, "teamIds") assert.NotContains(t, inputs, "projectV2Ids") })) + http.Register( + httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`), + httpmock.GraphQLMutation(` + { "data": { "replaceActorsForAssignable": { "__typename": "" } } } + `, func(inputs map[string]interface{}) { + assert.Equal(t, "NEWISSUEID", inputs["assignableId"]) + assert.Equal(t, []interface{}{"monalisa"}, inputs["actorLogins"]) + })) output, err := runCommand(http, true, `-t TITLE -b BODY -a monalisa -l bug -l todo -p roadmap -m 'big one.oh'`, nil) if err != nil { @@ -1091,27 +1103,27 @@ func TestIssueCreate_AtMeAssignee(t *testing.T) { "hasIssuesEnabled": true } } } `)) - http.Register( - httpmock.GraphQL(`query RepositoryAssignableActors\b`), - httpmock.StringResponse(` - { "data": { "repository": { "suggestedActors": { - "nodes": [ - { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" }, - { "login": "SomeOneElse", "id": "SOMEID", "name": "Someone else", "__typename": "User" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) http.Register( httpmock.GraphQL(`mutation IssueCreate\b`), httpmock.GraphQLMutation(` { "data": { "createIssue": { "issue": { + "id": "NEWISSUEID", "URL": "https://github.com/OWNER/REPO/issues/12" } } } } `, func(inputs map[string]interface{}) { assert.Equal(t, "hello", inputs["title"]) assert.Equal(t, "cash rules everything around me", inputs["body"]) - assert.Equal(t, []interface{}{"MONAID", "SOMEID"}, inputs["assigneeIds"]) + if v, ok := inputs["assigneeIds"]; ok { + t.Errorf("did not expect assigneeIds: %v", v) + } + })) + http.Register( + httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`), + httpmock.GraphQLMutation(` + { "data": { "replaceActorsForAssignable": { "__typename": "" } } } + `, func(inputs map[string]interface{}) { + assert.Equal(t, "NEWISSUEID", inputs["assignableId"]) + assert.Equal(t, []interface{}{"MonaLisa", "someoneelse"}, inputs["actorLogins"]) })) output, err := runCommand(http, true, `-a @me -a someoneelse -t hello -b "cash rules everything around me"`, nil) @@ -1134,26 +1146,27 @@ func TestIssueCreate_AtCopilotAssignee(t *testing.T) { "hasIssuesEnabled": true } } } `)) - http.Register( - httpmock.GraphQL(`query RepositoryAssignableActors\b`), - httpmock.StringResponse(` - { "data": { "repository": { "suggestedActors": { - "nodes": [ - { "login": "copilot-swe-agent", "id": "COPILOTID", "name": "Copilot (AI)", "__typename": "Bot" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) http.Register( httpmock.GraphQL(`mutation IssueCreate\b`), httpmock.GraphQLMutation(` { "data": { "createIssue": { "issue": { + "id": "NEWISSUEID", "URL": "https://github.com/OWNER/REPO/issues/12" } } } } `, func(inputs map[string]interface{}) { assert.Equal(t, "hello", inputs["title"]) assert.Equal(t, "cash rules everything around me", inputs["body"]) - assert.Equal(t, []interface{}{"COPILOTID"}, inputs["assigneeIds"]) + if v, ok := inputs["assigneeIds"]; ok { + t.Errorf("did not expect assigneeIds: %v", v) + } + })) + http.Register( + httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`), + httpmock.GraphQLMutation(` + { "data": { "replaceActorsForAssignable": { "__typename": "" } } } + `, func(inputs map[string]interface{}) { + assert.Equal(t, "NEWISSUEID", inputs["assignableId"]) + assert.Equal(t, []interface{}{"copilot-swe-agent"}, inputs["actorLogins"]) })) output, err := runCommand(http, true, `-a @copilot -t hello -b "cash rules everything around me"`, nil) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 0dbdb3987..76a30e24b 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -429,6 +429,7 @@ func createRun(opts *CreateOptions) error { if issueFeatures.ActorIsAssignable { state.ActorReviewers = true + state.ActorAssignees = true } var openURL string diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index ed2a9cf2e..fd53a92fc 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -916,17 +916,6 @@ func Test_createRun(t *testing.T) { return func() {} }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { - reg.Register( - httpmock.GraphQL(`query RepositoryAssignableUsers\b`), - httpmock.StringResponse(` - { "data": { "repository": { "assignableUsers": { - "nodes": [ - { "login": "hubot", "id": "HUBOTID", "name": "" }, - { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) reg.Register( httpmock.GraphQL(`query RepositoryLabelList\b`), httpmock.StringResponse(` @@ -975,11 +964,21 @@ func Test_createRun(t *testing.T) { } } } `, func(inputs map[string]interface{}) { assert.Equal(t, "NEWPULLID", inputs["pullRequestId"]) - assert.Equal(t, []interface{}{"MONAID"}, inputs["assigneeIds"]) + if _, ok := inputs["assigneeIds"]; ok { + t.Error("did not expect assigneeIds in updatePullRequest when ActorAssignees is true") + } assert.Equal(t, []interface{}{"BUGID", "TODOID"}, inputs["labelIds"]) assert.Equal(t, []interface{}{"ROADMAPID"}, inputs["projectIds"]) assert.Equal(t, "BIGONEID", inputs["milestoneId"]) })) + reg.Register( + httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`), + httpmock.GraphQLMutation(` + { "data": { "replaceActorsForAssignable": { "__typename": "" } } } + `, func(inputs map[string]interface{}) { + assert.Equal(t, "NEWPULLID", inputs["assignableId"]) + assert.Equal(t, []interface{}{"monalisa"}, inputs["actorLogins"]) + })) reg.Register( httpmock.GraphQL(`mutation RequestReviewsByLogin\b`), httpmock.GraphQLMutation(` diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 1b7c42be3..a70cbf1ff 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -142,6 +142,10 @@ func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]str e.Assignees.Value = assigneeSet.ToSlice() } + // TODO replaceActorsByLoginCleanup + // When ActorAssignees is true (github.com), this should compute the final + // assignee logins and return them directly without resolving to IDs, for use + // with ReplaceActorsForAssignableByLogin. a, err := e.Metadata.MembersToIDs(e.Assignees.Value) return &a, err } @@ -472,8 +476,9 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable, } // However, if we have Add/Remove operations (non-interactive flow), // we do need to fetch the assignees. - // TODO: KW noninteractive assignees need to migrate to directly use - // new logins input with ReplaceActorsForAssignable to prevent fetching. + // TODO replaceActorsByLoginCleanup + // When ActorAssignees is true, noninteractive assignees should use + // ReplaceActorsForAssignableByLogin to skip this fetch entirely. if len(editable.Assignees.Add) > 0 || len(editable.Assignees.Remove) > 0 { fetchAssignees = true } diff --git a/pkg/cmd/pr/shared/editable_http.go b/pkg/cmd/pr/shared/editable_http.go index 8cd51c349..097252eaa 100644 --- a/pkg/cmd/pr/shared/editable_http.go +++ b/pkg/cmd/pr/shared/editable_http.go @@ -66,6 +66,11 @@ func UpdateIssue(httpClient *http.Client, repo ghrepo.Interface, id string, isPR // other issue fields to ensure consistency with how legacy // user assignees are handled. // https://github.com/cli/cli/pull/10960#discussion_r2086725348 + // TODO replaceActorsByLoginCleanup + // When ActorAssignees is true (github.com), this should pass logins directly + // to ReplaceActorsForAssignableByLogin instead of resolving IDs. The ID-based + // path can be removed once both interactive and non-interactive edit flows + // are migrated to use logins. if options.Assignees.Edited && options.Assignees.ActorAssignees { apiClient := api.NewClientFromHTTP(httpClient) assigneeIds, err := options.AssigneeIds(apiClient, repo) @@ -73,7 +78,7 @@ func UpdateIssue(httpClient *http.Client, repo ghrepo.Interface, id string, isPR return err } - err = replaceActorAssigneesForEditable(apiClient, repo, id, assigneeIds) + err = api.ReplaceActorsForAssignableByID(apiClient, repo, id, *assigneeIds) if err != nil { return err } @@ -90,32 +95,6 @@ func UpdateIssue(httpClient *http.Client, repo ghrepo.Interface, id string, isPR return wg.Wait() } -func replaceActorAssigneesForEditable(apiClient *api.Client, repo ghrepo.Interface, id string, assigneeIds *[]string) error { - type ReplaceActorsForAssignableInput struct { - AssignableID githubv4.ID `json:"assignableId"` - ActorIDs []githubv4.ID `json:"actorIds"` - } - - params := ReplaceActorsForAssignableInput{ - AssignableID: githubv4.ID(id), - ActorIDs: *ghIds(assigneeIds), - } - - var mutation struct { - ReplaceActorsForAssignable struct { - TypeName string `graphql:"__typename"` - } `graphql:"replaceActorsForAssignable(input: $input)"` - } - - variables := map[string]interface{}{"input": params} - err := apiClient.Mutate(repo.RepoHost(), "ReplaceActorsForAssignable", &mutation, variables) - if err != nil { - return err - } - - return nil -} - func replaceIssueFields(httpClient *http.Client, repo ghrepo.Interface, id string, isPR bool, options Editable) error { apiClient := api.NewClientFromHTTP(httpClient) diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index d5c168e5f..17d40d639 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -64,6 +64,9 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par // When ActorReviewers is true, we use login-based mutation and don't need to resolve reviewer IDs. needReviewerIDs := len(tb.Reviewers) > 0 && !tb.ActorReviewers + // When ActorAssignees is true, we use login-based mutation and don't need to resolve assignee IDs. + needAssigneeIDs := len(tb.Assignees) > 0 && !tb.ActorAssignees + // Retrieve minimal information needed to resolve metadata if this was not previously cached from additional metadata survey. if tb.MetadataResult == nil { input := api.RepoMetadataInput{ @@ -71,12 +74,11 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par TeamReviewers: needReviewerIDs && slices.ContainsFunc(tb.Reviewers, func(r string) bool { return strings.ContainsRune(r, '/') }), - Assignees: len(tb.Assignees) > 0, - ActorAssignees: tb.ActorAssignees, - Labels: len(tb.Labels) > 0, - ProjectsV1: len(tb.ProjectTitles) > 0 && projectV1Support == gh.ProjectsV1Supported, - ProjectsV2: len(tb.ProjectTitles) > 0, - Milestones: len(tb.Milestones) > 0, + Assignees: needAssigneeIDs, + Labels: len(tb.Labels) > 0, + ProjectsV1: len(tb.ProjectTitles) > 0 && projectV1Support == gh.ProjectsV1Supported, + ProjectsV2: len(tb.ProjectTitles) > 0, + Milestones: len(tb.Milestones) > 0, } metadataResult, err := api.RepoMetadata(client, baseRepo, input) @@ -86,11 +88,17 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par tb.MetadataResult = metadataResult } - assigneeIDs, err := tb.MetadataResult.MembersToIDs(tb.Assignees) - if err != nil { - return fmt.Errorf("could not assign user: %w", err) + // When ActorAssignees is true (github.com), pass logins directly for use with + // ReplaceActorsForAssignable mutation. The ID-based else branch is for GHES compatibility. + if tb.ActorAssignees { + params["assigneeLogins"] = tb.Assignees + } else { + assigneeIDs, err := tb.MetadataResult.MembersToIDs(tb.Assignees) + if err != nil { + return fmt.Errorf("could not assign user: %w", err) + } + params["assigneeIds"] = assigneeIDs } - params["assigneeIds"] = assigneeIDs labelIDs, err := tb.MetadataResult.LabelsToIDs(tb.Labels) if err != nil { From b3cfe7454cd42e5c19a4766793eb37169cc8f1df Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Mon, 23 Mar 2026 15:33:37 -0600 Subject: [PATCH 2/7] refactor(pr edit, issue edit): use login-based assignee mutation for flag flows When ActorAssignees is true (github.com), the --add-assignee and --remove-assignee flag flows now pass logins directly to ReplaceActorsForAssignableByLogin instead of bulk fetching all assignable actors and resolving logins to node IDs. Changes: - New AssigneeLogins() method on Editable that computes the final login set (defaults + add - remove) without ID resolution - UpdateIssue: call AssigneeLogins + ByLogin when ActorAssignees is true - EditableOptionsFetch: skip assignee bulk fetch for flag flows on github.com (only fetch on GHES where ID resolution is needed) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/issue/edit/edit_test.go | 38 ++++++++------------- pkg/cmd/pr/edit/edit_test.go | 10 +++--- pkg/cmd/pr/shared/editable.go | 53 ++++++++++++++++++++++++++---- pkg/cmd/pr/shared/editable_http.go | 9 ++--- 4 files changed, 67 insertions(+), 43 deletions(-) diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index 2fcb85c56..c529f5ae2 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -527,17 +527,6 @@ func Test_editRun(t *testing.T) { }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { // Should only be one fetch of metadata. - reg.Register( - httpmock.GraphQL(`query RepositoryAssignableActors\b`), - httpmock.StringResponse(` - { "data": { "repository": { "suggestedActors": { - "nodes": [ - { "login": "hubot", "id": "HUBOTID", "__typename": "Bot" }, - { "login": "MonaLisa", "id": "MONAID", "__typename": "User" } - ], - "pageInfo": { "hasNextPage": false, "endCursor": "Mg" } - } } } } - `)) reg.Register( httpmock.GraphQL(`query RepositoryMilestoneList\b`), httpmock.StringResponse(` @@ -618,6 +607,17 @@ func Test_editRun(t *testing.T) { mockIssueGet(t, reg) mockIssueProjectItemsGet(t, reg) mockRepoMetadata(t, reg) + reg.Register( + httpmock.GraphQL(`query RepositoryAssignableActors\b`), + httpmock.StringResponse(` + { "data": { "repository": { "suggestedActors": { + "nodes": [ + { "login": "hubot", "id": "HUBOTID", "__typename": "Bot" }, + { "login": "monalisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) mockIssueUpdate(t, reg) mockIssueUpdateActorAssignees(t, reg) mockIssueUpdateLabels(t, reg) @@ -667,9 +667,9 @@ func Test_editRun(t *testing.T) { { "data": { "replaceActorsForAssignable": { "__typename": "" } } }`, func(inputs map[string]interface{}) { // Checking that despite the display name being returned - // from the EditFieldsSurvey, the ID is still + // from the EditFieldsSurvey, the login is still // used in the mutation. - require.Subset(t, inputs["actorIds"], []string{"MONAID", "HUBOTID"}) + require.Subset(t, inputs["actorLogins"], []interface{}{"hubot", "MonaLisa (Mona Display Name)"}) }), ) }, @@ -839,18 +839,6 @@ func mockIssueProjectItemsGet(_ *testing.T, reg *httpmock.Registry) { } func mockRepoMetadata(_ *testing.T, reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(`query RepositoryAssignableActors\b`), - httpmock.StringResponse(` - { "data": { "repository": { "suggestedActors": { - "nodes": [ - { "login": "hubot", "id": "HUBOTID", "__typename": "Bot" }, - { "login": "monalisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) - reg.Register( httpmock.GraphQL(`query RepositoryLabelList\b`), httpmock.StringResponse(` diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index e38fac3ca..2582c500b 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -415,7 +415,7 @@ func Test_editRun(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { // Non-interactive with Add/Remove doesn't need reviewers/assignees metadata // REST API accepts logins and team slugs directly - mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: true, labels: true, projects: true, milestones: true}) + mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: false, labels: true, projects: true, milestones: true}) mockPullRequestUpdate(reg) mockPullRequestUpdateActorAssignees(reg) mockRequestReviewsByLogin(reg) @@ -473,7 +473,7 @@ func Test_editRun(t *testing.T) { Fetcher: testFetcher{}, }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { - mockRepoMetadata(reg, mockRepoMetadataOptions{assignees: true, labels: true, projects: true, milestones: true}) + mockRepoMetadata(reg, mockRepoMetadataOptions{assignees: false, labels: true, projects: true, milestones: true}) mockPullRequestUpdate(reg) mockPullRequestUpdateActorAssignees(reg) mockPullRequestUpdateLabels(reg) @@ -547,7 +547,7 @@ func Test_editRun(t *testing.T) { }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { // Non-interactive with Remove doesn't need reviewers metadata - mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: true, labels: true, projects: true, milestones: true}) + mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: false, labels: true, projects: true, milestones: true}) mockPullRequestUpdate(reg) mockRequestReviewsByLogin(reg) mockPullRequestUpdateLabels(reg) @@ -932,9 +932,9 @@ func Test_editRun(t *testing.T) { { "data": { "replaceActorsForAssignable": { "__typename": "" } } }`, func(inputs map[string]interface{}) { // Checking that despite the display name being returned - // from the EditFieldsSurvey, the ID is still + // from the EditFieldsSurvey, the login is still // used in the mutation. - require.Subset(t, inputs["actorIds"], []string{"MONAID", "HUBOTID"}) + require.Subset(t, inputs["actorLogins"], []interface{}{"hubot", "monalisa (Mona Display Name)"}) }), ) }, diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index a70cbf1ff..bb6228b4e 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -150,6 +150,49 @@ func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]str return &a, err } +// AssigneeLogins computes the final list of assignee logins from the current +// defaults plus any Add/Remove operations. Unlike AssigneeIds, this does not +// resolve logins to node IDs, and is used on github.com where the +// ReplaceActorsForAssignable mutation accepts logins directly. +func (e Editable) AssigneeLogins(client *api.Client, repo ghrepo.Interface) ([]string, error) { + if !e.Assignees.Edited { + return nil, nil + } + + if len(e.Assignees.Add) != 0 || len(e.Assignees.Remove) != 0 { + meReplacer := NewMeReplacer(client, repo.RepoHost()) + copilotReplacer := NewCopilotReplacer(true) + + replaceSpecialAssigneeNames := func(value []string) ([]string, error) { + replaced, err := meReplacer.ReplaceSlice(value) + if err != nil { + return nil, err + } + replaced = copilotReplacer.ReplaceSlice(replaced) + return replaced, nil + } + + assigneeSet := set.NewStringSet() + assigneeSet.AddValues(e.Assignees.DefaultLogins) + + add, err := replaceSpecialAssigneeNames(e.Assignees.Add) + if err != nil { + return nil, err + } + assigneeSet.AddValues(add) + + remove, err := replaceSpecialAssigneeNames(e.Assignees.Remove) + if err != nil { + return nil, err + } + assigneeSet.RemoveValues(remove) + + e.Assignees.Value = assigneeSet.ToSlice() + } + + return e.Assignees.Value, nil +} + // ProjectIds returns a slice containing IDs of projects v1 that the issue or a PR has to be linked to. func (e Editable) ProjectIds() (*[]string, error) { if !e.Projects.Edited { @@ -474,12 +517,10 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable, if len(editable.Assignees.Add) == 0 && len(editable.Assignees.Remove) == 0 && editable.AssigneeSearchFunc == nil { fetchAssignees = true } - // However, if we have Add/Remove operations (non-interactive flow), - // we do need to fetch the assignees. - // TODO replaceActorsByLoginCleanup - // When ActorAssignees is true, noninteractive assignees should use - // ReplaceActorsForAssignableByLogin to skip this fetch entirely. - if len(editable.Assignees.Add) > 0 || len(editable.Assignees.Remove) > 0 { + // For non-interactive Add/Remove operations, we only need to fetch assignees + // on GHES where ID resolution is required. On github.com (ActorAssignees), + // logins are passed directly to the mutation. + if (len(editable.Assignees.Add) > 0 || len(editable.Assignees.Remove) > 0) && !editable.Assignees.ActorAssignees { fetchAssignees = true } } diff --git a/pkg/cmd/pr/shared/editable_http.go b/pkg/cmd/pr/shared/editable_http.go index 097252eaa..bb63ceb0f 100644 --- a/pkg/cmd/pr/shared/editable_http.go +++ b/pkg/cmd/pr/shared/editable_http.go @@ -66,19 +66,14 @@ func UpdateIssue(httpClient *http.Client, repo ghrepo.Interface, id string, isPR // other issue fields to ensure consistency with how legacy // user assignees are handled. // https://github.com/cli/cli/pull/10960#discussion_r2086725348 - // TODO replaceActorsByLoginCleanup - // When ActorAssignees is true (github.com), this should pass logins directly - // to ReplaceActorsForAssignableByLogin instead of resolving IDs. The ID-based - // path can be removed once both interactive and non-interactive edit flows - // are migrated to use logins. if options.Assignees.Edited && options.Assignees.ActorAssignees { apiClient := api.NewClientFromHTTP(httpClient) - assigneeIds, err := options.AssigneeIds(apiClient, repo) + logins, err := options.AssigneeLogins(apiClient, repo) if err != nil { return err } - err = api.ReplaceActorsForAssignableByID(apiClient, repo, id, *assigneeIds) + err = api.ReplaceActorsForAssignableByLogin(apiClient, repo, id, logins) if err != nil { return err } From e24f55d5a447c17898293f8a6276cac3ce2d7236 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Mon, 23 Mar 2026 15:34:44 -0600 Subject: [PATCH 3/7] refactor(pr edit): remove actor accumulation hack from assignee search The assigneeSearchFunc previously accumulated actors into editable.Metadata.AssignableActors so that MembersToIDs could later resolve logins to node IDs. Now that the edit flow uses AssigneeLogins + ReplaceActorsForAssignableByLogin on github.com, this accumulation is no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/pr/edit/edit.go | 7 ------- pkg/cmd/pr/shared/editable.go | 4 ---- 2 files changed, 11 deletions(-) diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 157fc8f8a..3a6a9d083 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -348,9 +348,6 @@ func editRun(opts *EditOptions) error { // assigneeSearchFunc is intended to be an arg for MultiSelectWithSearch // to return potential assignee actors. -// It also contains an important enclosure to update the editable's -// assignable actors metadata for later ID resolution - this is required -// while we continue to use IDs for mutating assignees with the GQL API. func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, editable *shared.Editable, assignableID string) func(string) prompter.MultiSelectSearchResult { searchFunc := func(input string) prompter.MultiSelectSearchResult { actors, availableAssigneesCount, err := api.SuggestedAssignableActors( @@ -382,10 +379,6 @@ func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, editable * } else { displayNames = append(displayNames, a.Login()) } - - // Update the assignable actors metadata in the editable struct - // so that updating the PR later can resolve the actor ID. - editable.Metadata.AssignableActors = append(editable.Metadata.AssignableActors, a) } return prompter.MultiSelectSearchResult{ Keys: logins, diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index bb6228b4e..a26c2a024 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -142,10 +142,6 @@ func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]str e.Assignees.Value = assigneeSet.ToSlice() } - // TODO replaceActorsByLoginCleanup - // When ActorAssignees is true (github.com), this should compute the final - // assignee logins and return them directly without resolving to IDs, for use - // with ReplaceActorsForAssignableByLogin. a, err := e.Metadata.MembersToIDs(e.Assignees.Value) return &a, err } From 947f8fb1b7c618f415ab69b36b6f28b194e97d35 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Mon, 23 Mar 2026 15:37:50 -0600 Subject: [PATCH 4/7] refactor(issue edit): wire up search-based assignee selection Add AssigneeSearchFunc to gh issue edit interactive flow, matching the pattern already used in gh pr edit. This eliminates the bulk RepositoryAssignableActors fetch for interactive assignee selection, using dynamic SuggestedAssignableActors search instead. Also clean up pr edit assigneeSearchFunc signature to remove the unused editable parameter (no longer needed after removing the actor accumulation hack). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/issue/edit/edit.go | 41 +++++++++++++++++++++++++++++++++ pkg/cmd/issue/edit/edit_test.go | 22 ------------------ pkg/cmd/pr/edit/edit.go | 4 ++-- pkg/cmd/pr/shared/editable.go | 18 ++++++++------- 4 files changed, 53 insertions(+), 32 deletions(-) diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 11921096a..c0cd0c89c 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -12,6 +12,7 @@ import ( fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/text" shared "github.com/cli/cli/v2/pkg/cmd/issue/shared" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" @@ -248,6 +249,13 @@ 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. + // Interactive mode only supports a single issue, so we use its ID for the search query. + if issueFeatures.ActorIsAssignable && opts.Interactive && len(issues) == 1 { + editable.AssigneeSearchFunc = assigneeSearchFunc(apiClient, baseRepo, issues[0].ID) + } + opts.IO.StartProgressIndicatorWithLabel("Fetching repository information") err = opts.FetchOptions(apiClient, baseRepo, &editable, opts.Detector.ProjectsV1()) opts.IO.StopProgressIndicator() @@ -351,3 +359,36 @@ func editRun(opts *EditOptions) error { return nil } + +func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, assignableID string) func(string) prompter.MultiSelectSearchResult { + return func(input string) prompter.MultiSelectSearchResult { + actors, availableAssigneesCount, err := api.SuggestedAssignableActors( + apiClient, + repo, + assignableID, + input) + if err != nil { + return prompter.MultiSelectSearchResult{Err: err} + } + + logins := make([]string, 0, len(actors)) + displayNames := make([]string, 0, len(actors)) + + for _, a := range actors { + if a.Login() == "" { + continue + } + logins = append(logins, a.Login()) + if a.DisplayName() != "" { + displayNames = append(displayNames, a.DisplayName()) + } else { + displayNames = append(displayNames, a.Login()) + } + } + return prompter.MultiSelectSearchResult{ + Keys: logins, + Labels: displayNames, + MoreResults: availableAssigneesCount, + } + } +} diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index c529f5ae2..d4dcce399 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -607,17 +607,6 @@ func Test_editRun(t *testing.T) { mockIssueGet(t, reg) mockIssueProjectItemsGet(t, reg) mockRepoMetadata(t, reg) - reg.Register( - httpmock.GraphQL(`query RepositoryAssignableActors\b`), - httpmock.StringResponse(` - { "data": { "repository": { "suggestedActors": { - "nodes": [ - { "login": "hubot", "id": "HUBOTID", "__typename": "Bot" }, - { "login": "monalisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) mockIssueUpdate(t, reg) mockIssueUpdateActorAssignees(t, reg) mockIssueUpdateLabels(t, reg) @@ -649,17 +638,6 @@ func Test_editRun(t *testing.T) { }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { mockIsssueNumberGetWithAssignedActors(t, reg, 123) - reg.Register( - httpmock.GraphQL(`query RepositoryAssignableActors\b`), - httpmock.StringResponse(` - { "data": { "repository": { "suggestedActors": { - "nodes": [ - { "login": "hubot", "id": "HUBOTID", "__typename": "Bot" }, - { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) mockIssueUpdate(t, reg) reg.Register( httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`), diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 3a6a9d083..ea0712491 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -303,7 +303,7 @@ func editRun(opts *EditOptions) error { // to legacy reviewer/assignee fetching. // TODO actorIsAssignableCleanup if issueFeatures.ActorIsAssignable { - editable.AssigneeSearchFunc = assigneeSearchFunc(apiClient, repo, &editable, pr.ID) + editable.AssigneeSearchFunc = assigneeSearchFunc(apiClient, repo, pr.ID) editable.ReviewerSearchFunc = reviewerSearchFunc(apiClient, repo, &editable, pr.ID) } @@ -348,7 +348,7 @@ func editRun(opts *EditOptions) error { // assigneeSearchFunc is intended to be an arg for MultiSelectWithSearch // to return potential assignee actors. -func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, editable *shared.Editable, assignableID string) func(string) prompter.MultiSelectSearchResult { +func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, assignableID string) func(string) prompter.MultiSelectSearchResult { searchFunc := func(input string) prompter.MultiSelectSearchResult { actors, availableAssigneesCount, err := api.SuggestedAssignableActors( apiClient, diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index a26c2a024..115d765f1 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -267,14 +267,16 @@ func (e Editable) MilestoneId() (*string, error) { // go routines. Fields that would be mutated will be copied. func (e *Editable) Clone() Editable { return Editable{ - Title: e.Title.clone(), - Body: e.Body.clone(), - Base: e.Base.clone(), - Reviewers: e.Reviewers.clone(), - Assignees: e.Assignees.clone(), - Labels: e.Labels.clone(), - Projects: e.Projects.clone(), - Milestone: e.Milestone.clone(), + Title: e.Title.clone(), + Body: e.Body.clone(), + Base: e.Base.clone(), + Reviewers: e.Reviewers.clone(), + ReviewerSearchFunc: e.ReviewerSearchFunc, + Assignees: e.Assignees.clone(), + AssigneeSearchFunc: e.AssigneeSearchFunc, + Labels: e.Labels.clone(), + Projects: e.Projects.clone(), + Milestone: e.Milestone.clone(), // Shallow copy since no mutation. Metadata: e.Metadata, } From 33783748f3d77ec149cf5b09fd54fe0fd45b2c0d Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Mon, 23 Mar 2026 17:36:52 -0600 Subject: [PATCH 5/7] review: address code review feedback - Fix tests: assert logins (not display names) in actorLogins - Remove dead ReplaceActorsForAssignableByID (no callers) - Extract shared AssigneeSearchFunc to pkg/cmd/pr/shared/editable.go - Remove duplicate assigneeSearchFunc from pr/edit and issue/edit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- api/queries_pr.go | 39 +++++----------------------- pkg/cmd/issue/edit/edit.go | 36 +------------------------- pkg/cmd/issue/edit/edit_test.go | 10 +++---- pkg/cmd/pr/edit/edit.go | 46 +-------------------------------- pkg/cmd/pr/edit/edit_test.go | 13 +++------- pkg/cmd/pr/shared/editable.go | 32 +++++++++++++++++++++++ 6 files changed, 47 insertions(+), 129 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 8b0d6b179..01846a9e6 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -602,6 +602,12 @@ func ReplaceActorsForAssignableByLogin(client *Client, repo ghrepo.Interface, as actorLogins[i] = githubv4.String(l) } + var mutation struct { + ReplaceActorsForAssignable struct { + TypeName string `graphql:"__typename"` + } `graphql:"replaceActorsForAssignable(input: $input)"` + } + variables := map[string]interface{}{ "input": ReplaceActorsForAssignableInput{ AssignableID: githubv4.ID(assignableID), @@ -609,39 +615,6 @@ func ReplaceActorsForAssignableByLogin(client *Client, repo ghrepo.Interface, as }, } - return replaceActorsForAssignable(client, repo, variables) -} - -// ReplaceActorsForAssignableByID calls the replaceActorsForAssignable mutation -// using actor node IDs. Used for GHES and edit flows that resolve IDs from search results. -func ReplaceActorsForAssignableByID(client *Client, repo ghrepo.Interface, assignableID string, actorIDs []string) error { - type ReplaceActorsForAssignableInput struct { - AssignableID githubv4.ID `json:"assignableId"` - ActorIDs []githubv4.ID `json:"actorIds"` - } - - ids := make([]githubv4.ID, len(actorIDs)) - for i, id := range actorIDs { - ids[i] = githubv4.ID(id) - } - - variables := map[string]interface{}{ - "input": ReplaceActorsForAssignableInput{ - AssignableID: githubv4.ID(assignableID), - ActorIDs: ids, - }, - } - - return replaceActorsForAssignable(client, repo, variables) -} - -func replaceActorsForAssignable(client *Client, repo ghrepo.Interface, variables map[string]interface{}) error { - var mutation struct { - ReplaceActorsForAssignable struct { - TypeName string `graphql:"__typename"` - } `graphql:"replaceActorsForAssignable(input: $input)"` - } - return client.Mutate(repo.RepoHost(), "ReplaceActorsForAssignable", &mutation, variables) } diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index c0cd0c89c..2fd632b5e 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -12,7 +12,6 @@ import ( fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" - "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/text" shared "github.com/cli/cli/v2/pkg/cmd/issue/shared" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" @@ -253,7 +252,7 @@ func editRun(opts *EditOptions) error { // Wire up search function for assignees when ActorIsAssignable 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 { - editable.AssigneeSearchFunc = assigneeSearchFunc(apiClient, baseRepo, issues[0].ID) + editable.AssigneeSearchFunc = prShared.AssigneeSearchFunc(apiClient, baseRepo, issues[0].ID) } opts.IO.StartProgressIndicatorWithLabel("Fetching repository information") @@ -359,36 +358,3 @@ func editRun(opts *EditOptions) error { return nil } - -func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, assignableID string) func(string) prompter.MultiSelectSearchResult { - return func(input string) prompter.MultiSelectSearchResult { - actors, availableAssigneesCount, err := api.SuggestedAssignableActors( - apiClient, - repo, - assignableID, - input) - if err != nil { - return prompter.MultiSelectSearchResult{Err: err} - } - - logins := make([]string, 0, len(actors)) - displayNames := make([]string, 0, len(actors)) - - for _, a := range actors { - if a.Login() == "" { - continue - } - logins = append(logins, a.Login()) - if a.DisplayName() != "" { - displayNames = append(displayNames, a.DisplayName()) - } else { - displayNames = append(displayNames, a.Login()) - } - } - return prompter.MultiSelectSearchResult{ - Keys: logins, - Labels: displayNames, - MoreResults: availableAssigneesCount, - } - } -} diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index d4dcce399..ef496d1de 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -629,8 +629,9 @@ func Test_editRun(t *testing.T) { require.Equal(t, []string{"hubot"}, eo.Assignees.Default) require.Equal(t, []string{"hubot"}, eo.Assignees.DefaultLogins) - // Adding MonaLisa as PR assignee, should preserve hubot. - eo.Assignees.Value = []string{"hubot", "MonaLisa (Mona Display Name)"} + // Adding MonaLisa as issue assignee, should preserve hubot. + // MultiSelectWithSearch returns Keys (logins), not display names. + eo.Assignees.Value = []string{"hubot", "MonaLisa"} return nil }, FetchOptions: prShared.FetchOptions, @@ -644,10 +645,7 @@ func Test_editRun(t *testing.T) { httpmock.GraphQLMutation(` { "data": { "replaceActorsForAssignable": { "__typename": "" } } }`, func(inputs map[string]interface{}) { - // Checking that despite the display name being returned - // from the EditFieldsSurvey, the login is still - // used in the mutation. - require.Subset(t, inputs["actorLogins"], []interface{}{"hubot", "MonaLisa (Mona Display Name)"}) + require.Subset(t, inputs["actorLogins"], []interface{}{"hubot", "MonaLisa"}) }), ) }, diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index ea0712491..0937c7533 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -303,7 +303,7 @@ func editRun(opts *EditOptions) error { // to legacy reviewer/assignee fetching. // TODO actorIsAssignableCleanup if issueFeatures.ActorIsAssignable { - editable.AssigneeSearchFunc = assigneeSearchFunc(apiClient, repo, pr.ID) + editable.AssigneeSearchFunc = shared.AssigneeSearchFunc(apiClient, repo, pr.ID) editable.ReviewerSearchFunc = reviewerSearchFunc(apiClient, repo, &editable, pr.ID) } @@ -346,50 +346,6 @@ func editRun(opts *EditOptions) error { return nil } -// assigneeSearchFunc is intended to be an arg for MultiSelectWithSearch -// to return potential assignee actors. -func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, assignableID string) func(string) prompter.MultiSelectSearchResult { - searchFunc := func(input string) prompter.MultiSelectSearchResult { - actors, availableAssigneesCount, err := api.SuggestedAssignableActors( - apiClient, - repo, - assignableID, - input) - if err != nil { - return prompter.MultiSelectSearchResult{ - Keys: nil, - Labels: nil, - MoreResults: 0, - Err: err, - } - } - - logins := make([]string, 0, len(actors)) - displayNames := make([]string, 0, len(actors)) - - for _, a := range actors { - if a.Login() != "" { - logins = append(logins, a.Login()) - } else { - continue - } - - if a.DisplayName() != "" { - displayNames = append(displayNames, a.DisplayName()) - } else { - displayNames = append(displayNames, a.Login()) - } - } - return prompter.MultiSelectSearchResult{ - Keys: logins, - Labels: displayNames, - MoreResults: availableAssigneesCount, - Err: nil, - } - } - return searchFunc -} - // reviewerSearchFunc is intended to be an arg for MultiSelectWithSearch // to return potential reviewer candidates (users, bots, and teams). // It also updates the editable's metadata for later ID resolution. diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 2582c500b..01acb7e0f 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -912,12 +912,8 @@ func Test_editRun(t *testing.T) { require.Equal(t, []string{"hubot"}, e.Assignees.DefaultLogins) // Adding monalisa as PR assignee, should preserve hubot. - e.Assignees.Value = []string{"hubot", "monalisa (Mona Display Name)"} - // Populate metadata to simulate what searchFunc would do during prompting - e.Metadata.AssignableActors = []api.AssignableActor{ - api.NewAssignableBot("HUBOTID", "hubot"), - api.NewAssignableUser("MONAID", "monalisa", "Mona Display Name"), - } + // MultiSelectWithSearch returns Keys (logins), not display names. + e.Assignees.Value = []string{"hubot", "monalisa"} return nil }, }, @@ -931,10 +927,7 @@ func Test_editRun(t *testing.T) { httpmock.GraphQLMutation(` { "data": { "replaceActorsForAssignable": { "__typename": "" } } }`, func(inputs map[string]interface{}) { - // Checking that despite the display name being returned - // from the EditFieldsSurvey, the login is still - // used in the mutation. - require.Subset(t, inputs["actorLogins"], []interface{}{"hubot", "monalisa (Mona Display Name)"}) + require.Subset(t, inputs["actorLogins"], []interface{}{"hubot", "monalisa"}) }), ) }, diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 115d765f1..566c9939f 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -611,3 +611,35 @@ func milestoneSurvey(p EditPrompter, title string, opts []string) (result string result = opts[selected] return } + +// AssigneeSearchFunc returns a search function for MultiSelectWithSearch that +// dynamically fetches assignable actors for the given assignable (Issue/PR) node ID. +func AssigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, assignableID string) func(string) prompter.MultiSelectSearchResult { + return func(input string) prompter.MultiSelectSearchResult { + actors, availableAssigneesCount, err := api.SuggestedAssignableActors( + apiClient, repo, assignableID, input) + if err != nil { + return prompter.MultiSelectSearchResult{Err: err} + } + + logins := make([]string, 0, len(actors)) + displayNames := make([]string, 0, len(actors)) + + for _, a := range actors { + if a.Login() == "" { + continue + } + logins = append(logins, a.Login()) + if a.DisplayName() != "" { + displayNames = append(displayNames, a.DisplayName()) + } else { + displayNames = append(displayNames, a.Login()) + } + } + return prompter.MultiSelectSearchResult{ + Keys: logins, + Labels: displayNames, + MoreResults: availableAssigneesCount, + } + } +} From 11f177a8c33f8a4dec3cb72c88d9df3eac75f035 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Mon, 23 Mar 2026 18:45:09 -0600 Subject: [PATCH 6/7] feat(pr create, issue create): search-based assignee selection in MetadataSurvey Wire up MultiSelectWithSearch for assignees in MetadataSurvey, replacing the static MultiSelect that required bulk fetching all assignable actors. This applies to both gh pr create and gh issue create interactive flows when selecting assignees via the 'Add metadata' prompt. Changes: - Add assigneeSearchFunc parameter to MetadataSurvey - Skip assignee bulk fetch when search func is available - New SearchRepoAssignableActors API function for repo-level search (create flows have no issue/PR node ID yet) - New RepoAssigneeSearchFunc in shared editable.go - Refactor actorsToSearchResult helper shared by both search functions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- api/queries_repo.go | 63 +++++++++++++++++++++++++++++ pkg/cmd/issue/create/create.go | 7 +++- pkg/cmd/issue/create/create_test.go | 21 ++++------ pkg/cmd/pr/create/create.go | 4 +- pkg/cmd/pr/shared/editable.go | 59 +++++++++++++++++---------- pkg/cmd/pr/shared/survey.go | 51 +++++++++++++---------- pkg/cmd/pr/shared/survey_test.go | 8 ++-- 7 files changed, 151 insertions(+), 62 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index d358255d8..e5806b91e 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -1298,6 +1298,69 @@ func RepoAssignableActors(client *Client, repo ghrepo.Interface) ([]AssignableAc return actors, nil } +// SearchRepoAssignableActors searches assignable actors for a repository with an optional +// query string. Unlike RepoAssignableActors which fetches all actors with pagination, this +// returns up to 10 results matching the query, suitable for search-based selection. +func SearchRepoAssignableActors(client *Client, repo ghrepo.Interface, query string) ([]AssignableActor, int, error) { + type responseData struct { + Repository struct { + AssignableUsers struct { + TotalCount int + } + SuggestedActors struct { + Nodes []struct { + User struct { + ID string + Login string + Name string + TypeName string `graphql:"__typename"` + } `graphql:"... on User"` + Bot struct { + ID string + Login string + TypeName string `graphql:"__typename"` + } `graphql:"... on Bot"` + } + } `graphql:"suggestedActors(first: 10, query: $query, capabilities: CAN_BE_ASSIGNED)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + var q *githubv4.String + if query != "" { + v := githubv4.String(query) + q = &v + } + + variables := map[string]interface{}{ + "owner": githubv4.String(repo.RepoOwner()), + "name": githubv4.String(repo.RepoName()), + "query": q, + } + + var result responseData + if err := client.Query(repo.RepoHost(), "SearchRepoAssignableActors", &result, variables); err != nil { + return nil, 0, err + } + + var actors []AssignableActor + for _, node := range result.Repository.SuggestedActors.Nodes { + if node.User.TypeName == "User" { + actors = append(actors, AssignableUser{ + id: node.User.ID, + login: node.User.Login, + name: node.User.Name, + }) + } else if node.Bot.TypeName == "Bot" { + actors = append(actors, AssignableBot{ + id: node.Bot.ID, + login: node.Bot.Login, + }) + } + } + + return actors, result.Repository.AssignableUsers.TotalCount, nil +} + type RepoLabel struct { ID string Name string diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index da3648c31..2a3dda638 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -12,6 +12,7 @@ import ( fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/text" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" @@ -313,7 +314,11 @@ func createRun(opts *CreateOptions) (err error) { Repo: baseRepo, State: &tb, } - err = prShared.MetadataSurvey(opts.Prompter, opts.IO, baseRepo, fetcher, &tb, projectsV1Support, nil) + var assigneeSearchFunc func(string) prompter.MultiSelectSearchResult + if issueFeatures.ActorIsAssignable { + assigneeSearchFunc = prShared.RepoAssigneeSearchFunc(apiClient, baseRepo) + } + err = prShared.MetadataSurvey(opts.Prompter, opts.IO, baseRepo, fetcher, &tb, projectsV1Support, nil, assigneeSearchFunc) if err != nil { return } diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index e02929a45..57de4c9b5 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -495,12 +495,18 @@ func Test_createRun(t *testing.T) { switch message { case "What would you like to add?": return prompter.IndexesFor(options, "Assignees") - case "Assignees": - return prompter.IndexesFor(options, "Copilot (AI)", "MonaLisa (Mona Display Name)") default: return nil, fmt.Errorf("unexpected multi-select prompt: %s", message) } } + pm.MultiSelectWithSearchFunc = func(message, searchPrompt string, defaults, persistentOptions []string, searchFunc func(string) prompter.MultiSelectSearchResult) ([]string, error) { + switch message { + case "Assignees": + return []string{"copilot-swe-agent", "MonaLisa"}, nil + default: + return nil, fmt.Errorf("unexpected multi-select-with-search prompt: %s", message) + } + } pm.SelectFunc = func(message, defaultValue string, options []string) (int, error) { switch message { case "What's next?": @@ -524,17 +530,6 @@ func Test_createRun(t *testing.T) { "viewerPermission": "WRITE" } } } `)) - r.Register( - httpmock.GraphQL(`query RepositoryAssignableActors\b`), - httpmock.StringResponse(` - { "data": { "repository": { "suggestedActors": { - "nodes": [ - { "login": "copilot-swe-agent", "id": "COPILOTID", "name": "Copilot (AI)", "__typename": "Bot" }, - { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) r.Register( httpmock.GraphQL(`mutation IssueCreate\b`), httpmock.GraphQLMutation(` diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 76a30e24b..d8230dc3c 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -406,6 +406,7 @@ func createRun(opts *CreateOptions) error { return err } var reviewerSearchFunc func(string) prompter.MultiSelectSearchResult + var assigneeSearchFunc func(string) prompter.MultiSelectSearchResult if issueFeatures.ActorIsAssignable { reviewerSearchFunc = func(query string) prompter.MultiSelectSearchResult { candidates, moreResults, err := api.SuggestedReviewerActorsForRepo(client, ctx.PRRefs.BaseRepo(), query) @@ -420,6 +421,7 @@ func createRun(opts *CreateOptions) error { } return prompter.MultiSelectSearchResult{Keys: keys, Labels: labels, MoreResults: moreResults} } + assigneeSearchFunc = shared.RepoAssigneeSearchFunc(client, ctx.PRRefs.BaseRepo()) } state, err := NewIssueState(*ctx, *opts) @@ -598,7 +600,7 @@ func createRun(opts *CreateOptions) error { Repo: ctx.PRRefs.BaseRepo(), State: state, } - err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.PRRefs.BaseRepo(), fetcher, state, projectsV1Support, reviewerSearchFunc) + err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.PRRefs.BaseRepo(), fetcher, state, projectsV1Support, reviewerSearchFunc, assigneeSearchFunc) if err != nil { return err } diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 566c9939f..1e13146c2 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -616,30 +616,45 @@ func milestoneSurvey(p EditPrompter, title string, opts []string) (result string // dynamically fetches assignable actors for the given assignable (Issue/PR) node ID. func AssigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, assignableID string) func(string) prompter.MultiSelectSearchResult { return func(input string) prompter.MultiSelectSearchResult { - actors, availableAssigneesCount, err := api.SuggestedAssignableActors( - apiClient, repo, assignableID, input) + actors, count, err := api.SuggestedAssignableActors(apiClient, repo, assignableID, input) if err != nil { return prompter.MultiSelectSearchResult{Err: err} } - - logins := make([]string, 0, len(actors)) - displayNames := make([]string, 0, len(actors)) - - for _, a := range actors { - if a.Login() == "" { - continue - } - logins = append(logins, a.Login()) - if a.DisplayName() != "" { - displayNames = append(displayNames, a.DisplayName()) - } else { - displayNames = append(displayNames, a.Login()) - } - } - return prompter.MultiSelectSearchResult{ - Keys: logins, - Labels: displayNames, - MoreResults: availableAssigneesCount, - } + return actorsToSearchResult(actors, count) + } +} + +// RepoAssigneeSearchFunc returns a search function for MultiSelectWithSearch that +// dynamically fetches assignable actors at the repository level. Used during create +// flows where no issue/PR node ID exists yet. +func RepoAssigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface) func(string) prompter.MultiSelectSearchResult { + return func(input string) prompter.MultiSelectSearchResult { + actors, count, err := api.SearchRepoAssignableActors(apiClient, repo, input) + if err != nil { + return prompter.MultiSelectSearchResult{Err: err} + } + return actorsToSearchResult(actors, count) + } +} + +func actorsToSearchResult(actors []api.AssignableActor, totalCount int) prompter.MultiSelectSearchResult { + logins := make([]string, 0, len(actors)) + displayNames := make([]string, 0, len(actors)) + + for _, a := range actors { + if a.Login() == "" { + continue + } + logins = append(logins, a.Login()) + if a.DisplayName() != "" { + displayNames = append(displayNames, a.DisplayName()) + } else { + displayNames = append(displayNames, a.Login()) + } + } + return prompter.MultiSelectSearchResult{ + Keys: logins, + Labels: displayNames, + MoreResults: totalCount, } } diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index cc66bbe5c..71cfcd063 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -154,7 +154,7 @@ type RepoMetadataFetcher interface { RepoMetadataFetch(api.RepoMetadataInput) (*api.RepoMetadataResult, error) } -func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface, fetcher RepoMetadataFetcher, state *IssueMetadataState, projectsV1Support gh.ProjectsV1Support, reviewerSearchFunc func(string) prompter.MultiSelectSearchResult) error { +func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface, fetcher RepoMetadataFetcher, state *IssueMetadataState, projectsV1Support gh.ProjectsV1Support, reviewerSearchFunc func(string) prompter.MultiSelectSearchResult, assigneeSearchFunc func(string) prompter.MultiSelectSearchResult) error { isChosen := func(m string) bool { for _, c := range state.Metadata { if m == c { @@ -184,11 +184,12 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface // When search-based reviewer selection is available, skip the expensive assignable-users // and teams fetch since reviewers are found dynamically via the search function. useReviewerSearch := state.ActorReviewers && reviewerSearchFunc != nil + useAssigneeSearch := state.ActorAssignees && assigneeSearchFunc != nil metadataInput := api.RepoMetadataInput{ Reviewers: isChosen("Reviewers") && !useReviewerSearch, TeamReviewers: isChosen("Reviewers") && !useReviewerSearch, - Assignees: isChosen("Assignees"), - ActorAssignees: isChosen("Assignees") && state.ActorAssignees, + Assignees: isChosen("Assignees") && !useAssigneeSearch, + ActorAssignees: isChosen("Assignees") && !useAssigneeSearch && state.ActorAssignees, Labels: isChosen("Labels"), ProjectsV1: isChosen("Projects") && projectsV1Support == gh.ProjectsV1Supported, ProjectsV2: isChosen("Projects"), @@ -212,24 +213,25 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface } // Populate the list of selectable assignees and their default selections. - // This logic maps the default assignees from `state` to the corresponding actors or users - // so that the correct display names are preselected in the prompt. + // When search-based selection is available, skip building the static list. var assignees []string var assigneesDefault []string - if state.ActorAssignees { - for _, u := range metadataResult.AssignableActors { - assignees = append(assignees, u.DisplayName()) + if !useAssigneeSearch { + if state.ActorAssignees { + for _, u := range metadataResult.AssignableActors { + assignees = append(assignees, u.DisplayName()) - if slices.Contains(state.Assignees, u.Login()) { - assigneesDefault = append(assigneesDefault, u.DisplayName()) + if slices.Contains(state.Assignees, u.Login()) { + assigneesDefault = append(assigneesDefault, u.DisplayName()) + } } - } - } else { - for _, u := range metadataResult.AssignableUsers { - assignees = append(assignees, u.DisplayName()) + } else { + for _, u := range metadataResult.AssignableUsers { + assignees = append(assignees, u.DisplayName()) - if slices.Contains(state.Assignees, u.Login()) { - assigneesDefault = append(assigneesDefault, u.DisplayName()) + if slices.Contains(state.Assignees, u.Login()) { + assigneesDefault = append(assigneesDefault, u.DisplayName()) + } } } } @@ -286,16 +288,23 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface } } if isChosen("Assignees") { - if len(assignees) > 0 { + if useAssigneeSearch { + selectedAssignees, err := p.MultiSelectWithSearch( + "Assignees", + "Search assignees", + state.Assignees, + []string{}, + assigneeSearchFunc) + if err != nil { + return err + } + values.Assignees = selectedAssignees + } else if len(assignees) > 0 { selected, err := p.MultiSelect("Assignees", assigneesDefault, assignees) if err != nil { return err } for _, i := range selected { - // Previously, this logic relied upon `assignees` being in `` or ` ()` form, - // however the inclusion of actors breaks this convention. - // Instead, we map the selected indexes to the source that populated `assignees` rather than - // relying on parsing the information out. if state.ActorAssignees { values.Assignees = append(values.Assignees, metadataResult.AssignableActors[i].Login()) } else { diff --git a/pkg/cmd/pr/shared/survey_test.go b/pkg/cmd/pr/shared/survey_test.go index 23ba96cef..384e54895 100644 --- a/pkg/cmd/pr/shared/survey_test.go +++ b/pkg/cmd/pr/shared/survey_test.go @@ -71,7 +71,7 @@ func TestMetadataSurvey_selectAll(t *testing.T) { Assignees: []string{"hubot"}, Type: PRMetadata, } - err := MetadataSurvey(pm, ios, repo, fetcher, state, gh.ProjectsV1Supported, nil) + err := MetadataSurvey(pm, ios, repo, fetcher, state, gh.ProjectsV1Supported, nil, nil) assert.NoError(t, err) assert.Equal(t, "", stdout.String()) @@ -117,7 +117,7 @@ func TestMetadataSurvey_keepExisting(t *testing.T) { Assignees: []string{"hubot"}, } - err := MetadataSurvey(pm, ios, repo, fetcher, state, gh.ProjectsV1Supported, nil) + err := MetadataSurvey(pm, ios, repo, fetcher, state, gh.ProjectsV1Supported, nil, nil) assert.NoError(t, err) assert.Equal(t, "", stdout.String()) @@ -146,7 +146,7 @@ func TestMetadataSurveyProjectV1Deprecation(t *testing.T) { return []int{0}, nil }) - err := MetadataSurvey(pm, ios, repo, fetcher, &IssueMetadataState{}, gh.ProjectsV1Supported, nil) + err := MetadataSurvey(pm, ios, repo, fetcher, &IssueMetadataState{}, gh.ProjectsV1Supported, nil, nil) require.ErrorContains(t, err, "expected test error") require.True(t, fetcher.projectsV1Requested, "expected projectsV1 to be requested") @@ -167,7 +167,7 @@ func TestMetadataSurveyProjectV1Deprecation(t *testing.T) { return []int{0}, nil }) - err := MetadataSurvey(pm, ios, repo, fetcher, &IssueMetadataState{}, gh.ProjectsV1Unsupported, nil) + err := MetadataSurvey(pm, ios, repo, fetcher, &IssueMetadataState{}, gh.ProjectsV1Unsupported, nil, nil) require.ErrorContains(t, err, "expected test error") require.False(t, fetcher.projectsV1Requested, "expected projectsV1 not to be requested") From 4e6bc78e0496d94e896818e8848a3bab20282109 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 24 Mar 2026 18:59:21 -0600 Subject: [PATCH 7/7] refactor(pr shared): extract SpecialAssigneeReplacer for @me and Copilot expansion The inline replaceSpecialAssigneeNames closures in AssigneeIds and AssigneeLogins were duplicated. Extract them into an exported SpecialAssigneeReplacer type that consolidates MeReplacer and CopilotReplacer into a single ReplaceSlice call, parameterised by actorAssignees and copilotUseLogin. Adopt the new type in the issue create flow as well, replacing the manual MeReplacer + conditional CopilotReplacer sequence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/issue/create/create.go | 12 ++---- pkg/cmd/pr/shared/editable.go | 68 ++++++++++++++++++---------------- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 2a3dda638..f9fab11a1 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -179,18 +179,12 @@ func createRun(opts *CreateOptions) (err error) { // Replace special values in assignees // For web mode, @copilot should be replaced by name; otherwise, login. - assigneeSet := set.NewStringSet() - meReplacer := prShared.NewMeReplacer(apiClient, baseRepo.RepoHost()) - copilotReplacer := prShared.NewCopilotReplacer(!opts.WebMode) - assignees, err := meReplacer.ReplaceSlice(opts.Assignees) + assigneeReplacer := prShared.NewSpecialAssigneeReplacer(apiClient, baseRepo.RepoHost(), issueFeatures.ActorIsAssignable, !opts.WebMode) + assignees, err := assigneeReplacer.ReplaceSlice(opts.Assignees) if err != nil { return err } - - // TODO actorIsAssignableCleanup - if issueFeatures.ActorIsAssignable { - assignees = copilotReplacer.ReplaceSlice(assignees) - } + assigneeSet := set.NewStringSet() assigneeSet.AddValues(assignees) tb := prShared.IssueMetadataState{ diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 1e13146c2..02fe5d33c 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -95,22 +95,7 @@ func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]str // If assignees came in from command line flags, we need to // curate the final list of assignees from the default list. if len(e.Assignees.Add) != 0 || len(e.Assignees.Remove) != 0 { - meReplacer := NewMeReplacer(client, repo.RepoHost()) - copilotReplacer := NewCopilotReplacer(true) - - replaceSpecialAssigneeNames := func(value []string) ([]string, error) { - replaced, err := meReplacer.ReplaceSlice(value) - if err != nil { - return nil, err - } - - // Only suppported for actor assignees. - if e.Assignees.ActorAssignees { - replaced = copilotReplacer.ReplaceSlice(replaced) - } - - return replaced, nil - } + replacer := NewSpecialAssigneeReplacer(client, repo.RepoHost(), e.Assignees.ActorAssignees, true) assigneeSet := set.NewStringSet() @@ -128,13 +113,13 @@ func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]str assigneeSet.AddValues(e.Assignees.Default) } - add, err := replaceSpecialAssigneeNames(e.Assignees.Add) + add, err := replacer.ReplaceSlice(e.Assignees.Add) if err != nil { return nil, err } assigneeSet.AddValues(add) - remove, err := replaceSpecialAssigneeNames(e.Assignees.Remove) + remove, err := replacer.ReplaceSlice(e.Assignees.Remove) if err != nil { return nil, err } @@ -156,28 +141,18 @@ func (e Editable) AssigneeLogins(client *api.Client, repo ghrepo.Interface) ([]s } if len(e.Assignees.Add) != 0 || len(e.Assignees.Remove) != 0 { - meReplacer := NewMeReplacer(client, repo.RepoHost()) - copilotReplacer := NewCopilotReplacer(true) - - replaceSpecialAssigneeNames := func(value []string) ([]string, error) { - replaced, err := meReplacer.ReplaceSlice(value) - if err != nil { - return nil, err - } - replaced = copilotReplacer.ReplaceSlice(replaced) - return replaced, nil - } + replacer := NewSpecialAssigneeReplacer(client, repo.RepoHost(), true, true) assigneeSet := set.NewStringSet() assigneeSet.AddValues(e.Assignees.DefaultLogins) - add, err := replaceSpecialAssigneeNames(e.Assignees.Add) + add, err := replacer.ReplaceSlice(e.Assignees.Add) if err != nil { return nil, err } assigneeSet.AddValues(add) - remove, err := replaceSpecialAssigneeNames(e.Assignees.Remove) + remove, err := replacer.ReplaceSlice(e.Assignees.Remove) if err != nil { return nil, err } @@ -189,6 +164,37 @@ func (e Editable) AssigneeLogins(client *api.Client, repo ghrepo.Interface) ([]s return e.Assignees.Value, nil } +// SpecialAssigneeReplacer expands special assignee names (@me, Copilot actors) +// in login slices. Use NewSpecialAssigneeReplacer to create one. +type SpecialAssigneeReplacer struct { + meReplacer *MeReplacer + copilotReplacer *CopilotReplacer + actorAssignees bool +} + +// NewSpecialAssigneeReplacer creates a replacer that expands @me and (when +// actorAssignees is true) Copilot actor names in assignee slices. +// copilotUseLogin controls whether Copilot actors are replaced with their +// login (true) or display name (false, used for web mode). +func NewSpecialAssigneeReplacer(client *api.Client, host string, actorAssignees bool, copilotUseLogin bool) *SpecialAssigneeReplacer { + return &SpecialAssigneeReplacer{ + meReplacer: NewMeReplacer(client, host), + copilotReplacer: NewCopilotReplacer(copilotUseLogin), + actorAssignees: actorAssignees, + } +} + +func (r *SpecialAssigneeReplacer) ReplaceSlice(logins []string) ([]string, error) { + replaced, err := r.meReplacer.ReplaceSlice(logins) + if err != nil { + return nil, err + } + if r.actorAssignees { + replaced = r.copilotReplacer.ReplaceSlice(replaced) + } + return replaced, nil +} + // ProjectIds returns a slice containing IDs of projects v1 that the issue or a PR has to be linked to. func (e Editable) ProjectIds() (*[]string, error) { if !e.Projects.Edited {