diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a76a7292..e5b665f6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -129,6 +129,15 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). `pr-merge-pool-supervisor` to the product-builder's supervisor launch list (18 total supervisors). Updated all numeric references, pre-flight checklists, and validation logic. +--- +### Fixed + +- **CLI (`agents actor remove`)** (#6491): Restores output parity with the + other actor commands by honoring `--format`/`-f` for JSON/YAML/plain/Rich + envelopes. Adds a Robot Framework regression test to assert the JSON + envelope structure and updates the CLI synopsis in `docs/specification.md` + to document the option. + --- ## [3.8.0] — 2026-04-05 diff --git a/docs/specification.md b/docs/specification.md index dbe7bd381..d9cdea69a 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -277,7 +277,7 @@ The following standards are integrated into the architecture: [(--temperature|-t) <TEMP>] [--allow-rxpy-in-run-mode] [--skill <SKILL>]... <NAME> <PROMPT> agents actor add --config|-c <FILE> [--update] -agents actor remove <NAME> +agents actor remove [--format <FORMAT>] <NAME> agents actor list agents actor show <NAME> agents actor context remove [--yes|-y] (--all|-a|<NAME>) diff --git a/features/actor_cli_yaml.feature b/features/actor_cli_yaml.feature index 36192eb68..001201e93 100644 --- a/features/actor_cli_yaml.feature +++ b/features/actor_cli_yaml.feature @@ -61,6 +61,11 @@ Feature: Actor CLI YAML-first alignment When I run actor remove with namespaced name Then the actor remove should succeed for namespaced name + Scenario: Actor remove outputs JSON format + Given an actor CLI runner + When I run actor remove with format json + Then the actor remove output should be valid JSON envelope + Scenario: Actor update outputs JSON format Given an actor CLI runner When I run actor update with format json diff --git a/features/steps/actor_cli_yaml_steps.py b/features/steps/actor_cli_yaml_steps.py index 2fe25b5f0..a4fa371e2 100644 --- a/features/steps/actor_cli_yaml_steps.py +++ b/features/steps/actor_cli_yaml_steps.py @@ -337,6 +337,63 @@ def step_remove_namespaced_ok(context: Any) -> None: ) +@when("I run actor remove with format json") +def step_remove_format_json(context: Any) -> None: + with ( + patch("cleveragents.cli.commands.actor._get_services") as mock_svc, + patch("cleveragents.cli.commands.actor._compute_actor_impact") as mock_impact, + ): + mock_registry = MagicMock() + mock_service = MagicMock() + actor = _make_actor( + name="local/remove-json", + provider="json-provider", + model="gpt-json", + ) + mock_registry.get_actor.return_value = actor + mock_impact.return_value = (2, 1, 3) + mock_svc.return_value = (mock_service, mock_registry) + + context.result = context.runner.invoke( + actor_app, + ["remove", actor.name, "--format", "json"], + ) + + context.mock_actor_registry = mock_registry + context.actor = actor + context.impact_counts = (2, 1, 3) + + +@then("the actor remove output should be valid JSON envelope") +def step_remove_json_valid(context: Any) -> None: + assert context.result.exit_code == 0 + parsed = json.loads(context.result.output.strip()) + assert _ENVELOPE_KEYS.issubset(parsed.keys()) + assert parsed["command"] == f"agents actor remove {context.actor.name}" + assert parsed["status"] == "ok" + assert parsed["exit_code"] == 0 + data = _unwrap_envelope(parsed) + assert isinstance(data, dict) + actor_data = data.get("actor_removed", {}) + assert actor_data.get("name") == context.actor.name + assert actor_data.get("provider") == context.actor.provider + assert actor_data.get("model") == context.actor.model + impact = data.get("impact", {}) + expected_sessions, expected_plans, expected_actions = context.impact_counts + assert impact.get("sessions") == expected_sessions + assert impact.get("active_plans") == expected_plans + assert impact.get("actions_referencing") == expected_actions + cleanup = data.get("cleanup", {}) + assert cleanup.get("config") == "kept on disk" + assert cleanup.get("contexts") == "0 orphaned" + messages = parsed.get("messages", []) + assert messages, "expected messages in envelope" + first_message = messages[0] + assert first_message.get("level") == "ok" + assert "Actor removed" in first_message.get("text", "") + context.mock_actor_registry.remove_actor.assert_called_once_with(context.actor.name) + + # ------------------------------------------------------------------ # Update with --format # ------------------------------------------------------------------ diff --git a/robot/actor_remove_cli.robot b/robot/actor_remove_cli.robot new file mode 100644 index 000000000..8e66a3f11 --- /dev/null +++ b/robot/actor_remove_cli.robot @@ -0,0 +1,18 @@ +*** Settings *** +Documentation Integration tests for actor remove CLI format output +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_actor_remove_cli.py + +*** Test Cases *** +Actor Remove Format JSON Emits Spec Envelope + [Documentation] Verify that ``actor remove --format json`` emits a spec-compliant envelope + [Tags] actor_remove_cli_format + ${result}= Run Process ${PYTHON} ${HELPER} remove-json cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} actor-remove-json-format-ok diff --git a/robot/helper_actor_remove_cli.py b/robot/helper_actor_remove_cli.py new file mode 100644 index 000000000..4e37fc877 --- /dev/null +++ b/robot/helper_actor_remove_cli.py @@ -0,0 +1,164 @@ +"""Helper script for Robot integration tests covering ``actor remove --format`` output. + +Exercises the real ``agents`` CLI via subprocess — no mocking of any kind. +A test actor is seeded via ``agents actor add``, then removed via +``agents actor remove --format json``, and the resulting JSON envelope is +validated against the spec. + +Usage:: + + python helper_actor_remove_cli.py + +Where *command* is one of: + +- ``remove-json`` — seed an actor, remove it with ``--format json``, validate envelope +""" + +from __future__ import annotations + +import json +import os +import sys +from pathlib import Path + +# Ensure src is importable when run from workspace root +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +# Ensure robot/ is on the import path for helper_e2e_common. +_ROBOT = str(Path(__file__).resolve().parent) +if _ROBOT not in sys.path: + sys.path.insert(0, _ROBOT) + +from helper_e2e_common import cleanup_workspace, run_cli, setup_workspace # noqa: E402 + +_ACTOR_NAME = "local/robot-remove-actor" + +_ACTOR_CONFIG: dict[str, object] = { + "name": _ACTOR_NAME, + "provider": "openai", + "model": "gpt-4", +} + + +def _write_actor_config(workspace: str) -> str: + """Write actor config JSON to a temp file and return its path.""" + config_path = os.path.join(workspace, "robot_remove_actor.json") + with open(config_path, "w", encoding="utf-8") as fh: + json.dump(_ACTOR_CONFIG, fh) + return config_path + + +def test_remove_format_json() -> None: + """Seed an actor via the real CLI, remove it with ``--format json``. + + Validates the resulting JSON envelope against the spec. + """ + workspace = setup_workspace(prefix="robot_actor_remove_") + try: + config_path = _write_actor_config(workspace) + + # Step 1: Add the actor using the real CLI. + add_result = run_cli( + "actor", + "add", + _ACTOR_NAME, + "--config", + config_path, + workspace=workspace, + ) + assert add_result.returncode == 0, ( + f"actor add failed (rc={add_result.returncode}):\n" + f"stdout: {add_result.stdout}\nstderr: {add_result.stderr}" + ) + + # Step 2: Remove the actor with --format json using the real CLI. + remove_result = run_cli( + "actor", + "remove", + _ACTOR_NAME, + "--format", + "json", + workspace=workspace, + ) + assert remove_result.returncode == 0, ( + f"actor remove --format json failed (rc={remove_result.returncode}):\n" + f"stdout: {remove_result.stdout}\nstderr: {remove_result.stderr}" + ) + + # Step 3: Parse and validate the JSON envelope. + output = remove_result.stdout.strip() + assert output, ( + f"actor remove --format json produced no output.\n" + f"stderr: {remove_result.stderr}" + ) + + payload = json.loads(output) + + assert payload["command"] == f"agents actor remove {_ACTOR_NAME}", ( + f"Unexpected command field: {payload.get('command')!r}" + ) + assert payload["status"] == "ok", ( + f"Unexpected status: {payload.get('status')!r}" + ) + assert payload["exit_code"] == 0, ( + f"Unexpected exit_code: {payload.get('exit_code')!r}" + ) + + data = payload["data"] + + removed = data.get("actor_removed", {}) + assert removed.get("name") == _ACTOR_NAME, ( + f"actor_removed.name mismatch: {removed.get('name')!r}" + ) + assert removed.get("provider") == _ACTOR_CONFIG["provider"], ( + f"actor_removed.provider mismatch: {removed.get('provider')!r}" + ) + assert removed.get("model") == _ACTOR_CONFIG["model"], ( + f"actor_removed.model mismatch: {removed.get('model')!r}" + ) + + impact = data.get("impact", {}) + assert "sessions" in impact, f"Missing 'sessions' in impact: {impact}" + assert "active_plans" in impact, f"Missing 'active_plans' in impact: {impact}" + assert "actions_referencing" in impact, ( + f"Missing 'actions_referencing' in impact: {impact}" + ) + + cleanup = data.get("cleanup", {}) + assert cleanup.get("config") == "kept on disk", ( + f"cleanup.config mismatch: {cleanup.get('config')!r}" + ) + assert "contexts" in cleanup, f"Missing 'contexts' in cleanup: {cleanup}" + + messages = payload.get("messages", []) + assert messages, f"Expected non-empty messages list, got: {messages}" + assert messages[0].get("level") == "ok", ( + f"Unexpected message level: {messages[0].get('level')!r}" + ) + assert "Actor removed" in messages[0].get("text", ""), ( + f"Expected 'Actor removed' in message text: {messages[0].get('text')!r}" + ) + + print("actor-remove-json-format-ok") + + finally: + cleanup_workspace(workspace) + + +def main() -> None: + command = sys.argv[1] if len(sys.argv) > 1 else "remove-json" + dispatch: dict[str, object] = { + "remove-json": test_remove_format_json, + } + handler = dispatch.get(command) + if handler is None: + print(f"Unknown command: {command}", file=sys.stderr) + sys.exit(1) + assert callable(handler) + handler() + + +if __name__ == "__main__": + main() diff --git a/src/cleveragents/cli/commands/actor.py b/src/cleveragents/cli/commands/actor.py index 661138475..1ae46cc96 100644 --- a/src/cleveragents/cli/commands/actor.py +++ b/src/cleveragents/cli/commands/actor.py @@ -779,12 +779,28 @@ def update( @app.command() -def remove(name: Annotated[str, typer.Argument(help="Actor name to remove")]) -> None: +def remove( + name: Annotated[str, typer.Argument(help="Actor name to remove")], + fmt: Annotated[ + str, + typer.Option("--format", "-f", help=_FORMAT_HELP), + ] = OutputFormat.RICH.value, +) -> None: """Remove a custom actor. Specify the namespaced name (e.g. ``local/my-actor``). """ + # Validate --format argument first; fail fast before any side effects. + fmt_value = fmt.lower() + _valid_formats = {f.value for f in OutputFormat} + if fmt_value not in _valid_formats: + raise typer.BadParameter( + f"Invalid format {fmt!r}. " + f"Supported values: {', '.join(sorted(_valid_formats))}", + param_hint="'--format'", + ) + service, registry = _get_services() try: # Get actor details before removal for display @@ -807,6 +823,40 @@ def remove(name: Annotated[str, typer.Argument(help="Actor name to remove")]) -> else: service.remove_actor(name) + command_name = f"agents actor remove {name}" + payload = { + "actor_removed": { + "name": name, + "provider": actor_provider, + "model": actor_model, + }, + "impact": { + "sessions": session_count, + "active_plans": active_plan_count, + "actions_referencing": action_count, + }, + "cleanup": { + "config": "kept on disk", + # NOTE: context-cleanup count is deferred; always 0 for now. + # Follow-up: implement dynamic orphaned-context detection. + "contexts": "0 orphaned", + }, + } + messages = [{"level": "ok", "text": "Actor removed"}] + + if fmt_value != OutputFormat.RICH.value: + rendered = format_output( + payload, + fmt_value, + command=command_name, + status="ok", + exit_code=0, + messages=messages, + ) + if rendered: + console.print(rendered) + return + # Display Actor Removed panel actor_info = ( f"[cyan bold]Name:[/cyan bold] {name}\n"