JSONbored · loopover-orb · Jul 28, 2026 · Jul 28, 2026
@@ -3109,24 +3109,43 @@ export async function countRecentAuditEventsForActorInRepoWithTargetSuffix(
   return row.count;
 }
 
-// #orb-stale-recheck-priority: the THREE self-resolving "denied" detail strings the executor's live staleness
-// rechecks can produce (agent-action-executor.ts's "8) Live ... re-verification" block -- duplicateStaleReason /
-// mergeableStaleReason / threadStaleReason, each suffixed with " — action not executed" by that block's shared
-// `audit("denied", ...)` call). Deliberately NOT imported from agent-action-executor.ts: that module imports
+// #orb-stale-recheck-priority: the self-resolving "denied" detail strings the executor can produce for a PR
+// that NOTHING will otherwise re-drive. Three come from its live staleness rechecks (agent-action-executor.ts's
+// "8) Live ... re-verification" block -- duplicateStaleReason / mergeableStaleReason / threadStaleReason); the
+// fourth is the merge-train wait. All are suffixed with " — action not executed" by the shared
+// `audit("denied", ...)` call. Deliberately NOT imported from agent-action-executor.ts: that module imports
 // FROM db/repositories.ts (installation tokens, PR records, ...), so importing back would create a real
 // module-load cycle -- same hazard agent-actions.ts's CONCRETE_EVIDENCE_BLOCKER_CODES comment documents for the
-// identical reason. A source-text parity test guards these three literals against producer-side drift instead.
-// Deliberately EXCLUDES a CI-staleness denial (ciStaleReason): CI flipping already re-triggers a fresh
-// evaluation via the check-run/status webhook that changed it, so it doesn't share the other three's "no
-// webhook ever reaches this PR" gap.
+// identical reason. A source-text parity test guards these literals against producer-side drift instead.
+//
+// #9483: the merge-train wait was originally excluded here as "durable, externally-actioned", on the theory
+// that the blocker merging would wake the waiter via maybeEnqueueSiblingRegateForMergedPr. That classification
+// was wrong, and the blocker MERGING is the only case it holds for. A blocker can also leave the train by
+// being CLOSED UNMERGED (that wake returns early on `!mergedAt`), by AGING past MERGE_TRAIN_MAX_WAIT_MS, or by
+// gaining the manual-review label (#9039 eviction) -- and the latter two are consulted only at the waiter's own
+// evaluation time, which nothing schedules. In every one of those three the waiter sat open indefinitely with
+// no signal, which under one-shot review reads to the contributor as a silent rejection. It is in fact the
+// PUREST instance of the gap this whole mechanism exists for: resolving the blocker fires a webhook about the
+// BLOCKER, never about the PR waiting on it.
+//
+// Bounded by construction, so this cannot become a retry storm: a matching PR inherits
+// surfaceRepairPriorityPullNumbers' existing REGATE_REPAIR_MAX_ATTEMPTS_PER_PR budget, and that budget's
+// lookback window is ROLLING and the same 24h as MERGE_TRAIN_MAX_WAIT_MS -- so a PR blocked long enough for its
+// blocker to age out of the train has also had its earliest attempts age out, and gets fresh looks exactly when
+// the age-out makes them useful.
+//
+// Still deliberately EXCLUDES a CI-staleness denial (ciStaleReason): CI flipping already re-triggers a fresh
+// evaluation via the check-run/status webhook that changed it, so it doesn't share the others' "no webhook ever
+// reaches this PR" gap.
 const STALE_RECHECK_DENIAL_DETAIL_PATTERN =
