refactor: immutable ToolsetGroup with self-describing tools and feature flags #1602
+2,576
−1,117
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
Contributor
There was a problem hiding this comment.
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
--featuresCLI flag andGITHUB_FEATURESenvironment variable support for comma-separated feature flags - Implemented
FeatureFlagCheckerinterface andcreateFeatureChecker()for O(1) flag lookup - Added
FeatureFlagEnableandFeatureFlagDisablefields 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) |
SamMorrowDrums
changed the title
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
force-pushed
the
SamMorrowDrums/server-tool-refactor-toolsets
branch
from
78c89a3 to
689a040
Compare
SamMorrowDrums
commented
Dec 14, 2025
internal/ghmcp/server.go
Outdated
| } | ||
| // Add deprecated tool aliases for backward compatibility | ||
| // See docs/deprecated-tool-aliases.md for the full list of renames | ||
| tsg.AddDeprecatedToolAliases(github.DeprecatedToolAliases) |
Collaborator
Author
There was a problem hiding this comment.
Wait, isn't this returning a new tsg? Should be immutable...
So I think this is incorrect usage.
| } | ||
|
|
||
| // mockGetRawClient returns a mock raw client for documentation generation | ||
| func mockGetRawClient(_ context.Context) (*raw.Client, error) { |
Collaborator
Author
There was a problem hiding this comment.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Major refactor of the toolsets package to create an elegant, immutable filtering system with self-describing tools.
Key Changes
Self-Describing Tools:
ToolsetMetadata(ID and description)Tool.Annotations.ReadOnlyHintAddReadTools()/AddWriteTools()Simplified ToolsetGroup:
NewToolsetGroup(tools, resources, prompts)- takes lists directlyToolsetstruct withSetDependencies()RegisterAll(ctx, server, deps)Immutable Filter Chain:
Feature Flags:
FeatureFlagEnable- tool only available when flag is onFeatureFlagDisable- tool excluded when flag is on--featuresCLI flag for local serverDeterministic Ordering:
Available*()methods return items sorted by toolset ID, then nameNew Files:
pkg/github/toolset_group.go-NewToolsetGroup()factorypkg/github/prompts.go-AllPrompts()pkg/github/resources.go-AllResources()docs/deprecated-tool-aliases.md- alias documentationUsage
# Enable specific features github-mcp-server stdio --features=my_feature,another_feature GITHUB_FEATURES=my_feature github-mcp-server stdioStack: