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

Open
#525 1 comment 0 reactions 0 assignees View on GitHub

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 /PIDC:/Program Files/Git/PID. With shell: false the same call works correctly.

Impact
  • /codex:cancel marks a background job cancelled, but the detached task-worker tree survives (the soft turn/interrupt path still works when a turnId is already known, but the hard-kill never lands).
  • The SessionEnd hook's job cleanup silently fails — cleanupSessionJobs swallows the error, leaking worker processes on every session end.
  • SpawnedCodexAppServerClient.close() on Windows relies on the same terminateProcessTree, so the shell-wrapped codex app-server grandchild can be leaked too.
  • Independently of MSYS: looksLikeMissingProcessMessage() matches English-only taskkill output (not found|no running instance|...). On non-English Windows locales taskkill prints 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
  1. Run taskkill with shell: 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 default shell: process.env.SHELL || true, which makes behavior depend on whatever SHELL happens to be.
  2. Prefer exit-code-based detection over locale-dependent stderr parsing: taskkill exits with code 128 when the process does not exist.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.