diff --git a/.github/workflows/mcp-diff.yml b/.github/workflows/mcp-diff.yml index 9c9c34b3a..fb2a555bf 100644 --- a/.github/workflows/mcp-diff.yml +++ b/.github/workflows/mcp-diff.yml @@ -20,7 +20,7 @@ jobs: fetch-depth: 0 - name: Run MCP Server Diff - uses: SamMorrowDrums/mcp-server-diff@v2 + uses: SamMorrowDrums/mcp-server-diff@v2.2.0 with: setup_go: "true" install_command: go mod download @@ -35,8 +35,10 @@ jobs: {"name": "read-only+dynamic", "args": "--read-only --dynamic-toolsets"}, {"name": "toolsets-repos", "args": "--toolsets=repos"}, {"name": "toolsets-issues", "args": "--toolsets=issues"}, + {"name": "toolsets-context", "args": "--toolsets=context"}, {"name": "toolsets-pull_requests", "args": "--toolsets=pull_requests"}, {"name": "toolsets-repos,issues", "args": "--toolsets=repos,issues"}, + {"name": "toolsets-issues,context", "args": "--toolsets=issues,context"}, {"name": "toolsets-all", "args": "--toolsets=all"}, {"name": "tools-get_me", "args": "--tools=get_me"}, {"name": "tools-get_me,list_issues", "args": "--tools=get_me,list_issues"}, diff --git a/pkg/inventory/instructions.go b/pkg/inventory/instructions.go index e4524eb43..02e90cd20 100644 --- a/pkg/inventory/instructions.go +++ b/pkg/inventory/instructions.go @@ -31,7 +31,7 @@ Tool usage guidance: instructions = append(instructions, baseInstruction) // Collect instructions from each enabled toolset - for _, toolset := range inv.AvailableToolsets() { + for _, toolset := range inv.EnabledToolsets() { if toolset.InstructionsFunc != nil { if toolsetInstructions := toolset.InstructionsFunc(inv); toolsetInstructions != "" { instructions = append(instructions, toolsetInstructions) diff --git a/pkg/inventory/instructions_test.go b/pkg/inventory/instructions_test.go index 7a347c1e2..e8e369b3d 100644 --- a/pkg/inventory/instructions_test.go +++ b/pkg/inventory/instructions_test.go @@ -7,6 +7,7 @@ import ( ) // createTestInventory creates an inventory with the specified toolsets for testing. +// All toolsets are enabled by default using WithToolsets([]string{"all"}). func createTestInventory(toolsets []ToolsetMetadata) *Inventory { // Create tools for each toolset so they show up in AvailableToolsets() var tools []ServerTool @@ -18,6 +19,7 @@ func createTestInventory(toolsets []ToolsetMetadata) *Inventory { inv, _ := NewBuilder(). SetTools(tools). + WithToolsets([]string{"all"}). Build() return inv @@ -203,3 +205,61 @@ func TestToolsetInstructionsFunc(t *testing.T) { }) } } + +// TestGenerateInstructionsOnlyEnabledToolsets verifies that generateInstructions +// only includes instructions from enabled toolsets, not all available toolsets. +// This is a regression test for https://github.com/github/github-mcp-server/issues/1897 +func TestGenerateInstructionsOnlyEnabledToolsets(t *testing.T) { + // Create tools for multiple toolsets + reposToolset := ToolsetMetadata{ + ID: "repos", + Description: "Repository tools", + InstructionsFunc: func(_ *Inventory) string { + return "REPOS_INSTRUCTIONS" + }, + } + issuesToolset := ToolsetMetadata{ + ID: "issues", + Description: "Issue tools", + InstructionsFunc: func(_ *Inventory) string { + return "ISSUES_INSTRUCTIONS" + }, + } + prsToolset := ToolsetMetadata{ + ID: "pull_requests", + Description: "PR tools", + InstructionsFunc: func(_ *Inventory) string { + return "PRS_INSTRUCTIONS" + }, + } + + tools := []ServerTool{ + {Toolset: reposToolset}, + {Toolset: issuesToolset}, + {Toolset: prsToolset}, + } + + // Build inventory with only "repos" toolset enabled + inv, err := NewBuilder(). + SetTools(tools). + WithToolsets([]string{"repos"}). + Build() + if err != nil { + t.Fatalf("Failed to build inventory: %v", err) + } + + result := generateInstructions(inv) + + // Should contain instructions from enabled toolset + if !strings.Contains(result, "REPOS_INSTRUCTIONS") { + t.Errorf("Expected instructions to contain 'REPOS_INSTRUCTIONS' for enabled toolset, but it did not. Result: %s", result) + } + + // Should NOT contain instructions from non-enabled toolsets + if strings.Contains(result, "ISSUES_INSTRUCTIONS") { + t.Errorf("Did not expect instructions to contain 'ISSUES_INSTRUCTIONS' for disabled toolset, but it did. Result: %s", result) + } + if strings.Contains(result, "PRS_INSTRUCTIONS") { + t.Errorf("Did not expect instructions to contain 'PRS_INSTRUCTIONS' for disabled toolset, but it did. Result: %s", result) + } +} diff --git a/pkg/inventory/registry.go b/pkg/inventory/registry.go index e4113b452..e2cd3a9e6 100644 --- a/pkg/inventory/registry.go +++ b/pkg/inventory/registry.go @@ -295,6 +295,28 @@ func (r *Inventory) AvailableToolsets(exclude ...ToolsetID) []ToolsetMetadata { return result } +// EnabledToolsets returns the unique toolsets that are enabled based on current filters. +// This is similar to AvailableToolsets but respects the enabledToolsets filter. +// Returns toolsets in sorted order by toolset ID. +func (r *Inventory) EnabledToolsets() []ToolsetMetadata { + // Get all available toolsets first (already sorted by ID) + allToolsets := r.AvailableToolsets() + + // If no filter is set, all toolsets are enabled + if r.enabledToolsets == nil { + return allToolsets + } + + // Filter to only enabled toolsets + var result []ToolsetMetadata + for _, ts := range allToolsets { + if r.enabledToolsets[ts.ID] { + result = append(result, ts) + } + } + return result +} + func (r *Inventory) Instructions() string { return r.instructions }