refactor: immutable ToolsetGroup with self-describing tools and feature flags #1602

SamMorrowDrums · 2025-12-13T22:54:18Z

Summary

Major refactor of the toolsets package to create an elegant, immutable filtering system with self-describing tools.

Key Changes

Self-Describing Tools:

Tools now embed their own ToolsetMetadata (ID and description)
Read-only hint is derived from Tool.Annotations.ReadOnlyHint
No more separate AddReadTools() / AddWriteTools()

Simplified ToolsetGroup:

NewToolsetGroup(tools, resources, prompts) - takes lists directly
No more Toolset struct with SetDependencies()
Dependencies are only needed at registration time: RegisterAll(ctx, server, deps)

Immutable Filter Chain:

filtered := tsg.
    WithReadOnly(true).
    WithToolsets([]string{"repos", "issues"}).
    WithTools([]string{"get_me"}).  // additive, bypasses toolset filter
    WithFeatureChecker(checker)       // for feature flags

tools := filtered.AvailableTools(ctx)

Feature Flags:

FeatureFlagEnable - tool only available when flag is on
FeatureFlagDisable - tool excluded when flag is on
--features CLI flag for local server
Context-based checker for remote server (actor extraction)

Deterministic Ordering:

All Available*() methods return items sorted by toolset ID, then name
Platform/language independent for consistent docs generation

New Files:

pkg/github/toolset_group.go - NewToolsetGroup() factory
pkg/github/prompts.go - AllPrompts()
pkg/github/resources.go - AllResources()
docs/deprecated-tool-aliases.md - alias documentation

Usage

# Enable specific features
github-mcp-server stdio --features=my_feature,another_feature
GITHUB_FEATURES=my_feature github-mcp-server stdio

Stack:

refactor: convert projects, labels, and dynamic_tools to NewTool pattern #1601 ← base

Add CLI flag and config support for feature flags in the local server: - Add --features flag to main.go (StringSlice, comma-separated) - Add EnabledFeatures field to StdioServerConfig and MCPServerConfig - Create createFeatureChecker() that builds a set from enabled features - Wire WithFeatureChecker() into the toolset group filter chain This enables tools/resources/prompts that have FeatureFlagEnable set to a flag name that is passed via --features. The checker uses a simple set membership test for O(1) lookup. Usage: github-mcp-server stdio --features=my_feature,another_feature GITHUB_FEATURES=my_feature github-mcp-server stdio

Copilot

Pull request overview

This PR adds feature flag support to the local GitHub MCP server, enabling runtime control over which tools, resources, and prompts are available based on enabled features. The implementation includes a new --features CLI flag, feature flag fields on ServerTool/ServerResource/ServerPrompt, and a filtering mechanism integrated into the toolset group's filter chain.

Key Changes:

Added --features CLI flag and GITHUB_FEATURES environment variable support for comma-separated feature flags
Implemented FeatureFlagChecker interface and createFeatureChecker() for O(1) flag lookup
Added FeatureFlagEnable and FeatureFlagDisable fields to ServerTool, ServerResourceTemplate, and ServerPrompt
Integrated feature flag filtering into WithFeatureChecker() method with immutable filter chaining

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`pkg/toolsets/toolsets.go`	Core feature flag filtering logic: `FeatureFlagChecker` type, `WithFeatureChecker()` method, `isFeatureFlagAllowed()` logic integrated into `Available*` methods
`pkg/toolsets/toolsets_test.go`	Comprehensive test coverage for feature flags (enable/disable/both/errors) across tools, resources, and prompts
`pkg/toolsets/server_tool.go`	Added `FeatureFlagEnable` and `FeatureFlagDisable` string fields to ServerTool, ServerResourceTemplate, and ServerPrompt
`pkg/github/toolset_group.go`	New factory function for creating ToolsetGroup with all tools/resources/prompts
`internal/ghmcp/server.go`	Added `EnabledFeatures` config field, `createFeatureChecker()` implementation, wired into filter chain via `WithFeatureChecker()`
`cmd/github-mcp-server/main.go`	Added `--features` persistent flag with viper binding and GITHUB_FEATURES env var support
Multiple `pkg/github/*.go` files	Updated all tool/resource/prompt constructors to include toolset metadata parameter (structural refactor supporting feature flags)

