From 3de9a103b5733110e13e0d5a19a15f250a230871 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 1 Sep 2026 10:18:16 +0200 Subject: [PATCH] fix(issues): report silently dropped labels Compare requested labels with the authoritative issue returned by create and update writes, and return a precise partial-failure result when GitHub omits them. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/github/issues.go | 56 ++++++++++++++++++++ pkg/github/issues_test.go | 106 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 9b6ee5da6b..0a46c4e44a 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -2941,6 +2941,50 @@ func resolveIssueTypeID(ctx context.Context, client *github.Client, owner, repo, return "", resp, fmt.Errorf("issue type %q was not found in %s/%s", issueTypeName, owner, repo) } +func unappliedIssueLabelsError(requested []string, issue *github.Issue) error { + applied := make([]string, 0, len(issue.Labels)) + for _, label := range issue.Labels { + if label != nil { + applied = append(applied, label.GetName()) + } + } + + missing := issueLabelDifference(requested, applied) + unexpected := issueLabelDifference(applied, requested) + if len(missing) == 0 && len(unexpected) == 0 { + return nil + } + + return fmt.Errorf( + "requested=%q, applied=%q, missing=%q, unexpected=%q, issue_url=%q; the caller may lack AddLabelsToLabelable permission", + requested, + applied, + missing, + unexpected, + issue.GetHTMLURL(), + ) +} + +func issueLabelDifference(labels, other []string) []string { + var difference []string + for _, label := range labels { + if containsIssueLabel(other, label) || containsIssueLabel(difference, label) { + continue + } + difference = append(difference, label) + } + return difference +} + +func containsIssueLabel(labels []string, target string) bool { + for _, label := range labels { + if strings.EqualFold(label, target) { + return true + } + } + return false +} + func CreateIssue(ctx context.Context, client *github.Client, owner string, repo string, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue) (*mcp.CallToolResult, error) { if title == "" { return utils.NewToolResultError("missing required parameter: title"), nil @@ -2981,6 +3025,12 @@ func CreateIssue(ctx context.Context, client *github.Client, owner string, repo return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create issue", resp, body), nil } + if len(labels) > 0 { + if err := unappliedIssueLabelsError(labels, issue); err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "issue created but requested labels were not fully applied", resp, err), nil + } + } + // Return minimal response with just essential information minimalResponse := MinimalResponse{ ID: fmt.Sprintf("%d", issue.GetID()), @@ -3206,6 +3256,12 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 } } + if updateOptions.LabelsProvided { + if err := unappliedIssueLabelsError(labels, updatedIssue); err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "issue updated but requested labels were not fully applied", resp, err), nil + } + } + // Return minimal response with just essential information minimalResponse := MinimalResponse{ ID: fmt.Sprintf("%d", updatedIssue.GetID()), diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index bf024b545a..e8b4cd2c13 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2090,6 +2090,112 @@ func Test_CreateIssue(t *testing.T) { } } +func TestIssueWriteReportsUnappliedLabels(t *testing.T) { + tests := []struct { + name string + method string + requestedLabels []string + appliedLabels []string + expectedMissing string + expectedUnexpected string + }{ + { + name: "create reports partially applied labels", + method: "create", + requestedLabels: []string{"bug", "enhancement"}, + appliedLabels: []string{"bug"}, + expectedMissing: `missing=["enhancement"]`, + expectedUnexpected: "unexpected=[]", + }, + { + name: "update reports silently dropped labels", + method: "update", + requestedLabels: []string{"enhancement"}, + appliedLabels: []string{"existing"}, + expectedMissing: `missing=["enhancement"]`, + expectedUnexpected: `unexpected=["existing"]`, + }, + { + name: "update reports labels that were not cleared", + method: "update", + requestedLabels: []string{}, + appliedLabels: []string{"existing"}, + expectedMissing: "missing=[]", + expectedUnexpected: `unexpected=["existing"]`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + responseLabels := make([]*github.Label, 0, len(tc.appliedLabels)) + for _, label := range tc.appliedLabels { + responseLabels = append(responseLabels, &github.Label{Name: label}) + } + responseIssue := &github.Issue{ + ID: github.Ptr(int64(123)), + Number: github.Ptr(123), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + Labels: responseLabels, + } + + endpoint := PostReposIssuesByOwnerByRepo + status := http.StatusCreated + if tc.method == "update" { + endpoint = PatchReposIssuesByOwnerByRepoByIssueNumber + status = http.StatusOK + } + restHTTPClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + endpoint: mockResponse(t, status, responseIssue), + }) + restRequests := &requestCountingTransport{inner: restHTTPClient.Transport} + restHTTPClient.Transport = restRequests + + gqlHTTPClient := githubv4mock.NewMockedHTTPClient() + gqlRequests := &requestCountingTransport{inner: gqlHTTPClient.Transport} + gqlHTTPClient.Transport = gqlRequests + + requestLabels := make([]any, len(tc.requestedLabels)) + for i, label := range tc.requestedLabels { + requestLabels[i] = label + } + requestArgs := map[string]any{ + "method": tc.method, + "owner": "owner", + "repo": "repo", + "labels": requestLabels, + } + if tc.method == "create" { + requestArgs["title"] = "Test issue" + } else { + requestArgs["issue_number"] = float64(123) + } + + deps := BaseDeps{ + Client: mustNewGHClient(t, restHTTPClient), + GQLClient: githubv4.NewClient(gqlHTTPClient), + } + serverTool := IssueWrite(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + request := createMCPRequest(requestArgs) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Equal(t, 1, restRequests.count, "label verification must use the write response without a readback") + assert.Zero(t, gqlRequests.count, "label verification must not make a GraphQL readback") + + resultText := getErrorResult(t, result).Text + assert.Contains(t, resultText, "issue "+tc.method+"d but requested labels were not fully applied") + assert.Contains(t, resultText, fmt.Sprintf("requested=%q", tc.requestedLabels)) + assert.Contains(t, resultText, fmt.Sprintf("applied=%q", tc.appliedLabels)) + assert.Contains(t, resultText, tc.expectedMissing) + assert.Contains(t, resultText, tc.expectedUnexpected) + assert.Contains(t, resultText, `issue_url="https://github.com/owner/repo/issues/123"`) + assert.Contains(t, resultText, "AddLabelsToLabelable permission") + }) + } +} + // Test_IssueWrite_MCPAppsFeature_UIGate verifies the MCP Apps feature UI gate // behavior: UI clients get a form message, non-UI clients execute directly. func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {