Content-Length: 79584 | pFad | http://github.com/github/github-mcp-server/pull/470.patch

thub.com From 0d6d026f7ffe305cd4a6a847d362930f5d5ec317 Mon Sep 17 00:00:00 2001 From: "Martin H. Normark" Date: Tue, 3 Jun 2025 23:18:47 +0200 Subject: [PATCH 01/11] Create 'add sub-issue' tool --- pkg/github/issues.go | 112 +++++++++++++++ pkg/github/issues_test.go | 293 ++++++++++++++++++++++++++++++++++++++ pkg/github/tools.go | 1 + 3 files changed, 406 insertions(+) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index ea068ed00..6327503fa 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -153,6 +153,118 @@ func AddIssueComment(getClient GetClientFn, t translations.TranslationHelperFunc } } +// AddSubIssue creates a tool to add a sub-issue to a parent issue. +func AddSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("add_sub_issue", + mcp.WithDescription(t("TOOL_ADD_SUB_ISSUE_DESCRIPTION", "Add a sub-issue to a parent issue in a GitHub repository.")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_ADD_SUB_ISSUE_USER_TITLE", "Add sub-issue"), + ReadOnlyHint: toBoolPtr(false), + }), + 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("The number of the parent issue"), + ), + mcp.WithNumber("sub_issue_id", + mcp.Required(), + mcp.Description("The ID of the sub-issue to add"), + ), + mcp.WithBoolean("replace_parent", + mcp.Description("When true, replaces the sub-issue's current parent issue"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := requiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := requiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + issueNumber, err := RequiredInt(request, "issue_number") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + subIssueID, err := RequiredInt(request, "sub_issue_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + replaceParent, err := OptionalParam[bool](request, "replace_parent") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + // Create the request body + requestBody := map[string]interface{}{ + "sub_issue_id": subIssueID, + } + if replaceParent { + requestBody["replace_parent"] = replaceParent + } + + // Since the go-github library might not have sub-issues support yet, + // we'll make a direct HTTP request using the client's HTTP client + reqBodyBytes, err := json.Marshal(requestBody) + if err != nil { + return nil, fmt.Errorf("failed to marshal request body: %w", err) + } + + url := fmt.Sprintf("https://api.github.com/repos/%s/%s/issues/%d/sub_issues", owner, repo, issueNumber) + req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(reqBodyBytes))) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + + // Use the same authentication as the GitHub client + httpClient := client.Client() + resp, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to add sub-issue: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + + if resp.StatusCode != http.StatusCreated { + return mcp.NewToolResultError(fmt.Sprintf("failed to add sub-issue: %s", string(body))), nil + } + + // Parse and re-marshal to ensure consistent formatting + var result interface{} + if err := json.Unmarshal(body, &result); err != nil { + return nil, fmt.Errorf("failed to unmarshal response: %w", err) + } + + r, err := json.Marshal(result) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + // SearchIssues creates a tool to search for issues and pull requests. func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_issues", diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 7c76d90f9..a7e4c4046 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1550,3 +1550,296 @@ func TestAssignCopilotToIssue(t *testing.T) { }) } } + +func Test_AddSubIssue(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := AddSubIssue(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "add_sub_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, "sub_issue_id") + assert.Contains(t, tool.InputSchema.Properties, "replace_parent") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "issue_number", "sub_issue_id"}) + + // Setup mock issue for success case (matches GitHub API response format) + mockIssue := &github.Issue{ + Number: github.Ptr(42), + Title: github.Ptr("Parent Issue"), + Body: github.Ptr("This is the parent issue with a sub-issue"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), + User: &github.User{ + Login: github.Ptr("testuser"), + }, + Labels: []*github.Label{ + { + Name: github.Ptr("enhancement"), + Color: github.Ptr("84b6eb"), + Description: github.Ptr("New feature or request"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedIssue *github.Issue + expectedErrMsg string + }{ + { + name: "successful sub-issue addition with all parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "POST", + }, + expectRequestBody(t, map[string]interface{}{ + "sub_issue_id": float64(123), + "replace_parent": true, + }).andThen( + mockResponse(t, http.StatusCreated, mockIssue), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "replace_parent": true, + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "successful sub-issue addition with minimal parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "POST", + }, + expectRequestBody(t, map[string]interface{}{ + "sub_issue_id": float64(456), + }).andThen( + mockResponse(t, http.StatusCreated, mockIssue), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(456), + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "successful sub-issue addition with replace_parent false", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "POST", + }, + expectRequestBody(t, map[string]interface{}{ + "sub_issue_id": float64(789), + }).andThen( + mockResponse(t, http.StatusCreated, mockIssue), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(789), + "replace_parent": false, + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "parent issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/999/sub_issues", + Method: "POST", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Parent issue not found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(999), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "failed to add sub-issue", + }, + { + name: "sub-issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "POST", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Sub-issue not found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(999), + }, + expectError: false, + expectedErrMsg: "failed to add sub-issue", + }, + { + name: "validation failed - sub-issue cannot be parent of itself", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "POST", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message": "Validation failed", "errors": [{"message": "Sub-issue cannot be a parent of itself"}]}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(42), + }, + expectError: false, + expectedErrMsg: "failed to add sub-issue", + }, + { + name: "insufficient permissions", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "POST", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message": "Must have write access to repository"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "failed to add sub-issue", + }, + { + name: "missing required parameter owner", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "POST", + }, + mockResponse(t, http.StatusCreated, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "missing required parameter: owner", + }, + { + name: "missing required parameter sub_issue_id", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "POST", + }, + mockResponse(t, http.StatusCreated, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedErrMsg: "missing required parameter: sub_issue_id", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := AddSubIssue(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + if tc.expectedErrMsg != "" { + 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.Body, *returnedIssue.Body) + assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) + assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) + assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index ba540d227..c00b4ddcc 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -59,6 +59,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(AddIssueComment(getClient, t)), toolsets.NewServerTool(UpdateIssue(getClient, t)), toolsets.NewServerTool(AssignCopilotToIssue(getGQLClient, t)), + toolsets.NewServerTool(AddSubIssue(getClient, t)), ) users := toolsets.NewToolset("users", "GitHub User related tools"). AddReadTools( From 080e498e26943c7372b91eee8e1a4379a2f8206c Mon Sep 17 00:00:00 2001 From: "Martin H. Normark" Date: Tue, 3 Jun 2025 23:40:39 +0200 Subject: [PATCH 02/11] Fix hardcoded API host --- pkg/github/issues.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 6327503fa..90e58cc88 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -223,7 +223,8 @@ func AddSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t return nil, fmt.Errorf("failed to marshal request body: %w", err) } - url := fmt.Sprintf("https://api.github.com/repos/%s/%s/issues/%d/sub_issues", owner, repo, issueNumber) + url := fmt.Sprintf("%srepos/%s/%s/issues/%d/sub_issues", + client.BaseURL.String(), owner, repo, issueNumber) req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(reqBodyBytes))) if err != nil { return nil, fmt.Errorf("failed to create request: %w", err) From 9b8ebc0ff34c657a350dfa47f1a405ac7e87e4fa Mon Sep 17 00:00:00 2001 From: "Martin H. Normark" Date: Tue, 3 Jun 2025 23:43:07 +0200 Subject: [PATCH 03/11] Create 'list sub-issues' tool --- pkg/github/issues.go | 98 +++++++++++++ pkg/github/issues_test.go | 286 ++++++++++++++++++++++++++++++++++++++ pkg/github/tools.go | 1 + 3 files changed, 385 insertions(+) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 90e58cc88..cc270a30c 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -266,6 +266,104 @@ func AddSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t } } +// ListSubIssues creates a tool to list sub-issues for a GitHub issue. +func ListSubIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("list_sub_issues", + mcp.WithDescription(t("TOOL_LIST_SUB_ISSUES_DESCRIPTION", "List sub-issues for a specific issue in a GitHub repository.")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_LIST_SUB_ISSUES_USER_TITLE", "List sub-issues"), + ReadOnlyHint: toBoolPtr(true), + }), + 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"), + ), + mcp.WithNumber("page", + mcp.Description("Page number for pagination (default: 1)"), + ), + mcp.WithNumber("per_page", + mcp.Description("Number of results per page (max 100, default: 30)"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := requiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := requiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + issueNumber, err := RequiredInt(request, "issue_number") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + page, err := OptionalIntParamWithDefault(request, "page", 1) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + perPage, err := OptionalIntParamWithDefault(request, "per_page", 30) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + // Since the go-github library might not have sub-issues support yet, + // we'll make a direct HTTP request using the client's HTTP client + url := fmt.Sprintf("%srepos/%s/%s/issues/%d/sub_issues?page=%d&per_page=%d", + client.BaseURL.String(), owner, repo, issueNumber, page, perPage) + req, err := http.NewRequestWithContext(ctx, "GET", url, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + + // Use the same authentication as the GitHub client + httpClient := client.Client() + resp, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to list sub-issues: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + + if resp.StatusCode != http.StatusOK { + return mcp.NewToolResultError(fmt.Sprintf("failed to list sub-issues: %s", string(body))), nil + } + + // Parse and re-marshal to ensure consistent formatting + var result interface{} + if err := json.Unmarshal(body, &result); err != nil { + return nil, fmt.Errorf("failed to unmarshal response: %w", err) + } + + r, err := json.Marshal(result) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + // SearchIssues creates a tool to search for issues and pull requests. func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_issues", diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index a7e4c4046..71fe81cd1 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -111,6 +111,9 @@ func Test_GetIssue(t *testing.T) { assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title) assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body) + assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) + assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) + assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) }) } } @@ -1843,3 +1846,286 @@ func Test_AddSubIssue(t *testing.T) { }) } } + +func Test_ListSubIssues(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := ListSubIssues(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "list_sub_issues", 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, "page") + assert.Contains(t, tool.InputSchema.Properties, "per_page") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "issue_number"}) + + // Setup mock sub-issues for success case + mockSubIssues := []*github.Issue{ + { + Number: github.Ptr(123), + Title: github.Ptr("Sub-issue 1"), + Body: github.Ptr("This is the first sub-issue"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + User: &github.User{ + Login: github.Ptr("user1"), + }, + Labels: []*github.Label{ + { + Name: github.Ptr("bug"), + Color: github.Ptr("d73a4a"), + Description: github.Ptr("Something isn't working"), + }, + }, + }, + { + Number: github.Ptr(124), + Title: github.Ptr("Sub-issue 2"), + Body: github.Ptr("This is the second sub-issue"), + State: github.Ptr("closed"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/124"), + User: &github.User{ + Login: github.Ptr("user2"), + }, + Assignees: []*github.User{ + {Login: github.Ptr("assignee1")}, + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedSubIssues []*github.Issue + expectedErrMsg string + }{ + { + name: "successful sub-issues listing with minimal parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "GET", + }, + expectQueryParams(t, map[string]string{ + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockSubIssues), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedSubIssues: mockSubIssues, + }, + { + name: "successful sub-issues listing with pagination", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "GET", + }, + expectQueryParams(t, map[string]string{ + "page": "2", + "per_page": "10", + }).andThen( + mockResponse(t, http.StatusOK, mockSubIssues), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "page": float64(2), + "per_page": float64(10), + }, + expectError: false, + expectedSubIssues: mockSubIssues, + }, + { + name: "successful sub-issues listing with empty result", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "GET", + }, + mockResponse(t, http.StatusOK, []*github.Issue{}), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedSubIssues: []*github.Issue{}, + }, + { + name: "parent issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/999/sub_issues", + Method: "GET", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(999), + }, + expectError: false, + expectedErrMsg: "failed to list sub-issues", + }, + { + name: "repository not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/nonexistent/repo/issues/42/sub_issues", + Method: "GET", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "nonexistent", + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedErrMsg: "failed to list sub-issues", + }, + { + name: "sub-issues feature gone/deprecated", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "GET", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusGone) + _, _ = w.Write([]byte(`{"message": "This feature has been deprecated"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedErrMsg: "failed to list sub-issues", + }, + { + name: "missing required parameter owner", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "GET", + }, + mockResponse(t, http.StatusOK, mockSubIssues), + ), + ), + requestArgs: map[string]interface{}{ + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedErrMsg: "missing required parameter: owner", + }, + { + name: "missing required parameter issue_number", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues", + Method: "GET", + }, + mockResponse(t, http.StatusOK, mockSubIssues), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + expectError: false, + expectedErrMsg: "missing required parameter: issue_number", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := ListSubIssues(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + if tc.expectedErrMsg != "" { + 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 returnedSubIssues []*github.Issue + err = json.Unmarshal([]byte(textContent.Text), &returnedSubIssues) + require.NoError(t, err) + + assert.Len(t, returnedSubIssues, len(tc.expectedSubIssues)) + for i, subIssue := range returnedSubIssues { + if i < len(tc.expectedSubIssues) { + assert.Equal(t, *tc.expectedSubIssues[i].Number, *subIssue.Number) + assert.Equal(t, *tc.expectedSubIssues[i].Title, *subIssue.Title) + assert.Equal(t, *tc.expectedSubIssues[i].State, *subIssue.State) + assert.Equal(t, *tc.expectedSubIssues[i].HTMLURL, *subIssue.HTMLURL) + assert.Equal(t, *tc.expectedSubIssues[i].User.Login, *subIssue.User.Login) + + if tc.expectedSubIssues[i].Body != nil { + assert.Equal(t, *tc.expectedSubIssues[i].Body, *subIssue.Body) + } + } + } + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index c00b4ddcc..c372abfaa 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -53,6 +53,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(SearchIssues(getClient, t)), toolsets.NewServerTool(ListIssues(getClient, t)), toolsets.NewServerTool(GetIssueComments(getClient, t)), + toolsets.NewServerTool(ListSubIssues(getClient, t)), ). AddWriteTools( toolsets.NewServerTool(CreateIssue(getClient, t)), From 9c527bf02c3a81c06a7879c6ae748cedc63accbf Mon Sep 17 00:00:00 2001 From: "Martin H. Normark" Date: Tue, 3 Jun 2025 23:49:36 +0200 Subject: [PATCH 04/11] Create 'remove sub-issue' tool --- pkg/github/issues.go | 103 +++++++++++++++ pkg/github/issues_test.go | 264 ++++++++++++++++++++++++++++++++++++++ pkg/github/tools.go | 1 + 3 files changed, 368 insertions(+) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index cc270a30c..80c3e251d 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -364,6 +364,109 @@ func ListSubIssues(getClient GetClientFn, t translations.TranslationHelperFunc) } } +// RemoveSubIssue creates a tool to remove a sub-issue from a parent issue. +func RemoveSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("remove_sub_issue", + mcp.WithDescription(t("TOOL_REMOVE_SUB_ISSUE_DESCRIPTION", "Remove a sub-issue from a parent issue in a GitHub repository.")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_REMOVE_SUB_ISSUE_USER_TITLE", "Remove sub-issue"), + ReadOnlyHint: toBoolPtr(false), + }), + 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("The number of the parent issue"), + ), + mcp.WithNumber("sub_issue_id", + mcp.Required(), + mcp.Description("The ID of the sub-issue to remove"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := requiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := requiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + issueNumber, err := RequiredInt(request, "issue_number") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + subIssueID, err := RequiredInt(request, "sub_issue_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + // Create the request body + requestBody := map[string]interface{}{ + "sub_issue_id": subIssueID, + } + + // Since the go-github library might not have sub-issues support yet, + // we'll make a direct HTTP request using the client's HTTP client + reqBodyBytes, err := json.Marshal(requestBody) + if err != nil { + return nil, fmt.Errorf("failed to marshal request body: %w", err) + } + + url := fmt.Sprintf("%srepos/%s/%s/issues/%d/sub_issue", + client.BaseURL.String(), owner, repo, issueNumber) + req, err := http.NewRequestWithContext(ctx, "DELETE", url, strings.NewReader(string(reqBodyBytes))) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + + // Use the same authentication as the GitHub client + httpClient := client.Client() + resp, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to remove sub-issue: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + + if resp.StatusCode != http.StatusOK { + return mcp.NewToolResultError(fmt.Sprintf("failed to remove sub-issue: %s", string(body))), nil + } + + // Parse and re-marshal to ensure consistent formatting + var result interface{} + if err := json.Unmarshal(body, &result); err != nil { + return nil, fmt.Errorf("failed to unmarshal response: %w", err) + } + + r, err := json.Marshal(result) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + // SearchIssues creates a tool to search for issues and pull requests. func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_issues", diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 71fe81cd1..8ecaf10a2 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2129,3 +2129,267 @@ func Test_ListSubIssues(t *testing.T) { }) } } + +func Test_RemoveSubIssue(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := RemoveSubIssue(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "remove_sub_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, "sub_issue_id") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "issue_number", "sub_issue_id"}) + + // Setup mock issue for success case (matches GitHub API response format - the updated parent issue) + mockIssue := &github.Issue{ + Number: github.Ptr(42), + Title: github.Ptr("Parent Issue"), + Body: github.Ptr("This is the parent issue after sub-issue removal"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), + User: &github.User{ + Login: github.Ptr("testuser"), + }, + Labels: []*github.Label{ + { + Name: github.Ptr("enhancement"), + Color: github.Ptr("84b6eb"), + Description: github.Ptr("New feature or request"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedIssue *github.Issue + expectedErrMsg string + }{ + { + name: "successful sub-issue removal", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issue", + Method: "DELETE", + }, + expectRequestBody(t, map[string]interface{}{ + "sub_issue_id": float64(123), + }).andThen( + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "parent issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/999/sub_issue", + Method: "DELETE", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(999), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "failed to remove sub-issue", + }, + { + name: "sub-issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issue", + Method: "DELETE", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Sub-issue not found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(999), + }, + expectError: false, + expectedErrMsg: "failed to remove sub-issue", + }, + { + name: "bad request - invalid sub_issue_id", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issue", + Method: "DELETE", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"message": "Invalid sub_issue_id"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(-1), + }, + expectError: false, + expectedErrMsg: "failed to remove sub-issue", + }, + { + name: "repository not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/nonexistent/repo/issues/42/sub_issue", + Method: "DELETE", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "nonexistent", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "failed to remove sub-issue", + }, + { + name: "insufficient permissions", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issue", + Method: "DELETE", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message": "Must have write access to repository"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "failed to remove sub-issue", + }, + { + name: "missing required parameter owner", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issue", + Method: "DELETE", + }, + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "missing required parameter: owner", + }, + { + name: "missing required parameter sub_issue_id", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issue", + Method: "DELETE", + }, + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectError: false, + expectedErrMsg: "missing required parameter: sub_issue_id", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := RemoveSubIssue(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + if tc.expectedErrMsg != "" { + 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.Body, *returnedIssue.Body) + assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) + assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) + assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index c372abfaa..da15d190b 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -61,6 +61,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(UpdateIssue(getClient, t)), toolsets.NewServerTool(AssignCopilotToIssue(getGQLClient, t)), toolsets.NewServerTool(AddSubIssue(getClient, t)), + toolsets.NewServerTool(RemoveSubIssue(getClient, t)), ) users := toolsets.NewToolset("users", "GitHub User related tools"). AddReadTools( From a0e757af01b133e88c310a49be8d44c5230bb134 Mon Sep 17 00:00:00 2001 From: "Martin H. Normark" Date: Wed, 4 Jun 2025 00:02:48 +0200 Subject: [PATCH 05/11] Fix Test_GetIssue mock data - add missing User field The assertion was already checking User.Login but the mock was incomplete --- pkg/github/issues_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 8ecaf10a2..34285633f 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -38,6 +38,9 @@ func Test_GetIssue(t *testing.T) { Body: github.Ptr("This is a test issue"), State: github.Ptr("open"), HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), + User: &github.User{ + Login: github.Ptr("testuser"), + }, } tests := []struct { From 39f8045be92e22e1b6bce917a51414c798b7f463 Mon Sep 17 00:00:00 2001 From: "Martin H. Normark" Date: Wed, 4 Jun 2025 00:05:56 +0200 Subject: [PATCH 06/11] Create 'reprioritize sub-issue' tool --- pkg/github/issues.go | 133 +++++++++++++++ pkg/github/issues_test.go | 343 ++++++++++++++++++++++++++++++++++++++ pkg/github/tools.go | 1 + 3 files changed, 477 insertions(+) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 80c3e251d..e8567641d 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -467,6 +467,139 @@ func RemoveSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) } } +// ReprioritizeSubIssue creates a tool to reprioritize a sub-issue to a different position in the parent list. +func ReprioritizeSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("reprioritize_sub_issue", + mcp.WithDescription(t("TOOL_REPRIORITIZE_SUB_ISSUE_DESCRIPTION", "Reprioritize a sub-issue to a different position in the parent issue's sub-issue list.")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_REPRIORITIZE_SUB_ISSUE_USER_TITLE", "Reprioritize sub-issue"), + ReadOnlyHint: toBoolPtr(false), + }), + 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("The number of the parent issue"), + ), + mcp.WithNumber("sub_issue_id", + mcp.Required(), + mcp.Description("The ID of the sub-issue to reprioritize"), + ), + mcp.WithNumber("after_id", + mcp.Description("The ID of the sub-issue to be prioritized after (either after_id OR before_id should be specified)"), + ), + mcp.WithNumber("before_id", + mcp.Description("The ID of the sub-issue to be prioritized before (either after_id OR before_id should be specified)"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := requiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := requiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + issueNumber, err := RequiredInt(request, "issue_number") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + subIssueID, err := RequiredInt(request, "sub_issue_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Handle optional positioning parameters + afterID, err := OptionalIntParam(request, "after_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + beforeID, err := OptionalIntParam(request, "before_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Validate that either after_id or before_id is specified, but not both + if afterID == 0 && beforeID == 0 { + return mcp.NewToolResultError("either after_id or before_id must be specified"), nil + } + if afterID != 0 && beforeID != 0 { + return mcp.NewToolResultError("only one of after_id or before_id should be specified, not both"), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + // Create the request body + requestBody := map[string]interface{}{ + "sub_issue_id": subIssueID, + } + if afterID != 0 { + requestBody["after_id"] = afterID + } + if beforeID != 0 { + requestBody["before_id"] = beforeID + } + + // Since the go-github library might not have sub-issues support yet, + // we'll make a direct HTTP request using the client's HTTP client + reqBodyBytes, err := json.Marshal(requestBody) + if err != nil { + return nil, fmt.Errorf("failed to marshal request body: %w", err) + } + + url := fmt.Sprintf("%srepos/%s/%s/issues/%d/sub_issues/priority", + client.BaseURL.String(), owner, repo, issueNumber) + req, err := http.NewRequestWithContext(ctx, "PATCH", url, strings.NewReader(string(reqBodyBytes))) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + + // Use the same authentication as the GitHub client + httpClient := client.Client() + resp, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to reprioritize sub-issue: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + + if resp.StatusCode != http.StatusOK { + return mcp.NewToolResultError(fmt.Sprintf("failed to reprioritize sub-issue: %s", string(body))), nil + } + + // Parse and re-marshal to ensure consistent formatting + var result interface{} + if err := json.Unmarshal(body, &result); err != nil { + return nil, fmt.Errorf("failed to unmarshal response: %w", err) + } + + r, err := json.Marshal(result) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + // SearchIssues creates a tool to search for issues and pull requests. func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_issues", diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 34285633f..d68099504 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2396,3 +2396,346 @@ func Test_RemoveSubIssue(t *testing.T) { }) } } + +func Test_ReprioritizeSubIssue(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := ReprioritizeSubIssue(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "reprioritize_sub_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, "sub_issue_id") + assert.Contains(t, tool.InputSchema.Properties, "after_id") + assert.Contains(t, tool.InputSchema.Properties, "before_id") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "issue_number", "sub_issue_id"}) + + // Setup mock issue for success case (matches GitHub API response format - the updated parent issue) + mockIssue := &github.Issue{ + Number: github.Ptr(42), + Title: github.Ptr("Parent Issue"), + Body: github.Ptr("This is the parent issue with reprioritized sub-issues"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), + User: &github.User{ + Login: github.Ptr("testuser"), + }, + Labels: []*github.Label{ + { + Name: github.Ptr("enhancement"), + Color: github.Ptr("84b6eb"), + Description: github.Ptr("New feature or request"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedIssue *github.Issue + expectedErrMsg string + }{ + { + name: "successful reprioritization with after_id", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues/priority", + Method: "PATCH", + }, + expectRequestBody(t, map[string]interface{}{ + "sub_issue_id": float64(123), + "after_id": float64(456), + }).andThen( + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "after_id": float64(456), + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "successful reprioritization with before_id", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues/priority", + Method: "PATCH", + }, + expectRequestBody(t, map[string]interface{}{ + "sub_issue_id": float64(123), + "before_id": float64(789), + }).andThen( + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "before_id": float64(789), + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "validation error - neither after_id nor before_id specified", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues/priority", + Method: "PATCH", + }, + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + }, + expectError: false, + expectedErrMsg: "either after_id or before_id must be specified", + }, + { + name: "validation error - both after_id and before_id specified", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues/priority", + Method: "PATCH", + }, + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "after_id": float64(456), + "before_id": float64(789), + }, + expectError: false, + expectedErrMsg: "only one of after_id or before_id should be specified, not both", + }, + { + name: "parent issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/999/sub_issues/priority", + Method: "PATCH", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(999), + "sub_issue_id": float64(123), + "after_id": float64(456), + }, + expectError: false, + expectedErrMsg: "failed to reprioritize sub-issue", + }, + { + name: "sub-issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues/priority", + Method: "PATCH", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Sub-issue not found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(999), + "after_id": float64(456), + }, + expectError: false, + expectedErrMsg: "failed to reprioritize sub-issue", + }, + { + name: "validation failed - positioning sub-issue not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues/priority", + Method: "PATCH", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message": "Validation failed", "errors": [{"message": "Positioning sub-issue not found"}]}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "after_id": float64(999), + }, + expectError: false, + expectedErrMsg: "failed to reprioritize sub-issue", + }, + { + name: "insufficient permissions", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues/priority", + Method: "PATCH", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message": "Must have write access to repository"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "after_id": float64(456), + }, + expectError: false, + expectedErrMsg: "failed to reprioritize sub-issue", + }, + { + name: "service unavailable", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues/priority", + Method: "PATCH", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusServiceUnavailable) + _, _ = w.Write([]byte(`{"message": "Service Unavailable"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "before_id": float64(456), + }, + expectError: false, + expectedErrMsg: "failed to reprioritize sub-issue", + }, + { + name: "missing required parameter owner", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues/priority", + Method: "PATCH", + }, + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "repo": "repo", + "issue_number": float64(42), + "sub_issue_id": float64(123), + "after_id": float64(456), + }, + expectError: false, + expectedErrMsg: "missing required parameter: owner", + }, + { + name: "missing required parameter sub_issue_id", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/issues/42/sub_issues/priority", + Method: "PATCH", + }, + mockResponse(t, http.StatusOK, mockIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "after_id": float64(456), + }, + expectError: false, + expectedErrMsg: "missing required parameter: sub_issue_id", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := ReprioritizeSubIssue(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + if tc.expectedErrMsg != "" { + 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.Body, *returnedIssue.Body) + assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) + assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) + assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index da15d190b..0fbb39a85 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -62,6 +62,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(AssignCopilotToIssue(getGQLClient, t)), toolsets.NewServerTool(AddSubIssue(getClient, t)), toolsets.NewServerTool(RemoveSubIssue(getClient, t)), + toolsets.NewServerTool(ReprioritizeSubIssue(getClient, t)), ) users := toolsets.NewToolset("users", "GitHub User related tools"). AddReadTools( From ef56393f801fe94e7abba4a0decd377c66ceea4c Mon Sep 17 00:00:00 2001 From: LuluBeatson Date: Thu, 17 Jul 2025 09:44:41 +0100 Subject: [PATCH 07/11] fixes --- pkg/github/issues.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 78cedeca2..f4d190662 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -159,7 +159,7 @@ func AddSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t mcp.WithDescription(t("TOOL_ADD_SUB_ISSUE_DESCRIPTION", "Add a sub-issue to a parent issue in a GitHub repository.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_ADD_SUB_ISSUE_USER_TITLE", "Add sub-issue"), - ReadOnlyHint: toBoolPtr(false), + ReadOnlyHint: ToBoolPtr(false), }), mcp.WithString("owner", mcp.Required(), @@ -182,11 +182,11 @@ func AddSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - owner, err := requiredParam[string](request, "owner") + owner, err := RequiredParam[string](request, "owner") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - repo, err := requiredParam[string](request, "repo") + repo, err := RequiredParam[string](request, "repo") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -272,7 +272,7 @@ func ListSubIssues(getClient GetClientFn, t translations.TranslationHelperFunc) mcp.WithDescription(t("TOOL_LIST_SUB_ISSUES_DESCRIPTION", "List sub-issues for a specific issue in a GitHub repository.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_LIST_SUB_ISSUES_USER_TITLE", "List sub-issues"), - ReadOnlyHint: toBoolPtr(true), + ReadOnlyHint: ToBoolPtr(true), }), mcp.WithString("owner", mcp.Required(), @@ -294,11 +294,11 @@ func ListSubIssues(getClient GetClientFn, t translations.TranslationHelperFunc) ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - owner, err := requiredParam[string](request, "owner") + owner, err := RequiredParam[string](request, "owner") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - repo, err := requiredParam[string](request, "repo") + repo, err := RequiredParam[string](request, "repo") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -370,7 +370,7 @@ func RemoveSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) mcp.WithDescription(t("TOOL_REMOVE_SUB_ISSUE_DESCRIPTION", "Remove a sub-issue from a parent issue in a GitHub repository.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_REMOVE_SUB_ISSUE_USER_TITLE", "Remove sub-issue"), - ReadOnlyHint: toBoolPtr(false), + ReadOnlyHint: ToBoolPtr(false), }), mcp.WithString("owner", mcp.Required(), @@ -390,11 +390,11 @@ func RemoveSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - owner, err := requiredParam[string](request, "owner") + owner, err := RequiredParam[string](request, "owner") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - repo, err := requiredParam[string](request, "repo") + repo, err := RequiredParam[string](request, "repo") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -473,7 +473,7 @@ func ReprioritizeSubIssue(getClient GetClientFn, t translations.TranslationHelpe mcp.WithDescription(t("TOOL_REPRIORITIZE_SUB_ISSUE_DESCRIPTION", "Reprioritize a sub-issue to a different position in the parent issue's sub-issue list.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_REPRIORITIZE_SUB_ISSUE_USER_TITLE", "Reprioritize sub-issue"), - ReadOnlyHint: toBoolPtr(false), + ReadOnlyHint: ToBoolPtr(false), }), mcp.WithString("owner", mcp.Required(), @@ -499,11 +499,11 @@ func ReprioritizeSubIssue(getClient GetClientFn, t translations.TranslationHelpe ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - owner, err := requiredParam[string](request, "owner") + owner, err := RequiredParam[string](request, "owner") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - repo, err := requiredParam[string](request, "repo") + repo, err := RequiredParam[string](request, "repo") if err != nil { return mcp.NewToolResultError(err.Error()), nil } From e7ac4b08c4a2dc8e124875ef300e5e62fc4e77cc Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Thu, 17 Jul 2025 11:15:27 +0100 Subject: [PATCH 08/11] use go github pck to add sub-issues --- pkg/github/issues.go | 58 +++++++++++++------------------------------- 1 file changed, 17 insertions(+), 41 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index f4d190662..20a00b2d1 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -9,6 +9,7 @@ import ( "strings" "time" + ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/translations" "github.com/go-viper/mapstructure/v2" "github.com/google/go-github/v73/github" @@ -175,7 +176,7 @@ func AddSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t ), mcp.WithNumber("sub_issue_id", mcp.Required(), - mcp.Description("The ID of the sub-issue to add"), + mcp.Description("The ID of the sub-issue to add. Note: This is NOT the same as the issue number."), ), mcp.WithBoolean("replace_parent", mcp.Description("When true, replaces the sub-issue's current parent issue"), @@ -208,56 +209,31 @@ func AddSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t return nil, fmt.Errorf("failed to get GitHub client: %w", err) } - // Create the request body - requestBody := map[string]interface{}{ - "sub_issue_id": subIssueID, - } - if replaceParent { - requestBody["replace_parent"] = replaceParent - } - - // Since the go-github library might not have sub-issues support yet, - // we'll make a direct HTTP request using the client's HTTP client - reqBodyBytes, err := json.Marshal(requestBody) - if err != nil { - return nil, fmt.Errorf("failed to marshal request body: %w", err) + subIssueRequest := github.SubIssueRequest{ + SubIssueID: int64(subIssueID), + ReplaceParent: ToBoolPtr(replaceParent), } - url := fmt.Sprintf("%srepos/%s/%s/issues/%d/sub_issues", - client.BaseURL.String(), owner, repo, issueNumber) - req, err := http.NewRequestWithContext(ctx, "POST", url, strings.NewReader(string(reqBodyBytes))) + subIssue, resp, err := client.SubIssue.Add(ctx, owner, repo, int64(issueNumber), subIssueRequest) if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to add sub-issue", + resp, + err, + ), nil } - req.Header.Set("Accept", "application/vnd.github+json") - req.Header.Set("Content-Type", "application/json") - req.Header.Set("X-GitHub-Api-Version", "2022-11-28") - - // Use the same authentication as the GitHub client - httpClient := client.Client() - resp, err := httpClient.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to add sub-issue: %w", err) - } - defer func() { _ = resp.Body.Close() }() - - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } + defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusCreated { + 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 add sub-issue: %s", string(body))), nil } - // Parse and re-marshal to ensure consistent formatting - var result interface{} - if err := json.Unmarshal(body, &result); err != nil { - return nil, fmt.Errorf("failed to unmarshal response: %w", err) - } - - r, err := json.Marshal(result) + r, err := json.Marshal(subIssue) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } From 71712b8f311fc1b55b47a3f2a7492453c0bc7757 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Fri, 18 Jul 2025 16:49:19 +0100 Subject: [PATCH 09/11] Update to use go github package --- pkg/github/issues.go | 46 ++++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 20a00b2d1..874347a96 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -296,48 +296,40 @@ func ListSubIssues(getClient GetClientFn, t translations.TranslationHelperFunc) return nil, fmt.Errorf("failed to get GitHub client: %w", err) } - // Since the go-github library might not have sub-issues support yet, - // we'll make a direct HTTP request using the client's HTTP client - url := fmt.Sprintf("%srepos/%s/%s/issues/%d/sub_issues?page=%d&per_page=%d", - client.BaseURL.String(), owner, repo, issueNumber, page, perPage) - req, err := http.NewRequestWithContext(ctx, "GET", url, nil) - if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) - } - - req.Header.Set("Accept", "application/vnd.github+json") - req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + opts := &github.IssueListOptions{ + ListOptions: github.ListOptions{ + Page: page, + PerPage: perPage, + }, + } - // Use the same authentication as the GitHub client - httpClient := client.Client() - resp, err := httpClient.Do(req) + subIssues, resp, err := client.SubIssue.ListByIssue(ctx, owner, repo, int64(issueNumber), opts) if err != nil { - return nil, fmt.Errorf("failed to list sub-issues: %w", err) + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to list sub-issues", + resp, + err, + ), nil } - defer func() { _ = resp.Body.Close() }() - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %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 list sub-issues: %s", string(body))), nil } - // Parse and re-marshal to ensure consistent formatting - var result interface{} - if err := json.Unmarshal(body, &result); err != nil { - return nil, fmt.Errorf("failed to unmarshal response: %w", err) - } - - r, err := json.Marshal(result) + r, err := json.Marshal(subIssues) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } return mcp.NewToolResultText(string(r)), nil } + } // RemoveSubIssue creates a tool to remove a sub-issue from a parent issue. From 26469adc85bf39de2ff1b6a0d826a7176042bcb3 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Fri, 18 Jul 2025 18:03:28 +0100 Subject: [PATCH 10/11] update description --- pkg/github/issues.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 874347a96..41f4b7664 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -354,7 +354,7 @@ func RemoveSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) ), mcp.WithNumber("sub_issue_id", mcp.Required(), - mcp.Description("The ID of the sub-issue to remove"), + mcp.Description("The ID of the sub-issue to remove. Note: This is NOT the same as the issue number."), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { From 34465bdb72fc5f95331d8bd1ccfdce26a2821938 Mon Sep 17 00:00:00 2001 From: tommaso-moro Date: Fri, 18 Jul 2025 18:20:58 +0100 Subject: [PATCH 11/11] update to use go github v73 --- pkg/github/issues.go | 92 +++++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 56 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 41f4b7664..502bb0cfb 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -457,7 +457,7 @@ func ReprioritizeSubIssue(getClient GetClientFn, t translations.TranslationHelpe ), mcp.WithNumber("sub_issue_id", mcp.Required(), - mcp.Description("The ID of the sub-issue to reprioritize"), + mcp.Description("The ID of the sub-issue to reprioritize. Note: This is NOT the same as the issue number."), ), mcp.WithNumber("after_id", mcp.Description("The ID of the sub-issue to be prioritized after (either after_id OR before_id should be specified)"), @@ -503,68 +503,48 @@ func ReprioritizeSubIssue(getClient GetClientFn, t translations.TranslationHelpe } client, err := getClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) - } - - // Create the request body - requestBody := map[string]interface{}{ - "sub_issue_id": subIssueID, - } - if afterID != 0 { - requestBody["after_id"] = afterID - } - if beforeID != 0 { - requestBody["before_id"] = beforeID - } - - // Since the go-github library might not have sub-issues support yet, - // we'll make a direct HTTP request using the client's HTTP client - reqBodyBytes, err := json.Marshal(requestBody) - if err != nil { - return nil, fmt.Errorf("failed to marshal request body: %w", err) - } - - url := fmt.Sprintf("%srepos/%s/%s/issues/%d/sub_issues/priority", - client.BaseURL.String(), owner, repo, issueNumber) - req, err := http.NewRequestWithContext(ctx, "PATCH", url, strings.NewReader(string(reqBodyBytes))) - if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) - } + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } - req.Header.Set("Accept", "application/vnd.github+json") - req.Header.Set("Content-Type", "application/json") - req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + subIssueRequest := github.SubIssueRequest{ + SubIssueID: int64(subIssueID), + } - // Use the same authentication as the GitHub client - httpClient := client.Client() - resp, err := httpClient.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to reprioritize sub-issue: %w", err) - } - defer func() { _ = resp.Body.Close() }() + if afterID != 0 { + afterIDInt64 := int64(afterID) + subIssueRequest.AfterID = &afterIDInt64 + } + if beforeID != 0 { + beforeIDInt64 := int64(beforeID) + subIssueRequest.BeforeID = &beforeIDInt64 + } - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } + subIssue, resp, err := client.SubIssue.Reprioritize(ctx, owner, repo, int64(issueNumber), subIssueRequest) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to reprioritize sub-issue", + resp, + err, + ), nil + } - if resp.StatusCode != http.StatusOK { - return mcp.NewToolResultError(fmt.Sprintf("failed to reprioritize sub-issue: %s", string(body))), nil - } + defer func() { _ = resp.Body.Close() }() - // Parse and re-marshal to ensure consistent formatting - var result interface{} - if err := json.Unmarshal(body, &result); err != nil { - return nil, fmt.Errorf("failed to unmarshal response: %w", err) - } + 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 reprioritize sub-issue: %s", string(body))), nil + } - r, err := json.Marshal(result) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } + r, err := json.Marshal(subIssue) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } - return mcp.NewToolResultText(string(r)), nil + return mcp.NewToolResultText(string(r)), nil } }

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!