From 69e37b570013426a76526b3719f7c6f4f3443865 Mon Sep 17 00:00:00 2001 From: Arya Soni Date: Wed, 9 Apr 2025 19:11:23 +0200 Subject: [PATCH 1/6] Add ability to view branches for a repo #141 --- pkg/github/repositories.go | 58 ++++++++++++++++++ pkg/github/repositories_test.go | 103 ++++++++++++++++++++++++++++++++ pkg/github/server.go | 1 + 3 files changed, 162 insertions(+) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 2dafd4ce..4efa5b2e 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -79,6 +79,64 @@ func ListCommits(client *github.Client, t translations.TranslationHelperFunc) (t } } +// ListBranches creates a tool to list branches in a GitHub repository. +func ListBranches(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("list_branches", + mcp.WithDescription(t("TOOL_LIST_BRANCHES_DESCRIPTION", "List branches in a GitHub repository")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + WithPagination(), + ), + 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 + } + pagination, err := OptionalPaginationParams(request) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + opts := &github.BranchListOptions{ + ListOptions: github.ListOptions{ + Page: pagination.page, + PerPage: pagination.perPage, + }, + } + + branches, resp, err := client.Repositories.ListBranches(ctx, owner, repo, opts) + if err != nil { + return nil, fmt.Errorf("failed to list branches: %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 branches: %s", string(body))), nil + } + + r, err := json.Marshal(branches) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + // CreateOrUpdateFile creates a tool to create or update a file in a GitHub repository. func CreateOrUpdateFile(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("create_or_update_file", diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 5c47183d..08260e34 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "net/http" + "net/http/httptest" + "net/url" "testing" "time" @@ -1293,3 +1295,104 @@ func Test_PushFiles(t *testing.T) { }) } } + +func Test_ListBranches(t *testing.T) { + // Create a test server + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/repos/owner/repo/branches", r.URL.Path) + assert.Equal(t, "GET", r.Method) + + // Check query parameters + query := r.URL.Query() + if page := query.Get("page"); page != "" { + assert.Equal(t, "2", page) + } + if perPage := query.Get("per_page"); perPage != "" { + assert.Equal(t, "30", perPage) + } + + // Return mock branches + mockBranches := []github.Branch{ + {Name: github.String("main")}, + {Name: github.String("develop")}, + } + mockResponse(t, http.StatusOK, mockBranches)(w, r) + })) + defer ts.Close() + + // Create a GitHub client using the test server URL + client := github.NewClient(nil) + client.BaseURL, _ = url.Parse(ts.URL + "/") + + // Create the tool + tool, handler := ListBranches(client, translations.NullTranslationHelper) + + // Test cases + tests := []struct { + name string + args map[string]interface{} + wantErr bool + errContains string + }{ + { + name: "success", + args: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "page": float64(2), + }, + wantErr: false, + }, + { + name: "missing owner", + args: map[string]interface{}{ + "repo": "repo", + }, + wantErr: true, + errContains: "missing required parameter: owner", + }, + { + name: "missing repo", + args: map[string]interface{}{ + "owner": "owner", + }, + wantErr: true, + errContains: "missing required parameter: repo", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create request + request := createMCPRequest(tt.args) + + // Call handler + result, err := handler(context.Background(), request) + if tt.wantErr { + require.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + return + } + + require.NoError(t, err) + textContent := getTextResult(t, result) + + // Verify response + var branches []github.Branch + err = json.Unmarshal([]byte(textContent.Text), &branches) + require.NoError(t, err) + assert.Len(t, branches, 2) + assert.Equal(t, "main", *branches[0].Name) + assert.Equal(t, "develop", *branches[1].Name) + }) + } + + // Verify tool definition + assert.Equal(t, "list_branches", tool.Name) + assert.Contains(t, tool.InputSchema.Required, "owner") + assert.Contains(t, tool.InputSchema.Required, "repo") + assert.NotContains(t, tool.InputSchema.Required, "page") + assert.NotContains(t, tool.InputSchema.Required, "perPage") +} diff --git a/pkg/github/server.go b/pkg/github/server.go index 80457a54..5ff29a1c 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -60,6 +60,7 @@ func NewServer(client *github.Client, version string, readOnly bool, t translati s.AddTool(SearchRepositories(client, t)) s.AddTool(GetFileContents(client, t)) s.AddTool(ListCommits(client, t)) + s.AddTool(ListBranches(client, t)) if !readOnly { s.AddTool(CreateOrUpdateFile(client, t)) s.AddTool(CreateRepository(client, t)) From 629922ef2963aca891a3d55738ec7731d1d0c159 Mon Sep 17 00:00:00 2001 From: Arya Soni Date: Thu, 10 Apr 2025 22:34:12 +0530 Subject: [PATCH 2/6] fix: update ListBranches test to use InputSchema and correct translation helper --- pkg/github/repositories_test.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index f833b2b3..bb9414f7 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -1369,15 +1369,18 @@ func Test_ListBranches(t *testing.T) { // Call handler result, err := handler(context.Background(), request) if tt.wantErr { - require.Error(t, err) + require.NoError(t, err) + textContent := getTextResult(t, result) if tt.errContains != "" { - assert.Contains(t, err.Error(), tt.errContains) + assert.Contains(t, textContent.Text, tt.errContains) } return } require.NoError(t, err) + require.NotNil(t, result) textContent := getTextResult(t, result) + require.NotEmpty(t, textContent.Text) // Verify response var branches []github.Branch @@ -1391,8 +1394,10 @@ func Test_ListBranches(t *testing.T) { // Verify tool definition assert.Equal(t, "list_branches", tool.Name) - assert.Contains(t, tool.InputSchema.Required, "owner") - assert.Contains(t, tool.InputSchema.Required, "repo") - assert.NotContains(t, tool.InputSchema.Required, "page") - assert.NotContains(t, tool.InputSchema.Required, "perPage") + 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, "page") + assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) } From 64cde128057412808b3cc7afb7f01a7010f31281 Mon Sep 17 00:00:00 2001 From: Arya Soni Date: Fri, 11 Apr 2025 00:06:48 +0530 Subject: [PATCH 3/6] fix: update ListBranches test to use InputSchema and correct translation helper --- pkg/github/repositories_test.go | 89 +++++++++++++++------------------ 1 file changed, 41 insertions(+), 48 deletions(-) diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index bb9414f7..4db691d4 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -4,8 +4,6 @@ import ( "context" "encoding/json" "net/http" - "net/http/httptest" - "net/url" "testing" "time" @@ -1297,42 +1295,37 @@ func Test_PushFiles(t *testing.T) { } func Test_ListBranches(t *testing.T) { - // Create a test server - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/repos/owner/repo/branches", r.URL.Path) - assert.Equal(t, "GET", r.Method) - - // Check query parameters - query := r.URL.Query() - if page := query.Get("page"); page != "" { - assert.Equal(t, "2", page) - } - if perPage := query.Get("per_page"); perPage != "" { - assert.Equal(t, "30", perPage) - } - - // Return mock branches - mockBranches := []github.Branch{ - {Name: github.String("main")}, - {Name: github.String("develop")}, - } - mockResponse(t, http.StatusOK, mockBranches)(w, r) - })) - defer ts.Close() - - // Create a GitHub client using the test server URL - client := github.NewClient(nil) - client.BaseURL, _ = url.Parse(ts.URL + "/") - - // Create the tool - tool, handler := ListBranches(stubGetClientFn(client), translations.NullTranslationHelper) + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := ListBranches(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "list_branches", 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, "page") + assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) + + // Setup mock branches for success case + mockBranches := []*github.Branch{ + { + Name: github.String("main"), + Commit: &github.RepositoryCommit{SHA: github.String("abc123")}, + }, + { + Name: github.String("develop"), + Commit: &github.RepositoryCommit{SHA: github.String("def456")}, + }, + } // Test cases tests := []struct { - name string - args map[string]interface{} - wantErr bool - errContains string + name string + args map[string]interface{} + mockResponses []mock.MockBackendOption + wantErr bool + errContains string }{ { name: "success", @@ -1341,6 +1334,12 @@ func Test_ListBranches(t *testing.T) { "repo": "repo", "page": float64(2), }, + mockResponses: []mock.MockBackendOption{ + mock.WithRequestMatch( + mock.GetReposBranchesByOwnerByRepo, + mockBranches, + ), + }, wantErr: false, }, { @@ -1363,16 +1362,19 @@ func Test_ListBranches(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Create mock client + mockClient := github.NewClient(mock.NewMockedHTTPClient(tt.mockResponses...)) + _, handler := ListBranches(stubGetClientFn(mockClient), translations.NullTranslationHelper) + // Create request request := createMCPRequest(tt.args) // Call handler result, err := handler(context.Background(), request) if tt.wantErr { - require.NoError(t, err) - textContent := getTextResult(t, result) + require.Error(t, err) if tt.errContains != "" { - assert.Contains(t, textContent.Text, tt.errContains) + assert.Contains(t, err.Error(), tt.errContains) } return } @@ -1383,7 +1385,7 @@ func Test_ListBranches(t *testing.T) { require.NotEmpty(t, textContent.Text) // Verify response - var branches []github.Branch + var branches []*github.Branch err = json.Unmarshal([]byte(textContent.Text), &branches) require.NoError(t, err) assert.Len(t, branches, 2) @@ -1391,13 +1393,4 @@ func Test_ListBranches(t *testing.T) { assert.Equal(t, "develop", *branches[1].Name) }) } - - // Verify tool definition - assert.Equal(t, "list_branches", 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, "page") - assert.Contains(t, tool.InputSchema.Properties, "perPage") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) } From b274df904b2bbec4ebbaf8a529708678ce9334b4 Mon Sep 17 00:00:00 2001 From: Arya Soni Date: Fri, 11 Apr 2025 00:10:50 +0530 Subject: [PATCH 4/6] fix: update ListBranches test to handle errors in tool result --- pkg/github/repositories_test.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 4db691d4..36fdde63 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -1347,16 +1347,18 @@ func Test_ListBranches(t *testing.T) { args: map[string]interface{}{ "repo": "repo", }, - wantErr: true, - errContains: "missing required parameter: owner", + mockResponses: []mock.MockBackendOption{}, + wantErr: false, + errContains: "missing required parameter: owner", }, { name: "missing repo", args: map[string]interface{}{ "owner": "owner", }, - wantErr: true, - errContains: "missing required parameter: repo", + mockResponses: []mock.MockBackendOption{}, + wantErr: false, + errContains: "missing required parameter: repo", }, } @@ -1381,6 +1383,13 @@ func Test_ListBranches(t *testing.T) { require.NoError(t, err) require.NotNil(t, result) + + if tt.errContains != "" { + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tt.errContains) + return + } + textContent := getTextResult(t, result) require.NotEmpty(t, textContent.Text) From 99060d7053dbfc3b9511b6885f79d9ed21fbdc23 Mon Sep 17 00:00:00 2001 From: Arya Soni Date: Fri, 11 Apr 2025 02:41:51 +0530 Subject: [PATCH 5/6] fix: replace deprecated github.String with github.Ptr --- pkg/github/repositories_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 36fdde63..48b44b3b 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -1310,12 +1310,12 @@ func Test_ListBranches(t *testing.T) { // Setup mock branches for success case mockBranches := []*github.Branch{ { - Name: github.String("main"), - Commit: &github.RepositoryCommit{SHA: github.String("abc123")}, + Name: github.Ptr("main"), + Commit: &github.RepositoryCommit{SHA: github.Ptr("abc123")}, }, { - Name: github.String("develop"), - Commit: &github.RepositoryCommit{SHA: github.String("def456")}, + Name: github.Ptr("develop"), + Commit: &github.RepositoryCommit{SHA: github.Ptr("def456")}, }, } From 266677d75d534371db0dd73059b7c7c5f4ffe782 Mon Sep 17 00:00:00 2001 From: Arya Soni Date: Fri, 11 Apr 2025 17:46:43 +0530 Subject: [PATCH 6/6] docs: add list_branches tool documentation to README --- README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README.md b/README.md index f85663ab..00f49b64 100644 --- a/README.md +++ b/README.md @@ -311,6 +311,13 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `branch`: Branch name (string, optional) - `sha`: File SHA if updating (string, optional) +- **list_branches** - List branches in a GitHub repository + + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `page`: Page number (number, optional) + - `perPage`: Results per page (number, optional) + - **push_files** - Push multiple files in a single commit - `owner`: Repository owner (string, required)