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.

[FuzzysTodd]

tijr

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot review overview

🟡 Changes recommended

Cached metadata lacks enforced ownership, and an additional public API break is undocumented.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 2

New issues introduced by this change (2)

Severity	Finding
	pkg/​github/​dependencies.go — This introduces another public source-breaking change that is not listed in the PR's Breaking…
	pkg/​inventory/​registry.go — These caches assume immutable inventory contents, but Builder.SetTools, SetResources, and…

[Copilot]

Copilot review overview

🟡 Changes recommended

Feature resolution can deadlock or incorrectly enable cyclic flags, and cached metadata is not protected from caller mutation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 4 · 1

New issues introduced by this change (3)

Severity	Finding
	pkg/​inventory/​features.go — Cycle detection does not necessarily fail the resolved flag closed. For a checker that computes…
	pkg/​inventory/​features.go — This wait can deadlock on a concurrent cross-feature cycle. If one goroutine owns A and its…
	pkg/​inventory/​builder.go — The metadata cache assumes immutable inventory contents, but Build retains the slices supplied to…

Pre-existing issues (2)

Severity	Finding
	pkg/​inventory/​registry.go — These caches assume immutable inventory contents, but Builder.SetTools, SetResources, and… View comment
	pkg/​github/​dependencies.go — This introduces another public source-breaking change that is not listed in the PR's Breaking… View comment

Suppressed comments (1)

pkg/github/dependencies.go:97

• This changes the public ToolDependencies interface, so external implementations and callers using string variables no longer compile. The PR's breaking-change section lists the inventory fields and FeatureFlagChecker, but not this interface change. Either preserve the string boundary and convert internally, or explicitly document ToolDependencies.IsFeatureEnabled as another breaking change.

	IsFeatureEnabled(ctx context.Context, flag inventory.FeatureFlag) bool

[Copilot]

Copilot review overview

🟡 Changes recommended

RegisterAll does not propagate its resolved feature state across tool, resource, and prompt registration.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 · 1

New issues introduced by this change (1)

Severity	Finding
	pkg/​inventory/​registry.go — The feature state created here is confined to this local ctx. RegisterAll then calls resource…

Pre-existing issues (1)

Severity	Finding
	pkg/​inventory/​registry.go — These caches assume immutable inventory contents, but Builder.SetTools, SetResources, and… View comment

Issues resolved since last review (4)

Severity	Finding
	pkg/​inventory/​builder.go — The metadata cache assumes immutable inventory contents, but Build retains the slices supplied to… View resolved comment
	pkg/​inventory/​features.go — This wait can deadlock on a concurrent cross-feature cycle. If one goroutine owns A and its… View resolved comment
	pkg/​inventory/​features.go — Cycle detection does not necessarily fail the resolved flag closed. For a checker that computes… View resolved comment
	pkg/​github/​dependencies.go — This introduces another public source-breaking change that is not listed in the PR's Breaking… View resolved comment

[Copilot]

Copilot review overview

🔵 Needs a closer look

Nested metadata remains shared despite the ownership contract, and duplicate-rule validation does not verify mutual exclusivity.

Review tier: Balanced
Findings: 1

Pre-existing issues (1)

Severity	Finding
	pkg/​inventory/​registry.go — These caches assume immutable inventory contents, but Builder.SetTools, SetResources, and… View comment

Issues resolved since last review (1)

Severity	Finding
	pkg/​inventory/​registry.go — The feature state created here is confined to this local ctx. RegisterAll then calls resource… View resolved comment

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

pkg/github/tools_validation_test.go:144

• A nonzero functional rule does not imply that duplicate variants are mutually exclusive. Two overlapping rules—or one ungated and one gated tool, since this map is keyed only by name—now bypass this test, and when both are enabled mcp.Server.AddTool silently replaces one definition (pkg/inventory/server_tool.go:159). For each duplicate-name group, evaluate the rules across the union of declared flags and fail whenever more than one variant is enabled for an assignment; retain only the explicit get_label exception.

	// First pass: identify tools that have feature flags (mutually exclusive at runtime)
	for _, tool := range tools {
		if !tool.FeatureRule.IsZero() {
			featureFlagged[tool.Tool.Name] = true

pkg/inventory/builder.go:96

• maps.Clone only copies the outer metadata map. Production tool metadata contains nested mutable maps/slices (for example pkg/github/ui_tools.go:45-48 and pkg/github/context_tools.go:58-62), so mutating the source after SetTools, or mutating nested metadata returned by AvailableTools, still changes the inventory and future registrations despite the new ownership contract. Deep-copy the JSON-like metadata values recursively (and apply the same protection to resource/prompt metadata), then cover a nested mutation in the ownership test.