Skip to content

Commit 8cc97a2

Browse files
committed
refactor(vmcp): unify composite tools and optimizer as session decorators
Both composite tools and the optimizer now implement the MultiSession decorator pattern (same as hijackPreventionDecorator) rather than having bespoke SDK wiring in handleSessionRegistrationImpl. New session decorators: - session/compositetools: appends composite tools to Tools(), routes their CallTool invocations to per-session workflow executors - session/optimizerdec: replaces Tools() with [find_tool, call_tool]; find_tool routes through the optimizer, call_tool delegates to the underlying session for normal backend routing sessionmanager.Manager gains DecorateSession() to swap in a wrapped session after creation. handleSessionRegistrationImpl becomes a flat decoration sequence (apply compositetools → apply optimizer → register tools) with no branching on optimizer vs non-optimizer paths. adapter.WorkflowExecutor/WorkflowResult become type aliases for the compositetools package types so the two layers share a single definition. adapter.CreateOptimizerTools is deleted; its logic lives in optimizerdec.
1 parent 1301dbc commit 8cc97a2

14 files changed

Lines changed: 839 additions & 306 deletions

File tree

pkg/vmcp/router/session_router.go

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package router
66
import (
77
"context"
88
"fmt"
9+
"strings"
910

1011
"github.com/stacklok/toolhive/pkg/vmcp"
1112
)
@@ -28,15 +29,46 @@ func NewSessionRouter(rt *vmcp.RoutingTable) Router {
2829

2930
// RouteTool resolves a tool name to its backend target using the session's
3031
// routing table directly.
32+
//
33+
// Two naming conventions are supported:
34+
//
35+
// 1. Exact key: the resolved/conflict-resolved name stored in the routing
36+
// table (e.g. "my-backend_echo" after prefix conflict resolution).
37+
//
38+
// 2. Dot convention "{workloadID}.{toolName}": the tool name is the original
39+
// backend capability name and the workload ID is the prefix. This mirrors
40+
// the isToolStepAccessible logic used when registering composite tools and
41+
// lets workflow step definitions remain stable regardless of the conflict
42+
// resolution strategy in use.
43+
//
44+
// The dot convention is necessary because composite workflow steps reference
45+
// tools by their pre-conflict-resolution name (e.g. "my-backend.echo"), while
46+
// the routing table may store them under a prefixed key ("my-backend_echo").
3147
func (r *sessionRouter) RouteTool(_ context.Context, toolName string) (*vmcp.BackendTarget, error) {
3248
if r.routingTable == nil || r.routingTable.Tools == nil {
3349
return nil, fmt.Errorf("%w: %s", ErrToolNotFound, toolName)
3450
}
35-
target, exists := r.routingTable.Tools[toolName]
36-
if !exists {
37-
return nil, fmt.Errorf("%w: %s", ErrToolNotFound, toolName)
51+
52+
// Fast path: exact key match.
53+
if target, exists := r.routingTable.Tools[toolName]; exists {
54+
return target, nil
3855
}
39-
return target, nil
56+
57+
// Fallback: dot convention "{workloadID}.{toolName}".
58+
// Workload IDs are Kubernetes resource names and cannot contain dots,
59+
// so the first dot unambiguously separates the workload ID from the
60+
// original backend capability name.
61+
if dotIdx := strings.Index(toolName, "."); dotIdx > 0 {
62+
workloadID := toolName[:dotIdx]
63+
capName := toolName[dotIdx+1:]
64+
for resolvedName, target := range r.routingTable.Tools {
65+
if target.WorkloadID == workloadID && target.GetBackendCapabilityName(resolvedName) == capName {
66+
return target, nil
67+
}
68+
}
69+
}
70+
71+
return nil, fmt.Errorf("%w: %s", ErrToolNotFound, toolName)
4072
}
4173

4274
// RouteResource resolves a resource URI to its backend target using the

pkg/vmcp/router/session_router_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,54 @@ func TestSessionRouter_RouteTool(t *testing.T) {
6565
expectError: true,
6666
errorContains: "tool not found",
6767
},
68+
{
69+
// Composite workflow steps use "{workloadID}.{toolName}" where toolName
70+
// is the original backend capability name. With prefix conflict resolution
71+
// the routing table key is "{workloadID}_toolName", so an exact match
72+
// fails. The dot-convention fallback must resolve it correctly.
73+
name: "dot convention resolved via workload ID and original capability name",
74+
routingTable: &vmcp.RoutingTable{
75+
Tools: map[string]*vmcp.BackendTarget{
76+
"my-backend_echo": {
77+
WorkloadID: "my-backend",
78+
WorkloadName: "My Backend",
79+
BaseURL: "http://my-backend:8080",
80+
OriginalCapabilityName: "echo",
81+
},
82+
},
83+
},
84+
toolName: "my-backend.echo",
85+
expectedID: "my-backend",
86+
expectError: false,
87+
},
88+
{
89+
name: "dot convention: workload not in session",
90+
routingTable: &vmcp.RoutingTable{
91+
Tools: map[string]*vmcp.BackendTarget{
92+
"other-backend_echo": {
93+
WorkloadID: "other-backend",
94+
OriginalCapabilityName: "echo",
95+
},
96+
},
97+
},
98+
toolName: "my-backend.echo",
99+
expectError: true,
100+
errorContains: "tool not found",
101+
},
102+
{
103+
name: "dot convention: capability name mismatch",
104+
routingTable: &vmcp.RoutingTable{
105+
Tools: map[string]*vmcp.BackendTarget{
106+
"my-backend_echo": {
107+
WorkloadID: "my-backend",
108+
OriginalCapabilityName: "echo",
109+
},
110+
},
111+
},
112+
toolName: "my-backend.fetch",
113+
expectError: true,
114+
errorContains: "tool not found",
115+
},
68116
}
69117

70118
for _, tt := range tests {

pkg/vmcp/server/adapter/handler_factory.go

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/stacklok/toolhive/pkg/vmcp/conversion"
2020
"github.com/stacklok/toolhive/pkg/vmcp/discovery"
2121
"github.com/stacklok/toolhive/pkg/vmcp/router"
22+
"github.com/stacklok/toolhive/pkg/vmcp/session/compositetools"
2223
)
2324

2425
//go:generate mockgen -destination=mocks/mock_handler_factory.go -package=mocks github.com/stacklok/toolhive/pkg/vmcp/server/adapter HandlerFactory
@@ -43,20 +44,13 @@ type HandlerFactory interface {
4344
}
4445

4546
// WorkflowExecutor executes composite tool workflows.
46-
// This interface abstracts the composer to enable testing without full composer setup.
47-
type WorkflowExecutor interface {
48-
// ExecuteWorkflow executes the workflow with the given parameters.
49-
ExecuteWorkflow(ctx context.Context, params map[string]any) (*WorkflowResult, error)
50-
}
47+
// Type alias for compositetools.WorkflowExecutor so that adapter consumers and
48+
// the session decorator share a single interface definition.
49+
type WorkflowExecutor = compositetools.WorkflowExecutor
5150

5251
// WorkflowResult represents the result of a workflow execution.
53-
type WorkflowResult struct {
54-
// Output contains the workflow output data (typically from the last step).
55-
Output map[string]any
56-
57-
// Error contains error information if the workflow failed.
58-
Error error
59-
}
52+
// Type alias for compositetools.WorkflowResult.
53+
type WorkflowResult = compositetools.WorkflowResult
6054

6155
// DefaultHandlerFactory creates MCP request handlers that route to backend workloads.
6256
type DefaultHandlerFactory struct {

pkg/vmcp/server/adapter/optimizer_adapter.go

Lines changed: 3 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -3,124 +3,10 @@
33

44
package adapter
55

6-
import (
7-
"context"
8-
"encoding/json"
9-
"fmt"
10-
11-
"github.com/mark3labs/mcp-go/mcp"
12-
"github.com/mark3labs/mcp-go/server"
13-
14-
"github.com/stacklok/toolhive/pkg/vmcp/optimizer"
15-
"github.com/stacklok/toolhive/pkg/vmcp/schema"
16-
)
17-
18-
// OptimizerToolNames defines the tool names exposed when optimizer is enabled.
6+
// OptimizerToolNames defines the tool names exposed when optimizer mode is enabled.
7+
// These constants are kept here for backwards compatibility with existing tests and
8+
// callers. The actual tool implementation lives in the optimizerdec session decorator.
199
const (
2010
FindToolName = "find_tool"
2111
CallToolName = "call_tool"
2212
)
23-
24-
// Pre-generated schemas for optimizer tools.
25-
// Generated at package init time so any schema errors panic at startup.
26-
var (
27-
findToolInputSchema = mustGenerateSchema[optimizer.FindToolInput]()
28-
callToolInputSchema = mustGenerateSchema[optimizer.CallToolInput]()
29-
)
30-
31-
// CreateOptimizerTools creates the SDK tools for optimizer mode.
32-
// When optimizer is enabled, only these two tools are exposed to clients
33-
// instead of all backend tools.
34-
func CreateOptimizerTools(opt optimizer.Optimizer) []server.ServerTool {
35-
return []server.ServerTool{
36-
{
37-
Tool: mcp.Tool{
38-
Name: FindToolName,
39-
Description: "Find and return tools that can help accomplish the user's request. " +
40-
"This searches available MCP server tools using semantic and keyword-based matching. " +
41-
"Use this function when you need to: " +
42-
"(1) discover what tools are available for a specific task, " +
43-
"(2) find the right tool(s) before attempting to solve a problem, " +
44-
"(3) check if required functionality exists in the current environment. " +
45-
"Returns matching tools ranked by relevance including their names, descriptions, " +
46-
"required parameters and schemas, plus token efficiency metrics showing " +
47-
"baseline_tokens, returned_tokens, and savings_percent. " +
48-
"Example: for 'Find good restaurants in San Jose', call with " +
49-
"tool_description='search the web' and tool_keywords='web search restaurants'. " +
50-
"Always call this before call_tool to discover the correct tool name and parameter schema.",
51-
RawInputSchema: findToolInputSchema,
52-
},
53-
Handler: createFindToolHandler(opt),
54-
},
55-
{
56-
Tool: mcp.Tool{
57-
Name: CallToolName,
58-
Description: "Execute a specific tool with the provided parameters. " +
59-
"Use this function to: " +
60-
"(1) run a tool after identifying it with find_tool, " +
61-
"(2) execute operations that require specific MCP server functionality, " +
62-
"(3) perform actions that go beyond your built-in capabilities. " +
63-
"Important: always use find_tool first to get the correct tool_name " +
64-
"and parameter schema before calling this function. " +
65-
"The parameters must match the tool's input schema as returned by find_tool. " +
66-
"Returns the tool's execution result which may include success/failure status, " +
67-
"result data or content, and error messages if execution failed.",
68-
RawInputSchema: callToolInputSchema,
69-
},
70-
Handler: createCallToolHandler(opt),
71-
},
72-
}
73-
}
74-
75-
// createFindToolHandler creates a handler for the find_tool optimizer operation.
76-
func createFindToolHandler(opt optimizer.Optimizer) func(context.Context, mcp.CallToolRequest) (*mcp.CallToolResult, error) {
77-
return func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
78-
input, err := schema.Translate[optimizer.FindToolInput](request.Params.Arguments)
79-
if err != nil {
80-
return mcp.NewToolResultError(fmt.Sprintf("invalid arguments: %v", err)), nil
81-
}
82-
83-
output, err := opt.FindTool(ctx, input)
84-
if err != nil {
85-
return mcp.NewToolResultError(fmt.Sprintf("find_tool failed: %v", err)), nil
86-
}
87-
88-
return mcp.NewToolResultStructuredOnly(output), nil
89-
}
90-
}
91-
92-
// createCallToolHandler creates a handler for the call_tool optimizer operation.
93-
func createCallToolHandler(opt optimizer.Optimizer) func(context.Context, mcp.CallToolRequest) (*mcp.CallToolResult, error) {
94-
return func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
95-
input, err := schema.Translate[optimizer.CallToolInput](request.Params.Arguments)
96-
if err != nil {
97-
return mcp.NewToolResultError(fmt.Sprintf("invalid arguments: %v", err)), nil
98-
}
99-
100-
result, err := opt.CallTool(ctx, input)
101-
if err != nil {
102-
// Exposing the error to the MCP client is important if you want it to correct its behavior.
103-
// Without information on the failure, the model is pretty much hopeless in figuring out the problem.
104-
return mcp.NewToolResultError(fmt.Sprintf("call_tool failed: %v", err)), nil
105-
}
106-
107-
return result, nil
108-
}
109-
}
110-
111-
// mustMarshalSchema marshals a schema to JSON, panicking on error.
112-
// This is safe because schemas are generated from known types at startup.
113-
// This should NOT be called by runtime code.
114-
func mustGenerateSchema[T any]() json.RawMessage {
115-
s, err := schema.GenerateSchema[T]()
116-
if err != nil {
117-
panic(fmt.Sprintf("failed to generate schema: %v", err))
118-
}
119-
120-
data, err := json.Marshal(s)
121-
if err != nil {
122-
panic(fmt.Sprintf("failed to marshal schema: %v", err))
123-
}
124-
125-
return data
126-
}

