From eac95091b30257271b00f9c13ffb3e853bb432bc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Nov 2025 11:12:18 +0000 Subject: [PATCH 1/3] Initial plan From 543f06150d2a8752ac6aaa51e361d42e8d792a87 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Nov 2025 11:25:37 +0000 Subject: [PATCH 2/3] Migrate git toolset to modelcontextprotocol/go-sdk - Remove //go:build ignore tag from git.go - Update imports to use modelcontextprotocol/go-sdk - Convert GetRepositoryTree tool schema to jsonschema format - Update handler signature to use new generics pattern - Update parameter extraction to use args map - Replace mcp.NewToolResult* with utils package helpers - Create dedicated git_test.go with updated test patterns - Update toolsnaps for get_repository_tree Related to #1428 Co-authored-by: omgitsads <4619+omgitsads@users.noreply.github.com> --- .../__toolsnaps__/get_repository_tree.snap | 36 ++-- pkg/github/git.go | 108 +++++----- pkg/github/git_test.go | 196 ++++++++++++++++++ pkg/github/repositories_test.go | 2 +- 4 files changed, 276 insertions(+), 66 deletions(-) create mode 100644 pkg/github/git_test.go diff --git a/pkg/github/__toolsnaps__/get_repository_tree.snap b/pkg/github/__toolsnaps__/get_repository_tree.snap index 0645bf241..882462883 100644 --- a/pkg/github/__toolsnaps__/get_repository_tree.snap +++ b/pkg/github/__toolsnaps__/get_repository_tree.snap @@ -1,38 +1,38 @@ { "annotations": { - "title": "Get repository tree", - "readOnlyHint": true + "readOnlyHint": true, + "title": "Get repository tree" }, "description": "Get the tree structure (files and directories) of a GitHub repository at a specific ref or SHA", "inputSchema": { + "type": "object", + "required": [ + "owner", + "repo" + ], "properties": { "owner": { - "description": "Repository owner (username or organization)", - "type": "string" + "type": "string", + "description": "Repository owner (username or organization)" }, "path_filter": { - "description": "Optional path prefix to filter the tree results (e.g., 'src/' to only show files in the src directory)", - "type": "string" + "type": "string", + "description": "Optional path prefix to filter the tree results (e.g., 'src/' to only show files in the src directory)" }, "recursive": { - "default": false, + "type": "boolean", "description": "Setting this parameter to true returns the objects or subtrees referenced by the tree. Default is false", - "type": "boolean" + "default": false }, "repo": { - "description": "Repository name", - "type": "string" + "type": "string", + "description": "Repository name" }, "tree_sha": { - "description": "The SHA1 value or ref (branch or tag) name of the tree. Defaults to the repository's default branch", - "type": "string" + "type": "string", + "description": "The SHA1 value or ref (branch or tag) name of the tree. Defaults to the repository's default branch" } - }, - "required": [ - "owner", - "repo" - ], - "type": "object" + } }, "name": "get_repository_tree" } \ No newline at end of file diff --git a/pkg/github/git.go b/pkg/github/git.go index cbbc8e3d7..c2a839132 100644 --- a/pkg/github/git.go +++ b/pkg/github/git.go @@ -1,5 +1,3 @@ -//go:build ignore - package github import ( @@ -10,9 +8,10 @@ import ( ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/translations" + "github.com/github/github-mcp-server/pkg/utils" "github.com/google/go-github/v79/github" - "github.com/mark3labs/mcp-go/mcp" - "github.com/mark3labs/mcp-go/server" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" ) // TreeEntryResponse represents a single entry in a Git tree. @@ -38,57 +37,69 @@ type TreeResponse struct { } // GetRepositoryTree creates a tool to get the tree structure of a GitHub repository. -func GetRepositoryTree(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { - return mcp.NewTool("get_repository_tree", - mcp.WithDescription(t("TOOL_GET_REPOSITORY_TREE_DESCRIPTION", "Get the tree structure (files and directories) of a GitHub repository at a specific ref or SHA")), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_GET_REPOSITORY_TREE_USER_TITLE", "Get repository tree"), - ReadOnlyHint: ToBoolPtr(true), - }), - mcp.WithString("owner", - mcp.Required(), - mcp.Description("Repository owner (username or organization)"), - ), - mcp.WithString("repo", - mcp.Required(), - mcp.Description("Repository name"), - ), - mcp.WithString("tree_sha", - mcp.Description("The SHA1 value or ref (branch or tag) name of the tree. Defaults to the repository's default branch"), - ), - mcp.WithBoolean("recursive", - mcp.Description("Setting this parameter to true returns the objects or subtrees referenced by the tree. Default is false"), - mcp.DefaultBool(false), - ), - mcp.WithString("path_filter", - mcp.Description("Optional path prefix to filter the tree results (e.g., 'src/' to only show files in the src directory)"), - ), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - owner, err := RequiredParam[string](request, "owner") +func GetRepositoryTree(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, mcp.ToolHandlerFor[map[string]any, any]) { + tool := mcp.Tool{ + Name: "get_repository_tree", + Description: t("TOOL_GET_REPOSITORY_TREE_DESCRIPTION", "Get the tree structure (files and directories) of a GitHub repository at a specific ref or SHA"), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_GET_REPOSITORY_TREE_USER_TITLE", "Get repository tree"), + ReadOnlyHint: true, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "tree_sha": { + Type: "string", + Description: "The SHA1 value or ref (branch or tag) name of the tree. Defaults to the repository's default branch", + }, + "recursive": { + Type: "boolean", + Description: "Setting this parameter to true returns the objects or subtrees referenced by the tree. Default is false", + Default: json.RawMessage(`false`), + }, + "path_filter": { + Type: "string", + Description: "Optional path prefix to filter the tree results (e.g., 'src/' to only show files in the src directory)", + }, + }, + Required: []string{"owner", "repo"}, + }, + } + + handler := mcp.ToolHandlerFor[map[string]any, any]( + func(ctx context.Context, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") if err != nil { - return mcp.NewToolResultError(err.Error()), nil + return utils.NewToolResultError(err.Error()), nil, nil } - repo, err := RequiredParam[string](request, "repo") + repo, err := RequiredParam[string](args, "repo") if err != nil { - return mcp.NewToolResultError(err.Error()), nil + return utils.NewToolResultError(err.Error()), nil, nil } - treeSHA, err := OptionalParam[string](request, "tree_sha") + treeSHA, err := OptionalParam[string](args, "tree_sha") if err != nil { - return mcp.NewToolResultError(err.Error()), nil + return utils.NewToolResultError(err.Error()), nil, nil } - recursive, err := OptionalBoolParamWithDefault(request, "recursive", false) + recursive, err := OptionalBoolParamWithDefault(args, "recursive", false) if err != nil { - return mcp.NewToolResultError(err.Error()), nil + return utils.NewToolResultError(err.Error()), nil, nil } - pathFilter, err := OptionalParam[string](request, "path_filter") + pathFilter, err := OptionalParam[string](args, "path_filter") if err != nil { - return mcp.NewToolResultError(err.Error()), nil + return utils.NewToolResultError(err.Error()), nil, nil } client, err := getClient(ctx) if err != nil { - return mcp.NewToolResultError("failed to get GitHub client"), nil + return utils.NewToolResultError("failed to get GitHub client"), nil, nil } // If no tree_sha is provided, use the repository's default branch @@ -99,7 +110,7 @@ func GetRepositoryTree(getClient GetClientFn, t translations.TranslationHelperFu "failed to get repository info", repoResp, err, - ), nil + ), nil, nil } treeSHA = *repoInfo.DefaultBranch } @@ -111,7 +122,7 @@ func GetRepositoryTree(getClient GetClientFn, t translations.TranslationHelperFu "failed to get repository tree", resp, err, - ), nil + ), nil, nil } defer func() { _ = resp.Body.Close() }() @@ -154,9 +165,12 @@ func GetRepositoryTree(getClient GetClientFn, t translations.TranslationHelperFu r, err := json.Marshal(response) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - return mcp.NewToolResultText(string(r)), nil - } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) + + return tool, handler } diff --git a/pkg/github/git_test.go b/pkg/github/git_test.go new file mode 100644 index 000000000..66cbccd6e --- /dev/null +++ b/pkg/github/git_test.go @@ -0,0 +1,196 @@ +package github + +import ( + "context" + "encoding/json" + "net/http" + "strings" + "testing" + + "github.com/github/github-mcp-server/internal/toolsnaps" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v79/github" + "github.com/google/jsonschema-go/jsonschema" + "github.com/migueleliasweb/go-github-mock/src/mock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_GetRepositoryTree(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := GetRepositoryTree(stubGetClientFn(mockClient), translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "get_repository_tree", tool.Name) + assert.NotEmpty(t, tool.Description) + + // Type assert the InputSchema to access its properties + inputSchema, ok := tool.InputSchema.(*jsonschema.Schema) + require.True(t, ok, "expected InputSchema to be *jsonschema.Schema") + assert.Contains(t, inputSchema.Properties, "owner") + assert.Contains(t, inputSchema.Properties, "repo") + assert.Contains(t, inputSchema.Properties, "tree_sha") + assert.Contains(t, inputSchema.Properties, "recursive") + assert.Contains(t, inputSchema.Properties, "path_filter") + assert.ElementsMatch(t, inputSchema.Required, []string{"owner", "repo"}) + + // Setup mock data + mockRepo := &github.Repository{ + DefaultBranch: github.Ptr("main"), + } + mockTree := &github.Tree{ + SHA: github.Ptr("abc123"), + Truncated: github.Ptr(false), + Entries: []*github.TreeEntry{ + { + Path: github.Ptr("README.md"), + Mode: github.Ptr("100644"), + Type: github.Ptr("blob"), + SHA: github.Ptr("file1sha"), + Size: github.Ptr(123), + URL: github.Ptr("https://api.github.com/repos/owner/repo/git/blobs/file1sha"), + }, + { + Path: github.Ptr("src/main.go"), + Mode: github.Ptr("100644"), + Type: github.Ptr("blob"), + SHA: github.Ptr("file2sha"), + Size: github.Ptr(456), + URL: github.Ptr("https://api.github.com/repos/owner/repo/git/blobs/file2sha"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedErrMsg string + }{ + { + name: "successfully get repository tree", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposByOwnerByRepo, + mockResponse(t, http.StatusOK, mockRepo), + ), + mock.WithRequestMatchHandler( + mock.GetReposGitTreesByOwnerByRepoByTreeSha, + mockResponse(t, http.StatusOK, mockTree), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + }, + { + name: "successfully get repository tree with path filter", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposByOwnerByRepo, + mockResponse(t, http.StatusOK, mockRepo), + ), + mock.WithRequestMatchHandler( + mock.GetReposGitTreesByOwnerByRepoByTreeSha, + mockResponse(t, http.StatusOK, mockTree), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "path_filter": "src/", + }, + }, + { + name: "repository not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposByOwnerByRepo, + 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": "nonexistent", + }, + expectError: true, + expectedErrMsg: "failed to get repository info", + }, + { + name: "tree not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposByOwnerByRepo, + mockResponse(t, http.StatusOK, mockRepo), + ), + mock.WithRequestMatchHandler( + mock.GetReposGitTreesByOwnerByRepoByTreeSha, + 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", + }, + expectError: true, + expectedErrMsg: "failed to get repository tree", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, handler := GetRepositoryTree(stubGetClientFromHTTPFn(tc.mockedClient), translations.NullTranslationHelper) + + // Create the tool request + request := createMCPRequest(tc.requestArgs) + + result, _, err := handler(context.Background(), &request, tc.requestArgs) + + if tc.expectError { + require.NoError(t, err) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + } else { + require.NoError(t, err) + require.False(t, result.IsError) + + // Parse the result and get the text content + textContent := getTextResult(t, result) + + // Parse the JSON response + var treeResponse map[string]interface{} + err := json.Unmarshal([]byte(textContent.Text), &treeResponse) + require.NoError(t, err) + + // Verify response structure + assert.Equal(t, "owner", treeResponse["owner"]) + assert.Equal(t, "repo", treeResponse["repo"]) + assert.Contains(t, treeResponse, "tree") + assert.Contains(t, treeResponse, "count") + assert.Contains(t, treeResponse, "sha") + assert.Contains(t, treeResponse, "truncated") + + // Check filtering if path_filter was provided + if pathFilter, exists := tc.requestArgs["path_filter"]; exists { + tree := treeResponse["tree"].([]interface{}) + for _, entry := range tree { + entryMap := entry.(map[string]interface{}) + path := entryMap["path"].(string) + assert.True(t, strings.HasPrefix(path, pathFilter.(string)), + "Path %s should start with filter %s", path, pathFilter) + } + } + } + }) + } +} diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index ea784fe43..21ee409c1 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -3373,7 +3373,7 @@ func Test_GetRepositoryTree(t *testing.T) { // Create the tool request request := createMCPRequest(tc.requestArgs) - result, err := handler(context.Background(), request) + result, _, err := handler(context.Background(), &request, tc.requestArgs) if tc.expectError { require.NoError(t, err) From c26bcc97344c9020aac586b75e9330506e6dc7bd Mon Sep 17 00:00:00 2001 From: LuluBeatson Date: Wed, 19 Nov 2025 14:50:26 +0000 Subject: [PATCH 3/3] re-add git toolset --- pkg/github/tools.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 95b36f8b9..6414953f2 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -194,10 +194,10 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG // toolsets.NewServerResourceTemplate(GetRepositoryResourceTagContent(getClient, getRawClient, t)), // toolsets.NewServerResourceTemplate(GetRepositoryResourcePrContent(getClient, getRawClient, t)), // ) - // git := toolsets.NewToolset(ToolsetMetadataGit.ID, ToolsetMetadataGit.Description). - // AddReadTools( - // toolsets.NewServerTool(GetRepositoryTree(getClient, t)), - // ) + git := toolsets.NewToolset(ToolsetMetadataGit.ID, ToolsetMetadataGit.Description). + AddReadTools( + toolsets.NewServerTool(GetRepositoryTree(getClient, t)), + ) // issues := toolsets.NewToolset(ToolsetMetadataIssues.ID, ToolsetMetadataIssues.Description). // AddReadTools( // toolsets.NewServerTool(IssueRead(getClient, getGQLClient, t, flags)), @@ -361,7 +361,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG // Add toolsets to the group tsg.AddToolset(contextTools) // tsg.AddToolset(repos) - // tsg.AddToolset(git) + tsg.AddToolset(git) // tsg.AddToolset(issues) // tsg.AddToolset(orgs) // tsg.AddToolset(users)