Skip to content

Commit 9a08648

Browse files
mattdhollowaymattdholloway
committed
move to unified pagination for consensus on params
1 parent 88f1255 commit 9a08648

File tree

2 files changed

+133
-10
lines changed

2 files changed

+133
-10
lines changed

‎pkg/github/discussions.go‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
3131
mcp.WithString("category",
3232
mcp.Description("Optional filter by discussion category ID. If provided, only discussions with this category are listed."),
3333
),
34-
WithGraphQLPagination(),
34+
WithUnifiedPagination(),
3535
),
3636
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
3737
// Required params
@@ -50,11 +50,12 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
5050
return mcp.NewToolResultError(err.Error()), nil
5151
}
5252

53-
// Get pagination parameters
54-
pagination, err := OptionalGraphQLPaginationParams(request)
53+
// Get unified pagination parameters and convert to GraphQL format
54+
unifiedPagination, err := OptionalUnifiedPaginationParams(request)
5555
if err != nil {
5656
return mcp.NewToolResultError(err.Error()), nil
5757
}
58+
pagination := unifiedPagination.ToGraphQLParams()
5859

5960
client, err := getGQLClient(ctx)
6061
if err != nil {
@@ -272,7 +273,7 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati
272273
mcp.WithString("owner", mcp.Required(), mcp.Description("Repository owner")),
273274
mcp.WithString("repo", mcp.Required(), mcp.Description("Repository name")),
274275
mcp.WithNumber("discussionNumber", mcp.Required(), mcp.Description("Discussion Number")),
275-
WithGraphQLPagination(),
276+
WithUnifiedPagination(),
276277
),
277278
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
278279
// Decode params
@@ -285,10 +286,12 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati
285286
return mcp.NewToolResultError(err.Error()), nil
286287
}
287288

288-
paginationParams, err := OptionalGraphQLPaginationParams(request)
289+
// Get unified pagination parameters and convert to GraphQL format
290+
unifiedPagination, err := OptionalUnifiedPaginationParams(request)
289291
if err != nil {
290292
return mcp.NewToolResultError(err.Error()), nil
291293
}
294+
paginationParams := unifiedPagination.ToGraphQLParams()
292295

293296
// Use default of 100 if neither first nor last is specified
294297
if paginationParams.First == nil && paginationParams.Last == nil {
@@ -356,7 +359,7 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl
356359
mcp.Required(),
357360
mcp.Description("Repository name"),
358361
),
359-
WithGraphQLPagination(),
362+
WithUnifiedPagination(),
360363
),
361364
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
362365
// Decode params
@@ -368,10 +371,12 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl
368371
return mcp.NewToolResultError(err.Error()), nil
369372
}
370373

371-
pagination, err := OptionalGraphQLPaginationParams(request)
374+
// Get unified pagination parameters and convert to GraphQL format
375+
unifiedPagination, err := OptionalUnifiedPaginationParams(request)
372376
if err != nil {
373377
return mcp.NewToolResultError(err.Error()), nil
374378
}
379+
pagination := unifiedPagination.ToGraphQLParams()
375380

376381
// Use default of 100 if neither first nor last is specified
377382
if pagination.First == nil && pagination.Last == nil {

‎pkg/github/server.go‎

Lines changed: 121 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,24 @@ func WithPagination() mcp.ToolOption {
191191
}
192192
}
193193

