fix(review): retry a 503 REES startup ping before escalating (#5006)

REES's own /v1/ping returns 503 specifically to mean "not configured/
ready yet" (server.ts checks its own REES_SHARED_SECRET before
anything else) -- the same benign startup-ordering race the engine's
probe already extends grace to for a refused connection, just via an
HTTP response instead of a connection failure. All 7 GITTENSORY-1J
events clustered in one ~5h window and never recurred, consistent
with a one-time deploy/restart race rather than a persistent
misconfiguration. Retry a 503 up to twice (500ms apart) before
logging rees_ping_error; any other status is still final immediately,
and a persistent 503 still escalates after the retries exhaust.

Author: JSONbored

src/review/enrichment-wire.ts

@@ -53,6 +53,32 @@ function sharedSecretWasNormalized(
   return (normalized ?? "") !== raw;
 }
 
+// REES's own /v1/ping returns 503 specifically to mean "not configured/ready yet" (server.ts: no
+// REES_SHARED_SECRET set on that side) -- the same benign startup-ordering race probeReesSecretAtStartup's
+// catch block already extends grace to for a refused connection (GITTENSORY-1J: 7 Sentry events, all in one
+// ~5h window, never recurring -- consistent with a one-time deploy/restart race, not a persistent
+// misconfiguration). Retry a few times before escalating; any other status is final on the first response.
+const REES_PING_NOT_READY_RETRIES = 2;
+const REES_PING_NOT_READY_RETRY_DELAY_MS = 500;
+
+async function fetchReesPingWithRetry(url: string, secret: string): Promise<Response> {
+  const request = () =>
+    fetch(url, {
+      method: "POST",
+      headers: {
+        "user-agent": "gittensory-selfhost/1.0",
+        authorization: `Bearer ${secret}`,
+      },
+      signal: AbortSignal.timeout(5000),
+    });
+  let response = await request();
+  for (let attempt = 0; attempt < REES_PING_NOT_READY_RETRIES && response.status === 503; attempt += 1) {
+    await new Promise((resolve) => setTimeout(resolve, REES_PING_NOT_READY_RETRY_DELAY_MS));
+    response = await request();
+  }
+  return response;
+}
+
 // Set true once the startup probe confirms REES rejects the shared secret (401/403). Once set,
 // buildReviewEnrichment skips every /v1/enrich call for the rest of this process's lifetime instead of
 // repeating a call that's confirmed to fail on every PR review, each one logging review_context_fetch_failed.
@@ -104,17 +130,7 @@ export function probeReesSecretAtStartup(env: Env): void {
   // Probe asynchronously — never block the server from starting.
   void (async () => {
     try {
-      const response = await fetch(
-        `${base.replace(/\/+$/, "")}/v1/ping`,
-        {
-          method: "POST",
-          headers: {
-            "user-agent": "gittensory-selfhost/1.0",
-            authorization: `Bearer ${sharedSecret}`,
-          },
-          signal: AbortSignal.timeout(5000),
-        },
-      );
+      const response = await fetchReesPingWithRetry(`${base.replace(/\/+$/, "")}/v1/ping`, sharedSecret);
       if (response.ok) {
         console.log(
           JSON.stringify({

test/unit/enrichment-wire.test.ts

@@ -144,6 +144,46 @@ describe("probeReesSecretAtStartup", () => {
     errSpy.mockRestore();
   });
 
+  it("REGRESSION (#5006, GITTENSORY-1J): retries a 503 ('not ready yet') a few times before escalating to rees_ping_error", async () => {
+    const fetchSpy = vi.fn(async () => ({ ok: false, status: 503 }) as Response);
+    globalThis.fetch = fetchSpy as unknown as typeof fetch;
+    const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+    probeReesSecretAtStartup(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }));
+    await new Promise((resolve) => setTimeout(resolve, 1100));
+    expect(fetchSpy).toHaveBeenCalledTimes(3); // the first attempt + 2 retries, all still 503
+    const parsed = errSpy.mock.calls.map((c) => JSON.parse(c[0] as string));
+    expect(parsed.some((p) => p.event === "rees_ping_error" && p.status === 503)).toBe(true);
+    errSpy.mockRestore();
+  });
+
+  it("REGRESSION (#5006): a 503 that clears on retry succeeds without ever escalating to rees_ping_error", async () => {
+    let calls = 0;
+    const fetchSpy = vi.fn(async () => {
+      calls += 1;
+      return (calls < 2 ? { ok: false, status: 503 } : { ok: true }) as Response;
+    });
+    globalThis.fetch = fetchSpy as unknown as typeof fetch;
+    const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
+    const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+    probeReesSecretAtStartup(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }));
+    await new Promise((resolve) => setTimeout(resolve, 1100));
+    expect(fetchSpy).toHaveBeenCalledTimes(2);
+    expect(logSpy.mock.calls.some((c) => JSON.parse(c[0] as string).event === "rees_ping_ok")).toBe(true);
+    expect(errSpy).not.toHaveBeenCalled();
+    logSpy.mockRestore();
+    errSpy.mockRestore();
+  });
+
+  it("does not retry a non-503 non-ok status — a single attempt is final", async () => {
+    const fetchSpy = vi.fn(async () => ({ ok: false, status: 500 }) as Response);
+    globalThis.fetch = fetchSpy as unknown as typeof fetch;
+    const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+    probeReesSecretAtStartup(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }));
+    await flush();
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+    errSpy.mockRestore();
+  });
+
   it("warns rees_ping_error (not throw) when the fetch itself rejects — REES may not be up yet", async () => {
     const fetchSpy = vi.fn(async () => {
       throw new Error("connect ECONNREFUSED");