Merge branch 'main' into fix-docs-multiline-indent

Author: SamMorrowDrums

internal/ghmcp/server.go

@@ -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)

pkg/github/deprecated_tool_aliases.go

@@ -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
+}

pkg/github/tools.go

@@ -378,6 +378,8 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG
 	tsg.AddToolset(stargazers)
 	tsg.AddToolset(labels)
 
+	tsg.AddDeprecatedToolAliases(DeprecatedToolAliases)
+
 	return tsg
 }
 

pkg/toolsets/toolsets.go

@@ -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
 	}
 }
 
@@ -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) {

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,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")
+	}
+}