From c7a25bcba906ae28a213bb4c52e7548ebae7dc39 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Mar 2026 11:58:18 +0000 Subject: [PATCH 1/3] Initial plan From e939fd73c1f2c89cab9bbda2dfab1c39eb11c99f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Mar 2026 12:04:08 +0000 Subject: [PATCH 2/3] Skip MCP Apps UI form when update includes a state change When using issue_write with method "update" and a state parameter (e.g. "closed"), the MCP Apps UI form was incorrectly shown. The form only handles title/body editing and would lose the state transition. Now when a state change is requested, the UI form is skipped and the update executes directly. Fixes github/github-mcp-server#798 Co-authored-by: mattdholloway <918573+mattdholloway@users.noreply.github.com> --- pkg/github/issues.go | 17 ++++-- pkg/github/issues_test.go | 106 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 5 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 9709a852c..980c355df 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1080,13 +1080,20 @@ Options are: if deps.GetFlags(ctx).InsidersMode && clientSupportsUI(ctx, req) && !uiSubmitted { if method == "update" { - issueNumber, numErr := RequiredInt(args, "issue_number") - if numErr != nil { - return utils.NewToolResultError("issue_number is required for update method"), nil, nil + // 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). + state, _ := OptionalParam[string](args, "state") + if state == "" { + 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 } - 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 + } 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 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") diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index e78a03fcb..d06721be7 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1000,6 +1000,112 @@ func Test_IssueWrite_InsidersMode_UIGate(t *testing.T) { assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1", "non-UI client should execute directly") }) + + t.Run("UI client with state change skips form and executes directly", func(t *testing.T) { + mockBaseIssue := &github.Issue{ + Number: github.Ptr(1), + Title: github.Ptr("Test"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/1"), + } + issueIDQueryResponse := githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{ + "id": "I_kwDOA0xdyM50BPaO", + }, + }, + }) + closeSuccessResponse := githubv4mock.DataResponse(map[string]any{ + "closeIssue": map[string]any{ + "issue": map[string]any{ + "id": "I_kwDOA0xdyM50BPaO", + "number": 1, + "url": "https://github.com/owner/repo/issues/1", + "state": "CLOSED", + }, + }, + }) + completedReason := IssueClosedStateReasonCompleted + + closeClient := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockBaseIssue), + })) + closeGQLClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + struct { + Repository struct { + Issue struct { + ID githubv4.ID + } `graphql:"issue(number: $issueNumber)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "issueNumber": githubv4.Int(1), + }, + issueIDQueryResponse, + ), + githubv4mock.NewMutationMatcher( + struct { + CloseIssue struct { + Issue struct { + ID githubv4.ID + Number githubv4.Int + URL githubv4.String + State githubv4.String + } + } `graphql:"closeIssue(input: $input)"` + }{}, + CloseIssueInput{ + IssueID: "I_kwDOA0xdyM50BPaO", + StateReason: &completedReason, + }, + nil, + closeSuccessResponse, + ), + )) + + closeDeps := BaseDeps{ + Client: closeClient, + GQLClient: closeGQLClient, + Flags: FeatureFlags{InsidersMode: true}, + } + closeHandler := serverTool.Handler(closeDeps) + + request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "closed", + "state_reason": "completed", + }) + result, err := closeHandler(ContextWithDeps(context.Background(), closeDeps), &request) + require.NoError(t, err) + + textContent := getTextResult(t, result) + assert.NotContains(t, textContent.Text, "Ready to update issue", + "state change should skip UI form") + assert.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/1", + "state change should execute directly and return issue URL") + }) + + t.Run("UI client update without state change returns form message", func(t *testing.T) { + request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "title": "New Title", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, "Ready to update issue #1", + "update without state should show UI form") + }) } func Test_ListIssues(t *testing.T) { From a627443632264dd1c15ec47a013a41a1f5a2ec5b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Mar 2026 13:39:03 +0000 Subject: [PATCH 3/3] Use key presence check instead of OptionalParam in UI gate Check for the "state" key directly in the args map rather than using OptionalParam and ignoring its error. This ensures that a wrongly-typed state value bypasses the UI form (falling through to the normal validation path) instead of silently showing the form. Co-authored-by: mattdholloway <918573+mattdholloway@users.noreply.github.com> --- pkg/github/issues.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 980c355df..05af64cab 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1083,8 +1083,7 @@ Options are: // 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). - state, _ := OptionalParam[string](args, "state") - if state == "" { + 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