fix: recursively enforce required properties in OpenAI tool schemas

e2e/cagent_exec_test.go

@@ -122,9 +122,7 @@ func TestExec_Mistral(t *testing.T) {
 func TestExec_Mistral_ToolCall(t *testing.T) {
 	out := cagent(t, "exec", "testdata/fs_tools.yaml", "--model=mistral/mistral-small", "How many files in testdata/working_dir? Only output the number.")
 
-	// NOTE: If you look at the LLM response, Mistral says it sees 2 files, yours truly got tired of re-running this test to get it to say "1".
-	// For now, just update the expected output
-	require.Equal(t, "\n--- Agent: root ---\n\nCalling list_directory(path: \"testdata/working_dir\")\n\nlist_directory response → \"FILE README.me\\n\"\n2", out)
+	require.Equal(t, "\n--- Agent: root ---\n\nCalling list_directory(path: \"testdata/working_dir\")\n\nlist_directory response → \"FILE README.me\\n\"\n1", out)
 }
 
 func TestExec_ToolCallsNeedAcceptance(t *testing.T) {

pkg/config/model_alias_test.go

@@ -30,6 +30,9 @@ func TestResolveModelAliases(t *testing.T) {
 	store, err := modelsdev.NewStore(modelsdev.WithCacheDir(t.TempDir()))
 	require.NoError(t, err)
 	store.SetDatabaseForTesting(mockData)
+	t.Cleanup(func() {
+		store.SetDatabaseForTesting(nil)
+	})
 
 	ctx := t.Context()
 

pkg/model/provider/openai/schema.go

@@ -19,78 +19,90 @@ func ConvertParametersToSchema(params any) (shared.FunctionParameters, error) {
 	return fixSchemaArrayItems(removeFormatFields(makeAllRequired(p))), nil
 }
 
-// makeAllRequired make all the parameters "required"
-// because that's what the Response API wants, now.
-func makeAllRequired(schema shared.FunctionParameters) shared.FunctionParameters {
-	if schema == nil {
-		return makeAllRequired(map[string]any{"type": "object", "properties": map[string]any{}})
-	}
+// walkSchema calls fn on the given schema node, then recursively walks into
+// properties, anyOf/oneOf/allOf variants, and array items.
+func walkSchema(schema map[string]any, fn func(map[string]any)) {
+	fn(schema)
 
-	properties, ok := schema["properties"].(map[string]any)
-	if !ok {
-		return schema
+	if properties, ok := schema["properties"].(map[string]any); ok {
+		for _, v := range properties {
+			if sub, ok := v.(map[string]any); ok {
+				walkSchema(sub, fn)
+			}
+		}
 	}
 
-	reallyRequired := map[string]bool{}
-	if required, ok := schema["required"].([]any); ok {
-		for _, name := range required {
-			reallyRequired[name.(string)] = true
+	for _, keyword := range []string{"anyOf", "oneOf", "allOf"} {
+		if variants, ok := schema[keyword].([]any); ok {
+			for _, v := range variants {
+				if sub, ok := v.(map[string]any); ok {
+					walkSchema(sub, fn)
+				}
+			}
 		}
 	}
 
-	// We can't use a nil 'required' attribute
-	newRequired := []any{}
+	if items, ok := schema["items"].(map[string]any); ok {
+		walkSchema(items, fn)
+	}
+}
 
-	// Sort property names for deterministic output
-	propNames := slices.Sorted(maps.Keys(properties))
+// makeAllRequired makes all object properties "required" throughout the schema,
+// because that's what the OpenAI Response API demands.
+// Properties that were not originally required are made nullable.
+func makeAllRequired(schema shared.FunctionParameters) shared.FunctionParameters {
+	if schema == nil {
+		schema = map[string]any{"type": "object", "properties": map[string]any{}}
+	}
 
-	for _, propName := range propNames {
-		newRequired = append(newRequired, propName)
-		if reallyRequired[propName] {
-			continue
+	walkSchema(schema, func(node map[string]any) {
+		properties, ok := node["properties"].(map[string]any)
+		if !ok {
+			return
 		}
 
-		// Make its type nullable
-		if propMap, ok := properties[propName].(map[string]any); ok {
-			if typeValue, ok := propMap["type"].(string); ok {
-				propMap["type"] = []string{typeValue, "null"}
+		originallyRequired := map[string]bool{}
+		if required, ok := node["required"].([]any); ok {
+			for _, name := range required {
+				originallyRequired[name.(string)] = true
 			}
 		}
-	}
 
-	schema["required"] = newRequired
-	schema["additionalProperties"] = false
+		newRequired := []any{}
+		for _, propName := range slices.Sorted(maps.Keys(properties)) {
+			newRequired = append(newRequired, propName)
+
+			// Make newly-required properties nullable
+			if !originallyRequired[propName] {
+				if propMap, ok := properties[propName].(map[string]any); ok {
+					if t, ok := propMap["type"].(string); ok {
+						propMap["type"] = []string{t, "null"}
+					}
+				}
+			}
+		}
+
+		node["required"] = newRequired
+		node["additionalProperties"] = false
+	})
+
 	return schema
 }
 
-// removeFormatFields removes the "format" field from all properties in the schema, recursively.
+// removeFormatFields removes the "format" field from all nodes in the schema.
 // OpenAI does not support the JSON Schema "format" keyword (e.g. "uri", "email", "date").
 func removeFormatFields(schema shared.FunctionParameters) shared.FunctionParameters {
 	if schema == nil {
 		return nil
 	}
 
-	removeFormatFieldsRecursive(schema)
+	walkSchema(schema, func(node map[string]any) {
+		delete(node, "format")
+	})
 
 	return schema
 }
 
-func removeFormatFieldsRecursive(schema map[string]any) {
-	delete(schema, "format")
-
-	if properties, ok := schema["properties"].(map[string]any); ok {
-		for _, propValue := range properties {
-			if prop, ok := propValue.(map[string]any); ok {
-				removeFormatFieldsRecursive(prop)
-			}
-		}
-	}
-
-	if items, ok := schema["items"].(map[string]any); ok {
-		removeFormatFieldsRecursive(items)
-	}
-}
-
 // In Docker Desktop 4.52, the MCP Gateway produces an invalid tools shema for `mcp-config-set`.
 func fixSchemaArrayItems(schema shared.FunctionParameters) shared.FunctionParameters {
 	propertiesValue, ok := schema["properties"]

pkg/model/provider/openai/schema_test.go

@@ -55,6 +55,118 @@ func TestMakeAllRequired_NilSchema(t *testing.T) {
 	assert.JSONEq(t, `{"additionalProperties":false,"properties":{},"type":"object","required":[]}`, string(buf))
 }
 
+func TestMakeAllRequired_AnyOf(t *testing.T) {
+	// Reproduces the chrome-devtools-mcp "emulate" tool schema where
+	// viewport has an anyOf with an object variant whose properties
+	// are not all listed in required. OpenAI rejects this.
+	schema := shared.FunctionParameters{
+		"type": "object",
+		"properties": map[string]any{
+			"viewport": map[string]any{
+				"anyOf": []any{
+					map[string]any{
+						"type": "object",
+						"properties": map[string]any{
+							"width":             map[string]any{"type": "number"},
+							"height":            map[string]any{"type": "number"},
+							"deviceScaleFactor": map[string]any{"type": "number"},
+						},
+						"required": []any{"width", "height"},
+					},
+					map[string]any{
+						"type": "null",
+					},
+				},
+			},
+		},
+		"required": []any{"viewport"},
+	}
+
+	updated := makeAllRequired(schema)
+
+	// Top-level: viewport must be required
+	required := updated["required"].([]any)
+	assert.Contains(t, required, "viewport")
+
+	// anyOf[0]: all properties must be required, including deviceScaleFactor
+	viewport := updated["properties"].(map[string]any)["viewport"].(map[string]any)
+	anyOf := viewport["anyOf"].([]any)
+	variant := anyOf[0].(map[string]any)
+	variantRequired := variant["required"].([]any)
+	assert.Len(t, variantRequired, 3)
+	assert.Contains(t, variantRequired, "width")
+	assert.Contains(t, variantRequired, "height")
+	assert.Contains(t, variantRequired, "deviceScaleFactor")
+
+	// deviceScaleFactor was not originally required, so its type should be nullable
+	dsf := variant["properties"].(map[string]any)["deviceScaleFactor"].(map[string]any)
+	assert.Equal(t, []string{"number", "null"}, dsf["type"])
+
+	// width was originally required, so its type should be unchanged
+	w := variant["properties"].(map[string]any)["width"].(map[string]any)
+	assert.Equal(t, "number", w["type"])
+}
+
+func TestMakeAllRequired_NestedProperties(t *testing.T) {
+	schema := shared.FunctionParameters{
+		"type": "object",
+		"properties": map[string]any{
+			"config": map[string]any{
+				"type": "object",
+				"properties": map[string]any{
+					"name":  map[string]any{"type": "string"},
+					"value": map[string]any{"type": "string"},
+				},
+				"required": []any{"name"},
+			},
+		},
+		"required": []any{"config"},
+	}
+
+	updated := makeAllRequired(schema)
+
+	// Nested object: all properties must be required
+	config := updated["properties"].(map[string]any)["config"].(map[string]any)
+	configRequired := config["required"].([]any)
+	assert.Len(t, configRequired, 2)
+	assert.Contains(t, configRequired, "name")
+	assert.Contains(t, configRequired, "value")
+
+	// value was not originally required, so its type should be nullable
+	value := config["properties"].(map[string]any)["value"].(map[string]any)
+	assert.Equal(t, []string{"string", "null"}, value["type"])
+}
+
+func TestMakeAllRequired_ArrayItems(t *testing.T) {
+	schema := shared.FunctionParameters{
+		"type": "object",
+		"properties": map[string]any{
+			"items": map[string]any{
+				"type": "array",
+				"items": map[string]any{
+					"type": "object",
+					"properties": map[string]any{
+						"id":   map[string]any{"type": "string"},
+						"name": map[string]any{"type": "string"},
+					},
+					"required": []any{"id"},
+				},
+			},
+		},
+		"required": []any{"items"},
+	}
+
+	updated := makeAllRequired(schema)
+
+	// Array items object: all properties must be required
+	itemsSchema := updated["properties"].(map[string]any)["items"].(map[string]any)
+	itemObj := itemsSchema["items"].(map[string]any)
+	itemRequired := itemObj["required"].([]any)
+	assert.Len(t, itemRequired, 2)
+	assert.Contains(t, itemRequired, "id")
+	assert.Contains(t, itemRequired, "name")
+}
+
 func TestRemoveFormatFields(t *testing.T) {
 	schema := map[string]any{
 		"type": "object",

pkg/modelsdev/store_test.go

@@ -39,6 +39,9 @@ func TestResolveModelAlias(t *testing.T) {
 	store, err := NewStore(WithCacheDir(t.TempDir()))
 	require.NoError(t, err)
 	store.SetDatabaseForTesting(mockData)
+	t.Cleanup(func() {
+		store.SetDatabaseForTesting(nil)
+	})
 
 	ctx := t.Context()
 

pkg/teamloader/teamloader_test.go

@@ -17,22 +17,9 @@ import (
 	"github.com/docker/cagent/pkg/config/latest"
 	"github.com/docker/cagent/pkg/environment"
 	"github.com/docker/cagent/pkg/model/provider/dmr"
-	"github.com/docker/cagent/pkg/modelsdev"
 	"github.com/docker/cagent/pkg/tools"
 )
 
-// TestMain sets up the test environment for all tests in this package.
-func TestMain(m *testing.M) {
-	store, err := modelsdev.NewStore()
-	if err != nil {
-		os.Exit(1)
-	}
-	store.SetDatabaseForTesting(&modelsdev.Database{
-		Providers: make(map[string]modelsdev.Provider),
-	})
-	os.Exit(m.Run())
-}
-
 // skipExamples contains example files that require cloud-specific configurations
 // (e.g., AWS profiles, GCP credentials) that can't be mocked with dummy env vars.
 var skipExamples = map[string]string{