perf(test): eliminate two more real-wait retry-backoff hotspots (#8573)

* perf(test): eliminate two more real-wait retry-backoff hotspots

fetchReesPingWithRetry (REES /v1/ping 503 retry) and netuid-verification's
fetchWithRetry both paid their real production backoff delay in tests that
exercise the retry path, adding ~4s of pure wall-clock wait across the
affected suites. Add the same settable-override pattern already used for
the other suite-wide retry delays, defaulting to the real production value
and only overridden to 0 in test/helpers/vitest-setup.ts.

Also switch the two probeReesSecretAtStartup regression tests off a fixed
1100ms sleep-then-assert (needed because the probe is fire-and-forget) onto
vi.waitFor, so they settle as soon as the now-fast retries actually finish
instead of always paying the old worst-case wait.

* perf(test): eliminate real ECONNRESET backoff sleeps in pg-queue tests

retryPoolQuery's real 500ms-multiplier backoff ran for real in the three
PG connection-resilience tests in selfhost-pg-queue.test.ts (only Date was
faked, not setTimeout) -- ~6.5s of pure wall-clock wait across those three
tests alone. Add the same settable-override pattern used elsewhere in this
file's fixtures, scoped to this test file only (not the global vitest-setup
default) since pg-queue.ts isn't otherwise imported broadly across the
suite. Production default and behavior unchanged.

Author: JSONbored

src/review/content-lane/netuid-verification.ts

@@ -37,6 +37,14 @@ const RETRYABLE_STATUS = new Set([408, 425, 429, 500, 502, 503, 504]);
 
 const sleep = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms));
 
