Skip to content

Commit 58829a7

Browse files
committed
wip injecting ff function into tool as dep
1 parent 8058d30 commit 58829a7

File tree

9 files changed

+306
-2
lines changed

9 files changed

+306
-2
lines changed

internal/ghmcp/server.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,9 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
198198
ghServer.AddReceivingMiddleware(addGitHubAPIErrorToContext)
199199
ghServer.AddReceivingMiddleware(addUserAgentsMiddleware(cfg, clients.rest, clients.gqlHTTP))
200200

201+
// Create feature checker
202+
featureChecker := createFeatureChecker(cfg.EnabledFeatures)
203+
201204
// Create dependencies for tool handlers
202205
deps := github.NewBaseDeps(
203206
clients.rest,
@@ -207,6 +210,7 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
207210
cfg.Translator,
208211
github.FeatureFlags{LockdownMode: cfg.LockdownMode},
209212
cfg.ContentWindowSize,
213+
featureChecker,
210214
)
211215

212216
// Inject dependencies into context for all tool handlers
@@ -222,7 +226,7 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
222226
WithReadOnly(cfg.ReadOnly).
223227
WithToolsets(enabledToolsets).
224228
WithTools(github.CleanTools(cfg.EnabledTools)).
225-
WithFeatureChecker(createFeatureChecker(cfg.EnabledFeatures))
229+
WithFeatureChecker(featureChecker)
226230

