Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions src/opensquilla/cli/agents_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from rich.table import Table

from opensquilla.agents.registry import AgentRegistry
from opensquilla.cli.output import emit_error
from opensquilla.onboarding.config_store import default_config_path, load_config, persist_config
from opensquilla.session.keys import normalize_agent_id

Expand All @@ -37,8 +38,12 @@ def _load_registry(config_path: Path | None) -> tuple[Path, Any, AgentRegistry]:
return target, cfg, AgentRegistry(cfg, config_path=target, persist_changes=False)


def _fail(exc: Exception) -> None:
typer.secho(f"Error: {exc}", fg=typer.colors.RED, err=True)
def _fail(exc: Exception, *, json_output: bool = False) -> None:
# The registry distinguishes the two failure kinds by exception type:
# _require_index raises KeyError for a missing agent, while a builtin-agent
# rejection or a duplicate id raises ValueError.
code = "NOT_FOUND" if isinstance(exc, KeyError) else "INVALID_ARGUMENT"
emit_error(str(exc), json_output=json_output, code=code)
raise typer.Exit(code=2) from exc


Expand Down Expand Up @@ -105,7 +110,7 @@ def agents_add(
)
)
except (ValueError, KeyError) as exc:
_fail(exc)
_fail(exc, json_output=json_output)

persist = _persist_agents_config(cfg, target, quiet=json_output)
if json_output:
Expand Down Expand Up @@ -134,7 +139,7 @@ def agents_delete(
try:
asyncio.run(registry.delete_agent(agent_id))
except (ValueError, KeyError) as exc:
_fail(exc)
_fail(exc, json_output=json_output)

persist = _persist_agents_config(cfg, target, quiet=json_output)
payload = {
Expand Down
4 changes: 2 additions & 2 deletions src/opensquilla/cli/channels_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
parse_channel_field_pairs,
)
from opensquilla.cli.gateway_rpc import confirm_or_exit, run_gateway_sync
from opensquilla.cli.output import print_json
from opensquilla.cli.output import emit_error, print_json
from opensquilla.cli.ui import ACCENT_HEADER, ACCENT_MARKUP
from opensquilla.cli.ui import console as ui_console
from opensquilla.onboarding.channel_specs import (
Expand Down Expand Up @@ -423,7 +423,7 @@ def channels_describe(
try:
spec = get_channel_setup_spec(type_name)
except KeyError as exc:
typer.secho(f"Error: {exc}", fg=typer.colors.RED, err=True)
emit_error(str(exc), json_output=json_output, code="NOT_FOUND")
raise typer.Exit(code=2) from exc

if json_output:
Expand Down
41 changes: 41 additions & 0 deletions tests/test_cli/test_agents_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,44 @@ def test_agents_delete_force_json_removes_config_entry(tmp_path, monkeypatch):
"stateDeleted": False,
}
assert load_config(target).agents == []


def test_agents_delete_json_emits_structured_error_for_missing_agent(tmp_path, monkeypatch):
"""--json must emit one JSON document on the failure path.

`sessions show --json` already does this; agents/channels bypassed it and
printed the human renderer's "Error: ..." line instead.
"""
_setenv(monkeypatch, tmp_path)

result = runner.invoke(app, ["agents", "delete", "qa-does-not-exist", "--force", "--json"])

assert result.exit_code == 2
payload = json.loads(result.stderr)
assert payload["error"]["code"] == "NOT_FOUND"
assert "qa-does-not-exist" in payload["error"]["message"]


def test_agents_add_json_emits_structured_error_for_duplicate(tmp_path, monkeypatch):
"""The same failure path in `agents add`, which also advertises --json."""
_setenv(monkeypatch, tmp_path)
first = runner.invoke(app, ["agents", "add", "ops", "--json"])
assert first.exit_code == 0, first.stdout

result = runner.invoke(app, ["agents", "add", "ops", "--json"])

assert result.exit_code == 2
payload = json.loads(result.stderr)
assert "already exists" in payload["error"]["message"]


def test_agents_delete_without_json_keeps_plain_text_error(tmp_path, monkeypatch):
"""Control: the human path must be byte-for-byte what it always was."""
_setenv(monkeypatch, tmp_path)

result = runner.invoke(app, ["agents", "delete", "qa-does-not-exist", "--force"])

assert result.exit_code == 2
combined = result.stdout + (result.stderr or "")
assert "Error: " in combined
assert "{" not in combined
25 changes: 25 additions & 0 deletions tests/test_cli/test_channels_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from __future__ import annotations

import json
from pathlib import Path

from typer.testing import CliRunner
Expand Down Expand Up @@ -444,3 +445,27 @@ def test_pairings_approve_admin_flag_passes_as_admin(tmp_path, monkeypatch):
)
]
assert "[admin]" in result.stdout


def test_channels_describe_json_emits_structured_error_for_unknown_type(tmp_path, monkeypatch):
"""`channels describe --json` printed a plain "Error: ..." line for an unknown type."""
_setenv(monkeypatch, tmp_path)

result = runner.invoke(app, ["channels", "describe", "qa-does-not-exist", "--json"])

assert result.exit_code == 2
payload = json.loads(result.stderr)
assert payload["error"]["code"] == "NOT_FOUND"
assert "qa-does-not-exist" in payload["error"]["message"]


def test_channels_describe_without_json_keeps_plain_text_error(tmp_path, monkeypatch):
"""Control: the human path is unchanged."""
_setenv(monkeypatch, tmp_path)

result = runner.invoke(app, ["channels", "describe", "qa-does-not-exist"])

assert result.exit_code == 2
combined = result.stdout + (result.stderr or "")
assert "Error: " in combined
assert "{" not in combined
Loading