Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -591,12 +591,14 @@ The following sets of tools are available (all are on by default):
- **update_issue** - Edit issue
- `assignees`: New assignees (string[], optional)
- `body`: New description (string, optional)
- `duplicate_of`: Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'. (number, optional)
- `issue_number`: Issue number to update (number, required)
- `labels`: New labels (string[], optional)
- `milestone`: New milestone number (number, optional)
- `owner`: Repository owner (string, required)
- `repo`: Repository name (string, required)
- `state`: New state (string, optional)
- `state_reason`: Reason for the state change. Ignored unless state is changed. (string, optional)
- `title`: New title (string, optional)
- `type`: New issue type (string, optional)

Expand Down
14 changes: 14 additions & 0 deletions pkg/github/__toolsnaps__/update_issue.snap
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
"description": "New description",
"type": "string"
},
"duplicate_of": {
"description": "Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'.",
"type": "number"
},
"issue_number": {
"description": "Issue number to update",
"type": "number"
Expand Down Expand Up @@ -48,6 +52,16 @@
],
"type": "string"
},
"state_reason": {
"description": "Reason for the state change. Ignored unless state is changed.",
"enum": [
"completed",
"not_planned",
"duplicate",
"reopened"
],
"type": "string"
},
"title": {
"description": "New title",
"type": "string"
Expand Down
176 changes: 161 additions & 15 deletions pkg/github/issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,25 @@ import (
"github.com/shurcooL/githubv4"
)

// CloseIssueInput represents the input for closing an issue via the GraphQL API.
// Used to extend the functionality of the githubv4 library to support closing issues as duplicates.
type CloseIssueInput struct {
IssueID githubv4.ID `json:"issueId"`
ClientMutationID *githubv4.String `json:"clientMutationId,omitempty"`
StateReason *IssueClosedStateReason `json:"stateReason,omitempty"`
DuplicateIssueID *githubv4.ID `json:"duplicateIssueId,omitempty"`
}

// IssueClosedStateReason represents the reason an issue was closed.
// Used to extend the functionality of the githubv4 library to support closing issues as duplicates.
type IssueClosedStateReason string

const (
IssueClosedStateReasonCompleted IssueClosedStateReason = "COMPLETED"
IssueClosedStateReasonDuplicate IssueClosedStateReason = "DUPLICATE"
IssueClosedStateReasonNotPlanned IssueClosedStateReason = "NOT_PLANNED"
)

// IssueFragment represents a fragment of an issue node in the GraphQL API.
type IssueFragment struct {
Number githubv4.Int
Expand Down Expand Up @@ -1099,7 +1118,7 @@ func ListIssues(getGQLClient GetGQLClientFn, t translations.TranslationHelperFun
}

// UpdateIssue creates a tool to update an existing issue in a GitHub repository.
func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
func UpdateIssue(getClient GetClientFn, getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
return mcp.NewTool("update_issue",
mcp.WithDescription(t("TOOL_UPDATE_ISSUE_DESCRIPTION", "Update an existing issue in a GitHub repository.")),
mcp.WithToolAnnotation(mcp.ToolAnnotation{
Expand Down Expand Up @@ -1128,6 +1147,13 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
mcp.Description("New state"),
mcp.Enum("open", "closed"),
),
mcp.WithString("state_reason",
mcp.Description("Reason for the state change. Ignored unless state is changed."),
mcp.Enum("completed", "not_planned", "duplicate", "reopened"),
),
mcp.WithNumber("duplicate_of",
mcp.Description("Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'."),
),
mcp.WithArray("labels",
mcp.Description("New labels"),
mcp.Items(
Expand Down Expand Up @@ -1167,6 +1193,8 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t

// Create the issue request with only provided fields
issueRequest := &github.IssueRequest{}
restUpdateNeeded := false
gqlUpdateNeeded := false

// Set optional parameters if provided
title, err := OptionalParam[string](request, "title")
Expand All @@ -1175,6 +1203,7 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
}
if title != "" {
issueRequest.Title = github.Ptr(title)
restUpdateNeeded = true
}

body, err := OptionalParam[string](request, "body")
Expand All @@ -1183,14 +1212,45 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
}
if body != "" {
issueRequest.Body = github.Ptr(body)
restUpdateNeeded = true
}

// Handle state and state_reason parameters
state, err := OptionalParam[string](request, "state")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
if state != "" {

stateReason, err := OptionalParam[string](request, "state_reason")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}

// Validate state_reason usage
if stateReason != "" && state == "" {
return mcp.NewToolResultError("state_reason can only be used when state is also provided"), nil
}
if state == "open" && stateReason != "" && stateReason != "reopened" {
return mcp.NewToolResultError("when state is 'open', state_reason can only be 'reopened'"), nil
}
if state == "closed" && stateReason != "" && stateReason != "completed" && stateReason != "not_planned" && stateReason != "duplicate" {
return mcp.NewToolResultError("when state is 'closed', state_reason can only be 'completed', 'not_planned', or 'duplicate'"), nil
}

// Use GraphQL for duplicate closure, REST for everything else
if state == "closed" && stateReason == "duplicate" {
gqlUpdateNeeded = true
} else if state != "" {
issueRequest.State = github.Ptr(state)
if stateReason != "" {
issueRequest.StateReason = github.Ptr(stateReason)
}
restUpdateNeeded = true
}

duplicateOf, err := OptionalIntParam(request, "duplicate_of")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}

// Get labels
Expand All @@ -1200,6 +1260,7 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
}
if len(labels) > 0 {
issueRequest.Labels = &labels
restUpdateNeeded = true
}

// Get assignees
Expand All @@ -1209,6 +1270,7 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
}
if len(assignees) > 0 {
issueRequest.Assignees = &assignees
restUpdateNeeded = true
}

milestone, err := OptionalIntParam(request, "milestone")
Expand All @@ -1218,6 +1280,7 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
if milestone != 0 {
milestoneNum := milestone
issueRequest.Milestone = &milestoneNum
restUpdateNeeded = true
}

// Get issue type
Expand All @@ -1227,34 +1290,117 @@ func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t
}
if issueType != "" {
issueRequest.Type = github.Ptr(issueType)
restUpdateNeeded = true
}

client, err := getClient(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
if !restUpdateNeeded && !gqlUpdateNeeded {
return mcp.NewToolResultError("No update parameters provided."), nil
}
updatedIssue, resp, err := client.Issues.Edit(ctx, owner, repo, issueNumber, issueRequest)
if err != nil {
return nil, fmt.Errorf("failed to update issue: %w", err)

// Handle REST API updates (title, body, state, labels, assignees, milestone, type, state_reason except "duplicate")
if restUpdateNeeded {
client, err := getClient(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
}

_, resp, err := client.Issues.Edit(ctx, owner, repo, issueNumber, issueRequest)
if err != nil {
return ghErrors.NewGitHubAPIErrorResponse(ctx,
"failed to update issue",
resp,
err,
), nil
}
defer func() { _ = resp.Body.Close() }()

if resp.StatusCode != http.StatusOK {
body, err := io.ReadAll(resp.Body)
if err != nil {
return nil, fmt.Errorf("failed to read response body: %w", err)
}
return mcp.NewToolResultError(fmt.Sprintf("failed to update issue: %s", string(body))), nil
}
}
defer func() { _ = resp.Body.Close() }()

if resp.StatusCode != http.StatusOK {
body, err := io.ReadAll(resp.Body)
// Handle GraphQL API updates (state_reason = "duplicate")
if gqlUpdateNeeded {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could consider extracting into 2 separate functions for the REST and GraphQL updaters, which could make unit testing a bit easier.

It also seems like the GraphQL capabilities are a superset of the REST API, so could we use only GraphQL?

Copy link
Contributor Author

@kerobbi kerobbi Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked better into this and we could indeed use only GQL (i.e., updateIssue / updateIssueInput). However, I believe the current hybrid approach is actually more optimal, mainly due to the amount of ID resolution overhead that would be needed if we go full GQL. I did refactor everything for better readability/separation of concerns, but more than happy to review this further if needed.

if duplicateOf == 0 {
return mcp.NewToolResultError("duplicate_of must be provided when state_reason is 'duplicate'"), nil
}

gqlClient, err := getGQLClient(ctx)
if err != nil {
return nil, fmt.Errorf("failed to read response body: %w", err)
return nil, fmt.Errorf("failed to get GraphQL client: %w", err)
}

var issueQuery struct {
Repository struct {
Issue struct {
ID githubv4.ID
} `graphql:"issue(number: $issueNumber)"`
DuplicateIssue struct {
ID githubv4.ID
} `graphql:"duplicateIssue: issue(number: $duplicateNumber)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}

err = gqlClient.Query(ctx, &issueQuery, map[string]interface{}{
"owner": githubv4.String(owner),
"repo": githubv4.String(repo),
"issueNumber": githubv4.Int(issueNumber), // #nosec G115 - issue numbers are always small positive integers
"duplicateNumber": githubv4.Int(duplicateOf), // #nosec G115 - issue numbers are always small positive integers
})
if err != nil {
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to find issues", err), nil
}

var mutation struct {
CloseIssue struct {
Issue struct {
ID githubv4.ID
Number githubv4.Int
URL githubv4.String
State githubv4.String
}
} `graphql:"closeIssue(input: $input)"`
}

duplicateStateReason := IssueClosedStateReasonDuplicate
err = gqlClient.Mutate(ctx, &mutation, CloseIssueInput{
IssueID: issueQuery.Repository.Issue.ID,
StateReason: &duplicateStateReason,
DuplicateIssueID: &issueQuery.Repository.DuplicateIssue.ID,
}, nil)
if err != nil {
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to close issue as duplicate", err), nil
}
return mcp.NewToolResultError(fmt.Sprintf("failed to update issue: %s", string(body))), nil
}

// Get the final state of the issue to return
client, err := getClient(ctx)
if err != nil {
return nil, err
}

finalIssue, resp, err := client.Issues.Get(ctx, owner, repo, issueNumber)
if err != nil {
return ghErrors.NewGitHubAPIErrorResponse(ctx, "Failed to get issue", resp, err), nil
}
defer func() {
if resp != nil && resp.Body != nil {
_ = resp.Body.Close()
}
}()

// Return minimal response with just essential information
minimalResponse := MinimalResponse{
URL: updatedIssue.GetHTMLURL(),
URL: finalIssue.GetHTMLURL(),
}

r, err := json.Marshal(minimalResponse)
if err != nil {
return nil, fmt.Errorf("failed to marshal response: %w", err)
return mcp.NewToolResultError(fmt.Sprintf("Failed to marshal response: %v", err)), nil
}

return mcp.NewToolResultText(string(r)), nil
Expand Down
Loading
Loading