+let netuidRetryBaseDelayMsOverride: number | null = null;
+
+/** Test-only: collapses fetchWithRetry's exponential backoff to near-zero so a retry/exhaustion test
+ *  doesn't pay real wall-clock time (the DEFAULT_BASE_DELAY_MS constant and production default are unchanged). */
+export function setNetuidRetryBaseDelayMsForTest(value: number | null): void {
+  netuidRetryBaseDelayMsOverride = value;
+}
+
 /** Minimal fetch-with-retry (inlined from reviewbot core/fetch-retry.ts defaults). Retries on a
  *  thrown error or a retryable status, with exponential backoff + a per-attempt timeout. */
 async function fetchWithRetry(
@@ -46,7 +54,7 @@ async function fetchWithRetry(
   opts: { retries?: number; baseDelayMs?: number; timeoutMs?: number } = {},
 ): Promise<Response> {
   const retries = opts.retries ?? DEFAULT_RETRIES;
-  const baseDelayMs = opts.baseDelayMs ?? DEFAULT_BASE_DELAY_MS;
+  const baseDelayMs = opts.baseDelayMs ?? netuidRetryBaseDelayMsOverride ?? DEFAULT_BASE_DELAY_MS;
   const timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS;
   let lastError: unknown;
   for (let attempt = 0; attempt <= retries; attempt += 1) {

src/review/enrichment-wire.ts

@@ -73,6 +73,18 @@ function sharedSecretWasNormalized(
 const REES_PING_NOT_READY_RETRIES = 2;
 const REES_PING_NOT_READY_RETRY_DELAY_MS = 500;
 
+let reesPingNotReadyRetryDelayMsOverride: number | null = null;
+
+/** Test-only: collapses the real inter-retry wait so probeReesSecretAtStartup's retry tests don't pay
+ *  REES_PING_NOT_READY_RETRIES * REES_PING_NOT_READY_RETRY_DELAY_MS of real wall-clock time. */
+export function setReesPingNotReadyRetryDelayMsForTest(value: number | null): void {
+  reesPingNotReadyRetryDelayMsOverride = value;
+}
+
+function reesPingNotReadyRetryDelayMs(): number {
+  return reesPingNotReadyRetryDelayMsOverride ?? REES_PING_NOT_READY_RETRY_DELAY_MS;
+}
+
 async function fetchReesPingWithRetry(url: string, secret: string): Promise<Response> {
   const request = () =>
     fetch(url, {
@@ -85,7 +97,7 @@ async function fetchReesPingWithRetry(url: string, secret: string): Promise<Resp
     });
   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));
+    await new Promise((resolve) => setTimeout(resolve, reesPingNotReadyRetryDelayMs()));
     response = await request();
   }
   return response;

src/selfhost/pg-queue.ts

@@ -86,17 +86,26 @@ function isPgSqlStateConnectionError(err: unknown): boolean {
   return hasErrorCode(err, PG_SQLSTATE_CONNECTION_CODES);
 }
 
+let pgRetryPoolQueryDelayMsOverride: number | null = null;
+
+/** Test-only: collapses retryPoolQuery's per-attempt backoff to near-zero so a connection-error retry
+ *  test doesn't pay real wall-clock time (the delayMs default and production behavior are unchanged). */
+export function setPgRetryPoolQueryDelayMsForTest(value: number | null): void {
+  pgRetryPoolQueryDelayMsOverride = value;
+}
+
 /** Retry a pool query up to `retries` times on transient connection errors, with a short delay
  *  between attempts. The pool will establish a new connection automatically. */
 async function retryPoolQuery<T>(fn: () => Promise<T>, retries = 3, delayMs = 500): Promise<T> {
+  const effectiveDelayMs = pgRetryPoolQueryDelayMsOverride ?? delayMs;
   let lastErr: unknown;
   for (let attempt = 0; attempt <= retries; attempt++) {
     try {
       return await fn();
     } catch (err) {
       lastErr = err;
       if (!isPgConnectionError(err) || attempt === retries) throw err;
-      await new Promise((resolve) => setTimeout(resolve, delayMs * (attempt + 1)));
+      await new Promise((resolve) => setTimeout(resolve, effectiveDelayMs * (attempt + 1)));
     }
   }
   throw lastErr;

test/helpers/vitest-setup.ts

@@ -9,7 +9,11 @@
 import { setReviewFilesEmptyRetryDelayMsForTest } from "../../src/github/backfill";
 import { setGithubRateLimitRetrySleepCapMsForTest } from "../../src/github/client";
 import { setMergeStateUnknownRetryDelayMsForTest } from "../../src/queue/ci-resolution";
+import { setReesPingNotReadyRetryDelayMsForTest } from "../../src/review/enrichment-wire";
+import { setNetuidRetryBaseDelayMsForTest } from "../../src/review/content-lane/netuid-verification";
 
 setReviewFilesEmptyRetryDelayMsForTest(0);
 setGithubRateLimitRetrySleepCapMsForTest(0);
 setMergeStateUnknownRetryDelayMsForTest(0);
+setReesPingNotReadyRetryDelayMsForTest(0);
+setNetuidRetryBaseDelayMsForTest(0);

test/unit/enrichment-wire.test.ts

@@ -150,8 +150,9 @@ describe("probeReesSecretAtStartup", () => {
     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
+    // fetchReesPingWithRetry is fire-and-forget; poll instead of sleeping the retry budget's worst case
+    // (the test's own vitest-setup override collapses the real inter-retry delay to 0, so this settles fast).
+    await vi.waitFor(() => 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();
@@ -167,8 +168,7 @@ describe("probeReesSecretAtStartup", () => {
     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);
+    await vi.waitFor(() => 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();

test/unit/selfhost-pg-queue.test.ts

@@ -2,7 +2,7 @@
 // Real-Postgres integration paths (migrations, pg-adapter translation) live in test/integration/selfhost-pg.test.ts.
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import type { Pool, QueryResult } from "pg";
-import { createPgQueue } from "../../src/selfhost/pg-queue";
+import { createPgQueue, setPgRetryPoolQueryDelayMsForTest } from "../../src/selfhost/pg-queue";
 import { queueSnapshotFromBinding } from "../../src/selfhost/queue-common";
 import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics";
 import { RetryableJobError } from "../../src/queue/retryable";
@@ -15,6 +15,11 @@ import type { JobMessage } from "../../src/types";
 // "unavailable" (null, never gates) here; individual host-load tests override the mock explicitly.
 vi.mock("../../src/selfhost/host-pressure", () => ({ hostLoadAvg1PerCore: vi.fn(() => null) }));
 
+// The PG connection-resilience tests below deliberately trigger retryPoolQuery's real ECONNRESET retry
+// path; collapse its per-attempt backoff to near-zero so they don't pay real wall-clock time for it
+// (the delayMs default and production behavior in src/selfhost/pg-queue.ts are unchanged).
+setPgRetryPoolQueryDelayMsForTest(0);
+
 const msg = (t: string): JobMessage => ({ type: t }) as unknown as JobMessage;
 const webhook = (sender: { login: string; type: string }, eventName = "issue_comment", action = "edited"): JobMessage =>
   ({