🌐 AI搜索 & 代理 主页
Skip to content

Commit 94ee074

Browse files
tommaso-moroCopilot
tommaso-moro
and
Copilot
authored
Add support for safe tool renaming (#1563)
* add suppport for tool aliases for backwards compatibility when tool names change * cleanup * log alias usage as warning * remove comments * remove mock data * remove unused code, move deprecated tool aliases to its own file * remove unused code and add tests * resolve tool aliases in its own explicit step * improve logic by returning aliases used in resolvetoolaliases * remove unused function * remove comments * remove comment * Update pkg/github/deprecated_tool_aliases.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * restore comment --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 90a1255 commit 94ee074

File tree

5 files changed

+182
-7
lines changed

5 files changed

+182
-7
lines changed

‎internal/ghmcp/server.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
176176

177177
// Register specific tools if configured
178178
if len(cfg.EnabledTools) > 0 {
179-
// Clean and validate tool names
180179
enabledTools := github.CleanTools(cfg.EnabledTools)
180+
enabledTools, _ = tsg.ResolveToolAliases(enabledTools)
181181

182182
// Register the specified tools (additive to any toolsets already enabled)
183183
err = tsg.RegisterSpecificTools(ghServer, enabledTools, cfg.ReadOnly)

‎pkg/github/deprecated_tool_aliases.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// deprecated_tool_aliases.go
2+
package github
3+
4+
// DeprecatedToolAliases maps old tool names to their new canonical names.
5+
// When tools are renamed, add an entry here to maintain backward compatibility.
6+
// Users referencing the old name will receive the new tool with a deprecation warning.
7+
//
8+
// Example:
9+
//
10+
// "get_issue": "issue_read",
11+
// "create_pr": "pull_request_create",
12+
var DeprecatedToolAliases = map[string]string{
13+
// Add entries as tools are renamed
14+
}

‎pkg/github/tools.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,8 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG
378378
tsg.AddToolset(stargazers)
379379
tsg.AddToolset(labels)
380380

381+
tsg.AddDeprecatedToolAliases(DeprecatedToolAliases)
382+
381383
return tsg
382384
}
383385

‎pkg/toolsets/toolsets.go‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,16 +192,24 @@ func (t *Toolset) AddReadTools(tools ...ServerTool) *Toolset {
192192
}
193193

194194
type ToolsetGroup struct {
195-
Toolsets map[string]*Toolset
196-
everythingOn bool
197-
readOnly bool
195+
Toolsets map[string]*Toolset
196+
deprecatedAliases map[string]string
197+
everythingOn bool
198+
readOnly bool
198199
}
199200

200201
func NewToolsetGroup(readOnly bool) *ToolsetGroup {
201202
return &ToolsetGroup{
202-
Toolsets: make(map[string]*Toolset),
203-
everythingOn: false,
204-
readOnly: readOnly,
203+
Toolsets: make(map[string]*Toolset),
204+
deprecatedAliases: make(map[string]string),
205+
everythingOn: false,
206+
readOnly: readOnly,
207+
}
208+
}
209+
210+
func (tg *ToolsetGroup) AddDeprecatedToolAliases(aliases map[string]string) {
211+
for oldName, newName := range aliases {
212+
tg.deprecatedAliases[oldName] = newName
205213
}
206214
}
207215

@@ -307,6 +315,26 @@ func NewToolDoesNotExistError(name string) *ToolDoesNotExistError {
307315
return &ToolDoesNotExistError{Name: name}
308316
}
309317

318+
// ResolveToolAliases resolves deprecated tool aliases to their canonical names.
319+
// It logs a warning to stderr for each deprecated alias that is resolved.
320+
// Returns:
321+
// - resolved: tool names with aliases replaced by canonical names
322+
// - aliasesUsed: map of oldName → newName for each alias that was resolved
323+
func (tg *ToolsetGroup) ResolveToolAliases(toolNames []string) (resolved []string, aliasesUsed map[string]string) {
324+
resolved = make([]string, 0, len(toolNames))
325+
aliasesUsed = make(map[string]string)
326+
for _, toolName := range toolNames {
327+
if canonicalName, isAlias := tg.deprecatedAliases[toolName]; isAlias {
328+
fmt.Fprintf(os.Stderr, "Warning: tool %q is deprecated, use %q instead\n", toolName, canonicalName)
329+
aliasesUsed[toolName] = canonicalName
330+
resolved = append(resolved, canonicalName)
331+
} else {
332+
resolved = append(resolved, toolName)
333+
}
334+
}
335+
return resolved, aliasesUsed
336+
}
337+
310338
// FindToolByName searches all toolsets (enabled or disabled) for a tool by name.
311339
// Returns the tool, its parent toolset name, and an error if not found.
312340
func (tg *ToolsetGroup) FindToolByName(toolName string) (*ServerTool, string, error) {

‎pkg/toolsets/toolsets_test.go‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,23 @@ package toolsets
33
import (
44
"errors"
55
"testing"
6+
7+
"github.com/modelcontextprotocol/go-sdk/mcp"
68
)
79

10+
// mockTool creates a minimal ServerTool for testing
11+
func mockTool(name string, readOnly bool) ServerTool {
12+
return ServerTool{
13+
Tool: mcp.Tool{
14+
Name: name,
15+
Annotations: &mcp.ToolAnnotations{
16+
ReadOnlyHint: readOnly,
17+
},
18+
},
19+
RegisterFunc: func(_ *mcp.Server) {},
20+
}
21+
}
22+
823
func TestNewToolsetGroupIsEmptyWithoutEverythingOn(t *testing.T) {
924
tsg := NewToolsetGroup(false)
1025
if len(tsg.Toolsets) != 0 {
@@ -262,3 +277,119 @@ func TestToolsetGroup_GetToolset(t *testing.T) {
262277
t.Errorf("expected error to be ToolsetDoesNotExistError, got %v", err)
263278
}
264279
}
280+
281+
func TestAddDeprecatedToolAliases(t *testing.T) {
282+
tsg := NewToolsetGroup(false)
283+
284+
// Test adding aliases
285+
tsg.AddDeprecatedToolAliases(map[string]string{
286+
"old_name": "new_name",
287+
"get_issue": "issue_read",
288+
"create_pr": "pull_request_create",
289+
})
290+
291+
if len(tsg.deprecatedAliases) != 3 {
292+
t.Errorf("expected 3 aliases, got %d", len(tsg.deprecatedAliases))
293+
}
294+
if tsg.deprecatedAliases["old_name"] != "new_name" {
295+
t.Errorf("expected alias 'old_name' -> 'new_name', got '%s'", tsg.deprecatedAliases["old_name"])
296+
}
297+
if tsg.deprecatedAliases["get_issue"] != "issue_read" {
298+
t.Errorf("expected alias 'get_issue' -> 'issue_read'")
299+
}
300+
if tsg.deprecatedAliases["create_pr"] != "pull_request_create" {
301+
t.Errorf("expected alias 'create_pr' -> 'pull_request_create'")
302+
}
303+
}
304+
305+
func TestResolveToolAliases(t *testing.T) {
306+
tsg := NewToolsetGroup(false)
307+
tsg.AddDeprecatedToolAliases(map[string]string{
308+
"get_issue": "issue_read",
309+
"create_pr": "pull_request_create",
310+
})
311+
312+
// Test resolving a mix of aliases and canonical names
313+
input := []string{"get_issue", "some_tool", "create_pr"}
314+
resolved, aliasesUsed := tsg.ResolveToolAliases(input)
315+
316+
// Verify resolved names
317+
if len(resolved) != 3 {
318+
t.Fatalf("expected 3 resolved names, got %d", len(resolved))
319+
}
320+
if resolved[0] != "issue_read" {
321+
t.Errorf("expected 'issue_read', got '%s'", resolved[0])
322+
}
323+
if resolved[1] != "some_tool" {
324+
t.Errorf("expected 'some_tool' (unchanged), got '%s'", resolved[1])
325+
}
326+
if resolved[2] != "pull_request_create" {
327+
t.Errorf("expected 'pull_request_create', got '%s'", resolved[2])
328+
}
329+
330+
// Verify aliasesUsed map
331+
if len(aliasesUsed) != 2 {
332+
t.Fatalf("expected 2 aliases used, got %d", len(aliasesUsed))
333+
}
334+
if aliasesUsed["get_issue"] != "issue_read" {
335+
t.Errorf("expected aliasesUsed['get_issue'] = 'issue_read', got '%s'", aliasesUsed["get_issue"])
336+
}
337+
if aliasesUsed["create_pr"] != "pull_request_create" {
338+
t.Errorf("expected aliasesUsed['create_pr'] = 'pull_request_create', got '%s'", aliasesUsed["create_pr"])
339+
}
340+
}
341+
342+
func TestFindToolByName(t *testing.T) {
343+
tsg := NewToolsetGroup(false)
344+
345+
// Create a toolset with a tool
346+
toolset := NewToolset("test-toolset", "Test toolset")
347+
toolset.readTools = append(toolset.readTools, mockTool("issue_read", true))
348+
tsg.AddToolset(toolset)
349+
350+
// Find by canonical name
351+
tool, toolsetName, err := tsg.FindToolByName("issue_read")
352+
if err != nil {
353+
t.Fatalf("expected no error, got %v", err)
354+
}
355+
if tool.Tool.Name != "issue_read" {
356+
t.Errorf("expected tool name 'issue_read', got '%s'", tool.Tool.Name)
357+
}
358+
if toolsetName != "test-toolset" {
359+
t.Errorf("expected toolset name 'test-toolset', got '%s'", toolsetName)
360+
}
361+
362+
// FindToolByName does NOT resolve aliases - it expects canonical names
363+
_, _, err = tsg.FindToolByName("get_issue")
364+
if err == nil {
365+
t.Error("expected error when using alias directly with FindToolByName")
366+
}
367+
}
368+
369+
func TestRegisterSpecificTools(t *testing.T) {
370+
tsg := NewToolsetGroup(false)
371+
372+
// Create a toolset with both read and write tools
373+
toolset := NewToolset("test-toolset", "Test toolset")
374+
toolset.readTools = append(toolset.readTools, mockTool("issue_read", true))
375+
toolset.writeTools = append(toolset.writeTools, mockTool("issue_write", false))
376+
tsg.AddToolset(toolset)
377+
378+
// Test registering with canonical names
379+
err := tsg.RegisterSpecificTools(nil, []string{"issue_read"}, false)
380+
if err != nil {
381+
t.Errorf("expected no error registering tool, got %v", err)
382+
}
383+
384+
// Test registering write tool in read-only mode (should skip but not error)
385+
err = tsg.RegisterSpecificTools(nil, []string{"issue_write"}, true)
386+
if err != nil {
387+
t.Errorf("expected no error when skipping write tool in read-only mode, got %v", err)
388+
}
389+
390+
// Test registering non-existent tool (should error)
391+
err = tsg.RegisterSpecificTools(nil, []string{"nonexistent"}, false)
392+
if err == nil {
393+
t.Error("expected error for non-existent tool")
394+
}
395+
}

0 commit comments

Comments
 (0)