Merge branch 'trunk' into eugene/release-verify

This commit is contained in:
Eugene 2025-06-16 08:49:58 -07:00 committed by GitHub
commit 8d0161fa5d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 568 additions and 121 deletions

View file

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

View file

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

View file

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

29
.github/workflows/pr-help-wanted.yml vendored Normal file
View file

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

View file

@ -0,0 +1,93 @@
#!/bin/bash
set -e
PR_URL="$1"
if [ -z "$PR_URL" ]; then
echo "Usage: $0 <PR_URL>"
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 - <<EOF
Thank you for your pull request! 🎉
This PR appears to fix the following issues that are not labeled with \`help wanted\`:
$ISSUE_LIST
As outlined in our [Contributing Guidelines](https://github.com/cli/cli/blob/trunk/.github/CONTRIBUTING.md), we expect that PRs are only created for issues that have been labeled \`help wanted\`.
While we appreciate your initiative, please note that:
- **PRs for non-\`help wanted\` issues may not be reviewed immediately** as they might not align with our current priorities
- **The issue might already be assigned** to a team member or planned for a specific release
- **We may need to close this PR**. For example, if it conflicts with ongoing work or architectural decisions
**What happens next:**
- Our team will review this PR and the associated issues
- We may add the \`help wanted\` label to the issues, if appropriate, and review this pull request
- In some cases, we may need to close the PR. For example, if it doesn't fit our current roadmap
Thank you for your understanding and contribution to the project! 🙏
*This comment was automatically generated by cliAutomation.*
EOF
echo "Posted comment on PR #$PR_NUM"
else
echo "All closing issues have 'help wanted' label - no action needed"
fi

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
}

View file

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

View file

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

4
go.mod
View file

@ -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
@ -56,7 +56,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

8
go.sum
View file

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

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

View file

@ -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, []string{"hubot"}, eo.Assignees.Default)
require.Equal(t, []string{"hubot"}, eo.Assignees.DefaultLogins)
// 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)),
)

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,9 +225,10 @@ 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()
editable.Assignees.DefaultLogins = pr.AssignedActors.Logins()
} else {
editable.Assignees.Default = pr.Assignees.Logins()
}

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,12 +526,37 @@ 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{},
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{},
},
@ -542,12 +573,35 @@ 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},
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{},
},
@ -563,12 +617,37 @@ 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},
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.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{},
},
@ -582,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{
@ -654,7 +798,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 }
} } } }
@ -801,48 +945,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
}

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

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/13",
})
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/13",
})
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

View file

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

View file

@ -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 `<run-id>`",
},
{
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)",
},
}