diff --git a/docs-site/src/content/docs/ja/reference/configuration/providers.md b/docs-site/src/content/docs/ja/reference/configuration/providers.md index 7c3ab0288a..29e09c1740 100644 --- a/docs-site/src/content/docs/ja/reference/configuration/providers.md +++ b/docs-site/src/content/docs/ja/reference/configuration/providers.md @@ -21,7 +21,7 @@ description: プロバイダー エントリ、認証、エンドポイント、 | `autoSwitchThreshold?` | `number` | `80` | 使用量ベースのプロアクティブ切り替えしきい値。`quota` は紐付け済み/未紐付けタスクの次のリクエストを再評価でき、`fill-first` は未紐付け割り当ての使い切り基準としてのみ使用し、通常の `round-robin` 選択は使用しません。既知の 5 時間、週次、30 日 quota window の最大スコアを使います。`0` は使用量ベースの切り替えだけを無効にし、未紐付け割り当てや障害回復は無効にしません。 | | `accountPoolStrategy?` | `"quota" \| "round-robin" \| "fill-first"` | `"quota"` | 新規/未紐付け Codex リクエストの割り当て戦略。live な `(parent thread id, quota scope)` affinity がなければ未紐付けで、プロキシ再起動や affinity リセット後は既存の表示タスクも未紐付けになり得ます。`quota` はアクティブアカウントがなければ既知 usage 最小の適格アカウントを選び、適格なアクティブアカウントが `autoSwitchThreshold` 未満なら維持します。しきい値到達後は、未紐付けリクエストまたは紐付け済みタスクの次のリクエストを usage の低い適格アカウントへ移せます。`round-robin` は未紐付けリクエストを均等分散し、`fill-first` は cooldown、使用不可、または drain threshold までアクティブアカウントへ割り当てます。 | | `accountPoolStickyLimit?` | `number` | `1` | 1 回の round-robin 選択で次へ進む前に保持する新規/未紐付けタスク割り当て数。カウンターは上流の成功後ではなくタスクの紐付け時に増えます。範囲 1–100。`accountPoolStrategy` が `round-robin` のときのみ。 | -| `upstreamFailoverThreshold?` | `number` | `3` |今後の新しいセッションがフェイルオーバーする前に一時的なエラーが連続して発生する。 `0` を無効に設定します。 | +| `upstreamFailoverThreshold?` | `number` | `3` |今後の新しいセッションがフェイルオーバーする前に一時的なエラーが連続して発生する。 `0` を無効に設定します。実証済みの接続前DNS/TCP到達不能障害はprovider-host単位で記録され、アカウントの健全性、クールダウン、スレッド/セッションの親和性、アクティブアカウントの選択、Poolルーティングには影響せず、この閾値にもカウントされません。 | | `modelCacheTtlMs?` | `number` | `300000` |プロバイダーごとの `/models` キャッシュの鮮度ウィンドウ。 | | `cacheRetention?` | `"none" \| "short" \| "long"` | `"short"` | Anthropic プロンプト キャッシュ ポリシー: 無効、5 分間の一時的、または 1 時間の延長。 | | `tokenGuardian?` | `OcxTokenGuardianConfig` |オフ |オプションのプロアクティブな OAuth 更新および Codex アカウントのウォームアップ ポリシー。 | diff --git a/docs-site/src/content/docs/ko/reference/configuration/providers.md b/docs-site/src/content/docs/ko/reference/configuration/providers.md index a389a79fca..f321e7307c 100644 --- a/docs-site/src/content/docs/ko/reference/configuration/providers.md +++ b/docs-site/src/content/docs/ko/reference/configuration/providers.md @@ -21,7 +21,7 @@ description: 공급자 항목, 인증, 엔드포인트, 모델 카탈로그, 할 | `autoSwitchThreshold?` | `number` | `80` | 사용량 기반 선제 전환 임계값입니다. `quota`는 바인딩된 작업과 바인딩 없는 작업의 다음 요청을 모두 재평가할 수 있고, `fill-first`는 바인딩 없는 작업 배정의 소진 기준으로만 사용하며, 기본 `round-robin` 선택은 이 값을 사용하지 않습니다. 알려진 5시간, 주간, 30일 quota window 중 가장 높은 점수를 씁니다. `0`은 사용량 기반 전환만 끄며 바인딩 없는 작업 배정이나 실패 복구는 끄지 않습니다. | | `accountPoolStrategy?` | `"quota" \| "round-robin" \| "fill-first"` | `"quota"` | 새 작업/바인딩 없는 Codex 요청의 계정 배정 전략입니다. `(parent thread id, quota scope)`의 live affinity가 없으면 바인딩 없는 요청이며, 프록시 재시작이나 affinity 초기화 뒤에는 기존에 보이던 작업도 바인딩이 없어질 수 있습니다. `quota`는 활성 계정이 없을 때 알려진 usage가 가장 낮은 적격 계정을 선택하고, 적격 활성 계정이 `autoSwitchThreshold` 미만이면 유지합니다. 임계값 도달 뒤에는 바인딩 없는 요청이나 바인딩된 작업의 다음 요청을 usage가 더 낮은 적격 계정으로 옮길 수 있습니다. `round-robin`은 바인딩 없는 요청을 균등 분배하고, `fill-first`는 cooldown, 사용 불가 또는 drain threshold까지 활성 계정에 배정합니다. | | `accountPoolStickyLimit?` | `number` | `1` | 한 round-robin 선택이 다음으로 넘어가기 전에 유지하는 새 작업/바인딩 없는 작업 배정 수입니다. 카운터는 업스트림 성공 뒤가 아니라 작업을 바인딩할 때 증가합니다. 범위 1–100이며 `accountPoolStrategy`가 `round-robin`일 때만 적용됩니다. | -| `upstreamFailoverThreshold?` | `number` | `3` | 연속된 일시적 실패가 이 횟수에 도달하면 이후 새 세션은 failover됩니다. `0`으로 두면 비활성화됩니다. | +| `upstreamFailoverThreshold?` | `number` | `3` | 연속된 일시적 실패가 이 횟수에 도달하면 이후 새 세션은 failover됩니다. `0`으로 두면 비활성화됩니다. 입증된 연결 전 DNS/TCP 도달 불가 실패는 provider-host 범위로 기록되며 계정 상태, 쿨다운, 스레드/세션 선호도, 활성 계정 선택 또는 Pool 라우팅에 영향을 주지 않고 이 임계값에도 집계되지 않습니다. | | `modelCacheTtlMs?` | `number` | `300000` | 공급자별 `/models` 캐시의 최신성 창입니다. | | `cacheRetention?` | `"none" \| "short" \| "long"` | `"short"` | Anthropic 프롬프트 캐시 정책입니다. 비활성, 5분짜리 임시, 1시간짜리 확장 중 하나입니다. | | `tokenGuardian?` | `OcxTokenGuardianConfig` | 꺼짐 | 선택적 선제 OAuth 갱신과 Codex 계정 워밍업 정책입니다. | diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index a9603702bf..4c75452549 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -22,7 +22,7 @@ authenticated. | `autoSwitchThreshold?` | `number` | `80` | Usage threshold for proactive switching. `quota` can re-evaluate both bound and unbound tasks on their next request; `fill-first` uses it only as the drain point for unbound assignment; normal `round-robin` selection does not use it. The score uses the hottest known 5h, weekly, or 30d quota window. `0` disables usage-based proactive switching only, not unbound assignment or failure recovery. | | `accountPoolStrategy?` | `"quota" \| "round-robin" \| "fill-first"` | `"quota"` | Assignment strategy for new/unbound Codex requests. A request is unbound when it has no live (parent thread id, quota scope) affinity; a visible existing task can become unbound after proxy restart or affinity reset. `quota` picks the lowest-usage eligible account when no active account exists, keeps an eligible active account below `autoSwitchThreshold`, and after the threshold may move an unbound request or proactively rebind a bound task to a lower-usage eligible account. `round-robin` distributes unbound requests evenly; `fill-first` keeps assigning unbound requests to the active account until cooldown, unavailability, or the configured drain threshold. | | `accountPoolStickyLimit?` | `number` | `1` | New/unbound task assignments retained on one round-robin selection before advancing; the counter advances when a task is bound, not after an upstream success. Range 1–100. | -| `upstreamFailoverThreshold?` | `number` | `3` | Consecutive transient failures before future new sessions fail over. Set `0` to disable. | +| `upstreamFailoverThreshold?` | `number` | `3` | Consecutive transient failures before future new sessions fail over. Set `0` to disable. Proven pre-connection DNS/TCP reachability failures are tracked at the provider-host level: they never affect account health, cooldowns, thread/session affinity, active-account selection, or Pool routing, and never count toward this threshold. | | `modelCacheTtlMs?` | `number` | `300000` | Freshness window for the per-provider `/models` cache. | | `cacheRetention?` | `"none" \| "short" \| "long"` | `"short"` | Anthropic prompt-cache policy: disabled, 5-minute ephemeral, or 1-hour extended. | | `tokenGuardian?` | `OcxTokenGuardianConfig` | off | Optional proactive OAuth refresh and Codex-account warmup policy. | diff --git a/docs-site/src/content/docs/ru/reference/configuration/providers.md b/docs-site/src/content/docs/ru/reference/configuration/providers.md index d2bb183b17..6f11149e59 100644 --- a/docs-site/src/content/docs/ru/reference/configuration/providers.md +++ b/docs-site/src/content/docs/ru/reference/configuration/providers.md @@ -22,7 +22,7 @@ description: Записи провайдеров, аутентификация, | `autoSwitchThreshold?` | `number` | `80` | Порог проактивного переключения по использованию. `quota` может повторно оценить следующий запрос как привязанной, так и непривязанной задачи; `fill-first` использует его только как точку исчерпания для непривязанных назначений; обычный `round-robin` его не использует. Оценка берёт самое горячее из окон 5 часов, недели и 30 дней. `0` отключает только переключение по использованию, но не назначение непривязанных задач и не восстановление после сбоев. | | `accountPoolStrategy?` | `"quota" \| "round-robin" \| "fill-first"` | `"quota"` | Стратегия назначения для новых/непривязанных запросов Codex. Запрос непривязан, если у него нет live affinity `(parent thread id, quota scope)`; видимая существующая задача может стать непривязанной после перезапуска прокси или сброса affinity. `quota` выбирает подходящий аккаунт с наименьшим известным usage, когда активного аккаунта нет, сохраняет подходящий активный аккаунт ниже `autoSwitchThreshold`, а после порога может перевести непривязанный запрос или следующий запрос привязанной задачи на подходящий аккаунт с меньшим usage. `round-robin` равномерно распределяет непривязанные запросы; `fill-first` назначает их активному аккаунту до cooldown, недоступности или порога исчерпания. | | `accountPoolStickyLimit?` | `number` | `1` | Число назначений новых/непривязанных задач на одном выборе round-robin перед переходом дальше. Счётчик растёт при привязке задачи, а не после успеха upstream. Диапазон 1–100; только при `accountPoolStrategy` = `round-robin`. | -| `upstreamFailoverThreshold?` | `number` | `3` | Сколько подряд transient failure допустить, прежде чем новые сессии начнут делать failover. `0` отключает эту логику. | +| `upstreamFailoverThreshold?` | `number` | `3` | Сколько подряд transient failure допустить, прежде чем новые сессии начнут делать failover. `0` отключает эту логику. Доказанные ошибки доступности DNS/TCP до соединения учитываются на уровне пары «провайдер, хост» и не влияют на здоровье аккаунта, кулдауны, привязку потока/сессии, выбор активного аккаунта или маршрутизацию пула, а также не учитываются в этом пороге. | | `modelCacheTtlMs?` | `number` | `300000` | Окно свежести для кэша `/models` на уровне провайдера. | | `cacheRetention?` | `"none" \| "short" \| "long"` | `"short"` | Политика prompt-cache Anthropic: отключено, 5-минутный ephemeral или 1-часовой extended. | | `tokenGuardian?` | `OcxTokenGuardianConfig` | off | Необязательная политика proactive OAuth refresh и warmup'а аккаунтов Codex. | diff --git a/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md b/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md index b400ffbd43..36dd4e2883 100644 --- a/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md +++ b/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md @@ -21,7 +21,7 @@ description: 提供者条目、身份验证、端点、模型目录、配额、 | `autoSwitchThreshold?` | `number` | `80` | 基于用量的主动切换阈值。`quota` 可在下一次请求中重新评估已绑定和未绑定任务;`fill-first` 仅把它用作未绑定分配的耗尽点;正常 `round-robin` 不使用它。分数取已知 5 小时、周或 30 天 quota window 的最高值。`0` 只关闭基于用量的主动切换,不关闭未绑定任务分配或故障恢复。 | | `accountPoolStrategy?` | `"quota" \| "round-robin" \| "fill-first"` | `"quota"` | 新建/未绑定 Codex 请求的分配策略。没有 live `(parent thread id, quota scope)` affinity 的请求属于未绑定;代理重启或 affinity 重置后,已有可见任务也可能未绑定。`quota` 在没有活跃账号时选择已知 usage 最低的合格账号;活跃账号合格且低于 `autoSwitchThreshold` 时继续使用;达到阈值后,可把未绑定请求或已绑定任务的下一次请求切换到 usage 更低的合格账号。`round-robin` 均匀分配未绑定请求;`fill-first` 在 cooldown、不可用或耗尽阈值前持续分配给活跃账号。 | | `accountPoolStickyLimit?` | `number` | `1` | 一次 round-robin 选择在推进前保留的新建/未绑定任务分配数。计数在任务绑定时增加,而不是在上游成功后增加。范围 1–100;仅当 `accountPoolStrategy` 为 `round-robin` 时生效。 | -| `upstreamFailoverThreshold?` | `number` | `3` | 连续发生多少次瞬态故障后,后续新会话会切换到备用上游。设为 `0` 可禁用。 | +| `upstreamFailoverThreshold?` | `number` | `3` | 连续发生多少次瞬态故障后,后续新会话会切换到备用上游。设为 `0` 可禁用。已证明的连接前 DNS/TCP 不可达故障按 provider-host 粒度记录,不影响账户健康、冷却、线程/会话亲和性、活动账户选择或 Pool 路由,也不会计入此阈值;未确认的失败仍归属账户。 | | `modelCacheTtlMs?` | `number` | `300000` | 每个提供者 `/models` 缓存的新鲜度窗口。 | | `cacheRetention?` | `"none" \| "short" \| "long"` | `"short"` | Anthropic 提示缓存策略:禁用、5 分钟临时缓存,或 1 小时扩展缓存。 | | `tokenGuardian?` | `OcxTokenGuardianConfig` | 关闭 | 可选的主动 OAuth 刷新与 Codex 账户预热策略。 | diff --git a/src/codex/routing.ts b/src/codex/routing.ts index b85e25e4ce..8510957abf 100644 --- a/src/codex/routing.ts +++ b/src/codex/routing.ts @@ -22,6 +22,7 @@ import type { OcxConfig } from "../types"; import { captureConfigGeneration, type GenerationContext } from "../lib/state-store-sweeper"; import { isCanonicalOpenAiForwardProvider } from "../providers/openai-tiers"; import { retainedUtf8Bytes } from "../lib/admission"; +import { recordUpstreamHostFailure } from "./upstream-host-health"; type ThreadAffinityEntry = { accountId: string; @@ -122,8 +123,8 @@ const quotaScopedHealth = new Map(); -export type CodexUpstreamOutcome = number | "connect_error" | "timeout"; -export type CodexUpstreamOutcomeClass = "success" | "credential" | "quota" | "transient" | "caller" | "unknown"; +export type CodexUpstreamOutcome = number | "connect_error" | "timeout" | "connect_neutral"; +export type CodexUpstreamOutcomeClass = "success" | "credential" | "quota" | "transient" | "caller" | "neutral" | "unknown"; export type CodexCooldownSource = "retry-after" | "reset-derived" | "default"; /** * Native Codex quota groups known to be independent upstream. Keep the mapping @@ -187,6 +188,10 @@ export type CodexUpstreamOutcomeMeta = { retryAfter?: string | null; resetAt?: unknown | unknown[]; now?: number; + /** (provider, host) ledger key for account-neutral reachability failures (#914). */ + hostKey?: string; + /** Stable transport code recorded alongside a neutral host failure. */ + lastFailureCode?: string; /** Native model selected for this request; used only for confirmed scoped quotas. */ modelId?: string; /** When set, clears affinity for this thread immediately on transient failure. */ @@ -321,9 +326,15 @@ export function computeCodexUsageScore(quota: { } export function classifyCodexUpstreamOutcome(outcome: CodexUpstreamOutcome): CodexUpstreamOutcomeClass { + if (outcome === "connect_neutral") return "neutral"; if (outcome === "connect_error" || outcome === "timeout") return "transient"; if (!Number.isFinite(outcome)) return "unknown"; if (outcome >= 200 && outcome < 300) return "success"; + // Explicit 3xx policy (#914): a redirect response is relayed as-is and is + // never account or host health evidence — it proves the host is reachable + // and says nothing about the credential. Relayed as the neutral class so a + // stray 3xx cannot increment an account's transient streak. + if (outcome >= 300 && outcome < 400) return "neutral"; if (outcome === 401 || outcome === 403) return "credential"; // 402 Payment Required is treated as quota exhaustion for pool cooldown/failover // (same-request alternate retry records this outcome for the depleted account). @@ -1442,6 +1453,13 @@ export function recordCodexUpstreamOutcome( outcome: CodexUpstreamOutcome, meta: CodexUpstreamOutcomeMeta = {}, ): void { + // Host-level evidence is account-independent (#914): a pre-connection + // reachability failure is recorded in the (provider, host) ledger even when + // there is no account to attribute, or the account's writer generation is + // stale — the early returns below must not gate it. + if (outcome === "connect_neutral" && meta.hostKey) { + recordUpstreamHostFailure(meta.hostKey, { code: meta.lastFailureCode, now: meta.now ?? Date.now() }); + } if (!accountId) return; const writerGeneration = meta.writerGeneration ?? captureConfigGeneration(); if (writerGeneration < lastReconciledGeneration && !liveHealthAccountIds.has(accountId)) return; @@ -1506,6 +1524,25 @@ export function recordCodexUpstreamOutcome( return; } + if (outcomeClass === "neutral") { + // A proven pre-connection reachability failure (DNS / TCP refusal) or a + // relayed 3xx is host-level, not account evidence: rotation cannot repair + // it and must not happen (#914). Conclude any owned probe lease, record the + // failure under the (provider, host) ledger when one is named, and leave + // account health, thread affinity, and the active account untouched. + const current = upstreamHealth.get(accountId); + const scopedProbe = meta.probeQuotaScope + ? scopedHealthFor(accountId, meta.probeQuotaScope) + : undefined; + if (scopedProbe && meta.probeQuotaScope && ownsProbeLease(scopedProbe, meta)) { + setScopedHealth(accountId, meta.probeQuotaScope, withProbeLeaseReleased(scopedProbe, now)); + } + if (ownsProbeLease(current, meta)) { + upstreamHealth.set(accountId, withProbeLeaseReleased(current!, now)); + } + return; + } + const lastFailureStatus = typeof outcome === "number" ? outcome : 0; if (outcomeClass === "credential") { // 401/403 quarantines the account for reauth. That supersedes quota state diff --git a/src/codex/upstream-host-health.ts b/src/codex/upstream-host-health.ts new file mode 100644 index 0000000000..0ca863b0b6 --- /dev/null +++ b/src/codex/upstream-host-health.ts @@ -0,0 +1,70 @@ +/** + * Observational ledger for pre-connection upstream reachability failures, + * keyed by (provider, host). Records ONLY — no circuit breaker, no admission + * change (issue #914 scope). Rotation decisions stay with account health; + * this ledger exists so a host-wide outage is visible as host-wide. + * + * Retention: bounded at 128 entries; on overflow the stalest entries by + * last-touch are pruned before insert, and failure timestamps older than the + * window are reconciled away on the next record — repeated provider/base-URL + * churn cannot grow the map for the process lifetime. + */ + +export const UPSTREAM_HOST_HEALTH_MAX_ENTRIES = 128; +export const UPSTREAM_HOST_FAILURE_WINDOW_MS = 10 * 60_000; + +export type UpstreamHostHealthEntry = { + consecutiveFailures: number; + lastFailureAt: number; + lastFailureCode?: string; + /** Recency marker for stalest-first pruning (not health semantics). */ + lastTouch: number; +}; + +const hostHealth = new Map(); + +export function upstreamHostHealthKey(provider: string, host: string): string { + return `${provider}|${host.toLowerCase()}`; +} + +function pruneForInsert(): void { + if (hostHealth.size < UPSTREAM_HOST_HEALTH_MAX_ENTRIES) return; + const entries = [...hostHealth.entries()].sort((a, b) => a[1].lastTouch - b[1].lastTouch); + for (const [key] of entries) { + if (hostHealth.size < UPSTREAM_HOST_HEALTH_MAX_ENTRIES) return; + hostHealth.delete(key); + } +} + +export function recordUpstreamHostFailure( + key: string, + opts: { code?: string; now?: number } = {}, +): void { + const now = opts.now ?? Date.now(); + const prior = hostHealth.get(key); + // Prune only for a genuinely new key: updating an existing entry must never + // evict an unrelated one. + if (prior === undefined) pruneForInsert(); + const stale = prior !== undefined && now - prior.lastFailureAt > UPSTREAM_HOST_FAILURE_WINDOW_MS; + const code = typeof opts.code === "string" && opts.code !== "" ? opts.code : prior?.lastFailureCode; + hostHealth.set(key, { + consecutiveFailures: stale || prior === undefined ? 1 : prior.consecutiveFailures + 1, + lastFailureAt: now, + lastTouch: now, + ...(code !== undefined ? { lastFailureCode: code } : {}), + }); +} + +/** Any real HTTP response from the host clears its reachability streak. */ +export function resetUpstreamHostHealth(key: string): void { + hostHealth.delete(key); +} + +export function getUpstreamHostHealth(key: string): UpstreamHostHealthEntry | null { + return hostHealth.get(key) ?? null; +} + +/** Test hook: clear the whole ledger. */ +export function clearUpstreamHostHealth(): void { + hostHealth.clear(); +} diff --git a/src/lib/upstream-reachability.ts b/src/lib/upstream-reachability.ts new file mode 100644 index 0000000000..5147f1f695 --- /dev/null +++ b/src/lib/upstream-reachability.ts @@ -0,0 +1,91 @@ +/** + * Classification of upstream fetch rejections that occur before any request + * bytes can reach the origin: DNS resolution failure and TCP connect refusal. + * + * Issue #914: these failures are machine/network-wide, not account-specific — + * every Codex pool account shares the provider host, so rotating accounts + * cannot repair them. The transport layer previously mapped every non-timeout + * rejection to `connect_error`, and at `upstreamFailoverThreshold` that streak + * soft-avoided a healthy account and cleared thread affinity. + * + * Bun collapses DNS failure and TCP refusal into one label class + * (`ConnectionRefused` / `FailedToOpenSocket`, errno 0); Node's undici emits + * the classic `ECONNREFUSED` / `ENOTFOUND` / `EAI_AGAIN` / `ENETUNREACH` / + * `ENETDOWN` / `EHOSTUNREACH` shapes. Both shapes are matched here, on `code` + * values only, through a bounded cause chain. Message substrings are never + * trusted (a message-only match is a negative case), and labels that can also + * appear after the origin saw the credential — `ECONNRESET`, `EPIPE`, TLS + * errors, unknown shapes — deliberately stay outside the set. + * + * Classifier semantics extracted from PR #966 (Yuxin-Qiao) with attribution; + * sidecar blast radius intentionally not inherited. + * + * MUST stay a leaf module: imports nothing from server.ts or adapters. + */ + +import { UpstreamRetryEvidenceError } from "./upstream-retry"; + +export const PRE_CONNECT_REACHABILITY_CODES = new Set([ + // Bun: DNS failure and TCP refusal share this class. + "ConnectionRefused", + "FailedToOpenSocket", + // Node undici / classic Node shapes. + "ECONNREFUSED", + "ENOTFOUND", + "EAI_AGAIN", + "ENETUNREACH", + "ENETDOWN", + "EHOSTUNREACH", +]); + +/** Upper bound on how far `cause` chains are inspected. */ +export const MAX_REACHABILITY_CAUSE_DEPTH = 3; + +/** + * True when the rejection (or a bounded `cause` of it) carries a proven + * pre-connection reachability code. Never matches message text. + */ +export function isPreConnectReachabilityError(err: unknown): boolean { + let current: unknown = err; + const seen = new Set(); + for (let depth = 0; depth < MAX_REACHABILITY_CAUSE_DEPTH; depth++) { + if (!(current instanceof Error) || seen.has(current)) return false; + seen.add(current); + const code = (current as { code?: unknown }).code; + if (typeof code === "string" && PRE_CONNECT_REACHABILITY_CODES.has(code)) return true; + current = current.cause; + } + return false; +} + +export type TransportFailureKind = "timeout" | "connect_neutral" | "connect_error"; + +/** + * Shared transport rejection classification for Codex pool upstream sends. + * Timeouts keep their existing identity (account-transient); proven pre-connect + * reachability failures become account-neutral; everything else (ECONNRESET, + * EPIPE, TLS, unknown shapes) keeps the existing `connect_error` + * account-attributed behavior. + */ +export function classifyTransportFailureKind(err: unknown): TransportFailureKind { + const evidence = err instanceof UpstreamRetryEvidenceError ? err : undefined; + const rejection = evidence ? evidence.cause : err; + if (rejection instanceof Error && rejection.name === "TimeoutError") return "timeout"; + if (isPreConnectReachabilityError(rejection)) { + // A transient upstream response (5xx) or a credential-visible connection + // reset before the rejection proves the host and credential path were + // reached: the failure is account-attributable, never the pre-connection + // neutral class (issue #914 review). + if (evidence && (evidence.transientStatuses.length > 0 || evidence.resetSeen)) return "connect_error"; + return "connect_neutral"; + } + return "connect_error"; +} + +/** Stable `code` carried by a transport rejection, when there is one. */ +export function transportErrorCode(err: unknown): string | undefined { + const rejection = err instanceof UpstreamRetryEvidenceError ? err.cause : err; + if (!(rejection instanceof Error)) return undefined; + const code = (rejection as { code?: unknown }).code; + return typeof code === "string" && code !== "" ? code : undefined; +} diff --git a/src/lib/upstream-retry.ts b/src/lib/upstream-retry.ts index 82023549c9..2ba8adb370 100644 --- a/src/lib/upstream-retry.ts +++ b/src/lib/upstream-retry.ts @@ -249,6 +249,39 @@ export interface TransientRetryOptions extends ResetRetryOptions { export type UpstreamSendRecovery = "connection-reset" | "transient-5xx"; type ReplayableFetch = (recovery?: UpstreamSendRecovery) => Promise; +/** + * Rejection thrown by the upstream retry helpers when the terminal attempt + * rejects after earlier attempts already produced credential-visible evidence: + * transient 5xx responses, or a connection reset after the request was read. + * + * That evidence proves the host and credential path were reached, so the + * failure must stay account-attributed even though the terminal promise looks + * like a transport rejection (issue #914 review: mixed 5xx/reset -> rejection + * must not be downgraded to the account-neutral pre-connection class). The + * original rejection is preserved as `cause` so its code and message stay + * inspectable. Extracted from PR #966 (Yuxin-Qiao) with attribution. + */ +export class UpstreamRetryEvidenceError extends Error { + constructor( + public readonly transientStatuses: readonly number[], + cause: unknown, + /** True when a connection-reset retry already reached the origin. */ + public readonly resetSeen = false, + ) { + const detail = cause instanceof Error ? cause.message : String(cause); + const kinds: string[] = []; + if (transientStatuses.length > 0) kinds.push("transient 5xx response(s)"); + if (resetSeen) kinds.push("a credential-visible connection reset"); + super( + kinds.length > 0 + ? `upstream fetch failed after ${kinds.join(" and ")}: ${detail}` + : `upstream fetch failed: ${detail}`, + { cause }, + ); + this.name = "UpstreamRetryEvidenceError"; + } +} + /** * Opt out of Bun's keep-alive pool after a connection-reset retry. * @@ -282,12 +315,22 @@ export async function fetchWithResetRetry( ): Promise { const attempts = Math.max(1, opts.attempts ?? RESET_RETRY_MAX_ATTEMPTS); let lastError: unknown; + let sawReset = false; for (let attempt = 0; attempt < attempts; attempt++) { if (opts.abortSignal?.aborted) throw abortError(opts.abortSignal); try { return await doFetch(attempt === 0 ? firstRecovery : "connection-reset"); } catch (err) { - if (opts.abortSignal?.aborted || !isConnectionResetError(err) || attempt === attempts - 1) throw err; + if (opts.abortSignal?.aborted) throw err; + if (!isConnectionResetError(err)) { + // A reset that already reached the origin is credential-visible + // evidence: keep it attached so the terminal rejection cannot be + // downgraded to the pre-connection neutral class (#914 review). + if (sawReset) throw new UpstreamRetryEvidenceError([], err, true); + throw err; + } + if (attempt === attempts - 1) throw err; + sawReset = true; lastError = err; console.warn( `[upstream-retry] connection reset${opts.label ? ` (${opts.label})` : ""} — retrying (${attempt + 2}/${attempts})`, @@ -317,6 +360,7 @@ export async function fetchWithTransientRetry( ): Promise { const attempts = Math.max(1, opts.attempts ?? TRANSIENT_RETRY_MAX_ATTEMPTS); const slowAttemptMs = opts.slowAttemptMs ?? TRANSIENT_RETRY_SLOW_ATTEMPT_MS; + const transientStatuses: number[] = []; let attemptStart = Date.now(); let res = await fetchWithResetRetry(doFetch, opts); for (let attempt = 0; attempt < attempts - 1; attempt++) { @@ -334,7 +378,14 @@ export async function fetchWithTransientRetry( cancelResponseBodyBestEffort(res); await sleepWithAbort(delay, opts.abortSignal); attemptStart = Date.now(); - res = await fetchWithResetRetry(doFetch, opts, "transient-5xx"); + transientStatuses.push(res.status); + try { + res = await fetchWithResetRetry(doFetch, opts, "transient-5xx"); + } catch (err) { + // Keep the prior 5xx evidence attached: the origin already responded, so + // this rejection is not pre-connection and must not classify as neutral. + throw new UpstreamRetryEvidenceError(transientStatuses, err); + } } return res; } diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index 74ba6171b9..883f076fb9 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -68,6 +68,8 @@ import { applyUpstreamRecoveryInit, type UpstreamSendRecovery, } from "../../lib/upstream-retry"; +import { classifyTransportFailureKind, transportErrorCode } from "../../lib/upstream-reachability"; +import { recordUpstreamHostFailure, resetUpstreamHostHealth, upstreamHostHealthKey } from "../../codex/upstream-host-health"; import { ForwardAdmissionCredentialError, validateForwardAdmissionCredential } from "../auth-cors"; import { listOpenAiForwardSidecarCandidates, resolveFirstUsableOpenAiSidecar, type ResolvedOpenAiForwardSidecar } from "../../providers/openai-sidecar"; import { isCanonicalOpenAiForwardProvider, supportsNativeResponsesCompactEndpoint } from "../../providers/openai-tiers"; @@ -110,7 +112,7 @@ import { hasResponsesItemIdRepair, relaySseWithResponsesItemIdRepair } from "../ import type { EffectiveSubagentRoster, SpawnAgentSurface } from "../../codex/catalog"; import { decodeRequestErrorResponse, handleResponses, usesCodexForwardPoolAuth } from "./core"; -import { fetchWithHeaderTimeout, providerFetch, safeHostLabel } from "./fetch-helpers"; +import { fetchWithHeaderTimeout, providerFetch, safeHostLabel, safeOriginLabel } from "./fetch-helpers"; export const COMPACT_RESPONSE_MAX_BYTES = 32 * 1024 * 1024; @@ -191,6 +193,8 @@ const COMPACT_PASSTHROUGH_HEADERS = [ "x-codex-primary-reset-at", "x-codex-secondary-reset-at", "x-codex-tertiary-reset-at", + // A relayed 3xx keeps its Location so the client can follow it (#914). + "location", ]; function compactResponseHeaders(upstream: Response): Headers { @@ -398,7 +402,15 @@ export async function handleResponsesCompact( connectMs, false, providerFetch(sendProvider), - ); + // Every credential-bearing forward send gets manual redirects, not only + // pool sends: direct mode carries the caller's credential too (#914). + sendProvider.authMode === "forward", + ).then(res => { + // Every real attempt response — including an intermediate 5xx the retry + // wrapper replaces — proves the host was reached (#914 review). + resetUpstreamHostHealth(upstreamHostHealthKey(route.providerName, safeOriginLabel(compactUrl))); + return res; + }); return recovery === "single" ? doFetch() : fetchWithTransientRetry(doFetch, { abortSignal: req.signal, label: safeHostLabel(compactUrl) }); @@ -417,7 +429,14 @@ export async function handleResponsesCompact( recordCompactPoolOutcome(outcomeCtx, 499); return formatErrorResponse(499, "client_cancelled", "Client cancelled compact request"); } - const outcome = err instanceof Error && err.name === "TimeoutError" ? "timeout" : "connect_error"; + const outcome = classifyTransportFailureKind(err); + // Host-level evidence stands regardless of pool membership (#914 review). + if (outcome === "connect_neutral") { + recordUpstreamHostFailure( + upstreamHostHealthKey(route.providerName, safeOriginLabel(compactUrl)), + { code: transportErrorCode(err) }, + ); + } recordCompactPoolOutcome(outcomeCtx, outcome); return formatErrorResponse(502, "upstream_error", "Failed to connect to compact upstream"); } @@ -485,7 +504,14 @@ export async function handleResponsesCompact( recordCompactPoolOutcome(outcomeCtx, 499); return formatErrorResponse(499, "client_cancelled", "Client cancelled compact request"); } - const outcome = err instanceof Error && err.name === "TimeoutError" ? "timeout" : "connect_error"; + const outcome = classifyTransportFailureKind(err); + // Host-level evidence stands regardless of pool membership (#914 review). + if (outcome === "connect_neutral") { + recordUpstreamHostFailure( + upstreamHostHealthKey(route.providerName, safeOriginLabel(compactUrl)), + { code: transportErrorCode(err) }, + ); + } recordCompactPoolOutcome(outcomeCtx, outcome); return formatErrorResponse(502, "upstream_error", "Failed to connect to compact upstream"); } diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 08cc244b4d..dd0f24e606 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -174,7 +174,9 @@ import type { EffectiveSubagentRoster, SpawnAgentSurface } from "../../codex/cat import { buildToolBridgeMaps, collabSurface, injectDeveloperMessage, multiAgentGuidanceText } from "./collaboration"; import { hasUnreadableEncryptedAgentTask, looksLikeBackendCiphertext, sanitizeEncryptedContentInPlace } from "./encrypted-payload"; -import { fetchWithHeaderTimeout, providerFetch, safeHostLabel } from "./fetch-helpers"; +import { fetchWithHeaderTimeout, providerFetch, safeHostLabel, safeOriginLabel } from "./fetch-helpers"; +import { classifyTransportFailureKind, transportErrorCode } from "../../lib/upstream-reachability"; +import { recordUpstreamHostFailure, resetUpstreamHostHealth, upstreamHostHealthKey } from "../../codex/upstream-host-health"; import { guardTerminalEventStream } from "./terminal-guard"; /** @@ -436,7 +438,12 @@ async function retryCodexPoolOnAlternateAccount( connectMs, stream, providerFetch(route.provider), + // Credential-bearing forward send: never follow a redirect into a + // dead-host rejection after the credential was seen (#914). + route.provider.authMode === "forward", ); + // A real HTTP response proves the host was reached (#914). + resetUpstreamHostHealth(upstreamHostHealthKey(route.providerName, safeOriginLabel(request.url))); return { kind: "retried", authCtx: retryAuthCtx, @@ -1742,7 +1749,16 @@ async function handleResponsesInner( const transportFailureResponse = (err: unknown): Response => { upstream.abort(); if (options.abortSignal?.aborted) return clientCancelledResponse(); - const outcome = err instanceof Error && err.name === "TimeoutError" ? "timeout" : "connect_error"; + const outcome = classifyTransportFailureKind(err); + // Host-level evidence stands regardless of pool membership: a direct + // forward send has no pool accounting, but the reachability failure is + // still host-wide, not account evidence (#914 review). + if (outcome === "connect_neutral") { + recordUpstreamHostFailure( + upstreamHostHealthKey(route.providerName, safeOriginLabel(request.url)), + { code: transportErrorCode(err) }, + ); + } if (usesCodexForwardPoolAuth(authCtx, route.provider)) { recordCodexUpstreamOutcome(config, authCtx.accountId, outcome, { threadId: req.headers.get("x-codex-parent-thread-id"), @@ -1769,7 +1785,14 @@ async function handleResponsesInner( method: request.method, headers: request.headers, body: request.body, - }, recovery), upstream.signal, connectMs, parsed.stream, providerFetch(route.provider)); + }, recovery), upstream.signal, connectMs, parsed.stream, providerFetch(route.provider), + route.provider.authMode === "forward") + // Every real attempt response — including an intermediate 5xx the + // retry wrapper replaces — proves the host was reached (#914 review). + .then(res => { + resetUpstreamHostHealth(upstreamHostHealthKey(route.providerName, safeOriginLabel(request.url))); + return res; + }); }, { abortSignal: upstream.signal, label: safeHostLabel(request.url) }, ); @@ -1823,7 +1846,12 @@ async function handleResponsesInner( method: request.method, headers: request.headers, body: request.body, - }, recovery), upstream.signal, connectMs, parsed.stream, providerFetch(route.provider)); + }, recovery), upstream.signal, connectMs, parsed.stream, providerFetch(route.provider), + route.provider.authMode === "forward") + .then(res => { + resetUpstreamHostHealth(upstreamHostHealthKey(route.providerName, safeOriginLabel(request.url))); + return res; + }); }, { abortSignal: upstream.signal, label: safeHostLabel(request.url) }, ); @@ -1948,6 +1976,17 @@ async function handleResponsesInner( // Codex renders that as the opaque "Unknown error" (#452). Combo attempts // keep their typed failure envelope. Non-empty bodies are relayed verbatim // (headers included) so pool-retry Activation B/D and client diagnostics stay intact. + // Manual-redirect policy (#914): a 3xx is relayed as-is (Location preserved + // through sanitizePassthroughHeaders) so a redirect to a dead host can never + // masquerade as a pre-connection failure after the credential was seen. + // The numeric outcome above already classified it neutral — no streak. + if (upstreamResponse.status >= 300 && upstreamResponse.status < 400) { + return new Response(upstreamResponse.body, { + status: upstreamResponse.status, + statusText: upstreamResponse.statusText, + headers: sanitizePassthroughHeaders(upstreamResponse.headers), + }); + } if (!upstreamResponse.ok) { if (options.comboAttempt) { const failure = await consumeComboFailure(upstreamResponse, options.abortSignal); diff --git a/src/server/responses/fetch-helpers.ts b/src/server/responses/fetch-helpers.ts index bc245e4fc4..1cbb466b44 100644 --- a/src/server/responses/fetch-helpers.ts +++ b/src/server/responses/fetch-helpers.ts @@ -118,6 +118,16 @@ export function safeHostLabel(url: string): string { } } +/** Canonical origin (scheme + host) for failure-attribution keys: http and + * https for the same host must not share one ledger entry (#914 review). */ +export function safeOriginLabel(url: string): string { + try { + return new URL(url).origin.toLowerCase(); + } catch { + return "upstream"; + } +} + export function providerFetch(provider: OcxProviderConfig): typeof globalThis.fetch { @@ -133,6 +143,7 @@ export async function fetchWithHeaderTimeout( timeoutMs: number, preferIdentityEncoding = false, executor: typeof globalThis.fetch = globalThis.fetch, + manualRedirect = false, ): Promise { const timeout = new AbortController(); const timer = setTimeout(() => { @@ -148,10 +159,13 @@ export async function fetchWithHeaderTimeout( return await executor(url, { ...init, headers, + // Credential-bearing sends opt into manual redirects so a 3xx is relayed + // as a Response instead of being followed into a rejection that is + // indistinguishable from a pre-connection failure (#914). + ...(manualRedirect ? { redirect: "manual" as const } : {}), signal: AbortSignal.any([abortSignal, timeout.signal]), }); } finally { clearTimeout(timer); } } - diff --git a/tests/issue-914-transport-attribution.test.ts b/tests/issue-914-transport-attribution.test.ts new file mode 100644 index 0000000000..103b098a3f --- /dev/null +++ b/tests/issue-914-transport-attribution.test.ts @@ -0,0 +1,167 @@ +import { beforeEach, describe, expect, test } from "bun:test"; +import { + classifyCodexUpstreamOutcome, + clearCodexUpstreamHealth, + clearThreadAccountMap, + getCodexUpstreamHealth, + getEffectiveActiveCodexAccountId, + isCodexAccountSoftAvoided, + recordCodexUpstreamOutcome, + resolveCodexAccountForThread, +} from "../src/codex/routing"; +import { + clearUpstreamHostHealth, + getUpstreamHostHealth, + upstreamHostHealthKey, +} from "../src/codex/upstream-host-health"; +import { classifyTransportFailureKind } from "../src/lib/upstream-reachability"; +import { fetchWithResetRetry, fetchWithTransientRetry } from "../src/lib/upstream-retry"; +import { saveCodexAccountCredential } from "../src/codex/account-store"; +import { getConfigPath } from "../src/config"; +import type { OcxConfig } from "../src/types"; +import { existsSync, mkdirSync, rmSync } from "node:fs"; +import { join } from "node:path"; + +const TEST_DIR = join(import.meta.dir, ".tmp-issue-914-test"); +let previousOpencodexHome: string | undefined; +let previousCodexHome: string | undefined; + +function makeConfig(overrides: Partial = {}): OcxConfig { + return { + providers: {}, + codexAccounts: [], + activeCodexAccountId: undefined, + autoSwitchThreshold: 80, + ...overrides, + } as OcxConfig; +} + +function saveTestCredential(id: string): void { + saveCodexAccountCredential(id, { + accessToken: `access-${id}`, + refreshToken: `refresh-${id}`, + expiresAt: Date.now() + 5 * 60_000, + chatgptAccountId: `acct-${id}`, + }); +} + +function makeTwoAccountConfig(overrides: Partial = {}): OcxConfig { + for (const id of ["a", "b"]) saveTestCredential(id); + return makeConfig({ + activeCodexAccountId: "a", + codexAccounts: ["a", "b"].map(id => ({ id, email: `${id}@example.test`, isMain: false })), + ...overrides, + }); +} + +beforeEach(() => { + previousOpencodexHome = process.env.OPENCODEX_HOME; + previousCodexHome = process.env.CODEX_HOME; + rmSync(TEST_DIR, { recursive: true, force: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + process.env.CODEX_HOME = TEST_DIR; + clearCodexUpstreamHealth(); + clearThreadAccountMap(); + clearUpstreamHostHealth(); +}); + +function restoreEnv(): void { + if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOpencodexHome; + if (previousCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodexHome; + rmSync(TEST_DIR, { recursive: true, force: true }); +} + +function coded(message: string, code: string): Error { + return Object.assign(new Error(message), { code }); +} + +describe("issue #914 — pre-connection failures never touch account health", () => { + test("three concurrent neutral failures leave streak, affinity, and active account untouched", () => { + try { + const config = makeTwoAccountConfig({ upstreamFailoverThreshold: 3 }); + expect(getConfigPath().startsWith(TEST_DIR)).toBe(true); + // Pin the thread to account A the way a real continue would. + resolveCodexAccountForThread(config, "thread-914", { now: Date.now() }); + expect(getEffectiveActiveCodexAccountId(config)).toBe("a"); + + const hostKey = upstreamHostHealthKey("openai", "chatgpt.com"); + for (let i = 0; i < 3; i++) { + recordCodexUpstreamOutcome(config, "a", "connect_neutral", { + threadId: "thread-914", + hostKey, + lastFailureCode: "ECONNREFUSED", + }); + } + + // No account evidence at the failover threshold: no streak, no soft-avoid, + // no affinity loss, no rotation. The host ledger carries the failure instead. + expect(getCodexUpstreamHealth("a")).toBeNull(); + expect(isCodexAccountSoftAvoided("a")).toBe(false); + expect(getEffectiveActiveCodexAccountId(config)).toBe("a"); + expect(getUpstreamHostHealth(hostKey)).toMatchObject({ consecutiveFailures: 3, lastFailureCode: "ECONNREFUSED" }); + } finally { + restoreEnv(); + } + }); + + test("a relayed 3xx is the neutral class: no account and no host evidence", () => { + try { + const config = makeTwoAccountConfig(); + for (const status of [301, 302, 307, 308]) { + expect(classifyCodexUpstreamOutcome(status)).toBe("neutral"); + recordCodexUpstreamOutcome(config, "a", status); + } + expect(getCodexUpstreamHealth("a")).toBeNull(); + expect(isCodexAccountSoftAvoided("a")).toBe(false); + expect(getUpstreamHostHealth(upstreamHostHealthKey("openai", "chatgpt.com"))).toBeNull(); + } finally { + restoreEnv(); + } + }); + + test("mixed evidence: 503 then a reachability rejection stays account-attributed", async () => { + try { + const config = makeTwoAccountConfig({ upstreamFailoverThreshold: 1 }); + let calls = 0; + const rejection = coded("refused", "ECONNREFUSED"); + const outcome = classifyTransportFailureKind(await fetchWithTransientRetry(async () => { + calls++; + if (calls === 1) return new Response("gw", { status: 503 }); + throw rejection; + }, { slowAttemptMs: 60_000 }).catch(err => err)); + expect(calls).toBe(2); + expect(outcome).toBe("connect_error"); + recordCodexUpstreamOutcome(config, "a", outcome, { threadId: "t-mixed" }); + expect(getCodexUpstreamHealth("a")).toMatchObject({ consecutiveFailures: 1 }); + } finally { + restoreEnv(); + } + }); + + test("mixed evidence: a reset then a reachability rejection stays account-attributed", async () => { + const rejection = coded("refused", "ECONNREFUSED"); + const err = await fetchWithResetRetry(async recovery => { + if (!recovery) throw coded("reset", "ECONNRESET"); + throw rejection; + }).catch((e: unknown) => e); + expect(classifyTransportFailureKind(err)).toBe("connect_error"); + }); + + test("a plain reachability rejection classifies neutral end to end", async () => { + const err = await fetchWithTransientRetry(async () => { + throw coded("refused", "ECONNREFUSED"); + }).catch((e: unknown) => e); + expect(classifyTransportFailureKind(err)).toBe("connect_neutral"); + }); + + test("real Bun dead-port fetch rejects with a neutral-classifiable shape", async () => { + // Real socket, not a hand-built error: localhost port 1 is never listening. + const err = await fetch("http://127.0.0.1:1/", { signal: AbortSignal.timeout(5_000) }) + .catch((e: unknown) => e); + expect(err).toBeInstanceOf(Error); + expect(classifyTransportFailureKind(err)).toBe("connect_neutral"); + }); +}); diff --git a/tests/server-auth.test.ts b/tests/server-auth.test.ts index 60d6736187..849a5a204f 100644 --- a/tests/server-auth.test.ts +++ b/tests/server-auth.test.ts @@ -14,9 +14,11 @@ import { clearCodexUpstreamHealth, clearThreadAccountMap, getCodexUpstreamHealth, + isCodexAccountSoftAvoided, recordCodexUpstreamOutcome, } from "../src/codex/routing"; import { loadConfig, saveConfig } from "../src/config"; +import { clearUpstreamHostHealth, getUpstreamHostHealth, recordUpstreamHostFailure, upstreamHostHealthKey } from "../src/codex/upstream-host-health"; import { deriveProviderPresets } from "../src/providers/derive"; import { MAIN_CODEX_ACCOUNT_ID } from "../src/codex/main-account"; import { @@ -2678,10 +2680,86 @@ describe("server local API auth", () => { }); expect(response.status).toBe(502); - expect(getCodexUpstreamHealth("pool-a")).toMatchObject({ - consecutiveFailures: 1, - lastFailureStatus: 0, + // #914: a dead-port refusal is pre-connection — host-wide, not account + // evidence. Account health stays untouched; the (provider, host) ledger + // records the failure instead. + expect(getCodexUpstreamHealth("pool-a")).toBeNull(); + expect(getUpstreamHostHealth(upstreamHostHealthKey("openai", "https://chatgpt.com"))) + .toMatchObject({ consecutiveFailures: 1, lastFailureCode: "ConnectionRefused" }); + } finally { + await server.stop(true); + } + }); + + test("passthrough pool send relays a 307 with Location and records no health evidence (#914)", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + clearCodexUpstreamHealth(); + clearThreadAccountMap(); + clearAccountNeedsReauth("pool-a"); + clearUpstreamHostHealth(); + + // The upstream answers 307 -> dead.invalid. Manual redirects must relay it + // (with Location) instead of following into a dead-host rejection. + const redirectTarget = "https://dead.invalid/x"; + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const requestUrl = typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; + const url = new URL(requestUrl); + if (url.hostname === "chatgpt.com") { + // The test would only reach this branch if the proxy wrongly followed + // the redirect itself — fail loudly instead of hanging on dead.invalid. + expect(url.hostname).not.toBe("dead.invalid"); + return new Response(null, { status: 307, headers: { location: redirectTarget } }); + } + return originalGlobalFetch(input, init); + }) as typeof fetch; + + saveConfig({ + port: 0, + defaultProvider: "openai", + openaiProviderTierVersion: 2, + providers: poolProviders(), + codexAccounts: [ + { id: "main", email: "main@example.test", isMain: true }, + { id: "pool-a", email: "pool@example.test", isMain: false, chatgptAccountId: "acct-pool-a" }, + ], + activeCodexAccountId: "pool-a", + upstreamFailoverThreshold: 3, + connectTimeoutMs: 200, + } as OcxConfig); + saveCodexAccountCredential("pool-a", { + accessToken: "pool-access-token", + refreshToken: "pool-refresh-token", + expiresAt: Date.now() + 5 * 60_000, + chatgptAccountId: "acct-pool-a", + }); + updateAccountQuota("pool-a", 10, 5); + + // Seed a pre-connection streak: the 307 is also a real HTTP response and + // must clear it. + const hostKey = upstreamHostHealthKey("openai", "https://chatgpt.com"); + recordUpstreamHostFailure(hostKey, { code: "ECONNREFUSED" }); + + const server = startServer(0); + try { + const response = await originalGlobalFetch(new URL("/v1/responses", server.url), { + method: "POST", + headers: { + "content-type": "application/json", + authorization: "Bearer inbound-main-token", + }, + body: JSON.stringify({ model: "gpt-test", input: "hello", stream: false }), + redirect: "manual", }); + + expect(response.status).toBe(307); + expect(response.headers.get("location")).toBe(redirectTarget); + // Neutral class: no account streak, no soft-avoid, no rotation, and the + // real response cleared the seeded host streak. + expect(getCodexUpstreamHealth("pool-a")).toBeNull(); + expect(isCodexAccountSoftAvoided("pool-a")).toBe(false); + expect(getUpstreamHostHealth(hostKey)).toBeNull(); } finally { await server.stop(true); } diff --git a/tests/upstream-reachability.test.ts b/tests/upstream-reachability.test.ts new file mode 100644 index 0000000000..860c2273db --- /dev/null +++ b/tests/upstream-reachability.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, test } from "bun:test"; +import { + classifyTransportFailureKind, + isPreConnectReachabilityError, + transportErrorCode, + MAX_REACHABILITY_CAUSE_DEPTH, +} from "../src/lib/upstream-reachability"; +import { UpstreamRetryEvidenceError } from "../src/lib/upstream-retry"; +import { + clearUpstreamHostHealth, + getUpstreamHostHealth, + recordUpstreamHostFailure, + resetUpstreamHostHealth, + upstreamHostHealthKey, + UPSTREAM_HOST_FAILURE_WINDOW_MS, + UPSTREAM_HOST_HEALTH_MAX_ENTRIES, +} from "../src/codex/upstream-host-health"; + +function coded(message: string, code: string, cause?: unknown): Error { + return Object.assign(new Error(message), { code, ...(cause !== undefined ? { cause } : {}) }); +} + +describe("isPreConnectReachabilityError", () => { + test("accepts Bun and Node pre-connect codes at cause depth 0-2", () => { + for (const code of ["ConnectionRefused", "FailedToOpenSocket", "ECONNREFUSED", "ENOTFOUND", "EAI_AGAIN", "ENETUNREACH", "ENETDOWN", "EHOSTUNREACH"]) { + expect(isPreConnectReachabilityError(coded("x", code))).toBe(true); + } + expect(isPreConnectReachabilityError( + new Error("outer", { cause: coded("mid", "ENOENT", coded("inner", "ECONNREFUSED")) }), + )).toBe(true); + }); + + test("rejects at the bounded depth, on cycles, non-Errors, and message-only text", () => { + // depth-3 chain: beyond MAX_REACHABILITY_CAUSE_DEPTH. + let deep: unknown = coded("inner", "ECONNREFUSED"); + for (let i = 0; i < MAX_REACHABILITY_CAUSE_DEPTH; i++) deep = new Error(`wrap${i}`, { cause: deep }); + expect(isPreConnectReachabilityError(deep)).toBe(false); + + const a: { cause?: unknown } = new Error("a"); + const b: { cause?: unknown } = new Error("b"); + a.cause = b; b.cause = a; + expect(isPreConnectReachabilityError(a)).toBe(false); + + expect(isPreConnectReachabilityError("ECONNREFUSED")).toBe(false); + expect(isPreConnectReachabilityError(new Error("ECONNREFUSED api.example.com"))).toBe(false); + expect(isPreConnectReachabilityError(null)).toBe(false); + }); + + test("reset/TLS/unknown shapes stay out of the pre-connect set", () => { + for (const code of ["ECONNRESET", "EPIPE", "ERR_TLS_CERT_ALTNAME_INVALID", "EPROTO", "ETIMEDOUT"]) { + expect(isPreConnectReachabilityError(coded("x", code))).toBe(false); + } + expect(isPreConnectReachabilityError(new Error("socket hang up"))).toBe(false); + }); +}); + +describe("classifyTransportFailureKind", () => { + test("TimeoutError keeps its own identity", () => { + const err = Object.assign(new Error("t"), { name: "TimeoutError" }); + expect(classifyTransportFailureKind(err)).toBe("timeout"); + }); + + test("plain pre-connect rejection is account-neutral", () => { + expect(classifyTransportFailureKind(coded("refused", "ECONNREFUSED"))).toBe("connect_neutral"); + expect(classifyTransportFailureKind(coded("refused", "ConnectionRefused"))).toBe("connect_neutral"); + }); + + test("reset, TLS, and unknown rejections stay account-attributed", () => { + expect(classifyTransportFailureKind(coded("reset", "ECONNRESET"))).toBe("connect_error"); + expect(classifyTransportFailureKind(coded("tls", "ERR_TLS_CERT_ALTNAME_INVALID"))).toBe("connect_error"); + expect(classifyTransportFailureKind(new Error("socket hang up"))).toBe("connect_error"); + }); + + test("a transient 5xx before the rejection erases the neutral class (mixed evidence)", () => { + const err = new UpstreamRetryEvidenceError([503], coded("refused", "ECONNREFUSED")); + expect(classifyTransportFailureKind(err)).toBe("connect_error"); + }); + + test("a credential-visible reset before the rejection erases the neutral class", () => { + const err = new UpstreamRetryEvidenceError([], coded("refused", "ECONNREFUSED"), true); + expect(classifyTransportFailureKind(err)).toBe("connect_error"); + }); + + test("an evidence wrapper without credential-visible evidence keeps the neutral class", () => { + const err = new UpstreamRetryEvidenceError([], coded("refused", "ECONNREFUSED")); + expect(classifyTransportFailureKind(err)).toBe("connect_neutral"); + }); + + test("transportErrorCode unwraps the evidence error", () => { + const err = new UpstreamRetryEvidenceError([502], coded("refused", "ECONNREFUSED")); + expect(transportErrorCode(err)).toBe("ECONNREFUSED"); + expect(transportErrorCode(new Error("x"))).toBeUndefined(); + }); +}); + +describe("upstream host health ledger", () => { + test("records, windows, resets, and prunes at the 128-entry cap", () => { + clearUpstreamHostHealth(); + const key = upstreamHostHealthKey("openai", "chatgpt.com"); + expect(key).toBe("openai|chatgpt.com"); + + recordUpstreamHostFailure(key, { code: "ECONNREFUSED", now: 1000 }); + recordUpstreamHostFailure(key, { now: 2000 }); + expect(getUpstreamHostHealth(key)).toMatchObject({ consecutiveFailures: 2, lastFailureCode: "ECONNREFUSED" }); + + // Stale window: a failure after the window restarts the streak. + recordUpstreamHostFailure(key, { now: 2000 + UPSTREAM_HOST_FAILURE_WINDOW_MS + 1 }); + expect(getUpstreamHostHealth(key)?.consecutiveFailures).toBe(1); + + resetUpstreamHostHealth(key); + expect(getUpstreamHostHealth(key)).toBeNull(); + + // Churn: many distinct providers/hosts never grow the map past the cap. + for (let i = 0; i < UPSTREAM_HOST_HEALTH_MAX_ENTRIES * 3; i++) { + recordUpstreamHostFailure(upstreamHostHealthKey(`p${i}`, `h${i}.test`), { now: 10_000 + i }); + } + let size = 0; + for (let i = 0; i < UPSTREAM_HOST_HEALTH_MAX_ENTRIES * 3; i++) { + if (getUpstreamHostHealth(upstreamHostHealthKey(`p${i}`, `h${i}.test`))) size++; + } + expect(size).toBeLessThanOrEqual(UPSTREAM_HOST_HEALTH_MAX_ENTRIES); + // The freshest entries survive stalest-first pruning. + const freshest = UPSTREAM_HOST_HEALTH_MAX_ENTRIES * 3 - 1; + expect(getUpstreamHostHealth(upstreamHostHealthKey(`p${freshest}`, `h${freshest}.test`))).not.toBeNull(); + clearUpstreamHostHealth(); + }); +});