diff --git a/docs/feature-flags.md b/docs/feature-flags.md index afd6a52c7..b04dfc2cd 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -178,7 +178,7 @@ runtime behavior (such as output formatting) won't appear here. - **update_issue_type** - Update Issue Type - **Required OAuth Scopes**: `repo` - - `is_suggestion`: If true, propose the issue type change instead of applying it. Defaults to false, which applies the change to the issue. (boolean, 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) - `owner`: Repository owner (username or organization) (string, required) diff --git a/pkg/github/__toolsnaps__/set_issue_fields.snap b/pkg/github/__toolsnaps__/set_issue_fields.snap index 979dde4fb..88c88fdc6 100644 --- a/pkg/github/__toolsnaps__/set_issue_fields.snap +++ b/pkg/github/__toolsnaps__/set_issue_fields.snap @@ -23,6 +23,10 @@ "description": "The GraphQL node ID of the issue field", "type": "string" }, + "is_suggestion": { + "description": "If true, this field value is sent to the API as a suggestion (suggest:true) rather than an applied value. Whether the value is applied or recorded as a proposal is determined by the API.", + "type": "boolean" + }, "number_value": { "description": "The value to set for a number field", "type": "number" diff --git a/pkg/github/__toolsnaps__/update_issue_labels.snap b/pkg/github/__toolsnaps__/update_issue_labels.snap index 89ff86b2f..3bdbdfc9e 100644 --- a/pkg/github/__toolsnaps__/update_issue_labels.snap +++ b/pkg/github/__toolsnaps__/update_issue_labels.snap @@ -22,6 +22,10 @@ }, { "properties": { + "is_suggestion": { + "description": "If true, this label is sent to the API as a suggestion (suggest:true) rather than an applied label. Whether the label is applied or recorded as a proposal is determined by the API.", + "type": "boolean" + }, "name": { "description": "Label name", "type": "string" diff --git a/pkg/github/__toolsnaps__/update_issue_type.snap b/pkg/github/__toolsnaps__/update_issue_type.snap index 7fb5fde89..da749cd46 100644 --- a/pkg/github/__toolsnaps__/update_issue_type.snap +++ b/pkg/github/__toolsnaps__/update_issue_type.snap @@ -8,7 +8,7 @@ "inputSchema": { "properties": { "is_suggestion": { - "description": "If true, propose the issue type change instead of applying it. Defaults to false, which applies the change to the issue.", + "description": "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.", "type": "boolean" }, "issue_number": { diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 88bd560b4..90b42b22c 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -2,7 +2,6 @@ package github import ( "context" - "encoding/json" "net/http" "strings" "testing" @@ -336,6 +335,84 @@ func TestGranularUpdateIssueLabels(t *testing.T) { } } +func TestGranularUpdateIssueLabelsSuggest(t *testing.T) { + tests := []struct { + name string + requestArgs map[string]any + expectedReq map[string]any + }{ + { + name: "single label suggested without rationale", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": []any{ + map[string]any{"name": "bug", "is_suggestion": true}, + }, + }, + expectedReq: map[string]any{ + "labels": []any{ + map[string]any{"name": "bug", "suggest": true}, + }, + }, + }, + { + name: "suggested label with rationale", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": []any{ + map[string]any{"name": "frontend", "rationale": "Mentions the UI button", "is_suggestion": true}, + }, + }, + expectedReq: map[string]any{ + "labels": []any{ + map[string]any{"name": "frontend", "rationale": "Mentions the UI button", "suggest": true}, + }, + }, + }, + { + name: "mix of plain, applied-with-rationale, and suggested labels", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": []any{ + "triage", + map[string]any{"name": "bug", "rationale": "Reports a crash when saving"}, + map[string]any{"name": "needs-design", "is_suggestion": true}, + }, + }, + expectedReq: map[string]any{ + "labels": []any{ + "triage", + map[string]any{"name": "bug", "rationale": "Reports a crash when saving"}, + map[string]any{"name": "needs-design", "suggest": true}, + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). + andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) + }) + } +} + func TestGranularUpdateIssueLabelsInvalidRationale(t *testing.T) { tests := []struct { name string @@ -463,62 +540,58 @@ func TestGranularUpdateIssueTypeSuggest(t *testing.T) { tests := []struct { name string requestArgs map[string]any - expected map[string]any + expectedReq map[string]any }{ { name: "suggest without rationale", requestArgs: map[string]any{ - "owner": "owner", - "repo": "repo", - "issue_number": float64(1), - "issue_type": "bug", - "suggest": true, + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "issue_type": "bug", + "is_suggestion": true, }, - expected: map[string]any{ - "owner": "owner", - "repo": "repo", - "issue_number": float64(1), - "issue_type": "bug", - "suggested": true, + expectedReq: map[string]any{ + "type": map[string]any{ + "value": "bug", + "suggest": true, + }, }, }, { name: "suggest with rationale", requestArgs: map[string]any{ - "owner": "owner", - "repo": "repo", - "issue_number": float64(1), - "issue_type": "feature", - "rationale": " Asks for dark mode support ", - "suggest": true, + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "issue_type": "feature", + "rationale": " Asks for dark mode support ", + "is_suggestion": true, }, - expected: map[string]any{ - "owner": "owner", - "repo": "repo", - "issue_number": float64(1), - "issue_type": "feature", - "rationale": "Asks for dark mode support", - "suggested": true, + expectedReq: map[string]any{ + "type": map[string]any{ + "value": "feature", + "rationale": "Asks for dark mode support", + "suggest": true, + }, }, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - // No HTTP handler registered: any API call would fail the test. - deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). + andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), + })) + deps := BaseDeps{Client: client} serverTool := GranularUpdateIssueType(translations.NullTranslationHelper) handler := serverTool.Handler(deps) request := createMCPRequest(tc.requestArgs) result, err := handler(ContextWithDeps(context.Background(), deps), &request) require.NoError(t, err) - require.False(t, result.IsError) - - textContent := getTextResult(t, result) - var got map[string]any - require.NoError(t, json.Unmarshal([]byte(textContent.Text), &got)) - assert.Equal(t, tc.expected, got) + assert.False(t, result.IsError) }) } } @@ -1312,4 +1385,97 @@ func TestGranularSetIssueFields(t *testing.T) { textContent := getTextResult(t, result) assert.Contains(t, textContent.Text, "field rationale must be 280 characters or less") }) + + t.Run("successful set with suggest flag", func(t *testing.T) { + suggestTrue := githubv4.Boolean(true) + matchers := []githubv4mock.Matcher{ + 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(5), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{"id": "ISSUE_123"}, + }, + }), + ), + githubv4mock.NewMutationMatcher( + struct { + SetIssueFieldValue struct { + Issue struct { + ID githubv4.ID + Number githubv4.Int + URL githubv4.String + } + IssueFieldValues []struct { + TextValue struct { + Value string + } `graphql:"... on IssueFieldTextValue"` + SingleSelectValue struct { + Name string + } `graphql:"... on IssueFieldSingleSelectValue"` + DateValue struct { + Value string + } `graphql:"... on IssueFieldDateValue"` + NumberValue struct { + Value float64 + } `graphql:"... on IssueFieldNumberValue"` + } + } `graphql:"setIssueFieldValue(input: $input)"` + }{}, + SetIssueFieldValueInput{ + IssueID: githubv4.ID("ISSUE_123"), + IssueFields: []IssueFieldCreateOrUpdateInput{ + { + FieldID: githubv4.ID("FIELD_1"), + TextValue: githubv4.NewString(githubv4.String("hello")), + Rationale: githubv4.NewString(githubv4.String("Reflects the reported severity")), + Suggest: &suggestTrue, + }, + }, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "setIssueFieldValue": map[string]any{ + "issue": map[string]any{ + "id": "ISSUE_123", + "number": 5, + "url": "https://github.com/owner/repo/issues/5", + }, + }, + }), + ), + } + + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matchers...)) + deps := BaseDeps{GQLClient: gqlClient} + serverTool := GranularSetIssueFields(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(5), + "fields": []any{ + map[string]any{ + "field_id": "FIELD_1", + "text_value": "hello", + "rationale": "Reflects the reported severity", + "is_suggestion": true, + }, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) + }) } diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 9e789c6d1..73fa75413 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -259,10 +259,11 @@ func GranularUpdateIssueAssignees(t translations.TranslationHelperFunc) inventor } // labelWithRationale represents the object form of a label entry, allowing a -// rationale to be sent alongside the label name. +// rationale and/or suggest flag to be sent alongside the label name. type labelWithRationale struct { Name string `json:"name"` Rationale string `json:"rationale,omitempty"` + Suggest bool `json:"suggest,omitempty"` } // labelsUpdateRequest is a custom request body for updating an issue's labels @@ -320,6 +321,11 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S "State the concrete signal (e.g. 'Reports a crash when saving' → bug).", MaxLength: jsonschema.Ptr(280), }, + "is_suggestion": { + Type: "boolean", + Description: "If true, this label is sent to the API as a suggestion (suggest:true) rather than an applied label. " + + "Whether the label is applied or recorded as a proposal is determined by the API.", + }, }, Required: []string{"name"}, }, @@ -362,7 +368,7 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S } } - anyRationale := false + useObjectForm := false payload := make([]any, 0, len(labelsSlice)) for _, item := range labelsSlice { switch v := item.(type) { @@ -381,14 +387,18 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S if len([]rune(rationale)) > 280 { return utils.NewToolResultError("label rationale must be 280 characters or less"), nil, nil } - if rationale == "" { + isSuggestion, err := OptionalParam[bool](v, "is_suggestion") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if rationale == "" && !isSuggestion { payload = append(payload, name) } else { - anyRationale = true - payload = append(payload, labelWithRationale{Name: name, Rationale: rationale}) + useObjectForm = true + payload = append(payload, labelWithRationale{Name: name, Rationale: rationale, Suggest: isSuggestion}) } default: - return utils.NewToolResultError("each label must be a string or an object with 'name' and optional 'rationale'"), nil, nil + return utils.NewToolResultError("each label must be a string or an object with 'name' and optional 'rationale' and/or 'is_suggestion'"), nil, nil } } @@ -398,10 +408,10 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S } var body any - if anyRationale { + if useObjectForm { body = &labelsUpdateRequest{Labels: payload} } else { - // Preserve the standard wire format when no rationale is supplied. + // Preserve the standard wire format when no rationale or suggest is supplied. names := make([]string, len(payload)) for i, p := range payload { names[i] = p.(string) @@ -461,10 +471,11 @@ func GranularUpdateIssueMilestone(t translations.TranslationHelperFunc) inventor } // issueTypeWithRationale represents the object form of the issue type field, -// allowing a rationale to be sent alongside the type name. +// allowing a rationale and/or suggest flag to be sent alongside the type name. type issueTypeWithRationale struct { Value string `json:"value"` - Rationale string `json:"rationale"` + Rationale string `json:"rationale,omitempty"` + Suggest bool `json:"suggest,omitempty"` } // issueTypeUpdateRequest is a custom request body for updating an issue type @@ -514,8 +525,8 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser }, "is_suggestion": { Type: "boolean", - Description: "If true, propose the issue type change instead of applying it. " + - "Defaults to false, which applies the change to the issue.", + Description: "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.", }, }, Required: []string{"owner", "repo", "issue_number", "issue_type"}, @@ -547,40 +558,23 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser if len([]rune(rationale)) > 280 { return utils.NewToolResultError("parameter rationale must be 280 characters or less"), nil, nil } - suggest, err := OptionalParam[bool](args, "suggest") + isSuggestion, err := OptionalParam[bool](args, "is_suggestion") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - if suggest { - suggestion := map[string]any{ - "owner": owner, - "repo": repo, - "issue_number": issueNumber, - "issue_type": issueType, - "suggested": true, - } - if rationale != "" { - suggestion["rationale"] = rationale - } - r, err := json.Marshal(suggestion) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal suggestion", err), nil, nil - } - return utils.NewToolResultText(string(r)), nil, nil - } - client, err := deps.GetClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } var body any - if rationale != "" { + if rationale != "" || isSuggestion { body = &issueTypeUpdateRequest{ Type: issueTypeWithRationale{ Value: issueType, Rationale: rationale, + Suggest: isSuggestion, }, } } else { @@ -893,6 +887,7 @@ type IssueFieldCreateOrUpdateInput struct { SingleSelectOptionID *githubv4.ID `json:"singleSelectOptionId,omitempty"` Delete *githubv4.Boolean `json:"delete,omitempty"` Rationale *githubv4.String `json:"rationale,omitempty"` + Suggest *githubv4.Boolean `json:"suggest,omitempty"` } // GranularSetIssueFields creates a tool to set issue field values on an issue using GraphQL. @@ -961,6 +956,11 @@ func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.Serv "State the concrete signal (e.g. 'Reports a crash when saving' → high priority).", MaxLength: jsonschema.Ptr(280), }, + "is_suggestion": { + Type: "boolean", + Description: "If true, this field value is sent to the API as a suggestion (suggest:true) rather than an applied value. " + + "Whether the value is applied or recorded as a proposal is determined by the API.", + }, }, Required: []string{"field_id"}, }, @@ -1073,6 +1073,15 @@ func GranularSetIssueFields(t translations.TranslationHelperFunc) inventory.Serv } } + isSuggestion, err := OptionalParam[bool](fieldMap, "is_suggestion") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if isSuggestion { + suggestVal := githubv4.Boolean(true) + input.Suggest = &suggestVal + } + issueFields = append(issueFields, input) }