Skip to content
Open
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
3 changes: 3 additions & 0 deletions cmd/github-mcp-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down
18 changes: 16 additions & 2 deletions internal/ghmcp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -198,15 +201,22 @@ 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,
clients.gql,
clients.raw,
clients.repoAccess,
cfg.Translator,
github.FeatureFlags{LockdownMode: cfg.LockdownMode},
github.FeatureFlags{
LockdownMode: cfg.LockdownMode,
Experimental: cfg.Experimental,
},
cfg.ContentWindowSize,
featureChecker,
)

// Inject dependencies into context for all tool handlers
Expand All @@ -222,7 +232,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 {
Expand Down Expand Up @@ -322,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
}
Expand Down Expand Up @@ -378,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,
Expand Down
1 change: 1 addition & 0 deletions internal/ghmcp/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ func TestNewMCPServer_CreatesSuccessfully(t *testing.T) {
Translator: translations.NullTranslationHelper,
ContentWindowSize: 5000,
LockdownMode: false,
Experimental: false,
}

// Create the server
Expand Down
31 changes: 31 additions & 0 deletions pkg/github/dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -77,6 +79,9 @@ type ToolDependencies interface {

// GetContentWindowSize returns the content window size for log truncation
GetContentWindowSize() int

// IsFeatureEnabled checks if a feature flag is enabled.
IsFeatureEnabled(ctx context.Context, flagName string) bool
}

// BaseDeps is the standard implementation of ToolDependencies for the local server.
Expand All @@ -93,8 +98,14 @@ type BaseDeps struct {
T translations.TranslationHelperFunc
Flags FeatureFlags
ContentWindowSize int

// Feature flag checker for runtime checks
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,
Expand All @@ -104,6 +115,7 @@ func NewBaseDeps(
t translations.TranslationHelperFunc,
flags FeatureFlags,
contentWindowSize int,
featureChecker inventory.FeatureFlagChecker,
) *BaseDeps {
return &BaseDeps{
Client: client,
Expand All @@ -113,6 +125,7 @@ func NewBaseDeps(
T: t,
Flags: flags,
ContentWindowSize: contentWindowSize,
featureChecker: featureChecker,
}
}

Expand Down Expand Up @@ -143,6 +156,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)
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error logging uses fmt.Fprintf to os.Stderr directly instead of using the project's logging infrastructure (pkg/log). This is inconsistent with how errors are logged elsewhere in the codebase. Consider using the established logging package for consistent error handling and formatting.

Copilot uses AI. Check for mistakes.
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).
Expand Down
108 changes: 108 additions & 0 deletions pkg/github/dependencies_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
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(_ 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(_ context.Context, _ 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(_ context.Context, _ 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")
}
2 changes: 1 addition & 1 deletion pkg/github/dynamic_tools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
1 change: 1 addition & 0 deletions pkg/github/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ package github
// FeatureFlags defines runtime feature toggles that adjust tool behavior.
type FeatureFlags struct {
LockdownMode bool
Experimental bool
}
Loading