Merge pull request #510 from xiarimr/fix-windows-encoding-crash
Some checks are pending
CI / build (push) Waiting to run
Test / python-smoke (push) Waiting to run
Test / web-build (push) Waiting to run

fix: prevent Windows subprocess output decode crashes
This commit is contained in:
Yang Haoran 2026-08-26 01:11:49 +08:00 committed by GitHub
commit 04486201fc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
34 changed files with 173 additions and 60 deletions

View file

@ -68,7 +68,7 @@ def run_bash(command: str) -> str:
return "Error: Dangerous command blocked"
try:
r = subprocess.run(command, shell=True, cwd=os.getcwd(),
capture_output=True, text=True, timeout=120)
capture_output=True, text=True, errors="replace", timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -57,8 +57,8 @@ def run_bash(command: str) -> str:
return "Error: Dangerous command blocked"
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True,
encoding="utf-8", errors="replace", timeout=120)
capture_output=True, text=True, errors="replace",
timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -108,7 +108,7 @@ def run_bash(command: str) -> str:
return "Error: Dangerous command blocked"
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120)
capture_output=True, text=True, errors="replace", timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -62,7 +62,7 @@ def run_bash(command: str) -> str:
return "Error: Dangerous command blocked"
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120)
capture_output=True, text=True, errors="replace", timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -133,7 +133,7 @@ def run_bash(command: str) -> str:
return "Error: Dangerous command blocked"
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120)
capture_output=True, text=True, errors="replace", timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -150,7 +150,7 @@ def run_bash(command: str) -> str:
return "Error: Dangerous command blocked"
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120)
capture_output=True, text=True, errors="replace", timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -140,7 +140,7 @@ def run_bash(command: str) -> str:
return "Error: Dangerous command blocked"
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120)
capture_output=True, text=True, errors="replace", timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -74,7 +74,7 @@ class BackgroundManager:
try:
r = subprocess.run(
command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=300
capture_output=True, text=True, errors="replace", timeout=300
)
output = (r.stdout + r.stderr).strip()[:50000]
status = "completed"
@ -130,7 +130,7 @@ def run_bash(command: str) -> str:
return "Error: Dangerous command blocked"
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120)
capture_output=True, text=True, errors="replace", timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -273,7 +273,7 @@ def _run_bash(command: str) -> str:
try:
r = subprocess.run(
command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120,
capture_output=True, text=True, errors="replace", timeout=120,
)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"

View file

@ -314,7 +314,7 @@ def _run_bash(command: str) -> str:
try:
r = subprocess.run(
command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120,
capture_output=True, text=True, errors="replace", timeout=120,
)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"

View file

@ -400,7 +400,7 @@ def _run_bash(command: str) -> str:
try:
r = subprocess.run(
command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120,
capture_output=True, text=True, errors="replace", timeout=120,
)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"

View file

@ -63,7 +63,7 @@ def detect_repo_root(cwd: Path) -> Path | None:
["git", "rev-parse", "--show-toplevel"],
cwd=cwd,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=10,
)
if r.returncode != 0:
@ -246,7 +246,7 @@ class WorktreeManager:
["git", "rev-parse", "--is-inside-work-tree"],
cwd=self.repo_root,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=10,
)
return r.returncode == 0
@ -260,7 +260,7 @@ class WorktreeManager:
["git", *args],
cwd=self.repo_root,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=120,
)
if r.returncode != 0:
@ -365,7 +365,7 @@ class WorktreeManager:
["git", "status", "--short", "--branch"],
cwd=path,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=60,
)
text = (r.stdout + r.stderr).strip()
@ -389,7 +389,7 @@ class WorktreeManager:
shell=True,
cwd=path,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=300,
)
out = (r.stdout + r.stderr).strip()
@ -498,7 +498,7 @@ def run_bash(command: str) -> str:
shell=True,
cwd=WORKDIR,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=120,
)
out = (r.stdout + r.stderr).strip()

View file

@ -89,7 +89,7 @@ def run_bash(command: str) -> str:
return "Error: Dangerous command blocked"
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120)
capture_output=True, text=True, errors="replace", timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:
@ -345,7 +345,7 @@ class BackgroundManager:
def _exec(self, tid: str, command: str, timeout: int):
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=timeout)
capture_output=True, text=True, errors="replace", timeout=timeout)
output = (r.stdout + r.stderr).strip()[:50000]
self.tasks[tid].update({"status": "completed", "result": output or "(no output)"})
except Exception as e:

View file

