Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 41 additions & 3 deletions core/bifrost.go
Original file line number Diff line number Diff line change
Expand Up @@ -1693,7 +1693,7 @@ func (bifrost *Bifrost) RegisterMCPTool(name, description string, handler func(a
return bifrost.mcpManager.RegisterTool(name, description, handler, toolSchema)
}

// ExecuteMCPTool executes an MCP tool call and returns the result as a tool message.
// ExecuteChatMCPTool executes an MCP tool call and returns the result as a chat message.
// This is the main public API for manual MCP tool execution.
//
// Parameters:
Expand All @@ -1703,7 +1703,7 @@ func (bifrost *Bifrost) RegisterMCPTool(name, description string, handler func(a
// Returns:
// - schemas.ChatMessage: Tool message with execution result
// - schemas.BifrostError: Any execution error
func (bifrost *Bifrost) ExecuteMCPTool(ctx context.Context, toolCall schemas.ChatAssistantMessageToolCall) (*schemas.ChatMessage, *schemas.BifrostError) {
func (bifrost *Bifrost) ExecuteChatMCPTool(ctx context.Context, toolCall schemas.ChatAssistantMessageToolCall) (*schemas.ChatMessage, *schemas.BifrostError) {
if bifrost.mcpManager == nil {
return nil, &schemas.BifrostError{
IsBifrostError: false,
Expand All @@ -1716,7 +1716,7 @@ func (bifrost *Bifrost) ExecuteMCPTool(ctx context.Context, toolCall schemas.Cha
}
}

result, err := bifrost.mcpManager.ExecuteTool(ctx, toolCall)
result, err := bifrost.mcpManager.ExecuteChatTool(ctx, toolCall)
if err != nil {
return nil, &schemas.BifrostError{
IsBifrostError: false,
Expand All @@ -1732,6 +1732,44 @@ func (bifrost *Bifrost) ExecuteMCPTool(ctx context.Context, toolCall schemas.Cha
return result, nil
}

// ExecuteResponsesMCPTool executes an MCP tool call and returns the result as a responses message.

// Parameters:
// - ctx: Execution context
// - toolCall: The tool call to execute (from assistant message)
//
// Returns:
// - schemas.ResponsesMessage: Tool message with execution result
// - schemas.BifrostError: Any execution error
func (bifrost *Bifrost) ExecuteResponsesMCPTool(ctx context.Context, toolCall *schemas.ResponsesToolMessage) (*schemas.ResponsesMessage, *schemas.BifrostError) {
if bifrost.mcpManager == nil {
return nil, &schemas.BifrostError{
IsBifrostError: false,
Error: &schemas.ErrorField{
Message: "MCP is not configured in this Bifrost instance",
},
ExtraFields: schemas.BifrostErrorExtraFields{
RequestType: schemas.ResponsesRequest, // MCP tools are used with responses requests
},
}
}

result, err := bifrost.mcpManager.ExecuteResponsesTool(ctx, toolCall)
if err != nil {
return nil, &schemas.BifrostError{
IsBifrostError: false,
Error: &schemas.ErrorField{
Message: err.Error(),
},
ExtraFields: schemas.BifrostErrorExtraFields{
RequestType: schemas.ResponsesRequest, // MCP tools are used with responses requests
},
}
}

return result, nil
}

// IMPORTANT: Running the MCP client management operations (GetMCPClients, AddMCPClient, RemoveMCPClient, EditMCPClientTools)
// may temporarily increase latency for incoming requests while the operations are being processed.
// These operations involve network I/O and connection management that require mutex locks
Expand Down
2 changes: 1 addition & 1 deletion core/chatbot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ func (s *ChatSession) handleToolCalls(assistantMessage schemas.ChatMessage) (str
stopChan, wg := startLoader()

// Execute the tool using Bifrost's integrated MCP functionality
toolResult, err := s.client.ExecuteMCPTool(context.Background(), toolCall)
toolResult, err := s.client.ExecuteChatMCPTool(context.Background(), toolCall)

// Stop loading animation
stopLoader(stopChan, wg)
Expand Down
38 changes: 34 additions & 4 deletions core/mcp/agent_adaptors.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,20 @@ import (
"github.com/maximhq/bifrost/core/schemas"
)

// agentAPIAdapter defines the interface for API-specific operations
// agentAPIAdapter defines the interface for API-specific operations in agent mode.
// This adapter pattern allows the agent execution logic to work with both Chat Completions
// and Responses APIs without requiring API-specific code in the agent loop.
//
// The adapter handles format conversions at the boundaries:
// - Responses API requests/responses are converted to/from Chat API format
// - Tool calls are extracted in Chat format for uniform processing
// - Results are converted back to the original API format for the response
//
// This design ensures that:
// 1. Tool execution logic is format-agnostic
// 2. Both APIs have feature parity
// 3. Conversions are localized to adapters
// 4. The agent loop remains API-neutral
type agentAPIAdapter interface {
// Extract conversation history from the original request
getConversationHistory() []interface{}
Expand All @@ -22,13 +35,17 @@ type agentAPIAdapter interface {
// Check if response has tool calls
hasToolCalls(response interface{}) bool

// Extract tool calls from response
// Extract tool calls from response.
// For Chat API: Returns tool calls directly from the response.
// For Responses API: Converts ResponsesMessage tool calls to ChatAssistantMessageToolCall for processing.
extractToolCalls(response interface{}) []schemas.ChatAssistantMessageToolCall

// Add assistant message with tool calls to conversation
addAssistantMessage(conversation []interface{}, response interface{}) []interface{}

// Add tool results to conversation
// Add tool results to conversation.
// For Chat API: Adds ChatMessage results directly.
// For Responses API: Converts ChatMessage results to ResponsesMessage via ToResponsesToolMessage().
addToolResults(conversation []interface{}, toolResults []*schemas.ChatMessage) []interface{}

// Create new request with updated conversation
Expand All @@ -53,7 +70,20 @@ type chatAPIAdapter struct {
makeReq func(ctx context.Context, req *schemas.BifrostChatRequest) (*schemas.BifrostChatResponse, *schemas.BifrostError)
}

// responsesAPIAdapter implements agentAPIAdapter for Responses API
// responsesAPIAdapter implements agentAPIAdapter for Responses API.
// It enables the agent mode execution loop to work with Responses API requests and responses
// by handling format conversions transparently.
//
// Key conversions performed:
// - extractToolCalls(): Converts ResponsesMessage tool calls to ChatAssistantMessageToolCall
// via BifrostResponsesResponse.ToBifrostChatResponse() and existing extraction logic
// - addToolResults(): Converts ChatMessage tool results back to ResponsesMessage
// via ChatMessage.ToResponsesMessages() and ToResponsesToolMessage()
// - createNewRequest(): Builds a new BifrostResponsesRequest from converted conversation
// - createResponseWithExecutedTools(): Creates a Responses response with results and pending tools
//
// This adapter enables full feature parity between Chat Completions and Responses APIs
// for tool execution in agent mode.
type responsesAPIAdapter struct {
originalReq *schemas.BifrostResponsesRequest
initialResponse *schemas.BifrostResponsesResponse
Expand Down
239 changes: 239 additions & 0 deletions core/mcp/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package mcp

import (
"context"
"encoding/json"
"testing"

"github.com/maximhq/bifrost/core/schemas"
Expand Down Expand Up @@ -478,3 +479,241 @@ func TestExecuteAgentForResponsesRequest_WithNonAutoExecutableTools(t *testing.T
t.Errorf("Expected 0 LLM calls for non-auto-executable tools, got %d", llmCaller.responsesCallCount)
}
}

// ============================================================================
// CONVERTER TESTS (Phase 2)
// ============================================================================

// TestResponsesToolMessageToChatAssistantMessageToolCall tests conversion of Responses tool message to Chat tool call
func TestResponsesToolMessageToChatAssistantMessageToolCall(t *testing.T) {
// Test with valid tool message
responsesToolMsg := &schemas.ResponsesToolMessage{
CallID: schemas.Ptr("call-123"),
Name: schemas.Ptr("calculate"),
Arguments: schemas.Ptr("{\"x\": 10, \"y\": 20}"),
}

chatToolCall := responsesToolMsg.ToChatAssistantMessageToolCall()

if chatToolCall == nil {
t.Fatal("Expected non-nil ChatAssistantMessageToolCall")
}

if chatToolCall.Type == nil || *chatToolCall.Type != "function" {
t.Errorf("Expected Type 'function', got %v", chatToolCall.Type)
}

if chatToolCall.Function.Name == nil || *chatToolCall.Function.Name != "calculate" {
t.Errorf("Expected Name 'calculate', got %v", chatToolCall.Function.Name)
}

if chatToolCall.Function.Arguments != `{"x": 10, "y": 20}` {
t.Errorf("Expected Arguments '{\"x\": 10, \"y\": 20}', got %s", chatToolCall.Function.Arguments)
}
}

// TestResponsesToolMessageToChatAssistantMessageToolCall_Nil tests nil handling
func TestResponsesToolMessageToChatAssistantMessageToolCall_Nil(t *testing.T) {
responsesToolMsg := &schemas.ResponsesToolMessage{
CallID: schemas.Ptr("call-123"),
Name: schemas.Ptr("calculate"),
Arguments: nil, // Test nil Arguments case
}

chatToolCall := responsesToolMsg.ToChatAssistantMessageToolCall()
if chatToolCall == nil {
t.Fatal("Expected non-nil ChatAssistantMessageToolCall")
}

// Assert that nil Arguments produces a valid empty JSON object
if chatToolCall.Function.Arguments != "{}" {
t.Errorf("Expected Arguments '{}' for nil input, got %q", chatToolCall.Function.Arguments)
}

// Verify it's valid JSON by attempting to unmarshal
var args map[string]interface{}
if err := json.Unmarshal([]byte(chatToolCall.Function.Arguments), &args); err != nil {
t.Errorf("Expected valid JSON, but unmarshaling failed: %v", err)
}
}

// TestChatMessageToResponsesToolMessage tests conversion of Chat tool result to Responses tool message
func TestChatMessageToResponsesToolMessage(t *testing.T) {
// Test with valid chat tool message
chatMsg := &schemas.ChatMessage{
Role: schemas.ChatMessageRoleTool,
ChatToolMessage: &schemas.ChatToolMessage{
ToolCallID: schemas.Ptr("call-123"),
},
Content: &schemas.ChatMessageContent{
ContentStr: schemas.Ptr("Result: 30"),
},
}

responsesMsg := chatMsg.ToResponsesToolMessage()

if responsesMsg == nil {
t.Fatal("Expected non-nil ResponsesMessage")
}

if responsesMsg.Type == nil || *responsesMsg.Type != schemas.ResponsesMessageTypeFunctionCallOutput {
t.Errorf("Expected Type 'function_call_output', got %v", responsesMsg.Type)
}

if responsesMsg.ResponsesToolMessage == nil {
t.Fatal("Expected non-nil ResponsesToolMessage")
}

if responsesMsg.ResponsesToolMessage.CallID == nil || *responsesMsg.ResponsesToolMessage.CallID != "call-123" {
t.Errorf("Expected CallID 'call-123', got %v", responsesMsg.ResponsesToolMessage.CallID)
}

if responsesMsg.ResponsesToolMessage.Output == nil {
t.Fatal("Expected non-nil Output")
}

if responsesMsg.ResponsesToolMessage.Output.ResponsesToolCallOutputStr == nil {
t.Fatal("Expected non-nil ResponsesToolCallOutputStr")
}

if *responsesMsg.ResponsesToolMessage.Output.ResponsesToolCallOutputStr != "Result: 30" {
t.Errorf("Expected Output 'Result: 30', got %s", *responsesMsg.ResponsesToolMessage.Output.ResponsesToolCallOutputStr)
}
}

// TestChatMessageToResponsesToolMessage_Nil tests nil handling
func TestChatMessageToResponsesToolMessage_Nil(t *testing.T) {
var chatMsg *schemas.ChatMessage

responsesMsg := chatMsg.ToResponsesToolMessage()

if responsesMsg != nil {
t.Errorf("Expected nil for nil input, got %v", responsesMsg)
}
}

// TestChatMessageToResponsesToolMessage_NoToolMessage tests with non-tool message
func TestChatMessageToResponsesToolMessage_NoToolMessage(t *testing.T) {
// Chat message without ChatToolMessage
chatMsg := &schemas.ChatMessage{
Role: schemas.ChatMessageRoleAssistant,
}

responsesMsg := chatMsg.ToResponsesToolMessage()

if responsesMsg != nil {
t.Errorf("Expected nil for non-tool message, got %v", responsesMsg)
}
}

// ============================================================================
// RESPONSES API TOOL CONVERSION TESTS (Phase 3)
// ============================================================================

// TestExecuteAgentForResponsesRequest_ConversionRoundTrip tests that tool calls survive format conversion
// This is a unit test of the conversion logic only, not full agent execution
func TestExecuteAgentForResponsesRequest_ConversionRoundTrip(t *testing.T) {
// Create a tool message in Responses format
responsesToolMsg := &schemas.ResponsesToolMessage{
CallID: schemas.Ptr("call-456"),
Name: schemas.Ptr("readToolFile"),
Arguments: schemas.Ptr("{\"file\": \"test.txt\"}"),
}

// Step 1: Convert Responses format to Chat format
chatToolCall := responsesToolMsg.ToChatAssistantMessageToolCall()

if chatToolCall == nil {
t.Fatal("Failed to convert Responses to Chat format")
}

if *chatToolCall.ID != "call-456" {
t.Errorf("ID lost in conversion: expected 'call-456', got %s", *chatToolCall.ID)
}

if *chatToolCall.Function.Name != "readToolFile" {
t.Errorf("Name lost in conversion: expected 'readToolFile', got %s", *chatToolCall.Function.Name)
}

if chatToolCall.Function.Arguments != "{\"file\": \"test.txt\"}" {
t.Errorf("Arguments lost in conversion: expected '%s', got %s",
"{\"file\": \"test.txt\"}", chatToolCall.Function.Arguments)
}

// Step 2: Simulate tool execution by creating a result message
chatResultMsg := &schemas.ChatMessage{
Role: schemas.ChatMessageRoleTool,
ChatToolMessage: &schemas.ChatToolMessage{
ToolCallID: chatToolCall.ID,
},
Content: &schemas.ChatMessageContent{
ContentStr: schemas.Ptr("File contents here"),
},
}

// Step 3: Convert tool result back to Responses format
responsesResultMsg := chatResultMsg.ToResponsesToolMessage()

if responsesResultMsg == nil {
t.Fatal("Failed to convert Chat result to Responses format")
}

if responsesResultMsg.ResponsesToolMessage.CallID == nil {
t.Error("CallID lost in round-trip conversion")
} else if *responsesResultMsg.ResponsesToolMessage.CallID != "call-456" {
t.Errorf("CallID changed in round-trip: expected 'call-456', got %s", *responsesResultMsg.ResponsesToolMessage.CallID)
}

// Verify output is preserved
if responsesResultMsg.ResponsesToolMessage.Output == nil {
t.Error("Output lost in conversion")
} else if responsesResultMsg.ResponsesToolMessage.Output.ResponsesToolCallOutputStr == nil {
t.Error("Output content lost in conversion")
} else if *responsesResultMsg.ResponsesToolMessage.Output.ResponsesToolCallOutputStr != "File contents here" {
t.Errorf("Output content changed: expected 'File contents here', got %s",
*responsesResultMsg.ResponsesToolMessage.Output.ResponsesToolCallOutputStr)
}

// Verify message type is correct
if responsesResultMsg.Type == nil || *responsesResultMsg.Type != schemas.ResponsesMessageTypeFunctionCallOutput {
t.Errorf("Expected message type 'function_call_output', got %v", responsesResultMsg.Type)
}
}

// TestExecuteAgentForResponsesRequest_OutputStructured tests conversion with structured output blocks
func TestExecuteAgentForResponsesRequest_OutputStructured(t *testing.T) {
chatResultMsg := &schemas.ChatMessage{
Role: schemas.ChatMessageRoleTool,
ChatToolMessage: &schemas.ChatToolMessage{
ToolCallID: schemas.Ptr("call-789"),
},
Content: &schemas.ChatMessageContent{
ContentBlocks: []schemas.ChatContentBlock{
{
Type: schemas.ChatContentBlockTypeText,
Text: schemas.Ptr("Block 1"),
},
{
Type: schemas.ChatContentBlockTypeText,
Text: schemas.Ptr("Block 2"),
},
},
},
}

responsesMsg := chatResultMsg.ToResponsesToolMessage()

if responsesMsg == nil {
t.Fatal("Expected non-nil ResponsesMessage for structured output")
}

if responsesMsg.ResponsesToolMessage.Output == nil {
t.Fatal("Expected non-nil Output for structured content")
}

if responsesMsg.ResponsesToolMessage.Output.ResponsesFunctionToolCallOutputBlocks == nil {
t.Error("Expected output blocks for structured content")
} else if len(responsesMsg.ResponsesToolMessage.Output.ResponsesFunctionToolCallOutputBlocks) != 2 {
t.Errorf("Expected 2 output blocks, got %d", len(responsesMsg.ResponsesToolMessage.Output.ResponsesFunctionToolCallOutputBlocks))
}
}
Loading