From 8b1c659bc36969bcd9a0e6c9d0d6e331d8e12de6 Mon Sep 17 00:00:00 2001 From: Kelsey Myers <52179263+kelsey-myers@users.noreply.github.com> Date: Mon, 6 Jul 2026 02:01:19 -0700 Subject: [PATCH 1/2] Add is_suggestion + rationale to update_issue_assignees --- docs/feature-flags.md | 2 +- .../__toolsnaps__/update_issue_assignees.snap | 41 +++- pkg/github/granular_tools_test.go | 151 +++++++++++++ pkg/github/issues_granular.go | 206 ++++++++++++++++-- 4 files changed, 380 insertions(+), 20 deletions(-) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index ce3c32e35f..5da87310c6 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -150,7 +150,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/pkg/github/__toolsnaps__/update_issue_assignees.snap b/pkg/github/__toolsnaps__/update_issue_assignees.snap index ab23f72a95..cd460122da 100644 --- a/pkg/github/__toolsnaps__/update_issue_assignees.snap +++ b/pkg/github/__toolsnaps__/update_issue_assignees.snap @@ -6,13 +6,48 @@ "readOnlyHint": false, "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 assignee. 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. 'Authored the file the crash originates in').", + "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 e302435ce5..6b884c06c5 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -271,6 +271,157 @@ func TestGranularUpdateIssueAssignees(t *testing.T) { assert.False(t, result.IsError) } +func TestGranularUpdateIssueAssigneesObjectForm(t *testing.T) { + tests := []struct { + name string + requestArgs map[string]any + expectedReq map[string]any + }{ + { + name: "assignee objects without intent serialize as strings", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"login": "octocat"}, + "monalisa", + }, + }, + expectedReq: map[string]any{ + "assignees": []any{"octocat", "monalisa"}, + }, + }, + { + name: "assignee suggested without rationale", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"login": "octocat", "is_suggestion": true}, + }, + }, + expectedReq: map[string]any{ + "assignees": []any{ + map[string]any{"login": "octocat", "suggest": true}, + }, + }, + }, + { + name: "suggested assignee with rationale and confidence", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"login": "octocat", "rationale": " Authored the crashing file ", "confidence": "high", "is_suggestion": true}, + }, + }, + expectedReq: map[string]any{ + "assignees": []any{ + map[string]any{"login": "octocat", "rationale": "Authored the crashing file", "confidence": "high", "suggest": true}, + }, + }, + }, + { + name: "mix of plain and suggested assignees", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + "monalisa", + map[string]any{"login": "octocat", "is_suggestion": true}, + }, + }, + expectedReq: map[string]any{ + "assignees": []any{ + "monalisa", + map[string]any{"login": "octocat", "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 TestGranularUpdateIssueAssigneesInvalidInput(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": "octocat", "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", + }, + { + name: "invalid confidence value", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{ + map[string]any{"login": "octocat", "confidence": "maybe"}, + }, + }, + expectedErrText: "confidence must be one of: low, medium, high", + }, + } + + 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 TestGranularUpdateIssueLabels(t *testing.T) { tests := []struct { name string diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index e965ce9458..844d29e7e2 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -238,29 +238,187 @@ func GranularUpdateIssueBody(t translations.TranslationHelperFunc) inventory.Ser // 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. 'Authored the file the crash originates in').", + 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 assignee. " + + "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 { + // Also accept []string for callers that pre-typed the array. + 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 + } + 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 + } + } + + 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 { + // Preserve the standard wire format when no rationale or suggest is supplied. + logins := make([]string, len(payload)) + for i, p := range payload { + logins[i] = p.(string) + } + body = &github.IssueRequest{Assignees: &logins} } - return &github.IssueRequest{Assignees: &assignees}, nil + + 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 @@ -279,6 +437,22 @@ type labelsUpdateRequest struct { Labels []any `json:"labels"` } +// 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"` +} + // GranularUpdateIssueLabels creates a tool to update an issue's labels. func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( From 5130b8c5b0c9ac2110260b7ddbf5baf9fa5462c1 Mon Sep 17 00:00:00 2001 From: Kelsey Myers <52179263+kelsey-myers@users.noreply.github.com> Date: Mon, 6 Jul 2026 02:36:40 -0700 Subject: [PATCH 2/2] Align assignee confidence to uppercase LOW/MEDIUM/HIGH; add non-string/object test --- .../__toolsnaps__/update_issue_assignees.snap | 10 +++++----- pkg/github/granular_tools_test.go | 14 ++++++++++++-- pkg/github/issues_granular.go | 11 ++++++----- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/pkg/github/__toolsnaps__/update_issue_assignees.snap b/pkg/github/__toolsnaps__/update_issue_assignees.snap index cd460122da..51a407c164 100644 --- a/pkg/github/__toolsnaps__/update_issue_assignees.snap +++ b/pkg/github/__toolsnaps__/update_issue_assignees.snap @@ -6,7 +6,7 @@ "readOnlyHint": false, "title": "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.", + "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": { @@ -20,11 +20,11 @@ { "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.", + "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" + "LOW", + "MEDIUM", + "HIGH" ], "type": "string" }, diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 6b884c06c5..6b4ab2346b 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -320,7 +320,7 @@ func TestGranularUpdateIssueAssigneesObjectForm(t *testing.T) { }, expectedReq: map[string]any{ "assignees": []any{ - map[string]any{"login": "octocat", "rationale": "Authored the crashing file", "confidence": "high", "suggest": true}, + map[string]any{"login": "octocat", "rationale": "Authored the crashing file", "confidence": "HIGH", "suggest": true}, }, }, }, @@ -402,7 +402,17 @@ func TestGranularUpdateIssueAssigneesInvalidInput(t *testing.T) { map[string]any{"login": "octocat", "confidence": "maybe"}, }, }, - expectedErrText: "confidence must be one of: low, medium, high", + expectedErrText: "confidence must be one of: LOW, MEDIUM, HIGH", + }, + { + name: "assignee entry is neither string nor object", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []any{float64(123)}, + }, + expectedErrText: "each assignee must be a string or an object", }, } diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 844d29e7e2..455122850b 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -242,7 +242,7 @@ func GranularUpdateIssueAssignees(t translations.TranslationHelperFunc) inventor 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."), + 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, @@ -286,8 +286,8 @@ func GranularUpdateIssueAssignees(t translations.TranslationHelperFunc) inventor }, "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"}, + 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", @@ -359,8 +359,9 @@ func GranularUpdateIssueAssignees(t translations.TranslationHelperFunc) inventor if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - if confidence != "" && confidence != "low" && confidence != "medium" && confidence != "high" { - return utils.NewToolResultError("confidence must be one of: low, medium, high"), 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 {