Normalize required fields in agent JSON schemas for OpenAI

OpenAI rejects schemas where optional properties are absent from the
required array. Promote all properties to required and mark formerly
optional ones nullable so the model knows it may pass null.

Also upgrade tool error log level from Warn to Error.

Signed-off-by: Bryan Frimin <bryan@probo.com>

pkg/agent/run.go

@@ -1279,7 +1279,7 @@ func executeSingleTool(
 			content = content[:200] + "... (truncated)"
 		}
 
-		logger.WarnCtx(
+		logger.ErrorCtx(
 			ctx,
 			"tool returned error",
 			log.String("tool", tool.Name()),

pkg/agent/schema.go

@@ -18,6 +18,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
+	"slices"
 
 	"github.com/google/jsonschema-go/jsonschema"
 )
@@ -37,6 +38,14 @@ func jsonSchemaFor[T any]() (json.RawMessage, error) {
 		return nil, fmt.Errorf("cannot marshal schema for %s: %w", t, err)
 	}
 
+	// OpenAI rejects schemas where required does not list every key in
+	// properties.  Promote optional properties into required and mark them
+	// nullable so the model knows it may pass null.
+	data, err = normalizeRequiredJSON(data)
+	if err != nil {
+		return nil, fmt.Errorf("cannot normalize schema for %s: %w", t, err)
+	}
+
 	return json.RawMessage(data), nil
 }
 
@@ -49,6 +58,122 @@ func mustJSONSchemaFor[T any]() json.RawMessage {
 	return schema
 }
 
+// normalizeRequiredJSON ensures every property key in an object schema also
+// appears in its required array.  Properties that were not originally required
+// are made nullable (their "type" becomes ["T","null"]) so the LLM knows it
+// may pass null for them.  The transformation is applied recursively so nested
+// object schemas are also normalised.
+func normalizeRequiredJSON(data []byte) ([]byte, error) {
+	var obj map[string]json.RawMessage
+	if err := json.Unmarshal(data, &obj); err != nil {
+		return data, nil
+	}
+
+	propsRaw, hasProps := obj["properties"]
+	if !hasProps {
+		if itemsRaw, ok := obj["items"]; ok {
+			n, err := normalizeRequiredJSON(itemsRaw)
+			if err != nil {
+				return nil, err
+			}
+			obj["items"] = n
+		}
+		if addlRaw, ok := obj["additionalProperties"]; ok {
+			n, err := normalizeRequiredJSON(addlRaw)
+			if err != nil {
+				return nil, err
+			}
+			obj["additionalProperties"] = n
+		}
+		return json.Marshal(obj)
+	}
+
+	var props map[string]json.RawMessage
+	if err := json.Unmarshal(propsRaw, &props); err != nil {
+		return data, nil
+	}
+
+	var required []string
+	if reqRaw, ok := obj["required"]; ok {
+		_ = json.Unmarshal(reqRaw, &required)
+	}
+
+	requiredSet := make(map[string]bool, len(required))
+	for _, r := range required {
+		requiredSet[r] = true
+	}
+
+	for name, propRaw := range props {
+		n, err := normalizeRequiredJSON(propRaw)
+		if err != nil {
+			return nil, err
+		}
+
+		if !requiredSet[name] {
+			n, err = makeNullableJSON(n)
+			if err != nil {
+				return nil, err
+			}
+			required = append(required, name)
+			requiredSet[name] = true
+		}
+
+		props[name] = n
+	}
+
+	propsData, err := json.Marshal(props)
+	if err != nil {
+		return nil, err
+	}
+	obj["properties"] = propsData
+
+	if len(required) > 0 {
+		reqData, err := json.Marshal(required)
+		if err != nil {
+			return nil, err
+		}
+		obj["required"] = reqData
+	}
+
+	return json.Marshal(obj)
+}
+
+// makeNullableJSON adds "null" to the "type" field of a JSON Schema object so
+// that the LLM understands it may pass null for optional properties.
+func makeNullableJSON(data []byte) ([]byte, error) {
+	var obj map[string]json.RawMessage
+	if err := json.Unmarshal(data, &obj); err != nil {
+		return data, nil
+	}
+
+	typeRaw, ok := obj["type"]
+	if !ok {
+		return data, nil
+	}
+
+	var single string
+	if err := json.Unmarshal(typeRaw, &single); err == nil {
+		if single != "null" {
+			arr, _ := json.Marshal([]string{single, "null"})
+			obj["type"] = arr
+		}
+		return json.Marshal(obj)
+	}
+
+	var arr []string
+	if err := json.Unmarshal(typeRaw, &arr); err == nil {
+		if slices.Contains(arr, "null") {
+			return data, nil
+		}
+		arr = append(arr, "null")
+		nullable, _ := json.Marshal(arr)
+		obj["type"] = nullable
+		return json.Marshal(obj)
+	}
+
+	return data, nil
+}
+
 // stripNullTypes removes "null" from union types produced by pointer fields
 // (e.g. ["null","string"] becomes "string") and clears integer bounds so that
 // LLM providers receive a clean schema without Go-specific type constraints.

