diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 1fc9397f7b..78ab041472 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -359,7 +359,7 @@ import { randomUUID } from "node:crypto"; import { isRetryableJobError, RetryableJobError } from "./retryable"; import { screenshotsAllowed } from "../review/visual-wire"; import { isVisualPath } from "../review/visual/paths"; -import { buildCapture, hasSuccessfulBotCapture, resolveVisualRoutes, type CaptureRoute } from "../review/visual/capture"; +import { buildCapture, fetchShotContentBlock, hasSuccessfulBotCapture, resolveVisualRoutes, type CaptureRoute } from "../review/visual/capture"; import { clearFallbackDispatchMarker, fallbackShotFileName, @@ -368,6 +368,13 @@ import { FALLBACK_WORKFLOW_NAME, parseFallbackRunCorrelation, } from "../review/visual/actions-fallback"; +import { + buildVisualRegressionFindings, + buildVisualVisionUserPrompt, + evaluateVisualVisionGate, + parseVisualVisionResponse, + VISUAL_VISION_SYSTEM_PROMPT, +} from "../review/visual/visual-findings"; import { incr } from "../selfhost/metrics"; import { renderReviewingPlaceholder, @@ -419,6 +426,7 @@ import { getLastRepoDocRefreshAttemptedAtBulk, performRepoDocRefresh } from "../ import { isRepoDocRefreshDue } from "../review/repo-doc-refresh-schedule"; import type { LocalBranchAnalysisInput } from "../signals/local-branch"; import { + callAiProvider, hasPublicReviewAssessment, isEnabled, runGittensoryAiReview, @@ -535,9 +543,10 @@ import { runSelfTuneBreaker, } from "../review/outcomes-wire"; import { neutralHoldReasonCode, recordNativeGateDecision } from "../review/parity-wire"; -import type { SubmissionOutcome } from "../review/submitter-reputation"; +import { getSubmitterReputation, type SubmissionOutcome } from "../review/submitter-reputation"; import type { AdvisoryFinding, + AiContentBlock, ContributorEvidenceRecord, ContributorRepoStatRecord, DetectedNotificationEvent, @@ -8187,6 +8196,97 @@ class RetryablePublicSurfacePublishFailedError extends RetryableJobError { } } +/** + * AI-vision analysis of a confirmed visual regression (#4111 wiring): the existing pixel-diff threshold can + * tell "the pixels changed" but not "does it look broken" — a route the capture pipeline already flagged + * changed gets ONE more look from a real vision-capable model. Mirrors runAiReviewForAdvisory's own shape + * (resolve reputation + BYOK, gate, call, parse, mutate `args.advisory.findings`) so it can be exercised + * directly in tests without driving the full webhook pipeline. STRICTLY ADVISORY: `visual_regression_finding` + * can never become a gate blocker (see visual-findings.ts's header) — this only ever adds a "Visual findings" + * collapsible to the comment. Never throws: any failure (a broken image fetch, a provider error, an + * unparseable response) degrades to "no finding added", exactly like the capture block it runs after. + */ +export async function runVisualVisionForAdvisory( + env: Env, + args: { + repoFullName: string; + pr: { number: number }; + author: string | null; + confirmedContributor: boolean; + settings: RepositorySettings; + advisory: { findings: AdvisoryFinding[] }; + routes: readonly CaptureRoute[]; + }, +): Promise { + if (args.routes.length === 0) return; + try { + const visionReputation = await getSubmitterReputation(env, args.repoFullName, args.author ?? undefined); + // BYOK resolution mirrors runAiReviewForAdvisory's own (re-resolved per-caller is this codebase's + // established convention for this exact 3-line block, not an anti-pattern — see e.g. runAiSlopForAdvisory). + const storedVisionKey = + args.confirmedContributor && args.settings.aiReviewByok + ? await getDecryptedRepositoryAiKey(env, args.repoFullName) + : null; + const visionProviderKey = + storedVisionKey && + (!args.settings.aiReviewProvider || args.settings.aiReviewProvider === storedVisionKey.provider) + ? { + provider: storedVisionKey.provider, + key: storedVisionKey.key, + model: args.settings.aiReviewModel ?? storedVisionKey.model, + } + : null; + const visionGate = evaluateVisualVisionGate({ + routes: args.routes, + reputationSignal: visionReputation.signal, + providerKey: visionProviderKey, + }); + if (!visionGate.run) return; + // evaluateVisualVisionGate only ever returns run:true when its own providerKey input (the SAME + // visionProviderKey resolved above) was non-null -- this is a defensive type-narrowing guard for the + // callAiProvider call below, not a reachable false case. + /* v8 ignore next -- see comment above */ + if (!visionProviderKey) return; + const images: AiContentBlock[] = []; + for (const route of visionGate.routes) { + // Show the model the viewport that actually crossed the pixel-diff threshold — a route can qualify via + // desktop, mobile, or both; preferring desktop only when BOTH changed keeps this a single before/after + // pair per route (the prompt's own "before, after order" contract), same as + // routeHasConfirmedVisualRegression's own desktop-first `||` check. + const useMobile = !route.diffUrl && Boolean(route.diffUrlMobile); + const beforeShotUrl = useMobile ? route.beforeUrlMobile : route.beforeUrl; + const afterShotUrl = useMobile ? route.afterUrlMobile : route.afterUrl; + if (!beforeShotUrl || !afterShotUrl) continue; + const [beforeBlock, afterBlock] = await Promise.all([ + fetchShotContentBlock(beforeShotUrl), + fetchShotContentBlock(afterShotUrl), + ]); + if (beforeBlock) images.push(beforeBlock); + if (afterBlock) images.push(afterBlock); + } + if (images.length === 0) return; + const visionResponse = await callAiProvider( + visionProviderKey, + VISUAL_VISION_SYSTEM_PROMPT, + buildVisualVisionUserPrompt(visionGate.routes), + 600, + images, + ); + if (!visionResponse.text) return; + const visionFindings = parseVisualVisionResponse(visionResponse.text); + args.advisory.findings.push(...buildVisualRegressionFindings(visionFindings)); + } catch (error) { + console.log( + JSON.stringify({ + event: "visual_vision_error", + repoFullName: args.repoFullName, + pull: args.pr.number, + message: errorMessage(error).slice(0, 200), + }), + ); + } +} + async function maybePublishPrPublicSurface( env: Env, installationId: number, @@ -8224,6 +8324,11 @@ async function maybePublishPrPublicSurface( }, ): Promise | undefined> { const author = pr.authorLogin ?? null; + // Hoisted out of the try-block below (where it's actually resolved) so the AI-vision step further down -- + // which needs the SAME already-resolved confirmed-Gittensor status for its own BYOK gate, mirroring + // runAiReviewForAdvisory's identical check -- can read it without a second, audit-event-duplicating + // getCachedOfficialMinerDetection lookup. Defaults false; only ever set true inside that try-block. + let confirmedContributor = false; // Resolve the repo's action mode ONCE for the whole publish pass and thread it into every GitHub write below, so // a dry-run / pause / global-freeze publishes NOTHING (check-run, comment, label) — the gate verdict is still // computed + returned for the disposition logic, the writes are just suppressed + audited. (#dry-run-chokepoint) @@ -8908,7 +9013,7 @@ async function maybePublishPrPublicSurface( // Resolve the author's confirmed-Gittensor status. It feeds on-chain SCORING and the public surface, but // it no longer gates the verdict — every author is hard-blocked the same way on a configured blocker, and // a clean PR passes the same way. (#gate-nonconfirmed) - const confirmedContributor = official?.status === "confirmed"; + confirmedContributor = official?.status === "confirmed"; // Anti-slop (#530/#532): only when opted in (slopGateMode !== "off"). Surface the deterministic slop // findings as advisory context, and feed the score to the gate (it only blocks under slop: block + the @@ -10431,6 +10536,18 @@ async function maybePublishPrPublicSurface( ); } } + // AI-vision analysis of a confirmed visual regression (#4111 wiring) — see runVisualVisionForAdvisory's + // own doc comment. Deliberately independent of the capture block above (its own try/catch there) so a + // vision failure can never affect the "Visual preview" section that block already rendered. + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author, + confirmedContributor, + settings, + advisory, + routes: beforeAfter, + }); // review.memory (#2181, apply slice of #1964): before the unified comment renders, suppress/demote // advisory (non-blocking) findings a maintainer already dismissed as false positives for this repo. ONLY // ever applied to `commentGate.warnings` -- NEVER `commentGate.blockers` -- so this can never change the diff --git a/src/review/visual/capture.ts b/src/review/visual/capture.ts index 548985a53e..a8d7910678 100644 --- a/src/review/visual/capture.ts +++ b/src/review/visual/capture.ts @@ -10,7 +10,8 @@ // buildCapture), adapted to gittensory bindings + origins. The agent-config-driven route rules, authed-route // preview session, and explicit-route override are intentionally dropped here — gittensory's UI uses the // default TanStack route convention; those hooks can return if a per-repo visual config is added. -import { sha256Hex } from "../../utils/crypto"; +import { base64Encode, sha256Hex } from "../../utils/crypto"; +import type { AiContentBlock } from "../../types"; import type { GitHubRateLimitAdmissionKey } from "../../github/client"; import { dispatchVisualCaptureFallback, fallbackShotR2Key, isFallbackDispatchInFlight, markFallbackDispatched } from "./actions-fallback"; import { @@ -88,6 +89,25 @@ export function hasSuccessfulBotCapture(routes: readonly CaptureRoute[]): boolea return routes.some(routeHasRealBeforeAfterPair); } +/** + * Fetch an already-captured shot (a `CaptureRoute.before*`/`after*` URL) and return it as an `AiContentBlock` + * for a vision-capable AI call (#4111 wiring) — every captured shot is a PNG (see `capturePage`'s + * `screenshot({type: "png", ...})` call in `./shot.ts`), so the MIME type is fixed rather than sniffed. + * Returns undefined on any fetch/read failure so one broken image degrades to "drop this image", never a + * thrown error — mirrors `capturePage`'s own "returns null on any failure so callers degrade gracefully" + * convention. + */ +export async function fetchShotContentBlock(url: string): Promise { + try { + const response = await fetch(url); + if (!response.ok) return undefined; + const bytes = new Uint8Array(await response.arrayBuffer()); + return { type: "image", data: base64Encode(bytes), mimeType: "image/png" }; + } catch { + return undefined; + } +} + /** Inputs the capture pipeline needs about the PR under review (resolved by the caller from gittensory data). */ export interface CaptureTarget { repoFullName: string; diff --git a/src/utils/crypto.ts b/src/utils/crypto.ts index eb4f7ee8e0..d731f3abb6 100644 --- a/src/utils/crypto.ts +++ b/src/utils/crypto.ts @@ -103,7 +103,7 @@ export async function decryptSecret(ciphertext: string, iv: string, keyMaterial: return new TextDecoder().decode(decrypted); } -function base64Encode(bytes: Uint8Array): string { +export function base64Encode(bytes: Uint8Array): string { let binary = ""; for (const byte of bytes) binary += String.fromCharCode(byte); return btoa(binary); diff --git a/test/unit/visual-capture.test.ts b/test/unit/visual-capture.test.ts index 08cf3bffcd..2d72479208 100644 --- a/test/unit/visual-capture.test.ts +++ b/test/unit/visual-capture.test.ts @@ -5,7 +5,7 @@ import { latestGitHubRestRateLimitObservation, } from "../../src/github/client"; import { fallbackShotR2Key, markFallbackDispatched } from "../../src/review/visual/actions-fallback"; -import { buildCapture, hasSuccessfulBotCapture, mapFilesToRoutes, resolvePreviewUrlTemplate, resolveVisualRoutes } from "../../src/review/visual/capture"; +import { buildCapture, fetchShotContentBlock, hasSuccessfulBotCapture, mapFilesToRoutes, resolvePreviewUrlTemplate, resolveVisualRoutes } from "../../src/review/visual/capture"; import type { CaptureRoute } from "../../src/review/visual/capture"; import * as pixelDiffModule from "../../src/review/visual/pixel-diff"; import * as previewUrlModule from "../../src/review/visual/preview-url"; @@ -1555,3 +1555,25 @@ describe("review.visual.actions_fallback (#4112 GitHub-Actions build-and-serve f expect(result.routes[0]?.afterUrl).toContain("placeholder=loading"); }); }); + +describe("fetchShotContentBlock (#4111)", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("returns a base64-encoded image content block on a successful fetch", async () => { + vi.stubGlobal("fetch", vi.fn(async () => new Response(new Uint8Array([137, 80, 78, 71]), { status: 200 }))); + const block = await fetchShotContentBlock("https://x/gittensory/shot?key=before"); + expect(block).toEqual({ type: "image", data: Buffer.from([137, 80, 78, 71]).toString("base64"), mimeType: "image/png" }); + }); + + it("returns undefined on a non-2xx response", async () => { + vi.stubGlobal("fetch", vi.fn(async () => new Response("not found", { status: 404 }))); + await expect(fetchShotContentBlock("https://x/gittensory/shot?key=missing")).resolves.toBeUndefined(); + }); + + it("returns undefined (never throws) when fetch itself rejects", async () => { + vi.stubGlobal("fetch", vi.fn(async () => { throw new Error("network down"); })); + await expect(fetchShotContentBlock("https://x/gittensory/shot?key=broken")).resolves.toBeUndefined(); + }); +}); diff --git a/test/unit/visual-vision-wiring.test.ts b/test/unit/visual-vision-wiring.test.ts new file mode 100644 index 0000000000..1d05df6c75 --- /dev/null +++ b/test/unit/visual-vision-wiring.test.ts @@ -0,0 +1,391 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { runVisualVisionForAdvisory } from "../../src/queue/processors"; +import * as repositories from "../../src/db/repositories"; +import { upsertRepositoryAiKey } from "../../src/db/repositories"; +import * as submitterReputation from "../../src/review/submitter-reputation"; +import type { CaptureRoute } from "../../src/review/visual/capture"; +import type { AdvisoryFinding, RepositorySettings } from "../../src/types"; +import { createTestEnv } from "../helpers/d1"; + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +const pr = { number: 3 }; +const repoFullName = "acme/widgets"; + +function byokEnv() { + return createTestEnv({ TOKEN_ENCRYPTION_SECRET: "vision-test-encryption-secret-32b" }); +} + +function byokSettings(over: Partial = {}): RepositorySettings { + return { aiReviewByok: true, ...over } as RepositorySettings; +} + +function findingsHolder(): { findings: AdvisoryFinding[] } { + return { findings: [] }; +} + +function route(over: Partial & { path: string }): CaptureRoute { + return { ...over }; +} + +function findingsResponse(findings: Array<{ path: string; body: string }>) { + return JSON.stringify({ findings }); +} + +function anthropicOk(text: string) { + return new Response(JSON.stringify({ content: [{ type: "text", text }] }), { status: 200 }); +} + +/** Routes fetch (shot PNGs) vs the AI provider call (api.anthropic.com) by URL, mirroring the shot-URL + * convention (`/gittensory/shot?key=...`) so a single fetch mock can serve both without a real network. */ +function stubShotsAndProvider(providerResponseText: string | null) { + vi.stubGlobal("fetch", vi.fn(async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.anthropic.com/v1/messages") { + return providerResponseText === null + ? new Response("upstream error", { status: 500 }) + : anthropicOk(providerResponseText); + } + if (url.includes("/gittensory/shot")) return new Response(new Uint8Array([1, 2, 3]), { status: 200, headers: { "content-type": "image/png" } }); + return new Response("not found", { status: 404 }); + })); +} + +describe("runVisualVisionForAdvisory", () => { + it("no-ops on an empty route list -- never touches D1 or the network", async () => { + const env = byokEnv(); + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: true, + settings: byokSettings(), + advisory: adv, + routes: [], + }); + expect(adv.findings).toEqual([]); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("handles a null author (ghost/deleted account) by treating it as an anonymous submitter, not a crash", async () => { + const env = byokEnv(); + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: null, + confirmedContributor: false, + settings: byokSettings({ aiReviewByok: false }), + advisory: adv, + routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=b", afterUrl: "https://x/gittensory/shot?key=a" })], + }); + expect(adv.findings).toEqual([]); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("declines when no route crossed the pixel-diff threshold (no_confirmed_regression) -- never resolves BYOK", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: true, + settings: byokSettings(), + advisory: adv, + routes: [route({ path: "/app", beforeUrl: "https://x/gittensory/shot?key=b", afterUrl: "https://x/gittensory/shot?key=a" })], + }); + expect(adv.findings).toEqual([]); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("declines for a low-reputation submitter even with a confirmed regression and BYOK configured", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); + // Reputation-signal derivation is submitter-reputation.ts's own concern (see submitter-reputation.test.ts); + // this test only verifies runVisualVisionForAdvisory correctly DECLINES on a "low" signal. + vi.spyOn(submitterReputation, "getSubmitterReputation").mockResolvedValueOnce({ + submissions: 6, + merged: 0, + closed: 6, + manual: 0, + closeRate: 1, + signal: "low", + }); + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "bob", + confirmedContributor: true, + settings: byokSettings(), + advisory: adv, + routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=b", afterUrl: "https://x/gittensory/shot?key=a" })], + }); + expect(adv.findings).toEqual([]); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("declines when BYOK is not configured (aiReviewByok off) even with a confirmed regression", async () => { + const env = byokEnv(); + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: true, + settings: byokSettings({ aiReviewByok: false }), + advisory: adv, + routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=b", afterUrl: "https://x/gittensory/shot?key=a" })], + }); + expect(adv.findings).toEqual([]); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("declines when the submitter is not a confirmed contributor, even with BYOK configured", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: false, + settings: byokSettings(), + advisory: adv, + routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=b", afterUrl: "https://x/gittensory/shot?key=a" })], + }); + expect(adv.findings).toEqual([]); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("skips BYOK (declines, falls back to nothing) when the declared provider doesn't match the stored key", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: true, + settings: byokSettings({ aiReviewProvider: "openai" }), + advisory: adv, + routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=b", afterUrl: "https://x/gittensory/shot?key=a" })], + }); + expect(adv.findings).toEqual([]); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("calls the BYOK vision provider with before+after images and publishes a returned finding (desktop route)", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); + stubShotsAndProvider(findingsResponse([{ path: "/app", body: "The submit button is clipped on the right edge." }])); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: true, + settings: byokSettings(), + advisory: adv, + routes: [ + route({ + path: "/app", + diffUrl: "https://x/gittensory/shot?key=diff-desktop", + beforeUrl: "https://x/gittensory/shot?key=before-desktop", + afterUrl: "https://x/gittensory/shot?key=after-desktop", + beforeUrlMobile: "https://x/gittensory/shot?key=before-mobile", + afterUrlMobile: "https://x/gittensory/shot?key=after-mobile", + }), + ], + }); + expect(adv.findings).toEqual([ + { + code: "visual_regression_finding", + severity: "warning", + title: "Possible visual regression: /app", + detail: "The submit button is clipped on the right edge.", + action: "Advisory only — verify against the Visual preview screenshots before deciding.", + }, + ]); + }); + + it("uses the mobile viewport's shots when only diffUrlMobile (not diffUrl) crossed the threshold", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); + const requestedUrls: string[] = []; + vi.stubGlobal("fetch", vi.fn(async (input: RequestInfo | URL) => { + const url = input.toString(); + requestedUrls.push(url); + if (url === "https://api.anthropic.com/v1/messages") return anthropicOk(findingsResponse([])); + if (url.includes("/gittensory/shot")) return new Response(new Uint8Array([1, 2, 3]), { status: 200 }); + return new Response("not found", { status: 404 }); + })); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: true, + settings: byokSettings(), + advisory: adv, + routes: [ + route({ + path: "/app", + diffUrlMobile: "https://x/gittensory/shot?key=diff-mobile", + beforeUrl: "https://x/gittensory/shot?key=before-desktop", + afterUrl: "https://x/gittensory/shot?key=after-desktop", + beforeUrlMobile: "https://x/gittensory/shot?key=before-mobile", + afterUrlMobile: "https://x/gittensory/shot?key=after-mobile", + }), + ], + }); + expect(requestedUrls).toContain("https://x/gittensory/shot?key=before-mobile"); + expect(requestedUrls).toContain("https://x/gittensory/shot?key=after-mobile"); + expect(requestedUrls).not.toContain("https://x/gittensory/shot?key=before-desktop"); + expect(requestedUrls).not.toContain("https://x/gittensory/shot?key=after-desktop"); + }); + + it("skips a route whose confirmed-changed viewport is missing its before/after shot URLs", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); + stubShotsAndProvider(findingsResponse([])); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: true, + settings: byokSettings(), + advisory: adv, + // diffUrl set (confirmed changed) but no beforeUrl/afterUrl at all -- degrades to "no images from this route". + routes: [route({ path: "/broken", diffUrl: "https://x/gittensory/shot?key=diff" })], + }); + expect(adv.findings).toEqual([]); + }); + + it("degrades gracefully when a shot image fetch fails -- proceeds with only the images that succeeded", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); + vi.stubGlobal("fetch", vi.fn(async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.anthropic.com/v1/messages") return anthropicOk(findingsResponse([])); + if (url.includes("key=before")) return new Response("not found", { status: 404 }); + if (url.includes("/gittensory/shot")) return new Response(new Uint8Array([1, 2, 3]), { status: 200 }); + return new Response("not found", { status: 404 }); + })); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: true, + settings: byokSettings(), + advisory: adv, + routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=before", afterUrl: "https://x/gittensory/shot?key=after" })], + }); + // The "after" image alone was enough to attempt the call; the model returned no findings either way. + expect(adv.findings).toEqual([]); + }); + + it("never calls the AI provider when every candidate route's images all fail to fetch", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); + const providerCalls: string[] = []; + vi.stubGlobal("fetch", vi.fn(async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.anthropic.com/v1/messages") { + providerCalls.push(url); + return anthropicOk(findingsResponse([])); + } + return new Response("not found", { status: 404 }); + })); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: true, + settings: byokSettings(), + advisory: adv, + routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=before", afterUrl: "https://x/gittensory/shot?key=after" })], + }); + expect(providerCalls).toEqual([]); + expect(adv.findings).toEqual([]); + }); + + it("adds no finding when the model returns a response with no usable JSON (fail-safe parse)", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); + stubShotsAndProvider("I looked at the screenshots and everything seems fine, no JSON here."); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: true, + settings: byokSettings(), + advisory: adv, + routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=before", afterUrl: "https://x/gittensory/shot?key=after" })], + }); + expect(adv.findings).toEqual([]); + }); + + it("adds no finding when the provider call itself fails (non-2xx) -- callAiProvider's own fail-safe", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); + stubShotsAndProvider(null); + const adv = findingsHolder(); + await runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: true, + settings: byokSettings(), + advisory: adv, + routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=before", afterUrl: "https://x/gittensory/shot?key=after" })], + }); + expect(adv.findings).toEqual([]); + }); + + it("swallows a thrown error from the BYOK key lookup and never lets it escape (visual_vision_error)", async () => { + const env = byokEnv(); + await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); + vi.spyOn(repositories, "getDecryptedRepositoryAiKey").mockRejectedValueOnce(new Error("D1 unavailable")); + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + const adv = findingsHolder(); + await expect( + runVisualVisionForAdvisory(env, { + repoFullName, + pr, + author: "alice", + confirmedContributor: true, + settings: byokSettings(), + advisory: adv, + routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=before", afterUrl: "https://x/gittensory/shot?key=after" })], + }), + ).resolves.toBeUndefined(); + expect(adv.findings).toEqual([]); + expect(fetchMock).not.toHaveBeenCalled(); + }); +});