Skip to content

Commit 1099990

Browse files
mattdhollowaymattdholloway
committed
add 'after' for pagination
1 parent 98fd144 commit 1099990

File tree

3 files changed

+126
-28
lines changed

3 files changed

+126
-28
lines changed

‎pkg/github/discussions.go‎

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
9393
HasNextPage bool
9494
EndCursor string
9595
}
96-
} `graphql:"discussions(first: $first, categoryId: $categoryId)"`
96+
} `graphql:"discussions(first: $first, after: $after, categoryId: $categoryId)"`
9797
} `graphql:"repository(owner: $owner, name: $repo)"`
9898
}
9999
vars := map[string]interface{}{
@@ -102,6 +102,11 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
102102
"categoryId": *categoryID,
103103
"first": githubv4.Int(*paginationParams.First),
104104
}
105+
if paginationParams.After != nil {
106+
vars["after"] = githubv4.String(*paginationParams.After)
107+
} else {
108+
vars["after"] = (*githubv4.String)(nil)
109+
}
105110
if err := client.Query(ctx, &query, vars); err != nil {
106111
return mcp.NewToolResultError(err.Error()), nil
107112
}
@@ -120,7 +125,16 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
120125
discussions = append(discussions, di)
121126
}
122127

123-
out, err = json.Marshal(discussions)
128+
// Create response with pagination info
129+
response := map[string]interface{}{
130+
"discussions": discussions,
131+
"pageInfo": map[string]interface{}{
132+
"hasNextPage": query.Repository.Discussions.PageInfo.HasNextPage,
133+
"endCursor": query.Repository.Discussions.PageInfo.EndCursor,
134+
},
135+
}
136+
137+
out, err = json.Marshal(response)
124138
if err != nil {
125139
return nil, fmt.Errorf("failed to marshal discussions: %w", err)
126140
}
@@ -142,14 +156,19 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
142156
HasNextPage bool
143157
EndCursor string
144158
}
145-
} `graphql:"discussions(first: $first)"`
159+
} `graphql:"discussions(first: $first, after: $after)"`
146160
} `graphql:"repository(owner: $owner, name: $repo)"`
147161
}
148162
vars := map[string]interface{}{
149163
"owner": githubv4.String(owner),
150164
"repo": githubv4.String(repo),
151165
"first": githubv4.Int(*paginationParams.First),
152166
}
167+
if paginationParams.After != nil {
168+
vars["after"] = githubv4.String(*paginationParams.After)
169+
} else {
170+
vars["after"] = (*githubv4.String)(nil)
171+
}
153172
if err := client.Query(ctx, &query, vars); err != nil {
154173
return mcp.NewToolResultError(err.Error()), nil
155174
}
@@ -168,7 +187,16 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp
168187
discussions = append(discussions, di)
169188
}
170189

171-
out, err = json.Marshal(discussions)
190+
// Create response with pagination info
191+
response := map[string]interface{}{
192+
"discussions": discussions,
193+
"pageInfo": map[string]interface{}{
194+
"hasNextPage": query.Repository.Discussions.PageInfo.HasNextPage,
195+
"endCursor": query.Repository.Discussions.PageInfo.EndCursor,
196+
},
197+
}
198+
199+
out, err = json.Marshal(response)
172200
if err != nil {
173201
return nil, fmt.Errorf("failed to marshal discussions: %w", err)
174202
}
@@ -314,7 +342,7 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati
314342
HasNextPage githubv4.Boolean
315343
EndCursor githubv4.String
316344
}
317-
} `graphql:"comments(first: $first)"`
345+
} `graphql:"comments(first: $first, after: $after)"`
318346
} `graphql:"discussion(number: $discussionNumber)"`
319347
} `graphql:"repository(owner: $owner, name: $repo)"`
320348
}
@@ -324,6 +352,11 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati
324352
"discussionNumber": githubv4.Int(params.DiscussionNumber),
325353
"first": githubv4.Int(*paginationParams.First),
326354
}
355+
if paginationParams.After != nil {
356+
vars["after"] = githubv4.String(*paginationParams.After)
357+
} else {
358+
vars["after"] = (*githubv4.String)(nil)
359+
}
327360
if err := client.Query(ctx, &q, vars); err != nil {
328361
return mcp.NewToolResultError(err.Error()), nil
329362
}
@@ -333,7 +366,16 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati
333366
comments = append(comments, &github.IssueComment{Body: github.Ptr(string(c.Body))})
334367
}
335368

