From 58829a72d02d5930d612c12b58ff6a989f213061 Mon Sep 17 00:00:00 2001 From: tonytrg Date: Mon, 12 Jan 2026 19:23:00 +0100 Subject: [PATCH 01/10] wip injecting ff function into tool as dep --- internal/ghmcp/server.go | 6 +- pkg/github/dependencies.go | 30 +++++++++ pkg/github/dependencies_test.go | 109 +++++++++++++++++++++++++++++++ pkg/github/dynamic_tools_test.go | 2 +- pkg/github/feature_flags.go | 4 ++ pkg/github/hello.go | 70 ++++++++++++++++++++ pkg/github/hello_test.go | 83 +++++++++++++++++++++++ pkg/github/server_test.go | 1 + pkg/github/tools.go | 3 + 9 files changed, 306 insertions(+), 2 deletions(-) create mode 100644 pkg/github/dependencies_test.go create mode 100644 pkg/github/hello.go create mode 100644 pkg/github/hello_test.go diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 165886606..0dcb001c5 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -198,6 +198,9 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) { ghServer.AddReceivingMiddleware(addGitHubAPIErrorToContext) ghServer.AddReceivingMiddleware(addUserAgentsMiddleware(cfg, clients.rest, clients.gqlHTTP)) + // Create feature checker + featureChecker := createFeatureChecker(cfg.EnabledFeatures) + // Create dependencies for tool handlers deps := github.NewBaseDeps( clients.rest, @@ -207,6 +210,7 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) { cfg.Translator, github.FeatureFlags{LockdownMode: cfg.LockdownMode}, cfg.ContentWindowSize, + featureChecker, ) // Inject dependencies into context for all tool handlers @@ -222,7 +226,7 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) { WithReadOnly(cfg.ReadOnly). WithToolsets(enabledToolsets). WithTools(github.CleanTools(cfg.EnabledTools)). - WithFeatureChecker(createFeatureChecker(cfg.EnabledFeatures)) + WithFeatureChecker(featureChecker) // Apply token scope filtering if scopes are known (for PAT filtering) if cfg.TokenScopes != nil { diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go index b41bf0b87..201e434b0 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -3,6 +3,8 @@ package github import ( "context" "errors" + "fmt" + "os" "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/lockdown" @@ -77,6 +79,11 @@ type ToolDependencies interface { // GetContentWindowSize returns the content window size for log truncation GetContentWindowSize() int + + // IsFeatureEnabled checks if a feature flag is enabled. + // Returns false if checker is nil or flag is not enabled. + // This allows tools to conditionally change behavior based on feature flags. + IsFeatureEnabled(ctx context.Context, flagName string) bool } // BaseDeps is the standard implementation of ToolDependencies for the local server. @@ -93,6 +100,9 @@ type BaseDeps struct { T translations.TranslationHelperFunc Flags FeatureFlags ContentWindowSize int + + // Feature flag checker for runtime checks + featureChecker inventory.FeatureFlagChecker } // NewBaseDeps creates a BaseDeps with the provided clients and configuration. @@ -104,6 +114,7 @@ func NewBaseDeps( t translations.TranslationHelperFunc, flags FeatureFlags, contentWindowSize int, + featureChecker inventory.FeatureFlagChecker, ) *BaseDeps { return &BaseDeps{ Client: client, @@ -113,6 +124,7 @@ func NewBaseDeps( T: t, Flags: flags, ContentWindowSize: contentWindowSize, + featureChecker: featureChecker, } } @@ -143,6 +155,24 @@ func (d BaseDeps) GetFlags() FeatureFlags { return d.Flags } // GetContentWindowSize implements ToolDependencies. func (d BaseDeps) GetContentWindowSize() int { return d.ContentWindowSize } +// IsFeatureEnabled checks if a feature flag is enabled. +// Returns false if the feature checker is nil, flag name is empty, or an error occurs. +// This allows tools to conditionally change behavior based on feature flags. +func (d BaseDeps) IsFeatureEnabled(ctx context.Context, flagName string) bool { + if d.featureChecker == nil || flagName == "" { + return false + } + + enabled, err := d.featureChecker(ctx, flagName) + if err != nil { + // Log error but don't fail the tool - treat as disabled + fmt.Fprintf(os.Stderr, "Feature flag check error for %q: %v\n", flagName, err) + return false + } + + return enabled +} + // NewTool creates a ServerTool that retrieves ToolDependencies from context at call time. // This avoids creating closures at registration time, which is important for performance // in servers that create a new server instance per request (like the remote server). diff --git a/pkg/github/dependencies_test.go b/pkg/github/dependencies_test.go new file mode 100644 index 000000000..8d3d3fa4d --- /dev/null +++ b/pkg/github/dependencies_test.go @@ -0,0 +1,109 @@ +package github_test + +import ( + "context" + "errors" + "testing" + + "github.com/github/github-mcp-server/pkg/github" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/stretchr/testify/assert" +) + +func TestIsFeatureEnabled_WithEnabledFlag(t *testing.T) { + t.Parallel() + + // Create a feature checker that returns true for "test_flag" + checker := func(ctx context.Context, flagName string) (bool, error) { + return flagName == "test_flag", nil + } + + // Create deps with the checker using NewBaseDeps + deps := github.NewBaseDeps( + nil, // client + nil, // gqlClient + nil, // rawClient + nil, // repoAccessCache + translations.NullTranslationHelper, + github.FeatureFlags{}, + 0, // contentWindowSize + checker, // featureChecker + ) + + // Test enabled flag + result := deps.IsFeatureEnabled(context.Background(), "test_flag") + assert.True(t, result, "Expected test_flag to be enabled") + + // Test disabled flag + result = deps.IsFeatureEnabled(context.Background(), "other_flag") + assert.False(t, result, "Expected other_flag to be disabled") +} + +func TestIsFeatureEnabled_WithoutChecker(t *testing.T) { + t.Parallel() + + // Create deps without feature checker (nil) + deps := github.NewBaseDeps( + nil, // client + nil, // gqlClient + nil, // rawClient + nil, // repoAccessCache + translations.NullTranslationHelper, + github.FeatureFlags{}, + 0, // contentWindowSize + nil, // featureChecker (nil) + ) + + // Should return false when checker is nil + result := deps.IsFeatureEnabled(context.Background(), "any_flag") + assert.False(t, result, "Expected false when checker is nil") +} + +func TestIsFeatureEnabled_EmptyFlagName(t *testing.T) { + t.Parallel() + + // Create a feature checker + checker := func(ctx context.Context, flagName string) (bool, error) { + return true, nil + } + + deps := github.NewBaseDeps( + nil, // client + nil, // gqlClient + nil, // rawClient + nil, // repoAccessCache + translations.NullTranslationHelper, + github.FeatureFlags{}, + 0, // contentWindowSize + checker, // featureChecker + ) + + // Should return false for empty flag name + result := deps.IsFeatureEnabled(context.Background(), "") + assert.False(t, result, "Expected false for empty flag name") +} + +func TestIsFeatureEnabled_CheckerError(t *testing.T) { + t.Parallel() + + // Create a feature checker that returns an error + checker := func(ctx context.Context, flagName string) (bool, error) { + return false, errors.New("checker error") + } + + deps := github.NewBaseDeps( + nil, // client + nil, // gqlClient + nil, // rawClient + nil, // repoAccessCache + translations.NullTranslationHelper, + github.FeatureFlags{}, + 0, // contentWindowSize + checker, // featureChecker + ) + + // Should return false and log error (not crash) + result := deps.IsFeatureEnabled(context.Background(), "error_flag") + assert.False(t, result, "Expected false when checker returns error") +} + diff --git a/pkg/github/dynamic_tools_test.go b/pkg/github/dynamic_tools_test.go index 8d12b78c2..3fc701eb4 100644 --- a/pkg/github/dynamic_tools_test.go +++ b/pkg/github/dynamic_tools_test.go @@ -133,7 +133,7 @@ func TestDynamicTools_EnableToolset(t *testing.T) { deps := DynamicToolDependencies{ Server: server, Inventory: reg, - ToolDeps: NewBaseDeps(nil, nil, nil, nil, translations.NullTranslationHelper, FeatureFlags{}, 0), + ToolDeps: NewBaseDeps(nil, nil, nil, nil, translations.NullTranslationHelper, FeatureFlags{}, 0, nil), T: translations.NullTranslationHelper, } diff --git a/pkg/github/feature_flags.go b/pkg/github/feature_flags.go index 047042e44..04ab4e7c2 100644 --- a/pkg/github/feature_flags.go +++ b/pkg/github/feature_flags.go @@ -4,3 +4,7 @@ package github type FeatureFlags struct { LockdownMode bool } + +// RemoteMCPExperimental is a long-lived feature flag for experimental remote MCP features. +// This flag enables experimental behaviors in tools that are being tested for remote server deployment. +const RemoteMCPExperimental = "remote_mcp_experimental" diff --git a/pkg/github/hello.go b/pkg/github/hello.go new file mode 100644 index 000000000..ff27ede77 --- /dev/null +++ b/pkg/github/hello.go @@ -0,0 +1,70 @@ +package github + +import ( + "context" + "encoding/json" + + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/scopes" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/github/github-mcp-server/pkg/utils" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// HelloWorld returns a simple greeting tool that demonstrates feature flag conditional behavior. +// This tool is for testing and demonstration purposes only. +func HelloWorld(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataContext, // Use existing "context" toolset + mcp.Tool{ + Name: "hello_world", + Description: t("TOOL_HELLO_WORLD_DESCRIPTION", "A simple greeting tool that demonstrates feature flag conditional behavior"), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_HELLO_WORLD_TITLE", "Hello World"), + ReadOnlyHint: true, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "name": { + Type: "string", + Description: "Name to greet (optional, defaults to 'World')", + }, + }, + }, + }, + []scopes.Scope{}, // No GitHub scopes required - purely demonstrative + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + // Extract name parameter (optional) + name := "World" + if nameArg, ok := args["name"].(string); ok && nameArg != "" { + name = nameArg + } + + // Check feature flag to determine greeting style + var greeting string + if deps.IsFeatureEnabled(ctx, RemoteMCPExperimental) { + // Experimental: More enthusiastic greeting + greeting = "🚀 Hello, " + name + "! Welcome to the EXPERIMENTAL future of MCP! 🎉" + } else { + // Default: Simple greeting + greeting = "Hello, " + name + "!" + } + + // Build response + response := map[string]any{ + "greeting": greeting, + "experimental_mode": deps.IsFeatureEnabled(ctx, RemoteMCPExperimental), + "timestamp": "2026-01-12", // Static for demonstration + } + + jsonBytes, err := json.Marshal(response) + if err != nil { + return utils.NewToolResultError("failed to marshal response"), nil, nil + } + + return utils.NewToolResultText(string(jsonBytes)), nil, nil + }, + ) +} diff --git a/pkg/github/hello_test.go b/pkg/github/hello_test.go new file mode 100644 index 000000000..d30c54e8f --- /dev/null +++ b/pkg/github/hello_test.go @@ -0,0 +1,83 @@ +package github_test + +import ( + "context" + "testing" + + "github.com/github/github-mcp-server/pkg/github" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHelloWorld_ToolDefinition(t *testing.T) { + t.Parallel() + + // Create tool + tool := github.HelloWorld(translations.NullTranslationHelper) + + // Verify tool definition + assert.Equal(t, "hello_world", tool.Tool.Name) + assert.NotEmpty(t, tool.Tool.Description) + assert.True(t, tool.Tool.Annotations.ReadOnlyHint, "hello_world should be read-only") + assert.NotNil(t, tool.Tool.InputSchema) + assert.NotNil(t, tool.HandlerFunc, "Tool must have a handler") + + // Verify it's in the context toolset + assert.Equal(t, "context", string(tool.Toolset.ID)) + + // Verify no scopes required + assert.Empty(t, tool.RequiredScopes) + + // Verify no feature flags set (tool itself isn't gated by flags) + assert.Empty(t, tool.FeatureFlagEnable) + assert.Empty(t, tool.FeatureFlagDisable) +} + +func TestHelloWorld_IsFeatureEnabledIntegration(t *testing.T) { + t.Parallel() + + // This test verifies that the feature flag checking mechanism works + // by testing deps.IsFeatureEnabled directly + + // Test 1: With feature flag disabled + checkerDisabled := func(ctx context.Context, flagName string) (bool, error) { + return false, nil + } + depsDisabled := github.NewBaseDeps( + nil, nil, nil, nil, + translations.NullTranslationHelper, + github.FeatureFlags{}, + 0, + checkerDisabled, + ) + + result := depsDisabled.IsFeatureEnabled(context.Background(), github.RemoteMCPExperimental) + assert.False(t, result, "Feature flag should be disabled") + + // Test 2: With feature flag enabled + checkerEnabled := func(ctx context.Context, flagName string) (bool, error) { + return flagName == github.RemoteMCPExperimental, nil + } + depsEnabled := github.NewBaseDeps( + nil, nil, nil, nil, + translations.NullTranslationHelper, + github.FeatureFlags{}, + 0, + checkerEnabled, + ) + + result = depsEnabled.IsFeatureEnabled(context.Background(), github.RemoteMCPExperimental) + assert.True(t, result, "Feature flag should be enabled") + + result = depsEnabled.IsFeatureEnabled(context.Background(), "other_flag") + assert.False(t, result, "Other flag should be disabled") +} + +func TestHelloWorld_FeatureFlagConstant(t *testing.T) { + t.Parallel() + + // Verify the constant exists and has the expected value + assert.Equal(t, "remote_mcp_experimental", github.RemoteMCPExperimental) + require.NotEmpty(t, github.RemoteMCPExperimental, "Feature flag constant should not be empty") +} diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index a59cd9a93..503ee39d0 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -55,6 +55,7 @@ func (s stubDeps) GetRepoAccessCache() *lockdown.RepoAccessCache { return s.repo func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.t } func (s stubDeps) GetFlags() FeatureFlags { return s.flags } func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize } +func (s stubDeps) IsFeatureEnabled(_ context.Context, _ string) bool { return false } // Helper functions to create stub client functions for error testing func stubClientFnFromHTTP(httpClient *http.Client) func(context.Context) (*github.Client, error) { diff --git a/pkg/github/tools.go b/pkg/github/tools.go index b15c4fc9a..cf537dd14 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -289,6 +289,9 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { GetLabelForLabelsToolset(t), ListLabels(t), LabelWrite(t), + + // Demonstration tool for feature flag conditional behavior + HelloWorld(t), } } From de1b271cc96e4b889f0954d5ac93968103c49536 Mon Sep 17 00:00:00 2001 From: tonytrg Date: Mon, 12 Jan 2026 20:21:46 +0100 Subject: [PATCH 02/10] remove debug --- pkg/github/tools.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index cf537dd14..b15c4fc9a 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -289,9 +289,6 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { GetLabelForLabelsToolset(t), ListLabels(t), LabelWrite(t), - - // Demonstration tool for feature flag conditional behavior - HelloWorld(t), } } From 03f7909a753aafdcefd95a8b18d71d271a751f4e Mon Sep 17 00:00:00 2001 From: tonytrg Date: Wed, 14 Jan 2026 10:16:21 +0100 Subject: [PATCH 03/10] fix linter --- pkg/github/dependencies_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/github/dependencies_test.go b/pkg/github/dependencies_test.go index 8d3d3fa4d..6c4ec0795 100644 --- a/pkg/github/dependencies_test.go +++ b/pkg/github/dependencies_test.go @@ -14,7 +14,7 @@ func TestIsFeatureEnabled_WithEnabledFlag(t *testing.T) { t.Parallel() // Create a feature checker that returns true for "test_flag" - checker := func(ctx context.Context, flagName string) (bool, error) { + checker := func(_ context.Context, flagName string) (bool, error) { return flagName == "test_flag", nil } @@ -63,7 +63,7 @@ func TestIsFeatureEnabled_EmptyFlagName(t *testing.T) { t.Parallel() // Create a feature checker - checker := func(ctx context.Context, flagName string) (bool, error) { + checker := func(_ context.Context, flagName string) (bool, error) { return true, nil } @@ -87,7 +87,7 @@ func TestIsFeatureEnabled_CheckerError(t *testing.T) { t.Parallel() // Create a feature checker that returns an error - checker := func(ctx context.Context, flagName string) (bool, error) { + checker := func(_ context.Context, flagName string) (bool, error) { return false, errors.New("checker error") } From baaab8339b5155620e28b47fee00a3612fa38cd8 Mon Sep 17 00:00:00 2001 From: tonytrg Date: Wed, 14 Jan 2026 10:54:12 +0100 Subject: [PATCH 04/10] add better test --- pkg/github/dependencies.go | 2 - pkg/github/dependencies_test.go | 5 +- pkg/github/hello_test.go | 125 +++++++++++++++++++++----------- pkg/github/server_test.go | 8 +- 4 files changed, 89 insertions(+), 51 deletions(-) diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go index 201e434b0..3c2125793 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -81,8 +81,6 @@ type ToolDependencies interface { GetContentWindowSize() int // IsFeatureEnabled checks if a feature flag is enabled. - // Returns false if checker is nil or flag is not enabled. - // This allows tools to conditionally change behavior based on feature flags. IsFeatureEnabled(ctx context.Context, flagName string) bool } diff --git a/pkg/github/dependencies_test.go b/pkg/github/dependencies_test.go index 6c4ec0795..d13160d4c 100644 --- a/pkg/github/dependencies_test.go +++ b/pkg/github/dependencies_test.go @@ -63,7 +63,7 @@ func TestIsFeatureEnabled_EmptyFlagName(t *testing.T) { t.Parallel() // Create a feature checker - checker := func(_ context.Context, flagName string) (bool, error) { + checker := func(_ context.Context, _ string) (bool, error) { return true, nil } @@ -87,7 +87,7 @@ func TestIsFeatureEnabled_CheckerError(t *testing.T) { t.Parallel() // Create a feature checker that returns an error - checker := func(_ context.Context, flagName string) (bool, error) { + checker := func(_ context.Context, _ string) (bool, error) { return false, errors.New("checker error") } @@ -106,4 +106,3 @@ func TestIsFeatureEnabled_CheckerError(t *testing.T) { result := deps.IsFeatureEnabled(context.Background(), "error_flag") assert.False(t, result, "Expected false when checker returns error") } - diff --git a/pkg/github/hello_test.go b/pkg/github/hello_test.go index d30c54e8f..9a106d05c 100644 --- a/pkg/github/hello_test.go +++ b/pkg/github/hello_test.go @@ -2,10 +2,12 @@ package github_test import ( "context" + "encoding/json" "testing" "github.com/github/github-mcp-server/pkg/github" "github.com/github/github-mcp-server/pkg/translations" + "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -34,50 +36,89 @@ func TestHelloWorld_ToolDefinition(t *testing.T) { assert.Empty(t, tool.FeatureFlagDisable) } -func TestHelloWorld_IsFeatureEnabledIntegration(t *testing.T) { +func TestHelloWorld_ConditionalBehavior(t *testing.T) { t.Parallel() - // This test verifies that the feature flag checking mechanism works - // by testing deps.IsFeatureEnabled directly - - // Test 1: With feature flag disabled - checkerDisabled := func(ctx context.Context, flagName string) (bool, error) { - return false, nil - } - depsDisabled := github.NewBaseDeps( - nil, nil, nil, nil, - translations.NullTranslationHelper, - github.FeatureFlags{}, - 0, - checkerDisabled, - ) - - result := depsDisabled.IsFeatureEnabled(context.Background(), github.RemoteMCPExperimental) - assert.False(t, result, "Feature flag should be disabled") - - // Test 2: With feature flag enabled - checkerEnabled := func(ctx context.Context, flagName string) (bool, error) { - return flagName == github.RemoteMCPExperimental, nil + tests := []struct { + name string + featureFlagEnabled bool + inputName string + expectedGreeting string + expectedExperimentalMode bool + }{ + { + name: "Feature flag disabled - default greeting", + featureFlagEnabled: false, + inputName: "Alice", + expectedGreeting: "Hello, Alice!", + expectedExperimentalMode: false, + }, + { + name: "Feature flag enabled - experimental greeting", + featureFlagEnabled: true, + inputName: "Alice", + expectedGreeting: "🚀 Hello, Alice! Welcome to the EXPERIMENTAL future of MCP! 🎉", + expectedExperimentalMode: true, + }, } - depsEnabled := github.NewBaseDeps( - nil, nil, nil, nil, - translations.NullTranslationHelper, - github.FeatureFlags{}, - 0, - checkerEnabled, - ) - - result = depsEnabled.IsFeatureEnabled(context.Background(), github.RemoteMCPExperimental) - assert.True(t, result, "Feature flag should be enabled") - - result = depsEnabled.IsFeatureEnabled(context.Background(), "other_flag") - assert.False(t, result, "Other flag should be disabled") -} -func TestHelloWorld_FeatureFlagConstant(t *testing.T) { - t.Parallel() - - // Verify the constant exists and has the expected value - assert.Equal(t, "remote_mcp_experimental", github.RemoteMCPExperimental) - require.NotEmpty(t, github.RemoteMCPExperimental, "Feature flag constant should not be empty") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Create feature checker based on test case + checker := func(_ context.Context, flagName string) (bool, error) { + if flagName == github.RemoteMCPExperimental { + return tt.featureFlagEnabled, nil + } + return false, nil + } + + // Create deps with the checker + deps := github.NewBaseDeps( + nil, nil, nil, nil, + translations.NullTranslationHelper, + github.FeatureFlags{}, + 0, + checker, + ) + + // Get the tool and its handler + tool := github.HelloWorld(translations.NullTranslationHelper) + handler := tool.Handler(deps) + + // Create request + args := map[string]any{} + if tt.inputName != "" { + args["name"] = tt.inputName + } + argsJSON, err := json.Marshal(args) + require.NoError(t, err) + + request := mcp.CallToolRequest{ + Params: &mcp.CallToolParamsRaw{ + Arguments: json.RawMessage(argsJSON), + }, + } + + // Call the handler with deps in context + ctx := github.ContextWithDeps(context.Background(), deps) + result, err := handler(ctx, &request) + require.NoError(t, err) + require.NotNil(t, result) + require.Len(t, result.Content, 1) + + // Parse the response - should be TextContent + textContent, ok := result.Content[0].(*mcp.TextContent) + require.True(t, ok, "expected content to be TextContent") + + var response map[string]any + err = json.Unmarshal([]byte(textContent.Text), &response) + require.NoError(t, err) + + // Verify the greeting matches expected based on feature flag + assert.Equal(t, tt.expectedGreeting, response["greeting"]) + assert.Equal(t, tt.expectedExperimentalMode, response["experimental_mode"]) + }) + } } diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index 503ee39d0..bd6d3344d 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -51,10 +51,10 @@ func (s stubDeps) GetRawClient(ctx context.Context) (*raw.Client, error) { return nil, nil } -func (s stubDeps) GetRepoAccessCache() *lockdown.RepoAccessCache { return s.repoAccessCache } -func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.t } -func (s stubDeps) GetFlags() FeatureFlags { return s.flags } -func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize } +func (s stubDeps) GetRepoAccessCache() *lockdown.RepoAccessCache { return s.repoAccessCache } +func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.t } +func (s stubDeps) GetFlags() FeatureFlags { return s.flags } +func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize } func (s stubDeps) IsFeatureEnabled(_ context.Context, _ string) bool { return false } // Helper functions to create stub client functions for error testing From fa0aa8cb6e9bbb1a508d6fc9da839bf035a40ae4 Mon Sep 17 00:00:00 2001 From: tonytrg Date: Wed, 14 Jan 2026 11:04:14 +0100 Subject: [PATCH 05/10] adding compile time check --- pkg/github/dependencies.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go index 3c2125793..15d807a24 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -103,6 +103,9 @@ type BaseDeps struct { featureChecker inventory.FeatureFlagChecker } +// Compile-time assertion to verify that BaseDeps implements the ToolDependencies interface. +var _ ToolDependencies = (*BaseDeps)(nil) + // NewBaseDeps creates a BaseDeps with the provided clients and configuration. func NewBaseDeps( client *gogithub.Client, From 15c037a23e24fd23a4f89aae4beda2b39c90a139 Mon Sep 17 00:00:00 2001 From: tonytrg Date: Wed, 14 Jan 2026 21:40:56 +0100 Subject: [PATCH 06/10] move experimental to seperate config/ff value --- cmd/github-mcp-server/main.go | 3 + internal/ghmcp/server.go | 12 ++- pkg/github/feature_flags.go | 5 +- .../{hello_test.go => feature_flags_test.go} | 81 +++++++++++++++++-- pkg/github/hello.go | 70 ---------------- pkg/github/server_test.go | 1 + 6 files changed, 89 insertions(+), 83 deletions(-) rename pkg/github/{hello_test.go => feature_flags_test.go} (53%) delete mode 100644 pkg/github/hello.go diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index cfb68be4e..72caa17cb 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -83,6 +83,7 @@ var ( LogFilePath: viper.GetString("log-file"), ContentWindowSize: viper.GetInt("content-window-size"), LockdownMode: viper.GetBool("lockdown-mode"), + Experimental: viper.GetBool("experimental"), RepoAccessCacheTTL: &ttl, } return ghmcp.RunStdioServer(stdioServerConfig) @@ -108,6 +109,7 @@ func init() { rootCmd.PersistentFlags().String("gh-host", "", "Specify the GitHub hostname (for GitHub Enterprise etc.)") rootCmd.PersistentFlags().Int("content-window-size", 5000, "Specify the content window size") rootCmd.PersistentFlags().Bool("lockdown-mode", false, "Enable lockdown mode") + rootCmd.PersistentFlags().Bool("experimental", false, "Enable experimental features") rootCmd.PersistentFlags().Duration("repo-access-cache-ttl", 5*time.Minute, "Override the repo access cache TTL (e.g. 1m, 0s to disable)") // Bind flag to viper @@ -122,6 +124,7 @@ func init() { _ = viper.BindPFlag("host", rootCmd.PersistentFlags().Lookup("gh-host")) _ = viper.BindPFlag("content-window-size", rootCmd.PersistentFlags().Lookup("content-window-size")) _ = viper.BindPFlag("lockdown-mode", rootCmd.PersistentFlags().Lookup("lockdown-mode")) + _ = viper.BindPFlag("experimental", rootCmd.PersistentFlags().Lookup("experimental")) _ = viper.BindPFlag("repo-access-cache-ttl", rootCmd.PersistentFlags().Lookup("repo-access-cache-ttl")) // Add subcommands diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 4ddc76a87..94d7c2794 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -64,6 +64,9 @@ type MCPServerConfig struct { // LockdownMode indicates if we should enable lockdown mode LockdownMode bool + // Experimental indicates if we should enable experimental features + Experimental bool + // Logger is used for logging within the server Logger *slog.Logger // RepoAccessTTL overrides the default TTL for repository access cache entries. @@ -208,7 +211,10 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) { clients.raw, clients.repoAccess, cfg.Translator, - github.FeatureFlags{LockdownMode: cfg.LockdownMode}, + github.FeatureFlags{ + LockdownMode: cfg.LockdownMode, + Experimental: cfg.Experimental, + }, cfg.ContentWindowSize, featureChecker, ) @@ -326,6 +332,9 @@ type StdioServerConfig struct { // LockdownMode indicates if we should enable lockdown mode LockdownMode bool + // Experimental indicates if we should enable experimental features + Experimental bool + // RepoAccessCacheTTL overrides the default TTL for repository access cache entries. RepoAccessCacheTTL *time.Duration } @@ -382,6 +391,7 @@ func RunStdioServer(cfg StdioServerConfig) error { Translator: t, ContentWindowSize: cfg.ContentWindowSize, LockdownMode: cfg.LockdownMode, + Experimental: cfg.Experimental, Logger: logger, RepoAccessTTL: cfg.RepoAccessCacheTTL, TokenScopes: tokenScopes, diff --git a/pkg/github/feature_flags.go b/pkg/github/feature_flags.go index 04ab4e7c2..e11847194 100644 --- a/pkg/github/feature_flags.go +++ b/pkg/github/feature_flags.go @@ -3,8 +3,5 @@ package github // FeatureFlags defines runtime feature toggles that adjust tool behavior. type FeatureFlags struct { LockdownMode bool + Experimental bool } - -// RemoteMCPExperimental is a long-lived feature flag for experimental remote MCP features. -// This flag enables experimental behaviors in tools that are being tested for remote server deployment. -const RemoteMCPExperimental = "remote_mcp_experimental" diff --git a/pkg/github/hello_test.go b/pkg/github/feature_flags_test.go similarity index 53% rename from pkg/github/hello_test.go rename to pkg/github/feature_flags_test.go index 9a106d05c..3817eb6ff 100644 --- a/pkg/github/hello_test.go +++ b/pkg/github/feature_flags_test.go @@ -1,22 +1,87 @@ -package github_test +package github import ( "context" "encoding/json" "testing" - "github.com/github/github-mcp-server/pkg/github" "github.com/github/github-mcp-server/pkg/translations" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/scopes" + "github.com/github/github-mcp-server/pkg/utils" + "github.com/google/jsonschema-go/jsonschema" ) +// RemoteMCPExperimental is a long-lived feature flag for experimental remote MCP features. +// This flag enables experimental behaviors in tools that are being tested for remote server deployment. +const RemoteMCPEnthusiasticGreeting = "remote_mcp_enthusiastic_greeting" + +// HelloWorld returns a simple greeting tool that demonstrates feature flag conditional behavior. +// This tool is for testing and demonstration purposes only. +func HelloWorld(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataContext, // Use existing "context" toolset + mcp.Tool{ + Name: "hello_world", + Description: t("TOOL_HELLO_WORLD_DESCRIPTION", "A simple greeting tool that demonstrates feature flag conditional behavior"), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_HELLO_WORLD_TITLE", "Hello World"), + ReadOnlyHint: true, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "name": { + Type: "string", + Description: "Name to greet (optional, defaults to 'World')", + }, + }, + }, + }, + []scopes.Scope{}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + // Extract name parameter (optional) + name := "World" + if nameArg, ok := args["name"].(string); ok && nameArg != "" { + name = nameArg + } + + // Check feature flag to determine greeting style + var greeting string + if deps.IsFeatureEnabled(ctx, RemoteMCPEnthusiasticGreeting) { + // Experimental: More enthusiastic greeting + greeting = "🚀 Hello, " + name + "! Welcome to the EXPERIMENTAL future of MCP! 🎉" + } else { + // Default: Simple greeting + greeting = "Hello, " + name + "!" + } + + // Build response + response := map[string]any{ + "greeting": greeting, + "experimental_mode": deps.IsFeatureEnabled(ctx, RemoteMCPEnthusiasticGreeting), + "timestamp": "2026-01-12", // Static for demonstration + } + + jsonBytes, err := json.Marshal(response) + if err != nil { + return utils.NewToolResultError("failed to marshal response"), nil, nil + } + + return utils.NewToolResultText(string(jsonBytes)), nil, nil + }, + ) +} + func TestHelloWorld_ToolDefinition(t *testing.T) { t.Parallel() // Create tool - tool := github.HelloWorld(translations.NullTranslationHelper) + tool := HelloWorld(translations.NullTranslationHelper) // Verify tool definition assert.Equal(t, "hello_world", tool.Tool.Name) @@ -68,23 +133,23 @@ func TestHelloWorld_ConditionalBehavior(t *testing.T) { // Create feature checker based on test case checker := func(_ context.Context, flagName string) (bool, error) { - if flagName == github.RemoteMCPExperimental { + if flagName == RemoteMCPEnthusiasticGreeting { return tt.featureFlagEnabled, nil } return false, nil } // Create deps with the checker - deps := github.NewBaseDeps( + deps := NewBaseDeps( nil, nil, nil, nil, translations.NullTranslationHelper, - github.FeatureFlags{}, + FeatureFlags{}, 0, checker, ) // Get the tool and its handler - tool := github.HelloWorld(translations.NullTranslationHelper) + tool := HelloWorld(translations.NullTranslationHelper) handler := tool.Handler(deps) // Create request @@ -102,7 +167,7 @@ func TestHelloWorld_ConditionalBehavior(t *testing.T) { } // Call the handler with deps in context - ctx := github.ContextWithDeps(context.Background(), deps) + ctx := ContextWithDeps(context.Background(), deps) result, err := handler(ctx, &request) require.NoError(t, err) require.NotNil(t, result) diff --git a/pkg/github/hello.go b/pkg/github/hello.go deleted file mode 100644 index ff27ede77..000000000 --- a/pkg/github/hello.go +++ /dev/null @@ -1,70 +0,0 @@ -package github - -import ( - "context" - "encoding/json" - - "github.com/github/github-mcp-server/pkg/inventory" - "github.com/github/github-mcp-server/pkg/scopes" - "github.com/github/github-mcp-server/pkg/translations" - "github.com/github/github-mcp-server/pkg/utils" - "github.com/google/jsonschema-go/jsonschema" - "github.com/modelcontextprotocol/go-sdk/mcp" -) - -// HelloWorld returns a simple greeting tool that demonstrates feature flag conditional behavior. -// This tool is for testing and demonstration purposes only. -func HelloWorld(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( - ToolsetMetadataContext, // Use existing "context" toolset - mcp.Tool{ - Name: "hello_world", - Description: t("TOOL_HELLO_WORLD_DESCRIPTION", "A simple greeting tool that demonstrates feature flag conditional behavior"), - Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_HELLO_WORLD_TITLE", "Hello World"), - ReadOnlyHint: true, - }, - InputSchema: &jsonschema.Schema{ - Type: "object", - Properties: map[string]*jsonschema.Schema{ - "name": { - Type: "string", - Description: "Name to greet (optional, defaults to 'World')", - }, - }, - }, - }, - []scopes.Scope{}, // No GitHub scopes required - purely demonstrative - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - // Extract name parameter (optional) - name := "World" - if nameArg, ok := args["name"].(string); ok && nameArg != "" { - name = nameArg - } - - // Check feature flag to determine greeting style - var greeting string - if deps.IsFeatureEnabled(ctx, RemoteMCPExperimental) { - // Experimental: More enthusiastic greeting - greeting = "🚀 Hello, " + name + "! Welcome to the EXPERIMENTAL future of MCP! 🎉" - } else { - // Default: Simple greeting - greeting = "Hello, " + name + "!" - } - - // Build response - response := map[string]any{ - "greeting": greeting, - "experimental_mode": deps.IsFeatureEnabled(ctx, RemoteMCPExperimental), - "timestamp": "2026-01-12", // Static for demonstration - } - - jsonBytes, err := json.Marshal(response) - if err != nil { - return utils.NewToolResultError("failed to marshal response"), nil, nil - } - - return utils.NewToolResultText(string(jsonBytes)), nil, nil - }, - ) -} diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index bd6d3344d..1d07a0de5 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -84,6 +84,7 @@ func stubRepoAccessCache(client *githubv4.Client, ttl time.Duration) *lockdown.R func stubFeatureFlags(enabledFlags map[string]bool) FeatureFlags { return FeatureFlags{ LockdownMode: enabledFlags["lockdown-mode"], + Experimental: enabledFlags["experimental"], } } From b54e64ea83a19881ba61146f3e4d6fd51c3b122b Mon Sep 17 00:00:00 2001 From: tonytrg Date: Wed, 14 Jan 2026 21:43:45 +0100 Subject: [PATCH 07/10] adding test var --- internal/ghmcp/server_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/ghmcp/server_test.go b/internal/ghmcp/server_test.go index 04c0989d4..be4f914c7 100644 --- a/internal/ghmcp/server_test.go +++ b/internal/ghmcp/server_test.go @@ -23,6 +23,7 @@ func TestNewMCPServer_CreatesSuccessfully(t *testing.T) { Translator: translations.NullTranslationHelper, ContentWindowSize: 5000, LockdownMode: false, + Experimental: false, } // Create the server From 42a8cbc3e4bf5a2354aec8c095f8adf143830219 Mon Sep 17 00:00:00 2001 From: tonytrg Date: Wed, 14 Jan 2026 22:31:56 +0100 Subject: [PATCH 08/10] fixing test --- pkg/github/feature_flags_test.go | 170 ++++++++++++++++--------------- 1 file changed, 90 insertions(+), 80 deletions(-) diff --git a/pkg/github/feature_flags_test.go b/pkg/github/feature_flags_test.go index 3817eb6ff..791014d86 100644 --- a/pkg/github/feature_flags_test.go +++ b/pkg/github/feature_flags_test.go @@ -13,16 +13,21 @@ import ( "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/utils" - "github.com/google/jsonschema-go/jsonschema" ) // RemoteMCPExperimental is a long-lived feature flag for experimental remote MCP features. // This flag enables experimental behaviors in tools that are being tested for remote server deployment. const RemoteMCPEnthusiasticGreeting = "remote_mcp_enthusiastic_greeting" +// FeatureChecker is an interface for checking if a feature flag is enabled. +type FeatureChecker interface { + // IsFeatureEnabled checks if a feature flag is enabled. + IsFeatureEnabled(ctx context.Context, flagName string) bool +} + // HelloWorld returns a simple greeting tool that demonstrates feature flag conditional behavior. // This tool is for testing and demonstration purposes only. -func HelloWorld(t translations.TranslationHelperFunc) inventory.ServerTool { +func HelloWorldTool(t translations.TranslationHelperFunc) inventory.ServerTool { return NewTool( ToolsetMetadataContext, // Use existing "context" toolset mcp.Tool{ @@ -32,39 +37,22 @@ func HelloWorld(t translations.TranslationHelperFunc) inventory.ServerTool { Title: t("TOOL_HELLO_WORLD_TITLE", "Hello World"), ReadOnlyHint: true, }, - InputSchema: &jsonschema.Schema{ - Type: "object", - Properties: map[string]*jsonschema.Schema{ - "name": { - Type: "string", - Description: "Name to greet (optional, defaults to 'World')", - }, - }, - }, }, []scopes.Scope{}, - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - // Extract name parameter (optional) - name := "World" - if nameArg, ok := args["name"].(string); ok && nameArg != "" { - name = nameArg - } + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, _ map[string]any) (*mcp.CallToolResult, any, error) { // Check feature flag to determine greeting style - var greeting string + greeting := "Hello, world!" if deps.IsFeatureEnabled(ctx, RemoteMCPEnthusiasticGreeting) { - // Experimental: More enthusiastic greeting - greeting = "🚀 Hello, " + name + "! Welcome to the EXPERIMENTAL future of MCP! 🎉" - } else { - // Default: Simple greeting - greeting = "Hello, " + name + "!" + greeting = greeting + " Welcome to the future of MCP! 🎉" + } + if deps.GetFlags().Experimental { + greeting = greeting + " Experimental features are enabled! 🚀" } // Build response response := map[string]any{ - "greeting": greeting, - "experimental_mode": deps.IsFeatureEnabled(ctx, RemoteMCPEnthusiasticGreeting), - "timestamp": "2026-01-12", // Static for demonstration + "greeting": greeting, } jsonBytes, err := json.Marshal(response) @@ -77,53 +65,24 @@ func HelloWorld(t translations.TranslationHelperFunc) inventory.ServerTool { ) } -func TestHelloWorld_ToolDefinition(t *testing.T) { - t.Parallel() - - // Create tool - tool := HelloWorld(translations.NullTranslationHelper) - - // Verify tool definition - assert.Equal(t, "hello_world", tool.Tool.Name) - assert.NotEmpty(t, tool.Tool.Description) - assert.True(t, tool.Tool.Annotations.ReadOnlyHint, "hello_world should be read-only") - assert.NotNil(t, tool.Tool.InputSchema) - assert.NotNil(t, tool.HandlerFunc, "Tool must have a handler") - - // Verify it's in the context toolset - assert.Equal(t, "context", string(tool.Toolset.ID)) - - // Verify no scopes required - assert.Empty(t, tool.RequiredScopes) - - // Verify no feature flags set (tool itself isn't gated by flags) - assert.Empty(t, tool.FeatureFlagEnable) - assert.Empty(t, tool.FeatureFlagDisable) -} - -func TestHelloWorld_ConditionalBehavior(t *testing.T) { +func TestHelloWorld_ConditionalBehavior_Featureflag(t *testing.T) { t.Parallel() tests := []struct { - name string - featureFlagEnabled bool - inputName string - expectedGreeting string - expectedExperimentalMode bool + name string + featureFlagEnabled bool + inputName string + expectedGreeting string }{ { - name: "Feature flag disabled - default greeting", - featureFlagEnabled: false, - inputName: "Alice", - expectedGreeting: "Hello, Alice!", - expectedExperimentalMode: false, + name: "Feature flag disabled - default greeting", + featureFlagEnabled: false, + expectedGreeting: "Hello, world!", }, { - name: "Feature flag enabled - experimental greeting", - featureFlagEnabled: true, - inputName: "Alice", - expectedGreeting: "🚀 Hello, Alice! Welcome to the EXPERIMENTAL future of MCP! 🎉", - expectedExperimentalMode: true, + name: "Feature flag enabled - enthusiastic greeting", + featureFlagEnabled: true, + expectedGreeting: "Hello, world! Welcome to the future of MCP! 🎉", }, } @@ -149,26 +108,78 @@ func TestHelloWorld_ConditionalBehavior(t *testing.T) { ) // Get the tool and its handler - tool := HelloWorld(translations.NullTranslationHelper) + tool := HelloWorldTool(translations.NullTranslationHelper) handler := tool.Handler(deps) - // Create request - args := map[string]any{} - if tt.inputName != "" { - args["name"] = tt.inputName - } - argsJSON, err := json.Marshal(args) + // Call the handler with deps in context + ctx := ContextWithDeps(context.Background(), deps) + result, err := handler(ctx, &mcp.CallToolRequest{ + Params: &mcp.CallToolParamsRaw{ + Arguments: json.RawMessage(`{}`), + }, + }) require.NoError(t, err) + require.NotNil(t, result) + require.Len(t, result.Content, 1) - request := mcp.CallToolRequest{ - Params: &mcp.CallToolParamsRaw{ - Arguments: json.RawMessage(argsJSON), - }, - } + // Parse the response - should be TextContent + textContent, ok := result.Content[0].(*mcp.TextContent) + require.True(t, ok, "expected content to be TextContent") + + var response map[string]any + err = json.Unmarshal([]byte(textContent.Text), &response) + require.NoError(t, err) + + // Verify the greeting matches expected based on feature flag + assert.Equal(t, tt.expectedGreeting, response["greeting"]) + }) + } +} + +func TestHelloWorld_ConditionalBehavior_Config(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + experimental bool + expectedGreeting string + }{ + { + name: "Experimental disabled - default greeting", + experimental: false, + expectedGreeting: "Hello, world!", + }, + { + name: "Experimental enabled - experimental greeting", + experimental: true, + expectedGreeting: "Hello, world! Experimental features are enabled! 🚀", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Create deps with the checker + deps := NewBaseDeps( + nil, nil, nil, nil, + translations.NullTranslationHelper, + FeatureFlags{Experimental: tt.experimental}, + 0, + nil, + ) + + // Get the tool and its handler + tool := HelloWorldTool(translations.NullTranslationHelper) + handler := tool.Handler(deps) // Call the handler with deps in context ctx := ContextWithDeps(context.Background(), deps) - result, err := handler(ctx, &request) + result, err := handler(ctx, &mcp.CallToolRequest{ + Params: &mcp.CallToolParamsRaw{ + Arguments: json.RawMessage(`{}`), + }, + }) require.NoError(t, err) require.NotNil(t, result) require.Len(t, result.Content, 1) @@ -183,7 +194,6 @@ func TestHelloWorld_ConditionalBehavior(t *testing.T) { // Verify the greeting matches expected based on feature flag assert.Equal(t, tt.expectedGreeting, response["greeting"]) - assert.Equal(t, tt.expectedExperimentalMode, response["experimental_mode"]) }) } } From 5d8561fe66ee5df011d4eb7bd794aaf8541fab7f Mon Sep 17 00:00:00 2001 From: tonytrg Date: Thu, 15 Jan 2026 11:26:37 +0100 Subject: [PATCH 09/10] adding flag and possibility to call feature checker --- pkg/github/feature_flags_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/github/feature_flags_test.go b/pkg/github/feature_flags_test.go index 791014d86..61caeb2e8 100644 --- a/pkg/github/feature_flags_test.go +++ b/pkg/github/feature_flags_test.go @@ -44,10 +44,10 @@ func HelloWorldTool(t translations.TranslationHelperFunc) inventory.ServerTool { // Check feature flag to determine greeting style greeting := "Hello, world!" if deps.IsFeatureEnabled(ctx, RemoteMCPEnthusiasticGreeting) { - greeting = greeting + " Welcome to the future of MCP! 🎉" + greeting += " Welcome to the future of MCP! 🎉" } if deps.GetFlags().Experimental { - greeting = greeting + " Experimental features are enabled! 🚀" + greeting += " Experimental features are enabled! 🚀" } // Build response From 30ae562abfa463252dd20ca9849b982ac9efe9fe Mon Sep 17 00:00:00 2001 From: tonytrg Date: Fri, 16 Jan 2026 11:14:11 +0100 Subject: [PATCH 10/10] fixing name --- cmd/github-mcp-server/main.go | 6 +++--- internal/ghmcp/server.go | 12 ++++++------ internal/ghmcp/server_test.go | 2 +- pkg/github/feature_flags.go | 2 +- pkg/github/feature_flags_test.go | 29 ++++++++++++++--------------- pkg/github/server_test.go | 2 +- 6 files changed, 26 insertions(+), 27 deletions(-) diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index 72caa17cb..af59ee0e6 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -83,7 +83,7 @@ var ( LogFilePath: viper.GetString("log-file"), ContentWindowSize: viper.GetInt("content-window-size"), LockdownMode: viper.GetBool("lockdown-mode"), - Experimental: viper.GetBool("experimental"), + InsiderMode: viper.GetBool("insider-mode"), RepoAccessCacheTTL: &ttl, } return ghmcp.RunStdioServer(stdioServerConfig) @@ -109,7 +109,7 @@ func init() { rootCmd.PersistentFlags().String("gh-host", "", "Specify the GitHub hostname (for GitHub Enterprise etc.)") rootCmd.PersistentFlags().Int("content-window-size", 5000, "Specify the content window size") rootCmd.PersistentFlags().Bool("lockdown-mode", false, "Enable lockdown mode") - rootCmd.PersistentFlags().Bool("experimental", false, "Enable experimental features") + rootCmd.PersistentFlags().Bool("insider-mode", false, "Enable insider features") rootCmd.PersistentFlags().Duration("repo-access-cache-ttl", 5*time.Minute, "Override the repo access cache TTL (e.g. 1m, 0s to disable)") // Bind flag to viper @@ -124,7 +124,7 @@ func init() { _ = viper.BindPFlag("host", rootCmd.PersistentFlags().Lookup("gh-host")) _ = viper.BindPFlag("content-window-size", rootCmd.PersistentFlags().Lookup("content-window-size")) _ = viper.BindPFlag("lockdown-mode", rootCmd.PersistentFlags().Lookup("lockdown-mode")) - _ = viper.BindPFlag("experimental", rootCmd.PersistentFlags().Lookup("experimental")) + _ = viper.BindPFlag("insider-mode", rootCmd.PersistentFlags().Lookup("insider-mode")) _ = viper.BindPFlag("repo-access-cache-ttl", rootCmd.PersistentFlags().Lookup("repo-access-cache-ttl")) // Add subcommands diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 94d7c2794..fa0ab047f 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -64,8 +64,8 @@ type MCPServerConfig struct { // LockdownMode indicates if we should enable lockdown mode LockdownMode bool - // Experimental indicates if we should enable experimental features - Experimental bool + // Insider indicates if we should enable experimental features + InsiderMode bool // Logger is used for logging within the server Logger *slog.Logger @@ -213,7 +213,7 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) { cfg.Translator, github.FeatureFlags{ LockdownMode: cfg.LockdownMode, - Experimental: cfg.Experimental, + InsiderMode: cfg.InsiderMode, }, cfg.ContentWindowSize, featureChecker, @@ -332,8 +332,8 @@ type StdioServerConfig struct { // LockdownMode indicates if we should enable lockdown mode LockdownMode bool - // Experimental indicates if we should enable experimental features - Experimental bool + // InsiderMode indicates if we should enable experimental features + InsiderMode bool // RepoAccessCacheTTL overrides the default TTL for repository access cache entries. RepoAccessCacheTTL *time.Duration @@ -391,7 +391,7 @@ func RunStdioServer(cfg StdioServerConfig) error { Translator: t, ContentWindowSize: cfg.ContentWindowSize, LockdownMode: cfg.LockdownMode, - Experimental: cfg.Experimental, + InsiderMode: cfg.InsiderMode, Logger: logger, RepoAccessTTL: cfg.RepoAccessCacheTTL, TokenScopes: tokenScopes, diff --git a/internal/ghmcp/server_test.go b/internal/ghmcp/server_test.go index be4f914c7..7854cb77f 100644 --- a/internal/ghmcp/server_test.go +++ b/internal/ghmcp/server_test.go @@ -23,7 +23,7 @@ func TestNewMCPServer_CreatesSuccessfully(t *testing.T) { Translator: translations.NullTranslationHelper, ContentWindowSize: 5000, LockdownMode: false, - Experimental: false, + InsiderMode: false, } // Create the server diff --git a/pkg/github/feature_flags.go b/pkg/github/feature_flags.go index e11847194..dbaa8cec2 100644 --- a/pkg/github/feature_flags.go +++ b/pkg/github/feature_flags.go @@ -3,5 +3,5 @@ package github // FeatureFlags defines runtime feature toggles that adjust tool behavior. type FeatureFlags struct { LockdownMode bool - Experimental bool + InsiderMode bool } diff --git a/pkg/github/feature_flags_test.go b/pkg/github/feature_flags_test.go index 61caeb2e8..fb50448af 100644 --- a/pkg/github/feature_flags_test.go +++ b/pkg/github/feature_flags_test.go @@ -15,8 +15,7 @@ import ( "github.com/github/github-mcp-server/pkg/utils" ) -// RemoteMCPExperimental is a long-lived feature flag for experimental remote MCP features. -// This flag enables experimental behaviors in tools that are being tested for remote server deployment. +// RemoteMCPEnthusiasticGreeting is a dummy test feature flag . const RemoteMCPEnthusiasticGreeting = "remote_mcp_enthusiastic_greeting" // FeatureChecker is an interface for checking if a feature flag is enabled. @@ -46,7 +45,7 @@ func HelloWorldTool(t translations.TranslationHelperFunc) inventory.ServerTool { if deps.IsFeatureEnabled(ctx, RemoteMCPEnthusiasticGreeting) { greeting += " Welcome to the future of MCP! 🎉" } - if deps.GetFlags().Experimental { + if deps.GetFlags().InsiderMode { greeting += " Experimental features are enabled! 🚀" } @@ -114,10 +113,10 @@ func TestHelloWorld_ConditionalBehavior_Featureflag(t *testing.T) { // Call the handler with deps in context ctx := ContextWithDeps(context.Background(), deps) result, err := handler(ctx, &mcp.CallToolRequest{ - Params: &mcp.CallToolParamsRaw{ - Arguments: json.RawMessage(`{}`), - }, - }) + Params: &mcp.CallToolParamsRaw{ + Arguments: json.RawMessage(`{}`), + }, + }) require.NoError(t, err) require.NotNil(t, result) require.Len(t, result.Content, 1) @@ -141,17 +140,17 @@ func TestHelloWorld_ConditionalBehavior_Config(t *testing.T) { tests := []struct { name string - experimental bool + insiderMode bool expectedGreeting string }{ { name: "Experimental disabled - default greeting", - experimental: false, + insiderMode: false, expectedGreeting: "Hello, world!", }, { name: "Experimental enabled - experimental greeting", - experimental: true, + insiderMode: true, expectedGreeting: "Hello, world! Experimental features are enabled! 🚀", }, } @@ -164,7 +163,7 @@ func TestHelloWorld_ConditionalBehavior_Config(t *testing.T) { deps := NewBaseDeps( nil, nil, nil, nil, translations.NullTranslationHelper, - FeatureFlags{Experimental: tt.experimental}, + FeatureFlags{InsiderMode: tt.insiderMode}, 0, nil, ) @@ -176,10 +175,10 @@ func TestHelloWorld_ConditionalBehavior_Config(t *testing.T) { // Call the handler with deps in context ctx := ContextWithDeps(context.Background(), deps) result, err := handler(ctx, &mcp.CallToolRequest{ - Params: &mcp.CallToolParamsRaw{ - Arguments: json.RawMessage(`{}`), - }, - }) + Params: &mcp.CallToolParamsRaw{ + Arguments: json.RawMessage(`{}`), + }, + }) require.NoError(t, err) require.NotNil(t, result) require.Len(t, result.Content, 1) diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index 1d07a0de5..e91fc851e 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -84,7 +84,7 @@ func stubRepoAccessCache(client *githubv4.Client, ttl time.Duration) *lockdown.R func stubFeatureFlags(enabledFlags map[string]bool) FeatureFlags { return FeatureFlags{ LockdownMode: enabledFlags["lockdown-mode"], - Experimental: enabledFlags["experimental"], + InsiderMode: enabledFlags["insider-mode"], } }