diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 6ebc2dec9..6b6ff7eaa 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -45,7 +45,7 @@ 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 you have all required values (especially ones the form does not collect, like reviewers) and the user has already confirmed the action. (boolean, optional, conditional — visible when remote_mcp_ui_apps is enabled unless the client explicitly indicates it does not support io.modelcontextprotocol/ui) + - `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 +69,7 @@ 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 you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, issue_fields, or state changes) and the user has already confirmed the action. (boolean, optional, conditional — visible when remote_mcp_ui_apps is enabled unless the client explicitly indicates it does not support io.modelcontextprotocol/ui) + - `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) diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 37488f544..373ead3b9 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -39,7 +39,7 @@ 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 you have all required values (especially ones the form does not collect, like reviewers) and the user has already confirmed the action. (boolean, optional, conditional — visible when remote_mcp_ui_apps is enabled unless the client explicitly indicates it does not support io.modelcontextprotocol/ui) + - `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 +63,7 @@ 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 you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, issue_fields, or state changes) and the user has already confirmed the action. (boolean, optional, conditional — visible when remote_mcp_ui_apps is enabled unless the client explicitly indicates it does not support io.modelcontextprotocol/ui) + - `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) diff --git a/pkg/github/__toolsnaps__/create_pull_request.snap b/pkg/github/__toolsnaps__/create_pull_request.snap index b2f14e390..5de1b229b 100644 --- a/pkg/github/__toolsnaps__/create_pull_request.snap +++ b/pkg/github/__toolsnaps__/create_pull_request.snap @@ -50,7 +50,7 @@ "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 you have all required values (especially ones the form does not collect, like reviewers) and the user has already confirmed the action.", + "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": { diff --git a/pkg/github/__toolsnaps__/issue_write.snap b/pkg/github/__toolsnaps__/issue_write.snap index 47d00c445..9d80b14d5 100644 --- a/pkg/github/__toolsnaps__/issue_write.snap +++ b/pkg/github/__toolsnaps__/issue_write.snap @@ -97,7 +97,7 @@ "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 you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, issue_fields, or state changes) and the user has already confirmed the action.", + "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": { diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 5479f3579..3af88f430 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1735,6 +1735,10 @@ var issueWriteFormParams = map[string]struct{}{ "body": {}, "issue_number": {}, "issue_fields": {}, + "labels": {}, + "assignees": {}, + "milestone": {}, + "type": {}, "state": {}, "state_reason": {}, "duplicate_of": {}, @@ -1743,9 +1747,11 @@ var issueWriteFormParams = map[string]struct{}{ } // issueWriteHasNonFormParams reports whether the call carries any parameter the -// issue_write MCP App form cannot represent (anything outside issueWriteFormParams, -// e.g. labels, assignees, milestones or issue types). Such calls must bypass -// the UI form and execute directly so the supplied values aren't silently dropped. +// 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 { @@ -1922,7 +1928,7 @@ Options are: // 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 you have all required values (especially ones the form does not collect, like labels, assignees, milestone, type, issue_fields, or state changes) and the user has already confirmed the action.", + 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"}, diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 2dea639f8..cb432c701 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1655,9 +1655,10 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) { assert.True(t, result.IsError, "form-routing stub should be marked IsError so agents don't claim success") }) - t.Run("UI client with labels skips form and executes directly", func(t *testing.T) { - // The form does not collect labels, so a call carrying them must bypass - // the form rather than silently drop them. + t.Run("UI client with labels routes through UI form", func(t *testing.T) { + // labels is now a form param (the issue-write view prefills and renders + // a label selector), so a call carrying them must route to the form + // rather than execute directly. request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{ "method": "create", "owner": "owner", @@ -1669,10 +1670,9 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) { require.NoError(t, err) textContent := getTextResult(t, result) - assert.NotContains(t, textContent.Text, "interactive form has been shown", - "labels should skip UI form") - assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1", - "labels call should execute directly and return issue URL") + assert.Contains(t, textContent.Text, "interactive form has been shown to the user for creating a new issue", + "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) { @@ -1768,14 +1768,15 @@ func Test_issueWriteHasNonFormParams(t *testing.T) { {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: true}, - {name: "assignees present", args: map[string]any{"title": "t", "assignees": []any{"octocat"}}, want: true}, - {name: "milestone present", args: map[string]any{"title": "t", "milestone": float64(2)}, want: true}, - {name: "type present", args: map[string]any{"title": "t", "type": "Bug"}, want: true}, + {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}, + {name: "type present", args: map[string]any{"title": "t", "type": "Bug"}, want: false}, {name: "issue_fields present", args: map[string]any{"issue_fields": []any{map[string]any{"field_name": "Priority"}}}, want: false}, {name: "state present", args: map[string]any{"state": "closed"}, want: false}, {name: "state_reason present", args: map[string]any{"state_reason": "completed"}, want: false}, {name: "duplicate_of present", args: map[string]any{"duplicate_of": float64(7)}, want: false}, + {name: "unknown non-schema param present", args: map[string]any{"title": "t", "not_a_real_param": "x"}, want: true}, {name: "nil value is ignored", args: map[string]any{"issue_fields": nil}, want: false}, } @@ -1796,13 +1797,11 @@ 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. - knownNonForm := map[string]struct{}{ - "assignees": {}, - "labels": {}, - "milestone": {}, - "type": {}, - } + // must trigger the safety-net bypass via issueWriteHasNonFormParams. 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. + knownNonForm := map[string]struct{}{} cases := []struct { name string diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index ef3e9c083..bf0eaa29b 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -717,7 +717,7 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo // 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 you have all required values (especially ones the form does not collect, like reviewers) and the user has already confirmed the action.", + 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"},