From 4ff8c2154ee0ca41aa938114ae4904aefaee1495 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Mon, 26 Jan 2026 18:01:08 +0100 Subject: [PATCH 1/3] Fix generateInstructions to use only enabled toolsets Previously, generateInstructions() iterated over AvailableToolsets() which returns all toolsets that have tools defined, rather than only the enabled toolsets based on WithToolsets() configuration. This caused instructions for all toolsets to be included regardless of which toolsets were actually enabled, leading to bloated instructions (e.g., 5886 chars vs 1226 chars when only 'repos' toolset is enabled). Changes: - Add EnabledToolsets() method to return only enabled toolset metadata - Update generateInstructions() to use EnabledToolsets() - Add regression test for the fix Fixes #1897 --- pkg/inventory/instructions.go | 2 +- pkg/inventory/instructions_test.go | 60 ++++++++++++++++++++++++++++++ pkg/inventory/registry.go | 22 +++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) 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 } From e2adae108b24d9f4b19e42a3e96e920777900d68 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Mon, 26 Jan 2026 18:05:47 +0100 Subject: [PATCH 2/3] Update mcp-server-diff action to v2.2.0 Updates to v2.2.0 which includes server instructions diff support for detecting issues like #1897. --- .github/workflows/mcp-diff.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/mcp-diff.yml b/.github/workflows/mcp-diff.yml index 9c9c34b3a..02f08f915 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 From def56a7567da544ef7633fefee74a45028b1da8a Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Mon, 26 Jan 2026 18:10:56 +0100 Subject: [PATCH 3/3] Add more toolset configurations to mcp-diff workflow Add toolsets-context and toolsets-issues,context configurations to improve test coverage for instruction generation with different toolset combinations. --- .github/workflows/mcp-diff.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/mcp-diff.yml b/.github/workflows/mcp-diff.yml index 02f08f915..fb2a555bf 100644 --- a/.github/workflows/mcp-diff.yml +++ b/.github/workflows/mcp-diff.yml @@ -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"},