@ -74,7 +74,7 @@ def run_bash(command: str) -> str:
return "Error: Dangerous command blocked"
try:
r = subprocess.run(command, shell=True, cwd=os.getcwd(),
capture_output=True, text=True, timeout=120)
capture_output=True, text=True, errors="replace", timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -56,8 +56,8 @@ def run_bash(command: str) -> str:
return "Error: Dangerous command blocked"
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True,
encoding="utf-8", errors="replace", timeout=120)
capture_output=True, text=True, errors="replace",
timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -63,7 +63,7 @@ SYSTEM = f"You are a coding agent at {WORKDIR}. All destructive operations requi
def run_bash(command: str) -> str:
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120)
capture_output=True, text=True, errors="replace", timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -52,7 +52,7 @@ SYSTEM = f"You are a coding agent at {WORKDIR}. Use tools to solve tasks. Act, d
def run_bash(command: str) -> str:
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120)
capture_output=True, text=True, errors="replace", timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -58,7 +58,7 @@ SYSTEM = (
def run_bash(command: str) -> str:
try:
r = subprocess.run(command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120)
capture_output=True, text=True, errors="replace", timeout=120)
out = (r.stdout + r.stderr).strip()
return out[:50000] if out else "(no output)"
except subprocess.TimeoutExpired:

View file

@ -58,7 +58,7 @@ def run_bash(command: str) -> str:
try:
result = subprocess.run(
command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120,
capture_output=True, text=True, errors="replace", timeout=120,
)
output = (result.stdout + result.stderr).strip()
return output[:50000] if output else "(no output)"

View file

@ -142,7 +142,7 @@ def run_bash(command: str) -> str:
try:
result = subprocess.run(
command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120,
capture_output=True, text=True, errors="replace", timeout=120,
)
output = (result.stdout + result.stderr).strip()
return output[:50000] if output else "(no output)"

View file

@ -79,7 +79,7 @@ def run_bash(command: str) -> str:
try:
result = subprocess.run(
command, shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=120,
capture_output=True, text=True, errors="replace", timeout=120,
)
output = (result.stdout + result.stderr).strip()
return output[:50000] if output else "(no output)"

View file

@ -545,7 +545,7 @@ def run_bash(command: str) -> str:
shell=True,
cwd=WORKDIR,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=120,
)
output = (result.stdout + result.stderr).strip()

View file

@ -282,7 +282,7 @@ def run_bash(command: str) -> str:
shell=True,
cwd=WORKDIR,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=120,
)
output = (result.stdout + result.stderr).strip()

View file

@ -88,7 +88,7 @@ def _run_bash_process(command: str) -> tuple[str, int | None]:
cwd=WORKDIR,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
text=True, errors="replace",
start_new_session=True,
)
with _shell_process_lock:

View file

@ -60,7 +60,7 @@ def run_bash(command: str) -> str:
shell=True,
cwd=WORKDIR,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=120,
)
output = (result.stdout + result.stderr).strip()

View file

@ -372,7 +372,7 @@ def _run_git(args: list[str], cwd: Path | None = None) -> tuple[bool, str]:
try:
result = subprocess.run(
["git", *args], cwd=cwd or WORKDIR,
capture_output=True, text=True, timeout=30,
capture_output=True, text=True, errors="replace", timeout=30,
)
except (OSError, subprocess.TimeoutExpired) as exc:
return False, f"{type(exc).__name__}: {exc}"
@ -670,7 +670,7 @@ def run_bash(command: str, cwd: Path | None = None) -> str:
shell=True,
cwd=cwd or WORKDIR,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=120,
)
output = (result.stdout + result.stderr).strip()

View file

@ -60,7 +60,7 @@ def run_bash(command: str) -> str:
shell=True,
cwd=WORKDIR,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=120,
)
output = (result.stdout + result.stderr).strip()

View file

@ -454,7 +454,7 @@ def _run_git(args: list[str], cwd: Path | None = None) -> tuple[bool, str]:
try:
result = subprocess.run(
["git", *args], cwd=cwd or WORKDIR,
capture_output=True, text=True, timeout=30,
capture_output=True, text=True, errors="replace", timeout=30,
)
except (OSError, subprocess.TimeoutExpired) as exc:
return False, f"{type(exc).__name__}: {exc}"
@ -903,7 +903,7 @@ def _run_bash_process(command: str, cwd: Path | None = None) -> tuple[str, int |
process = subprocess.Popen(
command, shell=True, cwd=cwd or WORKDIR,
stdout=subprocess.PIPE, stderr=subprocess.PIPE,
text=True, start_new_session=True,
text=True, errors="replace", start_new_session=True,
)
with _shell_process_lock:
_shell_processes.add(process)

View file

@ -761,7 +761,7 @@ class AgentSession:
shell=True,
cwd=self.workdir,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=120,
check=False,
)

View file

@ -70,7 +70,7 @@ def execute_tool(name: str, args: dict) -> str:
try:
r = subprocess.run(
args["command"], shell=True, cwd=WORKDIR,
capture_output=True, text=True, timeout=60
capture_output=True, text=True, errors="replace", timeout=60
)
return (r.stdout + r.stderr).strip() or "(empty)"
except subprocess.TimeoutExpired:

View file

@ -168,7 +168,7 @@ def run_bash(command: str) -> str:
shell=True,
cwd=WORKDIR,
capture_output=True,
text=True,
text=True, errors="replace",
timeout=60
)
output = (result.stdout + result.stderr).strip()

View file

