From b4e0d4d640d5ebe5709f3ef66d1dad3738438a71 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 16 Jan 2026 05:10:27 +0000 Subject: [PATCH 1/6] Initial plan From c203acdd5afa97fce226296fddcf64dd97473c4a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 16 Jan 2026 05:15:08 +0000 Subject: [PATCH 2/6] Add schema normalization middleware for tool definitions - Created mcp.NormalizeInputSchema() to fix broken object schemas - Fixes issue where object schemas lack properties field - Integrated middleware into unified.go tool registration - Added comprehensive unit tests for normalization logic - Added integration tests to verify end-to-end behavior Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- internal/mcp/schema_normalizer.go | 56 ++++++ internal/mcp/schema_normalizer_test.go | 195 +++++++++++++++++++ internal/server/schema_normalization_test.go | 153 +++++++++++++++ internal/server/unified.go | 5 +- 4 files changed, 408 insertions(+), 1 deletion(-) create mode 100644 internal/mcp/schema_normalizer.go create mode 100644 internal/mcp/schema_normalizer_test.go create mode 100644 internal/server/schema_normalization_test.go diff --git a/internal/mcp/schema_normalizer.go b/internal/mcp/schema_normalizer.go new file mode 100644 index 00000000..58f88bca --- /dev/null +++ b/internal/mcp/schema_normalizer.go @@ -0,0 +1,56 @@ +package mcp + +import ( + "log" + + "github.com/githubnext/gh-aw-mcpg/internal/logger" +) + +var logSchemaNormalizer = logger.New("mcp:schema_normalizer") + +// NormalizeInputSchema fixes common schema validation issues in tool definitions +// that can cause downstream validation errors. +// +// Known issues fixed: +// 1. Object schemas without properties: When a schema declares "type": "object" +// but is missing the required "properties" field, we add an empty properties +// object to make it valid per JSON Schema standards. +func NormalizeInputSchema(schema map[string]interface{}, toolName string) map[string]interface{} { + if schema == nil { + return schema + } + + // Check if this is an object type schema + typeVal, hasType := schema["type"] + if !hasType { + return schema + } + + typeStr, isString := typeVal.(string) + if !isString || typeStr != "object" { + return schema + } + + // Check if properties field exists + _, hasProperties := schema["properties"] + _, hasAdditionalProperties := schema["additionalProperties"] + + // If it's an object type but missing both properties and additionalProperties, + // add an empty properties object to make it valid + if !hasProperties && !hasAdditionalProperties { + log.Printf("Schema normalization: Adding empty properties to object schema for tool '%s'", toolName) + logSchemaNormalizer.Printf("Normalizing schema for tool %s: adding empty properties to object type", toolName) + logger.LogWarn("backend", "Tool schema normalized: %s - added empty properties to object type schema", toolName) + + // Create a copy of the schema to avoid modifying the original + normalized := make(map[string]interface{}) + for k, v := range schema { + normalized[k] = v + } + normalized["properties"] = map[string]interface{}{} + + return normalized + } + + return schema +} diff --git a/internal/mcp/schema_normalizer_test.go b/internal/mcp/schema_normalizer_test.go new file mode 100644 index 00000000..af6a0d49 --- /dev/null +++ b/internal/mcp/schema_normalizer_test.go @@ -0,0 +1,195 @@ +package mcp + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNormalizeInputSchema_NilSchema(t *testing.T) { + result := NormalizeInputSchema(nil, "test-tool") + assert.Nil(t, result, "Nil schema should return nil") +} + +func TestNormalizeInputSchema_EmptySchema(t *testing.T) { + schema := map[string]interface{}{} + result := NormalizeInputSchema(schema, "test-tool") + assert.Equal(t, schema, result, "Empty schema should be unchanged") +} + +func TestNormalizeInputSchema_NonObjectType(t *testing.T) { + testCases := []struct { + name string + schema map[string]interface{} + }{ + { + name: "string type", + schema: map[string]interface{}{ + "type": "string", + }, + }, + { + name: "number type", + schema: map[string]interface{}{ + "type": "number", + }, + }, + { + name: "array type", + schema: map[string]interface{}{ + "type": "array", + "items": map[string]interface{}{ + "type": "string", + }, + }, + }, + { + name: "boolean type", + schema: map[string]interface{}{ + "type": "boolean", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := NormalizeInputSchema(tc.schema, "test-tool") + assert.Equal(t, tc.schema, result, "Non-object schema should be unchanged") + }) + } +} + +func TestNormalizeInputSchema_ObjectWithProperties(t *testing.T) { + schema := map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "name": map[string]interface{}{ + "type": "string", + }, + "age": map[string]interface{}{ + "type": "number", + }, + }, + "required": []string{"name"}, + } + + result := NormalizeInputSchema(schema, "test-tool") + assert.Equal(t, schema, result, "Object schema with properties should be unchanged") +} + +func TestNormalizeInputSchema_ObjectWithAdditionalProperties(t *testing.T) { + schema := map[string]interface{}{ + "type": "object", + "additionalProperties": true, + } + + result := NormalizeInputSchema(schema, "test-tool") + assert.Equal(t, schema, result, "Object schema with additionalProperties should be unchanged") +} + +func TestNormalizeInputSchema_ObjectWithBothPropertiesAndAdditionalProperties(t *testing.T) { + schema := map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "name": map[string]interface{}{ + "type": "string", + }, + }, + "additionalProperties": false, + } + + result := NormalizeInputSchema(schema, "test-tool") + assert.Equal(t, schema, result, "Object schema with both properties and additionalProperties should be unchanged") +} + +func TestNormalizeInputSchema_ObjectWithoutPropertiesBroken(t *testing.T) { + // This is the broken schema case from GitHub MCP Server + schema := map[string]interface{}{ + "type": "object", + } + + result := NormalizeInputSchema(schema, "github-get_commit") + + require.NotNil(t, result, "Result should not be nil") + assert.Equal(t, "object", result["type"], "Type should remain object") + + // Check that properties was added + properties, hasProperties := result["properties"] + require.True(t, hasProperties, "Properties field should be added") + + // Check that properties is an empty map + propertiesMap, isMap := properties.(map[string]interface{}) + require.True(t, isMap, "Properties should be a map") + assert.Empty(t, propertiesMap, "Properties should be an empty map") +} + +func TestNormalizeInputSchema_ObjectWithoutPropertiesDoesNotModifyOriginal(t *testing.T) { + // Verify that the original schema is not modified + schema := map[string]interface{}{ + "type": "object", + } + + result := NormalizeInputSchema(schema, "test-tool") + + // Original should not have properties + _, originalHasProperties := schema["properties"] + assert.False(t, originalHasProperties, "Original schema should not be modified") + + // Result should have properties + _, resultHasProperties := result["properties"] + assert.True(t, resultHasProperties, "Result should have properties field") +} + +func TestNormalizeInputSchema_ComplexObjectWithoutProperties(t *testing.T) { + // Object schema with other fields but missing properties + schema := map[string]interface{}{ + "type": "object", + "title": "Complex Schema", + "description": "A complex schema without properties", + "examples": []interface{}{ + map[string]interface{}{ + "foo": "bar", + }, + }, + } + + result := NormalizeInputSchema(schema, "complex-tool") + + require.NotNil(t, result, "Result should not be nil") + + // Check all original fields are preserved + assert.Equal(t, "object", result["type"]) + assert.Equal(t, "Complex Schema", result["title"]) + assert.Equal(t, "A complex schema without properties", result["description"]) + assert.NotNil(t, result["examples"]) + + // Check that properties was added + properties, hasProperties := result["properties"] + require.True(t, hasProperties, "Properties field should be added") + assert.Empty(t, properties.(map[string]interface{}), "Properties should be empty") +} + +func TestNormalizeInputSchema_NonStringType(t *testing.T) { + // Edge case: type field is not a string + schema := map[string]interface{}{ + "type": 123, // invalid, but should not crash + } + + result := NormalizeInputSchema(schema, "test-tool") + assert.Equal(t, schema, result, "Schema with non-string type should be unchanged") +} + +func TestNormalizeInputSchema_MultipleToolNames(t *testing.T) { + // Verify that different tool names work correctly + schema := map[string]interface{}{ + "type": "object", + } + + tools := []string{"tool-1", "tool-2", "github-get_commit", "issue_read"} + for _, toolName := range tools { + result := NormalizeInputSchema(schema, toolName) + _, hasProperties := result["properties"] + assert.True(t, hasProperties, "Properties should be added for tool: "+toolName) + } +} diff --git a/internal/server/schema_normalization_test.go b/internal/server/schema_normalization_test.go new file mode 100644 index 00000000..78216457 --- /dev/null +++ b/internal/server/schema_normalization_test.go @@ -0,0 +1,153 @@ +package server + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/githubnext/gh-aw-mcpg/internal/config" + "github.com/githubnext/gh-aw-mcpg/internal/mcp" +) + +// TestSchemaNormalization_Integration tests that broken schemas from backends +// are automatically normalized when tools are registered. +func TestSchemaNormalization_Integration(t *testing.T) { + ctx := context.Background() + + // Create a minimal config + cfg := &config.Config{ + Servers: map[string]*config.ServerConfig{ + "test": { + Command: "echo", // Dummy command + Args: []string{}, + }, + }, + } + + // Create unified server + us, err := NewUnified(ctx, cfg) + require.NoError(t, err, "Failed to create unified server") + defer us.Close() + + testCases := []struct { + name string + toolName string + inputSchema map[string]interface{} + expectedSchema map[string]interface{} + }{ + { + name: "broken object schema without properties", + toolName: "get_commit", + inputSchema: map[string]interface{}{ + "type": "object", + }, + expectedSchema: map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{}, + }, + }, + { + name: "valid object schema with properties", + toolName: "issue_read", + inputSchema: map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "owner": map[string]interface{}{ + "type": "string", + }, + "repo": map[string]interface{}{ + "type": "string", + }, + }, + "required": []interface{}{"owner", "repo"}, + }, + expectedSchema: map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "owner": map[string]interface{}{ + "type": "string", + }, + "repo": map[string]interface{}{ + "type": "string", + }, + }, + "required": []interface{}{"owner", "repo"}, + }, + }, + { + name: "object schema with additionalProperties", + toolName: "list_items", + inputSchema: map[string]interface{}{ + "type": "object", + "additionalProperties": true, + }, + expectedSchema: map[string]interface{}{ + "type": "object", + "additionalProperties": true, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Simulate registering a tool with the given schema + prefixedName := "test___" + tc.toolName + + // Use the NormalizeInputSchema function directly + normalized := mcp.NormalizeInputSchema(tc.inputSchema, prefixedName) + + // Store the normalized tool + us.toolsMu.Lock() + us.tools[prefixedName] = &ToolInfo{ + Name: prefixedName, + Description: "Test tool", + BackendID: "test", + InputSchema: normalized, + } + us.toolsMu.Unlock() + + // Retrieve the tool and verify the schema + us.toolsMu.RLock() + tool, exists := us.tools[prefixedName] + us.toolsMu.RUnlock() + + require.True(t, exists, "Tool should exist") + assert.Equal(t, tc.expectedSchema, tool.InputSchema, "Schema should match expected normalized version") + + // Clean up + us.toolsMu.Lock() + delete(us.tools, prefixedName) + us.toolsMu.Unlock() + }) + } +} + +// TestSchemaNormalization_PreservesOriginal verifies that the normalization +// doesn't modify the original schema object +func TestSchemaNormalization_PreservesOriginal(t *testing.T) { + original := map[string]interface{}{ + "type": "object", + } + + // Make a copy to compare later + originalCopy := make(map[string]interface{}) + for k, v := range original { + originalCopy[k] = v + } + + // Normalize the schema + normalized := mcp.NormalizeInputSchema(original, "test-tool") + + // Verify original is unchanged + assert.Equal(t, originalCopy, original, "Original schema should not be modified") + + // Verify normalized has properties + _, hasProperties := normalized["properties"] + assert.True(t, hasProperties, "Normalized schema should have properties") + + // Verify original still doesn't have properties + _, originalHasProperties := original["properties"] + assert.False(t, originalHasProperties, "Original schema should not have properties") +} diff --git a/internal/server/unified.go b/internal/server/unified.go index 1faa9a01..a018a8f0 100644 --- a/internal/server/unified.go +++ b/internal/server/unified.go @@ -209,12 +209,15 @@ func (us *UnifiedServer) registerToolsFromBackend(serverID string) error { toolDesc := fmt.Sprintf("[%s] %s", serverID, tool.Description) toolNames = append(toolNames, prefixedName) + // Normalize the input schema to fix common validation issues + normalizedSchema := mcp.NormalizeInputSchema(tool.InputSchema, prefixedName) + // Store tool info for routed mode us.toolsMu.Lock() us.tools[prefixedName] = &ToolInfo{ Name: prefixedName, Description: toolDesc, - InputSchema: tool.InputSchema, + InputSchema: normalizedSchema, BackendID: serverID, } us.toolsMu.Unlock() From ea25ea8fe252c685f02fd9386d5c4f5728e603ec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 16 Jan 2026 05:16:50 +0000 Subject: [PATCH 3/6] Format code with gofmt Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- internal/mcp/schema_normalizer.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/mcp/schema_normalizer.go b/internal/mcp/schema_normalizer.go index 58f88bca..6850bfb7 100644 --- a/internal/mcp/schema_normalizer.go +++ b/internal/mcp/schema_normalizer.go @@ -12,9 +12,9 @@ var logSchemaNormalizer = logger.New("mcp:schema_normalizer") // that can cause downstream validation errors. // // Known issues fixed: -// 1. Object schemas without properties: When a schema declares "type": "object" -// but is missing the required "properties" field, we add an empty properties -// object to make it valid per JSON Schema standards. +// 1. Object schemas without properties: When a schema declares "type": "object" +// but is missing the required "properties" field, we add an empty properties +// object to make it valid per JSON Schema standards. func NormalizeInputSchema(schema map[string]interface{}, toolName string) map[string]interface{} { if schema == nil { return schema From 91581a88d4eb6608f9ccc948adfa84bb2e6da3a2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 16 Jan 2026 05:18:16 +0000 Subject: [PATCH 4/6] Document schema normalization feature in README Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index c1495036..39e149ee 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,10 @@ This gateway is used with [GitHub Agentic Workflows](https://github.com/githubne - **Spec-Compliant Validation**: Fail-fast validation with detailed error messages - **Variable Expansion**: Environment variable substitution with `${VAR_NAME}` syntax - **Type Normalization**: Automatic conversion of legacy `"local"` type to `"stdio"` +- **Schema Normalization**: Automatic fixing of malformed JSON schemas from backend MCP servers + - Adds missing `properties` field to object schemas + - Prevents downstream validation errors + - Transparent to both backends and clients - **Routing Modes**: - **Routed**: Each backend server accessible at `/mcp/{serverID}` - **Unified**: Single endpoint `/mcp` that routes to configured servers From 31e3b0749a1c1bcf2d82546492ca42ee8446cbee Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 16 Jan 2026 05:20:03 +0000 Subject: [PATCH 5/6] Remove redundant logging statements Consolidated three logging statements into a single structured logger call for consistency with the codebase. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- internal/mcp/schema_normalizer.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/internal/mcp/schema_normalizer.go b/internal/mcp/schema_normalizer.go index 6850bfb7..be72440a 100644 --- a/internal/mcp/schema_normalizer.go +++ b/internal/mcp/schema_normalizer.go @@ -1,13 +1,9 @@ package mcp import ( - "log" - "github.com/githubnext/gh-aw-mcpg/internal/logger" ) -var logSchemaNormalizer = logger.New("mcp:schema_normalizer") - // NormalizeInputSchema fixes common schema validation issues in tool definitions // that can cause downstream validation errors. // @@ -38,8 +34,6 @@ func NormalizeInputSchema(schema map[string]interface{}, toolName string) map[st // If it's an object type but missing both properties and additionalProperties, // add an empty properties object to make it valid if !hasProperties && !hasAdditionalProperties { - log.Printf("Schema normalization: Adding empty properties to object schema for tool '%s'", toolName) - logSchemaNormalizer.Printf("Normalizing schema for tool %s: adding empty properties to object type", toolName) logger.LogWarn("backend", "Tool schema normalized: %s - added empty properties to object type schema", toolName) // Create a copy of the schema to avoid modifying the original From e91805c1e13c141899afb919c897b233545b3840 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 16 Jan 2026 05:21:18 +0000 Subject: [PATCH 6/6] Fix type inconsistency in test for required field Changed required field from []string to []interface{} for consistency with JSON schema handling. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- internal/mcp/schema_normalizer_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mcp/schema_normalizer_test.go b/internal/mcp/schema_normalizer_test.go index af6a0d49..434aee09 100644 --- a/internal/mcp/schema_normalizer_test.go +++ b/internal/mcp/schema_normalizer_test.go @@ -71,7 +71,7 @@ func TestNormalizeInputSchema_ObjectWithProperties(t *testing.T) { "type": "number", }, }, - "required": []string{"name"}, + "required": []interface{}{"name"}, } result := NormalizeInputSchema(schema, "test-tool")