From 0611b9ca0f756f467dca00f7710685439c2c245f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 18 Mar 2026 14:03:50 +0000 Subject: [PATCH 01/11] chore(deps): bump google.golang.org/grpc from 1.79.2 to 1.79.3 Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.79.2 to 1.79.3. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](https://github.com/grpc/grpc-go/compare/v1.79.2...v1.79.3) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-version: 1.79.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index a3a55c3cd68..76b90741c47 100644 --- a/go.mod +++ b/go.mod @@ -54,7 +54,7 @@ require ( golang.org/x/sync v0.20.0 golang.org/x/term v0.40.0 golang.org/x/text v0.34.0 - google.golang.org/grpc v1.79.2 + google.golang.org/grpc v1.79.3 google.golang.org/protobuf v1.36.11 gopkg.in/h2non/gock.v1 v1.1.2 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index af66f223f17..54b5f05c4a0 100644 --- a/go.sum +++ b/go.sum @@ -640,8 +640,8 @@ google.golang.org/genproto/googleapis/api v0.0.0-20251202230838-ff82c1b0f217 h1: google.golang.org/genproto/googleapis/api v0.0.0-20251202230838-ff82c1b0f217/go.mod h1:+rXWjjaukWZun3mLfjmVnQi18E1AsFbDN9QdJ5YXLto= google.golang.org/genproto/googleapis/rpc v0.0.0-20251222181119-0a764e51fe1b h1:Mv8VFug0MP9e5vUxfBcE3vUkV6CImK3cMNMIDFjmzxU= google.golang.org/genproto/googleapis/rpc v0.0.0-20251222181119-0a764e51fe1b/go.mod h1:j9x/tPzZkyxcgEFkiKEEGxfvyumM01BEtsW8xzOahRQ= -google.golang.org/grpc v1.79.2 h1:fRMD94s2tITpyJGtBBn7MkMseNpOZU8ZxgC3MMBaXRU= -google.golang.org/grpc v1.79.2/go.mod h1:KmT0Kjez+0dde/v2j9vzwoAScgEPx/Bw1CYChhHLrHQ= +google.golang.org/grpc v1.79.3 h1:sybAEdRIEtvcD68Gx7dmnwjZKlyfuc61Dyo9pGXXkKE= +google.golang.org/grpc v1.79.3/go.mod h1:KmT0Kjez+0dde/v2j9vzwoAScgEPx/Bw1CYChhHLrHQ= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= 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 02/11] 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 d545ef59f85..f719e52f9ae 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 d0488c64b4b..8b0d6b17942 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 80c8f76d35b..e02929a4583 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 0dbdb3987b9..76a30e24b48 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 ed2a9cf2e63..fd53a92fcc7 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 1b7c42be3b6..a70cbf1ff1b 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 8cd51c34942..097252eaa35 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 d5c168e5f7f..17d40d6395c 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 03/11] 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 2fcb85c569a..c529f5ae294 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 e38fac3caa0..2582c500bbd 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 a70cbf1ff1b..bb6228b4e72 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 097252eaa35..bb63ceb0f62 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 04/11] 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 157fc8f8a7b..3a6a9d083e3 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 bb6228b4e72..a26c2a024a2 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 05/11] 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 11921096a1d..c0cd0c89cb6 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 c529f5ae294..d4dcce39946 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 3a6a9d083e3..ea071249173 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 a26c2a024a2..115d765f126 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 06/11] 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 | 37 ++++---------------------- 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, 46 insertions(+), 128 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 8b0d6b17942..01846a9e6d0 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -602,46 +602,19 @@ func ReplaceActorsForAssignableByLogin(client *Client, repo ghrepo.Interface, as 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) + var mutation struct { + ReplaceActorsForAssignable struct { + TypeName string `graphql:"__typename"` + } `graphql:"replaceActorsForAssignable(input: $input)"` } variables := map[string]interface{}{ "input": ReplaceActorsForAssignableInput{ AssignableID: githubv4.ID(assignableID), - ActorIDs: ids, + ActorLogins: actorLogins, }, } - 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 c0cd0c89cb6..2fd632b5ef7 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 d4dcce39946..ef496d1ded8 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 ea071249173..0937c753357 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 2582c500bbd..01acb7e0fba 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 115d765f126..566c9939f85 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 07/11] 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 | 51 ++++++++++++++--------- pkg/cmd/pr/shared/survey.go | 51 +++++++++++++---------- pkg/cmd/pr/shared/survey_test.go | 8 ++-- 7 files changed, 147 insertions(+), 58 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index d358255d8b9..e5806b91e90 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 da3648c31b2..2a3dda638e7 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 e02929a4583..57de4c9b579 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 76a30e24b48..d8230dc3c68 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 566c9939f85..1e13146c233 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} } + return actorsToSearchResult(actors, count) + } +} - logins := make([]string, 0, len(actors)) - displayNames := make([]string, 0, len(actors)) +// 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) + } +} - 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()) - } +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 } - return prompter.MultiSelectSearchResult{ - Keys: logins, - Labels: displayNames, - MoreResults: availableAssigneesCount, + 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 cc66bbe5c4d..71cfcd06343 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 23ba96ceff7..384e5489500 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 45596c9b638763bb8a97949b1bd24aa2d0d5530d Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 24 Mar 2026 18:00:42 +0100 Subject: [PATCH 08/11] Add AGENTS.md optimized for AI agent token efficiency Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- AGENTS.md | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 AGENTS.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000000..55a7c489fd3 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,141 @@ +# AGENTS.md + +This is the GitHub CLI (`gh`), a command-line tool for interacting with GitHub. The module path is `github.com/cli/cli/v2`. + +## Build, Test, and Lint + +```bash +make # Build (Unix) — outputs bin/gh +go run script/build.go # Build (Windows) +go test ./... # All unit tests +go test ./pkg/cmd/issue/list/... -run TestIssueList_nontty # Single test +go test -tags acceptance ./acceptance # Acceptance tests +make lint # golangci-lint (same as CI) +``` + +## Architecture + +Entry point: `cmd/gh/main.go` → `internal/ghcmd.Main()` → `pkg/cmd/root.NewCmdRoot()`. + +Key packages: +- `pkg/cmd///` — CLI command implementations +- `pkg/cmdutil/` — Factory, error types, flag helpers (`NilStringFlag`, `NilBoolFlag`, `StringEnumFlag`) +- `pkg/iostreams/` — I/O abstraction with TTY detection, color, pager +- `pkg/httpmock/` — HTTP mocking for tests +- `api/` — GitHub API client (GraphQL + REST) +- `internal/featuredetection/` — GitHub.com vs GHES capability detection +- `internal/tableprinter/` — Table output for list commands + +## Command Structure + +A command `gh foo bar` lives in `pkg/cmd/foo/bar/` with `bar.go`, `bar_test.go`, and optionally `http.go`/`http_test.go`. + +### Canonical Examples + +- **Command + tests**: `pkg/cmd/issue/list/list.go` and `list_test.go` +- **Factory wiring**: `pkg/cmd/factory/default.go` +- **Unit tests**: `internal/agents/detect_test.go` + +### The Options + Factory Pattern + +Every command follows this structure (see `pkg/cmd/issue/list/list.go`): + +1. `Options` struct with `IO`, `HttpClient`, `Config`, `BaseRepo` + flags +2. `NewCmdFoo(f *cmdutil.Factory, runF func(*FooOptions) error)` constructor — `runF` is the test injection point +3. Separate `fooRun(opts)` function with the business logic + +Key rules: +- Lazy-init `BaseRepo`, `Remotes`, `Branch` inside `RunE`, not the constructor +- Commands register in `pkg/cmd/root/root.go`; subcommand groups use `cmdutil.AddGroup()` + +### Command Examples and Help Text + +Use `heredoc.Doc` for examples with `#` comment lines and `$ ` command prefixes: +```go +Example: heredoc.Doc(` + # Do the thing + $ gh foo bar --flag value +`), +``` + +### JSON Output + +Add `--json`, `--jq`, `--template` flags via `cmdutil.AddJSONFlags(cmd, &opts.Exporter, fieldNames)`. In the run function: `if opts.Exporter != nil { return opts.Exporter.Write(opts.IO, data) }`. See `pkg/cmd/pr/list/list.go`. + +## Testing + +### HTTP Mocking + +Use `httpmock.Registry` with `defer reg.Verify(t)` to ensure all stubs are called: + +```go +reg := &httpmock.Registry{} +defer reg.Verify(t) + +reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO"), + httpmock.JSONResponse(someData), +) +reg.Register( + httpmock.GraphQL(`query PullRequestList\b`), + httpmock.FileResponse("./fixtures/prList.json"), +) +client := &http.Client{Transport: reg} +``` + +Common: `REST(method, path)`, `GraphQL(pattern)`, `JSONResponse(body)`, `FileResponse(path)`. See `pkg/httpmock/` for all matchers/responders. + +### IOStreams in Tests + +```go +ios, stdin, stdout, stderr := iostreams.Test() +ios.SetStdoutTTY(true) // simulate terminal +``` + +### Assertions + +Use `testify`. Always use `require` (not `assert`) for error checks so the test halts immediately: + +```go +require.NoError(t, err) +require.Error(t, err) +assert.Equal(t, "expected", actual) +``` + +### Generated Mocks + +Interfaces use `moq`: `//go:generate moq -rm -out prompter_mock.go . Prompter`. Run `go generate ./...` after interface changes. + +## Error Handling + +Error types in `pkg/cmdutil/errors.go`: +- `FlagErrorf(...)` — flag validation (prints usage) +- `cmdutil.SilentError` — exit 1, no message +- `cmdutil.CancelError` — user cancelled +- `cmdutil.PendingError` — outcome pending +- `cmdutil.NoResultsError` — empty results + +Use `cmdutil.MutuallyExclusive("message", cond1, cond2)` for mutually exclusive flags. + +## Feature Detection + +Commands using feature detection must include a `// TODO ` comment directly above the if-statement for linter compliance: + +```go +// TODO someFeatureCleanup +if features.SomeCapability { + // use new API +} else { + // fallback for older GHES +} +``` + +## API Patterns + +```go +client := api.NewClientFromHTTP(httpClient) +client.GraphQL(hostname, query, variables, &data) +client.REST(hostname, "GET", "repos/owner/repo", nil, &data) +``` + +For host resolution, use `cfg.Authentication().DefaultHost()` — not `ghinstance.Default()` which always returns `github.com`. \ No newline at end of file From d10b56e8e527ab559ed4fbf9e569a0525c848cbc Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 24 Mar 2026 19:55:51 +0100 Subject: [PATCH 09/11] Address review: table tests, godocs, code style Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- AGENTS.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 55a7c489fd3..98024f533c3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -106,6 +106,30 @@ assert.Equal(t, "expected", actual) Interfaces use `moq`: `//go:generate moq -rm -out prompter_mock.go . Prompter`. Run `go generate ./...` after interface changes. +### Table-Driven Tests + +Use table-driven tests for functions with multiple input/output scenarios. See `internal/agents/detect_test.go` or `pkg/cmd/issue/list/list_test.go` for examples: + +```go +tests := []struct { + name string + // inputs and expected outputs +}{ + {name: "descriptive case name", ...}, +} +for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // arrange, act, assert + }) +} +``` + +## Code Style + +- Add godoc comments to all exported functions, types, and constants +- Avoid unnecessary code comments — only comment when the *why* isn't obvious from the code +- Do not comment just to restate what the code does + ## Error Handling Error types in `pkg/cmdutil/errors.go`: From c51769c977da4a049a9609685f862834528fdbf3 Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 24 Mar 2026 17:05:34 +0100 Subject: [PATCH 10/11] Record agentic invocations in User-Agent header Detect which AI coding agent is invoking gh by checking well-known environment variables and include the agent name in the User-Agent header sent to GitHub APIs. Supported agents: Codex, Gemini CLI, Copilot CLI, OpenCode, Claude Code, and Amp. Generic AI_AGENT env var is also supported with validation to prevent header injection. Fixes github/cli#1111 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- api/client.go | 1 - api/http_client.go | 8 +- api/http_client_test.go | 14 ++ internal/agents/detect.go | 99 ++++++++++++ internal/agents/detect_test.go | 149 ++++++++++++++++++ internal/ghcmd/cmd.go | 3 +- .../verify/verify_integration_test.go | 8 +- pkg/cmd/factory/default.go | 42 ++--- pkg/cmd/factory/default_test.go | 26 +-- pkg/cmd/search/shared/shared_test.go | 2 +- 10 files changed, 311 insertions(+), 41 deletions(-) create mode 100644 internal/agents/detect.go create mode 100644 internal/agents/detect_test.go diff --git a/api/client.go b/api/client.go index 207fd86d368..8ee525df59d 100644 --- a/api/client.go +++ b/api/client.go @@ -15,7 +15,6 @@ import ( ) const ( - accept = "Accept" apiVersion = "X-GitHub-Api-Version" apiVersionValue = "2022-11-28" authorization = "Authorization" diff --git a/api/http_client.go b/api/http_client.go index 9957f6bc51d..532f79c7f9d 100644 --- a/api/http_client.go +++ b/api/http_client.go @@ -18,6 +18,7 @@ type tokenGetter interface { type HTTPClientOptions struct { AppVersion string + InvokingAgent string CacheTTL time.Duration Config tokenGetter EnableCache bool @@ -48,8 +49,13 @@ func NewHTTPClient(opts HTTPClientOptions) (*http.Client, error) { clientOpts.LogVerboseHTTP = opts.LogVerboseHTTP } + ua := fmt.Sprintf("GitHub CLI %s", opts.AppVersion) + if opts.InvokingAgent != "" { + ua = fmt.Sprintf("%s Agent/%s", ua, opts.InvokingAgent) + } + headers := map[string]string{ - userAgent: fmt.Sprintf("GitHub CLI %s", opts.AppVersion), + userAgent: ua, apiVersion: apiVersionValue, } clientOpts.Headers = headers diff --git a/api/http_client_test.go b/api/http_client_test.go index 824bc0f1b13..1c81b4aa7a5 100644 --- a/api/http_client_test.go +++ b/api/http_client_test.go @@ -20,6 +20,7 @@ func TestNewHTTPClient(t *testing.T) { type args struct { config tokenGetter appVersion string + invokingAgent string logVerboseHTTP bool skipDefaultHeaders bool } @@ -155,6 +156,18 @@ func TestNewHTTPClient(t *testing.T) { * Request took `), }, + { + name: "includes invoking agent in user-agent header", + args: args{ + appVersion: "v1.2.3", + invokingAgent: "copilot-cli", + }, + host: "github.com", + wantHeader: map[string][]string{ + "user-agent": {"GitHub CLI v1.2.3 Agent/copilot-cli"}, + }, + wantStderr: "", + }, } var gotReq *http.Request @@ -169,6 +182,7 @@ func TestNewHTTPClient(t *testing.T) { ios, _, _, stderr := iostreams.Test() client, err := NewHTTPClient(HTTPClientOptions{ AppVersion: tt.args.appVersion, + InvokingAgent: tt.args.invokingAgent, Config: tt.args.config, Log: ios.ErrOut, LogVerboseHTTP: tt.args.logVerboseHTTP, diff --git a/internal/agents/detect.go b/internal/agents/detect.go new file mode 100644 index 00000000000..fd330ed67e3 --- /dev/null +++ b/internal/agents/detect.go @@ -0,0 +1,99 @@ +package agents + +import ( + "fmt" + "os" + "regexp" +) + +// AgentName is a validated agent identifier safe for use in HTTP headers. +type AgentName string + +const ( + agentAmp AgentName = "amp" + agentClaudeCode AgentName = "claude-code" + agentCodex AgentName = "codex" + agentCopilotCLI AgentName = "copilot-cli" + agentGeminiCLI AgentName = "gemini-cli" + agentOpencode AgentName = "opencode" +) + +var validAgentName = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) + +// parseAgentName validates and returns an AgentName from a raw string. +// Only alphanumeric characters, hyphens, and underscores are allowed. +func parseAgentName(s string) (AgentName, error) { + if !validAgentName.MatchString(s) { + return "", fmt.Errorf("invalid agent name %q: must match [a-zA-Z0-9_-]+", s) + } + return AgentName(s), nil +} + +// Detect returns the name of the AI coding agent driving the CLI, +// or an empty AgentName if none is detected. +func Detect() AgentName { + return detectWith(os.LookupEnv) +} + +func detectWith(lookup func(string) (string, bool)) AgentName { + isSet := func(key string) bool { + v, ok := lookup(key) + return ok && v != "" + } + + valueOf := func(key string) string { + v, _ := lookup(key) + return v + } + + // Generic agent identifiers — checked first because they are the most specific signal. + if v, ok := lookup("AI_AGENT"); ok && v != "" { + if name, err := parseAgentName(v); err == nil { + return name + } + } + + // Tool-specific variables. + + // Check AGENT=amp before the more generic CLAUDECODE=1 since Amp sets both. + if valueOf("AGENT") == "amp" { + return agentAmp + } + + // OpenAI Codex CLI — https://github.com/openai/codex + // CODEX_SANDBOX: https://github.com/openai/codex/blob/95e1d5993985019ce0ce0d10689caf1375f95120/codex-rs/core/src/spawn.rs#L25 + // CODEX_THREAD_ID: https://github.com/openai/codex/blob/95e1d5993985019ce0ce0d10689caf1375f95120/codex-rs/core/src/exec_env.rs#L8 + // CODEX_CI: https://github.com/openai/codex/blob/95e1d5993985019ce0ce0d10689caf1375f95120/codex-rs/core/src/unified_exec/process_manager.rs#L64 + if isSet("CODEX_SANDBOX") || isSet("CODEX_CI") || isSet("CODEX_THREAD_ID") { + return agentCodex + } + + // Google Gemini CLI — https://github.com/google-gemini/gemini-cli + // GEMINI_CLI: https://github.com/google-gemini/gemini-cli/blob/46fd7b4864111032a1c7dfa1821b2000fc7531da/docs/tools/shell.md#L96-L97 + if isSet("GEMINI_CLI") { + return agentGeminiCLI + } + + // GitHub Copilot CLI + // No first-party docs + if isSet("COPILOT_CLI") { + return agentCopilotCLI + } + + // OpenCode — https://github.com/anomalyco/opencode + // OPENCODE: https://github.com/anomalyco/opencode/blob/fde201c286a83ff32dda9b41d61d734a4449fe70/packages/opencode/src/index.ts#L78-L80 + if isSet("OPENCODE") { + return agentOpencode + } + + // Anthropic Claude Code — https://docs.anthropic.com/en/docs/agents-and-tools/claude-code/overview + // CLAUDECODE: https://code.claude.com/docs/en/env-vars (CLAUDECODE section) + // Checked last because other agents (e.g. Amp) set CLAUDECODE=1 alongside their own vars. + if isSet("CLAUDECODE") { + // There is a CLAUDE_CODE_ENTRYPOINT env var that is set to `cli` or `desktop` etc, but it's not documented + // so we don't want to rely on it too heavily. We'll just return a generic claude-code agent name. + return agentClaudeCode + } + + return "" +} diff --git a/internal/agents/detect_test.go b/internal/agents/detect_test.go new file mode 100644 index 00000000000..6cb4f461195 --- /dev/null +++ b/internal/agents/detect_test.go @@ -0,0 +1,149 @@ +package agents + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func lookup(vars map[string]string) func(string) (string, bool) { + return func(key string) (string, bool) { + v, ok := vars[key] + return v, ok + } +} + +func TestParseAgentName(t *testing.T) { + tests := []struct { + name string + input string + want AgentName + wantErr bool + }{ + {name: "valid lowercase", input: "my-agent", want: "my-agent"}, + {name: "valid with underscore", input: "my_agent_v2", want: "my_agent_v2"}, + {name: "valid uppercase", input: "MyAgent", want: "MyAgent"}, + {name: "valid numbers", input: "agent123", want: "agent123"}, + {name: "spaces rejected", input: "my agent", wantErr: true}, + {name: "newline rejected", input: "my\nagent", wantErr: true}, + {name: "carriage return rejected", input: "my\ragent", wantErr: true}, + {name: "null byte rejected", input: "my\x00agent", wantErr: true}, + {name: "dot rejected", input: "my.agent", wantErr: true}, + {name: "slash rejected", input: "my/agent", wantErr: true}, + {name: "empty rejected", input: "", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseAgentName(tt.input) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tt.want, got) + } + }) + } +} + +func TestDetectWith(t *testing.T) { + tests := []struct { + name string + env map[string]string + wantAgent AgentName + }{ + { + name: "clean environment", + env: map[string]string{}, + wantAgent: "", + }, + { + name: "empty var is not detected", + env: map[string]string{"GEMINI_CLI": ""}, + wantAgent: "", + }, + { + name: "AGENT=amp detected as amp", + env: map[string]string{"AGENT": "amp"}, + wantAgent: "amp", + }, + { + name: "AGENT with non-amp value is ignored", + env: map[string]string{"AGENT": "other"}, + wantAgent: "", + }, + { + name: "AI_AGENT returns value as agent name", + env: map[string]string{"AI_AGENT": "some-agent"}, + wantAgent: "some-agent", + }, + { + name: "AI_AGENT with invalid characters is ignored", + env: map[string]string{"AI_AGENT": "bad\nagent"}, + wantAgent: "", + }, + { + name: "AI_AGENT with spaces is ignored", + env: map[string]string{"AI_AGENT": "bad agent"}, + wantAgent: "", + }, + { + name: "AI_AGENT takes priority over AGENT", + env: map[string]string{"AGENT": "amp", "AI_AGENT": "other"}, + wantAgent: "other", + }, + { + name: "CODEX_SANDBOX", + env: map[string]string{"CODEX_SANDBOX": "seatbelt"}, + wantAgent: "codex", + }, + { + name: "CODEX_CI", + env: map[string]string{"CODEX_CI": "1"}, + wantAgent: "codex", + }, + { + name: "CODEX_THREAD_ID", + env: map[string]string{"CODEX_THREAD_ID": "abc"}, + wantAgent: "codex", + }, + { + name: "GEMINI_CLI", + env: map[string]string{"GEMINI_CLI": "1"}, + wantAgent: "gemini-cli", + }, + { + name: "COPILOT_CLI", + env: map[string]string{"COPILOT_CLI": "1"}, + wantAgent: "copilot-cli", + }, + { + name: "OPENCODE", + env: map[string]string{"OPENCODE": "1"}, + wantAgent: "opencode", + }, + { + name: "CLAUDECODE", + env: map[string]string{"CLAUDECODE": "1"}, + wantAgent: "claude-code", + }, + { + name: "AGENT=amp takes priority over CLAUDECODE", + env: map[string]string{"AGENT": "amp", "CLAUDECODE": "1"}, + wantAgent: "amp", + }, + { + name: "invalid AI_AGENT falls through to tool-specific detection", + env: map[string]string{"AI_AGENT": "bad agent", "GEMINI_CLI": "1"}, + wantAgent: "gemini-cli", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := detectWith(lookup(tt.env)) + assert.Equal(t, tt.wantAgent, got) + }) + } +} diff --git a/internal/ghcmd/cmd.go b/internal/ghcmd/cmd.go index 9fc5bcefeaf..8690078c66e 100644 --- a/internal/ghcmd/cmd.go +++ b/internal/ghcmd/cmd.go @@ -15,6 +15,7 @@ import ( surveyCore "github.com/AlecAivazis/survey/v2/core" "github.com/AlecAivazis/survey/v2/terminal" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/agents" "github.com/cli/cli/v2/internal/build" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/config/migration" @@ -44,7 +45,7 @@ func Main() exitCode { buildVersion := build.Version hasDebug, _ := utils.IsDebugEnabled() - cmdFactory := factory.New(buildVersion) + cmdFactory := factory.New(buildVersion, string(agents.Detect())) stderr := cmdFactory.IOStreams.ErrOut ctx := context.Background() diff --git a/pkg/cmd/attestation/verify/verify_integration_test.go b/pkg/cmd/attestation/verify/verify_integration_test.go index d77f21f70d0..ec64cefa7a3 100644 --- a/pkg/cmd/attestation/verify/verify_integration_test.go +++ b/pkg/cmd/attestation/verify/verify_integration_test.go @@ -26,7 +26,7 @@ func TestVerifyIntegration(t *testing.T) { TUFMetadataDir: o.Some(t.TempDir()), } - cmdFactory := factory.New("test") + cmdFactory := factory.New("test", "") hc, err := cmdFactory.HttpClient() if err != nil { @@ -143,7 +143,7 @@ func TestVerifyIntegrationCustomIssuer(t *testing.T) { TUFMetadataDir: o.Some(t.TempDir()), } - cmdFactory := factory.New("test") + cmdFactory := factory.New("test", "") hc, err := cmdFactory.HttpClient() if err != nil { @@ -217,7 +217,7 @@ func TestVerifyIntegrationReusableWorkflow(t *testing.T) { TUFMetadataDir: o.Some(t.TempDir()), } - cmdFactory := factory.New("test") + cmdFactory := factory.New("test", "") hc, err := cmdFactory.HttpClient() if err != nil { @@ -310,7 +310,7 @@ func TestVerifyIntegrationReusableWorkflowSignerWorkflow(t *testing.T) { TUFMetadataDir: o.Some(t.TempDir()), } - cmdFactory := factory.New("test") + cmdFactory := factory.New("test", "") hc, err := cmdFactory.HttpClient() if err != nil { diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index b4884038754..48ec0c8fe0c 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -26,23 +26,23 @@ import ( var ssoHeader string var ssoURLRE = regexp.MustCompile(`\burl=([^;]+)`) -func New(appVersion string) *cmdutil.Factory { +func New(appVersion string, invokingAgent string) *cmdutil.Factory { f := &cmdutil.Factory{ AppVersion: appVersion, Config: configFunc(), // No factory dependencies ExecutableName: "gh", } - f.IOStreams = ioStreams(f) // Depends on Config - f.HttpClient = httpClientFunc(f, appVersion) // Depends on Config, IOStreams, and appVersion - f.PlainHttpClient = plainHttpClientFunc(f, appVersion) // Depends on IOStreams, and appVersion - f.GitClient = newGitClient(f) // Depends on IOStreams, and Executable - f.Remotes = remotesFunc(f) // Depends on Config, and GitClient - f.BaseRepo = BaseRepoFunc(f) // Depends on Remotes - f.Prompter = newPrompter(f) // Depends on Config and IOStreams - f.Browser = newBrowser(f) // Depends on Config, and IOStreams - f.ExtensionManager = extensionManager(f) // Depends on Config, HttpClient, and IOStreams - f.Branch = branchFunc(f) // Depends on GitClient + f.IOStreams = ioStreams(f) // Depends on Config + f.HttpClient = httpClientFunc(f, appVersion, invokingAgent) // Depends on Config, IOStreams, appVersion, and invokingAgent + f.PlainHttpClient = plainHttpClientFunc(f, appVersion, invokingAgent) // Depends on IOStreams, appVersion, and invokingAgent + f.GitClient = newGitClient(f) // Depends on IOStreams, and Executable + f.Remotes = remotesFunc(f) // Depends on Config, and GitClient + f.BaseRepo = BaseRepoFunc(f) // Depends on Remotes + f.Prompter = newPrompter(f) // Depends on Config and IOStreams + f.Browser = newBrowser(f) // Depends on Config, and IOStreams + f.ExtensionManager = extensionManager(f) // Depends on Config, HttpClient, and IOStreams + f.Branch = branchFunc(f) // Depends on GitClient return f } @@ -186,7 +186,7 @@ func remotesFunc(f *cmdutil.Factory) func() (ghContext.Remotes, error) { return rr.Resolver() } -func httpClientFunc(f *cmdutil.Factory, appVersion string) func() (*http.Client, error) { +func httpClientFunc(f *cmdutil.Factory, appVersion string, invokingAgent string) func() (*http.Client, error) { return func() (*http.Client, error) { io := f.IOStreams cfg, err := f.Config() @@ -194,10 +194,11 @@ func httpClientFunc(f *cmdutil.Factory, appVersion string) func() (*http.Client, return nil, err } opts := api.HTTPClientOptions{ - Config: cfg.Authentication(), - Log: io.ErrOut, - LogColorize: io.ColorEnabled(), - AppVersion: appVersion, + Config: cfg.Authentication(), + Log: io.ErrOut, + LogColorize: io.ColorEnabled(), + AppVersion: appVersion, + InvokingAgent: invokingAgent, } client, err := api.NewHTTPClient(opts) if err != nil { @@ -208,13 +209,14 @@ func httpClientFunc(f *cmdutil.Factory, appVersion string) func() (*http.Client, } } -func plainHttpClientFunc(f *cmdutil.Factory, appVersion string) func() (*http.Client, error) { +func plainHttpClientFunc(f *cmdutil.Factory, appVersion string, invokingAgent string) func() (*http.Client, error) { return func() (*http.Client, error) { io := f.IOStreams opts := api.HTTPClientOptions{ - Log: io.ErrOut, - LogColorize: io.ColorEnabled(), - AppVersion: appVersion, + Log: io.ErrOut, + LogColorize: io.ColorEnabled(), + AppVersion: appVersion, + InvokingAgent: invokingAgent, // This is required to prevent automatic setting of auth and other headers. SkipDefaultHeaders: true, } diff --git a/pkg/cmd/factory/default_test.go b/pkg/cmd/factory/default_test.go index 7e0ed4af6f5..7d84caa8f26 100644 --- a/pkg/cmd/factory/default_test.go +++ b/pkg/cmd/factory/default_test.go @@ -66,7 +66,7 @@ func Test_BaseRepo(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - f := New("1") + f := New("1", "") rr := &remoteResolver{ readRemotes: func() (git.RemoteSet, error) { return tt.remotes, nil @@ -204,7 +204,7 @@ func Test_SmartBaseRepo(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - f := New("1") + f := New("1", "") rr := &remoteResolver{ readRemotes: func() (git.RemoteSet, error) { return tt.remotes, nil @@ -297,7 +297,7 @@ func Test_OverrideBaseRepo(t *testing.T) { if tt.envOverride != "" { t.Setenv("GH_REPO", tt.envOverride) } - f := New("1") + f := New("1", "") rr := &remoteResolver{ readRemotes: func() (git.RemoteSet, error) { return tt.remotes, nil @@ -375,7 +375,7 @@ func Test_ioStreams_pager(t *testing.T) { t.Setenv(k, v) } } - f := New("1") + f := New("1", "") f.Config = func() (gh.Config, error) { if tt.config == nil { return config.NewBlankConfig(), nil @@ -418,7 +418,7 @@ func Test_ioStreams_prompt(t *testing.T) { t.Setenv(k, v) } } - f := New("1") + f := New("1", "") f.Config = func() (gh.Config, error) { if tt.config == nil { return config.NewBlankConfig(), nil @@ -496,7 +496,7 @@ func Test_ioStreams_spinnerDisabled(t *testing.T) { for k, v := range tt.env { t.Setenv(k, v) } - f := New("1") + f := New("1", "") f.Config = func() (gh.Config, error) { if tt.config == nil { return config.NewBlankConfig(), nil @@ -564,7 +564,7 @@ func Test_ioStreams_accessiblePrompterEnabled(t *testing.T) { for k, v := range tt.env { t.Setenv(k, v) } - f := New("1") + f := New("1", "") f.Config = func() (gh.Config, error) { if tt.config == nil { return config.NewBlankConfig(), nil @@ -642,7 +642,7 @@ func Test_ioStreams_colorLabels(t *testing.T) { t.Setenv(k, v) } } - f := New("1") + f := New("1", "") f.Config = func() (gh.Config, error) { if tt.config == nil { return config.NewBlankConfig(), nil @@ -683,13 +683,13 @@ func TestSSOURL(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - f := New("1") + f := New("1", "") f.Config = func() (gh.Config, error) { return config.NewBlankConfig(), nil } ios, _, _, stderr := iostreams.Test() f.IOStreams = ios - client, err := httpClientFunc(f, "v1.2.3")() + client, err := httpClientFunc(f, "v1.2.3", "")() require.NoError(t, err) req, err := http.NewRequest("GET", ts.URL, nil) if tt.sso != "" { @@ -718,13 +718,13 @@ func TestPlainHttpClient(t *testing.T) { })) defer ts.Close() - f := New("1") + f := New("1", "") f.Config = func() (gh.Config, error) { return config.NewBlankConfig(), nil } ios, _, _, _ := iostreams.Test() f.IOStreams = ios - client, err := plainHttpClientFunc(f, "v1.2.3")() + client, err := plainHttpClientFunc(f, "v1.2.3", "")() require.NoError(t, err) req, err := http.NewRequest("GET", ts.URL, nil) @@ -759,7 +759,7 @@ func TestNewGitClient(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - f := New("1") + f := New("1", "") f.Config = func() (gh.Config, error) { if tt.config == nil { return config.NewBlankConfig(), nil diff --git a/pkg/cmd/search/shared/shared_test.go b/pkg/cmd/search/shared/shared_test.go index 0700a688e9b..c66e0908f52 100644 --- a/pkg/cmd/search/shared/shared_test.go +++ b/pkg/cmd/search/shared/shared_test.go @@ -15,7 +15,7 @@ import ( ) func TestSearcher(t *testing.T) { - f := factory.New("1") + f := factory.New("1", "") f.Config = func() (gh.Config, error) { return config.NewBlankConfig(), nil } 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 11/11] 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 2a3dda638e7..f9fab11a1c7 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 1e13146c233..02fe5d33c50 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 {