Skip to content

fix(local): remove duplicate creationflags in _run_bash() on Windows - #28898

Closed
EloquentBrush0x wants to merge 1 commit into
NousResearch:mainfrom
EloquentBrush0x:fix/local-env-duplicate-creationflags-28874
Closed

fix(local): remove duplicate creationflags in _run_bash() on Windows#28898
EloquentBrush0x wants to merge 1 commit into
NousResearch:mainfrom
EloquentBrush0x:fix/local-env-duplicate-creationflags-28874

Conversation

@EloquentBrush0x

Copy link
Copy Markdown
Contributor

What does this PR do?

Root cause: #28874 added _popen_kwargs = {"creationflags": windows_hide_flags()} if _IS_WINDOWS else {} and **_popen_kwargs to the subprocess.Popen call in LocalEnvironment._run_bash(), but did not remove the pre-existing explicit argument:

creationflags=subprocess.CREATE_NO_WINDOW if _IS_WINDOWS else 0,

On Windows, Python raises TypeError: Popen.__init__() got multiple values for keyword argument 'creationflags' on every shell command execution — _run_bash() is completely broken.

Fix: remove the stale explicit creationflags= argument so **_popen_kwargs is the sole source of creationflags, matching the pattern already applied correctly in tools/process_registry.py (line 566) by the same PR.

Related Issue

Regression introduced in #28874.

Type of Change

  • 🐛 Bug fix

Changes Made

  • tools/environments/local.py: remove duplicate creationflags=subprocess.CREATE_NO_WINDOW if _IS_WINDOWS else 0 keyword argument from subprocess.Popen call in _run_bash() (-1 line)

How to Test

python3.11 -m pytest tests/tools/test_local_env_windows_msys.py tests/tools/test_local_env_blocklist.py -v --override-ini="addopts="

Checklist

  • Contributing Guide read | Conventional Commits | No duplicate PR
  • Single fix only | Platform: macOS
  • Docs — N/A | Cross-platform — Windows crash fix

_run_bash() passes both an explicit
`creationflags=subprocess.CREATE_NO_WINDOW if _IS_WINDOWS else 0`
keyword argument and `**_popen_kwargs` which on Windows contains
`{"creationflags": windows_hide_flags()}`. Python raises
`TypeError: Popen.__init__() got multiple values for keyword argument
'creationflags'` for every shell command on Windows.

Introduced in NousResearch#28874, which correctly removed the legacy
`creationflags=` line in `process_registry.py` but left it in
`local.py`. Fix: remove the stale explicit argument so `**_popen_kwargs`
is the sole source of `creationflags`, matching the pattern already in
`tools/process_registry.py` (line 566).
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for catching this Windows regression. This has since been fixed on main by the same one-line change, so I’m closing this as implemented on main.

Evidence from this automated hermes-sweeper review:

  • tools/environments/local.py:593 on current main builds _popen_kwargs = {"creationflags": windows_hide_flags()} if _IS_WINDOWS else {}.
  • tools/environments/local.py:605-606 passes cwd=_popen_cwd, **_popen_kwargs to subprocess.Popen() with no duplicate explicit creationflags= kwarg.
  • Commit 05f02640e1f763afb7afdae4efe6b450016a908f removes exactly the stale creationflags=subprocess.CREATE_NO_WINDOW if _IS_WINDOWS else 0 line from LocalEnvironment._run_bash().
  • That fix was merged via PR fix(windows): drop duplicate creationflags kwarg in LocalEnvironment run_bash #29059 as merge commit 62713c8b8926c5280a0dfbfcf42cf2c07ec8a047 and is contained in current origin/main.

@teknium1 teknium1 closed this Jun 15, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend/local Local shell execution P1 High — major feature broken, no workaround sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants