Skip to content

Commit 39dcba6

Browse files
refactor: simplify NewServerTool naming
- Rename NewServerToolWithRawContextHandler -> NewServerTool. This is the preferred constructor for raw mcp.ToolHandler tools because it avoids creating closures at registration time, which matters for per-request servers that re-register all tools on every request. - Rename deprecated generic NewServerTool[In, Out] -> NewServerToolWithDeps to free up the simpler name and make its closure-based nature explicit. The dynamic tools package is the only legitimate user of this constructor because DynamicToolDependencies differs from the standard ToolDependencies. - Remove deprecated NewServerToolFromHandler. Its only callers can use the new NewServerTool directly via context-injected deps. - Update all call sites in dependencies.go, dynamic_tools.go, and registry_test.go. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent bafcaf5 commit 39dcba6

4 files changed

Lines changed: 18 additions & 32 deletions

File tree

pkg/github/dependencies.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ func NewToolFromHandler(
253253
requiredScopes []scopes.Scope,
254254
handler func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest) (*mcp.CallToolResult, error),
255255
) inventory.ServerTool {
256-
st := inventory.NewServerToolWithRawContextHandler(tool, toolset, func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
256+
st := inventory.NewServerTool(tool, toolset, func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
257257
deps := MustDepsFromContext(ctx)
258258
return handler(ctx, deps, req)
259259
})

pkg/github/dynamic_tools.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ type DynamicToolDependencies struct {
3131
// tools (ToolDependencies), so they intentionally use the closure pattern.
3232
func NewDynamicTool(toolset inventory.ToolsetMetadata, tool mcp.Tool, handler func(deps DynamicToolDependencies) mcp.ToolHandlerFor[map[string]any, any]) inventory.ServerTool {
3333
//nolint:staticcheck // SA1019: Dynamic tools use a different deps structure, closure pattern is intentional
34-
return inventory.NewServerTool(tool, toolset, func(d any) mcp.ToolHandlerFor[map[string]any, any] {
34+
return inventory.NewServerToolWithDeps(tool, toolset, func(d any) mcp.ToolHandlerFor[map[string]any, any] {
3535
return handler(d.(DynamicToolDependencies))
3636
})
3737
}

pkg/inventory/registry_test.go

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func testToolsetMetadataWithDefault(id string, isDefault bool) ToolsetMetadata {
3838

3939
// mockToolWithDefault creates a mock tool with a default toolset flag
4040
func mockToolWithDefault(name string, toolsetID string, readOnly bool, isDefault bool) ServerTool {
41-
return NewServerToolFromHandler(
41+
return NewServerTool(
4242
mcp.Tool{
4343
Name: name,
4444
Annotations: &mcp.ToolAnnotations{
@@ -47,17 +47,15 @@ func mockToolWithDefault(name string, toolsetID string, readOnly bool, isDefault
4747
InputSchema: json.RawMessage(`{"type":"object","properties":{}}`),
4848
},
4949
testToolsetMetadataWithDefault(toolsetID, isDefault),
50-
func(_ any) mcp.ToolHandler {
51-
return func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
52-
return nil, nil
53-
}
50+
func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
51+
return nil, nil
5452
},
5553
)
5654
}
5755

5856
// mockTool creates a minimal ServerTool for testing
5957
func mockTool(name string, toolsetID string, readOnly bool) ServerTool {
60-
return NewServerToolFromHandler(
58+
return NewServerTool(
6159
mcp.Tool{
6260
Name: name,
6361
Annotations: &mcp.ToolAnnotations{
@@ -66,10 +64,8 @@ func mockTool(name string, toolsetID string, readOnly bool) ServerTool {
6664
InputSchema: json.RawMessage(`{"type":"object","properties":{}}`),
6765
},
6866
testToolsetMetadata(toolsetID),
69-
func(_ any) mcp.ToolHandler {
70-
return func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
71-
return nil, nil
72-
}
67+
func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
68+
return nil, nil
7369
},
7470
)
7571
}
@@ -1839,7 +1835,7 @@ func TestWithTools_DeprecatedAliasAndFeatureFlag(t *testing.T) {
18391835

18401836
// mockToolWithMeta creates a ServerTool with Meta for testing insiders mode
18411837
func mockToolWithMeta(name string, toolsetID string, meta map[string]any) ServerTool {
1842-
return NewServerToolFromHandler(
1838+
return NewServerTool(
18431839
mcp.Tool{
18441840
Name: name,
18451841
Annotations: &mcp.ToolAnnotations{
@@ -1849,10 +1845,8 @@ func mockToolWithMeta(name string, toolsetID string, meta map[string]any) Server
18491845
Meta: meta,
18501846
},
18511847
testToolsetMetadata(toolsetID),
1852-
func(_ any) mcp.ToolHandler {
1853-
return func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
1854-
return nil, nil
1855-
}
1848+
func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
1849+
return nil, nil
18561850
},
18571851
)
18581852
}

pkg/inventory/server_tool.go

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,13 @@ func (st *ServerTool) RegisterFunc(s *mcp.Server, deps any) {
118118
s.AddTool(&toolCopy, handler)
119119
}
120120

121-
// NewServerTool creates a ServerTool from a tool definition, toolset metadata, and a typed handler function.
121+
// NewServerToolWithDeps creates a ServerTool from a tool definition, toolset metadata, and a typed handler function.
122122
// The handler function takes dependencies (as any) and returns a typed handler.
123123
// Callers should type-assert deps to their typed dependencies struct.
124124
//
125125
// Deprecated: This creates closures at registration time. For better performance in
126126
// per-request server scenarios, use NewServerToolWithContextHandler instead.
127-
func NewServerTool[In any, Out any](tool mcp.Tool, toolset ToolsetMetadata, handlerFn func(deps any) mcp.ToolHandlerFor[In, Out]) ServerTool {
127+
func NewServerToolWithDeps[In any, Out any](tool mcp.Tool, toolset ToolsetMetadata, handlerFn func(deps any) mcp.ToolHandlerFor[In, Out]) ServerTool {
128128
return ServerTool{
129129
Tool: tool,
130130
Toolset: toolset,
@@ -166,22 +166,14 @@ func NewServerToolWithContextHandler[In any, Out any](tool mcp.Tool, toolset Too
166166
}
167167
}
168168

169-
// NewServerToolFromHandler creates a ServerTool from a tool definition, toolset metadata, and a raw handler function.
170-
// Use this when you have a handler that already conforms to mcp.ToolHandler.
171-
//
172-
// Deprecated: This creates closures at registration time. For better performance in
173-
// per-request server scenarios, use NewServerToolWithRawContextHandler instead.
174-
func NewServerToolFromHandler(tool mcp.Tool, toolset ToolsetMetadata, handlerFn func(deps any) mcp.ToolHandler) ServerTool {
175-
return ServerTool{Tool: tool, Toolset: toolset, HandlerFunc: handlerFn}
176-
}
177-
178-
// NewServerToolWithRawContextHandler creates a ServerTool with a raw handler that receives deps via context.
179-
// This is the preferred approach for tools that use mcp.ToolHandler directly because it doesn't
180-
// create closures at registration time.
169+
// NewServerTool creates a ServerTool with a raw handler that receives deps via context.
170+
// This is the preferred constructor for tools that use mcp.ToolHandler directly because
171+
// it doesn't create closures at registration time, which is critical for performance in
172+
// servers that create a new instance per request.
181173
//
182174
// The handler function is stored directly without wrapping in a deps closure.
183175
// Dependencies should be injected into context before calling tool handlers.
184-
func NewServerToolWithRawContextHandler(tool mcp.Tool, toolset ToolsetMetadata, handler mcp.ToolHandler) ServerTool {
176+
func NewServerTool(tool mcp.Tool, toolset ToolsetMetadata, handler mcp.ToolHandler) ServerTool {
185177
return ServerTool{
186178
Tool: tool,
187179
Toolset: toolset,

0 commit comments

Comments
 (0)