fix(cli): bound the OAuth browser open and say when it fails

clients/cli/README.md

@@ -301,7 +301,7 @@ Register `http://127.0.0.1:6276/oauth/callback` on static or enterprise IdPs tha
 | `--client-secret <secret>`    | —                        | OAuth client secret; overrides `client.json`.                                                                                                                                                                                                                                                                                                |
 | `--client-metadata-url <url>` | —                        | CIMD metadata URL; overrides `client.json`.                                                                                                                                                                                                                                                                                                  |
 | `--callback-url <url>`        | `MCP_OAUTH_CALLBACK_URL` | Redirect URI sent to the authorization server (default: `http://127.0.0.1:6276/oauth/callback`). Must bind a **loopback** host (`localhost` / `127.0.0.0/8` / `[::1]`) — the listener receives the authorization code over plaintext `http`, so a non-loopback host hard-errors (no opt-in; use a port-forward if the browser is elsewhere). |
-| —                             | `MCP_AUTO_OPEN_ENABLED`  | Browser auto-open **and** non-TTY interactive-OAuth admit: `true` (allow interactive OAuth without a TTY **and** force-open the browser — same as the web launcher), `false` (never open), unset (open on a TTY unless `VITEST` is set). For CI that must not hang, prefer `--stored-auth-only`.                                             |
+| —                             | `MCP_AUTO_OPEN_ENABLED`  | Browser auto-open **and** non-TTY interactive-OAuth admit: `true` (allow interactive OAuth without a TTY **and** force-open the browser — same as the web launcher), `false` (never open), unset (open on a TTY unless `VITEST` is set). The authorization URL is always printed first; an open that fails, or does not launch within 5s, adds a stderr line saying to open it by hand. For CI that must not hang, prefer `--stored-auth-only`.                                             |
 
 **Example** — list tools on an OAuth-protected server using stored tokens and CIMD from the command line:
 

clients/cli/__tests__/cli-oauth-navigation.test.ts

@@ -202,7 +202,7 @@ describe("createCliOAuthNavigation", () => {
     expect(lines.join("")).not.toContain("\u001b]8;;");
   });
 
