fix: preserve spaced Windows shim command lines

.claude/skills/developing-insta-cli/SKILL.md

@@ -118,7 +118,7 @@ skipped because its argv is split before commander ever sees it — there, pass
 | `npm error code EOTP` | Publish needs `--otp=<fresh 2FA code>` |
 | `npm publish` ENOENT package.json | Ran outside the repo root |
 | `npx insta@latest` behind the GH release | `publish-npm` job failed (OIDC trust/config?) — see step 4 |
-| `'C:\Program' is not recognized` from a spawned tool (win CI only) | `resolveSpawnable`'s cmd.exe hop strips the quotes around a spaced executable path (`C:\Program Files\…`). It exists for npm-installed `.cmd` shims — a real `.exe` (git, …) must be spawned directly, which finds it through PATHEXT anyway |
+| `'C:\Program' is not recognized` from a spawned tool (win CI only) | Callers of `resolveSpawnable` must pass its `windowsVerbatimArguments` option to `spawn`; cmd.exe needs the resolver's outer command quotes and inner argument quotes intact. Spawn real `.exe` files directly. |
 | CLI hits the wrong server in tests | Persisted `~/.insta/config.json` apiUrl; set `INSTA_API_URL` (≥0.0.7) or move the config aside |
 | `insta --api-url X compute exec …` works but `insta compute exec --api-url X …` says unknown option | exec's argv is split before commander (`splitExecArgs`); pass `--api-url` at the root for exec |
 

src/commands/setup.ts

