fix(tools): ensure tool parameters have valid JSON Schema properties field
Some MCP servers return tool schemas without a `properties` field in their parameters object. Strict OpenAI-compatible APIs like LM Studio validate tool schemas and reject requests where `properties` is missing, has the wrong type, or is a typed-nil map. This fix normalizes tool parameters in ToProviderDefs(): - If params is nil, create a valid JSON Schema with type and properties - If params exists but properties is missing/invalid, make a defensive copy and add an empty properties object (avoiding mutation of the original tool schema to prevent concurrent map write panics) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
748ac58dd1
commit
f3e65080ad
2 changed files with 137 additions and 0 deletions
|
|
@ -352,6 +352,31 @@ func (r *ToolRegistry) ToProviderDefs() []providers.ToolDefinition {
|
||||||
name, _ := fn["name"].(string)
|
name, _ := fn["name"].(string)
|
||||||
desc, _ := fn["description"].(string)
|
desc, _ := fn["description"].(string)
|
||||||
params, _ := fn["parameters"].(map[string]any)
|
params, _ := fn["parameters"].(map[string]any)
|
||||||
|
// Normalize params so they include a properties field for object schemas.
|
||||||
|
// While JSON Schema does not require properties, some MCP servers omit it
|
||||||
|
// and strict OpenAI-compatible API validators (e.g., LM Studio) then fail.
|
||||||
|
if params == nil {
|
||||||
|
params = map[string]any{
|
||||||
|
"type": "object",
|
||||||
|
"properties": map[string]any{},
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
// Ensure properties is a non-nil map[string]any. Some tools may provide
|
||||||
|
// properties with the wrong type or as a typed-nil map, which strict
|
||||||
|
// validators may reject.
|
||||||
|
propsVal, hasProps := params["properties"]
|
||||||
|
propsMap, okPropsMap := propsVal.(map[string]any)
|
||||||
|
if !hasProps || !okPropsMap || propsMap == nil {
|
||||||
|
// Make a defensive copy to avoid mutating the original tool schema,
|
||||||
|
// which could cause concurrent map writes if called from multiple goroutines.
|
||||||
|
paramsCopy := make(map[string]any, len(params)+1)
|
||||||
|
for k, v := range params {
|
||||||
|
paramsCopy[k] = v
|
||||||
|
}
|
||||||
|
paramsCopy["properties"] = map[string]any{}
|
||||||
|
params = paramsCopy
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
definitions = append(definitions, providers.ToolDefinition{
|
definitions = append(definitions, providers.ToolDefinition{
|
||||||
Type: "function",
|
Type: "function",
|
||||||
|
|
|
||||||
|
|
@ -300,6 +300,118 @@ func TestToolRegistry_ToProviderDefs(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestToolRegistry_ToProviderDefs_NilParams(t *testing.T) {
|
||||||
|
r := NewToolRegistry()
|
||||||
|
r.Register(&mockRegistryTool{
|
||||||
|
name: "nil-params",
|
||||||
|
desc: "tool with nil params",
|
||||||
|
params: nil,
|
||||||
|
result: SilentResult("ok"),
|
||||||
|
})
|
||||||
|
|
||||||
|
defs := r.ToProviderDefs()
|
||||||
|
if len(defs) != 1 {
|
||||||
|
t.Fatalf("expected 1 provider def, got %d", len(defs))
|
||||||
|
}
|
||||||
|
|
||||||
|
params := defs[0].Function.Parameters
|
||||||
|
if params == nil {
|
||||||
|
t.Fatal("expected non-nil parameters")
|
||||||
|
}
|
||||||
|
if params["type"] != "object" {
|
||||||
|
t.Errorf("expected type 'object', got %v", params["type"])
|
||||||
|
}
|
||||||
|
propsMap, ok := params["properties"].(map[string]any)
|
||||||
|
if !ok || propsMap == nil {
|
||||||
|
t.Errorf("expected properties to be non-nil map[string]any, got %T", params["properties"])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestToolRegistry_ToProviderDefs_MissingProperties(t *testing.T) {
|
||||||
|
r := NewToolRegistry()
|
||||||
|
originalParams := map[string]any{"type": "object"}
|
||||||
|
r.Register(&mockRegistryTool{
|
||||||
|
name: "missing-props",
|
||||||
|
desc: "tool with params missing properties",
|
||||||
|
params: originalParams,
|
||||||
|
result: SilentResult("ok"),
|
||||||
|
})
|
||||||
|
|
||||||
|
defs := r.ToProviderDefs()
|
||||||
|
if len(defs) != 1 {
|
||||||
|
t.Fatalf("expected 1 provider def, got %d", len(defs))
|
||||||
|
}
|
||||||
|
|
||||||
|
params := defs[0].Function.Parameters
|
||||||
|
propsMap, ok := params["properties"].(map[string]any)
|
||||||
|
if !ok || propsMap == nil {
|
||||||
|
t.Errorf("expected properties to be non-nil map[string]any, got %T", params["properties"])
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify original params was not mutated (defensive copy)
|
||||||
|
if _, ok := originalParams["properties"]; ok {
|
||||||
|
t.Error("original params should not be mutated")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestToolRegistry_ToProviderDefs_WrongTypeProperties(t *testing.T) {
|
||||||
|
r := NewToolRegistry()
|
||||||
|
// properties is a string instead of map[string]any
|
||||||
|
originalParams := map[string]any{"type": "object", "properties": "invalid"}
|
||||||
|
r.Register(&mockRegistryTool{
|
||||||
|
name: "wrong-type-props",
|
||||||
|
desc: "tool with wrong type properties",
|
||||||
|
params: originalParams,
|
||||||
|
result: SilentResult("ok"),
|
||||||
|
})
|
||||||
|
|
||||||
|
defs := r.ToProviderDefs()
|
||||||
|
if len(defs) != 1 {
|
||||||
|
t.Fatalf("expected 1 provider def, got %d", len(defs))
|
||||||
|
}
|
||||||
|
|
||||||
|
params := defs[0].Function.Parameters
|
||||||
|
propsMap, ok := params["properties"].(map[string]any)
|
||||||
|
if !ok || propsMap == nil {
|
||||||
|
t.Errorf("expected properties to be non-nil map[string]any, got %T", params["properties"])
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify original params was not mutated (defensive copy)
|
||||||
|
if originalParams["properties"] != "invalid" {
|
||||||
|
t.Error("original params should not be mutated")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestToolRegistry_ToProviderDefs_TypedNilProperties(t *testing.T) {
|
||||||
|
r := NewToolRegistry()
|
||||||
|
// properties is a typed-nil map
|
||||||
|
var typedNil map[string]any
|
||||||
|
originalParams := map[string]any{"type": "object", "properties": typedNil}
|
||||||
|
r.Register(&mockRegistryTool{
|
||||||
|
name: "typed-nil-props",
|
||||||
|
desc: "tool with typed-nil properties",
|
||||||
|
params: originalParams,
|
||||||
|
result: SilentResult("ok"),
|
||||||
|
})
|
||||||
|
|
||||||
|
defs := r.ToProviderDefs()
|
||||||
|
if len(defs) != 1 {
|
||||||
|
t.Fatalf("expected 1 provider def, got %d", len(defs))
|
||||||
|
}
|
||||||
|
|
||||||
|
params := defs[0].Function.Parameters
|
||||||
|
propsMap, ok := params["properties"].(map[string]any)
|
||||||
|
if !ok || propsMap == nil {
|
||||||
|
t.Errorf("expected properties to be non-nil map[string]any, got %T", params["properties"])
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify original params was not mutated (defensive copy)
|
||||||
|
// The original should still have the typed-nil value
|
||||||
|
if origProps, ok := originalParams["properties"].(map[string]any); !ok || origProps != nil {
|
||||||
|
t.Error("original params should not be mutated")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestToolRegistry_List(t *testing.T) {
|
func TestToolRegistry_List(t *testing.T) {
|
||||||
r := NewToolRegistry()
|
r := NewToolRegistry()
|
||||||
r.Register(newMockTool("x", ""))
|
r.Register(newMockTool("x", ""))
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue