diff --git a/agent/robot/DESIGN.md b/agent/robot/DESIGN.md index e753dc85..0c360a51 100644 --- a/agent/robot/DESIGN.md +++ b/agent/robot/DESIGN.md @@ -431,9 +431,16 @@ For each task: | Type | ExecutorID Format | Example | |------|-------------------|---------| | `assistant` | Agent ID | `experts.text-writer` | -| `mcp` | `clientID.toolName` | `filesystem.read_file` | +| `mcp` | `mcp_server.mcp_tool` | `ark.image.text2img.generate` | | `process` | Process name | `models.user.Find` | +**MCP Task Fields:** + +For MCP tasks, three fields are required: +- `executor_id`: Combined format `mcp_server.mcp_tool` +- `mcp_server`: MCP server/client ID (e.g., `ark.image.text2img`) +- `mcp_tool`: Tool name within the server (e.g., `generate`) + **Multi-Turn Conversation Flow:** For assistant tasks, P3 uses a multi-turn conversation approach: diff --git a/agent/robot/TECHNICAL.md b/agent/robot/TECHNICAL.md index 74683dda..c2375185 100644 --- a/agent/robot/TECHNICAL.md +++ b/agent/robot/TECHNICAL.md @@ -1322,9 +1322,13 @@ type Task struct { // Executor ExecutorType ExecutorType `json:"executor_type"` - ExecutorID string `json:"executor_id"` + ExecutorID string `json:"executor_id"` // unified ID: agent/assistant/process ID, or "mcp_server.mcp_tool" for MCP Args []any `json:"args,omitempty"` + // MCP-specific fields (required when executor_type is "mcp") + MCPServer string `json:"mcp_server,omitempty"` // MCP server/client ID (e.g., "ark.image.text2img") + MCPTool string `json:"mcp_tool,omitempty"` // MCP tool name (e.g., "generate") + // Validation (defined in P2, used in P3) ExpectedOutput string `json:"expected_output,omitempty"` // what the task should produce // ValidationRules supports two formats: diff --git a/agent/robot/executor/standard/runner.go b/agent/robot/executor/standard/runner.go index a38e97b6..30d9f4d5 100644 --- a/agent/robot/executor/standard/runner.go +++ b/agent/robot/executor/standard/runner.go @@ -252,20 +252,18 @@ func (r *Runner) generateDefaultReply(validation *robottypes.ValidationResult, t } // ExecuteMCPTask executes a task using an MCP tool -// ExecutorID format: "mcpClientID.toolName" (e.g., "filesystem.read_file") +// Requires task.MCPServer and task.MCPTool fields to be set +// executor_id is the combined form: "mcp_server.mcp_tool" (e.g., "ark.image.text2img.generate") func (r *Runner) ExecuteMCPTask(task *robottypes.Task, taskCtx *RunnerContext) (interface{}, error) { - // Parse MCP executor ID (format: clientID.toolName) - parts := strings.SplitN(task.ExecutorID, ".", 2) - if len(parts) != 2 { - return nil, fmt.Errorf("invalid MCP executor ID: %s (expected format: clientID.toolName)", task.ExecutorID) + // Validate MCP-specific fields + if task.MCPServer == "" || task.MCPTool == "" { + return nil, fmt.Errorf("MCP task requires mcp_server and mcp_tool fields (executor_id: %s)", task.ExecutorID) } - clientID, toolName := parts[0], parts[1] - // Get MCP client - client, err := mcp.Select(clientID) + client, err := mcp.Select(task.MCPServer) if err != nil { - return nil, fmt.Errorf("MCP client not found: %s: %w", clientID, err) + return nil, fmt.Errorf("MCP server not found: %s: %w", task.MCPServer, err) } // Build arguments map from task.Args @@ -281,9 +279,9 @@ func (r *Runner) ExecuteMCPTask(task *robottypes.Task, taskCtx *RunnerContext) ( } // Call MCP tool - result, err := client.CallTool(r.ctx.Context, toolName, args) + result, err := client.CallTool(r.ctx.Context, task.MCPTool, args) if err != nil { - return nil, fmt.Errorf("MCP tool call failed: %w", err) + return nil, fmt.Errorf("MCP tool call failed (%s.%s): %w", task.MCPServer, task.MCPTool, err) } return result, nil diff --git a/agent/robot/executor/standard/tasks.go b/agent/robot/executor/standard/tasks.go index 597667af..ce9bca9c 100644 --- a/agent/robot/executor/standard/tasks.go +++ b/agent/robot/executor/standard/tasks.go @@ -173,6 +173,14 @@ func ParseTask(data map[string]interface{}, index int) (*robottypes.Task, error) copy(task.Args, args) } + // MCP-specific fields (required when executor_type is "mcp") + if mcpServer, ok := data["mcp_server"].(string); ok { + task.MCPServer = mcpServer + } + if mcpTool, ok := data["mcp_tool"].(string); ok { + task.MCPTool = mcpTool + } + // Optional: expected_output (for P3 validation) if expectedOutput, ok := data["expected_output"].(string); ok { task.ExpectedOutput = expectedOutput @@ -324,6 +332,7 @@ func SortTasksByOrder(tasks []robottypes.Task) { // ValidateExecutorExists checks if the executor ID exists in available resources // This is an optional validation - tasks with unknown executors will still be created // but may fail during P3 execution +// For MCP tasks, pass mcpServer as the second parameter (executorID is ignored for MCP) func ValidateExecutorExists(executorID string, executorType robottypes.ExecutorType, robot *robottypes.Robot) bool { if robot == nil || robot.Config == nil || robot.Config.Resources == nil { return true // Skip validation if no resources configured @@ -339,6 +348,10 @@ func ValidateExecutorExists(executorID string, executorType robottypes.ExecutorT return false case robottypes.ExecutorMCP: + // For MCP, executorID can be either: + // 1. The mcp_server value (new format) + // 2. The combined mcp_server.mcp_tool format (for display) + // We validate against mcp_server (the MCP server/client ID) for _, mcp := range robot.Config.Resources.MCP { if mcp.ID == executorID { return true @@ -354,3 +367,18 @@ func ValidateExecutorExists(executorID string, executorType robottypes.ExecutorT return false } + +// ValidateMCPTask validates MCP task fields +// Returns an error if mcp_server or mcp_tool is missing for MCP tasks +func ValidateMCPTask(task *robottypes.Task) error { + if task.ExecutorType != robottypes.ExecutorMCP { + return nil + } + if task.MCPServer == "" { + return fmt.Errorf("MCP task %s: mcp_server field is required", task.ID) + } + if task.MCPTool == "" { + return fmt.Errorf("MCP task %s: mcp_tool field is required", task.ID) + } + return nil +} diff --git a/agent/robot/types/robot.go b/agent/robot/types/robot.go index 50f7f170..768a336a 100644 --- a/agent/robot/types/robot.go +++ b/agent/robot/types/robot.go @@ -224,9 +224,13 @@ type Task struct { // Executor ExecutorType ExecutorType `json:"executor_type"` - ExecutorID string `json:"executor_id"` + ExecutorID string `json:"executor_id"` // unified ID: agent/assistant/process ID, or "mcp_server.mcp_tool" for MCP Args []any `json:"args,omitempty"` + // MCP-specific fields (required when executor_type is "mcp") + MCPServer string `json:"mcp_server,omitempty"` // MCP server/client ID (e.g., "ark.image.text2img") + MCPTool string `json:"mcp_tool,omitempty"` // MCP tool name (e.g., "generate") + // Validation (defined in P2, used in P3) // ExpectedOutput describes what the task should produce (for LLM semantic validation) ExpectedOutput string `json:"expected_output,omitempty"` // e.g., "JSON with sales_total, growth_rate fields"