From cb39928e86b2ab15107401f6e9e8471e58ea8e7e Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Mon, 26 Oct 2026 11:45:46 -0400 Subject: [PATCH] fix(cli): report an invalid config.toml as one readable error instead of a traceback Fixes #1988 Co-Authored-By: Claude Sonnet 5.5 --- src/opensquilla/cli/agents_cmd.py | 5 +- src/opensquilla/cli/channels_cmd.py | 14 ++--- src/opensquilla/cli/config_errors.py | 47 +++++++++++++++ src/opensquilla/cli/doctor_cmd.py | 4 +- src/opensquilla/cli/gateway_cmd.py | 3 +- src/opensquilla/cli/gateway_rpc.py | 15 ++++- src/opensquilla/cli/onboard_cmd.py | 39 ++---------- src/opensquilla/cli/skills_cmd.py | 7 ++- tests/test_cli/test_config_load_errors.py | 72 +++++++++++++++++++++++ 9 files changed, 158 insertions(+), 48 deletions(-) create mode 100644 src/opensquilla/cli/config_errors.py create mode 100644 tests/test_cli/test_config_load_errors.py diff --git a/src/opensquilla/cli/agents_cmd.py b/src/opensquilla/cli/agents_cmd.py index 7e511bdb6e..ebed629106 100644 --- a/src/opensquilla/cli/agents_cmd.py +++ b/src/opensquilla/cli/agents_cmd.py @@ -14,7 +14,8 @@ from rich.table import Table from opensquilla.agents.registry import AgentRegistry -from opensquilla.onboarding.config_store import default_config_path, load_config, persist_config +from opensquilla.cli.config_errors import load_config_or_exit +from opensquilla.onboarding.config_store import default_config_path, persist_config from opensquilla.session.keys import normalize_agent_id agents_app = typer.Typer(help="Manage durable agents.") @@ -33,7 +34,7 @@ def _print_restart_notice() -> None: def _load_registry(config_path: Path | None) -> tuple[Path, Any, AgentRegistry]: target = config_path or default_config_path() - cfg = load_config(target) + cfg = load_config_or_exit(target) return target, cfg, AgentRegistry(cfg, config_path=target, persist_changes=False) diff --git a/src/opensquilla/cli/channels_cmd.py b/src/opensquilla/cli/channels_cmd.py index dfa3ba4eaf..b5e5025190 100644 --- a/src/opensquilla/cli/channels_cmd.py +++ b/src/opensquilla/cli/channels_cmd.py @@ -15,6 +15,7 @@ apply_channel_token, parse_channel_field_pairs, ) +from opensquilla.cli.config_errors import load_config_or_exit from opensquilla.cli.gateway_rpc import confirm_or_exit, run_gateway_sync from opensquilla.cli.output import print_json from opensquilla.cli.ui import ACCENT_HEADER, ACCENT_MARKUP @@ -24,7 +25,6 @@ list_channel_setup_specs, ) from opensquilla.onboarding.config_store import ( - load_config, persist_config, resolve_config_path, ) @@ -155,7 +155,7 @@ def channels_list( if json_output else _resolve_and_announce(config_path) ) - cfg = load_config(target) + cfg = load_config_or_exit(target) entries = list_channel_entries(cfg) if json_output: print_json(entries) @@ -261,7 +261,7 @@ def channels_add( apply_channel_token(payload, type_name, token) payload.update(parse_channel_field_pairs(fields, type_name)) - cfg = load_config(target) + cfg = load_config_or_exit(target) try: result = upsert_channel(cfg, entry_payload=payload) except (ValueError, KeyError) as exc: @@ -282,7 +282,7 @@ def channels_remove( config_path: Path | None = typer.Option(None, "--config"), ) -> None: target = _resolve_and_announce(config_path) - cfg = load_config(target) + cfg = load_config_or_exit(target) try: result = remove_channel(cfg, name=name) except KeyError as exc: @@ -299,7 +299,7 @@ def channels_enable( config_path: Path | None = typer.Option(None, "--config"), ) -> None: target = _resolve_and_announce(config_path) - cfg = load_config(target) + cfg = load_config_or_exit(target) try: result = set_channel_enabled(cfg, name=name, enabled=True) except KeyError as exc: @@ -316,7 +316,7 @@ def channels_disable( config_path: Path | None = typer.Option(None, "--config"), ) -> None: target = _resolve_and_announce(config_path) - cfg = load_config(target) + cfg = load_config_or_exit(target) try: result = set_channel_enabled(cfg, name=name, enabled=False) except KeyError as exc: @@ -340,7 +340,7 @@ def channels_edit( ) -> None: """Edit an existing channel; blank fields keep current values.""" target = _resolve_and_announce(config_path) - cfg = load_config(target) + cfg = load_config_or_exit(target) existing = next( ( e.model_dump(mode="python") diff --git a/src/opensquilla/cli/config_errors.py b/src/opensquilla/cli/config_errors.py new file mode 100644 index 0000000000..6be80fb4d5 --- /dev/null +++ b/src/opensquilla/cli/config_errors.py @@ -0,0 +1,47 @@ +"""CLI boundary for configuration load failures: one readable error, no traceback.""" + +from __future__ import annotations + +import tomllib +from pathlib import Path +from typing import NoReturn + +import typer +from pydantic import ValidationError + +from opensquilla.cli.ui import ACCENT_SOFT, error_console, markup_escape +from opensquilla.gateway.config_migration import ConfigParseError +from opensquilla.onboarding.config_store import load_config, resolve_config_path +from opensquilla.onboarding.next_steps import quote_cli_arg + +CONFIG_LOAD_ERRORS = (OSError, tomllib.TOMLDecodeError, ConfigParseError, ValidationError) + + +def format_config_load_error(exc: Exception) -> str: + if isinstance(exc, ValidationError): + errors = exc.errors() + if errors: + first = errors[0] + loc = ".".join(str(part) for part in first.get("loc", ())) + msg = str(first.get("msg") or "invalid value") + return f"{loc}: {msg}" if loc else msg + return str(exc).splitlines()[0] + + +def exit_config_load_error(exc: Exception, path: str | Path | None = None) -> NoReturn: + target, _ = resolve_config_path(path) + error_console.print(f"[red]OpenSquilla config error:[/red] {markup_escape(str(target))}") + error_console.print(f"[dim]Reason: {markup_escape(format_config_load_error(exc))}[/dim]") + error_console.print("[dim]Fix: edit or move this config, then rerun onboarding:[/dim]") + error_console.print( + f" [{ACCENT_SOFT}]opensquilla onboard --if-needed" + f" --config {markup_escape(quote_cli_arg(target))}[/]" + ) + raise typer.Exit(code=2) from exc + + +def load_config_or_exit(path: str | Path | None = None): + try: + return load_config(path) + except CONFIG_LOAD_ERRORS as exc: + exit_config_load_error(exc, path) diff --git a/src/opensquilla/cli/doctor_cmd.py b/src/opensquilla/cli/doctor_cmd.py index a3887bb85e..9fd83e8fcb 100644 --- a/src/opensquilla/cli/doctor_cmd.py +++ b/src/opensquilla/cli/doctor_cmd.py @@ -17,7 +17,7 @@ from opensquilla.cli.gateway_rpc import ( GatewayTarget, default_gateway_target, - gateway_url_from_config, + gateway_url_from_config_or_exit, ) from opensquilla.cli.output import print_json from opensquilla.cli.url_utils import normalize_gateway_url @@ -722,7 +722,7 @@ def _target_for_doctor( return GatewayTarget(normalize_gateway_url(gateway_url)) if config_path is not None: return GatewayTarget( - gateway_url_from_config(config_path), + gateway_url_from_config_or_exit(config_path), config_path, config_owns_target=True, ) diff --git a/src/opensquilla/cli/gateway_cmd.py b/src/opensquilla/cli/gateway_cmd.py index 6da0c2e7b5..d9e1bbac3c 100644 --- a/src/opensquilla/cli/gateway_cmd.py +++ b/src/opensquilla/cli/gateway_cmd.py @@ -18,6 +18,7 @@ import typer from pydantic import ValidationError +from opensquilla.cli.config_errors import exit_config_load_error from opensquilla.cli.gateway_lifecycle import ( DESKTOP_CONFIG_OUTSIDE_PROFILE, GatewayLifecycleManager, @@ -227,7 +228,7 @@ def _load_gateway_config( return GatewayConfig.load(config_path, read_only=read_only) except ValidationError as exc: if not has_gateway_port_error(exc): - raise + exit_config_load_error(exc, config_path) _invalid_gateway_port(action=action, json_output=json_output) except ConfigParseError as exc: console.print(f"[red]Invalid gateway config:[/red] {exc}") diff --git a/src/opensquilla/cli/gateway_rpc.py b/src/opensquilla/cli/gateway_rpc.py index 01fba5c043..4b03e9fd84 100644 --- a/src/opensquilla/cli/gateway_rpc.py +++ b/src/opensquilla/cli/gateway_rpc.py @@ -34,7 +34,7 @@ def default_gateway_target() -> GatewayTarget: ) if config_path := os.environ.get("OPENSQUILLA_GATEWAY_CONFIG_PATH"): return GatewayTarget( - gateway_url_from_config(config_path), + gateway_url_from_config_or_exit(config_path), config_path, config_owns_target=True, ) @@ -90,6 +90,17 @@ def gateway_url_from_config(config_path: str | Path) -> str: return normalize_gateway_url(f"ws://{host}:{int(config.port)}/ws") +def gateway_url_from_config_or_exit(config_path: str | Path) -> str: + """Like ``gateway_url_from_config``, for a config path the user named.""" + + from opensquilla.cli.config_errors import CONFIG_LOAD_ERRORS, exit_config_load_error + + try: + return gateway_url_from_config(config_path) + except CONFIG_LOAD_ERRORS as exc: + exit_config_load_error(exc, config_path) + + def _target_gateway( *, gateway_url: str | None, @@ -99,7 +110,7 @@ def _target_gateway( return GatewayTarget(normalize_gateway_url(gateway_url), config_path) if config_path is not None: return GatewayTarget( - gateway_url_from_config(config_path), + gateway_url_from_config_or_exit(config_path), config_path, config_owns_target=True, ) diff --git a/src/opensquilla/cli/onboard_cmd.py b/src/opensquilla/cli/onboard_cmd.py index c4f531a901..fb0f0640bf 100644 --- a/src/opensquilla/cli/onboard_cmd.py +++ b/src/opensquilla/cli/onboard_cmd.py @@ -11,6 +11,7 @@ from pydantic import ValidationError from rich.table import Table +from opensquilla.cli.config_errors import exit_config_load_error from opensquilla.cli.output import print_json from opensquilla.cli.ui import ( ACCENT, @@ -23,7 +24,7 @@ ) from opensquilla.gateway.config import GatewayConfig from opensquilla.gateway.config_migration import ConfigParseError -from opensquilla.onboarding.config_store import load_config, resolve_config_path +from opensquilla.onboarding.config_store import load_config from opensquilla.onboarding.errors import UserCancelledError from opensquilla.onboarding.flow import ( OnboardOptions, @@ -183,17 +184,6 @@ def _format_action_sections(status: OnboardingStatus, names: list[str]) -> str: return ", ".join(parts) if parts else "none" -def _format_config_load_error(exc: Exception) -> str: - if isinstance(exc, ValidationError): - errors = exc.errors() - if errors: - first = errors[0] - loc = ".".join(str(part) for part in first.get("loc", ())) - msg = str(first.get("msg") or "invalid value") - return f"{loc}: {msg}" if loc else msg - return str(exc).splitlines()[0] - - def _format_validation_error(exc: ValidationError) -> str: """Render a ValidationError as a field-naming summary. @@ -234,28 +224,11 @@ def _print_restart_guidance(result: object, config_path: Path | None = None) -> ) -def _exit_config_load_error(exc: Exception, path: str | Path | None = None) -> NoReturn: - target, _ = resolve_config_path(path) - config_arg = _config_cli_arg(target) - error_console.print( - f"[red]OpenSquilla config error:[/red] {markup_escape(str(target))}" - ) - error_console.print( - f"[dim]Reason: {markup_escape(_format_config_load_error(exc))}[/dim]" - ) - error_console.print("[dim]Fix: edit or move this config, then rerun onboarding:[/dim]") - error_console.print( - f" [{ACCENT_SOFT}]opensquilla onboard --if-needed" - f"{markup_escape(config_arg)}[/]" - ) - raise typer.Exit(code=2) from exc - - def _load_config_for_cli(path: str | Path | None = None): try: return load_config(path) except (OSError, tomllib.TOMLDecodeError, ConfigParseError, ValidationError) as exc: - _exit_config_load_error(exc, path) + exit_config_load_error(exc, path) def _has_stored_setup_state(cfg) -> bool: @@ -702,7 +675,7 @@ def onboard_command( # Corrupt config discovered mid-wizard (e.g. re-loaded after an # out-of-band edit): route through the same productized handoff as # `--if-needed`/`status` instead of a raw traceback. - _exit_config_load_error(exc, config_path) + exit_config_load_error(exc, config_path) except OSError as exc: # The config loaded fine above, so an OSError out of the wizard is a # runtime I/O failure (disk full at persist time, permissions revoked @@ -1778,7 +1751,7 @@ def configure_command( # instead of silently forwarding nothing to the wizard. _exit_incomplete_headless_flags(normalized, given) except (OSError, tomllib.TOMLDecodeError, ConfigParseError, ValidationError) as exc: - _exit_config_load_error(exc, config_path) + exit_config_load_error(exc, config_path) except (KeyError, TypeError, ValueError) as exc: error_console.print(f"[red]Error:[/red] {markup_escape(exc)}") raise typer.Exit(code=2) from exc @@ -1800,7 +1773,7 @@ def configure_command( # same operator intent as Esc/Ctrl+C, so the same productized exit. _exit_cancelled(exc) except (OSError, tomllib.TOMLDecodeError, ConfigParseError, ValidationError) as exc: - _exit_config_load_error(exc, config_path) + exit_config_load_error(exc, config_path) except (KeyError, TypeError, ValueError) as exc: # Mutation-level validation failures reachable from the wizard (e.g. # a blank required channel secret) must surface like the headless diff --git a/src/opensquilla/cli/skills_cmd.py b/src/opensquilla/cli/skills_cmd.py index b747ad0ea9..edc9159618 100644 --- a/src/opensquilla/cli/skills_cmd.py +++ b/src/opensquilla/cli/skills_cmd.py @@ -12,6 +12,7 @@ from rich.table import Table from rich.text import Text +from opensquilla.cli.config_errors import CONFIG_LOAD_ERRORS, exit_config_load_error from opensquilla.cli.gateway_rpc import ( default_gateway_token, default_gateway_url, @@ -223,7 +224,11 @@ def _build_offline_skill_loader() -> tuple[Any, Any]: from opensquilla.skills.loader import SkillLoader from opensquilla.skills.paths import resolve_skill_layer_dirs - config = GatewayConfig.load(os.environ.get("OPENSQUILLA_GATEWAY_CONFIG_PATH")) + config_path = os.environ.get("OPENSQUILLA_GATEWAY_CONFIG_PATH") + try: + config = GatewayConfig.load(config_path) + except CONFIG_LOAD_ERRORS as exc: + exit_config_load_error(exc, config_path) workspace_root = Path(config.workspace_dir) if config.workspace_dir else None workspace_override = Path(config.skills.workspace_dir) if config.skills.workspace_dir else None layer_dirs = resolve_skill_layer_dirs( diff --git a/tests/test_cli/test_config_load_errors.py b/tests/test_cli/test_config_load_errors.py new file mode 100644 index 0000000000..8165d3089c --- /dev/null +++ b/tests/test_cli/test_config_load_errors.py @@ -0,0 +1,72 @@ +"""A semantically invalid config.toml is reported as one readable error, not a traceback.""" + +from __future__ import annotations + +import pytest +from typer.testing import CliRunner + +from opensquilla.cli.main import app + +runner = CliRunner() + +SLACK_SOCKET_WITHOUT_APP_TOKEN = """\ +[channels] +[[channels.channels]] +name = "ops-slack" +type = "slack" +token = "xoxb-placeholder" +connection_mode = "socket" +slack_channel_id = "C12345" +""" + + +@pytest.fixture +def invalid_config(tmp_path, monkeypatch): + target = tmp_path / "config.toml" + target.write_text(SLACK_SOCKET_WITHOUT_APP_TOKEN, encoding="utf-8") + monkeypatch.setenv("OPENSQUILLA_GATEWAY_CONFIG_PATH", str(target)) + return target + + +@pytest.mark.parametrize( + "argv", + [ + ["channels", "list"], + ["channels", "list", "--json"], + ["channels", "enable", "ops-slack"], + ["channels", "remove", "ops-slack"], + ["agents", "list"], + ["skills", "list"], + ["gateway", "status"], + ], +) +def test_invalid_config_value_exits_with_readable_error(invalid_config, argv): + result = runner.invoke(app, argv) + + assert result.exit_code == 2 + assert isinstance(result.exception, SystemExit) + output = result.output + assert "OpenSquilla config error" in output + assert "slack socket channels require app_token" in output + assert "opensquilla onboard --if-needed" in output + assert output.count("OpenSquilla config error") == 1 + assert "Traceback" not in output + assert "errors.pydantic.dev" not in output + + +@pytest.mark.parametrize("argv", [["skills", "list"], ["gateway", "status"]]) +def test_invalid_implicit_config_is_reported_once(tmp_path, monkeypatch, argv): + state_dir = tmp_path / "state" + state_dir.mkdir() + (state_dir / "config.toml").write_text(SLACK_SOCKET_WITHOUT_APP_TOKEN, encoding="utf-8") + monkeypatch.delenv("OPENSQUILLA_GATEWAY_CONFIG_PATH", raising=False) + monkeypatch.delenv("OPENSQUILLA_GATEWAY_URL", raising=False) + monkeypatch.setenv("OPENSQUILLA_STATE_DIR", str(state_dir)) + monkeypatch.chdir(tmp_path) + + result = runner.invoke(app, argv) + + assert result.exit_code == 2 + assert result.output.count("OpenSquilla config error") == 1 + assert "slack socket channels require app_token" in result.output + assert "Traceback" not in result.output