Refactor feature flags to use functional availability rules

[SamMorrowDrums]

Summary

• replace FeatureFlagEnable, FeatureFlagEnableAll, and FeatureFlagDisable with typed functional FeatureRule predicates
• add public inventory.FeatureFlag and FeatureResolver types for local and remote server consumers
• deduplicate declared feature checks into request-owned state shared by inventory filtering and ToolDependencies.IsFeatureEnabled
• keep handler-only checks lazy and fail closed while caching each resolved flag
• preserve the separate user-controllable AllowedFeatureFlags/header allowlist
• migrate granular, consolidated, MCP Apps, issue dependency, duplicate detection, and file blame gates
• document the new contributor workflow

Breaking change

Inventory items now expose FeatureRule instead of the three legacy feature-gate fields, and FeatureFlagChecker accepts inventory.FeatureFlag instead of string.

Validation

• script/lint
• script/test
• script/generate-docs

[Copilot]

Pull request overview

Refactors feature gating around typed functional rules and shared request-scoped resolution.

Changes:

• Introduces FeatureFlag, FeatureRule, and cached resolution state.
• Migrates inventory items and GitHub tools from legacy flag fields.
• Updates HTTP/stdio integration, tests, generators, and documentation.

Show a summary per file

File	Description
script/print-mcp-diff-configs/main.go	Uses the header-compatible flag accessor.
pkg/inventory/server_tool.go	Replaces legacy tool gates with FeatureRule.
pkg/inventory/resources.go	Adds resource feature rules.
pkg/inventory/registry.go	Collects and pre-resolves required features.
pkg/inventory/registry_test.go	Migrates inventory feature tests.
pkg/inventory/prompts.go	Adds prompt feature rules.
pkg/inventory/filters.go	Evaluates functional rules during filtering.
pkg/inventory/features.go	Implements typed rules and resolution state.
pkg/inventory/features_test.go	Tests predicates and caching.
pkg/inventory/builder.go	Removes the legacy feature filter.
pkg/http/server.go	Adapts HTTP feature resolution to typed flags.
pkg/http/server_test.go	Updates HTTP checker tests.
pkg/http/handler.go	Seeds request-owned feature state.
pkg/http/handler_test.go	Migrates handler feature tests.
pkg/github/ui_tools.go	Migrates the UI tool gate.
pkg/github/ui_tools_test.go	Verifies the UI feature rule.
pkg/github/ui_capability_test.go	Uses typed UI flags.
pkg/github/tools.go	Types granular flags and converts header flags.
pkg/github/tools_validation_test.go	Updates gated-duplicate validation.
pkg/github/server.go	Updates feature configuration documentation.
pkg/github/server_test.go	Updates dependency stubs.
pkg/github/repositories.go	Migrates file-blame gating.
pkg/github/repositories_test.go	Verifies file-blame rules.
pkg/github/pullrequests.go	Migrates consolidated PR rules.
pkg/github/pullrequests_granular.go	Migrates granular PR rules.
pkg/github/issues.go	Migrates consolidated issue rules.
pkg/github/issues_test.go	Updates issue-rule assertions.
pkg/github/issues_granular.go	Migrates granular issue rules.
pkg/github/issue_dependencies.go	Migrates dependency-tool gates.
pkg/github/issue_dependencies_test.go	Updates dependency gate tests.
pkg/github/granular_tools_test.go	Tests granular functional rules.
pkg/github/find_duplicate.go	Migrates duplicate-detection gating.
pkg/github/find_duplicate_test.go	Updates duplicate gate tests.
pkg/github/feature_flags.go	Types flags and defines reusable rules.
pkg/github/feature_flags_test.go	Migrates feature-resolution tests.
pkg/github/dependencies.go	Shares resolution through dependencies.
pkg/github/dependencies_test.go	Updates dependency checker tests.
pkg/github/csv_output_test.go	Migrates CSV rule fixtures.
pkg/github/context_tools_test.go	Uses typed IFC flags.
pkg/github/actions_test.go	Updates functional-rule terminology.
internal/ghmcp/server.go	Adapts stdio feature checking.
docs/insiders-features.md	Documents shared functional resolution.
docs/feature-flags.md	Documents availability rules.
cmd/github-mcp-server/generate_docs.go	Updates default documentation checker.
cmd/github-mcp-server/feature_flag_docs.go	Types feature documentation generation.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 45/45 changed files
• Comments generated: 4
• Review effort level: Balanced

