fix: avoid false Web UI build failures on Windows GBK locales#23647
fix: avoid false Web UI build failures on Windows GBK locales#23647VinceZcrikl wants to merge 1 commit into
Conversation
|
Adding context in case this looks adjacent to #13368: this PR is intended to address a different Windows failure mode. #13368 focuses on:
This PR focuses specifically on the subprocess decoding path during In the failure I reproduced,
The change here makes the relevant npm subprocess calls explicit and locale-independent by setting UTF-8 decoding with replacement semantics for captured output. So this PR is meant to prevent a false-negative build result caused by output decoding, rather than fixing the separate shell / asset-sync / console-printing issues covered by #13368. |
|
hermes update → Fetching updates... [notice] A new release of pip is available: 25.0.1 -> 26.1.1 ✓ Code updated! → Syncing bundled skills... → Syncing bundled skills to all profiles... → Checking configuration for new options... ✓ Update complete! Tip: You can now select a provider and model: here is the error log I met when running hermes update on windows Edition Windows 11 Pro |
On Windows systems using a Chinese GBK locale, `hermes update` could misreport the Web UI build as failed even when `npm run build` actually succeeded. The failure was caused by Python decoding captured npm output with the process locale inside a background subprocess reader thread. When npm emitted bytes such as `0x85`, decoding under GBK raised `UnicodeDecodeError`, and Hermes then surfaced a misleading "Web UI build failed" warning. This change makes the npm install/npm ci path and the Web UI build step decode captured output explicitly as UTF-8 with `errors="replace"`. That keeps unexpected bytes from crashing output collection, preserves successful builds, and prevents false negatives during update on Windows. The patch also adds regression tests that verify these subprocess calls always use explicit UTF-8 decoding with replacement semantics.
94fff19 to
95a5274
Compare
|
Follow-up: I corrected the earlier inflated file diff. The large I have now republished the branch with LF-normalized file contents, and the PR diff is back to the expected logical size:
Current breakdown:
So the current diff should now accurately reflect only the intended code changes. |
|
Salvaged onto current main via #23850 — your commit is on main with your authorship preserved in git log. Thanks! |
* upstream/main: (1099 commits) chore: ruff auto-fix C401, C416, C408, PLR1722 (NousResearch#23940) feat(prompt-cache): cross-session 1h prefix cache for Claude on Anthropic / OpenRouter / Nous Portal (NousResearch#23828) chore: ruff auto-fix PLR6201 — tuple → set in membership tests (NousResearch#23937) chore(release): add AUTHOR_MAP entry for wuli666 fix(auxiliary): evict async wrappers on poisoned client (follow-up to NousResearch#23482) fix(cli,tui): align CJK / wide-char markdown tables (NousResearch#23863) chore: ruff auto-fixes — collapsible-else-if, if-stmt-min-max, dict.fromkeys (NousResearch#23926) fix(/model): surface Nous Portal models from remote catalog manifest (NousResearch#23912) fix(cli): defensive _slash_confirm_state access + AUTHOR_MAP fix: use TUI modal for slash confirmations rebuild model catalog fix(dashboard): validate dist exists when --skip-build is set fix(dashboard): fallback to stale dist, retry build, add --skip-build flag chore: AUTHOR_MAP entry for VinceZcrikl noreply (NousResearch#23647) fix: make web UI build output decoding robust on Windows fix(agent): catch ChatGPT-account Codex data-URL rejection so images are stripped instead of cascading to compression (NousResearch#23602) revert: roll back /goal checklist + /subgoal feature stack (NousResearch#23813) chore: AUTHOR_MAP entries for sudo-hardening salvage contributors fix(approval): catch sudo with stdin/askpass/shell privilege flags fix(terminal): block sudo -S password guessing when SUDO_PASSWORD is not set ...
Summary
npm run buildsubprocess output to use the same decoding strategyProblem
On Windows systems configured with a Chinese GBK locale,
hermes updatecould incorrectly report that the Web UI build failed even when the frontend build itself actually succeeded.The observed failure mode was:
UnicodeDecodeError: 'gbk' codec can't decode byte 0x85 ...Web UI build failed (hermes web will not be available)This was misleading because running
npm run buildmanually inweb/still succeeded.Root cause
The update path captures npm output through Python subprocess APIs. On affected Windows environments, that captured output was being decoded with the process locale instead of a fixed encoding. When npm emitted bytes that are valid in UTF-8 output but invalid under GBK decoding, Python raised
UnicodeDecodeErrorinside the background reader thread. That decoding failure then caused Hermes to treat the Web UI build path as failed even though the underlying npm command completed successfully.Fix
This patch makes the relevant npm subprocess calls explicit and locale-independent by setting:
text=Trueencoding="utf-8"errors="replace"The change is applied to:
npm ci/npm install)npm run build)Using UTF-8 matches npm's expected output in this workflow, and
errors="replace"ensures unexpected bytes do not crash output collection.Validation
I verified the fix with the following checks:
./venv/Scripts/python.exe -m pytest tests/hermes_cli/test_web_ui_build.py -q -o addopts=→13 passednpm run buildinweb/still completed successfullyWhy this approach
This is a narrow, low-risk fix: