fix: register all tools from all toolsets when skills are enabled

When skills handle progressive disclosure, the server should expose
all tools via tools/list regardless of toolset configuration. The
client decides tool visibility through skill allowedTools, not the
server.

This replaces the previous filtering approach — now inv.AllTools()
registers every tool with the MCP server, and skills reference the
full tool surface.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: SamMorrowDrums

pkg/github/server.go

@@ -125,13 +125,13 @@ func NewMCPServer(ctx context.Context, cfg *MCPServerConfig, deps ToolDependenci
 	}
 
 	// Register skill resources for MCP clients that support skills-based discovery.
-	// Filter allowedTools against actually-registered tools so skills only reference
-	// tools that exist for the current toolset configuration.
-	availableTools := make(map[string]struct{})
+	// When skills are present, register ALL tools from all toolsets so that skills
+	// can progressively reveal the full tool surface. The skills handle disclosure;
+	// the server exposes everything.
 	for _, tool := range inv.AllTools() {
-		availableTools[tool.Tool.Name] = struct{}{}
+		tool.RegisterFunc(ghServer, deps)
 	}
-	RegisterSkillResources(ghServer, availableTools)
+	RegisterSkillResources(ghServer)
 
 	return ghServer, nil
 }

pkg/github/skill_resources.go

@@ -508,18 +508,8 @@ func buildSkillContent(skill skillDefinition) string {
 // RegisterSkillResources registers all skill resources with the MCP server.
 // Each skill is a static resource with a skill:// URI that can be discovered
 // by MCP clients supporting the skills pattern.
-// The availableTools set filters each skill's allowedTools to only include
-// tools that are actually registered, ensuring skills work correctly
-// regardless of which toolsets are enabled.
-func RegisterSkillResources(s *mcp.Server, availableTools map[string]struct{}) {
+func RegisterSkillResources(s *mcp.Server) {
 	for _, skill := range allSkills() {
-		// Filter allowedTools to only include tools that are actually registered
-		filtered := filterAvailableTools(skill.allowedTools, availableTools)
-		if len(filtered) == 0 {
-			continue // Skip skills with no available tools
-		}
-		skill.allowedTools = filtered
-
 		content := buildSkillContent(skill)
 		uri := fmt.Sprintf("skill://github/%s/SKILL.md", skill.name)
 
@@ -546,18 +536,3 @@ func RegisterSkillResources(s *mcp.Server, availableTools map[string]struct{}) {
 		)
 	}
 }
-
-// filterAvailableTools returns only the tool names that exist in the available set.
-// If availableTools is nil, all tools are returned (no filtering).
-func filterAvailableTools(tools []string, availableTools map[string]struct{}) []string {
-	if availableTools == nil {
-		return tools
-	}
-	var filtered []string
-	for _, t := range tools {
-		if _, ok := availableTools[t]; ok {
-			filtered = append(filtered, t)
-		}
-	}
-	return filtered
-}

pkg/github/skill_resources_test.go

@@ -75,69 +75,10 @@ func TestRegisterSkillResources(t *testing.T) {
 		Version: "0.0.1",
 	}, nil)
 
-	// Should not panic with nil (no filtering)
-	RegisterSkillResources(server, nil)
+	// Should not panic
+	RegisterSkillResources(server)
 
 	// Verify the expected number of skills were registered by counting definitions
 	skills := allSkills()
 	assert.Equal(t, 27, len(skills), "expected 27 workflow-oriented skills")
 }
-
-func TestFilterAvailableTools(t *testing.T) {
-	tests := []struct {
-		name      string
-		tools     []string
-		available map[string]struct{}
-		expected  []string
-	}{
-		{
-			name:      "nil available returns all tools",
-			tools:     []string{"a", "b", "c"},
-			available: nil,
-			expected:  []string{"a", "b", "c"},
-		},
-		{
-			name:      "filters to only available tools",
-			tools:     []string{"a", "b", "c"},
-			available: map[string]struct{}{"a": {}, "c": {}},
-			expected:  []string{"a", "c"},
-		},
-		{
-			name:      "returns nil when no tools match",
-			tools:     []string{"a", "b"},
-			available: map[string]struct{}{"x": {}},
-			expected:  nil,
-		},
-		{
-			name:      "empty available set filters everything",
-			tools:     []string{"a", "b"},
-			available: map[string]struct{}{},
-			expected:  nil,
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			result := filterAvailableTools(tc.tools, tc.available)
-			assert.Equal(t, tc.expected, result)
-		})
-	}
-}
-
-func TestRegisterSkillResourcesFiltering(t *testing.T) {
-	t.Parallel()
-	// Only make get_me available — skills that have no overlap should be skipped
-	available := map[string]struct{}{"get_me": {}}
-
-	server := mcp.NewServer(&mcp.Implementation{
-		Name:    "test-server",
-		Version: "0.0.1",
-	}, nil)
-
-	RegisterSkillResources(server, available)
-
-	// get-context skill includes get_me, so it should be registered.
-	// Skills with zero available tools should be skipped entirely.
-	// We can't easily count resources on mcp.Server, but we verify no panic
-	// and the logic is correct via TestFilterAvailableTools above.
-}