336-
out, err := json.Marshal(comments)
369+
// Create response with pagination info
370+
response := map[string]interface{}{
371+
"comments": comments,
372+
"pageInfo": map[string]interface{}{
373+
"hasNextPage": q.Repository.Discussion.Comments.PageInfo.HasNextPage,
374+
"endCursor": string(q.Repository.Discussion.Comments.PageInfo.EndCursor),
375+
},
376+
}
377+
378+
out, err := json.Marshal(response)
337379
if err != nil {
338380
return nil, fmt.Errorf("failed to marshal comments: %w", err)
339381
}
@@ -407,14 +449,19 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl
407449
HasNextPage githubv4.Boolean
408450
EndCursor githubv4.String
409451
}
410-
} `graphql:"discussionCategories(first: $first)"`
452+
} `graphql:"discussionCategories(first: $first, after: $after)"`
411453
} `graphql:"repository(owner: $owner, name: $repo)"`
412454
}
413455
vars := map[string]interface{}{
414456
"owner": githubv4.String(params.Owner),
415457
"repo": githubv4.String(params.Repo),
416458
"first": githubv4.Int(*paginationParams.First),
417459
}
460+
if paginationParams.After != nil {
461+
vars["after"] = githubv4.String(*paginationParams.After)
462+
} else {
463+
vars["after"] = (*githubv4.String)(nil)
464+
}
418465
if err := client.Query(ctx, &q, vars); err != nil {
419466
return mcp.NewToolResultError(err.Error()), nil
420467
}
@@ -427,7 +474,16 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl
427474
})
428475
}
429476

430-
out, err := json.Marshal(categories)
477+
// Create response with pagination info
478+
response := map[string]interface{}{
479+
"categories": categories,
480+
"pageInfo": map[string]interface{}{
481+
"hasNextPage": q.Repository.DiscussionCategories.PageInfo.HasNextPage,
482+
"endCursor": string(q.Repository.DiscussionCategories.PageInfo.EndCursor),
483+
},
484+
}
485+
486+
out, err := json.Marshal(response)
431487
if err != nil {
432488
return nil, fmt.Errorf("failed to marshal discussion categories: %w", err)
433489
}

