Skip to content

Skip MCP App form when issue/PR write carries non-form params #2589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattdholloway merged 2 commits into from
Jun 1, 2026
Merged

Skip MCP App form when issue/PR write carries non-form params #2589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3094759
Skip MCP App form when issue/PR write carries non-form params
mattdholloway Jun 1, 2026
95b05a8
Forward original tool params on MCP App form submit
mattdholloway Jun 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 52 additions & 30 deletions pkg/github/issues.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1763,6 +1763,36 @@ func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[st
// IssueWriteUIResourceURI is the URI for the issue_write tool's MCP App UI resource.
const IssueWriteUIResourceURI = "ui://github-mcp-server/issue-write"

// issueWriteFormParams are the parameters the issue_write MCP App form collects
// and re-sends on submit. The form only supports title/body editing (plus the
// routing/identity fields), so any other parameter present on a call cannot be
// represented by the form.
var issueWriteFormParams = map[string]struct{}{
"method": {},
"owner": {},
"repo": {},
"title": {},
"body": {},
"issue_number": {},
"_ui_submitted": {},
}

// issueWriteHasNonFormParams reports whether the call carries any parameter the
// issue_write MCP App form cannot represent (anything outside issueWriteFormParams,
// e.g. labels, assignees, issue_fields or a state change). Such calls must bypass
// the UI form and execute directly so the supplied values aren'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
}

// IssueWrite is the FeatureFlagIssueFields-enabled variant of issue_write
// (with the issue_fields parameter). LegacyIssueWrite is served when the flag
// is off. Both register under the tool name "issue_write"; exactly one is
Expand Down Expand Up @@ -1908,26 +1938,22 @@ Options are:
return utils.NewToolResultError(err.Error()), nil, nil
}

// When MCP Apps are enabled and the client supports UI,
// check if this is a UI form submission. The UI sends _ui_submitted=true
// to distinguish form submissions from LLM calls.
// 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) 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")

