fix(review): stop redundant republishing for a draft fork PR's repeat CI-completion triggers (#6685)

JSONbored · JSONbored · commit 0d9c29f49dc6 · 2026-07-16T12:59:05.000-07:00
A draft fork PR's own CI produces one check_run.completed webhook per job (each arriving with an empty pull_requests[] payload, forcing the fork-resume fallback in webhook-coalesce.ts to re-run the whole evaluation pass), so a multi-job CI run republished the SAME comment/label surface once per completing job even though nothing about the PR changed between them. Confirmed live: 12 identical republishes of #6592 in 29 minutes -- the actual root cause of the ops_anomaly "review burst" false alarms that have been firing across the fleet for at least a day. The one-shot cadence guard already avoids wasting AI spend on these repeat passes (ai_slop_one_shot_skip, linked_issue_satisfaction_one_shot_skip, ai_review_auto_review_skipped all correctly fire), but nothing stopped the pipeline from proceeding to republish anyway. The only existing "skip republish if nothing changed" guard (canSkipCurrentSurface) is deliberately scoped to publicSurface: "off" repos, and requires gateEnabled -- which is false fleet-wide here since reviewCheckMode is disabled, so that guard never even engages for loopover/awesome-claude/metagraphed regardless of surface type. Add a narrower, independent skip specifically for the draft case: when autoReviewSkipReason is deterministically "review skipped (draft)" (not just reused from cache) and the head SHA matches what was last fully published, republishing is a provable no-op. Hoisted the reason into a try-block-independent variable (autoReviewSkipReasonForPublish) since the original is scoped inside the try block that ends before the publish site, mirroring the existing aiReviewExpected/aiReviewWasReused hoisting pattern. Closes #6685
diff --git a/src/queue/processors.ts b/src/queue/processors.ts
@@ -8297,6 +8297,9 @@ async function maybePublishPrPublicSurface(
   let aiReviewExpected = false;
   let aiReviewWasReused = false;
   let gateFinalized = false;
+  // #6685: hoisted the same way aiReviewExpected/aiReviewWasReused are above -- assigned inside the try block
+  // below, read at the draft-republish skip check past it (autoReviewSkipReason itself is try-block-scoped).
+  let autoReviewSkipReasonForPublish: string | null = null;
   const publishedOutputs: PublicSurfaceOutput[] = [];
   const failedOutputs: PublicSurfaceOutputFailure[] = [];
   const reviewedHeadSha = reviewedPullRequestHeadSha(pr.headSha, advisory.headSha);
@@ -8906,6 +8909,7 @@ async function maybePublishPrPublicSurface(
       addedLineCount: autoReviewAddedLineCount,
       changedFileCount: autoReviewChangedFileCount,
     }));
+    autoReviewSkipReasonForPublish = autoReviewSkipReason;
     // review.changed_files_summary (#1957) + review.effort_score (#1955): both deterministic, no-AI — resolve
     // them here, UNCONDITIONALLY, rather than inside the aiReviewWillRun-gated closure below. These sections
     // must still render whenever the manifest opts in even when the AI review itself is skipped this pass
