fix(auth): don't crash when no browser can be opened

src/auth/oauthServer.ts

@@ -113,8 +113,11 @@ export class OAuthServer {
       // popup takes both away. Going through the helper also puts this call
       // behind CAPY_WEB_NO_OPEN for the first time; before, a suite or CI run
       // that reached authentication opened the developer's own browser.
-      void openScreen(authUrl, { kind: 'handoff' }).then((plan) => {
-        if (plan.via !== 'suppressed') console.log(`✓ Opened browser for authentication`);
+      // Only claim the browser opened once it actually did. When it could not
+      // (no `xdg-open` in a VM or container), say nothing more: the link is
+      // already on screen above, and the callback server keeps waiting for it.
+      void openScreen(authUrl, { kind: 'handoff' }).then((result) => {
+        if (result.opened) console.log(`✓ Opened browser for authentication`);
       });
 
       const timeout = setTimeout(() => {

src/ui/openScreen.ts

@@ -340,29 +340,91 @@ export function planOpen(url: string, opts: PlanOptions): OpenPlan {
  * caller before this runs, so a browser that will not start is an inconvenience
  * and not a failure of the flow.
  */
-export async function openScreen(url: string, opts: PlanOptions): Promise<OpenPlan> {
+export async function openScreen(
+  url: string,
+  opts: PlanOptions,
+  deps: OpenDeps = {},
+): Promise<OpenPlan & { readonly opened: boolean }> {
   const plan = planOpen(url, opts);
+  const opened = await carryOut(plan, url, deps);
+  return { ...plan, opened };
+}
+
+/**
+ * A started process, as far as this module needs to know: Node's ChildProcess
+ * fits. `once` is optional because a stand-in (a test double, or a launcher
+ * that hands back nothing useful) may not have it; that counts as started.
+ */
+export interface Launchable {
+  readonly pid?: number;
+  once?(event: 'error', listener: (err: Error) => void): unknown;
+  unref?(): unknown;
+}
+
+/** Injection seams, so tests can stand in for a missing launcher without starting one. */
+export interface OpenDeps {
+  readonly spawnApp?: (exec: string, args: readonly string[]) => Launchable;
+  readonly openDefault?: (url: string) => Promise<Launchable>;
+}
+
+/**
+ * True when the process started, false when it could not be started (ENOENT:
+ * no `xdg-open` in a VM or container, no Chrome binary).
+ *
+ * Attaching the `error` listener is the load-bearing part: Node reports a
+ * failed spawn as an `error` event on the next tick, and an `error` event with
+ * no listener throws after every surrounding try/catch has returned, taking the
+ * whole CLI down. The listener stays attached, so a later error is harmless too.
+ *
+ * Whether it started is read from `pid`, which Node leaves undefined when the
+ * spawn failed — known synchronously, with no wait on an event. A stand-in with
+ * no `pid` field at all counts as started.
+ */
+export function launched(child: Launchable): boolean {
+  child?.once?.('error', () => {
+    /* reported through the return value; see above */
+  });
+  if (!child || !('pid' in child)) return true;
+  return typeof child.pid === 'number';
+}
+
+async function carryOut(plan: OpenPlan, url: string, deps: OpenDeps): Promise<boolean> {
+  if (plan.via === 'suppressed') return false;
+  if (plan.via === 'default-browser') return fallback(url, deps);
+  // Detached: the window outlives the CLI process, which is the whole point
+  // for an ending page served just before exit.
+  const started = await tryLaunch(() =>
+    (deps.spawnApp ?? spawnDetached)(plan.exec, plan.args),
+  );
+  return started ? true : fallback(url, deps);
+}
+
+function spawnDetached(exec: string, args: readonly string[]): Launchable {
+  return spawn(exec, [...args], { detached: true, stdio: 'ignore' });
+}
+
+async function tryLaunch(start: () => Launchable | Promise<Launchable>): Promise<boolean> {
   try {
-    if (plan.via === 'app-window') {
-      // Detached: the window outlives the CLI process, which is the whole point
-      // for an ending page served just before exit.
-      const child = spawn(plan.exec, plan.args, { detached: true, stdio: 'ignore' });
-      child.on('error', () => void fallback(url));
-      child.unref();
-    } else if (plan.via === 'default-browser') {
-      await fallback(url);
-    }
+    // No `await` between a synchronous spawn and `launched`: the failure
+    // arrives on the next tick, and an await here could let it land before the
+    // listener does. An async starter (the `open` package) returns its child
+    // in the same microtask drain, which still runs ahead of that tick.
+    const started = start();
+    const child = started instanceof Promise ? await started : started;
+    const ok = launched(child);
+    if (ok) child.unref?.();
+    return ok;
   } catch {
     /* best-effort; the printed URL is the real fallback */
+    return false;
   }
-  return plan;
 }
 
-async function fallback(url: string): Promise<void> {
-  try {
-    const open = (await import('open')).default;
-    await open(url);
-  } catch {
-    /* best-effort */
-  }
+async function defaultOpen(url: string): Promise<Launchable> {
+  const open = (await import('open')).default;
+  return open(url);
+}
+
+function fallback(url: string, deps: OpenDeps): Promise<boolean> {
+  return tryLaunch(() => (deps.openDefault ?? defaultOpen)(url));
 }

tests/ui/openScreen.test.ts

@@ -9,7 +9,9 @@
  * thin part that carries the plan out.
  */
 import { describe, test, expect } from 'bun:test';
-import { planOpen, type DefaultBrowser } from '../../src/ui/openScreen';
+import { spawn } from 'node:child_process';
+import { EventEmitter } from 'node:events';
+import { planOpen, openScreen, launched, type DefaultBrowser, type Launchable } from '../../src/ui/openScreen';
 
 const CHROME: DefaultBrowser = {
   family: 'chromium',
@@ -92,3 +94,75 @@ describe('which window a screen opens in', () => {
     expect(plan.args.filter((a) => a.includes('127.0.0.1'))).toHaveLength(1);
   });
 });
+
+/**
+ * Carrying the plan out when there is no browser to start — a VM, a container,
+ * a server over SSH. The defect being guarded is Node's own: a failed spawn is
+ * reported as an `error` event a tick later, and an `error` event with no
+ * listener throws after every try/catch has returned and kills the CLI.
+ */
+describe('opening when no browser can start', () => {
+  const MISSING = 'capy-test-no-such-launcher-7f3a';
+  const dialog = { kind: 'dialog' as const, browser: CHROME, env: env() };
+  const handoff = { kind: 'handoff' as const, env: env() };
+
+  /**
+   * How Node reports a missing executable: no throw, `pid` left undefined, and
+   * an `error` event on the next tick. An EventEmitter's unhandled `error`
+   * throws, so if the listener were missing this would fail the run exactly as
+   * it killed the CLI. Built by hand because Bun (which runs this suite) throws
+   * synchronously from `spawn` instead, which is not the path the CLI takes.
+   */
+  const nodeStyleMissing = (): Launchable => {
+    const child = Object.assign(new EventEmitter(), { pid: undefined });
+    process.nextTick(() =>
+      child.emit('error', Object.assign(new Error(`spawn ${MISSING} ENOENT`), { code: 'ENOENT' })),
+    );
+    return child;
+  };
+  /** Mirrors the `open` package: start inside an async function, return the child. */
+  const openMissing = async (): Promise<Launchable> => {
+    await Promise.resolve();
+    return nodeStyleMissing();
+  };
+
+  test('a missing launcher resolves as not opened instead of crashing', async () => {
+    const result = await openScreen(URL, handoff, { openDefault: openMissing });
+    expect(result.via).toBe('default-browser');
+    expect(result.opened).toBe(false);
+  });
+
+  test('a missing app-window binary falls back, and reports not opened when that fails too', async () => {
+    const result = await openScreen(URL, dialog, {
+      spawnApp: nodeStyleMissing,
+      openDefault: openMissing,
+    });
+    expect(result.via).toBe('app-window');
+    expect(result.opened).toBe(false);
+  });
+
+  test('a launcher that throws while starting is not opened', async () => {
+    const result = await openScreen(URL, handoff, {
+      openDefault: () => Promise.reject(new Error('boom')),
+    });
+    expect(result.opened).toBe(false);
+  });
+
+  test('a launcher that starts is opened', async () => {
+    const result = await openScreen(URL, handoff, {
+      openDefault: async () => spawn(process.execPath, ['-e', ''], { stdio: 'ignore' }),
+    });
+    expect(result.opened).toBe(true);
+  });
+
+  test('suppressed never starts anything and is not opened', async () => {
+    const result = await openScreen(URL, { kind: 'handoff', env: env({ CAPY_WEB_NO_OPEN: '1' }) }, {
+      openDefault: () => Promise.reject(new Error('must not be called')),
+    });
+    expect(result).toEqual({ via: 'suppressed', opened: false });
+  });
+
+  test('launched() treats an object without events as started', async () => {
+    expect(launched({})).toBe(true);
+  });
+});