From 48f0bb79bc6ba44a7c2fe644a3f69cd7467eda4b Mon Sep 17 00:00:00 2001 From: Travis Gockel Date: Fri, 14 Aug 2026 18:04:15 -0600 Subject: [PATCH] fix(issues): allow delete:false in issue_write issue_fields The `delete` property of issue_write's issue_fields items was declared with Enum: []any{true}, making true its only legal value. The property is optional, but a client that fills every property of a schema -- common, since OpenAI-style strict function calling requires every property to appear in `required` -- had no way to express "not deleting this field": there is no false in the enum and no null in the type. `value` offers no alternative either, being typed ["string","number","boolean"] with no null. The MCP Go SDK validates arguments against the resolved input schema before the handler runs, so delete: false was rejected at schema validation and never reached optionalIssueWriteFields. Such clients sent delete: true alongside a value instead and hit the handler's mutual-exclusion check, so issue_write could never set an issue field for them. Remove the enum so false is a legal no-op, and document that omitting the property or setting it to false leaves the field unchanged. No handler change is needed: the code already branches on `if deleteField`, so false falls through to the normal value path, and the mutual-exclusion check for delete: true still applies. Add tests for optionalIssueWriteFields, which had none. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/github/__toolsnaps__/issue_write.snap | 5 +- pkg/github/issues.go | 5 +- pkg/github/issues_test.go | 57 +++++++++++++++++++++++ 3 files changed, 61 insertions(+), 6 deletions(-) diff --git a/pkg/github/__toolsnaps__/issue_write.snap b/pkg/github/__toolsnaps__/issue_write.snap index 10efb6c6df..5211e7cb29 100644 --- a/pkg/github/__toolsnaps__/issue_write.snap +++ b/pkg/github/__toolsnaps__/issue_write.snap @@ -37,10 +37,7 @@ "additionalProperties": false, "properties": { "delete": { - "description": "Set to true to clear this field's current value on the issue. Cannot be combined with 'value' or 'field_option_name'.", - "enum": [ - true - ], + "description": "Set to true to clear this field's current value on the issue. Cannot be combined with 'value' or 'field_option_name'. Omit this property, or set it to false, to leave the field's current value unchanged.", "type": "boolean" }, "field_name": { diff --git a/pkg/github/issues.go b/pkg/github/issues.go index dfb823e26b..70bd00ab2c 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -2287,9 +2287,10 @@ Options are: }, "delete": { Type: "boolean", - Enum: []any{true}, Description: "Set to true to clear this field's current value on the " + - "issue. Cannot be combined with 'value' or 'field_option_name'.", + "issue. Cannot be combined with 'value' or 'field_option_name'. " + + "Omit this property, or set it to false, to leave the field's " + + "current value unchanged.", }, }, Required: []string{"field_name"}, diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 77380e5e21..f4917897fb 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2098,6 +2098,63 @@ func Test_issueWriteHasNonFormParams(t *testing.T) { } } +// Test_optionalIssueWriteFields covers parsing of issue_write's issue_fields +// items. The delete:false cases matter because the schema deliberately does not +// constrain 'delete' to a single value: clients that populate every property of +// a schema need a way to say "not deleting", and false must be a no-op that +// falls through to the normal value path. +func Test_optionalIssueWriteFields(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + item map[string]any + want issueWriteFieldInput + wantErr string + }{ + { + name: "delete false alongside a value sets the value", + item: map[string]any{"field_name": "Start date", "value": "2026-08-14", "delete": false, "field_option_name": ""}, + want: issueWriteFieldInput{FieldName: "Start date", Value: "2026-08-14"}, + }, + { + name: "delete true alone clears the field", + item: map[string]any{"field_name": "Start date", "delete": true}, + want: issueWriteFieldInput{FieldName: "Start date", Delete: true}, + }, + { + name: "delete omitted with field_option_name", + item: map[string]any{"field_name": "Priority", "field_option_name": "High"}, + want: issueWriteFieldInput{FieldName: "Priority", FieldOptionName: "High"}, + }, + { + name: "delete true with a value is rejected", + item: map[string]any{"field_name": "Start date", "value": "2026-08-14", "delete": true}, + wantErr: "cannot specify 'delete' together with 'value' or 'field_option_name'", + }, + { + name: "delete false with nothing to set is rejected", + item: map[string]any{"field_name": "Start date", "delete": false}, + wantErr: "must specify either value or field_option_name", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := optionalIssueWriteFields(map[string]any{"issue_fields": []any{tc.item}}) + if tc.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr) + return + } + require.NoError(t, err) + require.Len(t, got, 1) + assert.Equal(t, tc.want, got[0]) + }) + } +} + // Test_issueWriteSchemaClassification fails when a schema property is added // without classifying it as either form-resendable (issueWriteFormParams) or // known-non-form (knownNonForm below). Without this guard, an unclassified