From 3214afa44c345c391006bd8293e4d6298faea59c Mon Sep 17 00:00:00 2001 From: Allison Durham Date: Tue, 15 Apr 2025 15:27:26 -0500 Subject: [PATCH 1/3] change to `userMessage` --- acp/config/samples/acp_v1alpha1_claude_task.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acp/config/samples/acp_v1alpha1_claude_task.yaml b/acp/config/samples/acp_v1alpha1_claude_task.yaml index af17ba47..50267765 100644 --- a/acp/config/samples/acp_v1alpha1_claude_task.yaml +++ b/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?" \ No newline at end of file + userMessage: "Write me a haiku about the character found at https://swapi.dev/api/people/2?" From 0b0a041dfec7aebb7c534e3efabf98a48bfc9157 Mon Sep 17 00:00:00 2001 From: Allison Durham Date: Tue, 15 Apr 2025 15:27:41 -0500 Subject: [PATCH 2/3] fix bug in mcp server naming --- acp/config/manager/kustomization.yaml | 4 ++-- acp/internal/controller/task/task_controller.go | 16 ++++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/acp/config/manager/kustomization.yaml b/acp/config/manager/kustomization.yaml index 53e033b4..8be31060 100644 --- a/acp/config/manager/kustomization.yaml +++ b/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" diff --git a/acp/internal/controller/task/task_controller.go b/acp/internal/controller/task/task_controller.go index 0e2b8026..bc8c0a2d 100644 --- a/acp/internal/controller/task/task_controller.go +++ b/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 From 35f56eda6359b3f85ad83c1f3e652cbdd392bc80 Mon Sep 17 00:00:00 2001 From: Allison Durham Date: Tue, 15 Apr 2025 17:37:56 -0500 Subject: [PATCH 3/3] tests! --- acp/internal/adapters/mcp_adapter_test.go | 63 +++++++++++++++++++++++ acp/internal/adapters/suite_test.go | 13 +++++ 2 files changed, 76 insertions(+) create mode 100644 acp/internal/adapters/mcp_adapter_test.go create mode 100644 acp/internal/adapters/suite_test.go diff --git a/acp/internal/adapters/mcp_adapter_test.go b/acp/internal/adapters/mcp_adapter_test.go new file mode 100644 index 00000000..c9332eef --- /dev/null +++ b/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()) + }) + }) +}) diff --git a/acp/internal/adapters/suite_test.go b/acp/internal/adapters/suite_test.go new file mode 100644 index 00000000..5c111e71 --- /dev/null +++ b/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") +}