Skip to content

Commit 561a4a7

Browse files
mattdhollowayCopilot
authored andcommitted
Strip _meta.ui when client lacks UI capability
Per the MCP Apps 2026-01-26 spec, servers SHOULD check client capabilities before advertising UI-enabled tools. Extend the inventory strip gate to remove _meta.ui not only when the feature flag is off, but also when the request context explicitly reports the client lacks UI support (HasUISupport returns supported=false, ok=true). When the capability is unknown (ok=false, e.g. stdio paths), fall through to the existing feature-flag gate so existing behaviour is preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent b5397f6 commit 561a4a7

3 files changed

Lines changed: 130 additions & 11 deletions

File tree

pkg/http/handler_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,3 +904,44 @@ func TestInsidersRoutePreservesUIMeta(t *testing.T) {
904904
require.False(t, plainEnabled, "FF should be off for non-insiders ctx")
905905
require.Len(t, plainTools, 1)
906906
}
907+
908+
// TestUIMetaStrippedWhenClientLacksCapability verifies that even on the
909+
// /insiders path (where the feature flag is on), UI metadata is stripped from
910+
// tools/list responses when the client did NOT advertise the
911+
// io.modelcontextprotocol/ui extension capability. Per the 2026-01-26 MCP
912+
// Apps spec, servers SHOULD check client capabilities before exposing
913+
// UI-enabled tools.
914+
func TestUIMetaStrippedWhenClientLacksCapability(t *testing.T) {
915+
const uiURI = "ui://test/widget"
916+
uiTool := mockTool("with_ui", "repos", true)
917+
uiTool.Tool.Meta = mcp.Meta{"ui": map[string]any{"resourceUri": uiURI}}
918+
919+
checker := createHTTPFeatureChecker(nil, false)
920+
build := func() *inventory.Inventory {
921+
inv, err := inventory.NewBuilder().
922+
SetTools([]inventory.ServerTool{uiTool}).
923+
WithFeatureChecker(checker).
924+
WithToolsets([]string{"all"}).
925+
Build()
926+
require.NoError(t, err)
927+
return inv
928+
}
929+
930+
insidersCtx := ghcontext.WithInsidersMode(context.Background(), true)
931+
withoutUICap := ghcontext.WithUISupport(insidersCtx, false)
932+
withUICap := ghcontext.WithUISupport(insidersCtx, true)
933+
934+
stripped := build().ToolsForRegistration(withoutUICap)
935+
require.Len(t, stripped, 1)
936+
require.Nil(t, stripped[0].Tool.Meta["ui"], "_meta.ui should be stripped when client lacks UI capability")
937+
938+
preserved := build().ToolsForRegistration(withUICap)
939+
require.Len(t, preserved, 1)
940+
require.NotNil(t, preserved[0].Tool.Meta["ui"], "_meta.ui should be preserved when client advertises UI capability")
941+
require.Equal(t, uiURI, preserved[0].Tool.Meta["ui"].(map[string]any)["resourceUri"])
942+
943+
// Unknown capability falls through to the FF gate (insiders ctx → kept).
944+
unknown := build().ToolsForRegistration(insidersCtx)
945+
require.Len(t, unknown, 1)
946+
require.NotNil(t, unknown[0].Tool.Meta["ui"], "_meta.ui should be preserved when capability is unknown and FF is on")
947+
}

pkg/inventory/registry.go

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"slices"
88
"sort"
99

10+
ghcontext "github.com/github/github-mcp-server/pkg/context"
1011
"github.com/modelcontextprotocol/go-sdk/mcp"
1112
)
1213

