Skip to content

Commit bcc5275

Browse files
authored
Ignore unknown toolsets (github#1202)
Default to ignore unknown toolsets, with the option to return an error.
1 parent 250723d commit bcc5275

File tree

3 files changed

+34
-12
lines changed

3 files changed

+34
-12
lines changed

internal/ghmcp/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
142142

143143
// Create default toolsets
144144
tsg := github.DefaultToolsetGroup(cfg.ReadOnly, getClient, getGQLClient, getRawClient, cfg.Translator, cfg.ContentWindowSize)
145-
err = tsg.EnableToolsets(enabledToolsets)
145+
err = tsg.EnableToolsets(enabledToolsets, nil)
146146

147147
if err != nil {
148148
return nil, fmt.Errorf("failed to enable toolsets: %w", err)

pkg/toolsets/toolsets.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,23 +203,33 @@ func (tg *ToolsetGroup) IsEnabled(name string) bool {
203203
return feature.Enabled
204204
}
205205

206-
func (tg *ToolsetGroup) EnableToolsets(names []string) error {
206+
type EnableToolsetsOptions struct {
207+
ErrorOnUnknown bool
208+
}
209+
210+
func (tg *ToolsetGroup) EnableToolsets(names []string, options *EnableToolsetsOptions) error {
211+
if options == nil {
212+
options = &EnableToolsetsOptions{
213+
ErrorOnUnknown: false,
214+
}
215+
}
216+
207217
// Special case for "all"
208218
for _, name := range names {
209219
if name == "all" {
210220
tg.everythingOn = true
211221
break
212222
}
213223
err := tg.EnableToolset(name)
214-
if err != nil {
224+
if err != nil && options.ErrorOnUnknown {
215225
return err
216226
}
217227
}
218228
// Do this after to ensure all toolsets are enabled if "all" is present anywhere in list
219229
if tg.everythingOn {
220230
for name := range tg.Toolsets {
221231
err := tg.EnableToolset(name)
222-
if err != nil {
232+
if err != nil && options.ErrorOnUnknown {
223233
return err
224234
}
225235
}

pkg/toolsets/toolsets_test.go

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ func TestEnableToolsets(t *testing.T) {
134134
tsg.AddToolset(toolset2)
135135

136136
// Test enabling multiple toolsets
137-
err := tsg.EnableToolsets([]string{"toolset1", "toolset2"})
137+
err := tsg.EnableToolsets([]string{"toolset1", "toolset2"}, &EnableToolsetsOptions{})
138138
if err != nil {
139139
t.Errorf("Expected no error when enabling toolsets, got: %v", err)
140140
}
@@ -148,7 +148,19 @@ func TestEnableToolsets(t *testing.T) {
148148
}
149149

150150
// Test with non-existent toolset in the list
151-
err = tsg.EnableToolsets([]string{"toolset1", "non-existent"})
151+
err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, nil)
152+
if err != nil {
153+
t.Errorf("Expected no error when ignoring unknown toolsets, got: %v", err)
154+
}
155+
156+
err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, &EnableToolsetsOptions{
157+
ErrorOnUnknown: false,
158+
})
159+
if err != nil {
160+
t.Errorf("Expected no error when ignoring unknown toolsets, got: %v", err)
161+
}
162+
163+
err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}, &EnableToolsetsOptions{ErrorOnUnknown: true})
152164
if err == nil {
153165
t.Error("Expected error when enabling list with non-existent toolset")
154166
}
@@ -157,14 +169,14 @@ func TestEnableToolsets(t *testing.T) {
157169
}
158170

159171
// Test with empty list
160-
err = tsg.EnableToolsets([]string{})
172+
err = tsg.EnableToolsets([]string{}, &EnableToolsetsOptions{})
161173
if err != nil {
162174
t.Errorf("Expected no error with empty toolset list, got: %v", err)
163175
}
164176

165177
// Test enabling everything through EnableToolsets
166178
tsg = NewToolsetGroup(false)
167-
err = tsg.EnableToolsets([]string{"all"})
179+
err = tsg.EnableToolsets([]string{"all"}, &EnableToolsetsOptions{})
168180
if err != nil {
169181
t.Errorf("Expected no error when enabling 'all', got: %v", err)
170182
}
@@ -187,14 +199,14 @@ func TestEnableEverything(t *testing.T) {
187199
}
188200

189201
// Enable "all"
190-
err := tsg.EnableToolsets([]string{"all"})
202+
err := tsg.EnableToolsets([]string{"all"}, &EnableToolsetsOptions{})
191203
if err != nil {
192-
t.Errorf("Expected no error when enabling 'eall', got: %v", err)
204+
t.Errorf("Expected no error when enabling 'all', got: %v", err)
193205
}
194206

195207
// Verify everythingOn was set
196208
if !tsg.everythingOn {
197-
t.Error("Expected everythingOn to be true after enabling 'eall'")
209+
t.Error("Expected everythingOn to be true after enabling 'all'")
198210
}
199211

200212
// Verify the previously disabled toolset is now enabled
@@ -212,7 +224,7 @@ func TestIsEnabledWithEverythingOn(t *testing.T) {
212224
tsg := NewToolsetGroup(false)
213225

214226
// Enable "all"
215-
err := tsg.EnableToolsets([]string{"all"})
227+
err := tsg.EnableToolsets([]string{"all"}, &EnableToolsetsOptions{})
216228
if err != nil {
217229
t.Errorf("Expected no error when enabling 'all', got: %v", err)
218230
}

0 commit comments

Comments
 (0)