Content-Length: 39037 | pFad | http://github.com/github/github-mcp-server/pull/424.patch

thub.com From 33eb82cc8abe6f3e6262fd406a9ca08256df6abd Mon Sep 17 00:00:00 2001 From: LukasPoque Date: Wed, 21 May 2025 16:47:59 +0200 Subject: [PATCH 1/6] Add tool to star or unstar a repository for the user --- README.md | 5 ++ pkg/github/repositories.go | 72 +++++++++++++++++ pkg/github/repositories_test.go | 136 ++++++++++++++++++++++++++++++++ pkg/github/tools.go | 1 + 4 files changed, 214 insertions(+) diff --git a/README.md b/README.md index 352bb50eb..a0a107000 100644 --- a/README.md +++ b/README.md @@ -496,6 +496,11 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `page`: Page number (number, optional) - `perPage`: Results per page (number, optional) +- **toggle_repository_star** - Star or unstar a repository for the authenticated user + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `star`: True to star, false to unstar the repository (boolean, required) + - **create_repository** - Create a new GitHub repository - `name`: Repository name (string, required) - `description`: Repository description (string, optional) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 4403e2a19..5e1a58573 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -556,6 +556,78 @@ func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc) } } +// ToggleRepositoryStar creates a tool to star or unstar a repository. +func ToggleRepositoryStar(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("toggle_repository_star", + mcp.WithDescription(t("TOOL_TOGGLE_REPOSITORY_STAR_DESCRIPTION", "Star or unstar a GitHub repository with your account")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_TOGGLE_REPOSITORY_STAR_USER_TITLE", "Star/unstar repository"), + ReadOnlyHint: toBoolPtr(false), + }), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithBoolean("star", + mcp.Required(), + mcp.Description("True to star, false to unstar the repository"), + ), + ), + 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 + } + star, err := requiredParam[bool](request, "star") + 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) + } + + var resp *github.Response + var action string + + if star { + resp, err = client.Activity.Star(ctx, owner, repo) + action = "star" + } else { + resp, err = client.Activity.Unstar(ctx, owner, repo) + action = "unstar" + } + + if err != nil { + return nil, fmt.Errorf("failed to %s repository: %w", action, err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusNoContent { + 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 %s repository: %s", action, string(body))), nil + } + + resultAction := "starred" + if !star { + resultAction = "unstarred" + } + return mcp.NewToolResultText(fmt.Sprintf("Successfully %s repository %s/%s", resultAction, owner, repo)), nil + } +} + // DeleteFile creates a tool to delete a file in a GitHub repository. // This tool uses a more roundabout way of deleting a file than just using the client.Repositories.DeleteFile. // This is because REST file deletion endpoint (and client.Repositories.DeleteFile) don't add commit signing to the deletion commit, diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index e4edeee88..675a19e3d 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -1963,3 +1963,139 @@ func Test_GetTag(t *testing.T) { }) } } + +func Test_ToggleRepositoryStar(t *testing.T) { + // Verify tool definition + mockClient := github.NewClient(nil) + tool, _ := ToggleRepositoryStar(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "toggle_repository_star", 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, "star") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "star"}) + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedErrMsg string + expectedResult string + }{ + { + name: "successfully star repository", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/user/starred/owner/repo", + Method: "PUT", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "star": true, + }, + expectError: false, + expectedResult: "Successfully starred repository owner/repo", + }, + { + name: "successfully unstar repository", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/user/starred/owner/repo", + Method: "DELETE", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "star": false, + }, + expectError: false, + expectedResult: "Successfully unstarred repository owner/repo", + }, + { + name: "star repository fails with unauthorized", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/user/starred/owner/repo", + Method: "PUT", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message": "Requires authentication"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "star": true, + }, + expectError: true, + expectedErrMsg: "failed to star repository", + }, + { + name: "unstar repository fails with unauthorized", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/user/starred/owner/repo", + Method: "DELETE", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message": "Requires authentication"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "star": false, + }, + expectError: true, + expectedErrMsg: "failed to unstar repository", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := ToggleRepositoryStar(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 + } + + require.NoError(t, err) + + // Parse the result and get the text content if no error + textContent := getTextResult(t, result) + assert.Equal(t, tc.expectedResult, textContent.Text) + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index a04e7336b..6e4124bd4 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -39,6 +39,7 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn, toolsets.NewServerTool(CreateBranch(getClient, t)), toolsets.NewServerTool(PushFiles(getClient, t)), toolsets.NewServerTool(DeleteFile(getClient, t)), + toolsets.NewServerTool(ToggleRepositoryStar(getClient, t)), ) issues := toolsets.NewToolset("issues", "GitHub Issues related tools"). AddReadTools( From badd5da749e58d0180183386595bc6a63730d674 Mon Sep 17 00:00:00 2001 From: LukasPoque Date: Wed, 21 May 2025 16:48:41 +0200 Subject: [PATCH 2/6] Add a tool to check if a repository is stared or not --- README.md | 4 ++ pkg/github/repositories.go | 44 ++++++++++++ pkg/github/repositories_test.go | 118 ++++++++++++++++++++++++++++++++ pkg/github/tools.go | 1 + 4 files changed, 167 insertions(+) diff --git a/README.md b/README.md index a0a107000..9a7534990 100644 --- a/README.md +++ b/README.md @@ -496,6 +496,10 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `page`: Page number (number, optional) - `perPage`: Results per page (number, optional) +- **is_repository_starred** - Check if a repository is starred by the authenticated user + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - **toggle_repository_star** - Star or unstar a repository for the authenticated user - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 5e1a58573..be2307f79 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -556,6 +556,50 @@ func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc) } } +// IsRepositoryStarred creates a tool to check if a repository is starred by the authenticated user. +func IsRepositoryStarred(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("is_repository_starred", + mcp.WithDescription(t("TOOL_IS_REPOSITORY_STARRED_DESCRIPTION", "Check if a GitHub repository is starred by the authenticated user")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_IS_REPOSITORY_STARRED_USER_TITLE", "Check if repository is starred"), + ReadOnlyHint: toBoolPtr(true), + }), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + ), + 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 + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + isStarred, resp, err := client.Activity.IsStarred(ctx, owner, repo) + if err != nil && resp == nil { + return nil, fmt.Errorf("failed to check if repository is starred: %w", err) + } + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + + return mcp.NewToolResultText(fmt.Sprintf(`{"is_starred": %t}`, isStarred)), nil + } +} + // ToggleRepositoryStar creates a tool to star or unstar a repository. func ToggleRepositoryStar(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("toggle_repository_star", diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 675a19e3d..a31a6ae46 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -1964,6 +1964,124 @@ func Test_GetTag(t *testing.T) { } } +func Test_IsRepositoryStarred(t *testing.T) { + // Verify tool definition + mockClient := github.NewClient(nil) + tool, _ := IsRepositoryStarred(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "is_repository_starred", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedErrMsg string + isStarred bool + }{ + { + name: "repository is starred", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/user/starred/owner/repo", + Method: "GET", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) // Status code for "is starred" = true + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + expectError: false, + isStarred: true, + }, + { + name: "repository is not starred", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/user/starred/owner/repo", + Method: "GET", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) // Status code for "is starred" = false + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + expectError: false, + isStarred: false, + }, + { + name: "check starred fails with unauthorized", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/user/starred/owner/repo", + Method: "GET", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message": "Requires authentication"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + expectError: false, // The GitHub API returns false for not authenticated, not an error + isStarred: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := IsRepositoryStarred(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 + } + + require.NoError(t, err) + + // Parse the result and get the text content if no error + textContent := getTextResult(t, result) + + // Check if the result contains the correct starred status + var expectedResult string + if tc.isStarred { + expectedResult = `{"is_starred": true}` + } else { + expectedResult = `{"is_starred": false}` + } + assert.Contains(t, textContent.Text, expectedResult) + }) + } +} + func Test_ToggleRepositoryStar(t *testing.T) { // Verify tool definition mockClient := github.NewClient(nil) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 6e4124bd4..52c829a6d 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -31,6 +31,7 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn, toolsets.NewServerTool(ListBranches(getClient, t)), toolsets.NewServerTool(ListTags(getClient, t)), toolsets.NewServerTool(GetTag(getClient, t)), + toolsets.NewServerTool(IsRepositoryStarred(getClient, t)), ). AddWriteTools( toolsets.NewServerTool(CreateOrUpdateFile(getClient, t)), From 719f30adf1d67baec78a177a6e62c3fcaa7fecbe Mon Sep 17 00:00:00 2001 From: LukasPoque Date: Wed, 21 May 2025 17:55:04 +0200 Subject: [PATCH 3/6] Change 'star' param-type from bool to string in ToggleRepositoryStar function since'false' bool parameters are treated as missing by the server --- pkg/github/repositories.go | 16 +++++++++++++--- pkg/github/repositories_test.go | 19 +++++++++++++++---- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index be2307f79..018cf68eb 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -616,9 +616,9 @@ func ToggleRepositoryStar(getClient GetClientFn, t translations.TranslationHelpe mcp.Required(), mcp.Description("Repository name"), ), - mcp.WithBoolean("star", + mcp.WithString("star", mcp.Required(), - mcp.Description("True to star, false to unstar the repository"), + mcp.Description("'true' to star, 'false' to unstar the repository"), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -630,11 +630,21 @@ func ToggleRepositoryStar(getClient GetClientFn, t translations.TranslationHelpe if err != nil { return mcp.NewToolResultError(err.Error()), nil } - star, err := requiredParam[bool](request, "star") + starStr, err := requiredParam[string](request, "star") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + var star bool + switch starStr { + case "true": + star = true + case "false": + star = false + default: + return mcp.NewToolResultError("parameter 'star' must be exactly 'true' or 'false'"), nil + } + client, err := getClient(ctx) if err != nil { return nil, fmt.Errorf("failed to get GitHub client: %w", err) diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index a31a6ae46..1f72ca90b 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -2118,7 +2118,7 @@ func Test_ToggleRepositoryStar(t *testing.T) { requestArgs: map[string]interface{}{ "owner": "owner", "repo": "repo", - "star": true, + "star": "true", }, expectError: false, expectedResult: "Successfully starred repository owner/repo", @@ -2139,7 +2139,7 @@ func Test_ToggleRepositoryStar(t *testing.T) { requestArgs: map[string]interface{}{ "owner": "owner", "repo": "repo", - "star": false, + "star": "false", }, expectError: false, expectedResult: "Successfully unstarred repository owner/repo", @@ -2161,7 +2161,7 @@ func Test_ToggleRepositoryStar(t *testing.T) { requestArgs: map[string]interface{}{ "owner": "owner", "repo": "repo", - "star": true, + "star": "true", }, expectError: true, expectedErrMsg: "failed to star repository", @@ -2183,11 +2183,22 @@ func Test_ToggleRepositoryStar(t *testing.T) { requestArgs: map[string]interface{}{ "owner": "owner", "repo": "repo", - "star": false, + "star": "false", }, expectError: true, expectedErrMsg: "failed to unstar repository", }, + { + name: "invalid star parameter", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "star": "invalid", + }, + expectError: false, // We expect a tool error, not a Go error + expectedResult: "parameter 'star' must be exactly 'true' or 'false'", + }, } for _, tc := range tests { From 37d06f8e37d93bab27471e026930cf40552cbcdb Mon Sep 17 00:00:00 2001 From: LukasPoque Date: Wed, 21 May 2025 18:39:32 +0200 Subject: [PATCH 4/6] Add tool to list repositories starred by the authenticated user --- README.md | 6 ++ pkg/github/search.go | 70 +++++++++++++++ pkg/github/search_test.go | 173 ++++++++++++++++++++++++++++++++++++++ pkg/github/tools.go | 1 + 4 files changed, 250 insertions(+) diff --git a/README.md b/README.md index 9a7534990..ff8226502 100644 --- a/README.md +++ b/README.md @@ -559,6 +559,12 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `page`: Page number (number, optional) - `perPage`: Results per page (number, optional) +- **list_starred_repositories** - List repositories starred by the authenticated user + - `sort`: How to sort the results ('created' or 'updated') (string, optional) + - `direction`: Direction to sort ('asc' or 'desc') (string, optional) + - `page`: Page number (number, optional) + - `perPage`: Results per page (number, optional) + ### Code Scanning - **get_code_scanning_alert** - Get a code scanning alert diff --git a/pkg/github/search.go b/pkg/github/search.go index ac5e2994c..369f5d338 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -222,3 +222,73 @@ func SearchUsers(getClient GetClientFn, t translations.TranslationHelperFunc) (t return mcp.NewToolResultText(string(r)), nil } } + +// ListStarredRepositories creates a tool to list repositories starred by the authenticated user. +func ListStarredRepositories(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("list_starred_repositories", + mcp.WithDescription(t("TOOL_LIST_STARRED_REPOSITORIES_DESCRIPTION", "List repositories starred by the authenticated user")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_LIST_STARRED_REPOSITORIES_USER_TITLE", "List starred repositories"), + ReadOnlyHint: toBoolPtr(true), + }), + mcp.WithString("sort", + mcp.Description("How to sort the results ('created' or 'updated')"), + mcp.Enum("created", "updated"), + ), + mcp.WithString("direction", + mcp.Description("Direction to sort ('asc' or 'desc')"), + mcp.Enum("asc", "desc"), + ), + WithPagination(), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + sort, err := OptionalParam[string](request, "sort") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + direction, err := OptionalParam[string](request, "direction") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + pagination, err := OptionalPaginationParams(request) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + opts := &github.ActivityListStarredOptions{ + Sort: sort, + Direction: direction, + ListOptions: github.ListOptions{ + Page: pagination.page, + PerPage: pagination.perPage, + }, + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + // Empty string for user parameter means the authenticated user + starredRepos, resp, err := client.Activity.ListStarred(ctx, "", opts) + if err != nil { + return nil, fmt.Errorf("failed to list starred repositories: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != 200 { + 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 starred repositories: %s", string(body))), nil + } + + r, err := json.Marshal(starredRepos) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index b61518e47..bebbd4014 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -311,6 +311,179 @@ func Test_SearchCode(t *testing.T) { } } +func Test_ListStarredRepositories(t *testing.T) { + // Verify tool definition + mockClient := github.NewClient(nil) + tool, _ := ListStarredRepositories(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "list_starred_repositories", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "sort") + assert.Contains(t, tool.InputSchema.Properties, "direction") + assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Empty(t, tool.InputSchema.Required) // No required parameters + + // Setup mock starred repositories results + mockStarredRepos := []*github.StarredRepository{ + { + StarredAt: &github.Timestamp{}, + Repository: &github.Repository{ + ID: github.Ptr(int64(12345)), + Name: github.Ptr("repo-1"), + FullName: github.Ptr("owner/repo-1"), + HTMLURL: github.Ptr("https://github.com/owner/repo-1"), + Description: github.Ptr("Test repository 1"), + StargazersCount: github.Ptr(100), + Language: github.Ptr("Go"), + Fork: github.Ptr(false), + }, + }, + { + StarredAt: &github.Timestamp{}, + Repository: &github.Repository{ + ID: github.Ptr(int64(67890)), + Name: github.Ptr("repo-2"), + FullName: github.Ptr("owner/repo-2"), + HTMLURL: github.Ptr("https://github.com/owner/repo-2"), + Description: github.Ptr("Test repository 2"), + StargazersCount: github.Ptr(50), + Language: github.Ptr("JavaScript"), + Fork: github.Ptr(true), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedResult []*github.StarredRepository + expectedErrMsg string + }{ + { + name: "successful starred repositories list with all parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUserStarred, + expectQueryParams(t, map[string]string{ + "sort": "created", + "direction": "desc", + "page": "2", + "per_page": "10", + }).andThen( + mockResponse(t, http.StatusOK, mockStarredRepos), + ), + ), + ), + requestArgs: map[string]interface{}{ + "sort": "created", + "direction": "desc", + "page": float64(2), + "perPage": float64(10), + }, + expectError: false, + expectedResult: mockStarredRepos, + }, + { + name: "list starred repositories with default parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUserStarred, + expectQueryParams(t, map[string]string{ + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockStarredRepos), + ), + ), + ), + requestArgs: map[string]interface{}{}, + expectError: false, + expectedResult: mockStarredRepos, + }, + { + name: "list starred repositories with sort only", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUserStarred, + expectQueryParams(t, map[string]string{ + "sort": "updated", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockStarredRepos), + ), + ), + ), + requestArgs: map[string]interface{}{ + "sort": "updated", + }, + expectError: false, + expectedResult: mockStarredRepos, + }, + { + name: "list starred repositories fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUserStarred, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message": "Requires authentication"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{}, + expectError: true, + expectedErrMsg: "failed to list starred repositories", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := ListStarredRepositories(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 + } + + 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 returnedResult []*github.StarredRepository + err = json.Unmarshal([]byte(textContent.Text), &returnedResult) + require.NoError(t, err) + assert.Len(t, returnedResult, len(tc.expectedResult)) + + for i, repo := range returnedResult { + assert.Equal(t, *tc.expectedResult[i].Repository.ID, *repo.Repository.ID) + assert.Equal(t, *tc.expectedResult[i].Repository.Name, *repo.Repository.Name) + assert.Equal(t, *tc.expectedResult[i].Repository.FullName, *repo.Repository.FullName) + assert.Equal(t, *tc.expectedResult[i].Repository.HTMLURL, *repo.Repository.HTMLURL) + assert.Equal(t, *tc.expectedResult[i].Repository.Description, *repo.Repository.Description) + assert.Equal(t, *tc.expectedResult[i].Repository.StargazersCount, *repo.Repository.StargazersCount) + assert.Equal(t, *tc.expectedResult[i].Repository.Language, *repo.Repository.Language) + assert.Equal(t, *tc.expectedResult[i].Repository.Fork, *repo.Repository.Fork) + } + }) + } +} + func Test_SearchUsers(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 52c829a6d..49288277d 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -57,6 +57,7 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn, users := toolsets.NewToolset("users", "GitHub User related tools"). AddReadTools( toolsets.NewServerTool(SearchUsers(getClient, t)), + toolsets.NewServerTool(ListStarredRepositories(getClient, t)), ) pullRequests := toolsets.NewToolset("pull_requests", "GitHub Pull Request related tools"). AddReadTools( From 75d0781c90f2981cb551a1efa072e03720921a78 Mon Sep 17 00:00:00 2001 From: LukasPoque Date: Thu, 22 May 2025 10:06:22 +0200 Subject: [PATCH 5/6] Simplify parameter description for ListStarredRepositories --- pkg/github/search.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/github/search.go b/pkg/github/search.go index 369f5d338..c99bb101f 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -232,11 +232,11 @@ func ListStarredRepositories(getClient GetClientFn, t translations.TranslationHe ReadOnlyHint: toBoolPtr(true), }), mcp.WithString("sort", - mcp.Description("How to sort the results ('created' or 'updated')"), + mcp.Description("How to sort the results"), mcp.Enum("created", "updated"), ), mcp.WithString("direction", - mcp.Description("Direction to sort ('asc' or 'desc')"), + mcp.Description("Direction to sort"), mcp.Enum("asc", "desc"), ), WithPagination(), From 4c07dafe52d26c9cf5b283e1448471610b6edf83 Mon Sep 17 00:00:00 2001 From: LukasPoque Date: Thu, 22 May 2025 10:07:42 +0200 Subject: [PATCH 6/6] Refactor ListStarredRepositories to return simplified repository structure limited to important repo details --- pkg/github/search.go | 45 ++++++++++++++++++++++++++++- pkg/github/search_test.go | 59 ++++++++++++++++++++++++++++++--------- 2 files changed, 90 insertions(+), 14 deletions(-) diff --git a/pkg/github/search.go b/pkg/github/search.go index c99bb101f..7159137e8 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -284,7 +284,50 @@ func ListStarredRepositories(getClient GetClientFn, t translations.TranslationHe return mcp.NewToolResultError(fmt.Sprintf("failed to list starred repositories: %s", string(body))), nil } - r, err := json.Marshal(starredRepos) + // Filter results to only include starred_at, repo.full_name, repo.html_url, repo.description, repo.stargazers_count, repo.language + // This saves context tokens, further information can be requested via repository_info tool + type SimplifiedRepo struct { + FullName string `json:"full_name,omitempty"` + HTMLURL string `json:"html_url,omitempty"` + Description string `json:"description,omitempty"` + StargazersCount int `json:"stargazers_count,omitempty"` + Language string `json:"language,omitempty"` + } + + type SimplifiedStarredRepo struct { + StarredAt *github.Timestamp `json:"starred_at,omitempty"` + Repository SimplifiedRepo `json:"repository,omitempty"` + } + + filteredRepos := make([]SimplifiedStarredRepo, 0, len(starredRepos)) + for _, repo := range starredRepos { + simplifiedRepo := SimplifiedStarredRepo{ + StarredAt: repo.StarredAt, + Repository: SimplifiedRepo{}, + } + + if repo.Repository != nil { + if repo.Repository.FullName != nil { + simplifiedRepo.Repository.FullName = *repo.Repository.FullName + } + if repo.Repository.HTMLURL != nil { + simplifiedRepo.Repository.HTMLURL = *repo.Repository.HTMLURL + } + if repo.Repository.Description != nil { + simplifiedRepo.Repository.Description = *repo.Repository.Description + } + if repo.Repository.StargazersCount != nil { + simplifiedRepo.Repository.StargazersCount = *repo.Repository.StargazersCount + } + if repo.Repository.Language != nil { + simplifiedRepo.Repository.Language = *repo.Repository.Language + } + } + + filteredRepos = append(filteredRepos, simplifiedRepo) + } + + r, err := json.Marshal(filteredRepos) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index bebbd4014..ba353fa5a 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -354,12 +354,48 @@ func Test_ListStarredRepositories(t *testing.T) { }, } + type SimplifiedRepo struct { + FullName string `json:"full_name,omitempty"` + HTMLURL string `json:"html_url,omitempty"` + Description string `json:"description,omitempty"` + StargazersCount int `json:"stargazers_count,omitempty"` + Language string `json:"language,omitempty"` + } + + type SimplifiedStarredRepo struct { + StarredAt *github.Timestamp `json:"starred_at,omitempty"` + Repository SimplifiedRepo `json:"repository,omitempty"` + } + + expectedFilteredRepos := []SimplifiedStarredRepo{ + { + StarredAt: &github.Timestamp{}, + Repository: SimplifiedRepo{ + FullName: "owner/repo-1", + HTMLURL: "https://github.com/owner/repo-1", + Description: "Test repository 1", + StargazersCount: 100, + Language: "Go", + }, + }, + { + StarredAt: &github.Timestamp{}, + Repository: SimplifiedRepo{ + FullName: "owner/repo-2", + HTMLURL: "https://github.com/owner/repo-2", + Description: "Test repository 2", + StargazersCount: 50, + Language: "JavaScript", + }, + }, + } + tests := []struct { name string mockedClient *http.Client requestArgs map[string]interface{} expectError bool - expectedResult []*github.StarredRepository + expectedResult []SimplifiedStarredRepo expectedErrMsg string }{ { @@ -384,7 +420,7 @@ func Test_ListStarredRepositories(t *testing.T) { "perPage": float64(10), }, expectError: false, - expectedResult: mockStarredRepos, + expectedResult: expectedFilteredRepos, }, { name: "list starred repositories with default parameters", @@ -401,7 +437,7 @@ func Test_ListStarredRepositories(t *testing.T) { ), requestArgs: map[string]interface{}{}, expectError: false, - expectedResult: mockStarredRepos, + expectedResult: expectedFilteredRepos, }, { name: "list starred repositories with sort only", @@ -421,7 +457,7 @@ func Test_ListStarredRepositories(t *testing.T) { "sort": "updated", }, expectError: false, - expectedResult: mockStarredRepos, + expectedResult: expectedFilteredRepos, }, { name: "list starred repositories fails", @@ -465,20 +501,17 @@ func Test_ListStarredRepositories(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result - var returnedResult []*github.StarredRepository + var returnedResult []SimplifiedStarredRepo err = json.Unmarshal([]byte(textContent.Text), &returnedResult) require.NoError(t, err) assert.Len(t, returnedResult, len(tc.expectedResult)) for i, repo := range returnedResult { - assert.Equal(t, *tc.expectedResult[i].Repository.ID, *repo.Repository.ID) - assert.Equal(t, *tc.expectedResult[i].Repository.Name, *repo.Repository.Name) - assert.Equal(t, *tc.expectedResult[i].Repository.FullName, *repo.Repository.FullName) - assert.Equal(t, *tc.expectedResult[i].Repository.HTMLURL, *repo.Repository.HTMLURL) - assert.Equal(t, *tc.expectedResult[i].Repository.Description, *repo.Repository.Description) - assert.Equal(t, *tc.expectedResult[i].Repository.StargazersCount, *repo.Repository.StargazersCount) - assert.Equal(t, *tc.expectedResult[i].Repository.Language, *repo.Repository.Language) - assert.Equal(t, *tc.expectedResult[i].Repository.Fork, *repo.Repository.Fork) + assert.Equal(t, tc.expectedResult[i].Repository.FullName, repo.Repository.FullName) + assert.Equal(t, tc.expectedResult[i].Repository.HTMLURL, repo.Repository.HTMLURL) + assert.Equal(t, tc.expectedResult[i].Repository.Description, repo.Repository.Description) + assert.Equal(t, tc.expectedResult[i].Repository.StargazersCount, repo.Repository.StargazersCount) + assert.Equal(t, tc.expectedResult[i].Repository.Language, repo.Repository.Language) } }) }

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