Skip to content

Commit b55303d

Browse files
committed
fix: preserve typed batch label compatibility
1 parent b36634c commit b55303d

3 files changed

Lines changed: 159 additions & 4 deletions

File tree

‎pkg/github/__toolsnaps__/batch_update_issue_labels.snap‎

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,18 @@
77
"title": "Batch Update Issue Labels"
88
},
99
"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.",
10+
"icons": [
11+
{
12+
"mimeType": "image/png",
13+
"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",
14+
"theme": "light"
15+
},
16+
{
17+
"mimeType": "image/png",
18+
"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",
19+
"theme": "dark"
20+
}
21+
],
1022
"inputSchema": {
1123
"properties": {
1224
"operations": {
@@ -46,11 +58,13 @@
4658
},
4759
"owner": {
4860
"description": "Repository owner (username or organization)",
49-
"type": "string"
61+
"type": "string",
62+
"x-mcp-header": "owner"
5063
},
5164
"repo": {
5265
"description": "Repository name",
53-
"type": "string"
66+
"type": "string",
67+
"x-mcp-header": "repo"
5468
}
5569
},
5670
"required": [
@@ -60,5 +74,51 @@
6074
],
6175
"type": "object"
6276
},
63-
"name": "batch_update_issue_labels"
77+
"name": "batch_update_issue_labels",
78+
"outputSchema": {
79+
"additionalProperties": false,
80+
"properties": {
81+
"results": {
82+
"items": {
83+
"additionalProperties": false,
84+
"properties": {
85+
"applied": {
86+
"type": "boolean"
87+
},
88+
"error": {
89+
"type": "string"
90+
},
91+
"issue_number": {
92+
"type": "integer"
93+
},
94+
"labels": {
95+
"items": {
96+
"type": "string"
97+
},
98+
"type": [
99+
"null",
100+
"array"
101+
]
102+
}
103+
},
104+
"required": [
105+
"issue_number",
106+
"applied"
107+
],
108+
"type": "object"
109+
},
110+
"type": [
111+
"null",
112+
"array"
113+
]
114+
}
115+
},
116+
"required": [
117+
"results"
118+
],
119+
"type": [
120+
"null",
121+
"object"
122+
]
123+
}
64124
}

‎pkg/github/batch_labels_granular_test.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,17 @@ package github
22

