Fix default toolsets behavior when not in dynamic mode

When no toolsets are specified and dynamic mode is disabled, the server
should use the default toolsets. The bug was introduced when adding
dynamic toolsets support:

1. CleanToolsets(nil) was converting nil to empty slice
2. Empty slice passed to WithToolsets means 'no toolsets'
3. This resulted in zero tools being registered

Fix: Preserve nil for non-dynamic mode (nil = use defaults in WithToolsets)
and only set empty slice when dynamic mode is enabled without explicit
toolsets.

Author: SamMorrowDrums

cmd/github-mcp-server/main.go

@@ -41,10 +41,16 @@ var (
 			// it's because viper doesn't handle comma-separated values correctly for env
 			// vars when using GetStringSlice.
 			// https://github.com/spf13/viper/issues/380
+			//
+			// Additionally, viper.UnmarshalKey returns an empty slice even when the flag
+			// is not set, but we need nil to indicate "use defaults". So we check IsSet first.
 			var enabledToolsets []string
-			if err := viper.UnmarshalKey("toolsets", &enabledToolsets); err != nil {
-				return fmt.Errorf("failed to unmarshal toolsets: %w", err)
+			if viper.IsSet("toolsets") {
+				if err := viper.UnmarshalKey("toolsets", &enabledToolsets); err != nil {
+					return fmt.Errorf("failed to unmarshal toolsets: %w", err)
+				}
 			}
+			// else: enabledToolsets stays nil, meaning "use defaults"
 
 			// Parse tools (similar to toolsets)
 			var enabledTools []string

internal/ghmcp/server.go

@@ -103,14 +103,24 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
 		repoAccessCache = lockdown.GetInstance(gqlClient, repoAccessOpts...)
 	}
 
-	enabledToolsets := cfg.EnabledToolsets
-
-	// Clean up the passed toolsets (removes duplicates, whitespace)
-	enabledToolsets, invalidToolsets := github.CleanToolsets(enabledToolsets)
-
-	if len(invalidToolsets) > 0 {
-		fmt.Fprintf(os.Stderr, "Invalid toolsets ignored: %s\n", strings.Join(invalidToolsets, ", "))
+	// Determine enabled toolsets based on configuration:
+	// - nil means "use defaults" (unless dynamic mode without explicit toolsets)
+	// - empty slice means "no toolsets" (for dynamic mode to enable on demand)
+	// - explicit list means "use these toolsets"
+	var enabledToolsets []string
+	if cfg.EnabledToolsets != nil {
+		// Clean up explicitly passed toolsets (removes duplicates, whitespace)
+		var invalidToolsets []string
+		enabledToolsets, invalidToolsets = github.CleanToolsets(cfg.EnabledToolsets)
+		if len(invalidToolsets) > 0 {
+			fmt.Fprintf(os.Stderr, "Invalid toolsets ignored: %s\n", strings.Join(invalidToolsets, ", "))
+		}
+	} else if cfg.DynamicToolsets {
+		// Dynamic mode with no toolsets specified: start with no toolsets enabled
+		// so users can enable them on demand via the dynamic tools
+		enabledToolsets = []string{}
 	}
+	// else: enabledToolsets stays nil, which means "use defaults" in WithToolsets
 
 	// Generate instructions based on enabled toolsets
 	instructions := github.GenerateInstructions(enabledToolsets)
@@ -162,14 +172,6 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
 	// Clean tool names (WithTools will resolve any deprecated aliases)
 	enabledTools := github.CleanTools(cfg.EnabledTools)
 
-	// For dynamic toolsets mode:
-	// - If toolsets are explicitly provided (including "default"), honor them
-	// - If no toolsets are specified (nil), start with no toolsets enabled (empty slice)
-	//   so users can enable them on demand via the dynamic tools
-	if cfg.DynamicToolsets && cfg.EnabledToolsets == nil {
-		enabledToolsets = []string{}
-	}
-
 	// Apply filters based on configuration
 	// - WithReadOnly: filters out write tools when true
 	// - WithToolsets: nil=defaults, empty=none, handles "all"/"default" keywords