Merge pull request #2362 from dgageot/board/fix-docker-agent-issue-2354-231c9e1e

fix: handle missing type in schema and orphaned function calls in Responses API

Author: dgageot

pkg/model/provider/openai/client.go

@@ -663,6 +663,30 @@ func convertMessagesToResponseInput(messages []chat.Message) []responses.Respons
 			}
 		}
 	}
+	// Safety net: ensure every function_call has a matching function_call_output.
+	// The Responses API rejects requests with orphaned function calls.
+	// This can happen if tool execution was interrupted (e.g. user cancellation).
+	pendingCalls := make(map[string]bool)
+	for _, item := range input {
+		if item.OfFunctionCall != nil {
+			pendingCalls[item.OfFunctionCall.CallID] = true
+		}
+		if item.OfFunctionCallOutput != nil {
+			delete(pendingCalls, item.OfFunctionCallOutput.CallID)
+		}
+	}
+	for callID := range pendingCalls {
+		slog.Warn("Injecting placeholder output for orphaned function call", "call_id", callID)
+		input = append(input, responses.ResponseInputItemUnionParam{
+			OfFunctionCallOutput: &responses.ResponseInputItemFunctionCallOutputParam{
+				CallID: callID,
+				Output: responses.ResponseInputItemFunctionCallOutputOutputUnionParam{
+					OfString: param.NewOpt("(no output — tool call was not executed)"),
+				},
+			},
+		})
+	}
+
 	return input
 }
 

pkg/model/provider/openai/client_test.go

@@ -0,0 +1,71 @@
+package openai
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/docker/docker-agent/pkg/chat"
+	"github.com/docker/docker-agent/pkg/tools"
+)
+
+func TestConvertMessagesToResponseInput_OrphanedFunctionCall(t *testing.T) {
+	// Simulate a conversation where an assistant made 2 tool calls but only
+	// one has a result (the other was cancelled/interrupted).
+	messages := []chat.Message{
+		{Role: chat.MessageRoleUser, Content: "hello"},
+		{
+			Role: chat.MessageRoleAssistant,
+			ToolCalls: []tools.ToolCall{
+				{ID: "call_1", Type: "function", Function: tools.FunctionCall{Name: "tool_a", Arguments: "{}"}},
+				{ID: "call_2", Type: "function", Function: tools.FunctionCall{Name: "tool_b", Arguments: "{}"}},
+			},
+		},
+		{Role: chat.MessageRoleTool, Content: "result a", ToolCallID: "call_1"},
+		// call_2 has no result — orphaned
+	}
+
+	input := convertMessagesToResponseInput(messages)
+
+	// Count function calls and outputs
+	var callIDs, outputIDs []string
+	for _, item := range input {
+		if item.OfFunctionCall != nil {
+			callIDs = append(callIDs, item.OfFunctionCall.CallID)
+		}
+		if item.OfFunctionCallOutput != nil {
+			outputIDs = append(outputIDs, item.OfFunctionCallOutput.CallID)
+		}
+	}
+
+	require.Len(t, callIDs, 2)
+	require.Len(t, outputIDs, 2, "orphaned function call should get a placeholder output")
+
+	assert.Contains(t, outputIDs, "call_1")
+	assert.Contains(t, outputIDs, "call_2")
+}
+
+func TestConvertMessagesToResponseInput_NoOrphans(t *testing.T) {
+	// All tool calls have matching results — no placeholder needed.
+	messages := []chat.Message{
+		{Role: chat.MessageRoleUser, Content: "hello"},
+		{
+			Role: chat.MessageRoleAssistant,
+			ToolCalls: []tools.ToolCall{
+				{ID: "call_1", Type: "function", Function: tools.FunctionCall{Name: "tool_a", Arguments: "{}"}},
+			},
+		},
+		{Role: chat.MessageRoleTool, Content: "result a", ToolCallID: "call_1"},
+	}
+
+	input := convertMessagesToResponseInput(messages)
+
+	var outputCount int
+	for _, item := range input {
+		if item.OfFunctionCallOutput != nil {
+			outputCount++
+		}
+	}
+	assert.Equal(t, 1, outputCount, "should not inject extra outputs when all calls have results")
+}

pkg/model/provider/openai/schema.go

@@ -16,7 +16,7 @@ func ConvertParametersToSchema(params any) (shared.FunctionParameters, error) {
 		return nil, err
 	}
 
