From d164fa6ffaa36bf2eb82a12d432f077fcdd25a51 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Thu, 29 Jan 2026 09:44:37 +0200 Subject: [PATCH 1/2] Implement ExcludeAll feature for VirtualMCPServer tool filtering Add runtime support for the excludeAll configuration fields that were already defined in the CRD schema but not implemented: - AggregationConfig.ExcludeAllTools (global): excludes all tools from all backends when true - WorkloadToolConfig.ExcludeAll (per-workload): excludes all tools from a specific backend workload when true The ExcludeAll check takes precedence over Filter and Overrides settings, allowing users to easily exclude all tools from a backend without needing to specify a non-matching filter workaround. Changes: - Add ExcludeAll check in processBackendTools (tool_adapter.go) - Add excludeAllTools field and logic to defaultAggregator - Update NewDefaultAggregator to accept *AggregationConfig instead of []*WorkloadToolConfig for better encapsulation - Add comprehensive unit tests for both per-workload and global exclusion - Update E2E test to use ExcludeAll instead of filter workaround - Add new E2E test for global ExcludeAllTools feature Fixes #2779 Co-Authored-By: Claude Opus 4.5 --- cmd/vmcp/app/commands.go | 2 +- pkg/vmcp/aggregator/default_aggregator.go | 25 ++- .../aggregator/default_aggregator_test.go | 88 +++++++++ pkg/vmcp/aggregator/tool_adapter.go | 6 + pkg/vmcp/aggregator/tool_adapter_test.go | 44 +++++ .../virtualmcp_aggregation_filtering_test.go | 15 +- .../virtualmcp_excludeall_global_test.go | 179 ++++++++++++++++++ 7 files changed, 342 insertions(+), 17 deletions(-) create mode 100644 test/e2e/thv-operator/virtualmcp/virtualmcp_excludeall_global_test.go diff --git a/cmd/vmcp/app/commands.go b/cmd/vmcp/app/commands.go index c4ebc88845..00077bc02e 100644 --- a/cmd/vmcp/app/commands.go +++ b/cmd/vmcp/app/commands.go @@ -342,7 +342,7 @@ func runServe(cmd *cobra.Command, _ []string) error { if telemetryProvider != nil { tracerProvider = telemetryProvider.TracerProvider() } - agg := aggregator.NewDefaultAggregator(backendClient, conflictResolver, cfg.Aggregation.Tools, tracerProvider) + agg := aggregator.NewDefaultAggregator(backendClient, conflictResolver, cfg.Aggregation, tracerProvider) // Use DynamicRegistry for version-based cache invalidation // Works in both standalone (CLI with YAML config) and Kubernetes (operator-deployed) modes diff --git a/pkg/vmcp/aggregator/default_aggregator.go b/pkg/vmcp/aggregator/default_aggregator.go index ca51d207d8..eabbfe11af 100644 --- a/pkg/vmcp/aggregator/default_aggregator.go +++ b/pkg/vmcp/aggregator/default_aggregator.go @@ -25,24 +25,30 @@ type defaultAggregator struct { backendClient vmcp.BackendClient conflictResolver ConflictResolver toolConfigMap map[string]*config.WorkloadToolConfig // Maps backend ID to tool config + excludeAllTools bool // Global flag to exclude all tools tracer trace.Tracer } // NewDefaultAggregator creates a new default aggregator implementation. // conflictResolver handles tool name conflicts across backends. -// workloadConfigs specifies per-backend tool filtering and overrides. +// aggregationConfig specifies aggregation settings including tool filtering/overrides and excludeAllTools. // tracerProvider is used to create a tracer for distributed tracing (pass nil for no tracing). func NewDefaultAggregator( backendClient vmcp.BackendClient, conflictResolver ConflictResolver, - workloadConfigs []*config.WorkloadToolConfig, + aggregationConfig *config.AggregationConfig, tracerProvider trace.TracerProvider, ) Aggregator { // Build tool config map for quick lookup by backend ID toolConfigMap := make(map[string]*config.WorkloadToolConfig) - for _, wlConfig := range workloadConfigs { - if wlConfig != nil { - toolConfigMap[wlConfig.Workload] = wlConfig + var excludeAllTools bool + + if aggregationConfig != nil { + excludeAllTools = aggregationConfig.ExcludeAllTools + for _, wlConfig := range aggregationConfig.Tools { + if wlConfig != nil { + toolConfigMap[wlConfig.Workload] = wlConfig + } } } @@ -58,6 +64,7 @@ func NewDefaultAggregator( backendClient: backendClient, conflictResolver: conflictResolver, toolConfigMap: toolConfigMap, + excludeAllTools: excludeAllTools, tracer: tracer, } } @@ -91,7 +98,13 @@ func (a *defaultAggregator) QueryCapabilities(ctx context.Context, backend vmcp. } // Apply per-backend tool filtering and overrides (before conflict resolution) - processedTools := processBackendTools(ctx, backend.ID, capabilities.Tools, a.toolConfigMap[backend.ID]) + var processedTools []vmcp.Tool + if a.excludeAllTools { + logger.Debugf("ExcludeAllTools is true globally, returning empty tools for backend %s", backend.ID) + processedTools = []vmcp.Tool{} + } else { + processedTools = processBackendTools(ctx, backend.ID, capabilities.Tools, a.toolConfigMap[backend.ID]) + } // Convert to BackendCapabilities result := &BackendCapabilities{ diff --git a/pkg/vmcp/aggregator/default_aggregator_test.go b/pkg/vmcp/aggregator/default_aggregator_test.go index 6d07c23af5..11b4cb3fae 100644 --- a/pkg/vmcp/aggregator/default_aggregator_test.go +++ b/pkg/vmcp/aggregator/default_aggregator_test.go @@ -13,6 +13,7 @@ import ( "go.uber.org/mock/gomock" "github.com/stacklok/toolhive/pkg/vmcp" + "github.com/stacklok/toolhive/pkg/vmcp/config" "github.com/stacklok/toolhive/pkg/vmcp/mocks" ) @@ -345,3 +346,90 @@ func TestDefaultAggregator_AggregateCapabilities(t *testing.T) { assert.Equal(t, 1, result.Metadata.ResourceCount) }) } + +func TestDefaultAggregator_ExcludeAllTools(t *testing.T) { + t.Parallel() + + t.Run("global excludeAllTools returns empty tools", func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := mocks.NewMockBackendClient(ctrl) + backend := newTestBackend("backend1", withBackendName("Backend 1")) + + // Backend returns tools, but they should be excluded + expectedCaps := newTestCapabilityList( + withTools(newTestTool("test_tool", "backend1")), + withResources(newTestResource("test://resource", "backend1")), + withPrompts(newTestPrompt("test_prompt", "backend1")), + withLogging(true)) + + mockClient.EXPECT().ListCapabilities(gomock.Any(), gomock.Any()).Return(expectedCaps, nil) + + // Create aggregator with ExcludeAllTools: true + aggregationConfig := &config.AggregationConfig{ + ExcludeAllTools: true, + } + agg := NewDefaultAggregator(mockClient, nil, aggregationConfig, nil) + result, err := agg.QueryCapabilities(context.Background(), backend) + + require.NoError(t, err) + assert.Equal(t, "backend1", result.BackendID) + // Tools should be empty due to ExcludeAllTools + assert.Len(t, result.Tools, 0) + // Resources and prompts should be preserved + assert.Len(t, result.Resources, 1) + assert.Len(t, result.Prompts, 1) + assert.True(t, result.SupportsLogging) + }) + + t.Run("global excludeAllTools false allows tools through", func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := mocks.NewMockBackendClient(ctrl) + backend := newTestBackend("backend1", withBackendName("Backend 1")) + + expectedCaps := newTestCapabilityList( + withTools(newTestTool("test_tool", "backend1"))) + + mockClient.EXPECT().ListCapabilities(gomock.Any(), gomock.Any()).Return(expectedCaps, nil) + + // Create aggregator with ExcludeAllTools: false (default) + aggregationConfig := &config.AggregationConfig{ + ExcludeAllTools: false, + } + agg := NewDefaultAggregator(mockClient, nil, aggregationConfig, nil) + result, err := agg.QueryCapabilities(context.Background(), backend) + + require.NoError(t, err) + // Tools should come through + assert.Len(t, result.Tools, 1) + assert.Equal(t, "test_tool", result.Tools[0].Name) + }) + + t.Run("nil aggregationConfig allows tools through", func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := mocks.NewMockBackendClient(ctrl) + backend := newTestBackend("backend1", withBackendName("Backend 1")) + + expectedCaps := newTestCapabilityList( + withTools(newTestTool("test_tool", "backend1"))) + + mockClient.EXPECT().ListCapabilities(gomock.Any(), gomock.Any()).Return(expectedCaps, nil) + + // Create aggregator with nil aggregationConfig (default behavior) + agg := NewDefaultAggregator(mockClient, nil, nil, nil) + result, err := agg.QueryCapabilities(context.Background(), backend) + + require.NoError(t, err) + // Tools should come through + assert.Len(t, result.Tools, 1) + assert.Equal(t, "test_tool", result.Tools[0].Name) + }) +} diff --git a/pkg/vmcp/aggregator/tool_adapter.go b/pkg/vmcp/aggregator/tool_adapter.go index 231ed02480..de82b89138 100644 --- a/pkg/vmcp/aggregator/tool_adapter.go +++ b/pkg/vmcp/aggregator/tool_adapter.go @@ -29,6 +29,12 @@ func processBackendTools( return tools // No configuration for this backend } + // Check ExcludeAll first - takes precedence over Filter/Overrides + if workloadConfig.ExcludeAll { + logger.Debugf("ExcludeAll is true for backend %s, returning empty tools list", backendID) + return []vmcp.Tool{} + } + // If no filter or overrides configured, return tools as-is if len(workloadConfig.Filter) == 0 && len(workloadConfig.Overrides) == 0 { return tools diff --git a/pkg/vmcp/aggregator/tool_adapter_test.go b/pkg/vmcp/aggregator/tool_adapter_test.go index 14b6d5a29f..6a6cdae008 100644 --- a/pkg/vmcp/aggregator/tool_adapter_test.go +++ b/pkg/vmcp/aggregator/tool_adapter_test.go @@ -118,6 +118,50 @@ func TestProcessBackendTools(t *testing.T) { wantCount: 1, wantNames: []string{"renamed_tool1"}, }, + { + name: "excludeAll excludes all tools from workload", + backendID: "github", + tools: []vmcp.Tool{ + {Name: "create_pr", Description: "Create PR", BackendID: "github"}, + {Name: "merge_pr", Description: "Merge PR", BackendID: "github"}, + }, + workloadConfig: &config.WorkloadToolConfig{ + Workload: "github", + ExcludeAll: true, + }, + wantCount: 0, + wantNames: []string{}, + }, + { + name: "excludeAll takes precedence over filter", + backendID: "github", + tools: []vmcp.Tool{ + {Name: "create_pr", Description: "Create PR", BackendID: "github"}, + }, + workloadConfig: &config.WorkloadToolConfig{ + Workload: "github", + ExcludeAll: true, + Filter: []string{"create_pr"}, + }, + wantCount: 0, + wantNames: []string{}, + }, + { + name: "excludeAll takes precedence over overrides", + backendID: "github", + tools: []vmcp.Tool{ + {Name: "create_pr", Description: "Create PR", BackendID: "github"}, + }, + workloadConfig: &config.WorkloadToolConfig{ + Workload: "github", + ExcludeAll: true, + Overrides: map[string]*config.ToolOverride{ + "create_pr": {Name: "gh_create_pr"}, + }, + }, + wantCount: 0, + wantNames: []string{}, + }, } for _, tt := range tests { diff --git a/test/e2e/thv-operator/virtualmcp/virtualmcp_aggregation_filtering_test.go b/test/e2e/thv-operator/virtualmcp/virtualmcp_aggregation_filtering_test.go index 4e216606c4..534ce5ad2e 100644 --- a/test/e2e/thv-operator/virtualmcp/virtualmcp_aggregation_filtering_test.go +++ b/test/e2e/thv-operator/virtualmcp/virtualmcp_aggregation_filtering_test.go @@ -54,17 +54,14 @@ var _ = Describe("VirtualMCPServer Aggregation Filtering", Ordered, func() { Aggregation: &vmcpconfig.AggregationConfig{ ConflictResolution: "prefix", // Tool filtering: only allow echo from backend1, nothing from backend2 - // TODO(#2779): Currently there's no way to exclude all tools from a backend. - // Using a non-matching filter as a workaround until excludeAll is implemented. - // See: https://github.com/stacklok/toolhive/issues/2779 Tools: []*vmcpconfig.WorkloadToolConfig{ { Workload: backend1Name, Filter: []string{"echo"}, // Only expose echo tool }, { - Workload: backend2Name, - Filter: []string{"nonexistent_tool"}, // Filter out all tools (workaround) + Workload: backend2Name, + ExcludeAll: true, // Exclude all tools from backend2 }, }, }, @@ -149,15 +146,14 @@ var _ = Describe("VirtualMCPServer Aggregation Filtering", Ordered, func() { } Expect(hasBackend1Tool).To(BeTrue(), "Should have echo tool from backend1") - // Should NOT have any tool from backend2 (filtered with non-matching filter) - // TODO(#2779): Once excludeAll is implemented, update this test to use it + // Should NOT have any tool from backend2 (excluded with ExcludeAll: true) hasBackend2Tool := false for _, name := range toolNames { if strings.Contains(name, backend2Name) { hasBackend2Tool = true } } - Expect(hasBackend2Tool).To(BeFalse(), "Should NOT have any tool from backend2 (filtered out via non-matching filter)") + Expect(hasBackend2Tool).To(BeFalse(), "Should NOT have any tool from backend2 (excluded via ExcludeAll: true)") }) It("should still allow calling filtered tools", func() { @@ -194,8 +190,7 @@ var _ = Describe("VirtualMCPServer Aggregation Filtering", Ordered, func() { Expect(backend1Config.Filter).To(ContainElement("echo")) Expect(backend2Config).ToNot(BeNil()) - // TODO(#2779): Once excludeAll is implemented, update this to use excludeAll: true - Expect(backend2Config.Filter).To(ContainElement("nonexistent_tool"), "Backend2 should have non-matching filter as workaround") + Expect(backend2Config.ExcludeAll).To(BeTrue(), "Backend2 should have ExcludeAll: true") }) }) }) diff --git a/test/e2e/thv-operator/virtualmcp/virtualmcp_excludeall_global_test.go b/test/e2e/thv-operator/virtualmcp/virtualmcp_excludeall_global_test.go new file mode 100644 index 0000000000..260a39fc36 --- /dev/null +++ b/test/e2e/thv-operator/virtualmcp/virtualmcp_excludeall_global_test.go @@ -0,0 +1,179 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package virtualmcp + +import ( + "fmt" + "time" + + "github.com/mark3labs/mcp-go/mcp" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" + vmcpconfig "github.com/stacklok/toolhive/pkg/vmcp/config" + "github.com/stacklok/toolhive/test/e2e/images" +) + +var _ = Describe("VirtualMCPServer Global ExcludeAllTools", Ordered, func() { + var ( + testNamespace = "default" + mcpGroupName = "test-excludeall-global-group" + vmcpServerName = "test-vmcp-excludeall-global" + backend1Name = "yardstick-excludeall-a" + backend2Name = "yardstick-excludeall-b" + timeout = 3 * time.Minute + pollingInterval = 1 * time.Second + vmcpNodePort int32 + ) + + BeforeAll(func() { + By("Creating MCPGroup for global excludeAllTools test") + CreateMCPGroupAndWait(ctx, k8sClient, mcpGroupName, testNamespace, + "Test MCP Group for global excludeAllTools E2E tests", timeout, pollingInterval) + + By("Creating yardstick backend MCPServers in parallel") + CreateMultipleMCPServersInParallel(ctx, k8sClient, []BackendConfig{ + {Name: backend1Name, Namespace: testNamespace, GroupRef: mcpGroupName, Image: images.YardstickServerImage}, + {Name: backend2Name, Namespace: testNamespace, GroupRef: mcpGroupName, Image: images.YardstickServerImage}, + }, timeout, pollingInterval) + + By("Creating VirtualMCPServer with global excludeAllTools: true") + vmcpServer := &mcpv1alpha1.VirtualMCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: vmcpServerName, + Namespace: testNamespace, + }, + Spec: mcpv1alpha1.VirtualMCPServerSpec{ + Config: vmcpconfig.Config{ + Group: mcpGroupName, + Aggregation: &vmcpconfig.AggregationConfig{ + ConflictResolution: "prefix", + // Global flag to exclude all tools from all backends + ExcludeAllTools: true, + }, + }, + IncomingAuth: &mcpv1alpha1.IncomingAuthConfig{ + Type: "anonymous", + }, + ServiceType: "NodePort", + }, + } + Expect(k8sClient.Create(ctx, vmcpServer)).To(Succeed()) + + By("Waiting for VirtualMCPServer to be ready") + WaitForVirtualMCPServerReady(ctx, k8sClient, vmcpServerName, testNamespace, timeout, pollingInterval) + + By("Getting NodePort for VirtualMCPServer") + vmcpNodePort = GetVMCPNodePort(ctx, k8sClient, vmcpServerName, testNamespace, timeout, pollingInterval) + + By(fmt.Sprintf("VirtualMCPServer accessible at http://localhost:%d", vmcpNodePort)) + }) + + AfterAll(func() { + By("Cleaning up VirtualMCPServer") + vmcpServer := &mcpv1alpha1.VirtualMCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: vmcpServerName, + Namespace: testNamespace, + }, + } + _ = k8sClient.Delete(ctx, vmcpServer) + + By("Cleaning up backend MCPServers") + for _, backendName := range []string{backend1Name, backend2Name} { + backend := &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: backendName, + Namespace: testNamespace, + }, + } + _ = k8sClient.Delete(ctx, backend) + } + + By("Cleaning up MCPGroup") + mcpGroup := &mcpv1alpha1.MCPGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: mcpGroupName, + Namespace: testNamespace, + }, + } + _ = k8sClient.Delete(ctx, mcpGroup) + }) + + Context("when global excludeAllTools is enabled", func() { + It("should return empty tools list from all backends", func() { + By("Creating and initializing MCP client for VirtualMCPServer") + mcpClient, err := CreateInitializedMCPClient(vmcpNodePort, "toolhive-excludeall-test", 30*time.Second) + Expect(err).ToNot(HaveOccurred()) + defer mcpClient.Close() + + By("Listing tools from VirtualMCPServer") + listRequest := mcp.ListToolsRequest{} + tools, err := mcpClient.Client.ListTools(mcpClient.Ctx, listRequest) + Expect(err).ToNot(HaveOccurred()) + + By(fmt.Sprintf("VirtualMCPServer returns %d tools with excludeAllTools: true", len(tools.Tools))) + + // Verify tools list is empty due to global excludeAllTools + Expect(tools.Tools).To(BeEmpty(), "Should have no tools when excludeAllTools is true globally") + }) + + It("should still respond to MCP protocol requests", func() { + By("Creating and initializing MCP client") + mcpClient, err := CreateInitializedMCPClient(vmcpNodePort, "toolhive-excludeall-protocol-test", 30*time.Second) + Expect(err).ToNot(HaveOccurred()) + defer mcpClient.Close() + + By("Verifying server responds to tools/list even when empty") + listRequest := mcp.ListToolsRequest{} + tools, err := mcpClient.Client.ListTools(mcpClient.Ctx, listRequest) + Expect(err).ToNot(HaveOccurred(), "Server should respond to tools/list request") + Expect(tools).ToNot(BeNil(), "Response should not be nil") + + // The response should be valid but with empty tools + Expect(tools.Tools).To(BeEmpty()) + }) + }) + + Context("when verifying excludeAllTools configuration", func() { + It("should have correct aggregation configuration with excludeAllTools", func() { + vmcpServer := &mcpv1alpha1.VirtualMCPServer{} + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: vmcpServerName, + Namespace: testNamespace, + }, vmcpServer) + Expect(err).ToNot(HaveOccurred()) + + Expect(vmcpServer.Spec.Config.Aggregation).ToNot(BeNil()) + Expect(vmcpServer.Spec.Config.Aggregation.ExcludeAllTools).To(BeTrue(), + "Global excludeAllTools should be true") + }) + + It("should have backends discovered but tools excluded", func() { + // Verify backends are in the group + backends, err := GetMCPGroupBackends(ctx, k8sClient, mcpGroupName, testNamespace) + Expect(err).ToNot(HaveOccurred()) + Expect(backends).To(HaveLen(2), "Should have 2 backends in the group") + + // Verify each backend is running + for _, backend := range backends { + Expect(backend.Status.Phase).To(Equal(mcpv1alpha1.MCPServerPhaseRunning), + fmt.Sprintf("Backend %s should be running", backend.Name)) + } + + // Even though backends are running, tools should be excluded + mcpClient, err := CreateInitializedMCPClient(vmcpNodePort, "toolhive-backend-verify-test", 30*time.Second) + Expect(err).ToNot(HaveOccurred()) + defer mcpClient.Close() + + listRequest := mcp.ListToolsRequest{} + tools, err := mcpClient.Client.ListTools(mcpClient.Ctx, listRequest) + Expect(err).ToNot(HaveOccurred()) + Expect(tools.Tools).To(BeEmpty(), "Tools should be excluded despite backends being available") + }) + }) +}) From 9ab550a7d16f23c9f83e8a79feb4866e5835c93e Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Thu, 29 Jan 2026 19:18:33 +0200 Subject: [PATCH 2/2] Add test for ExcludeAll preserving routing table for composite tools When ExcludeAll is set on a backend's tools: - Tools are NOT advertised to the LLM (excluded from tools/list) - BUT tools ARE available in the routing table for composite tools to use This enables the use case where you want to hide raw backend tools from direct LLM access while still allowing curated composite tool workflows to use those backend tools internally. Implementation changes: - ExcludeAll is now applied in MergeCapabilities (when building advertised tools list), not in QueryCapabilities or processBackendTools - The routing table always contains ALL tools (after Filter/Overrides) - Filter and Overrides are still applied in processBackendTools Co-Authored-By: Claude Opus 4.5 --- pkg/vmcp/aggregator/default_aggregator.go | 57 +++++++-- .../aggregator/default_aggregator_test.go | 120 +++++++++++++++++- pkg/vmcp/aggregator/tool_adapter.go | 9 +- pkg/vmcp/aggregator/tool_adapter_test.go | 24 ++-- 4 files changed, 179 insertions(+), 31 deletions(-) diff --git a/pkg/vmcp/aggregator/default_aggregator.go b/pkg/vmcp/aggregator/default_aggregator.go index eabbfe11af..5265b641bc 100644 --- a/pkg/vmcp/aggregator/default_aggregator.go +++ b/pkg/vmcp/aggregator/default_aggregator.go @@ -98,13 +98,11 @@ func (a *defaultAggregator) QueryCapabilities(ctx context.Context, backend vmcp. } // Apply per-backend tool filtering and overrides (before conflict resolution) - var processedTools []vmcp.Tool - if a.excludeAllTools { - logger.Debugf("ExcludeAllTools is true globally, returning empty tools for backend %s", backend.ID) - processedTools = []vmcp.Tool{} - } else { - processedTools = processBackendTools(ctx, backend.ID, capabilities.Tools, a.toolConfigMap[backend.ID]) - } + // NOTE: ExcludeAll (both global and per-workload) is NOT applied here. + // This is intentional - we need all tools in the routing table so composite + // tools can call backend tools. ExcludeAll is applied in MergeCapabilities + // to control which tools are advertised to the LLM. + processedTools := processBackendTools(ctx, backend.ID, capabilities.Tools, a.toolConfigMap[backend.ID]) // Convert to BackendCapabilities result := &BackendCapabilities{ @@ -313,15 +311,24 @@ func (a *defaultAggregator) MergeCapabilities( } // Convert resolved tools to final vmcp.Tool format + // The routing table gets ALL tools (for composite tool routing) + // The advertised tools list only gets non-excluded tools (for LLM) tools := make([]vmcp.Tool, 0, len(resolved.Tools)) for _, resolvedTool := range resolved.Tools { - tools = append(tools, vmcp.Tool{ - Name: resolvedTool.ResolvedName, - Description: resolvedTool.Description, - InputSchema: resolvedTool.InputSchema, - BackendID: resolvedTool.BackendID, - }) + // Check if this tool should be excluded from the advertised list + // ExcludeAll only affects advertising, not routing + shouldAdvertise := a.shouldAdvertiseTool(resolvedTool.BackendID) + + if shouldAdvertise { + tools = append(tools, vmcp.Tool{ + Name: resolvedTool.ResolvedName, + Description: resolvedTool.Description, + InputSchema: resolvedTool.InputSchema, + BackendID: resolvedTool.BackendID, + }) + } + // ALWAYS add to routing table (for composite tools to call excluded backend tools) // Look up full backend information from registry backend := registry.Get(ctx, resolvedTool.BackendID) if backend == nil { @@ -479,3 +486,27 @@ func (a *defaultAggregator) AggregateCapabilities( return aggregated, nil } + +// shouldAdvertiseTool returns true if a tool from the given backend should be +// advertised to the LLM (included in tools/list response). +// +// ExcludeAll settings control advertising, not routing: +// - Tools excluded via ExcludeAll are NOT advertised to the LLM +// - BUT they ARE available in the routing table for composite tools to use +// +// This enables the use case where you want to hide raw backend tools from +// direct LLM access while still allowing curated composite workflows to use them. +func (a *defaultAggregator) shouldAdvertiseTool(backendID string) bool { + // Global ExcludeAllTools takes precedence - excludes all tools from all backends + if a.excludeAllTools { + return false + } + + // Check per-workload ExcludeAll setting + if wlConfig, exists := a.toolConfigMap[backendID]; exists && wlConfig.ExcludeAll { + return false + } + + // Tool should be advertised + return true +} diff --git a/pkg/vmcp/aggregator/default_aggregator_test.go b/pkg/vmcp/aggregator/default_aggregator_test.go index 11b4cb3fae..c15029bf40 100644 --- a/pkg/vmcp/aggregator/default_aggregator_test.go +++ b/pkg/vmcp/aggregator/default_aggregator_test.go @@ -350,7 +350,11 @@ func TestDefaultAggregator_AggregateCapabilities(t *testing.T) { func TestDefaultAggregator_ExcludeAllTools(t *testing.T) { t.Parallel() - t.Run("global excludeAllTools returns empty tools", func(t *testing.T) { + // NOTE: ExcludeAll is applied in MergeCapabilities, NOT in QueryCapabilities. + // This allows the routing table to contain all tools (for composite tools) + // while only filtering the advertised tools list. + + t.Run("QueryCapabilities returns all tools even with global excludeAllTools", func(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -358,7 +362,8 @@ func TestDefaultAggregator_ExcludeAllTools(t *testing.T) { mockClient := mocks.NewMockBackendClient(ctrl) backend := newTestBackend("backend1", withBackendName("Backend 1")) - // Backend returns tools, but they should be excluded + // Backend returns tools - they should still be returned by QueryCapabilities + // because ExcludeAll is applied later in MergeCapabilities expectedCaps := newTestCapabilityList( withTools(newTestTool("test_tool", "backend1")), withResources(newTestResource("test://resource", "backend1")), @@ -376,8 +381,9 @@ func TestDefaultAggregator_ExcludeAllTools(t *testing.T) { require.NoError(t, err) assert.Equal(t, "backend1", result.BackendID) - // Tools should be empty due to ExcludeAllTools - assert.Len(t, result.Tools, 0) + // Tools should still be present (ExcludeAll is applied in MergeCapabilities) + assert.Len(t, result.Tools, 1) + assert.Equal(t, "test_tool", result.Tools[0].Name) // Resources and prompts should be preserved assert.Len(t, result.Resources, 1) assert.Len(t, result.Prompts, 1) @@ -433,3 +439,109 @@ func TestDefaultAggregator_ExcludeAllTools(t *testing.T) { assert.Equal(t, "test_tool", result.Tools[0].Name) }) } + +func TestDefaultAggregator_ExcludeAllPreservesRoutingTableForCompositeTools(t *testing.T) { + t.Parallel() + + // This test verifies that ExcludeAll only affects the advertised tools list, + // NOT the routing table. This is important because composite tools need to + // route to backend tools that may be excluded from direct LLM access. + // + // Use case: A vMCP server may want to hide raw backend tools from the LLM + // (using ExcludeAll) while still allowing curated composite tool workflows + // to use those backend tools internally. + + t.Run("per-workload excludeAll preserves routing table for composite tools", func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := mocks.NewMockBackendClient(ctrl) + backends := []vmcp.Backend{ + newTestBackend("github", withBackendName("GitHub")), + } + + // Backend has tools that should be available for composite tools + caps := newTestCapabilityList( + withTools( + newTestTool("create_issue", "github"), + newTestTool("list_issues", "github"), + ), + ) + + mockClient.EXPECT().ListCapabilities(gomock.Any(), gomock.Any()).Return(caps, nil) + + // Configure ExcludeAll for the github backend + aggregationConfig := &config.AggregationConfig{ + Tools: []*config.WorkloadToolConfig{ + { + Workload: "github", + ExcludeAll: true, + }, + }, + } + + agg := NewDefaultAggregator(mockClient, nil, aggregationConfig, nil) + result, err := agg.AggregateCapabilities(context.Background(), backends) + + require.NoError(t, err) + assert.NotNil(t, result) + + // Advertised tools should be empty (excluded from LLM) + assert.Empty(t, result.Tools, "ExcludeAll should hide tools from LLM") + + // BUT the routing table should still contain the tools (for composite tools) + assert.NotNil(t, result.RoutingTable) + assert.Contains(t, result.RoutingTable.Tools, "create_issue", + "Routing table should contain excluded tools for composite tool use") + assert.Contains(t, result.RoutingTable.Tools, "list_issues", + "Routing table should contain excluded tools for composite tool use") + + // Verify the routing targets are properly configured + createIssueTarget := result.RoutingTable.Tools["create_issue"] + assert.NotNil(t, createIssueTarget) + assert.Equal(t, "github", createIssueTarget.WorkloadID) + }) + + t.Run("global excludeAllTools preserves routing table for composite tools", func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := mocks.NewMockBackendClient(ctrl) + backends := []vmcp.Backend{ + newTestBackend("slack", withBackendName("Slack")), + } + + // Backend has tools + caps := newTestCapabilityList( + withTools( + newTestTool("send_message", "slack"), + newTestTool("list_channels", "slack"), + ), + ) + + mockClient.EXPECT().ListCapabilities(gomock.Any(), gomock.Any()).Return(caps, nil) + + // Configure global ExcludeAllTools + aggregationConfig := &config.AggregationConfig{ + ExcludeAllTools: true, + } + + agg := NewDefaultAggregator(mockClient, nil, aggregationConfig, nil) + result, err := agg.AggregateCapabilities(context.Background(), backends) + + require.NoError(t, err) + assert.NotNil(t, result) + + // Advertised tools should be empty + assert.Empty(t, result.Tools, "Global ExcludeAllTools should hide all tools from LLM") + + // BUT routing table should still contain tools for composite tools + assert.NotNil(t, result.RoutingTable) + assert.Contains(t, result.RoutingTable.Tools, "send_message", + "Routing table should contain globally excluded tools for composite tool use") + assert.Contains(t, result.RoutingTable.Tools, "list_channels", + "Routing table should contain globally excluded tools for composite tool use") + }) +} diff --git a/pkg/vmcp/aggregator/tool_adapter.go b/pkg/vmcp/aggregator/tool_adapter.go index de82b89138..3b21aed074 100644 --- a/pkg/vmcp/aggregator/tool_adapter.go +++ b/pkg/vmcp/aggregator/tool_adapter.go @@ -29,11 +29,10 @@ func processBackendTools( return tools // No configuration for this backend } - // Check ExcludeAll first - takes precedence over Filter/Overrides - if workloadConfig.ExcludeAll { - logger.Debugf("ExcludeAll is true for backend %s, returning empty tools list", backendID) - return []vmcp.Tool{} - } + // NOTE: ExcludeAll is NOT applied here. ExcludeAll only affects which tools + // are advertised to the LLM, not which tools are available for routing. + // This allows composite tools to call backend tools that are excluded from + // direct LLM access. ExcludeAll is applied later in MergeCapabilities. // If no filter or overrides configured, return tools as-is if len(workloadConfig.Filter) == 0 && len(workloadConfig.Overrides) == 0 { diff --git a/pkg/vmcp/aggregator/tool_adapter_test.go b/pkg/vmcp/aggregator/tool_adapter_test.go index 6a6cdae008..7c5f696d53 100644 --- a/pkg/vmcp/aggregator/tool_adapter_test.go +++ b/pkg/vmcp/aggregator/tool_adapter_test.go @@ -119,7 +119,10 @@ func TestProcessBackendTools(t *testing.T) { wantNames: []string{"renamed_tool1"}, }, { - name: "excludeAll excludes all tools from workload", + // NOTE: processBackendTools does NOT apply ExcludeAll - it's applied + // later in MergeCapabilities. This allows the routing table to contain + // all tools (for composite tools) while only filtering the advertised tools. + name: "excludeAll is ignored by processBackendTools (applied in MergeCapabilities)", backendID: "github", tools: []vmcp.Tool{ {Name: "create_pr", Description: "Create PR", BackendID: "github"}, @@ -129,25 +132,28 @@ func TestProcessBackendTools(t *testing.T) { Workload: "github", ExcludeAll: true, }, - wantCount: 0, - wantNames: []string{}, + wantCount: 2, // All tools pass through - ExcludeAll is applied later + wantNames: []string{"create_pr", "merge_pr"}, }, { - name: "excludeAll takes precedence over filter", + // ExcludeAll is ignored here; filter is still applied + name: "excludeAll is ignored but filter still applies", backendID: "github", tools: []vmcp.Tool{ {Name: "create_pr", Description: "Create PR", BackendID: "github"}, + {Name: "merge_pr", Description: "Merge PR", BackendID: "github"}, }, workloadConfig: &config.WorkloadToolConfig{ Workload: "github", ExcludeAll: true, Filter: []string{"create_pr"}, }, - wantCount: 0, - wantNames: []string{}, + wantCount: 1, // Filter is applied, ExcludeAll is not + wantNames: []string{"create_pr"}, }, { - name: "excludeAll takes precedence over overrides", + // ExcludeAll is ignored here; overrides are still applied + name: "excludeAll is ignored but overrides still apply", backendID: "github", tools: []vmcp.Tool{ {Name: "create_pr", Description: "Create PR", BackendID: "github"}, @@ -159,8 +165,8 @@ func TestProcessBackendTools(t *testing.T) { "create_pr": {Name: "gh_create_pr"}, }, }, - wantCount: 0, - wantNames: []string{}, + wantCount: 1, // Override is applied, ExcludeAll is not + wantNames: []string{"gh_create_pr"}, }, }