Skip to content

Commit 80baa37

Browse files
committed
Use explicit null to clear issue types
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea8faa5c-7f26-4e2d-bf9c-6f0b5f173e8c
1 parent 888b35c commit 80baa37

13 files changed

Lines changed: 318 additions & 63 deletions

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -941,7 +941,7 @@ The following sets of tools are available:
941941
- `state`: New state (string, optional)
942942
- `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
943943
- `title`: Issue title (string, optional)
944-
- `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)
944+
- `type`: Type of this issue. For updates, pass null 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 | null, optional)
945945

946946
- **list_issue_fields** - List issue fields
947947
- **Required OAuth Scopes (any of)**: `repo`, `read:org`

cmd/github-mcp-server/generate_docs.go

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -273,19 +273,7 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {
273273
requiredStr = "required"
274274
}
275275

276-
var typeStr string
277-
278-
// Get the type and description
279-
switch prop.Type {
280-
case "array":
281-
if prop.Items != nil {
282-
typeStr = prop.Items.Type + "[]"
283-
} else {
284-
typeStr = "array"
285-
}
286-
default:
287-
typeStr = prop.Type
288-
}
276+
typeStr := schemaTypeString(prop)
289277

290278
// Indent any continuation lines in the description to maintain markdown formatting
291279
description := indentMultilineDescription(prop.Description, " ")
@@ -300,6 +288,37 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {
300288
}
301289
}
302290

291+
func schemaTypeString(schema *jsonschema.Schema) string {
292+
if schema == nil {
293+
return ""
294+
}
295+
if schema.Type == "array" {
296+
if schema.Items != nil {
297+
return schema.Items.Type + "[]"
298+
}
299+
return "array"
300+
}
301+
if schema.Type != "" {
302+
return schema.Type
303+
}
304+
if len(schema.Types) > 0 {
305+
return strings.Join(schema.Types, " | ")
306+
}
307+
308+
union := schema.AnyOf
309+
if len(union) == 0 {
310+
union = schema.OneOf
311+
}
312+
types := make([]string, 0, len(union))
313+
for _, member := range union {
314+
memberType := schemaTypeString(member)
315+
if memberType != "" && !slices.Contains(types, memberType) {
316+
types = append(types, memberType)
317+
}
318+
}
319+
return strings.Join(types, " | ")
320+
}
321+
303322
// scopesEqual checks if two scope slices contain the same elements (order-independent)
304323
func scopesEqual(a, b []string) bool {
305324
if len(a) != len(b) {

cmd/github-mcp-server/main_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"path/filepath"
66
"testing"
77

8+
"github.com/google/jsonschema-go/jsonschema"
89
"github.com/stretchr/testify/assert"
910
"github.com/stretchr/testify/require"
1011
)
@@ -36,3 +37,53 @@ func TestGitHubAppFlagsAreStdioOnly(t *testing.T) {
3637
assert.NotNil(t, stdioCmd.Flags().Lookup("app-id"))
3738
assert.Nil(t, httpCmd.Flags().Lookup("app-id"))
3839
}
40+
41+
func TestSchemaTypeString(t *testing.T) {
42+
tests := []struct {
43+
name string
44+
schema *jsonschema.Schema
45+
want string
46+
}{
47+
{name: "type", schema: &jsonschema.Schema{Type: "string"}, want: "string"},
48+
{name: "types", schema: &jsonschema.Schema{Types: []string{"string", "number"}}, want: "string | number"},
49+
{
50+
name: "anyOf",
51+
schema: &jsonschema.Schema{AnyOf: []*jsonschema.Schema{
52+
{Type: "string"},
53+
{Type: "null"},
54+
}},
55+
want: "string | null",
56+
},
57+
{
58+
name: "oneOf",
59+
schema: &jsonschema.Schema{OneOf: []*jsonschema.Schema{
60+
{Type: "number"},
61+
{Type: "string"},
62+
}},
63+
want: "number | string",
64+
},
65+
{
66+
name: "array",
67+
schema: &jsonschema.Schema{Type: "array", Items: &jsonschema.Schema{Type: "string"}},
68+
want: "string[]",
69+
},
70+
{
71+
name: "union array preserves existing rendering",
72+
schema: &jsonschema.Schema{
73+
Type: "array",
74+
Items: &jsonschema.Schema{AnyOf: []*jsonschema.Schema{
75+
{Type: "string"},
76+
{Type: "object"},
77+
}},
78+
},
79+
want: "[]",
80+
},
81+
{name: "untyped array", schema: &jsonschema.Schema{Type: "array"}, want: "array"},
82+
}
83+
84+
for _, tc := range tests {
85+
t.Run(tc.name, func(t *testing.T) {
86+
assert.Equal(t, tc.want, schemaTypeString(tc.schema))
87+
})
88+
}
89+
}

docs/feature-flags.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ runtime behavior (such as output formatting) won't appear here.
7171
- `state`: New state (string, optional)
7272
- `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
7373
- `title`: Issue title (string, optional)
74-
- `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)
74+
- `type`: Type of this issue. For updates, pass null 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 | null, optional)
7575