This commit adds comprehensive validation tests to ensure all MCP items have required metadata: - TestAllToolsHaveRequiredMetadata: Validates Toolset.ID and Annotations - TestAllToolsHaveValidToolsetID: Ensures toolsets are in AvailableToolsets() - TestAllResourcesHaveRequiredMetadata: Validates resource metadata - TestAllPromptsHaveRequiredMetadata: Validates prompt metadata - TestToolReadOnlyHintConsistency: Validates IsReadOnly() matches annotation - TestNoDuplicate*Names: Ensures unique names across tools/resources/prompts - TestAllToolsHaveHandlerFunc: Ensures all tools have handlers - TestDefaultToolsetsAreValid: Validates default toolset IDs - TestToolsetMetadataConsistency: Ensures consistent descriptions per toolset Also fixes a bug discovered by these tests: ToolsetMetadataGit was defined but not added to AvailableToolsets(), causing get_repository_tree to have an invalid toolset ID.

When no toolsets are specified and dynamic mode is disabled, the server should use the default toolsets. The bug was introduced when adding dynamic toolsets support: 1. CleanToolsets(nil) was converting nil to empty slice 2. Empty slice passed to WithToolsets means 'no toolsets' 3. This resulted in zero tools being registered Fix: Preserve nil for non-dynamic mode (nil = use defaults in WithToolsets) and only set empty slice when dynamic mode is enabled without explicit toolsets.

SamMorrowDrums · 2025-12-14T00:06:56Z

internal/ghmcp/server.go

-	}
+	// Add deprecated tool aliases for backward compatibility
+	// See docs/deprecated-tool-aliases.md for the full list of renames
+	tsg.AddDeprecatedToolAliases(github.DeprecatedToolAliases)


Wait, isn't this returning a new tsg? Should be immutable...

So I think this is incorrect usage.

SamMorrowDrums · 2025-12-14T09:44:22Z

cmd/github-mcp-server/generate_docs.go

-}
-
 // mockGetRawClient returns a mock raw client for documentation generation
 func mockGetRawClient(_ context.Context) (*raw.Client, error) {


I am sure this can be removed too...

- Rename AddDeprecatedToolAliases to WithDeprecatedToolAliases for immutable filter chain consistency (returns new ToolsetGroup) - Remove unused mockGetRawClient from generate_docs.go (use nil instead) - Remove legacy ServerTool functions (NewServerToolLegacy and NewServerToolFromHandlerLegacy) - no usages - Add panic in Handler()/RegisterFunc() when HandlerFunc is nil - Add HasHandler() method for checking if tool has a handler - Add tests for HasHandler and nil handler panic behavior - Update all tests to use new WithDeprecatedToolAliases pattern

SamMorrowDrums · 2025-12-14T12:08:53Z

cmd/github-mcp-server/generate_docs.go

-	repoAccessCache := lockdown.GetInstance(nil)
-	tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000, github.FeatureFlags{}, repoAccessCache)
+	// Create toolset group with mock clients (no deps needed for doc generation)
+	tsg := github.NewToolsetGroup(t, mockGetClient, nil)


Not sure the interface is right yet. Do we need deps here...

…lsetGroup This change applies the same HandlerFunc pattern used by tools to resources, allowing NewToolsetGroup to be fully stateless (only requiring translations). Key changes: - Add ResourceHandlerFunc type to toolsets package - Update ServerResourceTemplate to use HandlerFunc instead of direct Handler - Add HasHandler() and Handler(deps) methods to ServerResourceTemplate - Update RegisterResourceTemplates to take deps parameter - Refactor repository resource definitions to use HandlerFunc pattern - Make AllResources(t) stateless (only takes translations) - Make NewToolsetGroup(t) stateless (only takes translations) - Update generate_docs.go - no longer needs mock clients - Update tests to use new patterns This resolves the concern about mixed concerns in doc generation - the toolset metadata and resource templates can now be created without any runtime dependencies, while handlers are generated on-demand when deps are provided during registration.