@@ -10001,6 +10005,35 @@ async function maybePublishPrPublicSurface(
   if (!prelimHasPublicOutput) return finishPublicSurfacePublication();
   if (publicSurfaceSkipped || !official || !author)
     return finishPublicSurfacePublication();
+  // #6685 (review-burst): a draft fork PR's own CI produces one check_run.completed webhook per job (each
+  // arriving with an empty pull_requests[] payload, forcing the fork-resume fallback in webhook-coalesce.ts
+  // to re-run this whole pass), so a multi-job CI run on a draft PR republished the SAME surface once per
+  // completing job even though nothing about the PR changed between them -- confirmed live, 12 identical
+  // republishes of JSONbored/loopover#6592 in 29 minutes. autoReviewSkipReason === "review skipped (draft)"
+  // is a deterministic signal (from pr.isDraft + config, resolved before any AI/cache call) that this pass
+  // has nothing new to report from the review dimension; a matching lastPublishedSurfaceSha proves the head
+  // hasn't moved since the last full publish either. Together they're a provable no-op for the draft case
+  // specifically -- narrower than, and independent of, the check-run-only skip above (canSkipCurrentSurface
+  // requires gateEnabled, which is false fleet-wide here since reviewCheckMode is disabled), so this applies
+  // regardless of publicSurface/gateEnabled. !forceAiReview preserves an explicit maintainer re-trigger the
+  // same way that guard does.
+  if (
+    autoReviewSkipReasonForPublish === "review skipped (draft)" &&
+    !webhook.forceAiReview &&
+    advisory.headSha &&
+    advisory.headSha === pr.lastPublishedSurfaceSha
+  ) {
+    incr("loopover_public_surface_publish_skipped_current_total");
+    await recordAuditEvent(env, {
+      eventType: "github_app.public_surface_publish_skipped_current",
+      actor: author,
+      targetKey: `${repoFullName}#${pr.number}`,
+      outcome: "completed",
+      detail: "draft PR already current for this head; skipped republish",
+      metadata: { deliveryId: webhook.deliveryId, repoFullName, headSha: advisory.headSha },
+    }).catch(() => undefined);
+    return finishPublicSurfacePublication();
+  }
 
   const [github] = await Promise.all([
     fetchPublicContributorProfile(author, env),
diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts
@@ -6011,6 +6011,202 @@ describe("queue processors", () => {
       expect(reuseAudit?.n).toBe(2); // both later passes explicitly reused the durable cache
     });
 
+    it("REGRESSION (#6685): a draft PR's repeat fork-CI-completion triggers stop republishing once the head is already current", async () => {
+      const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
+      // reviewCheckMode: "disabled" reproduces the live incident exactly -- gateEnabled (shouldPublishReviewCheck
+      // && headSha) is false here, so the OLDER check-run-only skip guard (canSkipCurrentSurface, gated on
+      // gateEnabled) never engages; this fix must not depend on it.
+      await seedRegateChurnRepo(env, { publicSurface: "comment_and_label" });
+      await upsertRepoFocusManifest(env, "JSONbored/gittensory", { settings: { commentMode: "all_prs", publicSurface: "comment_and_label", checkRunMode: "off", reviewCheckMode: "disabled", aiReviewMode: "block" }, review: { auto_review: { skip_drafts: true, cadence: "continuous" } } });
+      await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 90, title: "Draft feature", state: "open", draft: true, user: { login: "contributor" }, head: { sha: "a90" }, labels: [], body: "Closes #1" } as never);
+      await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 90, status: "complete", reviewsSyncedAt: new Date().toISOString() });
+
+      const stickyComment: { current: { id: number; body: string } | null } = { current: null };
+      let commentPosts = 0;
+      let commentPatches = 0;
+      let labelPosts = 0;
+      vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
+        const url = input.toString();
+        const method = init?.method ?? "GET";
+        if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" });
+        if (url.includes("/pulls/90/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]);
+        if (url.endsWith("/pulls/90")) return Response.json({ number: 90, title: "Draft feature", state: "open", draft: true, user: { login: "contributor" }, head: { sha: "a90" }, labels: [], body: "Closes #1", mergeable_state: "clean" });
+        if (url.includes("/commits/a90/check-runs")) return Response.json({ total_count: 0, check_runs: [] });
+        if (url.includes("/commits/a90/status")) return Response.json({ state: "success", statuses: [] });
+        if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } });
+        if (url.includes("/issues/90/comments") && method === "GET") {
+          return Response.json(stickyComment.current ? [{ ...stickyComment.current, user: { login: "gittensory[bot]", type: "Bot" } }] : []);
+        }
+        if (url.includes("/issues/90/comments") && method === "POST") {
+          commentPosts += 1;
+          const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? "");
+          stickyComment.current = { id: 1, body };
+          return Response.json({ id: 1 }, { status: 201 });
+        }
+        if (url.includes("/issues/comments/1") && method === "PATCH") {
+          commentPatches += 1;
+          const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? "");
+          stickyComment.current = { id: 1, body };
+          return Response.json({ id: 1 }, { status: 200 });
+        }
+        if (url.includes("/issues/90/labels") && method === "GET") return Response.json([]);
+        if (url.includes("/issues/90/labels") && method === "POST") {
+          labelPosts += 1;
+          return Response.json([]);
+        }
+        if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } });
+        return Response.json({});
+      });
+
+      // First pass: the initial publish for this head -- always goes through in full (a CREATE placeholder,
+      // then a PATCH to the final settled content, mirroring #3379's own first-pass shape).
+      await processJob(env, { type: "agent-regate-pr", deliveryId: "fork-resume-1", repoFullName: "JSONbored/gittensory", prNumber: 90, installationId: 123 });
+      expect(commentPosts).toBe(1);
+      const patchesAfterFirst = commentPatches;
+      const labelPostsAfterFirst = labelPosts;
+      expect(labelPostsAfterFirst).toBeGreaterThan(0);
+
+      // Three more fork-resume passes over the SAME unchanged head SHA (mirroring one webhook per completing
+      // CI job on a fork PR, confirmed live: 12 of these in 29 minutes for JSONbored/loopover#6592).
+      await processJob(env, { type: "agent-regate-pr", deliveryId: "fork-resume-2", repoFullName: "JSONbored/gittensory", prNumber: 90, installationId: 123 });
+      await processJob(env, { type: "agent-regate-pr", deliveryId: "fork-resume-3", repoFullName: "JSONbored/gittensory", prNumber: 90, installationId: 123 });
+      await processJob(env, { type: "agent-regate-pr", deliveryId: "fork-resume-4", repoFullName: "JSONbored/gittensory", prNumber: 90, installationId: 123 });
+
+      expect(commentPosts).toBe(1); // never re-created
+      expect(commentPatches).toBe(patchesAfterFirst); // never rewritten either — the whole publish pass is skipped, not just diffed away
+
+      const publishedAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?")
+        .bind("github_app.pr_public_surface_published", "JSONbored/gittensory#90")
+        .first<{ n: number }>();
+      expect(publishedAudit?.n).toBe(1); // only the first pass ever counts as a publish
+
+      const skippedAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?")
+        .bind("github_app.public_surface_publish_skipped_current", "JSONbored/gittensory#90")
+        .first<{ n: number }>();
+      expect(skippedAudit?.n).toBe(3); // all three repeat fork-resume passes were proven redundant up-front
+    });
+
+    it("REGRESSION (#6685): a draft PR's republish-skip does NOT apply once a new commit changes the head, or when a maintainer forces a re-trigger", async () => {
+      const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
+      await seedRegateChurnRepo(env, { publicSurface: "comment_and_label" });
+      await upsertRepoFocusManifest(env, "JSONbored/gittensory", { settings: { commentMode: "all_prs", publicSurface: "comment_and_label", checkRunMode: "off", reviewCheckMode: "disabled", aiReviewMode: "block" }, review: { auto_review: { skip_drafts: true, cadence: "continuous" } } });
+      await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 91, title: "Draft feature", state: "open", draft: true, user: { login: "contributor" }, head: { sha: "b91" }, labels: [], body: "Closes #1" } as never);
+      await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 91, status: "complete", reviewsSyncedAt: new Date().toISOString() });
+
+      let headSha = "b91";
+      const stickyComment: { current: { id: number; body: string } | null } = { current: null };
+      let commentPosts = 0;
+      let commentPatches = 0;
+      vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
+        const url = input.toString();
+        const method = init?.method ?? "GET";
+        if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" });
+        if (url.includes(`/pulls/91/files`)) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]);
+        if (url.endsWith("/pulls/91")) return Response.json({ number: 91, title: "Draft feature", state: "open", draft: true, user: { login: "contributor" }, head: { sha: headSha }, labels: [], body: "Closes #1", mergeable_state: "clean" });
+        if (url.includes(`/commits/${headSha}/check-runs`)) return Response.json({ total_count: 0, check_runs: [] });
+        if (url.includes(`/commits/${headSha}/status`)) return Response.json({ state: "success", statuses: [] });
+        if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } });
+        if (url.includes("/issues/91/comments") && method === "GET") {
+          return Response.json(stickyComment.current ? [{ ...stickyComment.current, user: { login: "gittensory[bot]", type: "Bot" } }] : []);
+        }
+        if (url.includes("/issues/91/comments") && method === "POST") {
+          commentPosts += 1;
+          const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? "");
+          stickyComment.current = { id: 1, body };
+          return Response.json({ id: 1 }, { status: 201 });
+        }
+        if (url.includes("/issues/comments/1") && method === "PATCH") {
+          commentPatches += 1;
+          const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? "");
+          stickyComment.current = { id: 1, body };
+          return Response.json({ id: 1 }, { status: 200 });
+        }
+        if (url.includes("/issues/91/labels") && method === "GET") return Response.json([]);
+        if (url.includes("/issues/91/labels") && method === "POST") return Response.json([]);
+        if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } });
+        return Response.json({});
+      });
+
+      await processJob(env, { type: "agent-regate-pr", deliveryId: "fork-resume-1", repoFullName: "JSONbored/gittensory", prNumber: 91, installationId: 123 });
+      expect(commentPosts).toBe(1);
+      const patchesAfterFirst = commentPatches;
+
+      // A new commit lands (new head SHA) -- the next fork-resume pass must NOT be treated as redundant. A
+      // repeat publish to an EXISTING sticky comment is a PATCH (update in place), not a second POST, so a
+      // real republish shows up as an additional PATCH here, not another create.
+      headSha = "c91";
+      await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 91, title: "Draft feature", state: "open", draft: true, user: { login: "contributor" }, head: { sha: headSha }, labels: [], body: "Closes #1" } as never);
+      await processJob(env, { type: "agent-regate-pr", deliveryId: "fork-resume-2", repoFullName: "JSONbored/gittensory", prNumber: 91, installationId: 123 });
+      const patchesAfterNewHead = commentPatches;
+      expect(patchesAfterNewHead).toBeGreaterThan(patchesAfterFirst); // republished (as an update) for the new head, not skipped
+
+      const publishedAfterNewHead = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?")
+        .bind("github_app.pr_public_surface_published", "JSONbored/gittensory#91")
+        .first<{ n: number }>();
+      expect(publishedAfterNewHead?.n).toBe(2); // the first publish, plus this one for the new head
+
+      // A maintainer explicitly forces a fresh pass over the SAME (now-current) head -- must not be skipped either.
+      await processJob(env, { type: "agent-regate-pr", deliveryId: "fork-resume-3", repoFullName: "JSONbored/gittensory", prNumber: 91, installationId: 123, force: true });
+      expect(commentPatches).toBeGreaterThan(patchesAfterNewHead);
+      const publishedAfterForce = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?")
+        .bind("github_app.pr_public_surface_published", "JSONbored/gittensory#91")
+        .first<{ n: number }>();
+      expect(publishedAfterForce?.n).toBe(3);
+    });
+
+    it("swallows a failing draft-republish-skip audit write without throwing (#6685)", async () => {
+      const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
+      await seedRegateChurnRepo(env, { publicSurface: "comment_and_label" });
+      await upsertRepoFocusManifest(env, "JSONbored/gittensory", { settings: { commentMode: "all_prs", publicSurface: "comment_and_label", checkRunMode: "off", reviewCheckMode: "disabled", aiReviewMode: "block" }, review: { auto_review: { skip_drafts: true, cadence: "continuous" } } });
+      await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 92, title: "Draft feature", state: "open", draft: true, user: { login: "contributor" }, head: { sha: "d92" }, labels: [], body: "Closes #1" } as never);
+      await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 92, status: "complete", reviewsSyncedAt: new Date().toISOString() });
+
+      const stickyComment: { current: { id: number; body: string } | null } = { current: null };
+      vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
+        const url = input.toString();
+        const method = init?.method ?? "GET";
+        if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" });
+        if (url.includes("/pulls/92/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]);
+        if (url.endsWith("/pulls/92")) return Response.json({ number: 92, title: "Draft feature", state: "open", draft: true, user: { login: "contributor" }, head: { sha: "d92" }, labels: [], body: "Closes #1", mergeable_state: "clean" });
+        if (url.includes("/commits/d92/check-runs")) return Response.json({ total_count: 0, check_runs: [] });
+        if (url.includes("/commits/d92/status")) return Response.json({ state: "success", statuses: [] });
+        if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } });
+        if (url.includes("/issues/92/comments") && method === "GET") {
+          return Response.json(stickyComment.current ? [{ ...stickyComment.current, user: { login: "gittensory[bot]", type: "Bot" } }] : []);
+        }
+        if (url.includes("/issues/92/comments") && method === "POST") {
+          const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? "");
+          stickyComment.current = { id: 1, body };
+          return Response.json({ id: 1 }, { status: 201 });
+        }
+        if (url.includes("/issues/comments/1") && method === "PATCH") {
+          const body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? "");
+          stickyComment.current = { id: 1, body };
+          return Response.json({ id: 1 }, { status: 200 });
+        }
+        if (url.includes("/issues/92/labels") && method === "GET") return Response.json([]);
+        if (url.includes("/issues/92/labels") && method === "POST") return Response.json([]);
+        if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } });
+        return Response.json({});
+      });
+
+      // First pass publishes for real (populating lastPublishedSurfaceSha).
+      await processJob(env, { type: "agent-regate-pr", deliveryId: "fork-resume-1", repoFullName: "JSONbored/gittensory", prNumber: 92, installationId: 123 });
+
+      const originalRecordAuditEvent = repositoriesModule.recordAuditEvent;
+      const auditSpy = vi.spyOn(repositoriesModule, "recordAuditEvent").mockImplementation(async (auditEnv, event) => {
+        if (event.eventType === "github_app.public_surface_publish_skipped_current") throw new Error("audit DB down");
+        await originalRecordAuditEvent(auditEnv, event);
+      });
+
+      // The second (repeat, same-head) pass takes the draft-skip path -- its audit write fails, but the job
+      // must still resolve cleanly rather than throwing.
+      await expect(
+        processJob(env, { type: "agent-regate-pr", deliveryId: "fork-resume-2", repoFullName: "JSONbored/gittensory", prNumber: 92, installationId: 123 }),
+      ).resolves.toBeUndefined();
+      auditSpy.mockRestore();
+    });
+
     it("a PURE base-branch movement (no reviewed content change) triggers neither a fresh AI review nor a comment rewrite", async () => {
       let aiCalls = 0;
       const env = createTestEnv({
