From e0a22ff619528dae520260f39f235f5f50f4735e Mon Sep 17 00:00:00 2001 From: Paco Cartones <253313177+pacocartones@users.noreply.github.com> Date: Sun, 23 Aug 2026 19:24:15 +0200 Subject: [PATCH] test(cli): make the CLI suite pass on Windows (#11240) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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! --- tests/unit/cli/_helpers/shellArgs.mjs | 41 ++++++++++++++++++++++ tests/unit/cli/alias-resolver-7791.test.ts | 15 +++++--- tests/unit/cli/run-command.test.ts | 17 ++++----- 3 files changed, 61 insertions(+), 12 deletions(-) create mode 100644 tests/unit/cli/_helpers/shellArgs.mjs diff --git a/tests/unit/cli/_helpers/shellArgs.mjs b/tests/unit/cli/_helpers/shellArgs.mjs new file mode 100644 index 0000000000..148fb94174 --- /dev/null +++ b/tests/unit/cli/_helpers/shellArgs.mjs @@ -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; +} diff --git a/tests/unit/cli/alias-resolver-7791.test.ts b/tests/unit/cli/alias-resolver-7791.test.ts index 6a99522090..d6037796f4 100644 --- a/tests/unit/cli/alias-resolver-7791.test.ts +++ b/tests/unit/cli/alias-resolver-7791.test.ts @@ -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"); diff --git a/tests/unit/cli/run-command.test.ts b/tests/unit/cli/run-command.test.ts index aa2be59579..0767917450 100644 --- a/tests/unit/cli/run-command.test.ts +++ b/tests/unit/cli/run-command.test.ts @@ -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);