fix(mcp): open the login and consent pages in the browser on Windows

.changeset/windows-browser-launch.md

@@ -0,0 +1,5 @@
+---
+'@formio/mcp': patch
+---
+
+Open the portal-login and revisions-consent pages in the browser on Windows. Both pages were launched with `exec('start "<url>"')`, and `start` reads its first quoted argument as a window title, so Windows opened an empty console window titled with the URL and no browser — the tool call then waited out the login timeout with no page in front of the user. Both pages now go through one launcher that runs no shell: `open` on macOS, `xdg-open` on Linux, and `rundll32 url.dll,FileProtocolHandler` on Windows, each handed the URL as its own argument. The consent page also now reports a failed launch on stderr with its URL, as the login page already did, instead of ignoring it.

packages/mcp-server/README.md

@@ -238,7 +238,7 @@ The JWT is cached in `~/.formio/mcp-tokens.json` (mode `0600`), keyed by the res
 
 ### Headless environments
 
-By default the login page is served on an ephemeral port bound to `127.0.0.1` and the server shells out to `open`/`start`/`xdg-open`, which assumes a desktop browser on the same machine.
+By default the login page is served on an ephemeral port bound to `127.0.0.1` and the server launches the default browser with `open` (macOS), `xdg-open` (Linux), or the `url.dll` protocol handler (Windows) — directly, not through a shell — which assumes a desktop browser on the same machine.
 
 Where that assumption doesn't hold — a container, an SSH session, CI — you have three options:
 

packages/mcp-server/src/__tests__/auth-browserless.test.ts

@@ -1,9 +1,9 @@
-import { exec } from 'child_process';
+import { execFile } from 'child_process';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { authenticate } from '../auth.js';
 import { ResolvedFormioConfig } from '../config.js';
 