-  /^(duplicate-cluster winner #\d+ is no longer open|the base-branch conflict that justified this close has since cleared|the review thread\(s\) that justified this close are now all resolved) — action not executed$/;
+  /^(duplicate-cluster winner #\d+ is no longer open|the base-branch conflict that justified this close has since cleared|the review thread\(s\) that justified this close are now all resolved|merge train: waiting for older mergeable sibling #\d+) — action not executed$/;
 
 /**
  * PR numbers within `repoFullName` whose most recent `agent.action.close`/`agent.action.merge` attempt was
- * DENIED by one of the executor's live staleness rechecks (duplicate-cluster winner / base-conflict / review-
- * thread) within `sinceIso`, rather than by a durable, externally-actioned reason (a manual-review label, a
- * merge-train wait, contributor-cap contention, ...). Those rechecks exist precisely because the fact that
+ * DENIED by a self-resolving reason (duplicate-cluster winner / base-conflict / review-thread staleness, or a
+ * merge-train wait -- see the pattern's own comment for why the last one belongs here) within `sinceIso`,
+ * rather than by a durable, externally-actioned reason (a manual-review label, contributor-cap contention,
+ * ...). Those rechecks exist precisely because the fact that
  * justified the close/merge can flip WITHOUT a webhook ever notifying THIS pr -- a duplicate-cluster sibling
  * merging fires a webhook about the SIBLING, not this PR, so nothing naturally re-triggers a look here. Callers
  * (surfaceRepairPriorityPullNumbers) fold this into the SAME priority set the outage-repair path already uses,

@@ -11,6 +11,46 @@ const COALESCABLE_PULL_REQUEST_ACTIONS = new Set([
   "ready_for_review",
 ]);
 
+/**
+ * #9479: `synchronize` -- a PUSH -- gets its own PR-scoped coalesce key and a trailing quiet window; every other
+ * action above keeps the head-SHA-scoped key unchanged.
+ *
+ * Every dedup layer in the pipeline was keyed on the head SHA (this key, and the AI-review lock's
+ * `...@${headSha}:${mode}`), which is exactly the wrong key for a force-push storm: each push MINTS a new SHA, so
+ * five amend-and-repush cycles in a minute looked like five unrelated events and bought five full prologues
+ * (file list, up to 96k chars of grounding fetch, RAG + impact-map embeddings, an enrichment POST) and five LLM
+ * calls. Four of those five review heads no longer exist by the time their review lands. `skipStaleReviewOutput`
+ * suppresses the stale COMMENT, but only after the spend; the `review burst` ops alert reports the storm without
+ * throttling it.
+ *
+ * Dropping the SHA makes consecutive pushes collide, and the queue's coalesce keeps the LATEST payload while
+ * extending `run_after` (`GREATEST(run_after, ...)` in pg-queue/sqlite-queue's enqueue) -- so a burst converges to
+ * ONE job, at the last push plus the window, reviewing the head that actually survived.
+ *
+ * Deliberately a SEPARATE key from `pr-refresh` rather than simply dropping the SHA there. A shared PR-scoped key
+ * would let a push swallow a still-pending `opened`/`ready_for_review` in the same window, since coalescing
+ * overwrites the payload and only the last action survives -- trading a spend bug for a lost-lifecycle-event bug.
+ */
+const PUSH_COALESCE_ACTION = "synchronize";
+
+/** How long to hold a push before reviewing it, so a re-push lands inside the same window and replaces it. Sized
+ *  to match CI_COALESCE_WINDOW_SECONDS (processors.ts), the pipeline's existing burst window, and to comfortably
+ *  cover a human amend-and-force-push cycle without adding meaningful latency to an ordinary single push -- which
+ *  then waits on CI anyway. */
+export const PUSH_COALESCE_QUIET_WINDOW_SECONDS = 45;
+
+/**
+ * The delay to enqueue this delivery with. Non-zero ONLY for a push, whose key above is built to coalesce.
+ *
+ * On a queue that coalesces (self-host pg/sqlite, which is where the ORB runs) this is what CREATES the window:
+ * without a delay the first push is claimed immediately and later pushes find no pending row to merge into. On
+ * Cloudflare Queues, which has no job_key coalescing, it is a plain 45s deferral of push-triggered reviews and
+ * nothing more -- correct, just not a saving.
+ */
+export function githubWebhookCoalesceDelaySeconds(eventName: string, payload: GitHubWebhookPayload): number {
+  return eventName === "pull_request" && payload.action === PUSH_COALESCE_ACTION ? PUSH_COALESCE_QUIET_WINDOW_SECONDS : 0;
+}
+
 // #selfhost-backlog-convergence: every "labeled"/"unlabeled" delivery re-syncs the PR row
 // (upsertPullRequestFromGitHub) regardless of which specific label changed -- shouldProcessPullRequestPublicSurface
 // (processors.ts) additionally runs the public-surface pipeline itself, but only when the changed label is a
@@ -61,10 +101,11 @@ export function githubWebhookCoalesceKey(
     const pr =
       normalizedNumber(payload.pull_request?.number) ??
       normalizedNumber((payload as { number?: unknown }).number);
+    if (pr === null) return null;
+    // #9479: a push is keyed on the PR alone, so the NEXT push collapses into it. See PUSH_COALESCE_ACTION.
+    if (action === PUSH_COALESCE_ACTION) return `github-webhook:pr-push:${repo}#${pr}`;
     const headSha = normalizedSha(payload.pull_request?.head?.sha);
-    return pr !== null
-      ? `github-webhook:pr-refresh:${repo}#${pr}${headSha ? `@${headSha}` : ""}`
-      : null;
+    return `github-webhook:pr-refresh:${repo}#${pr}${headSha ? `@${headSha}` : ""}`;
   }
   if (eventName === "pull_request" && COALESCABLE_PULL_REQUEST_LABEL_ACTIONS.has(action)) {
     const pr =

@@ -8,6 +8,7 @@ import { isSelfHostedReviewRuntime } from "../selfhost/review-runtime";
 import { incr } from "../selfhost/metrics";
 import { getSelfHostRequestTraceParent } from "../selfhost/trace-context";
 import { isNonActionableWebhookNoise } from "./self-authored";
+import { githubWebhookCoalesceDelaySeconds } from "./webhook-coalesce";
 
 const DEFAULT_MAX_WEBHOOK_BODY_BYTES = 1024 * 1024;
 // #9054: how long a 'queued'/'superseded' webhook_events row may sit unprocessed before a redelivery of the
@@ -225,7 +226,11 @@ export async function enqueueWebhookByEnv(env: Env, deliveryId: string, eventNam
   try {
     // Send to the dedicated WEBHOOKS lane (not the shared JOBS queue) so a maintenance burst on JOBS can never
     // starve real GitHub events into the DLQ. (#audit-webhook-queue)
-    await env.WEBHOOKS.send(message);
+    // #9479: a push is deferred by a short quiet window so a force-push storm coalesces into one review of the
+    // head that survives, instead of buying a full prologue + LLM call per intermediate SHA. Zero for every
+    // other event, so this is a no-op for everything but `pull_request`/`synchronize` -- see
+    // githubWebhookCoalesceDelaySeconds.
+    await env.WEBHOOKS.send(message, { delaySeconds: githubWebhookCoalesceDelaySeconds(eventName, payload) });
   } catch (error) {
     // Enqueue failed: flip the event to "error" so the dedup guard above lets GitHub redeliver / the next pull
     // re-deliver, instead of treating the webhook as handled (#786). Also covers the deploy-ordering case where

@@ -12540,7 +12540,7 @@ async function maybePublishPrPublicSurface(
           // ordinary "nothing found" capture would, with no separate code path to maintain.
           const capture =
             reviewVisualConfig.enabled === false
-              ? { routes: [], interactions: [], previewPending: false }
+              ? { routes: [], interactions: [], previewPending: false, renderFailed: false }
               : await buildCapture(env, token, captureTarget, visualFiles, githubRateLimitAdmissionKeyForInstallation(installationId), reviewVisualConfig, changedCssFiles);
           beforeAfter = capture.routes;
           interactionPreviews = capture.interactions;
@@ -12566,8 +12566,14 @@ async function maybePublishPrPublicSurface(
           // preview deploy isn't live yet (capture.previewPending). Schedule a delayed re-review to re-capture
           // the now-ready shot — bounded by `attempt` so a never-resolving preview can't loop (the deployment_status
           // webhook also refills it; this is the backstop when that event is missed/late).
+          //
+          // #9464: `capture.renderFailed` joins previewPending here as a SECOND blip class. captureShot
+          // swallows a renderer error per shot and returns a null PNG, so buildCapture used to return
+          // normally -- previewPending false, nothing thrown -- and neither the #9030 nor the #9207 guard
+          // fired. The maintenance pass then read "no evidence, no retry pending" and CLOSED the PR one-shot.
+          // A browserless outage now degrades to "we could not capture evidence, holding" instead.
           const previewPollAttempt = webhook.previewPollAttempt ?? 0;
-          if (capture.previewPending) {
+          if (capture.previewPending || capture.renderFailed) {
             await scheduleVisualCaptureRetry(env, {
               webhook,
               repoFullName,

@@ -89,6 +89,18 @@ export interface CaptureResult {
   routes: CaptureRoute[];
   interactions: CaptureInteractionRoute[];
   previewPending: boolean;
+  /**
+   * #9464: at least one shot in this capture failed because the RENDERER broke (see `CaptureShotResult.
+   * renderFailed`) -- browserless down, saturated, or timing out. Distinct from `previewPending`, which means
+   * the page we want to render does not exist yet.
+   *
+   * Both are "we could not obtain evidence", and the screenshot-table gate must defer its one-shot close for
+   * either. Before this, a renderer failure was swallowed per shot and `buildCapture` returned NORMALLY with
+   * `previewPending: false` and no throw, so the #9030/#9207 blip guards -- which trigger only on
+   * previewPending or a thrown error -- never fired, and the gate closed legitimate visual PRs during a
+   * browserless outage.
+   */
+  renderFailed: boolean;
 }
 
 /** True when `url` is a persisted rendered shot. `capturePage` can also return an on-demand `?url=`
@@ -342,6 +354,10 @@ function resolveShotUrl(env: Env, key: string): string {
  * the cell shows a dash. Reuses an identical cached fingerprint (a deployment_status re-run filling "after"
  * cells would otherwise re-render the same screenshot — Browser Rendering is the costliest binding).
  */
+/** What `capturePage` and its no-preview sibling `resolveFallbackAfterShot` both resolve to. `renderFailed`
+ *  is optional because only the live-render path can observe it -- see {@link CaptureResult.renderFailed}. */
+type CapturedShot = { url?: string | undefined; thumbUrl?: string | undefined; png?: Uint8Array | undefined; renderFailed?: boolean | undefined };
+
 async function capturePage(
   env: Env,
   target: CaptureTarget,
@@ -361,8 +377,12 @@ async function capturePage(
   // theming ignores prefers-color-scheme (see shot.ts's CaptureShotOptions.theme doc). Only takes effect
   // together with `theme`; undefined (every pre-#4109 caller) ⇒ byte-identical to today.
   themeStorageKey?: string | undefined,
-): Promise<{ url?: string | undefined; thumbUrl?: string | undefined; png?: Uint8Array | undefined }> {
+): Promise<CapturedShot> {
   if (!page) return {};
+  // #9464: sticky for the whole call, because the only return that can carry it is the on-demand tail below --
+  // every earlier return is a definite outcome (cache hit, auth wall, a real PNG) that by construction cannot
+  // have come from a broken renderer.
+  let renderFailed = false;
   const shotBase = env.PUBLIC_API_ORIGIN; // this worker's public origin (serves /loopover/shot)
   // Carries the theme (#3678) and, when set, the storage key (#4109) so a LATER on-demand fetch of this
   // exact URL (e.g. a failed/never-persisted render retried by GitHub's image proxy) still requests the
@@ -405,7 +425,14 @@ async function capturePage(
       const bytes = await new Response(cached.body).arrayBuffer().then((buf) => new Uint8Array(buf)).catch(() => undefined);
       return { url, ...(thumbUrl ? { thumbUrl } : {}), ...(bytes ? { png: bytes } : {}) };
     }
-    const { png, authWalled } = await captureShot(env, page, viewport, theme ? { theme, ...(themeStorageKey ? { themeStorageKey } : {}) } : {}).catch(() => ({ png: null, authWalled: false }));
+    // The outer `.catch` reports renderFailed too: captureShot swallows its own renderer errors, so anything
+    // that still escapes it (a DNS-guard or token-mint throw) is likewise an outcome we could not determine.
+    const { png, authWalled, renderFailed: shotRenderFailed } = await captureShot(env, page, viewport, theme ? { theme, ...(themeStorageKey ? { themeStorageKey } : {}) } : {}).catch(() => ({
+      png: null,
+      authWalled: false,
+      renderFailed: true,
+    }));
+    renderFailed = shotRenderFailed;
     // A protected route that redirected to a sign-in wall: show an honest "requires authentication"
     // placeholder rather than caching/serving a screenshot of the login screen.
     if (authWalled) {
@@ -430,7 +457,7 @@ async function capturePage(
       return { url, ...(thumbUrl ? { thumbUrl } : {}), ...(includeBytes ? { png } : {}) };
     }
   }
-  return { url: onDemand };
+  return { url: onDemand, renderFailed };
 }
 
 /** Resolve the "after" shot when there is no real preview page to render (#4112): if `review.visual.
@@ -446,7 +473,7 @@ async function resolveFallbackAfterShot(
   viewportName: "desktop" | "mobile",
   actionsFallbackEnabled: boolean,
   placeholder: string | undefined,
-): Promise<{ url?: string | undefined; thumbUrl?: string | undefined; png?: Uint8Array | undefined }> {
+): Promise<CapturedShot> {
   // #6324: never produces a thumbUrl (the actions_fallback artifact is stored as-is, no display downscale
   // applied) -- typed here purely so this function's return shape matches capturePage's, since buildCapture
   // uses both interchangeably for the "after" desktop slot.
@@ -800,6 +827,10 @@ export async function buildCapture(
   // `theme` iteration is undefined (the untagged default pass).
   const themeStorageKey = visualConfig?.themeStorageKey ? visualConfig.themeStorageKey : undefined;
   const captureRoutes: CaptureRoute[] = [];
+  // #9464: ORed across every shot of every route/viewport/theme. ONE failed render is enough -- the gate's
+  // question is "could we have missed evidence?", and a partial outage answers yes. When some routes DID
+  // render, hasSuccessfulBotCapture already satisfies the gate and this never gets consulted.
+  let renderFailed = false;
   for (const theme of themes) {
     for (const path of routes) {
       const beforePage = prodBase ? joinUrl(prodBase, path) : "";
@@ -815,6 +846,7 @@ export async function buildCapture(
           ? capturePage(env, target, afterPage, "after", "mobile", MOBILE_VIEWPORT, diffAvailable, theme, themeStorageKey)
           : resolveFallbackAfterShot(env, target, path, "mobile", actionsFallbackEnabled, afterPlaceholder),
       ]);
+      if ([beforeShot, beforeMobileShot, afterShot, afterMobileShot].some((shot) => shot.renderFailed === true)) renderFailed = true;
       // A diff needs BOTH sides' real bytes — a placeholder/dash slot (no preview yet, auth-walled, render
       // failure) has no `png`, so compareCapturedScreenshots degrades to null exactly like a missing shot does.
       const [desktopDiff, mobileDiff] = diffAvailable
@@ -926,5 +958,5 @@ export async function buildCapture(
     }
   }
 
-  return { routes: captureRoutes, interactions: interactionRoutes, previewPending };
+  return { routes: captureRoutes, interactions: interactionRoutes, previewPending, renderFailed };
 }