adding error message for invalid toolsets

Author: tonytrg

internal/ghmcp/server.go

@@ -106,7 +106,12 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
 		},
 	}
 
-	enabledToolsets := cleanToolsets(cfg.EnabledToolsets, cfg.DynamicToolsets)
+	enabledToolsets, invalidToolsets := cleanToolsets(cfg.EnabledToolsets, cfg.DynamicToolsets)
+
+	// Log warning about invalid toolsets
+	if len(invalidToolsets) > 0 {
+		slog.Warn("invalid toolsets ignored", "toolsets", strings.Join(invalidToolsets, ", "))
+	}
 
 	// Generate instructions based on enabled toolsets
 	instructions := github.GenerateInstructions(enabledToolsets)
@@ -465,20 +470,32 @@ func (t *bearerAuthTransport) RoundTrip(req *http.Request) (*http.Response, erro
 // cleanToolsets handles special toolset keywords in the enabled toolsets list:
 // - Duplicates are removed from the result.
 // - Removes whitespaces
+// - Validates toolset names and returns invalid ones separately
 // - "all": Returns ["all"] immediately, ignoring all other toolsets (unless dynamicToolsets is true)
 // - "default": Replaces with the actual default toolset IDs from GetDefaultToolsetIDs()
 // When dynamicToolsets is true, filters out "all" from the enabled toolsets.
-func cleanToolsets(enabledToolsets []string, dynamicToolsets bool) []string {
+// Returns: (validToolsets, invalidToolsets)
+func cleanToolsets(enabledToolsets []string, dynamicToolsets bool) ([]string, []string) {
 	seen := make(map[string]bool)
 	result := make([]string, 0, len(enabledToolsets))
+	invalid := make([]string, 0)
+	validIDs := github.GetValidToolsetIDs()
 
 	// Add non-default toolsets, removing duplicates and trimming whitespace
 	for _, toolset := range enabledToolsets {
 		trimmed := strings.TrimSpace(toolset)
+		if trimmed == "" {
+			continue
+		}
 		if !seen[trimmed] {
 			seen[trimmed] = true
 			if trimmed != github.ToolsetMetadataDefault.ID && trimmed != github.ToolsetMetadataAll.ID {
-				result = append(result, trimmed)
+				// Validate the toolset name
+				if validIDs[trimmed] {
+					result = append(result, trimmed)
+				} else {
+					invalid = append(invalid, trimmed)
+				}
 			}
 		}
 	}
@@ -488,7 +505,7 @@ func cleanToolsets(enabledToolsets []string, dynamicToolsets bool) []string {
 
 	// Handle "all" keyword - return early if not in dynamic mode
 	if hasAll && !dynamicToolsets {
-		return []string{github.ToolsetMetadataAll.ID}
+		return []string{github.ToolsetMetadataAll.ID}, invalid
 	}
 
 	// Expand "default" keyword to actual default toolsets
@@ -501,5 +518,5 @@ func cleanToolsets(enabledToolsets []string, dynamicToolsets bool) []string {
 		}
 	}
 
-	return result
+	return result, invalid
 }

internal/ghmcp/server_test.go

@@ -13,6 +13,7 @@ func TestCleanToolsets(t *testing.T) {
 		input           []string
 		dynamicToolsets bool
 		expected        []string
+		expectedInvalid []string
 	}{
 		{
 			name:            "empty slice",
@@ -211,16 +212,41 @@ func TestCleanToolsets(t *testing.T) {
 			dynamicToolsets: false,
 			expected:        []string{"all"},
 		},
+		// Invalid toolset test cases
+		{
+			name:            "mix of valid and invalid toolsets",
+			input:           []string{"actions", "invalid_toolset", "gists", "typo_repo"},
+			dynamicToolsets: false,
+			expected:        []string{"actions", "gists"},
+			expectedInvalid: []string{"invalid_toolset", "typo_repo"},
+		},
+		{
+			name:            "invalid with whitespace",
+			input:           []string{" invalid_tool ", "  actions  ", " typo_gist "},
+			dynamicToolsets: false,
+			expected:        []string{"actions"},
+			expectedInvalid: []string{"invalid_tool", "typo_gist"},
+		},
+		{
+			name:            "empty string in toolsets",
+			input:           []string{"", "actions", "  ", "gists"},
+			dynamicToolsets: false,
+			expected:        []string{"actions", "gists"},
+			expectedInvalid: []string{},
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := cleanToolsets(tt.input, tt.dynamicToolsets)
+			result, invalid := cleanToolsets(tt.input, tt.dynamicToolsets)
 
-			// Check that the result has the correct length
 			require.Len(t, result, len(tt.expected), "result length should match expected length")
 
-			// Create a map for easier comparison since order might vary
+			if tt.expectedInvalid == nil {
+				tt.expectedInvalid = []string{}
+			}
+			require.Len(t, invalid, len(tt.expectedInvalid), "invalid length should match expected invalid length")
+
 			resultMap := make(map[string]bool)
 			for _, toolset := range result {
 				resultMap[toolset] = true
@@ -231,13 +257,21 @@ func TestCleanToolsets(t *testing.T) {
 				expectedMap[toolset] = true
 			}
 
-			// Check that both maps contain the same toolsets
+			invalidMap := make(map[string]bool)
+			for _, toolset := range invalid {
+				invalidMap[toolset] = true
+			}
+
+			expectedInvalidMap := make(map[string]bool)
+			for _, toolset := range tt.expectedInvalid {
+				expectedInvalidMap[toolset] = true
+			}
+
 			assert.Equal(t, expectedMap, resultMap, "result should contain all expected toolsets without duplicates")
+			assert.Equal(t, expectedInvalidMap, invalidMap, "invalid should contain all expected invalid toolsets")
 
-			// Verify no duplicates in result
 			assert.Len(t, resultMap, len(result), "result should not contain duplicates")
 
-			// Verify "default" is not in the result
 			assert.False(t, resultMap["default"], "result should not contain 'default'")
 		})
 	}

pkg/github/tools.go

@@ -133,6 +133,18 @@ func AvailableTools() []ToolsetMetadata {
 	}
 }
 
+// GetValidToolsetIDs returns a map of all valid toolset IDs for quick lookup
+func GetValidToolsetIDs() map[string]bool {
+	validIDs := make(map[string]bool)
+	for _, tool := range AvailableTools() {
+		validIDs[tool.ID] = true
+	}
+	// Add special keywords
+	validIDs[ToolsetMetadataAll.ID] = true
+	validIDs[ToolsetMetadataDefault.ID] = true
+	return validIDs
+}
+
 func GetDefaultToolsetIDs() []string {
 	return []string{
 		ToolsetMetadataContext.ID,