diff --git a/changelog.d/features/8443-credential-health-per-connection-interval.md b/changelog.d/features/8443-credential-health-per-connection-interval.md new file mode 100644 index 0000000000..1fd1d3e5a2 --- /dev/null +++ b/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) diff --git a/src/lib/credentialHealth/scheduler.ts b/src/lib/credentialHealth/scheduler.ts index 580111b9a1..7eff86c4df 100644 --- a/src/lib/credentialHealth/scheduler.ts +++ b/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 { 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 { id: string; provider: string; authType?: string; + healthCheckInterval?: number | null; }>; try { @@ -244,6 +256,7 @@ export async function sweep(): Promise { 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 { // 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 { 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 { diff --git a/tests/unit/credential-health-backoff-retry.test.ts b/tests/unit/credential-health-backoff-retry.test.ts index fb461df91c..f05b5774ac 100644 --- a/tests/unit/credential-health-backoff-retry.test.ts +++ b/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(); 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" ); -}); \ No newline at end of file +}); diff --git a/tests/unit/credential-health-interval.test.ts b/tests/unit/credential-health-interval.test.ts new file mode 100644 index 0000000000..8ef3af138c --- /dev/null +++ b/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, + 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(); + 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(); + 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(); + assert.equal(isConnectionDue(perConnTiming, "conn-fresh", Date.now(), DEFAULT_INTERVAL), true); +});