fix(windows): harden native CLI/TUI dependency bootstrap - #21631
Closed
OutThisLife wants to merge 2 commits into
Closed
fix(windows): harden native CLI/TUI dependency bootstrap#21631OutThisLife wants to merge 2 commits into
OutThisLife wants to merge 2 commits into
Conversation
Handle native Windows dependency edge cases by avoiding npm.ps1 execution-policy failures, persisting managed Node resolution, and validating runtime imports per platform.
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
invalid-argument-type |
3 |
First entries
tests/test_utf8_bootstrap.py:103: [invalid-argument-type] invalid-argument-type: Method `__getitem__` of type `bound method Top[dict[Unknown, Unknown]].__getitem__(key: Never, /) -> object` cannot be called with key of type `Literal["PYTHONIOENCODING"]` on object of type `Top[dict[Unknown, Unknown]]`
tests/test_utf8_bootstrap.py:102: [invalid-argument-type] invalid-argument-type: Method `__getitem__` of type `bound method Top[dict[Unknown, Unknown]].__getitem__(key: Never, /) -> object` cannot be called with key of type `Literal["PYTHONUTF8"]` on object of type `Top[dict[Unknown, Unknown]]`
tests/test_utf8_bootstrap.py:145: [invalid-argument-type] invalid-argument-type: Method `__getitem__` of type `bound method Top[dict[Unknown, Unknown]].__getitem__(key: Never, /) -> object` cannot be called with key of type `Literal["_HERMES_UTF8_REEXEC"]` on object of type `Top[dict[Unknown, Unknown]]`
✅ Fixed issues: none
Unchanged: 4033 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
alt-glitch
added
type/bug
Force UTF-8 defaults on legacy Windows by re-execing Hermes entrypoints with -X utf8, preventing locale codec crashes from implicit text encoding in file and stdio paths.
Collaborator
Author
|
@teknium1 maybe salvage whats useful from this into your PR? |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens native Windows bootstrap for Hermes’ CLI/TUI by improving PowerShell install reliability (Node/npm + Python extras), making LocalEnvironment PATH repair Windows-native, and introducing a Windows UTF-8 re-exec bootstrap to avoid legacy locale encoding failures in entrypoints.
Changes:
- Add
utf8_bootstrap.ensure_windows_utf8_mode()and wire it into key entrypoints to re-exec with-X utf8when needed. - Improve
scripts/install.ps1Windows robustness: safer npm invocation, Python extras fallback chain, and persistence of managed Node via PATH +HERMES_NODE. - Make
LocalEnvironmentPATH repair Windows-native (separator + case-insensitive dedupe) and add regression tests for PATH + installer branch logic.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
utf8_bootstrap.py |
Adds Windows UTF-8 bootstrap helper with optional marker-gated re-exec. |
tools/environments/local.py |
Makes PATH “saning” Windows-native (; separator, case-insensitive dedupe, avoids POSIX injection). |
tests/tools/test_local_env_blocklist.py |
Adds tests covering Windows PATH behavior and dedupe. |
tests/test_utf8_bootstrap.py |
Adds unit tests for UTF-8 bootstrap re-exec / marker / guard behaviors. |
tests/test_install_ps1_windows_dependency_paths.py |
Adds regression assertions to lock in critical Windows installer branches. |
scripts/install.ps1 |
Hardened installer: npm resolution workaround, Python extras fallback chain, managed Node persistence. |
run_agent.py |
Calls UTF-8 bootstrap in the agent entrypoint. |
hermes_cli/main.py |
Calls UTF-8 bootstrap early to protect CLI startup defaults. |
gateway/run.py |
Adds UTF-8 bootstrap call for direct gateway invocation. |
cli.py |
Adds UTF-8 bootstrap call in CLI main. |
acp_adapter/entry.py |
Calls UTF-8 bootstrap at ACP adapter import-time for legacy Windows defaults. |
pyproject.toml |
Ensures utf8_bootstrap is shipped via py-modules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+45
to
+49
| return False | ||
| if not reexec: | ||
| return False | ||
| if os.environ.get(_UTF8_REEXEC_GUARD) == "1": | ||
| return False |
Comment on lines
+23
to
+43
| """Ensure UTF-8 defaults on Windows. | ||
|
|
||
| Behavior: | ||
| - Always sets ``PYTHONUTF8=1`` and ``PYTHONIOENCODING=utf-8`` on Windows. | ||
| - If Python is already in UTF-8 mode, returns immediately. | ||
| - Otherwise re-execs the current interpreter with ``-X utf8`` (once), | ||
| unless marker-gated or explicitly disabled via ``reexec=False``. | ||
| - When ``module=...`` is supplied, re-execs as ``python -m <module>`` and | ||
| forwards original user args (excluding argv0), which avoids Windows | ||
| console-script ``.exe`` wrappers being treated as Python scripts. | ||
|
|
||
| Returns ``True`` only when a re-exec is attempted and the exec call | ||
| unexpectedly returns (e.g. under a patched test double). In normal | ||
| operation ``os.execvpe`` never returns on success. | ||
| """ | ||
| if sys.platform != "win32": | ||
| return False | ||
|
|
||
| os.environ.setdefault("PYTHONUTF8", "1") | ||
| os.environ.setdefault("PYTHONIOENCODING", "utf-8") | ||
|
|
Comment on lines
+83
to
+86
| $normalizedEntry = $Entry.Trim().TrimEnd("\") | ||
| foreach ($part in $parts) { | ||
| if ($part.Trim().TrimEnd("\") -ieq $normalizedEntry) { | ||
| return $CurrentPath |
Comment on lines
+18
to
+20
| def test_node_install_keeps_winget_and_zip_fallback_paths() -> None: | ||
| text = INSTALL_PS1.read_text() | ||
|
|
Contributor
|
Closing as superseded by #21561. Triage notes (medium confidence): Thanks for the contribution — the underlying problem this PR addresses has been resolved by the linked PR on current main. If you believe this was closed in error, please comment and we'll reopen. (Bulk-closed during a CLI PR triage sweep.) |
alt-glitch
added
comp/acp
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.
Summary
scripts/install.ps1execution-policy-safe for npm by resolvingnpm.cmdfirst, convertingnpm.ps1to.cmdwhen needed, and falling back tonode npm-cli.js.[all]->.[pty,mcp,honcho,acp]->.HERMES_NODEwhen Hermes installs Node under%LOCALAPPDATA%\\hermes\\nodeLocalEnvironmentPATH repair Windows-native (;separator, case-insensitive dedupe viantpath, no POSIX/usr/bininjection)-X utf8(marker-gated, single-pass) so implicit text-modeopen()calls use UTF-8 defaults on legacy Windows localesScope
--tuiinstall/bootstrap reliability, plus Windows default text encoding hardening at Hermes entrypointsTest plan
scripts/run_tests.sh /Users/brooklyn/www/hermes-agent-win-cli-tui/tests/test_utf8_bootstrap.py /Users/brooklyn/www/hermes-agent-win-cli-tui/tests/test_install_ps1_windows_dependency_paths.py /Users/brooklyn/www/hermes-agent-win-cli-tui/tests/tools/test_local_env_blocklist.pyNative Windows manual matrix
wingetpresent + restrictive execution policy (npm.ps1blocked): runscripts/install.ps1and confirm both repo-root andui-tuinpm installs succeedwinget: confirm Node ZIP fallback installs to%LOCALAPPDATA%\\hermes\\nodeand install completeshermes --tuistarts without needing manual Node path editsexecute_codewith non-ASCII file contents and verify no decode/encode crash from default text mode I/O