Skip to content

fix(computer_use): pin subprocess text-mode encoding to utf-8 - #63795

Closed
yuyzhen wants to merge 1 commit into
NousResearch:mainfrom
yuyzhen:fix/subprocess-utf8-encoding
Closed

fix(computer_use): pin subprocess text-mode encoding to utf-8#63795
yuyzhen wants to merge 1 commit into
NousResearch:mainfrom
yuyzhen:fix/subprocess-utf8-encoding

Conversation

@yuyzhen

@yuyzhen yuyzhen commented Jul 13, 2026

Copy link
Copy Markdown

Problem

On non-UTF-8 locales (e.g. Chinese Windows where the ANSI codepage is GBK/CP936), subprocess.run(..., text=True) without an explicit encoding= defaults to the system locale for decoding the child's stdout/stderr. cua-driver emits UTF-8 (emoji, Chinese paths, non-ASCII status), so the subprocess module's internal _readerthread blows up with a cascade like:

Exception in thread Thread-N (_readerthread):
Traceback (most recent call last):
  File ".../threading.py", line 1045, in _bootstrap_inner
    self.run()
  File ".../threading.py", line 982, in run
    self._target(*self._args, **self._kwargs)
  File ".../subprocess.py", line 1599, in _readerthread
    buffer.append(fh.read())
                  ^^^^^^^^^
UnicodeDecodeError: 'gbk' codec can't decode byte 0x94 in position 1711: illegal multibyte sequence

The main return path still works (partial stdout is captured up to the failure point), but the tracebacks spam stderr on every computer_use capture, mask real errors, and unsettle users on zh-CN Windows.

Fix

Pin encoding='utf-8', errors='replace' on every text-mode subprocess call in the computer_use tool:

  • tools/computer_use/cua_backend.py
    • _resolve_mcp_invocation (manifest discovery)
    • cua_driver_update_check (check-update --json)
    • _CuaDriverSession CLI fallback spawn
  • tools/computer_use/permissions.py — the _run helper

tools/computer_use/doctor.py already had encoding='utf-8' on its Popen; this PR brings the rest in line.

Verification

Before: on zh-CN Windows, computer_use(action='capture', app='screen', mode='vision') returns a valid result but stderr fills with 3+ UnicodeDecodeError tracebacks.

After (same host, same command): the capture returns cleanly, no background thread exceptions.

Notes

  • errors='replace' (not strict) is deliberate: cua-driver's output is authoritative UTF-8, but on the unlikely chance of a mixed-encoding byte we prefer a replacement char over crashing a reader thread.
  • No behaviour change on POSIX / UTF-8 locales — encoding='utf-8' matches the default there.

On non-UTF-8 locales (e.g. Chinese Windows with GBK ANSI codepage),
subprocess.run(..., text=True) defaults to the system locale for
decoding the child's stdout/stderr. cua-driver emits UTF-8 (emoji,
Chinese paths, etc.), so the internal _readerthread crashes with a
cascade of UnicodeDecodeError exceptions like:

  Exception in thread Thread-N (_readerthread):
    ...
    File ".../subprocess.py", line 1599, in _readerthread
      buffer.append(fh.read())
  UnicodeDecodeError: 'gbk' codec can't decode byte 0x94 ...

The main return path still works (proc.stdout is populated up to the
failure point), but the tracebacks spam stderr on every capture and
mask real errors.

Pin encoding='utf-8', errors='replace' on every text-mode subprocess
call in the computer_use tool. doctor.py already had this fix.
@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows P2 Medium — degraded but workaround exists labels Jul 13, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #60097 (otsune) fixes the same three cua_backend.py subprocess sites for the same non-UTF-8 decode crash; this PR covers those plus the additional permissions.py::_run site (incremental superset). Not a duplicate — the maintainer can pick the broader fix. Also part of the wider strict-UTF-8 subprocess family (#52249/#52969/#52700).

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Windows-locale fix. The premise remains present on current main: tools/computer_use/cua_backend.py:166, :257, and :923, plus tools/computer_use/permissions.py:74, use captured text=True subprocess output without explicit decoding. The change aligns them with the documented existing cua-driver policy in tools/computer_use/doctor.py:93-105.

Problems

  • No regression coverage is included. The existing subprocess spies in tests/computer_use/test_cua_spawn_env_sanitization.py:30-35 and tests/computer_use/test_cua_cli_fallback_env.py:39-41 retain env but not encoding or errors, so this exact guarantee is currently untested.

Suggested changes

  • Extend those runtime spies to capture kwargs and assert encoding="utf-8" and errors="replace" for the manifest, update-check, permission-probe, and CLI-fallback subprocess paths.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 16, 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/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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.

3 participants