Skip to content

Commit bb391a3

Browse files
Merge branch 'main' into mattdholloway-ui-get-pagination-guard
2 parents 8fe59c3 + 3c4749d commit bb391a3

7 files changed

Lines changed: 34 additions & 29 deletions

File tree

docs/feature-flags.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ runtime behavior (such as output formatting) won't appear here.
4545
- `owner`: Repository owner (string, required)
4646
- `repo`: Repository name (string, required)
4747
- `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], optional)
48-
- `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)
48+
- `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)
4949
- `title`: PR title (string, required)
5050

5151
- **get_me** - Get my user profile
@@ -69,7 +69,7 @@ runtime behavior (such as output formatting) won't appear here.
6969
- `milestone`: Milestone number (number, optional)
7070
- `owner`: Repository owner (string, required)
7171
- `repo`: Repository name (string, required)
72-
- `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)
72+
- `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)
7373
- `state`: New state (string, optional)
7474
- `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
7575
- `title`: Issue title (string, optional)

docs/insiders-features.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ The list below is generated from the Go source. It covers tool **inventory and s
3939
- `owner`: Repository owner (string, required)
4040
- `repo`: Repository name (string, required)
4141
- `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], optional)
42-
- `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)
42+
- `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)
4343
- `title`: PR title (string, required)
4444

4545
- **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
6363
- `milestone`: Milestone number (number, optional)
6464
- `owner`: Repository owner (string, required)
6565
- `repo`: Repository name (string, required)
66-
- `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)
66+
- `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)
6767
- `state`: New state (string, optional)
6868
- `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
6969
- `title`: Issue title (string, optional)

