diff --git a/internal/mcp/schema_normalizer.go b/internal/mcp/schema_normalizer.go index be72440a..261914fb 100644 --- a/internal/mcp/schema_normalizer.go +++ b/internal/mcp/schema_normalizer.go @@ -8,12 +8,21 @@ import ( // that can cause downstream validation errors. // // Known issues fixed: -// 1. Object schemas without properties: When a schema declares "type": "object" +// 1. Missing schema: When a backend returns no inputSchema (nil), we provide +// a default empty object schema that accepts any properties. This is required +// by the MCP SDK's Server.AddTool method. +// 2. 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 backend didn't provide a schema, use a default empty object schema + // This allows the tool to be registered and clients will see it accepts any parameters if schema == nil { - return schema + logger.LogWarn("backend", "Tool schema normalized: %s - backend provided no inputSchema, using default empty object schema", toolName) + return map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{}, + } } // Check if this is an object type schema diff --git a/internal/mcp/schema_normalizer_test.go b/internal/mcp/schema_normalizer_test.go index 434aee09..4b636e1e 100644 --- a/internal/mcp/schema_normalizer_test.go +++ b/internal/mcp/schema_normalizer_test.go @@ -9,7 +9,14 @@ import ( func TestNormalizeInputSchema_NilSchema(t *testing.T) { result := NormalizeInputSchema(nil, "test-tool") - assert.Nil(t, result, "Nil schema should return nil") + // When backend provides no schema, we return a default empty object schema + // This is required by the SDK's Server.AddTool method and allows clients + // to see that the tool accepts parameters (though any are allowed) + require.NotNil(t, result, "Nil schema should return default empty object schema") + assert.Equal(t, "object", result["type"], "Default schema should have type 'object'") + assert.Contains(t, result, "properties", "Default schema should have properties field") + properties := result["properties"].(map[string]interface{}) + assert.Empty(t, properties, "Default schema properties should be empty") } func TestNormalizeInputSchema_EmptySchema(t *testing.T) { diff --git a/internal/server/routed.go b/internal/server/routed.go index de5f5159..282a8528 100644 --- a/internal/server/routed.go +++ b/internal/server/routed.go @@ -211,18 +211,22 @@ func createFilteredServer(unifiedServer *UnifiedServer, backendID string) *sdk.S continue } - // Note: InputSchema is intentionally omitted to avoid validation errors - // when backend MCP servers use different JSON Schema versions (e.g., draft-07) - // than what the SDK supports (draft-2020-12) - sdk.AddTool(server, &sdk.Tool{ - Name: toolInfo.Name, // Without prefix for the client - Description: toolInfo.Description, - }, func(ctx context.Context, req *sdk.CallToolRequest, args interface{}) (*sdk.CallToolResult, interface{}, error) { + // Use Server.AddTool method (not sdk.AddTool function) to avoid schema validation + // This allows including InputSchema from backends using different JSON Schema versions + // Wrap the typed handler to match the simple ToolHandler signature + wrappedHandler := func(ctx context.Context, req *sdk.CallToolRequest) (*sdk.CallToolResult, error) { // Call the unified server's handler directly // This ensures we go through the same session and connection pool log.Printf("[ROUTED] Calling unified handler for: %s", toolNameCopy) - return handler(ctx, req, args) - }) + result, _, err := handler(ctx, req, nil) + return result, err + } + + server.AddTool(&sdk.Tool{ + Name: toolInfo.Name, // Without prefix for the client + Description: toolInfo.Description, + InputSchema: toolInfo.InputSchema, // Include schema for clients + }, wrappedHandler) } return server diff --git a/internal/server/tools_list_schema_test.go b/internal/server/tools_list_schema_test.go new file mode 100644 index 00000000..152defe3 --- /dev/null +++ b/internal/server/tools_list_schema_test.go @@ -0,0 +1,139 @@ +package server + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "encoding/json" + "github.com/githubnext/gh-aw-mcpg/internal/config" + "io" + "net/http" + "net/http/httptest" +) + +// TestToolsListIncludesInputSchema verifies that tools/list responses include +// inputSchema for all tools, which is required for clients to understand +// the parameter structure. +func TestToolsListIncludesInputSchema(t *testing.T) { + // Create a mock backend that returns a tool with inputSchema + mockBackend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + bodyBytes, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "Internal error", http.StatusInternalServerError) + return + } + + var request map[string]interface{} + if err := json.Unmarshal(bodyBytes, &request); err != nil { + http.Error(w, "Bad request", http.StatusBadRequest) + return + } + + method, _ := request["method"].(string) + requestID := request["id"] + + if method == "initialize" { + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Mcp-Session-Id", "backend-session-123") + json.NewEncoder(w).Encode(map[string]interface{}{ + "jsonrpc": "2.0", + "id": requestID, + "result": map[string]interface{}{ + "protocolVersion": "2024-11-05", + "capabilities": map[string]interface{}{}, + "serverInfo": map[string]interface{}{ + "name": "test-backend", + "version": "1.0.0", + }, + }, + }) + return + } + + if method == "tools/list" { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]interface{}{ + "jsonrpc": "2.0", + "id": requestID, + "result": map[string]interface{}{ + "tools": []map[string]interface{}{ + { + "name": "test_tool", + "description": "A test tool", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "body": map[string]interface{}{ + "type": "string", + "description": "The body parameter", + }, + }, + "required": []string{"body"}, + }, + }, + }, + }, + }) + return + } + + http.Error(w, "Unknown method", http.StatusBadRequest) + })) + defer mockBackend.Close() + + // Create gateway configuration with the mock backend + cfg := &config.Config{ + Servers: map[string]*config.ServerConfig{ + "testserver": { + Type: "http", + URL: mockBackend.URL, + Headers: map[string]string{ + "Authorization": "test-auth", + }, + }, + }, + } + + ctx := context.Background() + us, err := NewUnified(ctx, cfg) + require.NoError(t, err, "Failed to create unified server") + defer us.Close() + + // Check that tools registered in the UnifiedServer have InputSchema + us.toolsMu.RLock() + tools := us.tools + us.toolsMu.RUnlock() + + require.NotEmpty(t, tools, "Should have registered tools") + + // Find our test tool + var testTool *ToolInfo + for name, tool := range tools { + if tool.BackendID == "testserver" { + testTool = tool + t.Logf("Found tool: %s", name) + break + } + } + + require.NotNil(t, testTool, "Should have found test tool") + + // Verify the tool has InputSchema + assert.NotNil(t, testTool.InputSchema, "Tool MUST have InputSchema") + assert.NotEmpty(t, testTool.InputSchema, "InputSchema should not be empty") + + // Verify the schema structure + assert.Equal(t, "object", testTool.InputSchema["type"], "InputSchema should have type: object") + assert.Contains(t, testTool.InputSchema, "properties", "InputSchema should have properties") + + propertiesValue := testTool.InputSchema["properties"] + require.NotNil(t, propertiesValue, "properties value should not be nil") + properties, ok := propertiesValue.(map[string]interface{}) + require.True(t, ok, "properties should be a map[string]interface{}") + assert.Contains(t, properties, "body", "InputSchema should define the 'body' parameter") + + t.Logf("✓ Tool has proper InputSchema: %+v", testTool.InputSchema) +} diff --git a/internal/server/unified.go b/internal/server/unified.go index 6e65833c..33a8c56b 100644 --- a/internal/server/unified.go +++ b/internal/server/unified.go @@ -274,14 +274,27 @@ func (us *UnifiedServer) registerToolsFromBackend(serverID string) error { us.tools[prefixedName].Handler = handler us.toolsMu.Unlock() - // Register the tool with the SDK - // Note: InputSchema is intentionally omitted to avoid validation errors - // when backend MCP servers use different JSON Schema versions (e.g., draft-07) - // than what the SDK supports (draft-2020-12) - sdk.AddTool(us.server, &sdk.Tool{ + // Register the tool with the SDK using the Server.AddTool method (not sdk.AddTool function) + // The method version does NOT perform schema validation, allowing us to include + // InputSchema from backends that use different JSON Schema versions (e.g., draft-07) + // without validation errors. This is critical for clients to understand tool parameters. + // + // We need to wrap our typed handler to match the simpler ToolHandler signature. + // The typed handler signature: func(context.Context, *CallToolRequest, interface{}) (*CallToolResult, interface{}, error) + // The simple handler signature: func(context.Context, *CallToolRequest) (*CallToolResult, error) + wrappedHandler := func(ctx context.Context, req *sdk.CallToolRequest) (*sdk.CallToolResult, error) { + // Call the original typed handler + // The third parameter would be the pre-unmarshaled/validated input if using sdk.AddTool, + // but we handle unmarshaling ourselves in the handler, so we pass nil + result, _, err := handler(ctx, req, nil) + return result, err + } + + us.server.AddTool(&sdk.Tool{ Name: prefixedName, Description: toolDesc, - }, handler) + InputSchema: normalizedSchema, // Include the schema for clients to understand parameters + }, wrappedHandler) log.Printf("Registered tool: %s", prefixedName) }