From 67533fdaa49f3ec5e9b40300c6057aaf67abdbcb Mon Sep 17 00:00:00 2001 From: RossTarrant Date: Wed, 3 Jun 2026 10:39:00 +0100 Subject: [PATCH 1/2] fix: Empty assignees array should clear assignees --- pkg/github/issues.go | 42 +++++++++++++++++++++--- pkg/github/issues_test.go | 68 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 5 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 6e9cdae53..034802ea6 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1793,6 +1793,11 @@ func issueWriteHasNonFormParams(args map[string]any) bool { return false } +func issueWriteHasNonNilParam(args map[string]any, key string) bool { + value, ok := args[key] + return ok && value != nil +} + // IssueWrite is the FeatureFlagIssueFields-enabled variant of issue_write // (with the issue_fields parameter). LegacyIssueWrite is served when the flag // is off. Both register under the tool name "issue_write"; exactly one is @@ -1972,12 +1977,14 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + assigneesProvided := issueWriteHasNonNilParam(args, "assignees") // Get labels labels, err := OptionalStringArrayParam(args, "labels") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + labelsProvided := issueWriteHasNonNilParam(args, "labels") // Get optional milestone milestone, err := OptionalIntParam(args, "milestone") @@ -2049,7 +2056,10 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf) + result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf, UpdateIssueOptions{ + AssigneesProvided: assigneesProvided, + LabelsProvided: labelsProvided, + }) return result, nil, err default: return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil @@ -2204,12 +2214,14 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + assigneesProvided := issueWriteHasNonNilParam(args, "assignees") // Get labels labels, err := OptionalStringArrayParam(args, "labels") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + labelsProvided := issueWriteHasNonNilParam(args, "labels") // Get optional milestone milestone, err := OptionalIntParam(args, "milestone") @@ -2266,7 +2278,10 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, nil, nil, state, stateReason, duplicateOf) + result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, nil, nil, state, stateReason, duplicateOf, UpdateIssueOptions{ + AssigneesProvided: assigneesProvided, + LabelsProvided: labelsProvided, + }) return result, nil, err default: 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 return utils.NewToolResultText(string(r)), nil } -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) { +// UpdateIssueOptions controls which optional fields are included in an issue update request. +type UpdateIssueOptions struct { + // AssigneesProvided sends the assignees field even when the slice is empty. + AssigneesProvided bool + // LabelsProvided sends the labels field even when the slice is empty. + LabelsProvided bool +} + +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) { + updateOptions := UpdateIssueOptions{ + AssigneesProvided: len(assignees) > 0, + LabelsProvided: len(labels) > 0, + } + for _, opt := range opts { + updateOptions.AssigneesProvided = updateOptions.AssigneesProvided || opt.AssigneesProvided + updateOptions.LabelsProvided = updateOptions.LabelsProvided || opt.LabelsProvided + } + // Create the issue request with only provided fields issueRequest := &github.IssueRequest{} @@ -2343,11 +2375,11 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 issueRequest.Body = github.Ptr(body) } - if len(labels) > 0 { + if updateOptions.LabelsProvided { issueRequest.Labels = &labels } - if len(assignees) > 0 { + if updateOptions.AssigneesProvided { issueRequest.Assignees = &assignees } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index d794ad167..7e47cdb52 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2987,6 +2987,33 @@ func Test_UpdateIssue(t *testing.T) { expectError: false, expectedIssue: mockUpdatedIssue, }, + { + name: "partial update clears labels and assignees", + mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "labels": []any{}, + "assignees": []any{}, + }).andThen( + mockResponse(t, http.StatusOK, &github.Issue{ + Number: github.Ptr(123), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + }), + ), + }), + mockedGQLClient: githubv4mock.NewMockedHTTPClient(), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + "labels": []any{}, + "assignees": []any{}, + }, + expectError: false, + expectedIssue: &github.Issue{ + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + }, + }, { name: "partial update with issue fields reconciled by names", mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ @@ -3406,6 +3433,47 @@ func Test_UpdateIssue(t *testing.T) { } } +func Test_LegacyUpdateIssueClearsLabelsAndAssignees(t *testing.T) { + serverTool := LegacyIssueWrite(translations.NullTranslationHelper) + updatedIssue := &github.Issue{ + Number: github.Ptr(8), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/8"), + } + + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "labels": []any{}, + "assignees": []any{}, + }).andThen(mockResponse(t, http.StatusOK, updatedIssue)), + })) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + deps := BaseDeps{ + Client: client, + GQLClient: gqlClient, + } + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(8), + "labels": []any{}, + "assignees": []any{}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + if result.IsError { + t.Fatalf("Unexpected error result: %s", getErrorResult(t, result).Text) + } + textContent := getTextResult(t, result) + + var updateResp MinimalResponse + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &updateResp)) + assert.Equal(t, updatedIssue.GetHTMLURL(), updateResp.URL) +} + func Test_ParseISOTimestamp(t *testing.T) { tests := []struct { name string From 4ad4352769b11d10585353607b0ca3ced7e462b5 Mon Sep 17 00:00:00 2001 From: RossTarrant Date: Thu, 4 Jun 2026 09:04:26 +0100 Subject: [PATCH 2/2] Address feedback --- pkg/github/issues.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 034802ea6..ef9bbc430 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1793,11 +1793,6 @@ func issueWriteHasNonFormParams(args map[string]any) bool { return false } -func issueWriteHasNonNilParam(args map[string]any, key string) bool { - value, ok := args[key] - return ok && value != nil -} - // IssueWrite is the FeatureFlagIssueFields-enabled variant of issue_write // (with the issue_fields parameter). LegacyIssueWrite is served when the flag // is off. Both register under the tool name "issue_write"; exactly one is @@ -1977,14 +1972,16 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - assigneesProvided := issueWriteHasNonNilParam(args, "assignees") + assigneesValue, assigneesProvided := args["assignees"] + assigneesProvided = assigneesProvided && assigneesValue != nil // Get labels labels, err := OptionalStringArrayParam(args, "labels") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - labelsProvided := issueWriteHasNonNilParam(args, "labels") + labelsValue, labelsProvided := args["labels"] + labelsProvided = labelsProvided && labelsValue != nil // Get optional milestone milestone, err := OptionalIntParam(args, "milestone") @@ -2214,14 +2211,16 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - assigneesProvided := issueWriteHasNonNilParam(args, "assignees") + assigneesValue, assigneesProvided := args["assignees"] + assigneesProvided = assigneesProvided && assigneesValue != nil // Get labels labels, err := OptionalStringArrayParam(args, "labels") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - labelsProvided := issueWriteHasNonNilParam(args, "labels") + labelsValue, labelsProvided := args["labels"] + labelsProvided = labelsProvided && labelsValue != nil // Get optional milestone milestone, err := OptionalIntParam(args, "milestone")