-	return fixSchemaArrayItems(removeFormatFields(makeAllRequired(p))), nil
+	return fixSchemaArrayItems(removeFormatFields(ensureTypeFields(makeAllRequired(p)))), nil
 }
 
 // walkSchema calls fn on the given schema node, then recursively walks into
@@ -123,6 +123,23 @@ func makeAllRequired(schema shared.FunctionParameters) shared.FunctionParameters
 	return schema
 }
 
+// ensureTypeFields ensures every schema node that is a map has a "type" key.
+// OpenAI Responses API requires all schema nodes to have an explicit type.
+// Nodes with "properties" default to "object"; other nodes default to "object" as well.
+func ensureTypeFields(schema shared.FunctionParameters) shared.FunctionParameters {
+	if schema == nil {
+		return nil
+	}
+
+	walkSchema(schema, func(node map[string]any) {
+		if _, hasType := node["type"]; !hasType {
+			node["type"] = "object"
+		}
+	})
+
+	return schema
+}
+
 // 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 {

pkg/model/provider/openai/schema_test.go

@@ -372,6 +372,58 @@ func TestMakeAllRequired_TypeArrayWithObject(t *testing.T) {
 	assert.Equal(t, []string{"number", "null"}, age["type"])
 }
 
+func TestEnsureTypeFields_AdditionalPropertiesMissingType(t *testing.T) {
+	// Reproduces the Notion MCP notion-search tool schema where
+	// filters.additionalProperties is an object schema without a "type" key.
+	// OpenAI Responses API rejects schemas missing "type".
+	schema := shared.FunctionParameters{
+		"type": "object",
+		"properties": map[string]any{
+			"filters": map[string]any{
+				"type": "object",
+				"additionalProperties": map[string]any{
+					// No "type" key here — this is the bug
+					"properties": map[string]any{
+						"property": map[string]any{"type": "string"},
+					},
+				},
+			},
+		},
+		"required": []any{"filters"},
+	}
+
+	updated := ensureTypeFields(schema)
+
+	filters := updated["properties"].(map[string]any)["filters"].(map[string]any)
+	additionalProps := filters["additionalProperties"].(map[string]any)
+	assert.Equal(t, "object", additionalProps["type"], "additionalProperties should have type added")
+}
+
+func TestConvertParametersToSchema_AdditionalPropertiesMissingType(t *testing.T) {
+	// End-to-end test: the full pipeline should produce a valid schema
+	schema := map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"filters": map[string]any{
+				"type": "object",
+				"additionalProperties": map[string]any{
+					"properties": map[string]any{
+						"property": map[string]any{"type": "string"},
+					},
+				},
+			},
+		},
+		"required": []any{"filters"},
+	}
+
+	result, err := ConvertParametersToSchema(schema)
+	require.NoError(t, err)
+
+	filters := result["properties"].(map[string]any)["filters"].(map[string]any)
+	additionalProps := filters["additionalProperties"].(map[string]any)
+	assert.Equal(t, "object", additionalProps["type"])
+}
+
 func TestFixSchemaArrayItems(t *testing.T) {
 	schema := `{
   "properties": {

pkg/runtime/tool_dispatch.go

@@ -35,7 +35,7 @@ func (r *LocalRuntime) processToolCalls(ctx context.Context, sess *session.Sessi
 		agentToolMap[t.Name] = t
 	}
 
-	for _, toolCall := range calls {
+	for i, toolCall := range calls {
 		callCtx, callSpan := r.startSpan(ctx, "runtime.tool.call", trace.WithAttributes(
 			attribute.String("tool.name", toolCall.Function.Name),
 			attribute.String("tool.type", string(toolCall.Type)),
@@ -74,6 +74,14 @@ func (r *LocalRuntime) processToolCalls(ctx context.Context, sess *session.Sessi
 		if canceled {
 			callSpan.SetStatus(codes.Ok, "tool call canceled by user")
 			callSpan.End()
+
+			// Add error results for remaining unprocessed tool calls so the
+			// conversation history doesn't contain orphaned function calls
+			// without matching outputs (which the Responses API rejects).
+			for _, remaining := range calls[i+1:] {
+				remainingTool := agentToolMap[remaining.Function.Name]
+				r.addToolErrorResponse(ctx, sess, remaining, remainingTool, events, a, "The tool call was canceled because a previous tool call in the same batch was canceled by the user.")
+			}
 			return
 		}