From 46f9a99ec758bd55a5c3f77283e58cc34719ca4e Mon Sep 17 00:00:00 2001 From: Shurong Cao Date: Mon, 24 Aug 2026 19:12:29 +0800 Subject: [PATCH 1/5] feat(issues): add batch_update_issue_labels granular tool Add a feature-flagged batch labeling tool that applies add/remove label operations to multiple issues in a single MCP call, avoiding N sequential update_issue_labels round-trips for bulk triage workflows (e.g. 'label all issues from milestone X as priority/high'). - Each operation entry targets one issue with add and/or remove arrays; labels not named in the operation are left untouched. - All input is validated before any mutation: empty operations, duplicate issue numbers, missing add/remove, and empty label names fail the whole call up front. Per-issue API errors are reported individually in the JSON result with applied/error per issue. - Add phase uses POST /issues/{n}/labels (additive) and returns the full resulting label set; remove phase uses DELETE per label. Closes #2412 --- .../batch_update_issue_labels.snap | 64 ++++++ pkg/github/batch_labels_granular_test.go | 156 +++++++++++++ pkg/github/granular_tools_test.go | 2 + pkg/github/helper_test.go | 2 + pkg/github/issues_granular.go | 210 ++++++++++++++++++ pkg/github/tools.go | 1 + 6 files changed, 435 insertions(+) create mode 100644 pkg/github/__toolsnaps__/batch_update_issue_labels.snap create mode 100644 pkg/github/batch_labels_granular_test.go diff --git a/pkg/github/__toolsnaps__/batch_update_issue_labels.snap b/pkg/github/__toolsnaps__/batch_update_issue_labels.snap new file mode 100644 index 0000000000..1415656015 --- /dev/null +++ b/pkg/github/__toolsnaps__/batch_update_issue_labels.snap @@ -0,0 +1,64 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Batch Update Issue Labels" + }, + "description": "Apply label changes to multiple issues in one call. Each entry in operations targets one issue and can add labels, remove labels, or both. Labels not named in the operation are left untouched. Invalid input fails the whole call before any change is applied; per-issue API errors are reported individually.", + "inputSchema": { + "properties": { + "operations": { + "description": "One entry per issue, in any order. Each entry requires issue_number plus at least one non-empty of add or remove (arrays of label names). Duplicate issue numbers are rejected.", + "items": { + "additionalProperties": false, + "properties": { + "add": { + "description": "Label names to add to this issue (GitHub creates missing labels implicitly)", + "items": { + "minLength": 1, + "type": "string" + }, + "type": "array" + }, + "issue_number": { + "description": "The issue number to update", + "minimum": 1, + "type": "number" + }, + "remove": { + "description": "Label names to remove from this issue", + "items": { + "minLength": 1, + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "issue_number" + ], + "type": "object" + }, + "minItems": 1, + "type": "array" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "operations" + ], + "type": "object" + }, + "name": "batch_update_issue_labels" +} \ No newline at end of file diff --git a/pkg/github/batch_labels_granular_test.go b/pkg/github/batch_labels_granular_test.go new file mode 100644 index 0000000000..7b44870715 --- /dev/null +++ b/pkg/github/batch_labels_granular_test.go @@ -0,0 +1,156 @@ +package github + +import ( + "context" + "net/http" + "strings" + "testing" + + "github.com/github/github-mcp-server/pkg/translations" + gogithub "github.com/google/go-github/v89/github" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGranularBatchUpdateIssueLabels(t *testing.T) { + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]any + expectToolErr bool + expectedErrMsg string + }{ + { + name: "add and remove across multiple issues", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesByOwnerByRepoByIssueNumberLabels: expectRequestBody(t, []any{"bug", "priority/high"}). + andThen(mockResponse(t, http.StatusOK, []*gogithub.Label{ + {Name: "bug"}, + {Name: "priority/high"}, + })), + DeleteReposIssuesByOwnerByRepoByIssueNumberLabel: mockResponse(t, http.StatusNoContent, nil), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "operations": []any{ + map[string]any{ + "issue_number": float64(1), + "add": []any{"bug", "priority/high"}, + }, + map[string]any{ + "issue_number": float64(2), + "remove": []any{"wontfix"}, + }, + }, + }, + expectToolErr: false, + }, + { + name: "empty operations array rejected", + mockedClient: MockHTTPClientWithHandlers(nil), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "operations": []any{}, + }, + expectToolErr: true, + expectedErrMsg: "operations must contain at least one entry", + }, + { + name: "missing operations parameter rejected", + mockedClient: MockHTTPClientWithHandlers(nil), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + }, + expectToolErr: true, + expectedErrMsg: "missing required parameter: operations", + }, + { + name: "operation without add or remove rejected", + mockedClient: MockHTTPClientWithHandlers(nil), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "operations": []any{ + map[string]any{"issue_number": float64(1)}, + }, + }, + expectToolErr: true, + expectedErrMsg: "at least one non-empty of add or remove is required", + }, + { + name: "duplicate issue numbers rejected", + mockedClient: MockHTTPClientWithHandlers(nil), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "operations": []any{ + map[string]any{"issue_number": float64(1), "add": []any{"bug"}}, + map[string]any{"issue_number": float64(1), "remove": []any{"wontfix"}}, + }, + }, + expectToolErr: true, + expectedErrMsg: "duplicate issue_number 1", + }, + { + name: "empty label name in add rejected", + mockedClient: MockHTTPClientWithHandlers(nil), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "operations": []any{ + map[string]any{"issue_number": float64(1), "add": []any{""}}, + }, + }, + expectToolErr: true, + expectedErrMsg: "add contains an empty label name", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + deps := BaseDeps{Client: mustNewGHClient(t, tc.mockedClient)} + serverTool := GranularBatchUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + if tc.expectToolErr { + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + return + } + assert.False(t, result.IsError) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, `"issue_number":1`) + assert.Contains(t, textContent.Text, `"applied":true`) + }) + } +} + +func TestGranularBatchUpdateIssueLabelsPartialFailure(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesByOwnerByRepoByIssueNumberLabels: func(_ http.ResponseWriter, _ *http.Request) {}, + })) + deps := BaseDeps{Client: client} + serverTool := GranularBatchUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "operations": []any{ + map[string]any{"issue_number": float64(1), "add": []any{"bug"}}, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + // A per-issue API failure must surface as a tool error with a JSON body + // identifying the failing issue. + assert.True(t, result.IsError, "expected IsError on API failure") + textContent := getTextResult(t, result) + assert.True(t, strings.Contains(textContent.Text, "add failed")) +} diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 5ef0c0a662..008ebc7b13 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -38,6 +38,7 @@ func TestGranularToolSnaps(t *testing.T) { GranularUpdateIssueBody, GranularUpdateIssueAssignees, GranularUpdateIssueLabels, + GranularBatchUpdateIssueLabels, GranularUpdateIssueMilestone, GranularUpdateIssueType, GranularUpdateIssueState, @@ -84,6 +85,7 @@ func TestIssuesGranularToolset(t *testing.T) { "update_issue_body", "update_issue_assignees", "update_issue_labels", + "batch_update_issue_labels", "update_issue_milestone", "update_issue_type", "update_issue_state", diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index 5fc541d45d..3168138825 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -64,6 +64,8 @@ const ( GetReposIssuesCommentsByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/comments" PostReposIssuesByOwnerByRepo = "POST /repos/{owner}/{repo}/issues" PostReposIssuesCommentsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/comments" + PostReposIssuesByOwnerByRepoByIssueNumberLabels = "POST /repos/{owner}/{repo}/issues/{issue_number}/labels" + DeleteReposIssuesByOwnerByRepoByIssueNumberLabel = "DELETE /repos/{owner}/{repo}/issues/{issue_number}/labels/{name}" PostReposIssuesReactionsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/reactions" PatchReposIssuesByOwnerByRepoByIssueNumber = "PATCH /repos/{owner}/{repo}/issues/{issue_number}" GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/sub_issues" diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 0b1cdd0e7c..67d96fb336 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -706,6 +706,216 @@ func GranularUpdateIssueMilestone(t translations.TranslationHelperFunc) inventor ) } +// batchLabelOperation is one per-issue add/remove entry inside a +// batch_update_issue_labels call. +type batchLabelOperation struct { + IssueNumber int `json:"issue_number"` + Add []string `json:"add,omitempty"` + Remove []string `json:"remove,omitempty"` +} + +// GranularBatchUpdateIssueLabels creates a tool to apply label changes to +// several issues in a single call, avoiding N sequential update_issue_labels +// round-trips for bulk triage workflows. +func GranularBatchUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.ServerTool { + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "batch_update_issue_labels", + Description: t("TOOL_BATCH_UPDATE_ISSUE_LABELS_DESCRIPTION", "Apply label changes to multiple issues in one call. Each entry in operations targets one issue and can add labels, remove labels, or both. Labels not named in the operation are left untouched. Invalid input fails the whole call before any change is applied; per-issue API errors are reported individually."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_BATCH_UPDATE_ISSUE_LABELS_USER_TITLE", "Batch Update Issue Labels"), + 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", + }, + "operations": { + Type: "array", + Description: "One entry per issue, in any order. Each entry requires issue_number plus at least one non-empty of add or remove (arrays of label names). Duplicate issue numbers are rejected.", + MinItems: jsonschema.Ptr(1), + Items: &jsonschema.Schema{ + Type: "object", + AdditionalProperties: &jsonschema.Schema{Not: &jsonschema.Schema{}}, + Properties: map[string]*jsonschema.Schema{ + "issue_number": { + Type: "number", + Description: "The issue number to update", + Minimum: jsonschema.Ptr(1.0), + }, + "add": { + Type: "array", + Description: "Label names to add to this issue (GitHub creates missing labels implicitly)", + Items: &jsonschema.Schema{Type: "string", MinLength: jsonschema.Ptr(1)}, + }, + "remove": { + Type: "array", + Description: "Label names to remove from this issue", + Items: &jsonschema.Schema{Type: "string", MinLength: jsonschema.Ptr(1)}, + }, + }, + Required: []string{"issue_number"}, + }, + }, + }, + Required: []string{"owner", "repo", "operations"}, + }, + }, + []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 + } + + opsRaw, ok := args["operations"] + if !ok { + return utils.NewToolResultError("missing required parameter: operations"), nil, nil + } + opsSlice, ok := opsRaw.([]any) + if !ok { + return utils.NewToolResultError("parameter operations must be an array"), nil, nil + } + if len(opsSlice) == 0 { + return utils.NewToolResultError("operations must contain at least one entry"), nil, nil + } + + // Validate every operation up front so an invalid entry fails the + // whole call before any mutation is attempted. + planned := make([]batchLabelOperation, 0, len(opsSlice)) + seenIssues := make(map[int]bool, len(opsSlice)) + for i, item := range opsSlice { + opObj, ok := item.(map[string]any) + if !ok { + return utils.NewToolResultError(fmt.Sprintf("operations[%d]: each operation must be an object with issue_number and at least one of add or remove", i)), nil, nil + } + issueNumber, err := RequiredInt(opObj, "issue_number") + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("operations[%d]: %s", i, err.Error())), nil, nil + } + if seenIssues[issueNumber] { + return utils.NewToolResultError(fmt.Sprintf("operations[%d]: duplicate issue_number %d; merge the label changes into a single operation", i, issueNumber)), nil, nil + } + seenIssues[issueNumber] = true + + op := batchLabelOperation{IssueNumber: issueNumber} + if addRaw, hasAdd := opObj["add"]; hasAdd && addRaw != nil { + op.Add, err = OptionalStringArrayParam(opObj, "add") + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("operations[%d]: %s", i, err.Error())), nil, nil + } + } + if removeRaw, hasRemove := opObj["remove"]; hasRemove && removeRaw != nil { + op.Remove, err = OptionalStringArrayParam(opObj, "remove") + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("operations[%d]: %s", i, err.Error())), nil, nil + } + } + if len(op.Add) == 0 && len(op.Remove) == 0 { + return utils.NewToolResultError(fmt.Sprintf("operations[%d]: at least one non-empty of add or remove is required", i)), nil, nil + } + for _, l := range op.Add { + if strings.TrimSpace(l) == "" { + return utils.NewToolResultError(fmt.Sprintf("operations[%d]: add contains an empty label name", i)), nil, nil + } + } + for _, l := range op.Remove { + if strings.TrimSpace(l) == "" { + return utils.NewToolResultError(fmt.Sprintf("operations[%d]: remove contains an empty label name", i)), nil, nil + } + } + planned = append(planned, op) + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + type opOutcome struct { + IssueNumber int `json:"issue_number"` + Applied bool `json:"applied"` + Error string `json:"error,omitempty"` + Labels []string `json:"labels,omitempty"` + } + outcomes := make([]opOutcome, 0, len(planned)) + hadFailure := false + for _, op := range planned { + outcome := opOutcome{IssueNumber: op.IssueNumber, Applied: true} + + // Add phase: POST .../issues/{n}/labels adds without touching + // existing labels and returns the full resulting label set. + if len(op.Add) > 0 { + labels, resp, err := client.Issues.AddLabelsToIssue(ctx, owner, repo, op.IssueNumber, op.Add) + if resp != nil { + _ = resp.Body.Close() + } + if err != nil { + outcome.Applied = false + outcome.Error = fmt.Sprintf("add failed: %v", err) + hadFailure = true + outcomes = append(outcomes, outcome) + continue + } + outcome.Labels = make([]string, 0, len(labels)) + for _, l := range labels { + outcome.Labels = append(outcome.Labels, l.GetName()) + } + } + + // Remove phase. + failed := false + for _, label := range op.Remove { + resp, err := client.Issues.RemoveLabelForIssue(ctx, owner, repo, op.IssueNumber, label) + if resp != nil { + _ = resp.Body.Close() + } + if err != nil { + outcome.Error = fmt.Sprintf("remove %q failed: %v", label, err) + outcome.Applied = false + hadFailure = true + failed = true + break + } + } + if failed { + outcomes = append(outcomes, outcome) + continue + } + + outcomes = append(outcomes, outcome) + } + + response := struct { + Results []opOutcome `json:"results"` + }{Results: outcomes} + r, err := json.Marshal(response) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + result := utils.NewToolResultText(string(r)) + result.IsError = hadFailure + return result, nil, nil + }, + ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular + return st +} + // issueTypeWithIntent represents the object form of the issue type field, // allowing a rationale, confidence level, and/or suggest flag to be sent alongside the type name. type issueTypeWithIntent struct { diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 8d568878db..9bb2de8597 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -353,6 +353,7 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent GranularUpdateIssueBody(t), GranularUpdateIssueAssignees(t), GranularUpdateIssueLabels(t), + GranularBatchUpdateIssueLabels(t), GranularUpdateIssueMilestone(t), GranularUpdateIssueType(t), GranularUpdateIssueState(t), From d4711bfc93e808089d0bce76b6d71dab235bfcef Mon Sep 17 00:00:00 2001 From: Shurong Cao <170531907+CAOShurong@users.noreply.github.com> Date: Fri, 28 Aug 2026 05:04:44 +0800 Subject: [PATCH 2/5] fix(issues): adapt batch labels scope access --- pkg/github/issues_granular.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 67d96fb336..66cb0842d4 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -771,7 +771,7 @@ func GranularBatchUpdateIssueLabels(t translations.TranslationHelperFunc) invent Required: []string{"owner", "repo", "operations"}, }, }, - []scopes.Scope{scopes.Repo}, + scopes.RequireAll(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 { From 02e3b7107bcc5dc89d4d281e42e42827499821b9 Mon Sep 17 00:00:00 2001 From: CAOShurong <170531907+CAOShurong@users.noreply.github.com> Date: Tue, 13 Oct 2026 12:56:53 +0800 Subject: [PATCH 3/5] fix: keep batch label tool compatible with v2 module --- pkg/github/batch_labels_granular_test.go | 2 +- pkg/github/issues_granular.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/github/batch_labels_granular_test.go b/pkg/github/batch_labels_granular_test.go index e92cbe5a72..7d733d6917 100644 --- a/pkg/github/batch_labels_granular_test.go +++ b/pkg/github/batch_labels_granular_test.go @@ -7,7 +7,7 @@ import ( "testing" "time" - "github.com/github/github-mcp-server/pkg/translations" + "github.com/github/github-mcp-server/v2/pkg/translations" gogithub "github.com/google/go-github/v92/github" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index e68817008b..1229fbe08c 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -2,6 +2,7 @@ package github import ( "context" + "encoding/json" "fmt" "maps" "strings" From b36634cbc3c5e76b15f659d34dd105d8e1f73776 Mon Sep 17 00:00:00 2001 From: CAOShurong <170531907+CAOShurong@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:10:48 +0800 Subject: [PATCH 4/5] fix: register batch label tool with typed output --- pkg/github/granular_issue_types.go | 29 +++++ pkg/github/issues_granular.go | 105 +++++++----------- .../typed_granular_issue_outputs_test.go | 2 +- 3 files changed, 70 insertions(+), 66 deletions(-) diff --git a/pkg/github/granular_issue_types.go b/pkg/github/granular_issue_types.go index a1a8f47816..5723d2c61b 100644 --- a/pkg/github/granular_issue_types.go +++ b/pkg/github/granular_issue_types.go @@ -111,6 +111,35 @@ type GranularIssueLabelsInput struct { Labels []GranularIssueLabel `json:"labels"` } +// GranularBatchUpdateIssueLabelsInput contains one label mutation per issue. +// Add and Remove are both optional, but at least one must contain a label for +// every operation. +type GranularBatchUpdateIssueLabelsInput struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + Operations []GranularBatchUpdateIssueLabelsOperation `json:"operations"` +} + +type GranularBatchUpdateIssueLabelsOperation struct { + IssueNumber int `json:"issue_number"` + Add []string `json:"add,omitempty"` + Remove []string `json:"remove,omitempty"` +} + +// GranularBatchUpdateIssueLabelsResult reports the outcome for one issue. +type GranularBatchUpdateIssueLabelsResult struct { + IssueNumber int `json:"issue_number"` + Applied bool `json:"applied"` + Error string `json:"error,omitempty"` + Labels []string `json:"labels,omitempty"` +} + +// GranularBatchUpdateIssueLabelsOutput is the structured result of a batch +// label update. +type GranularBatchUpdateIssueLabelsOutput struct { + Results []GranularBatchUpdateIssueLabelsResult `json:"results"` +} + type GranularIssueTypeInput struct { GranularIssueCoordinate IssueType *string `json:"issue_type"` diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 1229fbe08c..c5ffba5582 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -485,19 +485,11 @@ func GranularUpdateIssueMilestone(t translations.TranslationHelperFunc) inventor ) } -// batchLabelOperation is one per-issue add/remove entry inside a -// batch_update_issue_labels call. -type batchLabelOperation struct { - IssueNumber int `json:"issue_number"` - Add []string `json:"add,omitempty"` - Remove []string `json:"remove,omitempty"` -} - // GranularBatchUpdateIssueLabels creates a tool to apply label changes to // several issues in a single call, avoiding N sequential update_issue_labels // round-trips for bulk triage workflows. func GranularBatchUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.ServerTool { - st := NewTool( + st := NewTool[GranularBatchUpdateIssueLabelsInput, *GranularBatchUpdateIssueLabelsOutput]( ToolsetMetadataIssues, mcp.Tool{ Name: "batch_update_issue_labels", @@ -551,59 +543,33 @@ func GranularBatchUpdateIssueLabels(t translations.TranslationHelperFunc) invent }, }, scopes.RequireAll(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 + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args GranularBatchUpdateIssueLabelsInput) (*mcp.CallToolResult, *GranularBatchUpdateIssueLabelsOutput, error) { + owner, repo := args.Owner, args.Repo + if owner == "" { + return utils.NewToolResultError("missing required parameter: owner"), nil, nil } - repo, err := RequiredParam[string](args, "repo") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - - opsRaw, ok := args["operations"] - if !ok { - return utils.NewToolResultError("missing required parameter: operations"), nil, nil + if repo == "" { + return utils.NewToolResultError("missing required parameter: repo"), nil, nil } - opsSlice, ok := opsRaw.([]any) - if !ok { - return utils.NewToolResultError("parameter operations must be an array"), nil, nil - } - if len(opsSlice) == 0 { + if len(args.Operations) == 0 { return utils.NewToolResultError("operations must contain at least one entry"), nil, nil } // Validate every operation up front so an invalid entry fails the // whole call before any mutation is attempted. - planned := make([]batchLabelOperation, 0, len(opsSlice)) - seenIssues := make(map[int]bool, len(opsSlice)) - for i, item := range opsSlice { - opObj, ok := item.(map[string]any) - if !ok { - return utils.NewToolResultError(fmt.Sprintf("operations[%d]: each operation must be an object with issue_number and at least one of add or remove", i)), nil, nil - } - issueNumber, err := RequiredInt(opObj, "issue_number") - if err != nil { - return utils.NewToolResultError(fmt.Sprintf("operations[%d]: %s", i, err.Error())), nil, nil + planned := make([]GranularBatchUpdateIssueLabelsOperation, 0, len(args.Operations)) + seenIssues := make(map[int]bool, len(args.Operations)) + for i, op := range args.Operations { + if op.IssueNumber == 0 { + return utils.NewToolResultError(fmt.Sprintf("operations[%d]: missing required parameter: issue_number", i)), nil, nil } - if seenIssues[issueNumber] { - return utils.NewToolResultError(fmt.Sprintf("operations[%d]: duplicate issue_number %d; merge the label changes into a single operation", i, issueNumber)), nil, nil - } - seenIssues[issueNumber] = true - - op := batchLabelOperation{IssueNumber: issueNumber} - if addRaw, hasAdd := opObj["add"]; hasAdd && addRaw != nil { - op.Add, err = OptionalStringArrayParam(opObj, "add") - if err != nil { - return utils.NewToolResultError(fmt.Sprintf("operations[%d]: %s", i, err.Error())), nil, nil - } + if op.IssueNumber < 0 { + return utils.NewToolResultError(fmt.Sprintf("operations[%d]: issue_number must be greater than 0", i)), nil, nil } - if removeRaw, hasRemove := opObj["remove"]; hasRemove && removeRaw != nil { - op.Remove, err = OptionalStringArrayParam(opObj, "remove") - if err != nil { - return utils.NewToolResultError(fmt.Sprintf("operations[%d]: %s", i, err.Error())), nil, nil - } + if seenIssues[op.IssueNumber] { + return utils.NewToolResultError(fmt.Sprintf("operations[%d]: duplicate issue_number %d; merge the label changes into a single operation", i, op.IssueNumber)), nil, nil } + seenIssues[op.IssueNumber] = true if len(op.Add) == 0 && len(op.Remove) == 0 { return utils.NewToolResultError(fmt.Sprintf("operations[%d]: at least one non-empty of add or remove is required", i)), nil, nil } @@ -625,16 +591,10 @@ func GranularBatchUpdateIssueLabels(t translations.TranslationHelperFunc) invent return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - type opOutcome struct { - IssueNumber int `json:"issue_number"` - Applied bool `json:"applied"` - Error string `json:"error,omitempty"` - Labels []string `json:"labels,omitempty"` - } - outcomes := make([]opOutcome, 0, len(planned)) + outcomes := make([]GranularBatchUpdateIssueLabelsResult, 0, len(planned)) hadFailure := false for _, op := range planned { - outcome := opOutcome{IssueNumber: op.IssueNumber, Applied: true} + outcome := GranularBatchUpdateIssueLabelsResult{IssueNumber: op.IssueNumber, Applied: true} // Add phase: POST .../issues/{n}/labels adds without touching // existing labels and returns the full resulting label set. @@ -679,22 +639,37 @@ func GranularBatchUpdateIssueLabels(t translations.TranslationHelperFunc) invent outcomes = append(outcomes, outcome) } - response := struct { - Results []opOutcome `json:"results"` - }{Results: outcomes} - r, err := json.Marshal(response) + output := &GranularBatchUpdateIssueLabelsOutput{Results: outcomes} + r, err := json.Marshal(output) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil } result := utils.NewToolResultText(string(r)) result.IsError = hadFailure - return result, nil, nil + return result, output, nil }, + granularIssueInputNormalizer(normalizeGranularBatchUpdateIssueLabelsArguments), ) st.FeatureRule = issuesGranularFeatureRule return st } +func normalizeGranularBatchUpdateIssueLabelsArguments(raw json.RawMessage) (json.RawMessage, error) { + var args map[string]any + if err := json.Unmarshal(raw, &args); err != nil { + return nil, err + } + for _, name := range []string{"owner", "repo"} { + if _, err := RequiredParam[string](args, name); err != nil { + return nil, err + } + } + if _, ok := args["operations"]; !ok { + return nil, fmt.Errorf("missing required parameter: operations") + } + return json.Marshal(args) +} + // issueTypeWithIntent represents the object form of the issue type field, // allowing a rationale, confidence level, and/or suggest flag to be sent alongside the type name. type issueTypeWithIntent struct { diff --git a/pkg/github/typed_granular_issue_outputs_test.go b/pkg/github/typed_granular_issue_outputs_test.go index d2b833d650..4915820d9f 100644 --- a/pkg/github/typed_granular_issue_outputs_test.go +++ b/pkg/github/typed_granular_issue_outputs_test.go @@ -23,7 +23,7 @@ import ( func granularIssueSession(t *testing.T, deps BaseDeps, protocol string) (*mcp.ClientSession, map[string]*jsonschema.Resolved) { t.Helper() tools := granularToolsForToolset(ToolsetMetadataIssues.ID, FeatureFlagIssuesGranular) - require.Len(t, tools, 18) + require.Len(t, tools, 19) inv, err := inventory.NewBuilder().SetTools(tools).WithToolsets([]string{"all"}). WithFeatureChecker(featureCheckerFor(inventory.FeatureFlag(FeatureFlagIssuesGranular))).Build() require.NoError(t, err) From b55303dc52a92eaca6eb142eb16d7eab019f2a43 Mon Sep 17 00:00:00 2001 From: CAOShurong <170531907+CAOShurong@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:22:27 +0800 Subject: [PATCH 5/5] fix: preserve typed batch label compatibility --- .../batch_update_issue_labels.snap | 66 +++++++++++++++- pkg/github/batch_labels_granular_test.go | 75 +++++++++++++++++++ pkg/github/issues_granular.go | 22 +++++- 3 files changed, 159 insertions(+), 4 deletions(-) diff --git a/pkg/github/__toolsnaps__/batch_update_issue_labels.snap b/pkg/github/__toolsnaps__/batch_update_issue_labels.snap index 1415656015..6d1fede0f8 100644 --- a/pkg/github/__toolsnaps__/batch_update_issue_labels.snap +++ b/pkg/github/__toolsnaps__/batch_update_issue_labels.snap @@ -7,6 +7,18 @@ "title": "Batch Update Issue Labels" }, "description": "Apply label changes to multiple issues in one call. Each entry in operations targets one issue and can add labels, remove labels, or both. Labels not named in the operation are left untouched. Invalid input fails the whole call before any change is applied; per-issue API errors are reported individually.", + "icons": [ + { + "mimeType": "image/png", + "src": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAAABmJLR0QA/wD/AP+gvaeTAAAC2UlEQVRIicWVMUyTaRjHf/+vVUpOJOdilOYoUvVreq2CDmLOwdlAS5xMbrrhBifjYG7QjYuJw108nZx1huLgYlw0QshxChUK2koxHHcuCmgsFfieG0or3kGPSoz/8X3f5/97nvfN8z7wmaVqm9FodFfR/EnMugy5giCAwYwgI9Ff9Hl9L9Lp1zUBgsGO+rqGwnlhF4CdwBSyAUwvS1G220zHBSFgHtmVxYWvfp2ZGSj8LyAcjgfZphTQZtAr7OdsZvSP9RIJR+LtQhcNusF+d1aUfPp05M8NAavmg8AOk3cmN56+s1Hpa7U/cviUYbcw3jgex9ZCKoBgsKM+0PDuAdBq2He5zOiTzZiXtc+NxRw598GeLRcWTuTz+UUAp3ygrqFwHmgzeWdqNQd4PpFOy/gedNQfaDz3UQXRaHRX0fNPGdzNZUZO12q+VuHIoV7g5Hu/1/IinX7tABTNnwR2SvRsxRzAzOsBGuuWfAkoX5FZFzCVHR95tFVAbiI9DEwj6wLwl5YVMTRYLTDsxjuRrgGYYz/kxkbvbXRWMOBB24cKYI9ks1X8hXQDaAaa5XG9WjImZgV71wI+m8qAv8y0t1pSmP0ITANTmHO2qqvRZDALlTdgHKyjWkx2YvQ2cHtTacMxYBhWK5DoF4TCkXj7Jg02VKsbOwI0Y+qvALZrOQXMC13cKkDSJWBuJUCqAhgbG3uF7IpBcn/k8KlPNQ+78U5QQuhy/vHjuQoAYPndwi9gw4bd2ufGYrWatxz8No50ExhaKsz9Vl6vAPL5/KKzoiTGG0fOg1oqCbvxTp/juw/M+zynu/yTwjoD58CBQ02ez/pAR4E+M69ntf3/o1Y3dqR050oAQz7P6Z6cfPRRw647MkOhUMAfaDyH+AloBKYNPRT292rQHoMOSp09J3TZlt5ezWazxX97VR3638RiX9ct+RImSyBcrDT0ETMyMp4ptRIgVX7QL6J/ALSUEwJ5rdg2AAAAAElFTkSuQmCC\r", + "theme": "light" + }, + { + "mimeType": "image/png", + "src": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAAABmJLR0QA/wD/AP+gvaeTAAABxElEQVRIibWVvW4TURCFv0vlNEDcIHAkKAERJOIKSjoUArwBPwUFFaKIIngAJARCPIgdh4cgRBYt6ZIAEYEqdhpEw0fhCbkKa68dxyNtsfNzzpnZu3NhwpYGBdUqcA+4A1wEZiK0DawD74FWSml3JAJ1CngGLAIngU1gFfgZKWeAG8AFoAu8At6mlH6VtqTOqJ/UP2pDnRuQO6c27VlbrQ0Dvq121Fulag7q5qPmW18SdSqUd9Qrw4Jn9bNR21YrRQkvYixDKy/AuB3jWjocqKpdtXFU8AxrOTqZzp2PgvnaMRDUA+tB7mypG+OCZ3hbahPgRPguAR9LihaicEu9WcKxClzOi/fU1wPAk7rjgX0uEfNG3cs7mJjtE+wA5/olpZQEHgNf6K2NJyW4NeD7v7c4WptjSc0svlMjdzyM2fbdOyOA7x/T+7mzGj9H8xgIWuquevpw4HmsivkxwBdC/WJRsBKLqqPOHgH8aqybtcJlF0m1WLndUToJ5V31q9r3NOYk7Wh1Wa0PyK3HzA3l/4H3uzIrwFNgCThF7/x/AH5EylngOnAe6AAvgXcppd9DEWRE08DdeIou/RVgJaXUGYQzUfsL+zmwV7BtIq0AAAAASUVORK5CYII=\r", + "theme": "dark" + } + ], "inputSchema": { "properties": { "operations": { @@ -46,11 +58,13 @@ }, "owner": { "description": "Repository owner (username or organization)", - "type": "string" + "type": "string", + "x-mcp-header": "owner" }, "repo": { "description": "Repository name", - "type": "string" + "type": "string", + "x-mcp-header": "repo" } }, "required": [ @@ -60,5 +74,51 @@ ], "type": "object" }, - "name": "batch_update_issue_labels" + "name": "batch_update_issue_labels", + "outputSchema": { + "additionalProperties": false, + "properties": { + "results": { + "items": { + "additionalProperties": false, + "properties": { + "applied": { + "type": "boolean" + }, + "error": { + "type": "string" + }, + "issue_number": { + "type": "integer" + }, + "labels": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + } + }, + "required": [ + "issue_number", + "applied" + ], + "type": "object" + }, + "type": [ + "null", + "array" + ] + } + }, + "required": [ + "results" + ], + "type": [ + "null", + "object" + ] + } } \ No newline at end of file diff --git a/pkg/github/batch_labels_granular_test.go b/pkg/github/batch_labels_granular_test.go index 7d733d6917..ae669b6d2b 100644 --- a/pkg/github/batch_labels_granular_test.go +++ b/pkg/github/batch_labels_granular_test.go @@ -2,13 +2,17 @@ package github import ( "context" + "encoding/json" "net/http" "strings" "testing" "time" + "github.com/github/github-mcp-server/v2/internal/toolsnaps" + "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/translations" gogithub "github.com/google/go-github/v92/github" + "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -207,3 +211,74 @@ func TestGranularBatchUpdateIssueLabelsTransport(t *testing.T) { }) } } + +func TestGranularBatchUpdateIssueLabelsModernTypedSession(t *testing.T) { + calls := 0 + httpClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesByOwnerByRepoByIssueNumberLabels: func(w http.ResponseWriter, r *http.Request) { + calls++ + expectPath(t, "/repos/owner/repo/issues/1/labels"). + andThen(expectRequestBody(t, []any{"bug"}). + andThen(mockResponse(t, http.StatusOK, []*gogithub.Label{{Name: "bug"}})))(w, r) + }, + }) + deps := BaseDeps{Client: mustNewGHClient(t, httpClient)} + tool := GranularBatchUpdateIssueLabels(translations.NullTranslationHelper) + inv, err := inventory.NewBuilder().SetTools([]inventory.ServerTool{tool}). + WithToolsets([]string{"all"}). + WithFeatureChecker(featureCheckerFor(inventory.FeatureFlag(FeatureFlagIssuesGranular))).Build() + require.NoError(t, err) + server := mcp.NewServer(&mcp.Implementation{Name: "batch-label-typed-test", Version: "v1"}, nil) + server.AddReceivingMiddleware(InjectDepsMiddleware(deps)) + inv.RegisterTools(context.Background(), server, deps) + session := connectCommentVisibilityClient(t, server, inventory.ProtocolVersionMultiRoundTrip) + + list, err := session.ListTools(context.Background(), nil) + require.NoError(t, err) + require.Len(t, list.Tools, 1) + require.Equal(t, tool.Tool.Name, list.Tools[0].Name) + require.NotNil(t, list.Tools[0].OutputSchema) + var schema jsonschema.Schema + require.NoError(t, json.Unmarshal([]byte(mustMarshalJSON(t, list.Tools[0].OutputSchema)), &schema)) + resolved, err := schema.Resolve(nil) + require.NoError(t, err) + require.NoError(t, toolsnaps.Test(tool.Tool.Name, *list.Tools[0])) + + result, err := session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: tool.Tool.Name, + Arguments: map[string]any{ + "owner": "owner", + "repo": "repo", + "operations": []any{map[string]any{ + // RequiredInt accepted this numeric string before typed decoding. + "issue_number": "1.0", + "add": []any{"bug"}, + }}, + }, + }) + require.NoError(t, err) + require.False(t, result.IsError, mustMarshalJSON(t, result)) + expected := `{"results":[{"issue_number":1,"applied":true,"labels":["bug"]}]}` + assert.JSONEq(t, expected, getTextResult(t, result).Text) + require.NotNil(t, result.StructuredContent) + assert.JSONEq(t, expected, mustMarshalJSON(t, result.StructuredContent)) + require.NoError(t, resolved.Validate(result.StructuredContent)) + assert.Equal(t, 1, calls) + + invalid, err := session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: tool.Tool.Name, + Arguments: map[string]any{ + "owner": "owner", + "repo": "repo", + "operations": []any{ + map[string]any{"issue_number": "1.0", "add": []any{"bug"}}, + map[string]any{"issue_number": "2", "add": []any{" "}}, + }, + }, + }) + require.NoError(t, err) + require.True(t, invalid.IsError) + assert.Nil(t, invalid.StructuredContent) + assert.Contains(t, getErrorResult(t, invalid).Text, "operations[1]") + assert.Equal(t, 1, calls, "validation must precede every mutation") +} diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index c5ffba5582..413bf3e953 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -664,9 +664,29 @@ func normalizeGranularBatchUpdateIssueLabelsArguments(raw json.RawMessage) (json return nil, err } } - if _, ok := args["operations"]; !ok { + operationsValue, ok := args["operations"] + if !ok { return nil, fmt.Errorf("missing required parameter: operations") } + operations, ok := operationsValue.([]any) + if !ok { + return nil, fmt.Errorf("parameter operations is not of type array, is %T", operationsValue) + } + for i, operationValue := range operations { + operation, ok := operationValue.(map[string]any) + if !ok { + return nil, fmt.Errorf("operations[%d] is not an object, is %T", i, operationValue) + } + issueNumber, ok := operation["issue_number"] + if !ok { + continue + } + normalizedIssueNumber, err := toInt(issueNumber) + if err != nil { + return nil, fmt.Errorf("operations[%d].issue_number: %w", i, err) + } + operation["issue_number"] = normalizedIssueNumber + } return json.Marshal(args) }