if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted {
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !issueWriteHasNonFormParams(args) {
if method == "update" {
// Skip the UI form when a state change is requested because
// the form only handles title/body editing and would lose the
// state transition (e.g. closing or reopening the issue).
if _, hasState := args["state"]; !hasState {
issueNumber, numErr := RequiredInt(args, "issue_number")
if numErr != nil {
return utils.NewToolResultError("issue_number is required for update method"), nil, nil
}
return utils.NewToolResultText(fmt.Sprintf("Ready to update issue #%d in %s/%s. IMPORTANT: The issue has NOT been updated yet. Do NOT tell the user the issue was updated. The user MUST click Submit in the form to update it.", issueNumber, owner, repo)), nil, nil
issueNumber, numErr := RequiredInt(args, "issue_number")
if numErr != nil {
return utils.NewToolResultError("issue_number is required for update method"), nil, nil
}
} else {
return utils.NewToolResultText(fmt.Sprintf("Ready to create an issue in %s/%s. IMPORTANT: The issue has NOT been created yet. Do NOT tell the user the issue was created. The user MUST click Submit in the form to create it.", owner, repo)), nil, nil
return utils.NewToolResultText(fmt.Sprintf("Ready to update issue #%d in %s/%s. IMPORTANT: The issue has NOT been updated yet. Do NOT tell the user the issue was updated. The user MUST click Submit in the form to update it.", issueNumber, owner, repo)), nil, nil
}
return utils.NewToolResultText(fmt.Sprintf("Ready to create an issue in %s/%s. IMPORTANT: The issue has NOT been created yet. Do NOT tell the user the issue was created. The user MUST click Submit in the form to create it.", owner, repo)), nil, nil
}

title, err := OptionalParam[string](args, "title")
Expand Down Expand Up @@ -2144,26 +2170,22 @@ Options are:
return utils.NewToolResultError(err.Error()), nil, nil
}

// When MCP Apps are enabled and the client supports UI,
// check if this is a UI form submission. The UI sends _ui_submitted=true
// to distinguish form submissions from LLM calls.
// 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) 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")

if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted {
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !issueWriteHasNonFormParams(args) {
if method == "update" {
// Skip the UI form when a state change is requested because
// the form only handles title/body editing and would lose the
// state transition (e.g. closing or reopening the issue).
if _, hasState := args["state"]; !hasState {
issueNumber, numErr := RequiredInt(args, "issue_number")
if numErr != nil {
return utils.NewToolResultError("issue_number is required for update method"), nil, nil
}
return utils.NewToolResultText(fmt.Sprintf("Ready to update issue #%d in %s/%s. IMPORTANT: The issue has NOT been updated yet. Do NOT tell the user the issue was updated. The user MUST click Submit in the form to update it.", issueNumber, owner, repo)), nil, nil
issueNumber, numErr := RequiredInt(args, "issue_number")
if numErr != nil {
return utils.NewToolResultError("issue_number is required for update method"), nil, nil
}
} else {
return utils.NewToolResultText(fmt.Sprintf("Ready to create an issue in %s/%s. IMPORTANT: The issue has NOT been created yet. Do NOT tell the user the issue was created. The user MUST click Submit in the form to create it.", owner, repo)), nil, nil
return utils.NewToolResultText(fmt.Sprintf("Ready to update issue #%d in %s/%s. IMPORTANT: The issue has NOT been updated yet. Do NOT tell the user the issue was updated. The user MUST click Submit in the form to update it.", issueNumber, owner, repo)), nil, nil
}
return utils.NewToolResultText(fmt.Sprintf("Ready to create an issue in %s/%s. IMPORTANT: The issue has NOT been created yet. Do NOT tell the user the issue was created. The user MUST click Submit in the form to create it.", owner, repo)), nil, nil
}

title, err := OptionalParam[string](args, "title")
Expand Down
123 changes: 123 additions & 0 deletions pkg/github/issues_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,129 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {
assert.Contains(t, textContent.Text, "Ready to update issue #1",
"update without state should show UI form")
})

t.Run("UI client with issue_fields skips form and executes directly", func(t *testing.T) {
// The MCP App form does not collect or re-send issue_fields, so a call
// carrying them must bypass the form and apply the values directly.
fieldsClient := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PostReposIssuesByOwnerByRepo: expectRequestBody(t, map[string]any{
"title": "Issue with fields",
"body": "",
"labels": []any{},
"assignees": []any{},
"issue_field_values": []any{
map[string]any{"field_id": float64(101), "value": "P1"},
},
}).andThen(
mockResponse(t, http.StatusCreated, &github.Issue{
Number: github.Ptr(125),
Title: github.Ptr("Issue with fields"),
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/125"),
State: github.Ptr("open"),
}),
),
}))
fieldsGQLClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
issueFieldWriteMetadataQuery{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
},
githubv4mock.DataResponse(map[string]any{
"repository": map[string]any{
"issueFields": map[string]any{
"nodes": []any{
map[string]any{
"__typename": "IssueFieldSingleSelect",
"fullDatabaseId": "101",
"name": "Priority",
"dataType": "single_select",
"options": []any{
map[string]any{"fullDatabaseId": "9001", "name": "P1"},
},
},
},
},
},
}),
),
))

fieldsDeps := BaseDeps{
Client: fieldsClient,
GQLClient: fieldsGQLClient,
featureChecker: featureCheckerFor(MCPAppsFeatureFlag),
}
fieldsHandler := serverTool.Handler(fieldsDeps)

request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"title": "Issue with fields",
"issue_fields": []any{
map[string]any{"field_name": "Priority", "field_option_name": "P1"},
},
})
result, err := fieldsHandler(ContextWithDeps(context.Background(), fieldsDeps), &request)
require.NoError(t, err)

textContent := getTextResult(t, result)
assert.NotContains(t, textContent.Text, "Ready to create an issue",
"issue_fields should skip UI form")
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/125",
"issue_fields call should execute directly and return issue URL")
})

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.
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"title": "Test",
"labels": []any{"bug"},
})
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)

textContent := getTextResult(t, result)
assert.NotContains(t, textContent.Text, "Ready to create an issue",
"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")
})
}

func Test_issueWriteHasNonFormParams(t *testing.T) {
t.Parallel()

tests := []struct {
name string
args map[string]any
want bool
}{
{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: "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: "issue_fields present", args: map[string]any{"issue_fields": []any{map[string]any{"field_name": "Priority"}}}, want: true},
{name: "state present", args: map[string]any{"state": "closed"}, want: true},
{name: "state_reason present", args: map[string]any{"state_reason": "completed"}, want: true},
{name: "duplicate_of present", args: map[string]any{"duplicate_of": float64(7)}, want: true},
{name: "nil value is ignored", args: map[string]any{"issue_fields": nil}, want: false},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tc.want, issueWriteHasNonFormParams(tc.args))
})
}
}

