From afa16f17d83b6685dc47f78a20ee384b6bee122e Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Thu, 16 Apr 2026 13:43:45 +0100 Subject: [PATCH 1/2] feat: add granular tool to set issue field values --- .../__toolsnaps__/set_issue_fields.snap | 70 ++++++ pkg/github/granular_tools_test.go | 134 +++++++++++ pkg/github/issues_granular.go | 226 ++++++++++++++++++ pkg/github/tools.go | 1 + 4 files changed, 431 insertions(+) create mode 100644 pkg/github/__toolsnaps__/set_issue_fields.snap diff --git a/pkg/github/__toolsnaps__/set_issue_fields.snap b/pkg/github/__toolsnaps__/set_issue_fields.snap new file mode 100644 index 000000000..7546ddc37 --- /dev/null +++ b/pkg/github/__toolsnaps__/set_issue_fields.snap @@ -0,0 +1,70 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Set Issue Fields" + }, + "description": "Set issue field values for an issue. Fields are organization-level custom fields (text, number, date, or single select). Use this to create or update field values on an issue.", + "inputSchema": { + "properties": { + "fields": { + "description": "Array of issue field values to set. Each element must have a 'field_id' (string, the GraphQL node ID of the field) and exactly one value field: 'text_value' for text fields, 'number_value' for number fields, 'date_value' (ISO 8601 date string) for date fields, or 'single_select_option_id' (the GraphQL node ID of the option) for single select fields. Set 'delete' to true to remove a field value.", + "items": { + "properties": { + "date_value": { + "description": "The value to set for a date field (ISO 8601 date string)", + "type": "string" + }, + "delete": { + "description": "Set to true to delete this field value", + "type": "boolean" + }, + "field_id": { + "description": "The GraphQL node ID of the issue field", + "type": "string" + }, + "number_value": { + "description": "The value to set for a number field", + "type": "number" + }, + "single_select_option_id": { + "description": "The GraphQL node ID of the option to set for a single select field", + "type": "string" + }, + "text_value": { + "description": "The value to set for a text field", + "type": "string" + } + }, + "required": [ + "field_id" + ], + "type": "object" + }, + "minItems": 1, + "type": "array" + }, + "issue_number": { + "description": "The issue number to update", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "fields" + ], + "type": "object" + }, + "name": "set_issue_fields" +} \ No newline at end of file diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index d50f6c552..4d7996e96 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -39,6 +39,7 @@ func TestGranularToolSnaps(t *testing.T) { GranularAddSubIssue, GranularRemoveSubIssue, GranularReprioritizeSubIssue, + GranularSetIssueFields, GranularUpdatePullRequestTitle, GranularUpdatePullRequestBody, GranularUpdatePullRequestState, @@ -81,6 +82,7 @@ func TestIssuesGranularToolset(t *testing.T) { "add_sub_issue", "remove_sub_issue", "reprioritize_sub_issue", + "set_issue_fields", } for _, name := range expected { assert.Contains(t, toolNames, name) @@ -774,3 +776,135 @@ func TestGranularUnresolveReviewThread(t *testing.T) { require.NoError(t, err) assert.False(t, result.IsError) } + +func TestGranularSetIssueFields(t *testing.T) { + t.Run("successful set with text value", func(t *testing.T) { + matchers := []githubv4mock.Matcher{ + // Mock the issue ID query + githubv4mock.NewQueryMatcher( + struct { + Repository struct { + Issue struct { + ID githubv4.ID + } `graphql:"issue(number: $issueNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "issueNumber": githubv4.Int(5), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{"id": "ISSUE_123"}, + }, + }), + ), + // Mock the setIssueFieldValue mutation + githubv4mock.NewMutationMatcher( + struct { + SetIssueFieldValue struct { + Issue struct { + ID githubv4.ID + Number githubv4.Int + URL githubv4.String + } + IssueFieldValues []struct { + Field struct { + Name string + } `graphql:"... on IssueFieldDateValue"` + } + } `graphql:"setIssueFieldValue(input: $input)"` + }{}, + SetIssueFieldValueInput{ + IssueID: githubv4.ID("ISSUE_123"), + IssueFields: []IssueFieldCreateOrUpdateInput{ + { + FieldID: githubv4.ID("FIELD_1"), + TextValue: githubv4.NewString(githubv4.String("hello")), + }, + }, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "setIssueFieldValue": map[string]any{ + "issue": map[string]any{ + "id": "ISSUE_123", + "number": 5, + "url": "https://github.com/owner/repo/issues/5", + }, + }, + }), + ), + } + + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matchers...)) + deps := BaseDeps{GQLClient: gqlClient} + serverTool := GranularSetIssueFields(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(5), + "fields": []any{ + map[string]any{"field_id": "FIELD_1", "text_value": "hello"}, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) + }) + + t.Run("missing required parameter fields", func(t *testing.T) { + deps := BaseDeps{} + serverTool := GranularSetIssueFields(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(5), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "missing required parameter: fields") + }) + + t.Run("empty fields array", func(t *testing.T) { + deps := BaseDeps{} + serverTool := GranularSetIssueFields(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(5), + "fields": []any{}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "fields array must not be empty") + }) + + t.Run("field missing value", func(t *testing.T) { + deps := BaseDeps{} + serverTool := GranularSetIssueFields(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(5), + "fields": []any{ + map[string]any{"field_id": "FIELD_1"}, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "each field must have a value") + }) +} diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 3daa1a62e..107f8a74f 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -15,6 +15,7 @@ import ( "github.com/google/go-github/v82/github" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/shurcooL/githubv4" ) // issueUpdateTool is a helper to create single-field issue update tools. @@ -593,3 +594,228 @@ func GranularReprioritizeSubIssue(t translations.TranslationHelperFunc) inventor st.FeatureFlagEnable = FeatureFlagIssuesGranular return st } + +// SetIssueFieldValueInput represents the input for the setIssueFieldValue GraphQL mutation. +type SetIssueFieldValueInput struct { + IssueID githubv4.ID `json:"issueId"` + IssueFields []IssueFieldCreateOrUpdateInput `json:"issueFields"` + ClientMutationID *githubv4.String `json:"clientMutationId,omitempty"` +} + +// IssueFieldCreateOrUpdateInput represents a single field value to set on an issue. +type IssueFieldCreateOrUpdateInput struct { + FieldID githubv4.ID `json:"fieldId"` + TextValue *githubv4.String `json:"textValue,omitempty"` + NumberValue *githubv4.Float `json:"numberValue,omitempty"` + DateValue *githubv4.String `json:"dateValue,omitempty"` + SingleSelectOptionID *githubv4.ID `json:"singleSelectOptionId,omitempty"` + Delete *githubv4.Boolean `json:"delete,omitempty"` +} + +// GranularSetIssueFields creates a tool to set issue field values on an issue using GraphQL. +func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "set_issue_fields", + Description: t("TOOL_SET_ISSUE_FIELDS_DESCRIPTION", "Set issue field values for an issue. Fields are organization-level custom fields (text, number, date, or single select). Use this to create or update field values on an issue."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_SET_ISSUE_FIELDS_USER_TITLE", "Set Issue Fields"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "issue_number": { + Type: "number", + Description: "The issue number to update", + Minimum: jsonschema.Ptr(1.0), + }, + "fields": { + Type: "array", + Description: "Array of issue field values to set. Each element must have a 'field_id' (string, the GraphQL node ID of the field) and exactly one value field: 'text_value' for text fields, 'number_value' for number fields, 'date_value' (ISO 8601 date string) for date fields, or 'single_select_option_id' (the GraphQL node ID of the option) for single select fields. Set 'delete' to true to remove a field value.", + MinItems: jsonschema.Ptr(1), + Items: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "field_id": { + Type: "string", + Description: "The GraphQL node ID of the issue field", + }, + "text_value": { + Type: "string", + Description: "The value to set for a text field", + }, + "number_value": { + Type: "number", + Description: "The value to set for a number field", + }, + "date_value": { + Type: "string", + Description: "The value to set for a date field (ISO 8601 date string)", + }, + "single_select_option_id": { + Type: "string", + Description: "The GraphQL node ID of the option to set for a single select field", + }, + "delete": { + Type: "boolean", + Description: "Set to true to delete this field value", + }, + }, + Required: []string{"field_id"}, + }, + }, + }, + Required: []string{"owner", "repo", "issue_number", "fields"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + fieldsRaw, ok := args["fields"] + if !ok { + return utils.NewToolResultError("missing required parameter: fields"), nil, nil + } + + // Accept both []any and []map[string]any input forms + var fieldMaps []map[string]any + switch v := fieldsRaw.(type) { + case []any: + for _, f := range v { + fieldMap, ok := f.(map[string]any) + if !ok { + return utils.NewToolResultError("each field must be an object with 'field_id' and a value"), nil, nil + } + fieldMaps = append(fieldMaps, fieldMap) + } + case []map[string]any: + fieldMaps = v + default: + return utils.NewToolResultError("invalid parameter: fields must be an array"), nil, nil + } + if len(fieldMaps) == 0 { + return utils.NewToolResultError("fields array must not be empty"), nil, nil + } + + issueFields := make([]IssueFieldCreateOrUpdateInput, 0, len(fieldMaps)) + for _, fieldMap := range fieldMaps { + fieldID, err := RequiredParam[string](fieldMap, "field_id") + if err != nil { + return utils.NewToolResultError("field_id is required and must be a string"), nil, nil + } + + input := IssueFieldCreateOrUpdateInput{ + FieldID: githubv4.ID(fieldID), + } + + // Check for exactly one value type (or delete) + hasValue := false + + if v, err := OptionalParam[string](fieldMap, "text_value"); err == nil && v != "" { + input.TextValue = githubv4.NewString(githubv4.String(v)) + hasValue = true + } + if v, err := OptionalParam[float64](fieldMap, "number_value"); err == nil { + if _, exists := fieldMap["number_value"]; exists { + gqlFloat := githubv4.Float(v) + input.NumberValue = &gqlFloat + hasValue = true + } + } + if v, err := OptionalParam[string](fieldMap, "date_value"); err == nil && v != "" { + input.DateValue = githubv4.NewString(githubv4.String(v)) + hasValue = true + } + if v, err := OptionalParam[string](fieldMap, "single_select_option_id"); err == nil && v != "" { + optionID := githubv4.ID(v) + input.SingleSelectOptionID = &optionID + hasValue = true + } + if _, exists := fieldMap["delete"]; exists { + del, err := OptionalParam[bool](fieldMap, "delete") + if err == nil && del { + deleteVal := githubv4.Boolean(true) + input.Delete = &deleteVal + hasValue = true + } + } + + if !hasValue { + return utils.NewToolResultError("each field must have a value (text_value, number_value, date_value, single_select_option_id) or delete: true"), nil, nil + } + + issueFields = append(issueFields, input) + } + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + } + + // Resolve issue node ID + issueID, _, err := fetchIssueIDs(ctx, gqlClient, owner, repo, issueNumber, 0) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to get issue", err), nil, nil + } + + // Execute the setIssueFieldValue mutation + var mutation struct { + SetIssueFieldValue struct { + Issue struct { + ID githubv4.ID + Number githubv4.Int + URL githubv4.String + } + IssueFieldValues []struct { + Field struct { + Name string + } `graphql:"... on IssueFieldDateValue"` + } + } `graphql:"setIssueFieldValue(input: $input)"` + } + + mutationInput := SetIssueFieldValueInput{ + IssueID: issueID, + IssueFields: issueFields, + } + + if err := gqlClient.Mutate(ctx, &mutation, mutationInput, nil); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to set issue field values", err), nil, nil + } + + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%v", mutation.SetIssueFieldValue.Issue.ID), + URL: string(mutation.SetIssueFieldValue.Issue.URL), + }) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular + return st +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 02b86a9d9..05a46a211 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -308,6 +308,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { GranularAddSubIssue(t), GranularRemoveSubIssue(t), GranularReprioritizeSubIssue(t), + GranularSetIssueFields(t), // Granular pull request tools (feature-flagged, replace consolidated update_pull_request/pull_request_review_write) GranularUpdatePullRequestTitle(t), From 8cfd9ad951a0be933cf7fcafeab9eb25d3d094f3 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Apr 2026 14:01:02 +0100 Subject: [PATCH 2/2] Enforce exactly one value key per field in set_issue_fields (#2339) * Initial plan * Enforce exactly one value key per field in set_issue_fields and add tests Address review feedback: - Change validation to count value keys and reject when multiple are provided (e.g., text_value + number_value, or text_value + delete). - Add unit tests for multiple value keys and value + delete scenarios. - Run generate-docs (no doc changes needed; README was already current). Agent-Logs-Url: https://github.com/github/github-mcp-server/sessions/7e89edb3-5315-42dd-bfa1-6c962f1ba137 Co-authored-by: mattdholloway <918573+mattdholloway@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mattdholloway <918573+mattdholloway@users.noreply.github.com> --- pkg/github/granular_tools_test.go | 38 +++++++++++++++++++++++++++++++ pkg/github/issues_granular.go | 19 +++++++++------- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 4d7996e96..883158bb2 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -907,4 +907,42 @@ func TestGranularSetIssueFields(t *testing.T) { textContent := getTextResult(t, result) assert.Contains(t, textContent.Text, "each field must have a value") }) + + t.Run("multiple value keys returns error", func(t *testing.T) { + deps := BaseDeps{} + serverTool := GranularSetIssueFields(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(5), + "fields": []any{ + map[string]any{"field_id": "FIELD_1", "text_value": "hello", "number_value": float64(42)}, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "each field must have exactly one value") + }) + + t.Run("value key with delete returns error", func(t *testing.T) { + deps := BaseDeps{} + serverTool := GranularSetIssueFields(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(5), + "fields": []any{ + map[string]any{"field_id": "FIELD_1", "text_value": "hello", "delete": true}, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "each field must have exactly one value") + }) } diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 107f8a74f..5dbd7d8d1 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -731,41 +731,44 @@ func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.Serv FieldID: githubv4.ID(fieldID), } - // Check for exactly one value type (or delete) - hasValue := false + // Count how many value keys are present; exactly one is required. + valueCount := 0 if v, err := OptionalParam[string](fieldMap, "text_value"); err == nil && v != "" { input.TextValue = githubv4.NewString(githubv4.String(v)) - hasValue = true + valueCount++ } if v, err := OptionalParam[float64](fieldMap, "number_value"); err == nil { if _, exists := fieldMap["number_value"]; exists { gqlFloat := githubv4.Float(v) input.NumberValue = &gqlFloat - hasValue = true + valueCount++ } } if v, err := OptionalParam[string](fieldMap, "date_value"); err == nil && v != "" { input.DateValue = githubv4.NewString(githubv4.String(v)) - hasValue = true + valueCount++ } if v, err := OptionalParam[string](fieldMap, "single_select_option_id"); err == nil && v != "" { optionID := githubv4.ID(v) input.SingleSelectOptionID = &optionID - hasValue = true + valueCount++ } if _, exists := fieldMap["delete"]; exists { del, err := OptionalParam[bool](fieldMap, "delete") if err == nil && del { deleteVal := githubv4.Boolean(true) input.Delete = &deleteVal - hasValue = true + valueCount++ } } - if !hasValue { + if valueCount == 0 { return utils.NewToolResultError("each field must have a value (text_value, number_value, date_value, single_select_option_id) or delete: true"), nil, nil } + if valueCount > 1 { + return utils.NewToolResultError("each field must have exactly one value (text_value, number_value, date_value, single_select_option_id) or delete: true, but multiple were provided"), nil, nil + } issueFields = append(issueFields, input) }