Skip to content

Commit d9be621

Browse files
mattdhollowaymattdholloway
committed
feat: implement interactive form handling for issue and pull request creation and updates
1 parent 0eccc22 commit d9be621

5 files changed

Lines changed: 186 additions & 15 deletions

File tree

‎pkg/github/issues.go‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1796,6 +1796,36 @@ func issueWriteHasNonFormParams(args map[string]any) bool {
17961796
return false
17971797
}
17981798

1799+
// issueWriteAwaitingFormResult builds the "awaiting form submission" stub
1800+
// returned when issue_write hands off to the MCP App form. The body is shared
1801+
// by IssueWrite and LegacyIssueWrite. The result is marked IsError=true so
1802+
// agents that bail on error don't claim success or chain dependent tool calls
1803+
// while the user is still interacting with the form; the host renders the UI
1804+
// regardless because rendering is keyed off the tool's _meta.ui resourceUri.
1805+
func issueWriteAwaitingFormResult(method, owner, repo string, issueNumber int) *mcp.CallToolResult {
1806+
var msg string
1807+
if method == "update" {
1808+
msg = fmt.Sprintf(
1809+
"An interactive form has been shown to the user for editing issue #%d in %s/%s. "+
1810+
"STOP — do not call any other tools, do not respond as if the issue was updated, "+
1811+
"and do not claim the operation succeeded. The issue has NOT been updated yet; "+
1812+
"only the form was rendered. Wait silently for the user to review and click Submit. "+
1813+
"When they do, the real result will be delivered to your context automatically.",
1814+
issueNumber, owner, repo,
1815+
)
1816+
} else {
1817+
msg = fmt.Sprintf(
1818+
"An interactive form has been shown to the user for creating a new issue in %s/%s. "+
1819+
"STOP — do not call any other tools, do not respond as if the issue was created, "+
1820+
"and do not claim the operation succeeded. The issue has NOT been created yet; "+
1821+
"only the form was rendered. Wait silently for the user to review and click Submit. "+
1822+
"When they do, the real result will be delivered to your context automatically.",
1823+
owner, repo,
1824+
)
1825+
}
1826+
return utils.NewToolResultAwaitingFormSubmission(msg)
1827+
}
1828+
17991829
// IssueWrite is the FeatureFlagIssueFields-enabled variant of issue_write
18001830
// (with the issue_fields parameter). LegacyIssueWrite is served when the flag
18011831
// is off. Both register under the tool name "issue_write"; exactly one is
@@ -1949,14 +1979,15 @@ Options are:
19491979
uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
19501980

19511981
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !issueWriteHasNonFormParams(args) {
1982+
issueNumber := 0
19521983
if method == "update" {
1953-
issueNumber, numErr := RequiredInt(args, "issue_number")
1984+
n, numErr := RequiredInt(args, "issue_number")
19541985
if numErr != nil {
19551986
return utils.NewToolResultError("issue_number is required for update method"), nil, nil
19561987
}
1957-
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
1988+
issueNumber = n
19581989
}
1959-
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
1990+
return issueWriteAwaitingFormResult(method, owner, repo, issueNumber), nil, nil
19601991
}
19611992

19621993
title, err := OptionalParam[string](args, "title")
@@ -2181,14 +2212,15 @@ Options are:
21812212
uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
21822213

21832214
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !issueWriteHasNonFormParams(args) {
2215+
issueNumber := 0
21842216
if method == "update" {
2185-
issueNumber, numErr := RequiredInt(args, "issue_number")
2217+
n, numErr := RequiredInt(args, "issue_number")
21862218
if numErr != nil {
21872219
return utils.NewToolResultError("issue_number is required for update method"), nil, nil
21882220
}
2189-
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
2221+
issueNumber = n
21902222
}
2191-
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
2223+
return issueWriteAwaitingFormResult(method, owner, repo, issueNumber), nil, nil
21922224
}
21932225

21942226
title, err := OptionalParam[string](args, "title")

‎pkg/github/issues_test.go‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1561,7 +1561,8 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {
15611561
require.NoError(t, err)
15621562

15631563
textContent := getTextResult(t, result)
1564-
assert.Contains(t, textContent.Text, "Ready to create an issue")
1564+
assert.Contains(t, textContent.Text, "interactive form has been shown to the user for creating a new issue")
1565+
assert.True(t, result.IsError, "form-routing stub should be marked IsError so agents don't claim success")
15651566
})
15661567

