fix(mcp-apps): defer _meta.ui strip to per-request RegisterTools

pkg/http/handler_test.go

@@ -825,3 +825,56 @@ func TestCrossOriginProtection(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) {
+	const uiURI = "ui://test/widget"
+	uiTool := mockTool("with_ui", "repos", true)
+	uiTool.Tool.Meta = mcp.Meta{"ui": map[string]any{"resourceUri": uiURI}}
+
+	checker := createHTTPFeatureChecker()
+	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)
+
+	// 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)
+}

pkg/inventory/builder.go

@@ -190,19 +190,6 @@ func cleanTools(tools []string) []string {
 	return cleaned
 }
 
-// checkFeatureFlag checks a feature flag at build time using the builder's feature checker.
-// Returns false if no checker is configured or the flag is not enabled.
-func (b *Builder) checkFeatureFlag(flag string) bool {
-	if b.featureChecker == nil {
-		return false
-	}
-	enabled, err := b.featureChecker(context.Background(), flag)
-	if err != nil {
-		return false
-	}
-	return enabled
-}
-
 // Build creates the final Inventory with all configuration applied.
 // This processes toolset filtering, tool name resolution, and sets up
 // the inventory for use. The returned Inventory is ready for use with
@@ -214,13 +201,6 @@ func (b *Builder) checkFeatureFlag(flag string) bool {
 func (b *Builder) Build() (*Inventory, error) {
 	tools := b.tools
 
-	// When MCP Apps feature flag is not enabled, strip UI metadata from tools
-	// so clients won't attempt to load UI resources.
-	// The feature checker is the single source of truth for flag evaluation.
-	if !b.checkFeatureFlag(mcpAppsFeatureFlag) {
-		tools = stripMCPAppsMetadata(tools)
-	}
-
 	r := &Inventory{
 		tools:             tools,
 		resourceTemplates: b.resourceTemplates,

pkg/inventory/registry.go

@@ -170,8 +170,19 @@ func (r *Inventory) ToolsetDescriptions() map[ToolsetID]string {
 
 // RegisterTools registers all available tools with the server using the provided dependencies.
 // The context is used for feature flag evaluation.
+//
+// MCP Apps UI metadata (`_meta.ui`) is stripped from the registered tools
+// when the MCP Apps feature flag is not enabled for this request. 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.
 func (r *Inventory) RegisterTools(ctx context.Context, s *mcp.Server, deps any) {
-	for _, tool := range r.AvailableTools(ctx) {
+	tools := r.AvailableTools(ctx)
+	if !r.checkFeatureFlag(ctx, mcpAppsFeatureFlag) {
+		tools = stripMCPAppsMetadata(tools)
+	}
+	for _, tool := range tools {
 		tool.RegisterFunc(s, deps)
 	}
 }

pkg/inventory/registry_test.go

@@ -1863,18 +1863,16 @@ func TestWithMCPApps_DisabledStripsUIMetadata(t *testing.T) {
 		"description": "kept",
 	})
 
-	// Default: MCP Apps is disabled - UI meta should be stripped
+	// Default: MCP Apps is disabled - UI meta should be stripped on registration.
 	reg := mustBuild(t, NewBuilder().SetTools([]ServerTool{toolWithUI}).WithToolsets([]string{"all"}))
-	available := reg.AvailableTools(context.Background())
+	registered := captureRegisteredTools(context.Background(), t, reg)
 
-	require.Len(t, available, 1)
-	// UI metadata should be stripped
-	if available[0].Tool.Meta["ui"] != nil {
+	require.Len(t, registered, 1)
+	if registered[0].Meta["ui"] != nil {
 		t.Errorf("Expected 'ui' meta to be stripped, but it was present")
 	}
-	// Other metadata should be preserved
-	if available[0].Tool.Meta["description"] != "kept" {
-		t.Errorf("Expected 'description' meta to be preserved, got %v", available[0].Tool.Meta["description"])
+	if registered[0].Meta["description"] != "kept" {
+		t.Errorf("Expected 'description' meta to be preserved, got %v", registered[0].Meta["description"])
 	}
 }
 
@@ -1947,20 +1945,18 @@ func TestWithMCPApps_ToolsWithoutUIMetaUnaffected(t *testing.T) {
 }
 
 func TestWithMCPApps_UIOnlyMetaBecomesNil(t *testing.T) {
-	// Tool with ONLY ui metadata - should become nil after stripping when MCP Apps is disabled
 	toolUIOnly := mockToolWithMeta("tool_ui_only", "toolset1", map[string]any{
 		"ui": map[string]any{"html": "<div>hello</div>"},
 	})
 
 	reg := mustBuild(t, NewBuilder().
 		SetTools([]ServerTool{toolUIOnly}).
 		WithToolsets([]string{"all"}))
-	available := reg.AvailableTools(context.Background())
+	registered := captureRegisteredTools(context.Background(), t, reg)
 
-	require.Len(t, available, 1)
-	// Meta should be nil since ui was the only key and MCP Apps is off by default
-	if available[0].Tool.Meta != nil {
-		t.Errorf("Expected Meta to be nil after stripping only key, got %v", available[0].Tool.Meta)
+	require.Len(t, registered, 1)
+	if registered[0].Meta != nil {
+		t.Errorf("Expected Meta to be nil after stripping only key, got %v", registered[0].Meta)
 	}
 }
 
@@ -2239,3 +2235,25 @@ func TestCreateExcludeToolsFilter(t *testing.T) {
 	require.NoError(t, err)
 	require.True(t, allowed, "allowed_tool should be included")
 }
+
+// captureRegisteredTools mirrors RegisterTools' per-request strip behavior so
+// tests can verify what the wire sees, without requiring tools to have real
+// handlers (RegisterTools panics on tools without HandlerFunc).
+func captureRegisteredTools(ctx context.Context, t *testing.T, reg *Inventory) []*mcp.Tool {
+	t.Helper()
+	tools := reg.AvailableTools(ctx)
+	out := make([]*mcp.Tool, 0, len(tools))
+	for i := range tools {
+		toolCopy := tools[i].Tool
+		out = append(out, &toolCopy)
+	}
+	if !reg.checkFeatureFlag(ctx, mcpAppsFeatureFlag) {
+		for _, tt := range out {
+			delete(tt.Meta, "ui")
+			if len(tt.Meta) == 0 {
+				tt.Meta = nil
+			}
+		}
+	}
+	return out
+}