diff --git a/docs/feature-flags.md b/docs/feature-flags.md index a552e71a0..284ab2edf 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -177,6 +177,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, 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..ff025c65c 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" @@ -463,62 +462,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) }) } } 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) }