Skip to content

fix(windows): decode wmic output with explicit encoding to survive non-UTF-8 locales (#17049)#17435

Merged
teknium1 merged 4 commits into
mainfrom
hermes/hermes-21e60ee8
Apr 29, 2026
Merged

fix(windows): decode wmic output with explicit encoding to survive non-UTF-8 locales (#17049)#17435
teknium1 merged 4 commits into
mainfrom
hermes/hermes-21e60ee8

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

hermes setup and hermes update both scan the Windows process table via wmic process get ProcessId,CommandLine /FORMAT:LIST with text=True and no explicit encoding. On systems whose default code page is not UTF-8 (e.g. cp936 on zh-CN), the subprocess reader thread crashes with UnicodeDecodeError, leaves result.stdout = None, and the subsequent .split("\\n") aborts both commands with AttributeError: 'NoneType' object has no attribute 'split' — exactly the trace in #17049.

This PR closes the bug at both call sites by passing encoding="utf-8", errors="ignore" and guarding against stdout is None. The parser only matches the ASCII prefixes CommandLine= and ProcessId=, so dropping undecodable bytes is safe.

Changes

  • hermes_cli/gateway.py_scan_gateway_pids() (setup wizard / service detection path).
  • hermes_cli/main.py_warn_stale_dashboard_processes() (hermes update path).
  • tests/hermes_cli/test_update_stale_dashboard.py — regression tests asserting the encoding kwargs reach subprocess.run and that a None stdout short-circuits instead of crashing.
  • scripts/release.pyAUTHOR_MAP entry for @Kailigithub.

Credit

Salvaged from two complementary contributor PRs, both preserved via rebase-merge:

(#17271 and #17090 attempted the same fix but bundled unrelated changes; closing those in favor of this PR.)

Validation

  • tests/hermes_cli/test_update_stale_dashboard.py — 12/12 pass (10 existing + 2 new).
  • tests/hermes_cli/test_gateway.py + test_gateway_service.py + test_update_gateway_restart.py — 169/169 pass.

Closes #17049.

briandevans and others added 4 commits April 29, 2026 05:07
…n Windows non-UTF-8 locales (#17049)

`hermes update` calls `_warn_stale_dashboard_processes()` to warn about
dashboard processes still running the pre-update Python backend. On
Windows, that scan shells out to `wmic process get ProcessId,CommandLine
/FORMAT:LIST` with `text=True` and no explicit encoding.

`wmic` emits text in the system code page (e.g. cp936 on zh-CN locales),
not UTF-8. Without an explicit `encoding=`, Python's default UTF-8
decoder crashes the subprocess reader thread with
`UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd0 ...`. In
Python 3.11 that crash is silently absorbed: `subprocess.run()` returns
a `CompletedProcess` with `result.stdout = None`, the next line calls
`result.stdout.split("\n")`, and `hermes update` aborts with the
exact `AttributeError: 'NoneType' object has no attribute 'split'`
trace reported in #17049.

Fix: pass `encoding="utf-8", errors="ignore"` so undecodable bytes
cannot take down the reader thread (the parsing only matches the ASCII
prefixes `CommandLine=` and `ProcessId=`, so dropping non-UTF-8 bytes
is safe), and short-circuit when `result.stdout is None` as a defensive
guard for environments where the reader thread still fails for other
reasons.

This is the same root cause as #17074 (which patches
`hermes_cli/gateway._scan_gateway_pids` for the `hermes setup` path).
That PR does not touch `_warn_stale_dashboard_processes`, so
`hermes update` remains broken on the same locales until this lands.

Regression test in `tests/hermes_cli/test_update_stale_dashboard.py`:
- `test_wmic_invoked_with_utf8_ignore_errors` asserts the explicit
  encoding/errors kwargs reach `subprocess.run`.
- `test_wmic_returns_none_stdout_does_not_crash` simulates the
  reader-thread-crashed `result.stdout=None` aftermath and asserts the
  function returns silently instead of raising AttributeError.

Both new tests fail against clean origin/main (7d46484) reproducing
the original AttributeError; both pass with this patch. The remaining
3 failures in `tests/hermes_cli/test_cmd_update.py` and
`test_update_autostash.py` are pre-existing baselines on origin/main —
they reproduce identically without this change and are unrelated to
the wmic scan.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ys.platform

Two fix-ups for #17123:

1. Reword the inline comment in `_warn_stale_dashboard_processes` to
   accurately describe the failure mode (locale-dependent decoder, not a
   "default UTF-8 decoder") and identify `errors="ignore"` as the
   load-bearing protection. Per Copilot's review.

2. Switch `TestWindowsWmicEncoding` from `patch("hermes_cli.main.sys")`
   to `monkeypatch.setattr(sys, "platform", "win32")` — the codebase's
   canonical pattern (e.g. `tests/hermes_cli/test_auth_ssl_macos.py`).
   The MagicMock-replacement approach passed locally on Python 3.12 but
   the platform-equality check failed under CI's xdist+Python 3.11,
   leaving both new tests red despite the fix being present.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pass encoding='utf-8', errors='ignore' and guard against result.stdout
being None so _scan_gateway_pids() no longer crashes with
UnicodeDecodeError + AttributeError on Windows systems whose default
code page is not UTF-8 (e.g. cp936 on zh-CN). The parser only matches
the ASCII prefixes CommandLine= and ProcessId=, so dropping undecodable
bytes is safe.

Closes #17049.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard labels Apr 29, 2026
@teknium1 teknium1 merged commit 5662ac2 into main Apr 29, 2026
11 of 12 checks passed
@teknium1 teknium1 deleted the hermes/hermes-21e60ee8 branch April 29, 2026 13:34
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 P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: UnicodeDecodeError and AttributeError in Windows gateway process scanning

4 participants