Do not run taskkill through a shell on Windows - #735
Open
mohammad-malik wants to merge 1 commit into
Open
Conversation
`runCommand` prefers `process.env.SHELL` on Windows, so when Claude Code runs
under Git Bash the process kill became `bash -c "taskkill /PID ..."`. MSYS path
conversion then rewrote the `/PID` switch into `C:/Program Files/Git/PID`, and
taskkill rejected it before touching the process:
ERROR: Invalid argument/option - 'C:/Program Files/Git/PID'.
Every `/codex:cancel` failed that way, as did the internal kills, so a job could
not be stopped from a Git Bash session at all.
taskkill takes `/`-switches and an argument array, so it never needed a shell.
Passing `shell: false` at this one call site fixes it without changing how any
other command runs: `runCommand` still prefers the user's shell everywhere else,
where the quoting it provides is wanted.
Verified on Windows 11 with SHELL=C:\Program Files\Git\bin\bash.exe, against a
real process that had a child of its own so the `/T` tree kill was exercised:
before the change taskkill never ran, after it the tree exits and taskkill
returns 0. The existing Windows test now also asserts the absence of a shell,
since the arguments alone were identical in both the working and broken cases.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
runCommandprefersprocess.env.SHELLon Windows. When Claude Code runs under Git Bash the process kill becamebash -c "taskkill /PID ...", and MSYS path conversion rewrote the/PIDswitch intoC:/Program Files/Git/PID. taskkill rejected it before touching the process:Every
/codex:cancelfailed that way, and so did the internal kills, so nothing could stop a running job from a Git Bash session.taskkill takes
/-switches and an argument array, so it never needed a shell. Passingshell: falseat this one call site fixes it and leaves every other command alone.runCommandstill prefers the user's shell everywhere else, where the quoting it provides is wanted. This follows the pattern from #447: fix the call site that cannot tolerate a shell, rather than changing the default for every command.Verification. Windows 11,
SHELL=C:\Program Files\Git\bin\bash.exe, against a real process with a child of its own so the/Ttree kill was exercised. Before the change taskkill never ran. After it,delivered: true, exit 0, and the tree is gone.The existing Windows test now asserts the absence of a shell too. The command and its arguments are identical in the working and the broken case, so only the shell option separates them and the old assertion could not have caught this.