Skip to content

Commit 1a2c2c0

Browse files
fix(context): preserve legacy team input schemas
Keep empty identifier rejection in the handler without tightening the published input schema. Pin modern text to structured output and preserve the legacy formatter fixtures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent a06e9e3 commit 1a2c2c0

3 files changed

Lines changed: 44 additions & 11 deletions

File tree

‎pkg/github/__toolsnaps__/get_team_members.snap‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,10 @@
1010
"properties": {
1111
"org": {
1212
"description": "Organization login (owner) that contains the team.",
13-
"minLength": 1,
1413
"type": "string"
1514
},
1615
"team_slug": {
1716
"description": "Team slug",
18-
"minLength": 1,
1917
"type": "string"
2018
}
2119
},

‎pkg/github/context_tools.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,6 @@ func GetTeamMembers(t translations.TranslationHelperFunc) inventory.ServerTool {
247247
"org": t("TOOL_GET_TEAM_MEMBERS_ORG_DESCRIPTION", "Organization login (owner) that contains the team."),
248248
"team_slug": t("TOOL_GET_TEAM_MEMBERS_TEAM_SLUG_DESCRIPTION", "Team slug"),
249249
})
250-
minLength := 1
251-
inputSchema.Properties["org"].MinLength = &minLength
252-
inputSchema.Properties["team_slug"].MinLength = &minLength
253250

