Add instruction generation for enabled toolsets and corresponding tests

Author: olaservo

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,16 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
 		}
 	}
 
+	// Generate instructions based on enabled toolsets
+	instructions := github.GenerateInstructions(enabledToolsets)
+	var serverOpts []server.ServerOption
+	if instructions != "" {
+		serverOpts = append(serverOpts, server.WithInstructions(instructions))
+	}
+	serverOpts = append(serverOpts, server.WithHooks(hooks))
+
+	ghServer := github.NewServer(cfg.Version, serverOpts...)
+
 	getClient := func(_ context.Context) (*gogithub.Client, error) {
 		return restClient, nil // closing over client
 	}

pkg/github/instructions.go

@@ -0,0 +1,88 @@
+package github
+
+import "strings"
+
+// GenerateInstructions creates server instructions based on enabled toolsets
+func GenerateInstructions(enabledToolsets []string) string {
+	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)
+		}
+	}
+	
+	// Cross-toolset conditional instructions
+	if contains(enabledToolsets, "pull_requests") && contains(enabledToolsets, "context") {
+		instructions = append(instructions, "For team workflows: Use 'get_teams' and 'get_team_members' before assigning PR reviewers.")
+	}
+	
+	if hasAnySecurityToolset(enabledToolsets) {
+		instructions = append(instructions, "Security alert priority: secret_scanning → dependabot → code_scanning alerts.")
+	}
+	
+	if contains(enabledToolsets, "issues") && contains(enabledToolsets, "pull_requests") {
+		instructions = append(instructions, "Link issues to PRs using 'closes #123' or 'fixes #123' in PR descriptions.")
+	}
+	
+	if contains(enabledToolsets, "repos") && contains(enabledToolsets, "actions") {
+		instructions = append(instructions, "For repository operations: Check workflow status before major changes using 'list_workflow_runs'.")
+	}
+	
+	// Only add base instruction if we have specific instructions
+	if len(instructions) > 0 {
+		baseInstruction := "GitHub MCP Server provides GitHub API tools. Common parameters: 'owner' (repo owner), 'repo' (repo name), 'page'/'perPage' (pagination)."
+		instructions = append([]string{baseInstruction}, instructions...)
+	}
+	
+	return strings.Join(instructions, " ")
+}
+
+// getToolsetInstructions returns specific instructions for individual toolsets
+func getToolsetInstructions(toolset string) string {
+	switch toolset {
+	case "pull_requests":
+		return "PR review workflow: Use 'create_pending_pull_request_review' → 'add_comment_to_pending_review' → 'submit_pending_pull_request_review' for complex reviews with line-specific comments."
+	case "actions":
+		return "CI/CD debugging: Use 'get_job_logs' with 'failed_only=true' and 'return_content=true' for immediate log analysis. Use 'rerun_failed_jobs' instead of 'rerun_workflow_run' to save resources."
+	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 "repos":
+		return "File operations: Use 'get_file_contents' to check if file exists before 'create_or_update_file'. Always specify 'sha' parameter when updating existing files. Use 'push_files' for multiple file operations in single commit."
+	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 "gists":
+		return "Gists: Use 'list_gists' with 'since' parameter to find recent gists. Specify 'public=false' for private gists in 'create_gist'."
+	case "discussions":
+		return "Discussions: Use 'list_discussion_categories' to understand available categories before creating discussions. Filter by category for better organization."
+	default:
+		return ""
+	}
+}
+
+// hasAnySecurityToolset checks if any security-related toolsets are enabled
+func hasAnySecurityToolset(toolsets []string) bool {
+	securityToolsets := []string{"code_security", "secret_protection", "dependabot"}
+	for _, security := range securityToolsets {
+		if contains(toolsets, security) {
+			return true
+		}
+	}
+	return false
+}
+
+// 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,181 @@
+package github
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestGenerateInstructions(t *testing.T) {
+	tests := []struct {
+		name             string
+		enabledToolsets  []string
+		expectedContains []string
+		expectedEmpty    bool
+	}{
+		{
+			name:            "empty toolsets",
+			enabledToolsets: []string{},
+			expectedEmpty:   true,
+		},
+		{
+			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:            "actions toolset",
+			enabledToolsets: []string{"actions"},
+			expectedContains: []string{
+				"get_job_logs",
+				"failed_only=true",
+				"rerun_failed_jobs",
+			},
+		},
+		{
+			name:            "security toolsets",
+			enabledToolsets: []string{"code_security", "secret_protection", "dependabot"},
+			expectedContains: []string{
+				"Security alert priority",
+				"secret_scanning",
+				"dependabot",
+				"code_scanning",
+			},
+		},
+		{
+			name:            "cross-toolset instructions",
+			enabledToolsets: []string{"context", "pull_requests"},
+			expectedContains: []string{
+				"get_me",
+				"get_teams",
+				"get_team_members",
+			},
+		},
+		{
+			name:            "issues and pull_requests combination",
+			enabledToolsets: []string{"issues", "pull_requests"},
+			expectedContains: []string{
+				"closes #123",
+				"fixes #123",
+				"search_issues",
+				"list_issue_types",
+			},
+		},
+	}
+
+	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 TestGetToolsetInstructions(t *testing.T) {
+	tests := []struct {
+		toolset  string
+		expected string
+	}{
+		{
+			toolset:  "pull_requests",
+			expected: "create_pending_pull_request_review",
+		},
+		{
+			toolset:  "actions",
+			expected: "get_job_logs",
+		},
+		{
+			toolset:  "issues",
+			expected: "list_issue_types",
+		},
+		{
+			toolset:  "repos",
+			expected: "get_file_contents",
+		},
+		{
+			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)
+				}
+			}
+		})
+	}
+}
+
+func TestHasAnySecurityToolset(t *testing.T) {
+	tests := []struct {
+		name     string
+		toolsets []string
+		expected bool
+	}{
+		{
+			name:     "no security toolsets",
+			toolsets: []string{"repos", "issues"},
+			expected: false,
+		},
+		{
+			name:     "has code_security",
+			toolsets: []string{"repos", "code_security"},
+			expected: true,
+		},
+		{
+			name:     "has secret_protection",
+			toolsets: []string{"secret_protection", "issues"},
+			expected: true,
+		},
+		{
+			name:     "has dependabot",
+			toolsets: []string{"dependabot"},
+			expected: true,
+		},
+		{
+			name:     "has all security toolsets",
+			toolsets: []string{"code_security", "secret_protection", "dependabot"},
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := hasAnySecurityToolset(tt.toolsets)
+			if result != tt.expected {
+				t.Errorf("Expected %v for toolsets %v, but got %v", tt.expected, tt.toolsets, result)
+			}
+		})
+	}
+}