From eae99b4a0019816d7fa1cea35f2b425e93a80d54 Mon Sep 17 00:00:00 2001
From: CTO <cto@paperclip.blockcast.net>
Date: Fri, 28 Aug 2026 10:05:27 +0000
Subject: [PATCH 1/7] fix(review-gate): linearize retired context migration

---
 .../paperclip/templates/deployment-api.yaml   |   4 +
 .../helm/paperclip/templates/statefulset.yaml |   4 +
 deploy/helm/paperclip/values.blockcast.yaml   |  22 +-
 deploy/helm/paperclip/values.yaml             |   6 +
 ...ithub_status_webhook_outbox_provenance.sql |  10 +
 packages/db/src/migrations/meta/_journal.json |   7 +
 .../schema/github_commit_status_deliveries.ts |  11 +-
 .../github-status-delivery-outbox.test.ts     | 163 +++++++++++-
 .../pr-comment-review-gate-check.test.ts      |  97 +++++++-
 .../pr-comment-review-gate-deployment.test.ts |  66 +++++
 .../__tests__/pr-comment-review-gate.test.ts  |  87 +++++++
 server/src/config.ts                          |  15 ++
 server/src/routes/github-webhook.ts           |  26 +-
 .../services/github-status-delivery-outbox.ts | 128 ++++++++--
 server/src/services/pr-comment-review-gate.ts | 235 ++++++++++++++++--
 15 files changed, 824 insertions(+), 57 deletions(-)
 create mode 100644 packages/db/src/migrations/0239_github_status_webhook_outbox_provenance.sql
 create mode 100644 server/src/__tests__/pr-comment-review-gate-deployment.test.ts

diff --git a/deploy/helm/paperclip/templates/deployment-api.yaml b/deploy/helm/paperclip/templates/deployment-api.yaml
index 520fb741ceb8..8557551ab16f 100644
--- a/deploy/helm/paperclip/templates/deployment-api.yaml
+++ b/deploy/helm/paperclip/templates/deployment-api.yaml
@@ -301,6 +301,10 @@ spec:
             - name: PAPERCLIP_PR_COMMENT_REVIEW_GATE_STATUS_CONTEXT
               value: {{ . | quote }}
             {{- end }}
+            {{- with ((.Values.githubApp).prCommentReviewGateRetiredStatusContexts) }}
+            - name: PAPERCLIP_PR_COMMENT_REVIEW_GATE_RETIRED_STATUS_CONTEXTS
+              value: {{ . | quote }}
+            {{- end }}
             {{- end }}
             {{- with .Values.env.extra }}
             {{- toYaml . | nindent 12 }}
diff --git a/deploy/helm/paperclip/templates/statefulset.yaml b/deploy/helm/paperclip/templates/statefulset.yaml
index b5a614679c4b..f443b6cfb90e 100644
--- a/deploy/helm/paperclip/templates/statefulset.yaml
+++ b/deploy/helm/paperclip/templates/statefulset.yaml
@@ -938,6 +938,10 @@ spec:
             - name: PAPERCLIP_PR_COMMENT_REVIEW_GATE_STATUS_CONTEXT
               value: {{ . | quote }}
             {{- end }}
+            {{- with ((.Values.githubApp).prCommentReviewGateRetiredStatusContexts) }}
+            - name: PAPERCLIP_PR_COMMENT_REVIEW_GATE_RETIRED_STATUS_CONTEXTS
+              value: {{ . | quote }}
+            {{- end }}
             {{- end }}
             {{- with .Values.env.extra }}
             {{- toYaml . | nindent 12 }}
diff --git a/deploy/helm/paperclip/values.blockcast.yaml b/deploy/helm/paperclip/values.blockcast.yaml
index dd0649822810..7e183c21e2bb 100644
--- a/deploy/helm/paperclip/values.blockcast.yaml
+++ b/deploy/helm/paperclip/values.blockcast.yaml
@@ -417,7 +417,9 @@ githubApp:
   # contexts it has observed recently, and this repo has never carried a commit
   # status, so the context has to be posted at least once before a human can
   # select it. Until then this is observe-only.
-  prCommentReviewGateStatusContext: "review/ally-comment"
+  # This gate intentionally stays outside `review/`: it is not review evidence.
+  prCommentReviewGateStatusContext: "gate/ally-comment-findings"
+  prCommentReviewGateRetiredStatusContexts: "review/ally-comment"
   # NOTE: `prReviewGateStatusContext` is shared by the legacy failure status
   # and the durable review-gate authority below. The latter owns
   # `review/ally-complete`, including its success writer.
@@ -439,6 +441,24 @@ githubApp:
   reviewGateExpectedAppId: "3966421"
   reviewGateExpectedInstallationId: "138085375"
   prReviewGateStatusContext: review/ally-complete
+  #
+  # Deliberately NOT under `review/` (BLO-29711). This gate reads only the
+  # comment-shaped surface, so "nothing attests this head" is a legitimate and
+  # common outcome for a formally-reviewed PR, and it must stay green there or
+  # every such PR deadlocks. A green under `review/` reads as review evidence,
+  # which this gate cannot supply — so the fail-open and the namespace cannot
+  # coexist. Moving the namespace is the half that is safe to change.
+  # Verified before renaming: `review/ally-comment` was not a required check on
+  # paperclip `master` (required: `verify`) nor on penstock-llm-proxy-core
+  # `main` (required: validate-and-build, secret-scan, redaction-tests,
+  # secrets-controls-static-check, review/ally-complete), so this drops nothing.
+  # NOTE: `prReviewGateStatusContext` is intentionally left unset here. Its only
+  # writer posts state=failure (reviewer chain exhausted / ended ambiguously) —
+  # nothing in the server ever posts success for it, and no workflow or script
+  # in this repo posts it either. It is meant to red-flag an already-required
+  # context owned by an external writer (as on onprem-k8s). Marking it required
+  # on this repo would leave every healthy PR at "Expected — waiting for status"
+  # forever. Do not enable + require it here without a success writer.
 
 # paperclip-plugin-gbrain reads per-agent OAuth client credentials through
 # Penstock/Authbot. The mounted Secret below contains only the managed service
diff --git a/deploy/helm/paperclip/values.yaml b/deploy/helm/paperclip/values.yaml
index 1c248e7469d0..5db57de58f77 100644
--- a/deploy/helm/paperclip/values.yaml
+++ b/deploy/helm/paperclip/values.yaml
@@ -256,6 +256,12 @@ githubApp:
   # required in branch protection so Critical/Important comment findings block
   # the head they explicitly attest to. Requires `statuses: write`.
   prCommentReviewGateStatusContext: ""
+  # -- Contexts this gate previously published to. Commit statuses cannot be
+  # deleted, so a rename strands the old context on every head that already
+  # carries it. Each name listed here is overwritten with a pointer to the live
+  # context on every evaluation, which supersedes the stale row and keeps a repo
+  # that still requires the old name satisfied. Comma-separated.
+  prCommentReviewGateRetiredStatusContexts: ""
 
 # -- Pod spec knobs.
 pod:
diff --git a/packages/db/src/migrations/0239_github_status_webhook_outbox_provenance.sql b/packages/db/src/migrations/0239_github_status_webhook_outbox_provenance.sql
new file mode 100644
index 000000000000..e70f6fe49dc5
--- /dev/null
+++ b/packages/db/src/migrations/0239_github_status_webhook_outbox_provenance.sql
@@ -0,0 +1,10 @@
+-- Webhook-triggered status deliveries have no heartbeat run or company row.
+-- Keep those writes durable in the same outbox instead of logging and losing
+-- a failed retired-context overwrite.
+ALTER TABLE "github_commit_status_deliveries"
+  ALTER COLUMN "company_id" DROP NOT NULL;--> statement-breakpoint
+ALTER TABLE "github_commit_status_deliveries"
+  ALTER COLUMN "source_run_id" DROP NOT NULL;
+--> statement-breakpoint
+ALTER TABLE "github_commit_status_deliveries"
+  ADD COLUMN IF NOT EXISTS "force_write" boolean DEFAULT false NOT NULL;
diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json
index ed1278241680..3ecf4a89de5e 100644
--- a/packages/db/src/migrations/meta/_journal.json
+++ b/packages/db/src/migrations/meta/_journal.json
@@ -1618,6 +1618,13 @@
       "when": 1788560000000,
       "tag": "0238_heartbeat_runs_status_only_document_write_refused",
       "breakpoints": true
+    },
+    {
+      "idx": 239,
+      "version": "7",
+      "when": 1788560060000,
+      "tag": "0239_github_status_webhook_outbox_provenance",
+      "breakpoints": true
     }
   ]
 }
diff --git a/packages/db/src/schema/github_commit_status_deliveries.ts b/packages/db/src/schema/github_commit_status_deliveries.ts
index 2e74e615a163..88d1380a2e9c 100644
--- a/packages/db/src/schema/github_commit_status_deliveries.ts
+++ b/packages/db/src/schema/github_commit_status_deliveries.ts
@@ -1,4 +1,4 @@
-import { pgTable, uuid, text, timestamp, integer, jsonb, index, uniqueIndex } from "drizzle-orm/pg-core";
+import { pgTable, uuid, text, timestamp, integer, jsonb, boolean, index, uniqueIndex } from "drizzle-orm/pg-core";
 import { companies } from "./companies.js";
 import { heartbeatRuns } from "./heartbeat_runs.js";
 
