Skip to content

Commit b6486a3

Browse files
authored
Add specifying state change reason to update_issue tool (#1073)
* add state_reason param * add close as duplicate functionality * refactor and improve tests * fix state reason validation logic and update tests * move state and state reason handling to gql * fix marshal failures * address latest feedback
1 parent 0c5cfc3 commit b6486a3

File tree

5 files changed

+577
-93
lines changed

5 files changed

+577
-93
lines changed

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,12 +591,14 @@ The following sets of tools are available (all are on by default):
591591
- **update_issue** - Edit issue
592592
- `assignees`: New assignees (string[], optional)
593593
- `body`: New description (string, optional)
594+
- `duplicate_of`: Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'. (number, optional)
594595
- `issue_number`: Issue number to update (number, required)
595596
- `labels`: New labels (string[], optional)
596597
- `milestone`: New milestone number (number, optional)
597598
- `owner`: Repository owner (string, required)
598599
- `repo`: Repository name (string, required)
599600
- `state`: New state (string, optional)
601+
- `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
600602
- `title`: New title (string, optional)
601603
- `type`: New issue type (string, optional)
602604

pkg/github/__toolsnaps__/update_issue.snap

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@
1717
"description": "New description",
1818
"type": "string"
1919
},
20+
"duplicate_of": {
21+
"description": "Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'.",
22+
"type": "number"
23+
},
2024
"issue_number": {
2125
"description": "Issue number to update",
2226
"type": "number"
@@ -48,6 +52,15 @@
4852
],
4953
"type": "string"
5054
},
55+
"state_reason": {
56+
"description": "Reason for the state change. Ignored unless state is changed.",
57+
"enum": [
58+
"completed",
59+
"not_planned",
60+
"duplicate"
61+
],
62+
"type": "string"
63+
},
5164
"title": {
5265
"description": "New title",
5366
"type": "string"

pkg/github/issues.go

Lines changed: 188 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,87 @@ import (
1818
"github.com/shurcooL/githubv4"
1919
)
2020

21+
// CloseIssueInput represents the input for closing an issue via the GraphQL API.
22+
// Used to extend the functionality of the githubv4 library to support closing issues as duplicates.
23+
type CloseIssueInput struct {
24+
IssueID githubv4.ID `json:"issueId"`
25+
ClientMutationID *githubv4.String `json:"clientMutationId,omitempty"`
26+
StateReason *IssueClosedStateReason `json:"stateReason,omitempty"`
27+
DuplicateIssueID *githubv4.ID `json:"duplicateIssueId,omitempty"`
28+
}
29+
30+
// IssueClosedStateReason represents the reason an issue was closed.
31+
// Used to extend the functionality of the githubv4 library to support closing issues as duplicates.
32+
type IssueClosedStateReason string
33+
34+
const (
35+
IssueClosedStateReasonCompleted IssueClosedStateReason = "COMPLETED"
36+
IssueClosedStateReasonDuplicate IssueClosedStateReason = "DUPLICATE"
37+
IssueClosedStateReasonNotPlanned IssueClosedStateReason = "NOT_PLANNED"
38+
)
39+
40+
// fetchIssueIDs retrieves issue IDs via the GraphQL API.
41+
// When duplicateOf is 0, it fetches only the main issue ID.
42+
// When duplicateOf is non-zero, it fetches both the main issue and duplicate issue IDs in a single query.
43+
func fetchIssueIDs(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, issueNumber int, duplicateOf int) (githubv4.ID, githubv4.ID, error) {
44+
// Build query variables common to both cases
45+
vars := map[string]interface{}{
46+
"owner": githubv4.String(owner),
47+
"repo": githubv4.String(repo),
48+
"issueNumber": githubv4.Int(issueNumber), // #nosec G115 - issue numbers are always small positive integers
49+
}
50+
51+
if duplicateOf == 0 {
52+
// Only fetch the main issue ID
53+
var query struct {
54+
Repository struct {
55+
Issue struct {
56+
ID githubv4.ID
57+
} `graphql:"issue(number: $issueNumber)"`
58+
} `graphql:"repository(owner: $owner, name: $repo)"`
59+
}
60+
61+
if err := gqlClient.Query(ctx, &query, vars); err != nil {
62+
return "", "", fmt.Errorf("failed to get issue ID")
63+
}
64+
65+
return query.Repository.Issue.ID, "", nil
66+
}
67+
68+
// Fetch both issue IDs in a single query
69+
var query struct {
70+
Repository struct {
71+
Issue struct {
72+
ID githubv4.ID
73+
} `graphql:"issue(number: $issueNumber)"`
74+
DuplicateIssue struct {
75+
ID githubv4.ID
76+
} `graphql:"duplicateIssue: issue(number: $duplicateOf)"`
77+
} `graphql:"repository(owner: $owner, name: $repo)"`
78+
}
79+
80+
// Add duplicate issue number to variables
81+
vars["duplicateOf"] = githubv4.Int(duplicateOf) // #nosec G115 - issue numbers are always small positive integers
82+
83+
if err := gqlClient.Query(ctx, &query, vars); err != nil {
84+
return "", "", fmt.Errorf("failed to get issue ID")
85+
}
86+
87+
return query.Repository.Issue.ID, query.Repository.DuplicateIssue.ID, nil
88+
}
89+
90+
// getCloseStateReason converts a string state reason to the appropriate enum value
91+
func getCloseStateReason(stateReason string) IssueClosedStateReason {
92+
switch stateReason {
93+
case "not_planned":
94+
return IssueClosedStateReasonNotPlanned
95+
case "duplicate":
96+
return IssueClosedStateReasonDuplicate
97+
default: // Default to "completed" for empty or "completed" values
98+
return IssueClosedStateReasonCompleted
99+
}
100+
}
101+
21102
// IssueFragment represents a fragment of an issue node in the GraphQL API.
22103
type IssueFragment struct {
23104
Number githubv4.Int
@@ -1100,7 +1181,7 @@ func ListIssues(getGQLClient GetGQLClientFn, t translations.TranslationHelperFun
11001181
}
11011182

11021183
// UpdateIssue creates a tool to update an existing issue in a GitHub repository.
1103-
func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
1184+
func UpdateIssue(getClient GetClientFn, getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
11041185
return mcp.NewTool("update_issue",
11051186
mcp.WithDescription(t("TOOL_UPDATE_ISSUE_DESCRIPTION", "Update an existing issue in a GitHub repository.")),
11061187
mcp.WithToolAnnotation(mcp.ToolAnnotation{
@@ -1125,10 +1206,6 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
11251206
mcp.WithString("body",
11261207
mcp.Description("New description"),
11271208
),
1128-
mcp.WithString("state",
1129-
mcp.Description("New state"),
1130-
mcp.Enum("open", "closed"),
1131-
),
11321209
mcp.WithArray("labels",
11331210
mcp.Description("New labels"),
11341211
mcp.Items(
@@ -1151,6 +1228,17 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
11511228
mcp.WithString("type",
11521229
mcp.Description("New issue type"),
11531230
),
1231+
mcp.WithString("state",
1232+
mcp.Description("New state"),
1233+
mcp.Enum("open", "closed"),
1234+
),
1235+
mcp.WithString("state_reason",
1236+
mcp.Description("Reason for the state change. Ignored unless state is changed."),
1237+
mcp.Enum("completed", "not_planned", "duplicate"),
1238+
),
1239+
mcp.WithNumber("duplicate_of",
1240+
mcp.Description("Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'."),
1241+
),
11541242
),
11551243
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
11561244
owner, err := RequiredParam[string](request, "owner")
@@ -1186,14 +1274,6 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
11861274
issueRequest.Body = github.Ptr(body)
11871275
}
11881276

1189-
state, err := OptionalParam[string](request, "state")
1190-
if err != nil {
1191-
return mcp.NewToolResultError(err.Error()), nil
1192-
}
1193-
if state != "" {
1194-
issueRequest.State = github.Ptr(state)
1195-
}
1196-
11971277
// Get labels
11981278
labels, err := OptionalStringArrayParam(request, "labels")
11991279
if err != nil {
@@ -1230,13 +1310,38 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
12301310
issueRequest.Type = github.Ptr(issueType)
12311311
}
12321312

1313+
// Handle state, state_reason and duplicateOf parameters
1314+
state, err := OptionalParam[string](request, "state")
1315+
if err != nil {
1316+
return mcp.NewToolResultError(err.Error()), nil
1317+
}
1318+
1319+
stateReason, err := OptionalParam[string](request, "state_reason")
1320+
if err != nil {
1321+
return mcp.NewToolResultError(err.Error()), nil
1322+
}
1323+
1324+
duplicateOf, err := OptionalIntParam(request, "duplicate_of")
1325+
if err != nil {
1326+
return mcp.NewToolResultError(err.Error()), nil
1327+
}
1328+
if duplicateOf != 0 && stateReason != "duplicate" {
1329+
return mcp.NewToolResultError("duplicate_of can only be used when state_reason is 'duplicate'"), nil
1330+
}
1331+
1332+
// Use REST API for non-state updates
12331333
client, err := getClient(ctx)
12341334
if err != nil {
12351335
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
12361336
}
1337+
12371338
updatedIssue, resp, err := client.Issues.Edit(ctx, owner, repo, issueNumber, issueRequest)
12381339
if err != nil {
1239-
return nil, fmt.Errorf("failed to update issue: %w", err)
1340+
return ghErrors.NewGitHubAPIErrorResponse(ctx,
1341+
"failed to update issue",
1342+
resp,
1343+
err,
1344+
), nil
12401345
}
12411346
defer func() { _ = resp.Body.Close() }()
12421347

@@ -1248,6 +1353,75 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
12481353
return mcp.NewToolResultError(fmt.Sprintf("failed to update issue: %s", string(body))), nil
12491354
}
12501355

1356+
// Use GraphQL API for state updates
1357+
if state != "" {
1358+
gqlClient, err := getGQLClient(ctx)
1359+
if err != nil {
1360+
return nil, fmt.Errorf("failed to get GraphQL client: %w", err)
1361+
}
1362+
1363+
// Mandate specifying duplicateOf when trying to close as duplicate
1364+
if state == "closed" && stateReason == "duplicate" && duplicateOf == 0 {
1365+
return mcp.NewToolResultError("duplicate_of must be provided when state_reason is 'duplicate'"), nil
1366+
}
1367+
1368+
// Get target issue ID (and duplicate issue ID if needed)
1369+
issueID, duplicateIssueID, err := fetchIssueIDs(ctx, gqlClient, owner, repo, issueNumber, duplicateOf)
1370+
if err != nil {
1371+
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to find issues", err), nil
1372+
}
1373+
1374+
switch state {
1375+
case "open":
1376+
// Use ReopenIssue mutation for opening
1377+
var mutation struct {
1378+
ReopenIssue struct {
1379+
Issue struct {
1380+
ID githubv4.ID
1381+
Number githubv4.Int
1382+
URL githubv4.String
1383+
State githubv4.String
1384+
}
1385+
} `graphql:"reopenIssue(input: $input)"`
1386+
}
1387+
1388+
err = gqlClient.Mutate(ctx, &mutation, githubv4.ReopenIssueInput{
1389+
IssueID: issueID,
1390+
}, nil)
1391+
if err != nil {
1392+
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to reopen issue", err), nil
1393+
}
1394+
case "closed":
1395+
// Use CloseIssue mutation for closing
1396+
var mutation struct {
1397+
CloseIssue struct {
1398+
Issue struct {
1399+
ID githubv4.ID
1400+
Number githubv4.Int
1401+
URL githubv4.String
1402+
State githubv4.String
1403+
}
1404+
} `graphql:"closeIssue(input: $input)"`
1405+
}
1406+
1407+
stateReasonValue := getCloseStateReason(stateReason)
1408+
closeInput := CloseIssueInput{
1409+
IssueID: issueID,
1410+
StateReason: &stateReasonValue,
1411+
}
1412+
1413+
// Set duplicate issue ID if needed
1414+
if stateReason == "duplicate" {
1415+
closeInput.DuplicateIssueID = &duplicateIssueID
1416+
}
1417+
1418+
err = gqlClient.Mutate(ctx, &mutation, closeInput, nil)
1419+
if err != nil {
1420+
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to close issue", err), nil
1421+
}
1422+
}
1423+
}
1424+
12511425
// Return minimal response with just essential information
12521426
minimalResponse := MinimalResponse{
12531427
ID: fmt.Sprintf("%d", updatedIssue.GetID()),

0 commit comments

Comments
 (0)