diff --git a/pkg/github/issues.go b/pkg/github/issues.go index ef9bbc4305..1d995108f2 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -2390,6 +2390,14 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 issueRequest.Type = github.Ptr(issueType) } + closeWithREST := state == "closed" && stateReason != "duplicate" + if closeWithREST { + issueRequest.State = github.Ptr(state) + if stateReason != "" { + issueRequest.StateReason = github.Ptr(stateReason) + } + } + if len(issueFieldValues) > 0 || len(fieldIDsToDelete) > 0 { // The REST update endpoint uses "set" semantics — it overwrites all existing // field values with whatever is sent. Fetch the current values first, merge in @@ -2434,7 +2442,7 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 } // Use GraphQL API for state updates - if state != "" { + if state != "" && !closeWithREST { // Mandate specifying duplicateOf when trying to close as duplicate if state == "closed" && stateReason == "duplicate" && duplicateOf == 0 { return utils.NewToolResultError("duplicate_of must be provided when state_reason is 'duplicate'"), nil diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 7e47cdb527..d953989b9c 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -3108,6 +3108,38 @@ func Test_UpdateIssue(t *testing.T) { expectError: true, expectedErrMsg: "failed to update issue", }, + { + name: "close issue with labels and completed reason", + mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "labels": []any{"infra", "fixed"}, + "state": "closed", + "state_reason": "completed", + }).andThen( + mockResponse(t, http.StatusOK, &github.Issue{ + Number: github.Ptr(123), + State: github.Ptr("closed"), + StateReason: github.Ptr("completed"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + Labels: []*github.Label{{Name: github.Ptr("infra")}, {Name: github.Ptr("fixed")}}, + }), + ), + }), + mockedGQLClient: githubv4mock.NewMockedHTTPClient(), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + "labels": []any{"infra", "fixed"}, + "state": "closed", + "state_reason": "completed", + }, + expectError: false, + expectedIssue: &github.Issue{ + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + }, + }, { name: "close issue as duplicate", mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ @@ -3217,25 +3249,12 @@ func Test_UpdateIssue(t *testing.T) { { name: "main issue not found when trying to close it", mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - PatchReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockBaseIssue), + PatchReposIssuesByOwnerByRepoByIssueNumber: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), }), - mockedGQLClient: githubv4mock.NewMockedHTTPClient( - githubv4mock.NewQueryMatcher( - struct { - Repository struct { - Issue struct { - ID githubv4.ID - } `graphql:"issue(number: $issueNumber)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - }{}, - map[string]any{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), - "issueNumber": githubv4.Int(999), - }, - githubv4mock.ErrorResponse("Could not resolve to an Issue with the number of 999."), - ), - ), + mockedGQLClient: githubv4mock.NewMockedHTTPClient(), requestArgs: map[string]any{ "method": "update", "owner": "owner", @@ -3245,7 +3264,7 @@ func Test_UpdateIssue(t *testing.T) { "state_reason": "not_planned", }, expectError: true, - expectedErrMsg: "Failed to find issues", + expectedErrMsg: "failed to update issue", }, { name: "duplicate issue not found when closing as duplicate",