fix(providers): surface real transport cause instead of bare "fetch failed" (#14309)

open-sse/utils/proxyFetch.ts already computes a detailed transport
diagnosis (DNS/socket error code, syscall, address) whenever a direct
fetch fails on both the pooled undici dispatcher and the native-fetch
fallback, attaching it to the thrown error as `.proxyFetchDetail`. But
src/lib/providers/validation/transport.ts::toValidationErrorResult()
only ever read `error.message` — always the generic "fetch failed"
string by undici/native-fetch design — and never `error.cause` (where
SafeOutboundFetchError's normalizeFetchFailure() stores the original
error carrying `.proxyFetchDetail`). The computed diagnosis was
silently discarded before it ever reached the dashboard.

toValidationErrorResult() now walks one level of `error.cause` when
the message is the generic "fetch failed" string and, if a
`.proxyFetchDetail` is found there, surfaces it (still routed through
sanitizeErrorMessage()) instead of the bare message.

Also extracted the direct-path detail-string builder (and the existing
redactProxyDetailsInMessage() redaction helper) out of proxyFetch.ts
into a new open-sse/utils/proxyFetchRedaction.ts module, applying the
same proxy-URL/credential redaction to the direct-path detail that the
proxy-path message already had — closing a latent redaction gap
between the two branches — without growing proxyFetch.ts past its
frozen file-size baseline.

This does not explain the reporter's own underlying transport failure
(likely local network/DNS/firewall config on their machine); it makes
that failure diagnosable instead of opaque.

changelog.d/fixes/13963-zcode-win32-shell-shim.md

@@ -1 +0,0 @@
-- fix(providers): use shell:true on win32 for zcode .cmd/.bat shim spawn (#13963)

changelog.d/fixes/14309-proxyfetch-invalid-arg.md

@@ -0,0 +1 @@
+- fix(providers): surface the real transport diagnosis (DNS/socket cause) instead of a bare "fetch failed" in provider validation errors (#14309)

open-sse/executors/zcodeProtocol.ts

@@ -1,5 +1,4 @@
 import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process";
-import { shouldUseShellForCommand } from "@/shared/services/cliRuntime";
 
 const HEADER_SIZE = 13;
 const REGULAR_MESSAGE = 1;
@@ -155,21 +154,6 @@ export function encodeZcodeRpcCall(
   return frame;
 }
 
-/**
- * Whether spawn() must go through the shell to launch `command`.
- *
- * On win32, npm installs global CLI wrappers (e.g. the `zcode`/ZCODE_BIN
- * shim) as `.cmd`/`.bat` files. Since Node's CVE-2024-27980 fix, `spawn()`
- * refuses to launch a `.cmd`/`.bat` target without `shell: true`, throwing
- * ENOENT/EINVAL instead (#13963, same class of bug as #8590/Qoder). The
- * bundled ZCODE_SERVER_NODE runtime path spawns a bare `node`/`node.exe`
- * binary (no `.cmd`/`.bat` extension) and must keep `shell: false` even on
- * win32 — `shouldUseShellForCommand()` already encodes that extension check.
- */
-export function shouldUseShellForZcodeCommand(command: string): boolean {
-  return shouldUseShellForCommand(command);
-}
-
 function errorFromPayload(payload: unknown, fallback: string): Error {
   if (payload && typeof payload === "object") {
     const record = payload as JsonRecord;
@@ -228,8 +212,7 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
         cwd: this.cwd,
         env: this.env ? { ...process.env, ...this.env } : process.env,
         stdio: ["pipe", "pipe", "pipe"],
-        // shell:true on win32 for a .cmd/.bat ZCode shim — see #13963/#8590.
-        shell: shouldUseShellForZcodeCommand(this.command),
+        shell: false,
         windowsHide: true,
       });
     } catch (error) {
@@ -277,11 +260,7 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
     });
 
     try {
-      await this.withTimeout(
-        readyPromise,
-        this.startupTimeoutMs,
-        "ZCode app-server handshake timed out"
-      );
+      await this.withTimeout(readyPromise, this.startupTimeoutMs, "ZCode app-server handshake timed out");
       this.ready = true;
     } catch (error) {
       await this.disposeChild(child);
@@ -300,10 +279,9 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
     this.pendingChunks.push(chunk);
     let total = 0;
     for (const part of this.pendingChunks) total += part.byteLength;
-    const buffer =
-      total === chunk.byteLength && this.pendingChunks.length > 0
-        ? chunk
-        : Buffer.concat(this.pendingChunks);
+    const buffer = total === chunk.byteLength && this.pendingChunks.length > 0
+      ? chunk
+      : Buffer.concat(this.pendingChunks);
     this.pendingChunks = [buffer];
 
     if (!this.handshakeDone) {
@@ -329,13 +307,11 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
       }
       const child = this.child;
       if (!child) return;
-      child.stdin.write(
-        `${JSON.stringify({
-          type: "zcode-hello-ack",
-          version: "omniroute",
-          clientId: `omniroute-${process.pid}`,
-        })}\n`
-      );
+      child.stdin.write(`${JSON.stringify({
+        type: "zcode-hello-ack",
+        version: "omniroute",
+        clientId: `omniroute-${process.pid}`,
+      })}\n`);
       this.handshakeDone = true;
     }
     this.consumeFrames();
@@ -390,12 +366,10 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
     if (type === RESPONSE_MESSAGE) {
       request.resolve(payload);
     } else {
-      request.reject(
-        errorFromPayload(
-          payload,
-          type === ERROR_MESSAGE ? "ZCode RPC request failed" : "ZCode RPC request canceled"
-        )
-      );
+      request.reject(errorFromPayload(
+        payload,
+        type === ERROR_MESSAGE ? "ZCode RPC request failed" : "ZCode RPC request canceled"
+      ));
     }
   }
 
