Skip to content

Commit 7b6646c

Browse files
fix(issues): report silently dropped labels (#3195)
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>
1 parent c2a9364 commit 7b6646c

2 files changed

Lines changed: 162 additions & 0 deletions

File tree

‎pkg/github/issues.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2941,6 +2941,50 @@ func resolveIssueTypeID(ctx context.Context, client *github.Client, owner, repo,
29412941
return "", resp, fmt.Errorf("issue type %q was not found in %s/%s", issueTypeName, owner, repo)
29422942
}
29432943

2944+
func unappliedIssueLabelsError(requested []string, issue *github.Issue) error {
2945+
applied := make([]string, 0, len(issue.Labels))
2946+
for _, label := range issue.Labels {
2947+
if label != nil {
2948+
applied = append(applied, label.GetName())
2949+
}
2950+
}
2951+
2952+
missing := issueLabelDifference(requested, applied)
2953+
unexpected := issueLabelDifference(applied, requested)
2954+
if len(missing) == 0 && len(unexpected) == 0 {
2955+
return nil
2956+
}
2957+
2958+
return fmt.Errorf(
2959+
"requested=%q, applied=%q, missing=%q, unexpected=%q, issue_url=%q; the caller may lack AddLabelsToLabelable permission",
2960+
requested,
2961+
applied,
2962+
missing,
2963+
unexpected,
2964+
issue.GetHTMLURL(),
2965+
)
2966+
}
2967+
2968+
func issueLabelDifference(labels, other []string) []string {
2969+
var difference []string
2970+
for _, label := range labels {
2971+
if containsIssueLabel(other, label) || containsIssueLabel(difference, label) {
2972+
continue
2973+
}
2974+
difference = append(difference, label)
2975+
}
2976+
return difference
2977+
}
2978+
2979+
func containsIssueLabel(labels []string, target string) bool {
2980+
for _, label := range labels {
2981+
if strings.EqualFold(label, target) {
2982+
return true
2983+
}
2984+
}
2985+
return false
2986+
}
2987+
29442988
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) {
29452989
if title == "" {
29462990
return utils.NewToolResultError("missing required parameter: title"), nil
@@ -2981,6 +3025,12 @@ func CreateIssue(ctx context.Context, client *github.Client, owner string, repo
29813025
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create issue", resp, body), nil
29823026
}
29833027

3028+
if len(labels) > 0 {
3029+
if err := unappliedIssueLabelsError(labels, issue); err != nil {
3030+
return ghErrors.NewGitHubAPIErrorResponse(ctx, "issue created but requested labels were not fully applied", resp, err), nil
3031+
}
3032+
}
3033+
29843034
// Return minimal response with just essential information
29853035
minimalResponse := MinimalResponse{
29863036
ID: fmt.Sprintf("%d", issue.GetID()),
@@ -3206,6 +3256,12 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4
32063256
}
32073257
}
32083258

3259+
if updateOptions.LabelsProvided {
3260+
if err := unappliedIssueLabelsError(labels, updatedIssue); err != nil {
3261+
return ghErrors.NewGitHubAPIErrorResponse(ctx, "issue updated but requested labels were not fully applied", resp, err), nil
3262+
}
3263+
}
3264+
32093265
// Return minimal response with just essential information
32103266
minimalResponse := MinimalResponse{
32113267
ID: fmt.Sprintf("%d", updatedIssue.GetID()),

‎pkg/github/issues_test.go‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2090,6 +2090,112 @@ func Test_CreateIssue(t *testing.T) {
20902090
}
20912091
}
20922092

2093+
func TestIssueWriteReportsUnappliedLabels(t *testing.T) {
2094+
tests := []struct {
2095+
name string
2096+
method string
2097+
requestedLabels []string
2098+
appliedLabels []string
2099+
expectedMissing string
2100+
expectedUnexpected string
2101+
}{
2102+
{
2103+
name: "create reports partially applied labels",
2104+
method: "create",
2105+
requestedLabels: []string{"bug", "enhancement"},
2106+
appliedLabels: []string{"bug"},
2107+
expectedMissing: `missing=["enhancement"]`,
2108+
expectedUnexpected: "unexpected=[]",
2109+
},
2110+
{
2111+
name: "update reports silently dropped labels",
2112+
method: "update",
2113+
requestedLabels: []string{"enhancement"},
2114+
appliedLabels: []string{"existing"},
2115+
expectedMissing: `missing=["enhancement"]`,
2116+
expectedUnexpected: `unexpected=["existing"]`,
2117+
},
2118+
{
2119+
name: "update reports labels that were not cleared",
2120+
method: "update",
2121+
requestedLabels: []string{},
2122+
appliedLabels: []string{"existing"},
2123+
expectedMissing: "missing=[]",
2124+
expectedUnexpected: `unexpected=["existing"]`,
2125+
},
2126+
}
2127+
2128+
for _, tc := range tests {
2129+
t.Run(tc.name, func(t *testing.T) {
2130+
responseLabels := make([]*github.Label, 0, len(tc.appliedLabels))
2131+
for _, label := range tc.appliedLabels {
2132+
responseLabels = append(responseLabels, &github.Label{Name: label})
2133+
}
2134+
responseIssue := &github.Issue{
2135+
ID: github.Ptr(int64(123)),
2136+
Number: github.Ptr(123),
2137+
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"),
2138+
Labels: responseLabels,
2139+
}
2140+
2141+
endpoint := PostReposIssuesByOwnerByRepo
2142+
status := http.StatusCreated
2143+
if tc.method == "update" {
2144+
endpoint = PatchReposIssuesByOwnerByRepoByIssueNumber
2145+
status = http.StatusOK
2146+
}
2147+
restHTTPClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
2148+
endpoint: mockResponse(t, status, responseIssue),
2149+
})
2150+
restRequests := &requestCountingTransport{inner: restHTTPClient.Transport}
2151+
restHTTPClient.Transport = restRequests
2152+
2153+
gqlHTTPClient := githubv4mock.NewMockedHTTPClient()
2154+
gqlRequests := &requestCountingTransport{inner: gqlHTTPClient.Transport}
2155+
gqlHTTPClient.Transport = gqlRequests
2156+
2157+
requestLabels := make([]any, len(tc.requestedLabels))
2158+
for i, label := range tc.requestedLabels {
2159+
requestLabels[i] = label
2160+
}
2161+
requestArgs := map[string]any{
2162+
"method": tc.method,
2163+
"owner": "owner",
2164+
"repo": "repo",
2165+
"labels": requestLabels,
2166+
}
2167+
if tc.method == "create" {
2168+
requestArgs["title"] = "Test issue"
2169+
} else {
2170+
requestArgs["issue_number"] = float64(123)
2171+
}
2172+
2173+
deps := BaseDeps{
2174+
Client: mustNewGHClient(t, restHTTPClient),
2175+
GQLClient: githubv4.NewClient(gqlHTTPClient),
2176+
}
2177+
serverTool := IssueWrite(translations.NullTranslationHelper)
2178+
handler := serverTool.Handler(deps)
2179+
request := createMCPRequest(requestArgs)
2180+
2181+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
2182+
require.NoError(t, err)
2183+
require.True(t, result.IsError)
2184+
assert.Equal(t, 1, restRequests.count, "label verification must use the write response without a readback")
2185+
assert.Zero(t, gqlRequests.count, "label verification must not make a GraphQL readback")
2186+
2187+
resultText := getErrorResult(t, result).Text
2188+
assert.Contains(t, resultText, "issue "+tc.method+"d but requested labels were not fully applied")
2189+
assert.Contains(t, resultText, fmt.Sprintf("requested=%q", tc.requestedLabels))
2190+
assert.Contains(t, resultText, fmt.Sprintf("applied=%q", tc.appliedLabels))
2191+
assert.Contains(t, resultText, tc.expectedMissing)
2192+
assert.Contains(t, resultText, tc.expectedUnexpected)
2193+
assert.Contains(t, resultText, `issue_url="https://github.com/owner/repo/issues/123"`)
2194+
assert.Contains(t, resultText, "AddLabelsToLabelable permission")
2195+
})
2196+
}
2197+
}
2198+
20932199
// Test_IssueWrite_MCPAppsFeature_UIGate verifies the MCP Apps feature UI gate
20942200
// behavior: UI clients get a form message, non-UI clients execute directly.
20952201
func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {

0 commit comments

Comments
 (0)