From 701d484516c9c2a744cc996cc83a43efb51ddd30 Mon Sep 17 00:00:00 2001 From: Alon Dahari Date: Wed, 17 Jun 2026 17:51:30 +0100 Subject: [PATCH] Add rationale and is_suggestion support to assignees in issue_write Update the issue_write MCP tool (IssueWrite, the FeatureFlagIssueFields-enabled variant) to accept assignees in polymorphic form: either plain strings (backward- compatible) or objects with login, rationale, confidence, and is_suggestion fields. When object-form assignees are provided, the handler: 1. Skips assignees in the standard UpdateIssue call 2. Makes a follow-up PATCH with the assignees in object form including intent metadata (rationale, confidence, suggest) This mirrors the pattern used for labels in GranularUpdateIssueLabels and type in GranularUpdateIssueType, gated behind FeatureFlagIssueFields. The REST API already supports: { "assignees": [{ "login": "octocat", "rationale": "...", "suggest": true }] } Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/feature-flags.md | 4 +- docs/insiders-features.md | 4 +- ...ssue_write_ff_remote_mcp_issue_fields.snap | 39 ++- pkg/github/issues.go | 188 +++++++++++++- pkg/github/issues_test.go | 236 ++++++++++++++++++ 5 files changed, 459 insertions(+), 12 deletions(-) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 590cb6597..08f59b64c 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -75,7 +75,7 @@ runtime behavior (such as output formatting) won't appear here. - `type`: Type of this issue. Only use if issue types are enabled for this repository. Use list_issue_types tool 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) - **ui_get** - Get UI data - - **Required OAuth Scopes**: `repo`, `read:org` + - **Required OAuth Scopes (any of)**: `repo`, `read:org` - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org` - `method`: The type of data to fetch (string, required) - `owner`: Repository owner (required for all methods) (string, required) @@ -99,7 +99,7 @@ runtime behavior (such as output formatting) won't appear here. - **issue_write** - Create or update issue/pull request - **Required OAuth Scopes**: `repo` - - `assignees`: Usernames to assign to this issue (string[], optional) + - `assignees`: Usernames to assign to this issue. ([], optional) - `body`: Issue body content (string, optional) - `duplicate_of`: Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'. (number, optional) - `issue_fields`: Issue field values to set or clear. Each item requires 'field_name' and exactly one of 'value', 'field_option_name', or 'delete: true'. (object[], optional) diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 3306b5cd8..f1e56db96 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -69,7 +69,7 @@ The list below is generated from the Go source. It covers tool **inventory and s - `type`: Type of this issue. Only use if issue types are enabled for this repository. Use list_issue_types tool 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) - **ui_get** - Get UI data - - **Required OAuth Scopes**: `repo`, `read:org` + - **Required OAuth Scopes (any of)**: `repo`, `read:org` - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `repo`, `write:org` - `method`: The type of data to fetch (string, required) - `owner`: Repository owner (required for all methods) (string, required) @@ -93,7 +93,7 @@ The list below is generated from the Go source. It covers tool **inventory and s - **issue_write** - Create or update issue/pull request - **Required OAuth Scopes**: `repo` - - `assignees`: Usernames to assign to this issue (string[], optional) + - `assignees`: Usernames to assign to this issue. ([], optional) - `body`: Issue body content (string, optional) - `duplicate_of`: Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'. (number, optional) - `issue_fields`: Issue field values to set or clear. Each item requires 'field_name' and exactly one of 'value', 'field_option_name', or 'delete: true'. (object[], optional) diff --git a/pkg/github/__toolsnaps__/issue_write_ff_remote_mcp_issue_fields.snap b/pkg/github/__toolsnaps__/issue_write_ff_remote_mcp_issue_fields.snap index 47d00c445..23973184b 100644 --- a/pkg/github/__toolsnaps__/issue_write_ff_remote_mcp_issue_fields.snap +++ b/pkg/github/__toolsnaps__/issue_write_ff_remote_mcp_issue_fields.snap @@ -15,9 +15,44 @@ "inputSchema": { "properties": { "assignees": { - "description": "Usernames to assign to this issue", + "description": "Usernames to assign to this issue.", "items": { - "type": "string" + "oneOf": [ + { + "description": "GitHub username", + "type": "string" + }, + { + "properties": { + "confidence": { + "description": "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.", + "enum": [ + "LOW", + "MEDIUM", + "HIGH" + ], + "type": "string" + }, + "is_suggestion": { + "description": "If true, this assignee is sent to the API as a suggestion (suggest:true) rather than a direct assignment. Whether the assignee is applied or recorded as a proposal is determined by the API.", + "type": "boolean" + }, + "login": { + "description": "GitHub username", + "type": "string" + }, + "rationale": { + "description": "One concise sentence explaining why this person is the right assignee. State the concrete signal (e.g. 'Owns the auth module where the bug was reported').", + "maxLength": 280, + "type": "string" + } + }, + "required": [ + "login" + ], + "type": "object" + } + ] }, "type": "array" }, diff --git a/pkg/github/issues.go b/pkg/github/issues.go index f98982f1e..e9cf64334 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1913,9 +1913,37 @@ Options are: }, "assignees": { Type: "array", - Description: "Usernames to assign to this issue", + Description: "Usernames to assign to this issue.", Items: &jsonschema.Schema{ - Type: "string", + OneOf: []*jsonschema.Schema{ + {Type: "string", Description: "GitHub username"}, + { + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "login": { + Type: "string", + Description: "GitHub username", + }, + "rationale": { + Type: "string", + Description: "One concise sentence explaining why this person is the right assignee. " + + "State the concrete signal (e.g. 'Owns the auth module where the bug was reported').", + MaxLength: jsonschema.Ptr(280), + }, + "confidence": { + Type: "string", + Description: "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.", + Enum: []any{"LOW", "MEDIUM", "HIGH"}, + }, + "is_suggestion": { + Type: "boolean", + Description: "If true, this assignee is sent to the API as a suggestion (suggest:true) rather than a direct assignment. " + + "Whether the assignee is applied or recorded as a proposal is determined by the API.", + }, + }, + Required: []string{"login"}, + }, + }, }, }, "labels": { @@ -2049,8 +2077,8 @@ Options are: return utils.NewToolResultError(err.Error()), nil, nil } - // Get assignees - assignees, err := OptionalStringArrayParam(args, "assignees") + // Get assignees (polymorphic: string or object with login/rationale/confidence/is_suggestion) + assignees, assigneesPayload, useAssigneeObjectForm, err := parsePolymorphicAssignees(args) if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } @@ -2129,16 +2157,45 @@ Options are: switch method { case "create": result, err := CreateIssue(ctx, client, owner, repo, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues) + if err != nil || result.IsError { + return result, nil, err + } + // If object-form assignees were used on create, apply them via a follow-up PATCH + if useAssigneeObjectForm { + textContent, ok := result.Content[0].(*mcp.TextContent) + if ok { + var created MinimalResponse + if jsonErr := json.Unmarshal([]byte(textContent.Text), &created); jsonErr == nil { + if issueNum, parseErr := parseIssueNumberFromURL(created.URL); parseErr == nil { + return patchAssigneesWithIntent(ctx, client, owner, repo, issueNum, assigneesPayload) + } + } + } + } return result, nil, err case "update": issueNumber, err := RequiredInt(args, "issue_number") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf, UpdateIssueOptions{ - AssigneesProvided: assigneesProvided, + // When object-form assignees are used, skip assignees in the standard + // UpdateIssue call and apply them via a separate PATCH with intent metadata. + updateAssignees := assignees + updateAssigneesProvided := assigneesProvided + if useAssigneeObjectForm { + updateAssignees = nil + updateAssigneesProvided = false + } + result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, updateAssignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf, UpdateIssueOptions{ + AssigneesProvided: updateAssigneesProvided, LabelsProvided: labelsProvided, }) + if err != nil || result.IsError { + return result, nil, err + } + if useAssigneeObjectForm { + return patchAssigneesWithIntent(ctx, client, owner, repo, issueNumber, assigneesPayload) + } return result, nil, err default: return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil @@ -2452,6 +2509,125 @@ type UpdateIssueOptions struct { LabelsProvided bool } +// assigneeWithIntent represents the object form of an assignee entry, allowing a +// rationale, confidence level, and/or suggest flag to be sent alongside the login. +type assigneeWithIntent struct { + Login string `json:"login"` + Rationale string `json:"rationale,omitempty"` + Confidence string `json:"confidence,omitempty"` + Suggest bool `json:"suggest,omitempty"` +} + +// assigneesUpdateRequest is a custom request body for updating an issue's assignees +// where individual assignees may optionally include a rationale. Each element of +// Assignees is either a string (login) or an assigneeWithIntent object. +type assigneesUpdateRequest struct { + Assignees []any `json:"assignees"` +} + +// parsePolymorphicAssignees parses the assignees parameter, which may be an array +// of strings or an array of objects with login, rationale, confidence, is_suggestion. +// Returns the plain login strings, the polymorphic payload, and whether object form is used. +func parsePolymorphicAssignees(args map[string]any) ([]string, []any, bool, error) { + assigneesRaw, ok := args["assignees"] + if !ok || assigneesRaw == nil { + return []string{}, nil, false, nil + } + assigneesSlice, ok := assigneesRaw.([]any) + if !ok { + if strs, ok := assigneesRaw.([]string); ok { + assigneesSlice = make([]any, len(strs)) + for i, s := range strs { + assigneesSlice[i] = s + } + } else { + return nil, nil, false, fmt.Errorf("parameter assignees must be an array") + } + } + + useObjectForm := false + logins := make([]string, 0, len(assigneesSlice)) + payload := make([]any, 0, len(assigneesSlice)) + for _, item := range assigneesSlice { + switch v := item.(type) { + case string: + logins = append(logins, v) + payload = append(payload, v) + case map[string]any: + login, err := RequiredParam[string](v, "login") + if err != nil { + return nil, nil, false, fmt.Errorf("each assignee object must have a 'login' string") + } + logins = append(logins, login) + rationale, err := OptionalParam[string](v, "rationale") + if err != nil { + return nil, nil, false, err + } + rationale = strings.TrimSpace(rationale) + if len([]rune(rationale)) > 280 { + return nil, nil, false, fmt.Errorf("assignee rationale must be 280 characters or less") + } + confidence, err := OptionalParam[string](v, "confidence") + if err != nil { + return nil, nil, false, err + } + confidence = normalizeConfidence(confidence) + if confidence != "" && confidence != "LOW" && confidence != "MEDIUM" && confidence != "HIGH" { + return nil, nil, false, fmt.Errorf("confidence must be one of: LOW, MEDIUM, HIGH") + } + isSuggestion, err := OptionalParam[bool](v, "is_suggestion") + if err != nil { + return nil, nil, false, err + } + if rationale == "" && !isSuggestion && confidence == "" { + payload = append(payload, login) + } else { + useObjectForm = true + payload = append(payload, assigneeWithIntent{Login: login, Rationale: rationale, Confidence: confidence, Suggest: isSuggestion}) + } + default: + return nil, nil, false, fmt.Errorf("each assignee must be a string or an object with 'login' and optional 'rationale', 'confidence', and/or 'is_suggestion'") + } + } + return logins, payload, useObjectForm, nil +} + +// patchAssigneesWithIntent sends a PATCH request with object-form assignees +// that include rationale, confidence, and/or suggest metadata. +func patchAssigneesWithIntent(ctx context.Context, client *github.Client, owner, repo string, issueNumber int, assigneesPayload []any) (*mcp.CallToolResult, any, error) { + body := &assigneesUpdateRequest{Assignees: assigneesPayload} + apiURL := fmt.Sprintf("repos/%s/%s/issues/%d", owner, repo, issueNumber) + req, err := client.NewRequest(ctx, "PATCH", apiURL, body) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil + } + + issue := &github.Issue{} + resp, err := client.Do(req, issue) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update issue assignees", resp, err), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", issue.GetID()), + URL: issue.GetHTMLURL(), + }) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil +} + +// parseIssueNumberFromURL extracts the issue number from a GitHub issue URL. +func parseIssueNumberFromURL(url string) (int, error) { + parts := strings.Split(url, "/") + if len(parts) == 0 { + return 0, fmt.Errorf("invalid issue URL: %s", url) + } + return strconv.Atoi(parts[len(parts)-1]) +} + func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue, fieldIDsToDelete []int64, state string, stateReason string, duplicateOf int, opts ...UpdateIssueOptions) (*mcp.CallToolResult, error) { updateOptions := UpdateIssueOptions{ AssigneesProvided: len(assignees) > 0, diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 5775daf37..4402b3234 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -3484,6 +3484,242 @@ func Test_LegacyUpdateIssueClearsLabelsAndAssignees(t *testing.T) { assert.Equal(t, updatedIssue.GetHTMLURL(), updateResp.URL) } +func Test_UpdateIssueWithAssigneeRationale(t *testing.T) { + serverTool := IssueWrite(translations.NullTranslationHelper) + + mockIssue := &github.Issue{ + Number: github.Ptr(42), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), + } + + tests := []struct { + name string + mockedRESTClient *http.Client + requestArgs map[string]any + expectError bool + expectedErrMsg string + }{ + { + name: "update with object-form assignees sends intent metadata", + mockedRESTClient: func() *http.Client { + callCount := 0 + return MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: func(w http.ResponseWriter, r *http.Request) { + callCount++ + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + if callCount == 1 { + // First call: UpdateIssue without assignees + require.NotContains(t, body, "assignees", "first PATCH should not contain assignees") + require.Equal(t, "Updated Title", body["title"]) + } else { + // Second call: patchAssigneesWithIntent + assignees, ok := body["assignees"].([]any) + require.True(t, ok, "second PATCH should contain assignees array") + require.Len(t, assignees, 2) + + a0, ok := assignees[0].(map[string]any) + require.True(t, ok) + assert.Equal(t, "octocat", a0["login"]) + assert.Equal(t, "Owns the auth module", a0["rationale"]) + assert.Equal(t, true, a0["suggest"]) + + a1, ok := assignees[1].(string) + require.True(t, ok) + assert.Equal(t, "user2", a1) + } + w.WriteHeader(http.StatusOK) + respBytes, _ := json.Marshal(mockIssue) + _, _ = w.Write(respBytes) + }, + }) + }(), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "title": "Updated Title", + "assignees": []any{ + map[string]any{ + "login": "octocat", + "rationale": "Owns the auth module", + "is_suggestion": true, + }, + "user2", + }, + }, + }, + { + name: "update with plain string assignees uses standard path", + mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "assignees": []any{"user1", "user2"}, + }).andThen( + mockResponse(t, http.StatusOK, mockIssue), + ), + }), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "assignees": []any{"user1", "user2"}, + }, + }, + { + name: "update with object-form assignees including confidence", + mockedRESTClient: func() *http.Client { + callCount := 0 + return MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: func(w http.ResponseWriter, r *http.Request) { + callCount++ + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + if callCount == 2 { + assignees, ok := body["assignees"].([]any) + require.True(t, ok) + require.Len(t, assignees, 1) + a0, ok := assignees[0].(map[string]any) + require.True(t, ok) + assert.Equal(t, "octocat", a0["login"]) + assert.Equal(t, "HIGH", a0["confidence"]) + assert.Equal(t, "Expert in this area", a0["rationale"]) + } + w.WriteHeader(http.StatusOK) + respBytes, _ := json.Marshal(mockIssue) + _, _ = w.Write(respBytes) + }, + }) + }(), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "assignees": []any{ + map[string]any{ + "login": "octocat", + "rationale": "Expert in this area", + "confidence": "HIGH", + }, + }, + }, + }, + { + name: "object-form assignee without login field fails", + mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "assignees": []any{ + map[string]any{ + "rationale": "Missing login", + }, + }, + }, + expectError: true, + expectedErrMsg: "each assignee object must have a 'login' string", + }, + { + name: "object-form assignee with invalid confidence fails", + mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "assignees": []any{ + map[string]any{ + "login": "octocat", + "confidence": "VERY_HIGH", + }, + }, + }, + expectError: true, + expectedErrMsg: "confidence must be one of: LOW, MEDIUM, HIGH", + }, + { + name: "object-form assignee with rationale exceeding 280 chars fails", + mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "assignees": []any{ + map[string]any{ + "login": "octocat", + "rationale": strings.Repeat("a", 281), + }, + }, + }, + expectError: true, + expectedErrMsg: "assignee rationale must be 280 characters or less", + }, + { + name: "object-form assignee with only login falls back to string form", + mockedRESTClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "assignees": []any{"octocat"}, + }).andThen( + mockResponse(t, http.StatusOK, mockIssue), + ), + }), + requestArgs: map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "assignees": []any{ + map[string]any{ + "login": "octocat", + }, + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + restClient := mustNewGHClient(t, tc.mockedRESTClient) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + deps := BaseDeps{ + Client: restClient, + GQLClient: gqlClient, + featureChecker: featureCheckerFor(FeatureFlagIssueFields), + } + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + if tc.expectError || tc.expectedErrMsg != "" { + require.NoError(t, err) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + if tc.expectedErrMsg != "" { + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + } + return + } + + require.NoError(t, err) + if result.IsError { + t.Fatalf("Unexpected error result: %s", getErrorResult(t, result).Text) + } + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + var resp MinimalResponse + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &resp)) + assert.Equal(t, "https://github.com/owner/repo/issues/42", resp.URL) + }) + } +} + func Test_ParseISOTimestamp(t *testing.T) { tests := []struct { name string