From 237f880af93742549b77e6249ae4d01cf6e24f09 Mon Sep 17 00:00:00 2001 From: Abhishek Divekar Date: Sun, 27 Sep 2026 11:02:19 +0530 Subject: [PATCH 1/2] fix(credential-health): stop the sweep defaulting to 5 minutes #12138 raised the credential health sweep default to 60 minutes but left the 5-minute value behind in four places, so the sweep could still run at 5 min: - getConnIntervalMs() kept `globalIntervalMs = 300_000` as its default parameter, so any caller omitting the second argument probed connections five times more often than the operator's configured cadence. Both current call sites pass it, so this was latent rather than live. - open-sse/config/constants.ts exported CREDENTIAL_HEALTH_CHECK_INTERVAL with a 300_000 fallback and a doc comment naming 5 minutes. Nothing reads it, but ENVIRONMENT.md cites it as the source of the default. - docs/reference/ENVIRONMENT.md advertised 300000. - src/lib/copilot/systemPrompt.ts documented 300000 in its config table. The per-connection fallback now reads the same DEFAULT_SWEEP_INTERVAL_MS the global resolver uses (hoisted into the probePolicy leaf so scheduler.ts can import it without a cycle), so the two cannot drift apart again. Three tests pin the 60-minute fallback, the 0 opt-out, and the per-connection override. --- .../credential-health-stale-5m-default.md | 1 + docs/reference/ENVIRONMENT.md | 2 +- open-sse/config/constants.ts | 4 ++-- src/lib/copilot/systemPrompt.ts | 2 +- src/lib/credentialHealth/probePolicy.ts | 7 ++++++- src/lib/credentialHealth/scheduler.ts | 11 +++++----- .../credential-health-sweep-interval.test.ts | 20 ++++++++++++++++++- 7 files changed, 36 insertions(+), 11 deletions(-) create mode 100644 changelog.d/fixes/credential-health-stale-5m-default.md diff --git a/changelog.d/fixes/credential-health-stale-5m-default.md b/changelog.d/fixes/credential-health-stale-5m-default.md new file mode 100644 index 00000000000..c83a2e007ca --- /dev/null +++ b/changelog.d/fixes/credential-health-stale-5m-default.md @@ -0,0 +1 @@ +- **fix(credential-health):** the credential health sweep could probe connections every 5 minutes instead of the 60-minute default #12138 established. `getConnIntervalMs` kept a `300_000` default parameter, and `docs/reference/ENVIRONMENT.md`, `open-sse/config/constants.ts` and the scheduler header comment all still advertised 5 minutes. The per-connection fallback now reads the same constant as the global default, so the two cannot drift apart again. Operators seeing frequent probe traffic should check `healthCheckInterval` on the connection and the Resilience settings interval — `0` disables probing entirely. diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index 5363eac4ea7..d8993219e22 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -180,7 +180,7 @@ OmniRoute uses **SQLite** (via `better-sqlite3`) for all persistence. These vari | `OMNIROUTE_SKIP_DB_HEALTHCHECK` | _(unset)_ | `src/lib/db/core.ts` / `src/lib/db/healthCheck.ts` | Set to `1` to skip the SQLite integrity health check on startup. Useful for faster boot on large databases. | | `NOTIFY_SOCKET` | _(unset)_ | systemd (sd_notify protocol) | Set by systemd when the process runs under a service unit with sd_notify integration; OmniRoute reads it (see `OMNIROUTE_DISABLE_SD_NOTIFY`) to send READY/WATCHDOG notifications. Never set by the user. | | `OMNIROUTE_DISABLE_SD_NOTIFY` | _(unset)_ | `scripts/dev/systemd-notify.mjs` | Set to `1` to disable systemd sd_notify (Type=notify / WatchdogSec=) even when running under a systemd unit. The notifier is a no-op outside systemd regardless. | -| `CREDENTIAL_HEALTH_CHECK_INTERVAL` | `300000` | `open-sse/config/constants.ts` / `src/lib/credentialHealth/scheduler.ts` | Interval (ms) for the background credential health check scheduler. Minimum: 10000 (10s). | +| `CREDENTIAL_HEALTH_CHECK_INTERVAL` | `3600000` | `open-sse/config/constants.ts` / `src/lib/credentialHealth/scheduler.ts` | Interval (ms) for the background credential health check scheduler. Minimum: 10000 (10s). | | `CREDENTIAL_HEALTH_CACHE_TTL` | `300000` | `open-sse/config/constants.ts` / `src/lib/credentialHealth/cache.ts` | TTL (ms) for cached credential health status. | | `OMNIROUTE_DISABLE_CREDENTIAL_HEALTH_CHECK` | `false` | `src/lib/credentialHealth/scheduler.ts` | Set to `1` or `true` to disable background periodic testing of provider connections. Search providers (SEARCH_VALIDATOR_CONFIGS in `src/lib/providers/validation/searchProviders.ts`, e.g. `tavily-search`) are always excluded from the sweep — their "validation" is a real billed upstream query, so they are never health-checked on a timer (#9970). | | `DEEP_HEALTH_CHECK_ENABLED` | `0` | `src/app/api/monitoring/health/route.ts` | Set to `1` to allow an authenticated caller to request `/api/monitoring/health?deep=1`, which samples the completions surface once per TTL. Anonymous callers never trigger the probe. | diff --git a/open-sse/config/constants.ts b/open-sse/config/constants.ts index a0f9be98806..797d904e027 100644 --- a/open-sse/config/constants.ts +++ b/open-sse/config/constants.ts @@ -326,7 +326,7 @@ export const MAX_TOOLS_LIMIT = 128; /** * Interval (ms) for the background credential health check scheduler. - * Default: 300000 (5 minutes). Minimum: 10000 (10 seconds). + * Default: 3600000 (60 minutes). Minimum: 10000 (10 seconds). */ export const CREDENTIAL_HEALTH_CHECK_INTERVAL = (() => { const raw = process.env.CREDENTIAL_HEALTH_CHECK_INTERVAL; @@ -334,7 +334,7 @@ export const CREDENTIAL_HEALTH_CHECK_INTERVAL = (() => { const parsed = Number(raw); if (Number.isFinite(parsed) && parsed >= 10_000) return parsed; } - return 300_000; + return 3_600_000; })(); /** diff --git a/src/lib/copilot/systemPrompt.ts b/src/lib/copilot/systemPrompt.ts index 3633e6e5a18..b74b85af789 100644 --- a/src/lib/copilot/systemPrompt.ts +++ b/src/lib/copilot/systemPrompt.ts @@ -146,7 +146,7 @@ Cache, compression, 1proxy, memory, skills tools | DATA_DIR | Data directory | ~/.omniroute/ | | PORT | HTTP server port | 20128 | | REQUIRE_API_KEY | Force API key auth | false | -| CREDENTIAL_HEALTH_CHECK_INTERVAL | Health check interval (ms) | 300000 | +| CREDENTIAL_HEALTH_CHECK_INTERVAL | Health check interval (ms) | 3600000 | | CREDENTIAL_HEALTH_CACHE_TTL | Credential cache TTL (ms) | 300000 | | OMNIROUTE_DISABLE_CREDENTIAL_HEALTH_CHECK | Disable health check | off | diff --git a/src/lib/credentialHealth/probePolicy.ts b/src/lib/credentialHealth/probePolicy.ts index 6b4d387e36c..2c6f8a39306 100644 --- a/src/lib/credentialHealth/probePolicy.ts +++ b/src/lib/credentialHealth/probePolicy.ts @@ -1,4 +1,9 @@ -const DEFAULT_SWEEP_INTERVAL_MS = 300_000; +/** + * The sweep cadence assumed when the caller cannot supply one. Lives in this + * leaf (scheduler.ts imports it) so the two cannot drift apart again: #12138 + * raised the real default to 60 min but left this at 5 min. + */ +export const DEFAULT_SWEEP_INTERVAL_MS = 3_600_000; const INCONCLUSIVE_RECHECK_MIN_MS = 30 * 60_000; const INCONCLUSIVE_RECHECK_MULTIPLIER = 6; const INCONCLUSIVE_WARNING_MARKER = "credential validity is inconclusive"; diff --git a/src/lib/credentialHealth/scheduler.ts b/src/lib/credentialHealth/scheduler.ts index 51507ed9e9e..fd27cc72e3d 100644 --- a/src/lib/credentialHealth/scheduler.ts +++ b/src/lib/credentialHealth/scheduler.ts @@ -10,7 +10,7 @@ * * Schedule: * - Initial delay: 30s after server boot (allows DB migrations to complete) - * - Interval: configurable via CREDENTIAL_HEALTH_CHECK_INTERVAL (default 5 min) + * - Interval: configurable via CREDENTIAL_HEALTH_CHECK_INTERVAL (default 60 min) * - Per-connection override: provider_connections.healthCheckInterval (minutes, * 0 = never test this connection) paces each connection individually * - Backoff on failure: 5min -> 10min -> 30min -> max 2h @@ -24,6 +24,7 @@ import { getProviderConnections } from "@/lib/db/providers"; import { getCachedSettings } from "@/lib/db/readCache"; import { setCredentialHealth, initCredentialCache } from "@/lib/credentialHealth/cache"; import { + DEFAULT_SWEEP_INTERVAL_MS, isCredentialProbeInconclusive, resolveInconclusiveProbeRecheckDelayMs, } from "@/lib/credentialHealth/probePolicy"; @@ -129,7 +130,7 @@ export function resolveCredentialHealthSweepInterval( const parsed = parseInt(envVal, 10); if (!isNaN(parsed) && parsed >= 10_000) return parsed; } - return 3_600_000; // default 60 min + return DEFAULT_SWEEP_INTERVAL_MS; } /** @@ -142,7 +143,7 @@ function getSweepInterval(): number { const parsed = parseInt(envVal, 10); if (!isNaN(parsed) && parsed >= 10_000) return parsed; } - return 3_600_000; // default 60 min + return DEFAULT_SWEEP_INTERVAL_MS; } /** @@ -151,9 +152,9 @@ function getSweepInterval(): number { * - `healthCheckInterval <= 0` → null (never test this connection — opt-out) * - absent → global sweep cadence (operator resilience setting, else env, else default) */ -function getConnIntervalMs( +export function getConnIntervalMs( conn: { healthCheckInterval?: number | null }, - globalIntervalMs = 300_000 + globalIntervalMs = DEFAULT_SWEEP_INTERVAL_MS ): number | null { const minutes = conn.healthCheckInterval; if (minutes === null || minutes === undefined) return globalIntervalMs; diff --git a/tests/unit/credential-health-sweep-interval.test.ts b/tests/unit/credential-health-sweep-interval.test.ts index e79a31dedc7..c786f78e4bd 100644 --- a/tests/unit/credential-health-sweep-interval.test.ts +++ b/tests/unit/credential-health-sweep-interval.test.ts @@ -5,7 +5,10 @@ import { mergeResilienceSettings, resolveResilienceSettings, } from "../../src/lib/resilience/settings.ts"; -import { resolveCredentialHealthSweepInterval } from "../../src/lib/credentialHealth/scheduler.ts"; +import { + getConnIntervalMs, + resolveCredentialHealthSweepInterval, +} from "../../src/lib/credentialHealth/scheduler.ts"; const ORIGINAL_ENV = process.env.CREDENTIAL_HEALTH_CHECK_INTERVAL; @@ -90,3 +93,18 @@ test("sweep interval: non-numeric stored interval falls back to env/default", () assert.equal(resolveCredentialHealthSweepInterval(settings), 60 * 60_000); }); }); + +test("per-connection interval falls back to the 60-minute default, not 5", () => { + // #12138 raised the global default to 60 min but left this helper defaulting + // to 300_000, so any caller omitting the second argument probed connections + // five times more often than the operator's configured cadence. + assert.equal(getConnIntervalMs({}), 60 * 60_000); +}); + +test("per-connection interval 0 opts that connection out entirely", () => { + assert.equal(getConnIntervalMs({ healthCheckInterval: 0 }, 60 * 60_000), null); +}); + +test("per-connection interval overrides the global cadence", () => { + assert.equal(getConnIntervalMs({ healthCheckInterval: 5 }, 60 * 60_000), 5 * 60_000); +}); From 0fef517b26727b1e6af7656ac5c87f21985bbf5c Mon Sep 17 00:00:00 2001 From: Abhishek Divekar Date: Sun, 27 Sep 2026 11:06:53 +0530 Subject: [PATCH 2/2] chore(changelog): name the fragment after PR #14924 --- ...-5m-default.md => 14924-credential-health-stale-5m-default.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/fixes/{credential-health-stale-5m-default.md => 14924-credential-health-stale-5m-default.md} (100%) diff --git a/changelog.d/fixes/credential-health-stale-5m-default.md b/changelog.d/fixes/14924-credential-health-stale-5m-default.md similarity index 100% rename from changelog.d/fixes/credential-health-stale-5m-default.md rename to changelog.d/fixes/14924-credential-health-stale-5m-default.md