diff --git a/js/.changeset/issue-191-shell-argv.md b/js/.changeset/issue-191-shell-argv.md new file mode 100644 index 0000000..4583299 --- /dev/null +++ b/js/.changeset/issue-191-shell-argv.md @@ -0,0 +1,5 @@ +--- +'command-stream': patch +--- + +Support `{ mode: 'shell', file, args }` ProcessRunner specifications for commands that require the platform shell, including Windows `.cmd` shims. Async and sync execution now delegate this form to Node's shell-enabled spawn APIs while preserving the existing streaming, capture, stdin, cwd, environment, and result behavior. diff --git a/js/README.md b/js/README.md index 66ebeed..c93d1e5 100644 --- a/js/README.md +++ b/js/README.md @@ -1243,6 +1243,38 @@ $`download-large-file` The enhanced `$` function returns a `ProcessRunner` instance that extends `EventEmitter`. +#### Command specifications + +`ProcessRunner` accepts three command specification shapes: + +```javascript +// Exact argv execution without a shell (preferred for native executables) +new ProcessRunner({ mode: 'exec', file, args }); + +// A completed command string interpreted by the platform shell +new ProcessRunner({ mode: 'shell', command }); + +// An executable and arguments routed through the platform shell +new ProcessRunner({ mode: 'shell', file, args }); +``` + +The shell `file`/`args` form delegates to Node's shell-enabled process spawning. It is useful on Windows for command shims such as `code.cmd`, which cannot be executed directly: + +```javascript +const install = new ProcessRunner( + { + mode: 'shell', + file: 'code.cmd', + args: ['--install-extension', 'publisher.extension'], + }, + { mirror: false } +); + +const result = await install; +``` + +As with any shell-enabled process, pass only trusted `file` and `args` values; shell metacharacters are interpreted by the platform shell. Use `mode: 'exec'` whenever the target is a native executable and exact argument boundaries are required. + #### Events - `data`: Emitted for each chunk with `{type: 'stdout'|'stderr', data: Buffer}` diff --git a/js/src/$.process-runner-execution.mjs b/js/src/$.process-runner-execution.mjs index bdc5c4b..7428d8c 100644 --- a/js/src/$.process-runner-execution.mjs +++ b/js/src/$.process-runner-execution.mjs @@ -3,7 +3,12 @@ import cp from 'child_process'; import { trace } from './$.trace.mjs'; -import { findAvailableShell, resolveSpawnCwd } from './$.shell.mjs'; +import { + buildCommandArgv, + isShellArgvSpec, + isShellCommandSpec, + resolveSpawnCwd, +} from './$.shell.mjs'; import { StreamUtils, safeWrite, asBuffer } from './$.stream-utils.mjs'; import { pumpReadable } from './$.quote.mjs'; import { createResult } from './$.result.mjs'; @@ -115,7 +120,7 @@ function spawnWithBun(argv, config) { * @returns {object} Child process */ function spawnWithNode(argv, config) { - const { cwd, env, isInteractive } = config; + const { cwd, env, isInteractive, shell } = config; trace( 'ProcessRunner', @@ -124,6 +129,7 @@ function spawnWithNode(argv, config) { command: argv[0], args: argv.slice(1), isInteractive, + shell, cwd, platform: process.platform, })}` @@ -133,6 +139,7 @@ function spawnWithNode(argv, config) { return cp.spawn(argv[0], argv.slice(1), { cwd, env, + shell, stdio: 'inherit', }); } @@ -140,6 +147,7 @@ function spawnWithNode(argv, config) { const child = cp.spawn(argv[0], argv.slice(1), { cwd, env, + shell, stdio: ['pipe', 'pipe', 'pipe'], detached: process.platform !== 'win32', }); @@ -166,12 +174,13 @@ function spawnWithNode(argv, config) { * @returns {object} Child process */ function spawnChild(argv, config) { - const { stdin } = config; + const { stdin, shell } = config; // Make sure we never try to spawn from a deleted/inaccessible working // directory, which would make the OS-level spawn fail (issue #44). config = { ...config, cwd: resolveSpawnCwd(config.cwd) }; const needsExplicitPipe = stdin !== 'inherit' && stdin !== 'ignore'; const preferNodeForInput = isBun && needsExplicitPipe; + const preferNodeForShellArgv = isBun && shell; trace( 'ProcessRunner', @@ -179,13 +188,14 @@ function spawnChild(argv, config) { `About to spawn process | ${JSON.stringify({ needsExplicitPipe, preferNodeForInput, + preferNodeForShellArgv, runtime: isBun ? 'Bun' : 'Node', command: argv[0], args: argv.slice(1), })}` ); - if (preferNodeForInput) { + if (preferNodeForInput || preferNodeForShellArgv) { return spawnWithNode(argv, config); } return isBun ? spawnWithBun(argv, config) : spawnWithNode(argv, config); @@ -558,10 +568,11 @@ function executeSyncBun(argv, options) { * @returns {object} Result object */ function executeSyncNode(argv, options) { - const { cwd, env, stdin } = options; + const { cwd, env, stdin, shell } = options; const proc = cp.spawnSync(argv[0], argv.slice(1), { cwd, env, + shell, input: getSyncStdinInput(stdin), encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'], @@ -586,7 +597,9 @@ function executeSyncNode(argv, options) { function executeSyncProcess(argv, options) { // Guard against a deleted/inaccessible working directory (issue #44). options = { ...options, cwd: resolveSpawnCwd(options.cwd) }; - return isBun ? executeSyncBun(argv, options) : executeSyncNode(argv, options); + return isBun && !options.shell + ? executeSyncBun(argv, options) + : executeSyncNode(argv, options); } /** @@ -1106,7 +1119,8 @@ export function attachExecutionMethods(ProcessRunner, deps) { } // Handle shell mode special cases - if (this.spec.mode === 'shell') { + const shellArgv = isShellArgvSpec(this.spec); + if (isShellCommandSpec(this.spec)) { const shellResult = await handleShellMode(this, deps); if (shellResult) { return this.finish(shellResult); @@ -1114,11 +1128,7 @@ export function attachExecutionMethods(ProcessRunner, deps) { } // Build command arguments - const shell = findAvailableShell(); - const argv = - this.spec.mode === 'shell' - ? [shell.cmd, ...shell.args, this.spec.command] - : [this.spec.file, ...this.spec.args]; + const argv = buildCommandArgv(this.spec); trace( 'ProcessRunner', @@ -1126,13 +1136,16 @@ export function attachExecutionMethods(ProcessRunner, deps) { `Constructed argv | ${JSON.stringify({ mode: this.spec.mode, argv, + shellArgv, originalCommand: this.spec.command, })}` ); // Log command if tracing enabled const traceCmd = - this.spec.mode === 'shell' ? this.spec.command : argv.join(' '); + this.spec.mode === 'shell' && !shellArgv + ? this.spec.command + : argv.join(' '); logShellTrace(globalShellSettings, traceCmd); // Detect interactive mode @@ -1157,6 +1170,7 @@ export function attachExecutionMethods(ProcessRunner, deps) { env, stdin, isInteractive, + shell: shellArgv, }); this.finish(result); @@ -1416,17 +1430,21 @@ export function attachExecutionMethods(ProcessRunner, deps) { this._mode = 'sync'; const { cwd, env, stdin } = this.options; - const shell = findAvailableShell(); - const argv = - this.spec.mode === 'shell' - ? [shell.cmd, ...shell.args, this.spec.command] - : [this.spec.file, ...this.spec.args]; + const shellArgv = isShellArgvSpec(this.spec); + const argv = buildCommandArgv(this.spec); const traceCmd = - this.spec.mode === 'shell' ? this.spec.command : argv.join(' '); + this.spec.mode === 'shell' && !shellArgv + ? this.spec.command + : argv.join(' '); logShellTrace(globalShellSettings, traceCmd); - const result = executeSyncProcess(argv, { cwd, env, stdin }); + const result = executeSyncProcess(argv, { + cwd, + env, + stdin, + shell: shellArgv, + }); return processSyncResult(this, result, globalShellSettings); }; diff --git a/js/src/$.shell.mjs b/js/src/$.shell.mjs index 7c27be9..ee14d43 100644 --- a/js/src/$.shell.mjs +++ b/js/src/$.shell.mjs @@ -9,6 +9,42 @@ import { trace } from './$.trace.mjs'; // Shell detection cache let cachedShell = null; +/** + * Check whether a shell spec supplies an executable and argument vector. + * @param {object} spec - ProcessRunner command specification + * @returns {boolean} + */ +export function isShellArgvSpec(spec) { + return spec.mode === 'shell' && typeof spec.file === 'string'; +} + +/** + * Check whether a shell spec supplies a completed command string. + * @param {object} spec - ProcessRunner command specification + * @returns {boolean} + */ +export function isShellCommandSpec(spec) { + return spec.mode === 'shell' && !isShellArgvSpec(spec); +} + +/** + * Build the command vector for a ProcessRunner command specification. + * @param {object} spec - ProcessRunner command specification + * @returns {string[]} + */ +export function buildCommandArgv(spec) { + if (isShellArgvSpec(spec)) { + return [spec.file, ...(spec.args ?? [])]; + } + + if (isShellCommandSpec(spec)) { + const shell = findAvailableShell(); + return [shell.cmd, ...shell.args, spec.command]; + } + + return [spec.file, ...spec.args]; +} + /** * Pick a directory that is known to exist for spawning a child process. * @returns {string} An existing fallback directory diff --git a/js/tests/fixtures/argprint.cmd b/js/tests/fixtures/argprint.cmd new file mode 100644 index 0000000..41628af --- /dev/null +++ b/js/tests/fixtures/argprint.cmd @@ -0,0 +1,2 @@ +@echo off +node "%~dp0argprint.mjs" %* diff --git a/js/tests/process-runner-shell-argv.test.mjs b/js/tests/process-runner-shell-argv.test.mjs new file mode 100644 index 0000000..58c722f --- /dev/null +++ b/js/tests/process-runner-shell-argv.test.mjs @@ -0,0 +1,59 @@ +#!/usr/bin/env node + +import { describe, expect, test } from 'bun:test'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { ProcessRunner } from '../src/$.mjs'; +import { isWindows } from './test-helper.mjs'; + +const fixturesDir = path.join( + path.dirname(fileURLToPath(import.meta.url)), + 'fixtures' +); +const argprint = path.join(fixturesDir, 'argprint.mjs'); + +function shellArgvSpec() { + if (isWindows) { + return { + mode: 'shell', + file: path.join(fixturesDir, 'argprint.cmd'), + args: ['--install-extension', 'publisher.extension'], + }; + } + + return { + mode: 'shell', + file: process.execPath, + args: [argprint, '--install-extension', 'publisher.extension'], + }; +} + +describe('ProcessRunner shell file/args mode', () => { + test('runs argv through the platform shell asynchronously', async () => { + const runner = new ProcessRunner(shellArgvSpec(), { + mirror: false, + stdin: 'ignore', + }); + + const result = await runner; + + expect(result.code).toBe(0); + expect(result.stdout).toBe( + 'ARG[--install-extension]\nARG[publisher.extension]\n' + ); + }); + + test('runs argv through the platform shell synchronously', () => { + const runner = new ProcessRunner(shellArgvSpec(), { + mirror: false, + stdin: 'ignore', + }); + + const result = runner.sync(); + + expect(result.code).toBe(0); + expect(result.stdout).toBe( + 'ARG[--install-extension]\nARG[publisher.extension]\n' + ); + }); +});