From bc181f1f52068dcc7ea2b3ad7ccfba139b886ebd Mon Sep 17 00:00:00 2001 From: Travis Cline Date: Fri, 29 Aug 2025 18:13:35 +0200 Subject: [PATCH] agents,llms/anthropic: prevent panics in agent parsing and Anthropic responses (#1380) - Add nil/bounds checks in OpenAI Functions Agent ParseOutput - Handle multiple tool calls properly instead of just first one - Fix Anthropic panic when content is nil or empty (fixes #993) - Prevent index out of bounds errors in agent parsing --- agents/openai_functions_agent.go | 21 ++++---- agents/openai_functions_agent_test.go | 69 +++++++++++++++++++++++++++ llms/anthropic/anthropicllm.go | 2 +- llms/anthropic/anthropicllm_test.go | 7 +++ 4 files changed, 90 insertions(+), 9 deletions(-) diff --git a/agents/openai_functions_agent.go b/agents/openai_functions_agent.go index f03635c5..a5bf8f4c 100644 --- a/agents/openai_functions_agent.go +++ b/agents/openai_functions_agent.go @@ -120,15 +120,17 @@ func (o *OpenAIFunctionsAgent) Plan( case llms.AIChatMessage: if len(p.ToolCalls) > 0 { + toolCallParts := make([]llms.ContentPart, 0, len(p.ToolCalls)) + for _, tc := range p.ToolCalls { + toolCallParts = append(toolCallParts, llms.ToolCall{ + ID: tc.ID, + Type: tc.Type, + FunctionCall: tc.FunctionCall, + }) + } mc = llms.MessageContent{ - Role: role, - Parts: []llms.ContentPart{ - llms.ToolCall{ - ID: p.ToolCalls[0].ID, - Type: p.ToolCalls[0].Type, - FunctionCall: p.ToolCalls[0].FunctionCall, - }, - }, + Role: role, + Parts: toolCallParts, } } else { mc = llms.MessageContent{ @@ -256,6 +258,9 @@ func (o *OpenAIFunctionsAgent) constructScratchPad(steps []schema.AgentStep) []l func (o *OpenAIFunctionsAgent) ParseOutput(contentResp *llms.ContentResponse) ( []schema.AgentAction, *schema.AgentFinish, error, ) { + if contentResp == nil || len(contentResp.Choices) == 0 { + return nil, nil, fmt.Errorf("no choices in response") + } choice := contentResp.Choices[0] // Check for new-style tool calls first diff --git a/agents/openai_functions_agent_test.go b/agents/openai_functions_agent_test.go index 25e74f89..34ac9664 100644 --- a/agents/openai_functions_agent_test.go +++ b/agents/openai_functions_agent_test.go @@ -12,6 +12,7 @@ import ( "github.com/tmc/langchaingo/agents" "github.com/tmc/langchaingo/chains" "github.com/tmc/langchaingo/internal/httprr" + "github.com/tmc/langchaingo/llms" "github.com/tmc/langchaingo/llms/openai" "github.com/tmc/langchaingo/prompts" "github.com/tmc/langchaingo/tools" @@ -140,3 +141,71 @@ func TestOpenAIFunctionsAgentComplexCalculation(t *testing.T) { t.Errorf("expected calculation result 30 in response, got: %s", result) } } + +// TestOpenAIFunctionsAgent_ParseOutput_NilResponse tests that ParseOutput handles nil response gracefully +func TestOpenAIFunctionsAgent_ParseOutput_NilResponse(t *testing.T) { + t.Parallel() + agent := &agents.OpenAIFunctionsAgent{} + + // Test with nil response - should return error instead of panic + _, _, err := agent.ParseOutput(nil) + if err == nil { + t.Error("expected error for nil response") + } +} + +// TestOpenAIFunctionsAgent_ParseOutput_EmptyChoices tests that ParseOutput handles empty choices gracefully +func TestOpenAIFunctionsAgent_ParseOutput_EmptyChoices(t *testing.T) { + t.Parallel() + agent := &agents.OpenAIFunctionsAgent{} + + // Test with empty choices - should return error instead of panic + resp := &llms.ContentResponse{ + Choices: []*llms.ContentChoice{}, + } + _, _, err := agent.ParseOutput(resp) + if err == nil { + t.Error("expected error for empty choices") + } +} + +// TestOpenAIFunctionsAgent_ParseOutput_MultipleToolCalls tests multiple tool calls handling +func TestOpenAIFunctionsAgent_ParseOutput_MultipleToolCalls(t *testing.T) { + t.Parallel() + agent := &agents.OpenAIFunctionsAgent{} + + // Test multiple tool calls - should handle all calls, not just first one + resp := &llms.ContentResponse{ + Choices: []*llms.ContentChoice{ + { + ToolCalls: []llms.ToolCall{ + { + ID: "call1", + FunctionCall: &llms.FunctionCall{ + Name: "calculator", + Arguments: `{"__arg1": "2+2"}`, + }, + }, + { + ID: "call2", + FunctionCall: &llms.FunctionCall{ + Name: "weather", + Arguments: `{"__arg1": "Seattle"}`, + }, + }, + }, + }, + }, + } + + actions, finish, err := agent.ParseOutput(resp) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if finish != nil { + t.Error("expected actions, got finish") + } + if len(actions) != 2 { + t.Errorf("expected 2 actions, got %d", len(actions)) + } +} diff --git a/llms/anthropic/anthropicllm.go b/llms/anthropic/anthropicllm.go index 9f146bdb..563cf869 100644 --- a/llms/anthropic/anthropicllm.go +++ b/llms/anthropic/anthropicllm.go @@ -153,7 +153,7 @@ func generateMessagesContent(ctx context.Context, o *LLM, messages []llms.Messag } return nil, fmt.Errorf("anthropic: failed to create message: %w", err) } - if result == nil { + if result == nil || len(result.Content) == 0 { return nil, ErrEmptyResponse } diff --git a/llms/anthropic/anthropicllm_test.go b/llms/anthropic/anthropicllm_test.go index 42f82010..d0750622 100644 --- a/llms/anthropic/anthropicllm_test.go +++ b/llms/anthropic/anthropicllm_test.go @@ -223,3 +223,10 @@ func TestCall(t *testing.T) { // Test that Call delegates to GenerateContent t.Skip("Call() requires integration testing with mock client") } + +func TestGenerateMessagesContent_EmptyContent(t *testing.T) { + // This test demonstrates the need for checking len(result.Content) == 0 + // Without the fix, accessing result.Content[0] would panic when Anthropic + // returns a response with nil or empty content (addresses issue #993) + t.Skip("Requires mock client - would demonstrate panic without len(result.Content) == 0 check") +}