fix(api): share one probe-target resolution between both proxy health checks (#10657)

Merged — locally validated (15/15 focused tests, typecheck:core clean, file-size/changelog gates green) after resolving base-drift against #10654 (both landed today, same file — combined import block, no logical conflict). Thanks!

src/app/api/settings/proxies/auto-test/route.ts

@@ -6,14 +6,20 @@ import { createProxyDispatcher, proxyConfigToUrl } from "@omniroute/open-sse/uti
 import { fetch as undiciFetch } from "undici";
 import { classifyProbeStatus } from "@/lib/proxyHealth/decision";
 import { resolveHealthCheckStatusWrite } from "@/lib/proxyHealth/statusPolicy";
+import {
+  resolveProbeConcurrency,
+  resolveProbeStaggerMs,
+  resolveProbeTarget,
+  waitForProbeSlot,
+} from "@/lib/proxyHealth/probeTarget";
 import { isValidationFailure, validateBody } from "@/shared/validation/helpers";
 import { createErrorResponse } from "@/lib/api/errorResponse";
 
 const TEST_TIMEOUT_MS = 5000;
-// Reachability probe target. Configurable so operators can point it at an
-// internal/self-hosted endpoint instead of the public default.
-const TEST_URL = process.env.PROXY_HEALTH_TEST_URL || "https://httpbin.org/ip";
-const CONCURRENCY = 10;
+// Shared with the background sweep — see src/lib/proxyHealth/probeTarget.ts.
+const TEST_URL = resolveProbeTarget();
+const CONCURRENCY = resolveProbeConcurrency();
+const STAGGER_MS = resolveProbeStaggerMs();
 
 const autoTestSchema = z.object({
   ids: z.array(z.string()).optional(),
@@ -149,7 +155,14 @@ export async function POST(request: Request) {
     const results: TestResult[] = [];
     for (let i = 0; i < proxiesToTest.length; i += CONCURRENCY) {
       const batch = proxiesToTest.slice(i, i + CONCURRENCY);
-      const batchResults = await Promise.allSettled(batch.map((proxy) => testSingleProxy(proxy)));
+      const batchResults = await Promise.allSettled(
+        batch.map(async (proxy, indexInBatch) => {
+          // Same intra-batch spacing as the background sweep: "Test All" fires the whole
+          // batch at once too, so it is just as capable of tripping a rate-limited target.
+          await waitForProbeSlot(indexInBatch, STAGGER_MS);
+          return testSingleProxy(proxy);
+        })
+      );
       for (const result of batchResults) {
         if (result.status === "fulfilled") results.push(result.value);
       }

src/lib/proxyHealth/probeTarget.ts

@@ -0,0 +1,84 @@
+/**
+ * Shared resolution of the reachability-probe parameters (#8411).
+ *
+ * The scheduler sweep and the bulk "Test All" endpoint each carried their own copy of the
+ * probe target and batch size. Both now resolve through here, so an operator tunes one
+ * surface instead of two that can silently drift apart.
+ *
+ * The resolvers are pure and take the environment as a parameter, so tests never have to
+ * mutate `process.env`.
+ */
+
+import { sleep } from "@omniroute/open-sse/utils/sleep";
+
+export const DEFAULT_PROBE_TARGET = "https://httpbin.org/ip";
+export const DEFAULT_PROBE_CONCURRENCY = 10;
+export const DEFAULT_PROBE_STAGGER_MS = 100;
+
+/**
+ * Upper bounds. Making the batch size configurable without a ceiling would let a single
+ * env var recreate the very probe storm this module exists to damp.
+ */
+export const MAX_PROBE_CONCURRENCY = 50;
+export const MAX_PROBE_STAGGER_MS = 5000;
+
+type ProbeEnv = Record<string, string | undefined>;
+
+function resolveBoundedInt(raw: string | undefined, fallback: number, min: number, max: number) {
+  const parsed = parseInt(raw ?? "", 10);
+  if (!Number.isFinite(parsed)) return fallback;
+  return Math.min(Math.max(parsed, min), max);
+}
+
+/**
+ * Deliberately keeps the historical `||` semantics: a target that is empty falls back to the
+ * default, but one made only of whitespace is passed through untouched. Trimming it here would
+ * silently change how an existing deployment behaves.
+ */
+export function resolveProbeTarget(env: ProbeEnv = process.env): string {
+  return env.PROXY_HEALTH_TEST_URL || DEFAULT_PROBE_TARGET;
+}
+
+/** Floored at 1: a zero batch size would make the `i += concurrency` loop never advance. */
+export function resolveProbeConcurrency(env: ProbeEnv = process.env): number {
+  return resolveBoundedInt(
+    env.PROXY_HEALTH_TEST_CONCURRENCY,
+    DEFAULT_PROBE_CONCURRENCY,
+    1,
+    MAX_PROBE_CONCURRENCY
+  );
+}
+
+export function resolveProbeStaggerMs(env: ProbeEnv = process.env): number {
+  return resolveBoundedInt(
+    env.PROXY_HEALTH_TEST_STAGGER_MS,
+    DEFAULT_PROBE_STAGGER_MS,
+    0,
+    MAX_PROBE_STAGGER_MS
+  );
+}
+
+/**
+ * Delay before the Nth probe of a batch starts.
+ *
+ * A batch fires `Promise.allSettled(batch.map(...))`, so without this every probe leaves at the
+ * same tick and a shared egress IP hits the target with `concurrency` simultaneous requests.
+ * Spacing the departures is what removes that spike; the delay is a plain multiple of the index
+ * rather than a random jitter so a sweep stays reproducible and exactly testable.
+ *
+ * The first probe of a batch always returns 0 — no batch is ever slowed down at its head.
+ */
+export function staggerDelayMs(indexInBatch: number, stepMs: number): number {
+  if (indexInBatch <= 0 || stepMs <= 0) return 0;
+  return indexInBatch * stepMs;
+}
+
+/**
+ * Hold a probe back until its slot in the batch. Call this from the batch `map`, before the
+ * probe itself: both call sites arm their timeout inside their own test function, so waiting
+ * out here is what keeps every probe's timeout budget whole.
+ */
+export async function waitForProbeSlot(indexInBatch: number, stepMs: number): Promise<void> {
+  const delay = staggerDelayMs(indexInBatch, stepMs);
+  if (delay > 0) await sleep(delay);
+}

src/lib/proxyHealth/scheduler.ts

@@ -34,16 +34,24 @@ import {
   decideProxyHealthAction,
   type ProxyProbeOutcome,
 } from "./decision.ts";
+import {
+  resolveProbeConcurrency,
+  resolveProbeStaggerMs,
+  resolveProbeTarget,
+  waitForProbeSlot,
+} from "./probeTarget.ts";
 
 // #6246: a HEAD to the public probe target through a legit (often loaded) proxy
 // can exceed a few seconds; the old 5s ceiling produced false negatives that
 // flipped healthy proxies to inactive. Raise it and treat our own timeout as
 // inconclusive (see testOneProxy) rather than a proxy failure.
 const TEST_TIMEOUT_MS = 15000;
-// Reachability probe target for proxy health checks. Configurable so operators
-// can point it at an internal/self-hosted endpoint instead of the public default.
-const TEST_URL = process.env.PROXY_HEALTH_TEST_URL || "https://httpbin.org/ip";
-const CONCURRENCY = 10;
+// Probe target, batch size and intra-batch spacing come from probeTarget.ts, which the
+// auto-test endpoint reads too — one surface to tune instead of two that can drift apart.
+// Resolved at module load, as these constants always were.
+const TEST_URL = resolveProbeTarget();
+const CONCURRENCY = resolveProbeConcurrency();
+const STAGGER_MS = resolveProbeStaggerMs();
 const INITIAL_DELAY_MS = 60_000;
 const DEFAULT_INTERVAL_MS = 600_000;
 const DEFAULT_REMOVE_AFTER = 3;
@@ -160,7 +168,10 @@ async function sweep(): Promise<void> {
   for (let i = 0; i < proxies.length; i += CONCURRENCY) {
     const batch = proxies.slice(i, i + CONCURRENCY);
     const results = await Promise.allSettled(
-      batch.map(async (proxy) => {
+      batch.map(async (proxy, indexInBatch) => {
+        // Spread the departures: without this the whole batch leaves at the same tick and a
+        // shared egress IP hits the target with CONCURRENCY simultaneous requests.
+        await waitForProbeSlot(indexInBatch, STAGGER_MS);
         const outcome = await testOneProxy(proxy);
         return { id: proxy.id, outcome };
       })