From 3dff6f1d5069ded92096b275fa164364d5b5c2f7 Mon Sep 17 00:00:00 2001 From: Ali jawwad <33836051+jawwad-ali@users.noreply.github.com> Date: Thu, 6 Aug 2026 18:17:36 +0500 Subject: [PATCH] fix(scripts): stop check-prerequisites text mode crashing on a legacy stdout code page (#3890) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(scripts): stop check-prerequisites text mode crashing on a legacy code page _check_file/_check_dir hard-code U+2713/U+2717 and print() them to sys.stdout. On Windows sys.stdout falls back to the ANSI code page whenever stdout is not a console — which is every time an agent or a workflow step captures the output — and U+2713 is unencodable in cp1252: stdout encoding: cp1252 UnicodeEncodeError: 'charmap' codec can't encode character '✓' So text mode aborted right after printing "AVAILABLE_DOCS:", losing every per-document line. Fall back to ASCII when stdout cannot encode the glyph. "[OK]"/"[FAIL]" is the rendering these markers already have in-tree: Test-FileExists in scripts/powershell/common.ps1 emits exactly those, and normalize_status_text in tests/parity_helpers.py maps the glyphs onto them, so the twins already treat the two forms as equivalent. Co-Authored-By: Claude Opus 5 (1M context) * test(scripts): cover both status markers in the cp1252 regression Review catch: the fixture left every reported document absent (the empty contracts/ also reports missing), so the test only ever called _status_marker(False). The assertion was `"[OK]" in out or "[FAIL]" in out`, which "[FAIL]" alone satisfied. Proved the hole by mutation: replacing the fallback body with a bare `return "[FAIL]"` — deleting the success branch outright — left the test GREEN. Add research.md so one document is present, and assert both markers explicitly. The strengthened test now kills all three mutations: fallback always "[FAIL]" -> FAILS (was passing) fallback always "[OK]" -> FAILS no fallback at all -> FAILS (the original bug) unmutated -> 12 passed, 8 skipped Missing documents are still present in the fixture, so the failure path stays covered too. Co-Authored-By: Claude Opus 5 (1M context) * fix(scripts): restore the _status_marker ASCII fallback The previous commit on this branch unintentionally reverted the source fix while adding the strengthened test, so the branch carried the test without the implementation it tests. Cause: my local verification script reverted the file for its red run with `git checkout upstream/main -- `, which writes the INDEX as well as the working tree. Restoring the working-tree copy afterwards left main's version staged, and the next commit captured it. Restores the fix from 275663b. Verified: 12 passed / 8 skipped, and the red run (source reverted) produces 1 new-vs-baseline failure. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- scripts/python/check_prerequisites.py | 25 ++++++++-- .../test_check_prerequisites_python_parity.py | 46 +++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/scripts/python/check_prerequisites.py b/scripts/python/check_prerequisites.py index 50c31cb51..e909ffb50 100644 --- a/scripts/python/check_prerequisites.py +++ b/scripts/python/check_prerequisites.py @@ -130,14 +130,31 @@ def _print_paths_only(paths: FeaturePaths, json_mode: bool) -> None: print(f"TASKS: {paths.tasks}") +def _status_marker(ok: bool) -> str: + """Return the status glyph, downgraded to ASCII when stdout cannot encode it. + + On Windows sys.stdout falls back to the ANSI code page whenever it is not a + console - a pipe or a file redirect, which is how agents and workflow steps + invoke these scripts - and U+2713 is unencodable in cp1252, so printing it + raised UnicodeEncodeError and aborted the report right after + "AVAILABLE_DOCS:". "[OK]"/"[FAIL]" is the ASCII rendering these markers + already have in-tree: see Test-FileExists in scripts/powershell/common.ps1 + and normalize_status_text in tests/parity_helpers.py. + """ + glyph = "✓" if ok else "✗" + try: + glyph.encode(getattr(sys.stdout, "encoding", None) or "utf-8") + except (LookupError, UnicodeEncodeError): + return "[OK]" if ok else "[FAIL]" + return glyph + + def _check_file(path: Path, description: str) -> None: - marker = "✓" if path.is_file() else "✗" - print(f" {marker} {description}") + print(f" {_status_marker(path.is_file())} {description}") def _check_dir(path: Path, description: str) -> None: - marker = "✓" if _dir_has_entries(path) else "✗" - print(f" {marker} {description}") + print(f" {_status_marker(_dir_has_entries(path))} {description}") def _print_text_results(paths: FeaturePaths, include_tasks: bool) -> None: diff --git a/tests/test_check_prerequisites_python_parity.py b/tests/test_check_prerequisites_python_parity.py index cdc02b915..5c5083f61 100644 --- a/tests/test_check_prerequisites_python_parity.py +++ b/tests/test_check_prerequisites_python_parity.py @@ -181,6 +181,52 @@ def test_python_text_output_matches_bash(prereq_repo: Path) -> None: assert _normalize_status_text(py.stdout) == _normalize_status_text(bash.stdout) +def test_python_text_output_survives_a_legacy_stdout_code_page( + prereq_repo: Path, +) -> None: + """Text mode must not crash when stdout cannot encode the status glyphs. + + On Windows sys.stdout falls back to the ANSI code page whenever it is not a + console — which is every time an agent or a workflow step captures the + output. U+2713 is unencodable in cp1252, so printing it raised + UnicodeEncodeError and truncated the report right after "AVAILABLE_DOCS:". + The ASCII fallback is the rendering these markers already have in-tree + (Test-FileExists in scripts/powershell/common.ps1, and + normalize_status_text here). + """ + feat = prereq_repo / "specs" / "001-my-feature" + feat.mkdir(parents=True) + (feat / "plan.md").write_text("# plan\n", encoding="utf-8") + # research.md is present and the rest are not, so BOTH status markers are + # produced in the same cp1252 subprocess: U+2713 for the available document + # and U+2717 for the missing ones. Asserting only one of them would let a + # fallback that always returned "[FAIL]" pass. + (feat / "research.md").write_text("# research\n", encoding="utf-8") + (feat / "contracts").mkdir() # present but empty -> reported missing + _write_feature_json(prereq_repo) + + env = _clean_env() + env["PYTHONIOENCODING"] = "cp1252" + result = _run(_py_cmd(prereq_repo, "--include-tasks"), prereq_repo, env=env) + + assert result.returncode == 0, result.stderr + assert "UnicodeEncodeError" not in result.stderr + assert "AVAILABLE_DOCS:" in result.stdout + # Every per-document line must still be there, not truncated away by the + # encode error. + for doc in ( + "research.md", + "data-model.md", + "contracts/", + "quickstart.md", + "tasks.md", + ): + assert doc in result.stdout, (doc, result.stdout) + # Both fallback markers, so neither branch of _status_marker can regress. + assert "[OK] research.md" in result.stdout, result.stdout + assert "[FAIL] quickstart.md" in result.stdout, result.stdout + + @requires_bash def test_python_help_output_matches_bash(prereq_repo: Path) -> None: bash = _run(_bash_cmd(prereq_repo, "--help"), prereq_repo)