fix(providers): classify transport errors, honor paired credentials, repair dead tests

pre-commit.sh

@@ -41,10 +41,39 @@ if [[ "$BRANCH_NAME" != "HEAD" ]]; then
   echo "⏭️  Skipping tests (continuous test suites require API keys - run manually with pnpm test)"
 
   # Adding formatted files to git stage.
+  #
+  # `git diff --name-only` (worktree vs index) lists every file prettier just
+  # reformatted on disk, but ALSO any unrelated file with in-progress edits
+  # that were never staged for this commit. Re-adding that raw list sweeps
+  # unrelated WIP into the commit. Only files that are BOTH just-reformatted
+  # AND already staged for this commit (index vs HEAD) should be re-added.
+  #
+  # This protects files you never staged. It does NOT give you partial
+  # staging: for a file with both staged and unstaged hunks, prettier
+  # formats the whole worktree copy and the `git add` below stages all of
+  # it, unstaged hunks included — exactly as before this change. Fixing
+  # that means formatting the staged blob in isolation and re-applying the
+  # unstaged patch, which is a larger change than this one.
   echo "📝 Adding formatted files to git stage..."
-  files="$(git diff --name-only --diff-filter=d)"
-  if [[ -n "$files" ]]; then
-    git add -- $files
+  staged_files=()
+  while IFS= read -r -d '' f; do
+    staged_files+=("$f")
+  done < <(git diff --cached --name-only --diff-filter=d -z)
+
+  if [[ ${#staged_files[@]} -gt 0 ]]; then
+    files_to_add=()
+    while IFS= read -r -d '' f; do
+      for s in "${staged_files[@]}"; do
+        if [[ "$f" == "$s" ]]; then
+          files_to_add+=("$f")
+          break
+        fi
+      done
+    done < <(git diff --name-only --diff-filter=d -z)
+
+    if [[ ${#files_to_add[@]} -gt 0 ]]; then
+      git add -- "${files_to_add[@]}"
+    fi
   fi
 
   echo "🎉 All pre-commit checks passed!"

src/cli/commands/setup.ts

@@ -46,6 +46,7 @@ import {
   createTogetherAIConfig,
   createVoyageConfig,
   createXaiConfig,
+  satisfiesFallbacks,
 } from "../../lib/utils/providerConfig.js";
 
 // Provider information database
@@ -493,7 +494,7 @@ export async function checkExistingConfigurations(): Promise<string[]> {
     }
     const requiredOk =
       (extraRequired ?? []).every((v) => !!process.env[v]) ||
-      (extraRequiredFallbacks ?? []).some((v) => !!process.env[v]);
+      satisfiesFallbacks(extraRequiredFallbacks, process.env);
     if (requiredOk) {
       configured.push(p.id);
     }

src/lib/constants/networkErrorCodes.ts

@@ -0,0 +1,21 @@
+/**
+ * Error codes that indicate a transient transport failure (dropped
+ * connection, socket reset, connect timeout) rather than a permanent
+ * misconfiguration. Shared between `proxy/proxyFetch.ts` (retry gating) and
+ * `utils/errorClassifier.ts` (NetworkError classification) so both stay in
+ * sync — a second hand-maintained copy would drift the two apart the same
+ * way the "5xx literal text vs statusCode" split did.
+ *
+ * undici's native `fetch()` wraps the real transport failure in
+ * `TypeError: fetch failed`, with the actionable code on `error.cause`
+ * (sometimes nested another level deep, e.g. a SocketError inside a
+ * ConnectTimeoutError) — never on the outer TypeError itself.
+ */
+export const TRANSIENT_NETWORK_CODES: ReadonlySet<string> = new Set([
+  "ECONNRESET",
+  "ETIMEDOUT",
+  "ECONNREFUSED",
+  "EPIPE",
+  "UND_ERR_SOCKET",
+  "UND_ERR_CONNECT_TIMEOUT",
+]);

src/lib/factories/providerDescriptors.ts

@@ -53,6 +53,9 @@ export const PROVIDER_DESCRIPTORS: readonly ProviderDescriptor[] = [
     timeouts: { generateMs: 45_000, streamMs: 120_000 },
     autoSelectPriority: 7,
     apiKeyFormatPattern: API_KEY_FORMATS.bedrock,
+    // Falls back to the AWS SDK's own default credential chain (shared
+    // profile, IAM role) when these env vars are absent — see field JSDoc.
+    credentialsResolvedExternally: true,
   },
   {
     name: AIProviderName.OPENAI,
@@ -116,12 +119,14 @@ export const PROVIDER_DESCRIPTORS: readonly ProviderDescriptor[] = [
       // priority than GOOGLE_APPLICATION_CREDENTIALS by the real gating
       // logic (hasGoogleCredentials() in googleVertex/client.ts and
       // googleVertex/utils.ts) — corrected here vs. the plan snippet, which
-      // omitted it.
+      // omitted it. GOOGLE_AUTH_CLIENT_EMAIL and GOOGLE_AUTH_PRIVATE_KEY are
+      // nested together because hasGoogleCredentials() only accepts them as
+      // a pair — either alone is not valid auth, unlike the other flat
+      // entries here which are each independently sufficient.
       extraRequiredFallbacks: [
         "GOOGLE_APPLICATION_CREDENTIALS_NEUROLINK",
         "GOOGLE_SERVICE_ACCOUNT_KEY",
-        "GOOGLE_AUTH_CLIENT_EMAIL",
-        "GOOGLE_AUTH_PRIVATE_KEY",
+        ["GOOGLE_AUTH_CLIENT_EMAIL", "GOOGLE_AUTH_PRIVATE_KEY"],
       ],
     },
     defaultModel: VertexModels.CLAUDE_4_6_SONNET,
@@ -131,6 +136,9 @@ export const PROVIDER_DESCRIPTORS: readonly ProviderDescriptor[] = [
     setupUrl: "https://console.cloud.google.com/",
     timeouts: { generateMs: 60_000, streamMs: 120_000 },
     autoSelectPriority: 3,
+    // OR-of-multiple-auth-paths (file / individual fields / base64 key) —
+    // not a flat AND-list of required env vars. See field JSDoc.
+    credentialsResolvedExternally: true,
   },
   {
     name: AIProviderName.ANTHROPIC,
@@ -268,6 +276,8 @@ export const PROVIDER_DESCRIPTORS: readonly ProviderDescriptor[] = [
     setupUrl: "https://docs.litellm.ai/docs/proxy/quick_start",
     timeouts: { generateMs: 300_000, streamMs: 120_000 },
     autoSelectPriority: 1,
+    // Documented zero-config local proxy — see field JSDoc.
+    credentialsResolvedExternally: true,
   },
   {
     name: AIProviderName.SAGEMAKER,

src/lib/proxy/proxyFetch.ts

@@ -15,6 +15,7 @@ import type {
   ProxyEnvironmentSnapshot,
 } from "../types/index.js";
 import { createHash } from "node:crypto";
+import { TRANSIENT_NETWORK_CODES } from "../constants/networkErrorCodes.js";
 
 async function getLangfuseContext(): Promise<LangfuseContext | undefined> {
   try {
@@ -106,16 +107,6 @@ function extractHostname(url: string | URL | RequestInfo): string {
   }
 }
 
-/** Error codes classified as transient (module-scope: the retry path is hot). */
-const TRANSIENT_NETWORK_CODES = new Set([
-  "ECONNRESET",
-  "ETIMEDOUT",
-  "ECONNREFUSED",
-  "EPIPE",
-  "UND_ERR_SOCKET",
-  "UND_ERR_CONNECT_TIMEOUT",
-]);
-
 /**
  * Classify a fetch failure as a transient network error worth retrying.
  *

src/lib/types/providers.ts

@@ -2005,8 +2005,8 @@ export type ProviderDescriptor = {
     modelFallbacks?: readonly string[];
     /** Additional env vars required alongside apiKey (e.g. AWS secret key, Azure endpoint). */
     extraRequired?: readonly string[];
-    /** Alternate ways to satisfy extraRequired when it isn't a plain env-var list (e.g. Vertex's file-path-OR-individual-fields auth). */
-    extraRequiredFallbacks?: readonly string[];
+    /** Alternate ways to satisfy extraRequired when it isn't a plain env-var list (e.g. Vertex's file-path-OR-individual-fields auth). Each entry is either a single env var name (satisfied alone) or a nested array of names that must ALL be present together (e.g. Vertex's GOOGLE_AUTH_CLIENT_EMAIL + GOOGLE_AUTH_PRIVATE_KEY pair, which is only valid as a pair). Evaluate with `satisfiesFallbacks()` (providerConfig.ts) rather than re-deriving this logic at each call site. */
+    extraRequiredFallbacks?: readonly (string | readonly string[])[];
     /** True when the provider is usable with zero configuration (local runtime with a documented default URL, or a documented non-secret default like LiteLLM's "sk-anything"). */
     optional?: boolean;
   };
@@ -2029,6 +2029,21 @@ export type ProviderDescriptor = {
   autoSelectPriority?: number;
   /** Format-validation regex sourced from providerConfig.ts's API_KEY_FORMATS, when one exists for this provider. */
   apiKeyFormatPattern?: RegExp;
+  /**
+   * True when this provider's credentials are resolved by an external chain
+   * or its own config validator rather than by plain env-var presence, so
+   * its required-env-vars can't be expressed as "every one of these exact
+   * names must be literally set". Examples: Vertex accepts a service-account
+   * file OR individual client-email/private-key fields OR a base64 key
+   * (an OR, not an AND, of auth paths); Bedrock falls back to the AWS SDK's
+   * own default credential chain (shared profile, IAM role) with no env
+   * vars required at all; LiteLLM is a documented zero-config local proxy.
+   * `ProviderHealthChecker.getRequiredEnvironmentVariables()` returns `[]`
+   * for these providers and defers to `checkProviderSpecificConfig()`'s
+   * dedicated per-provider check instead of deriving a flat AND-list from
+   * `envVars`.
+   */
+  credentialsResolvedExternally?: boolean;
 };
 
 // =============================================================================

src/lib/utils/errorClassifier.ts

@@ -22,28 +22,103 @@ import {
 } from "../types/index.js";
 import { TimeoutError } from "./timeout.js";
 import { duckTypedStatusCode } from "./providerRetry.js";
+import { TRANSIENT_NETWORK_CODES } from "../constants/networkErrorCodes.js";
+import { redactUrlsInText } from "./logSanitize.js";
+
+/** Bounded walk depth for `.cause` chains — matches the precedent in
+ * `proxy/proxyFetch.ts`'s `isTransientNetworkError`. Guards against
+ * pathological/cyclic `.cause` chains hanging classification. */
+const MAX_CAUSE_DEPTH = 5;
+
+/**
+ * Walk `error.cause` up to `MAX_CAUSE_DEPTH` links, guarded by a seen-set so
+ * a cyclic chain (`a.cause === a`, or a longer cycle) terminates instead of
+ * looping. Node's native `fetch` (undici) throws `TypeError: fetch failed`
+ * with the real transport error nested under `.cause` — sometimes another
+ * level deep (e.g. a SocketError inside a ConnectTimeoutError) — so a
+ * classifier that only reads the outer error's `.message`/`.code` never
+ * sees it.
+ */
+function collectCauseChain(error: unknown): Record<string, unknown>[] {
+  const chain: Record<string, unknown>[] = [];
+  const seen = new Set<unknown>();
+  let current: unknown = error;
+  while (
+    current &&
+    typeof current === "object" &&
+    !seen.has(current) &&
+    chain.length < MAX_CAUSE_DEPTH
+  ) {
+    seen.add(current);
+    const record = current as Record<string, unknown>;
+    chain.push(record);
+    current = record.cause;
+  }
+  return chain;
+}
+
+function firstString(
+  chain: Record<string, unknown>[],
+  key: "name" | "code",
+): string | undefined {
+  for (const record of chain) {
+    if (typeof record[key] === "string") {
+      return record[key] as string;
+    }
+  }
+  return undefined;
+}
 
 function buildErrorContext(
   error: unknown,
   provider: string,
   modelName?: string,
 ): ProviderErrorContext {
-  const record =
-    error && typeof error === "object"
-      ? (error as Record<string, unknown>)
-      : undefined;
-  const message =
-    typeof record?.message === "string"
-      ? record.message
+  const chain = collectCauseChain(error);
+  const top = chain[0];
+  const topMessage =
+    typeof top?.message === "string"
+      ? top.message
       : error instanceof Error
         ? error.message
         : "Unknown error";
+
+  // Compose (never replace) the message: append the deepest cause's message
+  // when it differs from the top, so existing rules matching the outer text
+  // (e.g. "rate limit", "model not found") keep matching, while the real
+  // transport failure buried in .cause becomes visible to rules that need
+  // it (e.g. a nested "ECONNREFUSED").
+  // The nested message is redacted before it is composed in: an undici cause
+  // carries the full request URL, so a presigned token would otherwise reach
+  // a client-facing error message through this path. Only the nested text is
+  // scrubbed — the provider's own top-level message is left alone, since
+  // several providers deliberately name their base URL in it.
+  const deepest = chain[chain.length - 1];
+  const deepestMessage =
+    typeof deepest?.message === "string"
+      ? redactUrlsInText(deepest.message)
+      : undefined;
+  const message =
+    deepestMessage && deepestMessage !== topMessage
+      ? `${topMessage}: ${deepestMessage}`
+      : topMessage;
+
+  // errorCode/errorName/statusCode: prefer the outer error's own value,
+  // falling back to the first cause in the chain that has one.
+  let statusCode: number | undefined;
+  for (const record of chain) {
+    statusCode = duckTypedStatusCode(record);
+    if (statusCode !== undefined) {
+      break;
+    }
+  }
+
   return {
     error,
     message,
-    statusCode: duckTypedStatusCode(error),
-    errorName: typeof record?.name === "string" ? record.name : undefined,
-    errorCode: typeof record?.code === "string" ? record.code : undefined,
+    statusCode,
+    errorName: firstString(chain, "name"),
+    errorCode: firstString(chain, "code"),
     provider,
     modelName,
   };
@@ -111,17 +186,45 @@ export const DEFAULT_ERROR_RULES: ProviderErrorRule[] = [
         : `${ctx.provider} model not found.`,
   },
   {
+    // Message regex covers providers/SDKs that surface a code as text
+    // (e.g. AWS SDK wrapping "ECONNRESET" into its own message). errorCode
+    // covers undici's native fetch(), which wraps transport failures as
+    // `TypeError: fetch failed` and puts the *structured* code
+    // (ECONNREFUSED, UND_ERR_SOCKET, ...) on a nested `.cause` rather than
+    // in any message text — buildErrorContext's cause walk surfaces it here.
     match: (ctx) =>
       /ECONNRESET|ENOTFOUND|ECONNREFUSED|ETIMEDOUT|network|connection/i.test(
         ctx.message,
-      ),
+      ) ||
+      (ctx.errorCode !== undefined &&
+        TRANSIENT_NETWORK_CODES.has(ctx.errorCode)),
     errorClass: NetworkError,
     message: (ctx) => `Connection error: ${ctx.message}`,
   },
   {
+    // Batch J Task 3: the old `/\b5\d\d\b/` matched ANY bare 3-digit number
+    // in [500,599) anywhere in the message — e.g. "max_tokens (500) exceeds
+    // model limit" — with no relation to an actual HTTP status. Tightened to
+    // require the number sit in a status-shaped context: immediately next
+    // to "error" (either order) or "status"/"status code" (a common HTTP
+    // client wrapper phrase, e.g. axios's "Request failed with status code
+    // 500"), with a bounded gap so unrelated digits nearby can't bridge the
+    // match — or a named 5xx phrase that needs no digit at all ("bad
+    // gateway", "service unavailable", "gateway timeout", "server error",
+    // which already covers "... Internal Server Error"). This changes the
+    // MATCHED MESSAGE TEXT only, never the classified class: when no rule
+    // matches, `classifyProviderError`'s fallback also returns
+    // `ProviderError` (see above) — the same class this rule assigns — so
+    // narrowing this regex can only move a message between "${provider}
+    // server error: ..." and "${provider} error: ...", never between error
+    // classes.
     match: (ctx) =>
-      (ctx.statusCode !== undefined && ctx.statusCode >= 500) ||
-      /\b5\d\d\b|server error/i.test(ctx.message),
+      (ctx.statusCode !== undefined &&
+        ctx.statusCode >= 500 &&
+        ctx.statusCode <= 599) ||
+      /server error|bad gateway|service unavailable|gateway timeout|\berror\b\D{0,12}\b5\d\d\b|\b5\d\d\b\D{0,12}\berror\b|\bstatus(?:\s*code)?\b\D{0,12}\b5\d\d\b/i.test(
+        ctx.message,
+      ),
     errorClass: ProviderError,
     message: (ctx) => `${ctx.provider} server error: ${ctx.message}`,
   },

src/lib/utils/providerConfig.ts

@@ -222,6 +222,35 @@ export function hasProviderCredentials(envVars: string[]): boolean {
   return envVars.some((envVar) => !!process.env[envVar]);
 }
 
+/**
+ * Evaluates a `ProviderDescriptor.envVars.extraRequiredFallbacks`-shaped
+ * list against an env-var source. Each entry is either a single env var
+ * name (satisfied on its own) or a nested array of names that must ALL be
+ * present together (e.g. Vertex's GOOGLE_AUTH_CLIENT_EMAIL +
+ * GOOGLE_AUTH_PRIVATE_KEY pair, which is only valid auth as a pair).
+ * Returns true when at least one entry is satisfied. The single evaluation
+ * site for this shape — every consumer (providerUtils.ts, providerHealth.ts,
+ * setup.ts, environmentManager.ts) must call this instead of re-deriving the
+ * same `.some()`/`.every()` logic, so they can't drift out of sync with each
+ * other or with the real auth gate (hasGoogleCredentials()).
+ * @param env Explicit env-var source (`process.env`, or a parsed .env file) —
+ *   never hardcoded, so callers checking a file's contents (not the live
+ *   process env) can reuse this too.
+ */
+export function satisfiesFallbacks(
+  fallbacks: readonly (string | readonly string[])[] | undefined,
+  env: Record<string, string | undefined>,
+): boolean {
+  if (!fallbacks) {
+    return false;
+  }
+  return fallbacks.some((entry) =>
+    typeof entry === "string"
+      ? !!env[entry]
+      : entry.every((name) => !!env[name]),
+  );
+}
+
 // =============================================================================
 // PROVIDER-SPECIFIC CONFIGURATION CREATORS
 // =============================================================================