From 5719a887c59473b57c1076a0d7648e4caee1b160 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Tue, 9 Dec 2025 13:00:01 +0000 Subject: [PATCH 01/14] add suppport for tool aliases for backwards compatibility when tool names change --- pkg/github/tools.go | 11 ++++++ pkg/toolsets/toolsets.go | 76 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index d37af98b8..133e2203f 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -160,6 +160,14 @@ func GetDefaultToolsetIDs() []string { } } +// DeprecatedToolAliases maps old tool names to their new canonical names. +// This allows tool renames without breaking existing user configurations. +// When a user requests an old tool name, it will silently resolve to the new name. +var DeprecatedToolAliases = map[string]string{ + "test_1": "get_me", + "test_2": "get_me", +} + func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetGQLClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc, contentWindowSize int, flags FeatureFlags, cache *lockdown.RepoAccessCache) *toolsets.ToolsetGroup { tsg := toolsets.NewToolsetGroup(readOnly) @@ -378,6 +386,9 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG tsg.AddToolset(stargazers) tsg.AddToolset(labels) + // Register deprecated tool aliases for backward compatibility + tsg.AddDeprecatedToolAliases(DeprecatedToolAliases) + return tsg } diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index d4964ee5f..03a13adb1 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -192,19 +192,42 @@ 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 // oldName → newName for renamed tools + 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, } } +// AddDeprecatedToolAlias registers an alias for a renamed tool. +// When a user requests oldName, it will resolve to newName. +// This allows tool renames without breaking existing user configurations. +func (tg *ToolsetGroup) AddDeprecatedToolAlias(oldName, newName string) { + tg.deprecatedAliases[oldName] = newName +} + +// AddDeprecatedToolAliases registers multiple aliases for renamed tools. +func (tg *ToolsetGroup) AddDeprecatedToolAliases(aliases map[string]string) { + for oldName, newName := range aliases { + tg.deprecatedAliases[oldName] = newName + } +} + +// IsDeprecatedToolAlias checks if a tool name is a deprecated alias. +// Returns the canonical name and true if it's an alias, or empty string and false otherwise. +func (tg *ToolsetGroup) IsDeprecatedToolAlias(toolName string) (canonicalName string, isAlias bool) { + canonicalName, isAlias = tg.deprecatedAliases[toolName] + return canonicalName, isAlias +} + func (tg *ToolsetGroup) AddToolset(ts *Toolset) { if tg.readOnly { ts.SetReadOnly() @@ -307,9 +330,23 @@ func NewToolDoesNotExistError(name string) *ToolDoesNotExistError { return &ToolDoesNotExistError{Name: name} } +// ToolLookupResult contains the result of a tool lookup operation. +type ToolLookupResult struct { + Tool *ServerTool + ToolsetName string + RequestedName string // The name that was requested (may differ from Tool.Name if alias was used) + AliasUsed bool // True if the requested name was a deprecated alias +} + // FindToolByName searches all toolsets (enabled or disabled) for a tool by name. +// It resolves deprecated aliases automatically. // Returns the tool, its parent toolset name, and an error if not found. func (tg *ToolsetGroup) FindToolByName(toolName string) (*ServerTool, string, error) { + // Resolve deprecated alias if applicable + if canonicalName, isAlias := tg.deprecatedAliases[toolName]; isAlias { + toolName = canonicalName + } + for toolsetName, toolset := range tg.Toolsets { // Check read tools for _, tool := range toolset.readTools { @@ -327,9 +364,36 @@ func (tg *ToolsetGroup) FindToolByName(toolName string) (*ServerTool, string, er return nil, "", NewToolDoesNotExistError(toolName) } +// FindToolWithAliasInfo searches all toolsets for a tool by name and returns +// additional metadata about whether a deprecated alias was used. +// Use this when you need to track/log deprecated alias usage (e.g., for telemetry). +func (tg *ToolsetGroup) FindToolWithAliasInfo(toolName string) (*ToolLookupResult, error) { + requestedName := toolName + aliasUsed := false + + // Check if this is a deprecated alias and resolve to canonical name + if canonicalName, isAlias := tg.deprecatedAliases[toolName]; isAlias { + toolName = canonicalName + aliasUsed = true + } + + tool, toolsetName, err := tg.FindToolByName(toolName) + if err != nil { + return nil, NewToolDoesNotExistError(requestedName) + } + + return &ToolLookupResult{ + Tool: tool, + ToolsetName: toolsetName, + RequestedName: requestedName, + AliasUsed: aliasUsed, + }, nil +} + // RegisterSpecificTools registers only the specified tools. // Respects read-only mode (skips write tools if readOnly=true). // Returns error if any tool is not found. +// Deprecated tool aliases are resolved automatically. func (tg *ToolsetGroup) RegisterSpecificTools(s *mcp.Server, toolNames []string, readOnly bool) error { var skippedTools []string for _, toolName := range toolNames { From a837e5798d2504d4c89742fd2b6107f5a1552a35 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Tue, 9 Dec 2025 13:00:12 +0000 Subject: [PATCH 02/14] cleanup --- pkg/toolsets/toolsets.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index 03a13adb1..1558e0c00 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -192,10 +192,10 @@ func (t *Toolset) AddReadTools(tools ...ServerTool) *Toolset { } type ToolsetGroup struct { - Toolsets map[string]*Toolset - deprecatedAliases map[string]string // oldName → newName for renamed tools - everythingOn bool - readOnly bool + Toolsets map[string]*Toolset + deprecatedAliases map[string]string // oldName → newName for renamed tools + everythingOn bool + readOnly bool } func NewToolsetGroup(readOnly bool) *ToolsetGroup { From 6883f920a684fbc13ef780f37ef5322dedb03b11 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Tue, 9 Dec 2025 13:03:30 +0000 Subject: [PATCH 03/14] log alias usage as warning --- pkg/toolsets/toolsets.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index 1558e0c00..c7db9d678 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -339,11 +339,12 @@ type ToolLookupResult struct { } // FindToolByName searches all toolsets (enabled or disabled) for a tool by name. -// It resolves deprecated aliases automatically. +// It resolves deprecated aliases automatically and logs a warning when an alias is used. // Returns the tool, its parent toolset name, and an error if not found. func (tg *ToolsetGroup) FindToolByName(toolName string) (*ServerTool, string, error) { // Resolve deprecated alias if applicable if canonicalName, isAlias := tg.deprecatedAliases[toolName]; isAlias { + fmt.Fprintf(os.Stderr, "Warning: tool %q is deprecated, use %q instead\n", toolName, canonicalName) toolName = canonicalName } From d77ae1307c45d3bc48fab60daba9e8c43c480e2d Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Wed, 10 Dec 2025 10:55:34 +0000 Subject: [PATCH 04/14] remove comments --- pkg/github/tools.go | 1 - pkg/toolsets/toolsets.go | 4 ---- 2 files changed, 5 deletions(-) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 133e2203f..9fd2ce4cd 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -386,7 +386,6 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG tsg.AddToolset(stargazers) tsg.AddToolset(labels) - // Register deprecated tool aliases for backward compatibility tsg.AddDeprecatedToolAliases(DeprecatedToolAliases) return tsg diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index c7db9d678..989e9515d 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -207,14 +207,10 @@ func NewToolsetGroup(readOnly bool) *ToolsetGroup { } } -// AddDeprecatedToolAlias registers an alias for a renamed tool. -// When a user requests oldName, it will resolve to newName. -// This allows tool renames without breaking existing user configurations. func (tg *ToolsetGroup) AddDeprecatedToolAlias(oldName, newName string) { tg.deprecatedAliases[oldName] = newName } -// AddDeprecatedToolAliases registers multiple aliases for renamed tools. func (tg *ToolsetGroup) AddDeprecatedToolAliases(aliases map[string]string) { for oldName, newName := range aliases { tg.deprecatedAliases[oldName] = newName From 66a56998232a8aac136f30d0ef37240462dcca75 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Thu, 11 Dec 2025 10:35:31 +0000 Subject: [PATCH 05/14] remove mock data --- pkg/github/tools.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 9fd2ce4cd..48a5097a3 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -163,10 +163,8 @@ func GetDefaultToolsetIDs() []string { // DeprecatedToolAliases maps old tool names to their new canonical names. // This allows tool renames without breaking existing user configurations. // When a user requests an old tool name, it will silently resolve to the new name. -var DeprecatedToolAliases = map[string]string{ - "test_1": "get_me", - "test_2": "get_me", -} +// Example: "get_issue" : "issue_read" +var DeprecatedToolAliases = map[string]string{} func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetGQLClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc, contentWindowSize int, flags FeatureFlags, cache *lockdown.RepoAccessCache) *toolsets.ToolsetGroup { tsg := toolsets.NewToolsetGroup(readOnly) From 8f9fa1d5c2242a1a2a62f9b1770cd1f03b55fed8 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Thu, 11 Dec 2025 11:39:22 +0000 Subject: [PATCH 06/14] remove unused code, move deprecated tool aliases to its own file --- pkg/github/deprecated_tool_aliases.go | 14 +++++++++++ pkg/github/tools.go | 6 ----- pkg/toolsets/toolsets.go | 34 --------------------------- 3 files changed, 14 insertions(+), 40 deletions(-) create mode 100644 pkg/github/deprecated_tool_aliases.go diff --git a/pkg/github/deprecated_tool_aliases.go b/pkg/github/deprecated_tool_aliases.go new file mode 100644 index 000000000..ddd9b7483 --- /dev/null +++ b/pkg/github/deprecated_tool_aliases.go @@ -0,0 +1,14 @@ +// 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 +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 48a5097a3..f21a9ae5b 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -160,12 +160,6 @@ func GetDefaultToolsetIDs() []string { } } -// DeprecatedToolAliases maps old tool names to their new canonical names. -// This allows tool renames without breaking existing user configurations. -// When a user requests an old tool name, it will silently resolve to the new name. -// Example: "get_issue" : "issue_read" -var DeprecatedToolAliases = map[string]string{} - func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetGQLClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc, contentWindowSize int, flags FeatureFlags, cache *lockdown.RepoAccessCache) *toolsets.ToolsetGroup { tsg := toolsets.NewToolsetGroup(readOnly) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index 989e9515d..3698303c0 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -326,14 +326,6 @@ func NewToolDoesNotExistError(name string) *ToolDoesNotExistError { return &ToolDoesNotExistError{Name: name} } -// ToolLookupResult contains the result of a tool lookup operation. -type ToolLookupResult struct { - Tool *ServerTool - ToolsetName string - RequestedName string // The name that was requested (may differ from Tool.Name if alias was used) - AliasUsed bool // True if the requested name was a deprecated alias -} - // FindToolByName searches all toolsets (enabled or disabled) for a tool by name. // It resolves deprecated aliases automatically and logs a warning when an alias is used. // Returns the tool, its parent toolset name, and an error if not found. @@ -361,32 +353,6 @@ func (tg *ToolsetGroup) FindToolByName(toolName string) (*ServerTool, string, er return nil, "", NewToolDoesNotExistError(toolName) } -// FindToolWithAliasInfo searches all toolsets for a tool by name and returns -// additional metadata about whether a deprecated alias was used. -// Use this when you need to track/log deprecated alias usage (e.g., for telemetry). -func (tg *ToolsetGroup) FindToolWithAliasInfo(toolName string) (*ToolLookupResult, error) { - requestedName := toolName - aliasUsed := false - - // Check if this is a deprecated alias and resolve to canonical name - if canonicalName, isAlias := tg.deprecatedAliases[toolName]; isAlias { - toolName = canonicalName - aliasUsed = true - } - - tool, toolsetName, err := tg.FindToolByName(toolName) - if err != nil { - return nil, NewToolDoesNotExistError(requestedName) - } - - return &ToolLookupResult{ - Tool: tool, - ToolsetName: toolsetName, - RequestedName: requestedName, - AliasUsed: aliasUsed, - }, nil -} - // RegisterSpecificTools registers only the specified tools. // Respects read-only mode (skips write tools if readOnly=true). // Returns error if any tool is not found. From 0d82b4b08012810712ea717d925c0a9a6be32f42 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Thu, 11 Dec 2025 11:46:28 +0000 Subject: [PATCH 07/14] remove unused code and add tests --- pkg/toolsets/toolsets.go | 4 - pkg/toolsets/toolsets_test.go | 152 ++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 4 deletions(-) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index 3698303c0..384c7e0d2 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -207,10 +207,6 @@ func NewToolsetGroup(readOnly bool) *ToolsetGroup { } } -func (tg *ToolsetGroup) AddDeprecatedToolAlias(oldName, newName string) { - tg.deprecatedAliases[oldName] = newName -} - func (tg *ToolsetGroup) AddDeprecatedToolAliases(aliases map[string]string) { for oldName, newName := range aliases { tg.deprecatedAliases[oldName] = newName diff --git a/pkg/toolsets/toolsets_test.go b/pkg/toolsets/toolsets_test.go index 3f4581f34..7bd60faaa 100644 --- a/pkg/toolsets/toolsets_test.go +++ b/pkg/toolsets/toolsets_test.go @@ -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 { @@ -262,3 +277,140 @@ 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 TestIsDeprecatedToolAlias(t *testing.T) { + tsg := NewToolsetGroup(false) + tsg.AddDeprecatedToolAliases(map[string]string{"old_tool": "new_tool"}) + + // Test with a deprecated alias + canonical, isAlias := tsg.IsDeprecatedToolAlias("old_tool") + if !isAlias { + t.Error("expected 'old_tool' to be recognized as an alias") + } + if canonical != "new_tool" { + t.Errorf("expected canonical name 'new_tool', got '%s'", canonical) + } + + // Test with a non-alias + canonical, isAlias = tsg.IsDeprecatedToolAlias("some_other_tool") + if isAlias { + t.Error("expected 'some_other_tool' to not be an alias") + } + if canonical != "" { + t.Errorf("expected empty canonical name, got '%s'", canonical) + } +} + +func TestFindToolByName_WithAlias(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) + + // Add an alias + tsg.AddDeprecatedToolAliases(map[string]string{"get_issue": "issue_read"}) + + // 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) + } + + // Find by deprecated alias (should resolve to canonical) + tool, toolsetName, err = tsg.FindToolByName("get_issue") + if err != nil { + t.Fatalf("expected no error when using alias, got %v", err) + } + if tool.Tool.Name != "issue_read" { + t.Errorf("expected tool name 'issue_read' when using alias, got '%s'", tool.Tool.Name) + } + if toolsetName != "test-toolset" { + t.Errorf("expected toolset name 'test-toolset', got '%s'", toolsetName) + } +} + +func TestFindToolByName_NotFound(t *testing.T) { + tsg := NewToolsetGroup(false) + + // Create a toolset with a tool + toolset := NewToolset("test-toolset", "Test toolset") + toolset.readTools = append(toolset.readTools, mockTool("some_tool", true)) + tsg.AddToolset(toolset) + + // Try to find a non-existent tool + _, _, err := tsg.FindToolByName("nonexistent_tool") + if err == nil { + t.Error("expected error for non-existent tool") + } + + var toolErr *ToolDoesNotExistError + if !errors.As(err, &toolErr) { + t.Errorf("expected ToolDoesNotExistError, got %T", err) + } +} + +func TestRegisterSpecificTools_WithAliases(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) + + // Add aliases + tsg.AddDeprecatedToolAliases(map[string]string{ + "get_issue": "issue_read", + "create_issue": "issue_write", + }) + + // Test registering with aliases (should work) + err := tsg.RegisterSpecificTools(nil, []string{"get_issue"}, false) + if err != nil { + t.Errorf("expected no error registering aliased tool, got %v", err) + } + + // Test registering write tool alias in read-only mode (should skip but not error) + err = tsg.RegisterSpecificTools(nil, []string{"create_issue"}, 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") + } +} From 9c96523fafda7867286c35953e0bd234f8013cf6 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Thu, 11 Dec 2025 13:29:22 +0000 Subject: [PATCH 08/14] resolve tool aliases in its own explicit step --- internal/ghmcp/server.go | 3 +- pkg/toolsets/toolsets.go | 28 ++++++++---- pkg/toolsets/toolsets_test.go | 80 +++++++++++++++-------------------- 3 files changed, 56 insertions(+), 55 deletions(-) diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index f4e67f264..8e271b394 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -176,8 +176,9 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) { // Register specific tools if configured if len(cfg.EnabledTools) > 0 { - // Clean and validate tool names + // Clean tool names and resolve deprecated aliases 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) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index 384c7e0d2..a3e245e01 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -322,16 +322,26 @@ func NewToolDoesNotExistError(name string) *ToolDoesNotExistError { return &ToolDoesNotExistError{Name: name} } -// FindToolByName searches all toolsets (enabled or disabled) for a tool by name. -// It resolves deprecated aliases automatically and logs a warning when an alias is used. -// Returns the tool, its parent toolset name, and an error if not found. -func (tg *ToolsetGroup) FindToolByName(toolName string) (*ServerTool, string, error) { - // Resolve deprecated alias if applicable - if canonicalName, isAlias := tg.deprecatedAliases[toolName]; isAlias { - fmt.Fprintf(os.Stderr, "Warning: tool %q is deprecated, use %q instead\n", toolName, canonicalName) - toolName = canonicalName +// ResolveToolAliases resolves deprecated tool aliases to their canonical names. +// It logs a warning to stderr for each deprecated alias that is resolved. +// Returns the list of tool names with aliases replaced by canonical names. +func (tg *ToolsetGroup) ResolveToolAliases(toolNames []string) []string { + resolved := make([]string, 0, len(toolNames)) + 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) + resolved = append(resolved, canonicalName) + } else { + resolved = append(resolved, toolName) + } } + return resolved +} +// FindToolByName searches all toolsets (enabled or disabled) for a tool by its canonical name. +// Returns the tool, its parent toolset name, and an error if not found. +// Note: This function does NOT resolve deprecated aliases. Use ResolveToolAliases first if needed. +func (tg *ToolsetGroup) FindToolByName(toolName string) (*ServerTool, string, error) { for toolsetName, toolset := range tg.Toolsets { // Check read tools for _, tool := range toolset.readTools { @@ -352,7 +362,7 @@ func (tg *ToolsetGroup) FindToolByName(toolName string) (*ServerTool, string, er // RegisterSpecificTools registers only the specified tools. // Respects read-only mode (skips write tools if readOnly=true). // Returns error if any tool is not found. -// Deprecated tool aliases are resolved automatically. +// Note: This function expects canonical tool names. Use ResolveToolAliases first if needed. func (tg *ToolsetGroup) RegisterSpecificTools(s *mcp.Server, toolNames []string, readOnly bool) error { var skippedTools []string for _, toolName := range toolNames { diff --git a/pkg/toolsets/toolsets_test.go b/pkg/toolsets/toolsets_test.go index 7bd60faaa..f336031a9 100644 --- a/pkg/toolsets/toolsets_test.go +++ b/pkg/toolsets/toolsets_test.go @@ -325,7 +325,32 @@ func TestIsDeprecatedToolAlias(t *testing.T) { } } -func TestFindToolByName_WithAlias(t *testing.T) { +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 := tsg.ResolveToolAliases(input) + + 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]) + } +} + +func TestFindToolByName(t *testing.T) { tsg := NewToolsetGroup(false) // Create a toolset with a tool @@ -333,9 +358,6 @@ func TestFindToolByName_WithAlias(t *testing.T) { toolset.readTools = append(toolset.readTools, mockTool("issue_read", true)) tsg.AddToolset(toolset) - // Add an alias - tsg.AddDeprecatedToolAliases(map[string]string{"get_issue": "issue_read"}) - // Find by canonical name tool, toolsetName, err := tsg.FindToolByName("issue_read") if err != nil { @@ -348,40 +370,14 @@ func TestFindToolByName_WithAlias(t *testing.T) { t.Errorf("expected toolset name 'test-toolset', got '%s'", toolsetName) } - // Find by deprecated alias (should resolve to canonical) - tool, toolsetName, err = tsg.FindToolByName("get_issue") - if err != nil { - t.Fatalf("expected no error when using alias, got %v", err) - } - if tool.Tool.Name != "issue_read" { - t.Errorf("expected tool name 'issue_read' when using alias, got '%s'", tool.Tool.Name) - } - if toolsetName != "test-toolset" { - t.Errorf("expected toolset name 'test-toolset', got '%s'", toolsetName) - } -} - -func TestFindToolByName_NotFound(t *testing.T) { - tsg := NewToolsetGroup(false) - - // Create a toolset with a tool - toolset := NewToolset("test-toolset", "Test toolset") - toolset.readTools = append(toolset.readTools, mockTool("some_tool", true)) - tsg.AddToolset(toolset) - - // Try to find a non-existent tool - _, _, err := tsg.FindToolByName("nonexistent_tool") + // FindToolByName does NOT resolve aliases - it expects canonical names + _, _, err = tsg.FindToolByName("get_issue") if err == nil { - t.Error("expected error for non-existent tool") - } - - var toolErr *ToolDoesNotExistError - if !errors.As(err, &toolErr) { - t.Errorf("expected ToolDoesNotExistError, got %T", err) + t.Error("expected error when using alias directly with FindToolByName") } } -func TestRegisterSpecificTools_WithAliases(t *testing.T) { +func TestRegisterSpecificTools(t *testing.T) { tsg := NewToolsetGroup(false) // Create a toolset with both read and write tools @@ -390,20 +386,14 @@ func TestRegisterSpecificTools_WithAliases(t *testing.T) { toolset.writeTools = append(toolset.writeTools, mockTool("issue_write", false)) tsg.AddToolset(toolset) - // Add aliases - tsg.AddDeprecatedToolAliases(map[string]string{ - "get_issue": "issue_read", - "create_issue": "issue_write", - }) - - // Test registering with aliases (should work) - err := tsg.RegisterSpecificTools(nil, []string{"get_issue"}, false) + // Test registering with canonical names + err := tsg.RegisterSpecificTools(nil, []string{"issue_read"}, false) if err != nil { - t.Errorf("expected no error registering aliased tool, got %v", err) + t.Errorf("expected no error registering tool, got %v", err) } - // Test registering write tool alias in read-only mode (should skip but not error) - err = tsg.RegisterSpecificTools(nil, []string{"create_issue"}, true) + // 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) } From 5192ff180e897180cedd9502d10801ec109fe9dc Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Thu, 11 Dec 2025 14:48:01 +0000 Subject: [PATCH 09/14] improve logic by returning aliases used in resolvetoolaliases --- internal/ghmcp/server.go | 2 +- pkg/toolsets/toolsets.go | 12 ++++++++---- pkg/toolsets/toolsets_test.go | 14 +++++++++++++- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 8e271b394..9fa05008a 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -178,7 +178,7 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) { if len(cfg.EnabledTools) > 0 { // Clean tool names and resolve deprecated aliases enabledTools := github.CleanTools(cfg.EnabledTools) - enabledTools = tsg.ResolveToolAliases(enabledTools) + enabledTools, _ = tsg.ResolveToolAliases(enabledTools) // Register the specified tools (additive to any toolsets already enabled) err = tsg.RegisterSpecificTools(ghServer, enabledTools, cfg.ReadOnly) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index a3e245e01..a947be74c 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -324,18 +324,22 @@ func NewToolDoesNotExistError(name string) *ToolDoesNotExistError { // ResolveToolAliases resolves deprecated tool aliases to their canonical names. // It logs a warning to stderr for each deprecated alias that is resolved. -// Returns the list of tool names with aliases replaced by canonical names. -func (tg *ToolsetGroup) ResolveToolAliases(toolNames []string) []string { - resolved := make([]string, 0, len(toolNames)) +// 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 + return resolved, aliasesUsed } // FindToolByName searches all toolsets (enabled or disabled) for a tool by its canonical name. diff --git a/pkg/toolsets/toolsets_test.go b/pkg/toolsets/toolsets_test.go index f336031a9..8652e94cf 100644 --- a/pkg/toolsets/toolsets_test.go +++ b/pkg/toolsets/toolsets_test.go @@ -334,8 +334,9 @@ func TestResolveToolAliases(t *testing.T) { // Test resolving a mix of aliases and canonical names input := []string{"get_issue", "some_tool", "create_pr"} - resolved := tsg.ResolveToolAliases(input) + resolved, aliasesUsed := tsg.ResolveToolAliases(input) + // Verify resolved names if len(resolved) != 3 { t.Fatalf("expected 3 resolved names, got %d", len(resolved)) } @@ -348,6 +349,17 @@ func TestResolveToolAliases(t *testing.T) { 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) { From 01f8d0399b4715acaaf058377abb1dfb32c3a616 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Thu, 11 Dec 2025 15:03:59 +0000 Subject: [PATCH 10/14] remove unused function --- internal/ghmcp/server.go | 1 - pkg/toolsets/toolsets.go | 9 +-------- pkg/toolsets/toolsets_test.go | 23 ----------------------- 3 files changed, 1 insertion(+), 32 deletions(-) diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 9fa05008a..ec2253157 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -176,7 +176,6 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) { // Register specific tools if configured if len(cfg.EnabledTools) > 0 { - // Clean tool names and resolve deprecated aliases enabledTools := github.CleanTools(cfg.EnabledTools) enabledTools, _ = tsg.ResolveToolAliases(enabledTools) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index a947be74c..5d038d45a 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -193,7 +193,7 @@ func (t *Toolset) AddReadTools(tools ...ServerTool) *Toolset { type ToolsetGroup struct { Toolsets map[string]*Toolset - deprecatedAliases map[string]string // oldName → newName for renamed tools + deprecatedAliases map[string]string everythingOn bool readOnly bool } @@ -213,13 +213,6 @@ func (tg *ToolsetGroup) AddDeprecatedToolAliases(aliases map[string]string) { } } -// IsDeprecatedToolAlias checks if a tool name is a deprecated alias. -// Returns the canonical name and true if it's an alias, or empty string and false otherwise. -func (tg *ToolsetGroup) IsDeprecatedToolAlias(toolName string) (canonicalName string, isAlias bool) { - canonicalName, isAlias = tg.deprecatedAliases[toolName] - return canonicalName, isAlias -} - func (tg *ToolsetGroup) AddToolset(ts *Toolset) { if tg.readOnly { ts.SetReadOnly() diff --git a/pkg/toolsets/toolsets_test.go b/pkg/toolsets/toolsets_test.go index 8652e94cf..6362aad0e 100644 --- a/pkg/toolsets/toolsets_test.go +++ b/pkg/toolsets/toolsets_test.go @@ -302,29 +302,6 @@ func TestAddDeprecatedToolAliases(t *testing.T) { } } -func TestIsDeprecatedToolAlias(t *testing.T) { - tsg := NewToolsetGroup(false) - tsg.AddDeprecatedToolAliases(map[string]string{"old_tool": "new_tool"}) - - // Test with a deprecated alias - canonical, isAlias := tsg.IsDeprecatedToolAlias("old_tool") - if !isAlias { - t.Error("expected 'old_tool' to be recognized as an alias") - } - if canonical != "new_tool" { - t.Errorf("expected canonical name 'new_tool', got '%s'", canonical) - } - - // Test with a non-alias - canonical, isAlias = tsg.IsDeprecatedToolAlias("some_other_tool") - if isAlias { - t.Error("expected 'some_other_tool' to not be an alias") - } - if canonical != "" { - t.Errorf("expected empty canonical name, got '%s'", canonical) - } -} - func TestResolveToolAliases(t *testing.T) { tsg := NewToolsetGroup(false) tsg.AddDeprecatedToolAliases(map[string]string{ From c9bccfb29acbf18e79b9d0d24b1d60d7be08ae5a Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Thu, 11 Dec 2025 15:05:30 +0000 Subject: [PATCH 11/14] remove comments --- pkg/toolsets/toolsets.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index 5d038d45a..438324581 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -335,9 +335,7 @@ func (tg *ToolsetGroup) ResolveToolAliases(toolNames []string) (resolved []strin return resolved, aliasesUsed } -// FindToolByName searches all toolsets (enabled or disabled) for a tool by its canonical name. // Returns the tool, its parent toolset name, and an error if not found. -// Note: This function does NOT resolve deprecated aliases. Use ResolveToolAliases first if needed. func (tg *ToolsetGroup) FindToolByName(toolName string) (*ServerTool, string, error) { for toolsetName, toolset := range tg.Toolsets { // Check read tools From 5ab2e9a099667d84a25e2360a7756915e831c81f Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Thu, 11 Dec 2025 15:07:18 +0000 Subject: [PATCH 12/14] remove comment --- pkg/toolsets/toolsets.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index 438324581..cbc2bab8b 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -357,7 +357,6 @@ func (tg *ToolsetGroup) FindToolByName(toolName string) (*ServerTool, string, er // RegisterSpecificTools registers only the specified tools. // Respects read-only mode (skips write tools if readOnly=true). // Returns error if any tool is not found. -// Note: This function expects canonical tool names. Use ResolveToolAliases first if needed. func (tg *ToolsetGroup) RegisterSpecificTools(s *mcp.Server, toolNames []string, readOnly bool) error { var skippedTools []string for _, toolName := range toolNames { From 32cebd77513afc694fecad99dbdee9d19be210b7 Mon Sep 17 00:00:00 2001 From: Tommaso Moro <37270480+tommaso-moro@users.noreply.github.com> Date: Thu, 11 Dec 2025 15:27:37 +0000 Subject: [PATCH 13/14] Update pkg/github/deprecated_tool_aliases.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/github/deprecated_tool_aliases.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/github/deprecated_tool_aliases.go b/pkg/github/deprecated_tool_aliases.go index ddd9b7483..4abdca14d 100644 --- a/pkg/github/deprecated_tool_aliases.go +++ b/pkg/github/deprecated_tool_aliases.go @@ -1,4 +1,4 @@ -// tool_aliases.go +// deprecated_tool_aliases.go package github // DeprecatedToolAliases maps old tool names to their new canonical names. From a01e05119b8ce5e30039da7be20c618613165974 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Thu, 11 Dec 2025 15:29:41 +0000 Subject: [PATCH 14/14] restore comment --- pkg/toolsets/toolsets.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index cbc2bab8b..d96b5fb50 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -335,6 +335,7 @@ func (tg *ToolsetGroup) ResolveToolAliases(toolNames []string) (resolved []strin 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) { for toolsetName, toolset := range tg.Toolsets {