Files
OmniRoute/tests/unit/cli/launch-codex-windows-spawn-args.test.ts
Suman 8e0d7e4ddd fix(cli): escape codex args and stop aborting on exit in launch-codex (#8856)
* fix(cli): escape codex args and stop aborting on exit in launch-codex

`launch-codex` spawns `codex.cmd` with `shell: true` on Windows, so Node joins
argv with plain spaces and no escaping (DEP0190). This mangles every Windows
invocation, not only the ones with a multi-word user argument, because the
injected `-c` provider flags carry quoted TOML values:

  ["-c","model_provider=omniroute", ...,
   "model_providers.omniroute.base_url=http://localhost:20128/v1",
   "fix","the","bug"]

cmd.exe strips the TOML quotes (`model_provider=omniroute` no longer parses as
a TOML string), splits multi-word arguments, and swallows everything after an
unquoted `&`. The same defect was fixed for `launch` in #8837; this ports it to
`launch-codex`, which that PR disclosed but left unfixed.

- extract the escaping into `bin/cli/utils/winShellArgs.mjs` and reuse it from
  both launchers instead of keeping a private copy in `launch.mjs`
- quote the codex argv (provider flags + profile + pass-through args) on the
  win32 shell path; argv is untouched off Windows, where no shell is involved
- replace `process.exit()` in the command action with `process.exitCode`: on any
  non-zero child exit it aborted with the libuv `!(handle->flags &
  UV_HANDLE_CLOSING)` assertion while the inherited stdio handles were closing

Test: `tests/unit/cli/launch-codex-windows-spawn-args.test.ts` pins the exact
encoding with golden strings (the cmd.exe round-trip is Windows-only and skips
on Linux CI, so without goldens CI would guard nothing) and round-trips the real
provider flags through a probe `.cmd` shim that forwards `%*`.

* docs(changelog): add fragment for #8856

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

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
2026-07-28 11:50:00 -03:00

130 lines
4.9 KiB
TypeScript

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 {
buildCodexProviderArgs,
quoteCodexArgs,
resolveCodexSpawn,
} from "../../../bin/cli/commands/launch-codex.mjs";
const isWindows = process.platform === "win32";
test("resolveCodexSpawn: win32 spawns codex.cmd through a shell", () => {
const { command, shell } = resolveCodexSpawn("win32");
assert.equal(command, "codex.cmd");
assert.equal(shell, true);
});
test("resolveCodexSpawn: non-Windows platforms spawn the bare binary without a shell", () => {
for (const platform of ["linux", "darwin", "freebsd"]) {
const { command, shell } = resolveCodexSpawn(platform);
assert.equal(command, "codex", `${platform} command`);
assert.equal(shell, undefined, `${platform} shell`);
}
});
test("quoteCodexArgs leaves argv untouched off Windows (no shell, no quoting)", () => {
const args = [...buildCodexProviderArgs("http://localhost:20128"), "explain this repo"];
assert.deepEqual(quoteCodexArgs(args, "linux"), args);
});
// The injected provider flags are the reason this bug bit every Windows
// invocation, not just the ones with a multi-word user argument: their TOML
// values are quoted, and cmd.exe eats those quotes when the line is unescaped.
test("quoteCodexArgs escapes the injected -c provider flags on win32", () => {
const input = buildCodexProviderArgs("http://localhost:20128");
const quoted = quoteCodexArgs(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("quoteCodexArgs: exact win32 encoding (golden)", () => {
const golden: Array<[string, string]> = [
["-c", '^^^"-c^^^"'],
[
'model_providers.omniroute.base_url="http://localhost:20128/v1"',
'^^^"model_providers.omniroute.base_url=\\^^^"http://localhost:20128/v1\\^^^"^^^"',
],
[
"model_providers.omniroute.requires_openai_auth=false",
'^^^"model_providers.omniroute.requires_openai_auth=false^^^"',
],
["fix the bug & ship it", '^^^"fix^^^ the^^^ bug^^^ ^^^&^^^ ship^^^ it^^^"'],
["", '""'],
];
for (const [input, expected] of golden) {
assert.equal(
quoteCodexArgs([input], "win32")[0],
expected,
`encoding of ${JSON.stringify(input)}`
);
}
});
test("quoteCodexArgs does not mutate the caller's array", () => {
const input = ["-c", "model_provider=\"omniroute\""];
quoteCodexArgs(input, "win32");
assert.deepEqual(input, ["-c", 'model_provider="omniroute"']);
});
// 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(
"quoteCodexArgs 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-codex-argv-"));
try {
// Mirror the real shape of `codex.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 = [
...buildCodexProviderArgs("http://localhost:20128"),
"--profile",
"auto-best-coding",
"In one short line: say BANANA",
'quotes " and & ampersands | pipes',
"percent %PATH% and caret ^ and bang !",
"trailing backslash \\",
"",
];
const received = await new Promise<string[]>((resolve, reject) => {
const child = spawn(probe, quoteCodexArgs(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 });
}
}
);