merge: v2/1564-cli-exit-codes (#1583)

clients/cli/README.md

@@ -149,6 +149,32 @@ npx @modelcontextprotocol/inspector --cli --catalog mcp.json --server my-http-se
 
 See [EMA / enterprise-managed auth](../../specification/v2_auth_ema.md) and [OAuth smoke testing](../../specification/v2_auth_smoke_testing.md) for configuration details and staging servers.
 
+## Exit codes & error envelopes
+
+Every non-zero exit maps to a stable failure class, so a programmatic caller
+(CI, a script, an agent) can branch on _why_ the CLI failed without scraping
+prose from stderr:
+
+| Code | Meaning |
+| ---- | ------- |
+| `0` | Success. |
+| `1` | Usage / unexpected error (the catch-all). |
+| `2` | No MCP App found on the tool (`--app-info` probe). |
+| `3` | Server requires authentication (401/403, `WWW-Authenticate`, OAuth). |
+| `4` | Server unreachable (DNS, connection refused, timeout, `fetch failed`). |
+| `5` | Tool error (`tools/call` returned `isError:true`, or the tool was not found). |
+
+On any non-zero exit the CLI also writes a single JSON line to **stderr** — the
+`ErrorEnvelope`:
+
+```json
+{ "error": { "code": "auth_required", "message": "Unauthorized", "status": 401, "url": "https://api.example/mcp" } }
+```
+
+The `code` is a stable identifier for the failure class; `message` is the
+human-readable error; `cause`, `status`, and `url` are included when known.
+Because it is one line, a caller can parse it with `2>&1 | tail -1 | jq .error`.
+
 ## Why use the CLI?
 
 While the Web Client provides a rich visual interface, the CLI is designed for:

clients/cli/__tests__/error-handler.test.ts

@@ -1,5 +1,11 @@
 import { describe, it, expect, vi, afterEach } from "vitest";
-import { handleError } from "../src/error-handler.js";
+import {
+  CliExitCodeError,
+  EXIT_CODES,
+  classifyError,
+  formatErrorOutput,
+  handleError,
+} from "../src/error-handler.js";
 
 /**
  * `handleError` is the binary's last-resort error sink (wired up in
@@ -13,33 +19,217 @@ describe("handleError", () => {
     vi.restoreAllMocks();
   });
 
-  it("logs an Error's message and exits with code 1", () => {
-    const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+  it("emits a JSON error envelope on stderr and exits with the classified code", () => {
+    const writeSpy = vi
+      .spyOn(process.stderr, "write")
+      .mockImplementation((() => true) as never);
     const exitSpy = vi
       .spyOn(process, "exit")
       .mockImplementation((() => undefined) as never);
 
     handleError(new Error("boom"));
 
-    expect(errorSpy).toHaveBeenCalledWith("boom");
-    expect(exitSpy).toHaveBeenCalledWith(1);
+    expect(exitSpy).toHaveBeenCalledWith(EXIT_CODES.USAGE);
+    const written = writeSpy.mock.calls[0]![0] as string;
+    const parsed = JSON.parse(written) as {
+      error: { code: string; message: string };
+    };
+    expect(parsed.error.code).toBe("error");
+    expect(parsed.error.message).toBe("boom");
   });
 
-  it("logs a string error verbatim", () => {
-    const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
-    vi.spyOn(process, "exit").mockImplementation((() => undefined) as never);
+  it("uses a CliExitCodeError's exitCode and envelope code", () => {
+    vi.spyOn(process.stderr, "write").mockImplementation((() => true) as never);
+    const exitSpy = vi
+      .spyOn(process, "exit")
+      .mockImplementation((() => undefined) as never);
 
-    handleError("plain failure");
+    handleError(
+      new CliExitCodeError(EXIT_CODES.NO_APP, "no app", { code: "no_app" }),
+    );
 
-    expect(errorSpy).toHaveBeenCalledWith("plain failure");
+    expect(exitSpy).toHaveBeenCalledWith(EXIT_CODES.NO_APP);
   });
+});
 
-  it("falls back to 'Unknown error' for non-Error, non-string values", () => {
-    const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
-    vi.spyOn(process, "exit").mockImplementation((() => undefined) as never);
+describe("classifyError", () => {
+  it("classifies a 401 status as AUTH_REQUIRED", () => {
+    const err = Object.assign(new Error("Unauthorized"), { status: 401 });
+    const { exitCode, envelope } = classifyError(err, {
+      url: "https://x.example/mcp",
+    });
+    expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED);
+    expect(envelope.code).toBe("auth_required");
+    expect(envelope.status).toBe(401);
+    expect(envelope.url).toBe("https://x.example/mcp");
+  });
+
+  it("classifies a WWW-Authenticate message as AUTH_REQUIRED without a status", () => {
+    const { exitCode } = classifyError(
+      new Error("Dynamic client registration failed: WWW-Authenticate Bearer"),
+    );
+    expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED);
+  });
+
+  it("classifies ENOTFOUND / fetch failed as UNREACHABLE", () => {
+    const err = new Error("fetch failed");
+    (err as { cause?: unknown }).cause = new Error(
+      "getaddrinfo ENOTFOUND no.such.host",
+    );
+    const { exitCode, envelope } = classifyError(err);
+    expect(exitCode).toBe(EXIT_CODES.UNREACHABLE);
+    expect(envelope.cause).toContain("ENOTFOUND");
+  });
+
+  it("classifies a connection-refused cause as UNREACHABLE", () => {
+    const err = new Error("connect ECONNREFUSED 127.0.0.1:1");
+    expect(classifyError(err).exitCode).toBe(EXIT_CODES.UNREACHABLE);
+  });
+
+  it("falls back to USAGE for an unrecognized Error", () => {
+    expect(classifyError(new Error("something else")).exitCode).toBe(
+      EXIT_CODES.USAGE,
+    );
+  });
+
+  it("handles a string error", () => {
+    const { exitCode, envelope } = classifyError("plain failure");
+    expect(exitCode).toBe(EXIT_CODES.USAGE);
+    expect(envelope.message).toBe("plain failure");
+  });
+
+  it("handles a non-Error, non-string value", () => {
+    const { envelope } = classifyError({ unexpected: true });
+    expect(envelope.message).toBe("Unknown error");
+  });
+
+  it("reads an HTTP status from a nested response object", () => {
+    const err = { response: { status: 401 } };
+    const { exitCode, envelope } = classifyError(err);
+    expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED);
+    expect(envelope.status).toBe(401);
+  });
+
+  it("preserves a CliExitCodeError's explicit envelope code over the default", () => {
+    const { envelope } = classifyError(
+      new CliExitCodeError(EXIT_CODES.TOOL_ERROR, "x", {
+        code: "tool_not_found",
+      }),
+    );
+    expect(envelope.code).toBe("tool_not_found");
+  });
+
+  it("carries a CliExitCodeError's explicit status and url onto the envelope", () => {
+    const { envelope } = classifyError(
+      new CliExitCodeError(EXIT_CODES.AUTH_REQUIRED, "denied", {
+        status: 401,
+        url: "https://api.example/mcp",
+      }),
+    );
+    expect(envelope.status).toBe(401);
+    expect(envelope.url).toBe("https://api.example/mcp");
+  });
+
+  it("falls back to the context url for a CliExitCodeError without its own url", () => {
+    const { envelope } = classifyError(
+      new CliExitCodeError(EXIT_CODES.UNREACHABLE, "down"),
+      { url: "https://ctx.example/mcp" },
+    );
+    expect(envelope.url).toBe("https://ctx.example/mcp");
+  });
+
+  it("derives a default envelope code for a bare CliExitCodeError", () => {
+    expect(
+      classifyError(new CliExitCodeError(EXIT_CODES.UNREACHABLE, "x")).envelope
+        .code,
+    ).toBe("unreachable");
+    expect(
+      classifyError(new CliExitCodeError(EXIT_CODES.NO_APP, "x")).envelope.code,
+    ).toBe("no_app");
+    expect(
+      classifyError(new CliExitCodeError(EXIT_CODES.AUTH_REQUIRED, "x"))
+        .envelope.code,
+    ).toBe("auth_required");
+    expect(
+      classifyError(new CliExitCodeError(EXIT_CODES.TOOL_ERROR, "x")).envelope
+        .code,
+    ).toBe("tool_error");
+    expect(
+      classifyError(new CliExitCodeError(EXIT_CODES.USAGE, "x")).envelope.code,
+    ).toBe("error");
+  });
+
+  it("terminates on a self-referential cause chain without recursing infinitely", () => {
+    // A cyclic `error.cause` must not hang the classifier; the depth cap bounds
+    // the walk and returns a finite string.
+    const err = new Error("looping");
+    (err as { cause?: unknown }).cause = err;
+    const { envelope } = classifyError(err);
+    expect(envelope.message).toBe("looping");
+    // Bounded by the depth cap: a finite string, not an infinite loop.
+    expect(typeof envelope.cause).toBe("string");
+    expect(envelope.cause).toContain("looping");
+  });
+
+  it("captures a non-Error cause as a string", () => {
+    const err = new Error("outer");
+    (err as { cause?: unknown }).cause = { reason: "blocked" };
+    const { envelope } = classifyError(err);
+    expect(envelope.cause).toBe("[object Object]");
+  });
 
-    handleError({ unexpected: true });
+  it("flattens a nested Error cause chain into one string", () => {
+    const inner = new Error("getaddrinfo ENOTFOUND host");
+    const middle = Object.assign(new Error("connect failed"), { cause: inner });
+    const outer = Object.assign(new Error("fetch failed"), { cause: middle });
+    const { envelope } = classifyError(outer);
+    expect(envelope.cause).toBe("connect failed: getaddrinfo ENOTFOUND host");
+  });
+
+  it("reads a numeric SDK-style .code as an HTTP status", () => {
+    const err = Object.assign(new Error("forbidden"), { code: 403 });
+    const { exitCode, envelope } = classifyError(err);
+    expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED);
+    expect(envelope.status).toBe(403);
+  });
+
+  it("ignores a JSON-RPC McpError numeric .code (outside the HTTP range) as a status", () => {
+    // MCP SDK's McpError carries a numeric `.code` that is a JSON-RPC error
+    // code (e.g. -32601 MethodNotFound), NOT an HTTP status. It must not leak
+    // into `status`, nor get misclassified as AUTH_REQUIRED.
+    const err = Object.assign(new Error("Method not found"), { code: -32601 });
+    const { exitCode, envelope } = classifyError(err);
+    expect(exitCode).toBe(EXIT_CODES.USAGE);
+    expect(envelope.status).toBeUndefined();
+  });
+
+  it("ignores a non-numeric node error code when reading the status", () => {
+    const err = Object.assign(new Error("getaddrinfo ENOTFOUND host"), {
+      code: "ENOTFOUND",
+    });
+    const { exitCode, envelope } = classifyError(err);
+    // The message matches the UNREACHABLE pattern, and the string node code
+    // must NOT be treated as an HTTP status.
+    expect(exitCode).toBe(EXIT_CODES.UNREACHABLE);
+    expect(envelope.status).toBeUndefined();
+  });
+});
+
+describe("formatErrorOutput", () => {
+  it("emits one JSON line on stderr ending in a newline", () => {
+    const { stderr, exitCode } = formatErrorOutput(new Error("nope"));
+    expect(stderr.endsWith("\n")).toBe(true);
+    expect(stderr.split("\n").length).toBe(2);
+    const parsed = JSON.parse(stderr) as { error: { message: string } };
+    expect(parsed.error.message).toBe("nope");
+    expect(exitCode).toBe(EXIT_CODES.USAGE);
+  });
 
-    expect(errorSpy).toHaveBeenCalledWith("Unknown error");
+  it("threads the context url into the envelope", () => {
+    const { stderr } = formatErrorOutput(new Error("nope"), {
+      url: "https://ctx.example/mcp",
+    });
+    const parsed = JSON.parse(stderr) as { error: { url?: string } };
+    expect(parsed.error.url).toBe("https://ctx.example/mcp");
   });
 });

clients/cli/__tests__/helpers/cli-runner.ts

@@ -1,4 +1,5 @@
 import { runCli as invokeCli } from "../../src/cli.js";
+import { formatErrorOutput } from "../../src/error-handler.js";
 
 export interface CliResult {
   exitCode: number | null;
@@ -52,7 +53,8 @@ function captureWrite(append: (text: string) => void) {
  * (`exitCode` / `stdout` / `stderr` / `output`) so every existing test and the
  * shared assertion helpers keep working unchanged:
  * - stdout/stderr are captured by temporarily patching `process.std*.write`
- *   (and `console.error`/`console.warn`, which commander and error paths use).
+ *   (which also captures `console.error`/`console.warn`, since those route
+ *   through `process.stderr.write`).
  * - A thrown error (bad args, failed connect, unsupported method) maps to
  *   `exitCode: 1` with the message appended to stderr — mirroring how the real
  *   binary (`index.ts`) routes errors through `handleError` → `process.exit(1)`.
@@ -77,8 +79,6 @@ export async function runCli(
 
   const originalStdoutWrite = process.stdout.write;
   const originalStderrWrite = process.stderr.write;
-  const originalConsoleError = console.error;
-  const originalConsoleWarn = console.warn;
 
   // Snapshot and apply env overrides; `undefined` records keys that did not
   // exist so they can be deleted (not set to the string "undefined") on restore.
@@ -93,15 +93,12 @@ export async function runCli(
   process.stdout.write = captureWrite((text) => {
     stdout += text;
   }) as typeof process.stdout.write;
+  // `console.error` / `console.warn` route through `process.stderr.write`, so
+  // patching the underlying stream captures them too — no separate console
+  // override is needed (the previous explicit overrides were redundant).
   process.stderr.write = captureWrite((text) => {
     stderr += text;
   }) as typeof process.stderr.write;
-  console.error = (...parts: unknown[]) => {
-    stderr += parts.map(String).join(" ") + "\n";
-  };
-  console.warn = (...parts: unknown[]) => {
-    stderr += parts.map(String).join(" ") + "\n";
-  };
 
   // `runCli` reads `process.argv.slice(2)`, so prepend two placeholder entries
   // ([node, script]) to mirror how the binary is launched.
@@ -127,14 +124,15 @@ export async function runCli(
   try {
     await Promise.race([invokeCli(argv), timeout]);
   } catch (error) {
-    exitCode = 1;
-    stderr += (error instanceof Error ? error.message : String(error)) + "\n";
+    // Mirror the binary's `handleError` exactly so tests observe the same exit
+    // code and stderr (one JSON envelope line) the real CLI would emit.
+    const out = formatErrorOutput(error);
+    exitCode = out.exitCode;
+    stderr += out.stderr;
   } finally {
     if (timer) clearTimeout(timer);
     process.stdout.write = originalStdoutWrite;
     process.stderr.write = originalStderrWrite;
-    console.error = originalConsoleError;
-    console.warn = originalConsoleWarn;
     for (const [key, value] of Object.entries(envBackup)) {
       if (value === undefined) delete process.env[key];
       else process.env[key] = value;

clients/cli/src/error-handler.ts

@@ -1,20 +1,216 @@
-function formatError(error: unknown): string {
-  let message: string;
+/**
+ * Exit-code map. Non-zero codes let an automated caller (CI, an agent) branch
+ * on the failure class without regex-scraping stderr:
+ *
+ *  - 0: success
+ *  - 1: usage / unexpected error (the catch-all; same as before this map)
+ *  - 2: `--app-info` probe found no MCP App on the tool
+ *  - 3: server requires authentication (401 / WWW-Authenticate / OAuth)
+ *  - 4: server unreachable (DNS, connect refused, timeout, fetch failure)
+ *  - 5: tool error (`tools/call` returned `isError:true`, or tool not found)
+ */
+export const EXIT_CODES = {
+  OK: 0,
+  USAGE: 1,
+  NO_APP: 2,
+  AUTH_REQUIRED: 3,
+  UNREACHABLE: 4,
+  TOOL_ERROR: 5,
+} as const;
 
-  if (error instanceof Error) {
-    message = error.message;
-  } else if (typeof error === "string") {
-    message = error;
-  } else {
-    message = "Unknown error";
+/** Machine-readable error envelope written as one JSON line on stderr. */
+export interface ErrorEnvelope {
+  /** Stable identifier for the failure class (e.g. "auth_required"). */
+  code: string;
+  /** Human-readable message. */
+  message: string;
+  /** The underlying error's `cause` (e.g. undici's ENOTFOUND), if any. */
+  cause?: string;
+  /** HTTP status when the failure was an HTTP-level response. */
+  status?: number;
+  /** The server URL the failure was against, when known. */
+  url?: string;
+}
+
+/**
+ * Thrown by the CLI to request a specific non-zero exit code without routing
+ * through the generic error path. {@link formatErrorOutput} reads `exitCode`
+ * and `envelope`; the in-process test runner does the same so tests observe
+ * the real code and stderr.
+ */
+export class CliExitCodeError extends Error {
+  constructor(
+    public readonly exitCode: number,
+    message: string,
+    public readonly envelope?: Partial<ErrorEnvelope>,
+  ) {
+    super(message);
+    this.name = "CliExitCodeError";
+  }
+}
+
+/** Depth cap for {@link causeOf} — bounds a pathological/self-referential
+ * `error.cause` chain so a cycle can never recurse infinitely. */
+const MAX_CAUSE_DEPTH = 16;
+
+/** Read an `Error.cause` chain into a single readable string, if present. */
+function causeOf(error: unknown, depth = 0): string | undefined {
+  if (depth >= MAX_CAUSE_DEPTH) return undefined;
+  if (!(error instanceof Error)) return undefined;
+  const cause = (error as { cause?: unknown }).cause;
+  if (cause === undefined || cause === null) return undefined;
+  if (cause instanceof Error) {
+    const nested = causeOf(cause, depth + 1);
+    return nested ? `${cause.message}: ${nested}` : cause.message;
+  }
+  return String(cause);
+}
+
+/** Best-effort HTTP status from common error shapes (undici, MCP SDK, fetch). */
+function statusOf(error: unknown): number | undefined {
+  if (error == null || typeof error !== "object") return undefined;
+  const e = error as {
+    status?: unknown;
+    code?: unknown;
+    response?: { status?: unknown };
+  };
+  if (typeof e.status === "number") return e.status;
+  if (typeof e.response?.status === "number") return e.response.status;
+  // SDK SseError / StreamableHTTPError expose `.code` as the HTTP status. Guard
+  // to the HTTP range (100-599) so a JSON-RPC McpError code (e.g. -32601
+  // MethodNotFound) is not mistaken for an HTTP status and leaked into the
+  // envelope or misclassified as AUTH_REQUIRED. String node codes like
+  // "ENOTFOUND" are already excluded by the numeric check.
+  if (typeof e.code === "number" && e.code >= 100 && e.code <= 599) {
+    return e.code;
+  }
+  return undefined;
+}
+
+const UNREACHABLE_PATTERN =
+  /ENOTFOUND|ECONNREFUSED|ECONNRESET|EAI_AGAIN|ETIMEDOUT|fetch failed|getaddrinfo|connect timed out|aborted/i;
+
+/**
+ * Classify an arbitrary error into an exit code and envelope. Used both by the
+ * binary's {@link handleError} and by callers that want to throw a
+ * {@link CliExitCodeError} with the right code up front.
+ */
+export function classifyError(
+  error: unknown,
+  context?: { url?: string },
+): { exitCode: number; envelope: ErrorEnvelope } {
+  const message =
+    error instanceof Error
+      ? error.message
+      : typeof error === "string"
+        ? error
+        : "Unknown error";
+  const cause = causeOf(error);
+  const status = statusOf(error);
+  const url = context?.url;
+
+  // A pre-classified error carries its own exit code; fill in any envelope
+  // fields the thrower didn't supply.
+  if (error instanceof CliExitCodeError) {
+    return {
+      exitCode: error.exitCode,
+      envelope: {
+        code: error.envelope?.code ?? codeForExit(error.exitCode),
+        message,
+        ...(cause !== undefined && { cause }),
+        ...(error.envelope?.status !== undefined && {
+          status: error.envelope.status,
+        }),
+        ...((error.envelope?.url ?? url) !== undefined && {
+          url: error.envelope?.url ?? url,
+        }),
+      },
+    };
   }
 
-  return message;
+  // 401 / OAuth-required → AUTH_REQUIRED so the caller can kick the auth flow.
+  if (
+    status === 401 ||
+    status === 403 ||
+    /WWW-Authenticate|Unauthorized|invalid_token|OAuth/i.test(
+      message + " " + (cause ?? ""),
+    )
+  ) {
+    return {
+      exitCode: EXIT_CODES.AUTH_REQUIRED,
+      envelope: {
+        code: "auth_required",
+        message,
+        ...(cause !== undefined && { cause }),
+        ...(status !== undefined && { status }),
+        ...(url !== undefined && { url }),
+      },
+    };
+  }
+
+  // Network-layer failure → UNREACHABLE so the caller can retry via a proxy.
+  if (UNREACHABLE_PATTERN.test(message + " " + (cause ?? ""))) {
+    return {
+      exitCode: EXIT_CODES.UNREACHABLE,
+      envelope: {
+        code: "unreachable",
+        message,
+        ...(cause !== undefined && { cause }),
+        ...(status !== undefined && { status }),
+        ...(url !== undefined && { url }),
+      },
+    };
+  }
+
+  return {
+    exitCode: EXIT_CODES.USAGE,
+    envelope: {
+      code: "error",
+      message,
+      ...(cause !== undefined && { cause }),
+      ...(status !== undefined && { status }),
+      ...(url !== undefined && { url }),
+    },
+  };
+}
+
+function codeForExit(exitCode: number): string {
+  switch (exitCode) {
+    case EXIT_CODES.NO_APP:
+      return "no_app";
+    case EXIT_CODES.AUTH_REQUIRED:
+      return "auth_required";
+    case EXIT_CODES.UNREACHABLE:
+      return "unreachable";
+    case EXIT_CODES.TOOL_ERROR:
+      return "tool_error";
+    default:
+      return "error";
+  }
+}
+
+/**
+ * Single source of truth for mapping a thrown error to the CLI's exit code and
+ * stderr text. The binary entry ({@link handleError}) and the in-process test
+ * runner both call this, so tests observe exactly what the binary would emit.
+ *
+ * stderr is one JSON line `{"error":{...}}` so a caller can `2>&1 | tail -1 |
+ * jq .error`; the message is inside the envelope so nothing is lost vs. the
+ * previous bare-message behaviour.
+ */
+export function formatErrorOutput(
+  error: unknown,
+  context?: { url?: string },
+): { exitCode: number; stderr: string } {
+  const { exitCode, envelope } = classifyError(error, context);
+  return {
+    exitCode,
+    stderr: JSON.stringify({ error: envelope }) + "\n",
+  };
 }
 
 export function handleError(error: unknown): never {
-  const errorMessage = formatError(error);
-  console.error(errorMessage);
-
-  process.exit(1);
+  const { exitCode, stderr } = formatErrorOutput(error);
+  process.stderr.write(stderr);
+  process.exit(exitCode);
 }