Skip to content

fix(tools): improve Windows local execution#12795

Closed
gabsprogrammer wants to merge 1 commit into
NousResearch:mainfrom
gabsprogrammer:gabs/fix-windows-local-execution
Closed

fix(tools): improve Windows local execution#12795
gabsprogrammer wants to merge 1 commit into
NousResearch:mainfrom
gabsprogrammer:gabs/fix-windows-local-execution

Conversation

@gabsprogrammer

Copy link
Copy Markdown

Summary

Fixes native Windows local terminal execution issues discovered while running Hermes from a Windows checkout with Git Bash installed.

What changed

  • Prefer Git Bash over Windows/WSL bash.exe launchers when resolving the local shell on Windows.
  • Use a Windows-safe stdout drain path instead of relying on select.select() for subprocess pipes.
  • Avoid waiting on inherited pipe handles from background children on Windows.
  • Use taskkill /T /F for Windows process-tree termination in local execution and process registry paths.
  • Add a small Git Bash compatibility shim so python3 maps to python when python3 is unavailable.
  • Use Win32 process exit-code checks for recovered host PID liveness on Windows.
  • Add/adjust tests for Windows shell selection, stdout draining, process recovery, and detached process killing.

Why

The existing local backend could execute commands on Windows but lose stdout because Windows pipes are not compatible with the POSIX select.select() drain loop. In this environment, LocalEnvironment.execute('echo hermes-local-test') returned returncode: 0 with empty output before the fix. The Windows PATH also resolved bash to the WSL launcher before Git Bash, which is not the shell the native Windows local backend expects.

Validation

Tested on Windows with Git Bash installed:

python -m pytest -o "addopts=" tests\tools\test_windows_compat.py tests\tools\test_base_environment.py tests\tools\test_local_tempdir.py tests\tools\test_terminal_timeout_output.py tests\tools\test_local_background_child_hang.py tests\tools\test_process_registry.py -q

Result: 86 passed.

Also ran:

python -m py_compile tools\environments\base.py tools\environments\local.py tools\process_registry.py tests\tools\test_windows_compat.py tests\tools\test_process_registry.py
git diff --check

git diff --check only reported expected CRLF conversion warnings from Git for this Windows checkout.

@gabsprogrammer

Copy link
Copy Markdown
Author

This does not claim full native Windows support; it fixes existing Windows-specific local execution paths and adds regression coverage

@gabsprogrammer gabsprogrammer marked this pull request as ready for review April 20, 2026 03:26
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets tool/terminal Terminal execution and process management backend/local Local shell execution labels Apr 23, 2026
@teknium1

teknium1 commented May 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for this — appreciate the work. We're closing the entire cluster of open native-Windows PRs (44 of them spanning installer, terminal routing, file ops, gateway PID handling, encoding, docs, and more) because the surface area needs a designed, consolidated approach rather than piecemeal merges. Cherry-picking individual fixes keeps leaving inconsistencies and we'd rather land Windows support properly, in one coherent pass.\n\nYour PR is catalogued in our internal Windows support plan. When we pick this back up (soon), we'll mine every PR in the cluster for its fix shape and credit all contributors whose work informs the final patch via lines. Watch for the consolidating PR and feel free to chime in with context on the specific failure mode you were hitting.\n\nClosing for now, not as a rejection of the fix — just queueing it for the designed rollout. Thanks again.

@teknium1 teknium1 closed this May 7, 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 comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants