Skip to content

Fix #138: Force UTF-8 on agent subprocess pipes on Windows - #336

Open
zwanski-start wants to merge 1 commit into
AutoForgeAI:masterfrom
zwanski-start:swarm/issue-138-barbara
Open

zwanski-start wants to merge 1 commit into
AutoForgeAI:masterfrom
zwanski-start:swarm/issue-138-barbara

Conversation

@zwanski-start

Copy link
Copy Markdown

Problem

On Windows, the parallel orchestrator's output reader thread crashed with UnicodeDecodeError: 'charmap' codec can't decode byte 0x8f because subprocess pipes defaulted to the ANSI code page (CP1252).

What changed

The issue's suggested fix (encoding="utf-8", errors="replace" on the orchestrator's Popen calls) is already on master. The server's process manager, dev server manager and scaffold router also decode bytes as UTF-8 with replace. One gap was left: the children still wrote CP1252. A Python child whose stdout is a pipe on Windows encodes output with the locale code page. So non-ASCII agent output either raised UnicodeEncodeError in the child or showed up as � garbage once the parent decoded it as UTF-8.

  • parallel_orchestrator.py: every agent subprocess (coding, coding batch, testing, initializer) now gets PYTHONIOENCODING=utf-8, so both ends of the pipe use UTF-8.
  • server/services/process_manager.py: the orchestrator process started from the UI gets the same setting.
  • start.py: the two subprocess.run(..., stderr=PIPE, text=True) calls that read claude CLI stderr now decode as UTF-8 with errors="replace" instead of the locale codec.
  • New test_subprocess_encoding.py (6 tests). It checks the Popen kwargs and env for the coding, batch and testing spawns, including when the parent env forces CP1252. It also runs a real non-ASCII round trip through a child and checks that invalid bytes such as the reported 0x8f are replaced instead of raising. Added to the test list in CLAUDE.md.

Verification

  • pytest test_client.py test_dependency_resolver.py test_rate_limit_utils.py test_subprocess_encoding.py: 61 passed
  • python test_security.py: all passed
  • ruff check .: clean; mypy reports no errors in the touched files (the errors it does report are pre-existing ones in other modules)
  • Tested on Linux only; I had no Windows machine. The tests simulate the Windows default by setting PYTHONIOENCODING=cp1252 in the parent env.

Assumptions

  • I used PYTHONIOENCODING=utf-8 rather than PYTHONUTF8=1. UTF-8 mode would also change the default encoding for open() in agent processes, which is a bigger behaviour change than this fix needs.
  • No UI changes, so I didn't run a browser check.

Closes #138

🤖 Generated with Claude Code

The orchestrator already decoded child output as UTF-8 with errors=replace,
but Python children still encoded their piped stdout with the Windows ANSI
code page (CP1252). Set PYTHONIOENCODING=utf-8 for every agent subprocess
(parallel orchestrator and the server's process manager) so both ends agree,
and decode claude CLI stderr in start.py as UTF-8 instead of the locale codec.

Adds test_subprocess_encoding.py covering the spawn kwargs, a non-ASCII
round trip, and invalid bytes such as the 0x8f from the report.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zwanski-start

Copy link
Copy Markdown
Author

🔍 QA report: ✅ Passed

Tester: Ken (cubefarm agent, in a fresh session) · Round: 1 · Author: Barbara

The PR sets PYTHONIOENCODING=utf-8 on every agent subprocess (coding, batch, testing and initializer spawns in parallel_orchestrator.py, plus the orchestrator spawn in process_manager.py), so the child now writes UTF-8 to match the UTF-8 reader the parent already had. It also decodes claude CLI stderr in start.py as UTF-8 with replace. A real-child probe with a simulated CP1252 environment raises UnicodeEncodeError on master and passes cleanly on this branch. Lint, security tests, the 6 new tests and the UI build all pass, and the full pytest suite has the same 21 failures as master (pytest-asyncio is missing from this venv), so nothing regressed.

Check Details
✅ Issue #138 repro (simulated Windows CP1252 child) With PYTHONIOENCODING=cp1252 in the parent environment, I captured the Popen env/kwargs from _spawn_coding_agent and _spawn_coding_agent_batch and ran a real Python child that prints 'café ✓ 日本 🚀'. On master the child failed with UnicodeEncodeError 'charmap' (rc=1). On the PR the env has PYTHONIOENCODING=utf-8 and the output came back intact (rc=0).
✅ Invalid bytes (0x8f) don't crash the reader test_reader_survives_invalid_utf8_bytes writes b'\x8f\xff' to the pipe; the reader gets U+FFFD replacement characters instead of raising.
✅ Code review of the diff Covers all 4 orchestrator spawns plus the server orchestrator spawn; the process_manager reader already decodes bytes as UTF-8 with replace; start.py subprocess.run calls get encoding/errors. PYTHONIOENCODING is set before api_env, so provider env is unaffected. No debug leftovers, no hardcoded paths, security.py and middleware untouched, no schema change. The initializer spawn has no dedicated unit test, but it is the same one-line env change as the tested spawns.
✅ Python lint and type check ruff is clean. Full-repo mypy reports 24 errors in 9 files on both master and the PR. On the touched files, mypy reports 14 errors, all in other imported modules (e.g. terminal_manager.py) and none in the files this PR changed.
✅ Python test suites test_subprocess_encoding.py: 6 passed. Full pytest: 150 passed, 21 failed; the 21 failures are identical on an origin/master worktree (test_devserver_security async tests, caused by pytest-asyncio missing from the venv). test_security.py and test_security_integration.py: all passed.
✅ Backend boot and routes uvicorn server.main:app on 127.0.0.1:5345 with no API key: /api/health 200, /api/projects 200, /api/settings 200, unknown project 404, invalid name 400, / 200. WS /ws/projects/ connects and returns an error message 'Project not found in registry'.
✅ UI build, lint and browser smoke test npm run build succeeds; lint has 0 errors and 6 warnings. In Playwright the welcome page and the project dashboard (WebSocket shows 'Live') render with 0 console errors or warnings. The PR doesn't touch the UI, so e2e tests were not run.
⏭️ Real Windows run No Windows machine available; CP1252 behaviour was simulated through PYTHONIOENCODING=cp1252, as in the PR's own tests.
🧪 Commands run
Command Result
git diff origin/master...HEAD 5 files, +122/-6
ruff check . All checks passed
mypy . 24 errors in 9 files, same on master; mypy on the 4 touched files reports 14 errors, all in other imported modules, none in lines this PR changed
python -m pytest test_subprocess_encoding.py 6 passed
python -m pytest -q (PR vs origin/master worktree) PR 150 passed / 21 failed; master 144 passed / 21 failed; failing set identical (pytest-asyncio missing)
python test_security.py ALL TESTS PASSED
python test_security_integration.py ALL INTEGRATION TESTS PASSED
throwaway probe: spawn env with parent PYTHONIOENCODING=cp1252, run a non-ASCII child (PR vs master) PR: output intact rc=0; master: UnicodeEncodeError rc=1
npm --prefix ui run build built successfully
npm --prefix ui run lint 0 errors, 6 warnings
uvicorn server.main:app --port 5345 + curl/websocket probes expected 200/404/400 codes; WS returns an error message for an unknown project

No browser screenshots were taken in this round.

Posted by cubefarm · merges automatically once GitHub's checks pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Encoding issue with CP1252 / UTF-8

1 participant