diff --git a/CHANGELOG.md b/CHANGELOG.md index abb8dd788..a376dc67c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,15 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **Built-in actors v3 YAML format** (#10883): Fixed `agents actor run` failing for + built-in actors (e.g., `openai/gpt-4`, `anthropic/claude-3-opus`) due to missing + v3 `type` field in stored configuration. `ActorRegistry.ensure_built_in_actors()` + now generates and persists v3 YAML text with `type: llm` and `description` fields, + ensuring built-in actors work identically to custom actors. The + `_generate_builtin_actor_yaml()` helper creates spec-compliant YAML that passes + `ReactiveConfigParser._is_v3_format()` validation. Includes BDD scenarios and unit + tests covering YAML generation, schema validation, and multiple provider handling. + - **Atomic `server_connect` config writes** (#993): Fixed `server_connect` in `cli/commands/server.py` to write all three config values (`server.url`, `server.namespace`, `server.tls-verify`) atomically. A snapshot of the config diff --git a/features/builtin_actor_v3_yaml.feature b/features/builtin_actor_v3_yaml.feature new file mode 100644 index 000000000..c2765c2d9 --- /dev/null +++ b/features/builtin_actor_v3_yaml.feature @@ -0,0 +1,33 @@ +Feature: Built-in actors work with v3 YAML format + As a developer + I want built-in actors to include v3 YAML text with type field + So that agents actor run works identically for built-in and custom actors + + Scenario: Built-in actor YAML includes type field + Given a configured provider registry with openai/gpt-4 + When built-in actors are ensured + Then the openai/gpt-4 actor should have yaml_text + And the yaml_text should contain "type: llm" + And the yaml_text should contain "description:" + + Scenario: Built-in actor YAML includes required v3 fields + Given a configured provider registry with anthropic/claude-3-opus + When built-in actors are ensured + Then the anthropic/claude-3-opus actor yaml_text should contain "name:" + And the yaml_text should contain "type: llm" + And the yaml_text should contain "model:" + And the yaml_text should contain "provider:" + And the yaml_text should contain "source: provider-registry" + + Scenario: Built-in actor works with agents actor run command + Given a configured provider registry with openai/gpt-4 + And the built-in actors have been ensured + When I call agents actor run with mock "openai/gpt-4" + Then the command should not fail with v3 format error + And the config parser should recognize the actor as v3 format + + Scenario: Built-in actor YAML structure matches v3 schema + Given a configured provider registry with openai/gpt-4 + When built-in actors are ensured + Then the generated yaml_text should be valid v3 ActorConfigSchema + And the yaml_text should parse without errors diff --git a/features/steps/builtin_actor_v3_yaml_steps.py b/features/steps/builtin_actor_v3_yaml_steps.py new file mode 100644 index 000000000..5106a29d4 --- /dev/null +++ b/features/steps/builtin_actor_v3_yaml_steps.py @@ -0,0 +1,181 @@ +"""Behave steps for built-in actor v3 YAML format.""" + +from __future__ import annotations + +import yaml +from behave import given, then, when +from behave.runner import Context + +from cleveragents.actor.schema import ActorConfigSchema, is_v3_yaml +from cleveragents.providers.registry import ( + ProviderCapabilities, + ProviderInfo, + ProviderType, +) + + +@given("a configured provider registry with {provider_model}") +def step_configured_provider(context: Context, provider_model: str) -> None: + """Set up a stubbed provider registry with a single provider. + + Args: + context: Behave context. + provider_model: Provider/model pair (e.g., "openai/gpt-4"). + """ + parts = provider_model.split("/") + provider_name = parts[0] + model_name = parts[1] if len(parts) > 1 else "default" + + from features.steps.actor_registry_steps import ( + _StubActorService, + _StubProviderRegistry, + _StubSettings, + _default_defaults, + ) + + context.actor_service = _StubActorService() + context.provider_registry = _StubProviderRegistry( + [ + ProviderInfo( + provider_type=ProviderType.OPENAI + if provider_name == "openai" + else ProviderType.ANTHROPIC, + name=provider_name, + api_key_env_var="ENV", + default_model=model_name, + capabilities=ProviderCapabilities(supports_tool_calls=True), + is_configured=True, + ) + ] + ) + context.settings = _StubSettings(_default_defaults()) + + from cleveragents.actor.registry import ActorRegistry + + context.registry = ActorRegistry( + actor_service=context.actor_service, + provider_registry=context.provider_registry, + settings=context.settings, + ) + + +@given("the built-in actors have been ensured") +def step_builtin_actors_ensured_as_given(context: Context) -> None: + """Ensure built-in actors as a precondition and set current actor.""" + context.registry.ensure_built_in_actors() + if context.actor_service.actors: + context.current_actor = next(iter(context.actor_service.actors.values())) + + +@when("built-in actors are ensured") +def step_ensure_builtin_actors(context: Context) -> None: + """Call ensure_built_in_actors on the registry.""" + context.generated_actors = context.registry.ensure_built_in_actors() + if context.actor_service.actors: + context.current_actor = next(iter(context.actor_service.actors.values())) + + +@when('I call agents actor run with mock "{actor_name}"') +def step_call_actor_run_with_mock(context: Context, actor_name: str) -> None: + """Simulate the actor-lookup phase of ``agents actor run`` using stub state. + + This step intentionally does NOT invoke the real CLI or any LLM. It only + exercises the part that matters for this feature: that the stub actor + service contains the actor and that ``context.current_actor`` is set so + subsequent assertion steps can inspect the v3 YAML. + """ + actor = context.actor_service.get_actor(actor_name) + assert actor is not None, ( + f"Actor '{actor_name}' not found in stub service after ensure_built_in_actors" + ) + context.current_actor = actor + + +@then("the {actor_name} actor should have yaml_text") +def step_actor_has_yaml_text(context: Context, actor_name: str) -> None: + """Verify the actor has yaml_text populated.""" + normalized_name = actor_name.replace("/", "/") + actor = context.actor_service.actors.get(normalized_name) + assert actor is not None, f"Actor {normalized_name} not found" + assert actor.yaml_text is not None, f"Actor {normalized_name} has no yaml_text" + context.current_actor = actor + + +@then('the yaml_text should contain "{expected_text}"') +def step_yaml_text_contains(context: Context, expected_text: str) -> None: + """Verify the yaml_text contains expected substring.""" + actor = getattr(context, "current_actor", None) + assert actor is not None, "No current actor set" + assert actor.yaml_text is not None, "Actor has no yaml_text" + assert expected_text in actor.yaml_text, ( + f"Expected '{expected_text}' in yaml_text, got: {actor.yaml_text}" + ) + + +@then('the {actor_name} actor yaml_text should contain "{expected_text}"') +def step_specific_actor_yaml_contains( + context: Context, actor_name: str, expected_text: str +) -> None: + """Verify a specific actor's yaml_text contains expected substring.""" + normalized_name = actor_name.replace("/", "/") + actor = context.actor_service.actors.get(normalized_name) + assert actor is not None, f"Actor {normalized_name} not found" + assert actor.yaml_text is not None, f"Actor {normalized_name} has no yaml_text" + assert expected_text in actor.yaml_text, ( + f"Expected '{expected_text}' in yaml_text, got: {actor.yaml_text}" + ) + context.current_actor = actor + + +@then("the command should not fail with v3 format error") +def step_no_v3_format_error(context: Context) -> None: + """Verify the actor would not fail due to missing v3 YAML format.""" + actor = getattr(context, "current_actor", None) + assert actor is not None, "No current actor set" + assert actor.yaml_text is not None, ( + "Actor has no yaml_text - would fail with v3 format error" + ) + config_blob = yaml.safe_load(actor.yaml_text) + assert is_v3_yaml(config_blob), ( + f"Actor yaml_text is not v3 format - would fail with v3 format error: {config_blob}" + ) + + +@then("the config parser should recognize the actor as v3 format") +def step_parser_recognizes_v3(context: Context) -> None: + """Verify the config parser recognizes the actor as v3 format.""" + actor = getattr(context, "current_actor", None) + assert actor is not None, "No current actor set" + + # Parse the yaml_text and check if it's recognized as v3 + if actor.yaml_text: + config_blob = yaml.safe_load(actor.yaml_text) + assert is_v3_yaml(config_blob), f"Config not recognized as v3: {config_blob}" + + +@then("the generated yaml_text should be valid v3 ActorConfigSchema") +def step_yaml_valid_v3_schema(context: Context) -> None: + """Verify the generated YAML is valid v3 ActorConfigSchema.""" + actor = getattr(context, "current_actor", None) + assert actor is not None, "No current actor set" + assert actor.yaml_text is not None, "Actor has no yaml_text" + + # Parse and validate against schema + config_blob = yaml.safe_load(actor.yaml_text) + try: + ActorConfigSchema.model_validate(config_blob) + except Exception as exc: + raise AssertionError(f"YAML is not valid v3 ActorConfigSchema: {exc}") from exc + + +@then("the yaml_text should parse without errors") +def step_yaml_parses_without_errors(context: Context) -> None: + """Verify the yaml_text can be parsed as valid YAML.""" + actor = getattr(context, "current_actor", None) + assert actor is not None, "No current actor set" + assert actor.yaml_text is not None, "Actor has no yaml_text" + + try: + yaml.safe_load(actor.yaml_text) + except yaml.YAMLError as exc: + raise AssertionError(f"YAML failed to parse: {exc}") from exc diff --git a/src/cleveragents/actor/registry.py b/src/cleveragents/actor/registry.py index 155780cf7..cfcf45b59 100644 --- a/src/cleveragents/actor/registry.py +++ b/src/cleveragents/actor/registry.py @@ -7,6 +7,7 @@ from dataclasses import asdict from typing import Any import pydantic +import yaml from cleveragents.actor.config import ActorConfiguration from cleveragents.actor.legacy_registry import add_legacy @@ -100,6 +101,43 @@ class ActorRegistry: "capabilities": asdict(capabilities) if capabilities else None, } + def _generate_builtin_actor_yaml( + self, + *, + provider: str, + model: str, + capabilities: ProviderCapabilities | None = None, + ) -> str: + """Generate v3 YAML text for a built-in actor. + + Creates a minimal v3 ActorConfigSchema YAML representation for built-in + actors, ensuring they include the required ``type`` and ``description`` + fields for proper v3 format recognition. + + Args: + provider: Provider name (e.g., "openai", "anthropic"). + model: Model identifier (e.g., "gpt-4", "claude-3-opus"). + capabilities: Optional provider capabilities. + + Returns: + YAML string in v3 format with type: llm. + """ + yaml_dict: dict[str, Any] = { + "name": self._actor_name(provider, model), + "type": "llm", + "model": f"{provider}/{model}", + "description": ( + f"Built-in actor from provider registry ({provider}/{model})" + ), + "provider": provider, + "capabilities": (asdict(capabilities) if capabilities else None), + "unsafe": False, + "source": "provider-registry", + } + # Remove None values to keep YAML clean + yaml_dict = {k: v for k, v in yaml_dict.items() if v is not None} + return yaml.safe_dump(yaml_dict, default_flow_style=False, sort_keys=False) + def _canonical_blob( self, config: ActorConfiguration, @@ -124,7 +162,12 @@ class ActorRegistry: return blob def ensure_built_in_actors(self) -> list[Actor]: - """Generate built-in actors from configured providers if missing.""" + """Generate built-in actors from configured providers if missing. + + Built-in actors are persisted with v3 YAML text including the ``type`` + and ``description`` fields, ensuring they work identically to custom + actors with the ``agents actor run`` command. + """ configured: list[ProviderInfo] = ( self._provider_registry.get_configured_providers() @@ -143,6 +186,12 @@ class ActorRegistry: source="provider-registry", capabilities=info.capabilities, ) + # Generate v3 YAML text for the built-in actor + yaml_text = self._generate_builtin_actor_yaml( + provider=provider_name, + model=model_id, + capabilities=info.capabilities, + ) # NOTE: ``name`` is sanitised via ``_actor_name()`` (slashes # replaced, lowercased) while ``provider`` and ``model`` # retain their raw values. This is intentional: ``name`` is @@ -164,6 +213,7 @@ class ActorRegistry: unsafe=False, set_default=False, is_built_in=True, + yaml_text=yaml_text, ) actors.append(actor) diff --git a/tests/actor/test_registry_builtin_yaml.py b/tests/actor/test_registry_builtin_yaml.py new file mode 100644 index 000000000..fcc1df1f9 --- /dev/null +++ b/tests/actor/test_registry_builtin_yaml.py @@ -0,0 +1,414 @@ +"""Unit tests for built-in actor v3 YAML generation.""" + +from __future__ import annotations + +import pytest +import yaml + +from cleveragents.actor.registry import ActorRegistry +from cleveragents.actor.schema import ActorConfigSchema, ActorType, is_v3_yaml +from cleveragents.application.services.actor_service import ActorService +from cleveragents.config.settings import ProviderDefaults, Settings +from cleveragents.providers.registry import ( + ProviderCapabilities, + ProviderInfo, + ProviderRegistry, + ProviderType, +) + + +class _StubActorService: + """Stub actor service for testing.""" + + def __init__(self) -> None: + self.actors: dict[str, dict] = {} + self.default_actor_name: str | None = None + + def upsert_actor( + self, + *, + name: str, + provider: str, + model: str, + config_blob: dict, + graph_descriptor: dict | None, + unsafe: bool, + set_default: bool, + is_built_in: bool, + yaml_text: str | None = None, + schema_version: str | None = None, + compiled_metadata: dict | None = None, + ) -> None: + """Store actor data for verification.""" + self.actors[name] = { + "name": name, + "provider": provider, + "model": model, + "config_blob": config_blob, + "graph_descriptor": graph_descriptor, + "unsafe": unsafe, + "is_built_in": is_built_in, + "yaml_text": yaml_text, + "schema_version": schema_version, + } + if set_default: + self.default_actor_name = name + + def get_default_actor(self) -> str | None: + """Return default actor name.""" + return self.default_actor_name + + def set_default_actor(self, name: str) -> None: + """Set default actor.""" + self.default_actor_name = name + + +class _StubProviderRegistry: + """Stub provider registry for testing.""" + + def __init__(self, providers: list[ProviderInfo]) -> None: + self._providers = providers + + def get_configured_providers(self) -> list[ProviderInfo]: + """Return configured providers.""" + return list(self._providers) + + +class _StubSettings: + """Stub settings for testing.""" + + def __init__(self, defaults: ProviderDefaults) -> None: + self._defaults = defaults + + def resolve_provider_defaults(self) -> ProviderDefaults: + """Return provider defaults.""" + return self._defaults + + +@pytest.fixture +def stub_actor_service() -> _StubActorService: + """Create a stub actor service.""" + return _StubActorService() + + +@pytest.fixture +def stub_settings() -> _StubSettings: + """Create stub settings.""" + return _StubSettings( + ProviderDefaults( + provider=None, provider_source="test", model=None, model_source="test" + ) + ) + + +class TestGenerateBuiltinActorYaml: + """Tests for _generate_builtin_actor_yaml method.""" + + def test_generates_yaml_with_type_field( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """YAML should include type: llm field.""" + provider_registry = _StubProviderRegistry([]) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + yaml_text = registry._generate_builtin_actor_yaml( + provider="openai", + model="gpt-4", + ) + + assert yaml_text is not None + assert "type: llm" in yaml_text + + def test_generates_yaml_with_description_field( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """YAML should include description field.""" + provider_registry = _StubProviderRegistry([]) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + yaml_text = registry._generate_builtin_actor_yaml( + provider="openai", + model="gpt-4", + ) + + assert "description:" in yaml_text + assert "Built-in actor from provider registry" in yaml_text + + def test_generates_yaml_with_name_field( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """YAML should include properly formatted name field.""" + provider_registry = _StubProviderRegistry([]) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + yaml_text = registry._generate_builtin_actor_yaml( + provider="openai", + model="gpt-4", + ) + + assert "name: openai/gpt-4" in yaml_text + + def test_generates_yaml_with_provider_field( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """YAML should include provider field.""" + provider_registry = _StubProviderRegistry([]) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + yaml_text = registry._generate_builtin_actor_yaml( + provider="anthropic", + model="claude-3-opus", + ) + + assert "provider: anthropic" in yaml_text + + def test_generates_yaml_with_model_field( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """YAML should include model field.""" + provider_registry = _StubProviderRegistry([]) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + yaml_text = registry._generate_builtin_actor_yaml( + provider="openai", + model="gpt-4", + ) + + assert "model: openai/gpt-4" in yaml_text + + def test_generates_yaml_with_source_field( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """YAML should include source: provider-registry field.""" + provider_registry = _StubProviderRegistry([]) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + yaml_text = registry._generate_builtin_actor_yaml( + provider="openai", + model="gpt-4", + ) + + assert "source: provider-registry" in yaml_text + + def test_generated_yaml_is_valid_v3_format( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """Generated YAML should be recognized as v3 format.""" + provider_registry = _StubProviderRegistry([]) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + yaml_text = registry._generate_builtin_actor_yaml( + provider="openai", + model="gpt-4", + ) + + config_blob = yaml.safe_load(yaml_text) + assert is_v3_yaml(config_blob), ( + "Generated YAML should be recognized as v3 format" + ) + + def test_generated_yaml_validates_against_schema( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """Generated YAML should validate against ActorConfigSchema.""" + provider_registry = _StubProviderRegistry([]) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + yaml_text = registry._generate_builtin_actor_yaml( + provider="openai", + model="gpt-4", + ) + + config_blob = yaml.safe_load(yaml_text) + schema = ActorConfigSchema.model_validate(config_blob) + assert schema.type == ActorType.LLM + assert schema.name == "openai/gpt-4" + assert schema.provider == "openai" + assert schema.model == "openai/gpt-4" + + def test_generated_yaml_includes_capabilities( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """Generated YAML should include capabilities when provided.""" + provider_registry = _StubProviderRegistry([]) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + capabilities = ProviderCapabilities(supports_tools=True, supports_vision=False) + yaml_text = registry._generate_builtin_actor_yaml( + provider="openai", + model="gpt-4", + capabilities=capabilities, + ) + + assert "capabilities:" in yaml_text + assert "supports_tools: true" in yaml_text + + def test_generated_yaml_sanitizes_slashes_in_name( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """Generated YAML name should sanitize slashes.""" + provider_registry = _StubProviderRegistry([]) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + yaml_text = registry._generate_builtin_actor_yaml( + provider="openrouter", + model="meta-llama/llama-3-70b-instruct", + ) + + # Name should have slashes replaced with dashes + assert "name: openrouter/meta-llama-llama-3-70b-instruct" in yaml_text + + +class TestEnsureBuiltInActorsWithYaml: + """Tests for ensure_built_in_actors with yaml_text generation.""" + + def test_ensure_built_in_actors_includes_yaml_text( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """Built-in actors should have yaml_text populated.""" + provider_registry = _StubProviderRegistry( + [ + ProviderInfo( + provider_type=ProviderType.OPENAI, + name="openai", + api_key_env_var="ENV", + default_model="gpt-4", + capabilities=ProviderCapabilities(supports_tools=True), + is_configured=True, + ) + ] + ) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + registry.ensure_built_in_actors() + + assert "openai/gpt-4" in stub_actor_service.actors + actor_data = stub_actor_service.actors["openai/gpt-4"] + assert actor_data["yaml_text"] is not None + assert "type: llm" in actor_data["yaml_text"] + + def test_ensure_built_in_actors_yaml_is_v3_format( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """Built-in actor yaml_text should be v3 format.""" + provider_registry = _StubProviderRegistry( + [ + ProviderInfo( + provider_type=ProviderType.ANTHROPIC, + name="anthropic", + api_key_env_var="ENV", + default_model="claude-3-opus", + capabilities=ProviderCapabilities(), + is_configured=True, + ) + ] + ) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + registry.ensure_built_in_actors() + + actor_data = stub_actor_service.actors["anthropic/claude-3-opus"] + assert actor_data["yaml_text"] is not None + config_blob = yaml.safe_load(actor_data["yaml_text"]) + assert is_v3_yaml(config_blob) + + def test_ensure_built_in_actors_multiple_providers( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """Multiple providers should each get yaml_text.""" + provider_registry = _StubProviderRegistry( + [ + ProviderInfo( + provider_type=ProviderType.OPENAI, + name="openai", + api_key_env_var="ENV", + default_model="gpt-4", + capabilities=ProviderCapabilities(supports_tools=True), + is_configured=True, + ), + ProviderInfo( + provider_type=ProviderType.ANTHROPIC, + name="anthropic", + api_key_env_var="ENV", + default_model="claude-3-opus", + capabilities=ProviderCapabilities(), + is_configured=True, + ), + ] + ) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + registry.ensure_built_in_actors() + + assert len(stub_actor_service.actors) == 2 + for actor_data in stub_actor_service.actors.values(): + assert actor_data["yaml_text"] is not None + assert "type: llm" in actor_data["yaml_text"] + + def test_ensure_built_in_actors_empty_when_no_providers( + self, stub_actor_service: _StubActorService, stub_settings: _StubSettings + ) -> None: + """No providers should result in no actors.""" + provider_registry = _StubProviderRegistry([]) + registry = ActorRegistry( + actor_service=stub_actor_service, + provider_registry=provider_registry, + settings=stub_settings, + ) + + actors = registry.ensure_built_in_actors() + + assert actors == [] + assert len(stub_actor_service.actors) == 0