From 82d574eb7b85c16d97a1697b3224505d72a7c49d Mon Sep 17 00:00:00 2001 From: Alix-007 <267018309+Alix-007@users.noreply.github.com> Date: Fri, 20 Mar 2026 14:37:47 +0800 Subject: [PATCH] fix(agent): separate empty-response and tool-limit fallbacks --- pkg/agent/loop.go | 9 ++- pkg/agent/loop_test.go | 129 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index ef2b9e28f..637cd506c 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -70,7 +70,8 @@ type processOptions struct { } const ( - defaultResponse = "I've completed processing but have no response to give. Increase `max_tool_iterations` in config.json." + defaultResponse = "The model returned an empty response. This may indicate a provider error or token limit." + toolLimitResponse = "I've reached `max_tool_iterations` without a final response. Increase `max_tool_iterations` in config.json if this task needs more tool steps." sessionKeyAgentPrefix = "agent:" metadataKeyAccountID = "account_id" metadataKeyGuildID = "guild_id" @@ -935,7 +936,11 @@ func (al *AgentLoop) runAgentLoop( // 4. Handle empty response if finalContent == "" { - finalContent = opts.DefaultResponse + if iteration >= agent.MaxIterations && agent.MaxIterations > 0 { + finalContent = toolLimitResponse + } else { + finalContent = opts.DefaultResponse + } } // 5. Save final assistant message to session diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index b6b6c2c6c..28eab03db 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -420,6 +420,29 @@ func (m *countingMockProvider) GetDefaultModel() string { return "counting-mock-model" } +type toolLimitOnlyProvider struct{} + +func (m *toolLimitOnlyProvider) Chat( + ctx context.Context, + messages []providers.Message, + tools []providers.ToolDefinition, + model string, + opts map[string]any, +) (*providers.LLMResponse, error) { + return &providers.LLMResponse{ + ToolCalls: []providers.ToolCall{{ + ID: "call_tool_limit_test", + Type: "function", + Name: "tool_limit_test_tool", + Arguments: map[string]any{"value": "x"}, + }}, + }, nil +} + +func (m *toolLimitOnlyProvider) GetDefaultModel() string { + return "tool-limit-only-model" +} + // mockCustomTool is a simple mock tool for registration testing type mockCustomTool struct{} @@ -442,6 +465,29 @@ func (m *mockCustomTool) Execute(ctx context.Context, args map[string]any) *tool return tools.SilentResult("Custom tool executed") } +type toolLimitTestTool struct{} + +func (m *toolLimitTestTool) Name() string { + return "tool_limit_test_tool" +} + +func (m *toolLimitTestTool) Description() string { + return "Tool used to exhaust the iteration budget in tests" +} + +func (m *toolLimitTestTool) Parameters() map[string]any { + return map[string]any{ + "type": "object", + "properties": map[string]any{ + "value": map[string]any{"type": "string"}, + }, + } +} + +func (m *toolLimitTestTool) Execute(ctx context.Context, args map[string]any) *tools.ToolResult { + return tools.SilentResult("tool limit test result") +} + // testHelper executes a message and returns the response type testHelper struct { al *AgentLoop @@ -1083,6 +1129,89 @@ func TestAgentLoop_ContextExhaustionRetry(t *testing.T) { } } +func TestAgentLoop_EmptyModelResponseUsesAccurateFallback(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "agent-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + Model: "test-model", + MaxTokens: 4096, + MaxToolIterations: 3, + }, + }, + } + + msgBus := bus.NewMessageBus() + provider := &simpleMockProvider{response: ""} + al := NewAgentLoop(cfg, msgBus, provider) + + response, err := al.ProcessDirectWithChannel(context.Background(), "hello", "empty-response", "test", "chat1") + if err != nil { + t.Fatalf("ProcessDirectWithChannel failed: %v", err) + } + if response != defaultResponse { + t.Fatalf("response = %q, want %q", response, defaultResponse) + } +} + +func TestAgentLoop_ToolLimitUsesDedicatedFallback(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "agent-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + Model: "test-model", + MaxTokens: 4096, + MaxToolIterations: 1, + }, + }, + } + + msgBus := bus.NewMessageBus() + provider := &toolLimitOnlyProvider{} + al := NewAgentLoop(cfg, msgBus, provider) + al.RegisterTool(&toolLimitTestTool{}) + + response, err := al.ProcessDirectWithChannel(context.Background(), "hello", "tool-limit", "test", "chat1") + if err != nil { + t.Fatalf("ProcessDirectWithChannel failed: %v", err) + } + if response != toolLimitResponse { + t.Fatalf("response = %q, want %q", response, toolLimitResponse) + } + + defaultAgent := al.registry.GetDefaultAgent() + if defaultAgent == nil { + t.Fatal("No default agent found") + } + route := al.registry.ResolveRoute(routing.RouteInput{ + Channel: "test", + Peer: &routing.RoutePeer{ + Kind: "direct", + ID: "cron", + }, + }) + history := defaultAgent.Sessions.GetHistory(route.SessionKey) + if len(history) != 4 { + t.Fatalf("history len = %d, want 4", len(history)) + } + assertRoles(t, history, "user", "assistant", "tool", "assistant") + if history[3].Content != toolLimitResponse { + t.Fatalf("final assistant content = %q, want %q", history[3].Content, toolLimitResponse) + } +} + // TestProcessDirectWithChannel_TriggersMCPInitialization verifies that // ProcessDirectWithChannel triggers MCP initialization when MCP is enabled. // Note: Manager is only initialized when at least one MCP server is configured