@@ -463,11 +437,7 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
     }
   }
 
-  private async withTimeout<T>(
-    promise: Promise<T>,
-    timeoutMs: number,
-    message: string
-  ): Promise<T> {
+  private async withTimeout<T>(promise: Promise<T>, timeoutMs: number, message: string): Promise<T> {
     let timer: ReturnType<typeof setTimeout> | undefined;
     try {
       return await Promise.race([

open-sse/utils/proxyFetch.ts

@@ -15,6 +15,7 @@ import {
 } from "./proxyDispatcher.ts";
 import tlsClient, { type TlsFetchOptions, guardTlsFirstByte } from "./tlsClient.ts";
 import { withUpstreamStatusCapture } from "./upstreamStatusCapture.ts";
+import { describeFallbackFailure, redactProxyDetailsInMessage } from "./proxyFetchRedaction.ts";
 import { isProxyReachable } from "@/lib/proxyHealth";
 import {
   isControlPlaneProxyDirectFallbackEnabled,
@@ -340,20 +341,6 @@ function isWreqProxySupported(proxyUrl: string): boolean {
   }
 }
 
-/**
- * Redact proxy URLs (and any bare `user:pass@host` credential tokens) from an
- * upstream transport-error message before it is surfaced. #10032 keeps the
- * underlying failure reason in the propagated error for diagnosability, but
- * the raw message can embed the full proxy URL — including userinfo
- * credentials — which must never bubble into response bodies (#9837, Hard
- * Rule #12).
- */
-function redactProxyDetailsInMessage(message: string): string {
-  return message
-    .replace(/\b(?:https?|socks[45][ah]?|socks):\/\/\S+/gi, "[redacted-proxy]")
-    .replace(/\b[^\s:@/]+:[^\s@/]*@\S+/g, "[redacted-proxy]");
-}
-
 function sanitizeTransportError(
   error: unknown,
   message: string,
@@ -908,7 +895,10 @@ async function patchedFetchUnrecorded(
             continue;
           }
           if (hasNonReplayableBody) {
-            const detail = `dispatcher=[${describeFetchCause(dispatcherError)}] native=[skipped: non-replayable request body]`;
+            const detail = describeFallbackFailure(
+              describeFetchCause(dispatcherError),
+              "skipped: non-replayable request body"
+            );
             console.warn(
               `[ProxyFetch] skipping native fetch fallback for non-replayable body: ${detail}`
             );
@@ -952,7 +942,10 @@ async function patchedFetchUnrecorded(
             return await _nativeFallback(input, options);
           } catch (nativeError) {
             // Surface both dispatcher and native causes immediately.
-            const detail = `dispatcher=[${describeFetchCause(dispatcherError)}] native=[${describeFetchCause(nativeError)}]`;
+            const detail = describeFallbackFailure(
+              describeFetchCause(dispatcherError),
+              describeFetchCause(nativeError)
+            );
             console.warn(`[ProxyFetch] native fetch fallback ALSO failed: ${detail}`);
             if (nativeError instanceof Error) {
               (nativeError as Error & { proxyFetchDetail?: string }).proxyFetchDetail = detail;

open-sse/utils/proxyFetchRedaction.ts

@@ -0,0 +1,27 @@
+// Extracted from proxyFetch.ts (frozen file-size baseline — #14309) so the
+// transport-error diagnostics built there can be redacted without growing
+// the frozen file.
+//
+// #10032 keeps the underlying transport failure reason in the propagated
+// error for diagnosability, but the raw message can embed a full proxy URL
+// — including userinfo credentials — which must never bubble into response
+// bodies (#9837, Hard Rule #12).
+
+/**
+ * Redact proxy URLs (and any bare `user:pass@host` credential tokens) from an
+ * upstream transport-error message before it is surfaced.
+ */
+export function redactProxyDetailsInMessage(message: string): string {
+  return message
+    .replace(/\b(?:https?|socks[45][ah]?|socks):\/\/\S+/gi, "[redacted-proxy]")
+    .replace(/\b[^\s:@/]+:[^\s@/]*@\S+/g, "[redacted-proxy]");
+}
+
+/**
+ * Builds the `.proxyFetchDetail` diagnosis for proxyFetch.ts's direct-path
+ * (pooled undici dispatcher + native fetch fallback) branches, redacted the
+ * same way as the proxy-path message (see redactProxyDetailsInMessage above).
+ */
+export function describeFallbackFailure(dispatcherCause: string, nativeDetail: string): string {
+  return redactProxyDetailsInMessage(`dispatcher=[${dispatcherCause}] native=[${nativeDetail}]`);
+}

scripts/check/check-public-creds.mjs

@@ -90,18 +90,18 @@ const ENV_KEY_RE = /(clientId|clientSecret|apiKey)Env\s*:/;
 //   The MiniMax family was extracted from services/usage.ts into services/usage/minimax.ts
 //   (god-file decomposition), so the FP moved with the getMiniMaxUsage signature.
 //
-//   open-sse/executors/zcodeProtocol.ts L336: `clientId: \`omniroute-${process.pid}\``
+//   open-sse/executors/zcodeProtocol.ts L313: `clientId: \`omniroute-${process.pid}\``
 //   is the per-process identifier in the local ZCode app-server handshake. It is
 //   generated from the process PID, is not an upstream OAuth/client credential, and
 //   must remain visible in the wire contract. Frozen by file:line:value key.
 //   NOTE: the key includes the LINE, so any edit that shifts this statement breaks
 //   the gate twice over — a stale-entry error plus a "new violation" for the same
-//   literal. That happened again at L313 -> L336 (#13963, win32 shell:true spawn
-//   fix). Re-point the line; do not remove the entry.
+//   literal. That is what happened here (L302 -> L313). Re-point the line; do not
+//   remove the entry.
 export const KNOWN_LITERAL_CREDS = new Set([
   "open-sse/services/usage/minimax.ts:213:minimax", // TODO(6A.8): pre-existing FP — TS fn-param type, not a credential (getMiniMaxUsage signature)
   "open-sse/services/usage/minimax.ts:213:minimax-cn", // TODO(6A.8): pre-existing FP — TS fn-param type, not a credential (getMiniMaxUsage signature)
-  "open-sse/executors/zcodeProtocol.ts:336:omniroute-${process.pid}", // local per-process ZCode handshake ID, not an upstream credential
+  "open-sse/executors/zcodeProtocol.ts:313:omniroute-${process.pid}", // local per-process ZCode handshake ID, not an upstream credential
 ]);
 
 /**

src/lib/providers/validation/transport.ts

@@ -178,6 +178,26 @@ export function toWebCookieValidationErrorResult(provider: string, error: unknow
   return toValidationErrorResult(error);
 }
 
+/**
+ * proxyFetch.ts computes a detailed transport diagnosis (DNS/socket error
+ * code, syscall, address) whenever a direct fetch fails on both the pooled
+ * undici dispatcher and the native-fetch fallback, and attaches it to the
+ * thrown error as `.proxyFetchDetail`. safeOutboundFetch's
+ * normalizeFetchFailure() then wraps that error in a SafeOutboundFetchError
+ * whose `.message` is copied from the generic "fetch failed" string and
+ * whose `.cause` is the original error carrying `.proxyFetchDetail`. Without
+ * this, the computed diagnosis never reaches the caller (#14309).
+ */
+function extractProxyFetchDetail(error: unknown): string | undefined {
+  if (!(error instanceof Error)) return undefined;
+  const cause = (error as Error & { cause?: unknown }).cause;
+  if (!(cause instanceof Error)) return undefined;
+  const detail = (cause as Error & { proxyFetchDetail?: unknown }).proxyFetchDetail;
+  return typeof detail === "string" && detail.length > 0 ? detail : undefined;
+}
+
+const GENERIC_TRANSPORT_FAILURE_PATTERN = /^fetch failed$/i;
+
 export function toValidationErrorResult(error: unknown) {
   let rawMessage: unknown = error || "Validation failed";
   try {
@@ -185,6 +205,17 @@ export function toValidationErrorResult(error: unknown) {
   } catch {
     rawMessage = "Validation failed";
   }
+  try {
+    if (
+      typeof rawMessage === "string" &&
+      GENERIC_TRANSPORT_FAILURE_PATTERN.test(rawMessage.trim())
+    ) {
+      const detail = extractProxyFetchDetail(error);
+      if (detail) rawMessage = `Network error: ${detail}`;
+    }
+  } catch {
+    // Diagnostic enrichment is advisory; never let it break error reporting.
+  }
   const message = sanitizeErrorMessage(rawMessage);
   let statusCode: number | null = null;
   let timeout = false;

tests/unit/check-public-creds.test.ts

@@ -68,10 +68,10 @@ test("allowlist freezes a literal by file:line:value key", () => {
 });
 
 test("allowlist preserves the local ZCode handshake client ID without weakening credential detection", () => {
-  // 335 newlines puts the statement on line 336, which is where it lives in
+  // 312 newlines puts the statement on line 313, which is where it lives in
   // zcodeProtocol.ts today. The allowlist key carries the line number, so this
-  // literal has to be kept in step with the source (it moved 302 -> 313 -> 336).
-  const src = `${"\n".repeat(335)}clientId: \`omniroute-\${process.pid}\`,`;
+  // literal has to be kept in step with the source (it moved 302 -> 313).
+  const src = `${"\n".repeat(312)}clientId: \`omniroute-\${process.pid}\`,`;
   assert.deepEqual(
     findLiteralCreds(src, KNOWN_LITERAL_CREDS, "open-sse/executors/zcodeProtocol.ts"),
     []

tests/unit/provider-validation-fetch-failed-diagnostics.test.ts

@@ -0,0 +1,61 @@
+// Repro for #14309 — "all provider validation fails with 'fetch failed'".
+//
+// open-sse/utils/proxyFetch.ts already computes a rich diagnostic string
+// (dispatcher cause + native-fallback cause, including the real DNS/socket
+// error code) whenever BOTH the pooled undici dispatcher path AND the
+// native-fetch fallback fail, and attaches it to the thrown error as
+// `.proxyFetchDetail` (open-sse/utils/proxyFetch.ts:953-961; proven attached
+// by the existing tests/unit/proxyfetch-undici-retry.test.ts).
+//
+// That thrown error then reaches safeOutboundFetch()'s catch block
+// (src/shared/network/safeOutboundFetch.ts::normalizeFetchFailure), which
+// wraps it into a `SafeOutboundFetchError` whose `.message` is copied from
+// the ORIGINAL error's generic "fetch failed" message and whose `.cause` is
+// the original error (carrying `.proxyFetchDetail`).
+//
+// `toValidationErrorResult()` in src/lib/providers/validation/transport.ts
+// — the function that turns that thrown error into the JSON body
+// `/api/providers/validate` sends to the dashboard — only ever reads
+// `error.message`. It never looks at `error.cause`, so the diagnostic detail
+// that was carefully computed two layers down is silently discarded before
+// it ever reaches the user, and the dashboard always shows the bare,
+// non-actionable "fetch failed" string regardless of the real underlying
+// cause (DNS failure, connection refused, TLS error, etc.) — exactly what
+// #14309 reports.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { toValidationErrorResult } from "../../src/lib/providers/validation/transport";
+import { SafeOutboundFetchError } from "../../src/shared/network/safeOutboundFetch";
+
+test("toValidationErrorResult should surface the computed proxyFetchDetail diagnosis (via error.cause) instead of the generic 'fetch failed' message (#14309)", () => {
+  // Mirrors exactly what proxyFetch.ts's native-fallback-also-failed branch
+  // attaches to the original error (open-sse/utils/proxyFetch.ts:955-958).
+  const nativeError = new Error("fetch failed") as Error & { proxyFetchDetail?: string };
+  nativeError.proxyFetchDetail =
+    "dispatcher=[fetch failed code=UND_ERR_SOCKET] native=[getaddrinfo ENOTFOUND api.mistral.ai code=ENOTFOUND syscall=getaddrinfo]";
+
+  // Mirrors exactly what safeOutboundFetch.ts's normalizeFetchFailure() produces
+  // for a generic (non-SafeOutboundFetchError, non-FetchTimeoutError) transport
+  // failure: message copied from the original error, cause = the original error.
+  const wrapped = new SafeOutboundFetchError(nativeError.message, {
+    code: "NETWORK_ERROR",
+    url: "https://api.mistral.ai/v1/models",
+    method: "GET",
+    attempts: 1,
+    isRetryable: true,
+    cause: nativeError,
+  });
+
+  const result = toValidationErrorResult(wrapped);
+
+  assert.notEqual(
+    result.error,
+    "fetch failed",
+    "expected behavior: a concrete transport diagnosis was computed two layers down (error.cause.proxyFetchDetail), so the response must not collapse to the bare, non-actionable 'fetch failed' string"
+  );
+  assert.match(
+    result.error || "",
+    /ENOTFOUND|UND_ERR_SOCKET/,
+    "expected behavior: the underlying DNS/socket error code should reach the dashboard so the operator can actually diagnose the failure"
+  );
+});

tests/unit/zcode-win32-spawn-13963.test.ts

@@ -1,78 +0,0 @@
-/**
- * Regression test for #13963 — Windows: `zc`/`zcode` provider always fails
- * with `spawn zcode ENOENT` even when ZCODE_BIN is set.
- *
- * Root cause: ZcodeAppServerClient.start() (open-sse/executors/zcodeProtocol.ts)
- * spawns the ZCode CLI with a hardcoded `shell: false`, regardless of
- * process.platform or the file extension of the resolved command. On
- * Windows, npm installs global CLI wrappers as `.cmd`/`.bat` shims, and
- * since Node's CVE-2024-27980 fix, `spawn()` refuses to launch a `.cmd`/
- * `.bat` target without `shell: true`, throwing ENOENT/EINVAL instead. This
- * is the same class of bug as #8590 (Qoder), already fixed elsewhere in
- * this repo (devin-cli.ts, auggie.ts, cliRuntime.ts's
- * shouldUseShellForCommand()).
- *
- * `shouldUseShellForZcodeCommand()` is a small, pure, exported helper so
- * this can be asserted directly without needing to intercept the live ESM
- * `spawn` binding (node:child_process.spawn is unmockable via mock.method()
- * without --experimental-test-module-mocks, which is not enabled in
- * `npm run test:unit` — see tests/unit/windows-hide-child-process-spawns-8131.test.ts).
- */
-
-import test from "node:test";
-import assert from "node:assert/strict";
-
-const { shouldUseShellForZcodeCommand } =
-  await import("@omniroute/open-sse/executors/zcodeProtocol");
-
-/** Temporarily override process.platform for the duration of `fn`. */
-function withPlatform<T>(platform: string, fn: () => T): T {
-  const original = Object.getOwnPropertyDescriptor(process, "platform")!;
-  Object.defineProperty(process, "platform", { value: platform, configurable: true });
-  try {
-    return fn();
-  } finally {
-    Object.defineProperty(process, "platform", original);
-  }
-}
-
-test("shouldUseShellForZcodeCommand returns true on win32 for a .cmd shim", () => {
-  const result = withPlatform("win32", () =>
-    shouldUseShellForZcodeCommand("C:\\Users\\aaaaa\\AppData\\Roaming\\npm\\zcode.cmd")
-  );
-  assert.equal(
-    result,
-    true,
-    "spawn() must use shell:true on win32 when the resolved ZCode binary is a " +
-      ".cmd/.bat shim, or launching it throws ENOENT/EINVAL (Node CVE-2024-27980 fix) " +
-      "— see #8590 (Qoder) for the same class of bug already fixed elsewhere in this repo"
-  );
-});
-
-test("shouldUseShellForZcodeCommand returns true on win32 for a .bat shim", () => {
-  const result = withPlatform("win32", () =>
-    shouldUseShellForZcodeCommand("C:\\Users\\aaaaa\\AppData\\Roaming\\npm\\zcode.bat")
-  );
-  assert.equal(result, true);
-});
-
-test("shouldUseShellForZcodeCommand stays false on win32 for the bundled node runtime path", () => {
-  // The ZCODE_SERVER_NODE bundled-runtime path (open-sse/executors/zcode.ts:96-101)
-  // spawns a bare `node`/`node.exe` binary, not a .cmd/.bat shim — must stay shell:false.
-  const result = withPlatform("win32", () =>
-    shouldUseShellForZcodeCommand("C:\\Users\\aaaaa\\.zcode\\server\\node.exe")
-  );
-  assert.equal(result, false);
-});
-
-test("shouldUseShellForZcodeCommand returns false on linux even for a .cmd-named command", () => {
-  const result = withPlatform("linux", () =>
-    shouldUseShellForZcodeCommand("/usr/local/bin/zcode.cmd")
-  );
-  assert.equal(result, false);
-});
-
-test("shouldUseShellForZcodeCommand returns false on darwin for the plain zcode binary", () => {
-  const result = withPlatform("darwin", () => shouldUseShellForZcodeCommand("zcode"));
-  assert.equal(result, false);
-});