pkg/github/__toolsnaps__/create_pull_request.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
"type": "array"
5151
},
5252
"show_ui": {
53-
"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.",
53+
"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.",
5454
"type": "boolean"
5555
},
5656
"title": {

pkg/github/__toolsnaps__/issue_write.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@
9797
"type": "string"
9898
},
9999
"show_ui": {
100-
"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.",
100+
"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.",
101101
"type": "boolean"
102102
},
103103
"state": {

pkg/github/issues.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,6 +1735,10 @@ var issueWriteFormParams = map[string]struct{}{
17351735
"body": {},
17361736
"issue_number": {},
17371737
"issue_fields": {},
1738+
"labels": {},
1739+
"assignees": {},
1740+
"milestone": {},
1741+
"type": {},
17381742
"state": {},
17391743
"state_reason": {},
17401744
"duplicate_of": {},
@@ -1743,9 +1747,11 @@ var issueWriteFormParams = map[string]struct{}{
17431747
}
17441748

17451749
// issueWriteHasNonFormParams reports whether the call carries any parameter the
1746-
// issue_write MCP App form cannot represent (anything outside issueWriteFormParams,
1747-
// e.g. labels, assignees, milestones or issue types). Such calls must bypass
1748-
// the UI form and execute directly so the supplied values aren't silently dropped.
1750+
// issue_write MCP App form cannot represent (anything outside issueWriteFormParams).
1751+
// The form collects (and prefills) every parameter in the tool's current input
1752+
// schema, so this is a forward-compatibility safety net: a parameter added to the
1753+
// schema in the future but not yet wired into the form trips this check and bypasses
1754+
// the UI so the supplied value isn't silently dropped.
17491755
func issueWriteHasNonFormParams(args map[string]any) bool {
17501756
for key, value := range args {
17511757
if value == nil {
@@ -1922,7 +1928,7 @@ Options are:
19221928
// which renders the stripped (non-UI) schema.
19231929
"show_ui": {
19241930
Type: "boolean",
1925-
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.",
1931+
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.",
19261932
},
19271933
},
19281934
Required: []string{"method", "owner", "repo"},

pkg/github/issues_test.go

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1655,9 +1655,10 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {
16551655
assert.True(t, result.IsError, "form-routing stub should be marked IsError so agents don't claim success")
16561656
})
16571657

1658-
t.Run("UI client with labels skips form and executes directly", func(t *testing.T) {
1659-
// The form does not collect labels, so a call carrying them must bypass
1660-
// the form rather than silently drop them.
1658+
t.Run("UI client with labels routes through UI form", func(t *testing.T) {
1659+
// labels is now a form param (the issue-write view prefills and renders
1660+
// a label selector), so a call carrying them must route to the form
1661+
// rather than execute directly.
16611662
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
16621663
"method": "create",
16631664
"owner": "owner",
@@ -1669,10 +1670,9 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {
16691670
require.NoError(t, err)
16701671

16711672
textContent := getTextResult(t, result)
1672-
assert.NotContains(t, textContent.Text, "interactive form has been shown",
1673-
"labels should skip UI form")
1674-
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
1675-
"labels call should execute directly and return issue URL")
1673+
assert.Contains(t, textContent.Text, "interactive form has been shown to the user for creating a new issue",
1674+
"labels should route through UI form")
1675+
assert.True(t, result.IsError, "form-routing stub should be marked IsError so agents don't claim success")
16761676
})
16771677

16781678
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) {
17681768
{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},
17691769
{name: "show_ui true is a form param", args: map[string]any{"title": "t", "show_ui": true}, want: false},
17701770
{name: "show_ui false is a form param", args: map[string]any{"title": "t", "show_ui": false}, want: false},
1771-
{name: "labels present", args: map[string]any{"title": "t", "labels": []any{"bug"}}, want: true},
1772-
{name: "assignees present", args: map[string]any{"title": "t", "assignees": []any{"octocat"}}, want: true},
1773-
{name: "milestone present", args: map[string]any{"title": "t", "milestone": float64(2)}, want: true},
1774-
{name: "type present", args: map[string]any{"title": "t", "type": "Bug"}, want: true},
1771+
{name: "labels present", args: map[string]any{"title": "t", "labels": []any{"bug"}}, want: false},
1772+
{name: "assignees present", args: map[string]any{"title": "t", "assignees": []any{"octocat"}}, want: false},
1773+
{name: "milestone present", args: map[string]any{"title": "t", "milestone": float64(2)}, want: false},
1774+
{name: "type present", args: map[string]any{"title": "t", "type": "Bug"}, want: false},
17751775
{name: "issue_fields present", args: map[string]any{"issue_fields": []any{map[string]any{"field_name": "Priority"}}}, want: false},
17761776
{name: "state present", args: map[string]any{"state": "closed"}, want: false},
17771777
{name: "state_reason present", args: map[string]any{"state_reason": "completed"}, want: false},
17781778
{name: "duplicate_of present", args: map[string]any{"duplicate_of": float64(7)}, want: false},
1779+
{name: "unknown non-schema param present", args: map[string]any{"title": "t", "not_a_real_param": "x"}, want: true},
17791780
{name: "nil value is ignored", args: map[string]any{"issue_fields": nil}, want: false},
17801781
}
17811782

@@ -1796,13 +1797,11 @@ func Test_issueWriteSchemaClassification(t *testing.T) {
17961797
t.Parallel()
17971798

17981799
// Schema properties the MCP App form cannot represent — their presence
1799-
// must trigger the safety-net bypass via issueWriteHasNonFormParams.
1800-
knownNonForm := map[string]struct{}{
1801-
"assignees": {},
1802-
"labels": {},
1803-
"milestone": {},
1804-
"type": {},
1805-
}
1800+
// must trigger the safety-net bypass via issueWriteHasNonFormParams. The
1801+
// form currently collects every schema property, so this allowlist is
1802+
// empty; add a property here only if it is added to the schema without
1803+
// corresponding form support.
1804+
knownNonForm := map[string]struct{}{}
18061805

18071806
cases := []struct {
18081807
name string

pkg/github/pullrequests.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
717717
// which renders the stripped (non-UI) schema.
718718
"show_ui": {
719719
Type: "boolean",
720-
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.",
720+
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.",
721721
},
722722
},
723723
Required: []string{"owner", "repo", "title", "head", "base"},

0 commit comments

Comments
 (0)