Merge branch 'main' into timrogers/planning-reaction-tools

Author: SamMorrowDrums

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)

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)

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": {

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": {

pkg/github/issues.go

@@ -1816,6 +1816,10 @@ var issueWriteFormParams = map[string]struct{}{
 	"body":          {},
 	"issue_number":  {},
 	"issue_fields":  {},
+	"labels":        {},
+	"assignees":     {},
+	"milestone":     {},
+	"type":          {},
 	"state":         {},
 	"state_reason":  {},
 	"duplicate_of":  {},
@@ -1824,9 +1828,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 {
@@ -2003,7 +2009,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"},

pkg/github/issues_test.go

@@ -1652,9 +1652,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",
@@ -1666,10 +1667,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) {
@@ -1765,14 +1765,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},
 	}
 
@@ -1793,13 +1794,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

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"},

pkg/github/ui_tools.go

@@ -20,6 +20,15 @@ import (
 	"github.com/shurcooL/githubv4"
 )
 
+// uiGetMaxPages bounds how many pages each ui_get pagination loop will fetch.
+// ui_get backs a synchronous UI picker (dropdowns for labels/assignees/etc. in
+// the MCP App issue/PR write surfaces), so responsiveness matters more than
+// completeness. At PerPage 100 this caps a call at ~1000 items and a bounded
+// number of API round-trips, keeping latency predictable on very large
+// repos/orgs. Results past the cap are truncated and surfaced via a "has_more"
+// flag, which is acceptable because the picker pairs truncation with typeahead.
+const uiGetMaxPages = 10
+
 // UIGet creates a tool to fetch UI data for MCP Apps.
 func UIGet(t translations.TranslationHelperFunc) inventory.ServerTool {
 	st := NewTool(
@@ -131,7 +140,8 @@ func uiGetLabels(ctx context.Context, deps ToolDependencies, args map[string]any
 
 	labels := make([]map[string]any, 0)
 	var totalCount int
-	for {
+	hasMore := false
+	for page := 1; ; page++ {
 		if err := client.Query(ctx, &query, vars); err != nil {
 			return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to list labels", err), nil, nil
 		}
@@ -147,12 +157,17 @@ func uiGetLabels(ctx context.Context, deps ToolDependencies, args map[string]any
 		if !query.Repository.Labels.PageInfo.HasNextPage {
 			break
 		}
+		if page >= uiGetMaxPages {
+			hasMore = true
+			break
+		}
 		vars["cursor"] = githubv4.NewString(query.Repository.Labels.PageInfo.EndCursor)
 	}
 
 	response := map[string]any{
 		"labels":     labels,
 		"totalCount": totalCount,
+		"has_more":   hasMore,
 	}
 
 	out, err := json.Marshal(response)
@@ -176,8 +191,9 @@ func uiGetAssignees(ctx context.Context, deps ToolDependencies, args map[string]
 
 	opts := &github.ListOptions{PerPage: 100}
 	var allAssignees []*github.User
+	hasMore := false
 
-	for {
+	for page := 1; ; page++ {
 		assignees, resp, err := client.Issues.ListAssignees(ctx, owner, repo, opts)
 		if err != nil {
 			return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list assignees", resp, err), nil, nil
@@ -189,6 +205,10 @@ func uiGetAssignees(ctx context.Context, deps ToolDependencies, args map[string]
 		if resp.NextPage == 0 {
 			break
 		}
+		if page >= uiGetMaxPages {
+			hasMore = true
+			break
+		}
 		opts.Page = resp.NextPage
 	}
 
@@ -203,6 +223,7 @@ func uiGetAssignees(ctx context.Context, deps ToolDependencies, args map[string]
 	out, err := json.Marshal(map[string]any{
 		"assignees":  result,
 		"totalCount": len(result),
+		"has_more":   hasMore,
 	})
 	if err != nil {
 		return utils.NewToolResultErrorFromErr("failed to marshal assignees", err), nil, nil
@@ -228,7 +249,8 @@ func uiGetMilestones(ctx context.Context, deps ToolDependencies, args map[string
 	}
 
 	var allMilestones []*github.Milestone
-	for {
+	hasMore := false
+	for page := 1; ; page++ {
 		milestones, resp, err := client.Issues.ListMilestones(ctx, owner, repo, opts)
 		if err != nil {
 			return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list milestones", resp, err), nil, nil
@@ -240,6 +262,10 @@ func uiGetMilestones(ctx context.Context, deps ToolDependencies, args map[string
 		if resp.NextPage == 0 {
 			break
 		}
+		if page >= uiGetMaxPages {
+			hasMore = true
+			break
+		}
 		opts.Page = resp.NextPage
 	}
 
@@ -262,6 +288,7 @@ func uiGetMilestones(ctx context.Context, deps ToolDependencies, args map[string
 	out, err := json.Marshal(map[string]any{
 		"milestones": result,
 		"totalCount": len(result),
+		"has_more":   hasMore,
 	})
 	if err != nil {
 		return utils.NewToolResultErrorFromErr("failed to marshal milestones", err), nil, nil
@@ -314,7 +341,8 @@ func uiGetBranches(ctx context.Context, deps ToolDependencies, args map[string]a
 	}
 
 	var allBranches []*github.Branch
-	for {
+	hasMore := false
+	for page := 1; ; page++ {
 		branches, resp, err := client.Repositories.ListBranches(ctx, owner, repo, opts)
 		if err != nil {
 			return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list branches", resp, err), nil, nil
@@ -326,6 +354,10 @@ func uiGetBranches(ctx context.Context, deps ToolDependencies, args map[string]a
 		if resp.NextPage == 0 {
 			break
 		}
+		if page >= uiGetMaxPages {
+			hasMore = true
+			break
+		}
 		opts.Page = resp.NextPage
 	}
 
@@ -337,6 +369,7 @@ func uiGetBranches(ctx context.Context, deps ToolDependencies, args map[string]a
 	r, err := json.Marshal(map[string]any{
 		"branches":   minimalBranches,
 		"totalCount": len(minimalBranches),
+		"has_more":   hasMore,
 	})
 	if err != nil {
 		return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil
@@ -446,7 +479,8 @@ func uiGetReviewers(ctx context.Context, deps ToolDependencies, args map[string]
 		ListOptions: github.ListOptions{PerPage: 100},
 	}
 	var allCollaborators []*github.User
-	for {
+	hasMore := false
+	for page := 1; ; page++ {
 		collaborators, resp, err := client.Repositories.ListCollaborators(ctx, owner, repo, collaboratorOpts)
 		if err != nil {
 			return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list reviewers", resp, err), nil, nil
@@ -458,12 +492,16 @@ func uiGetReviewers(ctx context.Context, deps ToolDependencies, args map[string]
 		if resp.NextPage == 0 {
 			break
 		}
+		if page >= uiGetMaxPages {
+			hasMore = true
+			break
+		}
 		collaboratorOpts.Page = resp.NextPage
 	}
 
 	teamOpts := &github.ListOptions{PerPage: 100}
 	var allTeams []*github.Team
-	for {
+	for page := 1; ; page++ {
 		teams, resp, err := client.Repositories.ListTeams(ctx, owner, repo, teamOpts)
 		if err != nil {
 			return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list reviewer teams", resp, err), nil, nil
@@ -475,6 +513,10 @@ func uiGetReviewers(ctx context.Context, deps ToolDependencies, args map[string]
 		if resp.NextPage == 0 {
 			break
 		}
+		if page >= uiGetMaxPages {
+			hasMore = true
+			break
+		}
 		teamOpts.Page = resp.NextPage
 	}
 
@@ -503,6 +545,7 @@ func uiGetReviewers(ctx context.Context, deps ToolDependencies, args map[string]
 		"users":      users,
 		"teams":      teams,
 		"totalCount": len(users) + len(teams),
+		"has_more":   hasMore,
 	})
 	if err != nil {
 		return utils.NewToolResultErrorFromErr("failed to marshal reviewers", err), nil, nil