diff --git a/pkg/github/__toolsnaps__/issue_write.snap b/pkg/github/__toolsnaps__/issue_write.snap index 10efb6c6df..d4968c4f2f 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. When false or omitted, this property is ignored. Cannot be true when 'value' or 'field_option_name' is provided.", "type": "boolean" }, "field_name": { @@ -48,11 +45,11 @@ "type": "string" }, "field_option_name": { - "description": "Option name for single-select fields. Validated against the field's options before the API call. Cannot be combined with 'value' or 'delete'.", + "description": "Option name for single-select fields. Validated against the field's options before the API call. Cannot be combined with 'value' or 'delete: true'.", "type": "string" }, "value": { - "description": "Value to set. Use for text, number, and date fields (date as YYYY-MM-DD). For single-select fields, prefer 'field_option_name' so the option is validated before the API call. Cannot be combined with 'field_option_name' or 'delete'.", + "description": "Value to set. Use for text, number, and date fields (date as YYYY-MM-DD). For single-select fields, prefer 'field_option_name' so the option is validated before the API call. Cannot be combined with 'field_option_name' or 'delete: true'.", "type": [ "string", "number", diff --git a/pkg/github/issues.go b/pkg/github/issues.go index dfb823e26b..a830333fa6 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -265,7 +265,10 @@ func optionalIssueWriteFields(args map[string]any) ([]issueWriteFieldInput, erro return nil, err } - deleteField, _ := OptionalParam[bool](itemMap, "delete") + deleteField, err := OptionalParam[bool](itemMap, "delete") + if err != nil { + return nil, err + } value, hasValue := itemMap["value"] if hasValue && value == nil { return nil, fmt.Errorf("value cannot be null for field %q", fieldName) @@ -2277,19 +2280,19 @@ Options are: Description: "Value to set. Use for text, number, and date fields " + "(date as YYYY-MM-DD). For single-select fields, prefer " + "'field_option_name' so the option is validated before the API " + - "call. Cannot be combined with 'field_option_name' or 'delete'.", + "call. Cannot be combined with 'field_option_name' or 'delete: true'.", }, "field_option_name": { Type: "string", Description: "Option name for single-select fields. Validated against " + "the field's options before the API call. Cannot be combined with " + - "'value' or 'delete'.", + "'value' or 'delete: true'.", }, "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. When false or omitted, this property is ignored. Cannot " + + "be true when 'value' or 'field_option_name' is provided.", }, }, Required: []string{"field_name"}, diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 77380e5e21..1ba7249560 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1823,8 +1823,8 @@ func Test_CreateIssue(t *testing.T) { "repo": "repo", "title": "Issue with fields", "issue_fields": []any{ - map[string]any{"field_name": "Priority", "field_option_name": "P1"}, - map[string]any{"field_name": "Customer", "value": "Acme"}, + map[string]any{"field_name": "Priority", "field_option_name": "P1", "delete": false}, + map[string]any{"field_name": "Customer", "value": "Acme", "delete": false}, }, }, expectError: false, @@ -1867,6 +1867,21 @@ func Test_CreateIssue(t *testing.T) { expectError: false, expectedErrMsg: "cannot specify both value and field_option_name", }, + { + name: "issue_fields rejects delete true with value", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}), + requestArgs: map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "title": "Invalid fields", + "issue_fields": []any{ + map[string]any{"field_name": "Start date", "value": "2026-08-14", "delete": true}, + }, + }, + expectError: false, + expectedErrMsg: "cannot specify 'delete' together with 'value' or 'field_option_name'", + }, } for _, tc := range tests { @@ -2098,6 +2113,84 @@ func Test_issueWriteHasNonFormParams(t *testing.T) { } } +func Test_IssueWriteIssueFieldsDeleteSchema(t *testing.T) { + t.Parallel() + + inputSchema := IssueWrite(translations.NullTranslationHelper).Tool.InputSchema.(*jsonschema.Schema) + deleteSchema := inputSchema.Properties["issue_fields"].Items.Properties["delete"] + + assert.Equal(t, "boolean", deleteSchema.Type) + assert.Empty(t, deleteSchema.Enum) + assert.Contains(t, deleteSchema.Description, "When false or omitted, this property is ignored") +} + +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 is ignored", + 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 false alongside field_option_name is ignored", + item: map[string]any{"field_name": "Priority", "field_option_name": "High", "delete": false}, + want: issueWriteFieldInput{FieldName: "Priority", FieldOptionName: "High"}, + }, + { + 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 true with field_option_name is rejected", + item: map[string]any{"field_name": "Priority", "field_option_name": "High", "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", + }, + { + name: "delete with invalid type is rejected", + item: map[string]any{"field_name": "Start date", "delete": "false"}, + wantErr: "parameter delete is not of type bool", + }, + } + + 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