pkg/agent/schema_test.go

@@ -25,6 +25,8 @@ import (
 func TestGenerateSchema_PointerFieldsStripNull(t *testing.T) {
 	t.Parallel()
 
+	// Pointer fields without omitempty are treated as required by the schema
+	// library. null is stripped and they appear as simple non-nullable types.
 	type Params struct {
 		Name  *string  `json:"name"`
 		Count *int     `json:"count"`
@@ -51,7 +53,6 @@ func TestGenerateSchema_PointerFieldsStripNull(t *testing.T) {
 	} {
 		prop := props[field.name].(map[string]any)
 		assert.Equal(t, field.wantType, prop["type"], "field %s", field.name)
-		assert.Nil(t, prop["types"], "field %s should not have union types", field.name)
 	}
 }
 
@@ -142,10 +143,12 @@ func TestGenerateSchema_NestedPointerStruct(t *testing.T) {
 
 	props := schema["properties"].(map[string]any)
 
+	// Pointer-to-struct without omitempty is in required with a simple type.
 	innerProp := props["inner"].(map[string]any)
 	assert.Equal(t, "object", innerProp["type"])
 	assert.Nil(t, innerProp["types"], "pointer to struct should not have union types")
 
+	// Nested pointer field without omitempty is also required and non-nullable.
 	innerProps := innerProp["properties"].(map[string]any)
 	valueProp := innerProps["value"].(map[string]any)
 	assert.Equal(t, "string", valueProp["type"])
@@ -239,11 +242,30 @@ func TestGenerateSchema_RequiredVsOptional(t *testing.T) {
 	var schema map[string]any
 	require.NoError(t, json.Unmarshal(raw, &schema))
 
+	// All properties must appear in required (OpenAI requirement).
 	required := schema["required"].([]any)
 	assert.Contains(t, required, "required")
 	assert.Contains(t, required, "also_needed")
-	assert.NotContains(t, required, "optional")
-	assert.NotContains(t, required, "omit_empty")
+	assert.Contains(t, required, "optional")
+	assert.Contains(t, required, "omit_empty")
+
+	// Optional properties must be nullable so the model can pass null.
+	props := schema["properties"].(map[string]any)
+
+	optionalType := props["optional"].(map[string]any)["type"].([]any)
+	assert.Contains(t, optionalType, "string")
+	assert.Contains(t, optionalType, "null")
+
+	omitEmptyType := props["omit_empty"].(map[string]any)["type"].([]any)
+	assert.Contains(t, omitEmptyType, "string")
+	assert.Contains(t, omitEmptyType, "null")
+
+	// Truly required properties must not be nullable.
+	requiredType := props["required"].(map[string]any)["type"]
+	assert.Equal(t, "string", requiredType)
+
+	alsoNeededType := props["also_needed"].(map[string]any)["type"]
+	assert.Equal(t, "integer", alsoNeededType)
 }
 
 func TestGenerateSchema_DeeplyNestedStructure(t *testing.T) {
@@ -327,8 +349,9 @@ func TestGenerateSchema_SliceOfStructs(t *testing.T) {
 	assert.Contains(t, itemProps, "count")
 
 	countProp := itemProps["count"].(map[string]any)
-	assert.Equal(t, "integer", countProp["type"])
-	assert.Nil(t, countProp["types"])
+	countType := countProp["type"].([]any)
+	assert.Contains(t, countType, "integer")
+	assert.Contains(t, countType, "null")
 	assert.Nil(t, countProp["minimum"])
 }
 

pkg/agent/tool_test.go

@@ -128,7 +128,7 @@ func TestFunctionTool_Definition(t *testing.T) {
 	)
 
 	t.Run(
-		"pointer fields are not nullable in schema",
+		"omitempty fields are nullable and in required",
 		func(t *testing.T) {
 			t.Parallel()
 
@@ -147,10 +147,16 @@ func TestFunctionTool_Definition(t *testing.T) {
 			var schema map[string]any
 			require.NoError(t, json.Unmarshal(tool.Definition().Parameters, &schema))
 
+			// OpenAI requires all properties to be in required; optional
+			// fields are represented as nullable.
+			required := schema["required"].([]any)
+			assert.Contains(t, required, "title")
+
 			props := schema["properties"].(map[string]any)
 			titleProp := props["title"].(map[string]any)
-			assert.Equal(t, "string", titleProp["type"])
-			assert.Nil(t, titleProp["types"])
+			titleType := titleProp["type"].([]any)
+			assert.Contains(t, titleType, "string")
+			assert.Contains(t, titleType, "null")
 		},
 	)
 }