Skip to content

Commit 7abccda

Browse files
fix(context): preserve advertised input schema compatibility
Keep legacy-visible schemas byte-compatible while applying unknown-field acceptance only to the cached runtime validation schema. Verify modern DTO text parity and preserve legacy formatted text. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 1a2c2c0 commit 7abccda

5 files changed

Lines changed: 53 additions & 28 deletions

File tree

‎pkg/github/__toolsnaps__/get_me.snap‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
},
1616
"description": "Get details of the authenticated GitHub user. Use this when a request is about the user's own profile for GitHub. Or when information is missing to build other tool calls.",
1717
"inputSchema": {
18-
"additionalProperties": true,
1918
"properties": {},
2019
"type": "object"
2120
},

‎pkg/github/__toolsnaps__/get_team_members.snap‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
},
77
"description": "Get member usernames of a specific team in an organization. Limited to organizations accessible with current credentials",
88
"inputSchema": {
9-
"additionalProperties": true,
109
"properties": {
1110
"org": {
1211
"description": "Organization login (owner) that contains the team.",

‎pkg/github/__toolsnaps__/get_teams.snap‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
},
77
"description": "Get details of the teams the user is a member of. Limited to organizations accessible with current credentials",
88
"inputSchema": {
9-
"additionalProperties": true,
109
"properties": {
1110
"user": {
1211
"description": "Username to get teams for. If not provided, uses the authenticated user.",

‎pkg/github/context_tools.go‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,19 @@ func contextToolInputSchema[In any](descriptions map[string]string) *jsonschema.
3838
if schema.Properties == nil {
3939
schema.Properties = make(map[string]*jsonschema.Schema)
4040
}
41-
schema.AdditionalProperties = &jsonschema.Schema{}
41+
schema.AdditionalProperties = nil
4242
for name, description := range descriptions {
4343
schema.Properties[name].Description = description
4444
}
4545
return schema
4646
}
4747

48+
func contextToolValidationSchema(schema *jsonschema.Schema) *jsonschema.Schema {
49+
validationSchema := schema.CloneSchemas()
50+
validationSchema.AdditionalProperties = &jsonschema.Schema{}
51+
return validationSchema
52+
}
53+
4854
// UserDetails contains additional fields about a GitHub user not already
4955
// present in MinimalUser. Used by get_me context tool but omitted from search_users.
5056
type UserDetails struct {
@@ -69,7 +75,8 @@ type UserDetails struct {
6975

7076
// GetMe creates a tool to get details of the authenticated user.
7177
func GetMe(t translations.TranslationHelperFunc) inventory.ServerTool {
72-
return NewTool[GetMeInput, MinimalUser](
78+
inputSchema := contextToolInputSchema[GetMeInput](nil)
79+
return NewToolWithSchemaOptions[GetMeInput, MinimalUser](
7380
ToolsetMetadataContext,
7481
mcp.Tool{
7582
Name: "get_me",
@@ -78,7 +85,7 @@ func GetMe(t translations.TranslationHelperFunc) inventory.ServerTool {
7885
Title: t("TOOL_GET_ME_USER_TITLE", "Get my user profile"),
7986
ReadOnlyHint: true,
8087
},
81-
InputSchema: contextToolInputSchema[GetMeInput](nil),
88+
InputSchema: inputSchema,
8289
Meta: mcp.Meta{
8390
"ui": map[string]any{
8491
"resourceUri": GetMeUIResourceURI,
@@ -87,6 +94,9 @@ func GetMe(t translations.TranslationHelperFunc) inventory.ServerTool {
8794
},
8895
},
8996
scopes.NoScopes(),
97+
inventory.TypedSchemaOptions{
98+
ValidationInputSchema: contextToolValidationSchema(inputSchema),
99+
},
90100
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, _ GetMeInput) (*mcp.CallToolResult, MinimalUser, error) {
91101
client, err := deps.GetClient(ctx)
92102
if err != nil {
@@ -148,7 +158,10 @@ type OrganizationTeams struct {
148158
}
149159

150160
func GetTeams(t translations.TranslationHelperFunc) inventory.ServerTool {
151-
return NewTool[GetTeamsInput, []OrganizationTeams](
161+
inputSchema := contextToolInputSchema[GetTeamsInput](map[string]string{
162+
"user": t("TOOL_GET_TEAMS_USER_DESCRIPTION", "Username to get teams for. If not provided, uses the authenticated user."),
163+
})
164+
return NewToolWithSchemaOptions[GetTeamsInput, []OrganizationTeams](
152165
ToolsetMetadataContext,
153166
mcp.Tool{
154167
Name: "get_teams",
@@ -157,11 +170,12 @@ func GetTeams(t translations.TranslationHelperFunc) inventory.ServerTool {
157170
Title: t("TOOL_GET_TEAMS_TITLE", "Get teams"),
158171
ReadOnlyHint: true,
159172
},
160-
InputSchema: contextToolInputSchema[GetTeamsInput](map[string]string{
161-
"user": t("TOOL_GET_TEAMS_USER_DESCRIPTION", "Username to get teams for. If not provided, uses the authenticated user."),
162-
}),
173+
InputSchema: inputSchema,
163174
},
164175
scopes.RequireAll(scopes.ReadOrg),
176+
inventory.TypedSchemaOptions{
177+
ValidationInputSchema: contextToolValidationSchema(inputSchema),
178+
},
165179
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args GetTeamsInput) (*mcp.CallToolResult, []OrganizationTeams, error) {
166180
var username string
167181
if args.User != "" {
@@ -248,7 +262,7 @@ func GetTeamMembers(t translations.TranslationHelperFunc) inventory.ServerTool {
248262
"team_slug": t("TOOL_GET_TEAM_MEMBERS_TEAM_SLUG_DESCRIPTION", "Team slug"),
249263
})
250264

251-
return NewTool[GetTeamMembersInput, []string](
265+
return NewToolWithSchemaOptions[GetTeamMembersInput, []string](
252266
ToolsetMetadataContext,
253267
mcp.Tool{
254268
Name: "get_team_members",
@@ -260,6 +274,9 @@ func GetTeamMembers(t translations.TranslationHelperFunc) inventory.ServerTool {
260274
InputSchema: inputSchema,
261275
},
262276
scopes.RequireAll(scopes.ReadOrg),
277+
inventory.TypedSchemaOptions{
278+
ValidationInputSchema: contextToolValidationSchema(inputSchema),
279+
},
263280
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args GetTeamMembersInput) (*mcp.CallToolResult, []string, error) {
264281
if args.Org == "" {
265282
return utils.NewToolResultError("missing required parameter: org"), nil, nil

‎pkg/github/context_tools_test.go‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/github/github-mcp-server/internal/githubv4mock"
1111
"github.com/github/github-mcp-server/internal/toolsnaps"
12+
"github.com/github/github-mcp-server/pkg/inventory"
1213
"github.com/github/github-mcp-server/pkg/translations"
1314
"github.com/google/go-github/v92/github"
1415
"github.com/google/jsonschema-go/jsonschema"
@@ -255,23 +256,30 @@ func TestContextToolsTypedRegistration(t *testing.T) {
255256
server := mcp.NewServer(&mcp.Implementation{Name: "test-server", Version: "v0.0.1"}, nil)
256257
server.AddReceivingMiddleware(InjectDepsMiddleware(deps))
257258
getMeTool := GetMe(translations.NullTranslationHelper)
258-
getMeTool.RegisterFunc(server, nil)
259259
getTeamsTool := GetTeams(translations.NullTranslationHelper)
260-
getTeamsTool.RegisterFunc(server, nil)
261260
teamMembersTool := GetTeamMembers(translations.NullTranslationHelper)
262-
teamMembersTool.RegisterFunc(server, nil)
261+
inv, err := inventory.NewBuilder().
262+
SetTools([]inventory.ServerTool{getMeTool, getTeamsTool, teamMembersTool}).
263+
WithToolsets([]string{"all"}).
264+
Build()
265+
require.NoError(t, err)
266+
inv.RegisterTools(context.Background(), server, deps)
263267

264268
serverTransport, clientTransport := mcp.NewInMemoryTransports()
265269
serverSession, err := server.Connect(context.Background(), serverTransport, nil)
266270
require.NoError(t, err)
267271
t.Cleanup(func() { _ = serverSession.Close() })
268272

269273
client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v0.0.1"}, nil)
270-
clientSession, err := client.Connect(context.Background(), clientTransport, nil)
274+
clientSession, err := client.Connect(context.Background(), clientTransport, &mcp.ClientSessionOptions{
275+
ProtocolVersion: inventory.ProtocolVersionMultiRoundTrip,
276+
})
271277
require.NoError(t, err)
278+
require.Equal(t, inventory.ProtocolVersionMultiRoundTrip, clientSession.InitializeResult().ProtocolVersion)
272279
t.Cleanup(func() { _ = clientSession.Close() })
273280

274-
list, err := clientSession.ListTools(context.Background(), nil)
281+
protocolMeta := mcp.Meta{mcp.MetaKeyProtocolVersion: inventory.ProtocolVersionMultiRoundTrip}
282+
list, err := clientSession.ListTools(context.Background(), &mcp.ListToolsParams{Meta: protocolMeta})
275283
require.NoError(t, err)
276284
require.Len(t, list.Tools, 3)
277285
outputSchemas := make(map[string]*jsonschema.Resolved)
@@ -299,9 +307,7 @@ func TestContextToolsTypedRegistration(t *testing.T) {
299307
AdditionalProperties *bool `json:"additionalProperties"`
300308
}
301309
require.NoError(t, json.Unmarshal(schemaJSON, &schemaMetadata))
302-
if schemaMetadata.AdditionalProperties != nil {
303-
assert.True(t, *schemaMetadata.AdditionalProperties, "explicit true must retain the legacy default")
304-
}
310+
assert.Nil(t, schemaMetadata.AdditionalProperties, "advertised input schema must match the legacy snapshot bytes")
305311
for propertyName, property := range schema.Properties {
306312
var propertySchema jsonschema.Schema
307313
require.NoError(t, json.Unmarshal(property, &propertySchema))
@@ -340,6 +346,7 @@ func TestContextToolsTypedRegistration(t *testing.T) {
340346
result, err := clientSession.CallTool(context.Background(), &mcp.CallToolParams{
341347
Name: "get_me",
342348
Arguments: map[string]any{"legacy_ignored_argument": true},
349+
Meta: protocolMeta,
343350
})
344351
require.NoError(t, err)
345352
require.False(t, result.IsError)
@@ -362,52 +369,56 @@ func TestContextToolsTypedRegistration(t *testing.T) {
362369
result, err = clientSession.CallTool(context.Background(), &mcp.CallToolParams{
363370
Name: "get_teams",
364371
Arguments: map[string]any{"user": "specificuser", "legacy_ignored_argument": true},
372+
Meta: protocolMeta,
365373
})
366374
require.NoError(t, err)
367375
require.False(t, result.IsError)
368376
structuredJSON, err = json.Marshal(result.StructuredContent)
369377
require.NoError(t, err)
370378
assert.JSONEq(t, `[{"org":"testorg","teams":[{"name":"team1","slug":"team1","description":"Team 1"}]}]`, string(structuredJSON))
371-
assert.Equal(t, string(structuredJSON), getTextResult(t, result).Text)
379+
assert.JSONEq(t, string(structuredJSON), getTextResult(t, result).Text)
372380
request = createMCPRequest(map[string]any{"user": "specificuser"})
373381
legacyResult, err = getTeamsTool.Handler(deps)(ContextWithDeps(context.Background(), deps), &request)
374382
require.NoError(t, err)
375383
assert.Equal(t, `[{"org":"testorg","teams":[{"name":"team1","slug":"team1","description":"Team 1"}]}]`, getTextResult(t, legacyResult).Text)
376384
require.NoError(t, outputSchemas["get_teams"].Validate(result.StructuredContent))
377-
assert.Equal(t, 1, graphQLCalls)
385+
assert.Equal(t, 2, graphQLCalls)
378386

379387
result, err = clientSession.CallTool(context.Background(), &mcp.CallToolParams{
380388
Name: "get_team_members",
381389
Arguments: map[string]any{"org": "testorg", "team_slug": "testteam", "legacy_ignored_argument": true},
390+
Meta: protocolMeta,
382391
})
383392
require.NoError(t, err)
384393
require.False(t, result.IsError)
385394
structuredJSON, err = json.Marshal(result.StructuredContent)
386395
require.NoError(t, err)
387396
assert.JSONEq(t, `["user1","user2"]`, string(structuredJSON))
388-
assert.Equal(t, string(structuredJSON), getTextResult(t, result).Text)
397+
assert.JSONEq(t, string(structuredJSON), getTextResult(t, result).Text)
389398
request = createMCPRequest(map[string]any{"org": "testorg", "team_slug": "testteam"})
390399
legacyResult, err = teamMembersTool.Handler(deps)(ContextWithDeps(context.Background(), deps), &request)
391400
require.NoError(t, err)
392401
assert.Equal(t, `["user1","user2"]`, getTextResult(t, legacyResult).Text)
393402
require.NoError(t, outputSchemas["get_team_members"].Validate(result.StructuredContent))
394-
assert.Equal(t, 2, graphQLCalls)
403+
assert.Equal(t, 4, graphQLCalls)
395404

396405
result, err = clientSession.CallTool(context.Background(), &mcp.CallToolParams{
397406
Name: "get_team_members",
398407
Arguments: map[string]any{},
408+
Meta: protocolMeta,
399409
})
400410
require.NoError(t, err)
401411
assert.True(t, result.IsError, "missing required arguments should be rejected by the inferred input schema")
402-
assert.Equal(t, 2, graphQLCalls, "schema validation must happen before invoking the handler")
412+
assert.Equal(t, 4, graphQLCalls, "schema validation must happen before invoking the handler")
403413

404414
result, err = clientSession.CallTool(context.Background(), &mcp.CallToolParams{
405415
Name: "get_team_members",
406416
Arguments: map[string]any{"org": "", "team_slug": "testteam"},
417+
Meta: protocolMeta,
407418
})
408419
require.NoError(t, err)
409420
assert.True(t, result.IsError, "empty required strings should remain invalid")
410-
assert.Equal(t, 2, graphQLCalls, "handler validation must reject empty identifiers before acquiring GraphQL")
421+
assert.Equal(t, 4, graphQLCalls, "handler validation must reject empty identifiers before acquiring GraphQL")
411422

412423
for _, tc := range []struct {
413424
name string
@@ -430,20 +441,20 @@ func TestContextToolsTypedRegistration(t *testing.T) {
430441
require.NoError(t, err)
431442
require.False(t, legacyResult.IsError)
432443
assert.Equal(t, tc.text, getTextResult(t, legacyResult).Text)
433-
result, err := clientSession.CallTool(context.Background(), &mcp.CallToolParams{Name: tc.name, Arguments: tc.args})
444+
result, err := clientSession.CallTool(context.Background(), &mcp.CallToolParams{Name: tc.name, Arguments: tc.args, Meta: protocolMeta})
434445
require.NoError(t, err)
435446
require.False(t, result.IsError)
436447
require.NotNil(t, result.StructuredContent, "empty successful collections must have structured content on the wire")
437448
require.Len(t, result.Content, 1, "SDK fallback must not duplicate the legacy text")
438449
structuredJSON, err := json.Marshal(result.StructuredContent)
439450
require.NoError(t, err)
440451
assert.JSONEq(t, tc.structured, string(structuredJSON))
441-
assert.Equal(t, string(structuredJSON), getTextResult(t, result).Text)
452+
assert.JSONEq(t, string(structuredJSON), getTextResult(t, result).Text)
442453
require.NoError(t, outputSchemas[tc.name].Validate(result.StructuredContent))
443454
}
444455

445456
failGetMe = true
446-
result, err = clientSession.CallTool(context.Background(), &mcp.CallToolParams{Name: "get_me"})
457+
result, err = clientSession.CallTool(context.Background(), &mcp.CallToolParams{Name: "get_me", Meta: protocolMeta})
447458
require.NoError(t, err)
448459
assert.True(t, result.IsError)
449460
assert.Nil(t, result.StructuredContent, "handler errors must not expose a successful typed output")

0 commit comments

Comments
 (0)