pkg/vmcp/server/adapter/optimizer_adapter_test.go

Lines changed: 4 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -4,103 +4,14 @@
44
package adapter
55

66
import (
7-
"context"
87
"testing"
98

10-
"github.com/mark3labs/mcp-go/mcp"
11-
"github.com/stretchr/testify/require"
12-
13-
"github.com/stacklok/toolhive/pkg/vmcp/optimizer"
9+
"github.com/stretchr/testify/assert"
1410
)
1511

16-
// mockOptimizer implements optimizer.Optimizer for testing.
17-
type mockOptimizer struct {
18-
findToolFunc func(ctx context.Context, input optimizer.FindToolInput) (*optimizer.FindToolOutput, error)
19-
callToolFunc func(ctx context.Context, input optimizer.CallToolInput) (*mcp.CallToolResult, error)
20-
}
21-
22-
func (m *mockOptimizer) FindTool(ctx context.Context, input optimizer.FindToolInput) (*optimizer.FindToolOutput, error) {
23-
if m.findToolFunc != nil {
24-
return m.findToolFunc(ctx, input)
25-
}
26-
return &optimizer.FindToolOutput{}, nil
27-
}
28-
29-
func (m *mockOptimizer) CallTool(ctx context.Context, input optimizer.CallToolInput) (*mcp.CallToolResult, error) {
30-
if m.callToolFunc != nil {
31-
return m.callToolFunc(ctx, input)
32-
}
33-
return mcp.NewToolResultText("ok"), nil
34-
}
35-
36-
func TestCreateOptimizerTools(t *testing.T) {
37-
t.Parallel()
38-
39-
opt := &mockOptimizer{}
40-
tools := CreateOptimizerTools(opt)
41-
42-
require.Len(t, tools, 2)
43-
require.Equal(t, FindToolName, tools[0].Tool.Name)
44-
require.Equal(t, CallToolName, tools[1].Tool.Name)
45-
}
46-
47-
func TestFindToolHandler(t *testing.T) {
12+
func TestOptimizerToolNameConstants(t *testing.T) {
4813
t.Parallel()
4914

50-
opt := &mockOptimizer{
51-
findToolFunc: func(_ context.Context, input optimizer.FindToolInput) (*optimizer.FindToolOutput, error) {
52-
require.Equal(t, "read files", input.ToolDescription)
53-
return &optimizer.FindToolOutput{
54-
Tools: []mcp.Tool{
55-
{
56-
Name: "read_file",
57-
Description: "Read a file",
58-
},
59-
},
60-
}, nil
61-
},
62-
}
63-
64-
tools := CreateOptimizerTools(opt)
65-
handler := tools[0].Handler
66-
67-
request := mcp.CallToolRequest{}
68-
request.Params.Arguments = map[string]any{
69-
"tool_description": "read files",
70-
}
71-
72-
result, err := handler(context.Background(), request)
73-
require.NoError(t, err)
74-
require.NotNil(t, result)
75-
require.False(t, result.IsError)
76-
require.Len(t, result.Content, 1)
77-
}
78-
79-
func TestCallToolHandler(t *testing.T) {
80-
t.Parallel()
81-
82-
opt := &mockOptimizer{
83-
callToolFunc: func(_ context.Context, input optimizer.CallToolInput) (*mcp.CallToolResult, error) {
84-
require.Equal(t, "read_file", input.ToolName)
85-
require.Equal(t, "/etc/hosts", input.Parameters["path"])
86-
return mcp.NewToolResultText("file contents here"), nil
87-
},
88-
}
89-
90-
tools := CreateOptimizerTools(opt)
91-
handler := tools[1].Handler
92-
93-
request := mcp.CallToolRequest{}
94-
request.Params.Arguments = map[string]any{
95-
"tool_name": "read_file",
96-
"parameters": map[string]any{
97-
"path": "/etc/hosts",
98-
},
99-
}
100-
101-
result, err := handler(context.Background(), request)
102-
require.NoError(t, err)
103-
require.NotNil(t, result)
104-
require.False(t, result.IsError)
105-
require.Len(t, result.Content, 1)
15+
assert.Equal(t, "find_tool", FindToolName)
16+
assert.Equal(t, "call_tool", CallToolName)
10617
}

0 commit comments

Comments
 (0)