From 5f5a0a51403f38f85f15f7ab6a55e702570f3b18 Mon Sep 17 00:00:00 2001 From: Javier Uruen Val Date: Mon, 24 Mar 2025 07:39:01 +0100 Subject: [PATCH] add support for the update_issue tool --- README.md | 14 ++- pkg/github/issues.go | 94 ++++++++++++++++++ pkg/github/issues_test.go | 198 ++++++++++++++++++++++++++++++++++++++ pkg/github/server.go | 1 + 4 files changed, 305 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 96d28ef7..b18a5964 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,18 @@ and set it as the GITHUB_PERSONAL_ACCESS_TOKEN environment variable. - `page`: Page number (number, optional) - `per_page`: Results per page (number, optional) +- **update_issue** - Update an existing issue in a GitHub repository + + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `issue_number`: Issue number to update (number, required) + - `title`: New title (string, optional) + - `body`: New description (string, optional) + - `state`: New state ('open' or 'closed') (string, optional) + - `labels`: Comma-separated list of new labels (string, optional) + - `assignees`: Comma-separated list of new assignees (string, optional) + - `milestone`: New milestone number (number, optional) + - **search_issues** - Search for issues and pull requests - `query`: Search query (string, required) - `sort`: Sort field (string, optional) @@ -368,8 +380,6 @@ Lots of things! Missing tools: - push_files (files array) -- list_issues (labels array) -- update_issue (labels and assignees arrays) - create_pull_request_review (comments array) Testing diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 0748e626..36130b98 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -361,6 +361,100 @@ func listIssues(client *github.Client, t translations.TranslationHelperFunc) (to } } +// updateIssue creates a tool to update an existing issue in a GitHub repository. +func updateIssue(client *github.Client, 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.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("issue_number", + mcp.Required(), + mcp.Description("Issue number to update"), + ), + mcp.WithString("title", + mcp.Description("New title"), + ), + mcp.WithString("body", + mcp.Description("New description"), + ), + mcp.WithString("state", + mcp.Description("New state ('open' or 'closed')"), + ), + mcp.WithString("labels", + mcp.Description("Comma-separated list of new labels"), + ), + mcp.WithString("assignees", + mcp.Description("Comma-separated list of new assignees"), + ), + mcp.WithNumber("milestone", + mcp.Description("New milestone number"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner := request.Params.Arguments["owner"].(string) + repo := request.Params.Arguments["repo"].(string) + issueNumber := int(request.Params.Arguments["issue_number"].(float64)) + + // Create the issue request with only provided fields + issueRequest := &github.IssueRequest{} + + // Set optional parameters if provided + if title, ok := request.Params.Arguments["title"].(string); ok && title != "" { + issueRequest.Title = github.Ptr(title) + } + + if body, ok := request.Params.Arguments["body"].(string); ok && body != "" { + issueRequest.Body = github.Ptr(body) + } + + if state, ok := request.Params.Arguments["state"].(string); ok && state != "" { + issueRequest.State = github.Ptr(state) + } + + if labels, ok := request.Params.Arguments["labels"].(string); ok && labels != "" { + labelsList := parseCommaSeparatedList(labels) + issueRequest.Labels = &labelsList + } + + if assignees, ok := request.Params.Arguments["assignees"].(string); ok && assignees != "" { + assigneesList := parseCommaSeparatedList(assignees) + issueRequest.Assignees = &assigneesList + } + + if milestone, ok := request.Params.Arguments["milestone"].(float64); ok { + milestoneNum := int(milestone) + issueRequest.Milestone = &milestoneNum + } + + updatedIssue, resp, err := client.Issues.Edit(ctx, owner, repo, issueNumber, issueRequest) + if err != nil { + return nil, fmt.Errorf("failed to update issue: %w", err) + } + 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 + } + + r, err := json.Marshal(updatedIssue) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + // parseISOTimestamp parses an ISO 8601 timestamp string into a time.Time object. // Returns the parsed time or an error if parsing fails. // Example formats supported: "2023-01-15T14:30:00Z", "2023-01-15" diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index b54d8fb4..4e8250fd 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -693,6 +693,204 @@ func Test_ListIssues(t *testing.T) { } } +func Test_UpdateIssue(t *testing.T) { + // Verify tool definition + mockClient := github.NewClient(nil) + tool, _ := updateIssue(mockClient, translations.NullTranslationHelper) + + assert.Equal(t, "update_issue", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "issue_number") + assert.Contains(t, tool.InputSchema.Properties, "title") + assert.Contains(t, tool.InputSchema.Properties, "body") + assert.Contains(t, tool.InputSchema.Properties, "state") + assert.Contains(t, tool.InputSchema.Properties, "labels") + assert.Contains(t, tool.InputSchema.Properties, "assignees") + assert.Contains(t, tool.InputSchema.Properties, "milestone") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "issue_number"}) + + // Setup mock issue for success case + mockIssue := &github.Issue{ + Number: github.Ptr(123), + Title: github.Ptr("Updated Issue Title"), + Body: github.Ptr("Updated issue description"), + State: github.Ptr("closed"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + Assignees: []*github.User{{Login: github.Ptr("assignee1")}, {Login: github.Ptr("assignee2")}}, + Labels: []*github.Label{{Name: github.Ptr("bug")}, {Name: github.Ptr("priority")}}, + Milestone: &github.Milestone{Number: github.Ptr(5)}, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedIssue *github.Issue + expectedErrMsg string + }{ + { + name: "update issue with all fields", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + "title": "Updated Issue Title", + "body": "Updated issue description", + "state": "closed", + "labels": "bug,priority", + "assignees": "assignee1,assignee2", + "milestone": float64(5), + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "update issue with minimal fields", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusOK, &github.Issue{ + Number: github.Ptr(123), + Title: github.Ptr("Only Title Updated"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + State: github.Ptr("open"), + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + "title": "Only Title Updated", + }, + expectError: false, + expectedIssue: &github.Issue{ + Number: github.Ptr(123), + Title: github.Ptr("Only Title Updated"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + State: github.Ptr("open"), + }, + }, + { + name: "update issue fails with not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesByOwnerByRepoByIssueNumber, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Issue not found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(999), + "title": "This issue doesn't exist", + }, + expectError: true, + expectedErrMsg: "failed to update issue", + }, + { + name: "update issue fails with validation error", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposIssuesByOwnerByRepoByIssueNumber, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message": "Invalid state value"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + "state": "invalid_state", + }, + expectError: true, + expectedErrMsg: "failed to update issue", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := updateIssue(client, translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + if err != nil { + assert.Contains(t, err.Error(), tc.expectedErrMsg) + } else { + // For errors returned as part of the result, not as an error + require.NotNil(t, result) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + } + return + } + + require.NoError(t, err) + + // Parse the result and get the text content if no error + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedIssue github.Issue + err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) + require.NoError(t, err) + + assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) + assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title) + assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) + assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) + + if tc.expectedIssue.Body != nil { + assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body) + } + + // Check assignees if expected + if tc.expectedIssue.Assignees != nil && len(tc.expectedIssue.Assignees) > 0 { + assert.Len(t, returnedIssue.Assignees, len(tc.expectedIssue.Assignees)) + for i, assignee := range returnedIssue.Assignees { + assert.Equal(t, *tc.expectedIssue.Assignees[i].Login, *assignee.Login) + } + } + + // Check labels if expected + if tc.expectedIssue.Labels != nil && len(tc.expectedIssue.Labels) > 0 { + assert.Len(t, returnedIssue.Labels, len(tc.expectedIssue.Labels)) + for i, label := range returnedIssue.Labels { + assert.Equal(t, *tc.expectedIssue.Labels[i].Name, *label.Name) + } + } + + // Check milestone if expected + if tc.expectedIssue.Milestone != nil { + assert.NotNil(t, returnedIssue.Milestone) + assert.Equal(t, *tc.expectedIssue.Milestone.Number, *returnedIssue.Milestone.Number) + } + }) + } +} + func Test_ParseISOTimestamp(t *testing.T) { tests := []struct { name string diff --git a/pkg/github/server.go b/pkg/github/server.go index 24c6761b..1c0df9a5 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -41,6 +41,7 @@ func NewServer(client *github.Client, readOnly bool, t translations.TranslationH s.AddTool(createIssue(client, t)) s.AddTool(addIssueComment(client, t)) s.AddTool(createIssue(client, t)) + s.AddTool(updateIssue(client, t)) } // Add GitHub tools - Pull Requests