fix(heartbeat): never grade the PR author's own run as the Ally review (BLO-34699)

server/src/__tests__/heartbeat-process-recovery.test.ts

@@ -2698,7 +2698,13 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
     expect(notice[0]?.body).toContain("Blockcast/libmmt#444");
     expect(notice[0]?.body).toContain(headSha);
     // The terminal fact itself, in words, not just a link to a red check.
-    expect(notice[0]?.body).toContain("No review was posted, and none is coming for this head");
+    expect(notice[0]?.body).toContain("No review was posted by that run");
+    // BLO-34699: and NOT a claim about the queue, which this notice never read.
+    // "none is coming for this head — a terminal outcome, not reviewer latency"
+    // was measured false on Blockcast/pim-multicast-gateway#3237 at the moment
+    // it was written, with a queued reviewer run for that exact head 4.67h old.
+    expect(notice[0]?.body).not.toContain("none is coming");
+    expect(notice[0]?.body).not.toContain("not reviewer latency");
 
     // The comment is the durable artifact; the wake is what actually reaches an
     // agent. `in_review` is excluded from inbox-lite by design, so without this

server/src/__tests__/heartbeat-retry-scheduling.test.ts

@@ -3293,8 +3293,17 @@ describeEmbeddedPostgres("heartbeat bounded retry scheduling", () => {
       return { events, runId };
     }
 
+    // BLO-34699: `reviewKind`/`prRole` are load-bearing, not decoration. The
+    // seeded `pr_review_output_missing` errorCode is only reachable through
+    // evaluatePrReviewCompletionEvidence, which returns `not_applicable` unless
+    // reviewKind === "pr_review" and prRole is absent or "reviewer" — so a
+    // snapshot without them is a shape production cannot mint. Omitting them
+    // also made the three negative cases below pass vacuously, for the tag
+    // check rather than for the condition each one names.
     const prReviewSnapshot = {
       wakeReason: "github_pr_synchronized",
+      reviewKind: "pr_review",
+      prRole: "reviewer",
       githubPrNumber: 7,
       githubRepoFullName: "Blockcast/hang",
       githubHeadSha: HEAD_SHA,
@@ -3351,6 +3360,30 @@ describeEmbeddedPostgres("heartbeat bounded retry scheduling", () => {
       expect(events.at(-1)?.message).toContain("Bounded retry exhausted");
       expect(events.some((e) => e.message.includes("gate status"))).toBe(false);
     });
+
+    // BLO-34699: the PR author's own agent is woken by its own review-request
+    // marker (BLO-19522) and carries the same repo/head/PR identity as the
+    // reviewer, so before the resolver read these tags a crashed author run was
+    // published as the reviewer's verdict on the head. Asserted at the call
+    // site, not only over the pure predicate: this is the case that must
+    // enqueue no `github_commit_status` delivery at all. One row per guard
+    // clause so neither can hide behind the other under mutation (BLO-34263) —
+    // `measured` is the shape seen on pim-multicast-gateway#3237.
+    it.each([
+      { label: "measured author shape (no reviewKind)", overrides: { reviewKind: undefined, prRole: "author" } },
+      { label: "author run that is tagged pr_review", overrides: { prRole: "author" } },
+    ])("writes no gate-status event for the PR author's own run — $label", async ({ overrides }) => {
+      process.env[GATE_CONTEXT_ENV] = "review/ally-complete";
+      const { events } = await exhaustPrReviewRun({ ...prReviewSnapshot, ...overrides });
+
+      expect(events.at(-1)?.message).toContain("Bounded retry exhausted");
+      expect(events.some((e) => e.message.includes("gate status"))).toBe(false);
+      const deliveries = await db
+        .select()
+        .from(githubCommitStatusDeliveries)
+        .where(eq(githubCommitStatusDeliveries.sha, HEAD_SHA));
+      expect(deliveries).toHaveLength(0);
+    });
   });
 
   it.each([

server/src/__tests__/pr-review-gate-status-target.test.ts

@@ -21,6 +21,13 @@ const HEAD_SHA = "45eb633e348a826f43dc68b0c25fe83a96300cea";
 function prReviewSnapshot(overrides: Record<string, unknown> = {}) {
   return {
     wakeReason: "github_pr_synchronized",
+    // BLO-34699: both reviewer wake constructors stamp these together, and the
+    // gate now requires them. Earlier revisions of this fixture omitted both,
+    // which made every case here indistinguishable from the PR AUTHOR's own
+    // self-wake — the shape that manufactured the false red this guard exists
+    // to stop.
+    reviewKind: "pr_review",
+    prRole: "reviewer",
     githubPrNumber: 7,
     githubRepoFullName: "Blockcast/hang",
     githubHeadSha: HEAD_SHA,
@@ -84,4 +91,58 @@ describe("resolvePrReviewGateStatusTarget", () => {
   it("carries a null prUrl through rather than fabricating a target link", () => {
     expect(resolvePrReviewGateStatusTarget(prReviewSnapshot({ githubPrUrl: undefined }), GATE)?.prUrl).toBeNull();
   });
+
+  /**
+   * BLO-34699 — the PR AUTHOR's own run must never be graded as the review.
+   *
+   * The author's agent is woken by its own `<!-- paperclip:review-request -->`
+   * marker (BLO-19522, deliberate). That wake carries the same repo, head SHA
+   * and PR number as the reviewer's, so every other clause in this resolver
+   * passes on it. Measured on Blockcast/pim-multicast-gateway#3237: an author
+   * run that died `k8s_pod_schedule_failed` at pod start with zero turns was
+   * written as `review/ally-complete = failure` while Ally's real review for
+   * that exact head was still queued — a red on a `ci-gate` peer that could
+   * not self-heal, because the gate re-runs on `pull_request_review:
+   * submitted` and Ally's common shape is a comment-shaped review.
+   *
+   * Mutation check (BLO-34263): delete either guard line in
+   * `resolvePrReviewGateStatusTarget` alone and one of these two must fail.
+   */
+  const AUTHOR_SNAPSHOT = {
+    // Verbatim shape of run 974efdbd-0b03-4beb-b3c3-74c8ea6f1341.
+    wakeReason: "github_pr_review_requested",
+    prRole: "author",
+    reviewKind: null,
+    githubEvent: "issue_comment",
+    githubPrNumber: 3237,
+    githubRepoFullName: "Blockcast/pim-multicast-gateway",
+    githubHeadSha: "8a38994bffe3ca61e29b4cfa1ed5f04995d6f7c3",
+    githubPrReviewRequestAuthorLogin: "github-actions[bot]",
+  };
+
+  it("does not post for the PR author's own self-wake run", () => {
+    expect(resolvePrReviewGateStatusTarget(AUTHOR_SNAPSHOT, GATE)).toBeNull();
+  });
+
+  it("does not post for an author run even if it carries the pr_review tag", () => {
+    // Isolates the `prRole` clause from the `reviewKind` clause: with only the
+    // reviewKind guard in place this snapshot would still resolve a target.
+    expect(
+      resolvePrReviewGateStatusTarget({ ...AUTHOR_SNAPSHOT, reviewKind: "pr_review" }, GATE),
+    ).toBeNull();
+  });
+
+  it("does not post for a PR-shaped wake carrying no pr_review tag at all", () => {
+    // Isolates the `reviewKind` clause: drops `prRole` so the second guard
+    // cannot be what rejects this.
+    const { prRole: _prRole, ...untagged } = AUTHOR_SNAPSHOT;
+    expect(resolvePrReviewGateStatusTarget(untagged, GATE)).toBeNull();
+  });
+
+  it("still posts for a genuine reviewer run — the BLO-17456 wedge stays visible", () => {
+    // The guard must not fail closed on the case the feature exists for.
+    expect(
+      resolvePrReviewGateStatusTarget(prReviewSnapshot({ prRole: "reviewer" }), GATE)?.sha,
+    ).toBe(HEAD_SHA);
+  });
 });

server/src/__tests__/pr-review-gate-verdict.test.ts

@@ -0,0 +1,92 @@
+/**
+ * BLO-34699: `producedPrReviewGateVerdict` — may this terminal run's outcome be
+ * published as a verdict about the PR head?
+ *
+ * A commit status is a claim about the HEAD, not about the run. A reviewer run
+ * whose pod never scheduled invoked no adapter, read no diff, and formed no
+ * judgement, so writing `review/ally-complete = failure` on its behalf asserts
+ * something nobody has established.
+ *
+ * Measured on Blockcast/paperclip 2026-09-19: four heads carried that status
+ * with "ended ambiguously and was not replayed; no review was confirmed", and a
+ * genuine non-stale formal review landed at that exact head on three of them
+ * 4h59m–5h48m later (#1929, #1931, #1932). The run behind #1931's stamp,
+ * b3ed7bde, died `k8s_pod_schedule_failed` with no `adapter.invoke` event.
+ * `review/ally-complete` has no writer that ever clears it, so #1929 still
+ * carried that red beside a `gate/ally-comment-findings: success` for the same
+ * head ~14h on — two gates contradicting each other about one fact.
+ *
+ * Mutation check (BLO-34263): each guard in the function has a test below that
+ * fails when that guard alone is reverted. Remove `return false` on the
+ * terminal-outcome line and `ignores a run that is not a PR-review terminal
+ * outcome` fails; replace the `adapterInvocationStarted` line with `return true`
+ * and both `k8s_pod_schedule_failed` cases fail.
+ */
+import { describe, expect, it } from "vitest";
+
+import { producedPrReviewGateVerdict } from "../services/heartbeat.js";
+
+function run(overrides: Record<string, unknown> = {}) {
+  return {
+    errorCode: null,
+    resultJson: null,
+    contextSnapshot: null,
+    ...overrides,
+  } as Parameters<typeof producedPrReviewGateVerdict>[0];
+}
+
+function invoked(started: boolean) {
+  return { externalLifecycleRecovery: { adapterInvocationStarted: started } };
+}
+
+describe("producedPrReviewGateVerdict", () => {
+  it("does not grade a pod that never scheduled", () => {
+    // Verbatim shape of run b3ed7bde: `adapterInvocationStarted` is not merely
+    // false here, it is never computed — `hasAdapterInvocationEvent` is
+    // consulted only for job_failed/job_missing — so the absent key is the
+    // production shape, not a contrived one.
+    expect(producedPrReviewGateVerdict(run({ errorCode: "k8s_pod_schedule_failed" }))).toBe(false);
+  });
+
+  it("does not grade a pod-schedule failure even with an explicit negative proof", () => {
+    expect(
+      producedPrReviewGateVerdict(
+        run({ errorCode: "k8s_pod_schedule_failed", resultJson: invoked(false) }),
+      ),
+    ).toBe(false);
+  });
+
+  it("grades a missing Job once the adapter demonstrably ran", () => {
+    // The genuine case this feature exists for: the reviewer was invoked and the
+    // Job then vanished, so no review is coming from that run. Guards against
+    // fixing the false positive by failing closed on every infra code.
+    expect(
+      producedPrReviewGateVerdict(run({ errorCode: "job_missing", resultJson: invoked(true) })),
+    ).toBe(true);
+  });
+
+  it("does not grade a missing Job that never reached its adapter", () => {
+    expect(
+      producedPrReviewGateVerdict(run({ errorCode: "job_missing", resultJson: invoked(false) })),
+    ).toBe(false);
+  });
+
+  it("still grades a reviewer run that ran and posted nothing", () => {
+    // BLO-17456's original case, unchanged: these arms already proved invocation.
+    for (const errorCode of ["pr_review_output_missing", "pr_review_verification_unavailable"]) {
+      expect(producedPrReviewGateVerdict(run({ errorCode, resultJson: invoked(true) }))).toBe(true);
+    }
+  });
+
+  it("ignores a run that is not a PR-review terminal outcome", () => {
+    // `process_lost` is ~67 of the reviewer's last 1000 runs and is handled by
+    // retry, not by the gate. Without the terminal-outcome guard the invocation
+    // proof alone would grade it.
+    expect(
+      producedPrReviewGateVerdict(run({ errorCode: "process_lost", resultJson: invoked(true) })),
+    ).toBe(false);
+    expect(producedPrReviewGateVerdict(run({ errorCode: null, resultJson: invoked(true) }))).toBe(
+      false,
+    );
+  });
+});