get it working

Author: tommaso-moro

internal/ghmcp/server.go

@@ -235,6 +235,11 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
 		fmt.Fprintf(os.Stderr, "Warning: unrecognized toolsets ignored: %s\n", strings.Join(unrecognized, ", "))
 	}
 
+	// Check for unrecognized tools and error out (unlike toolsets which just warn)
+	if unrecognized := inventory.UnrecognizedTools(); len(unrecognized) > 0 {
+		return nil, fmt.Errorf("unrecognized tools: %s", strings.Join(unrecognized, ", "))
+	}
+
 	// Register GitHub tools/resources/prompts from the inventory.
 	// In dynamic mode with no explicit toolsets, this is a no-op since enabledToolsets
 	// is empty - users enable toolsets at runtime via the dynamic tools below (but can

pkg/inventory/builder.go

@@ -145,9 +145,17 @@ 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 (resolve aliases and track unrecognized)
+	// Note: input is expected to be pre-cleaned (trimmed, deduped) via CleanTools
 	if len(b.additionalTools) > 0 {
 		r.additionalTools = make(map[string]bool, len(b.additionalTools))
+		var unrecognizedTools []string
 		for _, name := range b.additionalTools {
 			// Always include the original name - this handles the case where
 			// the tool exists but is controlled by a feature flag that's OFF.
@@ -157,8 +165,12 @@ 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)
 			}
 		}
+		r.unrecognizedTools = unrecognizedTools
 	}
 
 	return r

pkg/inventory/registry.go

@@ -58,6 +58,8 @@ type Inventory struct {
 	filters []ToolFilter
 	// unrecognizedToolsets holds toolset IDs that were requested but don't match any registered toolsets
 	unrecognizedToolsets []string
+	// unrecognizedTools holds tool names that were requested via WithTools but don't exist
+	unrecognizedTools []string
 }
 
 // UnrecognizedToolsets returns toolset IDs that were passed to WithToolsets but don't
@@ -66,6 +68,13 @@ func (r *Inventory) UnrecognizedToolsets() []string {
 	return r.unrecognizedToolsets
 }
 
+// UnrecognizedTools returns tool names that were passed to WithTools but don't
+// match any registered tools (and are not deprecated aliases). This is used to
+// error out when invalid tool names are specified.
+func (r *Inventory) UnrecognizedTools() []string {
+	return r.unrecognizedTools
+}
+
 // MCP method constants for use with ForMCPRequest.
 const (
 	MCPMethodInitialize             = "initialize"
@@ -112,6 +121,7 @@ func (r *Inventory) ForMCPRequest(method string, itemName string) *Inventory {
 		featureChecker:       r.featureChecker,
 		filters:              r.filters, // shared, not modified
 		unrecognizedToolsets: r.unrecognizedToolsets,
+		unrecognizedTools:    r.unrecognizedTools,
 	}
 
 	// Helper to clear all item types

pkg/inventory/registry_test.go

@@ -270,6 +270,77 @@ func TestUnrecognizedToolsets(t *testing.T) {
 	}
 }
 
+func TestUnrecognizedTools(t *testing.T) {
+	tools := []ServerTool{
+		mockTool("tool1", "toolset1", true),
+		mockTool("tool2", "toolset2", true),
+	}
+
+	deprecatedAliases := map[string]string{
+		"old_tool": "tool1",
+	}
+
+	tests := []struct {
+		name                 string
+		withTools            []string
+		expectedUnrecognized []string
+	}{
+		{
+			name:                 "all valid",
+			withTools:            []string{"tool1", "tool2"},
+			expectedUnrecognized: nil,
+		},
+		{
+			name:                 "one invalid",
+			withTools:            []string{"tool1", "blabla"},
+			expectedUnrecognized: []string{"blabla"},
+		},
+		{
+			name:                 "multiple invalid",
+			withTools:            []string{"invalid1", "tool1", "invalid2"},
+			expectedUnrecognized: []string{"invalid1", "invalid2"},
+		},
+		{
+			name:                 "deprecated alias is valid",
+			withTools:            []string{"old_tool"},
+			expectedUnrecognized: nil,
+		},
+		{
+			name:                 "mixed valid and deprecated alias",
+			withTools:            []string{"old_tool", "tool2"},
+			expectedUnrecognized: nil,
+		},
+		{
+			name:                 "empty input",
+			withTools:            []string{},
+			expectedUnrecognized: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			inv := NewBuilder().
+				SetTools(tools).
+				WithDeprecatedAliases(deprecatedAliases).
+				WithToolsets([]string{"all"}).
+				WithTools(tt.withTools).
+				Build()
+			unrecognized := inv.UnrecognizedTools()
+
+			if len(unrecognized) != len(tt.expectedUnrecognized) {
+				t.Fatalf("Expected %d unrecognized, got %d: %v",
+					len(tt.expectedUnrecognized), len(unrecognized), unrecognized)
+			}
+
+			for i, expected := range tt.expectedUnrecognized {
+				if unrecognized[i] != expected {
+					t.Errorf("Expected unrecognized[%d] = %q, got %q", i, expected, unrecognized[i])
+				}
+			}
+		})
+	}
+}
+
 func TestWithTools(t *testing.T) {
 	tools := []ServerTool{
 		mockTool("tool1", "toolset1", true),