-vi.mock('child_process', () => ({ exec: vi.fn() }));
+vi.mock('child_process', () => ({ execFile: vi.fn() }));
 
 const CONFIG: ResolvedFormioConfig = {
   baseUrl: 'https://formio.invalid/sub',
@@ -21,7 +21,7 @@ describe('authenticate on a browserless host', () => {
     delete process.env.SSH_CONNECTION;
     delete process.env.SSH_TTY;
     process.env.CI = 'true';
-    vi.mocked(exec).mockReset();
+    vi.mocked(execFile).mockReset();
     vi.spyOn(process.stderr, 'write').mockImplementation((): boolean => true);
   });
 
@@ -48,7 +48,7 @@ describe('authenticate on a browserless host', () => {
   it('never launches a browser', async () => {
     await expect(authenticate(CONFIG)).rejects.toThrow();
 
-    expect(vi.mocked(exec)).not.toHaveBeenCalled();
+    expect(vi.mocked(execFile)).not.toHaveBeenCalled();
   });
 
   it('never binds a port', async () => {

packages/mcp-server/src/__tests__/auth-headless.test.ts

@@ -1,10 +1,11 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
-import { exec } from 'child_process';
+import { execFile } from 'child_process';
 import net from 'node:net';
 import { authenticate } from '../auth.js';
+import { browserLaunchCommand } from '../browser-launch.js';
 import { ResolvedFormioConfig } from '../config.js';
 
-vi.mock('child_process', () => ({ exec: vi.fn() }));
+vi.mock('child_process', () => ({ execFile: vi.fn() }));
 
 // forceBrowser because these tests exercise the browser path itself: the suite
 // runs on CI, which is exactly what browserless detection refuses to launch a
@@ -52,7 +53,7 @@ describe('authenticate in headless environments', () => {
       stderr.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString());
       return true;
     });
-    vi.mocked(exec).mockReset();
+    vi.mocked(execFile).mockReset();
   });
 
   afterEach(() => {
@@ -94,13 +95,15 @@ describe('authenticate in headless environments', () => {
   // The spawn failure was previously swallowed, leaving no clue why nothing
   // opened.
   it('reports a failure to launch the browser instead of swallowing it', async () => {
-    vi.mocked(exec).mockImplementation(((
-      _cmd: string,
+    vi.mocked(execFile).mockImplementation(((
+      _file: string,
+      _args: readonly string[],
+      _options: unknown,
       cb?: (err: Error | null) => void
     ): unknown => {
-      cb?.(new Error('xdg-open ENOENT'));
+      cb?.(new Error('spawn xdg-open ENOENT'));
       return {};
-    }) as unknown as typeof exec);
+    }) as unknown as typeof execFile);
 
     await authenticate(DEFAULT_CONFIG, {
       onReady: async (port) => {
@@ -143,11 +146,13 @@ describe('authenticate in headless environments', () => {
     // Ephemeral, not a fixed port.
     expect(observedPort).toBeGreaterThan(1024);
 
-    const opener =
-      process.platform === 'darwin' ? 'open' : process.platform === 'win32' ? 'start' : 'xdg-open';
-    const command = vi.mocked(exec).mock.calls[0]?.[0] as string;
-    expect(command).toContain(opener);
-    expect(command).toContain(`http://127.0.0.1:${observedPort}/`);
+    // Launched without a shell, the URL as an argument of its own.
+    const loginUrl = `http://127.0.0.1:${observedPort}/`;
+    const { command, args } = browserLaunchCommand(loginUrl);
+    const [file, passedArgs] = vi.mocked(execFile).mock.calls[0] ?? [];
+    expect(file).toBe(command);
+    expect(passedArgs).toEqual(args);
+    expect(passedArgs).toContain(loginUrl);
   });
 
   it('does not reject when the login arrives before the timeout', async () => {

packages/mcp-server/src/__tests__/auth.test.ts

@@ -4,7 +4,7 @@ import { describe, it, expect, vi } from 'vitest';
 import { authenticate } from '../auth.js';
 import { ResolvedFormioConfig } from '../config.js';
 
-vi.mock('child_process', () => ({ exec: vi.fn() }));
+vi.mock('child_process', () => ({ execFile: vi.fn() }));
 
 // forceBrowser because these tests exercise the browser login path itself, and
 // the suite runs on CI — where browserless detection refuses to launch one.

packages/mcp-server/src/__tests__/browser-launch.test.ts

@@ -0,0 +1,84 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { execFile } from 'child_process';
+import { browserLaunchCommand, openInBrowser } from '../browser-launch.js';
+
+vi.mock('child_process', () => ({ execFile: vi.fn() }));
+
+const URL = 'http://127.0.0.1:53555/';
+
+describe('browserLaunchCommand', () => {
+  it('uses open on macOS', () => {
+    expect(browserLaunchCommand(URL, 'darwin')).toEqual({ command: 'open', args: [URL] });
+  });
+
+  it('uses xdg-open on Linux', () => {
+    expect(browserLaunchCommand(URL, 'linux')).toEqual({ command: 'xdg-open', args: [URL] });
+  });
+
+  // `start "<url>"` reads its first quoted argument as a window title, so on
+  // Windows it opened an empty console titled with the URL and no browser.
+  it('hands the URL to the protocol handler on Windows, never to start', () => {
+    const launch = browserLaunchCommand(URL, 'win32');
+    expect(launch).toEqual({
+      command: 'rundll32.exe',
+      args: ['url.dll,FileProtocolHandler', URL],
+    });
+    expect([launch.command, ...launch.args]).not.toContain('start');
+  });
+});
+
+describe('openInBrowser', () => {
+  beforeEach(() => {
+    vi.mocked(execFile).mockReset();
+  });
+
+  // execFile runs no shell, so the URL reaches the opener as one argument and
+  // nothing in it is parsed as a shell token on any platform.
+  it('launches without a shell, passing the URL as its own argument', () => {
+    openInBrowser(URL);
+
+    const { command, args } = browserLaunchCommand(URL);
+    expect(vi.mocked(execFile)).toHaveBeenCalledWith(
+      command,
+      args,
+      expect.objectContaining({ windowsHide: true }),
+      expect.any(Function)
+    );
+  });
+
+  it('reports a launch failure to the caller', () => {
+    vi.mocked(execFile).mockImplementation(((
+      _file: string,
+      _args: readonly string[],
+      _options: unknown,
+      cb: (err: Error | null) => void
+    ): unknown => {
+      cb(new Error('spawn xdg-open ENOENT'));
+      return {};
+    }) as unknown as typeof execFile);
+
+    const onError = vi.fn();
+    openInBrowser(URL, onError);
+
+    expect(onError).toHaveBeenCalledWith(
+      expect.objectContaining({ message: 'spawn xdg-open ENOENT' })
+    );
+  });
+
+  it('does not report success as a failure', () => {
+    vi.mocked(execFile).mockImplementation(((
+      _file: string,
+      _args: readonly string[],
+      _options: unknown,
+      cb: (err: Error | null) => void
+    ): unknown => {
+      cb(null);
+      return {};
+    }) as unknown as typeof execFile);
+
+    const onError = vi.fn();
+    openInBrowser(URL, onError);
+
+    expect(onError).not.toHaveBeenCalled();
+  });
+});

packages/mcp-server/src/__tests__/browser-prompts-launch.test.ts

@@ -0,0 +1,72 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { execFile } from 'child_process';
+import { browserLaunchCommand } from '../browser-launch.js';
+import { requestRevisionsLicenseConsent } from '../revisions/browser-prompts.js';
+
+vi.mock('child_process', () => ({ execFile: vi.fn() }));
+
+function postChoice(port: number, choice: string): Promise<Response> {
+  return fetch(`http://127.0.0.1:${port}/callback`, {
+    method: 'POST',
+    headers: { 'Content-Type': 'application/json' },
+    body: JSON.stringify({ choice }),
+  });
+}
+
+describe('opening the revisions consent page', () => {
+  let stderr: string[];
+
+  beforeEach(() => {
+    stderr = [];
+    vi.spyOn(process.stderr, 'write').mockImplementation((chunk): boolean => {
+      stderr.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString());
+      return true;
+    });
+    vi.mocked(execFile).mockReset();
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it('launches the browser without a shell, the URL as an argument of its own', async () => {
+    let consentUrl = '';
+    await requestRevisionsLicenseConsent('a deployment', 'an action', {
+      onReady: async (port) => {
+        consentUrl = `http://127.0.0.1:${port}/`;
+        await postChoice(port, 'continue');
+      },
+    });
+
+    const { command, args } = browserLaunchCommand(consentUrl);
+    const [file, passedArgs] = vi.mocked(execFile).mock.calls[0] ?? [];
+    expect(file).toBe(command);
+    expect(passedArgs).toEqual(args);
+  });
+
+  // The launch failure was previously ignored, so a user whose browser never
+  // opened had no URL to open by hand.
+  it('names the consent URL when the browser cannot be launched', async () => {
+    vi.mocked(execFile).mockImplementation(((
+      _file: string,
+      _args: readonly string[],
+      _options: unknown,
+      cb?: (err: Error | null) => void
+    ): unknown => {
+      cb?.(new Error('spawn xdg-open ENOENT'));
+      return {};
+    }) as unknown as typeof execFile);
+
+    let consentUrl = '';
+    await requestRevisionsLicenseConsent('a deployment', 'an action', {
+      onReady: async (port) => {
+        consentUrl = `http://127.0.0.1:${port}/`;
+        await postChoice(port, 'continue');
+      },
+    });
+
+    const logged = stderr.join('');
+    expect(logged).toMatch(/could not open a browser/i);
+    expect(logged).toContain(consentUrl);
+  });
+});

packages/mcp-server/src/__tests__/local-server-edges.test.ts

@@ -14,7 +14,7 @@ import { authenticate } from '../auth.js';
 import { ResolvedFormioConfig } from '../config.js';
 import { requestRevisionsLicenseConsent } from '../revisions/browser-prompts.js';
 
-vi.mock('child_process', () => ({ exec: vi.fn() }));
+vi.mock('child_process', () => ({ execFile: vi.fn() }));
 
 const CONFIG: ResolvedFormioConfig = {
   baseUrl: 'https://formio.invalid/sub',

packages/mcp-server/src/auth.ts

@@ -1,6 +1,6 @@
 import express from 'express';
-import { exec } from 'child_process';
 import { browserlessReason, currentBrowserEnvironment } from './browser-availability.js';
+import { openInBrowser } from './browser-launch.js';
 import { ResolvedFormioConfig } from './config.js';
 import { formioRawFetch } from './formio-client.js';
 
@@ -279,19 +279,11 @@ export async function authenticate(
       // no way to reach the login page.
       process.stderr.write(`Form.io login required: ${loginUrl}\n`);
 
-      const openCmd =
-        process.platform === 'darwin'
-          ? 'open'
-          : process.platform === 'win32'
-            ? 'start'
-            : 'xdg-open';
-      exec(`${openCmd} "${loginUrl}"`, (err) => {
-        if (err) {
-          process.stderr.write(
-            `Could not open a browser automatically (${err.message}). ` +
-              `Open the URL above manually to finish signing in.\n`
-          );
-        }
+      openInBrowser(loginUrl, (err) => {
+        process.stderr.write(
+          `Could not open a browser automatically (${err.message}). ` +
+            `Open the URL above manually to finish signing in.\n`
+        );
       });
       options?.onReady?.(port);
     }

packages/mcp-server/src/browser-launch.ts

@@ -0,0 +1,44 @@
+import { execFile } from 'child_process';
+
+export interface BrowserLaunchCommand {
+  command: string;
+  args: readonly string[];
+}
+
+/**
+ * The program that opens `url` in the user's default browser, with the URL as
+ * its own argument.
+ *
+ * Windows deliberately avoids `start`. It is a cmd.exe built-in, so it needs a
+ * shell, and it reads its first quoted argument as a window title: `start
+ * "http://…"` opens an empty console titled with the URL and launches nothing.
+ * The URL protocol handler opens the default browser directly, with no shell to
+ * parse the URL and no title slot for it to fall into.
+ */
+export function browserLaunchCommand(
+  url: string,
+  platform: NodeJS.Platform = process.platform
+): BrowserLaunchCommand {
+  switch (platform) {
+    case 'darwin':
+      return { command: 'open', args: [url] };
+    case 'win32':
+      return { command: 'rundll32.exe', args: ['url.dll,FileProtocolHandler', url] };
+    default:
+      return { command: 'xdg-open', args: [url] };
+  }
+}
+
+/**
+ * Opens `url` in the default browser without a shell. A launch failure goes to
+ * `onError`; the caller decides how to tell the user, since only it knows what
+ * the page was for.
+ */
+export function openInBrowser(url: string, onError?: (err: Error) => void): void {
+  const { command, args } = browserLaunchCommand(url);
+  execFile(command, args, { windowsHide: true }, (err) => {
+    if (err) {
+      onError?.(err);
+    }
+  });
+}

packages/mcp-server/src/revisions/browser-prompts.ts

@@ -4,7 +4,7 @@
 // care about.
 
 import express from 'express';
-import { exec } from 'child_process';
+import { openInBrowser } from '../browser-launch.js';
 
 export type RevisionsConsentChoice =
   'enable-original' | 'enable-current' | 'proceed-without-history' | 'cancel';
@@ -169,13 +169,12 @@ async function runBrowserConsent<TChoice>(
     if (addr && typeof addr !== 'string') {
       const consentUrl = `http://127.0.0.1:${addr.port}/`;
       if (options.openBrowser !== false) {
-        const openCmd =
-          process.platform === 'darwin'
-            ? 'open'
-            : process.platform === 'win32'
-              ? 'start'
-              : 'xdg-open';
-        exec(`${openCmd} "${consentUrl}"`);
+        openInBrowser(consentUrl, (err) => {
+          process.stderr.write(
+            `Could not open a browser automatically (${err.message}). ` +
+              `Open ${consentUrl} to make your choice.\n`
+          );
+        });
       }
       options.onReady?.(addr.port);
     }