Skip to content

Commit 67533fd

Browse files
RossTarrantRossTarrant
committed
fix: Empty assignees array should clear assignees
1 parent 2a5d38a commit 67533fd

2 files changed

Lines changed: 105 additions & 5 deletions

File tree

‎pkg/github/issues.go‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1793,6 +1793,11 @@ func issueWriteHasNonFormParams(args map[string]any) bool {
17931793
return false
17941794
}
17951795

1796+
func issueWriteHasNonNilParam(args map[string]any, key string) bool {
1797+
value, ok := args[key]
1798+
return ok && value != nil
1799+
}
1800+
17961801
// IssueWrite is the FeatureFlagIssueFields-enabled variant of issue_write
17971802
// (with the issue_fields parameter). LegacyIssueWrite is served when the flag
17981803
// is off. Both register under the tool name "issue_write"; exactly one is
@@ -1972,12 +1977,14 @@ Options are:
19721977
if err != nil {
19731978
return utils.NewToolResultError(err.Error()), nil, nil
19741979
}
1980+
assigneesProvided := issueWriteHasNonNilParam(args, "assignees")
19751981

19761982
// Get labels
19771983
labels, err := OptionalStringArrayParam(args, "labels")
19781984
if err != nil {
19791985
return utils.NewToolResultError(err.Error()), nil, nil
19801986
}
1987+
labelsProvided := issueWriteHasNonNilParam(args, "labels")
19811988

19821989
// Get optional milestone
19831990
milestone, err := OptionalIntParam(args, "milestone")
@@ -2049,7 +2056,10 @@ Options are:
20492056
if err != nil {
20502057
return utils.NewToolResultError(err.Error()), nil, nil
20512058
}
2052-
result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf)
2059+
result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf, UpdateIssueOptions{
2060+
AssigneesProvided: assigneesProvided,
2061+
LabelsProvided: labelsProvided,
2062+
})
20532063
return result, nil, err
20542064
default:
20552065
return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil
@@ -2204,12 +2214,14 @@ Options are:
22042214
if err != nil {
22052215
return utils.NewToolResultError(err.Error()), nil, nil
22062216
}
2217+
assigneesProvided := issueWriteHasNonNilParam(args, "assignees")
22072218

22082219
// Get labels
22092220
labels, err := OptionalStringArrayParam(args, "labels")
22102221
if err != nil {
22112222
return utils.NewToolResultError(err.Error()), nil, nil
22122223
}
2224+
labelsProvided := issueWriteHasNonNilParam(args, "labels")
22132225

22142226
// Get optional milestone
22152227
milestone, err := OptionalIntParam(args, "milestone")
@@ -2266,7 +2278,10 @@ Options are:
22662278
if err != nil {
22672279
return utils.NewToolResultError(err.Error()), nil, nil
22682280
}
2269-
result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, nil, nil, state, stateReason, duplicateOf)
2281+
result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, nil, nil, state, stateReason, duplicateOf, UpdateIssueOptions{
2282+
AssigneesProvided: assigneesProvided,
2283+
LabelsProvided: labelsProvided,
2284+
})
22702285
return result, nil, err
22712286
default:
22722287
return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil
@@ -2330,7 +2345,24 @@ func CreateIssue(ctx context.Context, client *github.Client, owner string, repo
23302345
return utils.NewToolResultText(string(r)), nil
23312346
}
23322347

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) {
2348+
// UpdateIssueOptions controls which optional fields are included in an issue update request.
2349+
type UpdateIssueOptions struct {
2350+
// AssigneesProvided sends the assignees field even when the slice is empty.
2351+
AssigneesProvided bool
2352+
// LabelsProvided sends the labels field even when the slice is empty.
2353+
LabelsProvided bool
2354+
}
2355+
2356+
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) {
2357+
updateOptions := UpdateIssueOptions{
2358+
AssigneesProvided: len(assignees) > 0,
2359+
LabelsProvided: len(labels) > 0,
2360+
}
2361+
for _, opt := range opts {
2362+
updateOptions.AssigneesProvided = updateOptions.AssigneesProvided || opt.AssigneesProvided
2363+
updateOptions.LabelsProvided = updateOptions.LabelsProvided || opt.LabelsProvided
2364+
}
2365+
23342366
// Create the issue request with only provided fields
23352367
issueRequest := &github.IssueRequest{}
23362368

@@ -2343,11 +2375,11 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4
23432375
issueRequest.Body = github.Ptr(body)
23442376
}
23452377

2346-
if len(labels) > 0 {
2378+
if updateOptions.LabelsProvided {
23472379
issueRequest.Labels = &labels
23482380
}
23492381

2350-
if len(assignees) > 0 {
2382+
if updateOptions.AssigneesProvided {
23512383
issueRequest.Assignees = &assignees
23522384
}
23532385

‎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)