Bug fix: invalid tool should error out

cmd/github-mcp-server/generate_docs.go

@@ -51,7 +51,8 @@ func generateReadmeDocs(readmePath string) error {
 	t, _ := translations.TranslationHelper()
 
 	// (not available to regular users) while including tools with FeatureFlagDisable.
-	r := github.NewInventory(t).WithToolsets([]string{"all"}).Build()
+	// Build() can only fail if WithTools specifies invalid tools - not used here
+	r, _ := github.NewInventory(t).WithToolsets([]string{"all"}).Build()
 
 	// Generate toolsets documentation
 	toolsetsDoc := generateToolsetsDoc(r)
@@ -341,7 +342,8 @@ func generateRemoteToolsetsDoc() string {
 	t, _ := translations.TranslationHelper()
 
 	// Build inventory - stateless
-	r := github.NewInventory(t).Build()
+	// Build() can only fail if WithTools specifies invalid tools - not used here
+	r, _ := github.NewInventory(t).Build()
 
 	// Generate table header (icon is combined with Name column)
 	buf.WriteString("| Name | Description | API URL | 1-Click Install (VS Code) | Read-only Link | 1-Click Read-only Install (VS Code) |\n")

cmd/github-mcp-server/list_scopes.go

@@ -121,7 +121,10 @@ func runListScopes() error {
 		inventoryBuilder = inventoryBuilder.WithTools(enabledTools)
 	}
 
-	inv := inventoryBuilder.Build()
+	inv, err := inventoryBuilder.Build()
+	if err != nil {
+		return fmt.Errorf("failed to build inventory: %w", err)
+	}
 
 	// Collect all tools and their scopes
 	output := collectToolScopes(inv, readOnly)

internal/ghmcp/server.go

@@ -221,15 +221,18 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
 		WithDeprecatedAliases(github.DeprecatedToolAliases).
 		WithReadOnly(cfg.ReadOnly).
 		WithToolsets(enabledToolsets).
-		WithTools(github.CleanTools(cfg.EnabledTools)).
+		WithTools(cfg.EnabledTools).
 		WithFeatureChecker(createFeatureChecker(cfg.EnabledFeatures))
 
 	// Apply token scope filtering if scopes are known (for PAT filtering)
 	if cfg.TokenScopes != nil {
 		inventoryBuilder = inventoryBuilder.WithFilter(github.CreateToolScopeFilter(cfg.TokenScopes))
 	}
 
-	inventory := inventoryBuilder.Build()
+	inventory, err := inventoryBuilder.Build()
+	if err != nil {
+		return nil, fmt.Errorf("failed to build inventory: %w", err)
+	}
 
 	if unrecognized := inventory.UnrecognizedToolsets(); len(unrecognized) > 0 {
 		fmt.Fprintf(os.Stderr, "Warning: unrecognized toolsets ignored: %s\n", strings.Join(unrecognized, ", "))

pkg/github/dynamic_tools_test.go

@@ -25,9 +25,10 @@ func createDynamicRequest(args map[string]any) *mcp.CallToolRequest {
 
 func TestDynamicTools_ListAvailableToolsets(t *testing.T) {
 	// Build a registry with no toolsets enabled (dynamic mode)
-	reg := NewInventory(translations.NullTranslationHelper).
+	reg, err := NewInventory(translations.NullTranslationHelper).
 		WithToolsets([]string{}).
 		Build()
+	require.NoError(t, err)
 
 	// Create a mock server
 	server := mcp.NewServer(&mcp.Implementation{Name: "test"}, nil)
@@ -73,9 +74,10 @@ func TestDynamicTools_ListAvailableToolsets(t *testing.T) {
 
 func TestDynamicTools_GetToolsetTools(t *testing.T) {
 	// Build a registry with no toolsets enabled (dynamic mode)
-	reg := NewInventory(translations.NullTranslationHelper).
+	reg, err := NewInventory(translations.NullTranslationHelper).
 		WithToolsets([]string{}).
 		Build()
+	require.NoError(t, err)
 
 	// Create a mock server
 	server := mcp.NewServer(&mcp.Implementation{Name: "test"}, nil)
@@ -122,9 +124,10 @@ func TestDynamicTools_GetToolsetTools(t *testing.T) {
 
 func TestDynamicTools_EnableToolset(t *testing.T) {
 	// Build a registry with no toolsets enabled (dynamic mode)
-	reg := NewInventory(translations.NullTranslationHelper).
+	reg, err := NewInventory(translations.NullTranslationHelper).
 		WithToolsets([]string{}).
 		Build()
+	require.NoError(t, err)
 
 	// Create a mock server
 	server := mcp.NewServer(&mcp.Implementation{Name: "test"}, nil)
@@ -170,9 +173,10 @@ func TestDynamicTools_EnableToolset(t *testing.T) {
 
 func TestDynamicTools_EnableToolset_InvalidToolset(t *testing.T) {
 	// Build a registry with no toolsets enabled (dynamic mode)
-	reg := NewInventory(translations.NullTranslationHelper).
+	reg, err := NewInventory(translations.NullTranslationHelper).
 		WithToolsets([]string{}).
 		Build()
+	require.NoError(t, err)
 
 	// Create a mock server
 	server := mcp.NewServer(&mcp.Implementation{Name: "test"}, nil)
@@ -203,7 +207,8 @@ func TestDynamicTools_EnableToolset_InvalidToolset(t *testing.T) {
 
 func TestDynamicTools_ToolsetsEnum(t *testing.T) {
 	// Build a registry
-	reg := NewInventory(translations.NullTranslationHelper).Build()
+	reg, err := NewInventory(translations.NullTranslationHelper).Build()
+	require.NoError(t, err)
 
 	// Get tools to verify they have proper enum values
 	tools := DynamicTools(reg)

pkg/github/scope_filter_test.go

@@ -167,11 +167,12 @@ func TestCreateToolScopeFilter_Integration(t *testing.T) {
 	filter := CreateToolScopeFilter([]string{"repo"})
 
 	// Build inventory with the filter
-	inv := inventory.NewBuilder().
+	inv, err := inventory.NewBuilder().
 		SetTools(tools).
 		WithToolsets([]string{"test"}).
 		WithFilter(filter).
 		Build()
+	require.NoError(t, err)
 
 	// Get available tools
 	availableTools := inv.AvailableTools(context.Background())

pkg/github/tools.go

@@ -309,7 +309,8 @@ func ToStringPtr(s string) *string {
 // GenerateToolsetsHelp generates the help text for the toolsets flag
 func GenerateToolsetsHelp() string {
 	// Get toolset group to derive defaults and available toolsets
-	r := NewInventory(stubTranslator).Build()
+	// Build() can only fail if WithTools specifies invalid tools - not used here
+	r, _ := NewInventory(stubTranslator).Build()
 
 	// Format default tools from metadata using strings.Builder
 	var defaultBuf strings.Builder
@@ -391,7 +392,8 @@ func AddDefaultToolset(result []string) []string {
 	result = RemoveToolset(result, string(ToolsetMetadataDefault.ID))
 
 	// Get default toolset IDs from the Inventory
-	r := NewInventory(stubTranslator).Build()
+	// Build() can only fail if WithTools specifies invalid tools - not used here
+	r, _ := NewInventory(stubTranslator).Build()
 	for _, id := range r.DefaultToolsetIDs() {
 		if !seen[string(id)] {
 			result = append(result, string(id))
@@ -443,7 +445,8 @@ func CleanTools(toolNames []string) []string {
 // GetDefaultToolsetIDs returns the IDs of toolsets marked as Default.
 // This is a convenience function that builds an inventory to determine defaults.
 func GetDefaultToolsetIDs() []string {
-	r := NewInventory(stubTranslator).Build()
+	// Build() can only fail if WithTools specifies invalid tools - not used here
+	r, _ := NewInventory(stubTranslator).Build()
 	ids := r.DefaultToolsetIDs()
 	result := make([]string, len(ids))
 	for i, id := range ids {

pkg/github/toolset_icons_test.go

@@ -13,7 +13,8 @@ import (
 // This prevents broken icon references from being merged.
 func TestAllToolsetIconsExist(t *testing.T) {
 	// Get all available toolsets from the inventory
-	inv := NewInventory(stubTranslator).Build()
+	inv, err := NewInventory(stubTranslator).Build()
+	require.NoError(t, err)
 	toolsets := inv.AvailableToolsets()
 
 	// Also test remote-only toolsets
@@ -72,7 +73,8 @@ func TestToolsetMetadataHasIcons(t *testing.T) {
 		"default": true, // Meta-toolset
 	}
 
-	inv := NewInventory(stubTranslator).Build()
+	inv, err := NewInventory(stubTranslator).Build()
+	require.NoError(t, err)
 	toolsets := inv.AvailableToolsets()
 
 	for _, ts := range toolsets {

pkg/inventory/builder.go

@@ -2,6 +2,7 @@ package inventory
 
 import (
 	"context"
+	"fmt"
 	"sort"
 	"strings"
 )
@@ -101,6 +102,7 @@ func (b *Builder) WithToolsets(toolsetIDs []string) *Builder {
 // WithTools specifies additional tools that bypass toolset filtering.
 // These tools are additive - they will be included even if their toolset is not enabled.
 // Read-only filtering still applies to these tools.
+// Input is cleaned (trimmed, deduplicated) during Build().
 // Deprecated tool aliases are automatically resolved to their canonical names during Build().
 // Returns self for chaining.
 func (b *Builder) WithTools(toolNames []string) *Builder {
@@ -127,11 +129,33 @@ func (b *Builder) WithFilter(filter ToolFilter) *Builder {
 	return b
 }
 
+// cleanTools trims whitespace and removes duplicates from tool names.
+// Empty strings after trimming are excluded.
+func cleanTools(tools []string) []string {
+	seen := make(map[string]bool)
+	var cleaned []string
+	for _, name := range tools {
+		trimmed := strings.TrimSpace(name)
+		if trimmed == "" {
+			continue
+		}
+		if !seen[trimmed] {
+			seen[trimmed] = true
+			cleaned = append(cleaned, trimmed)
+		}
+	}
+	return cleaned
+}
+
 // Build creates the final Inventory with all configuration applied.
 // This processes toolset filtering, tool name resolution, and sets up
 // the inventory for use. The returned Inventory is ready for use with
 // AvailableTools(), RegisterAll(), etc.
-func (b *Builder) Build() *Inventory {
+//
+// Build returns an error if any tools specified via WithTools() are not recognized
+// (i.e., they don't exist in the tool set and are not deprecated aliases).
+// This ensures invalid tool configurations fail fast at build time.
+func (b *Builder) Build() (*Inventory, error) {
 	r := &Inventory{
 		tools:             b.tools,
 		resourceTemplates: b.resourceTemplates,
@@ -145,10 +169,19 @@ func (b *Builder) Build() *Inventory {
 	// Process toolsets and pre-compute metadata in a single pass
 	r.enabledToolsets, r.unrecognizedToolsets, r.toolsetIDs, r.toolsetIDSet, r.defaultToolsetIDs, r.toolsetDescriptions = b.processToolsets()
 
-	// Process additional tools (resolve aliases)
+	// Build set of valid tool names for validation
+	validToolNames := make(map[string]bool, len(b.tools))
+	for i := range b.tools {
+		validToolNames[b.tools[i].Tool.Name] = true
+	}
+
+	// Process additional tools (clean, resolve aliases, and track unrecognized)
 	if len(b.additionalTools) > 0 {
-		r.additionalTools = make(map[string]bool, len(b.additionalTools))
-		for _, name := range b.additionalTools {
+		cleanedTools := cleanTools(b.additionalTools)
+
+		r.additionalTools = make(map[string]bool, len(cleanedTools))
+		var unrecognizedTools []string
+		for _, name := range cleanedTools {
 			// Always include the original name - this handles the case where
 			// the tool exists but is controlled by a feature flag that's OFF.
 			r.additionalTools[name] = true
@@ -157,11 +190,19 @@ func (b *Builder) Build() *Inventory {
 			// the new consolidated tool is available.
 			if canonical, isAlias := b.deprecatedAliases[name]; isAlias {
 				r.additionalTools[canonical] = true
+			} else if !validToolNames[name] {
+				// Not a valid tool and not a deprecated alias - track as unrecognized
+				unrecognizedTools = append(unrecognizedTools, name)
 			}
 		}
+
+		// Error out if there are unrecognized tools
+		if len(unrecognizedTools) > 0 {
+			return nil, fmt.Errorf("unrecognized tools: %s", strings.Join(unrecognizedTools, ", "))
+		}
 	}
 
-	return r
+	return r, nil
 }
 
 // processToolsets processes the toolsetIDs configuration and returns: