fix(windows): stop rejecting .cmd spawns under Program Files (x86)

The move of hasUnsafeWindowsBatchSyntax into src/shared/windows-batch-spawn.ts
silently added `(` and `)` to the cmd.exe denylist, so every .cmd shim or
argument path containing parentheses became unspawnable across nine call sites.
Parentheses only group commands and cannot chain one without a separator the
guard already rejects, so they are dropped again.

The rejected character set is now the single source for the user-facing error
strings, and `orca account add` translates the sentinel into a real message.

Co-authored-by: Orca <help@stably.ai>

src/cli/handlers/account.test.ts

@@ -49,7 +49,11 @@ vi.mock('../../shared/node-cli-command-resolution', () => ({
 import { ACCOUNT_HANDLERS } from './account'
 import type { HandlerContext } from '../dispatch'
 import type { RuntimeClient } from '../runtime-client'
-import { getCmdExePath } from '../../shared/windows-batch-spawn'
+import {
+  getCmdExePath,
+  WINDOWS_BATCH_UNSAFE_ARGUMENTS_ERROR,
+  WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL
+} from '../../shared/windows-batch-spawn'
 import { ACCOUNT_IMPORT_RUNTIME_CAPABILITY } from '../../shared/protocol-version'
 
 function successfulChild(): EventEmitter {
@@ -168,6 +172,35 @@ describe('account CLI handlers', () => {
     )
   })
 
+  it('launches a Windows shim installed under Program Files (x86)', async () => {
+    Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
+    const shim = 'C:\\Program Files (x86)\\nodejs\\codex.cmd'
+    resolveCliCommandMock.mockReturnValue(shim)
+
+    await ACCOUNT_HANDLERS['account add'](context('codex'))
+
+    expect(spawnMock).toHaveBeenCalledWith(
+      getCmdExePath(),
+      ['/d', '/c', shim, 'login', '--device-auth'],
+      expect.anything()
+    )
+  })
+
+  it('explains an unspawnable Windows shim path instead of leaking the error sentinel', async () => {
+    Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
+    resolveCliCommandMock.mockReturnValue('C:\\Users\\A&B\\codex.cmd')
+
+    const error = await ACCOUNT_HANDLERS['account add'](context('codex')).catch(
+      (thrown: unknown) => thrown
+    )
+
+    expect(spawnMock).not.toHaveBeenCalled()
+    const message = error instanceof Error ? error.message : String(error)
+    expect(message).not.toBe(WINDOWS_BATCH_UNSAFE_ARGUMENTS_ERROR)
+    expect(message).toContain('C:\\Users\\A&B\\codex.cmd')
+    expect(message).toContain(WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL)
+  })
+
   it('adds version-manager Node paths to the login child environment', async () => {
     const nodeBin = '/home/test/.nvm/versions/node/v22.0.0/bin'
     getVersionManagerBinPathsMock.mockReturnValue([nodeBin])

src/cli/handlers/account.ts

@@ -16,7 +16,11 @@ import {
   getVersionManagerBinPaths,
   resolveCliCommand
 } from '../../shared/node-cli-command-resolution'
-import { getSpawnArgsForWindows } from '../../shared/windows-batch-spawn'
+import {
+  getSpawnArgsForWindows,
+  UnsafeWindowsBatchArgumentsError,
+  WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL
+} from '../../shared/windows-batch-spawn'
 import { ACCOUNT_IMPORT_RUNTIME_CAPABILITY } from '../../shared/protocol-version'
 import type { RuntimeStatus } from '../../shared/runtime-types'
 import type { ClaudeRateLimitAccountsState, CodexRateLimitAccountsState } from '../../shared/types'
@@ -86,7 +90,25 @@ async function runAgentLoginInTerminal(
 ): Promise<void> {
   await new Promise<void>((resolvePromise, rejectPromise) => {
     const resolvedCommand = resolveCliCommand(command)
-    const { spawnCmd, spawnArgs } = getSpawnArgsForWindows(resolvedCommand, args)
+    let spawnCmd: string
+    let spawnArgs: string[]
+    try {
+      ;({ spawnCmd, spawnArgs } = getSpawnArgsForWindows(resolvedCommand, args))
+    } catch (error) {
+      // Why: the bare sentinel message reaches the user verbatim otherwise, with
+      // nothing naming the path or the characters that made it unspawnable.
+      rejectPromise(
+        error instanceof UnsafeWindowsBatchArgumentsError
+          ? new RuntimeClientError(
+              'invalid_environment',
+              `Cannot run \`${command}\` from "${resolvedCommand}": the path contains characters ` +
+                `cmd.exe would reinterpret. Install it somewhere without ` +
+                `${WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL} in the path.`
+            )
+          : error
+      )
+      return
+    }
     const env = addAgentNodePaths({ ...stripElectronRunAsNode(process.env), ...extraEnv })
     const child = spawn(spawnCmd, spawnArgs, {
       // Why: JSON mode reserves stdout for the response envelope while keeping

src/cli/handlers/skills.ts

@@ -12,7 +12,8 @@ import {
 } from '../../shared/tui-agent-detection-commands'
 import {
   getSpawnArgsForWindows,
-  UnsafeWindowsBatchArgumentsError
+  UnsafeWindowsBatchArgumentsError,
+  WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL
 } from '../../shared/windows-batch-spawn'
 import { isSkillsCliAgentKeyShaped, toSkillsCliAgentKeys } from '../../shared/skills-cli-agent-keys'
 import {
@@ -134,7 +135,7 @@ function runNpxSkills(args: string[]): Promise<number> {
         new RuntimeClientError(
           'invalid_environment',
           `Cannot run npx from "${resolved}": the path contains characters cmd.exe would ` +
-            'reinterpret. Install Node.js somewhere without & | < > ^ " % ! in the path.'
+            `reinterpret. Install Node.js somewhere without ${WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL} in the path.`
         )
       )
       return

src/cli/skills.test.ts

@@ -2,6 +2,7 @@ import { EventEmitter } from 'node:events'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import { delimiter } from 'node:path'
 import type * as CodexCliCommandModule from '../shared/node-cli-command-resolution'
+import { WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL } from '../shared/windows-batch-spawn'
 
 const {
   detectCommandsMock,
@@ -689,6 +690,26 @@ describe('orca skills CLI', () => {
     expect(spawnMock).not.toHaveBeenCalled()
     expect(process.exitCode).toBe(1)
     expect(String(errorSpy.mock.calls[0]?.[0])).toContain('cmd.exe would reinterpret')
+    // Why: remediation advice that names the wrong characters is unactionable.
+    expect(String(errorSpy.mock.calls[0]?.[0])).toContain(WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL)
+  })
+
+  it('runs npx from a Program Files (x86) install instead of refusing it', async () => {
+    const child = createFakeChild()
+    spawnMock.mockReturnValue(child)
+    vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
+    vi.stubEnv('ComSpec', 'C:\\Windows\\System32\\cmd.exe')
+    const npx = 'C:\\Program Files (x86)\\nodejs\\npx.cmd'
+    resolveCliCommandMock.mockReturnValue(npx)
+    vi.spyOn(process.stderr, 'write').mockImplementation(() => true)
+
+    const resultPromise = main(['skills', 'install', '--skill', 'alpha'], '/tmp/repo')
+    await vi.waitFor(() => expect(spawnMock).toHaveBeenCalled())
+    child.emit('exit', 0, null)
+    await resultPromise
+
+    expect(spawnMock.mock.calls[0]?.[0]).toBe('C:\\Windows\\System32\\cmd.exe')
+    expect(spawnMock.mock.calls[0]?.[1]?.slice(0, 3)).toEqual(['/d', '/c', npx])
   })
 
   it('never puts the current directory on the child PATH when npx is unresolvable', async () => {

src/main/skills/skill-update-run.test.ts

@@ -1,6 +1,10 @@
 import { EventEmitter } from 'node:events'
 import { describe, expect, it, vi } from 'vitest'
 import type { SkillUpdateRun } from '../../shared/skill-freshness'
+import {
+  getSpawnArgsForWindows,
+  WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL
+} from '../../shared/windows-batch-spawn'
 import { CANCEL_RELEASE_TIMEOUT_MS, SkillUpdateRunner } from './skill-update-run'
 
 class FakeChild extends EventEmitter {
@@ -293,6 +297,24 @@ describe('SkillUpdateRunner', () => {
     expect(run.state).toBe('error')
     expect(run.state === 'error' && run.failedNames).toEqual(['orca-cli'])
     expect(states.at(-1)?.state).toBe('error')
+    // Why: the message used to name a character set the guard no longer matches.
+    expect(run.state === 'error' && run.message).toContain(WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL)
+  })
+
+  it('spawns npx from a Program Files (x86) install instead of refusing it', () => {
+    const platform = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
+    try {
+      const npx = 'C:\\Program Files (x86)\\nodejs\\npx.cmd'
+      const { runner, spawnCalls } = makeRunner({
+        resolveCommand: () => npx,
+        buildSpawnArgs: getSpawnArgsForWindows
+      })
+
+      expect(runner.start(['orca-cli']).started).toBe(true)
+      expect(spawnCalls[0]?.args.slice(0, 3)).toEqual(['/d', '/c', npx])
+    } finally {
+      platform.mockRestore()
+    }
   })
 
   it('coalesces progress frames into one push instead of one per chunk', async () => {

src/main/skills/skill-update-run.ts

@@ -6,7 +6,7 @@ import {
 } from '../../shared/skill-freshness'
 import { resolveCliCommand } from '../codex-cli/command'
 import { killWithDescendantSweep } from '../pty-descendant-termination'
-import { getSpawnArgsForWindows } from '../win32-utils'
+import { getSpawnArgsForWindows, WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL } from '../win32-utils'
 
 // Why: `skills update` prints ANSI colour and \r + erase-line progress. We show
 // this log verbatim to the user but never parse it — `update` has no --json
@@ -101,9 +101,8 @@ export class SkillUpdateRunner {
       ;({ spawnCmd, spawnArgs } = buildSpawnArgs(npxCommand, npxArgs))
     } catch {
       // Why: the names are already canonical here, so this is the cmd.exe rail
-      // rejecting the resolved npx *path* — a profile directory containing `&`,
-      // `%` or `!` is enough. Publishing the failure keeps the dialog honest;
-      // returning a bare `started: false` would leave the button dead and silent.
+      // rejecting the resolved npx *path*. Publishing the failure keeps the dialog
+      // honest; a bare `started: false` would leave the button dead and silent.
       this.runToken += 1
       this.settling = false
       this.publish({
@@ -112,7 +111,9 @@ export class SkillUpdateRunner {
         finishedAt: this.deps.now(),
         output: '',
         failedNames: canonicalNames,
-        message: `Could not run ${npxCommand} safely from this location.`
+        message:
+          `Could not run ${npxCommand} safely from this location: its path contains one of ` +
+          `${WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL}, which cmd.exe would reinterpret.`
       })
       return { started: false, reason: 'unsafe-command-path' }
     }

src/main/win32-utils.test.ts

@@ -8,7 +8,8 @@ import {
   getSpawnArgsForWindows,
   isPermissionError,
   isWindowsBatchScript,
-  resolveWindowsCommand
+  resolveWindowsCommand,
+  WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL
 } from './win32-utils'
 
 function withPlatform<T>(platform: NodeJS.Platform, fn: () => T): T {
@@ -119,7 +120,7 @@ describe('getSpawnArgsForWindows', () => {
 
   it('rejects unsafe args for .cmd scripts on win32', () => {
     withPlatform('win32', () => {
-      for (const argument of ['hello & goodbye', 'close)', '(open']) {
+      for (const argument of ['hello & goodbye', 'a | b', 'x > y', '%PATH%', 'a\nb']) {
         expect(() => getSpawnArgsForWindows('C:\\tools\\agent.cmd', [argument])).toThrow(
           'UNSAFE_WINDOWS_BATCH_ARGUMENTS'
         )
@@ -129,11 +130,9 @@ describe('getSpawnArgsForWindows', () => {
 
   it('rejects unsafe command paths for .cmd scripts on win32', () => {
     withPlatform('win32', () => {
-      for (const command of ['C:\\bad&path\\agent.cmd', 'C:\\bad(path\\agent.cmd']) {
-        expect(() => getSpawnArgsForWindows(command, ['login'])).toThrow(
-          'UNSAFE_WINDOWS_BATCH_ARGUMENTS'
-        )
-      }
+      expect(() => getSpawnArgsForWindows('C:\\bad&path\\agent.cmd', ['login'])).toThrow(
+        'UNSAFE_WINDOWS_BATCH_ARGUMENTS'
+      )
     })
   })
 
@@ -144,6 +143,48 @@ describe('getSpawnArgsForWindows', () => {
       ).toEqual(['/d', '/c', 'C:\\tools\\agent.cmd', 'package,name;version'])
     })
   })
+
+  // Why: parentheses only group commands and cannot chain one without a separator
+  // the guard already rejects, so paren-bearing paths must stay spawnable.
+  it('spawns .cmd shims under Program Files (x86) and paren-bearing worktrees', () => {
+    withPlatform('win32', () => {
+      const npx = 'C:\\Program Files (x86)\\nodejs\\npx.cmd'
+      expect(getSpawnArgsForWindows(npx, ['C:\\dev\\app (fork)'])).toEqual({
+        spawnCmd: getCmdExePath(),
+        spawnArgs: ['/d', '/c', npx, 'C:\\dev\\app (fork)']
+      })
+      expect(getSpawnArgsForWindows('C:\\tools\\agent.cmd', ['close)', '(open']).spawnArgs).toEqual(
+        ['/d', '/c', 'C:\\tools\\agent.cmd', 'close)', '(open']
+      )
+    })
+  })
+
+  it('still rejects a paren-wrapped command chain', () => {
+    withPlatform('win32', () => {
+      expect(() => getSpawnArgsForWindows('C:\\tools\\agent.cmd', ['(x & calc.exe)'])).toThrow(
+        'UNSAFE_WINDOWS_BATCH_ARGUMENTS'
+      )
+    })
+  })
+
+  it('rejects exactly the characters the message advertises', () => {
+    withPlatform('win32', () => {
+      const advertised = WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL.split(' ')
+      expect(advertised.length).toBeGreaterThan(0)
+      for (const character of advertised) {
+        expect(() => getSpawnArgsForWindows('C:\\tools\\agent.cmd', [`a${character}b`])).toThrow(
+          'UNSAFE_WINDOWS_BATCH_ARGUMENTS'
+        )
+      }
+      // Why: anything not advertised must pass, or the message misleads the user.
+      for (const character of ['(', ')', ',', ';', '@', '#', '$', "'", '~', '=']) {
+        expect(advertised).not.toContain(character)
+        expect(() =>
+          getSpawnArgsForWindows('C:\\tools\\agent.cmd', [`a${character}b`])
+        ).not.toThrow()
+      }
+    })
+  })
 })
 
 describe('resolveWindowsCommand', () => {

src/main/win32-utils.ts

@@ -7,6 +7,7 @@ export {
   getSpawnArgsForWindows,
   isWindowsBatchScript,
   WINDOWS_BATCH_UNSAFE_ARGUMENTS_ERROR,
+  WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL,
   UnsafeWindowsBatchArgumentsError
 } from '../shared/windows-batch-spawn'
 

src/shared/windows-batch-spawn.ts

@@ -21,8 +21,22 @@ export class UnsafeWindowsBatchArgumentsError extends Error {
   }
 }
 
+// Why: cmd.exe re-parses the command line, and these are the characters that can
+// start a new command or expand a variable out of an otherwise inert argument.
+// `(`/`)` are deliberately absent: they only group commands, and grouping cannot
+// chain anything without one of the separators below, so rejecting them merely
+// broke every `C:\Program Files (x86)\...` shim and paren-bearing worktree path.
+const WINDOWS_BATCH_UNSAFE_CHARACTERS = ['&', '|', '<', '>', '^', '"', '%', '!'] as const
+
+/** The rejected characters, spelled for error messages so they cannot drift from the guard. */
+export const WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL = WINDOWS_BATCH_UNSAFE_CHARACTERS.join(' ')
+
+const UNSAFE_WINDOWS_BATCH_SYNTAX = new RegExp(
+  `[${WINDOWS_BATCH_UNSAFE_CHARACTERS.map((character) => character.replace(/[\\^\]-]/, '\\$&')).join('')}\\r\\n]`
+)
+
 function hasUnsafeWindowsBatchSyntax(value: string): boolean {
-  return /[&|<>^"%!()\r\n]/.test(value)
+  return UNSAFE_WINDOWS_BATCH_SYNTAX.test(value)
 }
 
 export function getSpawnArgsForWindows(