Bypass issue form for type clears

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: ea8faa5c-7f26-4e2d-bf9c-6f0b5f173e8c

Author: zwick

pkg/github/issues.go

@@ -2047,6 +2047,12 @@ var issueWriteFormParams = map[string]struct{}{
 	"_ui_submitted": {},
 }
 
+func shouldIssueWriteDeferToForm(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) bool {
+	issueType, issueTypeProvided, err := OptionalParamOK[string](args, "type")
+	explicitTypeClear := err == nil && issueTypeProvided && issueType == "" && args["method"] == "update"
+	return !explicitTypeClear && shouldDeferToForm(ctx, deps, req, args, issueWriteFormParams)
+}
+
 // issueWriteAwaitingFormResult builds the "awaiting form submission" stub
 // returned when issue_write hands off to the MCP App form. The body is shared
 // by IssueWrite and LegacyIssueWrite. The result is marked IsError=true so
@@ -2224,7 +2230,7 @@ Options are:
 
 			// Hand off to the interactive MCP App form unless this call must
 			// execute now (see shouldDeferToForm).
-			if shouldDeferToForm(ctx, deps, req, args, issueWriteFormParams) {
+			if shouldIssueWriteDeferToForm(ctx, deps, req, args) {
 				issueNumber := 0
 				if method == "update" {
 					n, numErr := RequiredInt(args, "issue_number")
@@ -2336,7 +2342,7 @@ Options are:
 				result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf, UpdateIssueOptions{
 					AssigneesProvided: assigneesProvided,
 					LabelsProvided:    labelsProvided,
-					ClearIssueType:    issueTypeProvided && issueType == "",
+					IssueTypeProvided: issueTypeProvided,
 				})
 				return result, nil, err
 			default:
@@ -2407,8 +2413,8 @@ type UpdateIssueOptions struct {
 	AssigneesProvided bool
 	// LabelsProvided sends the labels field even when the slice is empty.
 	LabelsProvided bool
-	// ClearIssueType sends an explicit null issue type.
-	ClearIssueType bool
+	// IssueTypeProvided sends the type field, including an explicit clear for an empty value.
+	IssueTypeProvided bool
 }
 
 func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue, fieldIDsToDelete []int64, state string, stateReason string, duplicateOf int, opts ...UpdateIssueOptions) (*mcp.CallToolResult, error) {
@@ -2419,7 +2425,7 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4
 	for _, opt := range opts {
 		updateOptions.AssigneesProvided = updateOptions.AssigneesProvided || opt.AssigneesProvided
 		updateOptions.LabelsProvided = updateOptions.LabelsProvided || opt.LabelsProvided
-		updateOptions.ClearIssueType = updateOptions.ClearIssueType || opt.ClearIssueType
+		updateOptions.IssueTypeProvided = updateOptions.IssueTypeProvided || opt.IssueTypeProvided
 	}
 
 	// Create the issue request with only provided fields
@@ -2493,7 +2499,7 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4
 		}
 	}
 
-	updatedIssue, resp, err := patchIssue(ctx, client, owner, repo, issueNumber, issueRequest, updateOptions.ClearIssueType)
+	updatedIssue, resp, err := patchIssue(ctx, client, owner, repo, issueNumber, issueRequest, issueType, updateOptions.IssueTypeProvided)
 	if err != nil {
 		return ghErrors.NewGitHubAPIErrorResponse(ctx,
 			"failed to update issue",
@@ -2631,8 +2637,8 @@ type updateIssueRequestWithNullableType struct {
 	Type *string `json:"type"`
 }
 
-func patchIssue(ctx context.Context, client *github.Client, owner, repo string, issueNumber int, issueRequest github.UpdateIssueRequest, clearIssueType bool) (*github.Issue, *github.Response, error) {
-	if !clearIssueType {
+func patchIssue(ctx context.Context, client *github.Client, owner, repo string, issueNumber int, issueRequest github.UpdateIssueRequest, issueType string, issueTypeProvided bool) (*github.Issue, *github.Response, error) {
+	if !issueTypeProvided || issueType != "" {
 		return client.Issues.Update(ctx, owner, repo, issueNumber, issueRequest)
 	}
 

pkg/github/issues_test.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"maps"
 	"net/http"
 	"strings"
 	"sync/atomic"
@@ -1859,7 +1860,6 @@ func Test_issueWriteHasNonFormParams(t *testing.T) {
 		{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: "type clear present", args: map[string]any{"title": "t", "type": ""}, 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},
@@ -2844,17 +2844,28 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) {
 func TestIssueWriteUpdatesIssueType(t *testing.T) {
 	tests := []struct {
 		name            string
-		issueTypeInput  string
+		args            map[string]any
 		wantRequestBody string
 	}{
 		{
-			name:            "set issue type",
-			issueTypeInput:  "Bug",
+			name: "omit issue type",
+			args: map[string]any{
+				"title": "Updated title",
+			},
+			wantRequestBody: `{"title":"Updated title"}`,
+		},
+		{
+			name: "set issue type",
+			args: map[string]any{
+				"type": "Bug",
+			},
 			wantRequestBody: `{"type":"Bug"}`,
 		},
 		{
-			name:            "clear issue type",
-			issueTypeInput:  "",
+			name: "clear issue type",
+			args: map[string]any{
+				"type": "",
+			},
 			wantRequestBody: `{"type":null}`,
 		},
 	}
@@ -2876,13 +2887,14 @@ func TestIssueWriteUpdatesIssueType(t *testing.T) {
 			}
 			serverTool := IssueWrite(translations.NullTranslationHelper)
 			handler := serverTool.Handler(deps)
-			request := createMCPRequest(map[string]any{
+			requestArgs := map[string]any{
 				"method":       "update",
 				"owner":        "owner",
 				"repo":         "repo",
 				"issue_number": float64(123),
-				"type":         tc.issueTypeInput,
-			})
+			}
+			maps.Copy(requestArgs, tc.args)
+			request := createMCPRequest(requestArgs)
 
 			result, err := handler(ContextWithDeps(context.Background(), deps), &request)
 			require.NoError(t, err)
@@ -2893,6 +2905,40 @@ func TestIssueWriteUpdatesIssueType(t *testing.T) {
 	}
 }
 
+func TestIssueWriteClearTypeBypassesMCPAppForm(t *testing.T) {
+	var gotRequestBody []byte
+	var readErr error
+	client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+		PatchReposIssuesByOwnerByRepoByIssueNumber: func(w http.ResponseWriter, r *http.Request) {
+			gotRequestBody, readErr = io.ReadAll(r.Body)
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`{"number":123,"html_url":"https://github.com/owner/repo/issues/123"}`))
+		},
+	}))
+	deps := BaseDeps{
+		Client:         client,
+		GQLClient:      githubv4.NewClient(githubv4mock.NewMockedHTTPClient()),
+		featureChecker: featureCheckerFor(MCPAppsFeatureFlag),
+	}
+	serverTool := IssueWrite(translations.NullTranslationHelper)
+	handler := serverTool.Handler(deps)
+	request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
+		"method":       "update",
+		"owner":        "owner",
+		"repo":         "repo",
+		"issue_number": float64(123),
+		"type":         "",
+	})
+
+	result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+	require.NoError(t, err)
+	require.False(t, result.IsError)
+	require.NoError(t, readErr)
+	require.JSONEq(t, `{"type":null}`, string(gotRequestBody))
+	textContent := getTextResult(t, result)
+	require.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/123")
+}
+
 func Test_UpdateIssue(t *testing.T) {
 	// Verify tool definition
 	serverTool := IssueWrite(translations.NullTranslationHelper)