fix: filter skill allowedTools against registered tools at startup

Skills now only include tools that are actually registered with the
MCP server, based on the active toolset configuration. Skills with
no available tools are skipped entirely. This prevents skills from
referencing non-existent tools (e.g., gist tools when the gists
toolset isn't enabled).

RegisterSkillResources now accepts a map of available tool names
built from the inventory at startup time.

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

Author: SamMorrowDrums

pkg/github/server.go

@@ -124,8 +124,14 @@ func NewMCPServer(ctx context.Context, cfg *MCPServerConfig, deps ToolDependenci
 		registerDynamicTools(ghServer, inv, deps, cfg.Translator)
 	}
 
-	// Register skill resources for MCP clients that support skills-based discovery
-	RegisterSkillResources(ghServer)
+	// 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{})
+	for _, tool := range inv.AllTools() {
+		availableTools[tool.Tool.Name] = struct{}{}
+	}
+	RegisterSkillResources(ghServer, availableTools)
 
 	return ghServer, nil
 }

pkg/github/skill_resources.go

@@ -508,8 +508,18 @@ 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.
-func RegisterSkillResources(s *mcp.Server) {
+// 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{}) {
 	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)
 
@@ -536,3 +546,18 @@ func RegisterSkillResources(s *mcp.Server) {
 		)
 	}
 }
+
+// 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,10 +75,69 @@ func TestRegisterSkillResources(t *testing.T) {
 		Version: "0.0.1",
 	}, nil)
 
-	// Should not panic
-	RegisterSkillResources(server)
+	// Should not panic with nil (no filtering)
+	RegisterSkillResources(server, nil)
 
 	// 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.
+}