diff --git a/packages/deepctl-cmd-billing/src/deepctl_cmd_billing/command.py b/packages/deepctl-cmd-billing/src/deepctl_cmd_billing/command.py index 080ede3..c0af07e 100644 --- a/packages/deepctl-cmd-billing/src/deepctl_cmd_billing/command.py +++ b/packages/deepctl-cmd-billing/src/deepctl_cmd_billing/command.py @@ -11,6 +11,7 @@ Config, DeepgramClient, get_output_format, + get_status_console, ) from rich.console import Console from rich.table import Table @@ -20,7 +21,7 @@ console = Console() # Status/progress chrome must never touch stdout, or it corrupts JSON/CSV # output that callers pipe into jq and friends. -status_console = Console(stderr=True) +status_console = get_status_console() class BillingCommand(BaseCommand): diff --git a/packages/deepctl-cmd-keys/src/deepctl_cmd_keys/command.py b/packages/deepctl-cmd-keys/src/deepctl_cmd_keys/command.py index fee0e50..71a10d5 100644 --- a/packages/deepctl-cmd-keys/src/deepctl_cmd_keys/command.py +++ b/packages/deepctl-cmd-keys/src/deepctl_cmd_keys/command.py @@ -2,14 +2,19 @@ from __future__ import annotations +import sys from typing import Any +import click from deepctl_core import ( AuthManager, BaseCommand, BaseResult, Config, DeepgramClient, + get_output_format, + get_status_console, + is_agentic, ) from rich.console import Console from rich.table import Table @@ -17,6 +22,9 @@ from .models import KeyCreatedInfo, KeyInfo, KeysResult console = Console() +# Status/progress chrome must never touch stdout, or it corrupts JSON/CSV +# output that callers pipe into jq and friends. +status_console = get_status_console() class KeysCommand(BaseCommand): @@ -137,9 +145,11 @@ def handle( create_key = kwargs.get("create", False) show_key = kwargs.get("show") delete_key = kwargs.get("delete") - project_id = kwargs.get("project_id") - - dry_run = kwargs.get("dry_run", False) + # Popped, not read: both are forwarded explicitly to _create_key, which + # also receives **kwargs. Leaving them in the dict passes each argument + # twice and raises TypeError before the body runs. + project_id = kwargs.pop("project_id", None) + dry_run = kwargs.pop("dry_run", False) try: if create_key: @@ -159,7 +169,7 @@ def handle( return self._list_keys(client, project_id, kwargs.get("status")) except Exception as e: - console.print(f"[red]Error:[/red] {e}") + status_console.print(f"[red]Error:[/red] {e}") return BaseResult(status="error", message=str(e)) def _list_keys( @@ -168,12 +178,12 @@ def _list_keys( project_id: str | None, status: str | None, ) -> BaseResult: - console.print("[blue]Fetching API keys...[/blue]") + status_console.print("[blue]Fetching API keys...[/blue]") result = client.list_keys(project_id=project_id, status=status) keys_raw = result.get("api_keys", result.get("keys", [])) if not keys_raw: - console.print("[yellow]No API keys found[/yellow]") + status_console.print("[yellow]No API keys found[/yellow]") return KeysResult(status="info", message="No keys found") key_models: list[KeyInfo] = [] @@ -208,8 +218,12 @@ def _list_keys( info.expiration_date[:10] if info.expiration_date else "never", ) - console.print(table) - console.print(f"\n[dim]{len(key_models)} key(s) found[/dim]") + # Human table only in default mode; json/yaml/csv are emitted by the + # framework from the returned result, so printing here would prepend + # non-parseable text to that output. + if get_output_format() == "default": + console.print(table) + console.print(f"\n[dim]{len(key_models)} key(s) found[/dim]") return KeysResult(status="success", keys=key_models, count=len(key_models)) @@ -231,18 +245,19 @@ def _create_key( tags = [t.strip() for t in tags_str.split(",")] if tags_str else None if dry_run: - console.print("[yellow]Dry run — no changes made[/yellow]") - console.print(f" Would create key: comment='{comment or '(none)'}'") - console.print(f" Scopes: {', '.join(scopes)}") - if expiration: - console.print(f" Expires: {expiration}") - if ttl: - console.print(f" TTL: {ttl}s") - if tags: - console.print(f" Tags: {', '.join(tags)}") + if get_output_format() == "default": + console.print("[yellow]Dry run — no changes made[/yellow]") + console.print(f" Would create key: comment='{comment or '(none)'}'") + console.print(f" Scopes: {', '.join(scopes)}") + if expiration: + console.print(f" Expires: {expiration}") + if ttl: + console.print(f" TTL: {ttl}s") + if tags: + console.print(f" Tags: {', '.join(tags)}") return BaseResult(status="dry_run", message="Dry run: key would be created") - console.print("[blue]Creating API key...[/blue]") + status_console.print("[blue]Creating API key...[/blue]") result = client.create_key( project_id=project_id, @@ -256,13 +271,18 @@ def _create_key( key_id = result.get("api_key_id", "") key_value = result.get("key", "") - console.print("[green]API key created successfully[/green]") - console.print(f" Key ID: {key_id}") - console.print(f" Comment: {comment or '(none)'}") - console.print(f" Scopes: {', '.join(scopes)}") - if key_value: - console.print(f"\n [bold yellow]Key: {key_value}[/bold yellow]") - console.print("[dim] Save this key now — it won't be shown again.[/dim]") + # The created key (secret included) is carried in the result for + # json/yaml/csv, so only render it for humans in default mode. + if get_output_format() == "default": + console.print("[green]API key created successfully[/green]") + console.print(f" Key ID: {key_id}") + console.print(f" Comment: {comment or '(none)'}") + console.print(f" Scopes: {', '.join(scopes)}") + if key_value: + console.print(f"\n [bold yellow]Key: {key_value}[/bold yellow]") + console.print( + "[dim] Save this key now — it won't be shown again.[/dim]" + ) created = KeyCreatedInfo( key_id=key_id, @@ -278,7 +298,7 @@ def _show_key( key_id: str, project_id: str | None, ) -> BaseResult: - console.print(f"[blue]Fetching key details:[/blue] {key_id}") + status_console.print(f"[blue]Fetching key details:[/blue] {key_id}") result = client.get_key(key_id, project_id=project_id) key_data = result.get("api_key", result) if isinstance(result, dict) else result @@ -292,14 +312,17 @@ def _show_key( tags=key_data.get("tags", []), ) - console.print("[green]Key Details:[/green]") - console.print(f" Key ID: {info.key_id}") - console.print(f" Comment: {info.comment or '(none)'}") - console.print(f" Scopes: {', '.join(info.scopes) if info.scopes else '-'}") - console.print(f" Created: {info.created or '-'}") - console.print(f" Expires: {info.expiration_date or 'never'}") - if info.tags: - console.print(f" Tags: {', '.join(info.tags)}") + if get_output_format() == "default": + console.print("[green]Key Details:[/green]") + console.print(f" Key ID: {info.key_id}") + console.print(f" Comment: {info.comment or '(none)'}") + console.print( + f" Scopes: {', '.join(info.scopes) if info.scopes else '-'}" + ) + console.print(f" Created: {info.created or '-'}") + console.print(f" Expires: {info.expiration_date or 'never'}") + if info.tags: + console.print(f" Tags: {', '.join(info.tags)}") return KeysResult(status="success", keys=[info], count=1) @@ -312,18 +335,52 @@ def _delete_key( dry_run: bool = False, ) -> BaseResult: if dry_run: - console.print("[yellow]Dry run — no changes made[/yellow]") - console.print(f" Would delete key: {key_id}") + if get_output_format() == "default": + console.print("[yellow]Dry run — no changes made[/yellow]") + console.print(f" Would delete key: {key_id}") return BaseResult( status="dry_run", message=f"Dry run: key {key_id} would be deleted" ) - if not yes and not self.confirm( - f"Delete API key {key_id}? This cannot be undone.", default=False - ): - return BaseResult(status="cancelled", message="Cancelled by user") + if not yes: + aborted = self._confirm_delete(key_id) + if aborted is not None: + return aborted - console.print(f"[blue]Deleting API key:[/blue] {key_id}") + status_console.print(f"[blue]Deleting API key:[/blue] {key_id}") client.delete_key(key_id, project_id=project_id) - console.print(f"[green]API key {key_id} deleted[/green]") + status_console.print(f"[green]API key {key_id} deleted[/green]") return BaseResult(status="success", message=f"Key {key_id} deleted") + + def _confirm_delete(self, key_id: str) -> BaseResult | None: + """Gate a delete behind confirmation; return a result to abort with. + + `BaseCommand.confirm` cannot be used here. It returns its default + (False) whenever any parameter came from the command line, and + `--delete KEY_ID` is itself such a parameter — so the prompt never + appeared and every `dg keys --delete ID` reported "Cancelled by user" + without asking and without deleting. Prompting on stderr keeps stdout + parseable under `-o json`. + + Returns: + None to proceed with the delete, or the result to return instead + """ + if is_agentic() or not sys.stdin.isatty(): + # No one is there to answer. Refusing is right, but it is a usage + # error rather than something the user chose to cancel. + return BaseResult( + status="error", + message=( + f"Refusing to delete key {key_id} without confirmation. " + "Pass --yes to confirm." + ), + ) + + if not click.confirm( + f"Delete API key {key_id}? This cannot be undone.", + default=False, + err=True, + ): + return BaseResult(status="cancelled", message="Delete cancelled") + + return None diff --git a/packages/deepctl-cmd-keys/tests/unit/test_keys_command.py b/packages/deepctl-cmd-keys/tests/unit/test_keys_command.py index 437f131..6b8dcc0 100644 --- a/packages/deepctl-cmd-keys/tests/unit/test_keys_command.py +++ b/packages/deepctl-cmd-keys/tests/unit/test_keys_command.py @@ -1,6 +1,6 @@ """Tests for keys command.""" -from unittest.mock import Mock +from unittest.mock import Mock, patch import pytest from deepctl_cmd_keys.command import KeysCommand @@ -99,9 +99,7 @@ def test_handle_list_keys( assert result.keys[0].key_id == "key1" assert result.keys[0].comment == "test" assert result.keys[0].scopes == ["member"] - mock_client.list_keys.assert_called_once_with( - project_id=None, status=None - ) + mock_client.list_keys.assert_called_once_with(project_id=None, status=None) def test_handle_list_keys_empty( self, command, mock_config, mock_auth_manager, mock_client @@ -184,9 +182,7 @@ def test_handle_show_key( assert result.keys[0].comment == "production key" assert result.keys[0].scopes == ["admin"] assert result.keys[0].tags == ["prod"] - mock_client.get_key.assert_called_once_with( - "key-id", project_id=None - ) + mock_client.get_key.assert_called_once_with("key-id", project_id=None) def test_handle_delete_key( self, command, mock_config, mock_auth_manager, mock_client @@ -204,13 +200,9 @@ def test_handle_delete_key( assert result.status == "success" assert "key-id" in result.message assert "deleted" in result.message.lower() - mock_client.delete_key.assert_called_once_with( - "key-id", project_id=None - ) + mock_client.delete_key.assert_called_once_with("key-id", project_id=None) - def test_handle_error( - self, command, mock_config, mock_auth_manager, mock_client - ): + def test_handle_error(self, command, mock_config, mock_auth_manager, mock_client): """Test client raises exception, returns error.""" mock_client.list_keys.side_effect = Exception("API connection failed") @@ -224,6 +216,286 @@ def test_handle_error( assert "API connection failed" in result.message +class TestKeysOutputFormat: + """`-o json` must leave stdout parseable (#98). + + `keys` was the one account command missed when the other seven were fixed, + so `dg -o json keys | jq` still failed on "Fetching API keys..." arriving + ahead of the JSON. + """ + + @pytest.fixture + def command(self): + return KeysCommand() + + @staticmethod + def _keys_response(): + return { + "api_keys": [ + { + "api_key": { + "api_key_id": "key1", + "comment": "ci-runner", + "scopes": ["member"], + "created": "2026-01-01", + "expiration_date": "", + } + } + ] + } + + @patch("deepctl_cmd_keys.command.get_output_format", return_value="default") + @patch("deepctl_cmd_keys.command.status_console") + @patch("deepctl_cmd_keys.command.console") + def test_default_mode_table_on_stdout_chrome_on_stderr( + self, mock_console, mock_status_console, _fmt, command + ): + client = Mock(spec=DeepgramClient) + client.list_keys.return_value = self._keys_response() + + command._list_keys(client, None, None) + + stderr_text = " ".join( + str(c.args[0]) for c in mock_status_console.print.call_args_list if c.args + ) + # The table rides stdout for humans... + mock_console.print.assert_called() + # ...while progress chrome only ever goes to stderr. + assert "Fetching API keys" in stderr_text + + @patch("deepctl_cmd_keys.command.get_output_format", return_value="json") + @patch("deepctl_cmd_keys.command.status_console") + @patch("deepctl_cmd_keys.command.console") + def test_json_mode_renders_no_human_output( + self, mock_console, mock_status_console, _fmt, command + ): + client = Mock(spec=DeepgramClient) + client.list_keys.return_value = self._keys_response() + + result = command._list_keys(client, None, None) + + # Nothing human-facing on stdout — the framework serialises the result. + mock_console.print.assert_not_called() + assert result.count == 1 + assert result.keys[0].key_id == "key1" + + @patch("deepctl_cmd_keys.command.get_output_format", return_value="json") + @patch("deepctl_cmd_keys.command.status_console") + @patch("deepctl_cmd_keys.command.console") + def test_json_mode_created_key_only_in_result( + self, mock_console, mock_status_console, _fmt, command + ): + """The secret must reach the caller via the result, not stray prints.""" + client = Mock(spec=DeepgramClient) + client.create_key.return_value = { + "api_key_id": "new-id", + "key": "secret-value", + } + + result = command._create_key(client, None, comment="ci", scopes="member") + + mock_console.print.assert_not_called() + assert result.created_key.key == "secret-value" + assert result.created_key.key_id == "new-id" + + @patch("deepctl_cmd_keys.command.get_output_format", return_value="json") + @patch("deepctl_cmd_keys.command.status_console") + @patch("deepctl_cmd_keys.command.console") + def test_json_mode_show_key_renders_nothing( + self, mock_console, mock_status_console, _fmt, command + ): + client = Mock(spec=DeepgramClient) + client.get_key.return_value = { + "api_key": {"api_key_id": "key1", "comment": "c", "scopes": ["member"]} + } + + result = command._show_key(client, "key1", None) + + mock_console.print.assert_not_called() + assert result.keys[0].key_id == "key1" + + +class TestKeysDryRun: + """`--dry-run` must reach its own code, not a TypeError. + + `handle` read `project_id` and `dry_run` off kwargs with `.get()` and then + forwarded `**kwargs` alongside them, so every argument arrived twice and + `--create --dry-run` failed with "got multiple values for argument" before + the dry-run body ran. + """ + + @pytest.fixture + def command(self): + return KeysCommand() + + @pytest.fixture + def mock_client(self): + return Mock(spec=DeepgramClient) + + @patch("deepctl_cmd_keys.command.get_output_format", return_value="json") + @patch("deepctl_cmd_keys.command.status_console") + @patch("deepctl_cmd_keys.command.console") + def test_create_dry_run_reports_dry_run_and_calls_nothing( + self, mock_console, _status, _fmt, command, mock_client + ): + result = command.handle( + Mock(spec=Config), + Mock(spec=AuthManager), + mock_client, + create=True, + dry_run=True, + project_id=None, + comment="probe", + scopes="member", + ttl=3600, + tags="a,b", + ) + + assert result.status == "dry_run" + mock_client.create_key.assert_not_called() + mock_console.print.assert_not_called() + + @patch("deepctl_cmd_keys.command.get_output_format", return_value="default") + @patch("deepctl_cmd_keys.command.status_console") + @patch("deepctl_cmd_keys.command.console") + def test_create_dry_run_renders_summary_for_humans( + self, mock_console, _status, _fmt, command, mock_client + ): + result = command.handle( + Mock(spec=Config), + Mock(spec=AuthManager), + mock_client, + create=True, + dry_run=True, + project_id=None, + comment="probe", + scopes="member", + ) + + assert result.status == "dry_run" + printed = " ".join( + str(c.args[0]) for c in mock_console.print.call_args_list if c.args + ) + assert "Dry run" in printed + assert "probe" in printed + + @patch("deepctl_cmd_keys.command.get_output_format", return_value="json") + @patch("deepctl_cmd_keys.command.status_console") + @patch("deepctl_cmd_keys.command.console") + def test_delete_dry_run_reports_dry_run_and_calls_nothing( + self, mock_console, _status, _fmt, command, mock_client + ): + result = command.handle( + Mock(spec=Config), + Mock(spec=AuthManager), + mock_client, + delete="key-id", + dry_run=True, + project_id=None, + ) + + assert result.status == "dry_run" + assert "key-id" in result.message + mock_client.delete_key.assert_not_called() + mock_console.print.assert_not_called() + + +class TestKeysDeleteConfirmation: + """`--delete` without `--yes` must not silently no-op. + + `BaseCommand.confirm` returns its default whenever any parameter came from + the command line, and `--delete KEY_ID` is itself such a parameter — so the + prompt never appeared and the command reported "Cancelled by user" without + asking anyone and without deleting anything. + """ + + @pytest.fixture + def command(self): + return KeysCommand() + + @pytest.fixture + def mock_client(self): + return Mock(spec=DeepgramClient) + + @patch("deepctl_cmd_keys.command.status_console") + @patch("deepctl_cmd_keys.command.is_agentic", return_value=True) + def test_non_interactive_without_yes_errors_and_deletes_nothing( + self, _agentic, _status, command, mock_client + ): + result = command._delete_key(mock_client, "key-id", None, yes=False) + + assert result.status == "error" + assert "--yes" in result.message + mock_client.delete_key.assert_not_called() + + @patch("deepctl_cmd_keys.command.status_console") + @patch("deepctl_cmd_keys.command.is_agentic", return_value=False) + def test_interactive_declined_cancels_and_deletes_nothing( + self, _agentic, _status, command, mock_client + ): + with ( + patch("deepctl_cmd_keys.command.sys.stdin.isatty", return_value=True), + patch("click.confirm", return_value=False) as mock_confirm, + ): + result = command._delete_key(mock_client, "key-id", None, yes=False) + + assert result.status == "cancelled" + mock_client.delete_key.assert_not_called() + # The prompt goes to stderr, so stdout stays parseable under -o json. + assert mock_confirm.call_args.kwargs["err"] is True + + @patch("deepctl_cmd_keys.command.status_console") + @patch("deepctl_cmd_keys.command.is_agentic", return_value=False) + def test_interactive_accepted_deletes( + self, _agentic, _status, command, mock_client + ): + with ( + patch("deepctl_cmd_keys.command.sys.stdin.isatty", return_value=True), + patch("click.confirm", return_value=True), + ): + result = command._delete_key(mock_client, "key-id", None, yes=False) + + assert result.status == "success" + mock_client.delete_key.assert_called_once_with("key-id", project_id=None) + + @patch("deepctl_cmd_keys.command.status_console") + def test_yes_skips_confirmation_entirely(self, _status, command, mock_client): + with patch("click.confirm", side_effect=AssertionError("prompted anyway")): + result = command._delete_key(mock_client, "key-id", None, yes=True) + + assert result.status == "success" + mock_client.delete_key.assert_called_once_with("key-id", project_id=None) + + +class TestKeysErrorPath: + """Errors are chrome, not payload: stderr, and a non-zero exit code.""" + + @pytest.fixture + def command(self): + return KeysCommand() + + @patch("deepctl_cmd_keys.command.get_output_format", return_value="json") + @patch("deepctl_cmd_keys.command.status_console") + @patch("deepctl_cmd_keys.command.console") + def test_error_goes_to_stderr_and_maps_to_exit_1( + self, mock_console, mock_status_console, _fmt, command + ): + client = Mock(spec=DeepgramClient) + client.list_keys.side_effect = Exception("API connection failed") + + result = command.handle( + Mock(spec=Config), Mock(spec=AuthManager), client, project_id=None + ) + + assert result.status == "error" + mock_console.print.assert_not_called() + stderr_text = " ".join( + str(c.args[0]) for c in mock_status_console.print.call_args_list if c.args + ) + assert "API connection failed" in stderr_text + assert command.exit_code_for(result) == 1 + + class TestKeysModels: """Test cases for keys models.""" diff --git a/packages/deepctl-cmd-members/src/deepctl_cmd_members/command.py b/packages/deepctl-cmd-members/src/deepctl_cmd_members/command.py index 88cc12f..cef8aaf 100644 --- a/packages/deepctl-cmd-members/src/deepctl_cmd_members/command.py +++ b/packages/deepctl-cmd-members/src/deepctl_cmd_members/command.py @@ -11,6 +11,7 @@ Config, DeepgramClient, get_output_format, + get_status_console, ) from rich.console import Console from rich.table import Table @@ -20,7 +21,7 @@ console = Console() # Status/progress chrome must never touch stdout, or it corrupts JSON/CSV # output that callers pipe into jq and friends. -status_console = Console(stderr=True) +status_console = get_status_console() class MembersCommand(BaseCommand): diff --git a/packages/deepctl-cmd-models/src/deepctl_cmd_models/command.py b/packages/deepctl-cmd-models/src/deepctl_cmd_models/command.py index c128b2b..610cbff 100644 --- a/packages/deepctl-cmd-models/src/deepctl_cmd_models/command.py +++ b/packages/deepctl-cmd-models/src/deepctl_cmd_models/command.py @@ -11,6 +11,7 @@ Config, DeepgramClient, get_output_format, + get_status_console, ) from rich.console import Console from rich.table import Table @@ -20,7 +21,7 @@ console = Console() # Status/progress chrome must never touch stdout, or it corrupts JSON/CSV # output that callers pipe into jq and friends. -status_console = Console(stderr=True) +status_console = get_status_console() class ModelsCommand(BaseCommand): diff --git a/packages/deepctl-cmd-projects/src/deepctl_cmd_projects/command.py b/packages/deepctl-cmd-projects/src/deepctl_cmd_projects/command.py index fbdcdf5..780cf62 100644 --- a/packages/deepctl-cmd-projects/src/deepctl_cmd_projects/command.py +++ b/packages/deepctl-cmd-projects/src/deepctl_cmd_projects/command.py @@ -9,6 +9,7 @@ Config, DeepgramClient, get_output_format, + get_status_console, ) from rich.console import Console @@ -17,7 +18,7 @@ console = Console() # Status/progress chrome must never touch stdout, or it corrupts JSON/CSV # output that callers pipe into jq and friends. -status_console = Console(stderr=True) +status_console = get_status_console() class ProjectsCommand(BaseCommand): diff --git a/packages/deepctl-cmd-read/src/deepctl_cmd_read/command.py b/packages/deepctl-cmd-read/src/deepctl_cmd_read/command.py index 613cf28..a51bcb9 100644 --- a/packages/deepctl-cmd-read/src/deepctl_cmd_read/command.py +++ b/packages/deepctl-cmd-read/src/deepctl_cmd_read/command.py @@ -13,6 +13,7 @@ Config, DeepgramClient, get_output_format, + get_status_console, ) from rich.console import Console @@ -21,7 +22,7 @@ console = Console() # Status/progress chrome must never touch stdout, or it corrupts JSON/CSV # output that callers pipe into jq and friends. -status_console = Console(stderr=True) +status_console = get_status_console() class ReadCommand(BaseCommand): diff --git a/packages/deepctl-cmd-requests/src/deepctl_cmd_requests/command.py b/packages/deepctl-cmd-requests/src/deepctl_cmd_requests/command.py index 51c47cc..d3ac77a 100644 --- a/packages/deepctl-cmd-requests/src/deepctl_cmd_requests/command.py +++ b/packages/deepctl-cmd-requests/src/deepctl_cmd_requests/command.py @@ -12,6 +12,7 @@ Config, DeepgramClient, get_output_format, + get_status_console, ) from rich.console import Console from rich.table import Table @@ -21,7 +22,7 @@ console = Console() # Status/progress chrome must never touch stdout, or it corrupts JSON/CSV # output that callers pipe into jq and friends. -status_console = Console(stderr=True) +status_console = get_status_console() class RequestsCommand(BaseCommand): diff --git a/packages/deepctl-cmd-usage/src/deepctl_cmd_usage/command.py b/packages/deepctl-cmd-usage/src/deepctl_cmd_usage/command.py index caf72df..26f2e7b 100644 --- a/packages/deepctl-cmd-usage/src/deepctl_cmd_usage/command.py +++ b/packages/deepctl-cmd-usage/src/deepctl_cmd_usage/command.py @@ -10,6 +10,7 @@ Config, DeepgramClient, get_output_format, + get_status_console, ) from deepctl_shared_utils import validate_date_format from rich.console import Console @@ -19,7 +20,7 @@ console = Console() # Status/progress chrome must never touch stdout, or it corrupts JSON/CSV # output that callers pipe into jq and friends. -status_console = Console(stderr=True) +status_console = get_status_console() class UsageCommand(BaseCommand): diff --git a/packages/deepctl-core/src/deepctl_core/__init__.py b/packages/deepctl-core/src/deepctl_core/__init__.py index dc7663a..6a6f86d 100644 --- a/packages/deepctl-core/src/deepctl_core/__init__.py +++ b/packages/deepctl-core/src/deepctl_core/__init__.py @@ -16,6 +16,7 @@ OutputFormatter, get_console, get_output_format, + get_status_console, is_agentic, print_error, print_info, @@ -51,6 +52,7 @@ "enable_timing", "get_console", "get_output_format", + "get_status_console", "get_timing_summary", "is_agentic", "is_timing_enabled", diff --git a/packages/deepctl-core/src/deepctl_core/base_command.py b/packages/deepctl-core/src/deepctl_core/base_command.py index 1ea6ebb..38f8b69 100644 --- a/packages/deepctl-core/src/deepctl_core/base_command.py +++ b/packages/deepctl-core/src/deepctl_core/base_command.py @@ -29,6 +29,19 @@ class BaseCommand(ABC): requires_project: bool = False ci_friendly: bool = True + # Result status -> process exit code, per the contract published in + # llms-full.txt: 0 = success, 1 = error, 2 = user interrupt. A command that + # reports failure must not exit 0 -- scripts and CI branch on the exit code, + # not on the "status" field inside the payload. "cancelled" is 2 rather than + # the shell's 130 to match that contract and main.py's own KeyboardInterrupt + # handler; `dg mcp` in particular returns cancelled straight out of its + # KeyboardInterrupt path. Any status not listed here exits 0 + # (success, succeeded, info, dry_run, warning). + EXIT_CODES: ClassVar[dict[str, int]] = { + "error": 1, + "cancelled": 2, + } + # Agent-oriented metadata examples: ClassVar[list[str]] = [] agent_help: str = "" @@ -147,6 +160,23 @@ def execute(self, ctx: click.Context, **kwargs: Any) -> None: stderr_console.print_exception() raise click.ClickException(str(e)) + # Payload is already on stdout; now make the exit code agree with it. + exit_code = self.exit_code_for(result) + if exit_code: + raise SystemExit(exit_code) + + def exit_code_for(self, result: Any) -> int: + """Map a command result to a process exit code. + + Args: + result: Command result (may be None) + + Returns: + Exit code for this result's status; 0 when it reported no failure + """ + status = str(getattr(result, "status", "") or "") + return self.EXIT_CODES.get(status, 0) + def is_guided(self, ctx: click.Context) -> bool: """True only when the user invoked this command with no input at all. @@ -285,9 +315,25 @@ def output_result(self, result: Any, config: Config) -> None: elif output_format == "csv": self._output_csv(result) else: - console.print(f"[red]Unknown output format:[/red] {output_format}") + # Diagnostics belong on stderr even here, or the complaint about + # the format corrupts the payload we fall back to. + stderr_console.print( + f"[red]Unknown output format:[/red] {output_format}" + ) self._output_json(result) + def _write_payload(self, text: str) -> None: + """Write already-serialised machine-readable text to stdout verbatim. + + Rich's default print would treat square brackets as style markup and + silently delete them, so a key comment of "[ci] runner" would lose the + "[ci]". It would also hard-wrap at the console width, injecting newlines + into the middle of a value. Both corrupt the payload, so markup, + highlighting and wrapping are all off. Routed through `console` rather + than `sys.stdout` so `--quiet` still suppresses output. + """ + console.print(text, markup=False, highlight=False, soft_wrap=True) + def _output_json(self, result: Any) -> None: """Output result as JSON.""" import json @@ -310,9 +356,11 @@ def _output_yaml(self, result: Any) -> None: import yaml if isinstance(result, dict | list): - console.print(yaml.dump(result, default_flow_style=False)) + self._write_payload(yaml.dump(result, default_flow_style=False)) else: - console.print(yaml.dump({"result": str(result)}, default_flow_style=False)) + self._write_payload( + yaml.dump({"result": str(result)}, default_flow_style=False) + ) def _output_table(self, result: Any) -> None: """Output result as table.""" @@ -359,7 +407,7 @@ def _output_csv(self, result: Any) -> None: dict_writer = csv.DictWriter(output, fieldnames=result[0].keys()) dict_writer.writeheader() dict_writer.writerows(result) - console.print(output.getvalue()) + self._write_payload(output.getvalue()) elif isinstance(result, dict): # Dictionary @@ -368,7 +416,7 @@ def _output_csv(self, result: Any) -> None: writer.writerow(["Key", "Value"]) for key, value in result.items(): writer.writerow([key, value]) - console.print(output.getvalue()) + self._write_payload(output.getvalue()) else: # Fallback to JSON diff --git a/packages/deepctl-core/src/deepctl_core/output.py b/packages/deepctl-core/src/deepctl_core/output.py index 685cb72..f7469db 100644 --- a/packages/deepctl-core/src/deepctl_core/output.py +++ b/packages/deepctl-core/src/deepctl_core/output.py @@ -505,3 +505,19 @@ def get_console() -> Console: Console instance """ return console + + +def get_status_console() -> Console: + """Get the console for status/progress chrome. + + Always writes to stderr, so anything printed through it can never corrupt + the machine-readable payload a command writes to stdout. Commands should + use this rather than declaring their own ``Console(stderr=True)`` — a + per-command console silently misses the agentic no-color settings, and a + command that reaches for a bare ``Console()`` reintroduces the stdout + pollution this exists to prevent. + + Returns: + Console instance writing to stderr + """ + return stderr_console diff --git a/packages/deepctl-core/tests/unit/test_base.py b/packages/deepctl-core/tests/unit/test_base.py index f2d3086..d494e0b 100644 --- a/packages/deepctl-core/tests/unit/test_base.py +++ b/packages/deepctl-core/tests/unit/test_base.py @@ -6,7 +6,13 @@ import click import pytest from click.testing import CliRunner -from deepctl_core import AuthManager, BaseCommand, Config, DeepgramClient +from deepctl_core import ( + AuthManager, + BaseCommand, + BaseResult, + Config, + DeepgramClient, +) # Mirrors the full key set of deepctl_core.output._output_config so a patched # stand-in can't drift from the real global. Spread with a format override, @@ -607,7 +613,11 @@ def test_output_result_yaml(self, mock_console, mock_command_class): import yaml expected_yaml = yaml.dump(result, default_flow_style=False) - mock_console.print.assert_called_once_with(expected_yaml) + # Machine-readable payloads go out verbatim: Rich must not interpret + # markup, highlight, or wrap them. + mock_console.print.assert_called_once_with( + expected_yaml, markup=False, highlight=False, soft_wrap=True + ) @pytest.mark.unit @patch("deepctl_core.base_command.console") @@ -688,12 +698,15 @@ def test_output_result_csv_dict(self, mock_console, mock_command_class): assert "age,30" in output @pytest.mark.unit + @patch("deepctl_core.base_command.stderr_console") @patch("deepctl_core.base_command.console") @patch( "deepctl_core.output._output_config", {**_OUTPUT_CONFIG_DEFAULTS, "format": "unknown"}, ) - def test_output_result_unknown_format(self, mock_console, mock_command_class): + def test_output_result_unknown_format( + self, mock_console, mock_stderr_console, mock_command_class + ): """Test output_result with unknown format falls back to JSON.""" command = mock_command_class() config = Mock(spec=Config) @@ -703,10 +716,12 @@ def test_output_result_unknown_format(self, mock_console, mock_command_class): with patch.object(command, "_output_json") as mock_output_json: command.output_result(result, config) - # Verify error message and fallback to JSON - mock_console.print.assert_called_once_with( + # The complaint goes to stderr so it cannot corrupt the JSON we fall + # back to on stdout. + mock_stderr_console.print.assert_called_once_with( "[red]Unknown output format:[/red] unknown" ) + mock_console.print.assert_not_called() mock_output_json.assert_called_once_with(result) @pytest.mark.unit @@ -1057,6 +1072,149 @@ def test_execute_sets_guided_true_for_bare_invocation( assert mock_command_class.captured_guided is True +class TestExitCodes: + """A command that reports failure must not exit 0 (#98). + + Scripts and CI branch on the exit code, not on the "status" field buried in + the payload, so `if dg -o json keys; then` took the success branch on a + failed call. + """ + + @pytest.fixture + def command(self): + class MockCommand(BaseCommand): + name = "test" + help = "Test command" + result: Any = None + + def handle(self, *args, **kwargs): + return MockCommand.result + + return MockCommand + + @pytest.mark.unit + @pytest.mark.parametrize( + ("status", "expected"), + [ + ("error", 1), + # 2, not the shell's 130: llms-full.txt publishes + # "0 = success, 1 = error, 2 = user interrupt", and main.py's + # KeyboardInterrupt handler already exits 2. + ("cancelled", 2), + ("success", 0), + ("succeeded", 0), + ("info", 0), + ("dry_run", 0), + ("warning", 0), + ("anything-unrecognised", 0), + ], + ) + def test_exit_code_for_status(self, command, status, expected): + assert command().exit_code_for(BaseResult(status=status)) == expected + + @pytest.mark.unit + def test_exit_code_for_result_without_status(self, command): + assert command().exit_code_for(None) == 0 + assert command().exit_code_for({"not": "a result"}) == 0 + + @pytest.mark.unit + @patch("deepctl_core.base_command.AuthManager") + @patch("deepctl_core.base_command.DeepgramClient") + def test_execute_exits_nonzero_on_error_result( + self, _client_class, _auth_class, command + ): + cmd = command() + command.result = BaseResult(status="error", message="API connection failed") + + with pytest.raises(SystemExit) as exc: + cmd.execute(_exit_code_ctx()) + + assert exc.value.code == 1 + + @pytest.mark.unit + @patch("deepctl_core.base_command.AuthManager") + @patch("deepctl_core.base_command.DeepgramClient") + def test_execute_exits_zero_on_success_result( + self, _client_class, _auth_class, command + ): + cmd = command() + command.result = BaseResult(status="success", message="done") + + # No SystemExit at all — a successful command must fall through. + cmd.execute(_exit_code_ctx()) + + +class TestMachineReadablePayloads: + """yaml/csv payloads must reach stdout byte-for-byte. + + Rich's default print treats square brackets as style markup and deletes + them, and hard-wraps at the console width; both silently corrupt user data + such as an API key comment of "[ci] runner". + """ + + @pytest.fixture + def command(self): + class MockCommand(BaseCommand): + name = "test" + help = "Test command" + + def handle(self, *args, **kwargs): + return None + + return MockCommand() + + @staticmethod + def _capture(command, method, payload): + import io + + from rich.console import Console + + buffer = io.StringIO() + # Deliberately narrow: a wrapping console would break the long value. + with patch( + "deepctl_core.base_command.console", + Console(file=buffer, width=40, no_color=True), + ): + getattr(command, method)(payload) + return buffer.getvalue() + + @pytest.mark.unit + def test_yaml_preserves_brackets_and_does_not_wrap(self, command): + comment = "[bold red]staging[/bold red] key " + "x" * 60 + out = self._capture(command, "_output_yaml", {"comment": comment}) + + assert "[bold red]staging[/bold red] key" in out + assert "x" * 60 in out + + @pytest.mark.unit + def test_csv_preserves_brackets_and_does_not_wrap(self, command): + comment = "[ci] runner " + "y" * 60 + out = self._capture(command, "_output_csv", {"comment": comment}) + + assert "[ci] runner" in out + assert "y" * 60 in out + + @pytest.mark.unit + def test_json_preserves_brackets(self, command): + out = self._capture(command, "_output_json", {"comment": "[ci] runner"}) + + assert "[ci] runner" in out + + +def _exit_code_ctx(): + """A minimal Click context good enough to drive BaseCommand.execute.""" + ctx = Mock(spec=click.Context) + ctx.obj = {"config": Config()} + ctx.command_path = "deepctl test" + ctx.params = {} + ctx.command = Mock() + ctx.command.params = [] + src = Mock() + src.name = "DEFAULT" + ctx.get_parameter_source = lambda _name: src + return ctx + + class TestConfirmPromptGating: """Verify confirm() and prompt() respect _guided alongside _agentic and ci_friendly.""" diff --git a/src/deepctl/main.py b/src/deepctl/main.py index c5828f9..db6aa0e 100644 --- a/src/deepctl/main.py +++ b/src/deepctl/main.py @@ -361,12 +361,18 @@ def main() -> None: # Preprocess arguments to handle hyphenated commands processed_args = preprocess_hyphenated_commands(args) + exit_code = 0 with TimingContext("cli_execution"), _telemetry_transaction(): try: cli(args=processed_args, standalone_mode=False) - except SystemExit: - # Click calls sys.exit() even in non-standalone mode - pass + except SystemExit as exc: + # Click calls sys.exit() even in non-standalone mode, so we + # cannot let this propagate — the notifications and timing + # summary below still need to run. But swallowing the code + # outright made every failure exit 0, which is what broke + # `if dg -o json keys; then ...` in scripts and CI. Carry it + # and re-raise at the end instead. + exit_code = exc.code if isinstance(exc.code, int) else 0 # Print update notifications if available (before timing summary) if print_pending_notification is not None: @@ -380,6 +386,9 @@ def main() -> None: if timing_requested: print_timing_summary(detailed_timing) + if exit_code: + sys.exit(exit_code) + except KeyboardInterrupt: _safe_console_print("\n[yellow]Operation cancelled by user[/yellow]") sys.exit(2)