diff --git a/CHANGELOG.md b/CHANGELOG.md index bee197b..b30dff5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ CalVer image tags (`stable-YYYY.MM.N`, `dev-YYYY.MM.N`) are produced for every C ## [Unreleased] +### Added +- `agnes query --json` is now a shortcut for `--format json` — paste-prompts and LLM-assisted analysts routinely reach for `--json` first, and the typer "Did you mean `--stdin`?" suggestion the missing flag previously produced was actively misleading. `--json --format ` is rejected as mutually exclusive (`--json --format json` is redundantly allowed). Closes #345 D. + +### Internal +- Added explicit 5xx-path regression test alongside the existing 4xx case for `agnes query --remote` to lock in the `raise typer.Exit(1)` rc=1 contract for any non-2xx response (`tests/test_cli_query.py::test_remote_query_5xx_exits_nonzero`). No code change — the existing exit-code logic already does the right thing; the test guards against future regression. Closes #345 C. + ### Fixed - **UI consistency pass** (I-UI-01..05): radio-card selected state on `/admin/tables` (14 cards get blue border + light bg highlight via `.sync-option-card:has(input:checked)`); promoted `.label-qualifier` / `.optional` to global rule (drops local duplicate); inline `` migrated to design tokens with bg + border; `.btn-google` hover hardcoded swatches replaced with vars; `.code-block code` border + radius reset for dark containers; `.form-textarea` promoted to global. Plus #340 follow-up: removed leftover Phase F2 `{% if data_source_type == 'keboola' %}` guard around edit-modal JS so handlers ship to every instance type (Discover button onclick call sites still respect the guard). Closes #347 (credit @MonikaFeigler). - `agnes refresh-marketplace` (non-bootstrap path) now re-applies diff --git a/cli/commands/query.py b/cli/commands/query.py index 3a455aa..bee82dd 100644 --- a/cli/commands/query.py +++ b/cli/commands/query.py @@ -15,10 +15,24 @@ def query_command( sql_opt: Optional[str] = typer.Option(None, "--sql", help="SQL query to execute (named option)"), remote: bool = typer.Option(False, "--remote", help="Execute on server instead of locally"), fmt: str = typer.Option("table", "--format", "-f", help="Output format: table, json, csv"), + json_flag: bool = typer.Option(False, "--json", help="Shortcut for --format json"), limit: int = typer.Option(1000, "--limit", help="Max rows to return"), stdin: bool = typer.Option(False, "--stdin", help="Read SQL from stdin as JSON {\"sql\": \"...\"}"), ): """Execute SQL query against DuckDB.""" + # `--json` is an alias for `--format json` (issue #345 D). Paste-prompts + # and LLM-assisted analysis routinely reach for `--json`; the typer + # "Did you mean --stdin?" suggestion that the absence of this flag + # produced was actively misleading. + if json_flag: + if fmt != "table" and fmt != "json": + typer.echo( + f"Error: --json and --format={fmt} are mutually exclusive.", + err=True, + ) + raise typer.Exit(1) + fmt = "json" + # Resolve SQL from exactly one of: positional, --sql, or --stdin sources_provided = sum([ sql is not None, diff --git a/tests/test_cli_query.py b/tests/test_cli_query.py index db9fe51..709c655 100644 --- a/tests/test_cli_query.py +++ b/tests/test_cli_query.py @@ -47,6 +47,51 @@ class TestRemoteQuery: assert "HTTP 400" in result.output assert "bad SQL" in result.output + def test_remote_query_5xx_exits_nonzero(self): + """5xx server errors propagate to a nonzero exit code (issue #345 C). + + Without this, ``set -e`` shells, CI pipelines, and any wrapper + script that checks ``$?`` to detect failure would silently + proceed even when the server returned ``HTTP 502:`` to stdout. + The reporter who filed #345 hit the exact rc=0-on-502 pattern + on a slightly older CLI build; this guards against any future + regression that drops the ``raise typer.Exit(1)`` from the + non-200 branch of ``_query_remote``. + """ + with patch("cli.client.api_post", return_value=_resp(502, text="bad gateway")): + result = runner.invoke(app, ["query", "SELECT 1", "--remote"]) + assert result.exit_code != 0, ( + f"Expected nonzero exit for HTTP 502, got rc={result.exit_code}. " + f"Output: {result.output}" + ) + assert "HTTP 502" in result.output + + def test_remote_query_json_alias_equals_format_json(self): + """``--json`` is a shortcut for ``--format json`` (issue #345 D). + + Paste-prompts and LLM-assisted analysts routinely reach for + ``agnes query --json`` first; the typer "Did you mean --stdin?" + suggestion the absence of this flag previously produced was + actively misleading. + """ + payload = {"columns": ["id"], "rows": [[1]], "truncated": False} + with patch("cli.client.api_post", return_value=_resp(200, payload)): + result = runner.invoke(app, ["query", "SELECT 1", "--remote", "--json"]) + assert result.exit_code == 0 + # Output is parseable JSON, equivalent to --format json + parsed = json.loads(result.output.strip()) + assert parsed == [{"id": 1}] + + def test_json_and_explicit_csv_format_are_mutually_exclusive(self): + """``--json --format csv`` is contradictory; reject with rc=1. + + ``--json --format json`` is allowed (redundant but consistent); + ``--json`` alone is the common case. + """ + result = runner.invoke(app, ["query", "SELECT 1", "--json", "--format", "csv"]) + assert result.exit_code == 1 + assert "mutually exclusive" in result.output + def test_remote_query_truncated(self): """Truncated result shows warning.""" payload = {"columns": ["id"], "rows": [[i] for i in range(5)], "truncated": True}