Skip to content

feat(js): support shell file/args specs (#191) - #195

Merged
konard merged 5 commits into
mainfrom
issue-191-70b38becaee6
Aug 11, 2026
Merged

feat(js): support shell file/args specs (#191)#195
konard merged 5 commits into
mainfrom
issue-191-70b38becaee6

Conversation

@konard

@konard konard commented Aug 11, 2026

Copy link
Copy Markdown
Member

Summary

  • accept { mode: 'shell', file, args } ProcessRunner specifications in async and sync modes
  • delegate that form to Node's shell-enabled spawn APIs, including when command-stream is running under Bun
  • preserve the existing command-string shell path and its virtual-command/operator handling
  • document the shell trust boundary and add a patch changeset

Root cause and reproduction

ProcessRunner treated every mode: 'shell' specification as a completed command string. A shell spec containing file and args therefore reached command-string parsing with command === undefined; async execution threw from command.includes(...), while sync execution attempted an invalid shell command and returned exit code 127.

js/tests/process-runner-shell-argv.test.mjs reproduces the requested code.cmd --install-extension ... shape. On Windows it invokes a real .cmd fixture; on other platforms it exercises the same Node shell-enabled spawn boundary. The test covers both awaited and synchronous runners and failed before the implementation.

Implementation

Shell specifications are now classified as either command-string or executable-plus-args forms. The latter bypasses command-string parsing, retains the separate command vector, and opts into child_process.spawn() or spawnSync() with shell: true. Existing streaming, capture, stdin, cwd, environment, cancellation, event, and result processing remains shared with native execution.

Because Node's shell-enabled spawning still invokes the platform shell, the README advises callers to pass only trusted values and to prefer mode: 'exec' for native executables requiring exact argument boundaries.

Verification

  • bun test js/tests/ --timeout 10000 — 799 passed, 5 skipped, 0 failed across 58 files
  • focused shell argv, start/run options, and sync suites — 41 passed, 0 failed
  • direct Node 20 async and sync compatibility check — passed
  • bun run lint — passed (one pre-existing warning in $.cd.mjs)
  • bun run format:check — passed
  • bun run check:duplication — passed
  • bun scripts/validate-changeset.mjs against the PR base/head — passed

Language parity

This is intentionally JavaScript-only: it exposes Node's platform shell command construction for .cmd shims and has no corresponding Rust runtime API. The PR is labeled parity-exempt.

Fixes #191

Adding .gitkeep for PR creation (default mode).
This file will be removed when the task is complete.

Issue: #191
@konard konard self-assigned this Aug 11, 2026
konard added 2 commits August 11, 2026 10:36
Delegate shell-enabled executable spawning to Node so Windows command
shims can receive separate argument arrays.

Closes #191
@konard konard added the parity-exempt Change is intentionally single-language; skip JS/Rust parity check label Aug 11, 2026
@konard konard changed the title [WIP] Support argv-safe Windows command shims in JavaScript shell mode feat(js): support shell file/args specs (#191) Aug 11, 2026
@konard
konard marked this pull request as ready for review August 11, 2026 10:58
@konard

konard commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

Working session summary

Implemented and finalized PR #195.

  • Added async and sync { mode: 'shell', file, args } support, including Windows .cmd shims.
  • Added cross-platform regression tests, documentation, security guidance, and a patch changeset.
  • Local suite: 799 passed, 5 skipped.
  • Lint, formatting, duplication, changeset validation, and Node compatibility passed.
  • Final CI passed on Windows, macOS, and Ubuntu.
  • Investigated the earlier Windows failure as an unrelated timing-sensitive SIGTERM test race; the new .cmd tests passed in that run and the clean final run passed without diagnostics.

The branch is synchronized, the worktree is clean, main is merged, and the PR is ready for review and merge.


This summary was automatically extracted from the AI working session output.

@konard

konard commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

🤖 Solution Draft Log

This log file contains the complete execution trace of the AI solution draft process.

💰 Cost estimation:

  • Model: GPT-5.6 Sol
  • Provider: OpenAI
  • Public pricing estimate: $23.625975

📊 Context and tokens usage:

  • 715.6K / 200K (358%) input tokens, 57.8K / 128K (45%) output tokens

Total: (715.6K + 13.9M cached) input tokens, 57.8K output tokens, $23.625975 cost

🤖 Models used:

  • Tool: OpenAI Codex
  • Requested: gpt-5.6-sol
  • Thinking level: xhigh (~31999 tokens)
  • Model: GPT 5.6 Sol (gpt-5.6-sol)

📎 Log file uploaded as Gist (5869KB)


Now working session is ended, feel free to review and add any feedback on the solution draft.

@konard
konard merged commit ba87d0c into main Aug 11, 2026
12 checks passed
@konard

konard commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

🎉 Auto-merged

This pull request has been automatically merged by hive-mind.

  • All CI checks have passed

Auto-merged by hive-mind with --auto-merge flag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parity-exempt Change is intentionally single-language; skip JS/Rust parity check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support argv-safe Windows command shims in JavaScript shell mode

1 participant