fix(github): exclude the panel's per-pass timestamp from the comment idempotency check (#9069)

src/github/comments.ts

@@ -26,6 +26,30 @@ export function closeExplanationMarker(closeKind: string | undefined): string {
 // `batch.length < 100` early-exit below still keeps a short comment list to a single request.
 const COMMENT_SEARCH_PAGE_LIMIT = 10;
 
+// The PR panel embeds a per-pass `<sub>Review updated: <timestamp></sub>` line (review/unified-comment.ts),
+// re-stamped from `reviewedAt ?? new Date()` on every render. Comparing raw bodies in the idempotency check
+// below therefore NEVER matched for the panel, defeating the skip entirely: every re-gate tick PATCHed GitHub
+// (a write + rate-limit cost) purely to move a clock, and each PATCH generated an inbound
+// `issue_comment.edited` delivery that ingress then classified as our own noise and discarded -- ~26% of all
+// lifetime webhook traffic (79,612 of ~309,600 deliveries) was this loop feeding itself (#9069).
+//
+// Normalizing the timestamp out of BOTH sides restores the skip. Deliberately COMPARE-ONLY: the body actually
+// posted keeps its real timestamp, and whenever some other part of the body does change, the PATCH carries the
+// fresh one along with it. The surviving timestamp then reads as "the review last CHANGED at X" rather than
+// "we last looked at X" -- the more useful meaning, and the one the line's own wording already implies.
+//
+// Bounded to `[^<]*` so it can only ever match this exact generated line: every caller-supplied string that
+// reaches a comment body is HTML-angle-escaped first (escapePublicHtmlAngles), so contributor text cannot
+// forge a `<sub>` wrapper here. Comments without the line (close explanations, visual follow-ups) are
+// untouched, keeping their comparison byte-exact as before.
+const VOLATILE_REVIEW_TIMESTAMP_LINE = /^<sub>Review updated: [^<]*<\/sub>$/gm;
+const VOLATILE_REVIEW_TIMESTAMP_PLACEHOLDER = "<sub>Review updated:</sub>";
+
+/** Body projection used ONLY for the idempotency equality check -- never for what gets posted. */
+export function comparableCommentBody(body: string): string {
+  return body.replace(VOLATILE_REVIEW_TIMESTAMP_LINE, VOLATILE_REVIEW_TIMESTAMP_PLACEHOLDER);
+}
+
 type IssueComment = {
   id: number;
   body?: string | null;
@@ -138,11 +162,16 @@ async function createOrUpdateIssueCommentWithMarker(
     }
     const canonical = canonicalMarkerComment(existing);
     if (canonical) {
-      // Idempotency (#4): skip the PATCH when the rendered body is byte-identical to what's already posted. The
+      // Idempotency (#4): skip the PATCH when the rendered body matches what's already posted. The
       // re-gate sweep re-renders the same surface every cycle for an unchanged PR; without this, every cycle PATCHes
       // GitHub (a write + rate-limit cost) for no visible change. Defense-in-depth alongside the head_sha publish
       // marker — also collapses a duplicate webhook delivery for the same commit.
-      if (canonical.body === body) {
+      // #9069: compared through comparableCommentBody so the panel's per-pass "Review updated" timestamp — which
+      // changes on every render and made this check unreachable for the panel — no longer counts as a change.
+      /* v8 ignore next -- `?? ""` is a type-level guard only: the marker filter above requires
+       * `comment.body?.includes(candidate)`, so any comment that becomes `canonical` provably has a non-empty
+       * string body. Kept because IssueComment types `body` as `string | null | undefined`. */
+      if (comparableCommentBody(canonical.body ?? "") === comparableCommentBody(body)) {
         await deleteDuplicateMarkerComments(octokit, owner, repo, existing, canonical.id);
         return { id: canonical.id, ...(canonical.html_url !== undefined ? { html_url: canonical.html_url } : {}), changed: false };
       }

test/unit/github-comments.test.ts

@@ -1,5 +1,5 @@
 import { afterEach, describe, expect, it, vi } from "vitest";
-import { closeExplanationMarker, createOrUpdateCloseExplanationComment, createOrUpdatePrIntelligenceComment, createOrUpdateVisualFollowupComment, PR_INTELLIGENCE_COMMENT_MARKER, VISUAL_FOLLOWUP_COMMENT_MARKER } from "../../src/github/comments";
+import { closeExplanationMarker, comparableCommentBody, createOrUpdateCloseExplanationComment, createOrUpdatePrIntelligenceComment, createOrUpdateVisualFollowupComment, PR_INTELLIGENCE_COMMENT_MARKER, VISUAL_FOLLOWUP_COMMENT_MARKER } from "../../src/github/comments";
 import { createTestEnv } from "../helpers/d1";
 import { generatePrivateKeyPem } from "../helpers/github-app-key";
 
@@ -420,6 +420,78 @@ describe("GitHub PR intelligence comments", () => {
     expect(calls.some((call) => call.startsWith("PATCH "))).toBe(false);
   });
 
+  it("skips the PATCH when only the panel's per-pass 'Review updated' timestamp differs (#9069)", async () => {
+    const privateKey = await generatePrivateKeyPem();
+    const posted = `${PR_INTELLIGENCE_COMMENT_MARKER}\n### result\n<sub>Review updated: 2026-07-26 15:15:11 UTC</sub>\nunchanged verdict`;
+    const rerendered = `${PR_INTELLIGENCE_COMMENT_MARKER}\n### result\n<sub>Review updated: 2026-07-26 16:41:02 UTC</sub>\nunchanged verdict`;
+    const calls: string[] = [];
+    vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
+      const url = input.toString();
+      calls.push(`${init?.method ?? "GET"} ${url}`);
+      if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
+      if (url.includes("/issues/12/comments") && (init?.method ?? "GET") === "GET") {
+        return Response.json([{ id: 303, body: posted, html_url: "https://github.com/comment/303", user: { login: "loopover-orb[bot]", type: "Bot" } }]);
+      }
+      return new Response("not found", { status: 404 });
+    });
+
+    const result = await createOrUpdatePrIntelligenceComment(createTestEnv({ GITHUB_APP_PRIVATE_KEY: privateKey }), 123, "JSONbored/gittensory", 12, rerendered);
+
+    // The whole point of #9069: a clock-only delta is NOT a content change, so no GitHub write and no
+    // self-inflicted issue_comment.edited delivery. changed:false also keeps the #6724 no-op accounting honest.
+    expect(result).toEqual({ id: 303, html_url: "https://github.com/comment/303", changed: false });
+    expect(calls.some((call) => call.startsWith("PATCH "))).toBe(false);
+  });
+
+  it("still PATCHes when real content changes alongside the timestamp (#9069 does not suppress real updates)", async () => {
+    const privateKey = await generatePrivateKeyPem();
+    const posted = `${PR_INTELLIGENCE_COMMENT_MARKER}\n### result\n<sub>Review updated: 2026-07-26 15:15:11 UTC</sub>\nCI failing`;
+    const rerendered = `${PR_INTELLIGENCE_COMMENT_MARKER}\n### result\n<sub>Review updated: 2026-07-26 16:41:02 UTC</sub>\nCI green`;
+    const calls: string[] = [];
+    let patchedBody = "";
+    vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
+      const url = input.toString();
+      calls.push(`${init?.method ?? "GET"} ${url}`);
+      if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
+      if (url.includes("/issues/12/comments") && (init?.method ?? "GET") === "GET") {
+        return Response.json([{ id: 404, body: posted, user: { login: "loopover-orb[bot]", type: "Bot" } }]);
+      }
+      if (url.includes("/issues/comments/404") && init?.method === "PATCH") {
+        patchedBody = (JSON.parse(String(init.body)) as { body: string }).body;
+        return Response.json({ id: 404 });
+      }
+      return new Response("not found", { status: 404 });
+    });
+
+    const result = await createOrUpdatePrIntelligenceComment(createTestEnv({ GITHUB_APP_PRIVATE_KEY: privateKey }), 123, "JSONbored/gittensory", 12, rerendered);
+
+    expect(result?.changed).toBe(true);
+    expect(calls.some((call) => call.startsWith("PATCH "))).toBe(true);
+    // Compare-only normalization: the body actually posted carries the REAL fresh timestamp, not the placeholder.
+    expect(patchedBody).toContain("Review updated: 2026-07-26 16:41:02 UTC");
+  });
+
+  it("INVARIANT (#9069): normalization is compare-only and confined to the generated timestamp line", () => {
+    // Regression guard for the exact loop that produced ~26% of lifetime webhook traffic: two renders of one
+    // unchanged panel must compare equal, while any real content delta must not.
+    const at = (stamp: string) => `${PR_INTELLIGENCE_COMMENT_MARKER}\n### result\n<sub>Review updated: ${stamp}</sub>\nverdict`;
+    expect(comparableCommentBody(at("2026-07-26 15:15:11 UTC"))).toBe(comparableCommentBody(at("2027-01-01 00:00:00 UTC")));
+    expect(comparableCommentBody(at("x"))).not.toBe(comparableCommentBody(`${PR_INTELLIGENCE_COMMENT_MARKER}\n### result\n<sub>Review updated: x</sub>\nDIFFERENT`));
+
+    // Bodies without the line (close explanations, visual follow-ups) stay byte-exact — no accidental widening.
+    const plain = `${PR_INTELLIGENCE_COMMENT_MARKER}\nclose explanation`;
+    expect(comparableCommentBody(plain)).toBe(plain);
+
+    // Must not swallow a same-named line carrying different surrounding text, and must not match across a
+    // forged `<` (contributor text is angle-escaped upstream, so `[^<]*` can only ever span the real stamp).
+    const forged = `${PR_INTELLIGENCE_COMMENT_MARKER}\n<sub>Review updated: a</sub>injected<sub>Review updated: b</sub>`;
+    expect(comparableCommentBody(forged)).toBe(forged); // neither line is on its own line → untouched
+
+    // Multi-occurrence safety: the /g regex must normalize every standalone occurrence, not just the first.
+    const twice = `<sub>Review updated: one</sub>\nmid\n<sub>Review updated: two</sub>`;
+    expect(comparableCommentBody(twice)).toBe("<sub>Review updated:</sub>\nmid\n<sub>Review updated:</sub>");
+  });
+
   it("rejects invalid repository names before calling GitHub", async () => {
     await expect(createOrUpdatePrIntelligenceComment(createTestEnv(), 123, "invalid", 12, "body")).rejects.toThrow(/Invalid repository full name/);
   });