Fix script args with DMR

pkg/model/provider/dmr/client.go

@@ -472,10 +472,6 @@ func (c *Client) CreateChatCompletionStream(ctx context.Context, messages []chat
 		slog.Debug("Adding tools to DMR request", "tool_count", len(requestTools))
 		toolsParam := make([]openai.ChatCompletionToolUnionParam, len(requestTools))
 		for i, tool := range requestTools {
-			// DMR requires the `description` key to be present; ensure a non-empty value
-			// NOTE(krissetto): workaround, remove when fixed upstream, this shouldn't be necceessary
-			desc := cmp.Or(tool.Description, "Function "+tool.Name)
-
 			parameters, err := ConvertParametersToSchema(tool.Parameters)
 			if err != nil {
 				slog.Error("Failed to convert tool parameters to DMR schema", "error", err, "tool", tool.Name)
@@ -488,9 +484,11 @@ func (c *Client) CreateChatCompletionStream(ctx context.Context, messages []chat
 				return nil, fmt.Errorf("converted parameters is not a map for tool %s", tool.Name)
 			}
 
+			// DMR requires the `description` key to be present; ensure a non-empty value
+			// NOTE(krissetto): workaround, remove when fixed upstream, this shouldn't be necessary
 			toolsParam[i] = openai.ChatCompletionFunctionTool(shared.FunctionDefinitionParam{
 				Name:        tool.Name,
-				Description: openai.String(desc),
+				Description: openai.String(cmp.Or(tool.Description, "Function "+tool.Name)),
 				Parameters:  paramsMap,
 			})
 		}
@@ -872,20 +870,6 @@ func (c *Client) Rerank(ctx context.Context, query string, documents []types.Doc
 	return scores, nil
 }
 
-// ConvertParametersToSchema converts parameters to DMR Schema format
-func ConvertParametersToSchema(params any) (any, error) {
-	m, err := tools.SchemaToMap(params)
-	if err != nil {
-		return nil, err
-	}
-
-	// DMR models tend to dislike `additionalProperties` in the schema
-	// e.g. ai/qwen3 and ai/gpt-oss
-	delete(m, "additionalProperties")
-
-	return m, nil
-}
-
 type speculativeDecodingOpts struct {
 	draftModel     string
 	numTokens      int

pkg/model/provider/dmr/schema.go

@@ -0,0 +1,19 @@
+package dmr
+
+import (
+	"github.com/docker/cagent/pkg/tools"
+)
+
+// ConvertParametersToSchema converts parameters to DMR Schema format
+func ConvertParametersToSchema(params any) (any, error) {
+	m, err := tools.SchemaToMap(params)
+	if err != nil {
+		return nil, err
+	}
+
+	// DMR models tend to dislike `additionalProperties` in the schema
+	// e.g. ai/qwen3 and ai/gpt-oss
+	delete(m, "additionalProperties")
+
+	return m, nil
+}

pkg/tools/builtin/script_shell.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"maps"
 	"os"
 	"os/exec"
 	"slices"
@@ -110,7 +111,7 @@ func (t *ScriptShellTool) Tools(context.Context) ([]tools.Tool, error) {
 
 		inputSchema, err := tools.SchemaToMap(map[string]any{
 			"type":       "object",
-			"properties": cfg.Args,
+			"properties": defaultPropertyTypes(cfg.Args, "string"),
 			"required":   cfg.Required,
 		})
 		if err != nil {
@@ -158,3 +159,20 @@ func (t *ScriptShellTool) execute(ctx context.Context, toolConfig *latest.Script
 
 	return tools.ResultSuccess(limitOutput(string(output))), nil
 }
+
+// defaultPropertyTypes returns a copy of properties where any property
+// missing a "type" field gets the given default type.
+func defaultPropertyTypes(properties map[string]any, defaultType string) map[string]any {
+	result := make(map[string]any, len(properties))
+	for k, v := range properties {
+		if prop, ok := v.(map[string]any); ok && prop["type"] == nil {
+			propCopy := make(map[string]any, len(prop)+1)
+			maps.Copy(propCopy, prop)
+			propCopy["type"] = defaultType
+			result[k] = propCopy
+			continue
+		}
+		result[k] = v
+	}
+	return result
+}

pkg/tools/builtin/script_shell_test.go

@@ -115,3 +115,38 @@ func TestNewScriptShellTool_MissingRequired(t *testing.T) {
 	require.Nil(t, tool)
 	require.ErrorContains(t, err, "tool 'docker_images' has required arg 'img' which is not defined in args")
 }
+
+func TestNewScriptShellTool_ArgWithoutType(t *testing.T) {
+	shellTools := map[string]latest.ScriptShellToolConfig{
+		"greet": {
+			Description: "Greet someone",
+			Cmd:         "echo Hello $name",
+			Args: map[string]any{
+				"name": map[string]any{
+					"description": "Name to greet",
+				},
+			},
+			Required: []string{"name"},
+		},
+	}
+
+	tool, err := NewScriptShellTool(shellTools, nil)
+	require.NoError(t, err)
+
+	allTools, err := tool.Tools(t.Context())
+	require.NoError(t, err)
+	assert.Len(t, allTools, 1)
+
+	schema, err := json.Marshal(allTools[0].Parameters)
+	require.NoError(t, err)
+	assert.JSONEq(t, `{
+	"type": "object",
+	"properties": {
+		"name": {
+			"description": "Name to greet",
+			"type": "string"
+		}
+	},
+	"required": ["name"]
+}`, string(schema))
+}

pkg/tools/schema.go

@@ -47,9 +47,41 @@ func SchemaToMap(params any) (map[string]any, error) {
 		delete(m, "required")
 	}
 
+	// Ensure all properties have a type set, recursively.
+	ensurePropertyTypes(m)
+
 	return m, nil
 }
 
+// ensurePropertyTypes recursively walks a JSON Schema map and ensures
+// every property has a "type" set, defaulting to "object" if missing.
+// It descends into nested "properties" and array "items".
+func ensurePropertyTypes(schema map[string]any) {
+	props, ok := schema["properties"].(map[string]any)
+	if !ok {
+		return
+	}
+
+	for _, v := range props {
+		prop, ok := v.(map[string]any)
+		if !ok {
+			continue
+		}
+
+		if prop["type"] == nil {
+			prop["type"] = "object"
+		}
+
+		// Recurse into nested object properties.
+		ensurePropertyTypes(prop)
+
+		// Recurse into array items.
+		if items, ok := prop["items"].(map[string]any); ok {
+			ensurePropertyTypes(items)
+		}
+	}
+}
+
 func ConvertSchema(params, v any) error {
 	// First unmarshal to a map to check we have a type and non-nil properties
 	m, err := SchemaToMap(params)

pkg/tools/schema_test.go

@@ -40,3 +40,149 @@ func TestSchemaToMap_MissingEmptyProperties(t *testing.T) {
 		"properties": map[string]any{},
 	}, m)
 }
+
+func TestSchemaToMap_PropertyWithoutType(t *testing.T) {
+	m, err := SchemaToMap(map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"name": map[string]any{
+				"type": "string",
+			},
+			"metadata": map[string]any{
+				"description": "some metadata",
+			},
+		},
+	})
+	require.NoError(t, err)
+
+	assert.Equal(t, map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"name": map[string]any{
+				"type": "string",
+			},
+			"metadata": map[string]any{
+				"type":        "object",
+				"description": "some metadata",
+			},
+		},
+	}, m)
+}
+
+func TestSchemaToMap_NestedPropertyWithoutType(t *testing.T) {
+	m, err := SchemaToMap(map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"config": map[string]any{
+				"type": "object",
+				"properties": map[string]any{
+					"host": map[string]any{
+						"type": "string",
+					},
+					"metadata": map[string]any{
+						"description": "nested metadata without type",
+					},
+				},
+			},
+		},
+	})
+	require.NoError(t, err)
+
+	assert.Equal(t, map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"config": map[string]any{
+				"type": "object",
+				"properties": map[string]any{
+					"host": map[string]any{
+						"type": "string",
+					},
+					"metadata": map[string]any{
+						"type":        "object",
+						"description": "nested metadata without type",
+					},
+				},
+			},
+		},
+	}, m)
+}
+
+func TestSchemaToMap_ArrayItemsPropertyWithoutType(t *testing.T) {
+	m, err := SchemaToMap(map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"items": map[string]any{
+				"type": "array",
+				"items": map[string]any{
+					"type": "object",
+					"properties": map[string]any{
+						"value": map[string]any{
+							"description": "value without type",
+						},
+					},
+				},
+			},
+		},
+	})
+	require.NoError(t, err)
+
+	assert.Equal(t, map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"items": map[string]any{
+				"type": "array",
+				"items": map[string]any{
+					"type": "object",
+					"properties": map[string]any{
+						"value": map[string]any{
+							"type":        "object",
+							"description": "value without type",
+						},
+					},
+				},
+			},
+		},
+	}, m)
+}
+
+func TestSchemaToMap_DeeplyNestedPropertyWithoutType(t *testing.T) {
+	m, err := SchemaToMap(map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"level1": map[string]any{
+				"type": "object",
+				"properties": map[string]any{
+					"level2": map[string]any{
+						"type": "object",
+						"properties": map[string]any{
+							"level3": map[string]any{
+								"description": "deeply nested without type",
+							},
+						},
+					},
+				},
+			},
+		},
+	})
+	require.NoError(t, err)
+
+	assert.Equal(t, map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"level1": map[string]any{
+				"type": "object",
+				"properties": map[string]any{
+					"level2": map[string]any{
+						"type": "object",
+						"properties": map[string]any{
+							"level3": map[string]any{
+								"type":        "object",
+								"description": "deeply nested without type",
+							},
+						},
+					},
+				},
+			},
+		},
+	}, m)
+}