227231
// Apply token scope filtering if scopes are known (for PAT filtering)
228232
if cfg.TokenScopes != nil {

pkg/github/dependencies.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package github
33
import (
44
"context"
55
"errors"
6+
"fmt"
7+
"os"
68

79
"github.com/github/github-mcp-server/pkg/inventory"
810
"github.com/github/github-mcp-server/pkg/lockdown"
@@ -77,6 +79,11 @@ type ToolDependencies interface {
7779

7880
// GetContentWindowSize returns the content window size for log truncation
7981
GetContentWindowSize() int
82+
83+
// IsFeatureEnabled checks if a feature flag is enabled.
84+
// Returns false if checker is nil or flag is not enabled.
85+
// This allows tools to conditionally change behavior based on feature flags.
86+
IsFeatureEnabled(ctx context.Context, flagName string) bool
8087
}
8188

8289
// BaseDeps is the standard implementation of ToolDependencies for the local server.
@@ -93,6 +100,9 @@ type BaseDeps struct {
93100
T translations.TranslationHelperFunc
94101
Flags FeatureFlags
95102
ContentWindowSize int
103+
104+
// Feature flag checker for runtime checks
105+
featureChecker inventory.FeatureFlagChecker
96106
}
97107

98108
// NewBaseDeps creates a BaseDeps with the provided clients and configuration.
@@ -104,6 +114,7 @@ func NewBaseDeps(
104114
t translations.TranslationHelperFunc,
105115
flags FeatureFlags,
106116
contentWindowSize int,
117+
featureChecker inventory.FeatureFlagChecker,
107118
) *BaseDeps {
108119
return &BaseDeps{
109120
Client: client,
@@ -113,6 +124,7 @@ func NewBaseDeps(
113124
T: t,
114125
Flags: flags,
115126
ContentWindowSize: contentWindowSize,
127+
featureChecker: featureChecker,
116128
}
117129
}
118130

@@ -143,6 +155,24 @@ func (d BaseDeps) GetFlags() FeatureFlags { return d.Flags }
143155
// GetContentWindowSize implements ToolDependencies.
144156
func (d BaseDeps) GetContentWindowSize() int { return d.ContentWindowSize }
145157

158+
// IsFeatureEnabled checks if a feature flag is enabled.
159+
// Returns false if the feature checker is nil, flag name is empty, or an error occurs.
160+
// This allows tools to conditionally change behavior based on feature flags.
161+
func (d BaseDeps) IsFeatureEnabled(ctx context.Context, flagName string) bool {
162+
if d.featureChecker == nil || flagName == "" {
163+
return false
164+
}
165+
166+
enabled, err := d.featureChecker(ctx, flagName)
167+
if err != nil {
168+
// Log error but don't fail the tool - treat as disabled
169+
fmt.Fprintf(os.Stderr, "Feature flag check error for %q: %v\n", flagName, err)
170+
return false
171+
}
172+
173+
return enabled
174+
}
175+
146176
// NewTool creates a ServerTool that retrieves ToolDependencies from context at call time.
147177
// This avoids creating closures at registration time, which is important for performance
148178
// in servers that create a new server instance per request (like the remote server).

pkg/github/dependencies_test.go

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package github_test
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
8+
"github.com/github/github-mcp-server/pkg/github"
9+
"github.com/github/github-mcp-server/pkg/translations"
10+
"github.com/stretchr/testify/assert"
11+
)
12+
13+
func TestIsFeatureEnabled_WithEnabledFlag(t *testing.T) {
14+
t.Parallel()
15+
16+
// Create a feature checker that returns true for "test_flag"
17+
checker := func(ctx context.Context, flagName string) (bool, error) {
18+
return flagName == "test_flag", nil
19+
}
20+
21+
// Create deps with the checker using NewBaseDeps
22+
deps := github.NewBaseDeps(
23+
nil, // client
24+
nil, // gqlClient
25+
nil, // rawClient
26+
nil, // repoAccessCache
27+
translations.NullTranslationHelper,
28+
github.FeatureFlags{},
29+
0, // contentWindowSize
30+
checker, // featureChecker
31+
)
32+
33+
// Test enabled flag
34+
result := deps.IsFeatureEnabled(context.Background(), "test_flag")
35+
assert.True(t, result, "Expected test_flag to be enabled")
36+
37+
// Test disabled flag
38+
result = deps.IsFeatureEnabled(context.Background(), "other_flag")
39+
assert.False(t, result, "Expected other_flag to be disabled")
40+
}
41+
42+
func TestIsFeatureEnabled_WithoutChecker(t *testing.T) {
43+
t.Parallel()
44+
45+
// Create deps without feature checker (nil)
46+
deps := github.NewBaseDeps(
47+
nil, // client
48+
nil, // gqlClient
49+
nil, // rawClient
50+
nil, // repoAccessCache
51+
translations.NullTranslationHelper,
52+
github.FeatureFlags{},
53+
0, // contentWindowSize
54+
nil, // featureChecker (nil)
55+
)
56+
57+
// Should return false when checker is nil
58+
result := deps.IsFeatureEnabled(context.Background(), "any_flag")
59+
assert.False(t, result, "Expected false when checker is nil")
60+
}
61+
62+
func TestIsFeatureEnabled_EmptyFlagName(t *testing.T) {
63+
t.Parallel()
64+
65+
// Create a feature checker
66+
checker := func(ctx context.Context, flagName string) (bool, error) {
67+
return true, nil
68+
}
69+
70+
deps := github.NewBaseDeps(
71+
nil, // client
72+
nil, // gqlClient
73+
nil, // rawClient
74+
nil, // repoAccessCache
75+
translations.NullTranslationHelper,
76+
github.FeatureFlags{},
77+
0, // contentWindowSize
78+
checker, // featureChecker
79+
)
80+
81+
// Should return false for empty flag name
82+
result := deps.IsFeatureEnabled(context.Background(), "")
83+
assert.False(t, result, "Expected false for empty flag name")
84+
}
85+
86+
func TestIsFeatureEnabled_CheckerError(t *testing.T) {
87+
t.Parallel()
88+
89+
// Create a feature checker that returns an error
90+
checker := func(ctx context.Context, flagName string) (bool, error) {
91+
return false, errors.New("checker error")
92+
}
93+
94+
deps := github.NewBaseDeps(
95+
nil, // client
96+
nil, // gqlClient
97+
nil, // rawClient
98+
nil, // repoAccessCache
99+
translations.NullTranslationHelper,
100+
github.FeatureFlags{},
101+
0, // contentWindowSize
102+
checker, // featureChecker
103+
)
104+
105+
// Should return false and log error (not crash)
106+
result := deps.IsFeatureEnabled(context.Background(), "error_flag")
107+
assert.False(t, result, "Expected false when checker returns error")
108+
}
109+

pkg/github/dynamic_tools_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func TestDynamicTools_EnableToolset(t *testing.T) {
133133
deps := DynamicToolDependencies{
134134
Server: server,
135135
Inventory: reg,
136-
ToolDeps: NewBaseDeps(nil, nil, nil, nil, translations.NullTranslationHelper, FeatureFlags{}, 0),
136+
ToolDeps: NewBaseDeps(nil, nil, nil, nil, translations.NullTranslationHelper, FeatureFlags{}, 0, nil),
137137
T: translations.NullTranslationHelper,
138138
}
139139

pkg/github/feature_flags.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,7 @@ package github
44
type FeatureFlags struct {
55
LockdownMode bool
66
}
7+
8+
// RemoteMCPExperimental is a long-lived feature flag for experimental remote MCP features.
9+
// This flag enables experimental behaviors in tools that are being tested for remote server deployment.
10+
const RemoteMCPExperimental = "remote_mcp_experimental"

pkg/github/hello.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package github
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
7+
"github.com/github/github-mcp-server/pkg/inventory"
8+
"github.com/github/github-mcp-server/pkg/scopes"
9+
"github.com/github/github-mcp-server/pkg/translations"
10+
"github.com/github/github-mcp-server/pkg/utils"
11+
"github.com/google/jsonschema-go/jsonschema"
12+
"github.com/modelcontextprotocol/go-sdk/mcp"
13+
)
14+
15+
// HelloWorld returns a simple greeting tool that demonstrates feature flag conditional behavior.
16+
// This tool is for testing and demonstration purposes only.
17+
func HelloWorld(t translations.TranslationHelperFunc) inventory.ServerTool {
18+
return NewTool(
19+
ToolsetMetadataContext, // Use existing "context" toolset
20+
mcp.Tool{
21+
Name: "hello_world",
22+
Description: t("TOOL_HELLO_WORLD_DESCRIPTION", "A simple greeting tool that demonstrates feature flag conditional behavior"),
23+
Annotations: &mcp.ToolAnnotations{
24+
Title: t("TOOL_HELLO_WORLD_TITLE", "Hello World"),
25+
ReadOnlyHint: true,
26+
},
27+
InputSchema: &jsonschema.Schema{
28+
Type: "object",
29+
Properties: map[string]*jsonschema.Schema{
30+
"name": {
31+
Type: "string",
32+
Description: "Name to greet (optional, defaults to 'World')",
33+
},
34+
},
35+
},
36+
},
37+
[]scopes.Scope{}, // No GitHub scopes required - purely demonstrative
38+
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
39+
// Extract name parameter (optional)
40+
name := "World"
41+
if nameArg, ok := args["name"].(string); ok && nameArg != "" {
42+
name = nameArg
43+
}
44+
45+
// Check feature flag to determine greeting style
46+
var greeting string
47+
if deps.IsFeatureEnabled(ctx, RemoteMCPExperimental) {
48+
// Experimental: More enthusiastic greeting
49+
greeting = "🚀 Hello, " + name + "! Welcome to the EXPERIMENTAL future of MCP! 🎉"
50+
} else {
51+
// Default: Simple greeting
52+
greeting = "Hello, " + name + "!"
53+
}
54+
55+
// Build response
56+
response := map[string]any{
57+
"greeting": greeting,
58+
"experimental_mode": deps.IsFeatureEnabled(ctx, RemoteMCPExperimental),
59+
"timestamp": "2026-01-12", // Static for demonstration
60+
}
61+
62+
jsonBytes, err := json.Marshal(response)
63+
if err != nil {
64+
return utils.NewToolResultError("failed to marshal response"), nil, nil
65+
}
66+
67+
return utils.NewToolResultText(string(jsonBytes)), nil, nil
68+
},
69+
)
70+
}

pkg/github/hello_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package github_test
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/github/github-mcp-server/pkg/github"
8+
"github.com/github/github-mcp-server/pkg/translations"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func TestHelloWorld_ToolDefinition(t *testing.T) {
14+
t.Parallel()
15+
16+
// Create tool
17+
tool := github.HelloWorld(translations.NullTranslationHelper)
18+
19+
// Verify tool definition
20+
assert.Equal(t, "hello_world", tool.Tool.Name)
21+
assert.NotEmpty(t, tool.Tool.Description)
22+
assert.True(t, tool.Tool.Annotations.ReadOnlyHint, "hello_world should be read-only")
23+
assert.NotNil(t, tool.Tool.InputSchema)
24+
assert.NotNil(t, tool.HandlerFunc, "Tool must have a handler")
25+
26+
// Verify it's in the context toolset
27+
assert.Equal(t, "context", string(tool.Toolset.ID))
28+
29+
// Verify no scopes required
30+
assert.Empty(t, tool.RequiredScopes)
31+
32+
// Verify no feature flags set (tool itself isn't gated by flags)
33+
assert.Empty(t, tool.FeatureFlagEnable)
34+
assert.Empty(t, tool.FeatureFlagDisable)
35+
}
36+
37+
func TestHelloWorld_IsFeatureEnabledIntegration(t *testing.T) {
38+
t.Parallel()
39+
40+
// This test verifies that the feature flag checking mechanism works
41+
// by testing deps.IsFeatureEnabled directly
42+
43+
// Test 1: With feature flag disabled
44+
checkerDisabled := func(ctx context.Context, flagName string) (bool, error) {
45+
return false, nil
46+
}
47+
depsDisabled := github.NewBaseDeps(
48+
nil, nil, nil, nil,
49+
translations.NullTranslationHelper,
50+
github.FeatureFlags{},
51+
0,
52+
checkerDisabled,
53+
)
54+
55+
result := depsDisabled.IsFeatureEnabled(context.Background(), github.RemoteMCPExperimental)
56+
assert.False(t, result, "Feature flag should be disabled")
57+
58+
// Test 2: With feature flag enabled
59+
checkerEnabled := func(ctx context.Context, flagName string) (bool, error) {
60+
return flagName == github.RemoteMCPExperimental, nil
61+
}
62+
depsEnabled := github.NewBaseDeps(
63+
nil, nil, nil, nil,
64+
translations.NullTranslationHelper,
65+
github.FeatureFlags{},
66+
0,
67+
checkerEnabled,
68+
)
69+
70+
result = depsEnabled.IsFeatureEnabled(context.Background(), github.RemoteMCPExperimental)
71+
assert.True(t, result, "Feature flag should be enabled")
72+
73+
result = depsEnabled.IsFeatureEnabled(context.Background(), "other_flag")
74+
assert.False(t, result, "Other flag should be disabled")
75+
}
76+
77+
func TestHelloWorld_FeatureFlagConstant(t *testing.T) {
78+
t.Parallel()
79+
80+
// Verify the constant exists and has the expected value
81+
assert.Equal(t, "remote_mcp_experimental", github.RemoteMCPExperimental)
82+
require.NotEmpty(t, github.RemoteMCPExperimental, "Feature flag constant should not be empty")
83+
}

pkg/github/server_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ func (s stubDeps) GetRepoAccessCache() *lockdown.RepoAccessCache { return s.repo
5555
func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.t }
5656
func (s stubDeps) GetFlags() FeatureFlags { return s.flags }
5757
func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize }
58+
func (s stubDeps) IsFeatureEnabled(_ context.Context, _ string) bool { return false }
5859

5960
// Helper functions to create stub client functions for error testing
6061
func stubClientFnFromHTTP(httpClient *http.Client) func(context.Context) (*github.Client, error) {

pkg/github/tools.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,9 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool {
289289
GetLabelForLabelsToolset(t),
290290
ListLabels(t),
291291
LabelWrite(t),
292+
293+
// Demonstration tool for feature flag conditional behavior
294+
HelloWorld(t),
292295
}
293296
}
294297

0 commit comments

Comments
 (0)