Skip to content

Commit 33849e9

Browse files
RossTarrantRossTarrant
authored
fix: Empty assignees array should clear assignees (#2600)
* fix: Empty assignees array should clear assignees
1 parent 2a5d38a commit 33849e9

2 files changed

Lines changed: 104 additions & 5 deletions

File tree

‎pkg/github/issues.go‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1972,12 +1972,16 @@ Options are:
19721972
if err != nil {
19731973
return utils.NewToolResultError(err.Error()), nil, nil
19741974
}
1975+
assigneesValue, assigneesProvided := args["assignees"]
1976+
assigneesProvided = assigneesProvided && assigneesValue != nil
19751977

19761978
// Get labels
19771979
labels, err := OptionalStringArrayParam(args, "labels")
19781980
if err != nil {
19791981
return utils.NewToolResultError(err.Error()), nil, nil
19801982
}
1983+
labelsValue, labelsProvided := args["labels"]
1984+
labelsProvided = labelsProvided && labelsValue != nil
19811985

19821986
// Get optional milestone
19831987
milestone, err := OptionalIntParam(args, "milestone")
@@ -2049,7 +2053,10 @@ Options are:
20492053
if err != nil {
20502054
return utils.NewToolResultError(err.Error()), nil, nil
20512055
}
2052-
result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf)
2056+
result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf, UpdateIssueOptions{
2057+
AssigneesProvided: assigneesProvided,
2058+
LabelsProvided: labelsProvided,
2059+
})
20532060
return result, nil, err
20542061
default:
20552062
return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil
@@ -2204,12 +2211,16 @@ Options are:
22042211
if err != nil {
22052212
return utils.NewToolResultError(err.Error()), nil, nil
22062213
}
2214+
assigneesValue, assigneesProvided := args["assignees"]
2215+
assigneesProvided = assigneesProvided && assigneesValue != nil
22072216

22082217
// Get labels
22092218
labels, err := OptionalStringArrayParam(args, "labels")
22102219
if err != nil {
22112220
return utils.NewToolResultError(err.Error()), nil, nil
22122221
}
2222+
labelsValue, labelsProvided := args["labels"]
2223+
labelsProvided = labelsProvided && labelsValue != nil
22132224

22142225
// Get optional milestone
22152226
milestone, err := OptionalIntParam(args, "milestone")
@@ -2266,7 +2277,10 @@ Options are:
22662277
if err != nil {
22672278
return utils.NewToolResultError(err.Error()), nil, nil
22682279
}
2269-
result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, nil, nil, state, stateReason, duplicateOf)
2280+
result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, nil, nil, state, stateReason, duplicateOf, UpdateIssueOptions{
2281+
AssigneesProvided: assigneesProvided,
2282+
LabelsProvided: labelsProvided,
2283+
})
22702284
return result, nil, err
22712285
default:
22722286
return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil
@@ -2330,7 +2344,24 @@ func CreateIssue(ctx context.Context, client *github.Client, owner string, repo
23302344
return utils.NewToolResultText(string(r)), nil
23312345
}
23322346

2333-
func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue, fieldIDsToDelete []int64, state string, stateReason string, duplicateOf int) (*mcp.CallToolResult, error) {
2347+
// UpdateIssueOptions controls which optional fields are included in an issue update request.
2348+
type UpdateIssueOptions struct {
2349+
// AssigneesProvided sends the assignees field even when the slice is empty.
2350+
AssigneesProvided bool
2351+
// LabelsProvided sends the labels field even when the slice is empty.
2352+
LabelsProvided bool
2353+
}
2354+
2355+
func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue, fieldIDsToDelete []int64, state string, stateReason string, duplicateOf int, opts ...UpdateIssueOptions) (*mcp.CallToolResult, error) {
2356+
updateOptions := UpdateIssueOptions{
2357+
AssigneesProvided: len(assignees) > 0,
2358+
LabelsProvided: len(labels) > 0,
2359+
}
2360+
for _, opt := range opts {
2361+
updateOptions.AssigneesProvided = updateOptions.AssigneesProvided || opt.AssigneesProvided
2362+
updateOptions.LabelsProvided = updateOptions.LabelsProvided || opt.LabelsProvided
2363+
}
2364+
23342365
// Create the issue request with only provided fields
23352366
issueRequest := &github.IssueRequest{}
23362367

@@ -2343,11 +2374,11 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4
23432374
issueRequest.Body = github.Ptr(body)
23442375
}
23452376

