Skip to content

fix(tui): decode startup subprocess output as utf-8 - #43790

Closed
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:fix/windows-tui-subprocess-decode
Closed

fix(tui): decode startup subprocess output as utf-8#43790
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:fix/windows-tui-subprocess-decode

Conversation

@helix4u

@helix4u helix4u commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a Windows TUI startup crash/noisy traceback path where captured npm subprocess output was decoded with the process default ANSI code page. On Windows, that can surface a UnicodeDecodeError from Python's internal subprocess._readerthread before the TUI finishes launching.

The TUI launcher now uses explicit UTF-8 decoding with replacement for the captured node/bootstrap and npm build/install calls. That keeps startup from printing a Python thread traceback when npm/node emits bytes outside the active Windows code page.

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/main.py: add encoding="utf-8", errors="replace" to TUI startup subprocess captures:
    • node bootstrap probe
    • TUI npm install
    • TUI dev prebuild
    • normal TUI npm run build
  • tests/hermes_cli/test_tui_npm_install.py: assert the TUI launcher npm captures use the explicit decode policy.

How to Test

  1. python -m py_compile hermes_cli/main.py tests/hermes_cli/test_tui_npm_install.py
  2. git diff --check -- hermes_cli/main.py tests/hermes_cli/test_tui_npm_install.py
  3. Direct .venv/bin/python launcher probe for normal build, install-plus-build, and dev prebuild passed.
  4. Manual Windows AppData install check: applying the same patch removed the UnicodeDecodeError: 'charmap' codec can't decode byte 0x8f startup traceback from hermes --tui.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11 AppData install, WSL syntax/probe checks

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

  • This skill is broadly useful to most users (if bundled) — see Contributing Guide — N/A
  • SKILL.md follows the standard format (frontmatter, trigger conditions, steps, pitfalls) — N/A
  • No external dependencies that aren't already available (prefer stdlib, curl, existing Hermes tools) — N/A
  • I've tested the skill end-to-end: hermes --toolsets skills -q "Use the X skill to do Y" — N/A

Screenshots / Logs

Reported Windows startup traceback:

UnicodeDecodeError: 'charmap' codec can't decode byte 0x8f ... subprocess.py ... _readerthread ... buffer.append(fh.read())

@helix4u
helix4u marked this pull request as ready for review June 10, 2026 22:00
@helix4u
helix4u marked this pull request as draft June 10, 2026 22:02
@helix4u
helix4u marked this pull request as ready for review June 10, 2026 22:03
@liuhao1024

Copy link
Copy Markdown
Contributor

Code Review Verification

Reviewed the diff (8 lines production across 4 call sites + 61 lines test). The fix is correct:

  • Problem: subprocess.run(text=True) without explicit encoding uses the system default encoding, which on Windows (and some Linux locales) is not UTF-8. Non-UTF-8 bytes in npm output cause UnicodeDecodeError.
  • Fix: Adds encoding="utf-8", errors="replace" to all 4 subprocess.run calls in _ensure_tui_node() and _node_bin(). errors="replace" ensures malformed bytes produce instead of crashing — safe for diagnostic output.
  • Scope: All 4 call sites in hermes_cli/main.py are covered (the _ensure_tui_node npm install, the ink build, the tsx dev prebuild, and the tui npm install).
  • Tests: _assert_utf8_replace_capture helper verifies all three kwargs at each call site. New test test_make_tui_argv_decodes_dev_prebuild_with_utf8_replace covers the dev-prebuild path.

No issues found. Clean defensive fix.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have labels Jun 10, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #44328 — your commit was cherry-picked onto current main with your authorship preserved (rebase-merge). We added one follow-up on top covering the same bug class in the WhatsApp bridge npm install capture. Thanks!

@teknium1 teknium1 closed this Jun 11, 2026
AIalliAI pushed a commit to AIalliAI/Hermes that referenced this pull request Jun 14, 2026
T02200059 pushed a commit to T02200059/hermes-agent that referenced this pull request Jun 18, 2026
xyshanren pushed a commit to xyshanren/hermes-agent-cn that referenced this pull request Jun 25, 2026
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
donbowman pushed a commit to donbowman/hermes-agent that referenced this pull request Jul 13, 2026
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants