From 05905560fad31eea2597ea8aaa2bf8135b05f668 Mon Sep 17 00:00:00 2001 From: Petr Date: Sun, 2 Aug 2026 09:48:51 +0200 Subject: [PATCH 1/2] fix(output): write --json as UTF-8 regardless of the console codepage (#546) `kbagent --json flow list` crashed with UnicodeEncodeError on a default Czech/Polish/Hungarian Windows console (cp1250) whenever the data held a non-ASCII character -- an arrow in a flow name was enough. `--json` exists to be piped into another program, so its bytes must not depend on the terminal's active codepage. The three JSON writers now go through `write_machine_output`, which writes UTF-8 through `sys.stdout.buffer` and so bypasses the text layer's encoder entirely. The text layer is flushed first so anything written through it keeps its place; a stdout with no binary buffer (captured or replaced streams) has no encoder to bypass and takes the plain write. Only the two pydantic paths actually crashed: `model_dump_json` emits raw non-ASCII, whereas `json.dumps` escapes it to \uXXXX under its ensure_ascii default. The error envelope is routed through the same helper anyway, and its test says plainly that it pins the invariant rather than reproducing the bug. The regression tests simulate the cp1250 encoder, so they run on every platform; the two covering the crash were confirmed to fail without this change. Side effect on Windows: JSON lines now end LF rather than CRLF, since the binary buffer does no newline translation. Machine output is the better place for that, and no parser cares. --- src/keboola_agent_cli/output.py | 33 ++++++++++++-- tests/test_output.py | 81 +++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 3 deletions(-) diff --git a/src/keboola_agent_cli/output.py b/src/keboola_agent_cli/output.py index 8f5b1581..3c4b925b 100644 --- a/src/keboola_agent_cli/output.py +++ b/src/keboola_agent_cli/output.py @@ -14,6 +14,33 @@ from .models import ErrorResponse, SuccessResponse +def write_machine_output(text: str) -> None: + """Write a machine-readable line to stdout as UTF-8, whatever the console is. + + ``--json`` exists to be piped into another program, so its bytes must not + depend on the terminal's active codepage. On a default Czech / Polish / + Hungarian Windows console (cp1250), ``sys.stdout.write`` raised + ``UnicodeEncodeError`` on any non-ASCII character in the payload -- an arrow + in a flow name was enough to make ``kbagent --json flow list`` unusable + (issue #546). Pydantic's ``model_dump_json`` emits raw non-ASCII, unlike + ``json.dumps``, whose ``ensure_ascii`` default escapes it away. + + Writing through ``sys.stdout.buffer`` bypasses the text layer's encoder + entirely, so the codepage never gets a say. The text layer is flushed first + so anything already written through it keeps its place in the stream. A + stdout with no binary buffer (captured or replaced streams) has no encoder + to bypass, so the plain write is already correct there. + """ + payload = text + "\n" + buffer = getattr(sys.stdout, "buffer", None) + if buffer is None: + sys.stdout.write(payload) + return + sys.stdout.flush() + buffer.write(payload.encode("utf-8")) + buffer.flush() + + class OutputFormatter: """Formats CLI output as either JSON (for machines/agents) or Rich (for humans). @@ -54,7 +81,7 @@ def output( """ if self.json_mode: response = SuccessResponse(status="ok", data=data) - sys.stdout.write(response.model_dump_json(indent=2) + "\n") + write_machine_output(response.model_dump_json(indent=2)) else: if human_formatter is not None: human_formatter(self.console, data) @@ -99,7 +126,7 @@ def error( "status": "error", "error": err.model_dump(exclude_none=True), } - sys.stdout.write(json.dumps(error_envelope, indent=2) + "\n") + write_machine_output(json.dumps(error_envelope, indent=2)) else: self.err_console.print(f"[bold red]Error:[/bold red] {message}") @@ -111,7 +138,7 @@ def success(self, message: str) -> None: """ if self.json_mode: response = SuccessResponse(status="ok", data={"message": message}) - sys.stdout.write(response.model_dump_json(indent=2) + "\n") + write_machine_output(response.model_dump_json(indent=2)) else: self.console.print(f"[bold green]Success:[/bold green] {message}") diff --git a/tests/test_output.py b/tests/test_output.py index 923b5cf6..9464d0c1 100644 --- a/tests/test_output.py +++ b/tests/test_output.py @@ -1,5 +1,6 @@ """Tests for OutputFormatter with JSON and Rich dual mode.""" +import io import json import sys from io import StringIO @@ -20,6 +21,7 @@ format_query_results, format_tool_result, format_tools_table, + write_machine_output, ) @@ -1074,3 +1076,82 @@ def test_falls_back_to_csv_preview_for_full_export(self) -> None: assert "Results:" in output assert "id,name" in output assert "alice" in output + + +class TestMachineOutputIsAlwaysUtf8: + """`--json` must not depend on the console codepage (issue #546). + + On a default Czech/Polish/Hungarian Windows console (cp1250) a single + non-ASCII character in the data -- an arrow in a flow name was the real + report -- made `kbagent --json flow list` raise UnicodeEncodeError instead + of printing JSON. The codepage is simulated here, so these run everywhere. + + Only the two pydantic paths reproduce the crash: `model_dump_json` emits + raw non-ASCII, whereas `json.dumps` escapes it to `\\uXXXX` under its + `ensure_ascii` default. The `error()` test below therefore pins the + invariant rather than the bug -- it is what fails if anyone later turns + `ensure_ascii` off. + """ + + ARROW_NAME = "extract → transform" + + @staticmethod + def _cp1250_stdout() -> io.TextIOWrapper: + """A stdout whose text layer cannot encode the payload, like cp1250.""" + return io.TextIOWrapper(io.BytesIO(), encoding="cp1250", newline="") + + def _capture(self, monkeypatch, action) -> bytes: + stream = self._cp1250_stdout() + monkeypatch.setattr(sys, "stdout", stream) + action() + stream.flush() + return cast(io.BytesIO, stream.buffer).getvalue() + + def test_output_survives_a_codepage_that_cannot_encode_the_data(self, monkeypatch) -> None: + formatter = OutputFormatter(json_mode=True) + + written = self._capture(monkeypatch, lambda: formatter.output({"name": self.ARROW_NAME})) + + # Bytes are UTF-8 and round-trip, rather than being lost or escaped. + assert json.loads(written.decode("utf-8"))["data"]["name"] == self.ARROW_NAME + + def test_success_survives_it_too(self, monkeypatch) -> None: + formatter = OutputFormatter(json_mode=True) + + written = self._capture(monkeypatch, lambda: formatter.success(self.ARROW_NAME)) + + assert json.loads(written.decode("utf-8"))["data"]["message"] == self.ARROW_NAME + + def test_error_survives_it_too(self, monkeypatch) -> None: + """Passes pre-fix too -- `json.dumps` escapes the arrow away. + + Kept as the guard on the invariant: it starts failing the moment the + error envelope stops escaping non-ASCII. + """ + formatter = OutputFormatter(json_mode=True) + + written = self._capture( + monkeypatch, lambda: formatter.error(self.ARROW_NAME, error_code="ERROR") + ) + + assert self.ARROW_NAME in json.loads(written.decode("utf-8"))["error"]["message"] + + def test_a_text_stream_without_a_binary_buffer_still_works(self, monkeypatch) -> None: + """Captured/replaced stdout has no encoder to bypass -- write plainly.""" + stream = StringIO() + monkeypatch.setattr(sys, "stdout", stream) + + write_machine_output('{"ok": true}') + + assert stream.getvalue() == '{"ok": true}\n' + + def test_human_mode_output_written_earlier_keeps_its_place(self, monkeypatch) -> None: + """Flushing the text layer first stops the two writers reordering.""" + stream = self._cp1250_stdout() + monkeypatch.setattr(sys, "stdout", stream) + + sys.stdout.write("first\n") + write_machine_output("second") + stream.flush() + + assert cast(io.BytesIO, stream.buffer).getvalue() == b"first\nsecond\n" From 88ccb5c062c657f5c00bc666963164e9af5cce65 Mon Sep 17 00:00:00 2001 From: Petr Date: Sun, 2 Aug 2026 10:00:31 +0200 Subject: [PATCH 2/2] fix(output): route the streamed agent NDJSON through the UTF-8 writer too (#546) Review catch: the fix covered the three OutputFormatter call sites but not `_render_stream_event`, which builds its own NDJSON line and wrote it straight to stdout. It uses `json.dumps(..., ensure_ascii=False)` -- deliberately, so event text stays readable -- which is exactly the property that crashes on cp1250. `kbagent --json agent run --stream` was therefore still broken on a non-UTF-8 Windows console. `kbagent http`'s `_print_json` moves to the same helper. That one uses the `ensure_ascii` default and so cannot crash today; it is routed for the invariant, and its docstring says which of the two it is rather than implying it was a bug. Both new tests confirmed load-bearing by reverting the change: the non-ASCII one fails with the reporter's `UnicodeEncodeError`. Also adds the type hints CONTRIBUTING requires to the tests added in the previous commit. --- src/keboola_agent_cli/commands/agent.py | 9 ++- src/keboola_agent_cli/commands/http_client.py | 13 ++-- tests/test_output.py | 61 +++++++++++++++++-- 3 files changed, 70 insertions(+), 13 deletions(-) diff --git a/src/keboola_agent_cli/commands/agent.py b/src/keboola_agent_cli/commands/agent.py index 9e7799ba..752181e2 100644 --- a/src/keboola_agent_cli/commands/agent.py +++ b/src/keboola_agent_cli/commands/agent.py @@ -23,7 +23,7 @@ import typer from ..errors import ConfigError, ErrorCode -from ..output import OutputFormatter +from ..output import OutputFormatter, write_machine_output from ..server.agents_store import AgentAction, Trigger from ..services.agent_service import AgentService from ._helpers import check_cli_permission, get_formatter, get_service @@ -394,8 +394,11 @@ def _render_stream_event(formatter: OutputFormatter, evt: dict[str, Any]) -> Non multi-line summary with exit code, elapsed time, response preview. """ if formatter.json_mode: - sys.stdout.write(json.dumps(evt, ensure_ascii=False) + "\n") - sys.stdout.flush() + # `ensure_ascii=False` keeps the event text readable, which means raw + # non-ASCII reaches stdout -- so it must not go through the console's + # encoder (issue #546). `write_machine_output` also flushes, which a + # stream consumer depends on. + write_machine_output(json.dumps(evt, ensure_ascii=False)) return event = evt.get("event", "?") data = evt.get("data") or {} diff --git a/src/keboola_agent_cli/commands/http_client.py b/src/keboola_agent_cli/commands/http_client.py index a2687a96..83a4ea0d 100644 --- a/src/keboola_agent_cli/commands/http_client.py +++ b/src/keboola_agent_cli/commands/http_client.py @@ -16,12 +16,12 @@ from __future__ import annotations import json -import sys from typing import Any import typer from ..constants import HTTP_DEFAULT_TIMEOUT +from ..output import write_machine_output from ..services.http_forwarder_service import ( ForwardedResponse, ForwarderError, @@ -41,15 +41,20 @@ def _print_json(_console: Any, data: Any) -> None: """Human-mode renderer: pipe-safe pretty JSON. - Uses ``sys.stdout.write`` instead of ``console.print`` because Rich - can soft-wrap long strings or escape markup, which breaks downstream + Writes straight to stdout instead of ``console.print`` because Rich can + soft-wrap long strings or escape markup, which breaks downstream ``json.loads`` consumers. Real-world case: an AI agent piped ``kbagent http get /openapi.json`` into ``python3 -c "json.load(sys.stdin)"`` and hit ``JSONDecodeError`` because Rich had reflowed lines. The output of ``kbagent http`` is virtually always parsed by something downstream (an LLM, a script, jq) -- so machine-clean stdout is the contract. + + Through :func:`write_machine_output` so that contract also survives a + non-UTF-8 console (issue #546). ``json.dumps`` escapes non-ASCII under its + ``ensure_ascii`` default, so this path cannot crash today; it uses the + helper so it stays correct if that default is ever dropped. """ - sys.stdout.write(json.dumps(data, indent=2) + "\n") + write_machine_output(json.dumps(data, indent=2)) def _resolve_service(ctx: typer.Context) -> HttpForwarderService: diff --git a/tests/test_output.py b/tests/test_output.py index 9464d0c1..9b8102ad 100644 --- a/tests/test_output.py +++ b/tests/test_output.py @@ -3,9 +3,11 @@ import io import json import sys +from collections.abc import Callable from io import StringIO from typing import cast +import pytest from rich.console import Console from keboola_agent_cli.output import ( @@ -1100,14 +1102,16 @@ def _cp1250_stdout() -> io.TextIOWrapper: """A stdout whose text layer cannot encode the payload, like cp1250.""" return io.TextIOWrapper(io.BytesIO(), encoding="cp1250", newline="") - def _capture(self, monkeypatch, action) -> bytes: + def _capture(self, monkeypatch: pytest.MonkeyPatch, action: Callable[[], None]) -> bytes: stream = self._cp1250_stdout() monkeypatch.setattr(sys, "stdout", stream) action() stream.flush() return cast(io.BytesIO, stream.buffer).getvalue() - def test_output_survives_a_codepage_that_cannot_encode_the_data(self, monkeypatch) -> None: + def test_output_survives_a_codepage_that_cannot_encode_the_data( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: formatter = OutputFormatter(json_mode=True) written = self._capture(monkeypatch, lambda: formatter.output({"name": self.ARROW_NAME})) @@ -1115,14 +1119,14 @@ def test_output_survives_a_codepage_that_cannot_encode_the_data(self, monkeypatc # Bytes are UTF-8 and round-trip, rather than being lost or escaped. assert json.loads(written.decode("utf-8"))["data"]["name"] == self.ARROW_NAME - def test_success_survives_it_too(self, monkeypatch) -> None: + def test_success_survives_it_too(self, monkeypatch: pytest.MonkeyPatch) -> None: formatter = OutputFormatter(json_mode=True) written = self._capture(monkeypatch, lambda: formatter.success(self.ARROW_NAME)) assert json.loads(written.decode("utf-8"))["data"]["message"] == self.ARROW_NAME - def test_error_survives_it_too(self, monkeypatch) -> None: + def test_error_survives_it_too(self, monkeypatch: pytest.MonkeyPatch) -> None: """Passes pre-fix too -- `json.dumps` escapes the arrow away. Kept as the guard on the invariant: it starts failing the moment the @@ -1136,7 +1140,9 @@ def test_error_survives_it_too(self, monkeypatch) -> None: assert self.ARROW_NAME in json.loads(written.decode("utf-8"))["error"]["message"] - def test_a_text_stream_without_a_binary_buffer_still_works(self, monkeypatch) -> None: + def test_a_text_stream_without_a_binary_buffer_still_works( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: """Captured/replaced stdout has no encoder to bypass -- write plainly.""" stream = StringIO() monkeypatch.setattr(sys, "stdout", stream) @@ -1145,7 +1151,9 @@ def test_a_text_stream_without_a_binary_buffer_still_works(self, monkeypatch) -> assert stream.getvalue() == '{"ok": true}\n' - def test_human_mode_output_written_earlier_keeps_its_place(self, monkeypatch) -> None: + def test_human_mode_output_written_earlier_keeps_its_place( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: """Flushing the text layer first stops the two writers reordering.""" stream = self._cp1250_stdout() monkeypatch.setattr(sys, "stdout", stream) @@ -1155,3 +1163,44 @@ def test_human_mode_output_written_earlier_keeps_its_place(self, monkeypatch) -> stream.flush() assert cast(io.BytesIO, stream.buffer).getvalue() == b"first\nsecond\n" + + +class TestStreamedAgentEventsAreUtf8: + """`--json ... --stream` writes NDJSON of its own, outside OutputFormatter. + + `_render_stream_event` builds each line with `json.dumps(..., ensure_ascii + =False)` -- deliberately, so event text stays readable -- which means raw + non-ASCII reaches stdout. That is precisely the property that crashes on a + cp1250 console, so this path needs the same UTF-8 write as the formatter + (issue #546). + """ + + def test_event_with_non_ascii_does_not_crash_on_a_cp1250_console( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + from keboola_agent_cli.commands.agent import _render_stream_event + + stream = io.TextIOWrapper(io.BytesIO(), encoding="cp1250", newline="") + monkeypatch.setattr(sys, "stdout", stream) + formatter = OutputFormatter(json_mode=True) + + _render_stream_event(formatter, {"event": "log", "data": {"msg": "načítám → hotovo"}}) + stream.flush() + + written = cast(io.BytesIO, stream.buffer).getvalue() + assert json.loads(written.decode("utf-8"))["data"]["msg"] == "načítám → hotovo" + + def test_each_event_is_flushed_so_consumers_see_it_immediately( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A stream consumer reads line by line; buffering would stall it.""" + from keboola_agent_cli.commands.agent import _render_stream_event + + stream = io.TextIOWrapper(io.BytesIO(), encoding="cp1250", newline="") + monkeypatch.setattr(sys, "stdout", stream) + formatter = OutputFormatter(json_mode=True) + + _render_stream_event(formatter, {"event": "init", "data": {}}) + + # Readable without an explicit flush by the test. + assert cast(io.BytesIO, stream.buffer).getvalue().endswith(b"\n")