- Replace slice joining with strings.Builder for all doc generation - Iterate AllTools() directly instead of ToolsetIDs()/ToolsForToolset() - Removes need for special 'dynamic' toolset handling (no tools = no output) - Context toolset still explicitly handled for custom description - Consistent pattern across generateToolsetsDoc, generateToolsDoc, generateRemoteToolsetsDoc, and generateDeprecatedAliasesTable

- Add AvailableToolsets() method that returns toolsets with actual tools - Support variadic exclude parameter for filtering out specific toolsets - Simplifies doc generation by removing manual skip logic - Naturally excludes empty toolsets (like 'dynamic') without special cases

- Add Default field to ToolsetMetadata and derive defaults from metadata - Move toolset validation into WithToolsets (trims whitespace, dedupes, tracks unrecognized) - Add UnrecognizedToolsets() method for warning about typos - Add DefaultToolsetIDs() method to derive defaults from metadata - Remove redundant functions: CleanToolsets, GetValidToolsetIDs, AvailableToolsets, GetDefaultToolsetIDs - Update DynamicTools to take ToolsetGroup for schema enum generation - Add stubTranslator for cases needing ToolsetGroup without translations This eliminates hardcoded toolset lists - everything is now derived from the actual registered tools and their metadata.

Copilot AI review requested due to automatic review settings December 13, 2025 22:54

SamMorrowDrums requested a review from a team as a code owner December 13, 2025 22:54

Copilot started reviewing on behalf of SamMorrowDrums December 13, 2025 22:54 View session

Copilot AI reviewed Dec 13, 2025

View reviewed changes

SamMorrowDrums changed the title ~~Add --features CLI flag for feature flag support~~ refactor: immutable ToolsetGroup with self-describing tools and feature flags Dec 13, 2025

SamMorrowDrums added 2 commits December 14, 2025 00:05

SamMorrowDrums force-pushed the SamMorrowDrums/server-tool-refactor-toolsets branch from 78c89a3 to 689a040 Compare December 13, 2025 23:13

SamMorrowDrums mentioned this pull request Dec 13, 2025

Add Octicon icons to MCP tools #1603

Open

SamMorrowDrums commented Dec 14, 2025

View reviewed changes

SamMorrowDrums added 5 commits December 14, 2025 19:06

refactor: simplify ForMCPRequest switch cases

b13d9fe

refactor(generate_docs): hoist success logging to generateAllDocs

2142b02

SamMorrowDrums force-pushed the SamMorrowDrums/server-tool-refactor-toolsets branch 6 times, most recently from 9ef4da2 to a536c15 Compare December 14, 2025 23:48

SamMorrowDrums force-pushed the SamMorrowDrums/server-tool-refactor-toolsets branch from a536c15 to 6b6a874 Compare December 14, 2025 23:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: immutable ToolsetGroup with self-describing tools and feature flags #1602

refactor: immutable ToolsetGroup with self-describing tools and feature flags #1602

SamMorrowDrums commented Dec 13, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

SamMorrowDrums Dec 14, 2025

Uh oh!

SamMorrowDrums Dec 14, 2025

Uh oh!

SamMorrowDrums Dec 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

refactor: immutable ToolsetGroup with self-describing tools and feature flags #1602

Are you sure you want to change the base?

refactor: immutable ToolsetGroup with self-describing tools and feature flags #1602

Conversation

SamMorrowDrums commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Key Changes

Usage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

SamMorrowDrums Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

SamMorrowDrums Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

SamMorrowDrums Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SamMorrowDrums commented Dec 13, 2025 •

edited

Loading