2346-
if len(labels) > 0 {
2377+
if updateOptions.LabelsProvided {
23472378
issueRequest.Labels = &labels
23482379
}
23492380

2350-
if len(assignees) > 0 {
2381+
if updateOptions.AssigneesProvided {
23512382
issueRequest.Assignees = &assignees
23522383
}
23532384

‎pkg/github/issues_test.go‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2987,6 +2987,33 @@ func Test_UpdateIssue(t *testing.T) {
29872987
expectError: false,
29882988
expectedIssue: mockUpdatedIssue,
29892989
},
2990+
{
2991+
name: "partial update clears labels and assignees",
2992+
mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
2993+
PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{
2994+
"labels": []any{},
2995+
"assignees": []any{},
2996+
}).andThen(
2997+
mockResponse(t, http.StatusOK, &github.Issue{
2998+
Number: github.Ptr(123),
2999+
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"),
3000+
}),
3001+
),
3002+
}),
3003+
mockedGQLClient: githubv4mock.NewMockedHTTPClient(),
3004+
requestArgs: map[string]any{
3005+
"method": "update",
3006+
"owner": "owner",
3007+
"repo": "repo",
3008+
"issue_number": float64(123),
3009+
"labels": []any{},
3010+
"assignees": []any{},
3011+
},
3012+
expectError: false,
3013+
expectedIssue: &github.Issue{
3014+
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"),
3015+
},
3016+
},
29903017
{
29913018
name: "partial update with issue fields reconciled by names",
29923019
mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -3406,6 +3433,47 @@ func Test_UpdateIssue(t *testing.T) {
34063433
}
34073434
}
34083435

3436+
func Test_LegacyUpdateIssueClearsLabelsAndAssignees(t *testing.T) {
3437+
serverTool := LegacyIssueWrite(translations.NullTranslationHelper)
3438+
updatedIssue := &github.Issue{
3439+
Number: github.Ptr(8),
3440+
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/8"),
3441+
}
3442+
3443+
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
3444+
PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{
3445+
"labels": []any{},
3446+
"assignees": []any{},
3447+
}).andThen(mockResponse(t, http.StatusOK, updatedIssue)),
3448+
}))
3449+
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient())
3450+
deps := BaseDeps{
3451+
Client: client,
3452+
GQLClient: gqlClient,
3453+
}
3454+
handler := serverTool.Handler(deps)
3455+
3456+
request := createMCPRequest(map[string]any{
3457+
"method": "update",
3458+
"owner": "owner",
3459+
"repo": "repo",
3460+
"issue_number": float64(8),
3461+
"labels": []any{},
3462+
"assignees": []any{},
3463+
})
3464+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
3465+
3466+
require.NoError(t, err)
3467+
if result.IsError {
3468+
t.Fatalf("Unexpected error result: %s", getErrorResult(t, result).Text)
3469+
}
3470+
textContent := getTextResult(t, result)
3471+
3472+
var updateResp MinimalResponse
3473+
require.NoError(t, json.Unmarshal([]byte(textContent.Text), &updateResp))
3474+
assert.Equal(t, updatedIssue.GetHTMLURL(), updateResp.URL)
3475+
}
3476+
34093477
func Test_ParseISOTimestamp(t *testing.T) {
34103478
tests := []struct {
34113479
name string

0 commit comments

Comments
 (0)