test(cli): make the CLI suite pass on Windows (#11240)

Validated on the combined 12-PR batch board: the touched CLI test files pass (alias-resolver-7791, run-command — incl. the new shellArgs helper), typecheck:core clean. Test-side only, no production behavior change. Thank you @pacocartones!
This commit is contained in:
Paco Cartones
2026-08-23 19:24:15 +02:00
committed by GitHub
parent 162ef913da
commit e0a22ff619
3 changed files with 61 additions and 12 deletions

View File

@@ -0,0 +1,41 @@
/**
* Reverse the Windows `shell: true` argument escaping so assertions can be
* written against the logical argv on every platform.
*
* `bin/cli/commands/run.mjs` escapes argv before spawning, because on win32 the
* launchers must go through cmd.exe to run npm `.cmd` shims (CVE-2024-27980)
* and Node's `shell: true` joins argv with no escaping at all (DEP0190). That
* escaping is correct and deliberate, but it means `plan.args` holds
* `^^^"--model^^^"` on Windows where it holds `--model` elsewhere.
*
* Tests care about *which* arguments a plan carries, not about how they survive
* cmd.exe, so they normalise first. Keep this in sync with
* `escapeWindowsShellArg` in bin/cli/utils/winShellArgs.mjs.
*/
/**
* @param {unknown} arg
* @returns {string}
*/
export function unescapeWindowsShellArg(arg) {
let s = String(arg);
// 1. undo the two caret passes applied to cmd.exe metacharacters
s = s.replace(/\^(.)/g, "$1").replace(/\^(.)/g, "$1");
// 2. drop the wrapping quotes added by the CRT argv layer
if (s.length >= 2 && s.startsWith('"') && s.endsWith('"')) s = s.slice(1, -1);
// 3. undo the doubled backslashes and the escaped embedded quotes
s = s.replace(/\\\\/g, "\\").replace(/\\"/g, '"');
return s;
}
/**
* Normalise a plan's argv to its logical form. A no-op off Windows.
*
* @param {unknown[]} args
* @param {NodeJS.Platform|string} [platform]
* @returns {string[]}
*/
export function logicalArgs(args, platform = process.platform) {
const list = [...(args ?? [])].map(String);
return platform === "win32" ? list.map(unescapeWindowsShellArg) : list;
}

View File

@@ -18,7 +18,7 @@ import { spawnSync } from "node:child_process";
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { fileURLToPath } from "node:url";
import { fileURLToPath, pathToFileURL } from "node:url";
import {
resolveAlias,
@@ -30,6 +30,13 @@ import {
const __dirname = fileURLToPath(new URL(".", import.meta.url));
const REPO_ROOT = join(__dirname, "..", "..", "..");
// The child scripts below `import()` these paths, and `import()` resolves its
// specifier as a URL. Replacing backslashes with forward slashes is not enough
// on Windows: the leading drive letter is then parsed as the URL scheme `e:`,
// which the ESM loader rejects with ERR_UNSUPPORTED_ESM_URL_SCHEME. Emit a
// real file:// URL instead.
const repoFileUrl = (relPath) => pathToFileURL(join(REPO_ROOT, relPath)).href;
describe("aliasResolver.resolveAlias (pure)", () => {
it("returns null for non-@/ specifiers (lets Node/tsx handle them)", () => {
assert.equal(resolveAlias("node:fs", REPO_ROOT), null);
@@ -260,11 +267,11 @@ describe("aliasResolver end-to-end (#7791 regression)", () => {
const script = `
await import("tsx/esm");
import { join } from "node:path";
import { registerAliasResolver } from "${join(REPO_ROOT, "bin/aliasResolver.mjs").replace(/\\/g, "/")}";
import { registerAliasResolver } from ${JSON.stringify(repoFileUrl("bin/aliasResolver.mjs"))};
const ok = await registerAliasResolver(${JSON.stringify(REPO_ROOT)});
if (!ok) { console.error("FAIL: registerAliasResolver returned false"); process.exit(2); }
try {
const m = await import(${JSON.stringify(join(REPO_ROOT, "src/shared/network/outboundUrlGuard.ts").replace(/\\/g, "/"))});
const m = await import(${JSON.stringify(repoFileUrl("src/shared/network/outboundUrlGuard.ts"))});
const keys = Object.keys(m).sort().join(",");
console.log("OK:" + keys);
} catch (err) {
@@ -286,7 +293,7 @@ describe("aliasResolver end-to-end (#7791 regression)", () => {
it("does not interfere with bare/relative specifiers (regression guard)", () => {
const script = `
import { registerAliasResolver } from "${join(REPO_ROOT, "bin/aliasResolver.mjs").replace(/\\/g, "/")}";
import { registerAliasResolver } from ${JSON.stringify(repoFileUrl("bin/aliasResolver.mjs"))};
await registerAliasResolver(${JSON.stringify(REPO_ROOT)});
// node:fs must still resolve via the default resolver
const fs = await import("node:fs");

View File

@@ -7,6 +7,7 @@ import {
resolveModelFromTargetOptions,
runCliTarget,
} from "../../../bin/cli/commands/run.mjs";
import { logicalArgs } from "./_helpers/shellArgs.mjs";
test("resolveRunTarget resolves aliases", () => {
assert.equal(resolveRunTarget("claude"), "claude");
@@ -39,7 +40,7 @@ test("buildRunPlan for claude includes env diff and model injection", async () =
assert.equal(plan.target, "claude");
assert.equal(plan.baseUrl, "http://localhost:20128");
assert.equal(plan.model, "gpt-5");
assert.equal(plan.args.includes("--help"), true);
assert.equal(logicalArgs(plan.args).includes("--help"), true);
assert.equal(plan.envDiff.changedOrAdded.includes("ANTHROPIC_AUTH_TOKEN"), true);
assert.equal(plan.authSource, "option");
assert.equal(plan.command.includes("claude"), true);
@@ -54,9 +55,9 @@ test("buildRunPlan for codex injects model into provider args", async () => {
assert.equal(plan.target, "codex");
assert.equal(plan.baseUrl, "http://localhost:20128");
assert.equal(plan.model, "glm/glm-4.5");
assert.equal(plan.args.includes("--help"), true);
assert.equal(logicalArgs(plan.args).includes("--help"), true);
assert.equal(
plan.args.some((a) => String(a).includes("model_providers.omniroute.model")),
logicalArgs(plan.args).some((a) => a.includes("model_providers.omniroute.model")),
true
);
assert.equal(plan.authSource, "option");
@@ -70,7 +71,7 @@ test("buildRunPlan for Aider uses its OpenAI-compatible root endpoint", async ()
);
assert.equal(plan.target, "aider");
assert.equal(plan.baseUrl, "https://relay.example.test");
assert.deepEqual(plan.args.slice(0, 2), ["--model", "openai/glm/glm-5.2"]);
assert.deepEqual(logicalArgs(plan.args).slice(0, 2), ["--model", "openai/glm/glm-5.2"]);
assert.equal(plan.envDiff.changedOrAdded.includes("OPENAI_API_BASE"), true);
assert.equal(plan.envDiff.changedOrAdded.includes("OPENAI_API_KEY"), true);
});
@@ -82,7 +83,7 @@ test("buildRunPlan for Goose injects provider and model without writing config",
["session"]
);
assert.equal(plan.target, "goose");
assert.deepEqual(plan.args, ["session"]);
assert.deepEqual(logicalArgs(plan.args), ["session"]);
assert.equal(plan.envDiff.changedOrAdded.includes("GOOSE_PROVIDER"), true);
assert.equal(plan.envDiff.changedOrAdded.includes("GOOSE_MODEL"), true);
assert.equal(plan.envDiff.changedOrAdded.includes("OPENAI_HOST"), true);
@@ -95,7 +96,7 @@ test("buildRunPlan for OpenCode uses an ephemeral compatible config", async () =
["run", "reply OK"]
);
assert.equal(plan.target, "opencode");
assert.deepEqual(plan.args.slice(0, 2), ["--model", "omniroute/glm/glm-5.2"]);
assert.deepEqual(logicalArgs(plan.args).slice(0, 2), ["--model", "omniroute/glm/glm-5.2"]);
assert.equal(plan.envDiff.changedOrAdded.includes("OPENCODE_CONFIG_CONTENT"), true);
assert.equal(plan.envDiff.changedOrAdded.includes("OMNIROUTE_API_KEY"), true);
assert.equal(plan.configOverlay, "OPENCODE_CONFIG_CONTENT (process environment only)");
@@ -109,7 +110,7 @@ test("buildRunPlan for Qwen requires a deterministic model and injects only env
["-p", "reply OK"]
);
assert.equal(plan.target, "qwen");
assert.deepEqual(plan.args.slice(0, 2), ["--model", "glm/glm-5.2"]);
assert.deepEqual(logicalArgs(plan.args).slice(0, 2), ["--model", "glm/glm-5.2"]);
assert.equal(plan.envDiff.changedOrAdded.includes("OMNIROUTE_API_KEY"), true);
assert.equal(plan.configOverlay, "temporary QWEN_HOME (removed after exit)");
await assert.rejects(
@@ -126,7 +127,7 @@ test("buildRunPlan for Gemini points the CLI at the /v1beta surface via env", as
);
assert.equal(plan.target, "gemini");
assert.equal(plan.baseUrl, "https://relay.example.test");
assert.deepEqual(plan.args.slice(0, 2), ["--model", "glm/glm-5.2"]);
assert.deepEqual(logicalArgs(plan.args).slice(0, 2), ["--model", "glm/glm-5.2"]);
assert.equal(plan.envDiff.changedOrAdded.includes("GOOGLE_GEMINI_BASE_URL"), true);
assert.equal(plan.envDiff.changedOrAdded.includes("GEMINI_API_KEY"), true);
assert.equal(plan.envDiff.changedOrAdded.includes("GEMINI_DEFAULT_AUTH_TYPE"), true);