diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index b0d69604..25ab1253 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -3360,6 +3360,38 @@ depend on the active console codepage. string-compares raw stdout bytes on Windows might. - **Captured or replaced streams fall back to the plain text write** (there is no binary buffer to bypass), so in-process test harnesses behave as before. +- Human (Rich) output got the same guarantee later, by a different mechanism -- + see the next entry. + +## Redirected human output is UTF-8 too, and no longer crashes (since v0.80.1) + +When stdout is **not** a terminal, kbagent reconfigures it to UTF-8. So every +byte you capture from kbagent -- machine or human, `--json` or Rich table -- is +UTF-8 on every platform. + +- **What this fixes.** Before v0.80.1, `kbagent semantic-layer --help` and + `kbagent context` exited **1** with `UnicodeEncodeError` on Windows whenever + their output was piped or redirected, and any Rich table truncated by width + emitted a lone `0x85` byte for its ellipsis. An agent shelling out and + capturing stdout got a crash or undecodable bytes; the same command typed + into a terminal worked fine, which made it look unreproducible. +- **The split is terminal vs not, not which codepage is set.** Since PEP 528, + CPython writes to a real Windows console through the console API and already + reports `utf-8`, so interactive sessions were never affected. Only the + redirected path fell back to the locale encoding (cp1252 / cp1250). Setting + `chcp` changes nothing once stdout is a pipe. +- **Terminals are deliberately left untouched.** Forcing UTF-8 bytes at a + console whose codepage is cp852 would replace a working display with + mojibake. Do not "fix" this by forcing UTF-8 everywhere. +- **Always decode kbagent output as UTF-8**, never with + `locale.getpreferredencoding()`. Same rule the `--json` entry above gives, now + true for human output as well. +- **Redirected Windows output now matches POSIX byte for byte**, including the + box-drawing characters Rich previously downgraded to ASCII when it thought the + stream could not encode them. A test that string-compares captured human + output across platforms will see them agree where they used to differ. +- **On kbagent older than v0.80.1**, the workaround is `PYTHONUTF8=1` or + `PYTHONIOENCODING=utf-8` in the environment you spawn kbagent from. ## Two commands were broken on Windows until v0.80.1 (since v0.80.1) diff --git a/src/keboola_agent_cli/changelog.py b/src/keboola_agent_cli/changelog.py index 2f642072..af79ccaa 100644 --- a/src/keboola_agent_cli/changelog.py +++ b/src/keboola_agent_cli/changelog.py @@ -72,6 +72,20 @@ "the Windows job; the rest are mostly POSIX-only permission assertions " "(`os.chmod` cannot narrow an ACL) and unguarded `fcntl` imports, which " "need skips before the whole suite can be gated on Windows.", + "Fix: redirected `kbagent` output no longer depends on the machine's codepage " + "on Windows. `kbagent semantic-layer --help` and `kbagent context` exited 1 " + "with `UnicodeEncodeError` the moment their output was piped or redirected, " + "and any Rich table truncated by width emitted a lone `0x85` for its ellipsis. " + "Since PEP 528 a real Windows console is written through the console API and " + "already reports `utf-8`, so an interactive session was never affected -- but " + "a pipe or a file falls back to the locale encoding (cp1252 on a Czech box), " + "which cannot represent an arrow, an em dash, or a box-drawing glyph. Whenever " + "stdout/stderr is NOT a terminal, kbagent now reconfigures it to UTF-8; " + "terminals are deliberately left untouched, because forcing UTF-8 bytes at a " + "cp852 console would replace a working display with mojibake. Redirected " + "Windows output now matches POSIX byte for byte. Issue #546 fixed this class " + "for `--json` by writing to `sys.stdout.buffer`; this covers the human/Rich " + "path, which cannot bypass the encoder because Rich owns the writes.", ], "0.80.0": [ "New: `kbagent auth login|status|logout` -- browser-based programmatic authentication as " diff --git a/src/keboola_agent_cli/cli.py b/src/keboola_agent_cli/cli.py index 54c30c65..94cb6a71 100644 --- a/src/keboola_agent_cli/cli.py +++ b/src/keboola_agent_cli/cli.py @@ -46,7 +46,7 @@ from .constants import EXIT_PERMISSION_DENIED from .errors import ErrorCode, PermissionDeniedError from .models import PermissionPolicy -from .output import OutputFormatter +from .output import OutputFormatter, force_utf8_when_redirected from .permissions import PermissionEngine from .services.agent_service import AgentService from .services.auth_service import AuthService @@ -83,6 +83,10 @@ from .services.version_service import VersionService from .services.workspace_service import WorkspaceService +# At import, not inside the root callback: Click renders `--help` while parsing, +# before any callback runs, and `--help` is one of the surfaces that crashed. +force_utf8_when_redirected() + app = typer.Typer( name="kbagent", help="Keboola Agent CLI -- AI-friendly interface to Keboola projects", diff --git a/src/keboola_agent_cli/output.py b/src/keboola_agent_cli/output.py index 3c4b925b..9e5d5c58 100644 --- a/src/keboola_agent_cli/output.py +++ b/src/keboola_agent_cli/output.py @@ -14,6 +14,58 @@ from .models import ErrorResponse, SuccessResponse +def force_utf8_when_redirected() -> None: + """Make redirected stdout/stderr UTF-8, so output never depends on a codepage. + + Windows only in practice, and only when the stream is **not** a terminal -- + which is exactly the split that makes this safe. Since PEP 528, CPython + writes to a real Windows console through the console API, so an interactive + ``kbagent`` already reports ``encoding=utf-8`` and renders anything. The + moment stdout is a pipe or a file, that path is gone and Python falls back + to the locale encoding (cp1252 on a Czech machine), which cannot represent + most of what Rich emits. Measured on Windows 11: + + ================== ============ ================== + stdout ``encoding`` ``"\\u2194"`` + ================== ============ ================== + console ``utf-8`` encodes + pipe / file ``cp1252`` ``UnicodeEncodeError`` + ================== ============ ================== + + So ``kbagent semantic-layer --help`` and ``kbagent context`` crash with exit + 1 the moment their output is piped or redirected, and tables truncated by + Rich emit a lone ``0x85`` for their ellipsis. That is the CLI's primary + audience: scripts, CI, and AI agents capturing output. + + Issue #546 fixed the same class of bug for ``--json`` by writing bytes + straight to ``sys.stdout.buffer`` (see :func:`write_machine_output`); this + covers the human/Rich path, which cannot bypass the encoder because Rich + owns the writes. + + Interactive terminals are deliberately left untouched -- they already work, + and forcing UTF-8 bytes at a console whose codepage is cp852 would turn a + working display into mojibake. Best effort throughout: a replaced or + captured stream that cannot be reconfigured is simply left as it is. + """ + for stream in (sys.stdout, sys.stderr): + reconfigure = getattr(stream, "reconfigure", None) + if reconfigure is None: + continue + try: + if stream.isatty(): + continue + if (stream.encoding or "").replace("-", "").replace("_", "").lower() == "utf8": + continue + # Preserve the stream's existing error handler: the default is + # `surrogateescape`, which is what round-trips undecodable bytes + # from filenames back out unchanged. + reconfigure(encoding="utf-8", errors=stream.errors or "strict") + except (AttributeError, OSError, ValueError): + # Not reconfigurable (pytest capture, a custom stream). The caller + # is no worse off than before. + continue + + def write_machine_output(text: str) -> None: """Write a machine-readable line to stdout as UTF-8, whatever the console is. diff --git a/tests/test_output.py b/tests/test_output.py index 4fff24f4..47a9fd02 100644 --- a/tests/test_output.py +++ b/tests/test_output.py @@ -15,6 +15,7 @@ OutputFormatter, _format_duration, _seconds_to_human, + force_utf8_when_redirected, format_config_detail, format_configs_table, format_doctor_panel, @@ -1217,3 +1218,79 @@ def test_each_event_is_flushed_so_consumers_see_it_immediately( # Readable without an explicit flush by the test. assert stdout.raw.getvalue().endswith(b"\n") + + +class TestForceUtf8WhenRedirected: + """Redirected output must not depend on the machine's codepage. + + Since PEP 528 a real Windows console already reports ``utf-8`` and renders + anything; the moment stdout becomes a pipe or a file, Python falls back to + the locale encoding (cp1252 on a Czech box) and Rich's output -- an arrow in + a docstring, a truncation ellipsis -- raises ``UnicodeEncodeError``. Only + the redirected half is touched, so interactive terminals cannot regress. + """ + + class _Stream: + """Minimal stand-in exposing the parts of TextIOWrapper we rely on.""" + + def __init__(self, *, encoding: str, tty: bool, errors: str = "surrogateescape") -> None: + self.encoding = encoding + self.errors = errors + self._tty = tty + self.reconfigured: list[dict[str, str]] = [] + + def isatty(self) -> bool: + return self._tty + + def reconfigure(self, *, encoding: str, errors: str) -> None: + self.reconfigured.append({"encoding": encoding, "errors": errors}) + self.encoding = encoding + self.errors = errors + + def _run(self, monkeypatch: pytest.MonkeyPatch, out: object, err: object) -> None: + monkeypatch.setattr(sys, "stdout", out) + monkeypatch.setattr(sys, "stderr", err) + force_utf8_when_redirected() + + def test_redirected_non_utf8_stream_is_switched_to_utf8( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + out = self._Stream(encoding="cp1252", tty=False) + err = self._Stream(encoding="cp1252", tty=False) + self._run(monkeypatch, out, err) + assert out.reconfigured == [{"encoding": "utf-8", "errors": "surrogateescape"}] + assert err.reconfigured == [{"encoding": "utf-8", "errors": "surrogateescape"}] + + def test_a_terminal_is_left_alone(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Forcing UTF-8 bytes at a cp852 console would create mojibake, not fix it.""" + out = self._Stream(encoding="cp852", tty=True) + self._run(monkeypatch, out, out) + assert out.reconfigured == [] + + @pytest.mark.parametrize("spelling", ["utf-8", "UTF-8", "utf8"]) + def test_already_utf8_is_not_touched( + self, spelling: str, monkeypatch: pytest.MonkeyPatch + ) -> None: + """POSIX, and a real Windows console, land here -- nothing to do.""" + out = self._Stream(encoding=spelling, tty=False) + self._run(monkeypatch, out, out) + assert out.reconfigured == [] + + def test_error_handler_is_preserved(self, monkeypatch: pytest.MonkeyPatch) -> None: + """`surrogateescape` is what round-trips undecodable filename bytes.""" + out = self._Stream(encoding="cp1252", tty=False, errors="backslashreplace") + self._run(monkeypatch, out, out) + assert out.reconfigured[0]["errors"] == "backslashreplace" + + def test_stream_without_reconfigure_is_ignored(self, monkeypatch: pytest.MonkeyPatch) -> None: + """pytest's capture objects and StringIO have no `reconfigure`.""" + self._run(monkeypatch, StringIO(), StringIO()) # must not raise + + def test_a_stream_that_refuses_is_survived(self, monkeypatch: pytest.MonkeyPatch) -> None: + """A detached or already-closed stream must not take the CLI down.""" + + class Hostile(TestForceUtf8WhenRedirected._Stream): + def reconfigure(self, *, encoding: str, errors: str) -> None: + raise OSError("stream is detached") + + self._run(monkeypatch, Hostile(encoding="cp1252", tty=False), StringIO())