diff --git a/src/github/backfill.ts b/src/github/backfill.ts index a50eb55f6f..89ee5ddc61 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -1905,6 +1905,81 @@ async function fetchPullRequestChecks( return { check_runs: checkRuns }; } +const CI_FAILING_CONCLUSIONS = new Set(["failure", "timed_out", "cancelled", "action_required", "startup_failure"]); +const CI_PASSING_CONCLUSIONS = new Set(["success", "neutral", "skipped"]); + +export type LiveCiAggregate = { + ciState: "passed" | "failed" | "pending" | "unverified"; + failingDetails: Array<{ name: string; summary?: string; detailsUrl?: string }>; +}; + +/** + * Fetch the head SHA's LIVE CI aggregate over BOTH GitHub Check-runs AND classic commit-statuses. This is the + * reviewbot `getAllChecksState` parity that the converged auto-maintain path needs: codecov (codecov/patch, + * codecov/project) and many other tools post a classic COMMIT-STATUS, not a check-run — fetching only + * `/check-runs` (what the backfill sync does) misses them entirely, which is why a red codecov was reported as + * "CI green". We aggregate ANY failing check/status → "failed"; else any still-running → "pending"; else any + * present → "passed"; none at all → "unverified". The disposition layer NEVER approves/merges unless "passed", + * and closes (non-owner) / holds (owner) on "failed". Best-effort: a fetch error degrades that source to empty. + */ +export async function fetchLiveCiAggregate(env: Env, repoFullName: string, headSha: string | null | undefined, token: string | undefined): Promise { + if (!headSha) return { ciState: "unverified", failingDetails: [] }; + const failingDetails: LiveCiAggregate["failingDetails"] = []; + let total = 0; + let anyPending = false; + + // 1) Check-runs (GitHub Actions jobs, CodeQL, app checks). + for (let page = 1; page <= PR_DETAIL_MAX_PAGES; page += 1) { + const result = await githubJsonWithHeaders<{ check_runs?: Array }>( + env, + repoFullName, + `/commits/${headSha}/check-runs?per_page=100&page=${page}`, + token, + ).catch(() => undefined); + if (!result) break; + for (const run of result.data.check_runs ?? []) { + total += 1; + const conclusion = (run.conclusion ?? "").toLowerCase(); + const status = (run.status ?? "").toLowerCase(); + if (conclusion ? CI_FAILING_CONCLUSIONS.has(conclusion) : false) { + const summary = [run.output?.title, run.output?.summary].find((value): value is string => typeof value === "string" && value.trim().length > 0)?.trim().slice(0, 200); + failingDetails.push({ name: run.name, ...(summary ? { summary } : {}), ...(run.details_url ? { detailsUrl: run.details_url } : {}) }); + } else if (conclusion ? CI_PASSING_CONCLUSIONS.has(conclusion) : status === "completed") { + // concluded and not failing → passing + } else { + anyPending = true; // queued / in_progress / not yet concluded + } + } + if (!hasNextPage(result.link)) break; + } + + // 2) Classic commit-statuses (codecov/patch, codecov/project, and any other status-API context). The + // combined endpoint returns the LATEST status per context, so a context that flipped red→green is counted + // once at its current state. + const statusResult = await githubJsonWithHeaders<{ statuses?: Array<{ context?: string | null; state?: string | null; description?: string | null; target_url?: string | null }> }>( + env, + repoFullName, + `/commits/${headSha}/status?per_page=100`, + token, + ).catch(() => undefined); + for (const ctx of statusResult?.data.statuses ?? []) { + total += 1; + const state = (ctx.state ?? "").toLowerCase(); + const name = ctx.context ?? "status"; + if (state === "failure" || state === "error") { + const summary = typeof ctx.description === "string" ? ctx.description.trim().slice(0, 200) : ""; + failingDetails.push({ name, ...(summary ? { summary } : {}), ...(ctx.target_url ? { detailsUrl: ctx.target_url } : {}) }); + } else if (state === "success") { + // passing + } else { + anyPending = true; // pending + } + } + + const ciState: LiveCiAggregate["ciState"] = failingDetails.length > 0 ? "failed" : anyPending ? "pending" : total > 0 ? "passed" : "unverified"; + return { ciState, failingDetails }; +} + async function fetchPullRequestDetailsFromGraphQl( env: Env, repoFullName: string, diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 4a4d54f484..b41383fdf3 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -64,6 +64,7 @@ import { backfillRepositorySegment, enqueueRepositoryOpenDataBackfill, fetchAndStorePullRequestFilesForReview, + fetchLiveCiAggregate, refreshContributorActivity, refreshInstallationHealth, refreshPullRequestDetails, @@ -167,6 +168,7 @@ import { buildReviewRagContext, isRagEnabled } from "../review/rag-wire"; import { indexRepo, reindexChangedPaths } from "../review/rag-index"; import { isReputationEnabled, recordReputationOutcome, shouldSkipAiForReputation } from "../review/reputation-wire"; import { isConvergenceRepoAllowed } from "../review/cutover-gate"; +import { deploymentStatusToPreview, type DeploymentStatusPayload } from "../review/visual/preview-url"; import { loadHardGuardrailGlobs } from "../review/guardrail-config"; import { isOpsEnabled, runOpsAlerts } from "../review/ops-wire"; import { isSelfTuneEnabled, runSelfTune } from "../review/selftune-wire"; @@ -528,12 +530,30 @@ async function sweepRepoRegate(env: Env, repoFullName: string | undefined): Prom const requireLinkedIssue = settings.requireLinkedIssue || settings.linkedIssueGateMode !== "off"; const verdicts: Record = {}; const flaggedPulls: number[] = []; + const sweepInstallationId = repo?.installationId ?? null; for (const pr of candidates) { const others = openPullRequests.filter((other) => other.number !== pr.number); const advisory = buildPullRequestAdvisory(repo, pr, { otherOpenPullRequests: others, requireLinkedIssue }); const gate = evaluateGateCheck(advisory, gateCheckPolicy(settings, null, undefined, pr.slopRisk ?? null)); verdicts[String(pr.number)] = gate.conclusion; if (gate.conclusion === "failure" || gate.conclusion === "action_required") flaggedPulls.push(pr.number); + // Backstop the CI-completion trigger: re-run auto-maintain so a clean+green+approved PR is merged and a + // red-CI non-owner PR is closed (owner held) even if its check_run/check_suite webhook was missed or + // coalesced. maybeRunAgentMaintenance self-guards on autonomy + fetches the live CI aggregate itself. + if (sweepInstallationId != null) { + await maybeRunAgentMaintenance(env, { + installationId: sweepInstallationId, + repoFullName, + repo, + pr, + settings, + otherOpenPullRequests: others, + deliveryId: `regate-sweep:${repoFullName}#${pr.number}`, + gate, + }).catch((error) => { + console.error(JSON.stringify({ level: "warn", event: "agent_maintenance_failed", deliveryId: `regate-sweep:${repoFullName}#${pr.number}`, repository: repoFullName, pullNumber: pr.number, error: errorMessage(error) })); + }); + } } await recordAuditEvent(env, { eventType: "agent.sweep.regate", @@ -580,9 +600,15 @@ async function maybeRunAgentMaintenance( // FIX B: resolve files via the shared resolver so an EMPTY stored list (the maintenance ran before the // detail-sync populated pull_request_files) can't silently empty changedPaths and let a guarded PR slip the // guardrail into an auto-merge — it inline-fetches the real changed paths when stored is still empty. - const [changedFiles, hardGuardrailGlobs] = await Promise.all([ + // CRITICAL CI POLICY (reviewbot ci_red parity): fetch the LIVE CI aggregate over BOTH check-runs AND classic + // commit-statuses (codecov posts a commit-status, NOT a check-run — the stored check_summaries miss it). The + // planner uses this to NEVER approve/merge a PR whose CI isn't green, to CLOSE a red-CI non-owner PR (citing + // the failing checks) / HOLD the owner's, and to DEFER entirely while CI is still pending. + const ciToken = await createInstallationToken(env, installationId).catch(() => undefined); + const [changedFiles, hardGuardrailGlobs, ciAggregate] = await Promise.all([ resolvePullRequestFilesForReview(env, { installationId, repoFullName, pullNumber: pr.number }), loadHardGuardrailGlobs(env, repoFullName), + fetchLiveCiAggregate(env, repoFullName, pr.headSha, ciToken ?? env.GITHUB_PUBLIC_TOKEN), ]); const changedPaths = changedFiles.map((file) => file.path).filter((path) => path.length > 0); const repoOwner = repoFullName.includes("/") ? repoFullName.slice(0, repoFullName.indexOf("/")) : ""; @@ -600,6 +626,8 @@ async function maybeRunAgentMaintenance( hardGuardrailGlobs, authorIsOwner, authorIsAutomationBot, + ciState: ciAggregate.ciState, + failingCheckNames: ciAggregate.failingDetails.map((detail) => detail.name), pr: { mergeableState: pr.mergeableState, reviewDecision: pr.reviewDecision, @@ -629,6 +657,99 @@ async function maybeRunAgentMaintenance( ); } +/** + * Re-review a STORED open PR (no payload PR) — rebuild the advisory + gate, re-publish the unified comment, and + * re-run auto-maintain. Shared by the CI-completion (check_suite/check_run) handler below, mirroring reviewbot's + * "the CI event WAKES the existing row and re-runs the full review". The PR's persisted head SHA is used as-is + * (never overwritten from the CI payload — reviewbot scope parity). Best-effort throughout. + */ +async function reReviewStoredPullRequest(env: Env, deliveryId: string, installationId: number, repoFullName: string, prNumber: number): Promise { + const [repo, settings] = await Promise.all([getRepository(env, repoFullName), resolveRepositorySettings(env, repoFullName)]); + const pr = await getPullRequest(env, repoFullName, prNumber); + if (!pr || pr.state !== "open") return; + const otherOpenPullRequests = await listOtherOpenPullRequests(env, repoFullName, prNumber); + const advisory = buildPullRequestAdvisory(repo, pr, { otherOpenPullRequests, requireLinkedIssue: shouldCollectLinkedIssueEvidence(settings) }); + await persistAdvisory(env, advisory); + if (shouldCollectSlopEvidence(settings) || settings.manifestPolicyGateMode !== "off") { + await refreshPullRequestDetails(env, repoFullName, prNumber).catch(() => undefined); + } + const gate = await maybePublishPrPublicSurface(env, installationId, repoFullName, pr, repo, settings, advisory, { deliveryId }).catch((error) => { + console.error(JSON.stringify({ level: "warn", event: "pr_public_surface_failed", deliveryId, repository: repoFullName, pullNumber: prNumber, error: errorMessage(error) })); + return undefined; + }); + await maybeRunAgentMaintenance(env, { installationId, repoFullName, repo, pr, settings, otherOpenPullRequests, deliveryId, gate }).catch((error) => { + console.error(JSON.stringify({ level: "warn", event: "agent_maintenance_failed", deliveryId, repository: repoFullName, pullNumber: prNumber, error: errorMessage(error) })); + }); +} + +// One CI run fires MANY check_run (one per job) + check_suite completions. Re-reviewing on every one storms the +// PR with duplicate reviews (and races the request_changes/approve dedup). reviewbot's CI_COALESCE_WINDOW parity: +// re-review a given PR at most once per this window. The re-review always re-fetches the LIVE CI, so the window +// only bounds FREQUENCY, never correctness — a later out-of-window completion + the hourly sweep + the merge-time +// re-check still catch the settled state. +const CI_COALESCE_WINDOW_SECONDS = 60; + +/** + * Coalesce CI-completion re-reviews: claims a per-PR window and returns true if this PR was already re-reviewed + * within CI_COALESCE_WINDOW_SECONDS (caller skips). KV-backed (REVIEW_CONFIG); a missing KV or a KV hiccup + * degrades to NO coalescing (returns false — never blocks a re-review, never throws). + */ +async function ciReReviewCoalesced(env: Env, repoFullName: string, prNumber: number): Promise { + if (!env.REVIEW_CONFIG) return false; + const key = `ci-coalesce:${repoFullName.toLowerCase()}#${prNumber}`; + try { + if (await env.REVIEW_CONFIG.get(key)) return true; // re-reviewed within the window → skip this event + await env.REVIEW_CONFIG.put(key, "1", { expirationTtl: CI_COALESCE_WINDOW_SECONDS }); // claim the window + return false; + } catch { + return false; + } +} + +/** + * THE auto-merge / close-on-red TRIGGER. A `check_run`/`check_suite` `completed` event means a PR's CI just + * settled — re-review the associated PR(s) so the now-green PR is merged and the now-red PR is closed (non-owner) + * / held (owner). Without this, a PR reviewed at open-time (CI still pending → deferred) is never re-evaluated. + * Resolves the PR number(s) from `payload[event].pull_requests[]` (reviewbot core/scope.ts parity), NOT from + * the CI head SHA. COALESCED so one CI run's ~20 completions collapse to one re-review. Returns true (handled). + */ +async function maybeReReviewOnCiCompletion(env: Env, deliveryId: string, eventName: string, payload: GitHubWebhookPayload): Promise { + if (eventName !== "check_run" && eventName !== "check_suite") return false; + if (payload.action !== "completed") return false; + const repoFullName = payload.repository?.full_name; + const installationId = getInstallationId(payload); + if (!repoFullName || !installationId) return false; + const node = (payload as Record)[eventName] as { pull_requests?: Array<{ number?: number | null }> } | undefined; + const prNumbers = [...new Set((node?.pull_requests ?? []).map((entry) => entry?.number).filter((value): value is number => typeof value === "number"))]; + if (prNumbers.length > 0 && isConvergenceRepoAllowed(env, repoFullName)) { + for (const prNumber of prNumbers) { + // Coalesce the CI-completion storm: skip if this PR was re-reviewed within the window. + if (await ciReReviewCoalesced(env, repoFullName, prNumber)) continue; + await reReviewStoredPullRequest(env, deliveryId, installationId, repoFullName, prNumber); + } + } + await recordWebhookEvent(env, { deliveryId, eventName, action: payload.action, installationId, repositoryFullName: repoFullName, payloadHash: "processed", status: "processed" }); + return true; +} + +/** + * deployment_status (success/failure) → re-review the associated PR so the before/after visual capture fills the + * "after" cell once the preview deploy finishes (or flips to a deploy-failed note). Mirrors reviewbot's + * deployment_status routing; the capture itself runs inside the re-published review (visual-capture path). + */ +async function maybeCaptureOnDeploymentStatus(env: Env, deliveryId: string, eventName: string, payload: GitHubWebhookPayload): Promise { + if (eventName !== "deployment_status") return false; + const repoFullName = payload.repository?.full_name; + const installationId = getInstallationId(payload); + if (!repoFullName || !installationId) return false; + const preview = deploymentStatusToPreview(payload as unknown as DeploymentStatusPayload); + if (preview && isConvergenceRepoAllowed(env, repoFullName)) { + await reReviewStoredPullRequest(env, deliveryId, installationId, repoFullName, preview.prNumber); + } + await recordWebhookEvent(env, { deliveryId, eventName, action: payload.action, installationId, repositoryFullName: repoFullName, payloadHash: "processed", status: "processed" }); + return true; +} + async function repairDataFidelity(env: Env, requestedBy: "schedule" | "api" | "test"): Promise { const [repositories, segments, signalSnapshots] = await Promise.all([listRepositories(env), listRepoSyncSegments(env), listLatestSignalSnapshotsByTarget(env)]); const requiredSegments = new Set(["labels", "open_issues", "open_pull_requests"]); @@ -1087,6 +1208,14 @@ async function processGitHubWebhook(env: Env, deliveryId: string, eventName: str return; } + // CI-completion re-review — THE auto-merge / close-on-red trigger. A check_run/check_suite completion + // carries no `payload.pull_request`, so it must be handled BEFORE the pull_request block: it wakes the + // stored PR row and re-reviews it now that CI has settled (merge on green, close-non-owner / hold-owner on + // red). Without this a PR that goes green/red AFTER its open-time review is never re-evaluated. + if (await maybeReReviewOnCiCompletion(env, deliveryId, eventName, payload)) return; + // deployment_status (preview deploy finished) → re-review so the visual before/after capture fills in. + if (await maybeCaptureOnDeploymentStatus(env, deliveryId, eventName, payload)) return; + if (payload.repository?.full_name && payload.pull_request) { const repoFullName = payload.repository.full_name; const pr = await upsertPullRequestFromGitHub(env, repoFullName, payload.pull_request); @@ -1991,24 +2120,25 @@ async function maybePublishPrPublicSurface( // check's conclusion to passed/failed/unverified; any failure (failure/timed_out/cancelled/action_required) // flips the whole PR to 'failed'. The gate decision stays authoritative for the comment's color (always // passed here), so these CI chips never spuriously flip the unified status to held/blocked. - const checkSummaries = await listCheckSummaries(env, repoFullName, pr.number); - const failedChecks = checkSummaries.filter((check) => { - const conclusion = (check.conclusion ?? "").toLowerCase(); - return conclusion === "failure" || conclusion === "timed_out" || conclusion === "cancelled" || conclusion === "action_required"; - }); - const anyPassed = checkSummaries.some((check) => (check.conclusion ?? "").toLowerCase() === "success"); - const ciState: MergeReadiness["ciState"] = failedChecks.length > 0 ? "failed" : anyPassed ? "passed" : "unverified"; - // FIX D3: per-failed-check WHY (codecov %/test/lint reason) from each check's output.title/summary or a - // commit-status description — the SAME extraction the grounding path uses (checkSummaryText), capped + - // public-safe (check name + short reason only). The renderer lists these under the CI chip. - const failingDetails: CheckFailureDetail[] = failedChecks.map((check) => { - const summary = checkFailureSummaryText(check); - return { name: check.name, ...(summary ? { summary } : {}), ...(check.detailsUrl ? { detailsUrl: check.detailsUrl } : {}) }; - }); + // CRITICAL (CI-green parity): the comment's CI state must reflect the LIVE aggregate over BOTH check-runs + // AND classic commit-statuses — codecov (codecov/patch) posts a commit-status the stored check_summaries + // never captured, which is why a red codecov was shown as "CI green". Use the SAME live fetch the + // auto-maintain planner uses so the public chip and the disposition can never disagree. "pending" folds to + // the "unverified" bucket for the 3-state comment chip (renders "CI pending"). + const ciToken = await createInstallationToken(env, installationId).catch(() => undefined); + const liveCi = await fetchLiveCiAggregate(env, repoFullName, pr.headSha, ciToken ?? env.GITHUB_PUBLIC_TOKEN); + const ciState: MergeReadiness["ciState"] = liveCi.ciState === "passed" ? "passed" : liveCi.ciState === "failed" ? "failed" : "unverified"; + // Per-failed-check WHY (codecov %/test/lint reason) from each check-run output or commit-status + // description — capped + public-safe (name + short reason only). The renderer lists these under the CI chip. + const failingDetails: CheckFailureDetail[] = liveCi.failingDetails.map((detail) => ({ + name: detail.name, + ...(detail.summary ? { summary: detail.summary } : {}), + ...(detail.detailsUrl ? { detailsUrl: detail.detailsUrl } : {}), + })); const mergeReadiness: MergeReadiness = { ciState, ...(pr.mergeableState ? { mergeStateLabel: pr.mergeableState } : {}), - ...(failedChecks.length > 0 ? { failingChecks: failedChecks.map((check) => check.name) } : {}), + ...(failingDetails.length > 0 ? { failingChecks: failingDetails.map((detail) => detail.name) } : {}), ...(failingDetails.length > 0 ? { failingDetails } : {}), }; // Visual before/after capture (visual-capture port). Fires ONLY when (1) the global flag + per-repo diff --git a/src/review/unified-comment-bridge.ts b/src/review/unified-comment-bridge.ts index 8f314ea977..1bbb051440 100644 --- a/src/review/unified-comment-bridge.ts +++ b/src/review/unified-comment-bridge.ts @@ -221,33 +221,39 @@ export type UnifiedCommentBridgeArgs = { }; /** - * Build the "Visual preview" collapsible from the before/after capture routes — a markdown table of image - * cells pointing at the public /gittensory/shot URLs. Uses GitHub markdown image syntax `![](url)` rather - * than raw `` tags ON PURPOSE: the unified renderer's `details()` HTML-escapes a collapsible body (a - * security control so caller text can't inject structure-changing HTML), which would turn a literal `` - * into inert `<img>` text — markdown image syntax has no angle brackets, so it survives the escape and - * still renders as an image. Public-safe by construction: every cell is a route path or a shot URL (no - * private rubric/scoring terms). Returns null when nothing is renderable (no route has any shot URL), so the - * section is omitted entirely rather than showing an empty table. + * Build the "Visual preview" collapsible from the before/after capture routes — a clean table whose cells are + * CLICKABLE THUMBNAILS: a small `` (GitHub caps it to the column width) wrapped in an `` to the + * SAME full-resolution shot, so a click opens the screenshot full-size. One row per route per viewport + * (desktop / mobile), with the route path as the caption and a before (production) vs after (this PR's preview) + * column. Emitted as TRUSTED raw HTML (`rawHtml: true`) so the `/` survive — public-safe by + * construction: every value is a first-party minted /gittensory/shot URL or a route path (no private rubric / + * scoring terms), and a stray `"` in a URL is neutralized so it can't break out of the attribute. Returns null + * when nothing is renderable (no route has any shot URL), so the section is omitted rather than shown empty. */ export function buildBeforeAfterCollapsible(routes: CaptureRoute[]): UnifiedCollapsible | null { - const rows = routes - .filter((route) => route.beforeUrl || route.afterUrl || route.beforeUrlMobile || route.afterUrlMobile) - .map((route) => { - // Escape `(`/`)`/`]` in the URL so a crafted shot URL can't break out of the markdown image token; the - // URLs are first-party (we mint them), but this keeps the cell robust regardless. - const cell = (url: string | undefined): string => (url ? `![preview](${url.replace(/[()\]]/g, encodeURIComponent)})` : "—"); - return `| \`${route.path.replace(/\|/g, "\\|")}\` | ${cell(route.beforeUrl)} | ${cell(route.afterUrl)} |`; - }); + const attr = (value: string): string => value.replace(/"/g, "%22"); + const alt = (value: string): string => value.replace(/"/g, "'"); + const cell = (url: string | undefined, label: string): string => + url ? `${alt(label)}` : "—"; + const rows: string[] = []; + for (const route of routes) { + const path = `\`${route.path.replace(/\|/g, "\\|")}\``; + if (route.beforeUrl || route.afterUrl) { + rows.push(`| ${path} | desktop | ${cell(route.beforeUrl, `before ${route.path}`)} | ${cell(route.afterUrl, `after ${route.path}`)} |`); + } + if (route.beforeUrlMobile || route.afterUrlMobile) { + rows.push(`| ${path} | mobile | ${cell(route.beforeUrlMobile, `before ${route.path} (mobile)`)} | ${cell(route.afterUrlMobile, `after ${route.path} (mobile)`)} |`); + } + } if (rows.length === 0) return null; const body = [ - "| Route | Before (production) | After (this PR's preview) |", - "| --- | --- | --- |", + "| Route | Viewport | Before (production) | After (this PR's preview) |", + "| --- | --- | --- | --- |", ...rows, "", - "_Before = production · After = this PR's preview deploy._", + "_Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy._", ].join("\n"); - return { title: "Visual preview", body }; + return { title: "Visual preview", body, rawHtml: true }; } /** diff --git a/src/review/unified-comment.ts b/src/review/unified-comment.ts index c06ed26259..979a832afa 100644 --- a/src/review/unified-comment.ts +++ b/src/review/unified-comment.ts @@ -182,6 +182,9 @@ export interface UnifiedSignalRow { export interface UnifiedCollapsible { title: string; body: string; + /** When true the body is TRUSTED raw HTML and is NOT angle-escaped — used only by the visual before/after + * table (a table of `` clickable thumbnails the bridge builds from first-party shot URLs). */ + rawHtml?: boolean; } /** The host (gittensory) side: brand, readiness score, signals, sections, re-run, footer. */ @@ -214,6 +217,11 @@ const SIGNAL_ICON: Record = { ok: "✅", warn /** Derive the single unified status from reviewbot's decision/recs/CI + the host override. */ export function deriveUnifiedStatus(input: UnifiedReviewInput, ctx: UnifiedCommentContext = {}): UnifiedCommentStatus { if (ctx.statusOverride) return ctx.statusOverride; + // A failing CI is NEVER "safe to merge": a red CI downgrades any otherwise-ready/merge verdict to blocked (the + // disposition layer then closes it for a non-owner author / holds it open for the owner). This runs BEFORE the + // explicit-verdict switch so an optimistic gate "merge" can't render a green "safe to merge" headline over a + // red CI — the exact bug where a PR with a failing codecov/patch showed "Approved — safe to merge". + if (input.readiness?.ciState === "failed") return "blocked"; // An explicit gate verdict is authoritative — it already weighed the reviewers + guardrails. switch (input.decision) { case "merge": @@ -231,7 +239,6 @@ export function deriveUnifiedStatus(input: UnifiedReviewInput, ctx: UnifiedComme const recs = input.recommendations ?? []; const hasConsensusBlocker = input.consensusBlocker ?? (input.blockers ?? []).length > 0; if (recs.includes("close") || hasConsensusBlocker) return "blocked"; - if (input.readiness?.ciState === "failed") return "held"; if (recs.length === 0) return "advisory"; if ((input.failedCount ?? 0) > 0 || recs.some((r) => r !== "merge")) return "held"; return "ready"; @@ -361,6 +368,13 @@ function details(title: string, body: string, sub?: string): string { return `
${safeTitle}${safeSub}\n\n${escapePublicHtmlAngles(body)}\n
`; } +/** Like details(), but the body is TRUSTED raw HTML and is NOT angle-escaped. Used only for the visual + * before/after table, whose body is built solely from first-party minted shot URLs + route paths (see + * buildBeforeAfterCollapsible). The title is still escaped. */ +function detailsRaw(title: string, body: string): string { + return `
${escapePublicHtmlAngles(title)}\n\n${body}\n
`; +} + /** Wrap the assembled body in a GitHub alert blockquote — this is the full-comment colored sidebar. */ function asAlert(alert: string, inner: string): string { const quoted = inner @@ -406,10 +420,15 @@ export function renderUnifiedReviewComment(input: UnifiedReviewInput, ctx: Unifi const nits = dedupeLines(input.nits ?? []); if (nits.length) blocks.push(details("Nits", bullets(nits), `${nits.length} non-blocking`)); for (const c of ctx.extraCollapsibles ?? []) { - if (c.body.trim()) blocks.push(details(c.title, c.body.trim())); + if (c.body.trim()) blocks.push(c.rawHtml ? detailsRaw(c.title, c.body.trim()) : details(c.title, c.body.trim())); } if (ctx.reRunLabel) blocks.push(`- [ ] ${ctx.reRunLabel}`); + // Color-coded status legend (key) — a quiet footer mapping each headline color/icon to its meaning, so a + // reader can tell at a glance what "this PR's status" means. Squares are the SAME ones used in the headline. + blocks.push( + `${STATUS_META.ready.square} Safe / merged · ${STATUS_META.advisory.square} Advisory · ${STATUS_META.held.square} Held for review · ${STATUS_META.blocked.square} Blocked / closed`, + ); if (ctx.footerMarkdown?.trim()) blocks.push(`---\n${ctx.footerMarkdown.trim()}`); return asAlert(meta.alert, blocks.join("\n\n")); diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index 7b330c46f0..888cb6a671 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -62,6 +62,17 @@ export type AgentActionPlanInput = { // accumulator like automation/readme-refresh, or dependabot/renovate). These are NEVER auto-closed — a noise // heuristic (duplicate/slop) must not kill a recurring maintainer-managed PR. They may still auto-merge. authorIsAutomationBot: boolean; + // Live CI aggregate over ALL of the PR's checks — required OR not, including non-required ones like + // codecov/patch and every commit-status (reviewbot parity). "passed" = every check completed and none + // failed; "failed" = at least one check failed; "pending" = at least one check still running; "unverified" + // = no checks reported (or CI can't be verified, e.g. a fork PR whose workflows await approval). The + // disposition layer NEVER approves/merges unless "passed", CLOSES a non-owner PR on "failed" (citing the + // failing checks) / HOLDS the owner's, and DEFERS every action while "pending" (settle-before-decide — the + // check-completion webhook re-runs this planner once CI settles). + ciState: "passed" | "failed" | "pending" | "unverified"; + // The names of the failing checks, surfaced in the close/request-changes reason so the contributor knows + // WHY (e.g. "codecov/patch"). Empty unless ciState === "failed". + failingCheckNames?: string[] | undefined; pr: { mergeableState?: string | null | undefined; reviewDecision?: string | null | undefined; @@ -92,6 +103,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne const slopGateMinScore = input.slopGateMinScore ?? DEFAULT_SLOP_GATE_MIN_SCORE; // Branch-protection-aware: required approvals are satisfied when the repo asks for none, or GitHub already // resolved the PR's reviews to APPROVED. + const failingCheckNames = input.failingCheckNames ?? []; const approvalsSatisfied = autoMaintain.requireApprovals === 0 || input.pr.reviewDecision === "APPROVED"; const level = (actionClass: AgentActionClass) => resolveAutonomy(input.autonomy, actionClass); const acting = (actionClass: AgentActionClass) => isActingAutonomyLevel(level(actionClass)); @@ -100,59 +112,83 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne // App/infra-neutral verdicts (not evaluated yet) never drive an action. if (input.conclusion === "neutral" || input.conclusion === "skipped") return actions; + // CI state over ALL of the PR's checks (required OR not — codecov/patch included) — reviewbot's ci_red + // parity. A red CI is NEVER approved/merged and is itself a close-worthy signal (non-owner); while CI is + // still running we take NO action and wait for the check-completion webhook to re-run this planner. + const ciPassed = input.ciState === "passed"; + const ciFailed = input.ciState === "failed"; + // Settle-before-decide: never approve / merge / close on a half-finished CI run. + if (input.ciState === "pending") return actions; + const blocking = isBlocking(input.conclusion); - const passing = input.conclusion === "success"; + const gatePassing = input.conclusion === "success"; // A changed path matching a hard guardrail forces manual review: suppress the irreversible dispositions // (merge / close) AND the auto-approve that could later satisfy a merge. label + request_changes still run. const guardrailHit = changedPathsHittingGuardrail(input.changedPaths, input.hardGuardrailGlobs).length > 0; - - // 1) label — reflect the verdict bucket. After the neutral/skipped return above, a non-blocking verdict is - // necessarily `success`. A passing PR that hit a hard guardrail is NOT auto-merge-ready (the irreversible - // dispositions below are all suppressed for it) — labeling it `ready-to-merge` would promise an auto-merge - // that never happens, so it gets `needs-human-review` instead. Idempotent: skip if the PR already carries - // the chosen label. + // Auto-merge-ready ONLY when the gate passes AND CI is green AND no guarded path is touched. A red, pending, + // or unverified CI is never approved/merged. + const readyToMerge = gatePassing && ciPassed && !guardrailHit; + const ciReason = ciFailed ? `CI is failing${failingCheckNames.length ? ` (${failingCheckNames.join(", ")})` : ""}` : ""; + + // 1) label — a blocking gate OR a red CI → changes-requested. A gate-passing PR that is not yet + // auto-mergeable (guarded path, or CI not green/unverified) → needs-human-review (labeling it + // `ready-to-merge` would promise an auto-merge that never happens). Only a gate-passing, CI-green, + // non-guarded PR gets `ready-to-merge`. Idempotent: skip if the PR already carries the chosen label. if (acting("label")) { - const label = blocking ? AGENT_LABEL_CHANGES : guardrailHit ? AGENT_LABEL_NEEDS_REVIEW : AGENT_LABEL_READY; - const reason = !blocking && guardrailHit ? `verdict=${input.conclusion}; guarded path forces human review` : `verdict=${input.conclusion}`; + const label = blocking || ciFailed ? AGENT_LABEL_CHANGES : readyToMerge ? AGENT_LABEL_READY : AGENT_LABEL_NEEDS_REVIEW; + const reason = ciFailed + ? `verdict=${input.conclusion}; ${ciReason}` + : !blocking && guardrailHit + ? `verdict=${input.conclusion}; guarded path forces human review` + : !blocking && !ciPassed + ? `verdict=${input.conclusion}; CI not green yet — held for human` + : `verdict=${input.conclusion}`; if (!hasLabel(input.pr.labels, label)) { actions.push({ actionClass: "label", requiresApproval: approval("label"), reason, label }); } } - // 2) review — approve XOR request-changes, and never re-post the same state. - if (blocking && acting("request_changes") && input.pr.reviewDecision !== "CHANGES_REQUESTED") { - const summary = input.blockerTitles.length ? input.blockerTitles.map((title) => `- ${title}`).join("\n") : "- The Gittensory Gate is not satisfied."; + // 2) review — approve XOR request-changes, never re-post the same state. A red CI forces request-changes + // (citing the failing checks) and is NEVER approved; approve fires only when the gate passes AND CI is green + // AND no guarded path is touched. + if ((blocking || ciFailed) && acting("request_changes") && input.pr.reviewDecision !== "CHANGES_REQUESTED") { + const lines = ciFailed ? [ciReason, ...input.blockerTitles] : [...input.blockerTitles]; + const summary = lines.length ? lines.map((line) => `- ${line}`).join("\n") : "- The Gittensory Gate is not satisfied."; + const reason = ciFailed ? `CI failing${input.blockerTitles.length ? ` + ${input.blockerTitles.length} blocker(s)` : ""}` : `${input.blockerTitles.length || 1} blocker(s)`; actions.push({ actionClass: "request_changes", requiresApproval: approval("request_changes"), - reason: `${input.blockerTitles.length || 1} blocker(s)`, - reviewBody: `Gittensory requests changes — the gate is not yet satisfied:\n\n${summary}`, + reason, + reviewBody: `Gittensory requests changes — ${ciFailed ? "CI is not green" : "the gate is not yet satisfied"}:\n\n${summary}`, }); - } else if (passing && acting("approve") && !guardrailHit && input.pr.reviewDecision !== "APPROVED") { + } else if (readyToMerge && acting("approve") && input.pr.reviewDecision !== "APPROVED") { actions.push({ actionClass: "approve", requiresApproval: approval("approve"), - reason: "gate passed", - reviewBody: "Gittensory approves — the gate is satisfied.", + reason: "gate passed, CI green", + reviewBody: "Gittensory approves — the gate is satisfied and CI is green.", }); } - // 3) disposition — merge a clean, approved, passing PR; otherwise close clear noise. Mutually exclusive. + // 3) disposition — merge a clean, approved, CI-green PR; otherwise close clear noise OR a red-CI PR (citing + // the failing checks). Owner + maintainer-automation PRs are NEVER closed (a red-CI owner PR is held via the + // request_changes above, left open for the maintainer). Mutually exclusive with merge. const mergeableClean = input.pr.mergeableState === "clean"; - const canMerge = passing && acting("merge") && mergeableClean && approvalsSatisfied && !guardrailHit; + const canMerge = readyToMerge && acting("merge") && mergeableClean && approvalsSatisfied; if (canMerge) { actions.push({ actionClass: "merge", requiresApproval: approval("merge"), - reason: `gate passed, mergeable, ${autoMaintain.requireApprovals} approval(s) satisfied`, + reason: `gate passed, CI green, mergeable, ${autoMaintain.requireApprovals} approval(s) satisfied`, mergeMethod: autoMaintain.mergeMethod, }); - } else if (acting("close") && !passing && !guardrailHit && !input.authorIsOwner && !input.authorIsAutomationBot) { - const noiseReasons: string[] = []; - if (input.pr.slopRisk != null && input.pr.slopRisk >= slopGateMinScore) noiseReasons.push(`slop score ${input.pr.slopRisk} ≥ ${slopGateMinScore}`); - if ((input.pr.linkedDuplicateCount ?? 0) > 0) noiseReasons.push("duplicate of another open PR"); - if (noiseReasons.length > 0) { - actions.push({ actionClass: "close", requiresApproval: approval("close"), reason: noiseReasons.join("; "), closeComment: closeMessage(noiseReasons) }); + } else if (acting("close") && (ciFailed || !gatePassing) && !guardrailHit && !input.authorIsOwner && !input.authorIsAutomationBot) { + const closeReasons: string[] = []; + if (ciFailed) closeReasons.push(ciReason); + if (input.pr.slopRisk != null && input.pr.slopRisk >= slopGateMinScore) closeReasons.push(`slop score ${input.pr.slopRisk} ≥ ${slopGateMinScore}`); + if ((input.pr.linkedDuplicateCount ?? 0) > 0) closeReasons.push("duplicate of another open PR"); + if (closeReasons.length > 0) { + actions.push({ actionClass: "close", requiresApproval: approval("close"), reason: closeReasons.join("; "), closeComment: closeMessage(closeReasons) }); } } diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index 2286f24629..7b38770301 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -12,6 +12,7 @@ function input(overrides: Partial & { conclusion: GateChec hardGuardrailGlobs: [], authorIsOwner: false, authorIsAutomationBot: false, + ciState: "passed", pr: { labels: [] }, ...overrides, }; @@ -77,11 +78,11 @@ describe("planAgentMaintenanceActions (#778)", () => { it("applies conservative defaults when autoMaintain / slopGateMinScore are omitted", () => { // no autoMaintain → requireApprovals defaults to 1 → a clean passing PR without APPROVED does NOT merge - expect(classes(planAgentMaintenanceActions({ conclusion: "success", blockerTitles: [], autonomy: { merge: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, authorIsAutomationBot: false, pr: { labels: [], mergeableState: "clean" } }))).not.toContain("merge"); + expect(classes(planAgentMaintenanceActions({ conclusion: "success", blockerTitles: [], autonomy: { merge: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, authorIsAutomationBot: false, ciState: "passed", pr: { labels: [], mergeableState: "clean" } }))).not.toContain("merge"); // no slopGateMinScore → defaults to 60 → slopRisk 70 counts as noise and closes - expect(classes(planAgentMaintenanceActions({ conclusion: "failure", blockerTitles: ["x"], autonomy: { close: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, authorIsAutomationBot: false, pr: { labels: [], slopRisk: 70 } }))).toContain("close"); + expect(classes(planAgentMaintenanceActions({ conclusion: "failure", blockerTitles: ["x"], autonomy: { close: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, authorIsAutomationBot: false, ciState: "passed", pr: { labels: [], slopRisk: 70 } }))).toContain("close"); // ...and slopRisk 50 is below the default → no close - expect(classes(planAgentMaintenanceActions({ conclusion: "failure", blockerTitles: ["x"], autonomy: { close: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, authorIsAutomationBot: false, pr: { labels: [], slopRisk: 50 } }))).not.toContain("close"); + expect(classes(planAgentMaintenanceActions({ conclusion: "failure", blockerTitles: ["x"], autonomy: { close: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, authorIsAutomationBot: false, ciState: "passed", pr: { labels: [], slopRisk: 50 } }))).not.toContain("close"); }); it("closes clear noise (high slop or duplicate) on a non-passing verdict, and never closes a passing PR", () => { @@ -177,7 +178,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("DOES auto-close the same noisy PR when the author is not the owner", () => { - const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"], authorIsOwner: false, authorIsAutomationBot: false, pr: { labels: [], slopRisk: 95 } }))); + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"], authorIsOwner: false, authorIsAutomationBot: false, ciState: "passed", pr: { labels: [], slopRisk: 95 } }))); expect(plan).toContain("close"); }); @@ -198,6 +199,57 @@ describe("planAgentMaintenanceActions (#778)", () => { expect(plan).toContain("merge"); }); }); + + describe("CI policy: a red CI is never approved/merged — closed (non-owner) / held (owner); pending defers", () => { + it("does NOT approve or merge a PR whose CI is failing, even when the gate passes and it is clean+approved", () => { + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { approve: "auto", merge: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch"], pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))); + expect(plan).not.toContain("approve"); + expect(plan).not.toContain("merge"); + }); + + it("closes a red-CI non-owner PR and cites the failing checks (even when the gate itself passes)", () => { + const close = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch"], pr: { labels: [] } })).find((a) => a.actionClass === "close"); + expect(close).toBeTruthy(); + expect(close?.reason).toContain("CI is failing"); + expect(close?.reason).toContain("codecov/patch"); + }); + + it("NEVER closes the owner's red-CI PR — it is held (request_changes), left open", () => { + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto", request_changes: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch"], authorIsOwner: true, pr: { labels: [] } }))); + expect(plan).not.toContain("close"); + expect(plan).toContain("request_changes"); + }); + + it("requests changes (never approves) on a red CI and cites the failing check", () => { + const rc = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { approve: "auto", request_changes: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch", "build"], pr: { labels: [] } })).find((a) => a.actionClass === "request_changes"); + expect(rc?.reviewBody).toContain("codecov/patch"); + expect(rc?.reviewBody).toContain("CI is not green"); + }); + + it("labels a red-CI PR changes-requested (not ready-to-merge)", () => { + const label = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto" }, ciState: "failed", failingCheckNames: ["codecov/patch"], pr: { labels: [] } })).find((a) => a.actionClass === "label"); + expect(label?.label).toBe(AGENT_LABEL_CHANGES); + }); + + it("DEFERS every action while CI is still pending (settle-before-decide)", () => { + expect(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto", approve: "auto", merge: "auto", close: "auto" }, ciState: "pending", pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))).toEqual([]); + }); + + it("HOLDS (needs-human-review, no merge/close/approve) a gate-passing PR whose CI is unverified", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto", approve: "auto", merge: "auto", close: "auto" }, ciState: "unverified", pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })); + const cls = classes(plan); + expect(cls).not.toContain("merge"); + expect(cls).not.toContain("close"); + expect(cls).not.toContain("approve"); + expect(plan.find((a) => a.actionClass === "label")?.label).toBe(AGENT_LABEL_NEEDS_REVIEW); + }); + + it("merges the same clean+approved PR on green CI but NOT on red CI", () => { + const base = { conclusion: "success" as const, autonomy: { merge: "auto" as const }, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }; + expect(classes(planAgentMaintenanceActions(input({ ...base, ciState: "passed" })))).toContain("merge"); + expect(classes(planAgentMaintenanceActions(input({ ...base, ciState: "failed" })))).not.toContain("merge"); + }); + }); }); describe("isProtectedAutomationAuthor", () => { diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 22aad01416..f836697952 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -2963,7 +2963,12 @@ describe("queue processors", () => { filesFetched += 1; return Response.json([{ filename: "src/cache.ts", additions: 5, deletions: 1, status: "modified", patch: "@@\n+const x = 1;" }]); } + // The review path now reads the LIVE CI aggregate (check-runs + commit-statuses). codecov/patch is a + // classic COMMIT-STATUS (not a check-run), so it comes from the combined-status endpoint; the check-runs + // list stays empty (it must, so the gate's own check-run upsert finds no pre-existing run to PATCH). if (url.includes("/check-runs") && method === "GET") return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/") && url.includes("/status")) + return Response.json({ state: "failure", statuses: [{ context: "codecov/patch", state: "failure", description: "60% of diff hit (target 97%)", target_url: "https://codecov.io/report" }] }); if (url.includes("/check-runs") && method === "POST") return Response.json({ id: 902 }, { status: 201 }); if (url.includes("/check-runs/902") && method === "PATCH") return Response.json({ id: 902 }); if (url.includes("/issues/3/comments") && method === "GET") return Response.json([]); diff --git a/test/unit/unified-comment.test.ts b/test/unit/unified-comment.test.ts index e6b91a390e..c87bdf1b9d 100644 --- a/test/unit/unified-comment.test.ts +++ b/test/unit/unified-comment.test.ts @@ -31,10 +31,17 @@ describe("deriveUnifiedStatus", () => { expect(deriveUnifiedStatus({ ...base, recommendations: [] })).toBe("advisory"); }); - it("held for manual / request_changes / failing CI", () => { + it("held for manual / request_changes", () => { expect(deriveUnifiedStatus({ ...base, decision: "manual" })).toBe("held"); expect(deriveUnifiedStatus({ ...base, recommendations: ["request_changes"] })).toBe("held"); - expect(deriveUnifiedStatus({ ...base, readiness: { ciState: "failed" } })).toBe("held"); + }); + + it("a failing CI is BLOCKED (never safe-to-merge) and overrides an optimistic merge verdict", () => { + // A red CI must never render "safe to merge". It downgrades even an explicit `merge` verdict to blocked. + expect(deriveUnifiedStatus({ ...base, readiness: { ciState: "failed" } })).toBe("blocked"); + expect(deriveUnifiedStatus({ ...base, decision: "merge", readiness: { ciState: "failed" } })).toBe("blocked"); + // green CI + merge verdict still renders ready. + expect(deriveUnifiedStatus({ ...base, decision: "merge", readiness: { ciState: "passed" } })).toBe("ready"); }); it("blocked for a close verdict or consensus blockers", () => { diff --git a/test/unit/visual-collapsible.test.ts b/test/unit/visual-collapsible.test.ts index b4b940f496..006086bb1f 100644 --- a/test/unit/visual-collapsible.test.ts +++ b/test/unit/visual-collapsible.test.ts @@ -30,21 +30,24 @@ const routes: CaptureRoute[] = [ ]; describe("buildBeforeAfterCollapsible", () => { - it("renders a 'Visual preview' table of markdown image cells pointing at the public shot URLs", () => { + it("renders a 'Visual preview' table of clickable-thumbnail cells pointing at the public shot URLs", () => { const c = buildBeforeAfterCollapsible(routes); expect(c).not.toBeNull(); expect(c?.title).toBe("Visual preview"); - expect(c?.body).toContain("| Route | Before (production) | After (this PR's preview) |"); + // Trusted raw HTML so the / survive (not angle-escaped). + expect(c?.rawHtml).toBe(true); + expect(c?.body).toContain("| Route | Viewport | Before (production) | After (this PR's preview) |"); expect(c?.body).toContain("`/app/analytics`"); - // Markdown image syntax (not raw ) so it survives the renderer's HTML-angle escaping. - expect(c?.body).toContain("![preview](https://api.example.dev/gittensory/shot?key=gittensory/shots/abc.png)"); - expect(c?.body).toContain("![preview](https://api.example.dev/gittensory/shot?key=gittensory/shots/def.png)"); - expect(c?.body).not.toContain(" wrapped in an to the SAME full-resolution shot. + expect(c?.body).toContain(' { const c = buildBeforeAfterCollapsible([{ path: "/", afterUrl: "https://api.example.dev/gittensory/shot?key=gittensory/shots/x.png" }]); - expect(c?.body).toContain("| `/` | — | ![preview]("); + expect(c?.body).toContain("| `/` | desktop | — | { diff --git a/wrangler.jsonc b/wrangler.jsonc index 2ac3c43cb0..d372af35db 100644 --- a/wrangler.jsonc +++ b/wrangler.jsonc @@ -56,7 +56,7 @@ // Convergence (visual capture): capture a before/after screenshot for PRs touching WEB-VISIBLE files // (frontend pages / public OG images). Needs the BROWSER + REVIEW_AUDIT bindings; runs only when this is // ON AND the repo is in GITTENSORY_REVIEW_REPOS. DEFAULT OFF — flag-OFF captures nothing (byte-identical). - "GITTENSORY_REVIEW_SCREENSHOTS": "false", + "GITTENSORY_REVIEW_SCREENSHOTS": "true", // Convergence (grounding): ground the AI reviewer prompt with the PR's finished CI status + the full // post-change content of the changed files, so a non-frontier model verifies claims instead of guessing. // Default OFF — flag-OFF keeps the reviewer prompt byte-identical and makes no extra GitHub fetch.