Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 50 additions & 0 deletions internal/mcp/schema_normalizer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package mcp

import (
"github.com/githubnext/gh-aw-mcpg/internal/logger"
)

// 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 {
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
}
195 changes: 195 additions & 0 deletions internal/mcp/schema_normalizer_test.go
Original file line number Diff line number Diff line change
@@ -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": []interface{}{"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)
}
}
Loading