diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 6ebc2dec9..231893ed4 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -153,7 +153,8 @@ runtime behavior (such as output formatting) won't appear here. - **update_issue_labels** - Update Issue Labels - **Required OAuth Scopes**: `repo` - `issue_number`: The issue number to update (number, required) - - `labels`: Labels to apply to this issue. ([], required) + - `labels`: Labels to apply to this issue. For 'remove', only each label's name is used. The 'rationale', 'confidence', and 'is_suggestion' fields are only honored when method is 'replace'. ([], required) + - `method`: How to apply the labels: 'replace' (default) sets the issue's labels to exactly the provided list, 'add' adds them while keeping the existing labels, and 'remove' removes the named labels without affecting the others. (string, optional) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) diff --git a/pkg/github/__toolsnaps__/update_issue_labels.snap b/pkg/github/__toolsnaps__/update_issue_labels.snap index 2b31d756b..22431d938 100644 --- a/pkg/github/__toolsnaps__/update_issue_labels.snap +++ b/pkg/github/__toolsnaps__/update_issue_labels.snap @@ -4,7 +4,7 @@ "openWorldHint": true, "title": "Update Issue Labels" }, - "description": "Update the labels of an existing issue. This replaces the current labels with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", + "description": "Manage the labels on an existing issue. Use method 'replace' (default) to set the issue's labels to exactly the provided list, 'add' to add labels without removing existing ones, or 'remove' to remove specific labels without touching the rest. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", "inputSchema": { "properties": { "issue_number": { @@ -13,7 +13,7 @@ "type": "number" }, "labels": { - "description": "Labels to apply to this issue.", + "description": "Labels to apply to this issue. For 'remove', only each label's name is used. The 'rationale', 'confidence', and 'is_suggestion' fields are only honored when method is 'replace'.", "items": { "oneOf": [ { @@ -54,6 +54,16 @@ }, "type": "array" }, + "method": { + "default": "replace", + "description": "How to apply the labels: 'replace' (default) sets the issue's labels to exactly the provided list, 'add' adds them while keeping the existing labels, and 'remove' removes the named labels without affecting the others.", + "enum": [ + "replace", + "add", + "remove" + ], + "type": "string" + }, "owner": { "description": "Repository owner (username or organization)", "type": "string" diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 4a274ac31..29109eaa2 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -317,6 +317,21 @@ func TestGranularUpdateIssueLabels(t *testing.T) { }, }, }, + { + name: "label names are trimmed for the replace operation", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": []any{ + " bug ", + map[string]any{"name": " enhancement "}, + }, + }, + expectedReq: map[string]any{ + "labels": []any{"bug", "enhancement"}, + }, + }, } for _, tc := range tests { @@ -445,6 +460,28 @@ func TestGranularUpdateIssueLabelsInvalidRationale(t *testing.T) { }, expectedErrText: "each label object must have a 'name' string", }, + { + name: "whitespace-only string label rejected", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": []any{" "}, + }, + expectedErrText: "label name cannot be empty", + }, + { + name: "whitespace-only object name rejected", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": []any{ + map[string]any{"name": " "}, + }, + }, + expectedErrText: "label name cannot be empty", + }, } for _, tc := range tests { @@ -564,6 +601,366 @@ func TestGranularUpdateIssueLabelsConfidence(t *testing.T) { } } +func TestGranularUpdateIssueLabelsAddOperation(t *testing.T) { + mockLabels := []*gogithub.Label{ + {Name: gogithub.Ptr("bug")}, + {Name: gogithub.Ptr("enhancement")}, + } + + t.Run("adds labels without replacing existing ones", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesLabelsByOwnerByRepoByIssueNumber: expectRequestBody(t, []any{"enhancement"}). + andThen(mockResponse(t, http.StatusOK, mockLabels)), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "add", + "labels": []any{"enhancement"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "bug") + assert.Contains(t, textContent.Text, "enhancement") + }) + + t.Run("accepts label objects and uses their names", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesLabelsByOwnerByRepoByIssueNumber: expectRequestBody(t, []any{"enhancement"}). + andThen(mockResponse(t, http.StatusOK, mockLabels)), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "add", + "labels": []any{map[string]any{"name": "enhancement", "rationale": "ignored for add"}}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + }) + + t.Run("de-duplicates labels before sending the request", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesLabelsByOwnerByRepoByIssueNumber: expectRequestBody(t, []any{"enhancement"}). + andThen(mockResponse(t, http.StatusOK, mockLabels)), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "add", + "labels": []any{"enhancement", "enhancement", " enhancement "}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + }) + + t.Run("requires at least one label", func(t *testing.T) { + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "add", + "labels": []any{}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, "at least one label is required") + }) +} + +func TestGranularUpdateIssueLabelsRemoveOperation(t *testing.T) { + t.Run("removes a single label and returns remaining labels", func(t *testing.T) { + remaining := []*gogithub.Label{{Name: gogithub.Ptr("enhancement")}} + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + DeleteReposIssuesLabelsByOwnerByRepoByIssueNumberByName: mockResponse(t, http.StatusOK, remaining), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "remove", + "labels": []any{"bug"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "enhancement") + assert.NotContains(t, textContent.Text, "bug") + }) + + t.Run("requires at least one label", func(t *testing.T) { + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "remove", + "labels": []any{}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, "at least one label is required") + }) + + t.Run("surfaces API error when label is not present", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + DeleteReposIssuesLabelsByOwnerByRepoByIssueNumberByName: mockResponse(t, http.StatusNotFound, map[string]any{ + "message": "Label does not exist", + }), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "remove", + "labels": []any{"nonexistent"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, "failed to remove label from issue") + }) + + t.Run("rejects whitespace-only label names", func(t *testing.T) { + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "remove", + "labels": []any{" "}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, "label name cannot be empty") + }) + + t.Run("de-duplicates labels before issuing deletes", func(t *testing.T) { + var deleteCount int + remaining := []*gogithub.Label{{Name: gogithub.Ptr("enhancement")}} + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + DeleteReposIssuesLabelsByOwnerByRepoByIssueNumberByName: func(w http.ResponseWriter, r *http.Request) { + deleteCount++ + mockResponse(t, http.StatusOK, remaining)(w, r) + }, + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "remove", + "labels": []any{"bug", "bug", " bug "}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + assert.Equal(t, 1, deleteCount, "duplicate labels should collapse to a single DELETE") + }) + + t.Run("escapes multi-word label names in the request path", func(t *testing.T) { + var capturedPath string + remaining := []*gogithub.Label{{Name: gogithub.Ptr("enhancement")}} + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + DeleteReposIssuesLabelsByOwnerByRepoByIssueNumberByName: func(w http.ResponseWriter, r *http.Request) { + capturedPath = r.URL.EscapedPath() + mockResponse(t, http.StatusOK, remaining)(w, r) + }, + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "remove", + "labels": []any{"good first issue"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + assert.Contains(t, capturedPath, "labels/good%20first%20issue") + }) + + t.Run("escapes slashes in label names in the request path", func(t *testing.T) { + // A label like "status/blocked" must stay a single, escaped path + // segment so the slash is not interpreted as a path separator. A + // catch-all handler is used here (rather than the keyed router used + // above) because the mock routes on the decoded path, where %2F would + // look like an extra segment. + var capturedPath string + remaining := []*gogithub.Label{{Name: gogithub.Ptr("enhancement")}} + client := mustNewGHClient(t, MockHTTPClientWithHandler(func(w http.ResponseWriter, r *http.Request) { + capturedPath = r.URL.EscapedPath() + mockResponse(t, http.StatusOK, remaining)(w, r) + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "remove", + "labels": []any{"status/blocked"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + assert.Contains(t, capturedPath, "labels/status%2Fblocked") + }) +} + +func TestGranularUpdateIssueLabelsInvalidOperation(t *testing.T) { + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "toggle", + "labels": []any{"bug"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, "method must be one of: replace, add, remove") +} + +func TestGranularUpdateIssueLabelsReplaceReturnsLabels(t *testing.T) { + issue := &gogithub.Issue{ + Number: gogithub.Ptr(1), + Labels: []*gogithub.Label{ + {Name: gogithub.Ptr("bug")}, + {Name: gogithub.Ptr("enhancement")}, + }, + } + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, issue), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": "replace", + "labels": []any{"bug", "enhancement"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + // All methods return the resulting label set for a consistent response shape. + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "bug") + assert.Contains(t, textContent.Text, "enhancement") +} + +func TestGranularUpdateIssueLabelsOperationNormalization(t *testing.T) { + t.Run("operation is trimmed and case-folded to add", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesLabelsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, []*gogithub.Label{ + {Name: gogithub.Ptr("bug")}, + }), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "method": " ADD ", + "labels": []any{"bug"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + }) + + t.Run("omitted operation defaults to replace", func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)}), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": []any{"bug"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + }) +} + func TestGranularUpdateIssueMilestone(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index 2ad173679..bf2880bf6 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -62,6 +62,8 @@ const ( PostReposIssuesByOwnerByRepo = "POST /repos/{owner}/{repo}/issues" PostReposIssuesCommentsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/comments" PatchReposIssuesByOwnerByRepoByIssueNumber = "PATCH /repos/{owner}/{repo}/issues/{issue_number}" + PostReposIssuesLabelsByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/labels" + DeleteReposIssuesLabelsByOwnerByRepoByIssueNumberByName = "DELETE /repos/{owner}/{repo}/issues/{issue_number}/labels/{name}" GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber = "GET /repos/{owner}/{repo}/issues/{issue_number}/sub_issues" PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber = "POST /repos/{owner}/{repo}/issues/{issue_number}/sub_issues" DeleteReposIssuesSubIssueByOwnerByRepoByIssueNumber = "DELETE /repos/{owner}/{repo}/issues/{issue_number}/sub_issue" diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 157d5595f..ad6b46a30 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -5,10 +5,12 @@ import ( "encoding/json" "fmt" "maps" + "net/url" "strings" ghcontext "github.com/github/github-mcp-server/pkg/context" ghErrors "github.com/github/github-mcp-server/pkg/errors" + "github.com/github/github-mcp-server/pkg/ifc" "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/translations" @@ -285,7 +287,7 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S ToolsetMetadataIssues, mcp.Tool{ Name: "update_issue_labels", - Description: t("TOOL_UPDATE_ISSUE_LABELS_DESCRIPTION", "Update the labels of an existing issue. This replaces the current labels with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), + Description: t("TOOL_UPDATE_ISSUE_LABELS_DESCRIPTION", "Manage the labels on an existing issue. Use method 'replace' (default) to set the issue's labels to exactly the provided list, 'add' to add labels without removing existing ones, or 'remove' to remove specific labels without touching the rest. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_UPDATE_ISSUE_LABELS_USER_TITLE", "Update Issue Labels"), ReadOnlyHint: false, @@ -308,9 +310,15 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S Description: "The issue number to update", Minimum: jsonschema.Ptr(1.0), }, + "method": { + Type: "string", + Description: "How to apply the labels: 'replace' (default) sets the issue's labels to exactly the provided list, 'add' adds them while keeping the existing labels, and 'remove' removes the named labels without affecting the others.", + Enum: []any{"replace", "add", "remove"}, + Default: json.RawMessage(`"replace"`), + }, "labels": { Type: "array", - Description: "Labels to apply to this issue.", + Description: "Labels to apply to this issue. For 'remove', only each label's name is used. The 'rationale', 'confidence', and 'is_suggestion' fields are only honored when method is 'replace'.", Items: &jsonschema.Schema{ OneOf: []*jsonschema.Schema{ {Type: "string", Description: "Label name"}, @@ -362,6 +370,18 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S return utils.NewToolResultError(err.Error()), nil, nil } + method, err := OptionalParam[string](args, "method") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + method = strings.ToLower(strings.TrimSpace(method)) + if method == "" { + method = "replace" + } + if method != "replace" && method != "add" && method != "remove" { + return utils.NewToolResultError("method must be one of: replace, add, remove"), nil, nil + } + labelsRaw, ok := args["labels"] if !ok { return utils.NewToolResultError("missing required parameter: labels"), nil, nil @@ -379,90 +399,224 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S } } - useObjectForm := false - payload := make([]any, 0, len(labelsSlice)) - for _, item := range labelsSlice { - switch v := item.(type) { - case string: - payload = append(payload, v) - case map[string]any: - name, err := RequiredParam[string](v, "name") - if err != nil { - return utils.NewToolResultError("each label object must have a 'name' string"), nil, nil - } - rationale, err := OptionalParam[string](v, "rationale") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - rationale = strings.TrimSpace(rationale) - if len([]rune(rationale)) > 280 { - return utils.NewToolResultError("label rationale must be 280 characters or less"), nil, nil - } - confidence, err := OptionalParam[string](v, "confidence") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - confidence = normalizeConfidence(confidence) - if confidence != "" && confidence != "LOW" && confidence != "MEDIUM" && confidence != "HIGH" { - return utils.NewToolResultError("confidence must be one of: LOW, MEDIUM, HIGH"), nil, nil - } - isSuggestion, err := OptionalParam[bool](v, "is_suggestion") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - if rationale == "" && !isSuggestion && confidence == "" { - payload = append(payload, name) - } else { - useObjectForm = true - payload = append(payload, labelWithIntent{Name: name, Rationale: rationale, Confidence: confidence, Suggest: isSuggestion}) - } - default: - return utils.NewToolResultError("each label must be a string or an object with 'name' and optional 'rationale', 'confidence', and/or 'is_suggestion'"), nil, nil - } - } - client, err := deps.GetClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - var body any - if useObjectForm { - body = &labelsUpdateRequest{Labels: payload} - } else { - // Preserve the standard wire format when no rationale or suggest is supplied. - names := make([]string, len(payload)) - for i, p := range payload { - names[i] = p.(string) - } - body = &github.IssueRequest{Labels: &names} + switch method { + case "add": + return addIssueLabels(ctx, deps, client, owner, repo, issueNumber, labelsSlice) + case "remove": + return removeIssueLabels(ctx, deps, client, owner, repo, issueNumber, labelsSlice) + default: + return replaceIssueLabels(ctx, deps, client, owner, repo, issueNumber, labelsSlice) } + }, + ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular + return st +} - apiURL := fmt.Sprintf("repos/%s/%s/issues/%d", owner, repo, issueNumber) - req, err := client.NewRequest(ctx, "PATCH", apiURL, body) +// issueLabelNames extracts plain label names from the labels argument, accepting +// either bare strings or objects with a "name" field. It is used by the 'add' +// and 'remove' methods, which only need the label name. Names are trimmed, +// rejected if empty, and de-duplicated (preserving first-seen order) so callers +// don't issue redundant API requests for the same label. +func issueLabelNames(labelsSlice []any) ([]string, error) { + seen := make(map[string]struct{}, len(labelsSlice)) + names := make([]string, 0, len(labelsSlice)) + for _, item := range labelsSlice { + var name string + switch v := item.(type) { + case string: + name = v + case map[string]any: + n, err := RequiredParam[string](v, "name") if err != nil { - return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil - } + return nil, fmt.Errorf("each label object must have a 'name' string") + } + name = n + default: + return nil, fmt.Errorf("each label must be a string or an object with a 'name'") + } + name = strings.TrimSpace(name) + if name == "" { + return nil, fmt.Errorf("label name cannot be empty") + } + if _, ok := seen[name]; ok { + continue + } + seen[name] = struct{}{} + names = append(names, name) + } + return names, nil +} - issue := &github.Issue{} - resp, err := client.Do(req, issue) +// replaceIssueLabels sets the issue's labels to exactly the provided list, +// optionally carrying per-label rationale/confidence/suggestion intent. It +// returns the issue's resulting labels, matching the 'add' and 'remove' +// methods so the tool's response shape is consistent across methods. +func replaceIssueLabels(ctx context.Context, deps ToolDependencies, client *github.Client, owner, repo string, issueNumber int, labelsSlice []any) (*mcp.CallToolResult, any, error) { + useObjectForm := false + payload := make([]any, 0, len(labelsSlice)) + for _, item := range labelsSlice { + switch v := item.(type) { + case string: + name := strings.TrimSpace(v) + if name == "" { + return utils.NewToolResultError("label name cannot be empty"), nil, nil + } + payload = append(payload, name) + case map[string]any: + name, err := RequiredParam[string](v, "name") if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update issue", resp, err), nil, nil + return utils.NewToolResultError("each label object must have a 'name' string"), nil, nil } - defer func() { _ = resp.Body.Close() }() - - r, err := json.Marshal(MinimalResponse{ - ID: fmt.Sprintf("%d", issue.GetID()), - URL: issue.GetHTMLURL(), - }) + name = strings.TrimSpace(name) + if name == "" { + return utils.NewToolResultError("label name cannot be empty"), nil, nil + } + rationale, err := OptionalParam[string](v, "rationale") if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } - return utils.NewToolResultText(string(r)), nil, nil - }, - ) - st.FeatureFlagEnable = FeatureFlagIssuesGranular - return st + rationale = strings.TrimSpace(rationale) + if len([]rune(rationale)) > 280 { + return utils.NewToolResultError("label rationale must be 280 characters or less"), nil, nil + } + confidence, err := OptionalParam[string](v, "confidence") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + confidence = normalizeConfidence(confidence) + if confidence != "" && confidence != "LOW" && confidence != "MEDIUM" && confidence != "HIGH" { + return utils.NewToolResultError("confidence must be one of: LOW, MEDIUM, HIGH"), nil, nil + } + isSuggestion, err := OptionalParam[bool](v, "is_suggestion") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if rationale == "" && !isSuggestion && confidence == "" { + payload = append(payload, name) + } else { + useObjectForm = true + payload = append(payload, labelWithIntent{Name: name, Rationale: rationale, Confidence: confidence, Suggest: isSuggestion}) + } + default: + return utils.NewToolResultError("each label must be a string or an object with 'name' and optional 'rationale', 'confidence', and/or 'is_suggestion'"), nil, nil + } + } + + var body any + if useObjectForm { + body = &labelsUpdateRequest{Labels: payload} + } else { + // Preserve the standard wire format when no rationale or suggest is supplied. + names := make([]string, len(payload)) + for i, p := range payload { + names[i] = p.(string) + } + body = &github.IssueRequest{Labels: &names} + } + + apiURL := fmt.Sprintf("repos/%s/%s/issues/%d", owner, repo, issueNumber) + req, err := client.NewRequest(ctx, "PATCH", apiURL, body) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil + } + + issue := &github.Issue{} + resp, err := client.Do(req, issue) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update issue", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + r, err := json.Marshal(issue.Labels) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + + result := utils.NewToolResultText(string(r)) + // Labels are structural repo metadata defined by collaborators + // (trusted); confidentiality follows repo visibility. + result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoMetadata) + return result, nil, nil +} + +// addIssueLabels adds labels to an issue without removing any existing labels. +func addIssueLabels(ctx context.Context, deps ToolDependencies, client *github.Client, owner, repo string, issueNumber int, labelsSlice []any) (*mcp.CallToolResult, any, error) { + names, err := issueLabelNames(labelsSlice) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if len(names) == 0 { + return utils.NewToolResultError("at least one label is required"), nil, nil + } + + labels, resp, err := client.Issues.AddLabelsToIssue(ctx, owner, repo, issueNumber, names) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add labels to issue", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + r, err := json.Marshal(labels) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + + result := utils.NewToolResultText(string(r)) + // Labels are structural repo metadata defined by collaborators + // (trusted); confidentiality follows repo visibility. + result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoMetadata) + return result, nil, nil +} + +// removeIssueLabels removes the named labels from an issue, leaving the rest in +// place. Labels are removed one at a time; the issue's remaining labels after +// the final removal are returned. +func removeIssueLabels(ctx context.Context, deps ToolDependencies, client *github.Client, owner, repo string, issueNumber int, labelsSlice []any) (*mcp.CallToolResult, any, error) { + names, err := issueLabelNames(labelsSlice) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if len(names) == 0 { + return utils.NewToolResultError("at least one label is required"), nil, nil + } + + var remaining []*github.Label + for _, name := range names { + // Build the DELETE request manually so the label name is URL-escaped. + // go-github's Issues.RemoveLabelForIssue interpolates the raw label + // into the path, so names containing spaces or slashes (e.g. "good + // first issue", "status/blocked") would produce a malformed URL or + // extra path segments. url.PathEscape keeps the label a single, + // correctly-encoded path segment. + apiURL := fmt.Sprintf("repos/%s/%s/issues/%d/labels/%s", owner, repo, issueNumber, url.PathEscape(name)) + req, err := client.NewRequest(ctx, "DELETE", apiURL, nil) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil + } + + // The API responds with the issue's remaining labels. + resp, err := client.Do(req, &remaining) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to remove label from issue", resp, err), nil, nil + } + _ = resp.Body.Close() + } + + r, err := json.Marshal(remaining) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + + result := utils.NewToolResultText(string(r)) + // Labels are structural repo metadata defined by collaborators + // (trusted); confidentiality follows repo visibility. + result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoMetadata) + return result, nil, nil } // GranularUpdateIssueMilestone creates a tool to update an issue's milestone.