From fe693ae628bf73e765d703ba212ca06fcedd863b Mon Sep 17 00:00:00 2001 From: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com> Date: Tue, 4 Aug 2026 04:31:59 +0800 Subject: [PATCH 1/5] fix(codex): keep pre-connection DNS/network failures off account health (#914) Bun 1.3.14 collapses DNS failure and TCP refusal into one pre-connect label class (ConnectionRefused / FailedToOpenSocket, errno 0, no cause), and the transport layer previously mapped every non-timeout rejection to connect_error. After upstreamFailoverThreshold such failures, a healthy pool account was soft-avoided, thread affinity cleared, and another account promoted - rotation that cannot repair a machine-wide outage. Classify proven pre-connection reachability failures as account-neutral: - new leaf classifier matching stable codes through a bounded cause chain (never message substrings); ECONNRESET/EPIPE/TLS/timeout/unknown shapes keep their existing account-attributed handling - new (provider, host) health ledger with its own threshold absorbs the failures; account streak, soft-avoid, affinity, and active account stay untouched, and any owned quota-probe lease is released - pool sends use redirect: manual, so a 3xx to a dead host is relayed as a Response and can never masquerade as a pre-connection rejection; 3xx is an explicit neutral class (no account or host evidence) - same classification on the compact path, the alternate-account send, and the search/live/images/web-search/vision sidecar outcome sites Verified on Bun 1.3.14: DNS failure and TCP refusal both reject with the pre-connect label class (errno 0, no cause), alternating between labels; read-then-close rejects with ECONNRESET and stays account-attributed. --- .../ja/reference/configuration/providers.md | 2 +- .../ko/reference/configuration/providers.md | 2 +- .../docs/reference/configuration/providers.md | 2 +- .../ru/reference/configuration/providers.md | 2 +- .../reference/configuration/providers.md | 2 +- src/codex/routing.ts | 90 +++++- src/lib/upstream-reachability.ts | 86 ++++++ src/providers/openai-sidecar.ts | 9 +- src/server/images.ts | 17 +- src/server/live.ts | 19 +- src/server/responses/compact.ts | 27 +- src/server/responses/core.ts | 21 +- src/server/responses/fetch-helpers.ts | 6 +- src/server/search.ts | 16 +- src/vision/describe.ts | 12 +- src/web-search/executor.ts | 19 +- tests/codex-routing.test.ts | 118 ++++++++ tests/issue-914-transport-attribution.test.ts | 286 ++++++++++++++++++ tests/server-auth.test.ts | 11 +- tests/upstream-reachability.test.ts | 114 +++++++ 20 files changed, 822 insertions(+), 39 deletions(-) create mode 100644 src/lib/upstream-reachability.ts create mode 100644 tests/issue-914-transport-attribution.test.ts create mode 100644 tests/upstream-reachability.test.ts 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 39a4513973..184b20c8d7 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到達不能障害はアカウント中立で、カウントされません。 | | `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 f3d08deefb..3ad3abe10d 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 도달 불가 실패는 계정 중립이며 집계되지 않습니다. | | `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 a04a142efc..b4834d1332 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 account-neutral and never count. | | `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 18d884525f..b9b16e548f 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 3702c00467..af7534842d 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 不可达故障与账户无关，不计入次数。 | | `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 63030c5c5b..a2445d09af 100644 --- a/src/codex/routing.ts +++ b/src/codex/routing.ts @@ -122,8 +122,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 @@ -133,6 +133,47 @@ export type CodexCooldownSource = "retry-after" | "reset-derived" | "default"; */ export type CodexQuotaScope = "shared" | "spark"; +/** + * Pre-connection reachability health for a (provider, host) pair (#914). + * + * DNS and TCP-level failures belong to the host, not to any single credential: + * every Codex pool account shares the provider host, so rotating accounts + * cannot repair a machine-wide outage. These failures are recorded here + * instead of on the account so the account streak, soft-avoid, thread affinity, + * and active-account state stay untouched. The ledger is observational today; + * {@link isHostConnectOutage} is the hook where a host-level response (for + * example a circuit break) would attach with its own audit. + */ +export interface HostConnectHealth { + consecutiveFailures: number; + lastFailureAt: number; + lastFailureCode?: string; +} + +/** Consecutive proven reachability failures before a host is in outage. */ +export const HOST_CONNECT_OUTAGE_THRESHOLD = 3; + +const hostConnectHealth = new Map(); + +export function hostConnectHealthKey(providerName: string, host: string): string { + return `${providerName.toLowerCase()}:${host.toLowerCase()}`; +} + +export function getHostConnectHealth(key: string): HostConnectHealth | null { + return hostConnectHealth.get(key) ?? null; +} + +export function isHostConnectOutage( + key: string, + threshold = HOST_CONNECT_OUTAGE_THRESHOLD, + now = Date.now(), +): boolean { + if (threshold <= 0) return false; + const health = hostConnectHealth.get(key); + if (!health || now - health.lastFailureAt > CODEX_FAILURE_WINDOW_MS) return false; + return health.consecutiveFailures >= threshold; +} + /** * Requests without a resolved native model retain the historic one-account-per- * thread behavior. Requests with a known quota scope get an independent @@ -197,6 +238,13 @@ export type CodexUpstreamOutcomeMeta = { * again (which would advance a round-robin ring twice). */ promoteAccountId?: string; + /** + * (provider, host) key for a proven pre-connection reachability failure + * (#914). Records the failure in the host ledger instead of account health. + */ + hostKey?: string; + /** Stable `code` on the classified rejection, kept for the host ledger. */ + lastFailureCode?: string; /** Generation captured when this routed account was selected. */ writerGeneration?: number; }; @@ -238,6 +286,7 @@ export function clearThreadAccountMapForAccount(accountId: string): void { export function clearCodexUpstreamHealth(): void { upstreamHealth.clear(); quotaScopedHealth.clear(); + hostConnectHealth.clear(); runtimeActiveCodexAccountId = undefined; } @@ -307,9 +356,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). @@ -1363,6 +1418,37 @@ export function recordCodexUpstreamOutcome( return; } + if (outcomeClass === "neutral") { + // A proven pre-connection reachability failure (DNS / TCP refusal) is host- + // wide: every pool account shares the provider host, so rotation cannot + // repair it and must not happen (#914). Conclude any owned probe lease + // without treating the failure as account evidence, record the failure + // under the (provider, host) ledger, 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)); + } + if (meta.hostKey) { + const prior = hostConnectHealth.get(meta.hostKey); + const stale = !!prior && now - prior.lastFailureAt > CODEX_FAILURE_WINDOW_MS; + hostConnectHealth.set(meta.hostKey, { + consecutiveFailures: stale ? 1 : (prior?.consecutiveFailures ?? 0) + 1, + lastFailureAt: now, + ...(typeof meta.lastFailureCode === "string" + ? { lastFailureCode: meta.lastFailureCode } + : {}), + }); + } + 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/lib/upstream-reachability.ts b/src/lib/upstream-reachability.ts new file mode 100644 index 0000000000..e66e9564ec --- /dev/null +++ b/src/lib/upstream-reachability.ts @@ -0,0 +1,86 @@ +/** + * 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 1.3.14 collapses DNS failure and TCP refusal into one label class + * (`ConnectionRefused` / `FailedToOpenSocket`, errno 0, no cause) and alternates + * between the two labels as its DNS cache is evicted; 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. + * + * MUST stay a leaf module: imports nothing from server.ts or adapters. + */ + +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; + for (let depth = 0; depth < MAX_REACHABILITY_CAUSE_DEPTH; depth++) { + if (!(current instanceof Error)) return false; + const code = (current as { code?: unknown }).code; + if (typeof code === "string" && PRE_CONNECT_REACHABILITY_CODES.has(code)) return true; + const cause = current.cause; + if (cause === undefined || cause === current) return false; + 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 { + if (err instanceof Error && err.name === "TimeoutError") return "timeout"; + if (isPreConnectReachabilityError(err)) return "connect_neutral"; + return "connect_error"; +} + +/** Stable `code` carried by a transport rejection, when there is one. */ +export function transportErrorCode(err: unknown): string | undefined { + if (!(err instanceof Error)) return undefined; + const code = (err as { code?: unknown }).code; + return typeof code === "string" && code !== "" ? code : undefined; +} + +/** Host component of a transport failure key, or null when the URL is malformed. */ +export function transportFailureHost(url: string): string | null { + try { + return new URL(url).host; + } catch { + return null; + } +} diff --git a/src/providers/openai-sidecar.ts b/src/providers/openai-sidecar.ts index 43b4e2396a..14b1704d7d 100644 --- a/src/providers/openai-sidecar.ts +++ b/src/providers/openai-sidecar.ts @@ -8,9 +8,10 @@ import { type CodexAccountSelectionAdmission, type CodexAuthContext, } from "../codex/auth-context"; -import { recordCodexUpstreamOutcome, type CodexUpstreamOutcome } from "../codex/routing"; +import { hostConnectHealthKey, recordCodexUpstreamOutcome, type CodexUpstreamOutcome } from "../codex/routing"; import { extractAccountId } from "../oauth/chatgpt"; import { ForwardAdmissionCredentialError, validateForwardAdmissionCredential } from "../server/auth-cors"; +import type { SidecarOutcomeMeta } from "../web-search/executor"; import type { CodexAccountMode, OcxConfig, OcxProviderConfig } from "../types"; import { isCanonicalOpenAiForwardProvider, @@ -28,7 +29,7 @@ export interface OpenAiForwardSidecarCandidate { export interface ResolvedOpenAiForwardSidecar extends OpenAiForwardSidecarCandidate { authContext: CodexAuthContext; headers: Headers; - recordOutcome?: (outcome: CodexUpstreamOutcome) => void; + recordOutcome?: (outcome: CodexUpstreamOutcome, meta?: SidecarOutcomeMeta) => void; } /** @@ -157,13 +158,15 @@ export async function resolveFirstUsableOpenAiSidecar( headers: headersForCodexAuthContext(incomingHeaders, authContext), ...(authContext.kind === "pool" || authContext.kind === "main-pool" ? { - recordOutcome: (outcome: CodexUpstreamOutcome) => recordCodexUpstreamOutcome( + recordOutcome: (outcome: CodexUpstreamOutcome, meta: SidecarOutcomeMeta = {}) => recordCodexUpstreamOutcome( config, authContext.accountId, outcome, { probeLeaseId: authContext.probeLeaseId, writerGeneration: authContext.writerGeneration, + ...(meta.host ? { hostKey: hostConnectHealthKey(candidate.providerName, meta.host) } : {}), + ...(meta.lastFailureCode ? { lastFailureCode: meta.lastFailureCode } : {}), }, ), } diff --git a/src/server/images.ts b/src/server/images.ts index a35ca84e69..c6b2646e65 100644 --- a/src/server/images.ts +++ b/src/server/images.ts @@ -24,7 +24,13 @@ import { } from "../codex/auth-context"; import { formatCodexProviderForLog } from "../codex/routing"; import { signalWithTimeout } from "../lib/abort"; +import { + classifyTransportFailureKind, + transportErrorCode, + transportFailureHost, +} from "../lib/upstream-reachability"; import { sidecarEnter } from "../lib/sidecar-tracker"; +import type { SidecarOutcomeMeta } from "../web-search/executor"; import type { OcxConfig } from "../types"; import { resolveFirstUsableOpenAiSidecar, selectImagesProvider } from "../providers/openai-sidecar"; import { getProviderRegistryEntry } from "../providers/registry"; @@ -467,12 +473,15 @@ export async function handleImages( if (req.signal.aborted) { return formatErrorResponse(499, "client_closed_request", `image ${endpoint} request canceled by client`); } - if (err instanceof Error && err.name === "TimeoutError") { - forward?.recordOutcome?.("timeout"); - // codex retries 5xx up to 4 more times; a retried 504 is acceptable for a transient hang. + const kind = classifyTransportFailureKind(err); + forward?.recordOutcome?.(kind, { + host: transportFailureHost(url) ?? undefined, + lastFailureCode: transportErrorCode(err), + }); + // codex retries 5xx up to 4 more times; a retried 504 is acceptable for a transient hang. + if (kind === "timeout") { return formatErrorResponse(504, "upstream_error", `image ${endpoint} upstream timed out`); } - forward?.recordOutcome?.("connect_error"); return formatErrorResponse( 502, "upstream_error", diff --git a/src/server/live.ts b/src/server/live.ts index c430e0fdb9..072122d573 100644 --- a/src/server/live.ts +++ b/src/server/live.ts @@ -33,7 +33,13 @@ import { } from "../codex/auth-context"; import { formatCodexProviderForLog } from "../codex/routing"; import { signalWithTimeout } from "../lib/abort"; +import { + classifyTransportFailureKind, + transportErrorCode, + transportFailureHost, +} from "../lib/upstream-reachability"; import { sidecarEnter } from "../lib/sidecar-tracker"; +import type { SidecarOutcomeMeta } from "../web-search/executor"; import type { OcxConfig } from "../types"; import { resolveFirstUsableOpenAiSidecar, selectOpenAiImagesProvider } from "../providers/openai-sidecar"; import { ForwardAdmissionCredentialError, validateForwardAdmissionCredential } from "./auth-cors"; @@ -151,7 +157,7 @@ export type LiveRelayTarget = { providerBaseUrl: string; usesBackendShape: boolean; keyed: boolean; - recordOutcome?: (status: number | "timeout" | "connect_error") => void; + recordOutcome?: (status: number | "timeout" | "connect_error" | "connect_neutral", meta?: SidecarOutcomeMeta) => void; }; function isChatGptBackendBaseUrl(baseUrl: string): boolean { @@ -487,7 +493,7 @@ export async function resolveLiveRelay( providerBaseUrl: provider.baseUrl, usesBackendShape: isChatGptBackendBaseUrl(provider.baseUrl), keyed: false, - recordOutcome: status => forward.recordOutcome?.(status), + recordOutcome: (status, meta) => forward.recordOutcome?.(status, meta), }; } if (forwardAuthError) return forwardAuthError; @@ -575,11 +581,14 @@ export async function handleLive( if (req.signal.aborted) { return formatErrorResponse(499, "client_closed_request", "live request canceled by client"); } - if (err instanceof Error && err.name === "TimeoutError") { - relay.recordOutcome?.("timeout"); + const kind = classifyTransportFailureKind(err); + relay.recordOutcome?.(kind, { + host: transportFailureHost(url) ?? undefined, + lastFailureCode: transportErrorCode(err), + }); + if (kind === "timeout") { return formatErrorResponse(504, "upstream_error", "live upstream timed out"); } - relay.recordOutcome?.("connect_error"); return formatErrorResponse( 502, "upstream_error", diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index 9a49a03f7b..a6574b44e3 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -59,9 +59,11 @@ import { } from "../../codex/auth-context"; import { formatCodexProviderForLog, + hostConnectHealthKey, recordCodexUpstreamOutcome, type CodexUpstreamOutcome, } from "../../codex/routing"; +import { classifyTransportFailureKind, transportErrorCode } from "../../lib/upstream-reachability"; import { fetchWithResetRetry, fetchWithTransientRetry, @@ -361,6 +363,8 @@ export async function handleResponsesCompact( retryAfter?: string | null; resetAt?: unknown | unknown[]; promoteAccountId?: string; + hostKey?: string; + lastFailureCode?: string; } = {}, ) => { if (!usesCodexForwardPoolAuth(ctx, route.provider)) return; @@ -397,6 +401,7 @@ export async function handleResponsesCompact( connectMs, false, providerFetch(sendProvider), + usesCodexForwardPoolAuth(authCtx, route.provider), ); return recovery === "single" ? doFetch() @@ -416,8 +421,15 @@ 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"; - recordCompactPoolOutcome(outcomeCtx, outcome); + const outcome = classifyTransportFailureKind(err); + recordCompactPoolOutcome(outcomeCtx, outcome, { + ...(outcome === "connect_neutral" + ? { + hostKey: hostConnectHealthKey(route.providerName, safeHostLabel(compactUrl)), + lastFailureCode: transportErrorCode(err), + } + : {}), + }); return formatErrorResponse(502, "upstream_error", "Failed to connect to compact upstream"); } @@ -484,8 +496,15 @@ 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"; - recordCompactPoolOutcome(outcomeCtx, outcome); + const outcome = classifyTransportFailureKind(err); + recordCompactPoolOutcome(outcomeCtx, outcome, { + ...(outcome === "connect_neutral" + ? { + hostKey: hostConnectHealthKey(route.providerName, safeHostLabel(compactUrl)), + lastFailureCode: transportErrorCode(err), + } + : {}), + }); 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 845c4a3384..3f6ecea5a1 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -83,10 +83,12 @@ import { import { computeQuotaCooldown, formatCodexProviderForLog, + hostConnectHealthKey, previewCodexAccountForRequest, recordCodexUpstreamOutcome, type CodexUpstreamOutcome, } from "../../codex/routing"; +import { classifyTransportFailureKind, transportErrorCode } from "../../lib/upstream-reachability"; import { fetchWithResetRetry, fetchWithTransientRetry, applyUpstreamRecoveryInit } from "../../lib/upstream-retry"; import { ForwardAdmissionCredentialError, validateForwardAdmissionCredential } from "../auth-cors"; import { createTranslatorBudget, isTranslatorBudgetExceededError, type TranslatorBudget } from "../../lib/translator-budget"; @@ -425,6 +427,7 @@ async function retryCodexPoolOnAlternateAccount( connectMs, stream, providerFetch(route.provider), + true, ); return { kind: "retried", @@ -1710,7 +1713,7 @@ 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); if (usesCodexForwardPoolAuth(authCtx, route.provider)) { recordCodexUpstreamOutcome(config, authCtx.accountId, outcome, { threadId: req.headers.get("x-codex-parent-thread-id"), @@ -1719,6 +1722,15 @@ async function handleResponsesInner( probeLeaseId: codexProbeLeaseId(authCtx), probeQuotaScope: codexProbeQuotaScope(authCtx), writerGeneration: authCtx.writerGeneration, + // Proven pre-connection reachability failures (DNS / TCP refusal) are + // host-wide, not account evidence: record them in the (provider, host) + // ledger instead of the account's failure streak (#914). + ...(outcome === "connect_neutral" + ? { + hostKey: hostConnectHealthKey(route.providerName, safeHostLabel(request.url)), + lastFailureCode: transportErrorCode(err), + } + : {}), }); } const msg = outcome === "timeout" @@ -1727,6 +1739,10 @@ async function handleResponsesInner( return formatErrorResponse(502, "upstream_error", msg); }; try { + // Manual redirect on pool sends: a 3xx surfaces as a Response instead of a + // followed redirect, so a server that redirects to a dead host can never + // masquerade as a pre-connection failure (recorded rejection class #914). + const poolUpstreamSend = usesCodexForwardPoolAuth(authCtx, route.provider); // Transient-5xx pre-stream retry (devlog/_plan/260716_claudecode_hardening/010): // the ChatGPT backend emits transient 502/520s that an immediate retry absorbs. // Body is a replayable string; nothing has streamed to the client yet. @@ -1737,7 +1753,8 @@ 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), + poolUpstreamSend); }, { abortSignal: upstream.signal, label: safeHostLabel(request.url) }, ); diff --git a/src/server/responses/fetch-helpers.ts b/src/server/responses/fetch-helpers.ts index bc245e4fc4..bba1c725d7 100644 --- a/src/server/responses/fetch-helpers.ts +++ b/src/server/responses/fetch-helpers.ts @@ -133,6 +133,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 +149,13 @@ export async function fetchWithHeaderTimeout( return await executor(url, { ...init, headers, + // Pool 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/src/server/search.ts b/src/server/search.ts index 27ce98cabf..8bb0f19a3b 100644 --- a/src/server/search.ts +++ b/src/server/search.ts @@ -21,6 +21,11 @@ import { import { codexAccountNamespaceForModel } from "../codex/account-namespace-match"; import { formatCodexProviderForLog } from "../codex/routing"; import { signalWithTimeout } from "../lib/abort"; +import { + classifyTransportFailureKind, + transportErrorCode, + transportFailureHost, +} from "../lib/upstream-reachability"; import { sidecarEnter } from "../lib/sidecar-tracker"; import type { OcxConfig } from "../types"; import { @@ -163,11 +168,12 @@ export async function handleSearch( if (req.signal.aborted) { return formatErrorResponse(499, "client_closed_request", "search request canceled by client"); } - if (err instanceof Error && err.name === "TimeoutError") { - upstream.recordOutcome?.("timeout"); - return formatErrorResponse(504, "upstream_error", "search upstream timed out"); - } - upstream.recordOutcome?.("connect_error"); + const kind = classifyTransportFailureKind(err); + upstream.recordOutcome?.(kind, { + host: transportFailureHost(url) ?? undefined, + lastFailureCode: transportErrorCode(err), + }); + if (kind === "timeout") return formatErrorResponse(504, "upstream_error", "search upstream timed out"); return formatErrorResponse( 502, "upstream_error", diff --git a/src/vision/describe.ts b/src/vision/describe.ts index 68744fd906..a9a5230bd9 100644 --- a/src/vision/describe.ts +++ b/src/vision/describe.ts @@ -5,6 +5,11 @@ import { redactSecretString } from "../lib/redact"; import { sidecarEnter } from "../lib/sidecar-tracker"; import { fetchWithResetRetry } from "../lib/upstream-retry"; import { parseSidecarSSE } from "../web-search/parse"; +import { + classifyTransportFailureKind, + transportErrorCode, + transportFailureHost, +} from "../lib/upstream-reachability"; import type { SidecarOutcomeRecorder } from "../web-search/executor"; export interface VisionSettings { @@ -114,8 +119,11 @@ export async function describeImage( if (!parsed.text.trim() && parsed.error) return { text: "", error: parsed.error }; return { text: parsed.text }; } catch (e) { - recordOutcome?.(e instanceof Error && e.name === "TimeoutError" ? "timeout" : "connect_error"); - const kind = e instanceof Error && e.name === "TimeoutError" ? "timeout" : "connect_error"; + const kind = classifyTransportFailureKind(e); + recordOutcome?.(kind, { + host: transportFailureHost(`${forwardProvider.baseUrl}/responses`) ?? undefined, + lastFailureCode: transportErrorCode(e), + }); console.warn(`[vision] sidecar ${kind} (${Date.now() - t0}ms)`); return { text: "", error: e instanceof Error ? e.message : String(e) }; } finally { diff --git a/src/web-search/executor.ts b/src/web-search/executor.ts index 290e1883b0..29c5debd23 100644 --- a/src/web-search/executor.ts +++ b/src/web-search/executor.ts @@ -4,6 +4,11 @@ import { signalWithTimeout, cancelBodyOnAbort } from "../lib/abort"; import { redactSecretString } from "../lib/redact"; import { sidecarEnter } from "../lib/sidecar-tracker"; import { fetchWithResetRetry } from "../lib/upstream-retry"; +import { + classifyTransportFailureKind, + transportErrorCode, + transportFailureHost, +} from "../lib/upstream-reachability"; import { parseSidecarSSE, type WebSearchResult } from "./parse"; import type { CodexUpstreamOutcome } from "../codex/routing"; @@ -31,7 +36,12 @@ export const IMAGE_INSTRUCTION = /** A search result, or an `error` string when the search couldn't run (surfaced as a tool result). */ export type SidecarOutcome = WebSearchResult & { error?: string }; -export type SidecarOutcomeRecorder = (outcome: CodexUpstreamOutcome) => void; +export type SidecarOutcomeMeta = { + /** Host the failure was observed against, for the (provider, host) ledger (#914). */ + host?: string; + lastFailureCode?: string; +}; +export type SidecarOutcomeRecorder = (outcome: CodexUpstreamOutcome, meta?: SidecarOutcomeMeta) => void; /** * Execute ONE web search via the gpt-mini sidecar through the ChatGPT forward backend — the only path @@ -94,8 +104,11 @@ export async function runWebSearch( detachBodyGuard(); } } catch (e) { - recordOutcome?.(e instanceof Error && e.name === "TimeoutError" ? "timeout" : "connect_error"); - const kind = e instanceof Error && e.name === "TimeoutError" ? "timeout" : "connect_error"; + const kind = classifyTransportFailureKind(e); + recordOutcome?.(kind, { + host: transportFailureHost(url) ?? undefined, + lastFailureCode: transportErrorCode(e), + }); console.warn(`[web-search] sidecar ${kind} for query "${query.slice(0, 80)}" (${Date.now() - t0}ms)`); return { text: "", sources: [], error: e instanceof Error ? e.message : String(e) }; } finally { diff --git a/tests/codex-routing.test.ts b/tests/codex-routing.test.ts index be0e8d23fd..bc72ea1e8e 100644 --- a/tests/codex-routing.test.ts +++ b/tests/codex-routing.test.ts @@ -8,6 +8,7 @@ import { CODEX_THREAD_AFFINITY_IDLE_TTL_MS, CODEX_THREAD_AFFINITY_MAX_ENTRIES, CODEX_THREAD_AFFINITY_REEVAL_INTERVAL_MS, + HOST_CONNECT_OUTAGE_THRESHOLD, classifyCodexUpstreamOutcome, clearCodexAccountCooldown, clearCodexUpstreamHealth, @@ -20,8 +21,11 @@ import { getCodexQuotaHealthSnapshot, getCodexAccountSoftAvoidUntil, getCodexUpstreamHealth, + getHostConnectHealth, + hostConnectHealthKey, isCodexAccountInCooldown, isCodexAccountSoftAvoided, + isHostConnectOutage, pickLowestUsageCodexAccount, parseRetryAfterMs, recordCodexUpstreamOutcome, @@ -319,10 +323,13 @@ describe("codex routing", () => { expect(classifyCodexUpstreamOutcome(403)).toBe("credential"); expect(classifyCodexUpstreamOutcome(429)).toBe("quota"); expect(classifyCodexUpstreamOutcome(402)).toBe("quota"); + expect(classifyCodexUpstreamOutcome(307)).toBe("neutral"); + expect(classifyCodexUpstreamOutcome(302)).toBe("neutral"); expect(classifyCodexUpstreamOutcome(422)).toBe("caller"); expect(classifyCodexUpstreamOutcome(503)).toBe("transient"); expect(classifyCodexUpstreamOutcome("connect_error")).toBe("transient"); expect(classifyCodexUpstreamOutcome("timeout")).toBe("transient"); + expect(classifyCodexUpstreamOutcome("connect_neutral")).toBe("neutral"); expect(classifyCodexUpstreamOutcome(102)).toBe("unknown"); }); @@ -388,6 +395,117 @@ describe("codex routing", () => { expect(resolveCodexAccountForThread("connect-next", config)).toBe("b"); }); + test("three neutral reachability failures never touch account health or routing state", () => { + const config = makeConfig(); + updateAccountQuota("a", 10); + updateAccountQuota("b", 20); + const thread = "neutral-reachability-thread"; + expect(resolveCodexAccountForThread(thread, config)).toBe("a"); + const key = hostConnectHealthKey("openai", "api.chatgpt.com"); + + recordCodexUpstreamOutcome(config, "a", "connect_neutral", { + hostKey: key, + lastFailureCode: "ConnectionRefused", + }); + recordCodexUpstreamOutcome(config, "a", "connect_neutral", { + hostKey: key, + lastFailureCode: "FailedToOpenSocket", + }); + recordCodexUpstreamOutcome(config, "a", "connect_neutral", { + hostKey: key, + lastFailureCode: "ConnectionRefused", + }); + + expect(getCodexUpstreamHealth("a")).toBeNull(); + expect(getCodexAccountSoftAvoidUntil("a")).toBeNull(); + expect(isCodexAccountSoftAvoided("a")).toBe(false); + expect(getEffectiveActiveCodexAccountId(config)).toBe("a"); + // The failure streak that would previously have tripped failover is gone: + // the bound thread and future threads both stay on "a". + expect(resolveCodexAccountForThread(thread, config)).toBe("a"); + expect(resolveCodexAccountForThread("neutral-next", config)).toBe("a"); + // The failures are recorded where they belong: the (provider, host) ledger. + expect(getHostConnectHealth(key)).toMatchObject({ + consecutiveFailures: 3, + lastFailureCode: "ConnectionRefused", + }); + expect(isHostConnectOutage(key, HOST_CONNECT_OUTAGE_THRESHOLD)).toBe(true); + expect(isHostConnectOutage("openai:other.example", HOST_CONNECT_OUTAGE_THRESHOLD)).toBe(false); + }); + + test("the host ledger forgets failures outside the failure window", () => { + const config = makeConfig(); + const now = 1_800_000_000_000; + const key = hostConnectHealthKey("openai", "api.chatgpt.com"); + recordCodexUpstreamOutcome(config, "a", "connect_neutral", { hostKey: key, now }); + recordCodexUpstreamOutcome(config, "a", "connect_neutral", { hostKey: key, now: now + 1_000 }); + + recordCodexUpstreamOutcome(config, "a", "connect_neutral", { + hostKey: key, + now: now + 2 * CODEX_FAILURE_WINDOW_MS, + }); + + expect(getHostConnectHealth(key)).toMatchObject({ consecutiveFailures: 1 }); + expect(isHostConnectOutage(key, HOST_CONNECT_OUTAGE_THRESHOLD, now + 2 * CODEX_FAILURE_WINDOW_MS)).toBe(false); + }); + + test("a neutral reachability failure releases an owned probe lease without clearing the cooldown", () => { + const config = makeConfig(); + const now = 1_800_000_000_000; + const resetAt = Math.floor((now + 4 * 24 * 60 * 60_000) / 1000); + recordCodexUpstreamOutcome(config, "a", 429, { resetAt, now }); + const probeAt = now + CODEX_QUOTA_PROBE_INTERVAL_MS; + const probeLeaseId = tryAcquireCodexQuotaProbeLease("a", probeAt)!; + const key = hostConnectHealthKey("openai", "api.chatgpt.com"); + + recordCodexUpstreamOutcome(config, "a", "connect_neutral", { + now: probeAt + 500, + probeLeaseId, + hostKey: key, + }); + + // The lease is handed back so a later probe can run, but the 429 cooldown + // itself is untouched (a reachability failure is not account evidence). + expect(getCodexUpstreamHealth("a")).toMatchObject({ + consecutiveFailures: 0, + cooldownUntil: now + 15 * 60_000, + }); + expect(getCodexUpstreamHealth("a")?.probeLeaseId).toBeUndefined(); + expect(isCodexAccountInCooldown("a", probeAt + 500)).toBe(true); + expect(getHostConnectHealth(key)).toMatchObject({ consecutiveFailures: 1 }); + }); + + test("a neutral failure never releases someone else's probe lease", () => { + const config = makeConfig(); + const now = 1_800_000_000_000; + const resetAt = Math.floor((now + 4 * 24 * 60 * 60_000) / 1000); + recordCodexUpstreamOutcome(config, "a", 429, { resetAt, now }); + const probeAt = now + CODEX_QUOTA_PROBE_INTERVAL_MS; + const probeLeaseId = tryAcquireCodexQuotaProbeLease("a", probeAt)!; + + recordCodexUpstreamOutcome(config, "a", "connect_neutral", { + now: probeAt + 500, + probeLeaseId: "someone-else", + hostKey: hostConnectHealthKey("openai", "api.chatgpt.com"), + }); + + expect(getCodexUpstreamHealth("a")?.probeLeaseId).toBe(probeLeaseId); + }); + + test("a recorded 3xx is relayed-class neutral and never increments the transient streak", () => { + const config = makeConfig(); + updateAccountQuota("a", 10); + updateAccountQuota("b", 20); + expect(resolveCodexAccountForThread("3xx-thread", config)).toBe("a"); + + recordCodexUpstreamOutcome(config, "a", 307); + recordCodexUpstreamOutcome(config, "a", 302); + recordCodexUpstreamOutcome(config, "a", 308); + + expect(getCodexUpstreamHealth("a")).toBeNull(); + expect(resolveCodexAccountForThread("3xx-next", config)).toBe("a"); + }); + test("429 with Retry-After records an account cooldown", () => { const config = makeConfig(); const now = 1_800_000_000_000; diff --git a/tests/issue-914-transport-attribution.test.ts b/tests/issue-914-transport-attribution.test.ts new file mode 100644 index 0000000000..5827606c8f --- /dev/null +++ b/tests/issue-914-transport-attribution.test.ts @@ -0,0 +1,286 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { saveCodexAccountCredential } from "../src/codex/account-store"; +import { clearAccountNeedsReauth, clearAccountQuota, updateAccountQuota } from "../src/codex/auth-api"; +import { + clearCodexUpstreamHealth, + clearThreadAccountMap, + getCodexUpstreamHealth, + getEffectiveActiveCodexAccountId, + getHostConnectHealth, + hostConnectHealthKey, + isCodexAccountSoftAvoided, + resolveCodexAccountForThread, +} from "../src/codex/routing"; +import { saveConfig } from "../src/config"; +import { startServer } from "../src/server"; +import type { OcxConfig } from "../src/types"; +import { installIsolatedCodexHome, type IsolatedCodexHome } from "./helpers/isolated-codex-home"; + +const ORIGINAL_FETCH = globalThis.fetch; +const CODEX_BASE = "https://chatgpt.com/backend-api/codex"; +const THREAD = "thread-914"; +let testDir = ""; +let previousHome: string | undefined; +let isolatedCodexHome: IsolatedCodexHome | null = null; +let outage = false; + +function stubPoolUpstream(): void { + outage = true; + globalThis.fetch = (async (input, init) => { + const requestUrl = input instanceof Request + ? input.url + : input instanceof URL + ? input.toString() + : String(input); + const url = new URL(requestUrl); + if (url.hostname === "chatgpt.com" && url.pathname.startsWith("/backend-api/codex")) { + if (outage) { + // Bun 1.3.14's real DNS-failure rejection shape (probe-verified 2026-08-04): + // code "ConnectionRefused", errno 0, no cause. + throw Object.assign(new Error("Unable to connect. Is the computer able to access the url?"), { + code: "ConnectionRefused", + errno: 0, + }); + } + const auth = new Headers(init?.headers).get("authorization") ?? ""; + return new Response(JSON.stringify({ + id: "resp-issue-914", + object: "response", + created_at: 1, + status: "completed", + model: "gpt-5.6-sol", + output: [{ + type: "message", + id: "msg-1", + role: "assistant", + content: [{ type: "output_text", text: auth.includes("access-a") ? "served-by-a" : "served-by-other" }], + }], + }), { status: 200, headers: { "content-type": "application/json" } }); + } + return ORIGINAL_FETCH(input, init); + }) as typeof fetch; +} + +function makePoolConfig(overrides: Partial = {}): OcxConfig { + return { + port: 0, + hostname: "127.0.0.1", + defaultProvider: "openai", + openaiProviderTierVersion: 2, + upstreamFailoverThreshold: 3, + accountPoolStrategy: "fill-first", + activeCodexAccountId: "a", + codexAccounts: [ + { id: "a", email: "a@example.test", isMain: false, chatgptAccountId: "acct-a" }, + { id: "b", email: "b@example.test", isMain: false, chatgptAccountId: "acct-b" }, + ], + providers: { + openai: { + adapter: "openai-responses", + baseUrl: CODEX_BASE, + authMode: "forward", + codexAccountMode: "pool", + }, + }, + ...overrides, + } as OcxConfig; +} + +function saveTestCredentials(): void { + for (const id of ["a", "b"]) { + saveCodexAccountCredential(id, { + accessToken: `access-${id}`, + refreshToken: `refresh-${id}`, + expiresAt: Date.now() + 10 * 60_000, + chatgptAccountId: `acct-${id}`, + }); + } +} + +function responsesRequest(serverUrl: string, thread = THREAD): Promise { + return ORIGINAL_FETCH(new URL("/v1/responses", serverUrl), { + method: "POST", + headers: { "content-type": "application/json", "x-codex-parent-thread-id": thread }, + body: JSON.stringify({ model: "gpt-5.6-sol", input: "hello", stream: false }), + }); +} + +function compactRequest(serverUrl: string, thread = THREAD): Promise { + return ORIGINAL_FETCH(new URL("/v1/responses/compact", serverUrl), { + method: "POST", + headers: { "content-type": "application/json", "x-codex-parent-thread-id": thread }, + body: JSON.stringify({ model: "gpt-5.6-sol", input: "compact this conversation" }), + }); +} + +function expectRoutingStateUnchanged(config: OcxConfig, hostLedgerKey: string, expectedFailures: number): void { + expect(getCodexUpstreamHealth("a")).toBeNull(); + expect(isCodexAccountSoftAvoided("a")).toBe(false); + expect(getEffectiveActiveCodexAccountId(config)).toBe("a"); + expect(resolveCodexAccountForThread(THREAD, config)).toBe("a"); + expect(getHostConnectHealth(hostLedgerKey)?.consecutiveFailures).toBe(expectedFailures); +} + +describe("issue #914: pre-connection reachability failures are account-neutral", () => { + beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + testDir = mkdtempSync(join(tmpdir(), "ocx-issue-914-")); + process.env.OPENCODEX_HOME = testDir; + isolatedCodexHome = installIsolatedCodexHome("ocx-issue-914-codex-"); + clearThreadAccountMap(); + clearCodexUpstreamHealth(); + clearAccountQuota(); + clearAccountNeedsReauth("a"); + clearAccountNeedsReauth("b"); + saveTestCredentials(); + updateAccountQuota("a", 10); + updateAccountQuota("b", 20); + }); + + afterEach(() => { + globalThis.fetch = ORIGINAL_FETCH; + clearAccountQuota(); + clearCodexUpstreamHealth(); + clearThreadAccountMap(); + clearAccountNeedsReauth("a"); + clearAccountNeedsReauth("b"); + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + isolatedCodexHome?.restore(); + isolatedCodexHome = null; + if (testDir) rmSync(testDir, { recursive: true, force: true }); + }); + + test("regular passthrough: three concurrent DNS failures return 502 but leave routing state unchanged", async () => { + const config = makePoolConfig(); + saveConfig(config); + stubPoolUpstream(); + const server = startServer(0); + try { + const concurrent = await Promise.all([ + responsesRequest(server.url.toString()), + responsesRequest(server.url.toString()), + responsesRequest(server.url.toString()), + ]); + for (const res of concurrent) expect(res.status).toBe(502); + expect((await responsesRequest(server.url.toString())).status).toBe(502); + + const key = hostConnectHealthKey("openai", "chatgpt.com"); + expectRoutingStateUnchanged(config, key, 4); + // The client-visible failure keeps the existing wording. + const body = await concurrent[0]!.text(); + expect(body).toContain("Provider unreachable"); + + // Recovery: the same thread stays on account A and succeeds. + outage = false; + const recovered = await responsesRequest(server.url.toString()); + expect(recovered.status).toBe(200); + expect(await recovered.text()).toContain("served-by-a"); + expect(getEffectiveActiveCodexAccountId(config)).toBe("a"); + expect(getCodexUpstreamHealth("a")).toBeNull(); + } finally { + await server.stop(true); + } + }); + + test("compact path: DNS failures do not rotate the account", async () => { + const config = makePoolConfig(); + saveConfig(config); + stubPoolUpstream(); + const server = startServer(0); + try { + const concurrent = await Promise.all([ + compactRequest(server.url.toString()), + compactRequest(server.url.toString()), + compactRequest(server.url.toString()), + ]); + for (const res of concurrent) expect(res.status).toBe(502); + + const key = hostConnectHealthKey("openai", "chatgpt.com"); + expectRoutingStateUnchanged(config, key, 3); + + outage = false; + const recovered = await compactRequest(server.url.toString()); + expect(recovered.status).toBe(200); + expect(getEffectiveActiveCodexAccountId(config)).toBe("a"); + } finally { + await server.stop(true); + } + }); + + test("real Bun pre-connection rejection (TCP refusal) is account-neutral", async () => { + const config = makePoolConfig(); + saveConfig(config); + // Keep the canonical provider config (pool mode is canonical-only) and remap + // only the socket destination: the proxy's fetch goes to a dead local port, + // so Bun itself performs the real pre-connect rejection (FailedToOpenSocket). + globalThis.fetch = (async (input, init) => { + const requestUrl = input instanceof Request + ? input.url + : input instanceof URL + ? input.toString() + : String(input); + if (requestUrl.startsWith(CODEX_BASE)) { + return ORIGINAL_FETCH(requestUrl.replace("https://chatgpt.com", "http://127.0.0.1:1"), init); + } + return ORIGINAL_FETCH(input, init); + }) as typeof fetch; + const server = startServer(0); + try { + const responses = await Promise.all([ + responsesRequest(server.url.toString()), + responsesRequest(server.url.toString()), + responsesRequest(server.url.toString()), + ]); + for (const res of responses) expect(res.status).toBe(502); + + const key = hostConnectHealthKey("openai", "chatgpt.com"); + expectRoutingStateUnchanged(config, key, 3); + expect(getHostConnectHealth(key)?.lastFailureCode).toMatch(/FailedToOpenSocket|ConnectionRefused/); + } finally { + await server.stop(true); + } + }); + + test("read-then-close resets stay account-attributed (#914 regression guard)", async () => { + const config = makePoolConfig(); + saveConfig(config); + // A server that reads the Authorization header and closes the socket rejects + // with ECONNRESET — the credential was seen, so this MUST keep failing over + // (the 2026-07-22 decision recorded in devlog/_fin/260722_issue_bug_sweep). + globalThis.fetch = (async (input, init) => { + const requestUrl = input instanceof Request + ? input.url + : input instanceof URL + ? input.toString() + : String(input); + if (requestUrl.startsWith(CODEX_BASE)) { + throw Object.assign(new Error("The socket connection was closed unexpectedly"), { + code: "ECONNRESET", + errno: 0, + }); + } + return ORIGINAL_FETCH(input, init); + }) as typeof fetch; + const server = startServer(0); + try { + const responses = await Promise.all([ + responsesRequest(server.url.toString(), "thread-reset"), + responsesRequest(server.url.toString(), "thread-reset"), + responsesRequest(server.url.toString(), "thread-reset"), + ]); + for (const res of responses) expect(res.status).toBe(502); + + expect(getCodexUpstreamHealth("a")).toMatchObject({ consecutiveFailures: 3, lastFailureStatus: 0 }); + expect(isCodexAccountSoftAvoided("a")).toBe(true); + expect(resolveCodexAccountForThread("thread-reset", config)).toBe("b"); + // Pre-connect ledger is untouched: ECONNRESET is not a reachability code. + expect(getHostConnectHealth(hostConnectHealthKey("openai", "chatgpt.com"))).toBeNull(); + } finally { + await server.stop(true); + } + }); +}); diff --git a/tests/server-auth.test.ts b/tests/server-auth.test.ts index 60d6736187..6e15c9389e 100644 --- a/tests/server-auth.test.ts +++ b/tests/server-auth.test.ts @@ -14,6 +14,8 @@ import { clearCodexUpstreamHealth, clearThreadAccountMap, getCodexUpstreamHealth, + getHostConnectHealth, + hostConnectHealthKey, recordCodexUpstreamOutcome, } from "../src/codex/routing"; import { loadConfig, saveConfig } from "../src/config"; @@ -2634,7 +2636,7 @@ describe("server local API auth", () => { } }, { timeout: 30_000 }); - test("passthrough connect failure records selected pool account health", async () => { + test("passthrough pre-connect reachability failure is account-neutral (#914)", async () => { if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); mkdirSync(TEST_DIR, { recursive: true }); process.env.OPENCODEX_HOME = TEST_DIR; @@ -2678,9 +2680,12 @@ describe("server local API auth", () => { }); expect(response.status).toBe(502); - expect(getCodexUpstreamHealth("pool-a")).toMatchObject({ + // A real Bun pre-connect rejection (FailedToOpenSocket/ConnectionRefused) + // belongs to the (provider, host) pair, never to the credential: the + // account streak, soft-avoid, and affinity stay untouched (#914). + expect(getCodexUpstreamHealth("pool-a")).toBeNull(); + expect(getHostConnectHealth(hostConnectHealthKey("openai", "chatgpt.com"))).toMatchObject({ consecutiveFailures: 1, - lastFailureStatus: 0, }); } 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..acbaf6e5ce --- /dev/null +++ b/tests/upstream-reachability.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, test } from "bun:test"; +import { + classifyTransportFailureKind, + isPreConnectReachabilityError, + MAX_REACHABILITY_CAUSE_DEPTH, + transportErrorCode, + transportFailureHost, +} from "../src/lib/upstream-reachability"; + +function errorWithCode(code: string | undefined, message: string, cause?: unknown): Error { + const err = new Error(message) as Error & { code?: string; cause?: unknown }; + if (code !== undefined) err.code = code; + if (cause !== undefined) err.cause = cause; + return err; +} + +// Shapes observed on Bun 1.3.14 (probe run 2026-08-04): DNS failure and TCP +// refusal both surface as ConnectionRefused / FailedToOpenSocket, errno 0, no +// cause, alternating between labels as the DNS cache is evicted. +describe("isPreConnectReachabilityError", () => { + test.each([ + ["Bun DNS failure", errorWithCode("ConnectionRefused", "Unable to connect. Is the computer able to access the url?")], + ["Bun TCP refusal", errorWithCode("FailedToOpenSocket", "Was there a typo in the url or port?")], + ["Bun alternated label", errorWithCode("FailedToOpenSocket", "Unable to connect. Is the computer able to access the url?")], + ["Node DNS failure", errorWithCode("ENOTFOUND", "getaddrinfo ENOTFOUND api.example.com")], + ["Node resolver transient", errorWithCode("EAI_AGAIN", "getaddrinfo EAI_AGAIN api.example.com")], + ["Node TCP refusal", errorWithCode("ECONNREFUSED", "connect ECONNREFUSED 127.0.0.1:443")], + ["Node network unreachable", errorWithCode("ENETUNREACH", "connect ENETUNREACH 10.0.0.1:443")], + ["Node network down", errorWithCode("ENETDOWN", "connect ENETDOWN")], + ["Node host unreachable", errorWithCode("EHOSTUNREACH", "connect EHOSTUNREACH")], + ])("classifies the %s shape as pre-connect", (_label, err) => { + expect(isPreConnectReachabilityError(err)).toBe(true); + }); + + test.each([ + ["established-socket reset", errorWithCode("ECONNRESET", "The socket connection was closed unexpectedly")], + ["pipeline close", errorWithCode("EPIPE", "broken pipe")], + ["TLS certificate mismatch", errorWithCode("ERR_TLS_CERT_ALTNAME_INVALID", 'ERR_TLS_CERT_ALTNAME_INVALID fetching "https://api.example.com"')], + ["unknown code", errorWithCode("SomethingElse", "boom")], + ["no code at all", new Error("boom")], + ["non-Error rejection", "socket hang up"], + ["null rejection", null], + ])("does not classify the %s shape as pre-connect", (_label, err) => { + expect(isPreConnectReachabilityError(err)).toBe(false); + }); + + test("message-only matches are rejected even when the text quotes the code", () => { + // The reporter-required negative: a message that merely contains the code + // (e.g. an upstream echoing it back) must not fire the classifier. + expect(isPreConnectReachabilityError(new Error("getaddrinfo ENOTFOUND api.example.com"))).toBe(false); + expect(isPreConnectReachabilityError(new Error("connect ECONNREFUSED 127.0.0.1:443"))).toBe(false); + }); + + test("inspects a bounded cause chain", () => { + const inner = errorWithCode("ENOTFOUND", "getaddrinfo ENOTFOUND"); + const mid = errorWithCode(undefined, "wrapped", inner); + const outer = errorWithCode(undefined, "wrapped again", mid); + expect(isPreConnectReachabilityError(outer)).toBe(true); + }); + + test("does not recurse past the depth bound", () => { + let current = errorWithCode("ENOTFOUND", "deep"); + for (let i = 0; i < MAX_REACHABILITY_CAUSE_DEPTH; i++) { + current = errorWithCode(undefined, `layer ${i}`, current); + } + expect(isPreConnectReachabilityError(current)).toBe(false); + }); + + test("stops at a non-Error cause instead of descending through it", () => { + const wrapped = errorWithCode(undefined, "wrapped", "string cause"); + expect(isPreConnectReachabilityError(wrapped)).toBe(false); + }); + + test("a self-referential cause does not loop forever", () => { + const cyclic = new Error("cycle") as Error & { cause?: unknown }; + cyclic.cause = cyclic; + expect(isPreConnectReachabilityError(cyclic)).toBe(false); + }); +}); + +describe("classifyTransportFailureKind", () => { + test("a timeout keeps its existing identity", () => { + const timeout = Object.assign(new Error("Timeout elapsed"), { name: "TimeoutError" }); + expect(classifyTransportFailureKind(timeout)).toBe("timeout"); + }); + + test("proven pre-connect shapes become neutral", () => { + expect(classifyTransportFailureKind(errorWithCode("ConnectionRefused", "unreachable"))).toBe("connect_neutral"); + expect(classifyTransportFailureKind(errorWithCode("ENOTFOUND", "unreachable"))).toBe("connect_neutral"); + }); + + test("everything else keeps the existing account-attributed connect_error", () => { + expect(classifyTransportFailureKind(errorWithCode("ECONNRESET", "reset"))).toBe("connect_error"); + expect(classifyTransportFailureKind(errorWithCode("EPIPE", "pipe"))).toBe("connect_error"); + expect(classifyTransportFailureKind(new Error("getaddrinfo ENOTFOUND"))).toBe("connect_error"); + expect(classifyTransportFailureKind("string rejection")).toBe("connect_error"); + expect(classifyTransportFailureKind(undefined)).toBe("connect_error"); + }); +}); + +describe("transport error helpers", () => { + test("transportErrorCode returns only a stable string code", () => { + expect(transportErrorCode(errorWithCode("ConnectionRefused", "x"))).toBe("ConnectionRefused"); + expect(transportErrorCode(errorWithCode("", "x"))).toBeUndefined(); + expect(transportErrorCode(new Error("x"))).toBeUndefined(); + expect(transportErrorCode("not an error")).toBeUndefined(); + }); + + test("transportFailureHost extracts the host or returns null", () => { + expect(transportFailureHost("https://api.chatgpt.com/backend-api/x")).toBe("api.chatgpt.com"); + expect(transportFailureHost("http://127.0.0.1:1/x")).toBe("127.0.0.1:1"); + expect(transportFailureHost("not a url")).toBeNull(); + }); +}); From 80cd0cee43e12d2ef6b1dc32f42c2f9b77e6b0c0 Mon Sep 17 00:00:00 2001 From: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com> Date: Tue, 4 Aug 2026 12:26:46 +0800 Subject: [PATCH 2/5] fix(codex): address #914 review blockers (5xx evidence, sidecar redirects) Keep prior transient 5xx statuses attached when fetchWithTransientRetry rejects, so a mixed 503 -> rejection stays account-attributed instead of being downgraded to the pre-connection neutral class (review blocker 1). Extend redirect:manual to the search/images/live/web-search/vision sidecar sends so a credential-bearing 3xx to a dead host is relayed as the neutral 3xx class (review blocker 2). Exact-account sidecar recorders now forward host/lastFailureCode into the (provider, host) ledger (Codex review P2). Tests: evidence-wrapper classification units, mixed-503 and redirect activation e2e, exact-account host-ledger coverage. --- src/lib/upstream-reachability.ts | 14 ++- src/lib/upstream-retry.ts | 31 ++++++- src/providers/openai-sidecar.ts | 7 +- src/server/images.ts | 2 + src/server/live.ts | 2 + src/server/search.ts | 2 + src/vision/describe.ts | 2 + src/web-search/executor.ts | 2 + tests/issue-914-transport-attribution.test.ts | 86 +++++++++++++++++++ tests/server-search.test.ts | 42 +++++++++ tests/upstream-reachability.test.ts | 34 ++++++++ 11 files changed, 220 insertions(+), 4 deletions(-) diff --git a/src/lib/upstream-reachability.ts b/src/lib/upstream-reachability.ts index e66e9564ec..200f3df3e3 100644 --- a/src/lib/upstream-reachability.ts +++ b/src/lib/upstream-reachability.ts @@ -21,6 +21,8 @@ * MUST stay a leaf module: imports nothing from server.ts or adapters. */ +import { TransientRetryEvidenceError } from "./upstream-retry"; + export const PRE_CONNECT_REACHABILITY_CODES = new Set([ // Bun: DNS failure and TCP refusal share this class. "ConnectionRefused", @@ -64,8 +66,16 @@ export type TransportFailureKind = "timeout" | "connect_neutral" | "connect_erro * account-attributed behavior. */ export function classifyTransportFailureKind(err: unknown): TransportFailureKind { - if (err instanceof Error && err.name === "TimeoutError") return "timeout"; - if (isPreConnectReachabilityError(err)) return "connect_neutral"; + const evidence = err instanceof TransientRetryEvidenceError ? 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) 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) return "connect_error"; + return "connect_neutral"; + } return "connect_error"; } diff --git a/src/lib/upstream-retry.ts b/src/lib/upstream-retry.ts index f767f045ee..6cdfb80ecb 100644 --- a/src/lib/upstream-retry.ts +++ b/src/lib/upstream-retry.ts @@ -148,6 +148,27 @@ export interface TransientRetryOptions extends ResetRetryOptions { export type UpstreamSendRecovery = "connection-reset" | "transient-5xx"; type ReplayableFetch = (recovery?: UpstreamSendRecovery) => Promise; +/** + * Rejection thrown by {@link fetchWithTransientRetry} when the final attempt + * rejects after earlier attempts already returned transient 5xx responses. + * + * A transient 5xx 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 -> 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. + */ +export class TransientRetryEvidenceError extends Error { + constructor( + public readonly transientStatuses: readonly number[], + cause: unknown, + ) { + const detail = cause instanceof Error ? cause.message : String(cause); + super(`upstream fetch failed after transient 5xx response(s): ${detail}`, { cause }); + this.name = "TransientRetryEvidenceError"; + } +} + /** * Opt out of Bun's keep-alive pool after a connection-reset retry. * @@ -216,6 +237,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++) { @@ -233,7 +255,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 TransientRetryEvidenceError(transientStatuses, err); + } } return res; } diff --git a/src/providers/openai-sidecar.ts b/src/providers/openai-sidecar.ts index 14b1704d7d..ae1bb2579f 100644 --- a/src/providers/openai-sidecar.ts +++ b/src/providers/openai-sidecar.ts @@ -124,7 +124,7 @@ export async function resolveFirstUsableOpenAiSidecar( ...candidate, authContext, headers: headersForCodexAuthContext(incomingHeaders, authContext), - recordOutcome: (outcome: CodexUpstreamOutcome) => recordCodexUpstreamOutcome( + recordOutcome: (outcome: CodexUpstreamOutcome, meta: SidecarOutcomeMeta = {}) => recordCodexUpstreamOutcome( config, authContext.accountId, outcome, @@ -134,6 +134,11 @@ export async function resolveFirstUsableOpenAiSidecar( probeLeaseId: authContext.probeLeaseId, probeQuotaScope: authContext.probeQuotaScope, writerGeneration: authContext.writerGeneration, + // Exact-account sends must feed the (provider, host) ledger exactly + // like pool sends do, or fixed-account DNS/TCP failures undercount + // hostConnectHealth (Codex review P2). + ...(meta.host ? { hostKey: hostConnectHealthKey(candidate.providerName, meta.host) } : {}), + ...(meta.lastFailureCode ? { lastFailureCode: meta.lastFailureCode } : {}), }, ), }; diff --git a/src/server/images.ts b/src/server/images.ts index c6b2646e65..10a4b379ef 100644 --- a/src/server/images.ts +++ b/src/server/images.ts @@ -454,6 +454,8 @@ export async function handleImages( headers, body: JSON.stringify(body), signal: linkedSignal.signal, + // #914: never follow a redirect into a dead-host rejection after the credential was seen. + redirect: "manual", }); // Buffer rather than stream: the payload is one JSON document (base64 image, typically a few // MB), and buffering keeps the timeout window covering the whole exchange. Cap the size to diff --git a/src/server/live.ts b/src/server/live.ts index 072122d573..f9dbf7d0d1 100644 --- a/src/server/live.ts +++ b/src/server/live.ts @@ -561,6 +561,8 @@ export async function handleLive( headers, body: outboundBody, signal: linkedSignal.signal, + // #914: never follow a redirect into a dead-host rejection after the credential was seen. + redirect: "manual", }); // Record every completed upstream response before body size handling so account health / // cooldown still updates when we reject an oversized payload. diff --git a/src/server/search.ts b/src/server/search.ts index 8bb0f19a3b..9bf3584927 100644 --- a/src/server/search.ts +++ b/src/server/search.ts @@ -154,6 +154,8 @@ export async function handleSearch( headers, body: JSON.stringify(relayBody), signal: linkedSignal.signal, + // #914: never follow a redirect into a dead-host rejection after the credential was seen. + redirect: "manual", }); const payload = await upstreamResponse.arrayBuffer(); if (payload.byteLength > SEARCH_RESPONSE_MAX_BYTES) { diff --git a/src/vision/describe.ts b/src/vision/describe.ts index a9a5230bd9..886fdd95e4 100644 --- a/src/vision/describe.ts +++ b/src/vision/describe.ts @@ -98,6 +98,8 @@ export async function describeImage( headers, body: JSON.stringify(body), signal: linkedSignal.signal, + // #914: never follow a redirect into a dead-host rejection after the credential was seen. + redirect: "manual", }), { abortSignal: linkedSignal.signal, label: "vision-sidecar" }, ); diff --git a/src/web-search/executor.ts b/src/web-search/executor.ts index 29c5debd23..431999ad85 100644 --- a/src/web-search/executor.ts +++ b/src/web-search/executor.ts @@ -88,6 +88,8 @@ export async function runWebSearch( headers, body: JSON.stringify(body), signal: linkedSignal.signal, + // #914: never follow a redirect into a dead-host rejection after the credential was seen. + redirect: "manual", }), { abortSignal: linkedSignal.signal, label: "web-search-sidecar" }, ); diff --git a/tests/issue-914-transport-attribution.test.ts b/tests/issue-914-transport-attribution.test.ts index 5827606c8f..6f886138cc 100644 --- a/tests/issue-914-transport-attribution.test.ts +++ b/tests/issue-914-transport-attribution.test.ts @@ -283,4 +283,90 @@ describe("issue #914: pre-connection reachability failures are account-neutral", await server.stop(true); } }); + + test("mixed transient 5xx then rejection stays account-attributed (#914 review)", async () => { + const config = makePoolConfig(); + saveConfig(config); + let upstreamCalls = 0; + globalThis.fetch = (async (input, init) => { + const requestUrl = input instanceof Request + ? input.url + : input instanceof URL + ? input.toString() + : String(input); + const url = new URL(requestUrl); + if (url.hostname === "chatgpt.com" && url.pathname.startsWith("/backend-api/codex")) { + upstreamCalls += 1; + if (upstreamCalls === 1) { + return new Response("Service Unavailable", { status: 503, headers: { "content-type": "text/plain" } }); + } + throw Object.assign(new Error("Unable to connect. Is the computer able to access the url?"), { + code: "ConnectionRefused", + errno: 0, + }); + } + return ORIGINAL_FETCH(input, init); + }) as typeof fetch; + const server = startServer(0); + try { + const res = await responsesRequest(server.url.toString(), "thread-mixed-503"); + expect(res.status).toBe(502); + + // The upstream already answered 503 before the final rejection, so the + // failure is account evidence — the pre-connection ledger must NOT swallow it. + expect(getCodexUpstreamHealth("a")).toMatchObject({ consecutiveFailures: 1, lastFailureStatus: 0 }); + expect(isCodexAccountSoftAvoided("a")).toBe(false); + expect(getHostConnectHealth(hostConnectHealthKey("openai", "chatgpt.com"))).toBeNull(); + } finally { + await server.stop(true); + } + }); + + test("pool sends relay 3xx instead of following a redirect into a dead host (#914 review)", async () => { + const config = makePoolConfig(); + saveConfig(config); + globalThis.fetch = (async (input, init) => { + const requestUrl = input instanceof Request + ? input.url + : input instanceof URL + ? input.toString() + : String(input); + const url = new URL(requestUrl); + if (url.hostname === "chatgpt.com" && url.pathname.startsWith("/backend-api/codex")) { + if (init?.redirect !== "manual") { + // What default-follow fetch would do: chase the 307 into a dead host and + // reject with a pre-connection shape AFTER the credential was seen. + throw Object.assign(new Error("Unable to connect. Is the computer able to access the url?"), { + code: "ConnectionRefused", + errno: 0, + }); + } + return new Response(JSON.stringify({ location: "https://dead.invalid/x" }), { + status: 307, + headers: { "content-type": "application/json", location: "https://dead.invalid/x" }, + }); + } + return ORIGINAL_FETCH(input, init); + }) as typeof fetch; + const server = startServer(0); + try { + // The relayed 307 carries a Location header, so the test client must not + // follow it (default-follow would chase it into the dead host). + const res = await ORIGINAL_FETCH(new URL("/v1/responses", server.url), { + method: "POST", + headers: { "content-type": "application/json", "x-codex-parent-thread-id": "thread-redirect" }, + body: JSON.stringify({ model: "gpt-5.6-sol", input: "hello", stream: false }), + redirect: "manual", + }); + expect(res.status).toBe(307); + + // 3xx is the neutral class: no account evidence, no host evidence. + expect(getCodexUpstreamHealth("a")).toBeNull(); + expect(isCodexAccountSoftAvoided("a")).toBe(false); + expect(getEffectiveActiveCodexAccountId(config)).toBe("a"); + expect(getHostConnectHealth(hostConnectHealthKey("openai", "chatgpt.com"))).toBeNull(); + } finally { + await server.stop(true); + } + }); }); diff --git a/tests/server-search.test.ts b/tests/server-search.test.ts index 2a1e2f680c..1165c4c37e 100644 --- a/tests/server-search.test.ts +++ b/tests/server-search.test.ts @@ -13,6 +13,8 @@ import { clearCodexUpstreamHealth, clearThreadAccountMap, getCodexUpstreamHealth, + getHostConnectHealth, + hostConnectHealthKey, recordCodexUpstreamOutcome, } from "../src/codex/routing"; import { loadConfig, saveConfig } from "../src/config"; @@ -352,6 +354,46 @@ test("an exact search account needing reauthentication fails closed with an acti } }); +test("an exact-account DNS/TCP failure feeds the (provider, host) ledger (#914 review P2)", async () => { + const config = exactSearchConfig(); + saveConfig(config); + saveExactSearchCredentials(); + globalThis.fetch = ((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" && url.pathname.startsWith("/backend-api/codex")) { + throw Object.assign(new Error("Unable to connect. Is the computer able to access the url?"), { + code: "ConnectionRefused", + errno: 0, + }); + } + return originalFetch(input, init); + }) as typeof fetch; + + const server = startServer(0); + try { + const response = await fetch(new URL("/v1/alpha/search", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ id: "search-session", model: "side/gpt-test" }), + }); + expect(response.status).toBe(502); + + // The fixed account keeps no streak, and the active Pool account is untouched. + expect(getCodexUpstreamHealth("pool-a")).toBeNull(); + expect(getCodexUpstreamHealth("pool-b")).toBeNull(); + expect(loadConfig().activeCodexAccountId).toBe("pool-b"); + // But the (provider, host) ledger must see the failure exactly like pool sends. + const hostKey = hostConnectHealthKey("openai", "chatgpt.com"); + expect(getHostConnectHealth(hostKey)).toMatchObject({ + consecutiveFailures: 1, + lastFailureCode: "ConnectionRefused", + }); + } finally { + await server.stop(true); + } +}); + test("zstd-compressed search request bodies are decoded before the relay", async () => { const captured: CapturedRequest[] = []; const upstream = fakeSearchUpstream(captured); diff --git a/tests/upstream-reachability.test.ts b/tests/upstream-reachability.test.ts index acbaf6e5ce..699f35725c 100644 --- a/tests/upstream-reachability.test.ts +++ b/tests/upstream-reachability.test.ts @@ -6,6 +6,7 @@ import { transportErrorCode, transportFailureHost, } from "../src/lib/upstream-reachability"; +import { TransientRetryEvidenceError } from "../src/lib/upstream-retry"; function errorWithCode(code: string | undefined, message: string, cause?: unknown): Error { const err = new Error(message) as Error & { code?: string; cause?: unknown }; @@ -96,6 +97,39 @@ describe("classifyTransportFailureKind", () => { expect(classifyTransportFailureKind("string rejection")).toBe("connect_error"); expect(classifyTransportFailureKind(undefined)).toBe("connect_error"); }); + + test("a rejection after a transient 5xx stays account-attributed, never neutral", () => { + // Mixed 5xx -> rejection: the upstream already answered 503 before the final + // attempt rejected, so the host and credential path were reached. The + // evidence wrapper must keep the failure out of the pre-connect class. + const wrapped = new TransientRetryEvidenceError( + [503], + errorWithCode("ConnectionRefused", "Unable to connect. Is the computer able to access the url?"), + ); + expect(classifyTransportFailureKind(wrapped)).toBe("connect_error"); + }); + + test("a timeout after a transient 5xx keeps the timeout identity", () => { + const rejection = Object.assign(new Error("Timeout elapsed"), { name: "TimeoutError" }); + const wrapped = new TransientRetryEvidenceError([503], rejection); + expect(classifyTransportFailureKind(wrapped)).toBe("timeout"); + }); + + test("an evidence wrapper without prior statuses classifies by its cause", () => { + const wrapped = new TransientRetryEvidenceError( + [], + errorWithCode("ConnectionRefused", "Unable to connect. Is the computer able to access the url?"), + ); + expect(classifyTransportFailureKind(wrapped)).toBe("connect_neutral"); + }); + + test("the evidence wrapper preserves the final rejection and its statuses", () => { + const rejection = errorWithCode("ConnectionRefused", "Unable to connect"); + const wrapped = new TransientRetryEvidenceError([503, 502], rejection); + expect(wrapped.cause).toBe(rejection); + expect(wrapped.transientStatuses).toEqual([503, 502]); + expect(transportErrorCode(wrapped)).toBeUndefined(); + }); }); describe("transport error helpers", () => { From 71bd8210a42afcc32a4b5decfc25012051433f12 Mon Sep 17 00:00:00 2001 From: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com> Date: Tue, 4 Aug 2026 13:42:44 +0800 Subject: [PATCH 3/5] fix(codex): manual redirects for all credential-bearing forward sends (#914) CodeRabbit review (commit 80cd0cee): direct-mode /v1/responses and /v1/responses/compact still default-followed redirects because the flag was gated on usesCodexForwardPoolAuth, which is false for direct auth. Gate manual redirects on provider.authMode === "forward" instead, so pool, direct, and exact sends all relay a 3xx as-is. Relay fixes: search and images copy the upstream Location header, and compact passes Location through, so a relayed 3xx stays followable. Docs (en/ja/ko/ru/zh-cn) now state that pre-connection DNS/TCP failures are tracked at provider-host scope and never affect account health, cooldowns, affinity, active-account selection, or Pool routing. Tests: direct-forward 307 activation for Responses and Compact, search 307 + Location preservation with no follow, and a Location assertion on the pool 307 e2e. --- .../ja/reference/configuration/providers.md | 2 +- .../ko/reference/configuration/providers.md | 2 +- .../docs/reference/configuration/providers.md | 2 +- .../ru/reference/configuration/providers.md | 2 +- .../reference/configuration/providers.md | 2 +- src/server/images.ts | 2 + src/server/responses/compact.ts | 6 +- src/server/responses/core.ts | 11 +- src/server/search.ts | 4 + tests/issue-914-transport-attribution.test.ts | 117 ++++++++++++++---- tests/server-search.test.ts | 44 +++++++ 11 files changed, 160 insertions(+), 34 deletions(-) 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 184b20c8d7..b04caf8529 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` を無効に設定します。接続前のDNS/TCP到達不能障害はアカウント中立で、カウントされません。 | +| `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 3ad3abe10d..7939aa72ef 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`으로 두면 비활성화됩니다. 연결 전 DNS/TCP 도달 불가 실패는 계정 중립이며 집계되지 않습니다. | +| `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 b4834d1332..c9cc39e3a9 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. Proven pre-connection DNS/TCP reachability failures are account-neutral and never count. | +| `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 b9b16e548f..543f15ccc9 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` отключает эту логику. Доказанные ошибки доступности DNS/TCP до соединения нейтральны для аккаунта и не учитываются. | +| `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 af7534842d..489c78b778 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` 可禁用。连接前的 DNS/TCP 不可达故障与账户无关，不计入次数。 | +| `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/server/images.ts b/src/server/images.ts index 10a4b379ef..48fbb6ff2f 100644 --- a/src/server/images.ts +++ b/src/server/images.ts @@ -468,6 +468,8 @@ export async function handleImages( const relayHeaders: Record = {}; const contentType = upstreamResponse.headers.get("content-type"); if (contentType) relayHeaders["content-type"] = contentType; + const location = upstreamResponse.headers.get("location"); + if (location) relayHeaders["location"] = location; return new Response(payload, { status: upstreamResponse.status, headers: relayHeaders }); } catch (err) { // Client cancel first: it aborts the linked signal too, and must not be logged as an diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index a6574b44e3..72768c2b7c 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -193,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 review). + "location", ]; function compactResponseHeaders(upstream: Response): Headers { @@ -401,7 +403,9 @@ export async function handleResponsesCompact( connectMs, false, providerFetch(sendProvider), - usesCodexForwardPoolAuth(authCtx, route.provider), + // Every credential-bearing forward send gets manual redirects, not only + // pool sends: direct mode carries the caller's credential too (#914). + sendProvider.authMode === "forward", ); return recovery === "single" ? doFetch() diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 3f6ecea5a1..a8f658d8d3 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -1739,10 +1739,11 @@ async function handleResponsesInner( return formatErrorResponse(502, "upstream_error", msg); }; try { - // Manual redirect on pool sends: a 3xx surfaces as a Response instead of a - // followed redirect, so a server that redirects to a dead host can never - // masquerade as a pre-connection failure (recorded rejection class #914). - const poolUpstreamSend = usesCodexForwardPoolAuth(authCtx, route.provider); + // Manual redirect on every credential-bearing forward send (pool AND + // direct): a 3xx surfaces as a Response instead of a followed redirect, + // so a server that redirects to a dead host can never masquerade as a + // pre-connection failure after the credential was seen (#914). + const forwardCredentialedSend = route.provider.authMode === "forward"; // Transient-5xx pre-stream retry (devlog/_plan/260716_claudecode_hardening/010): // the ChatGPT backend emits transient 502/520s that an immediate retry absorbs. // Body is a replayable string; nothing has streamed to the client yet. @@ -1754,7 +1755,7 @@ async function handleResponsesInner( headers: request.headers, body: request.body, }, recovery), upstream.signal, connectMs, parsed.stream, providerFetch(route.provider), - poolUpstreamSend); + forwardCredentialedSend); }, { abortSignal: upstream.signal, label: safeHostLabel(request.url) }, ); diff --git a/src/server/search.ts b/src/server/search.ts index 9bf3584927..c2644bb1c4 100644 --- a/src/server/search.ts +++ b/src/server/search.ts @@ -165,6 +165,10 @@ export async function handleSearch( const relayHeaders: Record = {}; const contentType = upstreamResponse.headers.get("content-type"); if (contentType) relayHeaders["content-type"] = contentType; + // A relayed 3xx keeps its Location so the client can follow it; the proxy + // must not strip it while also refusing to follow it (#914 review). + const location = upstreamResponse.headers.get("location"); + if (location) relayHeaders["location"] = location; return new Response(payload, { status: upstreamResponse.status, headers: relayHeaders }); } catch (err) { if (req.signal.aborted) { diff --git a/tests/issue-914-transport-attribution.test.ts b/tests/issue-914-transport-attribution.test.ts index 6f886138cc..3f89db655c 100644 --- a/tests/issue-914-transport-attribution.test.ts +++ b/tests/issue-914-transport-attribution.test.ts @@ -89,6 +89,24 @@ function makePoolConfig(overrides: Partial = {}): OcxConfig { } as OcxConfig; } +function makeDirectConfig(overrides: Partial = {}): OcxConfig { + return { + port: 0, + hostname: "127.0.0.1", + defaultProvider: "openai", + openaiProviderTierVersion: 2, + providers: { + openai: { + adapter: "openai-responses", + baseUrl: CODEX_BASE, + authMode: "forward", + codexAccountMode: "direct", + }, + }, + ...overrides, + } as OcxConfig; +} + function saveTestCredentials(): void { for (const id of ["a", "b"]) { saveCodexAccountCredential(id, { @@ -108,6 +126,48 @@ function responsesRequest(serverUrl: string, thread = THREAD): Promise }); } +// Direct mode carries the CALLER's credential upstream, so the same no-follow +// policy must apply without any pool state. The client keeps manual redirects +// so the relayed 307 is not followed into the dead host by the test itself. +function directForwardRequest(serverUrl: string, path = "/v1/responses"): Promise { + return ORIGINAL_FETCH(new URL(path, serverUrl), { + method: "POST", + headers: { "content-type": "application/json", authorization: "Bearer inbound-token" }, + body: JSON.stringify({ + model: "gpt-5.6-sol", + input: path.endsWith("/compact") ? "compact this conversation" : "hello", + stream: false, + }), + redirect: "manual", + }); +} + +function stubRedirectToDeadHost(): void { + globalThis.fetch = (async (input, init) => { + const requestUrl = input instanceof Request + ? input.url + : input instanceof URL + ? input.toString() + : String(input); + const url = new URL(requestUrl); + if (url.hostname === "chatgpt.com" && url.pathname.startsWith("/backend-api/codex")) { + if (init?.redirect !== "manual") { + // What default-follow fetch would do: chase the 307 into a dead host and + // reject with a pre-connection shape AFTER the credential was seen. + throw Object.assign(new Error("Unable to connect. Is the computer able to access the url?"), { + code: "ConnectionRefused", + errno: 0, + }); + } + return new Response(JSON.stringify({ location: "https://dead.invalid/x" }), { + status: 307, + headers: { "content-type": "application/json", location: "https://dead.invalid/x" }, + }); + } + return ORIGINAL_FETCH(input, init); + }) as typeof fetch; +} + function compactRequest(serverUrl: string, thread = THREAD): Promise { return ORIGINAL_FETCH(new URL("/v1/responses/compact", serverUrl), { method: "POST", @@ -325,29 +385,7 @@ describe("issue #914: pre-connection reachability failures are account-neutral", test("pool sends relay 3xx instead of following a redirect into a dead host (#914 review)", async () => { const config = makePoolConfig(); saveConfig(config); - globalThis.fetch = (async (input, init) => { - const requestUrl = input instanceof Request - ? input.url - : input instanceof URL - ? input.toString() - : String(input); - const url = new URL(requestUrl); - if (url.hostname === "chatgpt.com" && url.pathname.startsWith("/backend-api/codex")) { - if (init?.redirect !== "manual") { - // What default-follow fetch would do: chase the 307 into a dead host and - // reject with a pre-connection shape AFTER the credential was seen. - throw Object.assign(new Error("Unable to connect. Is the computer able to access the url?"), { - code: "ConnectionRefused", - errno: 0, - }); - } - return new Response(JSON.stringify({ location: "https://dead.invalid/x" }), { - status: 307, - headers: { "content-type": "application/json", location: "https://dead.invalid/x" }, - }); - } - return ORIGINAL_FETCH(input, init); - }) as typeof fetch; + stubRedirectToDeadHost(); const server = startServer(0); try { // The relayed 307 carries a Location header, so the test client must not @@ -359,6 +397,7 @@ describe("issue #914: pre-connection reachability failures are account-neutral", redirect: "manual", }); expect(res.status).toBe(307); + expect(res.headers.get("location")).toBe("https://dead.invalid/x"); // 3xx is the neutral class: no account evidence, no host evidence. expect(getCodexUpstreamHealth("a")).toBeNull(); @@ -369,4 +408,36 @@ describe("issue #914: pre-connection reachability failures are account-neutral", await server.stop(true); } }); + + test("direct forward sends relay 3xx instead of following a redirect into a dead host (#914 review)", async () => { + const config = makeDirectConfig(); + saveConfig(config); + stubRedirectToDeadHost(); + const server = startServer(0); + try { + const res = await directForwardRequest(server.url.toString()); + expect(res.status).toBe(307); + expect(res.headers.get("location")).toBe("https://dead.invalid/x"); + // Direct mode has no pool state to disturb, but it must not classify the + // credential-visible 307 as any health evidence either. + expect(getHostConnectHealth(hostConnectHealthKey("openai", "chatgpt.com"))).toBeNull(); + } finally { + await server.stop(true); + } + }); + + test("compact direct forward sends relay 3xx instead of following a redirect into a dead host (#914 review)", async () => { + const config = makeDirectConfig(); + saveConfig(config); + stubRedirectToDeadHost(); + const server = startServer(0); + try { + const res = await directForwardRequest(server.url.toString(), "/v1/responses/compact"); + expect(res.status).toBe(307); + expect(res.headers.get("location")).toBe("https://dead.invalid/x"); + expect(getHostConnectHealth(hostConnectHealthKey("openai", "chatgpt.com"))).toBeNull(); + } finally { + await server.stop(true); + } + }); }); diff --git a/tests/server-search.test.ts b/tests/server-search.test.ts index 1165c4c37e..0601a38cfc 100644 --- a/tests/server-search.test.ts +++ b/tests/server-search.test.ts @@ -394,6 +394,50 @@ test("an exact-account DNS/TCP failure feeds the (provider, host) ledger (#914 r } }); +test("a 307 search upstream is relayed with its Location and never followed (#914 review)", async () => { + const config = exactSearchConfig(); + saveConfig(config); + saveExactSearchCredentials(); + let upstreamCalls = 0; + globalThis.fetch = ((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" && url.pathname.startsWith("/backend-api/codex")) { + upstreamCalls += 1; + if (init?.redirect !== "manual") { + throw Object.assign(new Error("Unable to connect. Is the computer able to access the url?"), { + code: "ConnectionRefused", + errno: 0, + }); + } + return new Response(JSON.stringify({ location: "https://dead.invalid/x" }), { + status: 307, + headers: { "content-type": "application/json", location: "https://dead.invalid/x" }, + }); + } + return originalFetch(input, init); + }) as typeof fetch; + + const server = startServer(0); + try { + const response = await fetch(new URL("/v1/alpha/search", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ id: "search-session", model: "side/gpt-test" }), + redirect: "manual", + }); + expect(response.status).toBe(307); + expect(response.headers.get("location")).toBe("https://dead.invalid/x"); + // The redirect target was never contacted: manual redirects relay the 3xx as-is. + expect(upstreamCalls).toBe(1); + // 3xx stays the neutral class even for an exact-account send. + expect(getCodexUpstreamHealth("pool-a")).toBeNull(); + expect(loadConfig().activeCodexAccountId).toBe("pool-b"); + } finally { + await server.stop(true); + } +}); + test("zstd-compressed search request bodies are decoded before the relay", async () => { const captured: CapturedRequest[] = []; const upstream = fakeSearchUpstream(captured); From 78c824dd7dc5e41d8bee0fbe6ed6610903617ce9 Mon Sep 17 00:00:00 2001 From: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:35:33 +0800 Subject: [PATCH 4/5] fix(codex): carry reset evidence and scope manual redirects (#914) Codex review (commit 71bd8210): - fetchWithResetRetry now attaches credential-visible reset evidence when a retried ECONNRESET attempt is followed by a different terminal rejection, so a mixed reset -> ConnectionRefused stays account-attributed instead of classifying as pre-connection neutral. The evidence wrapper is renamed to UpstreamRetryEvidenceError and carries both transient 5xx statuses and a resetSeen flag; the classifier treats either as connect_error. - Manual redirects are scoped to the ChatGPT forward credential path in images and live relays. Keyed API-key providers keep default redirect following, so their same-origin 307/308 routing still resolves inside the proxy where the API key lives. - Docs (ja/ko/zh-cn) restore the 'proven' qualifier: unconfirmed failures remain account-attributed. Tests: reset-evidence classification units, a mixed reset -> reachability e2e on /v1/responses, and a host-ledger null assertion on the search 307 e2e. --- .../ja/reference/configuration/providers.md | 2 +- .../ko/reference/configuration/providers.md | 2 +- .../reference/configuration/providers.md | 2 +- src/lib/upstream-reachability.ts | 13 +++--- src/lib/upstream-retry.ts | 44 ++++++++++++++----- src/server/images.ts | 7 ++- src/server/live.ts | 7 ++- tests/issue-914-transport-attribution.test.ts | 42 ++++++++++++++++++ tests/server-search.test.ts | 1 + tests/upstream-reachability.test.ts | 30 ++++++++++--- 10 files changed, 120 insertions(+), 30 deletions(-) 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 b04caf8529..cbfbeb3afa 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` を無効に設定します。接続前のDNS/TCP到達不能障害はprovider-host単位で記録され、アカウントの健全性、クールダウン、スレッド/セッションの親和性、アクティブアカウントの選択、Poolルーティングには影響せず、この閾値にもカウントされません。 | +| `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 7939aa72ef..4a7cd42192 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`으로 두면 비활성화됩니다. 연결 전 DNS/TCP 도달 불가 실패는 provider-host 범위로 기록되며 계정 상태, 쿨다운, 스레드/세션 선호도, 활성 계정 선택 또는 Pool 라우팅에 영향을 주지 않고 이 임계값에도 집계되지 않습니다. | +| `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/zh-cn/reference/configuration/providers.md b/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md index 489c78b778..e2fbe27b11 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` 可禁用。连接前的 DNS/TCP 不可达故障按 provider-host 粒度记录，不影响账户健康、冷却、线程/会话亲和性、活动账户选择或 Pool 路由，也不会计入此阈值。 | +| `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/lib/upstream-reachability.ts b/src/lib/upstream-reachability.ts index 200f3df3e3..755ccd45b9 100644 --- a/src/lib/upstream-reachability.ts +++ b/src/lib/upstream-reachability.ts @@ -21,7 +21,7 @@ * MUST stay a leaf module: imports nothing from server.ts or adapters. */ -import { TransientRetryEvidenceError } from "./upstream-retry"; +import { UpstreamRetryEvidenceError } from "./upstream-retry"; export const PRE_CONNECT_REACHABILITY_CODES = new Set([ // Bun: DNS failure and TCP refusal share this class. @@ -66,14 +66,15 @@ export type TransportFailureKind = "timeout" | "connect_neutral" | "connect_erro * account-attributed behavior. */ export function classifyTransportFailureKind(err: unknown): TransportFailureKind { - const evidence = err instanceof TransientRetryEvidenceError ? err : undefined; + 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) 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) return "connect_error"; + // 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"; diff --git a/src/lib/upstream-retry.ts b/src/lib/upstream-retry.ts index 6cdfb80ecb..ed8234d28b 100644 --- a/src/lib/upstream-retry.ts +++ b/src/lib/upstream-retry.ts @@ -149,23 +149,35 @@ export type UpstreamSendRecovery = "connection-reset" | "transient-5xx"; type ReplayableFetch = (recovery?: UpstreamSendRecovery) => Promise; /** - * Rejection thrown by {@link fetchWithTransientRetry} when the final attempt - * rejects after earlier attempts already returned transient 5xx responses. + * 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. * - * A transient 5xx proves the host and credential path were reached, so the + * 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 -> 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. + * 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. */ -export class TransientRetryEvidenceError extends Error { +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); - super(`upstream fetch failed after transient 5xx response(s): ${detail}`, { cause }); - this.name = "TransientRetryEvidenceError"; + 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"; } } @@ -202,12 +214,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})`, @@ -261,7 +283,7 @@ export async function fetchWithTransientRetry( } 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 TransientRetryEvidenceError(transientStatuses, err); + throw new UpstreamRetryEvidenceError(transientStatuses, err); } } return res; diff --git a/src/server/images.ts b/src/server/images.ts index 48fbb6ff2f..cc66ad64dd 100644 --- a/src/server/images.ts +++ b/src/server/images.ts @@ -454,8 +454,11 @@ export async function handleImages( headers, body: JSON.stringify(body), signal: linkedSignal.signal, - // #914: never follow a redirect into a dead-host rejection after the credential was seen. - redirect: "manual", + // #914: never follow a redirect into a dead-host rejection after the + // credential was seen — on the ChatGPT forward path only. Keyed providers + // keep default redirect following: their 307/308 routing must resolve + // inside the proxy, where the API key still lives. + ...(forward ? { redirect: "manual" as const } : {}), }); // Buffer rather than stream: the payload is one JSON document (base64 image, typically a few // MB), and buffering keeps the timeout window covering the whole exchange. Cap the size to diff --git a/src/server/live.ts b/src/server/live.ts index f9dbf7d0d1..cc262e80f2 100644 --- a/src/server/live.ts +++ b/src/server/live.ts @@ -561,8 +561,11 @@ export async function handleLive( headers, body: outboundBody, signal: linkedSignal.signal, - // #914: never follow a redirect into a dead-host rejection after the credential was seen. - redirect: "manual", + // #914: never follow a redirect into a dead-host rejection after the + // credential was seen — on the ChatGPT forward path only. Keyed voice + // providers keep default redirect following (same-origin routing must + // resolve inside the proxy, where the API key still lives). + ...(relay.keyed ? {} : { redirect: "manual" as const }), }); // Record every completed upstream response before body size handling so account health / // cooldown still updates when we reject an oversized payload. diff --git a/tests/issue-914-transport-attribution.test.ts b/tests/issue-914-transport-attribution.test.ts index 3f89db655c..ca99dc7117 100644 --- a/tests/issue-914-transport-attribution.test.ts +++ b/tests/issue-914-transport-attribution.test.ts @@ -382,6 +382,48 @@ describe("issue #914: pre-connection reachability failures are account-neutral", } }); + test("mixed credential-visible reset then reachability rejection stays account-attributed (#914 review)", async () => { + const config = makePoolConfig(); + saveConfig(config); + let upstreamCalls = 0; + globalThis.fetch = (async (input, init) => { + const requestUrl = input instanceof Request + ? input.url + : input instanceof URL + ? input.toString() + : String(input); + const url = new URL(requestUrl); + if (url.hostname === "chatgpt.com" && url.pathname.startsWith("/backend-api/codex")) { + upstreamCalls += 1; + if (upstreamCalls === 1) { + // Read-then-close: the origin saw the credential, then reset the socket. + throw Object.assign(new Error("The socket connection was closed unexpectedly"), { + code: "ECONNRESET", + errno: 0, + }); + } + throw Object.assign(new Error("Unable to connect. Is the computer able to access the url?"), { + code: "ConnectionRefused", + errno: 0, + }); + } + return ORIGINAL_FETCH(input, init); + }) as typeof fetch; + const server = startServer(0); + try { + const res = await responsesRequest(server.url.toString(), "thread-mixed-reset"); + expect(res.status).toBe(502); + + // The reset reached the origin, so the reachability-shaped terminal + // rejection must not erase that account evidence. + expect(getCodexUpstreamHealth("a")).toMatchObject({ consecutiveFailures: 1, lastFailureStatus: 0 }); + expect(isCodexAccountSoftAvoided("a")).toBe(false); + expect(getHostConnectHealth(hostConnectHealthKey("openai", "chatgpt.com"))).toBeNull(); + } finally { + await server.stop(true); + } + }); + test("pool sends relay 3xx instead of following a redirect into a dead host (#914 review)", async () => { const config = makePoolConfig(); saveConfig(config); diff --git a/tests/server-search.test.ts b/tests/server-search.test.ts index 0601a38cfc..65e817244e 100644 --- a/tests/server-search.test.ts +++ b/tests/server-search.test.ts @@ -431,6 +431,7 @@ test("a 307 search upstream is relayed with its Location and never followed (#91 // The redirect target was never contacted: manual redirects relay the 3xx as-is. expect(upstreamCalls).toBe(1); // 3xx stays the neutral class even for an exact-account send. + expect(getHostConnectHealth(hostConnectHealthKey("openai", "chatgpt.com"))).toBeNull(); expect(getCodexUpstreamHealth("pool-a")).toBeNull(); expect(loadConfig().activeCodexAccountId).toBe("pool-b"); } finally { diff --git a/tests/upstream-reachability.test.ts b/tests/upstream-reachability.test.ts index 699f35725c..63e65221f5 100644 --- a/tests/upstream-reachability.test.ts +++ b/tests/upstream-reachability.test.ts @@ -6,7 +6,7 @@ import { transportErrorCode, transportFailureHost, } from "../src/lib/upstream-reachability"; -import { TransientRetryEvidenceError } from "../src/lib/upstream-retry"; +import { UpstreamRetryEvidenceError } from "../src/lib/upstream-retry"; function errorWithCode(code: string | undefined, message: string, cause?: unknown): Error { const err = new Error(message) as Error & { code?: string; cause?: unknown }; @@ -102,7 +102,7 @@ describe("classifyTransportFailureKind", () => { // Mixed 5xx -> rejection: the upstream already answered 503 before the final // attempt rejected, so the host and credential path were reached. The // evidence wrapper must keep the failure out of the pre-connect class. - const wrapped = new TransientRetryEvidenceError( + const wrapped = new UpstreamRetryEvidenceError( [503], errorWithCode("ConnectionRefused", "Unable to connect. Is the computer able to access the url?"), ); @@ -111,12 +111,29 @@ describe("classifyTransportFailureKind", () => { test("a timeout after a transient 5xx keeps the timeout identity", () => { const rejection = Object.assign(new Error("Timeout elapsed"), { name: "TimeoutError" }); - const wrapped = new TransientRetryEvidenceError([503], rejection); + const wrapped = new UpstreamRetryEvidenceError([503], rejection); expect(classifyTransportFailureKind(wrapped)).toBe("timeout"); }); - test("an evidence wrapper without prior statuses classifies by its cause", () => { - const wrapped = new TransientRetryEvidenceError( + test("a rejection after a credential-visible reset stays account-attributed, never neutral", () => { + // Mixed reset -> rejection: the first attempt reached the origin and was + // reset after the request was read; the next attempt failed to connect. + const wrapped = new UpstreamRetryEvidenceError( + [], + errorWithCode("ConnectionRefused", "Unable to connect. Is the computer able to access the url?"), + true, + ); + expect(classifyTransportFailureKind(wrapped)).toBe("connect_error"); + }); + + test("a timeout after a credential-visible reset keeps the timeout identity", () => { + const rejection = Object.assign(new Error("Timeout elapsed"), { name: "TimeoutError" }); + const wrapped = new UpstreamRetryEvidenceError([], rejection, true); + expect(classifyTransportFailureKind(wrapped)).toBe("timeout"); + }); + + test("an evidence wrapper without prior statuses or resets classifies by its cause", () => { + const wrapped = new UpstreamRetryEvidenceError( [], errorWithCode("ConnectionRefused", "Unable to connect. Is the computer able to access the url?"), ); @@ -125,9 +142,10 @@ describe("classifyTransportFailureKind", () => { test("the evidence wrapper preserves the final rejection and its statuses", () => { const rejection = errorWithCode("ConnectionRefused", "Unable to connect"); - const wrapped = new TransientRetryEvidenceError([503, 502], rejection); + const wrapped = new UpstreamRetryEvidenceError([503, 502], rejection, true); expect(wrapped.cause).toBe(rejection); expect(wrapped.transientStatuses).toEqual([503, 502]); + expect(wrapped.resetSeen).toBe(true); expect(transportErrorCode(wrapped)).toBeUndefined(); }); }); From bd55f1ab7fadd6357cdaf33aa9f475c9bdc31426 Mon Sep 17 00:00:00 2001 From: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com> Date: Wed, 5 Aug 2026 01:18:10 +0800 Subject: [PATCH 5/5] fix(codex): fail closed on cleartext-http credential sends (#914) CodeRabbit review (commit 71bd8210), CWE-319: - New credentialSendSchemeError guard in the shared outbound policy: https passes, http is allowed only for loopback hosts, and cleartext http to any remote host is rejected before a credential can leave the proxy. - Applied at the credential-bearing send boundaries: the /v1/responses forward passthrough, the compact forward path, and the images relay (both the ChatGPT forward credential and keyed provider API keys). - A hand-edited custom forward provider with an http baseUrl now gets a 502 at the send boundary instead of transmitting Authorization over cleartext http (loadConfig accepts that shape; management validation already reserves forward for the canonical built-in provider). Tests: scheme-guard units (https, loopback http, remote http, malformed), a /v1/responses e2e asserting 502 + zero outbound fetches for an http forward provider, and an images e2e for a keyed http provider. --- src/lib/destination-policy.ts | 23 ++++++++++ src/server/images.ts | 6 +++ src/server/responses/compact.ts | 8 ++++ src/server/responses/core.ts | 10 +++++ tests/destination-policy-resolved.test.ts | 24 ++++++++++- tests/issue-914-transport-attribution.test.ts | 43 +++++++++++++++++++ tests/server-images.test.ts | 41 ++++++++++++++++++ 7 files changed, 154 insertions(+), 1 deletion(-) diff --git a/src/lib/destination-policy.ts b/src/lib/destination-policy.ts index af68907365..c810e9b079 100644 --- a/src/lib/destination-policy.ts +++ b/src/lib/destination-policy.ts @@ -29,6 +29,29 @@ export type DestinationKind = | "unspecified" | "metadata"; +/** + * Scheme guard for credential-bearing outbound sends (#914 review): forward + * credentials and API keys must never be transmitted over cleartext http to a + * remote host. Loopback http stays allowed for local gateways and tests — + * loopback traffic never leaves the machine. + */ +export function credentialSendSchemeError(url: string): string | null { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return "baseUrl must be a valid URL"; + } + if (parsed.protocol === "https:") return null; + if (parsed.protocol === "http:" && isLoopbackDestinationHost(parsed.hostname)) return null; + return "baseUrl must use https for credential-bearing sends"; +} + +function isLoopbackDestinationHost(hostname: string): boolean { + const normalized = hostname.trim().toLowerCase().replace(/\.$/, ""); + return normalized === "" || normalized === "localhost" || normalized === "127.0.0.1" || normalized === "::1" || normalized === "[::1]"; +} + interface DestinationAssessment { kind: DestinationKind; detail: string; diff --git a/src/server/images.ts b/src/server/images.ts index cc66ad64dd..9826bc9ae9 100644 --- a/src/server/images.ts +++ b/src/server/images.ts @@ -29,6 +29,7 @@ import { transportErrorCode, transportFailureHost, } from "../lib/upstream-reachability"; +import { credentialSendSchemeError } from "../lib/destination-policy"; import { sidecarEnter } from "../lib/sidecar-tracker"; import type { SidecarOutcomeMeta } from "../web-search/executor"; import type { OcxConfig } from "../types"; @@ -444,6 +445,11 @@ export async function handleImages( } const timeoutMs = config.images?.timeoutMs ?? IMAGES_UPSTREAM_TIMEOUT_MS; + // Fail closed before any credential leaves the proxy: neither the ChatGPT + // forward credential nor a keyed provider's API key may cross cleartext http + // to a remote host (#914 review, CWE-319). + const schemeError = credentialSendSchemeError(url); + if (schemeError) return formatErrorResponse(502, "upstream_error", `image provider ${schemeError}`); const linkedSignal = signalWithTimeout(timeoutMs, req.signal); const sidecarExit = sidecarEnter("images"); try { diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index 72768c2b7c..6b80435377 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -64,6 +64,7 @@ import { type CodexUpstreamOutcome, } from "../../codex/routing"; import { classifyTransportFailureKind, transportErrorCode } from "../../lib/upstream-reachability"; +import { credentialSendSchemeError } from "../../lib/destination-policy"; import { fetchWithResetRetry, fetchWithTransientRetry, @@ -353,6 +354,13 @@ export async function handleResponsesCompact( // so routed-model reasoning items (reasoning_text content) don't 400 the ChatGPT backend. const compactBody = sanitizeReasoningInputContent(compactBodyRaw) as typeof compactBodyRaw; const compactUrl = `${base}/responses/compact`; + if (compactProvider.authMode === "forward") { + // Fail closed before any credential leaves the proxy: a hand-edited + // forward baseUrl must not transmit Authorization over cleartext http + // (#914 review, CWE-319). + const schemeError = credentialSendSchemeError(compactUrl); + if (schemeError) return formatErrorResponse(502, "upstream_error", `Compact forward provider ${schemeError}`); + } const compactThreadId = req.headers.get("x-codex-parent-thread-id"); const connectMs = config.connectTimeoutMs ?? 200_000; // Takes its context explicitly: the alternate-account flow below records a rejection diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index a8f658d8d3..e56e314493 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -90,6 +90,7 @@ import { } from "../../codex/routing"; import { classifyTransportFailureKind, transportErrorCode } from "../../lib/upstream-reachability"; import { fetchWithResetRetry, fetchWithTransientRetry, applyUpstreamRecoveryInit } from "../../lib/upstream-retry"; +import { credentialSendSchemeError } from "../../lib/destination-policy"; import { ForwardAdmissionCredentialError, validateForwardAdmissionCredential } from "../auth-cors"; import { createTranslatorBudget, isTranslatorBudgetExceededError, type TranslatorBudget } from "../../lib/translator-budget"; import { listOpenAiForwardSidecarCandidates, resolveFirstUsableOpenAiSidecar, type ResolvedOpenAiForwardSidecar } from "../../providers/openai-sidecar"; @@ -1744,6 +1745,15 @@ async function handleResponsesInner( // so a server that redirects to a dead host can never masquerade as a // pre-connection failure after the credential was seen (#914). const forwardCredentialedSend = route.provider.authMode === "forward"; + if (forwardCredentialedSend) { + // Fail closed before any credential leaves the proxy: a hand-edited + // forward baseUrl must not transmit Authorization over cleartext http + // (#914 review, CWE-319). + const schemeError = credentialSendSchemeError(request.url); + if (schemeError) { + return formatErrorResponse(502, "upstream_error", `Forward provider ${schemeError}`); + } + } // Transient-5xx pre-stream retry (devlog/_plan/260716_claudecode_hardening/010): // the ChatGPT backend emits transient 502/520s that an immediate retry absorbs. // Body is a replayable string; nothing has streamed to the client yet. diff --git a/tests/destination-policy-resolved.test.ts b/tests/destination-policy-resolved.test.ts index 2ae103c135..ba48e55946 100644 --- a/tests/destination-policy-resolved.test.ts +++ b/tests/destination-policy-resolved.test.ts @@ -4,10 +4,32 @@ import { describe, expect, mock, test } from "bun:test"; const lookupMock = mock(async (_hostname: string, _opts: unknown): Promise<{ address: string; family: number }[]> => []); mock.module("node:dns/promises", () => ({ lookup: lookupMock })); -const { providerDestinationConfigError, providerDestinationResolvedError, resolvePublicAddresses } = await import("../src/lib/destination-policy"); +const { credentialSendSchemeError, providerDestinationConfigError, providerDestinationResolvedError, resolvePublicAddresses } = await import("../src/lib/destination-policy"); const provider = (baseUrl: string, allowPrivateNetwork?: boolean) => ({ baseUrl, allowPrivateNetwork }); +describe("credentialSendSchemeError — cleartext credential guard (#914 review)", () => { + test("https destinations pass", () => { + expect(credentialSendSchemeError("https://chatgpt.com/backend-api/codex/responses")).toBeNull(); + expect(credentialSendSchemeError("https://api.openai.com/v1/images/generations")).toBeNull(); + }); + + test("loopback http destinations pass (local gateways and tests)", () => { + expect(credentialSendSchemeError("http://127.0.0.1:11434/v1")).toBeNull(); + expect(credentialSendSchemeError("http://localhost:8080/v1")).toBeNull(); + expect(credentialSendSchemeError("http://[::1]:8080/v1")).toBeNull(); + }); + + test("cleartext http to any remote host is rejected", () => { + expect(credentialSendSchemeError("http://insecure.example.test/v1")).toContain("https"); + expect(credentialSendSchemeError("http://93.184.216.34/v1")).toContain("https"); + }); + + test("malformed URLs are rejected", () => { + expect(credentialSendSchemeError("not a url")).toContain("valid URL"); + }); +}); + describe("providerDestinationConfigError — reserved IPv4 ranges (review finding, PR #96)", () => { const cases: [string, string][] = [ ["192.0.0.8", "reserved"], diff --git a/tests/issue-914-transport-attribution.test.ts b/tests/issue-914-transport-attribution.test.ts index ca99dc7117..9ef23af419 100644 --- a/tests/issue-914-transport-attribution.test.ts +++ b/tests/issue-914-transport-attribution.test.ts @@ -482,4 +482,47 @@ describe("issue #914: pre-connection reachability failures are account-neutral", await server.stop(true); } }); + + test("forward-mode sends refuse a cleartext-http destination before any credential leaves (#914 review)", async () => { + // A hand-edited custom forward provider (loadConfig does not reject the + // reserved management shape) must still fail closed at the send boundary. + saveConfig({ + port: 0, + hostname: "127.0.0.1", + defaultProvider: "insecure-forward", + openaiProviderTierVersion: 2, + providers: { + "insecure-forward": { + adapter: "openai-responses", + baseUrl: "http://insecure.invalid/backend-api/codex", + authMode: "forward", + defaultModel: "custom-test-model", + models: ["custom-test-model"], + }, + }, + } as OcxConfig); + const outbound: string[] = []; + globalThis.fetch = (async (input, init) => { + const requestUrl = input instanceof Request + ? input.url + : input instanceof URL + ? input.toString() + : String(input); + outbound.push(requestUrl); + return ORIGINAL_FETCH(input, init); + }) as typeof fetch; + const server = startServer(0); + try { + const res = await ORIGINAL_FETCH(new URL("/v1/responses", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "custom-test-model", input: "hello", stream: false }), + }); + expect(res.status).toBe(502); + expect(await res.text()).toContain("must use https"); + expect(outbound.some(url => url.includes("insecure.invalid"))).toBe(false); + } finally { + await server.stop(true); + } + }); }); diff --git a/tests/server-images.test.ts b/tests/server-images.test.ts index 437c76c25f..52f35d4a87 100644 --- a/tests/server-images.test.ts +++ b/tests/server-images.test.ts @@ -429,6 +429,47 @@ test("an invalid explicit Images provider returns 400 after bearer admission", a } }); +test("a keyed images provider with a cleartext-http remote baseUrl is refused before any send (#914 review)", async () => { + const outbound: string[] = []; + globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + const requestUrl = typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; + outbound.push(requestUrl); + return originalFetch(input, init); + }) as typeof fetch; + saveConfig({ + port: 0, + defaultProvider: "custom-images", + openaiProviderTierVersion: 2, + providers: { + "custom-images": { + adapter: "openai-responses", + baseUrl: "http://insecure.invalid/v1", + allowPrivateNetwork: true, + authMode: "key", + apiKey: "${OPENCODEX_TEST_IMAGES_API_KEY}", + }, + }, + images: { provider: "custom-images" }, + } as OcxConfig); + + const server = startServer(0); + try { + const response = await fetch(new URL("/v1/images/generations", server.url), { + method: "POST", + headers: { + "content-type": "application/json", + authorization: `Bearer ${DIRECT_CHATGPT_TOKEN}`, + }, + body: JSON.stringify({ prompt: "a cat", model: "gpt-image-2" }), + }); + expect(response.status).toBe(502); + expect(await response.text()).toContain("must use https"); + expect(outbound.some(url => url.includes("insecure.invalid"))).toBe(false); + } finally { + await server.stop(true); + } +}); + test("an invalid explicit Images provider fails closed instead of using another upstream", async () => { const captured: CapturedRequest[] = []; const upstream = fakeImagesUpstream(captured);