Skip to content

Allow unknown toolsets to be ignored #1202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 10, 2025
+34 −12
Merged

Allow unknown toolsets to be ignored #1202

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b4391c0
Allow unknown toolsets to be ignored
omgitsads Oct 10, 2025
54e0aff
Default to ignore unknown toolsets
omgitsads Oct 10, 2025
480beaa
Rename to error on unknown
omgitsads Oct 10, 2025
9a6bbc0
Merge branch 'main' into ignore-not-found-tools
omgitsads Oct 10, 2025
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
3 changes: 2 additions & 1 deletion internal/ghmcp/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 13 additions & 3 deletions pkg/toolsets/toolsets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,23 +203,33 @@ 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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we set it to true by default?

}
}

// Special case for "all"
for _, name := range names {
if name == "all" {
tg.everythingOn = true
break
}
err := tg.EnableToolset(name)
if err != nil {
if err != nil && !options.IgnoreUnknown {
return err
}
}
// Do this after to ensure all toolsets are enabled if "all" is present anywhere in list
if tg.everythingOn {
for name := range tg.Toolsets {
err := tg.EnableToolset(name)
if err != nil {
if err != nil && !options.IgnoreUnknown {
return err
}
}
Expand Down
21 changes: 13 additions & 8 deletions pkg/toolsets/toolsets_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -148,23 +148,28 @@ 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")
}
if !errors.Is(err, NewToolsetDoesNotExistError("non-existent")) {
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)
}
Expand All @@ -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
Expand All @@ -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)
}
Expand Down
Loading