diff --git a/changelog.d/fixes/10799-provider-health-inconclusive-probes.md b/changelog.d/fixes/10799-provider-health-inconclusive-probes.md new file mode 100644 index 0000000000..72aacacee0 --- /dev/null +++ b/changelog.d/fixes/10799-provider-health-inconclusive-probes.md @@ -0,0 +1 @@ +- **fix(providers):** Keep NVIDIA timeout probes and generic Antigravity/AGY HTTP 400 probes from poisoning credential health while preserving explicit Google geo-block handling ([#10799](https://github.com/diegosouzapw/OmniRoute/pull/10799)) — thanks @Zartharas diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index 77d54f7845..900f6220f8 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -304,6 +304,8 @@ "_rebaseline_2026_07_27_3850_relax_filesize_cap": "OWNER-APPROVED TEMPORARY relax for v3.8.50-3.8.54 PREPARE phase (docs/ROADMAP.md). cap 800->900 (+100), testCap 800->900 (+100). Targets: decompose-existing-frozen unchanged (frozen still only-shrink); this only relaxes the cap for NEW files in the decompose/extract-while-PREPARE phase (.51='executor registry in-place' and .52='combo.ts decomposition' create new leaf modules above 800). RE-TIGHTENING MANDATORY in v3.8.51: cap target 850 = 850 once decomposition wave stabilizes. SUPERSEDED by _rebaseline_2026_07_27_3850_relax_filesize_cap_v2_20pct (v1 +20% buffer) — retained for audit. Tracked via same roadmap issue.", "_rebaseline_2026_07_27_v3849_train1h": "Merge-train 1H (31 PRs) — owner-approved 2026-07-27. Two distinct causes, kept separate on purpose: (1) GENUINE irreducible growth at existing chokepoints — providerLimits/auth (#8632 Kimi quota-reset recovery), rateLimitManager (#8616 idle wedged limiters), models-catalog-route.test (#8610 OpenCode Go effort aliases); (2) COLLISION with #8585, which banked shrinks measured on the pre-train release tip while 30 sibling PRs in the SAME train grew those files again — chat/accountFallback (#8628), chatCore (#8613), videoGeneration (#8581), imageGeneration. The zero-headroom frozen entries cannot absorb either. Ceilings re-pinned to the post-merge tip; #8612 (also in this train) automates shrink-banking so this self-inflicted drift stops recurring. Detail: src/lib/usage/providerLimits.ts 1006->1013 (#8632); src/sse/services/auth.ts 2492->2508 (#8632); open-sse/services/rateLimitManager.ts 1014->1060 (#8616); src/sse/handlers/chat.ts 1842->1845 (#8628); open-sse/handlers/chatCore.ts 4939->4955 (#8613); open-sse/handlers/imageGeneration.ts 3100->3101 ((sem PR — teto do #8585)); open-sse/handlers/videoGeneration.ts 1038->1063 (#8581); open-sse/services/accountFallback.ts 1965->1966 (#8628); tests/unit/models-catalog-route.test.ts 1608->1636 (#8610)", "frozen": { + "_rebaseline_2026_08_20_10878_10799_provider_health_probes": "PRs #10878 (unsupported OpenAI-like validation probes stay neutral) + #10799 (preserve credential health on inconclusive NVIDIA-timeout/Antigravity-400 probes) own growth: src/app/api/providers/[id]/test/route.ts 946->1025 (+79, sum of both boarded together). Both add narrowly-scoped classification branches at the existing test-route dispatch chokepoint (unsupported-capability skip, credential-inconclusive detection) rather than new files, mirroring the prior 2026_06_27_5193 rebaseline of the same file. Covered by tests/unit/provider-validation-unsupported-neutral.test.ts + tests/unit/provider-health-inconclusive-probes.test.ts.", + "src/app/api/providers/[id]/test/route.ts": 1025, "_rebaseline_2026_06_22_4644_deepseek_web_tools": "PR #4644 (BugsBag/robust deepseek-web tool-call parsing): open-sse/executors/deepseek-web.ts 1117->1125 (+8). The new agentic tool-call path emits surrounding text + reasoning before tool_calls and swaps to the dedicated deepseekWebTools.ts parser; the +8 lines are cohesive wiring at the existing transformSSE chokepoint (the parser itself lives in the new deepseekWebTools.ts file, already under cap). The PR's own fast-gate (PR->release) does not run check:file-size, so this surfaced only at release reconcile. Covered by tests/unit/deepseek-web-tools-variants.test.ts + deepseek-web-tools-execute.test.ts.", "_rebaseline_2026_06_23_4712_deepseek_web_tool_results": "PR for #4712 (deepseek-web drops role:tool): open-sse/executors/deepseek-web.ts 1125->1148 (+23). messagesToPrompt() now folds role:\"tool\" results into the single-prompt transcript (recovering the tool name from the preceding assistant tool_calls by tool_call_id) instead of silently dropping them; the lines are cohesive wiring inside the existing function. Covered by tests/unit/deepseek-web-tool-result-prompt-4712.test.ts.", "_rebaseline_2026_06_24_headroom_strategy": "Headroom-aware connection selection (dario technique): combo.ts 3168->3180 (+12 = a new `else if (strategy === \"headroom\")` dispatch branch in handleComboChat that delegates to orderTargetsByHeadroom + its log line, plus the import). The actual logic lives OUT of the god-file: the pure ranker rankByHeadroom/computeHeadroom is the new leaf open-sse/services/combo/headroomRanking.ts (91 LOC, ; body?: string; acceptStatuses?: number[]; + inconclusiveStatuses?: number[]; checkExpiry?: boolean; refreshable?: boolean; getUrl?: (connection: any) => string; @@ -88,6 +90,39 @@ export interface OAuthTestConfigEntry { ) => OAuthTestProbeRequest | Promise; } +export interface OAuthProbeInconclusiveClassification { + warning: string; + diagnosisType: "ok"; + diagnosisCode: "probe_inconclusive"; +} + +export function classifyOAuthProbeInconclusive( + config: OAuthTestConfigEntry, + provider: string, + status: number, + bodyText: string +): OAuthProbeInconclusiveClassification | null { + if ( + !Array.isArray(config.inconclusiveStatuses) || + !config.inconclusiveStatuses.includes(status) + ) { + return null; + } + + // Preserve the current upstream geo-block contract. Google's explicit + // location refusal is an egress/upstream availability failure, not a + // successful connection-test result. + if ((provider === "antigravity" || provider === "agy") && isGeoBlockedError(bodyText)) { + return null; + } + + return { + warning: `${provider} probe returned HTTP ${status}; credential validity is inconclusive`, + diagnosisType: "ok", + diagnosisCode: "probe_inconclusive", + }; +} + export const OAUTH_TEST_CONFIG: Record = { claude: { // Claude doesn't have userinfo, we verify token exists and not expired @@ -126,6 +161,7 @@ export const OAUTH_TEST_CONFIG: Record = { // Real model-surface probe (see buildAntigravityProbe above): userinfo-only // probing stayed green while the model API was geo-blocked. buildProbe: buildAntigravityProbe, + inconclusiveStatuses: [400], refreshable: true, }, // `agy` is a separate connection id that shares the Antigravity backend and the same @@ -135,6 +171,7 @@ export const OAUTH_TEST_CONFIG: Record = { // perfectly good account. Probe the same model surface as antigravity. agy: { buildProbe: buildAntigravityProbe, + inconclusiveStatuses: [400], refreshable: true, }, xai: XAI_CHAT_OAUTH_TEST_CONFIG, diff --git a/src/app/api/providers/[id]/test/route.ts b/src/app/api/providers/[id]/test/route.ts index ec1b235d5e..7ac785c7f5 100644 --- a/src/app/api/providers/[id]/test/route.ts +++ b/src/app/api/providers/[id]/test/route.ts @@ -32,7 +32,7 @@ import { removeConnectionHealth } from "@omniroute/open-sse/services/apiKeyRotat import { isConnectionUnavailableToAuxiliaryActivity } from "@/lib/exclusiveLeaseIsolation"; import { classifyAmbiguousOrAuthError, type ClassifyFailureArgs } from "./mistralAmbiguousAuth"; import { buildApiKeyConnectionTestResult } from "./apiKeyTestResult"; -import { OAUTH_TEST_CONFIG } from "./oauthTestConfig"; +import { classifyOAuthProbeInconclusive, OAUTH_TEST_CONFIG } from "./oauthTestConfig"; import { isGeoBlockedError } from "@omniroute/open-sse/services/errorClassifier.ts"; // Bound the OAuth probe so a hung upstream can't block the connection-test queue @@ -511,6 +511,38 @@ export async function testOAuthConnection( if (builtProbe?.body) fetchInit.body = builtProbe.body; const res = await fetch(url, fetchInit); + const inconclusiveBody = + Array.isArray(config.inconclusiveStatuses) && config.inconclusiveStatuses.includes(res.status) + ? await res + .clone() + .text() + .catch(() => "") + : ""; + + const inconclusive = classifyOAuthProbeInconclusive( + config, + connection.provider, + res.status, + inconclusiveBody + ); + + if (inconclusive) { + return { + valid: true, + error: null, + warning: inconclusive.warning, + refreshed, + newTokens, + statusCode: res.status, + diagnosis: makeDiagnosis( + inconclusive.diagnosisType, + "upstream", + inconclusive.warning, + inconclusive.diagnosisCode + ), + }; + } + // Port of decolua/9router#347: some providers (Codex) intentionally trigger a // 400 because the probe body is invalid. A 400 from such a provider means auth // succeeded; only 401/403 means the token is bad. @@ -573,6 +605,39 @@ export async function testOAuthConnection( else if (config.body) retryInit.body = config.body; const retryRes = await fetch(url, retryInit); + const retryInconclusiveBody = + Array.isArray(config.inconclusiveStatuses) && + config.inconclusiveStatuses.includes(retryRes.status) + ? await retryRes + .clone() + .text() + .catch(() => "") + : ""; + + const retryInconclusive = classifyOAuthProbeInconclusive( + config, + connection.provider, + retryRes.status, + retryInconclusiveBody + ); + + if (retryInconclusive) { + return { + valid: true, + error: null, + warning: retryInconclusive.warning, + refreshed: true, + newTokens: tokens, + statusCode: retryRes.status, + diagnosis: makeDiagnosis( + retryInconclusive.diagnosisType, + "upstream", + retryInconclusive.warning, + retryInconclusive.diagnosisCode + ), + }; + } + const retryAccepted = retryRes.ok || (Array.isArray(config.acceptStatuses) && config.acceptStatuses.includes(retryRes.status)); diff --git a/src/lib/credentialHealth/probePolicy.ts b/src/lib/credentialHealth/probePolicy.ts new file mode 100644 index 0000000000..6b4d387e36 --- /dev/null +++ b/src/lib/credentialHealth/probePolicy.ts @@ -0,0 +1,23 @@ +const DEFAULT_SWEEP_INTERVAL_MS = 300_000; +const INCONCLUSIVE_RECHECK_MIN_MS = 30 * 60_000; +const INCONCLUSIVE_RECHECK_MULTIPLIER = 6; +const INCONCLUSIVE_WARNING_MARKER = "credential validity is inconclusive"; + +export function isCredentialProbeInconclusive(result: { + valid?: boolean; + warning?: unknown; +}): boolean { + return ( + result.valid === true && + typeof result.warning === "string" && + result.warning.toLowerCase().includes(INCONCLUSIVE_WARNING_MARKER) + ); +} + +export function resolveInconclusiveProbeRecheckDelayMs(sweepIntervalMs: number): number { + const interval = + Number.isFinite(sweepIntervalMs) && sweepIntervalMs > 0 + ? sweepIntervalMs + : DEFAULT_SWEEP_INTERVAL_MS; + return Math.max(INCONCLUSIVE_RECHECK_MIN_MS, interval * INCONCLUSIVE_RECHECK_MULTIPLIER); +} diff --git a/src/lib/credentialHealth/scheduler.ts b/src/lib/credentialHealth/scheduler.ts index 8dfca2f595..a407f6a326 100644 --- a/src/lib/credentialHealth/scheduler.ts +++ b/src/lib/credentialHealth/scheduler.ts @@ -24,6 +24,10 @@ import { removeCredentialHealth, initCredentialCache, } from "@/lib/credentialHealth/cache"; +import { + isCredentialProbeInconclusive, + resolveInconclusiveProbeRecheckDelayMs, +} from "@/lib/credentialHealth/probePolicy"; import { emit } from "@/lib/events/eventBus"; import { isAutomatedTestProcess } from "@/shared/utils/testProcess"; import { SEARCH_VALIDATOR_CONFIGS } from "@/lib/providers/validation/searchProviders"; @@ -149,13 +153,25 @@ async function testConnection( const state = getSchedulerState(); if (result.valid) { - // Success — reset failure count, space the next test by the - // per-connection interval (absent → global sweep interval), update cache + // Success resets failure state. Credential-inconclusive probes remain + // active but are checked less often because repeating an expensive probe + // does not add authentication evidence; an ordinary success is paced by + // the per-connection interval (absent → global sweep interval). state.failureCounts.delete(connectionId); - state.perConnTiming.set(connectionId, { - lastAttemptAt: startTime, - nextAttemptAt: startTime + intervalMs, - }); + + if (isCredentialProbeInconclusive(result)) { + const recheckDelayMs = resolveInconclusiveProbeRecheckDelayMs(getSweepInterval()); + state.perConnTiming.set(connectionId, { + lastAttemptAt: startTime, + nextAttemptAt: Date.now() + recheckDelayMs, + }); + } else { + state.perConnTiming.set(connectionId, { + lastAttemptAt: startTime, + nextAttemptAt: startTime + intervalMs, + }); + } + setCredentialHealth( connectionId, provider, diff --git a/src/lib/providers/validation/specialtyInline.ts b/src/lib/providers/validation/specialtyInline.ts index 7729fa4554..fc9102f5cd 100644 --- a/src/lib/providers/validation/specialtyInline.ts +++ b/src/lib/providers/validation/specialtyInline.ts @@ -223,6 +223,19 @@ export async function validateLongcatProvider({ apiKey, providerSpecificData, is } } +export function normalizeNvidiaValidationFailure(error: unknown) { + const failure = toValidationErrorResult(error); + if (failure.timeout) { + return { + valid: true, + error: null, + warning: "NVIDIA auth probe timed out; credential validity is inconclusive", + method: "chat_probe_inconclusive", + }; + } + return failure; +} + // NVIDIA NIM (#2463) — bypass the /models probe in favor of a direct // chat/completions probe. NVIDIA NIM's /models endpoint returns model // catalogs that vary by region and key-tier, and some keys 404 on it, @@ -262,7 +275,7 @@ export async function validateNvidiaProvider({ apiKey, providerSpecificData }: a // Any non-auth response (200, 400, 422, 429) means auth passed return { valid: true, error: null }; } catch (error: any) { - return toValidationErrorResult(error); + return normalizeNvidiaValidationFailure(error); } } diff --git a/tests/unit/provider-health-inconclusive-probes.test.ts b/tests/unit/provider-health-inconclusive-probes.test.ts new file mode 100644 index 0000000000..f6d693a847 --- /dev/null +++ b/tests/unit/provider-health-inconclusive-probes.test.ts @@ -0,0 +1,107 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { SafeOutboundFetchError } from "../../src/shared/network/safeOutboundFetch.ts"; +import { normalizeNvidiaValidationFailure } from "../../src/lib/providers/validation/specialtyInline.ts"; +import { + classifyOAuthProbeInconclusive, + OAUTH_TEST_CONFIG, +} from "../../src/app/api/providers/[id]/test/oauthTestConfig.ts"; +import { + isCredentialProbeInconclusive, + resolveInconclusiveProbeRecheckDelayMs, +} from "../../src/lib/credentialHealth/probePolicy.ts"; + +test("NVIDIA timeout probe is credential-inconclusive instead of an auth failure", () => { + const error = new SafeOutboundFetchError( + "Request to https://integrate.api.nvidia.com/v1/chat/completions timed out after 20000ms", + { + code: "TIMEOUT", + url: "https://integrate.api.nvidia.com/v1/chat/completions", + method: "POST", + attempts: 1, + isRetryable: true, + timeoutMs: 20_000, + } + ); + + const result = normalizeNvidiaValidationFailure(error) as { + valid: boolean; + error: string | null; + method?: string; + warning?: string; + }; + + assert.equal(result.valid, true); + assert.equal(result.error, null); + assert.equal(result.method, "chat_probe_inconclusive"); + assert.match(String(result.warning), /credential validity is inconclusive/i); + assert.equal(isCredentialProbeInconclusive(result), true); +}); + +test("NVIDIA non-timeout network failures remain failures", () => { + const error = new SafeOutboundFetchError("fetch failed", { + code: "NETWORK_ERROR", + url: "https://integrate.api.nvidia.com/v1/chat/completions", + method: "POST", + attempts: 1, + isRetryable: true, + }); + + const result = normalizeNvidiaValidationFailure(error); + + assert.equal(result.valid, false); + assert.equal(result.error, "fetch failed"); + assert.equal(isCredentialProbeInconclusive(result), false); +}); + +test("inconclusive probes back off without changing ordinary successful probes", () => { + assert.equal( + isCredentialProbeInconclusive({ valid: true, warning: "ordinary provider warning" }), + false + ); + assert.equal( + isCredentialProbeInconclusive({ + valid: false, + warning: "credential validity is inconclusive", + }), + false + ); + assert.equal(resolveInconclusiveProbeRecheckDelayMs(300_000), 1_800_000); + assert.equal(resolveInconclusiveProbeRecheckDelayMs(600_000), 3_600_000); +}); + +test("Antigravity and AGY keep generic HTTP 400 inconclusive while preserving geo-block failures", () => { + for (const provider of ["antigravity", "agy"] as const) { + const config = OAUTH_TEST_CONFIG[provider]; + + assert.deepEqual(config?.inconclusiveStatuses, [400]); + assert.ok(!config?.acceptStatuses?.includes(400)); + + const generic = classifyOAuthProbeInconclusive(config, provider, 400, ""); + + assert.ok(generic); + assert.equal(generic.diagnosisCode, "probe_inconclusive"); + assert.match(generic.warning, /credential validity is inconclusive/i); + assert.equal( + isCredentialProbeInconclusive({ + valid: true, + warning: generic.warning, + }), + true + ); + + assert.equal( + classifyOAuthProbeInconclusive( + config, + provider, + 400, + '{"error":"User location is not supported for the API use."}' + ), + null, + "explicit Google geo-blocks must fall through to upstream availability handling" + ); + + assert.equal(classifyOAuthProbeInconclusive(config, provider, 401, ""), null); + assert.equal(classifyOAuthProbeInconclusive(config, provider, 403, ""), null); + } +});