Skip to content

fix(doctor): decode subprocess output as utf-8 - #68546

Closed
shaoohh wants to merge 1 commit into
NousResearch:mainfrom
shaoohh:agent/fix-doctor-utf8-subprocess
Closed

fix(doctor): decode subprocess output as utf-8#68546
shaoohh wants to merge 1 commit into
NousResearch:mainfrom
shaoohh:agent/fix-doctor-utf8-subprocess

Conversation

@shaoohh

@shaoohh shaoohh commented Jul 21, 2026

Copy link
Copy Markdown

Summary

  • Decode text output from doctor diagnostic subprocesses explicitly as UTF-8 with replacement for malformed bytes.
  • Use the same safe decoding path for SSH connectivity probing and npm audit --json.

Root cause

subprocess.run(..., text=True) used the Windows ANSI code page by default. On GBK/CP936 hosts, UTF-8 subprocess output containing bytes invalid in GBK could raise a background-reader UnicodeDecodeError, leaving .stdout unavailable and causing doctor checks to fail or be silently skipped.

Testing

  • python -m py_compile hermes_cli/doctor.py tests/hermes_cli/test_doctor.py
  • Added a regression test that runs a child process emitting valid UTF-8 followed by an invalid byte, and verifies both stdout and stderr are decoded as —�.
  • scripts/run_tests.sh tests/hermes_cli/test_doctor.py -q (blocked locally: no project Python 3.11-3.13 virtual environment is installed; the only available Python is 3.14, which the project explicitly excludes)

Fixes #49499

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard platform/windows Native Windows-specific behavior or breakage area/i18n Localization, locales, translations sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 21, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as resolved by PR #70875 (merged, commit 0f732cb), which closed out this bug class codebase-wide: every text=True subprocess call now passes encoding="utf-8", errors="replace", and a CI linter rule (scripts/check-windows-footguns.py) rejects any future unguarded site. The sites this PR targeted are all guarded on current main — verified per-file. Credit for the class fix goes to @Stoltemberg (#55339, the original sweep) and @jinglun010-cpu (#60741 + the #60751 linter); thanks for your fix as well — the volume of independent PRs on this bug is what escalated it to a class-wide close-out.

@teknium1 teknium1 closed this Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/i18n Localization, locales, translations comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hermes doctor crashes with UnicodeDecodeError on Chinese Windows GBK locale

3 participants