fix(mcp-apps): reconcile the show/defer contract — render results, remove show_ui

cmd/github-mcp-server/generate_docs.go

@@ -265,8 +265,6 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {
 		}
 		sort.Strings(paramNames)
 
-		conditional := inventory.ConditionalSchemaPropertyDescriptions()
-
 		for i, propName := range paramNames {
 			prop := schema.Properties[propName]
 			required := slices.Contains(schema.Required, propName)
@@ -292,11 +290,7 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {
 			// Indent any continuation lines in the description to maintain markdown formatting
 			description := indentMultilineDescription(prop.Description, "    ")
 
-			if cond, isConditional := conditional[propName]; isConditional {
-				fmt.Fprintf(buf, "  - `%s`: %s (%s, %s, conditional — %s)", propName, description, typeStr, requiredStr, cond)
-			} else {
-				fmt.Fprintf(buf, "  - `%s`: %s (%s, %s)", propName, description, typeStr, requiredStr)
-			}
+			fmt.Fprintf(buf, "  - `%s`: %s (%s, %s)", propName, description, typeStr, requiredStr)
 			if i < len(paramNames)-1 {
 				buf.WriteString("\n")
 			}

docs/feature-flags.md

@@ -45,7 +45,6 @@ runtime behavior (such as output formatting) won't appear here.
   - `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)
-  - `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when the user has already confirmed the action and the form would be redundant. (boolean, optional, conditional — visible when remote_mcp_ui_apps is enabled unless the client explicitly indicates it does not support io.modelcontextprotocol/ui)
   - `title`: PR title (string, required)
 
 - **get_me** - Get my user profile
@@ -69,7 +68,6 @@ runtime behavior (such as output formatting) won't appear here.
   - `milestone`: Milestone number (number, optional)
   - `owner`: Repository owner (string, required)
   - `repo`: Repository name (string, required)
-  - `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when the user has already confirmed the action and the form would be redundant. (boolean, optional, conditional — visible when remote_mcp_ui_apps is enabled unless the client explicitly indicates it does not support io.modelcontextprotocol/ui)
   - `state`: New state (string, optional)
   - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
   - `title`: Issue title (string, optional)

docs/insiders-features.md

@@ -39,7 +39,6 @@ The list below is generated from the Go source. It covers tool **inventory and s
   - `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)
-  - `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when the user has already confirmed the action and the form would be redundant. (boolean, optional, conditional — visible when remote_mcp_ui_apps is enabled unless the client explicitly indicates it does not support io.modelcontextprotocol/ui)
   - `title`: PR title (string, required)
 
 - **get_me** - Get my user profile
@@ -63,7 +62,6 @@ The list below is generated from the Go source. It covers tool **inventory and s
   - `milestone`: Milestone number (number, optional)
   - `owner`: Repository owner (string, required)
   - `repo`: Repository name (string, required)
-  - `show_ui`: Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when the user has already confirmed the action and the form would be redundant. (boolean, optional, conditional — visible when remote_mcp_ui_apps is enabled unless the client explicitly indicates it does not support io.modelcontextprotocol/ui)
   - `state`: New state (string, optional)
   - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
   - `title`: Issue title (string, optional)

pkg/github/__toolsnaps__/create_pull_request.snap

@@ -49,10 +49,6 @@
         },
         "type": "array"
       },
-      "show_ui": {
-        "description": "Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when the user has already confirmed the action and the form would be redundant.",
-        "type": "boolean"
-      },
       "title": {
         "description": "PR title",
         "type": "string"

pkg/github/__toolsnaps__/issue_write.snap

@@ -96,10 +96,6 @@
         "description": "Repository name",
         "type": "string"
       },
-      "show_ui": {
-        "description": "Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when the user has already confirmed the action and the form would be redundant.",
-        "type": "boolean"
-      },
       "state": {
         "description": "New state",
         "enum": [

pkg/github/issues.go

@@ -1726,7 +1726,11 @@ const IssueWriteUIResourceURI = "ui://github-mcp-server/issue-write"
 
 // issueWriteFormParams are the parameters the issue_write MCP App form collects
 // and re-sends on submit. Any other parameter present on a call cannot be
-// represented by the form.
+// represented by the form. The form collects (and prefills) every parameter in
+// the tool's current input schema, so hasNonFormParams against this set is a
+// forward-compatibility safety net: a parameter added to the schema in the
+// future but not yet wired into the form trips the check and bypasses the form
+// so the supplied value isn't silently dropped.
 var issueWriteFormParams = map[string]struct{}{
 	"method":        {},
 	"owner":         {},
@@ -1742,28 +1746,9 @@ var issueWriteFormParams = map[string]struct{}{
 	"state":         {},
 	"state_reason":  {},
 	"duplicate_of":  {},
-	"show_ui":       {},
 	"_ui_submitted": {},
 }
 
-// issueWriteHasNonFormParams reports whether the call carries any parameter the
-// issue_write MCP App form cannot represent (anything outside issueWriteFormParams).
-// The form collects (and prefills) every parameter in the tool's current input
-// schema, so this is a forward-compatibility safety net: a parameter added to the
-// schema in the future but not yet wired into the form trips this check and bypasses
-// the UI so the supplied value isn't silently dropped.
-func issueWriteHasNonFormParams(args map[string]any) bool {
-	for key, value := range args {
-		if value == nil {
-			continue
-		}
-		if _, ok := issueWriteFormParams[key]; !ok {
-			return true
-		}
-	}
-	return false
-}
-
 // issueWriteAwaitingFormResult builds the "awaiting form submission" stub
 // returned when issue_write hands off to the MCP App form. The body is shared
 // by IssueWrite and LegacyIssueWrite. The result is marked IsError=true so
@@ -1919,17 +1904,6 @@ Options are:
 							Required: []string{"field_name"},
 						},
 					},
-					// show_ui is hidden from clients that do not advertise MCP App
-					// UI support. The strip happens per-request in
-					// inventory.ToolsForRegistration; it is present in the static
-					// schema (and therefore in toolsnaps and the feature-flag /
-					// insiders docs) so the UI-capable surface is fully
-					// documented. It is intentionally not in the main README,
-					// which renders the stripped (non-UI) schema.
-					"show_ui": {
-						Type:        "boolean",
-						Description: "Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when the user has already confirmed the action and the form would be redundant.",
-					},
 				},
 				Required: []string{"method", "owner", "repo"},
 			},
@@ -1950,20 +1924,9 @@ Options are:
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
 
-			// When MCP Apps are enabled and the client supports UI, route the
-			// call to the interactive form unless:
-			//   - it is itself a form submission (the UI sends _ui_submitted=true),
-			//   - the caller explicitly asked to skip the UI (show_ui=false), or
-			//   - it carries parameters the form cannot represent (e.g. labels,
-			//     assignees or issue_fields). Those must be applied directly so
-			//     their values aren't silently dropped.
-			uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
-			showUI, err := OptionalBoolParamWithDefault(args, "show_ui", true)
-			if err != nil {
-				return utils.NewToolResultError(err.Error()), nil, nil
-			}
-
-			if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && showUI && !issueWriteHasNonFormParams(args) {
+			// Hand off to the interactive MCP App form unless this call must
+			// execute now (see shouldDeferToForm).
+			if shouldDeferToForm(ctx, deps, req, args, issueWriteFormParams) {
 				issueNumber := 0
 				if method == "update" {
 					n, numErr := RequiredInt(args, "issue_number")

pkg/github/issues_test.go

@@ -1674,86 +1674,6 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {
 			"labels should route through UI form")
 		assert.True(t, result.IsError, "form-routing stub should be marked IsError so agents don't claim success")
 	})
-
-	t.Run("UI client with show_ui=false skips form and executes directly", func(t *testing.T) {
-		// show_ui=false is the explicit, model-facing way to opt out of the
-		// form. It must bypass the form even when every other condition would
-		// route the call there (UI capability, MCP Apps flag on, no
-		// _ui_submitted, only form params present).
-		request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
-			"method":  "create",
-			"owner":   "owner",
-			"repo":    "repo",
-			"title":   "Test",
-			"show_ui": false,
-		})
-		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
-		require.NoError(t, err)
-
-		textContent := getTextResult(t, result)
-		assert.NotContains(t, textContent.Text, "interactive form has been shown",
-			"show_ui=false should skip UI form")
-		assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
-			"show_ui=false call should execute directly and return issue URL")
-	})
-
-	t.Run("UI client with show_ui=true returns form message", func(t *testing.T) {
-		// show_ui=true is the explicit, redundant-with-the-default way to ask
-		// for the form. It must still route through the form and must not be
-		// treated as a non-form parameter that would trigger the safety-net
-		// bypass.
-		request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
-			"method":  "create",
-			"owner":   "owner",
-			"repo":    "repo",
-			"title":   "Test",
-			"show_ui": true,
-		})
-		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
-		require.NoError(t, err)
-
-		textContent := getTextResult(t, result)
-		assert.Contains(t, textContent.Text, "interactive form has been shown",
-			"show_ui=true should still route through the form")
-	})
-
-	t.Run("UI client with show_ui=false and _ui_submitted=true executes directly", func(t *testing.T) {
-		// _ui_submitted and show_ui=false are two ways to say "execute
-		// directly". When both are set there must be no conflict — the call
-		// still executes directly.
-		request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
-			"method":        "create",
-			"owner":         "owner",
-			"repo":          "repo",
-			"title":         "Test",
-			"show_ui":       false,
-			"_ui_submitted": true,
-		})
-		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
-		require.NoError(t, err)
-
-		textContent := getTextResult(t, result)
-		assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
-			"show_ui=false + _ui_submitted should execute directly")
-	})
-
-	t.Run("non-UI client with show_ui=false executes directly (no regression)", func(t *testing.T) {
-		// show_ui is irrelevant when the client does not support UI; the call
-		// must execute directly exactly as it does today.
-		request := createMCPRequest(map[string]any{
-			"method":  "create",
-			"owner":   "owner",
-			"repo":    "repo",
-			"title":   "Test",
-			"show_ui": false,
-		})
-		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
-		require.NoError(t, err)
-
-		textContent := getTextResult(t, result)
-		assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
-			"non-UI client should execute directly regardless of show_ui")
-	})
 }
 
 func Test_issueWriteHasNonFormParams(t *testing.T) {
@@ -1766,8 +1686,6 @@ func Test_issueWriteHasNonFormParams(t *testing.T) {
 	}{
 		{name: "no params", args: map[string]any{}, want: false},
 		{name: "only form params", args: map[string]any{"method": "create", "owner": "o", "repo": "r", "title": "t", "body": "b", "issue_number": float64(1), "_ui_submitted": true}, want: false},
-		{name: "show_ui true is a form param", args: map[string]any{"title": "t", "show_ui": true}, want: false},
-		{name: "show_ui false is a form param", args: map[string]any{"title": "t", "show_ui": false}, want: false},
 		{name: "labels present", args: map[string]any{"title": "t", "labels": []any{"bug"}}, want: false},
 		{name: "assignees present", args: map[string]any{"title": "t", "assignees": []any{"octocat"}}, want: false},
 		{name: "milestone present", args: map[string]any{"title": "t", "milestone": float64(2)}, want: false},
@@ -1783,7 +1701,7 @@ func Test_issueWriteHasNonFormParams(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
-			assert.Equal(t, tc.want, issueWriteHasNonFormParams(tc.args))
+			assert.Equal(t, tc.want, hasNonFormParams(tc.args, issueWriteFormParams))
 		})
 	}
 }
@@ -1797,7 +1715,7 @@ func Test_issueWriteSchemaClassification(t *testing.T) {
 	t.Parallel()
 
 	// Schema properties the MCP App form cannot represent — their presence
-	// must trigger the safety-net bypass via issueWriteHasNonFormParams. The
+	// must trigger the safety-net bypass via hasNonFormParams. The
 	// form currently collects every schema property, so this allowlist is
 	// empty; add a property here only if it is added to the schema without
 	// corresponding form support.

pkg/github/pullrequests.go

@@ -603,7 +603,6 @@ var pullRequestWriteFormParams = map[string]struct{}{
 	"draft":                 {},
 	"maintainer_can_modify": {},
 	"reviewers":             {},
-	"show_ui":               {},
 	"_ui_submitted":         {},
 }
 
@@ -621,34 +620,6 @@ var pullRequestUpdateFormParams = map[string]struct{}{
 	"_ui_submitted":         {},
 }
 
-// pullRequestWriteHasNonFormParams reports whether the call carries any parameter
-// the create_pull_request MCP App form cannot represent (anything outside
-// pullRequestWriteFormParams). Such calls must bypass the UI form and execute
-// directly so the supplied values aren't silently dropped.
-func pullRequestWriteHasNonFormParams(args map[string]any) bool {
-	for key, value := range args {
-		if value == nil {
-			continue
-		}
-		if _, ok := pullRequestWriteFormParams[key]; !ok {
-			return true
-		}
-	}
-	return false
-}
-
-func pullRequestUpdateHasNonFormParams(args map[string]any) bool {
-	for key, value := range args {
-		if value == nil {
-			continue
-		}
-		if _, ok := pullRequestUpdateFormParams[key]; !ok {
-			return true
-		}
-	}
-	return false
-}
-
 // CreatePullRequest creates a tool to create a new pull request.
 func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerTool {
 	return NewTool(
@@ -708,17 +679,6 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
 							Type: "string",
 						},
 					},
-					// show_ui is hidden from clients that do not advertise MCP App
-					// UI support. The strip happens per-request in
-					// inventory.ToolsForRegistration; it is present in the static
-					// schema (and therefore in toolsnaps and the feature-flag /
-					// insiders docs) so the UI-capable surface is fully
-					// documented. It is intentionally not in the main README,
-					// which renders the stripped (non-UI) schema.
-					"show_ui": {
-						Type:        "boolean",
-						Description: "Whether to render the MCP App form instead of executing the request immediately. Defaults to true. Set to false to skip the form and execute directly — useful when the user has already confirmed the action and the form would be redundant.",
-					},
 				},
 				Required: []string{"owner", "repo", "title", "head", "base"},
 			},
@@ -734,19 +694,9 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
 
-			// When MCP Apps are enabled and the client supports UI, route the
-			// call to the interactive form unless:
-			//   - it is itself a form submission (the UI sends _ui_submitted=true),
-			//   - the caller explicitly asked to skip the UI (show_ui=false), or
-			//   - it carries parameters the form cannot represent. Those must be
-			//     applied directly so their values aren't silently dropped.
-			uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
-			showUI, err := OptionalBoolParamWithDefault(args, "show_ui", true)
-			if err != nil {
-				return utils.NewToolResultError(err.Error()), nil, nil
-			}
-
-			if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && showUI && !pullRequestWriteHasNonFormParams(args) {
+			// Hand off to the interactive MCP App form unless this call must
+			// execute now (see shouldDeferToForm).
+			if shouldDeferToForm(ctx, deps, req, args, pullRequestWriteFormParams) {
 				return utils.NewToolResultAwaitingFormSubmission(fmt.Sprintf(
 					"An interactive form has been shown to the user for creating a new pull request in %s/%s. "+
 						"STOP — do not call any other tools, do not respond as if the pull request was created, "+
@@ -965,8 +915,9 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
 
-			uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
-			if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !pullRequestUpdateHasNonFormParams(args) {
+			// Hand off to the interactive MCP App form unless this call must
+			// execute now (see shouldDeferToForm).
+			if shouldDeferToForm(ctx, deps, req, args, pullRequestUpdateFormParams) {
 				return utils.NewToolResultAwaitingFormSubmission(fmt.Sprintf(
 					"An interactive form has been shown to the user for editing pull request #%d in %s/%s. "+
 						"STOP — do not call any other tools, do not respond as if the pull request was updated, "+