@@ -10,16 +10,13 @@ export const githubCommitStatusDeliveries = pgTable(
   "github_commit_status_deliveries",
   {
     id: uuid("id").primaryKey().defaultRandom(),
-    companyId: uuid("company_id")
-      .notNull()
-      .references(() => companies.id),
-    sourceRunId: uuid("source_run_id")
-      .notNull()
-      .references(() => heartbeatRuns.id, { onDelete: "cascade" }),
+    companyId: uuid("company_id").references(() => companies.id),
+    sourceRunId: uuid("source_run_id").references(() => heartbeatRuns.id, { onDelete: "cascade" }),
     repoFullName: text("repo_full_name").notNull(),
     sha: text("sha").notNull(),
     context: text("context").notNull(),
     state: text("state").notNull().default("failure"),
+    forceWrite: boolean("force_write").notNull().default(false),
     description: text("description").notNull(),
     targetUrl: text("target_url"),
     prNumber: integer("pr_number").notNull(),
diff --git a/server/src/__tests__/github-status-delivery-outbox.test.ts b/server/src/__tests__/github-status-delivery-outbox.test.ts
index bc586c63af18..e4d6fff7ff7b 100644
--- a/server/src/__tests__/github-status-delivery-outbox.test.ts
+++ b/server/src/__tests__/github-status-delivery-outbox.test.ts
@@ -25,12 +25,13 @@ const h = vi.hoisted(() => ({
 
 vi.mock("../config.js", () => ({ loadConfig: () => h.cfg }));
 
-import { _resetInstallationTokenCache } from "../services/github-app-auth.js";
+import { _resetInstallationTokenCache, githubPostCommitStatusDetailed } from "../services/github-app-auth.js";
 import {
   _classifyReviewerEvidenceError,
   enqueueGithubCommitStatusDelivery,
   pollGitHubCommitStatusDeliveriesOnce,
   resetStaleGitHubCommitStatusDeliveries,
+  withGithubStatusDeliveryLock,
 } from "../services/github-status-delivery-outbox.js";
 
 const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
@@ -598,6 +599,166 @@ describeEmbeddedPostgres("GitHub commit-status delivery outbox", () => {
     expect(events.at(-1)?.message).toContain("will retry");
   });
 
+  it("does not let a forced retirement retry overwrite a fresh clean evaluation", async () => {
+    setCreds();
+    const { delivery } = await seedRun();
+    const queuedAt = new Date(Date.now() - 60_000);
+    await db
+      .update(githubCommitStatusDeliveries)
+      .set({
+        context: "review/ally-comment",
+        forceWrite: true,
+        status: "queued",
+        createdAt: queuedAt,
+        nextAttemptAt: queuedAt,
+        updatedAt: queuedAt,
+      })
+      .where(eq(githubCommitStatusDeliveries.id, delivery.id));
+
+    const fetchMock = stubGithub({
+      latestStatuses: [
+        {
+          context: "review/ally-comment",
+          state: "success",
+          created_at: new Date().toISOString(),
+        },
+      ],
+      reviews: [],
+      comments: [],
+    });
+
+    await expect(pollGitHubCommitStatusDeliveriesOnce(db)).resolves.toBe(1);
+
+    expect(await readDelivery(delivery.id)).toMatchObject({
+      status: "skipped",
+      forceWrite: true,
+      lastResult: { reason: "fresh_success_status_exists" },
+    });
+    expect(fetchMock.mock.calls.some(([url]) => String(url).includes(`/statuses/${HEAD_SHA}`))).toBe(false);
+  });
+
+  it("does not let a stale forced success overwrite a newer blocking evaluation", async () => {
+    setCreds();
+    const { delivery } = await seedRun();
+    const queuedAt = new Date(Date.now() - 60_000);
+    await db
+      .update(githubCommitStatusDeliveries)
+      .set({
+        context: "review/ally-comment",
+        state: "success",
+        forceWrite: true,
+        status: "queued",
+        createdAt: queuedAt,
+        nextAttemptAt: queuedAt,
+        updatedAt: queuedAt,
+      })
+      .where(eq(githubCommitStatusDeliveries.id, delivery.id));
+
+    const fetchMock = stubGithub({
+      latestStatuses: [
+        {
+          context: "review/ally-comment",
+          state: "failure",
+          created_at: new Date().toISOString(),
+        },
+      ],
+      reviews: [],
+      comments: [],
+    });
+
+    await expect(pollGitHubCommitStatusDeliveriesOnce(db)).resolves.toBe(1);
+
+    expect(await readDelivery(delivery.id)).toMatchObject({
+      status: "skipped",
+      forceWrite: true,
+      lastResult: { reason: "newer_status_exists" },
+    });
+    expect(fetchMock.mock.calls.some(([url]) => String(url).includes(`/statuses/${HEAD_SHA}`))).toBe(false);
+  });
+
+  it("lets a clean evaluation win between the retry freshness check and its post", async () => {
+    setCreds();
+    const { delivery } = await seedRun();
+    const queuedAt = new Date(Date.now() - 60_000);
+    await db
+      .update(githubCommitStatusDeliveries)
+      .set({
+        context: "review/ally-comment",
+        forceWrite: true,
+        status: "queued",
+        createdAt: queuedAt,
+        nextAttemptAt: queuedAt,
+        updatedAt: queuedAt,
+        description: "stale failure payload",
+      })
+      .where(eq(githubCommitStatusDeliveries.id, delivery.id));
+
+    let statusReads = 0;
+    let releaseRetryRead!: () => void;
+    const retryReadStarted = new Promise<void>((resolve) => {
+      releaseRetryRead = resolve;
+    });
+    let releaseRetry!: () => void;
+    const retryReleased = new Promise<void>((resolve) => {
+      releaseRetry = resolve;
+    });
+    const postedBodies: Array<{ state?: string; description?: string }> = [];
+    const fetchMock = vi.fn(async (url: string | URL, init?: RequestInit) => {
+      const u = String(url);
+      if (u.includes("/access_tokens")) return jsonResponse({ token: "ghs_test", expires_at: FUTURE_ISO });
+      if (/\/commits\/[^/]+\/statuses(?:\?|$)/.test(u)) {
+        statusReads += 1;
+        if (statusReads === 2) {
+          retryReadStarted();
+          await retryReleased;
+          return jsonResponse([]);
+        }
+        if (statusReads === 1) return jsonResponse([]);
+        return jsonResponse([
+          {
+            context: "review/ally-comment",
+            state: "success",
+            created_at: new Date().toISOString(),
+          },
+        ]);
+      }
+      if (/\/statuses\/[0-9a-f]{7,40}(?:\?|$)/i.test(u)) {
+        if (init?.body) postedBodies.push(JSON.parse(String(init.body)) as { state?: string; description?: string });
+        return jsonResponse({ id: postedBodies.length }, true, 201);
+      }
+      if (u.includes("/pulls/") && u.includes("/reviews")) return jsonResponse([]);
+      if (u.includes("/issues/") && u.includes("/comments")) return jsonResponse([]);
+      throw new Error(`unexpected url ${u}`);
+    });
+    vi.stubGlobal("fetch", fetchMock);
+
+    const retry = pollGitHubCommitStatusDeliveriesOnce(db);
+    await retryReadStarted;
+
+    // The newer evaluation shares the same lock and publishes while the
+    // worker is between its first freshness read and lock acquisition.
+    await withGithubStatusDeliveryLock(
+      db,
+      "Blockcast/hang#" + HEAD_SHA,
+      () => githubPostCommitStatusDetailed({
+        repoFullName: "Blockcast/hang",
+        sha: HEAD_SHA,
+        context: "review/ally-comment",
+        state: "success",
+        description: "new clean evaluation",
+        targetUrl: null,
+      }),
+    );
+    releaseRetryRead();
+    await retry;
+
+    expect(postedBodies).toEqual([{ state: "success", description: "new clean evaluation" }]);
+    expect(await readDelivery(delivery.id)).toMatchObject({
+      status: "skipped",
+      lastResult: { reason: "fresh_success_status_exists" },
+    });
+  });
+
   it("retries a GitHub rate-limited 403 status write instead of marking it permanent", async () => {
     setCreds();
     const { delivery } = await seedRun();
diff --git a/server/src/__tests__/pr-comment-review-gate-check.test.ts b/server/src/__tests__/pr-comment-review-gate-check.test.ts
index 302873a99618..90098b5c0ccf 100644
--- a/server/src/__tests__/pr-comment-review-gate-check.test.ts
+++ b/server/src/__tests__/pr-comment-review-gate-check.test.ts
@@ -3,8 +3,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 const h = vi.hoisted(() => ({
   cfg: {
     prCommentReviewGateStatusContext: "",
+    prCommentReviewGateRetiredStatusContexts: [] as string[],
     prReviewerBotLogin: "allyblockcast[bot]",
-  } as Record<string, string>,
+  } as Record<string, unknown> & {
+    prCommentReviewGateStatusContext: string;
+    prCommentReviewGateRetiredStatusContexts: string[];
+    prReviewerBotLogin: string;
+  },
 }));
 
 vi.mock("../config.js", () => ({ loadConfig: () => h.cfg }));
@@ -50,6 +55,7 @@ function blockingCommentFor(headSha: string) {
 
 beforeEach(() => {
   h.cfg.prCommentReviewGateStatusContext = "review/ally-comment-gate";
+  h.cfg.prCommentReviewGateRetiredStatusContexts = [];
   h.cfg.prReviewerBotLogin = "allyblockcast[bot]";
   mockListComments.mockReset();
   mockListReviews.mockReset();
@@ -196,3 +202,92 @@ describe("runPrCommentReviewGateCheck", () => {
     expect(mockPostStatus).not.toHaveBeenCalled();
   }, 10_000);
 });
+
+// BLO-29711 AC#1. The gate moved off `review/ally-comment` to
+// `gate/ally-comment-findings`, but commit statuses cannot be deleted: every
+// head already stamped with the old context keeps showing its fail-open green
+// forever (42 of 43 open penstock PRs, measured 2026-08-22). Only the
+// credential that wrote those rows can overwrite them, which is this App's
+// installation token — so the supersede has to ride the gate's own evaluations.
+describe("retired status contexts", () => {
+  beforeEach(() => {
+    h.cfg.prCommentReviewGateStatusContext = "gate/ally-comment-findings";
+    h.cfg.prCommentReviewGateRetiredStatusContexts = ["review/ally-comment"];
+    mockPostStatus.mockResolvedValue({ ok: true, statusCode: 201 });
+  });
+
+  function postFor(context: string) {
+    return mockPostStatus.mock.calls.map(([arg]) => arg).find((arg) => arg.context === context);
+  }
+
+  it("supersedes the retired context with a pointer carrying no not-evaluated claim", async () => {
+    // The exact pre-rename state: nothing attests the head, so the live gate
+    // legitimately goes green under `gate/`. The stale `review/` row must stop
+    // asserting that nothing reviewed the head.
+    await expect(runPrCommentReviewGateCheck(TARGET)).resolves.toMatchObject({
+      posted: true,
+      verdict: { state: "success", outcome: "not_evaluated" },
+    });
+
+    expect(postFor("gate/ally-comment-findings")).toMatchObject({
+      state: "success",
+      description: "No Ally consolidated-review comment attests to reviewing this head.",
+    });
+
+    const retired = postFor("review/ally-comment");
+    expect(retired).toMatchObject({ sha: TARGET.headSha, state: "success" });
+    // This is what the census greps for. A retirement pointer that still
+    // admitted "nothing attests" would leave AC#1 failing under the old name.
+    expect(retired?.description).not.toMatch(
+      /no Ally consolidated-review comment attests|no head SHA was supplied/i,
+    );
+    expect(retired?.description).toContain("gate/ally-comment-findings");
+    expect(retired?.description.length).toBeLessThanOrEqual(140);
+  });
+
+  it("does not overwrite the live verdict when the live context is also listed as retired", async () => {
+    // A misconfiguration that would otherwise replace a real `failure` with a
+    // green pointer — the exact fail-open this issue exists to remove.
+    h.cfg.prCommentReviewGateRetiredStatusContexts = [
+      "review/ally-comment",
+      "gate/ally-comment-findings",
+    ];
+    mockListReviews.mockResolvedValue([blockingCommentFor(TARGET.headSha)]);
+
+    await expect(runPrCommentReviewGateCheck(TARGET)).resolves.toMatchObject({
+      posted: true,
+      verdict: { state: "failure", outcome: "blocking_finding" },
+    });
+
+    const liveWrites = mockPostStatus.mock.calls
+      .map(([arg]) => arg)
+      .filter((arg) => arg.context === "gate/ally-comment-findings");
+    expect(liveWrites).toHaveLength(1);
+    expect(liveWrites[0]).toMatchObject({ state: "failure" });
+  });
+
+  it("reports retirement failure after publishing the live verdict", async () => {
+    // Cleanup of a superseded row must never overwrite the live signal, but a
+    // failed retirement write must remain visible so a later webhook can retry
+    // it instead of silently leaving a required legacy row stale.
+    mockPostStatus.mockImplementation(async ({ context }: { context: string }) =>
+      context === "review/ally-comment"
+        ? { ok: false, retryable: false, reason: "commit_status_write_http_403" }
+        : { ok: true, statusCode: 201 },
+    );
+
+    await expect(runPrCommentReviewGateCheck(TARGET)).resolves.toMatchObject({
+      posted: false,
+      reason: "post_failed",
+      postFailure: "review/ally-comment: commit_status_write_http_403",
+    });
+    expect(postFor("gate/ally-comment-findings")).toBeDefined();
+  });
+
+  it("writes nothing extra when no context is retired", async () => {
+    h.cfg.prCommentReviewGateRetiredStatusContexts = [];
+
+    await expect(runPrCommentReviewGateCheck(TARGET)).resolves.toMatchObject({ posted: true });
+    expect(mockPostStatus).toHaveBeenCalledTimes(1);
+  });
+});
diff --git a/server/src/__tests__/pr-comment-review-gate-deployment.test.ts b/server/src/__tests__/pr-comment-review-gate-deployment.test.ts
new file mode 100644
index 000000000000..166f7dcaaa8d
--- /dev/null
+++ b/server/src/__tests__/pr-comment-review-gate-deployment.test.ts
@@ -0,0 +1,66 @@
+import { describe, expect, it } from "vitest";
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
+
+/**
+ * BLO-29711 AC#1. The gate's status context moved out of the `review/`
+ * namespace, and the context it vacated is superseded in place rather than left
+ * showing its final fail-open green forever.
+ *
+ * These assertions are on the deployment wiring, not the logic — the logic is
+ * covered in pr-comment-review-gate{,-check}.test.ts. A typo in an env-var name
+ * here does not fail any of those: the server reads an unset variable, the
+ * feature is silently inert, and every test still passes. That is the same
+ * "green while nothing is happening" shape this issue exists to remove, so the
+ * name is pinned on both sides of the wire.
+ */
+const repoRoot = process.cwd();
+
+function read(relativePath: string): string {
+  return readFileSync(join(repoRoot, relativePath), "utf8");
+}
+
+const CONTEXT_ENV = "PAPERCLIP_PR_COMMENT_REVIEW_GATE_STATUS_CONTEXT";
+const RETIRED_ENV = "PAPERCLIP_PR_COMMENT_REVIEW_GATE_RETIRED_STATUS_CONTEXTS";
+
+describe("comment-review-gate deployment wiring", () => {
+  it("reads both env vars under the names the chart sets", () => {
+    const config = read("server/src/config.ts");
+
+    // Both directions: the reader names them, and the chart writes them.
+    expect(config).toContain(`process.env.${CONTEXT_ENV}`);
+    expect(config).toContain(`process.env.${RETIRED_ENV}`);
+
+    for (const template of ["deploy/helm/paperclip/templates/deployment-api.yaml", "deploy/helm/paperclip/templates/statefulset.yaml"]) {
+      const rendered = read(template);
+      expect(rendered, `${template} must set ${CONTEXT_ENV}`).toContain(`- name: ${CONTEXT_ENV}`);
+      expect(rendered, `${template} must set ${RETIRED_ENV}`).toContain(`- name: ${RETIRED_ENV}`);
+    }
+  });
+
+  it("publishes the Blockcast gate outside the review/ namespace", () => {
+    const values = read("deploy/helm/paperclip/values.blockcast.yaml");
+
+    // A green under `review/` reads as review evidence. This gate observes only
+    // the comment surface, so "nothing attests this head" is both common and
+    // legitimately green — the two cannot coexist under that namespace.
+    expect(values).toContain('prCommentReviewGateStatusContext: "gate/ally-comment-findings"');
+    expect(values).not.toMatch(/prCommentReviewGateStatusContext:\s*"review\//);
+  });
+
+  it("retires the context it moved off, so the stale green is superseded rather than frozen", () => {
+    const values = read("deploy/helm/paperclip/values.blockcast.yaml");
+
+    // Commit statuses cannot be deleted. Without this the pre-rename green
+    // stands on every head that already carries it — 42 of 43 open
+    // penstock-llm-proxy-core PRs when measured on 2026-08-22.
+    expect(values).toContain('prCommentReviewGateRetiredStatusContexts: "review/ally-comment"');
+  });
+
+  it("stays inert for deployments that never opted in", () => {
+    const values = read("deploy/helm/paperclip/values.yaml");
+
+    expect(values).toContain('prCommentReviewGateStatusContext: ""');
+    expect(values).toContain('prCommentReviewGateRetiredStatusContexts: ""');
+  });
+});
diff --git a/server/src/__tests__/pr-comment-review-gate.test.ts b/server/src/__tests__/pr-comment-review-gate.test.ts
index 81ed44cf5ab2..142e5dc77f35 100644
--- a/server/src/__tests__/pr-comment-review-gate.test.ts
+++ b/server/src/__tests__/pr-comment-review-gate.test.ts
@@ -1,8 +1,15 @@
 import { describe, expect, it } from "vitest";
 
+// @ts-expect-error -- plain-JS census script; imported for its own predicate so
+// the retirement description is checked against the real thing, not a copy.
+import { admitsNothingEvaluated } from "../../../scripts/check-comment-review-gate-census.mjs";
+
 import {
+  commentReviewGateRetirementDescription,
+  commentReviewGateRetirementStatus,
   commentReviewGateVerdictIsMisreadable,
   evaluateCommentReviewGate,
+  retiredCommentReviewGateContexts,
 } from "../services/pr-comment-review-gate.js";
 
 const OLD_HEAD = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
@@ -624,3 +631,83 @@ describe("evaluateCommentReviewGate", () => {
     expect(verdict).toMatchObject({ state: "failure" });
   });
 });
+
+// BLO-29711 AC#1. The deployed context moved out of the `review/` namespace so
+// a green can no longer be misread as review evidence. Because commit statuses
+// cannot be deleted, the pre-rename rows have to be superseded in place.
+describe("retired context supersede", () => {
+  const LIVE = "gate/ally-comment-findings";
+
+  it("excludes the live context so a retirement pointer cannot overwrite a real verdict", () => {
+    expect(retiredCommentReviewGateContexts(["review/ally-comment", LIVE], LIVE)).toEqual([
+      "review/ally-comment",
+    ]);
+    // Case and padding are how an operator typo actually looks.
+    expect(retiredCommentReviewGateContexts([" Gate/Ally-Comment-Findings "], LIVE)).toEqual([]);
+  });
+
+  it("drops blanks and duplicates", () => {
+    expect(
+      retiredCommentReviewGateContexts(
+        ["review/ally-comment", "  ", "review/ally-comment", ""],
+        LIVE,
+      ),
+    ).toEqual(["review/ally-comment"]);
+    expect(retiredCommentReviewGateContexts(undefined, LIVE)).toEqual([]);
+  });
+
+  it("points at the live context without claiming anything about review", () => {
+    const description = commentReviewGateRetirementDescription(LIVE);
+
+    expect(description).toContain(LIVE);
+    // Asserted against the census's own predicate rather than a copy of its
+    // regex, so the two cannot drift apart: if the census ever broadens what it
+    // treats as a not-evaluated admission, this fails instead of silently
+    // leaving AC#1 failing under the retired context name.
+    expect(admitsNothingEvaluated(description)).toBe(false);
+  });
+
+  // The retired context may still be a *required* check on a deployment that
+  // has not yet switched the requirement to the live context — BLO-26602 is
+  // that migration, and this code cannot read branch protection to find out.
+  // A fixed green here would satisfy the required legacy check while the live
+  // context reports a blocking finding, letting a PR merge with unresolved
+  // Critical/Important findings: the fail-open of this very issue, restored
+  // through the cleanup path.
+  it("never writes a green retirement row while the live verdict is blocking", () => {
+    for (const verdict of [
+      { state: "failure", outcome: "blocking_finding" },
+      { state: "failure", outcome: "carried_finding" },
+    ] as const) {
+      const retirement = commentReviewGateRetirementStatus(LIVE, verdict);
+
+      expect(retirement.state).toBe("failure");
+      expect(retirement.description).toContain(LIVE);
+      // Still a pointer, and still no not-evaluated claim under the retired
+      // `review/`-prefixed name.
+      expect(admitsNothingEvaluated(retirement.description)).toBe(false);
+    }
+  });
+
+  it("mirrors a clean live verdict rather than inventing a state", () => {
+    for (const outcome of ["clean", "not_evaluated"] as const) {
+      const retirement = commentReviewGateRetirementStatus(LIVE, { state: "success", outcome });
+
+      expect(retirement.state).toBe("success");
+      expect(admitsNothingEvaluated(retirement.description)).toBe(false);
+    }
+  });
+
+  it("keeps the pointer intact within GitHub's 140-character description limit", () => {
+    // GitHub truncates at 140. The context name is the whole point of the
+    // pointer, so it must survive rather than being cut mid-name.
+    const longContext = `gate/${"x".repeat(120)}`;
+
+    expect(commentReviewGateRetirementDescription(LIVE).length).toBeLessThanOrEqual(140);
+    expect(commentReviewGateRetirementDescription(longContext).length).toBeLessThanOrEqual(140);
+    expect(commentReviewGateRetirementDescription(LIVE, "failure").length).toBeLessThanOrEqual(140);
+    expect(
+      commentReviewGateRetirementDescription(longContext, "failure").length,
+    ).toBeLessThanOrEqual(140);
+  });
+});
diff --git a/server/src/config.ts b/server/src/config.ts
index 9856bc7a6a4e..2c91bd7d527f 100644
--- a/server/src/config.ts
+++ b/server/src/config.ts
@@ -235,6 +235,13 @@ export interface Config {
   // Commit-status context for comment-shaped Ally findings. Empty by default:
   // operators must opt in and make the context required in branch protection.
   prCommentReviewGateStatusContext: string;
+  // Contexts this gate used to publish to and has since moved off. GitHub
+  // commit statuses have no delete, so a context left behind by a rename keeps
+  // showing its final write forever. After posting the live status the gate
+  // also writes each of these a retirement pointer, which supersedes the stale
+  // row in place and keeps any repo that still requires the old context
+  // satisfied (BLO-29711).
+  prCommentReviewGateRetiredStatusContexts: string[];
   telemetryEnabled: boolean;
 }
 
@@ -1054,6 +1061,14 @@ export function loadConfig(): Config {
     githubReviewGateExpectedAppId,
     githubReviewGateExpectedInstallationId,
     prReviewGateStatusContext,
+    prCommentReviewGateRetiredStatusContexts: [
+      ...new Set(
+        (process.env.PAPERCLIP_PR_COMMENT_REVIEW_GATE_RETIRED_STATUS_CONTEXTS ?? "")
+          .split(",")
+          .map((value) => value.trim())
+          .filter(Boolean),
+      ),
+    ],
     telemetryEnabled: fileConfig?.telemetry?.enabled ?? true,
   };
 }
diff --git a/server/src/routes/github-webhook.ts b/server/src/routes/github-webhook.ts
index 35a7165db1a8..a42feb34fd04 100644
--- a/server/src/routes/github-webhook.ts
+++ b/server/src/routes/github-webhook.ts
@@ -73,6 +73,7 @@ import {
   hasAllyConsolidatedReviewHeading,
 } from "../services/ally-review-detection.js";
 import { runPrCommentReviewGateCheck } from "../services/pr-comment-review-gate.js";
+import { enqueueGithubCommitStatusDelivery } from "../services/github-status-delivery-outbox.js";
 import { recoveryService } from "../services/recovery/service.js";
 import {
   GITHUB_SUPPRESSION_CAUSE_REVIEWER_LOCK_CONTENDED,
@@ -4233,11 +4234,34 @@ export function githubWebhookRoutes(db: Db, config: GithubWebhookConfig) {
       config.prReviewerBotLogin,
     );
     if (commentReviewGateTrigger) {
-      void (config.runPrCommentReviewGateCheck ?? runPrCommentReviewGateCheck)(commentReviewGateTrigger)
+      const commentReviewGateCheck = config.runPrCommentReviewGateCheck
+        ? config.runPrCommentReviewGateCheck(commentReviewGateTrigger)
+        : runPrCommentReviewGateCheck({ ...commentReviewGateTrigger, db });
+      void commentReviewGateCheck
         .then((result) => {
           // The disabled default must be silent; otherwise every PR webhook in
           // a deployment that has not opted in would emit a warning.
           if (!result.posted && result.reason === "not_configured") return;
+          if (!result.posted && result.retirementDeliveries) {
+            void Promise.all(result.retirementDeliveries.map((delivery) =>
+              enqueueGithubCommitStatusDelivery(db, {
+                repoFullName: commentReviewGateTrigger.repoFullName,
+                sha: delivery.sha,
+                context: delivery.context,
+                state: delivery.state,
+                description: delivery.description,
+                targetUrl: delivery.targetUrl,
+                prNumber: commentReviewGateTrigger.prNumber,
+                prUrl: commentReviewGateTrigger.prUrl,
+                forceWrite: true,
+              }),
+            )).catch((err) => {
+              logger.error(
+                { err, event: eventName, deliveryId, ...commentReviewGateTrigger },
+                "github webhook comment-review retired-context retry enqueue failed",
+              );
+            });
+          }
           logger[result.posted ? "info" : "warn"](
             { deliveryId, event: eventName, ...commentReviewGateTrigger, result },
             "github webhook comment-review gate check completed",
diff --git a/server/src/services/github-status-delivery-outbox.ts b/server/src/services/github-status-delivery-outbox.ts
index 9ccdc40f8a80..d05769e26dda 100644
--- a/server/src/services/github-status-delivery-outbox.ts
+++ b/server/src/services/github-status-delivery-outbox.ts
@@ -30,13 +30,26 @@ const RETRY_DELAYS_MS = [
   2 * 60 * 60_000,
 ];
 
+/** Serialize the final read and external write for one GitHub status key. */
+export async function withGithubStatusDeliveryLock<T>(
+  db: Db,
+  key: string,
+  operation: () => Promise<T>,
+): Promise<T> {
+  return db.transaction(async (tx) => {
+    await tx.execute(sql`select pg_advisory_xact_lock(hashtextextended(${key}, 0))`);
+    return operation();
+  });
+}
+
 export type EnqueueGithubCommitStatusDeliveryInput = {
-  companyId: string;
-  sourceRunId: string;
+  companyId?: string | null;
+  sourceRunId?: string | null;
   repoFullName: string;
   sha: string;
   context: string;
   state: GitHubCommitStatusState;
+  forceWrite?: boolean;
   description: string;
   targetUrl?: string | null;
   prNumber: number;
@@ -149,6 +162,43 @@ async function handleFreshCommitStatusIfPresent(db: Db, row: DeliveryRow): Promi
   return false;
 }
 
+// A forced retirement retry may supersede the status that existed when it was
+// queued, but it must not overwrite any later evaluation of the same head.
+async function handleFreshSuccessForForcedDelivery(db: Db, row: DeliveryRow): Promise<boolean> {
+  if (!row.forceWrite) return false;
+  const latestStatus = await githubGetLatestCommitStatusForContext({
+    repoFullName: row.repoFullName,
+    sha: row.sha,
+    context: row.context,
+  });
+  if (!latestStatus.ok) {
+    if (latestStatus.retryable) {
+      await retryOrFailDelivery(db, row, latestStatus.reason, latestStatus);
+    } else {
+      await failPermanentDelivery(db, row, latestStatus.reason, latestStatus);
+    }
+    return true;
+  }
+
+  const latest = latestStatus.status;
+  const createdAt = latest?.createdAt ? Date.parse(latest.createdAt) : NaN;
+  if (statusCreatedAtOrAfterQueueSecond(createdAt, row.createdAt)) {
+    await markTerminal(
+      db,
+      row,
+      "skipped",
+      "info",
+      `Skipped forced retired-context retry for ${row.context} on ${row.repoFullName}@${row.sha.slice(0, 7)} because a newer status already exists`,
+      {
+        reason: latest?.state === "success" ? "fresh_success_status_exists" : "newer_status_exists",
+        latestStatus: latest,
+      },
+    );
+    return true;
+  }
+  return false;
+}
+
 async function appendDeliveryRunEvent(
   db: Db,
   row: DeliveryRow,
@@ -156,6 +206,8 @@ async function appendDeliveryRunEvent(
   message: string,
   payload: Record<string, unknown>,
 ): Promise<void> {
+  if (!row.sourceRunId) return;
+
   const run = await db
     .select({
       id: heartbeatRuns.id,
@@ -296,31 +348,35 @@ async function failPermanentDelivery(
 }
 
 async function processDelivery(db: Db, row: DeliveryRow): Promise<void> {
-  if (await handleFreshCommitStatusIfPresent(db, row)) return;
+  if (!row.forceWrite) {
+    if (await handleFreshCommitStatusIfPresent(db, row)) return;
 
-  const evidence = await githubHasReviewerEvidenceForPr({
-    repoFullName: row.repoFullName,
-    prNumber: row.prNumber,
-    headSha: row.sha,
-  });
-  if ("found" in evidence && evidence.found) {
-    await markTerminal(
-      db,
-      row,
-      "skipped",
-      "info",
-      `Skipped PR-review gate status failure for ${row.context} on ${row.repoFullName}#${row.prNumber}; reviewer evidence exists`,
-      { reason: "reviewer_evidence_found", via: evidence.via },
-    );
-    return;
-  }
-  if ("error" in evidence) {
-    const classified = classifyReviewerEvidenceError(evidence.error);
-    if (classified.retryable) {
-      await retryOrFailDelivery(db, row, classified.reason, { evidence });
-    } else {
-      await failPermanentDelivery(db, row, classified.reason, { evidence });
+    const evidence = await githubHasReviewerEvidenceForPr({
+      repoFullName: row.repoFullName,
+      prNumber: row.prNumber,
+      headSha: row.sha,
+    });
+    if ("found" in evidence && evidence.found) {
+      await markTerminal(
+        db,
+        row,
+        "skipped",
+        "info",
+        `Skipped PR-review gate status failure for ${row.context} on ${row.repoFullName}#${row.prNumber}; reviewer evidence exists`,
+        { reason: "reviewer_evidence_found", via: evidence.via },
+      );
+      return;
+    }
+    if ("error" in evidence) {
+      const classified = classifyReviewerEvidenceError(evidence.error);
+      if (classified.retryable) {
+        await retryOrFailDelivery(db, row, classified.reason, { evidence });
+      } else {
+        await failPermanentDelivery(db, row, classified.reason, { evidence });
+      }
+      return;
     }
+  } else if (await handleFreshSuccessForForcedDelivery(db, row)) {
     return;
   }
 
@@ -332,7 +388,8 @@ async function processDelivery(db: Db, row: DeliveryRow): Promise<void> {
     );
     return;
   }
-  if (await handleFreshCommitStatusIfPresent(db, fencedRow)) return;
+  if (await handleFreshSuccessForForcedDelivery(db, fencedRow)) return;
+  if (!fencedRow.forceWrite && (await handleFreshCommitStatusIfPresent(db, fencedRow))) return;
 
   const postingRow = await refreshDeliveryClaimBeforeExternalWrite(db, fencedRow);
   if (!postingRow) {
@@ -344,7 +401,7 @@ async function processDelivery(db: Db, row: DeliveryRow): Promise<void> {
   }
   fencedRow = postingRow;
 
-  const posted = await githubPostCommitStatusDetailed({
+  const post = async () => githubPostCommitStatusDetailed({
     repoFullName: fencedRow.repoFullName,
     sha: fencedRow.sha,
     context: fencedRow.context,
@@ -352,6 +409,21 @@ async function processDelivery(db: Db, row: DeliveryRow): Promise<void> {
     description: fencedRow.description,
     targetUrl: fencedRow.targetUrl,
   });
+  const posted = fencedRow.forceWrite
+    ? await withGithubStatusDeliveryLock(
+        db,
+        `${fencedRow.repoFullName}#${fencedRow.sha}`,
+        async () => {
+          // The lock is shared with the live gate evaluation. Re-check inside
+          // it so a clean evaluation that won the lock cannot be overwritten.
+          if (await handleFreshSuccessForForcedDelivery(db, fencedRow)) {
+            return { ok: true as const, skipped: true as const };
+          }
+          return { ...(await post()), skipped: false as const };
+        },
+      )
+    : { ...(await post()), skipped: false as const };
+  if (posted.skipped) return;
   if (posted.ok) {
     await markTerminal(
       db,
@@ -390,6 +462,7 @@ export async function enqueueGithubCommitStatusDelivery(
     sha: input.sha,
     context: input.context,
     state: input.state,
+    forceWrite: input.forceWrite ?? false,
     description: input.description.slice(0, 140),
     targetUrl: input.targetUrl ?? null,
     prNumber: input.prNumber,
@@ -419,6 +492,7 @@ export async function enqueueGithubCommitStatusDelivery(
         prNumber: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.prNumber} else ${input.prNumber} end`,
         prUrl: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.prUrl} else ${input.prUrl ?? null} end`,
         state: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.state} else ${input.state} end`,
+        forceWrite: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.forceWrite} else ${input.forceWrite ?? false} end`,
         description: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.description} else ${input.description.slice(0, 140)} end`,
         targetUrl: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.targetUrl} else ${input.targetUrl ?? null} end`,
         status: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.status} else 'queued' end`,
diff --git a/server/src/services/pr-comment-review-gate.ts b/server/src/services/pr-comment-review-gate.ts
index dc8af8404e02..2804442e7487 100644
--- a/server/src/services/pr-comment-review-gate.ts
+++ b/server/src/services/pr-comment-review-gate.ts
@@ -12,6 +12,8 @@
  * gate unable to observe any real review (BLO-29711).
  */
 import { loadConfig } from "../config.js";
+import type { Db } from "@paperclipai/db";
+import { withGithubStatusDeliveryLock } from "./github-status-delivery-outbox.js";
 import {
   extractAllyPriorFindingDispositions,
   extractAllyReportedFindingRefs,
@@ -372,10 +374,11 @@ export function evaluateCommentReviewGate(input: {
  * A green status published under a `review/`-prefixed context reads as "this
  * head was reviewed and was clean". For the `not_evaluated` outcome that
  * reading is false, and no state can fix it: `pending`/`failure` on absence
- * would deadlock every formally-reviewed PR. The only remedy is to publish
- * outside the `review/` namespace, which is a branch-protection-coupled
- * change. Until then this predicate names the condition so it can be asserted
- * against and logged rather than silently shipped (BLO-29711).
+ * would deadlock every formally-reviewed PR. The remedy is to publish outside
+ * the `review/` namespace — done for the Blockcast deployment, whose live
+ * context is now `gate/ally-comment-findings`. This predicate stays as the
+ * assertion point so a future config change cannot silently move the gate back
+ * under `review/` (BLO-29711).
  */
 export function commentReviewGateVerdictIsMisreadable(
   verdict: CommentReviewGateVerdict,
@@ -388,15 +391,108 @@ export function commentReviewGateVerdictIsMisreadable(
   );
 }
 
+/**
+ * Contexts to supersede with a retirement pointer, given the live context.
+ *
+ * The live context is excluded even if an operator also lists it as retired:
+ * writing a retirement pointer over the verdict we just published would
+ * replace a real `failure` with a green, which is the exact fail-open this
+ * issue exists to remove.
+ */
+export function retiredCommentReviewGateContexts(
+  retired: readonly string[] | null | undefined,
+  liveContext: string,
+): string[] {
+  const live = liveContext.trim().toLowerCase();
+  const seen = new Set<string>();
+  const result: string[] = [];
+  for (const raw of retired ?? []) {
+    const context = raw?.trim();
+    if (!context) continue;
+    const key = context.toLowerCase();
+    if (key === live || seen.has(key)) continue;
+    seen.add(key);
+    result.push(context);
+  }
+  return result;
+}
+
+// GitHub truncates commit-status descriptions at 140 characters. The pointer to
+// the live context is the entire value of a retirement write, so fall back to a
+// shorter phrasing rather than letting the context name be cut in half.
+const MAX_COMMIT_STATUS_DESCRIPTION = 140;
+
+/**
+ * Description for a superseded context. Deliberately carries no claim about
+ * whether anything reviewed the head — that claim under a `review/`-prefixed
+ * green is the defect (BLO-29711) — only a pointer to where the verdict now
+ * lives. `scripts/check-comment-review-gate-census.mjs` flags a green `review/`
+ * status whose description admits nothing was evaluated; this text must not
+ * match that pattern.
+ *
+ * The blocking phrasing exists because the retirement write mirrors the live
+ * state (see `supersedeRetiredContexts`). A red row whose description only said
+ * "retired" would read as the retirement itself having failed.
+ */
+export function commentReviewGateRetirementDescription(
+  liveContext: string,
+  state: CommentReviewGateVerdict["state"] = "success",
+): string {
+  const target = liveContext.trim();
+  const [full, short] =
+    state === "failure"
+      ? [
+          `Retired. Unresolved finding stands; "${target}" carries the verdict.`,
+          `Retired. Unresolved finding; see "${target}".`,
+        ]
+      : [
+          `Retired. Comment-shaped review findings now publish to "${target}".`,
+          `Retired. Findings now publish to "${target}".`,
+        ];
+  if (full.length <= MAX_COMMIT_STATUS_DESCRIPTION) return full;
+  return short.slice(0, MAX_COMMIT_STATUS_DESCRIPTION);
+}
+
+/**
+ * The status row to write over a retired context, given the live verdict.
+ *
+ * Split out as a pure function so the mirroring invariant is testable without
+ * standing up the GitHub client: "a blocking live verdict never produces a
+ * green retirement row" is the property that keeps a still-required legacy
+ * context from being satisfied while the live one blocks. See
+ * `supersedeRetiredContexts` for why that case is reachable.
+ */
+export function commentReviewGateRetirementStatus(
+  liveContext: string,
+  verdict: Pick<CommentReviewGateVerdict, "state">,
+): { state: CommentReviewGateVerdict["state"]; description: string } {
+  return {
+    state: verdict.state,
+    description: commentReviewGateRetirementDescription(liveContext, verdict.state),
+  };
+}
+
 export type PrCommentReviewGateCheckResult =
   | { posted: true; verdict: CommentReviewGateVerdict }
-  | { posted: false; reason: "not_configured" | "fetch_failed" | "post_failed"; postFailure?: string };
+  | {
+      posted: false;
+      reason: "not_configured" | "fetch_failed" | "post_failed";
+      postFailure?: string;
+      retirementDeliveries?: Array<{
+        sha: string;
+        context: string;
+        state: CommentReviewGateVerdict["state"];
+        description: string;
+        targetUrl: string | null;
+      }>;
+    };
 
 export interface PrCommentReviewGateCheckInput {
   repoFullName: string;
   prNumber: number;
   headSha?: string | null;
   prUrl?: string | null;
+  db?: Db;
 }
 
 const TRANSIENT_RETRY_DELAYS_MS = [250, 1000];
@@ -510,19 +606,120 @@ async function executeCommentReviewGateCheck(
 
   warnOnceIfMisreadableContext(verdict, context);
 
-  const posted = await withBoundedRetry<GitHubCommitStatusPostResult>(
-    () =>
-      githubPostCommitStatusDetailed({
-        repoFullName: input.repoFullName,
-        sha: headSha,
-        context,
-        state: verdict.state,
-        description: verdict.reason,
-        targetUrl: input.prUrl ?? null,
-      }),
-    (result) => !result.ok && result.retryable,
-  );
-  if (!posted.ok) return { posted: false, reason: "post_failed", postFailure: posted.reason };
+  const publish = async (): Promise<PrCommentReviewGateCheckResult> => {
+    const posted = await withBoundedRetry<GitHubCommitStatusPostResult>(
+      () =>
+        githubPostCommitStatusDetailed({
+          repoFullName: input.repoFullName,
+          sha: headSha,
+          context,
+          state: verdict.state,
+          description: verdict.reason,
+          targetUrl: input.prUrl ?? null,
+        }),
+      (result) => !result.ok && result.retryable,
+    );
+    if (!posted.ok) return { posted: false, reason: "post_failed", postFailure: posted.reason };
+
+    const retirementFailures = await supersedeRetiredContexts(input, headSha, context, config, verdict);
+    if (retirementFailures.length > 0) {
+      return {
+        posted: false,
+        reason: "post_failed",
+        postFailure: retirementFailures.map((failure) => `${failure.context}: ${failure.reason}`).join(", "),
+        retirementDeliveries: retirementFailures.map((failure) => ({
+          sha: headSha,
+          context: failure.context,
+          state: failure.state,
+          description: failure.description,
+          targetUrl: input.prUrl ?? null,
+        })),
+      };
+    }
 
-  return { posted: true, verdict };
+    return { posted: true, verdict };
+  };
+
+  // Serialize the live verdict and retired-context writes with forced retries.
+  return input.db
+    ? withGithubStatusDeliveryLock(input.db, `${input.repoFullName}#${headSha}`, publish)
+    : publish();
+}
+
+/**
+ * Overwrite each retired context with a pointer to the live one.
+ *
+ * Why this is code in the gate rather than a one-shot sweep. GitHub's Commit
+ * Statuses API has create and list but no delete, so renaming the context
+ * cannot retract what was already written under the old name: every head that
+ * carries the old fail-open green keeps carrying it. Measured 2026-08-22, 42 of
+ * 43 open PRs in Blockcast/penstock-llm-proxy-core were in exactly that state.
+ * Only the credential that wrote those rows can overwrite them — the App's own
+ * installation token, the one used here — so an operator script cannot do it.
+ * Riding the gate's existing evaluations reaches each PR the next time it is
+ * evaluated, with no sweep and no human chore.
+ *
+ * State mirrors the live verdict rather than being a fixed `success`. A retired
+ * context is not necessarily a powerless one: an operator may still have it in
+ * required checks while the new context is not yet required (BLO-26602 is
+ * exactly that migration), and this code cannot see branch protection to find
+ * out — the App gets 403 on that endpoint. An unconditional green would then
+ * satisfy the still-required legacy check while the live context reports a
+ * blocking finding, letting a PR with unresolved Critical/Important findings
+ * merge: the same fail-open this issue exists to remove, reintroduced through
+ * the cleanup path. Mirroring costs nothing where the context is already
+ * non-required (the row is informational either way) and preserves the block
+ * where it is not. It also never paints a PR red that the live context is not
+ * already painting red, which was the original argument for a fixed `success`.
+ *
+ * Best-effort by construction: the live verdict is already published, and
+ * failing the check over cleanup of a superseded row would let a retired
+ * context break the live one.
+ */
+async function supersedeRetiredContexts(
+  input: PrCommentReviewGateCheckInput,
+  headSha: string,
+  liveContext: string,
+  config: ReturnType<typeof loadConfig>,
+  verdict: CommentReviewGateVerdict,
+): Promise<Array<{ context: string; reason: string; state: CommentReviewGateVerdict["state"]; description: string }>> {
+  const retiredContexts = retiredCommentReviewGateContexts(
+    config.prCommentReviewGateRetiredStatusContexts,
+    liveContext,
+  );
+  if (retiredContexts.length === 0) return [];
+
+  const retirement = commentReviewGateRetirementStatus(liveContext, verdict);
+  const failures = await Promise.all(
+    retiredContexts.map(async (retiredContext) => {
+      const post = () =>
+        withBoundedRetry<GitHubCommitStatusPostResult>(
+          () =>
+            githubPostCommitStatusDetailed({
+              repoFullName: input.repoFullName,
+              sha: headSha,
+              context: retiredContext,
+              state: retirement.state,
+              description: retirement.description,
+              targetUrl: input.prUrl ?? null,
+            }),
+          (attempt) => !attempt.ok && attempt.retryable,
+        );
+      const result = await post();
+      if (!result.ok) {
+        console.warn(
+          `[pr-comment-review-gate] Could not supersede retired context "${retiredContext}" on ` +
+            `${input.repoFullName}@${headSha.slice(0, 7)}: ${result.reason}. Queuing a durable retry.`,
+        );
+        return {
+          context: retiredContext,
+          reason: result.reason,
+          state: retirement.state,
+          description: retirement.description,
+        };
+      }
+      return null;
+    }),
+  );
+  return failures.filter((failure): failure is NonNullable<typeof failure> => failure !== null);
 }

From 2eee1ba7f5b512ca853d650f22583a7460fdd654 Mon Sep 17 00:00:00 2001
From: CTO <cto@paperclip.blockcast.net>
Date: Fri, 28 Aug 2026 10:27:23 +0000
Subject: [PATCH 2/7] fix(review-gate): lock evidence evaluation

---
 .../pr-comment-review-gate-check.test.ts      | 27 ++++++++
 server/src/services/pr-comment-review-gate.ts | 63 +++++++++----------
 2 files changed, 57 insertions(+), 33 deletions(-)

diff --git a/server/src/__tests__/pr-comment-review-gate-check.test.ts b/server/src/__tests__/pr-comment-review-gate-check.test.ts
index 90098b5c0ccf..7715c1b3ee5b 100644
--- a/server/src/__tests__/pr-comment-review-gate-check.test.ts
+++ b/server/src/__tests__/pr-comment-review-gate-check.test.ts
@@ -18,6 +18,7 @@ const mockListComments = vi.hoisted(() => vi.fn());
 const mockListReviews = vi.hoisted(() => vi.fn());
 const mockFetchHeadSha = vi.hoisted(() => vi.fn());
 const mockPostStatus = vi.hoisted(() => vi.fn());
+const mockStatusDeliveryLock = vi.hoisted(() => vi.fn());
 
 vi.mock("../services/github-app-auth.js", () => ({
   githubFetchPrHeadSha: mockFetchHeadSha,
@@ -36,6 +37,10 @@ vi.mock("../services/github-app-auth.js", () => ({
   },
 }));
 
+vi.mock("../services/github-status-delivery-outbox.js", () => ({
+  withGithubStatusDeliveryLock: mockStatusDeliveryLock,
+}));
+
 import { runPrCommentReviewGateCheck } from "../services/pr-comment-review-gate.js";
 
 const TARGET = {
@@ -61,6 +66,8 @@ beforeEach(() => {
   mockListReviews.mockReset();
   mockFetchHeadSha.mockReset();
   mockPostStatus.mockReset();
+  mockStatusDeliveryLock.mockReset();
+  mockStatusDeliveryLock.mockImplementation(async (_db, _key, operation) => operation());
   // Default both surfaces to empty; each test overrides the one it exercises.
   mockListComments.mockResolvedValue([]);
   mockListReviews.mockResolvedValue([]);
@@ -121,6 +128,26 @@ describe("runPrCommentReviewGateCheck", () => {
     expect(mockPostStatus).toHaveBeenCalledTimes(2);
   }, 10_000);
 
+  it("reads and evaluates evidence inside the shared delivery lock", async () => {
+    const events: string[] = [];
+    mockStatusDeliveryLock.mockImplementation(async (_db, _key, operation) => {
+      events.push("lock");
+      return operation();
+    });
+    mockListComments.mockImplementation(async () => {
+      events.push("fetch-comments");
+      return [blockingCommentFor(TARGET.headSha)];
+    });
+    mockPostStatus.mockImplementation(async () => {
+      events.push("post");
+      return { ok: true, statusCode: 201 };
+    });
+
+    await runPrCommentReviewGateCheck({ ...TARGET, db: {} as never });
+
+    expect(events).toEqual(["lock", "fetch-comments", "post"]);
+  });
+
   it("serializes overlapping evaluations for one PR/context", async () => {
     const events: string[] = [];
     let releaseFirstFetch!: () => void;
diff --git a/server/src/services/pr-comment-review-gate.ts b/server/src/services/pr-comment-review-gate.ts
index 2804442e7487..158b9d4ed351 100644
--- a/server/src/services/pr-comment-review-gate.ts
+++ b/server/src/services/pr-comment-review-gate.ts
@@ -574,39 +574,35 @@ async function executeCommentReviewGateCheck(
     ));
   if (!headSha) return { posted: false, reason: "fetch_failed" };
 
-  // Both surfaces, because Ally uses whichever is available to it: a
-  // `COMMENTED` pull_request_review on `/pulls/{n}/reviews`, or a plain issue
-  // comment. Measured over the 25 most recent PRs in this repo, 33 of 33
-  // consolidated reviews were reviews-API objects and none were issue
-  // comments, so reading only the latter made this gate structurally unable to
-  // observe a review (BLO-29711). Either surface failing to read leaves the
-  // prior status untouched rather than publishing a verdict from half the
-  // history.
-  const [issueComments, prReviews] = await Promise.all([
-    withBoundedRetry(
-      () => githubListIssueCommentsWithTimestamps({ repoFullName: input.repoFullName, prNumber: input.prNumber }),
-      (result) => result == null,
-    ),
-    withBoundedRetry(
-      () => githubListPrReviewsWithTimestamps({ repoFullName: input.repoFullName, prNumber: input.prNumber }),
-      (result) => result == null,
-    ),
-  ]);
-  if (issueComments == null || prReviews == null) return { posted: false, reason: "fetch_failed" };
-
-  const verdict = evaluateCommentReviewGate({
-    comments: [...issueComments, ...prReviews].map((comment) => ({
-      authorLogin: comment.login,
-      body: comment.body,
-      createdAt: comment.createdAt,
-    })),
-    headSha,
-    reviewerBotLogin,
-  });
-
-  warnOnceIfMisreadableContext(verdict, context);
-
   const publish = async (): Promise<PrCommentReviewGateCheckResult> => {
+    // Both surfaces, because Ally uses whichever is available to it: a
+    // `COMMENTED` pull_request_review on `/pulls/{n}/reviews`, or a plain issue
+    // comment. Read and evaluate them inside the shared lock. Otherwise two
+    // API pods can compute against different snapshots and publish an older
+    // verdict after a newer one (BLO-29711).
+    const [issueComments, prReviews] = await Promise.all([
+      withBoundedRetry(
+        () => githubListIssueCommentsWithTimestamps({ repoFullName: input.repoFullName, prNumber: input.prNumber }),
+        (result) => result == null,
+      ),
+      withBoundedRetry(
+        () => githubListPrReviewsWithTimestamps({ repoFullName: input.repoFullName, prNumber: input.prNumber }),
+        (result) => result == null,
+      ),
+    ]);
+    if (issueComments == null || prReviews == null) return { posted: false, reason: "fetch_failed" };
+
+    const verdict = evaluateCommentReviewGate({
+      comments: [...issueComments, ...prReviews].map((comment) => ({
+        authorLogin: comment.login,
+        body: comment.body,
+        createdAt: comment.createdAt,
+      })),
+      headSha,
+      reviewerBotLogin,
+    });
+
+    warnOnceIfMisreadableContext(verdict, context);
     const posted = await withBoundedRetry<GitHubCommitStatusPostResult>(
       () =>
         githubPostCommitStatusDetailed({
@@ -640,7 +636,8 @@ async function executeCommentReviewGateCheck(
     return { posted: true, verdict };
   };
 
-  // Serialize the live verdict and retired-context writes with forced retries.
+  // Serialize evidence reads, verdict computation, and all status writes with
+  // forced retries. The transaction-scoped lock is the cross-process boundary.
   return input.db
     ? withGithubStatusDeliveryLock(input.db, `${input.repoFullName}#${headSha}`, publish)
     : publish();

From 35e1352bb1ff6dbd704248aa3fb56bda8725dbc0 Mon Sep 17 00:00:00 2001
From: CTO <cto@paperclip.blockcast.net>
Date: Thu, 3 Sep 2026 07:42:07 +0000
Subject: [PATCH 3/7] test(review-gate): fix two self-inflicted defects in the
 gate's own tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Both were introduced by this branch and both failed silently in the way
BLO-29711 is about — a test that does not exercise what it claims.

github-status-delivery-outbox: the interleaving test swapped its two
promise/resolver pairs, so the fetch mock called a `Promise` as a function
and the resolver that unblocks the worker was never invoked. The test
deadlocked and was only visible as a 60s timeout, which an earlier note on
this branch mis-attributed to a pre-existing failure. `tsc` cannot catch it
because `server/tsconfig.json` excludes `src/__tests__`. Also assert the
exact posted body, so a post to the wrong context cannot pass.

pr-comment-review-gate-deployment: `repoRoot` came from `process.cwd()`, so
the reads resolved only when vitest was started from the repo root and threw
ENOENT from `server/`. Anchor to `import.meta.url` and reject an empty read,
which would otherwise satisfy every `not.toContain` assertion vacuously.

Co-Authored-By: Claude <noreply@anthropic.com>
---
 .../github-status-delivery-outbox.test.ts      | 18 +++++++++++-------
 .../pr-comment-review-gate-deployment.test.ts  | 15 +++++++++++++--
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/server/src/__tests__/github-status-delivery-outbox.test.ts b/server/src/__tests__/github-status-delivery-outbox.test.ts
index e4d6fff7ff7b..4561a435d5f7 100644
--- a/server/src/__tests__/github-status-delivery-outbox.test.ts
+++ b/server/src/__tests__/github-status-delivery-outbox.test.ts
@@ -702,14 +702,14 @@ describeEmbeddedPostgres("GitHub commit-status delivery outbox", () => {
     const retryReleased = new Promise<void>((resolve) => {
       releaseRetry = resolve;
     });
-    const postedBodies: Array<{ state?: string; description?: string }> = [];
+    const postedBodies: Array<{ state?: string; context?: string; description?: string }> = [];
     const fetchMock = vi.fn(async (url: string | URL, init?: RequestInit) => {
       const u = String(url);
       if (u.includes("/access_tokens")) return jsonResponse({ token: "ghs_test", expires_at: FUTURE_ISO });
       if (/\/commits\/[^/]+\/statuses(?:\?|$)/.test(u)) {
         statusReads += 1;
         if (statusReads === 2) {
-          retryReadStarted();
+          releaseRetryRead();
           await retryReleased;
           return jsonResponse([]);
         }
@@ -723,7 +723,7 @@ describeEmbeddedPostgres("GitHub commit-status delivery outbox", () => {
         ]);
       }
       if (/\/statuses\/[0-9a-f]{7,40}(?:\?|$)/i.test(u)) {
-        if (init?.body) postedBodies.push(JSON.parse(String(init.body)) as { state?: string; description?: string });
+        if (init?.body) postedBodies.push(JSON.parse(String(init.body)) as { state?: string; context?: string; description?: string });
         return jsonResponse({ id: postedBodies.length }, true, 201);
       }
       if (u.includes("/pulls/") && u.includes("/reviews")) return jsonResponse([]);
@@ -735,8 +735,10 @@ describeEmbeddedPostgres("GitHub commit-status delivery outbox", () => {
     const retry = pollGitHubCommitStatusDeliveriesOnce(db);
     await retryReadStarted;
 
-    // The newer evaluation shares the same lock and publishes while the
-    // worker is between its first freshness read and lock acquisition.
+    // The newer evaluation shares the same lock and publishes while the worker
+    // is paused inside its pre-lock freshness re-check, i.e. after it has
+    // claimed the row but before it holds the advisory lock. Asserted as the
+    // exact body so a post to the wrong context cannot pass.
     await withGithubStatusDeliveryLock(
       db,
       "Blockcast/hang#" + HEAD_SHA,
@@ -749,10 +751,12 @@ describeEmbeddedPostgres("GitHub commit-status delivery outbox", () => {
         targetUrl: null,
       }),
     );
-    releaseRetryRead();
+    releaseRetry();
     await retry;
 
-    expect(postedBodies).toEqual([{ state: "success", description: "new clean evaluation" }]);
+    expect(postedBodies).toEqual([
+      { state: "success", context: "review/ally-comment", description: "new clean evaluation" },
+    ]);
     expect(await readDelivery(delivery.id)).toMatchObject({
       status: "skipped",
       lastResult: { reason: "fresh_success_status_exists" },
diff --git a/server/src/__tests__/pr-comment-review-gate-deployment.test.ts b/server/src/__tests__/pr-comment-review-gate-deployment.test.ts
index 166f7dcaaa8d..427233c42725 100644
--- a/server/src/__tests__/pr-comment-review-gate-deployment.test.ts
+++ b/server/src/__tests__/pr-comment-review-gate-deployment.test.ts
@@ -1,6 +1,7 @@
 import { describe, expect, it } from "vitest";
 import { readFileSync } from "node:fs";
 import { join } from "node:path";
+import { fileURLToPath } from "node:url";
 
 /**
  * BLO-29711 AC#1. The gate's status context moved out of the `review/`
@@ -14,10 +15,20 @@ import { join } from "node:path";
  * "green while nothing is happening" shape this issue exists to remove, so the
  * name is pinned on both sides of the wire.
  */
-const repoRoot = process.cwd();
+// Resolved from this file, not from `process.cwd()`. Vitest is invoked from the
+// repo root by `pnpm test` and from `server/` by a filtered run, so a cwd-based
+// root makes these assertions pass or throw ENOENT depending on how the suite
+// was started — the reads must be anchored to the source tree instead.
+const repoRoot = fileURLToPath(new URL("../../..", import.meta.url));
 
 function read(relativePath: string): string {
-  return readFileSync(join(repoRoot, relativePath), "utf8");
+  const contents = readFileSync(join(repoRoot, relativePath), "utf8");
+  // An empty read would satisfy every `not.toContain` assertion below, so a
+  // future path regression must fail loudly rather than vacuously pass.
+  if (contents.trim().length === 0) {
+    throw new Error(`${relativePath} resolved to an empty file under ${repoRoot}`);
+  }
+  return contents;
 }
 
 const CONTEXT_ENV = "PAPERCLIP_PR_COMMENT_REVIEW_GATE_STATUS_CONTEXT";

From 114cc6948942daedee0166e6788986d0845a3bdb Mon Sep 17 00:00:00 2001
From: CTO <cto@paperclip.blockcast.net>
Date: Fri, 4 Sep 2026 07:01:34 +0000
Subject: [PATCH 4/7] fix(review-gate): address three Important findings from
 Ally review 5108460067
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

1. values.blockcast.yaml said `prReviewGateStatusContext` "is intentionally
   left unset here" while setting it to `review/ally-complete` twelve lines
   above, and asserted "nothing in the server ever posts success for it" next
   to a retained comment claiming the durable authority owns "its success
   writer". An operator reading it concluded the legacy context was inert while
   it was set and being published.

   Verified the actual writers before rewriting: heartbeat.ts's
   queueFailedPrReviewGateStatus posts state=failure only, and
   github-review-gate-authority.ts posts state=pending only, then hands off via
   repository_dispatch to the Penstock consumer — which is the sole success
   writer, and is external. So "no in-repo success writer" was true, "unset"
   and "only writer" were not. The NOTE now describes the value actually
   configured, and the namespace rationale moved up to sit above
   prCommentReviewGateStatusContext, the key it actually describes.

2. withGithubStatusDeliveryLock typed `operation` as `() => Promise<T>` and
   never threaded `tx`, so the forced-delivery path took a second pool
   connection inside the advisory-lock transaction. Combined with the gate
   holding that same lock across paginated GitHub reads and status POSTs, a
   saturated pool could leave the holder unable to finish, so the lock was
   never released. `operation` now receives `tx` and the in-lock re-check uses
   it, and the transaction sets lock_timeout / idle_in_transaction_session_
   timeout so neither waiting nor holding is unbounded.

3. `db` was optional on PrCommentReviewGateCheckInput and the publish path fell
   through to an unsynchronized write when absent, silently reopening the
   out-of-order-verdict race for any caller that forgot it. It is now required,
   the lock is unconditional, and a runtime guard fails closed rather than
   publishing a verdict nobody serialized — the type alone is not enough, since
   server/tsconfig.json excludes src/__tests__.

   The webhook seam at github-webhook.ts now forwards `db` to both branches, so
   the existing webhook test can assert production supplies the handle; it
   previously asserted the exact opposite (an argument with no `db`).

Refs BLO-29711
---
 deploy/helm/paperclip/values.blockcast.yaml   | 53 +++++++++++--------
 server/src/__tests__/github-webhook.test.ts   | 12 ++++-
 .../pr-comment-review-gate-check.test.ts      | 20 +++++++
 server/src/routes/github-webhook.ts           |  9 +++-
 .../services/github-status-delivery-outbox.ts | 53 +++++++++++++++----
 server/src/services/pr-comment-review-gate.ts | 28 ++++++++--
 6 files changed, 133 insertions(+), 42 deletions(-)

diff --git a/deploy/helm/paperclip/values.blockcast.yaml b/deploy/helm/paperclip/values.blockcast.yaml
index 7e183c21e2bb..4ef1dd469de2 100644
--- a/deploy/helm/paperclip/values.blockcast.yaml
+++ b/deploy/helm/paperclip/values.blockcast.yaml
@@ -417,12 +417,19 @@ githubApp:
   # contexts it has observed recently, and this repo has never carried a commit
   # status, so the context has to be posted at least once before a human can
   # select it. Until then this is observe-only.
-  # This gate intentionally stays outside `review/`: it is not review evidence.
+  #
+  # Deliberately NOT under `review/` (BLO-29711). This gate reads only the
+  # comment-shaped surface, so "nothing attests this head" is a legitimate and
+  # common outcome for a formally-reviewed PR, and it must stay green there or
+  # every such PR deadlocks. A green under `review/` reads as review evidence,
+  # which this gate cannot supply — so the fail-open and the namespace cannot
+  # coexist. Moving the namespace is the half that is safe to change.
+  # Verified before renaming: `review/ally-comment` was not a required check on
+  # paperclip `master` (required: `verify`) nor on penstock-llm-proxy-core
+  # `main` (required: validate-and-build, secret-scan, redaction-tests,
+  # secrets-controls-static-check, review/ally-complete), so this drops nothing.
   prCommentReviewGateStatusContext: "gate/ally-comment-findings"
   prCommentReviewGateRetiredStatusContexts: "review/ally-comment"
-  # NOTE: `prReviewGateStatusContext` is shared by the legacy failure status
-  # and the durable review-gate authority below. The latter owns
-  # `review/ally-complete`, including its success writer.
   # PEN-2073 rollout order:
   # 1. Freeze review-gate merges.
   # 2. Deploy this producer migration/code with both flags false; wait for every pod.
@@ -440,25 +447,27 @@ githubApp:
     - Blockcast/penstock-llm-proxy-core
   reviewGateExpectedAppId: "3966421"
   reviewGateExpectedInstallationId: "138085375"
+  # NOTE: `prReviewGateStatusContext` IS set here (below), and it is required to
+  # be: the chart hard-fails when `reviewGateCaptureEnabled` is true and this is
+  # empty (templates/_helpers.tpl). Two server writers share the one field, and
+  # neither posts success:
+  #   * the legacy failure writer — `queueFailedPrReviewGateStatus` in
+  #     server/src/services/heartbeat.ts — posts state=failure only (reviewer
+  #     chain exhausted / non-retryable external lifecycle end);
+  #   * the durable review-gate authority —
+  #     server/src/services/github-review-gate-authority.ts — posts
+  #     state=pending only, then hands off via `repository_dispatch`
+  #     (`review_gate_reconcile`) to the consumer in the Penstock repo. That
+  #     external consumer is the success writer for this context; nothing in
+  #     this repo, server or workflow, ever posts success for it.
+  # Both flags above are false today, so only the legacy failure writer can
+  # fire; the authority is pre-configured for the rollout, not yet live.
+  # Therefore: do NOT mark this context required on paperclip `master`. With no
+  # in-repo success writer, every healthy PR would sit at "Expected — waiting
+  # for status" forever. Requiring it is correct only where an external producer
+  # posts success — step 7 of the rollout above, on penstock-llm-proxy-core, or
+  # a pre-existing external writer as on onprem-k8s.
   prReviewGateStatusContext: review/ally-complete
-  #
-  # Deliberately NOT under `review/` (BLO-29711). This gate reads only the
-  # comment-shaped surface, so "nothing attests this head" is a legitimate and
-  # common outcome for a formally-reviewed PR, and it must stay green there or
-  # every such PR deadlocks. A green under `review/` reads as review evidence,
-  # which this gate cannot supply — so the fail-open and the namespace cannot
-  # coexist. Moving the namespace is the half that is safe to change.
-  # Verified before renaming: `review/ally-comment` was not a required check on
-  # paperclip `master` (required: `verify`) nor on penstock-llm-proxy-core
-  # `main` (required: validate-and-build, secret-scan, redaction-tests,
-  # secrets-controls-static-check, review/ally-complete), so this drops nothing.
-  # NOTE: `prReviewGateStatusContext` is intentionally left unset here. Its only
-  # writer posts state=failure (reviewer chain exhausted / ended ambiguously) —
-  # nothing in the server ever posts success for it, and no workflow or script
-  # in this repo posts it either. It is meant to red-flag an already-required
-  # context owned by an external writer (as on onprem-k8s). Marking it required
-  # on this repo would leave every healthy PR at "Expected — waiting for status"
-  # forever. Do not enable + require it here without a success writer.
 
 # paperclip-plugin-gbrain reads per-agent OAuth client credentials through
 # Penstock/Authbot. The mounted Secret below contains only the managed service
diff --git a/server/src/__tests__/github-webhook.test.ts b/server/src/__tests__/github-webhook.test.ts
index 5714c832f037..fe1c8656d900 100644
--- a/server/src/__tests__/github-webhook.test.ts
+++ b/server/src/__tests__/github-webhook.test.ts
@@ -2446,6 +2446,7 @@ describeEmbeddedPostgres("github-webhook route", () => {
       prNumber: number;
       headSha?: string | null;
       prUrl?: string | null;
+      db?: unknown;
     }> = [];
     let markCalled!: () => void;
     const called = new Promise<void>((resolve) => {
@@ -2484,11 +2485,18 @@ describeEmbeddedPostgres("github-webhook route", () => {
 
     expect(response.status).toBe(200);
     expect(response.body).toMatchObject({ ignored: "no_paperclip_identifier" });
-    expect(calls).toEqual([{
+    expect(calls).toHaveLength(1);
+    expect(calls[0]).toMatchObject({
       repoFullName: "Blockcast/paperclip",
       prNumber: 1049,
       prUrl: "https://github.com/Blockcast/paperclip/pull/1049",
-    }]);
+    });
+    // The db handle is the gate's only cross-process serialization boundary,
+    // and production supplying it is a property of THIS call site. Assert it
+    // on the argument the seam actually received: while the seam was invoked
+    // with the bare trigger, the real handle was unobservable from here and a
+    // caller could drop it without any test noticing.
+    expect(calls[0]!.db).toBeDefined();
   });
 
   it("leaves reviewer wakes queued when the webhook runs on the API tier", async () => {
diff --git a/server/src/__tests__/pr-comment-review-gate-check.test.ts b/server/src/__tests__/pr-comment-review-gate-check.test.ts
index 7715c1b3ee5b..4f34530e400e 100644
--- a/server/src/__tests__/pr-comment-review-gate-check.test.ts
+++ b/server/src/__tests__/pr-comment-review-gate-check.test.ts
@@ -43,11 +43,15 @@ vi.mock("../services/github-status-delivery-outbox.js", () => ({
 
 import { runPrCommentReviewGateCheck } from "../services/pr-comment-review-gate.js";
 
+// `db` is required on the input: the gate takes the shared delivery lock
+// unconditionally, so every caller — including these tests — must supply a
+// handle. The lock itself is mocked above, so a stub is sufficient here.
 const TARGET = {
   repoFullName: "Blockcast/paperclip",
   prNumber: 1022,
   headSha: "1234567890abcdef1234567890abcdef12345678",
   prUrl: "https://github.com/Blockcast/paperclip/pull/1022",
+  db: {} as never,
 };
 
 function blockingCommentFor(headSha: string) {
@@ -148,6 +152,22 @@ describe("runPrCommentReviewGateCheck", () => {
     expect(events).toEqual(["lock", "fetch-comments", "post"]);
   });
 
+  it("refuses to publish unsynchronized when db is missing", async () => {
+    mockListComments.mockResolvedValue([]);
+    mockPostStatus.mockResolvedValue({ ok: true, statusCode: 201 });
+
+    // The old shape fell through to an unlocked publish() whenever db was
+    // absent, so a caller could silently reopen the out-of-order-verdict race.
+    // Absence must now fail closed: no lock, no status write, and a loud error
+    // rather than a green nobody serialized.
+    await expect(
+      runPrCommentReviewGateCheck({ ...TARGET, db: undefined } as never),
+    ).rejects.toThrow(/requires `db`/);
+
+    expect(mockStatusDeliveryLock).not.toHaveBeenCalled();
+    expect(mockPostStatus).not.toHaveBeenCalled();
+  });
+
   it("serializes overlapping evaluations for one PR/context", async () => {
     const events: string[] = [];
     let releaseFirstFetch!: () => void;
diff --git a/server/src/routes/github-webhook.ts b/server/src/routes/github-webhook.ts
index a42feb34fd04..95ba690d2aa3 100644
--- a/server/src/routes/github-webhook.ts
+++ b/server/src/routes/github-webhook.ts
@@ -4234,9 +4234,14 @@ export function githubWebhookRoutes(db: Db, config: GithubWebhookConfig) {
       config.prReviewerBotLogin,
     );
     if (commentReviewGateTrigger) {
+      // Build the input once and hand the SAME object to both branches, so the
+      // injection seam observes the real argument — including `db`. When the
+      // seam was called with the bare trigger, no webhook-level test could
+      // assert that production actually supplies the serialization handle.
+      const commentReviewGateInput = { ...commentReviewGateTrigger, db };
       const commentReviewGateCheck = config.runPrCommentReviewGateCheck
-        ? config.runPrCommentReviewGateCheck(commentReviewGateTrigger)
-        : runPrCommentReviewGateCheck({ ...commentReviewGateTrigger, db });
+        ? config.runPrCommentReviewGateCheck(commentReviewGateInput)
+        : runPrCommentReviewGateCheck(commentReviewGateInput);
       void commentReviewGateCheck
         .then((result) => {
           // The disabled default must be silent; otherwise every PR webhook in
diff --git a/server/src/services/github-status-delivery-outbox.ts b/server/src/services/github-status-delivery-outbox.ts
index d05769e26dda..28d89e1c8c76 100644
--- a/server/src/services/github-status-delivery-outbox.ts
+++ b/server/src/services/github-status-delivery-outbox.ts
@@ -16,6 +16,10 @@ import {
 
 type DeliveryRow = typeof githubCommitStatusDeliveries.$inferSelect;
 type DbTransaction = Parameters<Parameters<Db["transaction"]>[0]>[0];
+// Either handle works for the delivery bookkeeping below. Code reached from
+// inside withGithubStatusDeliveryLock must use the transaction handle so the
+// critical section does not take a second pool connection.
+type DbHandle = Db | DbTransaction;
 type DeliveryTerminalStatus = "delivered" | "skipped" | "failed" | "failed_permanent";
 
 const POLL_INTERVAL_MS = 5_000;
@@ -30,15 +34,40 @@ const RETRY_DELAYS_MS = [
   2 * 60 * 60_000,
 ];
 
+// How long a waiter may queue for the delivery advisory lock before giving up,
+// and how long a holder may sit idle-in-transaction (i.e. inside its external
+// GitHub calls) before Postgres terminates it and releases the lock. Both are
+// far above the normal critical-section cost — they exist to make pool
+// exhaustion recoverable, not to bound healthy work.
+const DELIVERY_LOCK_WAIT_TIMEOUT_MS = 30_000;
+const DELIVERY_LOCK_HOLD_TIMEOUT_MS = 120_000;
+
 /** Serialize the final read and external write for one GitHub status key. */
 export async function withGithubStatusDeliveryLock<T>(
   db: Db,
   key: string,
-  operation: () => Promise<T>,
+  operation: (tx: DbTransaction) => Promise<T>,
 ): Promise<T> {
   return db.transaction(async (tx) => {
+    // Bound both sides of the lock. The critical section performs external
+    // GitHub I/O (paginated list calls and status POSTs with retries), so the
+    // holder sits idle-in-transaction pinning a pool connection, and every
+    // waiter queued on an untimed pg_advisory_xact_lock pins one too. Without
+    // these bounds a hung GitHub call can exhaust the pool, and once exhausted
+    // the holder cannot finish, so the lock is never released.
+    //
+    // set_config(..., true) is transaction-local; SET LOCAL cannot be
+    // parameterized, so it is spelled this way deliberately.
+    await tx.execute(
+      sql`select set_config('lock_timeout', ${`${DELIVERY_LOCK_WAIT_TIMEOUT_MS}ms`}, true)`,
+    );
+    await tx.execute(
+      sql`select set_config('idle_in_transaction_session_timeout', ${`${DELIVERY_LOCK_HOLD_TIMEOUT_MS}ms`}, true)`,
+    );
     await tx.execute(sql`select pg_advisory_xact_lock(hashtextextended(${key}, 0))`);
-    return operation();
+    // Hand the transaction handle to the caller: taking a second pool
+    // connection here is what makes the exhaustion above reachable.
+    return operation(tx);
   });
 }
 
@@ -111,7 +140,7 @@ function deliveryClaimWhere(row: DeliveryRow) {
   );
 }
 
-async function refreshDeliveryClaimBeforeExternalWrite(db: Db, row: DeliveryRow): Promise<DeliveryRow | null> {
+async function refreshDeliveryClaimBeforeExternalWrite(db: DbHandle, row: DeliveryRow): Promise<DeliveryRow | null> {
   const [updated] = await db
     .update(githubCommitStatusDeliveries)
     .set({ updatedAt: new Date() })
@@ -126,7 +155,7 @@ function statusCreatedAtOrAfterQueueSecond(statusCreatedAt: number, queuedAt: Da
   return statusCreatedAt >= queuedAtSecond;
 }
 
-async function handleFreshCommitStatusIfPresent(db: Db, row: DeliveryRow): Promise<boolean> {
+async function handleFreshCommitStatusIfPresent(db: DbHandle, row: DeliveryRow): Promise<boolean> {
   const latestStatus = await githubGetLatestCommitStatusForContext({
     repoFullName: row.repoFullName,
     sha: row.sha,
@@ -164,7 +193,7 @@ async function handleFreshCommitStatusIfPresent(db: Db, row: DeliveryRow): Promi
 
 // A forced retirement retry may supersede the status that existed when it was
 // queued, but it must not overwrite any later evaluation of the same head.
-async function handleFreshSuccessForForcedDelivery(db: Db, row: DeliveryRow): Promise<boolean> {
+async function handleFreshSuccessForForcedDelivery(db: DbHandle, row: DeliveryRow): Promise<boolean> {
   if (!row.forceWrite) return false;
   const latestStatus = await githubGetLatestCommitStatusForContext({
     repoFullName: row.repoFullName,
@@ -200,7 +229,7 @@ async function handleFreshSuccessForForcedDelivery(db: Db, row: DeliveryRow): Pr
 }
 
 async function appendDeliveryRunEvent(
-  db: Db,
+  db: DbHandle,
   row: DeliveryRow,
   level: "info" | "warn",
   message: string,
@@ -238,7 +267,7 @@ async function appendDeliveryRunEvent(
 }
 
 async function markTerminal(
-  db: Db,
+  db: DbHandle,
   row: DeliveryRow,
   status: DeliveryTerminalStatus,
   level: "info" | "warn",
@@ -278,7 +307,7 @@ async function markTerminal(
 }
 
 async function retryOrFailDelivery(
-  db: Db,
+  db: DbHandle,
   row: DeliveryRow,
   reason: string,
   result: Record<string, unknown>,
@@ -332,7 +361,7 @@ async function retryOrFailDelivery(
 }
 
 async function failPermanentDelivery(
-  db: Db,
+  db: DbHandle,
   row: DeliveryRow,
   reason: string,
   result: Record<string, unknown>,
@@ -413,10 +442,12 @@ async function processDelivery(db: Db, row: DeliveryRow): Promise<void> {
     ? await withGithubStatusDeliveryLock(
         db,
         `${fencedRow.repoFullName}#${fencedRow.sha}`,
-        async () => {
+        async (tx) => {
           // The lock is shared with the live gate evaluation. Re-check inside
           // it so a clean evaluation that won the lock cannot be overwritten.
-          if (await handleFreshSuccessForForcedDelivery(db, fencedRow)) {
+          // Use `tx`, not `db`: a second pool connection taken here is what
+          // lets a saturated pool wedge the lock holder.
+          if (await handleFreshSuccessForForcedDelivery(tx, fencedRow)) {
             return { ok: true as const, skipped: true as const };
           }
           return { ...(await post()), skipped: false as const };
diff --git a/server/src/services/pr-comment-review-gate.ts b/server/src/services/pr-comment-review-gate.ts
index 158b9d4ed351..5e3026121d65 100644
--- a/server/src/services/pr-comment-review-gate.ts
+++ b/server/src/services/pr-comment-review-gate.ts
@@ -492,7 +492,12 @@ export interface PrCommentReviewGateCheckInput {
   prNumber: number;
   headSha?: string | null;
   prUrl?: string | null;
-  db?: Db;
+  // Required, not optional. This handle is the only cross-process boundary
+  // serializing evaluations of one head: the in-process `gateEvaluationChains`
+  // map below does not span API pods. When it was optional, any caller that
+  // forgot it silently got the unsynchronized path and re-opened the
+  // out-of-order-verdict race. Test injection passes a stub.
+  db: Db;
 }
 
 const TRANSIENT_RETRY_DELAYS_MS = [250, 1000];
@@ -543,6 +548,19 @@ export async function runPrCommentReviewGateCheck(
   const context = config.prCommentReviewGateStatusContext.trim();
   if (!context) return { posted: false, reason: "not_configured" };
 
+  // Fail closed rather than evaluating unsynchronized. `db` is required by the
+  // type, but this module is reachable from JS callers and from tests that are
+  // excluded from `tsc`, so the invariant needs a runtime edge too. Publishing
+  // a verdict without the cross-process lock is the out-of-order-write bug this
+  // gate already had once; refusing to publish is the recoverable direction,
+  // because the next webhook for this head re-evaluates.
+  if (!input.db) {
+    throw new Error(
+      "runPrCommentReviewGateCheck requires `db`: it is the cross-process lock that keeps a " +
+        "stale verdict from overwriting a newer one. Pass the request's database handle.",
+    );
+  }
+
   const key = `${input.repoFullName}#${input.prNumber}#${context}`;
   return serializeGateEvaluation(key, () => executeCommentReviewGateCheck(input, context, config));
 }
@@ -637,10 +655,10 @@ async function executeCommentReviewGateCheck(
   };
 
   // Serialize evidence reads, verdict computation, and all status writes with
-  // forced retries. The transaction-scoped lock is the cross-process boundary.
-  return input.db
-    ? withGithubStatusDeliveryLock(input.db, `${input.repoFullName}#${headSha}`, publish)
-    : publish();
+  // forced retries. The transaction-scoped lock is the cross-process boundary,
+  // and it is unconditional: `db` is required precisely so there is no
+  // unsynchronized fall-through for a caller to reach by omission.
+  return withGithubStatusDeliveryLock(input.db, `${input.repoFullName}#${headSha}`, publish);
 }
 
 /**

From aa0b154d4f9b15ce16706e4a5781fd23d087eb92 Mon Sep 17 00:00:00 2001
From: CTO <cto@paperclip.blockcast.net>
Date: Fri, 4 Sep 2026 07:16:33 +0000
Subject: [PATCH 5/7] fix(review-gate): take both Suggestions from Ally review
 5108460067
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The retirement-description fallback did not deliver what its comment
promised. `short.slice(0, 140)` still cuts the context name mid-token and
drops the closing quote for a sufficiently long name — the exact "cut in
half" outcome the fallback exists to prevent. Elide the NAME instead, so the
sentence stays well-formed and the quoted pointer closes. Unreachable with
today's names (~75 chars), which is why the existing length-only assertion
passed either way; the test now asserts the shape, not just the length.

The NULL semantics of `preserveExistingDelivery` are load-bearing and were
silent: migration 0237 made source_run_id nullable, so `source_run_id = NULL`
is NULL rather than true for webhook-originated rows, every CASE takes its
ELSE branch, and a delivered/skipped row is re-queued. That is wanted — a
retirement write must be redone when a fresh failure re-enqueues the same key
— but a later reader could "fix" it to `is not distinct from` and silently
drop the re-delivery. Documented and pinned by test.

Refs BLO-29711
---
 .../github-status-delivery-outbox.test.ts     | 47 +++++++++++++++++++
 .../__tests__/pr-comment-review-gate.test.ts  | 12 +++++
 .../services/github-status-delivery-outbox.ts | 10 ++++
 server/src/services/pr-comment-review-gate.ts | 27 +++++++----
 4 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/server/src/__tests__/github-status-delivery-outbox.test.ts b/server/src/__tests__/github-status-delivery-outbox.test.ts
index 4561a435d5f7..81b204e18fdf 100644
--- a/server/src/__tests__/github-status-delivery-outbox.test.ts
+++ b/server/src/__tests__/github-status-delivery-outbox.test.ts
@@ -508,6 +508,53 @@ describeEmbeddedPostgres("GitHub commit-status delivery outbox", () => {
     expect(events.at(-1)?.message).toContain("Set PR-review gate status review/ally-complete to failure");
   });
 
+  it("re-queues a delivered webhook-originated row", async () => {
+    setCreds();
+    const { companyId, delivery } = await seedRun();
+    const deliveredAt = new Date(Date.now() - 60_000);
+    // Webhook-originated rows carry no source run (migration 0237 made
+    // source_run_id nullable), so preserveExistingDelivery compares NULL to
+    // NULL. In SQL that is NULL, not true, which is what lets a terminal row
+    // be revived. Pin it: if someone "fixes" the comparison to
+    // `is not distinct from`, the row below stays `delivered` and the retirement
+    // re-delivery is silently dropped.
+    await db
+      .update(githubCommitStatusDeliveries)
+      .set({
+        status: "delivered",
+        sourceRunId: null,
+        deliveredAt,
+        createdAt: deliveredAt,
+        updatedAt: deliveredAt,
+        nextAttemptAt: deliveredAt,
+        lastResult: { posted: { ok: true } },
+      })
+      .where(eq(githubCommitStatusDeliveries.id, delivery.id));
+
+    const revived = await enqueueGithubCommitStatusDelivery(db, {
+      companyId,
+      sourceRunId: null,
+      repoFullName: "Blockcast/hang",
+      sha: HEAD_SHA,
+      context: "review/ally-complete",
+      state: "failure",
+      description: "Retired. Findings now publish elsewhere.",
+      targetUrl: "https://github.com/Blockcast/hang/pull/7",
+      prNumber: 7,
+      prUrl: "https://github.com/Blockcast/hang/pull/7",
+      forceWrite: true,
+    });
+
+    expect(revived).toMatchObject({
+      id: delivery.id,
+      sourceRunId: null,
+      status: "queued",
+      attempts: 0,
+      forceWrite: true,
+      description: "Retired. Findings now publish elsewhere.",
+    });
+  });
+
   it("skips the failure write when an approved App review exists on GitHub", async () => {
     setCreds();
     const { delivery } = await seedRun();
diff --git a/server/src/__tests__/pr-comment-review-gate.test.ts b/server/src/__tests__/pr-comment-review-gate.test.ts
index 142e5dc77f35..9b116e3d44eb 100644
--- a/server/src/__tests__/pr-comment-review-gate.test.ts
+++ b/server/src/__tests__/pr-comment-review-gate.test.ts
@@ -709,5 +709,17 @@ describe("retired context supersede", () => {
     expect(
       commentReviewGateRetirementDescription(longContext, "failure").length,
     ).toBeLessThanOrEqual(140);
+
+    // Length alone was the weaker half of this promise: slicing the rendered
+    // sentence also satisfies it, while severing the name and dropping the
+    // closing quote — the exact "cut in half" outcome the fallback exists to
+    // prevent. Assert the sentence stays well-formed: the name is elided with
+    // an ellipsis and the quoted pointer still closes.
+    for (const state of ["success", "failure"] as const) {
+      const description = commentReviewGateRetirementDescription(longContext, state);
+      expect(description.length).toBeLessThanOrEqual(140);
+      expect(description).toMatch(/"[^"]*…"\.$/);
+      expect(description.split('"').length - 1).toBe(2);
+    }
   });
 });
diff --git a/server/src/services/github-status-delivery-outbox.ts b/server/src/services/github-status-delivery-outbox.ts
index 28d89e1c8c76..bb0f6e13256f 100644
--- a/server/src/services/github-status-delivery-outbox.ts
+++ b/server/src/services/github-status-delivery-outbox.ts
@@ -479,6 +479,16 @@ export async function enqueueGithubCommitStatusDelivery(
 ): Promise<DeliveryRow> {
   const now = new Date();
   const nowSql = sql`${now.toISOString()}::timestamptz`;
+  // NOTE the NULL semantics, which are load-bearing. Migration 0237 made
+  // source_run_id nullable, so for webhook-originated rows both sides of the
+  // comparison are NULL and `source_run_id = NULL` evaluates to NULL, not
+  // true. preserveExistingDelivery is therefore NULL, every CASE below takes
+  // its ELSE branch, and a `delivered`/`skipped` row is reset to `queued`.
+  // That is the wanted behavior: a retirement write that already delivered
+  // must be redone when a fresh failure re-enqueues the same key. Do NOT
+  // "correct" this to `is not distinct from` — that would make the comparison
+  // true for two NULLs, preserve the terminal row, and silently drop the
+  // re-delivery. Pinned by test: "re-queues a delivered webhook-originated row".
   const preserveExistingDelivery = sql`${
     githubCommitStatusDeliveries.status
   } = 'processing' or (${
diff --git a/server/src/services/pr-comment-review-gate.ts b/server/src/services/pr-comment-review-gate.ts
index 5e3026121d65..3171903cfb20 100644
--- a/server/src/services/pr-comment-review-gate.ts
+++ b/server/src/services/pr-comment-review-gate.ts
@@ -439,18 +439,25 @@ export function commentReviewGateRetirementDescription(
   state: CommentReviewGateVerdict["state"] = "success",
 ): string {
   const target = liveContext.trim();
-  const [full, short] =
+  const renderShort = (name: string) =>
     state === "failure"
-      ? [
-          `Retired. Unresolved finding stands; "${target}" carries the verdict.`,
-          `Retired. Unresolved finding; see "${target}".`,
-        ]
-      : [
-          `Retired. Comment-shaped review findings now publish to "${target}".`,
-          `Retired. Findings now publish to "${target}".`,
-        ];
+      ? `Retired. Unresolved finding; see "${name}".`
+      : `Retired. Findings now publish to "${name}".`;
+  const full =
+    state === "failure"
+      ? `Retired. Unresolved finding stands; "${target}" carries the verdict.`
+      : `Retired. Comment-shaped review findings now publish to "${target}".`;
   if (full.length <= MAX_COMMIT_STATUS_DESCRIPTION) return full;
-  return short.slice(0, MAX_COMMIT_STATUS_DESCRIPTION);
+  const short = renderShort(target);
+  if (short.length <= MAX_COMMIT_STATUS_DESCRIPTION) return short;
+  // Both phrasings overflow, so the context name itself is what is long.
+  // Elide the NAME rather than slicing the rendered sentence: a blind slice
+  // cuts the name mid-token and drops the closing quote, which is exactly the
+  // "cut in half" outcome the fallback exists to avoid. Unreachable with
+  // today's names; pinned by test so it stays true if a name grows.
+  const budget = MAX_COMMIT_STATUS_DESCRIPTION - renderShort("").length - 1;
+  if (budget <= 0) return short.slice(0, MAX_COMMIT_STATUS_DESCRIPTION);
+  return renderShort(`${target.slice(0, budget)}…`);
 }
 
 /**

From 74c178f7b7f9905a93dd70b82f7a3fb7a1d4bcf7 Mon Sep 17 00:00:00 2001
From: CTO <cto@paperclip.blockcast.net>
Date: Fri, 4 Sep 2026 19:20:00 +0000
Subject: [PATCH 6/7] fix(outbox): normalize omitted provenance keys before SQL
 interpolation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Ally review 5110429996 (Critical, head 9e7a4bdc).

enqueueGithubCommitStatusDelivery interpolated input.companyId and
input.sourceRunId bare into three sql templates. Both became optional in
this PR, and the webhook retirement call site omits them entirely, so
production passed undefined rather than null.

Verified against the pinned drizzle-orm 0.45.2: an undefined chunk renders
as the empty string with NO bound parameter, so

  source_run_id = ${undefined}  ->  source_run_id =
  else ${undefined} end        ->  else  end

both of which are Postgres 42601 at parse time. Every webhook-originated
retirement enqueue therefore rejected before writing a row, and the
.catch() at the call site logged it — so migration 0238's force_write
column, the DbHandle threading and the forced-retry lock had no reachable
caller. That is a fail-open, not just dead code: the outbox is what closes
the window where a retired legacy context sits green against a red live one.

Normalize once at the top and derive the insert values and both CASE arms
from those constants, so they cannot drift apart again. Pass the keys
explicitly at the webhook call site.

Test exercises the omitted shape; the existing sibling passes an explicit
null and structurally cannot catch this.

Rebased onto master: this branch's migration was renumbered 0237 -> 0238
because master took 0237 (heartbeat_runs_agent_queued_dispatch_index).
Journal rebuilt from master verbatim with one entry appended.
---
 .../github-status-delivery-outbox.test.ts     | 56 ++++++++++++++++++-
 server/src/routes/github-webhook.ts           |  8 +++
 .../services/github-status-delivery-outbox.ts | 23 ++++++--
 3 files changed, 80 insertions(+), 7 deletions(-)

diff --git a/server/src/__tests__/github-status-delivery-outbox.test.ts b/server/src/__tests__/github-status-delivery-outbox.test.ts
index 81b204e18fdf..cbc6532735ef 100644
--- a/server/src/__tests__/github-status-delivery-outbox.test.ts
+++ b/server/src/__tests__/github-status-delivery-outbox.test.ts
@@ -512,7 +512,7 @@ describeEmbeddedPostgres("GitHub commit-status delivery outbox", () => {
     setCreds();
     const { companyId, delivery } = await seedRun();
     const deliveredAt = new Date(Date.now() - 60_000);
-    // Webhook-originated rows carry no source run (migration 0237 made
+    // Webhook-originated rows carry no source run (migration 0238 made
     // source_run_id nullable), so preserveExistingDelivery compares NULL to
     // NULL. In SQL that is NULL, not true, which is what lets a terminal row
     // be revived. Pin it: if someone "fixes" the comparison to
@@ -555,6 +555,60 @@ describeEmbeddedPostgres("GitHub commit-status delivery outbox", () => {
     });
   });
 
+  it("enqueues a retirement when the provenance keys are omitted entirely", async () => {
+    setCreds();
+    // The webhook retirement call site omits `companyId` and `sourceRunId`
+    // rather than passing null, so both arrive as `undefined`. That is a
+    // DIFFERENT input from null: drizzle renders an `undefined` chunk as the
+    // empty string with no bound parameter, which turns
+    // `source_run_id = ${...}` into `source_run_id = ` and a CASE ELSE arm into
+    // `else  end` — Postgres 42601 at parse time. Every webhook-originated
+    // retirement therefore rejected before writing a row, so the outbox, the
+    // force_write column and the forced-retry lock had no reachable caller.
+    // The sibling test above passes `sourceRunId: null` explicitly and so
+    // cannot catch this; only the omitted shape reproduces production.
+    const { delivery } = await seedRun();
+    const deliveredAt = new Date(Date.now() - 60_000);
+    await db
+      .update(githubCommitStatusDeliveries)
+      .set({
+        status: "delivered",
+        companyId: null,
+        sourceRunId: null,
+        deliveredAt,
+        createdAt: deliveredAt,
+        updatedAt: deliveredAt,
+        nextAttemptAt: deliveredAt,
+        lastResult: { posted: { ok: true } },
+      })
+      .where(eq(githubCommitStatusDeliveries.id, delivery.id));
+
+    const revived = await enqueueGithubCommitStatusDelivery(db, {
+      repoFullName: "Blockcast/hang",
+      sha: HEAD_SHA,
+      context: "review/ally-complete",
+      state: "failure",
+      description: "Retired. Findings now publish elsewhere.",
+      targetUrl: "https://github.com/Blockcast/hang/pull/7",
+      prNumber: 7,
+      prUrl: "https://github.com/Blockcast/hang/pull/7",
+      forceWrite: true,
+    });
+
+    // Both the insert values and the two CASE arms must normalize to NULL, and
+    // the row must actually be revived — an omitted sourceRunId has to compare
+    // the same way an explicit null does, or the retirement is dropped.
+    expect(revived).toMatchObject({
+      id: delivery.id,
+      companyId: null,
+      sourceRunId: null,
+      status: "queued",
+      attempts: 0,
+      forceWrite: true,
+      description: "Retired. Findings now publish elsewhere.",
+    });
+  });
+
   it("skips the failure write when an approved App review exists on GitHub", async () => {
     setCreds();
     const { delivery } = await seedRun();
diff --git a/server/src/routes/github-webhook.ts b/server/src/routes/github-webhook.ts
index 95ba690d2aa3..981d863086e2 100644
--- a/server/src/routes/github-webhook.ts
+++ b/server/src/routes/github-webhook.ts
@@ -4250,6 +4250,14 @@ export function githubWebhookRoutes(db: Db, config: GithubWebhookConfig) {
           if (!result.posted && result.retirementDeliveries) {
             void Promise.all(result.retirementDeliveries.map((delivery) =>
               enqueueGithubCommitStatusDelivery(db, {
+                // Explicitly provenance-less: a retirement is triggered by the
+                // webhook, not by an agent run, so there is no company or run
+                // to attribute it to. Passing `null` rather than omitting the
+                // keys is deliberate — the enqueue normalizes either shape, but
+                // the omission read as an oversight to several reviewers and is
+                // what the NULL semantics of preserveExistingDelivery rely on.
+                companyId: null,
+                sourceRunId: null,
                 repoFullName: commentReviewGateTrigger.repoFullName,
                 sha: delivery.sha,
                 context: delivery.context,
diff --git a/server/src/services/github-status-delivery-outbox.ts b/server/src/services/github-status-delivery-outbox.ts
index bb0f6e13256f..86dd4814bfae 100644
--- a/server/src/services/github-status-delivery-outbox.ts
+++ b/server/src/services/github-status-delivery-outbox.ts
@@ -479,7 +479,18 @@ export async function enqueueGithubCommitStatusDelivery(
 ): Promise<DeliveryRow> {
   const now = new Date();
   const nowSql = sql`${now.toISOString()}::timestamptz`;
-  // NOTE the NULL semantics, which are load-bearing. Migration 0237 made
+  // Normalize the two optional provenance fields ONCE, before anything below
+  // reads them. `undefined` and `null` are different inputs to drizzle and only
+  // the second is safe here: an `undefined` chunk renders as the empty string
+  // with no bound parameter, so `source_run_id = ${undefined}` becomes
+  // `source_run_id = ` and a CASE ELSE arm becomes `else  end` — both Postgres
+  // 42601 at parse time. Webhook-originated retirements omit these keys
+  // entirely (`github-webhook.ts`), so that is the shape production actually
+  // sends. Deriving the insert values and both CASE arms from these two
+  // constants is what keeps them from drifting apart again.
+  const companyId = input.companyId ?? null;
+  const sourceRunId = input.sourceRunId ?? null;
+  // NOTE the NULL semantics, which are load-bearing. Migration 0238 made
   // source_run_id nullable, so for webhook-originated rows both sides of the
   // comparison are NULL and `source_run_id = NULL` evaluates to NULL, not
   // true. preserveExistingDelivery is therefore NULL, every CASE below takes
@@ -495,10 +506,10 @@ export async function enqueueGithubCommitStatusDelivery(
     githubCommitStatusDeliveries.status
   } in ('delivered', 'skipped') and ${
     githubCommitStatusDeliveries.sourceRunId
-  } = ${input.sourceRunId})`;
+  } = ${sourceRunId})`;
   const values = {
-    companyId: input.companyId,
-    sourceRunId: input.sourceRunId,
+    companyId,
+    sourceRunId,
     repoFullName: input.repoFullName,
     sha: input.sha,
     context: input.context,
@@ -528,8 +539,8 @@ export async function enqueueGithubCommitStatusDelivery(
         githubCommitStatusDeliveries.context,
       ],
       set: {
-        companyId: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.companyId} else ${input.companyId} end`,
-        sourceRunId: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.sourceRunId} else ${input.sourceRunId} end`,
+        companyId: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.companyId} else ${companyId} end`,
+        sourceRunId: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.sourceRunId} else ${sourceRunId} end`,
         prNumber: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.prNumber} else ${input.prNumber} end`,
         prUrl: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.prUrl} else ${input.prUrl ?? null} end`,
         state: sql`case when ${preserveExistingDelivery} then ${githubCommitStatusDeliveries.state} else ${input.state} end`,

From 1d9692ed3a8643414fa96665089d17739a9873a1 Mon Sep 17 00:00:00 2001
From: CTO <cto@paperclip.blockcast.net>
Date: Fri, 4 Sep 2026 19:37:48 +0000
Subject: [PATCH 7/7] refactor(review-gate): distinguish retirement failure
 from live-post failure
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Ally review 5110429996, Suggestion 2.

A failed retired-context cleanup returned reason "post_failed" even though
the live status HAD published — the branch is only reachable after the live
post succeeded. The result shape therefore stated the opposite of what
happened for the field that matters most, with retirementDeliveries as the
sole discriminator between "live write failed" and "live write succeeded,
cleanup did not".

New reason "retirement_failed" makes both states self-describing. No
consumer branches on "post_failed" (the webhook compares only
"not_configured" and keys the retry off retirementDeliveries), so this is
a reporting change with no behavioral effect on delivery.
---
 .../src/__tests__/pr-comment-review-gate-check.test.ts |  5 ++++-
 server/src/services/pr-comment-review-gate.ts          | 10 ++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/server/src/__tests__/pr-comment-review-gate-check.test.ts b/server/src/__tests__/pr-comment-review-gate-check.test.ts
index 4f34530e400e..fa3a494709a9 100644
--- a/server/src/__tests__/pr-comment-review-gate-check.test.ts
+++ b/server/src/__tests__/pr-comment-review-gate-check.test.ts
@@ -325,7 +325,10 @@ describe("retired status contexts", () => {
 
     await expect(runPrCommentReviewGateCheck(TARGET)).resolves.toMatchObject({
       posted: false,
-      reason: "post_failed",
+      // `retirement_failed`, not `post_failed`: the live verdict below DID
+      // publish. The two states must be distinguishable without inspecting
+      // `retirementDeliveries`.
+      reason: "retirement_failed",
       postFailure: "review/ally-comment: commit_status_write_http_403",
     });
     expect(postFor("gate/ally-comment-findings")).toBeDefined();
diff --git a/server/src/services/pr-comment-review-gate.ts b/server/src/services/pr-comment-review-gate.ts
index 3171903cfb20..b323d9e6fd7f 100644
--- a/server/src/services/pr-comment-review-gate.ts
+++ b/server/src/services/pr-comment-review-gate.ts
@@ -483,7 +483,7 @@ export type PrCommentReviewGateCheckResult =
   | { posted: true; verdict: CommentReviewGateVerdict }
   | {
       posted: false;
-      reason: "not_configured" | "fetch_failed" | "post_failed";
+      reason: "not_configured" | "fetch_failed" | "post_failed" | "retirement_failed";
       postFailure?: string;
       retirementDeliveries?: Array<{
         sha: string;
@@ -644,9 +644,15 @@ async function executeCommentReviewGateCheck(
 
     const retirementFailures = await supersedeRetiredContexts(input, headSha, context, config, verdict);
     if (retirementFailures.length > 0) {
+      // NOT "post_failed": the live status published successfully at line 643
+      // above, and only the retired-context cleanup did not. Reporting this as
+      // a post failure states the opposite of what happened for the field that
+      // matters most. `retirementDeliveries` used to be the sole discriminator
+      // between the two, which is easy to get wrong from outside — a distinct
+      // reason makes both states self-describing.
       return {
         posted: false,
-        reason: "post_failed",
+        reason: "retirement_failed",
         postFailure: retirementFailures.map((failure) => `${failure.context}: ${failure.reason}`).join(", "),
         retirementDeliveries: retirementFailures.map((failure) => ({
           sha: headSha,