Skip to content
Merged
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
32 changes: 32 additions & 0 deletions plugins/kbagent/skills/kbagent/references/gotchas.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
14 changes: 14 additions & 0 deletions src/keboola_agent_cli/changelog.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down
6 changes: 5 additions & 1 deletion src/keboola_agent_cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Comment thread
padak marked this conversation as resolved.

app = typer.Typer(
name="kbagent",
help="Keboola Agent CLI -- AI-friendly interface to Keboola projects",
Expand Down
52 changes: 52 additions & 0 deletions src/keboola_agent_cli/output.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
77 changes: 77 additions & 0 deletions tests/test_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
OutputFormatter,
_format_duration,
_seconds_to_human,
force_utf8_when_redirected,
format_config_detail,
format_configs_table,
format_doctor_panel,
Expand Down Expand Up @@ -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())