From 3e93371c6659a0e808d96758fcab9f012e5a0307 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Mon, 3 Aug 2026 10:19:21 -0400 Subject: [PATCH 1/9] Support clearing issue types Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea8faa5c-7f26-4e2d-bf9c-6f0b5f173e8c --- README.md | 2 +- cmd/github-mcp-server/generate_docs.go | 20 +++++--- docs/feature-flags.md | 4 +- docs/insiders-features.md | 2 +- pkg/github/__toolsnaps__/issue_write.snap | 7 ++- .../__toolsnaps__/update_issue_type.snap | 9 ++-- pkg/github/granular_tools_test.go | 33 +++++++++++++ pkg/github/issues.go | 49 ++++++++++++++----- pkg/github/issues_granular.go | 27 ++++++---- pkg/github/issues_test.go | 42 ++++++++++++++++ pkg/github/params.go | 18 +++++++ pkg/github/params_test.go | 47 ++++++++++++++++++ ui/src/apps/issue-write/App.tsx | 28 ++++++++--- 13 files changed, 244 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 71147033a3..7700d6a4f0 100644 --- a/README.md +++ b/README.md @@ -941,7 +941,7 @@ The following sets of tools are available: - `state`: New state (string, optional) - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) - `title`: Issue title (string, optional) - - `type`: Type of this issue. Only use if issue types are enabled for this repository. Use list_issue_types tool to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string, optional) + - `type`: Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter. (string | null, optional) - **list_issue_fields** - List issue fields - **Required OAuth Scopes (any of)**: `repo`, `read:org` diff --git a/cmd/github-mcp-server/generate_docs.go b/cmd/github-mcp-server/generate_docs.go index 212851c50d..9df1a1ad01 100644 --- a/cmd/github-mcp-server/generate_docs.go +++ b/cmd/github-mcp-server/generate_docs.go @@ -276,15 +276,19 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) { var typeStr string // Get the type and description - switch prop.Type { - case "array": - if prop.Items != nil { - typeStr = prop.Items.Type + "[]" - } else { - typeStr = "array" + if len(prop.Types) > 0 { + typeStr = strings.Join(prop.Types, " | ") + } else { + switch prop.Type { + case "array": + if prop.Items != nil { + typeStr = prop.Items.Type + "[]" + } else { + typeStr = "array" + } + default: + typeStr = prop.Type } - default: - typeStr = prop.Type } // Indent any continuation lines in the description to maintain markdown formatting diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 32891af62e..674190ff92 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -71,7 +71,7 @@ runtime behavior (such as output formatting) won't appear here. - `state`: New state (string, optional) - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) - `title`: Issue title (string, optional) - - `type`: Type of this issue. Only use if issue types are enabled for this repository. Use list_issue_types tool to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string, optional) + - `type`: Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter. (string | null, optional) - **ui_get** - Get UI data - **Required OAuth Scopes (any of)**: `repo`, `read:org` @@ -200,7 +200,7 @@ runtime behavior (such as output formatting) won't appear here. - `confidence`: How confident you are in this choice. Use 'HIGH' for clear signal or explicit user request, 'MEDIUM' for reasonable inference with some ambiguity, 'LOW' for best guess with limited signal. (string, optional) - `is_suggestion`: If true, this issue type change is sent to the API as a suggestion (suggest:true) rather than an applied value. Whether the type is applied or recorded as a proposal is determined by the API. (boolean, optional) - `issue_number`: The issue number to update (number, required) - - `issue_type`: The issue type to set (string, required) + - `issue_type`: The issue type to set, or null to remove the current type (string | null, required) - `owner`: Repository owner (username or organization) (string, required) - `rationale`: One concise sentence explaining what specifically about the issue led you to choose this type. State the concrete signal (e.g. 'Reports a crash when saving' → bug, 'Asks for dark mode support' → feature). (string, optional) - `repo`: Repository name (string, required) diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 10df187a91..594547fff5 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -65,7 +65,7 @@ The list below is generated from the Go source. It covers tool **inventory and s - `state`: New state (string, optional) - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) - `title`: Issue title (string, optional) - - `type`: Type of this issue. Only use if issue types are enabled for this repository. Use list_issue_types tool to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string, optional) + - `type`: Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter. (string | null, optional) - **ui_get** - Get UI data - **Required OAuth Scopes (any of)**: `repo`, `read:org` diff --git a/pkg/github/__toolsnaps__/issue_write.snap b/pkg/github/__toolsnaps__/issue_write.snap index 55fd2dbcc2..b64124b042 100644 --- a/pkg/github/__toolsnaps__/issue_write.snap +++ b/pkg/github/__toolsnaps__/issue_write.snap @@ -120,8 +120,11 @@ "type": "string" }, "type": { - "description": "Type of this issue. Only use if issue types are enabled for this repository. Use list_issue_types tool to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter.", - "type": "string" + "description": "Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter.", + "type": [ + "string", + "null" + ] } }, "required": [ diff --git a/pkg/github/__toolsnaps__/update_issue_type.snap b/pkg/github/__toolsnaps__/update_issue_type.snap index 21a2f64bd5..990cf6f042 100644 --- a/pkg/github/__toolsnaps__/update_issue_type.snap +++ b/pkg/github/__toolsnaps__/update_issue_type.snap @@ -6,7 +6,7 @@ "readOnlyHint": false, "title": "Update Issue Type" }, - "description": "Update the type of an existing issue (e.g. 'bug', 'feature'). When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", + "description": "Set or remove the type of an existing issue. Pass null to remove the current type. When setting a value, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", "inputSchema": { "properties": { "confidence": { @@ -28,8 +28,11 @@ "type": "number" }, "issue_type": { - "description": "The issue type to set", - "type": "string" + "description": "The issue type to set, or null to remove the current type", + "type": [ + "string", + "null" + ] }, "owner": { "description": "Repository owner (username or organization)", diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 58fd904e88..7887b92009 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -14,6 +14,7 @@ import ( "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/translations" gogithub "github.com/google/go-github/v89/github" + "github.com/google/jsonschema-go/jsonschema" "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -754,6 +755,9 @@ func TestGranularUpdateIssueMilestone(t *testing.T) { } func TestGranularUpdateIssueType(t *testing.T) { + toolSchema := GranularUpdateIssueType(translations.NullTranslationHelper).Tool.InputSchema.(*jsonschema.Schema) + assert.ElementsMatch(t, []string{"string", "null"}, toolSchema.Properties["issue_type"].Types) + tests := []struct { name string requestArgs map[string]any @@ -787,6 +791,18 @@ func TestGranularUpdateIssueType(t *testing.T) { }, }, }, + { + name: "remove type", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "issue_type": nil, + }, + expectedReq: map[string]any{ + "type": nil, + }, + }, } for _, tc := range tests { @@ -807,6 +823,23 @@ func TestGranularUpdateIssueType(t *testing.T) { } } +func TestGranularUpdateIssueTypeRequiresType(t *testing.T) { + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} + serverTool := GranularUpdateIssueType(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, "missing required parameter: issue_type") +} + func TestGranularUpdateIssueTypeSuggest(t *testing.T) { tests := []struct { name string diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 083c5465e0..7fd130215f 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -2048,8 +2048,7 @@ var issueWriteFormParams = map[string]struct{}{ } // 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 +// returned when issue_write hands off to the MCP App form. The result is marked IsError=true so // agents that bail on error don't claim success or chain dependent tool calls // while the user is still interacting with the form; the host renders the UI // regardless because rendering is keyed off the tool's _meta.ui resourceUri. @@ -2077,12 +2076,8 @@ func issueWriteAwaitingFormResult(method, owner, repo string, issueNumber int) * return utils.NewToolResultAwaitingFormSubmission(msg) } -// 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 -// active at a time via mutually exclusive feature-flag annotations. When the -// flag is removed, delete LegacyIssueWrite outright and drop the feature-flag -// fields on IssueWrite. +// IssueWrite creates the consolidated issue_write tool. It is disabled when +// the granular issue tools are enabled. func IssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( ToolsetMetadataIssues, @@ -2150,8 +2145,8 @@ Options are: Description: "Milestone number", }, "type": { - Type: "string", - Description: "Type of this issue. Only use if issue types are enabled for this repository. Use list_issue_types tool to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter.", + Types: []string{"string", "null"}, + Description: "Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter.", }, "state": { Type: "string", @@ -2275,10 +2270,14 @@ Options are: } // Get optional type - issueType, err := OptionalParam[string](args, "type") + issueTypeParam, issueTypeProvided, err := OptionalNullableStringParam(args, "type") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + issueType := "" + if issueTypeParam != nil { + issueType = *issueTypeParam + } // Handle state, state_reason and duplicateOf parameters state, err := OptionalParam[string](args, "state") @@ -2336,6 +2335,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 && issueTypeParam == nil, }) return result, nil, err default: @@ -2406,6 +2406,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 } 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) { @@ -2416,6 +2418,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 } // Create the issue request with only provided fields @@ -2489,7 +2492,7 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 } } - updatedIssue, resp, err := client.Issues.Update(ctx, owner, repo, issueNumber, issueRequest) + updatedIssue, resp, err := patchIssue(ctx, client, owner, repo, issueNumber, issueRequest, updateOptions.ClearIssueType) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update issue", @@ -2622,6 +2625,28 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 return utils.NewToolResultText(string(r)), nil } +type updateIssueRequestWithNullableType struct { + github.UpdateIssueRequest + 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 { + return client.Issues.Update(ctx, owner, repo, issueNumber, issueRequest) + } + + apiURL := fmt.Sprintf("repos/%s/%s/issues/%d", owner, repo, issueNumber) + body := &updateIssueRequestWithNullableType{UpdateIssueRequest: issueRequest} + req, err := client.NewRequest(ctx, http.MethodPatch, apiURL, body) + if err != nil { + return nil, nil, err + } + + issue := &github.Issue{} + resp, err := client.Do(req, issue) + return issue, resp, err +} + // ListIssues creates a tool to list issues in a GitHub repository. func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { schema := &jsonschema.Schema{ diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 314ead3eb4..da503b00a6 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -679,13 +679,13 @@ type issueTypeUpdateRequest struct { Type issueTypeWithIntent `json:"type"` } -// GranularUpdateIssueType creates a tool to update an issue's type. +// GranularUpdateIssueType creates a tool to set or clear an issue's type. func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "update_issue_type", - Description: t("TOOL_UPDATE_ISSUE_TYPE_DESCRIPTION", "Update the type of an existing issue (e.g. 'bug', 'feature'). When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), + Description: t("TOOL_UPDATE_ISSUE_TYPE_DESCRIPTION", "Set or remove the type of an existing issue. Pass null to remove the current type. When setting a value, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_UPDATE_ISSUE_TYPE_USER_TITLE", "Update Issue Type"), ReadOnlyHint: false, @@ -709,8 +709,8 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser Minimum: jsonschema.Ptr(1.0), }, "issue_type": { - Type: "string", - Description: "The issue type to set", + Types: []string{"string", "null"}, + Description: "The issue type to set, or null to remove the current type", }, "rationale": { Type: "string", @@ -746,10 +746,13 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - issueType, err := RequiredParam[string](args, "issue_type") + issueType, issueTypeProvided, err := OptionalNullableStringParam(args, "issue_type") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + if !issueTypeProvided { + return utils.NewToolResultError("missing required parameter: issue_type"), nil, nil + } rationale, err := OptionalParam[string](args, "rationale") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil @@ -770,6 +773,9 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + if issueType == nil && (rationale != "" || confidence != "" || isSuggestion) { + return utils.NewToolResultError("rationale, confidence, and is_suggestion cannot be used when removing an issue type"), nil, nil + } client, err := deps.GetClient(ctx) if err != nil { @@ -777,17 +783,20 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser } var body any - if rationale != "" || isSuggestion || confidence != "" { + switch { + case issueType == nil: + body = map[string]any{"type": nil} + case rationale != "" || isSuggestion || confidence != "": body = &issueTypeUpdateRequest{ Type: issueTypeWithIntent{ - Value: issueType, + Value: *issueType, Rationale: rationale, Confidence: confidence, Suggest: isSuggestion, }, } - } else { - body = &github.UpdateIssueRequest{Type: &issueType} + default: + body = &github.UpdateIssueRequest{Type: issueType} } apiURL := fmt.Sprintf("repos/%s/%s/issues/%d", owner, repo, issueNumber) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 3e0974862e..e28b55d843 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1434,6 +1434,7 @@ func Test_CreateIssue(t *testing.T) { assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "labels") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "milestone") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "type") + assert.ElementsMatch(t, []string{"string", "null"}, tool.InputSchema.(*jsonschema.Schema).Properties["type"].Types) assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "issue_fields") assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"method", "owner", "repo"}) @@ -1828,6 +1829,7 @@ 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": nil}, 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}, @@ -2942,6 +2944,46 @@ func Test_UpdateIssue(t *testing.T) { expectError: false, expectedIssue: mockUpdatedIssue, }, + { + name: "partial update sets issue type", + mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "type": "Bug", + }).andThen( + mockResponse(t, http.StatusOK, mockUpdatedIssue), + ), + }), + mockedGQLClient: githubv4mock.NewMockedHTTPClient(), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + "type": "Bug", + }, + expectError: false, + expectedIssue: mockUpdatedIssue, + }, + { + name: "partial update clears issue type", + mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "type": nil, + }).andThen( + mockResponse(t, http.StatusOK, mockUpdatedIssue), + ), + }), + mockedGQLClient: githubv4mock.NewMockedHTTPClient(), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + "type": nil, + }, + expectError: false, + expectedIssue: mockUpdatedIssue, + }, { name: "partial update clears labels and assignees", mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ diff --git a/pkg/github/params.go b/pkg/github/params.go index 9be51b94b9..b4fbdb9968 100644 --- a/pkg/github/params.go +++ b/pkg/github/params.go @@ -193,6 +193,24 @@ func OptionalParam[T any](args map[string]any, p string) (T, error) { return args[p].(T), nil } +// OptionalNullableStringParam returns a nullable string while preserving +// whether the parameter was omitted. +func OptionalNullableStringParam(args map[string]any, p string) (*string, bool, error) { + value, ok := args[p] + if !ok { + return nil, false, nil + } + if value == nil { + return nil, true, nil + } + + stringValue, ok := value.(string) + if !ok { + return nil, true, fmt.Errorf("parameter %s is not of type string or null, is %T", p, value) + } + return &stringValue, true, nil +} + // OptionalIntParam is a helper function that can be used to fetch a requested parameter from the request. // It does the following checks: // 1. Checks if the parameter is present in the request, if not, it returns its zero-value diff --git a/pkg/github/params_test.go b/pkg/github/params_test.go index cbac37fee5..2bde4afaf8 100644 --- a/pkg/github/params_test.go +++ b/pkg/github/params_test.go @@ -270,6 +270,53 @@ func Test_RequiredInt(t *testing.T) { }) } } + +func Test_OptionalNullableStringParam(t *testing.T) { + tests := []struct { + name string + params map[string]any + expected *string + expectedPresent bool + expectError bool + }{ + { + name: "omitted", + params: map[string]any{}, + expectedPresent: false, + }, + { + name: "explicit null", + params: map[string]any{"name": nil}, + expectedPresent: true, + }, + { + name: "string value", + params: map[string]any{"name": "Bug"}, + expected: github.Ptr("Bug"), + expectedPresent: true, + }, + { + name: "wrong type", + params: map[string]any{"name": 123}, + expectedPresent: true, + expectError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + value, present, err := OptionalNullableStringParam(tc.params, "name") + assert.Equal(t, tc.expectedPresent, present) + if tc.expectError { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, tc.expected, value) + }) + } +} + func Test_OptionalIntParam(t *testing.T) { tests := []struct { name string diff --git a/ui/src/apps/issue-write/App.tsx b/ui/src/apps/issue-write/App.tsx index 95a549f28b..79f97b574b 100644 --- a/ui/src/apps/issue-write/App.tsx +++ b/ui/src/apps/issue-write/App.tsx @@ -418,6 +418,7 @@ function CreateIssueApp() { // Issue types state const [availableIssueTypes, setAvailableIssueTypes] = useState([]); const [selectedIssueType, setSelectedIssueType] = useState(null); + const [issueTypeChanged, setIssueTypeChanged] = useState(false); const [issueTypesLoading, setIssueTypesLoading] = useState(false); // State transition state @@ -455,6 +456,7 @@ function CreateIssueApp() { const method = (toolInput?.method as string) || "create"; const issueNumber = toolInput?.issue_number as number | undefined; const isUpdateMode = method === "update" && issueNumber !== undefined; + const hasExplicitIssueType = Object.prototype.hasOwnProperty.call(toolInput ?? {}, "type"); // Initialize from toolInput or selected repo const owner = selectedRepo?.owner || (toolInput?.owner as string) || ""; @@ -712,7 +714,7 @@ function CreateIssueApp() { labels: false, assignees: false, milestone: false, - type: false, + type: hasExplicitIssueType && toolInput?.type === null, issueFields: false, }; setExistingIssueData(null); @@ -722,6 +724,7 @@ function CreateIssueApp() { setSelectedAssignees([]); setSelectedMilestone(null); setSelectedIssueType(null); + setIssueTypeChanged(hasExplicitIssueType && toolInput?.type === null); setCurrentState("open"); setStateReason("completed"); setDuplicateOf(""); @@ -750,7 +753,7 @@ function CreateIssueApp() { } else { setSelectedRepo(null); } - }, [toolInput]); + }, [toolInput, hasExplicitIssueType]); // Load existing issue data when in update mode useEffect(() => { @@ -800,7 +803,7 @@ function CreateIssueApp() { // Pre-fill issue type immediately from issue data const issueTypeName = issueData.type?.name || (typeof issueData.type === 'string' ? issueData.type : null); - if (issueTypeName && !prefillApplied.current.type) { + if (issueTypeName && !prefillApplied.current.type && !hasExplicitIssueType) { setSelectedIssueType({ id: issueTypeName, text: issueTypeName }); prefillApplied.current.type = true; } @@ -829,7 +832,7 @@ function CreateIssueApp() { }; loadExistingIssue(); - }, [isUpdateMode, owner, repo, issueNumber, app, callTool, existingIssueData]); + }, [isUpdateMode, owner, repo, issueNumber, app, callTool, existingIssueData, hasExplicitIssueType]); // Apply existing labels when available labels load useEffect(() => { @@ -1032,6 +1035,12 @@ function CreateIssueApp() { } if (selectedIssueType) { params.type = selectedIssueType.text; + } else if (issueTypeChanged) { + if (isUpdateMode) { + params.type = null; + } else { + delete params.type; + } } if (requestedState) { @@ -1115,6 +1124,7 @@ function CreateIssueApp() { selectedAssignees, selectedMilestone, selectedIssueType, + issueTypeChanged, isUpdateMode, issueNumber, stateReason, @@ -1533,7 +1543,10 @@ function CreateIssueApp() { <> {selectedIssueType && ( setSelectedIssueType(null)} + onSelect={() => { + setSelectedIssueType(null); + setIssueTypeChanged(true); + }} > Clear selection @@ -1542,7 +1555,10 @@ function CreateIssueApp() { setSelectedIssueType(type)} + onSelect={() => { + setSelectedIssueType(type); + setIssueTypeChanged(true); + }} > {type.text} From c8eea3e4d8bfee9b8f3edbfe7298b440c51e6ca8 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Mon, 3 Aug 2026 10:44:47 -0400 Subject: [PATCH 2/9] Narrow issue type removal to MCP tools Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea8faa5c-7f26-4e2d-bf9c-6f0b5f173e8c --- README.md | 2 +- cmd/github-mcp-server/generate_docs.go | 20 ++++++++---------- docs/feature-flags.md | 4 ++-- docs/insiders-features.md | 2 +- pkg/github/issues.go | 11 +++++++--- ui/src/apps/issue-write/App.tsx | 28 ++++++-------------------- 6 files changed, 26 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index 7700d6a4f0..cc0e091a0b 100644 --- a/README.md +++ b/README.md @@ -941,7 +941,7 @@ The following sets of tools are available: - `state`: New state (string, optional) - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) - `title`: Issue title (string, optional) - - `type`: Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter. (string | null, optional) + - `type`: Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter. (, optional) - **list_issue_fields** - List issue fields - **Required OAuth Scopes (any of)**: `repo`, `read:org` diff --git a/cmd/github-mcp-server/generate_docs.go b/cmd/github-mcp-server/generate_docs.go index 9df1a1ad01..212851c50d 100644 --- a/cmd/github-mcp-server/generate_docs.go +++ b/cmd/github-mcp-server/generate_docs.go @@ -276,19 +276,15 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) { var typeStr string // Get the type and description - if len(prop.Types) > 0 { - typeStr = strings.Join(prop.Types, " | ") - } else { - switch prop.Type { - case "array": - if prop.Items != nil { - typeStr = prop.Items.Type + "[]" - } else { - typeStr = "array" - } - default: - typeStr = prop.Type + switch prop.Type { + case "array": + if prop.Items != nil { + typeStr = prop.Items.Type + "[]" + } else { + typeStr = "array" } + default: + typeStr = prop.Type } // Indent any continuation lines in the description to maintain markdown formatting diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 674190ff92..d69f6db62e 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -71,7 +71,7 @@ runtime behavior (such as output formatting) won't appear here. - `state`: New state (string, optional) - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) - `title`: Issue title (string, optional) - - `type`: Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter. (string | null, optional) + - `type`: Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter. (, optional) - **ui_get** - Get UI data - **Required OAuth Scopes (any of)**: `repo`, `read:org` @@ -200,7 +200,7 @@ runtime behavior (such as output formatting) won't appear here. - `confidence`: How confident you are in this choice. Use 'HIGH' for clear signal or explicit user request, 'MEDIUM' for reasonable inference with some ambiguity, 'LOW' for best guess with limited signal. (string, optional) - `is_suggestion`: If true, this issue type change is sent to the API as a suggestion (suggest:true) rather than an applied value. Whether the type is applied or recorded as a proposal is determined by the API. (boolean, optional) - `issue_number`: The issue number to update (number, required) - - `issue_type`: The issue type to set, or null to remove the current type (string | null, required) + - `issue_type`: The issue type to set, or null to remove the current type (, required) - `owner`: Repository owner (username or organization) (string, required) - `rationale`: One concise sentence explaining what specifically about the issue led you to choose this type. State the concrete signal (e.g. 'Reports a crash when saving' → bug, 'Asks for dark mode support' → feature). (string, optional) - `repo`: Repository name (string, required) diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 594547fff5..487a3cfe50 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -65,7 +65,7 @@ The list below is generated from the Go source. It covers tool **inventory and s - `state`: New state (string, optional) - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) - `title`: Issue title (string, optional) - - `type`: Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter. (string | null, optional) + - `type`: Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter. (, optional) - **ui_get** - Get UI data - **Required OAuth Scopes (any of)**: `repo`, `read:org` diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 7fd130215f..52bc85d5fb 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -2048,7 +2048,8 @@ var issueWriteFormParams = map[string]struct{}{ } // issueWriteAwaitingFormResult builds the "awaiting form submission" stub -// returned when issue_write hands off to the MCP App form. The result is marked IsError=true so +// 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 // agents that bail on error don't claim success or chain dependent tool calls // while the user is still interacting with the form; the host renders the UI // regardless because rendering is keyed off the tool's _meta.ui resourceUri. @@ -2076,8 +2077,12 @@ func issueWriteAwaitingFormResult(method, owner, repo string, issueNumber int) * return utils.NewToolResultAwaitingFormSubmission(msg) } -// IssueWrite creates the consolidated issue_write tool. It is disabled when -// the granular issue tools are enabled. +// 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 +// active at a time via mutually exclusive feature-flag annotations. When the +// flag is removed, delete LegacyIssueWrite outright and drop the feature-flag +// fields on IssueWrite. func IssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( ToolsetMetadataIssues, diff --git a/ui/src/apps/issue-write/App.tsx b/ui/src/apps/issue-write/App.tsx index 79f97b574b..95a549f28b 100644 --- a/ui/src/apps/issue-write/App.tsx +++ b/ui/src/apps/issue-write/App.tsx @@ -418,7 +418,6 @@ function CreateIssueApp() { // Issue types state const [availableIssueTypes, setAvailableIssueTypes] = useState([]); const [selectedIssueType, setSelectedIssueType] = useState(null); - const [issueTypeChanged, setIssueTypeChanged] = useState(false); const [issueTypesLoading, setIssueTypesLoading] = useState(false); // State transition state @@ -456,7 +455,6 @@ function CreateIssueApp() { const method = (toolInput?.method as string) || "create"; const issueNumber = toolInput?.issue_number as number | undefined; const isUpdateMode = method === "update" && issueNumber !== undefined; - const hasExplicitIssueType = Object.prototype.hasOwnProperty.call(toolInput ?? {}, "type"); // Initialize from toolInput or selected repo const owner = selectedRepo?.owner || (toolInput?.owner as string) || ""; @@ -714,7 +712,7 @@ function CreateIssueApp() { labels: false, assignees: false, milestone: false, - type: hasExplicitIssueType && toolInput?.type === null, + type: false, issueFields: false, }; setExistingIssueData(null); @@ -724,7 +722,6 @@ function CreateIssueApp() { setSelectedAssignees([]); setSelectedMilestone(null); setSelectedIssueType(null); - setIssueTypeChanged(hasExplicitIssueType && toolInput?.type === null); setCurrentState("open"); setStateReason("completed"); setDuplicateOf(""); @@ -753,7 +750,7 @@ function CreateIssueApp() { } else { setSelectedRepo(null); } - }, [toolInput, hasExplicitIssueType]); + }, [toolInput]); // Load existing issue data when in update mode useEffect(() => { @@ -803,7 +800,7 @@ function CreateIssueApp() { // Pre-fill issue type immediately from issue data const issueTypeName = issueData.type?.name || (typeof issueData.type === 'string' ? issueData.type : null); - if (issueTypeName && !prefillApplied.current.type && !hasExplicitIssueType) { + if (issueTypeName && !prefillApplied.current.type) { setSelectedIssueType({ id: issueTypeName, text: issueTypeName }); prefillApplied.current.type = true; } @@ -832,7 +829,7 @@ function CreateIssueApp() { }; loadExistingIssue(); - }, [isUpdateMode, owner, repo, issueNumber, app, callTool, existingIssueData, hasExplicitIssueType]); + }, [isUpdateMode, owner, repo, issueNumber, app, callTool, existingIssueData]); // Apply existing labels when available labels load useEffect(() => { @@ -1035,12 +1032,6 @@ function CreateIssueApp() { } if (selectedIssueType) { params.type = selectedIssueType.text; - } else if (issueTypeChanged) { - if (isUpdateMode) { - params.type = null; - } else { - delete params.type; - } } if (requestedState) { @@ -1124,7 +1115,6 @@ function CreateIssueApp() { selectedAssignees, selectedMilestone, selectedIssueType, - issueTypeChanged, isUpdateMode, issueNumber, stateReason, @@ -1543,10 +1533,7 @@ function CreateIssueApp() { <> {selectedIssueType && ( { - setSelectedIssueType(null); - setIssueTypeChanged(true); - }} + onSelect={() => setSelectedIssueType(null)} > Clear selection @@ -1555,10 +1542,7 @@ function CreateIssueApp() { { - setSelectedIssueType(type); - setIssueTypeChanged(true); - }} + onSelect={() => setSelectedIssueType(type)} > {type.text} From 5d4090503488fe29207d313d93b81ca91fadd555 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Mon, 3 Aug 2026 11:47:08 -0400 Subject: [PATCH 3/9] Use empty strings to clear issue types Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea8faa5c-7f26-4e2d-bf9c-6f0b5f173e8c --- README.md | 2 +- docs/feature-flags.md | 4 +- docs/insiders-features.md | 2 +- pkg/github/__toolsnaps__/issue_write.snap | 7 +-- .../__toolsnaps__/update_issue_type.snap | 9 ++-- pkg/github/granular_tools_test.go | 6 +-- pkg/github/issues.go | 12 ++--- pkg/github/issues_granular.go | 16 +++---- pkg/github/issues_test.go | 38 +++++++++++++-- pkg/github/params.go | 18 ------- pkg/github/params_test.go | 47 ------------------- 11 files changed, 58 insertions(+), 103 deletions(-) diff --git a/README.md b/README.md index cc0e091a0b..7c5b954c93 100644 --- a/README.md +++ b/README.md @@ -941,7 +941,7 @@ The following sets of tools are available: - `state`: New state (string, optional) - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) - `title`: Issue title (string, optional) - - `type`: Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter. (, optional) + - `type`: Type of this issue. For updates, use an empty string to remove the current type. Only use if issue types are enabled for this repository. Use list_issue_types to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string, optional) - **list_issue_fields** - List issue fields - **Required OAuth Scopes (any of)**: `repo`, `read:org` diff --git a/docs/feature-flags.md b/docs/feature-flags.md index d69f6db62e..8b55a358ce 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -71,7 +71,7 @@ runtime behavior (such as output formatting) won't appear here. - `state`: New state (string, optional) - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) - `title`: Issue title (string, optional) - - `type`: Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter. (, optional) + - `type`: Type of this issue. For updates, use an empty string to remove the current type. Only use if issue types are enabled for this repository. Use list_issue_types to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string, optional) - **ui_get** - Get UI data - **Required OAuth Scopes (any of)**: `repo`, `read:org` @@ -200,7 +200,7 @@ runtime behavior (such as output formatting) won't appear here. - `confidence`: How confident you are in this choice. Use 'HIGH' for clear signal or explicit user request, 'MEDIUM' for reasonable inference with some ambiguity, 'LOW' for best guess with limited signal. (string, optional) - `is_suggestion`: If true, this issue type change is sent to the API as a suggestion (suggest:true) rather than an applied value. Whether the type is applied or recorded as a proposal is determined by the API. (boolean, optional) - `issue_number`: The issue number to update (number, required) - - `issue_type`: The issue type to set, or null to remove the current type (, required) + - `issue_type`: The issue type to set, or an empty string to remove the current type (string, required) - `owner`: Repository owner (username or organization) (string, required) - `rationale`: One concise sentence explaining what specifically about the issue led you to choose this type. State the concrete signal (e.g. 'Reports a crash when saving' → bug, 'Asks for dark mode support' → feature). (string, optional) - `repo`: Repository name (string, required) diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 487a3cfe50..11270dc662 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -65,7 +65,7 @@ The list below is generated from the Go source. It covers tool **inventory and s - `state`: New state (string, optional) - `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional) - `title`: Issue title (string, optional) - - `type`: Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter. (, optional) + - `type`: Type of this issue. For updates, use an empty string to remove the current type. Only use if issue types are enabled for this repository. Use list_issue_types to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter. (string, optional) - **ui_get** - Get UI data - **Required OAuth Scopes (any of)**: `repo`, `read:org` diff --git a/pkg/github/__toolsnaps__/issue_write.snap b/pkg/github/__toolsnaps__/issue_write.snap index b64124b042..4f790eb9f1 100644 --- a/pkg/github/__toolsnaps__/issue_write.snap +++ b/pkg/github/__toolsnaps__/issue_write.snap @@ -120,11 +120,8 @@ "type": "string" }, "type": { - "description": "Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter.", - "type": [ - "string", - "null" - ] + "description": "Type of this issue. For updates, use an empty string to remove the current type. Only use if issue types are enabled for this repository. Use list_issue_types to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter.", + "type": "string" } }, "required": [ diff --git a/pkg/github/__toolsnaps__/update_issue_type.snap b/pkg/github/__toolsnaps__/update_issue_type.snap index 990cf6f042..98fbc16d8c 100644 --- a/pkg/github/__toolsnaps__/update_issue_type.snap +++ b/pkg/github/__toolsnaps__/update_issue_type.snap @@ -6,7 +6,7 @@ "readOnlyHint": false, "title": "Update Issue Type" }, - "description": "Set or remove the type of an existing issue. Pass null to remove the current type. When setting a value, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", + "description": "Set or remove the type of an existing issue. Use an empty issue_type to remove the current type. When setting a value, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", "inputSchema": { "properties": { "confidence": { @@ -28,11 +28,8 @@ "type": "number" }, "issue_type": { - "description": "The issue type to set, or null to remove the current type", - "type": [ - "string", - "null" - ] + "description": "The issue type to set, or an empty string to remove the current type", + "type": "string" }, "owner": { "description": "Repository owner (username or organization)", diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 7887b92009..b5410d26fb 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -756,7 +756,7 @@ func TestGranularUpdateIssueMilestone(t *testing.T) { func TestGranularUpdateIssueType(t *testing.T) { toolSchema := GranularUpdateIssueType(translations.NullTranslationHelper).Tool.InputSchema.(*jsonschema.Schema) - assert.ElementsMatch(t, []string{"string", "null"}, toolSchema.Properties["issue_type"].Types) + assert.Equal(t, "string", toolSchema.Properties["issue_type"].Type) tests := []struct { name string @@ -792,12 +792,12 @@ func TestGranularUpdateIssueType(t *testing.T) { }, }, { - name: "remove type", + name: "remove type with empty string", requestArgs: map[string]any{ "owner": "owner", "repo": "repo", "issue_number": float64(1), - "issue_type": nil, + "issue_type": "", }, expectedReq: map[string]any{ "type": nil, diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 52bc85d5fb..d273599a95 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -2150,8 +2150,8 @@ Options are: Description: "Milestone number", }, "type": { - Types: []string{"string", "null"}, - Description: "Type of this issue. Use list_issue_types to get valid values. For updates, pass null to remove the current type. If the repository doesn't support issue types, omit this parameter.", + Type: "string", + Description: "Type of this issue. For updates, use an empty string to remove the current type. Only use if issue types are enabled for this repository. Use list_issue_types to get valid type values for this repository or its owner organization. If the repository doesn't support issue types, omit this parameter.", }, "state": { Type: "string", @@ -2275,14 +2275,10 @@ Options are: } // Get optional type - issueTypeParam, issueTypeProvided, err := OptionalNullableStringParam(args, "type") + issueType, issueTypeProvided, err := OptionalParamOK[string](args, "type") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - issueType := "" - if issueTypeParam != nil { - issueType = *issueTypeParam - } // Handle state, state_reason and duplicateOf parameters state, err := OptionalParam[string](args, "state") @@ -2340,7 +2336,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 && issueTypeParam == nil, + ClearIssueType: issueTypeProvided && issueType == "", }) return result, nil, err default: diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index da503b00a6..b0a9467cf0 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -685,7 +685,7 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser ToolsetMetadataIssues, mcp.Tool{ Name: "update_issue_type", - Description: t("TOOL_UPDATE_ISSUE_TYPE_DESCRIPTION", "Set or remove the type of an existing issue. Pass null to remove the current type. When setting a value, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), + Description: t("TOOL_UPDATE_ISSUE_TYPE_DESCRIPTION", "Set or remove the type of an existing issue. Use an empty issue_type to remove the current type. When setting a value, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_UPDATE_ISSUE_TYPE_USER_TITLE", "Update Issue Type"), ReadOnlyHint: false, @@ -709,8 +709,8 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser Minimum: jsonschema.Ptr(1.0), }, "issue_type": { - Types: []string{"string", "null"}, - Description: "The issue type to set, or null to remove the current type", + Type: "string", + Description: "The issue type to set, or an empty string to remove the current type", }, "rationale": { Type: "string", @@ -746,7 +746,7 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - issueType, issueTypeProvided, err := OptionalNullableStringParam(args, "issue_type") + issueType, issueTypeProvided, err := OptionalParamOK[string](args, "issue_type") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } @@ -773,7 +773,7 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - if issueType == nil && (rationale != "" || confidence != "" || isSuggestion) { + if issueType == "" && (rationale != "" || confidence != "" || isSuggestion) { return utils.NewToolResultError("rationale, confidence, and is_suggestion cannot be used when removing an issue type"), nil, nil } @@ -784,19 +784,19 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser var body any switch { - case issueType == nil: + case issueType == "": body = map[string]any{"type": nil} case rationale != "" || isSuggestion || confidence != "": body = &issueTypeUpdateRequest{ Type: issueTypeWithIntent{ - Value: *issueType, + Value: issueType, Rationale: rationale, Confidence: confidence, Suggest: isSuggestion, }, } default: - body = &github.UpdateIssueRequest{Type: issueType} + body = &github.UpdateIssueRequest{Type: &issueType} } apiURL := fmt.Sprintf("repos/%s/%s/issues/%d", owner, repo, issueNumber) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index e28b55d843..8aea6d0f6e 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1434,7 +1434,7 @@ func Test_CreateIssue(t *testing.T) { assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "labels") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "milestone") assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "type") - assert.ElementsMatch(t, []string{"string", "null"}, tool.InputSchema.(*jsonschema.Schema).Properties["type"].Types) + assert.Equal(t, "string", tool.InputSchema.(*jsonschema.Schema).Properties["type"].Type) assert.Contains(t, tool.InputSchema.(*jsonschema.Schema).Properties, "issue_fields") assert.ElementsMatch(t, tool.InputSchema.(*jsonschema.Schema).Required, []string{"method", "owner", "repo"}) @@ -1513,6 +1513,36 @@ func Test_CreateIssue(t *testing.T) { State: github.Ptr("open"), }, }, + { + name: "empty issue type is omitted when creating", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesByOwnerByRepo: expectRequestBody(t, map[string]any{ + "title": "Issue without a type", + "body": "", + }).andThen( + mockResponse(t, http.StatusCreated, &github.Issue{ + Number: github.Ptr(125), + Title: github.Ptr("Issue without a type"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/125"), + State: github.Ptr("open"), + }), + ), + }), + requestArgs: map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Issue without a type", + "type": "", + }, + expectError: false, + expectedIssue: &github.Issue{ + Number: github.Ptr(125), + Title: github.Ptr("Issue without a type"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/125"), + State: github.Ptr("open"), + }, + }, { name: "successful issue creation with issue fields reconciled by names", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ @@ -1829,7 +1859,7 @@ 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": nil}, 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}, @@ -2965,7 +2995,7 @@ func Test_UpdateIssue(t *testing.T) { expectedIssue: mockUpdatedIssue, }, { - name: "partial update clears issue type", + name: "partial update clears issue type from empty string", mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ "type": nil, @@ -2979,7 +3009,7 @@ func Test_UpdateIssue(t *testing.T) { "owner": "owner", "repo": "repo", "issue_number": float64(123), - "type": nil, + "type": "", }, expectError: false, expectedIssue: mockUpdatedIssue, diff --git a/pkg/github/params.go b/pkg/github/params.go index b4fbdb9968..9be51b94b9 100644 --- a/pkg/github/params.go +++ b/pkg/github/params.go @@ -193,24 +193,6 @@ func OptionalParam[T any](args map[string]any, p string) (T, error) { return args[p].(T), nil } -// OptionalNullableStringParam returns a nullable string while preserving -// whether the parameter was omitted. -func OptionalNullableStringParam(args map[string]any, p string) (*string, bool, error) { - value, ok := args[p] - if !ok { - return nil, false, nil - } - if value == nil { - return nil, true, nil - } - - stringValue, ok := value.(string) - if !ok { - return nil, true, fmt.Errorf("parameter %s is not of type string or null, is %T", p, value) - } - return &stringValue, true, nil -} - // OptionalIntParam is a helper function that can be used to fetch a requested parameter from the request. // It does the following checks: // 1. Checks if the parameter is present in the request, if not, it returns its zero-value diff --git a/pkg/github/params_test.go b/pkg/github/params_test.go index 2bde4afaf8..cbac37fee5 100644 --- a/pkg/github/params_test.go +++ b/pkg/github/params_test.go @@ -270,53 +270,6 @@ func Test_RequiredInt(t *testing.T) { }) } } - -func Test_OptionalNullableStringParam(t *testing.T) { - tests := []struct { - name string - params map[string]any - expected *string - expectedPresent bool - expectError bool - }{ - { - name: "omitted", - params: map[string]any{}, - expectedPresent: false, - }, - { - name: "explicit null", - params: map[string]any{"name": nil}, - expectedPresent: true, - }, - { - name: "string value", - params: map[string]any{"name": "Bug"}, - expected: github.Ptr("Bug"), - expectedPresent: true, - }, - { - name: "wrong type", - params: map[string]any{"name": 123}, - expectedPresent: true, - expectError: true, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - value, present, err := OptionalNullableStringParam(tc.params, "name") - assert.Equal(t, tc.expectedPresent, present) - if tc.expectError { - assert.Error(t, err) - return - } - assert.NoError(t, err) - assert.Equal(t, tc.expected, value) - }) - } -} - func Test_OptionalIntParam(t *testing.T) { tests := []struct { name string From 3a46e39829b05af304a95d1af0620d2102b51cc5 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Mon, 3 Aug 2026 11:56:51 -0400 Subject: [PATCH 4/9] Clarify issue type request tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea8faa5c-7f26-4e2d-bf9c-6f0b5f173e8c --- pkg/github/issues_test.go | 92 ++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 40 deletions(-) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 8aea6d0f6e..0d190f4f4b 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2841,6 +2841,58 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) { }) } +func TestIssueWriteUpdatesIssueType(t *testing.T) { + tests := []struct { + name string + issueTypeInput string + wantRequestBody string + }{ + { + name: "set issue type", + issueTypeInput: "Bug", + wantRequestBody: `{"type":"Bug"}`, + }, + { + name: "clear issue type", + issueTypeInput: "", + wantRequestBody: `{"type":null}`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(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()), + } + serverTool := IssueWrite(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + "type": tc.issueTypeInput, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + require.NoError(t, readErr) + require.JSONEq(t, tc.wantRequestBody, string(gotRequestBody)) + }) + } +} + func Test_UpdateIssue(t *testing.T) { // Verify tool definition serverTool := IssueWrite(translations.NullTranslationHelper) @@ -2974,46 +3026,6 @@ func Test_UpdateIssue(t *testing.T) { expectError: false, expectedIssue: mockUpdatedIssue, }, - { - name: "partial update sets issue type", - mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ - "type": "Bug", - }).andThen( - mockResponse(t, http.StatusOK, mockUpdatedIssue), - ), - }), - mockedGQLClient: githubv4mock.NewMockedHTTPClient(), - requestArgs: map[string]any{ - "method": "update", - "owner": "owner", - "repo": "repo", - "issue_number": float64(123), - "type": "Bug", - }, - expectError: false, - expectedIssue: mockUpdatedIssue, - }, - { - name: "partial update clears issue type from empty string", - mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ - "type": nil, - }).andThen( - mockResponse(t, http.StatusOK, mockUpdatedIssue), - ), - }), - mockedGQLClient: githubv4mock.NewMockedHTTPClient(), - requestArgs: map[string]any{ - "method": "update", - "owner": "owner", - "repo": "repo", - "issue_number": float64(123), - "type": "", - }, - expectError: false, - expectedIssue: mockUpdatedIssue, - }, { name: "partial update clears labels and assignees", mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ From c089d32d7ad8b1522eed0fd7d74bec39d2534851 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Mon, 3 Aug 2026 11:58:06 -0400 Subject: [PATCH 5/9] Keep issue type clearing policy-neutral Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea8faa5c-7f26-4e2d-bf9c-6f0b5f173e8c --- pkg/github/issues_granular.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index b0a9467cf0..64161acacd 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -773,10 +773,6 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - if issueType == "" && (rationale != "" || confidence != "" || isSuggestion) { - return utils.NewToolResultError("rationale, confidence, and is_suggestion cannot be used when removing an issue type"), nil, nil - } - client, err := deps.GetClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil From 45cac895161812d2d94040a324558a71b216f77f Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Mon, 3 Aug 2026 12:10:54 -0400 Subject: [PATCH 6/9] Bypass issue form for type clears Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea8faa5c-7f26-4e2d-bf9c-6f0b5f173e8c --- pkg/github/issues.go | 22 +++++++++----- pkg/github/issues_test.go | 64 +++++++++++++++++++++++++++++++++------ 2 files changed, 69 insertions(+), 17 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index d273599a95..5347cd862d 100644 --- a/pkg/github/issues.go +++ b/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) } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 0d190f4f4b..522242fa06 100644 --- a/pkg/github/issues_test.go +++ b/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) From f5f644cdc5cea2bedbc93c6957baa73b6d23398d Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Mon, 3 Aug 2026 12:20:15 -0400 Subject: [PATCH 7/9] Document mixed issue type clear routing Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea8faa5c-7f26-4e2d-bf9c-6f0b5f173e8c --- pkg/github/issues.go | 2 + pkg/github/issues_test.go | 86 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 5347cd862d..c9fbcc4c19 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -2050,6 +2050,8 @@ var issueWriteFormParams = map[string]struct{}{ 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" + // The form cannot preserve an empty type, so execute the complete call + // directly to avoid dropping the clear or any co-submitted values. return !explicitTypeClear && shouldDeferToForm(ctx, deps, req, args, issueWriteFormParams) } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 522242fa06..101cd21dc5 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2939,6 +2939,92 @@ func TestIssueWriteClearTypeBypassesMCPAppForm(t *testing.T) { require.Contains(t, textContent.Text, "https://github.com/owner/repo/issues/123") } +func TestIssueWriteClearTypeBypassesMCPAppFormWithStateChange(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"}`)) + }, + })) + issueID := githubv4.ID("I_kwDOA0xdyM50BPaO") + stateReason := IssueClosedStateReasonCompleted + gqlClient := 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(123), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{"id": issueID}, + }, + }), + ), + 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: issueID, + StateReason: &stateReason, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "closeIssue": map[string]any{ + "issue": map[string]any{ + "id": issueID, + "number": 123, + "url": "https://github.com/owner/repo/issues/123", + "state": "CLOSED", + }, + }, + }), + ), + )) + deps := BaseDeps{ + Client: client, + GQLClient: gqlClient, + 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": "", + "state": "closed", + "state_reason": "completed", + }) + + 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) From 0508cd7c3c67f8a58a8c97efbfe4f77a71a87383 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Mon, 3 Aug 2026 12:46:30 -0400 Subject: [PATCH 8/9] Clarify issue type form routing Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea8faa5c-7f26-4e2d-bf9c-6f0b5f173e8c --- pkg/github/issues.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index c9fbcc4c19..f73b5fcf0a 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -2047,14 +2047,6 @@ 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" - // The form cannot preserve an empty type, so execute the complete call - // directly to avoid dropping the clear or any co-submitted values. - 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 @@ -2232,7 +2224,15 @@ Options are: // Hand off to the interactive MCP App form unless this call must // execute now (see shouldDeferToForm). - if shouldIssueWriteDeferToForm(ctx, deps, req, args) { + deferToForm := shouldDeferToForm(ctx, deps, req, args, issueWriteFormParams) + if method == "update" { + if issueType, ok := args["type"].(string); ok && issueType == "" { + // The form replaces an empty type with the current type, so execute + // directly to preserve the clear and any co-submitted values. + deferToForm = false + } + } + if deferToForm { issueNumber := 0 if method == "update" { n, numErr := RequiredInt(args, "issue_number") From 888b35c32716a2c26426d26589c604af6b624805 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Mon, 3 Aug 2026 16:44:10 -0400 Subject: [PATCH 9/9] Clarify granular issue type test Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea8faa5c-7f26-4e2d-bf9c-6f0b5f173e8c --- pkg/github/granular_tools_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index b5410d26fb..85e29c7b2f 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -823,7 +823,7 @@ func TestGranularUpdateIssueType(t *testing.T) { } } -func TestGranularUpdateIssueTypeRequiresType(t *testing.T) { +func TestGranularUpdateIssueTypeRejectsMissingIssueType(t *testing.T) { deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} serverTool := GranularUpdateIssueType(translations.NullTranslationHelper) handler := serverTool.Handler(deps)