diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a8b5b1cb..d042e5f86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -125,6 +125,17 @@ retention controls via `max_audit_log_size` and `clear_audit_log()`. Includes expanded Behave and Robot integration coverage, 5 ASV benchmark suites, and `vulture_whitelist.py` entry. (#587) +- **Breaking (CLI):** `agents actor run` now takes positional `` and + `` arguments, aligning the command signature with the specification. + The previous `--prompt/-p` option is removed. `--config/-c` is preserved as + an optional fallback that overrides registry-based name resolution (spec + deviation documented in `_resolve_actor.py`). Shared resolution logic + extracted to `_resolve_actor.py` with `yaml.safe_dump`, early name + validation, input sanitisation, resilient `atexit`-based temp file cleanup, + and graceful error handling for missing actors, empty config data, and + non-serialisable config blobs (without exposing serializer internals in + user-facing errors). Comprehensive BDD and Robot Framework + tests cover all resolution paths and edge cases. (#901) - Added BuiltinAdapter class and MCP automatic resource slot creation. BuiltinAdapter wraps register_file_tools/register_git_tools/register_subplan_tool into a unified adapter interface. McpAdapter.infer_resource_slots() analyzes diff --git a/features/actor_run_signature.feature b/features/actor_run_signature.feature new file mode 100644 index 000000000..5f5318201 --- /dev/null +++ b/features/actor_run_signature.feature @@ -0,0 +1,130 @@ +@coverage @coverage_actor_run_signature +Feature: Actor run command positional arguments + The `agents actor run` command accepts positional NAME and PROMPT arguments + per the specification. The --config/-c option is preserved as an optional + fallback for direct YAML invocation. + + Background: + Given an actor CLI runner + And I have an actor JSON config file + + # ---------- Positional NAME and PROMPT arguments ---------- + + Scenario: Run with positional NAME and PROMPT using --config fallback + When I run actor run with positional args and config fallback + Then the actor run should call single shot without context manager + + Scenario: Run with positional NAME resolved from actor registry + When I run actor run with name resolved from the actor registry + Then the actor run should call single shot without context manager + + Scenario: Run with positional PROMPT containing flag-like text + When I run actor run with a positional prompt containing flag-like text + Then the actor run should pass the exact positional prompt + + Scenario: Run with positional NAME through registry integration path + When I run actor run through registry integration path + Then the actor run should call single shot without context manager + And the actor registry should have been consulted with requested actor name + + # ---------- Backward compatibility with --config ---------- + + Scenario: Config flag takes precedence over name-based resolution + When I run actor run with config flag taking precedence + Then the actor run should call single shot without context manager + + # ---------- Error: actor name not found in registry ---------- + + Scenario: Error when actor name not found in registry + When I run actor run with an unknown actor name + Then the actor run should exit with registry not found error + + # ---------- actor_run.py positional args ---------- + + Scenario: Actor-run module config flag takes precedence over name + When I invoke the actor-run module with config flag taking precedence + Then the actor run should call single shot without context manager + + Scenario: Actor-run module with positional NAME and PROMPT + When I invoke the actor-run module with positional args and config fallback + Then the actor run should call single shot without context manager + + Scenario: Actor-run module with name from registry + When I invoke the actor-run module with name from the actor registry + Then the actor run should call single shot without context manager + + Scenario: Actor-run module error when name not found + When I invoke the actor-run module with an unknown actor name + Then the actor run should exit with registry not found error + + # ---------- resolve_config_files unit tests ---------- + + Scenario: resolve_config_files returns config when provided + When I call resolve_config_files with a config list + Then the original config list is returned unchanged + + Scenario: resolve_config_files resolves actor from registry with yaml_text + When I call resolve_config_files with a name and no config + Then a temporary YAML file is created from the registry actor + + Scenario: resolve_config_files falls back to config_blob when yaml_text is empty + When I call resolve_config_files with an actor that has no yaml_text + Then a temporary YAML file is created from config_blob + + Scenario: resolve_config_files raises exit when actor has no config data + When I call resolve_config_files with an actor that has no config data + Then it should exit with a no-configuration-data error + + Scenario: resolve_config_files raises exit for unknown actor + When I call resolve_config_files with an unknown actor name directly + Then it should exit with a not-found error + + # ---------- Edge cases and error propagation ---------- + + Scenario: resolve_config_files raises exit when config_blob is empty dict + When I call resolve_config_files with an actor that has empty config_blob + Then it should exit with a no-configuration-data error + + Scenario: Infrastructure errors propagate through resolve_config_files + When I call resolve_config_files and the container raises an infrastructure error + Then the infrastructure error should propagate uncaught + + Scenario: resolve_config_files rejects empty string name with validation error + When I call resolve_config_files with an empty string name + Then the empty name should be rejected with a validation error + + Scenario: resolve_config_files sanitizes control characters in actor name errors + When I call resolve_config_files with a control-character actor name + Then the not-found error should not include control characters + + Scenario: resolve_config_files exits when config_blob cannot be serialised to YAML + When I call resolve_config_files with an actor whose config_blob is not YAML-serialisable + Then it should exit with a serialisation error + + # ---------- Review round 5 fixes ---------- + + Scenario: resolve_config_files falls back to config_blob when yaml_text is whitespace-only + When I call resolve_config_files with an actor that has whitespace-only yaml_text + Then a temporary YAML file is created from the whitespace actor config_blob + + Scenario: Config precedence does not consult the actor registry + When I run actor run with config flag and a registry spy + Then the actor registry should not have been consulted + + Scenario: Actor-run config precedence does not consult the actor registry + When I invoke the actor-run module with config flag and a registry spy + Then the actor registry should not have been consulted + + Scenario: Multiple config files are forwarded with positional NAME + When I run actor run with multiple config files and a positional name + Then all config files should be passed to the application + + # ---------- Review round 6 fixes ---------- + + Scenario: _cleanup_temp_files removes tracked temporary files + When I call resolve_config_files and then cleanup_temp_files + Then the temporary file should be removed by cleanup + + Scenario: actor_registry() infrastructure error propagates through resolve_config_files + When I call resolve_config_files and actor_registry raises an infrastructure error + Then the actor_registry infrastructure error should propagate uncaught diff --git a/features/steps/actor_cli_run_steps.py b/features/steps/actor_cli_run_steps.py index eb5445745..f68b8b950 100644 --- a/features/steps/actor_cli_run_steps.py +++ b/features/steps/actor_cli_run_steps.py @@ -97,7 +97,7 @@ def step_run_actor_with_load_and_context(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--context", "session-1", @@ -131,7 +131,7 @@ def step_run_actor_with_load_only(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--load-context", str(context.load_context_path), @@ -170,7 +170,7 @@ def step_run_actor_with_context_only(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--context", "session-2", @@ -200,7 +200,7 @@ def step_run_actor_without_context(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, ], ) @@ -234,7 +234,7 @@ def step_run_actor_with_load_context_and_rxpy(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--context", "session-rxpy", @@ -269,7 +269,7 @@ def step_run_actor_with_load_only_rxpy(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--load-context", str(context.load_context_path), @@ -309,7 +309,7 @@ def step_run_actor_with_context_only_rxpy(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--context", "session-rxpy-only", @@ -340,7 +340,7 @@ def step_run_actor_without_context_rxpy(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--allow-rxpy-in-run-mode", ], @@ -369,7 +369,7 @@ def step_run_actor_unsafe_error(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, ], ) @@ -398,7 +398,7 @@ def step_run_actor_clever_agents_error(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, ], ) @@ -432,7 +432,7 @@ def step_invoke_actor_run_with_load_and_context(context): [ "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--context", "actor-run-session", @@ -465,7 +465,7 @@ def step_invoke_actor_run_with_load_only(context): [ "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--load-context", str(context.load_context_path), @@ -503,7 +503,7 @@ def step_invoke_actor_run_with_context_only(context): [ "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--context", "actor-run-session-2", @@ -532,7 +532,7 @@ def step_invoke_actor_run_without_context(context): [ "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, ], ) @@ -559,7 +559,7 @@ def step_invoke_actor_run_unsafe_error(context): [ "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, ], ) @@ -587,7 +587,7 @@ def step_invoke_actor_run_clever_agents_exception(context): [ "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, ], ) @@ -684,7 +684,7 @@ def step_run_actor_with_single_skill(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--skill", "local/web-tools", @@ -715,7 +715,7 @@ def step_run_actor_with_multiple_skills(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--skill", "local/web-tools", @@ -755,7 +755,7 @@ def step_run_actor_with_unknown_skill(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--skill", "local/nonexistent", @@ -786,7 +786,7 @@ def step_invoke_actor_run_with_single_skill(context): [ "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--skill", "local/web-tools", @@ -816,7 +816,7 @@ def step_invoke_actor_run_with_multiple_skills(context): [ "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--skill", "local/web-tools", @@ -855,7 +855,7 @@ def step_invoke_actor_run_with_unknown_skill(context): [ "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--skill", "local/nonexistent", @@ -928,7 +928,7 @@ def step_run_actor_with_skill_and_context(context): "run", "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--skill", "local/web-tools", @@ -977,7 +977,7 @@ def step_invoke_actor_run_with_skill_and_context(context): [ "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--skill", "local/web-tools", @@ -1016,7 +1016,7 @@ def step_invoke_actor_run_with_duplicate_skills(context): [ "--config", str(context.actor_config_path), - "--prompt", + "test-actor", context.prompt, "--skill", "local/web-tools", diff --git a/features/steps/actor_run_signature_cli_steps.py b/features/steps/actor_run_signature_cli_steps.py new file mode 100644 index 000000000..da67b7cd7 --- /dev/null +++ b/features/steps/actor_run_signature_cli_steps.py @@ -0,0 +1,453 @@ +"""CLI invocation step definitions for actor run positional-argument tests. + +Covers the ``actor.py`` and ``actor_run.py`` CLI entry-points with +positional NAME and PROMPT arguments, config fallback, registry +resolution, error paths, config-precedence registry spy assertions, +and multiple config-file forwarding. +""" + +from __future__ import annotations + +import tempfile +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +import typer +from behave import then, when + +from cleveragents.cli.commands.actor import app as actor_app +from cleveragents.cli.commands.actor_run import app as actor_run_app +from features.steps.actor_run_signature_resolve_steps import ( + _get_combined_output, + _make_app, +) + + +def _not_found_resolve(name: str, config: list[Any]) -> list[Any]: + """Side-effect for _resolve_config_files that simulates registry miss.""" + typer.echo( + f"Error: Actor '{name}' not found in registry and no --config provided.", + err=True, + ) + raise typer.Exit(code=2) + + +# --------------------------------------------------------------------------- +# actor.py run command — positional args with --config fallback +# --------------------------------------------------------------------------- + + +@when("I run actor run with positional args and config fallback") +def step_run_actor_positional_with_config(context: Any) -> None: + context.prompt = "hello positional" + context.run_result = "positional response" + app_exec = _make_app(result=context.run_result) + + with patch( + "cleveragents.cli.commands.actor.ReactiveCleverAgentsApp", + return_value=app_exec, + ): + context.result = context.runner.invoke( + actor_app, + [ + "run", + "--config", + str(context.actor_config_path), + "my-actor", + context.prompt, + ], + ) + + context.app_exec = app_exec + + +@when("I run actor run with name resolved from the actor registry") +def step_run_actor_name_from_registry(context: Any) -> None: + context.prompt = "registry prompt" + context.run_result = "registry response" + app_exec = _make_app(result=context.run_result) + + # Mock _resolve_config_files to return the existing config path + # (simulating successful registry resolution) + def _mock_resolve(name: str, config: list[Any]) -> list[Any]: + return [context.actor_config_path] + + with ( + patch( + "cleveragents.cli.commands.actor._resolve_config_files", + side_effect=_mock_resolve, + ), + patch( + "cleveragents.cli.commands.actor.ReactiveCleverAgentsApp", + return_value=app_exec, + ), + ): + context.result = context.runner.invoke( + actor_app, + [ + "run", + "local/my-actor", + context.prompt, + ], + ) + + context.app_exec = app_exec + + +@when("I run actor run with a positional prompt containing flag-like text") +def step_run_actor_prompt_with_flag_like_text(context: Any) -> None: + context.prompt = "Fix the --output flag without changing --context" + context.run_result = "flag-like prompt response" + app_exec = _make_app(result=context.run_result) + + with patch( + "cleveragents.cli.commands.actor.ReactiveCleverAgentsApp", + return_value=app_exec, + ): + context.result = context.runner.invoke( + actor_app, + [ + "run", + "--config", + str(context.actor_config_path), + "my-actor", + context.prompt, + ], + ) + + context.app_exec = app_exec + + +@when("I run actor run through registry integration path") +def step_run_actor_registry_integration_path(context: Any) -> None: + context.prompt = "registry integration prompt" + context.run_result = "registry integration response" + app_exec = _make_app(result=context.run_result) + + mock_actor = MagicMock() + mock_actor.yaml_text = "name: integration-actor\ntype: custom\n" + mock_actor.config_blob = None + + mock_registry = MagicMock() + mock_registry.get.return_value = mock_actor + + mock_container = MagicMock() + mock_container.actor_registry.return_value = mock_registry + + with ( + patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ), + patch( + "cleveragents.cli.commands.actor.ReactiveCleverAgentsApp", + return_value=app_exec, + ) as mock_app_cls, + ): + context.result = context.runner.invoke( + actor_app, + [ + "run", + "local/integration-actor", + context.prompt, + ], + ) + + call_kwargs = mock_app_cls.call_args.kwargs + config_files = call_kwargs.get("config_files", []) + if config_files: + temp_config = Path(config_files[0]) + context.add_cleanup(lambda p=temp_config: p.unlink(missing_ok=True)) + + context.app_exec = app_exec + context.mock_registry = mock_registry + + +@when("I run actor run with config flag taking precedence") +def step_run_actor_config_takes_precedence(context: Any) -> None: + context.prompt = "precedence prompt" + context.run_result = "precedence response" + app_exec = _make_app(result=context.run_result) + + with patch( + "cleveragents.cli.commands.actor.ReactiveCleverAgentsApp", + return_value=app_exec, + ): + context.result = context.runner.invoke( + actor_app, + [ + "run", + "--config", + str(context.actor_config_path), + "ignored-name", + context.prompt, + ], + ) + + context.app_exec = app_exec + + +@when("I run actor run with an unknown actor name") +def step_run_actor_unknown_name(context: Any) -> None: + with patch( + "cleveragents.cli.commands.actor._resolve_config_files", + side_effect=_not_found_resolve, + ): + context.result = context.runner.invoke( + actor_app, + [ + "run", + "nonexistent/actor", + "test prompt", + ], + ) + + +# --------------------------------------------------------------------------- +# actor_run.py module — positional args +# --------------------------------------------------------------------------- + + +@when("I invoke the actor-run module with config flag taking precedence") +def step_invoke_actor_run_config_takes_precedence(context: Any) -> None: + context.prompt = "actor-run precedence prompt" + context.run_result = "actor-run precedence response" + app_exec = _make_app(result=context.run_result) + + with patch( + "cleveragents.cli.commands.actor_run.ReactiveCleverAgentsApp", + return_value=app_exec, + ): + context.result = context.runner.invoke( + actor_run_app, + [ + "--config", + str(context.actor_config_path), + "ignored-name", + context.prompt, + ], + ) + + context.app_exec = app_exec + + +@when("I invoke the actor-run module with positional args and config fallback") +def step_invoke_actor_run_positional_with_config(context: Any) -> None: + context.prompt = "actor-run positional" + context.run_result = "actor-run positional response" + app_exec = _make_app(result=context.run_result) + + with patch( + "cleveragents.cli.commands.actor_run.ReactiveCleverAgentsApp", + return_value=app_exec, + ): + context.result = context.runner.invoke( + actor_run_app, + [ + "--config", + str(context.actor_config_path), + "my-actor", + context.prompt, + ], + ) + + context.app_exec = app_exec + + +@when("I invoke the actor-run module with name from the actor registry") +def step_invoke_actor_run_name_from_registry(context: Any) -> None: + context.prompt = "actor-run registry prompt" + context.run_result = "actor-run registry response" + app_exec = _make_app(result=context.run_result) + + # Mock _resolve_config_files to return the existing config path + # (simulating successful registry resolution) + def _mock_resolve(name: str, config: list[Any]) -> list[Any]: + return [context.actor_config_path] + + with ( + patch( + "cleveragents.cli.commands.actor_run._resolve_config_files", + side_effect=_mock_resolve, + ), + patch( + "cleveragents.cli.commands.actor_run.ReactiveCleverAgentsApp", + return_value=app_exec, + ), + ): + context.result = context.runner.invoke( + actor_run_app, + [ + "local/my-actor", + context.prompt, + ], + ) + + context.app_exec = app_exec + + +@when("I invoke the actor-run module with an unknown actor name") +def step_invoke_actor_run_unknown_name(context: Any) -> None: + with patch( + "cleveragents.cli.commands.actor_run._resolve_config_files", + side_effect=_not_found_resolve, + ): + context.result = context.runner.invoke( + actor_run_app, + [ + "nonexistent/actor", + "test prompt", + ], + ) + + +# --------------------------------------------------------------------------- +# Shared assertions +# --------------------------------------------------------------------------- + + +@then("the actor run should exit with registry not found error") +def step_actor_run_registry_not_found(context: Any) -> None: + assert context.result.exit_code == 2 + combined = _get_combined_output(context.result) + assert "not found in registry" in combined + + +@then("the actor run should pass the exact positional prompt") +def step_actor_run_exact_prompt(context: Any) -> None: + assert context.result.exit_code == 0 + run_args = context.app_exec.run_single_shot.call_args.args + assert run_args[0] == context.prompt + + +@then("the actor registry should have been consulted with requested actor name") +def step_registry_consulted_with_actor_name(context: Any) -> None: + context.mock_registry.get.assert_called_once_with("local/integration-actor") + + +# --------------------------------------------------------------------------- +# Review round 5 — P3-2: config precedence with registry spy +# --------------------------------------------------------------------------- + + +@when("I run actor run with config flag and a registry spy") +def step_run_actor_config_with_registry_spy(context: Any) -> None: + context.prompt = "spy prompt" + context.run_result = "spy response" + app_exec = _make_app(result=context.run_result) + + mock_registry = MagicMock() + mock_container = MagicMock() + mock_container.actor_registry.return_value = mock_registry + + with ( + patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ), + patch( + "cleveragents.cli.commands.actor.ReactiveCleverAgentsApp", + return_value=app_exec, + ), + ): + context.result = context.runner.invoke( + actor_app, + [ + "run", + "--config", + str(context.actor_config_path), + "ignored-name", + context.prompt, + ], + ) + context.mock_registry = mock_registry + + +@when("I invoke the actor-run module with config flag and a registry spy") +def step_invoke_actor_run_config_with_registry_spy(context: Any) -> None: + context.prompt = "actor-run spy prompt" + context.run_result = "actor-run spy response" + app_exec = _make_app(result=context.run_result) + + mock_registry = MagicMock() + mock_container = MagicMock() + mock_container.actor_registry.return_value = mock_registry + + with ( + patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ), + patch( + "cleveragents.cli.commands.actor_run.ReactiveCleverAgentsApp", + return_value=app_exec, + ), + ): + context.result = context.runner.invoke( + actor_run_app, + [ + "--config", + str(context.actor_config_path), + "ignored-name", + context.prompt, + ], + ) + context.mock_registry = mock_registry + + +@then("the actor registry should not have been consulted") +def step_registry_not_consulted(context: Any) -> None: + assert context.result.exit_code == 0 + context.mock_registry.get.assert_not_called() + + +# --------------------------------------------------------------------------- +# Review round 5 — P3-3: multiple config files with positional NAME +# --------------------------------------------------------------------------- + + +@when("I run actor run with multiple config files and a positional name") +def step_run_actor_multiple_configs(context: Any) -> None: + context.prompt = "multi-config prompt" + context.run_result = "multi-config response" + app_exec = _make_app(result=context.run_result) + + second_config = Path(tempfile.gettempdir()) / "second_actor_config.yaml" + second_config.write_text( + "name: second\ntype: custom\nprovider: openai\nmodel: gpt-4\n", + encoding="utf-8", + ) + context.add_cleanup(lambda: second_config.unlink(missing_ok=True)) + context.second_actor_config_path = second_config + + with patch( + "cleveragents.cli.commands.actor.ReactiveCleverAgentsApp", + return_value=app_exec, + ) as mock_app_cls: + context.result = context.runner.invoke( + actor_app, + [ + "run", + "--config", + str(context.actor_config_path), + "--config", + str(second_config), + "ignored-name", + context.prompt, + ], + ) + context.mock_app_cls = mock_app_cls + + +@then("all config files should be passed to the application") +def step_all_configs_passed(context: Any) -> None: + assert context.result.exit_code == 0 + call_kwargs = context.mock_app_cls.call_args + config_files = call_kwargs[1].get( + "config_files", call_kwargs[0][0] if call_kwargs[0] else None + ) + assert config_files is not None + assert len(config_files) == 2 + assert Path(config_files[0]) == context.actor_config_path + assert Path(config_files[1]) == context.second_actor_config_path diff --git a/features/steps/actor_run_signature_resolve_steps.py b/features/steps/actor_run_signature_resolve_steps.py new file mode 100644 index 000000000..36d5f473f --- /dev/null +++ b/features/steps/actor_run_signature_resolve_steps.py @@ -0,0 +1,472 @@ +"""Step definitions for resolve_config_files unit tests, edge cases, and +error propagation in actor run positional-argument tests. + +This module contains the ``resolve_config_files`` unit-level BDD +scenarios as well as shared helpers used by +``actor_run_signature_cli_steps.py``. +""" + +from __future__ import annotations + +import tempfile +from pathlib import Path +from types import SimpleNamespace +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +import click +import typer +import yaml +from behave import then, when + +from cleveragents.cli.commands._resolve_actor import ( + _cleanup_temp_files, + _temp_files, + resolve_config_files, +) +from cleveragents.core.exceptions import InfrastructureError, NotFoundError + +# --------------------------------------------------------------------------- +# Shared helpers (also imported by actor_run_signature_cli_steps) +# --------------------------------------------------------------------------- + + +def _make_app( + *, + result: str, + config_global_context: dict[str, Any] | None = None, +) -> MagicMock: + app_exec = MagicMock() + app_exec.config = SimpleNamespace(global_context=dict(config_global_context or {})) + app_exec.run_single_shot = AsyncMock(return_value=result) + return app_exec + + +def _get_combined_output(result: Any) -> str: + """Return combined stdout + stderr from a CliRunner result.""" + output = result.output or "" + stderr = getattr(result, "stderr", None) or "" + return output + stderr + + +# --------------------------------------------------------------------------- +# resolve_config_files unit tests (P2-3 / P2-4) +# --------------------------------------------------------------------------- + + +@when("I call resolve_config_files with a config list") +def step_resolve_with_config_list(context: Any) -> None: + cfg = [Path(tempfile.gettempdir()) / "dummy.yaml"] + context.resolve_result = resolve_config_files("ignored", cfg) + context.expected_config = cfg + + +@then("the original config list is returned unchanged") +def step_config_returned_unchanged(context: Any) -> None: + assert context.resolve_result is context.expected_config + + +@when("I call resolve_config_files with a name and no config") +def step_resolve_with_name_no_config(context: Any) -> None: + mock_actor = MagicMock() + mock_actor.yaml_text = "name: test-actor\ntype: custom\n" + mock_actor.config_blob = None + + mock_registry = MagicMock() + mock_registry.get.return_value = mock_actor + + mock_container = MagicMock() + mock_container.actor_registry.return_value = mock_registry + + with patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ): + context.resolve_result = resolve_config_files("local/test-actor", []) + + context.mock_registry = mock_registry + + +@then("a temporary YAML file is created from the registry actor") +def step_temp_file_from_registry(context: Any) -> None: + assert len(context.resolve_result) == 1 + tmp_path = context.resolve_result[0] + context.add_cleanup(lambda p=tmp_path: p.unlink(missing_ok=True)) + assert tmp_path.exists() + content = tmp_path.read_text(encoding="utf-8") + assert "test-actor" in content + context.mock_registry.get.assert_called_once_with("local/test-actor") + + +@when("I call resolve_config_files with an actor that has no yaml_text") +def step_resolve_with_no_yaml_text(context: Any) -> None: + mock_actor = MagicMock() + mock_actor.yaml_text = "" + mock_actor.config_blob = {"name": "blob-actor", "type": "custom"} + + mock_registry = MagicMock() + mock_registry.get.return_value = mock_actor + + mock_container = MagicMock() + mock_container.actor_registry.return_value = mock_registry + + with patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ): + context.resolve_result = resolve_config_files("local/blob-actor", []) + + +@then("a temporary YAML file is created from config_blob") +def step_temp_file_from_config_blob(context: Any) -> None: + assert len(context.resolve_result) == 1 + tmp_path = context.resolve_result[0] + context.add_cleanup(lambda p=tmp_path: p.unlink(missing_ok=True)) + assert tmp_path.exists() + content = tmp_path.read_text(encoding="utf-8") + assert "blob-actor" in content + parsed = yaml.safe_load(content) + assert isinstance(parsed, dict) + assert parsed["name"] == "blob-actor" + + +@when("I call resolve_config_files with an actor that has no config data") +def step_resolve_with_no_config_data(context: Any) -> None: + mock_actor = MagicMock() + mock_actor.yaml_text = "" + mock_actor.config_blob = None + + mock_registry = MagicMock() + mock_registry.get.return_value = mock_actor + + mock_container = MagicMock() + mock_container.actor_registry.return_value = mock_registry + + captured_stderr: list[str] = [] + original_echo = typer.echo + + def _capture_echo(message: str = "", err: bool = False, **kwargs: Any) -> None: + if err: + captured_stderr.append(message) + original_echo(message, err=err, **kwargs) + + with ( + patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ), + patch( + "cleveragents.cli.commands._resolve_actor.typer.echo", + side_effect=_capture_echo, + ), + ): + try: + resolve_config_files("local/empty-actor", []) + context.resolve_exit_code = 0 + except (SystemExit, click.exceptions.Exit) as exc: + context.resolve_exit_code = getattr( + exc, "exit_code", getattr(exc, "code", 1) + ) + context.resolve_stderr = " ".join(captured_stderr) + + +@then("it should exit with a no-configuration-data error") +def step_exit_no_config_data(context: Any) -> None: + assert context.resolve_exit_code == 2 + assert "no configuration data" in context.resolve_stderr + + +@when("I call resolve_config_files with an unknown actor name directly") +def step_resolve_unknown_actor_directly(context: Any) -> None: + mock_registry = MagicMock() + mock_registry.get.side_effect = NotFoundError( + resource_type="actor", resource_id="nonexistent/actor" + ) + + mock_container = MagicMock() + mock_container.actor_registry.return_value = mock_registry + + captured_stderr: list[str] = [] + original_echo = typer.echo + + def _capture_echo(message: str = "", err: bool = False, **kwargs: Any) -> None: + if err: + captured_stderr.append(message) + original_echo(message, err=err, **kwargs) + + with ( + patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ), + patch( + "cleveragents.cli.commands._resolve_actor.typer.echo", + side_effect=_capture_echo, + ), + ): + try: + resolve_config_files("nonexistent/actor", []) + context.resolve_exit_code = 0 + except (SystemExit, click.exceptions.Exit) as exc: + context.resolve_exit_code = getattr( + exc, "exit_code", getattr(exc, "code", 1) + ) + context.resolve_stderr = " ".join(captured_stderr) + + +@then("it should exit with a not-found error") +def step_exit_not_found(context: Any) -> None: + assert context.resolve_exit_code == 2 + assert "not found in registry" in context.resolve_stderr + + +# --------------------------------------------------------------------------- +# Edge cases and error propagation (P2-3, P3-2, P3-4) +# --------------------------------------------------------------------------- + + +@when("I call resolve_config_files with an actor that has empty config_blob") +def step_resolve_with_empty_config_blob(context: Any) -> None: + mock_actor = MagicMock() + mock_actor.yaml_text = "" + mock_actor.config_blob = {} + + mock_registry = MagicMock() + mock_registry.get.return_value = mock_actor + + mock_container = MagicMock() + mock_container.actor_registry.return_value = mock_registry + + captured_stderr: list[str] = [] + original_echo = typer.echo + + def _capture_echo(message: str = "", err: bool = False, **kwargs: Any) -> None: + if err: + captured_stderr.append(message) + original_echo(message, err=err, **kwargs) + + with ( + patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ), + patch( + "cleveragents.cli.commands._resolve_actor.typer.echo", + side_effect=_capture_echo, + ), + ): + try: + resolve_config_files("local/empty-blob-actor", []) + context.resolve_exit_code = 0 + except (SystemExit, click.exceptions.Exit) as exc: + context.resolve_exit_code = getattr( + exc, "exit_code", getattr(exc, "code", 1) + ) + context.resolve_stderr = " ".join(captured_stderr) + + +@when("I call resolve_config_files and the container raises an infrastructure error") +def step_resolve_with_infrastructure_error(context: Any) -> None: + with patch( + "cleveragents.cli.commands._resolve_actor.get_container", + side_effect=InfrastructureError("database unavailable"), + ): + try: + resolve_config_files("local/some-actor", []) + context.infra_error_propagated = False + except InfrastructureError: + context.infra_error_propagated = True + + +@then("the infrastructure error should propagate uncaught") +def step_infra_error_propagated(context: Any) -> None: + assert context.infra_error_propagated is True + + +@when("I call resolve_config_files with an empty string name") +def step_resolve_with_empty_name(context: Any) -> None: + captured_stderr: list[str] = [] + original_echo = typer.echo + + def _capture_echo(message: str = "", err: bool = False, **kwargs: Any) -> None: + if err: + captured_stderr.append(message) + original_echo(message, err=err, **kwargs) + + with patch( + "cleveragents.cli.commands._resolve_actor.typer.echo", + side_effect=_capture_echo, + ): + try: + resolve_config_files("", []) + context.empty_name_exit_code = 0 + except (SystemExit, typer.Exit) as exc: + context.empty_name_exit_code = getattr( + exc, "exit_code", getattr(exc, "code", 1) + ) + context.resolve_stderr = " ".join(captured_stderr) + + +@then("the empty name should be rejected with a validation error") +def step_empty_name_rejected(context: Any) -> None: + assert context.empty_name_exit_code == 2 + assert "must not be empty" in context.resolve_stderr + + +# --------------------------------------------------------------------------- +# YAML serialisation failure (P2-1) +# --------------------------------------------------------------------------- + + +@when( + "I call resolve_config_files with an actor whose config_blob is not YAML-serialisable" +) +def step_resolve_with_unserializable_config_blob(context: Any) -> None: + mock_actor = MagicMock() + mock_actor.yaml_text = "" + # yaml.safe_dump cannot serialise arbitrary objects + mock_actor.config_blob = {"bad_value": object()} + + mock_registry = MagicMock() + mock_registry.get.return_value = mock_actor + + mock_container = MagicMock() + mock_container.actor_registry.return_value = mock_registry + + captured_stderr: list[str] = [] + original_echo = typer.echo + + def _capture_echo(message: str = "", err: bool = False, **kwargs: Any) -> None: + if err: + captured_stderr.append(message) + original_echo(message, err=err, **kwargs) + + with ( + patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ), + patch( + "cleveragents.cli.commands._resolve_actor.typer.echo", + side_effect=_capture_echo, + ), + ): + try: + resolve_config_files("local/bad-blob-actor", []) + context.resolve_exit_code = 0 + except (SystemExit, click.exceptions.Exit) as exc: + context.resolve_exit_code = getattr( + exc, "exit_code", getattr(exc, "code", 1) + ) + context.resolve_stderr = " ".join(captured_stderr) + + +@then("it should exit with a serialisation error") +def step_exit_serialisation_error(context: Any) -> None: + assert context.resolve_exit_code == 2 + assert "could not be serialised" in context.resolve_stderr + + +# --------------------------------------------------------------------------- +# Review round 5 — P3-1: whitespace-only yaml_text +# --------------------------------------------------------------------------- + + +@when("I call resolve_config_files with an actor that has whitespace-only yaml_text") +def step_resolve_with_whitespace_yaml_text(context: Any) -> None: + mock_actor = MagicMock() + mock_actor.yaml_text = " \n \t " + mock_actor.config_blob = {"name": "ws-actor", "type": "custom"} + + mock_registry = MagicMock() + mock_registry.get.return_value = mock_actor + + mock_container = MagicMock() + mock_container.actor_registry.return_value = mock_registry + + with patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ): + context.resolve_result = resolve_config_files("local/ws-actor", []) + + +@then("a temporary YAML file is created from the whitespace actor config_blob") +def step_temp_file_from_whitespace_config_blob(context: Any) -> None: + assert len(context.resolve_result) == 1 + tmp_path = context.resolve_result[0] + context.add_cleanup(lambda p=tmp_path: p.unlink(missing_ok=True)) + assert tmp_path.exists() + content = tmp_path.read_text(encoding="utf-8") + assert "ws-actor" in content + parsed = yaml.safe_load(content) + assert isinstance(parsed, dict) + assert parsed["name"] == "ws-actor" + + +# --------------------------------------------------------------------------- +# Review round 6 — m8: _cleanup_temp_files removes tracked files +# --------------------------------------------------------------------------- + + +@when("I call resolve_config_files and then cleanup_temp_files") +def step_resolve_then_cleanup(context: Any) -> None: + _cleanup_temp_files() + + mock_actor = MagicMock() + mock_actor.yaml_text = "name: cleanup-test-actor\ntype: custom\n" + mock_actor.config_blob = None + + mock_registry = MagicMock() + mock_registry.get.return_value = mock_actor + + mock_container = MagicMock() + mock_container.actor_registry.return_value = mock_registry + + with patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ): + result_paths = resolve_config_files("local/cleanup-test-actor", []) + + context.cleanup_tmp_path = result_paths[0] + # Verify file exists before cleanup + assert context.cleanup_tmp_path.exists() + # Verify path was tracked + assert str(context.cleanup_tmp_path) in _temp_files + + _cleanup_temp_files() + + +@then("the temporary file should be removed by cleanup") +def step_temp_file_removed_by_cleanup(context: Any) -> None: + assert not context.cleanup_tmp_path.exists() + assert len(_temp_files) == 0 + + +# --------------------------------------------------------------------------- +# Review round 6 — m9: actor_registry() raising an infrastructure error +# --------------------------------------------------------------------------- + + +@when("I call resolve_config_files and actor_registry raises an infrastructure error") +def step_resolve_with_actor_registry_error(context: Any) -> None: + mock_container = MagicMock() + mock_container.actor_registry.side_effect = InfrastructureError( + "actor registry unavailable" + ) + + with patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ): + try: + resolve_config_files("local/some-actor", []) + context.actor_registry_error_propagated = False + except InfrastructureError: + context.actor_registry_error_propagated = True + + +@then("the actor_registry infrastructure error should propagate uncaught") +def step_actor_registry_error_propagated(context: Any) -> None: + assert context.actor_registry_error_propagated is True diff --git a/features/steps/actor_run_signature_security_steps.py b/features/steps/actor_run_signature_security_steps.py new file mode 100644 index 000000000..65e3ab5e1 --- /dev/null +++ b/features/steps/actor_run_signature_security_steps.py @@ -0,0 +1,64 @@ +"""Security-oriented step definitions for actor run signature tests.""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock, patch + +import click +import typer +from behave import then, when + +from cleveragents.cli.commands._resolve_actor import resolve_config_files +from cleveragents.core.exceptions import NotFoundError + + +@when("I call resolve_config_files with a control-character actor name") +def step_resolve_with_control_character_name(context: Any) -> None: + actor_name = "local/\x1b[31mevil\x1b[0m\x00actor" + + mock_registry = MagicMock() + mock_registry.get.side_effect = NotFoundError( + resource_type="actor", + resource_id=actor_name, + ) + + mock_container = MagicMock() + mock_container.actor_registry.return_value = mock_registry + + captured_stderr: list[str] = [] + original_echo = typer.echo + + def _capture_echo(message: str = "", err: bool = False, **kwargs: Any) -> None: + if err: + captured_stderr.append(message) + original_echo(message, err=err, **kwargs) + + with ( + patch( + "cleveragents.cli.commands._resolve_actor.get_container", + return_value=mock_container, + ), + patch( + "cleveragents.cli.commands._resolve_actor.typer.echo", + side_effect=_capture_echo, + ), + ): + try: + resolve_config_files(actor_name, []) + context.resolve_exit_code = 0 + except (SystemExit, click.exceptions.Exit) as exc: + context.resolve_exit_code = getattr( + exc, "exit_code", getattr(exc, "code", 1) + ) + + context.resolve_stderr = " ".join(captured_stderr) + + +@then("the not-found error should not include control characters") +def step_error_sanitises_control_characters(context: Any) -> None: + assert context.resolve_exit_code == 2 + assert "not found in registry" in context.resolve_stderr + assert "\x1b" not in context.resolve_stderr + assert "\x00" not in context.resolve_stderr + assert "evil" in context.resolve_stderr diff --git a/robot/actor_run_signature.robot b/robot/actor_run_signature.robot new file mode 100644 index 000000000..4964cd913 --- /dev/null +++ b/robot/actor_run_signature.robot @@ -0,0 +1,49 @@ +*** Settings *** +Documentation Integration tests for actor run positional NAME/PROMPT signature +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_actor_run_signature.py + +*** Test Cases *** +Positional Args With Config Fallback + [Documentation] Verify that positional NAME + PROMPT work with --config fallback + ${result}= Run Process ${PYTHON} ${HELPER} positional-with-config cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} positional-with-config-ok + +Positional NAME From Actor Registry + [Documentation] Verify that NAME is resolved from the actor registry when no --config + ${result}= Run Process ${PYTHON} ${HELPER} positional-from-registry cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} positional-from-registry-ok + +Actor App Registry Resolution + [Documentation] Verify that NAME is resolved from the actor registry via actor_app + ${result}= Run Process ${PYTHON} ${HELPER} actor-app-registry cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} actor-app-registry-ok + +Unknown Actor Name Error + [Documentation] Verify that unknown actor name exits with error code 2 + ${result}= Run Process ${PYTHON} ${HELPER} unknown-actor-name cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} unknown-actor-name-ok + +Actor App Unknown Name Error + [Documentation] Verify that unknown actor name exits with error code 2 via actor_app + ${result}= Run Process ${PYTHON} ${HELPER} actor-app-unknown-name cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} actor-app-unknown-name-ok diff --git a/robot/context_delete_all_yes.robot b/robot/context_delete_all_yes.robot index f48be6d4e..77eb99b51 100644 --- a/robot/context_delete_all_yes.robot +++ b/robot/context_delete_all_yes.robot @@ -29,7 +29,7 @@ Test Delete Single Context With Yes Flag ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test message" + ... test-actor "test message" Directory Should Exist ${CONTEXT_DIR}/${context_name} @@ -54,7 +54,7 @@ Test Delete Single Context With Short Yes Flag ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test message" + ... test-actor "test message" Directory Should Exist ${CONTEXT_DIR}/${context_name} @@ -78,7 +78,7 @@ Test Delete Single Context With Confirmation Accept ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test message" + ... test-actor "test message" Directory Should Exist ${CONTEXT_DIR}/${context_name} @@ -102,7 +102,7 @@ Test Delete Single Context With Confirmation Reject ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test message" + ... test-actor "test message" Directory Should Exist ${CONTEXT_DIR}/${context_name} @@ -130,7 +130,7 @@ Test Delete All Contexts With All And Yes Flags ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" Run Process ${PYTHON} -m cleveragents actor run ... -c ${SIMPLE_CONFIG} @@ -138,7 +138,7 @@ Test Delete All Contexts With All And Yes Flags ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" Run Process ${PYTHON} -m cleveragents actor run ... -c ${SIMPLE_CONFIG} @@ -146,7 +146,7 @@ Test Delete All Contexts With All And Yes Flags ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" Directory Should Exist ${CONTEXT_DIR}/${ctx1} Directory Should Exist ${CONTEXT_DIR}/${ctx2} @@ -177,7 +177,7 @@ Test Delete All With Confirmation Accept ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" Run Process ${PYTHON} -m cleveragents actor run ... -c ${SIMPLE_CONFIG} @@ -185,7 +185,7 @@ Test Delete All With Confirmation Accept ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" # Delete all with confirmation ${result} = Run Process ${PYTHON} -m cleveragents context delete @@ -212,7 +212,7 @@ Test Delete All With Confirmation Reject ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" Run Process ${PYTHON} -m cleveragents actor run ... -c ${SIMPLE_CONFIG} @@ -220,7 +220,7 @@ Test Delete All With Confirmation Reject ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" # Delete all with confirmation rejected ${result} = Run Process ${PYTHON} -m cleveragents context delete @@ -288,7 +288,7 @@ Test Delete All Shows Context List Before Deletion ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" Run Process ${PYTHON} -m cleveragents actor run ... -c ${SIMPLE_CONFIG} @@ -296,7 +296,7 @@ Test Delete All Shows Context List Before Deletion ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" Run Process ${PYTHON} -m cleveragents actor run ... -c ${SIMPLE_CONFIG} @@ -304,7 +304,7 @@ Test Delete All Shows Context List Before Deletion ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" # Delete all with confirmation to see list ${result} = Run Process ${PYTHON} -m cleveragents context delete diff --git a/robot/context_management_test.robot b/robot/context_management_test.robot index 1c32b03f6..2869b76f2 100644 --- a/robot/context_management_test.robot +++ b/robot/context_management_test.robot @@ -37,7 +37,7 @@ Test Run With Context Creates Directory ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test message" + ... test-actor "test message" Should Be Equal As Integers ${result.rc} 0 Directory Should Exist ${CONTEXT_DIR}/${context_name} @@ -53,7 +53,7 @@ Test Context Delete ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" Directory Should Exist ${CONTEXT_DIR}/${context_name} @@ -77,7 +77,7 @@ Test Context Clear ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" # Clear it ${result} = Run Process ${PYTHON} -m cleveragents context clear diff --git a/robot/helper_actor_run_signature.py b/robot/helper_actor_run_signature.py new file mode 100644 index 000000000..2ce7dbd48 --- /dev/null +++ b/robot/helper_actor_run_signature.py @@ -0,0 +1,266 @@ +"""Helper script for actor_run_signature.robot integration tests. + +Tests that the ``actor run`` and ``actor-run`` CLI commands accept +positional NAME and PROMPT arguments while preserving --config fallback. +""" + +from __future__ import annotations + +import os +import sys +import tempfile +from pathlib import Path +from types import SimpleNamespace +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +# Ensure local source tree is importable +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +import typer # noqa: E402 +from typer.testing import CliRunner # noqa: E402 + +from cleveragents.cli.commands.actor import app as actor_app # noqa: E402 +from cleveragents.cli.commands.actor_run import app as actor_run_app # noqa: E402 + +runner = CliRunner() + +_SIMPLE_YAML = """\ +name: smoke-actor +type: custom +provider: openai +model: gpt-4 +tools: + - operation: identity +""" + + +def _write_yaml(content: str) -> str: + fd, path = tempfile.mkstemp(suffix=".yaml") + with os.fdopen(fd, "w", encoding="utf-8") as fh: + fh.write(content) + return path + + +def _make_app(*, result: str) -> MagicMock: + app_exec = MagicMock() + app_exec.config = SimpleNamespace(global_context={}) + app_exec.run_single_shot = AsyncMock(return_value=result) + return app_exec + + +def positional_with_config() -> None: + """Test positional NAME + PROMPT with --config fallback.""" + path = _write_yaml(_SIMPLE_YAML) + app_exec = _make_app(result="config response") + + try: + with patch( + "cleveragents.cli.commands.actor.ReactiveCleverAgentsApp", + return_value=app_exec, + ): + result = runner.invoke( + actor_app, + ["run", "--config", path, "my-actor", "hello world"], + ) + if result.exit_code != 0: + print( + f"FAIL: expected exit code 0, got {result.exit_code}", + file=sys.stderr, + ) + print(result.output, file=sys.stderr) + sys.exit(1) + if "config response" not in result.output: + print( + f"FAIL: expected 'config response' in output, got: {result.output}", + file=sys.stderr, + ) + sys.exit(1) + print("positional-with-config-ok") + finally: + Path(path).unlink(missing_ok=True) + + +def positional_from_registry() -> None: + """Test positional NAME resolved from the actor registry.""" + path = _write_yaml(_SIMPLE_YAML) + app_exec = _make_app(result="registry response") + + def _mock_resolve(name: str, config: list[Any]) -> list[Any]: + return [Path(path)] + + try: + with ( + patch( + "cleveragents.cli.commands.actor_run._resolve_config_files", + side_effect=_mock_resolve, + ), + patch( + "cleveragents.cli.commands.actor_run.ReactiveCleverAgentsApp", + return_value=app_exec, + ), + ): + result = runner.invoke( + actor_run_app, + ["local/my-actor", "hello from registry"], + ) + + if result.exit_code != 0: + print( + f"FAIL: expected exit code 0, got {result.exit_code}", + file=sys.stderr, + ) + print(result.output, file=sys.stderr) + sys.exit(1) + if "registry response" not in result.output: + print( + f"FAIL: expected 'registry response' in output, got: {result.output}", + file=sys.stderr, + ) + sys.exit(1) + print("positional-from-registry-ok") + finally: + Path(path).unlink(missing_ok=True) + + +def actor_app_registry_resolution() -> None: + """Test positional NAME resolved from the actor registry via actor_app.""" + path = _write_yaml(_SIMPLE_YAML) + app_exec = _make_app(result="actor-app registry response") + + def _mock_resolve(name: str, config: list[Any]) -> list[Any]: + return [Path(path)] + + try: + with ( + patch( + "cleveragents.cli.commands.actor._resolve_config_files", + side_effect=_mock_resolve, + ), + patch( + "cleveragents.cli.commands.actor.ReactiveCleverAgentsApp", + return_value=app_exec, + ), + ): + result = runner.invoke( + actor_app, + ["run", "local/my-actor", "hello from actor_app registry"], + ) + + if result.exit_code != 0: + print( + f"FAIL: expected exit code 0, got {result.exit_code}", + file=sys.stderr, + ) + print(result.output, file=sys.stderr) + sys.exit(1) + if "actor-app registry response" not in result.output: + print( + "FAIL: expected 'actor-app registry response' " + f"in output, got: {result.output}", + file=sys.stderr, + ) + sys.exit(1) + print("actor-app-registry-ok") + finally: + Path(path).unlink(missing_ok=True) + + +def unknown_actor_name() -> None: + """Test error when actor name is not found in registry.""" + + def _not_found_resolve(name: str, config: list[Any]) -> list[Any]: + typer.echo( + f"Error: Actor '{name}' not found in registry and no --config provided.", + err=True, + ) + raise typer.Exit(code=2) + + with patch( + "cleveragents.cli.commands.actor_run._resolve_config_files", + side_effect=_not_found_resolve, + ): + result = runner.invoke( + actor_run_app, + ["nonexistent/actor", "test prompt"], + ) + + if result.exit_code != 2: + print( + f"FAIL: expected exit code 2, got {result.exit_code}", + file=sys.stderr, + ) + print(result.output, file=sys.stderr) + sys.exit(1) + combined = (result.output or "") + (getattr(result, "stderr", None) or "") + if "not found in registry" not in combined: + print( + "FAIL: expected 'not found in registry' in output", + file=sys.stderr, + ) + print(combined, file=sys.stderr) + sys.exit(1) + print("unknown-actor-name-ok") + + +def actor_app_unknown_name() -> None: + """Test error when actor name is not found in registry via actor_app.""" + + def _not_found_resolve(name: str, config: list[Any]) -> list[Any]: + typer.echo( + f"Error: Actor '{name}' not found in registry and no --config provided.", + err=True, + ) + raise typer.Exit(code=2) + + with patch( + "cleveragents.cli.commands.actor._resolve_config_files", + side_effect=_not_found_resolve, + ): + result = runner.invoke( + actor_app, + ["run", "nonexistent/actor", "test prompt"], + ) + + if result.exit_code != 2: + print( + f"FAIL: expected exit code 2, got {result.exit_code}", + file=sys.stderr, + ) + print(result.output, file=sys.stderr) + sys.exit(1) + combined = (result.output or "") + (getattr(result, "stderr", None) or "") + if "not found in registry" not in combined: + print( + "FAIL: expected 'not found in registry' in output", + file=sys.stderr, + ) + print(combined, file=sys.stderr) + sys.exit(1) + print("actor-app-unknown-name-ok") + + +# --------------------------------------------------------------------------- +# Main dispatcher +# --------------------------------------------------------------------------- + +_COMMANDS: dict[str, Any] = { + "positional-with-config": positional_with_config, + "positional-from-registry": positional_from_registry, + "actor-app-registry": actor_app_registry_resolution, + "unknown-actor-name": unknown_actor_name, + "actor-app-unknown-name": actor_app_unknown_name, +} + + +def main() -> None: + if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS: + print(f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>", file=sys.stderr) + sys.exit(2) + _COMMANDS[sys.argv[1]]() + + +if __name__ == "__main__": + main() diff --git a/robot/helper_skill_actor_run.py b/robot/helper_skill_actor_run.py index 83ba2bf70..eaa6b6d62 100644 --- a/robot/helper_skill_actor_run.py +++ b/robot/helper_skill_actor_run.py @@ -47,7 +47,7 @@ def unknown_skill_exits_with_error() -> None: [ "--config", path, - "--prompt", + "test-actor", "hello", "--skill", "local/nonexistent-skill", @@ -104,7 +104,7 @@ def skill_flag_accepted() -> None: [ "--config", path, - "--prompt", + "test-actor", "hello", "--skill", "local/smoke-skill", diff --git a/robot/initial_next_command_test.robot b/robot/initial_next_command_test.robot index 8c39e352d..05b4a9608 100644 --- a/robot/initial_next_command_test.robot +++ b/robot/initial_next_command_test.robot @@ -31,7 +31,7 @@ Test Next Command With Null Writing Stage ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p !next discovery + ... test-actor !next discovery ... stderr=STDOUT ... timeout=120s on_timeout=kill @@ -46,7 +46,7 @@ Test Next Command With Null Writing Stage ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p !stage + ... test-actor !stage ... stderr=STDOUT ... timeout=120s on_timeout=kill diff --git a/robot/load_context_test.robot b/robot/load_context_test.robot index 9e704487d..cacec0893 100644 --- a/robot/load_context_test.robot +++ b/robot/load_context_test.robot @@ -27,7 +27,7 @@ Test Load Context Transiently With Run Command ... --load-context ${context_file} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test message" + ... test-actor "test message" Should Be Equal As Integers ${result.rc} 0 # Verify no context directory was created (transient) @@ -47,7 +47,7 @@ Test Load Context Into Named Context ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test message" + ... test-actor "test message" Should Be Equal As Integers ${result.rc} 0 @@ -72,7 +72,7 @@ Test Load Context Replaces Existing Named Context ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "original message" + ... test-actor "original message" Should Be Equal As Integers ${result1.rc} 0 @@ -87,7 +87,7 @@ Test Load Context Replaces Existing Named Context ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "new message" + ... test-actor "new message" Should Be Equal As Integers ${result2.rc} 0 @@ -108,7 +108,7 @@ Test Load Context From Export Format ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test data for export" + ... test-actor "test data for export" Should Be Equal As Integers ${result1.rc} 0 @@ -138,7 +138,7 @@ Test Load Context From Export Format ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "verify import" + ... test-actor "verify import" Should Be Equal As Integers ${result4.rc} 0 @@ -154,7 +154,7 @@ Test Load Context With Non-Existent File ... --load-context /nonexistent/file.json ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" Should Not Be Equal As Integers ${result.rc} 0 Should Contain Any ${result.stderr} does not exist No such file not found @@ -169,7 +169,7 @@ Test Load Context With Invalid JSON ... --load-context ${bad_json_file} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "test" + ... test-actor "test" Should Not Be Equal As Integers ${result.rc} 0 @@ -206,7 +206,7 @@ Test Load Context With All Components ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p "verify all components" + ... test-actor "verify all components" Should Be Equal As Integers ${result.rc} 0 diff --git a/robot/routing_prefix_stripping.robot b/robot/routing_prefix_stripping.robot index 8801ded49..6b7ed522d 100644 --- a/robot/routing_prefix_stripping.robot +++ b/robot/routing_prefix_stripping.robot @@ -24,7 +24,7 @@ Routing Prefix Stripping In Application ... -c ${DISCOVERY_CONFIG} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p test input + ... test-actor test input ... stderr=STDOUT ... timeout=120s on_timeout=kill @@ -42,7 +42,7 @@ Multiple Routing Prefixes Are Stripped ... -c ${BRAINSTORM_CONFIG} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p test input + ... test-actor test input ... stderr=STDOUT ... timeout=120s on_timeout=kill @@ -58,7 +58,7 @@ Content Without Prefix Remains Unchanged ... -c ${NO_PREFIX_CONFIG} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p test input + ... test-actor test input ... stderr=STDOUT ... timeout=120s on_timeout=kill @@ -73,7 +73,7 @@ SET Prefix Family Stripped ... -c ${SET_TOPIC_CONFIG} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p test input + ... test-actor test input ... stderr=STDOUT ... timeout=120s on_timeout=kill diff --git a/robot/rxpy_route_validation.robot b/robot/rxpy_route_validation.robot index 4174244b1..058f11bc8 100644 --- a/robot/rxpy_route_validation.robot +++ b/robot/rxpy_route_validation.robot @@ -27,8 +27,8 @@ Test Run Command Rejects RxPY Routes # Try to run with RxPY routes - should fail ${result} = Run Process ${PYTHON} -m cleveragents actor run ... -c ${RXPY_CONFIG} - ... -p test ... --unsafe + ... test-actor test ... stderr=STDOUT timeout=120s on_timeout=kill Should Not Be Equal As Integers ${result.rc} 0 Command should have failed @@ -49,10 +49,10 @@ Test Run Command With Context Does Not Create Context # Try to run with --context flag ${result} = Run Process ${PYTHON} -m cleveragents actor run ... -c ${RXPY_CONFIG} - ... -p test ... --unsafe ... --context ${context_name} ... --context-dir ${CONTEXT_DIR} + ... test-actor test ... stderr=STDOUT timeout=120s on_timeout=kill Should Not Be Equal As Integers ${result.rc} 0 Command should have failed @@ -73,7 +73,7 @@ Test Run Command With LangGraph Routes Succeeds ${result} = Run Process ${PYTHON} -m cleveragents actor run ... -c ${LANGGRAPH_CONFIG} ... --context-dir ${CONTEXT_DIR} - ... -p test + ... test-actor test ... stderr=STDOUT timeout=120s on_timeout=kill # For now, might fail on actual execution but shouldn't have route validation error @@ -89,11 +89,11 @@ Test Run Command Accepts RxPY Routes When Allowed ${context_name} = Set Variable rxpy_test_ctx_${UNIQUE_ID} ${result} = Run Process ${PYTHON} -m cleveragents actor run ... -c ${RXPY_CONFIG} - ... -p test ... --unsafe ... --allow-rxpy-in-run-mode ... --context ${context_name} ... --context-dir ${CONTEXT_DIR} + ... test-actor test ... stderr=STDOUT ... timeout=120s on_timeout=kill @@ -108,8 +108,8 @@ Test Error Message Content ${result} = Run Process ${PYTHON} -m cleveragents actor run ... -c ${RXPY_CONFIG} - ... -p test ... --unsafe + ... test-actor test ... stderr=STDOUT timeout=120s on_timeout=kill Should Not Be Equal As Integers ${result.rc} 0 @@ -129,8 +129,8 @@ Test Run With Nested Switch Operators ${result} = Run Process ${PYTHON} -m cleveragents actor run ... -c ${TEST_DIR}/nested_switch_config.yaml - ... -p test ... --unsafe + ... test-actor test ... stderr=STDOUT timeout=120s on_timeout=kill Should Not Be Equal As Integers ${result.rc} 0 @@ -145,8 +145,8 @@ Test Mixed Route Types Detection # Should detect RxPY routes even if LangGraph routes also present ${result} = Run Process ${PYTHON} -m cleveragents actor run ... -c ${TEST_DIR}/mixed_config.yaml - ... -p test ... --unsafe + ... test-actor test ... stderr=STDOUT timeout=120s on_timeout=kill Should Not Be Equal As Integers ${result.rc} 0 @@ -165,10 +165,10 @@ Test Context File Not Created On Multiple Runs FOR ${i} IN RANGE 3 ${result} = Run Process ${PYTHON} -m cleveragents actor run ... -c ${RXPY_CONFIG} - ... -p test ... --unsafe ... --context ${context_name} ... --context-dir ${CONTEXT_DIR} + ... test-actor test ... stderr=STDOUT timeout=120s on_timeout=kill Should Not Be Equal As Integers ${result.rc} 0 diff --git a/robot/scientific_paper_basic.robot b/robot/scientific_paper_basic.robot index 7dfbee65b..5e3a89da2 100644 --- a/robot/scientific_paper_basic.robot +++ b/robot/scientific_paper_basic.robot @@ -26,7 +26,7 @@ Scientific Paper Writer Basic Integration Test ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p Hello + ... test-actor Hello ... stderr=DEVNULL ... timeout=120s on_timeout=kill @@ -40,7 +40,7 @@ Scientific Paper Writer Basic Integration Test ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p !help + ... test-actor !help ... stderr=DEVNULL ... timeout=120s on_timeout=kill @@ -52,7 +52,7 @@ Scientific Paper Writer Basic Integration Test ... -c ${V2_CONFIG_DIR}/simple_echo_config.yaml ... --unsafe ... --allow-rxpy-in-run-mode - ... -p Test message + ... test-actor Test message ... stderr=DEVNULL ... timeout=120s on_timeout=kill diff --git a/robot/scientific_paper_e2e_test.robot b/robot/scientific_paper_e2e_test.robot index f596d757b..e8df0db5f 100644 --- a/robot/scientific_paper_e2e_test.robot +++ b/robot/scientific_paper_e2e_test.robot @@ -103,7 +103,7 @@ Scientific Paper Writer LaTeX Generation ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p !next latex_generation + ... test-actor !next latex_generation ... stderr=STDOUT ... timeout=180s Should Be Equal As Integers ${result.rc} 0 @@ -118,7 +118,7 @@ Scientific Paper Writer LaTeX Generation ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p Generate the complete LaTeX document + ... test-actor Generate the complete LaTeX document ... stderr=STDOUT ... timeout=180s Should Be Equal As Integers ${result.rc} 0 @@ -140,7 +140,7 @@ Scientific Paper Writer LaTeX Generation ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p !context + ... test-actor !context ... stderr=STDOUT ... timeout=30s Should Contain ${result.stdout} latex_source @@ -161,7 +161,7 @@ Scientific Paper Writer Stage Navigation ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p !stages + ... test-actor !stages ... stderr=STDOUT ... timeout=20s Should Contain ${result.stdout} intro @@ -173,7 +173,7 @@ Scientific Paper Writer Stage Navigation ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p !next + ... test-actor !next ... stderr=STDOUT ... timeout=20s Should Be Equal As Integers ${result.rc} 0 @@ -184,7 +184,7 @@ Scientific Paper Writer Stage Navigation ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p !stage + ... test-actor !stage ... stderr=STDOUT ... timeout=20s Should Not Contain ${stage_check.stdout} intro @@ -196,7 +196,7 @@ Scientific Paper Writer Stage Navigation ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p !help + ... test-actor !help ... stderr=STDOUT ... timeout=20s Should Contain ${result.stdout} Available Commands @@ -210,7 +210,7 @@ Scientific Paper Writer Stage Navigation ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p !stage + ... test-actor !stage ... stderr=STDOUT ... timeout=20s Should Contain ${result.stdout} Current Stage @@ -237,7 +237,7 @@ Run Paper Command ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p ${command} + ... test-actor ${command} ... stderr=STDOUT ... timeout=${timeout} Log Command: ${command} diff --git a/robot/scientific_paper_writer_test.robot b/robot/scientific_paper_writer_test.robot index 865c15429..3e2814b91 100644 --- a/robot/scientific_paper_writer_test.robot +++ b/robot/scientific_paper_writer_test.robot @@ -31,7 +31,7 @@ Test Context Export Import Commands ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p Hello world + ... test-actor Hello world ... timeout=120s on_timeout=kill # Export it @@ -62,7 +62,7 @@ Test Context List Command ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p First + ... test-actor First ... timeout=120s on_timeout=kill Run Process ${PYTHON} -m cleveragents actor run @@ -71,7 +71,7 @@ Test Context List Command ... --context-dir ${CONTEXT_DIR} ... --unsafe ... --allow-rxpy-in-run-mode - ... -p Second + ... test-actor Second ... timeout=120s on_timeout=kill # List them diff --git a/robot/system_prompt_template_rendering.robot b/robot/system_prompt_template_rendering.robot index 86f0d2c21..78966c7cf 100644 --- a/robot/system_prompt_template_rendering.robot +++ b/robot/system_prompt_template_rendering.robot @@ -50,7 +50,7 @@ System Prompt Template Variables Are Rendered With Runtime Context # Run the agent with --load-context - the LLM should receive a system prompt with rendered values # Since we can't easily intercept the actual LLM call, we verify the config was loaded correctly - ${result}= Run Process ${PYTHON} -m cleveragents actor run -c ${TEMP_DIR}/template_test.yaml --unsafe --allow-rxpy-in-run-mode --load-context ${TEMP_DIR}/test_context.json -p "Analyze this" + ${result}= Run Process ${PYTHON} -m cleveragents actor run -c ${TEMP_DIR}/template_test.yaml --unsafe --allow-rxpy-in-run-mode --load-context ${TEMP_DIR}/test_context.json test-actor "Analyze this" ... shell=False timeout=120s on_timeout=kill # The agent should have processed successfully (even if it times out, the config should load) diff --git a/src/cleveragents/cli/commands/_resolve_actor.py b/src/cleveragents/cli/commands/_resolve_actor.py new file mode 100644 index 000000000..a78259e07 --- /dev/null +++ b/src/cleveragents/cli/commands/_resolve_actor.py @@ -0,0 +1,122 @@ +"""Shared actor-config resolution for the ``actor run`` CLI commands. + +Both ``actor.py`` and ``actor_run.py`` need to resolve an actor name +to one or more YAML config-file paths. This module centralises that +logic so the two CLI entry-points stay DRY. + +Spec Deviation +-------------- +The specification (lines 4562-4566) defines ``agents actor run`` with +no ``--config/-c`` option. The implementation preserves ``--config`` +as an optional convenience for direct YAML invocation. When provided, +``--config`` takes precedence over registry-based name resolution. +Accepted in issue #901 AC: "`--config/-c` is removed or made optional +(for direct YAML invocation as a convenience)". +""" + +from __future__ import annotations + +import atexit +import tempfile +from pathlib import Path + +import typer +import yaml + +from cleveragents.application.container import get_container +from cleveragents.core.exceptions import NotFoundError + +# Module-level set + single atexit handler to avoid unbounded handler +# accumulation when resolve_config_files is called multiple times in the +# same process (e.g. test suites using CliRunner). +_temp_files: set[str] = set() + + +def _cleanup_temp_files() -> None: + """Remove all temporary YAML files created by resolve_config_files.""" + snapshot = list(_temp_files) + _temp_files.difference_update(snapshot) + for p in snapshot: + Path(p).unlink(missing_ok=True) + + +atexit.register(_cleanup_temp_files) + + +def _sanitize_name(name: str) -> str: + """Strip non-printable and ANSI control characters from *name*. + + Prevents user-controlled CLI input from injecting terminal escape + sequences into error messages rendered in CI logs or web dashboards. + """ + return "".join(c for c in name if c.isprintable()) + + +def resolve_config_files(name: str, config: list[Path]) -> list[Path]: + """Return config file paths, resolving *name* from the actor registry + when *config* is empty. + + When ``--config`` is provided the caller already has file paths and + *name* is ignored (backward-compatible). Otherwise *name* is looked + up in the actor registry and the YAML text stored on the actor is + written to a temporary file so the existing + :class:`ReactiveCleverAgentsApp` flow can consume it unchanged. + + Raises: + typer.Exit(code=2): When the actor name is empty, the actor is + not found in the registry and no ``--config`` was provided, + or when the actor has no configuration data. + Other exceptions from ``get_container()`` or + ``container.actor_registry()`` are **not** caught here so + callers can handle infrastructure failures in their own + ``try`` blocks. + """ + if config: + return config + + if not name or not name.strip(): + typer.echo("Error: Actor name must not be empty.", err=True) + raise typer.Exit(code=2) + + safe_name = _sanitize_name(name) + + container = get_container() + actor_registry = container.actor_registry() + + try: + actor = actor_registry.get(name) + except NotFoundError: + typer.echo( + f"Error: Actor '{safe_name}' not found in registry " + "and no --config provided.", + err=True, + ) + raise typer.Exit(code=2) from None + + yaml_text = (actor.yaml_text or "").strip() + if not yaml_text: + config_blob = actor.config_blob + if not config_blob: + typer.echo( + f"Error: Actor '{safe_name}' has no configuration data.", + err=True, + ) + raise typer.Exit(code=2) + try: + yaml_text = yaml.safe_dump(config_blob, default_flow_style=False) + except yaml.YAMLError: + typer.echo( + f"Error: Actor '{safe_name}' config_blob could not be " + "serialised to YAML.", + err=True, + ) + raise typer.Exit(code=2) from None + + with tempfile.NamedTemporaryFile( + delete=False, suffix=".yaml", mode="w", encoding="utf-8" + ) as tmp: + _temp_files.add(tmp.name) + tmp.write(yaml_text) + tmp.flush() + + return [Path(tmp.name)] diff --git a/src/cleveragents/cli/commands/actor.py b/src/cleveragents/cli/commands/actor.py index 4526322ea..7ae0582e0 100644 --- a/src/cleveragents/cli/commands/actor.py +++ b/src/cleveragents/cli/commands/actor.py @@ -5,7 +5,9 @@ import json from pathlib import Path from typing import Annotated, Any, cast +import click import typer +import yaml from rich.console import Console from rich.panel import Panel from rich.table import Table @@ -13,6 +15,9 @@ from rich.table import Table from cleveragents.actor.config import ActorConfiguration from cleveragents.actor.schema import actor_role_warnings from cleveragents.application.container import get_container +from cleveragents.cli.commands._resolve_actor import ( + resolve_config_files as _resolve_config_files, +) from cleveragents.cli.formatting import OutputFormat, format_output from cleveragents.core.exceptions import ( BusinessRuleViolation, @@ -41,8 +46,16 @@ _FORMAT_HELP = "Output format: json, yaml, plain, table, or rich (default: rich) @app.command() def run( + name: Annotated[ + str, + typer.Argument(help="Actor name to run (resolved from actor registry)"), + ], + prompt: Annotated[ + str, + typer.Argument(help="Prompt text to send to the actor"), + ], config: Annotated[ - list[Path], + list[Path] | None, typer.Option( "--config", "-c", @@ -51,10 +64,9 @@ def run( dir_okay=False, readable=True, resolve_path=True, - help="Path to one or more YAML/JSON configs", + help="YAML config paths (overrides registry-based name resolution)", ), - ], - prompt: Annotated[str, typer.Option("--prompt", "-p", help="Prompt to send")], + ] = None, output: Annotated[ Path | None, typer.Option( @@ -112,8 +124,9 @@ def run( ) -> None: """Run the reactive network once with actor-first configs.""" try: + resolved_config = _resolve_config_files(name, list(config or [])) app_exec = ReactiveCleverAgentsApp( - config_files=config, + config_files=resolved_config, verbose=verbose, unsafe=unsafe, temperature_override=temperature, @@ -137,10 +150,8 @@ def run( ctx_mgr.save_global_context(app_exec.config.global_context) return result if load_context: - import json as _json - with open(load_context, encoding="utf-8") as f: - data = _json.load(f) + data = json.load(f) if data.get("global_context") and app_exec.config: app_exec.config.global_context.update(data["global_context"]) return await app_exec.run_single_shot( @@ -172,6 +183,8 @@ def run( except UnsafeConfigurationError as exc: typer.echo(f"Error: {exc}", err=True) raise typer.Exit(code=1) from exc + except click.exceptions.Exit: + raise except CleverAgentsError as exc: typer.echo(f"Error: {exc}", err=True) raise typer.Exit(code=2) from exc @@ -210,8 +223,6 @@ def _load_config(config_path: Path | None) -> dict[str, Any] | None: data = json.loads(text) except json.JSONDecodeError: try: - import yaml - data = yaml.safe_load(text) except Exception as exc: # pragma: no cover - defensive raise typer.BadParameter(f"Failed to parse config: {exc}") from exc diff --git a/src/cleveragents/cli/commands/actor_run.py b/src/cleveragents/cli/commands/actor_run.py index b79f95e37..14b2d2cf2 100644 --- a/src/cleveragents/cli/commands/actor_run.py +++ b/src/cleveragents/cli/commands/actor_run.py @@ -1,12 +1,20 @@ from __future__ import annotations import asyncio +import json from pathlib import Path from typing import Annotated +import click import typer -from cleveragents.core.exceptions import CleverAgentsException, UnsafeConfigurationError +from cleveragents.cli.commands._resolve_actor import ( + resolve_config_files as _resolve_config_files, +) +from cleveragents.core.exceptions import ( + CleverAgentsError, + UnsafeConfigurationError, +) from cleveragents.reactive.application import ReactiveCleverAgentsApp from cleveragents.reactive.context_manager import ContextManager @@ -15,8 +23,16 @@ app = typer.Typer(help="Run reactive configs with actor-first semantics.") @app.command() def run( + name: Annotated[ + str, + typer.Argument(help="Actor name to run (resolved from actor registry)"), + ], + prompt: Annotated[ + str, + typer.Argument(help="Prompt text to send to the actor"), + ], config: Annotated[ - list[Path], + list[Path] | None, typer.Option( "--config", "-c", @@ -25,10 +41,9 @@ def run( dir_okay=False, readable=True, resolve_path=True, - help="Path to one or more YAML/JSON configs", + help="YAML config paths (overrides registry-based name resolution)", ), - ], - prompt: Annotated[str, typer.Option("--prompt", "-p", help="Prompt to send")], + ] = None, output: Annotated[ Path | None, typer.Option( @@ -85,8 +100,9 @@ def run( ) -> None: """Run the reactive network once with actor-first configs.""" try: + resolved_config = _resolve_config_files(name, list(config or [])) app_exec = ReactiveCleverAgentsApp( - config_files=config, + config_files=resolved_config, verbose=verbose, unsafe=unsafe, temperature_override=temperature, @@ -110,10 +126,8 @@ def run( ctx_mgr.save_global_context(app_exec.config.global_context) return result if load_context: - import json as _json - with open(load_context, encoding="utf-8") as f: - data = _json.load(f) + data = json.load(f) if data.get("global_context") and app_exec.config: app_exec.config.global_context.update(data["global_context"]) return await app_exec.run_single_shot( @@ -145,7 +159,9 @@ def run( except UnsafeConfigurationError as exc: typer.echo(f"Error: {exc}", err=True) raise typer.Exit(code=1) from exc - except CleverAgentsException as exc: + except click.exceptions.Exit: + raise + except CleverAgentsError as exc: typer.echo(f"Error: {exc}", err=True) raise typer.Exit(code=2) from exc except Exception as exc: # pragma: no cover