Skip to content

fix(windows): patch platform._syscmd_ver to survive non-UTF-8 output under PEP 540 - #69522

Closed
webtecnica wants to merge 2 commits into
NousResearch:mainfrom
webtecnica:fix/69413-windows-utf8
Closed

fix(windows): patch platform._syscmd_ver to survive non-UTF-8 output under PEP 540#69522
webtecnica wants to merge 2 commits into
NousResearch:mainfrom
webtecnica:fix/69413-windows-utf8

Conversation

@webtecnica

Copy link
Copy Markdown
Contributor

Fixes #69413

Description

On Windows, the OpenAI SDK generates platform headers by calling platform.platform(). This triggers:

platform.platform()
→ platform.win32_ver()
→ platform._syscmd_ver()
→ subprocess.check_output("ver", text=True, encoding="locale", shell=True)
→ subprocess.Popen._readerthread

When PYTHONUTF8=1 (PEP 540 UTF-8 mode) is active — which Hermes sets on Windows — locale.getencoding() returns "utf-8". However, the Windows ver command outputs text in the system's active ANSI code page (e.g. cp1252 on US-English). Bytes that are valid in cp1252 but not in UTF-8 — such as 0xe9 (é in cp1252, an invalid UTF-8 start byte) — raise UnicodeDecodeError inside the subprocess reader thread.

Fix

Added _patch_platform_syscmd_ver() to hermes_bootstrap.py that wraps platform._syscmd_ver with a try/except UnicodeDecodeError on Windows. When the ver command output can't be decoded as UTF-8, the patched function gracefully returns its input defaults.

Applied automatically on module import (every Hermes entry point already imports hermes_bootstrap first). No-op on POSIX.

Changes

File Change
hermes_bootstrap.py Added _patch_platform_syscmd_ver() function + module-level call
tests/test_hermes_bootstrap.py Added TestPatchPlatformSyscmdVer class (5 tests)

Test results

All 23 tests pass, 5 Windows-only skipped on Linux.

Closes #69413

…large contexts (NousResearch#69424)

Three-pronged fix for the stale-stream detector killing connections
before a slow local/cloud model finishes prompt prefill:

1. Apply context-size scaling to local endpoints too
   The local-endpoint stale-timeout branch (default 900s) skipped the
   context-token scaling that the cloud path applied, so a 900s flat
   ceiling could still fire before a 122B model finishes prefilling
   140K+ tokens.  Move scaling out of the  branch so both local
   and cloud paths get proportional timeouts:
   - >200K tokens → 1800s (30 min)
   - >100K tokens → 1200s (20 min)
   - >50K  tokens → 600s  (10 min)

2. Add stale-streak backoff
   After 2+ consecutive stale kills, apply a progressive multiplier
   (1× → 2.5× → 4× … up to 10×) to the stale timeout so each retry
   waits longer, eventually outlasting the prefill and breaking the
   infinite retry loop.  Resets on successful response.

3. Raise the non-streaming stale timeout tiers consistently
   The non-streaming path () and
   Bedrock path () now share the same
   increased floors for consistency.

Closes NousResearch#69424.
…under PEP 540

On Windows with PYTHONUTF8=1 (PEP 540 UTF-8 mode),
platform._syscmd_ver() calls subprocess.check_output(...,
text=True, encoding='locale', shell=True) to run the Windows
'ver' command.  Under PEP 540, locale.getencoding() returns
'utf-8', but the 'ver' command emits output in the system's
active ANSI code page (cp1252 on US-English).  Bytes that are
valid in cp1252 but not in UTF-8 — e.g. 0xe9 (é) — raise a
UnicodeDecodeError inside the subprocess reader thread.

The patch wraps platform._syscmd_ver with a try/except that
catches UnicodeDecodeError and gracefully returns input defaults,
matching the existing OSError/CalledProcessError fallback that
_win32_ver already handles.

Fixes NousResearch#69413
@isak-ialogics

Copy link
Copy Markdown
Contributor

Triage evidence only (not a merge decision): this PR currently contains commit 47bad20, which is the complete head commit of the separate #69521. As a result, the diff here includes unrelated changes to agent/chat_completion_helpers.py, run_agent.py, and tests/agent/test_non_stream_stale_timeout.py in addition to the Windows fix described by this PR. Concrete next action: rebase this branch onto current main and retain only fde3e142 (or otherwise drop 47bad20), then rerun CI so #69522 is scoped to hermes_bootstrap.py and its regression tests.

@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 platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 22, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing after a deep verification pass — the diagnosis deserves a detailed response because the issue you chased is partially real.

What's correct: on Python 3.11.0/3.11.1, platform._syscmd_ver lacks CPython's encoding="locale" fix (added in 3.11.2), so under PEP 540 UTF-8 mode the cmd /c ver output is strict-utf-8 decoded and raises on OEM-code-page bytes. That crash path was live in processes that don't run hermes_cli.main (slash workers, tui_gateway entry, run_agent).

Why this PR couldn't merge as-is: (1) the wrapper's supported_platforms=None default is forwarded into the original, which does sys.platform not in supported_platforms → guaranteed TypeError on every no-arg call — platform.win32_ver() invokes it exactly that way, so on Windows the patch breaks platform.platform() outright (the tests mock past this); (2) for Python ≥3.11.2, locale.getencoding() explicitly ignores UTF-8 mode, so the PR's core claim doesn't hold there; (3) the branch bundles an unrelated commit (#69521's stream-timeout changes).

What shipped instead (PR #71014): the existing full-stub suppress_platform_ver_console() now runs from hermes_bootstrap, covering every entry point — win32_ver() falls back to sys.getwindowsversion(), fixing both the decode crash on the old micros and the console flash, with a regression test pinning the no-arg-call contract. Thanks for surfacing the coverage gap — the bootstrap move exists because your PR pointed at the right set of exposed processes.

@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/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage 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.

Windows subprocess reader thread crashes on non-UTF-8 output

4 participants