Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 1 addition & 1 deletion internal/ghmcp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,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)
err = tsg.EnableToolsets(enabledToolsets, nil)

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
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: true,
}
}

// 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
28 changes: 20 additions & 8 deletions pkg/toolsets/toolsets_test.go
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,7 +148,19 @@ 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"}, 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")
}
Expand All @@ -157,14 +169,14 @@ func TestEnableToolsets(t *testing.T) {
}

// 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 +199,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 +224,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