15671568
t.Run("UI client with _ui_submitted executes directly", func(t *testing.T) {
@@ -1611,8 +1612,9 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {
16111612
require.NoError(t, err)
16121613

16131614
textContent := getTextResult(t, result)
1614-
assert.Contains(t, textContent.Text, "Ready to update issue #1",
1615+
assert.Contains(t, textContent.Text, "interactive form has been shown to the user for editing issue #1",
16151616
"state change should route through UI form")
1617+
assert.True(t, result.IsError, "form-routing stub should be marked IsError so agents don't claim success")
16161618
})
16171619

16181620
t.Run("UI client update without state change returns form message", func(t *testing.T) {
@@ -1627,8 +1629,9 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {
16271629
require.NoError(t, err)
16281630

16291631
textContent := getTextResult(t, result)
1630-
assert.Contains(t, textContent.Text, "Ready to update issue #1",
1632+
assert.Contains(t, textContent.Text, "interactive form has been shown to the user for editing issue #1",
16311633
"update without state should show UI form")
1634+
assert.True(t, result.IsError, "form-routing stub should be marked IsError so agents don't claim success")
16321635
})
16331636

16341637
t.Run("UI client with issue_fields routes through UI form", func(t *testing.T) {
@@ -1648,8 +1651,9 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {
16481651
require.NoError(t, err)
16491652

16501653
textContent := getTextResult(t, result)
1651-
assert.Contains(t, textContent.Text, "Ready to create an issue",
1654+
assert.Contains(t, textContent.Text, "interactive form has been shown to the user for creating a new issue",
16521655
"issue_fields should route through UI form")
1656+
assert.True(t, result.IsError, "form-routing stub should be marked IsError so agents don't claim success")
16531657
})
16541658

16551659
t.Run("UI client with labels skips form and executes directly", func(t *testing.T) {
@@ -1666,7 +1670,7 @@ func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) {
16661670
require.NoError(t, err)
16671671

16681672
textContent := getTextResult(t, result)
1669-
assert.NotContains(t, textContent.Text, "Ready to create an issue",
1673+
assert.NotContains(t, textContent.Text, "interactive form has been shown",
16701674
"labels should skip UI form")
16711675
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1",
16721676
"labels call should execute directly and return issue URL")

‎pkg/github/pullrequests.go‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,14 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
687687
uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
688688

689689
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !pullRequestWriteHasNonFormParams(args) {
690-
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
690+
return utils.NewToolResultAwaitingFormSubmission(fmt.Sprintf(
691+
"An interactive form has been shown to the user for creating a new pull request in %s/%s. "+
692+
"STOP — do not call any other tools, do not respond as if the pull request was created, "+
693+
"and do not claim the operation succeeded. The pull request has NOT been created yet; "+
694+
"only the form was rendered. Wait silently for the user to review and click Submit. "+
695+
"When they do, the real result will be delivered to your context automatically.",
696+
owner, repo,
697+
)), nil, nil
691698
}
692699

693700
// When creating PR, title/head/base are required
@@ -900,7 +907,14 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
900907

901908
uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted")
902909
if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted && !pullRequestUpdateHasNonFormParams(args) {
903-
return utils.NewToolResultText(fmt.Sprintf("Ready to update PR #%d in %s/%s. IMPORTANT: The PR has NOT been updated yet. Do NOT tell the user the PR was updated. The user MUST click Submit in the form to update it.", pullNumber, owner, repo)), nil, nil
910+
return utils.NewToolResultAwaitingFormSubmission(fmt.Sprintf(
911+
"An interactive form has been shown to the user for editing pull request #%d in %s/%s. "+
912+
"STOP — do not call any other tools, do not respond as if the pull request was updated, "+
913+
"and do not claim the operation succeeded. The pull request has NOT been updated yet; "+
914+
"only the form was rendered. Wait silently for the user to review and click Submit. "+
915+
"When they do, the real result will be delivered to your context automatically.",
916+
pullNumber, owner, repo,
917+
)), nil, nil
904918
}
905919

906920
_, draftProvided := args["draft"]

‎pkg/github/pullrequests_test.go‎

Lines changed: 99 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2450,7 +2450,8 @@ func Test_CreatePullRequest_MCPAppsFeature_UIGate(t *testing.T) {
24502450
require.NoError(t, err)
24512451

24522452
textContent := getTextResult(t, result)
2453-
assert.Contains(t, textContent.Text, "Ready to create a pull request")
2453+
assert.Contains(t, textContent.Text, "interactive form has been shown to the user for creating a new pull request")
2454+
assert.True(t, result.IsError, "form-routing stub should be marked IsError so agents don't claim success")
24542455
})
24552456

24562457
t.Run("UI client with _ui_submitted executes directly", func(t *testing.T) {
@@ -2501,13 +2502,109 @@ func Test_CreatePullRequest_MCPAppsFeature_UIGate(t *testing.T) {
25012502
require.NoError(t, err)
25022503

25032504
textContent := getTextResult(t, result)
2504-
assert.NotContains(t, textContent.Text, "Ready to create a pull request",
2505+
assert.NotContains(t, textContent.Text, "interactive form has been shown",
25052506
"non-form param should skip UI form")
25062507
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42",
25072508
"non-form param call should execute directly and return PR URL")
25082509
})
25092510
}
25102511

2512+
// Test_UpdatePullRequest_MCPAppsFeature_UIGate verifies the form-routing
2513+
// behavior for update_pull_request: UI clients without _ui_submitted get a
2514+
// pending-form stub (marked IsError so agents don't claim success), UI clients
2515+
// with _ui_submitted execute directly, non-UI clients execute directly, and
2516+
// UI clients carrying non-form params bypass the form.
2517+
func Test_UpdatePullRequest_MCPAppsFeature_UIGate(t *testing.T) {
2518+
t.Parallel()
2519+
2520+
mockPR := &github.PullRequest{
2521+
Number: github.Ptr(42),
2522+
Title: github.Ptr("Updated"),
2523+
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"),
2524+
Head: &github.PullRequestBranch{SHA: github.Ptr("abc"), Ref: github.Ptr("feature")},
2525+
Base: &github.PullRequestBranch{SHA: github.Ptr("def"), Ref: github.Ptr("main")},
2526+
User: &github.User{Login: github.Ptr("testuser")},
2527+
}
2528+
2529+
serverTool := UpdatePullRequest(translations.NullTranslationHelper)
2530+
2531+
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
2532+
PatchReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR),
2533+
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR),
2534+
}))
2535+
2536+
deps := BaseDeps{
2537+
Client: client,
2538+
GQLClient: githubv4.NewClient(nil),
2539+
featureChecker: featureCheckerFor(MCPAppsFeatureFlag),
2540+
}
2541+
handler := serverTool.Handler(deps)
2542+
2543+
t.Run("UI client without _ui_submitted returns form message", func(t *testing.T) {
2544+
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
2545+
"owner": "owner",
2546+
"repo": "repo",
2547+
"pullNumber": float64(42),
2548+
"title": "Updated",
2549+
})
2550+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
2551+
require.NoError(t, err)
2552+
2553+
textContent := getTextResult(t, result)
2554+
assert.Contains(t, textContent.Text, "interactive form has been shown to the user for editing pull request #42")
2555+
assert.True(t, result.IsError, "form-routing stub should be marked IsError so agents don't claim success")
2556+
})
2557+
2558+
t.Run("UI client with _ui_submitted executes directly", func(t *testing.T) {
2559+
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
2560+
"owner": "owner",
2561+
"repo": "repo",
2562+
"pullNumber": float64(42),
2563+
"title": "Updated",
2564+
"_ui_submitted": true,
2565+
})
2566+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
2567+
require.NoError(t, err)
2568+
2569+
textContent := getTextResult(t, result)
2570+
assert.False(t, result.IsError, "submitted form should execute successfully: %s", textContent.Text)
2571+
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42",
2572+
"submitted form should return the updated PR URL")
2573+
})
2574+
2575+
t.Run("non-UI client executes directly without _ui_submitted", func(t *testing.T) {
2576+
request := createMCPRequest(map[string]any{
2577+
"owner": "owner",
2578+
"repo": "repo",
2579+
"pullNumber": float64(42),
2580+
"title": "Updated",
2581+
})
2582+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
2583+
require.NoError(t, err)
2584+
2585+
textContent := getTextResult(t, result)
2586+
assert.False(t, result.IsError, "non-UI client should execute directly: %s", textContent.Text)
2587+
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42",
2588+
"non-UI client should return the updated PR URL")
2589+
})
2590+
2591+
t.Run("UI client with non-form param skips form and executes directly", func(t *testing.T) {
2592+
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
2593+
"owner": "owner",
2594+
"repo": "repo",
2595+
"pullNumber": float64(42),
2596+
"title": "Updated",
2597+
"unknown_param": "value",
2598+
})
2599+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
2600+
require.NoError(t, err)
2601+
2602+
textContent := getTextResult(t, result)
2603+
assert.NotContains(t, textContent.Text, "interactive form has been shown",
2604+
"non-form param should skip UI form")
2605+
})
2606+
}
2607+
25112608
func Test_pullRequestWriteHasNonFormParams(t *testing.T) {
25122609
t.Parallel()
25132610

‎pkg/utils/result.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,27 @@ func NewToolResultResourceLink(message string, link *mcp.ResourceLink) *mcp.Call
5959
IsError: false,
6060
}
6161
}
62+
63+
// NewToolResultAwaitingFormSubmission signals to the agent that a tool call
64+
// has been intercepted to show an MCP App form to the user and has NOT
65+
// performed the requested operation. The agent must stop, not chain dependent
66+
// tool calls, and not claim the operation succeeded. The result is marked
67+
// IsError=true so agents that bail on error don't proceed; the host still
68+
// renders the UI because rendering is keyed off the tool's _meta.ui, not the
69+
// result. The MCP App form will submit the operation directly when the user
70+
// clicks submit, after which a ui/update-model-context call delivers the real
71+
// outcome to the agent.
72+
func NewToolResultAwaitingFormSubmission(message string) *mcp.CallToolResult {
73+
return &mcp.CallToolResult{
74+
Content: []mcp.Content{
75+
&mcp.TextContent{
76+
Text: message,
77+
},
78+
},
79+
StructuredContent: map[string]any{
80+
"status": "awaiting_user_submission",
81+
"reason": "An interactive form is being shown to the user. The operation has not been performed.",
82+
},
83+
IsError: true,
84+
}
85+
}

0 commit comments

Comments
 (0)