From 0c2123bb3086a5b24374f18800dcb37c9be1910e Mon Sep 17 00:00:00 2001 From: Max Date: Fri, 16 Jan 2026 16:57:48 +0800 Subject: [PATCH] Enhance Input Formatting and Error Handling in Robot Executor - Introduced a new method to format available resources, improving clarity on the tools and agents accessible to the robot. - Updated the RunGoals and RunInspiration methods to include resource information in user prompts, ensuring achievable goals and insights. - Revised error messages in the executor to provide more context, enhancing debugging capabilities. - Added comprehensive tests for the new resource formatting functionality and updated existing tests to reflect changes in error handling. - Improved documentation for robot configuration, emphasizing the importance of available resources in goal and task generation. --- agent/robot/executor/standard/goals.go | 43 ++++--- agent/robot/executor/standard/goals_test.go | 119 +++++++++++++++++- agent/robot/executor/standard/input.go | 114 +++++++++++++---- agent/robot/executor/standard/input_test.go | 103 ++++++++++++++- agent/robot/executor/standard/inspiration.go | 10 +- .../executor/standard/inspiration_test.go | 22 +++- 6 files changed, 359 insertions(+), 52 deletions(-) diff --git a/agent/robot/executor/standard/goals.go b/agent/robot/executor/standard/goals.go index 17d38bdc..d7610c2b 100644 --- a/agent/robot/executor/standard/goals.go +++ b/agent/robot/executor/standard/goals.go @@ -62,6 +62,13 @@ func (e *Executor) RunGoals(ctx *robottypes.Context, exec *robottypes.Execution, } } + // Add available resources - critical for generating achievable goals + // Without knowing what tools are available, goals might be unachievable + resourcesContent := formatter.FormatAvailableResources(robot) + if resourcesContent != "" { + userContent += "\n\n" + resourcesContent + } + if userContent == "" { return fmt.Errorf("no input available for goals generation") } @@ -70,7 +77,7 @@ func (e *Executor) RunGoals(ctx *robottypes.Context, exec *robottypes.Execution, caller := NewAgentCaller() result, err := caller.CallWithMessages(ctx, agentID, userContent) if err != nil { - return fmt.Errorf("goals agent call failed: %w", err) + return fmt.Errorf("goals agent (%s) call failed: %w", agentID, err) } // Parse response as JSON @@ -98,36 +105,38 @@ func (e *Executor) RunGoals(ctx *robottypes.Context, exec *robottypes.Execution, // Extract delivery if delivery, ok := data["delivery"].(map[string]interface{}); ok { - exec.Goals.Delivery = parseDelivery(delivery) + exec.Goals.Delivery = ParseDelivery(delivery) } // Validate: content is required if exec.Goals.Content == "" { - return fmt.Errorf("goals agent returned empty content") + return fmt.Errorf("goals agent (%s) returned empty content", agentID) } return nil } -// parseDelivery converts map to DeliveryTarget struct -func parseDelivery(data map[string]interface{}) *robottypes.DeliveryTarget { +// ParseDelivery converts map to DeliveryTarget struct +// Returns nil if data is nil or type is invalid/missing +func ParseDelivery(data map[string]interface{}) *robottypes.DeliveryTarget { if data == nil { return nil } - target := &robottypes.DeliveryTarget{} + // Type is required - if missing or invalid, return nil + t, ok := data["type"].(string) + if !ok || t == "" { + return nil + } - // Parse and validate type - if t, ok := data["type"].(string); ok { - deliveryType := robottypes.DeliveryType(t) - switch deliveryType { - case robottypes.DeliveryEmail, robottypes.DeliveryWebhook, - robottypes.DeliveryFile, robottypes.DeliveryNotify: - target.Type = deliveryType - default: - // Invalid type - still set it but caller should validate - target.Type = deliveryType - } + deliveryType := robottypes.DeliveryType(t) + if !IsValidDeliveryType(deliveryType) { + // Invalid type - return nil to indicate parsing failure + return nil + } + + target := &robottypes.DeliveryTarget{ + Type: deliveryType, } // Parse recipients diff --git a/agent/robot/executor/standard/goals_test.go b/agent/robot/executor/standard/goals_test.go index 7d09cda4..8fcbb228 100644 --- a/agent/robot/executor/standard/goals_test.go +++ b/agent/robot/executor/standard/goals_test.go @@ -241,7 +241,7 @@ func TestRunGoalsErrorHandling(t *testing.T) { err := e.RunGoals(ctx, exec, nil) assert.Error(t, err) - assert.Contains(t, err.Error(), "agent call failed") + assert.Contains(t, err.Error(), "call failed") }) t.Run("returns error when no input available and no identity", func(t *testing.T) { @@ -377,6 +377,105 @@ func TestDeliveryTypeValidation(t *testing.T) { }) } +func TestParseDelivery(t *testing.T) { + t.Run("parses valid delivery with all fields", func(t *testing.T) { + data := map[string]interface{}{ + "type": "email", + "recipients": []interface{}{"user@example.com", "team@example.com"}, + "format": "markdown", + "template": "weekly-report", + "options": map[string]interface{}{ + "subject": "Weekly Report", + }, + } + + result := standard.ParseDelivery(data) + + require.NotNil(t, result) + assert.Equal(t, types.DeliveryEmail, result.Type) + assert.Equal(t, []string{"user@example.com", "team@example.com"}, result.Recipients) + assert.Equal(t, "markdown", result.Format) + assert.Equal(t, "weekly-report", result.Template) + assert.Equal(t, "Weekly Report", result.Options["subject"]) + }) + + t.Run("returns nil for nil data", func(t *testing.T) { + result := standard.ParseDelivery(nil) + assert.Nil(t, result) + }) + + t.Run("returns nil for missing type", func(t *testing.T) { + data := map[string]interface{}{ + "recipients": []interface{}{"user@example.com"}, + } + + result := standard.ParseDelivery(data) + assert.Nil(t, result) + }) + + t.Run("returns nil for empty type", func(t *testing.T) { + data := map[string]interface{}{ + "type": "", + "recipients": []interface{}{"user@example.com"}, + } + + result := standard.ParseDelivery(data) + assert.Nil(t, result) + }) + + t.Run("returns nil for invalid type", func(t *testing.T) { + data := map[string]interface{}{ + "type": "sms", + "recipients": []interface{}{"user@example.com"}, + } + + result := standard.ParseDelivery(data) + assert.Nil(t, result) + }) + + t.Run("handles missing optional fields", func(t *testing.T) { + data := map[string]interface{}{ + "type": "webhook", + } + + result := standard.ParseDelivery(data) + + require.NotNil(t, result) + assert.Equal(t, types.DeliveryWebhook, result.Type) + assert.Empty(t, result.Recipients) + assert.Empty(t, result.Format) + assert.Empty(t, result.Template) + assert.Nil(t, result.Options) + }) + + t.Run("handles non-string recipients gracefully", func(t *testing.T) { + data := map[string]interface{}{ + "type": "email", + "recipients": []interface{}{"valid@example.com", 123, nil, "another@example.com"}, + } + + result := standard.ParseDelivery(data) + + require.NotNil(t, result) + // Only string recipients should be included + assert.Equal(t, []string{"valid@example.com", "another@example.com"}, result.Recipients) + }) + + t.Run("parses all valid delivery types", func(t *testing.T) { + validTypes := []string{"email", "webhook", "file", "notify"} + + for _, dt := range validTypes { + data := map[string]interface{}{ + "type": dt, + } + + result := standard.ParseDelivery(data) + require.NotNil(t, result, "should parse type: %s", dt) + assert.Equal(t, types.DeliveryType(dt), result.Type) + } + }) +} + // ============================================================================ // InputFormatter Tests for P1 // ============================================================================ @@ -450,6 +549,10 @@ func TestInputFormatterFormatRobotIdentity(t *testing.T) { // ============================================================================ // createGoalsTestRobot creates a test robot with specified goals agent +// Includes available expert agents so the Goals Agent knows what resources are available +// +// Note: The agent IDs listed in Resources.Agents must exist in yao-dev-app/assistants/experts/ +// Current available experts: data-analyst, summarizer, text-writer, web-reader func createGoalsTestRobot(t *testing.T, agentID string) *types.Robot { t.Helper() return &types.Robot{ @@ -459,12 +562,24 @@ func createGoalsTestRobot(t *testing.T, agentID string) *types.Robot { Config: &types.Config{ Identity: &types.Identity{ Role: "Test Assistant", - Duties: []string{"Testing", "Validation"}, + Duties: []string{"Testing", "Validation", "Data Analysis", "Report Generation"}, }, Resources: &types.Resources{ Phases: map[types.Phase]string{ types.PhaseGoals: agentID, }, + // Available expert agents that can be delegated to + // These IDs correspond to assistants in yao-dev-app/assistants/experts/ + Agents: []string{ + "experts.data-analyst", // Data analysis and insights + "experts.summarizer", // Content summarization + "experts.text-writer", // Report and document generation + "experts.web-reader", // Web content extraction + }, + }, + // Knowledge base collections (if any) + KB: &types.KB{ + Collections: []string{"test-knowledge"}, }, }, } diff --git a/agent/robot/executor/standard/input.go b/agent/robot/executor/standard/input.go index c172ae9b..7b88c490 100644 --- a/agent/robot/executor/standard/input.go +++ b/agent/robot/executor/standard/input.go @@ -11,9 +11,9 @@ import ( // InputFormatter provides methods to format input data for assistant prompts // Each phase has specific input requirements: -// - P0 (Inspiration): ClockContext + Robot identity -// - P1 (Goals): InspirationReport (Clock) or TriggerInput (Human/Event) -// - P2 (Tasks): Goals + Available tools +// - P0 (Inspiration): ClockContext + Robot identity + Available resources +// - P1 (Goals): InspirationReport/TriggerInput + Robot identity + Available resources +// - P2 (Tasks): Goals + Available resources // - P3 (Run): Tasks // - P4 (Delivery): Task results // - P5 (Learning): Execution summary @@ -110,6 +110,86 @@ func (f *InputFormatter) FormatRobotIdentity(robot *robottypes.Robot) string { return sb.String() } +// FormatAvailableResources formats available resources (agents, MCP tools, KB, DB) as user message content +// Used by P0 (Inspiration) and P1 (Goals) to inform the agent what tools are available +// This is critical for generating achievable goals - without knowing available tools, +// the agent might generate goals that cannot be accomplished +func (f *InputFormatter) FormatAvailableResources(robot *robottypes.Robot) string { + if robot == nil || robot.Config == nil { + return "" + } + + var sb strings.Builder + hasContent := false + + // Available Agents + if robot.Config.Resources != nil && len(robot.Config.Resources.Agents) > 0 { + if !hasContent { + sb.WriteString("## Available Resources\n\n") + hasContent = true + } + sb.WriteString("### Agents\n") + sb.WriteString("These are the AI assistants you can delegate tasks to:\n") + for _, agent := range robot.Config.Resources.Agents { + sb.WriteString(fmt.Sprintf("- **%s**\n", agent)) + } + sb.WriteString("\n") + } + + // Available MCP Tools + if robot.Config.Resources != nil && len(robot.Config.Resources.MCP) > 0 { + if !hasContent { + sb.WriteString("## Available Resources\n\n") + hasContent = true + } + sb.WriteString("### MCP Tools\n") + sb.WriteString("These are the external tools and services you can use:\n") + for _, mcp := range robot.Config.Resources.MCP { + if len(mcp.Tools) > 0 { + sb.WriteString(fmt.Sprintf("- **%s**: %s\n", mcp.ID, strings.Join(mcp.Tools, ", "))) + } else { + sb.WriteString(fmt.Sprintf("- **%s**: all tools available\n", mcp.ID)) + } + } + sb.WriteString("\n") + } + + // Available Knowledge Base + if robot.Config.KB != nil && len(robot.Config.KB.Collections) > 0 { + if !hasContent { + sb.WriteString("## Available Resources\n\n") + hasContent = true + } + sb.WriteString("### Knowledge Base\n") + sb.WriteString("You have access to these knowledge collections:\n") + for _, collection := range robot.Config.KB.Collections { + sb.WriteString(fmt.Sprintf("- %s\n", collection)) + } + sb.WriteString("\n") + } + + // Available Database Models + if robot.Config.DB != nil && len(robot.Config.DB.Models) > 0 { + if !hasContent { + sb.WriteString("## Available Resources\n\n") + hasContent = true + } + sb.WriteString("### Database\n") + sb.WriteString("You can query these database models:\n") + for _, model := range robot.Config.DB.Models { + sb.WriteString(fmt.Sprintf("- %s\n", model)) + } + sb.WriteString("\n") + } + + if !hasContent { + return "" + } + + sb.WriteString("**Important**: Only plan goals and tasks that can be accomplished with the above resources.\n") + return sb.String() +} + // FormatInspirationReport formats InspirationReport as user message content // Used by P1 (Goals) phase when trigger is Clock func (f *InputFormatter) FormatInspirationReport(report *robottypes.InspirationReport) string { @@ -220,29 +300,11 @@ func (f *InputFormatter) FormatGoals(goals *robottypes.Goals, robot *robottypes. sb.WriteString(goals.Content) sb.WriteString("\n") - // Available resources (if robot config available) - if robot != nil && robot.Config != nil && robot.Config.Resources != nil { - sb.WriteString("\n## Available Resources\n\n") - - // Agents - if len(robot.Config.Resources.Agents) > 0 { - sb.WriteString("### Agents\n") - for _, agent := range robot.Config.Resources.Agents { - sb.WriteString(fmt.Sprintf("- %s\n", agent)) - } - } - - // MCP tools - if len(robot.Config.Resources.MCP) > 0 { - sb.WriteString("\n### MCP Tools\n") - for _, mcp := range robot.Config.Resources.MCP { - if len(mcp.Tools) > 0 { - sb.WriteString(fmt.Sprintf("- %s: %s\n", mcp.ID, strings.Join(mcp.Tools, ", "))) - } else { - sb.WriteString(fmt.Sprintf("- %s: all tools\n", mcp.ID)) - } - } - } + // Available resources - reuse FormatAvailableResources for consistency + resourcesContent := f.FormatAvailableResources(robot) + if resourcesContent != "" { + sb.WriteString("\n") + sb.WriteString(resourcesContent) } return sb.String() diff --git a/agent/robot/executor/standard/input_test.go b/agent/robot/executor/standard/input_test.go index 6d44dd72..60f2c748 100644 --- a/agent/robot/executor/standard/input_test.go +++ b/agent/robot/executor/standard/input_test.go @@ -126,6 +126,103 @@ func TestInputFormatterFormatInspirationReport(t *testing.T) { }) } +func TestInputFormatterFormatAvailableResources(t *testing.T) { + formatter := standard.NewInputFormatter() + + t.Run("formats all resource types", func(t *testing.T) { + robot := &types.Robot{ + MemberID: "test-robot", + Config: &types.Config{ + Resources: &types.Resources{ + Agents: []string{"data-analyst", "chart-gen", "report-writer"}, + MCP: []types.MCPConfig{ + {ID: "database", Tools: []string{"query", "insert"}}, + {ID: "email", Tools: []string{}}, // all tools + }, + }, + KB: &types.KB{ + Collections: []string{"sales-policies", "products"}, + }, + DB: &types.DB{ + Models: []string{"sales", "customers", "orders"}, + }, + }, + } + + result := formatter.FormatAvailableResources(robot) + + // Check structure + assert.Contains(t, result, "## Available Resources") + + // Check agents + assert.Contains(t, result, "### Agents") + assert.Contains(t, result, "data-analyst") + assert.Contains(t, result, "chart-gen") + assert.Contains(t, result, "report-writer") + + // Check MCP tools + assert.Contains(t, result, "### MCP Tools") + assert.Contains(t, result, "database") + assert.Contains(t, result, "query, insert") + assert.Contains(t, result, "email") + assert.Contains(t, result, "all tools available") + + // Check KB + assert.Contains(t, result, "### Knowledge Base") + assert.Contains(t, result, "sales-policies") + assert.Contains(t, result, "products") + + // Check DB + assert.Contains(t, result, "### Database") + assert.Contains(t, result, "sales") + assert.Contains(t, result, "customers") + assert.Contains(t, result, "orders") + + // Check important note + assert.Contains(t, result, "Only plan goals and tasks that can be accomplished") + }) + + t.Run("returns empty for nil robot", func(t *testing.T) { + result := formatter.FormatAvailableResources(nil) + assert.Empty(t, result) + }) + + t.Run("returns empty for robot without config", func(t *testing.T) { + robot := &types.Robot{MemberID: "test"} + result := formatter.FormatAvailableResources(robot) + assert.Empty(t, result) + }) + + t.Run("returns empty for robot without resources", func(t *testing.T) { + robot := &types.Robot{ + MemberID: "test", + Config: &types.Config{}, + } + result := formatter.FormatAvailableResources(robot) + assert.Empty(t, result) + }) + + t.Run("handles partial resources", func(t *testing.T) { + robot := &types.Robot{ + MemberID: "test", + Config: &types.Config{ + Resources: &types.Resources{ + Agents: []string{"single-agent"}, + }, + }, + } + + result := formatter.FormatAvailableResources(robot) + + assert.Contains(t, result, "## Available Resources") + assert.Contains(t, result, "### Agents") + assert.Contains(t, result, "single-agent") + assert.NotContains(t, result, "### MCP Tools") + assert.NotContains(t, result, "### Knowledge Base") + assert.NotContains(t, result, "### Database") + }) +} + func TestInputFormatterFormatTriggerInput(t *testing.T) { formatter := standard.NewInputFormatter() @@ -207,8 +304,10 @@ func TestInputFormatterFormatGoals(t *testing.T) { assert.Contains(t, result, "### Agents") assert.Contains(t, result, "data-analyzer") assert.Contains(t, result, "### MCP Tools") - assert.Contains(t, result, "database: query, insert") - assert.Contains(t, result, "email: all tools") + assert.Contains(t, result, "database") + assert.Contains(t, result, "query, insert") + assert.Contains(t, result, "email") + assert.Contains(t, result, "all tools available") }) t.Run("formats goals without robot", func(t *testing.T) { diff --git a/agent/robot/executor/standard/inspiration.go b/agent/robot/executor/standard/inspiration.go index 68c5be90..6354994f 100644 --- a/agent/robot/executor/standard/inspiration.go +++ b/agent/robot/executor/standard/inspiration.go @@ -41,17 +41,23 @@ func (e *Executor) RunInspiration(ctx *robottypes.Context, exec *robottypes.Exec formatter := NewInputFormatter() userContent := formatter.FormatClockContext(clock, robot) + // Add available resources - critical for generating achievable insights + resourcesContent := formatter.FormatAvailableResources(robot) + if resourcesContent != "" { + userContent += "\n\n" + resourcesContent + } + // Call agent caller := NewAgentCaller() result, err := caller.CallWithMessages(ctx, agentID, userContent) if err != nil { - return fmt.Errorf("inspiration agent call failed: %w", err) + return fmt.Errorf("inspiration agent (%s) call failed: %w", agentID, err) } // Parse response - get markdown content content := result.GetText() if content == "" { - return fmt.Errorf("inspiration agent returned empty response") + return fmt.Errorf("inspiration agent (%s) returned empty response", agentID) } // Build InspirationReport diff --git a/agent/robot/executor/standard/inspiration_test.go b/agent/robot/executor/standard/inspiration_test.go index bf2c2ece..c10834b0 100644 --- a/agent/robot/executor/standard/inspiration_test.go +++ b/agent/robot/executor/standard/inspiration_test.go @@ -201,7 +201,7 @@ func TestRunInspirationErrorHandling(t *testing.T) { // Real AgentCaller returns error for non-existent agent assert.Error(t, err) - assert.Contains(t, err.Error(), "agent call failed") + assert.Contains(t, err.Error(), "call failed") }) } @@ -233,7 +233,7 @@ func TestRunInspirationWithDefaultAgent(t *testing.T) { // In test environment, we expect it to fail with "agent not found" // In production, it would use the default agent if err != nil { - assert.Contains(t, err.Error(), "agent call failed") + assert.Contains(t, err.Error(), "call failed") } }) } @@ -314,6 +314,10 @@ func TestInputFormatterClockContext(t *testing.T) { // ============================================================================ // createTestRobot creates a test robot with specified inspiration agent +// Includes available expert agents so the Inspiration Agent knows what resources are available +// +// Note: The agent IDs listed in Resources.Agents must exist in yao-dev-app/assistants/experts/ +// Current available experts: data-analyst, summarizer, text-writer, web-reader func createTestRobot(t *testing.T, agentID string) *types.Robot { t.Helper() return &types.Robot{ @@ -323,12 +327,24 @@ func createTestRobot(t *testing.T, agentID string) *types.Robot { Config: &types.Config{ Identity: &types.Identity{ Role: "Test Assistant", - Duties: []string{"Testing"}, + Duties: []string{"Testing", "Data Analysis", "Report Generation"}, }, Resources: &types.Resources{ Phases: map[types.Phase]string{ types.PhaseInspiration: agentID, }, + // Available expert agents that can be delegated to + // These IDs correspond to assistants in yao-dev-app/assistants/experts/ + Agents: []string{ + "experts.data-analyst", // Data analysis and insights + "experts.summarizer", // Content summarization + "experts.text-writer", // Report and document generation + "experts.web-reader", // Web content extraction + }, + }, + // Knowledge base collections (if any) + KB: &types.KB{ + Collections: []string{"test-knowledge"}, }, }, }