From 0ba9010f7c82574910def746ec6366049667a8f5 Mon Sep 17 00:00:00 2001 From: Alon Dahari Date: Wed, 17 Jun 2026 17:47:16 +0100 Subject: [PATCH] Add rationale and is_suggestion to granular assignee tool Update GranularUpdateIssueAssignees to accept polymorphic input where each assignee can be a plain string (login) or an object with login, rationale, confidence, and is_suggestion fields. This follows the same pattern already used by GranularUpdateIssueLabels. When any assignee includes rationale, confidence, or is_suggestion, the tool builds a custom request body with object-form assignees. Otherwise, it preserves the standard wire format with plain string arrays for backward compatibility. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/feature-flags.md | 4 +- docs/insiders-features.md | 2 +- .../__toolsnaps__/update_issue_assignees.snap | 41 ++- pkg/github/granular_tools_test.go | 324 +++++++++++++++++- pkg/github/issues_granular.go | 206 ++++++++++- 5 files changed, 538 insertions(+), 39 deletions(-) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 590cb65975..ee60476cfb 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) @@ -179,7 +179,7 @@ runtime behavior (such as output formatting) won't appear here. - **update_issue_assignees** - Update Issue Assignees - **Required OAuth Scopes**: `repo` - - `assignees`: GitHub usernames to assign to this issue (string[], required) + - `assignees`: GitHub usernames to assign to this issue. ([], required) - `issue_number`: The issue number to update (number, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 3306b5cd85..69978d8c19 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) diff --git a/pkg/github/__toolsnaps__/update_issue_assignees.snap b/pkg/github/__toolsnaps__/update_issue_assignees.snap index 9c7261c9aa..80ea1bc7e9 100644 --- a/pkg/github/__toolsnaps__/update_issue_assignees.snap +++ b/pkg/github/__toolsnaps__/update_issue_assignees.snap @@ -4,13 +4,48 @@ "openWorldHint": true, "title": "Update Issue Assignees" }, - "description": "Update the assignees of an existing issue. This replaces the current assignees with the provided list.", + "description": "Update the assignees of an existing issue. This replaces the current assignees with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.", "inputSchema": { "properties": { "assignees": { - "description": "GitHub usernames to assign to this issue", + "description": "GitHub 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 an applied 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 what specifically about the issue led you to choose this assignee. State the concrete signal (e.g. 'Author of the component mentioned in the bug report').", + "maxLength": 280, + "type": "string" + } + }, + "required": [ + "login" + ], + "type": "object" + } + ] }, "type": "array" }, diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 4a274ac318..afbe3fb4d0 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -244,24 +244,314 @@ func TestGranularUpdateIssueBody(t *testing.T) { } func TestGranularUpdateIssueAssignees(t *testing.T) { - client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ - "assignees": []any{"user1", "user2"}, - }).andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), - })) - deps := BaseDeps{Client: client} - serverTool := GranularUpdateIssueAssignees(translations.NullTranslationHelper) - handler := serverTool.Handler(deps) + tests := []struct { + name string + requestArgs map[string]any + expectedReq map[string]any + }{ + { + name: "assignees as plain strings", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{"user1", "user2"}, + }, + expectedReq: map[string]any{ + "assignees": []any{"user1", "user2"}, + }, + }, + { + name: "assignee objects without rationale serialize as strings", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"login": "user1"}, + "user2", + }, + }, + expectedReq: map[string]any{ + "assignees": []any{"user1", "user2"}, + }, + }, + { + name: "mixed strings and assignee objects with rationale", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + "user1", + map[string]any{"login": "user2", "rationale": " Author of the component "}, + map[string]any{"login": "user3", "rationale": "Reviewer for this area"}, + }, + }, + expectedReq: map[string]any{ + "assignees": []any{ + "user1", + map[string]any{"login": "user2", "rationale": "Author of the component"}, + map[string]any{"login": "user3", "rationale": "Reviewer for this area"}, + }, + }, + }, + { + name: "[]string typed array still works", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []string{"user1", "user2"}, + }, + expectedReq: map[string]any{ + "assignees": []any{"user1", "user2"}, + }, + }, + } - request := createMCPRequest(map[string]any{ - "owner": "owner", - "repo": "repo", - "issue_number": float64(1), - "assignees": []string{"user1", "user2"}, - }) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - assert.False(t, result.IsError) + 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 := GranularUpdateIssueAssignees(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 TestGranularUpdateIssueAssigneesSuggest(t *testing.T) { + tests := []struct { + name string + requestArgs map[string]any + expectedReq map[string]any + }{ + { + name: "single assignee suggested without rationale", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"login": "user1", "is_suggestion": true}, + }, + }, + expectedReq: map[string]any{ + "assignees": []any{ + map[string]any{"login": "user1", "suggest": true}, + }, + }, + }, + { + name: "suggested assignee with rationale", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"login": "user1", "rationale": "Author of the component", "is_suggestion": true}, + }, + }, + expectedReq: map[string]any{ + "assignees": []any{ + map[string]any{"login": "user1", "rationale": "Author of the component", "suggest": true}, + }, + }, + }, + { + name: "mix of plain, applied-with-rationale, and suggested assignees", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + "user1", + map[string]any{"login": "user2", "rationale": "Author of the component"}, + map[string]any{"login": "user3", "is_suggestion": true}, + }, + }, + expectedReq: map[string]any{ + "assignees": []any{ + "user1", + map[string]any{"login": "user2", "rationale": "Author of the component"}, + map[string]any{"login": "user3", "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 := GranularUpdateIssueAssignees(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 TestGranularUpdateIssueAssigneesInvalidRationale(t *testing.T) { + tests := []struct { + name string + requestArgs map[string]any + expectedErrText string + }{ + { + name: "rationale too long", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"login": "user1", "rationale": strings.Repeat("a", 281)}, + }, + }, + expectedErrText: "assignee rationale must be 280 characters or less", + }, + { + name: "assignee object missing login", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"rationale": "no login provided"}, + }, + }, + expectedErrText: "each assignee object must have a 'login' string", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} + serverTool := GranularUpdateIssueAssignees(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrText) + }) + } +} + +func TestGranularUpdateIssueAssigneesConfidence(t *testing.T) { + tests := []struct { + name string + requestArgs map[string]any + expectedReq map[string]any + }{ + { + name: "assignee with confidence triggers object form", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"login": "user1", "confidence": "HIGH"}, + }, + }, + expectedReq: map[string]any{ + "assignees": []any{ + map[string]any{"login": "user1", "confidence": "HIGH"}, + }, + }, + }, + { + name: "assignee with confidence and rationale", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"login": "user1", "rationale": "Component author", "confidence": "MEDIUM"}, + }, + }, + expectedReq: map[string]any{ + "assignees": []any{ + map[string]any{"login": "user1", "rationale": "Component author", "confidence": "MEDIUM"}, + }, + }, + }, + { + name: "assignee confidence is normalized", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"login": "user1", "confidence": " high\t"}, + }, + }, + expectedReq: map[string]any{ + "assignees": []any{ + map[string]any{"login": "user1", "confidence": "HIGH"}, + }, + }, + }, + { + name: "invalid confidence value", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"login": "user1", "confidence": "very_high"}, + }, + }, + expectedReq: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if tc.expectedReq == nil { + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} + serverTool := GranularUpdateIssueAssignees(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, "confidence must be one of: LOW, MEDIUM, HIGH") + return + } + + 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 := GranularUpdateIssueAssignees(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 TestGranularUpdateIssueLabels(t *testing.T) { diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 157d5595fd..418a278360 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -236,31 +236,205 @@ func GranularUpdateIssueBody(t translations.TranslationHelperFunc) inventory.Ser ) } +// GranularUpdateIssueAssignees creates a tool to update an issue's assignees. +// 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"` +} + // GranularUpdateIssueAssignees creates a tool to update an issue's assignees. func GranularUpdateIssueAssignees(t translations.TranslationHelperFunc) inventory.ServerTool { - return issueUpdateTool(t, - "update_issue_assignees", - "Update the assignees of an existing issue. This replaces the current assignees with the provided list.", - "Update Issue Assignees", - map[string]*jsonschema.Schema{ - "assignees": { - Type: "array", - Description: "GitHub usernames to assign to this issue", - Items: &jsonschema.Schema{Type: "string"}, + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "update_issue_assignees", + Description: t("TOOL_UPDATE_ISSUE_ASSIGNEES_DESCRIPTION", "Update the assignees of an existing issue. This replaces the current assignees with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_UPDATE_ISSUE_ASSIGNEES_USER_TITLE", "Update Issue Assignees"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "issue_number": { + Type: "number", + Description: "The issue number to update", + Minimum: jsonschema.Ptr(1.0), + }, + "assignees": { + Type: "array", + Description: "GitHub usernames to assign to this issue.", + Items: &jsonschema.Schema{ + 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 what specifically about the issue led you to choose this assignee. " + + "State the concrete signal (e.g. 'Author of the component mentioned in the bug report').", + 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 an applied assignment. " + + "Whether the assignee is applied or recorded as a proposal is determined by the API.", + }, + }, + Required: []string{"login"}, + }, + }, + }, + }, + }, + Required: []string{"owner", "repo", "issue_number", "assignees"}, }, }, - []string{"assignees"}, - func(args map[string]any) (*github.IssueRequest, error) { - if _, ok := args["assignees"]; !ok { - return nil, fmt.Errorf("missing required parameter: assignees") + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil } - assignees, err := OptionalStringArrayParam(args, "assignees") + repo, err := RequiredParam[string](args, "repo") if err != nil { - return nil, err + return utils.NewToolResultError(err.Error()), nil, nil + } + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + assigneesRaw, ok := args["assignees"] + if !ok { + return utils.NewToolResultError("missing required parameter: assignees"), nil, 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 utils.NewToolResultError("parameter assignees must be an array"), nil, nil + } + } + + useObjectForm := false + payload := make([]any, 0, len(assigneesSlice)) + for _, item := range assigneesSlice { + switch v := item.(type) { + case string: + payload = append(payload, v) + case map[string]any: + login, err := RequiredParam[string](v, "login") + if err != nil { + return utils.NewToolResultError("each assignee object must have a 'login' string"), nil, nil + } + rationale, err := OptionalParam[string](v, "rationale") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + rationale = strings.TrimSpace(rationale) + if len([]rune(rationale)) > 280 { + return utils.NewToolResultError("assignee rationale must be 280 characters or less"), nil, nil + } + confidence, err := OptionalParam[string](v, "confidence") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + confidence = normalizeConfidence(confidence) + if confidence != "" && confidence != "LOW" && confidence != "MEDIUM" && confidence != "HIGH" { + return utils.NewToolResultError("confidence must be one of: LOW, MEDIUM, HIGH"), nil, nil + } + isSuggestion, err := OptionalParam[bool](v, "is_suggestion") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + 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 utils.NewToolResultError("each assignee must be a string or an object with 'login' and optional 'rationale', 'confidence', and/or 'is_suggestion'"), nil, nil + } } - return &github.IssueRequest{Assignees: &assignees}, nil + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + var body any + if useObjectForm { + body = &assigneesUpdateRequest{Assignees: payload} + } else { + names := make([]string, len(payload)) + for i, p := range payload { + names[i] = p.(string) + } + body = &github.IssueRequest{Assignees: &names} + } + + 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", 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 }, ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular + return st } // labelWithIntent represents the object form of a label entry, allowing a