Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
22 changes: 20 additions & 2 deletions code_puppy/plugins/customizable_commands/register_callbacks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
)
Expand Down
25 changes: 22 additions & 3 deletions code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_about.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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()
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down
39 changes: 28 additions & 11 deletions code_puppy/plugins/flux_bootstrap/bundled/scripts/flux_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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:
Expand Down Expand Up @@ -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))
Expand All @@ -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(
Expand Down Expand Up @@ -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():
Expand Down
82 changes: 78 additions & 4 deletions tests/plugins/test_customizable_commands_callbacks.py
Original file line number Diff line number Diff line change
Expand Up @@ -683,33 +683,107 @@ 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"):
expanded = rc._expand_exec_tokens(
"{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 (
_expand_exec_tokens,
)

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()
Loading