Skip to content

fix(credential-health): stop the sweep defaulting to 5 minutes - #14924

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
adivekar-utexas:fix/credential-health-check-stale-5m-default
Sep 29, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
adivekar-utexas:fix/credential-health-check-stale-5m-default

Conversation

@adivekar-utexas

Copy link
Copy Markdown
Contributor

What

#12138 raised the credential health sweep default to 60 minutes, but left the 5-minute value behind in four places. The sweep can therefore still run at 5 minutes — about 12x the intended cadence — issuing a real credential probe against every active connection.

The leftovers

Location Was Now
getConnIntervalMs(conn, globalIntervalMs = 300_000) 5 min default param shares DEFAULT_SWEEP_INTERVAL_MS
open-sse/config/constants.ts CREDENTIAL_HEALTH_CHECK_INTERVAL 300_000 3_600_000
docs/reference/ENVIRONMENT.md default 300000 3600000
src/lib/copilot/systemPrompt.ts config table 300000 3600000
scheduler.ts header comment "default 5 min" "default 60 min"

The first is the only functional one, and it is latent rather than live: both current call sites pass globalIntervalMs explicitly. It is exactly the trap that resurfaces the moment someone adds a third call site.

Why a shared constant rather than a corrected number

DEFAULT_SWEEP_INTERVAL_MS now lives in the probePolicy leaf, which scheduler.ts already imports (so no cycle), and both the global resolver and the per-connection fallback read it. The two cannot drift again.

The change inside resolveInconclusiveProbeRecheckDelayMs is behaviour-neutral: its only caller passes getSweepInterval(), which is always finite and positive, so that fallback branch is unreachable today.

Tests

Three cases added to tests/unit/credential-health-sweep-interval.test.ts:

  • the per-connection fallback is 60 minutes, not 5
  • healthCheckInterval: 0 opts the connection out entirely
  • a per-connection interval still overrides the global cadence

18/18 pass across the credential-health suites. check:env-doc-sync, check:file-size, typecheck:core, check:open-sse-typecheck and ESLint are all clean.

For operators landing here from probe traffic

If you are seeing frequent tiny completions against a connection, the sweep is the cause. Three levers, in precedence order:

  1. Per-connection — Edit Connection → "Health Check (min)" → 0 (never test this connection)
  2. Global — Settings → Resilience → credential health check → 0 disables the sweep
  3. Env — CREDENTIAL_HEALTH_CHECK_INTERVAL (ms, minimum 10000)

A stored intervalMinutes or healthCheckInterval overrides the 60-minute default, so if you are on 5 minutes today that is almost certainly an explicit value left behind from testing #12043 / #12138 rather than the default.

diegosouzapw#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.
@diegosouzapw
diegosouzapw merged commit f8b3bc0 into diegosouzapw:release/v3.8.51 Sep 29, 2026
11 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants