Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 internal/ghmcp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {

// Register specific tools if configured
if len(cfg.EnabledTools) > 0 {
// Clean and validate tool names
enabledTools := github.CleanTools(cfg.EnabledTools)
enabledTools, _ = tsg.ResolveToolAliases(enabledTools)

// Register the specified tools (additive to any toolsets already enabled)
err = tsg.RegisterSpecificTools(ghServer, enabledTools, cfg.ReadOnly)
Expand Down
14 changes: 14 additions & 0 deletions pkg/github/deprecated_tool_aliases.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// deprecated_tool_aliases.go
package github

// DeprecatedToolAliases maps old tool names to their new canonical names.
// When tools are renamed, add an entry here to maintain backward compatibility.
// Users referencing the old name will receive the new tool with a deprecation warning.
//
// Example:
//
// "get_issue": "issue_read",
// "create_pr": "pull_request_create",
var DeprecatedToolAliases = map[string]string{
// Add entries as tools are renamed
}
2 changes: 2 additions & 0 deletions pkg/github/tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,8 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG
tsg.AddToolset(stargazers)
tsg.AddToolset(labels)

tsg.AddDeprecatedToolAliases(DeprecatedToolAliases)

return tsg
}

Expand Down
40 changes: 34 additions & 6 deletions pkg/toolsets/toolsets.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,16 +192,24 @@ func (t *Toolset) AddReadTools(tools ...ServerTool) *Toolset {
}

type ToolsetGroup struct {
Toolsets map[string]*Toolset
everythingOn bool
readOnly bool
Toolsets map[string]*Toolset
deprecatedAliases map[string]string
everythingOn bool
readOnly bool
}

func NewToolsetGroup(readOnly bool) *ToolsetGroup {
return &ToolsetGroup{
Toolsets: make(map[string]*Toolset),
everythingOn: false,
readOnly: readOnly,
Toolsets: make(map[string]*Toolset),
deprecatedAliases: make(map[string]string),
everythingOn: false,
readOnly: readOnly,
}
}

func (tg *ToolsetGroup) AddDeprecatedToolAliases(aliases map[string]string) {
for oldName, newName := range aliases {
tg.deprecatedAliases[oldName] = newName
}
}

Expand Down Expand Up @@ -307,6 +315,26 @@ func NewToolDoesNotExistError(name string) *ToolDoesNotExistError {
return &ToolDoesNotExistError{Name: name}
}

// ResolveToolAliases resolves deprecated tool aliases to their canonical names.
// It logs a warning to stderr for each deprecated alias that is resolved.
// Returns:
// - resolved: tool names with aliases replaced by canonical names
// - aliasesUsed: map of oldName → newName for each alias that was resolved
func (tg *ToolsetGroup) ResolveToolAliases(toolNames []string) (resolved []string, aliasesUsed map[string]string) {
resolved = make([]string, 0, len(toolNames))
aliasesUsed = make(map[string]string)
for _, toolName := range toolNames {
if canonicalName, isAlias := tg.deprecatedAliases[toolName]; isAlias {
fmt.Fprintf(os.Stderr, "Warning: tool %q is deprecated, use %q instead\n", toolName, canonicalName)
aliasesUsed[toolName] = canonicalName
resolved = append(resolved, canonicalName)
} else {
resolved = append(resolved, toolName)
}
}
return resolved, aliasesUsed
}

// FindToolByName searches all toolsets (enabled or disabled) for a tool by name.
// Returns the tool, its parent toolset name, and an error if not found.
func (tg *ToolsetGroup) FindToolByName(toolName string) (*ServerTool, string, error) {
Expand Down
131 changes: 131 additions & 0 deletions pkg/toolsets/toolsets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,23 @@ package toolsets
import (
"errors"
"testing"

"github.com/modelcontextprotocol/go-sdk/mcp"
)

// mockTool creates a minimal ServerTool for testing
func mockTool(name string, readOnly bool) ServerTool {
return ServerTool{
Tool: mcp.Tool{
Name: name,
Annotations: &mcp.ToolAnnotations{
ReadOnlyHint: readOnly,
},
},
RegisterFunc: func(_ *mcp.Server) {},
}
}

func TestNewToolsetGroupIsEmptyWithoutEverythingOn(t *testing.T) {
tsg := NewToolsetGroup(false)
if len(tsg.Toolsets) != 0 {
Expand Down Expand Up @@ -262,3 +277,119 @@ func TestToolsetGroup_GetToolset(t *testing.T) {
t.Errorf("expected error to be ToolsetDoesNotExistError, got %v", err)
}
}

func TestAddDeprecatedToolAliases(t *testing.T) {
tsg := NewToolsetGroup(false)

// Test adding aliases
tsg.AddDeprecatedToolAliases(map[string]string{
"old_name": "new_name",
"get_issue": "issue_read",
"create_pr": "pull_request_create",
})

if len(tsg.deprecatedAliases) != 3 {
t.Errorf("expected 3 aliases, got %d", len(tsg.deprecatedAliases))
}
if tsg.deprecatedAliases["old_name"] != "new_name" {
t.Errorf("expected alias 'old_name' -> 'new_name', got '%s'", tsg.deprecatedAliases["old_name"])
}
if tsg.deprecatedAliases["get_issue"] != "issue_read" {
t.Errorf("expected alias 'get_issue' -> 'issue_read'")
}
if tsg.deprecatedAliases["create_pr"] != "pull_request_create" {
t.Errorf("expected alias 'create_pr' -> 'pull_request_create'")
}
}

func TestResolveToolAliases(t *testing.T) {
tsg := NewToolsetGroup(false)
tsg.AddDeprecatedToolAliases(map[string]string{
"get_issue": "issue_read",
"create_pr": "pull_request_create",
})

// Test resolving a mix of aliases and canonical names
input := []string{"get_issue", "some_tool", "create_pr"}
resolved, aliasesUsed := tsg.ResolveToolAliases(input)

// Verify resolved names
if len(resolved) != 3 {
t.Fatalf("expected 3 resolved names, got %d", len(resolved))
}
if resolved[0] != "issue_read" {
t.Errorf("expected 'issue_read', got '%s'", resolved[0])
}
if resolved[1] != "some_tool" {
t.Errorf("expected 'some_tool' (unchanged), got '%s'", resolved[1])
}
if resolved[2] != "pull_request_create" {
t.Errorf("expected 'pull_request_create', got '%s'", resolved[2])
}

// Verify aliasesUsed map
if len(aliasesUsed) != 2 {
t.Fatalf("expected 2 aliases used, got %d", len(aliasesUsed))
}
if aliasesUsed["get_issue"] != "issue_read" {
t.Errorf("expected aliasesUsed['get_issue'] = 'issue_read', got '%s'", aliasesUsed["get_issue"])
}
if aliasesUsed["create_pr"] != "pull_request_create" {
t.Errorf("expected aliasesUsed['create_pr'] = 'pull_request_create', got '%s'", aliasesUsed["create_pr"])
}
}

func TestFindToolByName(t *testing.T) {
tsg := NewToolsetGroup(false)

// Create a toolset with a tool
toolset := NewToolset("test-toolset", "Test toolset")
toolset.readTools = append(toolset.readTools, mockTool("issue_read", true))
tsg.AddToolset(toolset)

// Find by canonical name
tool, toolsetName, err := tsg.FindToolByName("issue_read")
if err != nil {
t.Fatalf("expected no error, got %v", err)
}
if tool.Tool.Name != "issue_read" {
t.Errorf("expected tool name 'issue_read', got '%s'", tool.Tool.Name)
}
if toolsetName != "test-toolset" {
t.Errorf("expected toolset name 'test-toolset', got '%s'", toolsetName)
}

// FindToolByName does NOT resolve aliases - it expects canonical names
_, _, err = tsg.FindToolByName("get_issue")
if err == nil {
t.Error("expected error when using alias directly with FindToolByName")
}
}

func TestRegisterSpecificTools(t *testing.T) {
tsg := NewToolsetGroup(false)

// Create a toolset with both read and write tools
toolset := NewToolset("test-toolset", "Test toolset")
toolset.readTools = append(toolset.readTools, mockTool("issue_read", true))
toolset.writeTools = append(toolset.writeTools, mockTool("issue_write", false))
tsg.AddToolset(toolset)

// Test registering with canonical names
err := tsg.RegisterSpecificTools(nil, []string{"issue_read"}, false)
if err != nil {
t.Errorf("expected no error registering tool, got %v", err)
}

// Test registering write tool in read-only mode (should skip but not error)
err = tsg.RegisterSpecificTools(nil, []string{"issue_write"}, true)
if err != nil {
t.Errorf("expected no error when skipping write tool in read-only mode, got %v", err)
}

// Test registering non-existent tool (should error)
err = tsg.RegisterSpecificTools(nil, []string{"nonexistent"}, false)
if err == nil {
t.Error("expected error for non-existent tool")
}
}