fix(windows): retire the bare read_text/write_text class — salvage 11 contributor PRs + sweep + linter - #71078
Merged
Conversation
Prefer UTF-8 for ~/.hermes/.env provider scans, then latin-1 for cp1252/Notepad files. Add regression test for invalid UTF-8 bytes.
Several file-I/O call sites still use open() / Path.read_text() /
Path.write_text() without an explicit encoding, so they fall back to
the platform default. On Windows CN/JP/KR locales (GBK/CP932/CP949)
any non-ASCII byte in a config/state/user-content file raises
UnicodeDecodeError or UnicodeEncodeError and crashes the caller.
to the remaining hot paths:
- agent/copilot_acp_client.py: fs/read_text_file and fs/write_text_file
(Copilot's read_file / write_file tools,
directly reported in #18637 bug 2)
- agent/model_metadata.py: context-length YAML cache load + two
save sites (context probing is on the
call path of every model invocation)
- agent/nous_rate_guard.py: cross-session rate-limit JSON state
(read + atomic write via os.fdopen)
- cron/scheduler.py: user config.yaml read in run_job
- gateway/delivery.py: cron output writes for AI-generated
content, very likely non-ASCII
yaml.dump call sites also gain allow_unicode=True so the emitted
YAML preserves non-ASCII chars as-is instead of emitting \u escape
sequences.
Adds regression tests that monkeypatch builtins.open / Path.read_text
/ Path.write_text to simulate a GBK locale: each test raises
UnicodeDecodeError / UnicodeEncodeError unless the caller explicitly
passes encoding='utf-8'. Verified that the tests fail on main and
pass with this change, on Linux as well as on Windows.
Refs #18637
Path.read_text() and Path.write_text() without explicit encoding default to the system locale encoding. On Windows this is typically cp1252, which causes UnicodeDecodeError for UTF-8 content (JSON configs, user data, service scripts). Add encoding="utf-8" to all read_text() and write_text() calls across 8 CLI files, matching the pattern established in PR #50534 (security_audit_startup.py) and ruff rule PLW1514. Fixed files: - main.py: 4 read_text calls - auth.py: 3 read_text calls - banner.py: 1 read_text + 1 write_text - service_manager.py: 1 read_text + 4 write_text - container_boot.py: 1 read_text + 4 write_text - doctor.py: 3 read_text calls - uninstall.py: 2 read_text calls - gateway.py: 1 write_text call
Path.read_text() without an explicit encoding uses the platform's
default encoding. On Windows this is typically cp1252 or mbcs, which
causes UnicodeDecodeError or silent data corruption when reading
UTF-8 content (JSON files, user text, config with non-ASCII chars).
This is the read-side companion to the write_text() encoding fix.
Fixed the most critical locations that read JSON data, user content,
and config files across 14 files with 31 call sites.
Pattern: .read_text() → .read_text(encoding='utf-8')
json.loads(path.read_text()) → json.loads(path.read_text(encoding='utf-8'))
On Windows with a non-UTF-8 system locale (e.g. CP936/GBK on zh-CN),
Python defaults stdout/stderr to the active codepage. tools/skills_sync.py
prints glyphs such as checkmark (U+2713) and up-arrow (U+2191) that GBK
cannot encode, raising UnicodeEncodeError mid-run.
The installer (scripts/install.ps1) captures this script's stdout and the
Rust bootstrap parses it as UTF-8 expecting a JSON result frame. A GBK
byte stream (or the traceback it triggers) surfaces as:
WARN stdout read error: stream did not contain valid UTF-8
stage=config-templates state=Failed
error=install.ps1 -Stage config-templates produced no JSON result frame
(exit=Some(0))
i.e. the stage fails even though the script exits 0. install.ps1 already
sets [Console]::OutputEncoding = UTF8, but that does not propagate to the
python.exe child (Python reads PYTHONIOENCODING / locale, not the console
encoding).
Fix in two places for defense in depth:
- tools/skills_sync.py: reconfigure sys.stdout/stderr to UTF-8 at import so
output is valid UTF-8 regardless of caller or active codepage.
- scripts/install.ps1: set PYTHONIOENCODING=utf-8 and PYTHONUTF8=1 (scoped
to the call, restored afterwards) around the skills_sync.py invocation.
Addresses hermes-sweeper review on PR #54866: the installer runs tools/skills_sync.py as a child python.exe whose PYTHONIOENCODING / PYTHONUTF8 the scoped install.ps1 block sets, but there was no regression test for this child-Python UTF-8 path. The existing test_child_process_inherits_utf8_mode covers a different (bootstrap entry-point) flow. Add TestSkillsSyncUtf8Guard: three subprocess tests that import skills_sync (triggering its import-time stdout/stderr reconfigure) and assert the checkmark/up-arrow glyphs the script prints at tools/skills_sync.py:596,675 emit valid UTF-8 and exit 0 even when the child env is left unset or explicitly hostile (gbk). A third test proves the guard is load-bearing by reproducing the crash without it. Also keep the new install.ps1 comment ASCII-only (the checkmark spelled out as U+2713) per the file's PS 5.1 parser-compatibility contract at scripts/install.ps1:79-80; the literal glyph in the comment violated that contract.
Path.write_text() without encoding defaults to system locale encoding. On Windows (cp1252), this silently corrupts non-ASCII content written to JSON files, config files, and cache files. This is the write-side counterpart to the read_text() encoding fix (PR #56115). PLW1514 only covers open() calls — Path methods are unguarded by ruff. 39 instances across 16 files, all passing py_compile. Files changed: - agent/copilot_acp_client.py (1) - tools/web_tools.py (1) - tools/xai_http.py (1) - tools/skills_hub.py (8) - gateway/slash_commands.py (1) - gateway/run.py (5) - gateway/dead_targets.py (1) - gateway/delivery.py (2) - gateway/platforms/qqbot/adapter.py (1) - hermes_cli/gateway.py (1) - hermes_cli/banner.py (1) - hermes_cli/service_manager.py (5) - hermes_cli/container_boot.py (5) - hermes_cli/uninstall.py (1) - hermes_cli/main.py (2) - hermes_cli/profiles.py (3)
On Windows with CJK locales (e.g. Chinese/GBK), pathlib.Path.read_text() defaults to the system encoding instead of UTF-8, causing UnicodeDecodeError when reading .env or .json config files that contain non-ASCII characters. Explicitly pass encoding='utf-8' to all read_text() and write_text() calls in the hindsight memory provider plugin.
The mem0 and hindsight memory-provider setup routines round-trip the user's ~/.hermes/.env: they read existing lines, update the keys they manage, and rewrite the whole file preserving every other line verbatim. Both used env_path.read_text() / write_text() with no encoding. read_text()/write_text() with no encoding fall back to the system locale (cp1252/GBK on Windows), so on a non-UTF-8 host the preserved lines get mangled or the call crashes on any non-ASCII value, and — because the reader never strips a BOM — a Notepad-edited .env makes the first key fail the in-place match and get duplicated instead of updated. Match the canonical .env readers in hermes_cli/config.py: read with encoding='utf-8-sig' (BOM-tolerant) and write with encoding='utf-8'. mem0/_setup.py already pins utf-8 for mem0.json, so this just aligns the .env path in the same file. Fixes both memory plugins in one class fix. Adds regression tests: a BOM'd .env updates the first key in place (locale-independent, fails without the fix) and non-ASCII existing lines survive the round-trip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up to review feedback: - mem0 _prompt_api_key read .env with the locale default, so a Notepad BOM hid the first key from the masked current-value lookup; read it with utf-8-sig + errors=replace like the canonical readers in hermes_cli/config.py. - hindsight _load_simple_env used plain utf-8; it also parses the Hermes .env during post_setup, where a BOM stuck to the first key. Switch to utf-8-sig + errors=replace. - Add hindsight regressions: BOM key matching in _load_simple_env and in the cloud post_setup writer, plus non-ASCII round-trip preservation, and a mem0 regression for the BOM'd masked-key lookup. The BOM tests fail without the fix on any platform. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`_render_distribution_plan` reads the target profile's `.env` to decide whether a required env var is already set (so it doesn't nag the user), using `Path.read_text()` with no encoding. Two bugs: 1. `Path.read_text()` defaults to the system locale (cp1251/GBK on Windows), which raises `UnicodeDecodeError` on any non-ASCII byte. The surrounding `except OSError` does NOT catch that — `UnicodeDecodeError` is a `ValueError` — so a mis-encoded `.env` aborts the entire install preview. 2. Even on a UTF-8 locale, a Notepad-added BOM prefixes the first key (`KEY`), so the very first required env var is mis-reported as "needs setting" when it is actually present. `.env` is written as UTF-8 everywhere in the codebase. Read it as `utf-8-sig` (tolerates the BOM) and also catch `UnicodeDecodeError` so a genuinely un-decodable file skips the pre-check instead of crashing. Regression tests: a BOM-prefixed `.env` whose first key must still read as "set", and an invalid-UTF-8 `.env` that must not abort the preview.
Path.read_text() and Path.write_text() without encoding= default to the system locale (cp1252 on Windows), which corrupts non-ASCII JSON content. Coverage-gap fix for files not addressed by prior encoding PRs: - tools/skills_hub.py: 6 read_text + 8 write_text (cache, index, lock files) - tools/skills_sync.py: 1 read_text (lock file) - tools/xai_http.py: 1 read_text + 1 write_text (auth store, marker) - agent/shell_hooks.py: 1 read_text (allowlist) - gateway/status.py: 1 read_text (PID file) - hermes_cli/banner.py: 1 read_text + 1 write_text (update cache) All sites read/write JSON or short text. No behavioral change on Linux (already UTF-8); fixes silent data corruption on Windows.
_read_hub_installed_names() reads ~/.hermes/skills/.hub/lock.json with a strict utf-8 decode. Hub skill descriptions can carry Windows-1252 typographic bytes (em-dash 0x97, smart quotes, bullets) as single high bytes; read_text(encoding="utf-8") then raises UnicodeDecodeError, which is a ValueError sibling not caught by the function's except (OSError, json.JSONDecodeError). It escapes and 500s the whole /api/skills endpoint, blanking the desktop Skills panel. Decode with errors="replace" so the offending byte degrades to U+FFFD and the structurally valid JSON — and every other skill — stays readable. Fixes #68053
…etup _setup_worktree read both files with the locale default encoding. On a cp1251/GBK Windows machine a UTF-8 include list either decodes to mojibake paths (non-ASCII entries silently not copied) or raises UnicodeDecodeError, which the enclosing handler logs at DEBUG and swallows — no include is copied at all, so the worktree starts without .env/keys and the agent breaks invisibly. A Notepad BOM likewise glues to the first include entry on every platform, and to the first .gitignore line, defeating the '.worktrees/' membership check and appending a duplicate entry on each run. Read both files with utf-8-sig + errors=replace, matching the canonical .env readers in hermes_cli/config.py (utf-8-sig because Notepad adds a BOM) and the UTF-8 append this same block already performs on .gitignore. Regression tests exercise the real cli._setup_worktree: the two BOM tests fail without the fix on any platform, the non-ASCII include test additionally reproduces the Windows locale failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ce skill scripts The bundled office skills (#68595) read user documents and agent-authored payloads with the locale-default codec: - docx/powerpoint validators/base.py opened OOXML part XML in text mode before handing it to lxml. On Windows (cp1251/GBK) the bytes decode to mojibake that lxml then parses, so validation runs against silently corrupted document text; on locales where the UTF-8 bytes don't decode the validator crashes with UnicodeDecodeError instead of validating. Opening as bytes lets lxml honor the encoding declared in the XML prolog. - The pdf form scripts (fill_fillable_fields, fill_pdf_form_with_annotations, create_validation_image, check_bounding_boxes) read the fields JSON the agent authors — UTF-8 by construction — with the locale codec, so non-ASCII form values (any Cyrillic/CJK/accented input) either crash or get written into the user's PDF as mojibake. The json.dump writers use ensure_ascii=True and were already safe; only the readers needed pinning. Adds a contract test asserting every document/payload reader is locale-independent, plus a live regression test that runs check_bounding_boxes.py on a non-ASCII fields.json under a forced non-UTF-8 locale — it fails without the fix on both POSIX (C locale) and Windows (cp1251 chokes on the 0x98 byte of U+2018).
…r rule AST-driven pass over every Path.read_text()/write_text() without an explicit encoding= across non-test code: 71 sites in 34 files (skills_hub, hermes_cli/main+profiles+service_manager+container_boot, mem0/hindsight/honcho plugins, achievements dashboard, release/CI scripts, productivity+comfyui skill helpers, agent/*). Verified zero positional-encoding collisions before insertion; per-file compile() check after. Adds a check-windows-footguns rule flagging bare single-line read_text/write_text (multi-line forms stay covered by the AST guard test from #38985). Together with the salvaged contributor commits this retires the ~169-site bare file-I/O class (#37423's long tail).
alt-glitch
added
type/bug
Contributor
૮ >ﻌ< ა ci reviewran on 9821f66 all good! |
This was referenced Jul 25, 2026
Closed
fix(gateway): pass encoding='utf-8' to all read_text/write_text in update flow (fixes #37423)
#37647
Closed
Closed
5 tasks
This was referenced Jul 26, 2026
This was referenced Aug 3, 2026
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
11 participants
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
Closes out the bare
read_text()/write_text()class — the file-I/O sibling of the subprocess decode class retired in #70875. All 139 remaining bare sites across non-test code now pass an explicitencoding=(utf-8, or BOM-tolerant utf-8-sig at.envreaders), and a newcheck-windows-footguns.pyrule fails CI on any future bare single-line call.On non-UTF-8 Windows (GBK/cp1252), bare
Path.read_text()decodes UTF-8 config/state/skills with the locale code page —UnicodeDecodeError(aValueError, sailing past theexcept OSErrorguards these sites sit in) or silent mojibake on write. This is the class behind #37423's long tail and roughly ten open contributor PRs.Salvages 11 contributor PRs (15 commits, chronological order, authorship preserved):
On top:
compile()after.envreaders the sweep normalized to plain utf-8, preserving the salvaged PRs' BOM contract (tests pin it)read_text/write_textnow failscheck-windows-footguns.py; multi-line forms stay covered by the AST guard test from fix(gateway): pass encoding="utf-8" to read_text/write_text in update path (#37423) #38985Already redundant on main, to be closed with credit separately: #69728/#69789 (auth.json — sites guarded), #49516/#54044 (memory_tool — main's
_read_raw_checkednow catchesUnicodeDecodeErrorwith the abort-don't-wipe contract).Validation
check-windows-footguns.py --allFixes the remaining scope of #37423. Closes #50655, #54241, #56385, #66856, #65440, #18782, #8469, #8898, #54866, #56725, #62123, #62617, #68798, #70289, #68062 (salvaged with credit).
Infographic