@@ -169,26 +170,50 @@ func (r *Inventory) ToolsetDescriptions() map[ToolsetID]string {
169170

170171
// ToolsForRegistration returns AvailableTools(ctx) post-processed exactly as
171172
// RegisterTools would expose them: with MCP Apps UI metadata stripped when
172-
// the remote_mcp_ui_apps feature flag is not enabled in ctx. Useful for
173-
// documentation generators and diagnostics that need the same view of the
174-
// tool surface the server would register.
173+
// the client cannot consume it. Useful for documentation generators and
174+
// diagnostics that need the same view of the tool surface the server would
175+
// register.
176+
//
177+
// The strip applies when EITHER of the following is true:
178+
//
179+
// - The remote_mcp_ui_apps feature flag is not enabled in ctx (server-side gate).
180+
// - The client explicitly did not advertise the io.modelcontextprotocol/ui
181+
// extension capability (per the 2026-01-26 MCP Apps spec, servers SHOULD
182+
// check client capabilities before exposing UI-enabled tools). When the
183+
// capability is unknown (e.g. stdio paths that do not populate the
184+
// context flag) the feature-flag gate is the sole source of truth.
175185
func (r *Inventory) ToolsForRegistration(ctx context.Context) []ServerTool {
176186
tools := r.AvailableTools(ctx)
177-
if !r.checkFeatureFlag(ctx, mcpAppsFeatureFlag) {
187+
if shouldStripMCPAppsMetadata(ctx, r.checkFeatureFlag(ctx, mcpAppsFeatureFlag)) {
178188
tools = stripMCPAppsMetadata(tools)
179189
}
180190
return tools
181191
}
182192

193+
// shouldStripMCPAppsMetadata centralises the strip decision so the same logic
194+
// is exercised by tests and by RegisterTools.
195+
func shouldStripMCPAppsMetadata(ctx context.Context, featureFlagEnabled bool) bool {
196+
if !featureFlagEnabled {
197+
return true
198+
}
199+
// Feature flag is on. Respect the client capability if it is known.
200+
if supported, ok := ghcontext.HasUISupport(ctx); ok && !supported {
201+
return true
202+
}
203+
return false
204+
}
205+
183206
// RegisterTools registers all available tools with the server using the provided dependencies.
184-
// The context is used for feature flag evaluation.
207+
// The context is used for feature flag evaluation and client capability checks.
185208
//
186209
// MCP Apps UI metadata (`_meta.ui`) is stripped from the registered tools
187-
// when the MCP Apps feature flag is not enabled for this request. The strip
188-
// happens here (rather than at Build() time) so the per-request context is
189-
// in scope — HTTP feature checkers that read insiders mode or user identity
190-
// from ctx would otherwise see context.Background() and falsely report the
191-
// flag off, even when the actual request arrived on the /insiders route.
210+
// when either the MCP Apps feature flag is not enabled for this request, or
211+
// the client did not advertise the io.modelcontextprotocol/ui extension. The
212+
// strip happens here (rather than at Build() time) so the per-request
213+
// context is in scope — HTTP feature checkers that read insiders mode or
214+
// user identity from ctx would otherwise see context.Background() and
215+
// falsely report the flag off, even when the actual request arrived on the
216+
// /insiders route.
192217
func (r *Inventory) RegisterTools(ctx context.Context, s *mcp.Server, deps any) {
193218
for _, tool := range r.ToolsForRegistration(ctx) {
194219
tool.RegisterFunc(s, deps)

pkg/inventory/registry_test.go

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"testing"
88

9+
ghcontext "github.com/github/github-mcp-server/pkg/context"
910
"github.com/modelcontextprotocol/go-sdk/mcp"
1011
"github.com/stretchr/testify/require"
1112
)
@@ -2211,7 +2212,7 @@ func captureRegisteredTools(ctx context.Context, t *testing.T, reg *Inventory) [
22112212
toolCopy := tools[i].Tool
22122213
out = append(out, &toolCopy)
22132214
}
2214-
if !reg.checkFeatureFlag(ctx, mcpAppsFeatureFlag) {
2215+
if shouldStripMCPAppsMetadata(ctx, reg.checkFeatureFlag(ctx, mcpAppsFeatureFlag)) {
22152216
for _, tt := range out {
22162217
delete(tt.Meta, "ui")
22172218
if len(tt.Meta) == 0 {
@@ -2221,3 +2222,55 @@ func captureRegisteredTools(ctx context.Context, t *testing.T, reg *Inventory) [
22212222
}
22222223
return out
22232224
}
2225+
2226+
// TestShouldStripMCPAppsMetadata verifies the spec-conformant strip decision:
2227+
// strip when the feature flag is off, OR when the client explicitly does not
2228+
// advertise the io.modelcontextprotocol/ui extension.
2229+
func TestShouldStripMCPAppsMetadata(t *testing.T) {
2230+
t.Parallel()
2231+
2232+
tests := []struct {
2233+
name string
2234+
setupCtx func() context.Context
2235+
ffOn bool
2236+
want bool
2237+
}{
2238+
{
2239+
name: "FF off, capability unknown -> strip",
2240+
setupCtx: context.Background,
2241+
ffOn: false,
2242+
want: true,
2243+
},
2244+
{
2245+
name: "FF off, capability present -> strip (FF wins)",
2246+
setupCtx: func() context.Context { return ghcontext.WithUISupport(context.Background(), true) },
2247+
ffOn: false,
2248+
want: true,
2249+
},
2250+
{
2251+
name: "FF on, capability unknown -> keep",
2252+
setupCtx: context.Background,
2253+
ffOn: true,
2254+
want: false,
2255+
},
2256+
{
2257+
name: "FF on, capability present -> keep",
2258+
setupCtx: func() context.Context { return ghcontext.WithUISupport(context.Background(), true) },
2259+
ffOn: true,
2260+
want: false,
2261+
},
2262+
{
2263+
name: "FF on, capability explicitly absent -> strip",
2264+
setupCtx: func() context.Context { return ghcontext.WithUISupport(context.Background(), false) },
2265+
ffOn: true,
2266+
want: true,
2267+
},
2268+
}
2269+
2270+
for _, tc := range tests {
2271+
t.Run(tc.name, func(t *testing.T) {
2272+
got := shouldStripMCPAppsMetadata(tc.setupCtx(), tc.ffOn)
2273+
require.Equal(t, tc.want, got)
2274+
})
2275+
}
2276+
}

0 commit comments

Comments
 (0)