Fix: agent-runner spawn() calls fail on Windows for npm-installed CLI tools (.cmd shim) - #154
Closed
priyamkarn wants to merge 3 commits into
Closed
priyamkarn wants to merge 3 commits into
priyamkarn wants to merge 3 commits into
Conversation
…LI tools codex.ts, opencode.ts, and openhands.ts all spawned their respective CLIs via node:child_process's spawn() without shell:true. On Windows, npm-installed global CLI tools are shimmed as .cmd files, which Windows' CreateProcess cannot execute directly -- it requires cmd.exe as an intermediary. Reproduced this directly (spawn EINVAL) while testing a fake opencode.cmd stand-in with the exact same spawn pattern the production code uses. Switches all three runners to cross-spawn, the standard solution for this (same package npm itself uses internally) -- it detects .cmd/.bat targets and wraps them through cmd.exe with correct argument escaping, without needing shell:true and its shell-injection risk.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#153
Bug
libs/agent-runner/{codex,opencode,openhands}.ts all spawn their respective CLIs
using node:child_process's spawn() without shell:true. On Windows, CLI tools
installed via
npm install -gare shimmed as .cmd files, not native .exe.Windows' CreateProcess (which spawn() calls fairly directly without shell:true)
cannot execute .cmd/.bat files at all -- they require cmd.exe /c as an
intermediary.
How this was found
While testing a fake opencode.cmd stand-in via the exact same spawn pattern
the production code uses, this reproduced immediately on a real Windows
machine:
Since production code uses the identical spawn("", args, {no shell:true})
pattern for all three CLIs, this points to the same failure in real use
whenever any of them is npm-installed on Windows -- meaning the background
auto-memory-update hook likely fails silently on Windows today.
Fix
Switched all three runners to cross-spawn (the standard, widely-used solution
for this -- same package npm uses internally). It detects .cmd/.bat targets
and transparently wraps them through cmd.exe with correct argument escaping,
avoiding both the original bug and the shell-injection risk of a blanket
shell:true.
Tests
scripts/check-agent-runner-cross-spawn.js checks, for all three runners:
misconfiguration the source check alone would miss).
correct argv when exercised with a fake spawn implementation.