Fix: Convert form parameters to schema-defined types before MCP tool calls (#397)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: samanhappy <2755122+samanhappy@users.noreply.github.com>

src/controllers/openApiController.ts

@@ -8,82 +8,13 @@ import {
 import { getServerByName } from '../services/mcpService.js';
 import { getGroupByIdOrName } from '../services/groupService.js';
 import { getNameSeparator } from '../config/index.js';
+import { convertParametersToTypes } from '../utils/parameterConversion.js';
 
 /**
  * Controller for OpenAPI generation endpoints
  * Provides OpenAPI specifications for MCP tools to enable OpenWebUI integration
  */
 
-/**
- * Convert query parameters to their proper types based on the tool's input schema
- */
-function convertQueryParametersToTypes(
-  queryParams: Record<string, any>,
-  inputSchema: Record<string, any>,
-): Record<string, any> {
-  if (!inputSchema || typeof inputSchema !== 'object' || !inputSchema.properties) {
-    return queryParams;
-  }
-
-  const convertedParams: Record<string, any> = {};
-  const properties = inputSchema.properties;
-
-  for (const [key, value] of Object.entries(queryParams)) {
-    const propDef = properties[key];
-    if (!propDef || typeof propDef !== 'object') {
-      // No schema definition found, keep as is
-      convertedParams[key] = value;
-      continue;
-    }
-
-    const propType = propDef.type;
-
-    try {
-      switch (propType) {
-        case 'integer':
-        case 'number':
-          // Convert string to number
-          if (typeof value === 'string') {
-            const numValue = propType === 'integer' ? parseInt(value, 10) : parseFloat(value);
-            convertedParams[key] = isNaN(numValue) ? value : numValue;
-          } else {
-            convertedParams[key] = value;
-          }
-          break;
-
-        case 'boolean':
-          // Convert string to boolean
-          if (typeof value === 'string') {
-            convertedParams[key] = value.toLowerCase() === 'true' || value === '1';
-          } else {
-            convertedParams[key] = value;
-          }
-          break;
-
-        case 'array':
-          // Handle array conversion if needed (e.g., comma-separated strings)
-          if (typeof value === 'string' && value.includes(',')) {
-            convertedParams[key] = value.split(',').map((item) => item.trim());
-          } else {
-            convertedParams[key] = value;
-          }
-          break;
-
-        default:
-          // For string and other types, keep as is
-          convertedParams[key] = value;
-          break;
-      }
-    } catch (error) {
-      // If conversion fails, keep the original value
-      console.warn(`Failed to convert parameter '${key}' to type '${propType}':`, error);
-      convertedParams[key] = value;
-    }
-  }
-
-  return convertedParams;
-}
-
 /**
  * Generate and return OpenAPI specification
  * GET /api/openapi.json
@@ -191,7 +122,7 @@ export const executeToolViaOpenAPI = async (req: Request, res: Response): Promis
 
     // Prepare arguments from query params (GET) or body (POST)
     let args = req.method === 'GET' ? req.query : req.body || {};
-    args = convertQueryParametersToTypes(args, inputSchema);
+    args = convertParametersToTypes(args, inputSchema);
 
     // Create a mock request structure that matches what handleCallToolRequest expects
     const mockRequest = {

src/controllers/toolController.ts

@@ -1,6 +1,8 @@
 import { Request, Response } from 'express';
 import { ApiResponse } from '../types/index.js';
-import { handleCallToolRequest } from '../services/mcpService.js';
+import { handleCallToolRequest, getServerByName } from '../services/mcpService.js';
+import { convertParametersToTypes } from '../utils/parameterConversion.js';
+import { getNameSeparator } from '../config/index.js';
 
 /**
  * Interface for tool call request
@@ -47,13 +49,31 @@ export const callTool = async (req: Request, res: Response): Promise<void> => {
       return;
     }
 
+    // Get the server info to access the tool's input schema
+    const serverInfo = getServerByName(server);
+    let inputSchema: Record<string, any> = {};
+
+    if (serverInfo) {
+      // Find the tool in the server's tools list
+      const fullToolName = `${server}${getNameSeparator()}${toolName}`;
+      const tool = serverInfo.tools.find(
+        (t: any) => t.name === fullToolName || t.name === toolName,
+      );
+      if (tool && tool.inputSchema) {
+        inputSchema = tool.inputSchema as Record<string, any>;
+      }
+    }
+
+    // Convert parameters to proper types based on the tool's input schema
+    const convertedArgs = convertParametersToTypes(toolArgs, inputSchema);
+
     // Create a mock request structure for handleCallToolRequest
     const mockRequest = {
       params: {
         name: 'call_tool',
         arguments: {
           toolName,
-          arguments: toolArgs,
+          arguments: convertedArgs,
         },
       },
     };
@@ -71,7 +91,7 @@ export const callTool = async (req: Request, res: Response): Promise<void> => {
       data: {
         content: result.content || [],
         toolName,
-        arguments: toolArgs,
+        arguments: convertedArgs,
       },
     };
 

src/utils/parameterConversion.ts

@@ -0,0 +1,93 @@
+/**
+ * Utility functions for converting parameter types based on JSON schema definitions
+ */
+
+/**
+ * Convert parameters to their proper types based on the tool's input schema
+ * This ensures that form-submitted string values are converted to the correct types
+ * (e.g., numbers, booleans, arrays) before being passed to MCP tools.
+ *
+ * @param params - The parameters to convert (typically from form submission)
+ * @param inputSchema - The JSON schema definition for the tool's input
+ * @returns The converted parameters with proper types
+ */
+export function convertParametersToTypes(
+  params: Record<string, any>,
+  inputSchema: Record<string, any>,
+): Record<string, any> {
+  if (!inputSchema || typeof inputSchema !== 'object' || !inputSchema.properties) {
+    return params;
+  }
+
+  const convertedParams: Record<string, any> = {};
+  const properties = inputSchema.properties;
+
+  for (const [key, value] of Object.entries(params)) {
+    const propDef = properties[key];
+    if (!propDef || typeof propDef !== 'object') {
+      // No schema definition found, keep as is
+      convertedParams[key] = value;
+      continue;
+    }
+
+    const propType = propDef.type;
+
+    try {
+      switch (propType) {
+        case 'integer':
+        case 'number':
+          // Convert string to number
+          if (typeof value === 'string') {
+            const numValue = propType === 'integer' ? parseInt(value, 10) : parseFloat(value);
+            convertedParams[key] = isNaN(numValue) ? value : numValue;
+          } else {
+            convertedParams[key] = value;
+          }
+          break;
+
+        case 'boolean':
+          // Convert string to boolean
+          if (typeof value === 'string') {
+            convertedParams[key] = value.toLowerCase() === 'true' || value === '1';
+          } else {
+            convertedParams[key] = value;
+          }
+          break;
+
+        case 'array':
+          // Handle array conversion if needed (e.g., comma-separated strings)
+          if (typeof value === 'string' && value.includes(',')) {
+            convertedParams[key] = value.split(',').map((item) => item.trim());
+          } else {
+            convertedParams[key] = value;
+          }
+          break;
+
+        case 'object':
+          // Handle object conversion if needed
+          if (typeof value === 'string') {
+            try {
+              convertedParams[key] = JSON.parse(value);
+            } catch {
+              // If parsing fails, keep as is
+              convertedParams[key] = value;
+            }
+          } else {
+            convertedParams[key] = value;
+          }
+          break;
+
+        default:
+          // For string and other types, keep as is
+          convertedParams[key] = value;
+          break;
+      }
+    } catch (error) {
+      // If conversion fails, keep the original value
+      console.warn(`Failed to convert parameter '${key}' to type '${propType}':`, error);
+      convertedParams[key] = value;
+    }
+  }
+
+  return convertedParams;
+}

tests/controllers/openApiController.test.ts

@@ -1,73 +1,7 @@
-// Simple unit test to validate the type conversion logic
-describe('Parameter Type Conversion Logic', () => {
-  // Extract the conversion function for testing
-  function convertQueryParametersToTypes(
-    queryParams: Record<string, any>,
-    inputSchema: Record<string, any>
-  ): Record<string, any> {
-    if (!inputSchema || typeof inputSchema !== 'object' || !inputSchema.properties) {
-      return queryParams;
-    }
-
-    const convertedParams: Record<string, any> = {};
-    const properties = inputSchema.properties;
-
-    for (const [key, value] of Object.entries(queryParams)) {
-      const propDef = properties[key];
-      if (!propDef || typeof propDef !== 'object') {
-        // No schema definition found, keep as is
-        convertedParams[key] = value;
-        continue;
-      }
-
-      const propType = propDef.type;
-      
-      try {
-        switch (propType) {
-          case 'integer':
-          case 'number':
-            // Convert string to number
-            if (typeof value === 'string') {
-              const numValue = propType === 'integer' ? parseInt(value, 10) : parseFloat(value);
-              convertedParams[key] = isNaN(numValue) ? value : numValue;
-            } else {
-              convertedParams[key] = value;
-            }
-            break;
-          
-          case 'boolean':
-            // Convert string to boolean
-            if (typeof value === 'string') {
-              convertedParams[key] = value.toLowerCase() === 'true' || value === '1';
-            } else {
-              convertedParams[key] = value;
-            }
-            break;
-          
-          case 'array':
-            // Handle array conversion if needed (e.g., comma-separated strings)
-            if (typeof value === 'string' && value.includes(',')) {
-              convertedParams[key] = value.split(',').map(item => item.trim());
-            } else {
-              convertedParams[key] = value;
-            }
-            break;
-          
-          default:
-            // For string and other types, keep as is
-            convertedParams[key] = value;
-            break;
-        }
-      } catch (error) {
-        // If conversion fails, keep the original value
-        console.warn(`Failed to convert parameter '${key}' to type '${propType}':`, error);
-        convertedParams[key] = value;
-      }
-    }
-
-    return convertedParams;
-  }
+import { convertParametersToTypes } from '../../src/utils/parameterConversion.js';
 
+// Integration tests for OpenAPI controller's parameter type conversion
+describe('OpenAPI Controller - Parameter Type Conversion Integration', () => {
   test('should convert integer parameters correctly', () => {
     const queryParams = {
       limit: '5',
@@ -84,7 +18,7 @@ describe('Parameter Type Conversion Logic', () => {
       }
     };
 
-    const result = convertQueryParametersToTypes(queryParams, inputSchema);
+    const result = convertParametersToTypes(queryParams, inputSchema);
 
     expect(result).toEqual({
       limit: 5,       // Converted to integer
@@ -107,7 +41,7 @@ describe('Parameter Type Conversion Logic', () => {
       }
     };
 
-    const result = convertQueryParametersToTypes(queryParams, inputSchema);
+    const result = convertParametersToTypes(queryParams, inputSchema);
 
     expect(result).toEqual({
       price: 19.99,
@@ -133,7 +67,7 @@ describe('Parameter Type Conversion Logic', () => {
       }
     };
 
-    const result = convertQueryParametersToTypes(queryParams, inputSchema);
+    const result = convertParametersToTypes(queryParams, inputSchema);
 
     expect(result).toEqual({
       enabled: true,
@@ -157,7 +91,7 @@ describe('Parameter Type Conversion Logic', () => {
       }
     };
 
-    const result = convertQueryParametersToTypes(queryParams, inputSchema);
+    const result = convertParametersToTypes(queryParams, inputSchema);
 
     expect(result).toEqual({
       tags: ['tag1', 'tag2', 'tag3'],
@@ -171,7 +105,7 @@ describe('Parameter Type Conversion Logic', () => {
       name: 'test'
     };
 
-    const result = convertQueryParametersToTypes(queryParams, {});
+    const result = convertParametersToTypes(queryParams, {});
 
     expect(result).toEqual({
       limit: '5',     // Should remain as string
@@ -192,7 +126,7 @@ describe('Parameter Type Conversion Logic', () => {
       }
     };
 
-    const result = convertQueryParametersToTypes(queryParams, inputSchema);
+    const result = convertParametersToTypes(queryParams, inputSchema);
 
     expect(result).toEqual({
       limit: 5,                     // Converted based on schema
@@ -214,7 +148,7 @@ describe('Parameter Type Conversion Logic', () => {
       }
     };
 
-    const result = convertQueryParametersToTypes(queryParams, inputSchema);
+    const result = convertParametersToTypes(queryParams, inputSchema);
 
     expect(result).toEqual({
       limit: 'not-a-number',  // Should remain as string when conversion fails

tests/utils/parameterConversion.test.ts

@@ -0,0 +1,259 @@
+import { convertParametersToTypes } from '../../src/utils/parameterConversion.js';
+
+describe('Parameter Conversion Utilities', () => {
+  describe('convertParametersToTypes', () => {
+    it('should convert string to number when schema type is number', () => {
+      const params = { count: '42' };
+      const schema = {
+        type: 'object',
+        properties: {
+          count: { type: 'number' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(result.count).toBe(42);
+      expect(typeof result.count).toBe('number');
+    });
+
+    it('should convert string to integer when schema type is integer', () => {
+      const params = { age: '25' };
+      const schema = {
+        type: 'object',
+        properties: {
+          age: { type: 'integer' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(result.age).toBe(25);
+      expect(typeof result.age).toBe('number');
+      expect(Number.isInteger(result.age)).toBe(true);
+    });
+
+    it('should convert string to boolean when schema type is boolean', () => {
+      const params = { enabled: 'true', disabled: 'false', flag: '1' };
+      const schema = {
+        type: 'object',
+        properties: {
+          enabled: { type: 'boolean' },
+          disabled: { type: 'boolean' },
+          flag: { type: 'boolean' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(result.enabled).toBe(true);
+      expect(result.disabled).toBe(false);
+      expect(result.flag).toBe(true);
+    });
+
+    it('should convert comma-separated string to array when schema type is array', () => {
+      const params = { tags: 'one,two,three' };
+      const schema = {
+        type: 'object',
+        properties: {
+          tags: { type: 'array' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(Array.isArray(result.tags)).toBe(true);
+      expect(result.tags).toEqual(['one', 'two', 'three']);
+    });
+
+    it('should parse JSON string to object when schema type is object', () => {
+      const params = { config: '{"key": "value", "nested": {"prop": 123}}' };
+      const schema = {
+        type: 'object',
+        properties: {
+          config: { type: 'object' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(typeof result.config).toBe('object');
+      expect(result.config).toEqual({ key: 'value', nested: { prop: 123 } });
+    });
+
+    it('should keep values unchanged when they already have the correct type', () => {
+      const params = { count: 42, enabled: true, tags: ['a', 'b'] };
+      const schema = {
+        type: 'object',
+        properties: {
+          count: { type: 'number' },
+          enabled: { type: 'boolean' },
+          tags: { type: 'array' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(result.count).toBe(42);
+      expect(result.enabled).toBe(true);
+      expect(result.tags).toEqual(['a', 'b']);
+    });
+
+    it('should keep string values unchanged when schema type is string', () => {
+      const params = { name: 'John Doe' };
+      const schema = {
+        type: 'object',
+        properties: {
+          name: { type: 'string' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(result.name).toBe('John Doe');
+      expect(typeof result.name).toBe('string');
+    });
+
+    it('should handle parameters without schema definition', () => {
+      const params = { unknown: 'value' };
+      const schema = {
+        type: 'object',
+        properties: {
+          known: { type: 'string' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(result.unknown).toBe('value');
+    });
+
+    it('should return original params when schema has no properties', () => {
+      const params = { key: 'value' };
+      const schema = { type: 'object' };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(result).toEqual(params);
+    });
+
+    it('should return original params when schema is null or undefined', () => {
+      const params = { key: 'value' };
+
+      const resultNull = convertParametersToTypes(params, null as any);
+      const resultUndefined = convertParametersToTypes(params, undefined as any);
+
+      expect(resultNull).toEqual(params);
+      expect(resultUndefined).toEqual(params);
+    });
+
+    it('should handle invalid number conversion gracefully', () => {
+      const params = { count: 'not-a-number' };
+      const schema = {
+        type: 'object',
+        properties: {
+          count: { type: 'number' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      // When conversion fails, it should keep original value
+      expect(result.count).toBe('not-a-number');
+    });
+
+    it('should handle invalid JSON string for object gracefully', () => {
+      const params = { config: '{invalid json}' };
+      const schema = {
+        type: 'object',
+        properties: {
+          config: { type: 'object' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      // When JSON parsing fails, it should keep original value
+      expect(result.config).toBe('{invalid json}');
+    });
+
+    it('should handle mixed parameter types correctly', () => {
+      const params = {
+        name: 'Test',
+        count: '10',
+        price: '19.99',
+        enabled: 'true',
+        tags: 'tag1,tag2',
+        config: '{"nested": true}',
+      };
+      const schema = {
+        type: 'object',
+        properties: {
+          name: { type: 'string' },
+          count: { type: 'integer' },
+          price: { type: 'number' },
+          enabled: { type: 'boolean' },
+          tags: { type: 'array' },
+          config: { type: 'object' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(result.name).toBe('Test');
+      expect(result.count).toBe(10);
+      expect(result.price).toBe(19.99);
+      expect(result.enabled).toBe(true);
+      expect(result.tags).toEqual(['tag1', 'tag2']);
+      expect(result.config).toEqual({ nested: true });
+    });
+
+    it('should handle empty string values', () => {
+      const params = { name: '', count: '', enabled: '' };
+      const schema = {
+        type: 'object',
+        properties: {
+          name: { type: 'string' },
+          count: { type: 'number' },
+          enabled: { type: 'boolean' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(result.name).toBe('');
+      // Empty string should remain as empty string for number (NaN check keeps original)
+      expect(result.count).toBe('');
+      // Empty string converts to false for boolean
+      expect(result.enabled).toBe(false);
+    });
+
+    it('should handle array that is already an array', () => {
+      const params = { tags: ['existing', 'array'] };
+      const schema = {
+        type: 'object',
+        properties: {
+          tags: { type: 'array' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(result.tags).toEqual(['existing', 'array']);
+    });
+
+    it('should handle object that is already an object', () => {
+      const params = { config: { key: 'value' } };
+      const schema = {
+        type: 'object',
+        properties: {
+          config: { type: 'object' },
+        },
+      };
+
+      const result = convertParametersToTypes(params, schema);
+
+      expect(result.config).toEqual({ key: 'value' });
+    });
+  });
+});