Fix Windows compatibility: subprocess preexec_fn is POSIX-only#85
Open
Le-Anh-Duy wants to merge 2 commits into
Open
Fix Windows compatibility: subprocess preexec_fn is POSIX-only#85Le-Anh-Duy wants to merge 2 commits into
Le-Anh-Duy wants to merge 2 commits into
Conversation
subprocess.Popen(preexec_fn=..., start_new_session=True) raised "preexec_fn is not supported on Windows platforms" for every LLM call and test-runner invocation, causing /cmind.encode and friends to fail outright on Windows. Branch on platform: POSIX keeps start_new_session + preexec_fn (killpg-based tree kill); Windows uses CREATE_NEW_PROCESS_GROUP and a new _kill_process_tree() helper that shells out to `taskkill /T /F` to reap the process tree instead. Verified end-to-end: reinstalled the patched package as the local cmind-cli tool and ran a full /cmind.encode against an external repo on Windows; it now completes successfully (previously failed on every LLM call).
Author
|
@microsoft-github-policy-service agree |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Windows incompatibility caused by using POSIX-only subprocess.Popen(preexec_fn=...) by branching process-spawn and process-tree cleanup behavior by platform.
Changes:
- Add a platform-aware process-tree termination helper and use it for timeout/exception cleanup.
- Make
Popeninvocation platform-specific: POSIX usesstart_new_session+preexec_fn, Windows usesCREATE_NEW_PROCESS_GROUP. - Expand
.gitignoreentries for CoderMind-managed local directories/files.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
CoderMind/scripts/common/llm_client.py |
Adds Windows-safe process spawning and cross-platform process-tree kill helper for LLM CLI invocations. |
CoderMind/scripts/code_gen/test_runner.py |
Reuses the shared process spawning/kill logic for pytest/project test execution with Windows support. |
.gitignore |
Adds ignore rules for CoderMind-managed local artifacts and config, with an exception for config.toml. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
On Windows, _kill_process_tree ignores taskkill's return code. If taskkill fails (e.g. access denied / process already exited / PID reuse edge), this function returns without killing the process, but callers then do proc.wait(), which can hang indefinitely. Please fall back to proc.kill() when taskkill returns non-zero (or use check=True and catch CalledProcessError). Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
|
@Le-Anh-Duy Thanks for your fix patch! CoderMind has not been officially tested on Windows platform. I tested your patch and it does fix the subprocess compatibility problem. However, the /cmind.encode command still fails with input errors on my Windows device. Does the /cmind.encode command run well at your side? |
Author
|
Hello, thanks for reviewing, it works fine on my machine, i'm investigating
the bug. As claude code suggested, it might be encoding issues
in CoderMind/scripts/update_graphs.py and llm_client.py:417-430, where the
program is trying to print special character like "→", which window console
might used its locale encoding system instead of utf-8, and cause the
issues. Can you try to test the patch on a smaller project with no special
character?
Vào Thứ 6, 10 thg 7, 2026 vào lúc 11:24 Qingtao Li <
***@***.***> đã viết:
… *QingtaoLi1* left a comment (microsoft/RPG-ZeroRepo#85)
<#85 (comment)>
@Le-Anh-Duy <https://github.com/Le-Anh-Duy> Thanks for your fix patch!
CoderMind has not been officially tested on Windows platform. I tested your
patch and it does fix the subprocess compatibility problem. However, the
/cmind.encode command still fails with input errors on my Windows device.
Does the /cmind.encode command run well at your side?
—
Reply to this email directly, view it on GitHub
<#85?email_source=notifications&email_token=BES56B6RKU7ITRAYPWPSEX35EBVYVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJTGE4TOMJUG4YKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4931971470>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BES56BZFEHCF7I4UZ7LNWTT5EBVYVAVCNFSNUABGKJSXA33TNF2G64TZHMYTCMBUGQZTKNJTHE5US43TOVSTWNBYGQYDCMZUGIYDNILWAI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Author
|
Hi Qingtao,
Following up on your question about whether /cmind.encode runs cleanly on
my Windows machine — I dug into it and found a real, reproducible Windows
bug. I've opened a pull request with the fix:
https://github.com/Le-Anh-Duy/RPG-ZeroRepo/tree/fix/windows-utf8-io
What I found: cmind script update_graphs.py status (the command behind the
SessionStart/post-commit hooks) crashes on Windows with:
UnicodeEncodeError: 'charmap' codec can't encode character '→' in position
...
Root cause: on Windows, a script's stdout is almost never a real console
(it's piped by cmind script, captured by a hook, etc.), so CPython falls
back to locale.getpreferredencoding() for stdio instead of UTF-8 — a legacy
code page (cp1252/cp936/...) on most Windows installs. update_graphs.py's
status output unconditionally prints an MCP-tools guidance line containing
"→" (functional areas → groups → features) whenever the RPG is readable, so
this isn't a rare edge case — it fires on the normal "status looks fine"
path.
Why I didn't catch this myself before you flagged it: the hook is
deliberately written to fail silently — cmind script update_graphs.py
status 2>/dev/null || echo '[CoderMind] RPG status unavailable' — so
instead of a visible crash, all I ever saw was that one bland fallback
line. It's easy to read past that as "just a status message" rather than
"this just crashed." I confirmed it had in fact been crashing on every
session for me too, from the very start — I'd just never noticed, because
it degrades gracefully instead of blocking anything.
What's still unconfirmed: whether this exact code path is what broke
/cmind.encode itself for you. run_encode.py / check_encode.py's own
print(json.dumps(...)) calls are ASCII-safe by default, so they shouldn't
hit this — which is also consistent with encode always working fine on my
end. If some other print()/log call deeper in the encode pipeline emitted a
non-ASCII character, it would crash the same way, just somewhere else. If
you still have it, could you send me the exact traceback/error text from
your /cmind.encode failure? That will tell us for sure whether it's the
same bug or a second one to fix.
Thanks again for pushing on this — happy to iterate further once I hear
back on the traceback.
Vào Thứ 6, 10 thg 7, 2026 vào lúc 11:24 Qingtao Li <
***@***.***> đã viết:
… *QingtaoLi1* left a comment (microsoft/RPG-ZeroRepo#85)
<#85 (comment)>
@Le-Anh-Duy <https://github.com/Le-Anh-Duy> Thanks for your fix patch!
CoderMind has not been officially tested on Windows platform. I tested your
patch and it does fix the subprocess compatibility problem. However, the
/cmind.encode command still fails with input errors on my Windows device.
Does the /cmind.encode command run well at your side?
—
Reply to this email directly, view it on GitHub
<#85?email_source=notifications&email_token=BES56B6RKU7ITRAYPWPSEX35EBVYVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJTGE4TOMJUG4YKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4931971470>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BES56BZFEHCF7I4UZ7LNWTT5EBVYVAVCNFSNUABGKJSXA33TNF2G64TZHMYTCMBUGQZTKNJTHE5US43TOVSTWNBYGQYDCMZUGIYDNILWAI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Comment on lines
11
to
13
| import os as _os | ||
| import platform as _platform | ||
| import re |
Comment on lines
23
to
25
| from common.llm_client import LLMClient | ||
| from common.llm_client import _IS_WINDOWS, _kill_process_tree, _set_pdeathsig | ||
| import json as _json |
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.
subprocess.Popen(preexec_fn=..., start_new_session=True) raised "preexec_fn is not supported on Windows platforms" for every LLM call and test-runner invocation, causing /cmind.encode and friends to fail outright on Windows.
Branch on platform: POSIX keeps start_new_session + preexec_fn (killpg-based tree kill); Windows uses CREATE_NEW_PROCESS_GROUP and a new _kill_process_tree() helper that shells out to
taskkill /T /Fto reap the process tree instead.Verified end-to-end: reinstalled the patched package as the local cmind-cli tool and ran a full /cmind.encode against an external repo on Windows; it now completes successfully (previously failed on every LLM call).