feat(credential-health): per-connection sweep interval via healthCheckInterval (#8443) (#10687)

Merged — validated together with a batch of related maxmad64bis PRs in one combined worktree (typecheck:core clean, complexity/cognitive-complexity/file-size/changelog gates green, focused tests passing). Thanks for the contribution!

changelog.d/features/8443-credential-health-per-connection-interval.md

@@ -0,0 +1,2 @@
+- **feat(credential-health):** pace the credential health sweep per connection via `provider_connections.healthCheckInterval` (minutes, 0 = never), with `CREDENTIAL_HEALTH_CHECK_INTERVAL` as the global default ([#8443](https://github.com/diegosouzapw/OmniRoute/issues/8443))
+- **behavior change:** `healthCheckInterval` is a shared column — it paces both the OAuth token refresh and the credential health sweep, and `0` disables both. The connection editor defaults it to 60, so configured OAuth connections are now credential-checked at 60min instead of the previous ~10min (aligned with the probe-volume goal of #8443)

src/lib/credentialHealth/scheduler.ts

@@ -11,7 +11,8 @@
  * Schedule:
  *   - Initial delay: 30s after server boot (allows DB migrations to complete)
  *   - Interval: configurable via CREDENTIAL_HEALTH_CHECK_INTERVAL (default 5 min)
- *   - OAuth connections: tested less frequently (2x interval)
+ *   - Per-connection override: provider_connections.healthCheckInterval (minutes,
+ *     0 = never test this connection) paces each connection individually
  *   - Backoff on failure: 5min -> 10min -> 30min -> max 2h
  *   - Resets to default on success
  */
@@ -31,7 +32,6 @@ import { SEARCH_VALIDATOR_CONFIGS } from "@/lib/providers/validation/searchProvi
 
 const BACKOFF_SCHEDULE = [300_000, 600_000, 1_800_000, 7_200_000]; // 5min, 10min, 30min, 2h
 const INITIAL_DELAY_MS = 30_000; // Wait for server boot
-const OAUTH_INTERVAL_MULTIPLIER = 2; // OAuth tested 2x less frequently
 const CONCURRENCY_LIMIT = 5; // Max simultaneous connection tests
 const LOG_PREFIX = "[CredentialHealth]";
 const TRUE_ENV_VALUES = new Set(["1", "true", "yes", "on"]);
@@ -90,30 +90,37 @@ function getSweepInterval(): number {
   return 300_000; // default 5 min
 }
 
+/**
+ * Resolve the per-connection sweep interval (ms).
+ * - `healthCheckInterval > 0` → minutes × 60 000 (per-connection override)
+ * - `healthCheckInterval <= 0` → null (never test this connection — opt-out)
+ * - absent → global env interval (getSweepInterval())
+ */
+function getConnIntervalMs(conn: { healthCheckInterval?: number | null }): number | null {
+  const minutes = conn.healthCheckInterval;
+  if (minutes === null || minutes === undefined) return getSweepInterval();
+  if (minutes <= 0) return null;
+  return minutes * 60_000;
+}
+
 function getNextBackoff(connectionId: string): number {
   const state = getSchedulerState();
   const failures = state.failureCounts.get(connectionId) ?? 0;
   return BACKOFF_SCHEDULE[Math.min(failures, BACKOFF_SCHEDULE.length - 1)];
 }
 
-function getMaxFailuresAcrossConnections(): number {
-  const state = getSchedulerState();
-  let max = 0;
-  for (const count of state.failureCounts.values()) {
-    if (count > max) max = count;
-  }
-  return max;
-}
-
 // ── Core Sweep Logic ─────────────────────────────────────────────────────
 
 async function testConnection(
   connectionId: string,
   provider: string,
-  isOAuth: boolean
+  intervalMs: number | null
 ): Promise<void> {
   const startTime = Date.now();
 
+  // Per-connection opt-out: healthCheckInterval <= 0 → never test.
+  if (intervalMs === null) return;
+
   let oldStatus: string | undefined;
   try {
     const { getCredentialHealth } = await import("@/lib/credentialHealth/cache");
@@ -130,9 +137,13 @@ async function testConnection(
     const state = getSchedulerState();
 
     if (result.valid) {
-      // Success — reset failure count + timing, update cache
+      // Success — reset failure count, space the next test by the
+      // per-connection interval (absent → global sweep interval), update cache
       state.failureCounts.delete(connectionId);
-      state.perConnTiming.delete(connectionId);
+      state.perConnTiming.set(connectionId, {
+        lastAttemptAt: startTime,
+        nextAttemptAt: startTime + intervalMs,
+      });
       setCredentialHealth(
         connectionId,
         provider,
@@ -228,6 +239,7 @@ export async function sweep(): Promise<void> {
       id: string;
       provider: string;
       authType?: string;
+      healthCheckInterval?: number | null;
     }>;
 
     try {
@@ -244,6 +256,7 @@ export async function sweep(): Promise<void> {
         id: string;
         provider: string;
         authType?: string;
+        healthCheckInterval?: number | null;
       }>;
     } catch (err) {
       console.error(LOG_PREFIX, "Failed to load provider connections:", err);
@@ -254,12 +267,14 @@ export async function sweep(): Promise<void> {
 
     // Compute backoff per connection — skip connections that aren't due yet
     const now = Date.now();
-    const interval = getSweepInterval();
 
     const dueConnections = connections.filter((conn) => {
+      const intervalMs = getConnIntervalMs(conn);
+      // Per-connection opt-out: never tested.
+      if (intervalMs === null) return false;
       const state_ = getSchedulerState();
       const timing = state_.perConnTiming.get(conn.id);
-      // No timing entry = never tested or healthy → due now
+      // No timing entry = never tested since boot → due now
       if (!timing) return true;
       // Time-based: due when the current time has passed the next attempt time
       return now >= timing.nextAttemptAt;
@@ -280,7 +295,7 @@ export async function sweep(): Promise<void> {
 
     for (const batch of batches) {
       await Promise.allSettled(
-        batch.map((conn) => testConnection(conn.id, conn.provider, conn.authType === "oauth"))
+        batch.map((conn) => testConnection(conn.id, conn.provider, getConnIntervalMs(conn)))
       );
     }
   } finally {

tests/unit/credential-health-backoff-retry.test.ts

@@ -6,10 +6,12 @@
  * a time-based per-connection backoff check (`nextAttemptAt`). This test
  * validates that:
  * 1. Connections with failures are retried after the backoff period elapses
- * 2. Healthy connections (no timing entry) are always due
+ * 2. Healthy connections (no timing entry) are always due; after a success the
+ *    connection is due again once its per-connection interval has elapsed
  * 3. OAuth connections respect the same time-based backoff
  * 4. Multiple failure levels have correct backoff durations
- * 5. The `scheduleSweep()` no longer couples to `maxFailuresAcrossConnections`
+ * 5. The `scheduleSweep()` runs on a stable interval independent of
+ *    per-connection failures
  */
 
 import test from "node:test";
@@ -108,19 +110,28 @@ test("never-tested connection is always due (no perConnTiming entry)", () => {
   );
 });
 
-test("connection after success (timing cleared) is due immediately", () => {
+test("connection after success (timing set to interval) is due after the interval elapses", () => {
   const perConnTiming = new Map<string, { lastAttemptAt: number; nextAttemptAt: number }>();
   const connId = "conn-bug-9289";
   const now = 1_000_000_000_000;
 
-  // Simulate failure then success (timing deleted)
-  perConnTiming.set(connId, { lastAttemptAt: now, nextAttemptAt: now + 600_000 });
-  perConnTiming.delete(connId); // On success, timing is cleared
+  // Simulate failure then success: timing now holds lastAttemptAt + interval
+  perConnTiming.set(connId, { lastAttemptAt: now, nextAttemptAt: now + DEFAULT_INTERVAL });
 
   assert.equal(
-    isConnectionDue(perConnTiming, connId, now),
+    isConnectionDue(perConnTiming, connId, now + DEFAULT_INTERVAL - 1),
+    false,
+    "Healthy connection should NOT be due before its interval elapses"
+  );
+  assert.equal(
+    isConnectionDue(perConnTiming, connId, now + DEFAULT_INTERVAL),
     true,
-    "Connection should be due immediately after success (timing cleared)"
+    "Healthy connection should be due at the interval boundary"
+  );
+  assert.equal(
+    isConnectionDue(perConnTiming, connId, now + DEFAULT_INTERVAL + 1),
+    true,
+    "Healthy connection should be due after its interval elapses"
   );
 });
 
@@ -152,7 +163,7 @@ test("multiple failure levels have correct backoff durations", () => {
 });
 
 test("scheduleSweep uses stable interval (decoupled from maxFailures)", () => {
-  // The fix decouples scheduleSweep from getMaxFailuresAcrossConnections.
+  // The fix decouples scheduleSweep from per-connection failures.
   // Previously, one failed connection would delay the global sweep for all
   // connections. Now the global sweep runs on a stable interval regardless
   // of individual connection failures. This test validates the new behavior
@@ -179,4 +190,4 @@ test("scheduleSweep uses stable interval (decoupled from maxFailures)", () => {
     true,
     "Failed connection should be due when its own backoff elapses"
   );
-});
+});

tests/unit/credential-health-interval.test.ts

@@ -0,0 +1,100 @@
+/**
+ * Unit tests for the per-connection credential health sweep interval (#8443).
+ *
+ * Mirrors the house pattern of credential-health-backoff-retry.test.ts: the
+ * scheduler predicates are replicated here rather than imported, because the
+ * scheduler module auto-initializes on import.
+ *
+ * Validates:
+ * 1. getConnIntervalMs: null → global env interval; >0 → minutes × 60 000;
+ *    <=0 → null (per-connection opt-out, never tested)
+ * 2. isConnectionDue: success timing (lastAttemptAt + interval) is respected —
+ *    not due before the interval, due at/after it
+ * 3. Opt-out connections (intervalMs null) are excluded from the due filter
+ */
+
+import test from "node:test";
+import assert from "node:assert/strict";
+
+// ── Constants (mirrored from scheduler.ts) ────────────────────────────────
+
+const DEFAULT_INTERVAL = 300_000; // 5 min
+
+// ── Helper: replicated scheduler predicates ───────────────────────────────
+
+function getConnIntervalMs(conn: { healthCheckInterval?: number | null }): number | null {
+  const minutes = conn.healthCheckInterval;
+  if (minutes === null || minutes === undefined) return DEFAULT_INTERVAL;
+  if (minutes <= 0) return null;
+  return minutes * 60_000;
+}
+
+function isConnectionDue(
+  perConnTiming: Map<string, { lastAttemptAt: number; nextAttemptAt: number }>,
+  connId: string,
+  now: number,
+  intervalMs: number | null
+): boolean {
+  // Per-connection opt-out: never tested.
+  if (intervalMs === null) return false;
+  const timing = perConnTiming.get(connId);
+  // No timing entry = never tested since boot → due now
+  if (!timing) return true;
+  // Time-based: due when the current time has passed the next attempt time
+  return now >= timing.nextAttemptAt;
+}
+
+// ── Tests ─────────────────────────────────────────────────────────────────
+
+test("getConnIntervalMs: absent healthCheckInterval falls back to the global interval", () => {
+  assert.equal(getConnIntervalMs({}), DEFAULT_INTERVAL);
+  assert.equal(getConnIntervalMs({ healthCheckInterval: null }), DEFAULT_INTERVAL);
+});
+
+test("getConnIntervalMs: positive minutes override becomes milliseconds", () => {
+  assert.equal(getConnIntervalMs({ healthCheckInterval: 1 }), 60_000);
+  assert.equal(getConnIntervalMs({ healthCheckInterval: 5 }), 300_000);
+  assert.equal(getConnIntervalMs({ healthCheckInterval: 60 }), 3_600_000);
+});
+
+test("getConnIntervalMs: zero or negative means opt-out (null)", () => {
+  assert.equal(getConnIntervalMs({ healthCheckInterval: 0 }), null);
+  assert.equal(getConnIntervalMs({ healthCheckInterval: -5 }), null);
+});
+
+test("opt-out connection (intervalMs null) is never due", () => {
+  const perConnTiming = new Map<string, { lastAttemptAt: number; nextAttemptAt: number }>();
+  const now = 1_000_000_000_000;
+  assert.equal(isConnectionDue(perConnTiming, "conn-optout", now, null), false);
+});
+
+test("connection with success timing is due at the interval boundary, not before", () => {
+  const perConnTiming = new Map<string, { lastAttemptAt: number; nextAttemptAt: number }>();
+  const connId = "conn-healthy";
+  const now = 1_000_000_000_000;
+  const intervalMs = DEFAULT_INTERVAL;
+
+  // Simulate a success: timing holds lastAttemptAt + interval
+  perConnTiming.set(connId, { lastAttemptAt: now, nextAttemptAt: now + intervalMs });
+
+  assert.equal(
+    isConnectionDue(perConnTiming, connId, now + intervalMs - 1, intervalMs),
+    false,
+    "Not due before the per-connection interval elapses"
+  );
+  assert.equal(
+    isConnectionDue(perConnTiming, connId, now + intervalMs, intervalMs),
+    true,
+    "Due at the interval boundary"
+  );
+  assert.equal(
+    isConnectionDue(perConnTiming, connId, now + intervalMs + 1, intervalMs),
+    true,
+    "Due after the interval elapses"
+  );
+});
+
+test("never-tested connection is always due (no perConnTiming entry)", () => {
+  const perConnTiming = new Map<string, { lastAttemptAt: number; nextAttemptAt: number }>();
+  assert.equal(isConnectionDue(perConnTiming, "conn-fresh", Date.now(), DEFAULT_INTERVAL), true);
+});