feat: partition tools by product/feature

Author: SamMorrowDrums

cmd/github-mcp-server/main.go

@@ -9,8 +9,10 @@ import (
 	stdlog "log"
 	"os"
 	"os/signal"
+	"strings"
 	"syscall"
 
+	"github.com/github/github-mcp-server/pkg/features"
 	"github.com/github/github-mcp-server/pkg/github"
 	iolog "github.com/github/github-mcp-server/pkg/log"
 	"github.com/github/github-mcp-server/pkg/translations"
@@ -46,13 +48,20 @@ var (
 			if err != nil {
 				stdlog.Fatal("Failed to initialize logger:", err)
 			}
+			enabledFeatures := viper.GetStringSlice("features")
+			features, err := initFeatures(enabledFeatures)
+			if err != nil {
+				stdlog.Fatal("Failed to initialize features:", err)
+			}
+
 			logCommands := viper.GetBool("enable-command-logging")
 			cfg := runConfig{
 				readOnly:           readOnly,
 				logger:             logger,
 				logCommands:        logCommands,
 				exportTranslations: exportTranslations,
 				prettyPrintJSON:    prettyPrintJSON,
+				features:           features,
 			}
 			if err := runStdioServer(cfg); err != nil {
 				stdlog.Fatal("failed to run stdio server:", err)
@@ -61,10 +70,45 @@ var (
 	}
 )
 
+func initFeatures(passedFeatures []string) (*features.FeatureSet, error) {
+	// Create a new feature set
+	fs := features.NewFeatureSet()
+
+	// Define all available features with their default state (disabled)
+	fs.AddFeature("repos", "Repository related tools", false)
+	fs.AddFeature("issues", "Issues related tools", false)
+	fs.AddFeature("search", "Search related tools", false)
+	fs.AddFeature("pull_requests", "Pull request related tools", false)
+	fs.AddFeature("code_security", "Code security related tools", false)
+	fs.AddFeature("experiments", "Experimental features that are not considered stable yet", false)
+
+	// fs.AddFeature("actions", "GitHub Actions related tools", false)
+	// fs.AddFeature("projects", "GitHub Projects related tools", false)
+	// fs.AddFeature("secret_protection", "Secret protection related tools", false)
+	// fs.AddFeature("gists", "Gist related tools", false)
+
+	// Env gets precedence over command line flags
+	if envFeats := os.Getenv("GITHUB_FEATURES"); envFeats != "" {
+		passedFeatures = []string{}
+		// Split envFeats by comma, trim whitespace, and add to the slice
+		for _, feature := range strings.Split(envFeats, ",") {
+			passedFeatures = append(passedFeatures, strings.TrimSpace(feature))
+		}
+	}
+
+	// Enable the requested features
+	if err := fs.EnableFeatures(passedFeatures); err != nil {
+		return nil, err
+	}
+
+	return fs, nil
+}
+
 func init() {
 	cobra.OnInitialize(initConfig)
 
 	// Add global flags that will be shared by all commands
+	rootCmd.PersistentFlags().StringSlice("features", []string{"repos", "issues", "pull_requests", "search"}, "A comma separated list of groups of tools to enable, defaults to issues/repos/search")
 	rootCmd.PersistentFlags().Bool("read-only", false, "Restrict the server to read-only operations")
 	rootCmd.PersistentFlags().String("log-file", "", "Path to log file")
 	rootCmd.PersistentFlags().Bool("enable-command-logging", false, "When enabled, the server will log all command requests and responses to the log file")
@@ -73,6 +117,7 @@ func init() {
 	rootCmd.PersistentFlags().Bool("pretty-print-json", false, "Pretty print JSON output")
 
 	// Bind flag to viper
+	_ = viper.BindPFlag("features", rootCmd.PersistentFlags().Lookup("features"))
 	_ = viper.BindPFlag("read-only", rootCmd.PersistentFlags().Lookup("read-only"))
 	_ = viper.BindPFlag("log-file", rootCmd.PersistentFlags().Lookup("log-file"))
 	_ = viper.BindPFlag("enable-command-logging", rootCmd.PersistentFlags().Lookup("enable-command-logging"))
@@ -113,6 +158,7 @@ type runConfig struct {
 	logCommands        bool
 	exportTranslations bool
 	prettyPrintJSON    bool
+	features           *features.FeatureSet
 }
 
 // JSONPrettyPrintWriter is a Writer that pretty prints input to indented JSON
@@ -158,7 +204,7 @@ func runStdioServer(cfg runConfig) error {
 	t, dumpTranslations := translations.TranslationHelper()
 
 	// Create
-	ghServer := github.NewServer(ghClient, version, cfg.readOnly, t)
+	ghServer := github.NewServer(ghClient, cfg.features, version, cfg.readOnly, t)
 	stdioServer := server.NewStdioServer(ghServer)
 
 	stdLogger := stdlog.New(cfg.logger.Writer(), "stdioserver", 0)

pkg/features/features.go

@@ -0,0 +1,68 @@
+package features
+
+import "fmt"
+
+type Feature struct {
+	Name        string
+	Description string
+	Enabled     bool
+}
+
+type FeatureSet struct {
+	Features     map[string]Feature
+	everythingOn bool
+}
+
+func NewFeatureSet() *FeatureSet {
+	return &FeatureSet{
+		Features:     make(map[string]Feature),
+		everythingOn: false,
+	}
+}
+
+func (fs *FeatureSet) AddFeature(name string, description string, enabled bool) {
+	fs.Features[name] = Feature{
+		Name:        name,
+		Description: description,
+		Enabled:     enabled,
+	}
+}
+
+func (fs *FeatureSet) IsEnabled(name string) bool {
+	// If everythingOn is true, all features are enabled
+	if fs.everythingOn {
+		return true
+	}
+
+	feature, exists := fs.Features[name]
+	if !exists {
+		return false
+	}
+	return feature.Enabled
+}
+
+func (fs *FeatureSet) EnableFeatures(names []string) error {
+	for _, name := range names {
+		err := fs.EnableFeature(name)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (fs *FeatureSet) EnableFeature(name string) error {
+	// Special case for "everything"
+	if name == "everything" {
+		fs.everythingOn = true
+		return nil
+	}
+
+	feature, exists := fs.Features[name]
+	if !exists {
+		return fmt.Errorf("feature %s does not exist", name)
+	}
+	feature.Enabled = true
+	fs.Features[name] = feature
+	return nil
+}

pkg/features/features_test.go

@@ -0,0 +1,219 @@
+package features
+
+import (
+	"testing"
+)
+
+func TestNewFeatureSet(t *testing.T) {
+	fs := NewFeatureSet()
+	if fs == nil {
+		t.Fatal("Expected NewFeatureSet to return a non-nil pointer")
+	}
+	if fs.Features == nil {
+		t.Fatal("Expected Features map to be initialized")
+	}
+	if len(fs.Features) != 0 {
+		t.Fatalf("Expected Features map to be empty, got %d items", len(fs.Features))
+	}
+	if fs.everythingOn {
+		t.Fatal("Expected everythingOn to be initialized as false")
+	}
+}
+
+func TestAddFeature(t *testing.T) {
+	fs := NewFeatureSet()
+
+	// Test adding a feature
+	fs.AddFeature("test-feature", "A test feature", true)
+
+	// Verify feature was added correctly
+	if len(fs.Features) != 1 {
+		t.Errorf("Expected 1 feature, got %d", len(fs.Features))
+	}
+
+	feature, exists := fs.Features["test-feature"]
+	if !exists {
+		t.Fatal("Feature was not added to the map")
+	}
+
+	if feature.Name != "test-feature" {
+		t.Errorf("Expected feature name to be 'test-feature', got '%s'", feature.Name)
+	}
+
+	if feature.Description != "A test feature" {
+		t.Errorf("Expected feature description to be 'A test feature', got '%s'", feature.Description)
+	}
+
+	if !feature.Enabled {
+		t.Error("Expected feature to be enabled")
+	}
+
+	// Test adding another feature
+	fs.AddFeature("another-feature", "Another test feature", false)
+
+	if len(fs.Features) != 2 {
+		t.Errorf("Expected 2 features, got %d", len(fs.Features))
+	}
+
+	// Test overriding existing feature
+	fs.AddFeature("test-feature", "Updated description", false)
+
+	feature = fs.Features["test-feature"]
+	if feature.Description != "Updated description" {
+		t.Errorf("Expected feature description to be updated to 'Updated description', got '%s'", feature.Description)
+	}
+
+	if feature.Enabled {
+		t.Error("Expected feature to be disabled after update")
+	}
+}
+
+func TestIsEnabled(t *testing.T) {
+	fs := NewFeatureSet()
+
+	// Test with non-existent feature
+	if fs.IsEnabled("non-existent") {
+		t.Error("Expected IsEnabled to return false for non-existent feature")
+	}
+
+	// Test with disabled feature
+	fs.AddFeature("disabled-feature", "A disabled feature", false)
+	if fs.IsEnabled("disabled-feature") {
+		t.Error("Expected IsEnabled to return false for disabled feature")
+	}
+
+	// Test with enabled feature
+	fs.AddFeature("enabled-feature", "An enabled feature", true)
+	if !fs.IsEnabled("enabled-feature") {
+		t.Error("Expected IsEnabled to return true for enabled feature")
+	}
+}
+
+func TestEnableFeature(t *testing.T) {
+	fs := NewFeatureSet()
+
+	// Test enabling non-existent feature
+	err := fs.EnableFeature("non-existent")
+	if err == nil {
+		t.Error("Expected error when enabling non-existent feature")
+	}
+
+	// Test enabling feature
+	fs.AddFeature("test-feature", "A test feature", false)
+
+	if fs.IsEnabled("test-feature") {
+		t.Error("Expected feature to be disabled initially")
+	}
+
+	err = fs.EnableFeature("test-feature")
+	if err != nil {
+		t.Errorf("Expected no error when enabling feature, got: %v", err)
+	}
+
+	if !fs.IsEnabled("test-feature") {
+		t.Error("Expected feature to be enabled after EnableFeature call")
+	}
+
+	// Test enabling already enabled feature
+	err = fs.EnableFeature("test-feature")
+	if err != nil {
+		t.Errorf("Expected no error when enabling already enabled feature, got: %v", err)
+	}
+}
+
+func TestEnableFeatures(t *testing.T) {
+	fs := NewFeatureSet()
+
+	// Prepare features
+	fs.AddFeature("feature1", "Feature 1", false)
+	fs.AddFeature("feature2", "Feature 2", false)
+
+	// Test enabling multiple features
+	err := fs.EnableFeatures([]string{"feature1", "feature2"})
+	if err != nil {
+		t.Errorf("Expected no error when enabling features, got: %v", err)
+	}
+
+	if !fs.IsEnabled("feature1") {
+		t.Error("Expected feature1 to be enabled")
+	}
+
+	if !fs.IsEnabled("feature2") {
+		t.Error("Expected feature2 to be enabled")
+	}
+
+	// Test with non-existent feature in the list
+	err = fs.EnableFeatures([]string{"feature1", "non-existent"})
+	if err == nil {
+		t.Error("Expected error when enabling list with non-existent feature")
+	}
+
+	// Test with empty list
+	err = fs.EnableFeatures([]string{})
+	if err != nil {
+		t.Errorf("Expected no error with empty feature list, got: %v", err)
+	}
+
+	// Test enabling everything through EnableFeatures
+	fs = NewFeatureSet()
+	err = fs.EnableFeatures([]string{"everything"})
+	if err != nil {
+		t.Errorf("Expected no error when enabling 'everything', got: %v", err)
+	}
+
+	if !fs.everythingOn {
+		t.Error("Expected everythingOn to be true after enabling 'everything' via EnableFeatures")
+	}
+}
+
+func TestEnableEverything(t *testing.T) {
+	fs := NewFeatureSet()
+
+	// Add a disabled feature
+	fs.AddFeature("test-feature", "A test feature", false)
+
+	// Verify it's disabled
+	if fs.IsEnabled("test-feature") {
+		t.Error("Expected feature to be disabled initially")
+	}
+
+	// Enable "everything"
+	err := fs.EnableFeature("everything")
+	if err != nil {
+		t.Errorf("Expected no error when enabling 'everything', got: %v", err)
+	}
+
+	// Verify everythingOn was set
+	if !fs.everythingOn {
+		t.Error("Expected everythingOn to be true after enabling 'everything'")
+	}
+
+	// Verify the previously disabled feature is now enabled
+	if !fs.IsEnabled("test-feature") {
+		t.Error("Expected feature to be enabled when everythingOn is true")
+	}
+
+	// Verify a non-existent feature is also enabled
+	if !fs.IsEnabled("non-existent") {
+		t.Error("Expected non-existent feature to be enabled when everythingOn is true")
+	}
+}
+
+func TestIsEnabledWithEverythingOn(t *testing.T) {
+	fs := NewFeatureSet()
+
+	// Enable "everything"
+	err := fs.EnableFeature("everything")
+	if err != nil {
+		t.Errorf("Expected no error when enabling 'everything', got: %v", err)
+	}
+
+	// Test that any feature name returns true with IsEnabled
+	if !fs.IsEnabled("some-feature") {
+		t.Error("Expected IsEnabled to return true for any feature when everythingOn is true")
+	}
+
+	if !fs.IsEnabled("another-feature") {
+		t.Error("Expected IsEnabled to return true for any feature when everythingOn is true")
+	}
+}