Skip to content

Commit 918a42f

Browse files
authored
feat: implement cursor pagination for dependabot alerts (#2651)
1 parent 8bbd902 commit 918a42f

12 files changed

Lines changed: 86 additions & 53 deletions

README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -717,8 +717,8 @@ The following sets of tools are available:
717717
- **list_dependabot_alerts** - List dependabot alerts
718718
- **Required OAuth Scopes**: `security_events`
719719
- **Accepted OAuth Scopes**: `repo`, `security_events`
720+
- `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional)
720721
- `owner`: The owner of the repository. (string, required)
721-
- `page`: Page number for pagination (min 1) (number, optional)
722722
- `perPage`: Results per page for pagination (min 1, max 100) (number, optional)
723723
- `repo`: The name of the repository. (string, required)
724724
- `severity`: Filter dependabot alerts by severity (string, optional)
@@ -755,7 +755,7 @@ The following sets of tools are available:
755755

756756
- **get_discussion_comments** - Get discussion comments
757757
- **Required OAuth Scopes**: `repo`
758-
- `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional)
758+
- `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional)
759759
- `discussionNumber`: Discussion Number (number, required)
760760
- `includeReplies`: When true, each top-level comment will include its replies nested within it (up to 100 replies per comment, which is the GitHub API maximum). Defaults to false. (boolean, optional)
761761
- `owner`: Repository owner (string, required)
@@ -769,7 +769,7 @@ The following sets of tools are available:
769769

770770
- **list_discussions** - List discussions
771771
- **Required OAuth Scopes**: `repo`
772-
- `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional)
772+
- `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional)
773773
- `category`: Optional filter by discussion category ID. If provided, only discussions with this category are listed. (string, optional)
774774
- `direction`: Order direction. (string, optional)
775775
- `orderBy`: Order discussions by field. If provided, the 'direction' also needs to be provided. (string, optional)
@@ -881,7 +881,7 @@ The following sets of tools are available:
881881

882882
- **list_issues** - List issues
883883
- **Required OAuth Scopes**: `repo`
884-
- `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional)
884+
- `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional)
885885
- `direction`: Order direction. If provided, the 'orderBy' also needs to be provided. (string, optional)
886886
- `labels`: Filter by labels (string[], optional)
887887
- `orderBy`: Order issues by field. If provided, the 'direction' also needs to be provided. (string, optional)

docs/feature-flags.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ runtime behavior (such as output formatting) won't appear here.
102102

103103
- **list_issues** - List issues
104104
- **Required OAuth Scopes**: `repo`
105-
- `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional)
105+
- `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional)
106106
- `direction`: Order direction. If provided, the 'orderBy' also needs to be provided. (string, optional)
107107
- `field_filters`: Filter by custom issue field values. Each entry takes a field_name and a value; the server looks up the field and coerces the value to its type (single-select option name, text, number, or YYYY-MM-DD date). (object[], optional)
108108
- `labels`: Filter by labels (string[], optional)

docs/insiders-features.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ The list below is generated from the Go source. It covers tool **inventory and s
9696

9797
- **list_issues** - List issues
9898
- **Required OAuth Scopes**: `repo`
99-
- `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional)
99+
- `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional)
100100
- `direction`: Order direction. If provided, the 'orderBy' also needs to be provided. (string, optional)
101101
- `field_filters`: Filter by custom issue field values. Each entry takes a field_name and a value; the server looks up the field and coerces the value to its type (single-select option name, text, number, or YYYY-MM-DD date). (object[], optional)
102102
- `labels`: Filter by labels (string[], optional)

pkg/github/__toolsnaps__/get_discussion_comments.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"inputSchema": {
88
"properties": {
99
"after": {
10-
"description": "Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs.",
10+
"description": "Cursor for pagination. Use the cursor from the previous response.",
1111
"type": "string"
1212
},
1313
"discussionNumber": {

pkg/github/__toolsnaps__/list_dependabot_alerts.snap

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,14 @@
66
"description": "List dependabot alerts in a GitHub repository.",
77
"inputSchema": {
88
"properties": {
9+
"after": {
10+
"description": "Cursor for pagination. Use the cursor from the previous response.",
11+
"type": "string"
12+
},
913
"owner": {
1014
"description": "The owner of the repository.",
1115
"type": "string"
1216
},
13-
"page": {
14-
"description": "Page number for pagination (min 1)",
15-
"minimum": 1,
16-
"type": "number"
17-
},
1817
"perPage": {
1918
"description": "Results per page for pagination (min 1, max 100)",
2019
"maximum": 100,

pkg/github/__toolsnaps__/list_discussions.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"inputSchema": {
88
"properties": {
99
"after": {
10-
"description": "Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs.",
10+
"description": "Cursor for pagination. Use the cursor from the previous response.",
1111
"type": "string"
1212
},
1313
"category": {

pkg/github/__toolsnaps__/list_issues.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"inputSchema": {
88
"properties": {
99
"after": {
10-
"description": "Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs.",
10+
"description": "Cursor for pagination. Use the cursor from the previous response.",
1111
"type": "string"
1212
},
1313
"direction": {

pkg/github/__toolsnaps__/list_issues_ff_remote_mcp_issue_fields.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"inputSchema": {
88
"properties": {
99
"after": {
10-
"description": "Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs.",
10+
"description": "Cursor for pagination. Use the cursor from the previous response.",
1111
"type": "string"
1212
},
1313
"direction": {

pkg/github/dependabot.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
120120
},
121121
Required: []string{"owner", "repo"},
122122
}
123-
WithPagination(schema)
123+
WithCursorPagination(schema)
124124

125125
return NewTool(
126126
ToolsetMetadataDependabot,
@@ -152,7 +152,7 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
152152
return utils.NewToolResultError(err.Error()), nil, nil
153153
}
154154

155-
pagination, err := OptionalPaginationParams(args)
155+
pagination, err := OptionalCursorPaginationParams(args)
156156
if err != nil {
157157
return utils.NewToolResultError(err.Error()), nil, nil
158158
}
@@ -165,9 +165,9 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
165165
alerts, resp, err := client.Dependabot.ListRepoAlerts(ctx, owner, repo, &github.ListAlertsOptions{
166166
State: ToStringPtr(state),
167167
Severity: ToStringPtr(severity),
168-
ListOptions: github.ListOptions{
169-
Page: pagination.Page,
168+
ListCursorOptions: github.ListCursorOptions{
170169
PerPage: pagination.PerPage,
170+
After: pagination.After,
171171
},
172172
})
173173
if err != nil {
@@ -187,7 +187,12 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
187187
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list alerts", resp, body), nil, nil
188188
}
189189

190-
r, err := json.Marshal(alerts)
190+
response := map[string]any{
191+
"alerts": alerts,
192+
"pageInfo": buildPageInfo(resp),
193+
}
194+
195+
r, err := json.Marshal(response)
191196
if err != nil {
192197
return utils.NewToolResultErrorFromErr("failed to marshal alerts", err), nil, err
193198
}

pkg/github/dependabot_test.go

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -154,19 +154,19 @@ func Test_ListDependabotAlerts(t *testing.T) {
154154
}
155155

156156
tests := []struct {
157-
name string
158-
mockedClient *http.Client
159-
requestArgs map[string]any
160-
expectError bool
161-
expectedAlerts []*github.DependabotAlert
162-
expectedErrMsg string
157+
name string
158+
mockedClient *http.Client
159+
requestArgs map[string]any
160+
expectError bool
161+
expectedAlerts []*github.DependabotAlert
162+
expectedNextCursor string
163+
expectedErrMsg string
163164
}{
164165
{
165166
name: "successful open alerts listing",
166167
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
167168
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{
168169
"state": "open",
169-
"page": "1",
170170
"per_page": "30",
171171
}).andThen(
172172
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&criticalAlert}),
@@ -185,7 +185,6 @@ func Test_ListDependabotAlerts(t *testing.T) {
185185
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
186186
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{
187187
"severity": "high",
188-
"page": "1",
189188
"per_page": "30",
190189
}).andThen(
191190
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&highSeverityAlert}),
@@ -203,7 +202,6 @@ func Test_ListDependabotAlerts(t *testing.T) {
203202
name: "successful all alerts listing",
204203
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
205204
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{
206-
"page": "1",
207205
"per_page": "30",
208206
}).andThen(
209207
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&criticalAlert, &highSeverityAlert}),
@@ -217,10 +215,10 @@ func Test_ListDependabotAlerts(t *testing.T) {
217215
expectedAlerts: []*github.DependabotAlert{&criticalAlert, &highSeverityAlert},
218216
},
219217
{
220-
name: "successful alerts listing with custom pagination",
218+
name: "successful alerts listing with cursor pagination",
221219
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
222220
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{
223-
"page": "3",
221+
"after": "Y3Vyc29yOnYyOpK5",
224222
"per_page": "100",
225223
}).andThen(
226224
mockResponse(t, http.StatusOK, []*github.DependabotAlert{&criticalAlert}),
@@ -229,12 +227,35 @@ func Test_ListDependabotAlerts(t *testing.T) {
229227
requestArgs: map[string]any{
230228
"owner": "owner",
231229
"repo": "repo",
232-
"page": float64(3),
230+
"after": "Y3Vyc29yOnYyOpK5",
233231
"perPage": float64(100),
234232
},
235233
expectError: false,
236234
expectedAlerts: []*github.DependabotAlert{&criticalAlert},
237235
},
236+
{
237+
name: "successful alerts listing surfaces next page cursor",
238+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
239+
GetReposDependabotAlertsByOwnerByRepo: expectQueryParams(t, map[string]string{
240+
"per_page": "30",
241+
}).andThen(
242+
func(w http.ResponseWriter, _ *http.Request) {
243+
w.Header().Set("Link", `<https://api.github.com/repos/owner/repo/dependabot/alerts?after=nextcursor123&per_page=30>; rel="next"`)
244+
w.WriteHeader(http.StatusOK)
245+
b, err := json.Marshal([]*github.DependabotAlert{&criticalAlert})
246+
require.NoError(t, err)
247+
_, _ = w.Write(b)
248+
},
249+
),
250+
}),
251+
requestArgs: map[string]any{
252+
"owner": "owner",
253+
"repo": "repo",
254+
},
255+
expectError: false,
256+
expectedAlerts: []*github.DependabotAlert{&criticalAlert},
257+
expectedNextCursor: "nextcursor123",
258+
},
238259
{
239260
name: "alerts listing fails",
240261
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -291,11 +312,17 @@ func Test_ListDependabotAlerts(t *testing.T) {
291312
textContent := getTextResult(t, result)
292313

293314
// Unmarshal and verify the result
294-
var returnedAlerts []*github.DependabotAlert
295-
err = json.Unmarshal([]byte(textContent.Text), &returnedAlerts)
315+
var returnedResult struct {
316+
Alerts []*github.DependabotAlert `json:"alerts"`
317+
PageInfo struct {
318+
HasNextPage bool `json:"hasNextPage"`
319+
NextCursor string `json:"nextCursor"`
320+
} `json:"pageInfo"`
321+
}
322+
err = json.Unmarshal([]byte(textContent.Text), &returnedResult)
296323
assert.NoError(t, err)
297-
assert.Len(t, returnedAlerts, len(tc.expectedAlerts))
298-
for i, alert := range returnedAlerts {
324+
assert.Len(t, returnedResult.Alerts, len(tc.expectedAlerts))
325+
for i, alert := range returnedResult.Alerts {
299326
assert.Equal(t, *tc.expectedAlerts[i].Number, *alert.Number)
300327
assert.Equal(t, *tc.expectedAlerts[i].HTMLURL, *alert.HTMLURL)
301328
assert.Equal(t, *tc.expectedAlerts[i].State, *alert.State)
@@ -304,6 +331,8 @@ func Test_ListDependabotAlerts(t *testing.T) {
304331
assert.Equal(t, *tc.expectedAlerts[i].SecurityAdvisory.Severity, *alert.SecurityAdvisory.Severity)
305332
}
306333
}
334+
assert.Equal(t, tc.expectedNextCursor, returnedResult.PageInfo.NextCursor)
335+
assert.Equal(t, tc.expectedNextCursor != "", returnedResult.PageInfo.HasNextPage)
307336
})
308337
}
309338
}

0 commit comments

Comments
 (0)