fix(cli): serve gains --no-open, no bare stack after headless browser-open failure

.changeset/cli-serve-no-open-4924.md

@@ -0,0 +1,28 @@
+---
+'@object-ui/cli': patch
+---
+
+`objectui serve` gains `--no-open`, matching the flag `objectui dev` already ships, and no
+longer prints a bare `Error: spawn xdg-open ENOENT` stack after its success banner in a
+headless environment.
+
+**`--no-open`.** `serve` hardcoded Vite's `open: true` and its only options were
+`--port`/`--host` — `dev` already had `--no-open` (`options.open !== false`), so the same
+invocation behaved inconsistently across the two commands. `serve` now threads the same
+flag the same way; the default is unchanged — with the flag omitted, `serve` still opens a
+browser exactly as it always has (objectui#4924).
+
+**The headless spawn failure.** Vite's own browser-open step already catches a failed
+`open(url)`, but reports it via `logger.error(err.stack || err.message)` with no
+`{ timestamp: true }`, so the default logger prints the bare Node `ChildProcess` stack with
+no `[vite]` prefix and no context — and because that promise chain is fire-and-forget from
+`server.listen()`, it lands *after* the "✓ Server started successfully!" banner, reading
+like a crash even though the server is fine. There's nothing in `serve.ts` to try/catch —
+the error never leaves Vite. `serve` now supplies a `customLogger` that wraps Vite's default
+logger and replaces exactly that message with a short, contextual line naming the missing
+opener binary (`(could not open the browser automatically — 'xdg-open' is not available in
+this environment; open the URL above manually)`); every other log call — including
+unrelated errors — passes through unchanged.
+
+Sibling card objectui#4923 (project-root detection on the same command) is intentionally
+untouched here; it is a separate defect with a separate PR.

packages/cli/src/__tests__/serve.test.ts

@@ -0,0 +1,198 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * Pins two independent fixes to `serve` (objectui#4924):
+ *
+ * 1. `--no-open` — `serve` hardcoded Vite's `open: true` and had no flag to
+ *    turn it off, unlike `dev` (which already ships `--no-open`). The
+ *    default must not change: `serve` opens a browser today and must keep
+ *    doing so when the flag is absent.
+ * 2. The headless spawn failure — when the browser-open step fails (no
+ *    `xdg-open`/opener binary available), Vite's own `openBrowser()`
+ *    reports it via `logger.error(err.stack || err.message)` with no
+ *    `{ timestamp: true }`, printing a bare Node stack with no `[vite]`
+ *    prefix, no context, and — because that promise chain is fire-and-forget
+ *    from `server.listen()` — on a *later* tick than our own success
+ *    banner, reading like a crash even though the server is fine.
+ *
+ * ## Why there is no `vi.mock('vite')` here
+ *
+ * `packages/**\/*.test.ts` runs in the root `unit` Vitest project with
+ * `isolate: false` (a shared module graph per worker, spanning every
+ * package's `*.test.ts`, not just this one) — see the same note in
+ * `project-source.test.ts`. Mocking `vite` there would leak into every
+ * other file sharing the worker. So instead of letting `serve()` run to
+ * completion against a real (or mocked) Vite server, both fixes are pinned
+ * on the two pure, exported seams `serve.ts` pulls the logic into:
+ * `resolveServeOpenOption` (the config value threading) and
+ * `createServeLogger` (the spawn-failure interception) — neither imports or
+ * touches the `vite` module's runtime behavior, so nothing needs mocking.
+ *
+ * `createServeLogger`'s fixture messages are not invented: they are the
+ * literal shape a Node `ChildProcess` ENOENT error's `.stack` takes,
+ * confirmed by both reading Vite 8.2.1's bundled
+ * `src/node/server/openBrowser.ts` (`startBrowserProcess` catches the
+ * failed `open(url)` and calls exactly `logger.error(err.stack ||
+ * err.message)`) and a live repro: `createServer({ server: { open: true },
+ * customLogger })` with `BROWSER` pointed at a nonexistent binary produced
+ * verbatim
+ *
+ *   Error: spawn objectui-repro-nonexistent-opener ENOENT
+ *       at ChildProcess._handle.onexit (node:internal/child_process:285:19)
+ *       ...
+ *
+ * after the "✓ Server started successfully!" banner — exactly the ordering
+ * and shape objectui#4924 reports, and exactly what `createServeLogger`
+ * turns into the short contextual line below.
+ */
+
+import { describe, expect, it, vi } from 'vitest';
+import type { Logger } from 'vite';
+
+import { createServeLogger, resolveServeOpenOption } from '../commands/serve.js';
+
+describe('resolveServeOpenOption', () => {
+  it('keeps the default of opening a browser when no flag is passed (options.open undefined)', () => {
+    expect(resolveServeOpenOption(undefined)).toBe(true);
+  });
+
+  it('keeps opening a browser when the flag resolves truthy (commander default for --no-open absent)', () => {
+    expect(resolveServeOpenOption(true)).toBe(true);
+  });
+
+  it('suppresses the open when --no-open sets options.open to false', () => {
+    expect(resolveServeOpenOption(false)).toBe(false);
+  });
+});
+
+/** A minimal stand-in satisfying Vite's `Logger` shape, with spies on every method. */
+function makeFakeBaseLogger(): Logger & {
+  errorCalls: Array<[string, unknown]>;
+} {
+  const errorCalls: Array<[string, unknown]> = [];
+  return {
+    errorCalls,
+    hasWarned: false,
+    info: vi.fn(),
+    warn: vi.fn(function (this: Logger) {
+      this.hasWarned = true;
+    }),
+    warnOnce: vi.fn(),
+    error: vi.fn(function (this: Logger, msg: string, opts?: unknown) {
+      this.hasWarned = true;
+      errorCalls.push([msg, opts]);
+    }),
+    clearScreen: vi.fn(),
+    hasErrorLogged: vi.fn(() => false),
+  };
+}
+
+describe('createServeLogger', () => {
+  it('replaces the bare browser-open ENOENT stack with a short contextual line instead of forwarding it to the base logger', () => {
+    const base = makeFakeBaseLogger();
+    const logger = createServeLogger(base);
+    const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+
+    const enoentStack =
+      'Error: spawn xdg-open ENOENT\n' +
+      '    at ChildProcess._handle.onexit (node:internal/child_process:285:19)\n' +
+      '    at onErrorNT (node:internal/child_process:483:16)\n' +
+      '    at process.processTicksAndRejections (node:internal/process/task_queues:89:21)';
+
+    logger.error(enoentStack);
+
+    // The raw stack must never reach the base logger (no bare stack after
+    // the success banner).
+    expect(base.errorCalls).toHaveLength(0);
+    // Something contextual replaces it, naming the missing opener.
+    expect(logSpy).toHaveBeenCalledTimes(1);
+    const printed = logSpy.mock.calls[0]?.[0] as string;
+    expect(printed).toContain('xdg-open');
+    expect(printed).not.toContain('at ChildProcess._handle.onexit');
+
+    logSpy.mockRestore();
+  });
+
+  it('still detects the failure when a different opener binary is named (Vite reads BROWSER; not Linux/xdg-open-specific)', () => {
+    const base = makeFakeBaseLogger();
+    const logger = createServeLogger(base);
+    const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+
+    logger.error('Error: spawn open ENOENT\n    at ChildProcess._handle.onexit (node:internal/child_process:285:19)');
+
+    expect(base.errorCalls).toHaveLength(0);
+    expect(logSpy).toHaveBeenCalledTimes(1);
+    expect(logSpy.mock.calls[0]?.[0] as string).toContain('open');
+
+    logSpy.mockRestore();
+  });
+
+  it('passes an unrelated error straight through to the base logger, unmodified (counter-probe: the interception is selective, not a blanket error swallow)', () => {
+    const base = makeFakeBaseLogger();
+    const logger = createServeLogger(base);
+    const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+
+    const realError = 'Error: Port 5173 is already in use';
+    const opts = { timestamp: true };
+    logger.error(realError, opts);
+
+    expect(base.errorCalls).toEqual([[realError, opts]]);
+    // No extra contextual console.log for a message that isn't the
+    // browser-open failure.
+    expect(logSpy).not.toHaveBeenCalled();
+
+    logSpy.mockRestore();
+  });
+
+  it('passes a spawn ENOENT for something that is not the browser opener through unmodified (counter-probe: pattern is anchored, not a bare "ENOENT" substring match)', () => {
+    const base = makeFakeBaseLogger();
+    const logger = createServeLogger(base);
+    const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+
+    // Not vite's `Error: spawn <cmd> ENOENT` shape (no "Error: spawn" prefix) —
+    // must not be swallowed just because "ENOENT" appears somewhere in it.
+    const unrelated = 'Failed to read config: open /tmp/vite.config.ts ENOENT';
+    logger.error(unrelated);
+
+    expect(base.errorCalls).toEqual([[unrelated, undefined]]);
+    expect(logSpy).not.toHaveBeenCalled();
+
+    logSpy.mockRestore();
+  });
+
+  it('delegates info/warn/warnOnce/clearScreen/hasErrorLogged to the base logger unchanged', () => {
+    const base = makeFakeBaseLogger();
+    const logger = createServeLogger(base);
+
+    logger.info('hello', { timestamp: true });
+    expect(base.info).toHaveBeenCalledWith('hello', { timestamp: true });
+
+    logger.warn('careful');
+    expect(base.warn).toHaveBeenCalledWith('careful', undefined);
+
+    logger.warnOnce('once');
+    expect(base.warnOnce).toHaveBeenCalledWith('once', undefined);
+
+    logger.clearScreen('info');
+    expect(base.clearScreen).toHaveBeenCalledWith('info');
+
+    const err = new Error('x');
+    logger.hasErrorLogged(err);
+    expect(base.hasErrorLogged).toHaveBeenCalledWith(err);
+  });
+
+  it('mirrors hasWarned live from the base logger rather than freezing it at construction time', () => {
+    const base = makeFakeBaseLogger();
+    const logger = createServeLogger(base);
+
+    expect(logger.hasWarned).toBe(false);
+    base.warn('something');
+    expect(logger.hasWarned).toBe(true);
+  });
+});

packages/cli/src/cli.ts

@@ -48,6 +48,7 @@ program
   .argument('[schema]', 'Path to JSON/YAML schema file, or a project / pages directory', 'app.json')
   .option('-p, --port <port>', 'Port to run the server on', '3000')
   .option('-h, --host <host>', 'Host to bind the server to', 'localhost')
+  .option('--no-open', 'Do not open browser automatically')
   .action(async (schema, options) => {
     try {
       await serve(schema, options);

packages/cli/src/commands/serve.ts

@@ -6,7 +6,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import { createServer } from 'vite';
+import { createServer, createLogger, type Logger } from 'vite';
 import react from '@vitejs/plugin-react';
 import { mkdirSync } from 'fs';
 import { join } from 'path';
@@ -19,6 +19,62 @@ import { isWorkspaceRoot, prepareWorkspaceTempApp } from '../utils/workspace-vit
 interface ServeOptions {
   port: string;
   host: string;
+  open?: boolean;
+}
+
+/**
+ * Mirrors `dev`'s existing `open: options.open !== false` (commander's
+ * `--no-open` sets `options.open` to `false`; every other case — the flag
+ * omitted, or `true` — must keep opening a browser, which is the default
+ * `serve` has always had and must not change) — pulled out so it can be
+ * unit-tested without booting a real Vite server (objectui#4924).
+ */
+export function resolveServeOpenOption(open?: boolean): boolean {
+  return open !== false;
+}
+
+/**
+ * Vite's own browser-open step (`startBrowserProcess` in
+ * `src/node/server/openBrowser.ts`) already catches a failed `open(url)` —
+ * but it reports it as `logger.error(err.stack || err.message)` with no
+ * `{ timestamp: true }`, so the default logger prints the bare Node stack
+ * with no `[vite]` prefix and no context. That promise chain is
+ * fire-and-forget from `server.listen()`, so the message lands on a later
+ * tick — after our own success banner — reading like a crash even though
+ * the server is fine. There's nothing to try/catch: the error never leaves
+ * Vite. `customLogger` is the supported interception point, so this wraps
+ * Vite's default logger and replaces exactly that message with a short,
+ * contextual line; every other log call passes through unchanged
+ * (objectui#4924, reproduced live: `Error: spawn xdg-open ENOENT` after
+ * "✓ Server started successfully!" with `BROWSER` pointed at a missing
+ * binary).
+ */
+const BROWSER_OPEN_SPAWN_ENOENT = /^Error: spawn (\S+) ENOENT/;
+
+export function createServeLogger(base: Logger): Logger {
+  return {
+    info: (msg, opts) => base.info(msg, opts),
+    warn: (msg, opts) => base.warn(msg, opts),
+    warnOnce: (msg, opts) => base.warnOnce(msg, opts),
+    error(msg, opts) {
+      const match = BROWSER_OPEN_SPAWN_ENOENT.exec(msg);
+      if (match) {
+        const [, command] = match;
+        console.log(
+          chalk.dim(
+            `  (could not open the browser automatically — '${command}' is not available in this environment; open the URL above manually)`
+          )
+        );
+        return;
+      }
+      base.error(msg, opts);
+    },
+    clearScreen: (type) => base.clearScreen(type),
+    hasErrorLogged: (error) => base.hasErrorLogged(error),
+    get hasWarned() {
+      return base.hasWarned;
+    },
+  };
 }
 
 export async function serve(schemaPath: string, options: ServeOptions) {
@@ -80,13 +136,14 @@ export async function serve(schemaPath: string, options: ServeOptions) {
     server: {
       port: parseInt(options.port),
       host: options.host,
-      open: true,
+      open: resolveServeOpenOption(options.open),
       fs: {
         // Allow serving the workspace sources the aliases point at
         allow: [cwd],
       },
     },
     plugins: [react()],
+    customLogger: createServeLogger(createLogger()),
     ...workspaceConfig,
   };