Skip to content

Add rationale and confidence to closing an issue #2802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
iulia-b wants to merge 1 commit into
base: main
Choose a base branch
from
+436 −22
Draft

Add rationale and confidence to closing an issue #2802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/feature-flags.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
25 changes: 24 additions & 1 deletion pkg/github/__toolsnaps__/update_issue_state.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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"
Expand Down
227 changes: 227 additions & 0 deletions pkg/github/granular_tools_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading
Loading