openai / openai/codex-plugin-cc
Windows: taskkill /PID is mangled by MSYS path conversion when SHELL is set (Git Bash under Claude Code) — cancel and SessionEnd cleanup never kill the process tree
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 33.3k
- Forks
- 2.3k
- PR merge metrics
- No merged PRs in 30d
Description
Summary
On Windows, terminateProcessTree() invokes taskkill /PID <pid> /T /F through a shell because runCommand defaults to shell: process.env.SHELL || true on win32 (plugins/codex/scripts/lib/process.mjs). When SHELL points to Git Bash — which is exactly the environment Claude Code provides on Windows (SHELL=/bin/bash.exe) — MSYS argument conversion rewrites the /PID switch into a filesystem path, so taskkill fails and the process tree is never killed.
Reproduction
Windows 11, plugin 1.0.6, Node 24.16.0, SHELL=/bin/bash.exe (default under Claude Code's bash tool):
const { spawnSync } = require("child_process");
const r = spawnSync("taskkill", ["/PID", "999999", "/T", "/F"],
{ shell: process.env.SHELL || true, encoding: "utf8", windowsHide: true });
console.log(r.status, r.stderr || r.stdout);
Output (status 1, cp866 mojibake from the Russian locale):
Ошибка: неправильный параметр или аргумент - 'C:/Program Files/Git/PID'.
MSYS has translated /PID → C:/Program Files/Git/PID. With shell: false the same call works correctly.
Impact
/codex:cancelmarks a background jobcancelled, but the detached task-worker tree survives (the softturn/interruptpath still works when aturnIdis already known, but the hard-kill never lands).- The
SessionEndhook's job cleanup silently fails —cleanupSessionJobsswallows the error, leaking worker processes on every session end. SpawnedCodexAppServerClient.close()on Windows relies on the sameterminateProcessTree, so the shell-wrappedcodex app-servergrandchild can be leaked too.- Independently of MSYS:
looksLikeMissingProcessMessage()matches English-onlytaskkilloutput (not found|no running instance|...). On non-English Windows localestaskkillprints localized (OEM-codepage) text, so the "process already gone" detection never matches and dead-process cases surface as thrown errors.
This also shows up when running the repo's own test suite on Windows: the cancel/session-end/status tests fail with the mangled-/PID assertion above (CI currently runs ubuntu-latest only, so it goes unnoticed).
Suggested fix
- Run
taskkillwithshell: false— its arguments are fixed strings, so the shell adds nothing. This mirrors what #447 already did for git ("repository-derived arguments must never pass through a shell"); the same reasoning applies to the win32 defaultshell: process.env.SHELL || true, which makes behavior depend on whateverSHELLhappens to be. - Prefer exit-code-based detection over locale-dependent stderr parsing:
taskkillexits with code 128 when the process does not exist.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in plugins/codex/scripts/lib/process.mjs at terminateProcessTree() and looksLikeMissingProcessMessage(), then run the cancel, session-end, and status tests on Windows with SHELL set to /bin/bash.exe. Done means process-tree cleanup works without MSYS mangling and already-exited processes are handled across Windows locales.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- operating-systems, tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100