Add server instructions based on toolsets

internal/ghmcp/server.go

@@ -105,8 +105,6 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
 		},
 	}
 
-	ghServer := github.NewServer(cfg.Version, server.WithHooks(hooks))
-
 	enabledToolsets := cfg.EnabledToolsets
 	if cfg.DynamicToolsets {
 		// filter "all" from the enabled toolsets
@@ -118,6 +116,14 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
 		}
 	}
 
+	// Generate instructions based on enabled toolsets
+	instructions := github.GenerateInstructions(enabledToolsets)
+
+	ghServer := github.NewServer(cfg.Version, 
+		server.WithInstructions(instructions),
+		server.WithHooks(hooks),
+	)
+
 	getClient := func(_ context.Context) (*gogithub.Client, error) {
 		return restClient, nil // closing over client
 	}

pkg/github/instructions.go

@@ -0,0 +1,62 @@
+package github
+
+import (
+	"os"
+	"strings"
+)
+
+// GenerateInstructions creates server instructions based on enabled toolsets
+func GenerateInstructions(enabledToolsets []string) string {
+	// For testing - add a flag to disable instructions
+	if os.Getenv("DISABLE_INSTRUCTIONS") == "true" {
+		return "" // Baseline mode
+	}
+
+	var instructions []string
+
+	// Core instruction - always included if context toolset enabled
+	if contains(enabledToolsets, "context") {
+		instructions = append(instructions, "Always call 'get_me' first to understand current user permissions and context.")
+	}
+
+	// Individual toolset instructions
+	for _, toolset := range enabledToolsets {
+		if inst := getToolsetInstructions(toolset); inst != "" {
+			instructions = append(instructions, inst)
+		}
+	}
+
+	// Base instruction with context management
+	baseInstruction := "The GitHub MCP Server provides GitHub API tools. Tool selection guidance: Use 'list_*' tools for broad, simple retrieval and pagination of all items of a type (e.g., all issues, all PRs, all branches) with basic filtering. Use 'search_*' tools for targeted queries with specific criteria, keywords, or complex filters (e.g., issues with certain text, PRs by author, code containing functions). Context management: 1) GitHub API responses can overflow context windows, 2) Process large datasets in batches of 5-10 items, 3) For summarization tasks, fetch minimal data first, then drill down into specifics."
+
+	allInstructions := []string{baseInstruction}
+	allInstructions = append(allInstructions, instructions...)
+
+	return strings.Join(allInstructions, " ")
+}
+
+// getToolsetInstructions returns specific instructions for individual toolsets
+func getToolsetInstructions(toolset string) string {
+	switch toolset {
+	case "pull_requests":
+		return "PR review workflow: Always use 'create_pending_pull_request_review' → 'add_comment_to_pending_review' → 'submit_pending_pull_request_review' for complex reviews with line-specific comments."
+	case "issues":
+		return "Issue workflow: Check 'list_issue_types' first for organizations to use proper issue types. Use 'search_issues' before creating new issues to avoid duplicates. Always set 'state_reason' when closing issues."
+	case "notifications":
+		return "Notifications: Filter by 'participating' for issues/PRs you're involved in. Use 'mark_all_notifications_read' with repository filters to avoid marking unrelated notifications."
+	case "discussions":
+		return "Discussions: Use 'list_discussion_categories' to understand available categories before creating discussions. Filter by category for better organization."
+	default:
+		return ""
+	}
+}
+
+// contains checks if a slice contains a specific string
+func contains(slice []string, item string) bool {
+	for _, s := range slice {
+		if s == item {
+			return true
+		}
+	}
+	return false
+}

pkg/github/instructions_test.go

