Skip to content

Commit 2a25285

Browse files
committed
merge in changes
1 parent 7830fe6 commit 2a25285

File tree

8 files changed

+31
-135
lines changed

8 files changed

+31
-135
lines changed

docs/server-configuration.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ Lockdown mode ensures the server only surfaces content in public repositories fr
394394
Insiders Mode unlocks experimental features. 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.
395395

396396
> [!NOTE]
397-
> Insiders mode also enables the `remote_mcp_ui_apps` feature flag for backward compatibility. See [MCP Apps](#mcp-apps) below.
397+
> Insiders mode enables a curated set of feature flags, including `remote_mcp_ui_apps`. See [MCP Apps](#mcp-apps) below.
398398
399399
<table>
400400
<tr><th>Remote Server</th><th>Local Server</th></tr>

internal/ghmcp/server.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,15 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
115115
return nil, fmt.Errorf("failed to create GitHub clients: %w", err)
116116
}
117117

118-
// Create feature checker — insiders mode transitionally enables remote_mcp_ui_apps
118+
// Create feature checker — insiders mode expands InsidersFeatureFlags
119119
enabledFeatures := cfg.EnabledFeatures
120-
if cfg.InsidersMode && !slices.Contains(enabledFeatures, github.MCPAppsFeatureFlag) {
121-
enabledFeatures = append(slices.Clone(enabledFeatures), github.MCPAppsFeatureFlag)
120+
if cfg.InsidersMode {
121+
enabledFeatures = slices.Clone(enabledFeatures)
122+
for _, flag := range github.InsidersFeatureFlags {
123+
if !slices.Contains(enabledFeatures, flag) {
124+
enabledFeatures = append(enabledFeatures, flag)
125+
}
126+
}
122127
}
123128
featureChecker := createFeatureChecker(enabledFeatures)
124129
mcpAppsEnabled := slices.Contains(enabledFeatures, github.MCPAppsFeatureFlag)
@@ -151,7 +156,6 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
151156
WithExcludeTools(cfg.ExcludeTools).
152157
WithServerInstructions().
153158
WithFeatureChecker(featureChecker).
154-
WithInsidersMode(cfg.InsidersMode).
155159
WithMCPApps(mcpAppsEnabled)
156160

157161
// Apply token scope filtering if scopes are known (for PAT filtering)

pkg/github/feature_flags.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
package github
22

3-
// MCPAppsFeatureFlag is the feature flag name that enables MCP Apps
4-
// (interactive UI forms) for supported tools. When enabled, tools like
5-
// get_me, issue_write, and create_pull_request can render rich UI via
6-
// the MCP Apps extension instead of plain text responses.
3+
// MCPAppsFeatureFlag is the feature flag name for MCP Apps (interactive UI forms).
74
const MCPAppsFeatureFlag = "remote_mcp_ui_apps"
85

6+
// InsidersFeatureFlags is the list of feature flags that insiders mode enables.
7+
// When insiders mode is active, all flags in this list are treated as enabled.
8+
// This is the single source of truth for what "insiders" means in terms of
9+
// feature flag expansion.
10+
var InsidersFeatureFlags = []string{
11+
MCPAppsFeatureFlag,
12+
}
13+
914
// FeatureFlags defines runtime feature toggles that adjust tool behavior.
1015
type FeatureFlags struct {
1116
LockdownMode bool

pkg/http/handler.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -262,15 +262,10 @@ func InventoryFiltersForRequest(r *http.Request, builder *inventory.Builder) *in
262262
builder = builder.WithReadOnly(true)
263263
}
264264

265-
insiders := ghcontext.IsInsidersMode(ctx)
266-
if insiders {
267-
builder = builder.WithInsidersMode(true)
268-
}
269-
270265
// Enable MCP Apps if the feature flag is present in the request headers
271-
// or if insiders mode is active (transitional: insiders implies remote_mcp_ui_apps).
266+
// or if insiders mode is active (insiders expands InsidersFeatureFlags).
272267
headerFeatures := ghcontext.GetHeaderFeatures(ctx)
273-
mcpApps := slices.Contains(headerFeatures, github.MCPAppsFeatureFlag) || insiders
268+
mcpApps := slices.Contains(headerFeatures, github.MCPAppsFeatureFlag) || ghcontext.IsInsidersMode(ctx)
274269
if mcpApps {
275270
builder = builder.WithMCPApps(true)
276271
}

pkg/http/server.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,20 +215,26 @@ func initGlobalToolScopeMap(t translations.TranslationHelperFunc) error {
215215

216216
// createHTTPFeatureChecker creates a feature checker that reads header features from context
217217
// and validates them against the knownFeatureFlags whitelist.
218-
// It also handles transitional behavior where insiders mode implies remote_mcp_ui_apps.
218+
// When insiders mode is active, InsidersFeatureFlags are also treated as enabled.
219219
func createHTTPFeatureChecker() inventory.FeatureFlagChecker {
220220
// Pre-compute whitelist as set for O(1) lookup
221221
knownSet := make(map[string]bool, len(knownFeatureFlags))
222222
for _, f := range knownFeatureFlags {
223223
knownSet[f] = true
224224
}
225225

226+
// Pre-compute insiders flags as set for O(1) lookup
227+
insidersSet := make(map[string]bool, len(github.InsidersFeatureFlags))
228+
for _, f := range github.InsidersFeatureFlags {
229+
insidersSet[f] = true
230+
}
231+
226232
return func(ctx context.Context, flag string) (bool, error) {
227233
if knownSet[flag] && slices.Contains(ghcontext.GetHeaderFeatures(ctx), flag) {
228234
return true, nil
229235
}
230-
// Transitional: insiders mode implies remote_mcp_ui_apps feature flag
231-
if flag == github.MCPAppsFeatureFlag && ghcontext.IsInsidersMode(ctx) {
236+
// Insiders mode enables all InsidersFeatureFlags
237+
if insidersSet[flag] && ghcontext.IsInsidersMode(ctx) {
232238
return true, nil
233239
}
234240
return false, nil

pkg/inventory/builder.go

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ type Builder struct {
4848
featureChecker FeatureFlagChecker
4949
filters []ToolFilter // filters to apply to all tools
5050
generateInstructions bool
51-
insidersMode bool
5251
mcpApps bool
5352
}
5453

@@ -155,15 +154,6 @@ func (b *Builder) WithExcludeTools(toolNames []string) *Builder {
155154
return b
156155
}
157156

158-
// WithInsidersMode enables or disables insiders mode features.
159-
// When insiders mode is disabled (default), tools marked InsidersOnly are removed
160-
// and insiders-only Meta keys are stripped.
161-
// Returns self for chaining.
162-
func (b *Builder) WithInsidersMode(enabled bool) *Builder {
163-
b.insidersMode = enabled
164-
return b
165-
}
166-
167157
// WithMCPApps enables or disables MCP Apps UI features.
168158
// When disabled (default), the "ui" Meta key is stripped from tools
169159
// so clients won't attempt to load UI resources.
@@ -214,11 +204,7 @@ func cleanTools(tools []string) []string {
214204
// (i.e., they don't exist in the tool set and are not deprecated aliases).
215205
// This ensures invalid tool configurations fail fast at build time.
216206
func (b *Builder) Build() (*Inventory, error) {
217-
// When insiders mode is disabled, strip insiders-only features from tools
218207
tools := b.tools
219-
if !b.insidersMode {
220-
tools = stripInsidersFeatures(b.tools)
221-
}
222208

223209
// When MCP Apps is disabled, strip UI metadata from tools
224210
if !b.mcpApps {
@@ -390,32 +376,6 @@ func (b *Builder) processToolsets() (map[ToolsetID]bool, []string, []ToolsetID,
390376
return enabledToolsets, unrecognized, allToolsetIDs, validIDs, defaultToolsetIDList, descriptions
391377
}
392378

393-
// insidersOnlyMetaKeys lists the Meta keys that are only available in insiders mode.
394-
// Add new experimental feature keys here to have them automatically stripped
395-
// when insiders mode is disabled.
396-
// Note: "ui" (MCP Apps) is now controlled by the remote_mcp_ui_apps feature flag via
397-
// WithMCPApps and mcpAppsMetaKeys, not by insiders mode.
398-
var insidersOnlyMetaKeys = []string{}
399-
400-
// stripInsidersFeatures removes insiders-only features from tools.
401-
// This includes removing tools marked with InsidersOnly and stripping
402-
// Meta keys listed in insidersOnlyMetaKeys from remaining tools.
403-
func stripInsidersFeatures(tools []ServerTool) []ServerTool {
404-
result := make([]ServerTool, 0, len(tools))
405-
for _, tool := range tools {
406-
// Skip tools marked as insiders-only
407-
if tool.InsidersOnly {
408-
continue
409-
}
410-
if stripped := stripMetaKeys(tool, insidersOnlyMetaKeys); stripped != nil {
411-
result = append(result, *stripped)
412-
} else {
413-
result = append(result, tool)
414-
}
415-
}
416-
return result
417-
}
418-
419379
// mcpAppsMetaKeys lists the Meta keys controlled by the remote_mcp_ui_apps feature flag.
420380
var mcpAppsMetaKeys = []string{
421381
"ui", // MCP Apps UI metadata

pkg/inventory/registry_test.go

Lines changed: 2 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1853,13 +1853,13 @@ func mockToolWithMeta(name string, toolsetID string, meta map[string]any) Server
18531853
)
18541854
}
18551855

1856-
func TestWithInsidersMode_DisabledStripsUIMetadata(t *testing.T) {
1856+
func TestWithMCPApps_DisabledStripsUIMetadata(t *testing.T) {
18571857
toolWithUI := mockToolWithMeta("tool_with_ui", "toolset1", map[string]any{
18581858
"ui": map[string]any{"html": "<div>hello</div>"},
18591859
"description": "kept",
18601860
})
18611861

1862-
// Default: insiders mode is disabled - UI meta should be stripped
1862+
// Default: MCP Apps is disabled - UI meta should be stripped
18631863
reg := mustBuild(t, NewBuilder().SetTools([]ServerTool{toolWithUI}).WithToolsets([]string{"all"}))
18641864
available := reg.AvailableTools(context.Background())
18651865

@@ -1899,40 +1899,6 @@ func TestWithMCPApps_EnabledPreservesUIMetadata(t *testing.T) {
18991899
}
19001900
}
19011901

1902-
func TestWithInsidersMode_EnabledPreservesInsidersOnlyTools(t *testing.T) {
1903-
normalTool := mockTool("normal", "toolset1", true)
1904-
insidersTool := mockTool("insiders_only", "toolset1", true)
1905-
insidersTool.InsidersOnly = true
1906-
1907-
// With insiders mode enabled, both tools should be available
1908-
reg := mustBuild(t, NewBuilder().
1909-
SetTools([]ServerTool{normalTool, insidersTool}).
1910-
WithToolsets([]string{"all"}).
1911-
WithInsidersMode(true))
1912-
available := reg.AvailableTools(context.Background())
1913-
1914-
require.Len(t, available, 2)
1915-
names := []string{available[0].Tool.Name, available[1].Tool.Name}
1916-
require.Contains(t, names, "normal")
1917-
require.Contains(t, names, "insiders_only")
1918-
}
1919-
1920-
func TestWithInsidersMode_DisabledRemovesInsidersOnlyTools(t *testing.T) {
1921-
normalTool := mockTool("normal", "toolset1", true)
1922-
insidersTool := mockTool("insiders_only", "toolset1", true)
1923-
insidersTool.InsidersOnly = true
1924-
1925-
// With insiders mode disabled, insiders-only tool should be removed
1926-
reg := mustBuild(t, NewBuilder().
1927-
SetTools([]ServerTool{normalTool, insidersTool}).
1928-
WithToolsets([]string{"all"}).
1929-
WithInsidersMode(false))
1930-
available := reg.AvailableTools(context.Background())
1931-
1932-
require.Len(t, available, 1)
1933-
require.Equal(t, "normal", available[0].Tool.Name)
1934-
}
1935-
19361902
func TestWithMCPApps_ToolsWithoutUIMetaUnaffected(t *testing.T) {
19371903
toolNoUI := mockToolWithMeta("tool_no_ui", "toolset1", map[string]any{
19381904
"description": "kept",
@@ -1991,25 +1957,6 @@ func TestWithMCPApps_UIOnlyMetaBecomesNil(t *testing.T) {
19911957
}
19921958
}
19931959

1994-
func TestWithMCPApps_EnabledPreservesUIMeta(t *testing.T) {
1995-
// Tool with ui metadata - should be preserved when MCP Apps is enabled
1996-
toolWithUI := mockToolWithMeta("tool_with_ui", "toolset1", map[string]any{
1997-
"ui": map[string]any{"html": "<div>hello</div>"},
1998-
"description": "kept",
1999-
})
2000-
2001-
reg := mustBuild(t, NewBuilder().
2002-
SetTools([]ServerTool{toolWithUI}).
2003-
WithToolsets([]string{"all"}).
2004-
WithMCPApps(true))
2005-
available := reg.AvailableTools(context.Background())
2006-
2007-
require.Len(t, available, 1)
2008-
require.NotNil(t, available[0].Tool.Meta, "Meta should be preserved with MCP Apps enabled")
2009-
require.NotNil(t, available[0].Tool.Meta["ui"], "ui key should be preserved with MCP Apps enabled")
2010-
require.Equal(t, "kept", available[0].Tool.Meta["description"])
2011-
}
2012-
20131960
func TestStripMetaKeys(t *testing.T) {
20141961
tests := []struct {
20151962
name string
@@ -2104,23 +2051,6 @@ func TestStripMCPAppsMetadata(t *testing.T) {
21042051
require.Nil(t, result[2].Tool.Meta)
21052052
}
21062053

2107-
func TestStripInsidersFeatures_RemovesInsidersOnlyTools(t *testing.T) {
2108-
// Create tools: one normal, one insiders-only, one normal
2109-
normalTool1 := mockTool("normal1", "toolset1", true)
2110-
insidersTool := mockTool("insiders_only", "toolset1", true)
2111-
insidersTool.InsidersOnly = true
2112-
normalTool2 := mockTool("normal2", "toolset1", true)
2113-
2114-
tools := []ServerTool{normalTool1, insidersTool, normalTool2}
2115-
2116-
result := stripInsidersFeatures(tools)
2117-
2118-
// Should only have 2 tools (insiders-only tool filtered out)
2119-
require.Len(t, result, 2)
2120-
require.Equal(t, "normal1", result[0].Tool.Name)
2121-
require.Equal(t, "normal2", result[1].Tool.Name)
2122-
}
2123-
21242054
func TestStripMetaKeys_MultipleKeys(t *testing.T) {
21252055
// This test verifies the mechanism works for multiple keys
21262056
keys := []string{"ui", "experimental_feature", "beta"}

pkg/inventory/server_tool.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,6 @@ type ServerTool struct {
8282
// This includes the required scopes plus any higher-level scopes that provide
8383
// the necessary permissions due to scope hierarchy.
8484
AcceptedScopes []string
85-
86-
// InsidersOnly marks this tool as only available when insiders mode is enabled.
87-
// When insiders mode is disabled, tools with this flag set are completely omitted.
88-
InsidersOnly bool
8985
}
9086

9187
// IsReadOnly returns true if this tool is marked as read-only via annotations.

0 commit comments

Comments
 (0)