From 6debfd6d80eb4f31305dfca2e4d44f5414da5529 Mon Sep 17 00:00:00 2001 From: shreyabaid007 Date: Mon, 4 Aug 2025 17:33:27 +0530 Subject: [PATCH 1/4] feat: add list_starred_repositories tool Add new tool to list starred repositories for a given user with support for: - Sorting by created or updated date - Ascending or descending order - Pagination with configurable page size - Comprehensive test coverage with snapshot testing --- README.md | 7 + .../list_starred_repositories.snap | 47 ++ pkg/github/starred_repos.go | 116 +++++ pkg/github/starred_repos_test.go | 462 ++++++++++++++++++ pkg/github/tools.go | 1 + 5 files changed, 633 insertions(+) create mode 100644 pkg/github/__toolsnaps__/list_starred_repositories.snap create mode 100644 pkg/github/starred_repos.go create mode 100644 pkg/github/starred_repos_test.go diff --git a/README.md b/README.md index b40974e20..eb1ae6fc9 100644 --- a/README.md +++ b/README.md @@ -848,6 +848,13 @@ The following sets of tools are available (all are on by default): - `repo`: Repository name (string, required) - `sha`: Commit SHA, branch or tag name to list commits of. If not provided, uses the default branch of the repository. If a commit SHA is provided, will list commits up to that SHA. (string, optional) +- **list_starred_repositories** - List starred repositories + - `direction`: Direction to sort repositories. Can be 'asc' or 'desc'. Default is 'desc'. (string, optional) + - `page`: Page number for pagination (min 1) (number, optional) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) + - `sort`: Sort order for repositories. Can be 'created' (when the repository was starred) or 'updated' (when the repository was last pushed to). Default is 'created'. (string, optional) + - `username`: GitHub username of the user whose starred repositories to list (string, required) + - **list_tags** - List tags - `owner`: Repository owner (string, required) - `page`: Page number for pagination (min 1) (number, optional) diff --git a/pkg/github/__toolsnaps__/list_starred_repositories.snap b/pkg/github/__toolsnaps__/list_starred_repositories.snap new file mode 100644 index 000000000..3c4b107c2 --- /dev/null +++ b/pkg/github/__toolsnaps__/list_starred_repositories.snap @@ -0,0 +1,47 @@ +{ + "annotations": { + "title": "List starred repositories", + "readOnlyHint": true + }, + "description": "List repositories that a user has starred on GitHub. Returns at least 30 results per page by default, but can return more if specified using the perPage parameter (up to 100).", + "inputSchema": { + "properties": { + "direction": { + "description": "Direction to sort repositories. Can be 'asc' or 'desc'. Default is 'desc'.", + "enum": [ + "asc", + "desc" + ], + "type": "string" + }, + "page": { + "description": "Page number for pagination (min 1)", + "minimum": 1, + "type": "number" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "sort": { + "description": "Sort order for repositories. Can be 'created' (when the repository was starred) or 'updated' (when the repository was last pushed to). Default is 'created'.", + "enum": [ + "created", + "updated" + ], + "type": "string" + }, + "username": { + "description": "GitHub username of the user whose starred repositories to list", + "type": "string" + } + }, + "required": [ + "username" + ], + "type": "object" + }, + "name": "list_starred_repositories" +} \ No newline at end of file diff --git a/pkg/github/starred_repos.go b/pkg/github/starred_repos.go new file mode 100644 index 000000000..e92832804 --- /dev/null +++ b/pkg/github/starred_repos.go @@ -0,0 +1,116 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v73/github" + "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" +) + +// ListStarredRepositories creates a tool to list repositories that a user has starred. +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 that a user has starred on GitHub. Returns at least 30 results per page by default, but can return more if specified using the perPage parameter (up to 100).")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_LIST_STARRED_REPOSITORIES_USER_TITLE", "List starred repositories"), + ReadOnlyHint: ToBoolPtr(true), + }), + mcp.WithString("username", + mcp.Required(), + mcp.Description("GitHub username of the user whose starred repositories to list"), + ), + mcp.WithString("sort", + mcp.Description("Sort order for repositories. Can be 'created' (when the repository was starred) or 'updated' (when the repository was last pushed to). Default is 'created'."), + mcp.Enum("created", "updated"), + ), + mcp.WithString("direction", + mcp.Description("Direction to sort repositories. Can be 'asc' or 'desc'. Default is 'desc'."), + mcp.Enum("asc", "desc"), + ), + WithPagination(), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + username, err := RequiredParam[string](request, "username") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + 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 + } + // Validate sort parameter + if sort != "" && sort != "created" && sort != "updated" { + return mcp.NewToolResultError("invalid value for sort parameter"), nil + } + + // Validate direction parameter + if direction != "" && direction != "asc" && direction != "desc" { + return mcp.NewToolResultError("invalid value for direction parameter"), nil + } + + // Validate pagination parameters directly from request before using OptionalPaginationParams + if pageVal, ok := request.GetArguments()["page"].(float64); ok { + if pageVal <= 0 { + return mcp.NewToolResultError("page must be greater than 0"), nil + } + } + + if perPageVal, ok := request.GetArguments()["perPage"].(float64); ok { + if perPageVal <= 0 || perPageVal > 100 { + return mcp.NewToolResultError("perPage must be between 1 and 100"), nil + } + } + + pagination, err := OptionalPaginationParams(request) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Set default perPage to 30 if not provided (this should not happen due to validation above) + perPage := pagination.PerPage + if perPage == 0 { + perPage = 30 + } + + // Set default page to 1 if not provided (this should not happen due to validation above) + page := pagination.Page + if page == 0 { + page = 1 + } + + opts := &github.ActivityListStarredOptions{ + Sort: sort, + Direction: direction, + ListOptions: github.ListOptions{ + Page: page, + PerPage: perPage, + }, + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + starredRepos, resp, err := client.Activity.ListStarred(ctx, username, opts) + if err != nil { + return nil, fmt.Errorf("failed to list starred repositories for user: %s: %w", username, err) + } + defer func() { _ = resp.Body.Close() }() + + 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/starred_repos_test.go b/pkg/github/starred_repos_test.go new file mode 100644 index 000000000..e0e0eba81 --- /dev/null +++ b/pkg/github/starred_repos_test.go @@ -0,0 +1,462 @@ +package github + +import ( + "context" + "encoding/json" + "net/http" + "testing" + "time" + + "github.com/github/github-mcp-server/internal/toolsnaps" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v73/github" + "github.com/migueleliasweb/go-github-mock/src/mock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_ListStarredRepositories(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := ListStarredRepositories(stubGetClientFn(mockClient), translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "list_starred_repositories", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "username") + 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.ElementsMatch(t, tool.InputSchema.Required, []string{"username"}) + + // Setup mock starred repositories for success case + mockStarredRepos := []*github.StarredRepository{ + { + Repository: &github.Repository{ + ID: github.Ptr(int64(1)), + Name: github.Ptr("awesome-repo"), + FullName: github.Ptr("owner/awesome-repo"), + Owner: &github.User{ + Login: github.Ptr("owner"), + }, + Description: github.Ptr("An awesome repository"), + HTMLURL: github.Ptr("https://github.com/owner/awesome-repo"), + StargazersCount: github.Ptr(100), + Language: github.Ptr("Go"), + Fork: github.Ptr(false), + Private: github.Ptr(false), + }, + StarredAt: &github.Timestamp{Time: time.Now().Add(-24 * time.Hour)}, + }, + { + Repository: &github.Repository{ + ID: github.Ptr(int64(2)), + Name: github.Ptr("cool-project"), + FullName: github.Ptr("another/cool-project"), + Owner: &github.User{ + Login: github.Ptr("another"), + }, + Description: github.Ptr("A cool project"), + HTMLURL: github.Ptr("https://github.com/another/cool-project"), + StargazersCount: github.Ptr(250), + Language: github.Ptr("JavaScript"), + Fork: github.Ptr(true), + Private: github.Ptr(false), + }, + StarredAt: &github.Timestamp{Time: time.Now().Add(-48 * time.Hour)}, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedStarredRepos []*github.StarredRepository + expectedErrMsg string + }{ + { + name: "successful starred repositories retrieval with minimal parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetUsersStarredByUsername, + mockStarredRepos, + ), + ), + requestArgs: map[string]interface{}{ + "username": "testuser", + }, + expectError: false, + expectedStarredRepos: mockStarredRepos, + }, + { + name: "successful starred repositories retrieval with all parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUsersStarredByUsername, + expectQueryParams(t, map[string]string{ + "sort": "created", + "direction": "desc", + "page": "2", + "per_page": "50", + }).andThen( + mockResponse(t, http.StatusOK, mockStarredRepos), + ), + ), + ), + requestArgs: map[string]interface{}{ + "username": "testuser", + "sort": "created", + "direction": "desc", + "page": float64(2), + "perPage": float64(50), + }, + expectError: false, + expectedStarredRepos: mockStarredRepos, + }, + { + name: "successful starred repositories retrieval with sort updated", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUsersStarredByUsername, + expectQueryParams(t, map[string]string{ + "sort": "updated", + "direction": "asc", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockStarredRepos), + ), + ), + ), + requestArgs: map[string]interface{}{ + "username": "testuser", + "sort": "updated", + "direction": "asc", + }, + expectError: false, + expectedStarredRepos: mockStarredRepos, + }, + { + name: "successful starred repositories retrieval with default perPage", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUsersStarredByUsername, + expectQueryParams(t, map[string]string{ + "page": "1", + "per_page": "30", // Default perPage should be 30 + }).andThen( + mockResponse(t, http.StatusOK, mockStarredRepos), + ), + ), + ), + requestArgs: map[string]interface{}{ + "username": "testuser", + "page": float64(1), + }, + expectError: false, + expectedStarredRepos: mockStarredRepos, + }, + { + name: "empty starred repositories list", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetUsersStarredByUsername, + []*github.StarredRepository{}, + ), + ), + requestArgs: map[string]interface{}{ + "username": "userwithnorepos", + }, + expectError: false, + expectedStarredRepos: []*github.StarredRepository{}, + }, + { + name: "user not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUsersStarredByUsername, + mockResponse(t, http.StatusNotFound, `{"message": "Not Found"}`), + ), + ), + requestArgs: map[string]interface{}{ + "username": "nonexistentuser", + }, + expectError: true, + expectedErrMsg: "failed to list starred repositories for user", + }, + { + name: "rate limit exceeded", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUsersStarredByUsername, + mockResponse(t, http.StatusForbidden, `{"message": "API rate limit exceeded"}`), + ), + ), + requestArgs: map[string]interface{}{ + "username": "testuser", + }, + expectError: true, + expectedErrMsg: "failed to list starred repositories for user", + }, + { + name: "missing required parameter username", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{}, + expectError: false, + expectedErrMsg: "missing required parameter: username", + }, + { + name: "invalid sort parameter", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{ + "username": "testuser", + "sort": "invalid", + }, + expectError: false, + expectedErrMsg: "invalid value for sort parameter", + }, + { + name: "invalid direction parameter", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{ + "username": "testuser", + "direction": "invalid", + }, + expectError: false, + expectedErrMsg: "invalid value for direction parameter", + }, + } + + 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 + } + + 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 returnedStarredRepos []*github.StarredRepository + err = json.Unmarshal([]byte(textContent.Text), &returnedStarredRepos) + require.NoError(t, err) + + assert.Len(t, returnedStarredRepos, len(tc.expectedStarredRepos)) + for i, starredRepo := range returnedStarredRepos { + if i < len(tc.expectedStarredRepos) { + assert.Equal(t, *tc.expectedStarredRepos[i].Repository.ID, *starredRepo.Repository.ID) + assert.Equal(t, *tc.expectedStarredRepos[i].Repository.Name, *starredRepo.Repository.Name) + assert.Equal(t, *tc.expectedStarredRepos[i].Repository.FullName, *starredRepo.Repository.FullName) + assert.Equal(t, *tc.expectedStarredRepos[i].Repository.Owner.Login, *starredRepo.Repository.Owner.Login) + assert.Equal(t, *tc.expectedStarredRepos[i].Repository.HTMLURL, *starredRepo.Repository.HTMLURL) + assert.NotNil(t, starredRepo.StarredAt) + + if tc.expectedStarredRepos[i].Repository.Description != nil { + assert.Equal(t, *tc.expectedStarredRepos[i].Repository.Description, *starredRepo.Repository.Description) + } + if tc.expectedStarredRepos[i].Repository.Language != nil { + assert.Equal(t, *tc.expectedStarredRepos[i].Repository.Language, *starredRepo.Repository.Language) + } + } + } + }) + } +} + +func Test_ListStarredRepositories_ParameterValidation(t *testing.T) { + // Test parameter validation without making HTTP requests + mockClient := github.NewClient(nil) + _, handler := ListStarredRepositories(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + tests := []struct { + name string + requestArgs map[string]interface{} + expectedErr string + }{ + { + name: "valid sort parameter - created", + requestArgs: map[string]interface{}{ + "username": "testuser", + "sort": "created", + }, + expectedErr: "", + }, + { + name: "valid sort parameter - updated", + requestArgs: map[string]interface{}{ + "username": "testuser", + "sort": "updated", + }, + expectedErr: "", + }, + { + name: "valid direction parameter - asc", + requestArgs: map[string]interface{}{ + "username": "testuser", + "direction": "asc", + }, + expectedErr: "", + }, + { + name: "valid direction parameter - desc", + requestArgs: map[string]interface{}{ + "username": "testuser", + "direction": "desc", + }, + expectedErr: "", + }, + { + name: "invalid sort parameter", + requestArgs: map[string]interface{}{ + "username": "testuser", + "sort": "popularity", + }, + expectedErr: "invalid value for sort parameter", + }, + { + name: "invalid direction parameter", + requestArgs: map[string]interface{}{ + "username": "testuser", + "direction": "random", + }, + expectedErr: "invalid value for direction parameter", + }, + { + name: "negative page parameter", + requestArgs: map[string]interface{}{ + "username": "testuser", + "page": float64(-1), + }, + expectedErr: "page must be greater than 0", + }, + { + name: "zero page parameter", + requestArgs: map[string]interface{}{ + "username": "testuser", + "page": float64(0), + }, + expectedErr: "page must be greater than 0", + }, + { + name: "negative perPage parameter", + requestArgs: map[string]interface{}{ + "username": "testuser", + "perPage": float64(-1), + }, + expectedErr: "perPage must be between 1 and 100", + }, + { + name: "zero perPage parameter", + requestArgs: map[string]interface{}{ + "username": "testuser", + "perPage": float64(0), + }, + expectedErr: "perPage must be between 1 and 100", + }, + { + name: "perPage parameter too large", + requestArgs: map[string]interface{}{ + "username": "testuser", + "perPage": float64(101), + }, + expectedErr: "perPage must be between 1 and 100", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + if tc.expectedErr != "" { + // Should return a tool error, not a Go error + require.NoError(t, err) + require.NotNil(t, result) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErr) + } else if err != nil { + // Should not return an error for valid parameters + // (though it might fail due to network issues in this test setup) + // If there's an error, it should be a network/client error, not a validation error + assert.NotContains(t, err.Error(), "invalid value") + assert.NotContains(t, err.Error(), "must be") + } + }) + } +} + +func Test_ListStarredRepositories_DefaultPerPage(t *testing.T) { + // Test that default perPage is set to 30 when not provided + mockStarredRepos := []*github.StarredRepository{ + { + Repository: &github.Repository{ + ID: github.Ptr(int64(1)), + Name: github.Ptr("test-repo"), + FullName: github.Ptr("owner/test-repo"), + Owner: &github.User{ + Login: github.Ptr("owner"), + }, + HTMLURL: github.Ptr("https://github.com/owner/test-repo"), + }, + StarredAt: &github.Timestamp{Time: time.Now()}, + }, + } + + mockedClient := mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUsersStarredByUsername, + expectQueryParams(t, map[string]string{ + "page": "1", + "per_page": "30", // Should default to 30 + }).andThen( + mockResponse(t, http.StatusOK, mockStarredRepos), + ), + ), + ) + + client := github.NewClient(mockedClient) + _, handler := ListStarredRepositories(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request without perPage parameter + request := createMCPRequest(map[string]interface{}{ + "username": "testuser", + }) + + // Call handler + result, err := handler(context.Background(), request) + + require.NoError(t, err) + textContent := getTextResult(t, result) + + var returnedStarredRepos []*github.StarredRepository + err = json.Unmarshal([]byte(textContent.Text), &returnedStarredRepos) + require.NoError(t, err) + assert.Len(t, returnedStarredRepos, 1) +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 7fb1d39c0..cb0e53edb 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -31,6 +31,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(ListBranches(getClient, t)), toolsets.NewServerTool(ListTags(getClient, t)), toolsets.NewServerTool(GetTag(getClient, t)), + toolsets.NewServerTool(ListStarredRepositories(getClient, t)), ). AddWriteTools( toolsets.NewServerTool(CreateOrUpdateFile(getClient, t)), From f7650d2d893f2252ad5f02d2d97eb2f1a8eaa475 Mon Sep 17 00:00:00 2001 From: shreyabaid007 Date: Mon, 4 Aug 2025 17:44:10 +0530 Subject: [PATCH 2/4] Remove snap file --- .../list_starred_repositories.snap | 47 ------------------- 1 file changed, 47 deletions(-) delete mode 100644 pkg/github/__toolsnaps__/list_starred_repositories.snap diff --git a/pkg/github/__toolsnaps__/list_starred_repositories.snap b/pkg/github/__toolsnaps__/list_starred_repositories.snap deleted file mode 100644 index 3c4b107c2..000000000 --- a/pkg/github/__toolsnaps__/list_starred_repositories.snap +++ /dev/null @@ -1,47 +0,0 @@ -{ - "annotations": { - "title": "List starred repositories", - "readOnlyHint": true - }, - "description": "List repositories that a user has starred on GitHub. Returns at least 30 results per page by default, but can return more if specified using the perPage parameter (up to 100).", - "inputSchema": { - "properties": { - "direction": { - "description": "Direction to sort repositories. Can be 'asc' or 'desc'. Default is 'desc'.", - "enum": [ - "asc", - "desc" - ], - "type": "string" - }, - "page": { - "description": "Page number for pagination (min 1)", - "minimum": 1, - "type": "number" - }, - "perPage": { - "description": "Results per page for pagination (min 1, max 100)", - "maximum": 100, - "minimum": 1, - "type": "number" - }, - "sort": { - "description": "Sort order for repositories. Can be 'created' (when the repository was starred) or 'updated' (when the repository was last pushed to). Default is 'created'.", - "enum": [ - "created", - "updated" - ], - "type": "string" - }, - "username": { - "description": "GitHub username of the user whose starred repositories to list", - "type": "string" - } - }, - "required": [ - "username" - ], - "type": "object" - }, - "name": "list_starred_repositories" -} \ No newline at end of file From 6631a3e00caac3fc9af940b8080d613552f00ca4 Mon Sep 17 00:00:00 2001 From: shreyabaid007 Date: Mon, 4 Aug 2025 17:57:36 +0530 Subject: [PATCH 3/4] remove code duplication and redundant validation --- pkg/github/starred_repos.go | 25 ---- pkg/github/starred_repos_test.go | 201 ------------------------------- 2 files changed, 226 deletions(-) diff --git a/pkg/github/starred_repos.go b/pkg/github/starred_repos.go index e92832804..ab158e6b2 100644 --- a/pkg/github/starred_repos.go +++ b/pkg/github/starred_repos.go @@ -46,41 +46,16 @@ func ListStarredRepositories(getClient GetClientFn, t translations.TranslationHe if err != nil { return mcp.NewToolResultError(err.Error()), nil } - // Validate sort parameter - if sort != "" && sort != "created" && sort != "updated" { - return mcp.NewToolResultError("invalid value for sort parameter"), nil - } - - // Validate direction parameter - if direction != "" && direction != "asc" && direction != "desc" { - return mcp.NewToolResultError("invalid value for direction parameter"), nil - } - - // Validate pagination parameters directly from request before using OptionalPaginationParams - if pageVal, ok := request.GetArguments()["page"].(float64); ok { - if pageVal <= 0 { - return mcp.NewToolResultError("page must be greater than 0"), nil - } - } - - if perPageVal, ok := request.GetArguments()["perPage"].(float64); ok { - if perPageVal <= 0 || perPageVal > 100 { - return mcp.NewToolResultError("perPage must be between 1 and 100"), nil - } - } - pagination, err := OptionalPaginationParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } - // Set default perPage to 30 if not provided (this should not happen due to validation above) perPage := pagination.PerPage if perPage == 0 { perPage = 30 } - // Set default page to 1 if not provided (this should not happen due to validation above) page := pagination.Page if page == 0 { page = 1 diff --git a/pkg/github/starred_repos_test.go b/pkg/github/starred_repos_test.go index e0e0eba81..fbe1eb38c 100644 --- a/pkg/github/starred_repos_test.go +++ b/pkg/github/starred_repos_test.go @@ -200,33 +200,6 @@ func Test_ListStarredRepositories(t *testing.T) { expectError: true, expectedErrMsg: "failed to list starred repositories for user", }, - { - name: "missing required parameter username", - mockedClient: mock.NewMockedHTTPClient(), - requestArgs: map[string]interface{}{}, - expectError: false, - expectedErrMsg: "missing required parameter: username", - }, - { - name: "invalid sort parameter", - mockedClient: mock.NewMockedHTTPClient(), - requestArgs: map[string]interface{}{ - "username": "testuser", - "sort": "invalid", - }, - expectError: false, - expectedErrMsg: "invalid value for sort parameter", - }, - { - name: "invalid direction parameter", - mockedClient: mock.NewMockedHTTPClient(), - requestArgs: map[string]interface{}{ - "username": "testuser", - "direction": "invalid", - }, - expectError: false, - expectedErrMsg: "invalid value for direction parameter", - }, } for _, tc := range tests { @@ -286,177 +259,3 @@ func Test_ListStarredRepositories(t *testing.T) { }) } } - -func Test_ListStarredRepositories_ParameterValidation(t *testing.T) { - // Test parameter validation without making HTTP requests - mockClient := github.NewClient(nil) - _, handler := ListStarredRepositories(stubGetClientFn(mockClient), translations.NullTranslationHelper) - - tests := []struct { - name string - requestArgs map[string]interface{} - expectedErr string - }{ - { - name: "valid sort parameter - created", - requestArgs: map[string]interface{}{ - "username": "testuser", - "sort": "created", - }, - expectedErr: "", - }, - { - name: "valid sort parameter - updated", - requestArgs: map[string]interface{}{ - "username": "testuser", - "sort": "updated", - }, - expectedErr: "", - }, - { - name: "valid direction parameter - asc", - requestArgs: map[string]interface{}{ - "username": "testuser", - "direction": "asc", - }, - expectedErr: "", - }, - { - name: "valid direction parameter - desc", - requestArgs: map[string]interface{}{ - "username": "testuser", - "direction": "desc", - }, - expectedErr: "", - }, - { - name: "invalid sort parameter", - requestArgs: map[string]interface{}{ - "username": "testuser", - "sort": "popularity", - }, - expectedErr: "invalid value for sort parameter", - }, - { - name: "invalid direction parameter", - requestArgs: map[string]interface{}{ - "username": "testuser", - "direction": "random", - }, - expectedErr: "invalid value for direction parameter", - }, - { - name: "negative page parameter", - requestArgs: map[string]interface{}{ - "username": "testuser", - "page": float64(-1), - }, - expectedErr: "page must be greater than 0", - }, - { - name: "zero page parameter", - requestArgs: map[string]interface{}{ - "username": "testuser", - "page": float64(0), - }, - expectedErr: "page must be greater than 0", - }, - { - name: "negative perPage parameter", - requestArgs: map[string]interface{}{ - "username": "testuser", - "perPage": float64(-1), - }, - expectedErr: "perPage must be between 1 and 100", - }, - { - name: "zero perPage parameter", - requestArgs: map[string]interface{}{ - "username": "testuser", - "perPage": float64(0), - }, - expectedErr: "perPage must be between 1 and 100", - }, - { - name: "perPage parameter too large", - requestArgs: map[string]interface{}{ - "username": "testuser", - "perPage": float64(101), - }, - expectedErr: "perPage must be between 1 and 100", - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - // Create call request - request := createMCPRequest(tc.requestArgs) - - // Call handler - result, err := handler(context.Background(), request) - - if tc.expectedErr != "" { - // Should return a tool error, not a Go error - require.NoError(t, err) - require.NotNil(t, result) - textContent := getTextResult(t, result) - assert.Contains(t, textContent.Text, tc.expectedErr) - } else if err != nil { - // Should not return an error for valid parameters - // (though it might fail due to network issues in this test setup) - // If there's an error, it should be a network/client error, not a validation error - assert.NotContains(t, err.Error(), "invalid value") - assert.NotContains(t, err.Error(), "must be") - } - }) - } -} - -func Test_ListStarredRepositories_DefaultPerPage(t *testing.T) { - // Test that default perPage is set to 30 when not provided - mockStarredRepos := []*github.StarredRepository{ - { - Repository: &github.Repository{ - ID: github.Ptr(int64(1)), - Name: github.Ptr("test-repo"), - FullName: github.Ptr("owner/test-repo"), - Owner: &github.User{ - Login: github.Ptr("owner"), - }, - HTMLURL: github.Ptr("https://github.com/owner/test-repo"), - }, - StarredAt: &github.Timestamp{Time: time.Now()}, - }, - } - - mockedClient := mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.GetUsersStarredByUsername, - expectQueryParams(t, map[string]string{ - "page": "1", - "per_page": "30", // Should default to 30 - }).andThen( - mockResponse(t, http.StatusOK, mockStarredRepos), - ), - ), - ) - - client := github.NewClient(mockedClient) - _, handler := ListStarredRepositories(stubGetClientFn(client), translations.NullTranslationHelper) - - // Create call request without perPage parameter - request := createMCPRequest(map[string]interface{}{ - "username": "testuser", - }) - - // Call handler - result, err := handler(context.Background(), request) - - require.NoError(t, err) - textContent := getTextResult(t, result) - - var returnedStarredRepos []*github.StarredRepository - err = json.Unmarshal([]byte(textContent.Text), &returnedStarredRepos) - require.NoError(t, err) - assert.Len(t, returnedStarredRepos, 1) -} From 87c77cc49761c023e0c9180dadae9918b00a51dc Mon Sep 17 00:00:00 2001 From: shreyabaid007 Date: Mon, 4 Aug 2025 17:58:29 +0530 Subject: [PATCH 4/4] add tool snap for list starred repos --- .../list_starred_repositories.snap | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 pkg/github/__toolsnaps__/list_starred_repositories.snap diff --git a/pkg/github/__toolsnaps__/list_starred_repositories.snap b/pkg/github/__toolsnaps__/list_starred_repositories.snap new file mode 100644 index 000000000..3c4b107c2 --- /dev/null +++ b/pkg/github/__toolsnaps__/list_starred_repositories.snap @@ -0,0 +1,47 @@ +{ + "annotations": { + "title": "List starred repositories", + "readOnlyHint": true + }, + "description": "List repositories that a user has starred on GitHub. Returns at least 30 results per page by default, but can return more if specified using the perPage parameter (up to 100).", + "inputSchema": { + "properties": { + "direction": { + "description": "Direction to sort repositories. Can be 'asc' or 'desc'. Default is 'desc'.", + "enum": [ + "asc", + "desc" + ], + "type": "string" + }, + "page": { + "description": "Page number for pagination (min 1)", + "minimum": 1, + "type": "number" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "sort": { + "description": "Sort order for repositories. Can be 'created' (when the repository was starred) or 'updated' (when the repository was last pushed to). Default is 'created'.", + "enum": [ + "created", + "updated" + ], + "type": "string" + }, + "username": { + "description": "GitHub username of the user whose starred repositories to list", + "type": "string" + } + }, + "required": [ + "username" + ], + "type": "object" + }, + "name": "list_starred_repositories" +} \ No newline at end of file