fix daemon browser opener on Windows (#953)

Co-authored-by: Sapientropic <194952804+Sapientropic@users.noreply.github.com>

apps/daemon/src/browser-open.ts

@@ -0,0 +1,78 @@
+import { spawn as nodeSpawn } from 'node:child_process';
+import type { ChildProcess, SpawnOptions } from 'node:child_process';
+
+type SupportedPlatform = NodeJS.Platform;
+
+export type BrowserOpenInvocation = {
+  command: string;
+  args: string[];
+  options: SpawnOptions;
+};
+
+type OpenBrowserDeps = {
+  platform?: SupportedPlatform;
+  spawn?: (command: string, args: string[], options: SpawnOptions) => ChildProcess;
+  warn?: (message: string) => void;
+  env?: NodeJS.ProcessEnv;
+};
+
+function quoteWindowsCommandArg(value: string, { force = false }: { force?: boolean } = {}): string {
+  if (value.length === 0) return '""';
+  if (!force && !/[\s"&<>|^%]/.test(value)) return value;
+  const escaped = value.replace(/"/g, '""').replace(/%/g, '"^%"');
+  return `"${escaped}"`;
+}
+
+export function createBrowserOpenInvocation(
+  platform: SupportedPlatform,
+  url: string,
+  env: NodeJS.ProcessEnv = process.env,
+): BrowserOpenInvocation {
+  if (platform === 'win32') {
+    const comspec = env.ComSpec || env.COMSPEC || 'cmd.exe';
+    // `start` is a cmd.exe builtin on Windows, not a real executable. The empty
+    // title argument keeps cmd from treating the URL itself as the window title.
+    // Match @open-design/platform's cmd.exe shim shape: Node's default Windows
+    // argv quoting uses backslash escapes that cmd.exe does not understand, so
+    // the inner command must be wrapped for `/s /c` and passed verbatim.
+    const inner = [
+      'start',
+      quoteWindowsCommandArg(''),
+      quoteWindowsCommandArg(url, { force: true }),
+    ].join(' ');
+    return {
+      command: comspec,
+      args: ['/d', '/s', '/c', `"${inner}"`],
+      options: { detached: true, stdio: 'ignore', windowsHide: true, windowsVerbatimArguments: true },
+    };
+  }
+
+  return {
+    command: platform === 'darwin' ? 'open' : 'xdg-open',
+    args: [url],
+    options: { detached: true, stdio: 'ignore' },
+  };
+}
+
+export function openBrowser(url: string, deps: OpenBrowserDeps = {}): ChildProcess | null {
+  const platform = deps.platform ?? process.platform;
+  const spawn = deps.spawn ?? nodeSpawn;
+  const warn = deps.warn ?? ((message: string) => console.warn(message));
+  const invocation = createBrowserOpenInvocation(platform, url, deps.env);
+
+  try {
+    const child = spawn(invocation.command, invocation.args, invocation.options);
+    // Browser opening is best-effort. A missing opener must not crash the daemon
+    // after the server has already started and printed its URL.
+    child.on('error', (error) => {
+      const detail = error instanceof Error ? error.message : String(error);
+      warn(`[od] failed to open browser: ${detail}`);
+    });
+    child.unref();
+    return child;
+  } catch (error) {
+    const detail = error instanceof Error ? error.message : String(error);
+    warn(`[od] failed to open browser: ${detail}`);
+    return null;
+  }
+}

apps/daemon/src/cli.ts

@@ -5,6 +5,7 @@ import { runLiveArtifactsMcpServer } from './mcp-live-artifacts-server.js';
 import { runConnectorsToolCli } from './tools-connectors-cli.js';
 import { runLiveArtifactsToolCli } from './tools-live-artifacts-cli.js';
 import { splitResearchSubcommand } from './research/cli-args.js';
+import { openBrowser } from './browser-open.js';
 
 const argv = process.argv.slice(2);
 
@@ -173,12 +174,7 @@ startServer({ port, host, returnServer: true }).then((started) => {
   process.on('SIGTERM', stop);
   console.log(`[od] listening on ${url}`);
   if (open) {
-    const opener = process.platform === 'darwin' ? 'open'
-      : process.platform === 'win32' ? 'start'
-      : 'xdg-open';
-    import('node:child_process').then(({ spawn }) => {
-      spawn(opener, [url], { detached: true, stdio: 'ignore' }).unref();
-    });
+    openBrowser(url);
   }
 });
 }

apps/daemon/tests/browser-open.test.ts

@@ -0,0 +1,40 @@
+import { EventEmitter } from 'node:events';
+import type { ChildProcess } from 'node:child_process';
+import { describe, expect, it, vi } from 'vitest';
+
+import { createBrowserOpenInvocation, openBrowser } from '../src/browser-open.js';
+
+describe('browser open helper', () => {
+  it('opens URLs on Windows through cmd.exe instead of spawning the shell builtin directly', () => {
+    const invocation = createBrowserOpenInvocation('win32', 'http://127.0.0.1:7456/');
+
+    expect(invocation.command).toMatch(/cmd(?:\.exe)?$/i);
+    expect(invocation.args.slice(0, 3)).toEqual(['/d', '/s', '/c']);
+    expect(invocation.args[3]).toContain('start "" "http://127.0.0.1:7456/"');
+    expect(invocation.options).toMatchObject({ windowsVerbatimArguments: true });
+  });
+
+  it('escapes cmd.exe metacharacters before opening Windows URLs', () => {
+    const invocation = createBrowserOpenInvocation('win32', 'https://example.test/a b?q=%USERPROFILE%&name="A"');
+
+    expect(invocation.args[3]).toMatch(/^"start "" /);
+    expect(invocation.args[3]).toContain('"^%"USERPROFILE"^%"');
+    expect(invocation.args[3]).toContain('name=""A""');
+  });
+
+  it('handles opener spawn errors without crashing the daemon', () => {
+    const child = new EventEmitter() as ChildProcess;
+    child.unref = vi.fn() as ChildProcess['unref'];
+    const spawn = vi.fn(() => child);
+    const warn = vi.fn();
+
+    openBrowser('http://127.0.0.1:7456/', {
+      platform: 'linux',
+      spawn,
+      warn,
+    });
+
+    expect(() => child.emit('error', new Error('spawn xdg-open ENOENT'))).not.toThrow();
+    expect(warn).toHaveBeenCalledWith(expect.stringContaining('failed to open browser'));
+  });
+});