diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 7ad71532f..9de0682f3 100644 --- a/internal/ghmcp/server.go +++ b/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 } diff --git a/pkg/github/instructions.go b/pkg/github/instructions.go new file mode 100644 index 000000000..3f72e707b --- /dev/null +++ b/pkg/github/instructions.go @@ -0,0 +1,60 @@ +package github + +import ( + "os" + "slices" + "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 slices.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 tools to interact with GitHub platform. + +Tool selection guidance: + 1. 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. + 2. 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. Use pagination whenever possible with batches of 5-10 items. + 2. Use minimal_output parameter set to true if the full information is not needed to accomplish a task.` + + 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 "## Pull Requests\n\nPR 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 "## Issues\n\nCheck '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 "discussions": + return "## Discussions\n\nUse 'list_discussion_categories' to understand available categories before creating discussions. Filter by category for better organization." + default: + return "" + } +} + diff --git a/pkg/github/instructions_test.go b/pkg/github/instructions_test.go new file mode 100644 index 000000000..8450dc1a1 --- /dev/null +++ b/pkg/github/instructions_test.go @@ -0,0 +1,166 @@ +package github + +import ( + "os" + "testing" +) + +func TestGenerateInstructions(t *testing.T) { + tests := []struct { + name string + enabledToolsets []string + expectedEmpty bool + }{ + { + name: "empty toolsets", + enabledToolsets: []string{}, + expectedEmpty: false, + }, + { + name: "only context toolset", + enabledToolsets: []string{"context"}, + expectedEmpty: false, + }, + { + name: "pull requests toolset", + enabledToolsets: []string{"pull_requests"}, + expectedEmpty: false, + }, + { + name: "issues toolset", + enabledToolsets: []string{"issues"}, + expectedEmpty: false, + }, + { + name: "discussions toolset", + enabledToolsets: []string{"discussions"}, + expectedEmpty: false, + }, + { + name: "multiple toolsets (context + pull_requests)", + enabledToolsets: []string{"context", "pull_requests"}, + expectedEmpty: false, + }, + { + name: "multiple toolsets (issues + pull_requests)", + enabledToolsets: []string{"issues", "pull_requests"}, + expectedEmpty: false, + }, + } + + 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) + } + } else { + if result == "" { + t.Errorf("Expected non-empty instructions but got empty result") + } + } + }) + } +} + +func TestGenerateInstructionsWithDisableFlag(t *testing.T) { + tests := []struct { + name string + disableEnvValue string + enabledToolsets []string + expectedEmpty bool + }{ + { + 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, + }, + { + name: "DISABLE_INSTRUCTIONS unset returns normal instructions", + disableEnvValue: "", + enabledToolsets: []string{"issues"}, + expectedEmpty: false, + }, + } + + 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) + } + } else { + if result == "" { + t.Errorf("Expected non-empty instructions but got empty result") + } + } + }) + } +} + +func TestGetToolsetInstructions(t *testing.T) { + tests := []struct { + toolset string + expectedEmpty bool + }{ + { + toolset: "pull_requests", + expectedEmpty: false, + }, + { + toolset: "issues", + expectedEmpty: false, + }, + { + toolset: "discussions", + expectedEmpty: false, + }, + { + toolset: "nonexistent", + expectedEmpty: true, + }, + } + + for _, tt := range tests { + t.Run(tt.toolset, func(t *testing.T) { + result := getToolsetInstructions(tt.toolset) + if tt.expectedEmpty { + if result != "" { + t.Errorf("Expected empty result for toolset '%s', but got: %s", tt.toolset, result) + } + } else { + if result == "" { + t.Errorf("Expected non-empty result for toolset '%s', but got empty", tt.toolset) + } + } + }) + } +} \ No newline at end of file