@@ -0,0 +1,222 @@
+package github
+
+import (
+	"os"
+	"strings"
+	"testing"
+)
+
+func TestGenerateInstructions(t *testing.T) {
+	tests := []struct {
+		name             string
+		enabledToolsets  []string
+		expectedContains []string
+		expectedEmpty    bool
+	}{
+		{
+			name:            "empty toolsets",
+			enabledToolsets: []string{},
+			expectedContains: []string{
+				"GitHub MCP Server provides GitHub API tools",
+				"Use 'list_*' tools for broad, simple retrieval",
+				"Use 'search_*' tools for targeted queries",
+				"context windows",
+			},
+		},
+		{
+			name:            "only context toolset",
+			enabledToolsets: []string{"context"},
+			expectedContains: []string{
+				"GitHub MCP Server provides GitHub API tools",
+				"Always call 'get_me' first",
+			},
+		},
+		{
+			name:            "pull requests toolset",
+			enabledToolsets: []string{"pull_requests"},
+			expectedContains: []string{
+				"create_pending_pull_request_review",
+				"add_comment_to_pending_review",
+				"submit_pending_pull_request_review",
+			},
+		},
+		{
+			name:            "issues toolset",
+			enabledToolsets: []string{"issues"},
+			expectedContains: []string{
+				"search_issues",
+				"list_issue_types",
+				"state_reason",
+			},
+		},
+		{
+			name:            "notifications toolset",
+			enabledToolsets: []string{"notifications"},
+			expectedContains: []string{
+				"participating",
+				"mark_all_notifications_read",
+				"repository filters",
+			},
+		},
+		{
+			name:            "discussions toolset",
+			enabledToolsets: []string{"discussions"},
+			expectedContains: []string{
+				"list_discussion_categories",
+				"Filter by category",
+			},
+		},
+		{
+			name:            "multiple toolsets (context + pull_requests)",
+			enabledToolsets: []string{"context", "pull_requests"},
+			expectedContains: []string{
+				"get_me",
+				"create_pending_pull_request_review",
+			},
+		},
+		{
+			name:            "multiple toolsets (issues + pull_requests)",
+			enabledToolsets: []string{"issues", "pull_requests"},
+			expectedContains: []string{
+				"search_issues",
+				"list_issue_types",
+				"create_pending_pull_request_review",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := GenerateInstructions(tt.enabledToolsets)
+
+			if tt.expectedEmpty {
+				if result != "" {
+					t.Errorf("Expected empty instructions but got: %s", result)
+				}
+				return
+			}
+
+			for _, expectedContent := range tt.expectedContains {
+				if !strings.Contains(result, expectedContent) {
+					t.Errorf("Expected instructions to contain '%s', but got: %s", expectedContent, result)
+				}
+			}
+		})
+	}
+}
+
+func TestGenerateInstructionsWithDisableFlag(t *testing.T) {
+	tests := []struct {
+		name              string
+		disableEnvValue   string
+		enabledToolsets   []string
+		expectedEmpty     bool
+		expectedContains  []string
+	}{
+		{
+			name:            "DISABLE_INSTRUCTIONS=true returns empty",
+			disableEnvValue: "true",
+			enabledToolsets: []string{"context", "issues", "pull_requests"},
+			expectedEmpty:   true,
+		},
+		{
+			name:            "DISABLE_INSTRUCTIONS=false returns normal instructions",
+			disableEnvValue: "false",
+			enabledToolsets: []string{"context"},
+			expectedEmpty:   false,
+			expectedContains: []string{
+				"GitHub MCP Server provides GitHub API tools",
+				"Always call 'get_me' first",
+			},
+		},
+		{
+			name:            "DISABLE_INSTRUCTIONS unset returns normal instructions",
+			disableEnvValue: "",
+			enabledToolsets: []string{"issues"},
+			expectedEmpty:   false,
+			expectedContains: []string{
+				"GitHub MCP Server provides GitHub API tools",
+				"search_issues",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Save original env value
+			originalValue := os.Getenv("DISABLE_INSTRUCTIONS")
+			defer func() {
+				if originalValue == "" {
+					os.Unsetenv("DISABLE_INSTRUCTIONS")
+				} else {
+					os.Setenv("DISABLE_INSTRUCTIONS", originalValue)
+				}
+			}()
+
+			// Set test env value
+			if tt.disableEnvValue == "" {
+				os.Unsetenv("DISABLE_INSTRUCTIONS")
+			} else {
+				os.Setenv("DISABLE_INSTRUCTIONS", tt.disableEnvValue)
+			}
+
+			result := GenerateInstructions(tt.enabledToolsets)
+
+			if tt.expectedEmpty {
+				if result != "" {
+					t.Errorf("Expected empty instructions but got: %s", result)
+				}
+				return
+			}
+
+			for _, expectedContent := range tt.expectedContains {
+				if !strings.Contains(result, expectedContent) {
+					t.Errorf("Expected instructions to contain '%s', but got: %s", expectedContent, result)
+				}
+			}
+		})
+	}
+}
+
+func TestGetToolsetInstructions(t *testing.T) {
+	tests := []struct {
+		toolset  string
+		expected string
+	}{
+		{
+			toolset:  "pull_requests",
+			expected: "create_pending_pull_request_review",
+		},
+		{
+			toolset:  "issues",
+			expected: "list_issue_types",
+		},
+		{
+			toolset:  "notifications",
+			expected: "participating",
+		},
+		{
+			toolset:  "discussions",
+			expected: "list_discussion_categories",
+		},
+		{
+			toolset:  "nonexistent",
+			expected: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.toolset, func(t *testing.T) {
+			result := getToolsetInstructions(tt.toolset)
+			if tt.expected == "" {
+				if result != "" {
+					t.Errorf("Expected empty result for toolset '%s', but got: %s", tt.toolset, result)
+				}
+			} else {
+				if !strings.Contains(result, tt.expected) {
+					t.Errorf("Expected instructions for '%s' to contain '%s', but got: %s", tt.toolset, tt.expected, result)
+				}
+			}
+		})
+	}
+}