From c69f82801985ed22406c226602f564567f6153f0 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 2 Jul 2025 16:22:12 -0400 Subject: [PATCH 01/11] Initial assign Copilot during issue create changes - updated command documentation for new `--assignee @copilot` syntax - updated `gh issue create` logic with feature detection for actor availability - fixed bug where `--assignee` would not select default assignees due to login vs display name mismatch - updated survey prompt logic to avoid parsing prompt options in favor of correlating with assignee / actor source One thing that is not included in these changes is incorporating the GraphQL mutation to replace assignees with actors, which I will continue iterating on. --- pkg/cmd/issue/create/create.go | 33 +++++++++++++++---- pkg/cmd/pr/shared/state.go | 3 +- pkg/cmd/pr/shared/survey.go | 58 ++++++++++++++++++++++++---------- 3 files changed, 70 insertions(+), 24 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 2978a21fc7b..b8a1782658c 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -16,6 +16,7 @@ import ( prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/pkg/set" "github.com/spf13/cobra" ) @@ -68,6 +69,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co Adding an issue to projects requires authorization with the %[1]sproject%[1]s scope. To authorize, run %[1]sgh auth refresh -s project%[1]s. + + The %[1]s--assignee%[1]s flag supports the following special values: + - %[1]s@me%[1]s: assign yourself + - %[1]s@copilot%[1]s: assign Copilot (not supported on GitHub Enterprise Server) `, "`"), Example: heredoc.Doc(` $ gh issue create --title "I found a bug" --body "Nothing works" @@ -75,6 +80,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co $ gh issue create --label bug --label "help wanted" $ gh issue create --assignee monalisa,hubot $ gh issue create --assignee "@me" + $ gh issue create --assignee "@copilot" $ gh issue create --project "Roadmap" $ gh issue create --template "Bug Report" `), @@ -158,6 +164,10 @@ func createRun(opts *CreateOptions) (err error) { } projectsV1Support := opts.Detector.ProjectsV1() + issueFeatures, err := opts.Detector.IssueFeatures() + if err != nil { + return err + } isTerminal := opts.IO.IsStdoutTTY() @@ -166,20 +176,29 @@ func createRun(opts *CreateOptions) (err error) { milestones = []string{opts.Milestone} } + // Replace special values in assignees + assigneeSet := set.NewStringSet() meReplacer := prShared.NewMeReplacer(apiClient, baseRepo.RepoHost()) + copilotReplacer := prShared.NewCopilotReplacer() assignees, err := meReplacer.ReplaceSlice(opts.Assignees) if err != nil { return err } + if issueFeatures.ActorIsAssignable { + assignees = copilotReplacer.ReplaceSlice(assignees) + } + assigneeSet.AddValues(assignees) + tb := prShared.IssueMetadataState{ - Type: prShared.IssueMetadata, - Assignees: assignees, - Labels: opts.Labels, - ProjectTitles: opts.Projects, - Milestones: milestones, - Title: opts.Title, - Body: opts.Body, + Type: prShared.IssueMetadata, + ActorAssignees: issueFeatures.ActorIsAssignable, + Assignees: assigneeSet.ToSlice(), + Labels: opts.Labels, + ProjectTitles: opts.Projects, + Milestones: milestones, + Title: opts.Title, + Body: opts.Body, } if opts.RecoverFile != "" { diff --git a/pkg/cmd/pr/shared/state.go b/pkg/cmd/pr/shared/state.go index 7e7da436d03..b9f2c029372 100644 --- a/pkg/cmd/pr/shared/state.go +++ b/pkg/cmd/pr/shared/state.go @@ -18,7 +18,8 @@ const ( type IssueMetadataState struct { Type metadataStateType - Draft bool + Draft bool + ActorAssignees bool Body string Title string diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index b6c927a2d9b..565242f0c41 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -3,6 +3,7 @@ package shared import ( "errors" "fmt" + "slices" "strings" "github.com/cli/cli/v2/api" @@ -177,13 +178,15 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface state.Metadata = append(state.Metadata, extraFieldsOptions[i]) } + // Retrieve and process data for survey prompts based on the extra fields selected metadataInput := api.RepoMetadataInput{ - Reviewers: isChosen("Reviewers"), - Assignees: isChosen("Assignees"), - Labels: isChosen("Labels"), - ProjectsV1: isChosen("Projects") && projectsV1Support == gh.ProjectsV1Supported, - ProjectsV2: isChosen("Projects"), - Milestones: isChosen("Milestone"), + Reviewers: isChosen("Reviewers"), + Assignees: isChosen("Assignees"), + ActorAssignees: isChosen("Assignees") && state.ActorAssignees, + Labels: isChosen("Labels"), + ProjectsV1: isChosen("Projects") && projectsV1Support == gh.ProjectsV1Supported, + ProjectsV2: isChosen("Projects"), + Milestones: isChosen("Milestone"), } metadataResult, err := fetcher.RepoMetadataFetch(metadataInput) if err != nil { @@ -199,9 +202,27 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface for _, t := range metadataResult.Teams { reviewers = append(reviewers, fmt.Sprintf("%s/%s", baseRepo.RepoOwner(), t.Slug)) } + + // In order to select any provided assignee, the following code needs to take the provided default assignees in `state` + // and translate them to the appropriate actor / user. var assignees []string - for _, u := range metadataResult.AssignableUsers { - assignees = append(assignees, u.DisplayName()) + var assigneesDefault []string + 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()) + } + } + } else { + for _, u := range metadataResult.AssignableUsers { + assignees = append(assignees, u.DisplayName()) + + if slices.Contains(state.Assignees, u.Login()) { + assigneesDefault = append(assigneesDefault, u.DisplayName()) + } + } } var labels []string for _, l := range metadataResult.Labels { @@ -219,6 +240,7 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface milestones = append(milestones, m.Title) } + // Prompt user for additional metadata based on selected fields values := struct { Reviewers []string Assignees []string @@ -242,12 +264,20 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface } if isChosen("Assignees") { if len(assignees) > 0 { - selected, err := p.MultiSelect("Assignees", state.Assignees, assignees) + selected, err := p.MultiSelect("Assignees", assigneesDefault, assignees) if err != nil { return err } for _, i := range selected { - values.Assignees = append(values.Assignees, assignees[i]) + // 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 { + values.Assignees = append(values.Assignees, metadataResult.AssignableUsers[i].Login()) + } } } else { fmt.Fprintln(io.ErrOut, "warning: no assignable users") @@ -297,6 +327,7 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface } } + // Update issue / pull request metadata state if isChosen("Reviewers") { var logins []string for _, r := range values.Reviewers { @@ -306,12 +337,7 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface state.Reviewers = logins } if isChosen("Assignees") { - var logins []string - for _, a := range values.Assignees { - // Extract user login from display name - logins = append(logins, (strings.Split(a, " "))[0]) - } - state.Assignees = logins + state.Assignees = values.Assignees } if isChosen("Labels") { state.Labels = values.Labels From 49821b20f9157a7ac795ed1c6e72fd7023e6a900 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Tue, 8 Jul 2025 15:19:09 -0400 Subject: [PATCH 02/11] Add non-TTY issue create copilot test, todos --- api/queries_repo.go | 3 +++ pkg/cmd/issue/create/create_test.go | 41 +++++++++++++++++++++++++++++ pkg/cmd/pr/shared/params.go | 1 + 3 files changed, 45 insertions(+) diff --git a/api/queries_repo.go b/api/queries_repo.go index 3190745ea99..5d3475b4f80 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -1063,6 +1063,7 @@ func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoRes } } + // TODO: Should this retrieve actors separate from `user`? // there is no way to look up projects nor milestones by name, so preload them all mi := RepoMetadataInput{ ProjectsV1: input.ProjectsV1, @@ -1079,6 +1080,8 @@ func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoRes query := &bytes.Buffer{} fmt.Fprint(query, "query RepositoryResolveMetadataIDs {\n") + // TODO: Should we really be using the `user` query to lookup bots even though it technically works? + // This logic is used to lookup information about assignees that were explicitly defined upfront for i, u := range users { fmt.Fprintf(query, "u%03d: user(login:%q){id,login}\n", i, u) } diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 1211c0c1d93..d5972e3c148 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -942,6 +942,47 @@ func TestIssueCreate_AtMeAssignee(t *testing.T) { assert.Equal(t, "https://github.com/OWNER/REPO/issues/12\n", output.String()) } +// TODO: Create an interactive variant of this test, which will ensure that Copilot is in the assignee list and can be selected +func TestIssueCreate_AtCopilotAssigneeNonInteractive(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } } + `)) + http.Register( + httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.StringResponse(` + { "data": { + "u000": { "login": "copilot-swe-agent", "id": "COPILOTID" } + } } + `), + ) + http.Register( + httpmock.GraphQL(`mutation IssueCreate\b`), + httpmock.GraphQLMutation(` + { "data": { "createIssue": { "issue": { + "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"]) + })) + + output, err := runCommand(http, true, `-a @copilot -t hello -b "cash rules everything around me"`, nil) + if err != nil { + t.Errorf("error running command `issue create`: %v", err) + } + + assert.Equal(t, "https://github.com/OWNER/REPO/issues/12\n", output.String()) +} + func TestIssueCreate_projectsV2(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index 1fa45652abd..8483280b069 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -107,6 +107,7 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par return err } + // TODO: Think we need to adapt the logic in `Editable.AssigneeIds()` which is called by `UpdateIssue()` here to replace @me and @copilot assigneeIDs, err := tb.MetadataResult.MembersToIDs(tb.Assignees) if err != nil { return fmt.Errorf("could not assign user: %w", err) From a8053d19a91adaaa0afdcec70f5291b346a2bbc6 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Fri, 11 Jul 2025 10:21:17 -0400 Subject: [PATCH 03/11] Resolve issue and PR metadata consistently This commit refactors how `gh issue create` and `gh pr create` retrieve information needed to resolve metadata to be more in line with the approach used in `gh issue edit` and `gh pr edit`. Previously, both commands used `prshared.fillMetadata(...)` function to retrieve assignees, reviewers, labels, and teams outside of `api.RepoMetadata(..)`. Now, these commands will consistently use the same logic and data for resolving metadata. --- api/queries_repo.go | 111 ------------------------------------ api/queries_repo_test.go | 82 -------------------------- pkg/cmd/pr/shared/params.go | 63 ++++++-------------- 3 files changed, 17 insertions(+), 239 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index 5d3475b4f80..8480e3868d6 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -1033,117 +1033,6 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput return &result, nil } -type RepoResolveInput struct { - Assignees []string - Reviewers []string - Labels []string - ProjectsV1 bool - ProjectsV2 bool - Milestones []string -} - -// RepoResolveMetadataIDs looks up GraphQL node IDs in bulk -func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoResolveInput) (*RepoMetadataResult, error) { - users := input.Assignees - hasUser := func(target string) bool { - for _, u := range users { - if strings.EqualFold(u, target) { - return true - } - } - return false - } - - var teams []string - for _, r := range input.Reviewers { - if i := strings.IndexRune(r, '/'); i > -1 { - teams = append(teams, r[i+1:]) - } else if !hasUser(r) { - users = append(users, r) - } - } - - // TODO: Should this retrieve actors separate from `user`? - // there is no way to look up projects nor milestones by name, so preload them all - mi := RepoMetadataInput{ - ProjectsV1: input.ProjectsV1, - ProjectsV2: input.ProjectsV2, - Milestones: len(input.Milestones) > 0, - } - result, err := RepoMetadata(client, repo, mi) - if err != nil { - return result, err - } - if len(users) == 0 && len(teams) == 0 && len(input.Labels) == 0 { - return result, nil - } - - query := &bytes.Buffer{} - fmt.Fprint(query, "query RepositoryResolveMetadataIDs {\n") - // TODO: Should we really be using the `user` query to lookup bots even though it technically works? - // This logic is used to lookup information about assignees that were explicitly defined upfront - for i, u := range users { - fmt.Fprintf(query, "u%03d: user(login:%q){id,login}\n", i, u) - } - if len(input.Labels) > 0 { - fmt.Fprintf(query, "repository(owner:%q,name:%q){\n", repo.RepoOwner(), repo.RepoName()) - for i, l := range input.Labels { - fmt.Fprintf(query, "l%03d: label(name:%q){id,name}\n", i, l) - } - fmt.Fprint(query, "}\n") - } - if len(teams) > 0 { - fmt.Fprintf(query, "organization(login:%q){\n", repo.RepoOwner()) - for i, t := range teams { - fmt.Fprintf(query, "t%03d: team(slug:%q){id,slug}\n", i, t) - } - fmt.Fprint(query, "}\n") - } - fmt.Fprint(query, "}\n") - - response := make(map[string]json.RawMessage) - err = client.GraphQL(repo.RepoHost(), query.String(), nil, &response) - if err != nil { - return result, err - } - - for key, v := range response { - switch key { - case "repository": - repoResponse := make(map[string]RepoLabel) - err := json.Unmarshal(v, &repoResponse) - if err != nil { - return result, err - } - for _, l := range repoResponse { - result.Labels = append(result.Labels, l) - } - case "organization": - orgResponse := make(map[string]OrgTeam) - err := json.Unmarshal(v, &orgResponse) - if err != nil { - return result, err - } - for _, t := range orgResponse { - result.Teams = append(result.Teams, t) - } - default: - user := struct { - Id string - Login string - Name string - }{} - err := json.Unmarshal(v, &user) - if err != nil { - return result, err - } - result.AssignableUsers = append(result.AssignableUsers, NewAssignableUser(user.Id, user.Login, user.Name)) - } - } - - return result, nil -} - type RepoProject struct { ID string `json:"id"` Name string `json:"name"` diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index c291fc4686a..c885b99682a 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -379,88 +379,6 @@ func Test_ProjectNamesToPaths(t *testing.T) { }) } -func Test_RepoResolveMetadataIDs(t *testing.T) { - http := &httpmock.Registry{} - client := newTestClient(http) - - repo, _ := ghrepo.FromFullName("OWNER/REPO") - input := RepoResolveInput{ - Assignees: []string{"monalisa", "hubot"}, - Reviewers: []string{"monalisa", "octocat", "OWNER/core", "/robots"}, - Labels: []string{"bug", "help wanted"}, - } - - expectedQuery := `query RepositoryResolveMetadataIDs { -u000: user(login:"monalisa"){id,login} -u001: user(login:"hubot"){id,login} -u002: user(login:"octocat"){id,login} -repository(owner:"OWNER",name:"REPO"){ -l000: label(name:"bug"){id,name} -l001: label(name:"help wanted"){id,name} -} -organization(login:"OWNER"){ -t000: team(slug:"core"){id,slug} -t001: team(slug:"robots"){id,slug} -} -} -` - responseJSON := ` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "u001": { "login": "hubot", "id": "HUBOTID" }, - "u002": { "login": "octocat", "id": "OCTOID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "Help Wanted", "id": "HELPID" } - }, - "organization": { - "t000": { "slug": "core", "id": "COREID" }, - "t001": { "slug": "Robots", "id": "ROBOTID" } - } - } } - ` - - http.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), - httpmock.GraphQLQuery(responseJSON, func(q string, _ map[string]interface{}) { - if q != expectedQuery { - t.Errorf("expected query %q, got %q", expectedQuery, q) - } - })) - - result, err := RepoResolveMetadataIDs(client, repo, input) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - expectedMemberIDs := []string{"MONAID", "HUBOTID", "OCTOID"} - memberIDs, err := result.MembersToIDs([]string{"monalisa", "hubot", "octocat"}) - if err != nil { - t.Errorf("error resolving members: %v", err) - } - if !sliceEqual(memberIDs, expectedMemberIDs) { - t.Errorf("expected members %v, got %v", expectedMemberIDs, memberIDs) - } - - expectedTeamIDs := []string{"COREID", "ROBOTID"} - teamIDs, err := result.TeamsToIDs([]string{"/core", "/robots"}) - if err != nil { - t.Errorf("error resolving teams: %v", err) - } - if !sliceEqual(teamIDs, expectedTeamIDs) { - t.Errorf("expected members %v, got %v", expectedTeamIDs, teamIDs) - } - - expectedLabelIDs := []string{"BUGID", "HELPID"} - labelIDs, err := result.LabelsToIDs([]string{"bug", "help wanted"}) - if err != nil { - t.Errorf("error resolving labels: %v", err) - } - if !sliceEqual(labelIDs, expectedLabelIDs) { - t.Errorf("expected members %v, got %v", expectedLabelIDs, labelIDs) - } -} - func TestMembersToIDs(t *testing.T) { t.Parallel() diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index 8483280b069..61d39611b68 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -55,59 +55,30 @@ func ValidURL(urlStr string) bool { return len(urlStr) < 8192 } -// Ensure that tb.MetadataResult object exists and contains enough pre-fetched API data to be able -// to resolve all object listed in tb to GraphQL IDs. -func fillMetadata(client *api.Client, baseRepo ghrepo.Interface, tb *IssueMetadataState, projectV1Support gh.ProjectsV1Support) error { - resolveInput := api.RepoResolveInput{} - - if len(tb.Assignees) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.AssignableUsers) == 0) { - resolveInput.Assignees = tb.Assignees - } - - if len(tb.Reviewers) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.AssignableUsers) == 0) { - resolveInput.Reviewers = tb.Reviewers - } - - if len(tb.Labels) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Labels) == 0) { - resolveInput.Labels = tb.Labels - } - - if len(tb.ProjectTitles) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Projects) == 0) { - if projectV1Support == gh.ProjectsV1Supported { - resolveInput.ProjectsV1 = true - } - - resolveInput.ProjectsV2 = true - } - - if len(tb.Milestones) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Milestones) == 0) { - resolveInput.Milestones = tb.Milestones - } - - metadataResult, err := api.RepoResolveMetadataIDs(client, baseRepo, resolveInput) - if err != nil { - return err - } - - if tb.MetadataResult == nil { - tb.MetadataResult = metadataResult - } else { - tb.MetadataResult.Merge(metadataResult) - } - - return nil -} - func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, params map[string]interface{}, tb *IssueMetadataState, projectV1Support gh.ProjectsV1Support) error { if !tb.HasMetadata() { return nil } - if err := fillMetadata(client, baseRepo, tb, projectV1Support); err != nil { - return err + // Retrieve minimal information needed to resolve metadata if this was not previously cached from additional metadata survey. + if tb.MetadataResult == nil { + input := api.RepoMetadataInput{ + Reviewers: len(tb.Reviewers) > 0, + 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, + } + + metadataResult, err := api.RepoMetadata(client, baseRepo, input) + if err != nil { + return err + } + tb.MetadataResult = metadataResult } - // TODO: Think we need to adapt the logic in `Editable.AssigneeIds()` which is called by `UpdateIssue()` here to replace @me and @copilot assigneeIDs, err := tb.MetadataResult.MembersToIDs(tb.Assignees) if err != nil { return fmt.Errorf("could not assign user: %w", err) From 554fe0f51f3d45b14163327dd209bd836c14710d Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Fri, 11 Jul 2025 12:19:36 -0400 Subject: [PATCH 04/11] Fix `gh issue create` tests, copilot web assign This commit is primarily focused on fixing the existing `gh issue create` tests with the changes to the underlying queries retrieving the data for resolving metadata. Additionally, a new test case for `gh issue create --web --assignee @copilot` was added to mirror existing tests. While exercising this capability, I found out the web UI apparently wants `copilot` instead of the `copilot-swe-agent` login, so changes to address that were a part of this commit. --- pkg/cmd/issue/create/create.go | 2 +- pkg/cmd/issue/create/create_test.go | 86 ++++++++++++++++++----------- pkg/cmd/pr/shared/editable.go | 2 +- pkg/cmd/pr/shared/params.go | 20 +++++-- pkg/cmd/pr/shared/params_test.go | 19 +++++-- 5 files changed, 83 insertions(+), 46 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index b8a1782658c..db3bd72014e 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -179,7 +179,7 @@ func createRun(opts *CreateOptions) (err error) { // Replace special values in assignees assigneeSet := set.NewStringSet() meReplacer := prShared.NewMeReplacer(apiClient, baseRepo.RepoHost()) - copilotReplacer := prShared.NewCopilotReplacer() + copilotReplacer := prShared.NewCopilotReplacer(opts.WebMode) assignees, err := meReplacer.ReplaceSlice(opts.Assignees) if err != nil { return err diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index d5972e3c148..1c9106ec425 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -305,6 +305,15 @@ func Test_createRun(t *testing.T) { wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=", wantsStderr: "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n", }, + { + name: "@copilot", + opts: CreateOptions{ + WebMode: true, + Assignees: []string{"@copilot"}, + }, + wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=copilot&body=", + wantsStderr: "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n", + }, { name: "project", opts: CreateOptions{ @@ -589,16 +598,17 @@ func TestIssueCreate_recover(t *testing.T) { "id": "REPOID", "hasIssuesEnabled": true } } }`)) + // Should only be one fetch of metadata. http.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query RepositoryLabelList\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "TODO", "id": "TODOID" } - } - } } + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } `)) http.Register( httpmock.GraphQL(`mutation IssueCreate\b`), @@ -777,15 +787,25 @@ func TestIssueCreate_metadata(t *testing.T) { http.StubRepoInfoResponse("OWNER", "REPO", "main") http.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query RepositoryAssignableActors\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "TODO", "id": "TODOID" } - } - } } + { "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(` + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } `)) http.Register( httpmock.GraphQL(`query RepositoryMilestoneList\b`), @@ -910,18 +930,16 @@ func TestIssueCreate_AtMeAssignee(t *testing.T) { } } } `)) http.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query RepositoryAssignableActors\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "u001": { "login": "SomeOneElse", "id": "SOMEID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "TODO", "id": "TODOID" } - } - } } - `), - ) + { "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(` @@ -956,13 +974,15 @@ func TestIssueCreate_AtCopilotAssigneeNonInteractive(t *testing.T) { } } } `)) http.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query RepositoryAssignableActors\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "copilot-swe-agent", "id": "COPILOTID" } - } } - `), - ) + { "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(` diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 2f51f2ae814..4e3bfab4cb2 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -118,7 +118,7 @@ func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]str // 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() + copilotReplacer := NewCopilotReplacer(false) replaceSpecialAssigneeNames := func(value []string) ([]string, error) { replaced, err := meReplacer.ReplaceSlice(value) diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index 61d39611b68..505d3a0a5ec 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -286,17 +286,25 @@ func (r *MeReplacer) ReplaceSlice(handles []string) ([]string, error) { } // CopilotReplacer resolves usages of `@copilot` to Copilot's login. -type CopilotReplacer struct{} +type CopilotReplacer struct { + webMode bool +} -func NewCopilotReplacer() *CopilotReplacer { - return &CopilotReplacer{} +func NewCopilotReplacer(webMode bool) *CopilotReplacer { + return &CopilotReplacer{ + webMode: webMode, + } } func (r *CopilotReplacer) replace(handle string) string { - if strings.EqualFold(handle, "@copilot") { - return api.CopilotActorLogin + if !strings.EqualFold(handle, "@copilot") { + return handle + } + // When Copilot is used as an assignee in web mode, the login is not used. + if r.webMode { + return "copilot" } - return handle + return api.CopilotActorLogin } // ReplaceSlice replaces usages of `@copilot` in a slice with Copilot's login. diff --git a/pkg/cmd/pr/shared/params_test.go b/pkg/cmd/pr/shared/params_test.go index 53eb6328fb6..f47f26fcede 100644 --- a/pkg/cmd/pr/shared/params_test.go +++ b/pkg/cmd/pr/shared/params_test.go @@ -192,17 +192,26 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) { handles []string } tests := []struct { - name string - args args - want []string + name string + webMode bool + args args + want []string }{ { - name: "replaces @copilot with copilot-swe-agent", + name: "replaces @copilot with copilot-swe-agent for non-web mode", args: args{ handles: []string{"monalisa", "@copilot", "hubot"}, }, want: []string{"monalisa", "copilot-swe-agent", "hubot"}, }, + { + name: "replaces @copilot with copilot for web mode", + webMode: true, + args: args{ + handles: []string{"monalisa", "@copilot", "hubot"}, + }, + want: []string{"monalisa", "copilot", "hubot"}, + }, { name: "handles no @copilot mentions", args: args{ @@ -241,7 +250,7 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := NewCopilotReplacer() + r := NewCopilotReplacer(tt.webMode) got := r.ReplaceSlice(tt.args.handles) require.Equal(t, tt.want, got) }) From fd1c31de59fb0c7f7b010f584b750fc839bd2f82 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Fri, 11 Jul 2025 15:32:17 -0400 Subject: [PATCH 05/11] Fix `gh pr create` tests from metadata change These are the minimum fixes necessary to repair `gh pr create` tests after refactoring the underlying metadata resolution logic. --- pkg/cmd/pr/create/create_test.go | 180 +++++++++++++++++++++---------- 1 file changed, 126 insertions(+), 54 deletions(-) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 5ae5a52e66a..7f1c8884faf 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -429,20 +429,29 @@ func Test_createRun(t *testing.T) { }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { reg.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) + reg.Register( + httpmock.GraphQL(`query RepositoryAssignableUsers\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "u001": { "login": "hubot", "id": "HUBOTID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "TODO", "id": "TODOID" } - }, - "organization": { - "t000": { "slug": "core", "id": "COREID" }, - "t001": { "slug": "robots", "id": "ROBOTID" } - } - } } + { "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(` + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } `)) reg.Register( httpmock.GraphQL(`query RepositoryMilestoneList\b`), @@ -455,6 +464,17 @@ func Test_createRun(t *testing.T) { "pageInfo": { "hasNextPage": false } } } } } `)) + reg.Register( + httpmock.GraphQL(`query OrganizationTeamList\b`), + httpmock.StringResponse(` + { "data": { "organization": { "teams": { + "nodes": [ + { "slug": "core", "id": "COREID" }, + { "slug": "robots", "id": "ROBOTID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) mockRetrieveProjects(t, reg) }, expectedOutputs: []string{ @@ -526,20 +546,29 @@ func Test_createRun(t *testing.T) { }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { reg.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) + reg.Register( + httpmock.GraphQL(`query RepositoryAssignableUsers\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "u001": { "login": "hubot", "id": "HUBOTID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "TODO", "id": "TODOID" } - }, - "organization": { - "t000": { "slug": "core", "id": "COREID" }, - "t001": { "slug": "robots", "id": "ROBOTID" } - } - } } + { "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(` + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } `)) reg.Register( httpmock.GraphQL(`query RepositoryMilestoneList\b`), @@ -552,6 +581,17 @@ func Test_createRun(t *testing.T) { "pageInfo": { "hasNextPage": false } } } } } `)) + reg.Register( + httpmock.GraphQL(`query OrganizationTeamList\b`), + httpmock.StringResponse(` + { "data": { "organization": { "teams": { + "nodes": [ + { "slug": "core", "id": "COREID" }, + { "slug": "robots", "id": "ROBOTID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) mockRetrieveProjects(t, reg) }, expectedOutputs: []string{ @@ -1028,20 +1068,29 @@ func Test_createRun(t *testing.T) { }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { reg.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) + reg.Register( + httpmock.GraphQL(`query RepositoryAssignableUsers\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "u001": { "login": "hubot", "id": "HUBOTID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "TODO", "id": "TODOID" } - }, - "organization": { - "t000": { "slug": "core", "id": "COREID" }, - "t001": { "slug": "robots", "id": "ROBOTID" } - } - } } + { "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(` + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } `)) reg.Register( httpmock.GraphQL(`query RepositoryMilestoneList\b`), @@ -1054,6 +1103,17 @@ func Test_createRun(t *testing.T) { "pageInfo": { "hasNextPage": false } } } } } `)) + reg.Register( + httpmock.GraphQL(`query OrganizationTeamList\b`), + httpmock.StringResponse(` + { "data": { "organization": { "teams": { + "nodes": [ + { "slug": "core", "id": "COREID" }, + { "slug": "robots", "id": "ROBOTID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) mockRetrieveProjects(t, reg) reg.Register( httpmock.GraphQL(`mutation PullRequestCreate\b`), @@ -1242,30 +1302,42 @@ func Test_createRun(t *testing.T) { tty: true, httpStubs: func(reg *httpmock.Registry, t *testing.T) { reg.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) + reg.Register( + httpmock.GraphQL(`query RepositoryAssignableUsers\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "jillValentine", "id": "JILLID" }, - "repository": {}, - "organization": {} - } } - `)) + { "data": { "repository": { "assignableUsers": { + "nodes": [ + { "login": "jillValentine", "id": "JILLID", "name": "Jill Valentine" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query OrganizationTeamList\b`), + httpmock.StringResponse(` + { "data": { "organization": { "teams": { + "nodes": [], + "pageInfo": { "hasNextPage": false } + } } } } + `)) reg.Register( httpmock.GraphQL(`mutation PullRequestCreateRequestReviews\b`), httpmock.GraphQLMutation(` - { "data": { "requestReviews": { - "clientMutationId": "" - } } } - `, func(inputs map[string]interface{}) { + { "data": { "requestReviews": { + "clientMutationId": "" + } } } + `, func(inputs map[string]interface{}) { assert.Equal(t, []interface{}{"JILLID"}, inputs["userIds"]) })) reg.Register( httpmock.GraphQL(`mutation PullRequestCreate\b`), httpmock.GraphQLMutation(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `, func(input map[string]interface{}) { + { "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } } + `, func(input map[string]interface{}) { assert.Equal(t, "recovered title", input["title"].(string)) assert.Equal(t, "recovered body", input["body"].(string)) })) From 21202ff992f82125f41b00dc4b03138b9f927811 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 16 Jul 2025 08:47:12 -0400 Subject: [PATCH 06/11] Improve comments from PR feedback --- pkg/cmd/pr/shared/survey.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 565242f0c41..af1fa871a6a 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -203,8 +203,9 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface reviewers = append(reviewers, fmt.Sprintf("%s/%s", baseRepo.RepoOwner(), t.Slug)) } - // In order to select any provided assignee, the following code needs to take the provided default assignees in `state` - // and translate them to the appropriate actor / user. + // 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. var assignees []string var assigneesDefault []string if state.ActorAssignees { From e2bed653dfb3ee6d70fcd12655a17069c38992e9 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 16 Jul 2025 14:10:26 -0400 Subject: [PATCH 07/11] Implement actor and user assignee tests --- pkg/cmd/issue/create/create_test.go | 197 +++++++++++++++++++++++++--- 1 file changed, 179 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 1c9106ec425..65731e56134 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -256,6 +256,7 @@ func Test_createRun(t *testing.T) { name string opts CreateOptions httpStubs func(*httpmock.Registry) + promptStubs func(*prompter.PrompterMock) wantsStdout string wantsStderr string wantsBrowse string @@ -264,7 +265,8 @@ func Test_createRun(t *testing.T) { { name: "no args", opts: CreateOptions{ - WebMode: true, + Detector: &fd.EnabledDetectorMock{}, + WebMode: true, }, wantsBrowse: "https://github.com/OWNER/REPO/issues/new", wantsStderr: "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n", @@ -272,9 +274,10 @@ func Test_createRun(t *testing.T) { { name: "title and body", opts: CreateOptions{ - WebMode: true, - Title: "myissue", - Body: "hello cli", + Detector: &fd.EnabledDetectorMock{}, + WebMode: true, + Title: "myissue", + Body: "hello cli", }, wantsBrowse: "https://github.com/OWNER/REPO/issues/new?body=hello+cli&title=myissue", wantsStderr: "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n", @@ -282,6 +285,7 @@ func Test_createRun(t *testing.T) { { name: "assignee", opts: CreateOptions{ + Detector: &fd.EnabledDetectorMock{}, WebMode: true, Assignees: []string{"monalisa"}, }, @@ -291,6 +295,7 @@ func Test_createRun(t *testing.T) { { name: "@me", opts: CreateOptions{ + Detector: &fd.EnabledDetectorMock{}, WebMode: true, Assignees: []string{"@me"}, }, @@ -308,6 +313,7 @@ func Test_createRun(t *testing.T) { { name: "@copilot", opts: CreateOptions{ + Detector: &fd.EnabledDetectorMock{}, WebMode: true, Assignees: []string{"@copilot"}, }, @@ -317,6 +323,7 @@ func Test_createRun(t *testing.T) { { name: "project", opts: CreateOptions{ + Detector: &fd.EnabledDetectorMock{}, WebMode: true, Projects: []string{"cleanup"}, }, @@ -373,7 +380,8 @@ func Test_createRun(t *testing.T) { { name: "has templates", opts: CreateOptions{ - WebMode: true, + Detector: &fd.EnabledDetectorMock{}, + WebMode: true, }, httpStubs: func(r *httpmock.Registry) { r.Register( @@ -393,8 +401,9 @@ func Test_createRun(t *testing.T) { { name: "too long body", opts: CreateOptions{ - WebMode: true, - Body: strings.Repeat("A", 9216), + Detector: &fd.EnabledDetectorMock{}, + WebMode: true, + Body: strings.Repeat("A", 9216), }, wantsErr: "cannot open in browser: maximum URL length exceeded", }, @@ -404,22 +413,23 @@ func Test_createRun(t *testing.T) { r.Register( httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` - { "data": { "repository": { - "id": "REPOID", - "hasIssuesEnabled": true - } } }`)) + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } }`)) r.Register( httpmock.GraphQL(`mutation IssueCreate\b`), httpmock.GraphQLMutation(` - { "data": { "createIssue": { "issue": { - "URL": "https://github.com/OWNER/REPO/issues/12" - } } } } - `, func(inputs map[string]interface{}) { + { "data": { "createIssue": { "issue": { + "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"]) })) }, opts: CreateOptions{ + Detector: &fd.EnabledDetectorMock{}, EditorMode: true, TitledEditSurvey: func(string, string) (string, string, error) { return "title", "body", nil }, }, @@ -457,6 +467,7 @@ func Test_createRun(t *testing.T) { })) }, opts: CreateOptions{ + Detector: &fd.EnabledDetectorMock{}, EditorMode: true, Template: "Bug report", TitledEditSurvey: func(title string, body string) (string, string, error) { return title, body, nil }, @@ -464,6 +475,152 @@ func Test_createRun(t *testing.T) { wantsStdout: "https://github.com/OWNER/REPO/issues/12\n", wantsStderr: "\nCreating issue in OWNER/REPO\n\n", }, + { + name: "interactive prompts with actor assignee display names when actors available", + opts: CreateOptions{ + Interactive: true, + Detector: &fd.EnabledDetectorMock{}, + Title: "test `gh issue create` actor assignees", + Body: "Actor assignees allow users and bots to be assigned to issues", + }, + promptStubs: func(pm *prompter.PrompterMock) { + firstConfirmSubmission := true + pm.InputFunc = func(message, defaultValue string) (string, error) { + switch message { + default: + return "", fmt.Errorf("unexpected input prompt: %s", message) + } + } + pm.MultiSelectFunc = func(message string, defaults []string, options []string) ([]int, error) { + 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.SelectFunc = func(message, defaultValue string, options []string) (int, error) { + switch message { + case "What's next?": + if firstConfirmSubmission { + firstConfirmSubmission = false + return prompter.IndexFor(options, "Add metadata") + } + return prompter.IndexFor(options, "Submit") + default: + return 0, fmt.Errorf("unexpected select prompt: %s", message) + } + } + }, + httpStubs: func(r *httpmock.Registry) { + r.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true, + "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(` + { "data": { "createIssue": { "issue": { + "URL": "https://github.com/OWNER/REPO/issues/12" + } } } } + `, func(inputs map[string]interface{}) { + assert.Equal(t, []interface{}{"COPILOTID", "MONAID"}, inputs["assigneeIds"]) + })) + }, + wantsStdout: "https://github.com/OWNER/REPO/issues/12\n", + wantsStderr: "\nCreating issue in OWNER/REPO\n\n", + }, + { + name: "interactive prompts with user assignee logins when actors unavailable", + opts: CreateOptions{ + Interactive: true, + Detector: &fd.DisabledDetectorMock{}, + Title: "test `gh issue create` user assignees", + Body: "User assignees allow only users to be assigned to issues", + }, + promptStubs: func(pm *prompter.PrompterMock) { + firstConfirmSubmission := true + pm.InputFunc = func(message, defaultValue string) (string, error) { + switch message { + default: + return "", fmt.Errorf("unexpected input prompt: %s", message) + } + } + pm.MultiSelectFunc = func(message string, defaults []string, options []string) ([]int, error) { + switch message { + case "What would you like to add?": + return prompter.IndexesFor(options, "Assignees") + case "Assignees": + return prompter.IndexesFor(options, "hubot", "MonaLisa (Mona Display Name)") + default: + return nil, fmt.Errorf("unexpected multi-select prompt: %s", message) + } + } + pm.SelectFunc = func(message, defaultValue string, options []string) (int, error) { + switch message { + case "What's next?": + if firstConfirmSubmission { + firstConfirmSubmission = false + return prompter.IndexFor(options, "Add metadata") + } + return prompter.IndexFor(options, "Submit") + default: + return 0, fmt.Errorf("unexpected select prompt: %s", message) + } + } + }, + httpStubs: func(r *httpmock.Registry) { + r.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true, + "viewerPermission": "WRITE" + } } } + `)) + r.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 } + } } } } + `)) + r.Register( + httpmock.GraphQL(`mutation IssueCreate\b`), + httpmock.GraphQLMutation(` + { "data": { "createIssue": { "issue": { + "URL": "https://github.com/OWNER/REPO/issues/12" + } } } } + `, func(inputs map[string]interface{}) { + assert.Equal(t, []interface{}{"HUBOTID", "MONAID"}, inputs["assigneeIds"]) + })) + }, + wantsStdout: "https://github.com/OWNER/REPO/issues/12\n", + wantsStderr: "\nCreating issue in OWNER/REPO\n\n", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -483,10 +640,15 @@ func Test_createRun(t *testing.T) { opts.BaseRepo = func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil } - opts.Detector = &fd.EnabledDetectorMock{} browser := &browser.Stub{} opts.Browser = browser + prompterMock := &prompter.PrompterMock{} + opts.Prompter = prompterMock + if tt.promptStubs != nil { + tt.promptStubs(prompterMock) + } + err := createRun(opts) if tt.wantsErr == "" { require.NoError(t, err) @@ -960,8 +1122,7 @@ func TestIssueCreate_AtMeAssignee(t *testing.T) { assert.Equal(t, "https://github.com/OWNER/REPO/issues/12\n", output.String()) } -// TODO: Create an interactive variant of this test, which will ensure that Copilot is in the assignee list and can be selected -func TestIssueCreate_AtCopilotAssigneeNonInteractive(t *testing.T) { +func TestIssueCreate_AtCopilotAssignee(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) From 4f0a5807bf75a60bb17cca7788052d48e99be13f Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 16 Jul 2025 14:42:11 -0400 Subject: [PATCH 08/11] Refactor CopilotReplacer logic based on PR feedback Thanks to @bagtoad, this commit refactors the argument to `NewCopilotReplacer(bool)` from being where this is used to what it effect is has. Because there is already precedence for display name being ` ()`, I worried there would be confusion for `Copilot (AI)` being display name for assignees and reviewers but `Copilot` when going to GitHub.com UI. Instead, I renamed the argument based on whether the login is returned / replaced. --- pkg/cmd/issue/create/create.go | 3 ++- pkg/cmd/pr/shared/editable.go | 2 +- pkg/cmd/pr/shared/params.go | 16 ++++++++-------- pkg/cmd/pr/shared/params_test.go | 22 ++++++++++++---------- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index db3bd72014e..bc38c52b356 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -177,9 +177,10 @@ 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) + copilotReplacer := prShared.NewCopilotReplacer(!opts.WebMode) assignees, err := meReplacer.ReplaceSlice(opts.Assignees) if err != nil { return err diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 4e3bfab4cb2..ffe1642da40 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -118,7 +118,7 @@ func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]str // 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(false) + copilotReplacer := NewCopilotReplacer(true) replaceSpecialAssigneeNames := func(value []string) ([]string, error) { replaced, err := meReplacer.ReplaceSlice(value) diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index 505d3a0a5ec..818e9fa4c2e 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -285,14 +285,15 @@ func (r *MeReplacer) ReplaceSlice(handles []string) ([]string, error) { return res, nil } -// CopilotReplacer resolves usages of `@copilot` to Copilot's login. +// CopilotReplacer resolves usages of `@copilot` to either Copilot's login or name. +// Login is generally needed for API calls; name is used when launching web browser. type CopilotReplacer struct { - webMode bool + returnLogin bool } -func NewCopilotReplacer(webMode bool) *CopilotReplacer { +func NewCopilotReplacer(returnLogin bool) *CopilotReplacer { return &CopilotReplacer{ - webMode: webMode, + returnLogin: returnLogin, } } @@ -300,11 +301,10 @@ func (r *CopilotReplacer) replace(handle string) string { if !strings.EqualFold(handle, "@copilot") { return handle } - // When Copilot is used as an assignee in web mode, the login is not used. - if r.webMode { - return "copilot" + if r.returnLogin { + return api.CopilotActorLogin } - return api.CopilotActorLogin + return "copilot" } // ReplaceSlice replaces usages of `@copilot` in a slice with Copilot's login. diff --git a/pkg/cmd/pr/shared/params_test.go b/pkg/cmd/pr/shared/params_test.go index f47f26fcede..63d7e1000d8 100644 --- a/pkg/cmd/pr/shared/params_test.go +++ b/pkg/cmd/pr/shared/params_test.go @@ -192,21 +192,21 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) { handles []string } tests := []struct { - name string - webMode bool - args args - want []string + name string + returnLogin bool + args args + want []string }{ { - name: "replaces @copilot with copilot-swe-agent for non-web mode", + name: "replaces @copilot with copilot-swe-agent for non-web mode", + returnLogin: true, args: args{ handles: []string{"monalisa", "@copilot", "hubot"}, }, want: []string{"monalisa", "copilot-swe-agent", "hubot"}, }, { - name: "replaces @copilot with copilot for web mode", - webMode: true, + name: "replaces @copilot with copilot for web mode", args: args{ handles: []string{"monalisa", "@copilot", "hubot"}, }, @@ -220,14 +220,16 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) { want: []string{"monalisa", "user", "hubot"}, }, { - name: "replaces multiple @copilot mentions", + name: "replaces multiple @copilot mentions", + returnLogin: true, args: args{ handles: []string{"@copilot", "user", "@copilot"}, }, want: []string{"copilot-swe-agent", "user", "copilot-swe-agent"}, }, { - name: "handles @copilot case-insensitively", + name: "handles @copilot case-insensitively", + returnLogin: true, args: args{ handles: []string{"@Copilot", "user", "@CoPiLoT"}, }, @@ -250,7 +252,7 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := NewCopilotReplacer(tt.webMode) + r := NewCopilotReplacer(tt.returnLogin) got := r.ReplaceSlice(tt.args.handles) require.Equal(t, tt.want, got) }) From 5cf3d3b9a5af13522ab912519488db4c6e4cd07c Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 16 Jul 2025 15:06:43 -0400 Subject: [PATCH 09/11] Use constant for Copilot name Based on PR feedback from @bagtoad, this commit creates a new constant for the Copilot bot name, which is used in the assignee / reviewer selection as well as replacing `@copilot` when going to GitHub.com UI --- api/queries_repo.go | 3 ++- pkg/cmd/pr/shared/params.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index 8480e3868d6..6552aa230cb 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -1082,6 +1082,7 @@ func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) // This is returned from assignable actors and issue/pr assigned actors. // We use this to check if the actor is Copilot. const CopilotActorLogin = "copilot-swe-agent" +const CopilotActorName = "Copilot" type AssignableActor interface { DisplayName() string @@ -1142,7 +1143,7 @@ func NewAssignableBot(id, login string) AssignableBot { func (b AssignableBot) DisplayName() string { if b.login == CopilotActorLogin { - return "Copilot (AI)" + return fmt.Sprintf("%s (AI)", CopilotActorName) } return b.Login() } diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index 818e9fa4c2e..d94f338c5e6 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -304,7 +304,7 @@ func (r *CopilotReplacer) replace(handle string) string { if r.returnLogin { return api.CopilotActorLogin } - return "copilot" + return api.CopilotActorName } // ReplaceSlice replaces usages of `@copilot` in a slice with Copilot's login. From b433792a44ba6a55a18a20a47100b65f14b52379 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 16 Jul 2025 15:09:13 -0400 Subject: [PATCH 10/11] Fix copilot replace test names In refactoring how this parameter works, I forgot to update the related tests to match. --- pkg/cmd/pr/shared/params_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/pr/shared/params_test.go b/pkg/cmd/pr/shared/params_test.go index 63d7e1000d8..1291b2c9991 100644 --- a/pkg/cmd/pr/shared/params_test.go +++ b/pkg/cmd/pr/shared/params_test.go @@ -198,7 +198,7 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) { want []string }{ { - name: "replaces @copilot with copilot-swe-agent for non-web mode", + name: "replaces @copilot with login", returnLogin: true, args: args{ handles: []string{"monalisa", "@copilot", "hubot"}, @@ -206,7 +206,7 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) { want: []string{"monalisa", "copilot-swe-agent", "hubot"}, }, { - name: "replaces @copilot with copilot for web mode", + name: "replaces @copilot with name", args: args{ handles: []string{"monalisa", "@copilot", "hubot"}, }, From 973718712aa9b32c1ab702eb520384ff826d0f5b Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 16 Jul 2025 15:36:53 -0400 Subject: [PATCH 11/11] Fix failing tests for Copilot replacement --- pkg/cmd/issue/create/create_test.go | 2 +- pkg/cmd/pr/shared/params_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 65731e56134..80c8f76d35b 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -317,7 +317,7 @@ func Test_createRun(t *testing.T) { WebMode: true, Assignees: []string{"@copilot"}, }, - wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=copilot&body=", + wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=Copilot&body=", wantsStderr: "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n", }, { diff --git a/pkg/cmd/pr/shared/params_test.go b/pkg/cmd/pr/shared/params_test.go index 1291b2c9991..3c95a9a5da9 100644 --- a/pkg/cmd/pr/shared/params_test.go +++ b/pkg/cmd/pr/shared/params_test.go @@ -210,7 +210,7 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) { args: args{ handles: []string{"monalisa", "@copilot", "hubot"}, }, - want: []string{"monalisa", "copilot", "hubot"}, + want: []string{"monalisa", "Copilot", "hubot"}, }, { name: "handles no @copilot mentions",