-  it("swallows browser-open failures after printing the URL", async () => {
+  it("tells the user to open the URL manually when the browser open fails", async () => {
     const lines: string[] = [];
     const openBrowser = vi.fn().mockRejectedValue(new Error("no browser"));
     const nav = createCliOAuthNavigation({
@@ -214,8 +214,29 @@ describe("createCliOAuthNavigation", () => {
       autoOpenEnabled: true,
     });
     nav.navigateToAuthorization(new URL("https://as.example/a"));
-    await vi.waitFor(() => expect(openBrowser).toHaveBeenCalledOnce());
-    expect(lines.join("")).toContain("Please navigate to:");
+    await vi.waitFor(() => expect(lines).toHaveLength(2));
+    expect(lines).toEqual([
+      "Please navigate to: https://as.example/a\n",
+      "Could not open a browser automatically (no browser). Open the URL above manually.\n",
+    ]);
+  });
+
+  it("reports a non-Error browser-open rejection verbatim", async () => {
+    const lines: string[] = [];
+    const openBrowser = vi.fn().mockRejectedValue("spawn failed");
+    const nav = createCliOAuthNavigation({
+      isTTY: true,
+      noColorEnv: "1",
+      write: (line) => lines.push(line),
+      openBrowser,
+      autoOpenControl: { armed: true },
+      autoOpenEnabled: true,
+    });
+    nav.navigateToAuthorization(new URL("https://as.example/a"));
+    await vi.waitFor(() => expect(lines).toHaveLength(2));
+    expect(lines[1]).toBe(
+      "Could not open a browser automatically (spawn failed). Open the URL above manually.\n",
+    );
   });
 
   it("writes to stderr and uses openUrl by default when armed on a TTY", async () => {

clients/cli/__tests__/open-url.test.ts

@@ -1,4 +1,4 @@
-import { beforeEach, describe, expect, it, vi } from "vitest";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
 const openMock = vi.fn().mockResolvedValue(undefined);
 
@@ -15,6 +15,10 @@ describe("openUrl", () => {
     openMock.mockResolvedValue(undefined);
   });
 
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
   it("forwards a string URL to open", async () => {
     const { openUrl } = await import("../src/open-url.js");
     await openUrl("https://example.com/auth");
@@ -28,4 +32,32 @@ describe("openUrl", () => {
       "https://example.com/callback?code=1",
     );
   });
+
+  it("rejects when the opener rejects", async () => {
+    openMock.mockRejectedValue(new Error("xdg-open not found"));
+    const { openUrl } = await import("../src/open-url.js");
+    await expect(openUrl("https://example.com/auth")).rejects.toThrow(
+      "xdg-open not found",
+    );
+  });
+
+  it("rejects when the opener does not settle within the timeout", async () => {
+    vi.useFakeTimers();
+    openMock.mockReturnValue(new Promise(() => {}));
+    const { openUrl } = await import("../src/open-url.js");
+    const pending = openUrl("https://example.com/auth", 2_000);
+    const assertion = expect(pending).rejects.toThrow(
+      "browser did not open within 2s",
+    );
+    await vi.advanceTimersByTimeAsync(2_000);
+    await assertion;
+  });
+
+  it("clears its timer once the opener resolves", async () => {
+    vi.useFakeTimers();
+    const { openUrl, OPEN_URL_TIMEOUT_MS } = await import("../src/open-url.js");
+    await openUrl("https://example.com/auth");
+    expect(vi.getTimerCount()).toBe(0);
+    expect(OPEN_URL_TIMEOUT_MS).toBe(5_000);
+  });
 });

clients/cli/src/cli-oauth-navigation.ts

@@ -121,8 +121,13 @@ export function createCliOAuthNavigation(
 
     try {
       await (options.openBrowser ?? openUrl)(href);
-    } catch {
-      // URL already printed; browser open is best-effort.
+    } catch (error) {
+      // URL already printed; browser open is best-effort — but say so, or a
+      // failed open reads as the flow silently waiting on a browser (#2410).
+      const reason = error instanceof Error ? error.message : String(error);
+      write(
+        `Could not open a browser automatically (${reason}). Open the URL above manually.\n`,
+      );
     }
   });
 }

clients/cli/src/open-url.ts

@@ -1,6 +1,37 @@
 import open from "open";
 
-/** Open a URL in the user's default browser (best-effort). */
-export async function openUrl(url: string | URL): Promise<void> {
-  await open(typeof url === "string" ? url : url.href);
+/**
+ * How long {@link openUrl} waits for the opener before giving up. `open`
+ * resolves once it has launched the platform opener, but before that it may
+ * probe the environment (WSL's default browser, the PowerShell path), and on a
+ * headless box or a container any of those can stall. The OAuth flow itself
+ * does not wait on this (`CallbackNavigation` fires its callback and discards
+ * the promise); the timeout exists so a stalled open settles as a failure and
+ * the caller's "open the URL manually" line is actually printed (#2410).
+ */
+export const OPEN_URL_TIMEOUT_MS = 5_000;
+
+/**
+ * Open a URL in the user's default browser (best-effort). Rejects when the
+ * opener fails or does not launch within `timeoutMs`; callers print the URL
+ * themselves and decide what to tell the user.
+ */
+export async function openUrl(
+  url: string | URL,
+  timeoutMs: number = OPEN_URL_TIMEOUT_MS,
+): Promise<void> {
+  const href = typeof url === "string" ? url : url.href;
+  let timer: ReturnType<typeof setTimeout> | undefined;
+  const timeout = new Promise<never>((_, reject) => {
+    timer = setTimeout(
+      () =>
+        reject(new Error(`browser did not open within ${timeoutMs / 1000}s`)),
+      timeoutMs,
+    );
+  });
+  try {
+    await Promise.race([open(href), timeout]);
+  } finally {
+    clearTimeout(timer);
+  }
 }