Skip to content

Commit 903e58c

Browse files
fix(review): paginate check-runs in findPreviewUrlFromChecks
Reuse findAcrossPages the same way getPreviewBuildState already does, so a preview URL on page 2+ of check-runs is still discovered. Closes #7779 Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent fa21f3e commit 903e58c

2 files changed

Lines changed: 86 additions & 8 deletions

File tree

src/review/visual/preview-url.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -243,15 +243,27 @@ export async function findPreviewUrlFromChecks(params: {
243243
const url = extractPreviewUrl(status.target_url);
244244
if (url) return url;
245245
}
246-
const checks = await githubJson<{ check_runs?: Array<{ status?: string; conclusion?: string; details_url?: string; output?: { summary?: string; text?: string } }> }>(
246+
// Paginate check-runs the same way getPreviewBuildState does (#7779): a commit with >100 check-runs
247+
// can push the Cloudflare Workers Builds check onto page 2+, and a single per_page=100 read would then
248+
// miss the preview URL even though getPreviewBuildState (which already paginates) still sees the build.
249+
return await findAcrossPages<
250+
{ status?: string; conclusion?: string; details_url?: string; output?: { summary?: string; text?: string } },
251+
string
252+
>(
247253
`${base}/commits/${encodeURIComponent(params.sha)}/check-runs?per_page=100`,
248254
opts,
249-
).catch(() => null);
250-
for (const run of checks?.check_runs ?? []) {
251-
if (run.status === "completed" && run.conclusion && run.conclusion !== "success") continue;
252-
const url = extractPreviewUrl(run.details_url) ?? extractPreviewUrl(run.output?.summary) ?? extractPreviewUrl(run.output?.text);
253-
if (url) return url;
254-
}
255+
(payload) =>
256+
(payload as { check_runs?: Array<{ status?: string; conclusion?: string; details_url?: string; output?: { summary?: string; text?: string } }> })
257+
?.check_runs ?? [],
258+
(runs) => {
259+
for (const run of runs) {
260+
if (run.status === "completed" && run.conclusion && run.conclusion !== "success") continue;
261+
const url = extractPreviewUrl(run.details_url) ?? extractPreviewUrl(run.output?.summary) ?? extractPreviewUrl(run.output?.text);
262+
if (url) return url;
263+
}
264+
return null;
265+
},
266+
);
255267
} catch (error) {
256268
console.log(JSON.stringify({ event: "preview_from_checks_error", repo: `${params.repo.owner}/${params.repo.repo}`, message: String(error).slice(0, 200) }));
257269
}

test/unit/preview-url.test.ts

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import { afterEach, describe, expect, it, vi } from "vitest";
22
import { clearGitHubResponseCacheForTest, githubRateLimitAdmissionKeyForInstallation, latestGitHubRestRateLimitObservation } from "../../src/github/client";
3-
import { extractPreviewUrl, findPreviewUrlFromPrComments, getLatestDeploymentStatus, getPreviewBuildState } from "../../src/review/visual/preview-url";
3+
import {
4+
extractPreviewUrl,
5+
findPreviewUrlFromChecks,
6+
findPreviewUrlFromPrComments,
7+
getLatestDeploymentStatus,
8+
getPreviewBuildState,
9+
} from "../../src/review/visual/preview-url";
410

511
/** GitHub's `Link` header for a page that advertises a next page (the exact shape findAcrossPages walks). */
612
const NEXT_LINK = '<https://api.github.com/resource?per_page=100&page=99>; rel="next", <https://api.github.com/resource?per_page=100&page=99>; rel="last"';
@@ -140,6 +146,66 @@ describe("preview-url pagination (#7450)", () => {
140146
expect(fetchMock).toHaveBeenCalledTimes(2);
141147
});
142148

149+
it("findPreviewUrlFromChecks follows Link: rel=next and finds the preview URL on page 2 (#7779)", async () => {
150+
const page1 = {
151+
check_runs: Array.from({ length: 100 }, (_v, i) => ({
152+
name: `ci-${i}`,
153+
status: "completed",
154+
conclusion: "success",
155+
details_url: "https://example.com/ci",
156+
})),
157+
};
158+
const page2 = {
159+
check_runs: [
160+
{
161+
name: "Cloudflare Workers Builds",
162+
status: "completed",
163+
conclusion: "success",
164+
details_url: "https://pr-42.app.workers.dev/path",
165+
},
166+
],
167+
};
168+
const fetchMock = vi.fn(async (input: RequestInfo | URL) => {
169+
const url = String(input);
170+
if (url.includes("/status") && !url.includes("check-runs")) return Response.json({ statuses: [] });
171+
if (url.includes("check-runs")) {
172+
return isPage2(input) ? Response.json(page2) : Response.json(page1, { headers: { link: NEXT_LINK } });
173+
}
174+
return Response.json({});
175+
});
176+
vi.stubGlobal("fetch", fetchMock);
177+
178+
await expect(findPreviewUrlFromChecks({ token: "t", repo: REPO, sha: "deadbeef" })).resolves.toBe("https://pr-42.app.workers.dev");
179+
const checkRunCalls = fetchMock.mock.calls.map((call) => String(call[0])).filter((url) => url.includes("check-runs"));
180+
expect(checkRunCalls).toHaveLength(2);
181+
expect(checkRunCalls[0]).toContain("/commits/deadbeef/check-runs?per_page=100");
182+
expect(checkRunCalls[0]).not.toContain("&page=");
183+
expect(checkRunCalls[1]).toContain("&page=2");
184+
});
185+
186+
it("findPreviewUrlFromChecks stops as soon as a preview URL is found on page 1 (#7779)", async () => {
187+
const fetchMock = vi.fn(async (input: RequestInfo | URL) => {
188+
const url = String(input);
189+
if (url.includes("/status") && !url.includes("check-runs")) return Response.json({ statuses: [] });
190+
return Response.json(
191+
{
192+
check_runs: [
193+
{
194+
name: "Cloudflare Workers Builds",
195+
status: "completed",
196+
conclusion: "success",
197+
details_url: "https://pr-1.app.workers.dev",
198+
},
199+
],
200+
},
201+
{ headers: { link: NEXT_LINK } },
202+
);
203+
});
204+
vi.stubGlobal("fetch", fetchMock);
205+
await expect(findPreviewUrlFromChecks({ token: "t", repo: REPO, sha: "abc" })).resolves.toBe("https://pr-1.app.workers.dev");
206+
expect(fetchMock.mock.calls.map((call) => String(call[0])).filter((url) => url.includes("check-runs"))).toHaveLength(1);
207+
});
208+
143209
it("getPreviewBuildState classifies a completed Workers Builds check as succeeded or failed", async () => {
144210
vi.stubGlobal("fetch", async () => Response.json({ check_runs: [{ name: "cloudflare pages", status: "completed", conclusion: "success" }] }));
145211
await expect(getPreviewBuildState({ token: "t", repo: REPO, sha: "s1" })).resolves.toBe("succeeded");

0 commit comments

Comments
 (0)