func Test_ListIssues(t *testing.T) {
Expand Down
41 changes: 37 additions & 4 deletions pkg/github/pullrequests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,37 @@ func GetPullRequestReviews(ctx context.Context, client *github.Client, deps Tool
// PullRequestWriteUIResourceURI is the URI for the create_pull_request tool's MCP App UI resource.
const PullRequestWriteUIResourceURI = "ui://github-mcp-server/pr-write"

// pullRequestWriteFormParams are the parameters the create_pull_request MCP App
// form collects and re-sends on submit. Any other parameter present on a call
// cannot be represented by the form.
var pullRequestWriteFormParams = map[string]struct{}{
"owner": {},
"repo": {},
"title": {},
"body": {},
"head": {},
"base": {},
"draft": {},
"maintainer_can_modify": {},
"_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
}

// CreatePullRequest creates a tool to create a new pull request.
func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerTool {
return NewTool(
Expand Down Expand Up @@ -611,12 +642,14 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
return utils.NewToolResultError(err.Error()), nil, nil
}

// When MCP Apps are enabled and the client supports UI,
// check if this is a UI form submission. The UI sends _ui_submitted=true
// to distinguish form submissions from LLM calls.
// 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) 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")

if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted {
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !pullRequestWriteHasNonFormParams(args) {
return utils.NewToolResultText(fmt.Sprintf("Ready to create a pull request in %s/%s. IMPORTANT: The PR has NOT been created yet. Do NOT tell the user the PR was created. The user MUST click Submit in the form to create it.", owner, repo)), nil, nil
}

Expand Down
43 changes: 43 additions & 0 deletions pkg/github/pullrequests_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2485,6 +2485,49 @@ func Test_CreatePullRequest_MCPAppsFeature_UIGate(t *testing.T) {
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42",
"non-UI client should execute directly")
})

t.Run("UI client with non-form param skips form and executes directly", func(t *testing.T) {
// A parameter the form does not collect must bypass the form rather than
// be silently dropped.
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
"owner": "owner",
"repo": "repo",
"title": "Test PR",
"head": "feature",
"base": "main",
"reviewers": []any{"octocat"},
})
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)

textContent := getTextResult(t, result)
assert.NotContains(t, textContent.Text, "Ready to create a pull request",
"non-form param should skip UI form")
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42",
"non-form param call should execute directly and return PR URL")
})
}

func Test_pullRequestWriteHasNonFormParams(t *testing.T) {
t.Parallel()

tests := []struct {
name string
args map[string]any
want bool
}{
{name: "no params", args: map[string]any{}, want: false},
{name: "only form params", args: map[string]any{"owner": "o", "repo": "r", "title": "t", "body": "b", "head": "h", "base": "b", "draft": true, "maintainer_can_modify": false, "_ui_submitted": true}, want: false},
{name: "unknown param present", args: map[string]any{"title": "t", "reviewers": []any{"octocat"}}, want: true},
{name: "nil value is ignored", args: map[string]any{"reviewers": nil}, want: false},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tc.want, pullRequestWriteHasNonFormParams(tc.args))
})
}
}

func TestCreateAndSubmitPullRequestReview(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion ui/src/apps/issue-write/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ function CreateIssueApp() {

try {
const params: Record<string, unknown> = {
...(toolInput as Record<string, unknown> | undefined),
method: isUpdateMode ? "update" : "create",
owner,
repo,
Expand Down Expand Up @@ -204,7 +205,7 @@ function CreateIssueApp() {
} finally {
setIsSubmitting(false);
}
}, [title, body, owner, repo, isUpdateMode, issueNumber, callTool, setModelContext]);
}, [title, body, owner, repo, isUpdateMode, issueNumber, toolInput, callTool, setModelContext]);

const body_node = (() => {
if (appError) {
Expand Down
3 changes: 2 additions & 1 deletion ui/src/apps/pr-write/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ function CreatePRApp() {

try {
const result = await callTool("create_pull_request", {
...(toolInput as Record<string, unknown> | undefined),
owner, repo,
title: title.trim(),
body: body.trim(),
Expand Down Expand Up @@ -193,7 +194,7 @@ function CreatePRApp() {
} finally {
setIsSubmitting(false);
}
}, [title, body, owner, repo, head, base, isDraft, maintainerCanModify, callTool, setModelContext]);
}, [title, body, owner, repo, head, base, isDraft, maintainerCanModify, toolInput, callTool, setModelContext]);

if (successPR) {
return (
Expand Down
Loading