fix(review): circuit-break REES enrichment on a confirmed auth rejection

src/review/enrichment-wire.ts

@@ -53,6 +53,21 @@ function sharedSecretWasNormalized(
   return (normalized ?? "") !== raw;
 }
 
+// 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.
+// Cleared only by a process restart -- exactly the action fixing the secret mismatch already requires.
+let reesAuthRejected = false;
+let reesAuthRejectedSkipLoggedCount = 0;
+const MAX_REES_AUTH_REJECTED_SKIP_LOGS = 3;
+
+/** Test-only: this module-level circuit-breaker state otherwise persists for the life of the process (by
+ *  design), which would leak across unrelated test cases sharing this module instance within one test file. */
+export function resetReesAuthRejectedForTests(): void {
+  reesAuthRejected = false;
+  reesAuthRejectedSkipLoggedCount = 0;
+}
+
 /**
  * Fire-and-forget startup probe that POSTs to REES /v1/ping to verify the shared secret matches.
  * Logs rees_ping_ok on success, or rees_secret_mismatch / rees_secret_missing / rees_ping_error on
@@ -109,13 +124,14 @@ export function probeReesSecretAtStartup(env: Env): void {
         );
       } else {
         const isAuthError = response.status === 401 || response.status === 403;
+        if (isAuthError) reesAuthRejected = true;
         console.error(
           JSON.stringify({
             level: "error",
             event: isAuthError ? "rees_secret_mismatch" : "rees_ping_error",
             status: response.status,
             message: isAuthError
-              ? `REES /v1/ping rejected the bearer token (${response.status}). The REES_SHARED_SECRET on this engine does not match the REES_SHARED_SECRET on the REES service. All /v1/enrich calls will fail until both are set to the same bare string.`
+              ? `REES /v1/ping rejected the bearer token (${response.status}). The REES_SHARED_SECRET on this engine does not match the REES_SHARED_SECRET on the REES service. All /v1/enrich calls are disabled for this process lifetime -- restart the engine after fixing both secrets.`
               : `REES /v1/ping returned an unexpected status (${response.status}). Check the REES service logs.`,
           }),
         );
@@ -370,6 +386,23 @@ export async function buildReviewEnrichment(
   const cfg = reesConfig(env);
   const base = cfg.REES_URL?.trim();
   if (!base) return undefined;
+  if (reesAuthRejected) {
+    // The startup probe already confirmed REES rejects this secret -- skip the call rather than repeat a
+    // guaranteed 401/403 on every single PR review. Cap the log volume; the operator already got the loud
+    // rees_secret_mismatch error at startup, this is just a reminder the skip is still active.
+    if (reesAuthRejectedSkipLoggedCount < MAX_REES_AUTH_REJECTED_SKIP_LOGS) {
+      reesAuthRejectedSkipLoggedCount += 1;
+      console.warn(
+        JSON.stringify({
+          level: "warn",
+          event: "rees_enrich_skipped_auth_rejected",
+          message:
+            "Skipping REES /v1/enrich call: startup probe confirmed the shared secret is rejected. Fix REES_SHARED_SECRET on both the engine and the REES service, then restart the engine.",
+        }),
+      );
+    }
+    return undefined;
+  }
   const sharedSecret = normalizeSharedSecret(cfg.REES_SHARED_SECRET);
   const authConfigured = Boolean(sharedSecret);
   const authSecretNormalized = sharedSecretWasNormalized(

test/unit/enrichment-wire.test.ts

@@ -4,6 +4,7 @@ import {
   buildReviewEnrichment,
   isReesGithubTokenForwardingEnabled,
   probeReesSecretAtStartup,
+  resetReesAuthRejectedForTests,
   resolveReesAnalyzers,
   resolveReesAnalyzerBudgetMs,
   resolveReesProfile,
@@ -65,6 +66,10 @@ describe("probeReesSecretAtStartup", () => {
   });
   afterEach(() => {
     globalThis.fetch = realFetch;
+    // The 401/403 test below sets the module-level reesAuthRejected circuit-breaker flag (#3738) for real --
+    // reset it so it can never leak into a LATER test in this file (including buildReviewEnrichment's own
+    // suite, which shares this same module instance via the static import above).
+    resetReesAuthRejectedForTests();
   });
 
   const flush = () => new Promise((resolve) => setTimeout(resolve, 0));
@@ -620,6 +625,85 @@ describe("buildReviewEnrichment", () => {
   });
 });
 
+describe("REGRESSION (#3738): REES auth-rejected circuit breaker", () => {
+  let realFetch: typeof fetch;
+  beforeEach(() => {
+    realFetch = globalThis.fetch;
+  });
+  afterEach(() => {
+    globalThis.fetch = realFetch;
+    resetReesAuthRejectedForTests();
+  });
+
+  const flush = () => new Promise((resolve) => setTimeout(resolve, 0));
+
+  it("buildReviewEnrichment proceeds normally by default (circuit breaker starts closed)", async () => {
+    const fetchSpy = vi.fn(async () => ({ ok: true, json: async () => ({ promptSection: "brief" }) }) as unknown as Response);
+    globalThis.fetch = fetchSpy as unknown as typeof fetch;
+    const r = await buildReviewEnrichment(env({ REES_URL: "https://r" }), input);
+    expect(r?.promptSection).toBe("brief");
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it("once the startup probe confirms a 401, buildReviewEnrichment skips every /v1/enrich call without fetching, for the rest of the process", async () => {
+    const fetchSpy = vi.fn(async () => ({ ok: false, status: 401 }) as Response);
+    globalThis.fetch = fetchSpy as unknown as typeof fetch;
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+    const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+    try {
+      probeReesSecretAtStartup(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }));
+      await flush();
+      fetchSpy.mockClear();
+
+      const r1 = await buildReviewEnrichment(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }), input);
+      const r2 = await buildReviewEnrichment(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }), input);
+      expect(r1).toBeUndefined();
+      expect(r2).toBeUndefined();
+      expect(fetchSpy).not.toHaveBeenCalled(); // never even attempted the doomed call
+      expect(
+        warnSpy.mock.calls.some((c) => JSON.parse(c[0] as string).event === "rees_enrich_skipped_auth_rejected"),
+      ).toBe(true);
+    } finally {
+      warnSpy.mockRestore();
+      errSpy.mockRestore();
+    }
+  });
+
+  it("caps the skip log at 3 occurrences to avoid spamming logs on every subsequent PR review", async () => {
+    globalThis.fetch = vi.fn(async () => ({ ok: false, status: 403 }) as Response) as unknown as typeof fetch;
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+    const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+    try {
+      probeReesSecretAtStartup(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }));
+      await flush();
+      for (let i = 0; i < 5; i += 1) {
+        await buildReviewEnrichment(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }), input);
+      }
+      const skipLogs = warnSpy.mock.calls.filter((c) => JSON.parse(c[0] as string).event === "rees_enrich_skipped_auth_rejected");
+      expect(skipLogs).toHaveLength(3);
+    } finally {
+      warnSpy.mockRestore();
+      errSpy.mockRestore();
+    }
+  });
+
+  it("a NON-auth probe failure (e.g. 500) does NOT trip the circuit breaker -- buildReviewEnrichment still attempts the call", 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(() => {});
+    try {
+      probeReesSecretAtStartup(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }));
+      await flush();
+      fetchSpy.mockClear();
+
+      await buildReviewEnrichment(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }), input);
+      expect(fetchSpy).toHaveBeenCalledTimes(1); // still attempted -- 500 isn't a confirmed secret mismatch
+    } finally {
+      errSpy.mockRestore();
+    }
+  });
+});
+
 describe("isReesGithubTokenForwardingEnabled", () => {
   it("defaults off and only turns on for explicit truthy values", () => {
     expect(isReesGithubTokenForwardingEnabled(env({}))).toBe(false);