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{}
This commit is contained in:
parent
20f8bb200b
commit
02c1792015
2 changed files with 102 additions and 25 deletions
|
|
@ -2,6 +2,7 @@ package tools
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
|
|
@ -51,26 +52,61 @@ func (t *MCPTool) Parameters() map[string]interface{} {
|
||||||
// The InputSchema is already a JSON Schema object
|
// The InputSchema is already a JSON Schema object
|
||||||
schema := t.tool.InputSchema
|
schema := t.tool.InputSchema
|
||||||
|
|
||||||
// Convert to map[string]interface{} for compatibility
|
// Handle nil schema
|
||||||
result := make(map[string]interface{})
|
if schema == nil {
|
||||||
|
return map[string]interface{}{
|
||||||
|
"type": "object",
|
||||||
|
"properties": map[string]interface{}{},
|
||||||
|
"required": []string{},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Use reflection to convert the schema
|
// Try direct conversion first (fast path)
|
||||||
// The schema should already be in the correct format
|
|
||||||
if schema != nil {
|
|
||||||
// Attempt to convert directly
|
|
||||||
if schemaMap, ok := schema.(map[string]interface{}); ok {
|
if schemaMap, ok := schema.(map[string]interface{}); ok {
|
||||||
return schemaMap
|
return schemaMap
|
||||||
}
|
}
|
||||||
|
|
||||||
// Otherwise, build it manually
|
// Handle json.RawMessage and []byte - unmarshal directly
|
||||||
result["type"] = "object"
|
var jsonData []byte
|
||||||
result["properties"] = map[string]interface{}{}
|
if rawMsg, ok := schema.(json.RawMessage); ok {
|
||||||
result["required"] = []string{}
|
jsonData = rawMsg
|
||||||
} else {
|
} else if bytes, ok := schema.([]byte); ok {
|
||||||
// Default schema when nil
|
jsonData = bytes
|
||||||
result["type"] = "object"
|
}
|
||||||
result["properties"] = map[string]interface{}{}
|
|
||||||
result["required"] = []string{}
|
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
|
return result
|
||||||
|
|
|
||||||
|
|
@ -144,6 +144,8 @@ func TestMCPTool_Parameters(t *testing.T) {
|
||||||
name string
|
name string
|
||||||
inputSchema interface{}
|
inputSchema interface{}
|
||||||
expectType string
|
expectType string
|
||||||
|
checkProperty string
|
||||||
|
expectProperty bool
|
||||||
}{
|
}{
|
||||||
{
|
{
|
||||||
name: "map schema",
|
name: "map schema",
|
||||||
|
|
@ -158,11 +160,34 @@ func TestMCPTool_Parameters(t *testing.T) {
|
||||||
"required": []string{"query"},
|
"required": []string{"query"},
|
||||||
},
|
},
|
||||||
expectType: "object",
|
expectType: "object",
|
||||||
|
checkProperty: "query",
|
||||||
|
expectProperty: true,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "nil schema",
|
name: "nil schema",
|
||||||
inputSchema: nil,
|
inputSchema: nil,
|
||||||
expectType: "object",
|
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 {
|
if params["type"] != tt.expectType {
|
||||||
t.Errorf("Expected type '%s', got '%v'", tt.expectType, params["type"])
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue