From b4391c0da83ea939c81c2c5a4a30bc908b95f340 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Fri, 10 Oct 2025 10:48:58 +0200 Subject: [PATCH 1/3] Allow unknown toolsets to be ignored --- internal/ghmcp/server.go | 3 ++- pkg/toolsets/toolsets.go | 16 +++++++++++++--- pkg/toolsets/toolsets_test.go | 21 +++++++++++++-------- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 34e374a24..63bdb2ae7 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -17,6 +17,7 @@ import ( "github.com/github/github-mcp-server/pkg/github" mcplog "github.com/github/github-mcp-server/pkg/log" "github.com/github/github-mcp-server/pkg/raw" + "github.com/github/github-mcp-server/pkg/toolsets" "github.com/github/github-mcp-server/pkg/translations" gogithub "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" @@ -142,7 +143,7 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) { // Create default toolsets tsg := github.DefaultToolsetGroup(cfg.ReadOnly, getClient, getGQLClient, getRawClient, cfg.Translator, cfg.ContentWindowSize) - err = tsg.EnableToolsets(enabledToolsets) + err = tsg.EnableToolsets(enabledToolsets, &toolsets.EnableToolsetsOptions{}) if err != nil { return nil, fmt.Errorf("failed to enable toolsets: %w", err) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index 85cd5d841..5f2d25f8a 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -203,7 +203,17 @@ func (tg *ToolsetGroup) IsEnabled(name string) bool { return feature.Enabled } -func (tg *ToolsetGroup) EnableToolsets(names []string) error { +type EnableToolsetsOptions struct { + IgnoreUnknown bool +} + +func (tg *ToolsetGroup) EnableToolsets(names []string, options *EnableToolsetsOptions) error { + if options == nil { + options = &EnableToolsetsOptions{ + IgnoreUnknown: false, + } + } + // Special case for "all" for _, name := range names { if name == "all" { @@ -211,7 +221,7 @@ func (tg *ToolsetGroup) EnableToolsets(names []string) error { break } err := tg.EnableToolset(name) - if err != nil { + if err != nil && !options.IgnoreUnknown { return err } } @@ -219,7 +229,7 @@ func (tg *ToolsetGroup) EnableToolsets(names []string) error { if tg.everythingOn { for name := range tg.Toolsets { err := tg.EnableToolset(name) - if err != nil { + if err != nil && !options.IgnoreUnknown { return err } } diff --git a/pkg/toolsets/toolsets_test.go b/pkg/toolsets/toolsets_test.go index d74c94bbb..116b6c8d9 100644 --- a/pkg/toolsets/toolsets_test.go +++ b/pkg/toolsets/toolsets_test.go @@ -134,7 +134,7 @@ func TestEnableToolsets(t *testing.T) { tsg.AddToolset(toolset2) // Test enabling multiple toolsets - err := tsg.EnableToolsets([]string{"toolset1", "toolset2"}) + err := tsg.EnableToolsets([]string{"toolset1", "toolset2"}, &EnableToolsetsOptions{}) if err != nil { t.Errorf("Expected no error when enabling toolsets, got: %v", err) } @@ -148,7 +148,7 @@ func TestEnableToolsets(t *testing.T) { } // Test with non-existent toolset in the list - err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}) + err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, &EnableToolsetsOptions{}) if err == nil { t.Error("Expected error when enabling list with non-existent toolset") } @@ -156,15 +156,20 @@ func TestEnableToolsets(t *testing.T) { t.Errorf("Expected ToolsetDoesNotExistError when enabling non-existent toolset, got: %v", err) } + err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, &EnableToolsetsOptions{IgnoreUnknown: true}) + if err != nil { + t.Errorf("Expected no error when ignoring unknown toolsets, got: %v", err) + } + // Test with empty list - err = tsg.EnableToolsets([]string{}) + err = tsg.EnableToolsets([]string{}, &EnableToolsetsOptions{}) if err != nil { t.Errorf("Expected no error with empty toolset list, got: %v", err) } // Test enabling everything through EnableToolsets tsg = NewToolsetGroup(false) - err = tsg.EnableToolsets([]string{"all"}) + err = tsg.EnableToolsets([]string{"all"}, &EnableToolsetsOptions{}) if err != nil { t.Errorf("Expected no error when enabling 'all', got: %v", err) } @@ -187,14 +192,14 @@ func TestEnableEverything(t *testing.T) { } // Enable "all" - err := tsg.EnableToolsets([]string{"all"}) + err := tsg.EnableToolsets([]string{"all"}, &EnableToolsetsOptions{}) if err != nil { - t.Errorf("Expected no error when enabling 'eall', got: %v", err) + t.Errorf("Expected no error when enabling 'all', got: %v", err) } // Verify everythingOn was set if !tsg.everythingOn { - t.Error("Expected everythingOn to be true after enabling 'eall'") + t.Error("Expected everythingOn to be true after enabling 'all'") } // Verify the previously disabled toolset is now enabled @@ -212,7 +217,7 @@ func TestIsEnabledWithEverythingOn(t *testing.T) { tsg := NewToolsetGroup(false) // Enable "all" - err := tsg.EnableToolsets([]string{"all"}) + err := tsg.EnableToolsets([]string{"all"}, &EnableToolsetsOptions{}) if err != nil { t.Errorf("Expected no error when enabling 'all', got: %v", err) } From 54e0aff959e09e35f831a43c8817e473b069377b Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Fri, 10 Oct 2025 14:48:33 +0200 Subject: [PATCH 2/3] Default to ignore unknown toolsets --- internal/ghmcp/server.go | 3 +-- pkg/toolsets/toolsets.go | 2 +- pkg/toolsets/toolsets_test.go | 19 +++++++++++++------ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 63bdb2ae7..2fb2fb19b 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -17,7 +17,6 @@ import ( "github.com/github/github-mcp-server/pkg/github" mcplog "github.com/github/github-mcp-server/pkg/log" "github.com/github/github-mcp-server/pkg/raw" - "github.com/github/github-mcp-server/pkg/toolsets" "github.com/github/github-mcp-server/pkg/translations" gogithub "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" @@ -143,7 +142,7 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) { // Create default toolsets tsg := github.DefaultToolsetGroup(cfg.ReadOnly, getClient, getGQLClient, getRawClient, cfg.Translator, cfg.ContentWindowSize) - err = tsg.EnableToolsets(enabledToolsets, &toolsets.EnableToolsetsOptions{}) + err = tsg.EnableToolsets(enabledToolsets, nil) if err != nil { return nil, fmt.Errorf("failed to enable toolsets: %w", err) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index 5f2d25f8a..096edf4ad 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -210,7 +210,7 @@ type EnableToolsetsOptions struct { func (tg *ToolsetGroup) EnableToolsets(names []string, options *EnableToolsetsOptions) error { if options == nil { options = &EnableToolsetsOptions{ - IgnoreUnknown: false, + IgnoreUnknown: true, } } diff --git a/pkg/toolsets/toolsets_test.go b/pkg/toolsets/toolsets_test.go index 116b6c8d9..12bf64cb8 100644 --- a/pkg/toolsets/toolsets_test.go +++ b/pkg/toolsets/toolsets_test.go @@ -148,7 +148,19 @@ func TestEnableToolsets(t *testing.T) { } // Test with non-existent toolset in the list - err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, &EnableToolsetsOptions{}) + err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, nil) + if err != nil { + t.Errorf("Expected no error when ignoring unknown toolsets, got: %v", err) + } + + err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, &EnableToolsetsOptions{ + IgnoreUnknown: true, + }) + if err != nil { + t.Errorf("Expected no error when ignoring unknown toolsets, got: %v", err) + } + + err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, &EnableToolsetsOptions{IgnoreUnknown: false}) if err == nil { t.Error("Expected error when enabling list with non-existent toolset") } @@ -156,11 +168,6 @@ func TestEnableToolsets(t *testing.T) { t.Errorf("Expected ToolsetDoesNotExistError when enabling non-existent toolset, got: %v", err) } - err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, &EnableToolsetsOptions{IgnoreUnknown: true}) - if err != nil { - t.Errorf("Expected no error when ignoring unknown toolsets, got: %v", err) - } - // Test with empty list err = tsg.EnableToolsets([]string{}, &EnableToolsetsOptions{}) if err != nil { From 480beaa11358af91b2aa9b1d5d41aecb38be3419 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Fri, 10 Oct 2025 14:53:41 +0200 Subject: [PATCH 3/3] Rename to error on unknown --- pkg/toolsets/toolsets.go | 8 ++++---- pkg/toolsets/toolsets_test.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go index 096edf4ad..96f1fc3ca 100644 --- a/pkg/toolsets/toolsets.go +++ b/pkg/toolsets/toolsets.go @@ -204,13 +204,13 @@ func (tg *ToolsetGroup) IsEnabled(name string) bool { } type EnableToolsetsOptions struct { - IgnoreUnknown bool + ErrorOnUnknown bool } func (tg *ToolsetGroup) EnableToolsets(names []string, options *EnableToolsetsOptions) error { if options == nil { options = &EnableToolsetsOptions{ - IgnoreUnknown: true, + ErrorOnUnknown: false, } } @@ -221,7 +221,7 @@ func (tg *ToolsetGroup) EnableToolsets(names []string, options *EnableToolsetsOp break } err := tg.EnableToolset(name) - if err != nil && !options.IgnoreUnknown { + if err != nil && options.ErrorOnUnknown { return err } } @@ -229,7 +229,7 @@ func (tg *ToolsetGroup) EnableToolsets(names []string, options *EnableToolsetsOp if tg.everythingOn { for name := range tg.Toolsets { err := tg.EnableToolset(name) - if err != nil && !options.IgnoreUnknown { + if err != nil && options.ErrorOnUnknown { return err } } diff --git a/pkg/toolsets/toolsets_test.go b/pkg/toolsets/toolsets_test.go index 12bf64cb8..3f4581f34 100644 --- a/pkg/toolsets/toolsets_test.go +++ b/pkg/toolsets/toolsets_test.go @@ -154,13 +154,13 @@ func TestEnableToolsets(t *testing.T) { } err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, &EnableToolsetsOptions{ - IgnoreUnknown: true, + ErrorOnUnknown: false, }) if err != nil { t.Errorf("Expected no error when ignoring unknown toolsets, got: %v", err) } - err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, &EnableToolsetsOptions{IgnoreUnknown: false}) + err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, &EnableToolsetsOptions{ErrorOnUnknown: true}) if err == nil { t.Error("Expected error when enabling list with non-existent toolset") }