Skip to content

fix(core,cli,gateway,plugins): add encoding='utf-8' to read_text() calls - #54241

Closed
AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/read-text-utf8-encoding-core
Closed

fix(core,cli,gateway,plugins): add encoding='utf-8' to read_text() calls#54241
AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/read-text-utf8-encoding-core

Conversation

@AlexFucuson9

Copy link
Copy Markdown
Contributor

Problem

Path.read_text() without an explicit encoding parameter 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.

This is the read-side companion to PR #54240 (write_text encoding fix). Files written with encoding="utf-8" must also be read with encoding="utf-8" to be cross-platform safe.

Scope

31 read_text() call sites across 14 files, covering:

Module Files Context
tools/skills_hub.py 3 Skill cache, index cache (JSON with ensure_ascii=False)
hermes_cli/auth.py 3 Auth credentials JSON
hermes_cli/profiles.py 5 Profile name, PID, wrapper scripts
hermes_cli/uninstall.py 2 Config path, wrapper script
hermes_cli/main.py 2 Auth JSON, agent response
hermes_cli/doctor.py 2 Lock file JSON, wrapper script
gateway/run.py 6 Voice mode, counts, prompt, pending JSON
agent/auxiliary_client.py 1 Auth JSON
agent/copilot_acp_client.py 1 Content file
agent/shell_hooks.py 1 Allowlist JSON
plugins/memory/mem0/__init__.py 1 Config JSON
plugins/memory/hindsight/__init__.py 1 Config JSON
plugins/memory/honcho/__init__.py 1 Config JSON
plugins/platforms/google_chat/oauth.py 2 OAuth token JSON

Fix

# Before
data = json.loads(path.read_text())

# After
data = json.loads(path.read_text(encoding="utf-8"))

Impact

  • Severity: P1 — data corruption / UnicodeDecodeError on Windows
  • Scope: 14 files, 31 lines changed
  • Risk: Minimal — explicit encoding matches what Linux/macOS already default to

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'))
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows P3 Low — cosmetic, nice to have labels Jun 28, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Mechanical fix adding explicit encoding='utf-8' to read_text() calls across core modules (7+ files). This is a correct cross-platform fix -- on Windows, read_text() without explicit encoding uses the system default (often cp1252), which can corrupt non-ASCII content.

Strengths:

  • Consistent application across all identified call sites
  • No behavioral change on Unix (UTF-8 is already the default)
  • Prevents UnicodeDecodeError on Windows with non-ASCII config/session files
  • Well-scoped to read_text() calls that parse JSON

No concerns.


Reviewed by Hermes Agent

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: LGTM

Mechanical change adding encoding='utf-8' to read_text() calls across 14 files. All changes are identical pattern: .read_text() -> .read_text(encoding='utf-8'). This ensures consistent encoding behavior across platforms (Windows in particular). No logic changes.

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Windows-compatibility cleanup. The underlying issue is still present on current main, but the current patch is incomplete against HEAD.

Problems

  • gateway/run.py:3227 still reads persisted gateway_voice_mode.json with bare read_text(). Additional persisted JSON/update-state reads remain at gateway/run.py:6092, :6122, :12587, :14508, :14646, :14740, and :14835.
  • tools/skills_hub.py:3366 writes cache JSON with ensure_ascii=False, while current readers at tools/skills_hub.py:1137, :3346, :3400, :3474, :3794, and :3860 still rely on the platform default encoding.
  • The diff has no test changes. A regression should cover reading non-ASCII UTF-8 persisted JSON through an affected loader.

Suggested changes

  • Refresh the audit against current main and cover the current Hermes-owned UTF-8 state paths, distinguishing them from externally authored files with unknown encodings.
  • Add a focused non-ASCII UTF-8 read regression test.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #71078 — your commit was cherry-picked onto current main with your authorship preserved in git log (rebase merge). Your 5-PR series (#50655 was authored under your earlier account, #54241, #56385, #66856, #65440) formed the backbone of the class-wide close-out: 68 of the 139 bare sites came from your commits, and the campaign's structure followed your directory-by-directory split. The remaining 71 sites were swept on top and a CI linter rule now prevents regressions. Thanks for the sustained, methodical work on this class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

4 participants