Skip to content

Commit a87351e

Browse files
refactor(features)!: use functional availability rules
Resolve declared inventory features once per request and share the request-owned cache with in-handler feature checks. BREAKING CHANGE: Inventory items now use FeatureRule instead of FeatureFlagEnable, FeatureFlagEnableAll, and FeatureFlagDisable; FeatureFlagChecker now accepts FeatureFlag. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1e4a1ca6-53f7-4158-af22-35d2448d0b13
1 parent 7b6646c commit a87351e

45 files changed

Lines changed: 746 additions & 375 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎cmd/github-mcp-server/feature_flag_docs.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func generateFeatureFlagsDocs(docsPath string) error {
3333
// whose registration or definition differs from the default user experience.
3434
// Each affected tool is printed with its full schema using the same writer
3535
// used by the README so the output style stays consistent.
36-
func generateFlaggedToolsDoc(flags []string, emptyMessage string) string {
36+
func generateFlaggedToolsDoc(flags []inventory.FeatureFlag, emptyMessage string) string {
3737
t, _ := translations.TranslationHelper()
3838
defaultTools := indexToolsByName(buildInventoryWithFlags(t, nil).ToolsForRegistration(context.Background()))
3939

@@ -73,8 +73,8 @@ func generateFlaggedToolsDoc(flags []string, emptyMessage string) string {
7373
// differs from the default-flagged inventory when only the given flag is on,
7474
// plus tools that exist only in the flag-on inventory. Results are sorted by
7575
// tool name.
76-
func flaggedToolDiff(t translations.TranslationHelperFunc, flag string, defaultTools map[string]inventory.ServerTool) []inventory.ServerTool {
77-
flagTools := buildInventoryWithFlags(t, map[string]bool{flag: true}).ToolsForRegistration(context.Background())
76+
func flaggedToolDiff(t translations.TranslationHelperFunc, flag inventory.FeatureFlag, defaultTools map[string]inventory.ServerTool) []inventory.ServerTool {
77+
flagTools := buildInventoryWithFlags(t, map[inventory.FeatureFlag]bool{flag: true}).ToolsForRegistration(context.Background())
7878

7979
out := make([]inventory.ServerTool, 0)
8080
seen := make(map[string]struct{}, len(flagTools))
@@ -99,8 +99,8 @@ func flaggedToolDiff(t translations.TranslationHelperFunc, flag string, defaultT
9999
// buildInventoryWithFlags constructs an inventory whose feature checker treats
100100
// the given flags as enabled and every other flag as disabled. Passing nil
101101
// produces the default-flagged inventory.
102-
func buildInventoryWithFlags(t translations.TranslationHelperFunc, enabled map[string]bool) *inventory.Inventory {
103-
checker := func(_ context.Context, flag string) (bool, error) {
102+
func buildInventoryWithFlags(t translations.TranslationHelperFunc, enabled map[inventory.FeatureFlag]bool) *inventory.Inventory {
103+
checker := func(_ context.Context, flag inventory.FeatureFlag) (bool, error) {
104104
return enabled[flag], nil
105105
}
106106
inv, _ := github.NewInventory(t).

‎cmd/github-mcp-server/generate_docs.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func init() {
3131

3232
// noFeatureFlagsChecker reports every feature flag as disabled. It models the
3333
// default user experience used by the generated documentation.
34-
func noFeatureFlagsChecker(_ context.Context, _ string) (bool, error) {
34+
func noFeatureFlagsChecker(_ context.Context, _ inventory.FeatureFlag) (bool, error) {
3535
return false, nil
3636
}
3737

@@ -61,9 +61,8 @@ func generateReadmeDocs(readmePath string) error {
6161

6262
// The README documents the default user experience: tools that are
6363
// enabled with no special flags set. Installing a checker that reports
64-
// every flag as disabled excludes tools gated by FeatureFlagEnable and
65-
// keeps the legacy variants of tools gated by FeatureFlagDisable, so
66-
// flag-gated duplicates don't appear twice.
64+
// every flag as disabled keeps the default variants selected by functional
65+
// feature rules, so flag-gated duplicates don't appear twice.
6766
// Build() can only fail if WithTools specifies invalid tools - not used here
6867
r, _ := github.NewInventory(t).
6968
WithToolsets([]string{"all"}).

‎docs/feature-flags.md‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,34 @@ Only flags listed in
3535
[`AllowedFeatureFlags`](../pkg/github/feature_flags.go) can be enabled by
3636
end users. Insiders-only flags are not user-toggleable.
3737

38+
## Declaring tool availability
39+
40+
Tools, resources, and prompts use `inventory.NewFeatureRule` when feature flags
41+
change whether they are available. Each rule declares the flags it references
42+
and evaluates them with a fail-closed `FeatureResolver`, so normal Go boolean
43+
expressions can represent AND, OR, NOT, and mixed conditions:
44+
45+
```go
46+
tool.FeatureRule = inventory.NewFeatureRule(
47+
[]inventory.FeatureFlag{x, y},
48+
func(featureAsBool inventory.FeatureResolver) bool {
49+
return !(featureAsBool(x) && featureAsBool(y))
50+
},
51+
)
52+
```
53+
54+
The service deduplicates the declared flags, resolves each one at most once for
55+
the request, and shares those values with tool dependencies. Feature checks
56+
inside handlers continue to use `deps.IsFeatureEnabled`.
57+
3858
---
3959

4060
## Tools affected by each flag
4161

42-
The list below is regenerated from the Go source. For each user-controllable
43-
feature flag, it lists every tool whose **inventory or input schema** differs
44-
from the default — either because the flag introduces a new tool, or because
45-
it selects a flag-aware variant of an existing tool. Flags that only affect
46-
runtime behavior (such as output formatting) won't appear here.
62+
The list below is regenerated by comparing the default tool surface with each
63+
user-controllable flag enabled individually. Complex multi-flag rules may
64+
require separate documentation. Flags that only affect runtime behavior (such
65+
as output formatting) won't appear here.
4766

4867
<!-- START AUTOMATED FEATURE FLAG TOOLS -->
4968

‎docs/insiders-features.md‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,13 @@ Insiders is a **meta feature flag** — the same shape as `default` or `all` for
207207
3. **Insiders expansion.** If insiders mode is on (`--insiders`, `/insiders` route, or `X-MCP-Insiders: true`), every flag in [`InsidersFeatureFlags`](../pkg/github/feature_flags.go) is unioned in. The insiders expansion is **not** re-validated against the allowlist — insiders is a server-controlled switch that can reach internal-only flags.
208208
4. **Server-side fallback (remote server only).** Any flag not yet decided falls back to the remote server's feature manager, which can roll a feature out independently of user input or insiders membership.
209209

210+
For tool availability, each functional feature rule statically declares the
211+
flags it reads. The service deduplicates those declarations, resolves every
212+
relevant flag once into request-owned state, and then evaluates all rules as
213+
in-memory boolean expressions. The same state backs
214+
`deps.IsFeatureEnabled`, so checks made inside a tool call reuse resolved values
215+
and lazily cache any handler-only flag using the live tool-call context.
216+
210217
`AllowedFeatureFlags` and `InsidersFeatureFlags` are deliberately independent sets:
211218

212219
- A flag in **`AllowedFeatureFlags` only** is a regular opt-in: users can turn it on, but insiders does not auto-enable it. Granular issues/PRs flags work this way.
@@ -219,5 +226,6 @@ Insiders is a **meta feature flag** — the same shape as `default` or `all` for
219226
2. Add it to `AllowedFeatureFlags` if end users should be able to opt in via
220227
`--features`, `X-MCP-Features`, or the `features` URL query parameter.
221228
3. Add it to `InsidersFeatureFlags` if insiders mode should turn it on automatically.
222-
4. Gate the behavior on the concrete flag (`deps.IsFeatureEnabled(ctx, FeatureFlagX)`), never on `cfg.InsidersMode`. There is a `TestGitHubPackageDoesNotReadInsidersMode` guard test that fails if `pkg/github` reads `InsidersMode` directly.
223-
5. The MCP-diff CI workflow picks up new entries in `AllowedFeatureFlags` automatically — see `.github/workflows/mcp-diff.yml`.
229+
4. For tool availability, attach an `inventory.NewFeatureRule` that declares every flag used by its predicate. For behavior inside a handler, use `deps.IsFeatureEnabled(ctx, FeatureFlagX)`.
230+
5. Gate on concrete flags, never on `cfg.InsidersMode`. There is a `TestGitHubPackageDoesNotReadInsidersMode` guard test that fails if `pkg/github` reads `InsidersMode` directly.
231+
6. The MCP-diff CI workflow picks up new entries in `AllowedFeatureFlags` automatically — see `.github/workflows/mcp-diff.yml`.

‎internal/ghmcp/server.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ type StdioServerConfig struct {
256256
EnabledTools []string
257257

258258
// EnabledFeatures is a list of feature flags that are enabled
259-
// Items with FeatureFlagEnable matching an entry in this list will be available
259+
// Tool feature rules evaluate entries in this list.
260260
EnabledFeatures []string
261261

262262
// ReadOnly indicates if we should only register read-only tools
@@ -435,8 +435,8 @@ func RunStdioServer(cfg StdioServerConfig) error {
435435
// using the centralized ResolveFeatureFlags function. For the local server,
436436
// features are resolved once at startup from --features CLI flag and insiders mode.
437437
func createFeatureChecker(enabledFeatures []string, insidersMode bool) inventory.FeatureFlagChecker {
438-
featureSet := github.ResolveFeatureFlags(enabledFeatures, insidersMode)
439-
return func(_ context.Context, flagName string) (bool, error) {
438+
featureSet := github.ResolveFeatureFlags(github.FeatureFlagsFromStrings(enabledFeatures), insidersMode)
439+
return func(_ context.Context, flagName inventory.FeatureFlag) (bool, error) {
440440
return featureSet[flagName], nil
441441
}
442442
}

‎pkg/github/actions_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ func Test_ActionsGetJobLogs(t *testing.T) {
574574
// Note: consolidated ActionsGetJobLogs has same tool name "get_job_logs" as the individual tool
575575
// but with different descriptions. We skip toolsnap validation here since the individual
576576
// tool's toolsnap already exists and is tested in Test_GetJobLogs.
577-
// The consolidated tool has FeatureFlagEnable set, so only one will be active at a time.
577+
// The functional feature rules ensure only one variant is active at a time.
578578
assert.Equal(t, "get_job_logs", toolDef.Tool.Name)
579579
assert.NotEmpty(t, toolDef.Tool.Description)
580580
inputSchema := toolDef.Tool.InputSchema.(*jsonschema.Schema)

‎pkg/github/context_tools_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/github/github-mcp-server/internal/githubv4mock"
1111
"github.com/github/github-mcp-server/internal/toolsnaps"
12+
"github.com/github/github-mcp-server/pkg/inventory"
1213
"github.com/github/github-mcp-server/pkg/translations"
1314
"github.com/google/go-github/v89/github"
1415
"github.com/modelcontextprotocol/go-sdk/mcp"
@@ -189,7 +190,7 @@ func Test_GetMe_IFC_FeatureFlag(t *testing.T) {
189190
translations.NullTranslationHelper,
190191
FeatureFlags{},
191192
0,
192-
func(_ context.Context, flagName string) (bool, error) {
193+
func(_ context.Context, flagName inventory.FeatureFlag) (bool, error) {
193194
return flagName == FeatureFlagIFCLabels && enabled, nil
194195
},
195196
stubExporters(),

‎pkg/github/csv_output_test.go‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,18 @@ func TestCSVOutputAppliedToDefaultListTools(t *testing.T) {
2929
require.Len(t, available, 2)
3030

3131
listing := requireToolByName(t, available, "list_things")
32-
assert.Empty(t, listing.FeatureFlagEnable)
33-
assert.Empty(t, listing.FeatureFlagDisable)
32+
assert.True(t, listing.FeatureRule.IsZero())
3433

3534
getting := requireToolByName(t, available, "get_thing")
36-
assert.Empty(t, getting.FeatureFlagEnable)
37-
assert.Empty(t, getting.FeatureFlagDisable)
35+
assert.True(t, getting.FeatureRule.IsZero())
3836
}
3937
}
4038

4139
func TestCSVOutputAppliesToFlagGatedListTools(t *testing.T) {
4240
enabledOnly := testCSVOutputTool("list_things", `[{"number":1}]`)
43-
enabledOnly.FeatureFlagEnable = FeatureFlagFileBlame
41+
enabledOnly.FeatureRule = featureEnabledRule(FeatureFlagFileBlame)
4442
disabledOnly := testCSVOutputTool("list_legacy_things", `[{"number":2}]`)
45-
disabledOnly.FeatureFlagDisable = []string{FeatureFlagFileBlame}
43+
disabledOnly.FeatureRule = featureDisabledRule(FeatureFlagFileBlame)
4644

4745
tools := withCSVOutput([]inventory.ServerTool{enabledOnly, disabledOnly})
4846
require.Len(t, tools, 2)
@@ -368,7 +366,7 @@ type csvOutputTestDeps struct {
368366
csvOn bool
369367
}
370368

371-
func (d csvOutputTestDeps) IsFeatureEnabled(_ context.Context, flag string) bool {
369+
func (d csvOutputTestDeps) IsFeatureEnabled(_ context.Context, flag inventory.FeatureFlag) bool {
372370
return flag == FeatureFlagCSVOutput && d.csvOn
373371
}
374372

‎pkg/github/dependencies.go‎

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"log/slog"
88
"net/http"
9-
"os"
109

1110
ghcontext "github.com/github/github-mcp-server/pkg/context"
1211
"github.com/github/github-mcp-server/pkg/http/transport"
@@ -95,7 +94,7 @@ type ToolDependencies interface {
9594
GetContentWindowSize() int
9695

9796
// IsFeatureEnabled checks if a feature flag is enabled.
98-
IsFeatureEnabled(ctx context.Context, flagName string) bool
97+
IsFeatureEnabled(ctx context.Context, flag inventory.FeatureFlag) bool
9998

10099
// Logger returns the structured logger, optionally enriched with
101100
// request-scoped data from ctx. Integrators provide their own slog.Handler
@@ -207,19 +206,8 @@ func (d BaseDeps) GetRequestStateSealer() RequestStateSealer { return d.StateSea
207206
// IsFeatureEnabled checks if a feature flag is enabled.
208207
// Returns false if the feature checker is nil, flag name is empty, or an error occurs.
209208
// This allows tools to conditionally change behavior based on feature flags.
210-
func (d BaseDeps) IsFeatureEnabled(ctx context.Context, flagName string) bool {
211-
if d.featureChecker == nil || flagName == "" {
212-
return false
213-
}
214-
215-
enabled, err := d.featureChecker(ctx, flagName)
216-
if err != nil {
217-
// Log error but don't fail the tool - treat as disabled
218-
fmt.Fprintf(os.Stderr, "Feature flag check error for %q: %v\n", flagName, err)
219-
return false
220-
}
221-
222-
return enabled
209+
func (d BaseDeps) IsFeatureEnabled(ctx context.Context, flag inventory.FeatureFlag) bool {
210+
return inventory.ResolveFeature(ctx, d.featureChecker, flag)
223211
}
224212

225213
// NewTool creates a ServerTool that retrieves ToolDependencies from context at call time.
@@ -496,17 +484,6 @@ func (d *RequestDeps) Metrics(ctx context.Context) metrics.Metrics {
496484
}
497485

498486
// IsFeatureEnabled checks if a feature flag is enabled.
499-
func (d *RequestDeps) IsFeatureEnabled(ctx context.Context, flagName string) bool {
500-
if d.featureChecker == nil || flagName == "" {
501-
return false
502-
}
503-
504-
enabled, err := d.featureChecker(ctx, flagName)
505-
if err != nil {
506-
// Log error but don't fail the tool - treat as disabled
507-
fmt.Fprintf(os.Stderr, "Feature flag check error for %q: %v\n", flagName, err)
508-
return false
509-
}
510-
511-
return enabled
487+
func (d *RequestDeps) IsFeatureEnabled(ctx context.Context, flag inventory.FeatureFlag) bool {
488+
return inventory.ResolveFeature(ctx, d.featureChecker, flag)
512489
}

‎pkg/github/dependencies_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
ghcontext "github.com/github/github-mcp-server/pkg/context"
1515
"github.com/github/github-mcp-server/pkg/github"
1616
"github.com/github/github-mcp-server/pkg/http/headers"
17+
"github.com/github/github-mcp-server/pkg/inventory"
1718
"github.com/github/github-mcp-server/pkg/observability"
1819
"github.com/github/github-mcp-server/pkg/observability/metrics"
1920
"github.com/github/github-mcp-server/pkg/translations"
@@ -202,7 +203,7 @@ func TestIsFeatureEnabled_WithEnabledFlag(t *testing.T) {
202203
t.Parallel()
203204

204205
// Create a feature checker that returns true for "test_flag"
205-
checker := func(_ context.Context, flagName string) (bool, error) {
206+
checker := func(_ context.Context, flagName inventory.FeatureFlag) (bool, error) {
206207
return flagName == "test_flag", nil
207208
}
208209

@@ -253,7 +254,7 @@ func TestIsFeatureEnabled_EmptyFlagName(t *testing.T) {
253254
t.Parallel()
254255

255256
// Create a feature checker
256-
checker := func(_ context.Context, _ string) (bool, error) {
257+
checker := func(_ context.Context, _ inventory.FeatureFlag) (bool, error) {
257258
return true, nil
258259
}
259260

@@ -388,7 +389,7 @@ func TestIsFeatureEnabled_CheckerError(t *testing.T) {
388389
t.Parallel()
389390

390391
// Create a feature checker that returns an error
391-
checker := func(_ context.Context, _ string) (bool, error) {
392+
checker := func(_ context.Context, _ inventory.FeatureFlag) (bool, error) {
392393
return false, errors.New("checker error")
393394
}
394395

0 commit comments

Comments
 (0)