Skip to content

Commit 03febb4

Browse files
committed
fix: close issues with REST state updates
1 parent 3422703 commit 03febb4

2 files changed

Lines changed: 47 additions & 20 deletions

File tree

pkg/github/issues.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2379,6 +2379,14 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4
23792379
issueRequest.Type = github.Ptr(issueType)
23802380
}
23812381

2382+
closeWithREST := state == "closed" && stateReason != "duplicate"
2383+
if closeWithREST {
2384+
issueRequest.State = github.Ptr(state)
2385+
if stateReason != "" {
2386+
issueRequest.StateReason = github.Ptr(stateReason)
2387+
}
2388+
}
2389+
23822390
if len(issueFieldValues) > 0 || len(fieldIDsToDelete) > 0 {
23832391
// The REST update endpoint uses "set" semantics — it overwrites all existing
23842392
// field values with whatever is sent. Fetch the current values first, merge in
@@ -2423,7 +2431,7 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4
24232431
}
24242432

24252433
// Use GraphQL API for state updates
2426-
if state != "" {
2434+
if state != "" && !closeWithREST {
24272435
// Mandate specifying duplicateOf when trying to close as duplicate
24282436
if state == "closed" && stateReason == "duplicate" && duplicateOf == 0 {
24292437
return utils.NewToolResultError("duplicate_of must be provided when state_reason is 'duplicate'"), nil

pkg/github/issues_test.go

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3108,6 +3108,38 @@ func Test_UpdateIssue(t *testing.T) {
31083108
expectError: true,
31093109
expectedErrMsg: "failed to update issue",
31103110
},
3111+
{
3112+
name: "close issue with labels and completed reason",
3113+
mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
3114+
PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{
3115+
"labels": []any{"infra", "fixed"},
3116+
"state": "closed",
3117+
"state_reason": "completed",
3118+
}).andThen(
3119+
mockResponse(t, http.StatusOK, &github.Issue{
3120+
Number: github.Ptr(123),
3121+
State: github.Ptr("closed"),
3122+
StateReason: github.Ptr("completed"),
3123+
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"),
3124+
Labels: []*github.Label{{Name: github.Ptr("infra")}, {Name: github.Ptr("fixed")}},
3125+
}),
3126+
),
3127+
}),
3128+
mockedGQLClient: githubv4mock.NewMockedHTTPClient(),
3129+
requestArgs: map[string]any{
3130+
"method": "update",
3131+
"owner": "owner",
3132+
"repo": "repo",
3133+
"issue_number": float64(123),
3134+
"labels": []any{"infra", "fixed"},
3135+
"state": "closed",
3136+
"state_reason": "completed",
3137+
},
3138+
expectError: false,
3139+
expectedIssue: &github.Issue{
3140+
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"),
3141+
},
3142+
},
31113143
{
31123144
name: "close issue as duplicate",
31133145
mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -3217,25 +3249,12 @@ func Test_UpdateIssue(t *testing.T) {
32173249
{
32183250
name: "main issue not found when trying to close it",
32193251
mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
3220-
PatchReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockBaseIssue),
3252+
PatchReposIssuesByOwnerByRepoByIssueNumber: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
3253+
w.WriteHeader(http.StatusNotFound)
3254+
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
3255+
}),
32213256
}),
3222-
mockedGQLClient: githubv4mock.NewMockedHTTPClient(
3223-
githubv4mock.NewQueryMatcher(
3224-
struct {
3225-
Repository struct {
3226-
Issue struct {
3227-
ID githubv4.ID
3228-
} `graphql:"issue(number: $issueNumber)"`
3229-
} `graphql:"repository(owner: $owner, name: $repo)"`
3230-
}{},
3231-
map[string]any{
3232-
"owner": githubv4.String("owner"),
3233-
"repo": githubv4.String("repo"),
3234-
"issueNumber": githubv4.Int(999),
3235-
},
3236-
githubv4mock.ErrorResponse("Could not resolve to an Issue with the number of 999."),
3237-
),
3238-
),
3257+
mockedGQLClient: githubv4mock.NewMockedHTTPClient(),
32393258
requestArgs: map[string]any{
32403259
"method": "update",
32413260
"owner": "owner",
@@ -3245,7 +3264,7 @@ func Test_UpdateIssue(t *testing.T) {
32453264
"state_reason": "not_planned",
32463265
},
32473266
expectError: true,
3248-
expectedErrMsg: "Failed to find issues",
3267+
expectedErrMsg: "failed to update issue",
32493268
},
32503269
{
32513270
name: "duplicate issue not found when closing as duplicate",

0 commit comments

Comments
 (0)