Skip to content

fix(doctor): UTF-8/latin-1 fallback when scanning .env - #8469

Closed
Tianworld wants to merge 1 commit into
NousResearch:mainfrom
Tianworld:fix/doctor-env-scan-encoding-fallback
Closed

fix(doctor): UTF-8/latin-1 fallback when scanning .env#8469
Tianworld wants to merge 1 commit into
NousResearch:mainfrom
Tianworld:fix/doctor-env-scan-encoding-fallback

Conversation

@Tianworld

Copy link
Copy Markdown
Contributor

Doctor scans ~/.hermes/.env\ for provider hints via
ead_text(). Without an encoding, or with UTF-8-only reads, cp1252-saved files (common on Windows) could raise \UnicodeDecodeError\ and abort the check. Try UTF-8 first, then latin-1, matching other .env handling in the repo.

Made with Cursor

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard duplicate This issue or pull request already exists labels Apr 28, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #8446 — same fix: UTF-8/latin-1 fallback for .env reads in hermes doctor. See also #8432 and #8415.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Windows-compatibility fix. Current main still has a strict env_path.read_text(encoding="utf-8") at hermes_cli/doctor.py:702; unlike the shared loader at hermes_cli/env_loader.py:154-158, it has no latin-1 fallback.

Problems

  • The PR has no regression test for invalid UTF-8 input. tests/hermes_cli/test_doctor.py:83-126 covers UTF-8 content under a simulated GBK locale, not the UnicodeDecodeError branch this PR adds.

Suggested changes

  • Add a doctor test using invalid UTF-8 .env bytes with an ASCII provider hint, and verify doctor reaches the tool-probe short-circuit without raising.
  • Salvage the fallback into the current hermes_cli/doctor.py:702 block; GitHub reports this stale branch as conflicting.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users area/install-update Installer, updater, packaging, wheels, doctor labels Jul 12, 2026
@Tianworld
Tianworld force-pushed the fix/doctor-env-scan-encoding-fallback branch from 728eb6b to cfc9892 Compare July 19, 2026 11:33
@Tianworld

Copy link
Copy Markdown
Contributor Author

Addressed review feedback:

  1. Rebased onto current \main\ (conflict resolved) and salvaged the UTF-8 → latin-1 fallback into the existing .env\ scan block in \hermes_cli/doctor.py.
  2. Added \ est_doctor_reads_invalid_utf8_env_via_latin1_fallback\ covering invalid UTF-8 bytes with an ASCII provider hint, verifying doctor reaches the tool-probe short-circuit without raising.

Prefer UTF-8 for ~/.hermes/.env provider scans, then latin-1 for cp1252/Notepad files. Add regression test for invalid UTF-8 bytes.
@Tianworld
Tianworld force-pushed the fix/doctor-env-scan-encoding-fallback branch from cfc9892 to 7ebdc99 Compare July 19, 2026 11:33
@alt-glitch alt-glitch added platform/windows Native Windows-specific behavior or breakage and removed duplicate This issue or pull request already exists labels Jul 20, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #8415 and the closed #8446/#8432 attempts, but this rebased PR targets the current hermes doctor scan and adds an invalid-UTF-8 regression test; it is not a duplicate of a closed attempt.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #71078 — your commit(s) were cherry-picked onto current main with your authorship preserved in git log (rebase merge). This PR was part of the class-wide close-out of bare read_text/write_text calls: all 139 remaining sites now pass explicit encoding, and a new CI linter rule prevents regressions. Thanks for the contribution!

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

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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.

3 participants