STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge branch 'trunk' into active-token-user

Browse source
This commit is contained in:
Anuraag (Rag) Agrawal 2025-06-03 09:43:15 +09:00 • committed by GitHub
commit 85c86531b4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 169 additions and 54 deletions
Download patch file Download diff file Expand all files Collapse all files

28
api/queries_pr.go
View file

@ -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
}

2
go.mod
View file

@ -14,7 +14,7 @@ require (
github.com/charmbracelet/glamour v0.9.2-0.20250319212134-549f544650e3
github.com/charmbracelet/huh v0.7.0
github.com/charmbracelet/lipgloss v1.1.1-0.20250319133953-166f707985bc
github.com/cli/go-gh/v2 v2.12.0
github.com/cli/go-gh/v2 v2.12.1
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24
github.com/cli/oauth v1.1.1
github.com/cli/safeexec v1.0.1

4
go.sum
View file

@ -137,8 +137,8 @@ github.com/charmbracelet/x/xpty v0.1.2/go.mod h1:XK2Z0id5rtLWcpeNiMYBccNNBrP2IJn
github.com/cli/browser v1.0.0/go.mod h1:IEWkHYbLjkhtjwwWlwTHW2lGxeS5gezEQBMLTwDHf5Q=
github.com/cli/browser v1.3.0 h1:LejqCrpWr+1pRqmEPDGnTZOjsMe7sehifLynZJuqJpo=
github.com/cli/browser v1.3.0/go.mod h1:HH8s+fOAxjhQoBUAsKuPCbqUuxZDhQ2/aD+SzsEfBTk=
github.com/cli/go-gh/v2 v2.12.0 h1:PIurZ13fXbWDbr2//6ws4g4zDbryO+iDuTpiHgiV+6k=
github.com/cli/go-gh/v2 v2.12.0/go.mod h1:+5aXmEOJsH9fc9mBHfincDwnS02j2AIA/DsTH0Bk5uw=
github.com/cli/go-gh/v2 v2.12.1 h1:SVt1/afj5FRAythyMV3WJKaUfDNsxXTIe7arZbwTWKA=
github.com/cli/go-gh/v2 v2.12.1/go.mod h1:+5aXmEOJsH9fc9mBHfincDwnS02j2AIA/DsTH0Bk5uw=
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24 h1:QDrhR4JA2n3ij9YQN0u5ZeuvRIIvsUGmf5yPlTS0w8E=
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24/go.mod h1:rr9GNING0onuVw8MnracQHn7PcchnFlP882Y0II2KZk=
github.com/cli/oauth v1.1.1 h1:459gD3hSjlKX9B1uXBuiAMdpXBUQ9QGf/NDcCpoQxPs=

4
pkg/cmd/issue/edit/edit.go
View file

@ -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`")

30
pkg/cmd/pr/edit/edit.go
View file

@ -3,7 +3,6 @@ package edit
import (
"fmt"
"net/http"
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
@ -171,8 +170,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`")
@ -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 {

24
pkg/cmd/pr/edit/edit_test.go
View file

@ -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},

31
pkg/cmd/pr/shared/finder.go
View file

@ -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()
}

76
pkg/cmd/pr/shared/finder_test.go
View file

@ -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

2
pkg/cmd/release/shared/fetch.go
View file

@ -71,6 +71,7 @@ type ReleaseAsset struct {
Name string
Label string
Size int64
Digest *string
State string
APIURL string `json:"url"`
@ -107,6 +108,7 @@ func (rel *Release) ExportData(fields []string) map[string]interface{} {
"name": a.Name,
"label": a.Label,
"size": a.Size,
"digest": a.Digest,
"state": a.State,
"createdAt": a.CreatedAt,
"updatedAt": a.UpdatedAt,

8
pkg/cmd/release/view/view.go
View file

@ -154,10 +154,14 @@ func renderReleaseTTY(io *iostreams.IOStreams, release *shared.Release) error {
if len(release.Assets) > 0 {
fmt.Fprintln(w, cs.Bold("Assets"))
//nolint:staticcheck // SA1019: Showing NAME|SIZE headers adds nothing to table.
table := tableprinter.New(io, tableprinter.NoHeader)
table := tableprinter.New(io, tableprinter.WithHeader("Name", "Digest", "Size"))
for _, a := range release.Assets {
table.AddField(a.Name)
if a.Digest == nil {
table.AddField("")
} else {
table.AddField(*a.Digest)
}
table.AddField(humanFileSize(a.Size))
table.EndRow()
}

14
pkg/cmd/release/view/view_test.go
View file

@ -150,8 +150,9 @@ func Test_viewRun(t *testing.T) {
Assets
windows.zip 12 B
linux.tgz 34 B
NAME DIGEST SIZE
windows.zip sha256:deadc0de 12 B
linux.tgz 34 B
View on GitHub: https://github.com/OWNER/REPO/releases/tags/v1.2.3
`),
@ -174,8 +175,9 @@ func Test_viewRun(t *testing.T) {
Assets
windows.zip 12 B
linux.tgz 34 B
NAME DIGEST SIZE
windows.zip sha256:deadc0de 12 B
linux.tgz 34 B
View on GitHub: https://github.com/OWNER/REPO/releases/tags/v1.2.3
`),
@ -248,8 +250,8 @@ func Test_viewRun(t *testing.T) {
"published_at": "%[1]s",
"html_url": "https://github.com/OWNER/REPO/releases/tags/v1.2.3",
"assets": [
{ "name": "windows.zip", "size": 12 },
{ "name": "linux.tgz", "size": 34 }
{ "name": "windows.zip", "size": 12, "digest": "sha256:deadc0de" },
{ "name": "linux.tgz", "size": 34, "digest": null }
]
}`, tt.releasedAt.Format(time.RFC3339), tt.releaseBody))