From 4aea2c8b305e0956b8bf87c069f653b7deac90e9 Mon Sep 17 00:00:00 2001
From: Hernan Inverso <hernaninverso@gmail.com>
Date: Sun, 10 May 2026 16:35:45 -0300
Subject: [PATCH] fix: address gemini-code-assist feedback on #2133
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

HIGH — open-sse/services/accountFallback.ts
  ProviderProfile type was missing useUpstream429BreakerHints, and the
  buildProviderProfile helper was not propagating the stored override.
  Result: resolvedProfile.useUpstream429BreakerHints was always undefined,
  so the circuit breaker silently ignored every user override and always
  fell back to the per-provider default — making the new UI toggle a no-op.

  Fix: add the optional field to ProviderProfile, populate it from
  resilience.connectionCooldown[category].useUpstream429BreakerHints in
  buildProviderProfile, and drop the now-unnecessary type cast at the
  configureProviderBreaker call site.

MEDIUM — src/shared/utils/classify429.ts (2 call sites)
  classify429FromError was casting response.headers / err.headers directly
  to Record<string, string>. That breaks when the upstream uses a native
  fetch Headers instance, because Headers does not respond to
  Object.entries (used downstream in getHeader). On those errors the
  classifier would silently see no headers and never produce a kind.

  Fix: add a normalizeHeaders(raw) helper that detects a Headers-like
  .entries() method and converts via Object.fromEntries, falling back to
  the previous plain-object treatment. Use it at both call sites.

All 39 existing tests still pass.
---
 open-sse/services/accountFallback.ts |  8 ++++++--
 src/shared/utils/classify429.ts      | 25 +++++++++++++++++++++++--
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/open-sse/services/accountFallback.ts b/open-sse/services/accountFallback.ts
index 4b496ad0c6..cbe147737c 100644
--- a/open-sse/services/accountFallback.ts
+++ b/open-sse/services/accountFallback.ts
@@ -32,6 +32,8 @@ import { resolveUseUpstream429BreakerHints } from "../../src/shared/utils/provid
 type ProviderProfile = {
   baseCooldownMs: number;
   useUpstreamRetryHints: boolean;
+  /** Issue #2100 follow-up. Stored override; undefined → per-provider default. */
+  useUpstream429BreakerHints?: boolean;
   maxBackoffSteps: number;
   failureThreshold: number;
   resetTimeoutMs: number;
@@ -188,6 +190,9 @@ function buildProviderProfile(
   return {
     baseCooldownMs: connectionCooldown.baseCooldownMs,
     useUpstreamRetryHints: connectionCooldown.useUpstreamRetryHints,
+    // Issue #2100 follow-up: propagate stored override (boolean | undefined)
+    // so the runtime resolver picks user setting first, then per-provider default.
+    useUpstream429BreakerHints: connectionCooldown.useUpstream429BreakerHints,
     maxBackoffSteps: connectionCooldown.maxBackoffSteps,
     failureThreshold: providerBreaker.failureThreshold,
     resetTimeoutMs: providerBreaker.resetTimeoutMs,
@@ -541,8 +546,7 @@ function configureProviderBreaker(
   // Issue #2100 follow-up: resolve useUpstream429BreakerHints from the
   // provider profile (stored override) or fall back to per-provider default.
   // Stored value type is `boolean | undefined` — never `null` after PATCH.
-  const userValue = (resolvedProfile as { useUpstream429BreakerHints?: boolean })
-    .useUpstream429BreakerHints;
+  const userValue = resolvedProfile.useUpstream429BreakerHints;
   const useHints = resolveUseUpstream429BreakerHints(provider, userValue);
   return getCircuitBreaker(provider, {
     failureThreshold: resolvedProfile.failureThreshold ?? resolvedProfile.circuitBreakerThreshold,
diff --git a/src/shared/utils/classify429.ts b/src/shared/utils/classify429.ts
index 66ae2e45d1..bf355a3f38 100644
--- a/src/shared/utils/classify429.ts
+++ b/src/shared/utils/classify429.ts
@@ -163,6 +163,27 @@ export function retryAfterFromResponse(response: {
   return parseRetryAfter(getHeader(response.headers, "retry-after"));
 }
 
+/**
+ * Normalize an unknown headers-like value into a plain `Record<string, string>`.
+ * Native `Headers` (from `fetch`) does NOT respond to `Object.entries` — it
+ * exposes `.entries()` instead. Without this normalization, `getHeader` would
+ * silently miss every header on a Headers instance.
+ */
+function normalizeHeaders(raw: unknown): Record<string, string> | undefined {
+  if (raw === null || typeof raw !== "object") return undefined;
+  const maybeIter = (raw as { entries?: unknown }).entries;
+  if (typeof maybeIter === "function") {
+    try {
+      return Object.fromEntries(
+        (raw as { entries: () => Iterable<[string, string]> }).entries(),
+      );
+    } catch {
+      // fall through to plain-object treatment
+    }
+  }
+  return raw as Record<string, string>;
+}
+
 /**
  * Adapter that takes an error thrown by an HTTP client (fetch wrapper, axios,
  * upstream SDK, etc.) and produces a {@link FailureKind} suitable for the
@@ -199,7 +220,7 @@ export function classify429FromError(err: unknown): FailureKind | undefined {
       status = resp.status;
     }
     if (resp.headers && typeof resp.headers === "object") {
-      headers = resp.headers as Record<string, string>;
+      headers = normalizeHeaders(resp.headers);
     }
     if (resp.data !== undefined) {
       body = resp.data;
@@ -209,7 +230,7 @@ export function classify429FromError(err: unknown): FailureKind | undefined {
   }
 
   if (headers === undefined && e.headers && typeof e.headers === "object") {
-    headers = e.headers as Record<string, string>;
+    headers = normalizeHeaders(e.headers);
   }
   if (body === undefined) {
     if (typeof e.body !== "undefined") {