From 7762ca189c3a80f0c044b11a06edef822f02c353 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Fri, 18 Sep 2026 21:31:19 +0530 Subject: [PATCH 1/3] fix: report output format failures as usage errors --- CHANGELOG.md | 2 ++ docs/output-contracts.md | 3 +++ lib/python/base_cli/_run.py | 8 +++++++ tests/test_optional_yaml_dependency.py | 33 ++++++++++++++++++++++++++ 4 files changed, 46 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec20c2e..3f00fa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and versions are tracked in the repo-root `VERSION` file. ### Fixed +- Surface expected output-format and optional-dependency failures as actionable + usage errors at the process boundary, including a stable JSON error code. - Preserve explicit application identities losslessly while using collision-resistant, path-safe runtime namespace components. - Give `BatteriesIncludedConfigLoader.cli_name` a documented identity role by diff --git a/docs/output-contracts.md b/docs/output-contracts.md index 684ed5d..5fe5fd2 100644 --- a/docs/output-contracts.md +++ b/docs/output-contracts.md @@ -4,6 +4,9 @@ and `ndjson` formats. Install `base-cli[yaml]` before selecting `yaml`; the other formats are available from the core package. The requested `text` format is presentation-aware: it renders a table on a TTY and tab-delimited rows when stdout is redirected or piped. +When an output format or its optional dependency is invalid, `run_app()` reports +an actionable usage error (exit code `2`); JSON mode uses the stable +`output_format_error` envelope code. Delimited output is intentionally automation-friendly: diff --git a/lib/python/base_cli/_run.py b/lib/python/base_cli/_run.py index 09dc442..5884e36 100644 --- a/lib/python/base_cli/_run.py +++ b/lib/python/base_cli/_run.py @@ -30,6 +30,7 @@ from .exit_codes import ExitCode from .json_contracts import dumps_envelope, error_envelope, success_envelope from .lifecycle_options import LifecycleOption, LifecycleOptions +from .output import OutputFormatError from .redaction import option_aliases_from_decls _MAX_JSON_CAPTURE_BYTES = 8 * 1_048_576 @@ -211,6 +212,13 @@ def run_app( _emit_json_error(state, outcome, str(exc), output_capture) return outcome.exit_code raise + except OutputFormatError as exc: + outcome = InvocationOutcome("output_format_error", "error", ExitCode.USAGE_ERROR) + if state.json_output: + _emit_json_error(state, outcome, str(exc), output_capture) + else: + print(f"Error: {exc}", file=sys.stderr) + return outcome.exit_code except Exception as exc: if reraise_unexpected: raise diff --git a/tests/test_optional_yaml_dependency.py b/tests/test_optional_yaml_dependency.py index 9e5b1d7..03e1f97 100644 --- a/tests/test_optional_yaml_dependency.py +++ b/tests/test_optional_yaml_dependency.py @@ -25,6 +25,39 @@ def test_yaml_output_explains_optional_install_when_yaml_is_missing(self) -> Non stream=stream, ) + def test_run_app_reports_missing_yaml_as_actionable_usage_error(self) -> None: + app = base_cli.App( + name="optional-yaml-output", + log_to_file=False, + lifecycle_options=base_cli.LifecycleOptions(json=base_cli.LifecycleOption("--json")), + ) + + @app.command() + def main(ctx: base_cli.Context) -> None: + del ctx + render_records( + ({"name": "value"},), + requested_format="yaml", + columns=(("NAME", "name"),), + ) + + for args in ([], ["--json"]): + with self.subTest(json=args == ["--json"]), tempfile.TemporaryDirectory() as home: + with mock.patch.dict(sys.modules, {"yaml": None}): + result = base_cli.testing.invoke(app, args, home=Path(home)) + + self.assertEqual(result.exit_code, base_cli.ExitCode.USAGE_ERROR) + if args: + payload = __import__("json").loads(result.stdout) + self.assertEqual(payload["code"], "output_format_error") + self.assertEqual(payload["details"]["exit_code"], base_cli.ExitCode.USAGE_ERROR) + self.assertIn("base-cli[yaml]", payload["message"]) + self.assertEqual(result.stderr, "") + else: + self.assertEqual(result.stdout, "") + self.assertIn("Error: PyYAML is required", result.stderr) + self.assertIn("base-cli[yaml]", result.stderr) + def test_yaml_config_explains_optional_install_when_yaml_is_missing(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "config.yaml" From 5387f36d96ea6f5cfed3a158cb4bb51487687894 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Fri, 18 Sep 2026 21:31:43 +0530 Subject: [PATCH 2/3] test: simplify JSON output assertion --- tests/test_optional_yaml_dependency.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_optional_yaml_dependency.py b/tests/test_optional_yaml_dependency.py index 03e1f97..5ae832a 100644 --- a/tests/test_optional_yaml_dependency.py +++ b/tests/test_optional_yaml_dependency.py @@ -1,6 +1,7 @@ from __future__ import annotations import io +import json import sys import tempfile import unittest @@ -48,7 +49,7 @@ def main(ctx: base_cli.Context) -> None: self.assertEqual(result.exit_code, base_cli.ExitCode.USAGE_ERROR) if args: - payload = __import__("json").loads(result.stdout) + payload = json.loads(result.stdout) self.assertEqual(payload["code"], "output_format_error") self.assertEqual(payload["details"]["exit_code"], base_cli.ExitCode.USAGE_ERROR) self.assertIn("base-cli[yaml]", payload["message"]) From f27104802e480c53f9742c3e4c6ae28a4a73b4dd Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Mon, 21 Sep 2026 00:22:11 +0530 Subject: [PATCH 3/3] fix: record output format failures as usage errors --- lib/python/base_cli/_lifecycle.py | 3 +++ lib/python/base_cli/_run.py | 2 ++ tests/test_app_run.py | 26 ++++++++++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/lib/python/base_cli/_lifecycle.py b/lib/python/base_cli/_lifecycle.py index 7816491..ebb9fab 100644 --- a/lib/python/base_cli/_lifecycle.py +++ b/lib/python/base_cli/_lifecycle.py @@ -11,6 +11,7 @@ from .context import Context from .exit_codes import ExitCode from .history import compact_optional_path, format_timestamp, status_for_exit_code +from .output import OutputFormatError @dataclass(frozen=True) @@ -125,6 +126,8 @@ def outcome_from_exception(click: Any, exc: BaseException) -> InvocationOutcome: return InvocationOutcome("interrupted", "aborted", ExitCode.INTERRUPTED) if isinstance(exc, EOFError): return InvocationOutcome("aborted", "error", ExitCode.FAILURE) + if isinstance(exc, OutputFormatError): + return InvocationOutcome("output_format_error", "error", ExitCode.USAGE_ERROR) if isinstance(exc, click.Abort): if isinstance(exc.__cause__, KeyboardInterrupt): return InvocationOutcome("interrupted", "aborted", ExitCode.INTERRUPTED) diff --git a/lib/python/base_cli/_run.py b/lib/python/base_cli/_run.py index 5884e36..2d703be 100644 --- a/lib/python/base_cli/_run.py +++ b/lib/python/base_cli/_run.py @@ -213,6 +213,8 @@ def run_app( return outcome.exit_code raise except OutputFormatError as exc: + if reraise_unexpected: + raise outcome = InvocationOutcome("output_format_error", "error", ExitCode.USAGE_ERROR) if state.json_output: _emit_json_error(state, outcome, str(exc), output_capture) diff --git a/tests/test_app_run.py b/tests/test_app_run.py index b542ce6..46dc976 100644 --- a/tests/test_app_run.py +++ b/tests/test_app_run.py @@ -11,6 +11,8 @@ from unittest import mock import base_cli +from base_cli._lifecycle import outcome_from_exception +from base_cli.output import OutputFormatError def generic_app(**kwargs: object) -> base_cli.App: @@ -18,6 +20,30 @@ def generic_app(**kwargs: object) -> base_cli.App: class RunAppTests(unittest.TestCase): + @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") + def test_output_format_errors_are_usage_outcomes_and_can_be_reraised(self) -> None: + import click + + outcome = outcome_from_exception(click, OutputFormatError("unsupported format")) + self.assertEqual((outcome.kind, outcome.exit_code), ("output_format_error", 2)) + + app = base_cli.App(name="output-format-reraise", log_to_file=False) + + @app.command() + def main(ctx: base_cli.Context) -> None: + del ctx + raise OutputFormatError("unsupported format") + + with ( + tempfile.TemporaryDirectory() as tmpdir, + mock.patch.dict( + os.environ, + {"HOME": tmpdir, "BASE_CLI_CACHE_DIR": str(Path(tmpdir) / ".cache")}, + ), + ): + with self.assertRaises(OutputFormatError): + base_cli.run_app(app, [], reraise_unexpected=True) + @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_malformed_click_exit_code_before_context_is_an_unexpected_error(self) -> None: import click