From cee0d93b60f25b5d595cdea23163513dc4b08aff Mon Sep 17 00:00:00 2001 From: Jason Cat Date: Fri, 9 Oct 2026 17:25:35 +0800 Subject: [PATCH] fix(sandbox): decode Git Bash output as UTF-8 so CJK output is not garbled (#6548) * fix(sandbox): decode Git Bash output as UTF-8 so CJK tools are not garbled Git Bash/MSYS always writes UTF-8 to its pipes, but the MSYS branch of LocalSandbox.execute_command called _run_windows_command without an encoding, so the capture fell back to locale.getpreferredencoding(). On a non-UTF-8 host (GBK on zh-CN Windows) that decodes UTF-8 bytes with the host code page and mangles every non-ASCII tool result. The pipe decoder replaces instead of raising, so the corruption is silent. The PowerShell branch right next to it has pinned UTF-8 since #5440. Decode the MSYS branch as UTF-8 too, and pin the behaviour in tests/test_local_sandbox_encoding.py: a unit test for the decoder choice and a live round-trip through a real Git Bash (skipped when it is absent). * fix(sandbox): keep locale fallback for native-child output under Git Bash Git Bash/MSYS writes its own output as UTF-8, but Windows-native children (python.exe, CLI shims) inherit the pipe and keep writing the host code page. Forcing UTF-8 with errors=replace turned that output into U+FFFD. Decode the captured buffer as UTF-8 first and retry with the locale when it is not valid UTF-8, so both MSYS UTF-8 output and native-child code-page output survive. The PowerShell branch is unchanged (its own output is pinned to UTF-8 by the preamble); the fallback is only wired on the MSYS path. Adds native-Python CJK regressions through execute_command. --------- Co-authored-by: Willem Jiang --- .../deerflow/sandbox/local/local_sandbox.py | 45 +++++- backend/tests/test_local_sandbox_encoding.py | 129 +++++++++++++++++- 2 files changed, 168 insertions(+), 6 deletions(-) diff --git a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py index cbfa76eab1..7a754852c9 100644 --- a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py +++ b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py @@ -40,16 +40,34 @@ class _BoundedPipeCapture: *, limit_bytes: int = _COMMAND_CAPTURE_LIMIT_BYTES, encoding: str = "utf-8", + fallback_encoding: str | None = None, normalize_newlines: bool = False, ) -> None: self._limit_bytes = limit_bytes self._encoding = encoding + self._fallback_encoding = fallback_encoding self._normalize_newlines = normalize_newlines self._chunks: list[bytes] = [] self._kept_bytes = 0 self._total_bytes = 0 self._lock = threading.Lock() + def _decode(self, data: bytes) -> str: + """Decode captured bytes, preferring the primary encoding when valid. + + A forced ``encoding`` describes the shell's *own* output. Windows-native + children (``python.exe``, CLI shims) inherit the pipe and keep writing the + host code page, which is not valid in the forced encoding. Rather than + replacing every such byte with U+FFFD, retry the whole buffer with + ``fallback_encoding`` (the locale) when the primary decode fails. + """ + if self._fallback_encoding is None: + return data.decode(self._encoding, errors="replace") + try: + return data.decode(self._encoding) + except UnicodeDecodeError: + return data.decode(self._fallback_encoding, errors="replace") + def append(self, chunk: bytes) -> None: with self._lock: self._total_bytes += len(chunk) @@ -67,7 +85,7 @@ class _BoundedPipeCapture: total_bytes = self._total_bytes kept_bytes = self._kept_bytes - output = data.decode(self._encoding, errors="replace") + output = self._decode(data) if self._normalize_newlines: # Match ``subprocess.run(..., text=True)``: text streams use universal # newlines, translating both CRLF and bare CR to LF. @@ -191,9 +209,10 @@ class LocalSandbox(Sandbox): name: str, *, encoding: str = "utf-8", + fallback_encoding: str | None = None, normalize_newlines: bool = False, ) -> tuple[_BoundedPipeCapture, threading.Thread]: - capture = _BoundedPipeCapture(encoding=encoding, normalize_newlines=normalize_newlines) + capture = _BoundedPipeCapture(encoding=encoding, fallback_encoding=fallback_encoding, normalize_newlines=normalize_newlines) thread = threading.Thread(target=LocalSandbox._drain_pipe, args=(fd, capture), name=name, daemon=True) thread.start() return capture, thread @@ -561,7 +580,24 @@ class LocalSandbox(Sandbox): "MSYS2_ARG_CONV_EXCL": exclusions, } - if self._is_powershell(shell): + if self._is_msys_shell(shell): + # Git Bash/MSYS writes its own output as UTF-8, so the host code + # page (GBK on zh-CN) must not decode it. Windows-native children + # it spawns (python.exe, CLI shims) inherit the pipe and keep + # writing the host code page, so a buffer that is not valid UTF-8 + # falls back to the locale instead of being replaced wholesale. + stdout, stderr, returncode, timed_out = self._run_windows_command( + args, + timeout, + sandbox_env, + encoding="utf-8", + fallback_encoding=locale.getpreferredencoding(False), + ) + elif self._is_powershell(shell): + # PowerShell is pinned to UTF-8 by the preamble above, so its own + # output must be decoded as UTF-8: any other code page (GBK on + # zh-CN) mangles non-ASCII output because the pipe decoder + # replaces instead of raising. stdout, stderr, returncode, timed_out = self._run_windows_command(args, timeout, sandbox_env, encoding="utf-8") else: stdout, stderr, returncode, timed_out = self._run_windows_command(args, timeout, sandbox_env) @@ -593,6 +629,7 @@ class LocalSandbox(Sandbox): env: dict[str, str] | None = None, *, encoding: str | None = None, + fallback_encoding: str | None = None, ) -> tuple[str, str, int, bool]: """Run with bounded capture, a process-tree timeout, and locale decoding unless overridden.""" timed_out = False @@ -630,12 +667,14 @@ class LocalSandbox(Sandbox): stdout_read_fd, "deerflow-bash-stdout-drain", encoding=encoding, + fallback_encoding=fallback_encoding, normalize_newlines=True, ) stderr_capture, stderr_thread = LocalSandbox._start_pipe_drain( stderr_read_fd, "deerflow-bash-stderr-drain", encoding=encoding, + fallback_encoding=fallback_encoding, normalize_newlines=True, ) diff --git a/backend/tests/test_local_sandbox_encoding.py b/backend/tests/test_local_sandbox_encoding.py index cb73cb024f..34b2906ae0 100644 --- a/backend/tests/test_local_sandbox_encoding.py +++ b/backend/tests/test_local_sandbox_encoding.py @@ -5,6 +5,7 @@ import subprocess import sys import pytest +from support.shell import require_posix_sh import deerflow.sandbox.local.local_sandbox as local_sandbox from deerflow.sandbox.local.local_sandbox import LocalSandbox, PathMapping, _BoundedPipeCapture @@ -23,6 +24,31 @@ def test_bounded_pipe_capture_decodes_non_utf8_output_with_configured_encoding() assert capture.read() == "caf\u00e9" +def test_bounded_pipe_capture_falls_back_to_locale_for_non_utf8_bytes(): + """A forced UTF-8 decode keeps the locale as a guarded fallback so output + from native children (which still use the host code page) survives.""" + capture = _BoundedPipeCapture(encoding="utf-8", fallback_encoding="cp936") + capture.append("\u4f60\u597d".encode("cp936")) + + assert capture.read() == "\u4f60\u597d" + + +def test_bounded_pipe_capture_prefers_primary_encoding_when_valid(): + capture = _BoundedPipeCapture(encoding="utf-8", fallback_encoding="cp936") + capture.append("\u4f60\u597d".encode("utf-8")) + + assert capture.read() == "\u4f60\u597d" + + +def test_bounded_pipe_capture_without_fallback_replaces_non_utf8_bytes(): + capture = _BoundedPipeCapture(encoding="utf-8") + capture.append("\u4f60\u597d".encode("cp936")) + + decoded = capture.read() + assert "\ufffd" in decoded + assert "\u4f60\u597d" not in decoded + + def test_bounded_pipe_capture_applies_text_mode_newline_normalization_when_enabled(): capture = _BoundedPipeCapture(normalize_newlines=True) capture.append(b"crlf\r\nbare-cr\rlf\n") @@ -92,6 +118,76 @@ assert output == expected + "\n\nStd Error:\n" + expected + "\n\nExit Code: 3", assert result.returncode == 0, result.stdout + result.stderr +@pytest.mark.skipif(os.name != "nt", reason="Requires a real Windows Git Bash/MSYS shell") +def test_windows_msys_cjk_roundtrip(): + """Git Bash/MSYS always emits UTF-8, so the host code page must not decode it.""" + shell = require_posix_sh() + if not LocalSandbox._is_msys_shell(shell): + pytest.skip(f"{shell} is not recognised as a Git Bash/MSYS shell") + probe = r""" +import sys +import deerflow.sandbox.local.local_sandbox as local_sandbox +from deerflow.sandbox.local.local_sandbox import LocalSandbox + +# Keep this regression effective even on an English or UTF-8 Windows runner. +local_sandbox.locale.getpreferredencoding = lambda _: "cp936" +LocalSandbox._get_shell = staticmethod(lambda: sys.argv[1]) +expected = "\u4f60\u597d \u65e5\u672c\u8a9e" +output = LocalSandbox("encoding-probe").execute_command(f"printf '%s\n' '{expected}'", timeout=15) +assert output == expected + "\n", ascii(output) +""" + env = {**os.environ, "PYTHONUTF8": "0"} + result = subprocess.run( + [sys.executable, "-c", probe, shell], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + env=env, + timeout=30, + check=False, + ) + assert result.returncode == 0, result.stdout + result.stderr + + +@pytest.mark.skipif(os.name != "nt", reason="Requires a real Windows Git Bash/MSYS shell") +def test_windows_msys_native_child_cjk_roundtrip(): + """A Windows-native child of Git Bash (python.exe) inherits the pipe and keeps + writing the host code page; the forced UTF-8 decode must fall back to the + locale for those bytes instead of replacing the text with U+FFFD.""" + shell = require_posix_sh() + if not LocalSandbox._is_msys_shell(shell): + pytest.skip(f"{shell} is not recognised as a Git Bash/MSYS shell") + probe = r""" +import sys +import deerflow.sandbox.local.local_sandbox as local_sandbox +from deerflow.sandbox.local.local_sandbox import LocalSandbox + +# Pin the fallback code page so the regression holds on non-zh-CN runners. +local_sandbox.locale.getpreferredencoding = lambda _: "cp936" +LocalSandbox._get_shell = staticmethod(lambda: sys.argv[1]) +expected = "\u4f60\u597d \u65e5\u672c\u8a9e" +interpreter = sys.executable.replace("\\", "/") +command = f"'{interpreter}' -c \"print('{expected}')\"" +# PYTHONIOENCODING makes the native child emit CP936 even on a UTF-8 host, +# mirroring a zh-CN host's native Python writing to a redirected pipe. +output = LocalSandbox("encoding-probe").execute_command(command, timeout=30, env={"PYTHONIOENCODING": "cp936"}) +assert output == expected + "\n", ascii(output) +""" + env = {**os.environ, "PYTHONUTF8": "0"} + result = subprocess.run( + [sys.executable, "-c", probe, shell], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + env=env, + timeout=60, + check=False, + ) + assert result.returncode == 0, result.stdout + result.stderr + + @pytest.mark.skipif(os.name == "nt", reason="POSIX capture semantics") def test_posix_command_capture_preserves_newlines(): stdout, stderr, returncode, timed_out = LocalSandbox._run_posix_command( @@ -293,10 +389,37 @@ def test_execute_command_forces_utf8_console_for_powershell_cjk_output(monkeypat assert calls[0][3] == "utf-8" +def test_execute_command_forces_utf8_decoding_with_locale_fallback_for_msys(monkeypatch): + """Git Bash/MSYS writes UTF-8 to its pipes; decoding with the host code page + (GBK on zh-CN) mangles CJK output silently because the pipe decoder replaces + instead of raising. The native children Git Bash spawns still write the host + code page, so the forced UTF-8 decode keeps the locale as a guarded fallback.""" + calls: list[tuple[list[str], float, dict[str, str], str | None, str | None]] = [] + + def fake_run(args, timeout, env, *, encoding=None, fallback_encoding=None): + calls.append((args, timeout, env, encoding, fallback_encoding)) + return "你好", "", 0, False + + monkeypatch.setattr(local_sandbox.os, "name", "nt") + monkeypatch.setattr(local_sandbox.os, "environ", {"PATH": r"C:\Program Files\Git\bin"}) + monkeypatch.setattr(local_sandbox.locale, "getpreferredencoding", lambda _: "cp936") + monkeypatch.setattr(LocalSandbox, "_get_shell", staticmethod(lambda: r"C:\Program Files\Git\bin\sh.exe")) + monkeypatch.setattr(LocalSandbox, "_run_windows_command", staticmethod(fake_run)) + + output = LocalSandbox("t").execute_command("echo 你好") + + assert output == "你好" + assert calls[0][0] == [r"C:\Program Files\Git\bin\sh.exe", "-c", "echo 你好"] + assert calls[0][3] == "utf-8" + # Native children of Git Bash keep writing the host code page; the locale is + # retained as a guarded fallback instead of forcing every byte through UTF-8. + assert calls[0][4] == "cp936" + + def test_execute_command_keeps_msys_path_conversion_for_host_commands_on_windows(monkeypatch): calls: list[tuple[list[str], float, dict[str, str]]] = [] - def fake_run(args, timeout, env): + def fake_run(args, timeout, env, *, encoding=None, fallback_encoding=None): calls.append((args, timeout, env)) return "ok", "", 0, False @@ -324,7 +447,7 @@ def test_execute_command_keeps_msys_path_conversion_for_host_commands_on_windows def test_execute_command_scopes_msys_path_conversion_exclusions_on_windows(monkeypatch): calls: list[tuple[list[str], float, dict[str, str]]] = [] - def fake_run(args, timeout, env): + def fake_run(args, timeout, env, *, encoding=None, fallback_encoding=None): calls.append((args, timeout, env)) return "ok", "", 0, False @@ -346,7 +469,7 @@ def test_execute_command_scopes_msys_path_conversion_exclusions_on_windows(monke def test_execute_command_ignores_root_msys_mapping_for_host_commands_on_windows(monkeypatch): calls: list[tuple[list[str], float, dict[str, str]]] = [] - def fake_run(args, timeout, env): + def fake_run(args, timeout, env, *, encoding=None, fallback_encoding=None): calls.append((args, timeout, env)) return "ok", "", 0, False