From 0e9690493d2579196b56768048bfd5056d4c0ca8 Mon Sep 17 00:00:00 2001 From: CodeAlex <59381946+CodeAlex52@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:30:07 +0800 Subject: [PATCH] fix(scripts): keep sync_labels.py from crashing on non-UTF-8 stdio (#7473) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(scripts): keep sync_labels.py from crashing on non-UTF-8 stdio The ✓ / ✗ status glyphs and the em dash in the Done line crashed the sync with UnicodeEncodeError whenever stdout was redirected on a host whose stdio encoding cannot represent them (Windows code pages, POSIX C locale, PYTHONIOENCODING=ascii) — after `gh label create --force` had already run, leaving a half-applied sync and a traceback. gh's UTF-8 diagnostics were additionally decoded with the host locale, mangling failure messages. Closes #7389. - Reconfigure stdout/stderr in main(): UTF-8 with backslashreplace when redirected, backslashreplace only on consoles. - Decode the gh subprocess as UTF-8 with errors="replace", matching scripts/support_bundle.py's _run_command. - Add backend/tests/test_sync_labels_script.py: hermetic offline cases (stub yaml, stub gh) running the real script under PYTHONIOENCODING=ascii for the success path, a UTF-8 gh failure, and --dry-run. All three go red on the unfixed script. * style(tests): fix sync-labels regression lint * docs(scripts): document label sync encoding policy --------- Co-authored-by: Willem Jiang --- backend/tests/test_sync_labels_script.py | 118 +++++++++++++++++++++++ scripts/AGENTS.md | 38 ++++---- scripts/sync_labels.py | 16 ++- 3 files changed, 153 insertions(+), 19 deletions(-) create mode 100644 backend/tests/test_sync_labels_script.py diff --git a/backend/tests/test_sync_labels_script.py b/backend/tests/test_sync_labels_script.py new file mode 100644 index 0000000000..fe733eda69 --- /dev/null +++ b/backend/tests/test_sync_labels_script.py @@ -0,0 +1,118 @@ +"""Offline regression tests for scripts/sync_labels.py. + +The status glyphs (✓ / ✗) used to crash the sync with UnicodeEncodeError after +`gh label create --force` had already run, on hosts whose stdio encoding cannot +represent them (Windows code pages, POSIX C locale, PYTHONIOENCODING=ascii); +`gh`'s UTF-8 diagnostics were decoded with the host locale instead. +""" + +from __future__ import annotations + +import os +import stat +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] + +# The script imports pyyaml at module load; the stub keeps these tests hermetic +# (pyyaml is not a backend dependency — the Makefile runs the script with +# `uv run --with pyyaml`). +STUB_YAML = """\ +def safe_load(text): + return { + "labels": [ + {"name": "test-label", "color": "#ff0000", "description": "中文 描述"}, + ] + } +""" + +LABELS_YAML = """\ +labels: + - name: test-label + color: ff0000 + description: "中文 描述" +""" + +pytestmark = pytest.mark.skipif(os.name == "nt", reason="the stub gh is a POSIX shell script") + + +@pytest.fixture +def script_checkout(tmp_path: Path) -> Path: + scripts_dir = tmp_path / "scripts" + scripts_dir.mkdir() + (tmp_path / ".github").mkdir() + (scripts_dir / "sync_labels.py").write_text( + (REPO_ROOT / "scripts" / "sync_labels.py").read_text(encoding="utf-8"), + encoding="utf-8", + ) + (tmp_path / ".github" / "labels.yml").write_text(LABELS_YAML, encoding="utf-8") + stubs = tmp_path / "stubs" + stubs.mkdir() + (stubs / "yaml.py").write_text(STUB_YAML, encoding="utf-8") + return tmp_path + + +def _stub_gh(tmp_path: Path, body: str) -> Path: + bin_dir = tmp_path / "bin" + bin_dir.mkdir(exist_ok=True) + gh = bin_dir / "gh" + gh.write_text(f"#!/bin/sh\n{body}\n", encoding="utf-8") + gh.chmod(gh.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) + return bin_dir + + +def _run(script_checkout: Path, tmp_path: Path, bin_dir: Path | None, *args: str): + env = { + **os.environ, + "PYTHONIOENCODING": "ascii", + "PYTHONPATH": str(script_checkout / "stubs"), + } + if bin_dir is not None: + env["PATH"] = f"{bin_dir}:{os.environ.get('PATH', '')}" + return subprocess.run( + [sys.executable, str(script_checkout / "scripts" / "sync_labels.py"), *args], + env=env, + capture_output=True, + check=False, + timeout=60, + ) + + +def test_success_path_emits_utf8_glyphs_into_non_utf8_stdout(script_checkout: Path, tmp_path: Path): + bin_dir = _stub_gh(tmp_path, "exit 0") + + result = _run(script_checkout, tmp_path, bin_dir, "--repo", "octo/demo") + + # Before the fix this crashed with UnicodeEncodeError after the label was + # already created on GitHub. + assert result.returncode == 0, result.stderr + assert "✓ test-label".encode() in result.stdout + assert "Done — 1 labels in sync.".encode() in result.stdout + assert b"UnicodeEncodeError" not in result.stderr + + +def test_gh_failure_decodes_utf8_and_reports_gracefully(script_checkout: Path, tmp_path: Path): + bin_dir = _stub_gh(tmp_path, "printf '%s\\n' '中文 失败' >&2\nexit 1") + + result = _run(script_checkout, tmp_path, bin_dir, "--repo", "octo/demo") + + # Before the fix the UTF-8 stderr was decoded with the host codec and + # raised UnicodeDecodeError instead of printing the failure line. + assert result.returncode == 1, result.stdout + assert "✗ test-label: 中文 失败".encode() in result.stderr + assert b"1 label(s) failed to sync" in result.stderr + assert b"Traceback" not in result.stderr + assert b"UnicodeDecodeError" not in result.stderr + + +def test_dry_run_works_under_ascii_stdout(script_checkout: Path, tmp_path: Path): + # No gh on PATH: the dry run must not invoke it. + result = _run(script_checkout, tmp_path, None, "--dry-run") + + assert result.returncode == 0, result.stderr + assert b"[dry-run]" in result.stdout + assert "Done — 1 labels in sync.".encode() in result.stdout diff --git a/scripts/AGENTS.md b/scripts/AGENTS.md index 6834fcfb3f..604ee99da5 100644 --- a/scripts/AGENTS.md +++ b/scripts/AGENTS.md @@ -25,19 +25,24 @@ path variables and compare storage defaults with the launcher's actual shell blo ## Dependency and Label Checks -`check_extension.py` uses Gateway's wheel asset validator; CI checks examples -in `extension-package-check.yml`. Tests: `backend/tests/test_extension_check_script.py`. +`check_extension.py`: Gateway wheel asset validator; example CI: +`extension-package-check.yml`. Tests: `backend/tests/test_extension_check_script.py`. -`check.py`: decode UTF-8 with replacement; pnpm uses -`PYTHONIOENCODING=utf-8:backslashreplace`. Keep Unicode/exit status in -`make check` in any locale. Tests: `backend/tests/test_check_script.py`. +`check.py`: UTF-8 replacement decoding; pnpm uses +`PYTHONIOENCODING=utf-8:backslashreplace`. Preserve `make check` Unicode/exit +status in any locale. Tests: `backend/tests/test_check_script.py`. -`doctor.py`/`setup_wizard.py`: at `main` start, keep stdout's encoding -with `errors="replace"` for legacy terminals/pipes. Guard `reconfigure` on -embedded/captured streams. Offline tests: `backend/tests/test_setup_script_encoding.py`. +`doctor.py`/`setup_wizard.py`: at `main` start, retain stdout encoding with +`errors="replace"`; guard `reconfigure` for embedded/captured streams. +Legacy-terminal/pipe tests: `backend/tests/test_setup_script_encoding.py`. -`check_label_declarations.py`: stdlib-only guard; reject unsupported -expressions. Tests: `backend/tests/test_label_declarations.py`. +`sync_labels.py`: guard stdout/stderr `reconfigure` at `main` start. Retain +TTY encodings; intentionally force UTF-8 on redirects for CI logs. Both use +`errors="backslashreplace"`; decode `gh` output as UTF-8 with replacement. +Tests (stub `gh`/`yaml`, ASCII stdio): `backend/tests/test_sync_labels_script.py`. + +`check_label_declarations.py`: stdlib-only; reject unsupported expressions. +Tests: `backend/tests/test_label_declarations.py`. ## Manual Claude OAuth Export @@ -495,14 +500,11 @@ The support bundle's `extensions_config.json` reader accepts UTF-8 with or without a leading BOM, matching the runtime loader. Preserve redaction and avoid flagging a valid BOM-prefixed file as a syntax error in triage output. -Support-bundle and doctor tool captures explicitly decode UTF-8 with replacement -for invalid bytes. Set `PYTHONIOENCODING=utf-8:backslashreplace` only in the copied -support-bundle child environment so Python helpers can print Unicode and escape -surrogates without aborting diagnostics or changing the parent. -Keep exit codes, timeouts and redaction intact; do not rely on the host locale. -Regressions use real local children, including ASCII/GBK capture defaults, -nonzero exits, surrogate characters and malformed output, without invoking -provider diagnostics. Doctor covers both `_run` streams and pnpm runner capture. +Support-bundle/doctor captures use UTF-8 with replacement, preserving exit codes, +timeouts and redaction. Only copied support-bundle child env sets +`PYTHONIOENCODING=utf-8:backslashreplace` for Unicode/surrogates; parent unchanged. +Offline real-child tests: ASCII/GBK defaults, nonzero exits, surrogates, +invalid bytes; no provider calls. Doctor covers both `_run` streams/pnpm capture. ## Production dotenv selection diff --git a/scripts/sync_labels.py b/scripts/sync_labels.py index 2264573b57..bdcf14a6ba 100644 --- a/scripts/sync_labels.py +++ b/scripts/sync_labels.py @@ -56,7 +56,9 @@ def sync_label(label: dict[str, str], repo: str | None, dry_run: bool) -> bool: print(f"[dry-run] {' '.join(cmd)}") return True - result = subprocess.run(cmd, capture_output=True, text=True) + # gh emits UTF-8; decoding with the host locale mangles (or crashes on) + # failure messages containing non-ASCII on cp1252/cp936/C-locale hosts. + result = subprocess.run(cmd, capture_output=True, encoding="utf-8", errors="replace") if result.returncode != 0: print(f" ✗ {name}: {result.stderr.strip()}", file=sys.stderr) return False @@ -65,6 +67,18 @@ def sync_label(label: dict[str, str], repo: str | None, dry_run: bool) -> bool: def main() -> int: + # The status glyphs (✓ / ✗) must survive hosts whose stdio encoding cannot + # represent them (Windows code pages, POSIX C locale): the crash happens + # after `gh label create --force` has already run, leaving a half-applied + # sync and a traceback instead of a status line. + for stream in (sys.stdout, sys.stderr): + if not hasattr(stream, "reconfigure"): + continue + if stream.isatty(): + stream.reconfigure(errors="backslashreplace") + else: + stream.reconfigure(encoding="utf-8", errors="backslashreplace") + parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--repo", help="Target repository as OWNER/NAME") parser.add_argument(