33
import (
44
"context"
5+
"encoding/json"
56
"net/http"
67
"strings"
78
"testing"
89
"time"
910

11+
"github.com/github/github-mcp-server/v2/internal/toolsnaps"
12+
"github.com/github/github-mcp-server/v2/pkg/inventory"
1013
"github.com/github/github-mcp-server/v2/pkg/translations"
1114
gogithub "github.com/google/go-github/v92/github"
15+
"github.com/google/jsonschema-go/jsonschema"
1216
"github.com/modelcontextprotocol/go-sdk/mcp"
1317
"github.com/stretchr/testify/assert"
1418
"github.com/stretchr/testify/require"
@@ -207,3 +211,74 @@ func TestGranularBatchUpdateIssueLabelsTransport(t *testing.T) {
207211
})
208212
}
209213
}
214+
215+
func TestGranularBatchUpdateIssueLabelsModernTypedSession(t *testing.T) {
216+
calls := 0
217+
httpClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
218+
PostReposIssuesByOwnerByRepoByIssueNumberLabels: func(w http.ResponseWriter, r *http.Request) {
219+
calls++
220+
expectPath(t, "/repos/owner/repo/issues/1/labels").
221+
andThen(expectRequestBody(t, []any{"bug"}).
222+
andThen(mockResponse(t, http.StatusOK, []*gogithub.Label{{Name: "bug"}})))(w, r)
223+
},
224+
})
225+
deps := BaseDeps{Client: mustNewGHClient(t, httpClient)}
226+
tool := GranularBatchUpdateIssueLabels(translations.NullTranslationHelper)
227+
inv, err := inventory.NewBuilder().SetTools([]inventory.ServerTool{tool}).
228+
WithToolsets([]string{"all"}).
229+
WithFeatureChecker(featureCheckerFor(inventory.FeatureFlag(FeatureFlagIssuesGranular))).Build()
230+
require.NoError(t, err)
231+
server := mcp.NewServer(&mcp.Implementation{Name: "batch-label-typed-test", Version: "v1"}, nil)
232+
server.AddReceivingMiddleware(InjectDepsMiddleware(deps))
233+
inv.RegisterTools(context.Background(), server, deps)
234+
session := connectCommentVisibilityClient(t, server, inventory.ProtocolVersionMultiRoundTrip)
235+
236+
list, err := session.ListTools(context.Background(), nil)
237+
require.NoError(t, err)
238+
require.Len(t, list.Tools, 1)
239+
require.Equal(t, tool.Tool.Name, list.Tools[0].Name)
240+
require.NotNil(t, list.Tools[0].OutputSchema)
241+
var schema jsonschema.Schema
242+
require.NoError(t, json.Unmarshal([]byte(mustMarshalJSON(t, list.Tools[0].OutputSchema)), &schema))
243+
resolved, err := schema.Resolve(nil)
244+
require.NoError(t, err)
245+
require.NoError(t, toolsnaps.Test(tool.Tool.Name, *list.Tools[0]))
246+
247+
result, err := session.CallTool(context.Background(), &mcp.CallToolParams{
248+
Name: tool.Tool.Name,
249+
Arguments: map[string]any{
250+
"owner": "owner",
251+
"repo": "repo",
252+
"operations": []any{map[string]any{
253+
// RequiredInt accepted this numeric string before typed decoding.
254+
"issue_number": "1.0",
255+
"add": []any{"bug"},
256+
}},
257+
},
258+
})
259+
require.NoError(t, err)
260+
require.False(t, result.IsError, mustMarshalJSON(t, result))
261+
expected := `{"results":[{"issue_number":1,"applied":true,"labels":["bug"]}]}`
262+
assert.JSONEq(t, expected, getTextResult(t, result).Text)
263+
require.NotNil(t, result.StructuredContent)
264+
assert.JSONEq(t, expected, mustMarshalJSON(t, result.StructuredContent))
265+
require.NoError(t, resolved.Validate(result.StructuredContent))
266+
assert.Equal(t, 1, calls)
267+
268+
invalid, err := session.CallTool(context.Background(), &mcp.CallToolParams{
269+
Name: tool.Tool.Name,
270+
Arguments: map[string]any{
271+
"owner": "owner",
272+
"repo": "repo",
273+
"operations": []any{
274+
map[string]any{"issue_number": "1.0", "add": []any{"bug"}},
275+
map[string]any{"issue_number": "2", "add": []any{" "}},
276+
},
277+
},
278+
})
279+
require.NoError(t, err)
280+
require.True(t, invalid.IsError)
281+
assert.Nil(t, invalid.StructuredContent)
282+
assert.Contains(t, getErrorResult(t, invalid).Text, "operations[1]")
283+
assert.Equal(t, 1, calls, "validation must precede every mutation")
284+
}

‎pkg/github/issues_granular.go‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -664,9 +664,29 @@ func normalizeGranularBatchUpdateIssueLabelsArguments(raw json.RawMessage) (json
664664
return nil, err
665665
}
666666
}
667-
if _, ok := args["operations"]; !ok {
667+
operationsValue, ok := args["operations"]
668+
if !ok {
668669
return nil, fmt.Errorf("missing required parameter: operations")
669670
}
671+
operations, ok := operationsValue.([]any)
672+
if !ok {
673+
return nil, fmt.Errorf("parameter operations is not of type array, is %T", operationsValue)
674+
}
675+
for i, operationValue := range operations {
676+
operation, ok := operationValue.(map[string]any)
677+
if !ok {
678+
return nil, fmt.Errorf("operations[%d] is not an object, is %T", i, operationValue)
679+
}
680+
issueNumber, ok := operation["issue_number"]
681+
if !ok {
682+
continue
683+
}
684+
normalizedIssueNumber, err := toInt(issueNumber)
685+
if err != nil {
686+
return nil, fmt.Errorf("operations[%d].issue_number: %w", i, err)
687+
}
688+
operation["issue_number"] = normalizedIssueNumber
689+
}
670690
return json.Marshal(args)
671691
}
672692

0 commit comments

Comments
 (0)