Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 18, 2025

Migrates the GetRepositoryTree tool from mark3labs/mcp-go to modelcontextprotocol/go-sdk as part of #1428.

Changes

  • Schema definition: DSL-based → explicit jsonschema.Schema structures

    // Before
    mcp.NewTool("get_repository_tree",
        mcp.WithString("owner", mcp.Required(), ...),
        mcp.WithBoolean("recursive", mcp.DefaultBool(false), ...),
    )
    
    // After
    tool := mcp.Tool{
        InputSchema: &jsonschema.Schema{
            Type: "object",
            Properties: map[string]*jsonschema.Schema{
                "owner": {Type: "string", Description: "..."},
                "recursive": {Type: "boolean", Default: json.RawMessage(`false`), ...},
            },
            Required: []string{"owner", "repo"},
        },
    }
  • Handler signature: Updated to generics pattern

    • Before: func(ctx, request) (result, error)
    • After: func(ctx, request, args) (result, any, error)
  • Parameter extraction: Uses args map[string]any directly instead of extracting from request

  • Result helpers: mcp.NewToolResult*utils.NewToolResult*

  • Test file: Created dedicated git_test.go with updated handler invocation and type assertions

  • Toolsnaps: Updated schema snapshot (property ordering changed, logically equivalent)

Original prompt

Migrate the git toolset to modelcontextprotocol/go-sdk.

This will be merged into the parent PR #1428, so reference that PR in the description to track the migration

Custom agent used: go-sdk-tool-migrator
Agent specializing in migrating MCP tools from mark3labs/mcp-go to modelcontextprotocol/go-sdk


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

- 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>
Copilot AI changed the title [WIP] Migrate git toolset to modelcontextprotocol/go-sdk Migrate git toolset to modelcontextprotocol/go-sdk Nov 18, 2025
Copilot finished work on behalf of omgitsads November 18, 2025 11:33
Copilot AI requested a review from omgitsads November 18, 2025 11:33
@LuluBeatson LuluBeatson self-requested a review November 19, 2025 14:50
@LuluBeatson LuluBeatson marked this pull request as ready for review November 19, 2025 14:51
@LuluBeatson LuluBeatson requested a review from a team as a code owner November 19, 2025 14:51
Copilot AI review requested due to automatic review settings November 19, 2025 14:51
Copy link
Contributor

@LuluBeatson LuluBeatson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image

Copilot finished reviewing on behalf of LuluBeatson November 19, 2025 14:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR migrates the get_repository_tree tool from the deprecated mark3labs/mcp-go SDK to the new modelcontextprotocol/go-sdk as part of the broader SDK migration effort (#1428).

Key changes:

  • Migrated tool schema definition from DSL-based to explicit jsonschema.Schema structures
  • Updated handler signature from 2-return to 3-return values pattern
  • Changed parameter extraction to use args map[string]any directly
  • Moved result helpers from mcp.NewToolResult* to utils.NewToolResult*

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/github/git.go Removed build ignore tag, updated imports to new SDK, migrated tool definition and handler to new SDK patterns
pkg/github/git_test.go Created new dedicated test file following migrated test patterns with toolsnap validation and comprehensive test cases
pkg/github/repositories_test.go Updated handler invocation signature in existing test (should be removed per comment)
pkg/github/tools.go Uncommented git toolset definition and registration to activate the migrated tool
pkg/github/toolsnaps/get_repository_tree.snap Updated schema snapshot with reordered properties (logically equivalent)
Comments suppressed due to low confidence (1)

pkg/github/repositories_test.go:3416

  • The Test_GetRepositoryTree test should be removed from repositories_test.go since:
  1. The test has been moved to the new git_test.go file as part of the migration to the git toolset
  2. This file (repositories_test.go) is build-ignored (//go:build ignore) and hasn't been migrated yet (still imports mark3labs/mcp-go)
  3. The updated handler invocation on this line is inconsistent with the rest of this file, which still uses the old 2-return-value signature

The entire Test_GetRepositoryTree function (lines 3243-3416) should be deleted from this file to avoid confusion and maintain consistency.

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)
	assert.Contains(t, tool.InputSchema.Properties, "owner")
	assert.Contains(t, tool.InputSchema.Properties, "repo")
	assert.Contains(t, tool.InputSchema.Properties, "tree_sha")
	assert.Contains(t, tool.InputSchema.Properties, "recursive")
	assert.Contains(t, tool.InputSchema.Properties, "path_filter")
	assert.ElementsMatch(t, tool.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)
					}
				}
			}
		})
	}
}

@omgitsads omgitsads merged commit eaf411c into omgitsads/go-sdk Nov 19, 2025
19 of 20 checks passed
@omgitsads omgitsads deleted the copilot/migrate-git-toolset-to-go-sdk branch November 19, 2025 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants