From a2f6bdc905b6d5d03b48abc5a47618f0b4ba5f1f Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Thu, 25 Jun 2026 10:57:26 +0100 Subject: [PATCH] Show issue_write MCP App form for labels/assignees/milestone/type The issue_write form-gating logic uses issueWriteFormParams to decide whether to render the MCP App form or execute directly: a call carrying any parameter outside that allowlist bypasses the form. The form prefills and re-submits labels, assignees, milestone and type, but those four were absent from the allowlist, so passing any of them skipped the confirmation form even though the form fully supports them. Add labels, assignees, milestone and type to issueWriteFormParams so the form is shown when they are present. The allowlist now covers every input-schema property, leaving issueWriteHasNonFormParams as a forward-compatibility safety net for properties added without form support. Also correct the now-inaccurate show_ui guidance on issue_write and create_pull_request: both descriptions claimed the form "does not collect" fields it actually collects (labels/assignees/milestone/type/issue_fields, and reviewers respectively). Update unit tests and regenerate toolsnaps and docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/feature-flags.md | 4 +-- docs/insiders-features.md | 4 +-- .../__toolsnaps__/create_pull_request.snap | 2 +- pkg/github/__toolsnaps__/issue_write.snap | 2 +- pkg/github/issues.go | 14 +++++--- pkg/github/issues_test.go | 35 +++++++++---------- pkg/github/pullrequests.go | 2 +- 7 files changed, 34 insertions(+), 29 deletions(-) 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"},