254251
return NewTool[GetTeamMembersInput, []string](
255252
ToolsetMetadataContext,

‎pkg/github/context_tools_test.go‎

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,33 @@ func TestContextToolsTypedRegistration(t *testing.T) {
295295
require.NoError(t, json.Unmarshal(schemaJSON, &schema))
296296
assert.Equal(t, "object", schema.Type, "input roots must remain non-nullable objects")
297297
assert.Empty(t, schema.AnyOf)
298+
var schemaMetadata struct {
299+
AdditionalProperties *bool `json:"additionalProperties"`
300+
}
301+
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+
}
305+
for propertyName, property := range schema.Properties {
306+
var propertySchema jsonschema.Schema
307+
require.NoError(t, json.Unmarshal(property, &propertySchema))
308+
assert.Empty(t, propertySchema.Enum, "input enums must match the legacy schema")
309+
assert.Nil(t, propertySchema.Minimum)
310+
assert.Nil(t, propertySchema.Maximum)
311+
assert.Nil(t, propertySchema.MinLength, "input bounds must match the legacy schema")
312+
assert.Nil(t, propertySchema.MaxLength)
313+
assert.Nil(t, propertySchema.Default)
314+
switch tool.Name + "." + propertyName {
315+
case "get_teams.user":
316+
assert.Equal(t, "Username to get teams for. If not provided, uses the authenticated user.", propertySchema.Description)
317+
case "get_team_members.org":
318+
assert.Equal(t, "Organization login (owner) that contains the team.", propertySchema.Description)
319+
case "get_team_members.team_slug":
320+
assert.Equal(t, "Team slug", propertySchema.Description)
321+
default:
322+
t.Fatalf("unexpected input property %q on tool %q", propertyName, tool.Name)
323+
}
324+
}
298325
switch tool.Name {
299326
case "get_me":
300327
assert.NotNil(t, schema.Properties, "empty input schemas must retain properties")
@@ -325,8 +352,11 @@ func TestContextToolsTypedRegistration(t *testing.T) {
325352
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returnedUser))
326353
legacyText, err := json.Marshal(returnedUser)
327354
require.NoError(t, err)
328-
assert.Equal(t, string(legacyText), getTextResult(t, result).Text)
329-
assert.Equal(t, `{"login":"testuser","profile_url":"https://github.com/testuser","details":{"public_repos":0,"public_gists":0,"followers":0,"following":0,"created_at":"2020-01-02T03:04:05Z","updated_at":"0001-01-01T00:00:00Z"}}`, getTextResult(t, result).Text)
355+
assert.JSONEq(t, string(legacyText), getTextResult(t, result).Text)
356+
request := createMCPRequest(map[string]any{})
357+
legacyResult, err := getMeTool.Handler(deps)(ContextWithDeps(context.Background(), deps), &request)
358+
require.NoError(t, err)
359+
assert.Equal(t, `{"login":"testuser","profile_url":"https://github.com/testuser","details":{"public_repos":0,"public_gists":0,"followers":0,"following":0,"created_at":"2020-01-02T03:04:05Z","updated_at":"0001-01-01T00:00:00Z"}}`, getTextResult(t, legacyResult).Text)
330360
require.NoError(t, outputSchemas["get_me"].Validate(result.StructuredContent))
331361

332362
result, err = clientSession.CallTool(context.Background(), &mcp.CallToolParams{
@@ -338,7 +368,11 @@ func TestContextToolsTypedRegistration(t *testing.T) {
338368
structuredJSON, err = json.Marshal(result.StructuredContent)
339369
require.NoError(t, err)
340370
assert.JSONEq(t, `[{"org":"testorg","teams":[{"name":"team1","slug":"team1","description":"Team 1"}]}]`, string(structuredJSON))
341-
assert.Equal(t, `[{"org":"testorg","teams":[{"name":"team1","slug":"team1","description":"Team 1"}]}]`, getTextResult(t, result).Text)
371+
assert.Equal(t, string(structuredJSON), getTextResult(t, result).Text)
372+
request = createMCPRequest(map[string]any{"user": "specificuser"})
373+
legacyResult, err = getTeamsTool.Handler(deps)(ContextWithDeps(context.Background(), deps), &request)
374+
require.NoError(t, err)
375+
assert.Equal(t, `[{"org":"testorg","teams":[{"name":"team1","slug":"team1","description":"Team 1"}]}]`, getTextResult(t, legacyResult).Text)
342376
require.NoError(t, outputSchemas["get_teams"].Validate(result.StructuredContent))
343377
assert.Equal(t, 1, graphQLCalls)
344378

@@ -351,7 +385,11 @@ func TestContextToolsTypedRegistration(t *testing.T) {
351385
structuredJSON, err = json.Marshal(result.StructuredContent)
352386
require.NoError(t, err)
353387
assert.JSONEq(t, `["user1","user2"]`, string(structuredJSON))
354-
assert.Equal(t, `["user1","user2"]`, getTextResult(t, result).Text)
388+
assert.Equal(t, string(structuredJSON), getTextResult(t, result).Text)
389+
request = createMCPRequest(map[string]any{"org": "testorg", "team_slug": "testteam"})
390+
legacyResult, err = teamMembersTool.Handler(deps)(ContextWithDeps(context.Background(), deps), &request)
391+
require.NoError(t, err)
392+
assert.Equal(t, `["user1","user2"]`, getTextResult(t, legacyResult).Text)
355393
require.NoError(t, outputSchemas["get_team_members"].Validate(result.StructuredContent))
356394
assert.Equal(t, 2, graphQLCalls)
357395

@@ -369,7 +407,7 @@ func TestContextToolsTypedRegistration(t *testing.T) {
369407
})
370408
require.NoError(t, err)
371409
assert.True(t, result.IsError, "empty required strings should remain invalid")
372-
assert.Equal(t, 2, graphQLCalls, "schema validation must reject empty required strings before the handler")
410+
assert.Equal(t, 2, graphQLCalls, "handler validation must reject empty identifiers before acquiring GraphQL")
373411

374412
for _, tc := range []struct {
375413
name string
@@ -397,10 +435,10 @@ func TestContextToolsTypedRegistration(t *testing.T) {
397435
require.False(t, result.IsError)
398436
require.NotNil(t, result.StructuredContent, "empty successful collections must have structured content on the wire")
399437
require.Len(t, result.Content, 1, "SDK fallback must not duplicate the legacy text")
400-
assert.Equal(t, tc.text, getTextResult(t, result).Text)
401438
structuredJSON, err := json.Marshal(result.StructuredContent)
402439
require.NoError(t, err)
403440
assert.JSONEq(t, tc.structured, string(structuredJSON))
441+
assert.Equal(t, string(structuredJSON), getTextResult(t, result).Text)
404442
require.NoError(t, outputSchemas[tc.name].Validate(result.StructuredContent))
405443
}
406444

0 commit comments

Comments
 (0)