From 87945b8ce9301034b2f110650ae12f987b1ebfc4 Mon Sep 17 00:00:00 2001 From: dexhorthy Date: Tue, 20 May 2025 17:49:10 -0700 Subject: [PATCH 1/3] wip --- acp/api/v1alpha1/mcpserver_types.go | 27 +++++++++++++++++++ .../bases/acp.humanlayer.dev_mcpservers.yaml | 24 +++++++++++++++++ acp/config/localdev/kustomization.yaml | 2 +- acp/config/samples/mcp_github.yaml | 16 +++++++++++ acp/internal/mcpmanager/mcpmanager.go | 11 ++++++++ 5 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 acp/config/samples/mcp_github.yaml diff --git a/acp/api/v1alpha1/mcpserver_types.go b/acp/api/v1alpha1/mcpserver_types.go index 4f1164b2..5667c766 100644 --- a/acp/api/v1alpha1/mcpserver_types.go +++ b/acp/api/v1alpha1/mcpserver_types.go @@ -100,6 +100,33 @@ type MCPTool struct { // +kubebuilder:pruning:PreserveUnknownFields // +optional InputSchema runtime.RawExtension `json:"inputSchema,omitempty"` + + // Annotations provides additional metadata about the tool's behavior + // +optional + Annotations *MCPToolAnnotations `json:"annotations,omitempty"` +} + +// MCPToolAnnotations provides metadata about a tool's behavior +type MCPToolAnnotations struct { + // Title is a human-readable title for the tool + // +optional + Title string `json:"title,omitempty"` + + // ReadOnlyHint indicates if the tool does not modify its environment + // +optional + ReadOnlyHint bool `json:"readOnlyHint,omitempty"` + + // DestructiveHint indicates if the tool may perform destructive updates + // +optional + DestructiveHint bool `json:"destructiveHint,omitempty"` + + // IdempotentHint indicates if repeated calls with same args have no additional effect + // +optional + IdempotentHint bool `json:"idempotentHint,omitempty"` + + // OpenWorldHint indicates if the tool interacts with external entities + // +optional + OpenWorldHint bool `json:"openWorldHint,omitempty"` } // MCPServerStatus defines the observed state of MCPServer diff --git a/acp/config/crd/bases/acp.humanlayer.dev_mcpservers.yaml b/acp/config/crd/bases/acp.humanlayer.dev_mcpservers.yaml index 78968b45..11f24079 100644 --- a/acp/config/crd/bases/acp.humanlayer.dev_mcpservers.yaml +++ b/acp/config/crd/bases/acp.humanlayer.dev_mcpservers.yaml @@ -163,6 +163,30 @@ spec: items: description: MCPTool represents a tool provided by an MCP server properties: + annotations: + description: Annotations provides additional metadata about + the tool's behavior + properties: + destructiveHint: + description: DestructiveHint indicates if the tool may perform + destructive updates + type: boolean + idempotentHint: + description: IdempotentHint indicates if repeated calls + with same args have no additional effect + type: boolean + openWorldHint: + description: OpenWorldHint indicates if the tool interacts + with external entities + type: boolean + readOnlyHint: + description: ReadOnlyHint indicates if the tool does not + modify its environment + type: boolean + title: + description: Title is a human-readable title for the tool + type: string + type: object description: description: Description of the tool type: string diff --git a/acp/config/localdev/kustomization.yaml b/acp/config/localdev/kustomization.yaml index 6ac4f8cd..96bbff3d 100644 --- a/acp/config/localdev/kustomization.yaml +++ b/acp/config/localdev/kustomization.yaml @@ -26,4 +26,4 @@ patches: images: - name: controller newName: controller - newTag: "202505141822" + newTag: "202505201737" diff --git a/acp/config/samples/mcp_github.yaml b/acp/config/samples/mcp_github.yaml new file mode 100644 index 00000000..7f965ef0 --- /dev/null +++ b/acp/config/samples/mcp_github.yaml @@ -0,0 +1,16 @@ +apiVersion: acp.humanlayer.dev/v1alpha1 +kind: MCPServer +metadata: + name: github +spec: + transport: stdio + command: npx + args: + - -y + - "@modelcontextprotocol/server-github" + env: + - name: GITHUB_PERSONAL_ACCESS_TOKEN + valueFrom: + secretKeyRef: + name: github-token + key: token diff --git a/acp/internal/mcpmanager/mcpmanager.go b/acp/internal/mcpmanager/mcpmanager.go index 0b871c77..10ceadeb 100644 --- a/acp/internal/mcpmanager/mcpmanager.go +++ b/acp/internal/mcpmanager/mcpmanager.go @@ -199,10 +199,21 @@ func (m *MCPServerManager) ConnectServer(ctx context.Context, mcpServer *acp.MCP } } + // Create annotations based on tool properties + annotations := &acp.MCPToolAnnotations{ + Title: tool.Name, // Use name as title by default + // Set hints based on tool properties or defaults + ReadOnlyHint: false, // Default to false for safety + DestructiveHint: false, + IdempotentHint: false, + OpenWorldHint: false, + } + tools = append(tools, acp.MCPTool{ Name: tool.Name, Description: tool.Description, InputSchema: runtime.RawExtension{Raw: inputSchemaBytes}, + Annotations: annotations, }) } From 25d8c50388338a219bc1f2339e32fccfd21ce188 Mon Sep 17 00:00:00 2001 From: dexhorthy Date: Tue, 20 May 2025 18:33:48 -0700 Subject: [PATCH 2/3] pull annotations --- acp/api/v1alpha1/zz_generated.deepcopy.go | 20 ++++++++++++++++++ acp/go.mod | 15 +++++-------- acp/go.sum | 8 +++++-- .../mcpserver/mcpserver_controller_test.go | 12 +++++++++++ acp/internal/mcpmanager/mcpmanager.go | 19 +++++++---------- acp/internal/mcpmanager/mcpmanager_test.go | 21 +++++++++++++++++++ 6 files changed, 72 insertions(+), 23 deletions(-) diff --git a/acp/api/v1alpha1/zz_generated.deepcopy.go b/acp/api/v1alpha1/zz_generated.deepcopy.go index 30c11802..2fd97113 100644 --- a/acp/api/v1alpha1/zz_generated.deepcopy.go +++ b/acp/api/v1alpha1/zz_generated.deepcopy.go @@ -625,6 +625,11 @@ func (in *MCPServerStatus) DeepCopy() *MCPServerStatus { func (in *MCPTool) DeepCopyInto(out *MCPTool) { *out = *in in.InputSchema.DeepCopyInto(&out.InputSchema) + if in.Annotations != nil { + in, out := &in.Annotations, &out.Annotations + *out = new(MCPToolAnnotations) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPTool. @@ -637,6 +642,21 @@ func (in *MCPTool) DeepCopy() *MCPTool { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MCPToolAnnotations) DeepCopyInto(out *MCPToolAnnotations) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPToolAnnotations. +func (in *MCPToolAnnotations) DeepCopy() *MCPToolAnnotations { + if in == nil { + return nil + } + out := new(MCPToolAnnotations) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Message) DeepCopyInto(out *Message) { *out = *in diff --git a/acp/go.mod b/acp/go.mod index 533bcd62..5c6807f9 100644 --- a/acp/go.mod +++ b/acp/go.mod @@ -3,10 +3,10 @@ module github.com/humanlayer/agentcontrolplane/acp go 1.24.0 require ( - github.com/mark3labs/mcp-go v0.15.0 + github.com/gin-gonic/gin v1.10.0 + github.com/mark3labs/mcp-go v0.29.0 github.com/onsi/ginkgo/v2 v2.23.2 github.com/onsi/gomega v1.36.2 - github.com/openai/openai-go v0.1.0-alpha.59 github.com/tmc/langchaingo v0.1.13 go.opentelemetry.io/otel v1.34.0 go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v1.34.0 @@ -33,12 +33,10 @@ require ( github.com/bytedance/sonic v1.13.2 // indirect github.com/bytedance/sonic/loader v0.2.4 // indirect github.com/cloudwego/base64x v0.1.5 // indirect - github.com/cloudwego/iasm v0.2.0 // indirect github.com/dlclark/regexp2 v1.10.0 // indirect github.com/gabriel-vasile/mimetype v1.4.8 // indirect github.com/gage-technologies/mistral-go v1.1.0 // indirect github.com/gin-contrib/sse v1.1.0 // indirect - github.com/gin-gonic/gin v1.10.0 // indirect github.com/go-playground/locales v0.14.1 // indirect github.com/go-playground/universal-translator v0.18.1 // indirect github.com/go-playground/validator/v10 v10.26.0 // indirect @@ -53,10 +51,7 @@ require ( github.com/mattn/go-isatty v0.0.20 // indirect github.com/pelletier/go-toml/v2 v2.2.4 // indirect github.com/pkoukk/tiktoken-go v0.1.6 // indirect - github.com/tidwall/gjson v1.14.4 // indirect - github.com/tidwall/match v1.1.1 // indirect - github.com/tidwall/pretty v1.2.1 // indirect - github.com/tidwall/sjson v1.2.5 // indirect + github.com/spf13/cast v1.7.1 // indirect github.com/twitchyliquid64/golang-asm v0.15.1 // indirect github.com/ugorji/go/codec v1.2.12 // indirect github.com/yosida95/uritemplate/v3 v3.0.2 // indirect @@ -106,7 +101,7 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect - github.com/pkg/errors v0.9.1 // indirect + github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.19.1 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.55.0 // indirect @@ -119,7 +114,7 @@ require ( go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.53.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.34.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.27.0 // indirect - go.opentelemetry.io/otel/metric v1.34.0 + go.opentelemetry.io/otel/metric v1.34.0 // indirect go.opentelemetry.io/otel/trace v1.34.0 go.opentelemetry.io/proto/otlp v1.5.0 // indirect go.uber.org/multierr v1.11.0 // indirect diff --git a/acp/go.sum b/acp/go.sum index 7c5efd51..8498d943 100644 --- a/acp/go.sum +++ b/acp/go.sum @@ -63,6 +63,8 @@ github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0 github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= +github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= +github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA= github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv5E= @@ -176,8 +178,8 @@ github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ= github.com/leodido/go-urn v1.4.0/go.mod h1:bvxc+MVxLKB4z00jd1z+Dvzr47oO32F/QSNjSBOlFxI= github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= -github.com/mark3labs/mcp-go v0.15.0 h1:lViiC4dk6chJHZccezaTzZLMOQVUXJDGNQPtzExr5NQ= -github.com/mark3labs/mcp-go v0.15.0/go.mod h1:xBB350hekQsJAK7gJAii8bcEoWemboLm2mRm5/+KBaU= +github.com/mark3labs/mcp-go v0.29.0 h1:sH1NBcumKskhxqYzhXfGc201D7P76TVXiT0fGVhabeI= +github.com/mark3labs/mcp-go v0.29.0/go.mod h1:rXqOudj/djTORU/ThxYx8fqEVj/5pvTuuebQ2RC7uk4= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -212,6 +214,8 @@ github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoG github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/spf13/cast v1.7.1 h1:cuNEagBQEHWN1FnbGEjCXL2szYEXqfJPbP2HNUaca9Y= +github.com/spf13/cast v1.7.1/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo= github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= diff --git a/acp/internal/controller/mcpserver/mcpserver_controller_test.go b/acp/internal/controller/mcpserver/mcpserver_controller_test.go index a3fecd76..09ea5acd 100644 --- a/acp/internal/controller/mcpserver/mcpserver_controller_test.go +++ b/acp/internal/controller/mcpserver/mcpserver_controller_test.go @@ -120,6 +120,13 @@ var _ = Describe("MCPServer Controller", func() { { Name: "test-tool", Description: "A test tool", + Annotations: &acp.MCPToolAnnotations{ + Title: "Test Tool", + ReadOnlyHint: true, + DestructiveHint: false, + IdempotentHint: true, + OpenWorldHint: false, + }, }, }, true }, @@ -147,6 +154,11 @@ var _ = Describe("MCPServer Controller", func() { } return createdMCPServer.Status.Connected && len(createdMCPServer.Status.Tools) == 1 && + createdMCPServer.Status.Tools[0].Name == "test-tool" && + createdMCPServer.Status.Tools[0].Annotations != nil && + createdMCPServer.Status.Tools[0].Annotations.Title == "Test Tool" && + createdMCPServer.Status.Tools[0].Annotations.ReadOnlyHint == true && + createdMCPServer.Status.Tools[0].Annotations.IdempotentHint == true && createdMCPServer.Status.Status == "Ready" }, time.Second*10, time.Millisecond*250).Should(BeTrue()) }) diff --git a/acp/internal/mcpmanager/mcpmanager.go b/acp/internal/mcpmanager/mcpmanager.go index 10ceadeb..a9ff05d3 100644 --- a/acp/internal/mcpmanager/mcpmanager.go +++ b/acp/internal/mcpmanager/mcpmanager.go @@ -201,12 +201,11 @@ func (m *MCPServerManager) ConnectServer(ctx context.Context, mcpServer *acp.MCP // Create annotations based on tool properties annotations := &acp.MCPToolAnnotations{ - Title: tool.Name, // Use name as title by default - // Set hints based on tool properties or defaults - ReadOnlyHint: false, // Default to false for safety - DestructiveHint: false, - IdempotentHint: false, - OpenWorldHint: false, + Title: tool.Annotations.Title, // Default title is the tool's name if Title is empty + ReadOnlyHint: tool.Annotations.ReadOnlyHint != nil && *tool.Annotations.ReadOnlyHint, + DestructiveHint: tool.Annotations.DestructiveHint != nil && *tool.Annotations.DestructiveHint, + IdempotentHint: tool.Annotations.IdempotentHint != nil && *tool.Annotations.IdempotentHint, + OpenWorldHint: tool.Annotations.OpenWorldHint != nil && *tool.Annotations.OpenWorldHint, } tools = append(tools, acp.MCPTool{ @@ -293,11 +292,9 @@ func (m *MCPServerManager) CallTool(ctx context.Context, serverName, toolName st result, err := conn.Client.CallTool(ctx, mcp.CallToolRequest{ Params: struct { - Name string `json:"name"` - Arguments map[string]interface{} `json:"arguments,omitempty"` - Meta *struct { - ProgressToken mcp.ProgressToken `json:"progressToken,omitempty"` - } `json:"_meta,omitempty"` + Name string `json:"name"` + Arguments interface{} `json:"arguments,omitempty"` + Meta *mcp.Meta `json:"_meta,omitempty"` }{ Name: toolName, Arguments: arguments, diff --git a/acp/internal/mcpmanager/mcpmanager_test.go b/acp/internal/mcpmanager/mcpmanager_test.go index a2f58b05..b13203cb 100644 --- a/acp/internal/mcpmanager/mcpmanager_test.go +++ b/acp/internal/mcpmanager/mcpmanager_test.go @@ -12,10 +12,13 @@ import ( ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" acp "github.com/humanlayer/agentcontrolplane/acp/api/v1alpha1" + mcpclient "github.com/mark3labs/mcp-go/client" "github.com/mark3labs/mcp-go/mcp" ) // MockMCPClient mocks the mcpclient.MCPClient interface for testing +var _ mcpclient.MCPClient = &MockMCPClient{} + type MockMCPClient struct { // Results initResult *mcp.InitializeResult @@ -74,6 +77,12 @@ func (m *MockMCPClient) ListTools(ctx context.Context, req mcp.ListToolsRequest) return m.toolsResult, m.toolsError } +// ListToolsByPage implements mcpclient.MCPClient +func (m *MockMCPClient) ListToolsByPage(ctx context.Context, req mcp.ListToolsRequest) (*mcp.ListToolsResult, error) { + m.toolsCallCount++ + return m.toolsResult, m.toolsError +} + // CallTool implements mcpclient.MCPClient func (m *MockMCPClient) CallTool(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { m.callToolCallCount++ @@ -95,10 +104,18 @@ func (m *MockMCPClient) ListResources(ctx context.Context, req mcp.ListResources return nil, nil } +func (m *MockMCPClient) ListResourcesByPage(ctx context.Context, req mcp.ListResourcesRequest) (*mcp.ListResourcesResult, error) { + return nil, nil +} + func (m *MockMCPClient) ListResourceTemplates(ctx context.Context, req mcp.ListResourceTemplatesRequest) (*mcp.ListResourceTemplatesResult, error) { return nil, nil } +func (m *MockMCPClient) ListResourceTemplatesByPage(ctx context.Context, req mcp.ListResourceTemplatesRequest) (*mcp.ListResourceTemplatesResult, error) { + return nil, nil +} + func (m *MockMCPClient) ReadResource(ctx context.Context, req mcp.ReadResourceRequest) (*mcp.ReadResourceResult, error) { return nil, nil } @@ -111,6 +128,10 @@ func (m *MockMCPClient) ListPrompts(ctx context.Context, req mcp.ListPromptsRequ return nil, nil } +func (m *MockMCPClient) ListPromptsByPage(ctx context.Context, req mcp.ListPromptsRequest) (*mcp.ListPromptsResult, error) { + return nil, nil +} + func (m *MockMCPClient) GetPrompt(ctx context.Context, req mcp.GetPromptRequest) (*mcp.GetPromptResult, error) { return nil, nil } From 280697d48153f4747afe58bad1e52f169677199a Mon Sep 17 00:00:00 2001 From: Dex Date: Tue, 20 May 2025 20:55:09 -0500 Subject: [PATCH 3/3] Update acp/internal/mcpmanager/mcpmanager.go --- acp/internal/mcpmanager/mcpmanager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acp/internal/mcpmanager/mcpmanager.go b/acp/internal/mcpmanager/mcpmanager.go index a9ff05d3..00128226 100644 --- a/acp/internal/mcpmanager/mcpmanager.go +++ b/acp/internal/mcpmanager/mcpmanager.go @@ -201,7 +201,7 @@ func (m *MCPServerManager) ConnectServer(ctx context.Context, mcpServer *acp.MCP // Create annotations based on tool properties annotations := &acp.MCPToolAnnotations{ - Title: tool.Annotations.Title, // Default title is the tool's name if Title is empty + Title: tool.Annotations.Title, ReadOnlyHint: tool.Annotations.ReadOnlyHint != nil && *tool.Annotations.ReadOnlyHint, DestructiveHint: tool.Annotations.DestructiveHint != nil && *tool.Annotations.DestructiveHint, IdempotentHint: tool.Annotations.IdempotentHint != nil && *tool.Annotations.IdempotentHint,