Skip to content

pkg/github: fix use of per page parameter #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 7, 2025
Merged

pkg/github: fix use of per page parameter #137

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a08c8a5
pkg/github: fix use of per page parameter
AlexanderYastrebov Apr 6, 2025
ba5ace3
Test issue search query params
williammartin Apr 7, 2025
1afbf53
Add pagination helper
williammartin Apr 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
@@ -204,7 +204,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
- `sort`: Sort field (string, optional)
- `order`: Sort order (string, optional)
- `page`: Page number (number, optional)
- `per_page`: Results per page (number, optional)
- `perPage`: Results per page (number, optional)

### Pull Requests

29 changes: 6 additions & 23 deletions pkg/github/issues.go
View file
Original file line number Diff line number Diff line change
@@ -162,15 +162,7 @@ func searchIssues(client *github.Client, t translations.TranslationHelperFunc) (
mcp.Description("Sort order ('asc' or 'desc')"),
mcp.Enum("asc", "desc"),
),
mcp.WithNumber("per_page",
mcp.Description("Results per page (max 100)"),
mcp.Min(1),
mcp.Max(100),
),
mcp.WithNumber("page",
mcp.Description("Page number"),
mcp.Min(1),
),
withPagination(),
),
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
query, err := requiredParam[string](request, "q")
@@ -185,11 +177,7 @@ func searchIssues(client *github.Client, t translations.TranslationHelperFunc) (
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
perPage, err := optionalIntParamWithDefault(request, "per_page", 30)
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
page, err := optionalIntParamWithDefault(request, "page", 1)
pagination, err := optionalPaginationParams(request)
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
@@ -198,8 +186,8 @@ func searchIssues(client *github.Client, t translations.TranslationHelperFunc) (
Sort: sort,
Order: order,
ListOptions: github.ListOptions{
PerPage: perPage,
Page: page,
PerPage: pagination.perPage,
Page: pagination.page,
},
}

@@ -375,12 +363,7 @@ func listIssues(client *github.Client, t translations.TranslationHelperFunc) (to
mcp.WithString("since",
mcp.Description("Filter by date (ISO 8601 timestamp)"),
),
mcp.WithNumber("page",
mcp.Description("Page number"),
),
mcp.WithNumber("per_page",
mcp.Description("Results per page"),
),
withPagination(),
),
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
owner, err := requiredParam[string](request, "owner")
@@ -432,7 +415,7 @@ func listIssues(client *github.Client, t translations.TranslationHelperFunc) (to
opts.Page = int(page)
}

if perPage, ok := request.Params.Arguments["per_page"].(float64); ok {
if perPage, ok := request.Params.Arguments["perPage"].(float64); ok {
opts.PerPage = int(perPage)
}

31 changes: 21 additions & 10 deletions pkg/github/issues_test.go
View file
Original file line number Diff line number Diff line change
@@ -244,7 +244,7 @@ func Test_SearchIssues(t *testing.T) {
assert.Contains(t, tool.InputSchema.Properties, "q")
assert.Contains(t, tool.InputSchema.Properties, "sort")
assert.Contains(t, tool.InputSchema.Properties, "order")
assert.Contains(t, tool.InputSchema.Properties, "per_page")
assert.Contains(t, tool.InputSchema.Properties, "perPage")
assert.Contains(t, tool.InputSchema.Properties, "page")
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"q"})

@@ -289,17 +289,28 @@ func Test_SearchIssues(t *testing.T) {
{
name: "successful issues search with all parameters",
mockedClient: mock.NewMockedHTTPClient(
mock.WithRequestMatch(
mock.WithRequestMatchHandler(
mock.GetSearchIssues,
mockSearchResult,
expectQueryParams(
t,
map[string]string{
"q": "repo:owner/repo is:issue is:open",
"sort": "created",
"order": "desc",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
),
),
),
requestArgs: map[string]interface{}{
"q": "repo:owner/repo is:issue is:open",
"sort": "created",
"order": "desc",
"page": float64(1),
"per_page": float64(30),
"q": "repo:owner/repo is:issue is:open",
"sort": "created",
"order": "desc",
"page": float64(1),
"perPage": float64(30),
},
expectError: false,
expectedResult: mockSearchResult,
@@ -567,7 +578,7 @@ func Test_ListIssues(t *testing.T) {
assert.Contains(t, tool.InputSchema.Properties, "direction")
assert.Contains(t, tool.InputSchema.Properties, "since")
assert.Contains(t, tool.InputSchema.Properties, "page")
assert.Contains(t, tool.InputSchema.Properties, "per_page")
assert.Contains(t, tool.InputSchema.Properties, "perPage")
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"})

// Setup mock issues for success case
@@ -641,7 +652,7 @@ func Test_ListIssues(t *testing.T) {
"direction": "desc",
"since": "2023-01-01T00:00:00Z",
"page": float64(1),
"per_page": float64(30),
"perPage": float64(30),
},
expectError: false,
expectedIssues: mockIssues,
17 changes: 4 additions & 13 deletions pkg/github/pullrequests.go
View file
Original file line number Diff line number Diff line change
@@ -94,12 +94,7 @@ func listPullRequests(client *github.Client, t translations.TranslationHelperFun
mcp.WithString("direction",
mcp.Description("Sort direction ('asc', 'desc')"),
),
mcp.WithNumber("per_page",
mcp.Description("Results per page (max 100)"),
),
mcp.WithNumber("page",
mcp.Description("Page number"),
),
withPagination(),
),
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
owner, err := requiredParam[string](request, "owner")
@@ -130,11 +125,7 @@ func listPullRequests(client *github.Client, t translations.TranslationHelperFun
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
perPage, err := optionalIntParamWithDefault(request, "per_page", 30)
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
page, err := optionalIntParamWithDefault(request, "page", 1)
pagination, err := optionalPaginationParams(request)
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
@@ -146,8 +137,8 @@ func listPullRequests(client *github.Client, t translations.TranslationHelperFun
Sort: sort,
Direction: direction,
ListOptions: github.ListOptions{
PerPage: perPage,
Page: page,
PerPage: pagination.perPage,
Page: pagination.page,
},
}

4 changes: 2 additions & 2 deletions pkg/github/pullrequests_test.go
View file
Original file line number Diff line number Diff line change
@@ -140,7 +140,7 @@ func Test_ListPullRequests(t *testing.T) {
assert.Contains(t, tool.InputSchema.Properties, "base")
assert.Contains(t, tool.InputSchema.Properties, "sort")
assert.Contains(t, tool.InputSchema.Properties, "direction")
assert.Contains(t, tool.InputSchema.Properties, "per_page")
assert.Contains(t, tool.InputSchema.Properties, "perPage")
assert.Contains(t, tool.InputSchema.Properties, "page")
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"})

@@ -190,7 +190,7 @@ func Test_ListPullRequests(t *testing.T) {
"state": "all",
"sort": "created",
"direction": "desc",
"per_page": float64(30),
"perPage": float64(30),
"page": float64(1),
},
expectError: false,
17 changes: 4 additions & 13 deletions pkg/github/repositories.go
View file
Original file line number Diff line number Diff line change
@@ -28,12 +28,7 @@ func listCommits(client *github.Client, t translations.TranslationHelperFunc) (t
mcp.WithString("sha",
mcp.Description("Branch name"),
),
mcp.WithNumber("page",
mcp.Description("Page number"),
),
mcp.WithNumber("perPage",
mcp.Description("Number of records per page"),
),
withPagination(),
),
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
owner, err := requiredParam[string](request, "owner")
@@ -48,20 +43,16 @@ func listCommits(client *github.Client, t translations.TranslationHelperFunc) (t
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
page, err := optionalIntParamWithDefault(request, "page", 1)
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
perPage, err := optionalIntParamWithDefault(request, "per_page", 30)
pagination, err := optionalPaginationParams(request)
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}

opts := &github.CommitsListOptions{
SHA: sha,
ListOptions: github.ListOptions{
Page: page,
PerPage: perPage,
Page: pagination.page,
PerPage: pagination.perPage,
},
}

8 changes: 4 additions & 4 deletions pkg/github/repositories_test.go
View file
Original file line number Diff line number Diff line change
@@ -582,10 +582,10 @@ func Test_ListCommits(t *testing.T) {
),
),
requestArgs: map[string]interface{}{
"owner": "owner",
"repo": "repo",
"page": float64(2),
"per_page": float64(10),
"owner": "owner",
"repo": "repo",
"page": float64(2),
"perPage": float64(10),
},
expectError: false,
expectedCommits: mockCommits,
57 changes: 12 additions & 45 deletions pkg/github/search.go
View file
Original file line number Diff line number Diff line change
@@ -20,31 +20,22 @@ func searchRepositories(client *github.Client, t translations.TranslationHelperF
mcp.Required(),
mcp.Description("Search query"),
),
mcp.WithNumber("page",
mcp.Description("Page number for pagination"),
),
mcp.WithNumber("perPage",
mcp.Description("Results per page (max 100)"),
),
withPagination(),
),
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
query, err := requiredParam[string](request, "query")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
page, err := optionalIntParamWithDefault(request, "page", 1)
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
perPage, err := optionalIntParamWithDefault(request, "perPage", 30)
pagination, err := optionalPaginationParams(request)
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}

opts := &github.SearchOptions{
ListOptions: github.ListOptions{
Page: page,
PerPage: perPage,
Page: pagination.page,
PerPage: pagination.perPage,
},
}

@@ -86,15 +77,7 @@ func searchCode(client *github.Client, t translations.TranslationHelperFunc) (to
mcp.Description("Sort order ('asc' or 'desc')"),
mcp.Enum("asc", "desc"),
),
mcp.WithNumber("per_page",
mcp.Description("Results per page (max 100)"),
mcp.Min(1),
mcp.Max(100),
),
mcp.WithNumber("page",
mcp.Description("Page number"),
mcp.Min(1),
),
withPagination(),
),
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
query, err := requiredParam[string](request, "q")
@@ -109,11 +92,7 @@ func searchCode(client *github.Client, t translations.TranslationHelperFunc) (to
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
perPage, err := optionalIntParamWithDefault(request, "per_page", 30)
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
page, err := optionalIntParamWithDefault(request, "page", 1)
pagination, err := optionalPaginationParams(request)
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
@@ -122,8 +101,8 @@ func searchCode(client *github.Client, t translations.TranslationHelperFunc) (to
Sort: sort,
Order: order,
ListOptions: github.ListOptions{
PerPage: perPage,
Page: page,
PerPage: pagination.perPage,
Page: pagination.page,
},
}

@@ -166,15 +145,7 @@ func searchUsers(client *github.Client, t translations.TranslationHelperFunc) (t
mcp.Description("Sort order ('asc' or 'desc')"),
mcp.Enum("asc", "desc"),
),
mcp.WithNumber("per_page",
mcp.Description("Results per page (max 100)"),
mcp.Min(1),
mcp.Max(100),
),
mcp.WithNumber("page",
mcp.Description("Page number"),
mcp.Min(1),
),
withPagination(),
),
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
query, err := requiredParam[string](request, "q")
@@ -189,11 +160,7 @@ func searchUsers(client *github.Client, t translations.TranslationHelperFunc) (t
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
perPage, err := optionalIntParamWithDefault(request, "per_page", 30)
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
page, err := optionalIntParamWithDefault(request, "page", 1)
pagination, err := optionalPaginationParams(request)
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
@@ -202,8 +169,8 @@ func searchUsers(client *github.Client, t translations.TranslationHelperFunc) (t
Sort: sort,
Order: order,
ListOptions: github.ListOptions{
PerPage: perPage,
Page: page,
PerPage: pagination.perPage,
Page: pagination.page,
},
}

Loading
Oops, something went wrong.