Skip to content

Commit 491a355

Browse files
committed
fix(github): constrain issue pagination size
1 parent 3d9f527 commit 491a355

3 files changed

Lines changed: 44 additions & 18 deletions

File tree

src/providers/github/actions.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,7 @@ export const githubActions: ActionDefinition[] = [
758758
action({
759759
name: "list_repository_issues",
760760
description:
761-
"List issues for a GitHub repository. Pull requests are filtered out of the response; pageInfo.fetched reports the raw page length before filtering, so paginating callers must continue while fetched equals the requested page size even when the issues array comes back short or empty.",
761+
"List issues for a GitHub repository. Pull requests are filtered out of the response; pageInfo.fetched reports the raw page length before filtering, so paginating callers must continue while fetched equals perPage (30 by default) even when the issues array comes back short or empty.",
762762
requiredScopes: githubRepoScopes,
763763
inputSchema: s.object({
764764
owner: nonEmptyString,
@@ -768,7 +768,13 @@ export const githubActions: ActionDefinition[] = [
768768
sort: s.stringEnum(["created", "updated", "comments"]),
769769
direction: s.stringEnum(["asc", "desc"]),
770770
since: s.string(),
771-
...optionalPaginationFields,
771+
perPage: s.integer({
772+
minimum: 1,
773+
maximum: 100,
774+
default: 30,
775+
description: "Number of results requested per page. Defaults to 30.",
776+
}),
777+
page: optionalPaginationFields.page,
772778
}),
773779
outputSchema: s.object({
774780
issues: s.array(githubIssueSchema),
@@ -778,7 +784,7 @@ export const githubActions: ActionDefinition[] = [
778784
fetched: s.integer({
779785
minimum: 0,
780786
description:
781-
"Number of items GitHub returned on this page before filtering. Continue paginating while this equals the requested page size.",
787+
"Number of items GitHub returned on this page before filtering. Continue paginating while this equals perPage, which defaults to 30.",
782788
}),
783789
},
784790
),

src/providers/github/runtime-issue.test.ts

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
1+
import type { JsonSchema } from "../../core/types.ts";
2+
13
import { describe, expect, it } from "vitest";
24
import { provider } from "./definition.ts";
35
import { issueActionHandlers } from "./runtime-issue.ts";
46

5-
function pageFetcher(items: unknown[]): typeof fetch {
6-
return async () =>
7-
new Response(JSON.stringify(items), {
7+
function pageFetcher(items: unknown[], onRequest?: (url: string) => void): typeof fetch {
8+
return async (url) => {
9+
onRequest?.(String(url));
10+
return new Response(JSON.stringify(items), {
811
status: 200,
912
headers: { "content-type": "application/json" },
1013
});
14+
};
1115
}
1216

1317
describe("list_repository_issues pagination signal", () => {
@@ -48,16 +52,31 @@ describe("list_repository_issues pagination signal", () => {
4852
expect(result.pageInfo.fetched).toBe(2);
4953
});
5054

51-
it("declares pageInfo.fetched in the action's output schema", () => {
52-
interface ObjectSchema {
53-
properties?: Record<string, ObjectSchema>;
54-
required?: string[];
55-
type?: string;
56-
}
55+
it("requests the documented default page size when perPage is omitted", async () => {
56+
let requestedUrl = "";
57+
await issueActionHandlers.list_repository_issues(
58+
{ owner: "acme", repo: "widgets" },
59+
{
60+
accessToken: "token",
61+
fetcher: pageFetcher([], (url) => {
62+
requestedUrl = url;
63+
}),
64+
},
65+
);
66+
67+
expect(new URL(requestedUrl).searchParams.get("per_page")).toBe("30");
68+
});
69+
70+
it("declares the pagination contract in the action schemas", () => {
5771
const action = provider.actions.find((entry) => entry.name === "list_repository_issues");
58-
const pageInfo = (action?.outputSchema as ObjectSchema | undefined)?.properties?.pageInfo;
72+
const inputProperties = action?.inputSchema.properties as Record<string, JsonSchema> | undefined;
73+
const outputProperties = action?.outputSchema.properties as Record<string, JsonSchema> | undefined;
74+
const perPage = inputProperties?.perPage;
75+
const pageInfo = outputProperties?.pageInfo;
76+
const pageInfoProperties = pageInfo?.properties as Record<string, JsonSchema> | undefined;
5977

60-
expect(pageInfo?.properties?.fetched?.type).toBe("integer");
61-
expect(pageInfo?.required).toContain("fetched");
78+
expect(perPage).toMatchObject({ type: "integer", minimum: 1, maximum: 100, default: 30 });
79+
expect(pageInfoProperties?.fetched?.type).toBe("integer");
80+
expect(pageInfo?.required as string[] | undefined).toContain("fetched");
6281
});
6382
});

src/providers/github/runtime-issue.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ export const issueActionHandlers: Record<string, GitHubActionHandler> = {
273273
};
274274

275275
async function listRepositoryIssues(input: Record<string, unknown>, accessToken: string, fetcher: typeof fetch) {
276+
const perPage = optionalInteger(input.perPage) ?? 30;
276277
const issues = await githubRequestJson<Record<string, unknown>[]>({
277278
path: `/repos/${encodeURIComponent(String(input.owner))}/${encodeURIComponent(String(input.repo))}/issues`,
278279
query: compactObject({
@@ -281,7 +282,7 @@ async function listRepositoryIssues(input: Record<string, unknown>, accessToken:
281282
sort: optionalString(input.sort),
282283
direction: optionalString(input.direction),
283284
since: optionalString(input.since),
284-
per_page: optionalInteger(input.perPage),
285+
per_page: perPage,
285286
page: optionalInteger(input.page),
286287
}),
287288
accessToken,
@@ -292,8 +293,8 @@ async function listRepositoryIssues(input: Record<string, unknown>, accessToken:
292293
// The raw GitHub page mixes issues and pull requests; filtering PRs out
293294
// destroys the only pagination signal page-number callers have (the raw
294295
// page length). `pageInfo.fetched` preserves it: a caller must continue
295-
// paginating while `fetched` equals the requested page size, even when
296-
// `issues` comes back short or empty.
296+
// paginating while `fetched` equals `perPage`, even when `issues` comes
297+
// back short or empty.
297298
issues: issues.filter((issue) => issue.pull_request == null),
298299
pageInfo: { fetched: issues.length },
299300
};

0 commit comments

Comments
 (0)