diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3456a92ed..cac8adf5a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -96,6 +96,36 @@ jobs: echo "Required environment variables are set (using CI fallbacks if secrets not available)" uv run pytest tests/ --ignore tests/integration -v --cov=code_puppy --cov-report=term-missing + # The exec-runner UTF-8 pipe contract is a Windows-only failure mode: child + # Pythons attached to a pipe default their stdio to the legacy codepage + # (cp1252) and crash on emoji, while macOS/Linux children default to UTF-8 -- + # so regressions are invisible on the main (macos) test job. The full suite + # has pre-existing, unrelated Windows failures, so this job deliberately + # scopes to the tests that pin the encoding contract instead of gating CI on + # a platform the suite has never run on. Widen it as Windows support grows. + windows-encoding: + runs-on: windows-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup Python 3.13 + uses: actions/setup-python@v5 + with: + python-version: '3.13' + + - name: Install uv + run: pip install uv + + - name: Setup uv virtual environment + run: uv venv + + - name: Install dependencies + run: uv pip install -e . + + - name: Run Windows encoding regression tests + run: uv run pytest tests/plugins/test_customizable_commands_callbacks.py -v --no-cov + quality: runs-on: ubuntu-latest steps: diff --git a/code_puppy/plugins/customizable_commands/register_callbacks.py b/code_puppy/plugins/customizable_commands/register_callbacks.py index 7becdc78c..d9bab7e25 100644 --- a/code_puppy/plugins/customizable_commands/register_callbacks.py +++ b/code_puppy/plugins/customizable_commands/register_callbacks.py @@ -329,6 +329,17 @@ def _run_exec_directive(directive: str, name: str, args: str = "") -> None: one in. Sets ``FORCE_COLOR=1`` so scripts emit ANSI even though stdout is a pipe; :func:`emit_shell_line` then renders the ANSI via Rich's ``Text.from_ansi``. + Also sets ``PYTHONIOENCODING=utf-8`` and decodes the captured output as + UTF-8 explicitly: on Windows a child Python attached to a pipe otherwise + defaults its stdout to the legacy codepage (cp1252) and dies with + ``UnicodeEncodeError`` the moment a script prints an emoji -- and the + parent-side ``text=True`` decode would mojibake for the same reason. + UTF-8 on both ends of the pipe, no exceptions. Deliberately NOT setting + ``PYTHONUTF8=1``: full UTF-8 mode also flips the default ``open()`` + encoding for every Python child of every exec command -- a much broader + semantic change than the stdio bug calls for, and ``PYTHONIOENCODING`` + alone is verified sufficient to prevent the crash (even for older + installed scripts still using Rich's legacy-Windows path). Any extra tokens the user typed after the command (e.g. ``/flux/status todo``) are shell-quoted and appended to the directive so dangerous shell chars in @@ -351,13 +362,20 @@ def _run_exec_directive(directive: str, name: str, args: str = "") -> None: full_cmd = directive + " " + " ".join(shlex.quote(t) for t in tokens) emit_info(f" Running exec for /{name}: {full_cmd}") - env = {**os.environ, "FORCE_COLOR": "1"} + env = { + **os.environ, + "FORCE_COLOR": "1", + # Force UTF-8 stdio in child Pythons; harmless for non-Python tools. + # (Narrowest lever on purpose -- see docstring for why not PYTHONUTF8.) + "PYTHONIOENCODING": "utf-8", + } try: result = subprocess.run( full_cmd, shell=True, capture_output=True, - text=True, + encoding="utf-8", + errors="replace", timeout=_EXEC_TIMEOUT_SECONDS, env=env, ) diff --git a/code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_about.py b/code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_about.py index adbce3368..c993cd6b8 100644 --- a/code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_about.py +++ b/code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_about.py @@ -94,7 +94,23 @@ def resolve_width(cli_width: int | None) -> int: return DEFAULT_WIDTH +def force_utf8_stdout() -> None: + """Reconfigure stdout to UTF-8 so emoji survive legacy Windows codepages. + + Under the exec runner the env already forces UTF-8 (PYTHONIOENCODING); + this covers standalone runs in a cp1252 console, where printing a single + emoji would otherwise raise UnicodeEncodeError. Best-effort by design -- + these scripts must never crash on an IO quirk. (Duplicated across the + flux_* scripts on purpose: each is a standalone, dependency-free file.) + """ + try: + sys.stdout.reconfigure(encoding="utf-8", errors="replace") + except Exception: + pass + + def main(argv: list[str] | None = None) -> int: + force_utf8_stdout() parser = argparse.ArgumentParser(description="Render /flux/about overview") parser.add_argument( "--source", @@ -107,9 +123,7 @@ def main(argv: list[str] | None = None) -> int: default=None, help=f"Render width in cols (default: $COLUMNS or {DEFAULT_WIDTH})", ) - parser.add_argument( - "--no-color", action="store_true", help="Disable ANSI color" - ) + parser.add_argument("--no-color", action="store_true", help="Disable ANSI color") args = parser.parse_args(argv) source = Path(args.source).expanduser() @@ -122,8 +136,13 @@ def main(argv: list[str] | None = None) -> int: # ``force_terminal=True`` makes Rich emit ANSI even though our stdout is a # pipe (the code-puppy exec runner captures it). ``no_color`` honors --no-color. + # ``legacy_windows=False`` keeps Rich off the win32 LegacyWindowsTerm path: + # our stdout is a pipe, not a console handle, so that path both loses the + # ANSI the runner re-renders AND crashes encoding emoji on cp1252. Plain + # ANSI bytes through the pipe are exactly what the runner contract wants. console = Console( force_terminal=True, + legacy_windows=False, color_system=None if args.no_color else "truecolor", width=resolve_width(args.width), no_color=args.no_color, diff --git a/code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_cheatsheet.py b/code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_cheatsheet.py index 8ecd3ea06..b4fee1dea 100644 --- a/code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_cheatsheet.py +++ b/code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_cheatsheet.py @@ -37,7 +37,7 @@ WHITE = "\033[1;37m" GREY = "\033[90m" -FRAKTUR_F = "\U0001D571" # mathematical bold fraktur capital F +FRAKTUR_F = "\U0001d571" # mathematical bold fraktur capital F def _default_docs() -> str: @@ -164,7 +164,24 @@ def render(pipelines: list[tuple[str, str, list[str]]], theme: Theme) -> str: return "\n".join(out) +def force_utf8_stdout() -> None: + """Reconfigure stdout to UTF-8 so emoji survive legacy Windows codepages. + + Under the exec runner the env already forces UTF-8 (PYTHONIOENCODING); + this covers standalone runs in a cp1252 console, where printing a single + emoji or box-drawing glyph would otherwise raise UnicodeEncodeError. + Best-effort by design -- these scripts must never crash on an IO quirk. + (Duplicated across the flux_* scripts on purpose: each is a standalone, + dependency-free file.) + """ + try: + sys.stdout.reconfigure(encoding="utf-8", errors="replace") + except Exception: + pass + + def main(argv: list[str] | None = None) -> int: + force_utf8_stdout() parser = argparse.ArgumentParser(description="Render //flux cheatsheet panel") parser.add_argument("--docs", default=DEFAULT_DOCS, help="flux _docs directory") parser.add_argument( @@ -200,14 +217,18 @@ def main(argv: list[str] | None = None) -> int: # environment even though isatty() is False. flux_about.py documents the # same contract via Rich's force_terminal=True -- keep the FORCE_COLOR branch # so the cheatsheet doesn't silently degrade to monochrome under the runner. - color_on = not args.no_color and (sys.stdout.isatty() or os.environ.get("FORCE_COLOR")) + color_on = not args.no_color and ( + sys.stdout.isatty() or os.environ.get("FORCE_COLOR") + ) theme = Theme(bool(color_on)) if not pipeline_md.is_file(): print(theme(GREY, f"(pipeline doc not found at {pipeline_md})")) return 1 - pipelines = parse_pipelines(pipeline_md.read_text(encoding="utf-8", errors="replace")) + pipelines = parse_pipelines( + pipeline_md.read_text(encoding="utf-8", errors="replace") + ) if args.pipeline: # Already validated + normalized above diff --git a/code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_status.py b/code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_status.py index 2019cafd4..6efff2796 100644 --- a/code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_status.py +++ b/code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_status.py @@ -40,11 +40,11 @@ WHITE = "\033[1;37m" # --- glyphs (escaped so emoji-stripping file writers leave them intact) ----- -FRAKTUR_F = "\U0001D571" # mathematical bold fraktur capital F -ICON_PROGRESS = "\U0001F504" # clockwise arrows -ICON_DONE = "\u2705" # white heavy check mark on green -ICON_REWORK = "\U0001F501" # repeat -DOT = "\u25CF" # solid circle +FRAKTUR_F = "\U0001d571" # mathematical bold fraktur capital F +ICON_PROGRESS = "\U0001f504" # clockwise arrows +ICON_DONE = "\u2705" # white heavy check mark on green +ICON_REWORK = "\U0001f501" # repeat +DOT = "\u25cf" # solid circle STATUS_STYLE = { "in-progress": (ICON_PROGRESS, ORANGE), @@ -65,8 +65,8 @@ # Panel layout constants (named so a column-width tweak can't silently break # the header/rule alignment). -_SECTION_PAD = 18 # extra width added to name_w + stage_w for section rules -_MIN_PANEL_W = 48 # floor so short content still frames nicely +_SECTION_PAD = 18 # extra width added to name_w + stage_w for section rules +_MIN_PANEL_W = 48 # floor so short content still frames nicely class Theme: @@ -256,9 +256,7 @@ def render(base: Path, sections: set[str], theme: Theme) -> str: # every severity column), not the generic section total_w -- otherwise # the rule is short by a few cols and drifts whenever SEV_COL_WIDTH # changes. - review_w = max( - name_w + sum(SEV_COL_WIDTH[s] for s in SEVERITIES), _MIN_PANEL_W - ) + review_w = max(name_w + sum(SEV_COL_WIDTH[s] for s in SEVERITIES), _MIN_PANEL_W) lines.append(rule(review_w, "\u2500", GREY, theme)) for name, sev in reviews: row = theme(CYAN, name.ljust(name_w)) @@ -274,7 +272,24 @@ def render(base: Path, sections: set[str], theme: Theme) -> str: return "\n".join(lines) +def force_utf8_stdout() -> None: + """Reconfigure stdout to UTF-8 so emoji survive legacy Windows codepages. + + Under the exec runner the env already forces UTF-8 (PYTHONIOENCODING); + this covers standalone runs in a cp1252 console, where printing a single + emoji or box-drawing glyph would otherwise raise UnicodeEncodeError. + Best-effort by design -- these scripts must never crash on an IO quirk. + (Duplicated across the flux_* scripts on purpose: each is a standalone, + dependency-free file.) + """ + try: + sys.stdout.reconfigure(encoding="utf-8", errors="replace") + except Exception: + pass + + def main(argv: list[str] | None = None) -> int: + force_utf8_stdout() parser = argparse.ArgumentParser(description="Render /flux/status panel") parser.add_argument("--base", default=None, help="Explicit flux base dir") parser.add_argument( @@ -318,7 +333,9 @@ def main(argv: list[str] | None = None) -> int: # environment even though isatty() is False. flux_about.py documents the # same contract via Rich's force_terminal=True -- keep the FORCE_COLOR branch # so colorized panels don't silently degrade to monochrome under the runner. - color_on = not args.no_color and (sys.stdout.isatty() or os.environ.get("FORCE_COLOR")) + color_on = not args.no_color and ( + sys.stdout.isatty() or os.environ.get("FORCE_COLOR") + ) theme = Theme(bool(color_on)) if not base.exists(): diff --git a/tests/plugins/test_customizable_commands_callbacks.py b/tests/plugins/test_customizable_commands_callbacks.py index cafd625e7..54045bf32 100644 --- a/tests/plugins/test_customizable_commands_callbacks.py +++ b/tests/plugins/test_customizable_commands_callbacks.py @@ -683,6 +683,8 @@ def test_python_token_expands_to_sys_executable(self): assert "{python}" not in expanded def test_script_and_command_tokens_resolve_under_config_dir(self): + import os + from code_puppy.plugins.customizable_commands import register_callbacks as rc with patch.object(rc, "CONFIG_DIR", "/tmp/cfghome"): @@ -690,22 +692,29 @@ def test_script_and_command_tokens_resolve_under_config_dir(self): "{python} {script:flux_status.py} --docs {command:flux/_docs}" ) - assert "/tmp/cfghome/scripts/flux_status.py" in expanded - assert "/tmp/cfghome/commands/flux/_docs" in expanded + # os.path.join yields the platform separator (backslash on Windows), + # so build the expected paths the same way instead of hard-coding '/'. + assert os.path.join("/tmp/cfghome", "scripts", "flux_status.py") in expanded + assert os.path.join("/tmp/cfghome", "commands", "flux", "_docs") in expanded # No shell-expansion / hard-coded home path residue. assert "~" not in expanded assert "{script:" not in expanded assert "{command:" not in expanded def test_path_with_spaces_is_quoted(self): + import os + from code_puppy.plugins.customizable_commands import register_callbacks as rc with patch.object(rc, "CONFIG_DIR", "/tmp/has space/cfg"): expanded = rc._expand_exec_tokens("{python} {script:flux_status.py}") # A space-bearing path must be quoted so shell=True doesn't split it - # into two args. shlex.quote wraps the whole token in single quotes. - assert "'/tmp/has space/cfg/scripts/flux_status.py'" in expanded + # into two args. POSIX (shlex.quote) wraps in single quotes; Windows + # (list2cmdline, for cmd.exe) wraps in double quotes. + path = os.path.join("/tmp/has space/cfg", "scripts", "flux_status.py") + quote = '"' if os.name == "nt" else "'" + assert f"{quote}{path}{quote}" in expanded def test_unknown_tokens_are_left_untouched(self): from code_puppy.plugins.customizable_commands.register_callbacks import ( @@ -713,3 +722,68 @@ def test_unknown_tokens_are_left_untouched(self): ) assert _expand_exec_tokens("echo {not_a_token}") == "echo {not_a_token}" + + +class TestExecUtf8Contract: + """The exec runner must speak UTF-8 on both ends of the pipe. + + Regression tests for the Windows cp1252 crash: a child Python attached to + a pipe defaults its stdout to the legacy codepage and raises + UnicodeEncodeError the moment a flux script prints an emoji; the parent's + locale-default decode mojibakes for the same reason. The runner therefore + injects PYTHONIOENCODING (the narrowest sufficient lever -- deliberately + NOT PYTHONUTF8, which would also flip default open() encoding for every + child) and decodes explicitly as UTF-8. + """ + + def test_runner_injects_utf8_env_and_decodes_utf8(self): + import os + + from unittest.mock import MagicMock + + from code_puppy.plugins.customizable_commands import register_callbacks as rc + + fake_result = MagicMock(stdout="ok", stderr="", returncode=0) + with patch.object(rc.subprocess, "run", return_value=fake_result) as mock_run: + rc._run_exec_directive("echo ok", "pinger") + + kwargs = mock_run.call_args.kwargs + assert kwargs["encoding"] == "utf-8" + assert kwargs["errors"] == "replace" + env = kwargs["env"] + assert env["PYTHONIOENCODING"] == "utf-8" + assert env["FORCE_COLOR"] == "1" + # Deliberately NOT injected: PYTHONUTF8 flips full UTF-8 mode (default + # open() encoding etc.) for every Python child of every exec command -- + # far broader than the stdio fix requires. Pin its absence so it can't + # sneak back in without a conscious decision. + assert "PYTHONUTF8" not in env or env["PYTHONUTF8"] == os.environ.get( + "PYTHONUTF8" + ) + + def test_child_python_emoji_survives_the_pipe(self, tmp_path): + """End-to-end: a real child Python printing an emoji must not crash. + + Without the env injection this reproduces the /flux/about failure on + Windows (cp1252 child stdout). The emoji must arrive intact and the + script must exit 0 (no 'exited with code 1' warning). + """ + import sys + + from code_puppy.plugins.customizable_commands import register_callbacks as rc + + script = tmp_path / "emoji.py" + script.write_text("print('\\U0001F300 flux')\n", encoding="utf-8") + directive = f"{rc._shell_quote(sys.executable)} {rc._shell_quote(str(script))}" + + lines = [] + with ( + patch.object( + rc, "emit_shell_line", side_effect=lambda line, **kw: lines.append(line) + ), + patch.object(rc, "emit_warning") as mock_warn, + ): + rc._run_exec_directive(directive, "emoji-test") + + assert any("\U0001f300 flux" in line for line in lines) + mock_warn.assert_not_called()