‎pkg/github/discussions_test.go‎

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -61,28 +61,31 @@ func Test_ListDiscussions(t *testing.T) {
6161
assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo"})
6262

6363
// Use exact string queries that match implementation output (from error messages)
64-
qDiscussions := "query($first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first){nodes{number,title,createdAt,category{name},url},pageInfo{hasNextPage,endCursor}}}}"
64+
qDiscussions := "query($after:String$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after){nodes{number,title,createdAt,category{name},url},pageInfo{hasNextPage,endCursor}}}}"
6565

66-
qDiscussionsFiltered := "query($categoryId:ID!$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, categoryId: $categoryId){nodes{number,title,createdAt,category{name},url},pageInfo{hasNextPage,endCursor}}}}"
66+
qDiscussionsFiltered := "query($after:String$categoryId:ID!$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after, categoryId: $categoryId){nodes{number,title,createdAt,category{name},url},pageInfo{hasNextPage,endCursor}}}}"
6767

6868
// Variables matching what GraphQL receives after JSON marshaling/unmarshaling
6969
varsListAll := map[string]interface{}{
7070
"owner": "owner",
7171
"repo": "repo",
7272
"first": float64(30),
73+
"after": (*string)(nil),
7374
}
7475

7576
varsRepoNotFound := map[string]interface{}{
7677
"owner": "owner",
7778
"repo": "nonexistent-repo",
7879
"first": float64(30),
80+
"after": (*string)(nil),
7981
}
8082

8183
varsDiscussionsFiltered := map[string]interface{}{
8284
"owner": "owner",
8385
"repo": "repo",
8486
"categoryId": "DIC_kwDOABC123",
8587
"first": float64(30),
88+
"after": (*string)(nil),
8689
}
8790

8891
tests := []struct {
@@ -155,15 +158,21 @@ func Test_ListDiscussions(t *testing.T) {
155158
require.NoError(t, err)
156159

157160
// Parse the structured response with pagination info
158-
var returnedDiscussions []*github.Discussion
159-
err = json.Unmarshal([]byte(text), &returnedDiscussions)
161+
var response struct {
162+
Discussions []*github.Discussion `json:"discussions"`
163+
PageInfo struct {
164+
HasNextPage bool `json:"hasNextPage"`
165+
EndCursor string `json:"endCursor"`
166+
} `json:"pageInfo"`
167+
}
168+
err = json.Unmarshal([]byte(text), &response)
160169
require.NoError(t, err)
161170

162-
assert.Len(t, returnedDiscussions, tc.expectedCount, "Expected %d discussions, got %d", tc.expectedCount, len(returnedDiscussions))
171+
assert.Len(t, response.Discussions, tc.expectedCount, "Expected %d discussions, got %d", tc.expectedCount, len(response.Discussions))
163172

164173
// Verify that all returned discussions have a category if filtered
165174
if _, hasCategory := tc.reqParams["category"]; hasCategory {
166-
for _, discussion := range returnedDiscussions {
175+
for _, discussion := range response.Discussions {
167176
require.NotNil(t, discussion.DiscussionCategory, "Discussion should have category")
168177
assert.NotEmpty(t, *discussion.DiscussionCategory.Name, "Discussion should have category name")
169178
}
@@ -266,14 +275,15 @@ func Test_GetDiscussionComments(t *testing.T) {
266275
assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussionNumber"})
267276

268277
// Use exact string query that matches implementation output
269-
qGetComments := "query($discussionNumber:Int!$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){comments(first: $first){nodes{body},pageInfo{hasNextPage,endCursor}}}}}"
278+
qGetComments := "query($after:String$discussionNumber:Int!$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){comments(first: $first, after: $after){nodes{body},pageInfo{hasNextPage,endCursor}}}}}"
270279

271280
// Variables matching what GraphQL receives after JSON marshaling/unmarshaling
272281
vars := map[string]interface{}{
273282
"owner": "owner",
274283
"repo": "repo",
275284
"discussionNumber": float64(1),
276285
"first": float64(100),
286+
"after": (*string)(nil),
277287
}
278288

279289
mockResponse := githubv4mock.DataResponse(map[string]any{
@@ -311,25 +321,32 @@ func Test_GetDiscussionComments(t *testing.T) {
311321
// Debug: print the actual JSON response
312322
t.Logf("JSON response: %s", textContent.Text)
313323

314-
var comments []*github.IssueComment
315-
err = json.Unmarshal([]byte(textContent.Text), &comments)
324+
var response struct {
325+
Comments []*github.IssueComment `json:"comments"`
326+
PageInfo struct {
327+
HasNextPage bool `json:"hasNextPage"`
328+
EndCursor string `json:"endCursor"`
329+
} `json:"pageInfo"`
330+
}
331+
err = json.Unmarshal([]byte(textContent.Text), &response)
316332
require.NoError(t, err)
317-
assert.Len(t, comments, 2)
333+
assert.Len(t, response.Comments, 2)
318334
expectedBodies := []string{"This is the first comment", "This is the second comment"}
319-
for i, comment := range comments {
335+
for i, comment := range response.Comments {
320336
assert.Equal(t, expectedBodies[i], *comment.Body)
321337
}
322338
}
323339

324340
func Test_ListDiscussionCategories(t *testing.T) {
325341
// Use exact string query that matches implementation output
326-
qListCategories := "query($first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussionCategories(first: $first){nodes{id,name},pageInfo{hasNextPage,endCursor}}}}"
342+
qListCategories := "query($after:String$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussionCategories(first: $first, after: $after){nodes{id,name},pageInfo{hasNextPage,endCursor}}}}"
327343

328344
// Variables matching what GraphQL receives after JSON marshaling/unmarshaling
329345
vars := map[string]interface{}{
330346
"owner": "owner",
331347
"repo": "repo",
332348
"first": float64(100),
349+
"after": (*string)(nil),
333350
}
334351

335352
mockResp := githubv4mock.DataResponse(map[string]any{
@@ -366,11 +383,17 @@ func Test_ListDiscussionCategories(t *testing.T) {
366383
// Debug: print the actual JSON response
367384
t.Logf("JSON response: %s", text)
368385

369-
var categories []map[string]string
370-
require.NoError(t, json.Unmarshal([]byte(text), &categories))
371-
assert.Len(t, categories, 2)
372-
assert.Equal(t, "123", categories[0]["id"])
373-
assert.Equal(t, "CategoryOne", categories[0]["name"])
374-
assert.Equal(t, "456", categories[1]["id"])
375-
assert.Equal(t, "CategoryTwo", categories[1]["name"])
386+
var response struct {
387+
Categories []map[string]string `json:"categories"`
388+
PageInfo struct {
389+
HasNextPage bool `json:"hasNextPage"`
390+
EndCursor string `json:"endCursor"`
391+
} `json:"pageInfo"`
392+
}
393+
require.NoError(t, json.Unmarshal([]byte(text), &response))
394+
assert.Len(t, response.Categories, 2)
395+
assert.Equal(t, "123", response.Categories[0]["id"])
396+
assert.Equal(t, "CategoryOne", response.Categories[0]["name"])
397+
assert.Equal(t, "456", response.Categories[1]["id"])
398+
assert.Equal(t, "CategoryTwo", response.Categories[1]["name"])
376399
}

‎pkg/github/server.go‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,15 +205,20 @@ func WithUnifiedPagination() mcp.ToolOption {
205205
mcp.Min(1),
206206
mcp.Max(100),
207207
)(tool)
208+
209+
mcp.WithString("after",
210+
mcp.Description("Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs."),
211+
)(tool)
208212
}
209213
}
210214

211215
type PaginationParams struct {
212216
Page int
213217
PerPage int
218+
After string
214219
}
215220

216-
// OptionalPaginationParams returns the "page" and "perPage" parameters from the request,
221+
// OptionalPaginationParams returns the "page", "perPage", and "after" parameters from the request,
217222
// or their default values if not present, "page" default is 1, "perPage" default is 30.
218223
// In future, we may want to make the default values configurable, or even have this
219224
// function returned from `withPagination`, where the defaults are provided alongside
@@ -227,18 +232,25 @@ func OptionalPaginationParams(r mcp.CallToolRequest) (PaginationParams, error) {
227232
if err != nil {
228233
return PaginationParams{}, err
229234
}
235+
after, err := OptionalParam[string](r, "after")
236+
if err != nil {
237+
return PaginationParams{}, err
238+
}
230239
return PaginationParams{
231240
Page: page,
232241
PerPage: perPage,
242+
After: after,
233243
}, nil
234244
}
235245

236246
type GraphQLPaginationParams struct {
237247
First *int32
248+
After *string
238249
}
239250

240251
// ToGraphQLParams converts REST API pagination parameters to GraphQL-specific parameters.
241252
// This converts page/perPage to first parameter for GraphQL queries.
253+
// If After is provided, it takes precedence over page-based pagination.
242254
func (p PaginationParams) ToGraphQLParams() (GraphQLPaginationParams, error) {
243255
if p.PerPage > 100 {
244256
return GraphQLPaginationParams{}, fmt.Errorf("perPage value %d exceeds maximum of 100", p.PerPage)
@@ -247,8 +259,15 @@ func (p PaginationParams) ToGraphQLParams() (GraphQLPaginationParams, error) {
247259
return GraphQLPaginationParams{}, fmt.Errorf("perPage value %d cannot be negative", p.PerPage)
248260
}
249261
first := int32(p.PerPage)
262+
263+
var after *string
264+
if p.After != "" {
265+
after = &p.After
266+
}
267+
250268
return GraphQLPaginationParams{
251269
First: &first,
270+
After: after,
252271
}, nil
253272
}
254273

0 commit comments

Comments
 (0)