From 8042166e7ef8ba3f5ad16e4a06047dcc69762885 Mon Sep 17 00:00:00 2001 From: Iulia B Date: Wed, 1 Jul 2026 07:56:55 +0000 Subject: [PATCH] Add rationale and confidence to closing an issue --- docs/feature-flags.md | 4 + .../__toolsnaps__/update_issue_state.snap | 25 +- pkg/github/granular_tools_test.go | 227 ++++++++++++++++++ pkg/github/issues_granular.go | 202 ++++++++++++++-- 4 files changed, 436 insertions(+), 22 deletions(-) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index ce3c32e35f..ad6f16d818 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -178,8 +178,12 @@ runtime behavior (such as output formatting) won't appear here. - **update_issue_state** - Update Issue State - **Required OAuth Scopes**: `repo` + - `confidence`: 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. (string, optional) + - `duplicate_of`: The issue number this issue is a duplicate of. Only valid when state_reason is 'duplicate' and is_suggestion is true. (number, optional) + - `is_suggestion`: If true, this state change is sent to the API as a suggestion (suggest:true) rather than an applied change. Whether the change is applied or recorded as a proposal is determined by the API. (boolean, optional) - `issue_number`: The issue number to update (number, required) - `owner`: Repository owner (username or organization) (string, required) + - `rationale`: One concise sentence explaining what specifically about the issue led you to choose this state. State the concrete signal (e.g. 'The reported crash is fixed in v2.1' → completed). (string, optional) - `repo`: Repository name (string, required) - `state`: The new state for the issue (string, required) - `state_reason`: The reason for the state change (only for closed state) (string, optional) diff --git a/pkg/github/__toolsnaps__/update_issue_state.snap b/pkg/github/__toolsnaps__/update_issue_state.snap index b14d737b7d..a395d93706 100644 --- a/pkg/github/__toolsnaps__/update_issue_state.snap +++ b/pkg/github/__toolsnaps__/update_issue_state.snap @@ -4,9 +4,27 @@ "openWorldHint": true, "title": "Update Issue State" }, - "description": "Update the state of an existing issue (open or closed), with an optional state reason.", + "description": "Update the state of an existing issue (open or closed), with an optional state reason. When closing, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the decision. Use is_suggestion to propose the change without applying it directly.", "inputSchema": { "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" + }, + "duplicate_of": { + "description": "The issue number of the canonical issue this issue duplicates. Only valid when state_reason is 'duplicate'. Required when is_suggestion is true. The issue number is resolved to a database ID before being sent to the API.", + "minimum": 1, + "type": "number" + }, + "is_suggestion": { + "description": "If true, this state change is sent to the API as a suggestion (suggest:true) rather than an applied change. Whether the change is applied or recorded as a proposal is determined by the API.", + "type": "boolean" + }, "issue_number": { "description": "The issue number to update", "minimum": 1, @@ -16,6 +34,11 @@ "description": "Repository owner (username or organization)", "type": "string" }, + "rationale": { + "description": "One concise sentence explaining what specifically about the issue led you to choose this state. State the concrete signal (e.g. 'The reported crash is fixed in v2.1' → completed).", + "maxLength": 280, + "type": "string" + }, "repo": { "description": "Repository name", "type": "string" diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index e302435ce5..cd5c9a0b0a 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -945,6 +945,233 @@ func TestGranularUpdateIssueState(t *testing.T) { } } +func TestGranularUpdateIssueStateSuggest(t *testing.T) { + tests := []struct { + name string + requestArgs map[string]any + expectedReq map[string]any + }{ + { + name: "suggest without rationale", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "closed", + "is_suggestion": true, + }, + expectedReq: map[string]any{ + "state": map[string]any{ + "value": "closed", + "suggest": true, + }, + }, + }, + { + name: "suggest with rationale and state_reason", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "closed", + "state_reason": "not_planned", + "rationale": " No activity in 6 months ", + "is_suggestion": true, + }, + expectedReq: map[string]any{ + "state": map[string]any{ + "value": "closed", + "rationale": "No activity in 6 months", + "suggest": true, + }, + "state_reason": "not_planned", + }, + }, + { + name: "rationale applied directly (no suggestion)", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "closed", + "rationale": "The reported crash is fixed in v2.1", + "confidence": "HIGH", + }, + expectedReq: map[string]any{ + "state": map[string]any{ + "value": "closed", + "rationale": "The reported crash is fixed in v2.1", + "confidence": "HIGH", + }, + }, + }, + } + + 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 := GranularUpdateIssueState(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 TestGranularUpdateIssueStateDuplicate(t *testing.T) { + const duplicateIssueID = int64(99999) + + tests := []struct { + name string + requestArgs map[string]any + expectedReq map[string]any + }{ + { + name: "suggestion duplicate close", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "closed", + "state_reason": "duplicate", + "is_suggestion": true, + "duplicate_of": float64(42), + }, + expectedReq: map[string]any{ + "state": map[string]any{ + "value": "closed", + "suggest": true, + }, + "state_reason": "duplicate", + "duplicate_issue_id": float64(duplicateIssueID), + }, + }, + { + name: "direct duplicate close", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "closed", + "state_reason": "duplicate", + "duplicate_of": float64(42), + }, + expectedReq: map[string]any{ + "state": map[string]any{"value": "closed"}, + "state_reason": "duplicate", + "duplicate_issue_id": float64(duplicateIssueID), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{ + ID: gogithub.Ptr(duplicateIssueID), + Number: gogithub.Ptr(42), + }), + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). + andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueState(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 TestGranularUpdateIssueStateInvalidRationale(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), + "state": "closed", + "rationale": strings.Repeat("a", 281), + }, + expectedErrText: "parameter rationale must be 280 characters or less", + }, + { + name: "duplicate_of without state_reason duplicate", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "closed", + "state_reason": "not_planned", + "is_suggestion": true, + "duplicate_of": float64(42), + }, + expectedErrText: "duplicate_of can only be used when state_reason is 'duplicate'", + }, + { + name: "suggestion duplicate without duplicate_of", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "closed", + "state_reason": "duplicate", + "is_suggestion": true, + }, + expectedErrText: "duplicate_of is required when suggesting a close as duplicate", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} + serverTool := GranularUpdateIssueState(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 TestGranularUpdateIssueStateInvalidConfidence(t *testing.T) { + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))} + serverTool := GranularUpdateIssueState(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "closed", + "confidence": "VERY_HIGH", + }) + 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") +} + // --- Pull request granular tool handler tests --- func TestGranularUpdatePullRequestTitle(t *testing.T) { diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index e965ce9458..d117efa340 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -642,39 +642,199 @@ func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.Ser return st } +// stateWithIntent represents the object form of the state field, allowing +// rationale, confidence, and/or suggest flag to be sent alongside the state value. +type stateWithIntent struct { + Value string `json:"value"` + Rationale string `json:"rationale,omitempty"` + Confidence string `json:"confidence,omitempty"` + Suggest bool `json:"suggest,omitempty"` +} + +// stateUpdateRequest is a custom request body for updating an issue's state +// with optional intent metadata, using the object form that the REST API accepts. +type stateUpdateRequest struct { + State stateWithIntent `json:"state"` + StateReason string `json:"state_reason,omitempty"` + DuplicateIssueID *int64 `json:"duplicate_issue_id,omitempty"` +} + // GranularUpdateIssueState creates a tool to update an issue's state. func GranularUpdateIssueState(t translations.TranslationHelperFunc) inventory.ServerTool { - return issueUpdateTool(t, - "update_issue_state", - "Update the state of an existing issue (open or closed), with an optional state reason.", - "Update Issue State", - map[string]*jsonschema.Schema{ - "state": { - Type: "string", - Description: "The new state for the issue", - Enum: []any{"open", "closed"}, + st := NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "update_issue_state", + Description: t("TOOL_UPDATE_ISSUE_STATE_DESCRIPTION", "Update the state of an existing issue (open or closed), with an optional state reason. When closing, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the decision. Use is_suggestion to propose the change without applying it directly."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_UPDATE_ISSUE_STATE_USER_TITLE", "Update Issue State"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), }, - "state_reason": { - Type: "string", - Description: "The reason for the state change (only for closed state)", - Enum: []any{"completed", "not_planned", "duplicate"}, + 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), + }, + "state": { + Type: "string", + Description: "The new state for the issue", + Enum: []any{"open", "closed"}, + }, + "state_reason": { + Type: "string", + Description: "The reason for the state change (only for closed state)", + Enum: []any{"completed", "not_planned", "duplicate"}, + }, + "rationale": { + Type: "string", + Description: "One concise sentence explaining what specifically about the issue led you to choose this state. " + + "State the concrete signal (e.g. 'The reported crash is fixed in v2.1' → completed).", + 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 state change is sent to the API as a suggestion (suggest:true) rather than an applied change. " + + "Whether the change is applied or recorded as a proposal is determined by the API.", + }, + "duplicate_of": { + Type: "number", + Description: "The issue number of the canonical issue this issue duplicates. Only valid when state_reason is 'duplicate'. Required when is_suggestion is true. The issue number is resolved to a database ID before being sent to the API.", + Minimum: jsonschema.Ptr(1.0), + }, + }, + Required: []string{"owner", "repo", "issue_number", "state"}, }, }, - []string{"state"}, - func(args map[string]any) (*github.IssueRequest, error) { + []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 + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } state, err := RequiredParam[string](args, "state") if err != nil { - return nil, err + return utils.NewToolResultError(err.Error()), nil, nil + } + stateReason, err := OptionalParam[string](args, "state_reason") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + rationale, err := OptionalParam[string](args, "rationale") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + rationale = strings.TrimSpace(rationale) + if len([]rune(rationale)) > 280 { + return utils.NewToolResultError("parameter rationale must be 280 characters or less"), nil, nil + } + confidence, err := OptionalParam[string](args, "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](args, "is_suggestion") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + duplicateOf, err := OptionalIntParam(args, "duplicate_of") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if duplicateOf != 0 && stateReason != "duplicate" { + return utils.NewToolResultError("duplicate_of can only be used when state_reason is 'duplicate'"), nil, nil + } + if isSuggestion && stateReason == "duplicate" && duplicateOf == 0 { + return utils.NewToolResultError("duplicate_of is required when suggesting a close as duplicate"), nil, nil } - req := &github.IssueRequest{State: &state} - stateReason, _ := OptionalParam[string](args, "state_reason") - if stateReason != "" { - req.StateReason = &stateReason + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - return req, nil + + var body any + if rationale != "" || isSuggestion || confidence != "" || duplicateOf != 0 { + req := &stateUpdateRequest{ + State: stateWithIntent{ + Value: state, + Rationale: rationale, + Confidence: confidence, + Suggest: isSuggestion, + }, + StateReason: stateReason, + } + if duplicateOf != 0 { + duplicateIssue, resp, err := client.Issues.Get(ctx, owner, repo, duplicateOf) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get duplicate issue", resp, err), nil, nil + } + _ = resp.Body.Close() + id := duplicateIssue.GetID() + req.DuplicateIssueID = &id + } + body = req + } else { + req := &github.IssueRequest{State: &state} + if stateReason != "" { + req.StateReason = &stateReason + } + body = req + } + + 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 } // GranularAddSubIssue creates a tool to add a sub-issue.