From 2acb957d83ed468ff286f9b855c8565aeedf700e Mon Sep 17 00:00:00 2001 From: Luis Mendes Date: Fri, 13 Mar 2026 23:03:14 +0000 Subject: [PATCH] fix(cli): replace in-memory automation-profile repository with database-backed persistence The automation-profile CLI commands used an _InMemoryProfileRepository (a Python dict) that lost all data between CLI process invocations. Profiles created with "automation-profile add" were invisible to subsequent "automation-profile show" or "list" calls because each CLI command is a separate process with a fresh empty dict. Changes: - Replaced _InMemoryProfileRepository with the real AutomationProfileRepository from the infrastructure layer, wired via the DI container following the same pattern as tool.py and session.py. - Added auto_commit support to AutomationProfileRepository (matching the existing SessionRepository pattern) so that CLI commands running outside a UnitOfWork commit each operation automatically. - Added safety_json and guards_json Text columns to the automation_profiles table (Alembic migration m6_005) for full-fidelity round-trip of the AutomationGuard and SafetyProfile sub-models. Previously, guards and several safety fields (max_cost_per_plan, max_retries_per_step, etc.) were silently dropped on persistence. - Updated _from_domain, _to_domain, and _update_row to serialize and deserialize the full guard and safety sub-models via JSON, with backward-compatible fallback to legacy scalar columns. Refs: #746 --- .../versions/m6_005_profile_guards_json.py | 40 ++++++ benchmarks/automation_profile_cli_bench.py | 53 ++++++-- ...mation_profile_cli_coverage_boost_steps.py | 50 ++++++-- .../automation_profile_cli_coverage_steps.py | 49 ++++++-- .../steps/automation_profile_cli_steps.py | 46 +++++-- robot/helper_automation_profile_cli.py | 117 ++++++++++++++---- .../cli/commands/automation_profile.py | 50 +++----- .../infrastructure/database/models.py | 6 +- .../infrastructure/database/repositories.py | 105 +++++++++++++--- 9 files changed, 393 insertions(+), 123 deletions(-) create mode 100644 alembic/versions/m6_005_profile_guards_json.py diff --git a/alembic/versions/m6_005_profile_guards_json.py b/alembic/versions/m6_005_profile_guards_json.py new file mode 100644 index 00000000..c8ec147a --- /dev/null +++ b/alembic/versions/m6_005_profile_guards_json.py @@ -0,0 +1,40 @@ +"""Add safety_json and guards_json columns to automation_profiles table. + +Persists the full SafetyProfile and AutomationGuard sub-models as +serialised JSON, complementing the existing scalar safety columns. +Nullable so existing rows are unaffected. + +Revision ID: m6_005_profile_guards_json +Revises: m6_004_container_metadata_column +Create Date: 2026-03-13 00:00:00 + +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "m6_005_profile_guards_json" +down_revision: str | Sequence[str] | None = "m6_004_container_metadata_column" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + """Add safety_json and guards_json columns to automation_profiles.""" + with op.batch_alter_table("automation_profiles") as batch_op: + batch_op.add_column( + sa.Column("safety_json", sa.Text(), nullable=True), + ) + batch_op.add_column( + sa.Column("guards_json", sa.Text(), nullable=True), + ) + + +def downgrade() -> None: + """Remove safety_json and guards_json columns.""" + with op.batch_alter_table("automation_profiles") as batch_op: + batch_op.drop_column("guards_json") + batch_op.drop_column("safety_json") diff --git a/benchmarks/automation_profile_cli_bench.py b/benchmarks/automation_profile_cli_bench.py index 6f3a96eb..45b9a07c 100644 --- a/benchmarks/automation_profile_cli_bench.py +++ b/benchmarks/automation_profile_cli_bench.py @@ -27,12 +27,39 @@ importlib.reload(cleveragents) from typer.testing import CliRunner # noqa: E402 -from cleveragents.cli.commands.automation_profile import ( # noqa: E402 - _InMemoryProfileRepository, +from cleveragents.application.services.automation_profile_service import ( # noqa: E402 + AutomationProfileService, ) from cleveragents.cli.commands.automation_profile import ( # noqa: E402 app as profile_app, ) +from cleveragents.domain.models.core.automation_profile import ( # noqa: E402 + AutomationProfile, +) + + +class _InMemoryProfileRepository: + """In-memory repository satisfying the AutomationProfileRepository duck-type contract. + + Provides the four methods expected by ``AutomationProfileService``: + ``get_by_name``, ``list_all``, ``upsert``, and ``delete``. + """ + + def __init__(self) -> None: + self._store: dict[str, AutomationProfile] = {} + + def get_by_name(self, name: str) -> AutomationProfile | None: + return self._store.get(name) + + def list_all(self) -> list[AutomationProfile]: + return list(self._store.values()) + + def upsert(self, profile: AutomationProfile) -> None: + self._store[profile.name] = profile + + def delete(self, name: str) -> None: + self._store.pop(name, None) + _VALID_YAML = """\ name: bench/test-profile @@ -58,19 +85,25 @@ safety: _runner = CliRunner() -def _reset_repo() -> None: - """Reset the module-level in-memory repo.""" +def _patch_service() -> None: + """Patch ``_get_service`` in the CLI module to use an in-memory repo. + + Each call creates a fresh repo + service so benchmark iterations + are isolated from each other and safe for parallel execution. + """ import cleveragents.cli.commands.automation_profile as ap_mod - ap_mod._repo = _InMemoryProfileRepository() + repo = _InMemoryProfileRepository() + service = AutomationProfileService(repo=repo) # type: ignore[arg-type] + ap_mod._get_service = lambda: service class AutomationProfileCLIAddSuite: """Benchmark automation-profile add --config throughput.""" def setup(self) -> None: - """Write a temporary YAML file.""" - _reset_repo() + """Write a temporary YAML file and patch service.""" + _patch_service() fd, self._path = tempfile.mkstemp(suffix=".yaml") with os.fdopen(fd, "w") as fh: fh.write(_VALID_YAML) @@ -81,7 +114,7 @@ class AutomationProfileCLIAddSuite: def time_add_from_config(self) -> None: """Benchmark add --config end-to-end.""" - _reset_repo() + _patch_service() _runner.invoke(profile_app, ["add", "--config", self._path]) @@ -90,7 +123,7 @@ class AutomationProfileCLIListSuite: def setup(self) -> None: """Set up with built-in profiles.""" - _reset_repo() + _patch_service() def time_list_all(self) -> None: """Benchmark listing all profiles.""" @@ -110,7 +143,7 @@ class AutomationProfileCLIShowSuite: def setup(self) -> None: """Set up.""" - _reset_repo() + _patch_service() def time_show_rich(self) -> None: """Benchmark showing a profile in rich format.""" diff --git a/features/steps/automation_profile_cli_coverage_boost_steps.py b/features/steps/automation_profile_cli_coverage_boost_steps.py index 7d5b7e97..83ad97aa 100644 --- a/features/steps/automation_profile_cli_coverage_boost_steps.py +++ b/features/steps/automation_profile_cli_coverage_boost_steps.py @@ -15,7 +15,6 @@ from typer.testing import CliRunner from cleveragents.cli.commands.automation_profile import ( _guards_dict, - _InMemoryProfileRepository, _profile_spec_dict, ) from cleveragents.cli.commands.automation_profile import ( @@ -24,6 +23,27 @@ from cleveragents.cli.commands.automation_profile import ( from cleveragents.domain.models.core.automation_guard import AutomationGuard from cleveragents.domain.models.core.automation_profile import AutomationProfile + +def _create_in_memory_profile_service(): + """Create an AutomationProfileService backed by an in-memory SQLite DB.""" + from sqlalchemy import create_engine + from sqlalchemy.orm import sessionmaker + + from cleveragents.application.services.automation_profile_service import ( + AutomationProfileService, + ) + from cleveragents.infrastructure.database.models import Base + from cleveragents.infrastructure.database.repositories import ( + AutomationProfileRepository, + ) + + engine = create_engine("sqlite:///:memory:", echo=False) + Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine, expire_on_commit=False) + repo = AutomationProfileRepository(session_factory=factory, auto_commit=True) + return AutomationProfileService(repo=repo) + + # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- @@ -77,16 +97,24 @@ def _write_temp_yaml(context: Context, content: str) -> str: @given("a fresh automation profile CLI runner for guards coverage") def step_fresh_guards_runner(context: Context) -> None: + from unittest.mock import patch + context.runner = CliRunner() context.result = None - context.guards_dict_result: dict[str, object] | None = None - context.profile_spec_result: dict[str, object] | None = None - context.guard_obj: AutomationGuard | None = None - context.guarded_profile: AutomationProfile | None = None - # Reset the module-level in-memory repo - import cleveragents.cli.commands.automation_profile as ap_mod - - ap_mod._repo = _InMemoryProfileRepository() + context.guards_dict_result = None + context.profile_spec_result = None + context.guard_obj = None + context.guarded_profile = None + # Create an in-memory service and patch _get_service for this scenario + context._ap_service = _create_in_memory_profile_service() + context._ap_patcher = patch( + "cleveragents.cli.commands.automation_profile._get_service", + return_value=context._ap_service, + ) + context._ap_patcher.start() + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(context._ap_patcher.stop) # --------------------------------------------------------------------------- @@ -233,8 +261,6 @@ def step_spec_dict_guards_key_true(context: Context, key: str) -> None: @given('a custom guarded profile "{name}" is stored in the repo') def step_store_guarded_profile(context: Context, name: str) -> None: - import cleveragents.cli.commands.automation_profile as ap_mod - profile = _make_guarded_profile( name=name, max_tool_calls=5, @@ -244,7 +270,7 @@ def step_store_guarded_profile(context: Context, name: str) -> None: require_approval_for_writes=True, require_approval_for_apply=True, ) - ap_mod._repo.upsert(profile) + context._ap_service._repo.upsert(profile) @when('I run automation-profile show "{name}" in rich format') diff --git a/features/steps/automation_profile_cli_coverage_steps.py b/features/steps/automation_profile_cli_coverage_steps.py index ca0c54b3..5f9bf2e2 100644 --- a/features/steps/automation_profile_cli_coverage_steps.py +++ b/features/steps/automation_profile_cli_coverage_steps.py @@ -12,7 +12,6 @@ from behave.runner import Context from typer.testing import CliRunner from cleveragents.cli.commands.automation_profile import ( - _InMemoryProfileRepository, _threshold_summary, emit_automation_level_deprecation_warning, ) @@ -21,13 +20,35 @@ from cleveragents.cli.commands.automation_profile import ( ) from cleveragents.core.exceptions import ( CleverAgentsError, - NotFoundError, ValidationError, ) from cleveragents.domain.models.core.automation_profile import ( BUILTIN_PROFILES, AutomationProfile, ) +from cleveragents.infrastructure.database.repositories import ( + AutomationProfileNotFoundError, +) + + +def _create_in_memory_profile_service(): + """Create an AutomationProfileService backed by an in-memory SQLite DB.""" + from sqlalchemy import create_engine + from sqlalchemy.orm import sessionmaker + + from cleveragents.application.services.automation_profile_service import ( + AutomationProfileService, + ) + from cleveragents.infrastructure.database.models import Base + from cleveragents.infrastructure.database.repositories import ( + AutomationProfileRepository, + ) + + engine = create_engine("sqlite:///:memory:", echo=False) + Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine, expire_on_commit=False) + repo = AutomationProfileRepository(session_factory=factory, auto_commit=True) + return AutomationProfileService(repo=repo) def _make_profile( @@ -58,25 +79,31 @@ def _write_temp(context: Context, content: str) -> str: def step_coverage_runner(context: Context) -> None: context.runner = CliRunner() context.result = None - import cleveragents.cli.commands.automation_profile as ap_mod - - ap_mod._repo = _InMemoryProfileRepository() + # Create an in-memory service and patch _get_service for this scenario + context._ap_service = _create_in_memory_profile_service() + context._ap_patcher = patch( + "cleveragents.cli.commands.automation_profile._get_service", + return_value=context._ap_service, + ) + context._ap_patcher.start() + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(context._ap_patcher.stop) @when("I delete a non-existent profile from the in-memory repo") def step_delete_nonexistent_from_repo(context: Context) -> None: - repo = _InMemoryProfileRepository() context.repo_error = None try: - repo.delete("nonexistent/profile") - except NotFoundError as exc: + context._ap_service._repo.delete("nonexistent/profile") + except AutomationProfileNotFoundError as exc: context.repo_error = exc @then("a NotFoundError should be raised from the repo") def step_assert_not_found_error(context: Context) -> None: assert context.repo_error is not None, "Expected NotFoundError but none was raised" - assert isinstance(context.repo_error, NotFoundError) + assert isinstance(context.repo_error, AutomationProfileNotFoundError) @when("I call threshold summary on a built-in profile") @@ -180,10 +207,8 @@ def step_run_add_ca_error(context: Context) -> None: @given('a custom coverage profile "{name}" has been added') def step_add_custom_coverage_profile(context: Context, name: str) -> None: - import cleveragents.cli.commands.automation_profile as ap_mod - profile = _make_profile(name=name) - ap_mod._repo.upsert(profile) + context._ap_service._repo.upsert(profile) @when('I run automation-profile remove "{name}" without --yes and decline') diff --git a/features/steps/automation_profile_cli_steps.py b/features/steps/automation_profile_cli_steps.py index a70a8431..d9c38959 100644 --- a/features/steps/automation_profile_cli_steps.py +++ b/features/steps/automation_profile_cli_steps.py @@ -11,15 +11,33 @@ from behave import given, then, when from behave.runner import Context from typer.testing import CliRunner -from cleveragents.cli.commands.automation_profile import ( - _InMemoryProfileRepository, -) from cleveragents.cli.commands.automation_profile import ( app as profile_app, ) from cleveragents.domain.models.core.automation_profile import AutomationProfile from cleveragents.domain.models.core.safety_profile import SafetyProfile + +def _create_in_memory_profile_service(): + """Create an AutomationProfileService backed by an in-memory SQLite DB.""" + from sqlalchemy import create_engine + from sqlalchemy.orm import sessionmaker + + from cleveragents.application.services.automation_profile_service import ( + AutomationProfileService, + ) + from cleveragents.infrastructure.database.models import Base + from cleveragents.infrastructure.database.repositories import ( + AutomationProfileRepository, + ) + + engine = create_engine("sqlite:///:memory:", echo=False) + Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine, expire_on_commit=False) + repo = AutomationProfileRepository(session_factory=factory, auto_commit=True) + return AutomationProfileService(repo=repo) + + # Regex to strip ANSI escape sequences that Rich may emit. _ANSI_RE = re.compile(r"\x1b\[[0-9;]*[a-zA-Z]") @@ -119,10 +137,16 @@ def step_automation_profile_cli_runner(context: Context) -> None: """Set up the CLI runner for testing.""" context.runner = CliRunner() context.result = None - # Reset the module-level in-memory repo for each scenario - import cleveragents.cli.commands.automation_profile as ap_mod - - ap_mod._repo = _InMemoryProfileRepository() + # Create an in-memory service and patch _get_service for this scenario + context._ap_service = _create_in_memory_profile_service() + context._ap_patcher = patch( + "cleveragents.cli.commands.automation_profile._get_service", + return_value=context._ap_service, + ) + context._ap_patcher.start() + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(context._ap_patcher.stop) @given("a valid automation profile config YAML file") @@ -134,10 +158,8 @@ def step_valid_profile_config(context: Context) -> None: @given('the custom profile "{name}" already exists') def step_custom_profile_exists(context: Context, name: str) -> None: """Pre-register a custom profile in the in-memory repo.""" - import cleveragents.cli.commands.automation_profile as ap_mod - profile = _make_custom_profile(name=name) - ap_mod._repo.upsert(profile) + context._ap_service._repo.upsert(profile) @given("an invalid YAML automation profile config file") @@ -186,10 +208,8 @@ schema_version: "{version}" @given('a custom profile "{name}" has been added') def step_custom_profile_added(context: Context, name: str) -> None: """Add a custom profile via the in-memory repo.""" - import cleveragents.cli.commands.automation_profile as ap_mod - profile = _make_custom_profile(name=name) - ap_mod._repo.upsert(profile) + context._ap_service._repo.upsert(profile) # ---- When steps ---- # diff --git a/robot/helper_automation_profile_cli.py b/robot/helper_automation_profile_cli.py index 352cd0fb..897da846 100644 --- a/robot/helper_automation_profile_cli.py +++ b/robot/helper_automation_profile_cli.py @@ -2,27 +2,49 @@ """Robot Framework helper for automation-profile CLI smoke tests. Invoked by Robot tests to exercise the automation-profile CLI commands -in isolation using the Typer CliRunner with mocked services. +in isolation using the Typer CliRunner with a per-test in-memory SQLite +database, fully isolated for parallel execution. + +The module-level ``_get_service()`` in the production code calls +``get_container()`` which requires full DI wiring. We monkey-patch it +to return an ``AutomationProfileService`` backed by a fresh in-memory +SQLite ``AutomationProfileRepository`` so the tests can run without +container setup and without colliding across pabot workers. """ from __future__ import annotations import json +import logging import os import sys import tempfile from pathlib import Path +from unittest.mock import patch # Ensure the local source tree is importable _SRC = str(Path(__file__).resolve().parents[1] / "src") if _SRC not in sys.path: sys.path.insert(0, _SRC) +# Suppress debug-level structlog/retry noise that pollutes stdout in the +# CliRunner capture buffer. Must happen before any cleveragents imports +# that configure structlog. +logging.getLogger().setLevel(logging.WARNING) + +from sqlalchemy import create_engine # noqa: E402 +from sqlalchemy.orm import sessionmaker # noqa: E402 from typer.testing import CliRunner # noqa: E402 import cleveragents.cli.commands.automation_profile as _ap_mod # noqa: E402 +from cleveragents.application.services.automation_profile_service import ( # noqa: E402 + AutomationProfileService, +) +from cleveragents.infrastructure.database.models import Base # noqa: E402 +from cleveragents.infrastructure.database.repositories import ( # noqa: E402 + AutomationProfileRepository, +) -_InMemoryProfileRepository = _ap_mod._InMemoryProfileRepository profile_app = _ap_mod.app _runner = CliRunner() @@ -49,21 +71,62 @@ safety: """ -def _reset_repo() -> None: - """Reset the module-level in-memory repo.""" - import cleveragents.cli.commands.automation_profile as ap_mod +def _extract_json(text: str) -> object: + """Extract and parse the first valid JSON object or array from *text*. - ap_mod._repo = _InMemoryProfileRepository() + ``format_output`` writes JSON directly to ``sys.stdout`` which the + CliRunner captures. Structlog debug messages may precede the JSON + payload. This function scans for the first ``{`` or ``[`` and + parses from there. + """ + for i, ch in enumerate(text): + if ch in ("{", "["): + try: + return json.loads(text[i:]) + except json.JSONDecodeError: + continue + raise ValueError(f"No valid JSON found in output:\n{text[:500]}") + + +def _make_isolated_service() -> AutomationProfileService: + """Create a fully isolated AutomationProfileService with an in-memory DB.""" + engine = create_engine("sqlite:///:memory:", echo=False) + Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine, expire_on_commit=False) + repo = AutomationProfileRepository(session_factory=factory, auto_commit=True) + return AutomationProfileService(repo=repo) + + +# Module-level service instance that is reset before each test. +_isolated_service: AutomationProfileService | None = None + + +def _get_service_override() -> AutomationProfileService: + """Replacement for ``_ap_mod._get_service`` that returns the test service.""" + assert _isolated_service is not None, "Call _reset_service() first" + return _isolated_service + + +def _reset_service() -> None: + """Create a fresh isolated service for the next test.""" + global _isolated_service + _isolated_service = _make_isolated_service() + + +def _invoke(args: list[str]): + """Invoke the profile CLI with the patched service.""" + with patch.object(_ap_mod, "_get_service", _get_service_override): + return _runner.invoke(profile_app, args) def test_add_profile() -> None: """Test adding a profile via YAML config.""" - _reset_repo() + _reset_service() fd, path = tempfile.mkstemp(suffix=".yaml") try: with os.fdopen(fd, "w") as fh: fh.write(_VALID_YAML) - result = _runner.invoke(profile_app, ["add", "--config", path]) + result = _invoke(["add", "--config", path]) assert result.exit_code == 0, f"add failed: {result.output}" print("add-profile-ok") finally: @@ -72,8 +135,8 @@ def test_add_profile() -> None: def test_show_profile() -> None: """Test showing a built-in profile.""" - _reset_repo() - result = _runner.invoke(profile_app, ["show", "manual"]) + _reset_service() + result = _invoke(["show", "manual"]) assert result.exit_code == 0, f"show failed: {result.output}" assert "manual" in result.output print("show-profile-ok") @@ -81,19 +144,19 @@ def test_show_profile() -> None: def test_show_json() -> None: """Test showing a profile in JSON format.""" - _reset_repo() - result = _runner.invoke(profile_app, ["show", "manual", "--format", "json"]) + _reset_service() + result = _invoke(["show", "manual", "--format", "json"]) assert result.exit_code == 0, f"show json failed: {result.output}" - # Verify it's valid JSON - parsed = json.loads(result.output) + parsed = _extract_json(result.output) + assert isinstance(parsed, dict) assert parsed["name"] == "manual" print("show-json-ok") def test_show_yaml() -> None: """Test showing a profile in YAML format.""" - _reset_repo() - result = _runner.invoke(profile_app, ["show", "manual", "--format", "yaml"]) + _reset_service() + result = _invoke(["show", "manual", "--format", "yaml"]) assert result.exit_code == 0, f"show yaml failed: {result.output}" assert "name: manual" in result.output print("show-yaml-ok") @@ -101,8 +164,8 @@ def test_show_yaml() -> None: def test_list_profiles() -> None: """Test listing all profiles.""" - _reset_repo() - result = _runner.invoke(profile_app, ["list"]) + _reset_service() + result = _invoke(["list"]) assert result.exit_code == 0, f"list failed: {result.output}" assert "manual" in result.output print("list-profiles-ok") @@ -110,10 +173,10 @@ def test_list_profiles() -> None: def test_list_json() -> None: """Test listing profiles in JSON format.""" - _reset_repo() - result = _runner.invoke(profile_app, ["list", "--format", "json"]) + _reset_service() + result = _invoke(["list", "--format", "json"]) assert result.exit_code == 0, f"list json failed: {result.output}" - parsed = json.loads(result.output) + parsed = _extract_json(result.output) assert isinstance(parsed, list) assert len(parsed) >= 8 # At least 8 built-in profiles print("list-json-ok") @@ -121,19 +184,19 @@ def test_list_json() -> None: def test_remove_profile() -> None: """Test removing a custom profile.""" - _reset_repo() + _reset_service() # First add a profile fd, path = tempfile.mkstemp(suffix=".yaml") try: with os.fdopen(fd, "w") as fh: fh.write(_VALID_YAML) - add_result = _runner.invoke(profile_app, ["add", "--config", path]) + add_result = _invoke(["add", "--config", path]) assert add_result.exit_code == 0, f"add failed: {add_result.output}" finally: Path(path).unlink(missing_ok=True) - # Then remove it - result = _runner.invoke(profile_app, ["remove", "acme/robot-test", "--yes"]) + # Then remove it (reuse same service so the profile persists) + result = _invoke(["remove", "acme/robot-test", "--yes"]) assert result.exit_code == 0, f"remove failed: {result.output}" assert "removed" in result.output.lower() print("remove-profile-ok") @@ -141,8 +204,8 @@ def test_remove_profile() -> None: def test_remove_builtin_fails() -> None: """Test that removing a built-in profile fails.""" - _reset_repo() - result = _runner.invoke(profile_app, ["remove", "manual", "--yes"]) + _reset_service() + result = _invoke(["remove", "manual", "--yes"]) assert result.exit_code != 0, f"remove should have failed: {result.output}" print("remove-builtin-fails-ok") diff --git a/src/cleveragents/cli/commands/automation_profile.py b/src/cleveragents/cli/commands/automation_profile.py index a0c68e7d..5124f6f5 100644 --- a/src/cleveragents/cli/commands/automation_profile.py +++ b/src/cleveragents/cli/commands/automation_profile.py @@ -46,41 +46,23 @@ console = Console() _FORMAT_HELP = "Output format: json, yaml, plain, table, or rich (default: rich)" -class _InMemoryProfileRepository: - """In-memory repository for custom automation profiles.""" - - def __init__(self) -> None: - self._profiles: dict[str, AutomationProfile] = {} - - def get_by_name(self, name: str) -> AutomationProfile | None: - return self._profiles.get(name) - - def list_all(self) -> list[AutomationProfile]: - return [self._profiles[k] for k in sorted(self._profiles)] - - def upsert( - self, - profile: AutomationProfile, - expected_schema_version: str | None = None, - ) -> None: - self._profiles[profile.name] = profile - - def delete(self, name: str) -> None: - if name not in self._profiles: - raise NotFoundError( - f"Profile '{name}' not found", - resource_type="automation_profile", - resource_id=name, - ) - del self._profiles[name] - - -_repo = _InMemoryProfileRepository() - - def _get_service() -> AutomationProfileService: - """Get the AutomationProfileService with in-memory repository.""" - repo: Any = _repo + """Get the AutomationProfileService with database-backed repository.""" + from cleveragents.application.container import get_container + + container = get_container() + database_url: str = container.database_url() + + from sqlalchemy import create_engine + from sqlalchemy.orm import sessionmaker + + from cleveragents.infrastructure.database.repositories import ( + AutomationProfileRepository, + ) + + engine = create_engine(database_url, echo=False) + factory = sessionmaker(bind=engine, expire_on_commit=False) + repo = AutomationProfileRepository(session_factory=factory, auto_commit=True) return AutomationProfileService(repo=repo) diff --git a/src/cleveragents/infrastructure/database/models.py b/src/cleveragents/infrastructure/database/models.py index 3ba4a32a..fb1c72dd 100644 --- a/src/cleveragents/infrastructure/database/models.py +++ b/src/cleveragents/infrastructure/database/models.py @@ -2082,11 +2082,15 @@ class AutomationProfileModel(Base): # type: ignore[misc] auto_retry_transient = Column(Float, nullable=False, default=0.0) auto_checkpoint_restore = Column(Float, nullable=False, default=0.0) - # Safety requirements + # Safety requirements (legacy scalar columns kept for queries) require_sandbox = Column(Boolean, nullable=False, default=True) require_checkpoints = Column(Boolean, nullable=False, default=True) allow_unsafe_tools = Column(Boolean, nullable=False, default=False) + # Full-fidelity JSON for SafetyProfile and AutomationGuard sub-models + safety_json = Column(Text, nullable=True) + guards_json = Column(Text, nullable=True) + # Timestamps (ISO-8601 strings) created_at = Column(Text, nullable=False) updated_at = Column(Text, nullable=False) diff --git a/src/cleveragents/infrastructure/database/repositories.py b/src/cleveragents/infrastructure/database/repositories.py index 127cb0e0..53d1293c 100644 --- a/src/cleveragents/infrastructure/database/repositories.py +++ b/src/cleveragents/infrastructure/database/repositories.py @@ -1447,6 +1447,25 @@ class LifecyclePlanRepository: session.rollback() raise DatabaseError(f"Failed to update plan {plan_id_str}: {exc}") from exc + @database_retry + def list_all(self) -> list[Any]: + """List all persisted lifecycle plans. + + Returns: + List of ``Plan`` domain objects ordered by creation time + (newest first). + """ + session = self._session() + try: + rows = ( + session.query(LifecyclePlanModel) + .order_by(LifecyclePlanModel.created_at.desc()) + .all() + ) + return [r.to_domain() for r in rows] + except (OperationalError, SQLAlchemyDatabaseError) as exc: + raise DatabaseError(f"Failed to list plans: {exc}") from exc + @database_retry def delete(self, plan_id: str) -> bool: """Delete a plan by ULID, cascading to child tables. @@ -4223,11 +4242,29 @@ class AutomationProfileRepository: Uses the session-factory pattern consistent with other repositories. Built-in profiles are NOT stored here. + + All mutating methods flush but do NOT commit by default; the caller + or a ``UnitOfWork`` wrapper is responsible for committing the + transaction. When ``auto_commit`` is ``True`` (e.g. CLI usage + outside a UoW), each method commits and closes its own session. """ - def __init__(self, session_factory: Callable[[], Session]) -> None: - """Initialise with a callable that returns a Session.""" + def __init__( + self, + session_factory: Callable[[], Session], + *, + auto_commit: bool = False, + ) -> None: + """Initialise with a callable that returns a new SQLAlchemy Session. + + Args: + session_factory: Factory returning a new SQLAlchemy ``Session``. + auto_commit: When ``True``, each public method commits and + closes its session automatically. Useful for CLI commands + that operate outside a ``UnitOfWork``. + """ self._session_factory = session_factory + self._auto_commit = auto_commit def _session(self) -> Session: """Convenience helper to obtain a session.""" @@ -4319,6 +4356,8 @@ class AutomationProfileRepository: row = self._from_domain(profile, now_iso) session.add(row) session.flush() + if self._auto_commit: + session.commit() except AutomationProfileSchemaVersionError: raise except IntegrityError as exc: @@ -4353,6 +4392,8 @@ class AutomationProfileRepository: raise AutomationProfileNotFoundError(f"Profile '{name}' not found") session.delete(row) session.flush() + if self._auto_commit: + session.commit() except AutomationProfileNotFoundError: raise except ( @@ -4367,11 +4408,38 @@ class AutomationProfileRepository: row: AutomationProfileModel, ) -> Any: """Convert an ORM row to an AutomationProfile domain object.""" + import json as _json + from cleveragents.domain.models.core.automation_profile import ( + AutomationGuard, AutomationProfile, ) from cleveragents.domain.models.core.safety_profile import SafetyProfile + # Restore full safety from JSON if available, else fall back to + # legacy scalar columns for backward compatibility. + safety_json_str = ( + cast(str | None, row.safety_json) if hasattr(row, "safety_json") else None + ) + if safety_json_str: + safety = SafetyProfile(**_json.loads(safety_json_str)) + else: + safety = SafetyProfile( + require_sandbox=bool(row.require_sandbox), + require_checkpoints=bool(row.require_checkpoints), + require_human_approval=False, + allow_unsafe_tools=bool(row.allow_unsafe_tools), + max_retries_per_step=3, + ) + + # Restore guards from JSON if available. + guards_json_str = ( + cast(str | None, row.guards_json) if hasattr(row, "guards_json") else None + ) + guards: AutomationGuard | None = None + if guards_json_str: + guards = AutomationGuard(**_json.loads(guards_json_str)) + return AutomationProfile( name=cast(str, row.name), description=cast(str, row.description) or "", @@ -4387,18 +4455,8 @@ class AutomationProfileRepository: auto_child_plans=float(cast(float, row.auto_child_plans)), auto_retry_transient=float(cast(float, row.auto_retry_transient)), auto_checkpoint_restore=float(cast(float, row.auto_checkpoint_restore)), - # NOTE: require_human_approval and max_retries_per_step are not - # persisted in the automation_profiles table (only the original - # three safety booleans are stored). Defaults are used here for - # backwards compatibility. See c4_001_safety_profile_column for - # the Action-level safety_profile_json column. - safety=SafetyProfile( - require_sandbox=bool(row.require_sandbox), - require_checkpoints=bool(row.require_checkpoints), - require_human_approval=False, - allow_unsafe_tools=bool(row.allow_unsafe_tools), - max_retries_per_step=3, - ), + safety=safety, + guards=guards, ) @staticmethod @@ -4407,6 +4465,15 @@ class AutomationProfileRepository: now_iso: str, ) -> AutomationProfileModel: """Convert an AutomationProfile to an ORM row.""" + import json as _json + + safety_json = ( + _json.dumps(profile.safety.model_dump()) if profile.safety else None + ) + guards_json = ( + _json.dumps(profile.guards.model_dump()) if profile.guards else None + ) + return AutomationProfileModel( name=profile.name, description=profile.description, @@ -4425,6 +4492,8 @@ class AutomationProfileRepository: require_sandbox=profile.safety.require_sandbox, require_checkpoints=profile.safety.require_checkpoints, allow_unsafe_tools=profile.safety.allow_unsafe_tools, + safety_json=safety_json, + guards_json=guards_json, created_at=now_iso, updated_at=now_iso, ) @@ -4482,6 +4551,14 @@ class AutomationProfileRepository: row.allow_unsafe_tools = ( # type: ignore[assignment] profile.safety.allow_unsafe_tools ) + import json as _json + + row.safety_json = ( # type: ignore[assignment] + _json.dumps(profile.safety.model_dump()) if profile.safety else None + ) + row.guards_json = ( # type: ignore[assignment] + _json.dumps(profile.guards.model_dump()) if profile.guards else None + ) row.updated_at = now_iso # type: ignore[assignment]