Merge pull request #75 from AdjectiveAllison/allison/eng-1215-acp-mcp-tool-naming-bug

Allison/eng 1215 acp mcp tool naming bug

Author: allisoneer

acp/config/manager/kustomization.yaml

@@ -4,5 +4,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
 images:
 - name: controller
-  newName: ghcr.io/humanlayer/agentcontrolplane
-  newTag: v0.5.0
+  newName: controller
+  newTag: "202504151521"

acp/config/samples/acp_v1alpha1_claude_task.yaml

@@ -5,4 +5,4 @@ metadata:
 spec:
   agentRef:
     name: claude-fetch-agent
-  message: "Write me a haiku about the character found at https://swapi.dev/api/people/2?"
+  userMessage: "Write me a haiku about the character found at https://swapi.dev/api/people/2?"

acp/internal/adapters/mcp_adapter_test.go

@@ -0,0 +1,63 @@
+package adapters
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	acp "github.com/humanlayer/agentcontrolplane/acp/api/v1alpha1"
+)
+
+var _ = Describe("MCP Adapter", func() {
+	Context("When converting MCP tools to LLM client tools", func() {
+		It("should correctly format tool names with server prefix", func() {
+			// Create sample tools with different names
+			tools := []acp.MCPTool{
+				{
+					Name:        "fetch",
+					Description: "Fetches data from URL",
+					InputSchema: runtime.RawExtension{Raw: []byte(`{"type":"object"}`)},
+				},
+				{
+					Name:        "calculate",
+					Description: "Performs calculations",
+					InputSchema: runtime.RawExtension{Raw: []byte(`{"type":"object"}`)},
+				},
+			}
+
+			// Test with a server name different from tool names
+			By("using a server name different from tool names")
+			serverName := "server-alpha"
+			result := ConvertMCPToolsToLLMClientTools(tools, serverName)
+
+			// Verify correct naming
+			Expect(result).To(HaveLen(2))
+			Expect(result[0].Function.Name).To(Equal("server-alpha__fetch"))
+			Expect(result[1].Function.Name).To(Equal("server-alpha__calculate"))
+
+			// Test with a server name similar to a tool name (the bug scenario)
+			By("using a server name that contains a tool name")
+			serverName = "fetch-server"
+			result = ConvertMCPToolsToLLMClientTools(tools, serverName)
+
+			// Verify correct naming - even when server name contains tool name
+			Expect(result).To(HaveLen(2))
+			Expect(result[0].Function.Name).To(Equal("fetch-server__fetch"))
+			Expect(result[1].Function.Name).To(Equal("fetch-server__calculate"))
+
+			// Verify the bug case specifically - tool name should never be used as server name
+			By("ensuring tool name is never used as server name (the bug case)")
+			for _, tool := range result {
+				Expect(tool.Function.Name).NotTo(Equal("fetch__fetch"))
+				Expect(tool.Function.Name).NotTo(Equal("calculate__calculate"))
+			}
+		})
+
+		It("should handle empty tool list", func() {
+			serverName := "test-server"
+			result := ConvertMCPToolsToLLMClientTools([]acp.MCPTool{}, serverName)
+			Expect(result).To(BeEmpty())
+		})
+	})
+})

acp/internal/adapters/suite_test.go

@@ -0,0 +1,13 @@
+package adapters
+
+import (
+	testing "testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestAdapters(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Adapters Suite")
+}

acp/internal/controller/task/task_controller.go

@@ -358,12 +358,16 @@ func (r *TaskReconciler) collectTools(ctx context.Context, agent *acp.Agent) []l
 	logger := log.FromContext(ctx)
 	tools := make([]llmclient.Tool, 0)
 
-	// Get tools from MCP manager
-	mcpTools := r.MCPManager.GetToolsForAgent(agent)
-
-	// Convert MCP tools to LLM tools
-	for _, mcpTool := range mcpTools {
-		tools = append(tools, adapters.ConvertMCPToolsToLLMClientTools([]acp.MCPTool{mcpTool}, mcpTool.Name)...)
+	// Iterate through each MCP server directly to maintain server-tool association
+	for _, serverRef := range agent.Spec.MCPServers {
+		mcpTools, found := r.MCPManager.GetTools(serverRef.Name)
+		if !found {
+			logger.Info("Server not found or has no tools", "server", serverRef.Name)
+			continue
+		}
+		// Use the correct server name when converting tools
+		tools = append(tools, adapters.ConvertMCPToolsToLLMClientTools(mcpTools, serverRef.Name)...)
+		logger.Info("Added MCP server tools", "server", serverRef.Name, "toolCount", len(mcpTools))
 	}
 
 	// Convert HumanContactChannel tools to LLM tools