From 3d201aaeef7bd201f21bfa994df876539cdc8306 Mon Sep 17 00:00:00 2001 From: Imran Siddique Date: Thu, 20 Aug 2026 21:26:15 -0700 Subject: [PATCH] fix(cli): a valid config was reported invalid because of a tick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a Windows console, whose default code page cannot encode U+2713, `cmcp validate-config` failed on its own success path: ✗ Config invalid: 'charmap' codec can't encode character '✓' in position 0: character maps to The config was valid. The success echo sat inside the same try block as the validation, so `except Exception` caught the UnicodeEncodeError from printing the result and relabelled it as a validation failure, then exited 1. A reader following the published quickstart was told their config was broken because the tool could not draw a tick. Two changes: - _marker() returns the glyph only when the target stream can encode it, and "OK" or "ERROR" otherwise. The marker is decoration; it should degrade rather than take the command down. - The success echo moves out of the try. Only the validation belongs there. A failure to print is not a failure to validate. Applied to validate-config and validate-bundle, the four sites that print non-ASCII. Nothing else in the package writes non-ASCII to stdout. Regression test asserts the second point directly: with the first echo raising, the command must never claim the config is invalid. It fails against the previous structure and passes against this one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013EQx4N5BzTQbY8kvXUsdkY --- src/cmcp_runtime/cli.py | 36 ++++++- tests/unit/test_cli_output_encoding.py | 128 +++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_cli_output_encoding.py diff --git a/src/cmcp_runtime/cli.py b/src/cmcp_runtime/cli.py index 01a22cfc..f0e2ab28 100644 --- a/src/cmcp_runtime/cli.py +++ b/src/cmcp_runtime/cli.py @@ -16,6 +16,31 @@ from cmcp_runtime.startup import RuntimeContext +def _marker(preferred: str, fallback: str, stream: object) -> str: + """Return `preferred` if `stream` can encode it, else `fallback`. + + A Windows console defaults to a legacy code page, and writing "✓" to + it raises UnicodeEncodeError. That killed `cmcp validate-config` on its + own success path: the config was valid, and the command still exited + non-zero with a codec traceback. The marker is decoration, so it degrades + rather than taking the command down with it. + """ + encoding = getattr(stream, "encoding", None) or "ascii" + try: + preferred.encode(encoding) + except (UnicodeEncodeError, LookupError): + return fallback + return preferred + + +def _ok() -> str: + return _marker("✓", "OK", sys.stdout) + + +def _bad() -> str: + return _marker("✗", "ERROR", sys.stderr) + + def build_server(ctx: RuntimeContext) -> MCPServer: """ Compose the running gateway from a validated RuntimeContext. @@ -249,11 +274,12 @@ def validate_config(config: str) -> None: try: load_config(config) - click.echo(f"✓ Config valid: {config}") except Exception as exc: - click.echo(f"✗ Config invalid: {exc}", err=True) + click.echo(f"{_bad()} Config invalid: {exc}", err=True) raise SystemExit(1) from exc + click.echo(f"{_ok()} Config valid: {config}") + @main.command("validate-bundle") @click.option("--bundle-path", required=True, type=click.Path(exists=True)) @@ -265,7 +291,7 @@ def validate_bundle(bundle_path: str, expected_hash: str) -> None: try: bundle = load_policy_bundle(bundle_path) except Exception as exc: - click.echo(f"✗ Bundle load error: {exc}", err=True) + click.echo(f"{_bad()} Bundle load error: {exc}", err=True) raise SystemExit(1) from exc bundle_hash = bundle.bundle_hash @@ -274,10 +300,10 @@ def validate_bundle(bundle_path: str, expected_hash: str) -> None: actual_hex = bundle_hash.removeprefix("sha256:") if actual_hex == expected_hex: - click.echo(f"✓ Bundle valid: {bundle_hash}") + click.echo(f"{_ok()} Bundle valid: {bundle_hash}") else: click.echo( - f"✗ Bundle hash mismatch: expected {expected_hash}, got {bundle_hash}", + f"{_bad()} Bundle hash mismatch: expected {expected_hash}, got {bundle_hash}", err=True, ) raise SystemExit(1) diff --git a/tests/unit/test_cli_output_encoding.py b/tests/unit/test_cli_output_encoding.py new file mode 100644 index 00000000..6ced0633 --- /dev/null +++ b/tests/unit/test_cli_output_encoding.py @@ -0,0 +1,128 @@ +""" +Regression tests for CLI status markers on a legacy console. + +`cmcp validate-config` printed "✓ Config valid: ..." from inside the same +try block that ran the validation. On a Windows console, whose default code +page cannot encode that character, the echo raised UnicodeEncodeError, the +`except Exception` caught it, and the command reported + + ✗ Config invalid: 'charmap' codec can't encode character '✓' ... + +then exited 1. The config was valid. A user following the published quickstart +was told their config was broken because the tool could not draw a tick. +""" + +from __future__ import annotations + +import pytest +from click.testing import CliRunner + +from cmcp_runtime.cli import _bad, _marker, _ok, main + + +class _Stream: + """Minimal stand-in for sys.stdout with a fixed encoding.""" + + def __init__(self, encoding: str | None) -> None: + self.encoding = encoding + + +@pytest.mark.parametrize( + ("encoding", "expected"), + [ + ("utf-8", "✓"), + ("UTF-8", "✓"), + ("cp1252", "OK"), + ("ascii", "OK"), + ("cp437", "OK"), + (None, "OK"), + ("not-a-real-codec", "OK"), + ], +) +def test_marker_falls_back_when_the_stream_cannot_encode_it(encoding, expected): + assert _marker("✓", "OK", _Stream(encoding)) == expected + + +def test_marker_never_raises_on_an_object_without_encoding(): + assert _marker("✓", "OK", object()) == "OK" + + +def test_ok_and_bad_return_something_printable(): + assert _ok() in ("✓", "OK") + assert _bad() in ("✗", "ERROR") + + +def _write_valid_config(tmp_path): + config = tmp_path / "cmcp-config.yaml" + config.write_text( + "attestation:\n" + " provider: auto\n" + " enforcement_mode: enforcing\n" + "policy_bundle_path: ./policies/\n" + "catalog_path: ./catalog.json\n" + 'listen_addr: "127.0.0.1:8443"\n', + encoding="utf-8", + ) + return config + + +def test_validate_config_succeeds_on_a_legacy_code_page(tmp_path): + """A valid config must validate where the tick cannot be drawn.""" + config = _write_valid_config(tmp_path) + result = CliRunner(charset="cp1252").invoke( + main, ["validate-config", "--config", str(config)] + ) + assert result.exit_code == 0, result.output + assert "Config valid" in result.output + assert "Config invalid" not in result.output + + +def test_validate_config_keeps_the_tick_where_utf8_is_available(tmp_path): + config = _write_valid_config(tmp_path) + result = CliRunner(charset="utf-8").invoke( + main, ["validate-config", "--config", str(config)] + ) + assert result.exit_code == 0, result.output + assert "✓ Config valid" in result.output + + +def test_an_actually_invalid_config_still_fails(tmp_path): + """The fallback must not turn a real validation failure into a pass.""" + config = tmp_path / "cmcp-config.yaml" + config.write_text("attestation: [this is not a mapping" + chr(10), encoding="utf-8") + result = CliRunner(charset="cp1252").invoke( + main, ["validate-config", "--config", str(config)] + ) + assert result.exit_code == 1 + assert "Config invalid" in result.output + + +def test_a_failed_success_message_is_never_relabelled_as_invalid(tmp_path, monkeypatch): + """The success echo must sit outside the try that runs the validation. + + A real Windows console raises UnicodeEncodeError from the write itself. + When the success echo lived inside the try, `except Exception` caught that + and printed "Config invalid: 'charmap' codec can't encode ...", so a valid + config was reported broken. Here the first echo raises and the test asserts + the command never claims the config is invalid. + """ + import cmcp_runtime.cli as cli + + config = _write_valid_config(tmp_path) + seen: list[str] = [] + calls = {"n": 0} + + def flaky_echo(message="", *args, **kwargs): + calls["n"] += 1 + if calls["n"] == 1: + raise UnicodeEncodeError("charmap", "✓", 0, 1, "unmapped") + seen.append(str(message)) + + monkeypatch.setattr(cli.click, "echo", flaky_echo) + + with pytest.raises(UnicodeEncodeError): + cli.validate_config.callback(config=str(config)) + + assert not any("Config invalid" in m for m in seen), ( + "a printing failure was relabelled as a validation failure: %r" % seen + )