Skip to content

fix: add encoding=utf-8, errors=replace to all subprocess text=True calls (#53428) - #53460

Closed
Sahil-SS9 wants to merge 2 commits into
NousResearch:mainfrom
Sahil-SS9:fix/issue-53428-subprocess-encoding-utf8
Closed

fix: add encoding=utf-8, errors=replace to all subprocess text=True calls (#53428)#53460
Sahil-SS9 wants to merge 2 commits into
NousResearch:mainfrom
Sahil-SS9:fix/issue-53428-subprocess-encoding-utf8

Conversation

@Sahil-SS9

Copy link
Copy Markdown
Contributor

Fixes #53428

Description

On Chinese Windows (CP936/GBK default encoding), any subprocess.run/Popen with text=True but no explicit encoding/errors params crashes _readerthread when child outputs UTF-8 Chinese text, triggering a cascading failure chain that eventually freezes the TUI and restarts the gateway.

This adds encoding="utf-8", errors="replace" to all 20 locations across 12 files where subprocess.run/Popen uses text=True without an explicit encoding parameter.

Files changed (12 files, 20 locations)

  • tui_gateway/server.py (5) — ACP child process, pdftoppm, hermes subprocess, quick commands, shell exec
  • tui_gateway/git_probe.py (1) — git probing
  • gateway/run.py (1) — systemctl show
  • agent/coding_context.py (1) — git helper
  • agent/anthropic_adapter.py (1) — security credential lookup
  • agent/context_references.py (2) — git and ripgrep helpers
  • agent/lsp/install.py (3) — npm, go, pip installers
  • agent/secret_sources/bitwarden.py (2) — ldd probe, bws run
  • agent/copilot_acp_client.py (1) — ACP subprocess
  • agent/shell_hooks.py (1) — shell hook execution
  • agent/skill_preprocessing.py (1) — inline shell execution
  • agent/transports/codex_app_server.py (1) — codex version check

Verification

python3 /tmp/find_missing_encoding.py # 0 results — all locations fixed

  • All 12 changed files compile cleanly
  • No secrets, personal refs, or unrelated changes
  • Conventional commit format

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/gateway Gateway runner, session dispatch, delivery comp/lsp Language Server Protocol integration (P2 policy) platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jun 27, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: this is the broadest entry in the Windows non-UTF-8 (CP936/GBK/cp950) subprocess-encoding rollout — 20 sites across 12 files. It overlaps with focused open PRs on individual sites: #52700 (tui_gateway/server.py), #49564 (hermes_cli/doctor.py), #43494 (gateway.py/gateway_windows.py), #25200 (windows_subprocess_kwargs() helper), #49510. Not a duplicate of any single one (superset scope; only tui_gateway/server.py overlaps #52700). Consider whether a shared windows_subprocess_kwargs() helper (per #25200) is preferable to repeating encoding='utf-8', errors='replace' at 20 call sites, so future subprocess additions inherit the fix. Maintainer to pick the consolidation strategy.

@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

Adds encoding="utf-8", errors="replace" to all 20 subprocess.run/Popen calls with text=True across 12 files. Fixes Chinese Windows (CP936/GBK) encoding crashes that cascade into TUI freezes and gateway restarts.

  • 20 additions, 20 deletions — pure mechanical change
  • 12 files touched but all are the same type of change
  • Under 15-file surface-area threshold
  • errors="replace" is the right choice for robustness (graceful degradation)
  • Fixes a real cross-platform bug (#53428)

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded by the consolidated Windows console-flash work tracked in #54220.

The relevant pieces from this PR/cluster have now landed through the targeted follow-up PRs #54236, #53892, and #54417, or are recorded in the umbrella tracker for any remaining native-Windows verification. Keeping this separate PR open would duplicate the tracker and the merged follow-up work.

Thanks for digging into this — the reports and PRs in this cluster helped identify the remaining spawn legs.

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/gateway Gateway runner, session dispatch, delivery comp/lsp Language Server Protocol integration (P2 policy) comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage 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.

subprocess.run(text=True) without encoding param triggers GBK crash on Chinese Windows (21 locations)

4 participants