Skip to content

Commit ccad93a

Browse files
yroblataskbot
andauthored
Coerce types for non-advertised backend tools in composite workflows (#4671)
When backend tools are excluded from advertising via `excludeAll` or `filter`, the workflow engine could not find their `InputSchema` for argument type coercion. This caused string arguments (e.g. "42") to be forwarded to the backend as-is instead of being coerced to the correct type (e.g. float64(42)), resulting in backend rejection. Root cause: `ProcessPreQueriedCapabilities` only returned the advertised tool list. Sessions stored only those tools, and the workflow engine used `sess.Tools()` for schema lookup — so hidden tools were invisible to it. Fix: - `ProcessPreQueriedCapabilities` now returns both `advertisedTools` (filtered, for MCP clients) and `allResolvedTools` (every resolved tool regardless of filter, for internal schema lookup). - `MultiSession` gains `AllTools() []vmcp.Tool`, backed by the new `allTools` field on `defaultMultiSession`. - The per-session workflow engine is built from `sess.AllTools()` so it can coerce arguments for any backend tool, whether advertised or not. Fixes #4287 Co-authored-by: taskbot <taskbot@users.noreply.github.com>
1 parent 033e051 commit ccad93a

11 files changed

Lines changed: 115 additions & 44 deletions

File tree

pkg/vmcp/aggregator/aggregator.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,16 @@ type Aggregator interface {
7070
// toolsByBackend maps backend WorkloadID → raw tools as returned by the backend.
7171
// targets maps backend WorkloadID → the pre-built BackendTarget for that backend.
7272
//
73-
// Returns the advertised tool list (resolved names, filtered) and a routing table
74-
// keyed by resolved name. Each routing table entry has OriginalCapabilityName set
75-
// so that GetBackendCapabilityName() translates back to the raw backend name.
73+
// Returns:
74+
// - advertisedTools: resolved tools that pass the advertising filter (for MCP clients)
75+
// - allResolvedTools: all resolved tools including non-advertised ones (for schema lookup)
76+
// - toolsRouting: routing table keyed by resolved name; each entry has OriginalCapabilityName
77+
// set so that GetBackendCapabilityName() translates back to the raw backend name.
7678
ProcessPreQueriedCapabilities(
7779
ctx context.Context,
7880
toolsByBackend map[string][]vmcp.Tool,
7981
targets map[string]*vmcp.BackendTarget,
80-
) (advertisedTools []vmcp.Tool, toolsRouting map[string]*vmcp.BackendTarget, err error)
82+
) (advertisedTools []vmcp.Tool, allResolvedTools []vmcp.Tool, toolsRouting map[string]*vmcp.BackendTarget, err error)
8183
}
8284

8385
// BackendCapabilities contains the raw capabilities from a single backend.

