From c08093c2b136353bc755459febf3e8afd46bf50e Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Wed, 14 Oct 2026 04:17:42 -0700 Subject: [PATCH] fix(shared): Windows .cmd shims no longer print DEP0190 on every spawn (#19385) Co-authored-by: Claude Opus 5.5 --- .../Adapters/CodexAdapterV2.test.ts | 7 ++- .../src/process/externalLauncher.test.ts | 47 ++++++++++++------- apps/server/src/processRunner.test.ts | 19 ++++---- .../providerMaintenanceRunner.test.ts | 17 +++---- packages/shared/src/shell.test.ts | 36 +++++++++----- packages/shared/src/shell.ts | 27 ++++------- scripts/dev-runner.ts | 9 ++-- scripts/mobile-native-static-check.ts | 2 +- 8 files changed, 89 insertions(+), 75 deletions(-) diff --git a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts index 0de461dd76e8e021676dc1e5cc29724023c03b42..ae29f052617b928b5b34b34ffbef6e5fe40813ed 100644 --- a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts @@ -693,8 +693,11 @@ describe("CodexAdapterV2 process spawning", () => { if (!ChildProcess.isStandardCommand(command)) { return; } - assert.equal(command.command, '^"C:\\npm\\codex.cmd^"'); - assert.deepEqual(command.args, ['^"app-server^"', '^"argument^ with^ spaces^"']); + assert.equal( + command.command, + '^"C:\\npm\\codex.cmd^" ^"app-server^" ^"argument^ with^ spaces^"', + ); + assert.deepEqual(command.args, []); assert.equal(command.options.shell, true); assert.equal(command.options.cwd, "C:\\workspace"); assert.deepEqual(command.options.env, { CUSTOM: "1" }); diff --git a/apps/server/src/process/externalLauncher.test.ts b/apps/server/src/process/externalLauncher.test.ts index 0314ffad432e073ec0f8f85af37d37fad3ea5dee..98a867d4591af62d08d847cb60910f161766eeb3 100644 --- a/apps/server/src/process/externalLauncher.test.ts +++ b/apps/server/src/process/externalLauncher.test.ts @@ -153,11 +153,15 @@ it.effect("launches an installed editor with platform-safe arguments", () => ); assert.ok(spawned); - assert.equal(spawned.command, '^"C:\\Program^ Files\\Microsoft^ VS^ Code\\bin\\code.CMD^"'); - assert.deepEqual(spawned.args, [ - '^"--goto^"', - '^"C:\\workspace^ with^ spaces\\src\\index.ts:12:4^"', - ]); + assert.equal( + spawned.command, + [ + '^"C:\\Program^ Files\\Microsoft^ VS^ Code\\bin\\code.CMD^"', + '^"--goto^"', + '^"C:\\workspace^ with^ spaces\\src\\index.ts:12:4^"', + ].join(" "), + ); + assert.deepEqual(spawned.args, []); assert.equal(spawned.options.shell, true); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); @@ -241,12 +245,16 @@ it.effect("launches Cursor in classic IDE mode through the Windows command shim" ); assert.ok(spawned); - assert.equal(spawned.command, '^"C:\\Program^ Files\\Cursor\\bin\\cursor.CMD^"'); - assert.deepEqual(spawned.args, [ - '^"--classic^"', - '^"--goto^"', - '^"C:\\workspace^ with^ spaces\\src\\index.ts:12:4^"', - ]); + assert.equal( + spawned.command, + [ + '^"C:\\Program^ Files\\Cursor\\bin\\cursor.CMD^"', + '^"--classic^"', + '^"--goto^"', + '^"C:\\workspace^ with^ spaces\\src\\index.ts:12:4^"', + ].join(" "), + ); + assert.deepEqual(spawned.args, []); assert.equal(spawned.options.shell, true); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); @@ -1033,12 +1041,13 @@ for (const { platform, installPath, editor, args } of [ ), ); assert.ok(spawned); + const shim = executable.endsWith(".cmd"); assert.equal( spawned.command, - executable.endsWith(".cmd") ? `^"${executable.replaceAll(" ", "^ ")}^"` : executable, + shim ? [`^"${executable.replaceAll(" ", "^ ")}^"`, ...args].join(" ") : executable, ); - assert.deepEqual(spawned.args, args); - assert.equal(spawned.options.shell, executable.endsWith(".cmd")); + assert.deepEqual(spawned.args, shim ? [] : args); + assert.equal(spawned.options.shell, shim); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); } @@ -1476,10 +1485,12 @@ it.effect("starts a Windows command shim for unicode paths with cmd metacharacte assert.isTrue(Exit.isSuccess(exit)); assert.ok(spawned); assert.equal(spawned.options.shell, true); - assert.deepEqual(spawned.args, [ - '^"--goto^"', - '^"C:\\Users\\jö\\R^&D^ ^(100^%^)\\naïve^ ^^file^!.ts:3:7^"', - ]); + assert.deepEqual(spawned.args, []); + assert.isTrue( + spawned.command.endsWith( + ' ^"--goto^" ^"C:\\Users\\jö\\R^&D^ ^(100^%^)\\naïve^ ^^file^!.ts:3:7^"', + ), + ); }), ); diff --git a/apps/server/src/processRunner.test.ts b/apps/server/src/processRunner.test.ts index 3bec75351baf7ce434046cfc734b3c40d23fbb5d..44c1d258a4a45beda040a283284184446b888abf 100644 --- a/apps/server/src/processRunner.test.ts +++ b/apps/server/src/processRunner.test.ts @@ -127,14 +127,17 @@ describe("runProcess", () => { it.effect("resolves and escapes Windows command shims before spawning", () => { const spawner = makeSpawner((command) => Effect.sync(() => { - expect(command.command).toBe('^"C:\\Users\\tester\\AppData\\Roaming\\npm\\az.cmd^"'); - expect(command.args).toEqual([ - '^"repos^"', - '^"pr^"', - '^"list^"', - '^"--source-branch^"', - '^"feature^ ^&^ release^"', - ]); + expect(command.command).toBe( + [ + '^"C:\\Users\\tester\\AppData\\Roaming\\npm\\az.cmd^"', + '^"repos^"', + '^"pr^"', + '^"list^"', + '^"--source-branch^"', + '^"feature^ ^&^ release^"', + ].join(" "), + ); + expect(command.args).toEqual([]); expect(command.options.shell).toBe(true); return makeHandle({ stdout: "[]" }); }), diff --git a/apps/server/src/provider/providerMaintenanceRunner.test.ts b/apps/server/src/provider/providerMaintenanceRunner.test.ts index 26b33a35f4a2a9dfd00584791fcd3d2bad043c12..514f33d3f45afc3dde770ff85956ba5e0bfaee16 100644 --- a/apps/server/src/provider/providerMaintenanceRunner.test.ts +++ b/apps/server/src/provider/providerMaintenanceRunner.test.ts @@ -943,21 +943,16 @@ describe("providerMaintenanceRunner", () => { const result = yield* runner.updateProvider(CODEX_DRIVER); // On win32, resolveSpawnCommand resolves `npm` to the `.cmd` shim and - // routes the spawn through cmd.exe (shell: true), escaping every arg. + // routes the spawn through cmd.exe (shell: true) as one escaped line. assert.strictEqual(captured.length, 1); const call = captured[0]; assert.ok(call, "expected the spawner to be invoked once"); - // The resolved command is the escaped `.cmd` path. Asserting the precise - // escaped string is brittle, so verify it carries the resolved shim and - // that shell mode was used. - assert.match(call.command, /npm\.cmd/i); + assert.strictEqual( + call.command, + '^"C:\\fake\\npm\\npm.cmd^" ^"install^" ^"-g^" ^"@openai/codex@latest^"', + ); + assert.deepEqual(call.args, []); assert.strictEqual(call.shell, true); - // Args are escaped for cmd.exe shell mode (each quoted) but still carry - // the original install command (`install -g @openai/codex@latest`) in order. - assert.strictEqual(call.args.length, 3); - assert.match(call.args[0] ?? "", /install/); - assert.match(call.args[1] ?? "", /-g/); - assert.match(call.args[2] ?? "", /@openai\/codex@latest/); assert.strictEqual(result.providers[0]?.updateState?.status, "succeeded"); }).pipe( Effect.provide( diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts index b9e10812f005f382bb04667ee5b1a14a0961f57e..77ee889057588a0463aa24945d2a22e9cc089545 100644 --- a/packages/shared/src/shell.test.ts +++ b/packages/shared/src/shell.test.ts @@ -681,11 +681,19 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { }), ); - it.effect("escapes the executable and arguments for Windows command shims", () => + it.effect("builds one escaped cmd.exe line for Windows command shims", () => Effect.gen(function* () { const command = yield* resolveSpawnCommand( "vp", - ["run", "value & calc", "%PATH%", 'quote"value'], + [ + "run", + "value & calc", + "%PATH%", + 'quote"value', + "C:\\dir with space\\", + 'slash\\"quote', + "x|y^", + ], { env: { PATH: "", PATHEXT: ".COM;.EXE;.BAT;.CMD" } }, ).pipe( Effect.provideService(HostProcess.Platform, "win32"), @@ -696,15 +704,21 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { ), ); - expect(command.shell).toBe(true); - expect(command.command).not.toContain(" & "); - expect(command.command).toContain("^&"); - expect(command.args).toEqual([ - '^"run^"', - '^"value^ ^&^ calc^"', - '^"^%PATH^%^"', - '^"quote\\^"value^"', - ]); + // No args alongside `shell: true`: Node prints DEP0190 for that pairing. + expect(command).toEqual({ + command: [ + String.raw`^"C:\Program^ Files\npm^ ^&^ tools\vp.cmd^"`, + String.raw`^"run^"`, + String.raw`^"value^ ^&^ calc^"`, + String.raw`^"^%PATH^%^"`, + String.raw`^"quote\^"value^"`, + String.raw`^"C:\dir^ with^ space\\^"`, + String.raw`^"slash\\\^"quote^"`, + String.raw`^"x^|y^^^^"`, + ].join(" "), + args: [], + shell: true, + }); }), ); diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index d6d8a84afae9ceba54c883df919f9bf11a9ab572..36200c4fa0c17b3acc3fc6a590ec5d3f05f424a9 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -46,10 +46,10 @@ const WINDOWS_SHELL_META_CHARS = /([()\][%!^"`<>&|;, *?])/g; /** * Escapes a single argument for `cmd.exe` shell mode (`spawn(..., { shell: true })` - * on Windows). Node joins the command and arguments with spaces and hands the - * resulting string to `cmd.exe` without any quoting, so every dynamic argument - * must be escaped to survive both cmd.exe parsing and the target program's - * `CommandLineToArgvW` parsing. Mirrors cross-spawn's argument escaping. + * on Windows). Node hands the command string to `cmd.exe /d /s /c ""` + * without any quoting, so every dynamic argument must be escaped to survive both + * cmd.exe parsing and the target program's `CommandLineToArgvW` parsing. + * Mirrors cross-spawn's argument escaping. */ function escapeWindowsShellArg(arg: string): string { // Double up backslashes that precede a double quote, then escape the quote @@ -63,18 +63,6 @@ function escapeWindowsShellArg(arg: string): string { return escaped.replace(WINDOWS_SHELL_META_CHARS, "^$1"); } -/** - * Escapes arguments for shell-mode spawns: applies {@link escapeWindowsShellArg} - * when the platform is `win32` (where `shell: true` routes through `cmd.exe`) - * and returns the arguments untouched everywhere else. - */ -function sanitizeShellModeArgsForPlatform( - args: ReadonlyArray, - platform: NodeJS.Platform, -): Array { - return platform === "win32" ? args.map(escapeWindowsShellArg) : [...args]; -} - export interface ResolvedSpawnCommand { readonly command: string; readonly args: ReadonlyArray; @@ -773,9 +761,12 @@ export const resolveSpawnCommand = Effect.fnUntraced(function* ( return { command: resolvedCommand, args: [...args], shell: false }; } + // Node runs a shell spawn as `cmd.exe /d /s /c " "`, but + // passing args with `shell: true` prints DEP0190. Build the same escaped line + // here and pass no args. return { - command: escapeWindowsShellArg(resolvedCommand), - args: sanitizeShellModeArgsForPlatform(args, platform), + command: [resolvedCommand, ...args].map(escapeWindowsShellArg).join(" "), + args: [], shell: true, }; }); diff --git a/scripts/dev-runner.ts b/scripts/dev-runner.ts index c8531d034c8f197ca393ade7004fc822450714ac..c8130119ac41d9be07238a1a5d18ed1e31267804 100644 --- a/scripts/dev-runner.ts +++ b/scripts/dev-runner.ts @@ -791,15 +791,12 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { } } - const spawnCommand = yield* resolveSpawnCommand( - "vp", - [...MODE_ARGS[input.mode], ...input.runArgs], - { env }, - ); + const args = [...MODE_ARGS[input.mode], ...input.runArgs]; + const spawnCommand = yield* resolveSpawnCommand("vp", args, { env }); const processContext = { mode: input.mode, executable: "vp" as const, - argumentCount: spawnCommand.args.length, + argumentCount: args.length, shell: spawnCommand.shell, } as const; const child = yield* ChildProcess.make(spawnCommand.command, spawnCommand.args, { diff --git a/scripts/mobile-native-static-check.ts b/scripts/mobile-native-static-check.ts index 9ed98ad4414e47a9dde847cad560176f7fc6f7af..52b9a84078e703c505cc89025c8282c632a542ea 100644 --- a/scripts/mobile-native-static-check.ts +++ b/scripts/mobile-native-static-check.ts @@ -127,7 +127,7 @@ export const runCommand = Effect.fn("runCommand")(function* ( const spawnCommand = yield* resolveSpawnCommand(command, args); const processContext = { command, - argumentCount: spawnCommand.args.length, + argumentCount: args.length, cwd, shell: spawnCommand.shell, } as const; -- 2.54.0