From ae7ae2378243cea6f66908cf951ae408ca3c3840 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 10:15:23 +0000 Subject: [PATCH 01/14] Initial plan From 452874563daba0107313e2c0d6fe9e5f3a694a21 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 10:28:28 +0000 Subject: [PATCH 02/14] Implement cursor-based pagination infrastructure and update repository tools Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/repositories.go | 115 +++++++++++-------- pkg/github/server.go | 226 ++++++++++++++++++++++++++++++------- 2 files changed, 255 insertions(+), 86 deletions(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index c188b0f68..f66e16cc6 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -107,7 +107,7 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too // ListCommits creates a tool to get commits of a branch in a repository. func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("list_commits", - mcp.WithDescription(t("TOOL_LIST_COMMITS_DESCRIPTION", "Get list of commits of a branch in a GitHub repository. Returns at least 30 results per page by default, but can return more if specified using the perPage parameter (up to 100).")), + mcp.WithDescription(t("TOOL_LIST_COMMITS_DESCRIPTION", "Get list of commits of a branch in a GitHub repository. Returns 10 results per page. Use the cursor parameter for pagination.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_LIST_COMMITS_USER_TITLE", "List commits"), ReadOnlyHint: ToBoolPtr(true), @@ -145,21 +145,18 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } - // Set default perPage to 30 if not provided - perPage := pagination.PerPage - if perPage == 0 { - perPage = 30 - } + + // Request one extra item to check if there are more results opts := &github.CommitsListOptions{ SHA: sha, Author: author, ListOptions: github.ListOptions{ - Page: pagination.Page, - PerPage: perPage, + Page: cursorParams.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra }, } @@ -185,18 +182,22 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t return mcp.NewToolResultError(fmt.Sprintf("failed to list commits: %s", string(body))), nil } + // Check if there are more results + hasMore := len(commits) > cursorParams.PerPage + if hasMore { + // Remove the extra item + commits = commits[:cursorParams.PerPage] + } + // Convert to minimal commits minimalCommits := make([]MinimalCommit, len(commits)) for i, commit := range commits { minimalCommits[i] = convertToMinimalCommit(commit, false) } - r, err := json.Marshal(minimalCommits) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(minimalCommits, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } @@ -227,15 +228,16 @@ func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) ( if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } + // Request one extra item to check if there are more results opts := &github.BranchListOptions{ ListOptions: github.ListOptions{ - Page: pagination.Page, - PerPage: pagination.PerPage, + Page: cursorParams.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra }, } @@ -262,18 +264,22 @@ func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) ( return mcp.NewToolResultError(fmt.Sprintf("failed to list branches: %s", string(body))), nil } + // Check if there are more results + hasMore := len(branches) > cursorParams.PerPage + if hasMore { + // Remove the extra item + branches = branches[:cursorParams.PerPage] + } + // Convert to minimal branches minimalBranches := make([]MinimalBranch, 0, len(branches)) for _, branch := range branches { minimalBranches = append(minimalBranches, convertToMinimalBranch(branch)) } - r, err := json.Marshal(minimalBranches) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(minimalBranches, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } @@ -1249,14 +1255,15 @@ func ListTags(getClient GetClientFn, t translations.TranslationHelperFunc) (tool if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } + // Request one extra item to check if there are more results opts := &github.ListOptions{ - Page: pagination.Page, - PerPage: pagination.PerPage, + Page: cursorParams.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra } client, err := getClient(ctx) @@ -1282,12 +1289,16 @@ func ListTags(getClient GetClientFn, t translations.TranslationHelperFunc) (tool return mcp.NewToolResultError(fmt.Sprintf("failed to list tags: %s", string(body))), nil } - r, err := json.Marshal(tags) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Check if there are more results + hasMore := len(tags) > cursorParams.PerPage + if hasMore { + // Remove the extra item + tags = tags[:cursorParams.PerPage] } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(tags, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } @@ -1405,14 +1416,15 @@ func ListReleases(getClient GetClientFn, t translations.TranslationHelperFunc) ( if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } + // Request one extra item to check if there are more results opts := &github.ListOptions{ - Page: pagination.Page, - PerPage: pagination.PerPage, + Page: cursorParams.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra } client, err := getClient(ctx) @@ -1434,12 +1446,16 @@ func ListReleases(getClient GetClientFn, t translations.TranslationHelperFunc) ( return mcp.NewToolResultError(fmt.Sprintf("failed to list releases: %s", string(body))), nil } - r, err := json.Marshal(releases) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Check if there are more results + hasMore := len(releases) > cursorParams.PerPage + if hasMore { + // Remove the extra item + releases = releases[:cursorParams.PerPage] } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(releases, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } @@ -1733,15 +1749,16 @@ func ListStarredRepositories(getClient GetClientFn, t translations.TranslationHe if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } + // Request one extra item to check if there are more results opts := &github.ActivityListStarredOptions{ ListOptions: github.ListOptions{ - Page: pagination.Page, - PerPage: pagination.PerPage, + Page: cursorParams.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra }, } if sort != "" { @@ -1783,6 +1800,13 @@ func ListStarredRepositories(getClient GetClientFn, t translations.TranslationHe return mcp.NewToolResultError(fmt.Sprintf("failed to list starred repositories: %s", string(body))), nil } + // Check if there are more results + hasMore := len(repos) > cursorParams.PerPage + if hasMore { + // Remove the extra item + repos = repos[:cursorParams.PerPage] + } + // Convert to minimal format minimalRepos := make([]MinimalRepository, 0, len(repos)) for _, starredRepo := range repos { @@ -1810,12 +1834,9 @@ func ListStarredRepositories(getClient GetClientFn, t translations.TranslationHe minimalRepos = append(minimalRepos, minimalRepo) } - r, err := json.Marshal(minimalRepos) - if err != nil { - return nil, fmt.Errorf("failed to marshal starred repositories: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(minimalRepos, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } diff --git a/pkg/github/server.go b/pkg/github/server.go index b46425d80..1ec1cca6c 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -191,55 +191,26 @@ func OptionalStringArrayParam(r mcp.CallToolRequest, p string) ([]string, error) // WithPagination adds REST API pagination parameters to a tool. // https://docs.github.com/en/rest/using-the-rest-api/using-pagination-in-the-rest-api +// WithPagination adds cursor-based pagination parameters to a tool. +// This replaces the old page/perPage approach with a simple cursor string. func WithPagination() mcp.ToolOption { return func(tool *mcp.Tool) { - mcp.WithNumber("page", - mcp.Description("Page number for pagination (min 1)"), - mcp.Min(1), - )(tool) - - mcp.WithNumber("perPage", - mcp.Description("Results per page for pagination (min 1, max 100)"), - mcp.Min(1), - mcp.Max(100), + mcp.WithString("cursor", + mcp.Description("Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page."), )(tool) } } -// WithUnifiedPagination adds REST API pagination parameters to a tool. -// GraphQL tools will use this and convert page/perPage to GraphQL cursor parameters internally. +// WithUnifiedPagination is deprecated and now redirects to WithPagination +// for consistency. All pagination is now cursor-based. func WithUnifiedPagination() mcp.ToolOption { - return func(tool *mcp.Tool) { - mcp.WithNumber("page", - mcp.Description("Page number for pagination (min 1)"), - mcp.Min(1), - )(tool) - - mcp.WithNumber("perPage", - mcp.Description("Results per page for pagination (min 1, max 100)"), - mcp.Min(1), - mcp.Max(100), - )(tool) - - mcp.WithString("after", - mcp.Description("Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs."), - )(tool) - } + return WithPagination() } -// WithCursorPagination adds only cursor-based pagination parameters to a tool (no page parameter). +// WithCursorPagination adds cursor-based pagination parameters to a tool. +// This is now the same as WithPagination for consistency. func WithCursorPagination() mcp.ToolOption { - return func(tool *mcp.Tool) { - mcp.WithNumber("perPage", - mcp.Description("Results per page for pagination (min 1, max 100)"), - mcp.Min(1), - mcp.Max(100), - )(tool) - - mcp.WithString("after", - mcp.Description("Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs."), - )(tool) - } + return WithPagination() } type PaginationParams struct { @@ -290,6 +261,25 @@ func OptionalCursorPaginationParams(r mcp.CallToolRequest) (CursorPaginationPara }, nil } +// GetCursorBasedParams extracts and decodes the cursor parameter from the request. +// Returns decoded pagination parameters with a fixed page size of 10. +func GetCursorBasedParams(r mcp.CallToolRequest) (*DecodedCursor, error) { + cursor, err := OptionalParam[string](r, "cursor") + if err != nil { + return nil, err + } + + decoded, err := DecodeCursor(cursor) + if err != nil { + return nil, err + } + + // Always use page size of 10 as per requirements + decoded.PerPage = 10 + + return decoded, nil +} + type CursorPaginationParams struct { PerPage int After string @@ -333,6 +323,164 @@ func (p PaginationParams) ToGraphQLParams() (*GraphQLPaginationParams, error) { return cursor.ToGraphQLParams() } +// CursorBasedPaginationParams represents the cursor-based pagination input +type CursorBasedPaginationParams struct { + Cursor string +} + +// DecodedCursor represents the decoded cursor information +type DecodedCursor struct { + Page int + PerPage int + After string // For GraphQL cursors +} + +// DecodeCursor decodes a cursor string into pagination parameters +// Cursor format: "page=2;perPage=10" for REST or just a GraphQL cursor string +func DecodeCursor(cursor string) (*DecodedCursor, error) { + if cursor == "" { + // Empty cursor means first page with default size + return &DecodedCursor{ + Page: 1, + PerPage: 10, + }, nil + } + + // Check if this is a GraphQL cursor (doesn't contain '=') + if !contains(cursor, "=") { + // This is a GraphQL cursor, return it as-is in After field + return &DecodedCursor{ + Page: 1, + PerPage: 10, + After: cursor, + }, nil + } + + // Parse REST API cursor format: "page=2;perPage=10" + decoded := &DecodedCursor{ + Page: 1, + PerPage: 10, + } + + parts := splitString(cursor, ";") + for _, part := range parts { + kv := splitString(part, "=") + if len(kv) != 2 { + continue + } + key := kv[0] + value := kv[1] + + switch key { + case "page": + var p int + _, err := fmt.Sscanf(value, "%d", &p) + if err == nil { + decoded.Page = p + } + case "perPage": + var pp int + _, err := fmt.Sscanf(value, "%d", &pp) + if err == nil { + decoded.PerPage = pp + } + } + } + + return decoded, nil +} + +// EncodeCursor encodes pagination parameters into a cursor string +func EncodeCursor(page int, perPage int) string { + return fmt.Sprintf("page=%d;perPage=%d", page, perPage) +} + +// EncodeGraphQLCursor returns the GraphQL cursor as-is +func EncodeGraphQLCursor(cursor string) string { + return cursor +} + +// PaginatedResponse wraps a response with pagination metadata +type PaginatedResponse struct { + Data interface{} `json:"data"` + MoreResults bool `json:"moreResults"` + NextCursor string `json:"nextCursor,omitempty"` +} + +// NewPaginatedRESTResponse creates a paginated response for REST API results. +// It fetches one extra item to determine if there are more results. +func NewPaginatedRESTResponse(data interface{}, currentPage int, pageSize int, hasMore bool) *PaginatedResponse { + resp := &PaginatedResponse{ + Data: data, + MoreResults: hasMore, + } + + if hasMore { + resp.NextCursor = EncodeCursor(currentPage+1, pageSize) + } + + return resp +} + +// NewPaginatedGraphQLResponse creates a paginated response for GraphQL API results. +func NewPaginatedGraphQLResponse(data interface{}, hasNextPage bool, endCursor string) *PaginatedResponse { + resp := &PaginatedResponse{ + Data: data, + MoreResults: hasNextPage, + } + + if hasNextPage && endCursor != "" { + resp.NextCursor = EncodeGraphQLCursor(endCursor) + } + + return resp +} + +// MarshalPaginatedResponse marshals a paginated response to JSON text result +func MarshalPaginatedResponse(resp *PaginatedResponse) *mcp.CallToolResult { + data, err := json.Marshal(resp) + if err != nil { + return mcp.NewToolResultErrorFromErr("failed to marshal paginated response", err) + } + return mcp.NewToolResultText(string(data)) +} + +// Helper function to check if a string contains a substring +func contains(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(s) > len(substr) && findSubstring(s, substr)) +} + +func findSubstring(s, substr string) bool { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +} + +// Helper function to split a string by a delimiter +func splitString(s, sep string) []string { + if s == "" { + return []string{} + } + if sep == "" { + return []string{s} + } + + var result []string + start := 0 + for i := 0; i <= len(s)-len(sep); i++ { + if s[i:i+len(sep)] == sep { + result = append(result, s[start:i]) + start = i + len(sep) + i += len(sep) - 1 + } + } + result = append(result, s[start:]) + return result +} + func MarshalledTextResult(v any) *mcp.CallToolResult { data, err := json.Marshal(v) if err != nil { From 6c1833acf7112fc6dfba5409a23cc417fe2d0184 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 10:33:24 +0000 Subject: [PATCH 03/14] Update actions, gists, and notifications tools to use cursor-based pagination Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/actions.go | 92 ++++++++++++++++++++++--------------- pkg/github/gists.go | 18 +++++--- pkg/github/notifications.go | 19 ++++---- 3 files changed, 76 insertions(+), 53 deletions(-) diff --git a/pkg/github/actions.go b/pkg/github/actions.go index 734109587..7712ea13a 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -50,8 +50,8 @@ func ListWorkflows(getClient GetClientFn, t translations.TranslationHelperFunc) return mcp.NewToolResultError(err.Error()), nil } - // Get optional pagination parameters - pagination, err := OptionalPaginationParams(request) + // Get cursor-based pagination parameters + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -61,10 +61,10 @@ func ListWorkflows(getClient GetClientFn, t translations.TranslationHelperFunc) return nil, fmt.Errorf("failed to get GitHub client: %w", err) } - // Set up list options + // Set up list options - request one extra to check for more results opts := &github.ListOptions{ - PerPage: pagination.PerPage, - Page: pagination.Page, + PerPage: cursorParams.PerPage + 1, + Page: cursorParams.Page, } workflows, resp, err := client.Actions.ListWorkflows(ctx, owner, repo, opts) @@ -73,12 +73,16 @@ func ListWorkflows(getClient GetClientFn, t translations.TranslationHelperFunc) } defer func() { _ = resp.Body.Close() }() - r, err := json.Marshal(workflows) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Check if there are more results + hasMore := len(workflows.Workflows) > cursorParams.PerPage + if hasMore { + // Remove the extra item + workflows.Workflows = workflows.Workflows[:cursorParams.PerPage] } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(workflows, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } @@ -183,8 +187,8 @@ func ListWorkflowRuns(getClient GetClientFn, t translations.TranslationHelperFun return mcp.NewToolResultError(err.Error()), nil } - // Get optional pagination parameters - pagination, err := OptionalPaginationParams(request) + // Get cursor-based pagination parameters + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -194,15 +198,15 @@ func ListWorkflowRuns(getClient GetClientFn, t translations.TranslationHelperFun return nil, fmt.Errorf("failed to get GitHub client: %w", err) } - // Set up list options + // Set up list options - request one extra to check for more results opts := &github.ListWorkflowRunsOptions{ Actor: actor, Branch: branch, Event: event, Status: status, ListOptions: github.ListOptions{ - PerPage: pagination.PerPage, - Page: pagination.Page, + PerPage: cursorParams.PerPage + 1, + Page: cursorParams.Page, }, } @@ -212,12 +216,16 @@ func ListWorkflowRuns(getClient GetClientFn, t translations.TranslationHelperFun } defer func() { _ = resp.Body.Close() }() - r, err := json.Marshal(workflowRuns) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Check if there are more results + hasMore := len(workflowRuns.WorkflowRuns) > cursorParams.PerPage + if hasMore { + // Remove the extra item + workflowRuns.WorkflowRuns = workflowRuns.WorkflowRuns[:cursorParams.PerPage] } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(workflowRuns, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } @@ -489,8 +497,8 @@ func ListWorkflowJobs(getClient GetClientFn, t translations.TranslationHelperFun return mcp.NewToolResultError(err.Error()), nil } - // Get optional pagination parameters - pagination, err := OptionalPaginationParams(request) + // Get cursor-based pagination parameters + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -500,12 +508,12 @@ func ListWorkflowJobs(getClient GetClientFn, t translations.TranslationHelperFun return nil, fmt.Errorf("failed to get GitHub client: %w", err) } - // Set up list options + // Set up list options - request one extra to check for more results opts := &github.ListWorkflowJobsOptions{ Filter: filter, ListOptions: github.ListOptions{ - PerPage: pagination.PerPage, - Page: pagination.Page, + PerPage: cursorParams.PerPage + 1, + Page: cursorParams.Page, }, } @@ -515,18 +523,22 @@ func ListWorkflowJobs(getClient GetClientFn, t translations.TranslationHelperFun } defer func() { _ = resp.Body.Close() }() + // Check if there are more results + hasMore := len(jobs.Jobs) > cursorParams.PerPage + if hasMore { + // Remove the extra item + jobs.Jobs = jobs.Jobs[:cursorParams.PerPage] + } + // Add optimization tip for failed job debugging response := map[string]any{ "jobs": jobs, "optimization_tip": "For debugging failed jobs, consider using get_job_logs with failed_only=true and run_id=" + fmt.Sprintf("%d", runID) + " to get logs directly without needing to list jobs first", } - r, err := json.Marshal(response) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(response, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } @@ -1006,8 +1018,8 @@ func ListWorkflowRunArtifacts(getClient GetClientFn, t translations.TranslationH } runID := int64(runIDInt) - // Get optional pagination parameters - pagination, err := OptionalPaginationParams(request) + // Get cursor-based pagination parameters + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -1017,10 +1029,10 @@ func ListWorkflowRunArtifacts(getClient GetClientFn, t translations.TranslationH return nil, fmt.Errorf("failed to get GitHub client: %w", err) } - // Set up list options + // Set up list options - request one extra to check for more results opts := &github.ListOptions{ - PerPage: pagination.PerPage, - Page: pagination.Page, + PerPage: cursorParams.PerPage + 1, + Page: cursorParams.Page, } artifacts, resp, err := client.Actions.ListWorkflowRunArtifacts(ctx, owner, repo, runID, opts) @@ -1029,12 +1041,16 @@ func ListWorkflowRunArtifacts(getClient GetClientFn, t translations.TranslationH } defer func() { _ = resp.Body.Close() }() - r, err := json.Marshal(artifacts) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Check if there are more results + hasMore := len(artifacts.Artifacts) > cursorParams.PerPage + if hasMore { + // Remove the extra item + artifacts.Artifacts = artifacts.Artifacts[:cursorParams.PerPage] } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(artifacts, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } diff --git a/pkg/github/gists.go b/pkg/github/gists.go index 47bfeb2bc..8df2a8ea7 100644 --- a/pkg/github/gists.go +++ b/pkg/github/gists.go @@ -40,15 +40,15 @@ func ListGists(getClient GetClientFn, t translations.TranslationHelperFunc) (too return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } opts := &github.GistListOptions{ ListOptions: github.ListOptions{ - Page: pagination.Page, - PerPage: pagination.PerPage, + Page: cursorParams.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra }, } @@ -80,12 +80,16 @@ func ListGists(getClient GetClientFn, t translations.TranslationHelperFunc) (too return mcp.NewToolResultError(fmt.Sprintf("failed to list gists: %s", string(body))), nil } - r, err := json.Marshal(gists) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Check if there are more results + hasMore := len(gists) > cursorParams.PerPage + if hasMore { + // Remove the extra item + gists = gists[:cursorParams.PerPage] } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(gists, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } diff --git a/pkg/github/notifications.go b/pkg/github/notifications.go index 4da04889c..c11c5bae2 100644 --- a/pkg/github/notifications.go +++ b/pkg/github/notifications.go @@ -78,7 +78,7 @@ func ListNotifications(getClient GetClientFn, t translations.TranslationHelperFu return mcp.NewToolResultError(err.Error()), nil } - paginationParams, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -88,8 +88,8 @@ func ListNotifications(getClient GetClientFn, t translations.TranslationHelperFu All: filter == FilterIncludeRead, Participating: filter == FilterOnlyParticipating, ListOptions: github.ListOptions{ - Page: paginationParams.Page, - PerPage: paginationParams.PerPage, + Page: cursorParams.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra }, } @@ -135,13 +135,16 @@ func ListNotifications(getClient GetClientFn, t translations.TranslationHelperFu return mcp.NewToolResultError(fmt.Sprintf("failed to get notifications: %s", string(body))), nil } - // Marshal response to JSON - r, err := json.Marshal(notifications) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Check if there are more results + hasMore := len(notifications) > cursorParams.PerPage + if hasMore { + // Remove the extra item + notifications = notifications[:cursorParams.PerPage] } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(notifications, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } From 110eba716b239e6f44a2316e47837edc18fe4e1f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 10:41:06 +0000 Subject: [PATCH 04/14] Update pull requests and issues tools to use cursor-based pagination Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/issues.go | 42 ++++++++++++++---------- pkg/github/pullrequests.go | 66 ++++++++++++++++++++++---------------- pkg/github/repositories.go | 22 +++++++------ 3 files changed, 77 insertions(+), 53 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 94f2f35e8..674f70e06 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -279,7 +279,7 @@ Options are: return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -298,9 +298,9 @@ Options are: case "get": return GetIssue(ctx, client, owner, repo, issueNumber) case "get_comments": - return GetIssueComments(ctx, client, owner, repo, issueNumber, pagination) + return GetIssueComments(ctx, client, owner, repo, issueNumber, cursorParams) case "get_sub_issues": - return GetSubIssues(ctx, client, owner, repo, issueNumber, pagination) + return GetSubIssues(ctx, client, owner, repo, issueNumber, cursorParams) case "get_labels": return GetIssueLabels(ctx, gqlClient, owner, repo, issueNumber) default: @@ -342,11 +342,11 @@ func GetIssue(ctx context.Context, client *github.Client, owner string, repo str return mcp.NewToolResultText(string(r)), nil } -func GetIssueComments(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { +func GetIssueComments(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, cursorParams *DecodedCursor) (*mcp.CallToolResult, error) { opts := &github.IssueListCommentsOptions{ ListOptions: github.ListOptions{ - Page: pagination.Page, - PerPage: pagination.PerPage, + Page: cursorParams.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra }, } @@ -364,19 +364,23 @@ func GetIssueComments(ctx context.Context, client *github.Client, owner string, return mcp.NewToolResultError(fmt.Sprintf("failed to get issue comments: %s", string(body))), nil } - r, err := json.Marshal(comments) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Check if there are more results + hasMore := len(comments) > cursorParams.PerPage + if hasMore { + // Remove the extra item + comments = comments[:cursorParams.PerPage] } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(comments, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } -func GetSubIssues(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { +func GetSubIssues(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, cursorParams *DecodedCursor) (*mcp.CallToolResult, error) { opts := &github.IssueListOptions{ ListOptions: github.ListOptions{ - Page: pagination.Page, - PerPage: pagination.PerPage, + Page: cursorParams.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra }, } @@ -399,12 +403,16 @@ func GetSubIssues(ctx context.Context, client *github.Client, owner string, repo return mcp.NewToolResultError(fmt.Sprintf("failed to list sub-issues: %s", string(body))), nil } - r, err := json.Marshal(subIssues) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Check if there are more results + hasMore := len(subIssues) > cursorParams.PerPage + if hasMore { + // Remove the extra item + subIssues = subIssues[:cursorParams.PerPage] } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(subIssues, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } func GetIssueLabels(ctx context.Context, client *githubv4.Client, owner string, repo string, issueNumber int) (*mcp.CallToolResult, error) { diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 4f5e1952c..997a4662d 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -73,7 +73,7 @@ Possible options: if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -92,13 +92,13 @@ Possible options: case "get_status": return GetPullRequestStatus(ctx, client, owner, repo, pullNumber) case "get_files": - return GetPullRequestFiles(ctx, client, owner, repo, pullNumber, pagination) + return GetPullRequestFiles(ctx, client, owner, repo, pullNumber, cursorParams) case "get_review_comments": - return GetPullRequestReviewComments(ctx, client, owner, repo, pullNumber, pagination) + return GetPullRequestReviewComments(ctx, client, owner, repo, pullNumber, cursorParams) case "get_reviews": return GetPullRequestReviews(ctx, client, owner, repo, pullNumber) case "get_comments": - return GetIssueComments(ctx, client, owner, repo, pullNumber, pagination) + return GetIssueComments(ctx, client, owner, repo, pullNumber, cursorParams) default: return nil, fmt.Errorf("unknown method: %s", method) } @@ -218,10 +218,10 @@ func GetPullRequestStatus(ctx context.Context, client *github.Client, owner, rep return mcp.NewToolResultText(string(r)), nil } -func GetPullRequestFiles(ctx context.Context, client *github.Client, owner, repo string, pullNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { +func GetPullRequestFiles(ctx context.Context, client *github.Client, owner, repo string, pullNumber int, cursorParams *DecodedCursor) (*mcp.CallToolResult, error) { opts := &github.ListOptions{ - PerPage: pagination.PerPage, - Page: pagination.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra + Page: cursorParams.Page, } files, resp, err := client.PullRequests.ListFiles(ctx, owner, repo, pullNumber, opts) if err != nil { @@ -241,19 +241,23 @@ func GetPullRequestFiles(ctx context.Context, client *github.Client, owner, repo return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request files: %s", string(body))), nil } - r, err := json.Marshal(files) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Check if there are more results + hasMore := len(files) > cursorParams.PerPage + if hasMore { + // Remove the extra item + files = files[:cursorParams.PerPage] } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(files, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } -func GetPullRequestReviewComments(ctx context.Context, client *github.Client, owner, repo string, pullNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { +func GetPullRequestReviewComments(ctx context.Context, client *github.Client, owner, repo string, pullNumber int, cursorParams *DecodedCursor) (*mcp.CallToolResult, error) { opts := &github.PullRequestListCommentsOptions{ ListOptions: github.ListOptions{ - PerPage: pagination.PerPage, - Page: pagination.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra + Page: cursorParams.Page, }, } @@ -275,12 +279,16 @@ func GetPullRequestReviewComments(ctx context.Context, client *github.Client, ow return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request review comments: %s", string(body))), nil } - r, err := json.Marshal(comments) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Check if there are more results + hasMore := len(comments) > cursorParams.PerPage + if hasMore { + // Remove the extra item + comments = comments[:cursorParams.PerPage] } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(comments, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } func GetPullRequestReviews(ctx context.Context, client *github.Client, owner, repo string, pullNumber int) (*mcp.CallToolResult, error) { @@ -777,7 +785,7 @@ func ListPullRequests(getClient GetClientFn, t translations.TranslationHelperFun if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -788,8 +796,8 @@ func ListPullRequests(getClient GetClientFn, t translations.TranslationHelperFun Sort: sort, Direction: direction, ListOptions: github.ListOptions{ - PerPage: pagination.PerPage, - Page: pagination.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra + Page: cursorParams.Page, }, } @@ -815,6 +823,13 @@ func ListPullRequests(getClient GetClientFn, t translations.TranslationHelperFun return mcp.NewToolResultError(fmt.Sprintf("failed to list pull requests: %s", string(body))), nil } + // Check if there are more results + hasMore := len(prs) > cursorParams.PerPage + if hasMore { + // Remove the extra item + prs = prs[:cursorParams.PerPage] + } + // sanitize title/body on each PR for _, pr := range prs { if pr == nil { @@ -828,12 +843,9 @@ func ListPullRequests(getClient GetClientFn, t translations.TranslationHelperFun } } - r, err := json.Marshal(prs) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(prs, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index f66e16cc6..0f70bd7ea 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -60,14 +60,14 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } opts := &github.ListOptions{ - Page: pagination.Page, - PerPage: pagination.PerPage, + Page: cursorParams.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra } client, err := getClient(ctx) @@ -92,15 +92,19 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too return mcp.NewToolResultError(fmt.Sprintf("failed to get commit: %s", string(body))), nil } + // Check if there are more files in the commit + hasMore := len(commit.Files) > cursorParams.PerPage + if hasMore { + // Remove the extra item + commit.Files = commit.Files[:cursorParams.PerPage] + } + // Convert to minimal commit minimalCommit := convertToMinimalCommit(commit, includeDiff) - r, err := json.Marshal(minimalCommit) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(minimalCommit, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } From a47b0fed98d6e3f5ba57dc39a256ce7b9740d76a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 10:45:49 +0000 Subject: [PATCH 05/14] Update search tools to use cursor-based pagination Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/search.go | 71 +++++++++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/pkg/github/search.go b/pkg/github/search.go index 4f396b6b0..82e778592 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -2,7 +2,6 @@ package github import ( "context" - "encoding/json" "fmt" "io" @@ -53,7 +52,7 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -65,8 +64,8 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF Sort: sort, Order: order, ListOptions: github.ListOptions{ - Page: pagination.Page, - PerPage: pagination.PerPage, + Page: cursorParams.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra }, } @@ -92,8 +91,15 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF return mcp.NewToolResultError(fmt.Sprintf("failed to search repositories: %s", string(body))), nil } + // Check if there are more results + hasMore := len(result.Repositories) > cursorParams.PerPage + if hasMore { + // Remove the extra item + result.Repositories = result.Repositories[:cursorParams.PerPage] + } + // Return either minimal or full response based on parameter - var r []byte + var responseData interface{} if minimalOutput { minimalRepos := make([]MinimalRepository, 0, len(result.Repositories)) for _, repo := range result.Repositories { @@ -126,24 +132,18 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF minimalRepos = append(minimalRepos, minimalRepo) } - minimalResult := &MinimalSearchRepositoriesResult{ + responseData = &MinimalSearchRepositoriesResult{ TotalCount: result.GetTotal(), IncompleteResults: result.GetIncompleteResults(), Items: minimalRepos, } - - r, err = json.Marshal(minimalResult) - if err != nil { - return nil, fmt.Errorf("failed to marshal minimal response: %w", err) - } } else { - r, err = json.Marshal(result) - if err != nil { - return nil, fmt.Errorf("failed to marshal full response: %w", err) - } + responseData = result } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(responseData, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } @@ -181,7 +181,7 @@ func SearchCode(getClient GetClientFn, t translations.TranslationHelperFunc) (to if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -190,8 +190,8 @@ func SearchCode(getClient GetClientFn, t translations.TranslationHelperFunc) (to Sort: sort, Order: order, ListOptions: github.ListOptions{ - PerPage: pagination.PerPage, - Page: pagination.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra + Page: cursorParams.Page, }, } @@ -218,12 +218,16 @@ func SearchCode(getClient GetClientFn, t translations.TranslationHelperFunc) (to return mcp.NewToolResultError(fmt.Sprintf("failed to search code: %s", string(body))), nil } - r, err := json.Marshal(result) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + // Check if there are more results + hasMore := len(result.CodeResults) > cursorParams.PerPage + if hasMore { + // Remove the extra item + result.CodeResults = result.CodeResults[:cursorParams.PerPage] } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(result, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } @@ -241,7 +245,7 @@ func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHand if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + cursorParams, err := GetCursorBasedParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -250,8 +254,8 @@ func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHand Sort: sort, Order: order, ListOptions: github.ListOptions{ - PerPage: pagination.PerPage, - Page: pagination.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra + Page: cursorParams.Page, }, } @@ -282,6 +286,13 @@ func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHand return mcp.NewToolResultError(fmt.Sprintf("failed to search %ss: %s", accountType, string(body))), nil } + // Check if there are more results + hasMore := len(result.Users) > cursorParams.PerPage + if hasMore { + // Remove the extra item + result.Users = result.Users[:cursorParams.PerPage] + } + minimalUsers := make([]MinimalUser, 0, len(result.Users)) for _, user := range result.Users { @@ -307,11 +318,9 @@ func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHand minimalResp.IncompleteResults = *result.IncompleteResults } - r, err := json.Marshal(minimalResp) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - return mcp.NewToolResultText(string(r)), nil + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(minimalResp, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil } } From d904e07ea37f994195c61576a6e823f35dba81e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 10:52:00 +0000 Subject: [PATCH 06/14] Update GraphQL tools for cursor-based pagination and fix one test Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/actions_test.go | 12 +++-- pkg/github/discussions.go | 95 ++++++++++++++++++-------------------- pkg/github/issues.go | 58 +++++++++-------------- 3 files changed, 76 insertions(+), 89 deletions(-) diff --git a/pkg/github/actions_test.go b/pkg/github/actions_test.go index 04863ba1d..abdbba8de 100644 --- a/pkg/github/actions_test.go +++ b/pkg/github/actions_test.go @@ -30,8 +30,7 @@ func Test_ListWorkflows(t *testing.T) { 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, "perPage") - assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) tests := []struct { @@ -122,8 +121,15 @@ func Test_ListWorkflows(t *testing.T) { } // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the workflows + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var response github.Workflows - err = json.Unmarshal([]byte(textContent.Text), &response) + err = json.Unmarshal(dataBytes, &response) require.NoError(t, err) assert.NotNil(t, response.TotalCount) assert.Greater(t, *response.TotalCount, 0) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index a25d12f8c..46cd90702 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -174,14 +174,16 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp return mcp.NewToolResultError(err.Error()), nil } - // Get pagination parameters and convert to GraphQL format - pagination, err := OptionalCursorPaginationParams(request) + // Get cursor-based pagination parameters + cursorParams, err := GetCursorBasedParams(request) if err != nil { return nil, err } - paginationParams, err := pagination.ToGraphQLParams() - if err != nil { - return nil, err + + // For GraphQL, the cursor is the 'after' value + var after *string + if cursorParams.After != "" { + after = &cursorParams.After } client, err := getGQLClient(ctx) @@ -195,13 +197,16 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp categoryID = &id } + // Use fixed page size of 10 for cursor-based pagination + first := int32(cursorParams.PerPage) + vars := map[string]interface{}{ "owner": githubv4.String(owner), "repo": githubv4.String(repo), - "first": githubv4.Int(*paginationParams.First), + "first": githubv4.Int(first), } - if paginationParams.After != nil { - vars["after"] = githubv4.String(*paginationParams.After) + if after != nil { + vars["after"] = githubv4.String(*after) } else { vars["after"] = (*githubv4.String)(nil) } @@ -237,22 +242,20 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp } // Create response with pagination info + hasNextPage := bool(pageInfo.HasNextPage) + var nextCursor string + if hasNextPage { + nextCursor = string(pageInfo.EndCursor) + } + response := map[string]interface{}{ "discussions": discussions, - "pageInfo": map[string]interface{}{ - "hasNextPage": pageInfo.HasNextPage, - "hasPreviousPage": pageInfo.HasPreviousPage, - "startCursor": string(pageInfo.StartCursor), - "endCursor": string(pageInfo.EndCursor), - }, - "totalCount": totalCount, + "totalCount": totalCount, } - out, err := json.Marshal(response) - if err != nil { - return nil, fmt.Errorf("failed to marshal discussions: %w", err) - } - return mcp.NewToolResultText(string(out)), nil + // Create paginated response + paginatedResp := NewPaginatedGraphQLResponse(response, hasNextPage, nextCursor) + return MarshalPaginatedResponse(paginatedResp), nil } } @@ -356,25 +359,16 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati return mcp.NewToolResultError(err.Error()), nil } - // Get pagination parameters and convert to GraphQL format - pagination, err := OptionalCursorPaginationParams(request) - if err != nil { - return nil, err - } - - // Check if pagination parameters were explicitly provided - _, perPageProvided := request.GetArguments()["perPage"] - paginationExplicit := perPageProvided - - paginationParams, err := pagination.ToGraphQLParams() + // Get cursor-based pagination parameters + cursorParams, err := GetCursorBasedParams(request) if err != nil { return nil, err } - // Use default of 30 if pagination was not explicitly provided - if !paginationExplicit { - defaultFirst := int32(DefaultGraphQLPageSize) - paginationParams.First = &defaultFirst + // For GraphQL, the cursor is the 'after' value + var after *string + if cursorParams.After != "" { + after = &cursorParams.After } client, err := getGQLClient(ctx) @@ -400,14 +394,16 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati } `graphql:"discussion(number: $discussionNumber)"` } `graphql:"repository(owner: $owner, name: $repo)"` } + // Use fixed page size of 10 for cursor-based pagination + first := int32(cursorParams.PerPage) vars := map[string]interface{}{ "owner": githubv4.String(params.Owner), "repo": githubv4.String(params.Repo), "discussionNumber": githubv4.Int(params.DiscussionNumber), - "first": githubv4.Int(*paginationParams.First), + "first": githubv4.Int(first), } - if paginationParams.After != nil { - vars["after"] = githubv4.String(*paginationParams.After) + if after != nil { + vars["after"] = githubv4.String(*after) } else { vars["after"] = (*githubv4.String)(nil) } @@ -421,23 +417,20 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati } // Create response with pagination info - response := map[string]interface{}{ - "comments": comments, - "pageInfo": map[string]interface{}{ - "hasNextPage": q.Repository.Discussion.Comments.PageInfo.HasNextPage, - "hasPreviousPage": q.Repository.Discussion.Comments.PageInfo.HasPreviousPage, - "startCursor": string(q.Repository.Discussion.Comments.PageInfo.StartCursor), - "endCursor": string(q.Repository.Discussion.Comments.PageInfo.EndCursor), - }, - "totalCount": q.Repository.Discussion.Comments.TotalCount, + hasNextPage := bool(q.Repository.Discussion.Comments.PageInfo.HasNextPage) + var nextCursor string + if hasNextPage { + nextCursor = string(q.Repository.Discussion.Comments.PageInfo.EndCursor) } - out, err := json.Marshal(response) - if err != nil { - return nil, fmt.Errorf("failed to marshal comments: %w", err) + response := map[string]interface{}{ + "comments": comments, + "totalCount": q.Repository.Discussion.Comments.TotalCount, } - return mcp.NewToolResultText(string(out)), nil + // Create paginated response + paginatedResp := NewPaginatedGraphQLResponse(response, hasNextPage, nextCursor) + return MarshalPaginatedResponse(paginatedResp), nil } } diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 674f70e06..9ec2d8594 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1294,30 +1294,16 @@ func ListIssues(getGQLClient GetGQLClientFn, t translations.TranslationHelperFun } hasLabels := len(labels) > 0 - // Get pagination parameters and convert to GraphQL format - pagination, err := OptionalCursorPaginationParams(request) - if err != nil { - return nil, err - } - - // Check if someone tried to use page-based pagination instead of cursor-based - if _, pageProvided := request.GetArguments()["page"]; pageProvided { - return mcp.NewToolResultError("This tool uses cursor-based pagination. Use the 'after' parameter with the 'endCursor' value from the previous response instead of 'page'."), nil - } - - // Check if pagination parameters were explicitly provided - _, perPageProvided := request.GetArguments()["perPage"] - paginationExplicit := perPageProvided - - paginationParams, err := pagination.ToGraphQLParams() + // Get cursor-based pagination parameters + cursorParams, err := GetCursorBasedParams(request) if err != nil { return nil, err } - // Use default of 30 if pagination was not explicitly provided - if !paginationExplicit { - defaultFirst := int32(DefaultGraphQLPageSize) - paginationParams.First = &defaultFirst + // For GraphQL, the cursor is the 'after' value + var after *string + if cursorParams.After != "" { + after = &cursorParams.After } client, err := getGQLClient(ctx) @@ -1325,17 +1311,20 @@ func ListIssues(getGQLClient GetGQLClientFn, t translations.TranslationHelperFun return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil } + // Use fixed page size of 10 for cursor-based pagination + first := int32(cursorParams.PerPage) + vars := map[string]interface{}{ "owner": githubv4.String(owner), "repo": githubv4.String(repo), "states": states, "orderBy": githubv4.IssueOrderField(orderBy), "direction": githubv4.OrderDirection(direction), - "first": githubv4.Int(*paginationParams.First), + "first": githubv4.Int(first), } - if paginationParams.After != nil { - vars["after"] = githubv4.String(*paginationParams.After) + if after != nil { + vars["after"] = githubv4.String(*after) } else { // Used within query, therefore must be set to nil and provided as $after vars["after"] = (*githubv4.String)(nil) @@ -1380,21 +1369,20 @@ func ListIssues(getGQLClient GetGQLClientFn, t translations.TranslationHelperFun } // Create response with issues + hasNextPage := bool(pageInfo.HasNextPage) + var nextCursor string + if hasNextPage { + nextCursor = string(pageInfo.EndCursor) + } + response := map[string]interface{}{ - "issues": issues, - "pageInfo": map[string]interface{}{ - "hasNextPage": pageInfo.HasNextPage, - "hasPreviousPage": pageInfo.HasPreviousPage, - "startCursor": string(pageInfo.StartCursor), - "endCursor": string(pageInfo.EndCursor), - }, + "issues": issues, "totalCount": totalCount, } - out, err := json.Marshal(response) - if err != nil { - return nil, fmt.Errorf("failed to marshal issues: %w", err) - } - return mcp.NewToolResultText(string(out)), nil + + // Create paginated response + paginatedResp := NewPaginatedGraphQLResponse(response, hasNextPage, nextCursor) + return MarshalPaginatedResponse(paginatedResp), nil } } From fd29e1019aa40bd640acbcdb305d55cb7807803e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 10:53:57 +0000 Subject: [PATCH 07/14] Update test assertions to check for cursor parameter instead of page/perPage Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/actions_test.go | 3 +-- pkg/github/gists_test.go | 3 +-- pkg/github/issues_test.go | 10 +++------- pkg/github/notifications_test.go | 3 +-- pkg/github/pullrequests_test.go | 9 +++------ pkg/github/repositories_test.go | 9 +++------ pkg/github/search_test.go | 12 ++++-------- 7 files changed, 16 insertions(+), 33 deletions(-) diff --git a/pkg/github/actions_test.go b/pkg/github/actions_test.go index abdbba8de..9a26773a3 100644 --- a/pkg/github/actions_test.go +++ b/pkg/github/actions_test.go @@ -429,8 +429,7 @@ func Test_ListWorkflowRunArtifacts(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "run_id") - assert.Contains(t, tool.InputSchema.Properties, "perPage") - assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "run_id"}) tests := []struct { diff --git a/pkg/github/gists_test.go b/pkg/github/gists_test.go index c27578ff9..a89187d5f 100644 --- a/pkg/github/gists_test.go +++ b/pkg/github/gists_test.go @@ -23,8 +23,7 @@ func Test_ListGists(t *testing.T) { assert.NotEmpty(t, tool.Description) assert.Contains(t, tool.InputSchema.Properties, "username") assert.Contains(t, tool.InputSchema.Properties, "since") - assert.Contains(t, tool.InputSchema.Properties, "page") - assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.Empty(t, tool.InputSchema.Required) // Setup mock gists for success case diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 03c57ce75..1a861c8e9 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -254,8 +254,7 @@ func Test_SearchIssues(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "sort") assert.Contains(t, tool.InputSchema.Properties, "order") - assert.Contains(t, tool.InputSchema.Properties, "perPage") - assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"}) // Setup mock search results @@ -749,7 +748,6 @@ 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, "after") - assert.Contains(t, tool.InputSchema.Properties, "perPage") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) // Mock issues data @@ -1598,8 +1596,7 @@ func Test_GetIssueComments(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "issue_number") - assert.Contains(t, tool.InputSchema.Properties, "page") - assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "issue_number"}) // Setup mock comments for success case @@ -2507,8 +2504,7 @@ func Test_GetSubIssues(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "issue_number") - assert.Contains(t, tool.InputSchema.Properties, "page") - assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "issue_number"}) // Setup mock sub-issues for success case diff --git a/pkg/github/notifications_test.go b/pkg/github/notifications_test.go index 98b132594..e4c2cf3c2 100644 --- a/pkg/github/notifications_test.go +++ b/pkg/github/notifications_test.go @@ -27,8 +27,7 @@ func Test_ListNotifications(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "before") assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") - assert.Contains(t, tool.InputSchema.Properties, "page") - assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Contains(t, tool.InputSchema.Properties, "cursor") // All fields are optional, so Required should be empty assert.Empty(t, tool.InputSchema.Required) diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index a66e2852a..fdbd5d317 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -590,8 +590,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, "perPage") - assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) // Setup mock PRs for success case @@ -836,8 +835,7 @@ func Test_SearchPullRequests(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "sort") assert.Contains(t, tool.InputSchema.Properties, "order") - assert.Contains(t, tool.InputSchema.Properties, "perPage") - assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"}) mockSearchResult := &github.IssuesSearchResult{ @@ -1142,8 +1140,7 @@ func Test_GetPullRequestFiles(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "pullNumber") - assert.Contains(t, tool.InputSchema.Properties, "page") - assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "pullNumber"}) // Setup mock PR files for success case diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 8baca434e..5ad6d40b6 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -765,8 +765,7 @@ func Test_ListCommits(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "sha") assert.Contains(t, tool.InputSchema.Properties, "author") - assert.Contains(t, tool.InputSchema.Properties, "page") - assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) // Setup mock commits for success case @@ -1675,8 +1674,7 @@ func Test_ListBranches(t *testing.T) { 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, "page") - assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) // Setup mock branches for success case @@ -2923,8 +2921,7 @@ func Test_ListStarredRepositories(t *testing.T) { 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.Contains(t, tool.InputSchema.Properties, "cursor") assert.Empty(t, tool.InputSchema.Required) // All parameters are optional // Setup mock starred repositories diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index c70682f74..37e598714 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -25,8 +25,7 @@ func Test_SearchRepositories(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "query") assert.Contains(t, tool.InputSchema.Properties, "sort") assert.Contains(t, tool.InputSchema.Properties, "order") - assert.Contains(t, tool.InputSchema.Properties, "page") - assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"}) // Setup mock search results @@ -239,8 +238,7 @@ func Test_SearchCode(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "query") assert.Contains(t, tool.InputSchema.Properties, "sort") assert.Contains(t, tool.InputSchema.Properties, "order") - assert.Contains(t, tool.InputSchema.Properties, "perPage") - assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"}) // Setup mock search results @@ -394,8 +392,7 @@ func Test_SearchUsers(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "query") assert.Contains(t, tool.InputSchema.Properties, "sort") assert.Contains(t, tool.InputSchema.Properties, "order") - assert.Contains(t, tool.InputSchema.Properties, "perPage") - assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"}) // Setup mock search results @@ -588,8 +585,7 @@ func Test_SearchOrgs(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "query") assert.Contains(t, tool.InputSchema.Properties, "sort") assert.Contains(t, tool.InputSchema.Properties, "order") - assert.Contains(t, tool.InputSchema.Properties, "perPage") - assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"}) // Setup mock search results From c64cfdc5868161020e879dc1ecf5e93de37c47ba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 11:16:06 +0000 Subject: [PATCH 08/14] Fix test response parsing to handle PaginatedResponse wrapper - partial update Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/__toolsnaps__/get_commit.snap | 15 ++---- pkg/github/__toolsnaps__/issue_read.snap | 15 ++---- pkg/github/__toolsnaps__/list_branches.snap | 15 ++---- pkg/github/__toolsnaps__/list_commits.snap | 17 ++---- pkg/github/__toolsnaps__/list_issues.snap | 10 +--- .../__toolsnaps__/list_notifications.snap | 15 ++---- .../__toolsnaps__/list_pull_requests.snap | 15 ++---- .../list_starred_repositories.snap | 15 ++---- pkg/github/__toolsnaps__/list_tags.snap | 15 ++---- .../__toolsnaps__/pull_request_read.snap | 15 ++---- pkg/github/__toolsnaps__/search_code.snap | 15 ++---- pkg/github/__toolsnaps__/search_issues.snap | 15 ++---- .../__toolsnaps__/search_pull_requests.snap | 15 ++---- .../__toolsnaps__/search_repositories.snap | 15 ++---- pkg/github/__toolsnaps__/search_users.snap | 15 ++---- pkg/github/actions_test.go | 9 +++- pkg/github/discussions_test.go | 15 +++--- pkg/github/gists_test.go | 16 ++++-- pkg/github/issues_test.go | 18 ++++++- pkg/github/pullrequests_test.go | 18 ++++++- pkg/github/repositories_test.go | 54 ++++++++++++++++--- pkg/github/search_test.go | 46 ++++++++++++++-- 22 files changed, 207 insertions(+), 191 deletions(-) diff --git a/pkg/github/__toolsnaps__/get_commit.snap b/pkg/github/__toolsnaps__/get_commit.snap index 1c2ecc9a3..301ec963f 100644 --- a/pkg/github/__toolsnaps__/get_commit.snap +++ b/pkg/github/__toolsnaps__/get_commit.snap @@ -6,6 +6,10 @@ "description": "Get details for a commit from a GitHub repository", "inputSchema": { "properties": { + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "include_diff": { "default": true, "description": "Whether to include file diffs and stats in the response. Default is true.", @@ -15,17 +19,6 @@ "description": "Repository owner", "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" - }, "repo": { "description": "Repository name", "type": "string" diff --git a/pkg/github/__toolsnaps__/issue_read.snap b/pkg/github/__toolsnaps__/issue_read.snap index 9e9462df6..5ee285ca3 100644 --- a/pkg/github/__toolsnaps__/issue_read.snap +++ b/pkg/github/__toolsnaps__/issue_read.snap @@ -6,6 +6,10 @@ "description": "Get information about a specific issue in a GitHub repository.", "inputSchema": { "properties": { + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "issue_number": { "description": "The number of the issue", "type": "number" @@ -24,17 +28,6 @@ "description": "The owner of the repository", "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" - }, "repo": { "description": "The name of the repository", "type": "string" diff --git a/pkg/github/__toolsnaps__/list_branches.snap b/pkg/github/__toolsnaps__/list_branches.snap index 492b6d527..e9fffe28a 100644 --- a/pkg/github/__toolsnaps__/list_branches.snap +++ b/pkg/github/__toolsnaps__/list_branches.snap @@ -6,21 +6,14 @@ "description": "List branches in a GitHub repository", "inputSchema": { "properties": { + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "owner": { "description": "Repository owner", "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" - }, "repo": { "description": "Repository name", "type": "string" diff --git a/pkg/github/__toolsnaps__/list_commits.snap b/pkg/github/__toolsnaps__/list_commits.snap index a802436c2..7da908aec 100644 --- a/pkg/github/__toolsnaps__/list_commits.snap +++ b/pkg/github/__toolsnaps__/list_commits.snap @@ -3,28 +3,21 @@ "title": "List commits", "readOnlyHint": true }, - "description": "Get list of commits of a branch in a GitHub repository. Returns at least 30 results per page by default, but can return more if specified using the perPage parameter (up to 100).", + "description": "Get list of commits of a branch in a GitHub repository. Returns 10 results per page. Use the cursor parameter for pagination.", "inputSchema": { "properties": { "author": { "description": "Author username or email address to filter commits by", "type": "string" }, + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "owner": { "description": "Repository owner", "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" - }, "repo": { "description": "Repository name", "type": "string" diff --git a/pkg/github/__toolsnaps__/list_issues.snap b/pkg/github/__toolsnaps__/list_issues.snap index 5475988c2..15acd4e94 100644 --- a/pkg/github/__toolsnaps__/list_issues.snap +++ b/pkg/github/__toolsnaps__/list_issues.snap @@ -6,8 +6,8 @@ "description": "List issues in a GitHub repository. For pagination, use the 'endCursor' from the previous response's 'pageInfo' in the 'after' parameter.", "inputSchema": { "properties": { - "after": { - "description": "Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs.", + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", "type": "string" }, "direction": { @@ -38,12 +38,6 @@ "description": "Repository owner", "type": "string" }, - "perPage": { - "description": "Results per page for pagination (min 1, max 100)", - "maximum": 100, - "minimum": 1, - "type": "number" - }, "repo": { "description": "Repository name", "type": "string" diff --git a/pkg/github/__toolsnaps__/list_notifications.snap b/pkg/github/__toolsnaps__/list_notifications.snap index 92f25eb4c..8f0f4f619 100644 --- a/pkg/github/__toolsnaps__/list_notifications.snap +++ b/pkg/github/__toolsnaps__/list_notifications.snap @@ -10,6 +10,10 @@ "description": "Only show notifications updated before the given time (ISO 8601 format)", "type": "string" }, + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "filter": { "description": "Filter notifications to, use default unless specified. Read notifications are ones that have already been acknowledged by the user. Participating notifications are those that the user is directly involved in, such as issues or pull requests they have commented on or created.", "enum": [ @@ -23,17 +27,6 @@ "description": "Optional repository owner. If provided with repo, only notifications for this repository are listed.", "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" - }, "repo": { "description": "Optional repository name. If provided with owner, only notifications for this repository are listed.", "type": "string" diff --git a/pkg/github/__toolsnaps__/list_pull_requests.snap b/pkg/github/__toolsnaps__/list_pull_requests.snap index fee7e2ff1..b6667bbf5 100644 --- a/pkg/github/__toolsnaps__/list_pull_requests.snap +++ b/pkg/github/__toolsnaps__/list_pull_requests.snap @@ -10,6 +10,10 @@ "description": "Filter by base branch", "type": "string" }, + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "direction": { "description": "Sort direction", "enum": [ @@ -26,17 +30,6 @@ "description": "Repository owner", "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" - }, "repo": { "description": "Repository name", "type": "string" diff --git a/pkg/github/__toolsnaps__/list_starred_repositories.snap b/pkg/github/__toolsnaps__/list_starred_repositories.snap index b02563ae2..a9c9ece32 100644 --- a/pkg/github/__toolsnaps__/list_starred_repositories.snap +++ b/pkg/github/__toolsnaps__/list_starred_repositories.snap @@ -6,6 +6,10 @@ "description": "List starred repositories", "inputSchema": { "properties": { + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "direction": { "description": "The direction to sort the results by.", "enum": [ @@ -14,17 +18,6 @@ ], "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": "How to sort the results. Can be either 'created' (when the repository was starred) or 'updated' (when the repository was last pushed to).", "enum": [ diff --git a/pkg/github/__toolsnaps__/list_tags.snap b/pkg/github/__toolsnaps__/list_tags.snap index fcb9853fd..1595a7b58 100644 --- a/pkg/github/__toolsnaps__/list_tags.snap +++ b/pkg/github/__toolsnaps__/list_tags.snap @@ -6,21 +6,14 @@ "description": "List git tags in a GitHub repository", "inputSchema": { "properties": { + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "owner": { "description": "Repository owner", "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" - }, "repo": { "description": "Repository name", "type": "string" diff --git a/pkg/github/__toolsnaps__/pull_request_read.snap b/pkg/github/__toolsnaps__/pull_request_read.snap index be9661aae..82ae82b82 100644 --- a/pkg/github/__toolsnaps__/pull_request_read.snap +++ b/pkg/github/__toolsnaps__/pull_request_read.snap @@ -6,6 +6,10 @@ "description": "Get information on a specific pull request in GitHub repository.", "inputSchema": { "properties": { + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "method": { "description": "Action to specify what pull request data needs to be retrieved from GitHub. \nPossible options: \n 1. get - Get details of a specific pull request.\n 2. get_diff - Get the diff of a pull request.\n 3. get_status - Get status of a head commit in a pull request. This reflects status of builds and checks.\n 4. get_files - Get the list of files changed in a pull request. Use with pagination parameters to control the number of results returned.\n 5. get_review_comments - Get the review comments on a pull request. They are comments made on a portion of the unified diff during a pull request review. Use with pagination parameters to control the number of results returned.\n 6. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method.\n 7. get_comments - Get comments on a pull request. Use this if user doesn't specifically want review comments. Use with pagination parameters to control the number of results returned.\n", "enum": [ @@ -23,17 +27,6 @@ "description": "Repository owner", "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" - }, "pullNumber": { "description": "Pull request number", "type": "number" diff --git a/pkg/github/__toolsnaps__/search_code.snap b/pkg/github/__toolsnaps__/search_code.snap index 4ef40c5f8..f28ba74d3 100644 --- a/pkg/github/__toolsnaps__/search_code.snap +++ b/pkg/github/__toolsnaps__/search_code.snap @@ -6,6 +6,10 @@ "description": "Fast and precise code search across ALL GitHub repositories using GitHub's native search engine. Best for finding exact symbols, functions, classes, or specific code patterns.", "inputSchema": { "properties": { + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "order": { "description": "Sort order for results", "enum": [ @@ -14,17 +18,6 @@ ], "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" - }, "query": { "description": "Search query using GitHub's powerful code search syntax. Examples: 'content:Skill language:Java org:github', 'NOT is:archived language:Python OR language:go', 'repo:github/github-mcp-server'. Supports exact matching, language filters, path filters, and more.", "type": "string" diff --git a/pkg/github/__toolsnaps__/search_issues.snap b/pkg/github/__toolsnaps__/search_issues.snap index bf1982411..6b6d9ccf2 100644 --- a/pkg/github/__toolsnaps__/search_issues.snap +++ b/pkg/github/__toolsnaps__/search_issues.snap @@ -6,6 +6,10 @@ "description": "Search for issues in GitHub repositories using issues search syntax already scoped to is:issue", "inputSchema": { "properties": { + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "order": { "description": "Sort order", "enum": [ @@ -18,17 +22,6 @@ "description": "Optional repository owner. If provided with repo, only issues for this repository are listed.", "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" - }, "query": { "description": "Search query using GitHub issues search syntax", "type": "string" diff --git a/pkg/github/__toolsnaps__/search_pull_requests.snap b/pkg/github/__toolsnaps__/search_pull_requests.snap index 811aa1322..bb74550c7 100644 --- a/pkg/github/__toolsnaps__/search_pull_requests.snap +++ b/pkg/github/__toolsnaps__/search_pull_requests.snap @@ -6,6 +6,10 @@ "description": "Search for pull requests in GitHub repositories using issues search syntax already scoped to is:pr", "inputSchema": { "properties": { + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "order": { "description": "Sort order", "enum": [ @@ -18,17 +22,6 @@ "description": "Optional repository owner. If provided with repo, only pull requests for this repository are listed.", "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" - }, "query": { "description": "Search query using GitHub pull request search syntax", "type": "string" diff --git a/pkg/github/__toolsnaps__/search_repositories.snap b/pkg/github/__toolsnaps__/search_repositories.snap index 99828380e..ca5804183 100644 --- a/pkg/github/__toolsnaps__/search_repositories.snap +++ b/pkg/github/__toolsnaps__/search_repositories.snap @@ -6,6 +6,10 @@ "description": "Find GitHub repositories by name, description, readme, topics, or other metadata. Perfect for discovering projects, finding examples, or locating specific repositories across GitHub.", "inputSchema": { "properties": { + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "minimal_output": { "default": true, "description": "Return minimal repository information (default: true). When false, returns full GitHub API repository objects.", @@ -19,17 +23,6 @@ ], "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" - }, "query": { "description": "Repository search query. Examples: 'machine learning in:name stars:\u003e1000 language:python', 'topic:react', 'user:facebook'. Supports advanced search syntax for precise filtering.", "type": "string" diff --git a/pkg/github/__toolsnaps__/search_users.snap b/pkg/github/__toolsnaps__/search_users.snap index 73ff7a43c..ac23497e9 100644 --- a/pkg/github/__toolsnaps__/search_users.snap +++ b/pkg/github/__toolsnaps__/search_users.snap @@ -6,6 +6,10 @@ "description": "Find GitHub users by username, real name, or other profile information. Useful for locating developers, contributors, or team members.", "inputSchema": { "properties": { + "cursor": { + "description": "Pagination cursor. Leave empty for the first page. Use the nextCursor value from the previous response to get the next page.", + "type": "string" + }, "order": { "description": "Sort order", "enum": [ @@ -14,17 +18,6 @@ ], "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" - }, "query": { "description": "User search query. Examples: 'john smith', 'location:seattle', 'followers:\u003e100'. Search is automatically scoped to type:user.", "type": "string" diff --git a/pkg/github/actions_test.go b/pkg/github/actions_test.go index 9a26773a3..b830f63a7 100644 --- a/pkg/github/actions_test.go +++ b/pkg/github/actions_test.go @@ -536,8 +536,15 @@ func Test_ListWorkflowRunArtifacts(t *testing.T) { } // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the artifact list + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var response github.ArtifactList - err = json.Unmarshal([]byte(textContent.Text), &response) + err = json.Unmarshal(dataBytes, &response) require.NoError(t, err) assert.NotNil(t, response.TotalCount) assert.Greater(t, *response.TotalCount, int64(0)) diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index b72f4ec1a..355da08e6 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -618,17 +618,18 @@ func Test_GetDiscussionComments(t *testing.T) { // (Lines removed) + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the response + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var response struct { Comments []*github.IssueComment `json:"comments"` - PageInfo struct { - HasNextPage bool `json:"hasNextPage"` - HasPreviousPage bool `json:"hasPreviousPage"` - StartCursor string `json:"startCursor"` - EndCursor string `json:"endCursor"` - } `json:"pageInfo"` TotalCount int `json:"totalCount"` } - err = json.Unmarshal([]byte(textContent.Text), &response) + err = json.Unmarshal(dataBytes, &response) require.NoError(t, err) assert.Len(t, response.Comments, 2) expectedBodies := []string{"This is the first comment", "This is the second comment"} diff --git a/pkg/github/gists_test.go b/pkg/github/gists_test.go index a89187d5f..8d6c016a7 100644 --- a/pkg/github/gists_test.go +++ b/pkg/github/gists_test.go @@ -100,16 +100,15 @@ func Test_ListGists(t *testing.T) { expectQueryParams(t, map[string]string{ "since": "2023-01-01T00:00:00Z", "page": "2", - "per_page": "5", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockGists), ), ), ), requestArgs: map[string]interface{}{ - "since": "2023-01-01T00:00:00Z", - "page": float64(2), - "perPage": float64(5), + "since": "2023-01-01T00:00:00Z", + "cursor": "page=2;perPage=10", }, expectError: false, expectedGists: mockGists, @@ -176,8 +175,15 @@ func Test_ListGists(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the gists + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedGists []*github.Gist - err = json.Unmarshal([]byte(textContent.Text), &returnedGists) + err = json.Unmarshal(dataBytes, &returnedGists) require.NoError(t, err) assert.Len(t, returnedGists, len(tc.expectedGists)) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 1a861c8e9..892678d0d 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1711,8 +1711,15 @@ func Test_GetIssueComments(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the comments + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedComments []*github.IssueComment - err = json.Unmarshal([]byte(textContent.Text), &returnedComments) + err = json.Unmarshal(dataBytes, &returnedComments) require.NoError(t, err) assert.Equal(t, len(tc.expectedComments), len(returnedComments)) if len(returnedComments) > 0 { @@ -2719,8 +2726,15 @@ func Test_GetSubIssues(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the sub-issues + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedSubIssues []*github.Issue - err = json.Unmarshal([]byte(textContent.Text), &returnedSubIssues) + err = json.Unmarshal(dataBytes, &returnedSubIssues) require.NoError(t, err) assert.Len(t, returnedSubIssues, len(tc.expectedSubIssues)) diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index fdbd5d317..c6ac3bf58 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -1257,8 +1257,15 @@ func Test_GetPullRequestFiles(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the files + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedFiles []*github.CommitFile - err = json.Unmarshal([]byte(textContent.Text), &returnedFiles) + err = json.Unmarshal(dataBytes, &returnedFiles) require.NoError(t, err) assert.Len(t, returnedFiles, len(tc.expectedFiles)) for i, file := range returnedFiles { @@ -1679,8 +1686,15 @@ func Test_GetPullRequestComments(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the comments + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedComments []*github.PullRequestComment - err = json.Unmarshal([]byte(textContent.Text), &returnedComments) + err = json.Unmarshal(dataBytes, &returnedComments) require.NoError(t, err) assert.Len(t, returnedComments, len(tc.expectedComments)) for i, comment := range returnedComments { diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 5ad6d40b6..eaef54f1c 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -741,8 +741,15 @@ func Test_GetCommit(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the commit + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedCommit github.RepositoryCommit - err = json.Unmarshal([]byte(textContent.Text), &returnedCommit) + err = json.Unmarshal(dataBytes, &returnedCommit) require.NoError(t, err) assert.Equal(t, *tc.expectedCommit.SHA, *returnedCommit.SHA) @@ -960,8 +967,15 @@ func Test_ListCommits(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the commits + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedCommits []MinimalCommit - err = json.Unmarshal([]byte(textContent.Text), &returnedCommits) + err = json.Unmarshal(dataBytes, &returnedCommits) require.NoError(t, err) assert.Len(t, returnedCommits, len(tc.expectedCommits)) for i, commit := range returnedCommits { @@ -1764,8 +1778,15 @@ func Test_ListBranches(t *testing.T) { require.NotEmpty(t, textContent.Text) // Verify response + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the branches + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var branches []*github.Branch - err = json.Unmarshal([]byte(textContent.Text), &branches) + err = json.Unmarshal(dataBytes, &branches) require.NoError(t, err) assert.Len(t, branches, 2) assert.Equal(t, "main", *branches[0].Name) @@ -2062,8 +2083,15 @@ func Test_ListTags(t *testing.T) { textContent := getTextResult(t, result) // Parse and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the tags + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedTags []*github.RepositoryTag - err = json.Unmarshal([]byte(textContent.Text), &returnedTags) + err = json.Unmarshal(dataBytes, &returnedTags) require.NoError(t, err) // Verify each tag @@ -2310,8 +2338,15 @@ func Test_ListReleases(t *testing.T) { require.NoError(t, err) textContent := getTextResult(t, result) + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the releases + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedReleases []*github.RepositoryRelease - err = json.Unmarshal([]byte(textContent.Text), &returnedReleases) + err = json.Unmarshal(dataBytes, &returnedReleases) require.NoError(t, err) assert.Len(t, returnedReleases, len(tc.expectedResult)) for i, rel := range returnedReleases { @@ -3051,8 +3086,15 @@ func Test_ListStarredRepositories(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the repositories + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedRepos []MinimalRepository - err = json.Unmarshal([]byte(textContent.Text), &returnedRepos) + err = json.Unmarshal(dataBytes, &returnedRepos) require.NoError(t, err) assert.Len(t, returnedRepos, tc.expectedCount) diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index 37e598714..ed9d43401 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -153,8 +153,15 @@ func Test_SearchRepositories(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the search result + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedResult MinimalSearchRepositoriesResult - err = json.Unmarshal([]byte(textContent.Text), &returnedResult) + err = json.Unmarshal(dataBytes, &returnedResult) require.NoError(t, err) assert.Equal(t, *tc.expectedResult.Total, returnedResult.TotalCount) assert.Equal(t, *tc.expectedResult.IncompleteResults, returnedResult.IncompleteResults) @@ -214,9 +221,17 @@ func Test_SearchRepositories_FullOutput(t *testing.T) { textContent := getTextResult(t, result) + // Unmarshal the paginated response first + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the full search result + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) // Unmarshal as full GitHub API response var returnedResult github.RepositoriesSearchResult - err = json.Unmarshal([]byte(textContent.Text), &returnedResult) + err = json.Unmarshal(dataBytes, &returnedResult) require.NoError(t, err) // Verify it's the full API response, not minimal @@ -364,8 +379,15 @@ func Test_SearchCode(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the search result + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedResult github.CodeSearchResult - err = json.Unmarshal([]byte(textContent.Text), &returnedResult) + err = json.Unmarshal(dataBytes, &returnedResult) require.NoError(t, err) assert.Equal(t, *tc.expectedResult.Total, *returnedResult.Total) assert.Equal(t, *tc.expectedResult.IncompleteResults, *returnedResult.IncompleteResults) @@ -559,8 +581,15 @@ func Test_SearchUsers(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the search result + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedResult MinimalSearchUsersResult - err = json.Unmarshal([]byte(textContent.Text), &returnedResult) + err = json.Unmarshal(dataBytes, &returnedResult) require.NoError(t, err) assert.Equal(t, *tc.expectedResult.Total, returnedResult.TotalCount) assert.Equal(t, *tc.expectedResult.IncompleteResults, returnedResult.IncompleteResults) @@ -722,8 +751,15 @@ func Test_SearchOrgs(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the search result + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedResult MinimalSearchUsersResult - err = json.Unmarshal([]byte(textContent.Text), &returnedResult) + err = json.Unmarshal(dataBytes, &returnedResult) require.NoError(t, err) assert.Equal(t, *tc.expectedResult.Total, returnedResult.TotalCount) assert.Equal(t, *tc.expectedResult.IncompleteResults, returnedResult.IncompleteResults) From 32b0231f33a08fbaada8e8dd3f3af594556dafee Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 11:21:07 +0000 Subject: [PATCH 09/14] Fix more test response parsing - notifications, pull requests, and discussions Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/discussions_test.go | 15 ++++++++------- pkg/github/notifications_test.go | 9 ++++++++- pkg/github/pullrequests_test.go | 9 ++++++++- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index 355da08e6..aa7836a73 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -442,17 +442,18 @@ func Test_ListDiscussions(t *testing.T) { require.NoError(t, err) // Parse the structured response with pagination info + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the response + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var response struct { Discussions []*github.Discussion `json:"discussions"` - PageInfo struct { - HasNextPage bool `json:"hasNextPage"` - HasPreviousPage bool `json:"hasPreviousPage"` - StartCursor string `json:"startCursor"` - EndCursor string `json:"endCursor"` - } `json:"pageInfo"` TotalCount int `json:"totalCount"` } - err = json.Unmarshal([]byte(text), &response) + err = json.Unmarshal(dataBytes, &response) require.NoError(t, err) assert.Len(t, response.Discussions, tc.expectedCount, "Expected %d discussions, got %d", tc.expectedCount, len(response.Discussions)) diff --git a/pkg/github/notifications_test.go b/pkg/github/notifications_test.go index e4c2cf3c2..d19a8bcb0 100644 --- a/pkg/github/notifications_test.go +++ b/pkg/github/notifications_test.go @@ -139,8 +139,15 @@ func Test_ListNotifications(t *testing.T) { require.False(t, result.IsError) textContent := getTextResult(t, result) t.Logf("textContent: %s", textContent.Text) + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the notifications + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returned []*github.Notification - err = json.Unmarshal([]byte(textContent.Text), &returned) + err = json.Unmarshal(dataBytes, &returned) require.NoError(t, err) require.NotEmpty(t, returned) assert.Equal(t, *tc.expectedResult[0].ID, *returned[0].ID) diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index c6ac3bf58..0239f0bf7 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -694,8 +694,15 @@ func Test_ListPullRequests(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the pull requests + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedPRs []*github.PullRequest - err = json.Unmarshal([]byte(textContent.Text), &returnedPRs) + err = json.Unmarshal(dataBytes, &returnedPRs) require.NoError(t, err) assert.Len(t, returnedPRs, 2) assert.Equal(t, *tc.expectedPRs[0].Number, *returnedPRs[0].Number) From 8e532ae472213ad0e100a4d494d3ca35ce9276a6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 11:24:47 +0000 Subject: [PATCH 10/14] Fix remaining test pagination parameters to use cursor format Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/pullrequests_test.go | 4 +--- pkg/github/repositories_test.go | 15 +++++++-------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 0239f0bf7..67ee8282a 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -626,7 +626,7 @@ func Test_ListPullRequests(t *testing.T) { "state": "all", "sort": "created", "direction": "desc", - "per_page": "30", + "per_page": "11", "page": "1", }).andThen( mockResponse(t, http.StatusOK, mockPRs), @@ -639,8 +639,6 @@ func Test_ListPullRequests(t *testing.T) { "state": "all", "sort": "created", "direction": "desc", - "perPage": float64(30), - "page": float64(1), }, expectError: false, expectedPRs: mockPRs, diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index eaef54f1c..e1594cdf1 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -904,17 +904,16 @@ func Test_ListCommits(t *testing.T) { mock.GetReposCommitsByOwnerByRepo, expectQueryParams(t, map[string]string{ "page": "2", - "per_page": "10", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockCommits), ), ), ), requestArgs: map[string]interface{}{ - "owner": "owner", - "repo": "repo", - "page": float64(2), - "perPage": float64(10), + "owner": "owner", + "repo": "repo", + "cursor": "page=2;perPage=10", }, expectError: false, expectedCommits: mockCommits, @@ -1714,9 +1713,9 @@ func Test_ListBranches(t *testing.T) { { name: "success", args: map[string]interface{}{ - "owner": "owner", - "repo": "repo", - "page": float64(2), + "owner": "owner", + "repo": "repo", + "cursor": "page=2;perPage=10", }, mockResponses: []mock.MockBackendOption{ mock.WithRequestMatch( From 252081f0d1bd9d6529c7fb46a70ad115c6599fa8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 11:27:14 +0000 Subject: [PATCH 11/14] Update test mock expectations from per_page=30 to per_page=11 Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/issues_test.go | 16 ++++++++-------- pkg/github/pullrequests_test.go | 14 +++++++------- pkg/github/repositories_test.go | 2 +- pkg/github/search_test.go | 22 +++++++++++----------- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 892678d0d..6ba99cf4c 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -307,7 +307,7 @@ func Test_SearchIssues(t *testing.T) { "sort": "created", "order": "desc", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -336,7 +336,7 @@ func Test_SearchIssues(t *testing.T) { "sort": "created", "order": "asc", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -363,7 +363,7 @@ func Test_SearchIssues(t *testing.T) { map[string]string{ "q": "is:issue bug", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -387,7 +387,7 @@ func Test_SearchIssues(t *testing.T) { map[string]string{ "q": "is:issue feature", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -425,7 +425,7 @@ func Test_SearchIssues(t *testing.T) { map[string]string{ "q": "repo:github/github-mcp-server is:issue is:open (label:critical OR label:urgent)", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -448,7 +448,7 @@ func Test_SearchIssues(t *testing.T) { map[string]string{ "q": "is:issue repo:github/github-mcp-server critical", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -473,7 +473,7 @@ func Test_SearchIssues(t *testing.T) { map[string]string{ "q": "is:issue repo:octocat/Hello-World bug", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -496,7 +496,7 @@ func Test_SearchIssues(t *testing.T) { map[string]string{ "q": "repo:github/github-mcp-server is:issue (label:critical OR label:urgent OR label:high-priority OR label:blocker)", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 67ee8282a..96241155f 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -892,7 +892,7 @@ func Test_SearchPullRequests(t *testing.T) { "sort": "created", "order": "desc", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -921,7 +921,7 @@ func Test_SearchPullRequests(t *testing.T) { "sort": "updated", "order": "asc", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -948,7 +948,7 @@ func Test_SearchPullRequests(t *testing.T) { map[string]string{ "q": "is:pr feature", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -972,7 +972,7 @@ func Test_SearchPullRequests(t *testing.T) { map[string]string{ "q": "is:pr review-required", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -1010,7 +1010,7 @@ func Test_SearchPullRequests(t *testing.T) { map[string]string{ "q": "is:pr repo:github/github-mcp-server is:open draft:false", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -1033,7 +1033,7 @@ func Test_SearchPullRequests(t *testing.T) { map[string]string{ "q": "is:pr repo:github/github-mcp-server author:octocat", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), @@ -1058,7 +1058,7 @@ func Test_SearchPullRequests(t *testing.T) { map[string]string{ "q": "is:pr repo:github/github-mcp-server (label:bug OR label:enhancement OR label:feature)", "page": "1", - "per_page": "30", + "per_page": "11", }, ).andThen( mockResponse(t, http.StatusOK, mockSearchResult), diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index e1594cdf1..fd7f64b11 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -882,7 +882,7 @@ func Test_ListCommits(t *testing.T) { "author": "username", "sha": "main", "page": "1", - "per_page": "30", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockCommits), ), diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index ed9d43401..56a03d74a 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -94,7 +94,7 @@ func Test_SearchRepositories(t *testing.T) { expectQueryParams(t, map[string]string{ "q": "golang test", "page": "1", - "per_page": "30", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSearchResult), ), @@ -199,7 +199,7 @@ func Test_SearchRepositories_FullOutput(t *testing.T) { expectQueryParams(t, map[string]string{ "q": "golang test", "page": "1", - "per_page": "30", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSearchResult), ), @@ -296,7 +296,7 @@ func Test_SearchCode(t *testing.T) { "sort": "indexed", "order": "desc", "page": "1", - "per_page": "30", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSearchResult), ), @@ -320,7 +320,7 @@ func Test_SearchCode(t *testing.T) { expectQueryParams(t, map[string]string{ "q": "fmt.Println language:go", "page": "1", - "per_page": "30", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSearchResult), ), @@ -456,7 +456,7 @@ func Test_SearchUsers(t *testing.T) { "sort": "followers", "order": "desc", "page": "1", - "per_page": "30", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSearchResult), ), @@ -480,7 +480,7 @@ func Test_SearchUsers(t *testing.T) { expectQueryParams(t, map[string]string{ "q": "type:user location:finland language:go", "page": "1", - "per_page": "30", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSearchResult), ), @@ -500,7 +500,7 @@ func Test_SearchUsers(t *testing.T) { expectQueryParams(t, map[string]string{ "q": "type:user location:seattle followers:>100", "page": "1", - "per_page": "30", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSearchResult), ), @@ -520,7 +520,7 @@ func Test_SearchUsers(t *testing.T) { expectQueryParams(t, map[string]string{ "q": "type:user (location:seattle OR location:california) followers:>50", "page": "1", - "per_page": "30", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSearchResult), ), @@ -653,7 +653,7 @@ func Test_SearchOrgs(t *testing.T) { expectQueryParams(t, map[string]string{ "q": "type:org github", "page": "1", - "per_page": "30", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSearchResult), ), @@ -673,7 +673,7 @@ func Test_SearchOrgs(t *testing.T) { expectQueryParams(t, map[string]string{ "q": "type:org location:california followers:>1000", "page": "1", - "per_page": "30", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSearchResult), ), @@ -693,7 +693,7 @@ func Test_SearchOrgs(t *testing.T) { expectQueryParams(t, map[string]string{ "q": "type:org (location:seattle OR location:california OR location:newyork) repos:>10", "page": "1", - "per_page": "30", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSearchResult), ), From ed5c3760108d10e78f8da0caeddf39f01b495975 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 11:30:44 +0000 Subject: [PATCH 12/14] Fix GraphQL test response structures and remaining pagination params Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/issues_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 6ba99cf4c..88d4fd975 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1009,17 +1009,18 @@ func Test_ListIssues(t *testing.T) { require.NoError(t, err) // Parse the structured response with pagination info + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the response + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var response struct { Issues []*github.Issue `json:"issues"` - PageInfo struct { - HasNextPage bool `json:"hasNextPage"` - HasPreviousPage bool `json:"hasPreviousPage"` - StartCursor string `json:"startCursor"` - EndCursor string `json:"endCursor"` - } `json:"pageInfo"` TotalCount int `json:"totalCount"` } - err = json.Unmarshal([]byte(text), &response) + err = json.Unmarshal(dataBytes, &response) require.NoError(t, err) assert.Len(t, response.Issues, tc.expectedCount, "Expected %d issues, got %d", tc.expectedCount, len(response.Issues)) @@ -1651,7 +1652,7 @@ func Test_GetIssueComments(t *testing.T) { mock.GetReposIssuesCommentsByOwnerByRepoByIssueNumber, expectQueryParams(t, map[string]string{ "page": "2", - "per_page": "10", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockComments), ), @@ -1662,8 +1663,7 @@ func Test_GetIssueComments(t *testing.T) { "owner": "owner", "repo": "repo", "issue_number": float64(42), - "page": float64(2), - "perPage": float64(10), + "cursor": "page=2;perPage=10", }, expectError: false, expectedComments: mockComments, From 2936eef3df742557aa85a6af597e84a6f43d72f8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 11:33:40 +0000 Subject: [PATCH 13/14] Fix SearchRepositories test pagination parameters Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/search_test.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index 56a03d74a..e8a47c94e 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -70,18 +70,17 @@ func Test_SearchRepositories(t *testing.T) { "sort": "stars", "order": "desc", "page": "2", - "per_page": "10", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSearchResult), ), ), ), requestArgs: map[string]interface{}{ - "query": "golang test", - "sort": "stars", - "order": "desc", - "page": float64(2), - "perPage": float64(10), + "query": "golang test", + "sort": "stars", + "order": "desc", + "cursor": "page=2;perPage=10", }, expectError: false, expectedResult: mockSearchResult, @@ -306,8 +305,6 @@ func Test_SearchCode(t *testing.T) { "query": "fmt.Println language:go", "sort": "indexed", "order": "desc", - "page": float64(1), - "perPage": float64(30), }, expectError: false, expectedResult: mockSearchResult, @@ -466,8 +463,6 @@ func Test_SearchUsers(t *testing.T) { "query": "location:finland language:go", "sort": "followers", "order": "desc", - "page": float64(1), - "perPage": float64(30), }, expectError: false, expectedResult: mockSearchResult, From b615d81b49639668d939bec3b3783e1f3859809e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 11:55:31 +0000 Subject: [PATCH 14/14] Complete cursor-based pagination test fixes - all tests passing Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/github/discussions_test.go | 16 ++++++++-------- pkg/github/issues_test.go | 34 ++++++++++++++++++--------------- pkg/github/pullrequests_test.go | 20 +++++++++++-------- pkg/github/search_utils.go | 26 +++++++++++++++---------- 4 files changed, 55 insertions(+), 41 deletions(-) diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index aa7836a73..030053db2 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -212,14 +212,14 @@ func Test_ListDiscussions(t *testing.T) { varsListAll := map[string]interface{}{ "owner": "owner", "repo": "repo", - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } varsRepoNotFound := map[string]interface{}{ "owner": "owner", "repo": "nonexistent-repo", - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } @@ -227,7 +227,7 @@ func Test_ListDiscussions(t *testing.T) { "owner": "owner", "repo": "repo", "categoryId": "DIC_kwDOABC123", - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } @@ -236,7 +236,7 @@ func Test_ListDiscussions(t *testing.T) { "repo": "repo", "orderByField": "CREATED_AT", "orderByDirection": "ASC", - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } @@ -245,7 +245,7 @@ func Test_ListDiscussions(t *testing.T) { "repo": "repo", "orderByField": "UPDATED_AT", "orderByDirection": "DESC", - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } @@ -255,14 +255,14 @@ func Test_ListDiscussions(t *testing.T) { "categoryId": "DIC_kwDOABC123", "orderByField": "CREATED_AT", "orderByDirection": "DESC", - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } varsOrgLevel := map[string]interface{}{ "owner": "owner", "repo": ".github", // This is what gets set when repo is not provided - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } @@ -578,7 +578,7 @@ func Test_GetDiscussionComments(t *testing.T) { "owner": "owner", "repo": "repo", "discussionNumber": float64(1), - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 88d4fd975..5119f8b1b 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -315,11 +315,9 @@ func Test_SearchIssues(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "query": "repo:owner/repo is:open", - "sort": "created", - "order": "desc", - "page": float64(1), - "perPage": float64(30), + "query": "repo:owner/repo is:open", + "sort": "created", + "order": "desc", }, expectError: false, expectedResult: mockSearchResult, @@ -553,8 +551,15 @@ func Test_SearchIssues(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the search result + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedResult github.IssuesSearchResult - err = json.Unmarshal([]byte(textContent.Text), &returnedResult) + err = json.Unmarshal(dataBytes, &returnedResult) require.NoError(t, err) assert.Equal(t, *tc.expectedResult.Total, *returnedResult.Total) assert.Equal(t, *tc.expectedResult.IncompleteResults, *returnedResult.IncompleteResults) @@ -747,7 +752,7 @@ func Test_ListIssues(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "orderBy") assert.Contains(t, tool.InputSchema.Properties, "direction") assert.Contains(t, tool.InputSchema.Properties, "since") - assert.Contains(t, tool.InputSchema.Properties, "after") + assert.Contains(t, tool.InputSchema.Properties, "cursor") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) // Mock issues data @@ -865,7 +870,7 @@ func Test_ListIssues(t *testing.T) { "states": []interface{}{"OPEN", "CLOSED"}, "orderBy": "CREATED_AT", "direction": "DESC", - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } @@ -875,7 +880,7 @@ func Test_ListIssues(t *testing.T) { "states": []interface{}{"OPEN"}, "orderBy": "CREATED_AT", "direction": "DESC", - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } @@ -885,7 +890,7 @@ func Test_ListIssues(t *testing.T) { "states": []interface{}{"CLOSED"}, "orderBy": "CREATED_AT", "direction": "DESC", - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } @@ -896,7 +901,7 @@ func Test_ListIssues(t *testing.T) { "labels": []interface{}{"bug", "enhancement"}, "orderBy": "CREATED_AT", "direction": "DESC", - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } @@ -906,7 +911,7 @@ func Test_ListIssues(t *testing.T) { "states": []interface{}{"OPEN", "CLOSED"}, "orderBy": "CREATED_AT", "direction": "DESC", - "first": float64(30), + "first": float64(10), "after": (*string)(nil), } @@ -2580,7 +2585,7 @@ func Test_GetSubIssues(t *testing.T) { mock.GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber, expectQueryParams(t, map[string]string{ "page": "2", - "per_page": "10", + "per_page": "11", }).andThen( mockResponse(t, http.StatusOK, mockSubIssues), ), @@ -2591,8 +2596,7 @@ func Test_GetSubIssues(t *testing.T) { "owner": "owner", "repo": "repo", "issue_number": float64(42), - "page": float64(2), - "perPage": float64(10), + "cursor": "page=2;perPage=10", }, expectError: false, expectedSubIssues: mockSubIssues, diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 96241155f..8a937bedb 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -900,11 +900,9 @@ func Test_SearchPullRequests(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "query": "repo:owner/repo is:open", - "sort": "created", - "order": "desc", - "page": float64(1), - "perPage": float64(30), + "query": "repo:owner/repo is:open", + "sort": "created", + "order": "desc", }, expectError: false, expectedResult: mockSearchResult, @@ -1115,8 +1113,15 @@ func Test_SearchPullRequests(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result + var paginatedResponse PaginatedResponse + err = json.Unmarshal([]byte(textContent.Text), &paginatedResponse) + require.NoError(t, err) + + // The data field contains the search result + dataBytes, err := json.Marshal(paginatedResponse.Data) + require.NoError(t, err) var returnedResult github.IssuesSearchResult - err = json.Unmarshal([]byte(textContent.Text), &returnedResult) + err = json.Unmarshal(dataBytes, &returnedResult) require.NoError(t, err) assert.Equal(t, *tc.expectedResult.Total, *returnedResult.Total) assert.Equal(t, *tc.expectedResult.IncompleteResults, *returnedResult.IncompleteResults) @@ -1206,8 +1211,7 @@ func Test_GetPullRequestFiles(t *testing.T) { "owner": "owner", "repo": "repo", "pullNumber": float64(42), - "page": float64(2), - "perPage": float64(10), + "cursor": "page=2;perPage=10", }, expectError: false, expectedFiles: mockFiles, diff --git a/pkg/github/search_utils.go b/pkg/github/search_utils.go index 00c5ae34b..08ebab31d 100644 --- a/pkg/github/search_utils.go +++ b/pkg/github/search_utils.go @@ -2,7 +2,6 @@ package github import ( "context" - "encoding/json" "fmt" "io" "net/http" @@ -73,9 +72,11 @@ func searchHandler( if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := OptionalPaginationParams(request) + + // Get cursor-based pagination parameters + cursorParams, err := GetCursorBasedParams(request) if err != nil { - return mcp.NewToolResultError(err.Error()), nil + return nil, err } opts := &github.SearchOptions{ @@ -83,8 +84,8 @@ func searchHandler( Sort: sort, Order: order, ListOptions: github.ListOptions{ - Page: pagination.Page, - PerPage: pagination.PerPage, + Page: cursorParams.Page, + PerPage: cursorParams.PerPage + 1, // Request one extra to check for more results }, } @@ -106,10 +107,15 @@ func searchHandler( return mcp.NewToolResultError(fmt.Sprintf("%s: %s", errorPrefix, string(body))), nil } - r, err := json.Marshal(result) - if err != nil { - return nil, fmt.Errorf("%s: failed to marshal response: %w", errorPrefix, err) + // Check if there are more results + hasMore := len(result.Issues) > cursorParams.PerPage + + // Trim to requested page size + if hasMore { + result.Issues = result.Issues[:cursorParams.PerPage] } - - return mcp.NewToolResultText(string(r)), nil + + // Create paginated response + paginatedResp := NewPaginatedRESTResponse(result, cursorParams.Page, cursorParams.PerPage, hasMore) + return MarshalPaginatedResponse(paginatedResp), nil }