@@ -172,7 +172,7 @@ export async function ensureCliInstalled(
 // guidance. Ignoring stdin keeps the installer's own output intact. (-y means no prompt anyway.)
 const defaultRunner: Runner = (cmdIn, argsIn) =>
   new Promise((resolve) => {
-    const { cmd, args } = resolveSpawnable(cmdIn, argsIn)
+    const { cmd, args, windowsVerbatimArguments } = resolveSpawnable(cmdIn, argsIn)
     const env: NodeJS.ProcessEnv = { ...process.env, AI_AGENT: process.env.AI_AGENT || 'insta', FORCE_COLOR: '0' }
     // When THIS process was launched by npx, npx exports its flags as npm_config_* env vars.
     // npm_config_package pins package resolution for every nested npm/npx child — the inner
@@ -181,7 +181,7 @@ const defaultRunner: Runner = (cmdIn, argsIn) =>
     // (deliberate user configuration).
     delete env.npm_config_package
     delete env.npm_config_call
-    const p = spawn(cmd, args, { stdio: ['ignore', 'pipe', 'pipe'], env })
+    const p = spawn(cmd, args, { stdio: ['ignore', 'pipe', 'pipe'], env, windowsVerbatimArguments })
     let output = ''
     const grab = (chunk: Buffer) => { output += chunk.toString() }
     p.stdout?.on('data', grab)

src/commands/upgrade.ts

@@ -46,7 +46,7 @@ export function resolveRunSpec(
   npmExecpath = spec.env.npm_execpath,
   execPath = process.execPath,
   platform: NodeJS.Platform = process.platform,
-): RunSpec {
+): RunSpec & { windowsVerbatimArguments?: boolean } {
   const resolved = resolveSpawnable(spec.cmd, spec.args, npmExecpath, execPath, platform, spec.env)
   return { ...spec, ...resolved }
 }
@@ -220,7 +220,7 @@ export function autoEnabled(home = homedir()): boolean {
 const spawnRun: Runner = ({ cmd, args, env }) =>
   new Promise<void>((resolve, reject) => {
     const resolved = resolveRunSpec({ cmd, args, env })
-    const p = spawn(resolved.cmd, resolved.args, { stdio: 'inherit', env })
+    const p = spawn(resolved.cmd, resolved.args, { stdio: 'inherit', env, windowsVerbatimArguments: resolved.windowsVerbatimArguments })
     p.on('error', reject)
     p.on('close', (code) => (code === 0 ? resolve() : reject(new Error(`upgrade failed (exit ${code})`))))
   })
@@ -238,7 +238,7 @@ const observeInstalled: Observer = (channel, installDir) =>
       // INSTA_NO_AUTOUPDATE: the child must not kick off yet another upgrade while we are reading it.
       const env: NodeJS.ProcessEnv = { ...process.env, INSTA_NO_AUTOUPDATE: '1' }
       const resolved = resolveRunSpec({ cmd, args: ['--version'], env })
-      const p = spawn(resolved.cmd, resolved.args, { stdio: ['ignore', 'pipe', 'ignore'], env })
+      const p = spawn(resolved.cmd, resolved.args, { stdio: ['ignore', 'pipe', 'ignore'], env, windowsVerbatimArguments: resolved.windowsVerbatimArguments })
       p.stdout.on('data', (d) => (out += String(d)))
       p.on('error', () => resolve(null))
       p.on('close', () => {

src/ensure-skills.ts

@@ -34,13 +34,13 @@ const defaultRunner: Runner = (cmdIn, argsIn, inherit = false) =>
   new Promise((resolve) => {
     // resolveSpawnable: on Windows `npx` is a .cmd shim spawn() refuses without a shell —
     // re-enter npm's CLI script via node instead (same treatment as `insta agent setup`).
-    const { cmd, args } = resolveSpawnable(cmdIn, argsIn)
+    const { cmd, args, windowsVerbatimArguments } = resolveSpawnable(cmdIn, argsIn)
     const env: NodeJS.ProcessEnv = { ...process.env, AI_AGENT: process.env.AI_AGENT || 'insta' }
     // npx exports its flags as npm_config_* to children; npm_config_package would pin the inner
     // `npx -y skills …` to whatever package launched this CLI (see setup.ts defaultRunner).
     delete env.npm_config_package
     delete env.npm_config_call
-    const p = spawn(cmd, args, { stdio: inherit ? 'inherit' : 'ignore', env })
+    const p = spawn(cmd, args, { stdio: inherit ? 'inherit' : 'ignore', env, windowsVerbatimArguments })
     p.on('error', () => resolve({ ok: false })) // e.g. npx not on PATH
     p.on('close', (code) => resolve({ ok: code === 0 }))
   })

src/spawn.ts

@@ -54,7 +54,7 @@ export function resolveSpawnable(
   platform: NodeJS.Platform = process.platform,
   env: NodeJS.ProcessEnv = process.env,
   systemRoot: string = process.env.SYSTEMROOT ?? process.env.windir ?? 'C:\\Windows',
-): { cmd: string; args: string[] } {
+): { cmd: string; args: string[]; windowsVerbatimArguments?: boolean } {
   const execIsNode = /(^|[\\/])node(\.exe)?$/i.test(execPath)
   if ((cmd === 'npm' || cmd === 'npx') && execIsNode) {
     if (npmExecpath && /(^|[\\/])np[mx](-cli)?\.[cm]?js$/.test(npmExecpath)) {
@@ -69,13 +69,16 @@ export function resolveSpawnable(
   }
 
   const bareShim = !/[\\/]/.test(cmd) && !/\.exe$/i.test(cmd)
-  if (platform === 'win32' && bareShim && !args.some((arg) => /[&|<>^%"]/.test(arg))) {
+  if (platform === 'win32' && bareShim && !args.some((arg) => /[&|<>^%"\r\n]/.test(arg))) {
     const absolute = whichOnPath(cmd, env, platform)
-    // The resolved path itself also becomes cmd.exe input. If a PATH directory contains a shell
-    // metacharacter, fail via the caller's normal bare-spawn fallback rather than interpret it.
-    // Pin cmd.exe to System32 too: CreateProcess-style lookup checks cwd before PATH.
-    if (absolute && !/[&|<>^%"]/.test(absolute)) {
-      return { cmd: win32.join(systemRoot, 'System32', 'cmd.exe'), args: ['/d', '/s', '/c', absolute, ...args] }
+    if (absolute && !/[&|<>^%"\r\n]/.test(absolute)) {
+      const command = [absolute, ...args].map((arg) => `"${arg.replace(/\\+$/, '$&$&')}"`).join(' ')
+      // cmd /s consumes the outer quotes; libuv must not escape the inner argument quotes.
+      return {
+        cmd: win32.join(systemRoot, 'System32', 'cmd.exe'),
+        args: ['/d', '/v:off', '/s', '/c', `"${command}"`],
+        windowsVerbatimArguments: true,
+      }
     }
   }
   return { cmd, args }

test/self-install.test.ts

@@ -164,14 +164,12 @@ test('resolveSpawnable re-enters npm/npx as node scripts so Windows .cmd shims a
   // Non-npm commands on Windows (claude is an npm-installed .cmd shim too) go through cmd.exe,
   // resolved to the shim's ABSOLUTE PATH first — cmd.exe searches the current directory before
   // PATH, so a bare name would let a claude.cmd planted in the project dir shadow the real CLI.
-  // Args stay UNquoted: libuv quotes spaced args when building the child command line, so
-  // pre-quoting would double up and deliver literal quote characters to the target.
   const shimBin = mkdtempSync(join(tmpdir(), 'insta-shim-'))
   const claudeShim = join(shimBin, 'claude.CMD')
   writeFileSync(claudeShim, '@echo off\n')
   const winEnv = { PATH: shimBin, PATHEXT: '.COM;.EXE;.BAT;.CMD' }
   expect(resolveSpawnable('claude', ['mcp', 'add', '--header', 'Authorization: Bearer x'], npmCli, winNode, 'win32', winEnv, 'C:\\Windows'))
-    .toEqual({ cmd: 'C:\\Windows\\System32\\cmd.exe', args: ['/d', '/s', '/c', claudeShim, 'mcp', 'add', '--header', 'Authorization: Bearer x'] })
+    .toEqual({ cmd: 'C:\\Windows\\System32\\cmd.exe', args: ['/d', '/v:off', '/s', '/c', `""${claudeShim}" "mcp" "add" "--header" "Authorization: Bearer x""`], windowsVerbatimArguments: true })
   // Not on PATH → passthrough; the bare spawn fails and the probe treats it as not-installed.
   expect(resolveSpawnable('claude', ['--version'], npmCli, winNode, 'win32', { PATH: '' }))
     .toEqual({ cmd: 'claude', args: ['--version'] })

test/spawn-windows.test.ts

@@ -0,0 +1,49 @@
+import { spawnSync } from 'node:child_process'
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join, win32 } from 'node:path'
+import { expect, test } from 'vitest'
+import { resolveSpawnable } from '../src/spawn.js'
+import { resolveRunSpec } from '../src/commands/upgrade.js'
+
+test('native Windows shim invocations preserve their quoted command line', () => {
+  const dir = mkdtempSync(join(tmpdir(), 'insta spaced '))
+  try {
+    const shim = join(dir, 'npx.CMD')
+    writeFileSync(shim, '@echo off\r\n')
+    expect(resolveSpawnable('npx', ['-y', 'two words', '', 'C:\\work dir\\'], '', 'insta.exe', 'win32', { PATH: dir, PATHEXT: '.CMD' }, 'C:\\Windows'))
+      .toEqual({
+        cmd: 'C:\\Windows\\System32\\cmd.exe',
+        args: ['/d', '/v:off', '/s', '/c', `""${shim}" "-y" "two words" "" "C:\\work dir\\\\""`],
+        windowsVerbatimArguments: true,
+      })
+  } finally { rmSync(dir, { recursive: true, force: true }) }
+})
+
+test.each(['&', '|', '<', '>', '^', '%', '"', '\r', '\n'])(
+  'Windows shim wrapping refuses shell syntax %j in arguments', (character) => {
+    const dir = mkdtempSync(join(tmpdir(), 'insta-shim-'))
+    try {
+      writeFileSync(join(dir, 'npx.CMD'), '@echo off\r\n')
+      const args = ['first', `x${character}y`]
+      expect(resolveSpawnable('npx', args, '', 'insta.exe', 'win32', { PATH: dir, PATHEXT: '.CMD' }))
+        .toEqual({ cmd: 'npx', args })
+    } finally { rmSync(dir, { recursive: true, force: true }) }
+  },
+)
+
+test.skipIf(process.platform !== 'win32')('Windows executes spaced npm shims with literal arguments', () => {
+  const dir = mkdtempSync(join(tmpdir(), 'insta spaced (shim)! '))
+  try {
+    writeFileSync(join(dir, 'npx.CMD'), '@echo off\r\n"%INSTA_TEST_NODE%" "%~dp0argv.cjs" %*\r\n')
+    writeFileSync(join(dir, 'argv.cjs'), 'console.log(JSON.stringify(process.argv.slice(2)))\n')
+    const env = { ...process.env, PATH: dir, PATHEXT: '.CMD', INSTA_TEST_NODE: process.execPath, INSTA_TEST_EXPAND: 'expanded' }
+    const args = ['-y', 'skills', 'add', 'two words', '', 'C:\\work dir\\', '*', '(parentheses)', '!INSTA_TEST_EXPAND!']
+    const spec = resolveRunSpec({ cmd: 'npx', args, env }, '', join(dir, 'insta.exe'), 'win32')
+    const result = spawnSync(spec.cmd, spec.args, { env, encoding: 'utf8', windowsVerbatimArguments: spec.windowsVerbatimArguments })
+    expect(result.error).toBeUndefined()
+    expect(result.status, result.stderr).toBe(0)
+    expect(JSON.parse(result.stdout.trim())).toEqual(args)
+    expect(spec.cmd).toBe(win32.join(process.env.SYSTEMROOT ?? process.env.windir ?? 'C:\\Windows', 'System32', 'cmd.exe'))
+  } finally { rmSync(dir, { recursive: true, force: true }) }
+})

test/upgrade-resolver.test.ts

@@ -55,7 +55,7 @@ test('the Windows version probe resolves the installed shim through a copied `Pa
   const shim = join(shimBin, 'insta.CMD')
   writeFileSync(shim, '@echo off\n')
   const cmdExe = win32.join(process.env.SYSTEMROOT ?? process.env.windir ?? 'C:\\Windows', 'System32', 'cmd.exe')
-  const wrapped = { cmd: cmdExe, args: ['/d', '/s', '/c', shim, '--version'] }
+  const wrapped = { cmd: cmdExe, args: ['/d', '/v:off', '/s', '/c', `""${shim}" "--version""`], windowsVerbatimArguments: true }
 
   // Windows' real casing, as a spread of process.env preserves it.
   const copied = { Path: shimBin, PATHEXT: '.CMD', INSTA_NO_AUTOUPDATE: '1' }