Content-Length: 15052 | pFad | http://github.com/github/github-mcp-server/pull/34.patch

thub.com From c53d788b2a315a92d724c3a317664c8d8063082d 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 push_files tool --- README.md | 14 +- pkg/github/repositories.go | 115 ++++++++++++ pkg/github/repositories_test.go | 307 ++++++++++++++++++++++++++++++++ pkg/github/server.go | 1 + 4 files changed, 431 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index e1e96efea..76ed3c987 100644 --- a/README.md +++ b/README.md @@ -148,6 +148,14 @@ and set it as the GITHUB_PERSONAL_ACCESS_TOKEN environment variable. - `branch`: Branch name (string, optional) - `sha`: File SHA if updating (string, optional) +- **push_files** - Push multiple files in a single commit + + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `branch`: Branch to push to (string, required) + - `files`: Files to push, each with path and content (array, required) + - `message`: Commit message (string, required) + - **search_repositories** - Search for GitHub repositories - `query`: Search query (string, required) @@ -385,12 +393,6 @@ I'd like to know more about my GitHub profile. ## TODO -Lots of things! - -Missing tools: - -- push_files (files array) - Testing - Integration tests diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 9e0540b87..6e3b176df 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -413,3 +413,118 @@ func createBranch(client *github.Client, t translations.TranslationHelperFunc) ( return mcp.NewToolResultText(string(r)), nil } } + +// pushFiles creates a tool to push multiple files in a single commit to a GitHub repository. +func pushFiles(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("push_files", + mcp.WithDescription(t("TOOL_PUSH_FILES_DESCRIPTION", "Push multiple files to a GitHub repository in a single commit")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithString("branch", + mcp.Required(), + mcp.Description("Branch to push to"), + ), + mcp.WithArray("files", + mcp.Required(), + mcp.Description("Array of file objects to push, each object with path (string) and content (string)"), + ), + mcp.WithString("message", + mcp.Required(), + mcp.Description("Commit message"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner := request.Params.Arguments["owner"].(string) + repo := request.Params.Arguments["repo"].(string) + branch := request.Params.Arguments["branch"].(string) + message := request.Params.Arguments["message"].(string) + + // Parse files parameter - this should be an array of objects with path and content + filesObj, ok := request.Params.Arguments["files"].([]interface{}) + if !ok { + return mcp.NewToolResultError("files parameter must be an array of objects with path and content"), nil + } + + // Get the reference for the branch + ref, resp, err := client.Git.GetRef(ctx, owner, repo, "refs/heads/"+branch) + if err != nil { + return nil, fmt.Errorf("failed to get branch reference: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + // Get the commit object that the branch points to + baseCommit, resp, err := client.Git.GetCommit(ctx, owner, repo, *ref.Object.SHA) + if err != nil { + return nil, fmt.Errorf("failed to get base commit: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + // Create tree entries for all files + var entries []*github.TreeEntry + + for _, file := range filesObj { + fileMap, ok := file.(map[string]interface{}) + if !ok { + return mcp.NewToolResultError("each file must be an object with path and content"), nil + } + + path, ok := fileMap["path"].(string) + if !ok || path == "" { + return mcp.NewToolResultError("each file must have a path"), nil + } + + content, ok := fileMap["content"].(string) + if !ok { + return mcp.NewToolResultError("each file must have content"), nil + } + + // Create a tree entry for the file + entries = append(entries, &github.TreeEntry{ + Path: github.Ptr(path), + Mode: github.Ptr("100644"), // Regular file mode + Type: github.Ptr("blob"), + Content: github.Ptr(content), + }) + } + + // Create a new tree with the file entries + newTree, resp, err := client.Git.CreateTree(ctx, owner, repo, *baseCommit.Tree.SHA, entries) + if err != nil { + return nil, fmt.Errorf("failed to create tree: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + // Create a new commit + commit := &github.Commit{ + Message: github.Ptr(message), + Tree: newTree, + Parents: []*github.Commit{{SHA: baseCommit.SHA}}, + } + newCommit, resp, err := client.Git.CreateCommit(ctx, owner, repo, commit, nil) + if err != nil { + return nil, fmt.Errorf("failed to create commit: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + // Update the reference to point to the new commit + ref.Object.SHA = newCommit.SHA + updatedRef, resp, err := client.Git.UpdateRef(ctx, owner, repo, ref, false) + if err != nil { + return nil, fmt.Errorf("failed to update reference: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + r, err := json.Marshal(updatedRef) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index e65ff151d..34e8850a6 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -908,3 +908,310 @@ func Test_CreateRepository(t *testing.T) { }) } } + +func Test_PushFiles(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := pushFiles(mockClient, translations.NullTranslationHelper) + + assert.Equal(t, "push_files", 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, "branch") + assert.Contains(t, tool.InputSchema.Properties, "files") + assert.Contains(t, tool.InputSchema.Properties, "message") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "branch", "files", "message"}) + + // Setup mock objects + mockRef := &github.Reference{ + Ref: github.Ptr("refs/heads/main"), + Object: &github.GitObject{ + SHA: github.Ptr("abc123"), + URL: github.Ptr("https://api.github.com/repos/owner/repo/git/trees/abc123"), + }, + } + + mockCommit := &github.Commit{ + SHA: github.Ptr("abc123"), + Tree: &github.Tree{ + SHA: github.Ptr("def456"), + }, + } + + mockTree := &github.Tree{ + SHA: github.Ptr("ghi789"), + } + + mockNewCommit := &github.Commit{ + SHA: github.Ptr("jkl012"), + Message: github.Ptr("Update multiple files"), + HTMLURL: github.Ptr("https://github.com/owner/repo/commit/jkl012"), + } + + mockUpdatedRef := &github.Reference{ + Ref: github.Ptr("refs/heads/main"), + Object: &github.GitObject{ + SHA: github.Ptr("jkl012"), + URL: github.Ptr("https://api.github.com/repos/owner/repo/git/trees/jkl012"), + }, + } + + // Define test cases + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedRef *github.Reference + expectedErrMsg string + }{ + { + name: "successful push of multiple files", + mockedClient: mock.NewMockedHTTPClient( + // Get branch reference + mock.WithRequestMatch( + mock.GetReposGitRefByOwnerByRepoByRef, + mockRef, + ), + // Get commit + mock.WithRequestMatch( + mock.GetReposGitCommitsByOwnerByRepoByCommitSha, + mockCommit, + ), + // Create tree + mock.WithRequestMatch( + mock.PostReposGitTreesByOwnerByRepo, + mockTree, + ), + // Create commit + mock.WithRequestMatch( + mock.PostReposGitCommitsByOwnerByRepo, + mockNewCommit, + ), + // Update reference + mock.WithRequestMatch( + mock.PatchReposGitRefsByOwnerByRepoByRef, + mockUpdatedRef, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "branch": "main", + "files": []interface{}{ + map[string]interface{}{ + "path": "README.md", + "content": "# Updated README\n\nThis is an updated README file.", + }, + map[string]interface{}{ + "path": "docs/example.md", + "content": "# Example\n\nThis is an example file.", + }, + }, + "message": "Update multiple files", + }, + expectError: false, + expectedRef: mockUpdatedRef, + }, + { + name: "fails when files parameter is invalid", + mockedClient: mock.NewMockedHTTPClient( + // No requests expected + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "branch": "main", + "files": "invalid-files-parameter", // Not an array + "message": "Update multiple files", + }, + expectError: false, // This returns a tool error, not a Go error + expectedErrMsg: "files parameter must be an array", + }, + { + name: "fails when files contains object without path", + mockedClient: mock.NewMockedHTTPClient( + // Get branch reference + mock.WithRequestMatch( + mock.GetReposGitRefByOwnerByRepoByRef, + mockRef, + ), + // Get commit + mock.WithRequestMatch( + mock.GetReposGitCommitsByOwnerByRepoByCommitSha, + mockCommit, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "branch": "main", + "files": []interface{}{ + map[string]interface{}{ + "content": "# Missing path", + }, + }, + "message": "Update file", + }, + expectError: false, // This returns a tool error, not a Go error + expectedErrMsg: "each file must have a path", + }, + { + name: "fails when files contains object without content", + mockedClient: mock.NewMockedHTTPClient( + // Get branch reference + mock.WithRequestMatch( + mock.GetReposGitRefByOwnerByRepoByRef, + mockRef, + ), + // Get commit + mock.WithRequestMatch( + mock.GetReposGitCommitsByOwnerByRepoByCommitSha, + mockCommit, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "branch": "main", + "files": []interface{}{ + map[string]interface{}{ + "path": "README.md", + // Missing content + }, + }, + "message": "Update file", + }, + expectError: false, // This returns a tool error, not a Go error + expectedErrMsg: "each file must have content", + }, + { + name: "fails to get branch reference", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposGitRefByOwnerByRepoByRef, + mockResponse(t, http.StatusNotFound, nil), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "branch": "non-existent-branch", + "files": []interface{}{ + map[string]interface{}{ + "path": "README.md", + "content": "# README", + }, + }, + "message": "Update file", + }, + expectError: true, + expectedErrMsg: "failed to get branch reference", + }, + { + name: "fails to get base commit", + mockedClient: mock.NewMockedHTTPClient( + // Get branch reference + mock.WithRequestMatch( + mock.GetReposGitRefByOwnerByRepoByRef, + mockRef, + ), + // Fail to get commit + mock.WithRequestMatchHandler( + mock.GetReposGitCommitsByOwnerByRepoByCommitSha, + mockResponse(t, http.StatusNotFound, nil), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "branch": "main", + "files": []interface{}{ + map[string]interface{}{ + "path": "README.md", + "content": "# README", + }, + }, + "message": "Update file", + }, + expectError: true, + expectedErrMsg: "failed to get base commit", + }, + { + name: "fails to create tree", + mockedClient: mock.NewMockedHTTPClient( + // Get branch reference + mock.WithRequestMatch( + mock.GetReposGitRefByOwnerByRepoByRef, + mockRef, + ), + // Get commit + mock.WithRequestMatch( + mock.GetReposGitCommitsByOwnerByRepoByCommitSha, + mockCommit, + ), + // Fail to create tree + mock.WithRequestMatchHandler( + mock.PostReposGitTreesByOwnerByRepo, + mockResponse(t, http.StatusInternalServerError, nil), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "branch": "main", + "files": []interface{}{ + map[string]interface{}{ + "path": "README.md", + "content": "# README", + }, + }, + "message": "Update file", + }, + expectError: true, + expectedErrMsg: "failed to create tree", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := pushFiles(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 returnedRef github.Reference + err = json.Unmarshal([]byte(textContent.Text), &returnedRef) + require.NoError(t, err) + + assert.Equal(t, *tc.expectedRef.Ref, *returnedRef.Ref) + assert.Equal(t, *tc.expectedRef.Object.SHA, *returnedRef.Object.SHA) + }) + } +} diff --git a/pkg/github/server.go b/pkg/github/server.go index 75ab0a5fe..a0993e2f3 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -66,6 +66,7 @@ func NewServer(client *github.Client, readOnly bool, t translations.TranslationH s.AddTool(createRepository(client, t)) s.AddTool(forkRepository(client, t)) s.AddTool(createBranch(client, t)) + s.AddTool(pushFiles(client, t)) } // Add GitHub tools - Search

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