pkg/vmcp/aggregator/default_aggregator.go

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ func (a *defaultAggregator) ProcessPreQueriedCapabilities(
502502
ctx context.Context,
503503
toolsByBackend map[string][]vmcp.Tool,
504504
targets map[string]*vmcp.BackendTarget,
505-
) ([]vmcp.Tool, map[string]*vmcp.BackendTarget, error) {
505+
) ([]vmcp.Tool, []vmcp.Tool, map[string]*vmcp.BackendTarget, error) {
506506
// Step 1: Apply per-backend overrides (renames, description changes).
507507
processed := make(map[string]*BackendCapabilities, len(toolsByBackend))
508508
for backendID, rawTools := range toolsByBackend {
@@ -515,11 +515,16 @@ func (a *defaultAggregator) ProcessPreQueriedCapabilities(
515515
// Step 2: Resolve naming conflicts across backends.
516516
resolved, err := a.ResolveConflicts(ctx, processed)
517517
if err != nil {
518-
return nil, nil, err
518+
return nil, nil, nil, err
519519
}
520520

521-
// Step 3: Build advertised list and routing table, applying advertising filter.
521+
// Step 3: Build advertised list, all-resolved list, and routing table.
522+
// advertisedTools is the subset shown to MCP clients (post-filter).
523+
// allResolvedTools includes every resolved tool regardless of advertising filter,
524+
// so that workflow engines can look up InputSchema for type coercion even when
525+
// a backend tool is hidden from clients via excludeAll or filter configuration.
522526
var advertisedTools []vmcp.Tool
527+
var allResolvedTools []vmcp.Tool
523528
routingTable := make(map[string]*vmcp.BackendTarget, len(resolved.Tools))
524529

525530
for _, rt := range resolved.Tools {
@@ -536,19 +541,21 @@ func (a *defaultAggregator) ProcessPreQueriedCapabilities(
536541
t.OriginalCapabilityName = actualBackendCapabilityName(a.toolConfigMap, rt.BackendID, rt.OriginalName)
537542
routingTable[rt.ResolvedName] = &t
538543

544+
resolved := vmcp.Tool{
545+
Name: rt.ResolvedName,
546+
Description: rt.Description,
547+
InputSchema: rt.InputSchema,
548+
OutputSchema: rt.OutputSchema,
549+
Annotations: rt.Annotations,
550+
BackendID: rt.BackendID,
551+
}
552+
allResolvedTools = append(allResolvedTools, resolved)
539553
if a.shouldAdvertiseTool(rt.BackendID, rt.OriginalName) {
540-
advertisedTools = append(advertisedTools, vmcp.Tool{
541-
Name: rt.ResolvedName,
542-
Description: rt.Description,
543-
InputSchema: rt.InputSchema,
544-
OutputSchema: rt.OutputSchema,
545-
Annotations: rt.Annotations,
546-
BackendID: rt.BackendID,
547-
})
554+
advertisedTools = append(advertisedTools, resolved)
548555
}
549556
}
550557

551-
return advertisedTools, routingTable, nil
558+
return advertisedTools, allResolvedTools, routingTable, nil
552559
}
553560

554561
// actualBackendCapabilityName returns the real capability name the backend uses,

pkg/vmcp/aggregator/default_aggregator_test.go

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,7 @@ func TestDefaultAggregator_ProcessPreQueriedCapabilities(t *testing.T) {
795795
}
796796

797797
agg := NewDefaultAggregator(nil, nil, nil, nil)
798-
advertised, routingTable, err := agg.ProcessPreQueriedCapabilities(
798+
advertised, allResolved, routingTable, err := agg.ProcessPreQueriedCapabilities(
799799
context.Background(), toolsByBackend, targets,
800800
)
801801

@@ -807,6 +807,9 @@ func TestDefaultAggregator_ProcessPreQueriedCapabilities(t *testing.T) {
807807
}
808808
assert.Contains(t, advertisedNames, "tool1")
809809
assert.Contains(t, advertisedNames, "tool2")
810+
// With no filter, allResolved must equal the advertised list.
811+
assert.ElementsMatch(t, advertised, allResolved,
812+
"without a filter, allResolvedTools must equal the advertised list")
810813
// Both tools must be in the routing table.
811814
assert.Contains(t, routingTable, "tool1")
812815
assert.Contains(t, routingTable, "tool2")
@@ -825,7 +828,7 @@ func TestDefaultAggregator_ProcessPreQueriedCapabilities(t *testing.T) {
825828
}
826829

827830
agg := NewDefaultAggregator(nil, nil, nil, nil)
828-
_, routingTable, err := agg.ProcessPreQueriedCapabilities(
831+
_, _, routingTable, err := agg.ProcessPreQueriedCapabilities(
829832
context.Background(), toolsByBackend, targets,
830833
)
831834

@@ -858,7 +861,7 @@ func TestDefaultAggregator_ProcessPreQueriedCapabilities(t *testing.T) {
858861
}
859862

860863
agg := NewDefaultAggregator(nil, nil, aggCfg, nil)
861-
advertised, routingTable, err := agg.ProcessPreQueriedCapabilities(
864+
advertised, _, routingTable, err := agg.ProcessPreQueriedCapabilities(
862865
context.Background(), toolsByBackend, targets,
863866
)
864867

@@ -892,7 +895,7 @@ func TestDefaultAggregator_ProcessPreQueriedCapabilities(t *testing.T) {
892895
}
893896

894897
agg := NewDefaultAggregator(nil, nil, nil, nil)
895-
advertised, routingTable, err := agg.ProcessPreQueriedCapabilities(
898+
advertised, _, routingTable, err := agg.ProcessPreQueriedCapabilities(
896899
context.Background(), toolsByBackend, targets,
897900
)
898901

@@ -926,13 +929,23 @@ func TestDefaultAggregator_ProcessPreQueriedCapabilities(t *testing.T) {
926929
}
927930

928931
agg := NewDefaultAggregator(nil, nil, aggCfg, nil)
929-
advertised, routingTable, err := agg.ProcessPreQueriedCapabilities(
932+
advertised, allResolved, routingTable, err := agg.ProcessPreQueriedCapabilities(
930933
context.Background(), toolsByBackend, targets,
931934
)
932935

933936
require.NoError(t, err)
934937
assert.Empty(t, advertised,
935938
"ExcludeAllTools must produce an empty advertised list")
939+
// allResolvedTools must contain all tools regardless of the advertising filter,
940+
// so the workflow engine can look up InputSchema for type coercion.
941+
allResolvedNames := make([]string, 0, len(allResolved))
942+
for _, tool := range allResolved {
943+
allResolvedNames = append(allResolvedNames, tool.Name)
944+
}
945+
assert.Contains(t, allResolvedNames, "tool1",
946+
"excluded tools must appear in allResolvedTools for composite tool schema lookup")
947+
assert.Contains(t, allResolvedNames, "tool2",
948+
"excluded tools must appear in allResolvedTools for composite tool schema lookup")
936949
// Tools must still be routable (composite tools need them).
937950
assert.Contains(t, routingTable, "tool1",
938951
"excluded tools must remain in the routing table for composite tool use")
@@ -963,7 +976,7 @@ func TestDefaultAggregator_ProcessPreQueriedCapabilities(t *testing.T) {
963976
}
964977

965978
agg := NewDefaultAggregator(nil, nil, aggCfg, nil)
966-
advertised, routingTable, err := agg.ProcessPreQueriedCapabilities(
979+
advertised, allResolved, routingTable, err := agg.ProcessPreQueriedCapabilities(
967980
context.Background(), toolsByBackend, targets,
968981
)
969982

@@ -975,6 +988,16 @@ func TestDefaultAggregator_ProcessPreQueriedCapabilities(t *testing.T) {
975988
}
976989
assert.Equal(t, []string{"allowed_tool"}, advertisedNames,
977990
"only tools matching the filter should be advertised")
991+
// allResolvedTools must include both tools so the workflow engine can
992+
// look up InputSchema for type coercion on hidden_tool.
993+
allResolvedNames := make([]string, 0, len(allResolved))
994+
for _, tool := range allResolved {
995+
allResolvedNames = append(allResolvedNames, tool.Name)
996+
}
997+
assert.Contains(t, allResolvedNames, "allowed_tool",
998+
"filtered-in tool must appear in allResolvedTools")
999+
assert.Contains(t, allResolvedNames, "hidden_tool",
1000+
"filtered-out tool must appear in allResolvedTools for composite tool schema lookup")
9781001
// Both tools remain routable (composite tools can call hidden_tool).
9791002
assert.Contains(t, routingTable, "allowed_tool",
9801003
"filtered-in tool should be in routing table")
@@ -995,7 +1018,7 @@ func TestDefaultAggregator_ProcessPreQueriedCapabilities(t *testing.T) {
9951018
}
9961019

9971020
agg := NewDefaultAggregator(nil, nil, nil, nil)
998-
advertised, routingTable, err := agg.ProcessPreQueriedCapabilities(
1021+
advertised, _, routingTable, err := agg.ProcessPreQueriedCapabilities(
9991022
context.Background(), toolsByBackend, targets,
10001023
)
10011024

@@ -1018,7 +1041,7 @@ func TestDefaultAggregator_ProcessPreQueriedCapabilities(t *testing.T) {
10181041
t.Parallel()
10191042

10201043
agg := NewDefaultAggregator(nil, nil, nil, nil)
1021-
advertised, routingTable, err := agg.ProcessPreQueriedCapabilities(
1044+
advertised, _, routingTable, err := agg.ProcessPreQueriedCapabilities(
10221045
context.Background(),
10231046
map[string][]vmcp.Tool{},
10241047
map[string]*vmcp.BackendTarget{},

pkg/vmcp/aggregator/mocks/mock_interfaces.go

Lines changed: 5 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/vmcp/server/session_management_integration_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ func newNoopMockFactory(t *testing.T) *sessionfactorymocks.MockMultiSessionFacto
6060
mock.EXPECT().GetMetadata().Return(map[string]string{}).AnyTimes()
6161
mock.EXPECT().SetMetadata(gomock.Any(), gomock.Any()).AnyTimes()
6262
mock.EXPECT().Tools().Return(nil).AnyTimes()
63+
mock.EXPECT().AllTools().Return(nil).AnyTimes()
6364
mock.EXPECT().Resources().Return(nil).AnyTimes()
6465
mock.EXPECT().Prompts().Return(nil).AnyTimes()
6566
mock.EXPECT().BackendSessions().Return(nil).AnyTimes()
@@ -110,6 +111,7 @@ func newMockFactory(t *testing.T, ctrl *gomock.Controller, tools []vmcp.Tool) (*
110111
toolsCopy := make([]vmcp.Tool, len(tools))
111112
copy(toolsCopy, tools)
112113
mock.EXPECT().Tools().Return(toolsCopy).AnyTimes()
114+
mock.EXPECT().AllTools().Return(toolsCopy).AnyTimes()
113115
mock.EXPECT().Resources().Return(nil).AnyTimes()
114116
mock.EXPECT().Prompts().Return(nil).AnyTimes()
115117
mock.EXPECT().BackendSessions().Return(nil).AnyTimes()

pkg/vmcp/server/sessionmanager/factory.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,12 @@ func compositeToolsDecorator(
150150
}
151151

152152
compositeToolsMeta := compositetools.ConvertWorkflowDefsToTools(sessionDefs)
153-
if err := compositetools.ValidateNoToolConflicts(sess.Tools(), compositeToolsMeta); err != nil {
153+
if err := compositetools.ValidateNoToolConflicts(sess.AllTools(), compositeToolsMeta); err != nil {
154154
slog.Warn("composite tool name conflict detected; skipping composite tools", "session_id", sess.ID(), "error", err)
155155
return sess, nil
156156
}
157157

158-
sessionComposer := composerFactory(sess.GetRoutingTable(), sess.Tools())
158+
sessionComposer := composerFactory(sess.GetRoutingTable(), sess.AllTools())
159159
sessionExecutors := make(map[string]compositetools.WorkflowExecutor, len(sessionDefs))
160160
for _, def := range sessionDefs {
161161
ex := newComposerWorkflowExecutor(sessionComposer, def)

pkg/vmcp/server/telemetry_integration_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ type backendAwareTestSession struct {
6464
}
6565

6666
func (s *backendAwareTestSession) Tools() []vmcp.Tool { return s.tools }
67+
func (s *backendAwareTestSession) AllTools() []vmcp.Tool { return s.tools }
6768
func (*backendAwareTestSession) Resources() []vmcp.Resource { return nil }
6869
func (*backendAwareTestSession) Prompts() []vmcp.Prompt { return nil }
6970
func (*backendAwareTestSession) BackendSessions() map[string]string { return nil }

pkg/vmcp/session/default_session.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,21 +61,30 @@ type defaultMultiSession struct {
6161
// All fields below are written once by MakeSession and are read-only thereafter.
6262
connections map[string]backend.Session
6363
routingTable *vmcp.RoutingTable
64-
tools []vmcp.Tool
64+
tools []vmcp.Tool // advertised tools (shown to MCP clients)
65+
allTools []vmcp.Tool // all resolved tools, including non-advertised ones
6566
resources []vmcp.Resource
6667
prompts []vmcp.Prompt
6768
backendSessions map[string]string
6869

6970
queue AdmissionQueue
7071
}
7172

72-
// Tools returns a snapshot copy of the tools available in this session.
73+
// Tools returns a snapshot copy of the advertised tools available in this session.
7374
func (s *defaultMultiSession) Tools() []vmcp.Tool {
7475
result := make([]vmcp.Tool, len(s.tools))
7576
copy(result, s.tools)
7677
return result
7778
}
7879

80+
// AllTools returns a snapshot copy of all resolved tools in this session,
81+
// including tools excluded from advertising to MCP clients.
82+
func (s *defaultMultiSession) AllTools() []vmcp.Tool {
83+
result := make([]vmcp.Tool, len(s.allTools))
84+
copy(result, s.allTools)
85+
return result
86+
}
87+
7988
// Resources returns a snapshot copy of the resources available in this session.
8089
func (s *defaultMultiSession) Resources() []vmcp.Resource {
8190
result := make([]vmcp.Resource, len(s.resources))

pkg/vmcp/session/factory.go

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -290,11 +290,14 @@ func buildRoutingTable(results []initResult) (*vmcp.RoutingTable, []vmcp.Tool, [
290290
// pipeline (overrides, conflict resolution, advertising filter) to the raw
291291
// backend capabilities in results, producing resolved tool names identical to
292292
// the standard aggregation path. Resources and prompts pass through unchanged.
293+
//
294+
// Returns the routing table, advertised tools (for MCP clients), all resolved
295+
// tools (for schema lookup), resources, prompts, and any error.
293296
func buildRoutingTableWithAggregator(
294297
ctx context.Context,
295298
agg aggregator.Aggregator,
296299
results []initResult,
297-
) (*vmcp.RoutingTable, []vmcp.Tool, []vmcp.Resource, []vmcp.Prompt, error) {
300+
) (*vmcp.RoutingTable, []vmcp.Tool, []vmcp.Tool, []vmcp.Resource, []vmcp.Prompt, error) {
298301
toolsByBackend := make(map[string][]vmcp.Tool, len(results))
299302
targets := make(map[string]*vmcp.BackendTarget, len(results))
300303
for i := range results {
@@ -303,9 +306,9 @@ func buildRoutingTableWithAggregator(
303306
targets[r.target.WorkloadID] = r.target
304307
}
305308

306-
allTools, toolsRouting, err := agg.ProcessPreQueriedCapabilities(ctx, toolsByBackend, targets)
309+
advertisedTools, allResolvedTools, toolsRouting, err := agg.ProcessPreQueriedCapabilities(ctx, toolsByBackend, targets)
307310
if err != nil {
308-
return nil, nil, nil, nil, err
311+
return nil, nil, nil, nil, nil, err
309312
}
310313

311314
rt := &vmcp.RoutingTable{
@@ -331,7 +334,7 @@ func buildRoutingTableWithAggregator(
331334
}
332335
}
333336

334-
return rt, allTools, allResources, allPrompts, nil
337+
return rt, advertisedTools, allResolvedTools, allResources, allPrompts, nil
335338
}
336339

337340
// MakeSessionWithID implements MultiSessionFactory.
@@ -458,19 +461,22 @@ func (f *defaultMultiSessionFactory) makeBaseSession(
458461
}
459462

460463
var (
461-
routingTable *vmcp.RoutingTable
462-
allTools []vmcp.Tool
463-
allResources []vmcp.Resource
464-
allPrompts []vmcp.Prompt
464+
routingTable *vmcp.RoutingTable
465+
advertisedTools []vmcp.Tool
466+
allResolvedTools []vmcp.Tool
467+
allResources []vmcp.Resource
468+
allPrompts []vmcp.Prompt
465469
)
466470
if f.aggregator != nil {
467471
var aggErr error
468-
routingTable, allTools, allResources, allPrompts, aggErr = buildRoutingTableWithAggregator(ctx, f.aggregator, results)
472+
routingTable, advertisedTools, allResolvedTools, allResources, allPrompts, aggErr =
473+
buildRoutingTableWithAggregator(ctx, f.aggregator, results)
469474
if aggErr != nil {
470475
return nil, fmt.Errorf("failed to process backend capabilities: %w", aggErr)
471476
}
472477
} else {
473-
routingTable, allTools, allResources, allPrompts = buildRoutingTable(results)
478+
routingTable, advertisedTools, allResources, allPrompts = buildRoutingTable(results)
479+
allResolvedTools = advertisedTools // no filter when no aggregator
474480
}
475481

476482
transportSess := transportsession.NewStreamableSession(sessID)
@@ -483,7 +489,8 @@ func (f *defaultMultiSessionFactory) makeBaseSession(
483489
Session: transportSess,
484490
connections: connections,
485491
routingTable: routingTable,
486-
tools: allTools,
492+
tools: advertisedTools,
493+
allTools: allResolvedTools,
487494
resources: allResources,
488495
prompts: allPrompts,
489496
backendSessions: backendSessions,

pkg/vmcp/session/types/mocks/mock_session.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)