Skip to content

refactor: simplify NewServerTool naming #2510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SamMorrowDrums merged 1 commit into from
May 20, 2026
+18 −32
Merged

refactor: simplify NewServerTool naming #2510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/github/dependencies.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func NewToolFromHandler(
requiredScopes []scopes.Scope,
handler func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest) (*mcp.CallToolResult, error),
) inventory.ServerTool {
st := inventory.NewServerToolWithRawContextHandler(tool, toolset, func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
st := inventory.NewServerTool(tool, toolset, func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
deps := MustDepsFromContext(ctx)
return handler(ctx, deps, req)
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/github/dynamic_tools.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type DynamicToolDependencies struct {
// tools (ToolDependencies), so they intentionally use the closure pattern.
func NewDynamicTool(toolset inventory.ToolsetMetadata, tool mcp.Tool, handler func(deps DynamicToolDependencies) mcp.ToolHandlerFor[map[string]any, any]) inventory.ServerTool {
//nolint:staticcheck // SA1019: Dynamic tools use a different deps structure, closure pattern is intentional
return inventory.NewServerTool(tool, toolset, func(d any) mcp.ToolHandlerFor[map[string]any, any] {
return inventory.NewServerToolWithDeps(tool, toolset, func(d any) mcp.ToolHandlerFor[map[string]any, any] {
return handler(d.(DynamicToolDependencies))
})
}
Expand Down
24 changes: 9 additions & 15 deletions pkg/inventory/registry_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func testToolsetMetadataWithDefault(id string, isDefault bool) ToolsetMetadata {

// mockToolWithDefault creates a mock tool with a default toolset flag
func mockToolWithDefault(name string, toolsetID string, readOnly bool, isDefault bool) ServerTool {
return NewServerToolFromHandler(
return NewServerTool(
mcp.Tool{
Name: name,
Annotations: &mcp.ToolAnnotations{
Expand All @@ -47,17 +47,15 @@ func mockToolWithDefault(name string, toolsetID string, readOnly bool, isDefault
InputSchema: json.RawMessage(`{"type":"object","properties":{}}`),
},
testToolsetMetadataWithDefault(toolsetID, isDefault),
func(_ any) mcp.ToolHandler {
return func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
return nil, nil
}
func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
return nil, nil
},
)
}

// mockTool creates a minimal ServerTool for testing
func mockTool(name string, toolsetID string, readOnly bool) ServerTool {
return NewServerToolFromHandler(
return NewServerTool(
mcp.Tool{
Name: name,
Annotations: &mcp.ToolAnnotations{
Expand All @@ -66,10 +64,8 @@ func mockTool(name string, toolsetID string, readOnly bool) ServerTool {
InputSchema: json.RawMessage(`{"type":"object","properties":{}}`),
},
testToolsetMetadata(toolsetID),
func(_ any) mcp.ToolHandler {
return func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
return nil, nil
}
func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
return nil, nil
},
)
}
Expand Down Expand Up @@ -1839,7 +1835,7 @@ func TestWithTools_DeprecatedAliasAndFeatureFlag(t *testing.T) {

// mockToolWithMeta creates a ServerTool with Meta for testing insiders mode
func mockToolWithMeta(name string, toolsetID string, meta map[string]any) ServerTool {
return NewServerToolFromHandler(
return NewServerTool(
mcp.Tool{
Name: name,
Annotations: &mcp.ToolAnnotations{
Expand All @@ -1849,10 +1845,8 @@ func mockToolWithMeta(name string, toolsetID string, meta map[string]any) Server
Meta: meta,
},
testToolsetMetadata(toolsetID),
func(_ any) mcp.ToolHandler {
return func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
return nil, nil
}
func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
return nil, nil
},
)
}
Expand Down
22 changes: 7 additions & 15 deletions pkg/inventory/server_tool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,13 @@ func (st *ServerTool) RegisterFunc(s *mcp.Server, deps any) {
s.AddTool(&toolCopy, handler)
}

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

// NewServerToolFromHandler creates a ServerTool from a tool definition, toolset metadata, and a raw handler function.
// Use this when you have a handler that already conforms to mcp.ToolHandler.
//
// Deprecated: This creates closures at registration time. For better performance in
// per-request server scenarios, use NewServerToolWithRawContextHandler instead.
func NewServerToolFromHandler(tool mcp.Tool, toolset ToolsetMetadata, handlerFn func(deps any) mcp.ToolHandler) ServerTool {
return ServerTool{Tool: tool, Toolset: toolset, HandlerFunc: handlerFn}
}

// NewServerToolWithRawContextHandler creates a ServerTool with a raw handler that receives deps via context.
// This is the preferred approach for tools that use mcp.ToolHandler directly because it doesn't
// create closures at registration time.
// NewServerTool creates a ServerTool with a raw handler that receives deps via context.
// This is the preferred constructor for tools that use mcp.ToolHandler directly because
// it doesn't create closures at registration time, which is critical for performance in
// servers that create a new instance per request.
//
// The handler function is stored directly without wrapping in a deps closure.
// Dependencies should be injected into context before calling tool handlers.
func NewServerToolWithRawContextHandler(tool mcp.Tool, toolset ToolsetMetadata, handler mcp.ToolHandler) ServerTool {
func NewServerTool(tool mcp.Tool, toolset ToolsetMetadata, handler mcp.ToolHandler) ServerTool {
return ServerTool{
Tool: tool,
Toolset: toolset,
Expand Down
Loading