From 19e25155fa99a0cfc73e18093ad6d177de79ad32 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 29 Sep 2026 15:24:44 +0200 Subject: [PATCH 1/3] Always enable MCP Apps UI; remove remote_mcp_ui_apps flag gate The remote_mcp_ui_apps flag is fully rolled out on the hosted server, but the OSS server still defaulted it off. Make MCP Apps unconditional: - _meta.ui is always emitted, stripped only when the client explicitly does not advertise the io.modelcontextprotocol/ui capability - ui_get is always registered - write tools defer to MCP App forms by default (still opt-out via mcp_apps_disable_form_deferral) - remove MCPAppsFeatureFlag and its AllowedFeatureFlags/InsidersFeatureFlags entries - update tests and regenerate docs Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 10 +++ cmd/github-mcp-server/generate_docs.go | 4 +- docs/feature-flags.md | 64 ---------------- docs/insiders-features.md | 89 ---------------------- docs/server-configuration.md | 53 ++----------- pkg/github/feature_flags.go | 5 -- pkg/github/feature_flags_benchmark_test.go | 2 +- pkg/github/feature_flags_test.go | 12 +-- pkg/github/issues_test.go | 2 +- pkg/github/pullrequests_test.go | 4 +- pkg/github/server.go | 7 +- pkg/github/tools_validation_test.go | 6 -- pkg/github/ui_capability.go | 7 +- pkg/github/ui_capability_test.go | 19 +---- pkg/github/ui_tools.go | 1 - pkg/github/ui_tools_test.go | 3 +- pkg/http/handler_test.go | 75 ++++++------------ pkg/http/server_test.go | 10 +-- pkg/inventory/builder.go | 9 +-- pkg/inventory/registry.go | 80 ++++--------------- pkg/inventory/registry_test.go | 55 ++++--------- 21 files changed, 97 insertions(+), 420 deletions(-) diff --git a/README.md b/README.md index d013303d81..1b6d6cb9db 100644 --- a/README.md +++ b/README.md @@ -704,6 +704,7 @@ The following sets of tools are available: person Context - **get_me** - Get my user profile + - **MCP App UI**: `ui://github-mcp-server/get-me` - No parameters required - **get_team_members** - Get team members @@ -715,6 +716,12 @@ The following sets of tools are available: - **OAuth Challenge Scopes**: `read:org` - `user`: Username to get teams for. If not provided, uses the authenticated user. (string, optional) +- **ui_get** - Get UI data + - **OAuth Challenge Scopes**: `repo`, `read:org` + - `method`: The type of data to fetch (string, required) + - `owner`: Repository owner (required for all methods) (string, required) + - `repo`: Repository name (required for labels, assignees, milestones, branches, issue fields, reviewers) (string, optional) +
@@ -983,6 +990,7 @@ The following sets of tools are available: - **issue_write** - Create or update issue/pull request - **OAuth Challenge Scopes**: `repo` + - **MCP App UI**: `ui://github-mcp-server/issue-write` - `assignees`: Usernames to assign to this issue (string[], optional) - `body`: Issue body content (string, optional) - `duplicate_of`: Issue number that this issue is a duplicate of. Required when state_reason is 'duplicate'. (number, optional) @@ -1238,6 +1246,7 @@ The following sets of tools are available: - **create_pull_request** - Open new pull request - **OAuth Challenge Scopes**: `repo` + - **MCP App UI**: `ui://github-mcp-server/pr-write` - `base`: Branch to merge into (string, required) - `body`: PR description (string, optional) - `draft`: Create as draft PR (boolean, optional) @@ -1316,6 +1325,7 @@ The following sets of tools are available: - **update_pull_request** - Edit pull request - **OAuth Challenge Scopes**: `repo` + - **MCP App UI**: `ui://github-mcp-server/pr-edit` - `base`: New base branch name (string, optional) - `body`: New description (string, optional) - `draft`: Mark pull request as draft (true) or ready for review (false) (boolean, optional) diff --git a/cmd/github-mcp-server/generate_docs.go b/cmd/github-mcp-server/generate_docs.go index a9a6c20f83..736513f6f1 100644 --- a/cmd/github-mcp-server/generate_docs.go +++ b/cmd/github-mcp-server/generate_docs.go @@ -222,9 +222,7 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) { fmt.Fprintf(buf, " - **OAuth Challenge Scopes**: `%s`\n", strings.Join(scopes, "`, `")) } - // MCP App UI metadata (only rendered when the remote_mcp_ui_apps flag - // applied to the inventory; for the no-flags README this section is - // stripped by inventory.ToolsForRegistration before rendering). + // MCP App UI metadata. if ui, ok := tool.Tool.Meta["ui"].(map[string]any); ok { if uri, ok := ui["resourceUri"].(string); ok && uri != "" { fmt.Fprintf(buf, " - **MCP App UI**: `%s`\n", uri) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index ec5282bd54..e48c2488b5 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -86,70 +86,6 @@ as output formatting) won't appear here. -### `remote_mcp_ui_apps` - -- **create_pull_request** - Open new pull request - - **OAuth Challenge Scopes**: `repo` - - **MCP App UI**: `ui://github-mcp-server/pr-write` - - `base`: Branch to merge into (string, required) - - `body`: PR description (string, optional) - - `draft`: Create as draft PR (boolean, optional) - - `head`: Branch containing changes (string, required) - - `maintainer_can_modify`: Allow maintainer edits (boolean, optional) - - `owner`: Repository owner (string, required) - - `repo`: Repository name (string, required) - - `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], optional) - - `title`: PR title (string, required) - -- **get_me** - Get my user profile - - **MCP App UI**: `ui://github-mcp-server/get-me` - - No parameters required - -- **issue_write** - Create or update issue/pull request - - **OAuth Challenge Scopes**: `repo` - - **MCP App UI**: `ui://github-mcp-server/issue-write` - - `assignees`: Usernames to assign to this issue (string[], optional) - - `body`: Issue body content (string, optional) - - `duplicate_of`: Issue number that this issue is a duplicate of. Required when state_reason is 'duplicate'. (number, optional) - - `issue_fields`: Issue field values to set or clear. Each item requires 'field_name' and exactly one of 'value', 'field_option_name', or 'delete: true'. (object[], optional) - - `issue_number`: Issue number to update (number, optional) - - `labels`: Labels to apply to this issue (string[], optional) - - `method`: Write operation to perform on a single issue. - Options are: - - 'create' - creates a new issue. - - 'update' - updates an existing issue. - (string, required) - - `milestone`: Milestone number (number, optional) - - `owner`: Repository owner (string, required) - - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create' and cannot be combined with issue_fields. The new issue is created and attached to this parent in the same operation. (number, optional) - - `parent_owner`: Repository owner of the parent issue. Must be provided with parent_repo. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided. (string, optional) - - `parent_repo`: Repository name of the parent issue. Must be provided with parent_owner. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided. (string, optional) - - `repo`: Repository name (string, required) - - `state`: New state (string, optional) - - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) - - `title`: Issue title (string, optional) - - `type`: Type of this issue. For updates, pass null to remove the current type. Only use if issue types are enabled for this repository. Use list_issue_types to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string | null, optional) - -- **ui_get** - Get UI data - - **OAuth Challenge Scopes**: `repo`, `read:org` - - `method`: The type of data to fetch (string, required) - - `owner`: Repository owner (required for all methods) (string, required) - - `repo`: Repository name (required for labels, assignees, milestones, branches, issue fields, reviewers) (string, optional) - -- **update_pull_request** - Edit pull request - - **OAuth Challenge Scopes**: `repo` - - **MCP App UI**: `ui://github-mcp-server/pr-edit` - - `base`: New base branch name (string, optional) - - `body`: New description (string, optional) - - `draft`: Mark pull request as draft (true) or ready for review (false) (boolean, optional) - - `maintainer_can_modify`: Allow maintainer edits (boolean, optional) - - `owner`: Repository owner (string, required) - - `pullNumber`: Pull request number to update (number, required) - - `repo`: Repository name (string, required) - - `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], optional) - - `state`: New state (string, optional) - - `title`: New title (string, optional) - ### `issues_granular` - **add_issue_comment_reaction** - Add Reaction to Issue or Pull Request Comment diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 4255e5bdd9..5738252c7c 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -26,70 +26,6 @@ The list below is generated from the Go source. It covers tool **inventory and s -### `remote_mcp_ui_apps` - -- **create_pull_request** - Open new pull request - - **OAuth Challenge Scopes**: `repo` - - **MCP App UI**: `ui://github-mcp-server/pr-write` - - `base`: Branch to merge into (string, required) - - `body`: PR description (string, optional) - - `draft`: Create as draft PR (boolean, optional) - - `head`: Branch containing changes (string, required) - - `maintainer_can_modify`: Allow maintainer edits (boolean, optional) - - `owner`: Repository owner (string, required) - - `repo`: Repository name (string, required) - - `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], optional) - - `title`: PR title (string, required) - -- **get_me** - Get my user profile - - **MCP App UI**: `ui://github-mcp-server/get-me` - - No parameters required - -- **issue_write** - Create or update issue/pull request - - **OAuth Challenge Scopes**: `repo` - - **MCP App UI**: `ui://github-mcp-server/issue-write` - - `assignees`: Usernames to assign to this issue (string[], optional) - - `body`: Issue body content (string, optional) - - `duplicate_of`: Issue number that this issue is a duplicate of. Required when state_reason is 'duplicate'. (number, optional) - - `issue_fields`: Issue field values to set or clear. Each item requires 'field_name' and exactly one of 'value', 'field_option_name', or 'delete: true'. (object[], optional) - - `issue_number`: Issue number to update (number, optional) - - `labels`: Labels to apply to this issue (string[], optional) - - `method`: Write operation to perform on a single issue. - Options are: - - 'create' - creates a new issue. - - 'update' - updates an existing issue. - (string, required) - - `milestone`: Milestone number (number, optional) - - `owner`: Repository owner (string, required) - - `parent_issue_number`: Issue number of the parent issue. Only used when method is 'create' and cannot be combined with issue_fields. The new issue is created and attached to this parent in the same operation. (number, optional) - - `parent_owner`: Repository owner of the parent issue. Must be provided with parent_repo. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided. (string, optional) - - `parent_repo`: Repository name of the parent issue. Must be provided with parent_owner. Omit both to use owner and repo. Only used when method is 'create' and parent_issue_number is provided. (string, optional) - - `repo`: Repository name (string, required) - - `state`: New state (string, optional) - - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) - - `title`: Issue title (string, optional) - - `type`: Type of this issue. For updates, pass null to remove the current type. Only use if issue types are enabled for this repository. Use list_issue_types to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string | null, optional) - -- **ui_get** - Get UI data - - **OAuth Challenge Scopes**: `repo`, `read:org` - - `method`: The type of data to fetch (string, required) - - `owner`: Repository owner (required for all methods) (string, required) - - `repo`: Repository name (required for labels, assignees, milestones, branches, issue fields, reviewers) (string, optional) - -- **update_pull_request** - Edit pull request - - **OAuth Challenge Scopes**: `repo` - - **MCP App UI**: `ui://github-mcp-server/pr-edit` - - `base`: New base branch name (string, optional) - - `body`: New description (string, optional) - - `draft`: Mark pull request as draft (true) or ready for review (false) (boolean, optional) - - `maintainer_can_modify`: Allow maintainer edits (boolean, optional) - - `owner`: Repository owner (string, required) - - `pullNumber`: Pull request number to update (number, required) - - `repo`: Repository name (string, required) - - `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], optional) - - `state`: New state (string, optional) - - `title`: New title (string, optional) - ### `file_blame` - **get_file_blame** - Get file blame information @@ -139,31 +75,6 @@ The list below is generated from the Go source. It covers tool **inventory and s --- -## MCP Apps - -[MCP Apps](https://modelcontextprotocol.io/docs/extensions/apps) is an extension to the Model Context Protocol that enables servers to deliver interactive user interfaces to end users. Instead of returning plain text that the LLM must interpret and relay, tools can render forms, profiles, and dashboards right in the chat using MCP Apps. - -This means you can interact with GitHub visually: fill out forms to create issues, see user profiles with avatars, open pull requests — all without leaving your agent chat. - -### Supported tools - -The following tools have MCP Apps UIs: - -| Tool | Description | -|------|-------------| -| `get_me` | Displays your GitHub user profile with avatar, bio, and stats in a rich card | -| `issue_write` | Opens an interactive form to create or update issues | -| `create_pull_request` | Provides a full PR creation form to create a pull request (or a draft pull request) | - -### Client requirements - -MCP Apps requires a host that supports the [MCP Apps extension](https://modelcontextprotocol.io/docs/extensions/apps). Currently tested and working with: - -- **VS Code Insiders** — enable via the `chat.mcp.apps.enabled` setting -- **Visual Studio Code** — enable via the `chat.mcp.apps.enabled` setting - ---- - ## CSV output for list tools CSV output mode returns supported list tool responses as CSV instead of JSON. This is intended to reduce response context for agents when scanning or summarising lists of GitHub data. diff --git a/docs/server-configuration.md b/docs/server-configuration.md index 42584362d9..25c641153e 100644 --- a/docs/server-configuration.md +++ b/docs/server-configuration.md @@ -345,7 +345,7 @@ As an intentional exception, content authored by trusted bot accounts (currently **Best for:** Users who want early access to experimental features and new tools before they reach general availability. -Insiders Mode unlocks experimental features, such as [MCP Apps](#mcp-apps) support. We created this mode to have a way to roll out experimental features and collect feedback. So if you are using Insiders, please don't hesitate to share your feedback with us! Features in Insiders Mode may change, evolve, or be removed based on user feedback. +Insiders Mode unlocks experimental features, such as [CSV output for list tools](./insiders-features.md#csv-output-for-list-tools). We created this mode to have a way to roll out experimental features and collect feedback. So if you are using Insiders, please don't hesitate to share your feedback with us! Features in Insiders Mode may change, evolve, or be removed based on user feedback. @@ -402,18 +402,18 @@ See [Insiders Features](./insiders-features.md) for a full list of what's availa [MCP Apps](https://modelcontextprotocol.io/docs/extensions/apps) is an extension to the Model Context Protocol that enables servers to deliver interactive user interfaces to end users. Instead of returning plain text that the LLM must interpret and relay, tools can render forms, profiles, and dashboards right in the chat. -MCP Apps is enabled by [Insiders Mode](#insiders-mode), or independently via the `remote_mcp_ui_apps` feature flag. +MCP Apps is enabled by default. Tools with MCP Apps UIs advertise them via `_meta.ui` to clients that support the [MCP Apps extension](https://modelcontextprotocol.io/docs/extensions/apps); the metadata is omitted for clients that do not advertise the `io.modelcontextprotocol/ui` capability. To keep MCP App result views enabled while making write tools execute directly -instead of first opening an interactive form, also enable the -`mcp_apps_disable_form_deferral` feature flag. For the remote server, send both -flags in the request header: +instead of first opening an interactive form, enable the +`mcp_apps_disable_form_deferral` feature flag. For the remote server, send the +flag in the request header: ```http -X-MCP-Features: remote_mcp_ui_apps,mcp_apps_disable_form_deferral +X-MCP-Features: mcp_apps_disable_form_deferral ``` -For the local server, pass both flags to `--features`. +For the local server, pass it to `--features`. **Supported tools:** @@ -422,47 +422,10 @@ For the local server, pass both flags to `--features`. | `get_me` | Displays your GitHub user profile with avatar, bio, and stats in a rich card | | `issue_write` | Opens an interactive form to create or update issues | | `create_pull_request` | Provides a full PR creation form to create a pull request (or a draft pull request) | +| `update_pull_request` | Opens an interactive form to edit a pull request | **Client requirements:** MCP Apps requires a host that supports the [MCP Apps extension](https://modelcontextprotocol.io/docs/extensions/apps). Currently tested with VS Code (`chat.mcp.apps.enabled` setting). -
Remote ServerLocal Server
- - - - - -
Remote ServerLocal Server
- -```json -{ - "type": "http", - "url": "https://api.githubcopilot.com/mcp/", - "headers": { - "X-MCP-Features": "remote_mcp_ui_apps" - } -} -``` - - - -```json -{ - "type": "stdio", - "command": "go", - "args": [ - "run", - "./cmd/github-mcp-server", - "stdio", - "--features=remote_mcp_ui_apps" - ], - "env": { - "GITHUB_PERSONAL_ACCESS_TOKEN": "${input:github_token}" - } -} -``` - -
- --- ### Scope Filtering diff --git a/pkg/github/feature_flags.go b/pkg/github/feature_flags.go index 94f335ebe5..321d8fa3bb 100644 --- a/pkg/github/feature_flags.go +++ b/pkg/github/feature_flags.go @@ -6,9 +6,6 @@ import ( "github.com/github/github-mcp-server/pkg/inventory" ) -// MCPAppsFeatureFlag is the feature flag name for MCP Apps (interactive UI forms). -const MCPAppsFeatureFlag = "remote_mcp_ui_apps" - // MCPAppsDisableFormDeferralFeatureFlag disables handing write-tool calls off // to MCP App forms while preserving MCP Apps UI metadata and result views. const MCPAppsDisableFormDeferralFeatureFlag = "mcp_apps_disable_form_deferral" @@ -47,7 +44,6 @@ const FeatureFlagThreadResolutionReason = "thread_resolution_reason" // Only flags in this list are accepted; unknown flags are silently ignored. // This is the single source of truth for which flags are user-controllable. var AllowedFeatureFlags = []string{ - MCPAppsFeatureFlag, MCPAppsDisableFormDeferralFeatureFlag, FeatureFlagCSVOutput, FeatureFlagIFCLabels, @@ -64,7 +60,6 @@ var AllowedFeatureFlags = []string{ // This is the single source of truth for what "insiders" means in terms of // feature flag expansion. var InsidersFeatureFlags = []string{ - MCPAppsFeatureFlag, FeatureFlagCSVOutput, FeatureFlagFileBlame, FeatureFlagIssueDependencies, diff --git a/pkg/github/feature_flags_benchmark_test.go b/pkg/github/feature_flags_benchmark_test.go index 79aa17d55d..97d4951ca6 100644 --- a/pkg/github/feature_flags_benchmark_test.go +++ b/pkg/github/feature_flags_benchmark_test.go @@ -146,7 +146,7 @@ func featureBenchmarkDistributions() []featureBenchmarkDistribution { { name: "mixed", enabled: map[string]bool{ - MCPAppsFeatureFlag: true, + FeatureFlagCSVOutput: true, FeatureFlagFileBlame: true, FeatureFlagIssuesGranular: true, FeatureFlagIssueDependencies: true, diff --git a/pkg/github/feature_flags_test.go b/pkg/github/feature_flags_test.go index cc3fbf0837..241d02c5dd 100644 --- a/pkg/github/feature_flags_test.go +++ b/pkg/github/feature_flags_test.go @@ -149,12 +149,12 @@ func TestResolveFeatureFlags(t *testing.T) { name: "no features, no insiders", enabledFeatures: nil, expectedFlags: nil, - unexpectedFlags: []string{MCPAppsFeatureFlag}, + unexpectedFlags: []string{FeatureFlagCSVOutput}, }, { name: "explicit feature enabled", - enabledFeatures: []string{MCPAppsFeatureFlag}, - expectedFlags: []string{MCPAppsFeatureFlag}, + enabledFeatures: []string{FeatureFlagCSVOutput}, + expectedFlags: []string{FeatureFlagCSVOutput}, }, { name: "MCP Apps form deferral can be disabled directly", @@ -191,8 +191,8 @@ func TestResolveFeatureFlags(t *testing.T) { }, { name: "mix of known and unknown flags", - enabledFeatures: []string{MCPAppsFeatureFlag, "unknown_flag"}, - expectedFlags: []string{MCPAppsFeatureFlag}, + enabledFeatures: []string{FeatureFlagCSVOutput, "unknown_flag"}, + expectedFlags: []string{FeatureFlagCSVOutput}, unexpectedFlags: []string{"unknown_flag"}, }, { @@ -214,7 +214,7 @@ func TestResolveFeatureFlags(t *testing.T) { }, { name: "explicit plus insiders deduplicates", - enabledFeatures: []string{MCPAppsFeatureFlag}, + enabledFeatures: []string{FeatureFlagCSVOutput}, insidersMode: true, expectedFlags: InsidersFeatureFlags, }, diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 7138148c21..e2abb2e8de 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2216,7 +2216,7 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) { deps := BaseDeps{ Client: client, GQLClient: githubv4.NewClient(nil), - featureChecker: featureCheckerFor(MCPAppsFeatureFlag), + featureChecker: featureCheckerFor(), } handler := serverTool.Handler(deps) diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index c0e392aea6..88facf8c51 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -2966,7 +2966,7 @@ func Test_CreatePullRequest_MCPAppsFeature_UIGate(t *testing.T) { deps := BaseDeps{ Client: client, GQLClient: githubv4.NewClient(nil), - featureChecker: featureCheckerFor(MCPAppsFeatureFlag), + featureChecker: featureCheckerFor(), } handler := serverTool.Handler(deps) @@ -3068,7 +3068,7 @@ func Test_UpdatePullRequest_MCPAppsFeature_UIGate(t *testing.T) { deps := BaseDeps{ Client: client, GQLClient: githubv4.NewClient(nil), - featureChecker: featureCheckerFor(MCPAppsFeatureFlag), + featureChecker: featureCheckerFor(), } handler := serverTool.Handler(deps) diff --git a/pkg/github/server.go b/pkg/github/server.go index 6e9b5b7566..4ce1f965b1 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -125,10 +125,9 @@ func NewMCPServer(ctx context.Context, cfg *MCPServerConfig, deps ToolDependenci inv.RegisterAll(ctx, ghServer, deps, cfg.ToolHandlerMiddleware...) // Register MCP App UI resources whenever the embedded UI assets are - // available. The resources are static HTML and are only referenced by - // tools when the remote_mcp_ui_apps feature flag is enabled for the - // request (the inventory strips the _meta.ui block otherwise via - // stripMCPAppsMetadata), so registering them unconditionally is safe. + // available. The resources are static HTML referenced by tools' _meta.ui + // block (the inventory strips that block for clients that do not support + // MCP Apps), so registering them unconditionally is safe. // Registering here — rather than in the stdio bootstrap — ensures the // remote/HTTP server also serves them, fixing the "-32002 Resource not // found" error clients hit after the tool returns a ui:// URI. diff --git a/pkg/github/tools_validation_test.go b/pkg/github/tools_validation_test.go index 3eacc8e1c2..dc3e758dd3 100644 --- a/pkg/github/tools_validation_test.go +++ b/pkg/github/tools_validation_test.go @@ -234,12 +234,6 @@ func TestFeatureRulesOverlap(t *testing.T) { assert.False(t, featureDeclarationsOverlap([]inventory.ServerTool{enabled, disabled})) } -func TestMCPAppsFeatureFlagMatchesInventory(t *testing.T) { - inv, err := NewInventory(stubTranslation).Build() - require.NoError(t, err) - assert.Contains(t, inv.RequiredFeatures(), inventory.FeatureFlag(MCPAppsFeatureFlag)) -} - // TestNoDuplicateResourceNames ensures all resources have unique names func TestNoDuplicateResourceNames(t *testing.T) { resources := AllResources(stubTranslation) diff --git a/pkg/github/ui_capability.go b/pkg/github/ui_capability.go index 3de6a39b74..b4f5ddf281 100644 --- a/pkg/github/ui_capability.go +++ b/pkg/github/ui_capability.go @@ -62,16 +62,15 @@ func hasNonFormParams(args map[string]any, formParams map[string]struct{}) bool // shouldDeferToForm is the single source of truth for the show/defer decision // shared by the form-backed write tools (create_pull_request, // update_pull_request, issue_write). It reports whether a call should be handed -// off to its MCP App form instead of executing now: defer only when MCP Apps -// are enabled, form deferral has not been disabled, the client can render UI, +// off to its MCP App form instead of executing now: defer only when form +// deferral has not been disabled, the client can render UI, // the call is not itself a form submission, and every supplied parameter can // be represented by the form (formParams is the tool's form-parameter // allowlist). When it returns false the handler executes directly; the host may // still render the tool's view, which renders the result rather than an input // form. func shouldDeferToForm(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any, formParams map[string]struct{}) bool { - return deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && - !deps.IsFeatureEnabled(ctx, MCPAppsDisableFormDeferralFeatureFlag) && + return !deps.IsFeatureEnabled(ctx, MCPAppsDisableFormDeferralFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted(args) && !hasNonFormParams(args, formParams) diff --git a/pkg/github/ui_capability_test.go b/pkg/github/ui_capability_test.go index 812d6fde89..7d3ee34c4a 100644 --- a/pkg/github/ui_capability_test.go +++ b/pkg/github/ui_capability_test.go @@ -100,27 +100,14 @@ func Test_shouldDeferToForm_featureFlags(t *testing.T) { want bool }{ { - name: "MCP Apps enabled defers to form", - enabledFlags: []inventory.FeatureFlag{MCPAppsFeatureFlag}, - want: true, + name: "defers to form by default", + want: true, }, { - name: "form deferral disabled executes directly", - enabledFlags: []inventory.FeatureFlag{ - MCPAppsFeatureFlag, - MCPAppsDisableFormDeferralFeatureFlag, - }, - want: false, - }, - { - name: "form deferral opt-out does not enable MCP Apps", + name: "form deferral disabled executes directly", enabledFlags: []inventory.FeatureFlag{MCPAppsDisableFormDeferralFeatureFlag}, want: false, }, - { - name: "MCP Apps disabled executes directly", - want: false, - }, } for _, tc := range tests { diff --git a/pkg/github/ui_tools.go b/pkg/github/ui_tools.go index b8d6cd31a5..60ff65a742 100644 --- a/pkg/github/ui_tools.go +++ b/pkg/github/ui_tools.go @@ -98,7 +98,6 @@ func UIGet(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } }) - st.FeatureRule = featureEnabledRule(MCPAppsFeatureFlag) return st } diff --git a/pkg/github/ui_tools_test.go b/pkg/github/ui_tools_test.go index d400752bdf..8a515d606d 100644 --- a/pkg/github/ui_tools_test.go +++ b/pkg/github/ui_tools_test.go @@ -12,7 +12,6 @@ import ( "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/internal/toolsnaps" - "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/translations" "github.com/google/go-github/v89/github" "github.com/google/jsonschema-go/jsonschema" @@ -106,7 +105,7 @@ func Test_UIGet(t *testing.T) { assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "repo") assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"method", "owner"}) assert.True(t, tool.Annotations.ReadOnlyHint, "ui_get should be read-only") - assert.Equal(t, []inventory.FeatureFlag{MCPAppsFeatureFlag}, serverTool.FeatureRule.Features()) + assert.Empty(t, serverTool.FeatureRule.Features(), "ui_get should not be feature-gated") // ui_get must be app-only so the host hides it from the agent's tool list // while keeping it callable by the views (MCP Apps 2026-01-26 spec). diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index ea37ec2a6a..c9fa1de095 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -1299,62 +1299,36 @@ func TestSubscriptionsListenIsRejected(t *testing.T) { } } -// TestInsidersRoutePreservesUIMeta is a regression test for the bug where -// _meta.ui was stripped from tools/list responses on the HTTP /insiders route. -// -// Before the fix: -// - buildStaticInventory called Build() on a builder configured with the -// HTTP feature checker (which reads insiders mode from the request ctx). -// - Build() invoked checkFeatureFlag(context.Background()) — bg ctx has no -// insiders mode, so the FF reported MCP Apps off, and stripMCPAppsMetadata -// ran eagerly against the static tool slice at server startup. -// - Per-request inventory factories then served pre-stripped tools regardless -// of whether the request actually came in via /insiders. -// -// After the fix: -// - Build() no longer touches MCP Apps metadata. -// - RegisterTools applies the strip per-request, using the request context -// where the HTTP feature checker correctly observes insiders mode. -func TestInsidersRoutePreservesUIMeta(t *testing.T) { +// TestUIMetaPreservedByDefault verifies that _meta.ui is preserved in the +// registered tool surface on both the /insiders route and the default route: +// MCP Apps UI metadata is no longer gated behind a feature flag. +func TestUIMetaPreservedByDefault(t *testing.T) { const uiURI = "ui://test/widget" uiTool := mockTool("with_ui", "repos", true) uiTool.Tool.Meta = mcp.Meta{"ui": map[string]any{"resourceUri": uiURI}} - checker := createHTTPFeatureChecker(nil, false) - build := func() *inventory.Inventory { - inv, err := inventory.NewBuilder(). - SetTools([]inventory.ServerTool{uiTool}). - WithFeatureChecker(checker). - WithToolsets([]string{"all"}). - Build() - require.NoError(t, err) - return inv - } - - // Simulate a /insiders request: ctx has insiders mode set. - insidersCtx := ghcontext.WithInsidersMode(context.Background(), true) + inv, err := inventory.NewBuilder(). + SetTools([]inventory.ServerTool{uiTool}). + WithFeatureChecker(createHTTPFeatureChecker(nil, false)). + WithToolsets([]string{"all"}). + Build() + require.NoError(t, err) - // AvailableTools no longer strips _meta.ui (post-fix), regardless of ctx. - // The strip lives in RegisterTools, gated on the per-request FF check. - insidersTools := build().AvailableTools(insidersCtx) - plainTools := build().AvailableTools(context.Background()) - - // On the /insiders path, the FF check returns true → no strip → _meta preserved. - enabled, _ := checker(insidersCtx, "remote_mcp_ui_apps") - require.True(t, enabled, "FF should be on for /insiders ctx") - require.Len(t, insidersTools, 1) - require.NotNil(t, insidersTools[0].Tool.Meta, "_meta should be present on /insiders") - require.Equal(t, uiURI, insidersTools[0].Tool.Meta["ui"].(map[string]any)["resourceUri"]) - - // On the non-insiders path, RegisterTools strips _meta.ui. - plainEnabled, _ := checker(context.Background(), "remote_mcp_ui_apps") - require.False(t, plainEnabled, "FF should be off for non-insiders ctx") - require.Len(t, plainTools, 1) + for name, ctx := range map[string]context.Context{ + "insiders": ghcontext.WithInsidersMode(context.Background(), true), + "default": context.Background(), + } { + t.Run(name, func(t *testing.T) { + tools := inv.ToolsForRegistration(ctx) + require.Len(t, tools, 1) + require.NotNil(t, tools[0].Tool.Meta, "_meta should be present") + require.Equal(t, uiURI, tools[0].Tool.Meta["ui"].(map[string]any)["resourceUri"]) + }) + } } -// TestUIMetaStrippedWhenClientLacksCapability verifies that even on the -// /insiders path (where the feature flag is on), UI metadata is stripped from -// tools/list responses when the client did NOT advertise the +// TestUIMetaStrippedWhenClientLacksCapability verifies that UI metadata is +// stripped from tools/list responses when the client did NOT advertise the // io.modelcontextprotocol/ui extension capability. Per the 2026-01-26 MCP // Apps spec, servers SHOULD check client capabilities before exposing // UI-enabled tools. @@ -1387,10 +1361,9 @@ func TestUIMetaStrippedWhenClientLacksCapability(t *testing.T) { require.NotNil(t, preserved[0].Tool.Meta["ui"], "_meta.ui should be preserved when client advertises UI capability") require.Equal(t, uiURI, preserved[0].Tool.Meta["ui"].(map[string]any)["resourceUri"]) - // Unknown capability falls through to the FF gate (insiders ctx → kept). unknown := build().ToolsForRegistration(insidersCtx) require.Len(t, unknown, 1) - require.NotNil(t, unknown[0].Tool.Meta["ui"], "_meta.ui should be preserved when capability is unknown and FF is on") + require.NotNil(t, unknown[0].Tool.Meta["ui"], "_meta.ui should be preserved when capability is unknown") } // TestMaxRequestBodyBytes checks the effective limit and, critically, that it diff --git a/pkg/http/server_test.go b/pkg/http/server_test.go index 500bb40611..903194082e 100644 --- a/pkg/http/server_test.go +++ b/pkg/http/server_test.go @@ -374,12 +374,6 @@ func TestCreateHTTPFeatureChecker(t *testing.T) { headerFeatures: []string{github.FeatureFlagPullRequestsGranular}, wantEnabled: true, }, - { - name: "MCP Apps flag accepted from header", - flagName: github.MCPAppsFeatureFlag, - headerFeatures: []string{github.MCPAppsFeatureFlag}, - wantEnabled: true, - }, { name: "MCP Apps form deferral opt-out accepted from header", flagName: github.MCPAppsDisableFormDeferralFeatureFlag, @@ -417,8 +411,8 @@ func TestCreateHTTPFeatureChecker(t *testing.T) { wantEnabled: false, }, { - name: "insiders mode enables MCP Apps without header", - flagName: github.MCPAppsFeatureFlag, + name: "insiders mode enables CSV output without header", + flagName: github.FeatureFlagCSVOutput, insidersMode: true, wantEnabled: true, }, diff --git a/pkg/inventory/builder.go b/pkg/inventory/builder.go index 60bda764b6..28c3aa4823 100644 --- a/pkg/inventory/builder.go +++ b/pkg/inventory/builder.go @@ -14,11 +14,6 @@ var ( ErrUnknownTools = errors.New("unknown tools specified in WithTools") ) -// mcpAppsFeatureFlag is the feature flag name that controls MCP Apps UI metadata. -// This is defined here to avoid importing pkg/github (which imports pkg/inventory). -// The value must match github.MCPAppsFeatureFlag. -const mcpAppsFeatureFlag FeatureFlag = "remote_mcp_ui_apps" - // ToolFilter is a function that determines if a tool should be included. // Returns true if the tool should be included, false to exclude it. type ToolFilter func(ctx context.Context, tool *ServerTool) (bool, error) @@ -374,13 +369,13 @@ func (b *Builder) processToolsets() (map[ToolsetID]bool, []string, []ToolsetID, return enabledToolsets, unrecognized, allToolsetIDs, validIDs, defaultToolsetIDList, descriptions } -// mcpAppsMetaKeys lists the Meta keys controlled by the remote_mcp_ui_apps feature flag. +// mcpAppsMetaKeys lists the Meta keys that carry MCP Apps UI metadata. var mcpAppsMetaKeys = []string{ "ui", // MCP Apps UI metadata } // stripMCPAppsMetadata removes MCP Apps UI metadata from tools when the -// remote_mcp_ui_apps feature flag is not enabled. +// client does not support MCP Apps UI. func stripMCPAppsMetadata(tools []ServerTool) []ServerTool { result := make([]ServerTool, 0, len(tools)) for _, tool := range tools { diff --git a/pkg/inventory/registry.go b/pkg/inventory/registry.go index 9439e6f201..38070e95d2 100644 --- a/pkg/inventory/registry.go +++ b/pkg/inventory/registry.go @@ -179,25 +179,16 @@ func (r *Inventory) ToolsetDescriptions() map[ToolsetID]string { // diagnostics that need the same view of the tool surface the server would // register. // -// The strip applies when EITHER of the following is true: -// -// - The remote_mcp_ui_apps feature flag is not enabled in ctx (server-side gate). -// - The client explicitly did not advertise the io.modelcontextprotocol/ui -// extension capability (per the 2026-01-26 MCP Apps spec, servers SHOULD -// check client capabilities before exposing UI-enabled tools). When the -// capability is unknown (e.g. stdio paths that do not populate the -// context flag) the feature-flag gate is the sole source of truth. +// MCP Apps UI metadata is stripped only when the client explicitly did not +// advertise the io.modelcontextprotocol/ui extension capability (per the +// 2026-01-26 MCP Apps spec, servers SHOULD check client capabilities before +// exposing UI-enabled tools). When the capability is unknown (e.g. stdio +// paths that do not populate the context flag) the metadata is kept. func (r *Inventory) ToolsForRegistration(ctx context.Context) []ServerTool { ctx = WithFeatureState(ctx, r.featureChecker) tools := r.availableTools(ctx) - if present, checkFeature := mcpAppsMetadataStatus(ctx, tools); present { - featureEnabled := false - if checkFeature { - featureEnabled = r.checkFeatureFlag(ctx, mcpAppsFeatureFlag) - } - if shouldStripMCPAppsMetadata(ctx, featureEnabled) { - tools = stripMCPAppsMetadata(tools) - } + if shouldStripMCPAppsMetadata(ctx) { + tools = stripMCPAppsMetadata(tools) } return tools } @@ -215,9 +206,6 @@ func (r *Inventory) RequiredFeatures() []FeatureFlag { for i := range r.prompts { features = appendFeatureDeclaration(features, r.prompts[i].FeatureRule) } - if r.usesMCPAppsMetadata() { - features = appendUniqueFeature(features, mcpAppsFeatureFlag) - } slices.Sort(features) return features } @@ -228,37 +216,6 @@ func (r *Inventory) WithFeatureState(ctx context.Context) context.Context { return WithFeatureState(ctx, r.featureChecker) } -func (r *Inventory) usesMCPAppsMetadata() bool { - for i := range r.tools { - if toolUsesMCPAppsMetadata(&r.tools[i]) { - return true - } - } - return false -} - -func mcpAppsMetadataStatus(ctx context.Context, tools []ServerTool) (present, checkFeature bool) { - for i := range tools { - if !toolUsesMCPAppsMetadata(&tools[i]) { - continue - } - present = true - if featureDecisionForToolAvailability(ctx, tools[i].availability()) != includeToolWithoutFeatureRule { - return true, true - } - } - return present, false -} - -func toolUsesMCPAppsMetadata(tool *ServerTool) bool { - for _, key := range mcpAppsMetaKeys { - if _, ok := tool.Tool.Meta[key]; ok { - return true - } - } - return false -} - func appendFeatureDeclaration(features []FeatureFlag, rule FeatureRule) []FeatureFlag { for _, feature := range rule.features { features = appendUniqueFeature(features, feature) @@ -267,29 +224,20 @@ func appendFeatureDeclaration(features []FeatureFlag, rule FeatureRule) []Featur } // shouldStripMCPAppsMetadata centralises the strip decision so the same logic -// is exercised by tests and by RegisterTools. -func shouldStripMCPAppsMetadata(ctx context.Context, featureFlagEnabled bool) bool { - if !featureFlagEnabled { - return true - } - // Feature flag is on. Respect the client capability if it is known. - if supported, ok := ghcontext.HasUISupport(ctx); ok && !supported { - return true - } - return false +// is exercised by tests and by RegisterTools. Metadata is stripped only when +// the client is known not to support MCP Apps UI. +func shouldStripMCPAppsMetadata(ctx context.Context) bool { + supported, ok := ghcontext.HasUISupport(ctx) + return ok && !supported } // RegisterTools registers all available tools with the server using the provided dependencies. // The context is used for feature flag evaluation and client capability checks. // // MCP Apps UI metadata (`_meta.ui`) is stripped from the registered tools when -// either the MCP Apps feature flag is not enabled for this request, or the -// client did not advertise the io.modelcontextprotocol/ui extension. The +// the client did not advertise the io.modelcontextprotocol/ui extension. The // strip happens here (rather than at Build() time) so the per-request -// context is in scope — HTTP feature checkers that read insiders mode or -// user identity from ctx would otherwise see context.Background() and -// falsely report the flag off, even when the actual request arrived on the -// /insiders route. +// context, which carries the client capability, is in scope. func (r *Inventory) RegisterTools(ctx context.Context, s *mcp.Server, deps any, middleware ...ToolHandlerMiddleware) { tools := r.ToolsForRegistration(ctx) addToolAvailabilityMiddleware(s, tools) diff --git a/pkg/inventory/registry_test.go b/pkg/inventory/registry_test.go index 2e21fc632c..b1cac1c6f6 100644 --- a/pkg/inventory/registry_test.go +++ b/pkg/inventory/registry_test.go @@ -1334,7 +1334,7 @@ func TestMetadataBehaviorRemainsLive(t *testing.T) { meta["typed_map"].(map[string]string)["value"] = "changed" meta["bytes"].([]byte)[0] = 'X' - require.Contains(t, inv.RequiredFeatures(), mcpAppsFeatureFlag) + require.Empty(t, inv.RequiredFeatures()) available := inv.AllTools() require.Equal(t, "changed", available[0].Tool.Meta["typed_map"].(map[string]string)["value"]) require.Equal(t, byte('X'), available[0].Tool.Meta["bytes"].([]byte)[0]) @@ -2088,15 +2088,14 @@ func mockToolWithMeta(name string, toolsetID string, meta map[string]any) Server ) } -func TestWithMCPApps_DisabledStripsUIMetadata(t *testing.T) { +func TestWithMCPApps_UnsupportedClientStripsUIMetadata(t *testing.T) { toolWithUI := mockToolWithMeta("tool_with_ui", "toolset1", map[string]any{ "ui": map[string]any{"html": "
hello
"}, "description": "kept", }) - // Default: MCP Apps is disabled - UI meta should be stripped on registration. reg := mustBuild(t, NewBuilder().SetTools([]ServerTool{toolWithUI}).WithToolsets([]string{"all"})) - registered := captureRegisteredTools(context.Background(), t, reg) + registered := captureRegisteredTools(ghcontext.WithUISupport(context.Background(), false), t, reg) require.Len(t, registered, 1) if registered[0].Meta["ui"] != nil { @@ -2107,27 +2106,22 @@ func TestWithMCPApps_DisabledStripsUIMetadata(t *testing.T) { } } -func TestWithMCPApps_EnabledPreservesUIMetadata(t *testing.T) { +func TestWithMCPApps_PreservesUIMetadataByDefault(t *testing.T) { uiData := map[string]any{"html": "
hello
"} toolWithUI := mockToolWithMeta("tool_with_ui", "toolset1", map[string]any{ "ui": uiData, "description": "kept", }) - // Feature checker enables MCP Apps - UI meta should be preserved - mcpAppsChecker := func(_ context.Context, flag string) (bool, error) { - return flag == string(mcpAppsFeatureFlag), nil - } reg := mustBuild(t, NewBuilder(). SetTools([]ServerTool{toolWithUI}). - WithToolsets([]string{"all"}). - WithFeatureChecker(mcpAppsChecker)) - available := reg.AvailableTools(context.Background()) + WithToolsets([]string{"all"})) + available := reg.ToolsForRegistration(context.Background()) require.Len(t, available, 1) // UI metadata should be preserved if available[0].Tool.Meta["ui"] == nil { - t.Errorf("Expected 'ui' meta to be preserved with MCP Apps enabled") + t.Errorf("Expected 'ui' meta to be preserved") } // Other metadata should also be preserved if available[0].Tool.Meta["description"] != "kept" { @@ -2142,7 +2136,6 @@ func TestWithMCPApps_ToolsWithoutUIMetaUnaffected(t *testing.T) { }) toolNilMeta := mockTool("tool_nil_meta", "toolset1", true) - // Test with MCP Apps disabled (default) - non-UI meta should be unaffected reg := mustBuild(t, NewBuilder(). SetTools([]ServerTool{toolNoUI, toolNilMeta}). WithToolsets([]string{"all"})) @@ -2183,7 +2176,7 @@ func TestWithMCPApps_UIOnlyMetaBecomesNil(t *testing.T) { reg := mustBuild(t, NewBuilder(). SetTools([]ServerTool{toolUIOnly}). WithToolsets([]string{"all"})) - registered := captureRegisteredTools(context.Background(), t, reg) + registered := captureRegisteredTools(ghcontext.WithUISupport(context.Background(), false), t, reg) require.Len(t, registered, 1) if registered[0].Meta != nil { @@ -2311,8 +2304,8 @@ func TestWithMCPApps_DoesNotMutateOriginalTools(t *testing.T) { tool := mockToolWithMeta("test", "toolset1", originalMeta) tools := []ServerTool{tool} - // Build with MCP Apps disabled (default) - should strip ui - _ = mustBuild(t, NewBuilder().SetTools(tools).WithToolsets([]string{"all"})) + reg := mustBuild(t, NewBuilder().SetTools(tools).WithToolsets([]string{"all"})) + _ = reg.ToolsForRegistration(ghcontext.WithUISupport(context.Background(), false)) // Original tool should be unchanged require.Equal(t, "data", tools[0].Tool.Meta["ui"], "original tool should not be mutated") @@ -2484,52 +2477,36 @@ func captureRegisteredTools(ctx context.Context, t *testing.T, reg *Inventory) [ } // TestShouldStripMCPAppsMetadata verifies the spec-conformant strip decision: -// strip when the feature flag is off, OR when the client explicitly does not -// advertise the io.modelcontextprotocol/ui extension. +// strip only when the client explicitly does not advertise the +// io.modelcontextprotocol/ui extension. func TestShouldStripMCPAppsMetadata(t *testing.T) { t.Parallel() tests := []struct { name string setupCtx func() context.Context - ffOn bool want bool }{ { - name: "FF off, capability unknown -> strip", - setupCtx: context.Background, - ffOn: false, - want: true, - }, - { - name: "FF off, capability present -> strip (FF wins)", - setupCtx: func() context.Context { return ghcontext.WithUISupport(context.Background(), true) }, - ffOn: false, - want: true, - }, - { - name: "FF on, capability unknown -> keep", + name: "capability unknown -> keep", setupCtx: context.Background, - ffOn: true, want: false, }, { - name: "FF on, capability present -> keep", + name: "capability present -> keep", setupCtx: func() context.Context { return ghcontext.WithUISupport(context.Background(), true) }, - ffOn: true, want: false, }, { - name: "FF on, capability explicitly absent -> strip", + name: "capability explicitly absent -> strip", setupCtx: func() context.Context { return ghcontext.WithUISupport(context.Background(), false) }, - ffOn: true, want: true, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := shouldStripMCPAppsMetadata(tc.setupCtx(), tc.ffOn) + got := shouldStripMCPAppsMetadata(tc.setupCtx()) require.Equal(t, tc.want, got) }) } From 55a343f7d387845a6b8f55f2624db804ceb29078 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 29 Sep 2026 16:15:31 +0200 Subject: [PATCH 2/3] Remove unused Inventory.checkFeatureFlag Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/inventory/filters.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/inventory/filters.go b/pkg/inventory/filters.go index b3dfbeb912..bcc0d58c5f 100644 --- a/pkg/inventory/filters.go +++ b/pkg/inventory/filters.go @@ -16,12 +16,6 @@ func (r *Inventory) isToolsetEnabled(toolsetID ToolsetID) bool { return true } -// checkFeatureFlag checks a feature flag using the feature checker. -// Returns false if checker is nil or returns an error (errors are logged). -func (r *Inventory) checkFeatureFlag(ctx context.Context, flagName FeatureFlag) bool { - return ResolveFeature(ctx, r.featureChecker, flagName) -} - // isToolEnabled checks if a specific tool is enabled based on current filters. // Filter evaluation order: // 1. Tool.Enabled (tool self-filtering) From d8d2c7127d04b2644e21c4d5f13c7ecec5a2ad28 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 29 Sep 2026 16:43:41 +0200 Subject: [PATCH 3/3] Omit app-only tools for clients without MCP Apps support When a client does not advertise io.modelcontextprotocol/ui, stripping _meta.ui from ui_get turned it into an ordinary model-visible tool, violating its app-only contract. Now that ui_get is no longer feature gated, omit tools whose ui.visibility excludes "model" instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/github/ui_tools_test.go | 20 ++++++++++++++++++++ pkg/inventory/builder.go | 31 ++++++++++++++++++++++++++++++- pkg/inventory/registry.go | 6 ++++-- pkg/inventory/registry_test.go | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 3 deletions(-) diff --git a/pkg/github/ui_tools_test.go b/pkg/github/ui_tools_test.go index 8a515d606d..8bf28c3674 100644 --- a/pkg/github/ui_tools_test.go +++ b/pkg/github/ui_tools_test.go @@ -12,6 +12,7 @@ import ( "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/internal/toolsnaps" + ghcontext "github.com/github/github-mcp-server/pkg/context" "github.com/github/github-mcp-server/pkg/translations" "github.com/google/go-github/v89/github" "github.com/google/jsonschema-go/jsonschema" @@ -582,3 +583,22 @@ func Test_marshalUIGetIssueFields_TrimsForUI(t *testing.T) { textField := fields[1].(map[string]any) assert.NotContains(t, textField, "options") } + +func Test_UIGet_OmittedForClientsWithoutUISupport(t *testing.T) { + inv, err := NewInventory(translations.NullTranslationHelper).WithToolsets([]string{"all"}).Build() + require.NoError(t, err) + + hasUIGet := func(ctx context.Context) bool { + for _, tool := range inv.ToolsForRegistration(ctx) { + if tool.Tool.Name == "ui_get" { + return true + } + } + return false + } + + assert.False(t, hasUIGet(ghcontext.WithUISupport(context.Background(), false)), + "app-only ui_get must not be exposed as a model-visible tool to non-UI clients") + assert.True(t, hasUIGet(ghcontext.WithUISupport(context.Background(), true))) + assert.True(t, hasUIGet(context.Background()), "ui_get is kept when UI support is unknown") +} diff --git a/pkg/inventory/builder.go b/pkg/inventory/builder.go index 28c3aa4823..e70df0eed3 100644 --- a/pkg/inventory/builder.go +++ b/pkg/inventory/builder.go @@ -375,10 +375,15 @@ var mcpAppsMetaKeys = []string{ } // stripMCPAppsMetadata removes MCP Apps UI metadata from tools when the -// client does not support MCP Apps UI. +// client does not support MCP Apps UI. App-only tools (ui.visibility without +// "model") are omitted entirely rather than being exposed as ordinary +// model-visible tools. func stripMCPAppsMetadata(tools []ServerTool) []ServerTool { result := make([]ServerTool, 0, len(tools)) for _, tool := range tools { + if isAppOnlyTool(&tool) { + continue + } if stripped := stripMetaKeys(tool, mcpAppsMetaKeys); stripped != nil { result = append(result, *stripped) } else { @@ -388,6 +393,30 @@ func stripMCPAppsMetadata(tools []ServerTool) []ServerTool { return result } +// isAppOnlyTool reports whether a tool declares MCP Apps visibility that +// excludes the model (e.g. `_meta.ui.visibility: ["app"]`). Such tools are +// only meant to be called by MCP App views, never by the model directly. +func isAppOnlyTool(tool *ServerTool) bool { + ui, ok := tool.Tool.Meta["ui"].(map[string]any) + if !ok { + return false + } + var visibility []string + switch v := ui["visibility"].(type) { + case []string: + visibility = v + case []any: + for _, item := range v { + if s, ok := item.(string); ok { + visibility = append(visibility, s) + } + } + default: + return false + } + return len(visibility) > 0 && !slices.Contains(visibility, "model") +} + // stripMetaKeys removes the specified Meta keys from a single tool. // Returns a modified copy if changes were made, nil otherwise. func stripMetaKeys(tool ServerTool, keys []string) *ServerTool { diff --git a/pkg/inventory/registry.go b/pkg/inventory/registry.go index 38070e95d2..02dc5b0fd7 100644 --- a/pkg/inventory/registry.go +++ b/pkg/inventory/registry.go @@ -182,8 +182,10 @@ func (r *Inventory) ToolsetDescriptions() map[ToolsetID]string { // MCP Apps UI metadata is stripped only when the client explicitly did not // advertise the io.modelcontextprotocol/ui extension capability (per the // 2026-01-26 MCP Apps spec, servers SHOULD check client capabilities before -// exposing UI-enabled tools). When the capability is unknown (e.g. stdio -// paths that do not populate the context flag) the metadata is kept. +// exposing UI-enabled tools). In that case app-only tools (whose +// _meta.ui.visibility excludes "model") are omitted entirely. When the +// capability is unknown (e.g. stdio paths that do not populate the context +// flag) the metadata is kept. func (r *Inventory) ToolsForRegistration(ctx context.Context) []ServerTool { ctx = WithFeatureState(ctx, r.featureChecker) tools := r.availableTools(ctx) diff --git a/pkg/inventory/registry_test.go b/pkg/inventory/registry_test.go index b1cac1c6f6..4e827936c6 100644 --- a/pkg/inventory/registry_test.go +++ b/pkg/inventory/registry_test.go @@ -2106,6 +2106,39 @@ func TestWithMCPApps_UnsupportedClientStripsUIMetadata(t *testing.T) { } } +func TestWithMCPApps_UnsupportedClientOmitsAppOnlyTools(t *testing.T) { + appOnly := mockToolWithMeta("app_only", "toolset1", map[string]any{ + "ui": map[string]any{"visibility": []string{"app"}}, + }) + appOnlyAny := mockToolWithMeta("app_only_any", "toolset1", map[string]any{ + "ui": map[string]any{"visibility": []any{"app"}}, + }) + modelAndApp := mockToolWithMeta("model_and_app", "toolset1", map[string]any{ + "ui": map[string]any{"resourceUri": "ui://x", "visibility": []string{"model", "app"}}, + }) + noVisibility := mockToolWithMeta("no_visibility", "toolset1", map[string]any{ + "ui": map[string]any{"resourceUri": "ui://y"}, + }) + reg := mustBuild(t, NewBuilder(). + SetTools([]ServerTool{appOnly, appOnlyAny, modelAndApp, noVisibility}). + WithToolsets([]string{"all"})) + + stripped := reg.ToolsForRegistration(ghcontext.WithUISupport(context.Background(), false)) + names := make([]string, 0, len(stripped)) + for _, tool := range stripped { + names = append(names, tool.Tool.Name) + require.Nil(t, tool.Tool.Meta["ui"], "ui meta should be stripped from %s", tool.Tool.Name) + } + require.ElementsMatch(t, []string{"model_and_app", "no_visibility"}, names) + + for _, ctx := range []context.Context{ + context.Background(), + ghcontext.WithUISupport(context.Background(), true), + } { + require.Len(t, reg.ToolsForRegistration(ctx), 4, "app-only tools should be kept when UI is supported or unknown") + } +} + func TestWithMCPApps_PreservesUIMetadataByDefault(t *testing.T) { uiData := map[string]any{"html": "
hello
"} toolWithUI := mockToolWithMeta("tool_with_ui", "toolset1", map[string]any{