From c5b2f18f66049140ea3a181abc1ff459f37bb19f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Bergstro=CC=88m?= Date: Mon, 11 Aug 2025 21:07:37 +0200 Subject: [PATCH 1/2] fix: add missing allowed_tools and require_approval parameters to ToolParamOfMcp The ToolParamOfMcp function was missing critical parameters (allowed_tools and require_approval) that are required for proper MCP integration according to OpenAI's documentation. This is a breaking change that adds two new required parameters: - allowedTools []string: List of specific tools that can be called - requireApproval string: Controls tool approval behavior (never/always/auto) Without these parameters, MCP integration fails even though tool discovery works. The function now properly populates the ToolMcpParam struct fields that were already available but not being set. Breaking Change: Function signature changed from: ToolParamOfMcp(serverLabel, serverURL string) To: ToolParamOfMcp(serverLabel, serverURL string, allowedTools []string, requireApproval string) Fixes MCP tool execution issues where requests would fail with 500 errors or never reach the MCP server despite successful tool discovery. --- responses/response.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/responses/response.go b/responses/response.go index 80a7d0b0..317491c1 100644 --- a/responses/response.go +++ b/responses/response.go @@ -12137,10 +12137,20 @@ func ToolParamOfComputerUsePreview(displayHeight int64, displayWidth int64, envi return ToolUnionParam{OfComputerUsePreview: &computerUsePreview} } -func ToolParamOfMcp(serverLabel string, serverURL string) ToolUnionParam { +func ToolParamOfMcp(serverLabel string, serverURL string, allowedTools []string, requireApproval string) ToolUnionParam { var mcp ToolMcpParam mcp.ServerLabel = serverLabel mcp.ServerURL = serverURL + if len(allowedTools) > 0 { + mcp.AllowedTools = ToolMcpAllowedToolsUnionParam{ + OfMcpAllowedTools: allowedTools, + } + } + if requireApproval != "" { + mcp.RequireApproval = ToolMcpRequireApprovalUnionParam{ + OfMcpToolApprovalSetting: param.NewOpt(requireApproval), + } + } return ToolUnionParam{OfMcp: &mcp} } From 363c3ddbe0b4e00b6731e5d7332db8cf69f1f062 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Bergstro=CC=88m?= Date: Mon, 11 Aug 2025 21:23:18 +0200 Subject: [PATCH 2/2] test: add comprehensive unit tests for ToolParamOfMcp function Tests verify that the function correctly handles: - All parameters provided (serverLabel, serverURL, allowedTools, requireApproval) - Empty allowedTools slice (should be omitted) - Nil allowedTools slice (should be omitted) - Empty requireApproval string (should be omitted) - Minimal parameters (both optional fields empty) Ensures the function properly sets AllowedTools and RequireApproval fields only when non-empty values are provided, using the param.Opt pattern correctly. --- responses/response_test.go | 147 +++++++++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) diff --git a/responses/response_test.go b/responses/response_test.go index 7b73951f..f34197c1 100644 --- a/responses/response_test.go +++ b/responses/response_test.go @@ -11,6 +11,7 @@ import ( "github.com/openai/openai-go/v2" "github.com/openai/openai-go/v2/internal/testutil" "github.com/openai/openai-go/v2/option" + "github.com/openai/openai-go/v2/packages/param" "github.com/openai/openai-go/v2/responses" "github.com/openai/openai-go/v2/shared" ) @@ -170,3 +171,149 @@ func TestResponseCancel(t *testing.T) { t.Fatalf("err should be nil: %s", err.Error()) } } + +func TestToolParamOfMcp(t *testing.T) { + // Test with all parameters + t.Run("with all parameters", func(t *testing.T) { + serverLabel := "test-server" + serverURL := "https://test-mcp-server.com" + allowedTools := []string{"search_products", "send_email"} + requireApproval := "never" + + tool := responses.ToolParamOfMcp(serverLabel, serverURL, allowedTools, requireApproval) + + if tool.OfMcp == nil { + t.Fatal("Expected OfMcp to be non-nil") + } + + mcp := tool.OfMcp + if mcp.ServerLabel != serverLabel { + t.Errorf("Expected ServerLabel to be %s, got %s", serverLabel, mcp.ServerLabel) + } + + if mcp.ServerURL != serverURL { + t.Errorf("Expected ServerURL to be %s, got %s", serverURL, mcp.ServerURL) + } + + if param.IsOmitted(mcp.AllowedTools) { + t.Error("Expected AllowedTools to be set") + } else { + if mcp.AllowedTools.OfMcpAllowedTools == nil { + t.Error("Expected OfMcpAllowedTools to be set") + } else { + tools := mcp.AllowedTools.OfMcpAllowedTools + if len(tools) != 2 || tools[0] != "search_products" || tools[1] != "send_email" { + t.Errorf("Expected allowedTools to be %v, got %v", allowedTools, tools) + } + } + } + + if param.IsOmitted(mcp.RequireApproval) { + t.Error("Expected RequireApproval to be set") + } else { + if param.IsOmitted(mcp.RequireApproval.OfMcpToolApprovalSetting) { + t.Error("Expected OfMcpToolApprovalSetting to be set") + } else { + if mcp.RequireApproval.OfMcpToolApprovalSetting.Value != requireApproval { + t.Errorf("Expected requireApproval to be %s, got %s", requireApproval, mcp.RequireApproval.OfMcpToolApprovalSetting.Value) + } + } + } + }) + + // Test with empty allowedTools + t.Run("with empty allowed tools", func(t *testing.T) { + serverLabel := "test-server" + serverURL := "https://test-mcp-server.com" + var allowedTools []string // empty slice + requireApproval := "auto" + + tool := responses.ToolParamOfMcp(serverLabel, serverURL, allowedTools, requireApproval) + + if tool.OfMcp == nil { + t.Fatal("Expected OfMcp to be non-nil") + } + + mcp := tool.OfMcp + if !param.IsOmitted(mcp.AllowedTools) { + t.Error("Expected AllowedTools to be omitted when empty slice provided") + } + + if param.IsOmitted(mcp.RequireApproval) { + t.Error("Expected RequireApproval to be set") + } + }) + + // Test with nil allowedTools + t.Run("with nil allowed tools", func(t *testing.T) { + serverLabel := "test-server" + serverURL := "https://test-mcp-server.com" + var allowedTools []string = nil // nil slice + requireApproval := "always" + + tool := responses.ToolParamOfMcp(serverLabel, serverURL, allowedTools, requireApproval) + + if tool.OfMcp == nil { + t.Fatal("Expected OfMcp to be non-nil") + } + + mcp := tool.OfMcp + if !param.IsOmitted(mcp.AllowedTools) { + t.Error("Expected AllowedTools to be omitted when nil slice provided") + } + }) + + // Test with empty requireApproval + t.Run("with empty require approval", func(t *testing.T) { + serverLabel := "test-server" + serverURL := "https://test-mcp-server.com" + allowedTools := []string{"search_products"} + requireApproval := "" // empty string + + tool := responses.ToolParamOfMcp(serverLabel, serverURL, allowedTools, requireApproval) + + if tool.OfMcp == nil { + t.Fatal("Expected OfMcp to be non-nil") + } + + mcp := tool.OfMcp + if !param.IsOmitted(mcp.RequireApproval) { + t.Error("Expected RequireApproval to be omitted when empty string provided") + } + + if param.IsOmitted(mcp.AllowedTools) { + t.Error("Expected AllowedTools to be set") + } + }) + + // Test minimal case (both optional parameters empty) + t.Run("minimal parameters", func(t *testing.T) { + serverLabel := "minimal-server" + serverURL := "https://minimal-server.com" + var allowedTools []string // empty + requireApproval := "" // empty + + tool := responses.ToolParamOfMcp(serverLabel, serverURL, allowedTools, requireApproval) + + if tool.OfMcp == nil { + t.Fatal("Expected OfMcp to be non-nil") + } + + mcp := tool.OfMcp + if mcp.ServerLabel != serverLabel { + t.Errorf("Expected ServerLabel to be %s, got %s", serverLabel, mcp.ServerLabel) + } + + if mcp.ServerURL != serverURL { + t.Errorf("Expected ServerURL to be %s, got %s", serverURL, mcp.ServerURL) + } + + if !param.IsOmitted(mcp.AllowedTools) { + t.Error("Expected AllowedTools to be omitted") + } + + if !param.IsOmitted(mcp.RequireApproval) { + t.Error("Expected RequireApproval to be omitted") + } + }) +}