[IrynaKulakova]

A few notes on the new feature-rule machinery. Overall direction looks right — replacing the three annotation fields with one declared predicate is a clear improvement, and the fail-closed semantics of the old featureFlagAllowed are preserved. Comments below are about the resolution state, not the rule model itself.

[IrynaKulakova]

s.mu is held across the s.checker(ctx, feature) call, which makes this non-reentrant: a checker that consults another flag (via ResolveFeature or deps.IsFeatureEnabled) on the same context deadlocks the request. Not an error — a hard hang. I reproduced it with a probe test against this branch:

checker := func(c context.Context, flag FeatureFlag) (bool, error) {
	if flag == "meta" {
		return ResolveFeature(c, nil, "base"), nil
	}
	return true, nil
}
ctx := WithResolvedFeatures(context.Background(), checker, nil)
ResolveFeature(ctx, nil, "meta") // never returns

The same lock also serializes every flag lookup for the request, which matters if the remote server's checker does I/O. Could we resolve outside the lock (double-check on insert) or use a per-flag sync.Once/singleflight?

[IrynaKulakova]

The checker argument is silently ignored whenever context state already exists, which has two consequences worth pinning down:

1. BaseDeps/RequestDeps pass their own featureChecker, but the inventory's checker wins if it installed state first. A consumer wiring different checkers for inventory vs. deps gets whichever ran first, with no signal.
2. Behavior change: IsFeatureEnabled with a nil deps checker previously always returned false. It now returns the inventory-resolved value. The doc comment on BaseDeps.IsFeatureEnabled still says "Returns false if the feature checker is nil", so at minimum that's stale.

Either drop the parameter and make context state authoritative, or document the precedence explicitly here.

[IrynaKulakova]

Undeclared-flag detection only fires if the predicate actually evaluates that branch. Because predicates short-circuit, a rule that reads an undeclared flag in a rarely-taken branch works fine until it doesn't, and then silently drops the tool from the surface with one line on stderr — hard to spot in production.

Declared sets here are 1–2 flags, so NewFeatureRule could probe the predicate over all 2^n combinations at construction time and fail loudly on an undeclared read. Failing that, a package-level guard test over the real inventory would catch it at CI time rather than at request time.

[IrynaKulakova]

This walk gets repeated several times per request: handler.ServeHTTP calls WithResolvedFeatures (→ RequiredFeatures + usesMCPAppsMetadata), then ToolsForRegistration runs both again, then AvailableTools runs requiredToolFeatures a third time. Each pass allocates a rule slice plus a dedup map over the whole inventory.

Since tools/resources/prompts are fixed after Build() (and after ForMCPRequest narrowing), could the required-feature set and the usesMCPAppsMetadata bool be computed once and stored on Inventory?

[IrynaKulakova]

This resolves and caches the flag values before the MCP receiving middlewares run (injectFeatureStateMiddleware, InjectDepsMiddleware). Fine for this repo's header-based checker, but a checker that reads actor/session data populated later in the chain would see a thinner context here, and the resulting value then sticks for the rest of the request.

The RegisterTools doc comment already warns about exactly this class of bug ("HTTP feature checkers that read insiders mode or user identity from ctx would otherwise see context.Background()"). Worth confirming against the remote server's checker before this lands — if it needs the enriched context, the resolution point should move after the middlewares.