Skip to content

Commit 6f5934a

Browse files
fatmcgavclaude
andcommitted
fix(web): paginate all PR comments when searching for existing summary
Replaces the single-page octokit.rest.issues.listComments call with octokit.paginate(...) so all comment pages are searched before deciding whether to create or update the summary comment. Without this, the marker could be missed on PRs with many comments, causing duplicate summary comments on re-runs. Tests updated to mock octokit.paginate directly instead of rest.issues.listComments, and assertions updated accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 9f9e1d7 commit 6f5934a

2 files changed

Lines changed: 13 additions & 12 deletions

File tree

packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,22 @@ const SINGLE_REVIEW: sourcebot_file_diff_review[] = [
3131
];
3232

3333
function makeMockOctokit(createReviewCommentResult: 'resolve' | 'reject' = 'resolve', existingComments: { id: number; body: string }[] = []) {
34-
return {
34+
const octokit = {
35+
paginate: vi.fn().mockResolvedValue(existingComments),
3536
rest: {
3637
pulls: {
3738
createReviewComment: createReviewCommentResult === 'resolve'
3839
? vi.fn().mockResolvedValue({})
3940
: vi.fn().mockRejectedValue(new Error('Unprocessable Entity')),
4041
},
4142
issues: {
42-
listComments: vi.fn().mockResolvedValue({ data: existingComments }),
43+
listComments: vi.fn(),
4344
createComment: vi.fn().mockResolvedValue({}),
4445
updateComment: vi.fn().mockResolvedValue({}),
4546
},
4647
},
47-
} as unknown as Octokit;
48+
} as any;
49+
return octokit;
4850
}
4951

5052
describe('githubPushPrReviews', () => {
@@ -161,7 +163,7 @@ describe('githubPushPrReviews – summary comment', () => {
161163

162164
await githubPushPrReviews(octokit, MOCK_PAYLOAD, [], undefined);
163165

164-
expect(octokit.rest.issues.listComments).not.toHaveBeenCalled();
166+
expect(octokit.paginate).not.toHaveBeenCalled();
165167
expect(octokit.rest.issues.createComment).not.toHaveBeenCalled();
166168
expect(octokit.rest.issues.updateComment).not.toHaveBeenCalled();
167169
});
@@ -171,11 +173,10 @@ describe('githubPushPrReviews – summary comment', () => {
171173

172174
await githubPushPrReviews(octokit, MOCK_PAYLOAD, [], 'Summary text');
173175

174-
expect(octokit.rest.issues.listComments).toHaveBeenCalledWith({
175-
owner: 'my-org',
176-
repo: 'my-repo',
177-
issue_number: 7,
178-
});
176+
expect(octokit.paginate).toHaveBeenCalledWith(
177+
octokit.rest.issues.listComments,
178+
{ owner: 'my-org', repo: 'my-repo', issue_number: 7 },
179+
);
179180
expect(octokit.rest.issues.createComment).toHaveBeenCalledOnce();
180181
const body = octokit.rest.issues.createComment.mock.calls[0][0].body as string;
181182
expect(body).toContain(SUMMARY_MARKER);
@@ -202,9 +203,9 @@ describe('githubPushPrReviews – summary comment', () => {
202203
expect(octokit.rest.issues.createComment).not.toHaveBeenCalled();
203204
});
204205

205-
test('does not throw when listComments fails', async () => {
206+
test('does not throw when paginate fails', async () => {
206207
const octokit = makeMockOctokit();
207-
octokit.rest.issues.listComments = vi.fn().mockRejectedValue(new Error('403 Forbidden'));
208+
octokit.paginate = vi.fn().mockRejectedValue(new Error('403 Forbidden'));
208209

209210
await expect(
210211
githubPushPrReviews(octokit, MOCK_PAYLOAD, [], 'Summary text'),

packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export const githubPushPrReviews = async (octokit: Octokit, pr_payload: sourcebo
1010
if (summary) {
1111
const SUMMARY_MARKER = "<!-- sourcebot-review-summary -->";
1212
try {
13-
const { data: comments } = await octokit.rest.issues.listComments({
13+
const comments = await octokit.paginate(octokit.rest.issues.listComments, {
1414
owner: pr_payload.owner,
1515
repo: pr_payload.repo,
1616
issue_number: pr_payload.number,

0 commit comments

Comments
 (0)