fix(cli): catch an opener that cannot be spawned instead of crashing

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

@@ -1,6 +1,22 @@
+import { EventEmitter } from "node:events";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
-const openMock = vi.fn().mockResolvedValue(undefined);
+/**
+ * A stand-in for the `ChildProcess` `open` resolves with. Like a real spawn,
+ * it reports the outcome on a later `process.nextTick`: `'spawn'` when the
+ * opener launched, `'error'` when it could not be spawned.
+ */
+function fakeChild(outcome: "spawn" | Error = "spawn"): EventEmitter {
+  const child = new EventEmitter();
+  process.nextTick(() =>
+    outcome === "spawn" ? child.emit("spawn") : child.emit("error", outcome),
+  );
+  return child;
+}
+
+const openMock = vi.fn<(...args: unknown[]) => Promise<EventEmitter>>(
+  async () => fakeChild(),
+);
 
 vi.mock("open", () => ({
   default: (...args: unknown[]) => openMock(...args),
@@ -12,7 +28,7 @@ vi.unmock("../src/open-url.js");
 describe("openUrl", () => {
   beforeEach(() => {
     openMock.mockClear();
-    openMock.mockResolvedValue(undefined);
+    openMock.mockImplementation(async () => fakeChild());
   });
 
   afterEach(() => {
@@ -41,6 +57,45 @@ describe("openUrl", () => {
     );
   });
 
+  it("rejects when the opener cannot be spawned, instead of crashing", async () => {
+    const enoent = Object.assign(new Error("spawn open ENOENT"), {
+      code: "ENOENT",
+    });
+    openMock.mockImplementation(async () => fakeChild(enoent));
+    const { openUrl } = await import("../src/open-url.js");
+    await expect(openUrl("https://example.com/auth")).rejects.toThrow(
+      "spawn open ENOENT",
+    );
+  });
+
+  it("catches a spawn failure when entered from a timer or I/O callback", async () => {
+    // From a macrotask, Node drains process.nextTick BEFORE promise reactions,
+    // and on macOS `open` reaches spawn() without an earlier await, so a
+    // listener chained on its promise alone would attach too late.
+    const enoent = Object.assign(new Error("spawn open ENOENT"), {
+      code: "ENOENT",
+    });
+    openMock.mockImplementation(async () => fakeChild(enoent));
+    const { openUrl } = await import("../src/open-url.js");
+    const outcome = await new Promise<unknown>((resolve) => {
+      setImmediate(() => {
+        openUrl("https://example.com/auth").then(
+          () => resolve("resolved"),
+          (err: unknown) => resolve(err),
+        );
+      });
+    });
+    expect(outcome).toBe(enoent);
+  });
+
+  it("absorbs an opener error that arrives after launch", async () => {
+    let child: EventEmitter | undefined;
+    openMock.mockImplementation(async () => (child = fakeChild()));
+    const { openUrl } = await import("../src/open-url.js");
+    await openUrl("https://example.com/auth");
+    expect(() => child!.emit("error", new Error("late"))).not.toThrow();
+  });
+
   it("rejects when the opener does not settle within the timeout", async () => {
     vi.useFakeTimers();
     openMock.mockReturnValue(new Promise(() => {}));

clients/cli/src/open-url.ts

@@ -1,3 +1,4 @@
+import type { ChildProcess } from "node:child_process";
 import open from "open";
 
 /**
@@ -11,10 +12,35 @@ import open from "open";
  */
 export const OPEN_URL_TIMEOUT_MS = 5_000;
 
+/**
+ * Settle once the opener process has actually launched. `open` resolves with
+ * the `ChildProcess` as soon as it has called `spawn()`, and a spawn failure
+ * (the opener binary missing from `PATH`: `ENOENT`) arrives afterwards as an
+ * `'error'` event on that child. `open` only listens for it when called with
+ * `{ wait: true }`, which is unusable here: on macOS it adds `open -W`, which
+ * waits for the browser to quit. Unlistened, the event crashes the process
+ * instead of reaching the caller's fallback (#2531).
+ *
+ * Chained directly onto `open`'s promise, this runs as a microtask. That is
+ * only ahead of the `process.nextTick` emitting `'error'` or `'spawn'` when
+ * `open` itself ran inside a microtask: from a timer or I/O callback, Node
+ * drains `nextTick` before promise reactions, and on macOS `open` reaches
+ * `spawn()` with no earlier `await`. So {@link openUrl} calls `open` from a
+ * queued microtask, which puts the spawn and this listener in the same drain
+ * whatever the caller's context. The listener stays attached (`on`, not
+ * `once`) so an error after settling is still absorbed.
+ */
+function launched(child: ChildProcess): Promise<void> {
+  return new Promise<void>((resolve, reject) => {
+    child.on("error", reject);
+    child.once("spawn", () => resolve());
+  });
+}
+
 /**
  * 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.
+ * opener fails, cannot be spawned, 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,
@@ -30,7 +56,10 @@ export async function openUrl(
     );
   });
   try {
-    await Promise.race([open(href), timeout]);
+    const opened = Promise.resolve()
+      .then(() => open(href))
+      .then(launched);
+    await Promise.race([opened, timeout]);
   } finally {
     clearTimeout(timer);
   }