Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/mcp-diff.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"},
Expand Down
2 changes: 1 addition & 1 deletion pkg/inventory/instructions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
60 changes: 60 additions & 0 deletions pkg/inventory/instructions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -18,6 +19,7 @@ func createTestInventory(toolsets []ToolsetMetadata) *Inventory {

inv, _ := NewBuilder().
SetTools(tools).
WithToolsets([]string{"all"}).
Build()

return inv
Expand Down Expand Up @@ -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)
}
}
22 changes: 22 additions & 0 deletions pkg/inventory/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}