Skip to content

fix(scripts): stop check-prerequisites text mode crashing on a legacy stdout code page - #3890

Open
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/check-prereq-ascii-fallback
Open

fix(scripts): stop check-prerequisites text mode crashing on a legacy stdout code page#3890
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/check-prereq-ascii-fallback

Conversation

@jawwad-ali

Copy link
Copy Markdown
Contributor

Problem

_check_file / _check_dir in scripts/python/check_prerequisites.py hard-code the U+2713 / U+2717 glyphs and print() them to sys.stdout:

marker = "✓" if path.is_file() else "✗"
print(f"  {marker} {description}")

On Windows sys.stdout falls back to the ANSI code page whenever stdout is not a console — a pipe or a file redirect, which is exactly how an agent or a workflow step invokes these scripts. U+2713 is unencodable in cp1252.

Reproduction on current main (81bf741)

stdout encoding: cp1252
UnicodeEncodeError: 'charmap' codec can't encode character '✓' in position 2

Text mode therefore aborts right after printing AVAILABLE_DOCS:, so the caller gets a header with no document lines under it — and a non-zero exit for a repository that is perfectly healthy.

Fix

Downgrade to ASCII only when stdout cannot encode the glyph, so a UTF-8 console is unaffected:

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

[OK] / [FAIL] is not invented — it is the ASCII rendering these markers already have in this repo:

Site Code
scripts/powershell/common.ps1:243 Write-Output " [OK] $Description"
scripts/powershell/common.ps1:246 Write-Output " [FAIL] $Description"
tests/parity_helpers.py:130-131 text.replace(" ✓ ", " [OK] ").replace(" ✗ ", " [FAIL] ")

The PowerShell twin emits the ASCII form natively, and the parity helper maps the glyphs onto it — so the test suite already treats the two forms as equivalent output. Nothing downstream distinguishes them.

Breaking risk: a UTF-8-capable stdout still gets the glyphs, byte-identical to today. The only case that changes is one that previously raised and truncated the report. Callers parsing the marker are already required to accept both forms, per normalize_status_text.

Scope note

scripts/python/setup_tasks.py:58-65 carries a byte-identical block and crashes the same way. I kept this PR to one script — happy to extend it to the twin here or as a follow-up, whichever you prefer.

Verification

  • Fail-before / pass-after: the new test (running the real script with PYTHONIOENCODING=cp1252) fails on unpatched src and passes with the fix. File: 1 failed → 12 passed, 8 skipped.
  • Scoped regression: failure set identical to the clean-main baseline captured on 81bf741 (0 pre-existing in scope).
  • uvx ruff@0.15.0 check src tests → clean

Note this test file is also touched by my open #3785 (a comment-only change to scripts/python/common.py), so expect a trivial append conflict if that merges first — happy to rebase.


Written with assistance from Claude Code. Bug found, reproduced, and verified by me on current main.

… 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) <noreply@anthropic.com>
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.

1 participant