194-
// WithGraphQLPagination adds GraphQL cursor-based pagination parameters to a tool.
195-
// https://docs.github.com/en/graphql/reference/objects#connection
196-
func WithGraphQLPagination() mcp.ToolOption {
194+
// WithUnifiedPagination adds both REST and GraphQL pagination parameters to a tool.
195+
// This allows tools to accept both page/perPage and cursor-based pagination parameters.
196+
// GraphQL tools should use this and convert page/perPage to first/after internally.
197+
func WithUnifiedPagination() mcp.ToolOption {
197198
return func(tool *mcp.Tool) {
199+
// REST API pagination parameters
200+
mcp.WithNumber("page",
201+
mcp.Description("Page number for pagination (min 1)"),
202+
mcp.Min(1),
203+
)(tool)
204+
205+
mcp.WithNumber("perPage",
206+
mcp.Description("Results per page for pagination (min 1, max 100)"),
207+
mcp.Min(1),
208+
mcp.Max(100),
209+
)(tool)
210+
211+
// GraphQL cursor-based pagination parameters
198212
mcp.WithNumber("first",
199213
mcp.Description("Number of items to return per page (min 1, max 100)"),
200214
mcp.Min(1),
@@ -249,6 +263,110 @@ type GraphQLPaginationParams struct {
249263
Before *string
250264
}
251265

266+
// UnifiedPaginationParams contains both REST and GraphQL pagination parameters
267+
type UnifiedPaginationParams struct {
268+
// REST API pagination
269+
Page int
270+
PerPage int
271+
272+
// GraphQL cursor-based pagination
273+
First *int32
274+
Last *int32
275+
After *string
276+
Before *string
277+
}
278+
279+
// ToGraphQLParams converts unified pagination parameters to GraphQL-specific parameters.
280+
// If cursor-based parameters (first/last/after/before) are provided, they take precedence.
281+
// Otherwise, page/perPage are converted to first/after equivalent.
282+
func (u UnifiedPaginationParams) ToGraphQLParams() GraphQLPaginationParams {
283+
// If any cursor-based parameters are explicitly set, use them directly
284+
if u.First != nil || u.Last != nil || u.After != nil || u.Before != nil {
285+
return GraphQLPaginationParams{
286+
First: u.First,
287+
Last: u.Last,
288+
After: u.After,
289+
Before: u.Before,
290+
}
291+
}
292+
293+
// Convert page/perPage to GraphQL parameters
294+
// For GraphQL, we use 'first' for perPage and ignore page for the initial request
295+
// (subsequent requests would use 'after' cursor from previous response)
296+
first := int32(u.PerPage)
297+
return GraphQLPaginationParams{
298+
First: &first,
299+
Last: nil,
300+
After: nil,
301+
Before: nil,
302+
}
303+
}
304+
305+
// OptionalUnifiedPaginationParams returns unified pagination parameters from the request.
306+
// It accepts both REST API (page/perPage) and GraphQL (first/last/after/before) parameters.
307+
func OptionalUnifiedPaginationParams(r mcp.CallToolRequest) (UnifiedPaginationParams, error) {
308+
var params UnifiedPaginationParams
309+
310+
// Get REST API pagination parameters with defaults
311+
page, err := OptionalIntParamWithDefault(r, "page", 1)
312+
if err != nil {
313+
return UnifiedPaginationParams{}, err
314+
}
315+
params.Page = page
316+
317+
perPage, err := OptionalIntParamWithDefault(r, "perPage", 30)
318+
if err != nil {
319+
return UnifiedPaginationParams{}, err
320+
}
321+
params.PerPage = perPage
322+
323+
// Get GraphQL pagination parameters
324+
if val, err := OptionalParam[float64](r, "first"); err != nil {
325+
return UnifiedPaginationParams{}, err
326+
} else if val != 0 {
327+
first := int32(val)
328+
params.First = &first
329+
}
330+
331+
if val, err := OptionalParam[float64](r, "last"); err != nil {
332+
return UnifiedPaginationParams{}, err
333+
} else if val != 0 {
334+
last := int32(val)
335+
params.Last = &last
336+
}
337+
338+
if val, err := OptionalParam[string](r, "after"); err != nil {
339+
return UnifiedPaginationParams{}, err
340+
} else if val != "" {
341+
params.After = &val
342+
}
343+
344+
if val, err := OptionalParam[string](r, "before"); err != nil {
345+
return UnifiedPaginationParams{}, err
346+
} else if val != "" {
347+
params.Before = &val
348+
}
349+
350+
// Validate GraphQL pagination parameters according to GraphQL connection spec
351+
// Only validate if any GraphQL parameters are provided
352+
if params.First != nil || params.Last != nil || params.After != nil || params.Before != nil {
353+
if params.First != nil && params.Last != nil {
354+
return UnifiedPaginationParams{}, fmt.Errorf("only one of 'first' or 'last' may be specified")
355+
}
356+
if params.After != nil && params.Before != nil {
357+
return UnifiedPaginationParams{}, fmt.Errorf("only one of 'after' or 'before' may be specified")
358+
}
359+
if params.After != nil && params.Last != nil {
360+
return UnifiedPaginationParams{}, fmt.Errorf("'after' cannot be used with 'last'. Did you mean to use 'before' instead?")
361+
}
362+
if params.Before != nil && params.First != nil {
363+
return UnifiedPaginationParams{}, fmt.Errorf("'before' cannot be used with 'first'. Did you mean to use 'after' instead?")
364+
}
365+
}
366+
367+
return params, nil
368+
}
369+
252370
// OptionalGraphQLPaginationParams returns the GraphQL cursor-based pagination parameters from the request.
253371
// It validates that the parameters are used correctly according to GraphQL connection spec.
254372
func OptionalGraphQLPaginationParams(r mcp.CallToolRequest) (GraphQLPaginationParams, error) {

0 commit comments

Comments
 (0)