diff --git a/src/review/visual/capture.ts b/src/review/visual/capture.ts index 5e75c9d96..50bd42cce 100644 --- a/src/review/visual/capture.ts +++ b/src/review/visual/capture.ts @@ -707,6 +707,9 @@ export async function buildCapture( let previewFailed = target.previewFailed === true; let previewUnobtainable = false; let previewPending = false; + // Whether the deployments read below threw. The structurally-unobtainable flag's contract (#9881) is that + // the deployments read SUCCEEDED and found none — so a call where it threw must never set the flag (#10059). + let deploymentsReadFailed = false; // Hoisted above the discovery block below (was previously computed after it) so the eternal-"loading"- // placeholder fix's `buildState === "absent"` branch can consult it -- seeing this whole file top to // bottom, its own later use (guarding the actions_fallback dispatch) is unchanged. @@ -721,8 +724,13 @@ export async function buildCapture( const status = await getLatestDeploymentStatus({ token, repo, sha: target.headSha, ref: target.headRef, apiVersion, rateLimitAdmissionKey }); previewBase = status.url ?? ""; previewFailed = status.failed; + // getLatestDeploymentStatus reports a read failure (403/rate-limit/5xx) via `error: true` rather than + // throwing — that call did NOT prove there is no deployment, so it must suppress previewUnobtainable + // exactly as a thrown read does (#10059). + if (status.error === true) deploymentsReadFailed = true; } catch { previewBase = ""; + deploymentsReadFailed = true; } if (!previewBase && !previewFailed && target.previewFromChecks && target.headSha) { previewBase = (await findPreviewUrlFromChecks({ token, repo, sha: target.headSha, apiVersion, rateLimitAdmissionKey })) ?? ""; @@ -745,7 +753,7 @@ export async function buildCapture( await recordPreviewPollAttempt(env, target.headSha); previewPending = true; } - } else if (buildState === "absent" && !actionsFallbackEnabled) { + } else if ((buildState === "absent" || buildState === "unreadable") && !actionsFallbackEnabled) { // Eternal-"loading"-placeholder fix: 'absent' means no Workers-Builds-named check-run was found // AT ALL, not "still building" -- previously this fell through as a silent no-op, leaving // previewPending/previewFailed both false, so the caller's afterPlaceholder always resolved to @@ -763,7 +771,11 @@ export async function buildCapture( // #9881: `absent` means no preview check-run was ever found, and the budget is now spent -- so // this is not a late or broken deploy, it is a repo with no preview pipeline. Recording it is // what lets the screenshot-table gate decline to CLOSE on evidence it could never obtain. - previewUnobtainable = true; + // #10059: record it ONLY on a PROVEN absence — the check-run read succeeded (`absent`, not the + // `unreadable` catch) AND the deployments read succeeded. A transient read failure is not proof. + if (buildState === "absent" && !deploymentsReadFailed) { + previewUnobtainable = true; + } } else { await recordPreviewPollAttempt(env, target.headSha); previewPending = true; diff --git a/src/review/visual/preview-url.ts b/src/review/visual/preview-url.ts index fadc335e2..9c2075e1c 100644 --- a/src/review/visual/preview-url.ts +++ b/src/review/visual/preview-url.ts @@ -307,8 +307,10 @@ export async function findPreviewUrlFromPrComments(params: { /** * State of the per-PR preview BUILD (Cloudflare Workers Builds check-run) for a head SHA, so capture can tell * "still building / its URL-comment is just lagging" (keep polling) apart from "failed" (show the terminal - * failed card) and "no preview build at all" (don't poll). Returns 'absent' on any read failure (fail-safe: - * never an infinite poll on a transient error). + * failed card) and "no preview build at all" (don't poll). A SUCCESSFUL read that finds no matching check-run + * returns 'absent'; a read that THREW returns 'unreadable' (still a value — the "never an infinite poll on a + * transient error" fail-safe is preserved — but a distinct one, so a caller can tell a proven "no build here" + * apart from "couldn't check", e.g. before recording structurally-unobtainable capture, #10059). */ export async function getPreviewBuildState(params: { token: string; @@ -316,7 +318,7 @@ export async function getPreviewBuildState(params: { sha: string; apiVersion?: string | undefined; rateLimitAdmissionKey?: GitHubRateLimitAdmissionKey | undefined; -}): Promise<"building" | "succeeded" | "failed" | "absent"> { +}): Promise<"building" | "succeeded" | "failed" | "absent" | "unreadable"> { const base = `https://api.github.com/repos/${params.repo.owner}/${params.repo.repo}`; const opts = { token: params.token, apiVersion: params.apiVersion, rateLimitAdmissionKey: params.rateLimitAdmissionKey }; try { @@ -333,7 +335,7 @@ export async function getPreviewBuildState(params: { ); return state ?? "absent"; } catch { - return "absent"; + return "unreadable"; } } diff --git a/test/unit/preview-url.test.ts b/test/unit/preview-url.test.ts index 31b8fd43f..6685c9bd3 100644 --- a/test/unit/preview-url.test.ts +++ b/test/unit/preview-url.test.ts @@ -204,9 +204,10 @@ describe("preview-url pagination (#7450)", () => { await expect(getPreviewBuildState({ token: "t", repo: REPO, sha: "s3" })).resolves.toBe("absent"); }); - it("getPreviewBuildState is bounded and degrades to absent on a later-page failure", async () => { + it("getPreviewBuildState is bounded, returns absent when the read completes empty, and unreadable when a later-page read fails (#10059)", async () => { const spin = vi.fn(async () => Response.json({ check_runs: [] }, { headers: { link: NEXT_LINK } })); vi.stubGlobal("fetch", spin); + // A read that COMPLETED across every page and found no build is a genuine absence. await expect(getPreviewBuildState({ token: "t", repo: REPO, sha: "spin" })).resolves.toBe("absent"); expect(spin).toHaveBeenCalledTimes(10); // PREVIEW_LIST_MAX_PAGES @@ -215,7 +216,9 @@ describe("preview-url pagination (#7450)", () => { return Response.json({ check_runs: [] }, { headers: { link: NEXT_LINK } }); }); vi.stubGlobal("fetch", failLater); - await expect(getPreviewBuildState({ token: "t", repo: REPO, sha: "fail" })).resolves.toBe("absent"); + // A read that THREW (here, mid-pagination) never proved absence — it is unreadable, still a value so the + // caller never infinite-polls, but distinct from a genuine empty read (#10059). + await expect(getPreviewBuildState({ token: "t", repo: REPO, sha: "fail" })).resolves.toBe("unreadable"); expect(failLater).toHaveBeenCalledTimes(2); }); diff --git a/test/unit/visual-capture.test.ts b/test/unit/visual-capture.test.ts index 2ef7a5821..998b1f173 100644 --- a/test/unit/visual-capture.test.ts +++ b/test/unit/visual-capture.test.ts @@ -342,6 +342,71 @@ describe("visual capture preview discovery", () => { await expect(previewPollAttemptCount(env, "budget-head-2")).resolves.toBe(MAX_PREVIEW_POLL_ATTEMPTS); }); + const captureAt = (env: Env, headSha: string, prNumber: number) => + buildCapture(env, "installation-token", { repoFullName: "owner/repo", prNumber, headSha, previewFromChecks: true }, ["apps/loopover-ui/src/routes/app.index.tsx"]); + + it("REGRESSION (#10059): a rejecting check-runs read at an exhausted budget yields previewUnobtainable false, not a proven absence", async () => { + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/deployments?")) return Response.json([]); + if (url.includes("/status")) return Response.json({ statuses: [] }); + if (url.includes("/check-runs")) throw new Error("simulated GitHub read failure"); + if (url.includes("/comments")) return Response.json([]); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "", REVIEW_AUDIT: memoryReviewAudit() }); + for (let i = 0; i < MAX_PREVIEW_POLL_ATTEMPTS; i += 1) await recordPreviewPollAttempt(env, "unreadable-head"); + const result = await captureAt(env, "unreadable-head", 21); + expect(result.previewUnobtainable).toBe(false); + expect(result.previewPending).toBe(false); + }); + + it("#9881 pinned: a SUCCEEDING check-runs read that finds no preview build at an exhausted budget yields previewUnobtainable true", async () => { + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/deployments?")) return Response.json([]); + if (url.includes("/status")) return Response.json({ statuses: [] }); + if (url.includes("/check-runs")) return Response.json({ check_runs: [{ name: "lint", status: "completed", conclusion: "success" }] }); + if (url.includes("/comments")) return Response.json([]); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "", REVIEW_AUDIT: memoryReviewAudit() }); + for (let i = 0; i < MAX_PREVIEW_POLL_ATTEMPTS; i += 1) await recordPreviewPollAttempt(env, "absent-head"); + const result = await captureAt(env, "absent-head", 22); + expect(result.previewUnobtainable).toBe(true); + }); + + it("REGRESSION (#10059): a rejecting deployments read suppresses previewUnobtainable even when the check-runs read succeeds empty", async () => { + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/deployments?")) throw new Error("simulated deployments read failure"); + if (url.includes("/status")) return Response.json({ statuses: [] }); + if (url.includes("/check-runs")) return Response.json({ check_runs: [{ name: "lint", status: "completed", conclusion: "success" }] }); + if (url.includes("/comments")) return Response.json([]); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "", REVIEW_AUDIT: memoryReviewAudit() }); + for (let i = 0; i < MAX_PREVIEW_POLL_ATTEMPTS; i += 1) await recordPreviewPollAttempt(env, "deploy-fail-head"); + const result = await captureAt(env, "deploy-fail-head", 23); + expect(result.previewUnobtainable).toBe(false); + }); + + it("REGRESSION (#10059): a sustained GitHub read failure across the whole poll budget never marks capture structurally unobtainable", async () => { + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/deployments?")) throw new Error("read failure"); + if (url.includes("/check-runs")) throw new Error("read failure"); + if (url.includes("/comments")) return Response.json([]); + if (url.includes("/status")) return Response.json({ statuses: [] }); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({ PUBLIC_API_ORIGIN: "https://worker.example", PUBLIC_SITE_ORIGIN: "", REVIEW_AUDIT: memoryReviewAudit() }); + for (let i = 0; i <= MAX_PREVIEW_POLL_ATTEMPTS; i += 1) { + const result = await captureAt(env, "sustained-fail-head", 24); + expect(result.previewUnobtainable).toBe(false); + } + }); + it("eternal-loading-placeholder fix: marks the capture pending (not silently ignored) when no matching preview check run exists at all (buildState 'absent') and no actions_fallback is configured", async () => { vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { const url = input.toString();