Skip to content

fix: Empty assignees array should clear assignees #2600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RossTarrant wants to merge 1 commit into
base: main
Choose a base branch
from
+105 −5
Open

fix: Empty assignees array should clear assignees #2600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 37 additions & 5 deletions pkg/github/issues.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Comment on lines +1796 to +1799
Copy link
Copy Markdown
Collaborator

@SamMorrowDrums SamMorrowDrums Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems a bit redundant to inline the helper.

It's both too specifically named for a generic operation and duplication to both the variants is also fine because in the end we will only be keeping one pathway.


// 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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{}

Expand All @@ -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
}

Expand Down
68 changes: 68 additions & 0 deletions pkg/github/issues_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand Down
Loading