fix(cli): escape claude args and stop aborting on exit in omniroute launch (#8837)

* fix(cli): escape claude args and stop aborting on exit in omniroute launch

Windows launches go through spawn(..., { shell: true }), which joins argv
with plain spaces and no escaping (Node DEP0190). Any argument containing a
space was split, so `omniroute launch -p "two words"` reached claude as
`-p two` plus stray positionals, and the prompt was silently truncated.

Escape each argument for cmd.exe instead: CRT argv rules first (double the
backslashes preceding a quote, escape embedded quotes, wrap in quotes), then
cmd metacharacters caret-escaped twice. The second pass is required because
claude.cmd is an npm shim that re-parses %* on the way to node; with a single
pass arguments still truncated at the first `&` or `|`.

The command action also called process.exit() on any non-zero exit. That tore
the loop down while the exited child's inherited stdio handles were still
closing and aborted the process with a libuv assertion
(!(handle->flags & UV_HANDLE_CLOSING), src/win/async.c:94, exit 0xC0000409)
instead of returning claude's exit code. Set process.exitCode and let the
loop drain.

Extracts resolveClaudeSpawn() alongside the existing resolveCodexSpawn()
precedent so both the platform choice and the escaping are unit-testable.

Tests: tests/unit/cli/launch-windows-spawn-args.test.ts (new, 7 tests).
Four pure-function tests plus golden strings pin the exact encoding on every
platform; a Windows-only test round-trips argv through a real npm-style .cmd
shim and asserts embedded quotes, `&`, `|`, `%PATH%`, `^`, `!`, a trailing
backslash and an empty string all arrive byte-identical.

Note: bin/cli/commands/launch-codex.mjs carries the identical defect (same
shell:true concatenation, same process.exit) and is left unchanged here.

* docs(changelog): add fragment for #8837

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

bin/cli/commands/launch.mjs

@@ -90,6 +90,60 @@ export function resolveLaunchTarget(opts = {}) {
   return { baseUrl, authToken };
 }
 
+/**
+ * #8246: on Windows, npm installs claude as a `.cmd` shim — spawn() without a
+ * shell cannot resolve PATHEXT shims (and Node refuses to exec `.cmd` directly
+ * since CVE-2024-27980), so the Windows path must go through cmd.exe.
+ *
+ * @param {NodeJS.Platform|string} platform
+ * @returns {{ command: string, shell: true|undefined }}
+ */
+export function resolveClaudeSpawn(platform) {
+  return platform === "win32"
+    ? { command: "claude.cmd", shell: true }
+    : { command: "claude", shell: undefined };
+}
+
+/** cmd.exe metacharacters that stay live inside a quoted argument. */
+const WIN_META_CHARS = /([()\][%!^"`<>&|;, *?])/g;
+
+/**
+ * Escape one argument for a cmd.exe command line built by `shell: true`.
+ *
+ * Two layers, in order:
+ *  1. the CRT argv rules the target binary parses (double the backslashes that
+ *     precede a quote, escape embedded quotes, wrap in quotes);
+ *  2. cmd.exe's metacharacters, caret-escaped — applied TWICE because the
+ *     target is an npm `.cmd` shim that forwards `%*` to node, so the line is
+ *     parsed by cmd a second time. Single-escaping truncated any argument at
+ *     the first `&` or `|`. (Same rule as cross-spawn's doubleEscapeMetaChars.)
+ *
+ * @param {unknown} arg
+ * @returns {string}
+ */
+function escapeWindowsShellArg(arg) {
+  const s = String(arg);
+  if (s === "") return '""';
+  let out = s.replace(/(\\*)"/g, '$1$1\\"').replace(/(\\*)$/, "$1$1");
+  out = `"${out}"`;
+  return out.replace(WIN_META_CHARS, "^$1").replace(WIN_META_CHARS, "^$1");
+}
+
+/**
+ * `shell: true` makes Node join argv with plain spaces and no escaping (the
+ * DEP0190 warning), so `-p "two words"` used to reach claude as `-p two` plus
+ * three stray positional arguments. Quote the args ourselves on that path.
+ * Off Windows there is no shell, so argv is passed through untouched.
+ *
+ * @param {string[]} args
+ * @param {NodeJS.Platform|string} platform
+ * @returns {string[]}
+ */
+export function quoteClaudeArgs(args, platform) {
+  const list = [...(args ?? [])];
+  return platform === "win32" ? list.map(escapeWindowsShellArg) : list;
+}
+
 /**
  * @param {{port?:string, remote?:string, token?:string, apiKey?:string, profile?:string, claudeHome?:string}} opts
  * @param {string[]} claudeArgs  pass-through args for the claude binary
@@ -120,13 +174,12 @@ export async function runLaunchCommand(opts = {}, claudeArgs = []) {
   const env = buildClaudeEnv(process.env, baseUrl, authToken, { configDir });
 
   return await new Promise((resolve) => {
-    // #8246: on Windows, npm installs claude as a .cmd shim — spawn() without
-    // shell:true cannot resolve PATHEXT shims and fails with ENOENT.
-    const claudeCommand = process.platform === "win32" ? "claude.cmd" : "claude";
-    const child = spawn(claudeCommand, claudeArgs, {
+    const { command, shell } = resolveClaudeSpawn(process.platform);
+    const child = spawn(command, quoteClaudeArgs(claudeArgs, process.platform), {
       env,
       stdio: "inherit",
-      ...(process.platform === "win32" ? { shell: true, windowsHide: true } : {}),
+      shell,
+      ...(process.platform === "win32" ? { windowsHide: true } : {}),
     });
     child.on("error", (err) => {
       if (err && err.code === "ENOENT") {
@@ -159,7 +212,10 @@ export function registerLaunch(program) {
     .allowExcessArguments(true)
     .argument("[claudeArgs...]", "arguments passed through to the claude binary")
     .action(async (claudeArgs, opts) => {
-      const exitCode = await runLaunchCommand(opts, claudeArgs ?? []);
-      if (exitCode !== 0) process.exit(exitCode);
+      // process.exit() here aborted the process with a libuv assertion on
+      // Windows (`!(handle->flags & UV_HANDLE_CLOSING)`, async.c:94): it tears
+      // the loop down while the inherited stdio handles of the just-exited
+      // child are still closing. Setting exitCode lets the loop drain first.
+      process.exitCode = await runLaunchCommand(opts, claudeArgs ?? []);
     });
 }

changelog.d/fixes/8837-cli-launch-windows-spawn.md

@@ -0,0 +1 @@
+- **CLI**: `omniroute launch` no longer truncates pass-through arguments at the first space on Windows — each argument is escaped for cmd.exe (CRT argv rules plus a double caret pass, required because `claude.cmd` re-parses `%*`). A non-zero child exit also stops aborting the process with a libuv assertion: the exit code is set on `process.exitCode` so the loop drains the inherited stdio handles first

tests/unit/cli/launch-windows-spawn-args.test.ts

@@ -0,0 +1,124 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { spawn } from "node:child_process";
+import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { join } from "node:path";
+import { tmpdir } from "node:os";
+
+import { quoteClaudeArgs, resolveClaudeSpawn } from "../../../bin/cli/commands/launch.mjs";
+
+const isWindows = process.platform === "win32";
+
+// Regression guard for #8246: on Windows the `claude` binary is an npm `.cmd`
+// shim that spawn() cannot resolve without a shell (bare "claude" -> ENOENT).
+test("resolveClaudeSpawn: win32 spawns claude.cmd through a shell", () => {
+  const { command, shell } = resolveClaudeSpawn("win32");
+  assert.equal(command, "claude.cmd");
+  assert.equal(shell, true);
+});
+
+test("resolveClaudeSpawn: non-Windows platforms spawn the bare binary without a shell", () => {
+  for (const platform of ["linux", "darwin", "freebsd"]) {
+    const { command, shell } = resolveClaudeSpawn(platform);
+    assert.equal(command, "claude", `${platform} command`);
+    assert.equal(shell, undefined, `${platform} shell`);
+  }
+});
+
+// Regression guard: `shell: true` makes Node concatenate argv unescaped
+// (DEP0190), so `-p "two words"` reached claude as `-p two` and the rest of the
+// prompt was parsed as separate arguments.
+test("quoteClaudeArgs leaves argv untouched off Windows (no shell, no quoting)", () => {
+  const args = ["-p", "two words", "--model", "sonnet"];
+  assert.deepEqual(quoteClaudeArgs(args, "linux"), args);
+});
+
+test("quoteClaudeArgs escapes every argument on win32", () => {
+  // cmd.exe parses the whole line, so each argument is quoted — not just the
+  // ones containing spaces. The exact encoding is asserted by the round-trip
+  // test below; here we only pin that nothing is passed through raw.
+  const input = ["-p", "two words", "--profile", "auto-best-coding"];
+  const quoted = quoteClaudeArgs(input, "win32");
+  assert.equal(quoted.length, input.length);
+  for (const [i, arg] of quoted.entries()) {
+    assert.notEqual(arg, input[i], `argument ${i} must be escaped`);
+    assert.match(arg, /"/, `argument ${i} must be quoted`);
+  }
+});
+
+// The cmd.exe round-trip below is the real proof, but it can only run on
+// Windows. These golden strings pin the exact encoding so CI (Linux) still
+// fails if the escaping changes — e.g. if the double caret-escape required by
+// the `.cmd` shim's %* re-parse is ever reduced back to a single pass.
+test("quoteClaudeArgs: exact win32 encoding (golden)", () => {
+  const golden: Array<[string, string]> = [
+    ["-p", '^^^"-p^^^"'],
+    ["auto-best-coding", '^^^"auto-best-coding^^^"'],
+    ["two words", '^^^"two^^^ words^^^"'],
+    ["a & b", '^^^"a^^^ ^^^&^^^ b^^^"'],
+    ['q"uote', '^^^"q\\^^^"uote^^^"'],
+    ["trail\\", '^^^"trail\\\\^^^"'],
+    ["%PATH%", '^^^"^^^%PATH^^^%^^^"'],
+    ["", '""'],
+  ];
+  for (const [input, expected] of golden) {
+    assert.equal(quoteClaudeArgs([input], "win32")[0], expected, `encoding of ${JSON.stringify(input)}`);
+  }
+});
+
+test("quoteClaudeArgs does not mutate the caller's array", () => {
+  const input = ["-p", "two words"];
+  quoteClaudeArgs(input, "win32");
+  assert.deepEqual(input, ["-p", "two words"]);
+});
+
+// The real contract: whatever we hand to spawn(shell:true) must arrive at the
+// child's argv byte-identical. Verified against a probe .cmd through the same
+// cmd.exe path the launcher uses.
+test(
+  "quoteClaudeArgs survives a real cmd.exe round-trip",
+  { skip: isWindows ? false : "windows-only: exercises the cmd.exe shell path" },
+  async () => {
+    const dir = mkdtempSync(join(tmpdir(), "omniroute-argv-"));
+    try {
+      // Mirror the real shape of `claude.cmd`: an npm .cmd shim forwarding %*
+      // to a node script. Printing argv as JSON keeps the oracle exact.
+      writeFileSync(join(dir, "argv.mjs"), "console.log(JSON.stringify(process.argv.slice(2)));\n");
+      const probe = join(dir, "probe.cmd");
+      writeFileSync(probe, ['@echo off', 'node "%~dp0argv.mjs" %*'].join("\r\n") + "\r\n");
+
+      const args = [
+        "-p",
+        "In one short line: say BANANA",
+        "--append-system-prompt",
+        'quotes " and & ampersands | pipes',
+        "percent %PATH% and caret ^ and bang !",
+        "trailing backslash \\",
+        "",
+        "--profile",
+        "auto-best-coding",
+      ];
+
+      const received = await new Promise<string[]>((resolve, reject) => {
+        const child = spawn(probe, quoteClaudeArgs(args, "win32"), {
+          shell: true,
+          windowsHide: true,
+        });
+        let out = "";
+        child.stdout.on("data", (c) => (out += c));
+        child.on("error", reject);
+        child.on("exit", () => {
+          try {
+            resolve(JSON.parse(out.trim().split(/\r?\n/).pop() ?? "[]"));
+          } catch (err) {
+            reject(new Error(`probe did not emit argv JSON: ${out}`, { cause: err }));
+          }
+        });
+      });
+
+      assert.deepEqual(received, args, "child argv must match what the caller passed");
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  }
+);