From 02c1792015baf4ed0e0f23c7f78e4373fa316959 Mon Sep 17 00:00:00 2001
From: yuchou87 <yuchou87@gmail.com>
Date: Mon, 16 Feb 2026 19:50:00 +0800
Subject: [PATCH] fix(tools): preserve MCP tool InputSchema via JSON
 marshal/unmarshal

- Handle json.RawMessage and []byte types by direct unmarshal
- Use JSON marshal/unmarshal for struct types to preserve schema
- Add test case for json.RawMessage schema
- Fixes issue where non-map schemas returned empty object

This fixes GitHub Copilot feedback that Parameters() was dropping
tool schema when InputSchema wasn't already map[string]interface{}
---
 pkg/tools/mcp_tool.go      | 72 ++++++++++++++++++++++++++++----------
 pkg/tools/mcp_tool_test.go | 55 +++++++++++++++++++++++++----
 2 files changed, 102 insertions(+), 25 deletions(-)

diff --git a/pkg/tools/mcp_tool.go b/pkg/tools/mcp_tool.go
index adc06ec23..c29ab8525 100644
--- a/pkg/tools/mcp_tool.go
+++ b/pkg/tools/mcp_tool.go
@@ -2,6 +2,7 @@ package tools
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"strings"
 
@@ -51,26 +52,61 @@ func (t *MCPTool) Parameters() map[string]interface{} {
 	// The InputSchema is already a JSON Schema object
 	schema := t.tool.InputSchema
 
-	// Convert to map[string]interface{} for compatibility
-	result := make(map[string]interface{})
-
-	// Use reflection to convert the schema
-	// The schema should already be in the correct format
-	if schema != nil {
-		// Attempt to convert directly
-		if schemaMap, ok := schema.(map[string]interface{}); ok {
-			return schemaMap
+	// Handle nil schema
+	if schema == nil {
+		return map[string]interface{}{
+			"type":       "object",
+			"properties": map[string]interface{}{},
+			"required":   []string{},
 		}
+	}
 
-		// Otherwise, build it manually
-		result["type"] = "object"
-		result["properties"] = map[string]interface{}{}
-		result["required"] = []string{}
-	} else {
-		// Default schema when nil
-		result["type"] = "object"
-		result["properties"] = map[string]interface{}{}
-		result["required"] = []string{}
+	// Try direct conversion first (fast path)
+	if schemaMap, ok := schema.(map[string]interface{}); ok {
+		return schemaMap
+	}
+
+	// Handle json.RawMessage and []byte - unmarshal directly
+	var jsonData []byte
+	if rawMsg, ok := schema.(json.RawMessage); ok {
+		jsonData = rawMsg
+	} else if bytes, ok := schema.([]byte); ok {
+		jsonData = bytes
+	}
+
+	if jsonData != nil {
+		var result map[string]interface{}
+		if err := json.Unmarshal(jsonData, &result); err == nil {
+			return result
+		}
+		// Fallback on error
+		return map[string]interface{}{
+			"type":       "object",
+			"properties": map[string]interface{}{},
+			"required":   []string{},
+		}
+	}
+
+	// For other types (structs, etc.), convert via JSON marshal/unmarshal
+	var err error
+	jsonData, err = json.Marshal(schema)
+	if err != nil {
+		// Fallback to empty schema if marshaling fails
+		return map[string]interface{}{
+			"type":       "object",
+			"properties": map[string]interface{}{},
+			"required":   []string{},
+		}
+	}
+
+	var result map[string]interface{}
+	if err := json.Unmarshal(jsonData, &result); err != nil {
+		// Fallback to empty schema if unmarshaling fails
+		return map[string]interface{}{
+			"type":       "object",
+			"properties": map[string]interface{}{},
+			"required":   []string{},
+		}
 	}
 
 	return result
diff --git a/pkg/tools/mcp_tool_test.go b/pkg/tools/mcp_tool_test.go
index 597bbb52b..92fb66234 100644
--- a/pkg/tools/mcp_tool_test.go
+++ b/pkg/tools/mcp_tool_test.go
@@ -141,9 +141,11 @@ func TestMCPTool_Description(t *testing.T) {
 // TestMCPTool_Parameters verifies parameter schema conversion
 func TestMCPTool_Parameters(t *testing.T) {
 	tests := []struct {
-		name        string
-		inputSchema interface{}
-		expectType  string
+		name           string
+		inputSchema    interface{}
+		expectType     string
+		checkProperty  string
+		expectProperty bool
 	}{
 		{
 			name: "map schema",
@@ -157,12 +159,35 @@ func TestMCPTool_Parameters(t *testing.T) {
 				},
 				"required": []string{"query"},
 			},
-			expectType: "object",
+			expectType:     "object",
+			checkProperty:  "query",
+			expectProperty: true,
 		},
 		{
-			name:        "nil schema",
-			inputSchema: nil,
-			expectType:  "object",
+			name:           "nil schema",
+			inputSchema:    nil,
+			expectType:     "object",
+			expectProperty: false,
+		},
+		{
+			name: "json.RawMessage schema",
+			inputSchema: []byte(`{
+				"type": "object",
+				"properties": {
+					"repo": {
+						"type": "string",
+						"description": "Repository name"
+					},
+					"stars": {
+						"type": "integer",
+						"description": "Minimum stars"
+					}
+				},
+				"required": ["repo"]
+			}`),
+			expectType:     "object",
+			checkProperty:  "repo",
+			expectProperty: true,
 		},
 	}
 
@@ -184,6 +209,22 @@ func TestMCPTool_Parameters(t *testing.T) {
 			if params["type"] != tt.expectType {
 				t.Errorf("Expected type '%s', got '%v'", tt.expectType, params["type"])
 			}
+
+			// Check if property exists when expected
+			if tt.checkProperty != "" {
+				properties, ok := params["properties"].(map[string]interface{})
+				if !ok && tt.expectProperty {
+					t.Errorf("Expected properties to be a map")
+					return
+				}
+				if ok {
+					_, hasProperty := properties[tt.checkProperty]
+					if hasProperty != tt.expectProperty {
+						t.Errorf("Expected property '%s' existence: %v, got: %v",
+							tt.checkProperty, tt.expectProperty, hasProperty)
+					}
+				}
+			}
 		})
 	}
 }