Skip to content

fix(windows): set UTF-8 encoding for Copilot ACP subprocess - #45144

Closed
MrRealORG wants to merge 1 commit into
NousResearch:mainfrom
MrRealORG:fix/45140-windows-unicode
Closed

fix(windows): set UTF-8 encoding for Copilot ACP subprocess#45144
MrRealORG wants to merge 1 commit into
NousResearch:mainfrom
MrRealORG:fix/45140-windows-unicode

Conversation

@MrRealORG

Copy link
Copy Markdown

Summary

Fixes #45140 - UnicodeDecodeError on Windows when Copilot CLI emits UTF-8 characters.

Problem:
On Windows, Hermes crashes when the Copilot ACP subprocess emits UTF-8 characters (e.g. emojis). The _stdout_reader thread errors with a UnicodeDecodeError because subprocess.Popen is called with text=True but without encoding, so Python uses the system codepage (cp1252) which cannot decode certain UTF-8 bytes.

Fix:
Added encoding='utf-8' and errors='replace' to the subprocess.Popen call in _run_prompt method (line ~440).

Testing:

  • Syntax validation passed
  • The subprocess will now properly decode UTF-8 output from Copilot CLI on Windows
  • Invalid bytes are replaced instead of crashing

How to Test:

  1. On Windows, configure Hermes with Copilot provider
  2. Trigger image analysis or any flow that prints emoji/non-latin characters
  3. Previously: Hermes would crash with UnicodeDecodeError
  4. Now: Characters display correctly, no crash

Closes #45140

…arch#45140)

UnicodeDecodeError on Windows when Copilot CLI emits UTF-8 characters like emojis.
Added encoding='utf-8' and errors='replace' to subprocess.Popen call.

Fixes NousResearch#45140
@liuhao1024

Copy link
Copy Markdown
Contributor

This change is a subset of #45099 which adds the same encoding="utf-8", errors="replace" to copilot_acp_client.py along with 7 other subprocess call sites across the codebase.

If #45099 is merged first, this PR becomes redundant. Consider closing this one in favor of #45099 which has broader coverage.

@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/acp Agent Communication Protocol adapter P2 Medium — degraded but workaround exists labels Jun 12, 2026
@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 labels Jul 14, 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

comp/acp Agent Communication Protocol adapter comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists 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.

[Bug]: UnicodeDecodeError in copilot_acp_client.py _stdout_reader on Windows

4 participants