From ed57df1a9af6c614b6396abbcf85b6a0aa0a35a3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 28 May 2025 14:26:44 +0000 Subject: [PATCH 01/13] chore(deps): bump google.golang.org/grpc from 1.72.0 to 1.72.2 Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.72.0 to 1.72.2. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](https://github.com/grpc/grpc-go/compare/v1.72.0...v1.72.2) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-version: 1.72.2 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 108c5722d..62aa7437d 100644 --- a/go.mod +++ b/go.mod @@ -54,7 +54,7 @@ require ( golang.org/x/sync v0.14.0 golang.org/x/term v0.32.0 golang.org/x/text v0.25.0 - google.golang.org/grpc v1.72.0 + google.golang.org/grpc v1.72.2 google.golang.org/protobuf v1.36.6 gopkg.in/h2non/gock.v1 v1.1.2 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index 8d50af689..8d55e736d 100644 --- a/go.sum +++ b/go.sum @@ -612,8 +612,8 @@ google.golang.org/genproto/googleapis/api v0.0.0-20250414145226-207652e42e2e h1: google.golang.org/genproto/googleapis/api v0.0.0-20250414145226-207652e42e2e/go.mod h1:085qFyf2+XaZlRdCgKNCIZ3afY2p4HHZdoIRpId8F4A= google.golang.org/genproto/googleapis/rpc v0.0.0-20250414145226-207652e42e2e h1:ztQaXfzEXTmCBvbtWYRhJxW+0iJcz2qXfd38/e9l7bA= google.golang.org/genproto/googleapis/rpc v0.0.0-20250414145226-207652e42e2e/go.mod h1:qQ0YXyHHx3XkvlzUtpXDkS29lDSafHMZBAZDc03LQ3A= -google.golang.org/grpc v1.72.0 h1:S7UkcVa60b5AAQTaO6ZKamFp1zMZSU0fGDK2WZLbBnM= -google.golang.org/grpc v1.72.0/go.mod h1:wH5Aktxcg25y1I3w7H69nHfXdOG3UiadoBtjh3izSDM= +google.golang.org/grpc v1.72.2 h1:TdbGzwb82ty4OusHWepvFWGLgIbNo1/SUynEN0ssqv8= +google.golang.org/grpc v1.72.2/go.mod h1:wH5Aktxcg25y1I3w7H69nHfXdOG3UiadoBtjh3izSDM= google.golang.org/protobuf v1.36.6 h1:z1NpPI8ku2WgiWnf+t9wTPsn6eP1L7ksHUlkfLvd9xY= google.golang.org/protobuf v1.36.6/go.mod h1:jduwjTPXsFjZGTmRluh+L6NjiWu7pchiJ2/5YcXBHnY= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= From 793a7ff459ba7b121c4c2f21186b983b73d69fa9 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 2 Jun 2025 14:33:38 +0200 Subject: [PATCH 02/13] Fix pr edit when URL is provided --- api/queries_pr.go | 28 ++++++++---- pkg/cmd/pr/edit/edit.go | 26 +---------- pkg/cmd/pr/edit/edit_test.go | 24 +++++++--- pkg/cmd/pr/shared/finder.go | 31 +++++++++++++ pkg/cmd/pr/shared/finder_test.go | 76 ++++++++++++++++++++++++++++++++ 5 files changed, 146 insertions(+), 39 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 525418a11..1d394e864 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -85,15 +85,25 @@ type PullRequest struct { Assignees Assignees AssignedActors AssignedActors - Labels Labels - ProjectCards ProjectCards - ProjectItems ProjectItems - Milestone *Milestone - Comments Comments - ReactionGroups ReactionGroups - Reviews PullRequestReviews - LatestReviews PullRequestReviews - ReviewRequests ReviewRequests + // AssignedActorsUsed is a GIGANTIC hack to carry around whether we expected AssignedActors to be requested + // on this PR. This is required because the Feature Detection of support for AssignedActors occurs inside the + // PR Finder, but knowledge of support is required at the command level. However, we can't easily construct + // the feature detector at the command level because it needs knowledge of the BaseRepo, which is only available + // inside the PR Finder. This is bad and we should feel bad. + // + // The right solution is to extract argument parsing from the PR Finder into each command, so that we have access + // to the BaseRepo and can construct the feature detector there. This is what happens in the issue commands with + // `shared.ParseIssueFromArg`. + AssignedActorsUsed bool + Labels Labels + ProjectCards ProjectCards + ProjectItems ProjectItems + Milestone *Milestone + Comments Comments + ReactionGroups ReactionGroups + Reviews PullRequestReviews + LatestReviews PullRequestReviews + ReviewRequests ReviewRequests ClosingIssuesReferences ClosingIssuesReferences } diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index c30c4f9bb..867ee9a4f 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -3,7 +3,6 @@ package edit import ( "fmt" "net/http" - "time" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" @@ -206,7 +205,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman func editRun(opts *EditOptions) error { findOptions := shared.FindOptions{ Selector: opts.SelectorArg, - Fields: []string{"id", "url", "title", "body", "baseRefName", "reviewRequests", "labels", "projectCards", "projectItems", "milestone"}, + Fields: []string{"id", "url", "title", "body", "baseRefName", "reviewRequests", "labels", "projectCards", "projectItems", "milestone", "assignees"}, Detector: opts.Detector, } @@ -215,27 +214,6 @@ func editRun(opts *EditOptions) error { return err } - if opts.Detector == nil { - baseRepo, err := opts.BaseRepo() - if err != nil { - return err - } - - cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) - opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost()) - } - - issueFeatures, err := opts.Detector.IssueFeatures() - if err != nil { - return err - } - - if issueFeatures.ActorIsAssignable { - findOptions.Fields = append(findOptions.Fields, "assignedActors") - } else { - findOptions.Fields = append(findOptions.Fields, "assignees") - } - pr, repo, err := opts.Finder.Find(findOptions) if err != nil { return err @@ -247,7 +225,7 @@ func editRun(opts *EditOptions) error { editable.Body.Default = pr.Body editable.Base.Default = pr.BaseRefName editable.Reviewers.Default = pr.ReviewRequests.Logins() - if issueFeatures.ActorIsAssignable { + if pr.AssignedActorsUsed { editable.Assignees.ActorAssignees = true editable.Assignees.Default = pr.AssignedActors.DisplayNames() } else { diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 3a1e8a548..0c40e3839 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -341,9 +341,11 @@ func Test_editRun(t *testing.T) { { name: "non-interactive", input: &EditOptions{ + Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", + URL: "https://github.com/OWNER/REPO/pull/123", + AssignedActorsUsed: true, }, ghrepo.New("OWNER", "REPO")), Interactive: false, Editable: shared.Editable{ @@ -403,9 +405,11 @@ func Test_editRun(t *testing.T) { { name: "non-interactive skip reviewers", input: &EditOptions{ + Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", + URL: "https://github.com/OWNER/REPO/pull/123", + AssignedActorsUsed: true, }, ghrepo.New("OWNER", "REPO")), Interactive: false, Editable: shared.Editable{ @@ -459,9 +463,11 @@ func Test_editRun(t *testing.T) { { name: "non-interactive remove all reviewers", input: &EditOptions{ + Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", + URL: "https://github.com/OWNER/REPO/pull/123", + AssignedActorsUsed: true, }, ghrepo.New("OWNER", "REPO")), Interactive: false, Editable: shared.Editable{ @@ -520,9 +526,11 @@ func Test_editRun(t *testing.T) { { name: "interactive", input: &EditOptions{ + Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", + URL: "https://github.com/OWNER/REPO/pull/123", + AssignedActorsUsed: true, }, ghrepo.New("OWNER", "REPO")), Interactive: true, Surveyor: testSurveyor{}, @@ -542,9 +550,11 @@ func Test_editRun(t *testing.T) { { name: "interactive skip reviewers", input: &EditOptions{ + Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", + URL: "https://github.com/OWNER/REPO/pull/123", + AssignedActorsUsed: true, }, ghrepo.New("OWNER", "REPO")), Interactive: true, Surveyor: testSurveyor{skipReviewers: true}, @@ -563,9 +573,11 @@ func Test_editRun(t *testing.T) { { name: "interactive remove all reviewers", input: &EditOptions{ + Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", + URL: "https://github.com/OWNER/REPO/pull/123", + AssignedActorsUsed: true, }, ghrepo.New("OWNER", "REPO")), Interactive: true, Surveyor: testSurveyor{removeAllReviewers: true}, diff --git a/pkg/cmd/pr/shared/finder.go b/pkg/cmd/pr/shared/finder.go index b8d23c978..a19c69669 100644 --- a/pkg/cmd/pr/shared/finder.go +++ b/pkg/cmd/pr/shared/finder.go @@ -242,6 +242,33 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err } } + // Ok this is super, super horrible so bear with me. + // The `assignees` field on a Pull Request exposes users that are assigned. It is also possible for bots to be + // assigned, but they only appear under the `assignedActors` field. Ideally, the caller of `Find` would determine + // the correct field to use based on the `fd.Detector` that is passed in, but they can't construct a detector + // because the BaseRepo is only determined within this function. The more correct solution is to do what I did with + // the issue commands and decouple argument parsing from API lookup. See PR #10811 for example. + var actorAssigneesUsed bool + if fields.Contains("assignees") { + if opts.Detector == nil { + cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) + opts.Detector = fd.NewDetector(cachedClient, f.baseRefRepo.RepoHost()) + } + + issueFeatures, err := opts.Detector.IssueFeatures() + if err != nil { + return nil, nil, fmt.Errorf("error detecting issue features: %v", err) + } + + // If actors are assignable on this host then we additionally request the `assignedActors` field. + // Note that we don't remove the `assignees` field because some commands (`pr view`) do not display actor + // assignees yet, so we have to have both sets of data. + if issueFeatures.ActorIsAssignable { + fields.Add("assignedActors") + actorAssigneesUsed = true + } + } + var pr *api.PullRequest if f.prNumber > 0 { // If we have a PR number, let's look it up @@ -297,6 +324,10 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err }) } + if actorAssigneesUsed { + pr.AssignedActorsUsed = true + } + return pr, f.baseRefRepo, g.Wait() } diff --git a/pkg/cmd/pr/shared/finder_test.go b/pkg/cmd/pr/shared/finder_test.go index abc754d1a..28c2337d3 100644 --- a/pkg/cmd/pr/shared/finder_test.go +++ b/pkg/cmd/pr/shared/finder_test.go @@ -9,6 +9,7 @@ import ( ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/httpmock" "github.com/stretchr/testify/require" @@ -705,6 +706,81 @@ func TestFind(t *testing.T) { } } +func TestFindAssignableActors(t *testing.T) { + t.Run("given actors are not assignable, do nothing special", func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + // Ensure we never request assignedActors + reg.Exclude(t, httpmock.GraphQL(`assignedActors`)) + reg.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(`{"data":{"repository":{ + "pullRequest":{"number":13} + }}}`)) + + f := finder{ + httpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + } + + pr, _, err := f.Find(FindOptions{ + Detector: &fd.DisabledDetectorMock{}, + Fields: []string{"assignees"}, + Selector: "https://github.com/cli/cli/pull/123", + }) + require.NoError(t, err) + + require.False(t, pr.AssignedActorsUsed, "expected PR not to have assigned actors used") + }) + + t.Run("given actors are assignable, request assignedActors and indicate that on the returned PR", func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + // Ensure that we only respond if assignedActors is requested + reg.Register( + httpmock.GraphQL(`assignedActors`), + httpmock.StringResponse(`{"data":{"repository":{ + "pullRequest":{ + "number":13, + "assignedActors": { + "nodes": [ + { + "id": "HUBOTID", + "login": "hubot", + "__typename": "Bot" + }, + { + "id": "MONAID", + "login": "MonaLisa", + "name": "Mona Display Name", + "__typename": "User" + } + ], + "totalCount": 2 + }} + }}}`)) + + f := finder{ + httpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + } + + pr, _, err := f.Find(FindOptions{ + Detector: &fd.EnabledDetectorMock{}, + Fields: []string{"assignees"}, + Selector: "https://github.com/cli/cli/pull/123", + }) + require.NoError(t, err) + + require.Equal(t, []string{"hubot", "MonaLisa"}, pr.AssignedActors.Logins()) + require.True(t, pr.AssignedActorsUsed, "expected PR to have assigned actors used") + }) +} + func stubBranchConfig(branchConfig git.BranchConfig, err error) func(context.Context, string) (git.BranchConfig, error) { return func(_ context.Context, branch string) (git.BranchConfig, error) { return branchConfig, err From 9a8031151ce5475cd2393350ad77b4886596d170 Mon Sep 17 00:00:00 2001 From: Tim Rogers Date: Mon, 2 Jun 2025 16:29:35 +0100 Subject: [PATCH 03/13] Document support for `@copilot` in `gh [pr|issue] edit --add-assignee` and `--remove-assignee` (#11056) * Document support for `@copilot` in `gh [pr|issue] edit --add-assignee` and `--remove-assignee` Following up on #10991, this updates the help text for `issue edit` and `pr edit`'s `--add-assignee` and `--remove-assignee` options to mention that you can use `@copilot`. This is already mentioned in the command-level help text, but not at the argument level, whereas the `@me` macro is. * Apply suggestion from @babakks Co-authored-by: Babak K. Shandiz * Apply suggestion from @babakks Co-authored-by: Babak K. Shandiz --------- Co-authored-by: Babak K. Shandiz --- pkg/cmd/issue/edit/edit.go | 4 ++-- pkg/cmd/pr/edit/edit.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index e959cde2b..b207a96fd 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -171,8 +171,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd.Flags().StringVarP(&opts.Editable.Title.Value, "title", "t", "", "Set the new title.") cmd.Flags().StringVarP(&opts.Editable.Body.Value, "body", "b", "", "Set the new body.") cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)") - cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Add, "add-assignee", nil, "Add assigned users by their `login`. Use \"@me\" to assign yourself.") - cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Remove, "remove-assignee", nil, "Remove assigned users by their `login`. Use \"@me\" to unassign yourself.") + cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Add, "add-assignee", nil, "Add assigned users by their `login`. Use \"@me\" to assign yourself, or \"@copilot\" to assign Copilot.") + cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Remove, "remove-assignee", nil, "Remove assigned users by their `login`. Use \"@me\" to unassign yourself, or \"@copilot\" to unassign Copilot.") cmd.Flags().StringSliceVar(&opts.Editable.Labels.Add, "add-label", nil, "Add labels by `name`") cmd.Flags().StringSliceVar(&opts.Editable.Labels.Remove, "remove-label", nil, "Remove labels by `name`") cmd.Flags().StringSliceVar(&opts.Editable.Projects.Add, "add-project", nil, "Add the issue to projects by `title`") diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index c30c4f9bb..072356a1a 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -171,8 +171,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd.Flags().StringVarP(&opts.Editable.Base.Value, "base", "B", "", "Change the base `branch` for this pull request") cmd.Flags().StringSliceVar(&opts.Editable.Reviewers.Add, "add-reviewer", nil, "Add reviewers by their `login`.") cmd.Flags().StringSliceVar(&opts.Editable.Reviewers.Remove, "remove-reviewer", nil, "Remove reviewers by their `login`.") - cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Add, "add-assignee", nil, "Add assigned users by their `login`. Use \"@me\" to assign yourself.") - cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Remove, "remove-assignee", nil, "Remove assigned users by their `login`. Use \"@me\" to unassign yourself.") + cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Add, "add-assignee", nil, "Add assigned users by their `login`. Use \"@me\" to assign yourself, or \"@copilot\" to assign Copilot.") + cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Remove, "remove-assignee", nil, "Remove assigned users by their `login`. Use \"@me\" to unassign yourself, or \"@copilot\" to unassign Copilot.") cmd.Flags().StringSliceVar(&opts.Editable.Labels.Add, "add-label", nil, "Add labels by `name`") cmd.Flags().StringSliceVar(&opts.Editable.Labels.Remove, "remove-label", nil, "Remove labels by `name`") cmd.Flags().StringSliceVar(&opts.Editable.Projects.Add, "add-project", nil, "Add the pull request to projects by `title`") From cde860be883b444e9151b3c560eea5c28420d871 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Tue, 3 Jun 2025 13:33:52 +0100 Subject: [PATCH 04/13] Iterate on `pr edit` problems with existing assignees This commit is a handful of changes around `gh pr edit --add-assignee` behavior. Most notably, fixing a bug where the assigned actors weren't being dropped. In addition to this, I was refactoring the testing setup to allow for individual test table scenarios could be contained. --- pkg/cmd/issue/edit/edit_test.go | 17 ++- pkg/cmd/pr/edit/edit.go | 1 + pkg/cmd/pr/edit/edit_test.go | 177 ++++++++++++++++++++++++-------- 3 files changed, 144 insertions(+), 51 deletions(-) diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index 4840cbf7a..7d2d36662 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -631,10 +631,11 @@ func Test_editRun(t *testing.T) { }, EditFieldsSurvey: func(p prShared.EditPrompter, eo *prShared.Editable, _ string) error { // Checking that the display name is being used in the prompt. - require.Equal(t, eo.Assignees.Default, []string{"hubot", "MonaLisa (Mona Display Name)"}) + require.Equal(t, eo.Assignees.Default, []string{"hubot"}) + require.Equal(t, eo.Assignees.DefaultLogins, []string{"hubot"}) - // Mocking a selection of only MonaLisa in the prompt. - eo.Assignees.Value = []string{"MonaLisa (Mona Display Name)"} + // Adding MonaLisa as PR assignee, should preserve hubot. + eo.Assignees.Value = []string{"hubot", "MonaLisa (Mona Display Name)"} return nil }, FetchOptions: prShared.FetchOptions, @@ -662,7 +663,7 @@ func Test_editRun(t *testing.T) { // Checking that despite the display name being returned // from the EditFieldsSurvey, the ID is still // used in the mutation. - require.Contains(t, inputs["actorIds"], "MONAID") + require.Subset(t, inputs["actorIds"], []string{"MONAID", "HUBOTID"}) }), ) }, @@ -809,15 +810,9 @@ func mockIsssueNumberGetWithAssignedActors(_ *testing.T, reg *httpmock.Registry, "id": "HUBOTID", "login": "hubot", "__typename": "Bot" - }, - { - "id": "MONAID", - "login": "MonaLisa", - "name": "Mona Display Name", - "__typename": "User" } ], - "totalCount": 2 + "totalCount": 1 } } } } }`, number)), ) diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 5e1f19b18..becbfce47 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -228,6 +228,7 @@ func editRun(opts *EditOptions) error { if pr.AssignedActorsUsed { editable.Assignees.ActorAssignees = true editable.Assignees.Default = pr.AssignedActors.DisplayNames() + editable.Assignees.DefaultLogins = pr.AssignedActors.Logins() } else { editable.Assignees.Default = pr.Assignees.Logins() } diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 0c40e3839..0b731cd00 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -532,8 +532,31 @@ func Test_editRun(t *testing.T) { URL: "https://github.com/OWNER/REPO/pull/123", AssignedActorsUsed: true, }, ghrepo.New("OWNER", "REPO")), - Interactive: true, - Surveyor: testSurveyor{}, + Interactive: true, + Surveyor: testSurveyor{ + fieldsToEdit: func(e *shared.Editable) error { + e.Title.Edited = true + e.Body.Edited = true + e.Reviewers.Edited = true + e.Assignees.Edited = true + e.Labels.Edited = true + e.Projects.Edited = true + e.Milestone.Edited = true + return nil + }, + editFields: func(e *shared.Editable, _ string) error { + e.Title.Value = "new title" + e.Body.Value = "new body" + e.Reviewers.Value = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"} + e.Assignees.Value = []string{"monalisa", "hubot"} + e.Labels.Value = []string{"feature", "TODO", "bug"} + e.Labels.Add = []string{"feature", "TODO", "bug"} + e.Labels.Remove = []string{"docs"} + e.Projects.Value = []string{"Cleanup", "CleanupV2"} + e.Milestone.Value = "GA" + return nil + }, + }, Fetcher: testFetcher{}, EditorRetriever: testEditorRetriever{}, }, @@ -556,8 +579,29 @@ func Test_editRun(t *testing.T) { URL: "https://github.com/OWNER/REPO/pull/123", AssignedActorsUsed: true, }, ghrepo.New("OWNER", "REPO")), - Interactive: true, - Surveyor: testSurveyor{skipReviewers: true}, + Interactive: true, + Surveyor: testSurveyor{ + fieldsToEdit: func(e *shared.Editable) error { + e.Title.Edited = true + e.Body.Edited = true + e.Assignees.Edited = true + e.Labels.Edited = true + e.Projects.Edited = true + e.Milestone.Edited = true + return nil + }, + editFields: func(e *shared.Editable, _ string) error { + e.Title.Value = "new title" + e.Body.Value = "new body" + e.Assignees.Value = []string{"monalisa", "hubot"} + e.Labels.Value = []string{"feature", "TODO", "bug"} + e.Labels.Add = []string{"feature", "TODO", "bug"} + e.Labels.Remove = []string{"docs"} + e.Projects.Value = []string{"Cleanup", "CleanupV2"} + e.Milestone.Value = "GA" + return nil + }, + }, Fetcher: testFetcher{}, EditorRetriever: testEditorRetriever{}, }, @@ -579,8 +623,30 @@ func Test_editRun(t *testing.T) { URL: "https://github.com/OWNER/REPO/pull/123", AssignedActorsUsed: true, }, ghrepo.New("OWNER", "REPO")), - Interactive: true, - Surveyor: testSurveyor{removeAllReviewers: true}, + Interactive: true, + Surveyor: testSurveyor{ + fieldsToEdit: func(e *shared.Editable) error { + e.Title.Edited = true + e.Body.Edited = true + e.Assignees.Edited = true + e.Labels.Edited = true + e.Projects.Edited = true + e.Milestone.Edited = true + return nil + }, + editFields: func(e *shared.Editable, _ string) error { + e.Title.Value = "new title" + e.Body.Value = "new body" + e.Reviewers.Remove = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external", "dependabot"} + e.Assignees.Value = []string{"monalisa", "hubot"} + e.Labels.Value = []string{"feature", "TODO", "bug"} + e.Labels.Add = []string{"feature", "TODO", "bug"} + e.Labels.Remove = []string{"docs"} + e.Projects.Value = []string{"Cleanup", "CleanupV2"} + e.Milestone.Value = "GA" + return nil + }, + }, Fetcher: testFetcher{}, EditorRetriever: testEditorRetriever{}, }, @@ -659,6 +725,59 @@ func Test_editRun(t *testing.T) { } } +func Test_editRun_actors(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(true) + ios.SetStdinTTY(true) + ios.SetStderrTTY(true) + + reg := &httpmock.Registry{} + defer reg.Verify(t) + 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 } + } } } } + `)) + mockPullRequestUpdate(reg) + mockPullRequestUpdateActorAssignees(reg) + + httpClient := func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } + baseRepo := func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil } + + input := &EditOptions{ + Detector: &fd.EnabledDetectorMock{}, + SelectorArg: "123", + Finder: shared.NewMockFinder("123", &api.PullRequest{ + URL: "https://github.com/OWNER/REPO/pull/123", + }, ghrepo.New("OWNER", "REPO")), + Interactive: false, + Editable: shared.Editable{ + Assignees: shared.EditableAssignees{ + EditableSlice: shared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Remove: []string{"octocat"}, + Edited: true, + }, + }, + }, + Fetcher: testFetcher{}, + } + input.IO = ios + input.HttpClient = httpClient + input.BaseRepo = baseRepo + + err := editRun(input) + assert.NoError(t, err) + assert.Equal(t, "https://github.com/OWNER/REPO/pull/123\n", stdout.String()) + assert.Equal(t, "", stderr.String()) +} + func mockRepoMetadata(reg *httpmock.Registry, skipReviewers bool) { reg.Register( httpmock.GraphQL(`query RepositoryAssignableActors\b`), @@ -666,7 +785,7 @@ func mockRepoMetadata(reg *httpmock.Registry, skipReviewers bool) { { "data": { "repository": { "suggestedActors": { "nodes": [ { "login": "hubot", "id": "HUBOTID", "__typename": "Bot" }, - { "login": "MonaLisa", "id": "MONAID", "__typename": "User" } + { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } ], "pageInfo": { "hasNextPage": false } } } } } @@ -813,48 +932,26 @@ func mockProjectV2ItemUpdate(reg *httpmock.Registry) { } type testFetcher struct{} -type testSurveyor struct { - skipReviewers bool - removeAllReviewers bool -} -type testEditorRetriever struct{} func (f testFetcher) EditableOptionsFetch(client *api.Client, repo ghrepo.Interface, opts *shared.Editable) error { return shared.FetchOptions(client, repo, opts) } -func (s testSurveyor) FieldsToEdit(e *shared.Editable) error { - e.Title.Edited = true - e.Body.Edited = true - if !s.skipReviewers { - e.Reviewers.Edited = true - } - e.Assignees.Edited = true - e.Labels.Edited = true - e.Projects.Edited = true - e.Milestone.Edited = true - return nil +type testSurveyor struct { + fieldsToEdit func(e *shared.Editable) error + editFields func(e *shared.Editable, editorCmd string) error } -func (s testSurveyor) EditFields(e *shared.Editable, _ string) error { - e.Title.Value = "new title" - e.Body.Value = "new body" - if !s.skipReviewers { - if s.removeAllReviewers { - e.Reviewers.Remove = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external", "dependabot"} - } else { - e.Reviewers.Value = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"} - } - } - e.Assignees.Value = []string{"monalisa", "hubot"} - e.Labels.Value = []string{"feature", "TODO", "bug"} - e.Labels.Add = []string{"feature", "TODO", "bug"} - e.Labels.Remove = []string{"docs"} - e.Projects.Value = []string{"Cleanup", "CleanupV2"} - e.Milestone.Value = "GA" - return nil +func (s testSurveyor) FieldsToEdit(e *shared.Editable) error { + return s.fieldsToEdit(e) } +func (s testSurveyor) EditFields(e *shared.Editable, editorCmd string) error { + return s.editFields(e, editorCmd) +} + +type testEditorRetriever struct{} + func (t testEditorRetriever) Retrieve() (string, error) { return "vim", nil } From a24d39ac87b14979f2a52e3f417f92d17e53186e Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Tue, 3 Jun 2025 13:48:01 +0100 Subject: [PATCH 05/13] Fix test, remove partial standalone test --- pkg/cmd/pr/edit/edit_test.go | 54 +----------------------------------- 1 file changed, 1 insertion(+), 53 deletions(-) diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 0b731cd00..99f949e8b 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -628,6 +628,7 @@ func Test_editRun(t *testing.T) { fieldsToEdit: func(e *shared.Editable) error { e.Title.Edited = true e.Body.Edited = true + e.Reviewers.Edited = true e.Assignees.Edited = true e.Labels.Edited = true e.Projects.Edited = true @@ -725,59 +726,6 @@ func Test_editRun(t *testing.T) { } } -func Test_editRun_actors(t *testing.T) { - ios, _, stdout, stderr := iostreams.Test() - ios.SetStdoutTTY(true) - ios.SetStdinTTY(true) - ios.SetStderrTTY(true) - - reg := &httpmock.Registry{} - defer reg.Verify(t) - 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 } - } } } } - `)) - mockPullRequestUpdate(reg) - mockPullRequestUpdateActorAssignees(reg) - - httpClient := func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } - baseRepo := func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil } - - input := &EditOptions{ - Detector: &fd.EnabledDetectorMock{}, - SelectorArg: "123", - Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", - }, ghrepo.New("OWNER", "REPO")), - Interactive: false, - Editable: shared.Editable{ - Assignees: shared.EditableAssignees{ - EditableSlice: shared.EditableSlice{ - Add: []string{"monalisa", "hubot"}, - Remove: []string{"octocat"}, - Edited: true, - }, - }, - }, - Fetcher: testFetcher{}, - } - input.IO = ios - input.HttpClient = httpClient - input.BaseRepo = baseRepo - - err := editRun(input) - assert.NoError(t, err) - assert.Equal(t, "https://github.com/OWNER/REPO/pull/123\n", stdout.String()) - assert.Equal(t, "", stderr.String()) -} - func mockRepoMetadata(reg *httpmock.Registry, skipReviewers bool) { reg.Register( httpmock.GraphQL(`query RepositoryAssignableActors\b`), From ed4b90104f290148229837324a2d2b63ad79be08 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 3 Jun 2025 14:20:37 +0100 Subject: [PATCH 06/13] test(pr): Add tests for actor assignees --- pkg/cmd/issue/edit/edit_test.go | 4 +- pkg/cmd/pr/edit/edit_test.go | 65 +++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index 7d2d36662..d14b2f462 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -631,8 +631,8 @@ func Test_editRun(t *testing.T) { }, EditFieldsSurvey: func(p prShared.EditPrompter, eo *prShared.Editable, _ string) error { // Checking that the display name is being used in the prompt. - require.Equal(t, eo.Assignees.Default, []string{"hubot"}) - require.Equal(t, eo.Assignees.DefaultLogins, []string{"hubot"}) + 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)"} diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 99f949e8b..374625912 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -661,6 +661,71 @@ func Test_editRun(t *testing.T) { }, stdout: "https://github.com/OWNER/REPO/pull/123\n", }, + { + name: "interactive prompts with actor assignee display names when actors available", + input: &EditOptions{ + Detector: &fd.EnabledDetectorMock{}, + SelectorArg: "123", + Finder: shared.NewMockFinder("123", &api.PullRequest{ + URL: "https://github.com/OWNER/REPO/pull/123", + AssignedActorsUsed: true, + AssignedActors: api.AssignedActors{ + Nodes: []api.Actor{ + { + ID: "HUBOTID", + Login: "hubot", + TypeName: "Bot", + }, + }, + TotalCount: 1, + }, + }, ghrepo.New("OWNER", "REPO")), + Interactive: true, + Surveyor: testSurveyor{ + fieldsToEdit: func(e *shared.Editable) error { + e.Assignees.Edited = true + return nil + }, + editFields: func(e *shared.Editable, _ string) error { + // Checking that the display name is being used in the prompt. + require.Equal(t, []string{"hubot"}, e.Assignees.Default) + 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)"} + return nil + }, + }, + Fetcher: testFetcher{}, + EditorRetriever: testEditorRetriever{}, + }, + httpStubs: func(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 } + } } } } + `)) + mockPullRequestUpdate(reg) + reg.Register( + httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`), + httpmock.GraphQLMutation(` + { "data": { "replaceActorsForAssignable": { "__typename": "" } } }`, + func(inputs map[string]interface{}) { + // Checking that despite the display name being returned + // from the EditFieldsSurvey, the ID is still + // used in the mutation. + require.Subset(t, inputs["actorIds"], []string{"MONAID", "HUBOTID"}) + }), + ) + }, + stdout: "https://github.com/OWNER/REPO/pull/123\n", + }, { name: "Legacy assignee users are fetched and updated on unsupported GitHub Hosts", input: &EditOptions{ From 2266c7a5b527d32bb80939a52bddaf99dc797db0 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 3 Jun 2025 14:57:13 +0000 Subject: [PATCH 07/13] chore(deps): bump mislav/bump-homebrew-formula-action from 3.2 to 3.4 Bumps [mislav/bump-homebrew-formula-action](https://github.com/mislav/bump-homebrew-formula-action) from 3.2 to 3.4. - [Release notes](https://github.com/mislav/bump-homebrew-formula-action/releases) - [Commits](https://github.com/mislav/bump-homebrew-formula-action/compare/942e550c6344cfdb9e1ab29b9bb9bf0c43efa19b...8e2baa47daaa8db10fcdeb04105dfa6850eb0d68) --- updated-dependencies: - dependency-name: mislav/bump-homebrew-formula-action dependency-version: '3.4' dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- .github/workflows/deployment.yml | 2 +- .github/workflows/homebrew-bump.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/deployment.yml b/.github/workflows/deployment.yml index 850cc19b7..d5cc67ec9 100644 --- a/.github/workflows/deployment.yml +++ b/.github/workflows/deployment.yml @@ -384,7 +384,7 @@ jobs: git diff --name-status @{upstream}.. fi - name: Bump homebrew-core formula - uses: mislav/bump-homebrew-formula-action@942e550c6344cfdb9e1ab29b9bb9bf0c43efa19b + uses: mislav/bump-homebrew-formula-action@8e2baa47daaa8db10fcdeb04105dfa6850eb0d68 if: inputs.environment == 'production' && !contains(inputs.tag_name, '-') with: formula-name: gh diff --git a/.github/workflows/homebrew-bump.yml b/.github/workflows/homebrew-bump.yml index 228f1a345..0b42803aa 100644 --- a/.github/workflows/homebrew-bump.yml +++ b/.github/workflows/homebrew-bump.yml @@ -17,7 +17,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Bump homebrew-core formula - uses: mislav/bump-homebrew-formula-action@942e550c6344cfdb9e1ab29b9bb9bf0c43efa19b + uses: mislav/bump-homebrew-formula-action@8e2baa47daaa8db10fcdeb04105dfa6850eb0d68 if: inputs.environment == 'production' && !contains(inputs.tag_name, '-') with: formula-name: gh From bde303dab28caac33926e38d5de6591a0b4c9a48 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 10 Jun 2025 13:57:31 +0100 Subject: [PATCH 08/13] test: fix test data const Signed-off-by: Babak K. Shandiz --- pkg/cmd/pr/shared/finder_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/pr/shared/finder_test.go b/pkg/cmd/pr/shared/finder_test.go index 28c2337d3..0fd96e09b 100644 --- a/pkg/cmd/pr/shared/finder_test.go +++ b/pkg/cmd/pr/shared/finder_test.go @@ -728,7 +728,7 @@ func TestFindAssignableActors(t *testing.T) { pr, _, err := f.Find(FindOptions{ Detector: &fd.DisabledDetectorMock{}, Fields: []string{"assignees"}, - Selector: "https://github.com/cli/cli/pull/123", + Selector: "https://github.com/cli/cli/pull/13", }) require.NoError(t, err) @@ -772,7 +772,7 @@ func TestFindAssignableActors(t *testing.T) { pr, _, err := f.Find(FindOptions{ Detector: &fd.EnabledDetectorMock{}, Fields: []string{"assignees"}, - Selector: "https://github.com/cli/cli/pull/123", + Selector: "https://github.com/cli/cli/pull/13", }) require.NoError(t, err) From 52bde8a3f7b1218cb11f6369671623ac0af81b48 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 10 Jun 2025 14:52:22 +0000 Subject: [PATCH 09/13] chore(deps): bump github.com/sigstore/protobuf-specs from 0.4.2 to 0.4.3 Bumps [github.com/sigstore/protobuf-specs](https://github.com/sigstore/protobuf-specs) from 0.4.2 to 0.4.3. - [Release notes](https://github.com/sigstore/protobuf-specs/releases) - [Changelog](https://github.com/sigstore/protobuf-specs/blob/main/CHANGELOG.md) - [Commits](https://github.com/sigstore/protobuf-specs/compare/v0.4.2...v0.4.3) --- updated-dependencies: - dependency-name: github.com/sigstore/protobuf-specs dependency-version: 0.4.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 a4c973df1..49b7c8ddf 100644 --- a/go.mod +++ b/go.mod @@ -44,7 +44,7 @@ require ( github.com/opentracing/opentracing-go v1.2.0 github.com/rivo/tview v0.0.0-20221029100920-c4a7e501810d github.com/shurcooL/githubv4 v0.0.0-20240120211514-18a1ae0e79dc - github.com/sigstore/protobuf-specs v0.4.2 + github.com/sigstore/protobuf-specs v0.4.3 github.com/sigstore/sigstore-go v1.0.0 github.com/spf13/cobra v1.9.1 github.com/spf13/pflag v1.0.6 diff --git a/go.sum b/go.sum index 718e0ca67..598a713a2 100644 --- a/go.sum +++ b/go.sum @@ -455,8 +455,8 @@ github.com/shurcooL/githubv4 v0.0.0-20240120211514-18a1ae0e79dc h1:vH0NQbIDk+mJL github.com/shurcooL/githubv4 v0.0.0-20240120211514-18a1ae0e79dc/go.mod h1:zqMwyHmnN/eDOZOdiTohqIUKUrTFX62PNlu7IJdu0q8= github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466 h1:17JxqqJY66GmZVHkmAsGEkcIu0oCe3AM420QDgGwZx0= github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466/go.mod h1:9dIRpgIY7hVhoqfe0/FcYp0bpInZaT7dc3BYOprrIUE= -github.com/sigstore/protobuf-specs v0.4.2 h1:bD5bnhctpGNiR+FAEZl7N95XkN8TJFrNMIcWLunDtxA= -github.com/sigstore/protobuf-specs v0.4.2/go.mod h1:+gXR+38nIa2oEupqDdzg4qSBT0Os+sP7oYv6alWewWc= +github.com/sigstore/protobuf-specs v0.4.3 h1:kRgJ+ciznipH9xhrkAbAEHuuxD3GhYnGC873gZpjJT4= +github.com/sigstore/protobuf-specs v0.4.3/go.mod h1:+gXR+38nIa2oEupqDdzg4qSBT0Os+sP7oYv6alWewWc= github.com/sigstore/rekor v1.3.10 h1:/mSvRo4MZ/59ECIlARhyykAlQlkmeAQpvBPlmJtZOCU= github.com/sigstore/rekor v1.3.10/go.mod h1:JvryKJ40O0XA48MdzYUPu0y4fyvqt0C4iSY7ri9iu3A= github.com/sigstore/sigstore v1.9.4 h1:64+OGed80+A4mRlNzRd055vFcgBeDghjZw24rPLZgDU= From ee3db50e43915adc8dd1c3014a0ac5f491f7e50b Mon Sep 17 00:00:00 2001 From: William Martin Date: Wed, 11 Jun 2025 13:47:18 +0200 Subject: [PATCH 10/13] Avoid requesting PR reviewer twice --- api/queries_repo.go | 23 +++++++------ api/queries_repo_test.go | 72 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index efbcfcb19..3190745ea 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -709,19 +709,22 @@ func (m *RepoMetadataResult) MembersToIDs(names []string) ([]string, error) { } // Look for ID in assignable actors if not found in assignable users - for _, a := range m.AssignableActors { - if strings.EqualFold(assigneeLogin, a.Login()) { - ids = append(ids, a.ID()) - found = true - break - } - if strings.EqualFold(assigneeLogin, a.DisplayName()) { - ids = append(ids, a.ID()) - found = true - break + if !found { + for _, a := range m.AssignableActors { + if strings.EqualFold(assigneeLogin, a.Login()) { + ids = append(ids, a.ID()) + found = true + break + } + if strings.EqualFold(assigneeLogin, a.DisplayName()) { + ids = append(ids, a.ID()) + found = true + break + } } } + // And if we still didn't find an ID, return an error if !found { return nil, fmt.Errorf("'%s' not found", assigneeLogin) } diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index 9040a0018..c291fc468 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -461,6 +461,78 @@ t001: team(slug:"robots"){id,slug} } } +func TestMembersToIDs(t *testing.T) { + t.Parallel() + + t.Run("finds ids in assignable users", func(t *testing.T) { + t.Parallel() + + repoMetadataResult := RepoMetadataResult{ + AssignableUsers: []AssignableUser{ + NewAssignableUser("MONAID", "monalisa", ""), + NewAssignableUser("MONAID2", "monalisa2", ""), + }, + AssignableActors: []AssignableActor{ + NewAssignableBot("HUBOTID", "hubot"), + }, + } + ids, err := repoMetadataResult.MembersToIDs([]string{"monalisa"}) + require.NoError(t, err) + require.Equal(t, []string{"MONAID"}, ids) + }) + + t.Run("finds ids by assignable actor logins", func(t *testing.T) { + t.Parallel() + + repoMetadataResult := RepoMetadataResult{ + AssignableActors: []AssignableActor{ + NewAssignableBot("HUBOTID", "hubot"), + NewAssignableUser("MONAID", "monalisa", ""), + }, + } + ids, err := repoMetadataResult.MembersToIDs([]string{"monalisa"}) + require.NoError(t, err) + require.Equal(t, []string{"MONAID"}, ids) + }) + + t.Run("finds ids by assignable actor display names", func(t *testing.T) { + t.Parallel() + + repoMetadataResult := RepoMetadataResult{ + AssignableActors: []AssignableActor{ + NewAssignableUser("MONAID", "monalisa", "mona"), + }, + } + ids, err := repoMetadataResult.MembersToIDs([]string{"monalisa (mona)"}) + require.NoError(t, err) + require.Equal(t, []string{"MONAID"}, ids) + }) + + t.Run("when a name appears in both assignable users and actors, the id is only returned once", func(t *testing.T) { + t.Parallel() + + repoMetadataResult := RepoMetadataResult{ + AssignableUsers: []AssignableUser{ + NewAssignableUser("MONAID", "monalisa", ""), + }, + AssignableActors: []AssignableActor{ + NewAssignableUser("MONAID", "monalisa", ""), + }, + } + ids, err := repoMetadataResult.MembersToIDs([]string{"monalisa"}) + require.NoError(t, err) + require.Equal(t, []string{"MONAID"}, ids) + }) + + t.Run("when id is not found, returns an error", func(t *testing.T) { + t.Parallel() + + repoMetadataResult := RepoMetadataResult{} + _, err := repoMetadataResult.MembersToIDs([]string{"monalisa"}) + require.Error(t, err) + }) +} + func sliceEqual(a, b []string) bool { if len(a) != len(b) { return false From 1eee56ec00ae1ff0c08e03adcd0d7a4c79dd79aa Mon Sep 17 00:00:00 2001 From: Dylan Ancel Date: Wed, 11 Jun 2025 16:28:03 +0200 Subject: [PATCH 11/13] Add accurate context when `run rerun` fails (#10774) * Add accurate context when run rerun fails * Update tests to verify behaviour for API errors Signed-off-by: Babak K. Shandiz * Use the new `httpmock.JSONErrorResponse` helper Signed-off-by: Babak K. Shandiz --------- Signed-off-by: Babak K. Shandiz Co-authored-by: Babak K. Shandiz --- pkg/cmd/run/rerun/rerun.go | 5 +---- pkg/cmd/run/rerun/rerun_test.go | 39 ++++++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/run/rerun/rerun.go b/pkg/cmd/run/rerun/rerun.go index 66d5d6f64..8777e0a8a 100644 --- a/pkg/cmd/run/rerun/rerun.go +++ b/pkg/cmd/run/rerun/rerun.go @@ -202,10 +202,7 @@ func rerunRun(client *api.Client, repo ghrepo.Interface, run *shared.Run, onlyFa if err != nil { var httpError api.HTTPError if errors.As(err, &httpError) && httpError.StatusCode == 403 { - if httpError.Message == "Unable to retry this workflow run because it was created over a month ago" { - return fmt.Errorf("run %d cannot be rerun; %s", run.ID, httpError.Message) - } - return fmt.Errorf("run %d cannot be rerun; its workflow file may be broken", run.ID) + return fmt.Errorf("run %d cannot be rerun; %s", run.ID, httpError.Message) } return fmt.Errorf("failed to rerun: %w", err) } diff --git a/pkg/cmd/run/rerun/rerun_test.go b/pkg/cmd/run/rerun/rerun_test.go index 0dc74129e..77f0a922f 100644 --- a/pkg/cmd/run/rerun/rerun_test.go +++ b/pkg/cmd/run/rerun/rerun_test.go @@ -14,6 +14,7 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/go-gh/v2/pkg/api" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -374,7 +375,7 @@ func TestRerun(t *testing.T) { errOut: "no recent runs have failed; please specify a specific ``", }, { - name: "unrerunnable", + name: "API error (403)", tty: true, opts: &RerunOptions{ RunID: "3", @@ -392,10 +393,42 @@ func TestRerun(t *testing.T) { })) reg.Register( httpmock.REST("POST", "repos/OWNER/REPO/actions/runs/3/rerun"), - httpmock.StatusStringResponse(403, "no")) + httpmock.JSONErrorResponse(403, api.HTTPError{ + StatusCode: 403, + Message: "blah blah", + }), + ) }, wantErr: true, - errOut: "run 3 cannot be rerun; its workflow file may be broken", + errOut: "run 3 cannot be rerun; blah blah", + }, + { + name: "API error (non-403)", + tty: true, + opts: &RerunOptions{ + RunID: "3", + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(workflowShared.WorkflowsPayload{ + Workflows: []workflowShared.Workflow{ + shared.TestWorkflow, + }, + })) + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/actions/runs/3/rerun"), + httpmock.JSONErrorResponse(500, api.HTTPError{ + StatusCode: 500, + Message: "blah blah", + }), + ) + }, + wantErr: true, + errOut: "failed to rerun: HTTP 500: blah blah (https://api.github.com/repos/OWNER/REPO/actions/runs/3/rerun)", }, } From f8a31330031b66716fa10c8fbdfb1f3d005e5f3d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 12 Jun 2025 14:33:02 +0000 Subject: [PATCH 12/13] chore(deps): bump actions/attest-build-provenance from 2.3.0 to 2.4.0 Bumps [actions/attest-build-provenance](https://github.com/actions/attest-build-provenance) from 2.3.0 to 2.4.0. - [Release notes](https://github.com/actions/attest-build-provenance/releases) - [Changelog](https://github.com/actions/attest-build-provenance/blob/main/RELEASE.md) - [Commits](https://github.com/actions/attest-build-provenance/compare/db473fddc028af60658334401dc6fa3ffd8669fd...e8998f949152b193b063cb0ec769d69d929409be) --- updated-dependencies: - dependency-name: actions/attest-build-provenance dependency-version: 2.4.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- .github/workflows/deployment.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deployment.yml b/.github/workflows/deployment.yml index d5cc67ec9..17758a3e6 100644 --- a/.github/workflows/deployment.yml +++ b/.github/workflows/deployment.yml @@ -309,7 +309,7 @@ jobs: rpmsign --addsign dist/*.rpm - name: Attest release artifacts if: inputs.environment == 'production' - uses: actions/attest-build-provenance@db473fddc028af60658334401dc6fa3ffd8669fd # v2.3.0 + uses: actions/attest-build-provenance@e8998f949152b193b063cb0ec769d69d929409be # v2.4.0 with: subject-path: "dist/gh_*" - name: Run createrepo From 928a326cee78313df3fc7085e2e113edc06bc98c Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 16 Jun 2025 17:09:04 +0200 Subject: [PATCH 13/13] Add workflow to check `help wanted` labelling (#11105) Co-authored-by: Kynan Ware <47394200+BagToad@users.noreply.github.com> Co-authored-by: Babak K. Shandiz Co-authored-by: Andy Feller --- .github/workflows/codeql.yml | 9 +- .github/workflows/pr-help-wanted.yml | 29 ++++++ .../workflows/scripts/check-help-wanted.sh | 93 +++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/pr-help-wanted.yml create mode 100755 .github/workflows/scripts/check-help-wanted.sh diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 8cd5ecbee..d74e1c142 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -18,6 +18,10 @@ permissions: jobs: CodeQL-Build: runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + language: ['go', 'actions'] steps: - name: Check out code @@ -26,13 +30,16 @@ jobs: - name: Initialize CodeQL uses: github/codeql-action/init@v3 with: - languages: go + languages: ${{ matrix.language }} queries: security-and-quality - name: Setup Go + if: matrix.language == 'go' uses: actions/setup-go@v5 with: go-version-file: 'go.mod' - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v3 + with: + category: "/language:${{ matrix.language }}" diff --git a/.github/workflows/pr-help-wanted.yml b/.github/workflows/pr-help-wanted.yml new file mode 100644 index 000000000..5475d2eff --- /dev/null +++ b/.github/workflows/pr-help-wanted.yml @@ -0,0 +1,29 @@ +name: PR Help Wanted Check +on: + pull_request_target: + types: [opened] + +permissions: + contents: none + issues: read + pull-requests: write + +jobs: + check-help-wanted: + runs-on: ubuntu-latest + steps: + - name: Check for issues without help-wanted label + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + PR_AUTHOR_TYPE: ${{ github.event.pull_request.user.type }} + if: !github.event.pull_request.draft + run: | + # Skip if PR is from a bot or org member + if [ "$PR_AUTHOR_TYPE" = "Bot" ] || "gh api orgs/cli/public_members/${PR_AUTHOR}" --silent 2>/dev/null + then + exit 0 + fi + + # Run the script to check for issues without help-wanted label + bash .github/scripts/check-help-wanted.sh ${{ github.event.pull_request.html_url }} diff --git a/.github/workflows/scripts/check-help-wanted.sh b/.github/workflows/scripts/check-help-wanted.sh new file mode 100755 index 000000000..59c12fecd --- /dev/null +++ b/.github/workflows/scripts/check-help-wanted.sh @@ -0,0 +1,93 @@ +#!/bin/bash + +set -e + +PR_URL="$1" + +if [ -z "$PR_URL" ]; then + echo "Usage: $0 " + echo "" + echo "Check if the PR references any non-help-wanted issues and, if so, comment" + echo "on it explaining why the team might close/dismiss it." + exit 1 +fi + +# Extract PR number from URL for logging +PR_NUM="$(basename "$PR_URL")" + +# Extract cli/cli closing issues references from PR +CLOSING_ISSUES="$(gh pr view "$PR_URL" --json closingIssuesReferences --jq '.closingIssuesReferences[] | select(.repository.name == "cli" and .repository.owner.login == "cli") | .number')" + +if [ -z "$CLOSING_ISSUES" ]; then + echo "No closing issues found for PR #$PR_NUM" + exit 0 +fi + +# Check each closing issue for 'help-wanted' label +ISSUES_WITHOUT_HELP_WANTED=() + +for issue_num in $CLOSING_ISSUES; do + echo "Checking issue #$issue_num for 'help wanted' label..." + + # Get issue labels + LABELS=$(gh issue view "$issue_num" --json labels --jq '.labels[].name') + + # Skip if the issue has the gh-attestion or gh-codespace label + # This is because the codeowners for these commands may not be public + # cli org members, and so unless we authenticate with a PAT, we can't + # know who is an external contributor or not. + # So we skip these issues to avoid falsely writing a comment + # on each PR opened by these codeowners. + if echo "$LABELS" | grep -q -e "gh-attestation" -e "gh-codespace"; then + echo "Issue #$issue_num is skipped due to labels" + continue + fi + + # Check if 'help wanted' label exists + if ! echo "$LABELS" | grep -q "help wanted"; then + ISSUES_WITHOUT_HELP_WANTED+=("$issue_num") + echo "Issue #$issue_num does not have 'help wanted' label" + else + echo "Issue #$issue_num has 'help wanted' label" + fi +done + +# If we found issues without 'help wanted' label, post a comment +if [ ${#ISSUES_WITHOUT_HELP_WANTED[@]} -gt 0 ]; then + echo "Found ${#ISSUES_WITHOUT_HELP_WANTED[@]} issues without 'help wanted' label" + + # Build issue list for comment + ISSUE_LIST="" + for issue_num in "${ISSUES_WITHOUT_HELP_WANTED[@]}"; do + ISSUE_LIST="$ISSUE_LIST- #$issue_num"$'\n' + done + + # Create comment message + gh pr comment "$PR_URL" --body-file - <