Merge pull request #1871 from vvoland/fix-notion-openai

Fix `mcp/notion` not working with OpenAI models

Author: rumpl

pkg/model/provider/openai/schema.go

@@ -20,7 +20,7 @@ func ConvertParametersToSchema(params any) (shared.FunctionParameters, error) {
 }
 
 // walkSchema calls fn on the given schema node, then recursively walks into
-// properties, anyOf/oneOf/allOf variants, and array items.
+// properties, anyOf/oneOf/allOf variants, array items, and additionalProperties.
 func walkSchema(schema map[string]any, fn func(map[string]any)) {
 	fn(schema)
 
@@ -45,6 +45,11 @@ func walkSchema(schema map[string]any, fn func(map[string]any)) {
 	if items, ok := schema["items"].(map[string]any); ok {
 		walkSchema(items, fn)
 	}
+
+	// additionalProperties can be a boolean or an object schema
+	if additionalProps, ok := schema["additionalProperties"].(map[string]any); ok {
+		walkSchema(additionalProps, fn)
+	}
 }
 
 // makeAllRequired makes all object properties "required" throughout the schema,

pkg/model/provider/openai/schema_test.go

@@ -167,6 +167,48 @@ func TestMakeAllRequired_ArrayItems(t *testing.T) {
 	assert.Contains(t, itemRequired, "name")
 }
 
+func TestMakeAllRequired_AdditionalProperties(t *testing.T) {
+	// Reproduces the Notion MCP tool schema where additionalProperties
+	// contains an object schema with its own properties (like bulleted_list_item).
+	// OpenAI requires all properties in additionalProperties schemas to also
+	// be listed in the required array.
+	schema := shared.FunctionParameters{
+		"type": "object",
+		"properties": map[string]any{
+			"children": map[string]any{
+				"type": "object",
+				"additionalProperties": map[string]any{
+					"type": "object",
+					"properties": map[string]any{
+						"bulleted_list_item": map[string]any{"type": "string"},
+						"numbered_list_item": map[string]any{"type": "string"},
+					},
+					"required": []any{"bulleted_list_item"},
+				},
+			},
+		},
+		"required": []any{"children"},
+	}
+
+	updated := makeAllRequired(schema)
+
+	// additionalProperties object: all properties must be required
+	children := updated["properties"].(map[string]any)["children"].(map[string]any)
+	additionalProps := children["additionalProperties"].(map[string]any)
+	additionalRequired := additionalProps["required"].([]any)
+	assert.Len(t, additionalRequired, 2)
+	assert.Contains(t, additionalRequired, "bulleted_list_item")
+	assert.Contains(t, additionalRequired, "numbered_list_item")
+
+	// numbered_list_item was not originally required, so its type should be nullable
+	numberedListItem := additionalProps["properties"].(map[string]any)["numbered_list_item"].(map[string]any)
+	assert.Equal(t, []string{"string", "null"}, numberedListItem["type"])
+
+	// bulleted_list_item was originally required, so its type should be unchanged
+	bulletedListItem := additionalProps["properties"].(map[string]any)["bulleted_list_item"].(map[string]any)
+	assert.Equal(t, "string", bulletedListItem["type"])
+}
+
 func TestRemoveFormatFields(t *testing.T) {
 	schema := map[string]any{
 		"type": "object",