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