4e37da2471
Merge the two separate database transactions in ensure_default_mock_actor() into a single atomic transaction to prevent Time-of-Check-Time-of-Use (TOCTOU) race conditions in concurrent environments (e.g., parallel test workers). The original implementation had a gap between checking for an existing default actor and creating a new mock actor. In concurrent scenarios, multiple threads could both observe no default and attempt to create one, leading to potential constraint violations or inconsistent state. The fix consolidates both operations into a single transaction, ensuring that the check and creation are atomic and safe for concurrent callers. Added BDD test scenarios to verify idempotency and concurrent safety: - Idempotency: calling ensure_default_mock_actor multiple times produces same result - Existing default safety: respects existing default actors - Single actor creation: ensures only one mock actor is created Closes #8448
326 lines
11 KiB
Python
326 lines
11 KiB
Python
"""Behave steps targeting actor service coverage gaps."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import ast
|
|
import os
|
|
from collections.abc import Callable
|
|
from datetime import datetime
|
|
|
|
from behave import given, then, when
|
|
from behave.runner import Context
|
|
|
|
from cleveragents.application.services.actor_service import ActorService
|
|
from cleveragents.config.settings import Settings
|
|
from cleveragents.core.exceptions import (
|
|
BusinessRuleViolation,
|
|
NotFoundError,
|
|
ValidationError,
|
|
)
|
|
from cleveragents.domain.models.core.actor import Actor
|
|
|
|
|
|
def _add_cleanup(context: Context, handler: Callable[[], None]) -> None:
|
|
if not hasattr(context, "_cleanup_handlers"):
|
|
context._cleanup_handlers = []
|
|
context._cleanup_handlers.append(handler)
|
|
|
|
|
|
class _StubActorRepository:
|
|
def __init__(self) -> None:
|
|
self._actors: dict[str, Actor] = {}
|
|
self._default_name: str | None = None
|
|
|
|
def list_all(self) -> list[Actor]:
|
|
return [self._actors[name] for name in sorted(self._actors.keys())]
|
|
|
|
def get_by_name(self, name: str) -> Actor | None:
|
|
return self._actors.get(name)
|
|
|
|
def upsert(self, actor: Actor) -> Actor:
|
|
self._actors[actor.name] = actor
|
|
if actor.is_default:
|
|
self.set_default(actor.name)
|
|
return actor
|
|
|
|
def delete(self, name: str) -> None:
|
|
actor = self._actors.get(name)
|
|
if actor is None:
|
|
raise ValueError("Actor does not exist")
|
|
if actor.is_built_in or actor.is_default:
|
|
raise ValueError("Cannot delete built-in or default actor")
|
|
del self._actors[name]
|
|
|
|
def set_default(self, name: str) -> Actor:
|
|
if name not in self._actors:
|
|
raise ValueError("Actor does not exist")
|
|
self._default_name = name
|
|
for actor in self._actors.values():
|
|
actor.is_default = actor.name == name
|
|
return self._actors[name]
|
|
|
|
def get_default(self) -> Actor | None:
|
|
if self._default_name and self._default_name in self._actors:
|
|
return self._actors[self._default_name]
|
|
for actor in self._actors.values():
|
|
if actor.is_default:
|
|
self._default_name = actor.name
|
|
return actor
|
|
return None
|
|
|
|
def clear(self) -> None:
|
|
self._actors.clear()
|
|
self._default_name = None
|
|
|
|
def count(self) -> int:
|
|
return len(self._actors)
|
|
|
|
|
|
class _StubTransaction:
|
|
def __init__(self, repo: _StubActorRepository) -> None:
|
|
self.actors = repo
|
|
|
|
def __enter__(self) -> _StubTransaction:
|
|
return self
|
|
|
|
def __exit__(self, exc_type, exc, tb) -> bool:
|
|
return False
|
|
|
|
|
|
class _StubUnitOfWork:
|
|
def __init__(self, repo: _StubActorRepository) -> None:
|
|
self._repo = repo
|
|
|
|
def transaction(self) -> _StubTransaction:
|
|
return _StubTransaction(self._repo)
|
|
|
|
|
|
def _make_actor(
|
|
name: str, *, is_default: bool = False, is_built_in: bool = False
|
|
) -> Actor:
|
|
now = datetime.now()
|
|
return Actor(
|
|
id=None,
|
|
name=name,
|
|
provider="provider",
|
|
model="model",
|
|
config_blob={},
|
|
config_hash=Actor.compute_hash({}),
|
|
graph_descriptor=None,
|
|
unsafe=False,
|
|
is_built_in=is_built_in,
|
|
is_default=is_default,
|
|
created_at=now,
|
|
updated_at=now,
|
|
)
|
|
|
|
|
|
@given("an actor service with stubbed dependencies")
|
|
def step_actor_service_stubbed(context: Context) -> None:
|
|
repo = _StubActorRepository()
|
|
context.actor_repo = repo
|
|
context.unit_of_work = _StubUnitOfWork(repo)
|
|
context.actor_service = ActorService(Settings(), context.unit_of_work)
|
|
|
|
original_env = os.environ.get("CLEVERAGENTS_TESTING_USE_MOCK_AI")
|
|
|
|
def cleanup_env() -> None:
|
|
if original_env is None:
|
|
os.environ.pop("CLEVERAGENTS_TESTING_USE_MOCK_AI", None)
|
|
else:
|
|
os.environ["CLEVERAGENTS_TESTING_USE_MOCK_AI"] = original_env
|
|
|
|
_add_cleanup(context, cleanup_env)
|
|
|
|
|
|
@when('I normalize actor name "{name}"')
|
|
def step_normalize_name(context: Context, name: str) -> None:
|
|
context.normalized_name = context.actor_service._normalize_name(name)
|
|
|
|
|
|
@then('the normalized actor name should be "{expected}"')
|
|
def step_normalized_actor_name_should_be(context: Context, expected: str) -> None:
|
|
assert context.normalized_name == expected, context.normalized_name
|
|
|
|
|
|
@when('I try to normalize actor name "{name}" without allowing built-ins')
|
|
def step_try_normalize_invalid(context: Context, name: str) -> None:
|
|
try:
|
|
context.actor_service._normalize_name(name, allow_built_in=False)
|
|
context.error = None
|
|
except Exception as exc:
|
|
context.error = exc
|
|
|
|
|
|
@when('I try to normalize actor name "" without allowing built-ins')
|
|
def step_try_normalize_invalid_empty(context: Context) -> None:
|
|
step_try_normalize_invalid(context, "")
|
|
|
|
|
|
@then('a ValidationError should be raised containing "{message}"')
|
|
def step_validation_error_message(context: Context, message: str) -> None:
|
|
assert isinstance(context.error, ValidationError), type(context.error)
|
|
assert message in str(context.error)
|
|
|
|
|
|
@given('existing actors named "{first}" and "{second}"')
|
|
def step_existing_actors(context: Context, first: str, second: str) -> None:
|
|
for name in (first, second):
|
|
context.actor_repo.upsert(_make_actor(name))
|
|
|
|
|
|
@when("I list all actors")
|
|
def step_list_actors(context: Context) -> None:
|
|
context.listed_actors = context.actor_service.list_actors()
|
|
|
|
|
|
@then("I should receive actors {actor_names}")
|
|
def step_should_receive_actors(context: Context, actor_names: str) -> None:
|
|
expected = ast.literal_eval(actor_names)
|
|
assert [actor.name for actor in context.listed_actors] == expected
|
|
|
|
|
|
@when('I attempt to fetch actor "{name}"')
|
|
def step_attempt_fetch_actor(context: Context, name: str) -> None:
|
|
try:
|
|
context.fetched_actor = context.actor_service.get_actor(name)
|
|
context.error = None
|
|
except Exception as exc:
|
|
context.error = exc
|
|
|
|
|
|
@then("a NotFoundError should be raised for the actor")
|
|
def step_not_found_error(context: Context) -> None:
|
|
assert isinstance(context.error, NotFoundError), type(context.error)
|
|
|
|
|
|
@given('a built-in actor named "{name}" already exists')
|
|
def step_builtin_actor_exists(context: Context, name: str) -> None:
|
|
context.actor_repo.upsert(_make_actor(name, is_built_in=True))
|
|
|
|
|
|
@when('I attempt to upsert "{name}" as custom')
|
|
def step_attempt_upsert_custom(context: Context, name: str) -> None:
|
|
try:
|
|
context.actor_service.upsert_actor(name=name, provider="p", model="m")
|
|
context.error = None
|
|
except Exception as exc:
|
|
context.error = exc
|
|
|
|
|
|
@then("a BusinessRuleViolation should be raised")
|
|
def step_business_rule_violation(context: Context) -> None:
|
|
assert isinstance(context.error, BusinessRuleViolation), type(context.error)
|
|
|
|
|
|
@given('an actor named "{name}" exists')
|
|
def step_actor_exists(context: Context, name: str) -> None:
|
|
context.actor_repo.upsert(_make_actor(name))
|
|
|
|
|
|
@when('I upsert "{name}" with set_default')
|
|
def step_upsert_with_set_default(context: Context, name: str) -> None:
|
|
context.saved_actor = context.actor_service.upsert_actor(
|
|
name=name, provider="p", model="m", set_default=True
|
|
)
|
|
|
|
|
|
@then('the stored actor "{name}" should be default')
|
|
def step_stored_actor_default(context: Context, name: str) -> None:
|
|
actor = context.actor_repo.get_by_name(name)
|
|
assert actor is not None, "Actor not found"
|
|
assert actor.is_default, "Actor was not marked as default"
|
|
assert context.actor_repo.get_default() is actor
|
|
|
|
|
|
@when('I try to remove actor "{name}"')
|
|
def step_try_remove_actor(context: Context, name: str) -> None:
|
|
try:
|
|
context.actor_service.remove_actor(name)
|
|
context.error = None
|
|
except Exception as exc:
|
|
context.error = exc
|
|
|
|
|
|
@when('I set default actor "{name}"')
|
|
def step_set_default_actor(context: Context, name: str) -> None:
|
|
try:
|
|
context.result_actor = context.actor_service.set_default_actor(name)
|
|
context.error = None
|
|
except Exception as exc:
|
|
context.error = exc
|
|
|
|
|
|
@then('the returned actor name should be "{name}"')
|
|
def step_returned_actor_name(context: Context, name: str) -> None:
|
|
assert context.result_actor is not None, "No actor returned"
|
|
assert context.result_actor.name == name
|
|
|
|
|
|
@then("no additional actors should be created")
|
|
def step_no_additional_actors(context: Context) -> None:
|
|
assert context.actor_repo.count() == context.initial_actor_count
|
|
|
|
|
|
@given('an actor named "{name}" exists as default')
|
|
def step_actor_exists_as_default(context: Context, name: str) -> None:
|
|
actor = _make_actor(name, is_default=True)
|
|
context.actor_repo.upsert(actor)
|
|
context.actor_repo.set_default(name)
|
|
|
|
|
|
@given('a mock actor named "{name}" exists without default flag')
|
|
def step_mock_actor_exists_without_default(context: Context, name: str) -> None:
|
|
actor = _make_actor(name, is_built_in=True, is_default=False)
|
|
context.actor_repo.upsert(actor)
|
|
|
|
|
|
@given("any existing actors are cleared")
|
|
def step_clear_existing_actors(context: Context) -> None:
|
|
context.actor_repo.clear()
|
|
|
|
|
|
@when('I ensure the default mock actor with env value "{value}"')
|
|
def step_ensure_default_mock_actor(context: Context, value: str) -> None:
|
|
context.initial_actor_count = context.actor_repo.count()
|
|
os.environ["CLEVERAGENTS_TESTING_USE_MOCK_AI"] = value
|
|
context.result_actor = context.actor_service.ensure_default_mock_actor()
|
|
|
|
|
|
@then("the result should be None")
|
|
def step_result_none(context: Context) -> None:
|
|
assert context.result_actor is None
|
|
|
|
|
|
def after_scenario(context: Context, scenario) -> None:
|
|
if hasattr(context, "_cleanup_handlers"):
|
|
for handler in context._cleanup_handlers:
|
|
handler()
|
|
|
|
|
|
@when('I ensure the default mock actor with env value "{value}" again')
|
|
def step_ensure_default_mock_actor_again(context: Context, value: str) -> None:
|
|
os.environ["CLEVERAGENTS_TESTING_USE_MOCK_AI"] = value
|
|
context.result_actor = context.actor_service.ensure_default_mock_actor()
|
|
|
|
|
|
@then('only one "{name}" actor should exist')
|
|
def step_only_one_actor_exists(context: Context, name: str) -> None:
|
|
actors = context.actor_repo.list_all()
|
|
count = sum(1 for actor in actors if actor.name == name)
|
|
assert count == 1, f"Expected 1 actor named {name}, but found {count}"
|
|
|
|
|
|
@then("only one default actor should exist")
|
|
def step_only_one_default_actor(context: Context) -> None:
|
|
actors = context.actor_repo.list_all()
|
|
default_count = sum(1 for actor in actors if actor.is_default)
|
|
assert default_count == 1, f"Expected 1 default actor, but found {default_count}"
|
|
|
|
|
|
@then("exactly one actor should exist")
|
|
def step_exactly_one_actor(context: Context) -> None:
|
|
actors = context.actor_repo.list_all()
|
|
count = len(actors)
|
|
assert count == 1, f"Expected 1 actor, but found {count}"
|