@ -63,7 +63,7 @@ def run(prompt, history=[]):
if b.type == "tool_use":
print(f"> {{b.input['command']}}")
try:
out = subprocess.run(b.input["command"], shell=True, capture_output=True, text=True, timeout=60)
out = subprocess.run(b.input["command"], shell=True, capture_output=True, text=True, errors="replace", timeout=60)
output = (out.stdout + out.stderr).strip() or "(empty)"
except Exception as e:
output = f"Error: {{e}}"
@ -133,7 +133,7 @@ def execute(name: str, args: dict) -> str:
if any(d in args["command"] for d in dangerous):
return "Error: Dangerous command blocked"
try:
r = subprocess.run(args["command"], shell=True, cwd=WORKDIR, capture_output=True, text=True, timeout=60)
r = subprocess.run(args["command"], shell=True, cwd=WORKDIR, capture_output=True, text=True, errors="replace", timeout=60)
return (r.stdout + r.stderr).strip()[:50000] or "(empty)"
except subprocess.TimeoutExpired:
return "Error: Timeout (60s)"

View file

@ -0,0 +1,113 @@
import ast
import os
import runpy
import shlex
import subprocess
import sys
import tempfile
from pathlib import Path
import pytest
from test_skill_loading import load_lesson
ROOT = Path(__file__).resolve().parents[1]
SCAFFOLD = ROOT / "skills" / "agent-builder" / "scripts" / "init_agent.py"
SOURCE_FILES = tuple(sorted({
*ROOT.glob("s[0-9][0-9]_*/code.py"),
*ROOT.glob("agents/*.py"),
*(ROOT / "skills" / "agent-builder").rglob("*.py"),
}))
def child_command(expression: str, stream: str = "stdout") -> str:
script = f"import sys; sys.{stream}.buffer.write({expression})"
args = [sys.executable, "-c", script]
return subprocess.list2cmdline(args) if os.name == "nt" else shlex.join(args)
@pytest.mark.parametrize(
("encoding", "payload", "expected"),
[
("utf-8", "'中文'.encode('utf-8')", "中文"),
("gbk", "'中文'.encode('gbk')", "中文"),
("gbk", "bytes([0xff])", "\ufffd"),
],
)
def test_s07_bash_decodes_output_without_crashing(
monkeypatch: pytest.MonkeyPatch,
encoding: str,
payload: str,
expected: str,
) -> None:
with tempfile.TemporaryDirectory() as tmp:
lesson = load_lesson(Path(tmp))
monkeypatch.setattr(lesson.subprocess.locale, "getencoding", lambda: encoding)
assert lesson.run_bash(child_command(payload)) == expected
def test_s07_bash_handles_stderr_and_empty_output(
monkeypatch: pytest.MonkeyPatch,
) -> None:
with tempfile.TemporaryDirectory() as tmp:
lesson = load_lesson(Path(tmp))
monkeypatch.setattr(lesson.subprocess.locale, "getencoding", lambda: "gbk")
assert lesson.run_bash(child_command("bytes([0xff])", "stderr")) == "\ufffd"
assert lesson.run_bash(child_command("b''")) == "(no output)"
def test_s07_bash_preserves_timeout_message(monkeypatch: pytest.MonkeyPatch) -> None:
with tempfile.TemporaryDirectory() as tmp:
lesson = load_lesson(Path(tmp))
def time_out(*args, **kwargs):
raise subprocess.TimeoutExpired(args[0], 120)
monkeypatch.setattr(lesson.subprocess, "run", time_out)
assert lesson.run_bash("slow command") == "Error: Timeout (120s)"
def find_missing_policy(tree: ast.AST, source: str) -> list[str]:
missing_policy: list[str] = []
for node in ast.walk(tree):
if not isinstance(node, ast.Call) or not isinstance(node.func, ast.Attribute):
continue
if not (
isinstance(node.func.value, ast.Name)
and node.func.value.id == "subprocess"
and node.func.attr in {"run", "Popen"}
):
continue
keywords = {keyword.arg: keyword.value for keyword in node.keywords}
text_mode = keywords.get("text")
if not (isinstance(text_mode, ast.Constant) and text_mode.value is True):
continue
errors = keywords.get("errors")
if not (isinstance(errors, ast.Constant) and errors.value == "replace"):
missing_policy.append(f"{source}:{node.lineno}")
return missing_policy
def test_subprocess_text_calls_replace_decode_errors() -> None:
missing_policy: list[str] = []
for path in SOURCE_FILES:
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
missing_policy.extend(find_missing_policy(tree, str(path.relative_to(ROOT))))
templates = runpy.run_path(str(SCAFFOLD))["TEMPLATES"]
for level, template in templates.items():
generated = template.format(name="test_agent")
missing_policy.extend(
find_missing_policy(ast.parse(generated), f"generated agent level {level}")
)
assert not missing_policy, (
"subprocess text output must use errors=\"replace\": "
+ ", ".join(missing_policy)
)

File diff suppressed because one or more lines are too long