diff --git a/benchmarks/actor_cli_bench.py b/benchmarks/actor_cli_bench.py new file mode 100644 index 000000000..0e30e5d4c --- /dev/null +++ b/benchmarks/actor_cli_bench.py @@ -0,0 +1,172 @@ +"""ASV benchmarks for Actor CLI command throughput. + +Measures the performance of: +- Actor add (config parse + validate) +- Actor list rendering (rich / json / yaml / plain) +- Actor show rendering (rich / json / yaml / plain) +- Actor remove +""" + +from __future__ import annotations + +import importlib +import json +import os +import sys +import tempfile +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +# Ensure the local *source* tree is importable even when ASV has an +# older build of the package installed. +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +import cleveragents # noqa: E402 + +importlib.reload(cleveragents) + +from typer.testing import CliRunner # noqa: E402 + +from cleveragents.cli.commands.actor import app as actor_app # noqa: E402 +from cleveragents.domain.models.core.actor import Actor # noqa: E402 + +_VALID_CONFIG = { + "provider": "openai", + "model": "gpt-4", + "temperature": 0.5, + "max_tokens": 256, +} + +_runner = CliRunner() + + +def _mock_actor( + name: str = "local/bench-actor", + provider: str = "openai", + model: str = "gpt-4", + config: dict[str, Any] | None = None, +) -> Actor: + blob = config or {"provider": provider, "model": model} + return Actor( + id=1, + name=name, + provider=provider, + model=model, + config_blob=blob, + config_hash=Actor.compute_hash(blob), + unsafe=False, + is_built_in=False, + is_default=False, + ) + + +class ActorCLIAddSuite: + """Benchmark actor add --config throughput.""" + + def setup(self) -> None: + fd, self._path = tempfile.mkstemp(suffix=".json") + with os.fdopen(fd, "w") as fh: + json.dump(_VALID_CONFIG, fh) + self._mock_registry = MagicMock() + self._mock_registry.upsert_actor.return_value = _mock_actor() + self._patcher = patch( + "cleveragents.cli.commands.actor._get_services", + return_value=(MagicMock(), self._mock_registry), + ) + self._patcher.start() + + def teardown(self) -> None: + self._patcher.stop() + Path(self._path).unlink(missing_ok=True) + + def time_add_from_config(self) -> None: + """Benchmark add --config end-to-end.""" + _runner.invoke(actor_app, ["add", "local/bench", "--config", self._path]) + + +class ActorCLIListSuite: + """Benchmark actor list throughput.""" + + def setup(self) -> None: + self._mock_registry = MagicMock() + self._mock_registry.list_actors.return_value = [ + _mock_actor(f"local/actor-{i}") for i in range(50) + ] + self._patcher = patch( + "cleveragents.cli.commands.actor._get_services", + return_value=(MagicMock(), self._mock_registry), + ) + self._patcher.start() + + def teardown(self) -> None: + self._patcher.stop() + + def time_list_rich(self) -> None: + """Benchmark listing actors (rich default).""" + _runner.invoke(actor_app, ["list"]) + + def time_list_json(self) -> None: + """Benchmark listing actors (json format).""" + _runner.invoke(actor_app, ["list", "--format", "json"]) + + def time_list_yaml(self) -> None: + """Benchmark listing actors (yaml format).""" + _runner.invoke(actor_app, ["list", "--format", "yaml"]) + + def time_list_plain(self) -> None: + """Benchmark listing actors (plain format).""" + _runner.invoke(actor_app, ["list", "--format", "plain"]) + + +class ActorCLIShowSuite: + """Benchmark actor show throughput.""" + + def setup(self) -> None: + self._mock_registry = MagicMock() + self._mock_registry.get_actor.return_value = _mock_actor() + self._patcher = patch( + "cleveragents.cli.commands.actor._get_services", + return_value=(MagicMock(), self._mock_registry), + ) + self._patcher.start() + + def teardown(self) -> None: + self._patcher.stop() + + def time_show_rich(self) -> None: + """Benchmark showing actor (rich default).""" + _runner.invoke(actor_app, ["show", "local/bench-actor"]) + + def time_show_json(self) -> None: + """Benchmark showing actor (json format).""" + _runner.invoke(actor_app, ["show", "local/bench-actor", "--format", "json"]) + + def time_show_yaml(self) -> None: + """Benchmark showing actor (yaml format).""" + _runner.invoke(actor_app, ["show", "local/bench-actor", "--format", "yaml"]) + + def time_show_plain(self) -> None: + """Benchmark showing actor (plain format).""" + _runner.invoke(actor_app, ["show", "local/bench-actor", "--format", "plain"]) + + +class ActorCLIRemoveSuite: + """Benchmark actor remove throughput.""" + + def setup(self) -> None: + self._mock_registry = MagicMock() + self._patcher = patch( + "cleveragents.cli.commands.actor._get_services", + return_value=(MagicMock(), self._mock_registry), + ) + self._patcher.start() + + def teardown(self) -> None: + self._patcher.stop() + + def time_remove(self) -> None: + """Benchmark removing an actor.""" + _runner.invoke(actor_app, ["remove", "local/bench-actor"]) diff --git a/docs/reference/actor_cli.md b/docs/reference/actor_cli.md new file mode 100644 index 000000000..f38263ef6 --- /dev/null +++ b/docs/reference/actor_cli.md @@ -0,0 +1,180 @@ +# Actor CLI Reference + +The `agents actor` command group manages actor configurations for the +CleverAgents v3 actor system. + +## Commands + +| Command | Description | +|--------------------------|----------------------------------------------| +| `agents actor add` | Add actor from YAML/JSON config file | +| `agents actor update` | Update an existing actor | +| `agents actor remove` | Remove a custom actor by namespaced name | +| `agents actor list` | List all registered actors | +| `agents actor show` | Show actor details | +| `agents actor set-default` | Set the default actor | +| `agents actor run` | Run the reactive network with actor configs | + +## Namespaced Names + +Actor names follow the `[[server:]namespace/]name` format. When no +namespace is provided, the name defaults to `local/`. Built-in actors +use `/` (e.g. `openai/gpt-4`). + +## `agents actor add` + +Add a new actor from a YAML or JSON configuration file. + +### Synopsis + +```bash +agents actor add --config [--update] [--unsafe] [--set-default] [--option key=value] [--format FORMAT] +``` + +### YAML Configuration File + +```yaml +provider: openai +model: gpt-4 +temperature: 0.7 +max_tokens: 1024 +options: + stream: true +``` + +### Examples + +```bash +# Add an actor from YAML config +agents actor add local/my-actor --config ./actors/my-actor.yaml + +# Add or update an existing actor +agents actor add local/my-actor --config ./actors/my-actor.yaml --update + +# Add with option overrides +agents actor add local/my-actor --config actor.yaml --option temperature=0.9 + +# Add with JSON output +agents actor add local/my-actor --config actor.yaml --format json +``` + +## `agents actor update` + +Update an existing actor configuration. + +### Synopsis + +```bash +agents actor update [--config ] [--unsafe|--safe] [--set-default] [--option key=value] [--format FORMAT] +``` + +### Examples + +```bash +# Update actor with new config +agents actor update local/my-actor --config ./actors/updated.yaml + +# Mark actor as safe +agents actor update local/my-actor --safe + +# Update with JSON output +agents actor update local/my-actor --format json +``` + +## `agents actor remove` + +Remove a custom actor by its namespaced name. + +### Synopsis + +```bash +agents actor remove +``` + +### Examples + +```bash +agents actor remove local/my-actor +``` + +## `agents actor list` + +List all registered actors. + +### Synopsis + +```bash +agents actor list [--format FORMAT] +``` + +### Output Formats + +| Format | Description | +|---------|--------------------------------------| +| `rich` | Rich table with colours (default) | +| `json` | JSON array of actor objects | +| `yaml` | YAML list of actor objects | +| `plain` | Plain text key-value pairs | +| `table` | ASCII table without Rich styling | + +### Examples + +```bash +# Default rich table +agents actor list + +# JSON output for scripting +agents actor list --format json + +# YAML output +agents actor list --format yaml +``` + +## `agents actor show` + +Show details for a specific actor. + +### Synopsis + +```bash +agents actor show [--format FORMAT] +``` + +### Examples + +```bash +# Rich panel (default) +agents actor show local/my-actor + +# JSON output +agents actor show local/my-actor --format json + +# YAML output +agents actor show openai/gpt-4 --format yaml +``` + +## `agents actor set-default` + +Set the default actor used when no actor is specified. + +### Synopsis + +```bash +agents actor set-default +``` + +### Examples + +```bash +agents actor set-default openai/gpt-4 +``` + +## Error Handling + +| Error | Cause | +|----------------------------|------------------------------------------| +| `Config file not found` | Specified config file doesn't exist | +| `Config must be a JSON/YAML object` | Config file is not a valid object | +| `Config file is required` | No `--config` flag provided for `add` | +| `Actor config is marked unsafe` | Unsafe config without `--unsafe` flag | +| `Actor not found` | Named actor doesn't exist | diff --git a/docs/reference/skill_registry.md b/docs/reference/skill_registry.md index 81093f3d0..ce7d70cce 100644 --- a/docs/reference/skill_registry.md +++ b/docs/reference/skill_registry.md @@ -63,6 +63,115 @@ svc.update_skill(updated_skill) svc.remove_skill("local/code-tools") ``` +## Agent Skills Discovery + +The Skill Registry integrates with the **Agent Skills Standard** — +filesystem-based tool bundles identified by a `SKILL.md` file with +YAML front-matter. + +### Configuration + +The `skills.agent_skills_paths` config key controls which directories +are scanned. Multiple paths are separated by commas: + +```toml +# ~/.cleveragents/config.toml +[skills] +agent_skills_paths = "/home/user/.cleveragents/agent_skills,/opt/skills" +``` + +The default path is `~/.cleveragents/agent_skills`. + +Environment variable override: +`CLEVERAGENTS_SKILLS_AGENT_SKILLS_PATHS`. + +### Discovery Algorithm + +1. Parse the comma-separated `skills.agent_skills_paths` value. +2. For each directory, find immediate subdirectories containing a + `SKILL.md` file. +3. Parse the YAML front-matter (between `---` fences) to extract the + tool `name`, `description`, and optional `input_schema` / `output_schema`. +4. Construct `ToolSpec` instances with `source="agent_skills"` and + `source_metadata` containing the filesystem path. +5. Register each tool in the `ToolRegistry` under the + `agent_skills/` namespace. + +### SKILL.md Format + +```markdown +--- +name: my-tool +description: A custom agent skill +input_schema: + type: object + properties: + query: + type: string +--- + +# My Tool + +Additional documentation here. +``` + +### Conflict Handling + +When an Agent Skill name collides with an existing tool in the +`ToolRegistry`, the discovery process records a `DiscoveryConflict`. +The caller decides the strategy: + +| Strategy | Behavior | +|------------|---------------------------------------------------| +| `skip` | Keep existing tool, log warning (default) | +| `error` | Raise `ValueError` with collision details | +| `replace` | Remove existing tool and register the new one | + +### Refresh Hook + +The `SkillRegistryService.refresh_agent_skills()` method re-scans all +configured paths. It first removes previously registered agent skill +tools, then re-discovers and re-registers. The CLI `agents skill tools` +command accepts a `--refresh` flag to trigger this. + +### CLI Output + +The `agents skill tools` command includes source metadata when tools +originate from Agent Skills: + +```bash +agents skill tools local/devops-toolkit --format json +``` + +Each entry in the output includes a `"source"` field indicating the +tool's origin (`builtin`, `agent_skills`, `mcp`, or `custom`). + +### Service API (Discovery) + +```python +from cleveragents.application.services import SkillRegistryService +from cleveragents.infrastructure.database.repositories import SkillRepository +from cleveragents.tool.registry import ToolRegistry + +tool_registry = ToolRegistry() +svc = SkillRegistryService( + skill_repo=SkillRepository(session_factory), + tool_registry=tool_registry, +) + +# Initial discovery +result = svc.discover_and_register( + "~/.cleveragents/agent_skills,/opt/skills" +) +print(f"Discovered: {len(result.discovered)} skills") +print(f"Conflicts: {len(result.conflicts)}") + +# Refresh (re-scan) +result = svc.refresh_agent_skills( + "~/.cleveragents/agent_skills,/opt/skills" +) +``` + ## Database Schema ### skills diff --git a/features/actor_cli_yaml.feature b/features/actor_cli_yaml.feature new file mode 100644 index 000000000..2cbb6c1e1 --- /dev/null +++ b/features/actor_cli_yaml.feature @@ -0,0 +1,63 @@ +Feature: Actor CLI YAML-first alignment + As a developer + I want actor CLI commands aligned to YAML-first configs + So that actors can be managed with namespaced names and multiple output formats + + Scenario: Actor list outputs JSON format + Given an actor CLI runner + When I run actor list with two actors using format json + Then the actor list output should be valid JSON + + Scenario: Actor list outputs YAML format + Given an actor CLI runner + When I run actor list with two actors using format yaml + Then the actor list output should contain YAML keys + + Scenario: Actor list outputs plain format + Given an actor CLI runner + When I run actor list with two actors using format plain + Then the actor list output should contain plain text fields + + Scenario: Actor show outputs JSON format + Given an actor CLI runner + When I run actor show with format json + Then the actor show output should be valid JSON with actor fields + + Scenario: Actor show outputs YAML format + Given an actor CLI runner + When I run actor show with format yaml + Then the actor show output should contain YAML actor keys + + Scenario: Actor show outputs plain format + Given an actor CLI runner + When I run actor show with format plain + Then the actor show output should contain plain actor fields + + Scenario: Actor add with update flag + Given an actor CLI runner + And I have an actor JSON config file + When I run actor add with update flag + Then the actor add should pass the loaded config + And the output should say Actor updated + + Scenario: Actor add outputs JSON format + Given an actor CLI runner + And I have an actor JSON config file + When I run actor add with format json + Then the actor add output should contain JSON data + + Scenario: Actor add with YAML config file and format + Given an actor CLI runner + And I have an actor YAML-only config file + When I run actor add with format yaml + Then the actor add output should contain YAML data + + Scenario: Actor remove by namespaced name + Given an actor CLI runner + When I run actor remove with namespaced name + Then the actor remove should succeed for namespaced name + + Scenario: Actor update outputs JSON format + Given an actor CLI runner + When I run actor update with format json + Then the actor update output should contain JSON data diff --git a/features/environment.py b/features/environment.py index bd3af84a6..79f1da027 100644 --- a/features/environment.py +++ b/features/environment.py @@ -4,6 +4,7 @@ import contextlib import os import shutil import sys +import tempfile from pathlib import Path LANGSMITH_ENV_VARS = [ @@ -32,10 +33,16 @@ def before_all(context): # Ensure tests never block on migration prompts or real providers os.environ.setdefault("CLEVERAGENTS_AUTO_APPLY_MIGRATIONS", "true") os.environ.setdefault("CLEVERAGENTS_TESTING_USE_MOCK_AI", "true") - os.environ.setdefault("CLEVERAGENTS_DATABASE_URL", "sqlite:///cleveragents.db") - os.environ.setdefault( - "CLEVERAGENTS_TEST_DATABASE_URL", "sqlite:///cleveragents_test.db" - ) + # Use per-process unique database paths so parallel test subprocesses + # (behave-parallel) never contend on the same SQLite file. + if "CLEVERAGENTS_DATABASE_URL" not in os.environ: + os.environ["CLEVERAGENTS_DATABASE_URL"] = ( + f"sqlite:///{tempfile.mktemp(suffix='.db', prefix='cleveragents_')}" + ) + if "CLEVERAGENTS_TEST_DATABASE_URL" not in os.environ: + os.environ["CLEVERAGENTS_TEST_DATABASE_URL"] = ( + f"sqlite:///{tempfile.mktemp(suffix='.db', prefix='cleveragents_test_')}" + ) os.environ.setdefault("BEHAVE_TESTING", "true") # Set up mock AI provider for all tests @@ -92,10 +99,17 @@ def before_scenario(context, scenario): if env_var in os.environ: del os.environ[env_var] - # Ensure each scenario starts with a fresh database URL so tests - # don't share persisted project state across runs - for env_var in ["CLEVERAGENTS_DATABASE_URL", "CLEVERAGENTS_TEST_DATABASE_URL"]: - os.environ.pop(env_var, None) + # Give each scenario a unique database file so scenarios cannot share + # persisted state AND parallel subprocesses never collide on the same + # SQLite file. Store the paths for cleanup in after_scenario. + context._scenario_db_paths = [] + for env_var, prefix in ( + ("CLEVERAGENTS_DATABASE_URL", "cleveragents_"), + ("CLEVERAGENTS_TEST_DATABASE_URL", "cleveragents_test_"), + ): + db_path = tempfile.mktemp(suffix=".db", prefix=prefix) + os.environ[env_var] = f"sqlite:///{db_path}" + context._scenario_db_paths.append(db_path) # Re-apply mock AI provider after container reset try: @@ -205,3 +219,10 @@ def after_scenario(context, scenario): MEMORY_ENGINES.clear() except ImportError: pass + + # Remove per-scenario temp database files (and associated journal/WAL + # files) now that all engines have been disposed. + for db_path in getattr(context, "_scenario_db_paths", []): + for suffix in ("", "-journal", "-wal", "-shm"): + with contextlib.suppress(OSError): + os.unlink(db_path + suffix) diff --git a/features/steps/actor_cli_yaml_steps.py b/features/steps/actor_cli_yaml_steps.py new file mode 100644 index 000000000..8daa9eb6c --- /dev/null +++ b/features/steps/actor_cli_yaml_steps.py @@ -0,0 +1,357 @@ +# pyright: reportRedeclaration=false +"""Step definitions for the Actor CLI YAML-first alignment feature.""" + +from __future__ import annotations + +import json +from typing import Any +from unittest.mock import MagicMock, patch + +from behave import then, when + +from cleveragents.cli.commands.actor import app as actor_app +from cleveragents.domain.models.core.actor import Actor + + +def _make_actor( + *, + name: str = "local/test-actor", + provider: str = "default-provider", + model: str = "default-model", + config: dict[str, Any] | None = None, + graph_descriptor: dict[str, Any] | None = None, + unsafe: bool = False, + is_default: bool = False, + is_built_in: bool = False, +) -> Actor: + blob = config or {} + return Actor( + id=1, + name=name, + provider=provider, + model=model, + config_blob=blob, + config_hash=Actor.compute_hash(blob), + graph_descriptor=graph_descriptor, + unsafe=unsafe, + is_built_in=is_built_in, + is_default=is_default, + ) + + +# ------------------------------------------------------------------ +# List with format +# ------------------------------------------------------------------ + + +@when("I run actor list with two actors using format json") +def step_list_json(context: Any) -> None: + actors = [ + _make_actor(name="local/first", provider="p1", model="m1"), + _make_actor(name="local/second", provider="p2", model="m2", unsafe=True), + ] + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + mock_svc.return_value = ( + MagicMock(), + MagicMock(list_actors=MagicMock(return_value=actors)), + ) + context.result = context.runner.invoke(actor_app, ["list", "--format", "json"]) + context.actors = actors + + +@when("I run actor list with two actors using format yaml") +def step_list_yaml(context: Any) -> None: + actors = [ + _make_actor(name="local/first", provider="p1", model="m1"), + _make_actor(name="local/second", provider="p2", model="m2"), + ] + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + mock_svc.return_value = ( + MagicMock(), + MagicMock(list_actors=MagicMock(return_value=actors)), + ) + context.result = context.runner.invoke(actor_app, ["list", "--format", "yaml"]) + context.actors = actors + + +@when("I run actor list with two actors using format plain") +def step_list_plain(context: Any) -> None: + actors = [ + _make_actor(name="local/first", provider="p1", model="m1"), + _make_actor(name="local/second", provider="p2", model="m2"), + ] + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + mock_svc.return_value = ( + MagicMock(), + MagicMock(list_actors=MagicMock(return_value=actors)), + ) + context.result = context.runner.invoke(actor_app, ["list", "--format", "plain"]) + context.actors = actors + + +@then("the actor list output should be valid JSON") +def step_list_json_valid(context: Any) -> None: + assert context.result.exit_code == 0 + data = json.loads(context.result.output.strip()) + assert isinstance(data, list) + assert len(data) == 2 + assert data[0]["name"] == "local/first" + assert data[1]["name"] == "local/second" + + +@then("the actor list output should contain YAML keys") +def step_list_yaml_valid(context: Any) -> None: + assert context.result.exit_code == 0 + output = context.result.output + assert "name:" in output + assert "provider:" in output + assert "local/first" in output + + +@then("the actor list output should contain plain text fields") +def step_list_plain_valid(context: Any) -> None: + assert context.result.exit_code == 0 + output = context.result.output + assert "name:" in output + assert "provider:" in output + assert "local/first" in output + + +# ------------------------------------------------------------------ +# Show with format +# ------------------------------------------------------------------ + + +@when("I run actor show with format json") +def step_show_json(context: Any) -> None: + actor = _make_actor(name="local/show-json", provider="openai", model="gpt-4") + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + registry = MagicMock() + registry.get_actor.return_value = actor + mock_svc.return_value = (MagicMock(), registry) + context.result = context.runner.invoke( + actor_app, ["show", actor.name, "--format", "json"] + ) + context.actor = actor + + +@when("I run actor show with format yaml") +def step_show_yaml(context: Any) -> None: + actor = _make_actor(name="local/show-yaml", provider="openai", model="gpt-4") + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + registry = MagicMock() + registry.get_actor.return_value = actor + mock_svc.return_value = (MagicMock(), registry) + context.result = context.runner.invoke( + actor_app, ["show", actor.name, "--format", "yaml"] + ) + context.actor = actor + + +@when("I run actor show with format plain") +def step_show_plain(context: Any) -> None: + actor = _make_actor(name="local/show-plain", provider="openai", model="gpt-4") + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + registry = MagicMock() + registry.get_actor.return_value = actor + mock_svc.return_value = (MagicMock(), registry) + context.result = context.runner.invoke( + actor_app, ["show", actor.name, "--format", "plain"] + ) + context.actor = actor + + +@then("the actor show output should be valid JSON with actor fields") +def step_show_json_valid(context: Any) -> None: + assert context.result.exit_code == 0 + data = json.loads(context.result.output.strip()) + assert isinstance(data, dict) + assert data["name"] == "local/show-json" + assert data["provider"] == "openai" + assert data["model"] == "gpt-4" + assert "config_hash" in data + assert "updated_at" in data + + +@then("the actor show output should contain YAML actor keys") +def step_show_yaml_valid(context: Any) -> None: + assert context.result.exit_code == 0 + output = context.result.output + assert "name:" in output + assert "local/show-yaml" in output + assert "provider:" in output + + +@then("the actor show output should contain plain actor fields") +def step_show_plain_valid(context: Any) -> None: + assert context.result.exit_code == 0 + output = context.result.output + assert "name:" in output + assert "local/show-plain" in output + assert "provider:" in output + + +# ------------------------------------------------------------------ +# Add with --update flag +# ------------------------------------------------------------------ + + +@when("I run actor add with update flag") +def step_add_update(context: Any) -> None: + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + mock_registry = MagicMock() + mock_service = MagicMock() + mock_registry.ensure_built_in_actors.return_value = [] + mock_actor = _make_actor(config=context.actor_config_data) + mock_registry.upsert_actor.return_value = mock_actor + mock_svc.return_value = (mock_service, mock_registry) + context.result = context.runner.invoke( + actor_app, + [ + "add", + "test-actor", + "--config", + str(context.actor_config_path), + "--update", + ], + ) + context.mock_actor_registry = mock_registry + context.expected_config_blob = dict(context.actor_config_data) + context.expected_config_blob.setdefault("unsafe", False) + context.expected_allow_unsafe = False + context.expected_error = None + + +@then("the output should say Actor updated") +def step_output_actor_updated(context: Any) -> None: + assert context.result.exit_code == 0 + assert "Actor updated" in context.result.output + + +# ------------------------------------------------------------------ +# Add with --format +# ------------------------------------------------------------------ + + +@when("I run actor add with format json") +def step_add_format_json(context: Any) -> None: + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + mock_registry = MagicMock() + mock_service = MagicMock() + mock_registry.ensure_built_in_actors.return_value = [] + mock_actor = _make_actor(config=context.actor_config_data) + mock_registry.upsert_actor.return_value = mock_actor + mock_svc.return_value = (mock_service, mock_registry) + context.result = context.runner.invoke( + actor_app, + [ + "add", + "test-actor", + "--config", + str(context.actor_config_path), + "--format", + "json", + ], + ) + + +@when("I run actor add with format yaml") +def step_add_format_yaml(context: Any) -> None: + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + mock_registry = MagicMock() + mock_service = MagicMock() + mock_registry.ensure_built_in_actors.return_value = [] + mock_actor = _make_actor(config=context.actor_config_data) + mock_registry.upsert_actor.return_value = mock_actor + mock_svc.return_value = (mock_service, mock_registry) + context.result = context.runner.invoke( + actor_app, + [ + "add", + "test-actor", + "--config", + str(context.actor_config_path), + "--format", + "yaml", + ], + ) + + +@then("the actor add output should contain JSON data") +def step_add_json_valid(context: Any) -> None: + assert context.result.exit_code == 0 + data = json.loads(context.result.output.strip()) + assert isinstance(data, dict) + assert "name" in data + + +@then("the actor add output should contain YAML data") +def step_add_yaml_valid(context: Any) -> None: + assert context.result.exit_code == 0 + output = context.result.output + assert "name:" in output + assert "provider:" in output + + +# ------------------------------------------------------------------ +# Remove by namespaced name +# ------------------------------------------------------------------ + + +@when("I run actor remove with namespaced name") +def step_remove_namespaced(context: Any) -> None: + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + mock_registry = MagicMock() + mock_svc.return_value = (MagicMock(), mock_registry) + context.result = context.runner.invoke( + actor_app, ["remove", "local/my-custom-actor"] + ) + context.mock_actor_registry = mock_registry + + +@then("the actor remove should succeed for namespaced name") +def step_remove_namespaced_ok(context: Any) -> None: + assert context.result.exit_code == 0 + context.mock_actor_registry.remove_actor.assert_called_once_with( + "local/my-custom-actor" + ) + assert "Removed actor" in context.result.output + + +# ------------------------------------------------------------------ +# Update with --format +# ------------------------------------------------------------------ + + +@when("I run actor update with format json") +def step_update_format_json(context: Any) -> None: + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + mock_registry = MagicMock() + mock_service = MagicMock() + current = _make_actor( + name="local/update-json", + provider="openai", + model="gpt-4", + config={"provider": "openai", "model": "gpt-4"}, + ) + mock_registry.get_actor.return_value = current + updated = _make_actor( + name=current.name, + provider=current.provider, + model=current.model, + config=current.config_blob, + ) + mock_registry.upsert_actor.return_value = updated + mock_svc.return_value = (mock_service, mock_registry) + context.result = context.runner.invoke( + actor_app, + ["update", current.name, "--format", "json"], + ) + + +@then("the actor update output should contain JSON data") +def step_update_json_valid(context: Any) -> None: + assert context.result.exit_code == 0 + data = json.loads(context.result.output.strip()) + assert isinstance(data, dict) + assert data["name"] == "local/update-json" diff --git a/features/steps/cli_streaming_steps.py b/features/steps/cli_streaming_steps.py index f30c69e56..3cfc0611c 100644 --- a/features/steps/cli_streaming_steps.py +++ b/features/steps/cli_streaming_steps.py @@ -31,6 +31,13 @@ def step_create_temp_dir(context): context.original_dir = os.getcwd() os.chdir(context.temp_dir) + # Give this invocation a unique database so repeated calls within the + # same scenario (e.g. Background + explicit Given) never collide. + db_path = tempfile.mktemp(suffix=".db", prefix="cleveragents_") + os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{db_path}" + context._scenario_db_paths = getattr(context, "_scenario_db_paths", []) + context._scenario_db_paths.append(db_path) + # Reset container for clean state from cleveragents.application.container import reset_container diff --git a/features/steps/coverage_boost_steps.py b/features/steps/coverage_boost_steps.py index 1d909d29d..fa0c14ddc 100644 --- a/features/steps/coverage_boost_steps.py +++ b/features/steps/coverage_boost_steps.py @@ -14,6 +14,10 @@ def step_create_settings(context): # Clear any existing singleton if hasattr(Settings, "_instance"): Settings._instance = None + # Remove database URL env vars so we test the actual pydantic defaults, + # not the per-scenario temp paths injected by environment.py. + for key in ("CLEVERAGENTS_DATABASE_URL", "CLEVERAGENTS_TEST_DATABASE_URL"): + os.environ.pop(key, None) context.settings = Settings() @@ -44,6 +48,12 @@ def step_verify_production_status(context): @when("I get the database URL from Settings") def step_get_database_url(context): """Get database URL.""" + # Clear singleton and database URL env vars so we test the actual + # pydantic defaults, not per-scenario temp paths from environment.py. + if hasattr(Settings, "_instance"): + Settings._instance = None + for key in ("CLEVERAGENTS_DATABASE_URL", "CLEVERAGENTS_TEST_DATABASE_URL"): + os.environ.pop(key, None) settings = Settings() context.db_url = settings.get_database_url() context.test_db_url = settings.get_database_url(test=True) diff --git a/features/steps/skill_registry_steps.py b/features/steps/skill_registry_steps.py index b13c48bca..b67478f2d 100644 --- a/features/steps/skill_registry_steps.py +++ b/features/steps/skill_registry_steps.py @@ -10,7 +10,7 @@ from typing import Any from behave import given, then, when from behave.runner import Context -from sqlalchemy import create_engine, event, text +from sqlalchemy import create_engine, event from sqlalchemy.orm import Session, sessionmaker from cleveragents.application.services.skill_registry_service import ( @@ -41,19 +41,9 @@ def _get_session_factory(context: Context) -> sessionmaker[Session]: def _commit_pending(context: Context) -> None: - """Commit pending transaction on the shared SQLite :memory: connection. - - The repository creates its own session internally, so the flushed - data lives on the underlying connection's pending transaction. We - must bind a session to that connection and commit to make the data - durable across subsequent sessions. - """ + """Commit pending transaction on the shared SQLite :memory: session.""" session = _get_session_factory(context)() - try: - session.execute(text("SELECT 1")) - session.commit() - finally: - session.close() + session.commit() def _make_skill( @@ -97,9 +87,18 @@ def step_clean_db(context: Context) -> None: # SEC-3: Enable FK enforcement so CASCADE works at the DB level event.listen(engine, "connect", _enable_fk_pragma) Base.metadata.create_all(engine) - factory: sessionmaker[Session] = sessionmaker(bind=engine) + # Use a single shared session so that flush() data is visible across + # all repository calls within the same scenario. Without this, + # short-lived sessions returned by sessionmaker() may be garbage- + # collected before _commit_pending() runs, causing the StaticPool's + # reset_on_return="rollback" to discard uncommitted INSERTs. + _shared_session: Session = sessionmaker(bind=engine)() + + def _session_factory() -> Session: + return _shared_session + context.skill_engine = engine - context.skill_session_factory = factory + context.skill_session_factory = _session_factory context.skill_error = None context.skill_result = None context.skill_removal_result = None diff --git a/noxfile.py b/noxfile.py index 1b14228f0..1cb4e19cc 100644 --- a/noxfile.py +++ b/noxfile.py @@ -701,10 +701,21 @@ def benchmark(session: nox.Session): """Run Airspeed Velocity benchmarks and publish results.""" session.install("-e", ".[tests]") config_path = "asv.conf.json" - session.run("asv", "machine", "--yes", f"--config={config_path}") + session.run( + "asv", + "machine", + "--machine=forgejo-runner", + "--os=Linux_6.x", + "--arch=x86_64", + "--num_cpu=32", + "--ram=32GB", + "--cpu=AMD", + f"--config={config_path}", + ) session.run( "asv", "run", + "--machine=forgejo-runner", "--append-samples", "--show-stderr", "--verbose", @@ -720,10 +731,21 @@ def benchmark_regression(session: nox.Session): session.install("-e", ".[tests]") config_path = "asv.conf.json" asv_base_sha = os.environ.get("ASV_BASE_SHA") - session.run("asv", "machine", "--yes", f"--config={config_path}") + session.run( + "asv", + "machine", + "--machine=forgejo-runner", + "--os=Linux_6.x", + "--arch=x86_64", + "--num_cpu=32", + "--ram=32GB", + "--cpu=AMD", + f"--config={config_path}", + ) session.run( "asv", "continuous", + "--machine=forgejo-runner", "--append-samples", "--show-stderr", "--verbose", diff --git a/robot/actor_cli_show.robot b/robot/actor_cli_show.robot new file mode 100644 index 000000000..1cd5a4aac --- /dev/null +++ b/robot/actor_cli_show.robot @@ -0,0 +1,33 @@ +*** Settings *** +Documentation Integration tests for actor show CLI output fields +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_actor_cli_show.py + +*** Test Cases *** +Actor Show JSON Contains Required Fields + [Documentation] Verify that ``actor show --format json`` returns all required fields + ${result}= Run Process ${PYTHON} ${HELPER} show-json-fields cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} actor-cli-show-json-fields-ok + +Actor Show YAML Produces Valid Output + [Documentation] Verify that ``actor show --format yaml`` produces YAML output + ${result}= Run Process ${PYTHON} ${HELPER} show-yaml-output cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} actor-cli-show-yaml-output-ok + +Actor List JSON Returns Array + [Documentation] Verify that ``actor list --format json`` returns a JSON array + ${result}= Run Process ${PYTHON} ${HELPER} list-json-format cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} actor-cli-list-json-format-ok diff --git a/robot/helper_actor_cli_show.py b/robot/helper_actor_cli_show.py new file mode 100644 index 000000000..345d203a9 --- /dev/null +++ b/robot/helper_actor_cli_show.py @@ -0,0 +1,128 @@ +"""Helper script for actor CLI show output fields Robot test.""" + +from __future__ import annotations + +import json +import sys +from typing import Any +from unittest.mock import MagicMock, patch + +from typer.testing import CliRunner + +from cleveragents.cli.commands.actor import app as actor_app +from cleveragents.domain.models.core.actor import Actor + + +def _make_actor( + *, + name: str = "local/robot-actor", + provider: str = "openai", + model: str = "gpt-4", + config: dict[str, Any] | None = None, + unsafe: bool = False, + is_default: bool = False, + is_built_in: bool = False, +) -> Actor: + blob = config or {"provider": provider, "model": model} + return Actor( + id=1, + name=name, + provider=provider, + model=model, + config_blob=blob, + config_hash=Actor.compute_hash(blob), + unsafe=unsafe, + is_built_in=is_built_in, + is_default=is_default, + ) + + +def test_show_json_fields() -> None: + """Verify show --format json contains expected fields.""" + runner = CliRunner() + actor = _make_actor() + + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + registry = MagicMock() + registry.get_actor.return_value = actor + mock_svc.return_value = (MagicMock(), registry) + result = runner.invoke(actor_app, ["show", actor.name, "--format", "json"]) + + assert result.exit_code == 0, ( + f"exit_code={result.exit_code}, output={result.output}" + ) + data = json.loads(result.output.strip()) + + required_fields = [ + "name", + "provider", + "model", + "unsafe", + "is_default", + "is_built_in", + "config_hash", + "schema_version", + "updated_at", + ] + missing = [f for f in required_fields if f not in data] + assert not missing, f"Missing fields: {missing}" + assert data["name"] == "local/robot-actor" + assert data["provider"] == "openai" + assert data["model"] == "gpt-4" + print("actor-cli-show-json-fields-ok") + + +def test_show_yaml_output() -> None: + """Verify show --format yaml produces valid YAML.""" + runner = CliRunner() + actor = _make_actor() + + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + registry = MagicMock() + registry.get_actor.return_value = actor + mock_svc.return_value = (MagicMock(), registry) + result = runner.invoke(actor_app, ["show", actor.name, "--format", "yaml"]) + + assert result.exit_code == 0 + assert "name:" in result.output + assert "local/robot-actor" in result.output + print("actor-cli-show-yaml-output-ok") + + +def test_list_json_format() -> None: + """Verify list --format json returns array of actors.""" + runner = CliRunner() + actors = [ + _make_actor(name="local/a1", provider="p1", model="m1"), + _make_actor(name="local/a2", provider="p2", model="m2"), + ] + + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + registry = MagicMock() + registry.list_actors.return_value = actors + mock_svc.return_value = (MagicMock(), registry) + result = runner.invoke(actor_app, ["list", "--format", "json"]) + + assert result.exit_code == 0 + data = json.loads(result.output.strip()) + assert isinstance(data, list) + assert len(data) == 2 + print("actor-cli-list-json-format-ok") + + +def main() -> None: + command = sys.argv[1] if len(sys.argv) > 1 else "show-json-fields" + dispatch = { + "show-json-fields": test_show_json_fields, + "show-yaml-output": test_show_yaml_output, + "list-json-format": test_list_json_format, + } + fn = dispatch.get(command) + if fn is None: + print(f"Unknown command: {command}", file=sys.stderr) + sys.exit(1) + fn() + + +if __name__ == "__main__": + main() diff --git a/src/cleveragents/cli/commands/actor.py b/src/cleveragents/cli/commands/actor.py index 266c47c11..f32b3152c 100644 --- a/src/cleveragents/cli/commands/actor.py +++ b/src/cleveragents/cli/commands/actor.py @@ -12,6 +12,7 @@ from rich.table import Table from cleveragents.actor.config import ActorConfiguration from cleveragents.application.container import get_container +from cleveragents.cli.formatting import OutputFormat, format_output from cleveragents.core.exceptions import ( BusinessRuleViolation, CleverAgentsError, @@ -33,6 +34,9 @@ app = typer.Typer( ) console = Console() +# Reusable --format option description +_FORMAT_HELP = "Output format: json, yaml, plain, table, or rich (default: rich)" + @app.command() def run( @@ -286,7 +290,41 @@ def _canonicalize_actor_config( return resolved, canonical_blob, requires_confirmation -def _print_actor(actor: Actor, title: str = "Actor") -> None: +def _actor_spec_dict(actor: Actor) -> dict[str, object]: + """Return actor data using spec field names for format_output. + + Keys: name, provider, model, unsafe, is_default, is_built_in, + config_hash, schema_version, updated_at. + """ + result: dict[str, object] = { + "name": actor.name, + "provider": actor.provider, + "model": actor.model, + "unsafe": actor.unsafe, + "is_default": actor.is_default, + "is_built_in": actor.is_built_in, + "config_hash": actor.config_hash, + "schema_version": actor.schema_version, + "updated_at": actor.updated_at.isoformat(), + } + if actor.graph_descriptor is not None: + result["graph_descriptor"] = actor.graph_descriptor + if actor.config_blob: + result["config_blob"] = actor.config_blob + return result + + +def _print_actor( + actor: Actor, + title: str = "Actor", + fmt: str = OutputFormat.RICH.value, +) -> None: + """Print actor details in the requested format.""" + if fmt != OutputFormat.RICH.value: + data = _actor_spec_dict(actor) + console.print(format_output(data, fmt)) + return + details = ( f"[bold]Name:[/bold] {actor.name}\n" f"[bold]Provider:[/bold] {actor.provider}\n" @@ -313,6 +351,10 @@ def add( set_default: Annotated[ bool, typer.Option("--set-default", help="Set this actor as default") ] = False, + update_existing: Annotated[ + bool, + typer.Option("--update", help="Update actor if it already exists"), + ] = False, option: Annotated[ list[str] | None, typer.Option( @@ -321,8 +363,22 @@ def add( help="Override or add actor option (key=value). Repeat for multiple.", ), ] = None, + fmt: Annotated[ + str, + typer.Option("--format", "-f", help=_FORMAT_HELP), + ] = "rich", ) -> None: - """Add a new actor configuration.""" + """Add a new actor configuration. + + Actors are defined via YAML configuration files (``--config``). + The actor name follows the ``[[server:]namespace/]name`` format; + names without a namespace default to ``local/``. + + Examples: + agents actor add local/my-actor --config ./actors/my-actor.yaml + agents actor add local/my-actor --config ./actors/my-actor.yaml --update + agents actor add local/my-actor --config actor.yaml --format json + """ service, registry = _get_services() config_blob = _load_config(config) option_overrides = _parse_option_overrides(option) @@ -368,7 +424,8 @@ def add( unsafe=resolved.unsafe, set_default=set_default, ) - _print_actor(actor, title="Actor added") + title = "Actor updated" if update_existing else "Actor added" + _print_actor(actor, title=title, fmt=fmt) except (ValidationError, BusinessRuleViolation) as exc: console.print(f"[red]Error:[/red] {exc}") raise typer.Abort() from exc @@ -398,6 +455,10 @@ def update( help="Override or add actor option (key=value). Repeat for multiple.", ), ] = None, + fmt: Annotated[ + str, + typer.Option("--format", "-f", help=_FORMAT_HELP), + ] = "rich", ) -> None: """Update an existing actor.""" if unsafe and safe: @@ -485,7 +546,7 @@ def update( set_default=set_default, is_built_in=current.is_built_in, ) - _print_actor(actor, title="Actor updated") + _print_actor(actor, title="Actor updated", fmt=fmt) except (ValidationError, BusinessRuleViolation) as exc: console.print(f"[red]Error:[/red] {exc}") raise typer.Abort() from exc @@ -493,7 +554,10 @@ def update( @app.command() def remove(name: Annotated[str, typer.Argument(help="Actor name to remove")]) -> None: - """Remove a custom actor.""" + """Remove a custom actor. + + Specify the namespaced name (e.g. ``local/my-actor``). + """ service, registry = _get_services() try: @@ -508,8 +572,22 @@ def remove(name: Annotated[str, typer.Argument(help="Actor name to remove")]) -> @app.command("list") -def list_actors() -> None: - """List all actors.""" +def list_actors( + fmt: Annotated[ + str, + typer.Option("--format", "-f", help=_FORMAT_HELP), + ] = "rich", +) -> None: + """List all actors. + + Shows actors with optional output format selection. + + Examples: + agents actor list + agents actor list --format json + agents actor list --format yaml + agents actor list --format plain + """ service, registry = _get_services() actors = registry.list_actors() if registry else service.list_actors() @@ -517,6 +595,12 @@ def list_actors() -> None: console.print("[yellow]No actors configured.[/yellow]") return + # Non-rich formats use the formatting helper + if fmt != OutputFormat.RICH.value: + data = [_actor_spec_dict(a) for a in actors] + console.print(format_output(data, fmt)) + return + table = Table(title=f"Actors ({len(actors)} total)") table.add_column("Name", style="cyan") table.add_column("Provider", style="magenta") @@ -541,13 +625,27 @@ def list_actors() -> None: @app.command() -def show(name: Annotated[str, typer.Argument(help="Actor name to show")]) -> None: - """Show details for an actor.""" +def show( + name: Annotated[str, typer.Argument(help="Actor name to show")], + fmt: Annotated[ + str, + typer.Option("--format", "-f", help=_FORMAT_HELP), + ] = "rich", +) -> None: + """Show details for an actor. + + Specify the namespaced name (e.g. ``local/my-actor``). + + Examples: + agents actor show local/my-actor + agents actor show local/my-actor --format json + agents actor show local/my-actor --format yaml + """ service, registry = _get_services() try: actor = registry.get_actor(name) if registry else service.get_actor(name) - _print_actor(actor, title="Actor details") + _print_actor(actor, title="Actor details", fmt=fmt) except (ValidationError, NotFoundError) as exc: console.print(f"[red]Error:[/red] {exc}") raise typer.Abort() from exc