7676
- **ui_get** - Get UI data
7777
- **Required OAuth Scopes (any of)**: `repo`, `read:org`
@@ -200,7 +200,7 @@ runtime behavior (such as output formatting) won't appear here.
200200
- `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)
201201
- `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)
202202
- `issue_number`: The issue number to update (number, required)
203-
- `issue_type`: The issue type to set, or an empty string to remove the current type (string, required)
203+
- `issue_type`: The issue type to set, or null to remove the current type (string | null, required)
204204
- `owner`: Repository owner (username or organization) (string, required)
205205
- `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)
206206
- `repo`: Repository name (string, required)

docs/insiders-features.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ The list below is generated from the Go source. It covers tool **inventory and s
6565
- `state`: New state (string, optional)
6666
- `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
6767
- `title`: Issue title (string, optional)
68-
- `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)
68+
- `type`: Type of this issue. For updates, pass null 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 | null, optional)
6969

7070
- **ui_get** - Get UI data
7171
- **Required OAuth Scopes (any of)**: `repo`, `read:org`

pkg/github/__toolsnaps__/issue_write.snap

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,16 @@
120120
"type": "string"
121121
},
122122
"type": {
123-
"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.",
124-
"type": "string"
123+
"anyOf": [
124+
{
125+
"minLength": 1,
126+
"type": "string"
127+
},
128+
{
129+
"type": "null"
130+
}
131+
],
132+
"description": "Type of this issue. For updates, pass null 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."
125133
}
126134
},
127135
"required": [

pkg/github/__toolsnaps__/update_issue_type.snap

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"readOnlyHint": false,
77
"title": "Update Issue Type"
88
},
9-
"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.",
9+
"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.",
1010
"inputSchema": {
1111
"properties": {
1212
"confidence": {
@@ -28,8 +28,16 @@
2828
"type": "number"
2929
},
3030
"issue_type": {
31-
"description": "The issue type to set, or an empty string to remove the current type",
32-
"type": "string"
31+
"anyOf": [
32+
{
33+
"minLength": 1,
34+
"type": "string"
35+
},
36+
{
37+
"type": "null"
38+
}
39+
],
40+
"description": "The issue type to set, or null to remove the current type"
3341
},
3442
"owner": {
3543
"description": "Repository owner (username or organization)",

pkg/github/granular_tools_test.go

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package github
33
import (
44
"context"
55
"encoding/json"
6+
"maps"
67
"net/http"
78
"strings"
89
"testing"
@@ -756,7 +757,11 @@ func TestGranularUpdateIssueMilestone(t *testing.T) {
756757

757758
func TestGranularUpdateIssueType(t *testing.T) {
758759
toolSchema := GranularUpdateIssueType(translations.NullTranslationHelper).Tool.InputSchema.(*jsonschema.Schema)
759-
assert.Equal(t, "string", toolSchema.Properties["issue_type"].Type)
760+
issueTypeSchema := toolSchema.Properties["issue_type"]
761+
require.Len(t, issueTypeSchema.AnyOf, 2)
762+
assert.Equal(t, "string", issueTypeSchema.AnyOf[0].Type)
763+
assert.Equal(t, 1, *issueTypeSchema.AnyOf[0].MinLength)
764+
assert.Equal(t, "null", issueTypeSchema.AnyOf[1].Type)
760765

761766
tests := []struct {
762767
name string
@@ -792,12 +797,12 @@ func TestGranularUpdateIssueType(t *testing.T) {
792797
},
793798
},
794799
{
795-
name: "remove type with empty string",
800+
name: "remove type with null",
796801
requestArgs: map[string]any{
797802
"owner": "owner",
798803
"repo": "repo",
799804
"issue_number": float64(1),
800-
"issue_type": "",
805+
"issue_type": nil,
801806
},
802807
expectedReq: map[string]any{
803808
"type": nil,
@@ -823,6 +828,23 @@ func TestGranularUpdateIssueType(t *testing.T) {
823828
}
824829
}
825830

831+
func TestGranularUpdateIssueTypeRejectsEmptyIssueType(t *testing.T) {
832+
deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))}
833+
serverTool := GranularUpdateIssueType(translations.NullTranslationHelper)
834+
handler := serverTool.Handler(deps)
835+
request := createMCPRequest(map[string]any{
836+
"owner": "owner",
837+
"repo": "repo",
838+
"issue_number": float64(1),
839+
"issue_type": "",
840+
})
841+
842+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
843+
require.NoError(t, err)
844+
errorContent := getErrorResult(t, result)
845+
assert.Contains(t, errorContent.Text, "parameter issue_type must not be empty")
846+
}
847+
826848
func TestGranularUpdateIssueTypeRejectsMissingIssueType(t *testing.T) {
827849
deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))}
828850
serverTool := GranularUpdateIssueType(translations.NullTranslationHelper)
@@ -840,6 +862,38 @@ func TestGranularUpdateIssueTypeRejectsMissingIssueType(t *testing.T) {
840862
assert.Contains(t, errorContent.Text, "missing required parameter: issue_type")
841863
}
842864

865+
func TestGranularUpdateIssueTypeRejectsMetadataWhenRemovingType(t *testing.T) {
866+
tests := []struct {
867+
name string
868+
args map[string]any
869+
}{
870+
{name: "rationale", args: map[string]any{"rationale": "live validation"}},
871+
{name: "confidence", args: map[string]any{"confidence": "HIGH"}},
872+
{name: "suggestion", args: map[string]any{"is_suggestion": true}},
873+
}
874+
875+
for _, tc := range tests {
876+
t.Run(tc.name, func(t *testing.T) {
877+
deps := BaseDeps{}
878+
serverTool := GranularUpdateIssueType(translations.NullTranslationHelper)
879+
handler := serverTool.Handler(deps)
880+
args := map[string]any{
881+
"owner": "owner",
882+
"repo": "repo",
883+
"issue_number": float64(1),
884+
"issue_type": nil,
885+
}
886+
maps.Copy(args, tc.args)
887+
request := createMCPRequest(args)
888+
889+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
890+
require.NoError(t, err)
891+
errorContent := getErrorResult(t, result)
892+
assert.Contains(t, errorContent.Text, "suggestion metadata is not supported when removing an issue type; omit rationale, confidence, and is_suggestion")
893+
})
894+
}
895+
}
896+
843897
func TestGranularUpdateIssueTypeSuggest(t *testing.T) {
844898
tests := []struct {
845899
name string

pkg/github/issues.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2150,8 +2150,11 @@ Options are:
21502150
Description: "Milestone number",
21512151
},
21522152
"type": {
2153-
Type: "string",
2154-
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.",
2153+
AnyOf: []*jsonschema.Schema{
2154+
{Type: "string", MinLength: jsonschema.Ptr(1)},
2155+
{Type: "null"},
2156+
},
2157+
Description: "Type of this issue. For updates, pass null 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.",
21552158
},
21562159
"state": {
21572160
Type: "string",
@@ -2226,8 +2229,8 @@ Options are:
22262229
// execute now (see shouldDeferToForm).
22272230
deferToForm := shouldDeferToForm(ctx, deps, req, args, issueWriteFormParams)
22282231
if method == "update" {
2229-
if issueType, ok := args["type"].(string); ok && issueType == "" {
2230-
// The form replaces an empty type with the current type, so execute
2232+
if issueType, ok := args["type"]; ok && issueType == nil {
2233+
// The form replaces a null type with the current type, so execute
22312234
// directly to preserve the clear and any co-submitted values.
22322235
deferToForm = false
22332236
}
@@ -2283,10 +2286,14 @@ Options are:
22832286
}
22842287

22852288
// Get optional type
2286-
issueType, issueTypeProvided, err := OptionalParamOK[string](args, "type")
2289+
issueTypeParam, issueTypeProvided, err := OptionalNullableStringParam(args, "type")
22872290
if err != nil {
22882291
return utils.NewToolResultError(err.Error()), nil, nil
22892292
}
2293+
issueType := ""
2294+
if issueTypeParam != nil {
2295+
issueType = *issueTypeParam
2296+
}
22902297

22912298
// Handle state, state_reason and duplicateOf parameters
22922299
state, err := OptionalParam[string](args, "state")
@@ -2415,7 +2422,7 @@ type UpdateIssueOptions struct {
24152422
AssigneesProvided bool
24162423
// LabelsProvided sends the labels field even when the slice is empty.
24172424
LabelsProvided bool
2418-
// IssueTypeProvided sends the type field, including an explicit clear for an empty value.
2425+
// IssueTypeProvided sends the type field, including an explicit clear.
24192426
IssueTypeProvided bool
24202427
}
24212428

0 commit comments

Comments
 (0)